Support $push array update operator (FerretDB#1819)

Closes FerretDB#503.
krosibahili · Jan 20, 2023 · 0a99144 · 0a99144
1 parent d479051
commit 0a99144
Show file tree

Hide file tree

Showing 12 changed files with 354 additions and 149 deletions.
diff --git a/Taskfile.yml b/Taskfile.yml
@@ -173,6 +173,7 @@ tasks:
     cmds:
       - >
         go test -count=1 {{.RACEFLAG}} -tags={{.BUILDTAGS}} -shuffle=on -coverpkg=../...
+        -timeout=15m
         -coverprofile=integration-pg.txt .
         -handler=pg
         -postgresql-url=postgres://[email protected]:5432/ferretdb?pool_min_conns=1

diff --git a/integration/commands_administration_test.go b/integration/commands_administration_test.go
@@ -631,7 +631,7 @@ func TestCommandsAdministrationDataSize(t *testing.T) {
 		assert.Equal(t, float64(1), must.NotFail(doc.Get("ok")))
 		assert.InDelta(t, float64(24_576), must.NotFail(doc.Get("size")), 24_576)
 		assert.InDelta(t, float64(4), must.NotFail(doc.Get("numObjects")), 4) // TODO https://github.com/FerretDB/FerretDB/issues/727
-		assert.InDelta(t, float64(150), must.NotFail(doc.Get("millis")), 150)
+		assert.InDelta(t, float64(200), must.NotFail(doc.Get("millis")), 200)
 	})
 
 	t.Run("NonExisting", func(t *testing.T) {

diff --git a/integration/distinct_compat_test.go b/integration/distinct_compat_test.go
@@ -27,10 +27,10 @@ import (
 
 // distinctCompatTestCase describes count compatibility test case.
 type distinctCompatTestCase struct {
-	field         string                   // required
-	skipForTigris string                   // optional
-	filter        bson.D                   // required
-	resultType    compatTestCaseResultType // defaults to nonEmptyResult
+	field      string                   // required
+	skip       string                   // optional
+	filter     bson.D                   // required
+	resultType compatTestCaseResultType // defaults to nonEmptyResult
 }
 
 func testDistinctCompat(t *testing.T, testCases map[string]distinctCompatTestCase) {
@@ -49,8 +49,8 @@ func testDistinctCompat(t *testing.T, testCases map[string]distinctCompatTestCas
 		t.Run(name, func(t *testing.T) {
 			t.Helper()
 
-			if tc.skipForTigris != "" {
-				setup.SkipForTigrisWithReason(t, tc.skipForTigris)
+			if tc.skip != "" {
+				t.Skip(t, tc.skip)
 			}
 
 			t.Parallel()
@@ -151,6 +151,7 @@ func TestDistinctCompat(t *testing.T) {
 		"DotNotation": {
 			field:  "v.foo",
 			filter: bson.D{},
+			skip:   "https://github.com/FerretDB/FerretDB/issues/1828",
 		},
 		"DotNotationArray": {
 			field:  "v.array.0",

diff --git a/integration/query_comparison_compat_test.go b/integration/query_comparison_compat_test.go
@@ -324,11 +324,14 @@ func TestQueryComparisonCompatGt(t *testing.T) {
 			filter: bson.D{{"v.foo", bson.D{{"$gt", int32(41)}}}},
 		},
 		"DocumentReverse": {
-			filter: bson.D{{"v", bson.D{
-				{"$gt", bson.D{
-					{"array", bson.A{int32(42), "foo", nil}}, {"42", "foo"}, {"foo", int32(42)},
+			filter: bson.D{
+				{"v", bson.D{
+					{"$gt", bson.D{
+						{"array", bson.A{int32(42), "foo", nil}}, {"42", "foo"}, {"foo", int32(42)},
+					}},
 				}},
-			}}},
+				{"_id", bson.D{{"$ne", "array-documents-nested"}}}, // satisfies the $gt condition
+			},
 			resultType: emptyResult,
 		},
 		"DocumentNull": {

diff --git a/integration/shareddata/composites.go b/integration/shareddata/composites.go
@@ -234,3 +234,35 @@ var ArrayRegexes = &Values[string]{
 		"array-regex": bson.A{primitive.Regex{Pattern: "foo", Options: "i"}, primitive.Regex{Pattern: "foo", Options: "i"}},
 	},
 }
+
+// ArrayDocuments contains array with documents with arrays: {"v": [{"foo": [{"bar": "hello"}]}, ...]}.
+// This data set is helpful for dot notation tests: v.0.foo.0.bar.
+var ArrayDocuments = &Values[string]{
+	name:     "ArrayDocuments",
+	handlers: []string{"pg"}, // TODO Enable for Tigris when tests issues are fixed https://github.com/FerretDB/FerretDB/issues/1834
+	validators: map[string]map[string]any{
+		"tigris": {
+			"$tigrisSchemaString": `{
+				"title": "%%collection%%",
+				"primary_key": ["_id"],
+				"properties": {
+					"v": {
+						"type": "array", "items": {
+							"type": "object",	
+							"properties": {	
+								"foo": {"type": "array", "items": {"type": "object", "properties": {"bar": {"type": "string"}}}}
+							}
+						}
+					},
+					"_id": {"type": "string"}
+				}
+			}`,
+		},
+	},
+	data: map[string]any{
+		"array-documents-nested": bson.A{bson.D{{"foo", bson.A{
+			bson.D{{"bar", "hello"}},
+			bson.D{{"bar", "world"}},
+		}}}},
+	},
+}
diff --git a/integration/shareddata/shareddata.go b/integration/shareddata/shareddata.go
@@ -73,6 +73,7 @@ func AllProviders() Providers {
 		ArrayDoubles,
 		ArrayInt32s,
 		ArrayRegexes,
+		ArrayDocuments,
 	}
 
 	// check that names are unique and randomize order

diff --git a/integration/shareddata_test.go b/integration/shareddata_test.go
@@ -31,7 +31,7 @@ import (
 )
 
 func TestEnvData(t *testing.T) {
-	notForTigris := []shareddata.Provider{shareddata.Scalars, shareddata.Composites, shareddata.Nulls}
+	notForTigris := []shareddata.Provider{shareddata.Scalars, shareddata.Composites, shareddata.Nulls, shareddata.ArrayDocuments}
 
 	// Setups one collection for each data set for all handlers and MongoDB.
 	t.Run("All", func(t *testing.T) {

diff --git a/integration/update_array_compat_test.go b/integration/update_array_compat_test.go
@@ -0,0 +1,120 @@
+// Copyright 2021 FerretDB Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package integration
+
+import (
+	"testing"
+
+	"go.mongodb.org/mongo-driver/bson"
+
+	"github.com/FerretDB/FerretDB/integration/setup"
+)
+
+func TestUpdateArrayCompatPop(t *testing.T) {
+	t.Parallel()
+
+	setup.SkipForTigrisWithReason(t, "https://github.com/FerretDB/FerretDB/issues/1834")
+
+	testCases := map[string]updateCompatTestCase{
+		"DuplicateKeys": {
+			update:     bson.D{{"$pop", bson.D{{"v", 1}, {"v", 1}}}},
+			resultType: emptyResult,
+		},
+		"Pop": {
+			update: bson.D{{"$pop", bson.D{{"v", 1}}}},
+		},
+		"PopFirst": {
+			update: bson.D{{"$pop", bson.D{{"v", -1}}}},
+		},
+		"NonExistentField": {
+			update:     bson.D{{"$pop", bson.D{{"non-existent-field", 1}}}},
+			resultType: emptyResult,
+		},
+		"DotNotation": {
+			filter: bson.D{{"_id", "array-documents-nested"}},
+			update: bson.D{{"$pop", bson.D{{"v.0.foo", 1}}}},
+		},
+		"DotNotationPopFirst": {
+			filter: bson.D{{"_id", "array-documents-nested"}},
+			update: bson.D{{"$pop", bson.D{{"v.0.foo", -1}}}},
+		},
+		"DotNotationNonArray": {
+			filter:     bson.D{{"_id", "array-documents-nested"}},
+			update:     bson.D{{"$pop", bson.D{{"v.0.foo.0.bar", 1}}}},
+			resultType: emptyResult,
+		},
+		"DotNotationNonExistentPath": {
+			update:     bson.D{{"$pop", bson.D{{"non.existent.path", 1}}}},
+			resultType: emptyResult,
+		},
+		"PopEmptyValue": {
+			update:     bson.D{{"$pop", bson.D{}}},
+			resultType: emptyResult,
+		},
+		"PopNotValidValueString": {
+			update:     bson.D{{"$pop", bson.D{{"v", "foo"}}}},
+			resultType: emptyResult,
+		},
+		"PopNotValidValueInt": {
+			update:     bson.D{{"$pop", bson.D{{"v", int32(42)}}}},
+			resultType: emptyResult,
+		},
+	}
+
+	testUpdateCompat(t, testCases)
+}
+
+func TestUpdateArrayCompatPush(t *testing.T) {
+	t.Parallel()
+
+	setup.SkipForTigrisWithReason(t, "https://github.com/FerretDB/FerretDB/issues/1834")
+
+	testCases := map[string]updateCompatTestCase{
+		"DuplicateKeys": {
+			update:     bson.D{{"$push", bson.D{{"v", "foo"}, {"v", "bar"}}}},
+			resultType: emptyResult, // conflict because of duplicate keys "v" set in $push
+		},
+		"String": {
+			update: bson.D{{"$push", bson.D{{"v", "foo"}}}},
+		},
+		"Int32": {
+			update:        bson.D{{"$push", bson.D{{"v", int32(42)}}}},
+			skipForTigris: "Some tests would fail because Tigris might convert int32 to float/int64 based on the schema",
+		},
+		"NonExistentField": {
+			update:        bson.D{{"$push", bson.D{{"non-existent-field", int32(42)}}}},
+			skipForTigris: "Tigris does not support adding new fields to documents",
+		},
+		"DotNotation": {
+			filter: bson.D{{"_id", "array-documents-nested"}},
+			update: bson.D{{"$push", bson.D{{"v.0.foo", bson.D{{"bar", "zoo"}}}}}},
+		},
+		"DotNotationNonArray": {
+			filter:     bson.D{{"_id", "array-documents-nested"}},
+			update:     bson.D{{"$push", bson.D{{"v.0.foo.0.bar", "boo"}}}},
+			resultType: emptyResult, // attempt to push to non-array
+		},
+		"DotNotationNonExistentPath": {
+			update:        bson.D{{"$push", bson.D{{"non.existent.path", int32(42)}}}},
+			skipForTigris: "Tigris does not support adding new fields to documents",
+		},
+		"TwoElements": {
+			update:        bson.D{{"$push", bson.D{{"non.existent.path", int32(42)}, {"v", int32(42)}}}},
+			skipForTigris: "Tigris does not support adding new fields to documents",
+		},
+	}
+
+	testUpdateCompat(t, testCases)
+}
diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
@@ -903,55 +903,6 @@ func TestUpdateFieldCompatSetOnInsertArray(t *testing.T) {
 	testUpdateCompat(t, testCases)
 }
 
-func TestUpdateFieldCompatPop(t *testing.T) {
-	t.Parallel()
-
-	testCases := map[string]updateCompatTestCase{
-		"DuplicateKeys": {
-			update:     bson.D{{"$pop", bson.D{{"v", 1}, {"v", 1}}}},
-			resultType: emptyResult,
-		},
-		"Pop": {
-			update:        bson.D{{"$pop", bson.D{{"v", 1}}}},
-			skipForTigris: "https://github.com/FerretDB/FerretDB/issues/1677",
-		},
-		"PopFirst": {
-			update:        bson.D{{"$pop", bson.D{{"v", -1}}}},
-			skipForTigris: "https://github.com/FerretDB/FerretDB/issues/1677",
-		},
-		"PopDotNotation": {
-			update: bson.D{{"$pop", bson.D{{"v.array", 1}}}},
-			skip:   "https://github.com/FerretDB/FerretDB/issues/1663",
-		},
-		"PopNoSuchKey": {
-			update:     bson.D{{"$pop", bson.D{{"foo", 1}}}},
-			resultType: emptyResult,
-		},
-		"PopEmptyValue": {
-			update:     bson.D{{"$pop", bson.D{}}},
-			resultType: emptyResult,
-		},
-		"PopNotValidValueString": {
-			update:     bson.D{{"$pop", bson.D{{"v", "foo"}}}},
-			resultType: emptyResult,
-		},
-		"PopNotValidValueInt": {
-			update:     bson.D{{"$pop", bson.D{{"v", int32(42)}}}},
-			resultType: emptyResult,
-		},
-		"PopLastAndFirst": {
-			update: bson.D{{"$pop", bson.D{{"v", 1}, {"v", -1}}}},
-			skip:   "https://github.com/FerretDB/FerretDB/issues/666",
-		},
-		"PopDotNotationNonArray": {
-			update: bson.D{{"$pop", bson.D{{"v.foo", 1}}}},
-			skip:   "https://github.com/FerretDB/FerretDB/issues/1663",
-		},
-	}
-
-	testUpdateCompat(t, testCases)
-}
-
 func TestUpdateFieldCompatMixed(t *testing.T) {
 	t.Parallel()