Merge branch 'main' into backend-interface

FerretDB · May 17, 2023 · e4aaffa · e4aaffa
2 parents 7fbc826 + 2619638
commit e4aaffa
Show file tree

Hide file tree

Showing 9 changed files with 149 additions and 61 deletions.
diff --git a/integration/aggregate_documents_compat_test.go b/integration/aggregate_documents_compat_test.go
@@ -1569,6 +1569,58 @@ func TestAggregateCompatProject(t *testing.T) {
 				bson.D{{"$project", bson.D{{"_id", false}, {"foo", primitive.Regex{Pattern: "^fo"}}}}},
 			},
 		},
+		"DotNotationInclude": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{
+					{"_id", true},
+					{"v.foo", true},
+				}}},
+			},
+		},
+		"DotNotationIncludeTwo": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{
+					{"_id", true},
+					{"v.foo", true},
+					{"v.array", true},
+				}}},
+			},
+		},
+		"DotNotationExclude": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{
+					{"_id", true},
+					{"v.foo", false},
+				}}},
+			},
+		},
+		"DotNotationExcludeTwo": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{
+					{"_id", true},
+					{"v.foo", false},
+					{"v.array", false},
+				}}},
+			},
+		},
+		"DotNotationExcludeSecondLevel": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{
+					{"_id", true},
+					{"v.array.42", false},
+				}}},
+			},
+		},
+		"DotNotationIncludeExclude": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{
+					{"_id", true},
+					{"v.foo", true},
+					{"v.array.42", false},
+				}}},
+			},
+			resultType: emptyResult,
+		},
 	}
 
 	testAggregateStagesCompat(t, testCases)

