From 64ee5e579adcec97122fc0ee1414031da3be0c6e Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@vmware.com>
Date: Mon, 2 Apr 2012 15:19:38 -0400
Subject: [PATCH] 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
---
 .../mvc/condition/RequestConditionHolder.java | 26 ++++++++--------
 .../RequestConditionHolderTests.java          | 31 ++++++++++++-------
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolder.java
index 3de3a9abb54..b28fef5f175 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolder.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolder.java
@@ -25,12 +25,12 @@ import javax.servlet.http.HttpServletRequest;
 /**
  * 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.
- * 
- * <p>An implementation of {@code RequestCondition} itself, a 
- * {@code RequestConditionHolder} decorates the held request condition allowing 
- * it to be combined and compared with other custom request conditions while 
+ *
+ * <p>An implementation of {@code RequestCondition} itself, a
+ * {@code RequestConditionHolder} decorates the held request condition allowing
+ * it to be combined and compared with other custom request conditions while
  * ensuring type and null safety.
- * 
+ *
  * @author Rossen Stoyanchev
  * @since 3.1
  */
@@ -38,9 +38,9 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
 
 	@SuppressWarnings("rawtypes")
 	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}
 	 */
 	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.
 	 * 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);
 		}
 	}
-	
+
 	/**
-	 * 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
 	 * holder, return the same holder instance.
 	 */
@@ -108,11 +108,11 @@ public final class RequestConditionHolder extends AbstractRequestCondition<Reque
 			return this;
 		}
 		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.
 	 * 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);
 		}
 	}
-	
+
 }
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolderTests.java
index 6dc6e0d88d1..9236bd9ea15 100644
--- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolderTests.java
+++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestConditionHolderTests.java
@@ -17,6 +17,7 @@
 package org.springframework.web.servlet.mvc.condition;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 
 import javax.servlet.http.HttpServletRequest;
@@ -24,15 +25,11 @@ import javax.servlet.http.HttpServletRequest;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 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.
- * 
+ *
  * @author Rossen Stoyanchev
  */
 public class RequestConditionHolderTests {
@@ -41,7 +38,7 @@ public class RequestConditionHolderTests {
 	public void combineEmpty() {
 		RequestConditionHolder empty = new RequestConditionHolder(null);
 		RequestConditionHolder notEmpty = new RequestConditionHolder(new ParamsRequestCondition("name"));
-		
+
 		assertSame(empty, empty.combine(new RequestConditionHolder(null)));
 		assertSame(notEmpty, notEmpty.combine(empty));
 		assertSame(notEmpty, empty.combine(notEmpty));
@@ -52,7 +49,7 @@ public class RequestConditionHolderTests {
 		RequestConditionHolder params1 = new RequestConditionHolder(new ParamsRequestCondition("name1"));
 		RequestConditionHolder params2 = new RequestConditionHolder(new ParamsRequestCondition("name2"));
 		RequestConditionHolder expected = new RequestConditionHolder(new ParamsRequestCondition("name1", "name2"));
-		
+
 		assertEquals(expected, params1.combine(params2));
 	}
 
@@ -67,14 +64,24 @@ public class RequestConditionHolderTests {
 	public void match() {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
 		request.setParameter("name1", "value1");
-		
+
 		RequestMethodsRequestCondition rm = new RequestMethodsRequestCondition(RequestMethod.GET, RequestMethod.POST);
 		RequestConditionHolder custom = new RequestConditionHolder(rm);
 		RequestMethodsRequestCondition expected = new RequestMethodsRequestCondition(RequestMethod.GET);
-		
+
 		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
 	public void matchEmpty() {
 		RequestConditionHolder empty = new RequestConditionHolder(null);
@@ -91,7 +98,7 @@ public class RequestConditionHolderTests {
 		assertEquals(1, params11.compareTo(params12, request));
 		assertEquals(-1, params12.compareTo(params11, request));
 	}
-	
+
 	@Test
 	public void compareEmpty() {
 		HttpServletRequest request = new MockHttpServletRequest();