diff options
author | A. Jesse Jiryu Davis <jesse@mongodb.com> | 2021-02-24 22:21:12 -0500 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-03-04 15:44:34 +0000 |
commit | 0b4dccad8115c1355597fdf79ccd6fe06ba8a2d3 (patch) | |
tree | 152e11bdf129c2d3cdd79721ca408e801e9b9bfb | |
parent | 43df182276e86958a43337341c9249e1f40fc825 (diff) | |
download | mongo-0b4dccad8115c1355597fdf79ccd6fe06ba8a2d3.tar.gz |
SERVER-53149 Specify getMore in IDL
-rw-r--r-- | src/mongo/db/commands/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/query/SConscript | 3 | ||||
-rw-r--r-- | src/mongo/db/query/cursor_response.idl | 34 | ||||
-rw-r--r-- | src/mongo/db/query/getmore_command.idl | 65 | ||||
-rw-r--r-- | src/mongo/db/query/getmore_request.cpp | 141 | ||||
-rw-r--r-- | src/mongo/db/query/getmore_request_test.cpp | 70 | ||||
-rw-r--r-- | src/mongo/s/commands/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/s/query/async_results_merger_test.cpp | 5 |
8 files changed, 172 insertions, 148 deletions
diff --git a/src/mongo/db/commands/SConscript b/src/mongo/db/commands/SConscript index 61469f03c6d..923a135445f 100644 --- a/src/mongo/db/commands/SConscript +++ b/src/mongo/db/commands/SConscript @@ -351,6 +351,7 @@ env.Library( '$BUILD_DIR/mongo/db/pipeline/aggregation_request_helper', '$BUILD_DIR/mongo/db/pipeline/process_interface/mongo_process_interface', '$BUILD_DIR/mongo/db/query/command_request_response', + '$BUILD_DIR/mongo/db/query/cursor_response_idl', '$BUILD_DIR/mongo/db/query_exec', '$BUILD_DIR/mongo/db/repl/replica_set_messages', '$BUILD_DIR/mongo/db/repl/tenant_migration_access_blocker', diff --git a/src/mongo/db/query/SConscript b/src/mongo/db/query/SConscript index a481cc6e021..914424d9626 100644 --- a/src/mongo/db/query/SConscript +++ b/src/mongo/db/query/SConscript @@ -191,6 +191,7 @@ env.Library( source=[ "distinct_command.idl", "find_command.idl", + "getmore_command.idl", "query_request_helper.cpp", "max_time_ms_parser.cpp", "tailable_mode.cpp", @@ -200,8 +201,6 @@ env.Library( "$BUILD_DIR/mongo/base", "$BUILD_DIR/mongo/db/api_parameters", "$BUILD_DIR/mongo/db/catalog/collection_catalog", - # TODO: This dependency edge can be removed when the 'allowDiskUse' option no longer depends - # on enabling test commands. "$BUILD_DIR/mongo/db/commands/test_commands_enabled", "$BUILD_DIR/mongo/db/pipeline/runtime_constants_idl", "$BUILD_DIR/mongo/db/repl/read_concern_args", diff --git a/src/mongo/db/query/cursor_response.idl b/src/mongo/db/query/cursor_response.idl index 6521d4a0e60..0679cb6ce5b 100644 --- a/src/mongo/db/query/cursor_response.idl +++ b/src/mongo/db/query/cursor_response.idl @@ -38,12 +38,9 @@ imports: - "mongo/idl/basic_types.idl" structs: - InitialResponseCursor: - description: "A struct representing an initial response cursor." + ResponseCursorBase: + description: "Common fields of initial and subsequent cursor responses." fields: - firstBatch: - description: "The first batch of the cursor." - type: array<object> id: cpp_name: "cursorId" description: "The cursor id of the cursor." @@ -63,9 +60,36 @@ structs: description: "Boolean represents whether partial results are being returned." type: optionalBool + InitialResponseCursor: + description: "A struct representing an initial response cursor." + inline_chained_structs: true + chained_structs: + ResponseCursorBase: ResponseCursorBase + fields: + firstBatch: + description: "The first batch of the cursor." + type: array<object> + CursorInitialReply: description: "A struct representing a initial cursor reply." fields: cursor: description: "A response cursor object." type: InitialResponseCursor + + GetMoreResponseCursor: + description: "A struct representing a subsequent response cursor." + inline_chained_structs: true + chained_structs: + ResponseCursorBase: ResponseCursorBase + fields: + nextBatch: + description: "The subsequent batch of the cursor." + type: array<object> + + CursorGetMoreReply: + description: "A struct representing a getMore cursor reply." + fields: + cursor: + description: "A response cursor object." + type: GetMoreResponseCursor diff --git a/src/mongo/db/query/getmore_command.idl b/src/mongo/db/query/getmore_command.idl new file mode 100644 index 00000000000..9e4626d546a --- /dev/null +++ b/src/mongo/db/query/getmore_command.idl @@ -0,0 +1,65 @@ +# Copyright(C) 2021 - 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 +# <http://www.mongodb.com/licensing/server-side-public-license>. +# +# 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" + - "mongo/db/query/cursor_response.idl" + - "mongo/db/repl/replication_types.idl" + +commands: + getMore: + cpp_name: GetMoreCommand + command_name: getMore + description: "Parser for the getMore command." + strict: true + namespace: type + # The command parameter is the cursor id, like {getMore: 12345}. + type: long + api_version: "1" + fields: + collection: + type: string + batchSize: + type: safeInt64 + optional: true + maxTimeMS: + description: "The awaitData timeout." + type: safeInt64 + optional: true + term: + type: long + optional: true + unstable: true + lastKnownCommittedOpTime: + type: optime + optional: true + unstable: true + reply_type: CursorGetMoreReply diff --git a/src/mongo/db/query/getmore_request.cpp b/src/mongo/db/query/getmore_request.cpp index 5327b9e6592..30d8c59337b 100644 --- a/src/mongo/db/query/getmore_request.cpp +++ b/src/mongo/db/query/getmore_request.cpp @@ -35,8 +35,10 @@ #include <boost/optional.hpp> +#include "mongo/db/api_parameters_gen.h" #include "mongo/db/commands.h" #include "mongo/db/namespace_string.h" +#include "mongo/db/query/getmore_command_gen.h" #include "mongo/db/repl/bson_extract_optime.h" #include "mongo/idl/command_generic_argument.h" #include "mongo/util/assert_util.h" @@ -44,21 +46,6 @@ namespace mongo { -namespace { - -const char kCollectionField[] = "collection"; -const char kBatchSizeField[] = "batchSize"; -const char kAwaitDataTimeoutField[] = "maxTimeMS"; -const char kTermField[] = "term"; -const char kLastKnownCommittedOpTimeField[] = "lastKnownCommittedOpTime"; -const char kApiVersion[] = "apiVersion"; -const char kApiStrict[] = "apiStrict"; -const char kApiDeprecationErrors[] = "apiDeprecationErrors"; - -} // namespace - -const char GetMoreRequest::kGetMoreCommandName[] = "getMore"; - GetMoreRequest::GetMoreRequest() : cursorid(0), batchSize(0) {} GetMoreRequest::GetMoreRequest(NamespaceString namespaceString, @@ -95,122 +82,52 @@ Status GetMoreRequest::isValid() const { // static StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbname, - const BSONObj& cmdObj) { - // Required fields. - boost::optional<CursorId> cursorid; - boost::optional<NamespaceString> nss; - - // Optional fields. - boost::optional<std::int64_t> batchSize; - boost::optional<Milliseconds> awaitDataTimeout; - boost::optional<long long> term; - boost::optional<repl::OpTime> lastKnownCommittedOpTime; - - for (BSONElement el : cmdObj) { - const auto fieldName = el.fieldNameStringData(); - - auto containsAPIParamField = fieldName == kApiVersion || fieldName == kApiStrict || - fieldName == kApiDeprecationErrors; + const BSONObj& cmdObj) try { + for (const auto& fieldName : + std::vector<StringData>{APIParametersFromClient::kApiVersionFieldName, + APIParametersFromClient::kApiStrictFieldName, + APIParametersFromClient::kApiDeprecationErrorsFieldName}) { uassert(4937600, str::stream() << "Cannot pass in API parameter field " << fieldName, - !containsAPIParamField); - - if (fieldName == kGetMoreCommandName) { - if (el.type() != BSONType::NumberLong) { - return {ErrorCodes::TypeMismatch, - str::stream() << "Field 'getMore' must be of type long in: " << cmdObj}; - } - - cursorid = el.Long(); - } else if (fieldName == kCollectionField) { - if (el.type() != BSONType::String) { - return {ErrorCodes::TypeMismatch, - str::stream() - << "Field 'collection' must be of type string in: " << cmdObj}; - } - - BSONElement collElt = cmdObj["collection"]; - const std::string coll = (collElt.type() == BSONType::String) ? collElt.String() : ""; - nss = NamespaceString(dbname, coll); - } else if (fieldName == kBatchSizeField) { - if (!el.isNumber()) { - return {ErrorCodes::TypeMismatch, - str::stream() << "Field 'batchSize' must be a number in: " << cmdObj}; - } - - batchSize = el.numberLong(); - } else if (fieldName == kAwaitDataTimeoutField) { - auto maxAwaitDataTime = parseMaxTimeMS(el); - if (!maxAwaitDataTime.isOK()) { - return maxAwaitDataTime.getStatus(); - } - - if (maxAwaitDataTime.getValue()) { - awaitDataTimeout = Milliseconds(maxAwaitDataTime.getValue()); - } - } else if (fieldName == kTermField) { - if (el.type() != BSONType::NumberLong) { - return {ErrorCodes::TypeMismatch, - str::stream() << "Field 'term' must be of type NumberLong in: " << cmdObj}; - } - term = el.Long(); - } else if (fieldName == kLastKnownCommittedOpTimeField) { - repl::OpTime ot; - Status status = bsonExtractOpTimeField(el.wrap(), kLastKnownCommittedOpTimeField, &ot); - if (!status.isOK()) { - return status; - } - lastKnownCommittedOpTime = ot; - } else if (!isGenericArgument(fieldName)) { - return {ErrorCodes::FailedToParse, - str::stream() << "Failed to parse: " << cmdObj << ". " - << "Unrecognized field '" << fieldName << "'."}; - } + !cmdObj.hasField(fieldName)); } - if (!cursorid) { - return {ErrorCodes::FailedToParse, - str::stream() << "Field 'getMore' missing in: " << cmdObj}; - } - - if (!nss) { - return {ErrorCodes::FailedToParse, - str::stream() << "Field 'collection' missing in: " << cmdObj}; - } + auto parsed = GetMoreCommand::parse({"getMore"}, cmdObj); + auto maxTimeMS = parsed.getMaxTimeMS(); GetMoreRequest request( - std::move(*nss), *cursorid, batchSize, awaitDataTimeout, term, lastKnownCommittedOpTime); + NamespaceString(dbname, parsed.getCollection()), + parsed.getCommandParameter(), + parsed.getBatchSize(), + // Treat maxTimeMS=0 the same as none. + (maxTimeMS && *maxTimeMS) ? boost::optional<Milliseconds>(*maxTimeMS) : boost::none, + parsed.getTerm() ? boost::optional<long long>(*parsed.getTerm()) : boost::none, + parsed.getLastKnownCommittedOpTime()); + Status validStatus = request.isValid(); if (!validStatus.isOK()) { return validStatus; } return request; +} catch (const DBException& exc) { + return exc.toStatus(); } BSONObj GetMoreRequest::toBSON() const { - BSONObjBuilder builder; - - builder.append(kGetMoreCommandName, cursorid); - builder.append(kCollectionField, nss.coll()); - - if (batchSize) { - builder.append(kBatchSizeField, *batchSize); - } - - if (awaitDataTimeout) { - builder.append(kAwaitDataTimeoutField, durationCount<Milliseconds>(*awaitDataTimeout)); - } - + auto cmd = GetMoreCommand(cursorid); + cmd.setDbName(nss.db()); + cmd.setCollection(nss.coll()); + cmd.setBatchSize(batchSize); + cmd.setLastKnownCommittedOpTime(lastKnownCommittedOpTime); if (term) { - builder.append(kTermField, *term); + cmd.setTerm(static_cast<int64_t>(*term)); } - - if (lastKnownCommittedOpTime) { - lastKnownCommittedOpTime->append(&builder, kLastKnownCommittedOpTimeField); + if (awaitDataTimeout) { + cmd.setMaxTimeMS(durationCount<Milliseconds>(*awaitDataTimeout)); } - return builder.obj(); + return cmd.toBSON({}); } } // namespace mongo diff --git a/src/mongo/db/query/getmore_request_test.cpp b/src/mongo/db/query/getmore_request_test.cpp index 78b235153f8..4273838b88c 100644 --- a/src/mongo/db/query/getmore_request_test.cpp +++ b/src/mongo/db/query/getmore_request_test.cpp @@ -44,7 +44,6 @@ using namespace mongo; TEST(GetMoreRequestTest, parseFromBSONEmptyCommandObject) { StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON("db", BSONObj()); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::FailedToParse, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONCursorIdNotNumeric) { @@ -52,32 +51,36 @@ TEST(GetMoreRequestTest, parseFromBSONCursorIdNotNumeric) { BSON("getMore" << "not a number" << "collection" - << "coll")); + << "coll" + << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::TypeMismatch, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONCursorIdNotLongLong) { - StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON("db", - BSON("getMore" - << "not a number" - << "collection" << 123)); + StatusWith<GetMoreRequest> result = + GetMoreRequest::parseFromBSON("db", + BSON("getMore" + << "not a number" + << "collection" << 123 << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::TypeMismatch, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONMissingCollection) { StatusWith<GetMoreRequest> result = - GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123))); + GetMoreRequest::parseFromBSON("db", + BSON("getMore" << CursorId(123) << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::FailedToParse, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONCollectionNotString) { StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON( - "db", BSON("getMore" << CursorId(123) << "collection" << 456)); + "db", + BSON("getMore" << CursorId(123) << "collection" << 456 << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::TypeMismatch, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONBatchSizeNotInteger) { @@ -86,25 +89,29 @@ TEST(GetMoreRequestTest, parseFromBSONBatchSizeNotInteger) { BSON("getMore" << CursorId(123) << "collection" << "coll" << "batchSize" - << "not a number")); + << "not a number" + << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::TypeMismatch, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONInvalidCursorId) { StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(0) << "collection" - << "coll")); + << "coll" + << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::BadValue, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONNegativeCursorId) { StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(-123) << "collection" - << "coll")); + << "coll" + << "$db" + << "db")); ASSERT_OK(result.getStatus()); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); ASSERT_EQUALS(CursorId(-123), result.getValue().cursorid); @@ -116,9 +123,9 @@ TEST(GetMoreRequestTest, parseFromBSONUnrecognizedFieldName) { GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "unknown_field" << 1)); + << "unknown_field" << 1 << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::FailedToParse, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONInvalidBatchSize) { @@ -126,9 +133,9 @@ TEST(GetMoreRequestTest, parseFromBSONInvalidBatchSize) { GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "batchSize" << -1)); + << "batchSize" << -1 << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::BadValue, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONInvalidBatchSizeOfZero) { @@ -136,16 +143,18 @@ TEST(GetMoreRequestTest, parseFromBSONInvalidBatchSizeOfZero) { GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "batchSize" << 0)); + << "batchSize" << 0 << "$db" + << "db")); ASSERT_NOT_OK(result.getStatus()); - ASSERT_EQUALS(ErrorCodes::BadValue, result.getStatus().code()); } TEST(GetMoreRequestTest, parseFromBSONNoBatchSize) { StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" - << "coll")); + << "coll" + << "$db" + << "db")); ASSERT_OK(result.getStatus()); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); ASSERT_EQUALS(CursorId(123), result.getValue().cursorid); @@ -157,7 +166,8 @@ TEST(GetMoreRequestTest, parseFromBSONBatchSizeProvided) { GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "batchSize" << 200)); + << "batchSize" << 200 << "$db" + << "db")); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); ASSERT_EQUALS(CursorId(123), result.getValue().cursorid); ASSERT(result.getValue().batchSize); @@ -170,7 +180,9 @@ TEST(GetMoreRequestTest, parseFromBSONIgnoreQueryOptions) { BSON("getMore" << CursorId(123) << "collection" << "coll" << "$queryOptions" - << "bar")); + << "bar" + << "$db" + << "db")); ASSERT_OK(result.getStatus()); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); ASSERT_EQUALS(CursorId(123), result.getValue().cursorid); @@ -181,7 +193,8 @@ TEST(GetMoreRequestTest, parseFromBSONHasMaxTimeMS) { GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "maxTimeMS" << 100)); + << "maxTimeMS" << 100 << "$db" + << "db")); ASSERT_OK(result.getStatus()); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); ASSERT(result.getValue().awaitDataTimeout); @@ -194,7 +207,8 @@ TEST(GetMoreRequestTest, parseFromBSONHasMaxTimeMSOfZero) { GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "maxTimeMS" << 0)); + << "maxTimeMS" << 0 << "$db" + << "db")); ASSERT_OK(result.getStatus()); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); ASSERT_EQUALS(CursorId(123), result.getValue().cursorid); diff --git a/src/mongo/s/commands/SConscript b/src/mongo/s/commands/SConscript index f3e809415b0..176468bf315 100644 --- a/src/mongo/s/commands/SConscript +++ b/src/mongo/s/commands/SConscript @@ -132,6 +132,7 @@ env.Library( '$BUILD_DIR/mongo/db/logical_session_cache_impl', '$BUILD_DIR/mongo/db/pipeline/aggregation', '$BUILD_DIR/mongo/db/query/command_request_response', + '$BUILD_DIR/mongo/db/query/cursor_response_idl', '$BUILD_DIR/mongo/db/query/map_reduce_output_format', '$BUILD_DIR/mongo/db/read_write_concern_defaults', '$BUILD_DIR/mongo/db/repl/hello_auth', diff --git a/src/mongo/s/query/async_results_merger_test.cpp b/src/mongo/s/query/async_results_merger_test.cpp index fd156343e2a..ec41e728f68 100644 --- a/src/mongo/s/query/async_results_merger_test.cpp +++ b/src/mongo/s/query/async_results_merger_test.cpp @@ -1094,7 +1094,10 @@ TEST_F(AsyncResultsMergerTest, GetMoreBatchSizes) { readyEvent = unittest::assertGet(arm->nextEvent()); BSONObj scheduledCmd = getNthPendingRequest(0).cmdObj; - auto request = GetMoreRequest::parseFromBSON("anydbname", scheduledCmd); + auto request = GetMoreRequest::parseFromBSON("anydbname", + scheduledCmd.addField(BSON("$db" + << "anydbname") + .firstElement())); ASSERT_OK(request.getStatus()); ASSERT_EQ(*request.getValue().batchSize, 1LL); ASSERT_EQ(request.getValue().cursorid, 1LL); |