diff options
author | Maddie Zechar <mez2113@columbia.edu> | 2022-04-21 22:12:28 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-04-21 22:46:07 +0000 |
commit | 49956338b388412bcc2883d2fe79e3727465c04c (patch) | |
tree | 2450fc22d57a68c19a4b3e01646daf7834c6b572 | |
parent | f06c72c0999e07319b909e1a983d7b71dbbf28ab (diff) | |
download | mongo-49956338b388412bcc2883d2fe79e3727465c04c.tar.gz |
SERVER-63850: Add the count command to API version 1
-rw-r--r-- | buildscripts/idl/idl_check_compatibility.py | 3 | ||||
-rw-r--r-- | jstests/core/version_api_v1_command_coverage.js | 2 | ||||
-rw-r--r-- | jstests/noPassthrough/api_version_parameters_shell.js | 2 | ||||
-rw-r--r-- | jstests/sharding/count1.js | 5 | ||||
-rw-r--r-- | jstests/sharding/libs/mongos_api_params_util.js | 8 | ||||
-rw-r--r-- | src/mongo/db/commands/count_cmd.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/query/count_command.idl | 39 | ||||
-rw-r--r-- | src/mongo/db/query/count_command_test.cpp | 15 | ||||
-rw-r--r-- | src/mongo/db/query/count_request.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/query/count_request.h | 13 | ||||
-rw-r--r-- | src/mongo/s/commands/cluster_count_cmd.cpp | 4 |
11 files changed, 63 insertions, 42 deletions
diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py index 93d2919bfdd..41955ce0109 100644 --- a/buildscripts/idl/idl_check_compatibility.py +++ b/buildscripts/idl/idl_check_compatibility.py @@ -122,6 +122,9 @@ ALLOW_ANY_TYPE_LIST: List[str] = [ 'aggregate-param-fromMongos', 'aggregate-param-$_requestReshardingResumeToken', 'aggregate-param-isMapReduceCommand', + 'count-param-hint', + 'count-param-limit', + 'count-param-maxTimeMS', 'find-param-filter', 'find-param-projection', 'find-param-sort', diff --git a/jstests/core/version_api_v1_command_coverage.js b/jstests/core/version_api_v1_command_coverage.js index b696109c3e8..b3b66c63813 100644 --- a/jstests/core/version_api_v1_command_coverage.js +++ b/jstests/core/version_api_v1_command_coverage.js @@ -43,7 +43,7 @@ const commands = [ {cmd: () => ({serverStatus: 1}), apiVersion1: false}, {cmd: () => ({usersInfo: 1}), apiVersion1: false}, {cmd: () => ({aggregate: testColl.getName(), pipeline: [], cursor: {}}), apiVersion1: true}, - {cmd: () => ({count: "system.js"}), apiVersion1: false}, + {cmd: () => ({count: "system.js"}), apiVersion1: true}, {cmd: () => ({create: counter_fun()}), apiVersion1: true}, {cmd: () => ({find: counter_fun()}), apiVersion1: true}, { diff --git a/jstests/noPassthrough/api_version_parameters_shell.js b/jstests/noPassthrough/api_version_parameters_shell.js index 9c21a25387a..05bf0bb7610 100644 --- a/jstests/noPassthrough/api_version_parameters_shell.js +++ b/jstests/noPassthrough/api_version_parameters_shell.js @@ -22,8 +22,8 @@ const testCases = [ [false, false, {testDeprecation: 1}, {version: '1', deprecationErrors: true}], [false, false, {testDeprecation: 1}, {version: '1', strict: true, deprecationErrors: true}], // tests with setParameter requireApiVersion: true. - [true, false, {count: 'collection'}, {version: '1', strict: true}], [true, true, {count: 'collection'}, {version: '1'}], + [true, true, {count: 'collection'}, {version: '1', strict: true}], [true, false, {ping: 1}, {}], [true, true, {ping: 1}, {version: '1'}], ]; diff --git a/jstests/sharding/count1.js b/jstests/sharding/count1.js index ce9ce017692..b820a436e8e 100644 --- a/jstests/sharding/count1.js +++ b/jstests/sharding/count1.js @@ -146,11 +146,12 @@ assert.commandFailedWithCode(db.runCommand({count: 'foo', query: {$c: {$abc: 3}} ErrorCodes.BadValue); // ii. Negative skip values should return error. -assert.commandFailedWithCode(db.runCommand({count: 'foo', skip: -2}), ErrorCodes.FailedToParse); +assert.commandFailedWithCode(db.runCommand({count: 'foo', skip: -2}), + [ErrorCodes.FailedToParse, 51024]); // iii. Negative skip values with positive limit should return error. assert.commandFailedWithCode(db.runCommand({count: 'foo', skip: -2, limit: 1}), - ErrorCodes.FailedToParse); + [ErrorCodes.FailedToParse, 51024]); // iv. Unknown options should return error. assert.commandFailedWithCode(db.runCommand({count: 'foo', random: true}), 40415); diff --git a/jstests/sharding/libs/mongos_api_params_util.js b/jstests/sharding/libs/mongos_api_params_util.js index 63e4f3e1ae4..cc2ca81a4b4 100644 --- a/jstests/sharding/libs/mongos_api_params_util.js +++ b/jstests/sharding/libs/mongos_api_params_util.js @@ -292,13 +292,13 @@ let MongosAPIParametersUtil = (function() { { commandName: "count", run: { - inAPIVersion1: false, + inAPIVersion1: true, shardCommandName: "count", permittedInTxn: false, command: () => ({count: "collection"}) }, explain: { - inAPIVersion1: false, + inAPIVersion1: true, shardCommandName: "explain", permittedInTxn: false, command: () => ({explain: {count: "collection"}}) @@ -307,13 +307,13 @@ let MongosAPIParametersUtil = (function() { { commandName: "count", run: { - inAPIVersion1: false, + inAPIVersion1: true, shardCommandName: "count", permittedInTxn: false, command: () => ({count: "collection", query: {x: 1}}) }, explain: { - inAPIVersion1: false, + inAPIVersion1: true, shardCommandName: "explain", permittedInTxn: false, command: () => ({explain: {count: "collection", query: {x: 1}}}) diff --git a/src/mongo/db/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp index 59b336b961d..fc36376c26e 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -68,6 +68,10 @@ class CmdCount : public BasicCommand { public: CmdCount() : BasicCommand("count") {} + const std::set<std::string>& apiVersions() const { + return kApiVersions1; + } + std::string help() const override { return "count objects in collection"; } diff --git a/src/mongo/db/query/count_command.idl b/src/mongo/db/query/count_command.idl index 85e6579555d..ef31d939ceb 100644 --- a/src/mongo/db/query/count_command.idl +++ b/src/mongo/db/query/count_command.idl @@ -29,10 +29,14 @@ global: cpp_namespace: "mongo" cpp_includes: + - "mongo/db/namespace_string.h" - "mongo/db/query/count_request.h" + - "mongo/db/query/max_time_ms_parser.h" imports: - "mongo/idl/basic_types.idl" + - "mongo/db/auth/action_type.idl" + - "mongo/db/auth/access_checks.idl" - "mongo/db/query/hint.idl" types: @@ -42,17 +46,11 @@ types: cpp_type: "std::int64_t" deserializer: ::count_request::countParseLimit - countSkip: - bson_serialization_type: any - description: "An int value for skip. Must be positive." - cpp_type: "std::int64_t" - deserializer: ::count_request::countParseSkip - maxTimeMS: bson_serialization_type: any description: "An int representing max time ms." - cpp_type: "std::int64_t" - deserializer: ::count_request::countParseMaxTime + cpp_type: "std::int32_t" + deserializer: "::mongo::parseMaxTimeMSForIDL" commands: count: @@ -61,43 +59,64 @@ commands: cpp_name: CountCommandRequest strict: true namespace: concatenate_with_db_or_uuid - api_version: "" + api_version: "1" + access_check: + complex: + - check: is_authorized_to_parse_namespace_element + - privilege: + resource_pattern: exact_namespace + action_type: find + - privilege: + resource_pattern: cluster + action_type: useUUID + # Dummy reply type as we won't use it to parse the count reply. + reply_type: OkReply fields: query: description: "A query that selects which documents to count in the collection or view." type: object default: BSONObj() + unstable: false limit: description: "The maximum number of matching documents to count." type: countLimit optional: true + unstable: false skip: description: "The number of matching documents to skip before counting subsequent results." - type: countSkip + type: safeInt64 optional: true + validator: { gte: 0 } + unstable: false hint: description: "The index name to use or the index specification document." type: indexHint default: BSONObj() + unstable: false collation: description: "The collation to use in the count command." type: object optional: true + unstable: false fields: description: "A BSONObj added by the shell. Left in for backwards compatibility." type: object ignore: true + unstable: true readConcern: description: "A BSONObj read concern." type: object optional: true + unstable: false maxTimeMS: type: maxTimeMS optional: true + unstable: false $queryOptions: description: "Unwrapped read preference." cpp_name: queryOptions type: object optional: true + unstable: false diff --git a/src/mongo/db/query/count_command_test.cpp b/src/mongo/db/query/count_command_test.cpp index c0039f46a3d..da0326746a3 100644 --- a/src/mongo/db/query/count_command_test.cpp +++ b/src/mongo/db/query/count_command_test.cpp @@ -117,6 +117,19 @@ TEST(CountCommandTest, LimitCannotBeMinLong) { CountCommandRequest::parse(ctxt, commandObj), AssertionException, ErrorCodes::BadValue); } +TEST(CountCommandTest, FailLargerThan32BitMaxTimeMS) { + const long long kLargerThan32BitInt = + static_cast<long long>(std::numeric_limits<int>::max()) + 1; + auto commandObj = BSON("count" + << "TestColl" + << "$db" + << "TestDB" + << "maxTimeMS" << kLargerThan32BitInt); + + ASSERT_THROWS_CODE( + CountCommandRequest::parse(ctxt, commandObj), AssertionException, ErrorCodes::BadValue); +} + TEST(CountCommandTest, FailParseBadSkipValue) { ASSERT_THROWS_CODE(CountCommandRequest::parse(ctxt, BSON("count" @@ -126,7 +139,7 @@ TEST(CountCommandTest, FailParseBadSkipValue) { << "query" << BSON("a" << BSON("$gte" << 11)) << "skip" << -1000)), AssertionException, - ErrorCodes::FailedToParse); + 51024); } TEST(CountCommandTest, FailParseBadCollationType) { diff --git a/src/mongo/db/query/count_request.cpp b/src/mongo/db/query/count_request.cpp index 0b6f1a68a71..50788947ec9 100644 --- a/src/mongo/db/query/count_request.cpp +++ b/src/mongo/db/query/count_request.cpp @@ -54,15 +54,5 @@ long long countParseLimit(const BSONElement& element) { return limit; } -long long countParseSkip(const BSONElement& element) { - uassert(ErrorCodes::BadValue, "skip value is not a valid number", element.isNumber()); - auto skip = uassertStatusOK(element.parseIntegerElementToNonNegativeLong()); - return skip; -} - -long long countParseMaxTime(const BSONElement& element) { - auto maxTimeVal = uassertStatusOK(parseMaxTimeMS(element)); - return static_cast<long long>(maxTimeVal); -} } // namespace count_request } // namespace mongo diff --git a/src/mongo/db/query/count_request.h b/src/mongo/db/query/count_request.h index 6f118d3144a..d3881297273 100644 --- a/src/mongo/db/query/count_request.h +++ b/src/mongo/db/query/count_request.h @@ -40,18 +40,5 @@ namespace count_request { * Throws on invalid values. */ long long countParseLimit(const BSONElement& element); - -/** - * Parses a skip for a CountCommandRequest. Errors if the value passed is negative. - * Throws on invalid values. - */ -long long countParseSkip(const BSONElement& element); - -/** - * Parses a maxTimeMS for a CountCommandRequest. Errors if the value passed is negative. - * Throws on invalid values. - */ -long long countParseMaxTime(const BSONElement& element); - } // namespace count_request } // namespace mongo diff --git a/src/mongo/s/commands/cluster_count_cmd.cpp b/src/mongo/s/commands/cluster_count_cmd.cpp index d22b8279a17..588a5e41814 100644 --- a/src/mongo/s/commands/cluster_count_cmd.cpp +++ b/src/mongo/s/commands/cluster_count_cmd.cpp @@ -53,6 +53,10 @@ class ClusterCountCmd : public ErrmsgCommandDeprecated { public: ClusterCountCmd() : ErrmsgCommandDeprecated("count") {} + const std::set<std::string>& apiVersions() const { + return kApiVersions1; + } + AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { return AllowedOnSecondary::kAlways; } |