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.

Sign up for GitHub

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

Jump to bottom

Fix Package Tangles #3656

Closed
garyrussell opened this issue Oct 27, 2021 · 4 comments · Fixed by #3658
Closed

Fix Package Tangles #3656

garyrussell opened this issue Oct 27, 2021 · 4 comments · Fixed by #3658
Assignees
@artembilan
Labels
type: refactoring
Milestone
5.5.6

Comments

@garyrussell
Copy link
Contributor

garyrussell commented Oct 27, 2021 •
edited

Screen Shot 2021-10-27 at 4 57 00 PM

Screen Shot 2021-10-27 at 4 57 34 PM

The first is easy to fix (move JavaUtils to ...support.util, then restore/deprecate the class in ...util).

The second is not so simple - moving the JsonNodeWrapperToJsonNodeConverter to ...support.json doesn't help; the issue is that @IntegrationConverter is in ...config and DefaultConfiguringBeanFactoryPostProcessor has references to ...json. Moving the converter and property accessor to ...config should solve it, but doesn't feel right.
@garyrussell garyrussell added this to the Backlog milestone Oct 27, 2021
@artembilan
Copy link
Member

Another way to fix the first one is to move JavaUtils to the top-level package.
This class is indeed has nothing to do with any infrastructure to be in the specific package. At the same time being in the top-level package it sets free our hands to use it from anywhere.
Another possible fix is to remove its usage from that new IntegrationProperties 😄 .

The second problem is probably easier to fix removing that @IntegrationConverter from the JsonNodeWrapperToJsonNodeConverter and register it into the target GenericConversionService some other way.

@garyrussell
Copy link
Contributor Author

Screen Shot 2021-10-28 at 4 08 07 PM

@garyrussell
Copy link
Contributor Author

Actually, the reference "out" of utils (to core) is a bit odd as well - utils are usually "used" by other classes, not vice versa.

Although, I suppose core is a special case.

@artembilan
Copy link
Member

Perhaps the IntegrationReactiveUtils has to go to that core then...

Those MessageSource, MessageProducer, MessageSelector etc. I feel better would live in the root package.
Or... Those ErrorMessagePublisher and MessagingTemplate are in the wrong place since they depend on support package and so on down to util.

@artembilan artembilan modified the milestones: Backlog, 5.5.6 Nov 1, 2021
@artembilan artembilan self-assigned this Nov 1, 2021
artembilan added a commit to artembilan/spring-integration that referenced this issue Nov 1, 2021
@artembilan
spring-projectsGH-3656: Fix package tangles
6579497 
Fixes spring-projects#3656

* Move `JavaUtils` from `util` package to the root one to break any possible
tangling to/from other packages.
* Deprecate an existing `JavaUtils` for backward compatibility
* Remove `@IntegrationConverter` from the `JsonNodeWrapperToJsonNodeConverter`
and register it manually in the `ConverterRegistrar`.
The bean registration for the `JsonNodeWrapperToJsonNodeConverter` must be removed
in the next `6.0`
* Remove usage of `IntegrationContextUtils` from the `support` package
@artembilan artembilan mentioned this issue Nov 1, 2021
Merged
garyrussell pushed a commit that referenced this issue Nov 1, 2021
@artembilan @garyrussell
GH-3656: Fix package tangles
fc7d338 
Fixes #3656

* Move `JavaUtils` from `util` package to the root one to break any possible
tangling to/from other packages.
* Deprecate an existing `JavaUtils` for backward compatibility
* Remove `@IntegrationConverter` from the `JsonNodeWrapperToJsonNodeConverter`
and register it manually in the `ConverterRegistrar`.
The bean registration for the `JsonNodeWrapperToJsonNodeConverter` must be removed
in the next `6.0`
* Remove usage of `IntegrationContextUtils` from the `support` package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@artembilan artembilan

Labels
type: refactoring
Projects
None yet
Milestone
5.5.6
Development

Successfully merging a pull request may close this issue.

GH-3656: Fix package tangles artembilan/spring-integration
2 participants
@garyrussell @artembilan