From 68a27debd1e64f2dc21288fc5d43181bb61cd8f1 Mon Sep 17 00:00:00 2001 From: Tomasz Grobelny Date: Mon, 29 Oct 2018 21:58:43 +0100 Subject: Fix file move operation for large number of files Signed-off-by: Tomasz Grobelny --- apps/files/js/filelist.js | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 9cc1856d52b..31d80bbb246 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -2162,7 +2162,31 @@ if (!_.isArray(fileNames)) { fileNames = [fileNames]; } - _.each(fileNames, function(fileName) { + + function Semaphore(max) { + var counter = 0; + var waiting = []; + + this.acquire = function() { + if(counter < max) { + counter++; + return new Promise(function(resolve) { resolve(); }); + } else { + return new Promise(function(resolve) { waiting.push(resolve); }); + } + }; + + this.release = function() { + counter--; + if (waiting.length > 0 && counter < max) { + counter++; + var promise = waiting.shift(); + promise(); + } + }; + } + + var moveFileFunction = function(fileName) { var $tr = self.findFileEl(fileName); self.showFileBusyState($tr, true); if (targetPath.charAt(targetPath.length - 1) !== '/') { @@ -2170,7 +2194,7 @@ // not overwrite it targetPath = targetPath + '/'; } - self.filesClient.move(dir + fileName, targetPath + fileName) + return self.filesClient.move(dir + fileName, targetPath + fileName) .done(function() { // if still viewing the same directory if (OC.joinPaths(self.getCurrentDirectory(), '/') === dir) { @@ -2201,11 +2225,22 @@ .always(function() { self.showFileBusyState($tr, false); }); - if (callback) { - callback(); - } + }; + + var mcSemaphore = new Semaphore(10); + var counter = 0; + _.each(fileNames, function(arg) { + mcSemaphore.acquire().then(function(){ + moveFileFunction(arg).then(function(){ + mcSemaphore.release(); + counter++; + }); + }); }); + if (callback) { + callback(); + } }, /** -- cgit v1.2.3 From 41687ef00fa5667467564770a4e29403e32d167f Mon Sep 17 00:00:00 2001 From: Tomasz Grobelny Date: Sat, 3 Nov 2018 23:01:32 +0000 Subject: Fix move related tests Signed-off-by: Tomasz Grobelny --- apps/files/js/filelist.js | 16 +++-- apps/files/tests/js/filelistSpec.js | 117 +++++++++++++++++++----------------- 2 files changed, 73 insertions(+), 60 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 31d80bbb246..02dcbf7650f 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -989,7 +989,7 @@ }); } - this.move(_.pluck(files, 'name'), targetPath); + var movePromise = this.move(_.pluck(files, 'name'), targetPath); // re-enable td elements to be droppable // sometimes the filename drop handler is still called after re-enable, @@ -997,6 +997,8 @@ setTimeout(function() { self.$el.find('td.filename.ui-droppable').droppable('enable'); }, 10); + + return movePromise; }, /** @@ -2229,8 +2231,8 @@ var mcSemaphore = new Semaphore(10); var counter = 0; - _.each(fileNames, function(arg) { - mcSemaphore.acquire().then(function(){ + var promises = _.map(fileNames, function(arg) { + return mcSemaphore.acquire().then(function(){ moveFileFunction(arg).then(function(){ mcSemaphore.release(); counter++; @@ -2238,9 +2240,11 @@ }); }); - if (callback) { - callback(); - } + return Promise.all(promises).then(function(){ + if (callback) { + callback(); + } + }); }, /** diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 961761c3e86..85fc31f8c0e 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -838,84 +838,91 @@ describe('OCA.Files.FileList tests', function() { moveStub.restore(); }); - it('Moves single file to target folder', function() { - fileList.move('One.txt', '/somedir'); + it('Moves single file to target folder', function(done) { + return fileList.move('One.txt', '/somedir').then(function(){ - expect(moveStub.calledOnce).toEqual(true); - expect(moveStub.getCall(0).args[0]).toEqual('/subdir/One.txt'); - expect(moveStub.getCall(0).args[1]).toEqual('/somedir/One.txt'); + expect(moveStub.calledOnce).toEqual(true); + expect(moveStub.getCall(0).args[0]).toEqual('/subdir/One.txt'); + expect(moveStub.getCall(0).args[1]).toEqual('/somedir/One.txt'); - deferredMove.resolve(201); + deferredMove.resolve(201); - expect(fileList.findFileEl('One.txt').length).toEqual(0); + expect(fileList.findFileEl('One.txt').length).toEqual(0); - // folder size has increased - expect(fileList.findFileEl('somedir').data('size')).toEqual(262); - expect(fileList.findFileEl('somedir').find('.filesize').text()).toEqual('262 B'); + // folder size has increased + expect(fileList.findFileEl('somedir').data('size')).toEqual(262); + expect(fileList.findFileEl('somedir').find('.filesize').text()).toEqual('262 B'); - expect(notificationStub.notCalled).toEqual(true); + expect(notificationStub.notCalled).toEqual(true); + done(); + }); }); - it('Moves list of files to target folder', function() { + it('Moves list of files to target folder', function(done) { var deferredMove1 = $.Deferred(); var deferredMove2 = $.Deferred(); moveStub.onCall(0).returns(deferredMove1.promise()); moveStub.onCall(1).returns(deferredMove2.promise()); - fileList.move(['One.txt', 'Two.jpg'], '/somedir'); + return fileList.move(['One.txt', 'Two.jpg'], '/somedir').then(function(){ - expect(moveStub.calledTwice).toEqual(true); - expect(moveStub.getCall(0).args[0]).toEqual('/subdir/One.txt'); - expect(moveStub.getCall(0).args[1]).toEqual('/somedir/One.txt'); - expect(moveStub.getCall(1).args[0]).toEqual('/subdir/Two.jpg'); - expect(moveStub.getCall(1).args[1]).toEqual('/somedir/Two.jpg'); + expect(moveStub.calledTwice).toEqual(true); + expect(moveStub.getCall(0).args[0]).toEqual('/subdir/One.txt'); + expect(moveStub.getCall(0).args[1]).toEqual('/somedir/One.txt'); + expect(moveStub.getCall(1).args[0]).toEqual('/subdir/Two.jpg'); + expect(moveStub.getCall(1).args[1]).toEqual('/somedir/Two.jpg'); - deferredMove1.resolve(201); + deferredMove1.resolve(201); - expect(fileList.findFileEl('One.txt').length).toEqual(0); + expect(fileList.findFileEl('One.txt').length).toEqual(0); - // folder size has increased during move - expect(fileList.findFileEl('somedir').data('size')).toEqual(262); - expect(fileList.findFileEl('somedir').find('.filesize').text()).toEqual('262 B'); + // folder size has increased during move + expect(fileList.findFileEl('somedir').data('size')).toEqual(262); + expect(fileList.findFileEl('somedir').find('.filesize').text()).toEqual('262 B'); - deferredMove2.resolve(201); + deferredMove2.resolve(201); - expect(fileList.findFileEl('Two.jpg').length).toEqual(0); + expect(fileList.findFileEl('Two.jpg').length).toEqual(0); - // folder size has increased - expect(fileList.findFileEl('somedir').data('size')).toEqual(12311); - expect(fileList.findFileEl('somedir').find('.filesize').text()).toEqual('12 KB'); + // folder size has increased + expect(fileList.findFileEl('somedir').data('size')).toEqual(12311); + expect(fileList.findFileEl('somedir').find('.filesize').text()).toEqual('12 KB'); - expect(notificationStub.notCalled).toEqual(true); + expect(notificationStub.notCalled).toEqual(true); + done(); + }); }); - it('Shows notification if a file could not be moved', function() { - fileList.move('One.txt', '/somedir'); + it('Shows notification if a file could not be moved', function(done) { + return fileList.move('One.txt', '/somedir').then(function(){ - expect(moveStub.calledOnce).toEqual(true); + expect(moveStub.calledOnce).toEqual(true); - deferredMove.reject(409); + deferredMove.reject(409); - expect(fileList.findFileEl('One.txt').length).toEqual(1); + expect(fileList.findFileEl('One.txt').length).toEqual(1); - expect(notificationStub.calledOnce).toEqual(true); - expect(notificationStub.getCall(0).args[0]).toEqual('Could not move "One.txt"'); + expect(notificationStub.calledOnce).toEqual(true); + expect(notificationStub.getCall(0).args[0]).toEqual('Could not move "One.txt"'); + done(); + }); }); it('Restores thumbnail if a file could not be moved', function() { - fileList.move('One.txt', '/somedir'); + return fileList.move('One.txt', '/somedir').then(function(){ - expect(fileList.findFileEl('One.txt').find('.thumbnail').parent().attr('class')) - .toContain('icon-loading-small'); + expect(fileList.findFileEl('One.txt').find('.thumbnail').parent().attr('class')) + .toContain('icon-loading-small'); - expect(moveStub.calledOnce).toEqual(true); + expect(moveStub.calledOnce).toEqual(true); - deferredMove.reject(409); + deferredMove.reject(409); - expect(fileList.findFileEl('One.txt').length).toEqual(1); + expect(fileList.findFileEl('One.txt').length).toEqual(1); - expect(notificationStub.calledOnce).toEqual(true); - expect(notificationStub.getCall(0).args[0]).toEqual('Could not move "One.txt"'); + expect(notificationStub.calledOnce).toEqual(true); + expect(notificationStub.getCall(0).args[0]).toEqual('Could not move "One.txt"'); - expect(OC.TestUtil.getImageUrl(fileList.findFileEl('One.txt').find('.thumbnail'))) - .toEqual(OC.imagePath('core', 'filetypes/text.svg')); + expect(OC.TestUtil.getImageUrl(fileList.findFileEl('One.txt').find('.thumbnail'))) + .toEqual(OC.imagePath('core', 'filetypes/text.svg')); + }); }); }); @@ -1757,7 +1764,7 @@ describe('OCA.Files.FileList tests', function() { expect(changeDirStub.getCall(0).args[0]).toEqual('/subdir/two/three with space'); changeDirStub.restore(); }); - it('dropping files on breadcrumb calls move operation', function() { + it('dropping files on breadcrumb calls move operation', function(done) { var testDir = '/subdir/two/three with space/four/five'; var moveStub = sinon.stub(filesClient, 'move').returns($.Deferred().promise()); fileList.changeDirectory(testDir); @@ -1775,14 +1782,16 @@ describe('OCA.Files.FileList tests', function() { $('') ]); // simulate drop event - fileList._onDropOnBreadCrumb(new $.Event('drop', {target: $crumb}), ui); + return fileList._onDropOnBreadCrumb(new $.Event('drop', {target: $crumb}), ui).then(function(){ - expect(moveStub.callCount).toEqual(2); - expect(moveStub.getCall(0).args[0]).toEqual(testDir + '/One.txt'); - expect(moveStub.getCall(0).args[1]).toEqual('/subdir/two/three with space/One.txt'); - expect(moveStub.getCall(1).args[0]).toEqual(testDir + '/Two.jpg'); - expect(moveStub.getCall(1).args[1]).toEqual('/subdir/two/three with space/Two.jpg'); - moveStub.restore(); + expect(moveStub.callCount).toEqual(2); + expect(moveStub.getCall(0).args[0]).toEqual(testDir + '/One.txt'); + expect(moveStub.getCall(0).args[1]).toEqual('/subdir/two/three with space/One.txt'); + expect(moveStub.getCall(1).args[0]).toEqual(testDir + '/Two.jpg'); + expect(moveStub.getCall(1).args[1]).toEqual('/subdir/two/three with space/Two.jpg'); + moveStub.restore(); + done(); + }); }); it('dropping files on same dir breadcrumb does nothing', function() { var testDir = '/subdir/two/three with space/four/five'; -- cgit v1.2.3