fix: deduplicate config listeners by identity

Fixes #88

Author: nobodyiam

CHANGES.md

@@ -10,6 +10,7 @@ Apollo Java 2.5.0
 * [Feature Added a new feature to get instance count by namespace.](https://github.com/apolloconfig/apollo-java/pull/103)
 * [Feature Support retry in open api client.](https://github.com/apolloconfig/apollo-java/pull/105)
 * [Support Spring Boot 4.0 bootstrap context package relocation for apollo-client-config-data](https://github.com/apolloconfig/apollo-java/pull/115)
+* [Fix change listener de-duplication by identity to avoid stale property names cache in Spring Cloud bootstrap dual-context initialization](https://github.com/apolloconfig/apollo-java/pull/121)
 
 ------------------
 All issues and pull requests are [here](https://github.com/apolloconfig/apollo-java/milestone/5?closed=1)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfig.java

@@ -99,7 +99,7 @@ public void addChangeListener(ConfigChangeListener listener, Set<String> interes
 
   @Override
   public void addChangeListener(ConfigChangeListener listener, Set<String> interestedKeys, Set<String> interestedKeyPrefixes) {
-    if (!m_listeners.contains(listener)) {
+    if (!containsListenerInstance(listener)) {
       m_listeners.add(listener);
       if (interestedKeys != null && !interestedKeys.isEmpty()) {
         m_interestedKeys.put(listener, Sets.newHashSet(interestedKeys));
@@ -114,7 +114,12 @@ public void addChangeListener(ConfigChangeListener listener, Set<String> interes
   public boolean removeChangeListener(ConfigChangeListener listener) {
     m_interestedKeys.remove(listener);
     m_interestedKeyPrefixes.remove(listener);
-    return m_listeners.remove(listener);
+    for (ConfigChangeListener addedListener : m_listeners) {
+      if (addedListener == listener) {
+        return m_listeners.remove(addedListener);
+      }
+    }
+    return false;
   }
 
   @Override
@@ -608,4 +613,13 @@ List<ConfigChange> calcPropertyChanges(String appId, String namespace, Propertie
 
     return changes;
   }
+
+  private boolean containsListenerInstance(ConfigChangeListener listener) {
+    for (ConfigChangeListener configChangeListener : m_listeners) {
+      if (configChangeListener == listener) {
+        return true;
+      }
+    }
+    return false;
+  }
 }

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/AbstractConfigTest.java

@@ -17,6 +17,8 @@
 package com.ctrip.framework.apollo.internals;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.spy;
@@ -28,6 +30,7 @@
 import com.ctrip.framework.apollo.enums.PropertyChangeType;
 import com.ctrip.framework.apollo.model.ConfigChange;
 import com.ctrip.framework.apollo.model.ConfigChangeEvent;
+import com.ctrip.framework.apollo.spring.config.CachedCompositePropertySource;
 import com.google.common.util.concurrent.SettableFuture;
 import java.util.Collections;
 import java.util.HashMap;
@@ -122,6 +125,36 @@ public void onChange(ConfigChangeEvent changeEvent) {
     verify(configChangeListener2, times(1)).onChange(eq(configChangeEvent));
   }
 
+  @Test
+  public void testFireConfigChange_twoCachedCompositePropertySourcesWithSameName_shouldBothBeNotified()
+      throws InterruptedException {
+    AbstractConfig abstractConfig = new ErrorConfig();
+    final String namespace = "app-namespace-listener-equals";
+    final String key = "great-key";
+
+    final CountingCachedCompositePropertySource listener1 =
+        new CountingCachedCompositePropertySource("ApolloBootstrapPropertySources");
+    final CountingCachedCompositePropertySource listener2 =
+        new CountingCachedCompositePropertySource("ApolloBootstrapPropertySources");
+
+    assertNotSame(listener1, listener2);
+    assertTrue(listener1.equals(listener2));
+
+    abstractConfig.addChangeListener(listener1, Collections.singleton(key));
+    abstractConfig.addChangeListener(listener2, Collections.singleton(key));
+
+    Map<String, ConfigChange> changes = new HashMap<>();
+    changes.put(key,
+        new ConfigChange(someAppId, namespace, key, "old-value", "new-value", PropertyChangeType.MODIFIED));
+
+    abstractConfig.fireConfigChange(someAppId, namespace, changes);
+
+    Thread.sleep(100);
+
+    assertEquals(1, listener1.changeCount.get());
+    assertEquals(1, listener2.changeCount.get());
+  }
+
   @Test
   public void testFireConfigChange_changes_notify_once()
       throws ExecutionException, InterruptedException, TimeoutException {
@@ -188,4 +221,18 @@ public ConfigSourceType getSourceType() {
       throw new UnsupportedOperationException();
     }
   }
-}
+
+  private static class CountingCachedCompositePropertySource extends CachedCompositePropertySource {
+    private final AtomicInteger changeCount = new AtomicInteger();
+
+    private CountingCachedCompositePropertySource(String name) {
+      super(name);
+    }
+
+    @Override
+    public void onChange(ConfigChangeEvent changeEvent) {
+      changeCount.incrementAndGet();
+      super.onChange(changeEvent);
+    }
+  }
+}