Skip to content

Commit 4f907a0

Browse files
authoredJul 31, 2018
feat(deprecation): create deprecation function
Create deprecateOptions wrapper to warn on using deprecated options of library methods. Fixes NODE-1430
1 parent 666b8fa commit 4f907a0

13 files changed

+627
-1
lines changed
 

‎lib/aggregation_cursor.js

+10
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,16 @@ AggregationCursor.prototype.unwind = function(field) {
281281
return this;
282282
};
283283

284+
/**
285+
* Return the cursor logger
286+
* @method
287+
* @return {Logger} return the cursor logger
288+
* @ignore
289+
*/
290+
AggregationCursor.prototype.getLogger = function() {
291+
return this.logger;
292+
};
293+
284294
AggregationCursor.prototype.get = AggregationCursor.prototype.toArray;
285295

286296
/**

‎lib/collection.js

+10
Original file line numberDiff line numberDiff line change
@@ -2073,4 +2073,14 @@ Collection.prototype.initializeOrderedBulkOp = function(options) {
20732073
return ordered(this.s.topology, this, options);
20742074
};
20752075

2076+
/**
2077+
* Return the db logger
2078+
* @method
2079+
* @return {Logger} return the db logger
2080+
* @ignore
2081+
*/
2082+
Collection.prototype.getLogger = function() {
2083+
return this.s.db.s.logger;
2084+
};
2085+
20762086
module.exports = Collection;

‎lib/command_cursor.js

+10
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,16 @@ CommandCursor.prototype.maxTimeMS = function(value) {
216216
return this;
217217
};
218218

219+
/**
220+
* Return the cursor logger
221+
* @method
222+
* @return {Logger} return the cursor logger
223+
* @ignore
224+
*/
225+
CommandCursor.prototype.getLogger = function() {
226+
return this.logger;
227+
};
228+
219229
CommandCursor.prototype.get = CommandCursor.prototype.toArray;
220230

