Improve default content type selection

rstoyanchev · rstoyanchev · commit 4d005b69872e · 2013-05-10T12:51:08.000-04:00
Previously ContentNegotiationManager continued with the next
ContentNegotiationStrategy only if the current one returned an empty
list. Now it also does that if the current ContentNegotiationStrategy
returns "*/*". Since the absence of an Accept header and "*/*" have
the same meaning, this allows a default content type to be used in
either case.

Issue: SPR-10513
diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java
@@ -50,6 +50,8 @@
  */
 public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {
 
+	private static final List<MediaType> MEDIA_TYPE_ALL = Arrays.asList(MediaType.ALL);
+
 	private final List<ContentNegotiationStrategy> contentNegotiationStrategies =
 			new ArrayList<ContentNegotiationStrategy>();
 
@@ -119,9 +121,10 @@ public void addFileExtensionResolvers(MediaTypeFileExtensionResolver... resolver
 	public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException {
 		for (ContentNegotiationStrategy strategy : this.contentNegotiationStrategies) {
 			List<MediaType> mediaTypes = strategy.resolveMediaTypes(webRequest);
-			if (!mediaTypes.isEmpty()) {
-				return mediaTypes;
+			if (mediaTypes.isEmpty() || mediaTypes.equals(MEDIA_TYPE_ALL)) {
+				continue;
 			}
+			return mediaTypes;
 		}
 		return Collections.emptyList();
 	}
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
@@ -122,6 +122,12 @@ public void setDefaultContentType() throws Exception {
 		ContentNegotiationManager manager = this.factoryBean.getObject();
 
 		assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));
+
+		// SPR-10513
+
+		this.servletRequest.addHeader("Accept", MediaType.ALL_VALUE);
+
+		assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));
 	}
 
 }

Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,8 @@`
`50`	`50`	`*/`
`51`	`51`	`public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {`
`52`	`52`
	`53`	`+ private static final List<MediaType> MEDIA_TYPE_ALL = Arrays.asList(MediaType.ALL);`
	`54`	`+`
`53`	`55`	`private final List<ContentNegotiationStrategy> contentNegotiationStrategies =`
`54`	`56`	`new ArrayList<ContentNegotiationStrategy>();`
`55`	`57`
`@@ -119,9 +121,10 @@ public void addFileExtensionResolvers(MediaTypeFileExtensionResolver... resolver`
`119`	`121`	`public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException {`
`120`	`122`	`for (ContentNegotiationStrategy strategy : this.contentNegotiationStrategies) {`
`121`	`123`	`List<MediaType> mediaTypes = strategy.resolveMediaTypes(webRequest);`
`122`		`- if (!mediaTypes.isEmpty()) {`
`123`		`- return mediaTypes;`
	`124`	`+ if (mediaTypes.isEmpty() \|\| mediaTypes.equals(MEDIA_TYPE_ALL)) {`
	`125`	`+ continue;`
`124`	`126`	`}`
	`127`	`+ return mediaTypes;`
`125`	`128`	`}`
`126`	`129`	`return Collections.emptyList();`
`127`	`130`	`}`
Original file line number	Diff line number	Diff line change
`@@ -122,6 +122,12 @@ public void setDefaultContentType() throws Exception {`
`122`	`122`	`ContentNegotiationManager manager = this.factoryBean.getObject();`
`123`	`123`
`124`	`124`	`assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));`
	`125`	`+`
	`126`	`+ // SPR-10513`
	`127`	`+`
	`128`	`+ this.servletRequest.addHeader("Accept", MediaType.ALL_VALUE);`
	`129`	`+`
	`130`	`+ assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));`
`125`	`131`	`}`
`126`	`132`
`127`	`133`	`}`