diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index 8922955d373..14ef420dee2 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -25,6 +25,9 @@ public class RemoteConfigServiceImpl implements ConfigurationChangesTypedListene private final List> subscribers = new CopyOnWriteArrayList<>(); private volatile byte[] current; + /** Key of the current document, guarded by the subscribers lock. */ + @Nullable private String currentConfigKey; + /** * Whether the current document was received before any subscriber, so the poller acknowledged it * without validation. Guarded by the subscribers lock. @@ -114,8 +117,11 @@ public synchronized void close() { this.configurationPoller.removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); this.configurationPoller.removeListeners(FFE_FLAGS); this.configurationPoller.stop(); - this.subscribers.clear(); - this.current = null; + synchronized (this.subscribers) { + this.subscribers.clear(); + this.current = null; + this.currentConfigKey = null; + } } /** @@ -134,6 +140,11 @@ public void accept( final PollingRateHinter pollingRateHinter) { final Iterator> snapshot; synchronized (this.subscribers) { + // RC applies a replacement before removing the old target. + if (content == null && !configKey.equals(this.currentConfigKey)) { + return; + } + this.currentConfigKey = content == null ? null : configKey; this.current = content; this.unvalidated = content != null && this.subscribers.isEmpty(); snapshot = this.subscribers.iterator(); diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index 728291d7291..f39cc2b0579 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java @@ -41,6 +41,7 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.slf4j.LoggerFactory; +import org.tabletest.junit.TableTest; class RemoteConfigServiceImplTest { private static final byte[] DOCUMENT = "{\"flags\":{}}".getBytes(UTF_8); @@ -80,6 +81,39 @@ void forwardsDocumentsAndRemovalsToAllSubscribers() { assertEquals(2, second.size()); } + @TableTest({ + "Scenario | previousKey | replacementKey", + "enable dependency | base | dependent ", + "disable dependency | dependent | base ", + "semver upgrade | base | semver " + }) + void keepsReplacementWhenPreviousConfigurationIsRemoved( + final String previousKey, final String replacementKey) { + final RemoteConfigServiceImpl service = + new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + final List received = new ArrayList<>(); + service.subscribe(received::add); + final byte[] replacement = "{\"flags\":{\"replacement\":{}}}".getBytes(UTF_8); + + // The RC poller applies the new target before removing the obsolete target. + service.accept(previousKey, DOCUMENT, null); + service.accept(replacementKey, replacement, null); + service.accept(previousKey, null, null); + + assertEquals(2, received.size(), "Removing the old target must not clear its replacement"); + assertArrayEquals(replacement, received.get(1)); + final List replayed = new ArrayList<>(); + service.subscribe(replayed::add); + assertEquals(1, replayed.size()); + assertArrayEquals(replacement, replayed.get(0)); + + service.accept(replacementKey, null, null); + assertEquals(3, received.size()); + assertNull(received.get(2)); + assertEquals(2, replayed.size()); + assertNull(replayed.get(1)); + } + @Test void reportsRejectedDocumentsOnceEverySubscriberIsNotified() { final RemoteConfigServiceImpl service = @@ -339,6 +373,7 @@ void reportsReceivedConfiguration() { @Test void reportsRemovedConfiguration() { + service.accept("config", DOCUMENT, mock(PollingRateHinter.class)); service.accept("config", null, mock(PollingRateHinter.class)); assertLogged("configuration removed: key=config");