From 618ab1f1ec9f41c5919c43a110fab10ee6184fbd Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 24 Feb 2020 17:48:54 +0000 Subject: [PATCH 1/2] Don't infer anything from existence of (possibly empty) DB directory. --- .../briarproject/bramble/util/IoUtils.java | 6 +++ .../bramble/account/AccountManagerImpl.java | 3 +- .../briarproject/bramble/db/H2Database.java | 4 +- .../bramble/db/HyperSqlDatabase.java | 7 ++- .../account/AccountManagerImplTest.java | 49 ------------------- 5 files changed, 16 insertions(+), 53 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/util/IoUtils.java b/bramble-api/src/main/java/org/briarproject/bramble/util/IoUtils.java index dea0fb423..d94f289f9 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/util/IoUtils.java +++ b/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; + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java index c89353fa9..c5972d86e 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java +++ b/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 - && databaseConfig.getDatabaseDirectory().isDirectory(); + return loadEncryptedDatabaseKey() != null; } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java index 9c6bbbe23..fe59c35a1 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java +++ b/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:"); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java index 610625a61..aad359498 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java +++ b/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; } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java index 4b6b63e7e..adf8f131f 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java +++ b/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() {{ From 77d037f06101d0ab1bc77fdd4a4eaf0315e56056 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 10 Mar 2020 11:27:54 +0000 Subject: [PATCH 2/2] Update javadocs. --- .../bramble/api/account/AccountManager.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/account/AccountManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/account/AccountManager.java index 2e1b5a951..dbca14e0b 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/account/AccountManager.java +++ b/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 - * the database directory exists and is a directory. + * Returns true if the encrypted database key can be loaded from disk. */ 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. - *

- * This method does not create the database directory, so - * {@link #accountExists()} will continue to return false until the - * database directory is created. + * given password and stores it on disk. {@link #accountExists()} will + * return true after this method returns true. */ boolean createAccount(String name, String password);