Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: optimize a few code paths #10570

Merged
merged 2 commits into from Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions lib/deferrable.js
Expand Up @@ -3,13 +3,12 @@
const { classToInvokable } = require('./utils');

class ABSTRACT {
static toString() {
const instance = new this();
return instance.toString.apply(instance, arguments);
static toString(...args) {
return new this().toString(...args);
}

toString() {
return this.toSql.apply(this, arguments);
toString(...args) {
return this.toSql(...args);
}

toSql() {
Expand Down
4 changes: 2 additions & 2 deletions lib/dialects/postgres/connection-manager.js
Expand Up @@ -77,10 +77,10 @@ class ConnectionManager extends AbstractConnectionManager {
this.oidParserMap = new Map();
}

getTypeParser(oid) {
getTypeParser(oid, ...args) {
if (this.oidParserMap.get(oid)) return this.oidParserMap.get(oid);

return this.lib.types.getTypeParser.apply(undefined, arguments);
return this.lib.types.getTypeParser(oid, ...args);
}

connect(config) {
Expand Down
32 changes: 16 additions & 16 deletions lib/model.js
Expand Up @@ -19,6 +19,14 @@ const Hooks = require('./hooks');
const associationsMixin = require('./associations/mixin');
const Op = require('./operators');

// This list will quickly become dated, but failing to maintain this list just means
// we won't throw a warning when we should. At least most common cases will forever be covered
// so we stop throwing erroneous warnings when we shouldn't.
const validQueryKeywords = new Set(['where', 'attributes', 'paranoid', 'include', 'order', 'limit', 'offset',
'transaction', 'lock', 'raw', 'logging', 'benchmark', 'having', 'searchPath', 'rejectOnEmpty', 'plain',
'scope', 'group', 'through', 'defaults', 'distinct', 'primary', 'exception', 'type', 'hooks', 'force',
'name']);

/**
* A Model represents a table in the database. Instances of this class represent a database row.
*
Expand Down Expand Up @@ -1763,15 +1771,7 @@ class Model {
return;
}

// This list will quickly become dated, but failing to maintain this list just means
// we won't throw a warning when we should. At least most common cases will forever be covered
// so we stop throwing erroneous warnings when we shouldn't.
const validQueryKeywords = ['where', 'attributes', 'paranoid', 'include', 'order', 'limit', 'offset',
'transaction', 'lock', 'raw', 'logging', 'benchmark', 'having', 'searchPath', 'rejectOnEmpty', 'plain',
'scope', 'group', 'through', 'defaults', 'distinct', 'primary', 'exception', 'type', 'hooks', 'force',
'name'];

const unrecognizedOptions = _.difference(Object.keys(options), validQueryKeywords);
const unrecognizedOptions = Object.keys(options).filter(k => !validQueryKeywords.has(k));
const unexpectedModelAttributes = _.intersection(unrecognizedOptions, validColumnNames);
if (!options.where && unexpectedModelAttributes.length > 0) {
logger.warn(`Model attributes (${unexpectedModelAttributes.join(', ')}) passed into finder method options of model ${this.name}, but the options.where object is empty. Did you forget to use options.where?`);
Expand Down Expand Up @@ -2320,13 +2320,13 @@ class Model {
.filter(name => this.rawAttributes[name])
.map(name => this.rawAttributes[name].field || name);

if (defaultFields) {
if (!_.intersection(Object.keys(err.fields), whereFields).length && _.intersection(Object.keys(err.fields), defaultFields).length) {
throw err;
}
const errFieldKeys = Object.keys(err.fields);
const errFieldsWhereIntersects = Utils.intersects(errFieldKeys, whereFields);
if (defaultFields && !errFieldsWhereIntersects && Utils.intersects(errFieldKeys, defaultFields)) {
throw err;
}

if (_.intersection(Object.keys(err.fields), whereFields).length) {
if (errFieldsWhereIntersects) {
_.each(err.fields, (value, key) => {
const name = this.fieldRawAttributesMap[key].fieldName;
if (value.toString() !== options.where[name].toString()) {
Expand Down Expand Up @@ -3795,7 +3795,7 @@ class Model {
args = [this, this.constructor.getTableName(options), values, where, options];
}

return this.constructor.QueryInterface[query].apply(this.constructor.QueryInterface, args)
return this.constructor.QueryInterface[query](...args)
.then(([result, rowsUpdated])=> {
if (versionAttr) {
// Check to see that a row was updated, otherwise it's an optimistic locking error.
Expand Down Expand Up @@ -3978,7 +3978,7 @@ class Model {
this.set(values, setOptions);

// Now we need to figure out which fields were actually affected by the setter.
const sideEffects = _.without.apply(this, [this.changed() || []].concat(changedBefore));
const sideEffects = _.without(this.changed(), ...changedBefore);
const fields = _.union(Object.keys(values), sideEffects);

if (!options.fields) {
Expand Down
2 changes: 1 addition & 1 deletion lib/sequelize.js
Expand Up @@ -1115,7 +1115,7 @@ class Sequelize {
args = [`${args[0]} Elapsed time: ${args[1]}ms`];
}

options.logging.apply(null, args);
options.logging(...args);
}
}

Expand Down
17 changes: 14 additions & 3 deletions lib/utils.js
Expand Up @@ -7,7 +7,7 @@ const uuidv1 = require('uuid/v1');
const uuidv4 = require('uuid/v4');
const Promise = require('./promise');
const operators = require('./operators');
const operatorsArray = _.values(operators);
const operatorsSet = new Set(_.values(operators));

let inflection = require('inflection');

Expand Down Expand Up @@ -492,7 +492,7 @@ exports.Where = Where;
* @private
*/
function getOperators(obj) {
return _.intersection(Object.getOwnPropertySymbols(obj || {}), operatorsArray);
return Object.getOwnPropertySymbols(obj).filter(s => operatorsSet.has(s));
}
exports.getOperators = getOperators;

Expand Down Expand Up @@ -528,7 +528,7 @@ exports.getComplexSize = getComplexSize;
* @private
*/
function isWhereEmpty(obj) {
return _.isEmpty(obj) && getOperators(obj).length === 0;
return !!obj && _.isEmpty(obj) && getOperators(obj).length === 0;
}
exports.isWhereEmpty = isWhereEmpty;

Expand Down Expand Up @@ -622,3 +622,14 @@ function nameIndex(index, tableName) {
return index;
}
exports.nameIndex = nameIndex;

/**
* Checks if 2 arrays intersect.
*
* @param {Array} arr1
* @param {Array} arr2
*/
SimonSchick marked this conversation as resolved.
Show resolved Hide resolved
function intersects(arr1, arr2) {
return arr1.some(v => arr2.includes(v));
}
exports.intersects = intersects;
2 changes: 1 addition & 1 deletion test/unit/dialects/mariadb/query-generator.test.js
Expand Up @@ -788,7 +788,7 @@ if (dialect === 'mariadb') {
// Options would normally be set by the query interface that instantiates the query-generator, but here we specify it explicitly
this.queryGenerator.options = Object.assign({}, this.queryGenerator.options, test.context && test.context.options || {});

const conditions = this.queryGenerator[suiteTitle].apply(this.queryGenerator, test.arguments);
const conditions = this.queryGenerator[suiteTitle](...test.arguments);
expect(conditions).to.deep.equal(test.expectation);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/dialects/mssql/query-generator.test.js
Expand Up @@ -295,7 +295,7 @@ if (current.dialect.name === 'mssql') {
}
].forEach(test => {
it(test.title, function() {
expectsql(this.queryGenerator.arithmeticQuery.apply(this.queryGenerator, test.arguments), {
expectsql(this.queryGenerator.arithmeticQuery(...test.arguments), {
mssql: test.expectation
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/dialects/mysql/query-generator.test.js
Expand Up @@ -738,7 +738,7 @@ if (dialect === 'mysql') {
// Options would normally be set by the query interface that instantiates the query-generator, but here we specify it explicitly
this.queryGenerator.options = Object.assign({}, this.queryGenerator.options, test.context && test.context.options || {});

const conditions = this.queryGenerator[suiteTitle].apply(this.queryGenerator, test.arguments);
const conditions = this.queryGenerator[suiteTitle](...test.arguments);
expect(conditions).to.deep.equal(test.expectation);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/dialects/postgres/query-generator.test.js
Expand Up @@ -1231,7 +1231,7 @@ if (dialect.startsWith('postgres')) {
// Options would normally be set by the query interface that instantiates the query-generator, but here we specify it explicitly
this.queryGenerator.options = Object.assign({}, this.queryGenerator.options, test.context && test.context.options || {});

const conditions = this.queryGenerator[suiteTitle].apply(this.queryGenerator, test.arguments);
const conditions = this.queryGenerator[suiteTitle](...test.arguments);
expect(conditions).to.deep.equal(test.expectation);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/dialects/sqlite/query-generator.test.js
Expand Up @@ -644,7 +644,7 @@ if (dialect === 'sqlite') {
// Options would normally be set by the query interface that instantiates the query-generator, but here we specify it explicitly
this.queryGenerator.options = Object.assign({}, this.queryGenerator.options, test.context && test.context.options || {});

const conditions = this.queryGenerator[suiteTitle].apply(this.queryGenerator, test.arguments);
const conditions = this.queryGenerator[suiteTitle](...test.arguments);
expect(conditions).to.deep.equal(test.expectation);
});
});
Expand Down