CBG-4750: Remove show_cv parameter and always return _cv in documents on REST API (#7654)

bbrks · Copilot · web-flow · commit a3a19afced71 · 2025-07-30T16:28:48.000+01:00
Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;
diff --git a/db/revision.go b/db/revision.go
@@ -27,6 +27,7 @@ type Body map[string]interface{}
 const (
 	BodyDeleted        = "_deleted"
 	BodyRev            = "_rev"
+	BodyCV             = "_cv"
 	BodyId             = "_id"
 	BodyRevisions      = "_revisions"
 	BodyAttachments    = "_attachments"
diff --git a/db/revision_cache_interface.go b/db/revision_cache_interface.go
@@ -301,7 +301,7 @@ func (rev *DocumentRevision) Mutable1xBody(ctx context.Context, db *DatabaseColl
 	}
 
 	if showCV && rev.CV != nil {
-		b["_cv"] = rev.CV.String()
+		b[BodyCV] = rev.CV.String()
 	}
 
 	if rev.Deleted {
diff --git a/docs/api/paths/admin/keyspace-docid.yaml b/docs/api/paths/admin/keyspace-docid.yaml
@@ -21,7 +21,6 @@ get:
     - $ref: ../../components/parameters.yaml#/rev
     - $ref: ../../components/parameters.yaml#/open_revs
     - $ref: ../../components/parameters.yaml#/show_exp
-    - $ref: ../../components/parameters.yaml#/show_cv
     - $ref: ../../components/parameters.yaml#/revs_from
     - $ref: ../../components/parameters.yaml#/atts_since
     - $ref: ../../components/parameters.yaml#/revs_limit
diff --git a/docs/api/paths/public/keyspace-docid.yaml b/docs/api/paths/public/keyspace-docid.yaml
@@ -15,7 +15,6 @@ get:
     - $ref: ../../components/parameters.yaml#/rev
     - $ref: ../../components/parameters.yaml#/open_revs
     - $ref: ../../components/parameters.yaml#/show_exp
-    - $ref: ../../components/parameters.yaml#/show_cv
     - $ref: ../../components/parameters.yaml#/revs_from
     - $ref: ../../components/parameters.yaml#/atts_since
     - $ref: ../../components/parameters.yaml#/revs_limit
diff --git a/rest/api_collections_test.go b/rest/api_collections_test.go
@@ -184,25 +184,32 @@ func TestNoCollectionsPutDocWithKeyspace(t *testing.T) {
 	})
 	defer rt.Close()
 
+	const docBody = `{"foo":"bar"}`
+
 	// can't put doc into invalid keyspaces
-	response := rt.SendAdminRequest("PUT", "/db.invalidScope.invalidCollection/doc1", "{}")
+	response := rt.SendAdminRequest(http.MethodPut, "/db.invalidScope.invalidCollection/doc1", docBody)
 	RequireStatus(t, response, http.StatusNotFound)
 
-	response = rt.SendAdminRequest("PUT", "/db.invalidCollection/doc1", "{}")
+	response = rt.SendAdminRequest(http.MethodPut, "/db.invalidCollection/doc1", docBody)
 	RequireStatus(t, response, http.StatusNotFound)
 
 	// can put doc into _default scope/collection explicitly ... or implicitly (tested elsewhere e.g: TestPutEmptyDoc)
-	response = rt.SendAdminRequest("PUT", "/db._default._default/doc1", "{}")
+	response = rt.SendAdminRequest(http.MethodPut, "/db._default._default/doc1", docBody)
 	RequireStatus(t, response, http.StatusCreated)
 
-	// retrieve doc in both ways (_default._default and no fully-qualified keyspace)
-	response = rt.SendAdminRequest("GET", "/db._default._default/doc1", "")
-	RequireStatus(t, response, http.StatusOK)
-	assert.Equal(t, `{"_id":"doc1","_rev":"1-ca9ad22802b66f662ff171f226211d5c"}`, string(response.BodyBytes()))
-
-	response = rt.SendAdminRequest("GET", "/db/doc1", "")
-	RequireStatus(t, response, http.StatusOK)
-	assert.Equal(t, `{"_id":"doc1","_rev":"1-ca9ad22802b66f662ff171f226211d5c"}`, string(response.BodyBytes()))
+	version, _ := rt.GetDoc("doc1")
+
+	// retrieve doc in both ways (explicit and implicit _default scope/collection)
+	for _, keyspace := range []string{
+		"db._default._default", // fully qualified
+		"db",                   // implicit
+	} {
+		body := rt.GetDocBodyFromKeyspace(keyspace, "doc1")
+		assert.Equal(t, "bar", body["foo"])
+		assert.Equal(t, "doc1", body["_id"])
+		assert.Equal(t, version.RevTreeID, body["_rev"])
+		assert.Equal(t, version.CV.String(), body["_cv"])
+	}
 }
 
 func TestSingleCollectionDCP(t *testing.T) {
diff --git a/rest/api_test.go b/rest/api_test.go
@@ -2349,19 +2349,22 @@ func TestPutEmptyDoc(t *testing.T) {
 	rt := NewRestTester(t, nil)
 	defer rt.Close()
 
-	response := rt.SendAdminRequest("PUT", "/{{.keyspace}}/doc", "{}")
+	const docID = "doc"
+
+	response := rt.SendAdminRequest("PUT", "/{{.keyspace}}/"+docID, "{}")
 	RequireStatus(t, response, http.StatusCreated)
 
-	response = rt.SendAdminRequest("GET", "/{{.keyspace}}/doc", "")
-	RequireStatus(t, response, http.StatusOK)
-	assert.Equal(t, `{"_id":"doc","_rev":"1-ca9ad22802b66f662ff171f226211d5c"}`, string(response.BodyBytes()))
+	body := rt.GetDocBody(docID)
+	assert.NotEmpty(t, body[db.BodyId])
+	assert.NotEmpty(t, body[db.BodyRev])
 
-	response = rt.SendAdminRequest("PUT", "/{{.keyspace}}/doc?rev=1-ca9ad22802b66f662ff171f226211d5c", `{"val": "newval"}`)
+	response = rt.SendAdminRequest("PUT", "/{{.keyspace}}/"+docID+"?rev="+body[db.BodyRev].(string), `{"val": "newval"}`)
 	RequireStatus(t, response, http.StatusCreated)
 
-	response = rt.SendAdminRequest("GET", "/{{.keyspace}}/doc", "")
-	RequireStatus(t, response, http.StatusOK)
-	assert.Equal(t, `{"_id":"doc","_rev":"2-2f981cadffde70e8a1d9dc386a410e0d","val":"newval"}`, string(response.BodyBytes()))
+	body = rt.GetDocBody(docID)
+	assert.NotEmpty(t, body[db.BodyId])
+	assert.NotEmpty(t, body[db.BodyRev])
+	assert.Equal(t, "newval", body["val"].(string))
 }
 
 func TestTombstonedBulkDocs(t *testing.T) {
diff --git a/rest/attachment_test.go b/rest/attachment_test.go
@@ -400,8 +400,8 @@ func TestManualAttachmentNewDoc(t *testing.T) {
 	response = rt.SendAdminRequest("GET", "/{{.keyspace}}/notexistyet", "")
 	RequireStatus(t, response, 200)
 	require.NoError(t, base.JSONUnmarshal(response.Body.Bytes(), &body))
-	// body should only have 3 top-level entries _id, _rev, _attachments
-	base.RequireKeysEqual(t, []string{"_id", "_rev", "_attachments"}, body)
+	// body should only have metadata entries - no actual document body contents
+	base.RequireKeysEqual(t, []string{db.BodyCV, db.BodyId, db.BodyRev, db.BodyAttachments}, body)
 	require.Equal(t, db.AttachmentMap{
 		"attach1": {
 			ContentType: "text/plain",
@@ -2978,6 +2978,7 @@ func TestBlipPushRevWithAttachment(t *testing.T) {
 			},
 			"_id":  docID,
 			"_rev": rtVersion.RevTreeID,
+			"_cv":  rtVersion.CV.String(),
 		}, body)
 
 		response := rt.SendAdminRequest(http.MethodGet, fmt.Sprintf("/{{.keyspace}}/%s/%s", docID, attachmentName), "")
diff --git a/rest/blip_api_delta_sync_test.go b/rest/blip_api_delta_sync_test.go
@@ -356,7 +356,8 @@ func TestBlipDeltaSyncNewAttachmentPull(t *testing.T) {
 		assert.Equal(t, doc1ID, respBody[db.BodyId])
 		require.Equal(t, db.Body{
 			"_id":  doc1ID,
-			"_rev": "2-10000d5ec533b29b117e60274b1e3653",
+			"_rev": version2.RevTreeID,
+			"_cv":  version2.CV.String(),
 			"greetings": []any{
 				map[string]any{"hello": "world!"},
 				map[string]any{"hi": "alice"},
diff --git a/rest/bootstrap_test.go b/rest/bootstrap_test.go
@@ -72,10 +72,13 @@ func TestBootstrapRESTAPISetup(t *testing.T) {
 	require.Equal(t, uint32(1234), *dbConfigResp.CacheConfig.RevCacheConfig.MaxItemCount)
 
 	// Sanity check to use the database
+	resp = BootstrapAdminRequest(t, sc, http.MethodGet, "/db1/doc1", ``)
+	resp.RequireStatus(http.StatusNotFound)
 	resp = BootstrapAdminRequest(t, sc, http.MethodPut, "/db1/doc1", `{"foo":"bar"}`)
 	resp.RequireResponse(http.StatusCreated, `{"id":"doc1","ok":true,"rev":"1-cd809becc169215072fd567eebd8b8de"}`)
 	resp = BootstrapAdminRequest(t, sc, http.MethodGet, "/db1/doc1", ``)
-	resp.RequireResponse(http.StatusOK, `{"_id":"doc1","_rev":"1-cd809becc169215072fd567eebd8b8de","foo":"bar"}`)
+	resp.RequireStatus(http.StatusOK)
+	assert.Contains(t, resp.Body, `"foo":"bar"`)
 
 	// Restart Sync Gateway
 	closeFn()
@@ -107,7 +110,8 @@ func TestBootstrapRESTAPISetup(t *testing.T) {
 
 	// Ensure it's _actually_ the same bucket
 	resp = BootstrapAdminRequest(t, sc, http.MethodGet, "/db1/doc1", ``)
-	resp.RequireResponse(http.StatusOK, `{"_id":"doc1","_rev":"1-cd809becc169215072fd567eebd8b8de","foo":"bar"}`)
+	resp.RequireStatus(http.StatusOK)
+	assert.Contains(t, resp.Body, `"foo":"bar"`)
 }
 
 // TestBootstrapDuplicateBucket will attempt to create two databases sharing the same collections and ensure this isn't allowed.
diff --git a/rest/bulk_api.go b/rest/bulk_api.go
@@ -479,7 +479,8 @@ func (h *handler) handleBulkGet() error {
 					HistoryFrom:      revsFrom,
 					AttachmentsSince: attsSince,
 					ShowExp:          showExp,
-					ShowCV:           showCV})
+					ShowCV:           showCV,
+				})
 			}
 
 			if err != nil {
diff --git a/rest/doc_api.go b/rest/doc_api.go
@@ -30,7 +30,7 @@ func (h *handler) handleGetDoc() error {
 	revid := h.getQuery("rev")
 	openRevs := h.getQuery("open_revs")
 	showExp := h.getBoolQuery("show_exp")
-	showCV := h.getBoolQuery("show_cv")
+	const showCV = true // Post-beta 4.0 _always_ returns CV - negligible impact to revtree-only clients and promotes CV as the preferred OCC value
 
 	if replicator2, _ := h.getOptBoolQuery("replicator2", false); replicator2 {
 		return h.handleGetDocReplicator2(docid, revid)
@@ -74,7 +74,8 @@ func (h *handler) handleGetDoc() error {
 			HistoryFrom:      revsFrom,
 			AttachmentsSince: attachmentsSince,
 			ShowExp:          showExp,
-			ShowCV:           showCV})
+			ShowCV:           showCV,
+		})
 		if err != nil {
 			if err == base.ErrImportCancelledPurged {
 				base.DebugfCtx(h.ctx(), base.KeyImport, fmt.Sprintf("Import cancelled as document %v is purged", base.UD(docid)))
@@ -141,7 +142,8 @@ func (h *handler) handleGetDoc() error {
 						HistoryFrom:      revsFrom,
 						AttachmentsSince: attachmentsSince,
 						ShowExp:          showExp,
-						ShowCV:           showCV})
+						ShowCV:           showCV,
+					})
 					if err != nil {
 						revBody = db.Body{"missing": revid} // TODO: More specific error
 					}
@@ -168,7 +170,8 @@ func (h *handler) handleGetDoc() error {
 					HistoryFrom:      revsFrom,
 					AttachmentsSince: attachmentsSince,
 					ShowExp:          showExp,
-					ShowCV:           showCV})
+					ShowCV:           showCV,
+				})
 				if err != nil {
 					revBody = db.Body{"missing": revid} // TODO: More specific error
 				} else {
diff --git a/rest/doc_api_test.go b/rest/doc_api_test.go
@@ -145,32 +145,32 @@ func TestGuestReadOnly(t *testing.T) {
 		},
 		}},
 	)
