SERVER-40915 View namespaces now included in the LockManager's ResourceId::toString()

Author: Dsolnik

src/mongo/db/views/view_catalog.cpp

@@ -39,6 +39,7 @@
 #include "mongo/base/status_with.h"
 #include "mongo/base/string_data.h"
 #include "mongo/bson/util/builder.h"
+#include "mongo/db/catalog/collection_catalog.h"
 #include "mongo/db/catalog/database.h"
 #include "mongo/db/commands/feature_compatibility_version_command_parser.h"
 #include "mongo/db/namespace_string.h"
@@ -195,9 +196,17 @@ Status ViewCatalog::_createOrUpdateView(WithLock lk,
 
     _durable->upsert(opCtx, viewName, viewDefBuilder.obj());
     _viewMap[viewName.ns()] = view;
-    opCtx->recoveryUnit()->onRollback([this, viewName]() {
+
+    // Register the view in the CollectionCatalog mapping from ResourceID->namespace
+    CollectionCatalog& catalog = CollectionCatalog::get(opCtx);
+    auto viewRid = ResourceId(RESOURCE_COLLECTION, viewName.ns());
+    catalog.addResource(viewRid, viewName.ns());
+
+    opCtx->recoveryUnit()->onRollback([this, viewName, opCtx, viewRid]() {
         this->_viewMap.erase(viewName.ns());
         this->_viewGraphNeedsRefresh = true;
+        CollectionCatalog& catalog = CollectionCatalog::get(opCtx);
+        catalog.removeResource(viewRid, viewName.ns());
     });
 
     // We may get invalidated, but we're exclusively locked, so the change must be ours.
@@ -411,8 +420,12 @@ Status ViewCatalog::modifyView(OperationContext* opCtx,
                       str::stream() << "invalid name for 'viewOn': " << viewOn.coll());
 
     ViewDefinition savedDefinition = *viewPtr;
-    opCtx->recoveryUnit()->onRollback([this, viewName, savedDefinition]() {
+
+    opCtx->recoveryUnit()->onRollback([this, viewName, savedDefinition, opCtx]() {
         this->_viewMap[viewName.ns()] = std::make_shared<ViewDefinition>(savedDefinition);
+        auto viewRid = ResourceId(RESOURCE_COLLECTION, viewName.ns());
+        CollectionCatalog& catalog = CollectionCatalog::get(opCtx);
+        catalog.addResource(viewRid, viewName.ns());
     });
 
     return _createOrUpdateView(lk,
@@ -446,9 +459,16 @@ Status ViewCatalog::dropView(OperationContext* opCtx, const NamespaceString& vie
     _durable->remove(opCtx, viewName);
     _viewGraph.remove(savedDefinition.name());
     _viewMap.erase(viewName.ns());
-    opCtx->recoveryUnit()->onRollback([this, viewName, savedDefinition]() {
+
+    CollectionCatalog& catalog = CollectionCatalog::get(opCtx);
+    auto viewRid = ResourceId(RESOURCE_COLLECTION, viewName.ns());
+    catalog.removeResource(viewRid, viewName.ns());
+
+    opCtx->recoveryUnit()->onRollback([this, viewName, savedDefinition, opCtx, viewRid]() {
         this->_viewGraphNeedsRefresh = true;
         this->_viewMap[viewName.ns()] = std::make_shared<ViewDefinition>(savedDefinition);
+        CollectionCatalog& catalog = CollectionCatalog::get(opCtx);
+        catalog.addResource(viewRid, viewName.ns());
     });
 
     // We may get invalidated, but we're exclusively locked, so the change must be ours.

src/mongo/db/views/view_catalog_test.cpp

@@ -39,6 +39,8 @@
 #include "mongo/bson/bsonmisc.h"
 #include "mongo/bson/bsonobj.h"
 #include "mongo/bson/bsonobjbuilder.h"
+#include "mongo/db/catalog/collection_catalog.h"
+#include "mongo/db/concurrency/lock_manager_defs.h"
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/db/query/collation/collator_factory_interface.h"
@@ -451,6 +453,96 @@ TEST_F(ViewCatalogFixture, LookupExistingView) {
     ASSERT(viewCatalog.lookup(opCtx.get(), "db.view"_sd));
 }
 
+TEST_F(ViewCatalogFixture, LookupRIDExistingView) {
+    const NamespaceString viewName("db.view");
+    const NamespaceString viewOn("db.coll");
+
+    ASSERT_OK(viewCatalog.createView(opCtx.get(), viewName, viewOn, emptyPipeline, emptyCollation));
+
+    auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd);
+    auto& collectionCatalog = CollectionCatalog::get(opCtx.get());
+    ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == "db.view");
+}
+
+TEST_F(ViewCatalogFixture, LookupRIDExistingViewRollback) {
+    const NamespaceString viewName("db.view");
+    const NamespaceString viewOn("db.coll");
+    {
+        WriteUnitOfWork wunit(opCtx.get());
+        ASSERT_OK(
+            viewCatalog.createView(opCtx.get(), viewName, viewOn, emptyPipeline, emptyCollation));
+    }
+    auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd);
+    auto& collectionCatalog = CollectionCatalog::get(opCtx.get());
+    ASSERT(!collectionCatalog.lookupResourceName(resourceID));
+}
+
+TEST_F(ViewCatalogFixture, LookupRIDAfterDrop) {
+    const NamespaceString viewName("db.view");
+    const NamespaceString viewOn("db.coll");
+
+    ASSERT_OK(viewCatalog.createView(opCtx.get(), viewName, viewOn, emptyPipeline, emptyCollation));
+    ASSERT_OK(viewCatalog.dropView(opCtx.get(), viewName));
+
+    auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd);
+    auto& collectionCatalog = CollectionCatalog::get(opCtx.get());
+    ASSERT(!collectionCatalog.lookupResourceName(resourceID));
+}
+
+TEST_F(ViewCatalogFixture, LookupRIDAfterDropRollback) {
+    const NamespaceString viewName("db.view");
+    const NamespaceString viewOn("db.coll");
+
+    auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd);
+    auto& collectionCatalog = CollectionCatalog::get(opCtx.get());
+    {
+        WriteUnitOfWork wunit(opCtx.get());
+        ASSERT_OK(
+            viewCatalog.createView(opCtx.get(), viewName, viewOn, emptyPipeline, emptyCollation));
+        ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == viewName.ns());
+        wunit.commit();
+    }
+
+    {
+        WriteUnitOfWork wunit(opCtx.get());
+        ASSERT_OK(viewCatalog.dropView(opCtx.get(), viewName));
+    }
+
+    ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == viewName.ns());
+}
+
+TEST_F(ViewCatalogFixture, LookupRIDAfterModify) {
+    const NamespaceString viewName("db.view");
+    const NamespaceString viewOn("db.coll");
+
+    auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd);
+    auto& collectionCatalog = CollectionCatalog::get(opCtx.get());
+    ASSERT_OK(viewCatalog.createView(opCtx.get(), viewName, viewOn, emptyPipeline, emptyCollation));
+    ASSERT_OK(viewCatalog.modifyView(opCtx.get(), viewName, viewOn, emptyPipeline));
+    ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == viewName.ns());
+}
+
+TEST_F(ViewCatalogFixture, LookupRIDAfterModifyRollback) {
+    const NamespaceString viewName("db.view");
+    const NamespaceString viewOn("db.coll");
+
+    auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd);
+    auto& collectionCatalog = CollectionCatalog::get(opCtx.get());
+    {
+        WriteUnitOfWork wunit(opCtx.get());
+        ASSERT_OK(
+            viewCatalog.createView(opCtx.get(), viewName, viewOn, emptyPipeline, emptyCollation));
+        ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == viewName.ns());
+        wunit.commit();
+    }
+    {
+        WriteUnitOfWork wunit(opCtx.get());
+        ASSERT_OK(viewCatalog.modifyView(opCtx.get(), viewName, viewOn, emptyPipeline));
+        ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == viewName.ns());
+    }
+    ASSERT(collectionCatalog.lookupResourceName(resourceID).get() == viewName.ns());
+}
+
 TEST_F(ViewCatalogFixture, CreateViewThenDropAndLookup) {
     const NamespaceString viewName("db.view");
     const NamespaceString viewOn("db.coll");