Skip to content

refactor: decompose & simplify streaming multipart body encoding#3430

Draft
yvasyliev wants to merge 3 commits into
OpenFeign:14.xfrom
yvasyliev:feature/pr-3414-refactoring
Draft

refactor: decompose & simplify streaming multipart body encoding#3430
yvasyliev wants to merge 3 commits into
OpenFeign:14.xfrom
yvasyliev:feature/pr-3414-refactoring

Conversation

@yvasyliev

Copy link
Copy Markdown
Contributor

Description

The 4th PR in a series related to #2734.

This PR is a follow-up to #3414. While #3414 successfully introduced streaming multipart/form-data request bodies, the supporting architecture brought along a heavy footprint of specialized encoders and resolvers (AbstractPartEncoder, LeafPartResolver, ArrayPartResolver, etc.).

This change partially reverts the over-extended class hierarchies introduced there and replaces them with a highly decomposed, modular, and simplified design that aligns strictly with the Single Responsibility Principle (SRP).

Proposed Changes

1. Separation of Concerns via Dedicated Factories

Instead of managing encoding across multiple deeply nested classes, the part creation pipeline is now orchestrated by a lightweight PartFactory that delegates cleanly to specialized components:

  • PartHeadersFactory: Handles the calculation and construction of content-disposition and field-specific metadata headers.
  • PartBodyFactory: Responsible for determining and invoking the correct encoding strategy for the part payload.

2. Guarded Delegation with ConditionalEncoder & EncoderPredicate

Introduces an expressive rule-matching layout to safely map core Feign encoders to specialized body media types:

  • EncoderPredicate: A functional abstraction used to check capability matches (such as evaluation of normalized Content-Type headers) before encoding begins.
  • ConditionalEncoder: A clean Decorator pattern implementation that guards a single delegate encoder and validates it against given capability criteria prior to execution, failing fast with a transparent exception if unsupported.

3. Substantial Code Reduction

By flattening the pipeline, we are able to completely remove a substantial amount of redundant structural overhead:

  • Removed: AbstractPartEncoder, PartEncoder, DelegatingPartEncoder, ByteArrayPartEncoder, DefaultPartEncoder, FilePartEncoder, InputStreamPartEncoder.
  • Removed: PartResolver, ArrayPartResolver, FormDataPartResolver, IterablePartResolver, LeafPartResolver.
  • Removed: XmlEncoder.

Related Pull Requests

Target Branch

  • 14.x

Verification Results

  • Existing streaming multipart form behavior remains fully intact.
  • Test assertions have been updated to match the streamlined structural improvements.

@yvasyliev yvasyliev marked this pull request as ready for review June 19, 2026 14:33

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for continuing to simplify the multipart streaming internals. I can't merge this shape on the 14.x line because it removes public API that downstream users can already compile against. In api/src/main/java/feign/codec/Encoder.java, the public default supports(String) method is removed, and api/src/main/java/feign/codec/JsonEncoder.java loses its content-type behavior. The diff also removes public multipart extension points such as PartEncoder/PartResolver and replaces the existing MultipartFormEncoder builder customizers like partBodyEncoders/partEncoders/partResolvers. Those are source/binary compatibility breaks for a minor branch.\n\nPlease keep the existing public/protected symbols working and add the new design alongside them, or deprecate the old surface with replacement javadocs first and leave removal for a future major. Once the refactor is additive from a downstream user's point of view, I can take another pass.

@yvasyliev

yvasyliev commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Hey @velo the removed public API was added in #3414 a week ago. I hardly believe there're any downstream users. Besides, the PR is opened against 14.x branch for the next major release

@yvasyliev yvasyliev requested a review from velo June 20, 2026 13:28
@yvasyliev yvasyliev marked this pull request as draft June 21, 2026 07:48
@velo

velo commented Jun 22, 2026

Copy link
Copy Markdown
Member

my bad, sorry, you are correct, on 14.x api can be broken

* Session login(@Param("username") String username, @Param("password") String password);
* </pre>
*/
@FunctionalInterface

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why?

* </pre>
*/
public class JsonEncoder implements feign.codec.JsonEncoder {
public class JsonEncoder implements Encoder {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is unacceptable, as graphql module requires a jsonEncoder/decoder and this breaks the pattern

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

Successfully merging this pull request may close these issues.

2 participants