Fix bug with custom RequestCondition

A custom RequestCondition which can be provided by overriding methods
in RequestMappingHandlerMapping worked only for conditions that match
and did not return null (as it should have) for conditions that don't
match.

Issues: SPR-9134
This commit is contained in:
Rossen Stoyanchev
2012-04-02 15:19:38 -04:00
parent 9833a4c385
commit 64ee5e579a
2 changed files with 32 additions and 25 deletions

View File

@@ -25,12 +25,12 @@ import javax.servlet.http.HttpServletRequest;
/** /**
* A holder for a {@link RequestCondition} useful when the type of the held * A holder for a {@link RequestCondition} useful when the type of the held
* request condition is not known ahead of time - e.g. custom condition. * request condition is not known ahead of time - e.g. custom condition.
* *
* <p>An implementation of {@code RequestCondition} itself, a * <p>An implementation of {@code RequestCondition} itself, a
* {@code RequestConditionHolder} decorates the held request condition allowing * {@code RequestConditionHolder} decorates the held request condition allowing
* it to be combined and compared with other custom request conditions while * it to be combined and compared with other custom request conditions while
* ensuring type and null safety. * ensuring type and null safety.
* *
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @since 3.1 * @since 3.1
*/ */
@@ -38,9 +38,9 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
@SuppressWarnings("rawtypes") @SuppressWarnings("rawtypes")
private final RequestCondition condition; private final RequestCondition condition;
/** /**
* Create a new holder to wrap the given request condition. * Create a new holder to wrap the given request condition.
* @param requestCondition the condition to hold, may be {@code null} * @param requestCondition the condition to hold, may be {@code null}
*/ */
public RequestConditionHolder(RequestCondition<?> requestCondition) { public RequestConditionHolder(RequestCondition<?> requestCondition) {
@@ -65,7 +65,7 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
} }
/** /**
* Combine the request conditions held by the two RequestConditionHolder * Combine the request conditions held by the two RequestConditionHolder
* instances after making sure the conditions are of the same type. * instances after making sure the conditions are of the same type.
* Or if one holder is empty, the other holder is returned. * Or if one holder is empty, the other holder is returned.
*/ */
@@ -97,9 +97,9 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
throw new ClassCastException("Incompatible request conditions: " + clazz + " and " + otherClazz); throw new ClassCastException("Incompatible request conditions: " + clazz + " and " + otherClazz);
} }
} }
/** /**
* Get the matching condition for the held request condition wrap it in a * Get the matching condition for the held request condition wrap it in a
* new RequestConditionHolder instance. Or otherwise if this is an empty * new RequestConditionHolder instance. Or otherwise if this is an empty
* holder, return the same holder instance. * holder, return the same holder instance.
*/ */
@@ -108,11 +108,11 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
return this; return this;
} }
RequestCondition<?> match = (RequestCondition<?>) condition.getMatchingCondition(request); RequestCondition<?> match = (RequestCondition<?>) condition.getMatchingCondition(request);
return new RequestConditionHolder(match); return (match != null) ? new RequestConditionHolder(match) : null;
} }
/** /**
* Compare the request conditions held by the two RequestConditionHolder * Compare the request conditions held by the two RequestConditionHolder
* instances after making sure the conditions are of the same type. * instances after making sure the conditions are of the same type.
* Or if one holder is empty, the other holder is preferred. * Or if one holder is empty, the other holder is preferred.
*/ */
@@ -132,5 +132,5 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
return condition.compareTo(other.condition, request); return condition.compareTo(other.condition, request);
} }
} }
} }

View File

