Skip to content

Commit 77b44a0

Browse files
aaronburtleCopilotAniruddh25
authored
Return default value when entity ttl is retrieved with entity cache enabled set to false (#3156)
## Why make this change? Closes #3148 Test change to help the pipeline pass relates to #2992 ## What is this change? Within the code paths for executing a REST request with the RuntimeCacheOptions enabled, we would attempt to get the entity cache ttl, however, if the entity had its cache options not enabled this would result in an exception, leading to a bad request returned to the user. However, the code path was already safeguarded by checking if we have entity cache enabled before executing a cache-based response, and so we did not need the exception when getting the entity cache ttl, and can safely return null to indicate that there is no cache ttl. ## How was this tested? A simple regression test is added that validates the new behavior. --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]>
1 parent b5841fd commit 77b44a0

4 files changed

Lines changed: 71 additions & 35 deletions

File tree

‎src/Config/ObjectModel/RuntimeConfig.cs‎

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,6 @@ public RuntimeConfig(
373373
}
374374

375375
SetupDataSourcesUsed();
376-
377376
}
378377

379378
/// <summary>
@@ -529,12 +528,13 @@ Runtime is not null && Runtime.Host is not null
529528

530529
/// <summary>
531530
/// Returns the ttl-seconds value for a given entity.
532-
/// If the property is not set, returns the global default value set in the runtime config.
533-
/// If the global default value is not set, the default value is used (5 seconds).
531+
/// If the entity explicitly sets ttl-seconds, that value is used.
532+
/// Otherwise, falls back to the global cache TTL setting.
533+
/// Callers are responsible for checking whether caching is enabled before using the result.
534534
/// </summary>
535535
/// <param name="entityName">Name of the entity to check cache configuration.</param>
536536
/// <returns>Number of seconds (ttl) that a cache entry should be valid before cache eviction.</returns>
537-
/// <exception cref="DataApiBuilderException">Raised when an invalid entity name is provided or if the entity has caching disabled.</exception>
537+
/// <exception cref="DataApiBuilderException">Raised when an invalid entity name is provided.</exception>
538538
public virtual int GetEntityCacheEntryTtl(string entityName)
539539
{
540540
if (!Entities.TryGetValue(entityName, out Entity? entityConfig))
@@ -545,31 +545,23 @@ public virtual int GetEntityCacheEntryTtl(string entityName)
545545
subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound);
546546
}
547547

548-
if (!entityConfig.IsCachingEnabled)
549-
{
550-
throw new DataApiBuilderException(
551-
message: $"{entityName} does not have caching enabled.",
552-
statusCode: HttpStatusCode.BadRequest,
553-
subStatusCode: DataApiBuilderException.SubStatusCodes.NotSupported);
554-
}
555-
556-
if (entityConfig.Cache.UserProvidedTtlOptions)
548+
if (entityConfig.Cache is not null && entityConfig.Cache.UserProvidedTtlOptions)
557549
{
558550
return entityConfig.Cache.TtlSeconds.Value;
559551
}
560-
else
561-
{
562-
return GlobalCacheEntryTtl();
563-
}
552+
553+
return GlobalCacheEntryTtl();
564554
}
565555

566556
/// <summary>
567557
/// Returns the cache level value for a given entity.
568-
/// If the property is not set, returns the default (L1L2) for a given entity.
558+
/// If the entity explicitly sets level, that value is used.
559+
/// Otherwise, falls back to the global cache level or the default.
560+
/// Callers are responsible for checking whether caching is enabled before using the result.
569561
/// </summary>
570562
/// <param name="entityName">Name of the entity to check cache configuration.</param>
571563
/// <returns>Cache level that a cache entry should be stored in.</returns>
572-
/// <exception cref="DataApiBuilderException">Raised when an invalid entity name is provided or if the entity has caching disabled.</exception>
564+
/// <exception cref="DataApiBuilderException">Raised when an invalid entity name is provided.</exception>
573565
public virtual EntityCacheLevel GetEntityCacheEntryLevel(string entityName)
574566
{
575567
if (!Entities.TryGetValue(entityName, out Entity? entityConfig))
@@ -580,22 +572,12 @@ public virtual EntityCacheLevel GetEntityCacheEntryLevel(string entityName)
580572
subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound);
581573
}
582574

