Update version isLatest

remove duplicate 'latest' versions
set version.isLatest from object controllers

app/src/controllers/object.js

@@ -311,7 +311,8 @@ const controller = {
         const version = await versionService.create({
           ...data,
           etag: s3Response.ETag,
-          s3VersionId: s3VersionId
+          s3VersionId: s3VersionId,
+          isLatest: true
         }, userId, trx);
         object.versionId = version.id;
 
@@ -597,7 +598,7 @@ const controller = {
       // if request is to delete a version
       if (data.s3VersionId) {
         // delete version in DB
-        await versionService.delete(objId, s3Response.VersionId);
+        await versionService.delete(objId, s3Response.VersionId, userId);
         // prune tags amd metadata
         await metadataService.pruneOrphanedMetadata();
         await tagService.pruneOrphanedTags();
@@ -1135,7 +1136,12 @@ const controller = {
         let version = undefined;
         if (s3Response.VersionId) { // Create new version if bucket versioning enabled
           const s3VersionId = s3Response.VersionId;
-          version = await versionService.create({ ...data, etag: s3Response.ETag, s3VersionId: s3VersionId }, userId, trx);
+          version = await versionService.create({
+            ...data,
+            etag: s3Response.ETag,
+            s3VersionId: s3VersionId,
+            isLatest: true
+          }, userId, trx);
         } else { // Update existing version when bucket versioning not enabled
           version = await versionService.update({
             ...data,
@@ -1265,7 +1271,12 @@ const controller = {
             let version = undefined;
             if (s3Response.VersionId) { // Create new version if bucket versioning enabled
               const s3VersionId = s3Response.VersionId;
-              version = await versionService.create({ ...data, etag: s3Response.ETag, s3VersionId: s3VersionId, isLatest: true }, userId, trx);
+              version = await versionService.create({
+                ...data,
+                etag: s3Response.ETag,
+                s3VersionId: s3VersionId,
+                isLatest: true
+              }, userId, trx);
             } else { // Update existing version when bucket versioning not enabled
               version = await versionService.update({
                 ...data,

app/src/db/models/tables/version.js

@@ -81,7 +81,7 @@ class Version extends Timestamps(Model) {
   static get jsonSchema() {
     return {
       type: 'object',
-      required: ['objectId'],
+      // required: ['objectId'],
       properties: {
         id: { type: 'string', minLength: 1, maxLength: 255 },
         s3VersionId: { type: ['string', 'null'], maxLength: 1024 },

app/src/services/sync.js

@@ -210,7 +210,7 @@ const service = {
       // Drop versions in COMS that are no longer in S3
       await Promise.all(comsVersions.map(async cv => {
         if (cv.s3VersionId && !s3Versions.some(s3v => (s3v.VersionId === cv.s3VersionId))) {
-          await versionService.delete(comsObject.id, (cv.s3VersionId ?? null), trx);
+          await versionService.delete(comsObject.id, (cv.s3VersionId ?? null), userId, trx);
         }
       }));
 
@@ -258,13 +258,9 @@ const service = {
           const comsVersion = comsVersions.find(cv => cv.s3VersionId === s3Version.VersionId);
 
           if (comsVersion) { // Version is in COMS
-            if (s3Version.IsLatest) { // Patch all isLatest flags if isLatest
-              const updated = await versionService.updateIsLatest({
-                id: comsVersion.id,
-                objectId: comsObject.id,
-                isLatest: s3Version.IsLatest
-              }, trx);
-              return { modified: true, version: updated[0] };
+            if (s3Version.IsLatest) { // Patch isLatest flags if changed
+              const updated = await versionService.updateIsLatest(comsVersion.id, trx);
+              return { modified: true, version: updated };
             } else { // Version record not modified
               return { version: comsVersion };
             }

app/src/services/version.js

@@ -55,11 +55,7 @@ const service = {
         .returning('*');
 
       // set all other versions to islatest: false
-      await Version.query(trx)
-        .update({ 'isLatest': false, 'objectId': objectId })
-        .whereNot({ 'id': response.id })
-        .andWhere('objectId', objectId)
-        .returning('*');
+      await service.removeDuplicateLatest(response.id, objectId, trx);
 
       if (!etrx) await trx.commit();
       return Promise.resolve(response);
@@ -96,12 +92,7 @@ const service = {
         .returning('*');
 
       // if new version is latest, set all other versions to islatest: false
-      if (data.isLatest) {
-        await Version.query(trx)
-          .update({ 'isLatest': false, 'objectId': data.id })
-          .whereNot({ 'id': response.id })
-          .andWhere('objectId', data.id);
-      }
+      if (data.isLatest) await service.removeDuplicateLatest(response.id, data.id, trx);
 
       if (!etrx) await trx.commit();
       return Promise.resolve(response);
@@ -116,11 +107,12 @@ const service = {
    * Delete a version record of an object
    * @param {string} objId The object uuid
    * @param {string} s3VersionId The version ID or null if deleting an object
+   * @param {string} [userId=undefined] An optional uuid of a user
    * @param {object} [etrx=undefined] An optional Objection Transaction object
    * @returns {Promise<integer>} The number of remaining versions in db after the delete
    * @throws The error encountered upon db transaction failure
    */
-  delete: async (objId, s3VersionId, etrx = undefined) => {
+  delete: async (objId, s3VersionId, userId = undefined, etrx = undefined) => {
     let trx;
     try {
       trx = etrx ? etrx : await Version.startTransaction();
@@ -133,16 +125,9 @@ const service = {
         .returning('*')
         .throwIfNotFound();
 
-      // set `isLatest: true` on most recent, if none exist with isLatest: true
-      const sq = await Version.query(trx)
-        .where({ 'objectId': objId })
-        .whereNot({ 'id': response[0].id })
-        .orderBy('createdAt', 'desc');
-      if (sq.length && !sq.some(v => v.isLatest).length) {
-        await Version.query(trx)
-          .update({ 'isLatest': true, 'objectId': objId })
-          .where({ 'id': sq[0]?.id, 'objectId': objId });
-      }
+      // sync other versions with isLatest
+      const { syncVersions } = require('./sync');
+      await syncVersions(objId, userId, trx);
 
       if (!etrx) await trx.commit();
       return Promise.resolve(response);
@@ -218,6 +203,40 @@ const service = {
     }
   },
 
+  /**
+   * @function removeDuplicateLatest
+   * Ensures only the specified `versionId` has isLatest: true
+   * @param {string} versionId COMS version uuid
+   * @param {string} objectId COMS object uuid
+   * @param {object} [etrx=undefined] An optional Objection Transaction object
+   * @returns {Promise<Array<object>>} Array of versions that were updated
+   */
+  removeDuplicateLatest: async(versionId, objectId, etrx = undefined) => {
+    let trx;
+    try {
+      trx = etrx ? etrx : await Version.startTransaction();
+
+      const allVersions = await Version.query(trx)
+        .where('objectId', objectId);
+
+      let updated = [];
+      if (allVersions.reduce((acc, curr) => curr.isLatest ? acc + 1 : acc, 0) > 1) {
+        // set all other versions to islatest: false
+        updated = await Version.query(trx)
+          .update({ isLatest: false })
+          .whereNot({ 'id': versionId })
+          .andWhere('objectId',  objectId)
+          .andWhere({ isLatest: true });
+      }
+
+      if (!etrx) await trx.commit();
+      return Promise.resolve(updated);
+    } catch (err) {
+      if (!etrx && trx) await trx.rollback();
+      throw err;
+    }
+  },
+
   /**
    * @function update
    * Updates a version of an object.
@@ -258,53 +277,33 @@ const service = {
 
   /**
    * @function updateIsLatest
-   * updates a given version with isLatest: true|false in COMS db
+   * Set specified version as latest in COMS db
    * and ensures only one version has isLatest: true
-   * @param {string} options.id COMS uuid of a version
-   * @param {string} options.objectId COMS uuid of an object
-   * @param {boolean} options.isLatest isLatest value to set in db
+   * @param {string} versionId COMS version uuid
    * @param {object} [etrx=undefined] An optional Objection Transaction object
-   * @returns {object} Version model of updated version in db
+   * @returns {object} Version model of provided version in db
    */
-  updateIsLatest: async ({ id, objectId, isLatest }, etrx = undefined) => {
+  updateIsLatest: async (versionId, etrx = undefined) => {
     // TODO: consider having accepting a `userId` argument for version.updatedBy when a version becomes 'latest'
     let trx;
     try {
       trx = etrx ? etrx : await Version.startTransaction();
-      // update this version
-      const updated = await Version.query(trx)
-        .update({ isLatest: isLatest, objectId: objectId })
-        .where({ id: id })
-        .returning('*');
-      // if we set this version with isLatest: true
-      if (isLatest) {
-        // set all other versions to islatest: false
-        await Version.query(trx)
-          .update({ isLatest: false, objectId: objectId })
-          .whereNot({ 'id': id })
-          .andWhere('objectId', objectId)
-          .returning('*');
-      }
-      // else we set this version with isLatest: false.
-      else {
-        // integrity process:
-        // if no other versions have isLatest: true
-        // set most recent to true
-        const sq = await Version.query(trx)
-          .where({ 'objectId': objectId })
-          .whereNot({ 'id': updated[0].id })
-          .orderBy('createdAt', 'desc');
 
-        if (sq.length && !sq.some(v => v.isLatest).length) {
-          await Version.query(trx)
-            .update({ 'isLatest': true, 'objectId': objectId })
-            .where({ 'id': sq[0]?.id, 'objectId': objectId });
-        }
+      const current = await Version.query(trx)
+        .findById(versionId)
+        .throwIfNotFound();
+
+      let updated;
+      // if the version is not already marked as isLatest
+      if (!current.isLatest) {
+        // update this version as latest and fetch
+        updated = await Version.query(trx)
+          .updateAndFetchById(versionId, { isLatest: true });
       }
+      await service.removeDuplicateLatest(versionId, current.objectId, trx);
 
       if (!etrx) await trx.commit();
-
-      return Promise.resolve(updated);
+      return Promise.resolve(updated ?? current);
     } catch (err) {
       if (!etrx && trx) await trx.rollback();
       throw err;

app/tests/unit/controllers/object.spec.js

@@ -364,14 +364,15 @@ describe('deleteObject', () => {
   it('should call version service to delete a version', async () => {
     // version delete request includes s3VersionId query param
     req.query = { s3VersionId: '123' };
+    getCurrentUserIdSpy.mockResolvedValue('456');
     // S3 returns version that was deleted
     storageDeleteObjectSpy.mockReturnValue({
       VersionId: '123'
     });
 
     await controller.deleteObject(req, res, next);
     expect(versionDeleteSpy).toHaveBeenCalledTimes(1);
-    expect(versionDeleteSpy).toHaveBeenCalledWith('xyz-789', '123');
+    expect(versionDeleteSpy).toHaveBeenCalledWith('xyz-789', '123', '456');
   });
 
   it('should delete object if object has no other remaining versions', async () => {

app/tests/unit/services/sync.spec.js

@@ -723,7 +723,7 @@ describe('syncVersions', () => {
       expect(Version.startTransaction).toHaveBeenCalledTimes(1);
       expect(createSpy).toHaveBeenCalledTimes(2);
       expect(deleteSpy).toHaveBeenCalledTimes(1);
-      expect(deleteSpy).toHaveBeenCalledWith(comsObject.id, validUuidv4, expect.any(Object));
+      expect(deleteSpy).toHaveBeenCalledWith(comsObject.id, validUuidv4, expect.any(String), expect.any(Object));
       expect(headObjectSpy).toHaveBeenCalledTimes(1);
       expect(headObjectSpy).toHaveBeenCalledWith(expect.objectContaining({
         filePath: comsObject.path,
@@ -776,11 +776,7 @@ describe('syncVersions', () => {
       expect(readSpy).toHaveBeenCalledTimes(0);
       expect(updateSpy).toHaveBeenCalledTimes(0);
       expect(updateIsLatestSpy).toHaveBeenCalledTimes(1);
-      expect(updateIsLatestSpy).toHaveBeenCalledWith(expect.objectContaining({
-        id: validUuidv4,
-        objectId: comsObject.id,
-        isLatest: true
-      }), expect.any(Object));
+      expect(updateIsLatestSpy).toHaveBeenCalledWith(validUuidv4, expect.any(Object));
       expect(versionTrx.commit).toHaveBeenCalledTimes(1);
     });
 

k6/createObject.js

@@ -27,28 +27,29 @@ export const options = {
   },
 };
 
-// create url with random tags
-const randomLetter = () => String.fromCharCode(65 + Math.floor(Math.random() * 26));
-const url = `${apiPath}/object?bucketId=${bucketId}&tagset[${randomLetter()}]=${randomLetter()}`;
-
-// create a random file name
-function randomFilename(length) {
-  let randomFilename = '';
-  let counter = 0;
-  while (counter < 6) {
-    randomFilename += randomLetter();
-    counter += 1;
-  }
-  return randomFilename + '.txt';
-}
-
 // open() the file as binary (with the 'b' argument, must be declared in init scope)
 // ref: https://k6.io/docs/examples/data-uploads/#multipart-request-uploading-a-file
 // eslint-disable-next-line
 const binFile = open(filePath, 'b');
 
 // run k6
 export default function () {
+
+  // create url with random tags
+  const randomLetter = () => String.fromCharCode(65 + Math.floor(Math.random() * 26));
+  const url = `${apiPath}/object?bucketId=${bucketId}&tagset[${randomLetter()}]=${randomLetter()}`;
+
+  // create a random file name
+  function randomFilename(length) {
+    let randomFilename = '';
+    let counter = 0;
+    while (counter < 6) {
+      randomFilename += randomLetter();
+      counter += 1;
+    }
+    return randomFilename + '.txt';
+  }
+
   const data = {
     // attach file, specify file name and content type
     file: http.file(binFile, randomFilename(), 'text/plain')