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

Default WebSocketMessageBrokerConfigurer is always overriding custom channel executor #42924

Closed
shmyer opened this issue Oct 29, 2024 · 6 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@shmyer
Copy link

shmyer commented Oct 29, 2024

When using Spring Websocket with Spring Boot, the latter provides a WebSocketMessagingAutoConfiguration which provides a default WebSocketMessageBrokerConfigurer which sets either the single task executor found in the context or the executor named "applicationTaskExecutor" for the inbound and outbound client channels:

WebSocketMessageConverterConfiguration(ObjectMapper objectMapper,
Map<String, AsyncTaskExecutor> taskExecutors) {
this.objectMapper = objectMapper;
this.executor = determineAsyncTaskExecutor(taskExecutors);
}
private static AsyncTaskExecutor determineAsyncTaskExecutor(Map<String, AsyncTaskExecutor> taskExecutors) {
if (taskExecutors.size() == 1) {
return taskExecutors.values().iterator().next();
}
return taskExecutors.get(TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME);
}

@Override
public void configureClientInboundChannel(ChannelRegistration registration) {
if (this.executor != null) {
registration.executor(this.executor);
}
}
@Override
public void configureClientOutboundChannel(ChannelRegistration registration) {
if (this.executor != null) {
registration.executor(this.executor);
}
}

When trying to set a custom executor for my inbound and outbound channels in my custom WebSocketMessageBrokerConfigurer implementation, it doesn't have any effect. The issue seems to be, that in DelegatingWebSocketMessageBrokerConfiguration the list of configurers always (at least in my case), processes the default configurer provided by Spring Boot after my custom one, causing my executor configuration to be overridden with the default one.

https://github.com/spring-projects/spring-framework/blob/c2c6bb25c68c231e8eb250809442987512755d62/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/DelegatingWebSocketMessageBrokerConfiguration.java#L49-L54

Debugging shows the order:
image

https://github.com/spring-projects/spring-framework/blob/c2c6bb25c68c231e8eb250809442987512755d62/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/DelegatingWebSocketMessageBrokerConfiguration.java#L71-L83

Instead, the DelegatingWebSocketMessageBrokerConfiguration should probably always process the default implementation first and then any custom configurers, probably via looking at @Order annotations. Now I don't know if that is an issue which must be handled in Spring Boot or in Spring Framework, but I can raise the issue in the latter project if you feel like it should be handled there.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 29, 2024
@philwebb
Copy link
Member

It looks to me like DelegatingWebSocketMessageBrokerConfiguration uses standard @Autowired constructors which will respect ordering. We probably need to add @Ordered to our bean, but I'm not sure if we can do that in a maintenance release.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Oct 29, 2024
@shmyer
Copy link
Author

shmyer commented Oct 29, 2024

DelegatingWebSocketMessageBrokerConfiguration is actually using setter injection instead of constructor injection, but I guess that what you said about ordering applies for setters, too?

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Oct 30, 2024
@philwebb philwebb added this to the 3.4.x milestone Oct 30, 2024
@philwebb
Copy link
Member

We consider this a bug and we're going to add @Order(0), but it's a risky change so we'll do it in 3.4.x.

@shmyer
Copy link
Author

shmyer commented Oct 30, 2024

Thanks! In the meantime I found a workaround by excluding the WebSocketMessagingAutoConfiguration via spring.autoconfigure.exclude and introducing any configuration required from it in my custom WebSocketMessageBrokerConfigurer.

@jack5505
Copy link

hello community can I work on this bugs

@philwebb philwebb self-assigned this Oct 31, 2024
@philwebb
Copy link
Member

Thanks for the offer @jack5505, but I started on this yesterday and forgot to assign myself. The code change is easy, but I'm also trying to create a test.

@philwebb philwebb modified the milestones: 3.4.x, 3.4.0 Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants