[SCB-2029] tiny improve to make extend filter features easier

apache · Jul 2, 2020 · fcc5ca8 · fcc5ca8
1 parent 5d6c9e1
commit fcc5ca8
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 43 deletions.
diff --git a/.../src/main/java/org/apache/servicecomb/common/rest/filter/inner/RestServerCodecFilter.java b/.../src/main/java/org/apache/servicecomb/common/rest/filter/inner/RestServerCodecFilter.java
@@ -59,7 +59,7 @@ public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode ne
         .thenCompose(response -> encodeResponse(invocation, response));
   }
 
-  private CompletableFuture<Invocation> decodeRequest(Invocation invocation) {
+  protected CompletableFuture<Invocation> decodeRequest(Invocation invocation) {
     HttpTransportContext transportContext = invocation.getTransportContext();
     HttpServletRequestEx requestEx = transportContext.getRequestEx();
 
@@ -71,7 +71,7 @@ private CompletableFuture<Invocation> decodeRequest(Invocation invocation) {
     return CompletableFuture.completedFuture(invocation);
   }
 
-  private CompletableFuture<Response> encodeResponse(Invocation invocation, Response response) {
+  protected CompletableFuture<Response> encodeResponse(Invocation invocation, Response response) {
     HttpTransportContext transportContext = invocation.getTransportContext();
     ProduceProcessor produceProcessor = transportContext.getProduceProcessor();
     HttpServletResponseEx responseEx = transportContext.getResponseEx();

diff --git a/...rg/apache/servicecomb/core/exception/converter/ConstraintViolationExceptionConverter.java b/...rg/apache/servicecomb/core/exception/converter/ConstraintViolationExceptionConverter.java
@@ -35,7 +35,7 @@
 import com.netflix.config.DynamicStringProperty;
 
 public class ConstraintViolationExceptionConverter implements ExceptionConverter<ConstraintViolationException> {
-  public static final short ORDER = Short.MAX_VALUE;
+  public static final int ORDER = Short.MAX_VALUE;
 
   public static final String KEY_CODE = "servicecomb.filters.validate.code";
 

diff --git a/.../main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java b/.../main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java
@@ -40,9 +40,11 @@
 public class DefaultExceptionConverter implements ExceptionConverter<Throwable> {
   private static final Logger LOGGER = LoggerFactory.getLogger(DefaultExceptionConverter.class);
 
+  public static final int ORDER = Integer.MAX_VALUE;
+
   @Override
   public int getOrder() {
-    return Integer.MAX_VALUE;
+    return ORDER;
   }
 
   @Override

diff --git a/...va/org/apache/servicecomb/core/exception/converter/IllegalArgumentExceptionConverter.java b/...va/org/apache/servicecomb/core/exception/converter/IllegalArgumentExceptionConverter.java
@@ -24,7 +24,7 @@
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 
 public class IllegalArgumentExceptionConverter implements ExceptionConverter<IllegalArgumentException> {
-  public static final short ORDER = Short.MAX_VALUE;
+  public static final int ORDER = Short.MAX_VALUE;
 
   @Override
   public int getOrder() {

diff --git a/...in/java/org/apache/servicecomb/core/exception/converter/InvocationExceptionConverter.java b/...in/java/org/apache/servicecomb/core/exception/converter/InvocationExceptionConverter.java
@@ -24,7 +24,7 @@
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 
 public class InvocationExceptionConverter implements ExceptionConverter<InvocationException> {
-  public static final byte ORDER = Byte.MAX_VALUE;
+  public static final int ORDER = Byte.MAX_VALUE;
 
   @Override
   public int getOrder() {

diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ParameterValidatorFilter.java
@@ -54,13 +54,16 @@ public class ParameterValidatorFilter implements Filter {
   private final ExecutableValidator validator;
 
   public ParameterValidatorFilter() {
-    ValidatorFactory factory =
-        Validation.byProvider(HibernateValidator.class)
-            .configure()
-            .propertyNodeNameProvider(new JacksonPropertyNodeNameProvider())
-            .messageInterpolator(messageInterpolator())
-            .buildValidatorFactory();
-    validator = factory.getValidator().forExecutables();
+    validator = createValidatorFactory()
+        .getValidator().forExecutables();
+  }
+
+  protected ValidatorFactory createValidatorFactory() {
+    return Validation.byProvider(HibernateValidator.class)
+        .configure()
+        .propertyNodeNameProvider(new JacksonPropertyNodeNameProvider())
+        .messageInterpolator(messageInterpolator())
+        .buildValidatorFactory();
   }
 
   private AbstractMessageInterpolator messageInterpolator() {
@@ -76,16 +79,20 @@ private boolean useResourceBundleMessageInterpolator() {
 
   @Override
   public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode nextNode) {
-    SwaggerProducerOperation producerOperation = invocation.getOperationMeta().getSwaggerProducerOperation();
-    Object instance = producerOperation.getProducerInstance();
-    Method method = producerOperation.getProducerMethod();
-    Object[] args = invocation.toProducerArguments();
-    Set<ConstraintViolation<Object>> violations = validator.validateParameters(instance, method, args, Default.class);
+    Set<ConstraintViolation<Object>> violations = doValidate(invocation);
     if (violations.size() > 0) {
       LOGGER.error("Parameter validation failed : " + violations.toString());
       return AsyncUtils.completeExceptionally(new ConstraintViolationException(violations));
     }
 
     return nextNode.onFilter(invocation);
   }
+
+  protected Set<ConstraintViolation<Object>> doValidate(Invocation invocation) {
+    SwaggerProducerOperation producerOperation = invocation.getOperationMeta().getSwaggerProducerOperation();
+    Object instance = producerOperation.getProducerInstance();
+    Method method = producerOperation.getProducerMethod();
+    Object[] args = invocation.toProducerArguments();
+    return validator.validateParameters(instance, method, args, Default.class);
+  }
 }
diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java
@@ -52,7 +52,7 @@ public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode ne
   }
 
   @SuppressWarnings("unchecked")
-  private CompletableFuture<Object> invoke(Invocation invocation, Object instance, Method method, Object[] args) {
+  protected CompletableFuture<Object> invoke(Invocation invocation, Object instance, Method method, Object[] args) {
     ContextUtils.setInvocationContext(invocation);
 
     try {
@@ -69,12 +69,12 @@ private CompletableFuture<Object> invoke(Invocation invocation, Object instance,
     }
   }
 
-  private Response convertResultToResponse(Invocation invocation, SwaggerProducerOperation producerOperation,
+  protected Response convertResultToResponse(Invocation invocation, SwaggerProducerOperation producerOperation,
       Object result) {
     return producerOperation.getResponseMapper().mapResponse(invocation.getStatus(), result);
   }
 
-  private void whenComplete(Invocation invocation, Throwable throwable) {
+  protected void whenComplete(Invocation invocation, Throwable throwable) {
     if (throwable != null) {
       Throwable unwrapped = Exceptions.unwrap(throwable);
       if (shouldPrintErrorLog(unwrapped)) {

diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ScheduleFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ScheduleFilter.java
@@ -39,7 +39,7 @@ public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode ne
         .thenComposeAsync(response -> runInExecutor(invocation, next), executor);
   }
 
-  private CompletableFuture<Response> runInExecutor(Invocation invocation, FilterNode next) {
+  protected CompletableFuture<Response> runInExecutor(Invocation invocation, FilterNode next) {
     invocation.onExecuteStart();
     InvocationStageTrace trace = invocation.getInvocationStageTrace();
     trace.startServerFiltersRequest();

diff --git a/.../src/main/java/org/apache/servicecomb/foundation/common/utils/LambdaMetafactoryUtils.java b/.../src/main/java/org/apache/servicecomb/foundation/common/utils/LambdaMetafactoryUtils.java
@@ -165,7 +165,7 @@ private static void checkAccess(Field field) {
     // This check is not accurate. Most of time package visible and protected access can be ignored, so simply do this.
     if (!Modifier.isPublic(field.getModifiers()) || !Modifier.isPublic(field.getDeclaringClass().getModifiers())) {
       throw new IllegalStateException(
-          String.format("Can not access field, a public field or and accessor is required."
+          String.format("Can not access field, a public field or accessor is required."
                   + "Declaring class is %s, field is %s",
               field.getDeclaringClass().getName(),
               field.getName()));

diff --git a/.../test/java/org/apache/servicecomb/foundation/common/utils/TestLambdaMetafactoryUtils.java b/.../test/java/org/apache/servicecomb/foundation/common/utils/TestLambdaMetafactoryUtils.java
@@ -16,6 +16,9 @@
  */
 package org.apache.servicecomb.foundation.common.utils;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+
 import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.List;
@@ -31,8 +34,10 @@
 import org.junit.Test;
 
 public class TestLambdaMetafactoryUtils {
-  static class Model {
-    private int f1;
+  public static class Model {
+    public int f1;
+
+    private int f2;
 
     public int getF1() {
       return f1;
@@ -91,19 +96,15 @@ public void createGetterSetterByMethod() throws Throwable {
   }
 
   @Test
-  public void createGetterSetterByField() throws Throwable {
-    Field f1 = Model.class.getDeclaredField("f1");
-    try {
-      LambdaMetafactoryUtils.createGetter(f1);
-      Assert.fail();
-    } catch (IllegalStateException e) {
-      Assert.assertTrue(true);
-    }
-    try {
-      LambdaMetafactoryUtils.createSetter(f1);
-      Assert.fail();
-    } catch (IllegalStateException e) {
-      Assert.assertTrue(true);
-    }
+  public void should_failed_when_createGetterSetterByField_and_field_is_not_public() throws Throwable {
+    Field field = Model.class.getDeclaredField("f2");
+    assertThat(catchThrowable(() -> LambdaMetafactoryUtils.createGetter(field)))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage(
+            "Can not access field, a public field or accessor is required.Declaring class is org.apache.servicecomb.foundation.common.utils.TestLambdaMetafactoryUtils$Model, field is f2");
+    assertThat(catchThrowable(() -> LambdaMetafactoryUtils.createSetter(field)))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage(
+            "Can not access field, a public field or accessor is required.Declaring class is org.apache.servicecomb.foundation.common.utils.TestLambdaMetafactoryUtils$Model, field is f2");
   }
 }
diff --git a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/Invoker.java b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/Invoker.java
@@ -116,7 +116,7 @@ protected SchemaMeta findSchemaMeta(MicroserviceMeta microserviceMeta) {
     return microserviceMeta.findSchemaMeta(consumerIntf.getName());
   }
 
-  private PojoConsumerMeta refreshMeta() {
+  protected PojoConsumerMeta refreshMeta() {
     MicroserviceReferenceConfig microserviceReferenceConfig = scbEngine
         .createMicroserviceReferenceConfig(microserviceName);
     MicroserviceMeta microserviceMeta = microserviceReferenceConfig.getLatestMicroserviceMeta();
@@ -185,7 +185,6 @@ public Object invoke(Object proxy, Method method, Object[] args) {
     return syncInvoke(invocation, consumerOperation);
   }
 
-
   public Map<String, Object> toArguments(Method method, Object[] args) {
     Map<String, Object> arguments = new HashMap<>();
     for (int i = 0; i < method.getParameterCount(); i++) {

diff --git a/...hway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerCodecFilter.java b/...hway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerCodecFilter.java
@@ -45,7 +45,7 @@ public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode ne
         .thenCompose(response -> encodeResponse(invocation, response));
   }
 
-  private CompletableFuture<Invocation> decodeRequest(Invocation invocation) {
+  protected CompletableFuture<Invocation> decodeRequest(Invocation invocation) {
     HighwayTransportContext transportContext = invocation.getTransportContext();
     try {
       HighwayCodec.decodeRequest(invocation,
@@ -58,7 +58,7 @@ private CompletableFuture<Invocation> decodeRequest(Invocation invocation) {
     }
   }
 
-  private CompletableFuture<Response> encodeResponse(Invocation invocation, Response response) {
+  protected CompletableFuture<Response> encodeResponse(Invocation invocation, Response response) {
     ResponseHeader header = new ResponseHeader();
     header.setStatusCode(response.getStatusCode());
     header.setReasonPhrase(response.getReasonPhrase());