SERVER-43894 dropCollection should return success if the collection does not exist AND no UUID was specified

8 files changed, 71 insertions, 56 deletions

diff --git a/jstests/core/ddl/drop_collection.js b/jstests/core/ddl/drop_collection.js
index 3dd4bafbe1b..4ac5d140364 100644
--- a/jstests/core/ddl/drop_collection.js
+++ b/jstests/core/ddl/drop_collection.js
@@ -25,18 +25,12 @@ function assertCollectionDropped(ns) {
 
 const coll = db[jsTestName() + "_coll"];
 
-const isMongos = db.adminCommand({isdbgrid: 1}).isdbgrid;
-
 jsTest.log("Drop Unexistent collection.");
 {
     // Drop the collection
-    let dropRes = db.runCommand({drop: coll.getName()});
-    if (isMongos) {
-        assert.commandWorked(dropRes);
-    } else {
-        // TODO SERVER-43894 Make dropping a nonexistent collection a noop
-        assert.commandWorkedOrFailedWithCode(dropRes, ErrorCodes.NamespaceNotFound);
-    }
+    // NamespaceNotFound will be returned by mongod versions earlier than 7.0.
+    assert.commandWorkedOrFailedWithCode(db.runCommand({drop: coll.getName()}),
+                                         ErrorCodes.NamespaceNotFound);
     assertCollectionDropped(coll.getFullName());
 }
 
@@ -51,13 +45,8 @@ jsTest.log("Drop existing collection.");
     assertCollectionDropped(coll.getFullName());
 
     // Test idempotency
-    let dropRes = db.runCommand({drop: coll.getName()});
-    if (isMongos) {
-        assert.commandWorked(dropRes);
-    } else {
-        // TODO SERVER-43894 Make dropping a nonexistent collection a noop
-        assert.commandWorkedOrFailedWithCode(dropRes, ErrorCodes.NamespaceNotFound);
-    }
+    // NamespaceNotFound will be returned by mongod versions earlier than 7.0.
+    assert.commandWorkedOrFailedWithCode(db.runCommand({drop: coll.getName()}));
     assertCollectionDropped(coll.getFullName());
 }
 
diff --git a/jstests/core/query/explain/explain_upsert.js b/jstests/core/query/explain/explain_upsert.js
index 6da5921b8f0..802cf4b5343 100644
--- a/jstests/core/query/explain/explain_upsert.js
+++ b/jstests/core/query/explain/explain_upsert.js
@@ -17,7 +17,7 @@ assert.commandWorked(explain);
 
 // Collection should still not exist.
 assert.eq(0, t.count());
-assert(!t.drop());
+assert(!db.getCollectionInfos({name: t.getName()}).length);
 
 // Add a document to the collection.
 t.insert({a: 3});
@@ -27,3 +27,4 @@ explain = db.runCommand(
     {explain: {update: t.getName(), updates: [{q: {a: 1}, u: {a: 1}, upsert: true}]}});
 assert.commandWorked(explain);
 assert.eq(1, t.count());
