From 0e2c285183e038a7c50acebf5c8fdd5a06c657d7 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Thu, 15 Oct 2026 16:02:21 -0400 Subject: [PATCH 1/4] add opt-in release of in-clause value lists after preprocessing --- .../launchdarkly/sdk/server/DataModel.java | 35 ++++-- .../sdk/server/DataModelPreprocessing.java | 28 +++++ .../sdk/server/DataModelSerialization.java | 3 +- .../server/DataModelPreprocessingTest.java | 92 ++++++++++++++++ .../server/DataModelSerializationTest.java | 100 ++++++++++++++++++ .../sdk/server/EvaluatorClauseTest.java | 38 +++++++ 6 files changed, 286 insertions(+), 10 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java index 1e919cfe..238d1fa7 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java @@ -365,11 +365,11 @@ static final class Clause { private final ContextKind contextKind; private final AttributeRef attribute; private final Operator op; - private final List values; //interpreted as an OR of values + private List values; //interpreted as an OR of values; null only after releaseValues() private final boolean negate; - + transient ClausePreprocessed preprocessed; - + Clause(ContextKind contextKind, AttributeRef attribute, Operator op, List values, boolean negate) { this.contextKind = contextKind; this.attribute = attribute; @@ -377,24 +377,41 @@ static final class Clause { this.values = values == null ? emptyList() : values; this.negate = negate; } - + ContextKind getContextKind() { return contextKind; } - + AttributeRef getAttribute() { return attribute; } - + Operator getOp() { return op; } - + // Guaranteed non-null List getValues() { - return values; + return values == null ? emptyList() : values; } - + + // The preprocessor calls this to drop the value list when the preprocessed value set holds the + // same values. This frees the memory of the duplicate list. Read values through getAllValues() + // to cover both states. + void releaseValues() { + values = null; + } + + // Returns the values whether or not releaseValues() dropped the list. The preprocessed set + // keeps the values in first-appearance order, so a released clause serializes its values in + // the original order, without duplicates. + Iterable getAllValues() { + if (values == null && preprocessed != null && preprocessed.valuesSet != null) { + return preprocessed.valuesSet; + } + return getValues(); + } + boolean isNegate() { return negate; } diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java index e24a07d7..6891fcb2 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java @@ -40,6 +40,26 @@ */ abstract class DataModelPreprocessing { private DataModelPreprocessing() {} + + /** + * The name of the system property that turns on the release of "in" clause value lists. + *

+ * When the property is "true", the preprocessor drops the value list of each "in" clause that + * has a preprocessed value set. This frees the memory of the duplicate list. Evaluation and + * serialization then read the values from the set. A released clause serializes its values in + * the original order, but without duplicate values. + *

+ * The default is off, so the serialized form of a clause stays identical to + * its previous form. + */ + static final String RELEASE_CLAUSE_VALUES_PROPERTY = "com.launchdarkly.sdk.server.releaseClauseValues"; + + private static volatile boolean releaseClauseValues = Boolean.getBoolean(RELEASE_CLAUSE_VALUES_PROPERTY); + + // For tests. Production code reads the system property once, at class load. + static void setReleaseClauseValues(boolean enabled) { + releaseClauseValues = enabled; + } static final class EvalResultsForSingleVariation { private final EvalResult regularResult; @@ -221,8 +241,16 @@ static void preprocessClause(Clause c) { // clause values. Converting the value list to a Set allows us to do a fast lookup instead of // a linear search. We do not do this for other operators (or if there are fewer than two // values) because the slight extra overhead of a Set is not worthwhile in those case. + // JMH measurements show the set lookup is faster than a list scan at every size of two or + // more values, so two is the right threshold for Java. if (values.size() > 1) { c.preprocessed = new ClausePreprocessed(ImmutableSet.copyOf(values), null); + if (releaseClauseValues) { + // The set holds the same values, so the clause does not need the list too. The set keeps + // first-appearance order, so serialization keeps the original order, but a duplicate + // value is dropped. Other operators always keep their list. + c.releaseValues(); + } } } else if (op == matches) { c.preprocessed = preprocessClauseValues(c.getValues(), v -> diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java index b203069a..4a890986 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java @@ -164,8 +164,9 @@ public void write(JsonWriter out, Clause c) throws IOException { } out.name("attribute").value(c.getAttribute() == null ? null : c.getAttribute().toString()); out.name("op").value(c.getOp() == null ? null : c.getOp().name()); + // getAllValues() reads from the preprocessed value set when the value list was released. out.name("values").beginArray(); - for (LDValue v: c.getValues()) { + for (LDValue v: c.getAllValues()) { gsonInstanceWithNullsAllowed().toJson(v, LDValue.class, out); } out.endArray(); diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java index 0fb0fc9c..e3af08fe 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java @@ -274,6 +274,98 @@ public void preprocessFlagDoesNotCreateClauseValuesMapForNonEqualityOperators() } } + @Test + public void preprocessFlagKeepsClauseValuesListByDefault() { + Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of(0), LDValue.of("a")); + + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertNotNull(pc.preprocessed.valuesSet); + assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0), LDValue.of("a")), pc.getValues()); + assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0), LDValue.of("a")), + ImmutableList.copyOf(pc.getAllValues())); + } + + @Test + public void preprocessFlagReleasesClauseValuesListWhenReleaseIsEnabled() { + DataModelPreprocessing.setReleaseClauseValues(true); + try { + Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of(0), LDValue.of("a")); + + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertEquals(ImmutableList.of(), pc.getValues()); + assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of(0)), pc.preprocessed.valuesSet); + // the set keeps first-appearance order, so getAllValues() drops only the duplicate + assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0)), + ImmutableList.copyOf(pc.getAllValues())); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + + @Test + public void releaseKeepsClauseValuesListWhenThereIsNoValueSet() { + DataModelPreprocessing.setReleaseClauseValues(true); + try { + Clause c = clause("x", Operator.in, LDValue.of("a")); + + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertNull(pc.preprocessed); + assertEquals(ImmutableList.of(LDValue.of("a")), pc.getValues()); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + + @Test + public void releaseKeepsClauseValuesListForNonEqualityOperators() { + // Evaluation and dependency analysis read the list directly for every operator other than + // "in" (this includes segmentMatch), so the release must not touch these clauses. + DataModelPreprocessing.setReleaseClauseValues(true); + try { + for (Operator op: Operator.getBuiltins()) { + if (op == Operator.in) { + continue; + } + Clause c = clause("x", op, LDValue.of("a"), LDValue.of("b")); + + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertEquals(op.toString(), ImmutableList.of(LDValue.of("a"), LDValue.of("b")), pc.getValues()); + } + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + + @Test + public void releaseAppliesToSegmentRuleClauses() { + DataModelPreprocessing.setReleaseClauseValues(true); + try { + Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of("b")); + SegmentRule rule = segmentRuleBuilder().clauses(c).build(); + Segment s = segmentBuilder("key").disablePreprocessing(true).rules(rule).build(); + + s.afterDeserialized(); + + Clause pc = s.getRules().get(0).getClauses().get(0); + assertEquals(ImmutableList.of(), pc.getValues()); + assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of("b")), pc.preprocessed.valuesSet); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + @Test public void preprocessFlagParsesClauseDate() { String time1Str = "2016-04-16T17:09:12-07:00"; diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java index c2a56ec9..667cd484 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java @@ -611,6 +611,106 @@ public void parsingFullDataSetFlagsAndSegments() throws Exception { .addAny(FEATURES, flag1, flag2).addAny(SEGMENTS, segment1, segment2).build(), allData); } + @Test + public void clauseWithReleasedValuesListIsSerializedFromValueSet() { + // With the release enabled, an "in" clause with 2 or more values drops its value list and the + // serializer writes the values from the preprocessed set. The set keeps first-appearance + // order, so the output keeps the original order and drops only the duplicate value. + DataModelPreprocessing.setReleaseClauseValues(true); + try { + LDValue clauseJson = LDValue.buildObject() + .put("attribute", "name") + .put("op", "in") + .put("values", LDValue.buildArray() + .add("a").add("b").add(1).add(true).add("a") + .build()) + .put("negate", false) + .build(); + assertClauseFromJson(clauseJson, c -> { + assertEquals(0, c.getValues().size()); + }); + + LDValue flagJson = baseBuilder("flag-key") + .put("rules", LDValue.buildArray() + .add(LDValue.buildObject() + .put("id", "ruleid") + .put("clauses", LDValue.arrayOf(clauseJson)) + .build()) + .build()) + .build(); + assertFlagFromJson(flagJson, flag -> { + String json = FEATURES.serialize(new ItemDescriptor(flag.getVersion(), flag)); + LDValue values = LDValue.parse(json) + .get("rules").get(0).get("clauses").get(0).get("values"); + assertEquals(LDValue.buildArray().add("a").add("b").add(1).add(true).build(), values); + + // the released form round-trips: parsing the new JSON gives the same values again + assertFlagFromJson(LDValue.parse(json), flag2 -> { + String json2 = FEATURES.serialize(new ItemDescriptor(flag2.getVersion(), flag2)); + assertJsonEquals(json, json2); + }); + }); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + + @Test + public void segmentClauseWithReleasedValuesListIsSerializedFromValueSet() { + DataModelPreprocessing.setReleaseClauseValues(true); + try { + LDValue segmentJson = baseBuilder("segment-key") + .put("rules", LDValue.buildArray() + .add(LDValue.buildObject() + .put("clauses", LDValue.buildArray() + .add(LDValue.buildObject() + .put("attribute", "name") + .put("op", "in") + .put("values", LDValue.buildArray().add("a").add("b").add("a").build()) + .put("negate", false) + .build()) + .build()) + .build()) + .build()) + .build(); + assertSegmentFromJson(segmentJson, segment -> { + assertEquals(0, segment.getRules().get(0).getClauses().get(0).getValues().size()); + + String json = SEGMENTS.serialize(new ItemDescriptor(segment.getVersion(), segment)); + LDValue values = LDValue.parse(json) + .get("rules").get(0).get("clauses").get(0).get("values"); + assertEquals(LDValue.buildArray().add("a").add("b").build(), values); + }); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + + @Test + public void clauseValuesAreSerializedUnchangedByDefault() { + LDValue clauseJson = LDValue.buildObject() + .put("attribute", "name") + .put("op", "in") + .put("values", LDValue.buildArray().add("a").add("b").add("a").build()) + .put("negate", false) + .build(); + LDValue flagJson = baseBuilder("flag-key") + .put("rules", LDValue.buildArray() + .add(LDValue.buildObject() + .put("id", "ruleid") + .put("clauses", LDValue.arrayOf(clauseJson)) + .build()) + .build()) + .build(); + assertFlagFromJson(flagJson, flag -> { + String json = FEATURES.serialize(new ItemDescriptor(flag.getVersion(), flag)); + LDValue values = LDValue.parse(json) + .get("rules").get(0).get("clauses").get(0).get("values"); + // the duplicate value and the original order are kept + assertEquals(LDValue.buildArray().add("a").add("b").add("a").build(), values); + }); + } + private void assertFlagFromJson(LDValue flagJson, Consumer action) { FeatureFlag flag = (FeatureFlag)FEATURES.deserialize(flagJson.toJsonString()).getItem(); action.accept(flag); diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java index 4c1317f5..ce564248 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java @@ -301,4 +301,42 @@ public void testSegmentMatchClauseIgnoresNonStringValues() throws Exception { Evaluator e = evaluatorBuilder().withStoredSegments(segment).build(); assertMatch(e, flag, context, true); } + + @Test + public void clauseCanMatchWhenValuesListIsReleased() throws Exception { + DataModelPreprocessing.setReleaseClauseValues(true); + try { + Clause clause = clause("name", Operator.in, LDValue.of("Bob"), LDValue.of("Carol")); + FeatureFlag f = booleanFlagWithClauses("flag", clause); + // the release dropped the list, so this match can only come from the value set + assertEquals(0, f.getRules().get(0).getClauses().get(0).getValues().size()); + + assertMatch(BASE_EVALUATOR, f, LDContext.builder("key").name("Carol").build(), true); + assertMatch(BASE_EVALUATOR, f, LDContext.builder("key").name("Don").build(), false); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + + @Test + public void segmentMatchClauseStillMatchesWhenReleaseIsEnabled() throws Exception { + // The release must not touch segmentMatch clauses, because segment matching reads the + // values list directly. + DataModelPreprocessing.setReleaseClauseValues(true); + try { + String segmentKey = "segkey"; + Clause inClause = clause("name", Operator.in, LDValue.of("Bob"), LDValue.of("Carol")); + Clause segmentClause = clauseMatchingSegment(segmentKey); + FeatureFlag flag = booleanFlagWithClauses("flag", segmentClause); + FeatureFlag flagWithBoth = booleanFlagWithClauses("flag2", inClause, segmentClause); + Segment segment = makeSegmentThatMatchesUser(segmentKey, "foo"); + + Evaluator e = evaluatorBuilder().withStoredSegments(segment).build(); + assertMatch(e, flag, LDContext.create("foo"), true); + assertMatch(e, flag, LDContext.create("bar"), false); + assertMatch(e, flagWithBoth, LDContext.builder("foo").name("Carol").build(), true); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } } From 65d5e91c3587751c80d8deaa759d6223eec7f298 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 16 Oct 2026 09:50:10 -0400 Subject: [PATCH 2/4] fix: do not reprocess list after released --- .../sdk/server/DataModelPreprocessing.java | 5 +++++ .../server/DataModelPreprocessingTest.java | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java index 6891fcb2..f3bef69c 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java @@ -243,6 +243,11 @@ static void preprocessClause(Clause c) { // values) because the slight extra overhead of a Set is not worthwhile in those case. // JMH measurements show the set lookup is faster than a list scan at every size of two or // more values, so two is the right threshold for Java. + if (c.preprocessed != null && c.preprocessed.valuesSet != null) { + // The clause already has its value set, and its value list can be released. A second + // preprocess pass must not build a new set from the possibly empty list. + return; + } if (values.size() > 1) { c.preprocessed = new ClausePreprocessed(ImmutableSet.copyOf(values), null); if (releaseClauseValues) { diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java index e3af08fe..d7893441 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java @@ -308,6 +308,27 @@ public void preprocessFlagReleasesClauseValuesListWhenReleaseIsEnabled() { } } + @Test + public void preprocessIsIdempotentWhenClauseValuesListIsReleased() { + // A clause is normally preprocessed only once. The guard makes a repeat pass safe: it must + // not replace the value set with one built from the released (empty) list. + DataModelPreprocessing.setReleaseClauseValues(true); + try { + Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of("b")); + + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of("b")), pc.preprocessed.valuesSet); + assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of("b")), + ImmutableList.copyOf(pc.getAllValues())); + } finally { + DataModelPreprocessing.setReleaseClauseValues(false); + } + } + @Test public void releaseKeepsClauseValuesListWhenThereIsNoValueSet() { DataModelPreprocessing.setReleaseClauseValues(true); From 051234cce9e15725fd2473af28093576d5bc98df Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 16 Oct 2026 15:00:23 -0400 Subject: [PATCH 3/4] fix: replace released clause value lists with the set's list view --- .../launchdarkly/sdk/server/DataModel.java | 23 +-- .../sdk/server/DataModelPreprocessing.java | 33 ++--- .../sdk/server/DataModelSerialization.java | 3 +- .../server/DataModelPreprocessingTest.java | 114 ++++++--------- .../server/DataModelSerializationTest.java | 131 +++++++++--------- .../sdk/server/EvaluatorClauseTest.java | 49 +++---- 6 files changed, 155 insertions(+), 198 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java index 238d1fa7..403fb46a 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java @@ -365,7 +365,7 @@ static final class Clause { private final ContextKind contextKind; private final AttributeRef attribute; private final Operator op; - private List values; //interpreted as an OR of values; null only after releaseValues() + private List values; // OR semantics. The preprocessor can replace it via releaseValues(). private final boolean negate; transient ClausePreprocessed preprocessed; @@ -392,24 +392,13 @@ Operator getOp() { // Guaranteed non-null List getValues() { - return values == null ? emptyList() : values; + return values; } - // The preprocessor calls this to drop the value list when the preprocessed value set holds the - // same values. This frees the memory of the duplicate list. Read values through getAllValues() - // to cover both states. - void releaseValues() { - values = null; - } - - // Returns the values whether or not releaseValues() dropped the list. The preprocessed set - // keeps the values in first-appearance order, so a released clause serializes its values in - // the original order, without duplicates. - Iterable getAllValues() { - if (values == null && preprocessed != null && preprocessed.valuesSet != null) { - return preprocessed.valuesSet; - } - return getValues(); + // The preprocessor calls this when the preprocessed value set holds the same values. The set's + // cached list view replaces the original list, which frees the memory of the duplicate list. + void releaseValues(List valuesFromSet) { + values = valuesFromSet; } boolean isNegate() { diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java index f3bef69c..c96f55ea 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java @@ -44,13 +44,14 @@ private DataModelPreprocessing() {} /** * The name of the system property that turns on the release of "in" clause value lists. *

- * When the property is "true", the preprocessor drops the value list of each "in" clause that - * has a preprocessed value set. This frees the memory of the duplicate list. Evaluation and - * serialization then read the values from the set. A released clause serializes its values in - * the original order, but without duplicate values. + * When the property is "true", the preprocessor replaces the value list of each "in" clause that + * has a preprocessed value set with the set's list view. This frees the memory of the duplicate + * list. The view keeps the values in first-appearance order, so a released clause serializes its + * values in the original order, but without duplicate values. Equal numbers in different forms, + * such as 1 and 1.0, count as duplicates. *

- * The default is off, so the serialized form of a clause stays identical to - * its previous form. + * The SDK reads the property once, at class load. The default is off. With the default, a + * clause serializes its values exactly as received. */ static final String RELEASE_CLAUSE_VALUES_PROPERTY = "com.launchdarkly.sdk.server.releaseClauseValues"; @@ -240,21 +241,15 @@ static void preprocessClause(Clause c) { // This is a special case where the clause is testing for an exact match against any of the // clause values. Converting the value list to a Set allows us to do a fast lookup instead of // a linear search. We do not do this for other operators (or if there are fewer than two - // values) because the slight extra overhead of a Set is not worthwhile in those case. - // JMH measurements show the set lookup is faster than a list scan at every size of two or - // more values, so two is the right threshold for Java. - if (c.preprocessed != null && c.preprocessed.valuesSet != null) { - // The clause already has its value set, and its value list can be released. A second - // preprocess pass must not build a new set from the possibly empty list. - return; - } + // values) because the slight extra overhead of a Set is not worthwhile in those cases. if (values.size() > 1) { - c.preprocessed = new ClausePreprocessed(ImmutableSet.copyOf(values), null); + ImmutableSet valuesSet = ImmutableSet.copyOf(values); + c.preprocessed = new ClausePreprocessed(valuesSet, null); if (releaseClauseValues) { - // The set holds the same values, so the clause does not need the list too. The set keeps - // first-appearance order, so serialization keeps the original order, but a duplicate - // value is dropped. Other operators always keep their list. - c.releaseValues(); + // Replace the list with the set's cached list view, which frees the duplicate list. + // Other operators always keep their list, because they read values by position or as + // segment keys. + c.releaseValues(valuesSet.asList()); } } } else if (op == matches) { diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java index 4a890986..b203069a 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelSerialization.java @@ -164,9 +164,8 @@ public void write(JsonWriter out, Clause c) throws IOException { } out.name("attribute").value(c.getAttribute() == null ? null : c.getAttribute().toString()); out.name("op").value(c.getOp() == null ? null : c.getOp().name()); - // getAllValues() reads from the preprocessed value set when the value list was released. out.name("values").beginArray(); - for (LDValue v: c.getAllValues()) { + for (LDValue v: c.getValues()) { gsonInstanceWithNullsAllowed().toJson(v, LDValue.class, out); } out.endArray(); diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java index d7893441..b018833b 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelPreprocessingTest.java @@ -15,6 +15,7 @@ import com.launchdarkly.sdk.server.DataModel.Target; import com.launchdarkly.sdk.server.DataModelPreprocessing.ClausePreprocessed; +import org.junit.After; import org.junit.Test; import java.time.Instant; @@ -284,66 +285,41 @@ public void preprocessFlagKeepsClauseValuesListByDefault() { Clause pc = f.getRules().get(0).getClauses().get(0); assertNotNull(pc.preprocessed.valuesSet); assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0), LDValue.of("a")), pc.getValues()); - assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0), LDValue.of("a")), - ImmutableList.copyOf(pc.getAllValues())); } - @Test - public void preprocessFlagReleasesClauseValuesListWhenReleaseIsEnabled() { - DataModelPreprocessing.setReleaseClauseValues(true); - try { - Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of(0), LDValue.of("a")); - - FeatureFlag f = flagFromClause(c); - f.afterDeserialized(); - - Clause pc = f.getRules().get(0).getClauses().get(0); - assertEquals(ImmutableList.of(), pc.getValues()); - assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of(0)), pc.preprocessed.valuesSet); - // the set keeps first-appearance order, so getAllValues() drops only the duplicate - assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0)), - ImmutableList.copyOf(pc.getAllValues())); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + @After + public void restoreReleaseClauseValues() { + DataModelPreprocessing.setReleaseClauseValues( + Boolean.getBoolean(DataModelPreprocessing.RELEASE_CLAUSE_VALUES_PROPERTY)); } @Test - public void preprocessIsIdempotentWhenClauseValuesListIsReleased() { - // A clause is normally preprocessed only once. The guard makes a repeat pass safe: it must - // not replace the value set with one built from the released (empty) list. + public void preprocessFlagReleasesClauseValuesListWhenReleaseIsEnabled() { DataModelPreprocessing.setReleaseClauseValues(true); - try { - Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of("b")); - FeatureFlag f = flagFromClause(c); - f.afterDeserialized(); - f.afterDeserialized(); + Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of(0), LDValue.of("a")); - Clause pc = f.getRules().get(0).getClauses().get(0); - assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of("b")), pc.preprocessed.valuesSet); - assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of("b")), - ImmutableList.copyOf(pc.getAllValues())); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of(0)), pc.preprocessed.valuesSet); + // the set's list view replaces the list, so the duplicate value is gone + assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of(0)), pc.getValues()); } @Test public void releaseKeepsClauseValuesListWhenThereIsNoValueSet() { DataModelPreprocessing.setReleaseClauseValues(true); - try { - Clause c = clause("x", Operator.in, LDValue.of("a")); - FeatureFlag f = flagFromClause(c); - f.afterDeserialized(); + Clause c = clause("x", Operator.in, LDValue.of("a")); - Clause pc = f.getRules().get(0).getClauses().get(0); - assertNull(pc.preprocessed); - assertEquals(ImmutableList.of(LDValue.of("a")), pc.getValues()); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertNull(pc.preprocessed); + assertEquals(ImmutableList.of(LDValue.of("a")), pc.getValues()); } @Test @@ -351,40 +327,34 @@ public void releaseKeepsClauseValuesListForNonEqualityOperators() { // Evaluation and dependency analysis read the list directly for every operator other than // "in" (this includes segmentMatch), so the release must not touch these clauses. DataModelPreprocessing.setReleaseClauseValues(true); - try { - for (Operator op: Operator.getBuiltins()) { - if (op == Operator.in) { - continue; - } - Clause c = clause("x", op, LDValue.of("a"), LDValue.of("b")); - - FeatureFlag f = flagFromClause(c); - f.afterDeserialized(); - - Clause pc = f.getRules().get(0).getClauses().get(0); - assertEquals(op.toString(), ImmutableList.of(LDValue.of("a"), LDValue.of("b")), pc.getValues()); + + for (Operator op: Operator.getBuiltins()) { + if (op == Operator.in) { + continue; } - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); + Clause c = clause("x", op, LDValue.of("a"), LDValue.of("b")); + + FeatureFlag f = flagFromClause(c); + f.afterDeserialized(); + + Clause pc = f.getRules().get(0).getClauses().get(0); + assertEquals(op.toString(), ImmutableList.of(LDValue.of("a"), LDValue.of("b")), pc.getValues()); } } @Test public void releaseAppliesToSegmentRuleClauses() { DataModelPreprocessing.setReleaseClauseValues(true); - try { - Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of("b")); - SegmentRule rule = segmentRuleBuilder().clauses(c).build(); - Segment s = segmentBuilder("key").disablePreprocessing(true).rules(rule).build(); - - s.afterDeserialized(); - - Clause pc = s.getRules().get(0).getClauses().get(0); - assertEquals(ImmutableList.of(), pc.getValues()); - assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of("b")), pc.preprocessed.valuesSet); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + + Clause c = clause("x", Operator.in, LDValue.of("a"), LDValue.of("b"), LDValue.of("a")); + SegmentRule rule = segmentRuleBuilder().clauses(c).build(); + Segment s = segmentBuilder("key").disablePreprocessing(true).rules(rule).build(); + + s.afterDeserialized(); + + Clause pc = s.getRules().get(0).getClauses().get(0); + assertEquals(ImmutableList.of(LDValue.of("a"), LDValue.of("b")), pc.getValues()); + assertEquals(ImmutableSet.of(LDValue.of("a"), LDValue.of("b")), pc.preprocessed.valuesSet); } @Test diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java index 667cd484..342f1e39 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelSerializationTest.java @@ -24,6 +24,7 @@ import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; import com.launchdarkly.sdk.server.subsystems.SerializationException; +import org.junit.After; import org.junit.Test; import java.util.Collections; @@ -611,79 +612,81 @@ public void parsingFullDataSetFlagsAndSegments() throws Exception { .addAny(FEATURES, flag1, flag2).addAny(SEGMENTS, segment1, segment2).build(), allData); } + @After + public void restoreReleaseClauseValues() { + DataModelPreprocessing.setReleaseClauseValues( + Boolean.getBoolean(DataModelPreprocessing.RELEASE_CLAUSE_VALUES_PROPERTY)); + } + @Test public void clauseWithReleasedValuesListIsSerializedFromValueSet() { - // With the release enabled, an "in" clause with 2 or more values drops its value list and the - // serializer writes the values from the preprocessed set. The set keeps first-appearance - // order, so the output keeps the original order and drops only the duplicate value. + // With the release enabled, an "in" clause with two or more values replaces its value list + // with the preprocessed set's list view. The view keeps first-appearance order, so the + // serialized output keeps the original order and drops only the duplicate value. DataModelPreprocessing.setReleaseClauseValues(true); - try { - LDValue clauseJson = LDValue.buildObject() - .put("attribute", "name") - .put("op", "in") - .put("values", LDValue.buildArray() - .add("a").add("b").add(1).add(true).add("a") - .build()) - .put("negate", false) - .build(); - assertClauseFromJson(clauseJson, c -> { - assertEquals(0, c.getValues().size()); - }); - LDValue flagJson = baseBuilder("flag-key") - .put("rules", LDValue.buildArray() - .add(LDValue.buildObject() - .put("id", "ruleid") - .put("clauses", LDValue.arrayOf(clauseJson)) - .build()) - .build()) - .build(); - assertFlagFromJson(flagJson, flag -> { - String json = FEATURES.serialize(new ItemDescriptor(flag.getVersion(), flag)); - LDValue values = LDValue.parse(json) - .get("rules").get(0).get("clauses").get(0).get("values"); - assertEquals(LDValue.buildArray().add("a").add("b").add(1).add(true).build(), values); - - // the released form round-trips: parsing the new JSON gives the same values again - assertFlagFromJson(LDValue.parse(json), flag2 -> { - String json2 = FEATURES.serialize(new ItemDescriptor(flag2.getVersion(), flag2)); - assertJsonEquals(json, json2); - }); + LDValue clauseJson = LDValue.buildObject() + .put("attribute", "name") + .put("op", "in") + .put("values", LDValue.buildArray() + .add("a").add("b").add(1).add(true).add("a") + .build()) + .put("negate", false) + .build(); + assertClauseFromJson(clauseJson, c -> { + // the release dropped the duplicate "a", so 4 of the 5 input values remain + assertEquals(4, c.getValues().size()); + }); + + LDValue flagJson = baseBuilder("flag-key") + .put("rules", LDValue.buildArray() + .add(LDValue.buildObject() + .put("id", "ruleid") + .put("clauses", LDValue.arrayOf(clauseJson)) + .build()) + .build()) + .build(); + assertFlagFromJson(flagJson, flag -> { + String json = FEATURES.serialize(new ItemDescriptor(flag.getVersion(), flag)); + LDValue values = LDValue.parse(json) + .get("rules").get(0).get("clauses").get(0).get("values"); + assertEquals(LDValue.buildArray().add("a").add("b").add(1).add(true).build(), values); + + // Parsing the released JSON gives the same values again. + assertFlagFromJson(LDValue.parse(json), flag2 -> { + String json2 = FEATURES.serialize(new ItemDescriptor(flag2.getVersion(), flag2)); + assertJsonEquals(json, json2); }); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + }); } @Test public void segmentClauseWithReleasedValuesListIsSerializedFromValueSet() { DataModelPreprocessing.setReleaseClauseValues(true); - try { - LDValue segmentJson = baseBuilder("segment-key") - .put("rules", LDValue.buildArray() - .add(LDValue.buildObject() - .put("clauses", LDValue.buildArray() - .add(LDValue.buildObject() - .put("attribute", "name") - .put("op", "in") - .put("values", LDValue.buildArray().add("a").add("b").add("a").build()) - .put("negate", false) - .build()) - .build()) - .build()) - .build()) - .build(); - assertSegmentFromJson(segmentJson, segment -> { - assertEquals(0, segment.getRules().get(0).getClauses().get(0).getValues().size()); - - String json = SEGMENTS.serialize(new ItemDescriptor(segment.getVersion(), segment)); - LDValue values = LDValue.parse(json) - .get("rules").get(0).get("clauses").get(0).get("values"); - assertEquals(LDValue.buildArray().add("a").add("b").build(), values); - }); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + + LDValue segmentJson = baseBuilder("segment-key") + .put("rules", LDValue.buildArray() + .add(LDValue.buildObject() + .put("clauses", LDValue.buildArray() + .add(LDValue.buildObject() + .put("attribute", "name") + .put("op", "in") + .put("values", LDValue.buildArray().add("a").add("b").add("a").build()) + .put("negate", false) + .build()) + .build()) + .build()) + .build()) + .build(); + assertSegmentFromJson(segmentJson, segment -> { + // the release dropped the duplicate "a", so 2 of the 3 input values remain + assertEquals(2, segment.getRules().get(0).getClauses().get(0).getValues().size()); + + String json = SEGMENTS.serialize(new ItemDescriptor(segment.getVersion(), segment)); + LDValue values = LDValue.parse(json) + .get("rules").get(0).get("clauses").get(0).get("values"); + assertEquals(LDValue.buildArray().add("a").add("b").build(), values); + }); } @Test @@ -706,7 +709,7 @@ public void clauseValuesAreSerializedUnchangedByDefault() { String json = FEATURES.serialize(new ItemDescriptor(flag.getVersion(), flag)); LDValue values = LDValue.parse(json) .get("rules").get(0).get("clauses").get(0).get("values"); - // the duplicate value and the original order are kept + // serialization keeps the duplicate value and the original order assertEquals(LDValue.buildArray().add("a").add("b").add("a").build(), values); }); } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java index ce564248..0f2a374d 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorClauseTest.java @@ -12,6 +12,7 @@ import com.launchdarkly.sdk.server.DataModel.Rule; import com.launchdarkly.sdk.server.DataModel.Segment; +import org.junit.After; import org.junit.Test; import static com.launchdarkly.sdk.EvaluationDetail.fromValue; @@ -302,20 +303,23 @@ public void testSegmentMatchClauseIgnoresNonStringValues() throws Exception { assertMatch(e, flag, context, true); } + @After + public void restoreReleaseClauseValues() { + DataModelPreprocessing.setReleaseClauseValues( + Boolean.getBoolean(DataModelPreprocessing.RELEASE_CLAUSE_VALUES_PROPERTY)); + } + @Test public void clauseCanMatchWhenValuesListIsReleased() throws Exception { DataModelPreprocessing.setReleaseClauseValues(true); - try { - Clause clause = clause("name", Operator.in, LDValue.of("Bob"), LDValue.of("Carol")); - FeatureFlag f = booleanFlagWithClauses("flag", clause); - // the release dropped the list, so this match can only come from the value set - assertEquals(0, f.getRules().get(0).getClauses().get(0).getValues().size()); - assertMatch(BASE_EVALUATOR, f, LDContext.builder("key").name("Carol").build(), true); - assertMatch(BASE_EVALUATOR, f, LDContext.builder("key").name("Don").build(), false); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + Clause clause = clause("name", Operator.in, LDValue.of("Bob"), LDValue.of("Carol"), LDValue.of("Bob")); + FeatureFlag f = booleanFlagWithClauses("flag", clause); + // the release replaced the list with the set's view, which drops the duplicate "Bob" + assertEquals(2, f.getRules().get(0).getClauses().get(0).getValues().size()); + + assertMatch(BASE_EVALUATOR, f, LDContext.builder("key").name("Carol").build(), true); + assertMatch(BASE_EVALUATOR, f, LDContext.builder("key").name("Don").build(), false); } @Test @@ -323,20 +327,17 @@ public void segmentMatchClauseStillMatchesWhenReleaseIsEnabled() throws Exceptio // The release must not touch segmentMatch clauses, because segment matching reads the // values list directly. DataModelPreprocessing.setReleaseClauseValues(true); - try { - String segmentKey = "segkey"; - Clause inClause = clause("name", Operator.in, LDValue.of("Bob"), LDValue.of("Carol")); - Clause segmentClause = clauseMatchingSegment(segmentKey); - FeatureFlag flag = booleanFlagWithClauses("flag", segmentClause); - FeatureFlag flagWithBoth = booleanFlagWithClauses("flag2", inClause, segmentClause); - Segment segment = makeSegmentThatMatchesUser(segmentKey, "foo"); - Evaluator e = evaluatorBuilder().withStoredSegments(segment).build(); - assertMatch(e, flag, LDContext.create("foo"), true); - assertMatch(e, flag, LDContext.create("bar"), false); - assertMatch(e, flagWithBoth, LDContext.builder("foo").name("Carol").build(), true); - } finally { - DataModelPreprocessing.setReleaseClauseValues(false); - } + String segmentKey = "segkey"; + Clause inClause = clause("name", Operator.in, LDValue.of("Bob"), LDValue.of("Carol")); + Clause segmentClause = clauseMatchingSegment(segmentKey); + FeatureFlag flag = booleanFlagWithClauses("flag", segmentClause); + FeatureFlag flagWithBoth = booleanFlagWithClauses("flag2", inClause, segmentClause); + Segment segment = makeSegmentThatMatchesUser(segmentKey, "foo"); + + Evaluator e = evaluatorBuilder().withStoredSegments(segment).build(); + assertMatch(e, flag, LDContext.create("foo"), true); + assertMatch(e, flag, LDContext.create("bar"), false); + assertMatch(e, flagWithBoth, LDContext.builder("foo").name("Carol").build(), true); } } From 25d8e2d06742d2e3cd4af9b904fe33f66b534d31 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 16 Oct 2026 17:52:59 -0400 Subject: [PATCH 4/4] fix: handles the case if java security policy denies access to sys prop --- .../sdk/server/DataModelPreprocessing.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java index c96f55ea..2a73685c 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModelPreprocessing.java @@ -55,7 +55,17 @@ private DataModelPreprocessing() {} */ static final String RELEASE_CLAUSE_VALUES_PROPERTY = "com.launchdarkly.sdk.server.releaseClauseValues"; - private static volatile boolean releaseClauseValues = Boolean.getBoolean(RELEASE_CLAUSE_VALUES_PROPERTY); + private static volatile boolean releaseClauseValues = readReleaseClauseValues(); + + // A security policy can deny reads of non-standard system properties. + // A denied read means the feature stays off. + private static boolean readReleaseClauseValues() { + try { + return Boolean.getBoolean(RELEASE_CLAUSE_VALUES_PROPERTY); + } catch (SecurityException e) { + return false; + } + } // For tests. Production code reads the system property once, at class load. static void setReleaseClauseValues(boolean enabled) {