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

Commit 2f7ac2d8 by Michael Kaufman Committed by Felix Becker

7425 Association identifiers in ordering and groups (#7454)

1 parent 2909ec1e
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
- [FIXED] Show a reasonable message when using renameColumn with a missing column [#6606](https://github.com/sequelize/sequelize/issues/6606) - [FIXED] Show a reasonable message when using renameColumn with a missing column [#6606](https://github.com/sequelize/sequelize/issues/6606)
- [PERFORMANCE] more efficient array handing for certain large queries [#7175](https://github.com/sequelize/sequelize/pull/7175) - [PERFORMANCE] more efficient array handing for certain large queries [#7175](https://github.com/sequelize/sequelize/pull/7175)
- [FIXED] Add `unique` indexes defined via options to `rawAttributes` [#7196] - [FIXED] Add `unique` indexes defined via options to `rawAttributes` [#7196]
- [FIXED] Removed support where `order` value is string and interpreted as `Sequelize.literal()`. [#6935](https://github.com/sequelize/sequelize/issues/6935) - [REMOVED] Removed support where `order` value is string and interpreted as `Sequelize.literal()`. [#6935](https://github.com/sequelize/sequelize/issues/6935)
- [CHANGED] `DataTypes.DATE` to use `DATETIMEOFFSET` [MSSQL] [#5403](https://github.com/sequelize/sequelize/issues/5403) - [CHANGED] `DataTypes.DATE` to use `DATETIMEOFFSET` [MSSQL] [#5403](https://github.com/sequelize/sequelize/issues/5403)
- [FIXED] Properly pass options to `sequelize.query` in `removeColumn` [MSSQL] [#7193](https://github.com/sequelize/sequelize/pull/7193) - [FIXED] Properly pass options to `sequelize.query` in `removeColumn` [MSSQL] [#7193](https://github.com/sequelize/sequelize/pull/7193)
- [FIXED] Updating `VIRTUAL` field throw `ER_EMPTY_QUERY` [#6356](https://github.com/sequelize/sequelize/issues/6356) - [FIXED] Updating `VIRTUAL` field throw `ER_EMPTY_QUERY` [#6356](https://github.com/sequelize/sequelize/issues/6356)
...@@ -63,6 +63,8 @@ ...@@ -63,6 +63,8 @@
- [FIXED] `bulkCreate` now runs in O(N) time instead of O(N^2) time. [#4247](https://github.com/sequelize/sequelize/issues/4247) - [FIXED] `bulkCreate` now runs in O(N) time instead of O(N^2) time. [#4247](https://github.com/sequelize/sequelize/issues/4247)
- [FIXED] Passing parameters to model getters [#7404](https://github.com/sequelize/sequelize/issues/7404) - [FIXED] Passing parameters to model getters [#7404](https://github.com/sequelize/sequelize/issues/7404)
- [FIXED] Model.validate runs validation hooks by default [#7182](https://github.com/sequelize/sequelize/pull/7182) - [FIXED] Model.validate runs validation hooks by default [#7182](https://github.com/sequelize/sequelize/pull/7182)
- [ADDED] Added support for associations aliases in orders and groups. [#7425](https://github.com/sequelize/sequelize/issues/7425)
- [REMOVED] Removes support for `{raw: 'injection goes here'}` for order and group. [#7188](https://github.com/sequelize/sequelize/issues/7188)
## BC breaks: ## BC breaks:
- Model.validate instance method now runs validation hooks by default. Previously you needed to pass { hooks: true }. You can override this behavior by passing { hooks: false } - Model.validate instance method now runs validation hooks by default. Previously you needed to pass { hooks: true }. You can override this behavior by passing { hooks: false }
......
...@@ -305,8 +305,6 @@ something.findOne({ ...@@ -305,8 +305,6 @@ something.findOne({
// will return otherfunction(`col1`, 12, 'lalala') DESC // will return otherfunction(`col1`, 12, 'lalala') DESC
[sequelize.fn('otherfunction', sequelize.fn('awesomefunction', sequelize.col('col'))), 'DESC'] [sequelize.fn('otherfunction', sequelize.fn('awesomefunction', sequelize.col('col'))), 'DESC']
// will return otherfunction(awesomefunction(`col`)) DESC, This nesting is potentially infinite! // will return otherfunction(awesomefunction(`col`)) DESC, This nesting is potentially infinite!
[{ raw: 'otherfunction(awesomefunction(`col`))' }, 'DESC']
// This won't be quoted, but direction will be added
] ]
}) })
``` ```
......
...@@ -270,10 +270,10 @@ Project.findAll({ offset: 5, limit: 5 }) ...@@ -270,10 +270,10 @@ Project.findAll({ offset: 5, limit: 5 })
`order` takes an array of items to order the query by or a sequelize method. Generally you will want to use a tuple/array of either attribute, direction or just direction to ensure proper escaping. `order` takes an array of items to order the query by or a sequelize method. Generally you will want to use a tuple/array of either attribute, direction or just direction to ensure proper escaping.
```js ```js
something.findOne({ Subtask.findAll({
order: [ order: [
// Will escape username and validate DESC against a list of valid direction parameters // Will escape username and validate DESC against a list of valid direction parameters
['username', 'DESC'], ['title', 'DESC'],
// Will order by max(age) // Will order by max(age)
sequelize.fn('max', sequelize.col('age')), sequelize.fn('max', sequelize.col('age')),
...@@ -284,22 +284,38 @@ something.findOne({ ...@@ -284,22 +284,38 @@ something.findOne({
// Will order by otherfunction(`col1`, 12, 'lalala') DESC // Will order by otherfunction(`col1`, 12, 'lalala') DESC
[sequelize.fn('otherfunction', sequelize.col('col1'), 12, 'lalala'), 'DESC'], [sequelize.fn('otherfunction', sequelize.col('col1'), 12, 'lalala'), 'DESC'],
// Will order by name on an associated User // Will order an associated model's created_at using the model name as the association's name.
[User, 'name', 'DESC'], [Task, 'createdAt', 'DESC'],
// Will order by name on an associated User aliased as Friend // Will order through an associated model's created_at using the model names as the associations' names.
[{model: User, as: 'Friend'}, 'name', 'DESC'], [Task, Project, 'createdAt', 'DESC'],
// Will order by name on a nested associated Company of an associated User // Will order by an associated model's created_at using the name of the association.
[User, Company, 'name', 'DESC'], ['Task', 'createdAt', 'DESC'],
// Will order by a nested associated model's created_at using the names of the associations.
['Task', 'Project', 'createdAt', 'DESC'],
// Will order by an associated model's created_at using an association object. (preferred method)
[Subtask.associations.Task, 'createdAt', 'DESC'],
// Will order by a nested associated model's created_at using association objects. (preferred method)
[Subtask.associations.Task, Task.associations.Project, 'createdAt', 'DESC'],
// Will order by an associated model's created_at using a simple association object.
[{model: Task, as: 'Task'}, 'createdAt', 'DESC'],
// Will order by a nested associated model's created_at simple association objects.
[{model: Task, as: 'Task'}, {model: Project, as: 'Project'}, 'createdAt', 'DESC']
] ]
// Will order by max age descending // Will order by max age descending
order: sequelize.literal('max(age) DESC') order: sequelize.literal('max(age) DESC')
// Will order by max age ascencding assuming ascencding is the default order when direction is omitted // Will order by max age ascending assuming ascending is the default order when direction is omitted
order: sequelize.fn('max', sequelize.col('age')) order: sequelize.fn('max', sequelize.col('age'))
// Will order by age ascencding assuming ascencding is the default order when direction is omitted // Will order by age ascending assuming ascending is the default order when direction is omitted
order: sequelize.col('age') order: sequelize.col('age')
}) })
``` ```
...@@ -183,39 +183,6 @@ function mapOptionFieldNames(options, Model) { ...@@ -183,39 +183,6 @@ function mapOptionFieldNames(options, Model) {
options.where = mapWhereFieldNames(options.where, Model); options.where = mapWhereFieldNames(options.where, Model);
} }
if (Array.isArray(options.order)) {
for (const oGroup of options.order) {
let OrderModel;
let attr;
let attrOffset;
if (Array.isArray(oGroup)) {
OrderModel = Model;
// Check if we have ['attr', 'DESC'] or [Model, 'attr', 'DESC']
if (typeof oGroup[oGroup.length - 2] === 'string') {
attrOffset = 2;
// Assume ['attr'], [Model, 'attr'] or [seq.fn('somefn', 1), 'DESC']
} else {
attrOffset = 1;
}
attr = oGroup[oGroup.length - attrOffset];
if (oGroup.length > attrOffset) {
OrderModel = oGroup[oGroup.length - (attrOffset + 1)];
if (OrderModel.model) {
OrderModel = OrderModel.model;
}
}
if (OrderModel.rawAttributes && OrderModel.rawAttributes[attr] && attr !== OrderModel.rawAttributes[attr].field) {
oGroup[oGroup.length - attrOffset] = OrderModel.rawAttributes[attr].field;
}
}
}
}
return options; return options;
} }
exports.mapOptionFieldNames = mapOptionFieldNames; exports.mapOptionFieldNames = mapOptionFieldNames;
......
...@@ -73,23 +73,6 @@ describe(Support.getTestDialectTeaser('Model'), function() { ...@@ -73,23 +73,6 @@ describe(Support.getTestDialectTeaser('Model'), function() {
return this.sequelize.sync({force: true}); return this.sequelize.sync({force: true});
}); });
it('should throw when 2nd order argument is not ASC or DESC', function () {
return expect(this.User.findAll({
order: [
['id', ';DELETE YOLO INJECTIONS']
]
})).to.eventually.be.rejectedWith(Error, 'Order must be \'ASC\' or \'DESC\', \';DELETE YOLO INJECTIONS\' given');
});
it('should throw with include when last order argument is not ASC or DESC', function () {
return expect(this.User.findAll({
include: [this.Group],
order: [
[this.Group, 'id', ';DELETE YOLO INJECTIONS']
]
})).to.eventually.be.rejectedWith(Error, 'Order must be \'ASC\' or \'DESC\', \';DELETE YOLO INJECTIONS\' given');
});
if (current.dialect.supports['ORDER NULLS']) { if (current.dialect.supports['ORDER NULLS']) {
it('should not throw with on NULLS LAST/NULLS FIRST', function () { it('should not throw with on NULLS LAST/NULLS FIRST', function () {
return this.User.findAll({ return this.User.findAll({
......
...@@ -212,11 +212,6 @@ if (dialect === 'mysql') { ...@@ -212,11 +212,6 @@ if (dialect === 'mysql') {
context: QueryGenerator, context: QueryGenerator,
needsSequelize: true needsSequelize: true
}, { }, {
title: 'raw arguments are neither quoted nor escaped',
arguments: ['myTable', {order: [[{raw: 'f1(f2(id))'}, 'DESC']]}],
expectation: 'SELECT * FROM `myTable` ORDER BY f1(f2(id)) DESC;',
context: QueryGenerator
}, {
title: 'functions can take functions as arguments', title: 'functions can take functions as arguments',
arguments: ['myTable', function(sequelize) { arguments: ['myTable', function(sequelize) {
return { return {
...@@ -577,6 +572,7 @@ if (dialect === 'mysql') { ...@@ -577,6 +572,7 @@ if (dialect === 'mysql') {
} }
QueryGenerator.options = _.assign(context.options, { timezone: '+00:00' }); QueryGenerator.options = _.assign(context.options, { timezone: '+00:00' });
QueryGenerator._dialect = this.sequelize.dialect; QueryGenerator._dialect = this.sequelize.dialect;
QueryGenerator.sequelize = this.sequelize;
var conditions = QueryGenerator[suiteTitle].apply(QueryGenerator, test.arguments); var conditions = QueryGenerator[suiteTitle].apply(QueryGenerator, test.arguments);
expect(conditions).to.deep.equal(test.expectation); expect(conditions).to.deep.equal(test.expectation);
}); });
......
...@@ -279,25 +279,20 @@ if (dialect.match(/^postgres/)) { ...@@ -279,25 +279,20 @@ if (dialect.match(/^postgres/)) {
expectation: 'SELECT * FROM "myTable" AS "myTable" ORDER BY "myTable"."id" DESC;', expectation: 'SELECT * FROM "myTable" AS "myTable" ORDER BY "myTable"."id" DESC;',
context: QueryGenerator, context: QueryGenerator,
needsSequelize: true needsSequelize: true
},{ }, {
arguments: ['myTable', {order: [['id', 'DESC'], ['name']]}, function(sequelize) {return sequelize.define('myTable', {});}], arguments: ['myTable', {order: [['id', 'DESC'], ['name']]}, function(sequelize) {return sequelize.define('myTable', {});}],
expectation: 'SELECT * FROM "myTable" AS "myTable" ORDER BY "myTable"."id" DESC, "myTable"."name";', expectation: 'SELECT * FROM "myTable" AS "myTable" ORDER BY "myTable"."id" DESC, "myTable"."name";',
context: QueryGenerator, context: QueryGenerator,
needsSequelize: true needsSequelize: true
},{ }, {
title: 'uses limit 0', title: 'uses limit 0',
arguments: ['myTable', {limit: 0}], arguments: ['myTable', {limit: 0}],
expectation: 'SELECT * FROM "myTable" LIMIT 0;', expectation: 'SELECT * FROM "myTable" LIMIT 0;',
context: QueryGenerator context: QueryGenerator
}, { }, {
title: 'uses offset 0', title: 'uses offset 0',
arguments: ['myTable', {offset: 0}], arguments: ['myTable', {offset: 0}],
expectation: 'SELECT * FROM "myTable" OFFSET 0;', expectation: 'SELECT * FROM "myTable" OFFSET 0;',
context: QueryGenerator
}, {
title: 'raw arguments are neither quoted nor escaped',
arguments: ['myTable', {order: [[{raw: 'f1(f2(id))'},'DESC']]}],
expectation: 'SELECT * FROM "myTable" ORDER BY f1(f2(id)) DESC;',
context: QueryGenerator context: QueryGenerator
}, { }, {
title: 'sequelize.where with .fn as attribute and default comparator', title: 'sequelize.where with .fn as attribute and default comparator',
...@@ -325,7 +320,7 @@ if (dialect.match(/^postgres/)) { ...@@ -325,7 +320,7 @@ if (dialect.match(/^postgres/)) {
expectation: 'SELECT * FROM "myTable" WHERE (LOWER("user"."name") LIKE \'%t%\' AND "myTable"."type" = 1);', expectation: 'SELECT * FROM "myTable" WHERE (LOWER("user"."name") LIKE \'%t%\' AND "myTable"."type" = 1);',
context: QueryGenerator, context: QueryGenerator,
needsSequelize: true needsSequelize: true
},{ }, {
title: 'functions can take functions as arguments', title: 'functions can take functions as arguments',
arguments: ['myTable', function(sequelize) { arguments: ['myTable', function(sequelize) {
return { return {
...@@ -923,6 +918,7 @@ if (dialect.match(/^postgres/)) { ...@@ -923,6 +918,7 @@ if (dialect.match(/^postgres/)) {
} }
QueryGenerator.options = _.assign(context.options, { timezone: '+00:00' }); QueryGenerator.options = _.assign(context.options, { timezone: '+00:00' });
QueryGenerator._dialect = this.sequelize.dialect; QueryGenerator._dialect = this.sequelize.dialect;
QueryGenerator.sequelize = this.sequelize;
var conditions = QueryGenerator[suiteTitle].apply(QueryGenerator, test.arguments); var conditions = QueryGenerator[suiteTitle].apply(QueryGenerator, test.arguments);
expect(conditions).to.deep.equal(test.expectation); expect(conditions).to.deep.equal(test.expectation);
}); });
......
...@@ -198,11 +198,6 @@ if (dialect === 'sqlite') { ...@@ -198,11 +198,6 @@ if (dialect === 'sqlite') {
context: QueryGenerator, context: QueryGenerator,
needsSequelize: true needsSequelize: true
}, { }, {
title: 'raw arguments are neither quoted nor escaped',
arguments: ['myTable', {order: [[{raw: 'f1(f2(id))'}, 'DESC']]}],
expectation: 'SELECT * FROM `myTable` ORDER BY f1(f2(id)) DESC;',
context: QueryGenerator
}, {
title: 'sequelize.where with .fn as attribute and default comparator', title: 'sequelize.where with .fn as attribute and default comparator',
arguments: ['myTable', function(sequelize) { arguments: ['myTable', function(sequelize) {
return { return {
...@@ -536,6 +531,7 @@ if (dialect === 'sqlite') { ...@@ -536,6 +531,7 @@ if (dialect === 'sqlite') {
} }
QueryGenerator.options = _.assign(context.options, { timezone: '+00:00' }); QueryGenerator.options = _.assign(context.options, { timezone: '+00:00' });
QueryGenerator._dialect = this.sequelize.dialect; QueryGenerator._dialect = this.sequelize.dialect;
QueryGenerator.sequelize = this.sequelize;
var conditions = QueryGenerator[suiteTitle].apply(QueryGenerator, test.arguments); var conditions = QueryGenerator[suiteTitle].apply(QueryGenerator, test.arguments);
expect(conditions).to.deep.equal(test.expectation); expect(conditions).to.deep.equal(test.expectation);
}); });
......
...@@ -195,255 +195,6 @@ suite(Support.getTestDialectTeaser('Utils'), () => { ...@@ -195,255 +195,6 @@ suite(Support.getTestDialectTeaser('Utils'), () => {
} }
}); });
}); });
test('string field order', function() {
expect(Utils.mapOptionFieldNames({
order: 'firstName DESC'
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'first_name'
}
}))).to.eql({
order: 'firstName DESC'
});
});
test('string in array order', function() {
expect(Utils.mapOptionFieldNames({
order: ['firstName DESC']
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'first_name'
}
}))).to.eql({
order: ['firstName DESC']
});
});
test('single field alias order', function() {
expect(Utils.mapOptionFieldNames({
order: [['firstName', 'DESC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'first_name'
}
}))).to.eql({
order: [['first_name', 'DESC']]
});
});
test('multi field alias order', function() {
expect(Utils.mapOptionFieldNames({
order: [['firstName', 'DESC'], ['lastName', 'ASC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'first_name'
},
lastName: {
type: DataTypes.STRING,
field: 'last_name'
}
}))).to.eql({
order: [['first_name', 'DESC'], ['last_name', 'ASC']]
});
});
test('multi field alias no direction order', function() {
expect(Utils.mapOptionFieldNames({
order: [['firstName'], ['lastName']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'first_name'
},
lastName: {
type: DataTypes.STRING,
field: 'last_name'
}
}))).to.eql({
order: [['first_name'], ['last_name']]
});
});
test('field alias to another field order', function() {
expect(Utils.mapOptionFieldNames({
order: [['firstName', 'DESC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'lastName'
},
lastName: {
type: DataTypes.STRING,
field: 'firstName'
}
}))).to.eql({
order: [['lastName', 'DESC']]
});
});
test('multi field no alias order', function() {
expect(Utils.mapOptionFieldNames({
order: [['firstName', 'DESC'], ['lastName', 'ASC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING
},
lastName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [['firstName', 'DESC'], ['lastName', 'ASC']]
});
});
test('multi field alias sub model order', function() {
const Location = Support.sequelize.define('Location', {
latLong: {
type: DataTypes.STRING,
field: 'lat_long'
}
});
const Item = Support.sequelize.define('Item', {
fontColor: {
type: DataTypes.STRING,
field: 'font_color'
}
});
expect(Utils.mapOptionFieldNames({
order: [[Item, Location, 'latLong', 'DESC'], ['lastName', 'ASC']]
}, Support.sequelize.define('User', {
lastName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[Item, Location, 'lat_long', 'DESC'], ['lastName', 'ASC']]
});
});
test('multi field alias sub model no direction order', function() {
const Location = Support.sequelize.define('Location', {
latLong: {
type: DataTypes.STRING,
field: 'lat_long'
}
});
const Item = Support.sequelize.define('Item', {
fontColor: {
type: DataTypes.STRING,
field: 'font_color'
}
});
expect(Utils.mapOptionFieldNames({
order: [[Item, Location, 'latLong'], ['lastName', 'ASC']]
}, Support.sequelize.define('User', {
lastName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[Item, Location, 'lat_long'], ['lastName', 'ASC']]
});
});
test('function order', function() {
const fn = Support.sequelize.fn('otherfn', 123);
expect(Utils.mapOptionFieldNames({
order: [[fn, 'ASC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[fn, 'ASC']]
});
});
test('function no direction order', function() {
const fn = Support.sequelize.fn('otherfn', 123);
expect(Utils.mapOptionFieldNames({
order: [[fn]]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[fn]]
});
});
test('string no direction order', function() {
expect(Utils.mapOptionFieldNames({
order: [['firstName']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING,
field: 'first_name'
}
}))).to.eql({
order: [['first_name']]
});
});
test('model alias order', function() {
const Item = Support.sequelize.define('Item', {
fontColor: {
type: DataTypes.STRING,
field: 'font_color'
}
});
expect(Utils.mapOptionFieldNames({
order: [[{ model: Item, as: 'another'}, 'fontColor', 'ASC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING
},
lastName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[{ model: Item, as: 'another'}, 'font_color', 'ASC']]
});
});
test('model alias no direction order', function() {
const Item = Support.sequelize.define('Item', {
fontColor: {
type: DataTypes.STRING,
field: 'font_color'
}
});
expect(Utils.mapOptionFieldNames({
order: [[{ model: Item, as: 'another'}, 'fontColor']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[{ model: Item, as: 'another'}, 'font_color']]
});
});
test('model alias wrong field order', function() {
const Item = Support.sequelize.define('Item', {
fontColor: {
type: DataTypes.STRING,
field: 'font_color'
}
});
expect(Utils.mapOptionFieldNames({
order: [[{ model: Item, as: 'another'}, 'firstName', 'ASC']]
}, Support.sequelize.define('User', {
firstName: {
type: DataTypes.STRING
}
}))).to.eql({
order: [[{ model: Item, as: 'another'}, 'firstName', 'ASC']]
});
});
}); });
suite('stack', () => { suite('stack', () => {
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!