-
Notifications
You must be signed in to change notification settings - Fork 313
Inherit default values for EntityCacheOption.Enabled and EntityCacheOption.Level from RuntimeCacheOptions #3155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
aaronburtle
wants to merge
7
commits into
main
Choose a base branch
from
dev/aaronburtle/InheritEntityCacheEnabldedAndLevelFromRuntimeSetting
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+189
−20
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
340ef73
take default for entity cache enabled and level from runtime cache se…
aaronburtle 44c8335
include test for new behavior
aaronburtle eea3044
addressing comments
aaronburtle 5649f2b
merge conflicts
aaronburtle 07a2bd9
revert accidental change
aaronburtle 6039a1e
addressing comments
aaronburtle d808b30
cleanup
aaronburtle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -565,7 +565,8 @@ public virtual int GetEntityCacheEntryTtl(string entityName) | |
|
|
||
| /// <summary> | ||
| /// Returns the cache level value for a given entity. | ||
| /// If the property is not set, returns the default (L1L2) for a given entity. | ||
| /// If the entity explicitly sets level, that value is used. | ||
| /// Otherwise, the level is inferred from the runtime cache Level2 configuration. | ||
| /// </summary> | ||
| /// <param name="entityName">Name of the entity to check cache configuration.</param> | ||
| /// <returns>Cache level that a cache entry should be stored in.</returns> | ||
|
|
@@ -592,36 +593,79 @@ public virtual EntityCacheLevel GetEntityCacheEntryLevel(string entityName) | |
| { | ||
| return entityConfig.Cache.Level.Value; | ||
| } | ||
| else | ||
| { | ||
| return EntityCacheLevel.L1L2; | ||
| } | ||
|
|
||
| return GlobalCacheEntryLevel(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Whether the caching service should be used for a given operation. This is determined by | ||
| /// - whether caching is enabled globally | ||
| /// - whether the datasource is SQL and session context is disabled. | ||
| /// Determines whether caching is enabled for a given entity, taking into account | ||
| /// inheritance from the runtime cache settings. | ||
| /// If the entity explicitly sets Enabled (Enabled.HasValue is true), that value is used. | ||
| /// If the entity does not set Enabled (Enabled is null), the runtime cache enabled value is inherited. | ||
| /// Using Enabled.HasValue instead of a separate UserProvided flag ensures correctness | ||
| /// regardless of whether the object was created via JsonConstructor or with-expression. | ||
| /// </summary> | ||
| /// <returns>Whether cache operations should proceed.</returns> | ||
| public virtual bool CanUseCache() | ||
| /// <param name="entityName">Name of the entity to check cache configuration.</param> | ||
| /// <returns>Whether caching is enabled for the entity.</returns> | ||
| /// <exception cref="DataApiBuilderException">Raised when an invalid entity name is provided.</exception> | ||
| public virtual bool IsEntityCachingEnabled(string entityName) | ||
| { | ||
| bool setSessionContextEnabled = DataSource.GetTypedOptions<MsSqlOptions>()?.SetSessionContext ?? true; | ||
| return IsCachingEnabled && !setSessionContextEnabled; | ||
| if (!Entities.TryGetValue(entityName, out Entity? entityConfig)) | ||
| { | ||
| throw new DataApiBuilderException( | ||
| message: $"{entityName} is not a valid entity.", | ||
| statusCode: HttpStatusCode.BadRequest, | ||
| subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); | ||
| } | ||
|
|
||
| // If the entity explicitly set Enabled, use that value. | ||
| if (entityConfig.Cache is not null && entityConfig.Cache.Enabled.HasValue) | ||
| { | ||
| return entityConfig.Cache.Enabled.Value; | ||
| } | ||
|
|
||
| // Otherwise, inherit from the runtime cache enabled setting. | ||
| return Runtime?.Cache?.Enabled is true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns the ttl-seconds value for the global cache entry. | ||
| /// If no value is explicitly set, returns the global default value. | ||
| /// </summary> | ||
| /// <returns>Number of seconds a cache entry should be valid before cache eviction.</returns> | ||
| public int GlobalCacheEntryTtl() | ||
| public virtual int GlobalCacheEntryTtl() | ||
| { | ||
| return Runtime is not null && Runtime.IsCachingEnabled && Runtime.Cache.UserProvidedTtlOptions | ||
| ? Runtime.Cache.TtlSeconds.Value | ||
| : EntityCacheOptions.DEFAULT_TTL_SECONDS; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns the cache level value for the global cache entry. | ||
| /// The level is inferred from the runtime cache Level2 configuration: | ||
| /// if Level2 is enabled, the level is L1L2; otherwise L1. | ||
| /// If runtime cache is not configured, the default cache level is used. | ||
| /// </summary> | ||
| /// <returns>Cache level that a cache entry should be stored in.</returns> | ||
| public virtual EntityCacheLevel GlobalCacheEntryLevel() | ||
| { | ||
| return Runtime?.Cache is not null | ||
| ? Runtime.Cache.InferredLevel | ||
| : EntityCacheOptions.DEFAULT_LEVEL; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Default cache level when level 2 Cache section is NOT specified in runtime is L1 (as is evident from the InferredCacheLevel). But for EntityCacheOptions.DEFAULT_LEVEL it seems to be L1L2. Is that accurate and what we want? |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// Whether the caching service should be used for a given operation. This is determined by | ||
| /// - whether caching is enabled globally | ||
| /// - whether the datasource is SQL and session context is disabled. | ||
| /// </summary> | ||
| /// <returns>Whether cache operations should proceed.</returns> | ||
| public virtual bool CanUseCache() | ||
| { | ||
| bool setSessionContextEnabled = DataSource.GetTypedOptions<MsSqlOptions>()?.SetSessionContext ?? true; | ||
| return IsCachingEnabled && !setSessionContextEnabled; | ||
| } | ||
|
|
||
| private void CheckDataSourceNamePresent(string dataSourceName) | ||
| { | ||
| if (!_dataSourceNameToDataSource.ContainsKey(dataSourceName)) | ||
|
|
||
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dont we need the "UserProvidedxyz" logic to Enabled?