From 043c7070e3dd73415c96b5ad5b0de0809904150e Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 11 Apr 2017 11:43:16 -0400 Subject: [PATCH] Polish default content type change Issue: SPR-15367 --- .../ContentNegotiationManagerFactoryBean.java | 12 +++++- .../FixedContentNegotiationStrategy.java | 37 ++++++++++++------- ...entNegotiationManagerFactoryBeanTests.java | 27 ++++++-------- .../ContentNegotiationConfigurer.java | 19 +++++----- .../ContentNegotiationConfigurerTests.java | 17 +++++---- 5 files changed, 64 insertions(+), 48 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java index 395de4772a..3861dc3ab7 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java +++ b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java @@ -233,7 +233,17 @@ public class ContentNegotiationManagerFactoryBean *

By default this is not set. * @see #setDefaultContentTypeStrategy */ - public void setDefaultContentType(List contentTypes) { + public void setDefaultContentType(MediaType contentType) { + this.defaultNegotiationStrategy = new FixedContentNegotiationStrategy(contentType); + } + + /** + * Set the default content types to use when no content type is requested. + *

By default this is not set. + * @see #setDefaultContentTypeStrategy + * @since 5.0 + */ + public void setDefaultContentTypes(List contentTypes) { this.defaultNegotiationStrategy = new FixedContentNegotiationStrategy(contentTypes); } diff --git a/spring-web/src/main/java/org/springframework/web/accept/FixedContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/FixedContentNegotiationStrategy.java index cb292a4d3e..6c913f651b 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/FixedContentNegotiationStrategy.java +++ b/spring-web/src/main/java/org/springframework/web/accept/FixedContentNegotiationStrategy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2017 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. @@ -16,12 +16,14 @@ package org.springframework.web.accept; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.http.MediaType; +import org.springframework.util.Assert; import org.springframework.web.context.request.NativeWebRequest; /** @@ -38,22 +40,30 @@ public class FixedContentNegotiationStrategy implements ContentNegotiationStrate /** - * Create an instance with the given content type. + * Constructor with a single default {@code MediaType}. */ - public FixedContentNegotiationStrategy(MediaType... contentTypes) { - this.contentTypes = Arrays.asList(contentTypes); + public FixedContentNegotiationStrategy(MediaType contentType) { + this(Collections.singletonList(contentType)); } - + /** - * Create an instance with the given content type. - * - *

