ASTERIXDB-1804: support more HTTP verbs
- add verb-based routing to AbstractServlet.handle (subclasses implement
verb-specific methods like get, post, ...)
- move construction logic for IServletRequests from HttpUtil to request
classes
- remove verb-checking from HttpServerHandler
Change-Id: I2d14ce9c3c34d345fe71a44518b1e95b79c37dab
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1516
Reviewed-by: abdullah alamoudi <bamousaa@gmail.com>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractServlet.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractServlet.java
index 7d24994..2795549 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractServlet.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractServlet.java
@@ -19,11 +19,19 @@
package org.apache.hyracks.http.server;
import java.util.concurrent.ConcurrentMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.apache.hyracks.http.api.IServlet;
import org.apache.hyracks.http.api.IServletRequest;
+import org.apache.hyracks.http.api.IServletResponse;
+
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpResponseStatus;
public abstract class AbstractServlet implements IServlet {
+ private static final Logger LOGGER = Logger.getLogger(AbstractServlet.class.getName());
+
protected final String[] paths;
protected final ConcurrentMap<String, Object> ctx;
private final int[] trims;
@@ -54,6 +62,67 @@
return ctx;
}
+ @Override
+ public void handle(IServletRequest request, IServletResponse response) {
+ try {
+ final HttpMethod method = request.getHttpRequest().method();
+ if (HttpMethod.GET.equals(method)) {
+ get(request, response);
+ } else if (HttpMethod.HEAD.equals(method)) {
+ head(request, response);
+ } else if (HttpMethod.POST.equals(method)) {
+ post(request, response);
+ } else if (HttpMethod.PUT.equals(method)) {
+ put(request, response);
+ } else if (HttpMethod.DELETE.equals(method)) {
+ delete(request, response);
+ } else if (HttpMethod.OPTIONS.equals(method)) {
+ options(request, response);
+ } else {
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+ } catch (Exception e) {
+ LOGGER.log(Level.WARNING, "Unhandled exception", e);
+ response.setStatus(HttpResponseStatus.INTERNAL_SERVER_ERROR);
+ }
+ }
+
+ @SuppressWarnings("squid:S1172")
+ protected void get(IServletRequest request, IServletResponse response) throws Exception {
+ // designed to be extended but an error in standard case
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+
+ @SuppressWarnings("squid:S1172")
+ protected void head(IServletRequest request, IServletResponse response) throws Exception {
+ // designed to be extended but an error in standard case
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+
+ @SuppressWarnings("squid:S1172")
+ protected void post(IServletRequest request, IServletResponse response) throws Exception {
+ // designed to be extended but an error in standard case
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+
+ @SuppressWarnings("squid:S1172")
+ protected void put(IServletRequest request, IServletResponse response) throws Exception {
+ // designed to be extended but an error in standard case
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+
+ @SuppressWarnings("squid:S1172")
+ protected void delete(IServletRequest request, IServletResponse response) throws Exception {
+ // designed to be extended but an error in standard case
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+
+ @SuppressWarnings("squid:S1172")
+ protected void options(IServletRequest request, IServletResponse response) throws Exception {
+ // designed to be extended but an error in standard case
+ response.setStatus(HttpResponseStatus.METHOD_NOT_ALLOWED);
+ }
+
public String path(IServletRequest request) {
int trim = -1;
if (paths.length > 1) {
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/GetRequest.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
similarity index 74%
rename from hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/GetRequest.java
rename to hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
index e666c0a..d271177 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/GetRequest.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
@@ -18,19 +18,25 @@
*/
package org.apache.hyracks.http.server;
+import java.io.IOException;
import java.util.List;
import java.util.Map;
+import io.netty.handler.codec.http.QueryStringDecoder;
import org.apache.hyracks.http.api.IServletRequest;
import org.apache.hyracks.http.server.utils.HttpUtil;
import io.netty.handler.codec.http.FullHttpRequest;
-public class GetRequest implements IServletRequest {
- private final FullHttpRequest request;
- private final Map<String, List<String>> parameters;
+public class BaseRequest implements IServletRequest {
+ protected final FullHttpRequest request;
+ protected final Map<String, List<String>> parameters;
- public GetRequest(FullHttpRequest request, Map<String, List<String>> parameters) {
+ public static IServletRequest create(FullHttpRequest request) throws IOException {
+ return new BaseRequest(request, new QueryStringDecoder(request.uri()).parameters());
+ }
+
+ protected BaseRequest(FullHttpRequest request, Map<String, List<String>> parameters) {
this.request = request;
this.parameters = parameters;
}
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
index cb0cd80..1d219ba 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
@@ -26,6 +26,7 @@
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.http.DefaultFullHttpResponse;
import io.netty.handler.codec.http.DefaultHttpResponse;
@@ -115,6 +116,9 @@
fullResponse(response.protocolVersion(), response.status(),
error == null ? ctx.alloc().buffer(0, 0) : error, response.headers());
}
+
+ // since the request failed, we need to close the channel on complete
+ future.addListener(ChannelFutureListener.CLOSE);
}
done = true;
}
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
index e23ede4..5b917cd 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
@@ -66,10 +66,6 @@
DefaultHttpResponse notFound =
new DefaultHttpResponse(request.protocolVersion(), HttpResponseStatus.NOT_FOUND);
ctx.write(notFound).addListener(ChannelFutureListener.CLOSE);
- } else if (request.method() != HttpMethod.GET && request.method() != HttpMethod.POST) {
- DefaultHttpResponse notAllowed =
- new DefaultHttpResponse(request.protocolVersion(), HttpResponseStatus.METHOD_NOT_ALLOWED);
- ctx.write(notAllowed).addListener(ChannelFutureListener.CLOSE);
} else {
handler = new HttpRequestHandler(ctx, servlet, HttpUtil.toServletRequest(request), chunkSize);
server.getExecutor().submit(handler);
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/PostRequest.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/PostRequest.java
index 29cfd89..1dcb088 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/PostRequest.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/PostRequest.java
@@ -18,31 +18,63 @@
*/
package org.apache.hyracks.http.server;
+import java.io.IOException;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.apache.hyracks.http.api.IServletRequest;
import org.apache.hyracks.http.server.utils.HttpUtil;
import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.QueryStringDecoder;
+import io.netty.handler.codec.http.multipart.Attribute;
+import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder;
+import io.netty.handler.codec.http.multipart.InterfaceHttpData;
+import io.netty.handler.codec.http.multipart.MixedAttribute;
-public class PostRequest implements IServletRequest {
- private final FullHttpRequest request;
+public class PostRequest extends BaseRequest implements IServletRequest {
+
+ private static final Logger LOGGER = Logger.getLogger(PostRequest.class.getName());
+
private final List<String> names;
private final List<String> values;
- private final Map<String, List<String>> parameters;
- public PostRequest(FullHttpRequest request, Map<String, List<String>> parameters, List<String> names,
- List<String> values) {
- this.request = request;
- this.parameters = parameters;
- this.names = names;
- this.values = values;
+ public static IServletRequest create(FullHttpRequest request) throws IOException {
+ List<String> names = new ArrayList<>();
+ List<String> values = new ArrayList<>();
+ HttpPostRequestDecoder decoder = null;
+ try {
+ decoder = new HttpPostRequestDecoder(request);
+ } catch (Exception e) {
+ //ignore. this means that the body of the POST request does not have key value pairs
+ LOGGER.log(Level.WARNING, "Failed to decode a post message. Fix the API not to have queries as POST body",
+ e);
+ }
+ if (decoder != null) {
+ try {
+ List<InterfaceHttpData> bodyHttpDatas = decoder.getBodyHttpDatas();
+ for (InterfaceHttpData data : bodyHttpDatas) {
+ if (data.getHttpDataType().equals(InterfaceHttpData.HttpDataType.Attribute)) {
+ Attribute attr = (MixedAttribute) data;
+ names.add(data.getName());
+ values.add(attr.getValue());
+ }
+ }
+ } finally {
+ decoder.destroy();
+ }
+ }
+ return new PostRequest(request, new QueryStringDecoder(request.uri()).parameters(), names, values);
}
- @Override
- public FullHttpRequest getHttpRequest() {
- return request;
+ protected PostRequest(FullHttpRequest request, Map<String, List<String>> parameters, List<String> names,
+ List<String> values) {
+ super(request, parameters);
+ this.names = names;
+ this.values = values;
}
@Override
@@ -54,9 +86,4 @@
}
return HttpUtil.getParameter(parameters, name);
}
-
- @Override
- public String getHeader(CharSequence name) {
- return request.headers().get(name);
- }
}
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
index aa9ebc5..735ee8a 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
@@ -19,29 +19,19 @@
package org.apache.hyracks.http.server.utils;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.logging.Level;
-import java.util.logging.Logger;
import org.apache.hyracks.http.api.IServletRequest;
import org.apache.hyracks.http.api.IServletResponse;
-import org.apache.hyracks.http.server.GetRequest;
+import org.apache.hyracks.http.server.BaseRequest;
import org.apache.hyracks.http.server.PostRequest;
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpMethod;
-import io.netty.handler.codec.http.QueryStringDecoder;
-import io.netty.handler.codec.http.multipart.Attribute;
-import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder;
-import io.netty.handler.codec.http.multipart.InterfaceHttpData;
-import io.netty.handler.codec.http.multipart.InterfaceHttpData.HttpDataType;
-import io.netty.handler.codec.http.multipart.MixedAttribute;
public class HttpUtil {
- private static final Logger LOGGER = Logger.getLogger(HttpUtil.class.getName());
private HttpUtil() {
}
@@ -80,40 +70,8 @@
}
}
- public static IServletRequest post(FullHttpRequest request) throws IOException {
- List<String> names = new ArrayList<>();
- List<String> values = new ArrayList<>();
- HttpPostRequestDecoder decoder = null;
- try {
- decoder = new HttpPostRequestDecoder(request);
- } catch (Exception e) {
- //ignore. this means that the body of the POST request does not have key value pairs
- LOGGER.log(Level.WARNING, "Failed to decode a post message. Fix the API not to have queries as POST body",
- e);
- }
- if (decoder != null) {
- try {
- List<InterfaceHttpData> bodyHttpDatas = decoder.getBodyHttpDatas();
- for (InterfaceHttpData data : bodyHttpDatas) {
- if (data.getHttpDataType().equals(HttpDataType.Attribute)) {
- Attribute attr = (MixedAttribute) data;
- names.add(data.getName());
- values.add(attr.getValue());
- }
- }
- } finally {
- decoder.destroy();
- }
- }
- return new PostRequest(request, new QueryStringDecoder(request.uri()).parameters(), names, values);
- }
-
- public static IServletRequest get(FullHttpRequest request) throws IOException {
- return new GetRequest(request, new QueryStringDecoder(request.uri()).parameters());
- }
-
public static IServletRequest toServletRequest(FullHttpRequest request) throws IOException {
- return request.method() == HttpMethod.GET ? HttpUtil.get(request) : HttpUtil.post(request);
+ return request.method() == HttpMethod.POST ? PostRequest.create(request) : BaseRequest.create(request);
}
public static void setContentType(IServletResponse response, String type, String charset) throws IOException {