Merge pull request bonaval#40 from bonaval/fix/reloadIfRequired

Fix/reload if required

Author: ddolcimascolo

index.js

@@ -72,6 +72,10 @@ var Temporal = function(model, sequelize, temporalOptions){
       return;
     }
 
+    function isVirtual(attribute) {
+      return attributes[attribute].type instanceof Sequelize.VIRTUAL;
+    }
+
     async function getDataValues() {
       if (!temporalOptions.full) {
         return obj._previousDataValues || obj.dataValues;
@@ -85,7 +89,7 @@ var Temporal = function(model, sequelize, temporalOptions){
       }
 
       const attributesToReload = Object.keys(attributes).filter(attribute => {
-        if (!temporalOptions.reloadIgnoredAttributes.includes(attribute) && !(attribute in obj.dataValues)) {
+        if (!temporalOptions.reloadIgnoredAttributes.includes(attribute) && !isVirtual(attribute) && !(attribute in obj.dataValues)) {
           return true;
         }
       });
@@ -97,7 +101,7 @@ var Temporal = function(model, sequelize, temporalOptions){
        * Model.reload() will do its magic to merge the newly fetched values directly in dataValues. #gg
        */
       if (attributesToReload.length > 0) {
-        await obj.reload({attributes: attributesToReload, transaction: options.transaction, paranoid: false, include: []})
+        await obj.reload({attributes: attributesToReload, transaction: options.transaction, paranoid: false, include: null})
       }
 
       return obj.dataValues;

test/test.js

@@ -7,7 +7,15 @@ var assert = chai.assert;
 var eventually = assert.eventually;
 
 describe('Read-only API', function(){
-  var sequelize, User, UserHistory;
+  var sequelize, User, UserHistory, Group, queries = [];
+
+  function logging(message) {
+    if (process.env.LOGGING === 'true') {
+      console.log(message);
+    }
+
+    queries.push(message.substring(message.indexOf(':') + 2));
+  }
 
   function freshDB(){
     return freshDBWithOptions();
@@ -22,12 +30,21 @@ describe('Read-only API', function(){
     sequelize = new Sequelize('', '', '', {
       dialect: 'sqlite',
       storage: __dirname + '/.test.sqlite',
-      logging: process.env.LOGGING === 'true' ? console.log : false
+      logging
     });
     User = Temporal(sequelize.define('User', {
-      name: Sequelize.TEXT
+      name: Sequelize.TEXT,
+      virtual: Sequelize.VIRTUAL(Sequelize.STRING, ['name']),
     }, modelOptions), sequelize, temporalOptions);
     UserHistory = sequelize.models.UserHistory;
+
+    Group = sequelize.define('Group', {
+      name: Sequelize.TEXT
+    });
+    Group.hasMany(User);
+    User.belongsTo(Group);
+    User.addScope('withGroup', { include: [Group] });
+
     return sequelize.sync({ force: true });
   }
 
@@ -41,6 +58,10 @@ describe('Read-only API', function(){
     }
   }
 
+  afterEach(function() {
+    queries.length = 0;
+  });
+
   describe('hooks', function(){
     beforeEach(freshDB);
     it('onCreate: should not store the new version in history db' , function(){
@@ -328,7 +349,7 @@ describe('Read-only API', function(){
 
     it('onUpdate: should store the previous version to the historyDB even if entity was partially loaded' , async function(){
       const created = await User.create({ name: 'name' });
-      const user = await User.findByPk(created.id, { attributes: ['id', 'name'] }); // Don't fetch timestamps
+      const user = await User.scope('withGroup').findByPk(created.id, { attributes: ['id', 'name'] }); // Don't fetch timestamps
 
       await user.update({ name: 'newName' });
       await user.update({ name: 'thirdName' });
@@ -348,6 +369,46 @@ describe('Read-only API', function(){
       assert.equal('name', initial.name);
       assert.equal('newName', firstUpdate.name);
       assert.equal('thirdName', secondUpdate.name);
+
+      const selects = queries.filter(query => query.startsWith('SELECT'));
+
+      assert.deepEqual(selects, [
+        "SELECT `User`.`id`, `User`.`name`, `Group`.`id` AS `Group.id`, `Group`.`name` AS `Group.name`, `Group`.`createdAt` AS `Group.createdAt`, `Group`.`updatedAt` AS `Group.updatedAt` FROM `Users` AS `User` LEFT OUTER JOIN `Groups` AS `Group` ON `User`.`GroupId` = `Group`.`id` WHERE (`User`.`deletedAt` IS NULL AND `User`.`id` = 1);",
+        "SELECT `createdAt`, `GroupId` FROM `Users` AS `User` WHERE `User`.`id` = 1;", // Reload for first update, no includes and only required fields. Second update does not need a reload
+        "SELECT `id`, `name`, `createdAt`, `updatedAt`, `deletedAt`, `hid`, `archivedAt` FROM `UserHistories` AS `UserHistory`;"
+      ])
+    });
+
+    it('onUpdate: should not reload virtual fields' , async function(){
+      const created = await User.create({ name: 'name' });
+      const user = await User.findByPk(created.id, { attributes: ['id', 'name', 'createdAt', 'GroupId'] }); // Don't fetch timestamps
+
+      await user.update({ name: 'newName' });
+      await user.update({ name: 'thirdName' });
+
+      const history = await UserHistory.findAll();
+
+      assert.equal(history.length, 3, 'initial revision and to updates saved');
+
+      const [initial, firstUpdate, secondUpdate] = history;
+
+      assert.equal(+initial.createdAt, +firstUpdate.createdAt, 'createdAt was saved during first update, despite not being eagerly loaded');
+      assert.equal(+initial.createdAt, +secondUpdate.createdAt, 'createdAt was saved during second update, despite not being eagerly loaded');
+
+      assert.isAtLeast(firstUpdate.updatedAt, initial.createdAt, 'updatedAt was saved during first update');
+      assert.isAtLeast(secondUpdate.updatedAt, firstUpdate.updatedAt, 'updatedAt was saved during second update');
+
+      assert.equal('name', initial.name);
+      assert.equal('newName', firstUpdate.name);
+      assert.equal('thirdName', secondUpdate.name);
+
+      const selects = queries.filter(query => query.startsWith('SELECT'));
+
+      // No reload
+      assert.deepEqual(selects, [
+        "SELECT `id`, `name`, `createdAt`, `GroupId` FROM `Users` AS `User` WHERE (`User`.`deletedAt` IS NULL AND `User`.`id` = 1);",
+        "SELECT `id`, `name`, `createdAt`, `updatedAt`, `deletedAt`, `hid`, `archivedAt` FROM `UserHistories` AS `UserHistory`;"
+      ])
     });
 
   });