You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2018/08/27 14:08:49 UTC
[camel] 02/03: 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
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 <cl...@gmail.com>
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");
}
};
}