SERVER-23480 make Command::parseNsCollectionRequired and oplog.cpp's parseNs return NamespaceString

12 files changed, 67 insertions, 64 deletions

diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp
index a67898637b9..4780c84f7a3 100644
--- a/src/mongo/db/commands.cpp
+++ b/src/mongo/db/commands.cpp
@@ -86,16 +86,17 @@ string Command::parseNsFullyQualified(const string& dbname, const BSONObj& cmdOb
     return first.String();
 }
 
-string Command::parseNsCollectionRequired(const string& dbname, const BSONObj& cmdObj) const {
+NamespaceString Command::parseNsCollectionRequired(const string& dbname,
+                                                   const BSONObj& cmdObj) const {
     // Accepts both BSON String and Symbol for collection name per SERVER-16260
     // TODO(kangas) remove Symbol support in MongoDB 3.0 after Ruby driver audit
     BSONElement first = cmdObj.firstElement();
-    uassert(17009,
-            "no collection name specified",
-            first.canonicalType() == canonicalizeBSONType(mongo::String) &&
-                first.valuestrsize() > 0);
-    std::string coll = first.valuestr();
-    return dbname + '.' + coll;
+    uassert(40072,
+            str::stream() << "collection name has invalid type " << typeName(first.type()),
+            first.canonicalType() == canonicalizeBSONType(mongo::String));
+    NamespaceString nss(dbname, first.valuestr());
+    uassert(ErrorCodes::InvalidNamespace, "Not a valid namespace", nss.isValid());
+    return nss;
 }
 
 /*virtual*/ string Command::parseNs(const string& dbname, const BSONObj& cmdObj) const {
diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h
index e16059515ae..d543f8a6dbd 100644
--- a/src/mongo/db/commands.h
+++ b/src/mongo/db/commands.h
@@ -72,7 +72,8 @@ protected:
 
     // The type of the first field in 'cmdObj' must be mongo::String or Symbol.
     // The first field is interpreted as a collection name.
-    std::string parseNsCollectionRequired(const std::string& dbname, const BSONObj& cmdObj) const;
+    NamespaceString parseNsCollectionRequired(const std::string& dbname,
+                                              const BSONObj& cmdObj) const;
 
 public:
     typedef StringMap<Command*> CommandMap;
diff --git a/src/mongo/db/commands/compact.cpp b/src/mongo/db/commands/compact.cpp
index bc8061dfaa5..8746aeab13c 100644
--- a/src/mongo/db/commands/compact.cpp
+++ b/src/mongo/db/commands/compact.cpp
@@ -92,7 +92,7 @@ public:
                      int,
                      string& errmsg,
                      BSONObjBuilder& result) {
-        const std::string nsToCompact = parseNsCollectionRequired(db, cmdObj);
+        NamespaceString nss = parseNsCollectionRequired(db, cmdObj);
 
         repl::ReplicationCoordinator* replCoord = repl::getGlobalReplicationCoordinator();
         if (replCoord->getMemberState().primary() && !cmdObj["force"].trueValue()) {
@@ -102,7 +102,6 @@ public:
             return false;
         }
 
-        NamespaceString nss(nsToCompact);
         if (!nss.isNormal()) {
             errmsg = "bad namespace name";
             return false;
diff --git a/src/mongo/db/commands/drop_indexes.cpp b/src/mongo/db/commands/drop_indexes.cpp
index 7bd1c824102..9829ce29e3c 100644
--- a/src/mongo/db/commands/drop_indexes.cpp
+++ b/src/mongo/db/commands/drop_indexes.cpp
@@ -90,8 +90,8 @@ public:
              int,
              string& errmsg,
              BSONObjBuilder& result) {
-        const std::string ns = parseNsCollectionRequired(dbname, jsobj);
-        return appendCommandStatus(result, dropIndexes(txn, NamespaceString(ns), jsobj, &result));
+        const NamespaceString nss = parseNsCollectionRequired(dbname, jsobj);
+        return appendCommandStatus(result, dropIndexes(txn, nss, jsobj, &result));
     }
 
 } cmdDropIndexes;
@@ -124,22 +124,22 @@ public:
              BSONObjBuilder& result) {
         DBDirectClient db(txn);
 
-        const std::string toDeleteNs = parseNsCollectionRequired(dbname, jsobj);
+        const NamespaceString toDeleteNs = parseNsCollectionRequired(dbname, jsobj);
 
         LOG(0) << "CMD: reIndex " << toDeleteNs << endl;
 
         ScopedTransaction transaction(txn, MODE_IX);
         Lock::DBLock dbXLock(txn->lockState(), dbname, MODE_X);
-        OldClientContext ctx(txn, toDeleteNs);
+        OldClientContext ctx(txn, toDeleteNs.ns());
 
-        Collection* collection = ctx.db()->getCollection(toDeleteNs);
+        Collection* collection = ctx.db()->getCollection(toDeleteNs.ns());
 
         if (!collection) {
             errmsg = "ns not found";
             return false;
         }
 
-        BackgroundOperation::assertNoBgOpInProgForNs(toDeleteNs);
+        BackgroundOperation::assertNoBgOpInProgForNs(toDeleteNs.ns());
 
         vector<BSONObj> all;
         {
diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp
index 2bc45997561..17f4b3d6be5 100644
--- a/src/mongo/db/commands/find_and_modify.cpp
+++ b/src/mongo/db/commands/find_and_modify.cpp
@@ -231,14 +231,14 @@ public:
                    ExplainCommon::Verbosity verbosity,
                    const rpc::ServerSelectionMetadata&,
                    BSONObjBuilder* out) const override {
-        const std::string fullNs = parseNsCollectionRequired(dbName, cmdObj);
-        Status allowedWriteStatus = userAllowedWriteNS(fullNs);
+        const NamespaceString fullNs = parseNsCollectionRequired(dbName, cmdObj);
+        Status allowedWriteStatus = userAllowedWriteNS(fullNs.ns());
         if (!allowedWriteStatus.isOK()) {
             return allowedWriteStatus;
         }
 
         StatusWith<FindAndModifyRequest> parseStatus =
-            FindAndModifyRequest::parseFromBSON(NamespaceString(fullNs), cmdObj);
+            FindAndModifyRequest::parseFromBSON(NamespaceString(fullNs.ns()), cmdObj);
         if (!parseStatus.isOK()) {
             return parseStatus.getStatus();
         }
@@ -322,14 +322,14 @@ public:
              BSONObjBuilder& result) override {
         // findAndModify command is not replicated directly.
         invariant(txn->writesAreReplicated());
-        const std::string fullNs = parseNsCollectionRequired(dbName, cmdObj);
-        Status allowedWriteStatus = userAllowedWriteNS(fullNs);
+        const NamespaceString fullNs = parseNsCollectionRequired(dbName, cmdObj);
+        Status allowedWriteStatus = userAllowedWriteNS(fullNs.ns());
         if (!allowedWriteStatus.isOK()) {
             return appendCommandStatus(result, allowedWriteStatus);
         }
 
         StatusWith<FindAndModifyRequest> parseStatus =
-            FindAndModifyRequest::parseFromBSON(NamespaceString(fullNs), cmdObj);
+            FindAndModifyRequest::parseFromBSON(NamespaceString(fullNs.ns()), cmdObj);
         if (!parseStatus.isOK()) {
             return appendCommandStatus(result, parseStatus.getStatus());
         }
diff --git a/src/mongo/db/commands/test_commands.cpp b/src/mongo/db/commands/test_commands.cpp
index f7fc056a931..b286aa3f252 100644
--- a/src/mongo/db/commands/test_commands.cpp
+++ b/src/mongo/db/commands/test_commands.cpp
@@ -223,7 +223,7 @@ public:
                      int,
                      string& errmsg,
                      BSONObjBuilder& result) {
-        const std::string fullNs = parseNsCollectionRequired(dbname, cmdObj);
+        const NamespaceString fullNs = parseNsCollectionRequired(dbname, cmdObj);
         int n = cmdObj.getIntField("n");
         bool inc = cmdObj.getBoolField("inc");  // inclusive range?
 
@@ -232,14 +232,14 @@ public:
                                        {ErrorCodes::BadValue, "n must be a positive integer"});
         }
 
