From 2bb921b5926d0fcf15a39662e5f8205d9a557503 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 15 Aug 2024 11:02:01 -0500 Subject: [PATCH] All booleans stored are nullable for consistency (#3747) --- .../auth/datasource/disk/AuthDiskSource.kt | 2 +- .../datasource/disk/AuthDiskSourceImpl.kt | 7 ++-- .../auth/manager/TrustedDeviceManagerImpl.kt | 2 +- .../auth/repository/AuthRepositoryImpl.kt | 3 +- .../datasource/disk/BaseDiskSource.kt | 36 +++++++------------ .../datasource/disk/PushDiskSourceImpl.kt | 2 +- .../datasource/disk/AuthDiskSourceTest.kt | 4 +-- 7 files changed, 23 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt index 688df35e0..e11a09db1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt @@ -60,7 +60,7 @@ interface AuthDiskSource { * * Note: This indicates intent to trust the device, the device may not be trusted yet. */ - fun getShouldTrustDevice(userId: String): Boolean + fun getShouldTrustDevice(userId: String): Boolean? /** * Stores the boolean indicating that the user has chosen to trust this device for the given diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt index ba0edf00a..1b75a2dde 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt @@ -140,10 +140,9 @@ class AuthDiskSourceImpl( ) } - override fun getShouldTrustDevice(userId: String): Boolean = - requireNotNull( - getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), default = false), - ) + override fun getShouldTrustDevice( + userId: String, + ): Boolean? = getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId)) override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) { putBoolean(SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), shouldTrustDevice) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/TrustedDeviceManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/TrustedDeviceManagerImpl.kt index 848c859f9..2cbb49a7e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/TrustedDeviceManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/TrustedDeviceManagerImpl.kt @@ -17,7 +17,7 @@ class TrustedDeviceManagerImpl( private val devicesService: DevicesService, ) : TrustedDeviceManager { override suspend fun trustThisDeviceIfNecessary(userId: String): Result = - if (!authDiskSource.getShouldTrustDevice(userId = userId)) { + if (authDiskSource.getShouldTrustDevice(userId = userId) != true) { false.asSuccess() } else { vaultSdkSource diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 9f2a35176..3784a4308 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -470,7 +470,8 @@ class AuthRepositoryImpl( userId = userId, email = account.profile.email, orgPublicKey = organizationKeys.publicKey, - rememberDevice = authDiskSource.getShouldTrustDevice(userId = userId), + rememberDevice = authDiskSource + .getShouldTrustDevice(userId = userId) == true, ) } .flatMap { keys -> diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/BaseDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/BaseDiskSource.kt index aadff4468..90f874bcf 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/BaseDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/BaseDiskSource.kt @@ -11,18 +11,15 @@ abstract class BaseDiskSource( private val sharedPreferences: SharedPreferences, ) { /** - * Gets the [Boolean] for the given [key] from [SharedPreferences], or return the [default] - * value if that key is not present. + * Gets the [Boolean] for the given [key] from [SharedPreferences], or returns `null` if that + * key is not present. */ - protected fun getBoolean( - key: String, - default: Boolean? = null, - ): Boolean? = + protected fun getBoolean(key: String): Boolean? = if (sharedPreferences.contains(key.withBase())) { sharedPreferences.getBoolean(key.withBase(), false) } else { // Make sure we can return a null value as a default if necessary - default + null } /** @@ -42,18 +39,15 @@ abstract class BaseDiskSource( } /** - * Gets the [Int] for the given [key] from [SharedPreferences], or return the [default] value - * if that key is not present. + * Gets the [Int] for the given [key] from [SharedPreferences], or returns `null` if that key + * is not present. */ - protected fun getInt( - key: String, - default: Int? = null, - ): Int? = + protected fun getInt(key: String): Int? = if (sharedPreferences.contains(key.withBase())) { sharedPreferences.getInt(key.withBase(), 0) } else { // Make sure we can return a null value as a default if necessary - default + null } /** @@ -73,18 +67,15 @@ abstract class BaseDiskSource( } /** - * Gets the [Long] for the given [key] from [SharedPreferences], or return the [default] value - * if that key is not present. + * Gets the [Long] for the given [key] from [SharedPreferences], or returns `null` if that key + * is not present. */ - protected fun getLong( - key: String, - default: Long? = null, - ): Long? = + protected fun getLong(key: String): Long? = if (sharedPreferences.contains(key.withBase())) { sharedPreferences.getLong(key.withBase(), 0) } else { // Make sure we can return a null value as a default if necessary - default + null } /** @@ -105,8 +96,7 @@ abstract class BaseDiskSource( protected fun getString( key: String, - default: String? = null, - ): String? = sharedPreferences.getString(key.withBase(), default) + ): String? = sharedPreferences.getString(key.withBase(), null) protected fun putString( key: String, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/PushDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/PushDiskSourceImpl.kt index 67fb8b4e4..736e5b36c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/PushDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/PushDiskSourceImpl.kt @@ -35,7 +35,7 @@ class PushDiskSourceImpl( } override fun getLastPushTokenRegistrationDate(userId: String): ZonedDateTime? { - return getLong(LAST_REGISTRATION_DATE_KEY.appendIdentifier(userId), null) + return getLong(LAST_REGISTRATION_DATE_KEY.appendIdentifier(userId)) ?.let { getZoneDateTimeFromBinaryLong(it) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt index adb08a423..eff273e2b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -145,7 +145,7 @@ class AuthDiskSourceTest { val shouldTrustDeviceKey = "bwPreferencesStorage:shouldTrustDevice_$userId" // Shared preferences and the disk source start with the same value. - assertFalse(authDiskSource.getShouldTrustDevice(userId = userId)) + assertNull(authDiskSource.getShouldTrustDevice(userId = userId)) assertFalse(fakeSharedPreferences.getBoolean(shouldTrustDeviceKey, false)) // Updating the disk source updates shared preferences @@ -154,7 +154,7 @@ class AuthDiskSourceTest { // Update SharedPreferences updates the disk source fakeSharedPreferences.edit { putBoolean(shouldTrustDeviceKey, false) } - assertFalse(authDiskSource.getShouldTrustDevice(userId = userId)) + assertFalse(authDiskSource.getShouldTrustDevice(userId = userId) ?: true) } @Test