Add "data:" prefix for multi-line SSE data field with Jackson

[sdeleuze]

@rstoyanchev Based on the Jackson PrettyPrinter based trick with discussed + Jackson capabilities to specify such PrettyPrinter at ObjectWriter level, I think I ended up with a pretty good solution in the sense it is not intrusive and it does not lead to performance penalty. Could you say me if that's fine for you?

Also could you confirm that we should apply that to 4.3.x branch as well?

Issue: SPR-14899

[rstoyanchev]

@sdeleuze I'm wondering why the Jackson2JsonEncoder has to check a hint from ServerSentEventHttpMessageWriter? Couldn't it work like it's done in the AbstractJackson2HttpMessageConverter by checking the content type of the response? If that hint is removed essentially the Jackson encoder and converter have independent, built-in support to pretty print SSE data correctly which is a good place to be.

[sdeleuze]

Good point, I missed the fact that encodeValue() is a private method and that encode() has the mimeType parameter, I will change that and if you are ok with the rest, merge to master.

[rstoyanchev]

Yes

[sdeleuze]

Hum in fact I am afraid we need the SSE hint, because the mime type we get at the encoder level is application/json and not text/event-stream. For Spring MVC that works because we retrieve the content type of the response, but on the encoder side, we don't have access to that so the hint is the only way I think.

[rstoyanchev]

It looks like ServerSentEventMessageWriter passes application/json when it delegates to the Jacson encoder. I'm not sure that should be so actually. It's probably okay for locating the encoder but the content type of the response is text/event-stream after all. Would it cause an issue to pass text/event-stream?

[sdeleuze]

In that Jackson particular case, that would work but nothing prevent an encoder implementation to re-validate the content type by calling canEncode() again + I find a little bit weird to pass not the same content type to canEncode() and encode(), that's why I tend to prefer using the SSE hint since hints are designed for such use case.

[rstoyanchev]

One could ask then if the JacksonEncoder supports "text/event-stream" but it's more like it's the ServerSentEventMessageWriter that supports it and delegates to other encoders. So the JacksonEncoder still only knows how to render JSON but it is also aware that's happening in the context of an SSE response.

Interestingly if JacksonEncoder was in spring-core where it could be shared with non-web modules, it would be harder or not possible to do this. Having a JacksonEncoder in spring-web is paying off and it's okay to make it aware of a hint from ServerSentEventMessageWriter, which is in the codec package above.

In short I'm okay with this. I would only change the hint to be more broadly about indicating that encoding is happening in the context of an SSE response -- something like SSE_CONTENT_HINT and in the Javadoc the "data:" prefixing is just an example of what the hint can be used for.