From 9f4380927254290227cda8d5db3a08809b6ce3d6 Mon Sep 17 00:00:00 2001 From: Justin Grant Date: Tue, 12 Feb 2019 06:52:19 -0800 Subject: [PATCH] fix(ejson): support array for replacer parameter in `EJSON.stringify` * Fix EJSON.stringify if replacer is array (not function) Currently, passing an array form of the `replacer` parameter of EJSON.stringify will not work, despite the documentation saying that it should work. The cause is that the current implementation checks whether `replacer` is of type `object`, and if it is then it assumes that the `replacer` parameter is really the `options` parameter (meaning that `replacer` and `space` have been omitted by the user). This logic breaks when `replacer` is an array (which is an `object` too). To fix this problem I added a check using `Array.isArray` so the replacement above will only happen if `replacer` is an object that's not an array. While I was at it, I also changed the weird (and eslint-defying) use of assignment expressions ( `if (...) (options = space), (space = 0);` format to a more conventional statement form of assignment. * Added test for array-valued replacers Made sure that array-valued replacers work correctly. Also making sure that function-valued replacers work too. * Fixed bugs in test cases * removed unnecessary async function to fix Node 6 * EJSON serializing of ObjectID (capital D) + perf fix While I was testing my other fix, I found and fixed two other problems with EJSON serialization: * Fixes #303 - ObjectID (capital "D") instances can't be serialized to Extended JSON * Fixes #302 - Extended JSON serializer runs twice for every nested document I also noticed two other potential problems that I didn't know how to fix, so I added TODO comments in this commit for tracking later. If you want me to split these changes up into separate PRs from my original commits, I'm happy to do so-- just let me know. * Update EJSON tests for ObjectId and ObjectID Added tests to ensure that EJSON support for ObjectId works for three variations: * import ObjectId from 'bson'; * import ObjectID from 'bson'; * `import ObjectID from 'mongodb'; // verifies the fix for #303` * fixed browser test Ignoring the require('mongodb').ObjectID tests on the browser tests where that package is not available. * another fix for browser tests * another try to avoid browser test import error * reverted changes moved to other PRs --- lib/extended_json.js | 12 +++++++++--- test/node/extended_json_tests.js | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/extended_json.js b/lib/extended_json.js index 24b17b3a..bfb03c35 100644 --- a/lib/extended_json.js +++ b/lib/extended_json.js @@ -169,9 +169,15 @@ const BSON_INT32_MAX = 0x7fffffff, * console.log(EJSON.stringify(doc)); */ function stringify(value, replacer, space, options) { - if (space != null && typeof space === 'object') (options = space), (space = 0); - if (replacer != null && typeof replacer === 'object') - (options = replacer), (replacer = null), (space = 0); + if (space != null && typeof space === 'object') { + options = space; + space = 0; + } + if (replacer != null && typeof replacer === 'object' && !Array.isArray(replacer)) { + options = replacer; + replacer = null; + space = 0; + } options = Object.assign({}, { relaxed: true }, options); const doc = Array.isArray(value) diff --git a/test/node/extended_json_tests.js b/test/node/extended_json_tests.js index b576354f..754992b2 100644 --- a/test/node/extended_json_tests.js +++ b/test/node/extended_json_tests.js @@ -253,4 +253,22 @@ describe('Extended JSON', function() { expect(result.test).to.equal(34.12); expect(result.test).to.be.a('number'); }); + + it('should work for function-valued and array-valued replacer parameters', function() { + const doc = { a: new Int32(10), b: new Int32(10) }; + + var replacerArray = ['a', '$numberInt']; + var serialized = EJSON.stringify(doc, replacerArray, 0, { relaxed: false }); + expect(serialized).to.equal('{"a":{"$numberInt":"10"}}'); + + serialized = EJSON.stringify(doc, replacerArray); + expect(serialized).to.equal('{"a":10}'); + + var replacerFunc = function (key, value) { return key === 'b' ? undefined : value; } + serialized = EJSON.stringify(doc, replacerFunc, 0, { relaxed: false }); + expect(serialized).to.equal('{"a":{"$numberInt":"10"}}'); + + serialized = EJSON.stringify(doc, replacerFunc); + expect(serialized).to.equal('{"a":10}'); + }); });