CBG-4751: Allow CV to be used as OCC value in REST API for document writes (updates/deletes) (#7693)

Author: bbrks

base/util.go

@@ -1825,3 +1825,18 @@ func KeysPresent[K comparable, V any](m map[K]V, keys []K) []K {
 	}
 	return result
 }
+
+// IsRevTreeID checks if the string looks like a RevTree ID.
+func IsRevTreeID(s string) bool {
+	// If we scan forwards past each digit until we hit `-`, we know this is a RevTree ID.
+	for i, r := range s {
+		if r >= '0' && r <= '9' {
+			continue
+		}
+		if r == '-' && i > 0 {
+			return true
+		}
+		break
+	}
+	return false
+}

base/util_test.go

@@ -1854,3 +1854,29 @@ func TestKeysPresent(t *testing.T) {
 		})
 	}
 }
+
+func TestIsRevTreeID(t *testing.T) {
+	tests := []struct {
+		value    string
+		expected bool
+	}{
+		{"1-abc", true},
+		{"1-abc123", true},
+		{"1-abc123def456", true},
+		{"1-abc123def456ghi789", true},
+		{"1-abc123def456ghi789jkl012", true},
+		{"123-abc123", true},
+		{"1234567890-a", true},
+		{"0-a", true},
+		{"1", false},
+		{"abc", false},
+		{"a-abc", false},
+		{"-abc", false},
+		{"1a@bc", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.value, func(t *testing.T) {
+			assert.Equalf(t, tt.expected, IsRevTreeID(tt.value), "IsRevTreeID(%v)", tt.value)
+		})
+	}
+}

db/blip_handler.go

