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 2020/02/07 08:04:37 UTC

[servicecomb-java-chassis] branch master updated: [SCB-1745]remove accessor modifier in reflection

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/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new 3227971  [SCB-1745]remove accessor modifier in reflection
3227971 is described below

commit 32279718147baf17456a2755bf4546d283cb3b6e
Author: liubao <bi...@qq.com>
AuthorDate: Thu Feb 6 16:39:08 2020 +0800

    [SCB-1745]remove accessor modifier in reflection
---
 .../common/rest/TestAbstractRestInvocation.java    | 40 ++++++++++++------
 .../org/apache/servicecomb/core/SCBEngine.java     | 38 ++++++++++-------
 .../config/client/TestConfigCenterClient.java      | 30 ++++++++-----
 .../foundation/common/event/SimpleSubscriber.java  | 20 ++++++++-
 .../common/utils/LambdaMetafactoryUtils.java       | 49 ++++++++--------------
 .../foundation/common/event/TestEventBus.java      | 25 +++++++----
 .../foundation/common/event/TestEventManager.java  | 37 ++++++++++++++--
 .../common/utils/TestLambdaMetafactoryUtils.java   | 19 +++++----
 .../internal/schema/scalar/TestEnumSchema.java     |  2 +-
 .../event/TestCircutBreakerEventNotifier.java      | 20 ++++++---
 .../loadbalance/TestLoadBalanceHandler2.java       | 23 ++++++----
 .../registry/AbstractServiceRegistry.java          |  2 +-
 12 files changed, 200 insertions(+), 105 deletions(-)

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 1ed6057..8dbe126 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
@@ -477,15 +477,23 @@ public class TestAbstractRestInvocation {
     Assert.assertSame(ProduceProcessorManager.PLAIN_PROCESSOR, restInvocation.produceProcessor);
   }
 
