In some protocols (HTTP, TCP) the stream of bytes needs to be parsed to be decoded and likewise chunks of encoded content can written out. In other protocols (e.g. RSocket, WebSocket) input and output streams are already split into discrete messages so that each DataBuffer can be decoded and is encoded in full.
Currently Encoder and Decoder contracts are good the former but unnecessarily cumbersome requiring to wrapping in a Mono and joining with DataBufferUtils. Even in WebFlux there are plenty of cases where we've run into this (multipart, form data, etc). We should add decodeDataBuffer to Decoder and encodeValue in Encoder.
Sub-classes of AbstractDataBufferDecoder already have a protected decodeDataBuffer method that does this (on joined buffers). I suspect Jackson and Jaxb2 implementations of decodeToMono could also skip the asynchronous parsing for decoding to Mono and extend this class, see #22783. Likewise Encoder implementations either have methods for encoding one value, or could benefit from one.
Comment From: tonydamage
This change breaks API compatibility of Encoder, Decoders writen against previous Spring versions.
The use of defaultmethods which throws UnsupportedOperationException is not good pattern, since
it hides the need to implement encodeValue on update of Spring Framework, and moves error from compile time to runtime (which is usually bad).
Looking at implementation this method is not optional, but required if working with Monos.
Better way to handle this change is any of these:
- default implementation provides backwards compatibility
- user (HttpEncoder) is able to detect if method is not implemented
- make method not default - this will break build of encoders which do not implement it, also raises better exception in runtime if wired against encoder which does not implement it.
Comment From: rstoyanchev
Looking at implementation this method is not optional, but required if working with Monos.
For Decoder there is decodeToMono and this new method is entirely optional. It is used in the new spring-messaging reactive infrastructure, and for spring-web it result primarily in internal refactoring. I don't expect that should be able to cause you any trouble.
For Encoder is where I'm guessing you have been impacted? It was initially also meant to be entirely optional but a later optimization was made in EncoderHttpMessagWriter for better handling of single values. My apologies for that. In retrospect that change in EncoderHttpMessagWriter could have been made in backwards compatible manner but it's a bit late for that now.
Comment From: seantsb
This caused some massive headaches on our end. Breaking changes like that should be noted in the release notes--can we update those to reflect that?
Comment From: seantsb
I guess the right place to make a note would be the wiki page -- I don't think I have edit access but maybe someone can make that note for 5.2?
Comment From: rstoyanchev
@seantsb are you referring to the Encoder#encodeValue method and the fact that it must be implemented in order to work with ServerSentEventHttpMessageWriter, or if not could you provide more details?
Comment From: seantsb
yep exactly; and more specifically that it will break existing implementations when doing the minor version upgrade to 5.2
Comment From: rstoyanchev
Done.
Comment From: seantsb
should it be under 5.2 instead?
Comment From: rstoyanchev
Yes it should be, thank you.
Comment From: seantsb
Thank you!