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 7e6138e48c6365edb65a61e9899a8e6677388b79..97aff78e98c58dd52799a8d592deb624278bcc14 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 @@ -68,7 +68,8 @@ import static org.briarproject.bramble.db.ExponentialBackoff.calculateExpiry; @NotNullByDefault abstract class JdbcDatabase implements Database<Connection> { - private static final int CODE_SCHEMA_VERSION = 33; + // Package access for testing + static final int CODE_SCHEMA_VERSION = 33; private static final String CREATE_SETTINGS = "CREATE TABLE settings" @@ -323,6 +324,7 @@ abstract class JdbcDatabase implements Database<Connection> { private boolean checkSchemaVersion(Connection txn) throws DbException { Settings s = getSettings(txn, DB_SETTINGS_NAMESPACE); int dataSchemaVersion = s.getInt(SCHEMA_VERSION_KEY, -1); + 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? @@ -342,7 +344,8 @@ abstract class JdbcDatabase implements Database<Connection> { return false; } - private Collection<Migration<Connection>> getMigrations() { + // Package access for testing + Collection<Migration<Connection>> getMigrations() { return Collections.emptyList(); } 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 new file mode 100644 index 0000000000000000000000000000000000000000..79ff1b792bd853879094a1cc06720ca4c4070646 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseMigrationTest.java @@ -0,0 +1,183 @@ +package org.briarproject.bramble.db; + +import org.briarproject.bramble.api.db.DatabaseConfig; +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.settings.Settings; +import org.briarproject.bramble.api.system.Clock; +import org.briarproject.bramble.system.SystemClock; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.briarproject.bramble.test.TestDatabaseConfig; +import org.briarproject.bramble.test.TestUtils; +import org.jmock.Expectations; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.sql.Connection; +import java.util.Collection; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.briarproject.bramble.db.DatabaseConstants.DB_SETTINGS_NAMESPACE; +import static org.briarproject.bramble.db.DatabaseConstants.SCHEMA_VERSION_KEY; +import static org.briarproject.bramble.db.JdbcDatabase.CODE_SCHEMA_VERSION; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +@NotNullByDefault +public abstract class DatabaseMigrationTest extends BrambleMockTestCase { + + private final File testDir = TestUtils.getTestDirectory(); + @SuppressWarnings("unchecked") + private final Migration<Connection> migration = + context.mock(Migration.class); + + 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; + + @Before + public void setUp() { + assertTrue(testDir.mkdirs()); + } + + @After + public void tearDown() { + TestUtils.deleteTestDirectory(testDir); + } + + @Test + public void testDoesNotRunMigrationsWhenCreatingDatabase() + throws Exception { + Database<Connection> db = createDatabase(singletonList(migration)); + assertFalse(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + db.close(); + } + + @Test(expected = DbException.class) + public void testThrowsExceptionIfDataSchemaVersionIsMissing() + throws Exception { + // Open the DB for the first time + Database<Connection> db = createDatabase(singletonList(migration)); + 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.open(); + } + + @Test + public void testDoesNotRunMigrationsIfSchemaVersionsMatch() + throws Exception { + // Open the DB for the first time + Database<Connection> db = createDatabase(singletonList(migration)); + assertFalse(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + db.close(); + // Reopen the DB - migrations should not be run + db = createDatabase(singletonList(migration)); + assertTrue(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + db.close(); + } + + @Test(expected = DbException.class) + public void testThrowsExceptionIfDataIsNewerThanCode() throws Exception { + // Open the DB for the first time + Database<Connection> db = createDatabase(singletonList(migration)); + 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.open(); + } + + @Test(expected = DbException.class) + public void testThrowsExceptionIfCodeIsNewerThanDataAndNoMigrations() + throws Exception { + // Open the DB for the first time + Database<Connection> db = createDatabase(emptyList()); + assertFalse(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 1); + db.close(); + // Reopen the DB - an exception should be thrown + db = createDatabase(emptyList()); + db.open(); + } + + @Test(expected = DbException.class) + public void testThrowsExceptionIfCodeIsNewerThanDataAndNoSuitableMigration() + throws Exception { + context.checking(new Expectations() {{ + oneOf(migration).getStartVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 2)); + oneOf(migration).getEndVersion(); + will(returnValue(CODE_SCHEMA_VERSION)); + }}); + + // Open the DB for the first time + Database<Connection> db = createDatabase(singletonList(migration)); + 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.open(); + } + + @Test + public void testRunsMigrationIfCodeIsNewerThanDataAndSuitableMigration() + throws Exception { + context.checking(new Expectations() {{ + oneOf(migration).getStartVersion(); + will(returnValue(CODE_SCHEMA_VERSION - 1)); + oneOf(migration).getEndVersion(); + will(returnValue(CODE_SCHEMA_VERSION)); + oneOf(migration).migrate(with(any(Connection.class))); + }}); + + // Open the DB for the first time + Database<Connection> db = createDatabase(singletonList(migration)); + assertFalse(db.open()); + // Override the data schema version + setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 1); + db.close(); + // Reopen the DB - the migration should be run + db = createDatabase(singletonList(migration)); + assertTrue(db.open()); + db.close(); + } + + private int getDataSchemaVersion(Database<Connection> db) + throws Exception { + Connection txn = db.startTransaction(); + Settings s = db.getSettings(txn, DB_SETTINGS_NAMESPACE); + db.commitTransaction(txn); + return s.getInt(SCHEMA_VERSION_KEY, -1); + } + + private void setDataSchemaVersion(Database<Connection> db, int version) + throws Exception { + Settings s = new Settings(); + s.putInt(SCHEMA_VERSION_KEY, version); + Connection txn = db.startTransaction(); + db.mergeSettings(txn, s, DB_SETTINGS_NAMESPACE); + db.commitTransaction(txn); + } +} 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 new file mode 100644 index 0000000000000000000000000000000000000000..1bf215ad7c4a21486a1528f23eaebe72fab8e817 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/H2MigrationTest.java @@ -0,0 +1,21 @@ +package org.briarproject.bramble.db; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.sql.Connection; +import java.util.Collection; + +@NotNullByDefault +public class H2MigrationTest extends DatabaseMigrationTest { + + @Override + Database<Connection> createDatabase( + Collection<Migration<Connection>> migrations) throws Exception { + return new H2Database(config, clock) { + @Override + Collection<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 new file mode 100644 index 0000000000000000000000000000000000000000..63cb8df81f1edbcd0c9a2158102c7c521215396e --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlMigrationTest.java @@ -0,0 +1,21 @@ +package org.briarproject.bramble.db; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.sql.Connection; +import java.util.Collection; + +@NotNullByDefault +public class HyperSqlMigrationTest extends DatabaseMigrationTest { + + @Override + Database<Connection> createDatabase( + Collection<Migration<Connection>> migrations) throws Exception { + return new HyperSqlDatabase(config, clock) { + @Override + Collection<Migration<Connection>> getMigrations() { + return migrations; + } + }; + } +}