Skip to content

Commit

Permalink
Document.toObject(): don't deep clone getters in order to avoid "Maxi…
Browse files Browse the repository at this point in the history
…mum call stack exceeded" error
  • Loading branch information
tommy351 committed Jan 21, 2015
1 parent 2016652 commit 53ce9eb
Show file tree
Hide file tree
Showing 7 changed files with 185 additions and 13 deletions.
11 changes: 1 addition & 10 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,8 @@ node_js:
- "0.10"
- "0.11"

before_script:
- npm install -g gulp
- npm install -g codeclimate-test-reporter

script:
- npm test

after_script:
- cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js
- cat ./coverage/lcov.info | codeclimate

addons:
code_climate:
repo_token: 47a797c76665f3d1ff692740cc7e87380b3455199a05620dd06e7b4ca67eb000
- cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js
8 changes: 7 additions & 1 deletion lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,18 @@ Document.prototype.toObject = function(){

for (var i = 0, len = keys.length; i < len; i++){
key = keys[i];
obj[key] = _.cloneDeep(this[key]);
// Don't deep clone getters in order to avoid "Maximum call stack size
// exceeded" error
obj[key] = isGetter(this, key) ? this[key] : _.cloneDeep(this[key]);
}

return obj;
};

function isGetter(obj, key){
return Object.getOwnPropertyDescriptor(obj, key).get;
}

