From 489de4fc092f35a4400aa3661b001bc0e5f3f623 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 13 Mar 2018 09:12:30 +0100 Subject: [PATCH] MOBILE-2333 core: Use parseJSON from text utils --- src/addon/messages/providers/sync.ts | 5 +- src/addon/mod/book/providers/book.ts | 2 +- src/classes/base-sync.ts | 10 ++-- src/classes/site.ts | 2 +- src/core/login/providers/helper.ts | 5 +- src/core/siteplugins/providers/helper.ts | 47 +++++++++---------- src/core/siteplugins/providers/siteplugins.ts | 11 ++--- src/directives/format-text.ts | 2 +- src/providers/filepool.ts | 4 +- src/providers/local-notifications.ts | 7 +-- src/providers/sites.ts | 43 ++++++----------- src/providers/utils/text.ts | 14 ++++-- src/providers/ws.ts | 15 ++---- 13 files changed, 74 insertions(+), 93 deletions(-) diff --git a/src/addon/messages/providers/sync.ts b/src/addon/messages/providers/sync.ts index 45dcfbb24..6e2dfcbfe 100644 --- a/src/addon/messages/providers/sync.ts +++ b/src/addon/messages/providers/sync.ts @@ -21,6 +21,7 @@ import { AddonMessagesOfflineProvider } from './messages-offline'; import { AddonMessagesProvider } from './messages'; import { CoreUserProvider } from '@core/user/providers/user'; import { CoreEventsProvider } from '@providers/events'; +import { CoreTextUtilsProvider } from '@providers/utils/text'; import { CoreUtilsProvider } from '@providers/utils/utils'; import { TranslateService } from '@ngx-translate/core'; import { CoreSyncProvider } from '@providers/sync'; @@ -37,8 +38,8 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { protected appProvider: CoreAppProvider, private messagesOffline: AddonMessagesOfflineProvider, private eventsProvider: CoreEventsProvider, private messagesProvider: AddonMessagesProvider, private userProvider: CoreUserProvider, private translate: TranslateService, private utils: CoreUtilsProvider, - syncProvider: CoreSyncProvider) { - super('AddonMessagesSync', sitesProvider, loggerProvider, appProvider, syncProvider); + syncProvider: CoreSyncProvider, protected textUtils: CoreTextUtilsProvider) { + super('AddonMessagesSync', sitesProvider, loggerProvider, appProvider, syncProvider, textUtils); } /** diff --git a/src/addon/mod/book/providers/book.ts b/src/addon/mod/book/providers/book.ts index b6d36e194..808a77128 100644 --- a/src/addon/mod/book/providers/book.ts +++ b/src/addon/mod/book/providers/book.ts @@ -288,7 +288,7 @@ export class AddonModBookProvider { return []; } - return JSON.parse(contents[0].content); + return this.textUtils.parseJSON(contents[0].content, []); } /** diff --git a/src/classes/base-sync.ts b/src/classes/base-sync.ts index e1e70a2c5..654b3bba8 100644 --- a/src/classes/base-sync.ts +++ b/src/classes/base-sync.ts @@ -16,6 +16,7 @@ import { CoreSitesProvider } from '@providers/sites'; import { CoreSyncProvider } from '@providers/sync'; import { CoreLoggerProvider } from '@providers/logger'; import { CoreAppProvider } from '@providers/app'; +import { CoreTextUtilsProvider } from '@providers/utils/text'; /** * Base class to create sync providers. It provides some common functions. @@ -44,7 +45,8 @@ export class CoreSyncBaseProvider { protected syncPromises: { [siteId: string]: { [uniqueId: string]: Promise } } = {}; constructor(component: string, protected sitesProvider: CoreSitesProvider, protected loggerProvider: CoreLoggerProvider, - protected appProvider: CoreAppProvider, protected syncProvider: CoreSyncProvider) { + protected appProvider: CoreAppProvider, protected syncProvider: CoreSyncProvider, + protected textUtils: CoreTextUtilsProvider) { this.logger = this.loggerProvider.getInstance(component); this.component = component; } @@ -115,11 +117,7 @@ export class CoreSyncBaseProvider { */ getSyncWarnings(id: string | number, siteId?: string): Promise { return this.syncProvider.getSyncRecord(this.component, id, siteId).then((entry) => { - try { - return JSON.parse(entry.warnings); - } catch (ex) { - return []; - } + return this.textUtils.parseJSON(entry.warnings, []); }).catch(() => { return []; }); diff --git a/src/classes/site.ts b/src/classes/site.ts index 3c3480f25..682561a4e 100644 --- a/src/classes/site.ts +++ b/src/classes/site.ts @@ -734,7 +734,7 @@ export class CoreSite { const expires = (entry.expirationTime - now) / 1000; this.logger.info(`Cached element found, id: ${id} expires in ${expires} seconds`); - return JSON.parse(entry.data); + return this.textUtils.parseJSON(entry.data, {}); } return Promise.reject(null); diff --git a/src/core/login/providers/helper.ts b/src/core/login/providers/helper.ts index 41443cce2..482a7b356 100644 --- a/src/core/login/providers/helper.ts +++ b/src/core/login/providers/helper.ts @@ -942,9 +942,8 @@ export class CoreLoginHelperProvider { const params = url.split(':::'); return this.configProvider.get(CoreConstants.LOGIN_LAUNCH_DATA).then((data): any => { - try { - data = JSON.parse(data); - } catch (ex) { + data = this.textUtils.parseJSON(data, null); + if (data === null) { return Promise.reject(null); } diff --git a/src/core/siteplugins/providers/helper.ts b/src/core/siteplugins/providers/helper.ts index 422b7791d..88f1c0013 100644 --- a/src/core/siteplugins/providers/helper.ts +++ b/src/core/siteplugins/providers/helper.ts @@ -18,6 +18,7 @@ import { CoreLangProvider } from '@providers/lang'; import { CoreLoggerProvider } from '@providers/logger'; import { CoreSite } from '@classes/site'; import { CoreSitesProvider } from '@providers/sites'; +import { CoreTextUtilsProvider } from '@providers/utils/text'; import { CoreUtilsProvider } from '@providers/utils/utils'; import { CoreSitePluginsProvider } from './siteplugins'; import { CoreCompileProvider } from '@core/compile/providers/compile'; @@ -59,7 +60,8 @@ export class CoreSitePluginsHelperProvider { private sitePluginsProvider: CoreSitePluginsProvider, private prefetchDelegate: CoreCourseModulePrefetchDelegate, private compileProvider: CoreCompileProvider, private utils: CoreUtilsProvider, private courseOptionsDelegate: CoreCourseOptionsDelegate, eventsProvider: CoreEventsProvider, - private courseFormatDelegate: CoreCourseFormatDelegate, private profileFieldDelegate: CoreUserProfileFieldDelegate) { + private courseFormatDelegate: CoreCourseFormatDelegate, private profileFieldDelegate: CoreUserProfileFieldDelegate, + private textUtils: CoreTextUtilsProvider) { this.logger = logger.getInstance('CoreSitePluginsHelperProvider'); // Fetch the plugins on login. @@ -201,15 +203,12 @@ export class CoreSitePluginsHelperProvider { isSitePluginEnabled(plugin: any, site: CoreSite): boolean { if (!site.isFeatureDisabled('sitePlugin_' + plugin.component + '_' + plugin.addon) && plugin.handlers) { // Site plugin not disabled. Check if it has handlers. - try { - if (!plugin.parsedHandlers) { - plugin.parsedHandlers = JSON.parse(plugin.handlers); - } - - return !!(plugin.parsedHandlers && Object.keys(plugin.parsedHandlers).length); - } catch (ex) { - this.logger.warn('Error parsing site plugin', ex); + if (!plugin.parsedHandlers) { + plugin.parsedHandlers = this.textUtils.parseJSON(plugin.handlers, null, + this.logger.error.bind(this.logger, 'Error parsing site plugin handlers')); } + + return !!(plugin.parsedHandlers && Object.keys(plugin.parsedHandlers).length); } return false; @@ -243,25 +242,23 @@ export class CoreSitePluginsHelperProvider { this.logger.debug('Load site plugin:', plugin); - try { - if (!plugin.parsedHandlers) { - plugin.parsedHandlers = JSON.parse(plugin.handlers); - } - if (!plugin.parsedLang && plugin.lang) { - plugin.parsedLang = JSON.parse(plugin.lang); - } + if (!plugin.parsedHandlers) { + plugin.parsedHandlers = this.textUtils.parseJSON(plugin.handlers, null, + this.logger.error.bind(this.logger, 'Error parsing site plugin handlers')); + } + if (!plugin.parsedLang && plugin.lang) { + plugin.parsedLang = this.textUtils.parseJSON(plugin.lang, null, + this.logger.error.bind(this.logger, 'Error parsing site plugin lang')); + } - this.sitePluginsProvider.hasSitePluginsLoaded = true; + this.sitePluginsProvider.hasSitePluginsLoaded = true; - // Register lang strings. - this.loadLangStrings(plugin); + // Register lang strings. + this.loadLangStrings(plugin); - // Register all the handlers. - for (const name in plugin.parsedHandlers) { - promises.push(this.registerHandler(plugin, name, plugin.parsedHandlers[name])); - } - } catch (ex) { - this.logger.warn('Error parsing site plugin', ex); + // Register all the handlers. + for (const name in plugin.parsedHandlers) { + promises.push(this.registerHandler(plugin, name, plugin.parsedHandlers[name])); } return this.utils.allPromises(promises); diff --git a/src/core/siteplugins/providers/siteplugins.ts b/src/core/siteplugins/providers/siteplugins.ts index 42640537d..572d7d780 100644 --- a/src/core/siteplugins/providers/siteplugins.ts +++ b/src/core/siteplugins/providers/siteplugins.ts @@ -20,6 +20,7 @@ import { CoreLangProvider } from '@providers/lang'; import { CoreLoggerProvider } from '@providers/logger'; import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { CoreSitesProvider } from '@providers/sites'; +import { CoreTextUtilsProvider } from '@providers/utils/text'; import { CoreUtilsProvider } from '@providers/utils/utils'; import { CoreConfigConstants } from '../../../configconstants'; import { CoreCoursesProvider } from '@core/courses/providers/courses'; @@ -66,7 +67,8 @@ export class CoreSitePluginsProvider { constructor(logger: CoreLoggerProvider, private sitesProvider: CoreSitesProvider, private utils: CoreUtilsProvider, private langProvider: CoreLangProvider, private appProvider: CoreAppProvider, private platform: Platform, - private filepoolProvider: CoreFilepoolProvider, private coursesProvider: CoreCoursesProvider) { + private filepoolProvider: CoreFilepoolProvider, private coursesProvider: CoreCoursesProvider, + private textUtils: CoreTextUtilsProvider) { this.logger = logger.getInstance('CoreUserProvider'); } @@ -215,11 +217,8 @@ export class CoreSitePluginsProvider { return this.sitesProvider.getCurrentSite().read('tool_mobile_get_content', data, preSets); }).then((result) => { if (result.otherdata) { - try { - result.otherdata = JSON.parse(result.otherdata) || {}; - } catch (ex) { - // Ignore errors. - } + result.otherdata = this.textUtils.parseJSON(result.otherdata, {}, + this.logger.error.bind(this.logger, 'Error parsing get_content otherdata', method)); } else { result.otherdata = {}; } diff --git a/src/directives/format-text.ts b/src/directives/format-text.ts index 3261db59b..330712d1f 100644 --- a/src/directives/format-text.ts +++ b/src/directives/format-text.ts @@ -383,7 +383,7 @@ export class CoreFormatTextDirective implements OnChanges { return; } - const data = JSON.parse(video.getAttribute('data-setup') || video.getAttribute('data-setup-lazy') || '{}'), + const data = this.textUtils.parseJSON(video.getAttribute('data-setup') || video.getAttribute('data-setup-lazy') || '{}'), youtubeId = data.techOrder && data.techOrder[0] && data.techOrder[0] == 'youtube' && data.sources && data.sources[0] && data.sources[0].src && this.youtubeGetId(data.sources[0].src); diff --git a/src/providers/filepool.ts b/src/providers/filepool.ts index c6fa08632..bb1ded4a5 100644 --- a/src/providers/filepool.ts +++ b/src/providers/filepool.ts @@ -2183,7 +2183,7 @@ export class CoreFilepoolProvider { return Promise.reject(null); } // Convert the links to an object. - entry.links = JSON.parse(entry.links); + entry.links = this.textUtils.parseJSON(entry.links, []); return entry; }); @@ -2421,7 +2421,7 @@ export class CoreFilepoolProvider { return Promise.reject(this.ERR_QUEUE_IS_EMPTY); } // Convert the links to an object. - item.links = JSON.parse(item.links); + item.links = this.textUtils.parseJSON(item.links, []); return this.processQueueItem(item); }, () => { diff --git a/src/providers/local-notifications.ts b/src/providers/local-notifications.ts index 4bd55cfac..c3339dfcf 100644 --- a/src/providers/local-notifications.ts +++ b/src/providers/local-notifications.ts @@ -19,6 +19,7 @@ import { CoreAppProvider } from './app'; import { CoreConfigProvider } from './config'; import { CoreLoggerProvider } from './logger'; import { CoreDomUtilsProvider } from './utils/dom'; +import { CoreTextUtilsProvider } from './utils/text'; import { CoreUtilsProvider } from './utils/utils'; import { SQLiteDB } from '@classes/sqlitedb'; import { CoreConstants } from '@core/constants'; @@ -112,7 +113,7 @@ export class CoreLocalNotificationsProvider { constructor(logger: CoreLoggerProvider, private localNotifications: LocalNotifications, private platform: Platform, private appProvider: CoreAppProvider, private utils: CoreUtilsProvider, private configProvider: CoreConfigProvider, - private domUtils: CoreDomUtilsProvider) { + private domUtils: CoreDomUtilsProvider, private textUtils: CoreTextUtilsProvider) { this.logger = logger.getInstance('CoreLocalNotificationsProvider'); this.appDB = appProvider.getDB(); this.appDB.createTablesFromSchema(this.tablesSchema); @@ -151,7 +152,7 @@ export class CoreLocalNotificationsProvider { scheduled.forEach((notif) => { if (typeof notif.data == 'string') { - notif.data = JSON.parse(notif.data); + notif.data = this.textUtils.parseJSON(notif.data); } if (typeof notif.data == 'object' && notif.data.siteId === siteId) { @@ -403,7 +404,7 @@ export class CoreLocalNotificationsProvider { notifications.forEach((notification) => { // Convert some properties to the needed types. notification.at = new Date(notification.at * 1000); - notification.data = notification.data ? JSON.parse(notification.data) : {}; + notification.data = notification.data ? this.textUtils.parseJSON(notification.data, {}) : {}; promises.push(this.scheduleNotification(notification)); }); diff --git a/src/providers/sites.ts b/src/providers/sites.ts index 435024ee0..c4ecc8268 100644 --- a/src/providers/sites.ts +++ b/src/providers/sites.ts @@ -19,6 +19,7 @@ import { CoreAppProvider } from './app'; import { CoreEventsProvider } from './events'; import { CoreLoggerProvider } from './logger'; import { CoreSitesFactoryProvider } from './sites-factory'; +import { CoreTextUtilsProvider } from './utils/text'; import { CoreUrlUtilsProvider } from './utils/url'; import { CoreUtilsProvider } from './utils/utils'; import { CoreConstants } from '@core/constants'; @@ -211,7 +212,8 @@ export class CoreSitesProvider { constructor(logger: CoreLoggerProvider, private http: HttpClient, private sitesFactory: CoreSitesFactoryProvider, private appProvider: CoreAppProvider, private utils: CoreUtilsProvider, private translate: TranslateService, - private eventsProvider: CoreEventsProvider, private urlUtils: CoreUrlUtilsProvider) { + private eventsProvider: CoreEventsProvider, private urlUtils: CoreUrlUtilsProvider, + private textUtils: CoreTextUtilsProvider) { this.logger = logger.getInstance('CoreSitesProvider'); this.appDB = appProvider.getDB(); @@ -787,18 +789,9 @@ export class CoreSitesProvider { info = entry.info, config = entry.config; - // Try to parse info and config. - try { - info = info ? JSON.parse(info) : info; - } catch (ex) { - // Ignore errors. - } - - try { - config = config ? JSON.parse(config) : config; - } catch (ex) { - // Ignore errors. - } + // Parse info and config. + info = info ? this.textUtils.parseJSON(info) : info; + config = config ? this.textUtils.parseJSON(config) : config; site = this.sitesFactory.makeSite(entry.id, entry.siteUrl, entry.token, info, entry.privateToken, config, entry.loggedOut == 1); @@ -862,21 +855,15 @@ export class CoreSitesProvider { const formattedSites = []; sites.forEach((site) => { if (!ids || ids.indexOf(site.id) > -1) { - // Try to parse info. - let siteInfo = site.info; - try { - siteInfo = siteInfo ? JSON.parse(siteInfo) : siteInfo; - } catch (ex) { - // Ignore errors. - } - - const basicInfo: CoreSiteBasicInfo = { - id: site.id, - siteUrl: site.siteUrl, - fullName: siteInfo && siteInfo.fullname, - siteName: siteInfo && siteInfo.sitename, - avatar: siteInfo && siteInfo.userpictureurl - }; + // Parse info. + const siteInfo = site.info ? this.textUtils.parseJSON(site.info) : site.info, + basicInfo: CoreSiteBasicInfo = { + id: site.id, + siteUrl: site.siteUrl, + fullName: siteInfo && siteInfo.fullname, + siteName: siteInfo && siteInfo.sitename, + avatar: siteInfo && siteInfo.userpictureurl + }; formattedSites.push(basicInfo); } }); diff --git a/src/providers/utils/text.ts b/src/providers/utils/text.ts index 11191ddaa..bb7d26c17 100644 --- a/src/providers/utils/text.ts +++ b/src/providers/utils/text.ts @@ -383,19 +383,25 @@ export class CoreTextUtilsProvider { } /** - * Same as Javascript's JSON.parse, but if an exception is thrown it will return the original text. + * Same as Javascript's JSON.parse, but it will handle errors. * * @param {string} json JSON text. + * @param {any} [defaultValue] Default value t oreturn if the parse fails. Defaults to the original value. + * @param {Function} [logErrorFn] An error to call with the exception to log the error. If not supplied, no error. * @return {any} JSON parsed as object or what it gets. */ - parseJSON(json: string): any { + parseJSON(json: string, defaultValue?: any, logErrorFn?: Function): any { try { return JSON.parse(json); } catch (ex) { - // Error, use the json text. + // Error, log the error if needed. + if (logErrorFn) { + logErrorFn(ex); + } } - return json; + // Error parsing, return the default value or the original value. + return typeof defaultValue != 'undefined' ? defaultValue : json; } /** diff --git a/src/providers/ws.ts b/src/providers/ws.ts index 01ccd7c71..660887a06 100644 --- a/src/providers/ws.ts +++ b/src/providers/ws.ts @@ -682,11 +682,7 @@ export class CoreWSProvider { } // Treat response. - try { - data = JSON.parse(data); - } catch (ex) { - // Ignore errors. - } + data = this.textUtils.parseJSON(data); // Some moodle web services return null. // If the responseExpected value is set then so long as no data is returned, we create a blank object. @@ -750,12 +746,9 @@ export class CoreWSProvider { }; return transfer.upload(filePath, uploadUrl, options, true).then((success) => { - let data: any = success.response; - try { - data = JSON.parse(data); - } catch (err) { - this.logger.error('Error parsing response from upload:', err, data); - + const data = this.textUtils.parseJSON(success.response, null, + this.logger.error.bind(this.logger, 'Error parsing response from upload')); + if (data === null) { return Promise.reject(this.translate.instant('core.errorinvalidresponse')); }