[GitHub] liubao68 closed pull request #532: [SCB-307] avoid to create unnecessary vertx context during send http re?

[GitBox <gi...@apache.org>]

liubao68 closed pull request #532: [SCB-307] avoid to create unnecessary vertx context during send http re?
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/532
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestEdgeInvocation.java b/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestEdgeInvocation.java
index 505e8d6b6..81160b49a 100644
--- a/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestEdgeInvocation.java
+++ b/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestEdgeInvocation.java
@@ -47,6 +47,8 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import io.vertx.core.Context;
+import io.vertx.core.impl.VertxImpl;
 import io.vertx.ext.web.RoutingContext;
 import io.vertx.ext.web.impl.HttpServerRequestWrapperForTest;
 import mockit.Deencapsulation;
@@ -57,7 +59,10 @@
   String microserviceName = "ms";
 
   @Mocked
-  RoutingContext context;
+  RoutingContext routingContext;
+
+  @Mocked
+  Context context;
 
   @Mocked
   HttpServerRequestWrapperForTest request;
@@ -79,11 +84,18 @@
 
   @Before
   public void setup() {
+    new Expectations(VertxImpl.class) {
+      {
+        VertxImpl.context();
+        result = context;
+      }
+    };
+
     Deencapsulation.setField(referenceConfig, "microserviceMeta", microserviceMeta);
     referenceConfig.setMicroserviceVersionRule("latest");
     referenceConfig.setTransport("rest");
 
-    edgeInvocation.init(microserviceName, context, "/base", httpServerFilters);
+    edgeInvocation.init(microserviceName, routingContext, "/base", httpServerFilters);
 
     requestEx = Deencapsulation.getField(edgeInvocation, "requestEx");
     responseEx = Deencapsulation.getField(edgeInvocation, "responseEx");
diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java
index 89daeb748..ca5510c59 100644
--- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java
+++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java
@@ -19,21 +19,29 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Objects;
 
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response.StatusType;
 
 import org.apache.servicecomb.foundation.common.http.HttpStatus;
 
+import io.vertx.core.Context;
+import io.vertx.core.Vertx;
 import io.vertx.core.http.HttpServerResponse;
 
 public class VertxServerResponseToHttpServletResponse extends AbstractHttpServletResponse {
+  private Context context;
+
   private HttpServerResponse serverResponse;
 
   private StatusType statusType;
 
   public VertxServerResponseToHttpServletResponse(HttpServerResponse serverResponse) {
+    this.context = Vertx.currentContext();
     this.serverResponse = serverResponse;
+
+    Objects.requireNonNull(context, "must run in vertx context.");
   }
 
   @Override
@@ -92,6 +100,17 @@ public String getHeader(String name) {
 
   @Override
   public void flushBuffer() throws IOException {
+    if (context == Vertx.currentContext()) {
+      internalFlushBuffer();
+      return;
+    }
+
+    context.runOnContext(V -> {
+      internalFlushBuffer();
+    });
+  }
+
+  public void internalFlushBuffer() {
     if (bodyBuffer == null) {
       serverResponse.end();
       return;
diff --git a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java
index d519c2c34..d821d1412 100644
--- a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java
+++ b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/http/TestVertxServerResponseToHttpServletResponse.java
@@ -26,14 +26,21 @@
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
+import io.vertx.core.Context;
+import io.vertx.core.Handler;
 import io.vertx.core.MultiMap;
 import io.vertx.core.buffer.Buffer;
 import io.vertx.core.http.HttpServerResponse;
+import io.vertx.core.impl.VertxImpl;
 import mockit.Deencapsulation;
+import mockit.Expectations;
 import mockit.Mock;
 import mockit.MockUp;
+import mockit.Mocked;
 
 public class TestVertxServerResponseToHttpServletResponse {
   MultiMap headers = MultiMap.caseInsensitiveMultiMap();
@@ -46,6 +53,14 @@
 
   boolean flushWithBody;
 
+  boolean runOnContextInvoked;
+
+  @Mocked
+  Context context;
+
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
   @Before
   public void setup() {
     serverResponse = new MockUp<HttpServerResponse>() {
@@ -87,9 +102,39 @@ void end(Buffer chunk) {
       }
     }.getMockInstance();
 
+    new Expectations(VertxImpl.class) {
+      {
+        VertxImpl.context();
+        result = context;
+      }
+    };
+
+    new MockUp<Context>(context) {
+      @Mock
+      void runOnContext(Handler<Void> action) {
+        runOnContextInvoked = true;
+        action.handle(null);
+      }
+    };
+
     response = new VertxServerResponseToHttpServletResponse(serverResponse);
   }
 
+  @Test
+  public void construct_invalid() throws IOException {
+    new Expectations(VertxImpl.class) {
+      {
+        VertxImpl.context();
+        result = null;
+      }
+    };
+
+    expectedException.expect(NullPointerException.class);
+    expectedException.expectMessage(Matchers.is("must run in vertx context."));
+
+    new VertxServerResponseToHttpServletResponse(serverResponse);
+  }
+
   @Test
   public void setContentType() {
     response.setContentType("json");
@@ -168,16 +213,36 @@ public void getHeaderNames() {
   }
 
   @Test
-  public void flushBufferNoBody() throws IOException {
+  public void flushBuffer_sameContext() throws IOException {
+    response.flushBuffer();
+
+    Assert.assertFalse(runOnContextInvoked);
+  }
+
+  @Test
+  public void flushBuffer_diffContext() throws IOException {
+    new Expectations(VertxImpl.class) {
+      {
+        VertxImpl.context();
+        result = null;
+      }
+    };
     response.flushBuffer();
 
+    Assert.assertTrue(runOnContextInvoked);
+  }
+
+  @Test
+  public void internalFlushBufferNoBody() throws IOException {
+    response.internalFlushBuffer();
+
     Assert.assertFalse(flushWithBody);
   }
 
   @Test
-  public void flushBufferWithBody() throws IOException {
+  public void internalFlushBufferWithBody() throws IOException {
     response.setBodyBuffer(Buffer.buffer());
-    response.flushBuffer();
+    response.internalFlushBuffer();
 
     Assert.assertTrue(flushWithBody);
   }
diff --git a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
index 6daafabb4..b567195db 100644
--- a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
+++ b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
@@ -36,7 +36,9 @@
 import org.junit.Test;
 
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.ErrorDataDecoderException;
+import io.vertx.core.Context;
 import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.impl.VertxImpl;
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.web.Router;
 import io.vertx.ext.web.RoutingContext;
@@ -149,9 +151,10 @@ public void failureHandlerErrorDataWithNormal(@Mocked RoutingContext context) {
   }
 
   @Test
-  public void onRequest(@Mocked HttpServerRequest request, @Mocked SocketAddress socketAdrress) {
+  public void onRequest(@Mocked Context context, @Mocked HttpServerRequest request,
+      @Mocked SocketAddress socketAdrress) {
     Map<String, Object> map = new HashMap<>();
-    RoutingContext context = new MockUp<RoutingContext>() {
+    RoutingContext routingContext = new MockUp<RoutingContext>() {
       @Mock
       RoutingContext put(String key, Object obj) {
         map.put(key, obj);
@@ -163,7 +166,14 @@ HttpServerRequest request() {
         return request;
       }
     }.getMockInstance();
-    Deencapsulation.invoke(dispatcher, "onRequest", context);
+
+    new Expectations(VertxImpl.class) {
+      {
+        VertxImpl.context();
+        result = context;
+      }
+    };
+    Deencapsulation.invoke(dispatcher, "onRequest", routingContext);
 
     Assert.assertEquals(RestProducerInvocation.class, map.get(RestConst.REST_PRODUCER_INVOCATION).getClass());
     Assert.assertTrue(invoked);


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services