From df9cbeb23c1cc29336aa3beb0dfc1661886bd5ed Mon Sep 17 00:00:00 2001 From: Miguel Lopez <4165471+lopezm1@users.noreply.github.com> Date: Fri, 30 Oct 2026 05:54:44 -0700 Subject: [PATCH 1/4] perf: index list targeting once per sync Included users, segment included/excluded keys and IsOneOf/NotOneOf values were scanned on every evaluation, and IsOneOf values were also re-parsed from JSON each time. Build each list into a set in afterDeserialization() instead, falling back to the original parse when a value isn't a JSON list. --- .../java/co/featbit/server/DataModel.java | 64 +++- .../java/co/featbit/server/EvaluatorImp.java | 14 +- .../co/featbit/server/ListTargetingTest.java | 328 ++++++++++++++++++ 3 files changed, 397 insertions(+), 9 deletions(-) create mode 100644 src/test/java/co/featbit/server/ListTargetingTest.java diff --git a/src/main/java/co/featbit/server/DataModel.java b/src/main/java/co/featbit/server/DataModel.java index a3498fd..edf4d79 100644 --- a/src/main/java/co/featbit/server/DataModel.java +++ b/src/main/java/co/featbit/server/DataModel.java @@ -11,8 +11,10 @@ import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; public abstract class DataModel { @@ -172,6 +174,8 @@ static class Segment implements DataStorageTypes.Item, JsonHelper.AfterJsonParse private final List included; private final List excluded; private final List rules; + private transient Set includedSet; + private transient Set excludedSet; Segment(String id, Boolean isArchived, Date updatedAt, List included, List excluded, List rules) { this.id = id; @@ -215,17 +219,25 @@ public List getRules() { } public Boolean isMatchUser(String userKeyId) { - if (getExcluded().contains(userKeyId)) { + if (excludedSet().contains(userKeyId)) { return Boolean.FALSE; } - if (getIncluded().contains(userKeyId)) { + if (includedSet().contains(userKeyId)) { return Boolean.TRUE; } // if no included or excluded, then it's to match rules return null; } + private Set includedSet() { + return includedSet == null ? toSet(getIncluded()) : includedSet; + } + + private Set excludedSet() { + return excludedSet == null ? toSet(getExcluded()) : excludedSet; + } + public DataStorageTypes.Item toArchivedItem() { return new ArchivedItem(this.id, this.timestamp); } @@ -233,6 +245,8 @@ public DataStorageTypes.Item toArchivedItem() { @Override public void afterDeserialization() { this.timestamp = updatedAt.getTime(); + this.includedSet = toSet(getIncluded()); + this.excludedSet = toSet(getExcluded()); } @Override @@ -391,11 +405,14 @@ public String getValue() { } } - static final class TargetUser { + @JsonAdapter(JsonHelper.AfterJsonParseDeserializableTypeAdapterFactory.class) + static final class TargetUser implements JsonHelper.AfterJsonParseDeserializable { private final List keyIds; private final String variationId; + private transient Set keyIdSet; + public TargetUser(List keyIds, String variationId) { this.keyIds = keyIds; this.variationId = variationId; @@ -410,7 +427,12 @@ public String getVariationId() { } public boolean isTargeted(String user) { - return getKeyIds().stream().anyMatch(key -> key.equals(user)); + return (keyIdSet == null ? toSet(getKeyIds()) : keyIdSet).contains(user); + } + + @Override + public void afterDeserialization() { + this.keyIdSet = toSet(getKeyIds()); } } @@ -447,13 +469,17 @@ public String getDispatchKey() { } } - static final class Condition { + @JsonAdapter(JsonHelper.AfterJsonParseDeserializableTypeAdapterFactory.class) + static final class Condition implements JsonHelper.AfterJsonParseDeserializable { private final String property; private final String op; private final String value; + // the parsed value of a list operator, built once per sync; null when the value is not a list + private transient Set valueSet; + Condition(String property, String op, String value) { this.property = property; this.op = op; @@ -471,6 +497,34 @@ public String getOp() { public String getValue() { return value; } + + Set getValueSet() { + return valueSet; + } + + @Override + public void afterDeserialization() { + // segment conditions have no op, see EvaluatorImp.ifUserMatchClause + String clause = StringUtils.isBlank(op) ? property : op; + if (!Evaluator.IS_ONE_OF_CLAUSE.equals(clause) + && !Evaluator.NOT_ONE_OF_CLAUSE.equals(clause) + && !Evaluator.IS_IN_SEGMENT_CLAUSE.equals(clause) + && !Evaluator.NOT_IN_SEGMENT_CLAUSE.equals(clause)) { + return; + } + try { + List values = JsonHelper.deserialize(value, new TypeToken>() { + }.getType()); + valueSet = values == null ? null : toSet(values); + } catch (RuntimeException e) { + // leave it to evaluation, which keeps the original behavior for an invalid value + valueSet = null; + } + } + } + + private static Set toSet(List values) { + return Collections.unmodifiableSet(new HashSet<>(values)); } static final class RolloutVariation { diff --git a/src/main/java/co/featbit/server/EvaluatorImp.java b/src/main/java/co/featbit/server/EvaluatorImp.java index 84ed600..b550ae3 100644 --- a/src/main/java/co/featbit/server/EvaluatorImp.java +++ b/src/main/java/co/featbit/server/EvaluatorImp.java @@ -138,8 +138,11 @@ private boolean ifUserMatchClause(FBUser user, DataModel.Condition condition) { private boolean inSegmentClause(FBUser user, DataModel.Condition condition) { String pv = user.getKey(); try { - List segments = JsonHelper.deserialize(condition.getValue(), new TypeToken>() { - }.getType()); + Collection segments = condition.getValueSet(); + if (segments == null) { + segments = JsonHelper.deserialize(condition.getValue(), new TypeToken>() { + }.getType()); + } return segments.stream().map(segmentGetter::get).anyMatch(segment -> { if (segment == null) { return false; @@ -223,8 +226,11 @@ private boolean containsClause(FBUser user, DataModel.Condition condition) { private boolean oneOfClause(FBUser user, DataModel.Condition condition) { String pv = user.getProperty(condition.getProperty()); try { - List clauseValues = JsonHelper.deserialize(condition.getValue(), new TypeToken>() { - }.getType()); + Collection clauseValues = condition.getValueSet(); + if (clauseValues == null) { + clauseValues = JsonHelper.deserialize(condition.getValue(), new TypeToken>() { + }.getType()); + } return pv != null && clauseValues.contains(pv); } catch (JsonParseException e) { return false; diff --git a/src/test/java/co/featbit/server/ListTargetingTest.java b/src/test/java/co/featbit/server/ListTargetingTest.java new file mode 100644 index 0000000..46e057f --- /dev/null +++ b/src/test/java/co/featbit/server/ListTargetingTest.java @@ -0,0 +1,328 @@ +package co.featbit.server; + +import co.featbit.commons.model.EvalDetail; +import co.featbit.commons.model.FBUser; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.IntStream; + +import static co.featbit.server.EvaluationReason.REASON_ERROR; +import static co.featbit.server.EvaluationReason.REASON_FALLTHROUGH; +import static co.featbit.server.EvaluationReason.REASON_RULE_MATCH; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Included users, segment included/excluded keys and IsOneOf/NotOneOf values are indexed when data is synced. + * These tests check that evaluation follows each replacement of that data, including under concurrent evaluation. + */ +class ListTargetingTest { + private static final String FLAG = "list-flag"; + private static final String SEGMENT_ID = "segment-id"; + private static final String ON = "on"; + private static final String OFF = "off"; + private static final Instant FIRST_UPDATE = Instant.parse("2024-01-01T00:00:00Z"); + + private FBClientImp client; + private int generation; + + @BeforeEach + void init() { + client = new FBClientImp("", new FBConfig.Builder() + .offline(true) + .streamingURL("http://localhost:1") + .eventURL("http://localhost:1") + .build()); + } + + @AfterEach + void dispose() throws IOException { + client.close(); + } + + @Test + void testIsOneOfInSegmentFollowsReplacedValues() { + syncSegment(userIdRule("IsOneOf", 0, 1, 2), Collections.emptyList(), Collections.emptyList()); + assertTrue(evaluate(1)); + assertFalse(evaluate(9)); + + syncSegment(userIdRule("IsOneOf", 2, 9), Collections.emptyList(), Collections.emptyList()); + assertFalse(evaluate(1)); + assertTrue(evaluate(2)); + assertTrue(evaluate(9)); + } + + @Test + void testNotOneOfFollowsReplacedValues() { + syncFlagRule(condition("user_id", "NotOneOf", userIds(0, 1))); + assertFalse(evaluate(1)); + assertTrue(evaluate(9)); + + syncFlagRule(condition("user_id", "NotOneOf", userIds(9))); + assertTrue(evaluate(1)); + assertFalse(evaluate(9)); + } + + @Test + void testIncludedUsersFollowReplacedKeys() { + syncFlag(targetUsers(0, 1), new JsonArray(), new JsonArray()); + assertTrue(evaluate(1)); + assertFalse(evaluate(5)); + + syncFlag(targetUsers(5), new JsonArray(), new JsonArray()); + assertFalse(evaluate(1)); + assertTrue(evaluate(5)); + } + + @Test + void testSegmentExcludedKeysWinAndFollowReplacement() { + syncSegment(null, Arrays.asList(key(0), key(1)), Collections.singletonList(key(1))); + assertTrue(evaluate(0)); + assertFalse(evaluate(1)); + assertFalse(evaluate(2)); + + syncSegment(null, Collections.singletonList(key(1)), Collections.emptyList()); + assertFalse(evaluate(0)); + assertTrue(evaluate(1)); + } + + @Test + void testOtherOperatorsStillUseTheRawValue() { + syncFlagRule(condition("user_id", "Equal", userId(3))); + assertTrue(evaluate(3)); + assertFalse(evaluate(4)); + + syncFlagRule(condition("user_id", "MatchRegex", "user-1[0-9]")); + assertFalse(evaluate(3)); + assertTrue(evaluate(11)); + } + + @Test + void testInvalidIsOneOfValueReturnsTheDefaultWithAnError() { + syncFlagRule(condition("user_id", "IsOneOf", "not a list")); + EvalDetail detail = detail(1, true); + assertTrue(detail.getVariation()); + assertEquals(REASON_ERROR, detail.getReason()); + } + + @Test + void testConcurrentEvaluationsDuringReplacements() throws Exception { + int size = 1000; + int[] first = IntStream.range(0, size).toArray(); + int[] second = IntStream.range(size, 2 * size).toArray(); + syncSegment(userIdRule("IsOneOf", first), Collections.emptyList(), Collections.emptyList()); + + AtomicBoolean done = new AtomicBoolean(); + ExecutorService readers = Executors.newFixedThreadPool(4); + List> results = new ArrayList<>(); + try { + for (int t = 0; t < 4; t++) { + results.add(readers.submit(() -> { + int evaluations = 0; + while (!done.get()) { + for (int index : new int[]{0, size - 1, size, 2 * size - 1}) { + String reason = detail(index, false).getReason(); + if (!reason.equals(REASON_RULE_MATCH) && !reason.equals(REASON_FALLTHROUGH)) { + throw new AssertionError("user " + index + ": " + reason); + } + evaluations++; + } + } + return evaluations; + })); + } + for (int i = 0; i < 200; i++) { + syncSegment(userIdRule("IsOneOf", i % 2 == 0 ? second : first), Collections.emptyList(), Collections.emptyList()); + } + } finally { + done.set(true); + readers.shutdown(); + } + for (Future result : results) { + assertTrue(result.get() > 0); + } + // an even number of replacements ends on the first list + assertTrue(evaluate(0)); + assertFalse(evaluate(size)); + } + + private boolean evaluate(int index) { + return detail(index, false).getVariation(); + } + + private EvalDetail detail(int index, boolean defaultValue) { + FBUser user = new FBUser.Builder(key(index)).userName(key(index)).custom("user_id", userId(index)).build(); + return client.boolVariationDetail(FLAG, user, defaultValue); + } + + private void syncSegment(JsonObject rule, List included, List excluded) { + JsonObject segment = new JsonObject(); + segment.addProperty("id", SEGMENT_ID); + segment.addProperty("isArchived", false); + segment.add("included", strings(included)); + segment.add("excluded", strings(excluded)); + JsonArray rules = new JsonArray(); + if (rule != null) { + rules.add(rule); + } + segment.add("rules", rules); + JsonArray segments = new JsonArray(); + segments.add(segment); + + JsonArray segmentIds = new JsonArray(); + segmentIds.add(SEGMENT_ID); + JsonArray flagRules = new JsonArray(); + flagRules.add(flagRule(condition("User is in segment", "", segmentIds.toString()))); + syncFlag(new JsonArray(), flagRules, segments); + } + + private void syncFlagRule(JsonObject condition) { + JsonArray rules = new JsonArray(); + rules.add(flagRule(condition)); + syncFlag(new JsonArray(), rules, new JsonArray()); + } + + private void syncFlag(JsonArray targetUsers, JsonArray rules, JsonArray segments) { + // the SDK only accepts data newer than what it holds + String updatedAt = FIRST_UPDATE.plusSeconds(++generation).toString(); + JsonObject fallthrough = new JsonObject(); + fallthrough.addProperty("dispatchKey", "keyId"); + fallthrough.addProperty("includedInExpt", true); + fallthrough.add("variations", rollout(OFF)); + + JsonArray variations = new JsonArray(); + variations.add(variation(ON, "true")); + variations.add(variation(OFF, "false")); + + JsonObject flag = new JsonObject(); + flag.addProperty("id", FLAG + "-id"); + flag.addProperty("key", FLAG); + flag.addProperty("name", FLAG); + flag.addProperty("updatedAt", updatedAt); + flag.addProperty("isEnabled", true); + flag.addProperty("variationType", "boolean"); + flag.add("variations", variations); + flag.addProperty("disabledVariationId", OFF); + flag.add("targetUsers", targetUsers); + flag.add("rules", rules); + flag.add("fallthrough", fallthrough); + flag.addProperty("exptIncludeAllTargets", true); + JsonArray flags = new JsonArray(); + flags.add(flag); + for (JsonElement segment : segments) { + segment.getAsJsonObject().addProperty("updatedAt", updatedAt); + } + + JsonObject data = new JsonObject(); + data.addProperty("eventType", "full"); + data.add("featureFlags", flags); + data.add("segments", segments); + JsonObject message = new JsonObject(); + message.addProperty("messageType", "data-sync"); + message.add("data", data); + assertTrue(client.initializeFromExternalJson(message.toString())); + } + + private static JsonArray targetUsers(int... indexes) { + JsonArray keyIds = new JsonArray(); + for (int index : indexes) { + keyIds.add(key(index)); + } + JsonObject targetUser = new JsonObject(); + targetUser.add("keyIds", keyIds); + targetUser.addProperty("variationId", ON); + JsonArray targetUsers = new JsonArray(); + targetUsers.add(targetUser); + return targetUsers; + } + + private static JsonObject userIdRule(String op, int... indexes) { + JsonArray conditions = new JsonArray(); + conditions.add(condition("user_id", op, userIds(indexes))); + JsonObject rule = new JsonObject(); + rule.addProperty("id", "user-id-rule"); + rule.addProperty("name", "user ids"); + rule.add("conditions", conditions); + return rule; + } + + private static JsonObject flagRule(JsonObject condition) { + JsonArray conditions = new JsonArray(); + conditions.add(condition); + JsonObject rule = new JsonObject(); + rule.addProperty("id", "flag-rule"); + rule.addProperty("name", "flag rule"); + rule.addProperty("dispatchKey", "keyId"); + rule.addProperty("includedInExpt", false); + rule.add("conditions", conditions); + rule.add("variations", rollout(ON)); + return rule; + } + + private static JsonObject condition(String property, String op, String value) { + JsonObject condition = new JsonObject(); + condition.addProperty("property", property); + condition.addProperty("op", op); + condition.addProperty("value", value); + return condition; + } + + private static JsonArray rollout(String variationId) { + JsonArray range = new JsonArray(); + range.add(0); + range.add(1); + JsonObject variation = new JsonObject(); + variation.addProperty("id", variationId); + variation.add("rollout", range); + variation.addProperty("exptRollout", 1); + JsonArray variations = new JsonArray(); + variations.add(variation); + return variations; + } + + private static JsonObject variation(String id, String value) { + JsonObject variation = new JsonObject(); + variation.addProperty("id", id); + variation.addProperty("name", value); + variation.addProperty("value", value); + return variation; + } + + private static String userIds(int... indexes) { + JsonArray userIds = new JsonArray(); + for (int index : indexes) { + userIds.add(userId(index)); + } + return userIds.toString(); + } + + private static JsonArray strings(List values) { + JsonArray array = new JsonArray(); + values.forEach(array::add); + return array; + } + + private static String key(int index) { + return "key-" + index; + } + + private static String userId(int index) { + return "user-" + index; + } +} From a45404200aba837b3b79260f2931c80494267b06 Mon Sep 17 00:00:00 2001 From: Miguel Lopez <4165471+lopezm1@users.noreply.github.com> Date: Fri, 30 Oct 2026 06:11:50 -0700 Subject: [PATCH 2/4] fix: check segments in their listed order Evaluation stops at the first matching segment, so keep the parsed segment IDs as a list instead of a set. With a set, a later segment whose value is invalid could be checked first and turn a match into an evaluation error. --- .../java/co/featbit/server/DataModel.java | 19 +++--- .../java/co/featbit/server/EvaluatorImp.java | 4 +- .../co/featbit/server/ListTargetingTest.java | 61 +++++++++++++++---- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/main/java/co/featbit/server/DataModel.java b/src/main/java/co/featbit/server/DataModel.java index edf4d79..e3b7c81 100644 --- a/src/main/java/co/featbit/server/DataModel.java +++ b/src/main/java/co/featbit/server/DataModel.java @@ -9,6 +9,7 @@ import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashSet; @@ -478,7 +479,7 @@ static final class Condition implements JsonHelper.AfterJsonParseDeserializable private final String value; // the parsed value of a list operator, built once per sync; null when the value is not a list - private transient Set valueSet; + private transient Collection parsedValues; Condition(String property, String op, String value) { this.property = property; @@ -498,27 +499,27 @@ public String getValue() { return value; } - Set getValueSet() { - return valueSet; + Collection getParsedValues() { + return parsedValues; } @Override public void afterDeserialization() { // segment conditions have no op, see EvaluatorImp.ifUserMatchClause String clause = StringUtils.isBlank(op) ? property : op; - if (!Evaluator.IS_ONE_OF_CLAUSE.equals(clause) - && !Evaluator.NOT_ONE_OF_CLAUSE.equals(clause) - && !Evaluator.IS_IN_SEGMENT_CLAUSE.equals(clause) - && !Evaluator.NOT_IN_SEGMENT_CLAUSE.equals(clause)) { + boolean oneOf = Evaluator.IS_ONE_OF_CLAUSE.equals(clause) || Evaluator.NOT_ONE_OF_CLAUSE.equals(clause); + boolean inSegment = Evaluator.IS_IN_SEGMENT_CLAUSE.equals(clause) || Evaluator.NOT_IN_SEGMENT_CLAUSE.equals(clause); + if (!oneOf && !inSegment) { return; } try { List values = JsonHelper.deserialize(value, new TypeToken>() { }.getType()); - valueSet = values == null ? null : toSet(values); + // segments are checked in the listed order and evaluation stops at the first match, so keep that order + parsedValues = values == null ? null : oneOf ? toSet(values) : Collections.unmodifiableList(values); } catch (RuntimeException e) { // leave it to evaluation, which keeps the original behavior for an invalid value - valueSet = null; + parsedValues = null; } } } diff --git a/src/main/java/co/featbit/server/EvaluatorImp.java b/src/main/java/co/featbit/server/EvaluatorImp.java index b550ae3..aa4afbe 100644 --- a/src/main/java/co/featbit/server/EvaluatorImp.java +++ b/src/main/java/co/featbit/server/EvaluatorImp.java @@ -138,7 +138,7 @@ private boolean ifUserMatchClause(FBUser user, DataModel.Condition condition) { private boolean inSegmentClause(FBUser user, DataModel.Condition condition) { String pv = user.getKey(); try { - Collection segments = condition.getValueSet(); + Collection segments = condition.getParsedValues(); if (segments == null) { segments = JsonHelper.deserialize(condition.getValue(), new TypeToken>() { }.getType()); @@ -226,7 +226,7 @@ private boolean containsClause(FBUser user, DataModel.Condition condition) { private boolean oneOfClause(FBUser user, DataModel.Condition condition) { String pv = user.getProperty(condition.getProperty()); try { - Collection clauseValues = condition.getValueSet(); + Collection clauseValues = condition.getParsedValues(); if (clauseValues == null) { clauseValues = JsonHelper.deserialize(condition.getValue(), new TypeToken>() { }.getType()); diff --git a/src/test/java/co/featbit/server/ListTargetingTest.java b/src/test/java/co/featbit/server/ListTargetingTest.java index 46e057f..093cad3 100644 --- a/src/test/java/co/featbit/server/ListTargetingTest.java +++ b/src/test/java/co/featbit/server/ListTargetingTest.java @@ -102,6 +102,31 @@ void testSegmentExcludedKeysWinAndFollowReplacement() { assertTrue(evaluate(1)); } + @Test + void testSegmentsInOneConditionAreCheckedInListedOrder() { + // a later segment with an invalid value must not be reached once an earlier one matches + for (int pair = 0; pair < 20; pair++) { + String matching = "segment-" + (2 * pair); + String invalid = "segment-" + (2 * pair + 1); + JsonArray segments = new JsonArray(); + segments.add(segment(matching, Collections.singletonList(key(1)), new JsonArray())); + JsonArray invalidRules = new JsonArray(); + invalidRules.add(segmentRule(condition("user_id", "IsOneOf", "not a list"))); + segments.add(segment(invalid, Collections.emptyList(), invalidRules)); + + JsonArray segmentIds = new JsonArray(); + segmentIds.add(matching); + segmentIds.add(invalid); + JsonArray rules = new JsonArray(); + rules.add(flagRule(condition("User is in segment", "", segmentIds.toString()))); + syncFlag(new JsonArray(), rules, segments); + + EvalDetail detail = detail(1, false); + assertTrue(detail.getVariation(), "pair " + pair + ": " + detail.getReason()); + assertEquals(REASON_RULE_MATCH, detail.getReason()); + } + } + @Test void testOtherOperatorsStillUseTheRawValue() { syncFlagRule(condition("user_id", "Equal", userId(3))); @@ -172,16 +197,12 @@ private EvalDetail detail(int index, boolean defaultValue) { } private void syncSegment(JsonObject rule, List included, List excluded) { - JsonObject segment = new JsonObject(); - segment.addProperty("id", SEGMENT_ID); - segment.addProperty("isArchived", false); - segment.add("included", strings(included)); - segment.add("excluded", strings(excluded)); JsonArray rules = new JsonArray(); if (rule != null) { rules.add(rule); } - segment.add("rules", rules); + JsonObject segment = segment(SEGMENT_ID, included, rules); + segment.add("excluded", strings(excluded)); JsonArray segments = new JsonArray(); segments.add(segment); @@ -192,6 +213,26 @@ private void syncSegment(JsonObject rule, List included, List ex syncFlag(new JsonArray(), flagRules, segments); } + private static JsonObject segment(String id, List included, JsonArray rules) { + JsonObject segment = new JsonObject(); + segment.addProperty("id", id); + segment.addProperty("isArchived", false); + segment.add("included", strings(included)); + segment.add("excluded", new JsonArray()); + segment.add("rules", rules); + return segment; + } + + private static JsonObject segmentRule(JsonObject condition) { + JsonArray conditions = new JsonArray(); + conditions.add(condition); + JsonObject rule = new JsonObject(); + rule.addProperty("id", "segment-rule"); + rule.addProperty("name", "segment rule"); + rule.add("conditions", conditions); + return rule; + } + private void syncFlagRule(JsonObject condition) { JsonArray rules = new JsonArray(); rules.add(flagRule(condition)); @@ -253,13 +294,7 @@ private static JsonArray targetUsers(int... indexes) { } private static JsonObject userIdRule(String op, int... indexes) { - JsonArray conditions = new JsonArray(); - conditions.add(condition("user_id", op, userIds(indexes))); - JsonObject rule = new JsonObject(); - rule.addProperty("id", "user-id-rule"); - rule.addProperty("name", "user ids"); - rule.add("conditions", conditions); - return rule; + return segmentRule(condition("user_id", op, userIds(indexes))); } private static JsonObject flagRule(JsonObject condition) { From 920b59417bf4471faef98d02ed021f4e4c58a692 Mon Sep 17 00:00:00 2001 From: Miguel Lopez <4165471+lopezm1@users.noreply.github.com> Date: Fri, 30 Oct 2026 09:04:48 -0700 Subject: [PATCH 3/4] test: start replacements once every reader is evaluating A reader thread that first ran after the replacements finished saw the done flag, returned zero evaluations and failed the test. Each reader now evaluates at least once and the replacements wait for all four, so they always overlap concurrent evaluations. --- src/test/java/co/featbit/server/ListTargetingTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/co/featbit/server/ListTargetingTest.java b/src/test/java/co/featbit/server/ListTargetingTest.java index 093cad3..937c367 100644 --- a/src/test/java/co/featbit/server/ListTargetingTest.java +++ b/src/test/java/co/featbit/server/ListTargetingTest.java @@ -15,9 +15,11 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.IntStream; @@ -154,13 +156,14 @@ void testConcurrentEvaluationsDuringReplacements() throws Exception { syncSegment(userIdRule("IsOneOf", first), Collections.emptyList(), Collections.emptyList()); AtomicBoolean done = new AtomicBoolean(); + CountDownLatch started = new CountDownLatch(4); ExecutorService readers = Executors.newFixedThreadPool(4); List> results = new ArrayList<>(); try { for (int t = 0; t < 4; t++) { results.add(readers.submit(() -> { int evaluations = 0; - while (!done.get()) { + do { for (int index : new int[]{0, size - 1, size, 2 * size - 1}) { String reason = detail(index, false).getReason(); if (!reason.equals(REASON_RULE_MATCH) && !reason.equals(REASON_FALLTHROUGH)) { @@ -168,10 +171,13 @@ void testConcurrentEvaluationsDuringReplacements() throws Exception { } evaluations++; } - } + started.countDown(); + } while (!done.get()); return evaluations; })); } + // replace only once every reader is evaluating, so replacements overlap evaluations + assertTrue(started.await(10, TimeUnit.SECONDS)); for (int i = 0; i < 200; i++) { syncSegment(userIdRule("IsOneOf", i % 2 == 0 ? second : first), Collections.emptyList(), Collections.emptyList()); } From b056214ea1a2fb8eb1cf26b8a268f03b50776c46 Mon Sep 17 00:00:00 2001 From: deleteLater Date: Tue, 3 Nov 2026 12:16:10 +0800 Subject: [PATCH 4/4] test: count down reader latch only once per thread --- src/test/java/co/featbit/server/ListTargetingTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/java/co/featbit/server/ListTargetingTest.java b/src/test/java/co/featbit/server/ListTargetingTest.java index 937c367..bf29c7b 100644 --- a/src/test/java/co/featbit/server/ListTargetingTest.java +++ b/src/test/java/co/featbit/server/ListTargetingTest.java @@ -163,6 +163,7 @@ void testConcurrentEvaluationsDuringReplacements() throws Exception { for (int t = 0; t < 4; t++) { results.add(readers.submit(() -> { int evaluations = 0; + boolean firstPass = true; do { for (int index : new int[]{0, size - 1, size, 2 * size - 1}) { String reason = detail(index, false).getReason(); @@ -171,7 +172,10 @@ void testConcurrentEvaluationsDuringReplacements() throws Exception { } evaluations++; } - started.countDown(); + if (firstPass) { + started.countDown(); + firstPass = false; + } } while (!done.get()); return evaluations; }));