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()) {