-        OldClientWriteContext ctx(txn, fullNs);
+        OldClientWriteContext ctx(txn, fullNs.ns());
         Collection* collection = ctx.getCollection();
 
         if (!collection) {
             return appendCommandStatus(
                 result,
                 {ErrorCodes::NamespaceNotFound,
-                 str::stream() << "collection " << fullNs << " does not exist"});
+                 str::stream() << "collection " << fullNs.ns() << " does not exist"});
         }
 
         if (!collection->isCapped()) {
@@ -252,8 +252,12 @@ public:
             // Scan backwards through the collection to find the document to start truncating from.
             // We will remove 'n' documents, so start truncating from the (n + 1)th document to the
             // end.
-            std::unique_ptr<PlanExecutor> exec(InternalPlanner::collectionScan(
-                txn, fullNs, collection, PlanExecutor::YIELD_MANUAL, InternalPlanner::BACKWARD));
+            std::unique_ptr<PlanExecutor> exec(
+                InternalPlanner::collectionScan(txn,
+                                                fullNs.ns(),
+                                                collection,
+                                                PlanExecutor::YIELD_MANUAL,
+                                                InternalPlanner::BACKWARD));
 
             for (int i = 0; i < n + 1; ++i) {
                 PlanExecutor::ExecState state = exec->getNext(nullptr, &end);
@@ -294,9 +298,9 @@ public:
                      int,
                      string& errmsg,
                      BSONObjBuilder& result) {
-        const std::string ns = parseNsCollectionRequired(dbname, cmdObj);
+        const NamespaceString nss = parseNsCollectionRequired(dbname, cmdObj);
 
-        return appendCommandStatus(result, emptyCapped(txn, NamespaceString(ns)));
+        return appendCommandStatus(result, emptyCapped(txn, nss));
     }
 };
 
