Skip to content
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

RESTEASY-1322: RFC 7232 preconditions evaluation #754

Merged
merged 13 commits into from Apr 6, 2016

Conversation

bedrin
Copy link
Contributor

@bedrin bedrin commented Mar 12, 2016

RESTEASY-1322: Resteasy processes If-Modified-Since header even when If-None-Match header is present

When both LastModifiedDate and ETag are passed to [evaluatePreconditions(java.util.Date lastModified, EntityTag eTag)](http://docs.oracle.com/javaee/6/api/javax/ws/rs/core/Request.html#evaluatePreconditions%28java.util.Date, javax.ws.rs.core.EntityTag%29) method, RestEasy will return NotModifies status code if any of them match with request headers.

However ETag should have higher priority.

Extract from RFC 7232:

A recipient must ignore If-Modified-Since if the request contains an If-None-Match header field; the condition in If-None-Match is considered to be a more accurate replacement for the condition in If-Modified-Since, and the two are only combined for the sake of interoperating with older intermediaries that might not implement If-None-Match.

In order to keep backward compatibility this feature is optional and can be enabled using resteasy.rfc7232preconditions configuration property

JessThrysoee and others added 6 commits April 1, 2015 23:38
RFC7232 Section 6 defines the order of evaluation when more than
one precondition is present for a conditional request.

Basicaly the algorightm is: compare the tags first, and compare
the dates only if the tags are absent.
…edence' into RESTEASY-1322-evaluate-preconditions

# Conflicts:
#	jaxrs/resteasy-jaxrs/src/main/java/org/jboss/resteasy/specimpl/RequestImpl.java
…edence' into RESTEASY-1322-evaluate-preconditions

# Conflicts:
#	jaxrs/resteasy-jaxrs/src/main/java/org/jboss/resteasy/specimpl/RequestImpl.java
@ronsigal
Copy link
Collaborator

Hi Dimitry,

Very nice job. I just have a few minor quibbles.

  • In RequestImpl, you have statically imported IF_MATCH and IF_NONE_MATCH, but all of the other references to HttpHeaderNames constants are not statically imported. Just a minor inconsistency that might be a bit confusing.
  • Note that

if (ifModifiedSince != null && (!isRfc7232preconditions() || (isRfc7232preconditions() && !headers.containsKey(IF_NONE_MATCH))))

is equivalent to

if (ifModifiedSince != null && (!isRfc7232preconditions() || (!headers.containsKey(IF_NONE_MATCH))))

and

if (ifUnmodifiedSince != null && (!isRfc7232preconditions() || (isRfc7232preconditions() && !headers.containsKey(IF_MATCH))))

is equivalent to

if (ifUnmodifiedSince != null && (!isRfc7232preconditions() || (!headers.containsKey(IF_MATCH))))

  • In the caching chapter of the Users Guide, the existing three sections are about built-in caching facilities in Resteasy. But calling Request.evaluatePreconditions() is different because it's done by application code. I would put the "HTTP preconditions" section at the end and note that it enables application managed caching.
  • In the "HTTP preconditions" section, the statement "By default Resteasy will return status code 304 (Not modified) or 412 (Precondition failed) if any of conditions is satisfied" isn't quite right. Resteasy returns 304 or 412 if a condition fails.
  • Also in the "HTTP preconditions" section, several '.' are missing.
  • Finally, in the "HTTP preconditions" section, you should specify that "resteasy.rfc7232preconditions" is a context parameter. Also, you should add it to the table in Section 3.7. "Configuration Switches".

Thank you for all your work.

-Ron

@bedrin
Copy link
Contributor Author

bedrin commented Mar 26, 2016

Fixed code style. Documentation still needs to be updated

@bedrin
Copy link
Contributor Author

bedrin commented Mar 30, 2016

I have updated the documentation. Ready for review

@ronsigal
Copy link
Collaborator

ronsigal commented Apr 4, 2016

Hi Dimitry,

Thanks for going that extra step. This PR is ready to merge.

-Ron

@liweinan liweinan merged commit 65cc86b into resteasy:master Apr 6, 2016
@bedrin bedrin deleted the RESTEASY-1322-evaluate-preconditions branch April 21, 2016 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants