SERVER-24467 Make MoveChunkRequest and dependencies comparable

This change implements the '==' operator on MoveChunkRequest and any
dependent classes which do not have it.

13 files changed, 190 insertions, 22 deletions

diff --git a/src/mongo/client/connection_string.cpp b/src/mongo/client/connection_string.cpp
index 5b7f87c8e02..16a6e78440d 100644
--- a/src/mongo/client/connection_string.cpp
+++ b/src/mongo/client/connection_string.cpp
@@ -164,8 +164,7 @@ void ConnectionString::_finishInit() {
     _string = ss.str();
 }
 
-// TODO: SERVER-23014
-bool ConnectionString::sameLogicalEndpoint(const ConnectionString& other) const {
+bool ConnectionString::operator==(const ConnectionString& other) const {
     if (_type != other._type) {
         return false;
     }
@@ -176,7 +175,7 @@ bool ConnectionString::sameLogicalEndpoint(const ConnectionString& other) const
         case MASTER:
             return _servers[0] == other._servers[0];
         case SET:
-            return _setName == other._setName;
+            return _setName == other._setName && _servers == other._servers;
         case CUSTOM:
             return _string == other._string;
         case LOCAL:
@@ -186,6 +185,10 @@ bool ConnectionString::sameLogicalEndpoint(const ConnectionString& other) const
     MONGO_UNREACHABLE;
 }
 
+bool ConnectionString::operator!=(const ConnectionString& other) const {
+    return !(*this == other);
+}
+
 StatusWith<ConnectionString> ConnectionString::parse(const std::string& url) {
     const std::string::size_type i = url.find('/');
 
diff --git a/src/mongo/client/connection_string.h b/src/mongo/client/connection_string.h
index f6b5ef419ec..91836e36812 100644
--- a/src/mongo/client/connection_string.h
+++ b/src/mongo/client/connection_string.h
@@ -110,12 +110,11 @@ public:
     }
 
     /**
-     * This returns true if this and other point to the same logical entity.
-     * For single nodes, thats the same address.
-     * For replica sets, thats just the same replica set name.
-     * For pair (deprecated) or sync cluster connections, that's the same hosts in any ordering.
+     * Returns true if two connection strings match in terms of their type and the exact order of
+     * their hosts.
      */
-    bool sameLogicalEndpoint(const ConnectionString& other) const;
+    bool operator==(const ConnectionString& other) const;
+    bool operator!=(const ConnectionString& other) const;
 
     DBClientBase* connect(std::string& errmsg, double socketTimeout = 0) const;
 
@@ -165,7 +164,6 @@ private:
      */
     explicit ConnectionString(ConnectionType connType);
 
-
     void _fillServers(std::string s);
     void _finishInit();
 
diff --git a/src/mongo/client/connection_string_test.cpp b/src/mongo/client/connection_string_test.cpp
index 655ff17c207..7592cc2c115 100644
--- a/src/mongo/client/connection_string_test.cpp
+++ b/src/mongo/client/connection_string_test.cpp
@@ -32,19 +32,28 @@
 
 #include "mongo/unittest/unittest.h"
 
+namespace mongo {
 namespace {
 
-using namespace mongo;
-/*
-   TODO: SERVER-23014 SYNC is gone but this can be a good check for equality
-TEST(ConnectionString, EqualitySync) {
-    ConnectionString cs(ConnectionString::SYNC, "a,b,c", "");
+using unittest::assertGet;
 
-    ASSERT(cs.sameLogicalEndpoint(ConnectionString(ConnectionString::SYNC, "a,b,c", "")));
-    ASSERT(cs.sameLogicalEndpoint(ConnectionString(ConnectionString::SYNC, "c,b,a", "")));
-    ASSERT(cs.sameLogicalEndpoint(ConnectionString(ConnectionString::SYNC, "c,a,b", "")));
+TEST(ConnectionString, EqualityOperatorMaster) {
+    const auto cs = assertGet(ConnectionString::parse("TestHostA:12345"));
+    ASSERT(cs == assertGet(ConnectionString::parse("TestHostA:12345")));
+    ASSERT_FALSE(cs != assertGet(ConnectionString::parse("TestHostA:12345")));
+    ASSERT(cs != assertGet(ConnectionString::parse("TestHostB:12345")));
+    ASSERT_FALSE(cs == assertGet(ConnectionString::parse("TestHostB:12345")));
+}
 
-    ASSERT(!cs.sameLogicalEndpoint(ConnectionString(ConnectionString::SYNC, "d,a,b", "")));
+TEST(ConnectionString, EqualityOperatorReplicaSet) {
+    const auto cs = assertGet(ConnectionString::parse("TestRS/TestHostA:12345,TestHostB:12345"));
+    ASSERT(cs == assertGet(ConnectionString::parse("TestRS/TestHostA:12345,TestHostB:12345")));
+    ASSERT_FALSE(cs == assertGet(ConnectionString::parse("TestHostA:12345")));
+    ASSERT_FALSE(cs ==
+                 assertGet(ConnectionString::parse("TestRS/TestHostB:12345,TestHostA:12345")));
+    ASSERT_FALSE(cs ==
+                 assertGet(ConnectionString::parse("TestRS1/TestHostA:12345,TestHostB:12345")));
 }
-*/
+
 }  // namespace
+}  // namespace mongo
diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript
index cccd437c4e7..350e0b14ce5 100644
--- a/src/mongo/s/SConscript
+++ b/src/mongo/s/SConscript
@@ -108,7 +108,6 @@ env.CppUnitTest(
         'catalog/type_mongos_test.cpp',
         'catalog/type_shard_test.cpp',
         'catalog/type_tags_test.cpp',
-        'chunk_diff_test.cpp',
         'chunk_version_test.cpp',
         'migration_secondary_throttle_options_test.cpp',
         'move_chunk_request_test.cpp',
@@ -121,6 +120,17 @@ env.CppUnitTest(
     ]
 )
 
+# This test is very slow in debug mode, so it is put in a separate binary by itself
+env.CppUnitTest(
+    target='chunk_diff_test',
+    source=[
+        'chunk_diff_test.cpp',
+    ],
+    LIBDEPS=[
+        'common',
+    ]
+)
+
 #
 # Implementations of components to perform cluster operations in mongos
 #
diff --git a/src/mongo/s/catalog/type_chunk.cpp b/src/mongo/s/catalog/type_chunk.cpp
index 55e7020fa01..1ab80f0e674 100644
--- a/src/mongo/s/catalog/type_chunk.cpp
+++ b/src/mongo/s/catalog/type_chunk.cpp
@@ -105,6 +105,14 @@ std::string ChunkRange::toString() const {
     return str::stream() << "[" << _minKey << ", " << _maxKey << ")";
 }
 
+bool ChunkRange::operator==(const ChunkRange& other) const {
+    return _minKey.woCompare(other._minKey) == 0 && _maxKey.woCompare(other._maxKey) == 0;
+}
+
+bool ChunkRange::operator!=(const ChunkRange& other) const {
+    return !(*this == other);
+}
+
 StatusWith<ChunkType> ChunkType::fromBSON(const BSONObj& source) {
     ChunkType chunk;
 
diff --git a/src/mongo/s/catalog/type_chunk.h b/src/mongo/s/catalog/type_chunk.h
index e731ed0ab43..f988a09028d 100644
--- a/src/mongo/s/catalog/type_chunk.h
+++ b/src/mongo/s/catalog/type_chunk.h
@@ -74,6 +74,13 @@ public:
 
     std::string toString() const;
 
+    /**
+     * Returns true if two chunk ranges match exactly in terms of the min and max keys (including
+     * element order within the keys).
+     */
+    bool operator==(const ChunkRange& other) const;
+    bool operator!=(const ChunkRange& other) const;
+
 private:
     BSONObj _minKey;
     BSONObj _maxKey;
diff --git a/src/mongo/s/commands/cluster_move_primary_cmd.cpp b/src/mongo/s/commands/cluster_move_primary_cmd.cpp
index f5ba8b9fb93..bc215790b7c 100644
--- a/src/mongo/s/commands/cluster_move_primary_cmd.cpp
+++ b/src/mongo/s/commands/cluster_move_primary_cmd.cpp
@@ -142,7 +142,7 @@ public:
         shared_ptr<Shard> fromShard = grid.shardRegistry()->getShard(txn, config->getPrimaryId());
         invariant(fromShard);
 
-        if (fromShard->getConnString().sameLogicalEndpoint(toShard->getConnString())) {
+        if (fromShard->getId() == toShard->getId()) {
             errmsg = "it is already the primary";
             return false;
         }
diff --git a/src/mongo/s/migration_secondary_throttle_options.cpp b/src/mongo/s/migration_secondary_throttle_options.cpp
index e6520c2e1c3..210c2e76219 100644
--- a/src/mongo/s/migration_secondary_throttle_options.cpp
+++ b/src/mongo/s/migration_secondary_throttle_options.cpp
@@ -172,4 +172,14 @@ BSONObj MigrationSecondaryThrottleOptions::toBSON() const {
     return builder.obj();
 }
 
+bool MigrationSecondaryThrottleOptions::operator==(
+    const MigrationSecondaryThrottleOptions& other) const {
+    return toBSON().woCompare(other.toBSON()) == 0;
+}
+
+bool MigrationSecondaryThrottleOptions::operator!=(
+    const MigrationSecondaryThrottleOptions& other) const {
+    return !(*this == other);
+}
+
 }  // namespace mongo
diff --git a/src/mongo/s/migration_secondary_throttle_options.h b/src/mongo/s/migration_secondary_throttle_options.h
index e6379250eea..d18f41a8dad 100644
--- a/src/mongo/s/migration_secondary_throttle_options.h
+++ b/src/mongo/s/migration_secondary_throttle_options.h
@@ -125,6 +125,12 @@ public:
     void append(BSONObjBuilder* builder) const;
     BSONObj toBSON() const;
 
+    /**
+     * Returns true if the options match exactly.
+     */
+    bool operator==(const MigrationSecondaryThrottleOptions& other) const;
+    bool operator!=(const MigrationSecondaryThrottleOptions& other) const;
+
 private:
     MigrationSecondaryThrottleOptions(SecondaryThrottleOption secondaryThrottle,
                                       boost::optional<BSONObj> writeConcernBSON);
diff --git a/src/mongo/s/migration_secondary_throttle_options_test.cpp b/src/mongo/s/migration_secondary_throttle_options_test.cpp
index ec65d099ef9..9bde6a2700b 100644
--- a/src/mongo/s/migration_secondary_throttle_options_test.cpp
+++ b/src/mongo/s/migration_secondary_throttle_options_test.cpp
@@ -182,5 +182,23 @@ TEST(MigrationSecondaryThrottleOptions, ParseFailsNotSpecifiedInCommandBSONWrite
     ASSERT_EQ(ErrorCodes::UnsupportedFormat, status.getStatus().code());
 }
 
+TEST(MigrationSecondaryThrottleOptions, EqualityOperatorSameValue) {
+    auto value1 = MigrationSecondaryThrottleOptions::createWithWriteConcern(
+        WriteConcernOptions("majority", WriteConcernOptions::SyncMode::JOURNAL, 30000));
+    auto value2 = MigrationSecondaryThrottleOptions::createWithWriteConcern(
+        WriteConcernOptions("majority", WriteConcernOptions::SyncMode::JOURNAL, 30000));
+
+    ASSERT(value1 == value2);
+}
+
+TEST(MigrationSecondaryThrottleOptions, EqualityOperatorDifferentValues) {
+    auto value1 = MigrationSecondaryThrottleOptions::createWithWriteConcern(
+        WriteConcernOptions("majority", WriteConcernOptions::SyncMode::JOURNAL, 30000));
+    auto value2 = MigrationSecondaryThrottleOptions::createWithWriteConcern(
+        WriteConcernOptions("majority", WriteConcernOptions::SyncMode::JOURNAL, 60000));
+
+    ASSERT(!(value1 == value2));
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/s/move_chunk_request.cpp b/src/mongo/s/move_chunk_request.cpp
index a3f7bd91fda..8370000d511 100644
--- a/src/mongo/s/move_chunk_request.cpp
+++ b/src/mongo/s/move_chunk_request.cpp
@@ -154,4 +154,31 @@ void MoveChunkRequest::appendAsCommand(BSONObjBuilder* builder,
     builder->append(kTakeDistLock, takeDistLock);
 }
 
+bool MoveChunkRequest::operator==(const MoveChunkRequest& other) const {
+    if (_nss != other._nss)
+        return false;
+    if (_configServerCS != other._configServerCS)
+        return false;
+    if (_fromShardId != other._fromShardId)
+        return false;
+    if (_toShardId != other._toShardId)
+        return false;
+    if (_range != other._range)
+        return false;
+    if (_maxChunkSizeBytes != other._maxChunkSizeBytes)
+        return false;
+    if (_secondaryThrottle != other._secondaryThrottle)
+        return false;
+    if (_waitForDelete != other._waitForDelete)
+        return false;
+    if (_takeDistLock != other._takeDistLock)
+        return false;
+
+    return true;
+}
+
+bool MoveChunkRequest::operator!=(const MoveChunkRequest& other) const {
+    return !(*this == other);
+}
+
 }  // namespace mongo
diff --git a/src/mongo/s/move_chunk_request.h b/src/mongo/s/move_chunk_request.h
index 6dadfbd2c3b..3cbb4101c1a 100644
--- a/src/mongo/s/move_chunk_request.h
+++ b/src/mongo/s/move_chunk_request.h
@@ -114,6 +114,13 @@ public:
         return _takeDistLock;
     }
 
+    /**
+     * Returns true if the requests match exactly in terms of the field values and the order of
+     * elements within the BSON-typed fields.
+     */
+    bool operator==(const MoveChunkRequest& other) const;
+    bool operator!=(const MoveChunkRequest& other) const;
+
 private:
     MoveChunkRequest(NamespaceString nss,
                      ChunkRange range,
diff --git a/src/mongo/s/move_chunk_request_test.cpp b/src/mongo/s/move_chunk_request_test.cpp
index b85feb62468..7bf5d8b46a6 100644
--- a/src/mongo/s/move_chunk_request_test.cpp
+++ b/src/mongo/s/move_chunk_request_test.cpp
@@ -29,6 +29,7 @@
 #include "mongo/platform/basic.h"
 
 #include "mongo/bson/bsonmisc.h"
+#include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/s/move_chunk_request.h"
 #include "mongo/unittest/unittest.h"
 
@@ -42,7 +43,7 @@ TEST(MoveChunkRequest, CreateAsCommandComplete) {
     BSONObjBuilder builder;
     MoveChunkRequest::appendAsCommand(
         &builder,
-        NamespaceString("TestDB.TestColl"),
+        NamespaceString("TestDB", "TestColl"),
         ChunkVersion(2, 3, OID::gen()),
         assertGet(ConnectionString::parse("TestConfigRS/CS1:12345,CS2:12345,CS3:12345")),
         "shard0001",
@@ -70,5 +71,69 @@ TEST(MoveChunkRequest, CreateAsCommandComplete) {
     ASSERT_EQ(true, request.getTakeDistLock());
 }
 
+TEST(MoveChunkRequest, EqualityOperatorSameValue) {
+    BSONObjBuilder builder;
+    MoveChunkRequest::appendAsCommand(
+        &builder,
+        NamespaceString("TestDB", "TestColl"),
+        ChunkVersion(2, 3, OID::gen()),
+        assertGet(ConnectionString::parse("TestConfigRS/CS1:12345,CS2:12345,CS3:12345")),
+        "shard0001",
+        "shard0002",
+        ChunkRange(BSON("Key" << -100), BSON("Key" << 100)),
+        1024,
+        MigrationSecondaryThrottleOptions::create(MigrationSecondaryThrottleOptions::kOff),
+        true,
+        true);
+
+    BSONObj obj = builder.obj();
+
+    auto value1 =
+        assertGet(MoveChunkRequest::createFromCommand(NamespaceString("TestDB", "TestColl"), obj));
+    auto value2 =
+        assertGet(MoveChunkRequest::createFromCommand(NamespaceString("TestDB", "TestColl"), obj));
+
+    ASSERT(value1 == value2);
+    ASSERT_FALSE(value1 != value2);
+}
+
+TEST(MoveChunkRequest, EqualityOperatorDifferentValues) {
+    BSONObjBuilder builder1;
+    MoveChunkRequest::appendAsCommand(
+        &builder1,
+        NamespaceString("TestDB", "TestColl"),
+        ChunkVersion(2, 3, OID::gen()),
+        assertGet(ConnectionString::parse("TestConfigRS/CS1:12345,CS2:12345,CS3:12345")),
+        "shard0001",
+        "shard0002",
+        ChunkRange(BSON("Key" << -100), BSON("Key" << 100)),
+        1024,
+        MigrationSecondaryThrottleOptions::create(MigrationSecondaryThrottleOptions::kOff),
+        true,
+        true);
+
+    auto value1 = assertGet(
+        MoveChunkRequest::createFromCommand(NamespaceString("TestDB", "TestColl"), builder1.obj()));
+
+    BSONObjBuilder builder2;
+    MoveChunkRequest::appendAsCommand(
+        &builder2,
+        NamespaceString("TestDB", "TestColl"),
+        ChunkVersion(2, 3, OID::gen()),
+        assertGet(ConnectionString::parse("TestConfigRS/CS1:12345,CS2:12345,CS3:12345")),
+        "shard0001",
+        "shard0002",
+        ChunkRange(BSON("Key" << 100), BSON("Key" << 200)),  // Different key ranges
+        1024,
+        MigrationSecondaryThrottleOptions::create(MigrationSecondaryThrottleOptions::kOff),
+        true,
+        true);
+    auto value2 = assertGet(
+        MoveChunkRequest::createFromCommand(NamespaceString("TestDB", "TestColl"), builder2.obj()));
+
+    ASSERT_FALSE(value1 == value2);
+    ASSERT(value1 != value2);
+}
+
 }  // namespace
 }  // namespace mongo