221231
/**

‎lib/cursor.js

+10
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,16 @@ Cursor.prototype._read = function() {
10391039
});
10401040
};
10411041

1042+
/**
1043+
* Return the cursor logger
1044+
* @method
1045+
* @return {Logger} return the cursor logger
1046+
* @ignore
1047+
*/
1048+
Cursor.prototype.getLogger = function() {
1049+
return this.logger;
1050+
};
1051+
10421052
Object.defineProperty(Cursor.prototype, 'readPreference', {
10431053
enumerable: true,
10441054
get: function() {

‎lib/db.js

+10
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,16 @@ Db.prototype.watch = function(pipeline, options) {
900900
return new ChangeStream(this, pipeline, options);
901901
};
902902

903+
/**
904+
* Return the db logger
905+
* @method
906+
* @return {Logger} return the db logger
907+
* @ignore
908+
*/
909+
Db.prototype.getLogger = function() {
910+
return this.s.logger;
911+
};
912+
903913
/**
904914
* Db close event
905915
*

‎lib/gridfs-stream/index.js

+10
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,16 @@ GridFSBucket.prototype.drop = function(callback) {
322322
});
323323
};
324324

325+
/**
326+
* Return the db logger
327+
* @method
328+
* @return {Logger} return the db logger
329+
* @ignore
330+
*/
331+
GridFSBucket.prototype.getLogger = function() {
332+
return this.s.db.s.logger;
333+
};
334+
325335
/**
326336
* @ignore
327337
*/

‎lib/mongo_client.js

+10
Original file line numberDiff line numberDiff line change
@@ -458,4 +458,14 @@ MongoClient.prototype.watch = function(pipeline, options) {
458458
return new ChangeStream(this, pipeline, options);
459459
};
460460

461+
/**
462+
* Return the mongo client logger
463+
* @method
464+
* @return {Logger} return the mongo client logger
465+
* @ignore
466+
*/
467+
MongoClient.prototype.getLogger = function() {
468+
return this.s.options.logger;
469+
};
470+
461471
module.exports = MongoClient;

‎lib/utils.js

+78-1
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,82 @@ function decorateWithReadConcern(command, coll) {
606606
}
607607
}
608608

609+
const emitProcessWarning = msg => process.emitWarning(msg, 'DeprecationWarning');
610+
const emitConsoleWarning = msg => console.error(msg);
611+
const emitDeprecationWarning = process.emitWarning ? emitProcessWarning : emitConsoleWarning;
612+
613+
/**
614+
* Default message handler for generating deprecation warnings.
615+
*
616+
* @param {string} name function name
617+
* @param {string} option option name
618+
* @return {string} warning message
619+
* @ignore
620+
* @api private
621+
*/
622+
function defaultMsgHandler(name, option) {
623+
return `${name} option [${option}] is deprecated and will be removed in a later version.`;
624+
}
625+
626+
/**
627+
* Deprecates a given function's options.
628+
*
629+
* @param {object} config configuration for deprecation
630+
* @param {string} config.name function name
631+
* @param {Array} config.deprecatedOptions options to deprecate
632+
* @param {number} config.optionsIndex index of options object in function arguments array
633+
* @param {function} [config.msgHandler] optional custom message handler to generate warnings
634+
* @param {function} fn the target function of deprecation
635+
* @return {function} modified function that warns once per deprecated option, and executes original function
636+
* @ignore
637+
* @api private
638+
*/
639+
function deprecateOptions(config, fn) {
640+
if (process.noDeprecation === true) {
641+
return fn;
642+
}
643+
644+
const msgHandler = config.msgHandler ? config.msgHandler : defaultMsgHandler;
645+
646+
const optionsWarned = new Set();
647+
function deprecated() {
648+
const options = arguments[config.optionsIndex];
649+
650+
// ensure options is a valid, non-empty object, otherwise short-circuit
651+
if (!isObject(options) || Object.keys(options).length === 0) {
652+
return fn.apply(this, arguments);
653+
}
654+
655+
config.deprecatedOptions.forEach(deprecatedOption => {
656+
if (options.hasOwnProperty(deprecatedOption) && !optionsWarned.has(deprecatedOption)) {
657+
optionsWarned.add(deprecatedOption);
658+
const msg = msgHandler(config.name, deprecatedOption);
659+
emitDeprecationWarning(msg);
660+
if (this && this.getLogger) {
661+
const logger = this.getLogger();
662+
if (logger) {
663+
logger.warn(msg);
664+
}
665+
}
666+
}
667+
});
668+
669+
return fn.apply(this, arguments);
670+
}
671+
672+
// These lines copied from https://github.com/nodejs/node/blob/25e5ae41688676a5fd29b2e2e7602168eee4ceb5/lib/internal/util.js#L73-L80
673+
// The wrapper will keep the same prototype as fn to maintain prototype chain
674+
Object.setPrototypeOf(deprecated, fn);
675+
if (fn.prototype) {
676+
// Setting this (rather than using Object.setPrototype, as above) ensures
677+
// that calling the unwrapped constructor gives an instanceof the wrapped
678+
// constructor.
679+
deprecated.prototype = fn.prototype;
680+
}
681+
682+
return deprecated;
683+
}
684+
609685
module.exports = {
610686
filterOptions,
611687
mergeOptions,
@@ -629,5 +705,6 @@ module.exports = {
629705
resolveReadPreference,
630706
isPromiseLike,
631707
decorateWithCollation,
632-
decorateWithReadConcern
708+
decorateWithReadConcern,
709+
deprecateOptions
633710
};

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"istanbul": "^0.4.5",
2929
"jsdoc": "3.5.5",
3030
"lodash.camelcase": "^4.3.0",
31+
"mocha-sinon": "^2.1.0",
3132
"mongodb-extjson": "^2.1.1",
3233
"mongodb-mock-server": "^1.0.0",
3334
"mongodb-test-runner": "^1.1.18",
+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
'use strict';
2+
const exec = require('child_process').exec;
3+
const chai = require('chai');
4+
const expect = chai.expect;
5+
const sinon = require('sinon');
6+
const sinonChai = require('sinon-chai');
7+
require('mocha-sinon');
8+
chai.use(sinonChai);
9+
10+
const utils = require('../tools/utils');
11+
const ClassWithLogger = utils.ClassWithLogger;
12+
const ClassWithoutLogger = utils.ClassWithoutLogger;
13+
const ClassWithUndefinedLogger = utils.ClassWithUndefinedLogger;
14+
const ensureCalledWith = utils.ensureCalledWith;
15+
16+
describe('Deprecation Warnings', function() {
17+
beforeEach(function() {
18+
this.sinon.stub(console, 'error');
19+
});
20+
21+
const defaultMessage = ' is deprecated and will be removed in a later version.';
22+
23+
it('node --no-deprecation flag should suppress all deprecation warnings', {
24+
metadata: { requires: { node: '>=6.0.0' } },
25+
test: function(done) {
26+
exec(
27+
'node --no-deprecation ./test/tools/deprecate_warning_test_program.js',
28+
(err, stdout, stderr) => {
29+
expect(err).to.be.null;
30+
expect(stdout).to.be.empty;
31+
expect(stderr).to.be.empty;
32+
done();
33+
}
34+
);
35+
}
36+
});
37+
38+
it('node --trace-deprecation flag should print stack trace to stderr', {
39+
metadata: { requires: { node: '>=6.0.0' } },
40+
test: function(done) {
41+
exec(
42+
'node --trace-deprecation ./test/tools/deprecate_warning_test_program.js',
43+
(err, stdout, stderr) => {
44+
expect(err).to.be.null;
45+
expect(stdout).to.be.empty;
46+
expect(stderr).to.not.be.empty;
47+
48+
// split stderr into separate lines, trimming the first line to just the warning message
49+
const split = stderr.split('\n');
50+
const warning = split
51+
.shift()
52+
.split(')')[1]
53+
.trim();
54+
55+
// ensure warning message matches expected
56+
expect(warning).to.equal(
57+
'DeprecationWarning: testDeprecationFlags option [maxScan]' + defaultMessage
58+
);
59+
60+
// ensure each following line is from the stack trace, i.e. 'at config.deprecatedOptions.forEach.deprecatedOption'
61+
split.pop();
62+
split.forEach(s => {
63+
expect(s.trim()).to.match(/^at/);
64+
});
65+
66+
done();
67+
}
68+
);
69+
}
70+
});
71+
72+
it('node --throw-deprecation flag should throw error when deprecated function is called', {
73+
metadata: { requires: { node: '>=6.0.0' } },
74+
test: function(done) {
75+
exec(
76+
'node --throw-deprecation ./test/tools/deprecate_warning_test_program.js this_arg_should_never_print',
77+
(err, stdout, stderr) => {
78+
expect(stderr).to.not.be.empty;
79+
expect(err).to.not.be.null;
80+
expect(err)
81+
.to.have.own.property('code')
82+
.that.equals(1);
83+
84+
// ensure stdout is empty, i.e. that the program threw an error before reaching the console.log statement
85+
expect(stdout).to.be.empty;
86+
done();
87+
}
88+
);
89+
}
90+
});
91+
92+
it('test behavior for classes with an associated logger', function() {
93+
const fakeClass = new ClassWithLogger();
94+
const logger = fakeClass.getLogger();
95+
const stub = sinon.stub(logger, 'warn');
96+
97+
fakeClass.f({ maxScan: 5, snapshot: true });
98+
fakeClass.f({ maxScan: 5, snapshot: true });
99+
expect(stub).to.have.been.calledTwice;
100+
ensureCalledWith(stub, [
101+
'f option [maxScan] is deprecated and will be removed in a later version.',
102+
'f option [snapshot] is deprecated and will be removed in a later version.'
103+
]);
104+
});
105+
106+
it('test behavior for classes without an associated logger', function() {
107+
const fakeClass = new ClassWithoutLogger();
108+
109+
function func() {
110+
fakeClass.f({ maxScan: 5, snapshot: true });
111+
}
112+
113+
expect(func).to.not.throw();
114+
});
115+
116+
it('test behavior for classes with an undefined logger', function() {
117+
const fakeClass = new ClassWithUndefinedLogger();
118+
119+
function func() {
120+
fakeClass.f({ maxScan: 5, snapshot: true });
121+
}
122+
123+
expect(func).to.not.throw();
124+
});
125+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
3+
// prevent this file from being imported; it is only for use in functional/deprecate_warning_tests.js
4+
if (require.main !== module) {
5+
return;
6+
}
7+
8+
const deprecateOptions = require('../../lib/utils.js').deprecateOptions;
9+
10+
const testDeprecationFlags = deprecateOptions(
11+
{
12+
name: 'testDeprecationFlags',
13+
deprecatedOptions: ['maxScan', 'snapshot', 'fields'],
14+
optionsIndex: 0
15+
},
16+
options => {
17+
if (options) options = null;
18+
}
19+
);
20+
21+
testDeprecationFlags({ maxScan: 0 });
22+
23+
// for tests that throw error on calling deprecated fn - this should never happen; stdout should be empty
24+
if (process.argv[2]) {
25+
console.log(process.argv[2]);
26+
}
27+
28+
process.nextTick(() => process.exit());

‎test/tools/utils.js

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use strict';
2+
3+
const Logger = require('mongodb-core').Logger;
4+
const deprecateOptions = require('../../lib/utils').deprecateOptions;
5+
const chai = require('chai');
6+
const expect = chai.expect;
7+
const sinonChai = require('sinon-chai');
8+
chai.use(sinonChai);
9+
10+
function makeTestFunction(config) {
11+
const fn = options => {
12+
if (options) options = null;
13+
};
14+
return deprecateOptions(config, fn);
15+
}
16+
17+
function ensureCalledWith(stub, args) {
18+
args.forEach(m => expect(stub).to.have.been.calledWith(m));
19+
}
20+
21+
// creation of class with a logger
22+
function ClassWithLogger() {
23+
this.logger = new Logger('ClassWithLogger');
24+
}
25+
26+
ClassWithLogger.prototype.f = makeTestFunction({
27+
name: 'f',
28+
deprecatedOptions: ['maxScan', 'snapshot', 'fields'],
29+
optionsIndex: 0
30+
});
31+
32+
ClassWithLogger.prototype.getLogger = function() {
33+
return this.logger;
34+
};
35+
36+
// creation of class without a logger
37+
function ClassWithoutLogger() {}
38+
39+
ClassWithoutLogger.prototype.f = makeTestFunction({
40+
name: 'f',
41+
deprecatedOptions: ['maxScan', 'snapshot', 'fields'],
42+
optionsIndex: 0
43+
});
44+
45+
// creation of class where getLogger returns undefined
46+
function ClassWithUndefinedLogger() {}
47+
48+
ClassWithUndefinedLogger.prototype.f = makeTestFunction({
49+
name: 'f',
50+
deprecatedOptions: ['maxScan', 'snapshot', 'fields'],
51+
optionsIndex: 0
52+
});
53+
54+
ClassWithUndefinedLogger.prototype.getLogger = function() {
55+
return undefined;
56+
};
57+
58+
module.exports = {
59+
makeTestFunction,
60+
ensureCalledWith,
61+
ClassWithLogger,
62+
ClassWithoutLogger,
63+
ClassWithUndefinedLogger
64+
};

‎test/unit/deprecate_warning_tests.js

+261
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
'use strict';
2+
const deprecateOptions = require('../../lib/utils').deprecateOptions;
3+
const chai = require('chai');
4+
const expect = chai.expect;
5+
const sinonChai = require('sinon-chai');
6+
require('mocha-sinon');
7+
chai.use(sinonChai);
8+
9+
const makeTestFunction = require('../tools/utils').makeTestFunction;
10+
const ensureCalledWith = require('../tools/utils').ensureCalledWith;
11+
12+
describe('Deprecation Warnings', function() {
13+
let messages = [];
14+
const deprecatedOptions = ['maxScan', 'snapshot', 'fields'];
15+
const defaultMessage = ' is deprecated and will be removed in a later version.';
16+
17+
before(function() {
18+
if (process.emitWarning) {
19+
process.on('warning', warning => {
20+
messages.push(warning.message);
21+
});
22+
}
23+
return;
24+
});
25+
26+
beforeEach(function() {
27+
this.sinon.stub(console, 'error');
28+
});
29+
30+
afterEach(function() {
31+
messages.length = 0;
32+
});
33+
34+
describe('Mult functions with same options', function() {
35+
beforeEach(function() {
36+
const f1 = makeTestFunction({
37+
name: 'f1',
38+
deprecatedOptions: deprecatedOptions,
39+
optionsIndex: 0
40+
});
41+
const f2 = makeTestFunction({
42+
name: 'f2',
43+
deprecatedOptions: deprecatedOptions,
44+
optionsIndex: 0
45+
});
46+
f1({ maxScan: 5 });
47+
f2({ maxScan: 5 });
48+
});
49+
50+
it('multiple functions with the same deprecated options should both warn', {
51+
metadata: { requires: { node: '>=6.0.0' } },
52+
test: function(done) {
53+
process.nextTick(() => {
54+
expect(messages).to.deep.equal([
55+
'f1 option [maxScan]' + defaultMessage,
56+
'f2 option [maxScan]' + defaultMessage
57+
]);
58+
expect(messages).to.have.a.lengthOf(2);
59+
done();
60+
});
61+
}
62+
});
63+
64+
it('multiple functions with the same deprecated options should both warn', {
65+
metadata: { requires: { node: '<6.0.0' } },
66+
test: function(done) {
67+
ensureCalledWith(console.error, [
68+
'f1 option [maxScan]' + defaultMessage,
69+
'f2 option [maxScan]' + defaultMessage
70+
]);
71+
expect(console.error).to.have.been.calledTwice;
72+
done();
73+
}
74+
});
75+
});
76+
77+
describe('Empty options object', function() {
78+
beforeEach(function() {
79+
const f = makeTestFunction({
80+
name: 'f',
81+
deprecatedOptions: deprecatedOptions,
82+
optionsIndex: 0
83+
});
84+
f({});
85+
});
86+
87+
it('should not warn if empty options object passed in', {
88+
metadata: { requires: { node: '>=6.0.0' } },
89+
test: function(done) {
90+
process.nextTick(() => {
91+
expect(messages).to.have.a.lengthOf(0);
92+
done();
93+
});
94+
}
95+
});
96+
97+
it('should not warn if empty options object passed in', {
98+
metadata: { requires: { node: '<6.0.0' } },
99+
test: function(done) {
100+
expect(console.error).to.have.not.been.called;
101+
done();
102+
}
103+
});
104+
});
105+
106+
describe('Custom Message Handler', function() {
107+
beforeEach(function() {
108+
const customMsgHandler = (name, option) => {
109+
return 'custom msg for function ' + name + ' and option ' + option;
110+
};
111+
112+
const f = makeTestFunction({
113+
name: 'f',
114+
deprecatedOptions: deprecatedOptions,
115+
optionsIndex: 0,
116+
msgHandler: customMsgHandler
117+
});
118+
119+
f({ maxScan: 5, snapshot: true, fields: 'hi' });
120+
});
121+
122+
it('should use user-specified message handler', {
123+
metadata: { requires: { node: '>=6.0.0' } },
124+
test: function(done) {
125+
process.nextTick(() => {
126+
expect(messages).to.deep.equal([
127+
'custom msg for function f and option maxScan',
128+
'custom msg for function f and option snapshot',
129+
'custom msg for function f and option fields'
130+
]);
131+
expect(messages).to.have.a.lengthOf(3);
132+
done();
133+
});
134+
}
135+
});
136+
137+
it('should use user-specified message handler', {
138+
metadata: { requires: { node: '<6.0.0' } },
139+
test: function(done) {
140+
ensureCalledWith(console.error, [
141+
'custom msg for function f and option maxScan',
142+
'custom msg for function f and option snapshot',
143+
'custom msg for function f and option fields'
144+
]);
145+
expect(console.error).to.have.been.calledThrice;
146+
done();
147+
}
148+
});
149+
});
150+
151+
describe('Warn once', function() {
152+
beforeEach(function() {
153+
const f = makeTestFunction({
154+
name: 'f',
155+
deprecatedOptions: deprecatedOptions,
156+
optionsIndex: 0
157+
});
158+
f({ maxScan: 5, fields: 'hi' });
159+
f({ maxScan: 5, fields: 'hi' });
160+
});
161+
162+
it('each function should only warn once per deprecated option', {
163+
metadata: { requires: { node: '>=6.0.0' } },
164+
test: function(done) {
165+
process.nextTick(() => {
166+
expect(messages).to.deep.equal([
167+
'f option [maxScan]' + defaultMessage,
168+
'f option [fields]' + defaultMessage
169+
]);
170+
expect(messages).to.have.a.lengthOf(2);
171+
done();
172+
});
173+
}
174+
});
175+
176+
it('each function should only warn once per deprecated option', {
177+
metadata: { requires: { node: '<6.0.0' } },
178+
test: function(done) {
179+
ensureCalledWith(console.error, [
180+
'f option [maxScan]' + defaultMessage,
181+
'f option [fields]' + defaultMessage
182+
]);
183+
expect(console.error).to.have.been.calledTwice;
184+
done();
185+
}
186+
});
187+
});
188+
189+
describe('Maintain functionality', function() {
190+
beforeEach(function() {
191+
const config = {
192+
name: 'f',
193+
deprecatedOptions: ['multiply', 'add'],
194+
optionsIndex: 0
195+
};
196+
197+
const operateBy2 = (options, num) => {
198+
if (options.multiply === true) {
199+
return num * 2;
200+
}
201+
if (options.add === true) {
202+
return num + 2;
203+
}
204+
};
205+
206+
const f = deprecateOptions(config, operateBy2);
207+
208+
const mult = f({ multiply: true }, 5);
209+
const add = f({ add: true }, 5);
210+
211+
expect(mult).to.equal(10);
212+
expect(add).to.equal(7);
213+
});
214+
215+
it('wrapped functions should maintain original functionality', {
216+
metadata: { requires: { node: '>=6.0.0' } },
217+
test: function(done) {
218+
process.nextTick(() => {
219+
expect(messages).to.deep.equal([
220+
'f option [multiply]' + defaultMessage,
221+
'f option [add]' + defaultMessage
222+
]);
223+
expect(messages).to.have.a.lengthOf(2);
224+
done();
225+
});
226+
}
227+
});
228+
229+
it('wrapped functions should maintain original functionality', {
230+
metadata: { requires: { node: '<6.0.0' } },
231+
test: function(done) {
232+
ensureCalledWith(console.error, [
233+
'f option [multiply]' + defaultMessage,
234+
'f option [add]' + defaultMessage
235+
]);
236+
expect(console.error).to.have.been.calledTwice;
237+
done();
238+
}
239+
});
240+
});
241+
242+
it('optionsIndex pointing to undefined should not error', function(done) {
243+
const f = makeTestFunction({
244+
name: 'f',
245+
deprecatedOptions: deprecatedOptions,
246+
optionsIndex: 0
247+
});
248+
expect(f).to.not.throw();
249+
done();
250+
});
251+
252+
it('optionsIndex not pointing to object should not error', function(done) {
253+
const f = makeTestFunction({
254+
name: 'f',
255+
deprecatedOptions: deprecatedOptions,
256+
optionsIndex: 0
257+
});
258+
expect(() => f('not-an-object')).to.not.throw();
259+
done();
260+
});
261+
});

0 commit comments

Comments
 (0)
Please sign in to comment.