Enhanced Biometric Flow

[jumaallan]

Story: https://app.shortcut.com/smileid/story/xxx

Summary

A few sentences/bullet points about the changes

Known Issues

Any shortcomings in your work. This may include corner cases not correctly handled or issues related
to but not within the scope of your PR. Design compromises should be discussed here if they were not
already discussed above.

Test Instructions

Concise test instructions on how to verify that your feature works as intended. This should include
changes to the development environment (if applicable) and all commands needed to run your work.

Screenshot

If applicable (e.g. UI changes), add screenshots to help explain your work.

[prfectionist]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Memory Leak The selfieFile bitmap is loaded but not properly disposed if an error occurs during processing. Consider wrapping bitmap loading in try-catch and ensuring proper cleanup. Race Condition Multiple properties (leftProgress, rightProgress, topProgress) are updated independently without synchronization. Consider using atomic operations or proper synchronization. Resource Cleanup Screen brightness changes may not be properly restored if the activity is destroyed abnormally. Consider adding additional cleanup in activity lifecycle.

[prfectionist]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Maintain suspend modifier on function that likely performs asynchronous operations The submitJob method signature change from suspend to non-suspend could lead to incorrect usage since it likely performs network operations. Consider keeping it as a suspend function. lib/src/main/java/com/smileidentity/SmileID.kt [299-302] @JvmStatic -fun submitJob( +suspend fun submitJob( jobId: String, deleteFilesOnSuccess: Boolean = true, scope: CoroutineScope = CoroutineScope(Dispatchers.IO), Suggestion importance[1-10]: 9 Why: Removing the suspend modifier from an asynchronous operation could lead to blocking calls and poor performance. This is a critical issue that could affect application behavior.	9
	Ensure thread safety when updating shared progress values Consider using atomics or synchronized blocks when updating progress values to ensure thread safety, as these values might be accessed from different threads. lib/src/main/java/com/smileidentity/viewmodel/ActiveLivenessTask.kt [109-111] -private var leftProgress = 0f -private var rightProgress = 0f -private var topProgress = 0f +private var leftProgress = AtomicFloat(0f) +private var rightProgress = AtomicFloat(0f) +private var topProgress = AtomicFloat(0f) Suggestion importance[1-10]: 7 Why: The suggestion addresses a potential thread safety issue in a concurrent environment, which could lead to race conditions when updating progress values.	7
Possible bug	Handle all possible portrait orientation values to prevent potential bugs The isPortraitOrientation function assumes portrait is only at 270 degrees, but it could also be at 90 degrees. Consider checking for both orientations. lib/src/main/java/com/smileidentity/viewmodel/SmartSelfieEnhancedViewModel.kt [184] -private fun isPortraitOrientation(degrees: Int): Boolean = degrees == 270 +private fun isPortraitOrientation(degrees: Int): Boolean = degrees == 270 || degrees == 90 Suggestion importance[1-10]: 8 Why: This is a valid bug fix that addresses a potential issue where portrait orientation at 90 degrees would be incorrectly handled, which could affect core functionality.	8
	Add error handling for potential security exceptions when modifying system settings The window attributes modification should be wrapped in a try-catch block as it could throw SecurityException if the app doesn't have the necessary permissions. lib/src/main/java/com/smileidentity/compose/components/ForceBrightness.kt [21-24] -window.attributes = attributes.apply { - screenBrightness = brightness - flags = flags or android.view.WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON +try { + window.attributes = attributes.apply { + screenBrightness = brightness + flags = flags or android.view.WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON + } +} catch (e: SecurityException) { + Timber.e(e, "Failed to modify window brightness") } Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential crash scenario by adding proper exception handling for security-related operations, which is important for app stability.	8
Performance	Improve bitmap memory management by recycling immediately after use instead of relying on disposal Consider recycling the bitmap immediately after drawing it to avoid memory leaks, rather than waiting for the DisposableEffect. You can use a remember key to track when the bitmap should be recycled. lib/src/main/java/com/smileidentity/compose/components/OvalCutout.kt [123-141] -if (selfieBitmap != null && ( +val bitmap = remember(selfieFile) { + selfieFile?.let { + BitmapFactory.decodeFile(it.absolutePath)?.also { bmp -> + bmp.asImageBitmap() + }?.also { bmp -> + bmp.recycle() + } + } +} +if (bitmap != null && ( state.selfieState is SelfieState.Processing || state.selfieState is SelfieState.Success ) ) { clipPath(ovalPath) { drawImage( - image = selfieBitmap.asImageBitmap(), + image = bitmap, dstSize = IntSize( width = constrainedSize.width.roundToInt(), height = constrainedSize.height.roundToInt(), ), dstOffset = IntOffset( x = ovalOffset.x.roundToInt(), y = ovalOffset.y.roundToInt(), ), ) } } Suggestion importance[1-10]: 8 Why: The suggestion improves memory management by ensuring bitmaps are recycled immediately after use rather than waiting for disposal, which helps prevent memory leaks and reduces memory pressure.	8
	Cache animation specifications to prevent unnecessary object creation during recomposition Cache the animation specification to avoid recreating it on each recomposition. Move the tween animation spec to a companion object or top-level constant. lib/src/main/java/com/smileidentity/compose/components/OvalCutout.kt [76-79] -val progressAnimationSpec = tween<Float>( - durationMillis = 50, - easing = FastOutSlowInEasing, +companion object { + private val ProgressAnimationSpec = tween<Float>( + durationMillis = 50, + easing = FastOutSlowInEasing, + ) +} + +// Usage: +val topProgress by animateFloatAsState( + targetValue = state.topProgress, + animationSpec = ProgressAnimationSpec, + label = "selfie_top_progress", ) Suggestion importance[1-10]: 6 Why: Moving the animation spec to a companion object prevents unnecessary object creation on each recomposition, improving performance for frequently animated components.	6
Maintainability	Extract duplicated code into a reusable function to improve maintainability Consider extracting the repeated vibration pattern into a reusable function to avoid code duplication between the Analyzing and Processing states. lib/src/main/java/com/smileidentity/compose/components/DirectiveHaptics.kt [24-27] -for (i in 0..2) { - haptic.performHapticFeedback(HapticFeedbackType.LongPress) - delay(100) +private suspend fun performVibrationPattern(haptic: HapticFeedback) { + repeat(3) { + haptic.performHapticFeedback(HapticFeedbackType.LongPress) + delay(100) + } } Suggestion importance[1-10]: 5 Why: The suggestion identifies and addresses code duplication in the vibration pattern implementation. While valid, this is a minor improvement that mainly affects code maintainability.	5
Enhancement	Use sealed interface instead of enum for better extensibility and type safety Consider using a sealed interface for LivenessType instead of an enum class to make it more extensible and type-safe. lib/src/main/java/com/smileidentity/viewmodel/SelfieViewModel.kt [124-128] -enum class LivenessType(val value: String) { - HeadPose("head_pose"), - Smile("smile"), +sealed interface LivenessType { + val value: String + + data object HeadPose : LivenessType { + override val value: String = "head_pose" + } + + data object Smile : LivenessType { + override val value: String = "smile" + } } Suggestion importance[1-10]: 4 Why: While the suggestion would provide better extensibility, the current enum implementation is sufficient for the current use case with only two simple types.	4
Best practice	Move magic numbers to named constants to improve code maintainability Consider using a constant for the consecutive failure threshold value of 5 to make it more maintainable and self-documenting. Move it to the companion object with other constants. lib/src/main/java/com/smileidentity/viewmodel/ActiveLivenessTask.kt [178] +private companion object { + private const val FAILURE_THRESHOLD = 5 +} + if (consecutiveFailedFrames >= FAILURE_THRESHOLD) { Suggestion importance[1-10]: 3 Why: The suggestion to move the FAILURE_THRESHOLD constant to the companion object with other constants is valid but has low impact since the value is already defined as a constant at the top of the file.	3