aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jco...@apache.org
Subject aurora git commit: Fix bug when checking to ensure that all db migrations on the classpath have been applied.
Date Fri, 08 Apr 2016 18:48:22 GMT
Repository: aurora
Updated Branches:
  refs/heads/master 5ecded788 -> 57468d221


Fix bug when checking to ensure that all db migrations on the classpath have been applied.

While testing applying the multiple migrations for storing images, I noticed that we were
rolling
one back on startup when no migration should be performed.

It turned out I misunderstood the semantics of the Java8 stream API filter/findFirst combination.

Reviewed at https://reviews.apache.org/r/45933/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/57468d22
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/57468d22
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/57468d22

Branch: refs/heads/master
Commit: 57468d2212c9d61bcd4f10022ac575ae2d036ffe
Parents: 5ecded7
Author: Joshua Cohen <jcohen@apache.org>
Authored: Fri Apr 8 13:48:05 2016 -0500
Committer: Joshua Cohen <jcohen@apache.org>
Committed: Fri Apr 8 13:48:05 2016 -0500

----------------------------------------------------------------------
 config/checkstyle/suppressions.xml              |   2 +-
 .../storage/db/MigrationManagerImpl.java        |   2 +-
 .../storage/db/MigrationManagerImplIT.java      | 101 +++++++++++++------
 .../db/testmigration/V002_TestMigration2.java   |  40 ++++++++
 4 files changed, 114 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/57468d22/config/checkstyle/suppressions.xml
----------------------------------------------------------------------
diff --git a/config/checkstyle/suppressions.xml b/config/checkstyle/suppressions.xml
index 30a32ce..d9fe92a 100644
--- a/config/checkstyle/suppressions.xml
+++ b/config/checkstyle/suppressions.xml
@@ -21,6 +21,6 @@ limitations under the License.
   <!-- Allow use of System.exit() in main. -->
   <suppress files="org/apache/aurora/scheduler/app/SchedulerMain.java"
             checks="RegexpSinglelineJava"/>
-  <suppress files="org/apache/aurora/scheduler/storage/db/testmigration/V001_TestMigration.java"
+  <suppress files="org/apache/aurora/scheduler/storage/db/testmigration/.*"
             checks="TypeName" />
 </suppressions>

