Till Westmann has submitted this change and it was merged. Change subject: ASTERIXDB-1804: support more HTTP verbs ......................................................................
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 <[email protected]> Tested-by: Jenkins <[email protected]> --- M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractServlet.java R hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/PostRequest.java M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java 6 files changed, 129 insertions(+), 69 deletions(-) Approvals: abdullah alamoudi: Looks good to me, approved Jenkins: Verified Objections: Jenkins: Violations found; Violations found 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 @@ -53,10 +85,5 @@ } } 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 { -- To view, visit https://asterix-gerrit.ics.uci.edu/1516 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2d14ce9c3c34d345fe71a44518b1e95b79c37dab Gerrit-PatchSet: 4 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Till Westmann <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]>
