From db1092f119d4e861e540f5c9c6caf09d7d7a3b0f Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 21 Jun 2016 10:35:00 -0400 Subject: [PATCH] Polish RequestMappingInfoTests --- .../mvc/method/RequestMappingInfo.java | 2 +- .../mvc/method/RequestMappingInfoTests.java | 221 ++++++------------ 2 files changed, 73 insertions(+), 150 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java index 8e923cff16..445f55f5a8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java @@ -34,7 +34,7 @@ import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondit import org.springframework.web.util.UrlPathHelper; /** - * Encapsulates the following request mapping conditions: + * A {@link RequestCondition} that consists of the following other conditions: *
    *
  1. {@link PatternsRequestCondition} *
  2. {@link RequestMethodsRequestCondition} diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java index ea95c1d385..879237584a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java @@ -19,21 +19,22 @@ package org.springframework.web.servlet.mvc.method; import java.util.Collections; import java.util.Comparator; import java.util.List; +import javax.servlet.http.HttpServletRequest; import org.junit.Test; import org.springframework.http.HttpHeaders; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.servlet.mvc.condition.ConsumesRequestCondition; -import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition; -import org.springframework.web.servlet.mvc.condition.ParamsRequestCondition; -import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition; -import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition; -import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondition; -import static java.util.Arrays.*; -import static org.junit.Assert.*; +import static java.util.Arrays.asList; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.springframework.web.bind.annotation.RequestMethod.GET; +import static org.springframework.web.servlet.mvc.method.RequestMappingInfo.paths; /** * Test fixture for {@link RequestMappingInfo} tests. @@ -43,9 +44,10 @@ import static org.junit.Assert.*; */ public class RequestMappingInfoTests { + @Test public void createEmpty() { - RequestMappingInfo info = new RequestMappingInfo(null, null, null, null, null, null, null); + RequestMappingInfo info = paths().build(); assertEquals(0, info.getPatternsCondition().getPatterns().size()); assertEquals(0, info.getMethodsCondition().getMethods().size()); @@ -58,19 +60,15 @@ public class RequestMappingInfoTests { @Test public void matchPatternsCondition() { - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); + HttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); - RequestMappingInfo info = new RequestMappingInfo( - new PatternsRequestCondition("/foo*", "/bar"), null, null, null, null, null, null); - RequestMappingInfo expected = new RequestMappingInfo( - new PatternsRequestCondition("/foo*"), null, null, null, null, null, null); + RequestMappingInfo info = paths("/foo*", "/bar").build(); + RequestMappingInfo expected = paths("/foo*").build(); assertEquals(expected, info.getMatchingCondition(request)); - info = new RequestMappingInfo( - new PatternsRequestCondition("/**", "/foo*", "/foo"), null, null, null, null, null, null); - expected = new RequestMappingInfo( - new PatternsRequestCondition("/foo", "/foo*", "/**"), null, null, null, null, null, null); + info = paths("/**", "/foo*", "/foo").build(); + expected = paths("/foo", "/foo*", "/**").build(); assertEquals(expected, info.getMatchingCondition(request)); } @@ -80,17 +78,12 @@ public class RequestMappingInfoTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.setParameter("foo", "bar"); - RequestMappingInfo info = - new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, - new ParamsRequestCondition("foo=bar"), null, null, null, null); + RequestMappingInfo info = paths("/foo").params("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(request); assertNotNull(match); - info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, - new ParamsRequestCondition("foo!=bar"), null, null, null, null); + info = paths("/foo").params("foo!=bar").build(); match = info.getMatchingCondition(request); assertNull(match); @@ -101,17 +94,12 @@ public class RequestMappingInfoTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.addHeader("foo", "bar"); - RequestMappingInfo info = - new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, - new HeadersRequestCondition("foo=bar"), null, null, null); + RequestMappingInfo info = paths("/foo").headers("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(request); assertNotNull(match); - info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, - new HeadersRequestCondition("foo!=bar"), null, null, null); + info = paths("/foo").headers("foo!=bar").build(); match = info.getMatchingCondition(request); assertNull(match); @@ -122,17 +110,12 @@ public class RequestMappingInfoTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.setContentType("text/plain"); - RequestMappingInfo info = - new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, null, - new ConsumesRequestCondition("text/plain"), null, null); + RequestMappingInfo info = paths("/foo").consumes("text/plain").build(); RequestMappingInfo match = info.getMatchingCondition(request); assertNotNull(match); - info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, null, - new ConsumesRequestCondition("application/xml"), null, null); + info = paths("/foo").consumes("application/xml").build(); match = info.getMatchingCondition(request); assertNull(match); @@ -143,17 +126,12 @@ public class RequestMappingInfoTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.addHeader("Accept", "text/plain"); - RequestMappingInfo info = - new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, null, null, - new ProducesRequestCondition("text/plain"), null); + RequestMappingInfo info = paths("/foo").produces("text/plain").build(); RequestMappingInfo match = info.getMatchingCondition(request); assertNotNull(match); - info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, null, null, - new ProducesRequestCondition("application/xml"), null); + info = paths("/foo").produces("application/xml").build(); match = info.getMatchingCondition(request); assertNull(match); @@ -164,153 +142,102 @@ public class RequestMappingInfoTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.setParameter("foo", "bar"); - RequestMappingInfo info = - new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, null, null, null, null, - new ParamsRequestCondition("foo=bar")); + RequestMappingInfo info = paths("/foo").params("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(request); assertNotNull(match); - info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), null, - new ParamsRequestCondition("foo!=bar"), null, null, null, - new ParamsRequestCondition("foo!=bar")); + info = paths("/foo").params("foo!=bar").params("foo!=bar").build(); match = info.getMatchingCondition(request); assertNull(match); } @Test - public void compareTwoHttpMethodsOneParam() { - RequestMappingInfo none = new RequestMappingInfo(null, null, null, null, null, null, null); - RequestMappingInfo oneMethod = - new RequestMappingInfo(null, - new RequestMethodsRequestCondition(RequestMethod.GET), null, null, null, null, null); - RequestMappingInfo oneMethodOneParam = - new RequestMappingInfo(null, - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo"), null, null, null, null); + public void compareToWithImpicitVsExplicitHttpMethodDeclaration() { + RequestMappingInfo noMethods = paths().build(); + RequestMappingInfo oneMethod = paths().methods(GET).build(); + RequestMappingInfo oneMethodOneParam = paths().methods(GET).params("foo").build(); - Comparator comparator = new Comparator() { - @Override - public int compare(RequestMappingInfo info, RequestMappingInfo otherInfo) { - return info.compareTo(otherInfo, new MockHttpServletRequest()); - } - }; + Comparator comparator = + (info, otherInfo) -> info.compareTo(otherInfo, new MockHttpServletRequest()); - List list = asList(none, oneMethod, oneMethodOneParam); + List list = asList(noMethods, oneMethod, oneMethodOneParam); Collections.shuffle(list); Collections.sort(list, comparator); assertEquals(oneMethodOneParam, list.get(0)); assertEquals(oneMethod, list.get(1)); - assertEquals(none, list.get(2)); + assertEquals(noMethods, list.get(2)); } @Test public void equals() { - RequestMappingInfo info1 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + RequestMappingInfo info1 = paths("/foo").methods(GET) + .params("foo=bar", "customFoo=customBar").headers("foo=bar") + .consumes("text/plain").produces("text/plain") + .build(); - RequestMappingInfo info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + RequestMappingInfo info2 = paths("/foo").methods(GET) + .params("foo=bar", "customFoo=customBar").headers("foo=bar") + .consumes("text/plain").produces("text/plain") + .build(); assertEquals(info1, info2); assertEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo", "/NOOOOOO"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + info2 = paths("/foo", "/NOOOOOO").methods(GET) + .params("foo=bar", "customFoo=customBar").headers("foo=bar") + .consumes("text/plain").produces("text/plain") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET, RequestMethod.POST), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + info2 = paths("/foo").methods(GET, RequestMethod.POST) + .params("foo=bar", "customFoo=customBar").headers("foo=bar") + .consumes("text/plain").produces("text/plain") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("/NOOOOOO"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + info2 = paths("/foo").methods(GET) + .params("/NOOOOOO", "customFoo=customBar").headers("foo=bar") + .consumes("text/plain").produces("text/plain") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("/NOOOOOO"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + info2 = paths("/foo").methods(GET) + .params("foo=bar", "customFoo=customBar").headers("/NOOOOOO") + .consumes("text/plain").produces("text/plain") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/NOOOOOO"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=customBar")); + info2 = paths("/foo").methods(GET) + .params("foo=bar", "customFoo=customBar").headers("foo=bar") + .consumes("text/NOOOOOO").produces("text/plain") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/NOOOOOO"), - new ParamsRequestCondition("customFoo=customBar")); + info2 = paths("/foo").methods(GET) + .params("foo=bar", "customFoo=customBar").headers("foo=bar") + .consumes("text/plain").produces("text/NOOOOOO") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); - info2 = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), - new RequestMethodsRequestCondition(RequestMethod.GET), - new ParamsRequestCondition("foo=bar"), - new HeadersRequestCondition("foo=bar"), - new ConsumesRequestCondition("text/plain"), - new ProducesRequestCondition("text/plain"), - new ParamsRequestCondition("customFoo=NOOOOOO")); + info2 = paths("/foo").methods(GET) + .params("foo=bar", "customFoo=NOOOOOO").headers("foo=bar") + .consumes("text/plain").produces("text/plain") + .build(); assertFalse(info1.equals(info2)); assertNotEquals(info1.hashCode(), info2.hashCode()); @@ -322,15 +249,11 @@ public class RequestMappingInfoTests { request.addHeader(HttpHeaders.ORIGIN, "http://domain.com"); request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "POST"); - RequestMappingInfo info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), new RequestMethodsRequestCondition(RequestMethod.POST), null, - null, null, null, null); + RequestMappingInfo info = paths("/foo").methods(RequestMethod.POST).build(); RequestMappingInfo match = info.getMatchingCondition(request); assertNotNull(match); - info = new RequestMappingInfo( - new PatternsRequestCondition("/foo"), new RequestMethodsRequestCondition(RequestMethod.OPTIONS), null, - null, null, null, null); + info = paths("/foo").methods(RequestMethod.OPTIONS).build(); match = info.getMatchingCondition(request); assertNull("Pre-flight should match the ACCESS_CONTROL_REQUEST_METHOD", match); }