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:
[email protected]
With regards,
Apache Git Services
