Consistent JSON array result for Flux<T> in Spring MVC

Issue: SPR-15456
This commit is contained in:
Rossen Stoyanchev 2017-04-20 09:17:52 -04:00
parent cc102c2fcd
commit d3b178a812
2 changed files with 31 additions and 30 deletions

View File

@ -27,6 +27,7 @@ import org.apache.commons.logging.LogFactory;
import org.reactivestreams.Publisher; import org.reactivestreams.Publisher;
import org.reactivestreams.Subscriber; import org.reactivestreams.Subscriber;
import org.reactivestreams.Subscription; import org.reactivestreams.Subscription;
import org.springframework.core.MethodParameter; import org.springframework.core.MethodParameter;
import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapter;
import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ReactiveAdapterRegistry;
@ -119,7 +120,6 @@ class ReactiveTypeHandler {
Collection<MediaType> mediaTypes = getMediaTypes(request); Collection<MediaType> mediaTypes = getMediaTypes(request);
Optional<MediaType> mediaType = mediaTypes.stream().filter(MimeType::isConcrete).findFirst(); Optional<MediaType> mediaType = mediaTypes.stream().filter(MimeType::isConcrete).findFirst();
boolean jsonArrayOfStrings = isJsonArrayOfStrings(elementType, mediaType);
if (adapter.isMultiValue()) { if (adapter.isMultiValue()) {
if (mediaTypes.stream().anyMatch(MediaType.TEXT_EVENT_STREAM::includes) || if (mediaTypes.stream().anyMatch(MediaType.TEXT_EVENT_STREAM::includes) ||
@ -133,7 +133,7 @@ class ReactiveTypeHandler {
new JsonEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); new JsonEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue);
return emitter; return emitter;
} }
if (CharSequence.class.isAssignableFrom(elementType) && !jsonArrayOfStrings) { if (CharSequence.class.isAssignableFrom(elementType) && !isJsonStringArray(elementType, mediaType)) {
ResponseBodyEmitter emitter = getEmitter(mediaType.orElse(MediaType.TEXT_PLAIN)); ResponseBodyEmitter emitter = getEmitter(mediaType.orElse(MediaType.TEXT_PLAIN));
new TextEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); new TextEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue);
return emitter; return emitter;
@ -142,7 +142,7 @@ class ReactiveTypeHandler {
// Not streaming... // Not streaming...
DeferredResult<Object> result = new DeferredResult<>(); DeferredResult<Object> result = new DeferredResult<>();
new DeferredResultSubscriber(result, jsonArrayOfStrings).connect(adapter, returnValue); new DeferredResultSubscriber(result, adapter).connect(adapter, returnValue);
WebAsyncUtils.getAsyncManager(request).startDeferredResultProcessing(result, mav); WebAsyncUtils.getAsyncManager(request).startDeferredResultProcessing(result, mav);
return null; return null;
@ -160,7 +160,7 @@ class ReactiveTypeHandler {
} }
@SuppressWarnings("OptionalUsedAsFieldOrParameterType") @SuppressWarnings("OptionalUsedAsFieldOrParameterType")
private boolean isJsonArrayOfStrings(Class<?> elementType, Optional<MediaType> mediaType) { private boolean isJsonStringArray(Class<?> elementType, Optional<MediaType> mediaType) {
return CharSequence.class.isAssignableFrom(elementType) && mediaType.filter(type -> return CharSequence.class.isAssignableFrom(elementType) && mediaType.filter(type ->
MediaType.APPLICATION_JSON.includes(type) || JSON_TYPE.includes(type)).isPresent(); MediaType.APPLICATION_JSON.includes(type) || JSON_TYPE.includes(type)).isPresent();
} }
@ -387,14 +387,14 @@ class ReactiveTypeHandler {
private final DeferredResult<Object> result; private final DeferredResult<Object> result;
private final boolean jsonArrayOfStrings; private final boolean multiValueSource;
private final CollectedValuesList values = new CollectedValuesList(); private final CollectedValuesList values = new CollectedValuesList();
DeferredResultSubscriber(DeferredResult<Object> result, boolean jsonArrayOfStrings) { DeferredResultSubscriber(DeferredResult<Object> result, ReactiveAdapter adapter) {
this.result = result; this.result = result;
this.jsonArrayOfStrings = jsonArrayOfStrings; this.multiValueSource = adapter.isMultiValue();
} }
@ -421,14 +421,14 @@ class ReactiveTypeHandler {
@Override @Override
public void onComplete() { public void onComplete() {
if (this.values.size() > 1) { if (this.values.size() > 1 || this.multiValueSource) {
this.result.setResult(this.values); this.result.setResult(this.values);
} }
else if (this.values.size() == 1) { else if (this.values.size() == 1) {
this.result.setResult(this.values.get(0)); this.result.setResult(this.values.get(0));
} }
else { else {
this.result.setResult(this.jsonArrayOfStrings ? this.values : null); this.result.setResult(null);
} }
} }
} }

View File

@ -16,7 +16,6 @@
package org.springframework.web.servlet.mvc.method.annotation; package org.springframework.web.servlet.mvc.method.annotation;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Set; import java.util.Set;
@ -53,6 +52,7 @@ import org.springframework.web.servlet.HandlerMapping;
import static junit.framework.TestCase.assertNull; import static junit.framework.TestCase.assertNull;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.springframework.web.method.ResolvableMethod.on; import static org.springframework.web.method.ResolvableMethod.on;
@ -127,15 +127,8 @@ public class ReactiveTypeHandlerTests {
@Test @Test
public void deferredResultSubscriberWithNoValues() throws Exception { public void deferredResultSubscriberWithNoValues() throws Exception {
// Empty -> null
MonoProcessor<String> monoEmpty = MonoProcessor.create(); MonoProcessor<String> monoEmpty = MonoProcessor.create();
testDeferredResultSubscriber(monoEmpty, Mono.class, monoEmpty::onComplete, null); testDeferredResultSubscriber(monoEmpty, Mono.class, monoEmpty::onComplete, null);
// Empty -> List[0] when JSON is preferred
this.servletRequest.addHeader("Accept", "application/json");
MonoProcessor<String> monoEmpty2 = MonoProcessor.create();
testDeferredResultSubscriber(monoEmpty2, Mono.class, monoEmpty2::onComplete, new ArrayList<>());
} }
@Test @Test
@ -177,23 +170,31 @@ public class ReactiveTypeHandlerTests {
public void jsonArrayOfStrings() throws Exception { public void jsonArrayOfStrings() throws Exception {
// Empty -> null // Empty -> null
testJsonPreferred("text/plain", null); testJsonNotPreferred("text/plain");
testJsonPreferred("text/plain, application/json", null); testJsonNotPreferred("text/plain, application/json");
testJsonPreferred("text/markdown", null); testJsonNotPreferred("text/markdown");
testJsonPreferred("foo/bar", null); testJsonNotPreferred("foo/bar");
// Empty -> List[0] when JSON is preferred // Empty -> List[0] when JSON is preferred
testJsonPreferred("application/json", Collections.emptyList()); testJsonPreferred("application/json");
testJsonPreferred("application/foo+json", Collections.emptyList()); testJsonPreferred("application/foo+json");
testJsonPreferred("application/json, text/plain", Collections.emptyList()); testJsonPreferred("application/json, text/plain");
testJsonPreferred("*/*, application/json, text/plain", Collections.emptyList()); testJsonPreferred("*/*, application/json, text/plain");
} }
private void testJsonPreferred(String acceptHeaderValue, Object expected) throws Exception { private void testJsonNotPreferred(String acceptHeaderValue) throws Exception {
resetRequest(); resetRequest();
this.servletRequest.addHeader("Accept", acceptHeaderValue); this.servletRequest.addHeader("Accept", acceptHeaderValue);
MonoProcessor<String> mono = MonoProcessor.create(); EmitterProcessor<String> processor = EmitterProcessor.create();
testDeferredResultSubscriber(mono, Mono.class, mono::onComplete, expected); ResponseBodyEmitter emitter = handleValue(processor, Flux.class);
assertNotNull(emitter);
}
private void testJsonPreferred(String acceptHeaderValue) throws Exception {
resetRequest();
this.servletRequest.addHeader("Accept", acceptHeaderValue);
EmitterProcessor<String> processor = EmitterProcessor.create();
testDeferredResultSubscriber(processor, Flux.class, processor::onComplete, Collections.emptyList());
} }
@Test @Test
@ -212,8 +213,8 @@ public class ReactiveTypeHandlerTests {
testSseResponse(false); testSseResponse(false);
// Requested media types are sorted // Requested media types are sorted
testJsonPreferred("text/plain;q=0.8, application/json;q=1.0", Collections.emptyList()); testJsonPreferred("text/plain;q=0.8, application/json;q=1.0");
testJsonPreferred("text/plain, application/json", null); testJsonNotPreferred("text/plain, application/json");
} }
private void testSseResponse(boolean expectSseEimtter) throws Exception { private void testSseResponse(boolean expectSseEimtter) throws Exception {