You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2018/04/20 01:50:25 UTC

[incubator-servicecomb-java-chassis] 01/09: SCB-483 HttpServerFilter support async beforeSendResponse

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git

commit be11d1da49bb417641121ebfa2b7307a07504463
Author: wujimin <wu...@huawei.com>
AuthorDate: Tue Apr 17 15:36:26 2018 +0800

    SCB-483 HttpServerFilter support async beforeSendResponse
---
 .../common/rest/AbstractRestInvocation.java        | 46 ++++++++++---
 .../common/rest/filter/HttpServerFilter.java       | 30 +++++++--
 ...HttpServerFilterBeforeSendResponseExecutor.java | 78 ++++++++++++++++++++++
 .../common/rest/TestAbstractRestInvocation.java    | 10 +--
 .../rest/filter/HttpServerFilterBaseForTest.java}  | 22 +++---
 .../common/rest/filter/TestHttpServerFilter.java   | 57 ++++++++++++++++
 ...HttpServerFilterBeforeSendResponseExecutor.java | 78 ++++++++++++++++++++++
 7 files changed, 288 insertions(+), 33 deletions(-)

diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
index 3dfb5cc..89be43e 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
@@ -17,11 +17,13 @@
 
 package org.apache.servicecomb.common.rest;
 
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.concurrent.CompletableFuture;
 
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response.Status;
@@ -31,6 +33,7 @@ import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessor;
 import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessorManager;
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
 import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
+import org.apache.servicecomb.common.rest.filter.HttpServerFilterBeforeSendResponseExecutor;
 import org.apache.servicecomb.common.rest.locator.OperationLocator;
 import org.apache.servicecomb.common.rest.locator.ServicePathManager;
 import org.apache.servicecomb.core.Const;
@@ -194,18 +197,11 @@ public abstract class AbstractRestInvocation {
       LOGGER.error("Failed to send rest response, operation:{}.",
           invocation.getMicroserviceQualifiedName(),
           e);
-    } finally {
-      requestEx.getAsyncContext().complete();
-      // if failed to locate path, then will not create invocation
-      // TODO: statistics this case
-      if (invocation != null) {
-        invocation.onFinish(response);
-      }
     }
   }
 
   @SuppressWarnings("deprecation")