@@ -17,6 +17,7 @@
package org.springframework.web.servlet.mvc.condition; package org.springframework.web.servlet.mvc.condition;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame; import static org.junit.Assert.assertSame;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
@@ -24,15 +25,11 @@ import javax.servlet.http.HttpServletRequest;
import org.junit.Test; import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition;
import org.springframework.web.servlet.mvc.condition.ParamsRequestCondition;
import org.springframework.web.servlet.mvc.condition.RequestConditionHolder;
import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondition;
/** /**
* A test fixture for * A test fixture for
* {code org.springframework.web.servlet.mvc.method.RequestConditionHolder} tests. * {code org.springframework.web.servlet.mvc.method.RequestConditionHolder} tests.
* *
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
*/ */
public class RequestConditionHolderTests { public class RequestConditionHolderTests {
@@ -41,7 +38,7 @@ public class RequestConditionHolderTests {
public void combineEmpty() { public void combineEmpty() {
RequestConditionHolder empty = new RequestConditionHolder(null); RequestConditionHolder empty = new RequestConditionHolder(null);
RequestConditionHolder notEmpty = new RequestConditionHolder(new ParamsRequestCondition("name")); RequestConditionHolder notEmpty = new RequestConditionHolder(new ParamsRequestCondition("name"));
assertSame(empty, empty.combine(new RequestConditionHolder(null))); assertSame(empty, empty.combine(new RequestConditionHolder(null)));
assertSame(notEmpty, notEmpty.combine(empty)); assertSame(notEmpty, notEmpty.combine(empty));
assertSame(notEmpty, empty.combine(notEmpty)); assertSame(notEmpty, empty.combine(notEmpty));
@@ -52,7 +49,7 @@ public class RequestConditionHolderTests {
RequestConditionHolder params1 = new RequestConditionHolder(new ParamsRequestCondition("name1")); RequestConditionHolder params1 = new RequestConditionHolder(new ParamsRequestCondition("name1"));
RequestConditionHolder params2 = new RequestConditionHolder(new ParamsRequestCondition("name2")); RequestConditionHolder params2 = new RequestConditionHolder(new ParamsRequestCondition("name2"));
RequestConditionHolder expected = new RequestConditionHolder(new ParamsRequestCondition("name1", "name2")); RequestConditionHolder expected = new RequestConditionHolder(new ParamsRequestCondition("name1", "name2"));
assertEquals(expected, params1.combine(params2)); assertEquals(expected, params1.combine(params2));
} }
@@ -67,14 +64,24 @@ public class RequestConditionHolderTests {
public void match() { public void match() {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
request.setParameter("name1", "value1"); request.setParameter("name1", "value1");
RequestMethodsRequestCondition rm = new RequestMethodsRequestCondition(RequestMethod.GET, RequestMethod.POST); RequestMethodsRequestCondition rm = new RequestMethodsRequestCondition(RequestMethod.GET, RequestMethod.POST);
RequestConditionHolder custom = new RequestConditionHolder(rm); RequestConditionHolder custom = new RequestConditionHolder(rm);
RequestMethodsRequestCondition expected = new RequestMethodsRequestCondition(RequestMethod.GET); RequestMethodsRequestCondition expected = new RequestMethodsRequestCondition(RequestMethod.GET);
assertEquals(expected, custom.getMatchingCondition(request).getCondition()); assertEquals(expected, custom.getMatchingCondition(request).getCondition());
} }
@Test
public void noMatch() {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
RequestMethodsRequestCondition rm = new RequestMethodsRequestCondition(RequestMethod.POST);
RequestConditionHolder custom = new RequestConditionHolder(rm);
assertNull(custom.getMatchingCondition(request));
}
@Test @Test
public void matchEmpty() { public void matchEmpty() {
RequestConditionHolder empty = new RequestConditionHolder(null); RequestConditionHolder empty = new RequestConditionHolder(null);
@@ -91,7 +98,7 @@ public class RequestConditionHolderTests {
assertEquals(1, params11.compareTo(params12, request)); assertEquals(1, params11.compareTo(params12, request));
assertEquals(-1, params12.compareTo(params11, request)); assertEquals(-1, params12.compareTo(params11, request));
} }
@Test @Test
public void compareEmpty() { public void compareEmpty() {
HttpServletRequest request = new MockHttpServletRequest(); HttpServletRequest request = new MockHttpServletRequest();