-
 	defer rt.Close()
 
-	rt.GetDatabase()
+	const docID = "doc"
+
 	// Write a document as admin
-	response := rt.SendAdminRequest("PUT", "/{{.keyspace}}/doc", "{}")
+	response := rt.SendAdminRequest(http.MethodPut, "/{{.keyspace}}/"+docID, `{"val": "test"}`)
 	RequireStatus(t, response, http.StatusCreated)
 
 	// Attempt to read as guest
-	response = rt.SendRequest("GET", "/{{.keyspace}}/doc", "")
-	RequireStatus(t, response, http.StatusOK)
-	assert.Equal(t, `{"_id":"doc","_rev":"1-ca9ad22802b66f662ff171f226211d5c"}`, string(response.BodyBytes()))
+	body := rt.GetDocBody(docID)
+	assert.Equal(t, "test", body["val"].(string))
+	rev := body["_rev"].(string)
 
 	// Attempt to write as guest
-	response = rt.SendRequest("PUT", "/{{.keyspace}}/doc?rev=1-ca9ad22802b66f662ff171f226211d5c", `{"val": "newval"}`)
+	response = rt.SendRequest(http.MethodPut, "/{{.keyspace}}/doc?rev="+rev, `{"val": "newval"}`)
 	RequireStatus(t, response, http.StatusForbidden)
 
 	// Attempt to access _blipsync as guest - blip sync handling for read-only GUEST is applied at replication level (to allow pull-only replications).
 	// Should succeed permission check, and only fail on websocket upgrade
