Make sync work across VV upgrades (#1913)

* Expose DataFile::_log() as public

* Make sync work across VV upgrades

The transition from tree-based revids to version vectors is tricky
for replication, and the prior approach (adding a fake 'legacy'
version when creating the vector) didn't turn out to work.

Instead, when a doc is upgraded in memory to a VectorRecord it keeps
its tree-based/legacy revids. When the doc is saved it creates a
version vector for the local Revision but also remembers the prior
legacy revid. Remote Revisions keep their tree-based IDs until VVs
are pulled from or pushed to those remotes.

Revision histories may now include one or more legacy revids at the
end. These are what allow the recipient of a revision (an active or
passive puller) to match up an incoming version vector with a local
legacy revid.

There's still a question of how long we need to keep the legacy revid
of the document. It only needs to be used once per remote, but in a
P2P scenario you don't know if you might have to sync with some new
peer that hasn't upgraded that doc yet. In any case the revid isn't
very big (~24 bytes) so it may not be worth removing.

* Removed unnecessary VV-upgrade constants

Some constants added earlier for VV upgrading and dealing with legacy
revids, which have been superseded by newer techniques, were nuked:

- kDocGetUpgraded / kUpgrade
- kLegacyRevSourceID
- VectorRecord::_versioning

* Fixed some broken test code

c4doc_getRevisionHistory requires doc be loaded with kDocGetAll.
VectorDoc correctly threw an exception on this.
Also, all this is doing is checking the current rev, so no need to
get the history.

Author: snej

C/include/c4DocumentTypes.h

@@ -51,7 +51,6 @@ typedef C4_ENUM(uint8_t, C4DocContentLevel){
         kDocGetMetadata,    ///< Only get revID and flags
         kDocGetCurrentRev,  ///< Get current revision body but not other revisions/remotes
         kDocGetAll,         ///< Get everything
-        kDocGetUpgraded,    ///< Get everything, upgrade to latest format (version vectors)
 };                          // Note: Same as litecore::ContentOption
 
 // Ignore warning about not initializing members, it must be this way to be C-compatible

C/tests/c4DatabaseTest.cc

@@ -1418,161 +1418,83 @@ N_WAY_TEST_CASE_METHOD(C4DatabaseTest, "Database Upgrade To Version Vectors", "[
 
     auto defaultColl = c4db_getDefaultCollection(db, nullptr);
 
-    SECTION("Read-Only") {
-        // Check doc 1:
-        C4Document* doc;
-        doc = c4coll_getDoc(defaultColl, "doc-001"_sl, true, kDocGetAll, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "2-c001d00d");
-        alloc_slice history(c4doc_getRevisionHistory(doc, 0, nullptr, 0));
-        CHECK(history == "2-c001d00d");
-        CHECK(doc->sequence == 7);
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"one","rev":"two"})");
-        c4doc_release(doc);
-
-        // Check doc 2:
-        doc = c4coll_getDoc(defaultColl, "doc-002"_sl, true, kDocGetAll, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "3-deadbeef");
-        history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(history == "3-deadbeef");
-        CHECK(doc->sequence == 9);
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"two","rev":"three"})");
-        alloc_slice remoteVers = c4doc_getRemoteAncestor(doc, 1);
-        CHECK(remoteVers == "3-deadbeef");
-        CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"two","rev":"three"})");
-        c4doc_release(doc);
-
-        // Check doc 3:
-        doc = c4coll_getDoc(defaultColl, "doc-003"_sl, true, kDocGetAll, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "3-deadbeef");
-        history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(history == "3-deadbeef, 2-c001d00d");
-        CHECK(doc->sequence == 11);
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"three","rev":"three"})");
-        remoteVers = c4doc_getRemoteAncestor(doc, 1);
-        CHECK(remoteVers == "2-c001d00d");
-        CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"three","rev":"two"})");
-        c4doc_release(doc);
-
-        // Check doc 4:
-        doc = c4coll_getDoc(defaultColl, "doc-004"_sl, true, kDocGetAll, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "3-deadbeef");
-        history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(history == "3-deadbeef");  // parent 2-c001d00d doesn't show up bc it isn't a remote
-        CHECK(doc->sequence == 14);
-        CHECK(doc->flags == (kDocConflicted | kDocExists));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"four","rev":"three"})");
-        remoteVers = c4doc_getRemoteAncestor(doc, 1);
-        CHECK(remoteVers == "3-cc");
-        REQUIRE(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(history == "3-cc");
-        CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"ans*wer":42})");
-        c4doc_release(doc);
-
-        // Check deleted doc:
-        doc = c4coll_getDoc(defaultColl, "doc-DEL"_sl, true, kDocGetAll, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "1-abcd");
-        history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(history == "1-abcd");
-        CHECK(doc->sequence == 6);
-        CHECK(doc->flags == (kDocDeleted | kDocExists));
-        c4doc_release(doc);
-    }
-
-    SECTION("Upgrading") {
-        // Note: The revID/version checks below hardcode the base timestamp used for upgrading legacy
-        // replicated revIDs. It's currently 0x1770000000000000 (see HybridClock.hh). If that value
-        // changes, or the scheme for converting rev-tree revIDs to versions changes, the values below
-        // need to change too.
-        REQUIRE(uint64_t(litecore::kMinValidTime) == 0x1770000000000000);
-
-        // Check doc 1:
-        C4Document* doc;
-        doc = c4coll_getDoc(defaultColl, "doc-001"_sl, true, kDocGetUpgraded, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "1770000000000002@*");
-        alloc_slice versionVector(c4doc_getRevisionHistory(doc, 0, nullptr, 0));
-        CHECK(versionVector == "1770000000000002@*");
-        CHECK(doc->sequence == 7);
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"one","rev":"two"})");
-        c4doc_release(doc);
-
-        // Check doc 2:
-        doc = c4coll_getDoc(defaultColl, "doc-002"_sl, true, kDocGetUpgraded, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(c4rev_getTimestamp(doc->revID) == uint64_t(litecore::kMinValidTime) + 3);
-        CHECK(slice(doc->revID) == "1770000000000003@?");  // 0x1770000000000003 = kMinValidTime + 3
-        versionVector = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(versionVector == "1770000000000003@?");
-        CHECK(doc->sequence == 9);
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"two","rev":"three"})");
-        alloc_slice remoteVers = c4doc_getRemoteAncestor(doc, 1);
-        CHECK(remoteVers == "1770000000000003@?");
-        CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"two","rev":"three"})");
+    // Check doc 1:
+    C4Document* doc;
+    doc = c4coll_getDoc(defaultColl, "doc-001"_sl, true, kDocGetAll, ERROR_INFO());
+    REQUIRE(doc);
+    CHECK(slice(doc->revID) == "2-c001d00d");
+    alloc_slice history(c4doc_getRevisionHistory(doc, 0, nullptr, 0));
+    CHECK(history == "2-c001d00d");
+    CHECK(doc->sequence == 7);
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"one","rev":"two"})");
+    c4doc_release(doc);
 