diff --git a/src/mongo/db/commands/touch.cpp b/src/mongo/db/commands/touch.cpp
index 0ad5056ccad..80ead76acc9 100644
--- a/src/mongo/db/commands/touch.cpp
+++ b/src/mongo/db/commands/touch.cpp
@@ -89,9 +89,7 @@ public:
                      int,
                      string& errmsg,
                      BSONObjBuilder& result) {
-        const std::string ns = parseNsCollectionRequired(dbname, cmdObj);
-
-        const NamespaceString nss(ns);
+        const NamespaceString nss = parseNsCollectionRequired(dbname, cmdObj);
         if (!nss.isNormal()) {
             errmsg = "bad namespace name";
             return false;
diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index ea1b9e4fb27..00f456eb91e 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -488,21 +488,21 @@ public:
                      int,
                      string& errmsg,
                      BSONObjBuilder& result) {
-        const std::string nsToDrop = parseNsCollectionRequired(dbname, cmdObj);
+        const NamespaceString nsToDrop = parseNsCollectionRequired(dbname, cmdObj);
 
-        if (NamespaceString::virtualized(nsToDrop)) {
+        if (NamespaceString::virtualized(nsToDrop.ns())) {
             errmsg = "can't drop a virtual collection";
             return false;
         }
 
         if ((repl::getGlobalReplicationCoordinator()->getReplicationMode() !=
              repl::ReplicationCoordinator::modeNone) &&
-            NamespaceString(nsToDrop).isOplog()) {
+            nsToDrop.isOplog()) {
             errmsg = "can't drop live oplog while replicating";
             return false;
         }
 
-        return appendCommandStatus(result, dropCollection(txn, NamespaceString(nsToDrop), result));
+        return appendCommandStatus(result, dropCollection(txn, nsToDrop, result));
     }
 
 } cmdDrop;
@@ -1033,8 +1033,8 @@ public:
              int,
              string& errmsg,
              BSONObjBuilder& result) {
-        const std::string ns = parseNsCollectionRequired(dbname, jsobj);
-        return appendCommandStatus(result, collMod(txn, NamespaceString(ns), jsobj, &result));
+        const NamespaceString nss = parseNsCollectionRequired(dbname, jsobj);
+        return appendCommandStatus(result, collMod(txn, nss, jsobj, &result));
     }
 
 } collectionModCommand;
