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

Package tangles in codec configurers [SPR-16064] #20613

Closed
spring-issuemaster opened this issue Oct 12, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Oct 12, 2017

Rob Winch opened SPR-16064 and commented

It appears there are some package tangles in the codec related classes. I believe most (if not all) are package private scope issues so it is possible this can be resolved passively. At quick glance there are the classes referring to classes within child packages that should be cleaned up:

import org.springframework.http.codec.json.Jackson2JsonDecoder;
import org.springframework.http.codec.json.Jackson2JsonEncoder;
import org.springframework.http.codec.json.Jackson2SmileDecoder;
import org.springframework.http.codec.json.Jackson2SmileEncoder;

import org.springframework.http.codec.multipart.MultipartHttpMessageReader;
import org.springframework.http.codec.multipart.SynchronossPartHttpMessageReader;

import org.springframework.http.codec.multipart.MultipartHttpMessageWriter;


Affects: 5.0 GA

Referenced from: commits 682186a

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 12, 2017

Juergen Hoeller commented

This is actually rather convoluted since the Jackson codec types from http.json appear in CodecConfigurer's DefaultCodecs signatures. So there is a definitely a subpackage circle between public types here...

A clean solution would be to move the entire codec configurer stuff to its own subpackage, separating it from the remaining http.codec contracts. However, it's rather late in the game for that, in particular since the codec configurers appear in several higher-level WebFlux config signatures :-(

So we'll have to see what we can sort out at this late point still. Removing the hard dependencies on the Jackson codec impls from public signatures would be a step forward, and possibly moving the non-public configurer impls to a subpackage, with the default impls resolved by type name (like we do in DispatcherServlet).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 12, 2017

Juergen Hoeller commented

The references from DefaultCodecs to Jackson2JsonDecoder and Jackson2JsonEncoder also have the side effect of bringing in hard jackson-databind dependencies in some scenarios, e.g. on a JVM in debugging mode. Such hard references are to be avoided at any cost in such a central configuration interface, therefore we have to reduce the types accepted by those JSON default configuration methods to Decoder and Encoder and just point to the Jackson implementation types with javadoc references, even if we typically expect instances of those to be passed in. This remains source-compatible so hopefully nobody minds if we fix this in 5.0.1 that way before our ecosystem starts building binary dependencies on those methods in other GA releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.