From f6a0add183cbdd07f441c3bcbaf19c67d3a530ea Mon Sep 17 00:00:00 2001 From: Ritesh Kumar Date: Wed, 12 Nov 2025 14:43:55 +0530 Subject: [PATCH 1/4] use flags to check if version is deleted --- db/crud.go | 7 ++-- db/crud_test.go | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ db/document.go | 39 +++++++++++++++----- 3 files changed, 127 insertions(+), 13 deletions(-) diff --git a/db/crud.go b/db/crud.go index 4c70ff6efc..7424428741 100644 --- a/db/crud.go +++ b/db/crud.go @@ -3626,10 +3626,7 @@ func (db *DatabaseCollectionWithUser) CheckProposedRev(ctx context.Context, doci return ProposedRev_OK, "" // Users can't upload design docs, so ignore them } - level := DocUnmarshalRev - if parentRevID == "" { - level = DocUnmarshalHistory // doc.History only needed in this case (see below) - } + level := DocUnmarshalRevAndFlags syncData, _, err := db.GetDocSyncDataNoImport(ctx, docid, level) if err != nil { if !base.IsDocNotFoundError(err) && !base.IsXattrNotFoundError(err) { @@ -3644,7 +3641,7 @@ func (db *DatabaseCollectionWithUser) CheckProposedRev(ctx context.Context, doci } else if syncData.GetRevTreeID() == parentRevID { // Proposed rev's parent is my current revision; OK to add: return ProposedRev_OK, "" - } else if parentRevID == "" && syncData.History[syncData.GetRevTreeID()].Deleted { + } else if parentRevID == "" && syncData.hasFlag(channels.Deleted) { // Proposed rev has no parent and doc is currently deleted; OK to add: return ProposedRev_OK, "" } else { diff --git a/db/crud_test.go b/db/crud_test.go index a470549aa6..947f0ca1b4 100644 --- a/db/crud_test.go +++ b/db/crud_test.go @@ -2282,3 +2282,97 @@ func TestSyncDataCVEqual(t *testing.T) { }) } } + +func TestProposedRev(t *testing.T) { + + base.SetUpTestLogging(t, base.LevelDebug, base.KeyAll) + + db, ctx := setupTestDB(t) + defer db.Close(ctx) + collection, ctx := GetSingleDatabaseCollectionWithUser(ctx, t, db) + + // create 3 documents + body := Body{"key1": "value1", "key2": 1234} + _, doc1, err := collection.Put(ctx, "doc1", body) + require.NoError(t, err) + doc1Rev := doc1.GetRevTreeID() + + _, doc2, err := collection.Put(ctx, "doc2", body) + require.NoError(t, err) + doc2Rev1 := doc2.GetRevTreeID() + _, doc2, err = collection.Put(ctx, "doc2", Body{"_rev": doc2Rev1, "key1": "value2", "key2": 5678}) + require.NoError(t, err) + doc2Rev2 := doc2.GetRevTreeID() + + _, doc3, err := collection.Put(ctx, "doc3", body) + require.NoError(t, err) + doc3Rev1 := doc3.GetRevTreeID() + _, doc3, err = collection.Put(ctx, "doc3", Body{"_rev": doc3Rev1, "_deleted": true}) + require.NoError(t, err) + + testCases := []struct { + name string + revID string + parentRevID string + expectedStatus ProposedRevStatus + currentRev string + docID string + }{ + { + name: "no existing document", + revID: "1-abc", + parentRevID: "", + expectedStatus: ProposedRev_OK_IsNew, + currentRev: "", + docID: "doc", + }, + { + name: "existing revision with no previous version", + revID: doc1Rev, + parentRevID: "", + expectedStatus: ProposedRev_Exists, + currentRev: "", + docID: "doc1", + }, + { + name: "existing revision with previous version", + revID: doc2Rev2, + parentRevID: doc2Rev1, + expectedStatus: ProposedRev_Exists, + currentRev: "", + docID: "doc2", + }, + { + name: "new revision with previous version", + revID: "2-abc", + parentRevID: doc1Rev, + expectedStatus: ProposedRev_OK, + currentRev: "", + docID: "doc1", + }, + { + name: "new revision with previous revision as tombstone", + revID: "1-abc", + parentRevID: "", + expectedStatus: ProposedRev_OK, + currentRev: "", + docID: "doc3", + }, + { + name: "conflicting revision with previous version", + revID: "2-abc", + parentRevID: doc2Rev1, + expectedStatus: ProposedRev_Conflict, + currentRev: doc2Rev2, + docID: "doc2", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + status, rev := collection.CheckProposedRev(ctx, tc.docID, tc.revID, tc.parentRevID) + assert.Equal(t, tc.expectedStatus, status) + assert.Equal(t, tc.currentRev, rev) + }) + } +} diff --git a/db/document.go b/db/document.go index c1df1165cb..4f0a7dedba 100644 --- a/db/document.go +++ b/db/document.go @@ -44,6 +44,7 @@ const ( DocUnmarshalRev // Unmarshals revTreeID + CAS only (no HLV) DocUnmarshalCAS // Unmarshals CAS (for import check) only DocUnmarshalNone // No unmarshalling (skips import/upgrade check) + DocUnmarshalRevAndFlags // Unmarshals revTreeID + CAS and Flags (no HLV) ) const ( @@ -167,6 +168,14 @@ func (sd *SyncData) SetCV(hlv *HybridLogicalVector) { sd.RevAndVersion.CurrentVersion = string(base.Uint64CASToLittleEndianHex(hlv.Version)) } +func (sd *SyncData) hasFlag(flag uint8) bool { + return sd.Flags&flag != 0 +} + +func (sd *SyncData) IsDeleted() bool { + return sd.hasFlag(channels.Deleted) +} + // RedactRawGlobalSyncData runs HashRedact on the given global sync data. func RedactRawGlobalSyncData(syncData []byte, redactSalt string) ([]byte, error) { if redactSalt == "" { @@ -306,6 +315,11 @@ type revOnlySyncData struct { CurrentRev channels.RevAndVersion `json:"rev"` } +type revAndFlagsSyncData struct { + revOnlySyncData + Flags uint8 `json:"flags"` +} + type casOnlySyncData struct { Cas string `json:"cas"` } @@ -331,10 +345,6 @@ func (doc *Document) MarshalBodyAndSync() (retBytes []byte, err error) { } } -func (doc *Document) IsDeleted() bool { - return doc.hasFlag(channels.Deleted) -} - func (doc *Document) BodyWithSpecialProperties(ctx context.Context) ([]byte, error) { bodyBytes, err := doc.BodyBytes(ctx) if err != nil { @@ -754,10 +764,6 @@ func (doc *Document) IsSGWrite(ctx context.Context, rawBody []byte) (isSGWrite b return true, true, false } -func (doc *Document) hasFlag(flag uint8) bool { - return doc.Flags&flag != 0 -} - func (doc *Document) setFlag(flag uint8, state bool) { if state { doc.Flags |= flag @@ -1336,6 +1342,23 @@ func (doc *Document) UnmarshalWithXattrs(ctx context.Context, data, syncXattrDat doc.SyncData = SyncData{} } doc._rawBody = data + case DocUnmarshalRevAndFlags: + // Unmarshal rev ,cas and flags from sync metadata + if syncXattrData != nil { + var revOnlyMeta revAndFlagsSyncData + unmarshalErr := base.JSONUnmarshal(syncXattrData, &revOnlyMeta) + if unmarshalErr != nil { + return pkgerrors.WithStack(base.RedactErrorf("Failed to UnmarshalWithXattrs() doc with id: %s (DocUnmarshalRev). Error: %v", base.UD(doc.ID), unmarshalErr)) + } + doc.SyncData = SyncData{ + RevAndVersion: revOnlyMeta.CurrentRev, + Cas: revOnlyMeta.Cas, + Flags: revOnlyMeta.Flags, + } + } else { + doc.SyncData = SyncData{} + } + doc._rawBody = data } // If there's no body, but there is an xattr, set deleted flag and initialize an empty body From fb2c95e86ab19bde1a2052d2b1792eaa877cc7ca Mon Sep 17 00:00:00 2001 From: Ritesh Kumar Date: Wed, 12 Nov 2025 14:49:45 +0530 Subject: [PATCH 2/4] lint fix --- db/crud_test.go | 2 +- db/document.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/db/crud_test.go b/db/crud_test.go index 947f0ca1b4..341cba0ac5 100644 --- a/db/crud_test.go +++ b/db/crud_test.go @@ -2307,7 +2307,7 @@ func TestProposedRev(t *testing.T) { _, doc3, err := collection.Put(ctx, "doc3", body) require.NoError(t, err) doc3Rev1 := doc3.GetRevTreeID() - _, doc3, err = collection.Put(ctx, "doc3", Body{"_rev": doc3Rev1, "_deleted": true}) + _, _, err = collection.Put(ctx, "doc3", Body{"_rev": doc3Rev1, "_deleted": true}) require.NoError(t, err) testCases := []struct { diff --git a/db/document.go b/db/document.go index 4f0a7dedba..2e18ed9bf7 100644 --- a/db/document.go +++ b/db/document.go @@ -37,14 +37,14 @@ const DocumentHistoryMaxEntriesPerChannel = 5 type DocumentUnmarshalLevel uint8 const ( - DocUnmarshalAll = DocumentUnmarshalLevel(iota) // Unmarshals metadata and body - DocUnmarshalSync // Unmarshals metadata - DocUnmarshalNoHistory // Unmarshals metadata excluding revtree history - DocUnmarshalHistory // Unmarshals revtree history + rev + CAS only - DocUnmarshalRev // Unmarshals revTreeID + CAS only (no HLV) - DocUnmarshalCAS // Unmarshals CAS (for import check) only - DocUnmarshalNone // No unmarshalling (skips import/upgrade check) - DocUnmarshalRevAndFlags // Unmarshals revTreeID + CAS and Flags (no HLV) + DocUnmarshalAll = DocumentUnmarshalLevel(iota) // Unmarshals metadata and body + DocUnmarshalSync // Unmarshals metadata + DocUnmarshalNoHistory // Unmarshals metadata excluding revtree history + DocUnmarshalHistory // Unmarshals revtree history + rev + CAS only + DocUnmarshalRev // Unmarshals revTreeID + CAS only (no HLV) + DocUnmarshalCAS // Unmarshals CAS (for import check) only + DocUnmarshalNone // No unmarshalling (skips import/upgrade check) + DocUnmarshalRevAndFlags // Unmarshals revTreeID + CAS and Flags (no HLV) ) const ( From 19d48a57a6fe5895f1ed64fc9161e6883d8933b0 Mon Sep 17 00:00:00 2001 From: Ritesh Kumar Date: Wed, 12 Nov 2025 14:52:19 +0530 Subject: [PATCH 3/4] fix few nits --- db/crud.go | 2 +- db/document.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/crud.go b/db/crud.go index 7424428741..61d98bf346 100644 --- a/db/crud.go +++ b/db/crud.go @@ -3641,7 +3641,7 @@ func (db *DatabaseCollectionWithUser) CheckProposedRev(ctx context.Context, doci } else if syncData.GetRevTreeID() == parentRevID { // Proposed rev's parent is my current revision; OK to add: return ProposedRev_OK, "" - } else if parentRevID == "" && syncData.hasFlag(channels.Deleted) { + } else if parentRevID == "" && syncData.IsDeleted() { // Proposed rev has no parent and doc is currently deleted; OK to add: return ProposedRev_OK, "" } else { diff --git a/db/document.go b/db/document.go index 2e18ed9bf7..7d7bf495d2 100644 --- a/db/document.go +++ b/db/document.go @@ -1343,12 +1343,12 @@ func (doc *Document) UnmarshalWithXattrs(ctx context.Context, data, syncXattrDat } doc._rawBody = data case DocUnmarshalRevAndFlags: - // Unmarshal rev ,cas and flags from sync metadata + // Unmarshal rev, cas and flags from sync metadata if syncXattrData != nil { var revOnlyMeta revAndFlagsSyncData unmarshalErr := base.JSONUnmarshal(syncXattrData, &revOnlyMeta) if unmarshalErr != nil { - return pkgerrors.WithStack(base.RedactErrorf("Failed to UnmarshalWithXattrs() doc with id: %s (DocUnmarshalRev). Error: %v", base.UD(doc.ID), unmarshalErr)) + return pkgerrors.WithStack(base.RedactErrorf("Failed to UnmarshalWithXattrs() doc with id: %s (DocUnmarshalRevAndFlags). Error: %v", base.UD(doc.ID), unmarshalErr)) } doc.SyncData = SyncData{ RevAndVersion: revOnlyMeta.CurrentRev, From 713a601cd69c7ff5bf1f41759250b8f0e53e3e30 Mon Sep 17 00:00:00 2001 From: Ritesh Kumar Date: Thu, 13 Nov 2025 21:02:23 +0530 Subject: [PATCH 4/4] fixes based on pr comments and added new test cases --- db/crud_test.go | 181 ++++++++++++++++++++++++++++++++++-------------- db/document.go | 2 + 2 files changed, 131 insertions(+), 52 deletions(-) diff --git a/db/crud_test.go b/db/crud_test.go index 341cba0ac5..23236a4b36 100644 --- a/db/crud_test.go +++ b/db/crud_test.go @@ -1810,7 +1810,7 @@ func TestPutExistingCurrentVersion(t *testing.T) { // Update the client's HLV to include the latest SGW version. incomingHLV.PreviousVersions[currentSourceID] = docUpdateVersion - // TODO: because currentRev isn't being updated, storeOldBodyInRevTreeAndUpdateCurrent isn't + // TODO: because expectedCurrentRev isn't being updated, storeOldBodyInRevTreeAndUpdateCurrent isn't // updating the document body. Need to review whether it makes sense to keep using // storeOldBodyInRevTreeAndUpdateCurrent, or if this needs a larger overhaul to support VV doc, cv, _, err = collection.PutExistingCurrentVersion(ctx, opts) @@ -2284,87 +2284,164 @@ func TestSyncDataCVEqual(t *testing.T) { } func TestProposedRev(t *testing.T) { - - base.SetUpTestLogging(t, base.LevelDebug, base.KeyAll) - db, ctx := setupTestDB(t) defer db.Close(ctx) collection, ctx := GetSingleDatabaseCollectionWithUser(ctx, t, db) // create 3 documents + const ( + SingleRevDoc = "SingleRevDoc" + MultiRevDoc = "MultiRevDoc" + TombstonedDoc = "TombstonedDoc" + ) body := Body{"key1": "value1", "key2": 1234} - _, doc1, err := collection.Put(ctx, "doc1", body) + _, doc1, err := collection.Put(ctx, SingleRevDoc, body) require.NoError(t, err) doc1Rev := doc1.GetRevTreeID() - _, doc2, err := collection.Put(ctx, "doc2", body) + _, doc2, err := collection.Put(ctx, MultiRevDoc, body) require.NoError(t, err) doc2Rev1 := doc2.GetRevTreeID() - _, doc2, err = collection.Put(ctx, "doc2", Body{"_rev": doc2Rev1, "key1": "value2", "key2": 5678}) + _, doc2, err = collection.Put(ctx, MultiRevDoc, Body{"_rev": doc2Rev1, "key1": "value2", "key2": 5678}) require.NoError(t, err) doc2Rev2 := doc2.GetRevTreeID() + _, doc2, err = collection.Put(ctx, MultiRevDoc, Body{"_rev": doc2Rev2, "key1": "value3", "key2": 91011}) + require.NoError(t, err) + doc2Rev3 := doc2.GetRevTreeID() - _, doc3, err := collection.Put(ctx, "doc3", body) + _, doc3, err := collection.Put(ctx, TombstonedDoc, body) require.NoError(t, err) doc3Rev1 := doc3.GetRevTreeID() - _, _, err = collection.Put(ctx, "doc3", Body{"_rev": doc3Rev1, "_deleted": true}) + _, _, err = collection.Put(ctx, TombstonedDoc, Body{"_rev": doc3Rev1, "_deleted": true}) require.NoError(t, err) testCases := []struct { - name string - revID string - parentRevID string - expectedStatus ProposedRevStatus - currentRev string - docID string + name string + revID string + parentRevID string + expectedStatus ProposedRevStatus + expectedCurrentRev string + docID string }{ { - name: "no existing document", - revID: "1-abc", - parentRevID: "", - expectedStatus: ProposedRev_OK_IsNew, - currentRev: "", - docID: "doc", + name: "no_existing_document-curr_rev-no_parent", + revID: "1-abc", + parentRevID: "", + expectedStatus: ProposedRev_OK_IsNew, + expectedCurrentRev: "", + docID: "doc", + }, + { + name: "no_existing_document-curr_rev-with_parent", + revID: "2-def", + parentRevID: "1-abc", + expectedStatus: ProposedRev_OK_IsNew, + expectedCurrentRev: "", + docID: "doc", + }, + { + name: "one_rev_doc-curr_rev-without_parent", + revID: doc1Rev, + parentRevID: "", + expectedStatus: ProposedRev_Exists, + expectedCurrentRev: "", + docID: SingleRevDoc, + }, + { + name: "one_rev_doc-incorrect_curr_rev-without_parent", + revID: "1-abc", + parentRevID: "", + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc1Rev, + docID: SingleRevDoc, + }, + { + name: "one_rev_doc-new_curr_rev-without_parent", + revID: "2-abc", + parentRevID: doc1Rev, + expectedStatus: ProposedRev_OK, + expectedCurrentRev: "", + docID: SingleRevDoc, + }, + { + name: "multi_rev_doc-rev1-without_parent", + revID: doc2Rev1, + parentRevID: "", + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc2Rev3, + docID: MultiRevDoc, + }, + { + name: "multi_rev_doc-rev2-with_rev1_parent", + revID: doc2Rev2, + parentRevID: doc2Rev1, + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc2Rev3, + docID: MultiRevDoc, + }, + { + name: "multi_rev_doc-rev2-with_incorrect_parent", + revID: doc2Rev2, + parentRevID: "1-abc", + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc2Rev3, + docID: MultiRevDoc, + }, + { + name: "multi_rev_doc-rev2-without_parent", + revID: doc2Rev2, + parentRevID: "", + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc2Rev3, + docID: MultiRevDoc, + }, + { + name: "multi_rev_doc-conflicting_rev2-with_parent", + revID: "2-abc", + parentRevID: doc2Rev1, + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc2Rev3, + docID: MultiRevDoc, }, { - name: "existing revision with no previous version", - revID: doc1Rev, - parentRevID: "", - expectedStatus: ProposedRev_Exists, - currentRev: "", - docID: "doc1", + name: "multi_rev_doc-conflicting_rev3-without_parent", + revID: doc2Rev3, + parentRevID: "", + expectedStatus: ProposedRev_Exists, + expectedCurrentRev: "", + docID: MultiRevDoc, }, { - name: "existing revision with previous version", - revID: doc2Rev2, - parentRevID: doc2Rev1, - expectedStatus: ProposedRev_Exists, - currentRev: "", - docID: "doc2", + name: "multi_rev_doc-conflicting_rev3-with_parent", + revID: doc2Rev3, + parentRevID: doc2Rev2, + expectedStatus: ProposedRev_Exists, + expectedCurrentRev: "", + docID: MultiRevDoc, }, { - name: "new revision with previous version", - revID: "2-abc", - parentRevID: doc1Rev, - expectedStatus: ProposedRev_OK, - currentRev: "", - docID: "doc1", + name: "multi_rev_doc-conflicting_rev3-with_incorrect_parent", + revID: doc2Rev3, + parentRevID: doc2Rev1, + expectedStatus: ProposedRev_Exists, + expectedCurrentRev: "", + docID: MultiRevDoc, }, { - name: "new revision with previous revision as tombstone", - revID: "1-abc", - parentRevID: "", - expectedStatus: ProposedRev_OK, - currentRev: "", - docID: "doc3", + name: "multi_rev_doc-conflicting_incorrect_rev3-with_parent", + revID: "3-abc", + parentRevID: doc2Rev2, + expectedStatus: ProposedRev_Conflict, + expectedCurrentRev: doc2Rev3, + docID: MultiRevDoc, }, { - name: "conflicting revision with previous version", - revID: "2-abc", - parentRevID: doc2Rev1, - expectedStatus: ProposedRev_Conflict, - currentRev: doc2Rev2, - docID: "doc2", + name: "new revision with previous revision as tombstone", + revID: "1-abc", + parentRevID: "", + expectedStatus: ProposedRev_OK, + expectedCurrentRev: "", + docID: TombstonedDoc, }, } @@ -2372,7 +2449,7 @@ func TestProposedRev(t *testing.T) { t.Run(tc.name, func(t *testing.T) { status, rev := collection.CheckProposedRev(ctx, tc.docID, tc.revID, tc.parentRevID) assert.Equal(t, tc.expectedStatus, status) - assert.Equal(t, tc.currentRev, rev) + assert.Equal(t, tc.expectedCurrentRev, rev) }) } } diff --git a/db/document.go b/db/document.go index 7d7bf495d2..72610a3208 100644 --- a/db/document.go +++ b/db/document.go @@ -168,10 +168,12 @@ func (sd *SyncData) SetCV(hlv *HybridLogicalVector) { sd.RevAndVersion.CurrentVersion = string(base.Uint64CASToLittleEndianHex(hlv.Version)) } +// hasFlag returns true if the document has this bit flag func (sd *SyncData) hasFlag(flag uint8) bool { return sd.Flags&flag != 0 } +// IsDeleted returns true if the document metadata indicates it is a tombstone. func (sd *SyncData) IsDeleted() bool { return sd.hasFlag(channels.Deleted) }