Add tests for update with replacement (#1044)

integration/delete_compat_test.go

@@ -98,7 +98,7 @@ func testDeleteCompat(t *testing.T, testCases map[string]deleteCompatTestCase) {
 					compatRes, compatErr := compatCollection.BulkWrite(ctx, models, opts)
 
 					if targetErr != nil {
-						t.Log(targetErr)
+						t.Logf("Target error: %v", targetErr)
 						targetErr = UnsetRaw(t, targetErr)
 						compatErr = UnsetRaw(t, compatErr)
 						assert.Equal(t, compatErr, targetErr)

integration/query_compat_test.go

@@ -79,7 +79,7 @@ func testQueryCompat(t *testing.T, testCases map[string]queryCompatTestCase) {
 					}
 
 					if targetErr != nil {
-						t.Log(targetErr)
+						t.Logf("Target error: %v", targetErr)
 						targetErr = UnsetRaw(t, targetErr)
 						compatErr = UnsetRaw(t, compatErr)
 						assert.Equal(t, compatErr, targetErr)

integration/setup/common.go

@@ -53,11 +53,24 @@ var (
 // SkipForTigris skips the current test for Tigris handler.
 //
 // This function should not be used lightly in new tests and should eventually be removed.
+//
+// Deprecated: use SkipForTigrisWithReason instead if you must.
 func SkipForTigris(tb testing.TB) {
+	SkipForTigrisWithReason(tb, "")
+}
+
+// SkipForTigrisWithReason skips the current test for Tigris handler.
+//
+// This function should not be used lightly in new tests and should eventually be removed.
+func SkipForTigrisWithReason(tb testing.TB, reason string) {
 	tb.Helper()
 
 	if *handlerF == "tigris" {
-		tb.Skip("Skipping for Tigris")
+		if reason == "" {
+			tb.Skipf("Skipping for Tigris")
+		} else {
+			tb.Skipf("Skipping for Tigris: %s", reason)
+		}
 	}
 }
 

integration/shareddata/composites.go

@@ -18,6 +18,7 @@ import (
 	"math"
 
 	"go.mongodb.org/mongo-driver/bson"
+	"go.mongodb.org/mongo-driver/bson/primitive"
 )
 
 // Composites contain composite values for tests.
@@ -76,3 +77,14 @@ var DocumentsStrings = &Values[string]{
 		// "document-empty":         bson.D{},
 	},
 }
+
+// DocumentsDocuments contains documents with documents with string values for tests.
+var DocumentsDocuments = &Values[primitive.ObjectID]{
+	name:     "DocumentsDocuments",
+	handlers: []string{"pg", "tigris"},
+	data: map[primitive.ObjectID]any{
+		{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}: bson.D{{"foo", int32(42)}},
+		// TODO Dealing with empty doc needs a schema to be defined https://github.com/FerretDB/FerretDB/issues/772
+		// {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}: bson.D{{"v", bson.D{}}},
+	},
+}

integration/shareddata/shareddata.go

@@ -59,6 +59,7 @@ func AllProviders() []Provider {
 
 		DocumentsDoubles,
 		DocumentsStrings,
+		DocumentsDocuments,
 	}
 
 	// check that names are unique and randomize order

integration/update_compat_test.go

@@ -18,18 +18,51 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/AlekSi/pointer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.mongodb.org/mongo-driver/bson"
+	"go.mongodb.org/mongo-driver/mongo"
 
 	"github.com/FerretDB/FerretDB/integration/setup"
 )
 
+func TestUpdateCompat(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]updateCompatTestCase{
+		"UpdateEmptyDocument": {
+			update:     bson.D{},
+			resultType: emptyResult,
+		},
+
+		"ReplaceSimple": {
+			replace: bson.D{{"v", "foo"}},
+		},
+		"ReplaceEmpty": {
+			replace:       bson.D{{"v", ""}},
+			skipForTigris: "TODO",
+		},
+		"ReplaceNull": {
+			replace:       bson.D{{"v", nil}},
+			skipForTigris: "TODO",
+		},
+		"ReplaceEmptyDocument": {
+			replace: bson.D{},
+			skip:    "https://github.com/FerretDB/FerretDB/issues/1045",
+		},
+	}
+
+	testUpdateCompat(t, testCases)
+}
+
 // updateCompatTestCase describes update compatibility test case.
 type updateCompatTestCase struct {
-	update        bson.D // required
-	skip          string // skips test if non-empty
-	skipForTigris bool   // skips test for Tigris if true
+	update        bson.D                   // required if replace is nil
+	replace       bson.D                   // required if update is nil
+	resultType    compatTestCaseResultType // defaults to nonEmptyResult
+	skip          string                   // skips test if non-empty
+	skipForTigris string                   // skips test for Tigris if non-empty
 }
 
 // testUpdateCompat tests update compatibility test cases.
@@ -44,19 +77,23 @@ func testUpdateCompat(t *testing.T, testCases map[string]updateCompatTestCase) {
 			if tc.skip != "" {
 				t.Skip(tc.skip)
 			}
-
-			if tc.skipForTigris {
-				setup.SkipForTigris(t)
+			if tc.skipForTigris != "" {
+				setup.SkipForTigrisWithReason(t, tc.skipForTigris)
 			}
 
 			t.Parallel()
 
 			// Use per-test setup because updates modify data set.
 			ctx, targetCollections, compatCollections := setup.SetupCompat(t)
 
-			update := tc.update
-			require.NotNil(t, update)
+			update, replace := tc.update, tc.replace
+			if update != nil {
+				require.Nil(t, replace, "`replace` must be nil if `update` is set")
+			} else {
+				require.NotNil(t, replace, "`replace` must be set if `update` is nil")
+			}
 
+			var nonEmptyResults bool
 			for i := range targetCollections {
 				targetCollection := targetCollections[i]
 				compatCollection := compatCollections[i]
@@ -72,18 +109,39 @@ func testUpdateCompat(t *testing.T, testCases map[string]updateCompatTestCase) {
 						t.Run(fmt.Sprint(id), func(t *testing.T) {
 							t.Helper()
 
-							targetUpdateRes, targetErr := targetCollection.UpdateByID(ctx, id, update)
-							compatUpdateRes, compatErr := compatCollection.UpdateByID(ctx, id, update)
+							filter := bson.D{{"_id", id}}
+							var targetUpdateRes, compatUpdateRes *mongo.UpdateResult
+							var targetErr, compatErr error
+
+							if update != nil {
+								targetUpdateRes, targetErr = targetCollection.UpdateOne(ctx, filter, update)
+								compatUpdateRes, compatErr = compatCollection.UpdateOne(ctx, filter, update)
+							} else {
+								targetUpdateRes, targetErr = targetCollection.ReplaceOne(ctx, filter, replace)
+								compatUpdateRes, compatErr = compatCollection.ReplaceOne(ctx, filter, replace)
+							}
 
 							if targetErr != nil {
-								t.Log(targetErr)
+								t.Logf("Target error: %v", targetErr)
 								targetErr = UnsetRaw(t, targetErr)
 								compatErr = UnsetRaw(t, compatErr)
+
+								// Skip updates that could not be performed due to Tigris schema validation.
+								if e, ok := targetErr.(mongo.CommandError); ok && e.Name == "DocumentValidationFailure" {
+									if e.HasErrorCodeWithMessage(121, "json schema validation failed for field") {
+										setup.SkipForTigrisWithReason(t, targetErr.Error())
+									}
+								}
+
 								assert.Equal(t, compatErr, targetErr)
 							} else {
 								require.NoError(t, compatErr, "compat error")
 							}
 
+							if pointer.Get(targetUpdateRes).ModifiedCount > 0 || pointer.Get(compatUpdateRes).ModifiedCount > 0 {
+								nonEmptyResults = true
+							}
+
 							assert.Equal(t, compatUpdateRes, targetUpdateRes)
 
 							var targetFindRes, compatFindRes bson.D
@@ -94,6 +152,15 @@ func testUpdateCompat(t *testing.T, testCases map[string]updateCompatTestCase) {
 					}
 				})
 			}
+
+			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)
+			}
 		})
 	}
 }

