Add header support for caching #2650
Merged
Conversation
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
aaronburtle
requested review from
Aniruddh25,
RubenCerna2079,
neeraj-sharma2592,
ravishetye,
sezal98 and
sourabh1007
as code owners
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
Aniruddh25
reviewed
Apr 16, 2025
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for handling caching directives via the HTTP Cache-Control header by incorporating new properties and logic into the SQL query processing and cache service layers. It introduces new cache getter and setter methods in the cache service, adds cache control handling in the SQL query engine, and updates the SQL query structure and REST request context to capture and validate the Cache-Control header.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Core/Services/Cache/DabCacheService.cs | Added TryGet and Set methods to retrieve/store JSON responses in the cache based on a derived cache key. |
| src/Core/Resolvers/SqlQueryEngine.cs | Updated the query execution flow to handle cache control options and to throw exceptions when only-if-cached is used with a cache miss. |
| src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs | Introduced CacheControlOption property and constants, with logic to extract and validate Cache-Control header values. |
| src/Core/Models/RestRequestContexts/RestRequestContext.cs | Added a new property to capture the Cache-Control header from REST requests. |
…/data-api-builder into dev/aaronburtle/HeaderCacheSupport
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
RubenCerna2079
dismissed
Aniruddh25’s stale review
The changes that were asked for are completed.
RubenCerna2079
approved these changes
May 23, 2025
github-project-automation
bot
moved this from Review In Progress
to Done
in Data API builder
RubenCerna2079
added a commit
that referenced
this pull request
May 29, 2025
## Why make this change? Closes #2604 ## What is this change? Inside of the `SqlQueryStructure`, there is a private constructor, that all of the public constructors will call. This private constructor offers a single point where all of the queries will bottleneck, and therefore we add the cache control information to the query structure at this point. Then when we are in the `SqlQueryEngine`, we can check for this cache control information and handle the query appropriately. The cache control options can be found here: #2253 and here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cache-Control#request_directives But for reference, they are `no-cache`, `no-store`, `only-if-cached`, which will mean we do not get from the cache, we do not store in the cache, and if there is a cache miss we return gateway timeout, respectively. ## How was this tested? Run against test suite. ## Sample Request(s) To test you need to include the relevant cache headers in the request, "no-cache", "no-store", or "only-if-cached" --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]> Co-authored-by: RubenCerna2079 <[email protected]>
2 tasks
RubenCerna2079
added a commit
that referenced
this pull request
May 29, 2025
## Why make this change? Closes #2604 ## What is this change? Inside of the `SqlQueryStructure`, there is a private constructor, that all of the public constructors will call. This private constructor offers a single point where all of the queries will bottleneck, and therefore we add the cache control information to the query structure at this point. Then when we are in the `SqlQueryEngine`, we can check for this cache control information and handle the query appropriately. The cache control options can be found here: #2253 and here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cache-Control#request_directives But for reference, they are `no-cache`, `no-store`, `only-if-cached`, which will mean we do not get from the cache, we do not store in the cache, and if there is a cache miss we return gateway timeout, respectively. ## How was this tested? Run against test suite. ## Sample Request(s) To test you need to include the relevant cache headers in the request, "no-cache", "no-store", or "only-if-cached" --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]> Co-authored-by: RubenCerna2079 <[email protected]>
aaronburtle
added a commit
that referenced
this pull request
May 29, 2025
## Why make this change? This change is made in order to add all of the commits for milestone 1.5 into its respective branch. ## What is this change? This change cherry-picks all of the commits that were added after the first release candidate. Cherry-picked commits: - #2648 #2657 #2617 #2659 #2655 #2633 #2667 #2673 #2650 #2695 #2702 #2688 ## How was this tested? - [ ] Integration Tests - [ ] Unit Tests ## Sample Request(s) --------- Co-authored-by: Sezal Chug <[email protected]> Co-authored-by: sezalchug <[email protected]> Co-authored-by: Tommaso Stocchi <[email protected]> Co-authored-by: Aaron Powell <[email protected]> Co-authored-by: aaronburtle <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]> Co-authored-by: Jerry Nixon <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Michael Staib <[email protected]> Co-authored-by: souvikghosh04 <[email protected]> Co-authored-by: Souvik Ghosh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Why make this change?
Closes #2604
What is this change?
Inside of the
SqlQueryStructure, there is a private constructor, that all of the public constructors will call. This private constructor offers a single point where all of the queries will bottleneck, and therefore we add the cache control information to the query structure at this point. Then when we are in theSqlQueryEngine, we can check for this cache control information and handle the query appropriately.The cache control options can be found
here: #2253
and here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cache-Control#request_directives
But for reference, they are
no-cache,no-store,only-if-cached, which will mean we do not get from the cache, we do not store in the cache, and if there is a cache miss we return gateway timeout, respectively.How was this tested?
Run against test suite.
Sample Request(s)
To test you need to include the relevant cache headers in the request, "no-cache", "no-store", or "only-if-cached"