Enable use of both @SendTo and @SendToUser on the same method [SPR-16891]

[spring-projects-issues]

Ethan Mcgee opened SPR-16891 and commented

It would be really nice if it were possible using spring messaging to return an individual message to both the user and simultaneously broadcast that message to everyone on different queues.  The user component could react to the personal message while other components on the page could react to the global message.

With this in mind, my proposal is that a wrapper annotation be added that allows multiple @SendTo and @SendToUser annotations to be applied.  Then if that annotation is present, each annotation is added individually.

An example of what I am proposing is below.

@MessageMapping("/document.save.{id}")
@SendTos(
    globalSends = {
        @SendTo("/topic/document.updated")
    },
    userSends = {
        @SendToUser(value = "/topic/document.save.complete.{id}", broadcast = false)
    }
)
 public Document save(@RequestBody Document document, @DestinationVariable Long id) {
    Document doc = documentService.storeDocument(document);
    return doc;
 }
  

---

Affects: 5.0.6

Issue Links:

• #18811 Annotated class with @SendToUser overrides @SendTo on method

Referenced from: pull request #1846, and commits f4bffea

[spring-projects-issues]

Stéphane Nicoll commented

FWIW @SendTo is also used by other transport protocol (i.e. JMS) so this has to be taken into account before expanding the scope of this annotation.

[spring-projects-issues]

Ethan Mcgee commented

I realized this after I started authoring the changes for the pull request.  I've amended the description with what I actually wound up doing which was a bit simpler than the original proposal and doesn't break backwards compatibility.

[spring-projects-issues]

Rossen Stoyanchev commented

The resulting syntax creates a lot of noise. Return values were only ever meant to fit the more common cases. For anything more advanced, just use SimpMessagingTemplate which is also quite succinct:

@MessageMapping("/document.save.{id}")
public void save(@Payload Document document, @DestinationVariable Long id) {
    Document doc = documentService.storeDocument(document);
    template.convertAndSendToUser("usr", "/topic/document.save.complete." + id, doc);
    template.convertAndSend("/topic/document.updated", doc);
}

[spring-projects-issues]

Ethan Mcgee commented

My only issue with that is that there is a fair amount of logic embedded in the annotations for capturing the username and session to allow sendToUser to work as expected without having to inject the Message as a parameter.  I like the simplicity afforded by the annotations as it means not having to inject additional parameters.  It also means letting spring continue handling capturing the session and user without having to duplicate that logic into a service.

Another way of accomplishing this request without resorting to a wrapper annotation would be to allow the @SendTo and @SendToUser annotations to be used simultaneously.  It would be more limited than what I proposed, but it would cover more use cases while still remaining relatively simple.

[spring-projects-issues]

Rossen Stoyanchev commented

The sessionId shouldn't be required in most cases, and in that sense infrastructure code like SendToMethodReturnValueHandler is not very useful to compare to. Controller code is more explicit and should be simple. If needed you can inject the session id with @Header("simpSessionId") and also the user with Principal.

That said allowing the use of both @SendTo and @SendToUser seems like a logical improvement that would be relatively straightforward to implement. You mentioned that would be more limited? Can you elaborate, how so? Given that each annotation already supports multiple destinations why would multiple annotations be helpful?

[spring-projects-issues]

Ethan Mcgee commented

Allowing only one @SendToUser annotation allows you to only send to session or broadcast to user not both.  Although, I can't think of a specific use case where that would be necessary.

I'll update my pull request so that @SendTo and @SendToUser can be used simultaneously so that it can be reviewed.

[spring-projects-issues]

Rossen Stoyanchev commented

Okay thanks.

[spring-projects-issues]

Ethan Mcgee commented

Pull request updated.

[spring-projects-issues]

Rossen Stoyanchev commented

Thanks for the quick update, but looking at #18811 I'm not sure we can go ahead with this after all, since a method-level @SendToUser can override a class-level @SendTo. This is not as well documented as it should be, and if nothing more, I'll turn this ticket into a documentation update, but we do need to preserve existing semantics.

That said, as I mentioned initially, the design idea for @SendTo and @SendToUser was always to be nothing more than syntactic sugar. Falling back on SimpMessagingTemplate should be a first-class option, but if is more cumbersome than it needs to be, we could consider improvements (Principal based send methods, injecting a session-aware template, etc.).

Also not to forget, a controller method can use SimpMessagingTemplate for general broadcasts in addition to returning a value with @SendToUser.

[spring-projects-issues]

Ethan Mcgee commented

Having the SimpMessagingTemplate know about the message (injected or passed as a parameter) would be nice.  Then all of the pieces around whether to use the username or sessionId could be hidden and that logic wouldn't have to be duplicated into a service in the application.

Right now, to send a message to a user, I'm having to duplicate the logic in the SendToMethodReturnValueHandler around capturing the sessionId and user.  It works, but it is a hack.

[spring-projects-issues]

Rossen Stoyanchev commented

I'm having to duplicate the logic in the SendToMethodReturnValueHandler

I don't think that's fair statement by any measure. The logic in SendToMethodReturnValueHandler is more complicated because it deals with class and method annotations and covers all possible declarative scenarios. By contrast a controller method uses the SimpMessagingTemplate directly and should be 1-2 lines max.

The two possible complications are having to inject the user and session id. Can you confirm that you're using session id for a specific reason? It should not be required in any authenticated scenario where the Principal is not null. So if it comes down to injecting a Principal and using the template, I really don't see any hacks here. Furthermore use of @SendToUser is still an option.

[spring-projects-issues]

Ethan Mcgee commented

Meh, it feels like duplication and a hack because the annotations already provide that functionality.  I'll just stick with using @SendToUser for the method and manually calling convertAndSendTo so I don't have to inject extra parameters into the method signature just to send a return message.

It would still be nicer, imo, to be able to do it all via annotations since the annotations encapsulate everything, but that is just me being picky.

[spring-projects-issues]

Rossen Stoyanchev commented

We'll go ahead with this after all. We will not change the override semantics, but we'll allow both on the method level. 

[spring-projects-issues]

Rossen Stoyanchev commented

@SendTo and @SendToUser are now supported on the same method. I've also updated the docs following the discussion here.