From e08439a91fc85ee4d55c136cd8aa5518cc0f4106 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Wed, 8 Jul 2020 11:18:35 +0200 Subject: [PATCH] MOBILE-3477 core: Reduce concurrent calls to local-notifications plugin --- src/addon/calendar/providers/calendar.ts | 90 ++++++------- src/classes/queue-runner.ts | 143 ++++++++++++++++++++ src/core/settings/pages/general/general.ts | 2 - src/providers/local-notifications.ts | 148 +++++++++++++-------- 4 files changed, 279 insertions(+), 104 deletions(-) create mode 100644 src/classes/queue-runner.ts diff --git a/src/addon/calendar/providers/calendar.ts b/src/addon/calendar/providers/calendar.ts index ed3891a00..86ddb51ba 100644 --- a/src/addon/calendar/providers/calendar.ts +++ b/src/addon/calendar/providers/calendar.ts @@ -1606,61 +1606,55 @@ export class AddonCalendarProvider { * @param siteId Site ID the event belongs to. If not defined, use current site. * @return Promise resolved when the notification is scheduled. */ - protected scheduleEventNotification(event: AddonCalendarAnyEvent, reminderId: number, time: number, siteId?: string) + protected async scheduleEventNotification(event: AddonCalendarAnyEvent, reminderId: number, time: number, siteId?: string) : Promise { - if (this.localNotificationsProvider.isAvailable()) { - siteId = siteId || this.sitesProvider.getCurrentSiteId(); + if (!this.localNotificationsProvider.isAvailable()) { + return; + } - if (time === 0) { - // Cancel if it was scheduled. + siteId = siteId || this.sitesProvider.getCurrentSiteId(); + + if (time === 0) { + // Cancel if it was scheduled. + return this.localNotificationsProvider.cancel(reminderId, AddonCalendarProvider.COMPONENT, siteId); + } + + if (time == -1) { + // If time is -1, get event default time to calculate the notification time. + time = await this.getDefaultNotificationTime(siteId); + + if (time == 0) { + // Default notification time is disabled, do not show. return this.localNotificationsProvider.cancel(reminderId, AddonCalendarProvider.COMPONENT, siteId); } - let promise; - if (time == -1) { - // If time is -1, get event default time to calculate the notification time. - promise = this.getDefaultNotificationTime(siteId).then((time) => { - if (time == 0) { - // Default notification time is disabled, do not show. - return this.localNotificationsProvider.cancel(reminderId, AddonCalendarProvider.COMPONENT, siteId); - } - - return event.timestart - (time * 60); - }); - } else { - promise = Promise.resolve(time); - } - - return promise.then((time) => { - time = time * 1000; - - if (time <= new Date().getTime()) { - // This reminder is over, don't schedule. Cancel if it was scheduled. - return this.localNotificationsProvider.cancel(reminderId, AddonCalendarProvider.COMPONENT, siteId); - } - - const notification: ILocalNotification = { - id: reminderId, - title: event.name, - text: this.timeUtils.userDate(event.timestart * 1000, 'core.strftimedaydatetime', true), - icon: 'file://assets/img/icons/calendar.png', - trigger: { - at: new Date(time) - }, - data: { - eventid: event.id, - reminderid: reminderId, - siteid: siteId - } - }; - - return this.localNotificationsProvider.schedule(notification, AddonCalendarProvider.COMPONENT, siteId); - }); - - } else { - return Promise.resolve(); + time = event.timestart - (time * 60); } + + time = time * 1000; + + if (time <= Date.now()) { + // This reminder is over, don't schedule. Cancel if it was scheduled. + return this.localNotificationsProvider.cancel(reminderId, AddonCalendarProvider.COMPONENT, siteId); + } + + const notification: ILocalNotification = { + id: reminderId, + title: event.name, + text: this.timeUtils.userDate(event.timestart * 1000, 'core.strftimedaydatetime', true), + icon: 'file://assets/img/icons/calendar.png', + trigger: { + at: new Date(time) + }, + data: { + eventid: event.id, + reminderid: reminderId, + siteid: siteId + } + }; + + return this.localNotificationsProvider.schedule(notification, AddonCalendarProvider.COMPONENT, siteId); } /** diff --git a/src/classes/queue-runner.ts b/src/classes/queue-runner.ts new file mode 100644 index 000000000..88849abdd --- /dev/null +++ b/src/classes/queue-runner.ts @@ -0,0 +1,143 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { CoreUtils, PromiseDefer } from '@providers/utils/utils'; + +/** + * Function to add to the queue. + */ +export type CoreQueueRunnerFunction = (...args: any[]) => T | Promise; + +/** + * Queue item. + */ +export type CoreQueueRunnerItem = { + /** + * Item ID. + */ + id: string; + + /** + * Function to execute. + */ + fn: CoreQueueRunnerFunction; + + /** + * Deferred with a promise resolved/rejected with the result of the function. + */ + deferred: PromiseDefer; +}; + +/** + * Options to pass to add item. + */ +export type CoreQueueRunnerAddOptions = { + /** + * Whether to allow having multiple entries with same ID in the queue. + */ + allowRepeated?: boolean; +}; + +/** + * A queue to prevent having too many concurrent executions. + */ +export class CoreQueueRunner { + protected queue: {[id: string]: CoreQueueRunnerItem} = {}; + protected orderedQueue: CoreQueueRunnerItem[] = []; + protected numberRunning = 0; + + constructor(protected maxParallel: number = 1) { } + + /** + * Get unique ID. + * + * @param id ID. + * @return Unique ID. + */ + protected getUniqueId(id: string): string { + let newId = id; + let num = 1; + + do { + newId = id + '-' + num; + num++; + } while (newId in this.queue); + + return newId; + } + + /** + * Process next item in the queue. + * + * @return Promise resolved when next item has been treated. + */ + protected async processNextItem(): Promise { + if (!this.orderedQueue.length || this.numberRunning >= this.maxParallel) { + // Queue is empty or max number of parallel runs reached, stop. + return; + } + + const item = this.orderedQueue.shift(); + this.numberRunning++; + + try { + const result = await item.fn(); + + item.deferred.resolve(result); + } catch (error) { + item.deferred.reject(error); + } finally { + delete this.queue[item.id]; + this.numberRunning--; + + this.processNextItem(); + } + } + + /** + * Add an item to the queue. + * + * @param id ID. + * @param fn Function to call. + * @param options Options. + * @return Promise resolved when the function has been executed. + */ + run(id: string, fn: CoreQueueRunnerFunction, options?: CoreQueueRunnerAddOptions): Promise { + options = options || {}; + + if (id in this.queue) { + if (!options.allowRepeated) { + // Item already in queue, return its promise. + return this.queue[id].deferred.promise; + } + + id = this.getUniqueId(id); + } + + // Add the item in the queue. + const item = { + id, + fn, + deferred: CoreUtils.instance.promiseDefer(), + }; + + this.queue[id] = item; + this.orderedQueue.push(item); + + // Process next item if we haven't reached the max yet. + this.processNextItem(); + + return item.deferred.promise; + } +} diff --git a/src/core/settings/pages/general/general.ts b/src/core/settings/pages/general/general.ts index 2ddc61e0c..89836bfd9 100644 --- a/src/core/settings/pages/general/general.ts +++ b/src/core/settings/pages/general/general.ts @@ -20,7 +20,6 @@ import { CoreFileProvider } from '@providers/file'; import { CoreEventsProvider } from '@providers/events'; import { CoreLangProvider } from '@providers/lang'; import { CoreDomUtilsProvider } from '@providers/utils/dom'; -import { CoreLocalNotificationsProvider } from '@providers/local-notifications'; import { CorePushNotificationsProvider } from '@core/pushnotifications/providers/pushnotifications'; import { CoreConfigConstants } from '../../../../configconstants'; import { CoreSettingsHelper } from '../../providers/helper'; @@ -54,7 +53,6 @@ export class CoreSettingsGeneralPage { protected langProvider: CoreLangProvider, protected domUtils: CoreDomUtilsProvider, protected pushNotificationsProvider: CorePushNotificationsProvider, - localNotificationsProvider: CoreLocalNotificationsProvider, protected settingsHelper: CoreSettingsHelper) { // Get the supported languages. diff --git a/src/providers/local-notifications.ts b/src/providers/local-notifications.ts index 2ac4dbdfb..c75391606 100644 --- a/src/providers/local-notifications.ts +++ b/src/providers/local-notifications.ts @@ -24,6 +24,7 @@ import { CoreLoggerProvider } from './logger'; import { CoreTextUtilsProvider } from './utils/text'; import { CoreUtilsProvider } from './utils/utils'; import { SQLiteDB } from '@classes/sqlitedb'; +import { CoreQueueRunner } from '@classes/queue-runner'; import { CoreConstants } from '@core/constants'; import { CoreConfigConstants } from '../configconstants'; import { Subject, Subscription } from 'rxjs'; @@ -112,6 +113,7 @@ export class CoreLocalNotificationsProvider { protected cancelSubscription: Subscription; protected addSubscription: Subscription; protected updateSubscription: Subscription; + protected queueRunner: CoreQueueRunner; // Queue to decrease the number of concurrent calls to the plugin (see MOBILE-3477). constructor(logger: CoreLoggerProvider, private localNotifications: LocalNotifications, private platform: Platform, private appProvider: CoreAppProvider, private utils: CoreUtilsProvider, private configProvider: CoreConfigProvider, @@ -119,6 +121,7 @@ export class CoreLocalNotificationsProvider { eventsProvider: CoreEventsProvider, private push: Push, private zone: NgZone) { this.logger = logger.getInstance('CoreLocalNotificationsProvider'); + this.queueRunner = new CoreQueueRunner(10); this.appDB = appProvider.getDB(); this.dbReady = appProvider.createTablesFromSchema(this.tablesSchema).catch(() => { // Ignore errors. @@ -176,9 +179,13 @@ export class CoreLocalNotificationsProvider { * @param siteId Site ID. * @return Promise resolved when the notification is cancelled. */ - cancel(id: number, component: string, siteId: string): Promise { - return this.getUniqueNotificationId(id, component, siteId).then((uniqueId) => { - return this.localNotifications.cancel(uniqueId); + async cancel(id: number, component: string, siteId: string): Promise { + const uniqueId = await this.getUniqueNotificationId(id, component, siteId); + + const queueId = 'cancel-' + uniqueId; + + await this.queueRunner.run(queueId, () => this.localNotifications.cancel(uniqueId), { + allowRepeated: true, }); } @@ -188,28 +195,29 @@ export class CoreLocalNotificationsProvider { * @param siteId Site ID. * @return Promise resolved when the notifications are cancelled. */ - cancelSiteNotifications(siteId: string): Promise { + async cancelSiteNotifications(siteId: string): Promise { if (!this.isAvailable()) { - return Promise.resolve(); + return; } else if (!siteId) { - return Promise.reject(null); + throw new Error('No site ID supplied.'); } - return this.localNotifications.getScheduled().then((scheduled) => { - const ids = []; + const scheduled = await this.getAllScheduled(); - scheduled.forEach((notif) => { - if (typeof notif.data == 'string') { - notif.data = this.textUtils.parseJSON(notif.data); - } + const ids = []; + const queueId = 'cancelSiteNotifications-' + siteId; - if (typeof notif.data == 'object' && notif.data.siteId === siteId) { - ids.push(notif.id); - } - }); + scheduled.forEach((notif) => { + notif.data = this.parseNotificationData(notif.data); - return this.localNotifications.cancel(ids); + if (typeof notif.data == 'object' && notif.data.siteId === siteId) { + ids.push(notif.id); + } + }); + + await this.queueRunner.run(queueId, () => this.localNotifications.cancel(ids), { + allowRepeated: true, }); } @@ -243,6 +251,15 @@ export class CoreLocalNotificationsProvider { }); } + /** + * Get all scheduled notifications. + * + * @return Promise resolved with the notifications. + */ + protected getAllScheduled(): Promise { + return this.queueRunner.run('allScheduled', () => this.localNotifications.getScheduled()); + } + /** * Get a code to create unique notifications. If there's no code assigned, create a new one. * @@ -359,9 +376,10 @@ export class CoreLocalNotificationsProvider { * Check if a notification has been triggered with the same trigger time. * * @param notification Notification to check. + * @param useQueue Whether to add the call to the queue. * @return Promise resolved with a boolean indicating if promise is triggered (true) or not. */ - async isTriggered(notification: ILocalNotification): Promise { + async isTriggered(notification: ILocalNotification, useQueue: boolean = true): Promise { await this.dbReady; try { @@ -374,7 +392,15 @@ export class CoreLocalNotificationsProvider { return stored.at === triggered; } catch (err) { - return this.localNotifications.isTriggered(notification.id); + if (useQueue) { + const queueId = 'isTriggered-' + notification.id; + + return this.queueRunner.run(queueId, () => this.localNotifications.isTriggered(notification.id), { + allowRepeated: true, + }); + } else { + return this.localNotifications.isTriggered(notification.id); + } } } @@ -405,6 +431,22 @@ export class CoreLocalNotificationsProvider { }); } + /** + * Parse some notification data. + * + * @param data Notification data. + * @return Parsed data. + */ + protected parseNotificationData(data: any): any { + if (!data) { + return {}; + } else if (typeof data == 'string') { + return this.textUtils.parseJSON(data, {}); + } else { + return data; + } + } + /** * Process the next request in queue. */ @@ -531,20 +573,20 @@ export class CoreLocalNotificationsProvider { * * @return Promise resolved when all notifications have been rescheduled. */ - rescheduleAll(): Promise { + async rescheduleAll(): Promise { // Get all the scheduled notifications. - return this.localNotifications.getScheduled().then((notifications) => { - const promises = []; + const notifications = await this.getAllScheduled(); - notifications.forEach((notification) => { - // Convert some properties to the needed types. - notification.data = notification.data ? this.textUtils.parseJSON(notification.data, {}) : {}; + await Promise.all(notifications.map(async (notification) => { + // Convert some properties to the needed types. + notification.data = this.parseNotificationData(notification.data); - promises.push(this.scheduleNotification(notification)); + const queueId = 'schedule-' + notification.id; + + await this.queueRunner.run(queueId, () => this.scheduleNotification(notification), { + allowRepeated: true, }); - - return Promise.all(promises); - }); + })); } /** @@ -557,35 +599,33 @@ export class CoreLocalNotificationsProvider { * @param alreadyUnique Whether the ID is already unique. * @return Promise resolved when the notification is scheduled. */ - schedule(notification: ILocalNotification, component: string, siteId: string, alreadyUnique?: boolean): Promise { - let promise; + async schedule(notification: ILocalNotification, component: string, siteId: string, alreadyUnique?: boolean): Promise { - if (alreadyUnique) { - promise = Promise.resolve(notification.id); - } else { - promise = this.getUniqueNotificationId(notification.id, component, siteId); + if (!alreadyUnique) { + notification.id = await this.getUniqueNotificationId(notification.id, component, siteId); } - return promise.then((uniqueId) => { - notification.id = uniqueId; - notification.data = notification.data || {}; - notification.data.component = component; - notification.data.siteId = siteId; + notification.data = notification.data || {}; + notification.data.component = component; + notification.data.siteId = siteId; - if (this.platform.is('android')) { - notification.icon = notification.icon || 'res://icon'; - notification.smallIcon = notification.smallIcon || 'res://smallicon'; - notification.color = notification.color || CoreConfigConstants.notificoncolor; + if (this.platform.is('android')) { + notification.icon = notification.icon || 'res://icon'; + notification.smallIcon = notification.smallIcon || 'res://smallicon'; + notification.color = notification.color || CoreConfigConstants.notificoncolor; - const led: any = notification.led || {}; - notification.led = { - color: led.color || 'FF9900', - on: led.on || 1000, - off: led.off || 1000 - }; - } + const led: any = notification.led || {}; + notification.led = { + color: led.color || 'FF9900', + on: led.on || 1000, + off: led.off || 1000 + }; + } - return this.scheduleNotification(notification); + const queueId = 'schedule-' + notification.id; + + await this.queueRunner.run(queueId, () => this.scheduleNotification(notification), { + allowRepeated: true, }); } @@ -595,9 +635,9 @@ export class CoreLocalNotificationsProvider { * @param notification Notification to schedule. * @return Promise resolved when scheduled. */ - protected scheduleNotification(notification: ILocalNotification): Promise { + protected scheduleNotification(notification: ILocalNotification): Promise { // Check if the notification has been triggered already. - return this.isTriggered(notification).then((triggered) => { + return this.isTriggered(notification, false).then((triggered) => { // Cancel the current notification in case it gets scheduled twice. return this.localNotifications.cancel(notification.id).finally(() => { if (!triggered) {