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

Modify to not use URLEncodedUtils #999

Merged

Conversation

AntCode97
Copy link
Contributor

Description

SimpleEndpoint uses org.apache.hc.core5.net.URLEncodedUtils, but we deprecated it and modified it to use our own implementation of pathEncoder.

This allows us to remove the dependency on HttpClient.

I've referenced URLEncodedUtils and made it work exactly the same, optimizing it by removing parameters that aren't needed.

Issues Resolved

Fixes #998

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Add some basic tests for the public method?

@AntCode97 AntCode97 force-pushed the modify-to-not-use-url-encoded-utils branch 2 times, most recently from 7d546c9 to 2a4b9b5 Compare May 28, 2024 09:53
@dblock
Copy link
Member

dblock commented May 28, 2024

This works for me. Let's add a line to CHANGELOG? This fixes a bug. You can say "Fix java.lang.NoSuchMethodError: org.apache.http.client.utils.URLEncodedUtils.formatSegments w/o httpclient" or something like that.

@dblock dblock added the backport 2.x Backport to 2.x branch label May 28, 2024
@reta
Copy link
Collaborator

reta commented May 31, 2024

Thank you @AntCode97 , I think we only are missing entry in CHANGELOG.md under [Unreleased 2.x] section, could you please add one? Thank you.

@AntCode97
Copy link
Contributor Author

Thank you @AntCode97 , I think we only are missing entry in CHANGELOG.md under [Unreleased 2.x] section, could you please add one? Thank you.

Okay. I've added it.

@reta
Copy link
Collaborator

reta commented May 31, 2024

@AntCode97 my apologies, there are conflicts and and DCO check is failing

@AntCode97 AntCode97 force-pushed the modify-to-not-use-url-encoded-utils branch 2 times, most recently from b0d800c to 0dcf634 Compare May 31, 2024 15:35
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: AntCode97 <dbswns97@gmail.com>
@AntCode97 AntCode97 force-pushed the modify-to-not-use-url-encoded-utils branch from 0dcf634 to cdf7840 Compare May 31, 2024 15:39
@AntCode97
Copy link
Contributor Author

@AntCode97 my apologies, there are conflicts and and DCO check is failing

Oh, I forgot about that, all fixed.

@reta
Copy link
Collaborator

reta commented May 31, 2024

@dblock all yours :)

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

@AntCode97 Thank you!

It solves the problem so let's merge.

I dislike the fact that we're using reflection very much. I would have much rather preferred either a neutral implementation or a change in class hierarchy that ends up calling the encode method in the right library. Since I'm only the talking head I opened #1008. Maybe I'll get to it or maybe you or @reta feel like a challenge :)

@dblock dblock merged commit cee6818 into opensearch-project:main May 31, 2024
54 checks passed
reta pushed a commit to reta/opensearch-java that referenced this pull request Jun 5, 2024
* modify to not use URLEncodedUtils

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add a license header

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add pathEncoder test

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use ByteBuffer in encodeBytes method

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use URLEncoder

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* delete PathEncoder

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use URLEncodedUtils per version per Classpath

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* fix to use MethodHandle instead of reflection

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add CHANGELOG.md

Signed-off-by: AntCode97 <dbswns97@gmail.com>

---------

Signed-off-by: AntCode97 <dbswns97@gmail.com>
reta pushed a commit to reta/opensearch-java that referenced this pull request Jun 5, 2024
* modify to not use URLEncodedUtils

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add a license header

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add pathEncoder test

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use ByteBuffer in encodeBytes method

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use URLEncoder

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* delete PathEncoder

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use URLEncodedUtils per version per Classpath

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* fix to use MethodHandle instead of reflection

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add CHANGELOG.md

Signed-off-by: AntCode97 <dbswns97@gmail.com>

---------

Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
reta pushed a commit to reta/opensearch-java that referenced this pull request Jun 5, 2024
* modify to not use URLEncodedUtils

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add a license header

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add pathEncoder test

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use ByteBuffer in encodeBytes method

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use URLEncoder

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* delete PathEncoder

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* modify to use URLEncodedUtils per version per Classpath

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* fix to use MethodHandle instead of reflection

Signed-off-by: AntCode97 <dbswns97@gmail.com>

* add CHANGELOG.md

Signed-off-by: AntCode97 <dbswns97@gmail.com>

---------

Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
dblock pushed a commit that referenced this pull request Jun 5, 2024
* modify to not use URLEncodedUtils



* add a license header



* add pathEncoder test



* modify to use ByteBuffer in encodeBytes method



* modify to use URLEncoder



* delete PathEncoder



* modify to use URLEncodedUtils per version per Classpath



* fix to use MethodHandle instead of reflection



* add CHANGELOG.md



---------

Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: AntCode97 <50140266+AntCode97@users.noreply.github.com>
reta added a commit to reta/opensearch-java that referenced this pull request Jun 5, 2024
…h-project#1012)

* modify to not use URLEncodedUtils



* add a license header



* add pathEncoder test



* modify to use ByteBuffer in encodeBytes method



* modify to use URLEncoder



* delete PathEncoder



* modify to use URLEncodedUtils per version per Classpath



* fix to use MethodHandle instead of reflection



* add CHANGELOG.md



---------

Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: AntCode97 <50140266+AntCode97@users.noreply.github.com>
dblock pushed a commit that referenced this pull request Jun 5, 2024
* modify to not use URLEncodedUtils



* add a license header



* add pathEncoder test



* modify to use ByteBuffer in encodeBytes method



* modify to use URLEncoder



* delete PathEncoder



* modify to use URLEncodedUtils per version per Classpath



* fix to use MethodHandle instead of reflection



* add CHANGELOG.md



---------

Signed-off-by: AntCode97 <dbswns97@gmail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: AntCode97 <50140266+AntCode97@users.noreply.github.com>
@BrendonFaleiro BrendonFaleiro mentioned this pull request Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The SimpleEndpoint class in version 2.10.3 references the URLEncodedUtils class in the wrong library.
3 participants