SERVER-62783 Get rid of ChunkVersion::toBSON

Special-case the generation of the {0:minor|major, 1:epoch, ...} format

13 files changed, 73 insertions, 74 deletions

diff --git a/src/mongo/db/pipeline/document_source_merge.idl b/src/mongo/db/pipeline/document_source_merge.idl
index 4def6572ddb..7948517e773 100644
--- a/src/mongo/db/pipeline/document_source_merge.idl
+++ b/src/mongo/db/pipeline/document_source_merge.idl
@@ -112,7 +112,7 @@ structs:
                              do not match.
 
             targetCollectionVersion:
-                type: ChunkVersion
+                type: ChunkVersionArrayWronglyEncodedAsBSONObjFormat
                 optional: true
                 description: If set, the collection's ChunkVersion found when parsed on mongos. Can
                              be used to check if a collection has since been dropped and re-created,
diff --git a/src/mongo/db/s/chunk_splitter.cpp b/src/mongo/db/s/chunk_splitter.cpp
index d3d867c932e..54a849f8b93 100644
--- a/src/mongo/db/s/chunk_splitter.cpp
+++ b/src/mongo/db/s/chunk_splitter.cpp
@@ -411,7 +411,7 @@ void ChunkSplitter::_runAutosplit(std::shared_ptr<ChunkSplitStateDriver> chunkSp
               "namespace"_attr = nss,
               "minKey"_attr = redact(chunk.getMin()),
               "maxKey"_attr = redact(chunk.getMax()),
-              "lastmod"_attr = redact(chunk.getLastmod().toBSON()),
+              "lastmod"_attr = chunk.getLastmod(),
               "splitPoints"_attr = splitPoints.size(),
               "maxChunkSizeBytes"_attr = maxChunkSizeBytes,
               "extraInfo"_attr =
diff --git a/src/mongo/db/s/config/configsvr_merge_chunks_command.cpp b/src/mongo/db/s/config/configsvr_merge_chunks_command.cpp
index c794f6714c4..60494a2155f 100644
--- a/src/mongo/db/s/config/configsvr_merge_chunks_command.cpp
+++ b/src/mongo/db/s/config/configsvr_merge_chunks_command.cpp
@@ -94,8 +94,8 @@ public:
                                                                       request().getChunkRange(),
                                                                       request().getShard(),
                                                                       request().getValidAfter()));
-            return ConfigSvrMergeResponse{
-                ChunkVersion::fromBSONThrowing(shardAndCollVers["shardVersion"].Obj())};
+            return ConfigSvrMergeResponse{ChunkVersion::parsePositionalFormat(
+                shardAndCollVers[ChunkVersion::kShardVersionField])};
         }
 
     private:
diff --git a/src/mongo/db/s/migration_coordinator_document.idl b/src/mongo/db/s/migration_coordinator_document.idl
index 028cb194582..391cea306aa 100644
--- a/src/mongo/db/s/migration_coordinator_document.idl
+++ b/src/mongo/db/s/migration_coordinator_document.idl
@@ -94,7 +94,7 @@ structs:
                 type: chunk_range
                 description: "The range being migrated."
             preMigrationChunkVersion:
