All booleans stored are nullable for consistency (#3747)

This commit is contained in:
David Perez 2024-08-15 11:02:01 -05:00 committed by GitHub
parent 18b58e75f8
commit 2bb921b592
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 23 additions and 33 deletions

View file

@ -60,7 +60,7 @@ interface AuthDiskSource {
* *
* Note: This indicates intent to trust the device, the device may not be trusted yet. * 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 * Stores the boolean indicating that the user has chosen to trust this device for the given

View file

@ -140,10 +140,9 @@ class AuthDiskSourceImpl(
) )
} }
override fun getShouldTrustDevice(userId: String): Boolean = override fun getShouldTrustDevice(
requireNotNull( userId: String,
getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), default = false), ): Boolean? = getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId))
)
override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) { override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) {
putBoolean(SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), shouldTrustDevice) putBoolean(SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), shouldTrustDevice)

View file

@ -17,7 +17,7 @@ class TrustedDeviceManagerImpl(
private val devicesService: DevicesService, private val devicesService: DevicesService,
) : TrustedDeviceManager { ) : TrustedDeviceManager {
override suspend fun trustThisDeviceIfNecessary(userId: String): Result<Boolean> = override suspend fun trustThisDeviceIfNecessary(userId: String): Result<Boolean> =
if (!authDiskSource.getShouldTrustDevice(userId = userId)) { if (authDiskSource.getShouldTrustDevice(userId = userId) != true) {
false.asSuccess() false.asSuccess()
} else { } else {
vaultSdkSource vaultSdkSource

View file

@ -470,7 +470,8 @@ class AuthRepositoryImpl(
userId = userId, userId = userId,
email = account.profile.email, email = account.profile.email,
orgPublicKey = organizationKeys.publicKey, orgPublicKey = organizationKeys.publicKey,
rememberDevice = authDiskSource.getShouldTrustDevice(userId = userId), rememberDevice = authDiskSource
.getShouldTrustDevice(userId = userId) == true,
) )
} }
.flatMap { keys -> .flatMap { keys ->

View file

@ -11,18 +11,15 @@ abstract class BaseDiskSource(
private val sharedPreferences: SharedPreferences, private val sharedPreferences: SharedPreferences,
) { ) {
/** /**
* Gets the [Boolean] for the given [key] from [SharedPreferences], or return the [default] * Gets the [Boolean] for the given [key] from [SharedPreferences], or returns `null` if that
* value if that key is not present. * key is not present.
*/ */
protected fun getBoolean( protected fun getBoolean(key: String): Boolean? =
key: String,
default: Boolean? = null,
): Boolean? =
if (sharedPreferences.contains(key.withBase())) { if (sharedPreferences.contains(key.withBase())) {
sharedPreferences.getBoolean(key.withBase(), false) sharedPreferences.getBoolean(key.withBase(), false)
} else { } else {
// Make sure we can return a null value as a default if necessary // 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 * Gets the [Int] for the given [key] from [SharedPreferences], or returns `null` if that key
* if that key is not present. * is not present.
*/ */
protected fun getInt( protected fun getInt(key: String): Int? =
key: String,
default: Int? = null,
): Int? =
if (sharedPreferences.contains(key.withBase())) { if (sharedPreferences.contains(key.withBase())) {
sharedPreferences.getInt(key.withBase(), 0) sharedPreferences.getInt(key.withBase(), 0)
} else { } else {
// Make sure we can return a null value as a default if necessary // 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 * Gets the [Long] for the given [key] from [SharedPreferences], or returns `null` if that key
* if that key is not present. * is not present.
*/ */
protected fun getLong( protected fun getLong(key: String): Long? =
key: String,
default: Long? = null,
): Long? =
if (sharedPreferences.contains(key.withBase())) { if (sharedPreferences.contains(key.withBase())) {
sharedPreferences.getLong(key.withBase(), 0) sharedPreferences.getLong(key.withBase(), 0)
} else { } else {
// Make sure we can return a null value as a default if necessary // 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( protected fun getString(
key: String, key: String,
default: String? = null, ): String? = sharedPreferences.getString(key.withBase(), null)
): String? = sharedPreferences.getString(key.withBase(), default)
protected fun putString( protected fun putString(
key: String, key: String,

View file

@ -35,7 +35,7 @@ class PushDiskSourceImpl(
} }
override fun getLastPushTokenRegistrationDate(userId: String): ZonedDateTime? { 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) } ?.let { getZoneDateTimeFromBinaryLong(it) }
} }

View file

@ -145,7 +145,7 @@ class AuthDiskSourceTest {
val shouldTrustDeviceKey = "bwPreferencesStorage:shouldTrustDevice_$userId" val shouldTrustDeviceKey = "bwPreferencesStorage:shouldTrustDevice_$userId"
// Shared preferences and the disk source start with the same value. // 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)) assertFalse(fakeSharedPreferences.getBoolean(shouldTrustDeviceKey, false))
// Updating the disk source updates shared preferences // Updating the disk source updates shared preferences
@ -154,7 +154,7 @@ class AuthDiskSourceTest {
// Update SharedPreferences updates the disk source // Update SharedPreferences updates the disk source
fakeSharedPreferences.edit { putBoolean(shouldTrustDeviceKey, false) } fakeSharedPreferences.edit { putBoolean(shouldTrustDeviceKey, false) }
assertFalse(authDiskSource.getShouldTrustDevice(userId = userId)) assertFalse(authDiskSource.getShouldTrustDevice(userId = userId) ?: true)
} }
@Test @Test