Refactor cache initialization to accept CacheManagerInterface for improved cache management#2474
Open
KaveeshaPiumini wants to merge 2 commits intoasgardeo:mainfrom
Open
Refactor cache initialization to accept CacheManagerInterface for improved cache management#2474KaveeshaPiumini wants to merge 2 commits intoasgardeo:mainfrom
KaveeshaPiumini wants to merge 2 commits intoasgardeo:mainfrom
Conversation
KaveeshaPiumini
requested review from
ThaminduDilshan,
darshanasbg,
hwupathum,
rajithacharith and
thiva-k
as code owners
📝 WalkthroughWalkthroughRefactors cache construction to use injected CacheManager across services and cache-backed stores; updates flow execution to accept an injected runtime crypto provider, changes flow context APIs to use FlowContextDB, and moves encryption duties into the flowexec service. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant FlowSvc as FlowExecService
participant Crypto as RuntimeCryptoSvc
participant Store as FlowStore (DB/Redis)
participant CacheMgr as CacheManager
rect rgba(0,128,255,0.5)
Client->>FlowSvc: Start/continue flow (EngineContext)
end
FlowSvc->>FlowSvc: FromEngineContext -> FlowContextDB
FlowSvc->>Crypto: Encrypt(FlowContextDB.Context)
Crypto-->>FlowSvc: ciphertext
FlowSvc->>Store: StoreFlowContext(FlowContextDB with ciphertext)
Store-->>FlowSvc: ack
Client->>FlowSvc: Retrieve flow
FlowSvc->>Store: GetFlowContext(executionID)
Store-->>FlowSvc: FlowContextDB (maybe ciphertext)
FlowSvc->>FlowSvc: detect encrypted?
FlowSvc->>Crypto: Decrypt(ciphertext) (if encrypted)
Crypto-->>FlowSvc: plaintext JSON
FlowSvc->>CacheMgr: use graph/cache as needed
FlowSvc->>Client: Execute/return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2474 +/- ##
==========================================
+ Coverage 85.97% 85.98% +0.01%
==========================================
Files 913 913
Lines 64543 64535 -8
==========================================
Hits 55492 55492
+ Misses 7046 7042 -4
+ Partials 2005 2001 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/internal/flow/core/graph_cache.go (1)
40-45: Update the constructor comment.
newGraphCacheno longer creates its own cache; it wraps the injected cache instance. The docstring should be adjusted so it doesn't mislead future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/core/graph_cache.go` around lines 40 - 45, Update the doc comment for newGraphCache to accurately describe behavior: state that newGraphCache returns a graphCache wrapper around the provided cache.CacheInterface[*graph] rather than creating a new in-memory cache; mention that it accepts an injected cache and returns a GraphCacheInterface backed by that cache (referencing newGraphCache, graphCache, GraphCacheInterface, and cache.CacheInterface[*graph]).backend/internal/flow/mgt/init.go (1)
94-95: Create flow caches only in modes that use cache-backed DB stores.
FlowByIDCacheandFlowByHandleCacheare currently created before the mode switch, so declarative mode allocates unused caches. Consider moving cache creation intocompositeanddefaultbranches only.♻️ Proposed refactor
- flowByIDCache := cache.GetCache[*CompleteFlowDefinition](cacheManager, "FlowByIDCache") - flowByHandleCache := cache.GetCache[*CompleteFlowDefinition](cacheManager, "FlowByHandleCache") - switch storeMode { case serverconst.StoreModeComposite: fileStore, _ := newFileBasedStore() + flowByIDCache := cache.GetCache[*CompleteFlowDefinition](cacheManager, "FlowByIDCache") + flowByHandleCache := cache.GetCache[*CompleteFlowDefinition](cacheManager, "FlowByHandleCache") dbStore, transactioner, err := newCacheBackedFlowStore(flowByIDCache, flowByHandleCache) if err != nil { return nil, nil, nil, err @@ default: + flowByIDCache := cache.GetCache[*CompleteFlowDefinition](cacheManager, "FlowByIDCache") + flowByHandleCache := cache.GetCache[*CompleteFlowDefinition](cacheManager, "FlowByHandleCache") store, transactioner, err := newCacheBackedFlowStore(flowByIDCache, flowByHandleCache) if err != nil { return nil, nil, nil, errAlso applies to: 100-101, 118-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/mgt/init.go` around lines 94 - 95, flowByIDCache and flowByHandleCache are being allocated unconditionally; move their creation into the branches that use cache-backed DB stores so declarative mode doesn't allocate unused caches. Remove the top-level calls to cache.GetCache[*CompleteFlowDefinition] that create flowByIDCache and flowByHandleCache and instead instantiate them inside the composite and default branches (the same branches that set up cache-backed DB stores), invoking cache.GetCache with the same args there; also update the other duplicated creations around the same region (the later occurrences analogous to lines 100-101 and 118-119) so all cache creation only happens when mode == "composite" or the default path that uses caches. Ensure any variables remain in scope or are declared conditionally to avoid use-before-declare errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/internal/flow/core/graph_cache.go`:
- Around line 40-45: Update the doc comment for newGraphCache to accurately
describe behavior: state that newGraphCache returns a graphCache wrapper around
the provided cache.CacheInterface[*graph] rather than creating a new in-memory
cache; mention that it accepts an injected cache and returns a
GraphCacheInterface backed by that cache (referencing newGraphCache, graphCache,
GraphCacheInterface, and cache.CacheInterface[*graph]).
In `@backend/internal/flow/mgt/init.go`:
- Around line 94-95: flowByIDCache and flowByHandleCache are being allocated
unconditionally; move their creation into the branches that use cache-backed DB
stores so declarative mode doesn't allocate unused caches. Remove the top-level
calls to cache.GetCache[*CompleteFlowDefinition] that create flowByIDCache and
flowByHandleCache and instead instantiate them inside the composite and default
branches (the same branches that set up cache-backed DB stores), invoking
cache.GetCache with the same args there; also update the other duplicated
creations around the same region (the later occurrences analogous to lines
100-101 and 118-119) so all cache creation only happens when mode == "composite"
or the default path that uses caches. Ensure any variables remain in scope or
are declared conditionally to avoid use-before-declare errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04df9ef4-4e09-4047-a4fe-199e6ffba43b
📒 Files selected for processing (19)
backend/cmd/server/servicemanager.gobackend/internal/application/cache_backed_store.gobackend/internal/application/init.gobackend/internal/application/init_test.gobackend/internal/cert/cache_backed_store.gobackend/internal/cert/init.gobackend/internal/entity/cache_backed_store.gobackend/internal/entity/init.gobackend/internal/flow/core/graph_cache.gobackend/internal/flow/core/init.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/mgt/cache_backed_store.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/init_test.gobackend/internal/system/cache/manager.gobackend/internal/system/cache/manager_test.gobackend/internal/userschema/cache_backed_store.gobackend/internal/userschema/init.gobackend/internal/userschema/init_test.go
KaveeshaPiumini
force-pushed
the
cache
branch
2 times, most recently
from
c472650 to
c7ec027
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/flowexec/service_test.go`:
- Line 127: Tests currently discard the error returned by core.Initialize (e.g.,
the call assigning flowFactory, _ := core.Initialize(cache.GetCacheManager()) in
service_test.go), which can hide setup failures; update each occurrence to
capture the error (flowFactory, err := core.Initialize(...)) and fail the test
when err != nil (use t.Fatalf or require.NoError(t, err) depending on test
style) so initialization failures stop the test immediately and surface the real
cause.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ade56c81-20e8-4803-9a42-ab7909b4de25
📒 Files selected for processing (18)
backend/cmd/server/servicemanager.gobackend/internal/cert/cache_backed_store.gobackend/internal/cert/init.gobackend/internal/entity/cache_backed_store.gobackend/internal/entity/init.gobackend/internal/flow/core/graph_cache.gobackend/internal/flow/core/init.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/mgt/cache_backed_store.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/init_test.gobackend/internal/inboundclient/cache_backed_store.gobackend/internal/inboundclient/init.gobackend/internal/system/cache/manager.gobackend/internal/system/cache/manager_test.gobackend/internal/userschema/cache_backed_store.gobackend/internal/userschema/init.gobackend/internal/userschema/init_test.go
✅ Files skipped from review due to trivial changes (3)
- backend/internal/cert/cache_backed_store.go
- backend/internal/flow/mgt/init_test.go
- backend/internal/userschema/init_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/internal/userschema/cache_backed_store.go
- backend/internal/flow/core/init.go
- backend/cmd/server/servicemanager.go
- backend/internal/entity/init.go
- backend/internal/userschema/init.go
- backend/internal/system/cache/manager_test.go
| _ = config.InitializeThunderRuntime("/tmp/test", testConfig) | ||
|
|
||
| flowFactory, _ := core.Initialize() | ||
| flowFactory, _ := core.Initialize(cache.GetCacheManager()) |
There was a problem hiding this comment.
Handle core.Initialize errors in tests instead of discarding them.
Line 127 (and the same pattern at Lines 301, 522, 586, 649, 740, 799, 866) ignores err, which can mask setup failures and make follow-up assertions noisy.
✅ Suggested fix pattern
- flowFactory, _ := core.Initialize(cache.GetCacheManager())
+ flowFactory, err := core.Initialize(cache.GetCacheManager())
+ assert.NoError(t, err)Also applies to: 301-301, 522-522, 586-586, 649-649, 740-740, 799-799, 866-866
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/flow/flowexec/service_test.go` at line 127, Tests currently
discard the error returned by core.Initialize (e.g., the call assigning
flowFactory, _ := core.Initialize(cache.GetCacheManager()) in service_test.go),
which can hide setup failures; update each occurrence to capture the error
(flowFactory, err := core.Initialize(...)) and fail the test when err != nil
(use t.Fatalf or require.NoError(t, err) depending on test style) so
initialization failures stop the test immediately and surface the real cause.
3e3aee6 to
7702881
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/flow/flowexec/init.go (1)
37-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject a nil crypto provider during initialization.
cryptoSvcis now required on both the write path and the resume path. Acceptingnilhere turns a DI mistake into a request-time failure the first timeEncryptorDecryptis reached.Suggested guard
import ( + "errors" "net/http" @@ func Initialize( mux *http.ServeMux, flowMgtService flowmgt.FlowMgtServiceInterface, applicationService application.ApplicationServiceInterface, executorRegistry executor.ExecutorRegistryInterface, observabilitySvc observability.ObservabilityServiceInterface, cryptoSvc crypto.RuntimeCryptoProvider, ) (FlowExecServiceInterface, error) { + if cryptoSvc == nil { + return nil, errors.New("flowexec requires a runtime crypto provider") + } + var flowStore flowStoreInterface var transactioner transaction.Transactioner🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/init.go` around lines 37 - 44, In Initialize add a nil-guard for cryptoSvc so initialization fails fast: inside Initialize (the function that returns FlowExecServiceInterface) check if cryptoSvc == nil and return a descriptive error (e.g., using fmt.Errorf or errors.New) instead of proceeding; this ensures any DI mistake surfaces at init time rather than when Encrypt/Decrypt are first called.
🧹 Nitpick comments (2)
backend/internal/flow/core/init.go (1)
25-28: Pass the typed graph cache directly instead of the manager.This initializer accepts
cache.CacheManagerInterfacebut only uses it to callGetInMemoryCache[*graph](cacheManager, "FlowGraphCache"). Passing the typed cache directly keeps the dependency explicit and avoids unnecessarily exposing a manager interface with unexported methods across package boundaries, which limits testability for external callers.All call sites (
servicemanager.go:244and test files) already have access to the cache manager and can invokeGetInMemoryCachebefore passing the result toInitialize.♻️ Suggested refactor
-func Initialize(cacheManager cache.CacheManagerInterface) (FlowFactoryInterface, GraphCacheInterface) { +func Initialize(graphCacheInst cache.CacheInterface[*graph]) (FlowFactoryInterface, GraphCacheInterface) { flowFactory := newFlowFactory() - graphCacheInst := cache.GetInMemoryCache[*graph](cacheManager, "FlowGraphCache") graphCache := newGraphCache(graphCacheInst) return flowFactory, graphCache }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/core/init.go` around lines 25 - 28, The Initialize function currently takes cache.CacheManagerInterface only to call GetInMemoryCache[*graph](...), so change Initialize to accept the typed in-memory cache returned by GetInMemoryCache[*graph] (the cache instance you pass into newGraphCache) instead of cache.CacheManagerInterface; update the signature for Initialize and its uses so callers call GetInMemoryCache[*graph](...) themselves and pass that result into Initialize, leaving newFlowFactory and newGraphCache usage unchanged and preserving return types FlowFactoryInterface and GraphCacheInterface.backend/internal/flow/flowexec/store_test.go (1)
419-425: ⚡ Quick winRename this test to match the new responsibility split.
These steps now verify plain JSON serialization, but the surrounding test still reads as an encryption round-trip. That makes the suite imply the store layer still owns confidentiality, even though
backend/internal/flow/flowexec/service.gonow does the encrypt/decrypt work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/store_test.go` around lines 419 - 425, The test currently exercising FromEngineContext and getContextContent is verifying plain JSON serialization but is named/described as an encryption round-trip; rename the test function and any top-level descriptive comments to reflect the new responsibility split (e.g., rename to TestFromEngineContext_SerializesPlainJSON or TestContextSerialization_PlainJSON) and update any inline comments to state that FromEngineContext serializes context to plain JSON (encryption/decryption is handled in flowexec/service.go) so the test suite no longer implies the store layer manages confidentiality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/internal/flow/flowexec/init.go`:
- Around line 37-44: In Initialize add a nil-guard for cryptoSvc so
initialization fails fast: inside Initialize (the function that returns
FlowExecServiceInterface) check if cryptoSvc == nil and return a descriptive
error (e.g., using fmt.Errorf or errors.New) instead of proceeding; this ensures
any DI mistake surfaces at init time rather than when Encrypt/Decrypt are first
called.
---
Nitpick comments:
In `@backend/internal/flow/core/init.go`:
- Around line 25-28: The Initialize function currently takes
cache.CacheManagerInterface only to call GetInMemoryCache[*graph](...), so
change Initialize to accept the typed in-memory cache returned by
GetInMemoryCache[*graph] (the cache instance you pass into newGraphCache)
instead of cache.CacheManagerInterface; update the signature for Initialize and
its uses so callers call GetInMemoryCache[*graph](...) themselves and pass that
result into Initialize, leaving newFlowFactory and newGraphCache usage unchanged
and preserving return types FlowFactoryInterface and GraphCacheInterface.
In `@backend/internal/flow/flowexec/store_test.go`:
- Around line 419-425: The test currently exercising FromEngineContext and
getContextContent is verifying plain JSON serialization but is named/described
as an encryption round-trip; rename the test function and any top-level
descriptive comments to reflect the new responsibility split (e.g., rename to
TestFromEngineContext_SerializesPlainJSON or TestContextSerialization_PlainJSON)
and update any inline comments to state that FromEngineContext serializes
context to plain JSON (encryption/decryption is handled in flowexec/service.go)
so the test suite no longer implies the store layer manages confidentiality.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: faaa083c-725d-446b-b22a-e50e75d29c3e
⛔ Files ignored due to path filters (1)
backend/tests/mocks/flow/flowexecmock/flowStoreInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (29)
backend/cmd/server/servicemanager.gobackend/internal/cert/cache_backed_store.gobackend/internal/cert/init.gobackend/internal/entity/cache_backed_store.gobackend/internal/entity/init.gobackend/internal/flow/core/graph_cache.gobackend/internal/flow/core/init.gobackend/internal/flow/flowexec/flowStoreInterface_mock_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model_test.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/flow/flowexec/redis_store_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/flowexec/store.gobackend/internal/flow/flowexec/store_test.gobackend/internal/flow/mgt/cache_backed_store.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/init_test.gobackend/internal/inboundclient/cache_backed_store.gobackend/internal/inboundclient/init.gobackend/internal/system/cache/manager.gobackend/internal/system/cache/manager_test.gobackend/internal/system/crypto/runtime/service.gobackend/internal/system/crypto/runtime/service_test.gobackend/internal/userschema/cache_backed_store.gobackend/internal/userschema/init.gobackend/internal/userschema/init_test.go
💤 Files with no reviewable changes (1)
- backend/internal/system/crypto/runtime/service_test.go
✅ Files skipped from review due to trivial changes (2)
- backend/internal/flow/core/graph_cache.go
- backend/internal/flow/mgt/init_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/internal/inboundclient/cache_backed_store.go
- backend/internal/flow/mgt/cache_backed_store.go
- backend/internal/flow/flowexec/service_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/internal/system/cache/manager_test.go (1)
155-178: ⚡ Quick winAdd one test for cache-manager isolation (true DI behavior).
All updated calls pass the singleton via
GetCacheManager(), so the new injectable-manager contract isn’t explicitly validated. Please add a test that uses two distinctCacheManagerinstances and asserts same(type, cacheName)resolves to different cache instances across managers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/system/cache/manager_test.go` around lines 155 - 178, Add a test that verifies DI isolation by creating two distinct CacheManager instances (instead of using GetCacheManager()) and asserting that calling GetCache[T](mgrA, cacheName) and GetCache[T](mgrB, cacheName) for the same type T and cacheName returns different cache instances; locate this alongside TestGetCacheMultipleTypes and use the existing GetCache and CacheManager symbols to create mgrA and mgrB, fetch caches for the same (type, cacheName) pair from each, and assert they are not the same.backend/internal/flow/flowexec/service.go (1)
604-610: ⚡ Quick winAvoid logging this 500-path in the service layer.
This branch already returns
InternalServerError; logging here will duplicate the handler-side server-error log and add noise around decryption failures.Based on learnings, in
backend/internal, server errors are logged at the handler layer, while services return aServiceErrorwithout logging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/service.go` around lines 604 - 610, Remove the duplicate service-layer log in the decryption failure path: inside the block that checks isContextEncrypted(dbModel.Context) where s.cryptoSvc.Decrypt(...) is called, delete the logger.Error(...) call and simply return the service error (currently &serviceerror.InternalServerError) without logging; leave the decryption call and the error return intact so the handler layer can perform the server-error logging per convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/internal/flow/flowexec/service.go`:
- Around line 604-610: Remove the duplicate service-layer log in the decryption
failure path: inside the block that checks isContextEncrypted(dbModel.Context)
where s.cryptoSvc.Decrypt(...) is called, delete the logger.Error(...) call and
simply return the service error (currently &serviceerror.InternalServerError)
without logging; leave the decryption call and the error return intact so the
handler layer can perform the server-error logging per convention.
In `@backend/internal/system/cache/manager_test.go`:
- Around line 155-178: Add a test that verifies DI isolation by creating two
distinct CacheManager instances (instead of using GetCacheManager()) and
asserting that calling GetCache[T](mgrA, cacheName) and GetCache[T](mgrB,
cacheName) for the same type T and cacheName returns different cache instances;
locate this alongside TestGetCacheMultipleTypes and use the existing GetCache
and CacheManager symbols to create mgrA and mgrB, fetch caches for the same
(type, cacheName) pair from each, and assert they are not the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2cb943b-e505-44e9-b058-8868d97824bc
📒 Files selected for processing (30)
backend/cmd/server/servicemanager.gobackend/internal/cert/cache_backed_store.gobackend/internal/cert/init.gobackend/internal/entity/cache_backed_store.gobackend/internal/entity/init.gobackend/internal/flow/core/graph_cache.gobackend/internal/flow/core/init.gobackend/internal/flow/flowexec/flowStoreInterface_mock_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model_test.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/flow/flowexec/redis_store_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/flowexec/store.gobackend/internal/flow/flowexec/store_test.gobackend/internal/flow/mgt/cache_backed_store.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/init_test.gobackend/internal/inboundclient/cache_backed_store.gobackend/internal/inboundclient/init.gobackend/internal/system/cache/manager.gobackend/internal/system/cache/manager_test.gobackend/internal/system/crypto/runtime/service.gobackend/internal/system/crypto/runtime/service_test.gobackend/internal/userschema/cache_backed_store.gobackend/internal/userschema/init.gobackend/internal/userschema/init_test.gobackend/tests/mocks/flow/flowexecmock/flowStoreInterface_mock.go
💤 Files with no reviewable changes (1)
- backend/internal/system/crypto/runtime/service_test.go
✅ Files skipped from review due to trivial changes (2)
- backend/internal/flow/mgt/init_test.go
- backend/cmd/server/servicemanager.go
🚧 Files skipped from review as they are similar to previous changes (10)
- backend/internal/system/crypto/runtime/service.go
- backend/internal/inboundclient/init.go
- backend/internal/flow/flowexec/init.go
- backend/internal/flow/flowexec/redis_store.go
- backend/internal/flow/flowexec/redis_store_test.go
- backend/internal/system/cache/manager.go
- backend/internal/flow/flowexec/store_test.go
- backend/internal/userschema/init_test.go
- backend/internal/flow/flowexec/flowStoreInterface_mock_test.go
- backend/internal/flow/flowexec/service_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request refactors the initialization and dependency injection of cache managers and cache-backed stores throughout the backend service layer. The main goal is to improve flexibility, testability, and consistency by explicitly passing cache manager dependencies rather than relying on global cache retrieval. Several service initializers and constructors are updated to receive cache manager or cache instances as parameters, and related tests are updated accordingly.
Key changes include:
Cache Manager Dependency Injection:
Updated service initializers for application, entity, certificate, and flow management to accept a
cache.CacheManagerInterfaceparameter, enabling explicit cache manager injection and improving testability. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Constructors for cache-backed stores in application, entity, and certificate modules now require explicit cache instances instead of fetching them internally, further decoupling cache usage from store logic. [1] [2] [3] [4]
Test Refactoring:
Internal API Consistency:
These changes collectively improve the maintainability and flexibility of the codebase by making cache dependencies explicit and easier to manage.
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit