Merge pull request #2300 from jtschoonhoven/master

Fix for issue #1829: down migrations operate only on *completed* migrations

lib/migrator.js

@@ -33,56 +33,64 @@ module.exports = (function() {
     }, options || {});
     }, options || {});
 
 
     return new Utils.CustomEventEmitter(function(emitter) {
     return new Utils.CustomEventEmitter(function(emitter) {
-      self.getUndoneMigrations(function(err, migrations) {
-        if (err) {
-          emitter.emit('error', err);
-        } else {
-          var chainer = new Utils.QueryChainer()
-            , from = migrations[0];
+      if(options.method === 'down') {
+        self.getCompletedMigrations(function(err, migrations){
+          migrations.reverse();
+          self.processMigrations(err, migrations, options, emitter);
+        });
+      } else {
+        self.getUndoneMigrations(function(err, migrations){
+          self.processMigrations(err, migrations, options, emitter);
+        });
+      }
+    }).run();
+  };
 
 
-          if (options.method === 'down') {
-            migrations.reverse();
-          }
+  Migrator.prototype.processMigrations = function(err, migrations, options, emitter) {
+    var self = this;
+    if (err) {
+      emitter.emit('error', err);
+    } else {
+      var chainer = new Utils.QueryChainer()
+        , from = migrations[0];
 
 
-          if (migrations.length === 0) {
-            self.options.logging('There are no pending migrations.');
-          } else {
-            self.options.logging('Running migrations...');
-          }
+      if (migrations.length === 0) {
+        self.options.logging('There are no pending migrations.');
+      } else {
+        self.options.logging('Running migrations...');
+      }
 
 
-          migrations.forEach(function(migration) {
-            var migrationTime;
-
-            chainer.add(migration, 'execute', [options], {
-              before: function(migration) {
-                self.options.logging(migration.filename);
-                migrationTime = process.hrtime();
-              },
-
-              after: function(migration) {
-                migrationTime = process.hrtime(migrationTime);
-                migrationTime = Math.round((migrationTime[0] * 1000) + (migrationTime[1] / 1000000));
-
-                self.options.logging('Completed in ' + migrationTime + 'ms');
-              },
-
-              success: function(migration, callback) {
-                if (options.method === 'down') {
-                  deleteUndoneMigration.call(self, from, migration, callback);
-                } else {
-                  saveSuccessfulMigration.call(self, from, migration, callback);
-                }
-              }
-            });
-          });
+      migrations.forEach(function(migration) {
+        var migrationTime;
 
 
-          chainer
-            .runSerially({ skipOnError: true })
-            .success(function() { emitter.emit('success', null); })
-            .error(function(err) { emitter.emit('error', err); });
-        }
+        chainer.add(migration, 'execute', [options], {
+          before: function(migration) {
+            self.options.logging(migration.filename || migration.id);
+            migrationTime = process.hrtime();
+          },
+
+          after: function(migration) {
+            migrationTime = process.hrtime(migrationTime);
+            migrationTime = Math.round((migrationTime[0] * 1000) + (migrationTime[1] / 1000000));
+
+            self.options.logging('Completed in ' + migrationTime + 'ms');
+          },
+
+          success: function(migration, callback) {
+            if (options.method === 'down') {
+              deleteUndoneMigration.call(self, from, migration, callback);
+            } else {
+              saveSuccessfulMigration.call(self, from, migration, callback);
+            }
+          }
+        });
       });
       });
-    }).run();
+
+      chainer
+        .runSerially({ skipOnError: true })
+        .success(function() { emitter.emit('success', null); })
+        .error(function(err) { emitter.emit('error', err); });
+    }
   };
   };
 
 
   Migrator.prototype.getUndoneMigrations = function(callback)  {
   Migrator.prototype.getUndoneMigrations = function(callback)  {
@@ -151,6 +159,41 @@ module.exports = (function() {
     }
     }
   };
   };
 
 
