--- name: implementing-android-code version: 0.1.4 description: This skill should be used when implementing Android code in Bitwarden. Covers critical patterns, gotchas, and anti-patterns unique to this codebase. Triggered by "How do I implement a ViewModel?", "Create a new screen", "Add navigation", "Write a repository", "BaseViewModel pattern", "State-Action-Event", "type-safe navigation", "@Serializable route", "SavedStateHandle persistence", "process death recovery", "handleAction", "sendAction", "Hilt module", "Repository pattern", "implementing a screen", "adding a data source", "handling navigation", "encrypted storage", "security patterns", "Clock injection", "DataState", or any questions about implementing features, screens, ViewModels, data sources, or navigation in the Bitwarden Android app. --- # Implementing Android Code - Bitwarden Quick Reference **This skill provides tactical guidance for Bitwarden-specific patterns.** For comprehensive architecture decisions and complete code style rules, consult `docs/ARCHITECTURE.md` and `docs/STYLE_AND_BEST_PRACTICES.md`. --- ## Critical Patterns Reference ### A. ViewModel Implementation (State-Action-Event Pattern) All ViewModels follow the **State-Action-Event (SAE)** pattern via `BaseViewModel`. **Key Requirements:** - Annotate with `@HiltViewModel` - State class MUST be `@Parcelize data class : Parcelable` - Implement `handleAction(action: A)` - MUST be synchronous - Post internal actions from coroutines using `sendAction()` - Save/restore state via `SavedStateHandle[KEY_STATE]` - Private action handlers: `private fun handle*` naming convention **Template**: See [ViewModel template](templates.md#viewmodel-template-state-action-event-pattern) **Pattern Summary:** ```kotlin @HiltViewModel class ExampleViewModel @Inject constructor( savedStateHandle: SavedStateHandle, private val repository: ExampleRepository, ) : BaseViewModel( initialState = savedStateHandle[KEY_STATE] ?: ExampleState(), ) { init { stateFlow.onEach { savedStateHandle[KEY_STATE] = it }.launchIn(viewModelScope) } override fun handleAction(action: ExampleAction) { // Synchronous dispatch only when (action) { is Action.Click -> handleClick() is Action.Internal.DataReceived -> handleDataReceived(action) } } private fun handleClick() { viewModelScope.launch { val result = repository.fetchData() sendAction(Action.Internal.DataReceived(result)) // Post internal action } } private fun handleDataReceived(action: Action.Internal.DataReceived) { mutableStateFlow.update { it.copy(data = action.result) } } } ``` **Reference:** - `ui/src/main/kotlin/com/bitwarden/ui/platform/base/BaseViewModel.kt` (see `handleAction` method) - `app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModel.kt` (see class declaration) **Critical Gotchas:** - ❌ **NEVER** update `mutableStateFlow` directly inside coroutines - ✅ **ALWAYS** post internal actions from coroutines, update state in `handleAction()` - ❌ **NEVER** forget `@IgnoredOnParcel` for sensitive data (causes security leak) - ✅ **ALWAYS** use `@Parcelize` on state classes for process death recovery - ✅ State restoration happens automatically if properly saved to `SavedStateHandle` --- ### B. Navigation Implementation (Type-Safe) All navigation uses **type-safe routes** with kotlinx.serialization. **Pattern Structure:** 1. `@Serializable` route data class with parameters 2. `...Args` helper class for extracting from `SavedStateHandle` 3. `NavGraphBuilder.{screen}Destination()` extension for adding screen to graph 4. `NavController.navigateTo{Screen}()` extension for navigation calls **Template**: See [Navigation template](templates.md#navigation-template-type-safe-routes) **Pattern Summary:** ```kotlin @Serializable data class ExampleRoute(val userId: String, val isEditMode: Boolean = false) data class ExampleArgs(val userId: String, val isEditMode: Boolean) fun SavedStateHandle.toExampleArgs(): ExampleArgs { val route = this.toRoute() return ExampleArgs(userId = route.userId, isEditMode = route.isEditMode) } fun NavController.navigateToExample( userId: String, isEditMode: Boolean = false, navOptions: NavOptions? = null, ) { this.navigate(route = ExampleRoute(userId, isEditMode), navOptions = navOptions) } fun NavGraphBuilder.exampleDestination(onNavigateBack: () -> Unit) { composableWithSlideTransitions { ExampleScreen(onNavigateBack = onNavigateBack) } } ``` **Reference:** `app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/login/LoginNavigation.kt` (see `LoginRoute` and extensions) **Key Benefits:** - ✅ Type safety: Compile-time errors for missing parameters - ✅ No string literals in navigation code - ✅ Automatic serialization/deserialization - ✅ Clear contract for screen dependencies --- ### C. Screen/Compose Implementation All screens follow consistent Compose patterns. **Template**: See [Screen/Compose template](templates.md#screencompose-template) **Key Patterns:** ```kotlin @Composable fun ExampleScreen( onNavigateBack: () -> Unit, viewModel: ExampleViewModel = hiltViewModel(), ) { val state by viewModel.stateFlow.collectAsStateWithLifecycle() EventsEffect(viewModel = viewModel) { event -> when (event) { ExampleEvent.NavigateBack -> onNavigateBack() } } BitwardenScaffold( topBar = { BitwardenTopAppBar( title = stringResource(R.string.title), navigationIcon = rememberVectorPainter(BitwardenDrawable.ic_back), onNavigationIconClick = { viewModel.trySendAction(ExampleAction.BackClick) }, ) }, ) { // UI content } } ``` **Reference:** `app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt` (see `LoginScreen` composable) **Essential Requirements:** - ✅ Use `hiltViewModel()` for dependency injection - ✅ Use `collectAsStateWithLifecycle()` for state (not `collectAsState()`) - ✅ Use `EventsEffect(viewModel)` for one-shot events - ✅ Use `Bitwarden*` prefixed components from `:ui` module **State Hoisting Rules:** - **ViewModel state**: Data that needs to survive process death or affects business logic - **UI-only state**: Temporary UI state (scroll position, text field focus) using `remember` or `rememberSaveable` --- ### D. Data Layer Implementation The data layer follows strict patterns for repositories, managers, and data sources. **Interface + Implementation Separation (ALWAYS)** **Template**: See [Data Layer template](templates.md#data-layer-template-repository--hilt-module) **Pattern Summary:** ```kotlin // Interface (injected via Hilt) interface ExampleRepository { suspend fun fetchData(id: String): ExampleResult val dataFlow: StateFlow> } // Implementation (NOT directly injected) class ExampleRepositoryImpl( private val exampleDiskSource: ExampleDiskSource, private val exampleService: ExampleService, ) : ExampleRepository { override suspend fun fetchData(id: String): ExampleResult { // NO exceptions thrown - return Result or sealed class return exampleService.getData(id).fold( onSuccess = { ExampleResult.Success(it.toModel()) }, onFailure = { ExampleResult.Error(it.message) }, ) } } // Sealed result class (domain-specific) sealed class ExampleResult { data class Success(val data: ExampleData) : ExampleResult() data class Error(val message: String?) : ExampleResult() } // Hilt Module @Module @InstallIn(SingletonComponent::class) object ExampleRepositoryModule { @Provides @Singleton fun provideExampleRepository( exampleDiskSource: ExampleDiskSource, exampleService: ExampleService, ): ExampleRepository = ExampleRepositoryImpl(exampleDiskSource, exampleService) } ``` **Reference:** - `app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt` - `app/src/main/kotlin/com/x8bit/bitwarden/data/tools/generator/repository/di/GeneratorRepositoryModule.kt` **Three-Layer Data Architecture:** 1. **Data Sources** - Raw data access (network, disk, SDK). Return `Result`, never throw. 2. **Managers** - Single responsibility business logic. Wrap OS/external services. 3. **Repositories** - Aggregate sources/managers. Return domain-specific sealed classes. **Critical Rules:** - ❌ **NEVER** throw exceptions in data layer - ✅ **ALWAYS** use interface + `...Impl` pattern - ✅ **ALWAYS** inject interfaces, never implementations - ✅ Data sources return `Result`, repositories return domain sealed classes - ✅ Use `StateFlow` for continuously observed data **KDoc on Interfaces vs. Implementations** KDoc on an interface member describes the **contract** — what the caller can rely on. It must not describe **how** the implementation fulfills that contract. Implementation details (which data source is consulted, what is cached, what is logged, which manager is delegated to, ordering of internal calls, etc.) belong on the `...Impl` member — and only when the *why* is non-obvious from the code itself. This applies to every interface in the codebase: repositories, managers, data sources, validators, and UI-layer interfaces alike. The rule is the same one CLAUDE.md states for comments in general — describe the contract or the non-obvious *why*, never the *what* a well-named identifier already conveys. ❌ **Wrong** — interface KDoc leaks the implementation: ```kotlin interface ExampleRepository { /** * Fetches data by reading from [ExampleDiskSource] first, falling back to * [ExampleService] on cache miss and writing the network result back to disk. */ suspend fun fetchData(id: String): ExampleResult } ``` ✅ **Right** — interface KDoc describes the contract; implementation details (if needed at all) live on the override: ```kotlin interface ExampleRepository { /** Returns the [ExampleData] for [id], or an error result on failure. */ suspend fun fetchData(id: String): ExampleResult } class ExampleRepositoryImpl(...) : ExampleRepository { // No KDoc needed — the disk-then-network pattern is visible in the code. override suspend fun fetchData(id: String): ExampleResult { ... } } ``` Red flags that an interface KDoc has drifted into implementation territory: - Names a concrete collaborator (`...DiskSource`, `...Service`, `...Manager`, `...Impl`) - Describes ordering of internal calls ("first ... then ...", "falls back to ...") - Mentions caching, retries, logging, or threading behavior that callers don't depend on - Restates the method signature in prose ("Suspends until X returns Y") If callers genuinely depend on a behavior (e.g., "this method is safe to call before vault unlock", "result is cached for the session"), that *is* part of the contract and belongs on the interface. The test: would a different valid implementation be free to change this? If yes, it's implementation detail — strip it. --- ### E. UI Components **Use Existing Components First:** The `:ui` module provides reusable `Bitwarden*` prefixed components. Search before creating new ones. **Common Components:** - `BitwardenFilledButton` - Primary action buttons - `BitwardenOutlinedButton` - Secondary action buttons - `BitwardenTextField` - Text input fields - `BitwardenPasswordField` - Password input with show/hide - `BitwardenSwitch` - Toggle switches - `BitwardenTopAppBar` - Toolbar/app bar - `BitwardenScaffold` - Screen container with scaffold - `BitwardenBasicDialog` - Simple dialogs - `BitwardenLoadingDialog` - Loading indicators **Component Discovery:** Search `ui/src/main/kotlin/com/bitwarden/ui/platform/components/` for existing `Bitwarden*` components. For build, test, and codebase discovery commands, use the **`build-test-verify`** skill. **When to Create New Reusable Components:** - Component used in 3+ places - Component needs consistent theming across app - Component has semantic meaning (accessibility) - Component has complex state management **New Component Requirements:** - Prefix with `Bitwarden` - Accept themed colors/styles from `BitwardenTheme` - Include preview composables for testing - Support accessibility (content descriptions, semantics) **String Resources:** New strings belong in the `:ui` module: `ui/src/main/res/values/strings.xml` - Use typographic apostrophes and quotes to avoid escape characters: `you’ll` not `you\'ll`, `“word”` not `\"word\"` - Reference strings via generated `BitwardenString` resource IDs - Do not add strings to other modules unless explicitly instructed --- ### F. Security Patterns **Encrypted vs Unencrypted Storage:** **Template**: See [Security templates](templates.md#security-templates) **Pattern Summary:** ```kotlin class ExampleDiskSourceImpl( @EncryptedPreferences encryptedSharedPreferences: SharedPreferences, @UnencryptedPreferences sharedPreferences: SharedPreferences, ) : BaseEncryptedDiskSource( encryptedSharedPreferences = encryptedSharedPreferences, sharedPreferences = sharedPreferences, ), ExampleDiskSource { fun storeAuthToken(token: String) { putEncryptedString(KEY_TOKEN, token) // Sensitive — uses base class method } fun storeThemePreference(isDark: Boolean) { putBoolean(KEY_THEME, isDark) // Non-sensitive — uses base class method } } ``` **Android Keystore (Biometric Keys):** - User-scoped encryption keys: `BiometricsEncryptionManager` - Keys stored in Android Keystore (hardware-backed when available) - Integrity validation on biometric state changes **Input Validation:** ```kotlin // Validation returns boolean, NEVER throws interface RequestValidator { fun validate(request: Request): Boolean } // Sanitization removes dangerous content fun String?.sanitizeTotpUri(issuer: String?, username: String?): String? { if (this.isNullOrBlank()) return null // Sanitize and return safe value } ``` **Security Checklist:** - ✅ Use `@EncryptedPreferences` for credentials, keys, tokens - ✅ Use `@UnencryptedPreferences` for UI state, preferences - ✅ Use `@IgnoredOnParcel` for sensitive ViewModel state - ❌ **NEVER** log sensitive data (passwords, tokens, vault items) - ✅ Validate all user input before processing - ✅ Use Timber for non-sensitive logging only --- ### G. Testing Patterns **ViewModel Testing:** **Template**: See [Testing templates](templates.md#testing-templates) **Pattern Summary:** ```kotlin class ExampleViewModelTest : BaseViewModelTest() { private val mockRepository: ExampleRepository = mockk() @Test fun `ButtonClick should fetch data and update state`() = runTest { val expectedResult = ExampleResult.Success(data = "test") coEvery { mockRepository.fetchData(any()) } returns expectedResult val viewModel = createViewModel() viewModel.trySendAction(ExampleAction.ButtonClick) viewModel.stateFlow.test { assertEquals(EXPECTED_STATE.copy(data = "test"), awaitItem()) } } private fun createViewModel(): ExampleViewModel = ExampleViewModel( savedStateHandle = SavedStateHandle(mapOf(KEY_STATE to EXPECTED_STATE)), repository = mockRepository, ) } ``` **Reference:** `app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt` **Key Testing Patterns:** - ✅ Extend `BaseViewModelTest` for proper dispatcher management - ✅ Use `runTest` from `kotlinx.coroutines.test` - ✅ Use Turbine's `.test { awaitItem() }` for Flow assertions - ✅ Use MockK: `coEvery` for suspend functions, `every` for sync - ✅ Test both state changes and event emissions - ✅ Test both success and failure Result paths **Flow Testing with Turbine:** ```kotlin // Test state and events simultaneously viewModel.stateEventFlow(backgroundScope) { stateFlow, eventFlow -> viewModel.trySendAction(ExampleAction.Submit) assertEquals(ExpectedState.Loading, stateFlow.awaitItem()) assertEquals(ExampleEvent.ShowSuccess, eventFlow.awaitItem()) } ``` **MockK Quick Reference:** ```kotlin coEvery { repository.fetchData(any()) } returns Result.success("data") // Suspend every { diskSource.getData() } returns "cached" // Sync coVerify { repository.fetchData("123") } // Verify ``` --- ### H. Clock/Time Handling All code needing current time must inject `Clock` for testability. **Key Requirements:** - ✅ Inject `Clock` via Hilt in ViewModels - ✅ Pass `Clock` as parameter in extension functions - ✅ Use `clock.instant()` to get current time - ❌ Never call `Instant.now()` or `DateTime.now()` directly - ❌ Never use `mockkStatic` for datetime classes in tests **Pattern Summary:** ```kotlin // ViewModel with Clock class MyViewModel @Inject constructor( private val clock: Clock, ) { val timestamp = clock.instant() } // Extension function with Clock parameter fun State.getTimestamp(clock: Clock): Instant = existingTime ?: clock.instant() // Test with fixed clock val FIXED_CLOCK = Clock.fixed( Instant.parse("2023-10-27T12:00:00Z"), ZoneOffset.UTC, ) ``` **Reference:** - `docs/STYLE_AND_BEST_PRACTICES.md` (see Time and Clock Handling section) - `core/src/main/kotlin/com/bitwarden/core/di/CoreModule.kt` (see `provideClock` function) **Critical Gotchas:** - ❌ `Instant.now()` creates hidden dependency, non-testable - ❌ `mockkStatic(Instant::class)` is fragile, can leak between tests - ✅ `Clock.fixed(...)` provides deterministic test behavior --- ### I. Kotlin Style Rules Project-specific style conventions enforced in code review. These supplement (not replace) `docs/STYLE_AND_BEST_PRACTICES.md`. **`when` branches with wrapped right-hand side require curly braces.** When a `when` branch's expression is too long to fit on the same line as the arrow and is wrapped to the next line, wrap the body in `{ }`. A bare `->` followed by an indented expression on its own line is rejected in review. ❌ **Wrong** — wrapped body without braces: ```kotlin when (type) { VaultItemCipherType.LOGIN -> VaultAddEditState.ViewState.Content.ItemType.Login() VaultItemCipherType.BANK_ACCOUNT -> VaultAddEditState.ViewState.Content.ItemType.BankAccount() VaultItemCipherType.DRIVERS_LICENSE -> VaultAddEditState.ViewState.Content.ItemType.DriversLicense() } ``` ✅ **Right** — wrapped body with braces: ```kotlin when (type) { VaultItemCipherType.LOGIN -> VaultAddEditState.ViewState.Content.ItemType.Login() VaultItemCipherType.BANK_ACCOUNT -> { VaultAddEditState.ViewState.Content.ItemType.BankAccount() } VaultItemCipherType.DRIVERS_LICENSE -> { VaultAddEditState.ViewState.Content.ItemType.DriversLicense() } } ``` Single-line branches (body fits on the same line as `->`) do **not** need braces. --- ## Bitwarden-Specific Anti-Patterns **General anti-patterns are documented in CLAUDE.md.** This section covers violations specific to Bitwarden's State-Action-Event, navigation, and data layer patterns: ❌ **NEVER update ViewModel state directly in coroutines** - Post internal actions, update state synchronously in `handleAction()` ❌ **NEVER inject `...Impl` classes** - Only inject interfaces via Hilt ❌ **NEVER create navigation without `@Serializable` routes** - No string-based navigation, always type-safe ❌ **NEVER use raw `Result` in repositories** - Use domain-specific sealed classes for better error handling ❌ **NEVER make state classes without `@Parcelize`** - All ViewModel state must survive process death ❌ **NEVER skip `SavedStateHandle` persistence for ViewModels** - Users lose form progress on process death ❌ **NEVER forget `@IgnoredOnParcel` for passwords/tokens** - Causes security vulnerability (sensitive data in parcel) ❌ **NEVER use generic `Exception` catching** - Catch specific exceptions only (`RemoteException`, `IOException`) ❌ **NEVER call `Instant.now()` or `DateTime.now()` directly** - Inject `Clock` via Hilt, use `clock.instant()` for testability ❌ **NEVER put implementation details in interface KDoc** - Interface KDoc describes the contract; concrete collaborators, call ordering, caching, and fallback behavior belong on the `...Impl` override (and only when the *why* is non-obvious). See section D for examples. --- ## Quick Reference For build, test, and codebase discovery commands, use the **`build-test-verify`** skill. **File Reference Format:** When pointing to specific code, use: `file_path:line_number` Example: `ui/src/main/kotlin/com/bitwarden/ui/platform/base/BaseViewModel.kt` (see `handleAction` method)