Merge branch 'main' into more-iterators-2723

FerretDB · Jun 2, 2023 · 2c3b544 · 2c3b544
2 parents ba79eed + 96858fd
commit 2c3b544
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 13 deletions.
diff --git a/integration/findandmodify_compat_test.go b/integration/findandmodify_compat_test.go
@@ -197,7 +197,7 @@ func TestFindAndModifyCompatUpdate(t *testing.T) {
 			},
 			skipForTigris: "schema validation would fail",
 		},
-		"InvalidOperator": {
+		"Conflict": {
 			command: bson.D{
 				{"query", bson.D{{"_id", bson.D{{"$exists", false}}}}},
 				{"update", bson.D{{"$invalid", "non-existent-field"}}},
@@ -214,6 +214,88 @@ func TestFindAndModifyCompatUpdate(t *testing.T) {
 			},
 			resultType: emptyResult,
 		},
+		"NoConflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int64"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v", 4}}},
+					{"$inc", bson.D{{"foo", 4}}},
+				}},
+			},
+		},
+		"EmptyKey": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int64"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"", 4}}},
+					{"$inc", bson.D{{"", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+		"EmptyKeyAndKey": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int64"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"", 4}}},
+					{"$inc", bson.D{{"v", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+		"InvalidOperator": {
+			command: bson.D{
+				{"query", bson.D{{"_id", bson.D{{"$exists", false}}}}},
+				{"update", bson.D{{"$invalid", "non-existent-field"}}},
+			},
+			resultType: emptyResult,
+		},
+	}
+
+	testFindAndModifyCompat(t, testCases)
+}
+
+func TestFindAndModifyCompatUpdateDotNotation(t *testing.T) {
+	testCases := map[string]findAndModifyCompatTestCase{
+		"Conflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v.0.field", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+		"NoConflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v.0.foo", 4}}},
+				}},
+			},
+		},
+		"NoIndex": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v.field", 4}}},
+				}},
+			},
+		},
+		"ParentConflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+
 		"ConflictKey": {
 			command: bson.D{
 				{"query", bson.D{{"_id", bson.D{{"$exists", false}}}}},

diff --git a/integration/shareddata/composites.go b/integration/shareddata/composites.go
@@ -98,6 +98,16 @@ var ArrayAndDocuments = &Values[string]{
 			bson.D{{"field", int32(44)}},
 			bson.D{{"foo", int32(42)}},
 		},
+		"array-documents-two-fields": bson.A{
+			bson.D{
+				{"field", int32(42)},
+				{"foo", int32(44)},
+			},
+			bson.D{
+				{"field", int32(44)},
+				{"foo", int32(42)},
+			},
+		},
 	},
 }
 

diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
@@ -902,11 +902,11 @@ func newUpdateError(code commonerrors.ErrorCode, msg, command string) error {
 // validateOperatorKeys returns error if any key contains empty path or
 // the same path prefix exists in other key or other document.
 func validateOperatorKeys(command string, docs ...*types.Document) error {
-	seen := map[string]struct{}{}
+	visitedPaths := []*types.Path{}
 
 	for _, doc := range docs {
 		for _, key := range doc.Keys() {
-			path, err := types.NewPathFromString(key)
+			nextPath, err := types.NewPathFromString(key)
 			if err != nil {
 				return newUpdateError(
 					commonerrors.ErrEmptyName,
@@ -918,23 +918,47 @@ func validateOperatorKeys(command string, docs ...*types.Document) error {
 				)
 			}
 
-			if _, ok := seen[path.Prefix()]; ok {
-				return newUpdateError(
-					commonerrors.ErrConflictingUpdateOperators,
-					fmt.Sprintf(
-						"Updating the path '%[1]s' would create a conflict at '%[1]s'", key,
-					),
-					command,
-				)
+			for _, oldPath := range visitedPaths {
+				if checkSlicePrefix(oldPath.Slice(), nextPath.Slice()) {
+					return newUpdateError(
+						commonerrors.ErrConflictingUpdateOperators,
+						fmt.Sprintf(
+							"Updating the path '%[1]s' would create a conflict at '%[1]s'", key,
+						),
+						command,
+					)
+				}
 			}
-
-			seen[path.Prefix()] = struct{}{}
+			visitedPaths = append(visitedPaths, &nextPath)
 		}
 	}
 
 	return nil
 }
 
+// checkSlicePrefix returns true if one slice is the beginning of another slice.
+// The example of slice prefix: arr1 = ["a","b","c"] arr2 = ["a","b"];
+// If both slices are empty, it returns true. If one slice is empty and another slice is not, it returns false.
+func checkSlicePrefix(arr1, arr2 []string) bool {
+	target, prefix := arr1, arr2
+
+	if len(target) < len(prefix) {
+		target, prefix = prefix, target
+	}
+
+	if len(prefix) == 0 {
+		return len(target) == 0
+	}
+
+	for i := range prefix {
+		if prefix[i] != target[i] {
+			return false
+		}
+	}
+
+	return true
+}
+
 // extractValueFromUpdateOperator gets operator "op" value and returns CommandError for `findAndModify`
 // WriteError error other commands if it is not a document.
 // For example, for update document