-                type: ChunkVersion
+                type: ChunkVersionArrayWronglyEncodedAsBSONObjFormat
                 description: "The version, at the start of the migration, of the chunk being moved."
             decision:
                 type: Decision
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index 47a3486bc19..3f829e6791f 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -269,7 +269,7 @@ ChunkMap ChunkMap::createMerged(
 BSONObj ChunkMap::toBSON() const {
     BSONObjBuilder builder;
 
-    builder.append("startingVersion"_sd, getVersion().toBSON());
+    getVersion().serializeToBSON("startingVersion"_sd, &builder);
     builder.append("chunkCount", static_cast<int64_t>(_chunkMap.size()));
 
     {
@@ -864,7 +864,7 @@ void ComparableChunkVersion::setChunkVersion(const ChunkVersion& version) {
 BSONObj ComparableChunkVersion::toBSONForLogging() const {
     BSONObjBuilder builder;
     if (_chunkVersion)
-        builder.append("chunkVersion"_sd, _chunkVersion->toBSON());
+        _chunkVersion->serializeToBSON("chunkVersion"_sd, &builder);
     else
         builder.append("chunkVersion"_sd, "None");
 
diff --git a/src/mongo/s/chunk_manager.h b/src/mongo/s/chunk_manager.h
index ef38af2edc4..3a3c6ec5a7a 100644
--- a/src/mongo/s/chunk_manager.h
+++ b/src/mongo/s/chunk_manager.h
@@ -37,7 +37,6 @@
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/query/collation/collator_interface.h"
 #include "mongo/s/chunk.h"
-#include "mongo/s/chunk_version.h"
 #include "mongo/s/client/shard.h"
 #include "mongo/s/database_version.h"
 #include "mongo/s/resharding/type_collection_fields_gen.h"
diff --git a/src/mongo/s/chunk_version.cpp b/src/mongo/s/chunk_version.cpp
index bf6f9010b1e..f11fae7f7cb 100644
--- a/src/mongo/s/chunk_version.cpp
+++ b/src/mongo/s/chunk_version.cpp
@@ -37,19 +37,18 @@ namespace mongo {
 
 constexpr StringData ChunkVersion::kShardVersionField;
 
-StatusWith<ChunkVersion> ChunkVersion::fromBSON(const BSONObj& obj) {
+ChunkVersion ChunkVersion::parsePositionalFormat(const BSONObj& obj) {
     BSONObjIterator it(obj);
-    if (!it.more())
-        return {ErrorCodes::BadValue, "Unexpected empty version array"};
+    uassert(ErrorCodes::BadValue, "Unexpected empty version array", it.more());
 
     // Expect the major and minor versions (must be present)
     uint64_t combined;
     {
         BSONElement tsPart = it.next();
-        if (tsPart.type() != bsonTimestamp)
-            return {ErrorCodes::TypeMismatch,
-                    str::stream() << "Invalid type " << tsPart.type()
-                                  << " for version major and minor part."};
+        uassert(ErrorCodes::TypeMismatch,
+                str::stream() << "Invalid type " << tsPart.type()
+                              << " for version major and minor part.",
+                tsPart.type() == bsonTimestamp);
         combined = tsPart.timestamp().asULL();
     }
 
@@ -57,10 +56,9 @@ StatusWith<ChunkVersion> ChunkVersion::fromBSON(const BSONObj& obj) {
     boost::optional<OID> epoch;
     {
         BSONElement epochPart = it.next();
-        if (epochPart.type() != jstOID)
-            return {ErrorCodes::TypeMismatch,
-                    str::stream() << "Invalid type " << epochPart.type()
-                                  << " for version epoch part."};
+        uassert(ErrorCodes::TypeMismatch,
+                str::stream() << "Invalid type " << epochPart.type() << " for version epoch part.",
+                epochPart.type() == jstOID);
         epoch = epochPart.OID();
     }
 
@@ -82,9 +80,9 @@ StatusWith<ChunkVersion> ChunkVersion::fromBSON(const BSONObj& obj) {
         timestamp =
             (epoch == UNSHARDED().epoch() ? UNSHARDED().getTimestamp() : IGNORED().getTimestamp());
     } else {
-        return {ErrorCodes::TypeMismatch,
-                str::stream() << "Invalid type " << nextElem.type()
-                              << " for version timestamp part."};
+        uasserted(ErrorCodes::TypeMismatch,
+                  str::stream() << "Invalid type " << nextElem.type()
+                                << " for version timestamp part.");
     }
 
     ChunkVersion version;
@@ -175,20 +173,20 @@ void ChunkVersion::serializeToBSON(StringData field, BSONObjBuilder* builder) co
     arr.append(_timestamp);
 }
 
+BSONObj ChunkVersion::toArrayWronglyEncodedAsBSONObj() const {
+    BSONArrayBuilder arr;
+    arr.appendTimestamp(_combined);
+    arr.append(_epoch);
+    arr.append(_timestamp);
+    return arr.obj();
+}
+
 void ChunkVersion::appendLegacyWithField(BSONObjBuilder* out, StringData field) const {
     out->appendTimestamp(field, _combined);
     out->append(field + "Epoch", _epoch);
     out->append(field + "Timestamp", _timestamp);
 }
 
-BSONObj ChunkVersion::toBSON() const {
-    BSONArrayBuilder b;
-    b.appendTimestamp(_combined);
-    b.append(_epoch);
-    b.append(_timestamp);
-    return b.arr();
-}
-
 std::string ChunkVersion::toString() const {
     return str::stream() << majorVersion() << "|" << minorVersion() << "||" << _epoch << "||"
                          << _timestamp.toString();
diff --git a/src/mongo/s/chunk_version.h b/src/mongo/s/chunk_version.h
index 08bc8efffdc..2be2a743b9e 100644
--- a/src/mongo/s/chunk_version.h
+++ b/src/mongo/s/chunk_version.h
@@ -62,23 +62,24 @@ public:
     ChunkVersion() : ChunkVersion(0, 0, OID(), Timestamp()) {}
 
     /**
-     * Parses 'obj', which is expected to have three elements: the major/minor versions, the object
-     * id, and the timestamp. The field names don't matter, so 'obj' can be a BSONArray.
-     */
-    static StatusWith<ChunkVersion> fromBSON(const BSONObj& obj);
-
-    /**
-     * A throwing version of 'fromBSON'.
+     * The methods below parse the "positional" formats of:
+     *
+     *  [major, minor, epoch, <optional canThrowSSVOnIgnored> timestamp]
+     *      OR
+     *  {0: major, 1:minor, 2:epoch, 3:<optional canThrowSSVOnIgnored>, 4:timestamp}
+     *
+     * The latter format was introduced by mistake in 4.4 and is no longer generated from 5.3
+     * onwards, but it is backwards compatible with the 5.2 and older binaries.
      */
-    static ChunkVersion fromBSONThrowing(const BSONObj& obj) {
-        return uassertStatusOK(fromBSON(obj));
+    static ChunkVersion parsePositionalFormat(const BSONObj& obj);
+    static ChunkVersion parsePositionalFormat(const BSONElement& element) {
+        return parsePositionalFormat(element.Obj());
     }
-
     static ChunkVersion fromBSONArrayThrowing(const BSONElement& element) {
         uassert(ErrorCodes::TypeMismatch,
                 "Invalid type for chunkVersion element. Expected an array",
                 element.type() == Array);
-        return fromBSONThrowing(element.Obj());
+        return parsePositionalFormat(element);
     }
 
     /**
@@ -210,6 +211,13 @@ public:
     }
 
     /**
+     * Serializes the version held by this object to 'out' in the form:
+     *  { ..., <field>: [ <combined major/minor>, <OID epoch>, <Timestamp> ], ... }.
+     */
+    void serializeToBSON(StringData fieldName, BSONObjBuilder* builder) const;
+    BSONObj toArrayWronglyEncodedAsBSONObj() const;
+
+    /**
      * NOTE: This format is being phased out. Use serializeToBSON instead.
      *
      * Serializes the version held by this object to 'out' in the legacy form:
@@ -219,14 +227,6 @@ public:
      */
     void appendLegacyWithField(BSONObjBuilder* out, StringData field) const;
 
-    BSONObj toBSON() const;
-
-    /**
-     * Serializes the version held by this object to 'out' in the form:
-     *  { ..., <field>: [ <combined major/minor>, <OID epoch>, <Timestamp> ], ... }.
-     */
-    void serializeToBSON(StringData field, BSONObjBuilder* builder) const;
-
     std::string toString() const;
 
     // Methods that are here for the purposes of parsing of ShardCollectionType only
diff --git a/src/mongo/s/chunk_version.idl b/src/mongo/s/chunk_version.idl
index e2e50aadcff..e2751378e41 100644
--- a/src/mongo/s/chunk_version.idl
+++ b/src/mongo/s/chunk_version.idl
@@ -34,11 +34,23 @@ global:
         - "mongo/s/chunk_version.h"
 
 types:
-    # serializes as { 0: <major/minor>, 1: <epoch>, 2: <timestamp> }
-    # equivalent to using ChunkVersion::toBSON / ChunkVersion::fromBSON
+    # The canonical serialisation format for ChunkVersions, which is written as:
+    #   [ <major/minor>, <epoch>, <timestamp> ]
     ChunkVersion:
+        bson_serialization_type: any
+        description: An object representing a chunk version for a collection.
+        cpp_type: ChunkVersion
+        serializer: ChunkVersion::serializeToBSON
+        deserializer: ChunkVersion::parsePositionalFormat
+
+    # DO NOT add any new usages of this format, use ChunkVersion above instead
+    #
+    # Serialisation format wrongly introduced in 4.4, which needs to be kept around for backwards
+    # compatibility with certain on-disk documents and on-wire messages. It is written as:
+    #   { 0: <major/minor>, 1: <epoch>, 2: <timestamp> } (note the array encoded as a BSON object)
+    ChunkVersionArrayWronglyEncodedAsBSONObjFormat:
         bson_serialization_type: object
         description: An object representing a chunk version for a collection.
         cpp_type: ChunkVersion
-        serializer: ChunkVersion::toBSON
-        deserializer: ChunkVersion::fromBSONThrowing
+        serializer: ChunkVersion::toArrayWronglyEncodedAsBSONObj
+        deserializer: ChunkVersion::parsePositionalFormat
diff --git a/src/mongo/s/request_types/ensure_chunk_version_is_greater_than.idl b/src/mongo/s/request_types/ensure_chunk_version_is_greater_than.idl
index 67aa936413f..b1f69cf9313 100644
--- a/src/mongo/s/request_types/ensure_chunk_version_is_greater_than.idl
+++ b/src/mongo/s/request_types/ensure_chunk_version_is_greater_than.idl
@@ -56,7 +56,7 @@ commands:
             version:
                 description: The version of the chunk, including major version, minor version, and
                              epoch.
-                type: ChunkVersion
+                type: ChunkVersionArrayWronglyEncodedAsBSONObjFormat
                 optional: false
             collectionUUID:
                 description: The collection's UUID. This field was introduced in 5.0 as mandatory.
diff --git a/src/mongo/s/request_types/merge_chunk_request.idl b/src/mongo/s/request_types/merge_chunk_request.idl
index 51b122717df..31b23d1e967 100644
--- a/src/mongo/s/request_types/merge_chunk_request.idl
+++ b/src/mongo/s/request_types/merge_chunk_request.idl
@@ -31,14 +31,14 @@
 global:
     cpp_namespace: "mongo"
     cpp_includes:
-        - "mongo/util/uuid.h"
-        - "mongo/s/chunk_version.h"
         - "mongo/s/request_types/merge_chunk_request_valid.h"
+        - "mongo/util/uuid.h"
     
 imports:
     - "mongo/idl/basic_types.idl"
-    - "mongo/s/sharding_types.idl"
     - "mongo/s/chunk_range.idl"
+    - "mongo/s/chunk_version.idl"
+    - "mongo/s/sharding_types.idl"
 
 types:
     # Non-IDL response used UUIDtoBSON instead of UUID::toCDR
@@ -49,22 +49,13 @@ types:
         deserializer: UUID::parse
         serializer: UUID::toBSON
 
-    # serialize [<major/minor>, <epoch>, <timestamp>]
-    # equivalent to using ChunkVersion::appendToCommand / ChunkVersion::parseFromCommand
-    ChunkVersionArray:
-       bson_serialization_type: any
-       description: "An object representing a chunk version for a collection."
-       cpp_type: "ChunkVersion"
-       serializer: "ChunkVersion::serializeToBSON"
-       deserializer: "ChunkVersion::fromBSONArrayThrowing"
-
 structs:
     ConfigSvrMergeResponse:
         description: "Response of the _configsvrCommitChunksMerge command."
         strict: false
         fields:
             shardVersion:
-                type: ChunkVersionArray
+                type: ChunkVersion
                 description: "Latest version of the shard."
 
 commands:
diff --git a/src/mongo/s/request_types/set_shard_version_request_test.cpp b/src/mongo/s/request_types/set_shard_version_request_test.cpp
index 13d73c863ee..9704d185785 100644
--- a/src/mongo/s/request_types/set_shard_version_request_test.cpp
+++ b/src/mongo/s/request_types/set_shard_version_request_test.cpp
@@ -35,11 +35,10 @@
 #include "mongo/unittest/unittest.h"
 
 namespace mongo {
+namespace {
 
 using unittest::assertGet;
 
-namespace {
-
 TEST(SetShardVersionRequest, ParseFull) {
     const ChunkVersion chunkVersion(1, 2, OID::gen(), Timestamp(1, 1));
 
@@ -109,7 +108,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFull) {
     ASSERT(!ssv.shouldForceRefresh());
     ASSERT(!ssv.isAuthoritative());
     ASSERT_EQ(ssv.getNS().ns(), "db.coll");
-    ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON());
+    ASSERT_EQ(ssv.getNSVersion().toString(), chunkVersion.toString());
 
     ASSERT_BSONOBJ_EQ(ssv.toBSON(),
                       BSON("setShardVersion"
@@ -129,7 +128,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFullAuthoritative) {
     ASSERT(!ssv.shouldForceRefresh());
     ASSERT(ssv.isAuthoritative());
     ASSERT_EQ(ssv.getNS().ns(), "db.coll");
-    ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON());
+    ASSERT_EQ(ssv.getNSVersion().toString(), chunkVersion.toString());
 
     ASSERT_BSONOBJ_EQ(ssv.toBSON(),
                       BSON("setShardVersion"
@@ -149,7 +148,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFullForceRefresh) {
     ASSERT(ssv.shouldForceRefresh());
     ASSERT(!ssv.isAuthoritative());
     ASSERT_EQ(ssv.getNS().ns(), "db.coll");
-    ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON());
+    ASSERT_EQ(ssv.getNSVersion().toString(), chunkVersion.toString());
 
     ASSERT_BSONOBJ_EQ(ssv.toBSON(),
                       BSON("setShardVersion"
diff --git a/src/mongo/s/request_types/sharded_ddl_commands.idl b/src/mongo/s/request_types/sharded_ddl_commands.idl
index 77e851fdf1d..878c9f142c7 100644
--- a/src/mongo/s/request_types/sharded_ddl_commands.idl
+++ b/src/mongo/s/request_types/sharded_ddl_commands.idl
@@ -84,7 +84,7 @@ structs:
         strict: false
         fields:
             collectionVersion:
-                type: ChunkVersion
+                type: ChunkVersionArrayWronglyEncodedAsBSONObjFormat
                 description: "Latest version of the collection"
                 optional: false
 
@@ -141,7 +141,7 @@ structs:
                 description: "UUID of the created collection"
                 optional: true
             collectionVersion:
-                type: ChunkVersion
+                type: ChunkVersionArrayWronglyEncodedAsBSONObjFormat
                 description: "Latest version of the collection"
                 optional: false