CBG-4400 re-enable skipped attachment tests (#7389)

db/database.go

@@ -929,6 +929,11 @@ func (db *Database) ReloadUser(ctx context.Context) error {
 	}
 }
 
+// NextSequence returns the new sequence number.
+func (db *DatabaseContext) NextSequence(ctx context.Context) (uint64, error) {
+	return db.sequences.nextSequence(ctx)
+}
+
 // ////// ALL DOCUMENTS:
 
 type IDRevAndSequence struct {

rest/attachment_test.go

@@ -1900,10 +1900,8 @@ func TestBasicAttachmentRemoval(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
-
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID, rawDoc, attKey, attBody)
+		CreateDocWithLegacyAttachment(t, rt, docID, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Get the document and grab the revID.
 		version, _ := rt.GetDoc(docID)
@@ -1927,13 +1925,11 @@ func TestBasicAttachmentRemoval(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID1, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
-
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID1, rawDoc, attKey, attBody)
+		CreateDocWithLegacyAttachment(t, rt, docID1, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Create another document referencing the same legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID2, rawDoc, attKey, attBody)
+		CreateDocWithLegacyAttachment(t, rt, docID2, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Get revID of the first document.
 		version, _ := rt.GetDoc(docID1)
@@ -1966,10 +1962,8 @@ func TestBasicAttachmentRemoval(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
-
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID, rawDoc, attKey, attBody)
+		CreateDocWithLegacyAttachment(t, rt, docID, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Get the document and grab the revID.
 		version, _ := rt.GetDoc(docID)
@@ -1993,27 +1987,21 @@ func TestBasicAttachmentRemoval(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID1, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
 
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID1, rawDoc, attKey, attBody)
+		doc1Version1 := CreateDocWithLegacyAttachment(t, rt, docID1, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Create another document referencing the same legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID2, rawDoc, attKey, attBody)
-
-		version, _ := rt.GetDoc(docID1)
+		doc2Version1 := CreateDocWithLegacyAttachment(t, rt, docID2, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Remove attachment from the first document via document update.
-		_ = rt.UpdateDoc(docID1, version, `{"prop":true}`)
+		_ = rt.UpdateDoc(docID1, doc1Version1, `{"prop":true}`)
 
 		// Check whether legacy attachment is still persisted in the bucket.
 		requireAttachmentFound(attKey, attBody)
 
-		// Get revID of the second document.
-		version, _ = rt.GetDoc(docID2)
-
 		// Remove attachment from the second document via document update.
-		_ = rt.UpdateDoc(docID2, version, `{"prop":true}`)
+		_ = rt.UpdateDoc(docID2, doc2Version1, `{"prop":true}`)
 
 		// Check whether legacy attachment is still persisted in the bucket.
 		requireAttachmentFound(attKey, attBody)
@@ -2031,13 +2019,8 @@ func TestBasicAttachmentRemoval(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
-
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID, rawDoc, attKey, attBody)
-
-		// Get the document.
-		_, _ = rt.GetDoc(docID)
+		CreateDocWithLegacyAttachment(t, rt, docID, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Purge the entire document.
 		rt.PurgeDoc(docID)
@@ -2055,13 +2038,11 @@ func TestBasicAttachmentRemoval(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID1, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
-
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID1, rawDoc, attKey, attBody)
+		CreateDocWithLegacyAttachment(t, rt, docID1, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Create another document referencing the same legacy attachment.
-		CreateDocWithLegacyAttachment(t, rt, docID2, rawDoc, attKey, attBody)
+		CreateDocWithLegacyAttachment(t, rt, docID2, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Get the first document.
 		_, _ = rt.GetDoc(docID1)
@@ -2673,7 +2654,8 @@ func TestCBLRevposHandling(t *testing.T) {
 }
 
 // Helper_Functions
-func CreateDocWithLegacyAttachment(t *testing.T, rt *RestTester, docID string, rawDoc []byte, attKey string, attBody []byte) {
+// CreateDocWithLegacyAttachment adds a document with a legacy attachment and returns the version of that document.
+func CreateDocWithLegacyAttachment(t *testing.T, rt *RestTester, docID string, rawDoc []byte, attKey string, attBody []byte) DocVersion {
 	// Write attachment directly to the datastore.
 	dataStore := rt.GetSingleDataStore()
 	_, err := dataStore.Add(attKey, 0, attBody)
@@ -2690,6 +2672,10 @@ func CreateDocWithLegacyAttachment(t *testing.T, rt *RestTester, docID string, r
 	// Migrate document metadata from document body to system xattr.
 	attachments := retrieveAttachmentMeta(t, rt, docID)
 	require.Len(t, attachments, 1)
+	// this will do an on demand import
+	docVersion, _ := rt.GetDoc(docID)
+	rt.WaitForPendingChanges()
+	return docVersion
 }
 
 func retrieveAttachmentMeta(t *testing.T, rt *RestTester, docID string) (attMeta map[string]interface{}) {
@@ -2699,13 +2685,16 @@ func retrieveAttachmentMeta(t *testing.T, rt *RestTester, docID string) (attMeta
 	return attachments
 }
 
-func rawDocWithAttachmentAndSyncMeta() []byte {
-	return []byte(`{
+// rawDocWithAttachmentAndSyncMeta returns a raw document with an attachment and sync metadata inline. RestTester is used to determine the sequence for the document.
+func rawDocWithAttachmentAndSyncMeta(rt *RestTester) []byte {
+	latestSeq, err := rt.GetDatabase().NextSequence(rt.Context())
+	require.NoError(rt.TB(), err)
+	return []byte(fmt.Sprintf(`{
    "_sync": {
       "rev": "1-5fc93bd36377008f96fdae2719c174ed",
-      "sequence": 2,
+      "sequence": %d,
       "recent_sequences": [
-         2
+         %d
       ],
       "history": {
          "revs": [
@@ -2731,7 +2720,7 @@ func rawDocWithAttachmentAndSyncMeta() []byte {
       "time_saved": "2021-09-01T17:33:03.054227821Z"
    },
   "key": "value"
-}`)
+}`, latestSeq, latestSeq))
 }
 
 // attachmentHeaders returns the headers needed to store an attachment.

rest/blip_api_attachment_test.go

@@ -340,8 +340,6 @@ func TestBlipPushPullNewAttachmentCommonAncestor(t *testing.T) {
 	})
 }
 func TestBlipPushPullNewAttachmentNoCommonAncestor(t *testing.T) {
-	t.Skip("Skip until CBG-4400 is fixed")
-
 	rtConfig := RestTesterConfig{
 		GuestEnabled: true,
 	}
@@ -361,21 +359,26 @@ func TestBlipPushPullNewAttachmentNoCommonAncestor(t *testing.T) {
 		// CBL creates revisions 1-abc, 2-abc, 3-abc, 4-abc on the client, with an attachment associated with rev 2.
 		// rev tree pruning on the CBL side, so 1-abc no longer exists.
 		// CBL replicates, sends to client as 4-abc history:[4-abc, 3-abc, 2-abc], attachment has revpos=2
-		bodyText := `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"data":"aGVsbG8gd29ybGQ="}}}`
-		rev := NewDocVersionFromFakeRev("2-abc")
-		// FIXME CBG-4400:  docID: doc1 was not found on the client - expecting to update doc based on parentVersion RevID: 2-abc
-		btcRunner.StoreRevOnClient(btc.id, docID, &rev, []byte(bodyText))
+		var latestVersion *DocVersion
+		var firstVersion DocVersion
+		for i := range 3 {
+			version := btcRunner.AddRev(btc.id, docID, latestVersion, []byte(fmt.Sprintf(`{"rev": %d, "greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"data":"aGVsbG8gd29ybGQ="}}}`, i)))
+			if latestVersion == nil {
+				firstVersion = version
+			}
+			latestVersion = &version
+		}
+		btcRunner.SingleCollection(btc.id).pruneVersion(docID, firstVersion)
 
-		bodyText = `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"revpos":2,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
-		docVersion, err := btcRunner.PushRevWithHistory(btc.id, docID, &rev, []byte(bodyText), 2, 0)
-		require.NoError(t, err)
-		require.NotNil(t, docVersion)
-		assert.Equal(t, "4-abc", docVersion.RevID)
+		bodyText := `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"revpos":2,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
+		version4 := btcRunner.AddRev(btc.id, docID, latestVersion, []byte(bodyText))
+		require.Equal(t, "4-abc", version4.RevID)
 
+		btcRunner.StartPushWithOpts(btc.id, BlipTesterPushOptions{Continuous: false})
 		// Wait for the document to be replicated at SG
-		rt.WaitForVersion(docID, *docVersion)
+		rt.WaitForVersion(docID, version4)
 
-		resp := btc.rt.SendAdminRequest(http.MethodGet, "/{{.keyspace}}/"+docID+"?rev="+docVersion.RevID, "")
+		resp := btc.rt.SendAdminRequest(http.MethodGet, "/{{.keyspace}}/"+docID+"?rev="+version4.RevID, "")
 		assert.Equal(t, http.StatusOK, resp.Code)
 
 		var respBody db.Body
@@ -557,7 +560,6 @@ func TestBlipAttachNameChange(t *testing.T) {
 
 // TestBlipLegacyAttachNameChange ensures that CBL name changes for legacy attachments are handled correctly
 func TestBlipLegacyAttachNameChange(t *testing.T) {
-	t.Skip("Skip until CBG-4400 is fixed")
 	rtConfig := &RestTesterConfig{
 		GuestEnabled: true,
 	}
@@ -576,33 +578,25 @@ func TestBlipLegacyAttachNameChange(t *testing.T) {
 		attBody := []byte(`hi`)
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID, digest)
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
 
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, client1.rt, docID, rawDoc, attKey, attBody)
-
-		// Get the document and grab the revID.
-		docVersion, _ := client1.rt.GetDoc(docID)
-
-		// Store the document and attachment on the test client
-		btcRunner.StoreRevOnClient(client1.id, docID, &docVersion, rawDoc)
-		// FIXME CBG-4400: docID: doc was not found on the client - expecting to update doc based on parentVersion RevID: 1-5fc93bd36377008f96fdae2719c174ed
-
-		btcRunner.AttachmentsLock(client1.id).Lock()
-		btcRunner.Attachments(client1.id)[digest] = attBody
-		btcRunner.AttachmentsLock(client1.id).Unlock()
+		version1 := CreateDocWithLegacyAttachment(t, client1.rt, docID, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
 		// Confirm attachment is in the bucket
 		attachmentAKey := db.MakeAttachmentKey(1, "doc", digest)
 		bucketAttachmentA, _, err := client1.rt.GetSingleDataStore().GetRaw(attachmentAKey)
 		require.NoError(t, err)
 		require.EqualValues(t, bucketAttachmentA, attBody)
 
+		btcRunner.StartPull(client1.id)
+		btcRunner.WaitForVersion(client1.id, docID, version1)
+
 		// Simulate changing only the attachment name over CBL
 		// Use revpos 2 to simulate revpos bug in CBL 2.8 - 3.0.0
-		docVersion = btcRunner.AddRev(client1.id, "doc", &docVersion, []byte(`{"key":"val","_attachments":{"attach":{"revpos":2,"content_type":"test/plain","length":2,"stub":true,"digest":"`+digest+`"}}}`))
+		version2 := btcRunner.AddRev(client1.id, "doc", &version1, []byte(`{"key":"val","_attachments":{"attach":{"revpos":2,"content_type":"test/plain","length":2,"stub":true,"digest":"`+digest+`"}}}`))
 
-		client1.rt.WaitForVersion("doc", docVersion)
+		btcRunner.StartPushWithOpts(client1.id, BlipTesterPushOptions{Continuous: false})
+		client1.rt.WaitForVersion("doc", version2)
 
 		resp := client1.rt.SendAdminRequest("GET", "/{{.keyspace}}/doc/attach", "")
 		RequireStatus(t, resp, http.StatusOK)
@@ -612,7 +606,6 @@ func TestBlipLegacyAttachNameChange(t *testing.T) {
 
 // TestBlipLegacyAttachDocUpdate ensures that CBL updates for documents associated with legacy attachments are handled correctly
 func TestBlipLegacyAttachDocUpdate(t *testing.T) {
-	t.Skip("Skip until CBG-4400 is fixed")
 
 	rtConfig := &RestTesterConfig{
 		GuestEnabled: true,
@@ -636,30 +629,24 @@ func TestBlipLegacyAttachDocUpdate(t *testing.T) {
 		digest := db.Sha1DigestKey(attBody)
 		attKey := db.MakeAttachmentKey(db.AttVersion1, docID, digest)
 		attName := "hi.txt"
-		rawDoc := rawDocWithAttachmentAndSyncMeta()
 
 		// Create a document with legacy attachment.
-		CreateDocWithLegacyAttachment(t, client1.rt, docID, rawDoc, attKey, attBody)
-
-		version, _ := client1.rt.GetDoc(docID)
+		version1 := CreateDocWithLegacyAttachment(t, client1.rt, docID, rawDocWithAttachmentAndSyncMeta(rt), attKey, attBody)
 
-		// Store the document and attachment on the test client
-		// FIXME CBG-4400: docID: doc was not found on the client - expecting to update doc based on parentVersion RevID: 1-5fc93bd36377008f96fdae2719c174ed
-		btcRunner.StoreRevOnClient(client1.id, docID, &version, rawDoc)
-		btcRunner.AttachmentsLock(client1.id).Lock()
-		btcRunner.Attachments(client1.id)[digest] = attBody
-		btcRunner.AttachmentsLock(client1.id).Unlock()
-
-		// Confirm attachment is in the bucket
 		attachmentAKey := db.MakeAttachmentKey(1, "doc", digest)
 		dataStore := client1.rt.GetSingleDataStore()
 		bucketAttachmentA, _, err := dataStore.GetRaw(attachmentAKey)
 		require.NoError(t, err)
 		require.EqualValues(t, bucketAttachmentA, attBody)
 
+		btcRunner.StartOneshotPull(client1.id)
+		btcRunner.WaitForVersion(client1.id, docID, version1)
+
+		btcRunner.StartPush(client1.id)
+
 		// Update the document, leaving body intact
-		version = btcRunner.AddRev(client1.id, "doc", &version, []byte(`{"key":"val1","_attachments":{"`+attName+`":{"revpos":2,"content_type":"text/plain","length":2,"stub":true,"digest":"`+digest+`"}}}`))
-		client1.rt.WaitForVersion("doc", version)
+		version2 := btcRunner.AddRev(client1.id, "doc", &version1, []byte(`{"key":"val1","_attachments":{"`+attName+`":{"revpos":2,"content_type":"text/plain","length":2,"stub":true,"digest":"`+digest+`"}}}`))
+		client1.rt.WaitForVersion("doc", version2)
 
 		resp := client1.rt.SendAdminRequest("GET", fmt.Sprintf("/{{.keyspace}}/doc/%s", attName), "")
 		RequireStatus(t, resp, http.StatusOK)
@@ -678,7 +665,6 @@ func TestBlipLegacyAttachDocUpdate(t *testing.T) {
 		if !errors.Is(err, sgbucket.MissingError{Key: v2Key}) {
 			var keyValueErr *gocb.KeyValueError
 			require.True(t, errors.As(err, &keyValueErr))
-			//require.Equal(t, keyValueErr.StatusCode, memd.StatusKeyNotFound)
 			require.Equal(t, keyValueErr.DocumentID, v2Key)
 		}
 	})

rest/blip_client_test.go

@@ -251,6 +251,17 @@ func (cd *clientDoc) getRev(version DocVersion) (*clientDocRev, error) {
 	return &rev, nil
 }
 
+// pruneVersion removes the given version from the document.
+func (cd *clientDoc) pruneVersion(t testing.TB, version DocVersion) {
+	cd.lock.Lock()
+	defer cd.lock.Unlock()
+	seq, ok := cd._seqsByVersions[version]
+	require.Less(t, seq, cd._latestSeq, "seq %d is the latest seq for doc %q, can not prune latest version", seq, cd.id)
+	require.True(t, ok, "version %v not found in seqsByVersions", version)
+	delete(cd._seqsByVersions, version)
+	delete(cd._revisionsBySeq, seq)
+}
+
 type BlipTesterCollectionClient struct {
 	parent *BlipTesterClient
 
@@ -894,15 +905,6 @@ func (btcRunner *BlipTestClientRunner) Run(test func(t *testing.T, SupportedBLIP
 	btcRunner.t.Run("revTree", func(t *testing.T) {
 		test(t, []string{db.CBMobileReplicationV3.SubprotocolString()})
 	})
-	// if test is not wanting version vector subprotocol to be run, return before we start this subtest
-	if btcRunner.SkipVersionVectorInitialization {
-		return
-	}
-	btcRunner.t.Run("versionVector", func(t *testing.T) {
-		t.Skip("skip VV subtest on master")
-		// bump sub protocol version here and pass into test function pending CBG-3253
-		test(t, nil)
-	})
 }
 
 func (btc *BlipTesterClient) tearDownBlipClientReplications() {
@@ -1412,10 +1414,6 @@ func (btc *BlipTesterCollectionClient) PushRevWithHistory(docID string, parentVe
 	return &newRev.version, nil
 }
 
-func (btc *BlipTesterCollectionClient) StoreRevOnClient(docID string, parentVersion *DocVersion, body []byte) {
-	btc.upsertDoc(docID, parentVersion, body)
-}
-
 func (btc *BlipTesterCollectionClient) ProcessInlineAttachments(inputBody []byte, revGen int) (outputBody []byte) {
 	if !bytes.Contains(inputBody, []byte(db.BodyAttachments)) {
 		return inputBody
@@ -1653,10 +1651,6 @@ func (btcRunner *BlipTestClientRunner) saveAttachment(clientID uint32, attachmen
 	return btcRunner.SingleCollection(clientID).saveAttachment(attachmentData)
 }
 
-func (btcRunner *BlipTestClientRunner) StoreRevOnClient(clientID uint32, docID string, parentVersion *DocVersion, body []byte) {
-	btcRunner.SingleCollection(clientID).StoreRevOnClient(docID, parentVersion, body)
-}
-
 func (btcRunner *BlipTestClientRunner) PushRevWithHistory(clientID uint32, docID string, parentVersion *DocVersion, body []byte, revCount, prunedRevCount int) (*DocVersion, error) {
 	return btcRunner.SingleCollection(clientID).PushRevWithHistory(docID, parentVersion, body, revCount, prunedRevCount)
 }
@@ -1731,3 +1725,12 @@ func (c *BlipTesterCollectionClient) lastSeq() clientSeq {
 	defer c.seqLock.RUnlock()
 	return c._seqLast
 }
+
+// pruneVersion removes the given version from the specified doc. This is not allowed for the latest version of a document.
+func (btcc *BlipTesterCollectionClient) pruneVersion(docID string, version DocVersion) {
+	btcc.seqLock.Lock()
+	defer btcc.seqLock.Unlock()
+	doc, ok := btcc._getClientDoc(docID)
+	require.True(btcc.TB(), ok, "docID %q not found")
+	doc.pruneVersion(btcc.TB(), version)
+}

rest/utilities_testing.go

@@ -1686,7 +1686,6 @@ func (bt *BlipTester) SendRevWithHistory(docId, docRev string, revHistory []stri
 	if len(revHistory) > 0 {
 		revRequest.Properties["history"] = strings.Join(revHistory, ",")
 	}
-
 	// Override any properties which have been supplied explicitly
 	for k, v := range properties {
 		revRequest.Properties[k] = v