STITCH-2134 Add unit tests for sync versioning scheme #75
Conversation
| // i. Find the corresponding local document config. | ||
| final CoreDocumentSynchronizationConfig docConfig = | ||
| nsConfig.getSynchronizedDocument(eventEntry.getKey().asDocument().get("_id")); | ||
| nsConfig.getSynchronizedDocument(eventEntry.getValue().getDocumentKey().get("_id")); |
There was a problem hiding this comment.
[nit] the arg can be BsonUtils.getDocumentId(eventEntry.getValue().getDocumentKey())
There was a problem hiding this comment.
This is the fix that we spoke about in Slack earlier.
There was a problem hiding this comment.
BsonUtils.getDocumentId() is just a helper that makes it so you don't have to do .get("_id") with the hardcoded "_id" string
| nsLock.writeLock().lockInterruptibly(); | ||
| try { | ||
| events.put(event.getData().getDocumentKey(), event.getData()); | ||
| events.put(event.getData().getDocumentKey().get("_id"), event.getData()); |
There was a problem hiding this comment.
[nit] the arg can be BsonUtils.getDocumentId(event.getData().getValue().getDocumentKey())
| */ | ||
| fun mockDeleteException(exception: Exception) | ||
| } | ||
| } |
| verify(ctx.collectionMock).insertOne(captor.capture()) | ||
| val versionDoc = | ||
| DocumentVersionInfo.fromVersionDoc(captor.value["__stitch_sync_version"]!!.asDocument()) | ||
| println(versionDoc) |
There was a problem hiding this comment.
[nit] is this println from debugging? also shouldn't we check the values in the versionDoc?
There was a problem hiding this comment.
This was exclusively for debugging. I'm not sure checking the inner values of version doc is valuable here, since we are really seeking out a specific result unrelated to what the versionDoc looks like.
| ctx.insertTestDocument() | ||
| ctx.doSyncPass() | ||
| // update the doc locally and queue a fake update remotely. | ||
| // neither of these will have versions. |
| ctx.insertTestDocument() | ||
| ctx.doSyncPass() | ||
| // update the doc locally and queue a fake update remotely. | ||
| // neither of these will have versions. |
| ctx.queueConsumableRemoteUpdateEvent( | ||
| ctx.testDocumentId, | ||
| pseudoUpdatedDocument, | ||
| TestVersionState.NEXT) |
There was a problem hiding this comment.
[q] why does this update not go through if it is indeed the next version? I think this update does go through, but the update at line 1473 doesn't go through. or am I understanding this wrong?
There was a problem hiding this comment.
The final update does not go through, because when we downgrade the remote version, our R2L code considers it an event generated by us, as intended.
| TestVersionState.NEW) | ||
|
|
||
| ctx.shouldConflictBeResolvedByRemote = true | ||
| // sync, creating a conflict. because remote has an empty version, |
There was a problem hiding this comment.
[nit q] I thought remote has a version with a new GUIDs, not an empty version (meaning no GUID at all). I would think that this case isn't supposed to delete anything, but rather replace the local doc with the new document in the update event
There was a problem hiding this comment.
oh I see what's happening now. The remote event is stale (but has a document with a new version GUID), but the remote collection itself no longer has the document, so the conflict is using the latest remote document which doesn't exist to resolve the conflict. maybe you should update the comments to clarify that
| ctx.insertTestDocument() | ||
| ctx.doSyncPass() | ||
| // update the doc locally and queue a fake update remotely. | ||
| // neither of these will have versions. |
| when (versionState) { | ||
| TestVersionState.NONE -> | ||
| fakeUpdateDoc.remove("__stitch_sync_version") | ||
| TestVersionState.PREVIOUS -> |
There was a problem hiding this comment.
[nit] might be worth throwing an IllegalStateException if documentVersionInfo.version.versionCounter <= 0 so we know we're not doing anything funky by having a negative version counter in the test.
| val fakeUpdateDoc = document.clone() | ||
| val cachedVersion = fakeUpdateDoc["__stitch_sync_version"] ?: getVersionForTestDocument() | ||
|
|
||
| if (cachedVersion != null) { |
SyncVersionConflictedUpdateRemoteWins
SyncVersionConflictedUpdateLocalWins
RemoteUpdateLocalAndRemoteEmptyVersion
RemoteUpdateLocalOrRemoteEmptyVersion
RemoteUpdateLocalVersionEqual
RemoteUpdateLocalVersionHigher
RemoteUpdateRemoteVersionHigher
RemoteUpdateDifferentGUIDs
RemoteUpdateDifferentGUIDsNewDoc