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

Refactor content negotiation and conditional requests implementation #229

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ordnungswidrig
Copy link
Member

This PR removes some of the more technical decision points. These do not make much sense as an extension point for the user. The remaining internal decision points, mostly branches depending on the request method are dimmed in the graph now.

Removed decisions:

  • accept-exists?
  • accept-language-exists?
  • accept-charset-exists?
  • accept-encoding-exists?
  • if-match-star
  • if-modified-since-exists?
  • if-modified-since-valid-date?
  • if-unmodified-since-exists?
  • if-unmodified-since-valid-date?
  • if-none-match-exists?
  • if-none-match-star?
  • if-match-exists?
  • if-match-star?

All changes are made backward compatible, except that the decisions ...-available? and those of conditional request must take into account that the header value can be absent or a special value, e.g. * in etag-matches-for-if-match.

This is the new graph:

trace

@ordnungswidrig ordnungswidrig force-pushed the refactor-conneg-and-condreq branch 2 times, most recently from 4ac1786 to b3190ef Compare August 31, 2015 11:42
@ordnungswidrig
Copy link
Member Author

@malcolmsparks any comments?

@malcolmsparks
Copy link
Member

Makes sense to make user extensions fit with sensible areas the user wants to specify, rather than allowing the user to subvert http semantics by overriding decision points that don't make sense.

The decision points that decide on the existence of the
Accept-... headers have been there just because of technical reasons and
did not had any practical application. In fact they cluterred the
decision graph and added noise.
@ordnungswidrig ordnungswidrig force-pushed the refactor-conneg-and-condreq branch from 5fab141 to 5101835 Compare April 22, 2016 11:21
Remove *-valid-date? and *-exists and more internal decision points
which did not make sense to be customized. The decision logic has been
incorporated into the implementation into the actual decision points.

Removes

* if-match-star
* if-modified-since-exists?
* if-modified-since-valid-date?
* if-unmodified-since-exists?
* if-unmodified-since-valid-date?
* if-none-match-exists?
* if-none-match-star?
* if-match-exists?
* if-match-star?
@ordnungswidrig ordnungswidrig force-pushed the refactor-conneg-and-condreq branch from 5101835 to 14d738d Compare August 5, 2016 09:25
@ordnungswidrig
Copy link
Member Author

Cleaned up the graph a bit, making it more compactly drawn and improved the trace view highlighting

bildschirmfoto 2016-08-05 um 11 31 16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants