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

Add setter for order for DefaultSimpUserRegistry [SPR-17023] #21561

Closed
spring-projects-issues opened this issue Jul 9, 2018 · 7 comments
Closed
Assignees
Labels
in: web type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 9, 2018

Filip Hrisafov opened SPR-17023 and commented

Currently the DefaultSimpUserRegistry and MultiServerUserRegistry are using the Ordered.LOWEST_PRECEDENCE.

We have listeners that are listening on SessionConnectedEvent and in some of our logic we are checking the SimpUserRegistry. However, if our listener is triggered before the registry ones then there is no user in the registry (if this is the first connection).

I currently managed to register my listener after the registry ones. However, I am not sure that this is deterministic enough. I just annotated it with @Order


Affects: 5.0.7

Issue Links:

  • #21679 More easily set the order on DefaultSimpUserRegistry
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 12, 2018

Rossen Stoyanchev commented

We could add a setOrder on DefaultSimpUserRegistry for starters. Are you using WebSocketMessageBrokerConfigurer or extending from WebSocketMessageBrokerConfigurationSupport?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 13, 2018

Filip Hrisafov commented

Having a setOrder might work for us. We are using the WebSocketMessageBrokerConfigurer. We are actually providing a library and users need to enable it through @EnableWebSocketMessageBroker. We have a small Spring Boot auto configuration to configure the broker actually.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 24, 2018

Rossen Stoyanchev commented

I've added setOrder on DefaultSimpUserRegistry that is also picked up by MultiServerUserRegistry. I'll stop here since I'm not familiar with the details of your setup, let me know if you need anything further.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 30, 2018

Filip Hrisafov commented

Thanks a lot Rossen. I think that this should be enough. It is a bit "difficult" to set the order (would need to either do provide our own WebSocketMessageBrokerConfigurationSupport, which I would like to avoid). However, I can probably do it with a BeanPostProcessor as well.

We are using Spring Boot, so I will try to do some more general auto configuration that can be used perhaps. Do you know if there are some plans to have Boot auto configurations for this components from Spring?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 31, 2018

Rossen Stoyanchev commented

Okay, in that case I can expose something on MessageBrokerRegistry. Since this ticket is already closed and released, please create a separate ticket for that, and I'll mark it for 5.1 RC2.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 6, 2018

Rossen Stoyanchev commented

Filip Hrisafov regarding your question about Boot plans for auto config, you might want to add a comment under spring-projects/spring-boot#65. It was closed recently due to lack of activity but nevertheless it's the best place to leave some record.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2018

Filip Hrisafov commented

Thanks a lot for everything Rossen. I just created #21679 for the setter. I will comment on the Boot issue as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants