From cd1b7ce13eb583946482b5eeba3b58c0ad308d16 Mon Sep 17 00:00:00 2001 From: Sophia Tan Date: Wed, 17 Aug 2022 21:59:59 +0000 Subject: SERVER-68421 Remove CommandHelper::parseNsCollectionRequired(StringData dbname, const BSONObj& cmdObj) --- src/mongo/db/catalog/create_collection.cpp | 10 +++++----- src/mongo/db/commands.cpp | 5 ----- src/mongo/db/commands.h | 3 --- src/mongo/db/commands/count_cmd.cpp | 4 ++-- src/mongo/db/commands/dbcommands.cpp | 2 +- .../replica_set_node_process_interface.cpp | 3 +-- src/mongo/db/repl/rollback_impl.cpp | 6 +++--- src/mongo/s/commands/cluster_collection_mod_cmd.cpp | 3 ++- src/mongo/s/commands/cluster_create_indexes_cmd.cpp | 5 +---- src/mongo/s/commands/cluster_find_and_modify_cmd.cpp | 2 +- src/mongo/s/commands/cluster_find_cmd.h | 16 ++++++++-------- .../s/commands/cluster_set_index_commit_quorum_cmd.cpp | 3 ++- 12 files changed, 26 insertions(+), 36 deletions(-) diff --git a/src/mongo/db/catalog/create_collection.cpp b/src/mongo/db/catalog/create_collection.cpp index c026a5b29c3..ae252d1db81 100644 --- a/src/mongo/db/catalog/create_collection.cpp +++ b/src/mongo/db/catalog/create_collection.cpp @@ -677,19 +677,19 @@ Status createCollection(OperationContext* opCtx, const CreateCommand& cmd) { // TODO SERVER-62395 Pass DatabaseName instead of dbName, and pass to isDbLockedForMode. Status createCollectionForApplyOps(OperationContext* opCtx, - const std::string& dbName, + const std::string& dbname, const boost::optional& ui, const BSONObj& cmdObj, const bool allowRenameOutOfTheWay, const boost::optional& idIndex) { - invariant(opCtx->lockState()->isDbLockedForMode(DatabaseName(boost::none, dbName), MODE_IX)); + const DatabaseName dbName(boost::none, dbname); + invariant(opCtx->lockState()->isDbLockedForMode(dbName, MODE_IX)); const NamespaceString newCollName(CommandHelpers::parseNsCollectionRequired(dbName, cmdObj)); auto newCmd = cmdObj; auto databaseHolder = DatabaseHolder::get(opCtx); - const DatabaseName tenantDbName(boost::none, dbName); - auto* const db = databaseHolder->getDb(opCtx, tenantDbName); + auto* const db = databaseHolder->getDb(opCtx, dbName); // If a UUID is given, see if we need to rename a collection out of the way, and whether the // collection already exists under a different name. If so, rename it into place. As this is @@ -735,7 +735,7 @@ Status createCollectionForApplyOps(OperationContext* opCtx, << ". Future collection name: " << newCollName); for (int tries = 0; needsRenaming && tries < 10; ++tries) { - auto tmpNameResult = makeUniqueCollectionName(opCtx, tenantDbName, "tmp%%%%%.create"); + auto tmpNameResult = makeUniqueCollectionName(opCtx, dbName, "tmp%%%%%.create"); if (!tmpNameResult.isOK()) { return tmpNameResult.getStatus().withContext(str::stream() << "Cannot generate temporary " diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index d3d92172ae4..7061cfb07bc 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -278,11 +278,6 @@ std::string CommandHelpers::parseNsFullyQualified(const BSONObj& cmdObj) { return nss.ns(); } -NamespaceString CommandHelpers::parseNsCollectionRequired(StringData dbname, - const BSONObj& cmdObj) { - return parseNsCollectionRequired({boost::none, dbname}, cmdObj); -} - NamespaceString CommandHelpers::parseNsCollectionRequired(const DatabaseName& dbName, const BSONObj& cmdObj) { // Accepts both BSON String and Symbol for collection name per SERVER-16260 diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index f23abe97a98..d903da77372 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -135,9 +135,6 @@ struct CommandHelpers { // The first field is interpreted as a collection name. static NamespaceString parseNsCollectionRequired(const DatabaseName& dbName, const BSONObj& cmdObj); - // TODO SERVER-68421: Remove this method once all call sites have been updated to pass - // DatabaseName. - static NamespaceString parseNsCollectionRequired(StringData dbname, const BSONObj& cmdObj); static NamespaceStringOrUUID parseNsOrUUID(const DatabaseName& dbName, const BSONObj& cmdObj); diff --git a/src/mongo/db/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp index 43377f76da6..5eeffc217b4 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -149,13 +149,13 @@ public: const OpMsgRequest& opMsgRequest, ExplainOptions::Verbosity verbosity, rpc::ReplyBuilderInterface* result) const override { - std::string dbname = opMsgRequest.getDatabase().toString(); + DatabaseName dbName(opMsgRequest.getValidatedTenantId(), opMsgRequest.getDatabase()); const BSONObj& cmdObj = opMsgRequest.body; // Acquire locks. The RAII object is optional, because in the case // of a view, the locks need to be released. boost::optional ctx; ctx.emplace(opCtx, - CommandHelpers::parseNsCollectionRequired(dbname, cmdObj), + CommandHelpers::parseNsCollectionRequired(dbName, cmdObj), AutoGetCollectionViewMode::kViewsPermitted); const auto nss = ctx->getNss(); diff --git a/src/mongo/db/commands/dbcommands.cpp b/src/mongo/db/commands/dbcommands.cpp index b19eb539ba3..750a3baac8b 100644 --- a/src/mongo/db/commands/dbcommands.cpp +++ b/src/mongo/db/commands/dbcommands.cpp @@ -465,7 +465,7 @@ public: Status checkAuthForOperation(OperationContext* opCtx, const std::string& dbname, const BSONObj& cmdObj) const final { - const auto nss = CommandHelpers::parseNsCollectionRequired(dbname, cmdObj); + const auto nss = CommandHelpers::parseNsCollectionRequired({boost::none, dbname}, cmdObj); auto as = AuthorizationSession::get(opCtx->getClient()); if (!as->isAuthorizedForActionsOnResource(ResourcePattern::forExactNamespace(nss), ActionType::collStats)) { diff --git a/src/mongo/db/pipeline/process_interface/replica_set_node_process_interface.cpp b/src/mongo/db/pipeline/process_interface/replica_set_node_process_interface.cpp index 9d2e685b185..e4cdf236d38 100644 --- a/src/mongo/db/pipeline/process_interface/replica_set_node_process_interface.cpp +++ b/src/mongo/db/pipeline/process_interface/replica_set_node_process_interface.cpp @@ -156,8 +156,7 @@ void ReplicaSetNodeProcessInterface::createCollection(OperationContext* opCtx, if (_canWriteLocally(opCtx, dbNs)) { return NonShardServerProcessInterface::createCollection(opCtx, dbName, cmdObj); } - // TODO SERVER-67519 change CommandHelpers::parseNsCollectionRequired to take in DatabaseName - auto ns = CommandHelpers::parseNsCollectionRequired(dbName.toStringWithTenantId(), cmdObj); + auto ns = CommandHelpers::parseNsCollectionRequired(dbName, cmdObj); uassertStatusOK(_executeCommandOnPrimary(opCtx, ns, cmdObj)); } diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp index 48224876ed9..f08e4c6ef66 100644 --- a/src/mongo/db/repl/rollback_impl.cpp +++ b/src/mongo/db/repl/rollback_impl.cpp @@ -443,7 +443,7 @@ StatusWith> RollbackImpl::_namespacesForOp(const Oplog // For all other command types, we should be able to parse the collection name from // the first command argument. try { - auto cmdNss = CommandHelpers::parseNsCollectionRequired(opNss.db(), obj); + auto cmdNss = CommandHelpers::parseNsCollectionRequired(opNss.dbName(), obj); namespaces.insert(cmdNss); } catch (const DBException& ex) { return ex.toStatus(); @@ -994,8 +994,8 @@ Status RollbackImpl::_processRollbackOp(OperationContext* opCtx, const OplogEntr PendingDropInfo info; info.count = *countResult; const auto& opNss = oplogEntry.getNss(); - info.nss = - CommandHelpers::parseNsCollectionRequired(opNss.db(), oplogEntry.getObject()); + info.nss = CommandHelpers::parseNsCollectionRequired(opNss.dbName(), + oplogEntry.getObject()); _pendingDrops[uuid] = info; _newCounts[uuid] = info.count; } else { diff --git a/src/mongo/s/commands/cluster_collection_mod_cmd.cpp b/src/mongo/s/commands/cluster_collection_mod_cmd.cpp index 2dd8cd57291..8b73ac91430 100644 --- a/src/mongo/s/commands/cluster_collection_mod_cmd.cpp +++ b/src/mongo/s/commands/cluster_collection_mod_cmd.cpp @@ -74,7 +74,8 @@ public: Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) const override { - const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbname, cmdObj)); + const NamespaceString nss( + CommandHelpers::parseNsCollectionRequired({boost::none, dbname}, cmdObj)); return auth::checkAuthForCollMod( client->getOperationContext(), AuthorizationSession::get(client), nss, cmdObj, true); } diff --git a/src/mongo/s/commands/cluster_create_indexes_cmd.cpp b/src/mongo/s/commands/cluster_create_indexes_cmd.cpp index ef3aacd159c..3a56ea99141 100644 --- a/src/mongo/s/commands/cluster_create_indexes_cmd.cpp +++ b/src/mongo/s/commands/cluster_create_indexes_cmd.cpp @@ -87,10 +87,7 @@ public: const BSONObj& cmdObj, const RequestParser&, BSONObjBuilder& output) final { - // TODO SERVER-67519 Change CommandHelpers::parseNs* methods to construct NamespaceStrings - // with tenantId - const NamespaceString nss( - CommandHelpers::parseNsCollectionRequired(dbName.toStringWithTenantId(), cmdObj)); + const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbName, cmdObj)); LOGV2_DEBUG(22750, 1, "createIndexes: {namespace} cmd: {command}", diff --git a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp index 81afeef46ab..d01690787ee 100644 --- a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp +++ b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp @@ -377,7 +377,7 @@ public: const OpMsgRequest& request, ExplainOptions::Verbosity verbosity, rpc::ReplyBuilderInterface* result) const override { - std::string dbName = request.getDatabase().toString(); + const DatabaseName dbName(request.getValidatedTenantId(), request.getDatabase()); const BSONObj& cmdObj = [&]() { // Check whether the query portion needs to be rewritten for FLE. auto findAndModifyRequest = write_ops::FindAndModifyCommandRequest::parse( diff --git a/src/mongo/s/commands/cluster_find_cmd.h b/src/mongo/s/commands/cluster_find_cmd.h index 899cba75722..5c4dca0cd78 100644 --- a/src/mongo/s/commands/cluster_find_cmd.h +++ b/src/mongo/s/commands/cluster_find_cmd.h @@ -68,7 +68,7 @@ public: std::unique_ptr parse(OperationContext* opCtx, const OpMsgRequest& opMsgRequest) override { // TODO: Parse into a QueryRequest here. - return std::make_unique(this, opMsgRequest, opMsgRequest.getDatabase()); + return std::make_unique(this, opMsgRequest); } AllowedOnSecondary secondaryAllowed(ServiceContext* context) const override { @@ -97,10 +97,10 @@ public: class Invocation final : public CommandInvocation { public: - Invocation(const ClusterFindCmdBase* definition, - const OpMsgRequest& request, - StringData dbName) - : CommandInvocation(definition), _request(request), _dbName(dbName) {} + Invocation(const ClusterFindCmdBase* definition, const OpMsgRequest& request) + : CommandInvocation(definition), + _request(request), + _dbName(request.getValidatedTenantId(), request.getDatabase()) {} private: bool supportsWriteConcern() const override { @@ -177,7 +177,7 @@ public: auto aggCmdOnView = uassertStatusOK(query_request_helper::asAggregationCommand(*findCommand)); auto viewAggregationCommand = - OpMsgRequest::fromDBAndBody(_dbName, aggCmdOnView).body; + OpMsgRequest::fromDBAndBody(_dbName.db(), aggCmdOnView).body; auto aggRequestOnView = aggregation_request_helper::parseFromBSON( opCtx, @@ -247,7 +247,7 @@ public: auto aggCmdOnView = uassertStatusOK( query_request_helper::asAggregationCommand(cq->getFindCommandRequest())); auto viewAggregationCommand = - OpMsgRequest::fromDBAndBody(_dbName, aggCmdOnView).body; + OpMsgRequest::fromDBAndBody(_dbName.db(), aggCmdOnView).body; auto aggRequestOnView = aggregation_request_helper::parseFromBSON( opCtx, @@ -303,7 +303,7 @@ public: } const OpMsgRequest& _request; - const StringData _dbName; + const DatabaseName _dbName; }; }; diff --git a/src/mongo/s/commands/cluster_set_index_commit_quorum_cmd.cpp b/src/mongo/s/commands/cluster_set_index_commit_quorum_cmd.cpp index cd066471f04..eb013343c76 100644 --- a/src/mongo/s/commands/cluster_set_index_commit_quorum_cmd.cpp +++ b/src/mongo/s/commands/cluster_set_index_commit_quorum_cmd.cpp @@ -88,7 +88,8 @@ public: Status checkAuthForOperation(OperationContext* opCtx, const std::string& dbName, const BSONObj& cmdObj) const override { - const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbName, cmdObj)); + const NamespaceString nss( + CommandHelpers::parseNsCollectionRequired({boost::none, dbName}, cmdObj)); if (!AuthorizationSession::get(opCtx->getClient()) ->isAuthorizedForActionsOnResource(ResourcePattern::forExactNamespace(nss), ActionType::createIndex)) { -- cgit v1.2.1