integration/update_field_compat_test.go

@@ -56,26 +56,27 @@ func TestUpdateFieldCompatUnset(t *testing.T) {
 		"Simple": {
 			update: bson.D{{"$unset", bson.D{{"v", ""}}}},
 		},
-		"NotExistedField": {
-			update: bson.D{{"$unset", bson.D{{"foo", ""}}}},
+		"NonExisting": {
+			update:     bson.D{{"$unset", bson.D{{"foo", ""}}}},
+			resultType: emptyResult,
 		},
-		"NestedField": {
+		"Nested": {
 			update: bson.D{{"$unset", bson.D{{"v", bson.D{{"array", ""}}}}}},
 		},
-		"DotNotationDocumentFieldExist": {
+		"DotNotationDocument": {
 			update: bson.D{{"$unset", bson.D{{"v.foo", ""}}}},
 		},
-		"DotNotationDocumentFieldNotExist": {
-			update: bson.D{{"$unset", bson.D{{"foo.bar", ""}}}},
+		"DotNotationDocumentNonExisting": {
+			update:     bson.D{{"$unset", bson.D{{"foo.bar", ""}}}},
+			resultType: emptyResult,
 		},
-		"DotNotationArrayFieldExist": {
-			update: bson.D{{"$unset", bson.D{{"v.array.0", int32(1)}}}},
+		"DotNotationArrayField": {
+			update:        bson.D{{"$unset", bson.D{{"v.array.0", ""}}}},
+			skipForTigris: "https://github.com/FerretDB/FerretDB/issues/908",
 		},
-		"DotNotationArrayFieldNotExist": {
-			update: bson.D{{"$unset", bson.D{{"foo.0.baz", int32(1)}}}},
-		},
-		"DocumentDotNotationArrFieldNotExist": {
-			update: bson.D{{"$unset", bson.D{{"v.0.foo", int32(1)}}}},
+		"DotNotationArrayNonExisting": {
+			update:     bson.D{{"$unset", bson.D{{"foo.0.baz", int32(1)}}}},
+			resultType: emptyResult,
 		},
 	}
 

integration/update_field_test.go

@@ -86,7 +86,7 @@ func TestUpdateFieldCurrentDate(t *testing.T) {
 
 	t.Run("currentDate", func(t *testing.T) {
 		// maxDifference is a maximum amount of seconds can differ the value in placeholder from actual value
-		maxDifference := time.Duration(2 * time.Minute)
+		maxDifference := time.Duration(3 * time.Minute)
 
 		now := primitive.NewDateTimeFromTime(time.Now().UTC())
 		nowTimestamp := primitive.Timestamp{T: uint32(time.Now().UTC().Unix()), I: uint32(0)}

internal/handlers/common/error.go

@@ -60,6 +60,9 @@ const (
 	// ErrInvalidNamespace indicates that the collection name is invalid.
 	ErrInvalidNamespace = ErrorCode(73) // InvalidNamespace
 
+	// ErrDocumentValidationFailure indicates that document validation failed.
+	ErrDocumentValidationFailure = ErrorCode(121) // DocumentValidationFailure
+
 	// ErrNotImplemented indicates that a flag or command is not implemented.
 	ErrNotImplemented = ErrorCode(238) // NotImplemented
 

internal/handlers/tigris/msg_update.go

@@ -21,6 +21,7 @@ import (
 	"github.com/tigrisdata/tigris-client-go/driver"
 
 	"github.com/FerretDB/FerretDB/internal/handlers/common"
+	"github.com/FerretDB/FerretDB/internal/handlers/tigris/tigrisdb"
 	"github.com/FerretDB/FerretDB/internal/tjson"
 	"github.com/FerretDB/FerretDB/internal/types"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
@@ -202,9 +203,16 @@ func (h *Handler) update(ctx context.Context, sp fetchParam, doc *types.Document
 	h.L.Sugar().Debugf("Update: %s", u)
 
 	_, err = h.db.Driver.UseDatabase(sp.db).Replace(ctx, sp.collection, []driver.Document{u})
-	if err != nil {
+	switch err := err.(type) {
+	case nil:
+		return 1, nil
+	case *driver.Error:
+		if tigrisdb.IsInvalidArgument(err) {
+			return 0, common.NewErrorMsg(common.ErrDocumentValidationFailure, err.Error())
+		}
+
+		return 0, lazyerrors.Error(err)
+	default:
 		return 0, lazyerrors.Error(err)
 	}
-
-	return 1, nil
 }

internal/handlers/tigris/tigrisdb/errors.go

@@ -15,23 +15,16 @@
 package tigrisdb
 
 import (
+	"github.com/AlekSi/pointer"
 	api "github.com/tigrisdata/tigris-client-go/api/server/v1"
 	"github.com/tigrisdata/tigris-client-go/driver"
 )
 
 // IsNotFound returns true if the error is "not found" error.
 // This function is implemented to keep nolint in a single place.
 func IsNotFound(err *driver.Error) bool {
-	if err == nil {
-		return false
-	}
-
 	//nolint:nosnakecase // Tigris named their const that way
-	if err.Code == api.Code_NOT_FOUND {
-		return true
-	}
-
-	return false
+	return pointer.Get(err).Code == api.Code_NOT_FOUND
 }
 
 // IsAlreadyExists returns true if the error is "already exists" error.
@@ -42,9 +35,12 @@ func IsAlreadyExists(err *driver.Error) bool {
 	}
 
 	//nolint:nosnakecase // Tigris named their const that way
-	if err.Code == api.Code_ALREADY_EXISTS {
-		return true
-	}
+	return pointer.Get(err).Code == api.Code_ALREADY_EXISTS
+}
 
-	return false
+// IsInvalidArgument returns true if the error is "invalid argument" error.
+// This function is implemented to keep nolint in a single place.
+func IsInvalidArgument(err *driver.Error) bool {
+	//nolint:nosnakecase // Tigris named their const that way
+	return pointer.Get(err).Code == api.Code_INVALID_ARGUMENT
 }