Don't auto save associations after autofetching them.

Closes #398

Author: dxg

Readme.md

@@ -657,6 +657,10 @@ patient.removeDoctors(docs, function...) // Removes specified doctors from join
 
 doctor.getPatients(function..)
 etc...
+
+// You can also do:
+patient.doctors = [doc1, doc2];
+patient.save(...)
 ```
 
 To associate a doctor to a patient:

lib/Associations/Many.js

@@ -227,6 +227,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan
 			}
 
 			association.model.find(conditions, options, cb);
+
 			return this;
 		},
 		enumerable: false,
@@ -409,6 +410,17 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan
 		enumerable: false,
 		writable: true
 	});
+
+	Object.defineProperty(Instance, association.name, {
+		get: function () {
+			return Instance.__opts.associations[association.name].value;
+		},
+		set: function (val) {
+			Instance.__opts.associations[association.name].changed = true;
+			Instance.__opts.associations[association.name].value   = val;
+		},
+		enumerable: true
+	});
 }
 
 function autoFetchInstance(Instance, association, opts, cb) {
@@ -426,7 +438,8 @@ function autoFetchInstance(Instance, association, opts, cb) {
 
 	Instance[association.getAccessor]({}, { autoFetchLimit: opts.autoFetchLimit - 1 }, function (err, Assoc) {
 		if (!err) {
-			Instance[association.name] = Assoc;
+			// Set this way to prevent setting 'changed' status
+			Instance.__opts.associations[association.name].value = Assoc;
 		}
 
 		return cb();

lib/Instance.js

@@ -11,6 +11,7 @@ function Instance(Model, opts) {
 	opts.id = opts.id || "id";
 	opts.changes = (opts.is_new ? Object.keys(opts.data) : []);
 	opts.extrachanges = [];
+	opts.associations = {};
 
 	var instance_saving = false;
 	var events = {};
@@ -297,18 +298,18 @@ function Instance(Model, opts) {
 
 
 		var _saveManyAssociation = function (assoc) {
-			if (!instance.hasOwnProperty(assoc.name)) return;
-			if (!Array.isArray(instance[assoc.name])) {
-				instance[assoc.name] = [ instance[assoc.name] ];
-			}
+			var assocVal = instance[assoc.name];
+
+			if (!Array.isArray(assocVal)) return;
+			if (!opts.associations[assoc.name].changed) return;
 
-			for (j = 0; j < instance[assoc.name].length; j++) {
-				if (!instance[assoc.name][j].isInstance) {
-					instance[assoc.name][j] = new assoc.model(instance[assoc.name][j]);
+			for (j = 0; j < assocVal.length; j++) {
+				if (!assocVal[j].isInstance) {
+					assocVal[j] = new assoc.model(assocVal[j]);
 				}
 			}
 
-			return saveAssociation(assoc.setAccessor, instance[assoc.name]);
+			saveAssociation(assoc.setAccessor, assocVal);
 		};
 
 		for (i = 0; i < opts.many_associations.length; i++) {
@@ -619,6 +620,10 @@ function Instance(Model, opts) {
 		},
 		enumerable: false
 	});
+	Object.defineProperty(instance, "__opts", {
+		value: opts,
+		enumerable: false
+	});
 	Object.defineProperty(instance, "model", {
 		value: function (cb) {
 			return Model;
@@ -632,6 +637,9 @@ function Instance(Model, opts) {
 			break;
 		}
 	}
+
+	opts.setupAssociations(instance);
+
 	for (i = 0; i < opts.one_associations.length; i++) {
 	    var asc = opts.one_associations[i];
 
@@ -666,9 +674,14 @@ function Instance(Model, opts) {
 		}
 	}
 	for (i = 0; i < opts.many_associations.length; i++) {
-		if (opts.data.hasOwnProperty(opts.many_associations[i].name)) {
-			instance[opts.many_associations[i].name] = opts.data[opts.many_associations[i].name];
-			delete opts.data[opts.many_associations[i].name];
+		var aName = opts.many_associations[i].name;
+		opts.associations[aName] = {
+			changed: false, data: opts.many_associations[i]
+		};
+
+		if (Array.isArray(opts.data[aName])) {
+			instance[aName] = opts.data[aName];
+			delete opts.data[aName];
 		}
 	}
 

lib/Model.js

@@ -83,6 +83,13 @@ function Model(opts) {
 			autoFetchLimit : inst_opts.autoFetchLimit,
 			cascadeRemove  : inst_opts.cascadeRemove
 		};
+
+		var setupAssociations = function (instance) {
+			OneAssociation.extend(model, instance, opts.driver, one_associations, assoc_opts);
+			ManyAssociation.extend(model, instance, opts.driver, many_associations, assoc_opts, createInstance);
+			ExtendAssociation.extend(model, instance, opts.driver, extend_associations, assoc_opts);
+		};
+
 		var pending  = 2, create_err = null;
 		var instance = new Instance(model, {
 			uid                    : inst_opts.uid, // singleton unique id
@@ -101,7 +108,8 @@ function Model(opts) {
 			one_associations       : one_associations,
 			many_associations      : many_associations,
 			extend_associations    : extend_associations,
-			association_properties : association_properties
+			association_properties : association_properties,
+			setupAssociations      : setupAssociations
 		});
 		instance.on("ready", function (err) {
 			if (--pending > 0) {
@@ -115,9 +123,6 @@ function Model(opts) {
 		if (model_fields !== null) {
 			LazyLoad.extend(instance, model, opts.properties);
 		}
-		OneAssociation.extend(model, instance, opts.driver, one_associations, assoc_opts);
-		ManyAssociation.extend(model, instance, opts.driver, many_associations, assoc_opts, createInstance);
-		ExtendAssociation.extend(model, instance, opts.driver, extend_associations, assoc_opts);
 
 		OneAssociation.autoFetch(instance, one_associations, assoc_opts, function () {
 			ManyAssociation.autoFetch(instance, many_associations, assoc_opts, function () {

test/integration/association-hasmany.js

@@ -541,5 +541,67 @@ describe("hasMany", function () {
 				});
 			});
 		});
+
+		it("should not auto save associations which were autofetched", function (done) {
+			Pet.all(function (err, pets) {
+				should.not.exist(err);
+				should.equal(pets.length, 2);
+
+				Person.create({ name: 'Paul' }, function (err, paul) {
+					should.not.exist(err);
+
+					Person.one({ name: 'Paul' }, function (err, paul2) {
+						should.not.exist(err);
+						should.equal(paul2.pets.length, 0);
+
+						paul.setPets(pets, function (err) {
+							should.not.exist(err);
+
+							// reload paul to make sure we have 2 pets
+							Person.one({ name: 'Paul' }, function (err, paul) {
+								should.not.exist(err);
+								should.equal(paul.pets.length, 2);
+
+								// Saving paul2 should NOT auto save associations and hence delete
+								// the associations we just created.
+								paul2.save(function (err) {
+									should.not.exist(err);
+
+									// let's check paul - pets should still be associated
+									Person.one({ name: 'Paul' }, function (err, paul) {
+										should.not.exist(err);
+										should.equal(paul.pets.length, 2);
+
+										done();
+									});
+								});
+							});
+						});
+					});
+				});
+			});
+		});
+
+		it("should save associations set by the user", function (done) {
+			Person.one({ name: 'John' }, function (err, john) {
+				should.not.exist(err);
+				should.equal(john.pets.length, 2);
+
+				john.pets = [];
+
+				john.save(function (err) {
+					should.not.exist(err);
+
+					// reload john to make sure pets were deleted
+					Person.one({ name: 'John' }, function (err, john) {
+						should.not.exist(err);
+						should.equal(john.pets.length, 0);
+
+						done();
+					});
+				});
+			});
+		});
+
 	});
 });