Merge branch 'main' into Set-default-pool_max_conns-for-PostgreSQL-1844

FerretDB · Jan 23, 2023 · 0a91e6b · 0a91e6b
2 parents c60778f + b77b7e8
commit 0a91e6b
Show file tree

Hide file tree

Showing 21 changed files with 405 additions and 187 deletions.
diff --git a/.github/ISSUE_TEMPLATE/chore.yml b/.github/ISSUE_TEMPLATE/chore.yml
@@ -29,9 +29,9 @@ body:
       label: Definition of Done
       description: What should be done to consider this issue done? List everything that applies.
       value: |
-        - unit tests added;
-        - integration tests added;
-        - compatibility tests added;
+        - all handlers updated;
+        - unit tests added/updated;
+        - integration/compatibility tests added/updated;
         - spot refactorings done;
         - user documentation updated;
         - something else?

diff --git a/.github/PROCESS.md b/.github/PROCESS.md
@@ -35,6 +35,7 @@ If the team thinks that the task is bigger than **L**, it should be decomposed i
 
 Unless the issue explicitly states otherwise, the following things are always in the scope:
 
+* All handlers.
 * Tests.
   See contributing documentation for general discussion about unit and integration tests.
 * Small spot refactorings.

diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -16,14 +16,12 @@ Closes #{issue_number}.
     please follow CONTRIBUTING.md.
 -->
 
-* [ ] I added unit tests for new functionality or bug fixes.
-* [ ] I added integration tests for new functionality or bug fixes.
-* [ ] I added compatibility tests for new functionality or bug fixes.
+* [ ] I added/updated unit tests.
+* [ ] I added/updated integration/compatibility tests.
+* [ ] I added/updated comments and checked rendering.
 * [ ] I made spot refactorings.
 * [ ] I updated user documentation.
 * [ ] I ran `task all`, and it passed.
-* [ ] I added/updated comments for both exported and unexported top-level declarations (functions, types, etc).
-* [ ] I checked comments rendering with `task godocs`.
-* [ ] I ensured that the title is good enough for the changelog.
+* [ ] I ensured that PR title is good enough for the changelog.
 * [ ] (for maintainers only) I set Reviewers ([`@FerretDB/core`](https://github.com/orgs/FerretDB/teams/core)), Assignee, Labels, Project and project's Sprint fields.
 * [ ] I marked all done items in this checklist.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -203,10 +203,14 @@ Some of our idiosyncrasies:
 Before submitting a pull request, please make sure that:
 
 1. Tests are added for new functionality or fixed bugs.
-2. `task all` passes.
-3. Comments are added or updated for all new and changed top-level declarations (functions, types, etc).
-   Both exported and unexported declarations should have comments.
-4. Comments are rendered correctly in the `task godocs` output.
+Typical test cases include:
+   * happy paths;
+   * dot notation for existing and non-existent paths.
+2. Comments are added or updated for all new or changed code.
+   Please add missing comments for all (both exported and unexported)
+   new and changed top-level declarations (functions, types, etc).
+3. Comments are rendered correctly in the `task godocs` output.
+4. `task all` passes.
 
 ## Contributing documentation
 

diff --git a/README.md b/README.md
@@ -34,7 +34,9 @@ and [contributing guidelines](CONTRIBUTING.md).
 ## Quickstart
 
 These steps describe a quick local setup.
-They are not suitable for most production use-cases because they keep all data inside containers.
+They are not suitable for most production use-cases because they keep all data
+inside containers and don't [encrypt incoming connections](https://docs.ferretdb.io/security/#securing-connections-with-tls/).
+For more configuration options check [Configuration flags and variables](https://docs.ferretdb.io/flags/) page.
 
 1. Store the following in the `docker-compose.yml` file:
 
@@ -69,13 +71,24 @@ They are not suitable for most production use-cases because they keep all data i
    `postgres` container runs PostgreSQL that would store data.
    `ferretdb` runs FerretDB.
 
-2. Start services with `docker compose up -d`.
+2. Fetch the latest version of FerretDB with `docker compose pull`.
+   Afterwards start services with `docker compose up -d`.
 
 3. If you have `mongosh` installed, just run it to connect to FerretDB.
-   If not, run the following command to run `mongosh` inside the temporary MongoDB container, attaching to the same Docker network:
+   It will use credentials passed in `mongosh` flags or MongoDB URI to authenticate to the PostgreSQL database.
+   You'll also need to set `authMechanism` to `PLAIN`.
+   The example URI would look like:
+
+   ```text
+   mongodb://username:password@localhost/ferretdb?authMechanism=PLAIN
+   ```
+
+   See [Security#Authentication](../security.md#authentication) for more details.
+
+   If you don't have `mongosh`, run the following command to run it inside the temporary MongoDB container, attaching to the same Docker network:
 
    ```sh
-   docker run --rm -it --network=ferretdb --entrypoint=mongosh mongo mongodb://ferretdb/
+   docker run --rm -it --network=ferretdb --entrypoint=mongosh mongo "mongodb://username:password@ferretdb/ferretdb?authMechanism=PLAIN"
    ```
 
 You can also install with FerretDB with the `.deb` and `.rpm` packages

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://username@127.0.0.1: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()