From 4f4f3fab7d23a571ff267bf4b11ed33aedea26dc Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 4 Mar 2010 13:54:24 +0000 Subject: [PATCH] SPR-6934 - AnnotationMethodHandlerAdapter should take into account request accept header ordering --- .../AnnotationMethodHandlerAdapter.java | 67 +++++++++++++++++-- .../RequestMappingInfoComparatorTests.java | 41 +++++++++++- .../ServletAnnotationControllerTests.java | 47 +++++++++++-- 3 files changed, 141 insertions(+), 14 deletions(-) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java index c6b19db4157..9619ee1e6cf 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java +++ b/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 matches = new ArrayList(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:
  • RHIs with {@linkplain RequestMappingInfo#matchedPaths better matched paths} take prescedence + * result in: + *
      + *
    • 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.
    • RHIs with one single {@linkplain RequestMappingInfo#methods request method} will be - * ordered before those without a method, or with more than one method.
    • RHIs with more {@linkplain - * RequestMappingInfo#params request parameters} will be ordered before those with less parameters
      • + * before those without. + *
    • RHIs with one single {@linkplain RequestMappingInfo#methods request method} will be + * ordered before those without a method, or with more than one method.
      • + *
    • RHIs with more {@linkplain RequestMappingInfo#params request parameters} will be ordered before those with + * less parameters
      • + * */ static class RequestMappingInfoComparator implements Comparator { private final Comparator pathComparator; - RequestMappingInfoComparator(Comparator pathComparator) { + private final ServerHttpRequest request; + + RequestMappingInfoComparator(Comparator 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 requestAccepts = request.getHeaders().getAccept(); + List info1Accepts = getAcceptHeaderValue(info1); + List 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 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 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(); + } + + + } } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java index ab55ae7a6bc..57fa7d82e6d 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java +++ b/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 { diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java index 6dad5c65567..adfb8dbf568 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java @@ -1200,21 +1200,44 @@ public class ServletAnnotationControllerTests { @Test - public void headers() throws ServletException, IOException { - initServlet(HeadersController.class); + public void contentTypeHeaders() throws ServletException, IOException { + 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 { @@ -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 {