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

added support for HTTP PATCH method #269

Closed
wants to merge 2 commits into
base: saxon96
from

Conversation

Projects
None yet
2 participants
@Conal-Tuohy
Contributor

Conal-Tuohy commented Mar 2, 2018

The HTTP PATCH method https://tools.ietf.org/html/rfc5789 allows for resources to be partially updated by sending a "patch document" as the body of the HTTP request. The patch document specifies a set of changes to make to the resource. In other ways it's very like PUT and POST.

@Conal-Tuohy

This comment has been minimized.

Contributor

Conal-Tuohy commented Mar 4, 2018

Not an HTTP method you use every day. ;-)

Incidentally, I wonder if it's worth a small refactoring to allow for additional obscure methods like PROPPATCH or for new or custom methods? In my first commit, I'd inadvertently handled PATCH using the HttpPost class from the Apache http library, and it actually worked fine, which makes me think that other methods could also be supported fairly generically. A concrete sub-class of HttpEntityEnclosingRequestBase could be used to implement any unknown method where a request entity body was provided and an additional concrete sub-class of HttpRequestBase could handle those cases where a request body was not present. Such a refactoring could still enforce that GET, HEAD, OPTIONS, etc, messages do not include an entity, but otherwise allow entity bodies for other methods.

I'd be happy to contribute that, if you're amenable, but in the meantime this at least allows for PATCH.

ndw added a commit that referenced this pull request Mar 4, 2018

ndw added a commit that referenced this pull request Mar 4, 2018

ndw added a commit that referenced this pull request Mar 4, 2018

@ndw

This comment has been minimized.

Owner

ndw commented Mar 4, 2018

Right. I've merged it manually. I'd be happy to accept a PR with your refactoring ideas.

If you're ready to move to saxon 9.7.x or 9.8.x, that'd be great. I'm looking to phase out support for 9.6. (In fact, I'd have done it on this PR except that it seemed rude to accept your patch and then not apply it to the branch you're apparently using! :-) )

@ndw ndw closed this Mar 4, 2018

@Conal-Tuohy

This comment has been minimized.

Contributor

Conal-Tuohy commented Mar 4, 2018

Thanks! And I will switch to a later branch (NB your README is out of date on the topic of branches).

I've made a note about the "additional methods" idea, but not sure when I'll get back to it as I'm swamped with work just now.

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