[SCB-607] refactor AbstractRestInvocation, set Invocation into Routin…

…gContext in createInvocation()

common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java

@@ -63,11 +63,6 @@ public abstract class AbstractRestInvocation {
 
   protected List<HttpServerFilter> httpServerFilters = Collections.emptyList();
 
-  /**
-   * this handler should be invoked after the invocation is created(which usually happened in createInvocation() method)
-   */
-  protected AfterCreateInvocationHandler afterCreateInvocationHandler;
-
   public void setHttpServerFilters(List<HttpServerFilter> httpServerFilters) {
     this.httpServerFilters = httpServerFilters;
   }
@@ -113,7 +108,7 @@ public String getContext(String key) {
   }
 
   protected void scheduleInvocation() {
-    prepareInvocation();
+    createInvocation();
     invocation.onStart();
     OperationMeta operationMeta = restOperationMeta.getOperationMeta();
 
@@ -138,17 +133,6 @@ protected void scheduleInvocation() {
     });
   }
 
-  /**
-   * Encapsulate the invocation preparation process to ensure the {@link #afterCreateInvocationHandler} is always
-   * invoked after the {@link #invocation} is created.
-   */
-  private void prepareInvocation() {
-    createInvocation();
-    if (null != afterCreateInvocationHandler) {
-      afterCreateInvocationHandler.handle(invocation);
-    }
-  }
-
   protected void runOnExecutor() {
     invocation.onStartExecute();
 
@@ -265,21 +249,4 @@ protected void onExecuteHttpServerFiltersFinish(Response response, Throwable e)
       invocation.onFinish(response);
     }
   }
-
-  public void setAfterCreateInvocationHandler(
-      AfterCreateInvocationHandler afterCreateInvocationHandler) {
-    this.afterCreateInvocationHandler = afterCreateInvocationHandler;
-  }
-
-  /**
-   * After the {@link AbstractRestInvocation#invocation} is instantiated, the {@link AbstractRestInvocation#invocation}
-   * will be passed into {@link AfterCreateInvocationHandler#handle(Invocation)} so that users can do something with it.
-   */
-  public static interface AfterCreateInvocationHandler {
-    /**
-     * @see AfterCreateInvocationHandler
-     * @param invocation {@link AbstractRestInvocation#invocation}
-     */
-    void handle(Invocation invocation);
-  }
 }

common/common-rest/src/main/java/org/apache/servicecomb/common/rest/VertxRestInvocation.java

@@ -0,0 +1,30 @@
+/*
+ * 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;
+
+import org.apache.servicecomb.foundation.vertx.http.VertxServerRequestToHttpServletRequest;
+
+public class VertxRestInvocation extends RestProducerInvocation {
+  @Override
+  protected void createInvocation() {
+    super.createInvocation();
+
+    ((VertxServerRequestToHttpServletRequest) this.requestEx).getContext()
+        .put(RestConst.REST_INVOCATION_CONTEXT, this.invocation);
+  }
+}

common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java

@@ -813,7 +813,6 @@ public void onStart(InvocationStartEvent event) {
     };
 
     Holder<Boolean> result = new Holder<>();
-    Holder<Invocation> createdInvocation = new Holder<>();
     restInvocation = new AbstractRestInvocationForTest() {
       @Override
       protected void runOnExecutor() {
@@ -822,15 +821,11 @@ protected void runOnExecutor() {
     };
     restInvocation.requestEx = requestEx;
     restInvocation.restOperationMeta = restOperation;
-    // prepareInvocation() is invoked in restInvocation.scheduleInvocation(),
-    // test whether the afterCreateInvocationHandler is invoked properly
-    restInvocation.setAfterCreateInvocationHandler(invocation -> createdInvocation.value = invocation);
 
     restInvocation.scheduleInvocation();
     EventManager.unregister(subscriber);
 
     Assert.assertTrue(result.value);
-    Assert.assertSame(this.invocation, createdInvocation.value);
     Assert.assertEquals(time, invocation.getStartTime());
     Assert.assertSame(invocation, eventHolder.value.getInvocation());
   }

common/common-rest/src/test/java/org/apache/servicecomb/common/rest/VertxRestInvocationTest.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;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.foundation.vertx.http.VertxServerRequestToHttpServletRequest;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import io.vertx.ext.web.RoutingContext;
+import mockit.Deencapsulation;
+import mockit.Mock;
+import mockit.MockUp;
+
+public class VertxRestInvocationTest {
+  @Test
+  public void testCreateInvocation() {
+    new MockUp<RestProducerInvocation>() {
+      /**
+       * Mock this method to avoid error
+       */
+      @Mock
+      void createInvocation() {
+      }
+    };
+
+    VertxRestInvocation vertxRestInvocation = new VertxRestInvocation();
+    VertxServerRequestToHttpServletRequest requestEx = Mockito.mock(VertxServerRequestToHttpServletRequest.class);
+    RoutingContext routingContext = Mockito.mock(RoutingContext.class);
+    Invocation invocation = Mockito.mock(Invocation.class);
+
+    Deencapsulation.setField(
+        vertxRestInvocation, "requestEx", requestEx);
+    Deencapsulation.setField(
+        vertxRestInvocation, "invocation", invocation);
+    Mockito.when(requestEx.getContext()).thenReturn(routingContext);
+
+    Deencapsulation.invoke(vertxRestInvocation, "createInvocation");
+
+    Mockito.verify(routingContext, Mockito.times(1)).put(RestConst.REST_INVOCATION_CONTEXT, invocation);
+  }
+}

