chore: Add read/write mode support for persistent store. - #206
Conversation
ab581db to
2fed977
Compare
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| public bool Initialized() | ||
| { | ||
| return _hasPersistence ? _persistentStore.Initialized() : _memoryStore.Initialized(); | ||
| return _activeReadStore.Initialized(); |
There was a problem hiding this comment.
I am not 100% on this. I think it results in correct LDClient behavior, but what needs validated is the persistent store restoration stuff. I think it will be fine as well.
If it isn't then this may need to be contingent on read/write mode. But it is not a critical problem either way.
| public void Apply(DataStoreTypes.ChangeSet<DataStoreTypes.ItemDescriptor> changeSet) | ||
| { | ||
| _txMemoryStore.Apply(changeSet); | ||
| MaybeSwitchStore(); |
There was a problem hiding this comment.
Switching earlier so a bad persistent store doesn't stop the SDK from using in-memory data.
| // TODO: Probably throw? | ||
| } | ||
| // The exception type doesn't matter here, as it will be converted to data store status. | ||
| throw new Exception("Failure to apply data set to persistent store."); |
There was a problem hiding this comment.
This comment knows too much!
There was a problem hiding this comment.
Also makes me wonder if Apply should have a bool result?
There was a problem hiding this comment.
I think this fits best with the current way data source updates works. Anything down the layers can throw an exception and its content can be included in a status update. The interface basically expects things to throw.
Basically this comment is just why it doesn't have a custom exception type, when it in a more general sense should.
There was a problem hiding this comment.
If we changed the exception type to "TransactionalApplyException", then the comment could go away.
2 participants
Adds support for controlling the read/write mode of the persistent store.
Added an exception for failure to write to the persistent store.
Adds a test suite for the WriteThroughStore.
Almost the entire PR size is from tests.
Note
Adds persistence mode to
WriteThroughStorewith conditional writes/init and improved apply error handling; wires mode inFDv2DataSystem; introduces a comprehensive test suite.FDv2DataSystem: Passconfiguration.DataSystem.PersistentDataStoreModetoWriteThroughStore.WriteThroughStore:_persistenceModeand ctor param to control persistent store behavior.ReadWritemode;ReadOnlyskips writes.Initialized()uses_activeReadStore.WriteThroughStoreTest: covers constructor behavior, init/get/getAll, upsert, apply (transactional vs legacy), store switching, initialized/selector/status monitoring, dispose, and error handling (including switch-to-memory on persistence failures).Written by Cursor Bugbot for commit 0e23440. This will update automatically on new commits. Configure here.