SERVER-23493 Cleanup CollectionMetadata::clonePlusChunk

The clonePlusChunk method is only used for testing purposes, so there is
no need for it to uassert. Add invariands and remove an unnecessary out
parameter.

1 files changed, 16 insertions, 40 deletions

diff --git a/src/mongo/db/s/collection_metadata.h b/src/mongo/db/s/collection_metadata.h
index d11b0ca2fff..38898e15e35 100644
--- a/src/mongo/db/s/collection_metadata.h
+++ b/src/mongo/db/s/collection_metadata.h
@@ -28,7 +28,6 @@
 
 #pragma once
 
-
 #include "mongo/base/disallow_copying.h"
 #include "mongo/base/owned_pointer_vector.h"
 #include "mongo/db/field_ref_set.h"
@@ -40,9 +39,6 @@ namespace mongo {
 
 class ChunkType;
 class MetadataLoader;
-class CollectionMetadata;
-
-typedef std::shared_ptr<const CollectionMetadata> CollectionMetadataPtr;
 
 /**
  * The collection metadata has metadata information about a collection, in particular the
@@ -60,12 +56,16 @@ class CollectionMetadata {
     MONGO_DISALLOW_COPYING(CollectionMetadata);
 
 public:
+    /**
+     * Use the MetadataLoader to fill the empty metadata from the config server, or use
+     * clone*() methods to use existing metadatas to build new ones.
+     *
+     * Unless you are the MetadataLoader or a test you should probably not be using this
+     * directly.
+     */
+    CollectionMetadata();
     ~CollectionMetadata();
 
-    //
-    // cloning support
-    //
-
     /**
      * Returns a new metadata's instance based on 'this's state by removing a 'pending' chunk.
      *
@@ -120,10 +120,6 @@ public:
                                    const ChunkVersion& newShardVersion,
                                    std::string* errMsg) const;
 
-    //
-    // verification logic
-    //
-
     /**
      * Returns true if the document key 'key' is a valid instance of a shard key for this
      * metadata.  The 'key' must contain exactly the same fields as the shard key pattern.
@@ -169,10 +165,6 @@ public:
      */
     bool getNextOrphanRange(const BSONObj& lookupKey, KeyRange* orphanRange) const;
 
-    //
-    // accessors
-    //
-
     ChunkVersion getCollVersion() const {
         return _collVersion;
     }
@@ -201,10 +193,6 @@ public:
         return _pendingMap.size();
     }
 
-    //
-    // reporting
-    //
-
     /**
      * BSON output of the metadata information.
      */
@@ -231,30 +219,18 @@ public:
     std::string toString() const;
 
     /**
-     * Use the MetadataLoader to fill the empty metadata from the config server, or use
-     * clone*() methods to use existing metadatas to build new ones.
+     * This method is used only for unit-tests and it returns a new metadata's instance based on
+     * 'this's state by adding 'chunk'. The new metadata can never be zero.
      *
-     * Unless you are the MetadataLoader or a test you should probably not be using this
-     * directly.
-     */
-    CollectionMetadata();
-
-    /**
-     * TESTING ONLY
-     *
-     * Returns a new metadata's instance based on 'this's state by adding 'chunk'. The new
-     * metadata can never be zero, though (see cloneMinus). The caller owns the new metadata.
-     *
-     * If a new metadata can't be created, returns NULL and fills in 'errMsg', if it was
-     * provided.
+     * It will fassert if the chunk bounds are incorrect or overlap an existing chunk.
      */
-    CollectionMetadata* clonePlusChunk(const ChunkType& chunk,
-                                       const ChunkVersion& newShardVersion,
-                                       std::string* errMsg) const;
+    std::unique_ptr<CollectionMetadata> clonePlusChunk(const BSONObj& minKey,
+                                                       const BSONObj& maxKey,
+                                                       const ChunkVersion& newShardVersion) const;
 
 private:
-    // Effectively, the MetadataLoader is this class's builder. So we open an exception
-    // and grant it friendship.
+    // Effectively, the MetadataLoader is this class's builder. So we open an exception and grant it
+    // friendship.
     friend class MetadataLoader;
 
     // a version for this collection that identifies the collection incarnation (ie, a