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 000000000..001f28f13 --- /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 000000000..9d3e16d33 --- /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 f12a0680b..ff6ef3885 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 93e7bf8c6..71b351452 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 { /** * 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 080a8ce8e..707234b1f 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 8b8c3562a..9aec96283 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 { - private static final int SCHEMA_VERSION = 30; - private static final int MIN_SCHEMA_VERSION = 30; + // Package access for testing + static final int CODE_SCHEMA_VERSION = 30; private static final String CREATE_SETTINGS = "CREATE TABLE settings" @@ -295,10 +297,10 @@ abstract class JdbcDatabase implements Database { 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); @@ -308,19 +310,49 @@ abstract class JdbcDatabase implements Database { } } - 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 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> 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 000000000..56b04effe --- /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 { + + /** + * 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 000000000..203f4dfd2 --- /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 migration = + context.mock(Migration.class, "migration"); + @SuppressWarnings("unchecked") + private final Migration migration1 = + context.mock(Migration.class, "migration1"); + + protected final DatabaseConfig config = + new TestDatabaseConfig(testDir, 1024 * 1024); + protected final Clock clock = new SystemClock(); + + abstract Database createDatabase( + List> migrations) throws Exception; + + @Before + public void setUp() { + assertTrue(testDir.mkdirs()); + } + + @After + public void tearDown() { + TestUtils.deleteTestDirectory(testDir); + } + + @Test + public void testDoesNotRunMigrationsWhenCreatingDatabase() + throws Exception { + Database 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 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 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 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 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 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 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 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 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 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 000000000..6a868fd42 --- /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 createDatabase(List> migrations) + throws Exception { + return new H2Database(config, clock) { + @Override + List> getMigrations() { + return migrations; + } + }; + } +}