From e396dc157c0bda1a08017b7c3f94846afe8850a5 Mon Sep 17 00:00:00 2001 From: wujimin Date: Fri, 26 Jan 2018 10:41:57 +0800 Subject: [PATCH] SCB-307 avoid to create unnecessary vertx context during send http response. --- .../edge/core/TestEdgeInvocation.java | 16 ++++- ...txServerResponseToHttpServletResponse.java | 19 +++++ ...txServerResponseToHttpServletResponse.java | 71 ++++++++++++++++++- .../rest/vertx/TestVertxRestDispatcher.java | 16 ++++- 4 files changed, 114 insertions(+), 8 deletions(-) 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 505e8d6b67a..81160b49a6d 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 @@ public class TestEdgeInvocation { String microserviceName = "ms"; @Mocked - RoutingContext context; + RoutingContext routingContext; + + @Mocked + Context context; @Mocked HttpServerRequestWrapperForTest request; @@ -79,11 +84,18 @@ public class TestEdgeInvocation { @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 89daeb7483c..ca5510c593b 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 Collection getHeaderNames() { @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 d519c2c34cd..d821d1412cd 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 @@ public class TestVertxServerResponseToHttpServletResponse { boolean flushWithBody; + boolean runOnContextInvoked; + + @Mocked + Context context; + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Before public void setup() { serverResponse = new MockUp() { @@ -87,9 +102,39 @@ void end(Buffer chunk) { } }.getMockInstance(); + new Expectations(VertxImpl.class) { + { + VertxImpl.context(); + result = context; + } + }; + + new MockUp(context) { + @Mock + void runOnContext(Handler 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 6daafabb471..b567195dbff 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 map = new HashMap<>(); - RoutingContext context = new MockUp() { + RoutingContext routingContext = new MockUp() { @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);