diff --git a/integration/aggregate_documents_test.go b/integration/aggregate_documents_test.go
@@ -0,0 +1,76 @@
+// 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"
+
+	"github.com/stretchr/testify/require"
+	"go.mongodb.org/mongo-driver/bson"
+	"go.mongodb.org/mongo-driver/mongo"
+
+	"github.com/FerretDB/FerretDB/integration/setup"
+)
+
+func TestAggregateProjectErrors(t *testing.T) {
+	t.Parallel()
+
+	for name, tc := range map[string]struct {
+		expectedErr *mongo.CommandError
+		altMessage  string
+		skip        string
+		pipeline    bson.A
+	}{
+		"EmptyPipeline": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{}}},
+			},
+			expectedErr: &mongo.CommandError{
+				Code:    51272,
+				Name:    "Location51272",
+				Message: "Invalid $project :: caused by :: projection specification must have at least one field",
+			},
+		},
+		"EmptyProjectionField": {
+			pipeline: bson.A{
+				bson.D{{"$project", bson.D{{"v", bson.D{}}}}},
+			},
+			expectedErr: &mongo.CommandError{
+				Code:    51270,
+				Name:    "Location51270",
+				Message: "Invalid $project :: caused by :: An empty sub-projection is not a valid value. Found empty object at path",
+			},
+			skip: "https://github.com/FerretDB/FerretDB/issues/2633",
+		},
+	} {
+		name, tc := name, tc
+		t.Run(name, func(t *testing.T) {
+			if tc.skip != "" {
+				t.Skip(tc.skip)
+			}
+
+			t.Parallel()
+			ctx, collection := setup.Setup(t)
+
+			_, err := collection.Aggregate(ctx, tc.pipeline)
+
+			if tc.expectedErr != nil {
+				AssertEqualAltCommandError(t, *tc.expectedErr, tc.altMessage, err)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}
diff --git a/internal/handlers/common/aggregations/projection.go b/internal/handlers/common/aggregations/projection.go
@@ -27,24 +27,24 @@ import (
 	"github.com/FerretDB/FerretDB/internal/util/must"
 )
 
-// errProjectionEmpty indicates projection field is empty.
-var errProjectionEmpty = errors.New("projection is empty")
-
 // ValidateProjection check projection document.
 // Document fields could be either included or excluded but not both.
 // Exception is for the _id field that could be included or excluded.
 //
 // Errors:
+//   - `ErrEmptyProject` when projection document is empty;
 //   - `ErrProjectionExIn` when there is exclusion in inclusion projection;
 //   - `ErrProjectionInEx` when there is inclusion in exclusion projection;
 //   - `ErrNotImplemented` when there is unimplemented projection operators and expressions;
-//   - `errProjectionEmpty` when projection document is empty.
 func ValidateProjection(projection *types.Document) (*types.Document, bool, error) {
 	validated := types.MakeDocument(0)
 
 	if projection.Len() == 0 {
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
-		return nil, false, errProjectionEmpty
+		return nil, false, commonerrors.NewCommandErrorMsgWithArgument(
+			commonerrors.ErrEmptyProject,
+			"Invalid $project :: caused by :: projection specification must have at least one field",
+			"$project (stage)",
+		)
 	}
 
 	var projectionVal *bool
@@ -62,7 +62,7 @@ func ValidateProjection(projection *types.Document) (*types.Document, bool, erro
 			return nil, false, lazyerrors.Error(err)
 		}
 
-		if strings.Contains(key, "$") {
+		if strings.HasPrefix(key, "$") {
 			return nil, false, commonerrors.NewCommandErrorMsg(
 				commonerrors.ErrNotImplemented,
 				fmt.Sprintf("projection operator $ is not supported in %s", key),
@@ -295,12 +295,10 @@ func includeProjection(path types.Path, source any, projected *types.Document) (
 			// set doc if projected has field from other projection field.
 			v := must.NotFail(projected.Get(key))
 			if d, ok := v.(*types.Document); ok {
-				// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 				doc = d
 			}
 
 			if arr, ok := v.(*types.Array); ok {
-				// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 				// use next prefix key with arr value, allowing array to parse existing
 				// projection fields.
 				doc = must.NotFail(types.NewDocument(path.TrimPrefix().Prefix(), arr))
@@ -318,7 +316,6 @@ func includeProjection(path types.Path, source any, projected *types.Document) (
 		case *types.Document:
 			setBySourceOrder(key, doc, source, projected)
 		case *types.Array:
-			// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 			projected.Set(key, arr)
 		}
 
@@ -333,7 +330,6 @@ func includeProjection(path types.Path, source any, projected *types.Document) (
 		if v, err := projected.Get(key); err == nil {
 			projectedArr, ok := v.(*types.Array)
 			if ok {
-				// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 				arr = projectedArr
 				inclusionExists = true
 			}
@@ -345,15 +341,13 @@ func includeProjection(path types.Path, source any, projected *types.Document) (
 			_, arrElem, err := iter.Next()
 			if err != nil {
 				if errors.Is(err, iterator.ErrIteratorDone) {
-					// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 					break
 				}
 
 				return nil, lazyerrors.Error(err)
 			}
 
 			if _, ok := arrElem.(*types.Document); !ok {
-				// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 				continue
 			}
 
@@ -380,22 +374,19 @@ func includeProjection(path types.Path, source any, projected *types.Document) (
 				doc = docVal
 			} else {
 				// first inclusion field, insert it to the doc.
-				// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 				arr.Append(doc)
 			}
 
 			if _, err = includeProjection(path, arrElem, doc); err != nil {
 				return nil, err
 			}
 
-			// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 			arr.Set(i, doc)
 			i++
 		}
 
 		return arr, nil
 	default:
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 		// field is not a document or an array, nothing to set.
 		return nil, nil
 	}
@@ -431,7 +422,6 @@ func excludeProjection(path types.Path, projected any) {
 			return
 		}
 
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 		// recursively remove field from the embeddedSource.
 		excludeProjection(path.TrimPrefix(), embeddedSource)
 
@@ -442,18 +432,15 @@ func excludeProjection(path types.Path, projected any) {
 			arrElem := must.NotFail(projected.Get(i))
 
 			if _, ok := arrElem.(*types.Document); !ok {
-				// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 				// not a document, cannot possibly be part of path, do nothing.
 				continue
 			}
 
-			// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 			excludeProjection(path, arrElem)
 		}
 
 		return
 	default:
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 		// not a path, nothing to exclude.
 		return
 	}
@@ -480,12 +467,10 @@ func setBySourceOrder(key string, val any, source, projected *types.Document) {
 		}
 
 		if newFieldIndex >= len(projectedKeys) {
-			// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 			break
 		}
 
 		if sourceKey == projectedKeys[newFieldIndex] {
-			// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 			newFieldIndex++
 		}
 	}
@@ -494,7 +479,6 @@ func setBySourceOrder(key string, val any, source, projected *types.Document) {
 
 	// remove fields of projected from newFieldIndex to the end
 	for i := newFieldIndex; i < len(projectedKeys); i++ {
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 		projected.Remove(projectedKeys[i])
 	}
 
@@ -503,7 +487,6 @@ func setBySourceOrder(key string, val any, source, projected *types.Document) {
 	// copy newFieldIndex-th to the end from tmp to projected
 	i := newFieldIndex
 	for _, key := range tmp.Keys()[newFieldIndex:] {
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
 		projected.Set(key, must.NotFail(tmp.Get(tmp.Keys()[i])))
 		i++
 	}

diff --git a/internal/handlers/common/aggregations/projection_iterator.go b/internal/handlers/common/aggregations/projection_iterator.go
@@ -15,8 +15,6 @@
 package aggregations
 
 import (
-	"errors"
-
 	"github.com/FerretDB/FerretDB/internal/types"
 	"github.com/FerretDB/FerretDB/internal/util/iterator"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
@@ -30,13 +28,8 @@ import (
 // Close method closes the underlying iterator.
 func ProjectionIterator(iter types.DocumentsIterator, closer *iterator.MultiCloser, projection *types.Document) (types.DocumentsIterator, error) { //nolint:lll // for readability
 	projectionValidated, inclusion, err := ValidateProjection(projection)
-	if errors.Is(err, errProjectionEmpty) {
-		// TODO: https://github.com/FerretDB/FerretDB/issues/2633
-		return iter, nil
-	}
-
 	if err != nil {
-		return nil, lazyerrors.Error(err)
+		return nil, err
 	}
 
 	res := &projectionIterator{

diff --git a/internal/handlers/common/aggregations/stages/project.go b/internal/handlers/common/aggregations/stages/project.go
@@ -16,8 +16,6 @@ package stages
 
 import (
 	"context"
-	"errors"
-	"fmt"
 
 	"github.com/FerretDB/FerretDB/internal/handlers/common"
 	"github.com/FerretDB/FerretDB/internal/handlers/common/aggregations"
@@ -50,17 +48,7 @@ func newProject(stage *types.Document) (aggregations.Stage, error) {
 		)
 	}
 
-	var cmdErr *commonerrors.CommandError
-
 	validated, inclusion, err := aggregations.ValidateProjection(fields)
-	if errors.As(err, &cmdErr) {
-		return nil, commonerrors.NewCommandErrorMsgWithArgument(
-			cmdErr.Code(),
-			fmt.Sprintf("Invalid $project :: caused by :: %s", cmdErr.Unwrap()),
-			"$project (stage)",
-		)
-	}
-
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/handlers/common/projection.go b/internal/handlers/common/projection.go
@@ -27,9 +27,6 @@ import (
 	"github.com/FerretDB/FerretDB/internal/util/must"
 )
 
-// errProjectionEmpty indicates projection field is empty.
-var errProjectionEmpty = errors.New("projection is empty")
-
 // ValidateProjection check projection document.
 // Document fields could be either included or excluded but not both.
 // Exception is for the _id field that could be included or excluded.
@@ -40,12 +37,12 @@ var errProjectionEmpty = errors.New("projection is empty")
 //   - `ErrProjectionExIn` when there is exclusion in inclusion projection;
 //   - `ErrProjectionInEx` when there is inclusion in exclusion projection;
 //   - `ErrNotImplemented` when there is unimplemented projection operators and expressions;
-//   - `errProjectionEmpty` when projection document is empty.
 func ValidateProjection(projection *types.Document) (*types.Document, bool, error) {
 	validated := types.MakeDocument(0)
 
 	if projection.Len() == 0 {
-		return nil, false, errProjectionEmpty
+		// empty projection is exclusion project.
+		return types.MakeDocument(0), false, nil
 	}
 
 	var projectionVal *bool
@@ -63,7 +60,7 @@ func ValidateProjection(projection *types.Document) (*types.Document, bool, erro
 			return nil, false, lazyerrors.Error(err)
 		}
 
-		if strings.Contains(key, "$") {
+		if strings.HasPrefix(key, "$") {
 			return nil, false, commonerrors.NewCommandErrorMsg(
 				commonerrors.ErrNotImplemented,
 				fmt.Sprintf("projection operator $ is not supported in %s", key),