Skip to content

Commit

Permalink
Changes to how sequelize.literal and cousins is escaped
Browse files Browse the repository at this point in the history
  • Loading branch information
@janmeier
janmeier committed Jun 17, 2014
1 parent 34fddc2 commit 6541fb4
Show file tree
Hide file tree
Showing 13 changed files with 175 additions and 143 deletions.
51 changes: 36 additions & 15 deletions lib/dialects/abstract/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,33 +638,29 @@ module.exports = (function() {
mainAttributes = mainAttributes && mainAttributes.map(function(attr) {
var addTable = true;

if (attr instanceof Utils.literal) {
return attr.toString(this);
}

if (attr instanceof Utils.fn || attr instanceof Utils.col) {
if (attr._isSequelizeMethod) {
return attr.toString(self);
}

if (Array.isArray(attr) && attr.length === 2) {
if (attr[0] instanceof Utils.fn || attr[0] instanceof Utils.col) {
if (attr[0]._isSequelizeMethod) {
attr[0] = attr[0].toString(self);
addTable = false;
} else {
if (attr[0].indexOf('(') === -1 && attr[0].indexOf(')') === -1) {
attr[0] = this.quoteIdentifier(attr[0]);
attr[0] = self.quoteIdentifier(attr[0]);
}
}
attr = [attr[0], this.quoteIdentifier(attr[1])].join(' as ');
attr = [attr[0], self.quoteIdentifier(attr[1])].join(' AS ');
} else {
attr = attr.indexOf(Utils.TICK_CHAR) < 0 && attr.indexOf('"') < 0 ? this.quoteIdentifiers(attr) : attr;
attr = attr.indexOf(Utils.TICK_CHAR) < 0 && attr.indexOf('"') < 0 ? self.quoteIdentifiers(attr) : attr;
}

if (options.include && attr.indexOf('.') === -1 && addTable) {
attr = mainTableAs + '.' + attr;
}
return attr;
}.bind(this));
});

// If no attributes specified, use *
mainAttributes = mainAttributes || (options.include ? [mainTableAs + '.*'] : ['*']);
Expand Down Expand Up @@ -700,18 +696,43 @@ module.exports = (function() {

// includeIgnoreAttributes is used by aggregate functions
if (options.includeIgnoreAttributes !== false) {

attributes = include.attributes.map(function(attr) {
var attrAs = attr;
var attrAs = attr,
verbatim = false;

if (Array.isArray(attr) && attr.length === 2) {
if (attr[0]._isSequelizeMethod) {
if (attr[0] instanceof Utils.literal ||
attr[0] instanceof Utils.cast ||
attr[0] instanceof Utils.fn
) {
verbatim = true;
}
}

attr = attr.map(function($attr) {
return $attr._isSequelizeMethod ? $attr.toString(self) : $attr;
});

attrAs = attr[1];
attr = attr[0];
} else if (attr instanceof Utils.literal) {
return attr.toString(self); // We trust the user to rename the field correctly
} else if (attr instanceof Utils.cast ||
attr instanceof Utils.fn
) {
throw new Error("Tried to select attributes using Sequelize.cast or Sequelize.fn without specifying an alias for the result, during eager loading. " +
"This means the attribute will not be added to the returned instance");
}

var prefix;
if (verbatim === true) {
prefix = attr;
} else {
prefix = self.quoteIdentifier(as) + '.' + self.quoteIdentifier(attr);
}
return self.quoteIdentifier(as) + '.' + self.quoteIdentifier(attr) + ' AS ' + self.quoteIdentifier(as + '.' + attrAs);
return prefix + ' AS ' + self.quoteIdentifier(as + '.' + attrAs);
});

if (include.subQuery && subQuery) {
Expand Down Expand Up @@ -1094,7 +1115,7 @@ module.exports = (function() {

result = (value === 'NULL') ? key + ' IS NULL' : [key, value].join('=');
}
} else if (Utils.isHash(smth)) {
} else if (Utils._.isPlainObject(smth)) {
if (prepend) {
if (tableName) options.keysEscaped = true;
smth = this.prependTableNameToHash(tableName, smth);
Expand Down Expand Up @@ -1125,7 +1146,7 @@ module.exports = (function() {
if (treatAsAnd) {
return treatAsAnd;
} else {
return !(arg instanceof Date) && ((arg instanceof Utils.and) || (arg instanceof Utils.or) || Utils.isHash(arg));
return !(arg instanceof Date) && ((arg instanceof Utils.and) || (arg instanceof Utils.or) || Utils._.isPlainObject(arg));
}
}, false);

Expand Down Expand Up @@ -1246,7 +1267,7 @@ module.exports = (function() {
, self = this
, joinedTables = {};

if (Utils.isHash(options.where)) {
if (Utils._.isPlainObject(options.where)) {
Object.keys(options.where).forEach(function(filterStr) {
var associationParts = filterStr.split('.')
, attributePart = associationParts.pop()
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/mysql/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ module.exports = (function() {
for (var name in attributes) {
var dataType = attributes[name];

if (Utils.isHash(dataType)) {
if (Utils._.isPlainObject(dataType)) {
var template;

if (dataType.type.toString() === DataTypes.ENUM.toString()) {
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/postgres/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ module.exports = (function() {
for (var name in attributes) {
var dataType = attributes[name];

if (Utils.isHash(dataType)) {
if (Utils._.isObject(dataType)) {
var template = '<%= type %>'
, replacements = { type: dataType.type };

Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/sqlite/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ module.exports = (function() {
for (var name in attributes) {
var dataType = attributes[name];

if (Utils.isHash(dataType)) {
if (Utils._.isObject(dataType)) {
var template = "<%= type %>"
, replacements = { type: dataType.type };

Expand Down
6 changes: 3 additions & 3 deletions lib/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ module.exports = (function() {
// If you don't specify a valid data type lets help you debug it
Utils._.each(attributes, function(attribute, name) {
var dataType;
if (Utils.isHash(attribute)) {
if (Utils._.isPlainObject(attribute)) {
// We have special cases where the type is an object containing
// the values (e.g. Sequelize.ENUM(value, value2) returns an object
// instead of a function)
Expand All @@ -81,7 +81,7 @@ module.exports = (function() {
} else {
dataType = attribute;
}

if (dataType === undefined) {
throw new Error('Unrecognized data type for field ' + name);
}
Expand Down Expand Up @@ -639,7 +639,7 @@ module.exports = (function() {
*
* @param {Object} [options] A hash of options to describe the scope of the search
* @param {Object} [options.where] A hash of attributes to describe your search. See above for examples.
* @param {Array<String>} [options.attributes] A list of the attributes that you want to select
* @param {Array<String>} [options.attributes] A list of the attributes that you want to select. To rename an attribute, you can pass an array, with two elements - the first is the name of the attribute in the DB (or some kind of expression such as `Sequelize.literal`, `Sequelize.fn` and so on), and the second is the name you want the attribute to have in the returned instance
* @param {Array<Object|Model>} [options.include] A list of associations to eagerly load using a left join. Supported is either `{ include: [ Model1, Model2, ...]}` or `{ include: [{ model: Model1, as: 'Alias' }]}`. If your association are set up with an `as` (eg. `X.hasMany(Y, { as: 'Z }`, you need to specify Z in the as attribute when eager loading Y).
* @param {Model} [optinos.include[].model] The model you want to eagerly load
* @param {String} [options.include[].as] The alias of the relation, in case the model you want to eagerly load is aliassed.
Expand Down
7 changes: 2 additions & 5 deletions lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ var Utils = module.exports = {
_where[i].in = _where[i]. in || [];
_where[i]. in .concat(where[i]);
}
else if (Utils.isHash(where[i])) {
else if (Utils._.isPlainObject(where[i])) {
Object.keys(where[i]).forEach(function(ii) {
logic = self.getWhereLogic(ii, where[i][ii]);

Expand Down Expand Up @@ -319,9 +319,6 @@ var Utils = module.exports = {
return '';
}
},
isHash: function(obj) {
return Utils._.isObject(obj) && !Array.isArray(obj) && !Buffer.isBuffer(obj);
},
hasChanged: function(attrValue, value) {
//If attribute value is Date, check value as a date
if (Utils._.isDate(attrValue) && !Utils._.isDate(value)) {
Expand Down Expand Up @@ -412,7 +409,7 @@ var Utils = module.exports = {

setAttributes: function(hash, identifier, instance, prefix) {
prefix = prefix || '';
if (this.isHash(identifier)) {
if (this._.isPlainObject(identifier)) {
this._.each(identifier, function(elem, key) {
hash[prefix + key] = Utils._.isString(instance) ? instance : Utils._.isObject(instance) ? instance[elem.key || elem] : null;
});
Expand Down
35 changes: 35 additions & 0 deletions test/include.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,41 @@ describe(Support.getTestDialectTeaser("Include"), function () {
})
})

it('should support Sequelize.literal and renaming of attributes in included model attributes', function () {
var Post = this.sequelize.define('Post',{});
var PostComment = this.sequelize.define('PostComment', {
someProperty: Sequelize.VIRTUAL, // Since we specify the AS part as a part of the literal string, not with sequelize syntax, we have to tell sequelize about the field
comment_title: Sequelize.STRING
});

Post.hasMany(PostComment);

return this.sequelize.sync({ force: true }).then(function () {
return Post.create({});
}).then(function (post) {
return post.createPostComment({
comment_title: 'WAT'
});
}).then(function () {
return Post.findAll({
include: [
{
model: PostComment,
attributes: [
Sequelize.literal('EXISTS(SELECT 1) AS "PostComments.someProperty"'),
[Sequelize.literal('EXISTS(SELECT 1)'), 'someProperty2'],
['comment_title', 'commentTitle']
]
}
]
})
}).then(function (posts) {
expect(posts[0].postComments[0].get('someProperty')).to.be.ok;
expect(posts[0].postComments[0].get('someProperty2')).to.be.ok;
expect(posts[0].postComments[0].get('commentTitle')).to.equal('WAT');
});
});

it('should support self associated hasMany (with through) include', function (done) {
var Group = this.sequelize.define('Group', {
name: DataTypes.STRING
Expand Down
20 changes: 20 additions & 0 deletions test/model/attributes.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,26 @@ describe(Support.getTestDialectTeaser("Model"), function () {
});
});
});

it('should support renaming of sequelize method fields', function () {
var User = this.sequelize.define('user', {
someProperty: Sequelize.VIRTUAL // Since we specify the AS part as a part of the literal string, not with sequelize syntax, we have to tell sequelize about the field
});

return this.sequelize.sync({ force: true }).then(function () {
return User.create({});
}).then(function () {
return User.findAll({
attributes: [
Sequelize.literal('EXISTS(SELECT 1) AS "someProperty"'),
[Sequelize.literal('EXISTS(SELECT 1)'), 'someProperty2']
]
});
}).then(function (users) {
expect(users[0].get('someProperty')).to.be.ok;
expect(users[0].get('someProperty2')).to.be.ok;
});
});

it('field names that are the same as property names should create, update, and read correctly', function () {
var self = this;
Expand Down
6 changes: 3 additions & 3 deletions test/mysql/query-generator.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ if (Support.dialectIsMySQL()) {
context: QueryGenerator
}, {
arguments: ['foo', { attributes: [['count(*)', 'count']] }],
expectation: 'SELECT count(*) as `count` FROM `foo`;',
expectation: 'SELECT count(*) AS `count` FROM `foo`;',
context: QueryGenerator
}, {
arguments: ['myTable', {where: "foo='bar'"}],
Expand Down Expand Up @@ -246,7 +246,7 @@ if (Support.dialectIsMySQL()) {
having: ['creationYear > ?', 2002]
}
}],
expectation: "SELECT *, YEAR(`createdAt`) as `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING creationYear > 2002;",
expectation: "SELECT *, YEAR(`createdAt`) AS `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING creationYear > 2002;",
context: QueryGenerator,
needsSequelize: true
}, {
Expand All @@ -258,7 +258,7 @@ if (Support.dialectIsMySQL()) {
having: { creationYear: { gt: 2002 } }
}
}],
expectation: "SELECT *, YEAR(`createdAt`) as `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING `creationYear` > 2002;",
expectation: "SELECT *, YEAR(`createdAt`) AS `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING `creationYear` > 2002;",
context: QueryGenerator,
needsSequelize: true
}, {
Expand Down
8 changes: 4 additions & 4 deletions test/postgres/query-generator.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ if (dialect.match(/^postgres/)) {
expectation: "SELECT * FROM \"myTable\" WHERE \"myTable\".\"id\"=2;"
}, {
arguments: ['foo', { attributes: [['count(*)', 'count']] }],
expectation: 'SELECT count(*) as \"count\" FROM \"foo\";'
expectation: 'SELECT count(*) AS \"count\" FROM \"foo\";'
}, {
arguments: ['myTable', {where: "foo='bar'"}],
expectation: "SELECT * FROM \"myTable\" WHERE foo='bar';"
Expand Down Expand Up @@ -330,7 +330,7 @@ if (dialect.match(/^postgres/)) {
having: ['creationYear > ?', 2002]
}
}],
expectation: "SELECT *, YEAR(\"createdAt\") as \"creationYear\" FROM \"myTable\" GROUP BY \"creationYear\", \"title\" HAVING creationYear > 2002;",
expectation: "SELECT *, YEAR(\"createdAt\") AS \"creationYear\" FROM \"myTable\" GROUP BY \"creationYear\", \"title\" HAVING creationYear > 2002;",
context: QueryGenerator,
needsSequelize: true
}, {
Expand All @@ -342,7 +342,7 @@ if (dialect.match(/^postgres/)) {
having: { creationYear: { gt: 2002 } }
}
}],
expectation: "SELECT *, YEAR(\"createdAt\") as \"creationYear\" FROM \"myTable\" GROUP BY \"creationYear\", \"title\" HAVING \"creationYear\" > 2002;",
expectation: "SELECT *, YEAR(\"createdAt\") AS \"creationYear\" FROM \"myTable\" GROUP BY \"creationYear\", \"title\" HAVING \"creationYear\" > 2002;",
context: QueryGenerator,
needsSequelize: true
}, {
Expand Down Expand Up @@ -395,7 +395,7 @@ if (dialect.match(/^postgres/)) {
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['foo', { attributes: [['count(*)', 'count']] }],
expectation: 'SELECT count(*) as count FROM foo;',
expectation: 'SELECT count(*) AS count FROM foo;',
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['myTable', {where: "foo='bar'"}],
Expand Down
6 changes: 3 additions & 3 deletions test/sqlite/query-generator.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ if (dialect === 'sqlite') {
context: QueryGenerator
}, {
arguments: ['foo', { attributes: [['count(*)', 'count']] }],
expectation: 'SELECT count(*) as `count` FROM `foo`;',
expectation: 'SELECT count(*) AS `count` FROM `foo`;',
context: QueryGenerator
}, {
arguments: ['myTable', {where: "foo='bar'"}],
Expand Down Expand Up @@ -231,7 +231,7 @@ if (dialect === 'sqlite') {
having: ['creationYear > ?', 2002]
}
}],
expectation: "SELECT *, YEAR(`createdAt`) as `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING creationYear > 2002;",
expectation: "SELECT *, YEAR(`createdAt`) AS `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING creationYear > 2002;",
context: QueryGenerator,
needsSequelize: true
}, {
Expand All @@ -243,7 +243,7 @@ if (dialect === 'sqlite') {
having: { creationYear: { gt: 2002 } }
}
}],
expectation: "SELECT *, YEAR(`createdAt`) as `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING `creationYear` > 2002;",
expectation: "SELECT *, YEAR(`createdAt`) AS `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING `creationYear` > 2002;",
context: QueryGenerator,
needsSequelize: true
}, {
Expand Down
Loading

0 comments on commit 6541fb4

Please sign in to comment.