583-
if (!entityConfig.IsCachingEnabled)
584-
{
585-
throw new DataApiBuilderException(
586-
message: $"{entityName} does not have caching enabled.",
587-
statusCode: HttpStatusCode.BadRequest,
588-
subStatusCode: DataApiBuilderException.SubStatusCodes.NotSupported);
589-
}
590-
591-
if (entityConfig.Cache.UserProvidedLevelOptions)
575+
if (entityConfig.Cache is not null && entityConfig.Cache.UserProvidedLevelOptions)
592576
{
593577
return entityConfig.Cache.Level.Value;
594578
}
595-
else
596-
{
597-
return EntityCacheLevel.L1L2;
598-
}
579+
580+
return EntityCacheOptions.DEFAULT_LEVEL;
599581
}
600582

601583
/// <summary>

‎src/Service.Tests/Caching/CachingConfigProcessingTests.cs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,4 +416,48 @@ private static string GetRawConfigJson(string globalCacheConfig, string entityCa
416416

417417
return expectedRuntimeConfigJson.ToString();
418418
}
419+
420+
/// <summary>
421+
/// Regression test: Validates that when global runtime cache is enabled but entity cache is disabled,
422+
/// GetEntityCacheEntryTtl and GetEntityCacheEntryLevel do not throw and return sensible defaults.
423+
/// Previously, these methods threw a DataApiBuilderException (BadRequest/NotSupported) when the entity
424+
/// had caching disabled, which caused 400 errors for valid requests when the global cache was enabled.
425+
/// These methods are now pure accessors that always return a value regardless of cache enablement.
426+
/// </summary>
427+
/// <param name="globalCacheConfig">Global cache configuration JSON fragment.</param>
428+
/// <param name="entityCacheConfig">Entity cache configuration JSON fragment.</param>
429+
/// <param name="expectedTtl">Expected TTL returned by GetEntityCacheEntryTtl.</param>
430+
/// <param name="expectedLevel">Expected cache level returned by GetEntityCacheEntryLevel.</param>
431+
[DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": 10 }", @",""cache"": { ""enabled"": false }", 10, EntityCacheLevel.L1L2, DisplayName = "Global cache enabled with custom TTL, entity cache disabled: entity returns global TTL and default level.")]
432+
[DataRow(@",""cache"": { ""enabled"": true }", @",""cache"": { ""enabled"": false }", 5, EntityCacheLevel.L1L2, DisplayName = "Global cache enabled with default TTL, entity cache disabled: entity returns default TTL and default level.")]
433+
[DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": 10 }", @"", 10, EntityCacheLevel.L1L2, DisplayName = "Global cache enabled with custom TTL, entity cache omitted: entity returns global TTL and default level.")]
434+
[DataTestMethod]
435+
public void GetEntityCacheEntryTtlAndLevel_DoesNotThrow_WhenRuntimeCacheEnabledAndEntityCacheDisabled(
436+
string globalCacheConfig,
437+
string entityCacheConfig,
438+
int expectedTtl,
439+
EntityCacheLevel expectedLevel)
440+
{
441+
// Arrange
442+
string fullConfig = GetRawConfigJson(globalCacheConfig: globalCacheConfig, entityCacheConfig: entityCacheConfig);
443+
RuntimeConfigLoader.TryParseConfig(
444+
json: fullConfig,
445+
out RuntimeConfig? config,
446+
replacementSettings: null);
447+
448+
Assert.IsNotNull(config, message: "Config must not be null, runtime config JSON deserialization failed.");
449+
Assert.IsTrue(config.IsCachingEnabled, message: "Global caching should be enabled for this test scenario.");
450+
451+
Entity entity = config.Entities.First().Value;
452+
Assert.IsFalse(entity.IsCachingEnabled, message: "Entity caching should be disabled for this test scenario.");
453+
454+
string entityName = config.Entities.First().Key;
455+
456+
// Act & Assert - These calls must not throw.
457+
int actualTtl = config.GetEntityCacheEntryTtl(entityName);
458+
EntityCacheLevel actualLevel = config.GetEntityCacheEntryLevel(entityName);
459+
460+
Assert.AreEqual(expected: expectedTtl, actual: actualTtl, message: "GetEntityCacheEntryTtl should return the global/default TTL when entity cache is disabled.");
461+
Assert.AreEqual(expected: expectedLevel, actual: actualLevel, message: "GetEntityCacheEntryLevel should return the default level when entity cache is disabled.");
462+
}
419463
}

