SPR-6934 - AnnotationMethodHandlerAdapter should take into account request accept header ordering

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java

@@ -64,6 +64,7 @@ import org.springframework.http.converter.FormHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.StringHttpMessageConverter;
 import org.springframework.http.converter.xml.SourceHttpMessageConverter;
+import org.springframework.http.server.ServerHttpRequest;
 import org.springframework.http.server.ServletServerHttpRequest;
 import org.springframework.http.server.ServletServerHttpResponse;
 import org.springframework.ui.ExtendedModelMap;
@@ -556,7 +557,7 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 			if (!targetHandlerMethods.isEmpty()) {
 				List<RequestMappingInfo> matches = new ArrayList<RequestMappingInfo>(targetHandlerMethods.keySet());
 				RequestMappingInfoComparator requestMappingInfoComparator =
-						new RequestMappingInfoComparator(pathComparator);
+						new RequestMappingInfoComparator(pathComparator, request);
 				Collections.sort(matches, requestMappingInfoComparator);
 				RequestMappingInfo bestMappingMatch = matches.get(0);
 				String bestMatchedPath = bestMappingMatch.bestMatchedPath();
@@ -935,19 +936,27 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 
 	/**
 	 * Comparator capable of sorting {@link RequestMappingInfo}s (RHIs) so that sorting a list with this comparator will
-	 * result in: <ul> <li>RHIs with {@linkplain RequestMappingInfo#matchedPaths better matched paths} take prescedence
+	 * result in:
+	 * <ul>
+	 * <li>RHIs with {@linkplain RequestMappingInfo#matchedPaths better matched paths} take prescedence
 	 * over those with a weaker match (as expressed by the {@linkplain PathMatcher#getPatternComparator(String) path
 	 * pattern comparator}.) Typically, this means that patterns without wild cards and uri templates will be ordered
-	 * before those without.</li> <li>RHIs with one single {@linkplain RequestMappingInfo#methods request method} will be
+	 * before those without.</li>
-	 * ordered before those without a method, or with more than one method.</li> <li>RHIs with more {@linkplain
+	 * <li>RHIs with one single {@linkplain RequestMappingInfo#methods request method} will be
-	 * RequestMappingInfo#params request parameters} will be ordered before those with less parameters</li> </ol>
+	 * ordered before those without a method, or with more than one method.</li>
+	 * <li>RHIs with more {@linkplain RequestMappingInfo#params request parameters} will be ordered before those with
+	 * less parameters</li>
+	 * </ol>
 	 */
 	static class RequestMappingInfoComparator implements Comparator<RequestMappingInfo> {
 
 		private final Comparator<String> pathComparator;
 
-		RequestMappingInfoComparator(Comparator<String> pathComparator) {
+		private final ServerHttpRequest request;
+
+		RequestMappingInfoComparator(Comparator<String> pathComparator, HttpServletRequest request) {
 			this.pathComparator = pathComparator;
+			this.request = new ServletServerHttpRequest(request);
 		}
 
 		public int compare(RequestMappingInfo info1, RequestMappingInfo info2) {
@@ -965,6 +974,10 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 			if (info1HeaderCount != info2HeaderCount) {
 				return info2HeaderCount - info1HeaderCount;
 			}
+			int acceptComparison = compareAcceptHeaders(info1, info2);
+			if (acceptComparison != 0) {
+				return acceptComparison;
+			}
 			int info1MethodCount = info1.methods.length;
 			int info2MethodCount = info2.methods.length;
 			if (info1MethodCount == 0 && info2MethodCount > 0) {
@@ -981,6 +994,46 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 			}
 			return 0;
 		}
+
+		private int compareAcceptHeaders(RequestMappingInfo info1, RequestMappingInfo info2) {
+			List<MediaType> requestAccepts = request.getHeaders().getAccept();
+			List<MediaType> info1Accepts = getAcceptHeaderValue(info1);
+			List<MediaType> info2Accepts = getAcceptHeaderValue(info2);
+
+			for (MediaType requestAccept : requestAccepts) {
+				int pos1 = indexOfIncluded(info1Accepts, requestAccept);
+				int pos2 = indexOfIncluded(info2Accepts, requestAccept);
+				if (pos1 != pos2) {
+					return pos2 - pos1;
+				}
+			}
+			return 0;
 		}
 
+		private int indexOfIncluded(List<MediaType> infoAccepts, MediaType requestAccept) {
+			for (int i = 0; i < infoAccepts.size(); i++) {
+				MediaType info1Accept = infoAccepts.get(i);
+				if (requestAccept.includes(info1Accept)) {
+					return i;
+				}
+			}
+			return -1;
+		}
+
+		private List<MediaType> getAcceptHeaderValue(RequestMappingInfo info) {
+			for (String header : info.headers) {
+				int separator = header.indexOf('=');
+				if (separator != -1) {
+					String key = header.substring(0, separator);
+					String value = header.substring(separator + 1);
+					if ("Accept".equalsIgnoreCase(key)) {
+						return MediaType.parseMediaTypes(value);
+					}
+				}
+			}
+			return Collections.emptyList();
+		}
+
+
+	}
 }

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -25,6 +25,7 @@ import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Test;
 
+import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.web.bind.annotation.RequestMethod;
 
 /**
@@ -47,7 +48,7 @@ public class RequestMappingInfoComparatorTests {
 
 	@Before
 	public void setUp() throws NoSuchMethodException {
-		comparator = new AnnotationMethodHandlerAdapter.RequestMappingInfoComparator(new MockComparator());
+		comparator = new AnnotationMethodHandlerAdapter.RequestMappingInfoComparator(new MockComparator(), new MockHttpServletRequest());
 
 		emptyInfo = new AnnotationMethodHandlerAdapter.RequestMappingInfo();
 
@@ -85,6 +86,42 @@ public class RequestMappingInfoComparatorTests {
 		assertEquals(emptyInfo, infos.get(4));
 	}
 
+	@Test
+	public void acceptHeaders() {
+		AnnotationMethodHandlerAdapter.RequestMappingInfo html = new AnnotationMethodHandlerAdapter.RequestMappingInfo();
+		html.headers = new String[] {"accept=text/html"};
+
+		AnnotationMethodHandlerAdapter.RequestMappingInfo xml = new AnnotationMethodHandlerAdapter.RequestMappingInfo();
+		xml.headers = new String[] {"accept=application/xml"};
+
+		AnnotationMethodHandlerAdapter.RequestMappingInfo none = new AnnotationMethodHandlerAdapter.RequestMappingInfo();
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addHeader("Accept", "application/xml, text/html");
+		comparator = new AnnotationMethodHandlerAdapter.RequestMappingInfoComparator(new MockComparator(), request);
+
+		assertTrue(comparator.compare(html, xml) > 0);
+		assertTrue(comparator.compare(xml, html) < 0);
+		assertTrue(comparator.compare(xml, none) < 0);
+		assertTrue(comparator.compare(none, xml) > 0);
+		assertTrue(comparator.compare(html, none) < 0);
+		assertTrue(comparator.compare(none, html) > 0);
+
+		request = new MockHttpServletRequest();
+		request.addHeader("Accept", "application/xml, text/*");
+		comparator = new AnnotationMethodHandlerAdapter.RequestMappingInfoComparator(new MockComparator(), request);
+
+		assertTrue(comparator.compare(html, xml) > 0);
+		assertTrue(comparator.compare(xml, html) < 0);
+
+		request = new MockHttpServletRequest();
+		request.addHeader("Accept", "application/pdf");
+		comparator = new AnnotationMethodHandlerAdapter.RequestMappingInfoComparator(new MockComparator(), request);
+
+		assertTrue(comparator.compare(html, xml) == 0);
+		assertTrue(comparator.compare(xml, html) == 0);
+	}
+
 
 	private static class MockComparator implements Comparator<String> {
 

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java

@@ -1200,22 +1200,45 @@ public class ServletAnnotationControllerTests {
 
 
 	@Test
-	public void headers() throws ServletException, IOException {
+	public void contentTypeHeaders() throws ServletException, IOException {
-		initServlet(HeadersController.class);
+		initServlet(ContentTypeHeadersController.class);
 
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/something");
+		MockHttpServletRequest request = new MockHttpServletRequest("POST", "/something");
 		request.addHeader("Content-Type", "application/pdf");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		servlet.service(request, response);
 		assertEquals("pdf", response.getContentAsString());
 
-		request = new MockHttpServletRequest("GET", "/something");
+		request = new MockHttpServletRequest("POST", "/something");
 		request.addHeader("Content-Type", "text/html");
 		response = new MockHttpServletResponse();
 		servlet.service(request, response);
 		assertEquals("text", response.getContentAsString());
 	}
 	
+	@Test
+	public void acceptHeaders() throws ServletException, IOException {
+		initServlet(AcceptHeadersController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/something");
+		request.addHeader("Accept", "text/html");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals("html", response.getContentAsString());
+
+		request = new MockHttpServletRequest("GET", "/something");
+		request.addHeader("Accept", "application/xml");
+		response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals("xml", response.getContentAsString());
+
+		request = new MockHttpServletRequest("GET", "/something");
+		request.addHeader("Accept", "application/xml, text/html");
+		response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals("xml", response.getContentAsString());
+	}
+
 	@Test
 	public void responseStatus() throws ServletException, IOException {
 		initServlet(ResponseStatusController.class);
@@ -2235,7 +2258,7 @@ public class ServletAnnotationControllerTests {
 	}
 
 	@Controller
-	public static class HeadersController {
+	public static class ContentTypeHeadersController {
 
 		@RequestMapping(value = "/something", headers = "content-type=application/pdf")
 		public void handlePdf(Writer writer) throws IOException {
@@ -2248,6 +2271,20 @@ public class ServletAnnotationControllerTests {
 		}
 	}
 
+	@Controller
+	public static class AcceptHeadersController {
+
+		@RequestMapping(value = "/something", headers = "accept=text/html")
+		public void handleHtml(Writer writer) throws IOException {
+			writer.write("html");
+		}
+
+		@RequestMapping(value = "/something", headers = "accept=application/xml")
+		public void handleXml(Writer writer) throws IOException {
+			writer.write("xml");
+		}
+	}
+
 	@Controller
 	public static class ResponseStatusController {