From 30cd1687c29f6159d799e859ba8c6cb4d74c381e Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 21 Apr 2017 13:37:41 +0200 Subject: Remove some netty usage in jdisc_http_service --- .../java/com/yahoo/application/Application.java | 5 +- .../java/com/yahoo/jdisc/http/HttpRequest.java | 13 ++++- .../yahoo/jdisc/http/core/HeaderFieldsUtil.java | 67 ++-------------------- .../com/yahoo/jdisc/http/HttpRequestTestCase.java | 37 ------------ 4 files changed, 17 insertions(+), 105 deletions(-) diff --git a/application/src/main/java/com/yahoo/application/Application.java b/application/src/main/java/com/yahoo/application/Application.java index cfcce72487b..5b91c8ff89c 100644 --- a/application/src/main/java/com/yahoo/application/Application.java +++ b/application/src/main/java/com/yahoo/application/Application.java @@ -20,7 +20,6 @@ import com.yahoo.search.rendering.Renderer; import com.yahoo.text.StringUtilities; import com.yahoo.text.Utf8; import com.yahoo.vespa.model.VespaModel; -import org.jboss.netty.channel.ChannelException; import org.xml.sax.SAXException; import java.io.File; @@ -316,7 +315,7 @@ public final class Application implements AutoCloseable { break; } catch (Error e) { // the container thinks this is really serious, in this case is it not in the cause is a BindException // catch bind error and reset container - if (e.getCause() != null && e.getCause() instanceof ChannelException && e.getCause().getCause() != null && e.getCause().getCause() instanceof BindException) { + if (e.getCause() != null && e.getCause().getCause() != null && e.getCause().getCause() instanceof BindException) { exception = (Exception) e.getCause().getCause(); com.yahoo.container.Container.resetInstance(); // this is needed to be able to recreate the container from config again } else { @@ -667,4 +666,4 @@ public final class Application implements AutoCloseable { } } } -} \ No newline at end of file +} diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java index ff80a8a845d..50806cc655d 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java @@ -9,7 +9,8 @@ import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest; import com.yahoo.jdisc.service.CurrentContainer; -import org.jboss.netty.handler.codec.http.QueryStringDecoder; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.util.MultiMap; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -84,7 +85,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { this.method = method; this.version = version; this.remoteAddress = remoteAddress; - this.parameters.putAll(new QueryStringDecoder(uri.toString(), true).getParameters()); + this.parameters.putAll(getUriQueryParameters(uri)); if (connectedAtMillis != null) { this.connectedAt = connectedAtMillis; } else { @@ -102,7 +103,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { this.method = method; this.version = version; this.remoteAddress = null; - this.parameters.putAll(new QueryStringDecoder(uri.toString(), true).getParameters()); + this.parameters.putAll(getUriQueryParameters(uri)); this.connectedAt = creationTime(TimeUnit.MILLISECONDS); } catch (RuntimeException e) { release(); @@ -110,6 +111,12 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { } } + private static Map> getUriQueryParameters(URI uri) { + MultiMap queryParameters = new MultiMap<>(); + new HttpURI(uri).decodeQueryTo(queryParameters); + return queryParameters; + } + public Method getMethod() { return method; } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/core/HeaderFieldsUtil.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/core/HeaderFieldsUtil.java index 065276962f7..710820e7259 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/core/HeaderFieldsUtil.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/core/HeaderFieldsUtil.java @@ -3,10 +3,7 @@ package com.yahoo.jdisc.http.core; import com.ning.http.client.RequestBuilder; import com.yahoo.jdisc.HeaderFields; -import org.jboss.netty.handler.codec.http.HttpChunkTrailer; -import org.jboss.netty.handler.codec.http.HttpHeaders; -import org.jboss.netty.handler.codec.http.HttpMessage; -import org.jboss.netty.handler.codec.http.HttpResponse; +import com.yahoo.jdisc.http.HttpHeaders; import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; @@ -27,24 +24,12 @@ public class HeaderFieldsUtil { HttpHeaders.Names.CONTENT_LENGTH, HttpHeaders.Names.TRANSFER_ENCODING)); - public static void copyHeaders(com.yahoo.jdisc.Response src, HttpResponse dst) { - copyHeaderFields(src.headers(), newSimpleHeaders(dst)); - } - public static void copyHeaders(com.yahoo.jdisc.Request src, RequestBuilder dst) { - copyHeaderFields(src.headers(), newSimpleHeaders(dst)); - } - - public static void copyTrailers(com.yahoo.jdisc.Response src, HttpResponse dst) { - copyTrailers(src, newSimpleHeaders(dst)); - } - - public static void copyTrailers(com.yahoo.jdisc.Response src, HttpChunkTrailer dst) { - copyTrailers(src, newSimpleHeaders(dst)); + copyHeaderFields(src.headers(), dst::addHeader); } public static void copyTrailers(com.yahoo.jdisc.Request src, RequestBuilder dst) { - copyTrailers(src, newSimpleHeaders(dst)); + copyTrailers(src, dst::addHeader); } public static void copyTrailers(com.yahoo.jdisc.Request src, ByteArrayOutputStream dst) { @@ -62,17 +47,6 @@ public class HeaderFieldsUtil { } } - @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter") - public static void copyTrailers(com.yahoo.jdisc.Response src, SimpleHeaders dst) { - if (!(src instanceof com.yahoo.jdisc.http.HttpResponse)) { - return; - } - final HeaderFields trailers = ((com.yahoo.jdisc.http.HttpResponse)src).trailers(); - synchronized (trailers) { - copyHeaderFields(trailers, dst); - } - } - private static void copyHeaderFields(HeaderFields src, SimpleHeaders dst) { for (Map.Entry> entry : src.entrySet()) { String key = entry.getKey(); @@ -88,16 +62,6 @@ public class HeaderFieldsUtil { } } - private static SimpleHeaders newSimpleHeaders(final RequestBuilder dst) { - return new SimpleHeaders() { - - @Override - public void addHeader(String name, String value) { - dst.addHeader(name, value); - } - }; - } - private static SimpleHeaders newSimpleHeaders(final ByteArrayOutputStream dst) { return new SimpleHeaders() { @@ -115,28 +79,7 @@ public class HeaderFieldsUtil { }; } - private static SimpleHeaders newSimpleHeaders(final HttpMessage dst) { - return new SimpleHeaders() { - - @Override - public void addHeader(String name, String value) { - dst.addHeader(name, value); - } - }; - } - - private static SimpleHeaders newSimpleHeaders(final HttpChunkTrailer dst) { - return new SimpleHeaders() { - - @Override - public void addHeader(String name, String value) { - dst.addHeader(name, value); - } - }; - } - - private static interface SimpleHeaders { - - public void addHeader(String name, String value); + private interface SimpleHeaders { + void addHeader(String name, String value); } } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java index 021a14b2ae7..cd106778129 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java @@ -4,10 +4,6 @@ package com.yahoo.jdisc.http; import com.yahoo.jdisc.Container; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.service.CurrentContainer; -import com.yahoo.jdisc.test.TestDriver; -import org.jboss.netty.handler.codec.http.HttpHeaders; -import org.jboss.netty.handler.codec.http.HttpMethod; -import org.jboss.netty.handler.codec.http.HttpVersion; import org.testng.annotations.Test; import java.net.InetSocketAddress; @@ -30,27 +26,6 @@ import static org.testng.AssertJUnit.assertTrue; */ public class HttpRequestTestCase { - @Test - public void requireThatMethodIsCompatibleWithNetty() { - assertMethod(HttpRequest.Method.OPTIONS, HttpMethod.OPTIONS); - assertMethod(HttpRequest.Method.GET, HttpMethod.GET); - assertMethod(HttpRequest.Method.HEAD, HttpMethod.HEAD); - assertMethod(HttpRequest.Method.POST, HttpMethod.POST); - assertMethod(HttpRequest.Method.PUT, HttpMethod.PUT); - assertMethod(HttpRequest.Method.PATCH, HttpMethod.PATCH); - assertMethod(HttpRequest.Method.DELETE, HttpMethod.DELETE); - assertMethod(HttpRequest.Method.TRACE, HttpMethod.TRACE); - assertMethod(HttpRequest.Method.CONNECT, HttpMethod.CONNECT); - assertEquals(9, HttpRequest.Method.values().length); - } - - @Test - public void requireThatVersionIsCompatibleWithNetty() { - assertVersion(HttpRequest.Version.HTTP_1_0, HttpVersion.HTTP_1_0); - assertVersion(HttpRequest.Version.HTTP_1_1, HttpVersion.HTTP_1_1); - assertEquals(2, HttpRequest.Version.values().length); - } - @Test public void requireThatSimpleServerConstructorsUseReasonableDefaults() { final URI uri = URI.create("http://localhost/"); @@ -221,18 +196,6 @@ public class HttpRequestTestCase { assertEquals(cookies, request.decodeCookieHeader()); } - private static void assertMethod(final HttpRequest.Method discMethod, final HttpMethod nettyMethod) { - assertEquals(discMethod, HttpRequest.Method.valueOf(nettyMethod.getName())); - assertEquals(discMethod, HttpRequest.Method.valueOf(nettyMethod.toString())); - assertEquals(nettyMethod, HttpMethod.valueOf(discMethod.toString())); - } - - private static void assertVersion(final HttpRequest.Version discVersion, final HttpVersion nettyVersion) { - assertEquals(discVersion, HttpRequest.Version.fromString(nettyVersion.getText())); - assertEquals(discVersion, HttpRequest.Version.fromString(nettyVersion.toString())); - assertEquals(nettyVersion, HttpVersion.valueOf(discVersion.toString())); - } - private static HttpRequest newRequest(final HttpRequest.Version version) throws Exception { return HttpRequest.newServerRequest( mockContainer(), -- cgit v1.2.3