+    // Check doc 2:
+    doc = c4coll_getDoc(defaultColl, "doc-002"_sl, true, kDocGetAll, ERROR_INFO());
+    REQUIRE(doc);
+    CHECK(slice(doc->revID) == "3-deadbeef");
+    history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
+    CHECK(history == "3-deadbeef");
+    CHECK(doc->sequence == 9);
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"two","rev":"three"})");
+    alloc_slice remoteVers = c4doc_getRemoteAncestor(doc, 1);
+    CHECK(remoteVers == "3-deadbeef");
+    CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"two","rev":"three"})");
+    {
         // update & save it:
-        {
-            TransactionHelper t(db);
-            C4Document*       newDoc = c4doc_update(doc, kFleeceBody, 0, ERROR_INFO());
-            REQUIRE(newDoc);
-            CHECK(slice(newDoc->revID) > "178532e35bcd0001@*");
-            alloc_slice newVersionVector(c4doc_getRevisionHistory(newDoc, 0, nullptr, 0));
-            CHECK(newVersionVector.hasSuffix("@*; 1770000000000003@?"));
-            c4doc_release(newDoc);
-        }
-        c4doc_release(doc);
+        TransactionHelper t(db);
+        C4Document*       newDoc = c4doc_update(doc, kFleeceBody, 0, ERROR_INFO());
+        REQUIRE(newDoc);
+        CHECK(slice(newDoc->revID).hasSuffix("@*"));
+        CHECK(slice(newDoc->revID) > "178532e35bcd0001@*");
+        alloc_slice newVersionVector(c4doc_getRevisionHistory(newDoc, 0, nullptr, 0));
+        CHECK(newVersionVector.hasSuffix("@*; 3-deadbeef"));
+        c4doc_release(newDoc);
+    }
+    c4doc_release(doc);
 
