From 680be3513532e6717923d45d0bdafec237a8acf8 Mon Sep 17 00:00:00 2001 From: Benjamin Pannell Date: Sat, 10 Aug 2013 14:56:27 +0200 Subject: [PATCH] Rewrote utils.checkConditions #293 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) --- lib/Model.js | 8 +- lib/Utilities.js | 99 ++++++++++++------- .../integration/association-hasone-reverse.js | 34 ++++++- 3 files changed, 102 insertions(+), 39 deletions(-) diff --git a/lib/Model.js b/lib/Model.js index 3c5ff61a..8aef7ca9 100644 --- a/lib/Model.js +++ b/lib/Model.js @@ -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, { @@ -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) { @@ -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")({ @@ -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) { diff --git a/lib/Utilities.js b/lib/Utilities.js index e6a3249a..c677e4ad 100644 --- a/lib/Utilities.js +++ b/lib/Utilities.js @@ -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; }; /** diff --git a/test/integration/association-hasone-reverse.js b/test/integration/association-hasone-reverse.js index 6c6b5079..12016f87 100644 --- a/test/integration/association-hasone-reverse.js +++ b/test/integration/association-hasone-reverse.js @@ -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(); }); @@ -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(); }); @@ -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(); + }); + }); + }); + }); + }); + }); }); }); });