From 40f54965f90cfa986a6b6aa0e693c53464f0bc80 Mon Sep 17 00:00:00 2001 From: Nick Zolnierz Date: Thu, 7 Mar 2019 13:01:29 -0500 Subject: SERVER-39256 Add a method for serializing a parsed distinct command --- .../db/matcher/schema/encrypt_schema_types.cpp | 1 - src/mongo/db/query/SConscript | 11 +++ src/mongo/db/query/distinct_command.idl | 57 ++++++++++++++ src/mongo/db/query/parsed_distinct.cpp | 69 ++++++----------- src/mongo/db/query/parsed_distinct_test.cpp | 86 +++++++++++++++++++--- 5 files changed, 167 insertions(+), 57 deletions(-) create mode 100644 src/mongo/db/query/distinct_command.idl diff --git a/src/mongo/db/matcher/schema/encrypt_schema_types.cpp b/src/mongo/db/matcher/schema/encrypt_schema_types.cpp index 76681c33c23..39f48f8ac2c 100644 --- a/src/mongo/db/matcher/schema/encrypt_schema_types.cpp +++ b/src/mongo/db/matcher/schema/encrypt_schema_types.cpp @@ -30,7 +30,6 @@ #include "mongo/platform/basic.h" #include "mongo/db/matcher/schema/encrypt_schema_types.h" -#include "mongo/idl/idl_parser.h" namespace mongo { diff --git a/src/mongo/db/query/SConscript b/src/mongo/db/query/SConscript index 16d3baecff1..15d09309b19 100644 --- a/src/mongo/db/query/SConscript +++ b/src/mongo/db/query/SConscript @@ -67,6 +67,16 @@ env.CppUnitTest( ] ) +env.Library( + target='distinct_command_idl', + source=[ + env.Idlc('distinct_command.idl')[0], + ], + LIBDEPS_PRIVATE=[ + '$BUILD_DIR/mongo/idl/idl_parser', + ], +) + # Shared mongod/mongos query code. env.Library( target="query_common", @@ -80,6 +90,7 @@ env.Library( "collation/collator_factory_icu", "collation/collator_icu", "datetime/init_timezone_data", + "distinct_command_idl", "explain_options", "query_planner", "query_request", diff --git a/src/mongo/db/query/distinct_command.idl b/src/mongo/db/query/distinct_command.idl new file mode 100644 index 00000000000..b9029ad6a94 --- /dev/null +++ b/src/mongo/db/query/distinct_command.idl @@ -0,0 +1,57 @@ +# Copyright (C) 2019-present MongoDB, Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the Server Side Public License, version 1, +# as published by MongoDB, Inc. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# Server Side Public License for more details. +# +# You should have received a copy of the Server Side Public License +# along with this program. If not, see +# . +# +# As a special exception, the copyright holders give permission to link the +# code of portions of this program with the OpenSSL library under certain +# conditions as described in each individual source file and distribute +# linked combinations including the program with the OpenSSL library. You +# must comply with the Server Side Public License in all respects for +# all of the code used other than as permitted herein. If you modify file(s) +# with this exception, you may extend this exception to your version of the +# file(s), but you are not obligated to do so. If you do not wish to do so, +# delete this exception statement from your version. If you delete this +# exception statement from all source files in the program, then also delete +# it in the license file. +# + +global: + cpp_namespace: "mongo" + +imports: + - "mongo/idl/basic_types.idl" + +commands: + distinct: + description: "Parser for the 'distinct' command." + cpp_name: DistinctCommand + namespace: concatenate_with_db_or_uuid + strict: true + fields: + key: + description: "The field path for which to return distinct values." + type: string + query: + description: "Optional query that filters the documents from which to retrieve the + distinct values." + type: object + optional: true + collation: + description: "Optional collation for the command." + type: object + optional: true + comment: + description: "Optional comment." + type: string + optional: true diff --git a/src/mongo/db/query/parsed_distinct.cpp b/src/mongo/db/query/parsed_distinct.cpp index c2ea055449f..9d342a1f2a7 100644 --- a/src/mongo/db/query/parsed_distinct.cpp +++ b/src/mongo/db/query/parsed_distinct.cpp @@ -34,8 +34,10 @@ #include "mongo/bson/bsonelement.h" #include "mongo/bson/util/bson_extract.h" #include "mongo/db/query/canonical_query.h" +#include "mongo/db/query/distinct_command_gen.h" #include "mongo/db/query/query_request.h" #include "mongo/db/repl/read_concern_args.h" +#include "mongo/idl/idl_parser.h" #include "mongo/stdx/memory.h" #include "mongo/util/mongoutils/str.h" @@ -119,45 +121,32 @@ StatusWith ParsedDistinct::parse(OperationContext* opCtx, const BSONObj& cmdObj, const ExtensionsCallback& extensionsCallback, bool isExplain) { - // Extract the key field. - BSONElement keyElt; - auto statusKey = bsonExtractTypedField(cmdObj, kKeyField, BSONType::String, &keyElt); - if (!statusKey.isOK()) { - return {statusKey}; + IDLParserErrorContext ctx("distinct"); + + DistinctCommand parsedDistinct(nss); + try { + parsedDistinct = DistinctCommand::parse(ctx, cmdObj); + } catch (...) { + return exceptionToStatus(); } - auto key = keyElt.valuestrsafe(); auto qr = stdx::make_unique(nss); - // Extract the query field. If the query field is nonexistent, an empty query is used. - if (BSONElement queryElt = cmdObj[kQueryField]) { - if (queryElt.type() == BSONType::Object) { - qr->setFilter(queryElt.embeddedObject()); - } else if (queryElt.type() != BSONType::jstNULL) { - return Status(ErrorCodes::TypeMismatch, - str::stream() << "\"" << kQueryField << "\" had the wrong type. Expected " - << typeName(BSONType::Object) - << " or " - << typeName(BSONType::jstNULL) - << ", found " - << typeName(queryElt.type())); - } + if (auto query = parsedDistinct.getQuery()) { + qr->setFilter(query.get()); } - // Extract the collation field, if it exists. - if (BSONElement collationElt = cmdObj[kCollationField]) { - if (collationElt.type() != BSONType::Object) { - return Status(ErrorCodes::TypeMismatch, - str::stream() << "\"" << kCollationField - << "\" had the wrong type. Expected " - << typeName(BSONType::Object) - << ", found " - << typeName(collationElt.type())); - } - qr->setCollation(collationElt.embeddedObject()); + if (auto collation = parsedDistinct.getCollation()) { + qr->setCollation(collation.get()); + } + + if (auto comment = parsedDistinct.getComment()) { + qr->setComment(comment.get().toString()); } - if (BSONElement readConcernElt = cmdObj[repl::ReadConcernArgs::kReadConcernFieldName]) { + // The IDL parser above does not handle generic command arguments. Since the underlying query + // request requires the following options, manually parse and verify them here. + if (auto readConcernElt = cmdObj[repl::ReadConcernArgs::kReadConcernFieldName]) { if (readConcernElt.type() != BSONType::Object) { return Status(ErrorCodes::TypeMismatch, str::stream() << "\"" << repl::ReadConcernArgs::kReadConcernFieldName @@ -169,19 +158,7 @@ StatusWith ParsedDistinct::parse(OperationContext* opCtx, qr->setReadConcern(readConcernElt.embeddedObject()); } - if (BSONElement commentElt = cmdObj[kCommentField]) { - if (commentElt.type() != BSONType::String) { - return Status(ErrorCodes::TypeMismatch, - str::stream() << "\"" << kCommentField - << "\" had the wrong type. Expected " - << typeName(BSONType::String) - << ", found " - << typeName(commentElt.type())); - } - qr->setComment(commentElt.str()); - } - - if (BSONElement queryOptionsElt = cmdObj[QueryRequest::kUnwrappedReadPrefField]) { + if (auto queryOptionsElt = cmdObj[QueryRequest::kUnwrappedReadPrefField]) { if (queryOptionsElt.type() != BSONType::Object) { return Status(ErrorCodes::TypeMismatch, str::stream() << "\"" << QueryRequest::kUnwrappedReadPrefField @@ -193,7 +170,7 @@ StatusWith ParsedDistinct::parse(OperationContext* opCtx, qr->setUnwrappedReadPref(queryOptionsElt.embeddedObject()); } - if (BSONElement maxTimeMSElt = cmdObj[QueryRequest::cmdOptionMaxTimeMS]) { + if (auto maxTimeMSElt = cmdObj[QueryRequest::cmdOptionMaxTimeMS]) { auto maxTimeMS = QueryRequest::parseMaxTimeMS(maxTimeMSElt); if (!maxTimeMS.isOK()) { return maxTimeMS.getStatus(); @@ -213,7 +190,7 @@ StatusWith ParsedDistinct::parse(OperationContext* opCtx, return cq.getStatus(); } - return ParsedDistinct(std::move(cq.getValue()), std::move(key)); + return ParsedDistinct(std::move(cq.getValue()), parsedDistinct.getKey().toString()); } } // namespace mongo diff --git a/src/mongo/db/query/parsed_distinct_test.cpp b/src/mongo/db/query/parsed_distinct_test.cpp index 84c0ec2298b..844a1af4844 100644 --- a/src/mongo/db/query/parsed_distinct_test.cpp +++ b/src/mongo/db/query/parsed_distinct_test.cpp @@ -51,7 +51,7 @@ TEST(ParsedDistinctTest, ConvertToAggregationNoQuery) { auto pd = ParsedDistinct::parse(opCtx, testns, - fromjson("{distinct: 'testcoll', key: 'x'}"), + fromjson("{distinct: 'testcoll', key: 'x', $db: 'testdb'}"), ExtensionsCallbackNoop(), !isExplain); ASSERT_OK(pd.getStatus()); @@ -95,8 +95,6 @@ TEST(ParsedDistinctTest, ConvertToAggregationWithAllOptions) { << "testcoll" << "key" << "x" - << "hint" - << BSON("b" << 5) << "collation" << BSON("locale" << "en_US") @@ -109,7 +107,9 @@ TEST(ParsedDistinctTest, ConvertToAggregationWithAllOptions) { << "comment" << "aComment" << "maxTimeMS" - << 100), + << 100 + << "$db" + << "testdb"), ExtensionsCallbackNoop(), !isExplain); ASSERT_OK(pd.getStatus()); @@ -153,11 +153,12 @@ TEST(ParsedDistinctTest, ConvertToAggregationWithQuery) { auto uniqueTxn = serviceContext.makeOperationContext(); OperationContext* opCtx = uniqueTxn.get(); - auto pd = ParsedDistinct::parse(opCtx, - testns, - fromjson("{distinct: 'testcoll', key: 'y', query: {z: 7}}"), - ExtensionsCallbackNoop(), - !isExplain); + auto pd = ParsedDistinct::parse( + opCtx, + testns, + fromjson("{distinct: 'testcoll', key: 'y', query: {z: 7}, $db: 'testdb'}"), + ExtensionsCallbackNoop(), + !isExplain); ASSERT_OK(pd.getStatus()); auto agg = pd.getValue().asAggregationCommand(); @@ -196,7 +197,7 @@ TEST(ParsedDistinctTest, ExplainNotIncludedWhenConvertingToAggregationCommand) { auto pd = ParsedDistinct::parse(opCtx, testns, - fromjson("{distinct: 'testcoll', key: 'x'}"), + fromjson("{distinct: 'testcoll', key: 'x', $db: 'testdb'}"), ExtensionsCallbackNoop(), isExplain); ASSERT_OK(pd.getStatus()); @@ -226,5 +227,70 @@ TEST(ParsedDistinctTest, ExplainNotIncludedWhenConvertingToAggregationCommand) { SimpleBSONObjComparator::kInstance.makeEqualTo())); } +TEST(ParsedDistinctTest, FailsToParseDistinctWithUnknownFields) { + QueryTestServiceContext serviceContext; + auto uniqueTxn = serviceContext.makeOperationContext(); + OperationContext* opCtx = uniqueTxn.get(); + + BSONObj cmdObj = fromjson(R"({ + distinct: 'testcoll', + key: "a", + $db: 'testdb', + unknown: 1 + })"); + + auto parsedDistinct = + ParsedDistinct::parse(opCtx, testns, cmdObj, ExtensionsCallbackNoop(), false); + ASSERT_EQ(parsedDistinct.getStatus().code(), 40415); +} + +TEST(QueryRequestTest, FailsToParseDistinctWithMissingKey) { + QueryTestServiceContext serviceContext; + auto uniqueTxn = serviceContext.makeOperationContext(); + OperationContext* opCtx = uniqueTxn.get(); + + BSONObj cmdObj = fromjson(R"({ + distinct: 'testns', + $db: 'testdb' + })"); + + auto parsedDistinct = + ParsedDistinct::parse(opCtx, testns, cmdObj, ExtensionsCallbackNoop(), false); + ASSERT_EQ(parsedDistinct.getStatus().code(), 40414); +} + +TEST(QueryRequestTest, FailsToParseDistinctWithInvalidKeyType) { + QueryTestServiceContext serviceContext; + auto uniqueTxn = serviceContext.makeOperationContext(); + OperationContext* opCtx = uniqueTxn.get(); + + BSONObj cmdObj = fromjson(R"({ + distinct: 'testns', + key: {a: 1}, + $db: 'test' + })"); + + auto parsedDistinct = + ParsedDistinct::parse(opCtx, testns, cmdObj, ExtensionsCallbackNoop(), false); + ASSERT_EQ(parsedDistinct.getStatus().code(), ErrorCodes::TypeMismatch); +} + +TEST(QueryRequestTest, InvalidGenericCommandArgumentsAreIgnored) { + QueryTestServiceContext serviceContext; + auto uniqueTxn = serviceContext.makeOperationContext(); + OperationContext* opCtx = uniqueTxn.get(); + + BSONObj cmdObj = fromjson(R"({ + distinct: 'testns', + key: 'a', + readConcern: {level: 'invalid'}, + $db: 'test' + })"); + + auto parsedDistinct = + ParsedDistinct::parse(opCtx, testns, cmdObj, ExtensionsCallbackNoop(), false); + ASSERT_OK(parsedDistinct.getStatus()); +} + } // namespace } // namespace mongo -- cgit v1.2.1