Merge branch 'bugfix/CLDSRV-816-delete-unversioned-object-versioned-bucket' into q/9.0

Author: bert-e

lib/api/apiUtils/object/versioning.js

@@ -98,6 +98,7 @@ function _storeNullVersionMD(bucketName, objKey, nullVersionId, objMD, log, cb)
             isNull2: true,
         });
     }
+    nullVersionMD.originOp = 's3:StoreNullVersion';
     metadata.putObjectMD(bucketName, objKey, nullVersionMD, { versionId }, log, err => {
         if (err) {
             log.debug('error from metadata storing null version as new version',

tests/unit/api/multipartUpload.js

@@ -2396,6 +2396,7 @@ describe('complete mpu with versioning', () => {
                             assert.strictEqual(objVal.completeInProgress, true);
                         } else {
                             assert.strictEqual(params.replayId, testUploadId);
+                            assert.strictEqual(objVal.originOp, 's3:ObjectCreated:CompleteMultipartUpload');
                             metadataBackend.putObject = origPutObject;
                         }
                         origPutObject(
@@ -2413,6 +2414,7 @@ describe('complete mpu with versioning', () => {
                 metadataBackend.putObject =
                     (putBucketName, objName, objVal, params, log, cb) => {
                         assert.strictEqual(params.oldReplayId, testUploadId);
+                        assert.strictEqual(objVal.originOp, 's3:ObjectCreated:Put');
                         metadataBackend.putObject = origPutObject;
                         origPutObject(
                             putBucketName, objName, objVal, params, log, cb);

tests/unit/api/objectCopy.js

@@ -113,6 +113,15 @@ describe('objectCopy with versioning', () => {
         objectCopy(authInfo, testObjectCopyRequest, sourceBucketName, objectKey,
             undefined, log, err => {
                 assert.ifError(err, `Unexpected err: ${err}`);
+                sinon.assert.calledWith(
+                    metadata.putObjectMD.lastCall,
+                    destBucketName,
+                    objectKey,
+                    sinon.match({ _data: { originOp: 's3:ObjectCreated:Copy' } }),
+                    sinon.match.any,
+                    sinon.match.any,
+                    sinon.match.any
+                );
                 setImmediate(() => {
                     versioningTestUtils
                         .assertDataStoreValues(ds, expectedValues);
@@ -276,7 +285,9 @@ describe('non-versioned objectCopy', () => {
                 undefined, log, next),
             async () => {
                 sinon.assert.calledWith(metadata.putObjectMD.lastCall,
-                    destBucketName, objectKey, any, sinon.match({
+                    destBucketName, objectKey, sinon.match({
+                        _data: { originOp: 's3:ObjectCreated:Copy' },
+                    }), sinon.match({
                         needOplogUpdate: undefined,
                         originOp: undefined,
                     }), any, any);
@@ -291,7 +302,9 @@ describe('non-versioned objectCopy', () => {
                 undefined, log, next),
             async () => {
                 sinon.assert.calledWith(metadata.putObjectMD.lastCall,
-                    destBucketName, objectKey, any, sinon.match({
+                    destBucketName, objectKey, sinon.match({
+                        _data: { originOp: 's3:ObjectCreated:Copy' },
+                    }), sinon.match({
                         needOplogUpdate: undefined,
                         originOp: undefined,
                     }), any, any);

tests/unit/api/objectDelete.js

@@ -7,7 +7,7 @@ const services = require('../../../lib/services');
 const { bucketPut } = require('../../../lib/api/bucketPut');
 const bucketPutACL = require('../../../lib/api/bucketPutACL');
 const constants = require('../../../constants');
-const { cleanup, DummyRequestLogger, makeAuthInfo } = require('../helpers');
+const { cleanup, DummyRequestLogger, makeAuthInfo, versioningTestUtils} = require('../helpers');
 const objectPut = require('../../../lib/api/objectPut');
 const { objectDelete, objectDeleteInternal } = require('../../../lib/api/objectDelete');
 const objectGet = require('../../../lib/api/objectGet');
@@ -16,6 +16,7 @@ const mpuUtils = require('../utils/mpuUtils');
 const metadataswitch = require('../metadataswitch');
 const { fakeMetadataArchive } = require('../../functional/aws-node-sdk/test/utils/init');
 const bucketPutNotification = require('../../../lib/api/bucketPutNotification');
+const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning');
 
 const any = sinon.match.any;
 const originalDeleteObject = services.deleteObject;
@@ -32,6 +33,8 @@ const lateDate = new Date();
 earlyDate.setMinutes(earlyDate.getMinutes() - 30);
 lateDate.setMinutes(lateDate.getMinutes() + 30);
 
+const enableVersioningRequest = versioningTestUtils.createBucketPutVersioningReq(bucketName, 'Enabled');
+
 function testAuth(bucketOwner, authUser, bucketPutReq, objPutReq, objDelReq,
     log, cb) {
     bucketPut(bucketOwner, bucketPutReq, log, () => {
@@ -371,3 +374,65 @@ describe('objectDelete API', () => {
         });
     });
 });
+
+describe('objectDelete API with versioning', () => {
+    let testPutObjectRequest;
+
+    beforeEach(() => {
+        cleanup();
+        testPutObjectRequest = new DummyRequest({
+            bucketName,
+            namespace,
+            objectKey,
+            headers: {},
+            url: `/${bucketName}/${objectKey}`,
+        }, postBody);
+
+        sinon.stub(services, 'deleteObject').callsFake(originalDeleteObject);
+        sinon.spy(metadataswitch, 'putObjectMD');
+        sinon.spy(metadataswitch, 'deleteObjectMD');
+    });
+
+    afterEach(() => {
+        sinon.restore();
+    });
+
+    const testBucketPutRequest = new DummyRequest({
+        bucketName,
+        namespace,
+        headers: {},
+        url: `/${bucketName}`,
+    });
+    const testDeleteRequest = new DummyRequest({
+        bucketName,
+        namespace,
+        objectKey,
+        headers: {},
+        url: `/${bucketName}/${objectKey}`,
+    });
+
+    it('should upgrade master-only document to a version document when storing a delete marker version', done => {
+        async.series([
+            next => bucketPut(authInfo, testBucketPutRequest, log, next),
+            next => objectPut(authInfo, testPutObjectRequest, undefined, log, next),
+            next => bucketPutVersioning(authInfo, enableVersioningRequest, log, next),
+            next => objectDelete(authInfo, testDeleteRequest, log, next),
+            async () => {
+                const calls = metadataswitch.putObjectMD.getCalls();
+                sinon.assert.calledWith(calls[calls.length - 2],
+                    bucketName, objectKey, sinon.match({
+                        versionId: sinon.match.truthy,
+                        isNull: true,
+                        originOp: 's3:StoreNullVersion',
+                    }), any, any, any);
+            },
+            async () => {
+                // New version document (delete marker) was created with the right originOp.
+                sinon.assert.calledWith(metadataswitch.putObjectMD.lastCall,
+                    bucketName, objectKey, sinon.match({
+                        _data: { originOp: 's3:ObjectRemoved:DeleteMarkerCreated' },
+                    }), any, any, any);
+            },
+        ], done);
+    });
+});

tests/unit/api/objectPut.js

@@ -980,6 +980,31 @@ describe('objectPut API with versioning', () => {
         });
     });
 
+    it('should set originOp when moving master-only document to a version document', done => {
+        async.series([
+            next => bucketPut(authInfo, testPutBucketRequest, log, next),
+            next => objectPut(authInfo, testPutObjectRequest, undefined, log, next),
+            next => bucketPutVersioning(authInfo, enableVersioningRequest, log, next),
+            next => objectPut(authInfo, testPutObjectRequest, undefined, log, next),
+            async () => {
+                // Old master-only document was moved to a proper version, with originOp overridden to prevent
+                // unexpected bucket notifications.
+                const calls = metadata.putObjectMD.getCalls();
+                sinon.assert.calledWith(calls[calls.length - 2],
+                    bucketName, objectName, sinon.match({
+                        originOp: 's3:StoreNullVersion',
+                    }), any, any, any);
+            },
+            async () => {
+                // New version document was created with the right originOp.
+                sinon.assert.calledWith(metadata.putObjectMD.lastCall,
+                    bucketName, objectName, sinon.match({
+                       _data: { originOp: 's3:ObjectCreated:Put' },
+                    }), any, any, any);
+            },
+        ], done);
+    });
+
     it('should not pass needOplogUpdate when writing new object', done => {
         async.series([
             next => bucketPut(authInfo, testPutBucketRequest, log, next),