http://git-wip-us.apache.org/repos/asf/aurora/blob/57468d22/src/main/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImpl.java
b/src/main/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImpl.java
index e2b8416..ca11e59 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImpl.java
@@ -94,7 +94,7 @@ public class MigrationManagerImpl implements MigrationManager {
       // which includes class in its equality check rather than checking instanceof. Instead
we just
       // find the first element in changes whose id matches our applied change. If it does
not exist
       // then this must be a rollback.
-      if (changes.stream().findFirst().filter(c -> c.getId().equals(change.getId())).isPresent())
{
+      if (changes.stream().anyMatch(c -> c.getId().equals(change.getId()))) {
         LOG.info("Change " + change.getId() + " has been applied, no other downgrades are
"
             + "necessary");
         break;

http://git-wip-us.apache.org/repos/asf/aurora/blob/57468d22/src/test/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImplIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImplIT.java
b/src/test/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImplIT.java
index 5edc2ad..d594bce 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImplIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/db/MigrationManagerImplIT.java
@@ -19,6 +19,7 @@ import java.sql.SQLException;
 import java.util.List;
 import java.util.Optional;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.io.CharStreams;
 import com.google.inject.Injector;
 
@@ -42,33 +43,91 @@ public class MigrationManagerImplIT {
         new DbModule.MigrationManagerModule(migrationLoader));
   }
 
+  /**
+   * Ensure all changes have been applied and their downgrade scripts stored appropriately.
+   *
+   * @param sqlSessionFactory The sql session factory.
+   * @param loader A migration loader.
+   * @throws Exception If the changes were not applied properly.
+   */
+  private void assertMigrationComplete(
+      SqlSessionFactory sqlSessionFactory,
+      MigrationLoader loader) throws Exception {
+
+    try (SqlSession session = sqlSessionFactory.openSession()) {
+      MigrationMapper mapper = session.getMapper(MigrationMapper.class);
+
+      List<MigrationChangelogEntry> appliedChanges = mapper.selectAll();
+
+      for (Change change : loader.getMigrations()) {
+        Optional<MigrationChangelogEntry> appliedChange = appliedChanges
+            .stream()
+            .filter(c -> c.getId().equals(change.getId()))
+            .findFirst();
+
+        assertTrue(appliedChange.isPresent());
+        assertEquals(
+            CharStreams.toString(loader.getScriptReader(change, true /* undo */)),
+            appliedChange.get().getDowngradeScript());
+      }
+
+      // As long as the tables exist, neither of these statements should fail.
+      try (Connection c = session.getConnection()) {
+        try (PreparedStatement select = c.prepareStatement("SELECT * FROM V001_test_table"))
{
+          select.execute();
+        }
+        try (PreparedStatement select = c.prepareStatement("SELECT * FROM V002_test_table2"))
{
+          select.execute();
+        }
+      }
+    }
+  }
+
   @Test
   public void testMigrate() throws Exception {
     MigrationLoader loader = new JavaMigrationLoader(
         "org.apache.aurora.scheduler.storage.db.testmigration");
     Injector injector = createMigrationInjector(loader);
 
+    injector.getInstance(MigrationManager.class).migrate();
+    assertMigrationComplete(injector.getInstance(SqlSessionFactory.class), loader);
+  }
+
+  @Test
+  public void testNoMigrationNecessary() throws Exception {
+    MigrationLoader loader = new JavaMigrationLoader(
+        "org.apache.aurora.scheduler.storage.db.testmigration");
+    Injector injector = createMigrationInjector(loader);
+
     MigrationManager migrationManager = injector.getInstance(MigrationManager.class);
 
     migrationManager.migrate();
 
     SqlSessionFactory sqlSessionFactory = injector.getInstance(SqlSessionFactory.class);
+    assertMigrationComplete(sqlSessionFactory, loader);
+
+    // Run the migration a second time, no changes should be made.
+    migrationManager.migrate();
+    assertMigrationComplete(sqlSessionFactory, loader);
+  }
 
+  private void assertRollbackComplete(SqlSessionFactory sqlSessionFactory) throws Exception
{
     try (SqlSession session = sqlSessionFactory.openSession()) {
       MigrationMapper mapper = session.getMapper(MigrationMapper.class);
-      List<MigrationChangelogEntry> appliedChanges = mapper.selectAll();
 
-      // Ensure all changes have been applied and their downgrade scripts stored appropriately.
-      for (Change change : loader.getMigrations()) {
-        Optional<MigrationChangelogEntry> appliedChange = appliedChanges
-            .stream()
-            .findFirst()
-            .filter(c -> c.getId().equals(change.getId()));
-
-        assertTrue(appliedChange.isPresent());
-        assertEquals(
-            CharStreams.toString(loader.getScriptReader(change, true /* undo */)),
-            appliedChange.get().getDowngradeScript());
+      assertTrue(mapper.selectAll().isEmpty());
+      try (Connection c = session.getConnection()) {
+        for (String table : ImmutableList.of("V001_test_table", "V002_test_table2")) {
+          try (PreparedStatement select = c.prepareStatement("SELECT * FROM " + table)) {
+            select.execute();
+            fail("Select from " + table + " should have failed, the table should have been
"
+                + "dropped.");
+          } catch (SQLException e) {
+            // This exception is expected.
+            assertTrue(
+                e.getMessage().startsWith("Table \"" + table.toUpperCase() + "\" not found"));
+          }
+        }
       }
     }
   }
@@ -93,22 +152,6 @@ public class MigrationManagerImplIT {
 
     rollbackManager.migrate();
 
-    SqlSessionFactory sqlSessionFactory = rollbackInjector.getInstance(SqlSessionFactory.class);
-
-    try (SqlSession session = sqlSessionFactory.openSession()) {
-      MigrationMapper mapper = session.getMapper(MigrationMapper.class);
-
-      assertTrue(mapper.selectAll().isEmpty());
-      try (Connection c = session.getConnection()) {
-        try (PreparedStatement select = c.prepareStatement("SELECT * FROM V001_test_table"))
{
-          select.execute();
-          fail("Select from V001_test_table should have failed, the table should have been
"
-              + "dropped.");
-        } catch (SQLException e) {
-          // This exception is expected.
-          assertTrue(e.getMessage().startsWith("Table \"V001_TEST_TABLE\" not found"));
-        }
-      }
-    }
+    assertRollbackComplete(rollbackInjector.getInstance(SqlSessionFactory.class));
   }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/57468d22/src/test/java/org/apache/aurora/scheduler/storage/db/testmigration/V002_TestMigration2.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/db/testmigration/V002_TestMigration2.java
b/src/test/java/org/apache/aurora/scheduler/storage/db/testmigration/V002_TestMigration2.java
new file mode 100644
index 0000000..621ca4c
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/storage/db/testmigration/V002_TestMigration2.java
@@ -0,0 +1,40 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.db.testmigration;
+
+import java.math.BigDecimal;
+
+import org.apache.ibatis.migration.MigrationScript;
+
+public class V002_TestMigration2 implements MigrationScript {
+  @Override
+  public BigDecimal getId() {
+    return BigDecimal.valueOf(2L);
+  }
+
+  @Override
+  public String getDescription() {
+    return "A second test migration";
+  }
+
+  @Override
+  public String getUpScript() {
+    return "CREATE TABLE V002_test_table2(id int);";
+  }
+
+  @Override
+  public String getDownScript() {
+    return "DROP TABLE V002_test_table2;";
+  }
+}


Mime
View raw message