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

Client Should Support HTTP Patch Method #1276

Open
hantsy opened this issue Jul 13, 2024 · 8 comments
Open

Client Should Support HTTP Patch Method #1276

hantsy opened this issue Jul 13, 2024 · 8 comments

Comments

@hantsy
Copy link

hantsy commented Jul 13, 2024

Currently SyncInvoker does not include a patch method, should add patch and align to JSON PATCH/Merge Patch support in the server side.

@jamezp
Copy link
Contributor

jamezp commented Jul 15, 2024

This can be done by invoking SyncInvoker.method("PATCH").

@mkarg
Copy link
Contributor

mkarg commented Jul 15, 2024

Currently SyncInvoker does not include a patch method, should add patch and align to JSON PATCH/Merge Patch support in the server side.

@hantsy Could you please elaborate on chosing the term "SHOULD"? IMHO this is rather nice to have.

@hantsy
Copy link
Author

hantsy commented Jul 16, 2024

For a Chinese, in my mind, SHOULD may be just an emphasis of nice to have :)

@hantsy
Copy link
Author

hantsy commented Jul 16, 2024

@jamezp Already used it, https://github.com/hantsy/jakartaee11-sandbox/blob/master/rest/src/test/java/com/example/it/ArticleResourceTest.java#L179

But here it requires a property for the client, .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true).

Of course, I hope a simple patch() to replace this.

@jamezp
Copy link
Contributor

jamezp commented Jul 16, 2024

@hantsy I'm not sure what HttpUrlConnectorProvider.SET_METHOD_WORKAROUND does, but it's implementation specific so it might still be needed.

I've got no strong opinion on adding or not adding really, just showing there is a fairly simply workaround for it.

@mkarg
Copy link
Contributor

mkarg commented Jul 16, 2024

For a Chinese, in my mind, SHOULD may be just an emphasis of nice to have :)

This term has special meaning in JAX-RS and other specifications: https://github.com/jakartaee/rest/blob/main/jaxrs-spec/src/main/asciidoc/chapters/introduction/_conventions.adoc. It is not country-dependent.

@jansupol
Copy link
Contributor

.property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)

This is Jersey specific, using reflection to allow JDK HttpUrlConnection for having an HTTP method not supported by the JDK. However, it does not work on JDK 16+ without --add-open on JDK packages, which may be forbidden in customer production environments.

The requirement is understandable, but difficult to reach. The JDK HttpUrlConnection does not support it, and the java.net.http implementation of HttpClient is very limiting in what it allows, many use-cases are not possible to implement.

The requirement, if mandatory, could lead to a requirement for each Jakarta-REST implementation to actually implement an HTTP Client that supports all the use-cases (or find a third-party client that is capable of doing so and be inherently dependent on it).

@hantsy
Copy link
Author

hantsy commented Aug 31, 2024

This is Jersey specific, using reflection to allow JDK HttpUrlConnection for having an HTTP method not supported by the JDK.

I would like use a modern Http connector enigne.

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

No branches or pull requests

4 participants