diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java index 74e7d5c173..b08fbe1c9b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -122,19 +122,27 @@ public final class HeadersRequestCondition extends AbstractRequestCondition - *
  • 0 if the two conditions have the same number of header expressions - *
  • Less than 0 if "this" instance has more header expressions - *
  • Greater than 0 if the "other" instance has more header expressions - * + * Compare to another condition based on header expressions. A condition + * is considered to be a more specific match, if it has: + *
      + *
    1. A greater number of expressions. + *
    2. A greater number of non-negated expressions with a concrete value. + *
    *

    It is assumed that both instances have been obtained via * {@link #getMatchingCondition(ServerWebExchange)} and each instance * contains the matching header expression only or is otherwise empty. */ @Override public int compareTo(HeadersRequestCondition other, ServerWebExchange exchange) { - return other.expressions.size() - this.expressions.size(); + int result = other.expressions.size() - this.expressions.size(); + if (result != 0) { + return result; + } + return (int) (getValueMatchCount(other.expressions) - getValueMatchCount(this.expressions)); + } + + private long getValueMatchCount(Set expressions) { + return expressions.stream().filter(e -> e.getValue() != null && !e.isNegated()).count(); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java index e0edf4411d..1974748ad5 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -104,19 +104,27 @@ public final class ParamsRequestCondition extends AbstractRequestCondition - *

  • 0 if the two conditions have the same number of parameter expressions - *
  • Less than 0 if "this" instance has more parameter expressions - *
  • Greater than 0 if the "other" instance has more parameter expressions - * + * Compare to another condition based on parameter expressions. A condition + * is considered to be a more specific match, if it has: + *
      + *
    1. A greater number of expressions. + *
    2. A greater number of non-negated expressions with a concrete value. + *
    *

    It is assumed that both instances have been obtained via * {@link #getMatchingCondition(ServerWebExchange)} and each instance * contains the matching parameter expressions only or is otherwise empty. */ @Override public int compareTo(ParamsRequestCondition other, ServerWebExchange exchange) { - return (other.expressions.size() - this.expressions.size()); + int result = other.expressions.size() - this.expressions.size(); + if (result != 0) { + return result; + } + return (int) (getValueMatchCount(other.expressions) - getValueMatchCount(this.expressions)); + } + + private long getValueMatchCount(Set expressions) { + return expressions.stream().filter(e -> e.getValue() != null && !e.isNegated()).count(); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/HeadersRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/HeadersRequestConditionTests.java index 8d85ae6420..c283963301 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/HeadersRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/HeadersRequestConditionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2018 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. @@ -20,14 +20,11 @@ import java.util.Collection; import org.junit.Test; -import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; +import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*; /** * Unit tests for {@link HeadersRequestCondition}. @@ -46,75 +43,75 @@ public class HeadersRequestConditionTests { } @Test - public void headerPresent() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("Accept", "")); + public void headerPresent() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "")); HeadersRequestCondition condition = new HeadersRequestCondition("accept"); assertNotNull(condition.getMatchingCondition(exchange)); } @Test - public void headerPresentNoMatch() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("bar", "")); + public void headerPresentNoMatch() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("bar", "")); HeadersRequestCondition condition = new HeadersRequestCondition("foo"); assertNull(condition.getMatchingCondition(exchange)); } @Test - public void headerNotPresent() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); + public void headerNotPresent() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/")); HeadersRequestCondition condition = new HeadersRequestCondition("!accept"); assertNotNull(condition.getMatchingCondition(exchange)); } @Test - public void headerValueMatch() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("foo", "bar")); + public void headerValueMatch() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("foo", "bar")); HeadersRequestCondition condition = new HeadersRequestCondition("foo=bar"); assertNotNull(condition.getMatchingCondition(exchange)); } @Test - public void headerValueNoMatch() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("foo", "bazz")); + public void headerValueNoMatch() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("foo", "bazz")); HeadersRequestCondition condition = new HeadersRequestCondition("foo=bar"); assertNull(condition.getMatchingCondition(exchange)); } @Test - public void headerCaseSensitiveValueMatch() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("foo", "bar")); + public void headerCaseSensitiveValueMatch() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("foo", "bar")); HeadersRequestCondition condition = new HeadersRequestCondition("foo=Bar"); assertNull(condition.getMatchingCondition(exchange)); } @Test - public void headerValueMatchNegated() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("foo", "baz")); + public void headerValueMatchNegated() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("foo", "baz")); HeadersRequestCondition condition = new HeadersRequestCondition("foo!=bar"); assertNotNull(condition.getMatchingCondition(exchange)); } @Test - public void headerValueNoMatchNegated() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("foo", "bar")); + public void headerValueNoMatchNegated() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("foo", "bar")); HeadersRequestCondition condition = new HeadersRequestCondition("foo!=bar"); assertNull(condition.getMatchingCondition(exchange)); } @Test - public void compareTo() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); + public void compareTo() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/")); HeadersRequestCondition condition1 = new HeadersRequestCondition("foo", "bar", "baz"); - HeadersRequestCondition condition2 = new HeadersRequestCondition("foo", "bar"); + HeadersRequestCondition condition2 = new HeadersRequestCondition("foo=a", "bar"); int result = condition1.compareTo(condition2, exchange); assertTrue("Invalid comparison result: " + result, result < 0); @@ -123,6 +120,27 @@ public class HeadersRequestConditionTests { assertTrue("Invalid comparison result: " + result, result > 0); } + @Test // SPR-16674 + public void compareToWithMoreSpecificMatchByValue() { + ServerWebExchange exchange = MockServerWebExchange.from(get("/")); + + HeadersRequestCondition condition1 = new HeadersRequestCondition("foo=a"); + HeadersRequestCondition condition2 = new HeadersRequestCondition("foo"); + + int result = condition1.compareTo(condition2, exchange); + assertTrue("Invalid comparison result: " + result, result < 0); + } + + @Test + public void compareToWithNegatedMatch() { + ServerWebExchange exchange = MockServerWebExchange.from(get("/")); + + HeadersRequestCondition condition1 = new HeadersRequestCondition("foo!=a"); + HeadersRequestCondition condition2 = new HeadersRequestCondition("foo"); + + assertEquals("Negated match should not count as more specific", + 0, condition1.compareTo(condition2, exchange)); + } @Test public void combine() { @@ -135,8 +153,8 @@ public class HeadersRequestConditionTests { } @Test - public void getMatchingCondition() throws Exception { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").header("foo", "bar")); + public void getMatchingCondition() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("foo", "bar")); HeadersRequestCondition condition = new HeadersRequestCondition("foo"); HeadersRequestCondition result = condition.getMatchingCondition(exchange); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ParamsRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ParamsRequestConditionTests.java index ad083c3c27..c447cb91dd 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ParamsRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ParamsRequestConditionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -23,12 +23,8 @@ import org.junit.Test; import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.web.server.ServerWebExchange; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get; +import static org.junit.Assert.*; +import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*; /** * Unit tests for {@link ParamsRequestCondition}. @@ -95,6 +91,28 @@ public class ParamsRequestConditionTests { assertTrue("Invalid comparison result: " + result, result > 0); } + @Test // SPR-16674 + public void compareToWithMoreSpecificMatchByValue() { + ServerWebExchange exchange = MockServerWebExchange.from(get("/")); + + ParamsRequestCondition condition1 = new ParamsRequestCondition("response_type=code"); + ParamsRequestCondition condition2 = new ParamsRequestCondition("response_type"); + + int result = condition1.compareTo(condition2, exchange); + assertTrue("Invalid comparison result: " + result, result < 0); + } + + @Test + public void compareToWithNegatedMatch() { + ServerWebExchange exchange = MockServerWebExchange.from(get("/")); + + ParamsRequestCondition condition1 = new ParamsRequestCondition("response_type!=code"); + ParamsRequestCondition condition2 = new ParamsRequestCondition("response_type"); + + assertEquals("Negated match should not count as more specific", + 0, condition1.compareTo(condition2, exchange)); + } + @Test public void combine() { ParamsRequestCondition condition1 = new ParamsRequestCondition("foo=bar"); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java index 61d043eec9..a88df25e16 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -122,19 +122,27 @@ public final class HeadersRequestCondition extends AbstractRequestCondition - *

  • 0 if the two conditions have the same number of header expressions - *
  • Less than 0 if "this" instance has more header expressions - *
  • Greater than 0 if the "other" instance has more header expressions - * + * Compare to another condition based on header expressions. A condition + * is considered to be a more specific match, if it has: + *
      + *
    1. A greater number of expressions. + *
    2. A greater number of non-negated expressions with a concrete value. + *
    *

    It is assumed that both instances have been obtained via * {@link #getMatchingCondition(HttpServletRequest)} and each instance * contains the matching header expression only or is otherwise empty. */ @Override public int compareTo(HeadersRequestCondition other, HttpServletRequest request) { - return other.expressions.size() - this.expressions.size(); + int result = other.expressions.size() - this.expressions.size(); + if (result != 0) { + return result; + } + return (int) (getValueMatchCount(other.expressions) - getValueMatchCount(this.expressions)); + } + + private long getValueMatchCount(Set expressions) { + return expressions.stream().filter(e -> e.getValue() != null && !e.isNegated()).count(); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java index bf26850495..cf40dc80bb 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -107,19 +107,27 @@ public final class ParamsRequestCondition extends AbstractRequestCondition - *

  • 0 if the two conditions have the same number of parameter expressions - *
  • Less than 0 if "this" instance has more parameter expressions - *
  • Greater than 0 if the "other" instance has more parameter expressions - * + * Compare to another condition based on parameter expressions. A condition + * is considered to be a more specific match, if it has: + *
      + *
    1. A greater number of expressions. + *
    2. A greater number of non-negated expressions with a concrete value. + *
    *

    It is assumed that both instances have been obtained via * {@link #getMatchingCondition(HttpServletRequest)} and each instance * contains the matching parameter expressions only or is otherwise empty. */ @Override public int compareTo(ParamsRequestCondition other, HttpServletRequest request) { - return (other.expressions.size() - this.expressions.size()); + int result = other.expressions.size() - this.expressions.size(); + if (result != 0) { + return result; + } + return (int) (getValueMatchCount(other.expressions) - getValueMatchCount(this.expressions)); + } + + private long getValueMatchCount(Set expressions) { + return expressions.stream().filter(e -> e.getValue() != null && !e.isNegated()).count(); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/HeadersRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/HeadersRequestConditionTests.java index d7420db69a..9a69a62c9b 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/HeadersRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/HeadersRequestConditionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2018 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. @@ -121,7 +121,7 @@ public class HeadersRequestConditionTests { MockHttpServletRequest request = new MockHttpServletRequest(); HeadersRequestCondition condition1 = new HeadersRequestCondition("foo", "bar", "baz"); - HeadersRequestCondition condition2 = new HeadersRequestCondition("foo", "bar"); + HeadersRequestCondition condition2 = new HeadersRequestCondition("foo=a", "bar"); int result = condition1.compareTo(condition2, request); assertTrue("Invalid comparison result: " + result, result < 0); @@ -130,6 +130,30 @@ public class HeadersRequestConditionTests { assertTrue("Invalid comparison result: " + result, result > 0); } + @Test // SPR-16674 + public void compareToWithMoreSpecificMatchByValue() { + MockHttpServletRequest request = new MockHttpServletRequest(); + + HeadersRequestCondition condition1 = new HeadersRequestCondition("foo=a"); + HeadersRequestCondition condition2 = new HeadersRequestCondition("foo"); + + int result = condition1.compareTo(condition2, request); + assertTrue("Invalid comparison result: " + result, result < 0); + + result = condition2.compareTo(condition1, request); + assertTrue("Invalid comparison result: " + result, result > 0); + } + + @Test + public void compareToWithNegatedMatch() { + MockHttpServletRequest request = new MockHttpServletRequest(); + + HeadersRequestCondition condition1 = new HeadersRequestCondition("foo!=a"); + HeadersRequestCondition condition2 = new HeadersRequestCondition("foo"); + + assertEquals("Negated match should not count as more specific", + 0, condition1.compareTo(condition2, request)); + } @Test public void combine() { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ParamsRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ParamsRequestConditionTests.java index 561f5b40f7..742c4ec134 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ParamsRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ParamsRequestConditionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -97,7 +97,7 @@ public class ParamsRequestConditionTests { MockHttpServletRequest request = new MockHttpServletRequest(); ParamsRequestCondition condition1 = new ParamsRequestCondition("foo", "bar", "baz"); - ParamsRequestCondition condition2 = new ParamsRequestCondition("foo", "bar"); + ParamsRequestCondition condition2 = new ParamsRequestCondition("foo=a", "bar"); int result = condition1.compareTo(condition2, request); assertTrue("Invalid comparison result: " + result, result < 0); @@ -106,6 +106,28 @@ public class ParamsRequestConditionTests { assertTrue("Invalid comparison result: " + result, result > 0); } + @Test // SPR-16674 + public void compareToWithMoreSpecificMatchByValue() { + MockHttpServletRequest request = new MockHttpServletRequest(); + + ParamsRequestCondition condition1 = new ParamsRequestCondition("response_type=code"); + ParamsRequestCondition condition2 = new ParamsRequestCondition("response_type"); + + int result = condition1.compareTo(condition2, request); + assertTrue("Invalid comparison result: " + result, result < 0); + } + + @Test + public void compareToWithNegatedMatch() { + MockHttpServletRequest request = new MockHttpServletRequest(); + + ParamsRequestCondition condition1 = new ParamsRequestCondition("response_type!=code"); + ParamsRequestCondition condition2 = new ParamsRequestCondition("response_type"); + + assertEquals("Negated match should not count as more specific", + 0, condition1.compareTo(condition2, request)); + } + @Test public void combine() { ParamsRequestCondition condition1 = new ParamsRequestCondition("foo=bar");