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

10 files changed, 47 insertions, 44 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);
 }