chore: adds conditional persistence propagation #114
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
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| public TestData shouldPersist(boolean shouldPersist) { | ||
| this.shouldPersist = shouldPersist; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
TestData.shouldPersist(false) doesn't prevent updates from persisting
Medium Severity
The shouldPersist(false) setting is only honored during the initial init() call via makeInitData(). Subsequent calls to update() and delete() use upsert() which always persists to the persistent store regardless of the shouldPersist setting. The documentation promises to "prevent test data from being written to persistent stores" but this only works for initial data, not for updates.
Additional Locations (2)
| * @param shouldPersist true if the data should be persisted to persistent stores, false otherwise | ||
| */ | ||
| public FullDataSet(Iterable<Map.Entry<DataKind, KeyedItems<TDescriptor>>> data) { | ||
| public FullDataSet(Iterable<Map.Entry<DataKind, KeyedItems<TDescriptor>>> data, boolean shouldPersist) { |
There was a problem hiding this comment.
Wouldn't this change be a major version break?
There was a problem hiding this comment.
I was assuming for v1 we would leave things alone, then for v2 we would specify this in the builders which instantiate the wrappers. So on the FDv2 path it would have persist = false.
Requirements
Related issues
SDK-1622
Describe the solution you've provided
Adds shouldPersist to change set classes.
Note
Introduces conditional persistence across data flow by adding
shouldPersisttoDataStoreTypes.FullDataSetandChangeSet, and propagating it through ingestion, translation, storage, and recovery paths.FullDataSetandChangeSetnow carryshouldPersist; equality/hashCode updated.DataModelSerialization.parseFullDataSetnow returns iterable for constructingFullDataSetwith persistence intent.FDv2ChangeSetTranslator.toChangeSetacceptsshouldPersist.shouldPersist=true. File data source buildsFullDataSet(..., false).TestDataaddsshouldPersist(boolean)and uses it when initializing.InMemoryDataStoretracks and preservesshouldPersistininit/apply/exportAll.WriteThroughStoreonly writes to persistent store whenshouldPersist=true; legacy paths preserved.PersistentDataStoreConvertercarries the flag;PersistentDataStoreWrapperpasses it throughinit, and during recovery skips persisting external data whenshouldPersist=false; cache-based recovery usestrue.DataModelDependencies.sortAllCollections/sortChangesetpreserveshouldPersist.Written by Cursor Bugbot for commit 6e14b7f. This will update automatically on new commits. Configure here.