Merge pull request #1013 from nobodyiam/fix-auto-update-with-deleted-keys

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/property/AutoUpdateConfigChangeListener.java

@@ -2,6 +2,7 @@ package com.ctrip.framework.apollo.spring.property;
 
 import com.ctrip.framework.apollo.ConfigChangeListener;
 import com.ctrip.framework.apollo.build.ApolloInjector;
+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.annotation.SpringValueProcessor;
@@ -57,8 +58,7 @@ public class AutoUpdateConfigChangeListener implements ConfigChangeListener{
       }
 
       // 2. check whether the value is really changed or not (since spring property sources have hierarchies)
-      ConfigChange configChange = changeEvent.getChange(key);
-      if (!Objects.equals(environment.getProperty(key), configChange.getNewValue())) {
+      if (!shouldTriggerAutoUpdate(changeEvent, key)) {
         continue;
       }
 
@@ -69,6 +69,23 @@ public class AutoUpdateConfigChangeListener implements ConfigChangeListener{
     }
   }
 
+  /**
+   * Check whether we should trigger the auto update or not.
+   * <br />
+   * For added or modified keys, we should trigger auto update if the current value in Spring equals to the new value.
+   * <br />
+   * For deleted keys, we will trigger auto update anyway.
+   */
+  private boolean shouldTriggerAutoUpdate(ConfigChangeEvent changeEvent, String changedKey) {
+    ConfigChange configChange = changeEvent.getChange(changedKey);
+
+    if (configChange.getChangeType() == PropertyChangeType.DELETED) {
+      return true;
+    }
+
+    return Objects.equals(environment.getProperty(changedKey), configChange.getNewValue());
+  }
+
   private void updateSpringValue(SpringValue springValue) {
     try {
       Object value = resolvePropertyValue(springValue);

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java

@@ -302,6 +302,36 @@ public class JavaConfigPlaceholderAutoUpdateTest extends AbstractSpringIntegrati
   }
 
   @Test
+  public void testAutoUpdateWithMultipleNamespacesWithSamePropertiesDeleted() throws Exception {
+    int someTimeout = 1000;
+    int someBatch = 2000;
+    int anotherBatch = 3000;
+
+    Properties applicationProperties = assembleProperties(BATCH_PROPERTY, String.valueOf(someBatch));
+    Properties fxApolloProperties =
+        assembleProperties(TIMEOUT_PROPERTY, String.valueOf(someTimeout), BATCH_PROPERTY, String.valueOf(anotherBatch));
+
+    SimpleConfig applicationConfig = prepareConfig(ConfigConsts.NAMESPACE_APPLICATION, applicationProperties);
+    SimpleConfig fxApolloConfig = prepareConfig(FX_APOLLO_NAMESPACE, fxApolloProperties);
+
+    AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(AppConfig2.class);
+
+    TestJavaConfigBean bean = context.getBean(TestJavaConfigBean.class);
+
+    assertEquals(someTimeout, bean.getTimeout());
+    assertEquals(someBatch, bean.getBatch());
+
+    Properties newProperties = new Properties();
+
+    applicationConfig.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);
+
+    TimeUnit.MILLISECONDS.sleep(50);
+
+    assertEquals(someTimeout, bean.getTimeout());
+    assertEquals(anotherBatch, bean.getBatch());
+  }
+
+  @Test
   public void testAutoUpdateWithDeletedPropertiesWithNoDefaultValue() throws Exception {
     int initialTimeout = 1000;
     int initialBatch = 2000;

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderAutoUpdateTest.java

@@ -263,6 +263,38 @@ public class XmlConfigPlaceholderAutoUpdateTest extends AbstractSpringIntegratio
   }
 
   @Test
+  public void testAutoUpdateWithMultipleNamespacesWithSamePropertiesDeleted() throws Exception {
+    int someTimeout = 1000;
+    int someBatch = 2000;
+    int anotherBatch = 3000;
+
+    Properties applicationProperties = assembleProperties(BATCH_PROPERTY,
+        String.valueOf(someBatch));
+    Properties fxApolloProperties = assembleProperties(TIMEOUT_PROPERTY,
+        String.valueOf(someTimeout), BATCH_PROPERTY, String.valueOf(anotherBatch));
+
+    SimpleConfig applicationConfig = prepareConfig(ConfigConsts.NAMESPACE_APPLICATION,
+        applicationProperties);
+    SimpleConfig fxApolloConfig = prepareConfig(FX_APOLLO_NAMESPACE, fxApolloProperties);
+
+    ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext("spring/XmlConfigPlaceholderTest3.xml");
+
+    TestXmlBean bean = context.getBean(TestXmlBean.class);
+
+    assertEquals(someTimeout, bean.getTimeout());
+    assertEquals(someBatch, bean.getBatch());
+
+    Properties newProperties = new Properties();
+
+    applicationConfig.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);
+
+    TimeUnit.MILLISECONDS.sleep(50);
+
+    assertEquals(someTimeout, bean.getTimeout());
+    assertEquals(anotherBatch, bean.getBatch());
+  }
+
+  @Test
   public void testAutoUpdateWithDeletedPropertiesWithNoDefaultValue() throws Exception {
     int initialTimeout = 1000;
     int initialBatch = 2000;