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

Adding dirty tracking #798

Merged
merged 3 commits into from
Aug 2, 2013
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/dao-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,14 +377,15 @@ module.exports = (function() {
}

DAOFactory.prototype.build = function(values, options) {
options = options || { isNewRecord: true }
options = options || { isNewRecord: true, isDirty: true }

var self = this
, instance = new this.DAO(values, this.options, options.isNewRecord)

instance.isNewRecord = options.isNewRecord
instance.daoFactoryName = this.name
instance.daoFactory = this
instance.isDirty = options.isDirty

return instance
}
Expand Down
27 changes: 25 additions & 2 deletions lib/dao.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,15 @@ module.exports = (function() {
DAO.prototype.getDataValue = function(name) {
return this.dataValues && this.dataValues.hasOwnProperty(name) ? this.dataValues[name] : this[name]
}
DAO.prototype.get = DAO.prototype.getDataValue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️


DAO.prototype.setDataValue = function(name, value) {
if (Utils.areChanged(this.dataValues[name], value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename that method to differs or didChange ?

this.isDirty = true
}
this.dataValues[name] = value
}
DAO.prototype.set = DAO.prototype.setDataValue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For real, my jaw dropped in awe at how simple yet powerful that single statement was :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is so wonderful about this? what am I missing!


// if an array with field names is passed to save()
// only those fields will be updated
Expand Down Expand Up @@ -149,6 +154,7 @@ module.exports = (function() {
}).run()
}
else if (this.isNewRecord) {
this.isDirty = false
return this.QueryInterface.insert(this, this.QueryInterface.QueryGenerator.addSchema(this.__factory), values)
} else {
var identifier = this.__options.hasPrimaryKeys ? this.primaryKeyValues : { id: this.id };
Expand All @@ -157,6 +163,7 @@ module.exports = (function() {
identifier = this.__options.whereCollection;
}

this.isDirty = false
var tableName = this.QueryInterface.QueryGenerator.addSchema(this.__factory)
, query = this.QueryInterface.update(this, tableName, values, identifier)

Expand Down Expand Up @@ -191,6 +198,7 @@ module.exports = (function() {
this[valueName] = obj.values[valueName]
}
}
this.isDirty = false
emitter.emit('success', this)
}.bind(this))
}.bind(this)).run()
Expand Down Expand Up @@ -224,14 +232,21 @@ module.exports = (function() {
readOnlyAttributes.push('updatedAt')
readOnlyAttributes.push('deletedAt')

var isDirty = this.isDirty
Utils._.each(updates, function(value, attr) {
var updateAllowed = (
(readOnlyAttributes.indexOf(attr) == -1) &&
(readOnlyAttributes.indexOf(Utils._.underscored(attr)) == -1) &&
(self.attributes.indexOf(attr) > -1)
)
updateAllowed && (self[attr] = value)
if (updateAllowed) {
if (Utils.areChanged(self[attr], value)) {
isDirty = true
}
self[attr] = value
}
})
this.isDirty = isDirty
}

DAO.prototype.destroy = function() {
Expand Down Expand Up @@ -330,7 +345,15 @@ module.exports = (function() {
// (the same is true for __defineSetter and 'prototype' getters)
if (has !== true) {
this.__defineGetter__(attribute, has.get || function() { return this.dataValues[attribute]; });
this.__defineSetter__(attribute, has.set || function(v) { this.dataValues[attribute] = v; });
this.__defineSetter__(attribute, has.set || function(v) {
if (Utils.areChanged(this.dataValues[attribute], v)) {
//Only dirty the object if the change is not due to id, touchedAt, createdAt or updatedAt being initiated
if (this.dataValues[attribute] || (attribute != 'id' && attribute != 'touchedAt' && attribute != 'createdAt' && attribute != 'updatedAt')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll also need to check if underscore is true and if so the fields are deleted_at, created_at, etc.

this.isDirty = true
}
}
this.dataValues[attribute] = v
});
}

this[attribute] = value;
Expand Down
6 changes: 3 additions & 3 deletions lib/dialects/abstract/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ module.exports = (function() {
result = transformRowsWithEagerLoadingIntoDaos.call(this, results)
} else {
result = results.map(function(result) {
return this.callee.build(result, { isNewRecord: false })
return this.callee.build(result, { isNewRecord: false, isDirty: false })
}.bind(this))
}

Expand All @@ -287,7 +287,7 @@ module.exports = (function() {

var transformRowWithEagerLoadingIntoDao = function(result, dao) {
// let's build the actual dao instance first...
dao = dao || this.callee.build(result[this.callee.tableName], { isNewRecord: false })
dao = dao || this.callee.build(result[this.callee.tableName], { isNewRecord: false, isDirty: false })

// ... and afterwards the prefetched associations
for (var tableName in result) {
Expand Down Expand Up @@ -323,7 +323,7 @@ module.exports = (function() {
accessor = accessor.slice(0,1).toLowerCase() + accessor.slice(1)

associationData.forEach(function(data) {
var daoInstance = associatedDaoFactory.build(data, { isNewRecord: false })
var daoInstance = associatedDaoFactory.build(data, { isNewRecord: false, isDirty: false })
, isEmpty = !Utils.firstValueOfHash(daoInstance.identifiers)

if (['BelongsTo', 'HasOne'].indexOf(association.associationType) > -1) {
Expand Down
14 changes: 14 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,20 @@ var Utils = module.exports = {
isHash: function(obj) {
return Utils._.isObject(obj) && !Array.isArray(obj);
},
areChanged: function(attrValue, value) {
//If attribute value is Date, check value as a date
if (Utils._.isDate(attrValue) && !Utils._.isDate(value)) {
value = new Date(value)
}
if (Utils._.isDate(attrValue)) {
return attrValue.valueOf() !== value.valueOf()
}
//If both are them are empty, don't set as changed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, should be: "...both of them...".

if ((attrValue === undefined || attrValue === null || attrValue === '') && (value === undefined || value === null || value === '')) {
return false
}
return attrValue !== value
},
argsArePrimaryKeys: function(args, primaryKeys) {
var result = (args.length == Object.keys(primaryKeys).length)
if (result) {
Expand Down
127 changes: 127 additions & 0 deletions test/dao.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe(Support.getTestDialectTeaser("DAO"), function () {
touchedAt: { type: DataTypes.DATE, defaultValue: DataTypes.NOW },
aNumber: { type: DataTypes.INTEGER },
bNumber: { type: DataTypes.INTEGER },
aDate: { type: DataTypes.DATE },

validateTest: {
type: DataTypes.INTEGER,
Expand Down Expand Up @@ -110,6 +111,132 @@ describe(Support.getTestDialectTeaser("DAO"), function () {
})
})

describe('isDirty', function() {
it('returns true for non-saved objects', function(done) {
var user = this.User.build({ username: 'user' })
expect(user.id).to.be.null
expect(user.isDirty).to.be.true
done()
})

it("returns false for saved objects", function(done) {
this.User.build({ username: 'user' }).save().success(function(user) {
expect(user.isDirty).to.be.false
done()
})
})

it("returns true for changed attribute", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
user.username = 'new'
expect(user.isDirty).to.be.true
done()
})
})

it("returns false for non-changed attribute", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
user.username = 'user'
expect(user.isDirty).to.be.false
done()
})
})

it("returns false for non-changed date attribute", function(done) {
this.User.create({ aDate: new Date(2013, 6, 31, 14, 25, 21) }).success(function(user) {
user.aDate = '2013-07-31 14:25:21'
expect(user.isDirty).to.be.false
done()
})
})

it("returns false for two empty attributes", function(done) {
this.User.create({ username: null }).success(function(user) {
user.username = ''
expect(user.isDirty).to.be.false
done()
})
})

it("returns true for bulk changed attribute", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
user.setAttributes({
username: 'new',
aNumber: 1
})
expect(user.isDirty).to.be.true
done()
})
})

it("returns false for bulk non-changed attribute", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
user.setAttributes({
username: 'user'
})
expect(user.isDirty).to.be.false
done()
})
})

it("returns true for changed and bulk non-changed attribute", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
user.aNumber = 23
user.setAttributes({
username: 'user'
})
expect(user.isDirty).to.be.true
done()
})
})

it("returns true for changed attribute and false for saved object", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
user.username = 'new'
expect(user.isDirty).to.be.true
user.save().success(function() {
expect(user.isDirty).to.be.false
done()
})
})
})

it("returns false for created objects", function(done) {
this.User.create({ username: 'user' }).success(function(user) {
expect(user.isDirty).to.be.false
done()
})
})

it("returns false for objects found by find method", function(done) {
var self = this
this.User.create({ username: 'user' }).success(function(user) {
self.User.find(user.id).success(function(user) {
expect(user.isDirty).to.be.false
done()
})
})
})

it("returns false for objects found by findAll method", function(done) {
var self = this
, users = []

for (var i = 0; i < 10; i++) {
users[users.length] = {username: 'user'}
}

this.User.bulkCreate(users).success(function() {
self.User.findAll().success(function(users) {
users.forEach(function(u) {
expect(u.isDirty).to.be.false
})
done()
})
})
})
})

describe('increment', function () {
beforeEach(function(done) {
this.User.create({ id: 1, aNumber: 0, bNumber: 0 }).complete(function(){
Expand Down