Fixes Sync bug

ListALlVersrions can return all objects below a path
Modified isBelowPrefix util function

Author: TimCsaky

app/src/components/utils.js

@@ -407,7 +407,8 @@ const utils = {
     if (typeof prefix !== 'string' || typeof path !== 'string') return false;
     else if (path === prefix) return false;
     else if (prefix === DELIMITER) return true;
-    else if (path.startsWith(prefix)) return true;
+    else if (path.startsWith(prefix + DELIMITER)) return true;
+    else if (prefix === '') return true;
     else return false;
   },
 

app/src/services/storage.js

@@ -263,47 +263,16 @@ const objectStorageService = {
     return this._getS3Client(data).send(new HeadObjectCommand(params));
   },
 
-  /**
-   * @function listAllObjects
-   * Lists all objects in the bucket with the prefix of `filePath`.
-   * Performs pagination behind the scenes if required.
-   * @param {string} [options.filePath=undefined] Optional filePath of the objects
-   * @param {string} [options.bucketId=undefined] Optional bucketId
-   * @param {boolean} [options.precisePath=true] Optional boolean for filtering results based on the precise path
-   * @returns {Promise<object[]>} An array of objects matching the criteria
-   */
-  async listAllObjects({ filePath = undefined, bucketId = undefined, precisePath = true } = {}) {
-    const key = filePath ?? (await utils.getBucket(bucketId)).key;
-    const path = key !== DELIMITER ? key : '';
-
-    const objects = [];
-
-    let incomplete = false;
-    let nextToken = undefined;
-    do {
-      const { Contents, IsTruncated, NextContinuationToken } = await this.listObjectsV2({
-        filePath: path,
-        continuationToken: nextToken,
-        bucketId: bucketId
-      });
-
-      if (Contents) objects.push(
-        ...Contents.filter(object => !precisePath || utils.isAtPath(path, object.Key))
-      );
-      incomplete = IsTruncated;
-      nextToken = NextContinuationToken;
-    } while (incomplete);
-
-    return Promise.resolve(objects);
-  },
-
   /**
    * @function listAllObjectVersions
    * Lists all objects in the bucket with the prefix of `filePath`.
    * Performs pagination behind the scenes if required.
    * @param {string} [options.filePath=undefined] Optional filePath of the objects
    * @param {string} [options.bucketId=undefined] Optional bucketId
-   * @param {boolean} [options.precisePath=true] Optional boolean for filtering results based on the precise path
+   * @param {boolean} [options.precisePath=true] Optional boolean for filtering results based on the precise path. 
+   * If true, only objects that exist directly at the filepath will be included in the results,
+   * If false, all objects nested in 'subfolders' below the filepath will be included
+   * match the precise path rather than below the prefix
    * @param {boolean} [options.filterLatest=false] Optional boolean for filtering results to only entries
    * with IsLatest being true
    * @returns {Promise<object>} An object containg an array of DeleteMarkers and Versions
@@ -328,12 +297,18 @@ const objectStorageService = {
 
       if (DeleteMarkers) deleteMarkers.push(
         ...DeleteMarkers
-          .filter(object => !precisePath || utils.isAtPath(path, object.Key))
+          .filter(object => {
+            if (precisePath) return utils.isAtPath(path, object.Key);
+            else return utils.isBelowPrefix(path, object.Key);
+          })
           .filter(object => !filterLatest || object.IsLatest === true)
       );
       if (Versions) versions.push(
         ...Versions
-          .filter(object => !precisePath || utils.isAtPath(path, object.Key))
+          .filter(object => {
+            if (precisePath) return utils.isAtPath(path, object.Key);
+            else return utils.isBelowPrefix(path, object.Key);
+          })
           .filter(object => !filterLatest || object.IsLatest === true)
       );
       incomplete = IsTruncated;
@@ -488,7 +463,7 @@ const objectStorageService = {
 
       // list all objects at and below provided path (key)
       if (recursive) {
-        const s3Response = await this.listAllObjectVersions({ filePath: key, bucketId: bucketId, precisePath: false });
+        const s3Response = await this.listAllObjectVersions({ filePath: key, bucketId: bucketId });
         log.info(`Found ${s3Response.Versions.length} object versions and ${s3Response.DeleteMarkers.length} 
         delete markers in S3 for bucketId ${data.bucketId}`, { function: 'syncBucketRecursive' });
         const s3Keys = [...new Set([

app/tests/unit/components/utils.spec.js

@@ -466,6 +466,11 @@ describe('isBelowPrefix', () => {
     [false, 'a/b', 'a/c'],
     [false, 'a/b/c', 'a/c/b'],
     [true, 'a/b/c', 'a/b/c/d/e/f'],
+    [true, 'a/b/c', 'a/b/c/d/e/f/g.jpg'],
+    [true, 'a/b/c', 'a/b/c/d/e/f/g hij.jpg'],
+    [false, 'a', 'abcd.jpeg'],
+    [true, 'a', 'a/bcd.jpeg'],
+    [true, '', 'abcd.jpeg'],
   ])('should return %j given prefix %j and path %j', (expected, prefix, path) => {
     expect(utils.isBelowPrefix(prefix, path)).toEqual(expected);
   });

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

@@ -509,154 +509,6 @@ describe('headObject', () => {
   });
 });
 
-describe('listAllObjects', () => {
-  const listObjectsV2Mock = jest.spyOn(service, 'listObjectsV2');
-
-  beforeEach(() => {
-    listObjectsV2Mock.mockReset();
-  });
-
-  afterAll(() => {
-    listObjectsV2Mock.mockRestore();
-  });
-
-  it('should call listObjectsV2 at least once and return an empty array', async () => {
-    listObjectsV2Mock.mockResolvedValue({ IsTruncated: false });
-
-    const result = await service.listAllObjects();
-
-    expect(result).toBeTruthy();
-    expect(Array.isArray(result)).toBeTruthy();
-    expect(result).toHaveLength(0);
-    expect(utils.getBucket).toHaveBeenCalledTimes(1);
-    expect(utils.isAtPath).toHaveBeenCalledTimes(0);
-    expect(listObjectsV2Mock).toHaveBeenCalledTimes(1);
-    expect(listObjectsV2Mock).toHaveBeenCalledWith(expect.objectContaining({
-      filePath: key
-    }));
-  });
-
-  it('should call listObjectsV2 at least once and return an empty array of objects', async () => {
-    listObjectsV2Mock.mockResolvedValue({ Contents: [], IsTruncated: false });
-
-    const result = await service.listAllObjects();
-
-    expect(result).toBeTruthy();
-    expect(Array.isArray(result)).toBeTruthy();
-    expect(result).toHaveLength(0);
-    expect(utils.getBucket).toHaveBeenCalledTimes(1);
-    expect(utils.isAtPath).toHaveBeenCalledTimes(0);
-    expect(listObjectsV2Mock).toHaveBeenCalledTimes(1);
-    expect(listObjectsV2Mock).toHaveBeenCalledWith(expect.objectContaining({
-      filePath: key
-    }));
-  });
-
-  it('should call listObjectsV2 multiple times and return an array of precise path objects', async () => {
-    const continueToken = 'token';
-    listObjectsV2Mock.mockResolvedValueOnce({
-      Contents: [{ Key: 'filePath/foo' }],
-      IsTruncated: true,
-      NextContinuationToken: continueToken
-    });
-    listObjectsV2Mock.mockResolvedValueOnce({
-      Contents: [{ Key: 'filePath/bar' }],
-      IsTruncated: false
-    });
-
-    const result = await service.listAllObjects();
-
-    expect(result).toBeTruthy();
-    expect(Array.isArray(result)).toBeTruthy();
-    expect(result).toHaveLength(2);
-    expect(result).toEqual(expect.arrayContaining([
-      { Key: 'filePath/foo' },
-      { Key: 'filePath/bar' }
-    ]));
-    expect(utils.getBucket).toHaveBeenCalledTimes(1);
-    expect(utils.isAtPath).toHaveBeenCalledTimes(2);
-    expect(listObjectsV2Mock).toHaveBeenCalledTimes(2);
-    expect(listObjectsV2Mock).toHaveBeenNthCalledWith(1, expect.objectContaining({
-      filePath: key
-    }));
-    expect(listObjectsV2Mock).toHaveBeenNthCalledWith(2, expect.objectContaining({
-      filePath: key,
-      continuationToken: continueToken
-    }));
-  });
-
-  it('should call listObjectsV2 multiple times and return an array of all path objects', async () => {
-    const continueToken = 'token';
-    listObjectsV2Mock.mockResolvedValueOnce({
-      Contents: [{ Key: 'filePath/test/foo' }],
-      IsTruncated: true,
-      NextContinuationToken: continueToken
-    });
-    listObjectsV2Mock.mockResolvedValueOnce({
-      Contents: [{ Key: 'filePath/test/bar' }],
-      IsTruncated: false
-    });
-
-    const result = await service.listAllObjects({ precisePath: false });
-
-    expect(result).toBeTruthy();
-    expect(Array.isArray(result)).toBeTruthy();
-    expect(result).toHaveLength(2);
-    expect(result).toEqual(expect.arrayContaining([
-      { Key: 'filePath/test/foo' },
-      { Key: 'filePath/test/bar' }
-    ]));
-    expect(utils.getBucket).toHaveBeenCalledTimes(1);
-    expect(utils.isAtPath).toHaveBeenCalledTimes(0);
-    expect(listObjectsV2Mock).toHaveBeenCalledTimes(2);
-    expect(listObjectsV2Mock).toHaveBeenNthCalledWith(1, expect.objectContaining({
-      filePath: key
-    }));
-    expect(listObjectsV2Mock).toHaveBeenNthCalledWith(2, expect.objectContaining({
-      filePath: key,
-      continuationToken: continueToken
-    }));
-  });
-
-  it(
-    'should call listObjectsV2 multiple times with the right bucketId and filePath, returning an array of objects',
-    async () => {
-      const continueToken = 'token';
-      const customPath = 'filePath/test';
-      listObjectsV2Mock.mockResolvedValueOnce({
-        Contents: [{ Key: 'filePath/test/foo' }],
-        IsTruncated: true,
-        NextContinuationToken: continueToken
-      });
-      listObjectsV2Mock.mockResolvedValueOnce({
-        Contents: [{ Key: 'filePath/test/bar' }],
-        IsTruncated: false
-      });
-
-      const result = await service.listAllObjects({ filePath: customPath, bucketId: bucket });
-
-      expect(result).toBeTruthy();
-      expect(Array.isArray(result)).toBeTruthy();
-      expect(result).toHaveLength(2);
-      expect(result).toEqual(expect.arrayContaining([
-        { Key: 'filePath/test/foo' },
-        { Key: 'filePath/test/bar' }
-      ]));
-      expect(utils.getBucket).toHaveBeenCalledTimes(0);
-      expect(utils.isAtPath).toHaveBeenCalledTimes(2);
-      expect(listObjectsV2Mock).toHaveBeenCalledTimes(2);
-      expect(listObjectsV2Mock).toHaveBeenNthCalledWith(1, expect.objectContaining({
-        filePath: customPath,
-        bucketId: bucket
-      }));
-      expect(listObjectsV2Mock).toHaveBeenNthCalledWith(2, expect.objectContaining({
-        filePath: customPath,
-        continuationToken: continueToken,
-        bucketId: bucket
-      }));
-    }
-  );
-});
 
 describe('listAllObjectVersions', () => {
   const listObjectVersionMock = jest.spyOn(service, 'listObjectVersion');