From 193823453567d03d2c41e35e9dd526311c0f6f3f Mon Sep 17 00:00:00 2001 From: Ruoxin Xu Date: Tue, 19 Jan 2021 15:34:37 +0000 Subject: SERVER-51624 Modify query commands to enforce API version 1 behaviour --- src/mongo/client/dbclient_cursor.h | 2 +- src/mongo/db/auth/authorization_session_impl.cpp | 4 +- src/mongo/db/auth/authorization_session_test.cpp | 68 +++---- src/mongo/db/catalog/index_key_validate.cpp | 10 +- src/mongo/db/commands.h | 2 +- src/mongo/db/commands/count_cmd.cpp | 7 +- src/mongo/db/commands/distinct.cpp | 7 +- src/mongo/db/commands/find_cmd.cpp | 13 +- src/mongo/db/commands/mr_common.h | 1 + src/mongo/db/commands/pipeline_command.cpp | 5 +- src/mongo/db/create_indexes.idl | 5 +- src/mongo/db/error_labels.cpp | 7 +- src/mongo/db/error_labels_test.cpp | 9 +- src/mongo/db/matcher/expression_optimize_test.cpp | 6 +- src/mongo/db/pipeline/SConscript | 2 +- src/mongo/db/pipeline/aggregate_command.idl | 14 +- .../db/pipeline/aggregation_request_helper.cpp | 26 ++- src/mongo/db/pipeline/aggregation_request_helper.h | 18 +- src/mongo/db/pipeline/aggregation_request_test.cpp | 101 ++++++----- .../db/pipeline/document_source_change_stream.cpp | 1 + src/mongo/db/pipeline/expression_context.cpp | 1 + src/mongo/db/pipeline/expression_context.h | 3 +- src/mongo/db/query/SConscript | 1 + src/mongo/db/query/canonical_query_test.cpp | 6 +- src/mongo/db/query/count_command_test.cpp | 10 +- src/mongo/db/query/count_request.cpp | 2 +- src/mongo/db/query/find_command.idl | 25 ++- src/mongo/db/query/getmore_request.cpp | 2 +- src/mongo/db/query/max_time_ms_parser.cpp | 68 +++++++ src/mongo/db/query/max_time_ms_parser.h | 61 +++++++ src/mongo/db/query/parsed_distinct.cpp | 2 +- src/mongo/db/query/parsed_distinct_test.cpp | 10 +- src/mongo/db/query/plan_cache_test.cpp | 3 +- src/mongo/db/query/query_planner_test_fixture.cpp | 6 +- src/mongo/db/query/query_request.cpp | 42 +---- src/mongo/db/query/query_request.h | 27 ++- src/mongo/db/query/query_request_test.cpp | 197 +++++++++++---------- .../periodic_sharded_index_consistency_checker.cpp | 4 +- src/mongo/db/service_entry_point_common.cpp | 5 +- src/mongo/dbtests/query_stage_subplan.cpp | 2 +- src/mongo/s/balancer_configuration_test.cpp | 2 +- .../s/catalog/sharding_catalog_client_test.cpp | 24 +-- .../catalog/sharding_catalog_write_retry_test.cpp | 3 +- src/mongo/s/catalog_cache_refresh_test.cpp | 8 +- src/mongo/s/cluster_identity_loader_test.cpp | 2 +- src/mongo/s/commands/cluster_aggregate_test.cpp | 2 +- src/mongo/s/commands/cluster_count_cmd.cpp | 14 +- src/mongo/s/commands/cluster_distinct_cmd.cpp | 13 +- src/mongo/s/commands/cluster_find_cmd.cpp | 18 +- src/mongo/s/commands/cluster_pipeline_cmd.cpp | 5 +- src/mongo/s/commands/strategy.cpp | 4 +- src/mongo/s/query/cluster_find.cpp | 3 +- src/mongo/s/query/results_merger_test_fixture.h | 3 +- src/mongo/s/sessions_collection_sharded.cpp | 3 +- src/mongo/s/sharding_router_test_fixture.cpp | 2 +- 55 files changed, 570 insertions(+), 321 deletions(-) create mode 100644 src/mongo/db/query/max_time_ms_parser.cpp create mode 100644 src/mongo/db/query/max_time_ms_parser.h (limited to 'src') diff --git a/src/mongo/client/dbclient_cursor.h b/src/mongo/client/dbclient_cursor.h index 75d9b1807aa..b40474bbd6f 100644 --- a/src/mongo/client/dbclient_cursor.h +++ b/src/mongo/client/dbclient_cursor.h @@ -35,13 +35,13 @@ #include "mongo/db/jsobj.h" #include "mongo/db/json.h" #include "mongo/db/namespace_string.h" -#include "mongo/db/pipeline/aggregate_command_gen.h" #include "mongo/rpc/message.h" namespace mongo { class AScopedConnection; class DBClientBase; +class AggregateCommand; /** Queries return a cursor object */ class DBClientCursor { diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp index 51f638ebc4a..e4f4e21bdc5 100644 --- a/src/mongo/db/auth/authorization_session_impl.cpp +++ b/src/mongo/db/auth/authorization_session_impl.cpp @@ -93,7 +93,9 @@ Status checkAuthForCreateOrModifyView(AuthorizationSession* authzSession, auto status = aggregation_request_helper::parseFromBSON( viewNs, BSON("aggregate" << viewOnNs.coll() << "pipeline" << viewPipeline << "cursor" << BSONObj() - << "$db" << viewOnNs.db())); + << "$db" << viewOnNs.db()), + boost::none, + false); if (!status.isOK()) return status.getStatus(); diff --git a/src/mongo/db/auth/authorization_session_test.cpp b/src/mongo/db/auth/authorization_session_test.cpp index 318a7cd509e..994c82850c0 100644 --- a/src/mongo/db/auth/authorization_session_test.cpp +++ b/src/mongo/db/auth/authorization_session_test.cpp @@ -595,7 +595,7 @@ TEST_F(AuthorizationSessionTest, AcquireUserObtainsAndValidatesAuthenticationRes } TEST_F(AuthorizationSessionTest, CannotAggregateEmptyPipelineWithoutFindAction) { - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << BSONArray() << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -607,7 +607,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateEmptyPipelineWithoutFindAction) TEST_F(AuthorizationSessionTest, CanAggregateEmptyPipelineWithFindAction) { authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << BSONArray() << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -623,7 +623,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateWithoutFindActionIfFirstStageNot BSONArray pipeline = BSON_ARRAY(BSON("$limit" << 1) << BSON("$collStats" << BSONObj()) << BSON("$indexStats" << BSONObj())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -637,7 +637,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateWithFindActionIfPipelineContains BSONArray pipeline = BSON_ARRAY(BSON("$limit" << 1) << BSON("$collStats" << BSONObj()) << BSON("$indexStats" << BSONObj())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -650,7 +650,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateCollStatsWithoutCollStatsAction) authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$collStats" << BSONObj())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -663,7 +663,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateCollStatsWithCollStatsAction) { authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::collStats})); BSONArray pipeline = BSON_ARRAY(BSON("$collStats" << BSONObj())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -676,7 +676,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateIndexStatsWithoutIndexStatsActio authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$indexStats" << BSONObj())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -689,7 +689,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateIndexStatsWithIndexStatsAction) { authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::indexStats})); BSONArray pipeline = BSON_ARRAY(BSON("$indexStats" << BSONObj())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -702,7 +702,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateCurrentOpAllUsersFalseWithoutInprog authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << false))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -715,7 +715,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersFalseWithoutInp authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << false))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -726,7 +726,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersFalseWithoutInp TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersFalseIfNotAuthenticatedOnMongoD) { BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << false))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -735,7 +735,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersFalseIfNotAuthe TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersFalseIfNotAuthenticatedOnMongoS) { BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << false))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -749,7 +749,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersTrueWithoutInpr authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << true))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -762,7 +762,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateCurrentOpAllUsersTrueWithoutInpr authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << true))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -776,7 +776,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateCurrentOpAllUsersTrueWithInprogActi Privilege(ResourcePattern::forClusterResource(), {ActionType::inprog})); BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << true))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -790,7 +790,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateCurrentOpAllUsersTrueWithInprogActi Privilege(ResourcePattern::forClusterResource(), {ActionType::inprog})); BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << true))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -804,7 +804,7 @@ TEST_F(AuthorizationSessionTest, CannotSpoofAllUsersTrueWithoutInprogActionOnMon BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << false << "allUsers" << true))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -818,7 +818,7 @@ TEST_F(AuthorizationSessionTest, CannotSpoofAllUsersTrueWithoutInprogActionOnMon BSONArray pipeline = BSON_ARRAY(BSON("$currentOp" << BSON("allUsers" << false << "allUsers" << true))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -832,7 +832,7 @@ TEST_F(AuthorizationSessionTest, AddPrivilegesForStageFailsIfOutNamespaceIsNotVa BSONArray pipeline = BSON_ARRAY(BSON("$out" << "")); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -846,7 +846,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateOutWithoutInsertAndRemoveOnTarge authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$out" << testBarNss.coll())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -871,7 +871,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateOutWithInsertAndRemoveOnTargetNames Privilege(testBarCollResource, {ActionType::insert, ActionType::remove})}); BSONArray pipeline = BSON_ARRAY(BSON("$out" << testBarNss.coll())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -880,7 +880,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateOutWithInsertAndRemoveOnTargetNames ASSERT_TRUE(authzSession->isAuthorizedForPrivileges(privileges)); auto aggNoBypassDocumentValidationReq = - uassertStatusOK(aggregation_request_helper::parseFromBSON( + uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "bypassDocumentValidation" << false << "cursor" << BSONObj() @@ -898,7 +898,7 @@ TEST_F(AuthorizationSessionTest, Privilege(testBarCollResource, {ActionType::insert, ActionType::remove})}); BSONArray pipeline = BSON_ARRAY(BSON("$out" << testBarNss.coll())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "bypassDocumentValidation" << true << "$db" << testFooNss.db()))); @@ -916,7 +916,7 @@ TEST_F(AuthorizationSessionTest, {ActionType::insert, ActionType::remove, ActionType::bypassDocumentValidation})}); BSONArray pipeline = BSON_ARRAY(BSON("$out" << testBarNss.coll())); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "bypassDocumentValidation" << true << "$db" << testFooNss.db()))); @@ -929,7 +929,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateLookupWithoutFindOnJoinedNamespa authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$lookup" << BSON("from" << testBarNss.coll()))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -943,7 +943,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateLookupWithFindOnJoinedNamespace) { Privilege(testBarCollResource, {ActionType::find})}); BSONArray pipeline = BSON_ARRAY(BSON("$lookup" << BSON("from" << testBarNss.coll()))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -960,7 +960,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateLookupWithoutFindOnNestedJoinedN BSONArray nestedPipeline = BSON_ARRAY(BSON("$lookup" << BSON("from" << testQuxNss.coll()))); BSONArray pipeline = BSON_ARRAY( BSON("$lookup" << BSON("from" << testBarNss.coll() << "pipeline" << nestedPipeline))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -977,7 +977,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateLookupWithFindOnNestedJoinedNamespa BSONArray nestedPipeline = BSON_ARRAY(BSON("$lookup" << BSON("from" << testQuxNss.coll()))); BSONArray pipeline = BSON_ARRAY( BSON("$lookup" << BSON("from" << testBarNss.coll() << "pipeline" << nestedPipeline))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -1024,8 +1024,8 @@ TEST_F(AuthorizationSessionTest, CheckAuthForAggregateWithDeeplyNestedLookup) { pipelineBuilder.doneFast(); cmdBuilder << "cursor" << BSONObj() << "$db" << testFooNss.db(); - auto aggReq = - uassertStatusOK(aggregation_request_helper::parseFromBSON(testFooNss, cmdBuilder.obj())); + auto aggReq = uassertStatusOK( + aggregation_request_helper::parseFromBSONForTests(testFooNss, cmdBuilder.obj())); PrivilegeVector privileges = uassertStatusOK(authzSession->getPrivilegesForAggregate(testFooNss, aggReq, false)); ASSERT_TRUE(authzSession->isAuthorizedForPrivileges(privileges)); @@ -1036,7 +1036,7 @@ TEST_F(AuthorizationSessionTest, CannotAggregateGraphLookupWithoutFindOnJoinedNa authzSession->assumePrivilegesForDB(Privilege(testFooCollResource, {ActionType::find})); BSONArray pipeline = BSON_ARRAY(BSON("$graphLookup" << BSON("from" << testBarNss.coll()))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -1050,7 +1050,7 @@ TEST_F(AuthorizationSessionTest, CanAggregateGraphLookupWithFindOnJoinedNamespac Privilege(testBarCollResource, {ActionType::find})}); BSONArray pipeline = BSON_ARRAY(BSON("$graphLookup" << BSON("from" << testBarNss.coll()))); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -1067,7 +1067,7 @@ TEST_F(AuthorizationSessionTest, BSONArray pipeline = BSON_ARRAY(fromjson("{$facet: {lookup: [{$lookup: {from: 'bar'}}], graphLookup: " "[{$graphLookup: {from: 'qux'}}]}}")); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); @@ -1096,7 +1096,7 @@ TEST_F(AuthorizationSessionTest, BSON_ARRAY(fromjson("{$facet: {lookup: [{$lookup: {from: 'bar'}}], graphLookup: " "[{$graphLookup: {from: 'qux'}}]}}")); - auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSON( + auto aggReq = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests( testFooNss, BSON("aggregate" << testFooNss.coll() << "pipeline" << pipeline << "cursor" << BSONObj() << "$db" << testFooNss.db()))); diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp index 4fb68a2535d..1cfbd968f5f 100644 --- a/src/mongo/db/catalog/index_key_validate.cpp +++ b/src/mongo/db/catalog/index_key_validate.cpp @@ -263,6 +263,7 @@ StatusWith validateIndexSpec( bool hasNamespaceField = false; bool hasVersionField = false; bool hasCollationField = false; + bool apiStrict = opCtx && APIParameters::get(opCtx).getAPIStrict().value_or(false); auto fieldNamesValidStatus = validateIndexSpecFieldNames(indexSpec); if (!fieldNamesValidStatus.isOK()) { @@ -297,12 +298,19 @@ StatusWith validateIndexSpec( // 'validateKeyPattern()'. It must currently be done here so that haystack indexes // continue to replicate correctly before the upgrade to FCV "4.9" is complete. const auto keyPattern = indexSpecElem.Obj(); - if (IndexNames::findPluginName(keyPattern) == IndexNames::GEO_HAYSTACK) { + const auto indexName = IndexNames::findPluginName(keyPattern); + if (indexName == IndexNames::GEO_HAYSTACK) { return {ErrorCodes::CannotCreateIndex, str::stream() << "GeoHaystack indexes cannot be created in version 4.9 and above"}; } + if (apiStrict && indexName == IndexNames::TEXT) { + return {ErrorCodes::APIStrictError, + str::stream() + << indexName << " indexes cannot be created with apiStrict: true"}; + } + // Here we always validate the key pattern according to the most recent rules, in order // to enforce that all new indexes have well-formed key patterns. Status keyPatternValidateStatus = diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index 5dcd2d94b77..723866e5292 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -46,7 +46,7 @@ #include "mongo/db/commands/server_status_metric.h" #include "mongo/db/commands/test_commands_enabled.h" #include "mongo/db/jsobj.h" -#include "mongo/db/query/explain.h" +#include "mongo/db/query/explain_options.h" #include "mongo/db/read_concern_support_result.h" #include "mongo/db/repl/read_concern_args.h" #include "mongo/db/request_execution_context.h" diff --git a/src/mongo/db/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp index 3bb4c6031f1..1164eb1e07c 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -163,8 +163,11 @@ public: auto viewAggCmd = OpMsgRequest::fromDBAndBody(nss.db(), viewAggregation.getValue()).body; - auto viewAggRequest = - aggregation_request_helper::parseFromBSON(nss, viewAggCmd, verbosity); + auto viewAggRequest = aggregation_request_helper::parseFromBSON( + nss, + viewAggCmd, + verbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false)); if (!viewAggRequest.isOK()) { return viewAggRequest.getStatus(); } diff --git a/src/mongo/db/commands/distinct.cpp b/src/mongo/db/commands/distinct.cpp index 70a710ff044..53c8497beee 100644 --- a/src/mongo/db/commands/distinct.cpp +++ b/src/mongo/db/commands/distinct.cpp @@ -161,8 +161,11 @@ public: auto viewAggCmd = OpMsgRequest::fromDBAndBody(nss.db(), viewAggregation.getValue()).body; - auto viewAggRequest = - aggregation_request_helper::parseFromBSON(nss, viewAggCmd, verbosity); + auto viewAggRequest = aggregation_request_helper::parseFromBSON( + nss, + viewAggCmd, + verbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false)); if (!viewAggRequest.isOK()) { return viewAggRequest.getStatus(); } diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp index 7e51b1538d6..32c6cdcb0b8 100644 --- a/src/mongo/db/commands/find_cmd.cpp +++ b/src/mongo/db/commands/find_cmd.cpp @@ -70,7 +70,11 @@ std::unique_ptr parseCmdObjectToQueryRequest(OperationContext* opC NamespaceString nss, BSONObj cmdObj, bool isExplain) { - auto qr = QueryRequest::makeFromFindCommand(std::move(cmdObj), isExplain, std::move(nss)); + auto qr = + QueryRequest::makeFromFindCommand(std::move(cmdObj), + isExplain, + std::move(nss), + APIParameters::get(opCtx).getAPIStrict().value_or(false)); if (!qr->getLegacyRuntimeConstants()) { qr->setLegacyRuntimeConstants(Variables::generateRuntimeConstants(opCtx)); } @@ -270,8 +274,11 @@ public: auto viewAggCmd = OpMsgRequest::fromDBAndBody(_dbName, viewAggregationCommand).body; // Create the agg request equivalent of the find operation, with the explain // verbosity included. - auto aggRequest = uassertStatusOK( - aggregation_request_helper::parseFromBSON(nss, viewAggCmd, verbosity)); + auto aggRequest = uassertStatusOK(aggregation_request_helper::parseFromBSON( + nss, + viewAggCmd, + verbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false))); try { // An empty PrivilegeVector is acceptable because these privileges are only diff --git a/src/mongo/db/commands/mr_common.h b/src/mongo/db/commands/mr_common.h index c3e80c11874..5e5c6dc55e7 100644 --- a/src/mongo/db/commands/mr_common.h +++ b/src/mongo/db/commands/mr_common.h @@ -36,6 +36,7 @@ #include "mongo/db/commands/map_reduce_gen.h" #include "mongo/db/jsobj.h" #include "mongo/db/namespace_string.h" +#include "mongo/db/pipeline/pipeline.h" namespace mongo::map_reduce_common { diff --git a/src/mongo/db/commands/pipeline_command.cpp b/src/mongo/db/commands/pipeline_command.cpp index 6535d11c2b1..341c42d1aca 100644 --- a/src/mongo/db/commands/pipeline_command.cpp +++ b/src/mongo/db/commands/pipeline_command.cpp @@ -73,7 +73,10 @@ public: const OpMsgRequest& opMsgRequest, boost::optional explainVerbosity) override { const auto aggregationRequest = uassertStatusOK(aggregation_request_helper::parseFromBSON( - opMsgRequest.getDatabase().toString(), opMsgRequest.body, explainVerbosity)); + opMsgRequest.getDatabase().toString(), + opMsgRequest.body, + explainVerbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false))); auto privileges = uassertStatusOK(AuthorizationSession::get(opCtx->getClient()) diff --git a/src/mongo/db/create_indexes.idl b/src/mongo/db/create_indexes.idl index 204e505a6cc..f91fb724213 100644 --- a/src/mongo/db/create_indexes.idl +++ b/src/mongo/db/create_indexes.idl @@ -85,6 +85,7 @@ structs: background: type: safeBool optional: true + unstable: true unique: type: safeBool optional: true @@ -97,12 +98,14 @@ structs: sparse: type: safeBool optional: true + unstable: true expireAfterSeconds: type: safeInt optional: true storageEngine: type: object_owned optional: true + unstable: true weights: type: object_owned optional: true @@ -130,6 +133,7 @@ structs: bucketSize: type: safeDouble optional: true + unstable: true collation: type: object_owned optional: true @@ -173,4 +177,3 @@ commands: description: 'Commit Quorum options' type: CommitQuorum optional: true - diff --git a/src/mongo/db/error_labels.cpp b/src/mongo/db/error_labels.cpp index 7ac35e006fd..2caa2c6c6fe 100644 --- a/src/mongo/db/error_labels.cpp +++ b/src/mongo/db/error_labels.cpp @@ -98,11 +98,12 @@ bool ErrorLabelBuilder::isResumableChangeStreamError() const { // Get the namespace string from CurOp. We will need it to build the LiteParsedPipeline. const auto nss = NamespaceString{CurOp::get(_opCtx)->getNS()}; + bool apiStrict = APIParameters::get(_opCtx).getAPIStrict().value_or(false); // Do enough parsing to confirm that this is a well-formed pipeline with a $changeStream. - const auto swLitePipe = [&nss, &cmdObj]() -> StatusWith { + const auto swLitePipe = [&nss, &cmdObj, apiStrict]() -> StatusWith { try { - auto aggRequest = - uassertStatusOK(aggregation_request_helper::parseFromBSON(nss, cmdObj)); + auto aggRequest = uassertStatusOK( + aggregation_request_helper::parseFromBSON(nss, cmdObj, boost::none, apiStrict)); return LiteParsedPipeline(aggRequest); } catch (const DBException& ex) { return ex.toStatus(); diff --git a/src/mongo/db/error_labels_test.cpp b/src/mongo/db/error_labels_test.cpp index 8a2545cec10..07b597ffaf6 100644 --- a/src/mongo/db/error_labels_test.cpp +++ b/src/mongo/db/error_labels_test.cpp @@ -293,7 +293,8 @@ TEST_F(ErrorLabelBuilderTest, ResumableChangeStreamErrorAppliesToChangeStreamAgg auto cmdObj = BSON("aggregate" << nss().coll() << "pipeline" << BSON_ARRAY(BSON("$changeStream" << BSONObj())) << "cursor" << BSONObj() << "$db" << nss().db()); - auto aggRequest = uassertStatusOK(aggregation_request_helper::parseFromBSON(nss(), cmdObj)); + auto aggRequest = + uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(nss(), cmdObj)); ASSERT_TRUE(LiteParsedPipeline(aggRequest).hasChangeStream()); // The label applies to a $changeStream "aggregate" command. @@ -317,7 +318,8 @@ TEST_F(ErrorLabelBuilderTest, ResumableChangeStreamErrorDoesNotApplyToNonResumab auto cmdObj = BSON("aggregate" << nss().coll() << "pipeline" << BSON_ARRAY(BSON("$changeStream" << BSONObj())) << "cursor" << BSONObj() << "$db" << nss().db()); - auto aggRequest = uassertStatusOK(aggregation_request_helper::parseFromBSON(nss(), cmdObj)); + auto aggRequest = + uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(nss(), cmdObj)); ASSERT_TRUE(LiteParsedPipeline(aggRequest).hasChangeStream()); // The label does not apply to a ChangeStreamFatalError error on a $changeStream aggregation. @@ -341,7 +343,8 @@ TEST_F(ErrorLabelBuilderTest, ResumableChangeStreamErrorDoesNotApplyToNonChangeS auto cmdObj = BSON("aggregate" << nss().coll() << "pipeline" << BSON_ARRAY(BSON("$match" << BSONObj())) << "cursor" << BSONObj() << "$db" << nss().db()); - auto aggRequest = uassertStatusOK(aggregation_request_helper::parseFromBSON(nss(), cmdObj)); + auto aggRequest = + uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(nss(), cmdObj)); ASSERT_FALSE(LiteParsedPipeline(aggRequest).hasChangeStream()); // The label does not apply to a non-$changeStream "aggregate" command. diff --git a/src/mongo/db/matcher/expression_optimize_test.cpp b/src/mongo/db/matcher/expression_optimize_test.cpp index f34ac14890e..e46b9aaddba 100644 --- a/src/mongo/db/matcher/expression_optimize_test.cpp +++ b/src/mongo/db/matcher/expression_optimize_test.cpp @@ -283,7 +283,7 @@ TEST(ExpressionOptimizeTest, IsValidGeoNearNaturalHint) { TEST(ExpressionOptimizeTest, IsValidNaturalSortIndexHint) { const bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand( + auto qr = QueryRequest::makeFromFindCommandForTests( fromjson("{find: 'testcoll', sort: {$natural: 1}, hint: {a: 1}, '$db': 'test'}"), isExplain); @@ -293,7 +293,7 @@ TEST(ExpressionOptimizeTest, IsValidNaturalSortIndexHint) { TEST(ExpressionOptimizeTest, IsValidNaturalSortNaturalHint) { const bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand( + auto qr = QueryRequest::makeFromFindCommandForTests( fromjson("{find: 'testcoll', sort: {$natural: 1}, hint: {$natural: 1}, '$db': 'test'}"), isExplain); @@ -303,7 +303,7 @@ TEST(ExpressionOptimizeTest, IsValidNaturalSortNaturalHint) { TEST(ExpressionOptimizeTest, IsValidNaturalSortNaturalHintDifferentDirections) { const bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand( + auto qr = QueryRequest::makeFromFindCommandForTests( fromjson("{find: 'testcoll', sort: {$natural: 1}, hint: {$natural: -1}, '$db': 'test'}"), isExplain); diff --git a/src/mongo/db/pipeline/SConscript b/src/mongo/db/pipeline/SConscript index 378744e7d31..4a26c0aa54f 100644 --- a/src/mongo/db/pipeline/SConscript +++ b/src/mongo/db/pipeline/SConscript @@ -39,7 +39,7 @@ env.Library( target='aggregation_request_helper', source=[ 'aggregation_request_helper.cpp', - env.Idlc('aggregate_command.idl')[0] + 'aggregate_command.idl', ], LIBDEPS=[ '$BUILD_DIR/mongo/base', diff --git a/src/mongo/db/pipeline/aggregate_command.idl b/src/mongo/db/pipeline/aggregate_command.idl index 80f83a4d1f9..10c3bf0b7e2 100644 --- a/src/mongo/db/pipeline/aggregate_command.idl +++ b/src/mongo/db/pipeline/aggregate_command.idl @@ -66,8 +66,8 @@ commands: command_name: aggregate strict: true namespace: concatenate_with_db - api_version: "" allow_global_collection_name: true + api_version: "1" fields: pipeline: description: "An unparsed version of the pipeline." @@ -119,32 +119,37 @@ commands: description: "True if this request originated from a mongoS." type: optionalBool - # TODO: Mark the undocumented parameters below as 'unstable'. $queryOptions: description: "The unwrapped readPreference object, if one was given to us by the mongos command processor. This object will be empty when no readPreference is specified or if the request does not originate from mongos." cpp_name: unwrappedReadPref type: object_owned optional: true + unstable: true $_requestReshardingResumeToken: description: "True if this requests resharding resume token." cpp_name: requestReshardingResumeToken type: optionalBool + unstable: true exchange: description: "An optional exchange specification for this request. If set it means that the request represents a producer running as a part of the exchange machinery. This is an internal option; we do not expect it to be set on requests from users or drivers." type: ExchangeSpec optional: true + unstable: true runtimeConstants: description: "A legacy way to specify constant variables available during execution. 'let' is now preferred." type: LegacyRuntimeConstants cpp_name: legacyRuntimeConstants optional: true + unstable: true isMapReduceCommand: description: "True if an aggregation was invoked by the MapReduce command." type: optionalBool + unstable: true collectionUUID: description: "The expected UUID of the namespace the aggregation executes on." type: uuid optional: true + unstable: true use44SortKeys: # TODO SERVER-47065: A 5.0 node still has to accept the 'use44SortKeys' field, since it # could be included in a command sent from a 4.4 mongos or 4.4 mongod. In 5.1, this @@ -152,6 +157,7 @@ commands: description: "If true, this aggregation will use the 4.4 sort key format." type: bool ignore: true + unstable: true useNewUpsert: # TODO SERVER-46751: we must retain the ability to ingest the 'useNewUpsert' field for # 5.0 upgrade purposes, since a 4.4 mongoS will always send {useNewUpsert:true} to the @@ -161,3 +167,7 @@ commands: description: "A flag to indicate wether or not to use new upsert option." type: bool ignore: true + unstable: true + # TODO SERVER-51622: This OkReply is just used as default to be able to mark "unstable" + # fields, and should be replaced when converting the output of aggregate command to IDL. + reply_type: OkReply diff --git a/src/mongo/db/pipeline/aggregation_request_helper.cpp b/src/mongo/db/pipeline/aggregation_request_helper.cpp index 0a51de0b871..0dfba6d8133 100644 --- a/src/mongo/db/pipeline/aggregation_request_helper.cpp +++ b/src/mongo/db/pipeline/aggregation_request_helper.cpp @@ -49,14 +49,32 @@ namespace aggregation_request_helper { StatusWith parseFromBSON( const std::string& dbName, const BSONObj& cmdObj, - boost::optional explainVerbosity) { - return parseFromBSON(parseNs(dbName, cmdObj), cmdObj, explainVerbosity); + boost::optional explainVerbosity, + bool apiStrict) { + return parseFromBSON(parseNs(dbName, cmdObj), cmdObj, explainVerbosity, apiStrict); +} + +StatusWith parseFromBSONForTests( + NamespaceString nss, + const BSONObj& cmdObj, + boost::optional explainVerbosity, + bool apiStrict) { + return parseFromBSON(nss, cmdObj, explainVerbosity, apiStrict); +} + +StatusWith parseFromBSONForTests( + const std::string& dbName, + const BSONObj& cmdObj, + boost::optional explainVerbosity, + bool apiStrict) { + return parseFromBSON(dbName, cmdObj, explainVerbosity, apiStrict); } StatusWith parseFromBSON( NamespaceString nss, const BSONObj& cmdObj, - boost::optional explainVerbosity) { + boost::optional explainVerbosity, + bool apiStrict) { // if the command object lacks field 'aggregate' or '$db', we will use the namespace in 'nss'. bool cmdObjChanged = false; @@ -70,7 +88,7 @@ StatusWith parseFromBSON( AggregateCommand request(nss); try { - request = AggregateCommand::parse(IDLParserErrorContext("aggregate"), + request = AggregateCommand::parse(IDLParserErrorContext("aggregate", apiStrict), cmdObjChanged ? cmdObjBob.obj() : cmdObj); } catch (const AssertionException&) { return exceptionToStatus(); diff --git a/src/mongo/db/pipeline/aggregation_request_helper.h b/src/mongo/db/pipeline/aggregation_request_helper.h index 7c04dfea823..203e7d41d25 100644 --- a/src/mongo/db/pipeline/aggregation_request_helper.h +++ b/src/mongo/db/pipeline/aggregation_request_helper.h @@ -69,7 +69,14 @@ static constexpr long long kDefaultBatchSize = 101; StatusWith parseFromBSON( NamespaceString nss, const BSONObj& cmdObj, - boost::optional explainVerbosity = boost::none); + boost::optional explainVerbosity, + bool apiStrict); + +StatusWith parseFromBSONForTests( + NamespaceString nss, + const BSONObj& cmdObj, + boost::optional explainVerbosity = boost::none, + bool apiStrict = false); /** * Convenience overload which constructs the request's NamespaceString from the given database @@ -78,7 +85,14 @@ StatusWith parseFromBSON( StatusWith parseFromBSON( const std::string& dbName, const BSONObj& cmdObj, - boost::optional explainVerbosity = boost::none); + boost::optional explainVerbosity, + bool apiStrict); + +StatusWith parseFromBSONForTests( + const std::string& dbName, + const BSONObj& cmdObj, + boost::optional explainVerbosity = boost::none, + bool apiStrict = false); /* * The first field in 'cmdObj' must be a string representing a valid collection name, or the diff --git a/src/mongo/db/pipeline/aggregation_request_test.cpp b/src/mongo/db/pipeline/aggregation_request_test.cpp index 95e0e9316f6..8dac4105fb3 100644 --- a/src/mongo/db/pipeline/aggregation_request_test.cpp +++ b/src/mongo/db/pipeline/aggregation_request_test.cpp @@ -72,7 +72,8 @@ TEST(AggregationRequestTest, ShouldParseAllKnownOptions) { uuid.appendToBuilder(&uuidBob, AggregateCommand::kCollectionUUIDFieldName); inputBson = inputBson.addField(uuidBob.obj().firstElement()); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON(nss, inputBson)); + auto request = + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests(nss, inputBson)); ASSERT_FALSE(request.getExplain()); ASSERT_TRUE(request.getAllowDiskUse()); ASSERT_TRUE(request.getFromMongos()); @@ -103,7 +104,8 @@ TEST(AggregationRequestTest, ShouldParseExplicitRequestReshardingResumeTokenFals const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], $_requestReshardingResumeToken: false, cursor: " "{}, $db: 'a'}"); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON(nss, inputBson)); + auto request = + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests(nss, inputBson)); ASSERT_FALSE(request.getRequestReshardingResumeToken()); } @@ -111,7 +113,8 @@ TEST(AggregationRequestTest, ShouldParseExplicitExplainTrue) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], explain: true, cursor: {}, $db: 'a'}"); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON(nss, inputBson)); + auto request = + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests(nss, inputBson)); ASSERT_TRUE(request.getExplain()); ASSERT(*request.getExplain() == ExplainOptions::Verbosity::kQueryPlanner); } @@ -121,7 +124,8 @@ TEST(AggregationRequestTest, ShouldParseExplicitExplainFalseWithCursorOption) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], explain: false, cursor: {batchSize: 10}, $db: " "'a'}"); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON(nss, inputBson)); + auto request = + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests(nss, inputBson)); ASSERT_FALSE(request.getExplain()); ASSERT_EQ( request.getCursor().getBatchSize().value_or(aggregation_request_helper::kDefaultBatchSize), @@ -132,7 +136,7 @@ TEST(AggregationRequestTest, ShouldParseWithSeparateQueryPlannerExplainModeArg) NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], cursor: {}, $db: 'a'}"); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON( + auto request = unittest::assertGet(aggregation_request_helper::parseFromBSONForTests( nss, inputBson, ExplainOptions::Verbosity::kQueryPlanner)); ASSERT_TRUE(request.getExplain()); ASSERT(*request.getExplain() == ExplainOptions::Verbosity::kQueryPlanner); @@ -142,7 +146,7 @@ TEST(AggregationRequestTest, ShouldParseWithSeparateQueryPlannerExplainModeArgAn NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], cursor: {batchSize: 10}, $db: 'a'}"); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON( + auto request = unittest::assertGet(aggregation_request_helper::parseFromBSONForTests( nss, inputBson, ExplainOptions::Verbosity::kExecStats)); ASSERT_TRUE(request.getExplain()); ASSERT(*request.getExplain() == ExplainOptions::Verbosity::kExecStats); @@ -158,7 +162,8 @@ TEST(AggregationRequestTest, ShouldParseExplainFlagWithReadConcern) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], explain: true, readConcern: {level: 'majority'}, " "$db: 'a'}"); - auto request = unittest::assertGet(aggregation_request_helper::parseFromBSON(nss, inputBson)); + auto request = + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests(nss, inputBson)); ASSERT_TRUE(request.getExplain()); ASSERT_BSONOBJ_EQ(*request.getReadConcern(), BSON("level" @@ -266,7 +271,7 @@ TEST(AggregationRequestTest, ShouldSetBatchSizeToDefaultOnEmptyCursorObject) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, $db: 'a'}"); - auto request = aggregation_request_helper::parseFromBSON(nss, inputBson); + auto request = aggregation_request_helper::parseFromBSONForTests(nss, inputBson); ASSERT_OK(request.getStatus()); ASSERT_EQ(request.getValue().getCursor().getBatchSize().value_or( aggregation_request_helper::kDefaultBatchSize), @@ -278,7 +283,7 @@ TEST(AggregationRequestTest, ShouldAcceptHintAsString) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], hint: 'a_1', cursor: {}, $db: " "'a'}"); - auto request = aggregation_request_helper::parseFromBSON(nss, inputBson); + auto request = aggregation_request_helper::parseFromBSONForTests(nss, inputBson); ASSERT_OK(request.getStatus()); ASSERT_BSONOBJ_EQ(request.getValue().getHint().value_or(BSONObj()), BSON("$hint" @@ -309,17 +314,17 @@ TEST(AggregationRequestTest, ShouldRejectNonArrayPipeline) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: {}, cursor: {}, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectPipelineArrayIfAnElementIsNotAnObject) { NamespaceString nss("a.collection"); BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [4], cursor: {}, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}, 4], cursor: {}, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonObjectCollation) { @@ -327,9 +332,9 @@ TEST(AggregationRequestTest, ShouldRejectNonObjectCollation) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, collation: 1, " "$db: 'a'}"); - ASSERT_NOT_OK( - aggregation_request_helper::parseFromBSON(NamespaceString("a.collection"), inputBson) - .getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(NamespaceString("a.collection"), + inputBson) + .getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonStringNonObjectHint) { @@ -337,9 +342,9 @@ TEST(AggregationRequestTest, ShouldRejectNonStringNonObjectHint) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, hint: 1, $db: " "'a'}"); - ASSERT_NOT_OK( - aggregation_request_helper::parseFromBSON(NamespaceString("a.collection"), inputBson) - .getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(NamespaceString("a.collection"), + inputBson) + .getStatus()); } TEST(AggregationRequestTest, ShouldRejectHintAsArray) { @@ -347,9 +352,9 @@ TEST(AggregationRequestTest, ShouldRejectHintAsArray) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, hint: [], $db: " "'a'}]}"); - ASSERT_NOT_OK( - aggregation_request_helper::parseFromBSON(NamespaceString("a.collection"), inputBson) - .getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(NamespaceString("a.collection"), + inputBson) + .getStatus()); } TEST(AggregationRequestTest, ShouldRejectExplainIfNumber) { @@ -357,7 +362,7 @@ TEST(AggregationRequestTest, ShouldRejectExplainIfNumber) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, explain: 1, $db: " "'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectExplainIfObject) { @@ -365,7 +370,7 @@ TEST(AggregationRequestTest, ShouldRejectExplainIfObject) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, explain: {}, $db: " "'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonBoolFromMongos) { @@ -373,7 +378,7 @@ TEST(AggregationRequestTest, ShouldRejectNonBoolFromMongos) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, fromMongos: 1, " "$db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonBoolNeedsMerge) { @@ -381,7 +386,7 @@ TEST(AggregationRequestTest, ShouldRejectNonBoolNeedsMerge) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, needsMerge: 1, " "fromMongos: true, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNeedsMergeIfFromMongosNotPresent) { @@ -389,7 +394,7 @@ TEST(AggregationRequestTest, ShouldRejectNeedsMergeIfFromMongosNotPresent) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, needsMerge: true, " "$db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonBoolNeedsMerge34) { @@ -397,7 +402,7 @@ TEST(AggregationRequestTest, ShouldRejectNonBoolNeedsMerge34) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, fromRouter: 1, " "$db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNeedsMergeIfNeedsMerge34AlsoPresent) { @@ -406,7 +411,7 @@ TEST(AggregationRequestTest, ShouldRejectNeedsMergeIfNeedsMerge34AlsoPresent) { "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, needsMerge: true, " "fromMongos: true, " "fromRouter: true, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectFromMongosIfNeedsMerge34AlsoPresent) { @@ -414,7 +419,7 @@ TEST(AggregationRequestTest, ShouldRejectFromMongosIfNeedsMerge34AlsoPresent) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, fromMongos: true, " "fromRouter: true, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonBoolAllowDiskUse) { @@ -422,7 +427,7 @@ TEST(AggregationRequestTest, ShouldRejectNonBoolAllowDiskUse) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, allowDiskUse: 1, " "$db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNonBoolIsMapReduceCommand) { @@ -430,21 +435,21 @@ TEST(AggregationRequestTest, ShouldRejectNonBoolIsMapReduceCommand) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, " "isMapReduceCommand: 1, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectNoCursorNoExplain) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectExplainTrueWithSeparateExplainArg) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], explain: true, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON( + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests( nss, inputBson, ExplainOptions::Verbosity::kExecStats) .getStatus()); } @@ -453,7 +458,7 @@ TEST(AggregationRequestTest, ShouldRejectExplainFalseWithSeparateExplainArg) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], explain: false, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON( + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests( nss, inputBson, ExplainOptions::Verbosity::kExecStats) .getStatus()); } @@ -462,7 +467,7 @@ TEST(AggregationRequestTest, ShouldRejectExplainExecStatsVerbosityWithReadConcer NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], readConcern: {level: 'majority'}, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON( + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests( nss, inputBson, ExplainOptions::Verbosity::kExecStats) .getStatus()); } @@ -472,14 +477,14 @@ TEST(AggregationRequestTest, ShouldRejectExplainWithWriteConcernMajority) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], explain: true, writeConcern: {w: 'majority'}, " "$db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectExplainExecStatsVerbosityWithWriteConcernMajority) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], writeConcern: {w: 'majority'}, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON( + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests( nss, inputBson, ExplainOptions::Verbosity::kExecStats) .getStatus()); } @@ -488,7 +493,7 @@ TEST(AggregationRequestTest, ShouldRejectRequestReshardingResumeTokenIfNonOplogN NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [], $_requestReshardingResumeToken: true, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, CannotParseNeedsMerge34) { @@ -496,7 +501,7 @@ TEST(AggregationRequestTest, CannotParseNeedsMerge34) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, fromRouter: true, " "$db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ParseNSShouldReturnAggregateOneNSIfAggregateFieldIsOne) { @@ -545,8 +550,9 @@ TEST(AggregationRequestTest, ParseFromBSONOverloadsShouldProduceIdenticalRequest NamespaceString nss("a.collection"); auto aggReqDBName = - unittest::assertGet(aggregation_request_helper::parseFromBSON("a", inputBSON)); - auto aggReqNSS = unittest::assertGet(aggregation_request_helper::parseFromBSON(nss, inputBSON)); + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests("a", inputBSON)); + auto aggReqNSS = + unittest::assertGet(aggregation_request_helper::parseFromBSONForTests(nss, inputBSON)); ASSERT_DOCUMENT_EQ(aggregation_request_helper::serializeToCommandDoc(aggReqDBName), aggregation_request_helper::serializeToCommandDoc(aggReqNSS)); @@ -556,14 +562,14 @@ TEST(AggregationRequestTest, ShouldRejectExchangeNotObject) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], exchage: '42', $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectExchangeInvalidSpec) { NamespaceString nss("a.collection"); const BSONObj inputBson = fromjson("{aggregate: 'collection', pipeline: [], exchage: {}, $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectInvalidWriteConcern) { @@ -571,15 +577,16 @@ TEST(AggregationRequestTest, ShouldRejectInvalidWriteConcern) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, writeConcern: " "'invalid', $db: 'a'}"); - ASSERT_NOT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_NOT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } TEST(AggregationRequestTest, ShouldRejectInvalidCollectionUUID) { NamespaceString nss("a.collection"); const BSONObj inputBSON = fromjson( "{aggregate: 'collection', pipeline: [{$match: {}}], collectionUUID: 2, $db: 'a'}"); - ASSERT_EQUALS(aggregation_request_helper::parseFromBSON(nss, inputBSON).getStatus().code(), - ErrorCodes::TypeMismatch); + ASSERT_EQUALS( + aggregation_request_helper::parseFromBSONForTests(nss, inputBSON).getStatus().code(), + ErrorCodes::TypeMismatch); } // @@ -591,7 +598,7 @@ TEST(AggregationRequestTest, ShouldIgnoreQueryOptions) { const BSONObj inputBson = fromjson( "{aggregate: 'collection', pipeline: [{$match: {a: 'abc'}}], cursor: {}, $queryOptions: " "{}, $db: 'a'}"); - ASSERT_OK(aggregation_request_helper::parseFromBSON(nss, inputBson).getStatus()); + ASSERT_OK(aggregation_request_helper::parseFromBSONForTests(nss, inputBson).getStatus()); } } // namespace diff --git a/src/mongo/db/pipeline/document_source_change_stream.cpp b/src/mongo/db/pipeline/document_source_change_stream.cpp index 7b80cb18d02..78d510b625d 100644 --- a/src/mongo/db/pipeline/document_source_change_stream.cpp +++ b/src/mongo/db/pipeline/document_source_change_stream.cpp @@ -34,6 +34,7 @@ #include "mongo/bson/simple_bsonelement_comparator.h" #include "mongo/db/bson/bson_helper.h" #include "mongo/db/commands/feature_compatibility_version_documentation.h" +#include "mongo/db/pipeline/aggregate_command_gen.h" #include "mongo/db/pipeline/change_stream_constants.h" #include "mongo/db/pipeline/document_path_support.h" #include "mongo/db/pipeline/document_source_change_stream_close_cursor.h" diff --git a/src/mongo/db/pipeline/expression_context.cpp b/src/mongo/db/pipeline/expression_context.cpp index 01d34d83cac..417e02815db 100644 --- a/src/mongo/db/pipeline/expression_context.cpp +++ b/src/mongo/db/pipeline/expression_context.cpp @@ -31,6 +31,7 @@ #include +#include "mongo/db/pipeline/aggregate_command_gen.h" #include "mongo/db/pipeline/expression_context.h" #include "mongo/db/pipeline/process_interface/stub_mongo_process_interface.h" #include "mongo/db/query/collation/collation_spec.h" diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h index 67fa0adcd50..33696061ecc 100644 --- a/src/mongo/db/pipeline/expression_context.h +++ b/src/mongo/db/pipeline/expression_context.h @@ -42,7 +42,6 @@ #include "mongo/db/exec/document_value/value_comparator.h" #include "mongo/db/namespace_string.h" #include "mongo/db/operation_context.h" -#include "mongo/db/pipeline/aggregate_command_gen.h" #include "mongo/db/pipeline/javascript_execution.h" #include "mongo/db/pipeline/legacy_runtime_constants_gen.h" #include "mongo/db/pipeline/process_interface/mongo_process_interface.h" @@ -58,6 +57,8 @@ namespace mongo { +class AggregateCommand; + class ExpressionContext : public RefCountable { public: static constexpr size_t kMaxSubPipelineViewDepth = 20; diff --git a/src/mongo/db/query/SConscript b/src/mongo/db/query/SConscript index 8938d57b904..6633ad8c4d4 100644 --- a/src/mongo/db/query/SConscript +++ b/src/mongo/db/query/SConscript @@ -177,6 +177,7 @@ env.Library( "distinct_command.idl", "find_command.idl", "query_request.cpp", + "max_time_ms_parser.cpp", "tailable_mode.cpp", "tailable_mode.idl", ], diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp index 41c5512e52f..b40f31d6f60 100644 --- a/src/mongo/db/query/canonical_query_test.cpp +++ b/src/mongo/db/query/canonical_query_test.cpp @@ -99,7 +99,7 @@ TEST(CanonicalQueryTest, IsValidSortKeyMetaProjection) { // Passing a sortKey meta-projection without a sort is an error. { const bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand( + auto qr = QueryRequest::makeFromFindCommandForTests( fromjson("{find: 'testcoll', projection: {foo: {$meta: 'sortKey'}}, '$db': 'test'}"), isExplain); auto cq = CanonicalQuery::canonicalize(opCtx.get(), std::move(qr)); @@ -109,7 +109,7 @@ TEST(CanonicalQueryTest, IsValidSortKeyMetaProjection) { // Should be able to successfully create a CQ when there is a sort. { const bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand( + auto qr = QueryRequest::makeFromFindCommandForTests( fromjson("{find: 'testcoll', projection: {foo: {$meta: 'sortKey'}}, sort: {bar: 1}, " "'$db': 'test'}"), isExplain); @@ -273,7 +273,7 @@ TEST(CanonicalQueryTest, CanonicalizeFromBaseQuery) { const std::string cmdStr = "{find:'bogusns', filter:{$or:[{a:1,b:1},{a:1,c:1}]}, projection:{a:1}, sort:{b:1}, '$db': " "'test'}"; - auto qr = QueryRequest::makeFromFindCommand(fromjson(cmdStr), isExplain); + auto qr = QueryRequest::makeFromFindCommandForTests(fromjson(cmdStr), isExplain); auto baseCq = assertGet(CanonicalQuery::canonicalize(opCtx.get(), std::move(qr))); MatchExpression* firstClauseExpr = baseCq->root()->getChild(0); diff --git a/src/mongo/db/query/count_command_test.cpp b/src/mongo/db/query/count_command_test.cpp index 65264d2a6e5..5792ee2e12b 100644 --- a/src/mongo/db/query/count_command_test.cpp +++ b/src/mongo/db/query/count_command_test.cpp @@ -164,7 +164,7 @@ TEST(CountCommandTest, ConvertToAggregationWithHint) { auto agg = uassertStatusOK(countCommandAsAggregationCommand(countCmd, testns)); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg).body; - auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSON(testns, cmdObj)); + auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(testns, cmdObj)); ASSERT_BSONOBJ_EQ(ar.getHint().value_or(BSONObj()), BSON("x" << 1)); std::vector expectedPipeline{BSON("$count" @@ -186,7 +186,7 @@ TEST(CountCommandTest, ConvertToAggregationWithQueryAndFilterAndLimit) { auto agg = uassertStatusOK(countCommandAsAggregationCommand(countCmd, testns)); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg).body; - auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSON(testns, cmdObj)); + auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(testns, cmdObj)); ASSERT_EQ(ar.getCursor().getBatchSize().value_or(aggregation_request_helper::kDefaultBatchSize), aggregation_request_helper::kDefaultBatchSize); ASSERT_EQ(ar.getNamespace(), testns); @@ -212,7 +212,7 @@ TEST(CountCommandTest, ConvertToAggregationWithMaxTimeMS) { auto agg = uassertStatusOK(countCommandAsAggregationCommand(countCmd, testns)); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg).body; - auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSON(testns, cmdObj)); + auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(testns, cmdObj)); ASSERT_EQ(ar.getMaxTimeMS().value_or(0), 100u); std::vector expectedPipeline{BSON("$count" @@ -235,7 +235,7 @@ TEST(CountCommandTest, ConvertToAggregationWithQueryOptions) { auto agg = uassertStatusOK(countCommandAsAggregationCommand(countCmd, testns)); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg).body; - auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSON(testns, cmdObj)); + auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(testns, cmdObj)); ASSERT_BSONOBJ_EQ(ar.getUnwrappedReadPref().value_or(BSONObj()), BSON("readPreference" << "secondary")); @@ -260,7 +260,7 @@ TEST(CountCommandTest, ConvertToAggregationWithReadConcern) { auto agg = uassertStatusOK(countCommandAsAggregationCommand(countCmd, testns)); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg).body; - auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSON(testns, cmdObj)); + auto ar = uassertStatusOK(aggregation_request_helper::parseFromBSONForTests(testns, cmdObj)); ASSERT_BSONOBJ_EQ(ar.getReadConcern().value_or(BSONObj()), BSON("level" << "linearizable")); diff --git a/src/mongo/db/query/count_request.cpp b/src/mongo/db/query/count_request.cpp index 8d5b47370eb..93a645841e3 100644 --- a/src/mongo/db/query/count_request.cpp +++ b/src/mongo/db/query/count_request.cpp @@ -61,7 +61,7 @@ long long countParseSkip(const BSONElement& element) { } long long countParseMaxTime(const BSONElement& element) { - auto maxTimeVal = uassertStatusOK(QueryRequest::parseMaxTimeMS(element)); + auto maxTimeVal = uassertStatusOK(parseMaxTimeMS(element)); return static_cast(maxTimeVal); } } // namespace count_request diff --git a/src/mongo/db/query/find_command.idl b/src/mongo/db/query/find_command.idl index 86b95c93bcf..fe2cc20be23 100644 --- a/src/mongo/db/query/find_command.idl +++ b/src/mongo/db/query/find_command.idl @@ -33,6 +33,7 @@ global: cpp_namespace: "mongo" cpp_includes: - "mongo/db/namespace_string.h" + - "mongo/db/query/max_time_ms_parser.h" imports: - "mongo/db/logical_session_id.idl" @@ -59,7 +60,7 @@ types: bson_serialization_type: any description: "An int representing max time ms." cpp_type: "std::int32_t" - deserializer: "::mongo::QueryRequest::parseMaxTimeMSForIDL" + deserializer: "::mongo::parseMaxTimeMSForIDL" commands: find: @@ -68,8 +69,8 @@ commands: description: "A struct representing the find command" strict: true namespace: concatenate_with_db_or_uuid - api_version: "" non_const_getter: true + api_version: "1" fields: filter: description: "The query predicate. If unspecified, then all documents in the collection @@ -106,6 +107,7 @@ commands: description: "Deprecated. Should use batchSize." type: safeInt64 optional: true + unstable: true singleBatch: description: "Determines whether to close the cursor after the first batch." type: optionalBool @@ -116,33 +118,42 @@ commands: min: description: "The inclusive lower bound for a specific index." type: object_owned_nonempty_serialize + unstable: true max: description: "The exclusive upper bound for a specific index." type: object_owned_nonempty_serialize + unstable: true returnKey: description: "If true, returns only the index keys in the resulting documents." type: optionalBool + unstable: true showRecordId: description: "Determines whether to return the record identifier for each document." type: optionalBool + unstable: true $queryOptions: description: "Deprecated. A mechanism to specify readPreference." cpp_name: unwrappedReadPref type: object_owned_nonempty_serialize + unstable: true tailable: description: "Returns a tailable cursor for a capped collections." type: optionalBool + unstable: true oplogReplay: description: "Deprecated. An internal command for replaying a replica set’s oplog." type: boolNoOpSerializer optional: true + unstable: true noCursorTimeout: description: "Prevents the server from timing out idle cursors after an inactivity period." type: optionalBool + unstable: true awaitData: description: "Use in conjunction with the tailable option to block a getMore command on the cursor temporarily if at the end of data rather than returning no data." type: optionalBool + unstable: true allowPartialResults: description: "For queries against a sharded collection, allows the command (or subsequent getMore commands) to return partial results, rather than an error, if one or more queried @@ -156,25 +167,31 @@ commands: description: "Deprecated." type: object_owned optional: true + unstable: true term: description: "Deprecated." type: safeInt64 optional: true + unstable: true readOnce: description: "Deprecated." type: optionalBool + unstable: true allowSpeculativeMajorityRead: description: "Deprecated." type: optionalBool + unstable: true $_requestResumeToken: description: "Deprecated." cpp_name: requestResumeToken type: optionalBool + unstable: true $_resumeAfter: description: "Deprecated." cpp_name: resumeAfter type: object_owned_nonempty_serialize default: mongo::BSONObj() + unstable: true _use44SortKeys: description: "An internal parameter used to determine the serialization format for sort keys. TODO SERVER-47065: A 4.7+ node still has to accept the '_use44SortKeys' field, since @@ -182,6 +199,7 @@ commands: code to tolerate the '_use44SortKeys' field can be deleted." type: bool optional: true + unstable: true maxTimeMS: description: "The cumulative time limit in milliseconds for processing operations on the cursor." @@ -196,3 +214,6 @@ commands: cpp_name: legacyRuntimeConstants type: LegacyRuntimeConstants optional: true + # TODO SERVER-51622: This OkReply is just used as default to be able to mark "unstable" fields, + # and should be replaced when converting the output of find command to IDL. + reply_type: OkReply diff --git a/src/mongo/db/query/getmore_request.cpp b/src/mongo/db/query/getmore_request.cpp index 14027c9245d..5327b9e6592 100644 --- a/src/mongo/db/query/getmore_request.cpp +++ b/src/mongo/db/query/getmore_request.cpp @@ -140,7 +140,7 @@ StatusWith GetMoreRequest::parseFromBSON(const std::string& dbna batchSize = el.numberLong(); } else if (fieldName == kAwaitDataTimeoutField) { - auto maxAwaitDataTime = QueryRequest::parseMaxTimeMS(el); + auto maxAwaitDataTime = parseMaxTimeMS(el); if (!maxAwaitDataTime.isOK()) { return maxAwaitDataTime.getStatus(); } diff --git a/src/mongo/db/query/max_time_ms_parser.cpp b/src/mongo/db/query/max_time_ms_parser.cpp new file mode 100644 index 00000000000..7f2c84f07ad --- /dev/null +++ b/src/mongo/db/query/max_time_ms_parser.cpp @@ -0,0 +1,68 @@ +/** + * 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 + * . + * + * 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. + */ + +#include "mongo/db/query/max_time_ms_parser.h" + +#include "mongo/bson/bsonobjbuilder.h" +#include "mongo/util/assert_util.h" + +namespace mongo { + +StatusWith parseMaxTimeMS(BSONElement maxTimeMSElt) { + if (!maxTimeMSElt.eoo() && !maxTimeMSElt.isNumber()) { + return StatusWith( + ErrorCodes::BadValue, + (StringBuilder() << maxTimeMSElt.fieldNameStringData() << " must be a number").str()); + } + long long maxTimeMSLongLong = maxTimeMSElt.safeNumberLong(); // returns 0 on EOO + + const long long maxVal = maxTimeMSElt.fieldNameStringData() == kMaxTimeMSOpOnlyField + ? (long long)(INT_MAX) + kMaxTimeMSOpOnlyMaxPadding + : INT_MAX; + if (maxTimeMSLongLong < 0 || maxTimeMSLongLong > maxVal) { + return StatusWith(ErrorCodes::BadValue, + (StringBuilder() + << maxTimeMSLongLong << " value for " + << maxTimeMSElt.fieldNameStringData() << " is out of range") + .str()); + } + double maxTimeMSDouble = maxTimeMSElt.numberDouble(); + if (maxTimeMSElt.type() == mongo::NumberDouble && floor(maxTimeMSDouble) != maxTimeMSDouble) { + return StatusWith( + ErrorCodes::BadValue, + (StringBuilder() << maxTimeMSElt.fieldNameStringData() << " has non-integral value") + .str()); + } + return StatusWith(static_cast(maxTimeMSLongLong)); +} + +int32_t parseMaxTimeMSForIDL(BSONElement maxTimeMSElt) { + return static_cast(uassertStatusOK(parseMaxTimeMS(maxTimeMSElt))); +} +} // namespace mongo diff --git a/src/mongo/db/query/max_time_ms_parser.h b/src/mongo/db/query/max_time_ms_parser.h new file mode 100644 index 00000000000..1e5ec985c77 --- /dev/null +++ b/src/mongo/db/query/max_time_ms_parser.h @@ -0,0 +1,61 @@ +/** + * 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 + * . + * + * 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. + */ + + +#pragma once + +#include "mongo/bson/bsonelement.h" +#include "mongo/bson/bsonobj.h" + +namespace mongo { + +static constexpr auto kMaxTimeMSOpOnlyField = "maxTimeMSOpOnly"; + +// A constant by which 'maxTimeMSOpOnly' values are allowed to exceed the max allowed value for +// 'maxTimeMS'. This is because mongod and mongos server processes add a small amount to the +// 'maxTimeMS' value they are given before passing it on as 'maxTimeMSOpOnly', to allow for +// clock precision. +static constexpr auto kMaxTimeMSOpOnlyMaxPadding = 100LL; + +/** + * Parses maxTimeMS from the BSONElement containing its value. + * The field name of the 'maxTimeMSElt' is used to determine what maximum value to enforce for + * the provided max time. 'maxTimeMSOpOnly' needs a slightly higher max value than regular + * 'maxTimeMS' to account for the case where a user provides the max possible value for + * 'maxTimeMS' to one server process (mongod or mongos), then that server process passes the max + * time on to another server as 'maxTimeMSOpOnly', but after adding a small amount to the max + * time to account for clock precision. This can push the 'maxTimeMSOpOnly' sent to the mongod + * over the max value allowed for users to provide. This is safe because 'maxTimeMSOpOnly' is + * only allowed to be provided for internal intra-cluster requests. + */ +StatusWith parseMaxTimeMS(BSONElement maxTimeMSElt); + +int32_t parseMaxTimeMSForIDL(BSONElement maxTimeMSElt); + +} // namespace mongo diff --git a/src/mongo/db/query/parsed_distinct.cpp b/src/mongo/db/query/parsed_distinct.cpp index 08647e6bf53..dc751b3eead 100644 --- a/src/mongo/db/query/parsed_distinct.cpp +++ b/src/mongo/db/query/parsed_distinct.cpp @@ -304,7 +304,7 @@ StatusWith ParsedDistinct::parse(OperationContext* opCtx, } if (auto maxTimeMSElt = cmdObj[QueryRequest::cmdOptionMaxTimeMS]) { - auto maxTimeMS = QueryRequest::parseMaxTimeMS(maxTimeMSElt); + auto maxTimeMS = parseMaxTimeMS(maxTimeMSElt); if (!maxTimeMS.isOK()) { return maxTimeMS.getStatus(); } diff --git a/src/mongo/db/query/parsed_distinct_test.cpp b/src/mongo/db/query/parsed_distinct_test.cpp index 54bf37496d1..88a083ade41 100644 --- a/src/mongo/db/query/parsed_distinct_test.cpp +++ b/src/mongo/db/query/parsed_distinct_test.cpp @@ -60,7 +60,7 @@ TEST(ParsedDistinctTest, ConvertToAggregationNoQuery) { ASSERT_OK(agg); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, cmdObj); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, cmdObj); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getCursor().getBatchSize().value_or( @@ -102,7 +102,7 @@ TEST(ParsedDistinctTest, ConvertToAggregationDottedPathNoQuery) { ASSERT_OK(agg); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, cmdObj); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, cmdObj); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getCursor().getBatchSize().value_or( @@ -169,7 +169,7 @@ TEST(ParsedDistinctTest, ConvertToAggregationWithAllOptions) { ASSERT_OK(agg); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, cmdObj); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, cmdObj); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getCursor().getBatchSize().value_or( @@ -218,7 +218,7 @@ TEST(ParsedDistinctTest, ConvertToAggregationWithQuery) { ASSERT_OK(agg); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, cmdObj); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, cmdObj); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getCursor().getBatchSize().value_or( @@ -263,7 +263,7 @@ TEST(ParsedDistinctTest, ExplainNotIncludedWhenConvertingToAggregationCommand) { ASSERT_FALSE(agg.getValue().hasField("explain")); auto cmdObj = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, cmdObj); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, cmdObj); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getNamespace(), testns); diff --git a/src/mongo/db/query/plan_cache_test.cpp b/src/mongo/db/query/plan_cache_test.cpp index e465979baaa..9df05d614bf 100644 --- a/src/mongo/db/query/plan_cache_test.cpp +++ b/src/mongo/db/query/plan_cache_test.cpp @@ -1049,7 +1049,8 @@ protected: solns.clear(); const bool isExplain = false; - std::unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + std::unique_ptr qr( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); const boost::intrusive_ptr expCtx; auto statusWithCQ = diff --git a/src/mongo/db/query/query_planner_test_fixture.cpp b/src/mongo/db/query/query_planner_test_fixture.cpp index 9d2929d2b33..520544baedc 100644 --- a/src/mongo/db/query/query_planner_test_fixture.cpp +++ b/src/mongo/db/query/query_planner_test_fixture.cpp @@ -443,7 +443,8 @@ void QueryPlannerTest::runQueryAsCommand(const BSONObj& cmdObj) { // If there is no '$db', append it. auto cmd = OpMsgRequest::fromDBAndBody(nss.db(), cmdObj).body; - std::unique_ptr qr(QueryRequest::makeFromFindCommand(cmd, isExplain, nss)); + std::unique_ptr qr( + QueryRequest::makeFromFindCommandForTests(cmd, isExplain, nss)); auto statusWithCQ = CanonicalQuery::canonicalize(opCtx.get(), @@ -468,7 +469,8 @@ void QueryPlannerTest::runInvalidQueryAsCommand(const BSONObj& cmdObj) { // If there is no '$db', append it. auto cmd = OpMsgRequest::fromDBAndBody(nss.db(), cmdObj).body; - std::unique_ptr qr(QueryRequest::makeFromFindCommand(cmd, isExplain, nss)); + std::unique_ptr qr( + QueryRequest::makeFromFindCommandForTests(cmd, isExplain, nss)); auto statusWithCQ = CanonicalQuery::canonicalize(opCtx.get(), diff --git a/src/mongo/db/query/query_request.cpp b/src/mongo/db/query/query_request.cpp index 90c7bf0fcf6..0d023b0dc75 100644 --- a/src/mongo/db/query/query_request.cpp +++ b/src/mongo/db/query/query_request.cpp @@ -68,9 +68,10 @@ void QueryRequest::refreshNSS(const NamespaceString& nss) { // static std::unique_ptr QueryRequest::makeFromFindCommand( - const BSONObj& cmdObj, bool isExplain, boost::optional nss) { + const BSONObj& cmdObj, bool isExplain, boost::optional nss, bool apiStrict) { + auto qr = std::make_unique( - FindCommand::parse(IDLParserErrorContext("FindCommand"), cmdObj)); + FindCommand::parse(IDLParserErrorContext("FindCommand", apiStrict), cmdObj)); // If there is an explicit namespace specified overwite it. if (nss) { @@ -93,6 +94,11 @@ std::unique_ptr QueryRequest::makeFromFindCommand( return qr; } +std::unique_ptr QueryRequest::makeFromFindCommandForTests( + const BSONObj& cmdObj, bool isExplain, boost::optional nss, bool apiStrict) { + return makeFromFindCommand(cmdObj, isExplain, nss, apiStrict); +} + BSONObj QueryRequest::asFindCommand() const { BSONObjBuilder bob; asFindCommand(&bob); @@ -206,38 +212,6 @@ Status QueryRequest::validate() const { } // static -StatusWith QueryRequest::parseMaxTimeMS(BSONElement maxTimeMSElt) { - if (!maxTimeMSElt.eoo() && !maxTimeMSElt.isNumber()) { - return StatusWith( - ErrorCodes::BadValue, - (StringBuilder() << maxTimeMSElt.fieldNameStringData() << " must be a number").str()); - } - long long maxTimeMSLongLong = maxTimeMSElt.safeNumberLong(); // returns 0 on EOO - - const long long maxVal = maxTimeMSElt.fieldNameStringData() == kMaxTimeMSOpOnlyField - ? (long long)(INT_MAX) + kMaxTimeMSOpOnlyMaxPadding - : INT_MAX; - if (maxTimeMSLongLong < 0 || maxTimeMSLongLong > maxVal) { - return StatusWith(ErrorCodes::BadValue, - (StringBuilder() - << maxTimeMSLongLong << " value for " - << maxTimeMSElt.fieldNameStringData() << " is out of range") - .str()); - } - double maxTimeMSDouble = maxTimeMSElt.numberDouble(); - if (maxTimeMSElt.type() == mongo::NumberDouble && floor(maxTimeMSDouble) != maxTimeMSDouble) { - return StatusWith( - ErrorCodes::BadValue, - (StringBuilder() << maxTimeMSElt.fieldNameStringData() << " has non-integral value") - .str()); - } - return StatusWith(static_cast(maxTimeMSLongLong)); -} - -int32_t QueryRequest::parseMaxTimeMSForIDL(BSONElement maxTimeMSElt) { - return static_cast(uassertStatusOK(QueryRequest::parseMaxTimeMS(maxTimeMSElt))); -} - bool QueryRequest::isTextScoreMeta(BSONElement elt) { // elt must be foo: {$meta: "textScore"} if (mongo::Object != elt.type()) { diff --git a/src/mongo/db/query/query_request.h b/src/mongo/db/query/query_request.h index 37d12b0d04f..bb06302c612 100644 --- a/src/mongo/db/query/query_request.h +++ b/src/mongo/db/query/query_request.h @@ -78,8 +78,16 @@ public: * Returns a heap allocated QueryRequest on success or an error if 'cmdObj' is not well * formed. */ - static std::unique_ptr makeFromFindCommand( - const BSONObj& cmdObj, bool isExplain, boost::optional nss = boost::none); + static std::unique_ptr makeFromFindCommand(const BSONObj& cmdObj, + bool isExplain, + boost::optional nss, + bool apiStrict); + + static std::unique_ptr makeFromFindCommandForTests( + const BSONObj& cmdObj, + bool isExplain, + boost::optional nss = boost::none, + bool apiStrict = false); /** * If _uuid exists for this QueryRequest, update the value of _nss. @@ -108,21 +116,6 @@ public: */ StatusWith asAggregationCommand() const; - /** - * Parses maxTimeMS from the BSONElement containing its value. - * The field name of the 'maxTimeMSElt' is used to determine what maximum value to enforce for - * the provided max time. 'maxTimeMSOpOnly' needs a slightly higher max value than regular - * 'maxTimeMS' to account for the case where a user provides the max possible value for - * 'maxTimeMS' to one server process (mongod or mongos), then that server process passes the max - * time on to another server as 'maxTimeMSOpOnly', but after adding a small amount to the max - * time to account for clock precision. This can push the 'maxTimeMSOpOnly' sent to the mongod - * over the max value allowed for users to provide. This is safe because 'maxTimeMSOpOnly' is - * only allowed to be provided for internal intra-cluster requests. - */ - static StatusWith parseMaxTimeMS(BSONElement maxTimeMSElt); - - static int32_t parseMaxTimeMSForIDL(BSONElement maxTimeMSElt); - /** * Helper function to identify text search sort key * Example: {a: {$meta: "textScore"}} diff --git a/src/mongo/db/query/query_request_test.cpp b/src/mongo/db/query/query_request_test.cpp index 26d014a996b..adee5c43cfc 100644 --- a/src/mongo/db/query/query_request_test.cpp +++ b/src/mongo/db/query/query_request_test.cpp @@ -198,8 +198,9 @@ TEST(QueryRequestTest, ForbidTailableWithNonNaturalSort) { "sort: {a: 1}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } TEST(QueryRequestTest, ForbidTailableWithSingleBatch) { @@ -209,8 +210,9 @@ TEST(QueryRequestTest, ForbidTailableWithSingleBatch) { "singleBatch: true, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } TEST(QueryRequestTest, AllowTailableWithNaturalSort) { @@ -220,7 +222,7 @@ TEST(QueryRequestTest, AllowTailableWithNaturalSort) { "sort: {$natural: 1}, '$db': 'test'}"); bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand(cmdObj, isExplain); + auto qr = QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); ASSERT_TRUE(qr->isTailable()); ASSERT_BSONOBJ_EQ(qr->getSort(), BSON("$natural" << 1)); } @@ -383,7 +385,7 @@ TEST(QueryRequestTest, ParseFromCommandBasic) { "projection: {_id: 0, a: 1}, '$db': 'test'}"); bool isExplain = false; - QueryRequest::makeFromFindCommand(cmdObj, isExplain); + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); } TEST(QueryRequestTest, ParseFromCommandWithOptions) { @@ -395,7 +397,7 @@ TEST(QueryRequestTest, ParseFromCommandWithOptions) { "showRecordId: true, '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); // Make sure the values from the command BSON are reflected in the QR. ASSERT(qr->showRecordId()); @@ -408,7 +410,7 @@ TEST(QueryRequestTest, ParseFromCommandHintAsString) { "hint: 'foo_1', '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); BSONObj hintObj = qr->getHint(); ASSERT_BSONOBJ_EQ(BSON("$hint" @@ -423,7 +425,7 @@ TEST(QueryRequestTest, ParseFromCommandValidSortProj) { "sort: {a: 1}, '$db': 'test'}"); bool isExplain = false; - QueryRequest::makeFromFindCommand(cmdObj, isExplain); + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); } TEST(QueryRequestTest, ParseFromCommandValidSortProjMeta) { @@ -433,7 +435,7 @@ TEST(QueryRequestTest, ParseFromCommandValidSortProjMeta) { "sort: {a: {$meta: 'textScore'}}, '$db': 'test'}"); bool isExplain = false; - QueryRequest::makeFromFindCommand(cmdObj, isExplain); + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); } TEST(QueryRequestTest, ParseFromCommandAllFlagsTrue) { @@ -447,7 +449,7 @@ TEST(QueryRequestTest, ParseFromCommandAllFlagsTrue) { "allowSpeculativeMajorityRead: true, '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); // Test that all the flags got set to true. ASSERT(qr->isTailable()); @@ -466,7 +468,7 @@ TEST(QueryRequestTest, OplogReplayFlagIsAllowedButIgnored) { << "test"); const bool isExplain = false; const NamespaceString nss{"test.testns"}; - auto qr = QueryRequest::makeFromFindCommand(cmdObj, isExplain); + auto qr = QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); // Verify that the 'oplogReplay' flag does not appear if we reserialize the request. auto reserialized = qr->asFindCommand(); @@ -480,7 +482,7 @@ TEST(QueryRequestTest, ParseFromCommandReadOnceDefaultsToFalse) { BSONObj cmdObj = fromjson("{find: 'testns', '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(!qr->isReadOnce()); } @@ -492,7 +494,7 @@ TEST(QueryRequestTest, ParseFromCommandValidMinMax) { "max: {a: 2}, '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); BSONObj expectedMin = BSON("a" << 1); ASSERT_EQUALS(0, expectedMin.woCompare(qr->getMin())); BSONObj expectedMax = BSON("a" << 2); @@ -518,7 +520,7 @@ TEST(QueryRequestTest, ParseFromCommandAllNonOptionFields) { .addField(rtcObj["runtimeConstants"]); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); // Check the values inside the QR. BSONObj expectedQuery = BSON("a" << 1); ASSERT_EQUALS(0, expectedQuery.woCompare(qr->getFilter())); @@ -552,7 +554,7 @@ TEST(QueryRequestTest, ParseFromCommandLargeLimit) { "limit: 8000000000, '$db': 'test'}"); // 8 * 1000 * 1000 * 1000 const bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_EQUALS(8LL * 1000 * 1000 * 1000, *qr->getLimit()); } @@ -564,7 +566,7 @@ TEST(QueryRequestTest, ParseFromCommandLargeBatchSize) { "batchSize: 8000000000, '$db': 'test'}"); // 8 * 1000 * 1000 * 1000 const bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_EQUALS(8LL * 1000 * 1000 * 1000, *qr->getBatchSize()); } @@ -576,7 +578,7 @@ TEST(QueryRequestTest, ParseFromCommandLargeSkip) { "skip: 8000000000, '$db': 'test'}"); // 8 * 1000 * 1000 * 1000 const bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_EQUALS(8LL * 1000 * 1000 * 1000, *qr->getSkip()); } @@ -591,7 +593,7 @@ TEST(QueryRequestTest, ParseFromCommandQueryWrongType) { "filter: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -603,7 +605,7 @@ TEST(QueryRequestTest, ParseFromCommandSortWrongType) { "sort: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -616,7 +618,7 @@ TEST(QueryRequestTest, ParseFromCommandProjWrongType) { "projection: 'foo', '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -630,7 +632,7 @@ TEST(QueryRequestTest, ParseFromCommandSkipWrongType) { "projection: {a: 1}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -644,7 +646,7 @@ TEST(QueryRequestTest, ParseFromCommandLimitWrongType) { "projection: {a: 1}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -658,7 +660,7 @@ TEST(QueryRequestTest, ParseFromCommandSingleBatchWrongType) { "projection: {a: 1}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -671,7 +673,7 @@ TEST(QueryRequestTest, ParseFromCommandUnwrappedReadPrefWrongType) { "$queryOptions: 1, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -684,8 +686,9 @@ TEST(QueryRequestTest, ParseFromCommandMaxTimeMSWrongType) { "maxTimeMS: true, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } @@ -696,7 +699,7 @@ TEST(QueryRequestTest, ParseFromCommandMaxWrongType) { "max: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -709,7 +712,7 @@ TEST(QueryRequestTest, ParseFromCommandMinWrongType) { "min: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -721,7 +724,7 @@ TEST(QueryRequestTest, ParseFromCommandReturnKeyWrongType) { "returnKey: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -733,7 +736,7 @@ TEST(QueryRequestTest, ParseFromCommandShowRecordIdWrongType) { "showRecordId: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -745,7 +748,7 @@ TEST(QueryRequestTest, ParseFromCommandTailableWrongType) { "tailable: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -757,7 +760,8 @@ TEST(QueryRequestTest, ParseFromCommandSlaveOkWrongType) { "slaveOk: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, 40415); + ASSERT_THROWS_CODE( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, 40415); } TEST(QueryRequestTest, ParseFromCommandOplogReplayWrongType) { @@ -767,7 +771,7 @@ TEST(QueryRequestTest, ParseFromCommandOplogReplayWrongType) { "oplogReplay: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -779,7 +783,7 @@ TEST(QueryRequestTest, ParseFromCommandNoCursorTimeoutWrongType) { "noCursorTimeout: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -792,7 +796,7 @@ TEST(QueryRequestTest, ParseFromCommandAwaitDataWrongType) { "awaitData: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -805,7 +809,8 @@ TEST(QueryRequestTest, ParseFromCommandExhaustWrongType) { "exhaust: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, 40415); + ASSERT_THROWS_CODE( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, 40415); } @@ -816,7 +821,7 @@ TEST(QueryRequestTest, ParseFromCommandPartialWrongType) { "allowPartialResults: 3, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -828,7 +833,7 @@ TEST(QueryRequestTest, ParseFromCommandReadConcernWrongType) { "readConcern: 'foo', '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -840,7 +845,7 @@ TEST(QueryRequestTest, ParseFromCommandCollationWrongType) { "collation: 'foo', '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -851,7 +856,7 @@ TEST(QueryRequestTest, ParseFromCommandReadOnceWrongType) { "readOnce: 1, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -865,7 +870,7 @@ TEST(QueryRequestTest, ParseFromCommandLegacyRuntimeConstantsWrongType) { << "test"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::TypeMismatch); } @@ -881,7 +886,7 @@ TEST(QueryRequestTest, ParseFromCommandLegacyRuntimeConstantsSubfieldsWrongType) << "$db" << "test"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), AssertionException, ErrorCodes::TypeMismatch); } @@ -896,8 +901,9 @@ TEST(QueryRequestTest, ParseFromCommandNegativeSkipError) { "skip: -3," "filter: {a: 3}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } TEST(QueryRequestTest, ParseFromCommandSkipIsZero) { @@ -906,7 +912,7 @@ TEST(QueryRequestTest, ParseFromCommandSkipIsZero) { "skip: 0," "filter: {a: 3}, '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_BSONOBJ_EQ(BSON("a" << 3), qr->getFilter()); ASSERT_FALSE(qr->getSkip()); } @@ -917,8 +923,9 @@ TEST(QueryRequestTest, ParseFromCommandNegativeLimitError) { "limit: -3," "filter: {a: 3}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } TEST(QueryRequestTest, ParseFromCommandLimitIsZero) { @@ -927,7 +934,7 @@ TEST(QueryRequestTest, ParseFromCommandLimitIsZero) { "limit: 0," "filter: {a: 3}, '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_BSONOBJ_EQ(BSON("a" << 3), qr->getFilter()); ASSERT_FALSE(qr->getLimit()); } @@ -938,14 +945,15 @@ TEST(QueryRequestTest, ParseFromCommandNegativeBatchSizeError) { "batchSize: -10," "filter: {a: 3}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } TEST(QueryRequestTest, ParseFromCommandBatchSizeZero) { BSONObj cmdObj = fromjson("{find: 'testns', batchSize: 0, '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(qr->getBatchSize()); ASSERT_EQ(0, *qr->getBatchSize()); @@ -955,7 +963,7 @@ TEST(QueryRequestTest, ParseFromCommandBatchSizeZero) { TEST(QueryRequestTest, ParseFromCommandDefaultBatchSize) { BSONObj cmdObj = fromjson("{find: 'testns', '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(!qr->getBatchSize()); ASSERT(!qr->getLimit()); @@ -969,7 +977,7 @@ TEST(QueryRequestTest, ParseFromCommandRequestResumeToken) { << "test"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(qr->getRequestResumeToken()); } @@ -982,7 +990,7 @@ TEST(QueryRequestTest, ParseFromCommandResumeToken) { << "test"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(!qr->getResumeAfter().isEmpty()); ASSERT(qr->getRequestResumeToken()); } @@ -997,7 +1005,7 @@ TEST(QueryRequestTest, ParseFromCommandEmptyResumeToken) { << "test"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(qr->getRequestResumeToken()); ASSERT(qr->getResumeAfter().isEmpty()); } @@ -1024,7 +1032,7 @@ TEST(QueryRequestTest, AsFindCommandAllNonOptionFields) { .addField(storage["runtimeConstants"]); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_BSONOBJ_EQ(cmdObj.removeField("$db"), qr->asFindCommand()); } @@ -1048,7 +1056,7 @@ TEST(QueryRequestTest, AsFindCommandWithUuidAllNonOptionFields) { .addField(storage["runtimeConstants"]); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_BSONOBJ_EQ(cmdObj.removeField("$db"), qr->asFindCommand()); } @@ -1069,7 +1077,7 @@ TEST(QueryRequestTest, AsFindCommandWithResumeToken) { << "test"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_BSONOBJ_EQ(cmdObj.removeField("$db"), qr->asFindCommand()); } @@ -1082,7 +1090,7 @@ TEST(QueryRequestTest, AsFindCommandWithEmptyResumeToken) { << "$_requestResumeToken" << true << "$_resumeAfter" << resumeAfter << "$db" << "test"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT(qr->asFindCommand().getField("$_resumeAftr").eoo()); } @@ -1097,7 +1105,8 @@ TEST(QueryRequestTest, ParseFromCommandMinMaxDifferentFieldsError) { "min: {a: 3}," "max: {b: 4}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, 51176); + ASSERT_THROWS_CODE( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, 51176); } TEST(QueryRequestTest, ParseCommandAllowNonMetaSortOnFieldWithMetaProject) { @@ -1108,13 +1117,13 @@ TEST(QueryRequestTest, ParseCommandAllowNonMetaSortOnFieldWithMetaProject) { "projection: {a: {$meta: 'textScore'}}," "sort: {a: 1}, '$db': 'test'}"); bool isExplain = false; - QueryRequest::makeFromFindCommand(cmdObj, isExplain); + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); cmdObj = fromjson( "{find: 'testns'," "projection: {a: {$meta: 'textScore'}}," "sort: {b: 1}, '$db': 'test'}"); - QueryRequest::makeFromFindCommand(cmdObj, isExplain); + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); } TEST(QueryRequestTest, ParseCommandAllowMetaSortOnFieldWithoutMetaProject) { @@ -1126,26 +1135,27 @@ TEST(QueryRequestTest, ParseCommandAllowMetaSortOnFieldWithoutMetaProject) { "sort: {a: {$meta: 'textScore'}}, '$db': 'test'}"); bool isExplain = false; - auto qr = QueryRequest::makeFromFindCommand(cmdObj, isExplain); + auto qr = QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); cmdObj = fromjson( "{find: 'testns'," "projection: {b: 1}," "sort: {a: {$meta: 'textScore'}}, '$db': 'test'}"); - qr = QueryRequest::makeFromFindCommand(cmdObj, isExplain); + qr = QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); } TEST(QueryRequestTest, ParseCommandForbidExhaust) { BSONObj cmdObj = fromjson("{find: 'testns', exhaust: true, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, 40415); + ASSERT_THROWS_CODE( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, 40415); } TEST(QueryRequestTest, ParseCommandIsFromFindCommand) { BSONObj cmdObj = fromjson("{find: 'testns', '$db': 'test'}"); bool isExplain = false; - unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, isExplain)); + unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain)); ASSERT_FALSE(qr->getNToReturn()); } @@ -1153,7 +1163,7 @@ TEST(QueryRequestTest, ParseCommandIsFromFindCommand) { TEST(QueryRequestTest, ParseCommandAwaitDataButNotTailable) { BSONObj cmdObj = fromjson("{find: 'testns', awaitData: true, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, ErrorCodes::FailedToParse); } @@ -1161,20 +1171,21 @@ TEST(QueryRequestTest, ParseCommandAwaitDataButNotTailable) { TEST(QueryRequestTest, ParseCommandFirstFieldNotString) { BSONObj cmdObj = fromjson("{find: 1, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE( - QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, ErrorCodes::BadValue); + ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), + DBException, + ErrorCodes::BadValue); } TEST(QueryRequestTest, ParseCommandIgnoreShardVersionField) { BSONObj cmdObj = fromjson("{find: 'test.testns', shardVersion: 'foo', '$db': 'test'}"); bool isExplain = false; - QueryRequest::makeFromFindCommand(cmdObj, isExplain); + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); } TEST(QueryRequestTest, DefaultQueryParametersCorrect) { BSONObj cmdObj = fromjson("{find: 'testns', '$db': 'test'}"); - std::unique_ptr qr(QueryRequest::makeFromFindCommand(cmdObj, false)); + std::unique_ptr qr(QueryRequest::makeFromFindCommandForTests(cmdObj, false)); ASSERT_FALSE(qr->getSkip()); ASSERT_FALSE(qr->getLimit()); @@ -1200,7 +1211,7 @@ TEST(QueryRequestTest, ParseCommandAllowDiskUseTrue) { BSONObj cmdObj = fromjson("{find: 'testns', allowDiskUse: true, '$db': 'test'}"); const bool isExplain = false; - auto result = QueryRequest::makeFromFindCommand(cmdObj, isExplain); + auto result = QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); ASSERT_EQ(true, result->allowDiskUse()); } @@ -1209,7 +1220,7 @@ TEST(QueryRequestTest, ParseCommandAllowDiskUseFalse) { BSONObj cmdObj = fromjson("{find: 'testns', allowDiskUse: false, '$db': 'test'}"); const bool isExplain = false; - auto result = QueryRequest::makeFromFindCommand(cmdObj, isExplain); + auto result = QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain); ASSERT_EQ(false, result->allowDiskUse()); } @@ -1224,7 +1235,8 @@ TEST(QueryRequestTest, ParseFromCommandForbidExtraField) { "foo: {a: 1}, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, 40415); + ASSERT_THROWS_CODE( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, 40415); } TEST(QueryRequestTest, ParseFromCommandForbidExtraOption) { @@ -1233,39 +1245,40 @@ TEST(QueryRequestTest, ParseFromCommandForbidExtraOption) { "foo: true, '$db': 'test'}"); bool isExplain = false; - ASSERT_THROWS_CODE(QueryRequest::makeFromFindCommand(cmdObj, isExplain), DBException, 40415); + ASSERT_THROWS_CODE( + QueryRequest::makeFromFindCommandForTests(cmdObj, isExplain), DBException, 40415); } TEST(QueryRequestTest, ParseMaxTimeMSStringValueFails) { BSONObj maxTimeObj = BSON(QueryRequest::cmdOptionMaxTimeMS << "foo"); - ASSERT_NOT_OK(QueryRequest::parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); + ASSERT_NOT_OK(parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); } TEST(QueryRequestTest, ParseMaxTimeMSNonIntegralValueFails) { BSONObj maxTimeObj = BSON(QueryRequest::cmdOptionMaxTimeMS << 100.3); - ASSERT_NOT_OK(QueryRequest::parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); + ASSERT_NOT_OK(parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); } TEST(QueryRequestTest, ParseMaxTimeMSOutOfRangeDoubleFails) { BSONObj maxTimeObj = BSON(QueryRequest::cmdOptionMaxTimeMS << 1e200); - ASSERT_NOT_OK(QueryRequest::parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); + ASSERT_NOT_OK(parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); } TEST(QueryRequestTest, ParseMaxTimeMSNegativeValueFails) { BSONObj maxTimeObj = BSON(QueryRequest::cmdOptionMaxTimeMS << -400); - ASSERT_NOT_OK(QueryRequest::parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); + ASSERT_NOT_OK(parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS])); } TEST(QueryRequestTest, ParseMaxTimeMSZeroSucceeds) { BSONObj maxTimeObj = BSON(QueryRequest::cmdOptionMaxTimeMS << 0); - auto maxTime = QueryRequest::parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS]); + auto maxTime = parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS]); ASSERT_OK(maxTime); ASSERT_EQ(maxTime.getValue(), 0); } TEST(QueryRequestTest, ParseMaxTimeMSPositiveInRangeSucceeds) { BSONObj maxTimeObj = BSON(QueryRequest::cmdOptionMaxTimeMS << 300); - auto maxTime = QueryRequest::parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS]); + auto maxTime = parseMaxTimeMS(maxTimeObj[QueryRequest::cmdOptionMaxTimeMS]); ASSERT_OK(maxTime); ASSERT_EQ(maxTime.getValue(), 300); } @@ -1276,7 +1289,7 @@ TEST(QueryRequestTest, ConvertToAggregationSucceeds) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT(ar.getValue().getPipeline().empty()); @@ -1294,7 +1307,7 @@ TEST(QueryRequestTest, ConvertToAggregationOmitsExplain) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT_FALSE(ar.getValue().getExplain()); ASSERT(ar.getValue().getPipeline().empty()); @@ -1309,7 +1322,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithHintSucceeds) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT_BSONOBJ_EQ(qr.getHint(), ar.getValue().getHint().value_or(BSONObj())); } @@ -1413,7 +1426,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithPipeline) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getCursor().getBatchSize().value_or( @@ -1441,7 +1454,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithBatchSize) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getNamespace(), testns); @@ -1462,7 +1475,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithMaxTimeMS) { ASSERT_EQ(cmdObj["maxTimeMS"].Int(), 9); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), cmdObj).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT_EQ(ar.getValue().getCursor().getBatchSize().value_or( @@ -1479,7 +1492,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithCollationSucceeds) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT(!ar.getValue().getExplain()); ASSERT(ar.getValue().getPipeline().empty()); @@ -1512,7 +1525,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithLegacyRuntimeConstantsSucceeds) { ASSERT_OK(agg); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT(ar.getValue().getLegacyRuntimeConstants().has_value()); ASSERT_EQ(ar.getValue().getLegacyRuntimeConstants()->getLocalNow(), rtc.getLocalNow()); @@ -1526,7 +1539,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithAllowDiskUseTrueSucceeds) { ASSERT_OK(agg.getStatus()); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT_EQ(true, ar.getValue().getAllowDiskUse()); } @@ -1538,7 +1551,7 @@ TEST(QueryRequestTest, ConvertToAggregationWithAllowDiskUseFalseSucceeds) { ASSERT_OK(agg.getStatus()); auto aggCmd = OpMsgRequest::fromDBAndBody(testns.db(), agg.getValue()).body; - auto ar = aggregation_request_helper::parseFromBSON(testns, aggCmd); + auto ar = aggregation_request_helper::parseFromBSONForTests(testns, aggCmd); ASSERT_OK(ar.getStatus()); ASSERT_EQ(false, ar.getValue().getAllowDiskUse()); } diff --git a/src/mongo/db/s/periodic_sharded_index_consistency_checker.cpp b/src/mongo/db/s/periodic_sharded_index_consistency_checker.cpp index 699d03671dd..986ab2cc65b 100644 --- a/src/mongo/db/s/periodic_sharded_index_consistency_checker.cpp +++ b/src/mongo/db/s/periodic_sharded_index_consistency_checker.cpp @@ -141,8 +141,8 @@ void PeriodicShardedIndexConsistencyChecker::_launchShardedIndexConsistencyCheck continue; } - auto request = uassertStatusOK( - aggregation_request_helper::parseFromBSON(nss, aggRequestBSON)); + auto request = uassertStatusOK(aggregation_request_helper::parseFromBSON( + nss, aggRequestBSON, boost::none, false)); auto catalogCache = Grid::get(opCtx)->catalogCache(); shardVersionRetry( diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index f4c090dcf54..e3288d13970 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -1482,10 +1482,9 @@ Future ExecCommandDatabase::_initiateCommand() try { // TODO SERVER-34277 Remove the special handling for maxTimeMS for getMores. This will require // introducing a new 'max await time' parameter for getMore, and eventually banning maxTimeMS // altogether on a getMore command. - const auto maxTimeMS = - Milliseconds{uassertStatusOK(QueryRequest::parseMaxTimeMS(cmdOptionMaxTimeMSField))}; + const auto maxTimeMS = Milliseconds{uassertStatusOK(parseMaxTimeMS(cmdOptionMaxTimeMSField))}; const auto maxTimeMSOpOnly = - Milliseconds{uassertStatusOK(QueryRequest::parseMaxTimeMS(maxTimeMSOpOnlyField))}; + Milliseconds{uassertStatusOK(parseMaxTimeMS(maxTimeMSOpOnlyField))}; if ((maxTimeMS > Milliseconds::zero() || maxTimeMSOpOnly > Milliseconds::zero()) && command->getLogicalOp() != LogicalOp::opGetMore) { diff --git a/src/mongo/dbtests/query_stage_subplan.cpp b/src/mongo/dbtests/query_stage_subplan.cpp index adae158b28f..afbfc280c15 100644 --- a/src/mongo/dbtests/query_stage_subplan.cpp +++ b/src/mongo/dbtests/query_stage_subplan.cpp @@ -97,7 +97,7 @@ protected: bool isExplain = false; // If there is no '$db', append it. auto cmd = OpMsgRequest::fromDBAndBody("test", cmdObj).body; - auto qr = QueryRequest::makeFromFindCommand(cmd, isExplain, NamespaceString()); + auto qr = QueryRequest::makeFromFindCommandForTests(cmd, isExplain, NamespaceString()); auto cq = unittest::assertGet( CanonicalQuery::canonicalize(opCtx(), diff --git a/src/mongo/s/balancer_configuration_test.cpp b/src/mongo/s/balancer_configuration_test.cpp index 7680a4e482f..3d4af2a4fbb 100644 --- a/src/mongo/s/balancer_configuration_test.cpp +++ b/src/mongo/s/balancer_configuration_test.cpp @@ -80,7 +80,7 @@ protected: rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss().ns(), "config.settings"); ASSERT_BSONOBJ_EQ(query->getFilter(), BSON("_id" << key)); diff --git a/src/mongo/s/catalog/sharding_catalog_client_test.cpp b/src/mongo/s/catalog/sharding_catalog_client_test.cpp index 35fcbeb8757..8a4aa9e2ad4 100644 --- a/src/mongo/s/catalog/sharding_catalog_client_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_client_test.cpp @@ -101,7 +101,7 @@ TEST_F(ShardingCatalogClientTest, GetCollectionExisting) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); // Ensure the query is correct ASSERT_EQ(query->nss(), CollectionType::ConfigNS); @@ -171,7 +171,7 @@ TEST_F(ShardingCatalogClientTest, GetDatabaseExisting) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), DatabaseType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), BSON(DatabaseType::name(expectedDb.getName()))); @@ -300,7 +300,7 @@ TEST_F(ShardingCatalogClientTest, GetAllShardsValid) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), ShardType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), BSONObj()); @@ -397,7 +397,7 @@ TEST_F(ShardingCatalogClientTest, GetChunksForNSWithSortAndLimit) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), ChunkType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), chunksQuery); @@ -454,7 +454,7 @@ TEST_F(ShardingCatalogClientTest, GetChunksForNSNoSortNoLimit) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), ChunkType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), chunksQuery); @@ -766,7 +766,7 @@ TEST_F(ShardingCatalogClientTest, GetCollectionsValidResultsNoDb) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), CollectionType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), BSONObj()); @@ -813,7 +813,7 @@ TEST_F(ShardingCatalogClientTest, GetCollectionsValidResultsWithDb) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), CollectionType::ConfigNS); { @@ -850,7 +850,7 @@ TEST_F(ShardingCatalogClientTest, GetCollectionsInvalidCollectionType) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), CollectionType::ConfigNS); { @@ -886,7 +886,7 @@ TEST_F(ShardingCatalogClientTest, GetDatabasesForShardValid) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), DatabaseType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), @@ -954,7 +954,7 @@ TEST_F(ShardingCatalogClientTest, GetTagsForCollection) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss(), TagsType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), BSON(TagsType::ns("TestDB.TestColl"))); @@ -1301,7 +1301,7 @@ TEST_F(ShardingCatalogClientTest, GetNewKeys) { ASSERT_EQ("admin", request.dbname); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); BSONObj expectedQuery( @@ -1353,7 +1353,7 @@ TEST_F(ShardingCatalogClientTest, GetNewKeysWithEmptyCollection) { ASSERT_EQ("admin", request.dbname); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); BSONObj expectedQuery( fromjson("{purpose: 'none'," diff --git a/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp b/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp index 6291f814fdf..849d094032e 100644 --- a/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp @@ -40,6 +40,7 @@ #include "mongo/db/client.h" #include "mongo/db/commands.h" #include "mongo/db/ops/write_ops.h" +#include "mongo/db/query/query_request.h" #include "mongo/db/storage/duplicate_key_error_info.h" #include "mongo/db/write_concern.h" #include "mongo/executor/task_executor.h" @@ -148,7 +149,7 @@ TEST_F(InsertRetryTest, RetryOnNetworkErrorFails) { void assertFindRequestHasFilter(const RemoteCommandRequest& request, BSONObj filter) { // If there is no '$db', append it. auto cmd = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj).body; - auto query = QueryRequest::makeFromFindCommand(cmd, false); + auto query = QueryRequest::makeFromFindCommandForTests(cmd, false); ASSERT_BSONOBJ_EQ(filter, query->getFilter()); } diff --git a/src/mongo/s/catalog_cache_refresh_test.cpp b/src/mongo/s/catalog_cache_refresh_test.cpp index 619e4f84077..b259aa94929 100644 --- a/src/mongo/s/catalog_cache_refresh_test.cpp +++ b/src/mongo/s/catalog_cache_refresh_test.cpp @@ -610,7 +610,7 @@ TEST_F(CatalogCacheRefreshTest, ChunkEpochChangeDuringIncrementalLoadRecoveryAft expectGetCollection(oldVersion.epoch(), shardKeyPattern); onFindCommand([&](const RemoteCommandRequest& request) { auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto diffQuery = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto diffQuery = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_BSONOBJ_EQ(BSON("ns" << kNss.ns() << "lastmod" << BSON("$gte" << Timestamp(oldVersion.majorVersion(), oldVersion.minorVersion()))), @@ -642,7 +642,7 @@ TEST_F(CatalogCacheRefreshTest, ChunkEpochChangeDuringIncrementalLoadRecoveryAft // Ensure it is a differential query but starting from version zero (to fetch all the // chunks) since the incremental refresh above produced a different version auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto diffQuery = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto diffQuery = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_BSONOBJ_EQ(BSON("ns" << kNss.ns() << "lastmod" << BSON("$gte" << Timestamp(0, 0))), diffQuery->getFilter()); @@ -696,7 +696,7 @@ TEST_F(CatalogCacheRefreshTest, IncrementalLoadAfterCollectionEpochChange) { onFindCommand([&](const RemoteCommandRequest& request) { // Ensure it is a differential query but starting from version zero auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto diffQuery = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto diffQuery = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_BSONOBJ_EQ(BSON("ns" << kNss.ns() << "lastmod" << BSON("$gte" << Timestamp(0, 0))), diffQuery->getFilter()); @@ -742,7 +742,7 @@ TEST_F(CatalogCacheRefreshTest, IncrementalLoadAfterSplit) { onFindCommand([&](const RemoteCommandRequest& request) { // Ensure it is a differential query auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto diffQuery = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto diffQuery = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_BSONOBJ_EQ( BSON("ns" << kNss.ns() << "lastmod" << BSON("$gte" << Timestamp(version.majorVersion(), version.minorVersion()))), diff --git a/src/mongo/s/cluster_identity_loader_test.cpp b/src/mongo/s/cluster_identity_loader_test.cpp index b46a7a234c4..bf35b79990d 100644 --- a/src/mongo/s/cluster_identity_loader_test.cpp +++ b/src/mongo/s/cluster_identity_loader_test.cpp @@ -77,7 +77,7 @@ public: rpc::TrackingMetadata::removeTrackingData(request.metadata)); auto opMsg = OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj); - auto query = QueryRequest::makeFromFindCommand(opMsg.body, false); + auto query = QueryRequest::makeFromFindCommandForTests(opMsg.body, false); ASSERT_EQ(query->nss().ns(), "config.version"); ASSERT_BSONOBJ_EQ(query->getFilter(), BSONObj()); diff --git a/src/mongo/s/commands/cluster_aggregate_test.cpp b/src/mongo/s/commands/cluster_aggregate_test.cpp index 17700666352..dfae0f183db 100644 --- a/src/mongo/s/commands/cluster_aggregate_test.cpp +++ b/src/mongo/s/commands/cluster_aggregate_test.cpp @@ -95,7 +95,7 @@ protected: NamespaceString nss{"a.collection"}; auto client = getServiceContext()->makeClient("ClusterCmdClient"); auto opCtx = client->makeOperationContext(); - auto request = aggregation_request_helper::parseFromBSON(nss, inputBson); + auto request = aggregation_request_helper::parseFromBSONForTests(nss, inputBson); if (request.getStatus() != Status::OK()) { return request.getStatus(); } diff --git a/src/mongo/s/commands/cluster_count_cmd.cpp b/src/mongo/s/commands/cluster_count_cmd.cpp index 3ade68c8bd7..22c69d70e43 100644 --- a/src/mongo/s/commands/cluster_count_cmd.cpp +++ b/src/mongo/s/commands/cluster_count_cmd.cpp @@ -129,8 +129,11 @@ public: auto aggCmdOnView = uassertStatusOK(countCommandAsAggregationCommand(countRequest, nss)); auto aggCmdOnViewObj = OpMsgRequest::fromDBAndBody(nss.db(), aggCmdOnView).body; - auto aggRequestOnView = - uassertStatusOK(aggregation_request_helper::parseFromBSON(nss, aggCmdOnViewObj)); + auto aggRequestOnView = uassertStatusOK(aggregation_request_helper::parseFromBSON( + nss, + aggCmdOnViewObj, + boost::none, + APIParameters::get(opCtx).getAPIStrict().value_or(false))); auto resolvedAggRequest = ex->asExpandedViewAggregation(aggRequestOnView); auto resolvedAggCmd = @@ -240,8 +243,11 @@ public: auto aggCmdOnViewObj = OpMsgRequest::fromDBAndBody(nss.db(), aggCmdOnView.getValue()).body; - auto aggRequestOnView = - aggregation_request_helper::parseFromBSON(nss, aggCmdOnViewObj, verbosity); + auto aggRequestOnView = aggregation_request_helper::parseFromBSON( + nss, + aggCmdOnViewObj, + verbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false)); if (!aggRequestOnView.isOK()) { return aggRequestOnView.getStatus(); } diff --git a/src/mongo/s/commands/cluster_distinct_cmd.cpp b/src/mongo/s/commands/cluster_distinct_cmd.cpp index e6167129a67..7d1e5bc7c0b 100644 --- a/src/mongo/s/commands/cluster_distinct_cmd.cpp +++ b/src/mongo/s/commands/cluster_distinct_cmd.cpp @@ -133,8 +133,11 @@ public: } auto viewAggCmd = OpMsgRequest::fromDBAndBody(nss.db(), aggCmdOnView.getValue()).body; - auto aggRequestOnView = - aggregation_request_helper::parseFromBSON(nss, viewAggCmd, verbosity); + auto aggRequestOnView = aggregation_request_helper::parseFromBSON( + nss, + viewAggCmd, + verbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false)); if (!aggRequestOnView.isOK()) { return aggRequestOnView.getStatus(); } @@ -208,7 +211,11 @@ public: uassertStatusOK(aggCmdOnView.getStatus()); auto viewAggCmd = OpMsgRequest::fromDBAndBody(nss.db(), aggCmdOnView.getValue()).body; - auto aggRequestOnView = aggregation_request_helper::parseFromBSON(nss, viewAggCmd); + auto aggRequestOnView = aggregation_request_helper::parseFromBSON( + nss, + viewAggCmd, + boost::none, + APIParameters::get(opCtx).getAPIStrict().value_or(false)); uassertStatusOK(aggRequestOnView.getStatus()); auto resolvedAggRequest = ex->asExpandedViewAggregation(aggRequestOnView.getValue()); diff --git a/src/mongo/s/commands/cluster_find_cmd.cpp b/src/mongo/s/commands/cluster_find_cmd.cpp index dbfd1d539ef..813bee06bad 100644 --- a/src/mongo/s/commands/cluster_find_cmd.cpp +++ b/src/mongo/s/commands/cluster_find_cmd.cpp @@ -61,7 +61,11 @@ std::unique_ptr parseCmdObjectToQueryRequest(OperationContext* opC NamespaceString nss, BSONObj cmdObj, bool isExplain) { - auto qr = QueryRequest::makeFromFindCommand(std::move(cmdObj), isExplain, std::move(nss)); + auto qr = + QueryRequest::makeFromFindCommand(std::move(cmdObj), + isExplain, + std::move(nss), + APIParameters::get(opCtx).getAPIStrict().value_or(false)); if (!qr->getReadConcern()) { if (opCtx->isStartingMultiDocumentTransaction() || !opCtx->inMultiDocumentTransaction()) { // If there is no explicit readConcern in the cmdObj, and this is either the first @@ -195,7 +199,10 @@ public: OpMsgRequest::fromDBAndBody(_dbName, aggCmdOnView).body; auto aggRequestOnView = uassertStatusOK(aggregation_request_helper::parseFromBSON( - ns(), viewAggregationCommand, verbosity)); + ns(), + viewAggregationCommand, + verbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false))); // An empty PrivilegeVector is acceptable because these privileges are only checked // on getMore and explain will not open a cursor. @@ -257,8 +264,11 @@ public: auto viewAggregationCommand = OpMsgRequest::fromDBAndBody(_dbName, aggCmdOnView).body; - auto aggRequestOnView = uassertStatusOK( - aggregation_request_helper::parseFromBSON(ns(), viewAggregationCommand)); + auto aggRequestOnView = uassertStatusOK(aggregation_request_helper::parseFromBSON( + ns(), + viewAggregationCommand, + boost::none, + APIParameters::get(opCtx).getAPIStrict().value_or(false))); auto bodyBuilder = result->getBodyBuilder(); uassertStatusOK(ClusterAggregate::retryOnViewError( diff --git a/src/mongo/s/commands/cluster_pipeline_cmd.cpp b/src/mongo/s/commands/cluster_pipeline_cmd.cpp index 631a5dabcc0..8568a46d9aa 100644 --- a/src/mongo/s/commands/cluster_pipeline_cmd.cpp +++ b/src/mongo/s/commands/cluster_pipeline_cmd.cpp @@ -74,7 +74,10 @@ public: const OpMsgRequest& opMsgRequest, boost::optional explainVerbosity) override { const auto aggregationRequest = uassertStatusOK(aggregation_request_helper::parseFromBSON( - opMsgRequest.getDatabase().toString(), opMsgRequest.body, explainVerbosity)); + opMsgRequest.getDatabase().toString(), + opMsgRequest.body, + explainVerbosity, + APIParameters::get(opCtx).getAPIStrict().value_or(false))); auto privileges = uassertStatusOK(AuthorizationSession::get(opCtx->getClient()) diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 5b4ccd3a82d..8a167a09548 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -508,8 +508,8 @@ Status ParseAndRunCommand::_prologue() { uassert(ErrorCodes::InvalidOptions, "no such command option $maxTimeMs; use maxTimeMS instead", request.body[QueryRequest::queryOptionMaxTimeMS].eoo()); - const int maxTimeMS = uassertStatusOK( - QueryRequest::parseMaxTimeMS(request.body[QueryRequest::cmdOptionMaxTimeMS])); + const int maxTimeMS = + uassertStatusOK(parseMaxTimeMS(request.body[QueryRequest::cmdOptionMaxTimeMS])); if (maxTimeMS > 0 && command->getLogicalOp() != LogicalOp::opGetMore) { opCtx->setDeadlineAfterNowBy(Milliseconds{maxTimeMS}, ErrorCodes::MaxTimeMSExpired); } diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp index 0cb7b9b8e31..7fac4105e93 100644 --- a/src/mongo/s/query/cluster_find.cpp +++ b/src/mongo/s/query/cluster_find.cpp @@ -160,7 +160,8 @@ StatusWith> transformQueryForShards( newQR->setSingleBatchField(false); // Any expansion of the 'showRecordId' flag should have already happened on mongos. - newQR->setShowRecordId(false); + if (newQR->showRecordId()) + newQR->setShowRecordId(false); invariant(newQR->validate()); return std::move(newQR); diff --git a/src/mongo/s/query/results_merger_test_fixture.h b/src/mongo/s/query/results_merger_test_fixture.h index d920ba7266c..d653bfe0bfa 100644 --- a/src/mongo/s/query/results_merger_test_fixture.h +++ b/src/mongo/s/query/results_merger_test_fixture.h @@ -72,7 +72,8 @@ protected: if (findCmd) { // If there is no '$db', append it. auto cmd = OpMsgRequest::fromDBAndBody(kTestNss.db(), *findCmd).body; - const auto qr = QueryRequest::makeFromFindCommand(cmd, false /* isExplain */, kTestNss); + const auto qr = + QueryRequest::makeFromFindCommandForTests(cmd, false /* isExplain */, kTestNss); if (!qr->getSort().isEmpty()) { params.setSort(qr->getSort().getOwned()); } diff --git a/src/mongo/s/sessions_collection_sharded.cpp b/src/mongo/s/sessions_collection_sharded.cpp index eccd1a732f4..76946cbd900 100644 --- a/src/mongo/s/sessions_collection_sharded.cpp +++ b/src/mongo/s/sessions_collection_sharded.cpp @@ -166,13 +166,14 @@ void SessionsCollectionSharded::removeRecords(OperationContext* opCtx, LogicalSessionIdSet SessionsCollectionSharded::findRemovedSessions( OperationContext* opCtx, const LogicalSessionIdSet& sessions) { + bool apiStrict = APIParameters::get(opCtx).getAPIStrict().value_or(false); auto send = [&](BSONObj toSend) -> BSONObj { // If there is no '$db', append it. toSend = OpMsgRequest::fromDBAndBody(NamespaceString::kLogicalSessionsNamespace.db(), toSend) .body; auto qr = QueryRequest::makeFromFindCommand( - toSend, false, NamespaceString::kLogicalSessionsNamespace); + toSend, false, NamespaceString::kLogicalSessionsNamespace, apiStrict); const boost::intrusive_ptr expCtx; auto cq = uassertStatusOK( diff --git a/src/mongo/s/sharding_router_test_fixture.cpp b/src/mongo/s/sharding_router_test_fixture.cpp index 670620bf52e..3e42317a93e 100644 --- a/src/mongo/s/sharding_router_test_fixture.cpp +++ b/src/mongo/s/sharding_router_test_fixture.cpp @@ -257,7 +257,7 @@ void ShardingTestFixture::expectGetShards(const std::vector& shards) // If there is no '$db', append it. auto cmd = OpMsgRequest::fromDBAndBody(nss.db(), request.cmdObj).body; - auto query = QueryRequest::makeFromFindCommand(cmd, false, nss); + auto query = QueryRequest::makeFromFindCommandForTests(cmd, false, nss); ASSERT_EQ(query->nss(), ShardType::ConfigNS); ASSERT_BSONOBJ_EQ(query->getFilter(), BSONObj()); -- cgit v1.2.1