Repository: asterixdb Updated Branches: refs/heads/master 55dd6ff1e -> 6fc1f7b31
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]> Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/6fc1f7b3 Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/6fc1f7b3 Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/6fc1f7b3 Branch: refs/heads/master Commit: 6fc1f7b31ee6d2c86c5478e6a0d0cf4d2b711bf0 Parents: 55dd6ff Author: Abdullah Alamoudi <[email protected]> Authored: Tue Feb 21 12:41:26 2017 -0800 Committer: Till Westmann <[email protected]> Committed: Tue Feb 21 13:46:14 2017 -0800 ---------------------------------------------------------------------- .../hyracks/http/server/AbstractServlet.java | 69 ++++++++++++++++++++ .../apache/hyracks/http/server/BaseRequest.java | 58 ++++++++++++++++ .../hyracks/http/server/ChunkedResponse.java | 4 ++ .../apache/hyracks/http/server/GetRequest.java | 52 --------------- .../hyracks/http/server/HttpServerHandler.java | 4 -- .../apache/hyracks/http/server/PostRequest.java | 59 ++++++++++++----- .../hyracks/http/server/utils/HttpUtil.java | 46 +------------ 7 files changed, 176 insertions(+), 116 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractServlet.java ---------------------------------------------------------------------- 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 @@ public abstract class AbstractServlet implements IServlet { 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) { http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java new file mode 100644 index 0000000..d271177 --- /dev/null +++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +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 BaseRequest implements IServletRequest { + protected final FullHttpRequest request; + protected final 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; + } + + @Override + public FullHttpRequest getHttpRequest() { + return request; + } + + @Override + public String getParameter(CharSequence name) { + return HttpUtil.getParameter(parameters, name); + } + + @Override + public String getHeader(CharSequence name) { + return request.headers().get(name); + } +} http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java ---------------------------------------------------------------------- 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 org.apache.hyracks.http.api.IServletResponse; 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 @@ public class ChunkedResponse implements IServletResponse { 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; } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/GetRequest.java ---------------------------------------------------------------------- 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/GetRequest.java deleted file mode 100644 index e666c0a..0000000 --- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/GetRequest.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.hyracks.http.server; - -import java.util.List; -import java.util.Map; - -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 GetRequest(FullHttpRequest request, Map<String, List<String>> parameters) { - this.request = request; - this.parameters = parameters; - } - - @Override - public FullHttpRequest getHttpRequest() { - return request; - } - - @Override - public String getParameter(CharSequence name) { - return HttpUtil.getParameter(parameters, name); - } - - @Override - public String getHeader(CharSequence name) { - return request.headers().get(name); - } -} http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java ---------------------------------------------------------------------- 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 @@ public class HttpServerHandler extends SimpleChannelInboundHandler<Object> { 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); http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/PostRequest.java ---------------------------------------------------------------------- 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,34 +18,66 @@ */ 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 extends BaseRequest implements IServletRequest { + + private static final Logger LOGGER = Logger.getLogger(PostRequest.class.getName()); -public class PostRequest implements IServletRequest { - private final FullHttpRequest request; 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, + 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); + } + + protected PostRequest(FullHttpRequest request, Map<String, List<String>> parameters, List<String> names, List<String> values) { - this.request = request; - this.parameters = parameters; + super(request, parameters); this.names = names; this.values = values; } @Override - public FullHttpRequest getHttpRequest() { - return request; - } - - @Override public String getParameter(CharSequence name) { for (int i = 0; i < names.size(); i++) { if (name.equals(names.get(i))) { @@ -54,9 +86,4 @@ public class PostRequest implements IServletRequest { } return HttpUtil.getParameter(parameters, name); } - - @Override - public String getHeader(CharSequence name) { - return request.headers().get(name); - } } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/6fc1f7b3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java ---------------------------------------------------------------------- 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 class HttpUtil { } } - 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 {
