From 10cb02f4759d21d5b38ca7f7ab67158ee3c804dc Mon Sep 17 00:00:00 2001 From: Julian Raufelder Date: Mon, 14 Feb 2022 17:10:42 +0100 Subject: [PATCH] Store vault metadata like format in database and refresh while unlock For example, if authentication recovery fails, we load the vault from the database, which may result in a vault format -1 before this commit. We try to avoid this by overwriting the state after the authentication succeeds again, but we can't always avoid the vault format being -1 if, for example, during authentication the user changes activity, so we now store these fields in the database and update them if they have changed during unlock to prevent these states. Alternatively we could have locked the vault but from a UX point of view it makes no sense that the user has to unlock the vault again just because e.g. the token has to be refreshed. --- data/build.gradle | 2 +- .../data/db/UpgradeDatabaseTest.kt | 25 +++++++++- .../org/cryptomator/data/db/Upgrade10To11.kt | 48 ++++++++++++++++++- .../data/db/entities/VaultEntity.java | 27 ++++++++++- .../data/db/mappers/VaultEntityMapper.java | 4 ++ ...UpdateVaultParameterIfChangedRemotely.java | 29 +++++++++++ .../presenter/VaultListPresenter.kt | 24 +++++++--- .../presenter/VaultListPresenterTest.java | 15 +++--- 8 files changed, 157 insertions(+), 17 deletions(-) create mode 100644 domain/src/main/java/org/cryptomator/domain/usecases/vault/UpdateVaultParameterIfChangedRemotely.java diff --git a/data/build.gradle b/data/build.gradle index 6bfed543..5284ae6b 100644 --- a/data/build.gradle +++ b/data/build.gradle @@ -70,7 +70,7 @@ android { } packagingOptions { resources { - excludes += ['META-INF/DEPENDENCIES', 'META-INF/NOTICE.md'] + excludes += ['META-INF/DEPENDENCIES', 'META-INF/NOTICE.md', 'META-INF/INDEX.LIST'] } } diff --git a/data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt b/data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt index 108b31a8..66589972 100644 --- a/data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt +++ b/data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt @@ -504,12 +504,23 @@ class UpgradeDatabaseTest { Assert.assertThat(it.count, CoreMatchers.`is`(1)) } + Sql.query("VAULT_ENTITY").executeOn(db).use { + it.moveToFirst() + Assert.assertThat(it.getString(it.getColumnIndex("FOLDER_CLOUD_ID")), CoreMatchers.`is`("3")) + Assert.assertThat(it.getString(it.getColumnIndex("FOLDER_PATH")), CoreMatchers.`is`("path")) + Assert.assertThat(it.getString(it.getColumnIndex("FOLDER_NAME")), CoreMatchers.`is`("name")) + Assert.assertThat(it.getString(it.getColumnIndex("CLOUD_TYPE")), CoreMatchers.`is`(CloudType.ONEDRIVE.name)) + Assert.assertThat(it.getString(it.getColumnIndex("PASSWORD")), CoreMatchers.`is`("password")) + Assert.assertThat(it.getString(it.getColumnIndex("POSITION")), CoreMatchers.`is`("10")) + Assert.assertThat(it.getString(it.getColumnIndex("FORMAT")), CoreMatchers.`is`("8")) + Assert.assertThat(it.getString(it.getColumnIndex("SHORTENING_THRESHOLD")), CoreMatchers.`is`("220")) + } + Sql.query("CLOUD_ENTITY").executeOn(db).use { Assert.assertThat(it.count, CoreMatchers.`is`(2)) } } - @Test fun upgrade10To11UsedOnedriveCloudPreservesCloud() { Upgrade0To1().applyTo(db, 0) @@ -552,6 +563,18 @@ class UpgradeDatabaseTest { Assert.assertThat(it.count, CoreMatchers.`is`(1)) } + Sql.query("VAULT_ENTITY").executeOn(db).use { + it.moveToFirst() + Assert.assertThat(it.getString(it.getColumnIndex("FOLDER_CLOUD_ID")), CoreMatchers.`is`("3")) + Assert.assertThat(it.getString(it.getColumnIndex("FOLDER_PATH")), CoreMatchers.`is`("path")) + Assert.assertThat(it.getString(it.getColumnIndex("FOLDER_NAME")), CoreMatchers.`is`("name")) + Assert.assertThat(it.getString(it.getColumnIndex("CLOUD_TYPE")), CoreMatchers.`is`(CloudType.ONEDRIVE.name)) + Assert.assertThat(it.getString(it.getColumnIndex("PASSWORD")), CoreMatchers.`is`("password")) + Assert.assertThat(it.getString(it.getColumnIndex("POSITION")), CoreMatchers.`is`("10")) + Assert.assertThat(it.getString(it.getColumnIndex("FORMAT")), CoreMatchers.`is`("8")) + Assert.assertThat(it.getString(it.getColumnIndex("SHORTENING_THRESHOLD")), CoreMatchers.`is`("220")) + } + Sql.query("CLOUD_ENTITY").executeOn(db).use { Assert.assertThat(it.count, CoreMatchers.`is`(3)) } diff --git a/data/src/main/java/org/cryptomator/data/db/Upgrade10To11.kt b/data/src/main/java/org/cryptomator/data/db/Upgrade10To11.kt index b143cc3d..263f18fd 100644 --- a/data/src/main/java/org/cryptomator/data/db/Upgrade10To11.kt +++ b/data/src/main/java/org/cryptomator/data/db/Upgrade10To11.kt @@ -6,19 +6,65 @@ import javax.inject.Singleton @Singleton internal class Upgrade10To11 @Inject constructor() : DatabaseUpgrade(10, 11) { - + private val defaultThreshold = 220 + private val defaultVaultFormat = 8 private val onedriveCloudId = 3L override fun internalApplyTo(db: Database, origin: Int) { db.beginTransaction() try { + addFormatAndShorteningToDbEntity(db) + addDefaultFormatAndShorteningThresholdToVaults(db) + deleteOnedriveCloudIfNotSetUp(db) + db.setTransactionSuccessful() } finally { db.endTransaction() } } + private fun addFormatAndShorteningToDbEntity(db: Database) { + Sql.alterTable("VAULT_ENTITY").renameTo("VAULT_ENTITY_OLD").executeOn(db) + Sql.createTable("VAULT_ENTITY") // + .id() // + .optionalInt("FOLDER_CLOUD_ID") // + .optionalText("FOLDER_PATH") // + .optionalText("FOLDER_NAME") // + .requiredText("CLOUD_TYPE") // + .optionalText("PASSWORD") // + .optionalInt("POSITION") // + .optionalInt("FORMAT") // + .optionalInt("SHORTENING_THRESHOLD") // + .foreignKey("FOLDER_CLOUD_ID", "CLOUD_ENTITY", Sql.SqlCreateTableBuilder.ForeignKeyBehaviour.ON_DELETE_SET_NULL) // + .executeOn(db) + + Sql.insertInto("VAULT_ENTITY") // + .select("_id", "FOLDER_CLOUD_ID", "FOLDER_PATH", "FOLDER_NAME", "PASSWORD", "POSITION", "CLOUD_ENTITY.TYPE") // + .columns("_id", "FOLDER_CLOUD_ID", "FOLDER_PATH", "FOLDER_NAME", "PASSWORD", "POSITION", "CLOUD_TYPE") // + .from("VAULT_ENTITY_OLD") // + .join("CLOUD_ENTITY", "VAULT_ENTITY_OLD.FOLDER_CLOUD_ID") // + .executeOn(db) + + Sql.dropIndex("IDX_VAULT_ENTITY_FOLDER_PATH_FOLDER_CLOUD_ID").executeOn(db) + + Sql.createUniqueIndex("IDX_VAULT_ENTITY_FOLDER_PATH_FOLDER_CLOUD_ID") // + .on("VAULT_ENTITY") // + .asc("FOLDER_PATH") // + .asc("FOLDER_CLOUD_ID") // + .executeOn(db) + + Sql.dropTable("VAULT_ENTITY_OLD").executeOn(db) + } + + + private fun addDefaultFormatAndShorteningThresholdToVaults(db: Database) { + Sql.update("VAULT_ENTITY") + .set("FORMAT", Sql.toInteger(defaultVaultFormat)) + .set("SHORTENING_THRESHOLD", Sql.toInteger(defaultThreshold)) + .executeOn(db) + } + private fun deleteOnedriveCloudIfNotSetUp(db: Database) { Sql.deleteFrom("CLOUD_ENTITY") .where("_id", Sql.eq(onedriveCloudId)) diff --git a/data/src/main/java/org/cryptomator/data/db/entities/VaultEntity.java b/data/src/main/java/org/cryptomator/data/db/entities/VaultEntity.java index b8d683fc..9f384b3c 100644 --- a/data/src/main/java/org/cryptomator/data/db/entities/VaultEntity.java +++ b/data/src/main/java/org/cryptomator/data/db/entities/VaultEntity.java @@ -29,6 +29,11 @@ public class VaultEntity extends DatabaseEntity { private String password; private Integer position; + + private Integer format; + + private Integer shorteningThreshold; + /** * Used for active entity operations. */ @@ -42,8 +47,8 @@ public class VaultEntity extends DatabaseEntity { @Generated(hash = 229273163) private transient Long folderCloud__resolvedKey; - @Generated(hash = 825602374) - public VaultEntity(Long id, Long folderCloudId, String folderPath, String folderName, @NotNull String cloudType, String password, Integer position) { + @Generated(hash = 530735379) + public VaultEntity(Long id, Long folderCloudId, String folderPath, String folderName, @NotNull String cloudType, String password, Integer position, Integer format, Integer shorteningThreshold) { this.id = id; this.folderCloudId = folderCloudId; this.folderPath = folderPath; @@ -51,6 +56,8 @@ public class VaultEntity extends DatabaseEntity { this.cloudType = cloudType; this.password = password; this.position = position; + this.format = format; + this.shorteningThreshold = shorteningThreshold; } @Generated(hash = 691253864) @@ -182,6 +189,22 @@ public class VaultEntity extends DatabaseEntity { this.position = position; } + public Integer getFormat() { + return this.format; + } + + public void setFormat(Integer format) { + this.format = format; + } + + public Integer getShorteningThreshold() { + return this.shorteningThreshold; + } + + public void setShorteningThreshold(Integer shorteningThreshold) { + this.shorteningThreshold = shorteningThreshold; + } + /** called by internal mechanisms, do not call yourself. */ @Generated(hash = 674742652) public void __setDaoSession(DaoSession daoSession) { diff --git a/data/src/main/java/org/cryptomator/data/db/mappers/VaultEntityMapper.java b/data/src/main/java/org/cryptomator/data/db/mappers/VaultEntityMapper.java index aabbfff1..2659922d 100644 --- a/data/src/main/java/org/cryptomator/data/db/mappers/VaultEntityMapper.java +++ b/data/src/main/java/org/cryptomator/data/db/mappers/VaultEntityMapper.java @@ -31,6 +31,8 @@ public class VaultEntityMapper extends EntityMapper { .withCloudType(CloudType.valueOf(entity.getCloudType())) // .withSavedPassword(entity.getPassword()) // .withPosition(entity.getPosition()) // + .withFormat(entity.getFormat()) // + .withShorteningThreshold(entity.getShorteningThreshold()) // .build(); } @@ -53,6 +55,8 @@ public class VaultEntityMapper extends EntityMapper { entity.setCloudType(domainObject.getCloudType().name()); entity.setPassword(domainObject.getPassword()); entity.setPosition(domainObject.getPosition()); + entity.setFormat(domainObject.getFormat()); + entity.setShorteningThreshold(domainObject.getShorteningThreshold()); return entity; } } diff --git a/domain/src/main/java/org/cryptomator/domain/usecases/vault/UpdateVaultParameterIfChangedRemotely.java b/domain/src/main/java/org/cryptomator/domain/usecases/vault/UpdateVaultParameterIfChangedRemotely.java new file mode 100644 index 00000000..8b85d17e --- /dev/null +++ b/domain/src/main/java/org/cryptomator/domain/usecases/vault/UpdateVaultParameterIfChangedRemotely.java @@ -0,0 +1,29 @@ +package org.cryptomator.domain.usecases.vault; + +import org.cryptomator.domain.Vault; +import org.cryptomator.domain.exception.BackendException; +import org.cryptomator.domain.repository.VaultRepository; +import org.cryptomator.generator.Parameter; +import org.cryptomator.generator.UseCase; + +@UseCase +class UpdateVaultParameterIfChangedRemotely { + + private final VaultRepository vaultRepository; + private final Vault vault; + + public UpdateVaultParameterIfChangedRemotely(VaultRepository vaultRepository, @Parameter Vault vault) { + this.vaultRepository = vaultRepository; + this.vault = vault; + } + + public Vault execute() throws BackendException { + Vault oldVault = vaultRepository.load(vault.getId()); + if(oldVault.getFormat() == vault.getFormat() && oldVault.getShorteningThreshold() == vault.getShorteningThreshold()) { + return vault; + } else { + return vaultRepository.store(vault); + } + } + +} diff --git a/presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt b/presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt index 4b19ef9d..521083a2 100644 --- a/presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt +++ b/presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt @@ -32,6 +32,7 @@ import org.cryptomator.domain.usecases.vault.LockVaultUseCase import org.cryptomator.domain.usecases.vault.MoveVaultPositionUseCase import org.cryptomator.domain.usecases.vault.RenameVaultUseCase import org.cryptomator.domain.usecases.vault.SaveVaultUseCase +import org.cryptomator.domain.usecases.vault.UpdateVaultParameterIfChangedRemotelyUseCase import org.cryptomator.generator.Callback import org.cryptomator.presentation.BuildConfig import org.cryptomator.presentation.CryptomatorApp @@ -78,6 +79,7 @@ class VaultListPresenter @Inject constructor( // private val licenseCheckUseCase: DoLicenseCheckUseCase, // private val updateCheckUseCase: DoUpdateCheckUseCase, // private val updateUseCase: DoUpdateUseCase, // + private val updateVaultParameterIfChangedRemotelyUseCase: UpdateVaultParameterIfChangedRemotelyUseCase, // private val networkConnectionCheck: NetworkConnectionCheck, // private val fileUtil: FileUtil, // private val authenticationExceptionHandler: AuthenticationExceptionHandler, // @@ -115,7 +117,7 @@ class VaultListPresenter @Inject constructor( // checkLicense() - if(sharedPreferencesHandler.usePhotoUpload()) { + if (sharedPreferencesHandler.usePhotoUpload()) { checkLocalStoragePermissionRegardingAutoUpload() } } @@ -399,16 +401,25 @@ class VaultListPresenter @Inject constructor( // @Callback fun vaultUnlockedVaultList(result: ActivityResult) { val cloud = result.intent().getSerializableExtra(SINGLE_RESULT) as Cloud - navigateToVaultContent(cloud) + getRootFolderOf(cloud) } - private fun navigateToVaultContent(cloud: Cloud) { + private fun getRootFolderOf(cloud: Cloud) { getRootFolderUseCase // .withCloud(cloud) // .run(object : DefaultResultHandler() { override fun onSuccess(folder: CloudFolder) { - val cryptoCloud = (folder.cloud as CryptoCloud) - val vault = cryptoCloud.vault + navigateToVaultContent(folder) + } + }) + } + + private fun navigateToVaultContent(folder: CloudFolder) { + val cryptoCloud = (folder.cloud as CryptoCloud) + updateVaultParameterIfChangedRemotelyUseCase // + .withVault(cryptoCloud.vault) // + .run(object : DefaultResultHandler() { + override fun onSuccess(vault: Vault) { view?.addOrUpdateVault(VaultModel(vault)) navigateToVaultContent(vault, folder) view?.showProgress(ProgressModel.COMPLETED) @@ -530,7 +541,8 @@ class VaultListPresenter @Inject constructor( // moveVaultPositionUseCase, // licenseCheckUseCase, // updateCheckUseCase, // - updateUseCase + updateUseCase, // + updateVaultParameterIfChangedRemotelyUseCase ) } } diff --git a/presentation/src/test/java/org/cryptomator/presentation/presenter/VaultListPresenterTest.java b/presentation/src/test/java/org/cryptomator/presentation/presenter/VaultListPresenterTest.java index 699008f6..874b49ee 100644 --- a/presentation/src/test/java/org/cryptomator/presentation/presenter/VaultListPresenterTest.java +++ b/presentation/src/test/java/org/cryptomator/presentation/presenter/VaultListPresenterTest.java @@ -1,5 +1,11 @@ package org.cryptomator.presentation.presenter; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static java.util.Arrays.asList; + import android.app.Activity; import org.cryptomator.data.util.NetworkConnectionCheck; @@ -20,6 +26,7 @@ import org.cryptomator.domain.usecases.vault.MoveVaultPositionUseCase; import org.cryptomator.domain.usecases.vault.RenameVaultUseCase; import org.cryptomator.domain.usecases.vault.SaveVaultUseCase; import org.cryptomator.domain.usecases.vault.UnlockToken; +import org.cryptomator.domain.usecases.vault.UpdateVaultParameterIfChangedRemotelyUseCase; import org.cryptomator.presentation.exception.ExceptionHandlers; import org.cryptomator.presentation.model.VaultModel; import org.cryptomator.presentation.model.mappers.CloudFolderModelMapper; @@ -37,12 +44,6 @@ import org.mockito.Mockito; import java.util.Collections; import java.util.List; -import static java.util.Arrays.asList; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - public class VaultListPresenterTest { private static final String A_NEW_VAULT_NAME = "Haribo"; @@ -103,6 +104,7 @@ public class VaultListPresenterTest { private DoLicenseCheckUseCase doLicenceCheckUsecase = Mockito.mock(DoLicenseCheckUseCase.class); private DoUpdateCheckUseCase updateCheckUseCase = Mockito.mock(DoUpdateCheckUseCase.class); private DoUpdateUseCase updateUseCase = Mockito.mock(DoUpdateUseCase.class); + private UpdateVaultParameterIfChangedRemotelyUseCase updateVaultParameterIfChangedRemotelyUseCase = Mockito.mock(UpdateVaultParameterIfChangedRemotelyUseCase.class); private NetworkConnectionCheck networkConnectionCheck = Mockito.mock(NetworkConnectionCheck.class); private FileUtil fileUtil = Mockito.mock(FileUtil.class); private AuthenticationExceptionHandler authenticationExceptionHandler = Mockito.mock(AuthenticationExceptionHandler.class); @@ -125,6 +127,7 @@ public class VaultListPresenterTest { doLicenceCheckUsecase, // updateCheckUseCase, // updateUseCase, // + updateVaultParameterIfChangedRemotelyUseCase, // networkConnectionCheck, // fileUtil, // authenticationExceptionHandler, //