From 0722a1a698986ae99769c0da1109020b8d84e4c8 Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 00:25:04 +0530 Subject: [PATCH 1/7] move files to new CDN --- README.md | 2 +- build.gradle | 2 +- changelog.md | 2 ++ .../fileuploader/FileUploaderService.groovy | 16 ++++++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a5e3374..77fcc28 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# File-Uploader Plugin (Latest 4.0.3) +# File-Uploader Plugin (Latest 4.0.4) [![Maintainability](https://api.codeclimate.com/v1/badges/13bfee73c29ecd2ea4b2/maintainability)](https://codeclimate.com/github/causecode/grails-file-uploader/maintainability) [![Test Coverage](https://api.codeclimate.com/v1/badges/13bfee73c29ecd2ea4b2/test_coverage)](https://codeclimate.com/github/causecode/grails-file-uploader/test_coverage) diff --git a/build.gradle b/build.gradle index 2a10607..c3088d5 100644 --- a/build.gradle +++ b/build.gradle @@ -15,7 +15,7 @@ buildscript { } } -version "4.0.3" +version "4.0.4" group "com.causecode.plugins" apply plugin: "idea" diff --git a/changelog.md b/changelog.md index e9a9207..ca45c9c 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,7 @@ # ChangeLog +## Version 4.0.4 [24-04-2019] + ## Version 4.0.3 [28-03-2019] ### Fixed diff --git a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy index 8005a2e..d35ad6b 100644 --- a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy +++ b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy @@ -414,6 +414,22 @@ class FileUploaderService { return Holders.flatConfig["fileuploader.groups.${fileGroup}.makePublic"] ? true : false } + boolean moveToNewCDN(CDNProvider toCDNProvider = CDNProvider.AMAZON, boolean makePublic = false) { + List uFileList, uFilesUploadFailuresList = [] + int offset = 0 + + while ((uFileList = UFile.createCriteria().list(max: 500, offset: 0) {}) && uFileList.size()) { + uFilesUploadFailuresList.addAll(moveFilesToCDN(uFileList, toCDNProvider, makePublic)) + + offset += 500 + } + + log.debug "Successfully moved files to new ${toCDNProvider.toString()} CDN and failed to upload total files" + + ": ${uFilesUploadFailuresList.size()}" + + return true + } + /** * Moves all UFiles stored at any CDN provider to the given CDN provider. Does not touch UFiles stored locally. * Needs to be executed only once. From dd750e68ee0f1f489798d80d9d23e2233b7e22af Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 11:08:18 +0530 Subject: [PATCH 2/7] method docs and query improvement --- .../fileuploader/FileUploaderService.groovy | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy index d35ad6b..349dca4 100644 --- a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy +++ b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy @@ -414,13 +414,33 @@ class FileUploaderService { return Holders.flatConfig["fileuploader.groups.${fileGroup}.makePublic"] ? true : false } - boolean moveToNewCDN(CDNProvider toCDNProvider = CDNProvider.AMAZON, boolean makePublic = false) { + /** + * This methods move all the instances of {@link UFile} which are not local to the destination {@link CDNProvider}. + * The migration from one CDN to another follows below steps. + * 1. A copy of {@link File} from the source CDN. + * 2. Upload it to the destination CDN. + * 3. Update the respective {@link UFile} instance. + * 4. Creates an instance of {@link UFileMoveHistory} which contains the {@link UFile} move history. + * + * @param toCDNProvider {@link CDNProvider} - Target destination CDN provider. + * @param makePublic {@link boolean} - All the URLs either public or signed. + * @return {@link boolean} Based on the successful move. + */ + boolean moveToNewCDN(CDNProvider toCDNProvider, boolean makePublic = false) { + if (!toCDNProvider) { + return false + } + List uFileList, uFilesUploadFailuresList = [] int offset = 0 - while ((uFileList = UFile.createCriteria().list(max: 500, offset: 0) {}) && uFileList.size()) { + while ((uFileList = UFile.createCriteria().list(max: 500, offset: 0) { + ne('type', CDNProvider.LOCAL) + }).size()) { uFilesUploadFailuresList.addAll(moveFilesToCDN(uFileList, toCDNProvider, makePublic)) + log.debug "Moved ${uFileList.size()} files to new CDN and failed count: ${uFilesUploadFailuresList.size()}" + offset += 500 } From e3f80484933031a527d72b698f4d4d170a50668e Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 11:14:59 +0530 Subject: [PATCH 3/7] query chanegs and log message --- .../com/causecode/fileuploader/FileUploaderService.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy index 349dca4..f000603 100644 --- a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy +++ b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy @@ -428,15 +428,15 @@ class FileUploaderService { */ boolean moveToNewCDN(CDNProvider toCDNProvider, boolean makePublic = false) { if (!toCDNProvider) { + log.debug 'Please provide the target CDN provider name.' + return false } List uFileList, uFilesUploadFailuresList = [] int offset = 0 - while ((uFileList = UFile.createCriteria().list(max: 500, offset: 0) { - ne('type', CDNProvider.LOCAL) - }).size()) { + while ((uFileList = UFile.createCriteria().list(max: 500, offset: 0) {}).size()) { uFilesUploadFailuresList.addAll(moveFilesToCDN(uFileList, toCDNProvider, makePublic)) log.debug "Moved ${uFileList.size()} files to new CDN and failed count: ${uFilesUploadFailuresList.size()}" From c79498c9d7d1abe9c6229804b561aa9de64710b7 Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 13:31:36 +0530 Subject: [PATCH 4/7] changes in query and log message --- .../com/causecode/fileuploader/FileUploaderService.groovy | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy index f000603..efa14e7 100644 --- a/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy +++ b/grails-app/services/com/causecode/fileuploader/FileUploaderService.groovy @@ -417,7 +417,7 @@ class FileUploaderService { /** * This methods move all the instances of {@link UFile} which are not local to the destination {@link CDNProvider}. * The migration from one CDN to another follows below steps. - * 1. A copy of {@link File} from the source CDN. + * 1. A copy of {@link File} from the source CDN {@link UFile} instance. * 2. Upload it to the destination CDN. * 3. Update the respective {@link UFile} instance. * 4. Creates an instance of {@link UFileMoveHistory} which contains the {@link UFile} move history. @@ -433,10 +433,14 @@ class FileUploaderService { return false } + log.debug "Migration from source CDN to target ${toCDNProvider.name()} has been started..." + List uFileList, uFilesUploadFailuresList = [] int offset = 0 - while ((uFileList = UFile.createCriteria().list(max: 500, offset: 0) {}).size()) { + while ((uFileList = UFile.createCriteria().list(max: 500, offset: offset) { + ne('type', CDNProvider.LOCAL) + }).size()) { uFilesUploadFailuresList.addAll(moveFilesToCDN(uFileList, toCDNProvider, makePublic)) log.debug "Moved ${uFileList.size()} files to new CDN and failed count: ${uFilesUploadFailuresList.size()}" From 73b97ba072a6afc23331d955ec3de9c15eface65 Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 15:30:51 +0530 Subject: [PATCH 5/7] test cases added --- .../causecode/fileuploader/DailyJob.groovy | 2 +- .../FileUploaderServiceSpec.groovy | 110 ++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy b/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy index 89c9f90..b4280e2 100644 --- a/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy +++ b/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy @@ -31,7 +31,7 @@ class DailyJob { UFile.withNewSession { temporaryUrlRenewerService.renewTemporaryURL() - fileUploaderService.moveFailedFilesToCDN() + fileUploaderService.moveFailedFilesToCDN() // create Issue github to retry at least 5 times } log.info 'Finished executing DailyJob.' diff --git a/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy b/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy index 4a977a3..b511aa7 100644 --- a/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy +++ b/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy @@ -5,6 +5,8 @@ import com.causecode.fileuploader.util.checksum.Algorithm import com.causecode.fileuploader.util.checksum.exceptions.DuplicateFileException import grails.buildtestdata.BuildDataTest import grails.buildtestdata.mixin.Build +import grails.gorm.CriteriaBuilder +import grails.gorm.PagedResultList import grails.testing.services.ServiceUnitTest import grails.util.Holders import groovy.json.JsonBuilder @@ -36,6 +38,23 @@ class FileUploaderServiceSpec extends BaseFileUploaderServiceSpecSetup implement service.providerService = providerService } + @SuppressWarnings('MapAsMethodParameter') + void mockCreateCriteria(List resultList) { + GroovyMock(UFile, global: true) + CriteriaBuilder criteriaBuilder = Mock() + + criteriaBuilder.list(_, _) >> { Map params, Closure closure -> + JsonBuilder jsonBuilder = new JsonBuilder() + jsonBuilder closure + + return resultList + } >> { + return [] + } + + UFile.createCriteria() >> criteriaBuilder + } + void "test isPublicGroup for various file groups"() { expect: 'Following conditions should pass' service.isPublicGroup('user') == true @@ -864,4 +883,95 @@ class FileUploaderServiceSpec extends BaseFileUploaderServiceSpecSetup implement StorageConfigurationException exception = thrown(StorageConfigurationException) exception.message == 'Container name not defined in the Config. Please define one.' } + + void "test moveToNewCDN method for successfully moving the files to the target CDN provider"() { + given: 'An instance of UFile' + UFile uFileOne = UFile.build(path: '/tmp/test1.txt', provider: CDNProvider.GOOGLE, fileGroup: 'testGoogle') + UFile uFileTwo = UFile.build(path: '/tmp/test2.txt', provider: CDNProvider.GOOGLE, fileGroup: 'testGoogle') + + and: 'Mocked the createCriteria method to return the UFiles list' + mockCreateCriteria([uFileOne, uFileTwo]) + + and: 'Mocked the moveFilesToCDN method to move the files to source CDN provider successfully' + service.metaClass.static.moveFilesToCDN = { List list, CDNProvider provider, boolean makePublic -> + list.each { UFile file -> + file.provider = CDNProvider.AMAZON + file.save() + } + } + + when: 'moveToNewCDN method is called' + assert uFileOne.provider == CDNProvider.GOOGLE + assert uFileTwo.provider == CDNProvider.GOOGLE + + boolean status = service.moveToNewCDN(CDNProvider.AMAZON, false) + + then: 'it should return true and all the UFile providers must be changed to target CDN' + status + uFileOne.provider == CDNProvider.AMAZON + uFileTwo.provider == CDNProvider.AMAZON + } + + void "test moveToNewCDN method for the failure case when we move files to target CDN"() { + given: 'An instance of UFile' + UFile uFileOne = UFile.build(path: '/tmp/test1.txt', provider: CDNProvider.GOOGLE, fileGroup: 'testGoogle') + UFile uFileTwo = UFile.build(path: '/tmp/test2.txt', provider: CDNProvider.GOOGLE, fileGroup: 'testGoogle') + + and: 'Mocked the createCriteria method to return the UFiles list' + mockCreateCriteria([uFileOne, uFileTwo]) + + and: 'Mocked the moveFilesToCDN method to return the list of failed documents' + service.metaClass.static.moveFilesToCDN = { List list, CDNProvider provider, boolean makePublic -> + list.each { UFile file -> + UFileMoveHistory.build(ufile: file, fromCDN: file.provider, toCDN: provider, status: MoveStatus.FAILURE, + details: 'failed to move.') + return false + } + + return list + } + + when: 'moveToNewCDN method is called' + boolean status = service.moveToNewCDN(CDNProvider.AMAZON, false) + + then: 'it should return true and all the UFile providers must not be changed' + status + uFileOne.provider == CDNProvider.GOOGLE + uFileTwo.provider == CDNProvider.GOOGLE + UFileMoveHistory.count() == 2 + } + + void "test moveToNewCDN method for amazon when exception occurs in a file URL"() { + given: 'An instance of UFile and File' + UFile uFileInstance = UFile.build(path: '/tmp/test.txt', provider: CDNProvider.GOOGLE, fileGroup: 'testGoogle') + File fileInstance = getFileInstance('/tmp/test.txt') + + and: 'Mocked method' + service.metaClass.getFileFromURL = { String url, String filename -> + throw new IOException('Error getting file from URL') + } + + and: 'Mocked Amazon provider instance' + mockGetProviderInstance(CDNProvider.AMAZON.name()) + + and: 'Mocked the uploadFile method of amazon' + mockUploadFileMethod(true) + + when: 'moveFilesToCDN method is called' + service.moveToNewCDN(CDNProvider.AMAZON, true) + + then: 'File won\'t be moved' + uFileInstance.provider == CDNProvider.GOOGLE + + cleanup: + fileInstance.delete() + } + + void "test moveToNewCDN method is called with null CDN provider name"() { + when: 'moveToNewCDN method is called' + boolean status = service.moveToNewCDN(null, false) + + then: 'it should return false' + !status + } } From 019edfb038279e602f81a6cefd0cb62af469118e Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 15:32:38 +0530 Subject: [PATCH 6/7] removed the comment --- grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy b/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy index b4280e2..89c9f90 100644 --- a/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy +++ b/grails-app/jobs/com/causecode/fileuploader/DailyJob.groovy @@ -31,7 +31,7 @@ class DailyJob { UFile.withNewSession { temporaryUrlRenewerService.renewTemporaryURL() - fileUploaderService.moveFailedFilesToCDN() // create Issue github to retry at least 5 times + fileUploaderService.moveFailedFilesToCDN() } log.info 'Finished executing DailyJob.' From 4f3b085d5259d932cd88101f16c1ec5ad83e653e Mon Sep 17 00:00:00 2001 From: Shivam Pandey Date: Wed, 24 Apr 2019 15:33:47 +0530 Subject: [PATCH 7/7] removed unnecessary import --- .../com/causecode/fileuploader/FileUploaderServiceSpec.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy b/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy index b511aa7..715e2dd 100644 --- a/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy +++ b/src/test/groovy/com/causecode/fileuploader/FileUploaderServiceSpec.groovy @@ -6,7 +6,6 @@ import com.causecode.fileuploader.util.checksum.exceptions.DuplicateFileExceptio import grails.buildtestdata.BuildDataTest import grails.buildtestdata.mixin.Build import grails.gorm.CriteriaBuilder -import grails.gorm.PagedResultList import grails.testing.services.ServiceUnitTest import grails.util.Holders import groovy.json.JsonBuilder