From 8bb08a2af995573413f80e3d228458e2dc675fd6 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Thu, 1 Feb 2018 17:07:54 +0000
Subject: [PATCH] Throw meaningful exceptions for schema errors.

---
 .../bramble/api/db/DataTooNewException.java   |  7 +++++
 .../bramble/api/db/DataTooOldException.java   |  8 ++++++
 .../bramble/api/db/DatabaseComponent.java     |  5 ++++
 .../org/briarproject/bramble/db/Database.java |  7 +++++
 .../briarproject/bramble/db/JdbcDatabase.java | 26 ++++++++++++-------
 .../bramble/db/DatabaseMigrationTest.java     |  8 +++---
 6 files changed, 49 insertions(+), 12 deletions(-)
 create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooNewException.java
 create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/db/DataTooOldException.java

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 0000000000..001f28f136
--- /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 0000000000..9d3e16d33b
--- /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 f12a0680bb..ff6ef3885b 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 93e7bf8c68..71b351452e 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/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java
index 1f22d75de3..de222fa253 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;
@@ -308,7 +310,7 @@ 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, CODE_SCHEMA_VERSION);
@@ -323,16 +325,21 @@ abstract class JdbcDatabase implements Database<Connection> {
 
 	/**
 	 * Compares the schema version stored in the database with the schema
-	 * version used by the current code, applies any suitable migrations to the
-	 * data if necessary, and returns true if the schema now matches the
-	 * current code.
+	 * 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 boolean checkSchemaVersion(Connection txn) throws DbException {
+	private void 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;
+		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();
@@ -346,7 +353,8 @@ abstract class JdbcDatabase implements Database<Connection> {
 				dataSchemaVersion = end;
 			}
 		}
-		return dataSchemaVersion == CODE_SCHEMA_VERSION;
+		if (dataSchemaVersion != CODE_SCHEMA_VERSION)
+			throw new DataTooOldException();
 	}
 
 	// Package access for testing
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 de6cabe05a..203f4dfd21 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
@@ -1,5 +1,7 @@
 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;
@@ -95,7 +97,7 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase {
 		db.close();
 	}
 
-	@Test(expected = DbException.class)
+	@Test(expected = DataTooNewException.class)
 	public void testThrowsExceptionIfDataIsNewerThanCode() throws Exception {
 		// Open the DB for the first time
 		Database<Connection> db = createDatabase(asList(migration, migration1));
@@ -109,7 +111,7 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase {
 		db.open();
 	}
 
-	@Test(expected = DbException.class)
+	@Test(expected = DataTooOldException.class)
 	public void testThrowsExceptionIfCodeIsNewerThanDataAndNoMigrations()
 			throws Exception {
 		// Open the DB for the first time
@@ -123,7 +125,7 @@ public abstract class DatabaseMigrationTest extends BrambleMockTestCase {
 		db.open();
 	}
 
-	@Test(expected = DbException.class)
+	@Test(expected = DataTooOldException.class)
 	public void testThrowsExceptionIfCodeIsNewerThanDataAndNoSuitableMigration()
 			throws Exception {
 		context.checking(new Expectations() {{
-- 
GitLab