Fix update with query operator for upsert option (#3028)

Closes #2945.
FerretDB · Jul 14, 2023 · 70f33ed · 70f33ed
1 parent 21f9faf
commit 70f33ed
Show file tree

Hide file tree

Showing 7 changed files with 493 additions and 8 deletions.
diff --git a/integration/update_compat_test.go b/integration/update_compat_test.go
@@ -155,6 +155,129 @@ func testUpdateCompat(t *testing.T, testCases map[string]updateCompatTestCase) {
 	}
 }
 
+// testUpdateManyCompatTestCase describes update compatibility test case.
+type testUpdateManyCompatTestCase struct { //nolint:vet // used for testing only
+	update     bson.D                   // required if replace is nil
+	filter     bson.D                   // defaults to bson.D{{"_id", id}}
+	updateOpts *options.UpdateOptions   // defaults to nil
+	resultType compatTestCaseResultType // defaults to nonEmptyResult
+	providers  []shareddata.Provider    // defaults to shareddata.AllProviders()
+
+	skip string // skips test if non-empty
+}
+
+// testUpdateManyCompat tests update compatibility test cases.
+func testUpdateManyCompat(t *testing.T, testCases map[string]testUpdateManyCompatTestCase) {
+	t.Helper()
+
+	for name, tc := range testCases {
+		name, tc := name, tc
+		t.Run(name, func(t *testing.T) {
+			t.Helper()
+
+			if tc.skip != "" {
+				t.Skip(tc.skip)
+			}
+
+			t.Parallel()
+
+			providers := shareddata.AllProviders()
+			if tc.providers != nil {
+				providers = tc.providers
+			}
+
+			s := setup.SetupCompatWithOpts(t, &setup.SetupCompatOpts{
+				Providers:                providers,
+				AddNonExistentCollection: true,
+			})
+			ctx, targetCollections, compatCollections := s.Ctx, s.TargetCollections, s.CompatCollections
+
+			update := tc.update
+			require.NotNil(t, update, "`update` must be set")
+
+			var nonEmptyResults bool
+			for i := range targetCollections {
+				targetCollection := targetCollections[i]
+				compatCollection := compatCollections[i]
+				t.Run(targetCollection.Name(), func(t *testing.T) {
+					t.Helper()
+
+					allDocs := FindAll(t, ctx, targetCollection)
+
+					for _, doc := range allDocs {
+						id, _ := ConvertDocument(t, doc).Get("_id")
+						require.NotNil(t, id)
+
+						t.Run(fmt.Sprint(id), func(t *testing.T) {
+							t.Helper()
+
+							filter := tc.filter
+							if tc.filter == nil {
+								filter = bson.D{{"_id", id}}
+							}
+
+							var targetUpdateRes, compatUpdateRes *mongo.UpdateResult
+							var targetErr, compatErr error
+
+							targetUpdateRes, targetErr = targetCollection.UpdateMany(ctx, filter, update, tc.updateOpts)
+							compatUpdateRes, compatErr = compatCollection.UpdateMany(ctx, filter, update, tc.updateOpts)
+
+							if targetErr != nil {
+								t.Logf("Target error: %v", targetErr)
+								t.Logf("Compat error: %v", compatErr)
+
+								// error messages are intentionally not compared
+								AssertMatchesWriteError(t, compatErr, targetErr)
+
+								return
+							}
+							require.NoError(t, compatErr, "compat error; target returned no error")
+
+							if pointer.Get(targetUpdateRes).ModifiedCount > 0 || pointer.Get(compatUpdateRes).ModifiedCount > 0 {
+								nonEmptyResults = true
+							}
+
+							assert.Equal(t, compatUpdateRes, targetUpdateRes)
+
+							opts := options.Find().SetSort(bson.D{{"_id", 1}})
+							targetCursor, targetErr := targetCollection.Find(ctx, bson.D{}, opts)
+							compatCursor, compatErr := compatCollection.Find(ctx, bson.D{}, opts)
+
+							if targetCursor != nil {
+								defer targetCursor.Close(ctx)
+							}
+							if compatCursor != nil {
+								defer compatCursor.Close(ctx)
+							}
+
+							require.NoError(t, targetErr)
+							require.NoError(t, compatErr)
+
+							targetRes := FetchAll(t, ctx, targetCursor)
+							compatRes := FetchAll(t, ctx, compatCursor)
+
+							AssertEqualDocumentsSlice(t, compatRes, targetRes)
+
+							if len(targetRes) > 0 || len(compatRes) > 0 {
+								nonEmptyResults = true
+							}
+						})
+					}
+				})
+			}
+
+			switch tc.resultType {
+			case nonEmptyResult:
+				assert.True(t, nonEmptyResults, "expected non-empty results (some documents should be modified)")
+			case emptyResult:
+				assert.False(t, nonEmptyResults, "expected empty results (no documents should be modified)")
+			default:
+				t.Fatalf("unknown result type %v", tc.resultType)
+			}
+		})
+	}
+}
+
 // updateCommandCompatTestCase describes update command compatibility test case.
 type updateCommandCompatTestCase struct {
 	multi      any                      // defaults to false, if true updates multiple documents

diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
@@ -21,6 +21,7 @@ import (
 
 	"go.mongodb.org/mongo-driver/bson"
 	"go.mongodb.org/mongo-driver/bson/primitive"
+	"go.mongodb.org/mongo-driver/mongo/options"
 
 	"github.com/FerretDB/FerretDB/integration/shareddata"
 	"github.com/FerretDB/FerretDB/internal/types"
@@ -229,6 +230,22 @@ func TestUpdateFieldCompatIncComplex(t *testing.T) {
 	testUpdateCompat(t, testCases)
 }
 
+func TestUpdateFieldCompatIncMulti(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]testUpdateManyCompatTestCase{
+		"InvalidInc": {
+			filter:     bson.D{{"v", bson.D{{"$eq", "non-existent"}}}},
+			update:     bson.D{{"$inc", bson.D{{"v", 1}}}},
+			updateOpts: options.Update().SetUpsert(true),
+			providers:  []shareddata.Provider{shareddata.Scalars},
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3044",
+		},
+	}
+
+	testUpdateManyCompat(t, testCases)
+}
+
 func TestUpdateFieldCompatMax(t *testing.T) {
 	t.Parallel()
 
@@ -817,6 +834,38 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 	testUpdateCompat(t, testCases)
 }
 
+func TestUpdateFieldCompatSetMulti(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]testUpdateManyCompatTestCase{
+		"QueryOperatorExists": {
+			filter:     bson.D{{"v", bson.D{{"$lt", 3}}}},
+			update:     bson.D{{"$set", bson.D{{"new", "val"}}}},
+			updateOpts: options.Update().SetUpsert(true),
+			// only use providers contain filter match, no match results in
+			// upsert with generated ID which is tested in integration test
+			providers: []shareddata.Provider{shareddata.Scalars, shareddata.Int32s, shareddata.Doubles},
+		},
+		"QueryOperatorUpsertFalse": {
+			filter:     bson.D{{"v", int32(4080)}},
+			update:     bson.D{{"$set", bson.D{{"new", "val"}}}},
+			updateOpts: options.Update().SetUpsert(false),
+		},
+		"QueryOperatorModified": {
+			filter:     bson.D{{"v", bson.D{{"$eq", 4080}}}},
+			update:     bson.D{{"$set", bson.D{{"new", "val"}}}},
+			updateOpts: options.Update().SetUpsert(false),
+		},
+		"QueryOperatorEmptySet": {
+			filter:     bson.D{{"v", bson.D{{"$eq", 4080}}}},
+			update:     bson.D{{"$set", bson.D{}}},
+			updateOpts: options.Update().SetUpsert(false),
+		},
+	}
+
+	testUpdateManyCompat(t, testCases)
+}
+
 func TestUpdateFieldCompatSetArray(t *testing.T) {
 	t.Parallel()