From d0c93f6416b7ee7cecf0d520e64c377141fecded Mon Sep 17 00:00:00 2001 From: sam marshall Date: Fri, 11 Oct 2019 16:03:28 +0100 Subject: [PATCH] MOBILE-3188 Web services: Record component that owns WS cache item This commit adds optional component and componentId fields to the preSets variable, which are recorded in the wscache table. These are then used by (a) core site plugins where a module cmid is available, and (b) the forum (as an example core implementation) so that module site plugins and forum provide the new data. (Note that this is not going to be very useful where we retrieve data for multiple activities at once, which happens for activities like Page. Still, it is optional.) --- src/addon/mod/forum/components/index/index.ts | 3 +- .../mod/forum/pages/discussion/discussion.ts | 9 +++--- src/addon/mod/forum/providers/forum.ts | 24 +++++++++----- src/addon/mod/forum/providers/helper.ts | 5 +-- .../mod/forum/providers/prefetch-handler.ts | 5 +-- src/classes/site.ts | 20 +++++++++++- .../core-siteplugins-module-index.html | 2 +- .../components/module-index/module-index.ts | 2 ++ .../plugin-content/plugin-content.ts | 9 +++++- src/core/siteplugins/providers/siteplugins.ts | 9 +++++- src/providers/sites.ts | 32 +++++++++++++++++-- 11 files changed, 97 insertions(+), 23 deletions(-) diff --git a/src/addon/mod/forum/components/index/index.ts b/src/addon/mod/forum/components/index/index.ts index 6795fccac..34ea7ba6c 100644 --- a/src/addon/mod/forum/components/index/index.ts +++ b/src/addon/mod/forum/components/index/index.ts @@ -354,7 +354,8 @@ export class AddonModForumIndexComponent extends CoreCourseModuleMainActivityCom this.page = 0; } - return this.forumProvider.getDiscussions(this.forum.id, this.selectedSortOrder.value, this.page).then((response) => { + return this.forumProvider.getDiscussions(this.forum.id, this.forum.cmid, + this.selectedSortOrder.value, this.page).then((response) => { let promise; if (this.usesGroups) { promise = this.forumProvider.formatDiscussionsGroups(this.forum.cmid, response.discussions); diff --git a/src/addon/mod/forum/pages/discussion/discussion.ts b/src/addon/mod/forum/pages/discussion/discussion.ts index 7ef3aa4de..2bfd3f449 100644 --- a/src/addon/mod/forum/pages/discussion/discussion.ts +++ b/src/addon/mod/forum/pages/discussion/discussion.ts @@ -323,7 +323,7 @@ export class AddonModForumDiscussionPage implements OnDestroy { let ratingInfo; return syncPromise.then(() => { - return this.forumProvider.getDiscussionPosts(this.discussionId).then((response) => { + return this.forumProvider.getDiscussionPosts(this.discussionId, this.cmId).then((response) => { onlinePosts = response.posts; ratingInfo = response.ratinginfo; }).then(() => { @@ -412,7 +412,7 @@ export class AddonModForumDiscussionPage implements OnDestroy { // The discussion object was not passed as parameter and there is no starting post. Should not happen. if (!this.discussion) { - promises.push(this.loadDiscussion(this.forumId, this.discussionId)); + promises.push(this.loadDiscussion(this.forumId, this.cmId, this.discussionId)); } return Promise.all(promises); @@ -479,13 +479,14 @@ export class AddonModForumDiscussionPage implements OnDestroy { * Convenience function to load discussion. * * @param forumId Forum ID. + * @param cmId Forum cmid. * @param discussionId Discussion ID. * @return Promise resolved when done. */ - protected loadDiscussion(forumId: number, discussionId: number): Promise { + protected loadDiscussion(forumId: number, cmId: number, discussionId: number): Promise { // Fetch the discussion if not passed as parameter. if (!this.discussion && forumId) { - return this.forumHelper.getDiscussionById(forumId, discussionId).then((discussion) => { + return this.forumHelper.getDiscussionById(forumId, cmId, discussionId).then((discussion) => { this.discussion = discussion; this.discussionId = this.discussion.discussion; }).catch(() => { diff --git a/src/addon/mod/forum/providers/forum.ts b/src/addon/mod/forum/providers/forum.ts index 2c9d03cdb..fa6cebdfc 100644 --- a/src/addon/mod/forum/providers/forum.ts +++ b/src/addon/mod/forum/providers/forum.ts @@ -492,15 +492,18 @@ export class AddonModForumProvider { * Get forum discussion posts. * * @param discussionId Discussion ID. + * @param cmId Forum cmid. * @param siteId Site ID. If not defined, current site. * @return Promise resolved with forum posts and rating info. */ - getDiscussionPosts(discussionId: number, siteId?: string): Promise<{posts: any[], ratinginfo?: CoreRatingInfo}> { + getDiscussionPosts(discussionId: number, cmId: number, siteId?: string): Promise<{posts: any[], ratinginfo?: CoreRatingInfo}> { const params = { discussionid: discussionId }; const preSets = { - cacheKey: this.getDiscussionPostsCacheKey(discussionId) + cacheKey: this.getDiscussionPostsCacheKey(discussionId), + component: AddonModForumProvider.COMPONENT, + componentId: cmId }; return this.sitesProvider.getSite(siteId).then((site) => { @@ -592,6 +595,7 @@ export class AddonModForumProvider { * Get forum discussions. * * @param forumId Forum ID. + * @param cmId Forum cmid * @param sortOrder Sort order. * @param page Page. * @param forceCache True to always get the value from cache. false otherwise. @@ -601,7 +605,8 @@ export class AddonModForumProvider { * discussion ID is discussion.discussion. * - canLoadMore: True if there may be more discussions to load. */ - getDiscussions(forumId: number, sortOrder?: number, page: number = 0, forceCache?: boolean, siteId?: string): Promise { + getDiscussions(forumId: number, cmId: number, sortOrder?: number, page: number = 0, + forceCache?: boolean, siteId?: string): Promise { sortOrder = sortOrder || AddonModForumProvider.SORTORDER_LASTPOST_DESC; return this.sitesProvider.getSite(siteId).then((site) => { @@ -626,7 +631,9 @@ export class AddonModForumProvider { } } const preSets: CoreSiteWSPreSets = { - cacheKey: this.getDiscussionsListCacheKey(forumId, sortOrder) + cacheKey: this.getDiscussionsListCacheKey(forumId, sortOrder), + component: AddonModForumProvider.COMPONENT, + componentId: cmId }; if (forceCache) { preSets.omitExpires = true; @@ -673,6 +680,7 @@ export class AddonModForumProvider { * If a page fails, the discussions until that page will be returned along with a flag indicating an error occurred. * * @param forumId Forum ID. + * @param cmId Forum cmid. * @param sortOrder Sort order. * @param forceCache True to always get the value from cache, false otherwise. * @param numPages Number of pages to get. If not defined, all pages. @@ -682,8 +690,8 @@ export class AddonModForumProvider { * - discussions: List of discussions. * - error: True if an error occurred, false otherwise. */ - getDiscussionsInPages(forumId: number, sortOrder?: number, forceCache?: boolean, numPages?: number, startPage?: number, - siteId?: string): Promise { + getDiscussionsInPages(forumId: number, cmId: number, sortOrder?: number, forceCache?: boolean, + numPages?: number, startPage?: number, siteId?: string): Promise { if (typeof numPages == 'undefined') { numPages = -1; } @@ -700,7 +708,7 @@ export class AddonModForumProvider { const getPage = (page: number): Promise => { // Get page discussions. - return this.getDiscussions(forumId, sortOrder, page, forceCache, siteId).then((response) => { + return this.getDiscussions(forumId, cmId, sortOrder, page, forceCache, siteId).then((response) => { result.discussions = result.discussions.concat(response.discussions); numPages--; @@ -753,7 +761,7 @@ export class AddonModForumProvider { this.getAvailableSortOrders().forEach((sortOrder) => { // We need to get the list of discussions to be able to invalidate their posts. - promises.push(this.getDiscussionsInPages(forum.id, sortOrder.value, true).then((response) => { + promises.push(this.getDiscussionsInPages(forum.id, forum.cmid, sortOrder.value, true).then((response) => { // Now invalidate the WS calls. const promises = []; diff --git a/src/addon/mod/forum/providers/helper.ts b/src/addon/mod/forum/providers/helper.ts index 99ca1b487..7b3db2efa 100644 --- a/src/addon/mod/forum/providers/helper.ts +++ b/src/addon/mod/forum/providers/helper.ts @@ -270,15 +270,16 @@ export class AddonModForumHelperProvider { * This function is inefficient because it needs to fetch all discussion pages in the worst case. * * @param forumId Forum ID. + * @param cmId Forum cmid * @param discussionId Discussion ID. * @param siteId Site ID. If not defined, current site. * @return Promise resolved with the discussion data. */ - getDiscussionById(forumId: number, discussionId: number, siteId?: string): Promise { + getDiscussionById(forumId: number, cmId: number, discussionId: number, siteId?: string): Promise { siteId = siteId || this.sitesProvider.getCurrentSiteId(); const findDiscussion = (page: number): Promise => { - return this.forumProvider.getDiscussions(forumId, undefined, page, false, siteId).then((response) => { + return this.forumProvider.getDiscussions(forumId, cmId, undefined, page, false, siteId).then((response) => { if (response.discussions && response.discussions.length > 0) { // Note that discussion.id is the main post ID but discussion ID is discussion.discussion. const discussion = response.discussions.find((discussion) => discussion.discussion == discussionId); diff --git a/src/addon/mod/forum/providers/prefetch-handler.ts b/src/addon/mod/forum/providers/prefetch-handler.ts index 00af16f83..5a75af10a 100644 --- a/src/addon/mod/forum/providers/prefetch-handler.ts +++ b/src/addon/mod/forum/providers/prefetch-handler.ts @@ -114,7 +114,8 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand protected getPostsForPrefetch(forum: any, siteId?: string): Promise { const promises = this.forumProvider.getAvailableSortOrders().map((sortOrder) => { // Get discussions in first 2 pages. - return this.forumProvider.getDiscussionsInPages(forum.id, sortOrder.value, false, 2, 0, siteId).then((response) => { + return this.forumProvider.getDiscussionsInPages(forum.id, forum.cmid, + sortOrder.value, false, 2, 0, siteId).then((response) => { if (response.error) { return Promise.reject(null); } @@ -122,7 +123,7 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand const promises = []; response.discussions.forEach((discussion) => { - promises.push(this.forumProvider.getDiscussionPosts(discussion.discussion, siteId)); + promises.push(this.forumProvider.getDiscussionPosts(discussion.discussion, forum.cmid, siteId)); }); return Promise.all(promises); diff --git a/src/classes/site.ts b/src/classes/site.ts index 463b5a027..c24685ef3 100644 --- a/src/classes/site.ts +++ b/src/classes/site.ts @@ -127,6 +127,17 @@ export interface CoreSiteWSPreSets { * Defaults to CoreSite.FREQUENCY_USUALLY. */ updateFrequency?: number; + + /** + * Component name. Optionally included if this request is being made on behalf of a specific + * component (e.g. activity). + */ + component?: string; + + /** + * Component id. Optionally included when 'component' is set. + */ + componentId?: number; } /** @@ -197,7 +208,7 @@ export class CoreSite { protected wsProvider: CoreWSProvider; // Variables for the database. - static WS_CACHE_TABLE = 'wscache'; + static WS_CACHE_TABLE = 'ws_cache'; static CONFIG_TABLE = 'core_site_config'; // Versions of Moodle releases. @@ -1128,6 +1139,13 @@ export class CoreSite { entry.key = preSets.cacheKey; } + if (preSets.component) { + entry.component = preSets.component; + if (preSets.componentId) { + entry.componentId = preSets.componentId; + } + } + return this.db.insertRecord(CoreSite.WS_CACHE_TABLE, entry); }); } diff --git a/src/core/siteplugins/components/module-index/core-siteplugins-module-index.html b/src/core/siteplugins/components/module-index/core-siteplugins-module-index.html index 538055ea5..1662b918c 100644 --- a/src/core/siteplugins/components/module-index/core-siteplugins-module-index.html +++ b/src/core/siteplugins/components/module-index/core-siteplugins-module-index.html @@ -9,4 +9,4 @@ - + diff --git a/src/core/siteplugins/components/module-index/module-index.ts b/src/core/siteplugins/components/module-index/module-index.ts index 48a6621e8..60347d9d4 100644 --- a/src/core/siteplugins/components/module-index/module-index.ts +++ b/src/core/siteplugins/components/module-index/module-index.ts @@ -37,6 +37,7 @@ export class CoreSitePluginsModuleIndexComponent implements OnInit, OnDestroy, C @ViewChild(CoreSitePluginsPluginContentComponent) content: CoreSitePluginsPluginContentComponent; component: string; + componentId: number; method: string; args: any; initResult: any; @@ -77,6 +78,7 @@ export class CoreSitePluginsModuleIndexComponent implements OnInit, OnDestroy, C if (handler) { this.component = handler.plugin.component; + this.componentId = this.module.id; this.method = handler.handlerSchema.method; this.args = { courseid: this.courseId, diff --git a/src/core/siteplugins/components/plugin-content/plugin-content.ts b/src/core/siteplugins/components/plugin-content/plugin-content.ts index 632a64222..cc3b74ff0 100644 --- a/src/core/siteplugins/components/plugin-content/plugin-content.ts +++ b/src/core/siteplugins/components/plugin-content/plugin-content.ts @@ -32,6 +32,7 @@ export class CoreSitePluginsPluginContentComponent implements OnInit, DoCheck { @ViewChild('compile') compileComponent: ElementRef; @Input() component: string; + @Input() componentId?: number; @Input() method: string; @Input() args: any; @Input() initResult: any; // Result of the init WS call of the handler. @@ -92,7 +93,13 @@ export class CoreSitePluginsPluginContentComponent implements OnInit, DoCheck { this.forceCompile = false; - return this.sitePluginsProvider.getContent(this.component, this.method, this.args, this.preSets).then((result) => { + const preSets = Object.assign({}, this.preSets); + preSets.component = this.component; + if (this.componentId) { + preSets.componentId = this.componentId; + } + + return this.sitePluginsProvider.getContent(this.component, this.method, this.args, preSets).then((result) => { this.content = result.templates.length ? result.templates[0].html : ''; // Load first template. this.javascript = result.javascript; this.otherData = result.otherdata; diff --git a/src/core/siteplugins/providers/siteplugins.ts b/src/core/siteplugins/providers/siteplugins.ts index b70686dd5..18a38b374 100644 --- a/src/core/siteplugins/providers/siteplugins.ts +++ b/src/core/siteplugins/providers/siteplugins.ts @@ -514,7 +514,14 @@ export class CoreSitePluginsProvider { promises.push(this.callWS(method, params, {cacheKey: cacheKey})); } else { // It's a method to get content. - promises.push(this.getContent(component, method, args).then((result) => { + const preSets = { + component: component, + componentId: undefined + }; + if (module) { + preSets.componentId = module.id; + } + promises.push(this.getContent(component, method, args, preSets).then((result) => { const subPromises = []; // Prefetch the files in the content. diff --git a/src/providers/sites.ts b/src/providers/sites.ts index b25283c92..22062270c 100644 --- a/src/providers/sites.ts +++ b/src/providers/sites.ts @@ -360,7 +360,7 @@ export class CoreSitesProvider { // Site schema for this provider. protected siteSchema: CoreSiteSchema = { name: 'CoreSitesProvider', - version: 1, + version: 2, canBeCleared: [ CoreSite.WS_CACHE_TABLE ], tables: [ { @@ -382,6 +382,14 @@ export class CoreSitesProvider { { name: 'expirationTime', type: 'INTEGER' + }, + { + name: 'component', + type: 'TEXT' + }, + { + name: 'componentId', + type: 'INTEGER' } ] }, @@ -399,7 +407,27 @@ export class CoreSitesProvider { } ] } - ] + ], + async migrate (db: SQLiteDB, oldVersion: number, siteId: string): Promise { + if (oldVersion && oldVersion < 2) { + const newTable = CoreSite.WS_CACHE_TABLE; + const oldTable = 'wscache'; + + try { + await db.tableExists(oldTable); + } catch (error) { + // Old table does not exist, ignore. + return; + } + // Cannot use insertRecordsFrom because there are extra fields, so manually code INSERT INTO. + await db.execute( + 'INSERT INTO ' + newTable + ' ' + + 'SELECT id, data, key, expirationTime, NULL as component, NULL as componentId ' + + 'FROM ' + oldTable); + + return await db.dropTable(oldTable); + } + } }; constructor(logger: CoreLoggerProvider,