diff --git a/app/apollo/apollo-octopus-public/src/commonMain/graphql/com/hedvig/android/apollo/octopus/schema.graphqls b/app/apollo/apollo-octopus-public/src/commonMain/graphql/com/hedvig/android/apollo/octopus/schema.graphqls index 3e30d7a249..9b640dcbe3 100644 --- a/app/apollo/apollo-octopus-public/src/commonMain/graphql/com/hedvig/android/apollo/octopus/schema.graphqls +++ b/app/apollo/apollo-octopus-public/src/commonMain/graphql/com/hedvig/android/apollo/octopus/schema.graphqls @@ -442,6 +442,22 @@ type AgreementDisplayItem { displaySubtitle: String displayValue: String! } +input AnswersInput { + relationship: String + isPregnant: Boolean + hasKids: Boolean + hasCat: Boolean + hasDog: Boolean + yourAge: String + partnerAge: String + isStudent: Boolean + housingType: String + houseAge: String + isRental: Boolean + hasCar: Boolean + carAge: String + wantsAllRisk: Boolean +} enum AppPlatform { IOS ANDROID @@ -678,6 +694,10 @@ type ChangeTierDeductibleFromAgreement { The tier level """ tierLevel: Int + """ + Optional tier description, if set use that rather than productVariant->tierDescription + """ + tierDescription: String """ List of display items for the quote """ @@ -714,6 +734,10 @@ type ChangeTierDeductibleQuote { The tier level, a smaller number means lower coverage. Use this to sort the tier list. """ tierLevel: Int + """ + Optional tier description, if set use that rather than productVariant->tierDescription + """ + tierDescription: String """ The deductible if any """ @@ -872,6 +896,7 @@ type Claim { isUploadingFilesEnabled: Boolean! infoText: String displayItems: [ClaimDisplayItem!]! + contractId: String """ Terms & conditions for the claim found using claims contractId and dateOfOccurrence, otherwise null. """ @@ -1003,7 +1028,7 @@ type ClaimIntentStep { """ A union of all the different kinds of "step content". """ -union ClaimIntentStepContent = ClaimIntentStepContentForm|ClaimIntentStepContentSelect|ClaimIntentStepContentTask|ClaimIntentStepContentAudioRecording|ClaimIntentStepContentFileUpload|ClaimIntentStepContentSummary|ClaimIntentStepContentDeflection|ClaimIntentStepContentDeflectionMessage +union ClaimIntentStepContent = ClaimIntentStepContentForm|ClaimIntentStepContentSelect|ClaimIntentStepContentTask|ClaimIntentStepContentAudioRecording|ClaimIntentStepContentFileUpload|ClaimIntentStepContentSummary|ClaimIntentStepContentDeflection|ClaimIntentStepContentDeflectionMessage|ClaimIntentStepContentInformation """ An audio recording step is one where the user is meant to record some audio. Submitted using `Mutation.claimIntentSubmitAudio`. @@ -1181,6 +1206,22 @@ This typically will be backed by a String - but other formats could appear. """ scalar ClaimIntentStepContentFormFieldValue """ +A read and acknowledge notice if the member needs to be shown some urgent information (if water is still leaking -> turn off the pipe). Not terminal +Submitted using `Mutation.claimIntentSubmitInformation`. +""" +type ClaimIntentStepContentInformation { + notice: String! + """ + So we can show a white/red border if info/critical respectively. + """ + severity: ClaimIntentStepContentInformationSeverity! + buttonTitle: String! +} +enum ClaimIntentStepContentInformationSeverity { + INFO + CRITICAL +} +""" A select step is one that contains a choice to select one of several alternatives. It can be seen as a special-case form with nicer rendering. Submitted using `Mutation.claimIntentSubmitSelect`. @@ -1268,6 +1309,9 @@ input ClaimIntentSubmitFormInput { stepId: ID! fields: [ClaimIntentFormSubmitInputField!]! } +input ClaimIntentSubmitInformationInput { + stepId: ID! +} input ClaimIntentSubmitSelectInput { stepId: ID! selectedId: ID! @@ -1662,6 +1706,10 @@ input CrossSellInput { A/B test experiments for attribution tracking """ experiments: [CrossSellExperimentInput!]! + """ + The contract that was changed in the flow. When set, enables addon recommendations for that contract. + """ + contractId: ID } enum CrossSellSource { HOME @@ -1677,6 +1725,10 @@ type CrossSellV2 { """ recommendedCrossSell: RecommendedCrossSell """ + Addon recommendation for the contract in input.contractId (null when not applicable) + """ + recommendedAddon: RecommendedAddonCrossSell + """ Other available cross-sells (behavior varies by userFlow) """ otherCrossSells: [CrossSell!]! @@ -2433,6 +2485,28 @@ type InsuranceEvidenceOutput { insuranceEvidenceInformation: InsuranceEvidenceInformation userError: UserError } +input InsuranceGuideInput { + """ + Email of the user. + """ + email: String + """ + Id of the insurance guide shopSession. + """ + shopSessionId: UUID! + """ + Insurance recommendation form answers. + """ + answers: AnswersInput! + """ + Insurance recommendations. + """ + recommendations: [RecommendationInput!]! +} +type InsuranceGuideOutput { + success: Boolean! + userError: UserError +} input InsurelyInitiateIframeDataCollectionInput { collectionId: String! partner: String @@ -3449,6 +3523,10 @@ type MoveIntentMutationOutput { Fail case """ userError: UserError + """ + Id of the new contract if it was created after finalizing the move + """ + newContractId: ID } type MoveIntentQuoteCost { quoteId: ID! @@ -3655,6 +3733,10 @@ type Mutation { """ claimIntentSubmitTask(input: ClaimIntentSubmitTaskInput!): ClaimIntentMutationOutput! """ + Acknowledge a step containing a `ClaimIntentStepContentInformation`, continuing the flow. + """ + claimIntentSubmitInformation(input: ClaimIntentSubmitInformationInput!): ClaimIntentMutationOutput! + """ Submit a step containing a `ClaimIntentStepContentSummary`. """ claimIntentSubmitSummary(input: ClaimIntentSubmitSummaryInput!): ClaimIntentMutationOutput! @@ -3729,6 +3811,10 @@ type Mutation { """ productOffersCancellationRequestedUpdate(productOfferIds: [UUID!]!, requested: Boolean!): ProductOffersMutationOutput! """ + Called to save insurance recommendation form and save recommendation email. + """ + insuranceGuide(input: InsuranceGuideInput!): InsuranceGuideOutput! + """ Create a `PriceIntent`. The `input.productName` has to be the name of a product among the ones listed at `Query.availableProducts`. The choice of product also affects which data will be needed for the intent to be confirmed and to produce offers. @@ -4305,6 +4391,10 @@ type ProductOffer { """ priceIntentId: UUID """ + Link to the price calculator for this offer (resumes the shop session). + """ + priceCalculatorLink: String + """ UI-safe masked form data used to generate the offer. PII fields (street, zipCode, city) are masked when address came from registration address lookup. """ priceIntentData: PricingFormData! @@ -4706,6 +4796,28 @@ type RecommendationExternalInsurance { """ dataCollectionId: String! } +input RecommendationInput { + productId: String! + name: String! + tier: String + productPageUrl: String! + priceCalculatorUrl: String! +} +""" +A recommendation to add an addon to an existing contract. The button opens the addon flow via an app deep link. +""" +type RecommendedAddonCrossSell { + id: ID! + title: String! + description: String! + buttonTitle: String! + """ + App deep link to the addon flow for this contract + """ + deepLink: String! + pillowImageSmall: StoryblokImageAsset! + pillowImageLarge: StoryblokImageAsset! +} type RecommendedCrossSell { crossSell: CrossSell! bannerText: String! diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BottomSheetSceneStrategyTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BottomSheetSceneStrategyTest.kt index 8f273e4e28..bd132738dd 100644 --- a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BottomSheetSceneStrategyTest.kt +++ b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BottomSheetSceneStrategyTest.kt @@ -15,6 +15,9 @@ internal class BottomSheetSceneStrategyTest { private fun entry(key: String, metadata: Map = emptyMap()) = NavEntry(key = key, metadata = metadata) { } + private fun capturingEntry(key: String, capture: String, metadata: Map = emptyMap()) = + NavEntry(key = key, metadata = metadata) { capture.length } + @Test fun `bottomSheet metadata is recognized as a bottom-sheet marker`() { assertThat(BottomSheetSceneStrategy.bottomSheet().keys).isNotEmpty() @@ -36,4 +39,22 @@ internal class BottomSheetSceneStrategyTest { assertThat(overlay.overlaidEntries.map { it.contentKey } == listOf("a")).isTrue() assertThat(overlay.entries.map { it.contentKey } == listOf("b")).isTrue() } + + @Test + fun `bottom-sheet scenes for the same content keys are equal despite distinct content lambdas`() { + fun buildScene(capture: String) = with(strategy()) { + with(SceneStrategyScope()) { + calculateScene( + listOf( + capturingEntry("a", capture), + capturingEntry("b", capture, BottomSheetSceneStrategy.bottomSheet()), + ), + ) + } + } + val first = buildScene("first")!! + val second = buildScene("second")!! + assertThat(first == second).isTrue() + assertThat(first.hashCode() == second.hashCode()).isTrue() + } } diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/NavUpBarSceneEqualityTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/NavUpBarSceneEqualityTest.kt new file mode 100644 index 0000000000..9a915850cd --- /dev/null +++ b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/NavUpBarSceneEqualityTest.kt @@ -0,0 +1,76 @@ +package com.hedvig.android.app.navigation + +import androidx.navigation3.runtime.NavEntry +import androidx.navigation3.scene.SceneDecoratorStrategyScope +import androidx.navigation3.scene.SceneStrategyScope +import androidx.navigation3.scene.SinglePaneSceneStrategy +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import com.hedvig.android.navigation.compose.NavSuiteSceneDecoratorStrategy +import com.hedvig.android.navigation.compose.navUpBarSceneDecoratorStrategy +import org.junit.Test + +/** + * Regression test for b/516312097: NavUpBarScene.equals must compare by key, not by the wrapped + * Scene's full equality (which in turn compares NavEntry.content lambda identity). + * + * Root cause: the old `data class NavUpBarScene` delegated equals to its constructor parameters. + * The wrapped `SinglePaneScene` compares `entry == other.entry`, and NavEntry equality is identity + * on the `content: @Composable () -> Unit` lambda. Between recompositions Nav3 creates fresh + * NavEntry instances for the same logical destination: same `contentKey`, different `content` + * lambda. That made two logically-identical NavUpBarScene wrappers unequal, causing the + * NavDisplay's AnimatedContent to treat them as different scenes mid-transition and triggering + * the SaveableStateProvider "Key used multiple times" crash. + * + * NavUpBarScene is `internal` to :navigation-compose; this test reaches it via the public + * [navUpBarSceneDecoratorStrategy] factory, which exercises the same code path that + * [NavSuiteSceneDecoratorStrategy] uses for the ShowUpBar mode. + */ +internal class NavUpBarSceneEqualityTest { + private fun singlePaneStrategy() = SinglePaneSceneStrategy() + + // Non-capturing `{ }` lambdas are folded by the compiler into a singleton, so entries built + // that way would share one content reference and would NOT reproduce the bug. Capturing entries + // guarantee distinct content lambda instances, which is the actual production scenario Nav3 + // creates between recompositions. + private fun capturingEntry(key: String, capture: String) = + NavEntry(key = key, metadata = NavSuiteSceneDecoratorStrategy.showNavBar()) { capture.length } + + @Test + fun `NavUpBarScene wrappers for same-key entries with distinct content lambdas are equal`() { + fun buildWrapped(capture: String) = with(navUpBarSceneDecoratorStrategy()) { + with(SceneDecoratorStrategyScope()) { + val singlePane = with(singlePaneStrategy()) { + with(SceneStrategyScope()) { + calculateScene(listOf(capturingEntry(key = "dest", capture = capture))) + } + }!! + decorateScene(singlePane) + } + } + + // Pre-condition: two SinglePaneScenes with the same contentKey but different content lambdas + // are NOT equal to each other. This documents the root cause: any scene wrapper that delegated + // equals to the wrapped scene would inherit this inequality and produce a spurious "new scene" + // mid-transition. + val rawFirst = with(singlePaneStrategy()) { + with(SceneStrategyScope()) { + calculateScene(listOf(capturingEntry(key = "dest", capture = "alpha"))) + } + }!! + val rawSecond = with(singlePaneStrategy()) { + with(SceneStrategyScope()) { + calculateScene(listOf(capturingEntry(key = "dest", capture = "beta"))) + } + }!! + assertThat(rawFirst == rawSecond).isFalse() + + // After the fix: two NavUpBarScene wrappers for the same contentKey must be equal, regardless + // of the content lambda identity of the underlying entries. + val first = buildWrapped("alpha") + val second = buildWrapped("beta") + assertThat(first == second).isTrue() + assertThat(first.hashCode() == second.hashCode()).isTrue() + } +} diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AndroidAccessTokenProvider.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AndroidAccessTokenProvider.kt index 43f777b1c7..85d92e02b8 100644 --- a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AndroidAccessTokenProvider.kt +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AndroidAccessTokenProvider.kt @@ -1,5 +1,6 @@ package com.hedvig.android.auth +import com.hedvig.android.auth.token.isTokenExpired import com.hedvig.android.core.common.di.AppScope import com.hedvig.android.logger.LogPriority import com.hedvig.android.logger.logcat @@ -8,8 +9,6 @@ import dev.zacsweers.metro.Inject import dev.zacsweers.metro.SingleIn import java.util.UUID import kotlin.time.Clock -import kotlin.time.Duration.Companion.seconds -import kotlin.time.Instant import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -26,7 +25,7 @@ internal class AndroidAccessTokenProvider( val requestId = "#${UUID.randomUUID().toString().takeLast(6)}" val accessToken = (authTokenService.authStatus.value as? AuthStatus.LoggedIn)?.accessToken logcat(LogPriority.VERBOSE) { "$requestId Got accessToken: $accessToken" } - if (accessToken?.expiryDate?.isExpired()?.not() == true) { + if (accessToken?.expiryDate?.isTokenExpired(clock)?.not() == true) { logcat(LogPriority.VERBOSE) { "$requestId Current AccessToken not expired, fast track to adding the header" } return accessToken.token } @@ -34,12 +33,12 @@ internal class AndroidAccessTokenProvider( val authTokens = authTokenService.getTokens() ?: return@withLock null.also { logcat(LogPriority.VERBOSE) { "$requestId Tokens were not stored, proceeding unauthenticated" } } - if (authTokens.accessToken.expiryDate.isExpired().not()) { + if (authTokens.accessToken.expiryDate.isTokenExpired(clock).not()) { logcat(LogPriority.VERBOSE) { "$requestId After lock, token was refreshed, proceeding with refreshed token" } return@withLock authTokens.accessToken.token } logcat(LogPriority.VERBOSE) { "$requestId Still an expired token at this point, try to refresh it" } - if (authTokens.refreshToken.expiryDate.isExpired()) { + if (authTokens.refreshToken.expiryDate.isTokenExpired(clock)) { logcat { "$requestId Refresh token expired, invalidating tokens and proceeding unauthenticated" } // If refresh is also expired, consider ourselves logged out authTokenService.logoutAndInvalidateTokens() @@ -55,17 +54,4 @@ internal class AndroidAccessTokenProvider( refreshedAccessToken.token } } - - private fun Instant.isExpired(): Boolean { - val bufferAdjustedExpirationInstant = this - expirationTimeBuffer - return bufferAdjustedExpirationInstant <= clock.now() - } - - companion object { - /** - * Assume the token expires a bit earlier than it really does, to give some room for the network requests and such - * to run and not risk assuming it's active but until the request goes through it's already become invalidated. - */ - private val expirationTimeBuffer = 60.seconds - } } diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt index de496f46c9..fbc29e9995 100644 --- a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt @@ -4,6 +4,7 @@ import com.hedvig.android.auth.event.AuthEventStorage import com.hedvig.android.auth.storage.AuthTokenStorage import com.hedvig.android.auth.token.AuthTokens import com.hedvig.android.auth.token.LocalRefreshToken +import com.hedvig.android.auth.token.isTokenExpired import com.hedvig.android.core.common.ApplicationScope import com.hedvig.android.core.common.di.AppScope import com.hedvig.android.logger.LogPriority @@ -16,6 +17,7 @@ import com.hedvig.authlib.RefreshTokenGrant import dev.zacsweers.metro.ContributesBinding import dev.zacsweers.metro.Inject import dev.zacsweers.metro.SingleIn +import kotlin.time.Clock import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.first @@ -30,11 +32,20 @@ internal class AuthTokenServiceImpl( private val authRepository: AuthRepository, private val authEventStorage: AuthEventStorage, coroutineScope: ApplicationScope, + private val clock: Clock = Clock.System, ) : AuthTokenService { override val authStatus: StateFlow = authTokenStorage.getTokens() .mapLatest { authTokens -> - val (accessToken, refreshToken) = authTokens ?: return@mapLatest AuthStatus.LoggedOut - AuthStatus.LoggedIn(accessToken, refreshToken) + val tokens = authTokens ?: return@mapLatest AuthStatus.LoggedOut + // A stored session whose refresh token has expired (or is within the expiration buffer) can no + // longer be exchanged for a new grant, so report it as logged out rather than logged in. Uses the + // same buffered expiry the request path uses, so both agree on when a refresh token is usable. + // Expiry is evaluated whenever storage emits; an in-session expiry is handled by the request path + // clearing the tokens, which re-triggers this mapping. + if (tokens.refreshToken.expiryDate.isTokenExpired(clock)) { + return@mapLatest AuthStatus.LoggedOut + } + AuthStatus.LoggedIn(tokens.accessToken, tokens.refreshToken) } .stateIn( coroutineScope, diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/TokenExpiry.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/TokenExpiry.kt new file mode 100644 index 0000000000..0daf7c02a9 --- /dev/null +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/TokenExpiry.kt @@ -0,0 +1,18 @@ +package com.hedvig.android.auth.token + +import kotlin.time.Clock +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * How long before its real expiry a token is already treated as expired, to leave room for the + * network round-trip that will use it: a token still valid at check time can lapse before the request + * (or the token refresh) it authorizes completes. + */ +internal val TOKEN_EXPIRATION_BUFFER: Duration = 60.seconds + +/** True when this expiry instant is at, past, or within [buffer] of [clock]'s current time. */ +internal fun Instant.isTokenExpired(clock: Clock, buffer: Duration = TOKEN_EXPIRATION_BUFFER): Boolean { + return this - buffer <= clock.now() +} diff --git a/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt b/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt new file mode 100644 index 0000000000..bcc5f7d4a3 --- /dev/null +++ b/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt @@ -0,0 +1,97 @@ +package com.hedvig.android.auth + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import com.hedvig.android.auth.event.AuthEventStorage +import com.hedvig.android.auth.storage.AuthTokenStorage +import com.hedvig.android.auth.test.FakeAuthRepository +import com.hedvig.android.core.common.ApplicationScope +import com.hedvig.android.core.datastore.TestPreferencesDataStore +import com.hedvig.android.logger.TestLogcatLoggingRule +import com.hedvig.android.test.clock.TestClock +import com.hedvig.authlib.AccessToken +import com.hedvig.authlib.RefreshToken +import kotlin.time.Clock +import kotlin.time.Duration.Companion.seconds +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +internal class AuthTokenServiceImplTest { + @get:Rule + val testFolder = TemporaryFolder() + + @get:Rule + val testLogcatLogger = TestLogcatLoggingRule() + + @Test + fun `authStatus is LoggedOut when the stored refresh token is expired`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 120), + ) + clock.advanceTimeBy(121.seconds) + val service = authTokenService(storage, clock) + + val status = service.authStatus.filterNotNull().first() + + assertThat(status).isEqualTo(AuthStatus.LoggedOut) + } + + @Test + fun `authStatus is LoggedIn when the stored refresh token is still valid`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 120), + ) + clock.advanceTimeBy(30.seconds) + val service = authTokenService(storage, clock) + + val status = service.authStatus.filterNotNull().first() + + assertThat(status).isInstanceOf(AuthStatus.LoggedIn::class) + } + + @Test + fun `authStatus is LoggedOut when the stored refresh token is within the expiration buffer`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 120), + ) + // 70s in, the refresh token is not yet past its 120s expiry, but it is inside the 60s buffer. + clock.advanceTimeBy(70.seconds) + val service = authTokenService(storage, clock) + + val status = service.authStatus.filterNotNull().first() + + assertThat(status).isEqualTo(AuthStatus.LoggedOut) + } + + private fun TestScope.authTokenService(storage: AuthTokenStorage, clock: Clock): AuthTokenService = + AuthTokenServiceImpl( + storage, + FakeAuthRepository(), + AuthEventStorage(), + ApplicationScope(backgroundScope), + clock, + ) + + private fun TestScope.authTokenStorage(clock: Clock) = AuthTokenStorage( + TestPreferencesDataStore( + datastoreTestFileDirectory = testFolder.newFolder("datastoreTempFolder"), + coroutineScope = backgroundScope, + ), + clock, + ) +} diff --git a/app/data/data-contract/src/commonMain/kotlin/com/hedvig/android/data/contract/ContractGroup.kt b/app/data/data-contract/src/commonMain/kotlin/com/hedvig/android/data/contract/ContractGroup.kt index 3f87d3382e..b1d9e29413 100644 --- a/app/data/data-contract/src/commonMain/kotlin/com/hedvig/android/data/contract/ContractGroup.kt +++ b/app/data/data-contract/src/commonMain/kotlin/com/hedvig/android/data/contract/ContractGroup.kt @@ -63,22 +63,22 @@ fun String.toContractGroup(): ContractGroup = when (this) { "DK_HOUSE", "SE_HOUSE", "SE_HOUSE_BAS", - "SE_HOUSE_MAX" - -> ContractGroup.HOUSE + "SE_HOUSE_MAX", + -> ContractGroup.HOUSE "DK_ACCIDENT", "NO_ACCIDENT", "SE_ACCIDENT", "SE_ACCIDENT_STUDENT", "DK_ACCIDENT_STUDENT", - -> ContractGroup.ACCIDENT + -> ContractGroup.ACCIDENT "DK_TRAVEL", "DK_TRAVEL_STUDENT", "NO_TRAVEL", "NO_TRAVEL_STUDENT", "NO_TRAVEL_YOUTH", - -> ContractGroup.TRAVEL + -> ContractGroup.TRAVEL "DK_HOME_CONTENT_RENT", "DK_HOME_CONTENT_STUDENT_RENT", @@ -90,7 +90,7 @@ fun String.toContractGroup(): ContractGroup = when (this) { "SE_QASA_LONG_TERM_RENTAL", "SE_QASA_SHORT_TERM_RENTAL", "NO_HOME_CONTENT_YOUTH_RENT", - -> ContractGroup.RENTAL + -> ContractGroup.RENTAL "NO_HOME_CONTENT_YOUTH_OWN", "DK_HOME_CONTENT_STUDENT_OWN", @@ -100,13 +100,13 @@ fun String.toContractGroup(): ContractGroup = when (this) { "SE_APARTMENT_BRF_BAS", "SE_APARTMENT_BRF_MAX", "SE_GROUP_APARTMENT_BRF", - -> ContractGroup.HOMEOWNER + -> ContractGroup.HOMEOWNER "NO_HOME_CONTENT_STUDENT_OWN", "NO_HOME_CONTENT_STUDENT_RENT", "SE_APARTMENT_STUDENT_BRF", "SE_APARTMENT_STUDENT_RENT", - -> ContractGroup.STUDENT + -> ContractGroup.STUDENT "SE_CAR_FULL", "SE_CAR_HALF", @@ -114,21 +114,21 @@ fun String.toContractGroup(): ContractGroup = when (this) { "SE_CAR_TRIAL_HALF", "SE_CAR_TRIAL_FULL", "SE_CAR_DECOMMISSIONED", - -> ContractGroup.CAR + -> ContractGroup.CAR "SE_CAT_BASIC", "SE_CAT_PREMIUM", "SE_CAT_STANDARD", - -> ContractGroup.CAT + -> ContractGroup.CAT "SE_DOG_BASIC", "SE_DOG_PREMIUM", "SE_DOG_STANDARD", - -> ContractGroup.DOG + -> ContractGroup.DOG "SE_VACATION_HOME_BAS", "SE_VACATION_HOME_STANDARD", - -> ContractGroup.COUNTRY_HOME + -> ContractGroup.COUNTRY_HOME "SE_QASA_LANDLORD" -> ContractGroup.QASA_LANDLORD diff --git a/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/BottomSheetSceneStrategy.kt b/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/BottomSheetSceneStrategy.kt index 50ba751e9e..b1a6ad691c 100644 --- a/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/BottomSheetSceneStrategy.kt +++ b/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/BottomSheetSceneStrategy.kt @@ -71,13 +71,15 @@ private class BottomSheetScene( override fun equals(other: Any?): Boolean { if (this === other) return true if (other !is BottomSheetScene<*>) return false - return key == other.key && previousEntries == other.previousEntries && entry == other.entry + return key == other.key && + previousEntries.map { it.contentKey } == other.previousEntries.map { it.contentKey } && + entry.contentKey == other.entry.contentKey } override fun hashCode(): Int { var result = key.hashCode() - result = 31 * result + previousEntries.hashCode() - result = 31 * result + entry.hashCode() + result = 31 * result + previousEntries.map { it.contentKey }.hashCode() + result = 31 * result + entry.contentKey.hashCode() return result } } diff --git a/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/NavSuiteSceneDecorator.kt b/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/NavSuiteSceneDecorator.kt index 2046f15677..f7f5c4f597 100644 --- a/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/NavSuiteSceneDecorator.kt +++ b/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/NavSuiteSceneDecorator.kt @@ -1,5 +1,6 @@ package com.hedvig.android.navigation.compose +import androidx.annotation.VisibleForTesting import androidx.compose.animation.EnterExitState import androidx.compose.animation.ExperimentalSharedTransitionApi import androidx.compose.animation.SharedTransitionScope @@ -148,7 +149,13 @@ fun rememberNavSuiteSceneDecoratorStrategy( } @OptIn(ExperimentalSharedTransitionApi::class) -private data class NavSuiteScene( +// Key-based equality: two NavSuiteScene wrappers are the same scene if they wrap a scene of the +// same class and the same key, regardless of NavEntry.content lambda identity. A data class default +// would delegate to the wrapped scene's equals, which in turn compares NavEntry references. Because +// NavEntry.content is a lambda created fresh each recomposition, two logically-identical entries can +// differ by identity, making the wrapping scene look "new" mid-transition. NavUpBarScene below uses +// the same equals/hashCode logic. +internal class NavSuiteScene( val scene: Scene, val sharedTransitionScope: SharedTransitionScope, val navigationSuiteType: () -> NavigationSuiteType, @@ -202,9 +209,23 @@ private data class NavSuiteScene( } } } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is NavSuiteScene<*>) return false + return key == other.key + } + + override fun hashCode(): Int = key.hashCode() } -private data class NavUpBarScene( +// Key-based equality: two NavUpBarScene wrappers are the same scene if they wrap a scene of the +// same class and the same key, regardless of NavEntry.content lambda identity. See the comment on +// NavSuiteScene above for the full rationale. NavSuiteScene uses identical equals/hashCode logic; it +// is not tested directly because its constructor requires a SharedTransitionScope that is hard to +// build outside a Compose host, so it relies on that identical-logic argument while direct test +// coverage lives in NavUpBarSceneEqualityTest. +internal class NavUpBarScene( val scene: Scene, val upBarContent: @Composable () -> Unit, ) : Scene by scene { @@ -223,8 +244,29 @@ private data class NavUpBarScene( } } } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is NavUpBarScene<*>) return false + return key == other.key + } + + override fun hashCode(): Int = key.hashCode() } +/** + * Test seam. Exposes the [LoneDeepLinkChrome.ShowUpBar] decoration path (wrapping a nav-bar scene in + * a [NavUpBarScene]) so [NavUpBarScene]'s key-based equality can be unit tested from `:app`. It + * cannot be tested inside `:navigation-compose`: that KMP android-library target has no host + * unit-test source set, and [NavUpBarScene] is `internal`. There is no production caller. The full + * suite chrome path is exercised through [rememberNavSuiteSceneDecoratorStrategy] in the app. + */ +@VisibleForTesting +fun navUpBarSceneDecoratorStrategy(upBarContent: @Composable () -> Unit = {}): SceneDecoratorStrategy = + SceneDecoratorStrategy { scene -> + if (!scene.metadata.showsNavBar()) scene else NavUpBarScene(scene = scene, upBarContent = upBarContent) + } + private const val NavSuiteSharedKey = "nav-suite-chrome" private object ShowNavBarKey : NavMetadataKey diff --git a/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/RetainedSaveableStateHolderNavEntryDecorator.kt b/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/RetainedSaveableStateHolderNavEntryDecorator.kt index b4f3175d4e..d027a02bc8 100644 --- a/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/RetainedSaveableStateHolderNavEntryDecorator.kt +++ b/app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/RetainedSaveableStateHolderNavEntryDecorator.kt @@ -58,6 +58,9 @@ internal fun rememberRetainedSaveableStateHolderNavEntryDecorator( decoratedKeys.remove(contentKey) } }, + // NavDisplay must never hand two live entries the same contentKey here, or SaveableStateProvider + // throws "Key was used multiple times". Our custom Scenes therefore compare by contentKey, + // not NavEntry identity (see BottomSheetScene and NavSuiteScene/NavUpBarScene). decorate = { entry -> if (entry.contentKey !in decoratedKeys) { decoratedKeys.add(entry.contentKey) diff --git a/docs/superpowers/plans/2026-07-01-nav3-duplicate-key-crash-fix.md b/docs/superpowers/plans/2026-07-01-nav3-duplicate-key-crash-fix.md new file mode 100644 index 0000000000..88da2424c4 --- /dev/null +++ b/docs/superpowers/plans/2026-07-01-nav3-duplicate-key-crash-fix.md @@ -0,0 +1,397 @@ +# Nav3 "Key used multiple times" Crash Fix Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate the `IllegalArgumentException: Key was used multiple times` crash (Crashlytics `f26e8107ace4f9542b53486a6a1a42af`, b/516312097) by making our custom Nav3 `Scene`s stable across recomposition and by stopping `authStatus` from reporting a transient `LoggedIn` for an unrecoverable (expired-refresh) session. + +**Architecture:** The crash fires in `SaveableStateProvider` when two live entries share a `contentKey`. Two independent contributors: (1) our custom `Scene`s compare `NavEntry` by value, which drags in `NavEntry.content ===` identity and makes a logically-identical scene look "new" during a transition; (2) `AuthTokenServiceImpl.authStatus` emits `LoggedIn` for a stored-but-expired session, which the reconciler turns into a rapid `Login -> Home -> Login` re-root mid scene-transition. We fix both at the source, add a comment/tracking trail to b/516312097, and bump `navigation3` to pick up the framework-side fix. + +**Tech Stack:** Kotlin, Jetpack Compose, Navigation 3 (`androidx.navigation3`), Metro DI, kotlinx.coroutines, JUnit4 + assertk + Turbine, kotlin.time.Clock. + +## Global Constraints + +- Never use the em-dash / spaced-dash " - " in prose or code comments. Use a comma, period, parentheses, or colon. (User global rule.) +- Run `./gradlew ktlintFormat` before every commit. Max line length 120, 2-space indent, trailing commas on. +- DI is Metro. No Koin, no `module {}`, no `get()`. +- Do not add strings to any `strings.xml` (Lokalise-managed). +- Gradle output can mask failures when piped: confirm an explicit `BUILD SUCCESSFUL` line, do not trust a `tail`/`grep` exit code. +- Current `androidx-navigation3` version floor: `1.2.0-alpha03` (`gradle/libs.versions.toml:46`). +- Gradle module paths: `:app` (`app/app`), `:auth-core-public` (`app/auth/auth-core-public`), `:navigation-compose` (`app/navigation/navigation-compose`). +- Branch off `develop` before starting: `git switch -c fix/nav3-duplicate-key-crash`. + +## Decisions locked (from planning) + +- **Refresh-token expiry:** exact expiry, no safety buffer. `authStatus` is `LoggedOut` only once the refresh token is strictly at/after `expiryDate`. +- **Nav3 bump:** yes, bump to the latest `1.2.0-alphaNN` in parallel (Task 5), keeping the app-side scene fixes regardless. +- **Test depth:** unit tests only. No instrumented/emulator repro. + +## Open decision (needs answer before Task 2's test step) + +- **D1 - testing the two decorator scenes (`NavSuiteScene`, `NavUpBarScene`):** their equality reduces to `key` (`scene::class to scene.key`, already a stable value), but constructing one in a test needs a Compose `SharedTransitionScope`, and the repo has no Robolectric-compose test infra today. Recommended: **do not** add a dedicated test for these two; the identical `NavEntry`-instability principle is proven by Task 1's `BottomSheetScene` test and the `key`-only equality is stable by construction. Alternative: add net-new Robolectric-compose infra and a `decorateScene` equality test. Task 2 is written for the recommended path; switch only if you pick the alternative. + +--- + +### Task 1: Stabilize `BottomSheetScene` equality + +**Files:** +- Modify: `app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/BottomSheetSceneStrategy.kt:71-82` +- Test: `app/app/src/test/kotlin/com/hedvig/android/app/navigation/BottomSheetSceneStrategyTest.kt` + +**Interfaces:** +- Consumes: `BottomSheetSceneStrategy.calculateScene(entries)` (existing), `BottomSheetSceneStrategy.bottomSheet()` metadata (existing). +- Produces: no signature change. `BottomSheetScene.equals`/`hashCode` now compare by `contentKey` only. + +- [ ] **Step 1: Write the failing test** + +Add to `BottomSheetSceneStrategyTest.kt` (the `entry(...)` helper already creates a fresh `content` lambda per call, so two calls with the same key differ only by lambda identity): + +```kotlin +@Test +fun `bottom-sheet scenes for the same content keys are equal despite distinct content lambdas`() { + fun buildScene() = with(strategy()) { + with(SceneStrategyScope()) { + calculateScene(listOf(entry("a"), entry("b", BottomSheetSceneStrategy.bottomSheet()))) + } + } + val first = buildScene()!! + val second = buildScene()!! + assertThat(first == second).isTrue() + assertThat(first.hashCode() == second.hashCode()).isTrue() +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `./gradlew :app:testDebugUnitTest --tests "com.hedvig.android.app.navigation.BottomSheetSceneStrategyTest"` +Expected: FAIL on the new test (`first == second` is false, because the current `equals` compares `entry`/`previousEntries` by `NavEntry` identity). + +- [ ] **Step 3: Replace `equals`/`hashCode` to compare by `contentKey`** + +In `BottomSheetSceneStrategy.kt`, replace the body of `equals`/`hashCode` (`:71-82`): + +```kotlin + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is BottomSheetScene<*>) return false + return key == other.key && + previousEntries.map { it.contentKey } == other.previousEntries.map { it.contentKey } && + entry.contentKey == other.entry.contentKey + } + + override fun hashCode(): Int { + var result = key.hashCode() + result = 31 * result + previousEntries.map { it.contentKey }.hashCode() + result = 31 * result + entry.contentKey.hashCode() + return result + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `./gradlew :app:testDebugUnitTest --tests "com.hedvig.android.app.navigation.BottomSheetSceneStrategyTest"` +Expected: PASS (all tests, including the two pre-existing ones). + +- [ ] **Step 5: Format and commit** + +```bash +./gradlew ktlintFormat +git add app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/BottomSheetSceneStrategy.kt \ + app/app/src/test/kotlin/com/hedvig/android/app/navigation/BottomSheetSceneStrategyTest.kt +git commit -m "Fix BottomSheetScene equality to compare by contentKey (b/516312097)" +``` + +--- + +### Task 2: Stabilize `NavSuiteScene` and `NavUpBarScene` equality + +**Files:** +- Modify: `app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/NavSuiteSceneDecorator.kt:150-156` (`NavSuiteScene`) and `:207-211` (`NavUpBarScene`) + +**Interfaces:** +- Consumes: `Scene.key` of the wrapped scene (`SinglePaneScene.key` is the entry `contentKey`, a stable value). +- Produces: no signature change. Both scenes now define explicit `equals`/`hashCode` keyed on their own `key` (`scene::class to scene.key`), instead of the `data class` default that delegated to the wrapped scene's `NavEntry`-identity comparison. + +Note: no code destructures or `copy()`s these classes (verified), so dropping `data class` is safe. + +- [ ] **Step 1: Convert `NavSuiteScene` from `data class` to `class` and add explicit equality** + +In `NavSuiteSceneDecorator.kt`, change the declaration at `:151` from `private data class NavSuiteScene(` to `private class NavSuiteScene(` and add, inside the class body (after `onRemove`/`content`, before the closing brace at `:205`): + +```kotlin + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is NavSuiteScene<*>) return false + return key == other.key + } + + override fun hashCode(): Int = key.hashCode() +``` + +- [ ] **Step 2: Convert `NavUpBarScene` from `data class` to `class` and add explicit equality** + +Change the declaration at `:207` from `private data class NavUpBarScene(` to `private class NavUpBarScene(` and add, inside the class body (before the closing brace at `:226`): + +```kotlin + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is NavUpBarScene<*>) return false + return key == other.key + } + + override fun hashCode(): Int = key.hashCode() +``` + +- [ ] **Step 3: Compile the module to verify no `data class` API was relied on** + +Run: `./gradlew :navigation-compose:compileDebugKotlin` (or `:navigation-compose:assemble` if the Android compile task name differs) +Expected: `BUILD SUCCESSFUL`, no "unresolved reference: copy/component1" errors. + +- [ ] **Step 4: (Per decision D1) No dedicated unit test** + +Leave the code comment added in Task 4 as the documentation trail. The `NavEntry`-instability principle is covered by Task 1's `BottomSheetScene` test, and `key` (`scene::class to scene.key`) is a stable value by construction. If you instead chose the alternative in D1, stop and add the Robolectric-compose test before continuing. + +- [ ] **Step 5: Format and commit** + +```bash +./gradlew ktlintFormat +git add app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/NavSuiteSceneDecorator.kt +git commit -m "Stabilize NavSuiteScene/NavUpBarScene equality by contentKey (b/516312097)" +``` + +--- + +### Task 3: Make `authStatus` treat an expired-refresh session as logged out + +**Files:** +- Modify: `app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt:28-43` +- Test: `app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt` (create) + +**Interfaces:** +- Consumes: `AuthTokenStorage.getTokens(): Flow`, `AuthTokens.refreshToken.expiryDate: Instant`, `kotlin.time.Clock`. +- Produces: `AuthTokenServiceImpl(authTokenStorage, authRepository, authEventStorage, coroutineScope, clock: Clock = Clock.System)` (new trailing `clock` param, defaulted). `authStatus` emits `LoggedOut` when the stored refresh token's `expiryDate <= clock.now()`. + +- [ ] **Step 1: Write the failing test** + +Create `AuthTokenServiceImplTest.kt`. It builds a real `AuthTokenStorage` over an in-memory datastore (mirroring `AndroidAccessTokenProviderTest`), seeds a session, advances a `TestClock` past the refresh expiry, then constructs the service so the eager `stateIn` maps at the advanced time: + +```kotlin +package com.hedvig.android.auth + +import app.cash.turbine.Turbine +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import com.hedvig.android.auth.event.AuthEventStorage +import com.hedvig.android.auth.storage.AuthTokenStorage +import com.hedvig.android.auth.test.FakeAuthRepository +import com.hedvig.android.core.common.ApplicationScope +import com.hedvig.android.test.clock.TestClock +import com.hedvig.android.test.datastore.TestPreferencesDataStore +import com.hedvig.authlib.AccessToken +import com.hedvig.authlib.RefreshToken +import kotlin.time.Clock +import kotlin.time.Duration.Companion.seconds +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +internal class AuthTokenServiceImplTest { + @get:Rule + val testFolder = TemporaryFolder() + + @Test + fun `authStatus is LoggedOut when the stored refresh token is expired`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 120), + ) + clock.advanceTimeBy(121.seconds) + val service = authTokenService(storage, clock) + + val status = service.authStatus.filterNotNull().first() + + assertThat(status).isEqualTo(AuthStatus.LoggedOut) + } + + @Test + fun `authStatus is LoggedIn when the stored refresh token is still valid`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 120), + ) + clock.advanceTimeBy(30.seconds) + val service = authTokenService(storage, clock) + + val status = service.authStatus.filterNotNull().first() + + assertThat(status).isInstanceOf(AuthStatus.LoggedIn::class) + } + + private fun TestScope.authTokenService( + storage: AuthTokenStorage, + clock: Clock, + ): AuthTokenService = AuthTokenServiceImpl( + storage, + FakeAuthRepository(), + AuthEventStorage(), + ApplicationScope(backgroundScope), + clock, + ) + + private fun TestScope.authTokenStorage(clock: Clock) = AuthTokenStorage( + TestPreferencesDataStore( + datastoreTestFileDirectory = testFolder.newFolder("datastoreTempFolder"), + coroutineScope = backgroundScope, + ), + clock, + ) +} +``` + +Note: confirm the exact `AccessToken`/`RefreshToken` constructor argument name (`expiryInSeconds`) against `com.hedvig.authlib` and the `TestPreferencesDataStore` import path used by `AndroidAccessTokenProviderTest` (copy them verbatim from that file if they differ). + +- [ ] **Step 2: Run test to verify it fails** + +Run: `./gradlew :auth-core-public:test --tests "com.hedvig.android.auth.AuthTokenServiceImplTest"` +Expected: FAIL on the expired case (`authStatus` currently returns `LoggedIn` because it only checks token presence). + +- [ ] **Step 3: Add the clock dependency and gate `LoggedIn` on refresh expiry** + +In `AuthTokenServiceImpl.kt`, add the import `import kotlin.time.Clock`, add the constructor param, and rewrite the `authStatus` mapping: + +```kotlin +internal class AuthTokenServiceImpl( + private val authTokenStorage: AuthTokenStorage, + private val authRepository: AuthRepository, + private val authEventStorage: AuthEventStorage, + coroutineScope: ApplicationScope, + private val clock: Clock = Clock.System, +) : AuthTokenService { + override val authStatus: StateFlow = authTokenStorage.getTokens() + .mapLatest { authTokens -> + val tokens = authTokens ?: return@mapLatest AuthStatus.LoggedOut + // A stored session whose refresh token has already expired is unrecoverable. There is no valid + // grant left to exchange, so the next authenticated request would fail its refresh and clear the + // tokens anyway. Reporting LoggedIn here roots the app in the logged-in scene for a few frames, + // then the forced logout flips it back to Login, a rapid re-root that crashes Nav3's scene + // SaveableStateHolder ("Key was used multiple times", b/516312097). Treat it as logged out up + // front. Expiry is evaluated whenever storage emits; an in-session expiry is still handled by the + // request-path refresh failure, which clears tokens and re-triggers this mapping. + if (tokens.refreshToken.expiryDate <= clock.now()) { + return@mapLatest AuthStatus.LoggedOut + } + AuthStatus.LoggedIn(tokens.accessToken, tokens.refreshToken) + } + .stateIn( + coroutineScope, + SharingStarted.Eagerly, + null, + ) +``` + +- [ ] **Step 4: Update the sibling test factory so it still compiles** + +`AndroidAccessTokenProviderTest.authTokenService(...)` constructs `AuthTokenServiceImpl(...)` positionally (`:188-193`). The new `clock` param is defaulted, so that call still compiles unchanged. No edit required. (If a compile error appears, add `Clock.System` as the trailing argument there.) + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `./gradlew :auth-core-public:test` +Expected: `BUILD SUCCESSFUL`, `AuthTokenServiceImplTest` passes and `AndroidAccessTokenProviderTest` still passes. + +- [ ] **Step 6: Format and commit** + +```bash +./gradlew ktlintFormat +git add app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt \ + app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt +git commit -m "Treat expired-refresh session as logged out to stop start-scene flap (b/516312097)" +``` + +--- + +### Task 4: Document the b/516312097 trail in code + +**Files:** +- Modify: `app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/RetainedSaveableStateHolderNavEntryDecorator.kt:64-66` (the `SaveableStateProvider` call site, the crash line) + +**Interfaces:** none. Comment only. + +- [ ] **Step 1: Add a comment at the crash site** + +Immediately above the `decorate = { entry -> ... saveableStateHolder.SaveableStateProvider(entry.contentKey) ... }` block, add: + +```kotlin + // NavDisplay must never hand two live entries the same contentKey here, or SaveableStateProvider + // throws "Key was used multiple times" (b/516312097). Our custom Scenes therefore compare by + // contentKey, not NavEntry identity (see BottomSheetScene and NavSuiteScene/NavUpBarScene), and + // AuthTokenServiceImpl avoids the transient logged-in start-scene flap that stressed this path. +``` + +- [ ] **Step 2: Format, compile, commit** + +```bash +./gradlew ktlintFormat +./gradlew :navigation-compose:compileDebugKotlin +git add app/navigation/navigation-compose/src/androidMain/kotlin/com/hedvig/android/navigation/compose/RetainedSaveableStateHolderNavEntryDecorator.kt +git commit -m "Document b/516312097 duplicate-key invariant at the SaveableStateProvider call site" +``` + +--- + +### Task 5: Bump `navigation3` to the latest 1.2.0 alpha + +**Files:** +- Modify: `gradle/libs.versions.toml:46` (`androidx-navigation3`) and, if needed for compatibility, `:47` (`androidx-lifecycle-viewmodel-navigation3`) + +**Interfaces:** none. Version bump. Keep all Task 1-3 changes regardless of the framework fix. + +- [ ] **Step 1: Find the latest published version** + +Check Google Maven for the newest `androidx.navigation3:navigation3-ui` under `1.2.0-alphaNN` (NN > 3). Record the version. Confirm whether `androidx.lifecycle:lifecycle-viewmodel-navigation3` (currently `2.10.0`) needs a matching bump per the release notes. + +- [ ] **Step 2: Update the version catalog** + +Set `androidx-navigation3 = "1.2.0-alpha0N"` in `gradle/libs.versions.toml:46` (use the version from Step 1). Update `androidx-lifecycle-viewmodel-navigation3` only if Step 1 showed it is required. + +- [ ] **Step 3: Build the app and run the nav + auth tests** + +Run: `./gradlew :app:assembleDebug :app:testDebugUnitTest :navigation-compose:test :auth-core-public:test` +Expected: `BUILD SUCCESSFUL`. If Nav3 changed public API (scene/decorator strategy signatures, `NavDisplay` params), fix call sites in `HedvigApp.kt`, `NavSuiteSceneDecorator.kt`, `BottomSheetSceneStrategy.kt`, and the entry-decorator files, then re-run. + +- [ ] **Step 4: Run the back-stack serialization guards** + +Run: `./gradlew :app:testDebugUnitTest --tests "*ExhaustiveBackStackSerializationTest" --tests "*BackstackTest"` +Expected: PASS (no regression in key serialization / back-stack behavior). + +- [ ] **Step 5: Commit** + +```bash +git add gradle/libs.versions.toml +git commit -m "Bump navigation3 to 1.2.0-alpha0N for the OverlayScene duplicate-key fix (b/516312097)" +``` + +--- + +## Self-Review + +**Spec coverage:** +- Scene-equals stability for all custom scenes: Task 1 (`BottomSheetScene`), Task 2 (`NavSuiteScene`, `NavUpBarScene`). Framework `SinglePaneScene` is not ours: covered by Task 5's bump. +- `authStatus` expiry flap: Task 3. +- Tracking / documentation trail: Task 4 (code) plus a `reference` memory to write out-of-band linking b/516312097 to our `NavSuiteScene`/`BottomSheetScene` exposure. +- Framework fix: Task 5. + +**Placeholder scan:** two verification points are intentionally left to the implementer because they depend on external facts, not design choices: the exact `com.hedvig.authlib` token constructor arg name / `TestPreferencesDataStore` import in Task 3 Step 1 (copy verbatim from `AndroidAccessTokenProviderTest`), and the latest `navigation3` version in Task 5 Step 1. + +**Type consistency:** `AuthTokenServiceImpl` gains a trailing `clock: Clock = Clock.System` used identically in the impl and both test factories. Scene `equals`/`hashCode` compare `contentKey`/`key` consistently across Tasks 1 and 2. + +## Manual verification (no automated repro, per decision) + +After all tasks: install a debug build, sign in, then force the reported condition by letting the stored refresh token expire (or shorten it) and cold-starting. Confirm the app lands on Login without crashing and that `authStatus` logs `LoggedOut` without a `Navigated to HomeKey` -> `Navigated to LoginKey` flap in Logcat. diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e068606eb6..fcd053d4a5 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -43,7 +43,7 @@ androidx-compose-runtime-retain = "1.11.1" androidx-datastore = "1.2.0" androidx-junit = "1.3.0" androidx-lifecycle = "2.10.0" -androidx-navigation3 = "1.2.0-alpha03" +androidx-navigation3 = "1.2.0-alpha04" androidx-lifecycle-viewmodel-navigation3 = "2.10.0" androidx-other-appCompat = "1.7.1" androidx-other-browser = "1.9.0"