CBL-6239: Fixed c4doc_getRevisionBody in a 2.x db (#2136)

VectorRecord didn't properly manage the `body` column of a doc from a
2.x db that doesn't have the `extra` column. In such a db the body is
the encoded rev tree, not just the current rev. VectorRecord was
returning that entire slice from its currentRevisionData method.

I added a new member _bodyDocRange, a slice that's just the current
rev within the _bodyDoc. Normally it's the same as _bodyDoc.data(),
but in a 2.x doc it encompasses just the current rev body.

Fixes CBL-6239

Co-authored-by: Jianmin Zhao <[email protected]>

Author: snej

C/tests/c4DatabaseTest.cc

@@ -1115,13 +1115,18 @@ static void testOpeningOlderDBFixture(const string& dbPath, C4DatabaseFlags with
     for ( unsigned i = 1; i <= 100; i++ ) {
         snprintf(docID, bufSize, "doc-%03u", i);
         INFO("Checking docID " << docID);
-        auto        defaultColl = c4db_getDefaultCollection(db, nullptr);
-        C4Document* doc         = c4coll_getDoc(defaultColl, slice(docID), true, kDocGetCurrentRev, ERROR_INFO());
+        auto                defaultColl = c4db_getDefaultCollection(db, nullptr);
+        c4::ref<C4Document> doc = c4coll_getDoc(defaultColl, slice(docID), true, kDocGetCurrentRev, ERROR_INFO());
         REQUIRE(doc);
         CHECK(((doc->flags & kDocDeleted) != 0) == (i > 50));
-        Dict body = c4doc_getProperties(doc);
-        CHECK(body["n"].asInt() == i);
-        c4doc_release(doc);
+        Dict root = c4doc_getProperties(doc);
+        CHECK(root["n"].asInt() == i);
+        // Test getting doc body from data [CBL-6239]:
+        slice body = c4doc_getRevisionBody(doc);
+        REQUIRE(body);
+        FLValue rootVal = FLValue_FromData(body, kFLUntrusted);
+        CHECK(rootVal);
+        CHECK(FLValue_GetType(rootVal) == kFLDict);
     }
 
     // Verify enumerating documents:

LiteCore/RevTrees/VectorRecord.cc

@@ -91,11 +91,10 @@ namespace litecore {
     static constexpr slice kLegacyRevIDKey   = "~";
     static constexpr slice kRevFlagsKey      = "&";
 
-    bool Revision::hasVersionVector() const { return revID.isVersion(); }
 
-    Version Revision::version() const { return VersionVector::readCurrentVersionFromBinary(revID); }
+#pragma mark - INITIALIZATION:
 
-    VersionVector Revision::versionVector() const { return VersionVector::fromBinary(revID); }
+    bool Revision::hasVersionVector() const { return revID.isVersion(); }
 
     VectorRecord::VectorRecord(KeyStore& store, const Record& rec)
         : _store(store)
@@ -133,6 +132,7 @@ namespace litecore {
         } else {
             if ( body ) {
                 _bodyDoc            = newLinkedFleeceDoc(body, kFLTrusted);
+                _bodyDocRange       = _bodyDoc.data();
                 _current.properties = _bodyDoc.asDict();
                 if ( !_current.properties )
                     error::_throw(error::CorruptRevisionData, "VectorRecord reading properties error");
@@ -180,10 +180,13 @@ namespace litecore {
         if ( _docFlags & DocumentFlags::kSynced ) revTree.setLatestRevisionOnRemote(1, curRev);
 
         if ( !extra ) {
-            // This is a v2.x document with body & rev-tree in `body`, and no `extra`:
+            // This is a v2.x document with body & rev-tree in `body`, and no `extra`.
+            // That means _bodyDoc's data is the entire rev-tree, not just the current rev data.
+            // This is why we have _bodyDocRange! Set it to the current rev itself:
             Assert(!_bodyDoc);
             _bodyDoc            = newLinkedFleeceDoc(body, kFLTrustedDontParse);
-            FLValue bodyProps   = FLValue_FromData(curRev->body(), kFLTrusted);
+            _bodyDocRange       = curRev->body();
+            FLValue bodyProps   = FLValue_FromData(_bodyDocRange, kFLTrusted);
             _current.properties = Value(bodyProps).asDict();
             if ( !_current.properties )
                 error::_throw(error::CorruptRevisionData, "VectorRecord reading 2.x properties error");
@@ -370,9 +373,13 @@ namespace litecore {
 
 #pragma mark - CURRENT REVISION:
 
+    Version Revision::version() const { return VersionVector::readCurrentVersionFromBinary(revID); }
+
+    VersionVector Revision::versionVector() const { return VersionVector::fromBinary(revID); }
+
     slice VectorRecord::currentRevisionData() const {
         requireBody();
-        return _bodyDoc.data();
+        return _bodyDocRange;
     }
 
     void VectorRecord::setCurrentRevision(const Revision& rev) {
@@ -383,7 +390,7 @@ namespace litecore {
 
     Dict VectorRecord::originalProperties() const {
         requireBody();
-        return _bodyDoc.asDict();
+        return Value(FLValue_FromData(_bodyDocRange, kFLTrusted)).asDict();
     }
 
     MutableDict VectorRecord::mutableProperties() {

LiteCore/RevTrees/VectorRecord.hh

@@ -255,22 +255,26 @@ namespace litecore {
         void                           clearPropertiesChanged() const;
         void                           updateDocFlags();
 
-        KeyStore&                    _store;              // The database KeyStore
-        FLEncoder                    _encoder{nullptr};   // Database shared Fleece Encoder
-        alloc_slice                  _docID;              // The docID
-        sequence_t                   _sequence;           // The Record's sequence
-        uint64_t                     _subsequence;        // The Record's subsequence
-        DocumentFlags                _docFlags;           // Document-level flags
-        alloc_slice                  _savedRevID;         // Revision ID saved in db (may == _revID)
-        alloc_slice                  _revID;              // Current revision ID backing store
-        Revision                     _current;            // Current revision
-        fleece::RetainedValue        _currentProperties;  // Retains local properties
-        fleece::Doc                  _bodyDoc;            // If saved, a Doc of the Fleece body
-        fleece::Doc                  _extraDoc;           // Fleece Doc holding record `extra`
-        fleece::Array                _revisions;          // Top-level parsed body; stores revs
-        mutable fleece::MutableArray _mutatedRevisions;   // Mutable version of `_revisions`
-        bool                         _changed{false};     // Set to true on explicit change
-        ContentOption                _whichContent;       // Which parts of record are available
+        KeyStore&                    _store;                // The database KeyStore
+        FLEncoder                    _encoder{nullptr};     // Database shared Fleece Encoder
+        alloc_slice                  _docID;                // The docID
+        sequence_t                   _sequence;             // The Record's sequence
+        uint64_t                     _subsequence;          // The Record's subsequence
+        DocumentFlags                _docFlags;             // Document-level flags
+        alloc_slice                  _savedRevID;           // Revision ID saved in db (may == _revID)
+        alloc_slice                  _revID;                // Current revision ID backing store
+        Revision                     _current;              // Current revision
+        fleece::RetainedValue        _currentProperties;    // Retains local properties
+        fleece::Doc                  _bodyDoc;              // If saved, a Doc of the `body` column
+        slice                        _bodyDocRange;         // Fleece data within _bodyDoc
+        fleece::Doc                  _extraDoc;             // Fleece Doc holding record `extra`
+        fleece::Array                _revisions;            // Top-level parsed body; stores revs
+        mutable fleece::MutableArray _mutatedRevisions;     // Mutable version of `_revisions`
+        Versioning                   _versioning;           // RevIDs or VersionVectors?
+        int                          _parentOfLocal{};      // (only used in imported revtree)
+        bool                         _changed{false};       // Set to true on explicit change
+        bool                         _revIDChanged{false};  // Has setRevID() been called?
+        ContentOption                _whichContent;         // Which parts of record are available
         // (Note: _changed doesn't reflect mutations to _properties; changed() checks for those.)
     };
 }  // namespace litecore