+  public static class SendResponseQuietlyNormalEventHandler {
+    private Holder<InvocationFinishEvent> eventHolder;
+
+    public SendResponseQuietlyNormalEventHandler(Holder<InvocationFinishEvent> eventHolder) {
+      this.eventHolder = eventHolder;
+    }
+
+    @Subscribe
+    public void onFinished(InvocationFinishEvent event) {
+      eventHolder.value = event;
+    }
+  }
+
   @Test
   public void sendResponseQuietlyNormal(@Mocked Response response) {
     Holder<InvocationFinishEvent> eventHolder = new Holder<>();
-    Object subscriber = new Object() {
-      @Subscribe
-      public void onFinished(InvocationFinishEvent event) {
-        eventHolder.value = event;
-      }
-    };
+    SendResponseQuietlyNormalEventHandler subscriber = new SendResponseQuietlyNormalEventHandler(eventHolder);
     EventManager.register(subscriber);
 
     Holder<Response> result = new Holder<>();
@@ -950,15 +958,23 @@ public class TestAbstractRestInvocation {
     Assert.assertSame(rejectedExecutionException, holder.value);
   }
 
+  public static class ScheduleInvocationEventHandler {
+    private Holder<InvocationStartEvent> eventHolder;
+
+    public ScheduleInvocationEventHandler(Holder<InvocationStartEvent> eventHolder) {
+      this.eventHolder = eventHolder;
+    }
+
+    @Subscribe
+    public void onFinished(InvocationStartEvent event) {
+      eventHolder.value = event;
+    }
+  }
+
   @Test
   public void scheduleInvocationNormal(@Mocked OperationMeta operationMeta) {
     Holder<InvocationStartEvent> eventHolder = new Holder<>();
-    Object subscriber = new Object() {
-      @Subscribe
-      public void onStart(InvocationStartEvent event) {
-        eventHolder.value = event;
-      }
-    };
+    Object subscriber = new ScheduleInvocationEventHandler(eventHolder);
     EventManager.register(subscriber);
 
     Executor executor = new ReactiveExecutor();
diff --git a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
index 674f9b1..915610b 100644
--- a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
+++ b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
@@ -276,21 +276,7 @@ public class SCBEngine {
    * <p>Check if {@code InstanceId} is null to judge whether the instance registry has succeeded.</p>
    */
   private void triggerAfterRegistryEvent() {
-    eventBus.register(new Object() {
-      @Subscribe
-      @EnableExceptionPropagation
-      public void afterRegistryInstance(MicroserviceInstanceRegisterTask microserviceInstanceRegisterTask) {
-        LOGGER.info("receive MicroserviceInstanceRegisterTask event, check instance Id...");
-
-        if (!StringUtils.isEmpty(RegistryUtils.getMicroserviceInstance().getInstanceId())) {
-          LOGGER.info("instance registry succeeds for the first time, will send AFTER_REGISTRY event.");
-          status = SCBStatus.UP;
-          triggerEvent(EventType.AFTER_REGISTRY);
-          EventManager.unregister(this);
-          LOGGER.warn("ServiceComb is ready.");
-        }
-      }
-    });
+    eventBus.register(new AfterRegistryEventHanlder(this));
   }
 
   @AllowConcurrentEvents
@@ -544,4 +530,26 @@ public class SCBEngine {
   public MicroserviceNameParser parseMicroserviceName(String microserviceName) {
     return new MicroserviceNameParser(getAppId(), microserviceName);
   }
+
+  public static class AfterRegistryEventHanlder {
+    private SCBEngine engine;
+
+    public AfterRegistryEventHanlder(SCBEngine engine) {
+      this.engine = engine;
+    }
+
+    @Subscribe
+    @EnableExceptionPropagation
+    public void afterRegistryInstance(MicroserviceInstanceRegisterTask microserviceInstanceRegisterTask) {
+      LOGGER.info("receive MicroserviceInstanceRegisterTask event, check instance Id...");
+
+      if (!StringUtils.isEmpty(RegistryUtils.getMicroserviceInstance().getInstanceId())) {
+        LOGGER.info("instance registry succeeds for the first time, will send AFTER_REGISTRY event.");
+        engine.setStatus(SCBStatus.UP);
+        engine.triggerEvent(EventType.AFTER_REGISTRY);
+        EventManager.unregister(this);
+        LOGGER.warn("ServiceComb is ready.");
+      }
+    }
+  }
 }
diff --git a/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestConfigCenterClient.java b/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestConfigCenterClient.java
index d6fad3a..46ec9b4 100644
--- a/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestConfigCenterClient.java
+++ b/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestConfigCenterClient.java
@@ -256,23 +256,31 @@ public class TestConfigCenterClient {
     refresh.run();
   }
 
+  public static class ConfigRefreshExceptionEvent {
+    Map<String, String> map;
+
+    public ConfigRefreshExceptionEvent(Map<String, String> map) {
+      this.map = map;
+    }
+
+    @Subscribe
+    public void testMsg(Object event) {
+      if (event instanceof ConnFailEvent) {
+        map.put("result", "Fail event trigger");
+      }
+      if (event instanceof ConnSuccEvent) {
+        map.put("result", "Succ event trigger");
+      }
+    }
+  }
+
   @SuppressWarnings("unchecked")
   @Test
   public void testConfigRefreshException(@Mocked ClientPoolManager<HttpClientWithContext> clientMgr,
       @Mocked HttpClientWithContext httpClientWithContext) {
     ConfigCenterConfigurationSourceImpl impl = new ConfigCenterConfigurationSourceImpl();
     Map<String, String> map = new HashMap<>();
-    EventManager.register(new Object() {
-      @Subscribe
-      public void testMsg(Object event) {
-        if (event instanceof ConnFailEvent) {
-          map.put("result", "Fail event trigger");
-        }
-        if (event instanceof ConnSuccEvent) {
-          map.put("result", "Succ event trigger");
-        }
-      }
-    });
+    EventManager.register(new ConfigRefreshExceptionEvent(map));
     UpdateHandler updateHandler = impl.new UpdateHandler();
     HttpClientRequest request = Mockito.mock(HttpClientRequest.class);
     Mockito.when(request.headers()).thenReturn(MultiMap.caseInsensitiveMultiMap());
diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java
index 9c5e146..2ff6998 100644
--- a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java
+++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java
@@ -17,6 +17,7 @@
 package org.apache.servicecomb.foundation.common.event;
 
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.function.Consumer;
 
 import org.apache.servicecomb.foundation.common.utils.LambdaMetafactoryUtils;
@@ -51,18 +52,20 @@ public class SimpleSubscriber {
       order = subscriberOrder.value();
     }
 
-    method.setAccessible(true);
     try {
       lambda = LambdaMetafactoryUtils.createLambda(instance, method, Consumer.class);
     } catch (Throwable throwable) {
       // because enhance LambdaMetafactoryUtils to support ALL_MODES by reflect
       // never run into this branch.
       // otherwise create a listener instance of anonymous class will run into this branch
-      LOGGER.warn("Failed to create lambda for method: {}, fallback to reflect.", method);
+      LOGGER.warn("Failed to create lambda for method: {}, fallback to reflect.", method, throwable);
+
+      checkAccess(method);
       lambda = event -> {
         try {
           method.invoke(instance, event);
         } catch (Throwable e) {
+          LOGGER.warn("Failed to call event listener {}.", method.getName());
           throw new IllegalStateException(e);
         }
       };
@@ -74,6 +77,19 @@ public class SimpleSubscriber {
     }
   }
 
+  private static void checkAccess(Method method) {
+    // This check is not accurate. Most of time package visible and protected access can be ignored, so simply do this.
+    if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
+      throw new IllegalStateException(
+          String.format(
+              "The event handler must be a public accessible method. NOTICE: "
+                  + "this is change from 2.0 and using higher version of JDK. "
+                  + "Declaring class is %s, method is %s",
+              method.getDeclaringClass().getName(),
+              method.getName()));
+    }
+  }
+
   public Object getInstance() {
     return instance;
   }
diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/LambdaMetafactoryUtils.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/LambdaMetafactoryUtils.java
index da690b5..d88cbdc 100644
--- a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/LambdaMetafactoryUtils.java
+++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/LambdaMetafactoryUtils.java
@@ -48,11 +48,6 @@ import org.apache.servicecomb.foundation.common.utils.bean.ShortGetter;
 import org.apache.servicecomb.foundation.common.utils.bean.ShortSetter;
 
 public final class LambdaMetafactoryUtils {
-  private static Field allowedModesField;
-
-  private static final int ALL_MODES = (MethodHandles.Lookup.PRIVATE | MethodHandles.Lookup.PROTECTED
-      | MethodHandles.Lookup.PACKAGE | MethodHandles.Lookup.PUBLIC);
-
   private static final Lookup LOOKUP = MethodHandles.lookup();
 
   private static final Map<Class<?>, Class<?>> GETTER_MAP = new HashMap<>();
@@ -60,7 +55,6 @@ public final class LambdaMetafactoryUtils {
   private static final Map<Class<?>, Class<?>> SETTER_MAP = new HashMap<>();
 
   static {
-    enhanceLambda();
     initGetterSetterMap();
   }
 
@@ -84,20 +78,6 @@ public final class LambdaMetafactoryUtils {
     SETTER_MAP.put(double.class, DoubleSetter.class);
   }
 
-  private static void enhanceLambda() {
-    try {
-      Field modifiersField = Field.class.getDeclaredField("modifiers");
-      modifiersField.setAccessible(true);
-
-      allowedModesField = Lookup.class.getDeclaredField("allowedModes");
-      allowedModesField.setAccessible(true);
-      int modifiers = allowedModesField.getModifiers();
-      modifiersField.setInt(allowedModesField, modifiers & ~Modifier.FINAL);
-    } catch (Throwable e) {
-      throw new IllegalStateException("Failed to init LambdaMetafactoryUtils.", e);
-    }
-  }
-
   private LambdaMetafactoryUtils() {
   }
 
@@ -114,17 +94,14 @@ public final class LambdaMetafactoryUtils {
   @SuppressWarnings("unchecked")
   public static <T> T createLambda(Object instance, Method instanceMethod, Class<?> functionalIntfCls) {
     try {
-      Lookup lookup = LOOKUP.in(instanceMethod.getDeclaringClass());
-      allowedModesField.set(lookup, ALL_MODES);
-
       Method intfMethod = findAbstractMethod(functionalIntfCls);
-      MethodHandle methodHandle = lookup.unreflect(instanceMethod);
+      MethodHandle methodHandle = LOOKUP.unreflect(instanceMethod);
 
       MethodType intfMethodType = MethodType.methodType(intfMethod.getReturnType(), intfMethod.getParameterTypes());
       MethodType instanceMethodType = MethodType
           .methodType(instanceMethod.getReturnType(), instanceMethod.getParameterTypes());
       CallSite callSite = LambdaMetafactory.metafactory(
-          lookup,
+          LOOKUP,
           intfMethod.getName(),
           MethodType.methodType(functionalIntfCls, instance.getClass()),
           intfMethodType,
@@ -144,16 +121,13 @@ public final class LambdaMetafactoryUtils {
       return null;
     }
     try {
-      Lookup lookup = LOOKUP.in(instanceMethod.getDeclaringClass());
-      allowedModesField.set(lookup, ALL_MODES);
-
       Method intfMethod = findAbstractMethod(functionalIntfCls);
-      MethodHandle methodHandle = lookup.unreflect(instanceMethod);
+      MethodHandle methodHandle = LOOKUP.unreflect(instanceMethod);
 
       MethodType intfMethodType = MethodType.methodType(intfMethod.getReturnType(), intfMethod.getParameterTypes());
       MethodType instanceMethodType = methodHandle.type();
       CallSite callSite = LambdaMetafactory.metafactory(
-          lookup,
+          LOOKUP,
           intfMethod.getName(),
           MethodType.methodType(functionalIntfCls),
           intfMethodType,
@@ -174,7 +148,7 @@ public final class LambdaMetafactoryUtils {
   // slower than reflect directly
   @SuppressWarnings("unchecked")
   public static <C, F> Getter<C, F> createGetter(Field field) {
-    field.setAccessible(true);
+    checkAccess(field);
     return instance -> {
       try {
         return (F) field.get(instance);
@@ -184,6 +158,17 @@ public final class LambdaMetafactoryUtils {
     };
   }
 
+  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."
+                  + "Declaring class is %s, field is %s",
+              field.getDeclaringClass().getName(),
+              field.getName()));
+    }
+  }
+
   public static <T> T createSetter(Method setMethod) throws Throwable {
     Class<?> setterCls = SETTER_MAP.getOrDefault(setMethod.getParameterTypes()[0], Setter.class);
     return createLambda(setMethod, setterCls);
@@ -191,7 +176,7 @@ public final class LambdaMetafactoryUtils {
 
   // slower than reflect directly
   public static <C, F> Setter<C, F> createSetter(Field field) {
-    field.setAccessible(true);
+    checkAccess(field);
     return (instance, value) -> {
       try {
         field.set(instance, value);
diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java
index 760fad7..337efda 100644
--- a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java
+++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java
@@ -32,7 +32,12 @@ public class TestEventBus {
 
   private List<Object> events = new ArrayList<>();
 
-  class SubscriberForTest {
+  public static class SubscriberForTest {
+    private List<Object> events;
+
+    public SubscriberForTest( List<Object> events) {
+      this.events = events;
+    }
     @Subscribe
     @AllowConcurrentEvents
     public void s1(Integer event) {
@@ -45,7 +50,13 @@ public class TestEventBus {
     }
   }
 
-  class SubscriberWithOrderForTest {
+  public static class SubscriberWithOrderForTest {
+    private List<Object> events;
+
+    public SubscriberWithOrderForTest( List<Object> events) {
+      this.events = events;
+    }
+
     @Subscribe
     @SubscriberOrder(100)
     public void s1(String event) {
@@ -61,7 +72,7 @@ public class TestEventBus {
 
   @Test
   public void order() {
-    eventBus.register(new SubscriberWithOrderForTest());
+    eventBus.register(new SubscriberWithOrderForTest(events));
 
     eventBus.post("value");
     Assert.assertThat(events, Matchers.contains("s2:value", "s1:value"));
@@ -69,7 +80,7 @@ public class TestEventBus {
 
   @Test
   public void unregister() {
-    Object obj = new SubscriberForTest();
+    Object obj = new SubscriberForTest(events);
 
     eventBus.register(obj);
     eventBus.unregister(obj);
@@ -77,7 +88,7 @@ public class TestEventBus {
 
   @Test
   public void oneSubscriber() {
-    Object obj = new SubscriberForTest();
+    Object obj = new SubscriberForTest(events);
 
     eventBus.register(obj);
 
@@ -97,8 +108,8 @@ public class TestEventBus {
 
   @Test
   public void twoSubscriber() {
-    Object obj1 = new SubscriberForTest();
-    Object obj2 = new SubscriberForTest();
+    Object obj1 = new SubscriberForTest(events);
+    Object obj2 = new SubscriberForTest(events);
 
     eventBus.register(obj1);
     eventBus.register(obj2);
diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventManager.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventManager.java
index bc0ae99..77df59d 100644
--- a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventManager.java
+++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventManager.java
@@ -47,7 +47,7 @@ public class TestEventManager {
     LogCollector collector = new LogCollector();
 
     test(this);
-    Assert.assertTrue(collector.getEvents().isEmpty());
+    Assert.assertTrue(collector.getEvents().isEmpty()); // ensure no warning logs
     collector.teardown();
   }
 
@@ -65,8 +65,39 @@ public class TestEventManager {
         iCount++;
       }
     };
-    test(listener);
-    Assert.assertTrue(collector.getEvents().isEmpty());
+    try {
+      test(listener);
+      Assert.fail();
+    } catch (IllegalStateException e) {
+      Assert.assertTrue(true);
+    }
+
+    collector.teardown();
+  }
+
+  @Test
+  public void anonymousListenerPublic() {
+    LogCollector collector = new LogCollector();
+    Object listener = new Object() {
+      @Subscribe
+      public void onObject(Object obj) {
+        objCount++;
+      }
+
+      @Subscribe
+      public void onInt(Integer obj) {
+        iCount++;
+      }
+    };
+    try {
+      test(listener);
+      Assert.fail();
+    } catch (IllegalStateException e) {
+      Assert.assertTrue(true);
+    }
+
+    // ensure logs: "LOGGER.warn("Failed to create lambda for method: {}, fallback to reflect.", method, throwable);"
+    Assert.assertTrue(!collector.getEvents().isEmpty());
     collector.teardown();
   }
 
diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestLambdaMetafactoryUtils.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestLambdaMetafactoryUtils.java
index 40672cb..476a097 100644
--- a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestLambdaMetafactoryUtils.java
+++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestLambdaMetafactoryUtils.java
@@ -24,10 +24,8 @@ import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
-import org.apache.servicecomb.foundation.common.utils.bean.Getter;
 import org.apache.servicecomb.foundation.common.utils.bean.IntGetter;
 import org.apache.servicecomb.foundation.common.utils.bean.IntSetter;
-import org.apache.servicecomb.foundation.common.utils.bean.Setter;
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Test;
@@ -84,10 +82,17 @@ public class TestLambdaMetafactoryUtils {
   @Test
   public void createGetterSetterByField() throws Throwable {
     Field f1 = Model.class.getDeclaredField("f1");
-    Getter<Model, Integer> getter = LambdaMetafactoryUtils.createGetter(f1);
-    Setter<Model, Integer> setter = LambdaMetafactoryUtils.createSetter(f1);
-
-    setter.set(model, 1);
-    Assert.assertEquals(1, (int) getter.get(model));
+    try {
+      LambdaMetafactoryUtils.createGetter(f1);
+      Assert.fail();
+    } catch (IllegalStateException e) {
+      Assert.assertTrue(true);
+    }
+    try {
+      LambdaMetafactoryUtils.createSetter(f1);
+      Assert.fail();
+    } catch (IllegalStateException e) {
+      Assert.assertTrue(true);
+    }
   }
 }
diff --git a/foundations/foundation-protobuf/src/test/java/org/apache/servicecomb/foundation/protobuf/internal/schema/scalar/TestEnumSchema.java b/foundations/foundation-protobuf/src/test/java/org/apache/servicecomb/foundation/protobuf/internal/schema/scalar/TestEnumSchema.java
index 7379403..bba3d61 100644
--- a/foundations/foundation-protobuf/src/test/java/org/apache/servicecomb/foundation/protobuf/internal/schema/scalar/TestEnumSchema.java
+++ b/foundations/foundation-protobuf/src/test/java/org/apache/servicecomb/foundation/protobuf/internal/schema/scalar/TestEnumSchema.java
@@ -38,7 +38,7 @@ public class TestEnumSchema extends TestSchemaBase {
     Assert.assertEquals(0, rootSerializer.serialize(scbMap).length);
   }
 
-  private static class EnumRoot {
+  public static class EnumRoot {
     private int color;
 
     public int getColor() {
diff --git a/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/event/TestCircutBreakerEventNotifier.java b/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/event/TestCircutBreakerEventNotifier.java
index 3f58ca7..9bcb4a8 100644
--- a/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/event/TestCircutBreakerEventNotifier.java
+++ b/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/event/TestCircutBreakerEventNotifier.java
@@ -43,17 +43,25 @@ public class TestCircutBreakerEventNotifier {
 
   Object receiveEvent = null;
 
+  public static class EventSubscriber {
+    List<AlarmEvent> taskList;
+
+    public EventSubscriber(List<AlarmEvent> taskList) {
+      this.taskList = taskList;
+    }
+
+    @Subscribe
+    public void onEvent(AlarmEvent circutBreakerEvent) {
+      taskList.add(circutBreakerEvent);
+    }
+  }
+
   @Before
   public void setUp() throws Exception {
     taskList = new ArrayList<>();
     circutBreakerEventNotifier = new CircutBreakerEventNotifier();
     commandKey = Mockito.mock(HystrixCommandKey.class);
-    receiveEvent = new Object() {
-      @Subscribe
-      public void onEvent(AlarmEvent circutBreakerEvent) {
-        taskList.add(circutBreakerEvent);
-      }
-    };
+    receiveEvent = new EventSubscriber(taskList);
     circutBreakerEventNotifier.eventBus.register(receiveEvent);
   }
 
diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java
index f7f3563..54d1792 100644
--- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java
+++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java
@@ -20,7 +20,6 @@ package org.apache.servicecomb.loadbalance;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
-import java.util.EventListener;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -241,6 +240,20 @@ public class TestLoadBalanceHandler2 {
     Assert.assertEquals("rest://localhost:9091", server.getEndpoint().getEndpoint());
   }
 
+  public class IsolationEndpointListener {
+    Holder<Integer> count;
+
+    public IsolationEndpointListener(Holder<Integer> count) {
+      this.count = count;
+    }
+
+    @Subscribe
+    public void listener(IsolationServerEvent event) {
+      count.value++;
+      Assert.assertSame("Isolation Endpoint", "rest://localhost:9090", event.getEndpoint().getEndpoint());
+    }
+  }
+
   @Test
   public void testIsolationEventWithEndpoint() throws Exception {
     ReferenceConfig referenceConfig = Mockito.mock(ReferenceConfig.class);
@@ -318,13 +331,7 @@ public class TestLoadBalanceHandler2 {
     ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.minIsolationTime", "10");
 
     Holder<Integer> count = new Holder<>(0);
-    EventListener isolationEndpointListener = new EventListener() {
-      @Subscribe
-      public void listener(IsolationServerEvent event) {
-        count.value++;
-        Assert.assertSame("Isolation Endpoint", "rest://localhost:9090", event.getEndpoint().getEndpoint());
-      }
-    };
+    IsolationEndpointListener isolationEndpointListener = new IsolationEndpointListener(count);
     EventManager.getEventBus().register(isolationEndpointListener);
     Assert.assertEquals(0, count.value.intValue());
     loadBalancer = handler.getOrCreateLoadBalancer(invocation);
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java
index 4f42750..70cd5a0 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java
@@ -344,7 +344,7 @@ public abstract class AbstractServiceRegistry implements ServiceRegistry {
 
   // post from watch eventloop, should refresh the exact microservice instances immediately
   @Subscribe
-  private void onMicroserviceInstanceChanged(MicroserviceInstanceChangedEvent changedEvent) {
+  public void onMicroserviceInstanceChanged(MicroserviceInstanceChangedEvent changedEvent) {
     executorService.execute(() -> appManager.onMicroserviceInstanceChanged(changedEvent));
   }