-	response = rt.SendRequest("GET", "/{{.db}}/_blipsync", "")
+	response = rt.SendRequest(http.MethodGet, "/{{.db}}/_blipsync", "")
 	RequireStatus(t, response, http.StatusUpgradeRequired)
 
 	// Verify matching on _blipsync path doesn't incorrectly match docs, attachments
-	response = rt.SendRequest("PUT", "/{{.keyspace}}/doc_named_blipsync", "")
+	response = rt.SendRequest(http.MethodPut, "/{{.keyspace}}/doc_named_blipsync", "")
 	RequireStatus(t, response, http.StatusForbidden)
-	response = rt.SendRequest("PUT", "/{{.keyspace}}/doc/_blipsync", "")
+	response = rt.SendRequest(http.MethodPut, "/{{.keyspace}}/doc/_blipsync", "")
 	RequireStatus(t, response, http.StatusForbidden)
 
 }
@@ -188,24 +188,14 @@ func TestGetDocWithCV(t *testing.T) {
 		headers   map[string]string
 		multipart bool
 	}{
-		{
-			name:   "get doc",
-			url:    "/{{.keyspace}}/doc1",
-			output: fmt.Sprintf(`{"_id":"%s","_rev":"%s","foo":"bar"}`, docID, docVersion.RevTreeID),
-		},
-		{
-			name:   "get doc with rev",
-			url:    fmt.Sprintf("/{{.keyspace}}/doc1?rev=%s", docVersion.RevTreeID),
-			output: fmt.Sprintf(`{"_id":"%s","_rev":"%s","foo":"bar"}`, docID, docVersion.RevTreeID),
-		},
 		{
 			name:   "get doc with cv",
-			url:    "/{{.keyspace}}/doc1?show_cv=true",
+			url:    "/{{.keyspace}}/doc1",
 			output: fmt.Sprintf(`{"_id":"%s","_rev":"%s","_cv":"%s","foo":"bar"}`, docID, docVersion.RevTreeID, docVersion.CV),
 		},
 		{
 			name:   "get doc with open_revs=all and cv no multipart",
-			url:    "/{{.keyspace}}/doc1?open_revs=all&show_cv=true",
+			url:    "/{{.keyspace}}/doc1?open_revs=all",
 			output: fmt.Sprintf(`[{"ok": {"_id":"%s","_rev":"%s","_cv":"%s","foo":"bar"}}]`, docID, docVersion.RevTreeID, docVersion.CV),
 			headers: map[string]string{
 				"Accept": "application/json",
@@ -214,7 +204,7 @@ func TestGetDocWithCV(t *testing.T) {
 
 		{
 			name:      "get doc with open_revs=all and cv",
-			url:       "/{{.keyspace}}/doc1?open_revs=all&show_cv=true",
+			url:       "/{{.keyspace}}/doc1?open_revs=all",
 			output:    fmt.Sprintf(`{"_id":"%s","_rev":"%s","_cv":"%s","foo":"bar"}`, docID, docVersion.RevTreeID, docVersion.CV),
 			multipart: true,
 		},
diff --git a/rest/utilities_testing_resttester.go b/rest/utilities_testing_resttester.go
@@ -46,24 +46,28 @@ func (rt *RestTester) UpdateTB(t *testing.T) {
 
 // GetDocBody returns the doc body for the given docID. If the document is not found, t.Fail will be called.
 func (rt *RestTester) GetDocBody(docID string) db.Body {
-	rawResponse := rt.SendAdminRequest("GET", "/{{.keyspace}}/"+docID, "")
-	RequireStatus(rt.TB(), rawResponse, 200)
+	return rt.GetDocBodyFromKeyspace("{{.keyspace}}", docID)
+}
+
+// GetDocBodyFromKeyspace returns the doc body for the given docID in the specified keyspace. If the document is not found, t.Fail will be called.
+func (rt *RestTester) GetDocBodyFromKeyspace(keyspace, docID string) db.Body {
+	rawResponse := rt.SendAdminRequest("GET", "/"+keyspace+"/"+docID, "")
+	RequireStatus(rt.TB(), rawResponse, http.StatusOK)
 	var body db.Body
 	require.NoError(rt.TB(), base.JSONUnmarshal(rawResponse.Body.Bytes(), &body))
+	require.NotNil(rt.TB(), body)
 	return body
 }
 
 // GetDoc returns the doc body and version for the given docID. If the document is not found, t.Fail will be called.
 func (rt *RestTester) GetDoc(docID string) (DocVersion, db.Body) {
 	rawResponse := rt.SendAdminRequest("GET", "/{{.keyspace}}/"+docID, "")
-	RequireStatus(rt.TB(), rawResponse, 200)
+	RequireStatus(rt.TB(), rawResponse, http.StatusOK)
 	var body db.Body
 	require.NoError(rt.TB(), base.JSONUnmarshal(rawResponse.Body.Bytes(), &body))
-	var r struct {
-		RevID *string `json:"_rev"`
-	}
-	require.NoError(rt.TB(), base.JSONUnmarshal(rawResponse.Body.Bytes(), &r))
-	return DocVersion{RevTreeID: *r.RevID}, body
+	version, err := db.ParseVersion(body[db.BodyCV].(string))
+	require.NoError(rt.TB(), err)
+	return DocVersion{RevTreeID: body[db.BodyRev].(string), CV: version}, body
 }
 
 // TriggerOnDemandImport will use the REST API to trigger on an demand import via GET. This function intentionally does not check error codes in case the document does not exist or is invalid to be imported.
@@ -89,7 +93,7 @@ func (rt *RestTester) CreateTestDoc(docid string) DocVersion {
 
 // PutDoc will upsert the document with a given contents.
 func (rt *RestTester) PutDoc(docID string, body string) DocVersion {
-	rawResponse := rt.SendAdminRequest("PUT", fmt.Sprintf("/%s/%s?show_cv=true", rt.GetSingleKeyspace(), docID), body)
+	rawResponse := rt.SendAdminRequest("PUT", fmt.Sprintf("/%s/%s", rt.GetSingleKeyspace(), docID), body)
 	RequireStatus(rt.TB(), rawResponse, 201)
 	return DocVersionFromPutResponse(rt.TB(), rawResponse)
 }
@@ -135,7 +139,7 @@ func (rt *RestTester) WaitForVersion(docID string, version DocVersion) {
 		require.NotEqual(rt.TB(), "", version.CV.String(), "Expected CV if RevTreeID in WaitForVersion")
 	}
 	require.EventuallyWithT(rt.TB(), func(c *assert.CollectT) {
-		rawResponse := rt.SendAdminRequest("GET", "/{{.keyspace}}/"+docID+"?show_cv=true", "")
+		rawResponse := rt.SendAdminRequest("GET", "/{{.keyspace}}/"+docID, "")
 		if !assert.Contains(c, []int{200, 201}, rawResponse.Code, "Unexpected status code for %s", rawResponse.Body.String()) {
 			return
 		}
@@ -145,7 +149,7 @@ func (rt *RestTester) WaitForVersion(docID string, version DocVersion) {
 			assert.Equal(c, version.RevTreeID, body.ExtractRev())
 		}
 		if !version.CV.IsEmpty() {
-			assert.Equal(c, version.CV.String(), body["_cv"].(string))
+			assert.Equal(c, version.CV.String(), body[db.BodyCV].(string))
 		}
 	}, 10*time.Second, 50*time.Millisecond)
 }

Original file line number	Diff line number	Diff line change
`@@ -301,7 +301,7 @@ func (rev DocumentRevision) Mutable1xBody(ctx context.Context, db DatabaseColl`
`301`	`301`	`}`
`302`	`302`
`303`	`303`	`if showCV && rev.CV != nil {`
`304`		`- b["_cv"] = rev.CV.String()`
	`304`	`+ b[BodyCV] = rev.CV.String()`
`305`	`305`	`}`
`306`	`306`
`307`	`307`	`if rev.Deleted {`
Original file line number	Diff line number	Diff line change
`@@ -479,7 +479,8 @@ func (h *handler) handleBulkGet() error {`
`479`	`479`	`HistoryFrom: revsFrom,`
`480`	`480`	`AttachmentsSince: attsSince,`
`481`	`481`	`ShowExp: showExp,`
`482`		`- ShowCV: showCV})`
	`482`	`+ ShowCV: showCV,`
	`483`	`+ })`
`483`	`484`	`}`
`484`	`485`
`485`	`486`	`if err != nil {`