Skip to content

Commit

Permalink
Rewrote utils.checkConditions dresende#293
Browse files Browse the repository at this point in the history 
Improves support for multi-key objects, still missing the ability to
handle multi-value conditions for multi-key objects.
Added a test to verify that it does work correctly for single-primarykey
objects with multiple possible values (generates a `foreignkey` IN
('val1', 'val2'...) statement)
  • Loading branch information
@notheotherben
notheotherben committed Aug 10, 2013
1 parent 7f49c12 commit 680be35
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 39 deletions.
8 changes: 4 additions & 4 deletions lib/Model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ function Model(opts) {
order = Utilities.standardizeOrder(order);
}
if (conditions) {
conditions = Utilities.checkConditions(conditions, model_fields, one_associations);
conditions = Utilities.checkConditions(conditions, one_associations);
}

var chain = new ChainFind(model, {
Expand Down Expand Up @@ -464,7 +464,7 @@ function Model(opts) {
}

if (conditions) {
conditions = Utilities.checkConditions(conditions, model_fields, one_associations);
conditions = Utilities.checkConditions(conditions, one_associations);
}

opts.driver.count(opts.table, conditions, {}, function (err, data) {
Expand All @@ -491,7 +491,7 @@ function Model(opts) {
}

if (conditions) {
conditions = Utilities.checkConditions(conditions, model_fields, one_associations);
conditions = Utilities.checkConditions(conditions, one_associations);
}

return new require("./AggregateFunctions")({
Expand Down Expand Up @@ -528,7 +528,7 @@ function Model(opts) {
}

if (conditions) {
conditions = Utilities.checkConditions(conditions, model_fields, one_associations);
conditions = Utilities.checkConditions(conditions, one_associations);
}

opts.driver.count(opts.table, conditions, {}, function (err, data) {
Expand Down
99 changes: 66 additions & 33 deletions lib/Utilities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,40 +45,73 @@ exports.standardizeOrder = function (order) {
};

/**
* Checks for:
*
* A) condition keys being association names of a model;
* B) condition values being instances;
* Operations
* A) Build an index of associations, with their name as the key
* B) Check for any conditions with a key in the association index
* C) Ensure that our condition supports array values
* D) Remove original condition (not DB compatible)
* E) Convert our association fields into an array, indexes are the same as model.id
* F) Itterate through values for the condition, only accept instances of the same type as the association
*/
exports.checkConditions = function (conditions, model_fields, one_associations) {
var k, i;

for (k in conditions) {
if (!conditions.hasOwnProperty(k)) continue;

// B)
if (Array.isArray(conditions[k])) {
for (i = 0; i < conditions[k].length; i++) {
if (conditions[k][i].isInstance) {
conditions[k][i] = conditions[k][i][conditions[k][i].model().id];
}
}
} else if (conditions[k].isInstance) {
conditions[k] = conditions[k][conditions[k].model().id];
}

// A)
if (model_fields.indexOf(k) == -1) {
for (i = 0; i < one_associations.length; i++) {
if (one_associations[i].name == k) {
conditions[Object.keys(one_associations[i].field).pop()] = conditions[k];
delete conditions[k];
}
}
}
}

return conditions;
exports.checkConditions = function (conditions, one_associations) {
var k, i, j;

// A)
var associations = {};
for (i = 0; i < one_associations.length; i++) {
associations[one_associations[i].name] = one_associations[i];
}

var or_conditions = [];

for (k in conditions) {
// B)
if (!associations.hasOwnProperty(k)) continue;

// C)
var values = conditions[k];
if (!Array.isArray(values)) values = [values];

// D)
delete conditions[k];

// E)
var association_fields = Object.keys(associations[k].field);
var model = associations[k].model;


// F)
//TODO: Need some way to specify OR conditions for sql-query
// If we get two instances for a column, with multiple keys,
// we should create two sub-conditions:
// WHERE (id1 = '...' AND id2 = '...') OR (id1 = ',,,' AND id2 = ',,,')
//ISSUE: dresende/node-orm2#293
for (i = 0; i < values.length; i++) {
if (values[i].isInstance && values[i].model() === model) {
if (association_fields.length == 1) {
if (typeof conditions[association_fields[0]] == 'undefined')
conditions[association_fields[0]] = values[i][model.id[0]];
else if(Array.isArray(conditions[association_fields[0]]))
conditions[association_fields[0]].push(values[i][model.id[0]]);
else
conditions[association_fields[0]] = [conditions[association_fields[0]], values[i][model.id[0]]];
} else {
if(values.length > 1) throw new Error('ORM does not yet support multiple values for multi-key objects');

var _conds = {};
for (j = 0; j < association_fields.length; i++) {
_conds[association_fields[j]] = values[i][model.id[j]];
}

or_conditions.push(_conds);
}
}
}
}

// G)
//TODO: Merge conditions with or_conditions once F's TODO is fixed
return conditions;
};

/**
Expand Down
34 changes: 32 additions & 2 deletions test/integration/association-hasone-reverse.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe("hasOne", function () {
Person.find({ pet_id: Deco.id }).first(function (err, owner) {
should.not.exist(err);
should.exist(owner);
should.equal(owner.id, John.id);
owner.id.should.equal(John.id);
done();
});

Expand All @@ -158,7 +158,7 @@ describe("hasOne", function () {
Person.find({ pet: Deco }).first(function (err, owner) {
should.not.exist(err);
should.exist(owner);
should.equal(owner.id, John.id);
owner.id.should.equal(John.id);
done();
});

Expand All @@ -167,6 +167,36 @@ describe("hasOne", function () {
});
});
});

it("should be able to find given a number of association instances with a single primary key", function (done) {
Person.find({ name: "John Doe" }).first(function (err, John) {
should.not.exist(err);
should.exist(John);
Pet.all(function (err, pets) {
should.not.exist(err);
should.exist(pets);
should.equal(pets.length, 2);

pets[0].hasOwner(function (err, has_owner) {
should.not.exist(err);
has_owner.should.be.false;

pets[0].setOwner(John, function (err) {
should.not.exist(err);

Person.find({ pet: pets }, function (err, owners) {
should.not.exist(err);
should.exist(owners);
owners.length.should.equal(1);

owners[0].id.should.equal(John.id);
done();
});
});
});
});
});
});
});
});
});

0 comments on commit 680be35

Please sign in to comment.