diff options
author | Benety Goh <benety@mongodb.com> | 2021-12-01 11:59:46 -0500 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-12-01 18:06:34 +0000 |
commit | 434222cb8f7dfe345f2bdfb748d0eeb1ae3ac570 (patch) | |
tree | 9e3d2a8f4f0a90d3ed5a2426dae99e4219ccf368 | |
parent | d5b0d000ec3629e6e58756feac1bf8e73cc8d074 (diff) | |
download | mongo-434222cb8f7dfe345f2bdfb748d0eeb1ae3ac570.tar.gz |
SERVER-61800 remove ParsedCollModRequest::viewPipeLine in favor of CollMod::getPipeline()
-rw-r--r-- | src/mongo/db/catalog/coll_mod.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/views/view.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/views/view.h | 2 | ||||
-rw-r--r-- | src/mongo/db/views/view_definition_test.cpp | 24 |
4 files changed, 12 insertions, 36 deletions
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index a62bf76ba6a..ad7c7fde53c 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -106,7 +106,6 @@ struct ParsedCollModRequest { // and ParsedCollModIndexRequest. BSONObj cmdObj; // owned ParsedCollModIndexRequest indexRequest; - BSONElement viewPipeLine = {}; std::string viewOn = {}; boost::optional<Collection::Validator> collValidator; boost::optional<ValidationActionEnum> collValidationAction; @@ -360,10 +359,8 @@ StatusWith<ParsedCollModRequest> parseCollModRequest(OperationContext* opCtx, return Status(ErrorCodes::InvalidOptions, "'pipeline' option only supported on a view"); } - if (e.type() != mongo::Array) { - return Status(ErrorCodes::InvalidOptions, "not a valid aggregation pipeline"); - } - cmr.viewPipeLine = e; + // Access this value through the generated CollMod IDL type. + // See CollModRequest::getPipeline(). } else if (fieldName == "viewOn") { cmr.numModifications++; if (!isView) { @@ -613,7 +610,6 @@ Status _collModInternal(OperationContext* opCtx, // Save both states of the ParsedCollModRequest to allow writeConflictRetries. ParsedCollModRequest cmrNew = std::move(statusW.getValue()); - auto viewPipeline = cmrNew.viewPipeLine; auto viewOn = cmrNew.viewOn; auto ts = cmd.getTimeseries(); @@ -633,8 +629,8 @@ Status _collModInternal(OperationContext* opCtx, // Handle collMod on a view and return early. The View Catalog handles the creation of oplog // entries for modifications on a view. if (view) { - if (viewPipeline) - view->setPipeline(viewPipeline); + if (cmd.getPipeline()) + view->setPipeline(*cmd.getPipeline()); if (!viewOn.empty()) view->setViewOn(NamespaceString(dbName, viewOn)); diff --git a/src/mongo/db/views/view.cpp b/src/mongo/db/views/view.cpp index 04987538658..1ee1c0b88aa 100644 --- a/src/mongo/db/views/view.cpp +++ b/src/mongo/db/views/view.cpp @@ -68,12 +68,10 @@ void ViewDefinition::setViewOn(const NamespaceString& viewOnNss) { _viewOnNss = viewOnNss; } -void ViewDefinition::setPipeline(const BSONElement& pipeline) { - invariant(pipeline.type() == Array); - _pipeline.clear(); - for (BSONElement e : pipeline.Obj()) { - BSONObj value = e.Obj(); - _pipeline.push_back(value.copy()); +void ViewDefinition::setPipeline(std::vector<mongo::BSONObj> pipeline) { + for (auto& stage : pipeline) { + stage = stage.copy(); } + _pipeline.swap(pipeline); } } // namespace mongo diff --git a/src/mongo/db/views/view.h b/src/mongo/db/views/view.h index 713e65e3f92..9b775506e3f 100644 --- a/src/mongo/db/views/view.h +++ b/src/mongo/db/views/view.h @@ -103,7 +103,7 @@ public: /** * Pipeline must be of type array. */ - void setPipeline(const BSONElement& pipeline); + void setPipeline(std::vector<mongo::BSONObj> pipeline); private: NamespaceString _viewNss; diff --git a/src/mongo/db/views/view_definition_test.cpp b/src/mongo/db/views/view_definition_test.cpp index 3971e39693d..31201e62920 100644 --- a/src/mongo/db/views/view_definition_test.cpp +++ b/src/mongo/db/views/view_definition_test.cpp @@ -110,35 +110,17 @@ TEST(ViewDefinitionTest, SetViewOnSucceedsIfNewViewOnIsInSameDatabaseAsView) { ASSERT_EQ(newViewOn, viewDef.viewOn()); } -DEATH_TEST_REGEX(ViewDefinitionTest, - SetPiplineFailsIfPipelineTypeIsNotArray, - R"#(Invariant failure.*pipeline.type\(\) == Array)#") { - ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr); - - // We'll pass in a BSONElement that could be a valid array, but is BSONType::Object rather than - // BSONType::Array. - BSONObjBuilder builder; - BSONArrayBuilder pipelineBuilder(builder.subobjStart("pipeline")); - pipelineBuilder.append(BSON("skip" << 7)); - pipelineBuilder.append(BSON("limit" << 4)); - pipelineBuilder.doneFast(); - BSONObj newPipeline = builder.obj(); - - viewDef.setPipeline(newPipeline["pipeline"]); -} - TEST(ViewDefinitionTest, SetPipelineSucceedsOnValidArrayBSONElement) { ViewDefinition viewDef(viewNss.db(), viewNss.coll(), backingNss.coll(), BSONObj(), nullptr); ASSERT(viewDef.pipeline().empty()); BSONObj matchStage = BSON("match" << BSON("x" << 9)); BSONObj sortStage = BSON("sort" << BSON("name" << -1)); - BSONObj newPipeline = BSON("pipeline" << BSON_ARRAY(matchStage << sortStage)); + std::vector<BSONObj> newPipeline{matchStage, sortStage}; - viewDef.setPipeline(newPipeline["pipeline"]); + viewDef.setPipeline(newPipeline); - std::vector<BSONObj> expectedPipeline{matchStage, sortStage}; + const auto& expectedPipeline = newPipeline; ASSERT(std::equal(expectedPipeline.begin(), expectedPipeline.end(), viewDef.pipeline().begin(), |