SERVER-29343 make lookupCollectionByUUID and lookupNSSByUUID const

4 files changed, 30 insertions, 13 deletions

diff --git a/src/mongo/util/SConscript b/src/mongo/util/SConscript
index ee8165d377b..3044c4e0959 100644
--- a/src/mongo/util/SConscript
+++ b/src/mongo/util/SConscript
@@ -119,6 +119,7 @@ env.Library(
     LIBDEPS=[
         '$BUILD_DIR/mongo/base',
         '$BUILD_DIR/mongo/db/catalog/collection',
+        '$BUILD_DIR/mongo/db/namespace_string',
         '$BUILD_DIR/mongo/util/decorable',
         '$BUILD_DIR/third_party/murmurhash3/murmurhash3',
     ],
diff --git a/src/mongo/util/uuid_catalog.cpp b/src/mongo/util/uuid_catalog.cpp
index 6ec799414c1..a73b1288b7a 100644
--- a/src/mongo/util/uuid_catalog.cpp
+++ b/src/mongo/util/uuid_catalog.cpp
@@ -25,10 +25,14 @@
  *    exception statement from all source files in the program, then also delete
  *    it in the license file.
  */
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kStorage
+
+#include "mongo/platform/basic.h"
 
 #include "uuid_catalog.h"
 
 #include "mongo/db/storage/recovery_unit.h"
+#include "mongo/util/log.h"
 #include "mongo/util/uuid.h"
 
 namespace mongo {
@@ -43,17 +47,16 @@ void UUIDCatalog::onCreateCollection(OperationContext* opCtx,
     opCtx->recoveryUnit()->onRollback([this, uuid] { _removeUUIDCatalogEntry(uuid); });
 }
 
-Collection* UUIDCatalog::lookupCollectionByUUID(CollectionUUID uuid) {
+Collection* UUIDCatalog::lookupCollectionByUUID(CollectionUUID uuid) const {
     stdx::lock_guard<stdx::mutex> lock(_catalogLock);
-    Collection* foundCol = _catalog[uuid];
-    return foundCol;
+    auto foundIt = _catalog.find(uuid);
+    return foundIt == _catalog.end() ? nullptr : foundIt->second;
 }
 
-NamespaceString UUIDCatalog::lookupNSSByUUID(CollectionUUID uuid) {
+NamespaceString UUIDCatalog::lookupNSSByUUID(CollectionUUID uuid) const {
     stdx::lock_guard<stdx::mutex> lock(_catalogLock);
-    Collection* foundCol = _catalog[uuid];
-    NamespaceString nss = foundCol ? foundCol->ns() : NamespaceString();
-    return nss;
+    auto foundIt = _catalog.find(uuid);
+    return foundIt == _catalog.end() ? NamespaceString() : foundIt->second->ns();
 }
 
 void UUIDCatalog::onDropCollection(OperationContext* opCtx, CollectionUUID uuid) {
@@ -66,7 +69,8 @@ void UUIDCatalog::_registerUUIDCatalogEntry(CollectionUUID uuid, Collection* col
     stdx::lock_guard<stdx::mutex> lock(_catalogLock);
     if (coll) {
         std::pair<CollectionUUID, Collection*> entry = std::make_pair(uuid, coll);
-        invariant(_catalog.insert(entry).second == true);
+        LOG(2) << "registering collection " << coll->ns() << " as having UUID " << uuid.toString();
+        invariant(_catalog.insert(entry).second);
     }
 }
 
diff --git a/src/mongo/util/uuid_catalog.h b/src/mongo/util/uuid_catalog.h
index f79b2ded90f..6f532745ba7 100644
--- a/src/mongo/util/uuid_catalog.h
+++ b/src/mongo/util/uuid_catalog.h
@@ -61,13 +61,13 @@ public:
      * to calling this function, or else the found Collection pointer
      * might no longer be valid when the call returns.
      */
-    Collection* lookupCollectionByUUID(CollectionUUID uuid);
+    Collection* lookupCollectionByUUID(CollectionUUID uuid) const;
 
     /* This function gets the NamespaceString from the Collection* pointer that
      * corresponds to CollectionUUID uuid. If there is no such pointer, an empty
      * NamespaceString is returned.
      */
-    NamespaceString lookupNSSByUUID(CollectionUUID uuid);
+    NamespaceString lookupNSSByUUID(CollectionUUID uuid) const;
 
     /* This function removes the entry for uuid from the UUID catalog. It
      * is called by the op observer when a collection is dropped.
@@ -75,7 +75,7 @@ public:
     void onDropCollection(OperationContext* opCtx, CollectionUUID uuid);
 
 private:
-    mongo::stdx::mutex _catalogLock;
+    mutable mongo::stdx::mutex _catalogLock;
     mongo::stdx::unordered_map<CollectionUUID, Collection*, CollectionUUID::Hash> _catalog;
 
     void _registerUUIDCatalogEntry(CollectionUUID uuid, Collection* coll);
diff --git a/src/mongo/util/uuid_catalog_test.cpp b/src/mongo/util/uuid_catalog_test.cpp
index eb1afba9b99..65720343f45 100644
--- a/src/mongo/util/uuid_catalog_test.cpp
+++ b/src/mongo/util/uuid_catalog_test.cpp
@@ -58,11 +58,10 @@ protected:
 namespace {
 
 TEST_F(UUIDCatalogTest, onCreateCollection) {
-    ASSERT(catalog.lookupCollectionByUUID(uuid) != nullptr);
+    ASSERT(catalog.lookupCollectionByUUID(uuid) == &col);
 }
 
 TEST_F(UUIDCatalogTest, lookupCollectionByUUID) {
-    ASSERT(catalog.lookupCollectionByUUID(uuid) != nullptr);
     // Ensure the string value of the NamespaceString of the obtained Collection is equal to
     // nss.ns().
     ASSERT_EQUALS(catalog.lookupCollectionByUUID(uuid)->ns().ns(), nss.ns());
@@ -77,6 +76,19 @@ TEST_F(UUIDCatalogTest, lookupNSSByUUID) {
     ASSERT_EQUALS(catalog.lookupNSSByUUID(CollectionUUID::gen()).ns(), NamespaceString().ns());
 }
 
+TEST_F(UUIDCatalogTest, insertAfterLookup) {
+    auto newUUID = CollectionUUID::gen();
+    NamespaceString newNss(nss.db(), "newcol");
+    Collection newCol(stdx::make_unique<CollectionMock>(newNss));
+
+    // Ensure that looking up non-existing UUIDs doesn't affect later registration of those UUIDs.
+    ASSERT(catalog.lookupCollectionByUUID(newUUID) == nullptr);
+    ASSERT(catalog.lookupNSSByUUID(newUUID) == NamespaceString());
+    catalog.onCreateCollection(&opCtx, &newCol, newUUID);
+    ASSERT_EQUALS(catalog.lookupCollectionByUUID(newUUID), &newCol);
+    ASSERT_EQUALS(catalog.lookupNSSByUUID(uuid), nss);
+}
+
 TEST_F(UUIDCatalogTest, onDropCollection) {
     catalog.onDropCollection(&opCtx, uuid);
     // Ensure the lookup returns a null pointer upon removing the uuid entry.