SERVER-66355 Fix DurableViewCatalog bug where nss was passed into dbName parameter; Don't require view namespace lock during oplog applicaion of db.system.views inserts

(cherry picked from commit 1a578ef55d316eb5fec5cc135cf056f6f17ab616)

3 files changed, 37 insertions, 29 deletions

diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index 5888c6211b9..30587d57e76 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -475,8 +475,11 @@ Status CollectionCatalog::createView(OperationContext* opCtx,
                                      const BSONArray& pipeline,
                                      const BSONObj& collation,
                                      const ViewsForDatabase::PipelineValidatorFn& pipelineValidator,
-                                     const bool updateDurableViewCatalog) const {
-    invariant(opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_IX));
+                                     const ViewUpsertMode insertViewMode) const {
+    // A view document direct write can occur via the oplog application path, which may only hold a
+    // lock on the collection being updated (the database views collection).
+    invariant(insertViewMode == ViewUpsertMode::kAlreadyDurableView ||
+              opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_IX));
     invariant(opCtx->lockState()->isCollectionLockedForMode(
         NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName), MODE_X));
 
@@ -514,7 +517,7 @@ Status CollectionCatalog::createView(OperationContext* opCtx,
                                      pipelineValidator,
                                      std::move(collator.getValue()),
                                      ViewsForDatabase{viewsForDb},
-                                     ViewUpsertMode::kCreateView);
+                                     insertViewMode);
     }
 
     return result;
@@ -1412,8 +1415,11 @@ Status CollectionCatalog::_createOrUpdateView(
     const ViewsForDatabase::PipelineValidatorFn& pipelineValidator,
     std::unique_ptr<CollatorInterface> collator,
     ViewsForDatabase&& viewsForDb,
-    ViewUpsertMode mode) const {
-    invariant(opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_IX));
+    ViewUpsertMode insertViewMode) const {
+    // A view document direct write can occur via the oplog application path, which may only hold a
+    // lock on the collection being updated (the database views collection).
+    invariant(insertViewMode == ViewUpsertMode::kAlreadyDurableView ||
+              opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_IX));
     invariant(opCtx->lockState()->isCollectionLockedForMode(
         NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName), MODE_X));
 
