From dbbd930f819106dac1ca546cff3b80b1a17f6806 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Thu, 19 Nov 2020 15:25:02 +0000 Subject: [PATCH] All: Added global logger Avoids having to pass around a logger to every module and sub-module, and makes it easier to set log level globally. --- packages/app-cli/app/app.js | 2 +- packages/app-cli/tests/test-utils.js | 2 + packages/app-desktop/app.ts | 6 -- packages/app-mobile/root.js | 2 + packages/lib/BaseApplication.ts | 62 ++++++++----------- packages/lib/Logger.ts | 62 ++++++++++++++----- packages/lib/database.js | 3 + packages/lib/services/plugins/Plugin.ts | 10 +-- .../lib/services/plugins/PluginService.ts | 23 ++++--- packages/lib/services/plugins/api/Global.ts | 5 +- packages/lib/services/plugins/api/Joplin.ts | 5 +- .../lib/services/plugins/api/JoplinPlugins.ts | 12 ++-- 12 files changed, 107 insertions(+), 87 deletions(-) diff --git a/packages/app-cli/app/app.js b/packages/app-cli/app/app.js index f8bf7397f..87098e3d2 100644 --- a/packages/app-cli/app/app.js +++ b/packages/app-cli/app/app.js @@ -426,7 +426,7 @@ class Application extends BaseApplication { const AppGui = require('./app-gui.js'); this.gui_ = new AppGui(this, this.store(), keymap); - this.gui_.setLogger(this.logger_); + this.gui_.setLogger(this.logger()); await this.gui_.start(); // Since the settings need to be loaded before the store is created, it will never diff --git a/packages/app-cli/tests/test-utils.js b/packages/app-cli/tests/test-utils.js index 6248d92c5..c2f9c07b1 100644 --- a/packages/app-cli/tests/test-utils.js +++ b/packages/app-cli/tests/test-utils.js @@ -147,6 +147,8 @@ logger.addTarget('console'); logger.addTarget('file', { path: `${logDir}/log.txt` }); logger.setLevel(Logger.LEVEL_WARN); // Set to DEBUG to display sync process in console +Logger.initializeGlobalLogger(logger); + BaseItem.loadClass('Note', Note); BaseItem.loadClass('Folder', Folder); BaseItem.loadClass('Resource', Resource); diff --git a/packages/app-desktop/app.ts b/packages/app-desktop/app.ts index 254748d66..67249fe0b 100644 --- a/packages/app-desktop/app.ts +++ b/packages/app-desktop/app.ts @@ -490,15 +490,9 @@ class Application extends BaseApplication { } private async initPluginService() { - const pluginLogger = new Logger(); - pluginLogger.addTarget(TargetType.File, { path: `${Setting.value('profileDir')}/log-plugins.txt` }); - pluginLogger.addTarget(TargetType.Console, { prefix: 'Plugin Service:' }); - pluginLogger.setLevel(Setting.value('env') == 'dev' ? Logger.LEVEL_DEBUG : Logger.LEVEL_INFO); - const service = PluginService.instance(); const pluginRunner = new PluginRunner(); - service.setLogger(pluginLogger); service.initialize(packageInfo.version, PlatformImplementation.instance(), pluginRunner, this.store()); const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states')); diff --git a/packages/app-mobile/root.js b/packages/app-mobile/root.js index 0f7d6c05e..914d0c5fb 100644 --- a/packages/app-mobile/root.js +++ b/packages/app-mobile/root.js @@ -399,6 +399,8 @@ async function initialize(dispatch) { mainLogger.setLevel(Logger.LEVEL_DEBUG); } + Logger.initializeGlobalLogger(mainLogger); + reg.setLogger(mainLogger); reg.setShowErrorMessageBoxHandler((message) => { alert(message); }); diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts index e1fa1c50f..7360218f7 100644 --- a/packages/lib/BaseApplication.ts +++ b/packages/lib/BaseApplication.ts @@ -46,13 +46,13 @@ const MigrationService = require('./services/MigrationService'); const { toSystemSlashes } = require('./path-utils'); const { setAutoFreeze } = require('immer'); +const appLogger = Logger.create('App'); + // const ntpClient = require('./vendor/ntp-client'); // ntpClient.dgram = require('dgram'); export default class BaseApplication { - private logger_: Logger; - private dbLogger_: Logger; private eventEmitter_: any; private scheduleAutoAddResourcesIID_: any = null; private database_: any = null; @@ -68,10 +68,7 @@ export default class BaseApplication { protected store_: any = null; constructor() { - this.logger_ = new Logger(); - this.dbLogger_ = new Logger(); this.eventEmitter_ = new EventEmitter(); - this.decryptionWorker_resourceMetadataButNotBlobDecrypted = this.decryptionWorker_resourceMetadataButNotBlobDecrypted.bind(this); } @@ -101,15 +98,13 @@ export default class BaseApplication { EncryptionService.instance_ = null; DecryptionWorker.instance_ = null; - this.logger_.info('Base application terminated...'); - this.logger_ = null; - this.dbLogger_ = null; + appLogger.info('Base application terminated...'); this.eventEmitter_ = null; this.decryptionWorker_resourceMetadataButNotBlobDecrypted = null; } logger() { - return this.logger_; + return appLogger; } public store() { @@ -289,7 +284,7 @@ export default class BaseApplication { parentType = BaseModel.TYPE_SMART_FILTER; } - this.logger().debug('Refreshing notes:', parentType, parentId); + appLogger.debug('Refreshing notes:', parentType, parentId); const options = { order: stateUtils.notesOrder(state.settings), @@ -460,7 +455,7 @@ export default class BaseApplication { } async generalMiddleware(store: any, next: any, action: any) { - // this.logger().debug('Reducer action', this.reducerActionToString(action)); + // appLogger.debug('Reducer action', this.reducerActionToString(action)); const result = next(action); const newState = store.getState(); @@ -701,35 +696,30 @@ export default class BaseApplication { const extraFlags = await this.readFlagsFromFile(`${profileDir}/flags.txt`); initArgs = Object.assign(initArgs, extraFlags); - this.logger_.addTarget(TargetType.File, { path: `${profileDir}/log.txt` }); - if (Setting.value('env') === 'dev' && !shim.isTestingEnv()) { - // this.logger_.addTarget(TargetType.Console, { level: Logger.LEVEL_DEBUG }); - } - this.logger_.setLevel(initArgs.logLevel); - reg.setLogger(this.logger_); + + + const globalLogger = new Logger(); + globalLogger.addTarget(TargetType.File, { path: `${profileDir}/log.txt` }); + if (Setting.value('appType') === 'desktop') { + globalLogger.addTarget(TargetType.Console); + } + globalLogger.setLevel(initArgs.logLevel); + Logger.initializeGlobalLogger(globalLogger); + + + + reg.setLogger(Logger.create('')); reg.dispatch = () => {}; - BaseService.logger_ = this.logger_; - // require('lib/ntpDate').setLogger(reg.logger()); + BaseService.logger_ = globalLogger; - this.dbLogger_.addTarget(TargetType.File, { path: `${profileDir}/log-database.txt` }); - this.dbLogger_.setLevel(initArgs.logLevel); - if (Setting.value('appType') === 'desktop') { - this.logger_.addTarget(TargetType.Console, { level: Logger.LEVEL_WARN }); - this.dbLogger_.addTarget(TargetType.Console, { level: Logger.LEVEL_WARN }); - } - - if (Setting.value('env') === 'dev') { - this.dbLogger_.setLevel(Logger.LEVEL_INFO); - } - - this.logger_.info(`Profile directory: ${profileDir}`); + appLogger.info(`Profile directory: ${profileDir}`); this.database_ = new JoplinDatabase(new DatabaseDriverNode()); this.database_.setLogExcludedQueryTypes(['SELECT']); - this.database_.setLogger(this.dbLogger_); + this.database_.setLogger(globalLogger); // if (Setting.value('env') === 'dev') { // if (shim.isElectron()) { @@ -756,7 +746,7 @@ export default class BaseApplication { await loadKeychainServiceAndSettings(KeychainServiceDriver); - this.logger_.info(`Client ID: ${Setting.value('clientId')}`); + appLogger.info(`Client ID: ${Setting.value('clientId')}`); if (Setting.value('firstStart')) { const locale = shim.detectAndSetLocale(Setting); @@ -817,9 +807,9 @@ export default class BaseApplication { KvStore.instance().setDb(reg.db()); - EncryptionService.instance().setLogger(this.logger_); + EncryptionService.instance().setLogger(globalLogger); BaseItem.encryptionService_ = EncryptionService.instance(); - DecryptionWorker.instance().setLogger(this.logger_); + DecryptionWorker.instance().setLogger(globalLogger); DecryptionWorker.instance().setEncryptionService(EncryptionService.instance()); DecryptionWorker.instance().setKvStore(KvStore.instance()); await EncryptionService.instance().loadMasterKeysFromSettings(); @@ -828,7 +818,7 @@ export default class BaseApplication { ResourceFetcher.instance().setFileApi(() => { return reg.syncTarget().fileApi(); }); - ResourceFetcher.instance().setLogger(this.logger_); + ResourceFetcher.instance().setLogger(globalLogger); ResourceFetcher.instance().on('downloadComplete', this.resourceFetcher_downloadComplete); ResourceFetcher.instance().start(); diff --git a/packages/lib/Logger.ts b/packages/lib/Logger.ts index f9ddb5301..dc294d872 100644 --- a/packages/lib/Logger.ts +++ b/packages/lib/Logger.ts @@ -26,6 +26,13 @@ interface Target { source?: string; } +interface LoggerWrapper { + debug: Function; + info: Function; + warn: Function; + error: Function; +} + class Logger { // For backward compatibility @@ -36,6 +43,7 @@ class Logger { public static LEVEL_DEBUG = LogLevel.Debug; public static fsDriver_: any = null; + private static globalLogger_: Logger = null; private targets_: Target[] = []; private level_: LogLevel = LogLevel.Info; @@ -46,6 +54,24 @@ class Logger { return Logger.fsDriver_; } + public static initializeGlobalLogger(logger: Logger) { + this.globalLogger_ = logger; + } + + private static get globalLogger(): Logger { + if (!this.globalLogger_) throw new Error('Global logger has not been initialized!!'); + return this.globalLogger_; + } + + static create(prefix: string): LoggerWrapper { + return { + debug: (...object: any[]) => this.globalLogger.log(LogLevel.Debug, prefix, ...object), + info: (...object: any[]) => this.globalLogger.log(LogLevel.Info, prefix, ...object), + warn: (...object: any[]) => this.globalLogger.log(LogLevel.Warn, prefix, ...object), + error: (...object: any[]) => this.globalLogger.log(LogLevel.Error, prefix, ...object), + }; + } + setLevel(level: LogLevel) { this.level_ = level; } @@ -132,14 +158,12 @@ class Logger { return this.level(); } - log(level: LogLevel, ...object: any[]) { + public log(level: LogLevel, prefix: string, ...object: any[]) { if (!this.targets_.length) return; - const timestamp = moment().format('YYYY-MM-DD HH:mm:ss'); - const line = `${timestamp}: `; - for (let i = 0; i < this.targets_.length; i++) { const target = this.targets_[i]; + const targetPrefix = prefix ? prefix : target.prefix; if (this.targetLevel(target) < level) continue; @@ -149,26 +173,30 @@ class Logger { if (level == LogLevel.Warn) fn = 'warn'; if (level == LogLevel.Info) fn = 'info'; const consoleObj = target.console ? target.console : console; - let items = [moment().format('HH:mm:ss')]; - if (target.prefix) { - items.push(target.prefix); - } - items = items.concat(...object); + const prefixItems = [moment().format('HH:mm:ss')]; + if (targetPrefix) prefixItems.push(targetPrefix); + const items = [`${prefixItems.join(': ')}:`].concat(...object); consoleObj[fn](...items); } else if (target.type == 'file') { - const serializedObject = this.objectsToString(...object); + const timestamp = moment().format('YYYY-MM-DD HH:mm:ss'); + const line = [timestamp]; + if (targetPrefix) line.push(targetPrefix); + line.push(this.objectsToString(...object)); try { - Logger.fsDriver().appendFileSync(target.path, `${line + serializedObject}\n`); + // TODO: Should log async + Logger.fsDriver().appendFileSync(target.path, `${line.join(': ')}\n`); } catch (error) { console.error('Cannot write to log file:', error); } } else if (target.type == 'database') { - const msg = this.objectsToString(...object); + const msg = []; + if (targetPrefix) msg.push(targetPrefix); + msg.push(this.objectsToString(...object)); const queries = [ { sql: 'INSERT INTO logs (`source`, `level`, `message`, `timestamp`) VALUES (?, ?, ?, ?)', - params: [target.source, level, msg, time.unixMs()], + params: [target.source, level, msg.join(': '), time.unixMs()], }, ]; @@ -188,16 +216,16 @@ class Logger { } error(...object: any[]) { - return this.log(LogLevel.Error, ...object); + return this.log(LogLevel.Error, null, ...object); } warn(...object: any[]) { - return this.log(LogLevel.Warn, ...object); + return this.log(LogLevel.Warn, null, ...object); } info(...object: any[]) { - return this.log(LogLevel.Info, ...object); + return this.log(LogLevel.Info, null, ...object); } debug(...object: any[]) { - return this.log(LogLevel.Debug, ...object); + return this.log(LogLevel.Debug, null, ...object); } static levelStringToId(s: string) { diff --git a/packages/lib/database.js b/packages/lib/database.js index 6a60d307a..f15e9cfc6 100644 --- a/packages/lib/database.js +++ b/packages/lib/database.js @@ -6,6 +6,7 @@ const shim = require('./shim').default; class Database { constructor(driver) { this.debugMode_ = false; + this.sqlQueryLogEnabled_ = false; this.driver_ = driver; this.logger_ = new Logger(); this.logExcludedQueryTypes_ = []; @@ -238,6 +239,8 @@ class Database { } logQuery(sql, params = null) { + if (!this.sqlQueryLogEnabled_) return; + if (this.logExcludedQueryTypes_.length) { const temp = sql.toLowerCase(); for (let i = 0; i < this.logExcludedQueryTypes_.length; i++) { diff --git a/packages/lib/services/plugins/Plugin.ts b/packages/lib/services/plugins/Plugin.ts index 18ded63b4..5ba883724 100644 --- a/packages/lib/services/plugins/Plugin.ts +++ b/packages/lib/services/plugins/Plugin.ts @@ -6,6 +6,8 @@ import { ContentScriptType } from './api/types'; import Logger from '../../Logger'; const EventEmitter = require('events'); +const logger = Logger.create('Plugin'); + interface ViewControllers { [key: string]: ViewController; } @@ -24,18 +26,16 @@ export default class Plugin { private baseDir_: string; private manifest_: PluginManifest; private scriptText_: string; - private logger_: Logger = null; private viewControllers_: ViewControllers = {}; private contentScripts_: ContentScripts = {}; private dispatch_: Function; private eventEmitter_: any; private devMode_: boolean = false; - constructor(baseDir: string, manifest: PluginManifest, scriptText: string, logger: Logger, dispatch: Function) { + constructor(baseDir: string, manifest: PluginManifest, scriptText: string, dispatch: Function) { this.baseDir_ = shim.fsDriver().resolve(baseDir); this.manifest_ = manifest; this.scriptText_ = scriptText; - this.logger_ = logger; this.dispatch_ = dispatch; this.eventEmitter_ = new EventEmitter(); } @@ -89,7 +89,7 @@ export default class Plugin { this.contentScripts_[type].push({ id, path: absolutePath }); - this.logger_.debug(`Plugin: ${this.id}: Registered content script: ${type}: ${id}: ${absolutePath}`); + logger.debug(`"${this.id}": Registered content script: ${type}: ${id}: ${absolutePath}`); this.dispatch_({ type: 'PLUGIN_CONTENT_SCRIPTS_ADD', @@ -117,7 +117,7 @@ export default class Plugin { } public deprecationNotice(goneInVersion: string, message: string) { - this.logger_.warn(`Plugin: ${this.id}: DEPRECATION NOTICE: ${message} This will stop working in version ${goneInVersion}.`); + logger.warn(`"${this.id}": DEPRECATION NOTICE: ${message} This will stop working in version ${goneInVersion}.`); } } diff --git a/packages/lib/services/plugins/PluginService.ts b/packages/lib/services/plugins/PluginService.ts index 8d9d4af26..6c84faca1 100644 --- a/packages/lib/services/plugins/PluginService.ts +++ b/packages/lib/services/plugins/PluginService.ts @@ -6,10 +6,13 @@ import BaseService from '../BaseService'; import shim from '../../shim'; import { filename, dirname, rtrimSlashes, basename } from '../../path-utils'; import Setting from '../../models/Setting'; +import Logger from '../../Logger'; const compareVersions = require('compare-versions'); const uslug = require('uslug'); const md5File = require('md5-file/promise'); +const logger = Logger.create('PluginService'); + // Plugin data is split into two: // // - First there's the service `plugins` property, which contains the @@ -213,7 +216,7 @@ export default class PluginService extends BaseService { distPath = `${path}/dist`; } - this.logger().info(`PluginService: Loading plugin from ${path}`); + logger.info(`Loading plugin from ${path}`); const scriptText = await fsDriver.readFile(`${distPath}/index.js`); const manifestText = await fsDriver.readFile(`${distPath}/manifest.json`); @@ -242,7 +245,7 @@ export default class PluginService extends BaseService { const manifest = manifestFromObject(manifestObj); - const plugin = new Plugin(baseDir, manifest, scriptText, this.logger(), (action: any) => this.store_.dispatch(action)); + const plugin = new Plugin(baseDir, manifest, scriptText, (action: any) => this.store_.dispatch(action)); for (const msg of deprecationNotices) { plugin.deprecationNotice('1.5', msg); @@ -274,7 +277,7 @@ export default class PluginService extends BaseService { for (const pluginPath of pluginPaths) { if (pluginPath.indexOf('_') === 0) { - this.logger().info(`PluginService: Plugin name starts with "_" and has not been loaded: ${pluginPath}`); + logger.info(`Plugin name starts with "_" and has not been loaded: ${pluginPath}`); continue; } @@ -289,7 +292,7 @@ export default class PluginService extends BaseService { this.setPluginAt(plugin.id, plugin); if (!this.pluginEnabled(settings, plugin.id)) { - this.logger().info(`PluginService: Not running disabled plugin: "${plugin.id}"`); + logger.info(`Not running disabled plugin: "${plugin.id}"`); continue; } @@ -297,14 +300,14 @@ export default class PluginService extends BaseService { await this.runPlugin(plugin); } catch (error) { - this.logger().error(`PluginService: Could not load plugin: ${pluginPath}`, error); + logger.error(`Could not load plugin: ${pluginPath}`, error); } } } public async runPlugin(plugin: Plugin) { if (compareVersions(this.appVersion_, plugin.manifest.app_min_version) < 0) { - throw new Error(`PluginService: Plugin "${plugin.id}" was disabled because it requires Joplin version ${plugin.manifest.app_min_version} and current version is ${this.appVersion_}.`); + throw new Error(`Plugin "${plugin.id}" was disabled because it requires Joplin version ${plugin.manifest.app_min_version} and current version is ${this.appVersion_}.`); } else { this.store_.dispatch({ type: 'PLUGIN_ADD', @@ -316,12 +319,12 @@ export default class PluginService extends BaseService { }); } - const pluginApi = new Global(this.logger(), this.platformImplementation_, plugin, this.store_); + const pluginApi = new Global(this.platformImplementation_, plugin, this.store_); return this.runner_.run(plugin, pluginApi); } public async installPlugin(jplPath: string): Promise { - this.logger().info(`PluginService: Installing plugin: "${jplPath}"`); + logger.info(`Installing plugin: "${jplPath}"`); const destPath = `${Setting.value('pluginDir')}/${basename(jplPath)}`; await shim.fsDriver().copy(jplPath, destPath); @@ -343,12 +346,12 @@ export default class PluginService extends BaseService { } public async uninstallPlugin(pluginId: string) { - this.logger().info(`PluginService: Uninstalling plugin: "${pluginId}"`); + logger.info(`Uninstalling plugin: "${pluginId}"`); const path = await this.pluginPath(pluginId); if (!path) { // Plugin might have already been deleted - this.logger().error(`PluginService: Could not find plugin path to uninstall - nothing will be done: ${pluginId}`); + logger.error(`Could not find plugin path to uninstall - nothing will be done: ${pluginId}`); } else { await shim.fsDriver().remove(path); } diff --git a/packages/lib/services/plugins/api/Global.ts b/packages/lib/services/plugins/api/Global.ts index 0a0f9ccfa..a7c7e0c87 100644 --- a/packages/lib/services/plugins/api/Global.ts +++ b/packages/lib/services/plugins/api/Global.ts @@ -1,6 +1,5 @@ import Plugin from '../Plugin'; import Joplin from './Joplin'; -import Logger from '../../../Logger'; /** * @ignore @@ -16,8 +15,8 @@ export default class Global { private requireWhiteList_: string[] = null; // private consoleWrapper_:any = null; - constructor(logger: Logger, implementation: any, plugin: Plugin, store: any) { - this.joplin_ = new Joplin(logger, implementation.joplin, plugin, store); + constructor(implementation: any, plugin: Plugin, store: any) { + this.joplin_ = new Joplin(implementation.joplin, plugin, store); // this.consoleWrapper_ = this.createConsoleWrapper(plugin.id); } diff --git a/packages/lib/services/plugins/api/Joplin.ts b/packages/lib/services/plugins/api/Joplin.ts index 0a782b168..b036f498f 100644 --- a/packages/lib/services/plugins/api/Joplin.ts +++ b/packages/lib/services/plugins/api/Joplin.ts @@ -7,7 +7,6 @@ import JoplinCommands from './JoplinCommands'; import JoplinViews from './JoplinViews'; import JoplinInterop from './JoplinInterop'; import JoplinSettings from './JoplinSettings'; -import Logger from '../../../Logger'; /** * This is the main entry point to the Joplin API. You can access various services using the provided accessors. @@ -35,9 +34,9 @@ export default class Joplin { private interop_: JoplinInterop = null; private settings_: JoplinSettings = null; - constructor(logger: Logger, implementation: any, plugin: Plugin, store: any) { + constructor(implementation: any, plugin: Plugin, store: any) { this.data_ = new JoplinData(); - this.plugins_ = new JoplinPlugins(logger, plugin); + this.plugins_ = new JoplinPlugins(plugin); this.workspace_ = new JoplinWorkspace(implementation.workspace, store); this.filters_ = new JoplinFilters(); this.commands_ = new JoplinCommands(); diff --git a/packages/lib/services/plugins/api/JoplinPlugins.ts b/packages/lib/services/plugins/api/JoplinPlugins.ts index 8bd71000a..be533e5de 100644 --- a/packages/lib/services/plugins/api/JoplinPlugins.ts +++ b/packages/lib/services/plugins/api/JoplinPlugins.ts @@ -2,16 +2,16 @@ import Plugin from '../Plugin'; import Logger from '../../../Logger'; import { ContentScriptType, Script } from './types'; +const logger = Logger.create('joplin.plugins'); + /** * This class provides access to plugin-related features. */ export default class JoplinPlugins { - private logger: Logger; private plugin: Plugin; - public constructor(logger: Logger, plugin: Plugin) { - this.logger = logger; + public constructor(plugin: Plugin) { this.plugin = plugin; } @@ -31,7 +31,7 @@ export default class JoplinPlugins { if (script.onStart) { const startTime = Date.now(); - this.logger.info(`Starting plugin: ${this.plugin.id}`); + logger.info(`Starting plugin: ${this.plugin.id}`); // We don't use `await` when calling onStart because the plugin might be awaiting // in that call too (for example, when opening a dialog on startup) so we don't @@ -42,9 +42,9 @@ export default class JoplinPlugins { // be handled correctly by loggers, etc. const newError: Error = new Error(error.message); newError.stack = error.stack; - this.logger.error(`Uncaught exception in plugin "${this.plugin.id}":`, newError); + logger.error(`Uncaught exception in plugin "${this.plugin.id}":`, newError); }).then(() => { - this.logger.info(`Finished running onStart handler: ${this.plugin.id} (Took ${Date.now() - startTime}ms)`); + logger.info(`Finished running onStart handler: ${this.plugin.id} (Took ${Date.now() - startTime}ms)`); this.plugin.emit('started'); }); }