Revert "Remove some netty usage in jdisc_http_service"

3 files changed, 102 insertions, 15 deletions

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 50806cc655d..ff80a8a845d 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,8 +9,7 @@ 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.eclipse.jetty.http.HttpURI;
-import org.eclipse.jetty.util.MultiMap;
+import org.jboss.netty.handler.codec.http.QueryStringDecoder;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -85,7 +84,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
             this.method = method;
             this.version = version;
             this.remoteAddress = remoteAddress;
-            this.parameters.putAll(getUriQueryParameters(uri));
+            this.parameters.putAll(new QueryStringDecoder(uri.toString(), true).getParameters());
             if (connectedAtMillis != null) {
                 this.connectedAt = connectedAtMillis;
             } else {
@@ -103,7 +102,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
             this.method = method;
             this.version = version;
             this.remoteAddress = null;
-            this.parameters.putAll(getUriQueryParameters(uri));
+            this.parameters.putAll(new QueryStringDecoder(uri.toString(), true).getParameters());
             this.connectedAt = creationTime(TimeUnit.MILLISECONDS);
         } catch (RuntimeException e) {
             release();
@@ -111,12 +110,6 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
         }
     }
 
-    private static Map<String, List<String>> getUriQueryParameters(URI uri) {
-        MultiMap<String> 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 710820e7259..065276962f7 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,7 +3,10 @@ package com.yahoo.jdisc.http.core;
 
 import com.ning.http.client.RequestBuilder;
 import com.yahoo.jdisc.HeaderFields;
-import com.yahoo.jdisc.http.HttpHeaders;
+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 java.io.ByteArrayOutputStream;
 import java.nio.charset.StandardCharsets;
@@ -24,12 +27,24 @@ 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(), dst::addHeader);
+        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));
     }
 
     public static void copyTrailers(com.yahoo.jdisc.Request src, RequestBuilder dst) {
-        copyTrailers(src, dst::addHeader);
+        copyTrailers(src, newSimpleHeaders(dst));
     }
 
     public static void copyTrailers(com.yahoo.jdisc.Request src, ByteArrayOutputStream dst) {
@@ -47,6 +62,17 @@ 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<String, List<String>> entry : src.entrySet()) {
             String key = entry.getKey();
@@ -62,6 +88,16 @@ 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() {
 
@@ -79,7 +115,28 @@ public class HeaderFieldsUtil {
         };
     }
 
-    private interface SimpleHeaders {
-        void addHeader(String name, String value);
+    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);
     }
 }
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 cd106778129..021a14b2ae7 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,6 +4,10 @@ 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;
@@ -27,6 +31,27 @@ 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/");
         HttpRequest request = HttpRequest.newServerRequest(mockContainer(), uri);
@@ -196,6 +221,18 @@ 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(),