From 69bcdfc17f6b65b6ea04722a56f2aa5c0a1bfb08 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 21 Oct 2019 14:23:34 +0200 Subject: [PATCH] Polishing --- .../codec/ResourceRegionEncoderTests.java | 11 +-- .../SynchronossPartHttpMessageReader.java | 13 +++- ...SynchronossPartHttpMessageReaderTests.java | 72 +++++++++++-------- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/spring-core/src/test/java/org/springframework/core/codec/ResourceRegionEncoderTests.java b/spring-core/src/test/java/org/springframework/core/codec/ResourceRegionEncoderTests.java index 18f374a005e..8d4a919fb8a 100644 --- a/spring-core/src/test/java/org/springframework/core/codec/ResourceRegionEncoderTests.java +++ b/spring-core/src/test/java/org/springframework/core/codec/ResourceRegionEncoderTests.java @@ -30,6 +30,7 @@ import reactor.test.StepVerifier; import org.springframework.core.ResolvableType; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; +import org.springframework.core.io.buffer.AbstractLeakCheckingTests; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.LeakAwareDataBufferFactory; @@ -45,18 +46,10 @@ import static org.assertj.core.api.Assertions.assertThat; * Test cases for {@link ResourceRegionEncoder} class. * @author Brian Clozel */ -class ResourceRegionEncoderTests { +class ResourceRegionEncoderTests extends AbstractLeakCheckingTests { private ResourceRegionEncoder encoder = new ResourceRegionEncoder(); - private LeakAwareDataBufferFactory bufferFactory = new LeakAwareDataBufferFactory(); - - - @AfterEach - void tearDown() throws Exception { - this.bufferFactory.checkForLeaks(); - } - @Test void canEncode() { ResolvableType resourceRegion = ResolvableType.forClass(ResourceRegion.class); diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java index 642e16688f7..338222e7c55 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java @@ -75,10 +75,17 @@ import org.springframework.util.Assert; */ public class SynchronossPartHttpMessageReader extends LoggingCodecSupport implements HttpMessageReader { - private final DataBufferFactory bufferFactory = new DefaultDataBufferFactory(); + private final DataBufferFactory bufferFactory; private final PartBodyStreamStorageFactory streamStorageFactory = new DefaultPartBodyStreamStorageFactory(); + public SynchronossPartHttpMessageReader() { + this.bufferFactory = new DefaultDataBufferFactory(); + } + + SynchronossPartHttpMessageReader(DataBufferFactory bufferFactory) { + this.bufferFactory = bufferFactory; + } @Override public List getReadableMediaTypes() { @@ -155,15 +162,15 @@ public class SynchronossPartHttpMessageReader extends LoggingCodecSupport implem parser.write(resultBytes); } catch (IOException ex) { - listener.onError("Exception thrown providing input to the parser", ex); + listener.onError("Exception thrown while providing input to the parser", ex); } finally { DataBufferUtils.release(buffer); } }, ex -> { try { - listener.onError("Request body input error", ex); parser.close(); + listener.onError("Request body input error", ex); } catch (IOException ex2) { listener.onError("Exception thrown while closing the parser", ex2); diff --git a/spring-web/src/test/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReaderTests.java b/spring-web/src/test/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReaderTests.java index b9fc46381ab..b8adb9e6044 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReaderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReaderTests.java @@ -17,19 +17,23 @@ package org.springframework.http.codec.multipart; import java.io.File; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Map; import org.junit.jupiter.api.Test; +import org.reactivestreams.Subscription; +import reactor.core.publisher.BaseSubscriber; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import org.springframework.core.ResolvableType; import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.buffer.AbstractLeakCheckingTests; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; -import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.core.io.buffer.support.DataBufferTestUtils; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.client.MultipartBodyBuilder; @@ -49,17 +53,20 @@ import static org.springframework.http.MediaType.MULTIPART_FORM_DATA; * * @author Sebastien Deleuze * @author Rossen Stoyanchev + * @author Brian Clozel */ -public class SynchronossPartHttpMessageReaderTests { +public class SynchronossPartHttpMessageReaderTests extends AbstractLeakCheckingTests { private final MultipartHttpMessageReader reader = - new MultipartHttpMessageReader(new SynchronossPartHttpMessageReader()); + new MultipartHttpMessageReader(new SynchronossPartHttpMessageReader(this.bufferFactory)); + private static final ResolvableType PARTS_ELEMENT_TYPE = + forClassWithGenerics(MultiValueMap.class, String.class, Part.class); @Test - public void canRead() { + void canRead() { assertThat(this.reader.canRead( - forClassWithGenerics(MultiValueMap.class, String.class, Part.class), + PARTS_ELEMENT_TYPE, MediaType.MULTIPART_FORM_DATA)).isTrue(); assertThat(this.reader.canRead( @@ -80,37 +87,30 @@ public class SynchronossPartHttpMessageReaderTests { } @Test - public void resolveParts() { + void resolveParts() { ServerHttpRequest request = generateMultipartRequest(); - ResolvableType elementType = forClassWithGenerics(MultiValueMap.class, String.class, Part.class); - MultiValueMap parts = this.reader.readMono(elementType, request, emptyMap()).block(); - assertThat(parts.size()).isEqualTo(2); + MultiValueMap parts = this.reader.readMono(PARTS_ELEMENT_TYPE, request, emptyMap()).block(); + + assertThat(parts).containsOnlyKeys("fooPart", "barPart"); - assertThat(parts.containsKey("fooPart")).isTrue(); Part part = parts.getFirst("fooPart"); - boolean condition1 = part instanceof FilePart; - assertThat(condition1).isTrue(); + assertThat(part).isInstanceOf(FilePart.class); assertThat(part.name()).isEqualTo("fooPart"); assertThat(((FilePart) part).filename()).isEqualTo("foo.txt"); DataBuffer buffer = DataBufferUtils.join(part.content()).block(); - assertThat(buffer.readableByteCount()).isEqualTo(12); - byte[] byteContent = new byte[12]; - buffer.read(byteContent); - assertThat(new String(byteContent)).isEqualTo("Lorem Ipsum."); + assertThat(DataBufferTestUtils.dumpString(buffer, StandardCharsets.UTF_8)).isEqualTo("Lorem Ipsum."); + DataBufferUtils.release(buffer); - assertThat(parts.containsKey("barPart")).isTrue(); part = parts.getFirst("barPart"); - boolean condition = part instanceof FormFieldPart; - assertThat(condition).isTrue(); + assertThat(part).isInstanceOf(FormFieldPart.class); assertThat(part.name()).isEqualTo("barPart"); assertThat(((FormFieldPart) part).value()).isEqualTo("bar"); } @Test // SPR-16545 - public void transferTo() { + void transferTo() { ServerHttpRequest request = generateMultipartRequest(); - ResolvableType elementType = forClassWithGenerics(MultiValueMap.class, String.class, Part.class); - MultiValueMap parts = this.reader.readMono(elementType, request, emptyMap()).block(); + MultiValueMap parts = this.reader.readMono(PARTS_ELEMENT_TYPE, request, emptyMap()).block(); assertThat(parts).isNotNull(); FilePart part = (FilePart) parts.getFirst("fooPart"); @@ -125,10 +125,18 @@ public class SynchronossPartHttpMessageReaderTests { } @Test - public void bodyError() { + void bodyError() { ServerHttpRequest request = generateErrorMultipartRequest(); - ResolvableType elementType = forClassWithGenerics(MultiValueMap.class, String.class, Part.class); - StepVerifier.create(this.reader.readMono(elementType, request, emptyMap())).verifyError(); + StepVerifier.create(this.reader.readMono(PARTS_ELEMENT_TYPE, request, emptyMap())).verifyError(); + } + + @Test + void readPartsWithoutDemand() { + ServerHttpRequest request = generateMultipartRequest(); + Mono> parts = this.reader.readMono(PARTS_ELEMENT_TYPE, request, emptyMap()); + ZeroDemandSubscriber subscriber = new ZeroDemandSubscriber(); + parts.subscribe(subscriber); + subscriber.cancel(); } @@ -142,16 +150,24 @@ public class SynchronossPartHttpMessageReaderTests { new MultipartHttpMessageWriter() .write(Mono.just(partsBuilder.build()), null, MediaType.MULTIPART_FORM_DATA, outputMessage, null) .block(Duration.ofSeconds(5)); - + Flux requestBody = outputMessage.getBody().map(buffer -> this.bufferFactory.wrap(buffer.asByteBuffer())); return MockServerHttpRequest.post("/") .contentType(outputMessage.getHeaders().getContentType()) - .body(outputMessage.getBody()); + .body(requestBody); } private ServerHttpRequest generateErrorMultipartRequest() { return MockServerHttpRequest.post("/") .header(CONTENT_TYPE, MULTIPART_FORM_DATA.toString()) - .body(Flux.just(new DefaultDataBufferFactory().wrap("invalid content".getBytes()))); + .body(Flux.just(this.bufferFactory.wrap("invalid content".getBytes()))); + } + + private static class ZeroDemandSubscriber extends BaseSubscriber> { + + @Override + protected void hookOnSubscribe(Subscription subscription) { + // Just subscribe without requesting + } } }