diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooNewException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooNewException.java new file mode 100644 index 0000000000000000000000000000000000000000..001f28f136807edf3902e6e71dd5366d05bebb18 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooNewException.java @@ -0,0 +1,7 @@ +package org.briarproject.bramble.api.db; + +/** + * Thrown when the database uses a newer schema than the current code. + */ +public class DataTooNewException extends DbException { +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooOldException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooOldException.java new file mode 100644 index 0000000000000000000000000000000000000000..9d3e16d33b9cba7f09d983bfb10654a2d700d31f --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooOldException.java @@ -0,0 +1,8 @@ +package org.briarproject.bramble.api.db; + +/** + * Thrown when the database uses an older schema than the current code and + * cannot be migrated. + */ +public class DataTooOldException extends DbException { +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java index f12a0680bbe2164e7bfdcc345039e7bdbcfe804c..ff6ef3885b8835e063be8fe2542dde15b8d77fdf 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java @@ -37,6 +37,11 @@ public interface DatabaseComponent { /** * Opens the database and returns true if the database already existed. + * + * @throws DataTooNewException if the data uses a newer schema than the + * current code + * @throws DataTooOldException if the data uses an older schema than the + * current code and cannot be migrated */ boolean open() throws DbException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java index 93e7bf8c685f5bea0fc4fb4127064161c80ee05b..71b351452eeff6224f68a912b2a0211dbd2f28fe 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java @@ -2,6 +2,8 @@ package org.briarproject.bramble.db; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.db.DataTooNewException; +import org.briarproject.bramble.api.db.DataTooOldException; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Metadata; import org.briarproject.bramble.api.identity.Author; @@ -37,6 +39,11 @@ interface Database<T> { /** * Opens the database and returns true if the database already existed. + * + * @throws DataTooNewException if the data uses a newer schema than the + * current code + * @throws DataTooOldException if the data uses an older schema than the + * current code and cannot be migrated */ boolean open() throws DbException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseConstants.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseConstants.java index 080a8ce8e28e0a9c8f4d232ed2da3b89c89004d1..707234b1ffa1c7272f649268e0cc9ff781523bc3 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseConstants.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseConstants.java @@ -23,10 +23,4 @@ interface DatabaseConstants { */ String SCHEMA_VERSION_KEY = "schemaVersion"; - /** - * The {@link Settings} key under which the minimum supported database - * schema version is stored. - */ - String MIN_SCHEMA_VERSION_KEY = "minSchemaVersion"; - } 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 eaf23f2aa24904c784a32c9da81fd01dbc6aa674..47ac1afdbca1372f316f95afe1c12b2ced81189c 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 @@ -3,6 +3,8 @@ package org.briarproject.bramble.db; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.db.DataTooNewException; +import org.briarproject.bramble.api.db.DataTooOldException; import org.briarproject.bramble.api.db.DbClosedException; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Metadata; @@ -47,6 +49,7 @@ import java.util.logging.Logger; import javax.annotation.Nullable; +import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.api.db.Metadata.REMOVE; import static org.briarproject.bramble.api.sync.Group.Visibility.INVISIBLE; @@ -57,7 +60,6 @@ import static org.briarproject.bramble.api.sync.ValidationManager.State.INVALID; import static org.briarproject.bramble.api.sync.ValidationManager.State.PENDING; import static org.briarproject.bramble.api.sync.ValidationManager.State.UNKNOWN; import static org.briarproject.bramble.db.DatabaseConstants.DB_SETTINGS_NAMESPACE; -import static org.briarproject.bramble.db.DatabaseConstants.MIN_SCHEMA_VERSION_KEY; import static org.briarproject.bramble.db.DatabaseConstants.SCHEMA_VERSION_KEY; import static org.briarproject.bramble.db.ExponentialBackoff.calculateExpiry; @@ -68,8 +70,8 @@ import static org.briarproject.bramble.db.ExponentialBackoff.calculateExpiry; @NotNullByDefault abstract class JdbcDatabase implements Database<Connection> { - private static final int SCHEMA_VERSION = 33; - private static final int MIN_SCHEMA_VERSION = 33; + // Package access for testing + static final int CODE_SCHEMA_VERSION = 33; private static final String CREATE_SETTINGS = "CREATE TABLE settings" @@ -292,10 +294,10 @@ abstract class JdbcDatabase implements Database<Connection> { Connection txn = startTransaction(); try { if (reopen) { - if (!checkSchemaVersion(txn)) throw new DbException(); + checkSchemaVersion(txn); } else { createTables(txn); - storeSchemaVersion(txn); + storeSchemaVersion(txn, CODE_SCHEMA_VERSION); } createIndexes(txn); commitTransaction(txn); @@ -305,19 +307,49 @@ abstract class JdbcDatabase implements Database<Connection> { } } - private boolean checkSchemaVersion(Connection txn) throws DbException { + /** + * Compares the schema version stored in the database with the schema + * version used by the current code and applies any suitable migrations to + * the data if necessary. + * + * @throws DataTooNewException if the data uses a newer schema than the + * current code + * @throws DataTooOldException if the data uses an older schema than the + * current code and cannot be migrated + */ + private void checkSchemaVersion(Connection txn) throws DbException { Settings s = getSettings(txn, DB_SETTINGS_NAMESPACE); - int schemaVersion = s.getInt(SCHEMA_VERSION_KEY, -1); - if (schemaVersion == SCHEMA_VERSION) return true; - if (schemaVersion < MIN_SCHEMA_VERSION) return false; - int minSchemaVersion = s.getInt(MIN_SCHEMA_VERSION_KEY, -1); - return SCHEMA_VERSION >= minSchemaVersion; + int dataSchemaVersion = s.getInt(SCHEMA_VERSION_KEY, -1); + if (dataSchemaVersion == -1) throw new DbException(); + if (dataSchemaVersion == CODE_SCHEMA_VERSION) return; + if (CODE_SCHEMA_VERSION < dataSchemaVersion) + throw new DataTooNewException(); + // Apply any suitable migrations in order + for (Migration<Connection> m : getMigrations()) { + int start = m.getStartVersion(), end = m.getEndVersion(); + 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, end); + dataSchemaVersion = end; + } + } + if (dataSchemaVersion != CODE_SCHEMA_VERSION) + throw new DataTooOldException(); } - private void storeSchemaVersion(Connection txn) throws DbException { + // Package access for testing + List<Migration<Connection>> getMigrations() { + return Collections.emptyList(); + } + + private void storeSchemaVersion(Connection txn, int version) + throws DbException { Settings s = new Settings(); - s.putInt(SCHEMA_VERSION_KEY, SCHEMA_VERSION); - s.putInt(MIN_SCHEMA_VERSION_KEY, MIN_SCHEMA_VERSION); + s.putInt(SCHEMA_VERSION_KEY, version); mergeSettings(txn, s, DB_SETTINGS_NAMESPACE); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Migration.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Migration.java new file mode 100644 index 0000000000000000000000000000000000000000..56b04effefaec5ca9e99e5b558d1d596c845c5fd --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Migration.java @@ -0,0 +1,18 @@ +package org.briarproject.bramble.db; + +import org.briarproject.bramble.api.db.DbException; + +interface Migration<T> { + + /** + * Returns the schema version from which this migration starts. + */ + int getStartVersion(); + + /** + * Returns the schema version at which this migration ends. + */ + int getEndVersion(); + + void migrate(T txn) throws DbException; +} 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..203f4dfd21f6fd2b09ceed436f3a10dfca9696b7 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseMigrationTest.java @@ -0,0 +1,233 @@ +package org.briarproject.bramble.db; + +import org.briarproject.bramble.api.db.DataTooNewException; +import org.briarproject.bramble.api.db.DataTooOldException; +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.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; +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, "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( + List<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(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(asList(migration, migration1)); + db.open(); + } + + @Test + public void testDoesNotRunMigrationsIfSchemaVersionsMatch() + throws Exception { + // Open the DB for the first time + 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(asList(migration, migration1)); + assertTrue(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + db.close(); + } + + @Test(expected = DataTooNewException.class) + public void testThrowsExceptionIfDataIsNewerThanCode() throws Exception { + // 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 + 1); + db.close(); + // Reopen the DB - an exception should be thrown + db = createDatabase(asList(migration, migration1)); + db.open(); + } + + @Test(expected = DataTooOldException.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 = DataTooOldException.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 - 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(asList(migration, migration1)); + assertFalse(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + // Override the data schema version + setDataSchemaVersion(db, CODE_SCHEMA_VERSION - 3); + db.close(); + // Reopen the DB - an exception should be thrown + db = createDatabase(asList(migration, migration1)); + db.open(); + } + + @Test + 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 - 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(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 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 - both migrations should be run + db = createDatabase(asList(migration, migration1)); + assertTrue(db.open()); + assertEquals(CODE_SCHEMA_VERSION, getDataSchemaVersion(db)); + 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..6a868fd42fa9ff2098087fbf9aa5b5c9f3d346c9 --- /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.List; + +@NotNullByDefault +public class H2MigrationTest extends DatabaseMigrationTest { + + @Override + Database<Connection> createDatabase(List<Migration<Connection>> migrations) + throws Exception { + return new H2Database(config, clock) { + @Override + 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 new file mode 100644 index 0000000000000000000000000000000000000000..70383ec27f6168cdb680a4fe85b8604d3625344a --- /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.List; + +@NotNullByDefault +public class HyperSqlMigrationTest extends DatabaseMigrationTest { + + @Override + Database<Connection> createDatabase(List<Migration<Connection>> migrations) + throws Exception { + return new HyperSqlDatabase(config, clock) { + @Override + List<Migration<Connection>> getMigrations() { + return migrations; + } + }; + } +}