-
Notifications
You must be signed in to change notification settings - Fork 1.1k
GH-10083: Apply Nullability to some core support packages #10360
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
GH-10083: Apply Nullability to some core support packages #10360
Conversation
...qp/src/main/java/org/springframework/integration/amqp/inbound/AmqpInboundChannelAdapter.java
Outdated
Show resolved
Hide resolved
39a4b45
to
a1437de
Compare
...rc/main/java/org/springframework/integration/support/channel/BeanFactoryChannelResolver.java
Outdated
Show resolved
Hide resolved
...ation-core/src/main/java/org/springframework/integration/support/context/NamedComponent.java
Outdated
Show resolved
Hide resolved
Th fix for AMQP is in. |
Related to: spring-projects#10083 This commit applies Nullability to `support.channel`, `support.context` and `support.converter` packages Signed-off-by: Tran Ngoc Nhan <[email protected]>
Signed-off-by: Tran Ngoc Nhan <[email protected]>
Signed-off-by: Tran Ngoc Nhan <[email protected]>
649983e
to
a0ce80d
Compare
Signed-off-by: Tran Ngoc Nhan <[email protected]>
@Nullable | ||
public String getComponentType() { | ||
return null; | ||
return "integration-object-support"; |
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 very good.
I believe we are in a situation where we should remove this method implementation here in the IntegrationObjectSupport
.
And ensure that it is implemented properly everywhere else.
Signed-off-by: Tran Ngoc Nhan <[email protected]>
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.
Thank you for an update!
You made me think a bit on the state of IntegrationObjectSupport
.
Some of my comments are not items for action, but some of them are really ask for you to change.
|
||
@Override | ||
public String getComponentType() { | ||
return "channel"; |
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.
header-channel-registry
.
I can argue that this class is not supposed to be an IntegrationObjectSupport
.
But that's different story.
|
||
@Override | ||
public String getComponentType() { | ||
return "converter"; |
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.
Same here: this is not an IntegrationObjectSupport
.
But separate story.
|
||
@Override | ||
public String getComponentType() { | ||
return "endpoint"; |
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.
I think this is not correct: every each AbstractEndpoint
must provide their proper implementation.
|
||
@Override | ||
public String getComponentType() { | ||
return "connection"; |
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.
connection-factory
?
|
||
@Override | ||
public String getComponentType() { | ||
return "processor"; |
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.
This one also must not be an IntegrationObjectSupport
.
Signed-off-by: Tran Ngoc Nhan <[email protected]>
44debbc
to
70e49b9
Compare
|
||
@Override | ||
public String getComponentType() { | ||
return "polling-endpoint"; |
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.
The SourcePollingChannelAdapter
does provide an impl of this method.
So, another one - PollingConsumer
- has to do that as well: polling-consumer
.
|
||
@Override | ||
public String getComponentType() { | ||
return "event-driven"; |
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.
Well, event-driven-consumer
to be precise.
|
||
@Override | ||
public String getComponentType() { | ||
return "message-producer"; |
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.
I think neither of abstract class should provide an impl.
That is really responsibility of the specific one.
Without an implementation here, we may spot a bug somewhere down the hierarchy, e.g. ReactiveMessageSourceProducer
.
|
||
@Override | ||
public String getComponentType() { | ||
return "reactive-streams"; |
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.
reactive-streams-consumer
?
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.
OK. I'm taking your PR locally for the final review and clean up before merge.
Thank you!
Merged as: 17e6a31 |
Related to: #10083
This commit applies Nullability to
support.channel
,support.context
andsupport.converter
packages