#374 - Add support for rollback on history function

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

@@ -57,7 +57,7 @@ public class DbOffline {
   public static void reset() {
     runningMigration = false;
     System.clearProperty(KEY);
-    logger.info("reset");
+    logger.debug("reset");
   }
 
 }

src/main/java/com/avaje/ebean/dbmigration/ddlgeneration/platform/BaseTableDdl.java

@@ -517,7 +517,7 @@ public class BaseTableDdl implements TableDdl {
       // make same changes to the history table
       String historyTable = historyTable(tableName);
       for (Column column : columns) {
-        regenerateHistoryTriggers(tableName, "added " + column.getName());
+        regenerateHistoryTriggers(tableName, HistoryTableUpdate.Change.ADD, column.getName());
         alterTableAddColumn(writer.apply(), historyTable, column, true);
         alterTableDropColumn(writer.rollback(), historyTable, column.getName());
       }
@@ -548,7 +548,7 @@ public class BaseTableDdl implements TableDdl {
     alterTableDropColumn(writer.drop(), tableName, dropColumn.getColumnName());
     if (isTrue(dropColumn.isWithHistory())) {
       // also drop from the history table
-      regenerateHistoryTriggers(tableName, "dropped "+dropColumn.getColumnName());
+      regenerateHistoryTriggers(tableName, HistoryTableUpdate.Change.DROP, dropColumn.getColumnName());
       alterTableDropColumn(writer.drop(), historyTable(tableName), dropColumn.getColumnName());
     }
 
@@ -562,9 +562,9 @@ public class BaseTableDdl implements TableDdl {
   public void generate(DdlWrite writer, AlterColumn alterColumn) throws IOException {
 
     if (isTrue(alterColumn.isHistoryExclude())) {
-      regenerateHistoryTriggers(alterColumn.getTableName(), "exclude " + alterColumn.getColumnName());
+      regenerateHistoryTriggers(alterColumn.getTableName(), HistoryTableUpdate.Change.EXCLUDE, alterColumn.getColumnName());
     } else if (isFalse(alterColumn.isHistoryExclude())) {
-      regenerateHistoryTriggers(alterColumn.getTableName(), "include " + alterColumn.getColumnName());
+      regenerateHistoryTriggers(alterColumn.getTableName(), HistoryTableUpdate.Change.INCLUDE, alterColumn.getColumnName());
     }
 
     if (hasValue(alterColumn.getDropForeignKey())) {
@@ -613,14 +613,14 @@ public class BaseTableDdl implements TableDdl {
   /**
    * Register the base table that we need to regenerate the history triggers on.
    */
-  protected void regenerateHistoryTriggers(String baseTableName, String columnComment) {
+  protected void regenerateHistoryTriggers(String baseTableName, HistoryTableUpdate.Change change, String column) {
 
     HistoryTableUpdate update = regenerateHistoryTriggers.get(baseTableName);
     if (update == null) {
       update = new HistoryTableUpdate(baseTableName);
       regenerateHistoryTriggers.put(baseTableName, update);
     }
-    update.addComment(columnComment);
+    update.add(change, column);
   }
 
   /**

src/main/java/com/avaje/ebean/dbmigration/ddlgeneration/platform/HistoryTableUpdate.java

@@ -10,9 +10,51 @@ import java.util.List;
  */
 public class HistoryTableUpdate {
 
+
+  /**
+   * Column change type.
+   */
+  public enum Change {
+    ADD,
+    DROP,
+    INCLUDE,
+    EXCLUDE
+  }
+
+  public static class Column {
+
+    public final Change change;
+    public final String column;
+    public Column(Change change, String column) {
+      this.change = change;
+      this.column = column;
+    }
+
+    public String description() {
+      return change.name().toLowerCase()+" "+column;
+    }
+
+    public void apply(List<String> includedColumns) {
+      switch (change) {
+        case ADD:
+        case INCLUDE: {
+          includedColumns.remove(column);
+          break;
+        }
+        case DROP:
+        case EXCLUDE: {
+          includedColumns.add(column);
+          break;
+        }
+        default:
+          throw new IllegalStateException("Unexpected change "+change);
+      }
+    }
+  }
+
   private final String baseTable;
 
-  private final List<String> comments = new ArrayList<String>();
+  private final List<Column> columnChanges = new ArrayList<Column>();
 
   /**
    * Construct with a given base table name.
@@ -21,11 +63,34 @@ public class HistoryTableUpdate {
     this.baseTable = baseTable;
   }
 
+  /**
+   * Return a description of the changes that cause the history trigger/function
+   * to be regenerated (added, dropped, included or excluded columns).
+   */
+  public String description() {
+    StringBuilder sb = new StringBuilder(90);
+    for (int i = 0; i < columnChanges.size(); i++) {
+      if (i > 0) {
+        sb.append(", ");
+      }
+      sb.append(columnChanges.get(i).description());
+    }
+    return sb.toString();
+  }
+
+  public void toRevertedColumns(List<String> includedColumns) {
+
+    for (Column columnChange : columnChanges) {
+      columnChange.apply(includedColumns);
+    }
+  }
+
+
   /**
    * Add a comment for column added, dropped, included or excluded.
    */
-  public void addComment(String comment) {
-    comments.add(comment);
+  public void add(Change change, String column) {
+    columnChanges.add(new Column(change, column));
   }
 
   /**
@@ -38,7 +103,7 @@ public class HistoryTableUpdate {
   /**
    * Return the comments.
    */
-  public List<String> getComments() {
-    return comments;
+  public List<Column> getColumnChanges() {
+    return columnChanges;
   }
 }

src/main/java/com/avaje/ebean/dbmigration/ddlgeneration/platform/PostgresHistoryDdl.java

@@ -10,6 +10,7 @@ import com.avaje.ebean.dbmigration.model.MColumn;
 import com.avaje.ebean.dbmigration.model.MTable;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
@@ -44,7 +45,7 @@ public class PostgresHistoryDdl implements PlatformHistoryDdl {
     if (table == null) {
       throw new IllegalStateException("MTable "+update.getBaseTable()+" not found in writer? (required for history DDL)");
     }
-    addStoredFunction(writer, table, update.getComments());
+    addStoredFunction(writer, table, update);
   }
 
   @Override
@@ -157,24 +158,36 @@ public class PostgresHistoryDdl implements PlatformHistoryDdl {
         .append("  for each row execute procedure ").append(procedureName).append("();").newLine().newLine();
   }
 
-  protected void addStoredFunction(DdlWrite writer, MTable table, List<String> comments) throws IOException {
+  protected void addStoredFunction(DdlWrite writer, MTable table, HistoryTableUpdate update) throws IOException {
 
     String procedureName = procedureName(table.getName());
+    String historyTable = historyTableName(table.getName());
+
+    List<String> includedColumns = includedColumnNames(table);
 
     DdlBuffer apply = writer.applyHistory();
 
-    if (comments != null && !comments.isEmpty()) {
+    if (update != null) {
       apply.append("-- Regenerated ").append(procedureName).newLine();
-      apply.append("-- changes: ");
-      for (int i = 0; i < comments.size(); i++) {
-        if (i > 0) {
-          apply.append(", ");
-        }
-        apply.append(comments.get(i));
-      }
-      apply.newLine();
+      apply.append("-- changes: ").append(update.description()).newLine();
     }
 
+    addFunction(apply, procedureName, historyTable, includedColumns);
+
+
+    if (update != null) {
+      // put a reverted version into the rollback buffer
+      update.toRevertedColumns(includedColumns);
+
+      DdlBuffer rollback = writer.rollback();
+      rollback.append("-- Revert regenerated ").append(procedureName).newLine();
+      rollback.append("-- revert changes: ").append(update.description()).newLine();
+
+      addFunction(rollback, procedureName, historyTable, includedColumns);
+    }
+  }
+
+  private void addFunction(DdlBuffer apply, String procedureName, String historyTable, List<String> includedColumns) throws IOException {
     apply
         .append("create or replace function ").append(procedureName).append("() returns trigger as $$").newLine()
         .append("begin").newLine();
@@ -184,13 +197,13 @@ public class PostgresHistoryDdl implements PlatformHistoryDdl {
         .append("    return new;").newLine().newLine();
     apply
         .append("  elsif (TG_OP = 'UPDATE') then").newLine();
-    appendInsertIntoHistory(apply, table);
+    appendInsertIntoHistory(apply, historyTable, includedColumns);
     apply
         .append("    NEW.").append(sysPeriod).append(" = tstzrange(CURRENT_TIMESTAMP,null);").newLine()
         .append("    return new;").newLine().newLine();
     apply
         .append("  elsif (TG_OP = 'DELETE') then").newLine();
-    appendInsertIntoHistory(apply, table);
+    appendInsertIntoHistory(apply, historyTable, includedColumns);
     apply
         .append("    return old;").newLine().newLine();
 
@@ -202,29 +215,39 @@ public class PostgresHistoryDdl implements PlatformHistoryDdl {
     apply.end();
   }
 
-  protected void appendInsertIntoHistory(DdlBuffer buffer, MTable table) throws IOException {
-
-    String historyTable = historyTableName(table.getName());
+  protected void appendInsertIntoHistory(DdlBuffer buffer, String historyTable, List<String> columns) throws IOException {
 
     buffer.append("    insert into ").append(historyTable).append(" (").append(sysPeriod).append(",");
-    appendColumnNames(buffer, table, "");
+    appendColumnNames(buffer, columns, "");
     buffer.append(") values (tstzrange(lower(OLD.").append(sysPeriod).append("), CURRENT_TIMESTAMP), ");
-    appendColumnNames(buffer, table, "OLD.");
+    appendColumnNames(buffer, columns, "OLD.");
     buffer.append(");").newLine();
   }
 
-  protected void appendColumnNames(DdlBuffer buffer, MTable table, String columnPrefix) throws IOException {
+  protected void appendColumnNames(DdlBuffer buffer, List<String> columns, String columnPrefix) throws IOException {
 
-    Collection<MColumn> columns = table.getColumns().values();
-    int i = 0;
-    for (MColumn column : columns) {
-      if (!column.isHistoryExclude()) {
-        if (++i > 1) {
-          buffer.append(", ");
-        }
-        buffer.append(columnPrefix);
-        buffer.append(column.getName());
+    for (int i=0; i< columns.size(); i++) {
+      if (i > 0) {
+        buffer.append(", ");
       }
+      buffer.append(columnPrefix);
+      buffer.append(columns.get(i));
     }
   }
+
+  /**
+   * Return the list of included columns in order.
+   */
+  protected List<String> includedColumnNames(MTable table) throws IOException {
+
+    Collection<MColumn> columns = table.getColumns().values();
+    List<String> includedColumns = new ArrayList<String>(columns.size());
+
+    for (MColumn column : columns) {
+      if (!column.isHistoryExclude()) {
+        includedColumns.add(column.getName());
+      }
+    }
+    return includedColumns;
+  }
 }

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

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

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

@@ -278,8 +278,8 @@ public class MTable {
     return withHistory;
   }
 
-  public void setWithHistory() {
-    withHistory = true;
+  public void setWithHistory(boolean withHistory) {
+    this.withHistory = withHistory;
   }
 
   public Map<String, MColumn> getColumns() {

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

@@ -90,7 +90,7 @@ public class MigrationModel {
 
     Migration migration = MigrationXmlReader.readMaybe(path);
     if (migration == null) {
-      logger.info("... no migration at path:{}", path);
+      logger.debug("... no migration at path:{}", path);
       return false;
     }
     readVersions.add(version);

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

@@ -1,11 +1,13 @@
 package com.avaje.ebean.dbmigration.model;
 
 import com.avaje.ebean.dbmigration.migration.AddColumn;
+import com.avaje.ebean.dbmigration.migration.AddHistoryTable;
 import com.avaje.ebean.dbmigration.migration.AlterColumn;
 import com.avaje.ebean.dbmigration.migration.ChangeSet;
 import com.avaje.ebean.dbmigration.migration.CreateIndex;
 import com.avaje.ebean.dbmigration.migration.CreateTable;
 import com.avaje.ebean.dbmigration.migration.DropColumn;
+import com.avaje.ebean.dbmigration.migration.DropHistoryTable;
 import com.avaje.ebean.dbmigration.migration.DropIndex;
 import com.avaje.ebean.dbmigration.migration.DropTable;
 import com.avaje.ebean.dbmigration.migration.Migration;
@@ -96,10 +98,38 @@ public class ModelContainer {
         applyChange((CreateIndex) change);
       } else if (change instanceof DropIndex) {
         applyChange((DropIndex) change);
+      } else if (change instanceof AddHistoryTable) {
+        applyChange((AddHistoryTable) change);
+      } else if (change instanceof DropHistoryTable) {
+        applyChange((DropHistoryTable) change);
       }
     }
   }
 
+  /**
+   * Set the withHistory flag on the associated base table.
+   */
+  private void applyChange(AddHistoryTable change) {
+
+    MTable table = tables.get(change.getBaseTable());
+    if (table == null) {
+      throw new IllegalStateException("Table [" + change.getBaseTable() + "] does not exist in model?");
+    }
+    table.setWithHistory(true);
+  }
+
+  /**
+   * Unset the withHistory flag on the associated base table.
+   */
+  private void applyChange(DropHistoryTable change) {
+
+    MTable table = tables.get(change.getBaseTable());
+    if (table == null) {
+      throw new IllegalStateException("Table [" + change.getBaseTable() + "] does not exist in model?");
+    }
+    table.setWithHistory(false);
+  }
+
   /**
    * Apply a CreateTable change to the model.
    */

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

@@ -29,18 +29,13 @@ public class ModelDiff {
   /**
    * List of 'create' type changes.
    */
-  private final List<Object> createChanges = new ArrayList<Object>();
+  private final List<Object> applyChanges = new ArrayList<Object>();
 
   /**
    * List of 'drop' type changes. Expected to be placed into a separate DDL script.
    */
   private final List<Object> dropChanges = new ArrayList<Object>();
 
-  /**
-   * List of 'drop' type changes. Expected to be placed into a separate DDL script.
-   */
-  private final List<Object> dropHistoryChanges = new ArrayList<Object>();
-
   /**
    * Construct with a base model.
    */
@@ -77,10 +72,10 @@ public class ModelDiff {
   }
 
   /**
-   * Return the list of 'create' changes.
+   * Return the list of 'apply' changes.
    */
-  public List<Object> getCreateChanges() {
-    return createChanges;
+  public List<Object> getApplyChanges() {
+    return applyChanges;
   }
 
   /**
@@ -95,10 +90,10 @@ public class ModelDiff {
    */
   public ChangeSet getApplyChangeSet() {
     // put the changes into a ChangeSet
-    ChangeSet createChangeSet = new ChangeSet();
-    createChangeSet.setType(ChangeSetType.APPLY);
-    createChangeSet.getChangeSetChildren().addAll(createChanges);
-    return createChangeSet;
+    ChangeSet applyChangeSet = new ChangeSet();
+    applyChangeSet.setType(ChangeSetType.APPLY);
+    applyChangeSet.getChangeSetChildren().addAll(applyChanges);
+    return applyChangeSet;
   }
 
   /**
@@ -162,7 +157,7 @@ public class ModelDiff {
    * Add CreateTable to the 'apply' changes.
    */
   protected void addNewTable(MTable newTable) {
-    createChanges.add(newTable.createTable());
+    applyChanges.add(newTable.createTable());
   }
 
   /**
@@ -185,14 +180,14 @@ public class ModelDiff {
    * Add the AlterColumn to the 'apply' changes.
    */
   public void addAlterColumn(AlterColumn alterColumn) {
-    createChanges.add(alterColumn);
+    applyChanges.add(alterColumn);
   }
 
   /**
    * Add the AlterColumn to the 'apply' changes.
    */
   public void addAddColumn(AddColumn addColumn) {
-    createChanges.add(addColumn);
+    applyChanges.add(addColumn);
   }
 
   /**
@@ -206,27 +201,27 @@ public class ModelDiff {
    * Add the AddHistoryTable to apply changes.
    */
   public void addAddHistoryTable(AddHistoryTable addHistoryTable) {
-    createChanges.add(addHistoryTable);
+    applyChanges.add(addHistoryTable);
   }
 
   /**
    * Add the DropHistoryTable to the 'drop history' changes.
    */
   public void addDropHistoryTable(DropHistoryTable dropHistoryTable) {
-    dropHistoryChanges.add(dropHistoryTable);
+    dropChanges.add(dropHistoryTable);
   }
 
   /**
    * Add the DropIndex to the 'apply' changes.
    */
   public void addDropIndex(DropIndex dropIndex) {
-    createChanges.add(dropIndex);
+    applyChanges.add(dropIndex);
   }
 
   /**
    * Add the CreateIndex to the 'apply' changes.
    */
   public void addCreateIndex(CreateIndex createIndex) {
-    createChanges.add(createIndex);
+    applyChanges.add(createIndex);
   }
 }

src/main/java/com/avaje/ebean/dbmigration/model/build/ModelBuildBeanVisitor.java

@@ -37,7 +37,7 @@ public class ModelBuildBeanVisitor implements BeanVisitor {
 
     MTable table = new MTable(descriptor.getBaseTable());
     if (descriptor.isHistorySupport()) {
-      table.setWithHistory();
+      table.setWithHistory(true);
       BeanProperty whenCreated = descriptor.findWhenCreatedProperty();
       if (whenCreated != null) {
         table.setWhenCreatedColumn(whenCreated.getDbColumn());

src/test/java/com/avaje/ebean/dbmigration/ddlgeneration/platform/HistoryTableUpdateTest.java

@@ -0,0 +1,76 @@
+package com.avaje.ebean.dbmigration.ddlgeneration.platform;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+public class HistoryTableUpdateTest {
+
+  @Test
+  public void testToRevertedColumns_add() throws Exception {
+
+    HistoryTableUpdate upd = new HistoryTableUpdate("mytab");
+    upd.add(HistoryTableUpdate.Change.ADD, "two");
+
+    List<String> current = current();
+    upd.toRevertedColumns(current);
+    assertThat(current).contains("one","three");
+  }
+
+  @Test
+  public void testToRevertedColumns_include() throws Exception {
+
+    HistoryTableUpdate upd = new HistoryTableUpdate("mytab");
+    upd.add(HistoryTableUpdate.Change.INCLUDE, "two");
+
+    List<String> current = current();
+    upd.toRevertedColumns(current);
+    assertThat(current).contains("one","three");
+  }
+
+  @Test
+  public void testToRevertedColumns_drop() throws Exception {
+
+    HistoryTableUpdate upd = new HistoryTableUpdate("mytab");
+    upd.add(HistoryTableUpdate.Change.DROP, "four");
+
+    List<String> current = current();
+    upd.toRevertedColumns(current);
+    assertThat(current).contains("one","two","three","four");
+  }
+
+  @Test
+  public void testToRevertedColumns_exclude() throws Exception {
+
+    HistoryTableUpdate upd = new HistoryTableUpdate("mytab");
+    upd.add(HistoryTableUpdate.Change.EXCLUDE, "four");
+
+    List<String> current = current();
+    upd.toRevertedColumns(current);
+    assertThat(current).contains("one","two","three","four");
+  }
+
+
+  @Test
+  public void testDescription() throws Exception {
+
+    HistoryTableUpdate upd = new HistoryTableUpdate("mytab");
+    upd.add(HistoryTableUpdate.Change.ADD, "two");
+    upd.add(HistoryTableUpdate.Change.DROP, "four");
+
+    assertThat(upd.description()).isEqualTo("add two, drop four");
+  }
+
+  List<String> current() {
+
+    List<String> currentColumns = new ArrayList<String>();
+    currentColumns.add("one");
+    currentColumns.add("two");
+    currentColumns.add("three");
+    return currentColumns;
+  }
+}

src/test/java/com/avaje/ebean/dbmigration/model/MColumnTest.java

@@ -25,7 +25,7 @@ public class MColumnTest {
     ModelDiff diff = diff();
     basic().compare(diff, table, basic());
 
-    assertThat(diff.getCreateChanges()).isEmpty();
+    assertThat(diff.getApplyChanges()).isEmpty();
     assertThat(diff.getDropChanges()).isEmpty();
   }
 
@@ -280,12 +280,12 @@ public class MColumnTest {
 
   @NotNull
   private AlterColumn getAlterColumn(ModelDiff diff) {
-    return (AlterColumn) diff.getCreateChanges().get(0);
+    return (AlterColumn) diff.getApplyChanges().get(0);
   }
 
   private void assertChanges(ModelDiff diff) {
     assertThat(diff.getDropChanges()).isEmpty();
-    assertThat(diff.getCreateChanges()).hasSize(1);
+    assertThat(diff.getApplyChanges()).hasSize(1);
   }
 
 }

src/test/java/com/avaje/ebean/dbmigration/model/MTableTest.java

@@ -56,7 +56,7 @@ public class MTableTest {
     ModelDiff diff = new ModelDiff();
     diff.compareTables(base(), newTable());
 
-    List<Object> createChanges = diff.getCreateChanges();
+    List<Object> createChanges = diff.getApplyChanges();
     assertThat(createChanges).hasSize(1);
     AddColumn addColumn = (AddColumn)createChanges.get(0);
     assertThat(addColumn.getColumn()).extracting("name").contains("comment");
@@ -76,7 +76,7 @@ public class MTableTest {
     ModelDiff diff = new ModelDiff();
     diff.compareTables(base(), newTableAdd2Columns());
 
-    List<Object> createChanges = diff.getCreateChanges();
+    List<Object> createChanges = diff.getApplyChanges();
     assertThat(createChanges).hasSize(1);
 
     AddColumn addColumn = (AddColumn)createChanges.get(0);
@@ -93,7 +93,7 @@ public class MTableTest {
     ModelDiff diff = new ModelDiff();
     diff.compareTables(base(), newTableModifiedColumn());
 
-    List<Object> createChanges = diff.getCreateChanges();
+    List<Object> createChanges = diff.getApplyChanges();
     assertThat(createChanges).hasSize(1);
 
     AlterColumn alterColumn = (AlterColumn)createChanges.get(0);

src/test/resources/ebean.properties

@@ -23,7 +23,7 @@ ebean.autofetch.traceUsageCollection=false
 
 ebean.ddl.generate=true
 ebean.ddl.run=true
-datasource.default=mysql
+datasource.default=h2
 
 ebean.persistBatch=NONE
 ebean.persistBatchOnCascade=ALL