Use insertOne for creating new version objects

In `putObjectVerCase1`, switch from using `updateOne` with upsert to
`insertOne` for creating new version objects.

The `putObjectVerCase1` function is designed exclusively for creating
new object versions, each with a freshly generated, unique version ID.
There is no intended scenario where a version object with the same ID
should already exist.

Using `updateOne` with `upsert:true` was therefore logically incorrect.
It could silently overwrite an existing version object in the case of a
version ID collision, leading to data corruption.

By switching to `insertOne`, the operation will now correctly fail with
a duplicate key error if a collision occurs. This fail-fast approach properly
treats a version ID collision as an error, ensuring system integrity.

Issue: ARSN-506

Author: Kerkesni

lib/storage/metadata/mongoclient/MongoClientInterface.ts

@@ -826,14 +826,11 @@ class MongoClientInterface {
         const masterKey = formatMasterKey(objName, params.vFormat);
         // initiating array of operations with version creation
         const ops: AnyBulkWriteOperation<ObjectMetastoreDocument>[] = [{
-            updateOne: {
-                filter: {
+            insertOne: {
+                document: {
                     _id: versionKey,
-                },
-                update: {
-                    $set: { _id: versionKey, value: objVal },
-                },
-                upsert: true,
+                    value: objVal,
+                } as ObjectMetastoreDocument,
             },
         }];
         // filter to get master

tests/functional/metadata/mongodb/putObject.spec.js

@@ -1,12 +1,14 @@
 const async = require('async');
 const assert = require('assert');
+const sinon = require('sinon');
 const werelogs = require('werelogs');
 const { MongoMemoryReplSet } = require('mongodb-memory-server');
 const { errors, versioning } = require('../../../../index');
 const logger = new werelogs.Logger('MongoClientInterface', 'debug', 'debug');
 const BucketInfo = require('../../../../lib/models/BucketInfo').default;
 const MetadataWrapper =
     require('../../../../lib/storage/metadata/MetadataWrapper');
+const { VersionID } = require('../../../../lib/versioning');
 const { BucketVersioningKeyFormat } = versioning.VersioningConstants;
 
 const IMPL_NAME = 'mongodb';
@@ -128,6 +130,7 @@ describe('MongoClientInterface:metadata.putObjectMD', () => {
             });
 
             afterEach(done => {
+                sinon.restore();
                 metadata.deleteBucket(BUCKET_NAME, logger, done);
             });
 
@@ -354,6 +357,71 @@ describe('MongoClientInterface:metadata.putObjectMD', () => {
                 ], done);
             });
 
+            it(`should fail on versionID conflict when putting a new version ${variation.it}`, done => {
+                const objVal = {
+                    key: OBJECT_NAME,
+                };
+                const params = {
+                    versioning: true,
+                    versionId: null,
+                    repairMaster: null,
+                };
+
+                // simulate a versionId collision by always generating the same versionId
+                const genVID = sinon.stub(VersionID, 'generateUniqueVersionId')
+                    .returns('test-version-id');
+
+                async.series([
+                    // We first create a master and a version
+                    next => metadata.putObjectMD(BUCKET_NAME, OBJECT_NAME, objVal, params, logger, next),
+                    // We put another version of the object with the same versionId
+                    next => metadata.putObjectMD(BUCKET_NAME, OBJECT_NAME, objVal, params, logger, next),
+                ], err => {
+                    assert(err.is.InternalError,
+                        `expected InternalError, got ${err.name} with message: ${err.message}`,
+                    );
+                    // make sure the retry triggered on the first collision detection
+                    assert(genVID.calledThrice,
+                        `expected generateUniqueVersionId to be called thrice, got ${genVID.callCount} times`);
+                    done();
+                });
+            });
+
+            it(`should succeed on version creation after a versionId collision ${variation.it}`, done => {
+                const objVal = {
+                    key: OBJECT_NAME,
+                };
+                const params = {
+                    versioning: true,
+                    versionId: null,
+                    repairMaster: null,
+                };
+
+                // simulate a versionId collision by always generating the same versionId
+                const genVID = sinon.stub(VersionID, 'generateUniqueVersionId')
+                    .onFirstCall().returns('test-version-id')
+                    .onSecondCall().returns('test-version-id') // trigger collision
+                    .onThirdCall().returns('test-version-id-retry'); // change versionId on retry
+
+                async.series([
+                    // We first create a master and a version
+                    next => metadata.putObjectMD(BUCKET_NAME, OBJECT_NAME, objVal, params, logger, next),
+                    // We put another version of the object with the same versionId
+                    next => metadata.putObjectMD(BUCKET_NAME, OBJECT_NAME, objVal, params, logger, next),
+                ], (err, res) => {
+                    assert.ifError(err, `expected no error, got ${err}`);
+                    // make sure the retry triggered on the first collision detection
+                    assert(genVID.calledThrice,
+                        `expected generateUniqueVersionId to be called thrice, got ${genVID.callCount} times`);
+                    // make sure the last call returned a different versionId
+                    const vid1 = JSON.parse(res[0]).versionId;
+                    const vid2 = JSON.parse(res[1]).versionId;
+                    assert.notStrictEqual(vid1, vid2,
+                        `expected different versionIds, got ${vid1} and ${vid2}`);
+                    done();
+                });
+            });
+
             itOnlyInV1(`Should delete master when last version is delete marker ${variation.it}`, done => {
                 const objVal = {
                     key: OBJECT_NAME,