Merge branch '594-db-migrations' into 'maintenance-0.16'

Backport: Migrate schema when opening database

See merge request akwizgran/briar!689

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 {
+}

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 {
+}

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;
 

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;
 

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";
-
 }

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 = 30;
+	// Package access for testing
-	private static final int MIN_SCHEMA_VERSION = 30;
+	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> {
 		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<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);
+		int dataSchemaVersion = s.getInt(SCHEMA_VERSION_KEY, -1);
-		if (schemaVersion == SCHEMA_VERSION) return true;
+		if (dataSchemaVersion == -1) throw new DbException();
-		if (schemaVersion < MIN_SCHEMA_VERSION) return false;
+		if (dataSchemaVersion == CODE_SCHEMA_VERSION) return;
-		int minSchemaVersion = s.getInt(MIN_SCHEMA_VERSION_KEY, -1);
+		if (CODE_SCHEMA_VERSION < dataSchemaVersion)
-		return SCHEMA_VERSION >= minSchemaVersion;
+			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(SCHEMA_VERSION_KEY, version);
-		s.putInt(MIN_SCHEMA_VERSION_KEY, MIN_SCHEMA_VERSION);
 		mergeSettings(txn, s, DB_SETTINGS_NAMESPACE);
 	}
 

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;
+}

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);
+	}
+}

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;
+			}
+		};
+	}
+}