@@ -852,6 +852,7 @@ func (bh *blipHandler) handleProposeChanges(rq *blip.Message) error {
 
 		changeIsVector := false
 		if versionVectorProtocol {
+			// TODO: CBG-4812 Use base.IsRevTreeID
 			changeIsVector = strings.Contains(rev, "@")
 		}
 		if versionVectorProtocol && changeIsVector {
@@ -1094,6 +1095,7 @@ func (bh *blipHandler) processRev(rq *blip.Message, stats *processRevStats) (err
 	var incomingHLV *HybridLogicalVector
 	// Build history/HLV
 	var legacyRevList []string
+	// TODO: CBG-4812 Use base.IsRevTreeID
 	changeIsVector := strings.Contains(rev, "@")
 	if !bh.useHLV() || !changeIsVector {
 		newDoc.RevID = rev

db/changes_test.go

@@ -418,7 +418,7 @@ func TestActiveOnlyCacheUpdate(t *testing.T) {
 	// Tombstone 5 documents
 	for i := 2; i <= 6; i++ {
 		key := fmt.Sprintf("%s_%d", t.Name(), i)
-		_, _, err = collection.DeleteDoc(ctx, key, revId)
+		_, _, err = collection.DeleteDoc(ctx, key, DocVersion{RevTreeID: revId})
 		require.NoError(t, err, "Couldn't delete document")
 	}
 

db/crud.go

@@ -310,13 +310,13 @@ func (db *DatabaseCollectionWithUser) Get1xBody(ctx context.Context, docid strin
 }
 
 // Get Rev with all-or-none history based on specified 'history' flag
-func (db *DatabaseCollectionWithUser) Get1xRevBody(ctx context.Context, docid, revid string, history bool, attachmentsSince []string) (Body, error) {
+func (db *DatabaseCollectionWithUser) Get1xRevBody(ctx context.Context, docid, revOrCV string, history bool, attachmentsSince []string) (Body, error) {
 	maxHistory := 0
 	if history {
 		maxHistory = math.MaxInt32
 	}
 
-	return db.Get1xRevBodyWithHistory(ctx, docid, revid, Get1xRevBodyOptions{
+	return db.Get1xRevBodyWithHistory(ctx, docid, revOrCV, Get1xRevBodyOptions{
 		MaxHistory:       maxHistory,
 		HistoryFrom:      nil,
 		AttachmentsSince: attachmentsSince,
@@ -333,8 +333,8 @@ type Get1xRevBodyOptions struct {
 }
 
 // Retrieves rev with request history specified as collection of revids (historyFrom)
-func (db *DatabaseCollectionWithUser) Get1xRevBodyWithHistory(ctx context.Context, docid, revtreeid string, opts Get1xRevBodyOptions) (Body, error) {
-	rev, err := db.getRev(ctx, docid, revtreeid, opts.MaxHistory, opts.HistoryFrom)
+func (db *DatabaseCollectionWithUser) Get1xRevBodyWithHistory(ctx context.Context, docid, revOrCV string, opts Get1xRevBodyOptions) (Body, error) {
+	rev, err := db.getRev(ctx, docid, revOrCV, opts.MaxHistory, opts.HistoryFrom)
 	if err != nil {
 		return nil, err
 	}
@@ -1093,7 +1093,11 @@ func (db *DatabaseCollectionWithUser) Put(ctx context.Context, docid string, bod
 	generation++
 	delete(body, BodyRev)
 
-	// Not extracting it yet because we need this property around to generate a rev ID
+	// remove CV before RevTreeID generation
+	matchCV, _ := body[BodyCV].(string)
+	delete(body, BodyCV)
+
+	// Not extracting it yet because we need this property around to generate a RevTreeID
 	deleted, _ := body[BodyDeleted].(bool)
 
 	expiry, err := body.ExtractExpiry()
@@ -1146,21 +1150,42 @@ func (db *DatabaseCollectionWithUser) Put(ctx context.Context, docid string, bod
 		}
 
 		var conflictErr error
-		// Make sure matchRev matches an existing leaf revision:
-		if matchRev == "" {
-			matchRev = doc.CurrentRev
-			if matchRev != "" {
-				// PUT with no parent rev given, but there is an existing current revision.
-				// This is OK as long as the current one is deleted.
-				if !doc.History[matchRev].Deleted {
-					conflictErr = base.HTTPErrorf(http.StatusConflict, "Document exists")
-				} else {
-					generation, _ = ParseRevID(ctx, matchRev)
-					generation++
+
+		// OCC check of matchCV against CV on the doc
+		if matchCV != "" {
+			if matchCV == doc.HLV.GetCurrentVersionString() {
+				// set matchRev to the current revision ID and allow existing codepaths to perform RevTree-based update.
+				matchRev = doc.CurrentRev
+				// bump generation based on retrieved RevTree ID
+				generation, _ = ParseRevID(ctx, matchRev)
+				generation++
+			} else if doc.hasFlag(channels.Conflict | channels.Hidden) {
+				// Can't use CV as an OCC Value when a document is in conflict, or we're updating the non-winning leaf
+				// There's no way to get from a given old CV to a RevTreeID to perform the update correctly, since we don't maintain linear history for a given SourceID.
+				// Reject the request and force the user to resolve the conflict using RevTree IDs which does have linear history available.
+				conflictErr = base.HTTPErrorf(http.StatusBadRequest, "Cannot use CV to modify a document in conflict - resolve first with RevTree ID")
+			} else {
+				conflictErr = base.HTTPErrorf(http.StatusConflict, "Document revision conflict")
+			}
+		}
+
+		if conflictErr == nil {
+			// Make sure matchRev matches an existing leaf revision:
+			if matchRev == "" {
+				matchRev = doc.CurrentRev
+				if matchRev != "" {
+					// PUT with no parent rev given, but there is an existing current revision.
+					// This is OK as long as the current one is deleted.
+					if !doc.History[matchRev].Deleted {
+						conflictErr = base.HTTPErrorf(http.StatusConflict, "Document exists")
+					} else {
+						generation, _ = ParseRevID(ctx, matchRev)
+						generation++
+					}
 				}
+			} else if !doc.History.isLeaf(matchRev) || db.IsIllegalConflict(ctx, doc, matchRev, deleted, false, nil) {
+				conflictErr = base.HTTPErrorf(http.StatusConflict, "Document revision conflict")
 			}
-		} else if !doc.History.isLeaf(matchRev) || db.IsIllegalConflict(ctx, doc, matchRev, deleted, false, nil) {
-			conflictErr = base.HTTPErrorf(http.StatusConflict, "Document revision conflict")
 		}
 
 		// Make up a new _rev, and add it to the history:
@@ -2893,7 +2918,8 @@ func (db *DatabaseCollectionWithUser) MarkPrincipalsChanged(ctx context.Context,
 
 // Creates a new document, assigning it a random doc ID.
 func (db *DatabaseCollectionWithUser) Post(ctx context.Context, body Body) (docid string, rev string, doc *Document, err error) {
-	if body[BodyRev] != nil {
+	// This error isn't very accurate, you just _cannot_ use POST to update an existing document - even if it does exist. We don't even bother checking for existence.
+	if body[BodyRev] != nil || body[BodyCV] != nil {
 		return "", "", nil, base.HTTPErrorf(http.StatusNotFound, "No previous revision to replace")
 	}
 
@@ -2914,8 +2940,9 @@ func (db *DatabaseCollectionWithUser) Post(ctx context.Context, body Body) (doci
 }
 
 // Deletes a document, by adding a new revision whose _deleted property is true.
-func (db *DatabaseCollectionWithUser) DeleteDoc(ctx context.Context, docid string, revid string) (string, *Document, error) {
-	body := Body{BodyDeleted: true, BodyRev: revid}
+func (db *DatabaseCollectionWithUser) DeleteDoc(ctx context.Context, docid string, docVersion DocVersion) (string, *Document, error) {
+	versionKey, versionStr := docVersion.Body1xKVPair()
+	body := Body{BodyDeleted: true, versionKey: versionStr}
 	newRevID, doc, err := db.Put(ctx, docid, body)
 	return newRevID, doc, err
 }
@@ -3342,6 +3369,7 @@ func (db *DatabaseCollectionWithUser) CheckProposedVersion(ctx context.Context,
 	// previousRev may be revTreeID or version
 	var previousVersion Version
 	previousRevFormat := "version"
+	// TODO: CBG-4812 Use base.IsRevTreeID
 	if !strings.Contains(previousRev, "@") {
 		previousRevFormat = "revTreeID"
 	}

db/crud_test.go

@@ -1158,7 +1158,7 @@ func TestGet1xRevAndChannels(t *testing.T) {
 	assert.Equal(t, []interface{}{"a"}, revisions[RevisionsIds])
 
 	// Delete the document, creating tombstone revision rev3
-	rev3, _, err := collection.DeleteDoc(ctx, docId, rev2)
+	rev3, _, err := collection.DeleteDoc(ctx, docId, DocVersion{RevTreeID: rev2})
 	require.NoError(t, err)
 	bodyBytes, removed, err = collection.get1xRevFromDoc(ctx, doc2, rev3, true)
 	assert.False(t, removed)

db/database_test.go

@@ -731,7 +731,7 @@ func TestGetDeleted(t *testing.T) {
 	rev1id, _, err := collection.Put(ctx, "doc1", body)
 	assert.NoError(t, err, "Put")
 
-	rev2id, _, err := collection.DeleteDoc(ctx, "doc1", rev1id)
+	rev2id, _, err := collection.DeleteDoc(ctx, "doc1", DocVersion{RevTreeID: rev1id})
 	assert.NoError(t, err, "DeleteDoc")
 
 	// Get the deleted doc with its history; equivalent to GET with ?revs=true
@@ -1395,7 +1395,7 @@ func TestAllDocsOnly(t *testing.T) {
 	}
 
 	// Now delete one document and try again:
-	_, _, err = collection.DeleteDoc(ctx, ids[23].DocID, ids[23].RevID)
+	_, _, err = collection.DeleteDoc(ctx, ids[23].DocID, DocVersion{RevTreeID: ids[23].RevID})
 	assert.NoError(t, err, "Couldn't delete doc 23")
 
 	alldocs, err = allDocIDs(ctx, collection.DatabaseCollection)
@@ -1726,7 +1726,7 @@ func TestConflicts(t *testing.T) {
 	)
 
 	// Delete 2-b; verify this makes 2-a current:
-	rev3, _, err := collection.DeleteDoc(ctx, "doc", "2-b")
+	rev3, _, err := collection.DeleteDoc(ctx, "doc", DocVersion{RevTreeID: "2-b"})
 	assert.NoError(t, err, "delete 2-b")
 
 	rawBody, _, _ = collection.dataStore.GetRaw("doc")
@@ -3369,7 +3369,7 @@ func TestTombstoneCompactionStopWithManager(t *testing.T) {
 		docID := fmt.Sprintf("doc%d", i)
 		rev, _, err := collection.Put(ctx, docID, Body{})
 		assert.NoError(t, err)
-		_, _, err = collection.DeleteDoc(ctx, docID, rev)
+		_, _, err = collection.DeleteDoc(ctx, docID, DocVersion{RevTreeID: rev})
 		assert.NoError(t, err)
 	}
 
@@ -3780,7 +3780,7 @@ func TestImportCompactPanic(t *testing.T) {
 	// Create a document, then delete it, to create a tombstone
 	rev, doc, err := collection.Put(ctx, "test", Body{})
 	require.NoError(t, err)
-	_, _, err = collection.DeleteDoc(ctx, doc.ID, rev)
+	_, _, err = collection.DeleteDoc(ctx, doc.ID, DocVersion{RevTreeID: rev})
 	require.NoError(t, err)
 	require.NoError(t, collection.WaitForPendingChanges(ctx))
 

db/document.go

@@ -1484,3 +1484,38 @@ func (doc *Document) ExtractDocVersion() DocVersion {
 		CV:        *doc.HLV.ExtractCurrentVersionFromHLV(),
 	}
 }
+
+// DocVersion represents a specific version of a document in an revID/HLV agnostic manner.
+// Users should access the properties via the CV and RevTreeID methods, to ensure there's a check that the specific version type is not empty.
+type DocVersion struct {
+	RevTreeID string
+	CV        Version
+}
+
+// String implements fmt.Stringer
+func (v DocVersion) String() string {
+	return fmt.Sprintf("RevTreeID:%s,CV:%#v", v.RevTreeID, v.CV)
+}
+
+// GoString implements fmt.GoStringer
+func (v DocVersion) GoString() string {
+	return fmt.Sprintf("DocVersion{RevTreeID:%s,CV:%#v}", v.RevTreeID, v.CV)
+}
+
+// GetRevTreeID returns the Revision Tree ID of the document version, and a bool indicating whether it is present.
+func (v DocVersion) GetRevTreeID() (string, bool) {
+	return v.RevTreeID, v.RevTreeID != ""
+}
+
+// GetCV returns the Current Version of the document, and a bool indicating whether it is present.
+func (v DocVersion) GetCV() (Version, bool) {
+	return v.CV, !v.CV.IsEmpty()
+}
+
+// Body1xKVPair returns the key and value to use in a 1.x-style document body for the given DocVersion.
+func (d DocVersion) Body1xKVPair() (bodyVersionKey, bodyVersionStr string) {
+	if cv, ok := d.GetCV(); ok {
+		return BodyCV, cv.String()
+	}
+	return BodyRev, d.RevTreeID
+}

db/utilities_hlv_testing.go

@@ -12,7 +12,6 @@ package db
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 	"strings"
 	"testing"
@@ -22,59 +21,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// DocVersion represents a specific version of a document in an revID/HLV agnostic manner.
-type DocVersion struct {
-	RevTreeID string
-	CV        Version
-}
-
-func (v DocVersion) String() string {
-	return fmt.Sprintf("RevTreeID:%s,CV:%#v", v.RevTreeID, v.CV)
-}
-
-func (v DocVersion) GoString() string {
-	return fmt.Sprintf("DocVersion{RevTreeID:%s,CV:%#v}", v.RevTreeID, v.CV)
-}
-
-func (v DocVersion) DocVersionRevTreeEqual(o DocVersion) bool {
-	if v.RevTreeID != o.RevTreeID {
-		return false
-	}
-	return true
-}
-
-func (v DocVersion) GetRev(useHLV bool) string {
-	if useHLV {
-		if v.CV.SourceID == "" {
-			return ""
-		}
-		return v.CV.String()
-	} else {
-		return v.RevTreeID
-	}
-}
-
-// RevIDGeneration returns the Rev ID generation for the current version
-func (v *DocVersion) RevIDGeneration() int {
-	if v == nil {
-		return 0
-	}
-	gen, err := strconv.ParseInt(strings.Split(v.RevTreeID, "-")[0], 10, 64)
-	if err != nil {
-		base.AssertfCtx(context.TODO(), "Error parsing generation from rev ID %q: %v", v.RevTreeID, err)
-		return 0
-	}
-	return int(gen)
-}
-
-// RevIDDigest returns the Rev ID digest for the current version
-func (v *DocVersion) RevIDDigest() string {
-	if v == nil {
-		return ""
-	}
-	return strings.Split(v.RevTreeID, "-")[1]
-}
-
 // HLVAgent performs HLV updates directly (not via SG) for simulating/testing interaction with non-SG HLV agents
 type HLVAgent struct {
 	t         *testing.T

docs/api/components/parameters.yaml

@@ -13,13 +13,13 @@ DB-config-If-Match:
   schema:
     type: string
   description: 'If set to a configuration''s Etag value, enables optimistic concurrency control for the request. Returns HTTP 412 if another update happened underneath this one.'
-If-Match:
+Document-If-Match:
   name: If-Match
   in: header
   required: false
   schema:
     type: string
-  description: The revision ID to target.
+  description: An optimistic concurrency control (OCC) value used to prevent conflicts. Use the value returned in the ETag response header of the GET request for the resource being updated, or the latest known Revision Tree ID or Current Version of the document.
 Include-channels:
   name: channels
   in: query
@@ -342,7 +342,7 @@ rev:
   schema:
     type: string
   example: 2-5145e1086bb8d1d71a531e9f6b543c58
-  description: The document revision to target.
+  description: The document revision to target. If this is a CV value, ensure the query parameter is URL encoded (`+`->`%2B`, `@`->`%40`, etc.)
 revs_from:
   name: revs_from
   in: query
@@ -393,7 +393,7 @@ show_cv:
   required: false
   schema:
     type: boolean
-  description: Output the current version of the version vector in the response as property `_cv`.
+  description: Output the Current Version in the response as property `_cv`.
 startkey:
   name: startkey
   in: query