CBG-4806: move conflict detection to common function to be shared between areas

Author: gregns1

db/crud.go

@@ -1267,6 +1267,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
@@ -1277,11 +1278,12 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
 				return nil, nil, false, nil, addNewerVersionsErr
 			}
 		} 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,50 @@ 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 CV's are the same
+	if localCV.Equal(*incomingCV) {
+		base.DebugfCtx(ctx, base.KeyCRUD, "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:
+	//  - SG writes document 1@cbs1
+	//  - CBL pulls document 1@cbs1
+	//  - SG writes document 2@cbs1
+	if incomingHLV.DominatesSource(*localCV) {
+		return false, nil
+	}
+
+	// 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) {
+		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
+}

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(nil, &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) {