diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java index 97aff78e98c58dd52799a8d592deb624278bcc14..03d94a3db12b7887e6e6642c0e3a7e489282994c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java @@ -311,7 +311,7 @@ abstract class JdbcDatabase implements Database<Connection> { if (!checkSchemaVersion(txn)) throw new DbException(); } else { createTables(txn); - storeSchemaVersion(txn); + storeSchemaVersion(txn, CODE_SCHEMA_VERSION); } createIndexes(txn); commitTransaction(txn); @@ -327,31 +327,31 @@ abstract class JdbcDatabase implements Database<Connection> { if (dataSchemaVersion == -1) return false; if (dataSchemaVersion == CODE_SCHEMA_VERSION) return true; if (CODE_SCHEMA_VERSION < dataSchemaVersion) return false; - // Do we have a suitable migration? + // Apply any suitable migrations in order for (Migration<Connection> m : getMigrations()) { int start = m.getStartVersion(), end = m.getEndVersion(); - if (start == dataSchemaVersion && end == CODE_SCHEMA_VERSION) { + if (start == dataSchemaVersion) { if (LOG.isLoggable(INFO)) LOG.info("Migrating from schema " + start + " to " + end); // Apply the migration m.migrate(txn); // Store the new schema version - storeSchemaVersion(txn); - return true; + storeSchemaVersion(txn, end); + dataSchemaVersion = end; } } - // No suitable migration - return false; + return dataSchemaVersion == CODE_SCHEMA_VERSION; } // Package access for testing - Collection<Migration<Connection>> getMigrations() { + List<Migration<Connection>> getMigrations() { return Collections.emptyList(); } - private void storeSchemaVersion(Connection txn) throws DbException { + private void storeSchemaVersion(Connection txn, int version) + throws DbException { Settings s = new Settings(); - s.putInt(SCHEMA_VERSION_KEY, CODE_SCHEMA_VERSION); + s.putInt(SCHEMA_VERSION_KEY, version); mergeSettings(txn, s, DB_SETTINGS_NAMESPACE); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseMigrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseMigrationTest.java index 79ff1b792bd853879094a1cc06720ca4c4070646..de6cabe05a520c58fd03796e0dd7dbf1f508d188 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseMigrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseMigrationTest.java @@ -16,8 +16,9 @@ import org.junit.Test; import java.io.File; import java.sql.Connection; -import java.util.Collection; +import java.util.List; +import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.briarproject.bramble.db.DatabaseConstants.DB_SETTINGS_NAMESPACE; @@ -33,14 +34,17 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase { private final File testDir = TestUtils.getTestDirectory(); @SuppressWarnings("unchecked") private final Migration<Connection> migration = - context.mock(Migration.class); + context.mock(Migration.class, "migration"); + @SuppressWarnings("unchecked") + private final Migration<Connection> migration1 = + context.mock(Migration.class, "migration1"); protected final DatabaseConfig config = new TestDatabaseConfig(testDir, 1024 * 1024); protected final Clock clock = new SystemClock(); abstract Database<Connection> createDatabase( - Collection<Migration<Connection>> migrations) throws Exception; + List<Migration<Connection>> migrations) throws Exception; @Before public void setUp() { @@ -65,14 +69,14 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase { public void testThrowsExceptionIfDataSchemaVersionIsMissing() throws Exception { // Open the DB for the first time - Database<Connection> db = createDatabase(singletonList(migration)); + Database<Connection> db = createDatabase(asList(migration, migration1)); assertFalse(db.open()); assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); // Override the data schema version setDataSchemaVersion(db, -1); db.close(); // Reopen the DB - an exception should be thrown - db = createDatabase(singletonList(migration)); + db = createDatabase(asList(migration, migration1)); db.open(); } @@ -80,12 +84,12 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase { public void testDoesNotRunMigrationsIfSchemaVersionsMatch() throws Exception { // Open the DB for the first time - Database<Connection> db = createDatabase(singletonList(migration)); + Database<Connection> db = createDatabase(asList(migration, migration1)); assertFalse(db.open()); assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); db.close(); // Reopen the DB - migrations should not be run - db = createDatabase(singletonList(migration)); + db = createDatabase(asList(migration, migration1)); assertTrue(db.open()); assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); db.close(); @@ -94,14 +98,14 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase { @Test(expected = DbException.class) public void testThrowsExceptionIfDataIsNewerThanCode() throws Exception { // Open the DB for the first time - Database<Connection> db = createDatabase(singletonList(migration)); + Database<Connection> db = createDatabase(asList(migration, migration1)); assertFalse(db.open()); assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); // Override the data schema version setDataSchemaVersion(db, CODE_SCHEMA_VERSION + 1); db.close(); // Reopen the DB - an exception should be thrown - db = createDatabase(singletonList(migration)); + db = createDatabase(asList(migration, migration1)); db.open(); } @@ -126,18 +130,22 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase { oneOf(migration).getStartVersion(); will(returnValue(CODE_SCHEMA_VERSION - 2)); oneOf(migration).getEndVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 1)); + oneOf(migration1).getStartVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 1)); + oneOf(migration1).getEndVersion(); will(returnValue(CODE_SCHEMA_VERSION)); }}); // Open the DB for the first time - Database<Connection> db = createDatabase(singletonList(migration)); + Database<Connection> db = createDatabase(asList(migration, migration1)); assertFalse(db.open()); assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); // Override the data schema version - setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 1); + setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 3); db.close(); // Reopen the DB - an exception should be thrown - db = createDatabase(singletonList(migration)); + db = createDatabase(asList(migration, migration1)); db.open(); } @@ -145,22 +153,62 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase { public void testRunsMigrationIfCodeIsNewerThanDataAndSuitableMigration() throws Exception { context.checking(new Expectations() {{ + // First migration should be run, increasing schema version by 2 oneOf(migration).getStartVersion(); - will(returnValue(CODE_SCHEMA_VERSION - 1)); + will(returnValue(CODE_SCHEMA_VERSION - 2)); oneOf(migration).getEndVersion(); will(returnValue(CODE_SCHEMA_VERSION)); oneOf(migration).migrate(with(any(Connection.class))); + // Second migration is not suitable and should be skipped + oneOf(migration1).getStartVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 1)); + oneOf(migration1).getEndVersion(); + will(returnValue(CODE_SCHEMA_VERSION)); }}); // Open the DB for the first time - Database<Connection> db = createDatabase(singletonList(migration)); + Database<Connection> db = createDatabase(asList(migration, migration1)); assertFalse(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); // Override the data schema version - setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 1); + setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 2); + db.close(); + // Reopen the DB - the first migration should be run + db = createDatabase(asList(migration, migration1)); + assertTrue(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + db.close(); + } + + @Test + public void testRunsMigrationsIfCodeIsNewerThanDataAndSuitableMigrations() + throws Exception { + context.checking(new Expectations() {{ + // First migration should be run, incrementing schema version + oneOf(migration).getStartVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 2)); + oneOf(migration).getEndVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 1)); + oneOf(migration).migrate(with(any(Connection.class))); + // Second migration should be run, incrementing schema version again + oneOf(migration1).getStartVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 1)); + oneOf(migration1).getEndVersion(); + will(returnValue(CODE_SCHEMA_VERSION)); + oneOf(migration1).migrate(with(any(Connection.class))); + }}); + + // Open the DB for the first time + Database<Connection> db = createDatabase(asList(migration, migration1)); + assertFalse(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + // Override the data schema version + setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 2); db.close(); - // Reopen the DB - the migration should be run - db = createDatabase(singletonList(migration)); + // Reopen the DB - both migrations should be run + db = createDatabase(asList(migration, migration1)); assertTrue(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); db.close(); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/H2MigrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/H2MigrationTest.java index 1bf215ad7c4a21486a1528f23eaebe72fab8e817..6a868fd42fa9ff2098087fbf9aa5b5c9f3d346c9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/H2MigrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/H2MigrationTest.java @@ -3,17 +3,17 @@ package org.briarproject.bramble.db; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.sql.Connection; -import java.util.Collection; +import java.util.List; @NotNullByDefault public class H2MigrationTest extends DatabaseMigrationTest { @Override - Database<Connection> createDatabase( - Collection<Migration<Connection>> migrations) throws Exception { + Database<Connection> createDatabase(List<Migration<Connection>> migrations) + throws Exception { return new H2Database(config, clock) { @Override - Collection<Migration<Connection>> getMigrations() { + List<Migration<Connection>> getMigrations() { return migrations; } }; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlMigrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlMigrationTest.java index 63cb8df81f1edbcd0c9a2158102c7c521215396e..70383ec27f6168cdb680a4fe85b8604d3625344a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlMigrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlMigrationTest.java @@ -3,17 +3,17 @@ package org.briarproject.bramble.db; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.sql.Connection; -import java.util.Collection; +import java.util.List; @NotNullByDefault public class HyperSqlMigrationTest extends DatabaseMigrationTest { @Override - Database<Connection> createDatabase( - Collection<Migration<Connection>> migrations) throws Exception { + Database<Connection> createDatabase(List<Migration<Connection>> migrations) + throws Exception { return new HyperSqlDatabase(config, clock) { @Override - Collection<Migration<Connection>> getMigrations() { + List<Migration<Connection>> getMigrations() { return migrations; } };