Skip duplicate ids (ericyd#104)

CN-Mika · May 11, 2019 · a3005aa · a3005aa
1 parent e42248e
commit a3005aa
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 2 deletions.
diff --git a/lib/FeatureFlag.ts b/lib/FeatureFlag.ts
@@ -0,0 +1,5 @@
+const FeatureFlag = {
+  SKIP_DUPLICATE_ID: false
+}
+
+export default FeatureFlag;
diff --git a/lib/FileService.ts b/lib/FileService.ts
@@ -11,6 +11,7 @@ import API from './API';
 import MimeType from './MimeType';
 import Constants from './Constants';
 import ErrorMessages from './ErrorMessages';
+import FeatureFlag from './FeatureFlag';
 import Logging from './util/Logging';
 
 export default class FileService {
@@ -221,9 +222,23 @@ export default class FileService {
         continue;
       }
 
+      if (FeatureFlag.SKIP_DUPLICATE_ID) {
+        // if item has already been completed, skip to avoid infinite loop bugs
+        if (this.properties.completed[item.id]) {
+          continue;
+        }
+      }
+
       // Copy each (files and folders are both represented the same in Google Drive)
       try {
         var newfile = this.copyFile(item);
+
+        if (FeatureFlag.SKIP_DUPLICATE_ID) {
+          // record that this file has been processed
+          this.properties.completed[item.id] = true;
+        }
+
+        // log the new file as successful
         Logging.logCopySuccess(ss, newfile, this.properties.timeZone);
       } catch (e) {
         this.properties.retryQueue.unshift({

diff --git a/lib/Properties.ts b/lib/Properties.ts
@@ -27,6 +27,7 @@ export default class Properties {
   totalRuntime: number;
   pageToken?: string;
   isOverMaxRuntime?: boolean;
+  completed: object;
 
   constructor(gDriveService: GDriveService) {
     this.gDriveService = gDriveService;
@@ -46,6 +47,7 @@ export default class Properties {
     this.remaining = [];
     this.timeZone = 'GMT-7';
     this.totalRuntime = 0;
+    this.completed = {};
 
     return this;
   }

diff --git a/test/FileService.test.js b/test/FileService.test.js
@@ -4,6 +4,7 @@ import GDriveService from '../lib/GDriveService';
 import Timer from '../lib/Timer';
 import Properties from '../lib/Properties';
 import Constants from '../lib/Constants';
+import FeatureFlag from '../lib/FeatureFlag';
 import Logging from '../lib/util/Logging';
 const PropertiesService = require('./mocks/PropertiesService');
 const assert = require('assert');
@@ -685,7 +686,7 @@ describe('FileService', function() {
       const stubLog = sinon.stub(Logging, 'logCopySuccess');
 
       // run actual
-      const items = [1, 2, 3];
+      const items = [{ id: 1 }, { id: 2 }, { id: 3 }];
       const itemsLength = items.length; // must set here because items is mutated in processFileList
       this.fileService.processFileList(items, this.userProperties, {
         spreadsheetStub: true
@@ -696,7 +697,7 @@ describe('FileService', function() {
       assert.equal(
         stubLog.callCount,
         itemsLength,
-        'Logging.logCopySuccess not called once'
+        `Logging.logCopySuccess not called ${itemsLength} times`
       );
       assert.equal(
         stubLog.getCall(0).args[0].spreadsheetStub,
@@ -715,6 +716,34 @@ describe('FileService', function() {
       stubCopy.restore();
       stubLog.restore();
     });
+
+    [
+      [true, 'not ', 3],
+      [false, '', 4],
+    ].forEach(([featureFlag, not, numberCopies]) => {
+      describe(`when FeatureFlag.SKIP_DUPLICATE_ID is ${featureFlag}`, function() {
+        it(`should ${not}copy the same ID twice`, function() {
+          // set up mocks
+          FeatureFlag.SKIP_DUPLICATE_ID = featureFlag
+          const stubCopy = sinon
+            .stub(this.fileService, 'copyFile')
+            .returns(this.mockFile);
+          Util.logCopySuccess = sinon.stub();
+
+          // run actual
+          const items = [{ id: 1 }, { id: 2 }, { id: 1 }, { id: 3 }];
+          this.fileService.processFileList(items, this.userProperties, {
+            spreadsheetStub: true
+          });
+
+          // assertions
+          assert.equal(stubCopy.callCount, numberCopies, `stubCopy not called ${numberCopies} times`);
+
+          // restore mocks
+          stubCopy.restore();
+        })
+      })
+    })
   });
 
   describe('isDescendant()', function() {

diff --git a/test/Properties.test.js b/test/Properties.test.js
@@ -15,6 +15,7 @@ describe('Properties', function() {
       .readFileSync('test/mocks/properties_document_stringified.txt')
       .toString();
   });
+
   describe('load()', function() {
     it('should assign properties to `this`', function() {
       // set up mocks
@@ -34,6 +35,7 @@ describe('Properties', function() {
       // reset mocks
       stubFile.restore();
     });
+
     it('should return parsing error if not JSON-parsable', function() {
       // set up mocks
       const stubFile = sinon.stub(this.gDriveService, 'downloadFile');
@@ -51,6 +53,7 @@ describe('Properties', function() {
       // reset mocks
       stubFile.restore();
     });
+
     it('should return human readable error if propertiesDocID is undefined', function() {
       // set up mocks
       const stubFile = sinon.stub(this.gDriveService, 'downloadFile');
@@ -69,6 +72,7 @@ describe('Properties', function() {
       stubFile.restore();
     });
   });
+
   describe('save()', function() {
     it('should throw critical error if properties cannot be serialized', function() {
       function Circular() {
@@ -84,6 +88,7 @@ describe('Properties', function() {
         'Failed to serialize script properties. This is a critical failure. Please start your copy again.'
       );
     });
+
     it('should update file with stringified props', function() {
       // set up mocks
       const stubUpdate = sinon.stub(this.gDriveService, 'updateFile');
@@ -111,6 +116,7 @@ describe('Properties', function() {
       );
     });
   });
+
   it('should increment totalRuntime', function() {
     this.properties.incrementTotalRuntime(50);
     assert.equal(
@@ -119,6 +125,7 @@ describe('Properties', function() {
       'totalRuntime not incremented properly'
     );
   });
+
   it('should determine if max runtime is exceeded', function() {
     assert(
       !this.properties.checkMaxRuntime(),

diff --git a/test/mocks/properties_document_stringified.txt b/test/mocks/properties_document_stringified.txt
@@ -12,6 +12,7 @@
 	"timeZone": "GMT-7",
 	"retryQueue": [],
 	"totalRuntime": 0,
+	"completed": {},
 	"leftovers": {
 		"kind": "drive#fileList",
 		"selfLink": "https://www.googleapis.com/drive/v2/files?maxResults=1000&q=%220BylyXjPAb3M1NWNjUjBhZzhJOUE%22+in+parents+and+trashed+%3D+false&alt=json",