Refactor logging to provide common logger from LoggerAdapter

Move logger logic de WinstonLoggerAdapter

Further improvements in configuration

Use logger instead of getLogger

- Removes PLog module

Reverts name changes

nits

Author: flovilmart

spec/Logger.spec.js

@@ -1,4 +1,4 @@
-var logger = require('../src/logger');
+var logging = require('../src/Adapters/Logger/WinstonLogger');
 var winston = require('winston');
 
 class TestTransport extends winston.Transport {
@@ -8,11 +8,15 @@ class TestTransport extends winston.Transport {
 }
 
 describe('Logger', () => {
+  // Test is excluded as will be refactored
   it('should add transport', () => {
-    const testTransport = new (TestTransport)({});
+    const testTransport = new (TestTransport)({
+      name: 'test'
+    });
     spyOn(testTransport, 'log');
-    logger.addTransport(testTransport);
-    logger.logger.info('hi');
+    logging.addTransport(testTransport);
+    logging.logger.info('hi');
     expect(testTransport.log).toHaveBeenCalled();
+    logging.removeTransport(testTransport);
   });
 });

spec/Subscription.spec.js

@@ -1,10 +1,10 @@
 var Subscription = require('../src/LiveQuery/Subscription').Subscription;
-
+let logger;
 describe('Subscription', function() {
 
   beforeEach(function() {
-    var mockError = jasmine.createSpy('error');
-    jasmine.mockLibrary('../src/LiveQuery/PLog', 'error', mockError);
+    logger = require('../src/logger').logger;
+    spyOn(logger, 'error').and.callThrough();
   });
 
   it('can be initialized', function() {
@@ -62,17 +62,15 @@ describe('Subscription', function() {
     var subscription = new Subscription('className', { key : 'value' }, 'hash');
     subscription.deleteClientSubscription(1, 1);
 
-    var PLog =require('../src/LiveQuery/PLog');
-    expect(PLog.error).toHaveBeenCalled();
+    expect(logger.error).toHaveBeenCalled();
   });
 
   it('can delete nonexistent request for one client', function() {
     var subscription = new Subscription('className', { key : 'value' }, 'hash');
     subscription.addClientSubscription(1, 1);
     subscription.deleteClientSubscription(1, 2);
 
-    var PLog =require('../src/LiveQuery/PLog');
-    expect(PLog.error).toHaveBeenCalled();
+    expect(logger.error).toHaveBeenCalled();
     expect(subscription.clientRequestIds.size).toBe(1);
     expect(subscription.clientRequestIds.get(1)).toEqual([1]);
   });
@@ -83,8 +81,7 @@ describe('Subscription', function() {
     subscription.addClientSubscription(1, 2);
     subscription.deleteClientSubscription(1, 2);
 
-    var PLog =require('../src/LiveQuery/PLog');
-    expect(PLog.error).not.toHaveBeenCalled();
+    expect(logger.error).not.toHaveBeenCalled();
     expect(subscription.clientRequestIds.size).toBe(1);
     expect(subscription.clientRequestIds.get(1)).toEqual([1]);
   });
@@ -96,8 +93,7 @@ describe('Subscription', function() {
     subscription.deleteClientSubscription(1, 1);
     subscription.deleteClientSubscription(1, 2);
 
-    var PLog =require('../src/LiveQuery/PLog');
-    expect(PLog.error).not.toHaveBeenCalled();
+    expect(logger.error).not.toHaveBeenCalled();
     expect(subscription.clientRequestIds.size).toBe(0);
   });
 
@@ -111,13 +107,8 @@ describe('Subscription', function() {
     subscription.deleteClientSubscription(2, 1);
     subscription.deleteClientSubscription(2, 2);
 
-    var PLog =require('../src/LiveQuery/PLog');
-    expect(PLog.error).not.toHaveBeenCalled();
+    expect(logger.error).not.toHaveBeenCalled();
     expect(subscription.clientRequestIds.size).toBe(1);
     expect(subscription.clientRequestIds.get(1)).toEqual([1]);
   });
-
-  afterEach(function(){
-    jasmine.restoreLibrary('../src/LiveQuery/PLog', 'error');
-  });
 });

spec/helper.js

@@ -342,8 +342,6 @@ global.fit_exclude_dbs = excluded => {
   }
 }
 
-// LiveQuery test setting
-require('../src/LiveQuery/PLog').logLevel = 'NONE';
 var libraryCache = {};
 jasmine.mockLibrary = function(library, name, mock) {
   var original = require(library)[name];

src/Adapters/Logger/LoggerAdapter.js

@@ -5,13 +5,16 @@
 // Adapter classes must implement the following functions:
 // * info(obj1 [, obj2, .., objN])
 // * error(obj1 [, obj2, .., objN])
-// * query(options, callback)
+// * query(options, callback) /* optional */
+// * configureLogger(options)
 // Default is WinstonLoggerAdapter.js
 
 export class LoggerAdapter {
+  constructor(options) {}
   info() {}
   error() {}
-  query(options, callback) {}
+  warn() {}
+  verbose() {}
 }
 
 export default LoggerAdapter;

src/Adapters/Logger/WinstonLogger.js

@@ -0,0 +1,93 @@
+import winston from 'winston';
+import fs from 'fs';
+import path from 'path';
+import DailyRotateFile from 'winston-daily-rotate-file';
+import _ from 'lodash';
+import defaults  from '../../logger';
+
+const logger = new winston.Logger();
+const additionalTransports = [];
+
+function updateTransports(options) {
+  let transports = Object.assign({}, logger.transports);
+  if (options) {
+    if (_.isNull(options.dirname)) {
+      delete transports['parse-server'];
+      delete transports['parse-server-error'];
+    } else if (!_.isUndefined(options.dirname)) {
+      transports['parse-server'] = new (DailyRotateFile)(
+        Object.assign({
+          filename: 'parse-server.info',
+          name: 'parse-server',
+        }, options));
+      transports['parse-server-error'] = new (DailyRotateFile)(
+        Object.assign({
+          filename: 'parse-server.err',
+          name: 'parse-server-error',
+          level: 'error'
+        }, options));
+    }
+
+    if (!process.env.TESTING || process.env.VERBOSE) {
+      transports.console = new (winston.transports.Console)(
+        Object.assign({
+          colorize: true,
+          name: 'console'
+        }, options));
+    }
+  }
+  // Mount the additional transports
+  additionalTransports.forEach((transport) => {
+    transports[transport.name] = transport;
+  });
+  logger.configure({
+    transports: _.values(transports)
+  });
+}
+
+export function configureLogger({
+  logsFolder = defaults.logsFolder,
+  jsonLogs = defaults.jsonLogs,
+  logLevel = winston.level,
+  verbose = defaults.verbose } = {}) {
+
+  if (verbose) {
+    logLevel = 'verbose';
+  }
+
+  winston.level = logLevel;
+  const options = {};
+
+  if (logsFolder) {
+    if (!path.isAbsolute(logsFolder)) {
+      logsFolder = path.resolve(process.cwd(), logsFolder);
+    }
+    try {
+      fs.mkdirSync(logsFolder);
+    } catch (exception) {}
+  }
+  options.dirname = logsFolder;
+  options.level = logLevel;
+  if (jsonLogs) {
+    options.json = true;
+    options.stringify = true;
+  }
+  updateTransports(options);
+}
+
+export function addTransport(transport) {
+  additionalTransports.push(transport);
+  updateTransports();
+}
+
+export function removeTransport(transport) {
+  let transportName = typeof transport == 'string' ? transport : transport.name;
+  let transports = Object.assign({}, logger.transports);
+  delete transports[transportName];
+  logger.configure({
+    transports: _.values(transports)
+  })
+}
+
+export { logger, addTransport, configureLogger, removeTransport };
+export default logger;

src/Adapters/Logger/WinstonLoggerAdapter.js

@@ -1,57 +1,21 @@
 import { LoggerAdapter } from './LoggerAdapter';
-import { logger, addTransport } from '../../logger';
+import { logger, addTransport, configureLogger } from './WinstonLogger';
 
 const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000;
-const CACHE_TIME = 1000 * 60;
-
-let currentDate = new Date();
-
-let simpleCache = {
-  timestamp: null,
-  from: null,
-  until: null,
-  order: null,
-  data: [],
-  level: 'info',
-};
 
 // returns Date object rounded to nearest day
 let _getNearestDay = (date) => {
   return new Date(date.getFullYear(), date.getMonth(), date.getDate());
 }
 
-// returns Date object of previous day
-let _getPrevDay = (date) => {
-  return new Date(date - MILLISECONDS_IN_A_DAY);
-}
-
-// returns the iso formatted file name
-let _getFileName = () => {
-  return _getNearestDay(currentDate).toISOString()
-}
-
-// check for valid cache when both from and util match.
-// cache valid for up to 1 minute
-let _hasValidCache = (from, until, level) => {
-  if (String(from) === String(simpleCache.from) &&
-    String(until) === String(simpleCache.until) &&
-    new Date() - simpleCache.timestamp < CACHE_TIME &&
-    level === simpleCache.level) {
-    return true;
+export class WinstonLoggerAdapter extends LoggerAdapter {
+  constructor(options) {
+    super();
+    if (options) {
+      configureLogger(options);
+    }
   }
-  return false;
-}
 
-// check that log entry has valid time stamp based on query
-let _isValidLogEntry = (from, until, entry) => {
-  var _entry = JSON.parse(entry),
-    timestamp = new Date(_entry.timestamp);
-  return timestamp >= from && timestamp <= until
-    ? true
-    : false
-};
-
-export class WinstonLoggerAdapter extends LoggerAdapter {
   info() {
     return logger.info.apply(undefined, arguments);
   }
@@ -60,6 +24,18 @@ export class WinstonLoggerAdapter extends LoggerAdapter {
     return logger.error.apply(undefined, arguments);
   }
 
+  warn() {
+    return logger.warn.apply(undefined, arguments);
+  }
+
+  verbose() {
+    return logger.verbose.apply(undefined, arguments);
+  }
+
+  log() {
+    return logger.log.apply(undefined, arguments);
+  }
+
   addTransport(transport) {
     // Note that this is calling addTransport
     // from logger.  See import - confusing.

src/Controllers/LoggerController.js

@@ -60,6 +60,10 @@ export class LoggerController extends AdaptableController {
       throw new Parse.Error(Parse.Error.PUSH_MISCONFIGURED,
         'Logger adapter is not availabe');
     }
+    if (typeof this.adapter.query !== 'function') {
+      throw new Parse.Error(Parse.Error.PUSH_MISCONFIGURED,
+        'Querying logs is not supported with this adapter');
+    }
     options = LoggerController.parseOptions(options);
     return this.adapter.query(options);
   }

src/LiveQuery/Client.js

@@ -1,5 +1,5 @@
-import PLog from './PLog';
 import Parse from 'parse/node';
+import logger from '../logger';
 
 import type { FlattenedObjectData } from './Subscription';
 export type Message = { [attr: string]: any };
@@ -37,7 +37,7 @@ class Client {
   }
 
   static pushResponse(parseWebSocket: any, message: Message): void {
-    PLog.verbose('Push Response : %j', message);
+    logger.verbose('Push Response : %j', message);
     parseWebSocket.send(message);
   }
 

src/LiveQuery/PLog.js

@@ -1,5 +1,5 @@
 import { ParsePubSub } from './ParsePubSub';
-import PLog from './PLog';
+import logger from '../logger';
 
 class ParseCloudCodePublisher {
   parsePublisher: Object;
@@ -20,7 +20,7 @@ class ParseCloudCodePublisher {
 
   // Request is the request object from cloud code functions. request.object is a ParseObject.
   _onCloudCodeMessage(type: string, request: any): void {
-    PLog.verbose('Raw request from cloud code current : %j | original : %j', request.object, request.original);
+    logger.verbose('Raw request from cloud code current : %j | original : %j', request.object, request.original);
     // We need the full JSON which includes className
     let message = {
       currentParseObject: request.object._toFullJSON()