Add Response HTTP caching via header values

For &13, a change will need to be made to handle injecting a Cache-Control header into the response when specified for endpoints. This will need to have multiple configurable levels, with the ability to modify and add custom configurations. To give us the best flexibility, this should be able to switch based on response status and set cache time and nature of caching at least.

added prioritymedium severitynormal statewip team:webdev webdev:java labels

assigned to @malowe

changed the description

Reading deeper, there is a @Cache annotation built into the REST package which lets us set hardcoded values as cache properties. This would be far easier to maintain as there wouldn't be additional mappings to maintain or filters, but also less flexible. We would lose the ability to filter by response type in switching to the baked in support.

Do we want to use the less flexible but much easier to maintain solution based on the existing annotation, or create our own?

Speaking w/ @cguindon, we came to the final point that for most of our endpoints the annotations will be good enough by the looks of it. We mainly care about attributes that can be easily mapped by the annotation. While not a perfect reproduction of the current nginx cache headers as there are no changes in cache headers for different statuses, it represents a large step up.

For cases where we have mixed authentication, we can use a solution similar to the following to set the needed headers:

boolean isAuthenticated = someCheck();
ResponseBuilder baseResponse =  isAuthenticated ? 
    RestResponse.ResponseBuilder.ok("authenticated-data").header(HttpHeaders.CACHE_CONTROL, "public, max-age=300") : 
    RestResponse.ResponseBuilder.ok("anon-data").header(HttpHeaders.CACHE_CONTROL, "no-store, no-cache, must-revalidate, max-age=0");
return baseResponse.header(HttpHeaders.VARY, HttpHeaders.AUTHORIZATION).build();

Currently, with the low impact of limitations of the out of the box support, it doesn't make sense to write a custom system that adds more complexity. In the future, we can always add it in if we find a case that isn't supported that is required.

mentioned in commit 723fe733

mentioned in merge request !239 (merged)

@malowe, I wanted to summarize our conversation from yesterday regarding how we will measure success for this MBO. Here are the three key requirements we should aim to achieve:

1. Develop and implement a robust solution that enables us to easily apply caching headers to our various APIs as needed. Consider the differing requirements of our APIs; for instance, some may benefit from longer caching durations, while others may require shorter time-to-live (TTL) values. We should prioritize any built-in functionality from Quarkus before creating a custom solution.

2. Create a list of use cases for our existing APIs where caching headers would be beneficial. This list will serve as action items to ensure we utilize this new functionality where needed.

3. Successfully deploy at least one service in production that utilizes the newly implemented caching header solution.

Let me know if you have any concerns or questions about this!

@malowe Before I can mark this MBO complete can you comment on how we achieved these 3 requirements?

This isn't the issue for the epic, just the addition to the SDK that helps support this in an easier to update way with validation testing. The MBO is still open, see https://gitlab.eclipse.org/eclipsefdn/it/internal/infra/configuration-management/puppet-modules/-/merge_requests/150

Not sure I am follwing, this is the issue we have set in our IT MBOs:

https://gitlab.eclipse.org/eclipsefdn/infrazilla/-/wikis/IT-MBOs-2024

This is also where I have listed the key measures for success.

This should have been pointed at &13 instead, so that may have been my mistake in passing you the base issue for the MBO instead of the epic created for the handling of the headers.

closed with merge request !239 (merged)

removed statewip label