edge/edge-core/src/main/java/org/apache/servicecomb/edge/core/EdgeInvocation.java

@@ -52,14 +52,16 @@ public class EdgeInvocation extends AbstractRestInvocation {
 
   protected String versionRule = DefinitionConst.VERSION_RULE_ALL;
 
+  protected RoutingContext routingContext;
+
   public void init(String microserviceName, RoutingContext context, String path,
       List<HttpServerFilter> httpServerFilters) {
     this.microserviceName = microserviceName;
     this.requestEx = new VertxServerRequestToHttpServletRequest(context, path);
     this.responseEx = new VertxServerResponseToHttpServletResponse(context.response());
+    this.routingContext = context;
     this.httpServerFilters = httpServerFilters;
     requestEx.setAttribute(RestConst.REST_REQUEST, requestEx);
-    setAfterCreateInvocationHandler(invocation -> context.put(RestConst.REST_INVOCATION_CONTEXT, invocation));
   }
 
   public void edgeInvoke() {
@@ -128,5 +130,6 @@ protected void createInvocation() {
     this.invocation.setSync(false);
     this.invocation.getHandlerContext().put(EDGE_INVOCATION_CONTEXT, Vertx.currentContext());
     this.invocation.setResponseExecutor(new ReactiveResponseExecutor());
+    this.routingContext.put(RestConst.REST_INVOCATION_CONTEXT, invocation);
   }
 }

edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestEdgeInvocation.java

@@ -22,8 +22,6 @@
 import java.util.List;
 import java.util.Map;
 
-import org.apache.servicecomb.common.rest.AbstractRestInvocation.AfterCreateInvocationHandler;
-import org.apache.servicecomb.common.rest.RestConst;
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
 import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
 import org.apache.servicecomb.common.rest.locator.OperationLocator;
@@ -48,10 +46,8 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.mockito.Mockito;
 
 import io.vertx.core.Context;
-import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.impl.VertxImpl;
 import io.vertx.ext.web.RoutingContext;
 import io.vertx.ext.web.impl.HttpServerRequestWrapperForTest;
@@ -236,22 +232,7 @@ public void createInvocation(@Mocked MicroserviceVersionMeta microserviceVersion
   }
 
   @Test
-  public void testSetAfterCreateInvocationHandler() {
-    EdgeInvocation edgeInvocation = new EdgeInvocation();
-    RoutingContext routingContext = Mockito.mock(RoutingContext.class);
-    HttpServerRequest httpServerRequest = Mockito.mock(HttpServerRequest.class);
-
-    Mockito.when(routingContext.request()).thenReturn(httpServerRequest);
-
-    // afterCreateInvocationHandler should be set in init()
-    edgeInvocation.init(microserviceName, routingContext, "/base", httpServerFilters);
-
-    AfterCreateInvocationHandler afterCreateInvocationHandler = Deencapsulation
-        .getField(edgeInvocation, "afterCreateInvocationHandler");
-
-    Invocation invocation = Mockito.mock(Invocation.class);
-    afterCreateInvocationHandler.handle(invocation);
-
-    Mockito.verify(routingContext, Mockito.times(1)).put(RestConst.REST_INVOCATION_CONTEXT, invocation);
+  public void testSetRoutingContext() {
+    Assert.assertSame(this.routingContext, edgeInvocation.routingContext);
   }
 }

foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/http/VertxServerRequestToHttpServletRequest.java

@@ -236,4 +236,8 @@ public Part getPart(String name) {
     final FileUpload fileUpload = upload.get();
     return new FileUploadPart(fileUpload);
   }
+
+  public RoutingContext getContext() {
+    return context;
+  }
 }

transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/VertxRestDispatcher.java

@@ -24,7 +24,7 @@
 
 import org.apache.servicecomb.common.rest.AbstractRestInvocation;
 import org.apache.servicecomb.common.rest.RestConst;
-import org.apache.servicecomb.common.rest.RestProducerInvocation;
+import org.apache.servicecomb.common.rest.VertxRestInvocation;
 import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.core.CseContext;
 import org.apache.servicecomb.core.Transport;
@@ -189,10 +189,8 @@ private void onRequest(RoutingContext context) {
     HttpServletRequestEx requestEx = new VertxServerRequestToHttpServletRequest(context);
     HttpServletResponseEx responseEx = new VertxServerResponseToHttpServletResponse(context.response());
 
-    RestProducerInvocation restProducerInvocation = new RestProducerInvocation();
-    context.put(RestConst.REST_PRODUCER_INVOCATION, restProducerInvocation);
-    restProducerInvocation.setAfterCreateInvocationHandler(
-        invocation -> context.put(RestConst.REST_INVOCATION_CONTEXT, invocation));
-    restProducerInvocation.invoke(transport, requestEx, responseEx, httpServerFilters);
+    VertxRestInvocation vertxRestInvocation = new VertxRestInvocation();
+    context.put(RestConst.REST_PRODUCER_INVOCATION, vertxRestInvocation);
+    vertxRestInvocation.invoke(transport, requestEx, responseEx, httpServerFilters);
   }
 }

transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java

@@ -25,9 +25,9 @@
 import javax.ws.rs.core.Response.Status;
 
 import org.apache.http.HttpHeaders;
-import org.apache.servicecomb.common.rest.AbstractRestInvocation.AfterCreateInvocationHandler;
 import org.apache.servicecomb.common.rest.RestConst;
 import org.apache.servicecomb.common.rest.RestProducerInvocation;
+import org.apache.servicecomb.common.rest.VertxRestInvocation;
 import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
 import org.apache.servicecomb.core.CseContext;
 import org.apache.servicecomb.core.Transport;
@@ -74,8 +74,6 @@ public class TestVertxRestDispatcher {
 
   boolean invoked;
 
-  AfterCreateInvocationHandler afterCreateInvocationHandler;
-
   @Before
   public void setUp() {
     dispatcher = new VertxRestDispatcher();
@@ -92,12 +90,6 @@ void invoke(Transport transport, HttpServletRequestEx requestEx, HttpServletResp
           List<HttpServerFilter> httpServerFilters) {
         invoked = true;
       }
-
-      @Mock
-      void setAfterCreateInvocationHandler(
-          AfterCreateInvocationHandler afterCreateInvocationHandler) {
-        TestVertxRestDispatcher.this.afterCreateInvocationHandler = afterCreateInvocationHandler;
-      }
     };
 
     CseContext.getInstance().setTransportManager(transportManager);
@@ -308,8 +300,7 @@ HttpServerRequest request() {
     };
     Deencapsulation.invoke(dispatcher, "onRequest", routingContext);
 
-    Assert.assertEquals(RestProducerInvocation.class, map.get(RestConst.REST_PRODUCER_INVOCATION).getClass());
-    Assert.assertNotNull(this.afterCreateInvocationHandler);
+    Assert.assertEquals(VertxRestInvocation.class, map.get(RestConst.REST_PRODUCER_INVOCATION).getClass());
     Assert.assertTrue(invoked);
   }