SERVER-67461 Convert the renameCollection command to a TypedCommand

4 files changed, 247 insertions, 104 deletions

diff --git a/jstests/serverless/native_tenant_data_isolation_basic_dollar_tenant.js b/jstests/serverless/native_tenant_data_isolation_basic_dollar_tenant.js
index 1909e286073..bf0cc3a1b43 100644
--- a/jstests/serverless/native_tenant_data_isolation_basic_dollar_tenant.js
+++ b/jstests/serverless/native_tenant_data_isolation_basic_dollar_tenant.js
@@ -1,4 +1,4 @@
-// Test basic db operations in multitenancy.
+// Test basic db operations in multitenancy using $tenant.
 
 (function() {
 "use strict";
@@ -14,23 +14,30 @@ const adminDb = mongod.getDB('admin');
 assert.commandWorked(adminDb.runCommand({createUser: 'admin', pwd: 'pwd', roles: ['root']}));
 assert(adminDb.auth('admin', 'pwd'));
 
-{
-    // Test the IDL defined commands with $tenant.
-    const kTenant = ObjectId();
-    const testDb = mongod.getDB('myDb0');
-    const testColl = testDb.getCollection('myColl0');
+const kTenant = ObjectId();
+const kOtherTenant = ObjectId();
+const kDbName = 'myDb';
+const kCollName = 'myColl';
+const testDb = mongod.getDB(kDbName);
+const testColl = testDb.getCollection(kCollName);
+
+// In this jstest, the collection (defined by kCollName) and the document "{_id: 0, a: 1, b: 1}"
+// for the tenant (defined by kTenant) will be reused by all command tests. So, any test which
+// changes the collection name or document should reset it.
 
+// Test create and listCollections command on collection.
+{
     // Create a collection for the tenant kTenant, and then create a view on the collection.
     assert.commandWorked(
         testColl.getDB().createCollection(testColl.getName(), {'$tenant': kTenant}));
     assert.commandWorked(testDb.runCommand(
-        {"create": "view1", "viewOn": "myColl0", pipeline: [], '$tenant': kTenant}));
+        {"create": "view1", "viewOn": kCollName, pipeline: [], '$tenant': kTenant}));
 
     const colls = assert.commandWorked(
         testDb.runCommand({listCollections: 1, nameOnly: true, '$tenant': kTenant}));
     assert.eq(3, colls.cursor.firstBatch.length, tojson(colls.cursor.firstBatch));
     const expectedColls = [
-        {"name": "myColl0", "type": "collection"},
+        {"name": kCollName, "type": "collection"},
         {"name": "system.views", "type": "collection"},
         {"name": "view1", "type": "view"}
     ];
@@ -38,25 +45,69 @@ assert(adminDb.auth('admin', 'pwd'));
 
     // These collections should not be accessed with a different tenant.
     const collsWithDiffTenant = assert.commandWorked(
-        testDb.runCommand({listCollections: 1, nameOnly: true, '$tenant': ObjectId()}));
+        testDb.runCommand({listCollections: 1, nameOnly: true, '$tenant': kOtherTenant}));
     assert.eq(0, collsWithDiffTenant.cursor.firstBatch.length);
+}
 
-    // Insert a document to the collection.
+// Test insert and findAndModify command.
+{
     assert.commandWorked(testDb.runCommand(
-        {insert: 'myColl0', documents: [{_id: 0, a: 1, b: 1}], '$tenant': kTenant}));
+        {insert: kCollName, documents: [{_id: 0, a: 1, b: 1}], '$tenant': kTenant}));
 
-    // Find and modify the document.
     const fad1 = assert.commandWorked(testDb.runCommand(
-        {findAndModify: "myColl0", query: {a: 1}, update: {$inc: {a: 10}}, '$tenant': kTenant}));
+        {findAndModify: kCollName, query: {a: 1}, update: {$inc: {a: 10}}, '$tenant': kTenant}));
     assert.eq({_id: 0, a: 1, b: 1}, fad1.value);
-    const fad2 = assert.commandWorked(testDb.runCommand(
-        {findAndModify: "myColl0", query: {a: 11}, update: {$inc: {a: 10}}, '$tenant': kTenant}));
+    const fad2 = assert.commandWorked(testDb.runCommand({
+        findAndModify: kCollName,
+        query: {a: 11},
+        update: {$set: {a: 1, b: 1}},
+        '$tenant': kTenant
+    }));
     assert.eq({_id: 0, a: 11, b: 1}, fad2.value);
     // This document should not be accessed with a different tenant.
