Test that isChangedAndReset obeys generation.

2 files changed, 26 insertions, 11 deletions

diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java
index 6fd157ee67f..446630d41c3 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java
@@ -177,21 +177,17 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
         this.config.set(new ConfigState<>(true, generation, true, config));
     }
 
-    void setConfigIncGen(T config) {
+    // Only used by {@link FileConfigSubscription}
+    protected void setConfigIncGen(T config) {
         ConfigState<T> prev = this.config.get();
-        while ( !this.config.compareAndSet(prev, new ConfigState<>(true, prev.getGeneration()+1, true, config))) {
-            prev = this.config.get();
-        }
+        setConfig(prev.getGeneration() + 1, config);
     }
 
-    void setConfigIfChangedIncGen(T config) {
+    // Only used by {@link FileConfigSubscription} and {@link ConfigSetSubscription}
+    protected void setConfigIfChangedIncGen(T config) {
         ConfigState<T> prev = this.config.get();
-        while (!this.config.compareAndSet(prev, new ConfigState<>(true, prev.getGeneration() + 1, !config.equals(prev.getConfig()), config))) {
-            prev = this.config.get();
-        }
-    }
-    void setConfigIfChanged(Long generation, T config) {
-        this.config.set(new ConfigState<>(true, generation, !config.equals(this.config.get().getConfig()), config));
+        this.config.set(new ConfigState<>(true, prev.getGeneration() + 1,
+                                          !config.equals(prev.getConfig()), config));
     }
 
     void setGeneration(Long generation) {
diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java
index 14669e969ca..561bdb1f4d4 100644
--- a/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java
@@ -18,6 +18,8 @@ import java.nio.file.Files;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
@@ -72,12 +74,29 @@ public class FileConfigSubscriptionTest {
         assertThat(configState.getConfig().intval(), is(33));
         assertTrue(configState.isConfigChanged());
         assertTrue(configState.isGenerationChanged());
+
+        assertTrue(sub.isConfigChangedAndReset(7L));
+        assertSame(configState, sub.getConfigState());
+        assertTrue(configState.isConfigChanged());
+        assertTrue(configState.isGenerationChanged());
+        assertTrue(sub.isConfigChangedAndReset(2L));
+        assertNotSame(configState, sub.getConfigState());
+        configState = sub.getConfigState();
+        assertFalse(configState.isConfigChanged());
+        assertFalse(configState.isGenerationChanged());
+
         sub.reload(2);
         assertTrue(sub.nextConfig(1000));
         configState = sub.getConfigState();
         assertThat(configState.getConfig().intval(), is(33));
         assertFalse(configState.isConfigChanged());
         assertTrue(configState.isGenerationChanged());
+
+        assertFalse(sub.isConfigChangedAndReset(3L));
+        assertNotSame(configState, sub.getConfigState());
+        configState = sub.getConfigState();
+        assertFalse(configState.isConfigChanged());
+        assertFalse(configState.isGenerationChanged());
     }
 
     @Test