+  Migrator.prototype.getCompletedMigrations = function(callback)  {
+    var self = this;
+
+    var filterByMigrationId = function(migrations, allMigrationIds, callback) {
+      return migrations.filter(function(migration) {
+        return allMigrationIds.indexOf(migration.migrationId) > -1;
+      });
+    };
+
+    var migrationFiles = fs.readdirSync(this.options.path).filter(function(file) {
+      return self.options.filesFilter.test(file);
+    });
+
+    var migrations = migrationFiles.map(function(file) {
+      return new Migration(self, self.options.path + '/' + file);
+    });
+
+    migrations = migrations.sort(function(a, b) {
+      return parseInt(a.filename.split('-')[0]) - parseInt(b.filename.split('-')[0]);
+    });
+
+    var existingMigrationIds = migrations.map(function(migration) {
+      return migration.migrationId;
+    });
+
+    getAllMigrationIdsFromDatabase.call(self)
+    .then(function(allMigrationIds) {
+      allMigrationIds = allMigrationIds.map(function(migration) {
+        return parseInt(migration.to);
+      });
+      var completedMigrations = filterByMigrationId(migrations, allMigrationIds);
+      callback(null, completedMigrations || []);
+    });
+  };
+
   Migrator.prototype.findOrCreateSequelizeMetaDAO = function(syncOptions) {
   Migrator.prototype.findOrCreateSequelizeMetaDAO = function(syncOptions) {
     var self = this;
     var self = this;
 
 
@@ -323,15 +366,17 @@ module.exports = (function() {
 
 
   var deleteUndoneMigration = Migrator.prototype.deleteUndoneMigration = function(from, to, callback) {
   var deleteUndoneMigration = Migrator.prototype.deleteUndoneMigration = function(from, to, callback) {
     var self = this;
     var self = this;
-
     self.findOrCreateSequelizeMetaDAO().success(function(SequelizeMeta) {
     self.findOrCreateSequelizeMetaDAO().success(function(SequelizeMeta) {
-      SequelizeMeta
-        .find({ where: { from: from.migrationId.toString(), to: to.migrationId.toString() } })
-        .success(function(meta) {
+      SequelizeMeta.find({ where: { from: from.migrationId.toString(), to: to.migrationId.toString() } })
+      .success(function(meta) {
+        if(meta){
           meta.destroy().success(callback);
           meta.destroy().success(callback);
-        });
+        } else {
+          callback();
+        }
+      });
     });
     });
   };
   };
 
 
   return Migrator;
   return Migrator;
-})();
+})();
\ No newline at end of file

test/migrator.test.js

@@ -27,6 +27,44 @@ describe(Support.getTestDialectTeaser("Migrator"), function() {
     }.bind(this)
     }.bind(this)
   })
   })
 
 
+  describe('getCompletedMigrations', function(){
+    it("supports coffee files", function(done) {
+      this.init({ filesFilter: /\.coffee$/, to: 20111117063700 }, function(migrator, SequelizeMeta) {
+        SequelizeMeta.create({ from: null, to: 20111117063700 }).success(function() {
+          migrator.getCompletedMigrations(function(err, migrations) {
+            expect(err).to.be.null
+            expect(migrations).to.have.length(1)
+            done()
+          })
+        })
+      })
+    })
+
+    it("only returns migrations if timestamps in db are after timestamps of files", function(done) {
+      this.init({ to: 20111117063700 }, function(migrator, SequelizeMeta) {
+        SequelizeMeta.create({ from: null, to: 20111117063700 }).success(function() {
+          migrator.getCompletedMigrations(function(err, migrations) {
+            expect(err).to.be.null
+            expect(migrations).to.have.length(1)
+            done()
+          })
+        })
+      })
+    })
+
+    it("returns no migrations if timestamps in db are before timestamps of files", function(done) {
+      this.init({ to: 20111117063700 }, function(migrator, SequelizeMeta) {
+        SequelizeMeta.create({ from: null, to: 20101117063700 }).success(function() {
+          migrator.getCompletedMigrations(function(err, migrations) {
+            expect(err).to.be.null
+            expect(migrations).to.have.length(0)
+            done()
+          })
+        })
+      })
+    })
+  })
+
   describe('getUndoneMigrations', function() {
   describe('getUndoneMigrations', function() {
     it("supports coffee files", function(done) {
     it("supports coffee files", function(done) {
       this.init({
       this.init({