From 00553eb23389a3213f384835b5c7d821a0bde1b4 Mon Sep 17 00:00:00 2001 From: Sviatlana Zuiko Date: Mon, 14 Mar 2022 13:34:52 +0300 Subject: Revert "SERVER-63850 Add the count command to API version 1" This reverts commit 1ca10441ee35fcea2a88a0c522fa0890017f5fbd. --- buildscripts/idl/idl_check_compatibility.py | 3 -- jstests/core/version_api_v1_command_coverage.js | 2 +- .../noPassthrough/api_version_parameters_shell.js | 2 +- jstests/sharding/libs/mongos_api_params_util.js | 8 ++--- src/mongo/db/commands/count_cmd.cpp | 4 --- src/mongo/db/query/count_command.idl | 41 ++++++---------------- src/mongo/db/query/count_command_test.cpp | 15 +------- src/mongo/db/query/count_request.cpp | 10 ++++++ src/mongo/db/query/count_request.h | 13 +++++++ src/mongo/s/commands/cluster_count_cmd.cpp | 4 --- 10 files changed, 41 insertions(+), 61 deletions(-) diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py index 8be8092201e..0e54ab30e49 100644 --- a/buildscripts/idl/idl_check_compatibility.py +++ b/buildscripts/idl/idl_check_compatibility.py @@ -118,9 +118,6 @@ 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 46ff078147e..cafc8561fb7 100644 --- a/jstests/core/version_api_v1_command_coverage.js +++ b/jstests/core/version_api_v1_command_coverage.js @@ -45,7 +45,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: true}, + {cmd: () => ({count: "system.js"}), apiVersion1: false}, {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 4b4cb5e8984..52d5f407df1 100644 --- a/jstests/noPassthrough/api_version_parameters_shell.js +++ b/jstests/noPassthrough/api_version_parameters_shell.js @@ -24,8 +24,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/libs/mongos_api_params_util.js b/jstests/sharding/libs/mongos_api_params_util.js index 4485023b70e..914f6b70462 100644 --- a/jstests/sharding/libs/mongos_api_params_util.js +++ b/jstests/sharding/libs/mongos_api_params_util.js @@ -297,13 +297,13 @@ let MongosAPIParametersUtil = (function() { { commandName: "count", run: { - inAPIVersion1: true, + inAPIVersion1: false, shardCommandName: "count", permittedInTxn: false, command: () => ({count: "collection"}) }, explain: { - inAPIVersion1: true, + inAPIVersion1: false, shardCommandName: "explain", permittedInTxn: false, command: () => ({explain: {count: "collection"}}) @@ -312,13 +312,13 @@ let MongosAPIParametersUtil = (function() { { commandName: "count", run: { - inAPIVersion1: true, + inAPIVersion1: false, shardCommandName: "count", permittedInTxn: false, command: () => ({count: "collection", query: {x: 1}}) }, explain: { - inAPIVersion1: true, + inAPIVersion1: false, 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 44d00d4dcb2..5c29dbea68b 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -68,10 +68,6 @@ class CmdCount : public BasicCommand { public: CmdCount() : BasicCommand("count") {} - const std::set& 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 e27659b06eb..8fcfbb2003b 100644 --- a/src/mongo/db/query/count_command.idl +++ b/src/mongo/db/query/count_command.idl @@ -29,16 +29,13 @@ 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/crypto/fle_field_schema.idl" - - "mongo/db/auth/action_type.idl" - - "mongo/db/auth/access_checks.idl" - "mongo/db/query/hint.idl" + - "mongo/crypto/fle_field_schema.idl" types: countLimit: @@ -47,11 +44,17 @@ 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::int32_t" - deserializer: "::mongo::parseMaxTimeMSForIDL" + cpp_type: "std::int64_t" + deserializer: ::count_request::countParseMaxTime commands: count: @@ -60,69 +63,47 @@ commands: cpp_name: CountCommandRequest strict: true namespace: concatenate_with_db_or_uuid - 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 + api_version: "" 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: safeInt64 + type: countSkip 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 encryptionInformation: description: "Encryption Information schema and other tokens for CRUD commands" type: EncryptionInformation optional: true - unstable: true diff --git a/src/mongo/db/query/count_command_test.cpp b/src/mongo/db/query/count_command_test.cpp index da0326746a3..c0039f46a3d 100644 --- a/src/mongo/db/query/count_command_test.cpp +++ b/src/mongo/db/query/count_command_test.cpp @@ -117,19 +117,6 @@ TEST(CountCommandTest, LimitCannotBeMinLong) { CountCommandRequest::parse(ctxt, commandObj), AssertionException, ErrorCodes::BadValue); } -TEST(CountCommandTest, FailLargerThan32BitMaxTimeMS) { - const long long kLargerThan32BitInt = - static_cast(std::numeric_limits::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" @@ -139,7 +126,7 @@ TEST(CountCommandTest, FailParseBadSkipValue) { << "query" << BSON("a" << BSON("$gte" << 11)) << "skip" << -1000)), AssertionException, - 51024); + ErrorCodes::FailedToParse); } TEST(CountCommandTest, FailParseBadCollationType) { diff --git a/src/mongo/db/query/count_request.cpp b/src/mongo/db/query/count_request.cpp index 50788947ec9..0b6f1a68a71 100644 --- a/src/mongo/db/query/count_request.cpp +++ b/src/mongo/db/query/count_request.cpp @@ -54,5 +54,15 @@ 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(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 d3881297273..6f118d3144a 100644 --- a/src/mongo/db/query/count_request.h +++ b/src/mongo/db/query/count_request.h @@ -40,5 +40,18 @@ 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 588a5e41814..d22b8279a17 100644 --- a/src/mongo/s/commands/cluster_count_cmd.cpp +++ b/src/mongo/s/commands/cluster_count_cmd.cpp @@ -53,10 +53,6 @@ class ClusterCountCmd : public ErrmsgCommandDeprecated { public: ClusterCountCmd() : ErrmsgCommandDeprecated("count") {} - const std::set& apiVersions() const { - return kApiVersions1; - } - AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { return AllowedOnSecondary::kAlways; } -- cgit v1.2.1