Skip to content

Commit

Permalink
Significantly simplified code that handled $push, $pushAll, $pull, an…
Browse files Browse the repository at this point in the history 
…d $pullAll combinations upon Model#save.
  • Loading branch information
@bnoguchi
bnoguchi committed Feb 17, 2011
1 parent 5a42710 commit 1e8cda4
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 73 deletions.
67 changes: 23 additions & 44 deletions lib/mongoose/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,63 +92,42 @@ Model.prototype.save = function (fn) {
return { path: path, value: self.getValue(path), schema: self.schema.path(path) };
}).reduce( function (delta, data) {
var type = data.value
, schema = data.schema;
, schema = data.schema
, atomics, val, obj;

// a MongooseArray or MongooseNumber
if (type._path && type.doAtomics) {
['$push', '$pull'].forEach( function (opType) {
var ops = type._atomics.filter( function (op) {
return op[0] === opType;
})
, opsAll = type._atomics.filter( function (op) {
return op[0] === (opType + 'All');
});
if (ops.length > 1 || (ops.length === 1 && opsAll.length > 0)) {
// If we have more than one $push (or $pull)
// Or if we have at least one $push and at least one $pushAll
// (or $pull and $pullAll)

// Then collapse everything into one $pushAll (or $pullAll)

type._atomics = type._atomics.filter( function (op) {
return op[0] !== opType;
});
if (opsAll.length > 0) {
type._atomics = type._atomics.filter( function (op) {
return op[0] !== (opType + 'All');
});
atomics = type._atomics;
for (var op in atomics) {
if (op === '$pushAll' || op === '$pullAll') {
if (atomics[op].length === 1) {
val = atomics[op][0];
delete atomics[op];
op = op.replace('All', '');
atomics[op] = val;
}
var whatToAll = [];
opsAll.forEach( function (op) {
whatToAll = whatToAll.concat(op[1]);
});
whatToAll = whatToAll.concat( ops.map( function (op) {
return op[1];
}) );
type._atomics.push([opType + 'All', whatToAll]);
}
});
type._atomics.forEach( function (op) {
var obj = delta[op[0]] = delta[op[0]] || {};
if (op[0] === '$pull' || op[0] === '$push') {
if (op[1].constructor !== Object) {
op[1] = schema.cast(op[1])[0];
val = atomics[op];
obj = delta[op] = delta[op] || {};
if (op === '$pull' || op === '$push') {
if (val.constructor !== Object) {
val = schema.cast(val)[0];
}
}
obj[type._path] = op[1].toObject
? op[1].toObject() // If the value is an array
: Array.isArray(op[1])
? op[1].map( function (mem) {
obj[type._path] = val.toObject
? val.toObject() // If the value is an array
: Array.isArray(val)
? val.map( function (mem) {
return mem.toObject
? mem.toObject()
: mem.valueOf
? mem.valueOf()
: mem;
})
: op[1].valueOf
? op[1].valueOf() // Numbers
: op[1];
});
: val.valueOf
? val.valueOf() // Numbers
: val;
}
} else {
// normalize MongooseArray or MongooseNumber
if (type instanceof MongooseArray) {
Expand Down
46 changes: 22 additions & 24 deletions lib/mongoose/types/array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function MongooseArray (values, path, doc) {
var arr = [];
arr.push.apply(arr, values);
arr.__proto__ = MongooseArray.prototype;
arr._atomics = [];
arr._atomics = {};
arr.validators = [];
arr._path = path;
arr._parent = doc;
Expand Down Expand Up @@ -84,8 +84,14 @@ MongooseArray.prototype._markModified = function () {
* @api private
*/

MongooseArray.prototype._registerAtomic = function (op) {
this._atomics.push(op);
MongooseArray.prototype._registerAtomic = function (op, val) {
var atomics = this._atomics
if (op === '$pullAll' || op === '$pushAll') {
atomics[op] || (atomics[op] = []);
atomics[op] = atomics[op].concat(val);
} else {
atomics[op] = val;
}
this._markModified();
return this;
};
Expand All @@ -98,7 +104,7 @@ MongooseArray.prototype._registerAtomic = function (op) {
*/

MongooseArray.prototype.__defineGetter__('doAtomics', function () {
return this._atomics.length;
return Object.keys(this._atomics).length;
});

/**
Expand All @@ -115,10 +121,9 @@ MongooseArray.prototype.push = function () {
var values = Array.prototype.map.call(arguments, this._cast, this)
, ret = oldPush.apply(this, values);

if (1 === values.length)
this._registerAtomic(['$push', values[0]]);
else
this._registerAtomic(['$pushAll', values]);
// $pushAll might be fibbed (could be $push). But it makes it easier to
// handle what could have been $push, $pushAll combos
this._registerAtomic('$pushAll', values);

return ret;
};
Expand Down Expand Up @@ -151,7 +156,7 @@ MongooseArray.prototype.$pushAll = function (value) {
var length = this.length;
this.nonAtomicPush.apply(this, value);
// make sure we access the casted elements
this._registerAtomic(['$pushAll', this.slice(length) ]);
this._registerAtomic('$pushAll', this.slice(length));
return this;
};

Expand All @@ -162,7 +167,7 @@ MongooseArray.prototype.$pushAll = function (value) {
*/

MongooseArray.prototype.$pop = function () {
this._registerAtomic(['$pop', '1']);
this._registerAtomic('$pop', '1');
return this.pop();
};

Expand All @@ -173,7 +178,7 @@ MongooseArray.prototype.$pop = function () {
*/

MongooseArray.prototype.$shift = function () {
this._registerAtomic(['$shift', '-1']);
this._registerAtomic('$shift', '-1');
return this.shift();
};

Expand Down Expand Up @@ -216,18 +221,10 @@ MongooseArray.prototype.$pull = function () {
}
} else if (~values.indexOf(mem)) oldArr.splice(i, 1);
}
if (1 === values.length) {
if (values[0] instanceof EmbeddedDocument) {
this._registerAtomic(['$pull', {_id: values[0]._id}]);
} else {
this._registerAtomic(['$pull', values[0]]);
}
if (values[0] instanceof EmbeddedDocument) {
this._registerAtomic('$pullAll', values.map( function (v) { return {_id: v._id}; } ));
} else {
if (values[0] instanceof EmbeddedDocument) {
this._registerAtomic(['$pullAll', values.map( function (v) { return {_id: v._id}; } )]);
} else {
this._registerAtomic(['$pullAll', values]);
}
this._registerAtomic('$pullAll', values);
}
return this;
};
Expand All @@ -239,7 +236,7 @@ MongooseArray.prototype.$pull = function () {
*/

MongooseArray.prototype.$pullAll = function (values) {
if (values && values.length)
if (values && values.length) {
var oldArr = this._parent.get(this._path)
, i = oldArr.length, mem;
while (i--) {
Expand All @@ -250,7 +247,8 @@ MongooseArray.prototype.$pullAll = function (values) {
}
} else if (~values.indexOf(mem)) oldArr.splice(i, 1);
}
this._registerAtomic(['$pullAll', values]);
this._registerAtomic('$pullAll', values);
}
return this;
};

Expand Down
6 changes: 3 additions & 3 deletions lib/mongoose/types/number.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
function MongooseNumber (value, path, doc) {
var number = new Number(value);
number.__proto__ = MongooseNumber.prototype;
number._atomics = [];
number._atomics = {};
number._path = path;
number._parent = doc;
return number;
Expand All @@ -37,7 +37,7 @@ MongooseNumber.prototype.increment = function(value){
, value = Number(value) || 1;
if (isNaN(value)) value = 1;
this._parent.setValue(this._path, schema.cast(this + value));
this._parent.getValue(this._path)._atomics = [['$inc', value || 1]];
this._parent.getValue(this._path)._atomics['$inc'] = value || 1;
this._parent.activePaths.modify(this._path);
return this;
};
Expand All @@ -50,7 +50,7 @@ MongooseNumber.prototype.increment = function(value){
*/

MongooseNumber.prototype.__defineGetter__('doAtomics', function () {
return this._atomics.length;
return Object.keys(this._atomics).length;
});

/**
Expand Down
2 changes: 1 addition & 1 deletion test/types.array.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module.exports = {
a.should.be.an.instanceof(Array);
a.should.be.an.instanceof(MongooseArray);
Array.isArray(a).should.be.true;
Array.isArray(a._atomics).should.be.true;
(a._atomics.constructor).should.eql(Object);
}

};
2 changes: 1 addition & 1 deletion test/types.number.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
a.should.be.an.instanceof(MongooseNumber);
a.toString().should.eql('5');

Array.isArray(a._atomics).should.be.true;
(a._atomics.constructor).should.eql(Object);
}

};

0 comments on commit 1e8cda4

Please sign in to comment.