Skip to content

Commit

Permalink
fix(ejson): support array for replacer parameter in EJSON.stringify
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
justingrant authored and mbroadst committed Feb 12, 2019
1 parent 2e08392 commit 9f43809
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
12 changes: 9 additions & 3 deletions lib/extended_json.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions test/node/extended_json_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}');
});
});

0 comments on commit 9f43809

Please sign in to comment.