This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/camel.git
commit 7331d83f073ab52dcc846efb7b5b8582e44b590b Author: Claus Ibsen <[email protected]> AuthorDate: Mon Aug 27 15:48:34 2018 +0200 CAMEL-12753: Fixed camel-servlet,camel-jetty consumer with OPTIONS in rest-dsl to return mutiple verbs if the same context-path has more http verbs like GET,POST etc. Thanks to Tomas Turek for unit test --- .../org/apache/camel/http/common/CamelServlet.java | 20 ++++++++++------ .../common/HttpServletResolveConsumerStrategy.java | 8 ++----- .../common/ServletResolveConsumerStrategy.java | 4 ++-- .../component/jetty/CamelContinuationServlet.java | 17 ++++++++----- .../component/jetty/rest/RestJettyOptionsTest.java | 28 ++++++++++++++++++---- .../servlet/rest/RestServletOptionsTest.java | 26 ++++++++++++++++---- 6 files changed, 72 insertions(+), 31 deletions(-) diff --git a/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java b/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java index 9df103f..166cb2e 100644 --- a/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java +++ b/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.stream.Collector; +import java.util.stream.Collectors; import javax.servlet.AsyncContext; import javax.servlet.ServletConfig; import javax.servlet.ServletException; @@ -141,14 +143,18 @@ public class CamelServlet extends HttpServlet { // if its an OPTIONS request then return which method is allowed if ("OPTIONS".equals(request.getMethod()) && !consumer.isOptionsEnabled()) { - String s; - if (consumer.getEndpoint().getHttpMethodRestrict() != null) { - s = "OPTIONS," + consumer.getEndpoint().getHttpMethodRestrict(); - } else { - // allow them all - s = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH"; + String allowedMethods = METHODS.stream().filter(m -> getServletResolveConsumerStrategy().isHttpMethodAllowed(request, m, getConsumers())).collect(Collectors.joining(",")); + if (allowedMethods == null && consumer.getEndpoint().getHttpMethodRestrict() != null) { + allowedMethods = consumer.getEndpoint().getHttpMethodRestrict(); + } + if (allowedMethods == null) { + // allow them all + allowedMethods = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH"; + } + if (!allowedMethods.contains("OPTIONS")) { + allowedMethods = allowedMethods + ",OPTIONS"; } - response.addHeader("Allow", s); + response.addHeader("Allow", allowedMethods); response.setStatus(HttpServletResponse.SC_OK); return; } diff --git a/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java b/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java index e08827a..9b369c3 100644 --- a/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java +++ b/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java @@ -48,14 +48,10 @@ public class HttpServletResolveConsumerStrategy implements ServletResolveConsume HttpConsumer answer = consumers.get(path); List<HttpConsumer> candidates = resolveCandidates(request, method, consumers); + // extra filter by restrict + candidates = candidates.stream().filter(c -> matchRestMethod(method, c.getEndpoint().getHttpMethodRestrict())).collect(Collectors.toList()); if (candidates.size() == 1) { answer = candidates.get(0); - } else { - // extra filter by restrict - candidates = candidates.stream().filter(c -> matchRestMethod(method, c.getEndpoint().getHttpMethodRestrict())).collect(Collectors.toList()); - if (candidates.size() == 1) { - answer = candidates.get(0); - } } return answer; diff --git a/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java b/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java index 039f74b..c1ea400 100644 --- a/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java +++ b/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java @@ -40,8 +40,8 @@ public interface ServletResolveConsumerStrategy { * @param request the http request * @param method the http method * @param consumers the map of registered consumers - * @return the consumer to service the request, or <tt>null</tt> if no match, - * which sends back a {@link javax.servlet.http.HttpServletResponse#SC_METHOD_NOT_ALLOWED} to the client. + * @return <tt>true</tt> if the method is allowed and can be serviced. Otherwise a + * {@link javax.servlet.http.HttpServletResponse#SC_METHOD_NOT_ALLOWED} is returned to the client. */ boolean isHttpMethodAllowed(HttpServletRequest request, String method, Map<String, HttpConsumer> consumers); diff --git a/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java b/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java index 5e4cdff..d0a16d5 100644 --- a/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java +++ b/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java @@ -21,6 +21,7 @@ import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -105,14 +106,18 @@ public class CamelContinuationServlet extends CamelServlet { // if its an OPTIONS request then return which method is allowed if ("OPTIONS".equals(request.getMethod()) && !consumer.isOptionsEnabled()) { - String s; - if (consumer.getEndpoint().getHttpMethodRestrict() != null) { - s = "OPTIONS," + consumer.getEndpoint().getHttpMethodRestrict(); - } else { + String allowedMethods = METHODS.stream().filter(m -> getServletResolveConsumerStrategy().isHttpMethodAllowed(request, m, getConsumers())).collect(Collectors.joining(",")); + if (allowedMethods == null && consumer.getEndpoint().getHttpMethodRestrict() != null) { + allowedMethods = consumer.getEndpoint().getHttpMethodRestrict(); + } + if (allowedMethods == null) { // allow them all - s = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH"; + allowedMethods = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH"; + } + if (!allowedMethods.contains("OPTIONS")) { + allowedMethods = allowedMethods + ",OPTIONS"; } - response.addHeader("Allow", s); + response.addHeader("Allow", allowedMethods); response.setStatus(HttpServletResponse.SC_OK); return; } diff --git a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java index 7f6536e..34577177 100644 --- a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java +++ b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java @@ -34,12 +34,26 @@ public class RestJettyOptionsTest extends BaseJettyTest { }); assertEquals(200, exchange.getOut().getHeader(Exchange.HTTP_RESPONSE_CODE)); - assertEquals("OPTIONS,GET", exchange.getOut().getHeader("ALLOW")); + assertEquals("GET,OPTIONS", exchange.getOut().getHeader("ALLOW")); assertEquals("", exchange.getOut().getBody(String.class)); - exchange = fluentTemplate.to("http://localhost:"; + getPort() + "/users/v1/123").withHeader(Exchange.HTTP_METHOD, "OPTIONS").send(); + exchange = fluentTemplate.to("http://localhost:"; + getPort() + "/users/v1/id/123").withHeader(Exchange.HTTP_METHOD, "OPTIONS").send(); assertEquals(200, exchange.getOut().getHeader(Exchange.HTTP_RESPONSE_CODE)); - assertEquals("OPTIONS,PUT", exchange.getOut().getHeader("ALLOW")); + assertEquals("PUT,OPTIONS", exchange.getOut().getHeader("ALLOW")); + assertEquals("", exchange.getOut().getBody(String.class)); + } + + @Test + public void testJettyServerMultipleOptions() throws Exception { + Exchange exchange = template.request("http://localhost:"; + getPort() + "/users/v2/options", new Processor() { + @Override + public void process(Exchange exchange) throws Exception { + exchange.getIn().setHeader(Exchange.HTTP_METHOD, "OPTIONS"); + } + }); + + assertEquals(200, exchange.getOut().getHeader(Exchange.HTTP_RESPONSE_CODE)); + assertEquals("GET,POST,OPTIONS", exchange.getOut().getHeader("ALLOW")); assertEquals("", exchange.getOut().getBody(String.class)); } @@ -55,8 +69,12 @@ public class RestJettyOptionsTest extends BaseJettyTest { rest("/users/") .get("v1/customers") .to("mock:customers") - .put("v1/{id}") - .to("mock:id"); + .put("v1/id/{id}") + .to("mock:id") + .get("v2/options") + .to("mock:options") + .post("v2/options") + .to("mock:options"); } }; } diff --git a/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java index f6df318..b5454f1 100644 --- a/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java +++ b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java @@ -42,14 +42,26 @@ public class RestServletOptionsTest extends ServletCamelRouterTestSupport { WebResponse response = client.getResponse(req); assertEquals(200, response.getResponseCode()); - assertEquals("OPTIONS,GET", response.getHeaderField("ALLOW")); + assertEquals("GET,OPTIONS", response.getHeaderField("ALLOW")); assertEquals("", response.getText()); - req = new OptionsMethodWebRequest(CONTEXT_URL + "/services/users/v1/123"); + req = new OptionsMethodWebRequest(CONTEXT_URL + "/services/users/v1/id/123"); response = client.getResponse(req); assertEquals(200, response.getResponseCode()); - assertEquals("OPTIONS,PUT", response.getHeaderField("ALLOW")); + assertEquals("PUT,OPTIONS", response.getHeaderField("ALLOW")); + assertEquals("", response.getText()); + } + + @Test + public void testMultipleServletOptions() throws Exception { + WebRequest req = new OptionsMethodWebRequest(CONTEXT_URL + "/services/users/v2/options"); + ServletUnitClient client = newClient(); + client.setExceptionsThrownOnErrorStatus(false); + WebResponse response = client.getResponse(req); + + assertEquals(200, response.getResponseCode()); + assertEquals("GET,POST,OPTIONS", response.getHeaderField("ALLOW")); assertEquals("", response.getText()); } @@ -65,8 +77,12 @@ public class RestServletOptionsTest extends ServletCamelRouterTestSupport { rest("/users/") .get("v1/customers") .to("mock:customers") - .put("v1/{id}") - .to("mock:id"); + .put("v1/id/{id}") + .to("mock:id") + .get("v2/options") + .to("mock:options") + .post("v2/options") + .to("mock:options"); } }; }
