CBG-4806: move conflict detection to common function (#7683)

Author: gregns1

db/crud.go

@@ -1298,6 +1298,7 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
 		revTreeConflictChecked := false
 		var parent string
 		var currentRevIndex int
+		var isConflict bool
 
 		// Conflict check here
 		// if doc has no HLV defined this is a new doc we haven't seen before, skip conflict check
@@ -1314,11 +1315,12 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
 				}
 			}
 		} else {
-			if doc.HLV.isDominating(newDocHLV) {
+			isConflict, err = IsInConflict(ctx, doc.HLV, newDocHLV)
+			if err != nil && errors.Is(err, ErrNoNewVersionsToAdd) {
 				base.DebugfCtx(ctx, base.KeyCRUD, "PutExistingCurrentVersion(%q): No new versions to add.  existing: %#v  new:%#v", base.UD(newDoc.ID), doc.HLV, newDocHLV)
 				return nil, nil, false, nil, base.ErrUpdateCancel // No new revisions to add
 			}
-			if newDocHLV.isDominating(doc.HLV) {
+			if !isConflict {
 				// update hlv for all newer incoming source version pairs
 				addNewerVersionsErr := doc.HLV.AddNewerVersions(newDocHLV)
 				if addNewerVersionsErr != nil {

db/hybrid_logical_vector.go

@@ -9,7 +9,9 @@
 package db
 
 import (
+	"context"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"maps"
 	"sort"
@@ -689,3 +691,51 @@ func (hlv *HybridLogicalVector) UnmarshalJSON(inputjson []byte) error {
 func (hlv HybridLogicalVector) GoString() string {
 	return fmt.Sprintf("HybridLogicalVector{CurrentVersionCAS:%d, SourceID:%s, Version:%d, PreviousVersions:%#+v, MergeVersions:%#+v}", hlv.CurrentVersionCAS, hlv.SourceID, hlv.Version, hlv.PreviousVersions, hlv.MergeVersions)
 }
+
+// ErrNoNewVersionsToAdd will be thrown when there are no new versions from incoming HLV to be added to HLV that is local
+var ErrNoNewVersionsToAdd = errors.New("no new versions to add to HLV")
+
+// IsInConflict is used to identify if two HLV's are in conflict or not. Will return boolean to indicate if in conflict
+// or not and will error for the following cases:
+//   - Local HLV dominates incoming HLV (meaning local version is a newer version that the incoming one)
+//   - Local CV matches incoming CV, so no new versions to add
+func IsInConflict(ctx context.Context, localHLV, incomingHLV *HybridLogicalVector) (bool, error) {
+	incomingCV := incomingHLV.ExtractCurrentVersionFromHLV()
+	localCV := localHLV.ExtractCurrentVersionFromHLV()
+
+	// check if incoming CV and local CV are the same. This is needed here given that if both CV's are the same the
+	// below check to check local revision is newer than incoming revision will pass given the check and will add the
+	// incoming versions even if CVs are the same
+	if localCV.Equal(*incomingCV) {
+		base.TracefCtx(ctx, base.KeyVV, "incoming CV %#+v is equal to local revision %#+v", incomingCV, localCV)
+		return false, ErrNoNewVersionsToAdd
+	}
+
+	// standard no conflict case. In the simple case, this happens when:
+	//  - Client A writes document 1@srcA
+	//  - Client B pulls document 1@srcA from Client A
+	//  - Client A writes document 2@srcA
+	//	- Client B pulls document 2@srcA from Client A
+	if incomingHLV.DominatesSource(*localCV) {
+		return false, nil
+	}
+
+	// local revision is newer than incoming revision. Common case:
+	// - Client A writes document 1@srcA
+	// - Client A pushes to Client B as 1@srcA
+	// - Client A pulls document 1@srcA from Client B
+	//
+	// NOTE: without P2P replication, this should not be the case and we would not get this revision, since Client A
+	// would respond to a Client B changes message that Client A does not need this revision
+	if localHLV.DominatesSource(*incomingCV) {
+		return false, ErrNoNewVersionsToAdd
+	}
+	// Check if conflict has been previously resolved.
+	// - If merge versions are empty, then it has not be resolved.
+	// - If merge versions do not match, then it has not been resolved.
+	if len(incomingHLV.MergeVersions) != 0 && len(localHLV.MergeVersions) != 0 && maps.Equal(incomingHLV.MergeVersions, localHLV.MergeVersions) {
+		base.DebugfCtx(ctx, base.KeyVV, "merge versions match between local HLV %#v and incoming HLV %#v, conflict previously resolved", localHLV, incomingCV)
+		return false, nil
+	}
+	return true, nil
+}

db/hybrid_logical_vector_test.go

@@ -1413,3 +1413,52 @@ func TestAddVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestIsInConflict(t *testing.T) {
+	testCases := []struct {
+		name               string
+		localHLV           string
+		incomingHLV        string
+		expectedInConflict bool
+		expectedError      bool
+	}{
+		{
+			name:          "CV equal",
+			localHLV:      "111@abc;123@def",
+			incomingHLV:   "111@abc;123@ghi",
+			expectedError: true,
+		},
+		{
+			name:        "no conflict case",
+			localHLV:    "111@abc;123@def",
+			incomingHLV: "112@abc;123@ghi",
+		},
+		{
+			name:          "local revision is newer",
+			localHLV:      "111@abc;123@def",
+			incomingHLV:   "100@abc;123@ghi",
+			expectedError: true,
+		},
+		{
+			name:        "merge versions match",
+			localHLV:    "130@abc,123@def,100@ghi;50@jkl",
+			incomingHLV: "150@mno,123@def,100@ghi;50@jkl",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			localHLV, _, err := extractHLVFromBlipString(tc.localHLV)
+			require.NoError(t, err)
+			incomingHLV, _, err := extractHLVFromBlipString(tc.incomingHLV)
+			require.NoError(t, err)
+
+			inConflict, err := IsInConflict(t.Context(), localHLV, incomingHLV)
+			if tc.expectedError {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+			require.Equal(t, tc.expectedInConflict, inConflict)
+		})
+	}
+}

rest/utilities_testing_blip_client.go

@@ -16,7 +16,6 @@ import (
 	"encoding/base64"
 	"fmt"
 	"iter"
-	"maps"
 	"net/http"
 	"slices"
 	"strconv"
@@ -325,35 +324,16 @@ func (cd *clientDoc) _hasConflict(t testing.TB, incomingHLV *db.HybridLogicalVec
 	incomingCV := incomingHLV.ExtractCurrentVersionFromHLV()
 	localCV := localHLV.ExtractCurrentVersionFromHLV()
 	// safety check - ensure SG is not sending a rev that we already had - ensures changes feed messaging is working correctly to prevent
+	// this check is also performed in the function below but we should keep this here for extra context on FailNow call
 	if localCV.Equal(*incomingCV) {
 		require.FailNow(t, fmt.Sprintf("incoming CV %#+v is equal to local revision %#+v - this should've been filtered via changes response before ending up as a rev. This is only true if there is a single replication occurring, two simultaneous replications (e.g. P2P) could cause this. If there are multiple replications, modify code.", incomingCV, latestRev))
 	}
-	// standard no conflict case. In the simple case, this happens when:
-	//  - SG writes document 1@cbs1
-	//  - CBL pulls document 1@cbs1
-	//  - SG writes document 2@cbs1
-	if incomingHLV.DominatesSource(*localCV) {
-		return false
-	}
 
-	// local revision is newer than incoming revision. Common case:
-	// - CBL writes document 1@cbl1
-	// - CBL pushes to SG as 1@cbl1
-	// - CBL pulls document 1@cbl1
-	//
-	// NOTE: without P2P replication, this should not be the case and we would not get this revision, since CBL
-	// would respond to a SG changes message that CBL does not need this revision
-	if localHLV.DominatesSource(*incomingCV) {
+	inConflict, err := db.IsInConflict(t.Context(), &localHLV, incomingHLV)
+	if err != nil {
 		require.FailNow(t, fmt.Sprintf("incoming CV %#+v has lower version than the local revision %#+v - this should've been filtered via changes response before ending up as a rev. blip tester would reply that to Sync Gateway that it doesn't need this revision", incomingCV, localHLV))
-		return false
-	}
-	// Check if conflict has been previously resolved.
-	// - If merge versions are empty, then it has not be resolved.
-	// - If merge versions do not match, then it has not been resolved.
-	if len(incomingHLV.MergeVersions) != 0 && len(localHLV.MergeVersions) != 0 && maps.Equal(incomingHLV.MergeVersions, localHLV.MergeVersions) {
-		return false
 	}
-	return true
+	return inConflict
 }
 
 func (btcc *BlipTesterCollectionClient) _resolveConflict(incomingHLV *db.HybridLogicalVector, incomingBody []byte, localDoc *clientDocRev) (body []byte, hlv db.HybridLogicalVector) {