Cache Prime Fix: Reuse entity factory for cache prime

[shubham1g5]

Technical Summary

https://dimagi.atlassian.net/browse/SC-4402

This is necessary to be able to populate cache correctly in primeCache as otherwise we don't have values like mTemplateIsCachable and mCacheHost correctly initialized which results into the primeCache method to exit early and not populate cache into memory resulting into a performance hit.

Feature Flag

Cache and Lazy Load

Safety Assurance

Safety story

The change allows us to reuse the factory class which should be harmless for normal use cases that doesn't involve entity caching. The pass on integration test should confirm this further.

QA Plan

None

Labels and Review

• Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
• Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request propagates a new factory parameter through various caching and entity loading operations. In the AndroidAsyncNodeEntityFactory.kt file, two calls to primeEntityCacheForDetail are modified to include the current instance (this). In the EntityLoaderHelper.kt, a nullable factory parameter is added to its constructor, and subsequent calls within the class now use a non-null assertion on this parameter. In EntityLoaderTask.java, the constructor is updated to pass an additional parameter (a null factory) when instantiating EntityLoaderHelper. Finally, changes in the PrimeEntityCacheHelper.kt introduce an optional factory argument to both the primeEntityCacheForDetail and primeCacheForDetail methods. This set of changes standardizes and extends the injection and usage of factory instances across methods, without altering the overall control flow or logical conditions in these classes.

Sequence Diagram(s)

sequenceDiagram
    participant AA as AndroidAsyncNodeEntityFactory
    participant PEC as PrimeEntityCacheHelper
    participant ELH as EntityLoaderHelper
    participant F as Factory Instance

    AA->>PEC: primeEntityCacheForDetail(..., factory: this)
    PEC->>PEC: Validate caching conditions
    PEC->>ELH: Instantiate EntityLoaderHelper(..., factory)
    ELH->>F: Invoke methods (expandReferenceList, prepareEntities, etc.)

Loading

sequenceDiagram
    participant ET as EntityLoaderTask
    participant ELH as EntityLoaderHelper
    participant F as Factory Instance

    ET->>ELH: Construct EntityLoaderHelper(..., factory: null)
    ELH->>ELH: If factory is null, execute local instantiation logic

Loading

Possibly related PRs

• [No op] Refactor entityLoaderTask to use a helper class #2907: Adjustments in entity factory usage and caching logic in primeEntityCacheForDetail mirror changes in this PR.
• MInor Tweaks for Cache and Lazy Load #2982: Modifications to the primeEntityCacheForDetail method signature directly relate to the updated parameter passing in this PR.
• Fixes for Entity Cache and Lazy loading work #2998: Similar updates to the method signature of primeEntityCacheForDetail in PrimeEntityCacheHelper provide a direct code-level connection.

Suggested labels

skip-integration-tests

Suggested reviewers

• OrangeAndGreen
• avazirna
• pm-dimagi
• ctsims

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3600813 and f5d2a67.

📒 Files selected for processing (4)

• app/src/org/commcare/entity/AndroidAsyncNodeEntityFactory.kt (2 hunks)
• app/src/org/commcare/tasks/EntityLoaderHelper.kt (5 hunks)
• app/src/org/commcare/tasks/EntityLoaderTask.java (1 hunks)
• app/src/org/commcare/tasks/PrimeEntityCacheHelper.kt (3 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (2)

app/src/org/commcare/tasks/EntityLoaderTask.java (2)

app/src/org/commcare/tasks/PrimeEntityCacheHelper.kt (2)

• entityLoaderHelper (36-256)
• evalCtx (212-214)

app/src/org/commcare/tasks/EntityLoaderHelper.kt (1)

• detail (23-131)

app/src/org/commcare/tasks/EntityLoaderHelper.kt (1)

app/src/org/commcare/models/database/user/models/CommCareEntityStorageCache.java (1)

• CommCareEntityStorageCache (42-438)

🔇 Additional comments (15)

app/src/org/commcare/tasks/EntityLoaderTask.java (1)

45-45: Updated EntityLoaderHelper constructor call to pass null factory parameter.

The constructor call now passes null for the newly added factory parameter, which is consistent with the changes in the EntityLoaderHelper class. This will allow EntityLoaderHelper to initialize the factory internally based on the detail configuration.

app/src/org/commcare/entity/AndroidAsyncNodeEntityFactory.kt (2)

56-62: Added 'this' as factory parameter to primeEntityCacheForDetail.

The this reference is now passed to primeEntityCacheForDetail, allowing the method to reuse the current factory instance. This is crucial for ensuring that necessary factory state (like mTemplateIsCachable and mCacheHost) is correctly propagated for proper cache population.

---

70-76: Added 'this' as factory parameter to primeEntityCacheForDetail.

Similarly to the above change, passing this ensures that the current factory state is preserved when priming the cache, preventing early exits from the primeCache method and ensuring proper cache population.

app/src/org/commcare/tasks/EntityLoaderHelper.kt (7)

27-28: Added nullable factory parameter to EntityLoaderHelper constructor.

This change allows for an external factory to be passed to the EntityLoaderHelper, enabling state preservation across the caching pipeline. The parameter is marked as nullable with a default value of null, maintaining backward compatibility.

---

36-50: Modified factory initialization to conditionally create a new factory when null.

The initialization logic now checks if a factory was provided, and only creates a new one if needed. This allows for factory reuse while maintaining the previous behavior when no factory is provided. This change is essential for propagating the factory state through the cache priming process.

---

66-70: Updated factory method calls with non-null assertions.

The code now uses non-null assertions (!!) on the factory reference, as the initialization block ensures it will never be null. This is safe given the initialization logic, but requires that all uses of the class pass a non-null factory or allow it to be initialized internally.

---

86-87: Updated expandReferenceList call with non-null assertion.

Added non-null assertion on the factory reference for consistency with other method calls.

---

93-94: Updated cacheEntities call with non-null assertion.

Added non-null assertion on the factory reference for consistency with other method calls.

---

115-116: Updated getEntity call with non-null assertion.

Added non-null assertion on the factory reference for consistency with other method calls.

---

129-130: Updated markAsCancelled call with non-null assertion.

Added non-null assertion on the factory reference for consistency with other method calls.

app/src/org/commcare/tasks/PrimeEntityCacheHelper.kt (5)

18-18: Added import for AsyncNodeEntityFactory.

This import is required for the new factory parameter type in the method signatures.

---

135-137: Added optional factory parameter to primeEntityCacheForDetail method.

This change allows the AsyncNodeEntityFactory to be passed through to the caching process, ensuring that factory state is preserved. The parameter is optional with a default value of null, maintaining backward compatibility.

---

140-140: Updated primeCacheForDetail call to pass the factory parameter.

The method call now passes the factory parameter to the inner method, ensuring factory state is maintained throughout the caching process.

---

189-190: Added optional factory parameter to primeCacheForDetail method.

Similar to the public method, this internal method now accepts an optional factory parameter, allowing factory state to be preserved throughout the caching pipeline.

---

194-196: Updated EntityLoaderHelper initialization to use the provided factory.

The EntityLoaderHelper is now initialized with the factory passed from upstream, and sets the entity progress listener on the factory. The non-null assertion is safe as the factory will be initialized by EntityLoaderHelper if not provided.

✨ Finishing Touches

• 📝 Generate Docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai plan to trigger planning for file edits and PR creation.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[avazirna]

Makes sense to me.