-  protected void sendResponse(Response response) throws Exception {
+  protected void sendResponse(Response response) {
     if (response.getHeaders().getHeaderMap() != null) {
       for (Entry<String, List<Object>> entry : response.getHeaders().getHeaderMap().entrySet()) {
         for (Object value : entry.getValue()) {
@@ -220,10 +216,38 @@ public abstract class AbstractRestInvocation {
     responseEx.setAttribute(RestConst.INVOCATION_HANDLER_RESPONSE, response);
     responseEx.setAttribute(RestConst.INVOCATION_HANDLER_PROCESSOR, produceProcessor);
 
-    for (HttpServerFilter filter : httpServerFilters) {
-      filter.beforeSendResponse(invocation, responseEx);
+    executeHttpServerFilters(response);
+  }
+
+  protected void executeHttpServerFilters(Response response) {
+    HttpServerFilterBeforeSendResponseExecutor exec =
+        new HttpServerFilterBeforeSendResponseExecutor(httpServerFilters, invocation, responseEx);
+    CompletableFuture<Void> future = exec.run();
+    future.whenComplete((v, e) -> {
+      onExecuteHttpServerFiltersFinish(response, e);
+    });
+  }
+
+  protected void onExecuteHttpServerFiltersFinish(Response response, Throwable e) {
+    if (e != null) {
+      LOGGER.error("Failed to execute HttpServerFilters, operation:{}.",
+          invocation.getMicroserviceQualifiedName(),
+          e);
     }
 
-    responseEx.flushBuffer();
+    try {
+      responseEx.flushBuffer();
+    } catch (IOException flushException) {
+      LOGGER.error("Failed to flush rest response, operation:{}.",
+          invocation.getMicroserviceQualifiedName(),
+          flushException);
+    }
+
+    requestEx.getAsyncContext().complete();
+    // if failed to locate path, then will not create invocation
+    // TODO: statistics this case
+    if (invocation != null) {
+      invocation.onFinish(response);
+    }
   }
 }
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java
index af0f6b1..594c262 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java
@@ -17,6 +17,8 @@
 
 package org.apache.servicecomb.common.rest.filter;
 
+import java.util.concurrent.CompletableFuture;
+
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
@@ -30,10 +32,30 @@ public interface HttpServerFilter {
     return false;
   }
 
-  // if finished, then return a none null response
-  // if return a null response, then sdk will call next filter.afterReceiveRequest
+  /**
+   * @return if finished, then return a none null response<br>
+   * if return a null response, then sdk will call next filter.afterReceiveRequest
+   */
   Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx requestEx);
 
-  // invocation maybe null
-  void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx);
+  /**
+   * @param invocation maybe null
+   */
+  default CompletableFuture<Void> asyncBeforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) {
+    CompletableFuture<Void> future = new CompletableFuture<>();
+    try {
+      beforeSendResponse(invocation, responseEx);
+      future.complete(null);
+    } catch (Throwable e) {
+      future.completeExceptionally(e);
+    }
+    return future;
+  }
+
+  /**
+   * @param invocation maybe null
+   */
+  default void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) {
+
+  }
 }
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBeforeSendResponseExecutor.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBeforeSendResponseExecutor.java
new file mode 100644
index 0000000..70889cc
--- /dev/null
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBeforeSendResponseExecutor.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.servicecomb.common.rest.filter;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
+
+public class HttpServerFilterBeforeSendResponseExecutor {
+  private List<HttpServerFilter> httpServerFilters;
+
+  private Invocation invocation;
+
+  private HttpServletResponseEx responseEx;
+
+  private int currentIndex;
+
+  private CompletableFuture<Void> future = new CompletableFuture<Void>();
+
+  public HttpServerFilterBeforeSendResponseExecutor(List<HttpServerFilter> httpServerFilters, Invocation invocation,
+      HttpServletResponseEx responseEx) {
+    this.httpServerFilters = httpServerFilters;
+    this.invocation = invocation;
+    this.responseEx = responseEx;
+  }
+
+  public CompletableFuture<Void> run() {
+    doRun();
+
+    return future;
+  }
+
+  protected CompletableFuture<Void> safeInvoke(HttpServerFilter httpServerFilter) {
+    try {
+      return httpServerFilter.asyncBeforeSendResponse(invocation, responseEx);
+    } catch (Throwable e) {
+      CompletableFuture<Void> eFuture = new CompletableFuture<Void>();
+      eFuture.completeExceptionally(e);
+      return eFuture;
+    }
+  }
+
+  protected void doRun() {
+    if (currentIndex == httpServerFilters.size()) {
+      future.complete(null);
+      return;
+    }
+
+    HttpServerFilter httpServerFilter = httpServerFilters.get(currentIndex);
+    currentIndex++;
+
+    CompletableFuture<Void> stepFuture = safeInvoke(httpServerFilter);
+    stepFuture.whenComplete((v, e) -> {
+      if (e == null) {
+        doRun();
+        return;
+      }
+
+      future.completeExceptionally(e);
+    });
+  }
+}
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
index 72d0b13..6d82bc6 100644
--- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
@@ -29,6 +29,7 @@ import javax.xml.ws.Holder;
 import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessorManager;
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
 import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
+import org.apache.servicecomb.common.rest.filter.HttpServerFilterBaseForTest;
 import org.apache.servicecomb.common.rest.locator.OperationLocator;
 import org.apache.servicecomb.common.rest.locator.ServicePathManager;
 import org.apache.servicecomb.core.Const;
@@ -403,6 +404,7 @@ public class TestAbstractRestInvocation {
       @Override
       protected void sendResponse(Response response) {
         result.value = response;
+        super.sendResponse(response);
       }
     };
     initRestInvocation();
@@ -648,12 +650,12 @@ public class TestAbstractRestInvocation {
       }
     }.getMockInstance();
 