-        // Check doc 3:
-        doc = c4coll_getDoc(defaultColl, "doc-003"_sl, true, kDocGetUpgraded, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "1770000000000003@*");
-        versionVector = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(versionVector == "1770000000000003@*; 1770000000000002@?");
-        CHECK(doc->sequence == 11);
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"three","rev":"three"})");
-        remoteVers = c4doc_getRemoteAncestor(doc, 1);
-        CHECK(remoteVers == "1770000000000002@?");
-        CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"three","rev":"two"})");
-        c4doc_release(doc);
+    // Check doc 3:
+    doc = c4coll_getDoc(defaultColl, "doc-003"_sl, true, kDocGetAll, ERROR_INFO());
+    REQUIRE(doc);
+    CHECK(slice(doc->revID) == "3-deadbeef");
+    history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
+    CHECK(history == "3-deadbeef, 2-c001d00d");
+    CHECK(doc->sequence == 11);
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"three","rev":"three"})");
+    remoteVers = c4doc_getRemoteAncestor(doc, 1);
+    CHECK(remoteVers == "2-c001d00d");
+    CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"three","rev":"two"})");
+    c4doc_release(doc);
 
-        // Check doc 4:
-        doc = c4coll_getDoc(defaultColl, "doc-004"_sl, true, kDocGetUpgraded, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "1770000000000003@*");
-        versionVector = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(versionVector == "1770000000000003@*");
-        CHECK(doc->sequence == 14);
-        CHECK(doc->flags == (kDocConflicted | kDocExists));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"four","rev":"three"})");
-        remoteVers = c4doc_getRemoteAncestor(doc, 1);
-        CHECK(remoteVers == "1770000000000003@?");
-        REQUIRE(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        versionVector = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(versionVector == "1770000000000003@?");
-        CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
-        CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"ans*wer":42})");
-        c4doc_release(doc);
+    // Check doc 4:
+    doc = c4coll_getDoc(defaultColl, "doc-004"_sl, true, kDocGetAll, ERROR_INFO());
+    REQUIRE(doc);
+    CHECK(slice(doc->revID) == "3-deadbeef");
+    history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
+    CHECK(history == "3-deadbeef");  // parent 2-c001d00d doesn't show up bc it isn't a remote
+    CHECK(doc->sequence == 14);
+    CHECK(doc->flags == (kDocConflicted | kDocExists));
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"doc":"four","rev":"three"})");
+    remoteVers = c4doc_getRemoteAncestor(doc, 1);
+    CHECK(remoteVers == "3-cc");
+    REQUIRE(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
+    history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
+    CHECK(history == "3-cc");
+    CHECK(c4doc_selectRevision(doc, remoteVers, true, WITH_ERROR()));
+    CHECK(Dict(c4doc_getProperties(doc)).toJSONString() == R"({"ans*wer":42})");
+    c4doc_release(doc);
 
-        // Check deleted doc:
-        doc = c4coll_getDoc(defaultColl, "doc-DEL"_sl, true, kDocGetUpgraded, ERROR_INFO());
-        REQUIRE(doc);
-        CHECK(slice(doc->revID) == "1770000000000001@*");
-        versionVector = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
-        CHECK(versionVector == "1770000000000001@*");
-        CHECK(doc->sequence == 6);
-        CHECK(doc->flags == (kDocDeleted | kDocExists));
-        c4doc_release(doc);
-    }
+    // Check deleted doc:
+    doc = c4coll_getDoc(defaultColl, "doc-DEL"_sl, true, kDocGetAll, ERROR_INFO());
+    REQUIRE(doc);
+    CHECK(slice(doc->revID) == "1-abcd");
+    history = c4doc_getRevisionHistory(doc, 0, nullptr, 0);
+    CHECK(history == "1-abcd");
+    CHECK(doc->sequence == 6);
+    CHECK(doc->flags == (kDocDeleted | kDocExists));
+    c4doc_release(doc);
 }
 
 // CBL-3706: Previously, calling these functions after deleting the default collection causes

C/tests/c4Test.cc

@@ -248,10 +248,15 @@ void C4Test::closeDB() {
     db = nullptr;
 }
 
-void C4Test::reopenDB() {
-    // Update _dbConfig in case db was reopened with different flags or encryption:
+void C4Test::syncDBConfig() {
+    REQUIRE(db);
     _dbConfig.flags         = c4db_getConfig2(db)->flags;
     _dbConfig.encryptionKey = c4db_getConfig2(db)->encryptionKey;
+}
+
+void C4Test::reopenDB() {
+    // Update _dbConfig in case db was reopened with different flags or encryption:
+    syncDBConfig();
 
     closeDB();
     db = c4db_openNamed(kDatabaseName, &_dbConfig, ERROR_INFO());

C/tests/c4Test.hh

@@ -217,6 +217,8 @@ class C4Test {
 
     [[nodiscard]] const C4DatabaseConfig2& dbConfig() const { return _dbConfig; }
 
+    void syncDBConfig();
+
     [[nodiscard]] C4StorageEngine storageType() const { return _storage; }
 
     [[nodiscard]] bool isSQLite() const { return storageType() == kC4SQLiteStorageEngine; }