Merge branch '1367-db-race' into 'master'

Don't infer anything from existence of (possibly empty) DB directory

Closes #1528 and #1367

See merge request briar/briar!1238

bramble-api/src/main/java/org/briarproject/bramble/api/account/AccountManager.java

@@ -13,7 +13,8 @@ public interface AccountManager {
 	 * Returns true if the manager has the database key. This will be false
 	 * before {@link #createAccount(String, String)} or {@link #signIn(String)}
 	 * has been called, and true after {@link #createAccount(String, String)}
-	 * or {@link #signIn(String)} has returned true, until the process exits.
+	 * or {@link #signIn(String)} has returned true, until
+	 * {@link #deleteAccount()} is called or the process exits.
 	 */
 	boolean hasDatabaseKey();
 
@@ -22,25 +23,22 @@ public interface AccountManager {
 	 * before {@link #createAccount(String, String)} or {@link #signIn(String)}
 	 * has been called, and non-null after
 	 * {@link #createAccount(String, String)} or {@link #signIn(String)} has
-	 * returned true, until the process exits.
+	 * returned true, until {@link #deleteAccount()} is called or the process
+	 * exits.
 	 */
 	@Nullable
 	SecretKey getDatabaseKey();
 
 	/**
-	 * Returns true if the encrypted database key can be loaded from disk, and
+	 * Returns true if the encrypted database key can be loaded from disk.
-	 * the database directory exists and is a directory.
 	 */
 	boolean accountExists();
 
 	/**
 	 * Creates an identity with the given name and registers it with the
 	 * {@link IdentityManager}. Creates a database key, encrypts it with the
-	 * given password and stores it on disk.
+	 * given password and stores it on disk. {@link #accountExists()} will
-	 * <p/>
+	 * return true after this method returns true.
-	 * This method does not create the database directory, so
-	 * {@link #accountExists()} will continue to return false until the
-	 * database directory is created.
 	 */
 	boolean createAccount(String name, String password);
 

bramble-api/src/main/java/org/briarproject/bramble/util/IoUtils.java

@@ -117,4 +117,10 @@ public class IoUtils {
 			throw new IOException(e);
 		}
 	}
+
+	public static boolean isNonEmptyDirectory(File f) {
+		if (!f.isDirectory()) return false;
+		File[] children = f.listFiles();
+		return children != null && children.length > 0;
+	}
 }

bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java

@@ -155,8 +155,7 @@ class AccountManagerImpl implements AccountManager {
 	@Override
 	public boolean accountExists() {
 		synchronized (stateChangeLock) {
-			return loadEncryptedDatabaseKey() != null
+			return loadEncryptedDatabaseKey() != null;
-					&& databaseConfig.getDatabaseDirectory().isDirectory();
 		}
 	}
 

bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java

@@ -25,6 +25,7 @@ import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.db.JdbcUtils.tryToClose;
+import static org.briarproject.bramble.util.IoUtils.isNonEmptyDirectory;
 import static org.briarproject.bramble.util.LogUtils.logFileOrDir;
 
 /**
@@ -69,8 +70,9 @@ class H2Database extends JdbcDatabase {
 			LOG.info("Contents of account directory before opening DB:");
 			logFileOrDir(LOG, INFO, dir.getParentFile());
 		}
-		boolean reopen = !dir.mkdirs();
+		boolean reopen = isNonEmptyDirectory(dir);
 		if (LOG.isLoggable(INFO)) LOG.info("Reopening DB: " + reopen);
+		if (!reopen && dir.mkdirs()) LOG.info("Created database directory");
 		super.open("org.h2.Driver", reopen, key, listener);
 		if (LOG.isLoggable(INFO)) {
 			LOG.info("Contents of account directory after opening DB:");

bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java

@@ -20,9 +20,11 @@ import java.util.logging.Logger;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
+import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.db.JdbcUtils.tryToClose;
+import static org.briarproject.bramble.util.IoUtils.isNonEmptyDirectory;
 
 /**
  * Contains all the HSQLDB-specific code for the database.
@@ -64,7 +66,10 @@ class HyperSqlDatabase extends JdbcDatabase {
 	public boolean open(SecretKey key, @Nullable MigrationListener listener)
 			throws DbException {
 		this.key = key;
-		boolean reopen = !config.getDatabaseDirectory().mkdirs();
+		File dir = config.getDatabaseDirectory();
+		boolean reopen = isNonEmptyDirectory(dir);
+		if (LOG.isLoggable(INFO)) LOG.info("Reopening DB: " + reopen);
+		if (!reopen && dir.mkdirs()) LOG.info("Created database directory");
 		super.open("org.hsqldb.jdbc.JDBCDriver", reopen, key, listener);
 		return reopen;
 	}

bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java

@@ -239,55 +239,6 @@ public class AccountManagerImplTest extends BrambleMockTestCase {
 		assertFalse(keyBackupFile.exists());
 	}
 
-	@Test
-	public void testAccountExistsReturnsFalseIfDbDirectoryDoesNotExist()
-			throws Exception {
-		storeDatabaseKey(keyFile, encryptedKeyHex);
-		storeDatabaseKey(keyBackupFile, encryptedKeyHex);
-
-		assertFalse(dbDir.exists());
-
-		assertFalse(accountManager.accountExists());
-
-		assertEquals(encryptedKeyHex, loadDatabaseKey(keyFile));
-		assertEquals(encryptedKeyHex, loadDatabaseKey(keyBackupFile));
-		assertFalse(dbDir.exists());
-	}
-
-	@Test
-	public void testAccountExistsReturnsFalseIfDbDirectoryIsNotDirectory()
-			throws Exception {
-		storeDatabaseKey(keyFile, encryptedKeyHex);
-		storeDatabaseKey(keyBackupFile, encryptedKeyHex);
-
-		assertTrue(dbDir.createNewFile());
-		assertFalse(dbDir.isDirectory());
-
-		assertFalse(accountManager.accountExists());
-
-		assertEquals(encryptedKeyHex, loadDatabaseKey(keyFile));
-		assertEquals(encryptedKeyHex, loadDatabaseKey(keyBackupFile));
-		assertTrue(dbDir.exists());
-		assertFalse(dbDir.isDirectory());
-	}
-
-	@Test
-	public void testAccountExistsReturnsTrueIfDbDirectoryIsDirectory()
-			throws Exception {
-		storeDatabaseKey(keyFile, encryptedKeyHex);
-		storeDatabaseKey(keyBackupFile, encryptedKeyHex);
-
-		assertTrue(dbDir.mkdirs());
-		assertTrue(dbDir.isDirectory());
-
-		assertTrue(accountManager.accountExists());
-
-		assertEquals(encryptedKeyHex, loadDatabaseKey(keyFile));
-		assertEquals(encryptedKeyHex, loadDatabaseKey(keyBackupFile));
-		assertTrue(dbDir.exists());
-		assertTrue(dbDir.isDirectory());
-	}
-
 	@Test
 	public void testCreateAccountStoresDbKey() throws Exception {
 		context.checking(new Expectations() {{