From bea2a0cbc6d9a190390e20549cfc2502f1f375dd Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Fri, 24 Aug 2018 11:05:39 +0200 Subject: [PATCH] MOBILE-2563 course: Decrease calls to core_course_get_contents --- .../mod/resource/providers/module-handler.ts | 99 +++++++++---------- .../resource/providers/prefetch-handler.ts | 7 +- src/addon/mod/resource/providers/resource.ts | 2 +- src/addon/mod/url/components/index/index.ts | 4 +- src/addon/mod/url/providers/link-handler.ts | 1 + src/addon/mod/url/providers/module-handler.ts | 5 +- src/addon/mod/url/providers/url.ts | 2 +- .../classes/module-grade-handler.ts | 10 +- .../classes/module-index-handler.ts | 10 +- src/core/course/providers/course.ts | 57 ++++++++--- src/core/course/providers/helper.ts | 6 +- 11 files changed, 129 insertions(+), 74 deletions(-) diff --git a/src/addon/mod/resource/providers/module-handler.ts b/src/addon/mod/resource/providers/module-handler.ts index 8a9822f33..79700acb2 100644 --- a/src/addon/mod/resource/providers/module-handler.ts +++ b/src/addon/mod/resource/providers/module-handler.ts @@ -66,7 +66,7 @@ export class AddonModResourceModuleHandler implements CoreCourseModuleHandler { }; const handlerData: CoreCourseModuleHandlerData = { - icon: this.courseProvider.getModuleIconSrc('resource'), + icon: this.courseProvider.getModuleIconSrc(this.modName), title: module.name, class: 'addon-mod_resource-handler', showDownloadButton: true, @@ -88,16 +88,12 @@ export class AddonModResourceModuleHandler implements CoreCourseModuleHandler { } ] }; - this.getResourceData(module, courseId).then((data) => { + this.getResourceData(module, courseId, handlerData).then((data) => { handlerData.icon = data.icon; handlerData.extraBadge = data.extra; handlerData.extraBadgeColor = 'light'; }); - this.hideOpenButton(module, courseId).then((hideOpenButton) => { - handlerData.buttons[0].hidden = hideOpenButton; - }); - return handlerData; } @@ -109,7 +105,8 @@ export class AddonModResourceModuleHandler implements CoreCourseModuleHandler { * @return {Promise} Resolved when done. */ protected hideOpenButton(module: any, courseId: number): Promise { - return this.courseProvider.loadModuleContents(module, courseId).then(() => { + return this.courseProvider.loadModuleContents(module, courseId, undefined, false, false, undefined, this.modName) + .then(() => { return this.prefetchDelegate.getModuleStatus(module, courseId).then((status) => { return status !== CoreConstants.DOWNLOADED || this.resourceHelper.isDisplayedInIframe(module); }); @@ -123,55 +120,55 @@ export class AddonModResourceModuleHandler implements CoreCourseModuleHandler { * @param {number} courseId The course ID. * @return {Promise} Resource data. */ - protected getResourceData(module: any, courseId: number): Promise { - return this.resourceProvider.getResourceData(courseId, module.id).then((info) => { - let promise; + protected getResourceData(module: any, courseId: number, handlerData: CoreCourseModuleHandlerData): Promise { + const promises = []; + let resourceInfo; - if (info.contentfiles && info.contentfiles.length == 1) { - promise = Promise.resolve(info.contentfiles); - } else { - promise = this.courseProvider.loadModuleContents(module, courseId).then(() => { - if (module.contents.length) { - return module.contents; - } - }); + // Check if the button needs to be shown or not. This also loads the module contents. + promises.push(this.hideOpenButton(module, courseId).then((hideOpenButton) => { + handlerData.buttons[0].hidden = hideOpenButton; + })); + + // Get the resource data. + promises.push(this.resourceProvider.getResourceData(courseId, module.id).then((info) => { + resourceInfo = info; + })); + + return Promise.all(promises).then(() => { + const files = module.contents && module.contents.length ? module.contents : resourceInfo.contentfiles, + resourceData = { + icon: '', + extra: '' + }, + options = this.textUtils.unserialize(resourceInfo.displayoptions), + extra = []; + + if (files && files.length) { + const file = files[0]; + resourceData.icon = this.mimetypeUtils.getFileIcon(file.filename); + + if (options.showsize) { + const size = files.reduce((result, file) => { + return result + file.filesize; + }, 0); + extra.push(this.textUtils.bytesToSize(size, 1)); + } + if (options.showtype) { + extra.push(this.mimetypeUtils.getMimetypeDescription(file)); + } } - return promise.then((files) => { - const resourceData = { - icon: '', - extra: '' - }, - options = this.textUtils.unserialize(info.displayoptions), - extra = []; + if (resourceData.icon == '') { + resourceData.icon = this.courseProvider.getModuleIconSrc(this.modName); + } - if (files && files.length) { - const file = files[0]; - resourceData.icon = this.mimetypeUtils.getFileIcon(file.filename); + if (options.showdate) { + extra.push(this.translate.instant('addon.mod_resource.uploadeddate', + {$a: moment(resourceInfo.timemodified * 1000).format('LLL')})); + } + resourceData.extra += extra.join(' '); - if (options.showsize) { - const size = files.reduce((result, file) => { - return result + file.filesize; - }, 0); - extra.push(this.textUtils.bytesToSize(size, 1)); - } - if (options.showtype) { - extra.push(this.mimetypeUtils.getMimetypeDescription(file)); - } - } - - if (resourceData.icon == '') { - resourceData.icon = this.courseProvider.getModuleIconSrc('resource'); - } - - if (options.showdate) { - extra.push(this.translate.instant('addon.mod_resource.uploadeddate', - {$a: moment(info.timemodified * 1000).format('LLL')})); - } - resourceData.extra += extra.join(' '); - - return resourceData; - }); + return resourceData; }); } diff --git a/src/addon/mod/resource/providers/prefetch-handler.ts b/src/addon/mod/resource/providers/prefetch-handler.ts index 93b675a0e..1a2342acf 100644 --- a/src/addon/mod/resource/providers/prefetch-handler.ts +++ b/src/addon/mod/resource/providers/prefetch-handler.ts @@ -70,6 +70,11 @@ export class AddonModResourcePrefetchHandler extends CoreCourseResourcePrefetchH promises.push(this.resourceProvider.getResourceData(courseId, module.id)); } + /* When prefetching we usually use ignoreCache=true. However, this WS call can return a lot of data, so if + a user downloads resources 1 by 1 we would be downloading the same data over and over again. Since + this data won't change often it's probably better to use ignoreCache=false. */ + promises.push(this.courseProvider.getModule(module.id, courseId, undefined, false, false, undefined, this.modName)); + return Promise.all(promises); }); } @@ -96,7 +101,7 @@ export class AddonModResourcePrefetchHandler extends CoreCourseResourcePrefetchH const promises = []; promises.push(this.resourceProvider.invalidateResourceData(courseId)); - promises.push(this.courseProvider.invalidateModule(module.id)); + promises.push(this.courseProvider.invalidateModule(module.id, undefined, this.modName)); return Promise.all(promises); } diff --git a/src/addon/mod/resource/providers/resource.ts b/src/addon/mod/resource/providers/resource.ts index 99559bb42..cd09eabc6 100644 --- a/src/addon/mod/resource/providers/resource.ts +++ b/src/addon/mod/resource/providers/resource.ts @@ -104,7 +104,7 @@ export class AddonModResourceProvider { promises.push(this.invalidateResourceData(courseId, siteId)); promises.push(this.filepoolProvider.invalidateFilesByComponent(siteId, AddonModResourceProvider.COMPONENT, moduleId)); - promises.push(this.courseProvider.invalidateModule(moduleId, siteId)); + promises.push(this.courseProvider.invalidateModule(moduleId, siteId, 'resource')); return this.utils.allPromises(promises); } diff --git a/src/addon/mod/url/components/index/index.ts b/src/addon/mod/url/components/index/index.ts index fdbc1678c..d1602299f 100644 --- a/src/addon/mod/url/components/index/index.ts +++ b/src/addon/mod/url/components/index/index.ts @@ -78,7 +78,7 @@ export class AddonModUrlIndexComponent extends CoreCourseModuleMainResourceCompo canGetUrl = false; // Fallback in case is not prefetched or not available. - return this.courseProvider.getModule(this.module.id, this.courseId); + return this.courseProvider.getModule(this.module.id, this.courseId, undefined, false, false, undefined, 'url'); }).then((url) => { this.description = url.intro || url.description; this.dataRetrieved.emit(url); @@ -95,7 +95,7 @@ export class AddonModUrlIndexComponent extends CoreCourseModuleMainResourceCompo if (!mod.contents || !mod.contents.length) { // Try to load module contents, it's needed to get the URL with parameters. - return this.courseProvider.loadModuleContents(mod, this.courseId); + return this.courseProvider.loadModuleContents(mod, this.courseId, undefined, false, false, undefined, 'url'); } } }).then(() => { diff --git a/src/addon/mod/url/providers/link-handler.ts b/src/addon/mod/url/providers/link-handler.ts index d880a0b9a..8d9cc9da0 100644 --- a/src/addon/mod/url/providers/link-handler.ts +++ b/src/addon/mod/url/providers/link-handler.ts @@ -22,6 +22,7 @@ import { CoreCourseHelperProvider } from '@core/course/providers/helper'; @Injectable() export class AddonModUrlLinkHandler extends CoreContentLinksModuleIndexHandler { name = 'AddonModUrlLinkHandler'; + useModNameToGetModule = true; constructor(courseHelper: CoreCourseHelperProvider) { super(courseHelper, 'AddonModUrl', 'url'); diff --git a/src/addon/mod/url/providers/module-handler.ts b/src/addon/mod/url/providers/module-handler.ts index f1162e636..57480c0d5 100644 --- a/src/addon/mod/url/providers/module-handler.ts +++ b/src/addon/mod/url/providers/module-handler.ts @@ -50,7 +50,7 @@ export class AddonModUrlModuleHandler implements CoreCourseModuleHandler { */ getData(module: any, courseId: number, sectionId: number): CoreCourseModuleHandlerData { const handlerData = { - icon: this.courseProvider.getModuleIconSrc('url'), + icon: this.courseProvider.getModuleIconSrc(this.modName), title: module.name, class: 'addon-mod_url-handler', showDownloadButton: false, @@ -89,7 +89,8 @@ export class AddonModUrlModuleHandler implements CoreCourseModuleHandler { * @return {Promise} Resolved when done. */ protected hideLinkButton(module: any, courseId: number): Promise { - return this.courseProvider.loadModuleContents(module, courseId).then(() => { + return this.courseProvider.loadModuleContents(module, courseId, undefined, false, false, undefined, this.modName) + .then(() => { return !(module.contents && module.contents[0] && module.contents[0].fileurl); }); } diff --git a/src/addon/mod/url/providers/url.ts b/src/addon/mod/url/providers/url.ts index 2472dccae..19750f8c7 100644 --- a/src/addon/mod/url/providers/url.ts +++ b/src/addon/mod/url/providers/url.ts @@ -102,7 +102,7 @@ export class AddonModUrlProvider { const promises = []; promises.push(this.invalidateUrlData(courseId, siteId)); - promises.push(this.courseProvider.invalidateModule(moduleId, siteId)); + promises.push(this.courseProvider.invalidateModule(moduleId, siteId, 'url')); return this.utils.allPromises(promises); } diff --git a/src/core/contentlinks/classes/module-grade-handler.ts b/src/core/contentlinks/classes/module-grade-handler.ts index 02ade7789..a97b30a2a 100644 --- a/src/core/contentlinks/classes/module-grade-handler.ts +++ b/src/core/contentlinks/classes/module-grade-handler.ts @@ -30,6 +30,13 @@ export class CoreContentLinksModuleGradeHandler extends CoreContentLinksHandlerB */ canReview: boolean; + /** + * If this boolean is set to true, the app will retrieve all modules with this modName with a single WS call. + * This reduces the number of WS calls, but it isn't recommended for modules that can return a lot of contents. + * @type {boolean} + */ + protected useModNameToGetModule = false; + /** * Construct the handler. * @@ -69,7 +76,8 @@ export class CoreContentLinksModuleGradeHandler extends CoreContentLinksHandlerB this.sitesProvider.getSite(siteId).then((site) => { if (!params.userid || params.userid == site.getUserId()) { // No user specified or current user. Navigate to module. - this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId); + this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId, undefined, + this.useModNameToGetModule ? this.modName : undefined); } else if (this.canReview) { // Use the goToReview function. this.goToReview(url, params, courseId, siteId, navCtrl); diff --git a/src/core/contentlinks/classes/module-index-handler.ts b/src/core/contentlinks/classes/module-index-handler.ts index 2a10bee94..fbb65afba 100644 --- a/src/core/contentlinks/classes/module-index-handler.ts +++ b/src/core/contentlinks/classes/module-index-handler.ts @@ -21,6 +21,13 @@ import { CoreCourseHelperProvider } from '@core/course/providers/helper'; */ export class CoreContentLinksModuleIndexHandler extends CoreContentLinksHandlerBase { + /** + * If this boolean is set to true, the app will retrieve all modules with this modName with a single WS call. + * This reduces the number of WS calls, but it isn't recommended for modules that can return a lot of contents. + * @type {boolean} + */ + protected useModNameToGetModule = false; + /** * Construct the handler. * @@ -52,7 +59,8 @@ export class CoreContentLinksModuleIndexHandler extends CoreContentLinksHandlerB return [{ action: (siteId, navCtrl?): void => { - this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId); + this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId, undefined, + this.useModNameToGetModule ? this.modName : undefined); } }]; } diff --git a/src/core/course/providers/course.ts b/src/core/course/providers/course.ts index a491ab610..efdbf618e 100644 --- a/src/core/course/providers/course.ts +++ b/src/core/course/providers/course.ts @@ -198,10 +198,12 @@ export class CoreCourseProvider { * @param {boolean} [preferCache] True if shouldn't call WS if data is cached, false otherwise. * @param {boolean} [ignoreCache] True if it should ignore cached data (it will always fail in offline or server down). * @param {string} [siteId] Site ID. If not defined, current site. + * @param {string} [modName] If set, the app will retrieve all modules of this type with a single WS call. This reduces the + * number of WS calls, but it isn't recommended for modules that can return a lot of contents. * @return {Promise} Promise resolved with the module. */ getModule(moduleId: number, courseId?: number, sectionId?: number, preferCache?: boolean, ignoreCache?: boolean, - siteId?: string): Promise { + siteId?: string, modName?: string): Promise { siteId = siteId || this.sitesProvider.getCurrentSiteId(); let promise; @@ -226,18 +228,27 @@ export class CoreCourseProvider { const params = { courseid: courseId, - options: [ - { - name: 'cmid', - value: moduleId - } - ] + options: [] }, preSets: any = { - cacheKey: this.getModuleCacheKey(moduleId), omitExpires: preferCache }; + // If modName is set, retrieve all modules of that type. Otherwise get only the module. + if (modName) { + params.options.push({ + name: 'modname', + value: modName + }); + preSets.cacheKey = this.getModuleByModNameCacheKey(modName); + } else { + params.options.push({ + name: 'cmid', + value: moduleId + }); + preSets.cacheKey = this.getModuleCacheKey(moduleId); + } + if (!preferCache && ignoreCache) { preSets.getFromCache = 0; preSets.emergencyCache = 0; @@ -376,6 +387,16 @@ export class CoreCourseProvider { return this.ROOT_CACHE_KEY + 'module:' + moduleId; } + /** + * Get cache key for module by modname WS calls. + * + * @param {string} modName Name of the module. + * @return {string} Cache key. + */ + protected getModuleByModNameCacheKey(modName: string): string { + return this.ROOT_CACHE_KEY + 'module:modName:' + modName; + } + /** * Returns the source to a module icon. * @@ -518,11 +539,20 @@ export class CoreCourseProvider { * * @param {number} moduleId Module ID. * @param {string} [siteId] Site ID. If not defined, current site. + * @param {string} [modName] Module name. E.g. 'label', 'url', ... * @return {Promise} Promise resolved when the data is invalidated. */ - invalidateModule(moduleId: number, siteId?: string): Promise { + invalidateModule(moduleId: number, siteId?: string, modName?: string): Promise { return this.sitesProvider.getSite(siteId).then((site) => { - return site.invalidateWsCacheForKey(this.getModuleCacheKey(moduleId)); + const promises = []; + + if (modName) { + promises.push(site.invalidateWsCacheForKey(this.getModuleByModNameCacheKey(modName))); + } + + promises.push(site.invalidateWsCacheForKey(this.getModuleCacheKey(moduleId))); + + return Promise.all(promises); }); } @@ -574,16 +604,19 @@ export class CoreCourseProvider { * @param {boolean} [preferCache] True if shouldn't call WS if data is cached, false otherwise. * @param {boolean} [ignoreCache] True if it should ignore cached data (it will always fail in offline or server down). * @param {string} [siteId] Site ID. If not defined, current site. + * @param {string} [modName] If set, the app will retrieve all modules of this type with a single WS call. This reduces the + * number of WS calls, but it isn't recommended for modules that can return a lot of contents. * @return {Promise} Promise resolved when loaded. */ loadModuleContents(module: any, courseId?: number, sectionId?: number, preferCache?: boolean, ignoreCache?: boolean, - siteId?: string): Promise { + siteId?: string, modName?: string): Promise { + if (!ignoreCache && module.contents && module.contents.length) { // Already loaded. return Promise.resolve(); } - return this.getModule(module.id, courseId, sectionId, preferCache, ignoreCache, siteId).then((mod) => { + return this.getModule(module.id, courseId, sectionId, preferCache, ignoreCache, siteId, modName).then((mod) => { module.contents = mod.contents; }); } diff --git a/src/core/course/providers/helper.ts b/src/core/course/providers/helper.ts index 859b36fa9..87eb371d9 100644 --- a/src/core/course/providers/helper.ts +++ b/src/core/course/providers/helper.ts @@ -887,9 +887,11 @@ export class CoreCourseHelperProvider { * @param {string} [siteId] Site ID. If not defined, current site. * @param {number} [courseId] Course ID. If not defined we'll try to retrieve it from the site. * @param {number} [sectionId] Section the module belongs to. If not defined we'll try to retrieve it from the site. + * @param {string} [modName] If set, the app will retrieve all modules of this type with a single WS call. This reduces the + * number of WS calls, but it isn't recommended for modules that can return a lot of contents. * @return {Promise} Promise resolved when done. */ - navigateToModule(moduleId: number, siteId?: string, courseId?: number, sectionId?: number): Promise { + navigateToModule(moduleId: number, siteId?: string, courseId?: number, sectionId?: number, modName?: string): Promise { siteId = siteId || this.sitesProvider.getCurrentSiteId(); const modal = this.domUtils.showModalLoading(); @@ -923,7 +925,7 @@ export class CoreCourseHelperProvider { site = s; // Get the module. - return this.courseProvider.getModule(moduleId, courseId, sectionId, false, false, siteId); + return this.courseProvider.getModule(moduleId, courseId, sectionId, false, false, siteId, modName); }).then((module) => { const params = { course: { id: courseId },