-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Multi encoder #3485
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
Open
trumpetinc
wants to merge
91
commits into
OpenFeign:14.x
Choose a base branch
from
trumpetinc:multi-encoder
base: 14.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+789
−77
Open
Multi encoder #3485
Changes from all commits
Commits
Show all changes
91 commits
Select commit
Hold shift + click to select a range
cf28371
build(deps-dev): Bump io.undertow:undertow-core
dependabot[bot] 7a5d815
Merge pull request #3452 from OpenFeign/dependabot/maven/io.undertow-…
github-actions[bot] 46d604f
build(deps): Bump org.junit:junit-bom from 6.1.0 to 6.1.1
dependabot[bot] 6bda942
Merge pull request #3455 from OpenFeign/dependabot/maven/org.junit-ju…
github-actions[bot] 68815c8
build(deps): Bump org.apache.httpcomponents.client5:httpclient5
dependabot[bot] e2ce674
build(deps): Bump com.gradle:common-custom-user-data-maven-extension
dependabot[bot] 0181b69
build(deps): Bump com.google.http-client:google-http-client
dependabot[bot] 9d573b4
Merge pull request #3457 from OpenFeign/dependabot/maven/com.gradle-c…
github-actions[bot] 0fd7247
Merge pull request #3456 from OpenFeign/dependabot/maven/org.apache.h…
github-actions[bot] 2ea2dec
Merge pull request #3458 from OpenFeign/dependabot/maven/com.google.h…
github-actions[bot] b6474fa
build(deps): Bump com.github.jknack:handlebars from 4.5.2 to 4.5.3
dependabot[bot] 7446c1f
build(deps-dev): Bump com.gradle:develocity-maven-extension
dependabot[bot] d9da80c
Merge pull request #3461 from OpenFeign/dependabot/maven/com.gradle-d…
github-actions[bot] d7ea97b
Merge pull request #3460 from OpenFeign/dependabot/maven/com.github.j…
github-actions[bot] 0056795
build(deps): Bump org.openrewrite.recipe:rewrite-migrate-java
dependabot[bot] f10e817
build(deps-dev): Bump org.openrewrite.maven:rewrite-maven-plugin
dependabot[bot] a52a378
Merge pull request #3465 from OpenFeign/dependabot/maven/org.openrewr…
github-actions[bot] 5b81580
Merge pull request #3467 from OpenFeign/dependabot/maven/org.openrewr…
github-actions[bot] 935954a
build(deps): Bump org.openrewrite.recipe:rewrite-testing-frameworks
dependabot[bot] 8896c30
Merge pull request #3466 from OpenFeign/dependabot/maven/org.openrewr…
github-actions[bot] a527c35
build(deps): Bump netty.version from 4.2.15.Final to 4.2.16.Final
dependabot[bot] 7510344
Merge pull request #3470 from OpenFeign/dependabot/maven/netty.versio…
github-actions[bot] 6b0a283
build(deps): Bump jackson.version from 2.22.0 to 2.22.1
dependabot[bot] 5f749db
build(deps-dev): Bump vertx.version in /vertx/feign-vertx5-test
dependabot[bot] 2a8209a
Merge pull request #3472 from OpenFeign/dependabot/maven/vertx/feign-…
github-actions[bot] 14e1842
Merge pull request #3471 from OpenFeign/dependabot/maven/jackson.vers…
github-actions[bot] cbb53a6
build(deps-dev): Bump vertx.version in /vertx/feign-vertx4-test
dependabot[bot] 9e2aad2
Merge pull request #3473 from OpenFeign/dependabot/maven/vertx/feign-…
github-actions[bot] 7965183
build(deps): Bump tools.jackson:jackson-bom from 3.2.0 to 3.2.1
dependabot[bot] 710dab7
build(deps): Bump org.junit:junit-bom from 6.1.1 to 6.1.2
dependabot[bot] 9c51c3b
Merge pull request #3474 from OpenFeign/dependabot/maven/tools.jackso…
github-actions[bot] 33bf63b
Merge pull request #3475 from OpenFeign/dependabot/maven/org.junit-ju…
github-actions[bot] 2f548d5
feat: allow registering multiple encoders in Feign
yvasyliev cd9b364
build(deps-dev): Bump vertx.version in /vertx/feign-vertx4-test
dependabot[bot] a3403f0
build(deps-dev): Bump vertx.version in /vertx/feign-vertx5-test
dependabot[bot] 0629332
Merge pull request #3478 from OpenFeign/dependabot/maven/vertx/feign-…
github-actions[bot] ed41e7a
Merge pull request #3477 from OpenFeign/dependabot/maven/vertx/feign-…
github-actions[bot] e9b0f35
build(deps): Bump kotlin.version from 2.4.0 to 2.4.10
dependabot[bot] 855c1f6
build(deps-dev): Bump org.openrewrite.maven:rewrite-maven-plugin
dependabot[bot] b5ab18f
build(deps): Bump org.openrewrite.recipe:rewrite-migrate-java
dependabot[bot] 26a3c4a
Merge pull request #3479 from OpenFeign/dependabot/maven/kotlin.versi…
github-actions[bot] 9009483
Merge pull request #3480 from OpenFeign/dependabot/maven/org.openrewr…
github-actions[bot] 5370c8e
Merge pull request #3481 from OpenFeign/dependabot/maven/org.openrewr…
github-actions[bot] e461dc4
build(deps): Bump org.openrewrite.recipe:rewrite-testing-frameworks
dependabot[bot] 661d121
Merge pull request #3482 from OpenFeign/dependabot/maven/org.openrewr…
github-actions[bot] df2c15b
feat: make `feign.json.JsonEncoder` implement `feign.codec.JsonEncoder`
yvasyliev f130cce
docs: document new multi-encoder features
yvasyliev 10c9265
docs: fix typos in docs
yvasyliev a696a4a
feat: make `Encode#encode` method return boolean
yvasyliev 4621dd7
docs: fix typos and add clarity to the docs
yvasyliev 7451e9d
build(deps): Bump org.json:json from 20260522 to 20260719
dependabot[bot] c39b706
Merge pull request #3484 from OpenFeign/dependabot/maven/org.json-jso…
github-actions[bot] 2db791b
Merge remote-tracking branch 'yvasyliev/feature/multi-encoder' into m…
trumpetinc c5c19f2
Remove json content-type checks
trumpetinc 4d7b063
Remove XML content-type checks
trumpetinc 0a767ad
Remove migration note about json and xml content-type headers
trumpetinc 0386c8d
Merge branch 'master' into multi-encoder
trumpetinc fb503f7
Updating PR hyperlink
trumpetinc 4ed95e6
Put main.java.version back to 11 (sorry - shouldn't have changed it)
trumpetinc db95df3
Clean up unused imports
trumpetinc 3a93590
Merge remote-tracking branch 'origin/multiloggertest_fix_windows_file…
trumpetinc 6e91211
trying to clean up code formatting for git-code-format-maven-plugin
trumpetinc ef33b17
Merge remote-tracking branch 'origin/multiloggertest_fix_windows_file…
trumpetinc 5b11c91
Rename DelegateEncoder to MultiEncoder
trumpetinc 36761fc
MultiEncoder can't be referenced from interfaces in api module
trumpetinc ffbf4c3
MultiEncoder cleanup
trumpetinc d00312c
Add PredicateEncoder and supporting classes
trumpetinc ea970f6
Remove unnused import
trumpetinc 7af1863
Formatting
trumpetinc d00ad86
Formatting
trumpetinc 095acdb
Add license
trumpetinc b484275
Remove lombok from core
trumpetinc 6710ffc
Fix formatting
trumpetinc 5922082
Update changelog for changes in MultiEncoder, link to new pull request
trumpetinc f6e3743
Merge remote-tracking branch 'Feign/14.x' into multi-encoder
trumpetinc 5440c15
Remove library version changes
trumpetinc c830176
private constructor
trumpetinc b3e884d
Add javadoc
trumpetinc a6f05c4
Use Mockito more
trumpetinc da348f5
Make class public
trumpetinc 49b7d91
Fix toString
trumpetinc ee828e7
Swap constructor argument order (predicte first, then delegate)
trumpetinc 9106c0b
Clean up migration document
trumpetinc 983a244
Fix formatting
trumpetinc 6bdb435
Set maven-jar-plugin.version back to 3.5.0
trumpetinc af582ce
Static factory should be public
trumpetinc 60984fb
Remove reference to canEncode() method. Improve 14. example so it shows
trumpetinc 85c98a9
Rename PredicateEncoder to PredicatingEncoder
trumpetinc f1f07b0
fix code formatting
trumpetinc 2b715d0
Fix/improve javadocs
trumpetinc 9134d7d
Revert unnecessary version bumps
trumpetinc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /* | ||
| * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package feign.codec; | ||
|
|
||
| import feign.RequestTemplate; | ||
| import java.lang.reflect.Type; | ||
|
|
||
| /** A predicate that determines whether a given object can be encoded by an encoder. */ | ||
| @FunctionalInterface | ||
| public interface EncoderPredicate { | ||
|
|
||
| /** | ||
| * Tests whether the given object can be encoded by an encoder. | ||
| * | ||
| * @param object the object to be encoded | ||
| * @param bodyType the type of the object to be encoded | ||
| * @param template the request template that will be used to encode the object | ||
| * @return {@code true} if the object can be encoded, {@code false} otherwise | ||
| */ | ||
| boolean test(Object object, Type bodyType, RequestTemplate template); | ||
| } |
79 changes: 79 additions & 0 deletions
79
api/src/test/java/feign/RequestTemplateFactoryResolverTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /* | ||
| * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package feign; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
| import static org.junit.jupiter.params.provider.Arguments.arguments; | ||
| import static org.mockito.Mockito.mock; | ||
|
|
||
| import feign.codec.EncodeException; | ||
| import java.util.Map; | ||
| import java.util.function.Supplier; | ||
| import java.util.stream.Stream; | ||
| import org.junit.jupiter.api.Nested; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.Arguments; | ||
| import org.junit.jupiter.params.provider.FieldSource; | ||
|
|
||
| class RequestTemplateFactoryResolverTest { | ||
| @Nested | ||
| class BuildFormEncodedTemplateFromArgsTest { | ||
| @Test | ||
| void shouldThrowEncodeException() { | ||
| var methodMetadata = new MethodMetadata(); | ||
| var variables = Map.<String, Object>of("data", "Hello, World!"); | ||
| var factory = | ||
| new RequestTemplateFactoryResolver.BuildFormEncodedTemplateFromArgs( | ||
| methodMetadata, mock(), mock(), mock()); | ||
|
|
||
| methodMetadata.formParams().add("data"); | ||
|
|
||
| assertThrows( | ||
| EncodeException.class, | ||
| () -> factory.resolve(new Object[0], new RequestTemplate(), variables)); | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
| class BuildEncodedTemplateFromArgsTest { | ||
| private static final Supplier<Stream<Arguments>> shouldThrowEncodeException = | ||
| () -> { | ||
| var methodMetadata1 = new MethodMetadata(); | ||
| methodMetadata1.alwaysEncodeBody(true); | ||
|
|
||
| var methodMetadata2 = new MethodMetadata(); | ||
| methodMetadata2.bodyIndex(0); | ||
|
|
||
| return Stream.of( | ||
| arguments(methodMetadata1, new Object[0]), | ||
| arguments(methodMetadata2, new Object[] {"Hello, World!"})); | ||
| }; | ||
|
|
||
| @ParameterizedTest | ||
| @FieldSource | ||
| void shouldThrowEncodeException(MethodMetadata methodMetadata, Object[] argv) { | ||
| var factory = | ||
| new RequestTemplateFactoryResolver.BuildEncodedTemplateFromArgs( | ||
| methodMetadata, mock(), mock(), mock()); | ||
| var mutable = new RequestTemplate(); | ||
|
|
||
| mutable.methodMetadata(methodMetadata); | ||
|
|
||
| assertThrows(EncodeException.class, () -> factory.resolve(argv, mutable, Map.of())); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not critical; the
isJsonContentType&isXmlContentTypestatic methods share the same business logic. Is it possible to extract a private static method with two parameters (RequestTemplate template, String regex)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, certainly possible - quick question, though: Right now, I do not have a predicate for json or xml (in fact, these two methods theoretically should be removed from this PR).
I think that adding XmlContentTypeEncoderPredicate and JsonContentTypeEncoderPredicate would be fine - but should that maybe be in a separate PR? I was trying to keep this PR focused on raw capability...
Also, for another PR ( https://github.com/OpenFeign/feign/pull/3494/changes#diff-3d5ee4752b168285974eb09fc4782f489edeadba936b5c71dc59ff6a043d779d ), I have introduced a dedicated content-type header parser that handles charset sub-elements, etc...). It may be better to use that, then compare the actual extracted content-type to the regex.