From b3e37257fa141bf22238bfef0b80dfb493acd5dd Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:03:10 +0000 Subject: [PATCH] fix: Reject override snapshots and files that contain a deleted flag or segment The override layer never holds a deleted item (sdk-specs#288, Requirement 1.1.1.1). A snapshot from any source that contains one is rejected as a whole: the layer is unchanged, no flag change listener is notified, and the rejection is logged at Error level with the kind and the key (1.2.8.1). The file override source fails the load of a file that has a flag or segment entry marked as deleted, as for a file that cannot be parsed: the overrides in effect stay, the failure names the file and the key, and the retry applies (1.3.6.2). An entry with "deleted": false is an ordinary definition. The file data source keeps accepting deleted entries. --- .../sdk/server/OverrideLayer.java | 45 ++++++++++- .../sdk/server/OverrideSinkImpl.java | 10 ++- .../server/integrations/FileOverrides.java | 3 +- .../integrations/OverrideFileLoader.java | 23 +++++- .../sdk/server/subsystems/OverrideSink.java | 4 +- .../sdk/server/OverrideLayerTest.java | 62 +++++++++++++-- .../sdk/server/OverrideSinkImplTest.java | 75 ++++++++++++++++++- .../server/integrations/DataLoaderTest.java | 17 +++++ .../integrations/FileOverrideSourceTest.java | 59 +++++++++++++++ .../integrations/OverrideFileLoaderTest.java | 69 +++++++++++++++++ 10 files changed, 353 insertions(+), 14 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideLayer.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideLayer.java index ce1f55ea..057890bb 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideLayer.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideLayer.java @@ -20,6 +20,9 @@ * nested collections with the source's entity, and the layer never writes to them. The source's * entity is never marked, so a source may retain the entities it supplied and supply them again. *

+ * The layer never holds a deleted item. Serving one would hide the LaunchDarkly data for its key, + * but an override only adds or replaces definitions, so a replacement that contains one is rejected. + *

* This class is package-private and should not be used by application code. */ final class OverrideLayer implements DataModelDependencies.ExternalDependencies { @@ -50,11 +53,34 @@ static final class Replacement { // segments. Rebuilt with the contents on every replacement and swapped with them. private volatile DataModelDependencies.DependencyTracker dependencies = new DataModelDependencies.DependencyTracker(); + /** + * Thrown by {@link OverrideLayer#setAll(Iterable)} when the data contains a deleted item. The layer + * never holds one, so the data is rejected as a whole and the layer is unchanged. + */ + static final class DeletedItemException extends IllegalArgumentException { + private static final long serialVersionUID = 1L; + + DeletedItemException(DataKind kind, String key) { + super(describeKind(kind) + " \"" + key + "\" is marked as deleted"); + } + + private static String describeKind(DataKind kind) { + if (kind == DataModel.FEATURES) { + return "flag"; + } + if (kind == DataModel.SEGMENTS) { + return "segment"; + } + return "item of kind " + (kind == null ? "null" : kind.getName()); + } + } + /** * Atomically replaces the entire layer contents. A null or empty collection clears the layer. * * @param data the complete set of entries, grouped by data kind * @return the previous and the new contents + * @throws DeletedItemException if the data contains a deleted item; the layer is unchanged */ Replacement setAll(Iterable>> data) { Map> collected = new HashMap<>(); @@ -69,6 +95,11 @@ Replacement setAll(Iterable>> dat continue; } for (Map.Entry item : kindItems) { + // Nothing is swapped in until every entry is collected, so a rejection leaves the layer as + // it was. + if (isDeleted(item.getValue())) { + throw new DeletedItemException(kindEntry.getKey(), item.getKey()); + } ItemDescriptor copied = markedCopy(item.getValue()); items.put(item.getKey(), copied); tracker.updateDependenciesFrom(kindEntry.getKey(), item.getKey(), copied); @@ -137,8 +168,20 @@ boolean isEmpty() { return !nonEmpty; } + // Reports whether a descriptor is a deleted item: the placeholder with no item, which + // DataKind.deserialize produces for a definition marked as deleted, or a flag or segment that is + // marked as deleted. + private static boolean isDeleted(ItemDescriptor item) { + if (item == null) { + return false; + } + Object entity = item.getItem(); + return entity == null + || (entity instanceof DataModel.VersionedData && ((DataModel.VersionedData) entity).isDeleted()); + } + // Returns a copy of the descriptor whose item carries the override marker. An item of another - // type, or a deleted item placeholder, is returned as is. + // type is returned as is. static ItemDescriptor markedCopy(ItemDescriptor item) { Object entity = item.getItem(); if (entity instanceof FeatureFlag) { diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideSinkImpl.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideSinkImpl.java index 280eed07..fa9ef804 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideSinkImpl.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/OverrideSinkImpl.java @@ -61,6 +61,9 @@ final class OverrideSinkImpl implements OverrideSink { * Atomically replaces the entire override layer, then notifies listeners of every flag whose * merged-view evaluation may have changed. Calls are serialized, so overlapping updates from a * source cannot interleave. + *

+ * A snapshot that contains a deleted item is rejected as a whole: the error is logged, the layer + * keeps its contents, and no listener is notified. */ @Override public synchronized void setOverrides(Iterable>> data) { @@ -68,7 +71,12 @@ public synchronized void setOverrides(Iterable * Files can be rewritten in place: if the source finds a file empty while it is being rewritten, it * keeps the overrides in effect until the new content is written. To remove a file's overrides, diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java index f58488c5..39c3c77c 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java @@ -40,8 +40,9 @@ * This loader is separate from the file data source's loader because the override source has * different rules: a configured file that does not exist contributes no entries, entries keep the * versions that the documents specify, a value-only entry becomes a flag that is on and serves the - * value by fallthrough, the format of a file is chosen by its extension, and every failure to read or - * parse a file is reported as a file data error. The file data source keeps its own behavior. + * value by fallthrough, the format of a file is chosen by its extension, a flag or segment entry that + * is marked as deleted fails the load, and every failure to read or parse a file is reported as a file + * data error. The file data source keeps its own behavior. *

* Whether a file with no content is valid depends on its format, like the rest of its content. A JSON * file with no content is not a JSON document, so it fails to parse. A YAML file with no content is a @@ -426,7 +427,8 @@ private int[] addEntries(Map> data, Path p try { if (document.flags != null) { for (Map.Entry e : document.flags.entrySet()) { - if (add(data, FEATURES, e.getKey(), FlagFactory.flagFromJson(e.getValue()))) { + ItemDescriptor flag = notDeleted("flag", e.getKey(), FlagFactory.flagFromJson(e.getValue())); + if (add(data, FEATURES, e.getKey(), flag)) { flagsAdded++; } } @@ -440,7 +442,8 @@ private int[] addEntries(Map> data, Path p } if (document.segments != null) { for (Map.Entry e : document.segments.entrySet()) { - if (add(data, SEGMENTS, e.getKey(), FlagFactory.segmentFromJson(e.getValue()))) { + ItemDescriptor segment = notDeleted("segment", e.getKey(), FlagFactory.segmentFromJson(e.getValue())); + if (add(data, SEGMENTS, e.getKey(), segment)) { segmentsAdded++; } } @@ -456,6 +459,18 @@ private int[] addEntries(Map> data, Path p return new int[] { flagsAdded, segmentsAdded }; } + // A definition marked as deleted deserializes to a deleted item placeholder. In an override file it + // is a mistake, not a way to remove an override, which is done by removing the entry, so it fails the + // load like a file that cannot be parsed. It fails even when the duplicate keys handling would drop + // the entry, because the file still needs correcting. The file data source keeps accepting it. + private static ItemDescriptor notDeleted(String kind, String key, ItemDescriptor item) throws FileDataException { + if (item.getItem() == null) { + throw new FileDataException(kind + " \"" + key + "\" is marked as deleted; remove the entry to remove its override", + null); + } + return item; + } + /** * Records that the result of a load is now the data in effect: it was applied, or it was skipped * because it is unchanged. Whether each file contributed content to the data in effect decides diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/subsystems/OverrideSink.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/subsystems/OverrideSink.java index b8fab1e5..eb1494a6 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/subsystems/OverrideSink.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/subsystems/OverrideSink.java @@ -25,7 +25,9 @@ public interface OverrideSink { * and {@link com.launchdarkly.sdk.server.DataModel#SEGMENTS}, with item descriptors whose items * are the SDK's flag and segment model objects, as produced by * {@link DataKind#deserialize(String)}. Sources supply ordinary, fully parsed entities. The SDK - * itself marks the entries as overrides. The source's objects are never modified. + * itself marks the entries as overrides. The source's objects are never modified. A snapshot that + * contains a deleted flag or segment is rejected as a whole: the SDK logs an error and the overrides + * in effect stay. *

* This method is safe to call from any thread. The SDK serializes calls, and the new layer * contents are visible to evaluations when the call returns. diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideLayerTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideLayerTest.java index de4f46cb..3fb0c43a 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideLayerTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideLayerTest.java @@ -31,6 +31,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; @SuppressWarnings("javadoc") public class OverrideLayerTest { @@ -119,13 +120,64 @@ public void emptyOrNullSnapshotClearsLayer() { assertTrue(layer.isEmpty()); } + static Iterable>> withItem( + Iterable>> data, DataKind kind, String key, ItemDescriptor item) { + List>> result = new ArrayList<>(); + for (Map.Entry> e : data) { + result.add(e); + } + result.add(new AbstractMap.SimpleEntry<>(kind, + new KeyedItems<>(Collections.singletonList(new AbstractMap.SimpleEntry<>(key, item))))); + return result; + } + + private static void assertRejected(OverrideLayer layer, + Iterable>> data, String expectedMessage) { + try { + layer.setAll(data); + fail("expected the replacement to be rejected"); + } catch (OverrideLayer.DeletedItemException e) { + assertEquals(expectedMessage, e.getMessage()); + } + } + + @Test + public void replacementThatContainsADeletedItemIsRejectedAndTheLayerIsUnchanged() { + OverrideLayer layer = new OverrideLayer(); + layer.setAll(flagsOnly(flagBuilder("kept").version(1).build())); + ItemDescriptor before = layer.get(FEATURES, "kept"); + + Iterable>> otherEntries = + flagsOnly(flagBuilder("kept").version(2).build(), flagBuilder("added").build()); + // The placeholder that DataKind.deserialize produces for a definition marked as deleted. + assertRejected(layer, withItem(otherEntries, FEATURES, "gone", + FEATURES.deserialize("{\"key\":\"gone\",\"version\":3,\"deleted\":true}")), + "flag \"gone\" is marked as deleted"); + assertRejected(layer, withItem(otherEntries, SEGMENTS, "gone-segment", + SEGMENTS.deserialize("{\"key\":\"gone-segment\",\"version\":3,\"deleted\":true}")), + "segment \"gone-segment\" is marked as deleted"); + // A model object that is marked as deleted. + assertRejected(layer, withItem(otherEntries, FEATURES, "gone", + new ItemDescriptor(3, flagBuilder("gone").version(3).deleted(true).build())), + "flag \"gone\" is marked as deleted"); + assertRejected(layer, withItem(otherEntries, SEGMENTS, "gone-segment", + new ItemDescriptor(3, segmentBuilder("gone-segment").version(3).deleted(true).build())), + "segment \"gone-segment\" is marked as deleted"); + + assertSame(before, layer.get(FEATURES, "kept")); + assertNull(layer.get(FEATURES, "added")); + assertNull(layer.get(FEATURES, "gone")); + assertNull(layer.get(SEGMENTS, "gone-segment")); + assertEquals(1, layer.all(FEATURES).size()); + assertTrue(layer.all(SEGMENTS).isEmpty()); + } + @Test - public void deletedPlaceholderIsStoredAsIs() { + public void definitionWithDeletedFalseIsStored() { OverrideLayer layer = new OverrideLayer(); - ItemDescriptor deleted = ItemDescriptor.deletedItem(5); - layer.setAll(Collections.singletonList(new AbstractMap.SimpleEntry<>(FEATURES, - new KeyedItems<>(Collections.singletonList(new AbstractMap.SimpleEntry<>("gone", deleted)))))); - assertSame(deleted, layer.get(FEATURES, "gone")); + layer.setAll(withItem(flagsOnly(), FEATURES, "f", + FEATURES.deserialize("{\"key\":\"f\",\"version\":1,\"deleted\":false}"))); + assertTrue(((FeatureFlag) layer.get(FEATURES, "f").getItem()).isOverride()); } @Test diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideSinkImplTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideSinkImplTest.java index ca7a3916..d7544eeb 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideSinkImplTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/OverrideSinkImplTest.java @@ -1,16 +1,23 @@ package com.launchdarkly.sdk.server; +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogCapture; +import com.launchdarkly.logging.Logs; import com.launchdarkly.sdk.LDValue; import com.launchdarkly.sdk.server.DataModel.FeatureFlag; import com.launchdarkly.sdk.server.DataModel.Segment; import com.launchdarkly.sdk.server.OverrideOverlayStoreTest.FakeBaseStore; import com.launchdarkly.sdk.server.interfaces.FlagChangeEvent; import com.launchdarkly.sdk.server.interfaces.FlagChangeListener; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.DataKind; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; import org.junit.Test; import java.util.Collections; import java.util.HashSet; +import java.util.Map; import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -23,11 +30,13 @@ import static com.launchdarkly.sdk.server.ModelBuilders.segmentBuilder; import static com.launchdarkly.sdk.server.OverrideLayerTest.collections; import static com.launchdarkly.sdk.server.OverrideLayerTest.flagsOnly; +import static com.launchdarkly.sdk.server.OverrideLayerTest.withItem; import static com.launchdarkly.sdk.server.TestComponents.nullLogger; import static com.launchdarkly.sdk.server.TestComponents.sharedExecutor; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @SuppressWarnings("javadoc") @@ -36,7 +45,9 @@ public class OverrideSinkImplTest { private final OverrideLayer layer = new OverrideLayer(); private final EventBroadcasterImpl broadcaster = EventBroadcasterImpl.forFlagChangeEvents(sharedExecutor, nullLogger); - private final OverrideSinkImpl sink = new OverrideSinkImpl(layer, base, broadcaster, nullLogger); + private final LogCapture logCapture = Logs.capture(); + private final OverrideSinkImpl sink = new OverrideSinkImpl(layer, base, broadcaster, + LDLogger.withAdapter(logCapture, "")); private final BlockingQueue events = new LinkedBlockingQueue<>(); private void listen() { @@ -226,4 +237,66 @@ public void valueChangeInSameVersionIsDetectedThroughSerializedForm() throws Exc sink.setOverrides(flagsOnly(flagBuilder("a").version(1).variations(LDValue.of("y")).build())); assertEquals(keys("a"), awaitKeys(1)); } + + private void assertRejectedSnapshotLogged(String kind, String key) { + assertTrue(logCapture.getMessageStrings().toString(), logCapture.getMessageStrings().contains( + "ERROR:Rejected an override snapshot because " + kind + " \"" + key + + "\" is marked as deleted; the overrides in effect are unchanged")); + } + + @Test + public void snapshotWithADeletedFlagIsRejected() throws Exception { + listen(); + sink.setOverrides(flagsOnly(flagBuilder("a").version(1).build())); + assertEquals(keys("a"), awaitKeys(1)); + ItemDescriptor before = layer.get(DataModel.FEATURES, "a"); + + // The other entries of the snapshot would each be a change. + Iterable>> snapshot = withItem( + flagsOnly(flagBuilder("a").version(2).build(), flagBuilder("b").build()), + DataModel.FEATURES, "gone", DataModel.FEATURES.deserialize("{\"key\":\"gone\",\"version\":3,\"deleted\":true}")); + sink.setOverrides(snapshot); + + assertSame(before, layer.get(DataModel.FEATURES, "a")); + assertNull(layer.get(DataModel.FEATURES, "b")); + assertNull(layer.get(DataModel.FEATURES, "gone")); + assertNoMoreEvents(); + assertRejectedSnapshotLogged("flag", "gone"); + + // The sink still applies a later snapshot. + sink.setOverrides(flagsOnly(flagBuilder("a").version(2).build())); + assertEquals(keys("a"), awaitKeys(1)); + } + + @Test + public void snapshotWithADeletedSegmentIsRejected() throws Exception { + // A LaunchDarkly flag depends on the deleted segment, so applying it would notify that flag. + base.withFlag(flagBuilder("uses-segment").on(true) + .rules(ruleBuilder().id("r").variation(0).clauses(clauseMatchingSegment("seg")).build()).build()); + listen(); + sink.setOverrides(collections(Collections.emptyList(), + Collections.singletonList(segmentBuilder("other").version(1).build()))); + assertNoMoreEvents(); + ItemDescriptor before = layer.get(DataModel.SEGMENTS, "other"); + + sink.setOverrides(withItem(flagsOnly(flagBuilder("a").build()), DataModel.SEGMENTS, "seg", + DataModel.SEGMENTS.deserialize("{\"key\":\"seg\",\"version\":3,\"deleted\":true}"))); + + assertSame(before, layer.get(DataModel.SEGMENTS, "other")); + assertNull(layer.get(DataModel.SEGMENTS, "seg")); + assertNull(layer.get(DataModel.FEATURES, "a")); + assertNoMoreEvents(); + assertRejectedSnapshotLogged("segment", "seg"); + } + + @Test + public void snapshotWithADeletedItemIsRejectedWhenNothingIsListening() { + sink.setOverrides(flagsOnly(flagBuilder("a").version(1).build())); + sink.setOverrides(withItem(flagsOnly(flagBuilder("b").build()), DataModel.FEATURES, "gone", + ItemDescriptor.deletedItem(3))); + + assertEquals(1, layer.get(DataModel.FEATURES, "a").getVersion()); + assertNull(layer.get(DataModel.FEATURES, "b")); + assertRejectedSnapshotLogged("flag", "gone"); + } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/DataLoaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/DataLoaderTest.java index 90580e3c..f090fa33 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/DataLoaderTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/DataLoaderTest.java @@ -9,6 +9,7 @@ import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; import com.launchdarkly.testhelpers.JsonTestValue; +import com.launchdarkly.testhelpers.TempFile; import org.junit.Assert; import org.junit.Test; @@ -151,6 +152,22 @@ public void duplicateKeysCanBeAllowed() throws Exception { jsonIncludes("{\"included\":[\"user1\"]}")); // value from first file } + @Test + public void entriesMarkedAsDeletedStillLoadAsDeletedItems() throws Exception { + // The file override source rejects such entries. The file data source keeps accepting them, as + // it always has; rejecting them here would be a breaking change. + try (TempFile f = TempFile.create()) { + f.setContents("{\"flags\":{\"gone\":{\"key\":\"gone\",\"deleted\":true}}," + + "\"segments\":{\"gone-segment\":{\"key\":\"gone-segment\",\"deleted\":true}}}"); + DataLoader ds = new DataLoader(FileData.dataSource().filePaths(f.getPath()).sources); + int version = ds.load(builder); + + Map> data = toDataMap(new FullDataSet<>(builder.build())); + assertThat(data.get(FEATURES).get("gone"), equalTo(ItemDescriptor.deletedItem(version))); + assertThat(data.get(SEGMENTS).get("gone-segment"), equalTo(ItemDescriptor.deletedItem(version))); + } + } + @Test public void versionsAreIncrementedForEachLoad() throws Exception { DataLoader ds = new DataLoader(FileData.dataSource().filePaths( diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileOverrideSourceTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileOverrideSourceTest.java index de95ee6e..ed8ea15e 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileOverrideSourceTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileOverrideSourceTest.java @@ -362,6 +362,65 @@ public void malformedFileAtStartupSuppliesNothingAndLogsError() throws Exception } } + private void awaitLogMessage(String message) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (!logCapture.getMessageStrings().contains(message)) { + if (System.currentTimeMillis() > deadline) { + fail("expected log message " + message + " but got " + logCapture.getMessageStrings()); + } + Thread.sleep(20); + } + } + + // An edit that adds an entry marked as deleted fails the reload like a malformed edit: the + // overrides in effect stay, nothing is supplied, and the failure names the file and the key. Other + // changes in the same edit are not applied either. + private void entryMarkedAsDeletedFailsTheReload(String document, String kind, String key) throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", docWith("flag", "first")); + RecordingSink sink = new RecordingSink(); + start(FileOverrides.source().filePaths(file), sink); + assertEquals("first", flagValue(sink.await(), "flag")); + + Files.write(file, document.getBytes()); + awaitLogMessage("ERROR:Unable to load flags: file " + file + ": " + kind + " \"" + key + + "\" is marked as deleted; remove the entry to remove its override"); + sink.assertNoSnapshot(200); + + // Removing the entry corrects the file. + Files.write(file, docWith("flag", "second").getBytes()); + assertEquals("second", flagValue(sink.await(), "flag")); + } + } + + @Test + public void flagMarkedAsDeletedFailsTheReloadAndKeepsTheOverrides() throws Exception { + entryMarkedAsDeletedFailsTheReload("{\"flagValues\":{\"flag\":\"changed\"}," + + "\"flags\":{\"gone\":{\"key\":\"gone\",\"version\":1,\"deleted\":true}}}", "flag", "gone"); + } + + @Test + public void segmentMarkedAsDeletedFailsTheReloadAndKeepsTheOverrides() throws Exception { + entryMarkedAsDeletedFailsTheReload("{\"flagValues\":{\"flag\":\"changed\"}," + + "\"segments\":{\"gone\":{\"key\":\"gone\",\"version\":1,\"deleted\":true}}}", "segment", "gone"); + } + + @Test + public void entriesWithDeletedFalseLoad() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", + "{\"flags\":{\"flag\":{\"key\":\"flag\",\"version\":1,\"on\":true,\"variations\":[\"value\"]," + + "\"fallthrough\":{\"variation\":0},\"deleted\":false}}," + + "\"segments\":{\"seg\":{\"key\":\"seg\",\"version\":1,\"deleted\":false}}}"); + RecordingSink sink = new RecordingSink(); + start(FileOverrides.source().filePaths(file), sink); + + Map> snapshot = sink.await(); + assertEquals("value", flagValue(snapshot, "flag")); + assertNotNull(snapshot.get(SEGMENTS.getName()).get("seg").getItem()); + } + } + private void reloadsOnChange(ChangeDetection mode) throws Exception { try (TempDir dir = TempDir.create()) { Path file = write(dir, "overrides.json", docWith("flag", "first")); diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java index cdd8665f..336c046e 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java @@ -36,6 +36,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -201,6 +202,74 @@ public void modelRejectedDocumentIsAFileDataError() throws Exception { } } + private static void assertMarkedAsDeleted(OverrideFileLoader loader, Path file, String kind, String key) + throws Exception { + try { + loader.load(Duration.ZERO); + fail("expected exception"); + } catch (FileDataException e) { + assertEquals("file " + file + ": " + kind + " \"" + key + + "\" is marked as deleted; remove the entry to remove its override", e.getMessage()); + assertNull(e.getCause()); + } + } + + @Test + public void flagMarkedAsDeletedFailsTheLoad() throws Exception { + try (TempDir dir = TempDir.create()) { + Path good = write(dir, "good.json", "{\"flagValues\":{\"a\":1}}"); + Path file = write(dir, "overrides.json", "{\"flagValues\":{\"b\":2}," + + "\"flags\":{\"f\":{\"key\":\"f\",\"version\":1,\"deleted\":true}}}"); + assertMarkedAsDeleted(loader(good, file), file, "flag", "f"); + + Path yaml = write(dir, "overrides.yaml", "flags:\n f:\n key: f\n version: 1\n deleted: true\n"); + assertMarkedAsDeleted(loader(yaml), yaml, "flag", "f"); + } + } + + @Test + public void segmentMarkedAsDeletedFailsTheLoad() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", "{\"flagValues\":{\"b\":2}," + + "\"segments\":{\"s\":{\"key\":\"s\",\"version\":1,\"deleted\":true}}}"); + assertMarkedAsDeleted(loader(file), file, "segment", "s"); + + Path yaml = write(dir, "overrides.yaml", "segments:\n s:\n key: s\n deleted: true\n"); + assertMarkedAsDeleted(loader(yaml), yaml, "segment", "s"); + } + } + + @Test + public void entryMarkedAsDeletedFailsTheLoadEvenWhenItIsADroppedDuplicate() throws Exception { + // The duplicate keys handling would drop the entry, but the file still needs correcting. + try (TempDir dir = TempDir.create()) { + Path first = write(dir, "first.json", "{\"flags\":{\"f\":{\"key\":\"f\",\"version\":1}}}"); + Path second = write(dir, "second.json", "{\"flags\":{\"f\":{\"key\":\"f\",\"version\":2,\"deleted\":true}}}"); + assertMarkedAsDeleted(new OverrideFileLoader(Arrays.asList(first, second), FileData.DuplicateKeysHandling.IGNORE), + second, "flag", "f"); + } + } + + @Test + public void entriesWithDeletedFalseAreOrdinaryDefinitions() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", + "{\"flags\":{\"f\":{\"key\":\"f\",\"version\":4,\"on\":true,\"variations\":[\"x\"]," + + "\"fallthrough\":{\"variation\":0},\"deleted\":false}}," + + "\"segments\":{\"s\":{\"key\":\"s\",\"version\":5,\"deleted\":false}}}"); + LoadResult result = loader(file).load(Duration.ZERO); + + Map> data = toMap(result); + assertNotNull(data.get(FEATURES).get("f").getItem()); + assertEquals(4, data.get(FEATURES).get("f").getVersion()); + assertEquals(LDValue.buildArray().add("x").build(), json(FEATURES, data.get(FEATURES).get("f")).get("variations")); + assertNotNull(data.get(SEGMENTS).get("s").getItem()); + assertEquals(5, data.get(SEGMENTS).get("s").getVersion()); + assertEquals(1, result.getFlagCount()); + assertEquals(1, result.getSegmentCount()); + } + } + @Test public void duplicateKeysFailByDefaultAcrossFilesAndWithinAFile() throws Exception { try (TempDir dir = TempDir.create()) {