Merge pull request #330 from Bhashinee/master

Provide first class support for essential request and response properties
wso2 · Jun 7, 2019 · 319a59a · 319a59a
2 parents 6d1ab31 + fde8102
commit 319a59a
Show file tree

Hide file tree

Showing 41 changed files with 124 additions and 102 deletions.
diff --git a/....transport.http.netty/src/main/java/org/wso2/transport/http/netty/contract/Constants.java b/....transport.http.netty/src/main/java/org/wso2/transport/http/netty/contract/Constants.java
@@ -105,8 +105,6 @@ public final class Constants {
     public static final String PROTOCOL = "PROTOCOL";
     public static final String HTTP_SCHEME = "http";
     public static final String HTTPS_SCHEME = "https";
-    public static final String HTTP_VERSION = "HTTP_VERSION";
-    public static final String HTTP_METHOD = "HTTP_METHOD";
     public static final String JKS = "JKS";
     public static final String TLS_PROTOCOL = "TLS";
     public static final String REQUIRE = "require";
@@ -135,6 +133,8 @@ public final class Constants {
     public static final String HTTP_POST_METHOD = "POST";
     public static final String HTTP_HEAD_METHOD = "HEAD";
 
+    public static final int OK_200 = 200;
+
     //HTTP server connector creation parameters
     public static final String HTTP_HOST = "host";
     public static final String HTTP_PORT = "port";
@@ -148,7 +148,6 @@ public final class Constants {
     public static final String HTTP_TRUST_STORE_PASS = "trustStorePassword";
     public static final String TLS_STORE_TYPE = "tlsStoreType";
 
-    public static final String HTTP_STATUS_CODE = "HTTP_STATUS_CODE";
     public static final String RESOLVED_REQUESTED_URI = "RESOLVED_REQUESTED_URI";
 
     public static final String HTTP_REASON_PHRASE = "HTTP_REASON_PHRASE";
@@ -160,7 +159,7 @@ public final class Constants {
     public static final String DEFAULT_VERSION_HTTP_1_1 = "HTTP/1.1";
     public static final float HTTP_1_1 = 1.1f;
     public static final float HTTP_1_0 = 1.0f;
-    public static final float HTTP_2_0 = 2.0f;
+    public static final String HTTP_2_0 = "2.0";
     public static final String HTTP_VERSION_PREFIX = "HTTP/";
     public static final String HTTP_1_1_VERSION = "1.1";
     public static final String HTTP_2_0_VERSION = "2.0";
@@ -172,7 +171,6 @@ public final class Constants {
     public static final String REMOTE_ADDRESS = "REMOTE_ADDRESS";
     public static final String REMOTE_HOST = "REMOTE_HOST";
     public static final String REMOTE_PORT = "REMOTE_PORT";
-    public static final String REQUEST_URL = "REQUEST_URL";
     public static final String ORIGIN_HOST = "ORIGIN_HOST";
 
     public static final String CHANNEL_ID = "CHANNEL_ID";

diff --git a/.../src/main/java/org/wso2/transport/http/netty/contractimpl/DefaultHttpClientConnector.java b/.../src/main/java/org/wso2/transport/http/netty/contractimpl/DefaultHttpClientConnector.java
@@ -89,7 +89,7 @@ public DefaultHttpClientConnector(ConnectionManager connectionManager, SenderCon
         this.http2ConnectionManager = connectionManager.getHttp2ConnectionManager();
         this.senderConfiguration = senderConfiguration;
         initTargetChannelProperties(senderConfiguration);
-        if (Float.valueOf(senderConfiguration.getHttpVersion()) == Constants.HTTP_2_0) {
+        if (Constants.HTTP_2_0.equals(senderConfiguration.getHttpVersion())) {
             isHttp2 = true;
         }
         this.clientEventGroup = clientEventGroup;

diff --git a/...c/main/java/org/wso2/transport/http/netty/contractimpl/DefaultHttpWsConnectorFactory.java b/...c/main/java/org/wso2/transport/http/netty/contractimpl/DefaultHttpWsConnectorFactory.java
@@ -85,7 +85,7 @@ public ServerConnector createServerConnector(ServerBootstrapConfiguration server
             setSslContext(serverConnectorBootstrap, sslConfig, listenerConfig);
         }
         serverConnectorBootstrap.addIdleTimeout(listenerConfig.getSocketIdleTimeout());
-        if (Constants.HTTP_2_0 == Float.valueOf(listenerConfig.getVersion())) {
+        if (Constants.HTTP_2_0.equals(listenerConfig.getVersion())) {
             serverConnectorBootstrap.setHttp2Enabled(true);
         }
         serverConnectorBootstrap.addHttpTraceLogHandler(listenerConfig.isHttpTraceLogEnabled());
@@ -118,15 +118,15 @@ private void setSslContext(ServerConnectorBootstrap serverConnectorBootstrap, SS
             serverConnectorBootstrap.addOcspStapling(sslConfig.isOcspStaplingEnabled());
             serverConnectorBootstrap.addSslHandlerFactory(sslHandlerFactory);
             if (sslConfig.getKeyStore() != null) {
-                if (Constants.HTTP_2_0 == Float.valueOf(listenerConfig.getVersion())) {
+                if (Constants.HTTP_2_0.equals(listenerConfig.getVersion())) {
                     serverConnectorBootstrap
                             .addHttp2SslContext(sslHandlerFactory.createHttp2TLSContextForServer(sslConfig));
                 } else {
                     serverConnectorBootstrap
                             .addKeystoreSslContext(sslHandlerFactory.createSSLContextFromKeystores(true));
                 }
             } else {
-                if (Constants.HTTP_2_0 == Float.valueOf(listenerConfig.getVersion())) {
+                if (Constants.HTTP_2_0.equals(listenerConfig.getVersion())) {
                     serverConnectorBootstrap
                             .addHttp2SslContext(sslHandlerFactory.createHttp2TLSContextForServer(sslConfig));
                 } else {

diff --git a/...port.http.netty/src/main/java/org/wso2/transport/http/netty/contractimpl/common/Util.java b/...port.http.netty/src/main/java/org/wso2/transport/http/netty/contractimpl/common/Util.java
@@ -85,6 +85,7 @@
 import static org.wso2.transport.http.netty.contract.Constants.HTTP_SCHEME;
 import static org.wso2.transport.http.netty.contract.Constants.IS_PROXY_ENABLED;
 import static org.wso2.transport.http.netty.contract.Constants.MUTUAL_SSL_HANDSHAKE_RESULT;
+import static org.wso2.transport.http.netty.contract.Constants.OK_200;
 import static org.wso2.transport.http.netty.contract.Constants.PROTOCOL;
 import static org.wso2.transport.http.netty.contract.Constants.REMOTE_CLIENT_CLOSED_WHILE_WRITING_OUTBOUND_RESPONSE_HEADERS;
 import static org.wso2.transport.http.netty.contract.Constants.TO;
@@ -110,12 +111,11 @@ private static String getStringValue(HttpCarbonMessage msg, String key, String d
         return value;
     }
 
-    private static int getIntValue(HttpCarbonMessage msg, String key, int defaultValue) {
-        Integer value = (Integer) msg.getProperty(key);
+    private static int getIntValue(HttpCarbonMessage msg) {
+        Integer value = msg.getHttpStatusCode();
         if (value == null) {
-            return defaultValue;
+            return OK_200;
         }
-
         return value;
     }
 
@@ -171,7 +171,7 @@ private static void setOutboundRespHeaders(HttpCarbonMessage outboundResponseMsg
     }
 
     public static HttpResponseStatus getHttpResponseStatus(HttpCarbonMessage msg) {
-        int statusCode = Util.getIntValue(msg, Constants.HTTP_STATUS_CODE, 200);
+        int statusCode = Util.getIntValue(msg);
         String reasonPhrase = Util.getStringValue(msg, Constants.HTTP_REASON_PHRASE,
                 HttpResponseStatus.valueOf(statusCode).reasonPhrase());
         return new HttpResponseStatus(statusCode, reasonPhrase);
@@ -209,9 +209,9 @@ private static String getRequestPath(HttpCarbonMessage outboundRequestMsg) {
 
     private static HttpVersion getHttpVersion(HttpCarbonMessage outboundRequestMsg) {
         HttpVersion httpVersion;
-        if (null != outboundRequestMsg.getProperty(Constants.HTTP_VERSION)) {
+        if (null != outboundRequestMsg.getHttpVersion()) {
             httpVersion = new HttpVersion(Constants.HTTP_VERSION_PREFIX
-                    + outboundRequestMsg.getProperty(Constants.HTTP_VERSION), true);
+                    + outboundRequestMsg.getHttpVersion(), true);
         } else {
             httpVersion = new HttpVersion(Constants.DEFAULT_VERSION_HTTP_1_1, true);
         }
@@ -220,8 +220,8 @@ private static HttpVersion getHttpVersion(HttpCarbonMessage outboundRequestMsg)
 
     private static HttpMethod getHttpMethod(HttpCarbonMessage outboundRequestMsg) {
         HttpMethod httpMethod;
-        if (null != outboundRequestMsg.getProperty(Constants.HTTP_METHOD)) {
-            httpMethod = new HttpMethod((String) outboundRequestMsg.getProperty(Constants.HTTP_METHOD));
+        if (null != outboundRequestMsg.getHttpMethod()) {
+            httpMethod = new HttpMethod(outboundRequestMsg.getHttpMethod());
         } else {
             httpMethod = new HttpMethod(Constants.HTTP_POST_METHOD);
         }
@@ -711,9 +711,8 @@ public static HttpCarbonMessage createInboundReqCarbonMsg(HttpRequest httpReques
         inboundRequestMsg.setProperty(Constants.CHNL_HNDLR_CTX, ctx);
         inboundRequestMsg.setProperty(Constants.SRC_HANDLER, sourceHandler);
         HttpVersion protocolVersion = httpRequestHeaders.protocolVersion();
-        inboundRequestMsg.setProperty(Constants.HTTP_VERSION,
-                protocolVersion.majorVersion() + "." + protocolVersion.minorVersion());
-        inboundRequestMsg.setProperty(Constants.HTTP_METHOD, httpRequestHeaders.method().name());
+        inboundRequestMsg.setHttpVersion(protocolVersion.majorVersion() + "." + protocolVersion.minorVersion());
+        inboundRequestMsg.setHttpMethod(httpRequestHeaders.method().name());
         InetSocketAddress localAddress = null;
 
         //This check was added because in case of netty embedded channel, this could be of type 'EmbeddedSocketAddress'.
@@ -732,7 +731,7 @@ public static HttpCarbonMessage createInboundReqCarbonMsg(HttpRequest httpReques
 
         inboundRequestMsg.setProperty(Constants.LOCAL_ADDRESS, ctx.channel().localAddress());
         inboundRequestMsg.setProperty(Constants.REMOTE_ADDRESS, sourceHandler.getRemoteAddress());
-        inboundRequestMsg.setProperty(Constants.REQUEST_URL, httpRequestHeaders.uri());
+        inboundRequestMsg.setRequestUrl(httpRequestHeaders.uri());
         inboundRequestMsg.setProperty(Constants.TO, httpRequestHeaders.uri());
         inboundRequestMsg.setProperty(MUTUAL_SSL_HANDSHAKE_RESULT,
                 ctx.channel().attr(Constants.MUTUAL_SSL_RESULT_ATTRIBUTE).get());
@@ -755,7 +754,7 @@ public static HttpCarbonMessage createInboundRespCarbonMsg(ChannelHandlerContext
                 new PooledDataStreamerFactory(ctx.alloc()));
 
         inboundResponseMsg.setProperty(Constants.DIRECTION, Constants.DIRECTION_RESPONSE);
-        inboundResponseMsg.setProperty(Constants.HTTP_STATUS_CODE, httpResponseHeaders.status().code());
+        inboundResponseMsg.setHttpStatusCode(httpResponseHeaders.status().code());
 
         //copy required properties for service chaining from incoming carbon message to the response carbon message
         //copy shared worker pool
@@ -776,7 +775,7 @@ public static boolean isKeepAlive(KeepAliveConfig keepAliveConfig, HttpCarbonMes
             throws ConfigurationException {
         switch (keepAliveConfig) {
         case AUTO:
-            return Float.valueOf((String) outboundRequestMsg.getProperty(Constants.HTTP_VERSION)) > Constants.HTTP_1_0;
+            return Float.valueOf(outboundRequestMsg.getHttpVersion()) > Constants.HTTP_1_0;
         case ALWAYS:
             return true;
         case NEVER:

diff --git a/...rc/main/java/org/wso2/transport/http/netty/contractimpl/common/states/Http2StateUtil.java b/...rc/main/java/org/wso2/transport/http/netty/contractimpl/common/states/Http2StateUtil.java
@@ -54,17 +54,14 @@
 import java.net.InetSocketAddress;
 
 import static org.wso2.transport.http.netty.contract.Constants.CHNL_HNDLR_CTX;
-import static org.wso2.transport.http.netty.contract.Constants.HTTP_METHOD;
 import static org.wso2.transport.http.netty.contract.Constants.HTTP_SCHEME;
-import static org.wso2.transport.http.netty.contract.Constants.HTTP_VERSION;
 import static org.wso2.transport.http.netty.contract.Constants.INBOUND_REQUEST;
 import static org.wso2.transport.http.netty.contract.Constants.LISTENER_INTERFACE_ID;
 import static org.wso2.transport.http.netty.contract.Constants.LISTENER_PORT;
 import static org.wso2.transport.http.netty.contract.Constants.LOCAL_ADDRESS;
 import static org.wso2.transport.http.netty.contract.Constants.POOLED_BYTE_BUFFER_FACTORY;
 import static org.wso2.transport.http.netty.contract.Constants.PROMISED_STREAM_REJECTED_ERROR;
 import static org.wso2.transport.http.netty.contract.Constants.PROTOCOL;
-import static org.wso2.transport.http.netty.contract.Constants.REQUEST_URL;
 import static org.wso2.transport.http.netty.contract.Constants.TO;
 
 /**
@@ -118,8 +115,8 @@ public static HttpCarbonRequest setupCarbonRequest(HttpRequest httpRequest, Http
         sourceReqCMsg.setProperty(CHNL_HNDLR_CTX, ctx);
         sourceReqCMsg.setProperty(Constants.SRC_HANDLER, http2SourceHandler);
         HttpVersion protocolVersion = httpRequest.protocolVersion();
-        sourceReqCMsg.setProperty(HTTP_VERSION, protocolVersion.majorVersion() + "." + protocolVersion.minorVersion());
-        sourceReqCMsg.setProperty(HTTP_METHOD, httpRequest.method().name());
+        sourceReqCMsg.setHttpVersion(protocolVersion.majorVersion() + "." + protocolVersion.minorVersion());
+        sourceReqCMsg.setHttpMethod(httpRequest.method().name());
 
         InetSocketAddress localAddress = null;
         //This check was added because in case of netty embedded channel, this could be of type 'EmbeddedSocketAddress'.
@@ -131,7 +128,7 @@ public static HttpCarbonRequest setupCarbonRequest(HttpRequest httpRequest, Http
         sourceReqCMsg.setProperty(LISTENER_INTERFACE_ID, http2SourceHandler.getInterfaceId());
         sourceReqCMsg.setProperty(PROTOCOL, HTTP_SCHEME);
         String uri = httpRequest.uri();
-        sourceReqCMsg.setProperty(REQUEST_URL, uri);
+        sourceReqCMsg.setRequestUrl(uri);
         sourceReqCMsg.setProperty(TO, uri);
         return sourceReqCMsg;
     }
@@ -312,23 +309,23 @@ public static int initiateStream(ChannelHandlerContext ctx, Http2Connection conn
     }
 
     /**
-     * Returns the stream id of next stream.
+     * Returns the stream id of next stream. This method should only be called from an io thread.
      *
      * @param conn the HTTP2 connection
      * @return the next stream id
      */
-    private static synchronized int getNextStreamId(Http2Connection conn) {
+    private static int getNextStreamId(Http2Connection conn) {
         return conn.local().incrementAndGetNextStreamId();
     }
 
     /**
-     * Creates a stream with given stream id.
+     * Creates a stream with given stream id. This method should only be called from an io thread.
      *
      * @param conn     the HTTP2 connection
      * @param streamId the id of the stream
      * @throws Http2Exception if a protocol-related error occurred
      */
-    private static synchronized void createStream(Http2Connection conn, int streamId) throws Http2Exception {
+    private static void createStream(Http2Connection conn, int streamId) throws Http2Exception {
         conn.local().createStream(streamId, false);
         if (LOG.isDebugEnabled()) {
             LOG.debug("Stream created streamId: {}", streamId);

diff --git a/...tty/src/main/java/org/wso2/transport/http/netty/contractimpl/common/states/StateUtil.java b/...tty/src/main/java/org/wso2/transport/http/netty/contractimpl/common/states/StateUtil.java
@@ -95,11 +95,11 @@ public static void writeRequestHeaders(HttpCarbonMessage httpOutboundRequest,
     }
 
     private static void setHttpVersionProperty(HttpCarbonMessage httpOutboundRequest, String httpVersion) {
-        if (Float.valueOf(httpVersion) == Constants.HTTP_2_0) {
+        if (Constants.HTTP_2_0.equals(httpVersion)) {
             // Upgrade request of HTTP/2 should be a HTTP/1.1 request
-            httpOutboundRequest.setProperty(Constants.HTTP_VERSION, String.valueOf(Constants.HTTP_1_1));
+            httpOutboundRequest.setHttpVersion(String.valueOf(Constants.HTTP_1_1));
         } else {
-            httpOutboundRequest.setProperty(Constants.HTTP_VERSION, httpVersion);
+            httpOutboundRequest.setHttpVersion(httpVersion);
         }
     }
 

diff --git a/.../src/main/java/org/wso2/transport/http/netty/contractimpl/listener/RequestDataHolder.java b/.../src/main/java/org/wso2/transport/http/netty/contractimpl/listener/RequestDataHolder.java
@@ -19,7 +19,6 @@
 package org.wso2.transport.http.netty.contractimpl.listener;
 
 import io.netty.handler.codec.http.HttpHeaderNames;
-import org.wso2.transport.http.netty.contract.Constants;
 import org.wso2.transport.http.netty.message.HttpCarbonMessage;
 
 /**
@@ -42,8 +41,8 @@ public RequestDataHolder(HttpCarbonMessage requestMessage) {
         contentTypeHeaderValue = requestMessage.getHeader(HttpHeaderNames.CONTENT_TYPE.toString());
         transferEncodingHeaderValue = requestMessage.getHeader(HttpHeaderNames.TRANSFER_ENCODING.toString());
         contentLengthHeaderValue = requestMessage.getHeader(HttpHeaderNames.CONTENT_LENGTH.toString());
-        httpMethod = (String) requestMessage.getProperty(Constants.HTTP_METHOD);
-        httpVersion = (String) requestMessage.getProperty(Constants.HTTP_VERSION);
+        httpMethod = requestMessage.getHttpMethod();
+        httpVersion = requestMessage.getHttpVersion();
     }
 
     /**

diff --git a/...etty/src/main/java/org/wso2/transport/http/netty/contractimpl/listener/SourceHandler.java b/...etty/src/main/java/org/wso2/transport/http/netty/contractimpl/listener/SourceHandler.java
@@ -249,7 +249,7 @@ private void notifyErrorListenerAtConnectedState(String errorMsg) {
     private void setRequestProperties() {
         inboundRequestMsg.setPipeliningEnabled(pipeliningEnabled); //Value of listener config
         String connectionHeaderValue = inboundRequestMsg.getHeader(HttpHeaderNames.CONNECTION.toString());
-        String httpVersion = (String) inboundRequestMsg.getProperty(Constants.HTTP_VERSION);
+        String httpVersion = inboundRequestMsg.getHttpVersion();
         inboundRequestMsg.setKeepAlive(isKeepAliveConnection(keepAliveConfig, connectionHeaderValue,
                 httpVersion));
     }

diff --git a/.../org/wso2/transport/http/netty/contractimpl/listener/WebSocketServerHandshakeHandler.java b/.../org/wso2/transport/http/netty/contractimpl/listener/WebSocketServerHandshakeHandler.java
@@ -173,9 +173,8 @@ private HttpCarbonRequest setupHttpCarbonRequest(HttpRequest httpRequest, Channe
         sourceReqCmsg.setProperty(Constants.CHNL_HNDLR_CTX, ctx);
         sourceReqCmsg.setProperty(Constants.SRC_HANDLER, this);
         HttpVersion protocolVersion = httpRequest.protocolVersion();
-        sourceReqCmsg.setProperty(Constants.HTTP_VERSION,
-                                  protocolVersion.majorVersion() + "." + protocolVersion.minorVersion());
-        sourceReqCmsg.setProperty(Constants.HTTP_METHOD, httpRequest.method().name());
+        sourceReqCmsg.setHttpVersion(protocolVersion.majorVersion() + "." + protocolVersion.minorVersion());
+        sourceReqCmsg.setHttpMethod(httpRequest.method().name());
         InetSocketAddress localAddress = null;
 
         //This check was added because in case of netty embedded channel, this could be of type 'EmbeddedSocketAddress'.
@@ -194,7 +193,7 @@ private HttpCarbonRequest setupHttpCarbonRequest(HttpRequest httpRequest, Channe
 
         sourceReqCmsg.setProperty(Constants.LOCAL_ADDRESS, ctx.channel().localAddress());
         sourceReqCmsg.setProperty(Constants.REMOTE_ADDRESS, ctx.channel().remoteAddress());
-        sourceReqCmsg.setProperty(Constants.REQUEST_URL, httpRequest.uri());
+        sourceReqCmsg.setRequestUrl(httpRequest.uri());
         sourceReqCmsg.setProperty(Constants.TO, httpRequest.uri());
 
         return sourceReqCmsg;

diff --git a/...o2/transport/http/netty/contractimpl/listener/states/Expect100ContinueHeaderReceived.java b/...o2/transport/http/netty/contractimpl/listener/states/Expect100ContinueHeaderReceived.java
@@ -41,7 +41,6 @@
 import org.wso2.transport.http.netty.message.HttpCarbonMessage;
 
 import static io.netty.buffer.Unpooled.copiedBuffer;
-import static org.wso2.transport.http.netty.contract.Constants.HTTP_STATUS_CODE;
 import static org.wso2.transport.http.netty.contract.Constants
         .IDLE_TIMEOUT_TRIGGERED_BEFORE_INITIATING_100_CONTINUE_RESPONSE;
 import static org.wso2.transport.http.netty.contract.Constants
@@ -90,7 +89,7 @@ public void writeOutboundResponseHeaders(HttpCarbonMessage outboundResponseMsg,
     @Override
     public void writeOutboundResponseBody(HttpOutboundRespListener outboundResponseListener,
                                           HttpCarbonMessage outboundResponseMsg, HttpContent httpContent) {
-        if (outboundResponseMsg.getProperty(HTTP_STATUS_CODE).equals(HttpResponseStatus.CONTINUE.code())) {
+        if (outboundResponseMsg.getHttpStatusCode() == HttpResponseStatus.CONTINUE.code()) {
             messageStateContext.setListenerState(
                     new Response100ContinueSent(outboundResponseListener, sourceHandler, messageStateContext));
         } else {

diff --git a/...ain/java/org/wso2/transport/http/netty/contractimpl/listener/states/ReceivingHeaders.java b/...ain/java/org/wso2/transport/http/netty/contractimpl/listener/states/ReceivingHeaders.java
@@ -25,7 +25,6 @@
 import io.netty.handler.codec.http.HttpRequest;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.wso2.transport.http.netty.contract.Constants;
 import org.wso2.transport.http.netty.contract.ServerConnectorException;
 import org.wso2.transport.http.netty.contract.ServerConnectorFuture;
 import org.wso2.transport.http.netty.contractimpl.HttpOutboundRespListener;
@@ -69,7 +68,7 @@ public ReceivingHeaders(SourceHandler sourceHandler, MessageStateContext message
     @Override
     public void readInboundRequestHeaders(HttpCarbonMessage inboundRequestMsg, HttpRequest inboundRequestHeaders) {
         this.inboundRequestMsg = inboundRequestMsg;
-        this.httpVersion = Float.parseFloat((String) inboundRequestMsg.getProperty(Constants.HTTP_VERSION));
+        this.httpVersion = Float.parseFloat(inboundRequestMsg.getHttpVersion());
         boolean continueRequest = is100ContinueRequest(inboundRequestMsg);
         if (continueRequest) {
             messageStateContext.setListenerState(new Expect100ContinueHeaderReceived(messageStateContext, sourceHandler,