‎src/Service.Tests/Configuration/ConfigurationTests.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2903,7 +2903,7 @@ public async Task ValidateErrorMessageForMutationWithoutReadPermission()
29032903
}";
29042904
string queryName = "stock_by_pk";
29052905

2906-
ValidateMutationSucceededAtDbLayer(server, client, graphQLQuery, queryName, authToken, AuthorizationResolver.ROLE_AUTHENTICATED);
2906+
await ValidateMutationSucceededAtDbLayer(server, client, graphQLQuery, queryName, authToken, AuthorizationResolver.ROLE_AUTHENTICATED);
29072907
}
29082908
finally
29092909
{
@@ -3225,7 +3225,7 @@ public async Task ValidateInheritanceOfReadPermissionFromAnonymous()
32253225
/// <param name="query">GraphQL query/mutation text</param>
32263226
/// <param name="queryName">GraphQL query/mutation name</param>
32273227
/// <param name="authToken">Auth token for the graphQL request</param>
3228-
private static async void ValidateMutationSucceededAtDbLayer(TestServer server, HttpClient client, string query, string queryName, string authToken, string clientRoleHeader)
3228+
private static async Task ValidateMutationSucceededAtDbLayer(TestServer server, HttpClient client, string query, string queryName, string authToken, string clientRoleHeader)
32293229
{
32303230
JsonElement queryResponse = await GraphQLRequestExecutor.PostGraphQLRequestAsync(
32313231
client,
@@ -3237,6 +3237,7 @@ private static async void ValidateMutationSucceededAtDbLayer(TestServer server,
32373237
clientRoleHeader: clientRoleHeader);
32383238

32393239
Assert.IsNotNull(queryResponse);
3240+
Assert.AreNotEqual(JsonValueKind.Null, queryResponse.ValueKind, "Expected a JSON object response but received null.");
32403241
Assert.IsFalse(queryResponse.TryGetProperty("errors", out _));
32413242
}
32423243

‎src/Service.Tests/Configuration/Telemetry/AzureLogAnalyticsTests.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,18 @@ public async Task TestAzureLogAnalyticsFlushServiceSucceed(string message, LogLe
120120

121121
_ = Task.Run(() => flusherService.StartAsync(tokenSource.Token));
122122

123-
await Task.Delay(2000);
123+
// Poll until the log appears (the flusher service needs time to dequeue and upload)
124+
int maxWaitMs = 10000;
125+
int pollIntervalMs = 100;
126+
int elapsed = 0;
127+
while (customClient.LogAnalyticsLogs.Count == 0 && elapsed < maxWaitMs)
128+
{
129+
await Task.Delay(pollIntervalMs);
130+
elapsed += pollIntervalMs;
131+
}
124132

125133
// Assert
134+
Assert.IsTrue(customClient.LogAnalyticsLogs.Count > 0, $"Expected at least one log entry after waiting {elapsed}ms, but found none.");
126135
AzureLogAnalyticsLogs actualLog = customClient.LogAnalyticsLogs[0];
127136
Assert.AreEqual(logLevel.ToString(), actualLog.LogLevel);
128137
Assert.AreEqual(message, actualLog.Message);

0 commit comments

Comments
 (0)