diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java b/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java index 824cd1a54..c0bb70726 100644 --- a/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java +++ b/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java @@ -159,6 +159,17 @@ if (!failoverReactor.isFailoverSwitch(serviceKey)) { return null == serviceInfo.getHosts() || (pushEmptyProtection && !serviceInfo.validate()); } + /** + * isChangedServiceInfo. + * + * @param oldService old service data + * @param newService new service data + * @return {@code true} if oldService is not equal newService, {@code false} otherwise. + */ + public boolean isChangedServiceInfo(ServiceInfo oldService, ServiceInfo newService) { + return getServiceInfoDiff(oldService, newService).hasDifferent(); + } + private InstancesDiff getServiceInfoDiff(ServiceInfo oldService, ServiceInfo newService) { InstancesDiff instancesDiff = new InstancesDiff(); if (null == oldService) { diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/event/InstancesChangeEvent.java b/client/src/main/java/com/alibaba/nacos/client/naming/event/InstancesChangeEvent.java index 4d7be6b06..f0bde6682 100644 --- a/client/src/main/java/com/alibaba/nacos/client/naming/event/InstancesChangeEvent.java +++ b/client/src/main/java/com/alibaba/nacos/client/naming/event/InstancesChangeEvent.java @@ -43,6 +43,10 @@ public class InstancesChangeEvent extends Event { private InstancesDiff instancesDiff; + public InstancesChangeEvent(String eventScope, String serviceName, String groupName, String clusters, List hosts) { + this(eventScope, serviceName, groupName, clusters, hosts, null); + } + public InstancesChangeEvent(String eventScope, String serviceName, String groupName, String clusters, List hosts, InstancesDiff diff) { this.eventScope = eventScope; this.serviceName = serviceName; diff --git a/client/src/main/java/com/alibaba/nacos/client/selector/SelectorManager.java b/client/src/main/java/com/alibaba/nacos/client/selector/SelectorManager.java index 6b4caed6c..deda8f62e 100644 --- a/client/src/main/java/com/alibaba/nacos/client/selector/SelectorManager.java +++ b/client/src/main/java/com/alibaba/nacos/client/selector/SelectorManager.java @@ -19,6 +19,7 @@ package com.alibaba.nacos.client.selector; import com.alibaba.nacos.common.utils.CollectionUtils; import com.alibaba.nacos.common.utils.ConcurrentHashSet; +import java.util.Collections; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -56,7 +57,7 @@ public class SelectorManager> { * @return the set of SelectorWrappers */ public Set getSelectorWrappers(String subId) { - return selectorMap.get(subId); + return selectorMap.getOrDefault(subId, Collections.emptySet()); } /** diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/NacosNamingServiceTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/NacosNamingServiceTest.java index ce53a3c49..7b1b0275b 100644 --- a/client/src/test/java/com/alibaba/nacos/client/naming/NacosNamingServiceTest.java +++ b/client/src/test/java/com/alibaba/nacos/client/naming/NacosNamingServiceTest.java @@ -30,6 +30,7 @@ import com.alibaba.nacos.client.naming.event.InstancesChangeEvent; import com.alibaba.nacos.client.naming.event.InstancesChangeNotifier; import com.alibaba.nacos.client.naming.remote.NamingClientProxy; import com.alibaba.nacos.client.naming.remote.http.NamingHttpClientProxy; +import com.alibaba.nacos.client.naming.selector.NamingSelectorFactory; import com.alibaba.nacos.client.naming.selector.NamingSelectorWrapper; import com.alibaba.nacos.client.naming.utils.CollectionUtils; import com.alibaba.nacos.client.naming.utils.UtilAndComs; @@ -47,11 +48,11 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import static com.alibaba.nacos.client.naming.selector.NamingSelectorFactory.getUniqueClusterString; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static com.alibaba.nacos.client.naming.selector.NamingSelectorFactory.getUniqueClusterString; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; @@ -870,8 +871,8 @@ class NacosNamingServiceTest { }; //when client.subscribe(serviceName, groupName, NamingSelectorFactory.HEALTHY_SELECTOR, listener); - NamingSelectorWrapper wrapper = new NamingSelectorWrapper(serviceName, groupName, - Constants.NULL, NamingSelectorFactory.HEALTHY_SELECTOR, listener); + NamingSelectorWrapper wrapper = new NamingSelectorWrapper(serviceName, groupName, Constants.NULL, + NamingSelectorFactory.HEALTHY_SELECTOR, listener); //then verify(changeNotifier, times(1)).registerListener(groupName, serviceName, wrapper); verify(proxy, times(1)).subscribe(serviceName, groupName, Constants.NULL); @@ -884,7 +885,8 @@ class NacosNamingServiceTest { //when client.subscribe(serviceName, groupName, null); //then - verify(changeNotifier, never()).registerListener(groupName, serviceName, "", null); + verify(changeNotifier, never()).registerListener(groupName, serviceName, + new NamingSelectorWrapper(NamingSelectorFactory.newIpSelector(""), null)); verify(proxy, never()).subscribe(serviceName, groupName, ""); } @@ -896,7 +898,7 @@ class NacosNamingServiceTest { EventListener listener = event -> { }; - when(changeNotifier.isSubscribed(serviceName, Constants.DEFAULT_GROUP)).thenReturn(false); + when(changeNotifier.isSubscribed(Constants.DEFAULT_GROUP, serviceName)).thenReturn(false); //when client.unsubscribe(serviceName, listener); //then @@ -914,7 +916,7 @@ class NacosNamingServiceTest { EventListener listener = event -> { }; - when(changeNotifier.isSubscribed(serviceName, groupName)).thenReturn(false); + when(changeNotifier.isSubscribed(groupName, serviceName)).thenReturn(false); //when client.unsubscribe(serviceName, groupName, listener); @@ -933,7 +935,7 @@ class NacosNamingServiceTest { EventListener listener = event -> { }; - when(changeNotifier.isSubscribed(serviceName, Constants.DEFAULT_GROUP)).thenReturn(false); + when(changeNotifier.isSubscribed(Constants.DEFAULT_GROUP, serviceName)).thenReturn(false); //when client.unsubscribe(serviceName, clusterList, listener); @@ -953,7 +955,7 @@ class NacosNamingServiceTest { EventListener listener = event -> { }; - when(changeNotifier.isSubscribed(serviceName, groupName)).thenReturn(false); + when(changeNotifier.isSubscribed(groupName, serviceName)).thenReturn(false); //when client.unsubscribe(serviceName, groupName, clusterList, listener); @@ -972,12 +974,11 @@ class NacosNamingServiceTest { EventListener listener = event -> { }; - when(changeNotifier.isSubscribed(serviceName, groupName)).thenReturn(false); - + when(changeNotifier.isSubscribed(groupName, serviceName)).thenReturn(false); + //when client.unsubscribe(serviceName, groupName, NamingSelectorFactory.HEALTHY_SELECTOR, listener); - NamingSelectorWrapper wrapper = new NamingSelectorWrapper(NamingSelectorFactory.HEALTHY_SELECTOR, - listener); + NamingSelectorWrapper wrapper = new NamingSelectorWrapper(NamingSelectorFactory.HEALTHY_SELECTOR, listener); //then verify(changeNotifier, times(1)).deregisterListener(groupName, serviceName, wrapper); verify(proxy, times(1)).unsubscribe(serviceName, groupName, Constants.NULL); diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeEventTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeEventTest.java index 2038b6bcb..e6e7cd3bd 100644 --- a/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeEventTest.java +++ b/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeEventTest.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class InstancesChangeEventTest { @@ -47,10 +48,10 @@ class InstancesChangeEventTest { assertEquals(hosts.size(), hosts1.size()); assertEquals(hosts.get(0), hosts1.get(0)); InstancesDiff diff1 = event.getInstancesDiff(); - Assert.assertTrue(diff1.hasDifferent()); - Assert.assertEquals(diff.getAddedInstances().size(), diff1.getAddedInstances().size()); - Assert.assertEquals(diff.getAddedInstances().get(0), diff.getAddedInstances().get(0)); - Assert.assertEquals(diff.getRemovedInstances().size(), diff1.getRemovedInstances().size()); - Assert.assertEquals(diff.getModifiedInstances().size(), diff1.getModifiedInstances().size()); + assertTrue(diff1.hasDifferent()); + assertEquals(diff.getAddedInstances().size(), diff1.getAddedInstances().size()); + assertEquals(diff.getAddedInstances().get(0), diff.getAddedInstances().get(0)); + assertEquals(diff.getRemovedInstances().size(), diff1.getRemovedInstances().size()); + assertEquals(diff.getModifiedInstances().size(), diff1.getModifiedInstances().size()); } } \ No newline at end of file diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeNotifierTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeNotifierTest.java index 19b70cd1f..4509e71e9 100644 --- a/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeNotifierTest.java +++ b/client/src/test/java/com/alibaba/nacos/client/naming/event/InstancesChangeNotifierTest.java @@ -21,8 +21,10 @@ import com.alibaba.nacos.api.naming.listener.EventListener; import com.alibaba.nacos.api.naming.pojo.Instance; import com.alibaba.nacos.api.naming.pojo.ServiceInfo; import com.alibaba.nacos.api.naming.selector.NamingSelector; -import com.alibaba.nacos.client.naming.selector.NamingSelectorWrapper; +import com.alibaba.nacos.client.naming.selector.DefaultNamingSelector; import com.alibaba.nacos.client.naming.selector.NamingSelectorFactory; +import com.alibaba.nacos.client.naming.selector.NamingSelectorWrapper; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -33,6 +35,7 @@ import java.util.concurrent.Executor; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -42,138 +45,123 @@ import static org.mockito.Mockito.when; class InstancesChangeNotifierTest { + private static final String EVENT_SCOPE_CASE = "scope-001"; + + private static final String GROUP_CASE = "a"; + + private static final String SERVICE_NAME_CASE = "b"; + + private static final String CLUSTER_STR_CASE = "c"; + + InstancesChangeNotifier instancesChangeNotifier; + + @BeforeEach + public void setUp() { + instancesChangeNotifier = new InstancesChangeNotifier(EVENT_SCOPE_CASE); + } + @Test void testRegisterListener() { - String eventScope = "scope-001"; - String group = "a"; - String name = "b"; - String clusterStr = "c"; - List clusters = Collections.singletonList(clusterStr); - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); + List clusters = Collections.singletonList(CLUSTER_STR_CASE); EventListener listener = Mockito.mock(EventListener.class); NamingSelector selector = NamingSelectorFactory.newClusterSelector(clusters); - NamingSelectorWrapper wrapper = new NamingSelectorWrapper(name, group, clusterStr, selector, listener); - instancesChangeNotifier.registerListener(group, name, wrapper); + NamingSelectorWrapper wrapper = new NamingSelectorWrapper(SERVICE_NAME_CASE, GROUP_CASE, CLUSTER_STR_CASE, + selector, listener); + instancesChangeNotifier.registerListener(GROUP_CASE, SERVICE_NAME_CASE, wrapper); List subscribeServices = instancesChangeNotifier.getSubscribeServices(); assertEquals(1, subscribeServices.size()); - assertEquals(group, subscribeServices.get(0).getGroupName()); - assertEquals(name, subscribeServices.get(0).getName()); - assertEquals(clusters, subscribeServices.get(0).getClusters()); + assertEquals(GROUP_CASE, subscribeServices.get(0).getGroupName()); + assertEquals(SERVICE_NAME_CASE, subscribeServices.get(0).getName()); + assertNull(subscribeServices.get(0).getClusters()); List hosts = new ArrayList<>(); Instance ins = new Instance(); hosts.add(ins); InstancesDiff diff = new InstancesDiff(); diff.setAddedInstances(hosts); - InstancesChangeEvent event = new InstancesChangeEvent(eventScope, name, group, clusterStr, hosts, diff); + InstancesChangeEvent event = new InstancesChangeEvent(EVENT_SCOPE_CASE, SERVICE_NAME_CASE, GROUP_CASE, + CLUSTER_STR_CASE, hosts, diff); assertTrue(instancesChangeNotifier.scopeMatches(event)); } @Test void testDeregisterListener() { - String eventScope = "scope-001"; - String group = "a"; - String name = "b"; - String clusterStr = "c"; - List clusters = Collections.singletonList(clusterStr); - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); + List clusters = Collections.singletonList(CLUSTER_STR_CASE); EventListener listener = Mockito.mock(EventListener.class); NamingSelector selector = NamingSelectorFactory.newClusterSelector(clusters); NamingSelectorWrapper wrapper = new NamingSelectorWrapper(selector, listener); - instancesChangeNotifier.registerListener(group, name, wrapper); + instancesChangeNotifier.registerListener(GROUP_CASE, SERVICE_NAME_CASE, wrapper); List subscribeServices = instancesChangeNotifier.getSubscribeServices(); assertEquals(1, subscribeServices.size()); - instancesChangeNotifier.deregisterListener(group, name, wrapper); + instancesChangeNotifier.deregisterListener(GROUP_CASE, SERVICE_NAME_CASE, wrapper); List subscribeServices2 = instancesChangeNotifier.getSubscribeServices(); assertEquals(0, subscribeServices2.size()); - - instancesChangeNotifier.deregisterListener(group, name, clusters, listener); - assertEquals(0, subscribeServices2.size()); } @Test void testIsSubscribed() { - String eventScope = "scope-001"; - String group = "a"; - String name = "b"; - String clusterStr = "c"; - List clusters = Collections.singletonList(clusterStr); - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); + List clusters = Collections.singletonList(CLUSTER_STR_CASE); EventListener listener = Mockito.mock(EventListener.class); NamingSelector selector = NamingSelectorFactory.newClusterSelector(clusters); - assertFalse(instancesChangeNotifier.isSubscribed(group, name)); + assertFalse(instancesChangeNotifier.isSubscribed(GROUP_CASE, SERVICE_NAME_CASE)); - NamingSelectorWrapper wrapper = new NamingSelectorWrapper(name, group, clusterStr, selector, listener); - instancesChangeNotifier.registerListener(group, name, wrapper); - assertTrue(instancesChangeNotifier.isSubscribed(group, name)); + NamingSelectorWrapper wrapper = new NamingSelectorWrapper(SERVICE_NAME_CASE, GROUP_CASE, CLUSTER_STR_CASE, + selector, listener); + instancesChangeNotifier.registerListener(GROUP_CASE, SERVICE_NAME_CASE, wrapper); + assertTrue(instancesChangeNotifier.isSubscribed(GROUP_CASE, SERVICE_NAME_CASE)); } @Test void testOnEvent() { - String eventScope = "scope-001"; - String group = "a"; - String name = "b"; - String clusterStr = "c"; - List clusters = Collections.singletonList(clusterStr); - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); + List clusters = Collections.singletonList(CLUSTER_STR_CASE); NamingSelector selector = NamingSelectorFactory.newClusterSelector(clusters); EventListener listener = Mockito.mock(EventListener.class); - NamingSelectorWrapper wrapper = new NamingSelectorWrapper(name, group, clusterStr, selector, listener); - instancesChangeNotifier.registerListener(group, name, wrapper); + NamingSelectorWrapper wrapper = new NamingSelectorWrapper(SERVICE_NAME_CASE, GROUP_CASE, CLUSTER_STR_CASE, + selector, listener); + instancesChangeNotifier.registerListener(GROUP_CASE, SERVICE_NAME_CASE, wrapper); Instance instance = new Instance(); InstancesDiff diff = new InstancesDiff(null, Collections.singletonList(instance), null); - instance.setClusterName("c"); - InstancesChangeEvent event1 = new InstancesChangeEvent(null, name, group, clusterStr, Collections.emptyList(), - diff); + instance.setClusterName(CLUSTER_STR_CASE); + InstancesChangeEvent event1 = new InstancesChangeEvent(null, SERVICE_NAME_CASE, GROUP_CASE, CLUSTER_STR_CASE, + Collections.emptyList(), diff); instancesChangeNotifier.onEvent(event1); Mockito.verify(listener, times(1)).onEvent(any()); } @Test void testOnEventWithoutListener() { - String eventScope = "scope-001"; - String group = "a"; - String name = "b"; - String clusters = "c"; InstancesChangeEvent event1 = Mockito.mock(InstancesChangeEvent.class); - when(event1.getClusters()).thenReturn(clusters); - when(event1.getGroupName()).thenReturn(group); - when(event1.getServiceName()).thenReturn(name); + when(event1.getClusters()).thenReturn(CLUSTER_STR_CASE); + when(event1.getGroupName()).thenReturn(GROUP_CASE); + when(event1.getServiceName()).thenReturn(SERVICE_NAME_CASE); EventListener listener = Mockito.mock(EventListener.class); - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); - instancesChangeNotifier.registerListener(group, name + "c", clusters, listener); + instancesChangeNotifier.registerListener(GROUP_CASE, SERVICE_NAME_CASE + "c", new NamingSelectorWrapper( + NamingSelectorFactory.newClusterSelector(Collections.singletonList(CLUSTER_STR_CASE)), listener)); instancesChangeNotifier.onEvent(event1); Mockito.verify(listener, never()).onEvent(any()); } @Test void testOnEventByExecutor() { - String eventScope = "scope-001"; - String group = "a"; - String name = "b"; - String clusters = "c"; - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); AbstractEventListener listener = Mockito.mock(AbstractEventListener.class); Executor executor = mock(Executor.class); when(listener.getExecutor()).thenReturn(executor); - instancesChangeNotifier.registerListener(group, name, clusters, listener); - InstancesChangeEvent event1 = Mockito.mock(InstancesChangeEvent.class); - when(event1.getClusters()).thenReturn(clusters); - when(event1.getGroupName()).thenReturn(group); - when(event1.getServiceName()).thenReturn(name); - - instancesChangeNotifier.onEvent(event1); + instancesChangeNotifier.registerListener(GROUP_CASE, SERVICE_NAME_CASE, + new NamingSelectorWrapper(new DefaultNamingSelector(instance -> true), listener)); + InstancesDiff instancesDiff = new InstancesDiff(); + instancesDiff.setRemovedInstances(Collections.singletonList(new Instance())); + InstancesChangeEvent event = new InstancesChangeEvent(EVENT_SCOPE_CASE, SERVICE_NAME_CASE, GROUP_CASE, + CLUSTER_STR_CASE, new ArrayList<>(), instancesDiff); + instancesChangeNotifier.onEvent(event); Mockito.verify(executor).execute(any()); } @Test void testSubscribeType() { - String eventScope = "scope-001"; - InstancesChangeNotifier instancesChangeNotifier = new InstancesChangeNotifier(eventScope); assertEquals(InstancesChangeEvent.class, instancesChangeNotifier.subscribeType()); } } \ No newline at end of file diff --git a/client/src/test/java/com/alibaba/nacos/client/selector/SelectorManagerTest.java b/client/src/test/java/com/alibaba/nacos/client/selector/SelectorManagerTest.java index b3cbace49..dd15a7ca7 100644 --- a/client/src/test/java/com/alibaba/nacos/client/selector/SelectorManagerTest.java +++ b/client/src/test/java/com/alibaba/nacos/client/selector/SelectorManagerTest.java @@ -25,7 +25,6 @@ import java.util.Random; import java.util.Set; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; @@ -39,7 +38,7 @@ public class SelectorManagerTest { selectorManager.addSelectorWrapper(subId, sw); assertTrue(selectorManager.getSelectorWrappers(subId).contains(sw)); selectorManager.removeSelectorWrapper(subId, sw); - assertNull(selectorManager.getSelectorWrappers(subId)); + assertTrue(selectorManager.getSelectorWrappers(subId).isEmpty()); } @Test