-    const fad3 = assert.commandWorked(testDb.runCommand(
-        {findAndModify: "myColl0", query: {b: 1}, update: {$inc: {b: 10}}, '$tenant': ObjectId()}));
+    const fad3 = assert.commandWorked(testDb.runCommand({
+        findAndModify: kCollName,
+        query: {b: 1},
+        update: {$inc: {b: 10}},
+        '$tenant': kOtherTenant
+    }));
     assert.eq(null, fad3.value);
 }
 
+// Test renameCollection command.
+{
+    const fromName = kDbName + "." + kCollName;
+    const toName = fromName + "_renamed";
+    assert.commandWorked(adminDb.runCommand(
+        {renameCollection: fromName, to: toName, dropTarget: true, '$tenant': kTenant}));
+
+    // Verify the the renamed collection by findAndModify existing documents.
+    const fad1 = assert.commandWorked(testDb.runCommand({
+        findAndModify: kCollName + "_renamed",
+        query: {a: 1},
+        update: {$inc: {a: 10}},
+        '$tenant': kTenant
+    }));
+    assert.eq({_id: 0, a: 1, b: 1}, fad1.value);
+
+    // This collection should not be accessed with a different tenant.
+    assert.commandFailedWithCode(
+        adminDb.runCommand(
+            {renameCollection: toName, to: fromName, dropTarget: true, '$tenant': kOtherTenant}),
+        ErrorCodes.NamespaceNotFound);
+
+    // Reset the collection name so other test cases can still access this collection with kCollName
+    // after this test.
+    assert.commandWorked(adminDb.runCommand(
+        {renameCollection: toName, to: fromName, dropTarget: true, '$tenant': kTenant}));
+    const fad2 = assert.commandWorked(testDb.runCommand({
+        findAndModify: kCollName,
+        query: {a: 11},
+        update: {$set: {a: 1, b: 1}},
+        '$tenant': kTenant
+    }));
+    assert.eq({_id: 0, a: 11, b: 1}, fad2.value);
+}
+
 MongoRunner.stopMongod(mongod);
 })();
diff --git a/jstests/serverless/native_tenant_data_isolation_basic_security_token.js b/jstests/serverless/native_tenant_data_isolation_basic_security_token.js
index 0051a337e04..4527bfb0530 100644
--- a/jstests/serverless/native_tenant_data_isolation_basic_security_token.js
+++ b/jstests/serverless/native_tenant_data_isolation_basic_security_token.js
@@ -14,10 +14,18 @@ const adminDb = mongod.getDB('admin');
 assert.commandWorked(adminDb.runCommand({createUser: 'admin', pwd: 'pwd', roles: ['root']}));
 assert(adminDb.auth('admin', 'pwd'));
 
