fix: avoid deleting excluded objects (#50)

jeanbmar · May 25, 2023 · 8c597d9 · 8c597d9
1 parent a8abc1d
commit 8c597d9
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,16 @@
 
 All notable changes to this project will be documented in this file.
 
+## [4.2.0]
+
+### Bug Fixes
+
+* prevent deleting excluded files by default to match aws cli behavior
+
+### Features
+
+* add a `deleteExcluded` option to delete excluded files
+
 ## [4.1.0]
 
 ### Features

diff --git a/README.md b/README.md
@@ -220,6 +220,7 @@ Similar to AWS CLI ``aws s3 sync localDir bucketPrefix [options]``.
 - `options` *<SyncBucketWithLocalOptions\>*
   - `dryRun` *<boolean\>* Equivalent to CLI ``--dryrun`` option
   - `del` *<boolean\>* Equivalent to CLI ``--delete`` option
+  - `deleteExcluded` *<boolean\>* Delete **excluded** target objects even if they match a source object. Ignored if `del` is false. See [this](https://github.com/aws/aws-cli/issues/4923) CLI issue.
   - `sizeOnly` *<boolean\>* Equivalent to CLI ``--size-only`` option
   - `relocations` *<Relocation[]\>* Allows uploading objects to remote folders without mirroring the source directory structure. Each relocation is as a callback taking a string posix path param and returning a relocated string posix path.
   - `filters` *<Filter[]\>* [Almost](https://github.com/jeanbmar/s3-sync-client/issues/30) equivalent to CLI ``--exclude`` and ``--include`` options. Filters can be specified using plain objects including either an `include` or `exclude` property. The `include` and `exclude` properties are functions that take an object key and return a boolean.
@@ -243,6 +244,7 @@ Similar to AWS CLI ``aws s3 sync bucketPrefix localDir [options]``.
 - `options` *<SyncLocalWithBucketOptions\>*
   - `dryRun` *<boolean\>* Equivalent to CLI ``--dryrun`` option
   - `del` *<boolean\>* Equivalent to CLI ``--delete`` option
+  - `deleteExcluded` *<boolean\>* Delete **excluded** target objects even if they match a source object. Ignored if `del` is false. See [this](https://github.com/aws/aws-cli/issues/4923) CLI issue.
   - `sizeOnly` *<boolean\>* Equivalent to CLI ``--size-only`` option
   - `relocations` *<Relocation[]\>* Allows downloading objects to local directories without mirroring the source folder structure. Each relocation is as a callback taking a string posix path param and returning a relocated string posix path.
   - `filters` *<Filter[]\>* [Almost](https://github.com/jeanbmar/s3-sync-client/issues/30) equivalent to CLI ``--exclude`` and ``--include`` options. Filters can be specified using plain objects including either an `include` or `exclude` property. The `include` and `exclude` properties are functions that take an object key and return a boolean.
@@ -265,6 +267,7 @@ Similar to AWS CLI ``aws s3 sync sourceBucketPrefix targetBucketPrefix [options]
 - `options` *<SyncBucketWithBucketOptions\>*
   - `dryRun` *<boolean\>* Equivalent to CLI ``--dryrun`` option
   - `del` *<boolean\>* Equivalent to CLI ``--delete`` option
+  - `deleteExcluded` *<boolean\>* Delete **excluded** target objects even if they match a source object. Ignored if `del` is false. See [this](https://github.com/aws/aws-cli/issues/4923) CLI issue.
   - `sizeOnly` *<boolean\>* Equivalent to CLI ``--size-only`` option
   - `relocations` *<Relocation[]\>* Allows copying objects to remote folders without mirroring the source folder structure. Each relocation is as a callback taking a string posix path param and returning a relocated string posix path.
   - `filters` *<Filter[]\>* [Almost](https://github.com/jeanbmar/s3-sync-client/issues/30) equivalent to CLI ``--exclude`` and ``--include`` options. Filters can be specified using plain objects including either an `include` or `exclude` property. The `include` and `exclude` properties are functions that take an object key and return a boolean.

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "s3-sync-client",
-  "version": "4.1.2",
+  "version": "4.2.0",
   "description": "AWS CLI s3 sync for Node.js provides a modern client to perform S3 sync operations between file systems and S3 buckets in the spirit of the official AWS CLI command",
   "keywords": [
     "aws",

diff --git a/src/commands/SyncBucketWithBucketCommand.ts b/src/commands/SyncBucketWithBucketCommand.ts
@@ -15,6 +15,7 @@ export type SyncBucketWithBucketCommandInput = {
   targetBucketPrefix: string;
   dryRun?: boolean;
   del?: boolean;
+  deleteExcluded?: boolean;
   sizeOnly?: boolean;
   relocations?: Relocation[];
   filters?: Filter[];
@@ -35,6 +36,7 @@ export class SyncBucketWithBucketCommand {
   targetBucketPrefix: string;
   dryRun: boolean;
   del: boolean;
+  deleteExcluded: boolean;
   sizeOnly: boolean;
   relocations: Relocation[];
   filters: Filter[];
@@ -48,6 +50,7 @@ export class SyncBucketWithBucketCommand {
     this.targetBucketPrefix = input.targetBucketPrefix;
     this.dryRun = input.dryRun ?? false;
     this.del = input.del ?? false;
+    this.deleteExcluded = input.deleteExcluded ?? false;
     this.sizeOnly = input.sizeOnly ?? false;
     this.relocations = input.relocations ?? [];
     this.filters = input.filters ?? [];
@@ -88,20 +91,14 @@ export class SyncBucketWithBucketCommand {
             : currentPath,
         ...this.relocations,
       ];
-    sourceObjects.forEach((sourceObject) =>
-      sourceObject.applyFilters(this.filters)
-    );
-    const includedSourceObjects = sourceObjects.filter(
-      (sourceObject) => sourceObject.isIncluded
-    );
-    includedSourceObjects.forEach((sourceObject) =>
-      sourceObject.applyRelocations(this.relocations)
-    );
-    const diff = SyncObject.diff(
-      includedSourceObjects,
-      targetObjects,
-      this.sizeOnly
-    );
+    sourceObjects.forEach((sourceObject) => {
+      sourceObject.applyFilters(this.filters);
+      sourceObject.applyRelocations(this.relocations);
+    });
+    const diff = SyncObject.diff(sourceObjects, targetObjects, {
+      sizeOnly: this.sizeOnly,
+      deleteExcluded: this.deleteExcluded,
+    });
     const commands = [];
     if (!this.dryRun) {
       commands.push(

diff --git a/src/commands/SyncBucketWithLocalCommand.ts b/src/commands/SyncBucketWithLocalCommand.ts
@@ -24,6 +24,7 @@ export type SyncBucketWithLocalCommandInput = {
   bucketPrefix: string;
   dryRun?: boolean;
   del?: boolean;
+  deleteExcluded?: boolean;
   sizeOnly?: boolean;
   relocations?: Relocation[];
   filters?: Filter[];
@@ -47,6 +48,7 @@ export class SyncBucketWithLocalCommand {
   bucketPrefix: string;
   dryRun: boolean;
   del: boolean;
+  deleteExcluded: boolean;
   sizeOnly: boolean;
   relocations: Relocation[];
   filters: Filter[];
@@ -63,6 +65,7 @@ export class SyncBucketWithLocalCommand {
     this.bucketPrefix = input.bucketPrefix;
     this.dryRun = input.dryRun ?? false;
     this.del = input.del ?? false;
+    this.deleteExcluded = input.deleteExcluded ?? false;
     this.sizeOnly = input.sizeOnly ?? false;
     this.relocations = input.relocations ?? [];
     this.filters = input.filters ?? [];
@@ -85,20 +88,14 @@ export class SyncBucketWithLocalCommand {
         (currentPath) => `${prefix}/${currentPath}`,
         ...this.relocations,
       ];
-    sourceObjects.forEach((sourceObject) =>
-      sourceObject.applyFilters(this.filters)
-    );
-    const includedSourceObjects = sourceObjects.filter(
-      (sourceObject) => sourceObject.isIncluded
-    );
-    includedSourceObjects.forEach((sourceObject) =>
-      sourceObject.applyRelocations(this.relocations)
-    );
-    const diff = SyncObject.diff(
-      includedSourceObjects,
-      targetObjects,
-      this.sizeOnly
-    );
+    sourceObjects.forEach((sourceObject) => {
+      sourceObject.applyFilters(this.filters);
+      sourceObject.applyRelocations(this.relocations);
+    });
+    const diff = SyncObject.diff(sourceObjects, targetObjects, {
+      sizeOnly: this.sizeOnly,
+      deleteExcluded: this.deleteExcluded,
+    });
     const commands = [];
     if (!this.dryRun) {
       commands.push(

diff --git a/src/commands/SyncLocalWithBucketCommand.ts b/src/commands/SyncLocalWithBucketCommand.ts
@@ -17,6 +17,7 @@ export type SyncLocalWithBucketCommandInput = {
   localDir: string;
   dryRun?: boolean;
   del?: boolean;
+  deleteExcluded?: boolean;
   sizeOnly?: boolean;
   relocations?: Relocation[];
   filters?: Filter[];
@@ -37,6 +38,7 @@ export class SyncLocalWithBucketCommand {
   localDir: string;
   dryRun: boolean;
   del: boolean;
+  deleteExcluded: boolean;
   sizeOnly: boolean;
   relocations: Relocation[];
   filters: Filter[];
@@ -50,6 +52,7 @@ export class SyncLocalWithBucketCommand {
     this.localDir = input.localDir;
     this.dryRun = input.dryRun ?? false;
     this.del = input.del ?? false;
+    this.deleteExcluded = input.deleteExcluded ?? false;
     this.sizeOnly = input.sizeOnly ?? false;
     this.relocations = input.relocations ?? [];
     this.filters = input.filters ?? [];
@@ -75,20 +78,14 @@ export class SyncLocalWithBucketCommand {
             : currentPath,
         ...this.relocations,
       ];
-    sourceObjects.forEach((sourceObject) =>
-      sourceObject.applyFilters(this.filters)
-    );
-    const includedSourceObjects = sourceObjects.filter(
-      (sourceObject) => sourceObject.isIncluded
-    );
-    includedSourceObjects.forEach((sourceObject) => {
+    sourceObjects.forEach((sourceObject) => {
+      sourceObject.applyFilters(this.filters);
       sourceObject.applyRelocations(this.relocations);
     });
-    const diff = SyncObject.diff(
-      includedSourceObjects,
-      targetObjects,
-      this.sizeOnly
-    );
+    const diff = SyncObject.diff(sourceObjects, targetObjects, {
+      sizeOnly: this.sizeOnly,
+      deleteExcluded: this.deleteExcluded,
+    });
     const commands = [];
     if (!this.dryRun) {
       commands.push(

diff --git a/src/fs/SyncObject.ts b/src/fs/SyncObject.ts
@@ -12,6 +12,11 @@ export type Diff = {
   deleted: SyncObject[];
 };
 
+export type DiffOptions = {
+  sizeOnly?: boolean;
+  deleteExcluded?: boolean;
+};
+
 export abstract class SyncObject {
   id: string;
   size: number;
@@ -27,7 +32,7 @@ export abstract class SyncObject {
   static diff(
     sourceObjects: SyncObject[],
     targetObjects: SyncObject[],
-    sizeOnly: boolean = false
+    options?: DiffOptions
   ): Diff {
     const sourceObjectMap = new Map(
       sourceObjects.map((sourceObject) => [sourceObject.id, sourceObject])
@@ -38,19 +43,25 @@ export abstract class SyncObject {
     const created = [];
     const updated = [];
     sourceObjectMap.forEach((sourceObject) => {
+      if (!sourceObject.isIncluded) return;
       const targetObject = targetObjectMap.get(sourceObject.id);
       if (targetObject === undefined) {
         created.push(sourceObject);
       } else if (
         sourceObject.size !== targetObject.size ||
-        (!sizeOnly && sourceObject.lastModified > targetObject.lastModified)
+        (options?.sizeOnly !== true &&
+          sourceObject.lastModified > targetObject.lastModified)
       ) {
         updated.push(sourceObject);
       }
     });
     const deleted = [];
     targetObjectMap.forEach((targetObject) => {
-      if (!sourceObjectMap.has(targetObject.id)) {
+      const sourceObject = sourceObjectMap.get(targetObject.id);
+      if (
+        sourceObject === undefined ||
+        (!sourceObject.isIncluded && options?.deleteExcluded === true)
+      ) {
         deleted.push(targetObject);
       }
     });

diff --git a/test/S3SyncClient.test.ts b/test/S3SyncClient.test.ts
@@ -248,6 +248,42 @@ test('s3 sync client', async (t) => {
       assert(hasObject(objects, 'xmoj') === true);
     });
 
+    // https://github.com/jeanbmar/s3-sync-client/issues/30
+    await b.test('does not delete excluded files', async () => {
+      await syncClient.send(
+        new SyncBucketWithLocalCommand({
+          localDir: path.join(DATA_DIR, 'def/jkl'),
+          bucketPrefix: BUCKET,
+          del: true,
+          filters: [{ exclude: (key) => key.startsWith('xmot') }],
+        })
+      );
+      const objects = await syncClient.send(
+        new ListBucketObjectsCommand({
+          bucket: BUCKET,
+        })
+      );
+      assert(hasObject(objects, 'xmot') === true);
+    });
+
+    await b.test('deletes excluded files', async () => {
+      await syncClient.send(
+        new SyncBucketWithLocalCommand({
+          localDir: path.join(DATA_DIR, 'def/jkl'),
+          bucketPrefix: BUCKET,
+          del: true,
+          deleteExcluded: true,
+          filters: [{ exclude: (key) => key.startsWith('xmot') }],
+        })
+      );
+      const objects = await syncClient.send(
+        new ListBucketObjectsCommand({
+          bucket: BUCKET,
+        })
+      );
+      assert(hasObject(objects, 'xmot') === false);
+    });
+
     await b.test(
       'syncs a single dir with a bucket using relocation',
       async () => {
@@ -588,43 +624,44 @@ test('s3 sync client', async (t) => {
 
   await t.test('diffs sync objects', async (d) => {
     const bucketObjects = [
-      { id: 'abc/created', lastModified: 0, size: 1 },
-      { id: 'abc/updated1', lastModified: 1, size: 1 },
-      { id: 'abc/updated2', lastModified: 0, size: 2 },
-      { id: 'abc/unchanged', lastModified: 0, size: 1 },
+      { id: 'abc/created', lastModified: 0, size: 1, isIncluded: true },
+      { id: 'abc/updated1', lastModified: 1, size: 1, isIncluded: true },
+      { id: 'abc/updated2', lastModified: 0, size: 2, isIncluded: true },
+      { id: 'abc/unchanged', lastModified: 0, size: 1, isIncluded: true },
     ] as BucketObject[];
     const localObjects = [
-      { id: 'abc/unchanged', lastModified: 0, size: 1 },
-      { id: 'abc/updated1', lastModified: 0, size: 1 },
-      { id: 'abc/updated2', lastModified: 0, size: 1 },
-      { id: 'deleted', lastModified: 0, size: 1 },
+      { id: 'abc/unchanged', lastModified: 0, size: 1, isIncluded: true },
+      { id: 'abc/updated1', lastModified: 0, size: 1, isIncluded: true },
+      { id: 'abc/updated2', lastModified: 0, size: 1, isIncluded: true },
+      { id: 'deleted', lastModified: 0, size: 1, isIncluded: true },
     ] as LocalObject[];
 
     await d.test('computes sync operations on objects', () => {
       const diff = SyncObject.diff(bucketObjects, localObjects);
       assert.deepStrictEqual(diff.created, [
-        { id: 'abc/created', size: 1, lastModified: 0 },
+        { id: 'abc/created', size: 1, lastModified: 0, isIncluded: true },
       ]);
       assert.deepStrictEqual(diff.updated, [
-        { id: 'abc/updated1', size: 1, lastModified: 1 },
-        { id: 'abc/updated2', size: 2, lastModified: 0 },
+        { id: 'abc/updated1', size: 1, lastModified: 1, isIncluded: true },
+        { id: 'abc/updated2', size: 2, lastModified: 0, isIncluded: true },
       ]);
       assert.deepStrictEqual(diff.deleted, [
-        { id: 'deleted', size: 1, lastModified: 0 },
+        { id: 'deleted', size: 1, lastModified: 0, isIncluded: true },
       ]);
     });
 
     await d.test('computes sync sizeOnly operations on objects', () => {
-      const sizeOnly = true;
-      const diff = SyncObject.diff(bucketObjects, localObjects, sizeOnly);
+      const diff = SyncObject.diff(bucketObjects, localObjects, {
+        sizeOnly: true,
+      });
       assert.deepStrictEqual(diff.created, [
-        { id: 'abc/created', size: 1, lastModified: 0 },
+        { id: 'abc/created', size: 1, lastModified: 0, isIncluded: true },
       ]);
       assert.deepStrictEqual(diff.updated, [
-        { id: 'abc/updated2', size: 2, lastModified: 0 },
+        { id: 'abc/updated2', size: 2, lastModified: 0, isIncluded: true },
       ]);
       assert.deepStrictEqual(diff.deleted, [
-        { id: 'deleted', size: 1, lastModified: 0 },
+        { id: 'deleted', size: 1, lastModified: 0, isIncluded: true },
       ]);
     });
   });
@@ -671,6 +708,7 @@ test('s3 sync client', async (t) => {
               { include: (key) => key.startsWith('def/jkl') },
             ],
             del: true,
+            deleteExcluded: true,
           })
         );
         const objects = await syncClient.send(