From 27b5d2b288bdaf8c791dc28227e69cfa36cc68af Mon Sep 17 00:00:00 2001 From: Ilya Lukyanovich Date: Fri, 31 May 2019 16:48:02 +0300 Subject: [PATCH 1/2] Optional @RequestPart Mono arg resolves to Mono.empty Closes gh-23060 --- .../RequestPartMethodArgumentResolver.java | 6 +- ...equestPartMethodArgumentResolverTests.java | 61 +++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java index 6682f39c20e..7241078c06f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java @@ -99,13 +99,9 @@ public class RequestPartMethodArgumentResolver extends AbstractMessageReaderArgu // Mono or Flux MethodParameter elementType = parameter.nested(); if (Part.class.isAssignableFrom(elementType.getNestedParameterType())) { - parts = (adapter.isMultiValue() ? parts : parts.take(1)); return Mono.just(adapter.fromPublisher(parts)); } - // We have to decode the content for each part, one at a time - if (adapter.isMultiValue()) { - return Mono.just(decodePartValues(parts, elementType, bindingContext, exchange, isRequired)); - } + return Mono.just(adapter.fromPublisher(decodePartValues(parts, elementType, bindingContext, exchange, isRequired))); } // or Mono diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java index fa4a3ef059d..093d37b75dc 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java @@ -60,6 +60,7 @@ import static org.springframework.web.method.MvcAnnotationPredicates.*; /** * Unit tests for {@link RequestPartMethodArgumentResolver}. * @author Rossen Stoyanchev + * @author Ilya Lukyanovich */ public class RequestPartMethodArgumentResolverTests { @@ -131,6 +132,15 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("James", actual.get(1).getName()); } + @Test + public void listPersonNotRequired() { + MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(List.class, Person.class); + MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); + List actual = resolveArgument(param, bodyBuilder); + + assertThat(actual).isEmpty(); + } + @Test public void monoPerson() { MethodParameter param = this.testMethod.annot(requestPart()).arg(Mono.class, Person.class); @@ -141,6 +151,15 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("Jones", actual.block().getName()); } + @Test + public void monoPersonNotRequired() { + MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Mono.class, Person.class); + MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); + Mono actual = resolveArgument(param, bodyBuilder); + + assertThat(actual.block()).isNull(); + } + @Test public void fluxPerson() { MethodParameter param = this.testMethod.annot(requestPart()).arg(Flux.class, Person.class); @@ -154,6 +173,15 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("James", persons.get(1).getName()); } + @Test + public void fluxPersonNotRequired() { + MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Flux.class, Person.class); + MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); + Flux actual = resolveArgument(param, bodyBuilder); + + assertThat(actual.collectList().block()).isEmpty(); + } + @Test public void part() { MethodParameter param = this.testMethod.annot(requestPart()).arg(Part.class); @@ -177,6 +205,15 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("{\"name\":\"James\"}", partToUtf8String(actual.get(1))); } + @Test + public void listPartNotRequired() { + MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(List.class, Part.class); + MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); + List actual = resolveArgument(param, bodyBuilder); + + assertThat(actual).isEmpty(); + } + @Test public void monoPart() { MethodParameter param = this.testMethod.annot(requestPart()).arg(Mono.class, Part.class); @@ -188,6 +225,15 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("{\"name\":\"Jones\"}", partToUtf8String(part)); } + @Test + public void monoPartNotRequired() { + MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Mono.class, Part.class); + MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); + Mono actual = resolveArgument(param, bodyBuilder); + + assertThat(actual.block()).isNull(); + } + @Test public void fluxPart() { MethodParameter param = this.testMethod.annot(requestPart()).arg(Flux.class, Part.class); @@ -201,6 +247,15 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("{\"name\":\"James\"}", partToUtf8String(parts.get(1))); } + @Test + public void fluxPartNotRequired() { + MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Flux.class, Part.class); + MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); + Flux actual = resolveArgument(param, bodyBuilder); + + assertThat(actual.collectList().block()).isEmpty(); + } + @Test public void personRequired() { MethodParameter param = this.testMethod.annot(requestPart()).arg(Person.class); @@ -278,7 +333,13 @@ public class RequestPartMethodArgumentResolverTests { @RequestPart("name") Flux partFlux, @RequestPart("name") List partList, @RequestPart(name = "anotherPart", required = false) Person anotherPerson, + @RequestPart(name = "name", required = false) Mono anotherPersonMono, + @RequestPart(name = "name", required = false) Flux anotherPersonFlux, + @RequestPart(name = "name", required = false) List anotherPersonList, @RequestPart(name = "anotherPart", required = false) Part anotherPart, + @RequestPart(name = "name", required = false) Mono anotherPartMono, + @RequestPart(name = "name", required = false) Flux anotherPartFlux, + @RequestPart(name = "name", required = false) List anotherPartList, Person notAnnotated) {} From 29b765909475edab507697f4ee2c7795c3125efb Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 2 Jul 2019 09:30:06 +0100 Subject: [PATCH 2/2] Polish --- .../RequestPartMethodArgumentResolver.java | 11 +++--- ...equestPartMethodArgumentResolverTests.java | 36 ++++++++++--------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java index 7241078c06f..85e8d19a6ea 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -96,15 +96,12 @@ public class RequestPartMethodArgumentResolver extends AbstractMessageReaderArgu ReactiveAdapter adapter = getAdapterRegistry().getAdapter(parameter.getParameterType()); if (adapter != null) { - // Mono or Flux MethodParameter elementType = parameter.nested(); - if (Part.class.isAssignableFrom(elementType.getNestedParameterType())) { - return Mono.just(adapter.fromPublisher(parts)); - } - return Mono.just(adapter.fromPublisher(decodePartValues(parts, elementType, bindingContext, exchange, isRequired))); + return Mono.just(adapter.fromPublisher( + Part.class.isAssignableFrom(elementType.getNestedParameterType()) ? + parts : decodePartValues(parts, elementType, bindingContext, exchange, isRequired))); } - // or Mono return decodePartValues(parts, parameter, bindingContext, exchange, isRequired) .next().cast(Object.class); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java index 093d37b75dc..4256a0f49ec 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestPartMethodArgumentResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,9 +53,13 @@ import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebInputException; -import static org.junit.Assert.*; -import static org.springframework.core.ResolvableType.*; -import static org.springframework.web.method.MvcAnnotationPredicates.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.springframework.core.ResolvableType.forClass; +import static org.springframework.web.method.MvcAnnotationPredicates.requestPart; /** * Unit tests for {@link RequestPartMethodArgumentResolver}. @@ -132,13 +136,13 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("James", actual.get(1).getName()); } - @Test + @Test // gh-23060 public void listPersonNotRequired() { MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(List.class, Person.class); MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); List actual = resolveArgument(param, bodyBuilder); - assertThat(actual).isEmpty(); + assertEquals(Collections.emptyList(), actual); } @Test @@ -151,13 +155,13 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("Jones", actual.block().getName()); } - @Test + @Test // gh-23060 public void monoPersonNotRequired() { MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Mono.class, Person.class); MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); Mono actual = resolveArgument(param, bodyBuilder); - assertThat(actual.block()).isNull(); + assertNull(actual.block()); } @Test @@ -173,13 +177,13 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("James", persons.get(1).getName()); } - @Test + @Test // gh-23060 public void fluxPersonNotRequired() { MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Flux.class, Person.class); MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); Flux actual = resolveArgument(param, bodyBuilder); - assertThat(actual.collectList().block()).isEmpty(); + assertEquals(Collections.emptyList(), actual.collectList().block()); } @Test @@ -205,13 +209,13 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("{\"name\":\"James\"}", partToUtf8String(actual.get(1))); } - @Test + @Test // gh-23060 public void listPartNotRequired() { MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(List.class, Part.class); MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); List actual = resolveArgument(param, bodyBuilder); - assertThat(actual).isEmpty(); + assertEquals(Collections.emptyList(), actual); } @Test @@ -225,13 +229,13 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("{\"name\":\"Jones\"}", partToUtf8String(part)); } - @Test + @Test // gh-23060 public void monoPartNotRequired() { MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Mono.class, Part.class); MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); Mono actual = resolveArgument(param, bodyBuilder); - assertThat(actual.block()).isNull(); + assertNull(actual.block()); } @Test @@ -247,13 +251,13 @@ public class RequestPartMethodArgumentResolverTests { assertEquals("{\"name\":\"James\"}", partToUtf8String(parts.get(1))); } - @Test + @Test // gh-23060 public void fluxPartNotRequired() { MethodParameter param = this.testMethod.annot(requestPart().notRequired()).arg(Flux.class, Part.class); MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder(); Flux actual = resolveArgument(param, bodyBuilder); - assertThat(actual.collectList().block()).isEmpty(); + assertEquals(Collections.emptyList(), actual.collectList().block()); } @Test