From 6971f31c4c279314fc642d73a6772c44c64ede31 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Tue, 9 Oct 2018 17:45:34 -0400 Subject: [PATCH 01/10] initial progress (not tests) passing tests, but no new tests add some question comments progress with removing committed versions incomplete server test changes --- .../internal/SyncMongoClientIntTests.kt | 177 +++++++++--------- .../CoreDocumentSynchronizationConfig.java | 51 ++--- .../sync/internal/DataSynchronizer.java | 160 ++++++++++++---- .../sync/internal/DocumentVersionInfo.java | 148 +++++++++++++-- .../internal/SyncMongoClientIntTests.kt | 15 +- 5 files changed, 368 insertions(+), 183 deletions(-) diff --git a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index ede0d6ed4..23ae1d96e 100644 --- a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -167,39 +167,39 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { goOffline() val doc3 = Document("so", "syncy") val insResult = Tasks.await(coll.insertOneAndSync(doc3)) - assertEquals(doc3, withoutVersionId(Tasks.await(coll.findOneById(insResult.insertedId))!!)) + assertEquals(doc3, withoutSyncVersion(Tasks.await(coll.findOneById(insResult.insertedId))!!)) streamAndSync() assertNull(Tasks.await(remoteColl.find(Document("_id", doc3["_id"])).first())) goOnline() streamAndSync() - assertEquals(doc3, withoutVersionId(Tasks.await(remoteColl.find(Document("_id", doc3["_id"])).first())!!)) + assertEquals(doc3, withoutSyncVersion(Tasks.await(remoteColl.find(Document("_id", doc3["_id"])).first())!!)) // 3. updating a document locally that has been updated remotely should invoke the conflict // resolver. val sem = watchForEvents(this.namespace) val result2 = Tasks.await(remoteColl.updateOne( doc1Filter, - withNewVersionIdSet(doc1Update))) + withNewSyncVersionSet(doc1Update))) sem.acquire() assertEquals(1, result2.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) val result3 = Tasks.await(coll.updateOneById( doc1Id, doc1Update)) assertEquals(1, result3.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) // first pass will invoke the conflict handler and update locally but not remotely yet streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) expectedDocument["foo"] = 4 expectedDocument.remove("fooOps") - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) // second pass will update with the ack'd version id streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) } } @@ -231,14 +231,14 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() // Update remote - val remoteUpdate = withNewVersionIdSet(Document("\$set", Document("remote", "update"))) + val remoteUpdate = withNewSyncVersionSet(Document("\$set", Document("remote", "update"))) val sem = watchForEvents(this.namespace) var result = Tasks.await(remoteColl.updateOne(doc1Filter, remoteUpdate)) sem.acquire() assertEquals(1, result.matchedCount) val expectedRemoteDocument = Document(doc) expectedRemoteDocument["remote"] = "update" - assertEquals(expectedRemoteDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedRemoteDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) // Update local val localUpdate = Document("\$set", Document("local", "updateWow")) @@ -246,18 +246,18 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { assertEquals(1, result.matchedCount) val expectedLocalDocument = Document(doc) expectedLocalDocument["local"] = "updateWow" - assertEquals(expectedLocalDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) // first pass will invoke the conflict handler and update locally but not remotely yet streamAndSync() - assertEquals(expectedRemoteDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedRemoteDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) expectedLocalDocument["remote"] = "update" - assertEquals(expectedLocalDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) // second pass will update with the ack'd version id streamAndSync() - assertEquals(expectedLocalDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) - assertEquals(expectedLocalDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) } } @@ -282,20 +282,20 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val expectedDocument = Document(doc) val sem = watchForEvents(this.namespace) - var result = Tasks.await(remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 2))))) + var result = Tasks.await(remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 2))))) sem.acquire() assertEquals(1, result.matchedCount) expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) result = Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) } } @@ -320,20 +320,20 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val expectedDocument = Document(doc) val sem = watchForEvents(this.namespace) - var result = Tasks.await(remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 2))))) + var result = Tasks.await(remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 2))))) sem.acquire() assertEquals(1, result.matchedCount) expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) result = Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) } } @@ -359,8 +359,8 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val result = Tasks.await(coll.deleteOneById(doc1Id)) assertEquals(1, result.deletedCount) - val expectedDocument = withoutVersionId(Document(doc)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + val expectedDocument = withoutSyncVersion(Document(doc)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) assertNull(Tasks.await(coll.findOneById(doc1Id))) goOnline() @@ -393,7 +393,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, Tasks.await(remoteColl.updateOne( doc1Filter, - withNewVersionIdSet(doc1Update))).matchedCount) + withNewSyncVersionSet(doc1Update))).matchedCount) goOffline() val result = Tasks.await(coll.deleteOneById(doc1Id)) @@ -401,16 +401,16 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val expectedDocument = Document(doc) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) assertNull(Tasks.await(coll.findOneById(doc1Id))) goOnline() streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) expectedDocument.remove("hello") expectedDocument.remove("foo") expectedDocument["well"] = "shoot" - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) } } @@ -433,16 +433,16 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, doc1Update)).matchedCount) - val expectedDocument = withoutVersionId(Document(doc)) + val expectedDocument = withoutSyncVersion(Document(doc)) expectedDocument["foo"] = 1 assertNull(Tasks.await(remoteColl.find(doc1Filter).first())) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) goOnline() streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) } } @@ -464,20 +464,20 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { goOnline() streamAndSync() - val expectedDocument = withoutVersionId(Document(doc)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + val expectedDocument = withoutSyncVersion(Document(doc)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, doc1Update)).matchedCount) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) } } @@ -496,25 +496,25 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val doc = Tasks.await(coll.findOneById(insertResult.insertedId))!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) val doc1Filter = Document("_id", doc1Id) - val expectedDocument = withoutVersionId(Document(doc)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + val expectedDocument = withoutSyncVersion(Document(doc)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) assertEquals(1, Tasks.await(coll.deleteOneById(doc1Id)).deletedCount) Tasks.await(coll.insertOneAndSync(doc)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, doc1Update)).matchedCount) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) } } @@ -621,21 +621,25 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val wait = watchForEvents(this.namespace, 2) Tasks.await(remoteColl.deleteOne(doc1Filter)) - Tasks.await(remoteColl.insertOne(withNewVersionId(doc))) + Tasks.await(remoteColl.insertOne(withNewSyncVersion(doc))) wait.acquire() + // TODO(QUESTION FOR REVIEWER): this test seems funky to me. We do the udpate, but + // the change is not reflected in any of the following finds. Why is that? Is the remote + // update with the new version ID supposed to make it so that this update is lost? + // it would be nice if we could have some comments in these tests. assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))).matchedCount) streamAndSync() - assertEquals(doc, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(doc, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) val expectedDocument = Document("_id", doc1Id.value) expectedDocument["hello"] = "again" - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) } } @@ -647,7 +651,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val remoteColl = getTestCollRemote() val docToInsert = Document("hello", "world") - Tasks.await(remoteColl.insertOne(withNewVersionId(docToInsert))) + Tasks.await(remoteColl.insertOne(withNewSyncVersion(docToInsert))) val doc = Tasks.await(remoteColl.find(docToInsert).first())!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) @@ -661,10 +665,10 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))).matchedCount) streamAndSync() - val expectedDocument = Document(withoutVersionId(doc)) + val expectedDocument = Document(withoutSyncVersion(doc)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) } } @@ -676,7 +680,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val remoteColl = getTestCollRemote() val docToInsert = Document("hello", "world") - Tasks.await(remoteColl.insertOne(withNewVersionId(docToInsert))) + Tasks.await(remoteColl.insertOne(withNewSyncVersion(docToInsert))) val doc = Tasks.await(remoteColl.find(docToInsert).first())!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) @@ -691,15 +695,15 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { Assert.assertNotNull(Tasks.await(coll.findOneById(doc1Id))) val sem = watchForEvents(this.namespace) - assertEquals(1, Tasks.await(remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 1))))).matchedCount) + assertEquals(1, Tasks.await(remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 1))))).matchedCount) sem.acquire() assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))).matchedCount) streamAndSync() - val expectedDocument = Document(withoutVersionId(doc)) + val expectedDocument = Document(withoutSyncVersion(doc)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) goOffline() assertNull(Tasks.await(coll.findOneById(doc1Id))) @@ -736,17 +740,17 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() val expectedDocument = Document(doc) - var result = Tasks.await(remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 2))))) + var result = Tasks.await(remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 2))))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) powerCycleDevice() coll.configure(DefaultSyncConflictResolvers.localWins(), null, null) result = Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) powerCycleDevice() coll.configure(DefaultSyncConflictResolvers.localWins(), null, null) @@ -758,12 +762,12 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() // resolves the conflict expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) powerCycleDevice() coll.configure(DefaultSyncConflictResolvers.localWins(), null, null) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) } } @@ -776,7 +780,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { coll.configure(failingConflictHandler, null, null) val doc1Id = Tasks.await(coll.insertOneAndSync(docToInsert)).insertedId - assertEquals(docToInsert, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(docToInsert, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) coll.desyncOne(doc1Id) streamAndSync() assertNull(Tasks.await(coll.findOneById(doc1Id))) @@ -802,12 +806,12 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() val expectedDocument = Document(docToInsert) expectedDocument["friend"] = "welcome" - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) - assertEquals(docToInsert, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(docToInsert, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(Tasks.await(coll.findOneById(doc1Id))!!)) - assertEquals(expectedDocument, withoutVersionId(Tasks.await(remoteColl.find(doc1Filter).first())!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(remoteColl.find(doc1Filter).first())!!)) } } @@ -985,23 +989,28 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { return newDoc } - private fun withoutVersionId(document: Document): Document { + private fun withoutSyncVersion(document: Document): Document { val newDoc = Document(document) newDoc.remove("__stitch_sync_version") return newDoc } - private fun withNewVersionId(document: Document): Document { - val newDocument = Document(HashMap(document)) - newDocument["__stitch_sync_version"] = UUID.randomUUID().toString() - return newDocument - } - - private fun withNewVersionIdSet(document: Document): Document { + private fun withNewSyncVersionSet(document: Document): Document { return appendDocumentToKey( "\$set", document, - Document("__stitch_sync_version", UUID.randomUUID().toString())) + Document("__stitch_sync_version", freshSyncVersionDoc())) + } + + private fun withNewSyncVersion(document: Document): Document { + val newDocument = Document(java.util.HashMap(document)) + newDocument["__stitch_sync_version"] = freshSyncVersionDoc() + + return newDocument + } + + private fun freshSyncVersionDoc(): Document { + return Document("spv", 1).append("id", UUID.randomUUID().toString()).append("v", 0L) } private fun appendDocumentToKey(key: String, on: Document, toAppend: Document): Document { diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java index ad4677a6f..60f645dc7 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java @@ -56,16 +56,10 @@ class CoreDocumentSynchronizationConfig { private final ReadWriteLock docLock; private ChangeEvent lastUncommittedChangeEvent; private long lastResolution; - private BsonValue lastKnownRemoteVersion; + private BsonDocument lastKnownRemoteVersion; private boolean isStale; private boolean isFrozen; - // TODO: How can this be trimmed? The same version could appear after we see it once. That - // may be a non-issue. - // TODO: To get rid of this, an ordering on versions would be needed and would have to be - // abided by other clients sync and non-sync alike. - private Set committedVersions; - CoreDocumentSynchronizationConfig( final MongoCollection docsColl, final MongoNamespace namespace, @@ -77,7 +71,6 @@ class CoreDocumentSynchronizationConfig { this.docLock = new ReentrantReadWriteLock(); this.lastResolution = -1; this.lastKnownRemoteVersion = null; - this.committedVersions = new HashSet<>(); this.lastUncommittedChangeEvent = null; this.isStale = false; } @@ -92,7 +85,6 @@ class CoreDocumentSynchronizationConfig { this.docLock = config.docLock; this.lastResolution = config.lastResolution; this.lastKnownRemoteVersion = config.lastKnownRemoteVersion; - this.committedVersions = config.committedVersions; this.lastUncommittedChangeEvent = config.lastUncommittedChangeEvent; this.isStale = config.isStale; this.isFrozen = config.isFrozen; @@ -103,8 +95,7 @@ private CoreDocumentSynchronizationConfig( final BsonValue documentId, final ChangeEvent lastUncommittedChangeEvent, final long lastResolution, - final BsonValue lastVersion, - final Set committedVersions, + final BsonDocument lastVersion, final boolean isStale, final boolean isFrozen ) { @@ -112,7 +103,6 @@ private CoreDocumentSynchronizationConfig( this.documentId = documentId; this.lastResolution = lastResolution; this.lastKnownRemoteVersion = lastVersion; - this.committedVersions = committedVersions; this.lastUncommittedChangeEvent = lastUncommittedChangeEvent; this.docLock = new ReentrantReadWriteLock(); this.docsColl = null; @@ -226,11 +216,14 @@ void setSomePendingWrites( * * @param atTime the time at which the write occurred. * @param atVersion the version for which the write occurred. + * // TODO(QUESTION FOR REVIEW): so per my other comments, this means the + * // version before the write occured, right? + * * @param changeEvent the description of the write/change. */ void setSomePendingWrites( final long atTime, - final BsonValue atVersion, + final BsonDocument atVersion, final ChangeEvent changeEvent ) { docLock.writeLock().lock(); @@ -239,9 +232,6 @@ void setSomePendingWrites( this.lastResolution = atTime; this.lastKnownRemoteVersion = atVersion; - if (atVersion != null) { - this.committedVersions.add(atVersion); - } docsColl.replaceOne( getDocFilter(namespace, documentId), this); @@ -250,14 +240,12 @@ void setSomePendingWrites( } } - void setPendingWritesComplete(final BsonValue atVersion) { + void setPendingWritesComplete(final BsonDocument atVersion) { docLock.writeLock().lock(); try { this.lastUncommittedChangeEvent = null; this.lastKnownRemoteVersion = atVersion; - if (atVersion != null) { - this.committedVersions.add(atVersion); - } + docsColl.replaceOne( getDocFilter(namespace, documentId), this); @@ -341,7 +329,7 @@ public long getLastResolution() { } } - public BsonValue getLastKnownRemoteVersion() { + public BsonDocument getLastKnownRemoteVersion() { docLock.readLock().lock(); try { return lastKnownRemoteVersion; @@ -350,15 +338,6 @@ public BsonValue getLastKnownRemoteVersion() { } } - public boolean hasCommittedVersion(final BsonValue version) { - docLock.readLock().lock(); - try { - return committedVersions.contains(version); - } finally { - docLock.readLock().unlock(); - } - } - /** * Possibly coalesces the newest change event to match the user's original intent. For example, * an unsynchronized insert and update is still an insert. @@ -440,8 +419,7 @@ BsonDocument toBsonDocument() { // TODO: This may put the doc above the 16MiB but ignore for now. asDoc.put(ConfigCodec.Fields.LAST_UNCOMMITTED_CHANGE_EVENT, encoded); } - final BsonArray committedVersions = new BsonArray(new ArrayList<>(this.committedVersions)); - asDoc.put(ConfigCodec.Fields.COMMITTED_VERSIONS, committedVersions); + asDoc.put(ConfigCodec.Fields.IS_STALE, new BsonBoolean(isStale)); asDoc.put(ConfigCodec.Fields.IS_FROZEN, new BsonBoolean(isFrozen)); return asDoc; @@ -455,7 +433,6 @@ static CoreDocumentSynchronizationConfig fromBsonDocument(final BsonDocument doc keyPresent(ConfigCodec.Fields.NAMESPACE_FIELD, document); keyPresent(ConfigCodec.Fields.SCHEMA_VERSION_FIELD, document); keyPresent(ConfigCodec.Fields.LAST_RESOLUTION_FIELD, document); - keyPresent(ConfigCodec.Fields.COMMITTED_VERSIONS, document); keyPresent(ConfigCodec.Fields.IS_STALE, document); keyPresent(ConfigCodec.Fields.IS_FROZEN, document); @@ -471,12 +448,10 @@ static CoreDocumentSynchronizationConfig fromBsonDocument(final BsonDocument doc final MongoNamespace namespace = new MongoNamespace(document.getString(ConfigCodec.Fields.NAMESPACE_FIELD).getValue()); - final BsonArray committedVersionsArr = document.getArray(ConfigCodec.Fields.COMMITTED_VERSIONS); - final Set committedVersions = new HashSet<>(committedVersionsArr); - final BsonValue lastVersion; + final BsonDocument lastVersion; if (document.containsKey(ConfigCodec.Fields.LAST_KNOWN_REMOTE_VERSION_FIELD)) { - lastVersion = document.get(ConfigCodec.Fields.LAST_KNOWN_REMOTE_VERSION_FIELD); + lastVersion = document.getDocument(ConfigCodec.Fields.LAST_KNOWN_REMOTE_VERSION_FIELD); } else { lastVersion = null; } @@ -498,7 +473,6 @@ static CoreDocumentSynchronizationConfig fromBsonDocument(final BsonDocument doc lastUncommittedChangeEvent, document.getNumber(ConfigCodec.Fields.LAST_RESOLUTION_FIELD).longValue(), lastVersion, - committedVersions, document.getBoolean(ConfigCodec.Fields.IS_STALE).getValue(), document.getBoolean(ConfigCodec.Fields.IS_FROZEN, new BsonBoolean(false)).getValue()); } @@ -537,7 +511,6 @@ static class Fields { static final String LAST_RESOLUTION_FIELD = "last_resolution"; static final String LAST_KNOWN_REMOTE_VERSION_FIELD = "last_known_remote_version"; static final String LAST_UNCOMMITTED_CHANGE_EVENT = "last_uncommitted_change_event"; - static final String COMMITTED_VERSIONS = "committed_versions"; static final String IS_STALE = "is_stale"; static final String IS_FROZEN = "is_frozen"; } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index 998845c9a..1ddc41607 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -67,12 +67,14 @@ import org.bson.BsonArray; import org.bson.BsonBoolean; import org.bson.BsonDocument; +import org.bson.BsonInt64; import org.bson.BsonString; import org.bson.BsonValue; import org.bson.Document; import org.bson.codecs.Codec; import org.bson.codecs.configuration.CodecRegistries; import org.bson.codecs.configuration.CodecRegistry; +import org.bson.conversions.Bson; import org.bson.diagnostics.Logger; import org.bson.diagnostics.Loggers; @@ -529,20 +531,30 @@ private void syncRemoteChangeEventToLocal( if (remoteDoc == null || remoteDoc.size() == 0) { lastKnownRemoteVersion = null; } else { - lastKnownRemoteVersion = DocumentVersionInfo - .getRemoteVersionInfo(remoteDoc).getCurrentVersion(); - - if (docConfig.hasCommittedVersion(lastKnownRemoteVersion)) { - // Skip this event since we generated it. - logger.info(String.format( - Locale.US, - "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote change event was " - + "generated by us; dropping the event", - logicalT, - nsConfig.getNamespace(), - docConfig.getDocumentId())); - return; - } + DocumentVersionInfo remoteVersionInfo = DocumentVersionInfo + .getRemoteVersionInfo(remoteDoc); + lastKnownRemoteVersion = remoteVersionInfo.getVersionDoc(); + + DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig, localDocument); + + // TODO(QUESTION FOR REVIEW): The spec seems to indicate that we should make this check in + // place of the committed versions check, but I have two questions: + // 1. this alone does not make all the tests pass. is this because this check doesn't + // adequately cover all cases where we need to drop the handling of the event? or is it + // becase + if ((localVersion.getVersionDoc() != null && lastKnownRemoteVersion != null) && + (localVersion.getInstanceId().equals(remoteVersionInfo.getInstanceId())) && + (localVersion.getVersionCounter() >= remoteVersionInfo.getVersionCounter())) { + // Skip this event since we generated it. + logger.info(String.format( + Locale.US, + "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote change event was " + + "generated by us; dropping the event", + logicalT, + nsConfig.getNamespace(), + docConfig.getDocumentId())); + return; + } } if (docConfig.getLastUncommittedChangeEvent() == null) { @@ -597,7 +609,7 @@ private void syncRemoteChangeEventToLocal( lastKnownLocalVersion = docConfig.getLastKnownRemoteVersion(); } else { lastKnownLocalVersion = DocumentVersionInfo - .getLocalVersionInfo(docConfig, localDocument).getCurrentVersion(); + .getLocalVersionInfo(docConfig, localDocument).getVersionDoc(); } // There is a pending write that must skip R2L if both versions are null. The absence of a @@ -713,14 +725,17 @@ private void syncLocalToRemote() { illegalStateException ); } - final DocumentVersionInfo versionInfo = + final DocumentVersionInfo localVersionInfo = DocumentVersionInfo.getLocalVersionInfo(docConfig, localDoc); - localDoc.put(DOCUMENT_VERSION_FIELD, versionInfo.getCurrentVersion()); + localDoc.put( + DOCUMENT_VERSION_FIELD, + localVersionInfo.getVersionDoc() + ); final RemoteUpdateResult result; try { result = remoteColl.updateOne( - versionInfo.getFilter(), + localVersionInfo.getFilter(), localDoc); } catch (final StitchServiceException ex) { this.emitError( @@ -759,9 +774,12 @@ private void syncLocalToRemote() { illegalStateException ); } - final DocumentVersionInfo versionInfo = + final DocumentVersionInfo localVersionInfo = DocumentVersionInfo.getLocalVersionInfo(docConfig, localDoc); - localDoc.put(DOCUMENT_VERSION_FIELD, versionInfo.getCurrentVersion()); + localDoc.put( + DOCUMENT_VERSION_FIELD, + localVersionInfo.getVersionDoc() + ); final BsonDocument translatedUpdate = new BsonDocument(); if (!localChangeEvent.getUpdateDescription().getUpdatedFields().isEmpty()) { @@ -770,7 +788,7 @@ private void syncLocalToRemote() { localChangeEvent.getUpdateDescription().getUpdatedFields().entrySet()) { sets.put(fieldValue.getKey(), fieldValue.getValue()); } - sets.put(DOCUMENT_VERSION_FIELD, versionInfo.getCurrentVersion()); + sets.put(DOCUMENT_VERSION_FIELD, localVersionInfo.getVersionDoc()); translatedUpdate.put("$set", sets); } if (!localChangeEvent.getUpdateDescription().getRemovedFields().isEmpty()) { @@ -785,7 +803,7 @@ private void syncLocalToRemote() { final RemoteUpdateResult result; try { result = remoteColl.updateOne( - versionInfo.getFilter(), + localVersionInfo.getFilter(), translatedUpdate.isEmpty() // See: changeEventForLocalUpdate for why we do this ? localDoc : translatedUpdate); @@ -1020,7 +1038,7 @@ private void resolveConflict( return; } - final BsonValue remoteVersion; + final BsonDocument remoteVersion; if (remoteEvent.getOperationType() == ChangeEvent.OperationType.DELETE) { // We expect there will be no version on the document. Note: it's very possible // that the document could be reinserted at this point with no version field and we @@ -1029,7 +1047,7 @@ private void resolveConflict( } else { final DocumentVersionInfo remoteVersionInfo = DocumentVersionInfo .getRemoteVersionInfo(remoteEvent.getFullDocument()); - remoteVersion = remoteVersionInfo.getCurrentVersion(); + remoteVersion = remoteVersionInfo.getVersionDoc(); } final boolean acceptRemote = @@ -1281,7 +1299,11 @@ public void insertOneAndSync( final MongoNamespace namespace, final BsonDocument document ) { - final BsonDocument docToInsert = withNewVersion(document); + // apply a fresh synchronization version to the document + final BsonDocument docToInsert = withNewVersion( + document, + DocumentVersionInfo.getFreshVersionDocument() + ); getLocalCollection(namespace).insertOne(docToInsert); final ChangeEvent event = changeEventForLocalInsert(namespace, docToInsert, true); @@ -1315,7 +1337,26 @@ public UpdateResult updateOneById( return UpdateResult.acknowledged(0, 0L, null); } - final BsonDocument updateWithVersion = withNewVersionIdSet(update); + // apply a version if none exists on the document (this will ensure that remote documents + // with no version will get a version when they are updated for the first time) + final BsonDocument applyVersionFilter = getDocumentIdFilter(documentId); + applyVersionFilter.put(DOCUMENT_VERSION_FIELD, new BsonDocument("$exists", BsonBoolean.FALSE)); + + getLocalCollection(namespace) + .updateOne( + applyVersionFilter, + new BsonDocument("$set", + new BsonDocument( + DOCUMENT_VERSION_FIELD, + DocumentVersionInfo.getFreshVersionDocument() + ) + ) + ); + + // if a version was just applied, we still want to increment it because the document with the + // newly applied version is technically its own version + final BsonDocument updateWithVersion = withVersionCounterIncField(update); + final BsonDocument result = getLocalCollection(namespace) .findOneAndUpdate( getDocumentIdFilter(documentId), @@ -1345,7 +1386,7 @@ private void replaceOrUpsertOneFromResolution( final MongoNamespace namespace, final BsonValue documentId, final BsonDocument document, - final BsonValue atVersion, + final BsonDocument atVersion, final boolean fromDelete ) { // TODO: lock down id @@ -1355,24 +1396,51 @@ private void replaceOrUpsertOneFromResolution( return; } - final BsonDocument docToReplace = withNewVersion(document); + // TODO(QUESTION FOR REVIEW): in the spec, we specify that replaces should result in an + // increment rather than a new instance ID. if atVersion is not null, we can do this, but if + // it doesn't, meaning we are replacing a document with no version, we simply use a fresh + // version with a new instance ID. My question is, would it be better if we just always + // use a fresh version with a new instance ID? We don't have a huge amount of information + // here about the original document, and it seems like a replace is semantically similar to a + // delete followed by insert, which would result in a new instance ID rather than an increment + final BsonDocument docToReplace; + final BsonDocument newVersion; + if (atVersion == null) { + newVersion = DocumentVersionInfo.getFreshVersionDocument(); + docToReplace = withNewVersion( + document, + newVersion + ); + } else { + newVersion = DocumentVersionInfo.withIncrementedVersionCounter(atVersion); + docToReplace = withNewVersion( + document, + newVersion + ); + } final BsonDocument result = getLocalCollection(namespace) .findOneAndReplace( getDocumentIdFilter(documentId), docToReplace, new FindOneAndReplaceOptions().upsert(true).returnDocument(ReturnDocument.AFTER)); final ChangeEvent event; + + // TODO(QUESTION FOR REVIEW): it seems like we were previously setting pending writes at the + // old version, rather than the new version. Was this intentional? It seems like we should be + // setting the pending writes at the newly generated version. I'm confused though because if I + // use the new version here instead, tests fail because updates don't end up happening on a + // sync pass. if (fromDelete) { event = changeEventForLocalInsert(namespace, result, true); config.setSomePendingWrites( logicalT, - atVersion, + atVersion, //TODO: why don't we want newVersion?, event); } else { event = changeEventForLocalReplace(namespace, documentId, result, true); config.setSomePendingWrites( logicalT, - atVersion, + atVersion, //TODO: why don't we want newVersion? event); } emitEvent(documentId, event); @@ -1390,7 +1458,7 @@ private void replaceOrUpsertOneFromRemote( final MongoNamespace namespace, final BsonValue documentId, final BsonDocument document, - final BsonValue atVersion + final BsonDocument atVersion ) { // TODO: lock down id final CoreDocumentSynchronizationConfig config = @@ -1399,11 +1467,18 @@ private void replaceOrUpsertOneFromRemote( return; } + // TODO(QUESTION FOR REVIEW): Unlike replaceOrUpsertOneFromResolution, the original code + // here did not replace the version of the document being used to replace the document at + // documentId. Was this intentional? I assume that because we're updating something locally + // with a remote version, we want to keep the remote version, but then why are we + // setting a pending write here? + getLocalCollection(namespace) .findOneAndReplace( getDocumentIdFilter(documentId), document, new FindOneAndReplaceOptions().upsert(true)); + config.setPendingWritesComplete(atVersion); emitEvent(documentId, changeEventForLocalReplace(namespace, documentId, document, false)); } @@ -1457,7 +1532,7 @@ public DeleteResult deleteOneById( private void deleteOneFromResolution( final MongoNamespace namespace, final BsonValue documentId, - final BsonValue atVersion + final BsonDocument atVersion ) { // TODO: lock down id final CoreDocumentSynchronizationConfig config = @@ -1697,9 +1772,12 @@ private static BsonDocument getDocumentIdFilter(final BsonValue documentId) { * @param document the document to attach a new version to. * @return a document with a new version id to the given document. */ - private static BsonDocument withNewVersion(final BsonDocument document) { + private static BsonDocument withNewVersion( + final BsonDocument document, + final BsonDocument newVersion + ) { final BsonDocument newDocument = BsonUtils.copyOfDocument(document); - newDocument.put(DOCUMENT_VERSION_FIELD, new BsonString(UUID.randomUUID().toString())); + newDocument.put(DOCUMENT_VERSION_FIELD, newVersion); return newDocument; } @@ -1709,10 +1787,18 @@ private static BsonDocument withNewVersion(final BsonDocument document) { * @param document the document to attach a new version set modifier to. * @return a document with a new version id set modifier to the given document. */ - private static BsonDocument withNewVersionIdSet(final BsonDocument document) { + private static BsonDocument withVersionCounterIncField(final BsonDocument document) { return BsonUtils.mergeSubdocumentAtKey( - "$set", + "$inc", document, - new BsonDocument(DOCUMENT_VERSION_FIELD, new BsonString(UUID.randomUUID().toString()))); + new BsonDocument( + String.format( + "%s.%s", + DOCUMENT_VERSION_FIELD, + DocumentVersionInfo.VERSION_COUNTER_FIELD + ), + new BsonInt64(1) + ) + ); } } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index 7664c2114..7e31fb4d7 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -22,21 +22,105 @@ import javax.annotation.Nullable; import org.bson.BsonBoolean; import org.bson.BsonDocument; +import org.bson.BsonInt32; +import org.bson.BsonInt64; +import org.bson.BsonString; import org.bson.BsonValue; -final class DocumentVersionInfo { - private final BsonValue version; +import java.util.UUID; + +public final class DocumentVersionInfo { + private final int syncProtocolVersion; + private final String instanceId; + private final long versionCounter; + + + static final String SYNC_PROTOCOL_VERSION_FIELD = "spv"; + static final String INSTANCE_ID_FIELD = "id"; + static final String VERSION_COUNTER_FIELD = "v"; + private final BsonDocument filter; + private final BsonDocument versionDoc; + + private DocumentVersionInfo( + final BsonDocument version, + final BsonValue documentId, + final BsonDocument versionForFilter + ) { + if (version != null) { + this.versionDoc = version; + this.syncProtocolVersion = versionDoc.getInt32(SYNC_PROTOCOL_VERSION_FIELD).getValue(); + this.instanceId = versionDoc.getString(INSTANCE_ID_FIELD).getValue(); + this.versionCounter = versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue(); + } else { + this.versionDoc = null; + + this.syncProtocolVersion = -1; + this.instanceId = null; + this.versionCounter = -1; + } + + this.filter = getVersionedFilter(documentId, versionForFilter); + } + +// private DocumentVersionInfo( +// final BsonValue documentId, +// final int syncProtocolVersion, +// final String instanceId, +// final long versionCounter +// ) { +// this.syncProtocolVersion = syncProtocolVersion; +// this.instanceId = instanceId; +// this.versionCounter = versionCounter; +// +// this.versionDoc = constructVersionDoc(); +// this.filter = getVersionedFilter(documentId, versionDoc); +// } + + public BsonDocument getVersionDoc() { + return versionDoc; + } - private DocumentVersionInfo(final BsonValue version, final BsonDocument filter) { - this.version = version; - this.filter = filter; +// private BsonDocument constructVersionDoc() { +// BsonDocument versionDoc = new BsonDocument(); +// +// versionDoc.append(SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(syncProtocolVersion)); +// versionDoc.append(INSTANCE_ID_FIELD, new BsonString(instanceId)); +// versionDoc.append(VERSION_COUNTER_FIELD, new BsonInt64(versionCounter)); +// +// return versionDoc; +// } + + public int getSyncProtocolVersion() { + return syncProtocolVersion; + } + + public String getInstanceId() { + return instanceId; } - public BsonValue getCurrentVersion() { - return version; + public long getVersionCounter() { + return versionCounter; } +// public DocumentVersionInfo withIncrementedVersionCounter() { +// return new DocumentVersionInfo( +// this.documentId, +// this.syncProtocolVersion, +// this.instanceId, +// this.versionCounter + 1 +// ); +// } +// +// public DocumentVersionInfo withNewInstanceId() { +// return new DocumentVersionInfo( +// this.documentId, +// this.syncProtocolVersion, +// UUID.randomUUID().toString(), +// 0 +// ); +// } + public BsonDocument getFilter() { return filter; } @@ -45,19 +129,47 @@ static DocumentVersionInfo getLocalVersionInfo( final CoreDocumentSynchronizationConfig docConfig, final BsonDocument localDocument ) { - final BsonValue version = getDocumentVersion(localDocument); + final BsonDocument version = getDocumentVersion(localDocument); return new DocumentVersionInfo( - version, - getVersionedFilter( - docConfig.getDocumentId(), docConfig.getLastKnownRemoteVersion())); + version, docConfig.getDocumentId(), docConfig.getLastKnownRemoteVersion() + ); } static DocumentVersionInfo getRemoteVersionInfo(final BsonDocument remoteDocument) { - final BsonValue version = getDocumentVersion(remoteDocument); - return new DocumentVersionInfo( - version, - getVersionedFilter( - BsonUtils.getDocumentId(remoteDocument), version)); + final BsonDocument version = getDocumentVersion(remoteDocument); + return new DocumentVersionInfo(version, BsonUtils.getDocumentId(remoteDocument), version); + } + +// public static DocumentVersionInfo createFreshDocumentVersion(BsonDocument document) { +// return new DocumentVersionInfo( +// BsonUtils.getDocumentId(document), +// 1, +// UUID.randomUUID().toString(), +// 0 +// ); +// } + + public static BsonDocument getFreshVersionDocument() { + BsonDocument versionDoc = new BsonDocument(); + + versionDoc.append(SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(1)); + versionDoc.append(INSTANCE_ID_FIELD, new BsonString(UUID.randomUUID().toString())); + versionDoc.append(VERSION_COUNTER_FIELD, new BsonInt64(0)); + + return versionDoc; + } + + public static BsonDocument withIncrementedVersionCounter(BsonDocument versionDoc) { + BsonDocument newVersionDoc = new BsonDocument(); + + newVersionDoc.put(SYNC_PROTOCOL_VERSION_FIELD, versionDoc.get(SYNC_PROTOCOL_VERSION_FIELD)); + newVersionDoc.put(INSTANCE_ID_FIELD, versionDoc.get(INSTANCE_ID_FIELD)); + newVersionDoc.put( + VERSION_COUNTER_FIELD, + new BsonInt64(versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue() + 1L) + ); + + return newVersionDoc; } /** @@ -65,11 +177,11 @@ static DocumentVersionInfo getRemoteVersionInfo(final BsonDocument remoteDocumen * @param document the document to get the version from. * @return the version of the given document, if any; returns null otherwise. */ - static BsonValue getDocumentVersion(final BsonDocument document) { + static BsonDocument getDocumentVersion(final BsonDocument document) { if (document == null || !document.containsKey(DOCUMENT_VERSION_FIELD)) { return null; } - return document.get(DOCUMENT_VERSION_FIELD); + return document.getDocument(DOCUMENT_VERSION_FIELD, null); } /** diff --git a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 69b997ff9..b9b9b72eb 100644 --- a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -991,16 +991,21 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { } private fun withNewVersionId(document: Document): Document { - val newDocument = Document(HashMap(document)) - newDocument["__stitch_sync_version"] = UUID.randomUUID().toString() + val newDocument = Document(java.util.HashMap(document)) + newDocument["__stitch_sync_version"] = freshSyncVersionDoc() + return newDocument } private fun withNewVersionIdSet(document: Document): Document { return appendDocumentToKey( - "\$set", - document, - Document("__stitch_sync_version", UUID.randomUUID().toString())) + "\$set", + document, + Document("__stitch_sync_version", freshSyncVersionDoc())) + } + + private fun freshSyncVersionDoc(): Document { + return Document("spv", 1).append("id", UUID.randomUUID().toString()).append("v", 0L) } private fun appendDocumentToKey(key: String, on: Document, toAppend: Document): Document { From c53cd8b1c176a706b04970d9d8a5885fa817c7c7 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Thu, 11 Oct 2018 11:33:46 -0400 Subject: [PATCH 02/10] eric patch --- .../sync/internal/DataSynchronizer.java | 121 ++++----- .../sync/internal/DocumentVersionInfo.java | 8 +- .../internal/SyncMongoClientIntTests.kt | 230 +++++++++--------- 3 files changed, 166 insertions(+), 193 deletions(-) diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index 1ddc41607..be561df46 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -20,6 +20,7 @@ import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalInsert; import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalReplace; import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalUpdate; +import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.DocumentVersionInfo.getDocumentVersion; import com.mongodb.MongoClientSettings; import com.mongodb.MongoNamespace; @@ -56,7 +57,6 @@ import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.UUID; import java.util.concurrent.Callable; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -68,13 +68,11 @@ import org.bson.BsonBoolean; import org.bson.BsonDocument; import org.bson.BsonInt64; -import org.bson.BsonString; import org.bson.BsonValue; import org.bson.Document; import org.bson.codecs.Codec; import org.bson.codecs.configuration.CodecRegistries; import org.bson.codecs.configuration.CodecRegistry; -import org.bson.conversions.Bson; import org.bson.diagnostics.Logger; import org.bson.diagnostics.Loggers; @@ -535,7 +533,7 @@ private void syncRemoteChangeEventToLocal( .getRemoteVersionInfo(remoteDoc); lastKnownRemoteVersion = remoteVersionInfo.getVersionDoc(); - DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig, localDocument); + DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig); // TODO(QUESTION FOR REVIEW): The spec seems to indicate that we should make this check in // place of the committed versions check, but I have two questions: @@ -574,7 +572,7 @@ private void syncRemoteChangeEventToLocal( nsConfig.getNamespace(), docConfig.getDocumentId(), remoteChangeEvent.getFullDocument(), - DocumentVersionInfo.getDocumentVersion(remoteChangeEvent.getFullDocument())); + getDocumentVersion(remoteChangeEvent.getFullDocument())); return; case DELETE: logger.info(String.format( @@ -609,7 +607,7 @@ private void syncRemoteChangeEventToLocal( lastKnownLocalVersion = docConfig.getLastKnownRemoteVersion(); } else { lastKnownLocalVersion = DocumentVersionInfo - .getLocalVersionInfo(docConfig, localDocument).getVersionDoc(); + .getLocalVersionInfo(docConfig).getVersionDoc(); } // There is a pending write that must skip R2L if both versions are null. The absence of a @@ -684,12 +682,17 @@ private void syncLocalToRemote() { BsonDocument remoteDocument = null; boolean remoteDocumentFetched = false; + final DocumentVersionInfo localVersionInfo = + DocumentVersionInfo.getLocalVersionInfo(docConfig); + final BsonDocument nextVersion; + switch (localChangeEvent.getOperationType()) { case INSERT: { + nextVersion = DocumentVersionInfo.getFreshVersionDocument(); // It's possible that we may insert after a delete happened and we didn't get a // notification for it. There's nothing we can do about this. try { - remoteColl.insertOne(localChangeEvent.getFullDocument()); + remoteColl.insertOne(withNewVersion(localChangeEvent.getFullDocument(), nextVersion)); } catch (final StitchServiceException ex) { if (ex.getErrorCode() != StitchServiceErrorCode.MONGODB_ERROR || !ex.getMessage().contains("E11000")) { @@ -724,19 +727,16 @@ private void syncLocalToRemote() { illegalStateException.getMessage(), illegalStateException ); + continue; } - final DocumentVersionInfo localVersionInfo = - DocumentVersionInfo.getLocalVersionInfo(docConfig, localDoc); - localDoc.put( - DOCUMENT_VERSION_FIELD, - localVersionInfo.getVersionDoc() - ); + final BsonDocument nextDoc = withNextVersionCounter(localDoc, localVersionInfo); + nextVersion = getDocumentVersion(nextDoc); final RemoteUpdateResult result; try { result = remoteColl.updateOne( localVersionInfo.getFilter(), - localDoc); + nextDoc); } catch (final StitchServiceException ex) { this.emitError( docConfig, @@ -773,13 +773,10 @@ private void syncLocalToRemote() { illegalStateException.getMessage(), illegalStateException ); + continue; } - final DocumentVersionInfo localVersionInfo = - DocumentVersionInfo.getLocalVersionInfo(docConfig, localDoc); - localDoc.put( - DOCUMENT_VERSION_FIELD, - localVersionInfo.getVersionDoc() - ); + final BsonDocument nextDoc = withNextVersionCounter(localDoc, localVersionInfo); + nextVersion = DocumentVersionInfo.getDocumentVersion(nextDoc); final BsonDocument translatedUpdate = new BsonDocument(); if (!localChangeEvent.getUpdateDescription().getUpdatedFields().isEmpty()) { @@ -788,7 +785,7 @@ private void syncLocalToRemote() { localChangeEvent.getUpdateDescription().getUpdatedFields().entrySet()) { sets.put(fieldValue.getKey(), fieldValue.getValue()); } - sets.put(DOCUMENT_VERSION_FIELD, localVersionInfo.getVersionDoc()); + sets.put(DOCUMENT_VERSION_FIELD, nextVersion); translatedUpdate.put("$set", sets); } if (!localChangeEvent.getUpdateDescription().getRemovedFields().isEmpty()) { @@ -806,7 +803,7 @@ private void syncLocalToRemote() { localVersionInfo.getFilter(), translatedUpdate.isEmpty() // See: changeEventForLocalUpdate for why we do this - ? localDoc : translatedUpdate); + ? nextDoc : translatedUpdate); } catch (final StitchServiceException ex) { emitError( docConfig, @@ -835,6 +832,7 @@ private void syncLocalToRemote() { } case DELETE: { + nextVersion = null; final RemoteDeleteResult result; try { result = remoteColl.deleteOne(DocumentVersionInfo.getVersionedFilter( @@ -887,6 +885,7 @@ private void syncLocalToRemote() { docConfig.getDocumentId(), localChangeEvent.getOperationType().toString()) ); + continue; } logger.info(String.format( @@ -915,6 +914,8 @@ private void syncLocalToRemote() { docConfig, remoteChangeEvent); } else { + // TODO(ERIC->ADAM): This event may contain old version info. We should be filtering out the version + // anyway from local and remote events. final ChangeEvent committedEvent = docConfig.getLastUncommittedChangeEvent(); emitEvent(docConfig.getDocumentId(), new ChangeEvent<>( @@ -925,7 +926,7 @@ private void syncLocalToRemote() { committedEvent.getDocumentKey(), committedEvent.getUpdateDescription(), false)); - docConfig.setPendingWritesComplete(DocumentVersionInfo.getDocumentVersion(localDoc)); + docConfig.setPendingWritesComplete(nextVersion); } } } @@ -1299,21 +1300,16 @@ public void insertOneAndSync( final MongoNamespace namespace, final BsonDocument document ) { - // apply a fresh synchronization version to the document - final BsonDocument docToInsert = withNewVersion( - document, - DocumentVersionInfo.getFreshVersionDocument() - ); - getLocalCollection(namespace).insertOne(docToInsert); + getLocalCollection(namespace).insertOne(document); final ChangeEvent event = - changeEventForLocalInsert(namespace, docToInsert, true); + changeEventForLocalInsert(namespace, document, true); final CoreDocumentSynchronizationConfig config = syncConfig.addSynchronizedDocument( namespace, - BsonUtils.getDocumentId(docToInsert) + BsonUtils.getDocumentId(document) ); config.setSomePendingWrites(logicalT, event); triggerListeningToNamespace(namespace); - emitEvent(BsonUtils.getDocumentId(docToInsert), event); + emitEvent(BsonUtils.getDocumentId(document), event); } /** @@ -1337,36 +1333,16 @@ public UpdateResult updateOneById( return UpdateResult.acknowledged(0, 0L, null); } - // apply a version if none exists on the document (this will ensure that remote documents - // with no version will get a version when they are updated for the first time) - final BsonDocument applyVersionFilter = getDocumentIdFilter(documentId); - applyVersionFilter.put(DOCUMENT_VERSION_FIELD, new BsonDocument("$exists", BsonBoolean.FALSE)); - - getLocalCollection(namespace) - .updateOne( - applyVersionFilter, - new BsonDocument("$set", - new BsonDocument( - DOCUMENT_VERSION_FIELD, - DocumentVersionInfo.getFreshVersionDocument() - ) - ) - ); - - // if a version was just applied, we still want to increment it because the document with the - // newly applied version is technically its own version - final BsonDocument updateWithVersion = withVersionCounterIncField(update); - final BsonDocument result = getLocalCollection(namespace) .findOneAndUpdate( getDocumentIdFilter(documentId), - updateWithVersion, + update, new FindOneAndUpdateOptions().returnDocument(ReturnDocument.AFTER)); if (result == null) { return UpdateResult.acknowledged(0, 0L, null); } final ChangeEvent event = - changeEventForLocalUpdate(namespace, documentId, updateWithVersion, result, true); + changeEventForLocalUpdate(namespace, documentId, update, result, true); config.setSomePendingWrites( logicalT, event); @@ -1781,24 +1757,23 @@ private static BsonDocument withNewVersion( return newDocument; } - /** - * Adds and returns a document with a new version id set modifier to the given document. - * - * @param document the document to attach a new version set modifier to. - * @return a document with a new version id set modifier to the given document. - */ - private static BsonDocument withVersionCounterIncField(final BsonDocument document) { - return BsonUtils.mergeSubdocumentAtKey( - "$inc", - document, - new BsonDocument( - String.format( - "%s.%s", - DOCUMENT_VERSION_FIELD, - DocumentVersionInfo.VERSION_COUNTER_FIELD - ), - new BsonInt64(1) - ) - ); + private static BsonDocument getNextVersion( + final DocumentVersionInfo versionInfo + ) { + if (versionInfo.getVersionDoc() == null) { + return DocumentVersionInfo.getFreshVersionDocument(); + } + final BsonDocument nextVersion = BsonUtils.copyOfDocument(versionInfo.getVersionDoc()); + nextVersion.put( + DocumentVersionInfo.VERSION_COUNTER_FIELD, + new BsonInt64(versionInfo.getVersionCounter() + 1)); + return nextVersion; + } + + private static BsonDocument withNextVersionCounter( + final BsonDocument document, + final DocumentVersionInfo versionInfo + ) { + return withNewVersion(document, getNextVersion(versionInfo)); } } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index 7e31fb4d7..218d784cb 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -53,8 +53,8 @@ private DocumentVersionInfo( this.instanceId = versionDoc.getString(INSTANCE_ID_FIELD).getValue(); this.versionCounter = versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue(); } else { + // TODO(ERIC->ADAM): These seem like odd values as opposed to just indicating there's no version info this.versionDoc = null; - this.syncProtocolVersion = -1; this.instanceId = null; this.versionCounter = -1; @@ -126,12 +126,10 @@ public BsonDocument getFilter() { } static DocumentVersionInfo getLocalVersionInfo( - final CoreDocumentSynchronizationConfig docConfig, - final BsonDocument localDocument + final CoreDocumentSynchronizationConfig docConfig ) { - final BsonDocument version = getDocumentVersion(localDocument); return new DocumentVersionInfo( - version, docConfig.getDocumentId(), docConfig.getLastKnownRemoteVersion() + docConfig.getLastKnownRemoteVersion(), docConfig.getDocumentId(), docConfig.getLastKnownRemoteVersion() ); } diff --git a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index b9b9b72eb..6e3656e9f 100644 --- a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -166,39 +166,39 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { goOffline() val doc3 = Document("so", "syncy") val insResult = coll.insertOneAndSync(doc3) - assertEquals(doc3, withoutVersionId(coll.findOneById(insResult.insertedId)!!)) + assertEquals(doc3, withoutSyncVersion(coll.findOneById(insResult.insertedId)!!)) streamAndSync() assertNull(remoteColl.find(Document("_id", doc3["_id"])).first()) goOnline() streamAndSync() - assertEquals(doc3, withoutVersionId(remoteColl.find(Document("_id", doc3["_id"])).first()!!)) + assertEquals(doc3, withoutSyncVersion(remoteColl.find(Document("_id", doc3["_id"])).first()!!)) // 3. updating a document locally that has been updated remotely should invoke the conflict // resolver. val sem = watchForEvents(this.namespace) val result2 = remoteColl.updateOne( doc1Filter, - withNewVersionIdSet(doc1Update)) + withNewSyncVersionSet(doc1Update)) sem.acquire() assertEquals(1, result2.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) val result3 = coll.updateOneById( doc1Id, doc1Update) assertEquals(1, result3.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) // first pass will invoke the conflict handler and update locally but not remotely yet streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) expectedDocument["foo"] = 4 expectedDocument.remove("fooOps") - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) // second pass will update with the ack'd version id streamAndSync() - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) } } @@ -230,14 +230,14 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { streamAndSync() // Update remote - val remoteUpdate = withNewVersionIdSet(Document("\$set", Document("remote", "update"))) + val remoteUpdate = withNewSyncVersionSet(Document("\$set", Document("remote", "update"))) val sem = watchForEvents(this.namespace) var result = remoteColl.updateOne(doc1Filter, remoteUpdate) sem.acquire() assertEquals(1, result.matchedCount) val expectedRemoteDocument = Document(doc) expectedRemoteDocument["remote"] = "update" - assertEquals(expectedRemoteDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedRemoteDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) // Update local val localUpdate = Document("\$set", Document("local", "updateWow")) @@ -245,18 +245,18 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { assertEquals(1, result.matchedCount) val expectedLocalDocument = Document(doc) expectedLocalDocument["local"] = "updateWow" - assertEquals(expectedLocalDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) // first pass will invoke the conflict handler and update locally but not remotely yet streamAndSync() - assertEquals(expectedRemoteDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedRemoteDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) expectedLocalDocument["remote"] = "update" - assertEquals(expectedLocalDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) // second pass will update with the ack'd version id streamAndSync() - assertEquals(expectedLocalDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) - assertEquals(expectedLocalDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) + assertEquals(expectedLocalDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) } } @@ -281,20 +281,20 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val expectedDocument = Document(doc) val sem = watchForEvents(this.namespace) - var result = remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 2)))) + var result = remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 2)))) sem.acquire() assertEquals(1, result.matchedCount) expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) result = coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) } } @@ -319,20 +319,20 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val expectedDocument = Document(doc) val sem = watchForEvents(this.namespace) - var result = remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 2)))) + var result = remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 2)))) sem.acquire() assertEquals(1, result.matchedCount) expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) result = coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) } } @@ -358,8 +358,8 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val result = coll.deleteOneById(doc1Id) assertEquals(1, result.deletedCount) - val expectedDocument = withoutVersionId(Document(doc)) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + val expectedDocument = withoutSyncVersion(Document(doc)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) assertNull(coll.findOneById(doc1Id)) goOnline() @@ -392,7 +392,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, remoteColl.updateOne( doc1Filter, - withNewVersionIdSet(doc1Update)).matchedCount) + withNewSyncVersionSet(doc1Update)).matchedCount) goOffline() val result = coll.deleteOneById(doc1Id) @@ -400,16 +400,16 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val expectedDocument = Document(doc) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) assertNull(coll.findOneById(doc1Id)) goOnline() streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) expectedDocument.remove("hello") expectedDocument.remove("foo") expectedDocument["well"] = "shoot" - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) } } @@ -432,16 +432,16 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, coll.updateOneById(doc1Id, doc1Update).matchedCount) - val expectedDocument = withoutVersionId(Document(doc)) + val expectedDocument = withoutSyncVersion(Document(doc)) expectedDocument["foo"] = 1 assertNull(remoteColl.find(doc1Filter).first()) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) goOnline() streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) } } @@ -463,20 +463,20 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { goOnline() streamAndSync() - val expectedDocument = withoutVersionId(Document(doc)) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + val expectedDocument = withoutSyncVersion(Document(doc)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, coll.updateOneById(doc1Id, doc1Update).matchedCount) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) } } @@ -495,25 +495,25 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val doc = coll.findOneById(insertResult.insertedId)!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) val doc1Filter = Document("_id", doc1Id) - val expectedDocument = withoutVersionId(Document(doc)) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + val expectedDocument = withoutSyncVersion(Document(doc)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) assertEquals(1, coll.deleteOneById(doc1Id).deletedCount) coll.insertOneAndSync(doc) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) val doc1Update = Document("\$inc", Document("foo", 1)) assertEquals(1, coll.updateOneById(doc1Id, doc1Update).matchedCount) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) } } @@ -620,21 +620,21 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val wait = watchForEvents(this.namespace, 2) remoteColl.deleteOne(doc1Filter) - remoteColl.insertOne(withNewVersionId(doc)) + remoteColl.insertOne(withNewSyncVersion(doc)) wait.acquire() assertEquals(1, coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1))).matchedCount) streamAndSync() - assertEquals(doc, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(doc, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) val expectedDocument = Document("_id", doc1Id.value) expectedDocument["hello"] = "again" - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) } } @@ -646,7 +646,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val remoteColl = getTestCollRemote() val docToInsert = Document("hello", "world") - remoteColl.insertOne(withNewVersionId(docToInsert)) + remoteColl.insertOne(withNewSyncVersion(docToInsert)) val doc = remoteColl.find(docToInsert).first()!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) @@ -660,10 +660,10 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { assertEquals(1, coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1))).matchedCount) streamAndSync() - val expectedDocument = Document(withoutVersionId(doc)) + val expectedDocument = Document(withoutSyncVersion(doc)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) } } @@ -675,7 +675,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val remoteColl = getTestCollRemote() val docToInsert = Document("hello", "world") - remoteColl.insertOne(withNewVersionId(docToInsert)) + remoteColl.insertOne(withNewSyncVersion(docToInsert)) val doc = remoteColl.find(docToInsert).first()!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) @@ -690,15 +690,15 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { assertNotNull(coll.findOneById(doc1Id)) val sem = watchForEvents(this.namespace) - assertEquals(1, remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 1)))).matchedCount) + assertEquals(1, remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 1)))).matchedCount) sem.acquire() assertEquals(1, coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1))).matchedCount) streamAndSync() - val expectedDocument = Document(withoutVersionId(doc)) + val expectedDocument = Document(withoutSyncVersion(doc)) expectedDocument["foo"] = 1 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) goOffline() assertNull(coll.findOneById(doc1Id)) @@ -735,17 +735,17 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { streamAndSync() val expectedDocument = Document(doc) - var result = remoteColl.updateOne(doc1Filter, withNewVersionIdSet(Document("\$inc", Document("foo", 2)))) + var result = remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 2)))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 3 - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) powerCycleDevice() coll.configure(DefaultSyncConflictResolvers.localWins(), null, null) result = coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1))) assertEquals(1, result.matchedCount) expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) powerCycleDevice() coll.configure(DefaultSyncConflictResolvers.localWins(), null, null) @@ -757,12 +757,12 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { streamAndSync() // resolves the conflict expectedDocument["foo"] = 2 - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) powerCycleDevice() coll.configure(DefaultSyncConflictResolvers.localWins(), null, null) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) } } @@ -775,7 +775,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { coll.configure(failingConflictHandler, null, null) val doc1Id = coll.insertOneAndSync(docToInsert).insertedId - assertEquals(docToInsert, withoutVersionId(coll.findOneById(doc1Id)!!)) + assertEquals(docToInsert, withoutSyncVersion(coll.findOneById(doc1Id)!!)) coll.desyncOne(doc1Id) streamAndSync() assertNull(coll.findOneById(doc1Id)) @@ -801,12 +801,12 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { streamAndSync() val expectedDocument = Document(docToInsert) expectedDocument["friend"] = "welcome" - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) - assertEquals(docToInsert, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) + assertEquals(docToInsert, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) streamAndSync() - assertEquals(expectedDocument, withoutVersionId(coll.findOneById(doc1Id)!!)) - assertEquals(expectedDocument, withoutVersionId(remoteColl.find(doc1Filter).first()!!)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id)!!)) + assertEquals(expectedDocument, withoutSyncVersion(remoteColl.find(doc1Filter).first()!!)) } } @@ -847,7 +847,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { // create a conflict var sem = watchForEvents(namespace) - remoteColl.updateOne(Document("_id", result.insertedId), withNewVersionIdSet(Document("\$inc", Document("foo", 2)))) + remoteColl.updateOne(Document("_id", result.insertedId), withNewSyncVersionSet(Document("\$inc", Document("foo", 2)))) sem.acquire() // do a sync pass, and throw an error during the conflict resolver @@ -866,7 +866,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { // it should not have updated the local doc, as the local doc should be frozen assertEquals( withoutId(expectedDoc), - withoutVersionId(withoutId(testSync.find(Document("_id", result.insertedId)).first()!!))) + withoutSyncVersion(withoutId(testSync.find(Document("_id", result.insertedId)).first()!!))) // update the local doc. this should unfreeze the config testSync.updateOneById(result.insertedId, Document("\$set", Document("no", "op"))) @@ -876,7 +876,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { // this should still be the remote doc since remote wins assertEquals( withoutId(nextDoc), - withoutVersionId(withoutId(testSync.find(Document("_id", result.insertedId)).first()!!))) + withoutSyncVersion(withoutId(testSync.find(Document("_id", result.insertedId)).first()!!))) // update the doc remotely val lastDoc = Document("good night", "computer") @@ -884,7 +884,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { sem = watchForEvents(namespace) remoteColl.updateOne( Document("_id", result.insertedId), - withNewVersionId(lastDoc) + withNewSyncVersion(lastDoc) ) sem.acquire() @@ -894,43 +894,43 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { assertEquals( withoutId(lastDoc), - withoutVersionId( + withoutSyncVersion( withoutId(testSync.find(Document("_id", result.insertedId)).first()!!))) } } - @Test - fun testConfigure() { - val testSync = getTestSync() - val remoteColl = getTestCollRemote() - - val docToInsert = Document("hello", "world") - val insertedId = testSync.insertOneAndSync(docToInsert).insertedId - - var hasConflictHandlerBeenInvoked = false - var hasChangeEventListenerBeenInvoked = false - - testSync.configure( - { _: BsonValue, _: ChangeEvent, remoteEvent: ChangeEvent -> - hasConflictHandlerBeenInvoked = true - assertEquals(remoteEvent.fullDocument["fly"], "away") - remoteEvent.fullDocument - }, - { _: BsonValue, _: ChangeEvent -> - hasChangeEventListenerBeenInvoked = true - }, - { _, _ -> } - ) - - val sem = watchForEvents(namespace) - remoteColl.insertOne(Document("_id", insertedId).append("fly", "away")) - sem.acquire() - - streamAndSync() - - assertTrue(hasConflictHandlerBeenInvoked) - assertTrue(hasChangeEventListenerBeenInvoked) - } +// @Test +// fun testConfigure() { +// val testSync = getTestSync() +// val remoteColl = getTestCollRemote() +// +// val docToInsert = Document("hello", "world") +// val insertedId = testSync.insertOneAndSync(docToInsert).insertedId +// +// var hasConflictHandlerBeenInvoked = false +// var hasChangeEventListenerBeenInvoked = false +// +// testSync.configure( +// { _: BsonValue, _: ChangeEvent, remoteEvent: ChangeEvent -> +// hasConflictHandlerBeenInvoked = true +// assertEquals(remoteEvent.fullDocument["fly"], "away") +// remoteEvent.fullDocument +// }, +// { _: BsonValue, _: ChangeEvent -> +// hasChangeEventListenerBeenInvoked = true +// }, +// { _, _ -> } +// ) +// +// val sem = watchForEvents(namespace) +// remoteColl.insertOne(Document("_id", insertedId).append("fly", "away")) +// sem.acquire() +// +// streamAndSync() +// +// assertTrue(hasConflictHandlerBeenInvoked) +// assertTrue(hasChangeEventListenerBeenInvoked) +// } private fun streamAndSync() { val dataSync = (mongoClient as RemoteMongoClientImpl).dataSynchronizer @@ -984,20 +984,20 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { return newDoc } - private fun withoutVersionId(document: Document): Document { + private fun withoutSyncVersion(document: Document): Document { val newDoc = Document(document) newDoc.remove("__stitch_sync_version") return newDoc } - private fun withNewVersionId(document: Document): Document { + private fun withNewSyncVersion(document: Document): Document { val newDocument = Document(java.util.HashMap(document)) newDocument["__stitch_sync_version"] = freshSyncVersionDoc() return newDocument } - private fun withNewVersionIdSet(document: Document): Document { + private fun withNewSyncVersionSet(document: Document): Document { return appendDocumentToKey( "\$set", document, From 3b8e923595802b5954bb3f502269f6e3eb9fd2d0 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Thu, 11 Oct 2018 13:30:08 -0400 Subject: [PATCH 03/10] add a working test for the versioning scheme --- .../internal/SyncMongoClientIntTests.kt | 118 +++++++++++- .../internal/SyncMongoClientIntTests.kt | 174 ++++++++++++++---- 2 files changed, 247 insertions(+), 45 deletions(-) diff --git a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 23ae1d96e..64bb6aba0 100644 --- a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -23,9 +23,7 @@ import org.bson.Document import org.bson.types.ObjectId import org.junit.After import org.junit.Assert -import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull -import org.junit.Assert.fail +import org.junit.Assert.* import org.junit.Assume import org.junit.Before import org.junit.Test @@ -852,7 +850,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { // create a conflict var sem = watchForEvents(namespace) - Tasks.await(remoteColl.updateOne(Document("_id", result.insertedId), withNewVersionIdSet(Document("\$inc", Document("foo", 2))))) + Tasks.await(remoteColl.updateOne(Document("_id", result.insertedId), withNewSyncVersionSet(Document("\$inc", Document("foo", 2))))) sem.acquire() // do a sync pass, and throw an error during the conflict resolver @@ -871,7 +869,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { // it should not have updated the local doc, as the local doc should be frozen assertEquals( withoutId(expectedDoc), - withoutVersionId(withoutId(Tasks.await(testSync.find(Document("_id", result.insertedId)).first())!!))) + withoutSyncVersion(withoutId(Tasks.await(testSync.find(Document("_id", result.insertedId)).first())!!))) // update the local doc. this should unfreeze the config Tasks.await(testSync.updateOneById(result.insertedId, Document("\$set", Document("no", "op")))) @@ -881,7 +879,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { // this should still be the remote doc since remote wins assertEquals( withoutId(nextDoc), - withoutVersionId(withoutId(Tasks.await(testSync.find(Document("_id", result.insertedId)).first())!!))) + withoutSyncVersion(withoutId(Tasks.await(testSync.find(Document("_id", result.insertedId)).first())!!))) // update the doc remotely val lastDoc = Document("good night", "computer") @@ -889,7 +887,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { sem = watchForEvents(namespace) Tasks.await(remoteColl.updateOne( Document("_id", result.insertedId), - withNewVersionId(lastDoc) + withNewSyncVersion(lastDoc) )) sem.acquire() @@ -899,7 +897,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { assertEquals( withoutId(lastDoc), - withoutVersionId( + withoutSyncVersion( withoutId(Tasks.await(testSync.find(Document("_id", result.insertedId)).first())!!))) } } @@ -937,6 +935,98 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { Assert.assertTrue(hasChangeEventListenerBeenInvoked) } + @Test + fun testSyncVersioningScheme() { + testSyncInBothDirections { + val coll = getTestSync() + + val remoteColl = getTestCollRemote() + + val docToInsert = Document("hello", "world") + + coll.configure(failingConflictHandler, null, null) + val insertResult = Tasks.await(coll.insertOneAndSync(docToInsert)) + + val doc = Tasks.await(coll.findOneById(insertResult.insertedId))!! + val doc1Id = BsonObjectId(doc.getObjectId("_id")) + val doc1Filter = Document("_id", doc1Id) + + goOnline() + streamAndSync() + val expectedDocument = Document(doc) + + // the remote document after an initial insert should have a fresh instance ID, and a + // version counter of 0 + val firstRemoteDoc = Tasks.await(remoteColl.find(doc1Filter).first())!! + assertEquals(expectedDocument, withoutSyncVersion(firstRemoteDoc)) + + assertEquals(0, versionCounterOf(firstRemoteDoc)) + + assertEquals(expectedDocument, Tasks.await(coll.findOneById(doc1Id))!!) + + // the remote document after a local update, but before a sync pass, should have the + // same version as the original document, and be equivalent to the unupdated document + val doc1Update = Document("\$inc", Document("foo", 1)) + assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, doc1Update)).matchedCount) + + val secondRemoteDocBeforeSyncPass = Tasks.await(remoteColl.find(doc1Filter).first())!! + assertEquals(expectedDocument, withoutSyncVersion(secondRemoteDocBeforeSyncPass)) + assertEquals(versionOf(firstRemoteDoc), versionOf(secondRemoteDocBeforeSyncPass)) + + expectedDocument["foo"] = 1 + assertEquals(expectedDocument, Tasks.await(coll.findOneById(doc1Id))!!) + + // the remote document after a local update, and after a sync pass, should have a new + // version with the same instance ID as the original document, a version counter + // incremented by 1, and be equivalent to the updated document. + streamAndSync() + val secondRemoteDoc = Tasks.await(remoteColl.find(doc1Filter).first())!! + assertEquals(expectedDocument, withoutSyncVersion(secondRemoteDoc)) + assertEquals(instanceIdOf(firstRemoteDoc), instanceIdOf(secondRemoteDoc)) + assertEquals(1, versionCounterOf(secondRemoteDoc)) + + assertEquals(expectedDocument, Tasks.await(coll.findOneById(doc1Id))!!) + + // the remote document after a local delete and local insert, but before a sync pass, + // should have the same version as the previous document + assertEquals(1, Tasks.await(coll.deleteOneById(doc1Id)).deletedCount) + Tasks.await(coll.insertOneAndSync(doc)) + + val thirdRemoteDocBeforeSyncPass = Tasks.await(remoteColl.find(doc1Filter).first())!! + assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDocBeforeSyncPass)) + + expectedDocument.remove("foo") + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id)))!!) + + // the remote document after a local delete and local insert, and after a sync pass, + // should have the same instance ID as before and a version count, since the change + // events are coalesced into a single update event + streamAndSync() + + val thirdRemoteDoc = Tasks.await(remoteColl.find(doc1Filter).first())!! + assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDoc)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id)))) + + assertEquals(instanceIdOf(secondRemoteDoc), instanceIdOf(thirdRemoteDoc)) + assertEquals(2, versionCounterOf(thirdRemoteDoc)) + + // the remote document after a local delete, a sync pass, a local insert, and after + // another sync pass should have a new instance ID, with a version counter of zero, + // since the change events are not coalesced + assertEquals(1, Tasks.await(coll.deleteOneById(doc1Id)).deletedCount) + streamAndSync() + Tasks.await(coll.insertOneAndSync(doc)) + streamAndSync() + + val fourthRemoteDoc = Tasks.await(remoteColl.find(doc1Filter).first())!! + assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDoc)) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id)))) + + assertNotEquals(instanceIdOf(secondRemoteDoc), instanceIdOf(fourthRemoteDoc)) + assertEquals(0, versionCounterOf(fourthRemoteDoc)) + } + } + private fun streamAndSync() { val dataSync = (mongoClient as RemoteMongoClientImpl).dataSynchronizer if (testNetworkMonitor.connectedState) { @@ -1013,6 +1103,18 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { return Document("spv", 1).append("id", UUID.randomUUID().toString()).append("v", 0L) } + private fun versionOf(document: Document): Document { + return document["__stitch_sync_version"] as Document + } + + private fun versionCounterOf(document: Document): Long { + return versionOf(document)["v"] as Long + } + + private fun instanceIdOf(document: Document): String { + return versionOf(document)["id"] as String + } + private fun appendDocumentToKey(key: String, on: Document, toAppend: Document): Document { val newDocument = Document(HashMap(on)) var found = false diff --git a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 6e3656e9f..497ab1c22 100644 --- a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -20,11 +20,7 @@ import org.bson.BsonValue import org.bson.Document import org.bson.types.ObjectId import org.junit.After -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue -import org.junit.Assert.assertNull -import org.junit.Assert.assertNotNull -import org.junit.Assert.fail +import org.junit.Assert.* import org.junit.Assume import org.junit.Before import org.junit.Test @@ -899,38 +895,38 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { } } -// @Test -// fun testConfigure() { -// val testSync = getTestSync() -// val remoteColl = getTestCollRemote() -// -// val docToInsert = Document("hello", "world") -// val insertedId = testSync.insertOneAndSync(docToInsert).insertedId -// -// var hasConflictHandlerBeenInvoked = false -// var hasChangeEventListenerBeenInvoked = false -// -// testSync.configure( -// { _: BsonValue, _: ChangeEvent, remoteEvent: ChangeEvent -> -// hasConflictHandlerBeenInvoked = true -// assertEquals(remoteEvent.fullDocument["fly"], "away") -// remoteEvent.fullDocument -// }, -// { _: BsonValue, _: ChangeEvent -> -// hasChangeEventListenerBeenInvoked = true -// }, -// { _, _ -> } -// ) -// -// val sem = watchForEvents(namespace) -// remoteColl.insertOne(Document("_id", insertedId).append("fly", "away")) -// sem.acquire() -// -// streamAndSync() -// -// assertTrue(hasConflictHandlerBeenInvoked) -// assertTrue(hasChangeEventListenerBeenInvoked) -// } + @Test + fun testConfigure() { + val testSync = getTestSync() + val remoteColl = getTestCollRemote() + + val docToInsert = Document("hello", "world") + val insertedId = testSync.insertOneAndSync(docToInsert).insertedId + + var hasConflictHandlerBeenInvoked = false + var hasChangeEventListenerBeenInvoked = false + + testSync.configure( + { _: BsonValue, _: ChangeEvent, remoteEvent: ChangeEvent -> + hasConflictHandlerBeenInvoked = true + assertEquals(remoteEvent.fullDocument["fly"], "away") + remoteEvent.fullDocument + }, + { _: BsonValue, _: ChangeEvent -> + hasChangeEventListenerBeenInvoked = true + }, + { _, _ -> } + ) + + val sem = watchForEvents(namespace) + remoteColl.insertOne(Document("_id", insertedId).append("fly", "away")) + sem.acquire() + + streamAndSync() + + assertTrue(hasConflictHandlerBeenInvoked) + assertTrue(hasChangeEventListenerBeenInvoked) + } private fun streamAndSync() { val dataSync = (mongoClient as RemoteMongoClientImpl).dataSynchronizer @@ -943,6 +939,98 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { dataSync.doSyncPass() } + @Test + fun testSyncVersioningScheme() { + testSyncInBothDirections { + val coll = getTestSync() + + val remoteColl = getTestCollRemote() + + val docToInsert = Document("hello", "world") + + coll.configure(failingConflictHandler, null, null) + val insertResult = coll.insertOneAndSync(docToInsert) + + val doc = coll.findOneById(insertResult.insertedId) + val doc1Id = BsonObjectId(doc.getObjectId("_id")) + val doc1Filter = Document("_id", doc1Id) + + goOnline() + streamAndSync() + val expectedDocument = Document(doc) + + // the remote document after an initial insert should have a fresh instance ID, and a + // version counter of 0 + val firstRemoteDoc = remoteColl.find(doc1Filter).first()!! + assertEquals(expectedDocument, withoutSyncVersion(firstRemoteDoc)) + + assertEquals(0, versionCounterOf(firstRemoteDoc)) + + assertEquals(expectedDocument, coll.findOneById(doc1Id)) + + // the remote document after a local update, but before a sync pass, should have the + // same version as the original document, and be equivalent to the unupdated document + val doc1Update = Document("\$inc", Document("foo", 1)) + assertEquals(1, coll.updateOneById(doc1Id, doc1Update).matchedCount) + + val secondRemoteDocBeforeSyncPass = remoteColl.find(doc1Filter).first()!! + assertEquals(expectedDocument, withoutSyncVersion(secondRemoteDocBeforeSyncPass)) + assertEquals(versionOf(firstRemoteDoc), versionOf(secondRemoteDocBeforeSyncPass)) + + expectedDocument["foo"] = 1 + assertEquals(expectedDocument, coll.findOneById(doc1Id)) + + // the remote document after a local update, and after a sync pass, should have a new + // version with the same instance ID as the original document, a version counter + // incremented by 1, and be equivalent to the updated document. + streamAndSync() + val secondRemoteDoc = remoteColl.find(doc1Filter).first()!! + assertEquals(expectedDocument, withoutSyncVersion(secondRemoteDoc)) + assertEquals(instanceIdOf(firstRemoteDoc), instanceIdOf(secondRemoteDoc)) + assertEquals(1, versionCounterOf(secondRemoteDoc)) + + assertEquals(expectedDocument, coll.findOneById(doc1Id)) + + // the remote document after a local delete and local insert, but before a sync pass, + // should have the same version as the previous document + assertEquals(1, coll.deleteOneById(doc1Id).deletedCount) + coll.insertOneAndSync(doc) + + val thirdRemoteDocBeforeSyncPass = remoteColl.find(doc1Filter).first()!! + assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDocBeforeSyncPass)) + + expectedDocument.remove("foo") + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id))!!) + + // the remote document after a local delete and local insert, and after a sync pass, + // should have the same instance ID as before and a version count, since the change + // events are coalesced into a single update event + streamAndSync() + + val thirdRemoteDoc = remoteColl.find(doc1Filter).first()!! + assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDoc)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id))) + + assertEquals(instanceIdOf(secondRemoteDoc), instanceIdOf(thirdRemoteDoc)) + assertEquals(2, versionCounterOf(thirdRemoteDoc)) + + // the remote document after a local delete, a sync pass, a local insert, and after + // another sync pass should have a new instance ID, with a version counter of zero, + // since the change events are not coalesced + assertEquals(1, coll.deleteOneById(doc1Id).deletedCount) + streamAndSync() + coll.insertOneAndSync(doc) + streamAndSync() + + val fourthRemoteDoc = remoteColl.find(doc1Filter).first()!! + assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDoc)) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id))) + + assertNotEquals(instanceIdOf(secondRemoteDoc), instanceIdOf(fourthRemoteDoc)) + assertEquals(0, versionCounterOf(fourthRemoteDoc)) + } + } + private fun watchForEvents( namespace: MongoNamespace, n: Int = 1 @@ -1008,6 +1096,18 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { return Document("spv", 1).append("id", UUID.randomUUID().toString()).append("v", 0L) } + private fun versionOf(document: Document): Document { + return document["__stitch_sync_version"] as Document + } + + private fun versionCounterOf(document: Document): Long { + return versionOf(document)["v"] as Long + } + + private fun instanceIdOf(document: Document): String { + return versionOf(document)["id"] as String + } + private fun appendDocumentToKey(key: String, on: Document, toAppend: Document): Document { val newDocument = Document(HashMap(on)) var found = false From f8ba543de0a400e2dc15f418ef07f1f0a6ad2453 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Thu, 11 Oct 2018 16:50:15 -0400 Subject: [PATCH 04/10] add test for unsupported spv, and clean up --- .../internal/SyncMongoClientIntTests.kt | 40 +++- .../CoreDocumentSynchronizationConfig.java | 32 ++- .../sync/internal/DataSynchronizer.java | 92 ++++----- .../sync/internal/DocumentVersionInfo.java | 182 ++++++++---------- .../internal/SyncMongoClientIntTests.kt | 37 ++++ 5 files changed, 226 insertions(+), 157 deletions(-) diff --git a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 64bb6aba0..61f9142ed 100644 --- a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -16,6 +16,7 @@ import com.mongodb.stitch.core.internal.common.OperationResult import com.mongodb.stitch.core.services.mongodb.remote.sync.ConflictHandler import com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent import com.mongodb.stitch.core.services.mongodb.remote.sync.DefaultSyncConflictResolvers +import com.mongodb.stitch.core.services.mongodb.remote.sync.ErrorListener import org.bson.BsonDocument import org.bson.BsonObjectId import org.bson.BsonValue @@ -996,7 +997,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDocBeforeSyncPass)) expectedDocument.remove("foo") - assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id)))!!) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id)))) // the remote document after a local delete and local insert, and after a sync pass, // should have the same instance ID as before and a version count, since the change @@ -1027,6 +1028,32 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { } } + @Test + fun testUnsupportedSpvFails() { + val coll = getTestSync() + + val remoteColl = getTestCollRemote() + + val docToInsert = withNewUnsupportedSyncVersion(Document("hello", "world")) + + coll.configure(failingConflictHandler, null, null) + val insertResult = Tasks.await(remoteColl.insertOne(docToInsert)) + + val doc = Tasks.await(remoteColl.find(docToInsert).first())!! + val doc1Id = BsonObjectId(doc.getObjectId("_id")) + val doc1Filter = Document("_id", doc1Id) + + coll.syncOne(doc1Id) + + assertTrue(coll.syncedIds.contains(doc1Id)) + + // syncing on this document with an unsupported spv should cause the document to desync + goOnline() + streamAndSync() + + assertFalse(coll.syncedIds.contains(doc1Id)) + } + private fun streamAndSync() { val dataSync = (mongoClient as RemoteMongoClientImpl).dataSynchronizer if (testNetworkMonitor.connectedState) { @@ -1099,6 +1126,17 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { return newDocument } + private fun withNewUnsupportedSyncVersion(document: Document): Document { + val newDocument = Document(java.util.HashMap(document)) + val badVersion = freshSyncVersionDoc() + badVersion.remove("spv") + badVersion.append("spv", 2) + + newDocument["__stitch_sync_version"] = badVersion + + return newDocument + } + private fun freshSyncVersionDoc(): Document { return Document("spv", 1).append("id", UUID.randomUUID().toString()).append("v", 0L) } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java index 60f645dc7..32854dec0 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java @@ -22,14 +22,10 @@ import com.mongodb.client.MongoCollection; import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.annotation.Nonnull; -import org.bson.BsonArray; import org.bson.BsonBinary; import org.bson.BsonBinaryReader; import org.bson.BsonBinaryWriter; @@ -254,6 +250,19 @@ void setPendingWritesComplete(final BsonDocument atVersion) { } } + void setLastKnownRemoteVersion(final BsonDocument lastKnownRemoteVersion) { + docLock.writeLock().lock(); + try { + this.lastKnownRemoteVersion = lastKnownRemoteVersion; + + docsColl.replaceOne( + getDocFilter(namespace, documentId), + this); + } finally { + docLock.writeLock().unlock(); + } + } + // Equality on documentId @Override public boolean equals(final Object object) { @@ -338,6 +347,21 @@ public BsonDocument getLastKnownRemoteVersion() { } } + public boolean hasCommittedVersion(final DocumentVersionInfo version) { + docLock.readLock().lock(); + try { + DocumentVersionInfo remoteVersionInfo = + DocumentVersionInfo.fromVersionDoc(lastKnownRemoteVersion); + + return ((version.isNonEmptyVersion() && remoteVersionInfo.isNonEmptyVersion() && + (version.getSyncProtocolVersion() == remoteVersionInfo.getSyncProtocolVersion()) && + (version.getInstanceId().equals(remoteVersionInfo.getInstanceId())) && + (version.getVersionCounter() >= remoteVersionInfo.getVersionCounter()))); + } finally { + docLock.readLock().unlock(); + } + } + /** * Possibly coalesces the newest change event to match the user's original intent. For example, * an unsynchronized insert and update is still an insert. diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index be561df46..a44de6559 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -20,7 +20,7 @@ import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalInsert; import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalReplace; import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalUpdate; -import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.DocumentVersionInfo.getDocumentVersion; +import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.DocumentVersionInfo.getDocumentVersionDoc; import com.mongodb.MongoClientSettings; import com.mongodb.MongoNamespace; @@ -67,7 +67,6 @@ import org.bson.BsonArray; import org.bson.BsonBoolean; import org.bson.BsonDocument; -import org.bson.BsonInt64; import org.bson.BsonValue; import org.bson.Document; import org.bson.codecs.Codec; @@ -525,7 +524,7 @@ private void syncRemoteChangeEventToLocal( final BsonDocument docFilter = getDocumentIdFilter(docConfig.getDocumentId()); final BsonDocument localDocument = localColl.find(docFilter).first(); - final BsonValue lastKnownRemoteVersion; + final BsonDocument lastKnownRemoteVersion; if (remoteDoc == null || remoteDoc.size() == 0) { lastKnownRemoteVersion = null; } else { @@ -535,14 +534,33 @@ private void syncRemoteChangeEventToLocal( DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig); - // TODO(QUESTION FOR REVIEW): The spec seems to indicate that we should make this check in - // place of the committed versions check, but I have two questions: - // 1. this alone does not make all the tests pass. is this because this check doesn't - // adequately cover all cases where we need to drop the handling of the event? or is it - // becase - if ((localVersion.getVersionDoc() != null && lastKnownRemoteVersion != null) && - (localVersion.getInstanceId().equals(remoteVersionInfo.getInstanceId())) && - (localVersion.getVersionCounter() >= remoteVersionInfo.getVersionCounter())) { + if (remoteVersionInfo.isNonEmptyVersion() && remoteVersionInfo.getSyncProtocolVersion() != 1) { + desyncDocumentFromRemote(nsConfig.getNamespace(), docConfig.getDocumentId()); + + emitError(docConfig, + String.format( + Locale.US, + "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s got a remote" + + "document with an unsupported synchronization protocol version %d; " + + "dropping the event, and desyncing the document", + logicalT, + nsConfig.getNamespace(), + docConfig.getDocumentId(), + remoteVersionInfo.getSyncProtocolVersion(), + remoteChangeEvent.getOperationType().toString())); + + return; + } + + // TODO(QUESTION FOR REVIEW): The testFrozenDocumentConfig test fails if I call this, but + // other tests pass. Shouldn't the receipt of this document indicate that this is the last + // known remote version of this document,? +// docConfig.setLastKnownRemoteVersion(lastKnownRemoteVersion); + + // TODO(QFR): I ask because when we are looking at hasCommittedVersion, we are using the + // lastKnownRemoteVersion of the doc config rather than the last known remote version based + // on the incoming change event we got here, which would make this not conform to spec. + if (docConfig.hasCommittedVersion(localVersion)) { // Skip this event since we generated it. logger.info(String.format( Locale.US, @@ -572,7 +590,7 @@ private void syncRemoteChangeEventToLocal( nsConfig.getNamespace(), docConfig.getDocumentId(), remoteChangeEvent.getFullDocument(), - getDocumentVersion(remoteChangeEvent.getFullDocument())); + getDocumentVersionDoc(remoteChangeEvent.getFullDocument())); return; case DELETE: logger.info(String.format( @@ -730,7 +748,7 @@ private void syncLocalToRemote() { continue; } final BsonDocument nextDoc = withNextVersionCounter(localDoc, localVersionInfo); - nextVersion = getDocumentVersion(nextDoc); + nextVersion = getDocumentVersionDoc(nextDoc); final RemoteUpdateResult result; try { @@ -776,7 +794,7 @@ private void syncLocalToRemote() { continue; } final BsonDocument nextDoc = withNextVersionCounter(localDoc, localVersionInfo); - nextVersion = DocumentVersionInfo.getDocumentVersion(nextDoc); + nextVersion = DocumentVersionInfo.getDocumentVersionDoc(nextDoc); final BsonDocument translatedUpdate = new BsonDocument(); if (!localChangeEvent.getUpdateDescription().getUpdatedFields().isEmpty()) { @@ -1372,28 +1390,11 @@ private void replaceOrUpsertOneFromResolution( return; } - // TODO(QUESTION FOR REVIEW): in the spec, we specify that replaces should result in an - // increment rather than a new instance ID. if atVersion is not null, we can do this, but if - // it doesn't, meaning we are replacing a document with no version, we simply use a fresh - // version with a new instance ID. My question is, would it be better if we just always - // use a fresh version with a new instance ID? We don't have a huge amount of information - // here about the original document, and it seems like a replace is semantically similar to a - // delete followed by insert, which would result in a new instance ID rather than an increment - final BsonDocument docToReplace; - final BsonDocument newVersion; - if (atVersion == null) { - newVersion = DocumentVersionInfo.getFreshVersionDocument(); - docToReplace = withNewVersion( - document, - newVersion - ); - } else { - newVersion = DocumentVersionInfo.withIncrementedVersionCounter(atVersion); - docToReplace = withNewVersion( - document, - newVersion - ); - } + final BsonDocument docToReplace = withNextVersionCounter( + document, + DocumentVersionInfo.getLocalVersionInfo(config) + ); + final BsonDocument result = getLocalCollection(namespace) .findOneAndReplace( getDocumentIdFilter(documentId), @@ -1757,23 +1758,10 @@ private static BsonDocument withNewVersion( return newDocument; } - private static BsonDocument getNextVersion( - final DocumentVersionInfo versionInfo - ) { - if (versionInfo.getVersionDoc() == null) { - return DocumentVersionInfo.getFreshVersionDocument(); - } - final BsonDocument nextVersion = BsonUtils.copyOfDocument(versionInfo.getVersionDoc()); - nextVersion.put( - DocumentVersionInfo.VERSION_COUNTER_FIELD, - new BsonInt64(versionInfo.getVersionCounter() + 1)); - return nextVersion; - } - private static BsonDocument withNextVersionCounter( - final BsonDocument document, - final DocumentVersionInfo versionInfo + final BsonDocument document, + final DocumentVersionInfo versionInfo ) { - return withNewVersion(document, getNextVersion(versionInfo)); + return withNewVersion(document, DocumentVersionInfo.getNextVersion(versionInfo)); } } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index 218d784cb..edfa311ee 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -19,6 +19,8 @@ import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.DataSynchronizer.DOCUMENT_VERSION_FIELD; import com.mongodb.stitch.core.internal.common.BsonUtils; + +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.bson.BsonBoolean; import org.bson.BsonDocument; @@ -29,99 +31,83 @@ import java.util.UUID; -public final class DocumentVersionInfo { - private final int syncProtocolVersion; - private final String instanceId; - private final long versionCounter; - +final class DocumentVersionInfo { + @Nullable private final VersionValues versionValues; + @Nullable private final BsonDocument versionDoc; + @Nullable private final BsonDocument filter; + + private class VersionValues { + final int syncProtocolVersion; + final String instanceId; + final long versionCounter; + + VersionValues( + final int syncProtocolVersion, + final String instanceId, + final long versionCounter) { + this.syncProtocolVersion = syncProtocolVersion; + this.instanceId = instanceId; + this.versionCounter = versionCounter; + } - static final String SYNC_PROTOCOL_VERSION_FIELD = "spv"; - static final String INSTANCE_ID_FIELD = "id"; - static final String VERSION_COUNTER_FIELD = "v"; + } - private final BsonDocument filter; - private final BsonDocument versionDoc; + private static final String SYNC_PROTOCOL_VERSION_FIELD = "spv"; + private static final String INSTANCE_ID_FIELD = "id"; + private static final String VERSION_COUNTER_FIELD = "v"; private DocumentVersionInfo( - final BsonDocument version, - final BsonValue documentId, - final BsonDocument versionForFilter + @Nullable final BsonDocument version, + @Nullable final BsonValue documentId ) { if (version != null) { this.versionDoc = version; - this.syncProtocolVersion = versionDoc.getInt32(SYNC_PROTOCOL_VERSION_FIELD).getValue(); - this.instanceId = versionDoc.getString(INSTANCE_ID_FIELD).getValue(); - this.versionCounter = versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue(); + this.versionValues = new VersionValues( + versionDoc.getInt32(SYNC_PROTOCOL_VERSION_FIELD).getValue(), + versionDoc.getString(INSTANCE_ID_FIELD).getValue(), + versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue() + ); } else { - // TODO(ERIC->ADAM): These seem like odd values as opposed to just indicating there's no version info this.versionDoc = null; - this.syncProtocolVersion = -1; - this.instanceId = null; - this.versionCounter = -1; + this.versionValues = null; } - this.filter = getVersionedFilter(documentId, versionForFilter); + if (documentId != null) { + this.filter = getVersionedFilter(documentId, version); + } else { + this.filter = null; + } } -// private DocumentVersionInfo( -// final BsonValue documentId, -// final int syncProtocolVersion, -// final String instanceId, -// final long versionCounter -// ) { -// this.syncProtocolVersion = syncProtocolVersion; -// this.instanceId = instanceId; -// this.versionCounter = versionCounter; -// -// this.versionDoc = constructVersionDoc(); -// this.filter = getVersionedFilter(documentId, versionDoc); -// } - - public BsonDocument getVersionDoc() { + @Nullable BsonDocument getVersionDoc() { return versionDoc; } -// private BsonDocument constructVersionDoc() { -// BsonDocument versionDoc = new BsonDocument(); -// -// versionDoc.append(SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(syncProtocolVersion)); -// versionDoc.append(INSTANCE_ID_FIELD, new BsonString(instanceId)); -// versionDoc.append(VERSION_COUNTER_FIELD, new BsonInt64(versionCounter)); -// -// return versionDoc; -// } - - public int getSyncProtocolVersion() { - return syncProtocolVersion; + boolean isNonEmptyVersion() { return versionValues != null; } + + private @Nonnull VersionValues getVersionValuesThrowIfNull() { + if (this.versionValues == null) { + throw new IllegalStateException( + "Attempting to access sync version information on a document with no version." + ); + } + + return this.versionValues; } - public String getInstanceId() { - return instanceId; + int getSyncProtocolVersion() { + return getVersionValuesThrowIfNull().syncProtocolVersion; } - public long getVersionCounter() { - return versionCounter; + String getInstanceId() { + return getVersionValuesThrowIfNull().instanceId; } -// public DocumentVersionInfo withIncrementedVersionCounter() { -// return new DocumentVersionInfo( -// this.documentId, -// this.syncProtocolVersion, -// this.instanceId, -// this.versionCounter + 1 -// ); -// } -// -// public DocumentVersionInfo withNewInstanceId() { -// return new DocumentVersionInfo( -// this.documentId, -// this.syncProtocolVersion, -// UUID.randomUUID().toString(), -// 0 -// ); -// } - - public BsonDocument getFilter() { + long getVersionCounter() { + return getVersionValuesThrowIfNull().versionCounter; + } + + @Nullable BsonDocument getFilter() { return filter; } @@ -129,53 +115,36 @@ static DocumentVersionInfo getLocalVersionInfo( final CoreDocumentSynchronizationConfig docConfig ) { return new DocumentVersionInfo( - docConfig.getLastKnownRemoteVersion(), docConfig.getDocumentId(), docConfig.getLastKnownRemoteVersion() + docConfig.getLastKnownRemoteVersion(), + docConfig.getDocumentId() ); } static DocumentVersionInfo getRemoteVersionInfo(final BsonDocument remoteDocument) { - final BsonDocument version = getDocumentVersion(remoteDocument); - return new DocumentVersionInfo(version, BsonUtils.getDocumentId(remoteDocument), version); + final BsonDocument version = getDocumentVersionDoc(remoteDocument); + return new DocumentVersionInfo(version, BsonUtils.getDocumentId(remoteDocument)); } -// public static DocumentVersionInfo createFreshDocumentVersion(BsonDocument document) { -// return new DocumentVersionInfo( -// BsonUtils.getDocumentId(document), -// 1, -// UUID.randomUUID().toString(), -// 0 -// ); -// } + static DocumentVersionInfo fromVersionDoc(final BsonDocument versionDoc) { + return new DocumentVersionInfo(versionDoc, null); + } - public static BsonDocument getFreshVersionDocument() { + static BsonDocument getFreshVersionDocument() { BsonDocument versionDoc = new BsonDocument(); versionDoc.append(SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(1)); versionDoc.append(INSTANCE_ID_FIELD, new BsonString(UUID.randomUUID().toString())); - versionDoc.append(VERSION_COUNTER_FIELD, new BsonInt64(0)); + versionDoc.append(VERSION_COUNTER_FIELD, new BsonInt64(0L)); return versionDoc; } - public static BsonDocument withIncrementedVersionCounter(BsonDocument versionDoc) { - BsonDocument newVersionDoc = new BsonDocument(); - - newVersionDoc.put(SYNC_PROTOCOL_VERSION_FIELD, versionDoc.get(SYNC_PROTOCOL_VERSION_FIELD)); - newVersionDoc.put(INSTANCE_ID_FIELD, versionDoc.get(INSTANCE_ID_FIELD)); - newVersionDoc.put( - VERSION_COUNTER_FIELD, - new BsonInt64(versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue() + 1L) - ); - - return newVersionDoc; - } - /** - * Returns the version of the given document, if any; returns null otherwise. + * Returns the version document of the given document, if any; returns null otherwise. * @param document the document to get the version from. * @return the version of the given document, if any; returns null otherwise. */ - static BsonDocument getDocumentVersion(final BsonDocument document) { + static BsonDocument getDocumentVersionDoc(final BsonDocument document) { if (document == null || !document.containsKey(DOCUMENT_VERSION_FIELD)) { return null; } @@ -193,7 +162,7 @@ static BsonDocument getDocumentVersion(final BsonDocument document) { * @return a query filter for the given document _id and version for a remote operation. */ static BsonDocument getVersionedFilter( - final BsonValue documentId, + @Nonnull final BsonValue documentId, @Nullable final BsonValue version ) { final BsonDocument filter = new BsonDocument("_id", documentId); @@ -204,4 +173,17 @@ static BsonDocument getVersionedFilter( } return filter; } + + static BsonDocument getNextVersion( + final DocumentVersionInfo versionInfo + ) { + if (versionInfo.getVersionDoc() == null) { + return getFreshVersionDocument(); + } + final BsonDocument nextVersion = BsonUtils.copyOfDocument(versionInfo.getVersionDoc()); + nextVersion.put( + VERSION_COUNTER_FIELD, + new BsonInt64(versionInfo.getVersionCounter() + 1)); + return nextVersion; + } } diff --git a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 497ab1c22..89b398a55 100644 --- a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -1031,6 +1031,32 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { } } + @Test + fun testUnsupportedSpvFails() { + val coll = getTestSync() + + val remoteColl = getTestCollRemote() + + val docToInsert = withNewUnsupportedSyncVersion(Document("hello", "world")) + + coll.configure(failingConflictHandler, null, null) + val insertResult = remoteColl.insertOne(docToInsert) + + val doc = remoteColl.find(docToInsert).first()!! + val doc1Id = BsonObjectId(doc.getObjectId("_id")) + val doc1Filter = Document("_id", doc1Id) + + coll.syncOne(doc1Id) + + assertTrue(coll.syncedIds.contains(doc1Id)) + + // syncing on this document with an unsupported spv should cause the document to desync + goOnline() + streamAndSync() + + assertFalse(coll.syncedIds.contains(doc1Id)) + } + private fun watchForEvents( namespace: MongoNamespace, n: Int = 1 @@ -1092,6 +1118,17 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { Document("__stitch_sync_version", freshSyncVersionDoc())) } + private fun withNewUnsupportedSyncVersion(document: Document): Document { + val newDocument = Document(java.util.HashMap(document)) + val badVersion = freshSyncVersionDoc() + badVersion.remove("spv") + badVersion.append("spv", 2) + + newDocument["__stitch_sync_version"] = badVersion + + return newDocument + } + private fun freshSyncVersionDoc(): Document { return Document("spv", 1).append("id", UUID.randomUUID().toString()).append("v", 0L) } From 9957b78cc28eb3bbd0d8024f2a69200d7ff309d9 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Thu, 11 Oct 2018 17:41:53 -0400 Subject: [PATCH 05/10] fix checkstyle and findbugs --- .../CoreDocumentSynchronizationConfig.java | 10 +-- .../sync/internal/DataSynchronizer.java | 41 +++++------ .../sync/internal/DocumentVersionInfo.java | 71 ++++++++++++++++--- 3 files changed, 86 insertions(+), 36 deletions(-) diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java index 32854dec0..00f6d9f58 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java @@ -350,13 +350,13 @@ public BsonDocument getLastKnownRemoteVersion() { public boolean hasCommittedVersion(final DocumentVersionInfo version) { docLock.readLock().lock(); try { - DocumentVersionInfo remoteVersionInfo = + final DocumentVersionInfo remoteVersionInfo = DocumentVersionInfo.fromVersionDoc(lastKnownRemoteVersion); - return ((version.isNonEmptyVersion() && remoteVersionInfo.isNonEmptyVersion() && - (version.getSyncProtocolVersion() == remoteVersionInfo.getSyncProtocolVersion()) && - (version.getInstanceId().equals(remoteVersionInfo.getInstanceId())) && - (version.getVersionCounter() >= remoteVersionInfo.getVersionCounter()))); + return ((version.isNonEmptyVersion() && remoteVersionInfo.isNonEmptyVersion() + && (version.getSyncProtocolVersion() == remoteVersionInfo.getSyncProtocolVersion()) + && (version.getInstanceId().equals(remoteVersionInfo.getInstanceId())) + && (version.getVersionCounter() >= remoteVersionInfo.getVersionCounter()))); } finally { docLock.readLock().unlock(); } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index a44de6559..eea94c3b8 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -528,21 +528,22 @@ private void syncRemoteChangeEventToLocal( if (remoteDoc == null || remoteDoc.size() == 0) { lastKnownRemoteVersion = null; } else { - DocumentVersionInfo remoteVersionInfo = DocumentVersionInfo + final DocumentVersionInfo remoteVersionInfo = DocumentVersionInfo .getRemoteVersionInfo(remoteDoc); lastKnownRemoteVersion = remoteVersionInfo.getVersionDoc(); - DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig); + final DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig); - if (remoteVersionInfo.isNonEmptyVersion() && remoteVersionInfo.getSyncProtocolVersion() != 1) { + if (remoteVersionInfo.isNonEmptyVersion() && + remoteVersionInfo.getSyncProtocolVersion() != 1) { desyncDocumentFromRemote(nsConfig.getNamespace(), docConfig.getDocumentId()); emitError(docConfig, String.format( Locale.US, "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s got a remote" - + "document with an unsupported synchronization protocol version %d; " - + "dropping the event, and desyncing the document", + + "document with an unsupported synchronization protocol version " + + "%d; dropping the event, and desyncing the document", logicalT, nsConfig.getNamespace(), docConfig.getDocumentId(), @@ -554,23 +555,23 @@ private void syncRemoteChangeEventToLocal( // TODO(QUESTION FOR REVIEW): The testFrozenDocumentConfig test fails if I call this, but // other tests pass. Shouldn't the receipt of this document indicate that this is the last - // known remote version of this document,? -// docConfig.setLastKnownRemoteVersion(lastKnownRemoteVersion); + // known remote version of this document? + // docConfig.setLastKnownRemoteVersion(lastKnownRemoteVersion); // TODO(QFR): I ask because when we are looking at hasCommittedVersion, we are using the // lastKnownRemoteVersion of the doc config rather than the last known remote version based // on the incoming change event we got here, which would make this not conform to spec. if (docConfig.hasCommittedVersion(localVersion)) { - // Skip this event since we generated it. - logger.info(String.format( - Locale.US, - "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote change event was " - + "generated by us; dropping the event", - logicalT, - nsConfig.getNamespace(), - docConfig.getDocumentId())); - return; - } + // Skip this event since we generated it. + logger.info(String.format( + Locale.US, + "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote change event was " + + "generated by us; dropping the event", + logicalT, + nsConfig.getNamespace(), + docConfig.getDocumentId())); + return; + } } if (docConfig.getLastUncommittedChangeEvent() == null) { @@ -932,8 +933,8 @@ private void syncLocalToRemote() { docConfig, remoteChangeEvent); } else { - // TODO(ERIC->ADAM): This event may contain old version info. We should be filtering out the version - // anyway from local and remote events. + // TODO(ERIC->ADAM): This event may contain old version info. We should be filtering out + // the version anyway from local and remote events. final ChangeEvent committedEvent = docConfig.getLastUncommittedChangeEvent(); emitEvent(docConfig.getDocumentId(), new ChangeEvent<>( @@ -1762,6 +1763,6 @@ private static BsonDocument withNextVersionCounter( final BsonDocument document, final DocumentVersionInfo versionInfo ) { - return withNewVersion(document, DocumentVersionInfo.getNextVersion(versionInfo)); + return withNewVersion(document, versionInfo.getNextVersion()); } } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index edfa311ee..0203d1a05 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -20,6 +20,7 @@ import com.mongodb.stitch.core.internal.common.BsonUtils; +import java.util.UUID; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.bson.BsonBoolean; @@ -29,12 +30,11 @@ import org.bson.BsonString; import org.bson.BsonValue; -import java.util.UUID; final class DocumentVersionInfo { @Nullable private final VersionValues versionValues; @Nullable private final BsonDocument versionDoc; - @Nullable private final BsonDocument filter; + @Nullable private final BsonDocument filter; private class VersionValues { final int syncProtocolVersion; @@ -49,7 +49,6 @@ private class VersionValues { this.instanceId = instanceId; this.versionCounter = versionCounter; } - } private static final String SYNC_PROTOCOL_VERSION_FIELD = "spv"; @@ -83,7 +82,13 @@ private DocumentVersionInfo( return versionDoc; } - boolean isNonEmptyVersion() { return versionValues != null; } + /** + * Returns whether this version is non-empty (i.e. a version from a document with no version) + * @return true if the version is non-empty, false if the version is empty. + */ + boolean isNonEmptyVersion() { + return versionValues != null; + } private @Nonnull VersionValues getVersionValuesThrowIfNull() { if (this.versionValues == null) { @@ -95,22 +100,45 @@ private DocumentVersionInfo( return this.versionValues; } + /** + * Returns the synchronization protocol version of this version. + * @return an int representing the synchronization protocol version of this version. + */ int getSyncProtocolVersion() { return getVersionValuesThrowIfNull().syncProtocolVersion; } + /** + * Returns the GUID instance id of this version. + * @return a String representing the instance id of this version. + */ String getInstanceId() { return getVersionValuesThrowIfNull().instanceId; } + /** + * Returns the version counter of this version. + * @return a long representing the version counter of this version. + */ long getVersionCounter() { return getVersionValuesThrowIfNull().versionCounter; } + /** + * Gets a filter that will only return the document in a query if it matches the current version. + * Will return null if a document ID was not specified when the version info was constructed. + * + * @return a BsonDocument representing the filter to request a document at this version + */ @Nullable BsonDocument getFilter() { return filter; } + /** + * Returns the current version info for a locally synchronized document. + * @param docConfig the CoreDocumentSynchronizationConfig to get the version info from. + * @return a DocumentVersionInfo + */ static DocumentVersionInfo getLocalVersionInfo( final CoreDocumentSynchronizationConfig docConfig ) { @@ -120,17 +148,34 @@ static DocumentVersionInfo getLocalVersionInfo( ); } + /** + * Returns the current version info for a provided remote document. + * @param remoteDocument the remote BSON document from which to extract version info + * @return a DocumentVersionInfo + */ static DocumentVersionInfo getRemoteVersionInfo(final BsonDocument remoteDocument) { final BsonDocument version = getDocumentVersionDoc(remoteDocument); return new DocumentVersionInfo(version, BsonUtils.getDocumentId(remoteDocument)); } + /** + * Returns a DocumentVersionInfo constructed from a raw version document. The returned + * DocumentVersionInfo will have no document ID specified, so it will always return a null + * filter if the filter is requested. + * @param versionDoc the raw version document from which to extract version info + * @return a DocumentVersionInfo + */ static DocumentVersionInfo fromVersionDoc(final BsonDocument versionDoc) { return new DocumentVersionInfo(versionDoc, null); } + /** + * Returns a BSON version document representing a new version with a new instance ID, and + * version counter of zero. + * @return a BsonDocument representing a synchronization version + */ static BsonDocument getFreshVersionDocument() { - BsonDocument versionDoc = new BsonDocument(); + final BsonDocument versionDoc = new BsonDocument(); versionDoc.append(SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(1)); versionDoc.append(INSTANCE_ID_FIELD, new BsonString(UUID.randomUUID().toString())); @@ -174,16 +219,20 @@ static BsonDocument getVersionedFilter( return filter; } - static BsonDocument getNextVersion( - final DocumentVersionInfo versionInfo - ) { - if (versionInfo.getVersionDoc() == null) { + /** + * Given a DocumentVersionInfo, returns a BSON document representing the next version. This means + * and incremented version count for a non-empty version, or a fresh version document for an + * empty version. + * @return a BsonDocument representing a synchronization version + */ + BsonDocument getNextVersion() { + if (this.getVersionDoc() == null) { return getFreshVersionDocument(); } - final BsonDocument nextVersion = BsonUtils.copyOfDocument(versionInfo.getVersionDoc()); + final BsonDocument nextVersion = BsonUtils.copyOfDocument(this.getVersionDoc()); nextVersion.put( VERSION_COUNTER_FIELD, - new BsonInt64(versionInfo.getVersionCounter() + 1)); + new BsonInt64(this.getVersionCounter() + 1)); return nextVersion; } } From 33213d408b3b6f7aa5c5118b06230b5af93097dc Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Thu, 11 Oct 2018 18:14:18 -0400 Subject: [PATCH 06/10] self review and fix lint --- .../internal/SyncMongoClientIntTests.kt | 37 ++++++++--------- .../sync/internal/DataSynchronizer.java | 41 ++++++++++--------- .../internal/SyncMongoClientIntTests.kt | 14 ++++--- 3 files changed, 48 insertions(+), 44 deletions(-) diff --git a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 61f9142ed..bf6f99a58 100644 --- a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -16,15 +16,19 @@ import com.mongodb.stitch.core.internal.common.OperationResult import com.mongodb.stitch.core.services.mongodb.remote.sync.ConflictHandler import com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent import com.mongodb.stitch.core.services.mongodb.remote.sync.DefaultSyncConflictResolvers -import com.mongodb.stitch.core.services.mongodb.remote.sync.ErrorListener import org.bson.BsonDocument import org.bson.BsonObjectId import org.bson.BsonValue import org.bson.Document import org.bson.types.ObjectId import org.junit.After -import org.junit.Assert -import org.junit.Assert.* +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail import org.junit.Assume import org.junit.Before import org.junit.Test @@ -577,7 +581,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { coll.syncOne(doc1Id) streamAndSync() assertEquals(doc, Tasks.await(coll.findOneById(doc1Id))) - Assert.assertNotNull(Tasks.await(coll.findOneById(doc1Id))) + assertNotNull(Tasks.await(coll.findOneById(doc1Id))) goOffline() Tasks.await(remoteColl.deleteOne(doc1Filter)) @@ -586,11 +590,11 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { goOnline() streamAndSync() assertNull(Tasks.await(remoteColl.find(doc1Filter).first())) - Assert.assertNotNull(Tasks.await(coll.findOneById(doc1Id))) + assertNotNull(Tasks.await(coll.findOneById(doc1Id))) streamAndSync() - Assert.assertNotNull(Tasks.await(remoteColl.find(doc1Filter).first())) - Assert.assertNotNull(Tasks.await(coll.findOneById(doc1Id))) + assertNotNull(Tasks.await(remoteColl.find(doc1Filter).first())) + assertNotNull(Tasks.await(coll.findOneById(doc1Id))) } } @@ -616,17 +620,13 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() assertEquals(doc, Tasks.await(coll.findOneById(doc1Id))) - Assert.assertNotNull(Tasks.await(coll.findOneById(doc1Id))) + assertNotNull(Tasks.await(coll.findOneById(doc1Id))) val wait = watchForEvents(this.namespace, 2) Tasks.await(remoteColl.deleteOne(doc1Filter)) Tasks.await(remoteColl.insertOne(withNewSyncVersion(doc))) wait.acquire() - // TODO(QUESTION FOR REVIEWER): this test seems funky to me. We do the udpate, but - // the change is not reflected in any of the following finds. Why is that? Is the remote - // update with the new version ID supposed to make it so that this update is lost? - // it would be nice if we could have some comments in these tests. assertEquals(1, Tasks.await(coll.updateOneById(doc1Id, Document("\$inc", Document("foo", 1)))).matchedCount) streamAndSync() @@ -691,7 +691,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { coll.syncOne(doc1Id) streamAndSync() assertEquals(doc, Tasks.await(coll.findOneById(doc1Id))) - Assert.assertNotNull(Tasks.await(coll.findOneById(doc1Id))) + assertNotNull(Tasks.await(coll.findOneById(doc1Id))) val sem = watchForEvents(this.namespace) assertEquals(1, Tasks.await(remoteColl.updateOne(doc1Filter, withNewSyncVersionSet(Document("\$inc", Document("foo", 1))))).matchedCount) @@ -843,7 +843,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { // do a sync pass, synchronizing the doc streamAndSync() - Assert.assertNotNull(Tasks.await(remoteColl.find(Document("_id", testDoc.get("_id"))).first())) + assertNotNull(Tasks.await(remoteColl.find(Document("_id", testDoc.get("_id"))).first())) // update the doc val expectedDoc = Document("hello", "computer") @@ -857,7 +857,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { // do a sync pass, and throw an error during the conflict resolver // freezing the document streamAndSync() - Assert.assertTrue(errorEmitted) + assertTrue(errorEmitted) // update the doc remotely val nextDoc = Document("hello", "friend") @@ -932,8 +932,8 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() - Assert.assertTrue(hasConflictHandlerBeenInvoked) - Assert.assertTrue(hasChangeEventListenerBeenInvoked) + assertTrue(hasConflictHandlerBeenInvoked) + assertTrue(hasChangeEventListenerBeenInvoked) } @Test @@ -1037,11 +1037,10 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val docToInsert = withNewUnsupportedSyncVersion(Document("hello", "world")) coll.configure(failingConflictHandler, null, null) - val insertResult = Tasks.await(remoteColl.insertOne(docToInsert)) + Tasks.await(remoteColl.insertOne(docToInsert)) val doc = Tasks.await(remoteColl.find(docToInsert).first())!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) - val doc1Filter = Document("_id", doc1Id) coll.syncOne(doc1Id) diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index eea94c3b8..793ab69fe 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -20,7 +20,6 @@ import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalInsert; import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalReplace; import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent.changeEventForLocalUpdate; -import static com.mongodb.stitch.core.services.mongodb.remote.sync.internal.DocumentVersionInfo.getDocumentVersionDoc; import com.mongodb.MongoClientSettings; import com.mongodb.MongoNamespace; @@ -534,8 +533,8 @@ private void syncRemoteChangeEventToLocal( final DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig); - if (remoteVersionInfo.isNonEmptyVersion() && - remoteVersionInfo.getSyncProtocolVersion() != 1) { + if (remoteVersionInfo.isNonEmptyVersion() + && remoteVersionInfo.getSyncProtocolVersion() != 1) { desyncDocumentFromRemote(nsConfig.getNamespace(), docConfig.getDocumentId()); emitError(docConfig, @@ -591,7 +590,7 @@ private void syncRemoteChangeEventToLocal( nsConfig.getNamespace(), docConfig.getDocumentId(), remoteChangeEvent.getFullDocument(), - getDocumentVersionDoc(remoteChangeEvent.getFullDocument())); + DocumentVersionInfo.getDocumentVersionDoc(remoteChangeEvent.getFullDocument())); return; case DELETE: logger.info(String.format( @@ -748,8 +747,8 @@ private void syncLocalToRemote() { ); continue; } - final BsonDocument nextDoc = withNextVersionCounter(localDoc, localVersionInfo); - nextVersion = getDocumentVersionDoc(nextDoc); + final BsonDocument nextDoc = withNextVersion(localDoc, localVersionInfo); + nextVersion = DocumentVersionInfo.getDocumentVersionDoc(nextDoc); final RemoteUpdateResult result; try { @@ -794,7 +793,7 @@ private void syncLocalToRemote() { ); continue; } - final BsonDocument nextDoc = withNextVersionCounter(localDoc, localVersionInfo); + final BsonDocument nextDoc = withNextVersion(localDoc, localVersionInfo); nextVersion = DocumentVersionInfo.getDocumentVersionDoc(nextDoc); final BsonDocument translatedUpdate = new BsonDocument(); @@ -1391,7 +1390,7 @@ private void replaceOrUpsertOneFromResolution( return; } - final BsonDocument docToReplace = withNextVersionCounter( + final BsonDocument docToReplace = withNextVersion( document, DocumentVersionInfo.getLocalVersionInfo(config) ); @@ -1403,8 +1402,8 @@ private void replaceOrUpsertOneFromResolution( new FindOneAndReplaceOptions().upsert(true).returnDocument(ReturnDocument.AFTER)); final ChangeEvent event; - // TODO(QUESTION FOR REVIEW): it seems like we were previously setting pending writes at the - // old version, rather than the new version. Was this intentional? It seems like we should be + // TODO(QUESTION FOR REVIEW): it seems like we are setting pending writes at the + // old version, rather than the new version. Is this intentional? It seems like we should be // setting the pending writes at the newly generated version. I'm confused though because if I // use the new version here instead, tests fail because updates don't end up happening on a // sync pass. @@ -1445,18 +1444,11 @@ private void replaceOrUpsertOneFromRemote( return; } - // TODO(QUESTION FOR REVIEW): Unlike replaceOrUpsertOneFromResolution, the original code - // here did not replace the version of the document being used to replace the document at - // documentId. Was this intentional? I assume that because we're updating something locally - // with a remote version, we want to keep the remote version, but then why are we - // setting a pending write here? - getLocalCollection(namespace) .findOneAndReplace( getDocumentIdFilter(documentId), document, new FindOneAndReplaceOptions().upsert(true)); - config.setPendingWritesComplete(atVersion); emitEvent(documentId, changeEventForLocalReplace(namespace, documentId, document, false)); } @@ -1745,10 +1737,11 @@ private static BsonDocument getDocumentIdFilter(final BsonValue documentId) { } /** - * Adds and returns a document with a new version id to the given document. + * Adds and returns a document with a new version to the given document. * * @param document the document to attach a new version to. - * @return a document with a new version id to the given document. + * @param newVersion the version to attach to the document + * @return a document with a new version to the given document. */ private static BsonDocument withNewVersion( final BsonDocument document, @@ -1759,7 +1752,15 @@ private static BsonDocument withNewVersion( return newDocument; } - private static BsonDocument withNextVersionCounter( + /** + * Returns a document with a new version added to the given document. The added version is + * the provided version with a version counter incremented by one, or a fresh version if the + * provided version is empty. + * @param document the document to attach the next version to. + * @param versionInfo the version from which the next version will be derived + * @return a document with the newly updated version added to the given document. + */ + private static BsonDocument withNextVersion( final BsonDocument document, final DocumentVersionInfo versionInfo ) { diff --git a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index 89b398a55..e89dbb990 100644 --- a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -20,7 +20,13 @@ import org.bson.BsonValue import org.bson.Document import org.bson.types.ObjectId import org.junit.After -import org.junit.Assert.* +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail import org.junit.Assume import org.junit.Before import org.junit.Test @@ -1000,7 +1006,7 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { assertEquals(expectedDocument, withoutSyncVersion(thirdRemoteDocBeforeSyncPass)) expectedDocument.remove("foo") - assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id))!!) + assertEquals(expectedDocument, withoutSyncVersion(coll.findOneById(doc1Id))) // the remote document after a local delete and local insert, and after a sync pass, // should have the same instance ID as before and a version count, since the change @@ -1040,12 +1046,10 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val docToInsert = withNewUnsupportedSyncVersion(Document("hello", "world")) coll.configure(failingConflictHandler, null, null) - val insertResult = remoteColl.insertOne(docToInsert) + remoteColl.insertOne(docToInsert) val doc = remoteColl.find(docToInsert).first()!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) - val doc1Filter = Document("_id", doc1Id) - coll.syncOne(doc1Id) assertTrue(coll.syncedIds.contains(doc1Id)) From 7bc974490b3a17ff8f414c083497d2b3c3e71876 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Fri, 12 Oct 2018 10:42:29 -0400 Subject: [PATCH 07/10] appease linter --- .../mongodb/remote/sync/internal/DataSynchronizer.java | 5 ++--- .../mongodb/remote/sync/internal/DocumentVersionInfo.java | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index 793ab69fe..253513197 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -540,14 +540,13 @@ private void syncRemoteChangeEventToLocal( emitError(docConfig, String.format( Locale.US, - "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s got a remote" + "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s got a remote " + "document with an unsupported synchronization protocol version " + "%d; dropping the event, and desyncing the document", logicalT, nsConfig.getNamespace(), docConfig.getDocumentId(), - remoteVersionInfo.getSyncProtocolVersion(), - remoteChangeEvent.getOperationType().toString())); + remoteVersionInfo.getSyncProtocolVersion())); return; } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index 0203d1a05..6f942093c 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -36,7 +36,7 @@ final class DocumentVersionInfo { @Nullable private final BsonDocument versionDoc; @Nullable private final BsonDocument filter; - private class VersionValues { + private static class VersionValues { final int syncProtocolVersion; final String instanceId; final long versionCounter; From 677de20693ca359c2fac6318cf01114478a20b65 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Fri, 12 Oct 2018 14:03:46 -0400 Subject: [PATCH 08/10] address PR comments --- .../internal/SyncMongoClientIntTests.kt | 15 ++- .../CoreDocumentSynchronizationConfig.java | 18 ++-- .../sync/internal/DataSynchronizer.java | 39 +++---- .../sync/internal/DocumentVersionInfo.java | 102 ++++++++++-------- .../internal/SyncMongoClientIntTests.kt | 12 ++- 5 files changed, 99 insertions(+), 87 deletions(-) diff --git a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index bf6f99a58..c6af7800b 100644 --- a/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/android/services/mongodb-remote/src/androidTest/java/com/mongodb/stitch/android/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -16,6 +16,7 @@ import com.mongodb.stitch.core.internal.common.OperationResult import com.mongodb.stitch.core.services.mongodb.remote.sync.ConflictHandler import com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent import com.mongodb.stitch.core.services.mongodb.remote.sync.DefaultSyncConflictResolvers +import com.mongodb.stitch.core.services.mongodb.remote.sync.ErrorListener import org.bson.BsonDocument import org.bson.BsonObjectId import org.bson.BsonValue @@ -35,6 +36,7 @@ import org.junit.Test import java.lang.Exception import java.util.UUID import java.util.concurrent.Semaphore +import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { @@ -986,7 +988,7 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { assertEquals(instanceIdOf(firstRemoteDoc), instanceIdOf(secondRemoteDoc)) assertEquals(1, versionCounterOf(secondRemoteDoc)) - assertEquals(expectedDocument, Tasks.await(coll.findOneById(doc1Id))!!) + assertEquals(expectedDocument, withoutSyncVersion(Tasks.await(coll.findOneById(doc1Id))!!)) // the remote document after a local delete and local insert, but before a sync pass, // should have the same version as the previous document @@ -1036,12 +1038,16 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { val docToInsert = withNewUnsupportedSyncVersion(Document("hello", "world")) - coll.configure(failingConflictHandler, null, null) + val errorEmittedSem = Semaphore(0) + coll.configure( + failingConflictHandler, + null, + ErrorListener { documentId, error -> errorEmittedSem.release() }) + Tasks.await(remoteColl.insertOne(docToInsert)) val doc = Tasks.await(remoteColl.find(docToInsert).first())!! val doc1Id = BsonObjectId(doc.getObjectId("_id")) - coll.syncOne(doc1Id) assertTrue(coll.syncedIds.contains(doc1Id)) @@ -1051,6 +1057,9 @@ class SyncMongoClientIntTests : BaseStitchAndroidIntTest() { streamAndSync() assertFalse(coll.syncedIds.contains(doc1Id)) + + // an error should also have been emitted + assertTrue(errorEmittedSem.tryAcquire(10, TimeUnit.SECONDS)) } private fun streamAndSync() { diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java index 00f6d9f58..06c06a2ad 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java @@ -212,9 +212,6 @@ void setSomePendingWrites( * * @param atTime the time at which the write occurred. * @param atVersion the version for which the write occurred. - * // TODO(QUESTION FOR REVIEW): so per my other comments, this means the - * // version before the write occured, right? - * * @param changeEvent the description of the write/change. */ void setSomePendingWrites( @@ -347,16 +344,19 @@ public BsonDocument getLastKnownRemoteVersion() { } } - public boolean hasCommittedVersion(final DocumentVersionInfo version) { + public boolean hasCommittedVersion(final DocumentVersionInfo versionInfo) { docLock.readLock().lock(); try { - final DocumentVersionInfo remoteVersionInfo = + final DocumentVersionInfo localVersionInfo = DocumentVersionInfo.fromVersionDoc(lastKnownRemoteVersion); - return ((version.isNonEmptyVersion() && remoteVersionInfo.isNonEmptyVersion() - && (version.getSyncProtocolVersion() == remoteVersionInfo.getSyncProtocolVersion()) - && (version.getInstanceId().equals(remoteVersionInfo.getInstanceId())) - && (version.getVersionCounter() >= remoteVersionInfo.getVersionCounter()))); + return ((versionInfo.hasVersion() && localVersionInfo.hasVersion() + && (versionInfo.getVersion().getSyncProtocolVersion() + == localVersionInfo.getVersion().getSyncProtocolVersion()) + && (versionInfo.getVersion().getInstanceId() + .equals(localVersionInfo.getVersion().getInstanceId())) + && (versionInfo.getVersion().getVersionCounter() + <= localVersionInfo.getVersion().getVersionCounter()))); } finally { docLock.readLock().unlock(); } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index 253513197..ae626bfa2 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -523,18 +523,16 @@ private void syncRemoteChangeEventToLocal( final BsonDocument docFilter = getDocumentIdFilter(docConfig.getDocumentId()); final BsonDocument localDocument = localColl.find(docFilter).first(); - final BsonDocument lastKnownRemoteVersion; + final BsonDocument currentRemoteVersion; if (remoteDoc == null || remoteDoc.size() == 0) { - lastKnownRemoteVersion = null; + currentRemoteVersion = null; } else { - final DocumentVersionInfo remoteVersionInfo = DocumentVersionInfo + final DocumentVersionInfo currentRemoteVersionInfo = DocumentVersionInfo .getRemoteVersionInfo(remoteDoc); - lastKnownRemoteVersion = remoteVersionInfo.getVersionDoc(); - - final DocumentVersionInfo localVersion = DocumentVersionInfo.getLocalVersionInfo(docConfig); + currentRemoteVersion = currentRemoteVersionInfo.getVersionDoc(); - if (remoteVersionInfo.isNonEmptyVersion() - && remoteVersionInfo.getSyncProtocolVersion() != 1) { + if (currentRemoteVersionInfo.hasVersion() + && currentRemoteVersionInfo.getVersion().getSyncProtocolVersion() != 1) { desyncDocumentFromRemote(nsConfig.getNamespace(), docConfig.getDocumentId()); emitError(docConfig, @@ -546,20 +544,12 @@ private void syncRemoteChangeEventToLocal( logicalT, nsConfig.getNamespace(), docConfig.getDocumentId(), - remoteVersionInfo.getSyncProtocolVersion())); + currentRemoteVersionInfo.getVersion().getSyncProtocolVersion())); return; } - // TODO(QUESTION FOR REVIEW): The testFrozenDocumentConfig test fails if I call this, but - // other tests pass. Shouldn't the receipt of this document indicate that this is the last - // known remote version of this document? - // docConfig.setLastKnownRemoteVersion(lastKnownRemoteVersion); - - // TODO(QFR): I ask because when we are looking at hasCommittedVersion, we are using the - // lastKnownRemoteVersion of the doc config rather than the last known remote version based - // on the incoming change event we got here, which would make this not conform to spec. - if (docConfig.hasCommittedVersion(localVersion)) { + if (docConfig.hasCommittedVersion(currentRemoteVersionInfo)) { // Skip this event since we generated it. logger.info(String.format( Locale.US, @@ -630,7 +620,7 @@ private void syncRemoteChangeEventToLocal( // There is a pending write that must skip R2L if both versions are null. The absence of a // version is effectively a version. The pending write, if it's not a delete, should be // setting a new version anyway. - if (lastKnownLocalVersion == null && lastKnownRemoteVersion == null) { + if (lastKnownLocalVersion == null && currentRemoteVersion == null) { logger.info(String.format( Locale.US, "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote and local have same " @@ -931,7 +921,7 @@ private void syncLocalToRemote() { docConfig, remoteChangeEvent); } else { - // TODO(ERIC->ADAM): This event may contain old version info. We should be filtering out + // TODO(STITCH-1972): This event may contain old version info. We should be filtering out // the version anyway from local and remote events. final ChangeEvent committedEvent = docConfig.getLastUncommittedChangeEvent(); @@ -1401,22 +1391,17 @@ private void replaceOrUpsertOneFromResolution( new FindOneAndReplaceOptions().upsert(true).returnDocument(ReturnDocument.AFTER)); final ChangeEvent event; - // TODO(QUESTION FOR REVIEW): it seems like we are setting pending writes at the - // old version, rather than the new version. Is this intentional? It seems like we should be - // setting the pending writes at the newly generated version. I'm confused though because if I - // use the new version here instead, tests fail because updates don't end up happening on a - // sync pass. if (fromDelete) { event = changeEventForLocalInsert(namespace, result, true); config.setSomePendingWrites( logicalT, - atVersion, //TODO: why don't we want newVersion?, + atVersion, event); } else { event = changeEventForLocalReplace(namespace, documentId, result, true); config.setSomePendingWrites( logicalT, - atVersion, //TODO: why don't we want newVersion? + atVersion, event); } emitEvent(documentId, event); diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index 6f942093c..a99f1001e 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -32,16 +32,22 @@ final class DocumentVersionInfo { - @Nullable private final VersionValues versionValues; + @Nullable private final Version version; @Nullable private final BsonDocument versionDoc; @Nullable private final BsonDocument filter; - private static class VersionValues { + private static class Fields { + private static final String SYNC_PROTOCOL_VERSION_FIELD = "spv"; + private static final String INSTANCE_ID_FIELD = "id"; + private static final String VERSION_COUNTER_FIELD = "v"; + } + + static class Version { final int syncProtocolVersion; final String instanceId; final long versionCounter; - VersionValues( + Version( final int syncProtocolVersion, final String instanceId, final long versionCounter) { @@ -49,11 +55,31 @@ private static class VersionValues { this.instanceId = instanceId; this.versionCounter = versionCounter; } - } - private static final String SYNC_PROTOCOL_VERSION_FIELD = "spv"; - private static final String INSTANCE_ID_FIELD = "id"; - private static final String VERSION_COUNTER_FIELD = "v"; + /** + * Returns the synchronization protocol version of this version. + * @return an int representing the synchronization protocol version of this version. + */ + int getSyncProtocolVersion() { + return syncProtocolVersion; + } + + /** + * Returns the GUID instance id of this version. + * @return a String representing the instance id of this version. + */ + String getInstanceId() { + return instanceId; + } + + /** + * Returns the version counter of this version. + * @return a long representing the version counter of this version. + */ + long getVersionCounter() { + return versionCounter; + } + } private DocumentVersionInfo( @Nullable final BsonDocument version, @@ -61,14 +87,14 @@ private DocumentVersionInfo( ) { if (version != null) { this.versionDoc = version; - this.versionValues = new VersionValues( - versionDoc.getInt32(SYNC_PROTOCOL_VERSION_FIELD).getValue(), - versionDoc.getString(INSTANCE_ID_FIELD).getValue(), - versionDoc.getInt64(VERSION_COUNTER_FIELD).getValue() + this.version = new Version( + versionDoc.getInt32(Fields.SYNC_PROTOCOL_VERSION_FIELD).getValue(), + versionDoc.getString(Fields.INSTANCE_ID_FIELD).getValue(), + versionDoc.getInt64(Fields.VERSION_COUNTER_FIELD).getValue() ); } else { this.versionDoc = null; - this.versionValues = null; + this.version = null; } if (documentId != null) { @@ -86,42 +112,24 @@ private DocumentVersionInfo( * Returns whether this version is non-empty (i.e. a version from a document with no version) * @return true if the version is non-empty, false if the version is empty. */ - boolean isNonEmptyVersion() { - return versionValues != null; + boolean hasVersion() { + return version != null; } - private @Nonnull VersionValues getVersionValuesThrowIfNull() { - if (this.versionValues == null) { + /** + * Returns the concrete version values of this version info object. Will throw an + * IllegalStateException if the version info has no version (i.e if hasVersion would return + * false) + * @return a Version representing the concrete version values of this info object. + */ + @Nonnull Version getVersion() { + if (this.version == null) { throw new IllegalStateException( "Attempting to access sync version information on a document with no version." ); } - return this.versionValues; - } - - /** - * Returns the synchronization protocol version of this version. - * @return an int representing the synchronization protocol version of this version. - */ - int getSyncProtocolVersion() { - return getVersionValuesThrowIfNull().syncProtocolVersion; - } - - /** - * Returns the GUID instance id of this version. - * @return a String representing the instance id of this version. - */ - String getInstanceId() { - return getVersionValuesThrowIfNull().instanceId; - } - - /** - * Returns the version counter of this version. - * @return a long representing the version counter of this version. - */ - long getVersionCounter() { - return getVersionValuesThrowIfNull().versionCounter; + return this.version; } /** @@ -177,9 +185,9 @@ static DocumentVersionInfo fromVersionDoc(final BsonDocument versionDoc) { static BsonDocument getFreshVersionDocument() { final BsonDocument versionDoc = new BsonDocument(); - versionDoc.append(SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(1)); - versionDoc.append(INSTANCE_ID_FIELD, new BsonString(UUID.randomUUID().toString())); - versionDoc.append(VERSION_COUNTER_FIELD, new BsonInt64(0L)); + versionDoc.append(Fields.SYNC_PROTOCOL_VERSION_FIELD, new BsonInt32(1)); + versionDoc.append(Fields.INSTANCE_ID_FIELD, new BsonString(UUID.randomUUID().toString())); + versionDoc.append(Fields.VERSION_COUNTER_FIELD, new BsonInt64(0L)); return versionDoc; } @@ -226,13 +234,13 @@ static BsonDocument getVersionedFilter( * @return a BsonDocument representing a synchronization version */ BsonDocument getNextVersion() { - if (this.getVersionDoc() == null) { + if (!this.hasVersion() || this.getVersionDoc() == null) { return getFreshVersionDocument(); } final BsonDocument nextVersion = BsonUtils.copyOfDocument(this.getVersionDoc()); nextVersion.put( - VERSION_COUNTER_FIELD, - new BsonInt64(this.getVersionCounter() + 1)); + Fields.VERSION_COUNTER_FIELD, + new BsonInt64(this.getVersion().getVersionCounter() + 1)); return nextVersion; } } diff --git a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt index e89dbb990..f1700ef55 100644 --- a/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt +++ b/server/services/mongodb-remote/src/test/java/com/mongodb/stitch/server/services/mongodb/remote/internal/SyncMongoClientIntTests.kt @@ -10,6 +10,7 @@ import com.mongodb.stitch.core.internal.common.Callback import com.mongodb.stitch.core.internal.common.OperationResult import com.mongodb.stitch.core.services.mongodb.remote.sync.ConflictHandler import com.mongodb.stitch.core.services.mongodb.remote.sync.DefaultSyncConflictResolvers +import com.mongodb.stitch.core.services.mongodb.remote.sync.ErrorListener import com.mongodb.stitch.core.services.mongodb.remote.sync.internal.ChangeEvent import com.mongodb.stitch.server.services.mongodb.remote.RemoteMongoClient import com.mongodb.stitch.server.services.mongodb.remote.RemoteMongoCollection @@ -33,6 +34,7 @@ import org.junit.Test import java.lang.Exception import java.util.UUID import java.util.concurrent.Semaphore +import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger class SyncMongoClientIntTests : BaseStitchServerIntTest() { @@ -1045,7 +1047,12 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { val docToInsert = withNewUnsupportedSyncVersion(Document("hello", "world")) - coll.configure(failingConflictHandler, null, null) + val errorEmittedSem = Semaphore(0) + coll.configure( + failingConflictHandler, + null, + ErrorListener { documentId, error -> errorEmittedSem.release() }) + remoteColl.insertOne(docToInsert) val doc = remoteColl.find(docToInsert).first()!! @@ -1059,6 +1066,9 @@ class SyncMongoClientIntTests : BaseStitchServerIntTest() { streamAndSync() assertFalse(coll.syncedIds.contains(doc1Id)) + + // an error should also have been emitted + assertTrue(errorEmittedSem.tryAcquire(10, TimeUnit.SECONDS)) } private fun watchForEvents( From be063aa6d28673fc937780270959d5038ffa26f5 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Fri, 12 Oct 2018 15:31:18 -0400 Subject: [PATCH 09/10] self review cleanup --- .../core/internal/common/BsonUtils.java | 7 + .../CoreDocumentSynchronizationConfig.java | 14 -- .../sync/internal/DataSynchronizer.java | 120 ++++++------------ .../sync/internal/DocumentVersionInfo.java | 6 +- 4 files changed, 53 insertions(+), 94 deletions(-) diff --git a/core/sdk/src/main/java/com/mongodb/stitch/core/internal/common/BsonUtils.java b/core/sdk/src/main/java/com/mongodb/stitch/core/internal/common/BsonUtils.java index 1f0d4f928..26091cedf 100644 --- a/core/sdk/src/main/java/com/mongodb/stitch/core/internal/common/BsonUtils.java +++ b/core/sdk/src/main/java/com/mongodb/stitch/core/internal/common/BsonUtils.java @@ -142,6 +142,13 @@ public static BsonValue getDocumentId(final BsonDocument document) { return document.get("_id"); } + public static BsonValue getDocumentId( + final BsonDocument document, + final BsonValue defaultValue + ) { + return document.get("_id", null); + } + /** * Returns a copy of the given document. * @param document the document to copy. diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java index 06c06a2ad..698cc5870 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/CoreDocumentSynchronizationConfig.java @@ -247,19 +247,6 @@ void setPendingWritesComplete(final BsonDocument atVersion) { } } - void setLastKnownRemoteVersion(final BsonDocument lastKnownRemoteVersion) { - docLock.writeLock().lock(); - try { - this.lastKnownRemoteVersion = lastKnownRemoteVersion; - - docsColl.replaceOne( - getDocFilter(namespace, documentId), - this); - } finally { - docLock.writeLock().unlock(); - } - } - // Equality on documentId @Override public boolean equals(final Object object) { @@ -443,7 +430,6 @@ BsonDocument toBsonDocument() { // TODO: This may put the doc above the 16MiB but ignore for now. asDoc.put(ConfigCodec.Fields.LAST_UNCOMMITTED_CHANGE_EVENT, encoded); } - asDoc.put(ConfigCodec.Fields.IS_STALE, new BsonBoolean(isStale)); asDoc.put(ConfigCodec.Fields.IS_FROZEN, new BsonBoolean(isFrozen)); return asDoc; diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java index ae626bfa2..b70084991 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DataSynchronizer.java @@ -400,7 +400,6 @@ private void syncRemoteToLocal() { logicalT)); for (final NamespaceSynchronizationConfig nsConfig : syncConfig) { - final MongoCollection localColl = getLocalCollection(nsConfig.getNamespace()); final Map> remoteChangeEvents = instanceChangeStreamListener.getEventsForNamespace(nsConfig.getNamespace()); @@ -431,7 +430,7 @@ private void syncRemoteToLocal() { unseenIds.remove(docConfig.getDocumentId()); latestDocumentIds.remove(docConfig.getDocumentId()); - syncRemoteChangeEventToLocal(nsConfig, docConfig, eventEntry.getValue(), localColl); + syncRemoteChangeEventToLocal(nsConfig, docConfig, eventEntry.getValue()); } for (final BsonValue docId : unseenIds) { @@ -451,8 +450,7 @@ private void syncRemoteToLocal() { docId, latestDocumentMap.get(docId), false - ), - localColl); + )); docConfig.setStale(false); } @@ -476,7 +474,7 @@ private void syncRemoteToLocal() { nsConfig.getNamespace(), unseenId, docConfig.hasUncommittedWrites() - ), localColl); + )); } } } @@ -493,13 +491,11 @@ private void syncRemoteToLocal() { * @param nsConfig the namespace configuration. * @param docConfig the document configuration related to the event. * @param remoteChangeEvent the remote change event to synchronize into the local database. - * @param localColl the local collection where the document lives. */ private void syncRemoteChangeEventToLocal( final NamespaceSynchronizationConfig nsConfig, final CoreDocumentSynchronizationConfig docConfig, - final ChangeEvent remoteChangeEvent, - final MongoCollection localColl + final ChangeEvent remoteChangeEvent ) { if (docConfig.hasUncommittedWrites() && docConfig.getLastResolution() == logicalT) { logger.info(String.format( @@ -520,48 +516,40 @@ private void syncRemoteChangeEventToLocal( remoteChangeEvent.getOperationType().toString())); final BsonDocument remoteDoc = remoteChangeEvent.getFullDocument(); - final BsonDocument docFilter = getDocumentIdFilter(docConfig.getDocumentId()); - final BsonDocument localDocument = localColl.find(docFilter).first(); + final DocumentVersionInfo currentRemoteVersionInfo = DocumentVersionInfo + .getRemoteVersionInfo(remoteDoc); - final BsonDocument currentRemoteVersion; - if (remoteDoc == null || remoteDoc.size() == 0) { - currentRemoteVersion = null; - } else { - final DocumentVersionInfo currentRemoteVersionInfo = DocumentVersionInfo - .getRemoteVersionInfo(remoteDoc); - currentRemoteVersion = currentRemoteVersionInfo.getVersionDoc(); - - if (currentRemoteVersionInfo.hasVersion() - && currentRemoteVersionInfo.getVersion().getSyncProtocolVersion() != 1) { - desyncDocumentFromRemote(nsConfig.getNamespace(), docConfig.getDocumentId()); + if (currentRemoteVersionInfo.hasVersion() + && currentRemoteVersionInfo.getVersion().getSyncProtocolVersion() != 1) { + desyncDocumentFromRemote(nsConfig.getNamespace(), docConfig.getDocumentId()); - emitError(docConfig, - String.format( - Locale.US, - "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s got a remote " - + "document with an unsupported synchronization protocol version " - + "%d; dropping the event, and desyncing the document", - logicalT, - nsConfig.getNamespace(), - docConfig.getDocumentId(), - currentRemoteVersionInfo.getVersion().getSyncProtocolVersion())); + emitError(docConfig, + String.format( + Locale.US, + "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s got a remote " + + "document with an unsupported synchronization protocol version " + + "%d; dropping the event, and desyncing the document", + logicalT, + nsConfig.getNamespace(), + docConfig.getDocumentId(), + currentRemoteVersionInfo.getVersion().getSyncProtocolVersion())); - return; - } + return; + } - if (docConfig.hasCommittedVersion(currentRemoteVersionInfo)) { - // Skip this event since we generated it. - logger.info(String.format( - Locale.US, - "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote change event was " - + "generated by us; dropping the event", - logicalT, - nsConfig.getNamespace(), - docConfig.getDocumentId())); - return; - } + if (docConfig.hasCommittedVersion(currentRemoteVersionInfo)) { + // Skip this event since we generated it. + logger.info(String.format( + Locale.US, + "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote change event was " + + "generated by us; dropping the event", + logicalT, + nsConfig.getNamespace(), + docConfig.getDocumentId())); + return; } + if (docConfig.getLastUncommittedChangeEvent() == null) { switch (remoteChangeEvent.getOperationType()) { case REPLACE: @@ -609,18 +597,13 @@ private void syncRemoteChangeEventToLocal( // Check for pending writes on this version { - final BsonValue lastKnownLocalVersion; - if (localDocument == null) { - lastKnownLocalVersion = docConfig.getLastKnownRemoteVersion(); - } else { - lastKnownLocalVersion = DocumentVersionInfo - .getLocalVersionInfo(docConfig).getVersionDoc(); - } + final DocumentVersionInfo lastKnownLocalVersionInfo = DocumentVersionInfo + .getLocalVersionInfo(docConfig); - // There is a pending write that must skip R2L if both versions are null. The absence of a + // There is a pending write that must skip R2L if both versions are empty. The absence of a // version is effectively a version. The pending write, if it's not a delete, should be // setting a new version anyway. - if (lastKnownLocalVersion == null && currentRemoteVersion == null) { + if (!lastKnownLocalVersionInfo.hasVersion() && !currentRemoteVersionInfo.hasVersion()) { logger.info(String.format( Locale.US, "t='%d': syncRemoteChangeEventToLocal ns=%s documentId=%s remote and local have same " @@ -736,8 +719,8 @@ private void syncLocalToRemote() { ); continue; } - final BsonDocument nextDoc = withNextVersion(localDoc, localVersionInfo); - nextVersion = DocumentVersionInfo.getDocumentVersionDoc(nextDoc); + nextVersion = localVersionInfo.getNextVersion(); + final BsonDocument nextDoc = withNewVersion(localDoc, nextVersion); final RemoteUpdateResult result; try { @@ -782,8 +765,8 @@ private void syncLocalToRemote() { ); continue; } - final BsonDocument nextDoc = withNextVersion(localDoc, localVersionInfo); - nextVersion = DocumentVersionInfo.getDocumentVersionDoc(nextDoc); + nextVersion = localVersionInfo.getNextVersion(); + final BsonDocument nextDoc = withNewVersion(localDoc, nextVersion); final BsonDocument translatedUpdate = new BsonDocument(); if (!localChangeEvent.getUpdateDescription().getUpdatedFields().isEmpty()) { @@ -1379,18 +1362,12 @@ private void replaceOrUpsertOneFromResolution( return; } - final BsonDocument docToReplace = withNextVersion( - document, - DocumentVersionInfo.getLocalVersionInfo(config) - ); - final BsonDocument result = getLocalCollection(namespace) .findOneAndReplace( getDocumentIdFilter(documentId), - docToReplace, + document, new FindOneAndReplaceOptions().upsert(true).returnDocument(ReturnDocument.AFTER)); final ChangeEvent event; - if (fromDelete) { event = changeEventForLocalInsert(namespace, result, true); config.setSomePendingWrites( @@ -1735,19 +1712,4 @@ private static BsonDocument withNewVersion( newDocument.put(DOCUMENT_VERSION_FIELD, newVersion); return newDocument; } - - /** - * Returns a document with a new version added to the given document. The added version is - * the provided version with a version counter incremented by one, or a fresh version if the - * provided version is empty. - * @param document the document to attach the next version to. - * @param versionInfo the version from which the next version will be derived - * @return a document with the newly updated version added to the given document. - */ - private static BsonDocument withNextVersion( - final BsonDocument document, - final DocumentVersionInfo versionInfo - ) { - return withNewVersion(document, versionInfo.getNextVersion()); - } } diff --git a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java index a99f1001e..9b097533c 100644 --- a/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java +++ b/core/services/mongodb-remote/src/main/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfo.java @@ -163,7 +163,11 @@ static DocumentVersionInfo getLocalVersionInfo( */ static DocumentVersionInfo getRemoteVersionInfo(final BsonDocument remoteDocument) { final BsonDocument version = getDocumentVersionDoc(remoteDocument); - return new DocumentVersionInfo(version, BsonUtils.getDocumentId(remoteDocument)); + return new DocumentVersionInfo( + version, + remoteDocument != null + ? BsonUtils.getDocumentId(remoteDocument, null) : null + ); } /** From 6f81bfed41ab41ba086cfb8b861ae5b0deabd9d3 Mon Sep 17 00:00:00 2001 From: Adam Chelminski Date: Fri, 12 Oct 2018 17:18:21 -0400 Subject: [PATCH 10/10] add unit test for DocumentVersionInfo --- .../internal/DocumentVersionInfoUnitTests.kt | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 core/services/mongodb-remote/src/test/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfoUnitTests.kt diff --git a/core/services/mongodb-remote/src/test/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfoUnitTests.kt b/core/services/mongodb-remote/src/test/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfoUnitTests.kt new file mode 100644 index 000000000..1a79ec725 --- /dev/null +++ b/core/services/mongodb-remote/src/test/java/com/mongodb/stitch/core/services/mongodb/remote/sync/internal/DocumentVersionInfoUnitTests.kt @@ -0,0 +1,39 @@ +package com.mongodb.stitch.core.services.mongodb.remote.sync.internal + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class DocumentVersionInfoUnitTests { + @Test + fun testGetNextVersion() { + val emptyVersion = DocumentVersionInfo.fromVersionDoc(null) + assertFalse(emptyVersion.hasVersion()) + assertNull(emptyVersion.versionDoc) + assertNull(emptyVersion.filter) + + // the next version from an empty version should be a non-empty version with a version + // counter of zero. + val nextVersion = DocumentVersionInfo.fromVersionDoc(emptyVersion.nextVersion) + assertTrue(nextVersion.hasVersion()) + assertNotNull(nextVersion.versionDoc) + assertNull(nextVersion.filter) + + assertEquals(1, nextVersion.version.syncProtocolVersion) + assertEquals(0, nextVersion.version.versionCounter) + + // the next version from a non-empty version should be the same version, but with the + // version counter incremented by one + val incrementedVersion = DocumentVersionInfo.fromVersionDoc(nextVersion.nextVersion) + assertTrue(nextVersion.hasVersion()) + assertNotNull(incrementedVersion .versionDoc) + assertNull(incrementedVersion.filter) + + assertEquals(1, incrementedVersion.version.syncProtocolVersion) + assertEquals(1, incrementedVersion.version.versionCounter) + assertEquals(nextVersion.version.instanceId, incrementedVersion.version.instanceId) + } +}