/**
* Returns a string representing the document.
*
Expand Down
26 changes: 26 additions & 0 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ function extractPropKey(key){
* @static
*/
exports.getProp = function(obj, key){
if (typeof obj !== 'object') throw new TypeError('obj must be an object!');
if (!key) throw new TypeError('key is required!');

var token = extractPropKey(key);
var result = obj[token.shift()];
var len = token.length;
Expand All @@ -50,6 +53,9 @@ exports.getProp = function(obj, key){
* @static
*/
exports.setProp = function(obj, key, value){
if (typeof obj !== 'object') throw new TypeError('obj must be an object!');
if (!key) throw new TypeError('key is required!');

var token = extractPropKey(key);
var lastKey = token.pop();
var len = token.length;
Expand Down Expand Up @@ -79,6 +85,9 @@ exports.setProp = function(obj, key, value){
* @static
*/
exports.delProp = function(obj, key){
if (typeof obj !== 'object') throw new TypeError('obj must be an object!');
if (!key) throw new TypeError('key is required!');

var token = extractPropKey(key);
var lastKey = token.pop();
var len = token.length;
Expand Down Expand Up @@ -113,6 +122,10 @@ exports.delProp = function(obj, key){
* @param {Function} fn
*/
exports.setGetter = function(obj, key, fn){
if (typeof obj !== 'object') throw new TypeError('obj must be an object!');
if (!key) throw new TypeError('key is required!');
if (typeof fn !== 'function') throw new TypeError('fn must be a function!');

var token = extractPropKey(key);
var lastKey = token.pop();
var len = token.length;
Expand Down Expand Up @@ -143,6 +156,8 @@ exports.setGetter = function(obj, key, fn){
* @static
*/
exports.arr2obj = function(arr, value){
if (!Array.isArray(arr)) throw new TypeError('arr must be an array!');

var obj = {};
var i = arr.length;

Expand All @@ -163,6 +178,9 @@ exports.arr2obj = function(arr, value){
* @static
*/
exports.arrayEqual = function(a, b){
if (!Array.isArray(a)) throw new TypeError('a must be an array!');
if (!Array.isArray(b)) throw new TypeError('b must be an array!');

var i = a.length;

if (i !== b.length) return false;
Expand All @@ -182,6 +200,8 @@ exports.arrayEqual = function(a, b){
* @return {Array}
*/
function cloneArray(arr){
if (!Array.isArray(arr)) throw new TypeError('arr must be an array!');

var len = arr.length;
var result = new Array(len);

Expand All @@ -204,6 +224,8 @@ exports.cloneArray = cloneArray;
* @static
*/
exports.contains = function(arr, input){
if (!Array.isArray(arr)) throw new TypeError('arr must be an array!');

var i = arr.length;

if (!i) return false;
Expand All @@ -224,6 +246,8 @@ exports.contains = function(arr, input){
* @static
*/
exports.reverse = function(arr){
if (!Array.isArray(arr)) throw new TypeError('arr must be an array!');

var len = arr.length;
var tmp;

Expand All @@ -247,6 +271,8 @@ exports.reverse = function(arr){
* @static
*/
exports.shuffle = function(arr){
if (!Array.isArray(arr)) throw new TypeError('arr must be an array!');

var len = arr.length;
var i, tmp;

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"url": "https://raw.github.com/tommy351/warehouse/master/LICENSE"
},
"dependencies": {
"JSONStream": "^0.9.0",
"JSONStream": "^0.10.0",
"bluebird": "^2.3.11",
"cuid": "^1.2.4",
"lodash": "^2.4.1"
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/db.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"meta":{"version":1,"warehouse":"1.0.0-rc.2"},"models":{"Test":[{"_id":"A"},{"_id":"B"},{"_id":"C"}]}}
{"meta":{"version":1,"warehouse":"1.0.0-rc.3"},"models":{"Test":[{"_id":"A"},{"_id":"B"},{"_id":"C"}]}}
21 changes: 21 additions & 0 deletions test/scripts/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,27 @@ describe('Document', function(){
doc.toObject().should.not.be.instanceOf(User.Document);
});

it('toObject() - don\'t deep clone getters', function(){
var db = new Database();

var userSchema = new Schema({
name: String,
age: Number
});

userSchema.virtual('users').get(function(){
return User.find({});
});

var User = db.model('User', userSchema);

return User.insert({}).then(function(data){
return User.findById(data._id);
}).then(function(data){
data.toObject().should.be.ok;
});
});

it('toString()', function(){
var doc = User.new({});
doc.toString().should.eql(JSON.stringify(doc));
Expand Down
128 changes: 128 additions & 0 deletions test/scripts/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,22 @@ describe('util', function(){
util.getProp(obj, 'd.e.f').should.eql(obj.d.e.f);
});

it('getProp() - obj must be an object', function(){
try {
util.getProp();
} catch (err){
err.should.have.property('message', 'obj must be an object!');
}
});

it('getProp() - key is required', function(){
try {
util.getProp({});
} catch (err){
err.should.have.property('message', 'key is required!');
}
});

it('setProp()', function(){
var obj = {
a: {
Expand All @@ -44,6 +60,22 @@ describe('util', function(){
obj.d.e.f.should.eql('haha');
});

it('setProp() - obj must be an object', function(){
try {
util.setProp();
} catch (err){
err.should.have.property('message', 'obj must be an object!');
}
});

it('setProp() - key is required', function(){
try {
util.setProp({});
} catch (err){
err.should.have.property('message', 'key is required!');
}
});

it('delProp()', function(){
var obj = {
a: {
Expand All @@ -67,6 +99,22 @@ describe('util', function(){
should.not.exist(obj.d.e.f);
});

it('delProp() - obj must be an object', function(){
try {
util.delProp();
} catch (err){
err.should.have.property('message', 'obj must be an object!');
}
});

it('delProp() - key is required', function(){
try {
util.delProp({});
} catch (err){
err.should.have.property('message', 'key is required!');
}
});

it('setGetter()', function(){
var obj = {
a: {
Expand Down Expand Up @@ -101,36 +149,116 @@ describe('util', function(){
obj.a.c.h.should.eql('ach');
});

it('setGetter() - obj must be an object', function(){
try {
util.setGetter();
} catch (err){
err.should.have.property('message', 'obj must be an object!');
}
});

it('setGetter() - key is required', function(){
try {
util.setGetter({});
} catch (err){
err.should.have.property('message', 'key is required!');
}
});

it('setGetter() - fn must be a function', function(){
try {
util.setGetter({}, 'test');
} catch (err){
err.should.have.property('message', 'fn must be a function!');
}
});

it('arr2obj()', function(){
util.arr2obj(['a', 'b'], 1).should.eql({a: 1, b: 1});
});

it('arr2obj() - arr must be an array', function(){
try {
util.arr2obj();
} catch (err){
err.should.have.property('message', 'arr must be an array!');
}
});

it('arrayEqual()', function(){
util.arrayEqual(['a', 'b'], ['a', 'b']).should.be.true;
util.arrayEqual(['1', 2], ['1', '2']).should.be.false;
});

it('arrayEqual() - a must be an array', function(){
try {
util.arrayEqual();
} catch (err){
err.should.have.property('message', 'a must be an array!');
}
});

it('arrayEqual() - b must be an array', function(){
try {
util.arrayEqual([]);
} catch (err){
err.should.have.property('message', 'b must be an array!');
}
});

it('cloneArray()', function(){
util.cloneArray([1, 2, 3]).should.eql([1, 2, 3]);
util.cloneArray([1, [2, 3], [4, [5, 6]]]).should.eql([1, [2, 3], [4, [5, 6]]]);
});

it('cloneArray() - arr must be an array', function(){
try {
util.cloneArray();
} catch (err){
err.should.have.property('message', 'arr must be an array!');
}
});

it('contains()', function(){
util.contains(['1', '2', 3], '1').should.be.true;
util.contains(['1', '2', 3], '3').should.be.false;
});

it('contains() - arr must be an array', function(){
try {
util.contains();
} catch (err){
err.should.have.property('message', 'arr must be an array!');
}
});

it('reverse()', function(){
var arr = [1, '2', 'w'];

util.reverse(arr).should.eql(['w', '2', 1]);
});

it('reverse() - arr must be an array', function(){
try {
util.reverse();
} catch (err){
err.should.have.property('message', 'arr must be an array!');
}
});

it('shuffle()', function(){
var arr = util.shuffle([1, 2, 3]);
arr.sort().should.eql([1, 2, 3]);
});

it('parseArgs() - arr must be an array', function(){
try {
util.parseArgs();
} catch (err){
err.should.have.property('message', 'arr must be an array!');
}
});

it('parseArgs()', function(){
util.parseArgs('name').should.eql({name: 1});
util.parseArgs('name', -1).should.eql({name: -1});
Expand Down

0 comments on commit 53ce9eb

Please sign in to comment.