From 826c9a0f309b19dcc7be5566630119a79d06ecd7 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Mon, 16 Dec 2019 12:40:47 +0100 Subject: [PATCH 1/2] MOBILE-3213 files: Refactor files size calculation to avoid network requests --- .../submission/file/providers/handler.ts | 69 +++---------------- .../onlinetext/providers/handler.ts | 34 +++------ src/providers/file-helper.ts | 63 ++++++++++++++++- 3 files changed, 79 insertions(+), 87 deletions(-) diff --git a/src/addon/mod/assign/submission/file/providers/handler.ts b/src/addon/mod/assign/submission/file/providers/handler.ts index 8ff4f6599..6a459dadd 100644 --- a/src/addon/mod/assign/submission/file/providers/handler.ts +++ b/src/addon/mod/assign/submission/file/providers/handler.ts @@ -14,11 +14,8 @@ // limitations under the License. import { Injectable, Injector } from '@angular/core'; -import { CoreFileProvider } from '@providers/file'; import { CoreFileSessionProvider } from '@providers/file-session'; -import { CoreFilepoolProvider } from '@providers/filepool'; -import { CoreSitesProvider } from '@providers/sites'; -import { CoreWSProvider } from '@providers/ws'; +import { CoreFileHelperProvider } from '@providers/file-helper'; import { CoreUtilsProvider } from '@providers/utils/utils'; import { CoreFileUploaderProvider } from '@core/fileuploader/providers/fileuploader'; import { @@ -39,11 +36,10 @@ export class AddonModAssignSubmissionFileHandler implements AddonModAssignSubmis name = 'AddonModAssignSubmissionFileHandler'; type = 'file'; - constructor(private sitesProvider: CoreSitesProvider, private wsProvider: CoreWSProvider, - private assignProvider: AddonModAssignProvider, private assignOfflineProvider: AddonModAssignOfflineProvider, + constructor(private assignProvider: AddonModAssignProvider, private assignOfflineProvider: AddonModAssignOfflineProvider, private assignHelper: AddonModAssignHelperProvider, private fileSessionProvider: CoreFileSessionProvider, - private fileUploaderProvider: CoreFileUploaderProvider, private filepoolProvider: CoreFilepoolProvider, - private fileProvider: CoreFileProvider, private utils: CoreUtilsProvider) { } + private fileUploaderProvider: CoreFileUploaderProvider, private fileHelper: CoreFileHelperProvider, + private utils: CoreUtilsProvider) { } /** * Whether the plugin can be edited in offline for existing submissions. In general, this should return false if the @@ -157,24 +153,9 @@ export class AddonModAssignSubmissionFileHandler implements AddonModAssignSubmis * @return The size (or promise resolved with size). */ getSizeForCopy(assign: AddonModAssignAssign, plugin: AddonModAssignPlugin): number | Promise { - const files = this.assignProvider.getSubmissionPluginAttachments(plugin), - promises = []; - let totalSize = 0; + const files = this.assignProvider.getSubmissionPluginAttachments(plugin); - files.forEach((file) => { - promises.push(this.wsProvider.getRemoteFileSize(file.fileurl).then((size) => { - if (size == -1) { - // Couldn't determine the size, reject. - return Promise.reject(null); - } - - totalSize += size; - })); - }); - - return Promise.all(promises).then(() => { - return totalSize; - }); + return this.fileHelper.getTotalFilesSize(files); } /** @@ -188,45 +169,11 @@ export class AddonModAssignSubmissionFileHandler implements AddonModAssignSubmis */ getSizeForEdit(assign: AddonModAssignAssign, submission: AddonModAssignSubmission, plugin: AddonModAssignPlugin, inputData: any): number | Promise { - const siteId = this.sitesProvider.getCurrentSiteId(); - // Check if there's any change. if (this.hasDataChanged(assign, submission, plugin, inputData)) { - const files = this.fileSessionProvider.getFiles(AddonModAssignProvider.COMPONENT, assign.id), - promises = []; - let totalSize = 0; + const files = this.fileSessionProvider.getFiles(AddonModAssignProvider.COMPONENT, assign.id); - files.forEach((file) => { - if (file.filename && !file.name) { - // It's a remote file. First check if we have the file downloaded since it's more reliable. - promises.push(this.filepoolProvider.getFilePathByUrl(siteId, file.fileurl).then((path) => { - return this.fileProvider.getFile(path).then((fileEntry) => { - return this.fileProvider.getFileObjectFromFileEntry(fileEntry); - }).then((file) => { - totalSize += file.size; - }); - }).catch(() => { - // Error getting the file, maybe it's not downloaded. Get remote size. - return this.wsProvider.getRemoteFileSize(file.fileurl).then((size) => { - if (size == -1) { - // Couldn't determine the size, reject. - return Promise.reject(null); - } - - totalSize += size; - }); - })); - } else if (file.name) { - // It's a local file, get its size. - promises.push(this.fileProvider.getFileObjectFromFileEntry(file).then((file) => { - totalSize += file.size; - })); - } - }); - - return Promise.all(promises).then(() => { - return totalSize; - }); + return this.fileHelper.getTotalFilesSize(files); } else { // Nothing has changed, we won't upload any file. return 0; diff --git a/src/addon/mod/assign/submission/onlinetext/providers/handler.ts b/src/addon/mod/assign/submission/onlinetext/providers/handler.ts index bc800d716..77e0ec4e4 100644 --- a/src/addon/mod/assign/submission/onlinetext/providers/handler.ts +++ b/src/addon/mod/assign/submission/onlinetext/providers/handler.ts @@ -16,7 +16,7 @@ import { Injectable, Injector } from '@angular/core'; import { TranslateService } from '@ngx-translate/core'; import { CoreSitesProvider } from '@providers/sites'; -import { CoreWSProvider } from '@providers/ws'; +import { CoreFileHelperProvider } from '@providers/file-helper'; import { CoreTextUtilsProvider } from '@providers/utils/text'; import { AddonModAssignProvider, AddonModAssignAssign, AddonModAssignSubmission, AddonModAssignPlugin @@ -34,9 +34,10 @@ export class AddonModAssignSubmissionOnlineTextHandler implements AddonModAssign name = 'AddonModAssignSubmissionOnlineTextHandler'; type = 'onlinetext'; - constructor(private translate: TranslateService, private sitesProvider: CoreSitesProvider, private wsProvider: CoreWSProvider, - private textUtils: CoreTextUtilsProvider, private assignProvider: AddonModAssignProvider, - private assignOfflineProvider: AddonModAssignOfflineProvider, private assignHelper: AddonModAssignHelperProvider) { } + constructor(private translate: TranslateService, private sitesProvider: CoreSitesProvider, + private fileHelper: CoreFileHelperProvider, private textUtils: CoreTextUtilsProvider, + private assignProvider: AddonModAssignProvider, private assignOfflineProvider: AddonModAssignOfflineProvider, + private assignHelper: AddonModAssignHelperProvider) { } /** * Whether the plugin can be edited in offline for existing submissions. In general, this should return false if the @@ -124,30 +125,13 @@ export class AddonModAssignSubmissionOnlineTextHandler implements AddonModAssign * @param plugin The plugin object. * @return The size (or promise resolved with size). */ - getSizeForCopy(assign: AddonModAssignAssign, plugin: AddonModAssignPlugin): number | Promise { + async getSizeForCopy(assign: AddonModAssignAssign, plugin: AddonModAssignPlugin): Promise { const text = this.assignProvider.getSubmissionPluginText(plugin, true), - files = this.assignProvider.getSubmissionPluginAttachments(plugin), - promises = []; - let totalSize = text.length; + files = this.assignProvider.getSubmissionPluginAttachments(plugin); - if (!files.length) { - return totalSize; - } + const filesSize = await this.fileHelper.getTotalFilesSize(files); - files.forEach((file) => { - promises.push(this.wsProvider.getRemoteFileSize(file.fileurl).then((size) => { - if (size == -1) { - // Couldn't determine the size, reject. - return Promise.reject(null); - } - - totalSize += size; - })); - }); - - return Promise.all(promises).then(() => { - return totalSize; - }); + return text.length + filesSize; } /** diff --git a/src/providers/file-helper.ts b/src/providers/file-helper.ts index a9fc44c1e..73daa59b3 100644 --- a/src/providers/file-helper.ts +++ b/src/providers/file-helper.ts @@ -18,6 +18,7 @@ import { CoreAppProvider } from './app'; import { CoreFileProvider } from './file'; import { CoreFilepoolProvider } from './filepool'; import { CoreSitesProvider } from './sites'; +import { CoreWSProvider } from './ws'; import { CoreUtilsProvider } from './utils/utils'; import { CoreConstants } from '@core/constants'; @@ -29,7 +30,7 @@ export class CoreFileHelperProvider { constructor(private fileProvider: CoreFileProvider, private filepoolProvider: CoreFilepoolProvider, private sitesProvider: CoreSitesProvider, private appProvider: CoreAppProvider, private translate: TranslateService, - private utils: CoreUtilsProvider) { } + private utils: CoreUtilsProvider, private wsProvider: CoreWSProvider) { } /** * Convenience function to open a file, downloading it if needed. @@ -273,4 +274,64 @@ export class CoreFileHelperProvider { return false; } + + /** + * Calculate the total size of the given files. + * + * @param files The files to check. + * @return Total files size. + */ + async getTotalFilesSize(files: any[]): Promise { + let totalSize = 0; + + for (const file of files) { + totalSize += await this.getFileSize(file); + } + + return totalSize; + } + + /** + * Calculate the file size. + * + * @param file The file to check. + * @return File size. + */ + async getFileSize(file: any): Promise { + if (file.filesize) { + return file.filesize; + } + + // If it's a remote file. First check if we have the file downloaded since it's more reliable. + if (file.filename && !file.name) { + try { + const siteId = this.sitesProvider.getCurrentSiteId(); + + const path = await this.filepoolProvider.getFilePathByUrl(siteId, file.fileurl); + const fileEntry = await this.fileProvider.getFile(path); + const fileObject = await this.fileProvider.getFileObjectFromFileEntry(fileEntry); + + return fileObject.size; + } catch (error) { + // Error getting the file, maybe it's not downloaded. Get remote size. + const size = await this.wsProvider.getRemoteFileSize(file.fileurl); + + if (size === -1) { + throw new Error('Couldn\'t determine file size: ' + file.fileurl); + } + + return size; + } + } + + // If it's a local file, get its size. + if (file.name) { + const fileObject = await this.fileProvider.getFileObjectFromFileEntry(file); + + return fileObject.size; + } + + throw new Error('Couldn\'t determine file size: ' + file.fileurl); + } + } From 89e90624b3d283fad034bead99360d512653c822 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Mon, 16 Dec 2019 15:44:31 +0100 Subject: [PATCH 2/2] MOBILE-3213 files: Improve download message with 0 bytes. --- scripts/langindex.json | 1 + src/assets/lang/en.json | 1 + src/core/course/lang/en.json | 1 + src/providers/utils/dom.ts | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/langindex.json b/scripts/langindex.json index 5200cc6d6..ed6b18340 100644 --- a/scripts/langindex.json +++ b/scripts/langindex.json @@ -1393,6 +1393,7 @@ "core.course.confirmdeletemodulefiles": "local_moodlemobileapp", "core.course.confirmdownload": "local_moodlemobileapp", "core.course.confirmdownloadunknownsize": "local_moodlemobileapp", + "core.course.confirmdownloadzerosize": "local_moodlemobileapp", "core.course.confirmlimiteddownload": "local_moodlemobileapp", "core.course.confirmpartialdownloadsize": "local_moodlemobileapp", "core.course.contents": "local_moodlemobileapp", diff --git a/src/assets/lang/en.json b/src/assets/lang/en.json index e0fb0fd71..2256cf466 100644 --- a/src/assets/lang/en.json +++ b/src/assets/lang/en.json @@ -1391,6 +1391,7 @@ "core.course.confirmdeletemodulefiles": "Are you sure you want to delete these files?", "core.course.confirmdownload": "You are about to download {{size}}.{{availableSpace}} Are you sure you want to continue?", "core.course.confirmdownloadunknownsize": "It was not possible to calculate the size of the download.{{availableSpace}} Are you sure you want to continue?", + "core.course.confirmdownloadzerosize": "You are about to start downloading.{{availableSpace}} Are you sure you want to continue?", "core.course.confirmlimiteddownload": "You are not currently connected to Wi-Fi. ", "core.course.confirmpartialdownloadsize": "You are about to download at least {{size}}.{{availableSpace}} Are you sure you want to continue?", "core.course.contents": "Contents", diff --git a/src/core/course/lang/en.json b/src/core/course/lang/en.json index 2d2d0156f..2fe5518e9 100644 --- a/src/core/course/lang/en.json +++ b/src/core/course/lang/en.json @@ -8,6 +8,7 @@ "confirmdeletemodulefiles": "Are you sure you want to delete these files?", "confirmdownload": "You are about to download {{size}}.{{availableSpace}} Are you sure you want to continue?", "confirmdownloadunknownsize": "It was not possible to calculate the size of the download.{{availableSpace}} Are you sure you want to continue?", + "confirmdownloadzerosize": "You are about to start downloading.{{availableSpace}} Are you sure you want to continue?", "confirmpartialdownloadsize": "You are about to download at least {{size}}.{{availableSpace}} Are you sure you want to continue?", "confirmlimiteddownload": "You are not currently connected to Wi-Fi. ", "contents": "Contents", diff --git a/src/providers/utils/dom.ts b/src/providers/utils/dom.ts index 6bc976dd6..91e4b7ad1 100644 --- a/src/providers/utils/dom.ts +++ b/src/providers/utils/dom.ts @@ -200,7 +200,7 @@ export class CoreDomUtilsProvider { { size: readableSize, availableSpace: availableSpace })); } else if (alwaysConfirm || size.size >= wifiThreshold || (this.appProvider.isNetworkAccessLimited() && size.size >= limitedThreshold)) { - message = message || 'core.course.confirmdownload'; + message = message || (size.size === 0 ? 'core.course.confirmdownloadzerosize' : 'core.course.confirmdownload'); return this.showConfirm(wifiPrefix + this.translate.instant(message, { size: readableSize, availableSpace: availableSpace }));