Skip to content

Add header support for caching #2650

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
wants to merge 12 commits into
base: main
Choose a base branch
from
41 changes: 39 additions & 2 deletions src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ public class SqlQueryStructure : BaseSqlQueryStructure
/// </summary>
public GroupByMetadata GroupByMetadata { get; private set; }

public string? CacheControlOption { get; set; }

public const string CACHE_CONTROL = "Cache-Control";

public const string CACHE_CONTROL_NO_STORE = "no-store";

public const string CACHE_CONTROL_NO_CACHE = "no-cache";

public const string CACHE_CONTROL_ONLY_IF_CACHED = "only-if-cached";

public HashSet<string> cacheControlHeaderOptions = new(
new[] { CACHE_CONTROL, CACHE_CONTROL_NO_STORE, CACHE_CONTROL_NO_CACHE, CACHE_CONTROL_ONLY_IF_CACHED },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is CACHE_CONTROL i.e. Cache-Control itself a valid value of this option? IMO this should not be in the HashSet since you would be giving out an exception for this value if its actually an invalid value

If its valid, what does it mean?

StringComparer.OrdinalIgnoreCase);

/// <summary>
/// Generate the structure for a SQL query based on GraphQL query
/// information.
Expand Down Expand Up @@ -150,6 +164,7 @@ public SqlQueryStructure(
: this(sqlMetadataProvider,
authorizationResolver,
gQLFilterParser,
gQLFilterParser.GetHttpContextFromMiddlewareContext(ctx).Request.Headers,
predicates: null,
entityName: entityName,
counter: counter)
Expand Down Expand Up @@ -217,6 +232,7 @@ public SqlQueryStructure(
}

HttpContext httpContext = GraphQLFilterParser.GetHttpContextFromMiddlewareContext(ctx);

// Process Authorization Policy of the entity being processed.
AuthorizationPolicyHelpers.ProcessAuthorizationPolicies(EntityActionOperation.Read, queryStructure: this, httpContext, authorizationResolver, sqlMetadataProvider);

Expand Down Expand Up @@ -255,6 +271,7 @@ public SqlQueryStructure(
: this(sqlMetadataProvider,
authorizationResolver,
gQLFilterParser,
httpRequestHeaders: httpContext?.Request.Headers,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can httpContext be null here? If not, do we need httpContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be null here. For GraphQL read operation we intentionally leave this null, and then use that fact to sort out when to process authorization policies. This flow was causing a bunch of tests to fail when I was forcing an httpContext to always get passed through, which is why now I just grab the Headers, and allow the context to remain null when its passed in as such.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it could be null only for GraphQL read, we need not worried here, right? Since the constructor is actually for Rest only:

public SqlQueryStructure(
RestRequestContext context,
ISqlMetadataProvider sqlMetadataProvider,

also the argument to the constructor itself is not nullable

        **HttpContext httpContext)**

predicates: null,
entityName: context.EntityName,
counter: new IncrementingInteger(),
Expand Down Expand Up @@ -380,10 +397,10 @@ private SqlQueryStructure(
: this(sqlMetadataProvider,
authorizationResolver,
gQLFilterParser,
gQLFilterParser.GetHttpContextFromMiddlewareContext(ctx).Request.Headers,
predicates: null,
entityName: entityName,
counter: counter
)
counter: counter)
{
_ctx = ctx;
IOutputType outputType = schemaField.Type;
Expand Down Expand Up @@ -541,6 +558,7 @@ private SqlQueryStructure(
ISqlMetadataProvider metadataProvider,
IAuthorizationResolver authorizationResolver,
GQLFilterParser gQLFilterParser,
IHeaderDictionary? httpRequestHeaders,
List<Predicate>? predicates = null,
string entityName = "",
IncrementingInteger? counter = null,
Expand All @@ -559,6 +577,25 @@ private SqlQueryStructure(
ColumnLabelToParam = new();
FilterPredicates = string.Empty;
OrderByColumns = new();
AddCacheControlOptions(httpRequestHeaders);
}

private void AddCacheControlOptions(IHeaderDictionary? httpRequestHeaders)
{
// Set the cache control based on the request header if it exists.
if (httpRequestHeaders is not null && httpRequestHeaders.TryGetValue(CACHE_CONTROL, out Microsoft.Extensions.Primitives.StringValues cacheControlOption))
{
CacheControlOption = cacheControlOption;
}

if (!string.IsNullOrEmpty(CacheControlOption) &&
!cacheControlHeaderOptions.Contains(CacheControlOption))
{
throw new DataApiBuilderException(
message: "Request Header Cache-Control is invalid: " + CacheControlOption,
statusCode: HttpStatusCode.BadRequest,
subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
}
}

/// <summary>
Expand Down
50 changes: 46 additions & 4 deletions src/Core/Resolvers/SqlQueryEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Azure.DataApiBuilder.Core.Services;
using Azure.DataApiBuilder.Core.Services.Cache;
using Azure.DataApiBuilder.Core.Services.MetadataProviders;
using Azure.DataApiBuilder.Service.Exceptions;
using Azure.DataApiBuilder.Service.GraphQLBuilder;
using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries;
using HotChocolate.Resolvers;
Expand Down Expand Up @@ -329,10 +330,45 @@ public object ResolveList(JsonElement array, IObjectField fieldSchema, ref IMeta
if (!dbPolicyConfigured && entityCacheEnabled)
{
DatabaseQueryMetadata queryMetadata = new(queryText: queryString, dataSource: dataSourceName, queryParameters: structure.Parameters);
JsonElement result = await _cache.GetOrSetAsync<JsonElement>(queryExecutor, queryMetadata, cacheEntryTtl: runtimeConfig.GetEntityCacheEntryTtl(entityName: structure.EntityName));
byte[] jsonBytes = JsonSerializer.SerializeToUtf8Bytes(result);
JsonDocument cacheServiceResponse = JsonDocument.Parse(jsonBytes);
return cacheServiceResponse;
JsonElement? result;
switch (structure.CacheControlOption)
{
// Do not get result from cache even if it exists, still cache result.
case SqlQueryStructure.CACHE_CONTROL_NO_CACHE:
Copy link
Contributor

@Aniruddh25 Aniruddh25 Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some automated tests for these control options?

result = await queryExecutor.ExecuteQueryAsync(
sqltext: queryMetadata.QueryText,
parameters: queryMetadata.QueryParameters,
dataReaderHandler: queryExecutor.GetJsonResultAsync<JsonElement>,
httpContext: _httpContextAccessor.HttpContext!,
args: null,
dataSourceName: queryMetadata.DataSource);
_cache.Set<JsonElement?>(queryMetadata, cacheEntryTtl: runtimeConfig.GetEntityCacheEntryTtl(entityName: structure.EntityName), result);
return ParseResultIntoJsonDocument(result);
// Do not store result even if valid, stil get from cache if available.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add a new line before the comment

case SqlQueryStructure.CACHE_CONTROL_NO_STORE:
result = _cache.TryGet<JsonElement?>(queryMetadata);
result = result.HasValue ? result.Value :
await queryExecutor.ExecuteQueryAsync(
sqltext: queryMetadata.QueryText,
parameters: queryMetadata.QueryParameters,
dataReaderHandler: queryExecutor.GetJsonResultAsync<JsonElement>,
httpContext: _httpContextAccessor.HttpContext!,
args: null,
dataSourceName: queryMetadata.DataSource);
return ParseResultIntoJsonDocument(result);
// Only return query response if it exists in cache, return gateway timeout otherwise.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add new line before comment

case SqlQueryStructure.CACHE_CONTROL_ONLY_IF_CACHED:
result = _cache.TryGet<JsonElement?>(queryMetadata);
result = result.HasValue ? result :
throw new DataApiBuilderException(
message: "Header 'only-if-cached' was used but item was not found in cache.",
statusCode: System.Net.HttpStatusCode.GatewayTimeout,
subStatusCode: DataApiBuilderException.SubStatusCodes.ItemNotFound);
return ParseResultIntoJsonDocument(result);
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add new line before default

result = await _cache.GetOrSetAsync<JsonElement>(queryExecutor, queryMetadata, cacheEntryTtl: runtimeConfig.GetEntityCacheEntryTtl(entityName: structure.EntityName));
return ParseResultIntoJsonDocument(result);
Comment on lines +369 to +370
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused by this, shouldn't there be two different cases, one if CacheControlOption is null and one for CACHE_CONTROL?

}
}
}

Expand All @@ -351,6 +387,12 @@ public object ResolveList(JsonElement array, IObjectField fieldSchema, ref IMeta
return response;
}

private static JsonDocument? ParseResultIntoJsonDocument(JsonElement? result)
{
byte[] jsonBytes = JsonSerializer.SerializeToUtf8Bytes(result);
return JsonDocument.Parse(jsonBytes);
}

// <summary>
// Given the SqlExecuteStructure structure, obtains the query text and executes it against the backend.
// Unlike a normal query, result from database may not be JSON. Instead we treat output as SqlMutationEngine does (extract by row).
Expand Down
34 changes: 34 additions & 0 deletions src/Core/Services/Cache/DabCacheService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,40 @@ public DabCacheService(IFusionCache cache, ILogger<DabCacheService>? logger, IHt
return result;
}

/// <summary>
/// Try to get cacheValue from the cache with the derived cache key.
/// </summary>
/// <typeparam name="JsonElement">The type of value in the cache</typeparam>
/// <param name="queryMetadata">Metadata used to create a cache key or fetch a response from the database.</param>
/// <returns>JSON Response</returns>
public MaybeValue<JsonElement>? TryGet<JsonElement>(DatabaseQueryMetadata queryMetadata)
{
string cacheKey = CreateCacheKey(queryMetadata);
return _cache.TryGet<JsonElement>(key: cacheKey);
}

/// <summary>
/// Store cacheValue into the cache with the derived cache key.
/// </summary>
/// <typeparam name="JsonElement">The type of value in the cache</typeparam>
/// <param name="queryMetadata">Metadata used to create a cache key or fetch a response from the database.</param>
/// <param name="cacheEntryTtl">Number of seconds the cache entry should be valid before eviction.</param>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be a section for cacheValue?

public void Set<JsonElement>(
DatabaseQueryMetadata queryMetadata,
int cacheEntryTtl,
JsonElement? cacheValue)
{
string cacheKey = CreateCacheKey(queryMetadata);
_cache.Set(
key: cacheKey,
value: cacheValue,
(FusionCacheEntryOptions options) =>
{
options.SetSize(EstimateCacheEntrySize(cacheKey: cacheKey, cacheValue: cacheValue?.ToString()));
options.SetDuration(duration: TimeSpan.FromSeconds(cacheEntryTtl));
});
}

/// <summary>
/// Attempts to fetch response from cache. If there is a cache miss, invoke executeQueryAsync Func to get a response
/// </summary>
Expand Down
Loading