- * List is ordered in the same manner as a "quality" parameter on incoming requests. - * If destinations which do not support any of the media types provided are present, - * end the list with {@link MediaType#ALL} to allow standard media type determination + * Constructor with an ordered List of default {@code MediaType}'s to return + * for use in applications that support a variety of content types. + *

Consider appending {@link MediaType#ALL} at the end if destinations + * are present which do not support any of the other default media types. + * @since 5.0 */ public FixedContentNegotiationStrategy(List contentTypes) { - this.contentTypes = contentTypes; + Assert.notNull(contentTypes, "'contentTypes' must not be null"); + this.contentTypes = Collections.unmodifiableList(contentTypes); + } + + + /** + * Return the configured list of media types. + */ + public List getContentTypes() { + return this.contentTypes; } @@ -62,7 +72,6 @@ public class FixedContentNegotiationStrategy implements ContentNegotiationStrate if (logger.isDebugEnabled()) { logger.debug("Requested media types: " + this.contentTypes); } - return this.contentTypes; } diff --git a/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java b/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java index 0a86763274..10a83ffab8 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java @@ -19,6 +19,7 @@ package org.springframework.web.accept; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.Before; @@ -161,35 +162,31 @@ public class ContentNegotiationManagerFactoryBeanTests { assertEquals(Collections.emptyList(), manager.resolveMediaTypes(this.webRequest)); } - + @Test public void setDefaultContentType() throws Exception { - this.factoryBean.setDefaultContentType(Arrays.asList(MediaType.APPLICATION_JSON)); + this.factoryBean.setDefaultContentType(MediaType.APPLICATION_JSON); this.factoryBean.afterPropertiesSet(); ContentNegotiationManager manager = this.factoryBean.getObject(); - assertEquals(Collections.singletonList(MediaType.APPLICATION_JSON), - manager.resolveMediaTypes(this.webRequest)); + assertEquals(MediaType.APPLICATION_JSON, manager.resolveMediaTypes(this.webRequest).get(0)); // SPR-10513 this.servletRequest.addHeader("Accept", MediaType.ALL_VALUE); - assertEquals(Collections.singletonList(MediaType.APPLICATION_JSON), - manager.resolveMediaTypes(this.webRequest)); + assertEquals(MediaType.APPLICATION_JSON, manager.resolveMediaTypes(this.webRequest).get(0)); } - - @Test - public void setMultipleDefaultContentTypess() throws Exception { - this.factoryBean.setDefaultContentType(Arrays.asList(MediaType.APPLICATION_JSON, MediaType.ALL)); + + @Test // SPR-15367 + public void setDefaultContentTypes() throws Exception { + List mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.ALL); + this.factoryBean.setDefaultContentTypes(mediaTypes); this.factoryBean.afterPropertiesSet(); ContentNegotiationManager manager = this.factoryBean.getObject(); - assertEquals(Arrays.asList(MediaType.APPLICATION_JSON, MediaType.ALL), - manager.resolveMediaTypes(this.webRequest)); + assertEquals(mediaTypes, manager.resolveMediaTypes(this.webRequest)); - // SPR-15367 this.servletRequest.addHeader("Accept", MediaType.ALL_VALUE); - assertEquals(Arrays.asList(MediaType.APPLICATION_JSON, MediaType.ALL), - manager.resolveMediaTypes(this.webRequest)); + assertEquals(mediaTypes, manager.resolveMediaTypes(this.webRequest)); } @Test // SPR-12286 diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java index b0f5fed7df..8b87f07acc 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java @@ -219,19 +219,18 @@ public class ContentNegotiationConfigurer { } /** - * Set the default content type to use when no content type is requested. - *

- * Media types are ordered in the same manner as a "quality" parameter on incoming - * requests. If destinations which do not support any of the media types provided are - * present, end the list with {@link MediaType#ALL} to allow standard media type - * determination - *

- * By default this is not set. - * + * Set the default content type(s) to use when no content type is requested + * in order of priority. + * + *

If destinations are present that do not support any of the given media + * types, consider appending {@link MediaType#ALL} at the end. + * + *

By default this is not set. + * * @see #defaultContentTypeStrategy */ public ContentNegotiationConfigurer defaultContentType(MediaType... defaultContentTypes) { - this.factory.setDefaultContentType(Arrays.asList(defaultContentTypes)); + this.factory.setDefaultContentTypes(Arrays.asList(defaultContentTypes)); return this; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurerTests.java index 7a4d5e7b2c..7c42ceed11 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurerTests.java @@ -20,6 +20,7 @@ import java.util.Collections; import org.junit.Before; import org.junit.Test; + import org.springframework.http.MediaType; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.web.accept.ContentNegotiationManager; @@ -27,7 +28,7 @@ import org.springframework.web.accept.FixedContentNegotiationStrategy; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; /** * Test fixture for {@link ContentNegotiationConfigurer} tests. @@ -55,7 +56,7 @@ public class ContentNegotiationConfigurerTests { this.servletRequest.setRequestURI("/flower.gif"); assertEquals("Should be able to resolve file extensions by default", - Arrays.asList(MediaType.IMAGE_GIF), manager.resolveMediaTypes(this.webRequest)); + MediaType.IMAGE_GIF, manager.resolveMediaTypes(this.webRequest).get(0)); this.servletRequest.setRequestURI("/flower?format=gif"); this.servletRequest.addParameter("format", "gif"); @@ -67,7 +68,7 @@ public class ContentNegotiationConfigurerTests { this.servletRequest.addHeader("Accept", MediaType.IMAGE_GIF_VALUE); assertEquals("Should resolve Accept header by default", - Arrays.asList(MediaType.IMAGE_GIF), manager.resolveMediaTypes(this.webRequest)); + MediaType.IMAGE_GIF, manager.resolveMediaTypes(this.webRequest).get(0)); } @Test @@ -76,7 +77,7 @@ public class ContentNegotiationConfigurerTests { ContentNegotiationManager manager = this.configurer.getContentNegotiationManager(); this.servletRequest.setRequestURI("/flower.json"); - assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest)); + assertEquals(MediaType.APPLICATION_JSON, manager.resolveMediaTypes(this.webRequest).get(0)); } @Test @@ -89,7 +90,7 @@ public class ContentNegotiationConfigurerTests { this.servletRequest.setRequestURI("/flower"); this.servletRequest.addParameter("f", "json"); - assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest)); + assertEquals(MediaType.APPLICATION_JSON, manager.resolveMediaTypes(this.webRequest).get(0)); } @Test @@ -108,9 +109,9 @@ public class ContentNegotiationConfigurerTests { this.configurer.defaultContentType(MediaType.APPLICATION_JSON); ContentNegotiationManager manager = this.configurer.getContentNegotiationManager(); - assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest)); + assertEquals(MediaType.APPLICATION_JSON, manager.resolveMediaTypes(this.webRequest).get(0)); } - + @Test public void setMultipleDefaultContentTypes() throws Exception { this.configurer.defaultContentType(MediaType.APPLICATION_JSON, MediaType.ALL); @@ -124,6 +125,6 @@ public class ContentNegotiationConfigurerTests { this.configurer.defaultContentTypeStrategy(new FixedContentNegotiationStrategy(MediaType.APPLICATION_JSON)); ContentNegotiationManager manager = this.configurer.getContentNegotiationManager(); - assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest)); + assertEquals(MediaType.APPLICATION_JSON, manager.resolveMediaTypes(this.webRequest).get(0)); } }