feat: Implement StateManipulator with cleaner, more efficient conflict resolution #2946
Conversation
stbrody
force-pushed
the
state-manipulator-cdb-2725
branch
from
6b425c1 to
e340e38
Compare
| @@ -0,0 +1,129 @@ | |||
| import { jest } from '@jest/globals' | |||
There was a problem hiding this comment.
this test file is not intended to be exhaustive. I just happened to write a simple test at this layer while trying to figure out how to write a test at the lower StateManipulator layer. Figured it out, but figured I'd leave this test in because why not. I expect to more fully flesh out the tests for the StreamLoader in a subsequent PR though
| ).rejects.toThrow(/rejected by conflict resolution/) | ||
| }) | ||
|
|
||
| describe('pickLogToAccept', () => { |
There was a problem hiding this comment.
these tests are ported almost directly from conflict-resolution.test.ts.
| return log1[log1.length - 1].cid.bytes < log2[log2.length - 1].cid.bytes ? log1 : log2 | ||
| } | ||
|
|
||
| async _applyLogToState_noCacaoVerification( |
There was a problem hiding this comment.
Not a big deal but I don't really like mixing casing conventions like this, couldn't this method be named _applyLogToStateWithoutCacaoVerification for example please?
| throwOnInvalidCommit: boolean | ||
| throwIfStale: boolean | ||
| throwOnConflict: boolean |
There was a problem hiding this comment.
Do these default to true or false when applying the logic please?
If they default to true and must be explicitly set to false, could we inverse the logic please? Ex:
allowInvalidCommit: boolean
allowStale: boolean
allowConflict: booleanWith these flags having to be set explicitly to true not to throw? I think that would make it clearer the default behavior is to be strict, unless explicitly set?
There was a problem hiding this comment.
they don't actually default to anything, it is required for the caller to pass all 3 in explicitly. There isn't really a clear case for which behavior should be the "default" - they are all set differently in different contexts. E.g. throwOnInvalidCommit is true for user-initiated writes coming in via the http api, but false for writes that come in via pubsub.
No description provided.