diff --git a/src/mongo/db/geo/haystack.cpp b/src/mongo/db/geo/haystack.cpp
index 9fc3a2cccf8..6e736ca91ea 100644
--- a/src/mongo/db/geo/haystack.cpp
+++ b/src/mongo/db/geo/haystack.cpp
@@ -93,9 +93,9 @@ public:
              int,
              string& errmsg,
              BSONObjBuilder& result) {
-        const std::string ns = parseNsCollectionRequired(dbname, cmdObj);
+        const NamespaceString nss = parseNsCollectionRequired(dbname, cmdObj);
 
-        AutoGetCollectionForRead ctx(txn, ns);
+        AutoGetCollectionForRead ctx(txn, nss.ns());
 
         Collection* collection = ctx.getCollection();
         if (!collection) {
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 5acc636c0cb..94602815666 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -611,11 +611,11 @@ void createOplog(OperationContext* txn) {
 namespace {
 NamespaceString parseNs(const string& ns, const BSONObj& cmdObj) {
     BSONElement first = cmdObj.firstElement();
-    uassert(28635,
-            "no collection name specified",
-            first.canonicalType() == canonicalizeBSONType(mongo::String) &&
-                first.valuestrsize() > 0);
+    uassert(40073,
+            str::stream() << "collection name has invalid type " << typeName(first.type()),
+            first.canonicalType() == canonicalizeBSONType(mongo::String));
     std::string coll = first.valuestr();
+    uassert(28635, "no collection name specified", !coll.empty());
     return NamespaceString(NamespaceString(ns).db().toString(), coll);
 }
 
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 2d34c22ddf0..cf4d5051c93 100644
--- a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
+++ b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
@@ -87,7 +87,7 @@ public:
                            ExplainCommon::Verbosity verbosity,
                            const rpc::ServerSelectionMetadata& serverSelectionMetadata,
                            BSONObjBuilder* out) const {
-        const string ns = parseNsCollectionRequired(dbName, cmdObj);
+        const NamespaceString nss = parseNsCollectionRequired(dbName, cmdObj);
 
         auto status = Grid::get(txn)->catalogCache()->getDatabase(txn, dbName);
         uassertStatusOK(status);
@@ -96,10 +96,10 @@ public:
         shared_ptr<ChunkManager> chunkMgr;
         shared_ptr<Shard> shard;
 
-        if (!conf->isShardingEnabled() || !conf->isSharded(ns)) {
+        if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) {
             shard = Grid::get(txn)->shardRegistry()->getShard(txn, conf->getPrimaryId());
         } else {
-            chunkMgr = _getChunkManager(txn, conf, ns);
+            chunkMgr = _getChunkManager(txn, conf, nss);
 
             const BSONObj query = cmdObj.getObjectField("query");
 
@@ -123,7 +123,7 @@ public:
         Timer timer;
 
         BSONObjBuilder result;
-        bool ok = _runCommand(txn, conf, chunkMgr, shard->getId(), ns, explainCmd.obj(), result);
+        bool ok = _runCommand(txn, conf, chunkMgr, shard->getId(), nss, explainCmd.obj(), result);
         long long millisElapsed = timer.millis();
 
         if (!ok) {
@@ -150,16 +150,16 @@ public:
                      int options,
                      std::string& errmsg,
                      BSONObjBuilder& result) {
-        const string ns = parseNsCollectionRequired(dbName, cmdObj);
+        const NamespaceString nss = parseNsCollectionRequired(dbName, cmdObj);
 
         // findAndModify should only be creating database if upsert is true, but this would require
         // that the parsing be pulled into this function.
         auto conf = uassertStatusOK(dbutil::implicitCreateDb(txn, dbName));
-        if (!conf->isShardingEnabled() || !conf->isSharded(ns)) {
-            return _runCommand(txn, conf, nullptr, conf->getPrimaryId(), ns, cmdObj, result);
+        if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) {
+            return _runCommand(txn, conf, nullptr, conf->getPrimaryId(), nss, cmdObj, result);
         }
 
-        shared_ptr<ChunkManager> chunkMgr = _getChunkManager(txn, conf, ns);
+        shared_ptr<ChunkManager> chunkMgr = _getChunkManager(txn, conf, nss);
 
         const BSONObj query = cmdObj.getObjectField("query");
 
@@ -172,7 +172,7 @@ public:
         BSONObj shardKey = status.getValue();
         shared_ptr<Chunk> chunk = chunkMgr->findIntersectingChunk(txn, shardKey);
 
-        bool ok = _runCommand(txn, conf, chunkMgr, chunk->getShardId(), ns, cmdObj, result);
+        bool ok = _runCommand(txn, conf, chunkMgr, chunk->getShardId(), nss, cmdObj, result);
         if (ok) {
             // check whether split is necessary (using update object for size heuristic)
             if (mongosGlobalParams.shouldAutoSplit) {
@@ -186,8 +186,8 @@ public:
 private:
     shared_ptr<ChunkManager> _getChunkManager(OperationContext* txn,
                                               shared_ptr<DBConfig> conf,
-                                              const string& ns) const {
-        shared_ptr<ChunkManager> chunkMgr = conf->getChunkManager(txn, ns);
+                                              const NamespaceString& nss) const {
+        shared_ptr<ChunkManager> chunkMgr = conf->getChunkManager(txn, nss.ns());
         massert(13002, "shard internal error chunk manager should never be null", chunkMgr);
 
         return chunkMgr;
@@ -216,13 +216,13 @@ private:
                      shared_ptr<DBConfig> conf,
                      shared_ptr<ChunkManager> chunkManager,
                      const ShardId& shardId,
-                     const string& ns,
+                     const NamespaceString& nss,
                      const BSONObj& cmdObj,
                      BSONObjBuilder& result) const {
         BSONObj res;
 
         const auto shard = Grid::get(txn)->shardRegistry()->getShard(txn, shardId);
-        ShardConnection conn(shard->getConnString(), ns, chunkManager);
+        ShardConnection conn(shard->getConnString(), nss.ns(), chunkManager);
         bool ok = conn->runCommand(conf->name(), cmdObj, res);
         conn.done();
 
diff --git a/src/mongo/s/commands/commands_public.cpp b/src/mongo/s/commands/commands_public.cpp
index 8e248b45d49..f15d63d65b4 100644
--- a/src/mongo/s/commands/commands_public.cpp
+++ b/src/mongo/s/commands/commands_public.cpp
@@ -430,14 +430,14 @@ public:
              int options,
              string& errmsg,
              BSONObjBuilder& output) {
-        const string ns = parseNsCollectionRequired(dbName, cmdObj);
+        const NamespaceString nss = parseNsCollectionRequired(dbName, cmdObj);
 
         auto conf = uassertStatusOK(grid.catalogCache()->getDatabase(txn, dbName));
-        if (!conf->isShardingEnabled() || !conf->isSharded(ns)) {
+        if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) {
             return passthrough(txn, conf, cmdObj, output);
         }
 
-        ChunkManagerPtr cm = conf->getChunkManager(txn, ns);
+        ChunkManagerPtr cm = conf->getChunkManager(txn, nss.ns());
         massert(40051, "chunk manager should not be null", cm);
 
         vector<Strategy::CommandResult> results;
@@ -552,20 +552,20 @@ public:
             return appendCommandStatus(result, status.getStatus());
         }
 
-        const string fullns = dbName + "." + cmdObj.firstElement().valuestrsafe();
+        const NamespaceString fullns = parseNsCollectionRequired(dbName, cmdObj);
 
         log() << "DROP: " << fullns;
 
         const auto& db = status.getValue();
-        if (!db->isShardingEnabled() || !db->isSharded(fullns)) {
+        if (!db->isShardingEnabled() || !db->isSharded(fullns.ns())) {
             log() << "\tdrop going to do passthrough";
             return passthrough(txn, db, cmdObj, result);
         }
 
-        uassertStatusOK(grid.catalogManager(txn)->dropCollection(txn, NamespaceString(fullns)));
+        uassertStatusOK(grid.catalogManager(txn)->dropCollection(txn, fullns));
 
         // Force a full reload next time the just dropped namespace is accessed
-        db->invalidateNs(fullns);
+        db->invalidateNs(fullns.ns());
 
         return true;
     }