-    HttpServerFilter filter = new MockUp<HttpServerFilter>() {
-      @Mock
-      void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) {
+    HttpServerFilter filter = new HttpServerFilterBaseForTest() {
+      @Override
+      public void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) {
         buffer.appendString("-filter");
       }
-    }.getMockInstance();
+    };
 
     initRestInvocation();
     List<HttpServerFilter> httpServerFilters = SPIServiceUtils.loadSortedService(HttpServerFilter.class);
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBaseForTest.java
similarity index 63%
copy from common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java
copy to common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBaseForTest.java
index af0f6b1..24ccd95 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/HttpServerFilter.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/HttpServerFilterBaseForTest.java
@@ -14,26 +14,20 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.servicecomb.common.rest.filter;
 
 import org.apache.servicecomb.core.Invocation;
-import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
-import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
 import org.apache.servicecomb.swagger.invocation.Response;
 
-public interface HttpServerFilter {
-  int getOrder();
-
-  default boolean needCacheRequest(OperationMeta operationMeta) {
-    return false;
+public class HttpServerFilterBaseForTest implements HttpServerFilter {
+  @Override
+  public int getOrder() {
+    return 0;
   }
 
-  // if finished, then return a none null response
-  // if return a null response, then sdk will call next filter.afterReceiveRequest
-  Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx requestEx);
-
-  // invocation maybe null
-  void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx);
+  @Override
+  public Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx requestEx) {
+    return null;
+  }
 }
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilter.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilter.java
new file mode 100644
index 0000000..d7b0d01
--- /dev/null
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilter.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.servicecomb.common.rest.filter;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class TestHttpServerFilter {
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  @Test
+  public void asyncSucc() throws InterruptedException, ExecutionException {
+    HttpServerFilter filter = new HttpServerFilterBaseForTest();
+
+    CompletableFuture<Void> future = filter.asyncBeforeSendResponse(null, null);
+    Assert.assertNull(future.get());
+  }
+
+  @Test
+  public void asyncFailed() throws InterruptedException, ExecutionException {
+    HttpServerFilter filter = new HttpServerFilterBaseForTest() {
+      @Override
+      public void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) {
+        throw new Error("");
+      }
+    };
+
+    expectedException.expect(ExecutionException.class);
+    expectedException.expectCause(Matchers.instanceOf(Error.class));
+
+    CompletableFuture<Void> future = filter.asyncBeforeSendResponse(null, null);
+    future.get();
+  }
+}
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilterBeforeSendResponseExecutor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilterBeforeSendResponseExecutor.java
new file mode 100644
index 0000000..ba565ab
--- /dev/null
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/TestHttpServerFilterBeforeSendResponseExecutor.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.servicecomb.common.rest.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
+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 mockit.Mocked;
+
+public class TestHttpServerFilterBeforeSendResponseExecutor {
+  @Mocked
+  Invocation invocation;
+
+  @Mocked
+  HttpServletResponseEx responseEx;
+
+  List<HttpServerFilter> httpServerFilters = new ArrayList<>();
+
+  HttpServerFilterBeforeSendResponseExecutor executor =
+      new HttpServerFilterBeforeSendResponseExecutor(httpServerFilters, invocation, responseEx);
+
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  @Before
+  public void setup() {
+    httpServerFilters.add(new HttpServerFilterBaseForTest());
+  }
+
+  @Test
+  public void runSucc() throws InterruptedException, ExecutionException {
+    CompletableFuture<Void> result = executor.run();
+
+    Assert.assertNull(result.get());
+  }
+
+  @Test
+  public void runFail() throws InterruptedException, ExecutionException {
+    httpServerFilters.add(new HttpServerFilterBaseForTest() {
+      @Override
+      public CompletableFuture<Void> asyncBeforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx) {
+        throw new Error("");
+      }
+    });
+
+    CompletableFuture<Void> result = executor.run();
+
+    expectedException.expect(ExecutionException.class);
+    expectedException.expectCause(Matchers.instanceOf(Error.class));
+
+    result.get();
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
liubao@apache.org.