chore: adds TestDataV2 to implement Synchronizer #124
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/TestData.java
Outdated
Show resolved
Hide resolved
| * @param condition condition to poll; when it returns true, this method returns | ||
| * @throws InterruptedException if the current thread is interrupted while waiting | ||
| * @throws AssertionError if the condition does not become true before the timeout | ||
| */ |
There was a problem hiding this comment.
For reviewers: Not sure if these helpers are really warranted, but it seems to make usability better.
lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/TestDataV2.java
Show resolved
Hide resolved
tanderson-ld
changed the title
|
|
||
| ModelBuilders.FlagBuilder expectedFlag = flagBuilder("flag1").version(1).salt("") | ||
| .on(true).offVariation(1).fallthroughVariation(0).variations(true, false); | ||
| assertJsonEquals(flagJson(expectedFlag, 2), flagJson(flag1)); |
There was a problem hiding this comment.
Test assertion uses incorrect expected version number
Medium Severity
The addsFlag() test asserts against flagJson(expectedFlag, 2) but the flag being added is a NEW flag created after initialization, which gets version 1 (not 2). The update() method calculates version as oldVersion + 1 where oldVersion = 0 for new flags. This is inconsistent with the verifyFlag() helper at line 256 which correctly uses version 1 for the same scenario. The assertion compares mismatched versions, making the test either fail or validate incorrectly.
| } | ||
| return CompletableFuture.anyOf(shutdownFuture, future).thenApply(r -> (FDv2SourceResult) r); | ||
| } | ||
| } |
There was a problem hiding this comment.
Split synchronized blocks create unnecessary race condition window
Low Severity
The next() method uses two separate synchronized (queueLock) blocks when they could be combined into one. Releasing and reacquiring the lock between setting initialSent and processing the queue creates a window where concurrent callers could interleave. If multiple threads call next() simultaneously, the second thread could steal the initial changeset from the first thread. The two blocks can be combined since makeFullChangeSet() doesn't create a deadlock risk with the current lock ordering.
Requirements
Note
Low Risk
Primarily adds new test-only integration code and tests, with minimal surface-area changes to existing
TestData(constructor visibility and docs).Overview
Adds
TestDataV2, a new in-memory test data provider that implements the FDv2SynchronizerAPI, emitting an initial fullChangeSetand subsequent partialChangeSets onupdate()/delete(), plus optional simulated status events andawaitPropagationhelpers.Reuses the existing
TestData.FlagBuilderfor flag construction by loosening its constructors’ visibility, and adds comprehensive unit tests validating initialization, updates/deletes, and flag JSON output.Written by Cursor Bugbot for commit c0e3ce6. This will update automatically on new commits. Configure here.