-{
-    const kTenant = ObjectId();
-    const tokenConn = new Mongo(mongod.host);
+const kTenant = ObjectId();
+const kOtherTenant = ObjectId();
+const kDbName = 'test';
+const kCollName = 'myColl0';
+const tokenConn = new Mongo(mongod.host);
+
+// In this jstest, the collection (defined by kCollName) and the document "{_id: 0, a: 1, b: 1}"
+// for the tenant (defined by kTenant) will be reused by all command tests. So, any test which
+// changes the collection name or document should reset it.
 
+// Test commands using a security token for one tenant.
+{
     // Create a user for kTenant and then set the security token on the connection.
     assert.commandWorked(mongod.getDB('$external').runCommand({
         createUser: "readWriteUserTenant1",
@@ -28,37 +36,63 @@ assert(adminDb.auth('admin', 'pwd'));
         _createSecurityToken({user: "readWriteUserTenant1", db: '$external', tenant: kTenant}));
 
     // Create a collection for the tenant kTenant and then insert into it.
-    const tokenDB = tokenConn.getDB('test');
-    assert.commandWorked(tokenDB.createCollection('myColl0'));
+    const tokenDB = tokenConn.getDB(kDbName);
+    assert.commandWorked(tokenDB.createCollection(kCollName));
     assert.commandWorked(
-        tokenDB.runCommand({insert: 'myColl0', documents: [{_id: 0, a: 1, b: 1}]}));
+        tokenDB.runCommand({insert: kCollName, documents: [{_id: 0, a: 1, b: 1}]}));
 
     // Find and modify the document.
-    const fad1 = assert.commandWorked(
-        tokenDB.runCommand({findAndModify: "myColl0", query: {a: 1}, update: {$inc: {a: 10}}}));
-    assert.eq({_id: 0, a: 1, b: 1}, fad1.value);
-    const fad2 = assert.commandWorked(
-        tokenDB.runCommand({findAndModify: "myColl0", query: {a: 11}, update: {$inc: {a: 10}}}));
-    assert.eq({_id: 0, a: 11, b: 1}, fad2.value);
+    {
+        const fad1 = assert.commandWorked(
+            tokenDB.runCommand({findAndModify: kCollName, query: {a: 1}, update: {$inc: {a: 10}}}));
+        assert.eq({_id: 0, a: 1, b: 1}, fad1.value);
+        const fad2 = assert.commandWorked(tokenDB.runCommand(
+            {findAndModify: kCollName, query: {a: 11}, update: {$set: {a: 1, b: 1}}}));
+        assert.eq({_id: 0, a: 11, b: 1}, fad2.value);
+    }
 
     // Create a view on the collection.
-    assert.commandWorked(
-        tokenDB.runCommand({"create": "view1", "viewOn": "myColl0", pipeline: []}));
-
-    const colls = assert.commandWorked(tokenDB.runCommand({listCollections: 1, nameOnly: true}));
-    assert.eq(3, colls.cursor.firstBatch.length, tojson(colls.cursor.firstBatch));
-    const expectedColls = [
-        {"name": "myColl0", "type": "collection"},
-        {"name": "system.views", "type": "collection"},
-        {"name": "view1", "type": "view"}
-    ];
-    assert(arrayEq(expectedColls, colls.cursor.firstBatch), tojson(colls.cursor.firstBatch));
-
-    // Create a user for a different tenant, and set the security token on the connection. Then,
-    // check that this tenant cannot access the other tenant's collection. We reuse the same
-    // connection, but swap the token out.
-    const kOtherTenant = ObjectId();
+    {
+        assert.commandWorked(
+            tokenDB.runCommand({"create": "view1", "viewOn": kCollName, pipeline: []}));
+
+        const colls =
+            assert.commandWorked(tokenDB.runCommand({listCollections: 1, nameOnly: true}));
+        assert.eq(3, colls.cursor.firstBatch.length, tojson(colls.cursor.firstBatch));
+        const expectedColls = [
+            {"name": kCollName, "type": "collection"},
+            {"name": "system.views", "type": "collection"},
+            {"name": "view1", "type": "view"}
+        ];
+        assert(arrayEq(expectedColls, colls.cursor.firstBatch), tojson(colls.cursor.firstBatch));
+    }
+
+    // Rename the collection.
+    {
+        const fromName = kDbName + '.' + kCollName;
+        const toName = fromName + "_renamed";
+        const tokenAdminDB = tokenConn.getDB('admin');
+        assert.commandWorked(
+            tokenAdminDB.runCommand({renameCollection: fromName, to: toName, dropTarget: true}));
+
+        // Verify the the renamed collection by findAndModify existing documents.
+        const fad1 = assert.commandWorked(tokenDB.runCommand(
+            {findAndModify: kCollName + "_renamed", query: {a: 1}, update: {$set: {a: 11, b: 1}}}));
+        assert.eq({_id: 0, a: 1, b: 1}, fad1.value);
+
+        // Reset the collection name and document data.
+        assert.commandWorked(
+            tokenAdminDB.runCommand({renameCollection: toName, to: fromName, dropTarget: true}));
+        assert.commandWorked(tokenDB.runCommand(
+            {findAndModify: kCollName, query: {a: 11}, update: {$set: {a: 1, b: 1}}}));
+    }
+}
 
+// Test commands using a security token for a different tenant and check that this tenant cannot
+// access the other tenant's collection.
+{
+    // Create a user for a different tenant, and set the security token on the connection.
+    // We reuse the same connection, but swap the token out.
     assert.commandWorked(mongod.getDB('$external').runCommand({
         createUser: "readWriteUserTenant2",
         '$tenant': kOtherTenant,
@@ -67,28 +101,80 @@ assert(adminDb.auth('admin', 'pwd'));
     tokenConn._setSecurityToken(_createSecurityToken(
         {user: "readWriteUserTenant2", db: '$external', tenant: kOtherTenant}));
 
-    const tokenDB2 = tokenConn.getDB('test');
+    const tokenDB2 = tokenConn.getDB(kDbName);
     const fadOtherUser = assert.commandWorked(
-        tokenDB2.runCommand({findAndModify: "myColl0", query: {b: 1}, update: {$inc: {b: 10}}}));
+        tokenDB2.runCommand({findAndModify: kCollName, query: {b: 1}, update: {$inc: {b: 10}}}));
     assert.eq(null, fadOtherUser.value);
 
-    // Check that a privleged user with ActionType::useTenant can run findAndModify on the doc when
-    // passing the correct tenant, but not when passing a different tenant.
+    const fromName = kDbName + '.' + kCollName;
+    const toName = fromName + "_renamed";
+    assert.commandFailedWithCode(
+        adminDb.runCommand({renameCollection: fromName, to: toName, dropTarget: true}),
+        ErrorCodes.NamespaceNotFound);
+}
+
+// Test commands using a privleged user with ActionType::useTenant and check the user can run
+// commands on the doc when passing the correct tenant, but not when passing a different
+// tenant.
+{
     const privelegedConn = new Mongo(mongod.host);
     assert(privelegedConn.getDB('admin').auth('admin', 'pwd'));
     const privelegedDB = privelegedConn.getDB('test');
 
-    const fadCorrectDollarTenant = assert.commandWorked(privelegedDB.runCommand(
-        {findAndModify: "myColl0", query: {b: 1}, update: {$inc: {b: 10}}, '$tenant': kTenant}));
-    assert.eq({_id: 0, a: 21, b: 1}, fadCorrectDollarTenant.value);
-
-    const fadOtherDollarTenant = assert.commandWorked(privelegedDB.runCommand({
-        findAndModify: "myColl0",
-        query: {b: 1},
-        update: {$inc: {b: 10}},
-        '$tenant': kOtherTenant
-    }));
-    assert.eq(null, fadOtherDollarTenant.value);
+    // Find and modify the document.
+    {
+        const fadCorrectDollarTenant = assert.commandWorked(privelegedDB.runCommand({
+            findAndModify: kCollName,
+            query: {b: 1},
+            update: {$inc: {b: 10}},
+            '$tenant': kTenant
+        }));
+        assert.eq({_id: 0, a: 1, b: 1}, fadCorrectDollarTenant.value);
+
+        const fadOtherDollarTenant = assert.commandWorked(privelegedDB.runCommand({
+            findAndModify: kCollName,
+            query: {b: 11},
+            update: {$inc: {b: 10}},
+            '$tenant': kOtherTenant
+        }));
+        assert.eq(null, fadOtherDollarTenant.value);
+
+        // Reset document data.
+        assert.commandWorked(privelegedDB.runCommand({
+            findAndModify: kCollName,
+            query: {b: 11},
+            update: {$set: {a: 1, b: 1}},
+            '$tenant': kTenant
+        }));
+    }
+
+    // Rename the collection.
+    {
+        const fromName = kDbName + '.' + kCollName;
+        const toName = fromName + "_renamed";
+        const privelegedAdminDB = privelegedConn.getDB('admin');
+        assert.commandWorked(privelegedAdminDB.runCommand(
+            {renameCollection: fromName, to: toName, dropTarget: true, '$tenant': kTenant}));
+
+        // Verify the the renamed collection by findAndModify existing documents.
+        const fad1 = assert.commandWorked(privelegedDB.runCommand({
+            findAndModify: kCollName + "_renamed",
+            query: {a: 1},
+            update: {$set: {a: 11, b: 1}},
+            '$tenant': kTenant
+        }));
+        assert.eq({_id: 0, a: 1, b: 1}, fad1.value);
+
+        // Reset the collection name and document data.
+        assert.commandWorked(privelegedAdminDB.runCommand(
+            {renameCollection: toName, to: fromName, dropTarget: true, '$tenant': kTenant}));
+        assert.commandWorked(privelegedDB.runCommand({
+            findAndModify: kCollName,
+            query: {a: 11},
+            update: {$set: {a: 1, b: 1}},
+            '$tenant': kTenant
+        }));
+    }
 }
 
 MongoRunner.stopMongod(mongod);
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp
index 95f223169fa..05c2bc09a4f 100644
--- a/src/mongo/db/catalog/rename_collection.cpp
+++ b/src/mongo/db/catalog/rename_collection.cpp
@@ -299,7 +299,7 @@ Status renameCollectionWithinDB(OperationContext* opCtx,
     invariant(source.db() == target.db());
     DisableDocumentValidation validationDisabler(opCtx);
 
-    AutoGetDb autoDb(opCtx, source.db(), MODE_IX);
+    AutoGetDb autoDb(opCtx, source.dbName(), MODE_IX);
 
     boost::optional<Lock::CollectionLock> sourceLock;
     boost::optional<Lock::CollectionLock> targetLock;
@@ -353,7 +353,7 @@ Status renameCollectionWithinDBForApplyOps(OperationContext* opCtx,
     invariant(source.db() == target.db());
     DisableDocumentValidation validationDisabler(opCtx);
 
-    AutoGetDb autoDb(opCtx, source.db(), MODE_X);
+    AutoGetDb autoDb(opCtx, source.dbName(), MODE_X);
 
     auto status = checkSourceAndTargetNamespaces(
         opCtx, source, target, options, /* targetExistsAllowed */ true);
diff --git a/src/mongo/db/commands/rename_collection_cmd.cpp b/src/mongo/db/commands/rename_collection_cmd.cpp
index aacffc21439..e9e13a65bec 100644
--- a/src/mongo/db/commands/rename_collection_cmd.cpp
+++ b/src/mongo/db/commands/rename_collection_cmd.cpp
@@ -59,67 +59,73 @@ using std::stringstream;
 
 namespace {
 
-class CmdRenameCollection : public ErrmsgCommandDeprecated {
+class CmdRenameCollection final : public TypedCommand<CmdRenameCollection> {
 public:
-    CmdRenameCollection() : ErrmsgCommandDeprecated("renameCollection") {}
+    using Request = RenameCollectionCommand;
+
     virtual bool adminOnly() const {
         return true;
     }
     AllowedOnSecondary secondaryAllowed(ServiceContext*) const override {
         return AllowedOnSecondary::kNever;
     }
-    virtual bool supportsWriteConcern(const BSONObj& cmd) const override {
-        return true;
-    }
 
     bool collectsResourceConsumptionMetrics() const override {
         return true;
     }
 
-    virtual Status checkAuthForCommand(Client* client,
-                                       const std::string& dbname,
-                                       const BSONObj& cmdObj) const {
-        return rename_collection::checkAuthForRenameCollectionCommand(client, dbname, cmdObj);
-    }
     std::string help() const override {
         return " example: { renameCollection: foo.a, to: bar.b }";
     }
 
-    NamespaceString parseNs(const DatabaseName& dbName, const BSONObj& cmdObj) const override {
-        return NamespaceString(dbName.tenantId(), CommandHelpers::parseNsFullyQualified(cmdObj));
+    bool allowedWithSecurityToken() const final {
+        return true;
     }
 
-    virtual bool errmsgRun(OperationContext* opCtx,
-                           const string& dbname,
-                           const BSONObj& cmdObj,
-                           string& errmsg,
-                           BSONObjBuilder& result) {
-        auto renameRequest =
-            RenameCollectionCommand::parse(IDLParserContext("renameCollection"), cmdObj);
-
-        const auto& fromNss = renameRequest.getCommandParameter();
-        const auto& toNss = renameRequest.getTo();
-
-        uassert(
-            ErrorCodes::IllegalOperation, "Can't rename a collection to itself", fromNss != toNss);
-
-        RenameCollectionOptions options;
-        options.stayTemp = renameRequest.getStayTemp();
-        options.expectedSourceUUID = renameRequest.getCollectionUUID();
-        stdx::visit(
-            OverloadedVisitor{
-                [&options](bool dropTarget) { options.dropTarget = dropTarget; },
-                [&options](const UUID& uuid) {
-                    options.dropTarget = true;
-                    options.expectedTargetUUID = uuid;
+    class Invocation final : public InvocationBase {
+    public:
+        using InvocationBase::InvocationBase;
+
+        void typedRun(OperationContext* opCtx) {
+            const auto& fromNss = ns();
+            const auto& toNss = request().getTo();
+
+            uassert(ErrorCodes::IllegalOperation,
+                    "Can't rename a collection to itself",
+                    fromNss != toNss);
+
+            RenameCollectionOptions options;
+            options.stayTemp = request().getStayTemp();
+            options.expectedSourceUUID = request().getCollectionUUID();
+            stdx::visit(
+                OverloadedVisitor{
+                    [&options](bool dropTarget) { options.dropTarget = dropTarget; },
+                    [&options](const UUID& uuid) {
+                        options.dropTarget = true;
+                        options.expectedTargetUUID = uuid;
+                    },
                 },
-            },
-            renameRequest.getDropTarget());
-
-        validateAndRunRenameCollection(
-            opCtx, renameRequest.getCommandParameter(), renameRequest.getTo(), options);
-        return true;
-    }
+                request().getDropTarget());
+
+            validateAndRunRenameCollection(opCtx, fromNss, toNss, options);
+        }
+
+    private:
+        NamespaceString ns() const override {
+            return request().getCommandParameter();
+        }
+
+        bool supportsWriteConcern() const override {
+            return true;
+        }
+
+        void doCheckAuthorization(OperationContext* opCtx) const override {
+            uassertStatusOK(rename_collection::checkAuthForRenameCollectionCommand(
+                opCtx->getClient(),
+                request().getDbName().toStringWithTenantId(),
+                request().toBSON(BSONObj())));
+        }
+    };
 
 } cmdrenamecollection;