+assert(db.getCollectionInfos({name: t.getName()}).length);
diff --git a/jstests/noPassthrough/durable_view_catalog.js b/jstests/noPassthrough/durable_view_catalog.js
index 39db5b0b855..0ecbd2e6a43 100644
--- a/jstests/noPassthrough/durable_view_catalog.js
+++ b/jstests/noPassthrough/durable_view_catalog.js
@@ -79,7 +79,7 @@ assert.commandFailedWithCode(viewsDB.runCommand({collMod: "view2", viewOn: "view
 
 // Checks that dropping a nonexistent view or collection is not affected by an invalid view existing
 // in the view catalog.
-assert.commandFailedWithCode(viewsDB.runCommand({drop: "view4"}), ErrorCodes.NamespaceNotFound);
+assert.commandWorked(viewsDB.runCommand({drop: "view4"}));
 assert.commandFailedWithCode(viewsDB.runCommand({listCollections: 1}),
                              ErrorCodes.InvalidViewDefinition);
 
diff --git a/jstests/replsets/noop_writes_wait_for_write_concern.js b/jstests/replsets/noop_writes_wait_for_write_concern.js
index bdcc6df093c..1fc4bfdcb13 100644
--- a/jstests/replsets/noop_writes_wait_for_write_concern.js
+++ b/jstests/replsets/noop_writes_wait_for_write_concern.js
@@ -222,7 +222,7 @@ commands.push({
         assert.commandWorkedIgnoringWriteConcernErrors(db.runCommand({drop: collName}));
     },
     confirmFunc: function(res) {
-        assert.commandFailedWithCode(res, ErrorCodes.NamespaceNotFound);
+        assert.commandWorkedIgnoringWriteConcernErrors(res);
     }
 });
 
@@ -256,6 +256,18 @@ function testCommandWithWriteConcern(cmd) {
     cmd.req.writeConcern = {w: 3, wtimeout: 1000};
     jsTest.log("Testing " + tojson(cmd.req));
 
+    // Don't run drop cmd on older versions. Starting in v7.0 drop returns OK on non-existent
+    // collections, instead of a NamespaceNotFound error.
+    if (cmd.req["drop"] !== undefined) {
+        const primaryShell = new Mongo(primary.host);
+        const primaryBinVersion = primaryShell.getDB("admin").serverStatus()["version"];
+        if (primaryBinVersion != MongoRunner.getBinVersionFor("latest")) {
+            jsTest.log(
+                "Skipping test: drop on non-existent collections in older versions returns an error.");
+            return;
+        }
+    }
+
     dropTestCollection();
 
     cmd.setupFunc();
diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index c131f69dad0..75f4b02f4c2 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -734,11 +734,8 @@ Status CollectionCatalog::dropView(OperationContext* opCtx, const NamespaceStrin
     invariant(_viewsForDatabase.contains(viewName.dbName()));
     const ViewsForDatabase& viewsForDb = *_getViewsForDatabase(opCtx, viewName.dbName());
     assertViewCatalogValid(viewsForDb);
-
-    // Make sure the view exists before proceeding.
-    if (auto viewPtr = viewsForDb.lookup(viewName); !viewPtr) {
-        return {ErrorCodes::NamespaceNotFound,
-                str::stream() << "cannot drop missing view: " << viewName.ns()};
+    if (!viewsForDb.lookup(viewName)) {
+        return Status::OK();
     }
 
     Status result = Status::OK();
diff --git a/src/mongo/db/catalog/drop_collection.cpp b/src/mongo/db/catalog/drop_collection.cpp
index 4d4c792dcd7..c005bddb28a 100644
--- a/src/mongo/db/catalog/drop_collection.cpp
+++ b/src/mongo/db/catalog/drop_collection.cpp
@@ -59,24 +59,24 @@ namespace {
 MONGO_FAIL_POINT_DEFINE(hangDropCollectionBeforeLockAcquisition);
 MONGO_FAIL_POINT_DEFINE(hangDuringDropCollection);
 
-Status _checkNssAndReplState(OperationContext* opCtx,
-                             const CollectionPtr& coll,
-                             const NamespaceString& nss,
-                             const boost::optional<UUID>& expectedUUID = boost::none) {
+/**
+ * Checks that the collection has the 'expectedUUID' if given.
+ * Checks that writes are allowed to 'coll' -- e.g. whether this server is PRIMARY.
+ */
+Status _checkUUIDAndReplState(OperationContext* opCtx,
+                              const CollectionPtr& coll,
+                              const NamespaceString& nss,
+                              const boost::optional<UUID>& expectedUUID = boost::none) {
     try {
         checkCollectionUUIDMismatch(opCtx, nss, coll, expectedUUID);
     } catch (const DBException& ex) {
         return ex.toStatus();
     }
 
-    if (!coll) {
-        return Status(ErrorCodes::NamespaceNotFound, "ns not found");
-    }
-
     if (opCtx->writesAreReplicated() &&
-        !repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, coll->ns())) {
+        !repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, nss)) {
         return Status(ErrorCodes::NotWritablePrimary,
-                      str::stream() << "Not primary while dropping collection " << coll->ns());
+                      str::stream() << "Not primary while dropping collection " << nss);
     }
 
     return Status::OK();
@@ -125,11 +125,7 @@ Status _dropView(OperationContext* opCtx,
                  const NamespaceString& collectionName,
                  const boost::optional<UUID>& expectedUUID,
                  DropReply* reply) {
-    if (!db) {
-        Status status = Status(ErrorCodes::NamespaceNotFound, "ns not found");
-        audit::logDropView(opCtx->getClient(), collectionName, "", {}, status.code());
-        return status;
-    }
+    invariant(db);
 
     // Views don't have UUIDs so if the expectedUUID is specified, we will always throw.
     try {
@@ -141,9 +137,9 @@ Status _dropView(OperationContext* opCtx,
     auto view =
         CollectionCatalog::get(opCtx)->lookupViewWithoutValidatingDurable(opCtx, collectionName);
     if (!view) {
-        Status status = Status(ErrorCodes::NamespaceNotFound, "ns not found");
-        audit::logDropView(opCtx->getClient(), collectionName, "", {}, status.code());
-        return status;
+        audit::logDropView(
+            opCtx->getClient(), collectionName, "", {}, ErrorCodes::NamespaceNotFound);
+        return Status::OK();
     }
 
     // Validates the view or throws an "invalid view" error.
@@ -207,9 +203,13 @@ Status _abortIndexBuildsAndDrop(OperationContext* opCtx,
 
     CollectionPtr coll =
         CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, startingNss);
-    Status status = _checkNssAndReplState(opCtx, coll, startingNss, expectedUUID);
+
+    // Even if the collection doesn't exist, UUID mismatches must return an error.
+    Status status = _checkUUIDAndReplState(opCtx, coll, startingNss, expectedUUID);
     if (!status.isOK()) {
         return status;
+    } else if (!coll) {
+        return Status::OK();
     }
 
     warnEncryptedCollectionsIfNeeded(opCtx, coll);
@@ -262,9 +262,13 @@ Status _abortIndexBuildsAndDrop(OperationContext* opCtx,
         opCtx->recoveryUnit()->abandonSnapshot();
 
         coll = CollectionCatalog::get(opCtx)->lookupCollectionByUUID(opCtx, collectionUUID);
-        status = _checkNssAndReplState(opCtx, coll, startingNss, expectedUUID);
+
+        // Even if the collection doesn't exist, UUID mismatches must return an error.
+        status = _checkUUIDAndReplState(opCtx, coll, startingNss, expectedUUID);
         if (!status.isOK()) {
             return status;
+        } else if (!coll) {
+            return Status::OK();
         }
 
         // Check if any new index builds were started while releasing the collection lock
@@ -311,9 +315,13 @@ Status _dropCollectionForApplyOps(OperationContext* opCtx,
     Lock::CollectionLock collLock(opCtx, collectionName, MODE_X);
     const CollectionPtr& coll =
         CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, collectionName);
-    Status status = _checkNssAndReplState(opCtx, coll, collectionName);
+
+    // Even if the collection doesn't exist, UUID mismatches must return an error.
+    Status status = _checkUUIDAndReplState(opCtx, coll, collectionName);
     if (!status.isOK()) {
         return status;
+    } else if (!coll) {
+        return Status::OK();
     }
 
     if (MONGO_unlikely(hangDuringDropCollection.shouldFail())) {
@@ -369,7 +377,7 @@ Status _dropCollection(OperationContext* opCtx,
                                                         collectionName.coll().toString(),
                                                         boost::none),
                              "Database does not exist"}
-                    : Status(ErrorCodes::NamespaceNotFound, "ns not found");
+                    : Status::OK();
             }
 
             if (CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, collectionName)) {
@@ -457,15 +465,18 @@ Status _dropCollection(OperationContext* opCtx,
                     return dropTimeseries(bucketsNs, false);
                 }
 
+                // There is no collection or view at the namespace. Check whether a UUID was given
+                // and error if so because the caller expects the collection to exist. If no UUID
+                // was given, then it is OK to return success.
                 try {
                     checkCollectionUUIDMismatch(opCtx, collectionName, nullptr, expectedUUID);
                 } catch (const DBException& ex) {
                     return ex.toStatus();
                 }
 
-                Status status = Status(ErrorCodes::NamespaceNotFound, "ns not found");
-                audit::logDropView(opCtx->getClient(), collectionName, "", {}, status.code());
-                return status;
+                audit::logDropView(
+                    opCtx->getClient(), collectionName, "", {}, ErrorCodes::NamespaceNotFound);
+                return Status::OK();
             }
             if (view->timeseries() &&
                 CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, view->viewOn())) {
@@ -478,9 +489,15 @@ Status _dropCollection(OperationContext* opCtx,
             return _dropView(opCtx, db, collectionName, expectedUUID, reply);
         });
     } catch (ExceptionFor<ErrorCodes::NamespaceNotFound>&) {
-        // The shell requires that NamespaceNotFound error codes return the "ns not found"
-        // string.
-        return Status(ErrorCodes::NamespaceNotFound, "ns not found");
+        // Any unhandled namespace not found errors should be converted into success. Unless the
+        // caller specified a UUID and expects the collection to exist.
+        try {
+            checkCollectionUUIDMismatch(opCtx, collectionName, nullptr, expectedUUID);
+        } catch (const DBException& ex) {
+            return ex.toStatus();
+        }
+
+        return Status::OK();
     }
 }
 }  // namespace
@@ -559,7 +576,7 @@ Status dropCollectionForApplyOps(OperationContext* opCtx,
         AutoGetDb autoDb(opCtx, collectionName.dbName(), MODE_IX);
         Database* db = autoDb.getDb();
         if (!db) {
-            return Status(ErrorCodes::NamespaceNotFound, "ns not found");
+            return Status::OK();
         }
 
         const CollectionPtr& coll =
diff --git a/src/mongo/db/catalog/rename_collection_test.cpp b/src/mongo/db/catalog/rename_collection_test.cpp
index 8605bb21e37..40de0350973 100644
--- a/src/mongo/db/catalog/rename_collection_test.cpp
+++ b/src/mongo/db/catalog/rename_collection_test.cpp
@@ -804,8 +804,7 @@ TEST_F(RenameCollectionTest, RenameCollectionForApplyOpsSourceAndTargetDoNotExis
     auto uuid = UUID::gen();
     auto cmd = BSON("renameCollection" << _sourceNss.ns() << "to" << _targetNss.ns() << "dropTarget"
                                        << "true");
-    ASSERT_EQUALS(ErrorCodes::NamespaceNotFound,
-                  renameCollectionForApplyOps(_opCtx.get(), uuid, boost::none, cmd, {}));
+    ASSERT_OK(renameCollectionForApplyOps(_opCtx.get(), uuid, boost::none, cmd, {}));
     ASSERT_FALSE(_collectionExists(_opCtx.get(), _sourceNss));
     ASSERT_FALSE(_collectionExists(_opCtx.get(), _targetNss));
 }
diff --git a/src/mongo/db/views/view_catalog_test.cpp b/src/mongo/db/views/view_catalog_test.cpp
index 7533b434c1d..1b1232dba64 100644
--- a/src/mongo/db/views/view_catalog_test.cpp
+++ b/src/mongo/db/views/view_catalog_test.cpp
@@ -469,7 +469,7 @@ TEST_F(ViewCatalogFixture, CannotCreateViewIfItsFullyResolvedPipelineWouldExceed
 
 TEST_F(ViewCatalogFixture, DropMissingView) {
     NamespaceString viewName = NamespaceString::createNamespaceString_forTest("db.view");
-    ASSERT_NOT_OK(dropView(operationContext(), viewName));
+    ASSERT_OK(dropView(operationContext(), viewName));
 }
 
 TEST_F(ViewCatalogFixture, ModifyMissingView) {