SERVER-70043 Thread-through CollectionPtr into the onDelete OpObserver

Co-authored-by: Daniel Gómez Ferro <daniel.gomezferro@mongodb.com>

4 files changed, 34 insertions, 22 deletions

diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript
index 2b1b866efae..09f7d8e3cc9 100644
--- a/src/mongo/db/auth/SConscript
+++ b/src/mongo/db/auth/SConscript
@@ -551,6 +551,7 @@ env.CppUnitTest(
         '$BUILD_DIR/mongo/base',
         '$BUILD_DIR/mongo/client/sasl_client',
         '$BUILD_DIR/mongo/db/common',
+        '$BUILD_DIR/mongo/db/concurrency/exception_util',
         '$BUILD_DIR/mongo/db/pipeline/pipeline',
         '$BUILD_DIR/mongo/db/repl/oplog',
         '$BUILD_DIR/mongo/db/repl/oplog_interface_local',
diff --git a/src/mongo/db/auth/auth_op_observer.cpp b/src/mongo/db/auth/auth_op_observer.cpp
index a2ddfa79949..72a86220328 100644
--- a/src/mongo/db/auth/auth_op_observer.cpp
+++ b/src/mongo/db/auth/auth_op_observer.cpp
@@ -74,10 +74,9 @@ void AuthOpObserver::onUpdate(OperationContext* opCtx, const OplogUpdateEntryArg
 }
 
 void AuthOpObserver::aboutToDelete(OperationContext* opCtx,
-                                   NamespaceString const& nss,
-                                   const UUID& uuid,
+                                   const CollectionPtr& coll,
                                    BSONObj const& doc) {
-    audit::logRemoveOperation(opCtx->getClient(), nss, doc);
+    audit::logRemoveOperation(opCtx->getClient(), coll->ns(), doc);
 
     // Extract the _id field from the document. If it does not have an _id, use the
     // document itself as the _id.
@@ -85,14 +84,13 @@ void AuthOpObserver::aboutToDelete(OperationContext* opCtx,
 }
 
 void AuthOpObserver::onDelete(OperationContext* opCtx,
-                              const NamespaceString& nss,
-                              const UUID& uuid,
+                              const CollectionPtr& coll,
                               StmtId stmtId,
                               const OplogDeleteEntryArgs& args) {
     auto& documentId = documentIdDecoration(opCtx);
     invariant(!documentId.isEmpty());
     AuthorizationManager::get(opCtx->getServiceContext())
-        ->logOp(opCtx, "d", nss, documentId, nullptr);
+        ->logOp(opCtx, "d", coll->ns(), documentId, nullptr);
 }
 
 void AuthOpObserver::onCreateCollection(OperationContext* opCtx,
diff --git a/src/mongo/db/auth/auth_op_observer.h b/src/mongo/db/auth/auth_op_observer.h
index 51d0e7c5af8..83dc89521d2 100644
--- a/src/mongo/db/auth/auth_op_observer.h
+++ b/src/mongo/db/auth/auth_op_observer.h
@@ -112,13 +112,11 @@ public:
     void onUpdate(OperationContext* opCtx, const OplogUpdateEntryArgs& args) final;
 
     void aboutToDelete(OperationContext* opCtx,
-                       const NamespaceString& nss,
-                       const UUID& uuid,
+                       const CollectionPtr& coll,
                        const BSONObj& doc) final;
 
     void onDelete(OperationContext* opCtx,
-                  const NamespaceString& nss,
-                  const UUID& uuid,
+                  const CollectionPtr& coll,
                   StmtId stmtId,
                   const OplogDeleteEntryArgs& args) final;
 
diff --git a/src/mongo/db/auth/auth_op_observer_test.cpp b/src/mongo/db/auth/auth_op_observer_test.cpp
index 831d9b6fb05..56d9e80c82e 100644
--- a/src/mongo/db/auth/auth_op_observer_test.cpp
+++ b/src/mongo/db/auth/auth_op_observer_test.cpp
@@ -32,6 +32,7 @@
 #include "mongo/db/auth/auth_op_observer.h"
 #include "mongo/db/auth/authorization_manager.h"
 #include "mongo/db/client.h"
+#include "mongo/db/concurrency/exception_util.h"
 #include "mongo/db/concurrency/locker_noop.h"
 #include "mongo/db/db_raii.h"
 #include "mongo/db/dbdirectclient.h"
@@ -74,8 +75,23 @@ public:
         // Ensure that we are primary.
         auto replCoord = repl::ReplicationCoordinator::get(opCtx.get());
         ASSERT_OK(replCoord->setFollowerMode(repl::MemberState::RS_PRIMARY));
+
+        // Create test collection
+        writeConflictRetry(opCtx.get(), "createColl", _nss.ns(), [&] {
+            opCtx->recoveryUnit()->setTimestampReadSource(RecoveryUnit::ReadSource::kNoTimestamp);
+            opCtx->recoveryUnit()->abandonSnapshot();
+
+            WriteUnitOfWork wunit(opCtx.get());
+            AutoGetCollection collRaii(opCtx.get(), _nss, MODE_X);
+
+            auto db = collRaii.ensureDbExists(opCtx.get());
+            invariant(db->createCollection(opCtx.get(), _nss, {}));
+            wunit.commit();
+        });
     }
 
+    NamespaceString _nss = {"test", "coll"};
+
 private:
     // Creates a reasonable set of ReplSettings for most tests.  We need to be able to
     // override this to create a larger oplog.
@@ -127,16 +143,15 @@ TEST_F(AuthOpObserverTest, OnRollbackDoesntInvalidateAuthCacheWhenNoAuthNamespac
 }
 
 TEST_F(AuthOpObserverTest, MultipleAboutToDeleteAndOnDelete) {
-    auto uuid = UUID::gen();
     AuthOpObserver opObserver;
     auto opCtx = cc().makeOperationContext();
     NamespaceString nss = {"test", "coll"};
-    AutoGetDb autoDb(opCtx.get(), nss.dbName(), MODE_X);
     WriteUnitOfWork wunit(opCtx.get());
-    opObserver.aboutToDelete(opCtx.get(), nss, uuid, BSON("_id" << 1));
-    opObserver.onDelete(opCtx.get(), nss, uuid, {}, {});
-    opObserver.aboutToDelete(opCtx.get(), nss, uuid, BSON("_id" << 1));
-    opObserver.onDelete(opCtx.get(), nss, uuid, {}, {});
+    AutoGetCollection autoColl(opCtx.get(), nss, MODE_IX);
+    opObserver.aboutToDelete(opCtx.get(), *autoColl, BSON("_id" << 1));
+    opObserver.onDelete(opCtx.get(), *autoColl, {}, {});
+    opObserver.aboutToDelete(opCtx.get(), *autoColl, BSON("_id" << 1));
+    opObserver.onDelete(opCtx.get(), *autoColl, {}, {});
 }
 
 DEATH_TEST_F(AuthOpObserverTest, AboutToDeleteMustPreceedOnDelete, "invariant") {
@@ -144,18 +159,18 @@ DEATH_TEST_F(AuthOpObserverTest, AboutToDeleteMustPreceedOnDelete, "invariant")
     auto opCtx = cc().makeOperationContext();
     cc().swapLockState(std::make_unique<LockerNoop>());
     NamespaceString nss = {"test", "coll"};
-    opObserver.onDelete(opCtx.get(), nss, UUID::gen(), {}, {});
+    AutoGetCollection autoColl(opCtx.get(), nss, MODE_IX);
+    opObserver.onDelete(opCtx.get(), *autoColl, {}, {});
 }
 
 DEATH_TEST_F(AuthOpObserverTest, EachOnDeleteRequiresAboutToDelete, "invariant") {
-    auto uuid = UUID::gen();
     AuthOpObserver opObserver;
     auto opCtx = cc().makeOperationContext();
     cc().swapLockState(std::make_unique<LockerNoop>());
-    NamespaceString nss = {"test", "coll"};
-    opObserver.aboutToDelete(opCtx.get(), nss, uuid, {});
-    opObserver.onDelete(opCtx.get(), nss, uuid, {}, {});
-    opObserver.onDelete(opCtx.get(), nss, uuid, {}, {});
+    AutoGetCollection autoColl(opCtx.get(), _nss, MODE_IX);
+    opObserver.aboutToDelete(opCtx.get(), *autoColl, {});
+    opObserver.onDelete(opCtx.get(), *autoColl, {}, {});
+    opObserver.onDelete(opCtx.get(), *autoColl, {}, {});
 }
 
 }  // namespace