From cd13936f86945057758404fa1bdf35f1919f5aaf Mon Sep 17 00:00:00 2001 From: Sophia Tan Date: Thu, 11 Aug 2022 01:54:08 +0000 Subject: SERVER-67827 Convert non-sharding ErrmsgCommandDeprecated commands to BasicCommand --- src/mongo/db/commands/collection_to_capped.cpp | 57 +++++++-------- src/mongo/db/commands/compact.cpp | 30 ++++---- src/mongo/db/commands/dbhash.cpp | 39 ++++------ src/mongo/db/commands/driverHelpers.cpp | 18 ++--- src/mongo/db/commands/drop_indexes.cpp | 26 +++---- src/mongo/db/commands/fsync.cpp | 40 +++++------ src/mongo/db/commands/get_last_error.cpp | 13 ++-- src/mongo/db/commands/map_reduce_agg.cpp | 2 + src/mongo/db/commands/map_reduce_agg.h | 1 + src/mongo/db/commands/map_reduce_command.cpp | 14 ++-- src/mongo/db/commands/map_reduce_command_base.h | 11 ++- src/mongo/db/commands/parameters.cpp | 95 +++++++++++-------------- src/mongo/db/commands/test_commands.cpp | 22 +++--- 13 files changed, 163 insertions(+), 205 deletions(-) (limited to 'src/mongo/db/commands') diff --git a/src/mongo/db/commands/collection_to_capped.cpp b/src/mongo/db/commands/collection_to_capped.cpp index 80faf2d0f9a..456c76ceb13 100644 --- a/src/mongo/db/commands/collection_to_capped.cpp +++ b/src/mongo/db/commands/collection_to_capped.cpp @@ -44,9 +44,9 @@ namespace mongo { namespace { -class CmdCloneCollectionAsCapped : public ErrmsgCommandDeprecated { +class CmdCloneCollectionAsCapped : public BasicCommand { public: - CmdCloneCollectionAsCapped() : ErrmsgCommandDeprecated("cloneCollectionAsCapped") {} + CmdCloneCollectionAsCapped() : BasicCommand("cloneCollectionAsCapped") {} AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { return AllowedOnSecondary::kNever; } @@ -79,13 +79,13 @@ public: out->push_back(Privilege(ResourcePattern::forExactNamespace(nss), targetActions)); } - bool errmsgRun(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& jsobj, - std::string& errmsg, - BSONObjBuilder& result) { - const auto fromElt = jsobj["cloneCollectionAsCapped"]; - const auto toElt = jsobj["toCollection"]; + + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + const auto fromElt = cmdObj["cloneCollectionAsCapped"]; + const auto toElt = cmdObj["toCollection"]; uassert(ErrorCodes::TypeMismatch, "'cloneCollectionAsCapped' must be of type String", @@ -104,16 +104,13 @@ public: str::stream() << "Invalid target collection name: " << to, NamespaceString::validCollectionName(to)); - double size = jsobj.getField("size").number(); - bool temp = jsobj.getField("temp").trueValue(); + double size = cmdObj.getField("size").number(); + bool temp = cmdObj.getField("temp").trueValue(); - if (size == 0) { - errmsg = "invalid command spec"; - return false; - } + uassert(ErrorCodes::InvalidOptions, "invalid command spec", size != 0); - NamespaceString fromNs(dbname, from); - NamespaceString toNs(dbname, to); + NamespaceString fromNs(dbName, from); + NamespaceString toNs(dbName, to); AutoGetCollection autoColl(opCtx, fromNs, MODE_X); Lock::CollectionLock collLock(opCtx, toNs, MODE_X); @@ -127,7 +124,7 @@ public: Database* const db = autoColl.getDb(); if (!db) { uasserted(ErrorCodes::NamespaceNotFound, - str::stream() << "database " << dbname << " not found"); + str::stream() << "database " << dbName.toString() << " not found"); } cloneCollectionAsCapped(opCtx, db, fromNs, toNs, size, temp); @@ -140,9 +137,9 @@ public: * Converts the given collection to a capped collection w/ the specified size. This command is not * highly used, and is not currently supported with sharded environments. */ -class CmdConvertToCapped : public ErrmsgCommandDeprecated { +class CmdConvertToCapped : public BasicCommand { public: - CmdConvertToCapped() : ErrmsgCommandDeprecated("convertToCapped") {} + CmdConvertToCapped() : BasicCommand("convertToCapped") {} AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { return AllowedOnSecondary::kNever; } @@ -165,18 +162,14 @@ public: out->push_back(Privilege(parseResourcePattern(dbname, cmdObj), actions)); } - bool errmsgRun(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& jsobj, - std::string& errmsg, - BSONObjBuilder& result) { - const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbname, jsobj)); - long long size = jsobj.getField("size").safeNumberLong(); - - if (size == 0) { - errmsg = "invalid command spec"; - return false; - } + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbName, cmdObj)); + long long size = cmdObj.getField("size").safeNumberLong(); + + uassert(ErrorCodes::InvalidOptions, "invalid command spec", size != 0); convertToCapped(opCtx, nss, size); return true; diff --git a/src/mongo/db/commands/compact.cpp b/src/mongo/db/commands/compact.cpp index 26ee8ab5d4b..e20e9905b19 100644 --- a/src/mongo/db/commands/compact.cpp +++ b/src/mongo/db/commands/compact.cpp @@ -47,7 +47,7 @@ namespace mongo { using std::string; using std::stringstream; -class CompactCmd : public ErrmsgCommandDeprecated { +class CompactCmd : public BasicCommand { public: virtual bool supportsWriteConcern(const BSONObj& cmd) const override { return false; @@ -72,28 +72,22 @@ public: "{ compact : , [force:] }\n" " force - allows to run on a replica set primary\n"; } - CompactCmd() : ErrmsgCommandDeprecated("compact") {} + CompactCmd() : BasicCommand("compact") {} - virtual bool errmsgRun(OperationContext* opCtx, - const string& db, - const BSONObj& cmdObj, - string& errmsg, - BSONObjBuilder& result) { - NamespaceString nss = CommandHelpers::parseNsCollectionRequired(db, cmdObj); + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + NamespaceString nss = CommandHelpers::parseNsCollectionRequired(dbName, cmdObj); repl::ReplicationCoordinator* replCoord = repl::ReplicationCoordinator::get(opCtx); - if (replCoord->getMemberState().primary() && !cmdObj["force"].trueValue()) { - errmsg = + uassert(ErrorCodes::IllegalOperation, "will not run compact on an active replica set primary as this is a slow blocking " - "operation. use force:true to force"; - return false; - } + "operation. use force:true to force", + !replCoord->getMemberState().primary() || cmdObj["force"].trueValue()); - if (nss.isSystem()) { - // Items in system.* cannot be moved as there might be pointers to them. - errmsg = "can't compact a system namespace"; - return false; - } + // Items in system.* cannot be moved as there might be pointers to them. + uassert(ErrorCodes::InvalidNamespace, "can't compact a system namespace", !nss.isSystem()); // This command is internal to the storage engine and should not block oplog application. ShouldNotConflictWithSecondaryBatchApplicationBlock noPBWMBlock(opCtx->lockState()); diff --git a/src/mongo/db/commands/dbhash.cpp b/src/mongo/db/commands/dbhash.cpp index e00188d8cea..4b63110308c 100644 --- a/src/mongo/db/commands/dbhash.cpp +++ b/src/mongo/db/commands/dbhash.cpp @@ -61,9 +61,9 @@ namespace mongo { namespace { -class DBHashCmd : public ErrmsgCommandDeprecated { +class DBHashCmd : public BasicCommand { public: - DBHashCmd() : ErrmsgCommandDeprecated("dbHash", "dbhash") {} + DBHashCmd() : BasicCommand("dbHash", "dbhash") {} virtual bool supportsWriteConcern(const BSONObj& cmd) const override { return false; @@ -114,11 +114,10 @@ public: out->push_back(Privilege(ResourcePattern::forDatabaseName(dbname), actions)); } - virtual bool errmsgRun(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& cmdObj, - std::string& errmsg, - BSONObjBuilder& result) { + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { Timer timer; std::set desiredCollections; @@ -126,16 +125,13 @@ public: BSONObjIterator i(cmdObj["collections"].Obj()); while (i.more()) { BSONElement e = i.next(); - if (e.type() != String) { - errmsg = "collections entries have to be strings"; - return false; - } + uassert(ErrorCodes::BadValue, + "collections entries have to be strings", + e.type() == String); desiredCollections.insert(e.String()); } } - // TODO SERVER-67827: Pass dbName obj directly. - const DatabaseName dbName(boost::none, dbname); // For empty databasename on first command field, the following code depends on the "." // on ns to find the invalid empty db name instead of checking empty db name directly. const std::string ns = parseNs(dbName, cmdObj).ns(); @@ -226,8 +222,7 @@ public: shouldNotConflictBlock.emplace(opCtx->lockState()); } - // TODO SERVER-67827: Pass dbName obj directly. - AutoGetDb autoDb(opCtx, DatabaseName(boost::none, ns), lockMode); + AutoGetDb autoDb(opCtx, dbName, lockMode); Database* db = autoDb.getDb(); result.append("host", prettyHostName()); @@ -239,17 +234,13 @@ public: std::map collectionToUUIDMap; std::set cappedCollectionSet; - bool noError = true; catalog::forEachCollectionFromDb( opCtx, dbName, MODE_IS, [&](const CollectionPtr& collection) { auto collNss = collection->ns(); - if (collNss.size() - 1 <= dbname.size()) { - errmsg = str::stream() - << "weird fullCollectionName [" << collNss.toString() << "]"; - noError = false; - return false; - } + uassert(ErrorCodes::BadValue, + str::stream() << "weird fullCollectionName [" << collNss.toString() << "]", + collNss.size() - 1 > dbName.db().size()); if (repl::ReplicationCoordinator::isOplogDisabledForNS(collNss)) { return true; @@ -282,8 +273,6 @@ public: return true; }); - if (!noError) - return false; BSONObjBuilder bb(result.subobjStart("collections")); BSONArrayBuilder cappedCollections; @@ -318,7 +307,7 @@ public: result.append("md5", hash); result.appendNumber("timeMillis", timer.millis()); - return 1; + return true; } private: diff --git a/src/mongo/db/commands/driverHelpers.cpp b/src/mongo/db/commands/driverHelpers.cpp index 3a3ca1b8704..f6db10738db 100644 --- a/src/mongo/db/commands/driverHelpers.cpp +++ b/src/mongo/db/commands/driverHelpers.cpp @@ -51,9 +51,9 @@ namespace mongo { using std::string; -class BasicDriverHelper : public ErrmsgCommandDeprecated { +class BasicDriverHelper : public BasicCommand { public: - BasicDriverHelper(const char* name) : ErrmsgCommandDeprecated(name) {} + BasicDriverHelper(const char* name) : BasicCommand(name) {} virtual bool supportsWriteConcern(const BSONObj& cmd) const override { return false; @@ -70,15 +70,11 @@ public: virtual void addRequiredPrivileges(const std::string& dbname, const BSONObj& cmdObj, std::vector* out) const {} // No auth required - virtual bool errmsgRun(OperationContext* opCtx, - const string&, - const BSONObj& cmdObj, - string& errmsg, - BSONObjBuilder& result) { - if (cmdObj.firstElement().type() != jstOID) { - errmsg = "not oid"; - return false; - } + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + uassert(ErrorCodes::InvalidBSONType, "not oid", cmdObj.firstElement().type() == jstOID); const OID& oid = cmdObj.firstElement().__oid(); result.append("oid", oid); diff --git a/src/mongo/db/commands/drop_indexes.cpp b/src/mongo/db/commands/drop_indexes.cpp index 0f82ec1dde1..ae15287286e 100644 --- a/src/mongo/db/commands/drop_indexes.cpp +++ b/src/mongo/db/commands/drop_indexes.cpp @@ -55,6 +55,7 @@ #include "mongo/db/timeseries/timeseries_commands_conversion_helper.h" #include "mongo/db/vector_clock.h" #include "mongo/logv2/log.h" +#include "mongo/util/assert_util.h" #include "mongo/util/exit_code.h" #include "mongo/util/quick_exit.h" @@ -118,7 +119,7 @@ public: }; } cmdDropIndexes; -class CmdReIndex : public ErrmsgCommandDeprecated { +class CmdReIndex : public BasicCommand { public: AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { // Even though reIndex is a standalone-only command, this will return that the command is @@ -139,19 +140,18 @@ public: actions.addAction(ActionType::reIndex); out->push_back(Privilege(parseResourcePattern(dbname, cmdObj), actions)); } - CmdReIndex() : ErrmsgCommandDeprecated("reIndex") {} + CmdReIndex() : BasicCommand("reIndex") {} - bool errmsgRun(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& jsobj, - std::string& errmsg, - BSONObjBuilder& result) { + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { LOGV2_WARNING(6508600, "The reIndex command is deprecated. For more information, see " "https://mongodb.com/docs/manual/reference/command/reIndex/"); const NamespaceString toReIndexNss = - CommandHelpers::parseNsCollectionRequired(dbname, jsobj); + CommandHelpers::parseNsCollectionRequired(dbName, cmdObj); LOGV2(20457, "CMD: reIndex {namespace}", "CMD reIndex", "namespace"_attr = toReIndexNss); @@ -218,12 +218,12 @@ public: const BSONObj key = spec.getObjectField("key"); const Status keyStatus = index_key_validate::validateKeyPattern(key, defaultIndexVersion); - if (!keyStatus.isOK()) { - errmsg = str::stream() + + uassertStatusOKWithContext( + keyStatus, + str::stream() << "Cannot rebuild index " << spec << ": " << keyStatus.reason() - << " For more info see http://dochub.mongodb.org/core/index-validation"; - return false; - } + << " For more info see http://dochub.mongodb.org/core/index-validation"); } } diff --git a/src/mongo/db/commands/fsync.cpp b/src/mongo/db/commands/fsync.cpp index 08730c44930..c9555a9b1ab 100644 --- a/src/mongo/db/commands/fsync.cpp +++ b/src/mongo/db/commands/fsync.cpp @@ -88,13 +88,13 @@ private: static bool _shutdownTaskRegistered; }; -class FSyncCommand : public ErrmsgCommandDeprecated { +class FSyncCommand : public BasicCommand { public: static const char* url() { return "http://dochub.mongodb.org/core/fsynccommand"; } - FSyncCommand() : ErrmsgCommandDeprecated("fsync") {} + FSyncCommand() : BasicCommand("fsync") {} virtual ~FSyncCommand() { // The FSyncLockThread is owned by the FSyncCommand and accesses FsyncCommand state. It must @@ -127,15 +127,14 @@ public: actions.addAction(ActionType::fsync); out->push_back(Privilege(ResourcePattern::forClusterResource(), actions)); } - virtual bool errmsgRun(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& cmdObj, - std::string& errmsg, - BSONObjBuilder& result) { - if (opCtx->lockState()->isLocked()) { - errmsg = "fsync: Cannot execute fsync command from contexts that hold a data lock"; - return false; - } + + bool run(OperationContext* opCtx, + const DatabaseName&, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + uassert(ErrorCodes::IllegalOperation, + "fsync: Cannot execute fsync command from contexts that hold a data lock", + !opCtx->lockState()->isLocked()); const bool lock = cmdObj["lock"].trueValue(); LOGV2(20461, "CMD fsync: lock:{lock}", "CMD fsync", "lock"_attr = lock); @@ -275,9 +274,9 @@ private: } fsyncCmd; -class FSyncUnlockCommand : public ErrmsgCommandDeprecated { +class FSyncUnlockCommand : public BasicCommand { public: - FSyncUnlockCommand() : ErrmsgCommandDeprecated("fsyncUnlock") {} + FSyncUnlockCommand() : BasicCommand("fsyncUnlock") {} bool supportsWriteConcern(const BSONObj& cmd) const override { return false; @@ -300,11 +299,10 @@ public: return isAuthorized ? Status::OK() : Status(ErrorCodes::Unauthorized, "Unauthorized"); } - bool errmsgRun(OperationContext* opCtx, - const std::string& db, - const BSONObj& cmdObj, - std::string& errmsg, - BSONObjBuilder& result) override { + bool run(OperationContext* opCtx, + const DatabaseName&, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { LOGV2(20465, "command: unlock requested"); Lock::ExclusiveLock lk(opCtx->lockState(), commandMutex); @@ -312,10 +310,8 @@ public: stdx::unique_lock stateLock(fsyncCmd.lockStateMutex); auto lockCount = fsyncCmd.getLockCount_inLock(); - if (lockCount == 0) { - errmsg = "fsyncUnlock called when not locked"; - return false; - } + + uassert(ErrorCodes::IllegalOperation, "fsyncUnlock called when not locked", lockCount != 0); fsyncCmd.releaseLock_inLock(stateLock); diff --git a/src/mongo/db/commands/get_last_error.cpp b/src/mongo/db/commands/get_last_error.cpp index 063ebd12636..23d7d68e3ed 100644 --- a/src/mongo/db/commands/get_last_error.cpp +++ b/src/mongo/db/commands/get_last_error.cpp @@ -40,9 +40,9 @@ namespace mongo { namespace { -class CmdGetLastError : public ErrmsgCommandDeprecated { +class CmdGetLastError : public BasicCommand { public: - CmdGetLastError() : ErrmsgCommandDeprecated("getLastError", "getlasterror") {} + CmdGetLastError() : BasicCommand("getLastError", "getlasterror") {} virtual bool supportsWriteConcern(const BSONObj& cmd) const override { return false; } @@ -61,11 +61,10 @@ public: return "no longer supported"; } - bool errmsgRun(OperationContext* opCtx, - const std::string&, - const BSONObj&, - std::string&, - BSONObjBuilder&) { + bool run(OperationContext* opCtx, + const DatabaseName&, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { uasserted(5739000, "getLastError command is not supported"); return false; } diff --git a/src/mongo/db/commands/map_reduce_agg.cpp b/src/mongo/db/commands/map_reduce_agg.cpp index 7d5de91cc91..073e655dcb3 100644 --- a/src/mongo/db/commands/map_reduce_agg.cpp +++ b/src/mongo/db/commands/map_reduce_agg.cpp @@ -118,6 +118,7 @@ auto makeExpressionContext(OperationContext* opCtx, } // namespace bool runAggregationMapReduce(OperationContext* opCtx, + const DatabaseName& dbName, const BSONObj& cmd, BSONObjBuilder& result, boost::optional verbosity) { @@ -139,6 +140,7 @@ bool runAggregationMapReduce(OperationContext* opCtx, Timer cmdTimer; + // TODO SERVER-68721: create IDLParserContext with tenant id of dbName. auto parsedMr = MapReduceCommandRequest::parse(IDLParserContext("mapReduce"), cmd); auto expCtx = makeExpressionContext(opCtx, parsedMr, verbosity); auto runnablePipeline = [&]() { diff --git a/src/mongo/db/commands/map_reduce_agg.h b/src/mongo/db/commands/map_reduce_agg.h index e8448de66c7..24acb70f0a7 100644 --- a/src/mongo/db/commands/map_reduce_agg.h +++ b/src/mongo/db/commands/map_reduce_agg.h @@ -45,6 +45,7 @@ namespace mongo::map_reduce_agg { * Executes a mapReduce command against a replica set/standalone. */ bool runAggregationMapReduce(OperationContext* opCtx, + const DatabaseName& dbName, const BSONObj& cmd, BSONObjBuilder& result, boost::optional verbosity); diff --git a/src/mongo/db/commands/map_reduce_command.cpp b/src/mongo/db/commands/map_reduce_command.cpp index be76d462be1..a29f1db0e60 100644 --- a/src/mongo/db/commands/map_reduce_command.cpp +++ b/src/mongo/db/commands/map_reduce_command.cpp @@ -62,18 +62,18 @@ public: } void _explainImpl(OperationContext* opCtx, + const DatabaseName& dbName, const BSONObj& cmd, BSONObjBuilder& result, boost::optional verbosity) const override { - map_reduce_agg::runAggregationMapReduce(opCtx, cmd, result, verbosity); + map_reduce_agg::runAggregationMapReduce(opCtx, dbName, cmd, result, verbosity); } - bool errmsgRun(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& cmd, - std::string& errmsg, - BSONObjBuilder& result) final { - return map_reduce_agg::runAggregationMapReduce(opCtx, cmd, result, boost::none); + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + return map_reduce_agg::runAggregationMapReduce(opCtx, dbName, cmdObj, result, boost::none); } } mapReduceCommand; diff --git a/src/mongo/db/commands/map_reduce_command_base.h b/src/mongo/db/commands/map_reduce_command_base.h index f3806c40766..48a9522e0d8 100644 --- a/src/mongo/db/commands/map_reduce_command_base.h +++ b/src/mongo/db/commands/map_reduce_command_base.h @@ -35,9 +35,9 @@ namespace mongo { -class MapReduceCommandBase : public ErrmsgCommandDeprecated { +class MapReduceCommandBase : public BasicCommand { public: - MapReduceCommandBase() : ErrmsgCommandDeprecated("mapReduce", "mapreduce") {} + MapReduceCommandBase() : BasicCommand("mapReduce", "mapreduce") {} std::string help() const override { return "Runs the mapReduce command. See http://dochub.mongodb.org/core/mapreduce for " @@ -89,6 +89,7 @@ public: } virtual void _explainImpl(OperationContext* opCtx, + const DatabaseName& dbName, const BSONObj& cmd, BSONObjBuilder& result, boost::optional verbosity) const = 0; @@ -100,7 +101,11 @@ public: auto builder = result->getBodyBuilder(); auto explain = boost::make_optional(verbosity); try { - _explainImpl(opCtx, request.body, builder, explain); + _explainImpl(opCtx, + DatabaseName(request.getValidatedTenantId(), request.getDatabase()), + request.body, + builder, + explain); } catch (...) { return exceptionToStatus(); } diff --git a/src/mongo/db/commands/parameters.cpp b/src/mongo/db/commands/parameters.cpp index e9478a9501c..c498273f107 100644 --- a/src/mongo/db/commands/parameters.cpp +++ b/src/mongo/db/commands/parameters.cpp @@ -208,9 +208,9 @@ Mutex autoServiceDescriptorMutex; std::string autoServiceDescriptorValue; } // namespace -class CmdGet : public ErrmsgCommandDeprecated { +class CmdGet : public BasicCommand { public: - CmdGet() : ErrmsgCommandDeprecated("getParameter") {} + CmdGet() : BasicCommand("getParameter") {} AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { return AllowedOnSecondary::kAlways; } @@ -237,11 +237,10 @@ public: h += "{ getParameter:'*' } or { getParameter:{allParameters: true} } to get everything\n"; return h; } - bool errmsgRun(OperationContext* opCtx, - const string& dbname, - const BSONObj& cmdObj, - string& errmsg, - BSONObjBuilder& result) override { + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { const auto options = parseGetParameterOptions(cmdObj.firstElement()); const bool all = options.getAllParameters(); @@ -263,17 +262,14 @@ public: } } } - if (before == result.len()) { - errmsg = "no option found to get"; - return false; - } + uassert(ErrorCodes::InvalidOptions, "no option found to get", before != result.len()); return true; } } cmdGet; -class CmdSet : public ErrmsgCommandDeprecated { +class CmdSet : public BasicCommand { public: - CmdSet() : ErrmsgCommandDeprecated("setParameter") {} + CmdSet() : BasicCommand("setParameter") {} AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { return AllowedOnSecondary::kAlways; } @@ -297,11 +293,10 @@ public: appendParameterNames(&h); return h; } - bool errmsgRun(OperationContext* opCtx, - const string& dbname, - const BSONObj& cmdObj, - string& errmsg, - BSONObjBuilder& result) { + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { int numSet = 0; bool found = false; @@ -330,28 +325,24 @@ public: ServerParameter::Map::const_iterator foundParameter = parameterMap.find(parameterName); // Check to see if this is actually a valid parameter - if (foundParameter == parameterMap.end()) { - errmsg = str::stream() << "attempted to set unrecognized parameter [" - << parameterName << "], use help:true to see options "; - return false; - } + uassert(ErrorCodes::InvalidOptions, + str::stream() << "attempted to set unrecognized parameter [" << parameterName + << "], use help:true to see options ", + foundParameter != parameterMap.end()); // Make sure we are allowed to change this parameter - if (!foundParameter->second->allowedToChangeAtRuntime()) { - errmsg = str::stream() - << "not allowed to change [" << parameterName << "] at runtime"; - return false; - } + uassert(ErrorCodes::IllegalOperation, + str::stream() << "not allowed to change [" << parameterName << "] at runtime", + foundParameter->second->allowedToChangeAtRuntime()); // Make sure we are only setting this parameter once - if (parametersToSet.count(parameterName)) { - errmsg = str::stream() - << "attempted to set parameter [" << parameterName - << "] twice in the same setParameter command, " - << "once to value: [" << parametersToSet[parameterName].toString(false) - << "], and once to value: [" << parameter.toString(false) << "]"; - return false; - } + uassert(ErrorCodes::InvalidOptions, + str::stream() << "attempted to set parameter [" << parameterName + << "] twice in the same setParameter command, " + << "once to value: [" + << parametersToSet[parameterName].toString(false) + << "], and once to value: [" << parameter.toString(false) << "]", + parametersToSet.count(parameterName) == 0); parametersToSet[parameterName] = parameter; } @@ -367,20 +358,19 @@ public: ServerParameter::Map::const_iterator foundParameter = parameterMap.find(parameterName); - if (foundParameter == parameterMap.end()) { - errmsg = str::stream() << "Parameter: " << parameterName << " that was " - << "avaliable during our first lookup in the registered " - << "parameters map is no longer available."; - return false; - } + uassert(ErrorCodes::InvalidOptions, + str::stream() << "Parameter: " << parameterName << " that was " + << "avaliable during our first lookup in the registered " + << "parameters map is no longer available.", + foundParameter != parameterMap.end()); - if (parameterName == "requireApiVersion" && parameter.trueValue() && - (serverGlobalParams.clusterRole == ClusterRole::ConfigServer || - serverGlobalParams.clusterRole == ClusterRole::ShardServer)) { - errmsg = str::stream() - << "Cannot set parameter requireApiVersion=true on a shard or config server"; - return false; - } + uassert( + ErrorCodes::IllegalOperation, + str::stream() + << "Cannot set parameter requireApiVersion=true on a shard or config server", + parameterName != "requireApiVersion" || !parameter.trueValue() || + (serverGlobalParams.clusterRole != ClusterRole::ConfigServer && + serverGlobalParams.clusterRole != ClusterRole::ShardServer)); auto oldValueObj = ([&] { BSONObjBuilder bb; @@ -435,10 +425,9 @@ public: numSet++; } - if (numSet == 0 && !found) { - errmsg = "no option found to set, use help:true to see options "; - return false; - } + uassert(ErrorCodes::InvalidOptions, + "no option found to set, use help:true to see options ", + numSet != 0 || found); return true; } diff --git a/src/mongo/db/commands/test_commands.cpp b/src/mongo/db/commands/test_commands.cpp index 4b3e63cc9dc..5cedacf760f 100644 --- a/src/mongo/db/commands/test_commands.cpp +++ b/src/mongo/db/commands/test_commands.cpp @@ -61,9 +61,9 @@ using std::string; using std::stringstream; /* For testing only, not for general use. Enabled via command-line */ -class GodInsert : public ErrmsgCommandDeprecated { +class GodInsert : public BasicCommand { public: - GodInsert() : ErrmsgCommandDeprecated("godinsert") {} + GodInsert() : BasicCommand("godinsert") {} virtual bool adminOnly() const { return false; } @@ -80,20 +80,17 @@ public: std::string help() const override { return "internal. for testing only."; } - virtual bool errmsgRun(OperationContext* opCtx, - const string& dbname, - const BSONObj& cmdObj, - string& errmsg, - BSONObjBuilder& result) { - const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbname, cmdObj)); + bool run(OperationContext* opCtx, + const DatabaseName& dbName, + const BSONObj& cmdObj, + BSONObjBuilder& result) override { + const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbName, cmdObj)); LOGV2(20505, "Test-only command 'godinsert' invoked coll:{collection}", "Test-only command 'godinsert' invoked", "collection"_attr = nss.coll()); BSONObj obj = cmdObj["obj"].embeddedObjectUserCheck(); - // TODO SERVER-66561 Use DatabaseName obj passed in - DatabaseName dbName(boost::none, dbname); Lock::DBLock lk(opCtx, dbName, MODE_X); OldClientContext ctx(opCtx, nss); Database* db = ctx.db(); @@ -104,10 +101,7 @@ public: CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, nss); if (!collection) { collection = db->createCollection(opCtx, nss); - if (!collection) { - errmsg = "could not create collection"; - return false; - } + uassert(ErrorCodes::CannotCreateCollection, "could not create collection", collection); } OpDebug* const nullOpDebug = nullptr; Status status = collection_internal::insertDocument( -- cgit v1.2.1