From a40b54997de16a922bc90827b8c6a5f869a1116f Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Mon, 30 Jan 2023 19:32:26 +0200 Subject: [PATCH 1/8] Support decoding empty DataBuffers for Decoders that support it Some decoders support decoding a proper payload from empty DataBuffers. One good example is the ProtobufDecoder. This patch adds a method canDecodeEmptyDataBuffer() to the Decoder interface, allowing the Decoder to signal that it is able to do that. In this case, the PayloadMethodArgumentResolver should pass the empty DataBuffer to the Decoder, and in all other cases behave like before. --- .../java/org/springframework/core/codec/Decoder.java | 9 +++++++++ .../reactive/PayloadMethodArgumentResolver.java | 9 ++++++--- .../http/codec/protobuf/ProtobufDecoder.java | 5 +++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/codec/Decoder.java b/spring-core/src/main/java/org/springframework/core/codec/Decoder.java index cc5271d5c01f..635705c908a1 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Decoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Decoder.java @@ -53,6 +53,15 @@ public interface Decoder { */ boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType); + /** + * Whether the decoder supports decoding messages from empty data buffers + * @return {@code true} if supported, {@code false} otherwise + * @since 6.0.5 + */ + default boolean canDecodeEmptyDataBuffer() { + return false; + } + /** * Decode a {@link DataBuffer} input stream into a Flux of {@code T}. * @param inputStream the {@code DataBuffer} input stream to decode diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java index adbeea73b0b8..c95dcd7b2156 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java @@ -231,7 +231,7 @@ private Mono decodeContent(MethodParameter parameter, Message message if (decoder.canDecode(elementType, mimeType)) { if (adapter != null && adapter.isMultiValue()) { Flux flux = content - .filter(this::nonEmptyDataBuffer) + .filter(dataBuffer -> nonEmptyDataBuffer(dataBuffer, decoder)) .map(buffer -> decoder.decode(buffer, elementType, mimeType, hints)) .onErrorResume(ex -> Flux.error(handleReadError(parameter, message, ex))); if (isContentRequired) { @@ -245,7 +245,7 @@ private Mono decodeContent(MethodParameter parameter, Message message else { // Single-value (with or without reactive type wrapper) Mono mono = content.next() - .filter(this::nonEmptyDataBuffer) + .filter(dataBuffer -> nonEmptyDataBuffer(dataBuffer, decoder)) .map(buffer -> decoder.decode(buffer, elementType, mimeType, hints)) .onErrorResume(ex -> Mono.error(handleReadError(parameter, message, ex))); if (isContentRequired) { @@ -263,7 +263,10 @@ private Mono decodeContent(MethodParameter parameter, Message message message, parameter, "Cannot decode to [" + targetType + "]" + message)); } - private boolean nonEmptyDataBuffer(DataBuffer buffer) { + private boolean nonEmptyDataBuffer(DataBuffer buffer, Decoder decoder) { + if (decoder.canDecodeEmptyDataBuffer()) { + return true; + } if (buffer.readableByteCount() > 0) { return true; } diff --git a/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java b/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java index 776f7695642b..eb156aafd6e3 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java @@ -125,6 +125,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType return Message.class.isAssignableFrom(elementType.toClass()) && supportsMimeType(mimeType); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public Flux decode(Publisher inputStream, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { From dd06e772f3f266897211ec294e1fc096c60d0209 Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Mon, 30 Jan 2023 20:07:24 +0200 Subject: [PATCH 2/8] fix checkstyle complaint --- .../src/main/java/org/springframework/core/codec/Decoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-core/src/main/java/org/springframework/core/codec/Decoder.java b/spring-core/src/main/java/org/springframework/core/codec/Decoder.java index 635705c908a1..1f1e8cd9a42f 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Decoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Decoder.java @@ -54,7 +54,7 @@ public interface Decoder { boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType); /** - * Whether the decoder supports decoding messages from empty data buffers + * Whether the decoder supports decoding messages from empty data buffers. * @return {@code true} if supported, {@code false} otherwise * @since 6.0.5 */ From c808ba71dd729f559f63b074f09ef5b08cee5ffd Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Mon, 6 Feb 2023 20:17:40 +0200 Subject: [PATCH 3/8] mark Decoders capable of decoding empty DataBuffers as such --- .../org/springframework/core/codec/ByteArrayDecoder.java | 5 +++++ .../org/springframework/core/codec/ByteBufferDecoder.java | 5 +++++ .../org/springframework/core/codec/DataBufferDecoder.java | 5 +++++ .../org/springframework/core/codec/Netty5BufferDecoder.java | 5 +++++ .../org/springframework/core/codec/NettyByteBufDecoder.java | 5 +++++ .../java/org/springframework/core/codec/ResourceDecoder.java | 5 +++++ .../java/org/springframework/core/codec/StringDecoder.java | 5 +++++ 7 files changed, 35 insertions(+) diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java index 65f8222d1774..fb3866fbe656 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java @@ -44,6 +44,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType return (elementType.resolve() == byte[].class && super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public byte[] decode(DataBuffer dataBuffer, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java index 6481ef86dbf1..93ad105241da 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java @@ -47,6 +47,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public ByteBuffer decode(DataBuffer dataBuffer, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java index 788eafdb0e69..23b67ea56370 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java @@ -56,6 +56,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public Flux decode(Publisher input, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java index 150db2b893e6..7ae13cad3e5e 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java @@ -48,6 +48,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public Buffer decode(DataBuffer dataBuffer, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java index 569a5cc83246..988a3f648727 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java @@ -48,6 +48,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public ByteBuf decode(DataBuffer dataBuffer, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java index 4e9552a650a0..6f94595ff3ec 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java @@ -56,6 +56,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public Flux decode(Publisher inputStream, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java index 953ddbd51e42..5e87a1eed972 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java @@ -105,6 +105,11 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType return (elementType.resolve() == String.class && super.canDecode(elementType, mimeType)); } + @Override + public boolean canDecodeEmptyDataBuffer() { + return true; + } + @Override public Flux decode(Publisher input, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { From 4482ea111a99ab9ce6dc35c8106b1baa4d8f6040 Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Mon, 6 Feb 2023 20:18:17 +0200 Subject: [PATCH 4/8] add tests for Decoders that can decode empty DataBuffers --- .../codec/AbstractDecoderTests.java | 50 +++++++++++++++++++ .../codec/protobuf/ProtobufDecoderTests.java | 5 ++ 2 files changed, 55 insertions(+) diff --git a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java index 97454ed162d4..75672d34ef8f 100644 --- a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java +++ b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java @@ -36,6 +36,7 @@ import org.springframework.util.MimeType; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.springframework.core.io.buffer.DataBufferUtils.release; /** * Abstract base class for {@link Decoder} unit tests. Subclasses need to implement @@ -131,6 +132,7 @@ protected void testDecodeAll(Publisher input, ResolvableType out testDecodeError(input, outputType, mimeType, hints); testDecodeCancel(input, outputType, mimeType, hints); testDecodeEmpty(outputType, mimeType, hints); + testDecodeEmptyBuffer(outputType, mimeType, hints); } /** @@ -258,6 +260,25 @@ protected void testDecodeEmpty(ResolvableType outputType, @Nullable MimeType mim StepVerifier.create(result).verifyComplete(); } + /** + * Test a {@link Decoder#decode decode} scenario where the input stream is an empty buffer. + * The output is expected to be filled when the decoder supports it. + * + * @param outputType the desired output type + * @param mimeType the mime type to use for decoding. May be {@code null}. + * @param hints the hints used for decoding. May be {@code null}. + */ + protected void testDecodeEmptyBuffer(ResolvableType outputType, MimeType mimeType, Map hints) { + if (!this.decoder.canDecodeEmptyDataBuffer()) { + return; + } + DataBuffer buffer = this.bufferFactory.allocateBuffer(0); + Object result = this.decoder.decode(buffer, outputType, mimeType, hints); + releaseBufferIfIdentical(buffer, result); + Assert.notNull(result, "result expected to be non null"); + Assert.isAssignable(outputType.toClass(), result.getClass(), "result not of specified type"); + } + // Mono /** @@ -306,6 +327,7 @@ protected void testDecodeToMonoAll(Publisher input, ResolvableTy testDecodeToMonoError(input, outputType, mimeType, hints); testDecodeToMonoCancel(input, outputType, mimeType, hints); testDecodeToMonoEmpty(outputType, mimeType, hints); + testDecodeToMonoEmptyBuffer(outputType, mimeType, hints); } /** @@ -419,6 +441,28 @@ protected void testDecodeToMonoEmpty(ResolvableType outputType, @Nullable MimeTy StepVerifier.create(result).verifyComplete(); } + /** + * Test a {@link Decoder#decodeToMono decode} scenario where the input stream is an empty buffer. + * The output is expected to be filled when the decoder supports it. + * + * @param outputType the desired output type + * @param mimeType the mime type to use for decoding. May be {@code null}. + * @param hints the hints used for decoding. May be {@code null}. + */ + protected void testDecodeToMonoEmptyBuffer(ResolvableType outputType, @Nullable MimeType mimeType, + @Nullable Map hints) { + if (!this.decoder.canDecodeEmptyDataBuffer()) { + return; + } + + DataBuffer buffer = this.bufferFactory.allocateBuffer(0); + Mono result = this.decoder.decodeToMono(Mono.just(buffer), outputType, mimeType, hints) + .doOnNext(value -> releaseBufferIfIdentical(buffer, value)); + StepVerifier.create(result) + .expectNextMatches(next -> outputType.toClass().isInstance(next)) + .verifyComplete(); + } + /** * Creates a deferred {@link DataBuffer} containing the given bytes. * @param bytes the bytes that are to be stored in the buffer @@ -432,6 +476,12 @@ protected Mono dataBuffer(byte[] bytes) { }); } + private void releaseBufferIfIdentical(DataBuffer buffer, Object value) { + if (buffer == value) { + release(buffer); + } + } + /** * Exception used in {@link #testDecodeError} and {@link #testDecodeToMonoError} */ diff --git a/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java index 81b26e4f7df2..0db3293837e5 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java @@ -221,6 +221,11 @@ public void exceedMaxSize() { testDecode(input, Msg.class, step -> step.verifyError(DecodingException.class)); } + @Test + public void decodeEmpty() { + testDecodeEmptyBuffer(ResolvableType.forClass(Msg.class), null, null); + } + private Mono dataBuffer(Msg msg) { return Mono.fromCallable(() -> { byte[] bytes = msg.toByteArray(); From c29c51561200c8d674ccd964732b5572296bd136 Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Mon, 6 Feb 2023 20:52:41 +0200 Subject: [PATCH 5/8] fix checkstyle --- .../core/testfixture/codec/AbstractDecoderTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java index 75672d34ef8f..3cedf42050a7 100644 --- a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java +++ b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java @@ -450,14 +450,15 @@ protected void testDecodeToMonoEmpty(ResolvableType outputType, @Nullable MimeTy * @param hints the hints used for decoding. May be {@code null}. */ protected void testDecodeToMonoEmptyBuffer(ResolvableType outputType, @Nullable MimeType mimeType, - @Nullable Map hints) { + @Nullable Map hints) { + if (!this.decoder.canDecodeEmptyDataBuffer()) { return; } DataBuffer buffer = this.bufferFactory.allocateBuffer(0); Mono result = this.decoder.decodeToMono(Mono.just(buffer), outputType, mimeType, hints) - .doOnNext(value -> releaseBufferIfIdentical(buffer, value)); + .doOnNext(value -> releaseBufferIfIdentical(buffer, value)); StepVerifier.create(result) .expectNextMatches(next -> outputType.toClass().isInstance(next)) .verifyComplete(); From a29f5576f51033885dda605d0bad0fa9be16a263 Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Mon, 6 Feb 2023 21:36:10 +0200 Subject: [PATCH 6/8] Fix failing test in spring-messaging The `filterNonEmptyDataBuffer()` method was added via the fix for gh-26344. The comment states that this fix was done because for some Decoders, an empty input may not yield a valid message. Since this is now fixed by asking the decoder whether it can decode an empty DataBuffer, the test is now able to yield a value from empty input. --- .../rsocket/RSocketClientToServerIntegrationTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-messaging/src/test/java/org/springframework/messaging/rsocket/RSocketClientToServerIntegrationTests.java b/spring-messaging/src/test/java/org/springframework/messaging/rsocket/RSocketClientToServerIntegrationTests.java index 12e416645d37..e8c3cfea658a 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/rsocket/RSocketClientToServerIntegrationTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/rsocket/RSocketClientToServerIntegrationTests.java @@ -167,7 +167,7 @@ public void echoChannel() { @Test // gh-26344 public void echoChannelWithEmptyInput() { Flux result = requester.route("echo-channel-empty").data(Flux.empty()).retrieveFlux(String.class); - StepVerifier.create(result).verifyComplete(); + StepVerifier.create(result).expectNext(" echoed").verifyComplete(); } @Test From 4ece71dbd3d60c158e071037d8bfda7f04d9c792 Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Wed, 8 Feb 2023 17:57:26 +0200 Subject: [PATCH 7/8] rename canDecodeEmptyDataBuffer to canDecodeEmptyMessage, to better reflect that its only meant for complete messages --- .../org/springframework/core/codec/ByteArrayDecoder.java | 2 +- .../org/springframework/core/codec/ByteBufferDecoder.java | 2 +- .../org/springframework/core/codec/DataBufferDecoder.java | 2 +- .../main/java/org/springframework/core/codec/Decoder.java | 6 ++++-- .../org/springframework/core/codec/Netty5BufferDecoder.java | 2 +- .../org/springframework/core/codec/NettyByteBufDecoder.java | 2 +- .../org/springframework/core/codec/ResourceDecoder.java | 2 +- .../java/org/springframework/core/codec/StringDecoder.java | 2 +- .../core/testfixture/codec/AbstractDecoderTests.java | 4 ++-- .../annotation/reactive/PayloadMethodArgumentResolver.java | 2 +- .../http/codec/protobuf/ProtobufDecoder.java | 2 +- 11 files changed, 15 insertions(+), 13 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java index fb3866fbe656..06e224a989af 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java @@ -45,7 +45,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java index 93ad105241da..143018ee9b03 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java @@ -48,7 +48,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java index 23b67ea56370..2fa7fddcfa06 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java @@ -57,7 +57,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/Decoder.java b/spring-core/src/main/java/org/springframework/core/codec/Decoder.java index 1f1e8cd9a42f..52983ace05e1 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Decoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Decoder.java @@ -54,11 +54,13 @@ public interface Decoder { boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType); /** - * Whether the decoder supports decoding messages from empty data buffers. + * Whether the decoder supports decoding an Object of its target type from an + * empty message. When it is true, the decoder will always return a non-null + * value from its {@code decode} method when an empty message is decoded. * @return {@code true} if supported, {@code false} otherwise * @since 6.0.5 */ - default boolean canDecodeEmptyDataBuffer() { + default boolean canDecodeEmptyMessage() { return false; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java index 7ae13cad3e5e..cb415941ef03 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Netty5BufferDecoder.java @@ -49,7 +49,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java index 988a3f648727..df81878a4616 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/NettyByteBufDecoder.java @@ -49,7 +49,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java index 6f94595ff3ec..7a63e6ea8c70 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java @@ -57,7 +57,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java index 5e87a1eed972..19c8e7c3bbe4 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java @@ -106,7 +106,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } diff --git a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java index 3cedf42050a7..8c652f0ceaa0 100644 --- a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java +++ b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java @@ -269,7 +269,7 @@ protected void testDecodeEmpty(ResolvableType outputType, @Nullable MimeType mim * @param hints the hints used for decoding. May be {@code null}. */ protected void testDecodeEmptyBuffer(ResolvableType outputType, MimeType mimeType, Map hints) { - if (!this.decoder.canDecodeEmptyDataBuffer()) { + if (!this.decoder.canDecodeEmptyMessage()) { return; } DataBuffer buffer = this.bufferFactory.allocateBuffer(0); @@ -452,7 +452,7 @@ protected void testDecodeToMonoEmpty(ResolvableType outputType, @Nullable MimeTy protected void testDecodeToMonoEmptyBuffer(ResolvableType outputType, @Nullable MimeType mimeType, @Nullable Map hints) { - if (!this.decoder.canDecodeEmptyDataBuffer()) { + if (!this.decoder.canDecodeEmptyMessage()) { return; } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java index c95dcd7b2156..1b1e33f5a8d7 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/PayloadMethodArgumentResolver.java @@ -264,7 +264,7 @@ private Mono decodeContent(MethodParameter parameter, Message message } private boolean nonEmptyDataBuffer(DataBuffer buffer, Decoder decoder) { - if (decoder.canDecodeEmptyDataBuffer()) { + if (decoder.canDecodeEmptyMessage()) { return true; } if (buffer.readableByteCount() > 0) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java b/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java index eb156aafd6e3..b8383a4a7984 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/protobuf/ProtobufDecoder.java @@ -126,7 +126,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType } @Override - public boolean canDecodeEmptyDataBuffer() { + public boolean canDecodeEmptyMessage() { return true; } From 0d03e2cc70d37016a44e72ca9429f4ee17cc5711 Mon Sep 17 00:00:00 2001 From: Martin Schrodt Date: Wed, 8 Feb 2023 18:40:26 +0200 Subject: [PATCH 8/8] improve testDecodeToMonoEmptyMessage() to use 2 empty source buffers, to better reflect the usecase --- .../codec/AbstractDecoderTests.java | 38 ++++++++++++++----- .../codec/protobuf/ProtobufDecoderTests.java | 2 +- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java index 8c652f0ceaa0..02830794913c 100644 --- a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java +++ b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/codec/AbstractDecoderTests.java @@ -115,6 +115,7 @@ protected void testDecodeAll(Publisher input, Class *
  • {@link #testDecodeError(Publisher, ResolvableType, MimeType, Map)}
    • *
  • {@link #testDecodeCancel(Publisher, ResolvableType, MimeType, Map)}
    • *
  • {@link #testDecodeEmpty(ResolvableType, MimeType, Map)}
    • + *
  • {@link #testDecodeEmptyMessage(ResolvableType, MimeType, Map)}
    • * * * @param input the input to be provided to the decoder @@ -132,7 +133,7 @@ protected void testDecodeAll(Publisher input, ResolvableType out testDecodeError(input, outputType, mimeType, hints); testDecodeCancel(input, outputType, mimeType, hints); testDecodeEmpty(outputType, mimeType, hints); - testDecodeEmptyBuffer(outputType, mimeType, hints); + testDecodeEmptyMessage(outputType, mimeType, hints); } /** @@ -268,13 +269,13 @@ protected void testDecodeEmpty(ResolvableType outputType, @Nullable MimeType mim * @param mimeType the mime type to use for decoding. May be {@code null}. * @param hints the hints used for decoding. May be {@code null}. */ - protected void testDecodeEmptyBuffer(ResolvableType outputType, MimeType mimeType, Map hints) { + protected void testDecodeEmptyMessage(ResolvableType outputType, MimeType mimeType, Map hints) { if (!this.decoder.canDecodeEmptyMessage()) { return; } DataBuffer buffer = this.bufferFactory.allocateBuffer(0); Object result = this.decoder.decode(buffer, outputType, mimeType, hints); - releaseBufferIfIdentical(buffer, result); + releaseDataBufferIfIdentical(buffer, result); Assert.notNull(result, "result expected to be non null"); Assert.isAssignable(outputType.toClass(), result.getClass(), "result not of specified type"); } @@ -310,6 +311,7 @@ protected void testDecodeToMonoAll(Publisher input, *
  • {@link #testDecodeToMonoError(Publisher, ResolvableType, MimeType, Map)}
    • *
  • {@link #testDecodeToMonoCancel(Publisher, ResolvableType, MimeType, Map)}
    • *
  • {@link #testDecodeToMonoEmpty(ResolvableType, MimeType, Map)}
    • + *
  • {@link #testDecodeToMonoEmptyMessage(ResolvableType, MimeType, Map)}
    • * * * @param input the input to be provided to the decoder @@ -327,7 +329,7 @@ protected void testDecodeToMonoAll(Publisher input, ResolvableTy testDecodeToMonoError(input, outputType, mimeType, hints); testDecodeToMonoCancel(input, outputType, mimeType, hints); testDecodeToMonoEmpty(outputType, mimeType, hints); - testDecodeToMonoEmptyBuffer(outputType, mimeType, hints); + testDecodeToMonoEmptyMessage(outputType, mimeType, hints); } /** @@ -449,16 +451,19 @@ protected void testDecodeToMonoEmpty(ResolvableType outputType, @Nullable MimeTy * @param mimeType the mime type to use for decoding. May be {@code null}. * @param hints the hints used for decoding. May be {@code null}. */ - protected void testDecodeToMonoEmptyBuffer(ResolvableType outputType, @Nullable MimeType mimeType, + protected void testDecodeToMonoEmptyMessage(ResolvableType outputType, @Nullable MimeType mimeType, @Nullable Map hints) { if (!this.decoder.canDecodeEmptyMessage()) { return; } - DataBuffer buffer = this.bufferFactory.allocateBuffer(0); - Mono result = this.decoder.decodeToMono(Mono.just(buffer), outputType, mimeType, hints) - .doOnNext(value -> releaseBufferIfIdentical(buffer, value)); + Flux source = Flux.range(0, 2) + .map(i -> this.bufferFactory.allocateBuffer(0)); + + Mono result = this.decoder.decodeToMono(source, outputType, mimeType, hints) + .doOnNext(this::releaseIfDataBuffer); + StepVerifier.create(result) .expectNextMatches(next -> outputType.toClass().isInstance(next)) .verifyComplete(); @@ -477,12 +482,27 @@ protected Mono dataBuffer(byte[] bytes) { }); } - private void releaseBufferIfIdentical(DataBuffer buffer, Object value) { + /** + * If {@code value} is referentially identical to {@code buffer}, release it. + * @param buffer the {@link DataBuffer} that is compared + * @param value the {@link Object} that is compared + */ + private void releaseDataBufferIfIdentical(DataBuffer buffer, Object value) { if (buffer == value) { release(buffer); } } + /** + * If {@code value} is a {@link DataBuffer}, release it. + * @param value the {@link Object} that is checked + */ + private void releaseIfDataBuffer(Object value) { + if (value instanceof DataBuffer) { + release((DataBuffer) value); + } + } + /** * Exception used in {@link #testDecodeError} and {@link #testDecodeToMonoError} */ diff --git a/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java index 0db3293837e5..72711a141352 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/protobuf/ProtobufDecoderTests.java @@ -223,7 +223,7 @@ public void exceedMaxSize() { @Test public void decodeEmpty() { - testDecodeEmptyBuffer(ResolvableType.forClass(Msg.class), null, null); + testDecodeEmptyMessage(ResolvableType.forClass(Msg.class), null, null); } private Mono dataBuffer(Msg msg) {