From 6892543a396421ea9b10cfe28301834edd84efff Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Mon, 7 Mar 2016 22:48:55 +0530 Subject: [PATCH] refactor(relations): refactor relations code --- src/Lucid/Model/Mixins/Dates.js | 10 ++- src/Lucid/Model/index.js | 2 +- src/Lucid/QueryBuilder/methods.js | 11 +++ src/Lucid/Relations/BelongsTo.js | 9 ++- src/Lucid/Relations/BelongsToMany.js | 102 ++++++++++++++++++++++--- src/Lucid/Relations/Relation.js | 28 ++++--- test/unit/lucid.relations.spec.js | 108 ++++++++++++++++++++++++++- 7 files changed, 243 insertions(+), 27 deletions(-) diff --git a/src/Lucid/Model/Mixins/Dates.js b/src/Lucid/Model/Mixins/Dates.js index eee962db..dd4d3ad2 100644 --- a/src/Lucid/Model/Mixins/Dates.js +++ b/src/Lucid/Model/Mixins/Dates.js @@ -16,9 +16,13 @@ const moment = require('moment') * sets create timestamp on an object of values only * if create timestamps are enabled by a given * model. + * * @method setCreateTimestamp - * @param {Obhect} values + * + * @param {Object} values * @return {Object} + * + * @public */ Dates.setCreateTimestamp = function (values) { if (this.constructor.createTimestamp) { @@ -34,7 +38,7 @@ Dates.setCreateTimestamp = function (values) { * * @method setUpdateTimestamp * - * @param {Obhect} values + * @param {Object} values * @return {Object} * * @public @@ -53,7 +57,7 @@ Dates.setUpdateTimestamp = function (values) { * * @method setDeleteTimestamp * - * @param {Obhect} values + * @param {Object} values * @return {Object} * * @public diff --git a/src/Lucid/Model/index.js b/src/Lucid/Model/index.js index 09604b41..ae339b28 100644 --- a/src/Lucid/Model/index.js +++ b/src/Lucid/Model/index.js @@ -530,7 +530,7 @@ class Model { */ get $dirty () { return _.pickBy(this.attributes, (value, key) => { - return typeof (this.original[key]) === 'undefined' || this.original[key] !== value + return (typeof (this.original[key]) === 'undefined' || this.original[key] !== value) && !key.startsWith('_pivot_') }) } diff --git a/src/Lucid/QueryBuilder/methods.js b/src/Lucid/QueryBuilder/methods.js index 1473626e..05a6a64d 100644 --- a/src/Lucid/QueryBuilder/methods.js +++ b/src/Lucid/QueryBuilder/methods.js @@ -32,6 +32,12 @@ methods.fetch = function (target) { */ const globalScope = target.HostModel.globalScope let eagerlyFetched = [] + + /** + * call all global scopes before executing the query + * chain. This is the last time someone can modify + * the existing query chain + */ if (_.size(globalScope)) { _.each(globalScope, (scopeMethod) => { scopeMethod(this) @@ -39,6 +45,11 @@ methods.fetch = function (target) { } let results = yield target.modelQueryBuilder + + /** + * eagerly fetch all relations which are set for eagerLoad and + * also the previous query execution returned some results. + */ if (_.size(target.eagerLoad.withRelations) && _.size(results)) { eagerlyFetched = yield target.eagerLoad.load(results, target.HostModel) } diff --git a/src/Lucid/Relations/BelongsTo.js b/src/Lucid/Relations/BelongsTo.js index dd5a1f82..9535a513 100644 --- a/src/Lucid/Relations/BelongsTo.js +++ b/src/Lucid/Relations/BelongsTo.js @@ -11,6 +11,8 @@ const Relation = require('./Relation') const NE = require('node-exceptions') +const CatLog = require('cat-log') +const logger = new CatLog('adonis:lucid') class ModelRelationAssociateException extends NE.LogicalException {} class ModelRelationSaveException extends NE.LogicalException {} @@ -67,11 +69,14 @@ class BelongsTo extends Relation { */ associate (relatedInstance) { if (relatedInstance instanceof this.related === false) { - throw new ModelRelationAssociateException('associate accepts an instance of related model') + throw new ModelRelationAssociateException('Associate accepts an instance of related model') } - if (!relatedInstance[this.toKey]) { + if (relatedInstance.isNew()) { throw new ModelRelationAssociateException('Cannot associate an unsaved related model') } + if (!relatedInstance[this.toKey]) { + logger.warn(`Trying to associate relationship with ${this.toKey} as foriegnKey, whose value is falsy`) + } this.parent[this.fromKey] = relatedInstance[this.toKey] } diff --git a/src/Lucid/Relations/BelongsToMany.js b/src/Lucid/Relations/BelongsToMany.js index b1704934..cb1e3f91 100644 --- a/src/Lucid/Relations/BelongsToMany.js +++ b/src/Lucid/Relations/BelongsToMany.js @@ -13,6 +13,8 @@ const Relation = require('./Relation') const NE = require('node-exceptions') const _ = require('lodash') const helpers = require('../QueryBuilder/helpers') +const CatLog = require('cat-log') +const logger = new CatLog('adonis:lucid') class ModelRelationException extends NE.LogicalException {} class ModelRelationSaveException extends NE.LogicalException {} class ModelRelationAttachException extends NE.LogicalException {} @@ -25,13 +27,19 @@ class BelongsToMany extends Relation { this.parent = parent this.related = related this.relatedQuery = this.related.query() - this.pivotTable = pivotTable - this.toKey = relatedPrimaryKey - this.fromKey = primaryKey - this.pivotLocalKey = pivotLocalKey - this.pivotOtherKey = pivotOtherKey + this.pivotTable = pivotTable // post_comment + this.toKey = relatedPrimaryKey // comments -> id + this.fromKey = primaryKey // post -> id + this.pivotLocalKey = pivotLocalKey // post_id + this.pivotOtherKey = pivotOtherKey // comment_id this.pivotPrefix = '_pivot_' this.pivotItems = [] + + /** + * helper method to query the pivot table. One + * can also do it manually by prefixing the + * pivot table name. + */ this.relatedQuery.wherePivot = function () { const args = _.toArray(arguments) args[0] = `${self.pivotTable}.${args[0]}` @@ -39,6 +47,12 @@ class BelongsToMany extends Relation { } } + /** + * makes the join query to be used by other + * methods. + * + * @public + */ _makeJoinQuery () { const self = this const selectionKeys = [ @@ -66,8 +80,11 @@ class BelongsToMany extends Relation { * @public */ fetch () { + if (this.parent.isNew()) { + throw new ModelRelationException('Cannot fetch related model from an unsaved model instance') + } if (!this.parent[this.fromKey]) { - throw new ModelRelationException('cannot fetch related model from an unsaved model instance') + logger.warn(`Trying to fetch relationship with ${this.fromKey} as primaryKey, whose value is falsy`) } this._makeJoinQuery() this.relatedQuery.where(`${this.pivotTable}.${this.pivotLocalKey}`, this.parent[this.fromKey]) @@ -84,8 +101,11 @@ class BelongsToMany extends Relation { * @public */ first () { + if (this.parent.isNew()) { + throw new ModelRelationException('Cannot fetch related model from an unsaved model instance') + } if (!this.parent[this.fromKey]) { - throw new ModelRelationException('cannot fetch related model from an unsaved model instance') + logger.warn(`Trying to fetch relationship with ${this.fromKey} as primaryKey, whose value is falsy`) } this._makeJoinQuery() this.relatedQuery.where(`${this.pivotTable}.${this.pivotLocalKey}`, this.parent[this.fromKey]) @@ -157,8 +177,17 @@ class BelongsToMany extends Relation { */ * attach (references, pivotValues) { pivotValues = pivotValues || {} + + if (!_.isArray(references) && !_.isObject(references)) { + throw new ModelRelationAttachException('attach expects an array or an object of values to be attached') + } + + if (this.parent.isNew()) { + throw new ModelRelationAttachException('Cannot attach values for an unsaved model instance') + } + if (!this.parent[this.fromKey]) { - throw new ModelRelationAttachException('cannot attach values for an unsaved model') + logger.warn(`Trying to attach values with ${this.fromKey} as primaryKey, whose value is falsy`) } if (_.isArray(references)) { @@ -178,17 +207,57 @@ class BelongsToMany extends Relation { yield this.relatedQuery.queryBuilder.table(this.pivotTable).insert(values) } + /** + * removes the relationship stored inside a pivot table. If + * references are not defined all relationships will be + * deleted + * @method detach + * @param {Array} [references] + * @return {Number} + * + * @public + */ * detach (references) { + if (this.parent.isNew()) { + throw new ModelRelationException('Cannot detach values for an unsaved model instance') + } if (!this.parent[this.fromKey]) { - throw new ModelRelationAttachException('cannot detach values for an unsaved model') + logger.warn(`Trying to attach values with ${this.fromKey} as primaryKey, whose value is falsy`) } const query = this.relatedQuery.queryBuilder.table(this.pivotTable).where(`${this.pivotLocalKey}`, this.parent[this.fromKey]) + + /** + * if references have been passed, then only remove them + */ if (_.isArray(references)) { query.whereIn(`${this.pivotOtherKey}`, references) } return yield query.delete() } + /** + * shorthand for detach and then attach + * + * @param {Array} [references] + * @param {Object} [pivotValues] + * @return {Number} + * + * @public + */ + * sync (references, pivotValues) { + yield this.detach() + return yield this.attach(references, pivotValues) + } + + /** + * saves the related model and creates the relationship + * inside the pivot table. + * + * @param {Object} relatedInstance + * @return {Boolean} + * + * @public + */ * save (relatedInstance) { if (relatedInstance instanceof this.related === false) { throw new ModelRelationSaveException('save accepts an instance of related model') @@ -205,6 +274,21 @@ class BelongsToMany extends Relation { return isSaved } + /** + * creates the related model instance and calls save on it + * + * @param {Object} values + * @return {Boolean} + * + * @public + */ + * create (values) { + const RelatedModel = this.related + const relatedInstance = new RelatedModel(values) + yield this.save(relatedInstance) + return relatedInstance + } + } module.exports = BelongsToMany diff --git a/src/Lucid/Relations/Relation.js b/src/Lucid/Relations/Relation.js index bdac1e21..605d6a80 100644 --- a/src/Lucid/Relations/Relation.js +++ b/src/Lucid/Relations/Relation.js @@ -11,6 +11,8 @@ require('harmony-reflect') const proxyHandler = require('./proxyHandler') +const CatLog = require('cat-log') +const logger = new CatLog('adonis:lucid') const NE = require('node-exceptions') class ModelRelationException extends NE.LogicalException {} class ModelRelationSaveException extends NE.LogicalException {} @@ -29,8 +31,11 @@ class Relation { * @public */ first () { + if (this.parent.isNew()) { + throw new ModelRelationException('Cannot fetch related model from an unsaved model instance') + } if (!this.parent[this.fromKey]) { - throw new ModelRelationException('cannot fetch related model from an unsaved model instance') + logger.warn(`Trying to fetch relationship with ${this.fromKey} as primaryKey, whose value is falsy`) } this.relatedQuery.where(this.toKey, this.parent[this.fromKey]) return this.relatedQuery.first() @@ -44,8 +49,11 @@ class Relation { * @public */ fetch () { + if (this.parent.isNew()) { + throw new ModelRelationException('Cannot fetch related model from an unsaved model instance') + } if (!this.parent[this.fromKey]) { - throw new ModelRelationException('cannot fetch related model from an unsaved model instance') + logger.warn(`Trying to fetch relationship with ${this.fromKey} as primaryKey, whose value is falsy`) } this.relatedQuery.where(this.toKey, this.parent[this.fromKey]) return this.relatedQuery.fetch() @@ -90,10 +98,13 @@ class Relation { */ * save (relatedInstance) { if (relatedInstance instanceof this.related === false) { - throw new ModelRelationSaveException('save accepts an instance of related model') + throw new ModelRelationSaveException('Save accepts an instance of related model') + } + if (this.parent.isNew()) { + throw new ModelRelationSaveException('Cannot save relation for an unsaved model instance') } if (!this.parent[this.fromKey]) { - throw new ModelRelationSaveException('cannot save relation for an unsaved model instance') + logger.warn(`Trying to save relationship with ${this.fromKey} as primaryKey, whose value is falsy`) } relatedInstance[this.toKey] = this.parent[this.fromKey] return yield relatedInstance.save() @@ -109,11 +120,10 @@ class Relation { * @public */ * create (values) { - if (!this.parent[this.fromKey]) { - throw new ModelRelationSaveException('cannot create relation for an unsaved model instance') - } - values[this.toKey] = this.parent[this.fromKey] - return yield this.related.create(values) + const RelatedModel = this.related + const relatedInstance = new RelatedModel(values) + yield this.save(relatedInstance) + return relatedInstance } } diff --git a/test/unit/lucid.relations.spec.js b/test/unit/lucid.relations.spec.js index 62adb193..83d305eb 100644 --- a/test/unit/lucid.relations.spec.js +++ b/test/unit/lucid.relations.spec.js @@ -116,7 +116,7 @@ describe('Relations', function () { yield supplier.account().where('age', 22).fetch() expect(true).to.equal(false) } catch (e) { - expect(e.message).to.match(/cannot fetch related model from an unsaved model instance/) + expect(e.message).to.match(/cannot fetch related model from an unsaved model instance/i) } }) @@ -1339,7 +1339,7 @@ describe('Relations', function () { const fn = function () { return comment.post().associate(post) } - expect(fn).to.throw(/associate accepts an instance of related model/) + expect(fn).to.throw(/associate accepts an instance of related model/i) }) it('should throw an error when trying to associate a related model which is unsaved', function * () { @@ -1358,7 +1358,7 @@ describe('Relations', function () { const fn = function () { return comment.post().associate(post) } - expect(fn).to.throw(/Cannot associate an unsaved related model/) + expect(fn).to.throw(/Cannot associate an unsaved related model/i) }) it('should throw an error when trying to call save method on a belongsTo relation', function * () { @@ -1675,6 +1675,29 @@ describe('Relations', function () { yield relationFixtures.truncate(Database, 'course_student') }) + it('should throw an error when not passing array of object to the attach method', function * () { + const savedStudent = yield relationFixtures.createRecords(Database, 'students', {name: 'ricky', id: 29}) + class Course extends Model { + } + class Student extends Model { + courses () { + return this.belongsToMany(Course) + } + } + Course.bootIfNotBooted() + const student = yield Student.find(savedStudent[0]) + expect(student instanceof Student).to.equal(true) + expect(student.id).to.equal(savedStudent[0]) + try { + yield student.courses().attach('foo') + expect(true).to.equal(false) + } catch (e) { + expect(e.name).to.equal('ModelRelationAttachException') + expect(e.message).to.match(/attach expects an array or an object of values to be attached/i) + } + yield relationFixtures.truncate(Database, 'students') + }) + it('should be able to attach related models with their ids', function * () { const savedStudent = yield relationFixtures.createRecords(Database, 'students', {name: 'ricky', id: 29}) const savedCourse = yield relationFixtures.createRecords(Database, 'courses', {title: 'geometry', id: 12}) @@ -1859,5 +1882,84 @@ describe('Relations', function () { yield relationFixtures.truncate(Database, 'courses') yield relationFixtures.truncate(Database, 'course_student') }) + + it('should be detach all mappings from pivot table and attach the given ones', function * () { + const savedStudent = yield relationFixtures.createRecords(Database, 'students', {name: 'ricky', id: 29}) + const savedCourse = yield relationFixtures.createRecords(Database, 'courses', {title: 'geometry', id: 12}) + const savedCourse1 = yield relationFixtures.createRecords(Database, 'courses', {title: 'geometry', id: 38}) + yield relationFixtures.createRecords(Database, 'course_student', [{student_id: savedStudent[0], course_id: savedCourse[0]}]) + class Course extends Model { + } + class Student extends Model { + courses () { + return this.belongsToMany(Course) + } + } + + Course.bootIfNotBooted() + const student = yield Student.find(savedStudent[0]) + expect(student instanceof Student).to.equal(true) + expect(student.id).to.equal(savedStudent[0]) + const courses = yield student.courses().fetch() + expect(courses.size()).to.equal(1) + expect(courses.first()._pivot_course_id).to.equal(savedCourse[0]) + expect(courses.first()._pivot_student_id).to.equal(savedStudent[0]) + yield student.courses().sync(savedCourse1) + + const newCourses = yield student.courses().fetch() + expect(newCourses.size()).to.equal(1) + expect(newCourses.first()._pivot_course_id).to.equal(savedCourse1[0]) + expect(newCourses.first()._pivot_student_id).to.equal(savedStudent[0]) + + yield relationFixtures.truncate(Database, 'students') + yield relationFixtures.truncate(Database, 'courses') + yield relationFixtures.truncate(Database, 'course_student') + }) + + it('should be able create a related model and put relation into pivot table', function * () { + const savedStudent = yield relationFixtures.createRecords(Database, 'students', {name: 'ricky', id: 29}) + class Course extends Model { + } + class Student extends Model { + courses () { + return this.belongsToMany(Course) + } + } + + Course.bootIfNotBooted() + const student = yield Student.find(savedStudent[0]) + expect(student instanceof Student).to.equal(true) + expect(student.id).to.equal(savedStudent[0]) + const course = yield student.courses().create({title: 'chemistry'}) + expect(course.id).not.to.equal(undefined) + expect(course._pivot_student_id).to.equal(student.id) + expect(course._pivot_course_id).to.equal(course.id) + + yield relationFixtures.truncate(Database, 'students') + yield relationFixtures.truncate(Database, 'courses') + yield relationFixtures.truncate(Database, 'course_student') + }) + + it('should consider pivot properties as dirty properties', function * () { + const savedStudent = yield relationFixtures.createRecords(Database, 'students', {name: 'ricky', id: 29}) + class Course extends Model { + } + class Student extends Model { + courses () { + return this.belongsToMany(Course) + } + } + + Course.bootIfNotBooted() + const student = yield Student.find(savedStudent[0]) + expect(student instanceof Student).to.equal(true) + expect(student.id).to.equal(savedStudent[0]) + const course = yield student.courses().create({title: 'chemistry'}) + expect(course.$dirty).deep.equal({}) + + yield relationFixtures.truncate(Database, 'students') + yield relationFixtures.truncate(Database, 'courses') + yield relationFixtures.truncate(Database, 'course_student') + }) }) })