Skip to content

Commit 43bdaa8

Browse files
RIT3shSapataRIT3shSapata
authored
CBG-3689: CheckProposedRev does not unmarshall the entire history (#7875)
1 parent dbc6264 commit 43bdaa8

File tree

3 files changed

+214
-21
lines changed

3 files changed

+214
-21
lines changed

‎db/crud.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3635,10 +3635,7 @@ func (db *DatabaseCollectionWithUser) CheckProposedRev(ctx context.Context, doci
36353635
return ProposedRev_OK, "" // Users can't upload design docs, so ignore them
36363636
}
36373637

3638-
level := DocUnmarshalRev
3639-
if parentRevID == "" {
3640-
level = DocUnmarshalHistory // doc.History only needed in this case (see below)
3641-
}
3638+
level := DocUnmarshalRevAndFlags
36423639
syncData, _, err := db.GetDocSyncDataNoImport(ctx, docid, level)
36433640
if err != nil {
36443641
if !base.IsDocNotFoundError(err) && !base.IsXattrNotFoundError(err) {
@@ -3653,7 +3650,7 @@ func (db *DatabaseCollectionWithUser) CheckProposedRev(ctx context.Context, doci
36533650
} else if syncData.GetRevTreeID() == parentRevID {
36543651
// Proposed rev's parent is my current revision; OK to add:
36553652
return ProposedRev_OK, ""
3656-
} else if parentRevID == "" && syncData.History[syncData.GetRevTreeID()].Deleted {
3653+
} else if parentRevID == "" && syncData.IsDeleted() {
36573654
// Proposed rev has no parent and doc is currently deleted; OK to add:
36583655
return ProposedRev_OK, ""
36593656
} else {

‎db/crud_test.go‎

Lines changed: 172 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1810,7 +1810,7 @@ func TestPutExistingCurrentVersion(t *testing.T) {
18101810

18111811
// Update the client's HLV to include the latest SGW version.
18121812
incomingHLV.PreviousVersions[currentSourceID] = docUpdateVersion
1813-
// TODO: because currentRev isn't being updated, storeOldBodyInRevTreeAndUpdateCurrent isn't
1813+
// TODO: because expectedCurrentRev isn't being updated, storeOldBodyInRevTreeAndUpdateCurrent isn't
18141814
// updating the document body. Need to review whether it makes sense to keep using
18151815
// storeOldBodyInRevTreeAndUpdateCurrent, or if this needs a larger overhaul to support VV
18161816
doc, cv, _, err = collection.PutExistingCurrentVersion(ctx, opts)
@@ -2282,3 +2282,174 @@ func TestSyncDataCVEqual(t *testing.T) {
22822282
})
22832283
}
22842284
}
2285+
2286+
func TestProposedRev(t *testing.T) {
2287+
db, ctx := setupTestDB(t)
2288+
defer db.Close(ctx)
2289+
collection, ctx := GetSingleDatabaseCollectionWithUser(ctx, t, db)
2290+
2291+
// create 3 documents
2292+
const (
2293+
SingleRevDoc = "SingleRevDoc"
2294+
MultiRevDoc = "MultiRevDoc"
2295+
TombstonedDoc = "TombstonedDoc"
2296+
)
2297+
body := Body{"key1": "value1", "key2": 1234}
2298+
_, doc1, err := collection.Put(ctx, SingleRevDoc, body)
2299+
require.NoError(t, err)
2300+
doc1Rev := doc1.GetRevTreeID()
2301+
2302+
_, doc2, err := collection.Put(ctx, MultiRevDoc, body)
2303+
require.NoError(t, err)
2304+
doc2Rev1 := doc2.GetRevTreeID()
2305+
_, doc2, err = collection.Put(ctx, MultiRevDoc, Body{"_rev": doc2Rev1, "key1": "value2", "key2": 5678})
2306+
require.NoError(t, err)
2307+
doc2Rev2 := doc2.GetRevTreeID()
2308+
_, doc2, err = collection.Put(ctx, MultiRevDoc, Body{"_rev": doc2Rev2, "key1": "value3", "key2": 91011})
2309+
require.NoError(t, err)
2310+
doc2Rev3 := doc2.GetRevTreeID()
2311+
2312+
_, doc3, err := collection.Put(ctx, TombstonedDoc, body)
2313+
require.NoError(t, err)
2314+
doc3Rev1 := doc3.GetRevTreeID()
2315+
_, _, err = collection.Put(ctx, TombstonedDoc, Body{"_rev": doc3Rev1, "_deleted": true})
2316+
require.NoError(t, err)
2317+
2318+
testCases := []struct {
2319+
name string
2320+
revID string
2321+
parentRevID string
2322+
expectedStatus ProposedRevStatus
2323+
expectedCurrentRev string
2324+
docID string
2325+
}{
2326+
{
2327+
name: "no_existing_document-curr_rev-no_parent",
2328+
revID: "1-abc",
2329+
parentRevID: "",
2330+
expectedStatus: ProposedRev_OK_IsNew,
2331+
expectedCurrentRev: "",
2332+
docID: "doc",
2333+
},
2334+
{
2335+
name: "no_existing_document-curr_rev-with_parent",
2336+
revID: "2-def",
2337+
parentRevID: "1-abc",
2338+
expectedStatus: ProposedRev_OK_IsNew,
2339+
expectedCurrentRev: "",
2340+
docID: "doc",
2341+
},
2342+
{
2343+
name: "one_rev_doc-curr_rev-without_parent",
2344+
revID: doc1Rev,
2345+
parentRevID: "",
2346+
expectedStatus: ProposedRev_Exists,
2347+
expectedCurrentRev: "",
2348+
docID: SingleRevDoc,
2349+
},
2350+
{
2351+
name: "one_rev_doc-incorrect_curr_rev-without_parent",
2352+
revID: "1-abc",
2353+
parentRevID: "",
2354+
expectedStatus: ProposedRev_Conflict,
2355+
expectedCurrentRev: doc1Rev,
2356+
docID: SingleRevDoc,
2357+
},
2358+
{
2359+
name: "one_rev_doc-new_curr_rev-without_parent",
2360+
revID: "2-abc",
2361+
parentRevID: doc1Rev,
2362+
expectedStatus: ProposedRev_OK,
2363+
expectedCurrentRev: "",
2364+
docID: SingleRevDoc,
2365+
},
2366+
{
2367+
name: "multi_rev_doc-rev1-without_parent",
2368+
revID: doc2Rev1,
2369+
parentRevID: "",
2370+
expectedStatus: ProposedRev_Conflict,
2371+
expectedCurrentRev: doc2Rev3,
2372+
docID: MultiRevDoc,
2373+
},
2374+
{
2375+
name: "multi_rev_doc-rev2-with_rev1_parent",
2376+
revID: doc2Rev2,
2377+
parentRevID: doc2Rev1,
2378+
expectedStatus: ProposedRev_Conflict,
2379+
expectedCurrentRev: doc2Rev3,
2380+
docID: MultiRevDoc,
2381+
},
2382+
{
2383+
name: "multi_rev_doc-rev2-with_incorrect_parent",
2384+
revID: doc2Rev2,
2385+
parentRevID: "1-abc",
2386+
expectedStatus: ProposedRev_Conflict,
2387+
expectedCurrentRev: doc2Rev3,
2388+
docID: MultiRevDoc,
2389+
},
2390+
{
2391+
name: "multi_rev_doc-rev2-without_parent",
2392+
revID: doc2Rev2,
2393+
parentRevID: "",
2394+
expectedStatus: ProposedRev_Conflict,
2395+
expectedCurrentRev: doc2Rev3,
2396+
docID: MultiRevDoc,
2397+
},
2398+
{
2399+
name: "multi_rev_doc-conflicting_rev2-with_parent",
2400+
revID: "2-abc",
2401+
parentRevID: doc2Rev1,
2402+
expectedStatus: ProposedRev_Conflict,
2403+
expectedCurrentRev: doc2Rev3,
2404+
docID: MultiRevDoc,
2405+
},
2406+
{
2407+
name: "multi_rev_doc-conflicting_rev3-without_parent",
2408+
revID: doc2Rev3,
2409+
parentRevID: "",
2410+
expectedStatus: ProposedRev_Exists,
2411+
expectedCurrentRev: "",
2412+
docID: MultiRevDoc,
2413+
},
2414+
{
2415+
name: "multi_rev_doc-conflicting_rev3-with_parent",
2416+
revID: doc2Rev3,
2417+
parentRevID: doc2Rev2,
2418+
expectedStatus: ProposedRev_Exists,
2419+
expectedCurrentRev: "",
2420+
docID: MultiRevDoc,
2421+
},
2422+
{
2423+
name: "multi_rev_doc-conflicting_rev3-with_incorrect_parent",
2424+
revID: doc2Rev3,
2425+
parentRevID: doc2Rev1,
2426+
expectedStatus: ProposedRev_Exists,
2427+
expectedCurrentRev: "",
2428+
docID: MultiRevDoc,
2429+
},
2430+
{
2431+
name: "multi_rev_doc-conflicting_incorrect_rev3-with_parent",
2432+
revID: "3-abc",
2433+
parentRevID: doc2Rev2,
2434+
expectedStatus: ProposedRev_Conflict,
2435+
expectedCurrentRev: doc2Rev3,
2436+
docID: MultiRevDoc,
2437+
},
2438+
{
2439+
name: "new revision with previous revision as tombstone",
2440+
revID: "1-abc",
2441+
parentRevID: "",
2442+
expectedStatus: ProposedRev_OK,
2443+
expectedCurrentRev: "",
2444+
docID: TombstonedDoc,
2445+
},
2446+
}
2447+
2448+
for _, tc := range testCases {
2449+
t.Run(tc.name, func(t *testing.T) {
2450+
status, rev := collection.CheckProposedRev(ctx, tc.docID, tc.revID, tc.parentRevID)
2451+
assert.Equal(t, tc.expectedStatus, status)
2452+
assert.Equal(t, tc.expectedCurrentRev, rev)
2453+
})
2454+
}
2455+
}

‎db/document.go‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,14 @@ const DocumentHistoryMaxEntriesPerChannel = 5
3737
type DocumentUnmarshalLevel uint8
3838

3939
const (
40-
DocUnmarshalAll = DocumentUnmarshalLevel(iota) // Unmarshals metadata and body
41-
DocUnmarshalSync // Unmarshals metadata
42-
DocUnmarshalNoHistory // Unmarshals metadata excluding revtree history
43-
DocUnmarshalHistory // Unmarshals revtree history + rev + CAS only
44-
DocUnmarshalRev // Unmarshals revTreeID + CAS only (no HLV)
45-
DocUnmarshalCAS // Unmarshals CAS (for import check) only
46-
DocUnmarshalNone // No unmarshalling (skips import/upgrade check)
40+
DocUnmarshalAll = DocumentUnmarshalLevel(iota) // Unmarshals metadata and body
41+
DocUnmarshalSync // Unmarshals metadata
42+
DocUnmarshalNoHistory // Unmarshals metadata excluding revtree history
43+
DocUnmarshalHistory // Unmarshals revtree history + rev + CAS only
44+
DocUnmarshalRev // Unmarshals revTreeID + CAS only (no HLV)
45+
DocUnmarshalCAS // Unmarshals CAS (for import check) only
46+
DocUnmarshalNone // No unmarshalling (skips import/upgrade check)
47+
DocUnmarshalRevAndFlags // Unmarshals revTreeID + CAS and Flags (no HLV)
4748
)
4849

4950
const (
@@ -167,6 +168,16 @@ func (sd *SyncData) SetCV(hlv *HybridLogicalVector) {
167168
sd.RevAndVersion.CurrentVersion = string(base.Uint64CASToLittleEndianHex(hlv.Version))
168169
}
169170

171+
// hasFlag returns true if the document has this bit flag
172+
func (sd *SyncData) hasFlag(flag uint8) bool {
173+
return sd.Flags&flag != 0
174+
}
175+
176+
// IsDeleted returns true if the document metadata indicates it is a tombstone.
177+
func (sd *SyncData) IsDeleted() bool {
178+
return sd.hasFlag(channels.Deleted)
179+
}
180+
170181
// RedactRawGlobalSyncData runs HashRedact on the given global sync data.
171182
func RedactRawGlobalSyncData(syncData []byte, redactSalt string) ([]byte, error) {
172183
if redactSalt == "" {
@@ -306,6 +317,11 @@ type revOnlySyncData struct {
306317
CurrentRev channels.RevAndVersion `json:"rev"`
307318
}
308319

320+
type revAndFlagsSyncData struct {
321+
revOnlySyncData
322+
Flags uint8 `json:"flags"`
323+
}
324+
309325
type casOnlySyncData struct {
310326
Cas string `json:"cas"`
311327
}
@@ -331,10 +347,6 @@ func (doc *Document) MarshalBodyAndSync() (retBytes []byte, err error) {
331347
}
332348
}
333349

334-
func (doc *Document) IsDeleted() bool {
335-
return doc.hasFlag(channels.Deleted)
336-
}
337-
338350
func (doc *Document) BodyWithSpecialProperties(ctx context.Context) ([]byte, error) {
339351
bodyBytes, err := doc.BodyBytes(ctx)
340352
if err != nil {
@@ -754,10 +766,6 @@ func (doc *Document) IsSGWrite(ctx context.Context, rawBody []byte) (isSGWrite b
754766
return true, true, false
755767
}
756768

757-
func (doc *Document) hasFlag(flag uint8) bool {
758-
return doc.Flags&flag != 0
759-
}
760-
761769
func (doc *Document) setFlag(flag uint8, state bool) {
762770
if state {
763771
doc.Flags |= flag
@@ -1336,6 +1344,23 @@ func (doc *Document) UnmarshalWithXattrs(ctx context.Context, data, syncXattrDat
13361344
doc.SyncData = SyncData{}
13371345
}
13381346
doc._rawBody = data
1347+
case DocUnmarshalRevAndFlags:
1348+
// Unmarshal rev, cas and flags from sync metadata
1349+
if syncXattrData != nil {
1350+
var revOnlyMeta revAndFlagsSyncData
1351+
unmarshalErr := base.JSONUnmarshal(syncXattrData, &revOnlyMeta)
1352+
if unmarshalErr != nil {
1353+
return pkgerrors.WithStack(base.RedactErrorf("Failed to UnmarshalWithXattrs() doc with id: %s (DocUnmarshalRevAndFlags). Error: %v", base.UD(doc.ID), unmarshalErr))
1354+
}
1355+
doc.SyncData = SyncData{
1356+
RevAndVersion: revOnlyMeta.CurrentRev,
1357+
Cas: revOnlyMeta.Cas,
1358+
Flags: revOnlyMeta.Flags,
1359+
}
1360+
} else {
1361+
doc.SyncData = SyncData{}
1362+
}
1363+
doc._rawBody = data
13391364
}
13401365

13411366
// If there's no body, but there is an xattr, set deleted flag and initialize an empty body

0 commit comments

Comments
 (0)