#564 - DDL DB Migration - migration generation on history table, pending drop columns missing in the history view and tigger

src/main/java/com/avaje/ebean/dbmigration/DbMigration.java

@@ -220,6 +220,9 @@ public class DbMigration {
     }
   }
 
+  /**
+   * Generate the diff migration.
+   */
   private void generateDiff(Request request) throws IOException {
 
     List<String> pendingDrops = request.getPendingDrops();
@@ -227,15 +230,18 @@ public class DbMigration {
       logger.info("Pending un-applied drops in versions {}", pendingDrops);
     }
 
-    ModelDiff diff = request.createDiff();
-    if (diff.isEmpty()) {
+    Migration migration = request.createDiffMigration();
+    if (migration == null) {
       logger.info("no changes detected - no migration written");
     } else {
       // there were actually changes to write
-      generateMigration(request, diff.getMigration(), null);
+      generateMigration(request, migration, null);
     }
   }
 
+  /**
+   * Generate the migration based on the pendingDrops from a prior version.
+   */
   private void generatePendingDrop(Request request, String pendingVersion) throws IOException {
 
     Migration migration = request.migrationForPendingDrop(pendingVersion);
@@ -290,12 +296,12 @@ public class DbMigration {
     }
 
     /**
-     * Create an return the diff of the current model to the migration model.
+     * Create and return the diff of the current model to the migration model.
      */
-    public ModelDiff createDiff() {
+    public Migration createDiffMigration() {
       ModelDiff diff = new ModelDiff(migrated);
       diff.compareTo(current);
-      return diff;
+      return diff.isEmpty() ? null : diff.getMigration();
     }
   }
 

src/main/java/com/avaje/ebean/dbmigration/model/CurrentModel.java

@@ -159,12 +159,7 @@ public class CurrentModel {
     ModelDiff diff = new ModelDiff();
     diff.compareTo(model);
 
-    List<Object> applyChanges = diff.getApplyChanges();
-
-    // put the changes into a ChangeSet
-    ChangeSet applyChangeSet = new ChangeSet();
-    applyChangeSet.getChangeSetChildren().addAll(applyChanges);
-    return applyChangeSet;
+    return diff.getApplyChangeSet();
   }
 
 }

src/main/java/com/avaje/ebean/dbmigration/model/MTable.java

@@ -356,8 +356,9 @@ public class MTable {
     return withHistory;
   }
 
-  public void setWithHistory(boolean withHistory) {
+  public MTable setWithHistory(boolean withHistory) {
     this.withHistory = withHistory;
+    return this;
   }
 
   public List<String> allHistoryColumns(boolean includeDropped) {

src/main/java/com/avaje/ebean/dbmigration/model/ModelContainer.java

@@ -13,7 +13,6 @@ import com.avaje.ebean.dbmigration.migration.DropIndex;
 import com.avaje.ebean.dbmigration.migration.DropTable;
 import com.avaje.ebean.dbmigration.migration.Migration;
 
-import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -45,8 +44,7 @@ public class ModelContainer {
    * Adjust the FK references on all the draft tables.
    */
   public void adjustDraftReferences() {
-    Collection<MTable> tables = this.tables.values();
-    for (MTable table : tables) {
+    for (MTable table : this.tables.values()) {
       if (table.isDraft()) {
         table.adjustReferences(this);
       }
@@ -94,7 +92,7 @@ public class ModelContainer {
         pendingDrops.add(version, changeSet);
 
       } else if (isDropsFor(changeSet)) {
-        pendingDrops.appliedDropsFor(MigrationVersion.parse(changeSet.getDropsFor()));
+        pendingDrops.appliedDropsFor(changeSet);
       }
       if (!isDropsFor(changeSet)) {
         applyChangeSet(changeSet);
@@ -286,10 +284,26 @@ public class ModelContainer {
     pendingDrops.registerPendingHistoryDropColumns(newModel);
   }
 
+  /**
+   * Register any pending drop columns on history tables.  These columns are now not in the current
+   * logical model but we still need to include them in the history views and triggers until they
+   * are actually dropped.
+   */
+  public void registerPendingHistoryDropColumns(ChangeSet changeSet) {
+    for (Object change : changeSet.getChangeSetChildren()) {
+      if (change instanceof DropColumn) {
+        DropColumn dropColumn = (DropColumn) change;
+        if (Boolean.TRUE.equals(dropColumn.isWithHistory())) {
+          registerPendingDropColumn(dropColumn);
+        }
+      }
+    }
+  }
+
   /**
    * Register a drop column on a history tables that has not been applied yet.
    */
-  public void registerPendingDropColumn(DropColumn dropColumn) {
+  private void registerPendingDropColumn(DropColumn dropColumn) {
 
     MTable table = getTable(dropColumn.getTableName());
     if (table == null) {

src/main/java/com/avaje/ebean/dbmigration/model/ModelDiff.java

@@ -66,16 +66,14 @@ public class ModelDiff {
   public Migration getMigration() {
 
     Migration migration = new Migration();
-    ChangeSet applyChangeSet = getApplyChangeSet();
-    if (!applyChangeSet.getChangeSetChildren().isEmpty()) {
+    if (!applyChanges.isEmpty()) {
       // add a non empty apply changeSet
-      migration.getChangeSet().add(applyChangeSet);
+      migration.getChangeSet().add(getApplyChangeSet());
     }
 
-    ChangeSet dropChangeSet = getDropChangeSet();
-    if (!dropChangeSet.getChangeSetChildren().isEmpty()) {
+    if (!dropChanges.isEmpty()) {
       // add a non empty drop changeSet
-      migration.getChangeSet().add(dropChangeSet);
+      migration.getChangeSet().add(getDropChangeSet());
     }
     return migration;
   }
@@ -83,14 +81,14 @@ public class ModelDiff {
   /**
    * Return the list of 'apply' changes.
    */
-  public List<Object> getApplyChanges() {
+  List<Object> getApplyChanges() {
     return applyChanges;
   }
 
   /**
    * Return the list of 'drop' changes.
    */
-  public List<Object> getDropChanges() {
+  List<Object> getDropChanges() {
     return dropChanges;
   }
 
@@ -108,7 +106,7 @@ public class ModelDiff {
   /**
    * Return the 'drop' changeSet.
    */
-  public ChangeSet getDropChangeSet() {
+  ChangeSet getDropChangeSet() {
     // put the changes into a ChangeSet
     ChangeSet createChangeSet = new ChangeSet();
     createChangeSet.setType(ChangeSetType.PENDING_DROPS);
@@ -156,7 +154,12 @@ public class ModelDiff {
       }
     }
 
+    // register un-applied ones from the previous migrations
     baseModel.registerPendingHistoryDropColumns(newModel);
+    if (!dropChanges.isEmpty()) {
+      // register new ones created just now as part of this diff
+      newModel.registerPendingHistoryDropColumns(getDropChangeSet());
+    }
   }
 
   protected void addDropTable(MTable existingTable) {

src/main/java/com/avaje/ebean/dbmigration/model/PendingDrops.java

@@ -3,6 +3,7 @@ package com.avaje.ebean.dbmigration.model;
 import com.avaje.ebean.dbmigration.migration.ChangeSet;
 import com.avaje.ebean.dbmigration.migration.ChangeSetType;
 import com.avaje.ebean.dbmigration.migration.DropColumn;
+import com.avaje.ebean.dbmigration.migration.DropTable;
 import com.avaje.ebean.dbmigration.migration.Migration;
 
 import java.util.ArrayList;
@@ -48,9 +49,12 @@ public class PendingDrops {
    * All the pending drops for this migration version have been applied so we need
    * to remove the (unsuppressed) pending drops for this version.
    */
-  public boolean appliedDropsFor(MigrationVersion version) {
+  public boolean appliedDropsFor(ChangeSet changeSet) {
+
+    MigrationVersion version = MigrationVersion.parse(changeSet.getDropsFor());
+
     Entry entry = map.get(version.normalised());
-    if (entry.removeDrops()) {
+    if (entry.removeDrops(changeSet)) {
       // it had no suppressForever changeSets so remove completely
       map.remove(version.normalised());
       return true;
@@ -70,8 +74,11 @@ public class PendingDrops {
     Entry entry = getEntry(pendingVersion);
 
     Migration migration = new Migration();
-    for (ChangeSet changeSet : entry.list) {
+    Iterator<ChangeSet> it = entry.list.iterator();
+    while (it.hasNext()) {
+      ChangeSet changeSet = it.next();
       if (!isSuppressForever(changeSet)) {
+        it.remove();
         changeSet.setType(ChangeSetType.APPLY);
         changeSet.setDropsFor(entry.version.asString());
         migration.getChangeSet().add(changeSet);
@@ -113,14 +120,7 @@ public class PendingDrops {
 
     for (Entry entry : map.values()) {
       for (ChangeSet changeSet : entry.list) {
-        for (Object change : changeSet.getChangeSetChildren()) {
-          if (change instanceof DropColumn) {
-            DropColumn dropColumn = (DropColumn) change;
-            if (Boolean.TRUE.equals(dropColumn.isWithHistory())) {
-              newModel.registerPendingDropColumn(dropColumn);
-            }
-          }
-        }
+        newModel.registerPendingHistoryDropColumns(changeSet);
       }
     }
   }
@@ -129,10 +129,16 @@ public class PendingDrops {
    * Return true if there is an Entry for the given version.
    */
   boolean testContainsEntryFor(MigrationVersion version) {
-
     return map.containsKey(version.normalised());
   }
 
+  /**
+   * Return the Entry for the given version.
+   */
+  Entry testGetEntryFor(MigrationVersion version) {
+    return map.get(version.normalised());
+  }
+
   static class Entry {
 
     final MigrationVersion version;
@@ -175,18 +181,79 @@ public class PendingDrops {
      * Remove the drops that are not suppressForever and return true if that
      * removed all the changeSets (and there are no suppressForever ones).
      */
-    boolean removeDrops() {
+    boolean removeDrops(ChangeSet appliedDrops) {
 
       Iterator<ChangeSet> iterator = list.iterator();
       while (iterator.hasNext()) {
         ChangeSet next = iterator.next();
         if (!isSuppressForever(next)) {
-          iterator.remove();
+          removeMatchingChanges(next, appliedDrops);
+          if (next.getChangeSetChildren().isEmpty()) {
+            iterator.remove();
+          }
         }
       }
 
       return list.isEmpty();
     }
+
+    /**
+     * Remove the applied drops from the pending ones matching by table name and column name.
+     */
+    private void removeMatchingChanges(ChangeSet pendingDrops, ChangeSet appliedDrops) {
+
+      List<Object> pending = pendingDrops.getChangeSetChildren();
+      Iterator<Object> iterator = pending.iterator();
+      while (iterator.hasNext()) {
+        Object pendingDrop = iterator.next();
+        if (pendingDrop instanceof DropColumn && dropColumnIn((DropColumn)pendingDrop, appliedDrops)) {
+          iterator.remove();
+
+        } else if (pendingDrop instanceof DropTable && dropTableIn((DropTable)pendingDrop, appliedDrops)) {
+          iterator.remove();
+        }
+      }
+    }
+
+    /**
+     * Return true if the pendingDrop is contained in the appliedDrops.
+     */
+    private boolean dropTableIn(DropTable pendingDrop, ChangeSet appliedDrops) {
+      for (Object o : appliedDrops.getChangeSetChildren()) {
+        if (o instanceof DropTable && sameTable(pendingDrop, (DropTable)o)) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+    /**
+     * Return true if the pendingDrop is contained in the appliedDrops.
+     */
+    private boolean dropColumnIn(DropColumn pendingDrop, ChangeSet appliedDrops) {
+      for (Object o : appliedDrops.getChangeSetChildren()) {
+        if (o instanceof DropColumn && sameColumn(pendingDrop, (DropColumn) o)) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+    /**
+     * Return true if the DropTable match by table name.
+     */
+    private boolean sameTable(DropTable pendingDrop, DropTable o) {
+      return pendingDrop.getName().equals(o.getName());
+    }
+
+    /**
+     * Return true if the DropColumns match by table and column name.
+     */
+    private boolean sameColumn(DropColumn pending, DropColumn o) {
+      return pending.getColumnName().equals(o.getColumnName())
+          && pending.getTableName().equals(o.getTableName());
+    }
+
   }
 
   private static boolean isSuppressForever(ChangeSet next) {