Skip to content

Conversation

@helix
Copy link
Contributor

@helix helix commented Aug 21, 2018

  1. There was a typo on the must-revalidate directive (missing dash)
  2. The Cache-Control header didn't completely prevent HTTP caching
  3. The Last-Modified header is advantageously replaced by right
    Expires and Cache-Control headers

1. There was a typo on the `must-revalidate` directive (missing dash)
2. The `Cache-Control` header didn't completely prevent HTTP caching
3. The `Last-Modified` header is advantageously replaced by right
   `Expires` and `Cache-Control` headers
@helix helix force-pushed the fix/rest-http-cache branch from ca8223a to a1a15bb Compare August 21, 2018 16:57
switch (this.policy) {
case NO_CACHE:
MultivaluedMap<String, Object> headers = responseContext.getHeaders();
headers.putSingle(HttpHeaders.LAST_MODIFIED, new Date());
Copy link
Member

Choose a reason for hiding this comment

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

Removing this header was intended?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is intentional because this header is only making sense when you want to actually cache the resource.

@adrienlauer
Copy link
Member

After reading the provided links (SO and RFC), I agree this is the right way to do it.
An integration test is failing but I'll fix it and merge this PR. Thanks for your contribution!

@adrienlauer adrienlauer mentioned this pull request Aug 27, 2018
@adrienlauer adrienlauer merged commit a1a15bb into seedstack:master Aug 27, 2018
@helix
Copy link
Contributor Author

helix commented Sep 5, 2018

@adrienlauer woops, indeed I forgot to update the tests... my bad. Thanks for the merge though!

PS: sorry for the late response, I didn't receive any email notifications from GitHub... weird

hervestern pushed a commit to hervestern/seed that referenced this pull request Aug 10, 2020
Also removes useless DependencyProvider SPI (for optional dependencies which are
now handled directly).

Also improves instantiation of the EL ExpressionFactory.

Also makes the validation message interpolator automatically adapt to whether the
runtime environment supports EL or not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants