From 403cfc94f0e2fa603c5ace95e94235b104a2b4cf Mon Sep 17 00:00:00 2001 From: Caleb Derosier <125901828+caleb-livefront@users.noreply.github.com> Date: Tue, 9 Apr 2024 14:51:05 -0600 Subject: [PATCH] Clean up generator minimum length implementation (#1244) --- .../feature/generator/GeneratorViewModel.kt | 22 ++++---------- .../feature/generator/GeneratorScreenTest.kt | 4 +-- .../generator/GeneratorViewModelTest.kt | 30 +++++++++---------- 3 files changed, 22 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt index 47e0cc257..8a1eecdb5 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt @@ -300,7 +300,7 @@ class GeneratorViewModel @Inject constructor( is Password -> { val minLength = policy.minLength ?: Password.PASSWORD_LENGTH_SLIDER_MIN val password = Password( - rawLength = max(options.length, minLength), + length = max(options.length, minLength), minLength = minLength, useCapitals = options.hasUppercase || policy.useUpper == true, capitalsEnabled = policy.useUpper != true, @@ -764,14 +764,10 @@ class GeneratorViewModel @Inject constructor( updatePasswordType { currentPasswordType -> currentPasswordType.copy( - rawLength = adjustedLength.coerceIn( - minimumValue = currentPasswordType.minLength, - maximumValue = currentPasswordType.maxLength, - ), + length = max(adjustedLength, currentPasswordType.minimumLength), isUserInteracting = action.isUserInteracting, ) } - updatePasswordLength() } private fun handleToggleCapitalLetters( @@ -1416,11 +1412,10 @@ class GeneratorViewModel @Inject constructor( private fun updatePasswordLength() { updatePasswordType { currentPasswordType -> currentPasswordType.copy( - rawLength = currentPasswordType.length, + length = max(currentPasswordType.length, currentPasswordType.minimumLength), ) } } - private inline fun updatePasswordType( crossinline block: (Password) -> Password, ) { @@ -1741,7 +1736,7 @@ data class GeneratorState( * Represents a standard PASSWORD type, with configurable options for * length, character types, and requirements. * - * @property rawLength The length of the generated password. + * @property length The length of the generated password. * @property minLength The number available on the low end of the slider. * @property maxLength The number available on the high end of the slider. * @property useCapitals Whether to include capital letters. @@ -1766,7 +1761,7 @@ data class GeneratorState( */ @Parcelize data class Password( - private val rawLength: Int = DEFAULT_PASSWORD_LENGTH, + val length: Int = DEFAULT_PASSWORD_LENGTH, val minLength: Int = PASSWORD_LENGTH_SLIDER_MIN, val maxLength: Int = PASSWORD_LENGTH_SLIDER_MAX, val useCapitals: Boolean = true, @@ -1790,13 +1785,6 @@ data class GeneratorState( override val displayStringResId: Int get() = PasscodeTypeOption.PASSWORD.labelRes - /** - * The larger of the user-set length and the minimum length this password can be - * (as determined by which characters are enabled, and how many). - */ - val length: Int - get() = max(rawLength, minimumLength) - companion object { private const val DEFAULT_PASSWORD_LENGTH: Int = 14 private const val MIN_NUMBERS: Int = 1 diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt index c45126c60..4b1f24ea3 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt @@ -375,8 +375,8 @@ class GeneratorScreenTest : BaseComposeTest() { } // This value would be 128 in a real scenario, because length passed here depends on the - // `sliderValue` which is indirectly updated via the call verified above. However, because - // the view model is a mock, the `length` value that `sliderValue` depends on will not + // internal length which is indirectly updated via the call verified above. However, because + // the view model is a mock, the length value that internal value depends on will not // actually get updated from its original value, and thus will be its original value of 14. verify { viewModel.trySendAction( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt index a99587e11..567f692a7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt @@ -148,7 +148,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { initialPasscodeState.copy( selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 14, + length = 14, minLength = 10, maxLength = 128, useCapitals = true, @@ -182,7 +182,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { initialPasscodeState.copy( selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 14, + length = 14, minLength = 5, maxLength = 128, useCapitals = true, @@ -499,7 +499,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { initialPasscodeState.copy( selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 14, + length = 14, minLength = 10, useCapitals = true, capitalsEnabled = false, @@ -821,7 +821,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = newLength, + length = newLength, ), ), ) @@ -863,7 +863,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 5, + length = 5, useCapitals = false, ), ), @@ -882,7 +882,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 5, // 0 uppercase + 1 lowercase + 4 numbers + 0 special chars + length = 5, // 0 uppercase + 1 lowercase + 4 numbers + 0 special chars minNumbers = 4, useCapitals = false, useNumbers = true, @@ -908,7 +908,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars + length = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars minNumbers = 4, useCapitals = true, ), @@ -951,7 +951,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 5, + length = 5, useLowercase = false, ), ), @@ -970,7 +970,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 5, // 1 uppercase + 0 lowercase + 4 numbers + 0 special chars + length = 5, // 1 uppercase + 0 lowercase + 4 numbers + 0 special chars minNumbers = 4, useLowercase = false, useNumbers = true, @@ -996,7 +996,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars + length = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars minNumbers = 4, useLowercase = true, ), @@ -1105,7 +1105,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 5, // the current slider value, which the min doesn't exceed + length = 5, // the current slider value, which the min doesn't exceed minNumbers = minNumbers, useNumbers = false, ), @@ -1129,7 +1129,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 7, // 1 uppercase + 1 lowercase + 5 numbers + 0 special + length = 7, // 1 uppercase + 1 lowercase + 5 numbers + 0 special minNumbers = minNumbers, useNumbers = true, ), @@ -1173,7 +1173,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 5, + length = 5, minSpecial = minSpecial, useSpecialChars = false, ), @@ -1197,7 +1197,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = 8, // 1 uppercase + 1 lowercase + 1 number + 5 special chars + length = 8, // 1 uppercase + 1 lowercase + 1 number + 5 special chars minSpecial = minSpecial, useSpecialChars = true, ), @@ -2098,7 +2098,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { generatedText = generatedText, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - rawLength = length, + length = length, useCapitals = useCapitals, useLowercase = useLowercase, useNumbers = useNumbers,