@@ -1437,20 +1443,20 @@ Status CollectionCatalog::_createOrUpdateView(
     // If the view is already in the durable view catalog, we don't need to validate the graph. If
     // we need to update the durable view catalog, we need to check that the resulting dependency
     // graph is acyclic and within the maximum depth.
-    const bool viewGraphNeedsValidation = mode != ViewUpsertMode::kAlreadyDurableView;
+    const bool viewGraphNeedsValidation = insertViewMode != ViewUpsertMode::kAlreadyDurableView;
     Status graphStatus =
         viewsForDb.upsertIntoGraph(opCtx, view, pipelineValidator, viewGraphNeedsValidation);
     if (!graphStatus.isOK()) {
         return graphStatus;
     }
 
-    if (mode != ViewUpsertMode::kAlreadyDurableView) {
+    if (insertViewMode != ViewUpsertMode::kAlreadyDurableView) {
         viewsForDb.durable->upsert(opCtx, viewName, viewDef);
     }
 
     viewsForDb.valid = false;
     auto res = [&] {
-        switch (mode) {
+        switch (insertViewMode) {
             case ViewUpsertMode::kCreateView:
             case ViewUpsertMode::kAlreadyDurableView:
                 return viewsForDb.insert(opCtx, viewDef);
diff --git a/src/mongo/db/catalog/collection_catalog.h b/src/mongo/db/catalog/collection_catalog.h
index a4b2891a1db..9d2d3c06354 100644
--- a/src/mongo/db/catalog/collection_catalog.h
+++ b/src/mongo/db/catalog/collection_catalog.h
@@ -115,6 +115,19 @@ public:
         }
     };
 
+    enum class ViewUpsertMode {
+        // Insert all data for that view into the view map, view graph, and durable view catalog.
+        kCreateView,
+
+        // Insert into the view map and view graph without reinserting the view into the durable
+        // view catalog. Skip view graph validation.
+        kAlreadyDurableView,
+
+        // Reload the view map, insert into the view graph (flagging it as needing refresh), and
+        // update the durable view catalog.
+        kUpdateView,
+    };
+
     static std::shared_ptr<const CollectionCatalog> get(ServiceContext* svcCtx);
     static std::shared_ptr<const CollectionCatalog> get(OperationContext* opCtx);
 
@@ -147,7 +160,8 @@ public:
      *
      * Must be in WriteUnitOfWork. View creation rolls back if the unit of work aborts.
      *
-     * Caller must ensure corresponding database exists.
+     * Caller must ensure corresponding database exists. Expects db.system.views MODE_X lock and
+     * view namespace MODE_IX lock (unless 'insertViewMode' is set to kAlreadyDurableView).
      */
     Status createView(OperationContext* opCtx,
                       const NamespaceString& viewName,
@@ -155,7 +169,7 @@ public:
                       const BSONArray& pipeline,
                       const BSONObj& collation,
                       const ViewsForDatabase::PipelineValidatorFn& pipelineValidator,
-                      bool updateDurableViewCatalog = true) const;
+                      ViewUpsertMode insertViewMode = ViewUpsertMode::kCreateView) const;
 
     /**
      * Drop the view named 'viewName'.
@@ -540,19 +554,6 @@ private:
      */
     void _replaceViewsForDatabase(StringData dbName, ViewsForDatabase&& views);
 
-    enum class ViewUpsertMode {
-        // Insert all data for that view into the view map, view graph, and durable view catalog.
-        kCreateView,
-
-        // Insert into the view map and view graph without reinserting the view into the durable
-        // view catalog. Skip view graph validation.
-        kAlreadyDurableView,
-
-        // Reload the view map, insert into the view graph (flagging it as needing refresh), and
-        // update the durable view catalog.
-        kUpdateView,
-    };
-
     /**
      * Helper to take care of shared functionality for 'createView(...)' and 'modifyView(...)'.
      */
@@ -563,7 +564,7 @@ private:
                                const ViewsForDatabase::PipelineValidatorFn& pipelineValidator,
                                std::unique_ptr<CollatorInterface> collator,
                                ViewsForDatabase&& viewsForDb,
-                               ViewUpsertMode mode) const;
+                               ViewUpsertMode insertViewMode) const;
 
     /**
      * Returns true if this CollectionCatalog instance is part of an ongoing batched catalog write.
diff --git a/src/mongo/db/views/durable_view_catalog.cpp b/src/mongo/db/views/durable_view_catalog.cpp
index 9faf0362a31..e5281a8c63c 100644
--- a/src/mongo/db/views/durable_view_catalog.cpp
+++ b/src/mongo/db/views/durable_view_catalog.cpp
@@ -59,6 +59,10 @@ namespace {
 void validateViewDefinitionBSON(OperationContext* opCtx,
                                 const BSONObj& viewDefinition,
                                 StringData dbName) {
+    // Internal callers should always pass in a valid 'dbName' against which to compare the
+    // 'viewDefinition'.
+    invariant(NamespaceString::validDBName(dbName));
+
     bool valid = true;
 
     for (const BSONElement& e : viewDefinition) {
@@ -120,7 +124,7 @@ Status DurableViewCatalog::onExternalInsert(OperationContext* opCtx,
                                             const BSONObj& doc,
                                             const NamespaceString& name) {
     try {
-        validateViewDefinitionBSON(opCtx, doc, name.toString());
+        validateViewDefinitionBSON(opCtx, doc, name.db());
     } catch (const DBException& e) {
         return e.toStatus();
     }
@@ -130,9 +134,6 @@ Status DurableViewCatalog::onExternalInsert(OperationContext* opCtx,
     NamespaceString viewOn(name.db(), doc.getStringField("viewOn"));
     BSONArray pipeline(doc.getObjectField("pipeline"));
     BSONObj collation(doc.getObjectField("collation"));
-    // Set updateDurableViewCatalog to false because the view has already been inserted into the
-    // durable view catalog.
-    const bool updateDurableViewCatalog = false;
 
     return catalog->createView(opCtx,
                                viewName,
@@ -140,7 +141,7 @@ Status DurableViewCatalog::onExternalInsert(OperationContext* opCtx,
                                pipeline,
                                collation,
                                view_catalog_helpers::validatePipeline,
-                               updateDurableViewCatalog);
+                               CollectionCatalog::ViewUpsertMode::kAlreadyDurableView);
 }
 
 void DurableViewCatalog::onSystemViewsCollectionDrop(OperationContext* opCtx,