fix serverInterceptor order error (LogNet#56)

imyxiao · jvmlet · commit eb6fbf87ae57 · 2017-09-06T14:11:20.000+03:00
Good catch , thanks @imyxiao. Do you need release or snapshot build is OK ?
diff --git a/grpc-spring-boot-starter-demo/src/test/java/org/lognet/springboot/grpc/OrderedInterceptorsTest.java b/grpc-spring-boot-starter-demo/src/test/java/org/lognet/springboot/grpc/OrderedInterceptorsTest.java
@@ -1,19 +1,10 @@
 package org.lognet.springboot.grpc;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
-import io.grpc.ManagedChannel;
-import io.grpc.ManagedChannelBuilder;
-import io.grpc.Metadata;
-import io.grpc.ServerCall;
+import io.grpc.*;
 import io.grpc.ServerCall.Listener;
-import io.grpc.ServerCallHandler;
-import io.grpc.ServerInterceptor;
 import io.grpc.examples.GreeterGrpc;
 import io.grpc.examples.GreeterGrpc.GreeterBlockingStub;
 import io.grpc.examples.GreeterOuterClass;
-import java.util.ArrayList;
-import java.util.List;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -26,6 +17,11 @@
 import org.springframework.core.annotation.Order;
 import org.springframework.test.context.junit4.SpringRunner;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
 
 /**
  * Created by 310242212 on 11-Sep-16.
@@ -111,26 +107,26 @@ public <ReqT, RespT> Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call,
       }
     }
 
-    // interceptors without any annotation will always be executed last, losing to any defined @Order
     @GRpcGlobalInterceptor
-    static class UnorderedInterceptor implements ServerInterceptor {
+    @Order // no value means lowest priority amongst all @Ordered, but higher priority than iceptors without the annot
+    static class DefaultOrderedInterceptor implements ServerInterceptor {
 
       @Override
       public <ReqT, RespT> Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call, Metadata headers,
-          ServerCallHandler<ReqT, RespT> next) {
-        calledInterceptors.add(100);
+                                                        ServerCallHandler<ReqT, RespT> next) {
+        calledInterceptors.add(10);
         return next.startCall(call, headers);
       }
     }
 
+    // interceptors without any annotation will always be executed last, losing to any defined @Order
     @GRpcGlobalInterceptor
-    @Order // no value means lowest priority amongst all @Ordered, but higher priority than iceptors without the annot
-    static class DefaultOrderedInterceptor implements ServerInterceptor {
+    static class UnorderedInterceptor implements ServerInterceptor {
 
       @Override
       public <ReqT, RespT> Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call, Metadata headers,
           ServerCallHandler<ReqT, RespT> next) {
-        calledInterceptors.add(10);
+        calledInterceptors.add(100);
         return next.startCall(call, headers);
       }
     }
diff --git a/grpc-spring-boot-starter/src/main/java/org/lognet/springboot/grpc/GRpcServerRunner.java b/grpc-spring-boot-starter/src/main/java/org/lognet/springboot/grpc/GRpcServerRunner.java
@@ -1,14 +1,7 @@
 package org.lognet.springboot.grpc;
 
-import io.grpc.BindableService;
-import io.grpc.Server;
-import io.grpc.ServerBuilder;
-import io.grpc.ServerInterceptor;
-import io.grpc.ServerInterceptors;
-import io.grpc.ServerServiceDefinition;
+import io.grpc.*;
 import io.grpc.health.v1.HealthCheckResponse;
-import io.grpc.inprocess.InProcessServerBuilder;
-import io.grpc.netty.NettyServerBuilder;
 import io.grpc.services.HealthStatusManager;
 import lombok.extern.slf4j.Slf4j;
 import org.lognet.springboot.grpc.autoconfigure.GRpcServerProperties;
@@ -19,16 +12,13 @@
 import org.springframework.boot.CommandLineRunner;
 import org.springframework.context.support.AbstractApplicationContext;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
+import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.core.annotation.Order;
 import org.springframework.core.type.StandardMethodMetadata;
 
 import java.lang.annotation.Annotation;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.ConcurrentSkipListSet;
+import java.lang.reflect.Method;
+import java.util.*;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -115,12 +105,24 @@ private ServerServiceDefinition bindInterceptors(ServerServiceDefinition service
                 gRpcService.applyGlobalInterceptors() ? globalInterceptors.stream() : Stream.empty(),
                 privateInterceptors)
                 .distinct()
+                .sorted(serverInterceptorComparator())
                 .collect(Collectors.toList());
-        interceptors.sort(AnnotationAwareOrderComparator.INSTANCE);
-        Collections.reverse(interceptors);
         return ServerInterceptors.intercept(serviceDefinition, interceptors);
     }
 
+    private Comparator<Object> serverInterceptorComparator() {
+        return AnnotationAwareOrderComparator.INSTANCE.thenComparing((o1, o2) -> {
+            boolean p1 = isOrderAnnotated(o1);
+            boolean p2 = isOrderAnnotated(o2);
+            return p1 && !p2 ? -1 : p2 && !p1 ? 1 : 0;
+        }).reversed();
+    }
+
+    private boolean isOrderAnnotated(Object obj) {
+        Order ann = obj instanceof Method ? AnnotationUtils.findAnnotation((Method) obj, Order.class) :
+                AnnotationUtils.findAnnotation(obj.getClass(), Order.class);
+        return ann != null;
+    }
 
     private void startDaemonAwaitThread() {
         Thread awaitThread = new Thread(()->{
