不要怂,就是干,撸起袖子干!

Commit 1219b188 by Mick Hansen

Merge pull request #1189 from mickhansen/include-order-fix

Fix for order by when you only have 1:1 includes
2 parents 6df7d196 81f917ea
...@@ -10,24 +10,25 @@ module.exports = (function() { ...@@ -10,24 +10,25 @@ module.exports = (function() {
this.options = options this.options = options
this.isSingleAssociation = true this.isSingleAssociation = true
this.isSelfAssociation = (this.source.tableName == this.target.tableName) this.isSelfAssociation = (this.source.tableName == this.target.tableName)
this.as = this.options.as
if (this.isSelfAssociation && !this.options.foreignKey && !!this.options.as) { if (this.isSelfAssociation && !this.options.foreignKey && !!this.as) {
this.options.foreignKey = Utils._.underscoredIf(Utils.singularize(this.options.as, this.source.options.language) + "Id", this.source.options.underscored) this.options.foreignKey = Utils._.underscoredIf(Utils.singularize(this.source.tableName, this.source.options.language) + "Id", this.source.options.underscored)
} }
if (!this.options.as) { if (!this.as) {
this.options.as = Utils.singularize(this.target.tableName, this.target.options.language) this.as = this.options.as = Utils.singularize(this.target.tableName, this.target.options.language)
} }
this.associationAccessor = this.isSelfAssociation this.associationAccessor = this.isSelfAssociation
? Utils.combineTableNames(this.target.tableName, this.options.as) ? Utils.combineTableNames(this.target.tableName, this.as)
: this.options.as : this.as
this.options.useHooks = options.useHooks this.options.useHooks = options.useHooks
this.accessors = { this.accessors = {
get: Utils._.camelize('get_' + this.options.as), get: Utils._.camelize('get_' + this.as),
set: Utils._.camelize('set_' + this.options.as) set: Utils._.camelize('set_' + this.as)
} }
} }
...@@ -38,6 +39,7 @@ module.exports = (function() { ...@@ -38,6 +39,7 @@ module.exports = (function() {
, keyType = ((this.target.hasPrimaryKeys && targetKeys.length === 1) ? this.target.rawAttributes[targetKeys[0]].type : DataTypes.INTEGER) , keyType = ((this.target.hasPrimaryKeys && targetKeys.length === 1) ? this.target.rawAttributes[targetKeys[0]].type : DataTypes.INTEGER)
this.identifier = this.options.foreignKey || Utils._.underscoredIf(Utils.singularize(this.target.tableName, this.target.options.language) + "Id", this.source.options.underscored) this.identifier = this.options.foreignKey || Utils._.underscoredIf(Utils.singularize(this.target.tableName, this.target.options.language) + "Id", this.source.options.underscored)
newAttributes[this.identifier] = { type: this.options.keyType || keyType } newAttributes[this.identifier] = { type: this.options.keyType || keyType }
Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.target, this.source, this.options) Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.target, this.source, this.options)
Utils._.defaults(this.source.rawAttributes, newAttributes) Utils._.defaults(this.source.rawAttributes, newAttributes)
......
...@@ -1199,11 +1199,16 @@ module.exports = (function() { ...@@ -1199,11 +1199,16 @@ module.exports = (function() {
var validateIncludedElements = function(options) { var validateIncludedElements = function(options) {
options.includeNames = [] options.includeNames = []
options.includeMap = {} options.includeMap = {}
options.hasSingleAssociation = false
options.hasMultiAssociation = false
options.include = options.include.map(function(include) { options.include = options.include.map(function(include) {
include = validateIncludedElement.call(this, include, options.daoFactory) include = validateIncludedElement.call(this, include, options.daoFactory)
if (include.association.isMultiAssociation) options.hasMultiAssociation = true
if (include.association.isSingleAssociation) options.hasSingleAssociation = true
options.includeMap[include.as] = include options.includeMap[include.as] = include
options.includeNames.push(include.as) options.includeNames.push(include.as)
return include return include
}.bind(this)) }.bind(this))
}; };
......
...@@ -437,7 +437,7 @@ module.exports = (function() { ...@@ -437,7 +437,7 @@ module.exports = (function() {
options.attributes = optAttributes.join(', ') options.attributes = optAttributes.join(', ')
} }
var conditionalJoins = this.getConditionalJoins(options, factory), var conditionalJoins = ((options.hasMultiAssociation && (options.limit || options.offset)) || !options.include) && this.getConditionalJoins(options, factory),
query; query;
if (conditionalJoins) { if (conditionalJoins) {
...@@ -449,7 +449,7 @@ module.exports = (function() { ...@@ -449,7 +449,7 @@ module.exports = (function() {
} }
if (options.hasOwnProperty('where')) { if (options.hasOwnProperty('where')) {
options.where = this.getWhereConditions(options.where, tableName, factory) options.where = this.getWhereConditions(options.where, tableName, factory, options)
query += " WHERE " + options.where query += " WHERE " + options.where
} }
...@@ -535,13 +535,13 @@ module.exports = (function() { ...@@ -535,13 +535,13 @@ module.exports = (function() {
/* /*
Takes something and transforms it into values of a where condition. Takes something and transforms it into values of a where condition.
*/ */
getWhereConditions: function(smth, tableName, factory) { getWhereConditions: function(smth, tableName, factory, options) {
var result = null var result = null
, where = {} , where = {}
if (Utils.isHash(smth)) { if (Utils.isHash(smth)) {
smth = Utils.prependTableNameToHash(tableName, smth) smth = Utils.prependTableNameToHash(tableName, smth)
result = this.hashToWhereConditions(smth, factory) result = this.hashToWhereConditions(smth, factory, options)
} else if (typeof smth === 'number') { } else if (typeof smth === 'number') {
var primaryKeys = !!factory ? Object.keys(factory.primaryKeys) : [] var primaryKeys = !!factory ? Object.keys(factory.primaryKeys) : []
if (primaryKeys.length > 0) { if (primaryKeys.length > 0) {
...@@ -601,7 +601,7 @@ module.exports = (function() { ...@@ -601,7 +601,7 @@ module.exports = (function() {
return dao; return dao;
}, },
isAssociationFilter: function(filterStr, dao){ isAssociationFilter: function(filterStr, dao, options){
if(!dao){ if(!dao){
return false; return false;
} }
...@@ -613,7 +613,6 @@ module.exports = (function() { ...@@ -613,7 +613,6 @@ module.exports = (function() {
, attributePart = associationParts.pop() , attributePart = associationParts.pop()
, self = this , self = this
return associationParts.every(function (attribute) { return associationParts.every(function (attribute) {
var association = self.findAssociation(attribute, dao); var association = self.findAssociation(attribute, dao);
if (!association) return false; if (!association) return false;
...@@ -622,16 +621,25 @@ module.exports = (function() { ...@@ -622,16 +621,25 @@ module.exports = (function() {
}) && dao.rawAttributes.hasOwnProperty(attributePart); }) && dao.rawAttributes.hasOwnProperty(attributePart);
}, },
getAssociationFilterColumn: function(filterStr, dao){ getAssociationFilterColumn: function(filterStr, dao, options){
var associationParts = filterStr.split('.') var associationParts = filterStr.split('.')
, attributePart = associationParts.pop() , attributePart = associationParts.pop()
, self = this , self = this
, association
, keyParts = []
associationParts.forEach(function (attribute) { associationParts.forEach(function (attribute) {
dao = self.findAssociation(attribute, dao).target; association = self.findAssociation(attribute, dao)
dao = association.target;
if (options.include) {
keyParts.push(association.as || association.options.as || dao.tableName)
}
}) })
return dao.tableName + '.' + attributePart; if (options.include) {
return this.quoteIdentifier(keyParts.join('.')) + '.' + this.quote(attributePart);
}
return this.quoteIdentifiers(dao.tableName + '.' + attributePart);
}, },
getConditionalJoins: function(options, originalDao){ getConditionalJoins: function(options, originalDao){
...@@ -645,7 +653,7 @@ module.exports = (function() { ...@@ -645,7 +653,7 @@ module.exports = (function() {
, attributePart = associationParts.pop() , attributePart = associationParts.pop()
, dao = originalDao , dao = originalDao
if (self.isAssociationFilter(filterStr, dao)) { if (self.isAssociationFilter(filterStr, dao, options)) {
associationParts.forEach(function (attribute) { associationParts.forEach(function (attribute) {
var association = self.findAssociation(attribute, dao); var association = self.findAssociation(attribute, dao);
...@@ -692,19 +700,21 @@ module.exports = (function() { ...@@ -692,19 +700,21 @@ module.exports = (function() {
Takes a hash and transforms it into a mysql where condition: {key: value, key2: value2} ==> key=value AND key2=value2 Takes a hash and transforms it into a mysql where condition: {key: value, key2: value2} ==> key=value AND key2=value2
The values are transformed by the relevant datatype. The values are transformed by the relevant datatype.
*/ */
hashToWhereConditions: function(hash, dao) { hashToWhereConditions: function(hash, dao, options) {
var result = [] var result = []
for (var key in hash) { for (var key in hash) {
var value = hash[key] var value = hash[key]
, _key
, _value = null
if(this.isAssociationFilter(key, dao)){ if(this.isAssociationFilter(key, dao, options)){
key = this.getAssociationFilterColumn(key, dao); _key = key = this.getAssociationFilterColumn(key, dao, options);
} else {
_key = this.quoteIdentifiers(key)
} }
//handle qualified key names //handle qualified key names
var _key = this.quoteIdentifiers(key)
, _value = null
if (Array.isArray(value)) { if (Array.isArray(value)) {
result.push(this.arrayValue(value, key, _key, dao)) result.push(this.arrayValue(value, key, _key, dao))
......
...@@ -18,7 +18,6 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() { ...@@ -18,7 +18,6 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
Task.belongsTo(Project); Task.belongsTo(Project);
Project.hasMany(Task); Project.hasMany(Task);
this.sequelize.sync({ force: true }).success(function() { this.sequelize.sync({ force: true }).success(function() {
User.bulkCreate([{ User.bulkCreate([{
username: 'leia' username: 'leia'
...@@ -47,8 +46,15 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() { ...@@ -47,8 +46,15 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
Task.findAll({ Task.findAll({
where: { where: {
'project.user.username': 'leia' 'project.user.username': 'leia'
} }/*,
}).success(function(tasks){ include: [
{model: Project, include: [
User
]}
]*/
}).done(function(err, tasks){
expect(err).not.to.be.ok
try{ try{
expect(tasks.length).to.be.equal(2); expect(tasks.length).to.be.equal(2);
expect(tasks[0].title).to.be.equal('fight empire'); expect(tasks[0].title).to.be.equal('fight empire');
...@@ -106,7 +112,12 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() { ...@@ -106,7 +112,12 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
where: { where: {
'project.user.username': 'leia', 'project.user.username': 'leia',
'project.user.id': 1 'project.user.id': 1
} }/*,
include: [
{model: Project, include: [
User
]}
]*/
}).success(function(tasks){ }).success(function(tasks){
try{ try{
expect(tasks.length).to.be.equal(2); expect(tasks.length).to.be.equal(2);
...@@ -134,7 +145,6 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() { ...@@ -134,7 +145,6 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
Task.belongsTo(Project); Task.belongsTo(Project);
Project.hasMany(Task); Project.hasMany(Task);
this.sequelize.sync({ force: true }).success(function() { this.sequelize.sync({ force: true }).success(function() {
User.bulkCreate([{ User.bulkCreate([{
username: 'leia' username: 'leia'
...@@ -164,8 +174,13 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() { ...@@ -164,8 +174,13 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
User.findAll({ User.findAll({
where: { where: {
'projects.tasks.title': 'fight empire' 'projects.tasks.title': 'fight empire'
} }/*,
}).success(function(users){ include: [
{model: Project, include: [
Task
]}
]*/
}).done(function(err, users){
try{ try{
expect(users.length).to.be.equal(1); expect(users.length).to.be.equal(1);
expect(users[0].username).to.be.equal('leia'); expect(users[0].username).to.be.equal('leia');
...@@ -208,7 +223,10 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() { ...@@ -208,7 +223,10 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
User.findAll({ User.findAll({
where: { where: {
'projects.title': 'republic' 'projects.title': 'republic'
} }/*,
include: [
{model: Project}
]*/
}).success(function(users){ }).success(function(users){
try{ try{
expect(users.length).to.be.equal(1); expect(users.length).to.be.equal(1);
......
...@@ -920,8 +920,8 @@ describe(Support.getTestDialectTeaser("DAO"), function () { ...@@ -920,8 +920,8 @@ describe(Support.getTestDialectTeaser("DAO"), function () {
overdue_days: DataTypes.INTEGER overdue_days: DataTypes.INTEGER
}, { timestamps: false }) }, { timestamps: false })
this.UserEager.hasMany(this.ProjectEager, { as: 'Projects' }) this.UserEager.hasMany(this.ProjectEager, { as: 'Projects' })
this.ProjectEager.belongsTo(this.UserEager, { as: 'Poobah' }) this.ProjectEager.belongsTo(this.UserEager, { as: 'Poobah' })
self.UserEager.sync({force: true}).success(function() { self.UserEager.sync({force: true}).success(function() {
self.ProjectEager.sync({force: true}).success(function() { self.ProjectEager.sync({force: true}).success(function() {
...@@ -1016,7 +1016,6 @@ describe(Support.getTestDialectTeaser("DAO"), function () { ...@@ -1016,7 +1016,6 @@ describe(Support.getTestDialectTeaser("DAO"), function () {
self.ProjectEager.create({ title: 'party', overdue_days: 2 }).success(function(party) { self.ProjectEager.create({ title: 'party', overdue_days: 2 }).success(function(party) {
user.setProjects([homework, party]).success(function() { user.setProjects([homework, party]).success(function() {
self.ProjectEager.findAll({include: [{model: self.UserEager, as: 'Poobah'}]}).success(function(projects) { self.ProjectEager.findAll({include: [{model: self.UserEager, as: 'Poobah'}]}).success(function(projects) {
expect(projects.length).to.equal(2) expect(projects.length).to.equal(2)
expect(projects[0].poobah).to.exist expect(projects[0].poobah).to.exist
expect(projects[1].poobah).to.exist expect(projects[1].poobah).to.exist
......
...@@ -743,84 +743,90 @@ describe(Support.getTestDialectTeaser("Include"), function () { ...@@ -743,84 +743,90 @@ describe(Support.getTestDialectTeaser("Include"), function () {
}) })
}) })
xit('should support the order attribute on multiple associated entities', function(done) { it('should support ordering with only belongsTo includes', function(done) {
var User = this.sequelize.define('User', {}) var User = this.sequelize.define('User', {})
, Item = this.sequelize.define('Item', {'test': DataTypes.STRING}) , Item = this.sequelize.define('Item', {'test': DataTypes.STRING})
, Order = this.sequelize.define('Order', {'position': DataTypes.INTEGER}) , Order = this.sequelize.define('Order', {'position': DataTypes.INTEGER})
User.belongsTo(Item, {'as': 'itemA'}) User.belongsTo(Item, {'as': 'itemA', foreignKey: 'itemA_id'})
User.belongsTo(Item, {'as': 'itemB'}) User.belongsTo(Item, {'as': 'itemB', foreignKey: 'itemB_id'})
User.belongsTo(Order) User.belongsTo(Order)
Item.hasMany(User)
Order.hasMany(User) this.sequelize.sync().done(function() {
async.auto({
this.sequelize.sync().done(function() { users: function(callback) {
async.auto({ User.bulkCreate([{}, {}, {}]).done(function() {
users: function(callback) { User.findAll().done(callback)
User.bulkCreate([{}, {}, {}]).done(function() { })
User.findAll().done(callback) },
}) items: function(callback) {
}, Item.bulkCreate([
items: function(callback) { {'test': 'abc'},
Item.bulkCreate([ {'test': 'def'},
{'test': 'abc'}, {'test': 'ghi'},
{'test': 'def'}, {'test': 'jkl'}
{'test': 'ghi'}, ]).done(function() {
{'test': 'jkl'} Item.findAll({order: ['id']}).done(callback)
]).done(function() { })
Item.findAll().done(callback) },
}) orders: function(callback) {
}, Order.bulkCreate([
orders: function(callback) { {'position': 2},
Order.bulkCreate([ {'position': 3},
{'position': 2}, {'position': 1}
{'position': 3}, ]).done(function() {
{'position': 1} Order.findAll({order: ['id']}).done(callback)
]).done(function() { })
Order.findAll().done(callback) },
}) associate: ['users', 'items', 'orders', function(callback, results) {
}, var chainer = new Sequelize.Utils.QueryChainer()
associate: ['users', 'items', 'orders', function(callback, results) {
var chainer = new Sequelize.Utils.QueryChainer()
var user1 = results.users[0] var user1 = results.users[0]
var user2 = results.users[1] var user2 = results.users[1]
var user3 = results.users[2] var user3 = results.users[2]
var item1 = results.items[0] var item1 = results.items[0]
var item2 = results.items[1] var item2 = results.items[1]
var item3 = results.items[2] var item3 = results.items[2]
var item4 = results.items[3] var item4 = results.items[3]
var order1 = results.orders[0] var order1 = results.orders[0]
var order2 = results.orders[1] var order2 = results.orders[1]
var order3 = results.orders[2] var order3 = results.orders[2]
chainer.add(user1.setItemA(item1)) chainer.add(user1.setItemA(item1))
chainer.add(user1.setItemB(item2)) chainer.add(user1.setItemB(item2))
chainer.add(user1.setOrder(order3)) chainer.add(user1.setOrder(order3))
chainer.add(user2.setItemA(item3)) chainer.add(user2.setItemA(item3))
chainer.add(user2.setItemB(item4)) chainer.add(user2.setItemB(item4))
chainer.add(user2.setOrder(order2)) chainer.add(user2.setOrder(order2))
chainer.add(user3.setItemA(item1)) chainer.add(user3.setItemA(item1))
chainer.add(user3.setItemB(item4)) chainer.add(user3.setItemB(item4))
chainer.add(user3.setOrder(order1)) chainer.add(user3.setOrder(order1))
chainer.run().done(callback) chainer.run().done(callback)
}]}, function() { }]
User.findAll({'where': {'itemA.test': 'abc'}, 'include': [{'model': Item, 'as': 'itemA'}, {'model': Item, 'as': 'itemB'}, Order], 'order': 'order.position'}).done(function(err, as) { }, function() {
expect(err).not.to.be.ok User.findAll({
expect(as.length).to.eql(2) 'where': {'itemA.test': 'abc'},
'include': [
{'model': Item, 'as': 'itemA'},
{'model': Item, 'as': 'itemB'},
Order],
'order': ['Order.position']
}).done(function(err, as) {
expect(err).not.to.be.ok
expect(as.length).to.eql(2)
expect(as[0].itemA[0].test).to.eql('abc') expect(as[0].itemA.test).to.eql('abc')
expect(as[1].itemA[0].test).to.eql('abc') expect(as[1].itemA.test).to.eql('abc')
expect(as[0].order.position).to.eql(1) expect(as[0].order.position).to.eql(1)
expect(as[1].order.position).to.eql(3) expect(as[1].order.position).to.eql(2)
done() done()
}) })
}) })
}) })
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!