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

Commit 88a340da by u9r52sld Committed by Sushant

refactor(scopes): just in time options conforming (#9735)

1 parent fc644f4b
...@@ -757,6 +757,54 @@ class Model { ...@@ -757,6 +757,54 @@ class Model {
return index; return index;
} }
static _uniqIncludes(options) {
if (!options.include) return;
options.include = _(options.include)
.groupBy(include => `${include.model && include.model.name}-${include.as}`)
.map(includes => this._assignOptions(...includes))
.value();
}
static _baseMerge(...args) {
_.assignWith(...args);
this._conformOptions(args[0]);
this._uniqIncludes(args[0]);
return args[0];
}
static _mergeFunction(objValue, srcValue, key) {
if (Array.isArray(objValue) && Array.isArray(srcValue)) {
return _.union(objValue, srcValue);
} else if (key === 'where') {
if (srcValue instanceof Utils.SequelizeMethod) {
srcValue = { [Op.and]: srcValue };
}
if (_.isPlainObject(objValue) && _.isPlainObject(srcValue)) {
return Object.assign(objValue, srcValue);
}
} else if (key === 'attributes' && _.isPlainObject(objValue) && _.isPlainObject(srcValue)) {
return _.assignWith(objValue, srcValue, (objValue, srcValue) => {
if (Array.isArray(objValue) && Array.isArray(srcValue)) {
return _.union(objValue, srcValue);
}
});
}
return srcValue === undefined ? objValue : srcValue;
}
static _assignOptions(...args) {
return this._baseMerge(...args, this._mergeFunction);
}
static _defaultsOptions(...args) {
return this._baseMerge(...args, (...cbArgs) => {
[cbArgs[0], cbArgs[1]] = [cbArgs[1], cbArgs[0]];
return this._mergeFunction(...cbArgs);
});
}
/** /**
* Initialize a model, representing a table in the DB, with attributes and options. * Initialize a model, representing a table in the DB, with attributes and options.
* *
...@@ -980,16 +1028,6 @@ class Model { ...@@ -980,16 +1028,6 @@ class Model {
this._scope = this.options.defaultScope; this._scope = this.options.defaultScope;
this._scopeNames = ['defaultScope']; this._scopeNames = ['defaultScope'];
if (_.isPlainObject(this._scope)) {
this._conformOptions(this._scope, this);
}
_.each(this.options.scopes, scope => {
if (_.isPlainObject(scope)) {
this._conformOptions(scope, this);
}
});
this.sequelize.modelManager.addModel(this); this.sequelize.modelManager.addModel(this);
this.sequelize.runHooks('afterDefine', this); this.sequelize.runHooks('afterDefine', this);
...@@ -1428,8 +1466,6 @@ class Model { ...@@ -1428,8 +1466,6 @@ class Model {
throw new Error(`The scope ${name} already exists. Pass { override: true } as options to silence this error`); throw new Error(`The scope ${name} already exists. Pass { override: true } as options to silence this error`);
} }
this._conformOptions(scope, this);
if (name === 'defaultScope') { if (name === 'defaultScope') {
this.options.defaultScope = this._scope = scope; this.options.defaultScope = this._scope = scope;
} else { } else {
...@@ -1525,29 +1561,12 @@ class Model { ...@@ -1525,29 +1561,12 @@ class Model {
if (_.isFunction(scope)) { if (_.isFunction(scope)) {
scope = scope(); scope = scope();
this._conformOptions(scope, self);
} }
} }
} }
if (scope) { if (scope) {
_.assignWith(self._scope, scope, (objectValue, sourceValue, key) => { this._assignOptions(self._scope, scope);
if (key === 'where') {
if (Array.isArray(sourceValue)) {
return sourceValue;
}
if (sourceValue instanceof Utils.SequelizeMethod) {
sourceValue = { [Op.and]: sourceValue };
}
return Object.assign(objectValue || {}, sourceValue);
}
if (['attributes', 'include', 'group'].includes(key) && Array.isArray(objectValue) && Array.isArray(sourceValue)) {
return objectValue.concat(sourceValue);
}
return objectValue ? objectValue : sourceValue;
});
self._scopeNames.push(scopeName ? scopeName : 'defaultScope'); self._scopeNames.push(scopeName ? scopeName : 'defaultScope');
} else { } else {
throw new sequelizeErrors.SequelizeScopeError(`Invalid scope ${scopeName} called.`); throw new sequelizeErrors.SequelizeScopeError(`Invalid scope ${scopeName} called.`);
...@@ -1925,7 +1944,6 @@ class Model { ...@@ -1925,7 +1944,6 @@ class Model {
*/ */
static aggregate(attribute, aggregateFunction, options) { static aggregate(attribute, aggregateFunction, options) {
options = Utils.cloneDeep(options); options = Utils.cloneDeep(options);
options = _.defaults(options, { attributes: [] });
this._injectScope(options); this._injectScope(options);
this._conformOptions(options, this); this._conformOptions(options, this);
...@@ -1943,6 +1961,7 @@ class Model { ...@@ -1943,6 +1961,7 @@ class Model {
aggregateColumn = this.sequelize.fn('DISTINCT', aggregateColumn); aggregateColumn = this.sequelize.fn('DISTINCT', aggregateColumn);
} }
options.attributes = _.intersection(options.attributes, options.group);
options.attributes.push([this.sequelize.fn(aggregateFunction, aggregateColumn), aggregateFunction]); options.attributes.push([this.sequelize.fn(aggregateFunction, aggregateColumn), aggregateFunction]);
if (!options.dataType) { if (!options.dataType) {
...@@ -3058,36 +3077,10 @@ class Model { ...@@ -3058,36 +3077,10 @@ class Model {
} }
} }
// Inject _scope into options. Includes should have been conformed (conformOptions) before calling this // Inject _scope into options.
static _injectScope(options) { static _injectScope(options) {
const scope = Utils.cloneDeep(this._scope); const scope = Utils.cloneDeep(this._scope);
this._defaultsOptions(options, scope);
const filteredScope = _.omit(scope, 'include'); // Includes need special treatment
Utils.defaults(options, filteredScope);
Utils.defaults(options.where, filteredScope.where);
if (scope.include) {
options.include = options.include || [];
// Reverse so we consider the latest include first.
// This is used if several scopes specify the same include - the last scope should take precedence
for (const scopeInclude of scope.include.reverse()) {
if (scopeInclude.all || !options.include.some(item => {
const isSameModel = item.model && item.model.name === scopeInclude.model.name;
if (!isSameModel || !item.as) return isSameModel;
if (scopeInclude.as) {
return item.as === scopeInclude.as;
} else {
const association = scopeInclude.association || this.getAssociationForAlias(scopeInclude.model, scopeInclude.as);
return association ? item.as === association.as : false;
}
})) {
options.include.push(scopeInclude);
}
}
}
} }
static inspect() { static inspect() {
......
...@@ -302,6 +302,122 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -302,6 +302,122 @@ describe(Support.getTestDialectTeaser('Model'), () => {
expect(user.company.projects).to.have.length(1); expect(user.company.projects).to.have.length(1);
}); });
}); });
describe('with different format', () => {
it('should not throw error', function() {
const Child = this.sequelize.define('Child');
const Parent = this.sequelize.define('Parent', {}, {
defaultScope: {
include: [{ model: Child }]
},
scopes: {
children: {
include: [Child]
}
}
});
Parent.addScope('alsoChildren', {
include: [{ model: Child }]
});
Child.belongsTo(Parent);
Parent.hasOne(Child);
return this.sequelize.sync({ force: true }).then(() => {
return Promise.all([Child.create(), Parent.create()]);
}).then(([child, parent]) => {
return parent.setChild(child);
}).then(() => {
return Parent.scope('children', 'alsoChildren').findOne();
});
});
});
describe('with find options', () => {
it('should merge includes correctly', function() {
const Child = this.sequelize.define('Child', { name: Sequelize.STRING });
const Parent = this.sequelize.define('Parent', { name: Sequelize.STRING });
Parent.addScope('testScope1', {
include: [{
model: Child,
where: {
name: 'child2'
}
}]
});
Parent.hasMany(Child);
return this.sequelize.sync({ force: true })
.then(() => {
return Promise.all([
Parent.create({ name: 'parent1' }).then(parent => parent.createChild({ name: 'child1' })),
Parent.create({ name: 'parent2' }).then(parent => parent.createChild({ name: 'child2' }))
]);
})
.then(() => {
return Parent.scope('testScope1').findOne({
include: [{
model: Child,
attributes: { exclude: ['name'] }
}]
});
})
.then(parent => {
expect(parent.get('name')).to.equal('parent2');
expect(parent.Children).to.have.length(1);
expect(parent.Children[0].dataValues).not.to.have.property('name');
});
});
});
});
describe('scope with options', () => {
it('should return correct object included foreign_key', function() {
const Child = this.sequelize.define('Child', {
secret: Sequelize.STRING
}, {
scopes: {
public: {
attributes: {
exclude: ['secret']
}
}
}
});
const Parent = this.sequelize.define('Parent');
Child.belongsTo(Parent);
Parent.hasOne(Child);
return this.sequelize.sync({ force: true })
.then(() => Child.create({ secret: 'super secret' }))
.then(() => Child.scope('public').findOne())
.then(user => {
expect(user.dataValues).to.have.property('ParentId');
expect(user.dataValues).not.to.have.property('secret');
});
});
it('should return correct object included foreign_key with defaultScope', function() {
const Child = this.sequelize.define('Child', {
secret: Sequelize.STRING
}, {
defaultScope: {
attributes: {
exclude: ['secret']
}
}
});
const Parent = this.sequelize.define('Parent');
Child.belongsTo(Parent);
return this.sequelize.sync({ force: true })
.then(() => Child.create({ secret: 'super secret' }))
.then(() => Child.findOne())
.then(user => {
expect(user.dataValues).to.have.property('ParentId');
expect(user.dataValues).not.to.have.property('secret');
});
});
}); });
}); });
}); });
......
...@@ -84,6 +84,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -84,6 +84,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
describe('attribute exclude / include', () => { describe('attribute exclude / include', () => {
const User = current.define('user', { const User = current.define('user', {
password: DataTypes.STRING, password: DataTypes.STRING,
value: DataTypes.INTEGER,
name: DataTypes.STRING name: DataTypes.STRING
}, { }, {
defaultScope: { defaultScope: {
...@@ -94,28 +95,22 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -94,28 +95,22 @@ describe(Support.getTestDialectTeaser('Model'), () => {
scopes: { scopes: {
aScope: { aScope: {
attributes: { attributes: {
exclude: ['password'] exclude: ['value']
} }
} }
} }
}); });
it('should be able to exclude in defaultScope #4735', () => { it('should not expand attributes', () => {
expect(User._scope.attributes).to.deep.equal([ expect(User._scope.attributes).to.deep.equal({ exclude: ['password'] });
'id', });
'name',
'createdAt', it('should not expand attributes', () => {
'updatedAt' expect(User.scope('aScope')._scope.attributes).to.deep.equal({ exclude: ['value'] });
]);
}); });
it('should be able to exclude in a scope #4925', () => { it('should unite attributes with array', () => {
expect(User.scope('aScope')._scope.attributes).to.deep.equal([ expect(User.scope('aScope', 'defaultScope')._scope.attributes).to.deep.equal({ exclude: ['value', 'password'] });
'id',
'name',
'createdAt',
'updatedAt'
]);
}); });
}); });
...@@ -127,7 +122,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -127,7 +122,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
it('should apply default scope', () => { it('should apply default scope', () => {
expect(Company._scope).to.deep.equal({ expect(Company._scope).to.deep.equal({
include: [{ model: Project }], include: [Project],
where: { active: true } where: { active: true }
}); });
}); });
...@@ -267,7 +262,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -267,7 +262,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
where: { where: {
this: 'that' this: 'that'
}, },
include: [Project] include: [{ model: Project }]
}); });
expect(Company.scope('newScope')._scope).to.deep.equal({ expect(Company.scope('newScope')._scope).to.deep.equal({
...@@ -310,7 +305,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -310,7 +305,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
it('allows me to override a default scope', () => { it('allows me to override a default scope', () => {
Company.addScope('defaultScope', { Company.addScope('defaultScope', {
include: [Project] include: [{ model: Project }]
}, { override: true }); }, { override: true });
expect(Company._scope).to.deep.equal({ expect(Company._scope).to.deep.equal({
...@@ -318,7 +313,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -318,7 +313,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
}); });
}); });
it('works with exclude and include attributes', () => { it('should keep exclude and include attributes', () => {
Company.addScope('newIncludeScope', { Company.addScope('newIncludeScope', {
attributes: { attributes: {
include: ['foobar'], include: ['foobar'],
...@@ -327,15 +322,29 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -327,15 +322,29 @@ describe(Support.getTestDialectTeaser('Model'), () => {
}); });
expect(Company.scope('newIncludeScope')._scope).to.deep.equal({ expect(Company.scope('newIncludeScope')._scope).to.deep.equal({
attributes: ['id', 'updatedAt', 'foobar'] attributes: {
include: ['foobar'],
exclude: ['createdAt']
}
}); });
}); });
it('should be able to merge scopes with the same include', () => {
Company.addScope('project', {
include: [{ model: Project, where: { something: false, somethingElse: 99 }}]
});
Company.addScope('alsoProject', {
include: [{ model: Project, where: { something: true }, limit: 1 }]
});
expect(Company.scope(['project', 'alsoProject'])._scope).to.deep.equal({
include: [{ model: Project, where: { something: true, somethingElse: 99 }, limit: 1 }]
});
});
}); });
describe('_injectScope', () => { describe('_injectScope', () => {
it('should be able to merge scope and where', () => { it('should be able to merge scope and where', () => {
const scope = { Sequelize.Model._scope = {
where: { where: {
something: true, something: true,
somethingElse: 42 somethingElse: 42
...@@ -351,9 +360,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -351,9 +360,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
limit: 9 limit: 9
}; };
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options).to.deep.equal({ expect(options).to.deep.equal({
where: { where: {
...@@ -365,43 +372,39 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -365,43 +372,39 @@ describe(Support.getTestDialectTeaser('Model'), () => {
}); });
}); });
it('should be able to overwrite multiple scopes with the same include', () => { it('should be able to merge scopes with the same include', () => {
const scope = { Sequelize.Model._scope = {
include: [ include: [
{ model: Project, where: { something: false }}, { model: Project, where: { something: false, somethingElse: 99 }},
{ model: Project, where: { something: true }} { model: Project, where: { something: true }, limit: 1 }
] ]
}; };
const options = {}; const options = {};
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options.include).to.have.length(1); expect(options.include).to.have.length(1);
expect(options.include[0]).to.deep.equal({ model: Project, where: { something: true }}); expect(options.include[0]).to.deep.equal({ model: Project, where: { something: true, somethingElse: 99 }, limit: 1 });
}); });
it('should be able to override scoped include', () => { it('should be able to merge scoped include', () => {
const scope = { Sequelize.Model._scope = {
include: [{ model: Project, where: { something: false }}] include: [{ model: Project, where: { something: false, somethingElse: 99 }}]
}; };
const options = { const options = {
include: [{ model: Project, where: { something: true }}] include: [{ model: Project, where: { something: true }, limit: 1 }]
}; };
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options.include).to.have.length(1); expect(options.include).to.have.length(1);
expect(options.include[0]).to.deep.equal({ model: Project, where: { something: true }}); expect(options.include[0]).to.deep.equal({ model: Project, where: { something: true, somethingElse: 99 }, limit: 1 });
}); });
it('should be able to merge aliased includes with the same model', () => { it('should be able to merge aliased includes with the same model', () => {
const scope = { Sequelize.Model._scope = {
include: [{model: User, as: 'someUser'}] include: [{model: User, as: 'someUser'}]
}; };
...@@ -409,17 +412,15 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -409,17 +412,15 @@ describe(Support.getTestDialectTeaser('Model'), () => {
include: [{model: User, as: 'otherUser'}] include: [{model: User, as: 'otherUser'}]
}; };
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options.include).to.have.length(2); expect(options.include).to.have.length(2);
expect(options.include[0]).to.deep.equal({model: User, as: 'otherUser'}); expect(options.include[0]).to.deep.equal({model: User, as: 'someUser'});
expect(options.include[1]).to.deep.equal({model: User, as: 'someUser'}); expect(options.include[1]).to.deep.equal({model: User, as: 'otherUser'});
}); });
it('should be able to merge scoped include with include in find', () => { it('should be able to merge scoped include with include in find', () => {
const scope = { Sequelize.Model._scope = {
include: [ include: [
{ model: Project, where: { something: false }} { model: Project, where: { something: false }}
] ]
...@@ -431,18 +432,16 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -431,18 +432,16 @@ describe(Support.getTestDialectTeaser('Model'), () => {
] ]
}; };
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options.include).to.have.length(2); expect(options.include).to.have.length(2);
expect(options.include[0]).to.deep.equal({ model: User, where: { something: true }}); expect(options.include[0]).to.deep.equal({ model: Project, where: { something: false }});
expect(options.include[1]).to.deep.equal({ model: Project, where: { something: false }}); expect(options.include[1]).to.deep.equal({ model: User, where: { something: true }});
}); });
describe('include all', () => { describe('include all', () => {
it('scope with all', () => { it('scope with all', () => {
const scope = { Sequelize.Model._scope = {
include: [ include: [
{ all: true } { all: true }
] ]
...@@ -454,18 +453,16 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -454,18 +453,16 @@ describe(Support.getTestDialectTeaser('Model'), () => {
] ]
}; };
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options.include).to.have.length(2); expect(options.include).to.have.length(2);
expect(options.include[0]).to.deep.equal({ model: User, where: { something: true }}); expect(options.include[0]).to.deep.equal({ all: true });
expect(options.include[1]).to.deep.equal({ all: true }); expect(options.include[1]).to.deep.equal({ model: User, where: { something: true }});
}); });
it('options with all', () => { it('options with all', () => {
const scope = { Sequelize.Model._scope = {
include: [ include: [
{ model: User, where: { something: true }} { model: User, where: { something: true }}
] ]
...@@ -477,13 +474,11 @@ describe(Support.getTestDialectTeaser('Model'), () => { ...@@ -477,13 +474,11 @@ describe(Support.getTestDialectTeaser('Model'), () => {
] ]
}; };
Sequelize.Model._injectScope.call({ Sequelize.Model._injectScope(options);
_scope: scope
}, options);
expect(options.include).to.have.length(2); expect(options.include).to.have.length(2);
expect(options.include[0]).to.deep.equal({ all: true }); expect(options.include[0]).to.deep.equal({ model: User, where: { something: true }});
expect(options.include[1]).to.deep.equal({ model: User, where: { something: true }}); expect(options.include[1]).to.deep.equal({ all: true });
}); });
}); });
}); });
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!