Skip to content

Refactor logging to provide common logger from LoggerAdapter #2478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Aug 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions spec/AdapterLoader.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ describe("AdapterLoader", ()=>{
done();
});

it("should instantiate an adapter from npm module", (done) => {
var adapter = loadAdapter({
module: 'parse-server-fs-adapter'
});

expect(typeof adapter).toBe('object');
expect(typeof adapter.createFile).toBe('function');
expect(typeof adapter.deleteFile).toBe('function');
expect(typeof adapter.getFileData).toBe('function');
expect(typeof adapter.getFileLocation).toBe('function');
done();
});

it("should instantiate an adapter from function/Class", (done) => {
var adapter = loadAdapter({
adapter: FilesAdapter
Expand Down
8 changes: 7 additions & 1 deletion spec/InstallationsRouter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var InstallationsRouter = require('../src/Routers/InstallationsRouter').Installa

var config = new Config('test');

describe('InstallationsRouter', () => {
describe_only_db(['mongo'])('InstallationsRouter', () => {
it('uses find condition from request.body', (done) => {
var androidDeviceRequest = {
'installationId': '12345678-abcd-abcd-abcd-123456789abc',
Expand Down Expand Up @@ -71,6 +71,9 @@ describe('InstallationsRouter', () => {
var results = res.response.results;
expect(results.length).toEqual(1);
done();
}).catch((err) => {
fail(JSON.stringify(err));
done();
});
});

Expand Down Expand Up @@ -172,6 +175,9 @@ describe('InstallationsRouter', () => {
expect(response.results.length).toEqual(0);
expect(response.count).toEqual(2);
done();
}).catch((err) => {
fail(JSON.stringify(err));
done();
});
});
});
53 changes: 49 additions & 4 deletions spec/Logger.spec.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -9,10 +9,55 @@ class TestTransport extends winston.Transport {

describe('Logger', () => {
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);
expect(Object.keys(logging.logger.transports).length).toBe(4);
logging.logger.info('hi');
expect(testTransport.log).toHaveBeenCalled();
logging.removeTransport(testTransport);
expect(Object.keys(logging.logger.transports).length).toBe(3);
});

it('should have files transports', (done) => {
reconfigureServer().then(() => {
let transports = logging.logger.transports;
let transportKeys = Object.keys(transports);
expect(transportKeys.length).toBe(3);
done();
});
});

it('should disable files logs', (done) => {
reconfigureServer({
logsFolder: null
}).then(() => {
let transports = logging.logger.transports;
let transportKeys = Object.keys(transports);
expect(transportKeys.length).toBe(1);
done();
});
});

it('should enable JSON logs', (done) => {
// Force console transport
reconfigureServer({
logsFolder: null,
jsonLogs: true,
silent: false
}).then(() => {
let spy = spyOn(process.stdout, 'write');
logging.logger.info('hi', {key: 'value'});
expect(process.stdout.write).toHaveBeenCalled();
var firstLog = process.stdout.write.calls.first().args[0];
expect(firstLog).toEqual(JSON.stringify({key: 'value', level: 'info', message: 'hi' })+'\n');
return reconfigureServer({
jsonLogs: false
});
}).then(() => {
done();
});
});
});
25 changes: 8 additions & 17 deletions spec/Subscription.spec.js
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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]);
});
Expand All @@ -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]);
});
Expand All @@ -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);
});

Expand All @@ -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');
});
});
4 changes: 2 additions & 2 deletions spec/WinstonLoggerAdapter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('info logs', () => {

it("Verify INFO logs", (done) => {
var winstonLoggerAdapter = new WinstonLoggerAdapter();
winstonLoggerAdapter.info('testing info logs', () => {
winstonLoggerAdapter.log('info', 'testing info logs', () => {
winstonLoggerAdapter.query({
from: new Date(Date.now() - 500),
size: 100,
Expand All @@ -29,7 +29,7 @@ describe('info logs', () => {
describe('error logs', () => {
it("Verify ERROR logs", (done) => {
var winstonLoggerAdapter = new WinstonLoggerAdapter();
winstonLoggerAdapter.error('testing error logs', () => {
winstonLoggerAdapter.log('error', 'testing error logs', () => {
winstonLoggerAdapter.query({
from: new Date(Date.now() - 500),
size: 100,
Expand Down
3 changes: 1 addition & 2 deletions spec/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ var defaultConfiguration = {
webhookKey: 'hook',
masterKey: 'test',
fileKey: 'test',
silent: !process.env.VERBOSE,
push: {
'ios': {
cert: 'prodCert.pem',
Expand Down Expand Up @@ -352,8 +353,6 @@ global.describe_only_db = db => {
}
}

// LiveQuery test setting
require('../src/LiveQuery/PLog').logLevel = 'NONE';
var libraryCache = {};
jasmine.mockLibrary = function(library, name, mock) {
var original = require(library)[name];
Expand Down
5 changes: 2 additions & 3 deletions src/Adapters/Files/GridStoreAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@

import { MongoClient, GridStore, Db} from 'mongodb';
import { FilesAdapter } from './FilesAdapter';

const DefaultMongoURI = 'mongodb://localhost:27017/parse';
import defaults from '../../defaults';

export class GridStoreAdapter extends FilesAdapter {
_databaseURI: string;
_connectionPromise: Promise<Db>;

constructor(mongoDatabaseURI = DefaultMongoURI) {
constructor(mongoDatabaseURI = defaults.DefaultMongoURI) {
super();
this._databaseURI = mongoDatabaseURI;
this._connect();
Expand Down
10 changes: 4 additions & 6 deletions src/Adapters/Logger/LoggerAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@
// Allows you to change the logger mechanism
//
// Adapter classes must implement the following functions:
// * info(obj1 [, obj2, .., objN])
// * error(obj1 [, obj2, .., objN])
// * query(options, callback)
// * log() {}
// * query(options, callback) /* optional */
// Default is WinstonLoggerAdapter.js

export class LoggerAdapter {
info() {}
error() {}
query(options, callback) {}
constructor(options) {}
log(level, message, /* meta */) {}
}

export default LoggerAdapter;
100 changes: 100 additions & 0 deletions src/Adapters/Logger/WinstonLogger.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
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 '../../defaults';

const logger = new winston.Logger();
const additionalTransports = [];

function updateTransports(options) {
let transports = Object.assign({}, logger.transports);
if (options) {
let silent = options.silent;
delete options.silent;
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));
}

transports.console = new (winston.transports.Console)(
Object.assign({
colorize: true,
name: 'console',
silent
}, 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,
silent = defaults.silent } = {}) {

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) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output to console or don't catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May fail if the folder exists, so we just let go

}
options.dirname = logsFolder;
options.level = logLevel;
options.silent = silent;

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)
});
_.remove(additionalTransports, (transport) => {
return transport.name === transportName;
});
}

export { logger, addTransport, configureLogger, removeTransport };
export default logger;
Loading