Merge pull request #540 from Automattic/adam/514_avatar_reload

Make working with cache busting easier

gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPicker.kt

@@ -30,7 +30,6 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
-import androidx.compose.runtime.key
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
@@ -213,12 +212,11 @@ internal fun AvatarPicker(uiState: AvatarPickerUiState, onEvent: (AvatarPickerEv
                     modifier = Modifier.padding(start = 16.dp, end = 16.dp, bottom = 24.dp),
                 )
             }
-            key(uiState.avatarUpdates) {
-                ProfileCard(
-                    profile = uiState.profile,
-                    modifier = Modifier.padding(horizontal = 16.dp),
-                )
-            }
+            ProfileCard(
+                profile = uiState.profile,
+                avatarCacheBuster = uiState.avatarCacheBuster.toString(),
+                modifier = Modifier.padding(horizontal = 16.dp),
+            )
             Box(
                 modifier = Modifier
                     .fillMaxWidth()

gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerUiState.kt

@@ -20,9 +20,9 @@ internal data class AvatarPickerUiState(
     val scrollToIndex: Int? = null,
     val failedUploads: Set<AvatarUploadFailure> = emptySet(),
     val failedUploadDialog: AvatarUploadFailure? = null,
-    val avatarUpdates: Int = 0,
     val downloadManagerDisabled: Boolean = false,
     val nonSelectedAvatarAlertVisible: Boolean = false,
+    val avatarCacheBuster: Long? = null,
 ) {
     val avatarsSectionUiState: AvatarsSectionUiState? = emailAvatars?.mapToUiModel()?.let {
         AvatarsSectionUiState(

gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerViewModel.kt

@@ -13,6 +13,8 @@ import com.gravatar.quickeditor.data.models.QuickEditorError
 import com.gravatar.quickeditor.data.repository.AvatarRepository
 import com.gravatar.quickeditor.ui.editor.AvatarPickerContentLayout
 import com.gravatar.quickeditor.ui.editor.GravatarQuickEditorParams
+import com.gravatar.quickeditor.ui.time.Clock
+import com.gravatar.quickeditor.ui.time.SystemClock
 import com.gravatar.restapi.models.Avatar
 import com.gravatar.services.ErrorType
 import com.gravatar.services.GravatarResult
@@ -41,6 +43,7 @@ internal class AvatarPickerViewModel(
     private val avatarRepository: AvatarRepository,
     private val imageDownloader: ImageDownloader,
     private val fileUtils: FileUtils,
+    private val clock: Clock,
 ) : ViewModel() {
     private val _uiState =
         MutableStateFlow(AvatarPickerUiState(email = email, avatarPickerContentLayout = avatarPickerContentLayout))
@@ -194,7 +197,7 @@ internal class AvatarPickerViewModel(
                         _uiState.update { currentState ->
                             currentState.copy(
                                 selectingAvatarId = null,
-                                avatarUpdates = currentState.avatarUpdates.inc(),
+                                avatarCacheBuster = clock.getTimeMillis(),
                             )
                         }
                         _actions.send(AvatarPickerAction.AvatarSelected)
@@ -239,10 +242,10 @@ internal class AvatarPickerViewModel(
                         currentState.copy(
                             uploadingAvatar = null,
                             scrollToIndex = null,
-                            avatarUpdates = if (avatar.selected == true) {
-                                currentState.avatarUpdates.inc()
+                            avatarCacheBuster = if (avatar.selected == true) {
+                                clock.getTimeMillis()
                             } else {
-                                currentState.avatarUpdates
+                                currentState.avatarCacheBuster
                             },
                         )
                     }
@@ -388,10 +391,10 @@ internal class AvatarPickerViewModel(
         if (isSelectedAvatar) _actions.send(AvatarPickerAction.AvatarSelected)
         _uiState.update { currentState ->
             currentState.copy(
-                avatarUpdates = if (isSelectedAvatar) {
-                    currentState.avatarUpdates.inc()
+                avatarCacheBuster = if (isSelectedAvatar) {
+                    clock.getTimeMillis()
                 } else {
-                    currentState.avatarUpdates
+                    currentState.avatarCacheBuster
                 },
             )
         }
@@ -469,6 +472,7 @@ internal class AvatarPickerViewModelFactory(
             avatarRepository = QuickEditorContainer.getInstance().avatarRepository,
             imageDownloader = QuickEditorContainer.getInstance().imageDownloader,
             fileUtils = QuickEditorContainer.getInstance().fileUtils,
+            clock = SystemClock(),
         ) as T
     }
 }

gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/components/ProfileCard.kt

@@ -16,17 +16,25 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
+import com.gravatar.AvatarQueryOptions
+import com.gravatar.extensions.avatarUrl
 import com.gravatar.extensions.defaultProfile
 import com.gravatar.restapi.models.Profile
 import com.gravatar.ui.GravatarTheme
 import com.gravatar.ui.components.ComponentState
 import com.gravatar.ui.components.ProfileSummary
 import com.gravatar.ui.components.atomic.Avatar
+import com.gravatar.ui.components.transform
 
 @Composable
-internal fun ProfileCard(profile: ComponentState<Profile>?, modifier: Modifier = Modifier) {
+internal fun ProfileCard(
+    profile: ComponentState<Profile>?,
+    modifier: Modifier = Modifier,
+    avatarCacheBuster: String? = null,
+) {
     GravatarCard(modifier) { backgroundColor ->
         profile?.let {
             ProfileSummary(
@@ -36,11 +44,17 @@ internal fun ProfileCard(profile: ComponentState<Profile>?, modifier: Modifier =
                     .background(backgroundColor)
                     .padding(horizontal = 16.dp, vertical = 11.dp),
                 avatar = {
+                    val sizePx = with(LocalDensity.current) { 72.dp.roundToPx() }
                     Avatar(
-                        state = profile,
+                        state = profile.transform {
+                            avatarUrl(
+                                AvatarQueryOptions {
+                                    preferredSize = sizePx
+                                },
+                            ).url(avatarCacheBuster).toString()
+                        },
                         size = 72.dp,
                         modifier = Modifier.clip(CircleShape),
-                        forceRefresh = true,
                     )
                 },
             )

gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/time/Clock.kt

@@ -0,0 +1,11 @@
+package com.gravatar.quickeditor.ui.time
+
+internal interface Clock {
+    fun getTimeMillis(): Long
+}
+
+internal class SystemClock : Clock {
+    override fun getTimeMillis(): Long {
+        return System.currentTimeMillis()
+    }
+}

gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerViewModelTest.kt

@@ -11,6 +11,7 @@ import com.gravatar.quickeditor.data.models.QuickEditorError
 import com.gravatar.quickeditor.data.repository.AvatarRepository
 import com.gravatar.quickeditor.ui.CoroutineTestRule
 import com.gravatar.quickeditor.ui.editor.AvatarPickerContentLayout
+import com.gravatar.quickeditor.ui.time.Clock
 import com.gravatar.restapi.models.Avatar
 import com.gravatar.restapi.models.Error
 import com.gravatar.services.ErrorType
@@ -45,6 +46,7 @@ class AvatarPickerViewModelTest {
     private val avatarRepository = mockk<AvatarRepository>()
     private val fileUtils = mockk<FileUtils>()
     private val imageDownloader = mockk<ImageDownloader>()
+    private val clock = mockk<Clock>()
     private val avatarsFlow = MutableSharedFlow<List<Avatar>>(replay = 1)
 
     private lateinit var viewModel: AvatarPickerViewModel
@@ -69,6 +71,7 @@ class AvatarPickerViewModelTest {
         coEvery { profileService.retrieveCatching(email) } returns GravatarResult.Failure(ErrorType.Unknown())
         coEvery { avatarRepository.refreshAvatars(email) } returns GravatarResult.Success(emptyList())
         coEvery { avatarRepository.getAvatars(email) } returns avatarsFlow
+        coEvery { clock.getTimeMillis() } returns 1
     }
 
     @Test
@@ -211,7 +214,6 @@ class AvatarPickerViewModelTest {
                 selectingAvatarId = avatars.last().imageId,
                 scrollToIndex = 0,
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
             )
             assertEquals(
                 avatarPickerUiState,
@@ -220,7 +222,7 @@ class AvatarPickerViewModelTest {
             assertEquals(
                 avatarPickerUiState.copy(
                     selectingAvatarId = null,
-                    avatarUpdates = 1,
+                    avatarCacheBuster = 1,
                 ),
                 awaitItem(),
             )
@@ -672,7 +674,6 @@ class AvatarPickerViewModelTest {
                 uploadingAvatar = uriOne,
                 scrollToIndex = 0,
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
                 nonSelectedAvatarAlertVisible = true,
             )
             assertEquals(
@@ -690,7 +691,7 @@ class AvatarPickerViewModelTest {
                 uploadingAvatar = null,
                 scrollToIndex = null,
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 1,
+                avatarCacheBuster = 1,
                 nonSelectedAvatarAlertVisible = true,
             )
             assertEquals(
@@ -725,7 +726,6 @@ class AvatarPickerViewModelTest {
                 error = null,
                 profile = ComponentState.Loaded(profile),
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
                 scrollToIndex = 0,
                 nonSelectedAvatarAlertVisible = false,
             )
@@ -735,7 +735,7 @@ class AvatarPickerViewModelTest {
             )
 
             avatarPickerUiState = avatarPickerUiState.copy(
-                avatarUpdates = 1,
+                avatarCacheBuster = 1,
             )
             assertEquals(
                 avatarPickerUiState,
@@ -776,7 +776,6 @@ class AvatarPickerViewModelTest {
                 error = null,
                 profile = ComponentState.Loaded(profile),
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
                 scrollToIndex = 0,
                 nonSelectedAvatarAlertVisible = false,
             )
@@ -815,7 +814,6 @@ class AvatarPickerViewModelTest {
                 error = null,
                 profile = ComponentState.Loaded(profile),
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
                 scrollToIndex = 0,
                 nonSelectedAvatarAlertVisible = false,
             )
@@ -854,7 +852,6 @@ class AvatarPickerViewModelTest {
                 error = null,
                 profile = ComponentState.Loaded(profile),
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
                 scrollToIndex = 0,
                 nonSelectedAvatarAlertVisible = false,
             )
@@ -908,7 +905,7 @@ class AvatarPickerViewModelTest {
                     avatarPickerUiState.copy(
                         uploadingAvatar = null,
                         scrollToIndex = null,
-                        avatarUpdates = 1,
+                        avatarCacheBuster = 1,
                     ),
                     awaitItem(),
                 )
@@ -923,7 +920,7 @@ class AvatarPickerViewModelTest {
                         emailAvatars = updatedAvatars.toEmailAvatars(),
                         uploadingAvatar = null,
                         scrollToIndex = null,
-                        avatarUpdates = 1,
+                        avatarCacheBuster = 1,
                         nonSelectedAvatarAlertVisible = false,
                     ),
                     awaitItem(),
@@ -957,7 +954,6 @@ class AvatarPickerViewModelTest {
                 error = null,
                 profile = ComponentState.Loaded(profile),
                 avatarPickerContentLayout = avatarPickerContentLayout,
-                avatarUpdates = 0,
                 scrollToIndex = 0,
             )
             assertEquals(
@@ -966,7 +962,7 @@ class AvatarPickerViewModelTest {
             )
 
             avatarPickerUiState = avatarPickerUiState.copy(
-                avatarUpdates = 1,
+                avatarCacheBuster = 1,
             )
             assertEquals(
                 avatarPickerUiState,
@@ -1209,5 +1205,6 @@ class AvatarPickerViewModelTest {
         avatarRepository = avatarRepository,
         fileUtils = fileUtils,
         imageDownloader = imageDownloader,
+        clock = clock,
     )
 }

gravatar-ui/api/gravatar-ui.api

@@ -49,6 +49,10 @@ public final class com/gravatar/ui/components/ComponentState$Loading : com/grava
 	public fun toString ()Ljava/lang/String;
 }
 
+public final class com/gravatar/ui/components/ComponentStateKt {
+	public static final fun transform (Lcom/gravatar/ui/components/ComponentState;Lkotlin/jvm/functions/Function1;)Lcom/gravatar/ui/components/ComponentState;
+}
+
 public final class com/gravatar/ui/components/ComposableSingletons$ComponentStateKt {
 	public static final field INSTANCE Lcom/gravatar/ui/components/ComposableSingletons$ComponentStateKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function3;
@@ -144,6 +148,7 @@ public final class com/gravatar/ui/components/atomic/AboutMeKt {
 public final class com/gravatar/ui/components/atomic/AvatarKt {
 	public static final fun Avatar-EUb7tLY (Lcom/gravatar/restapi/models/Profile;FLandroidx/compose/ui/Modifier;Lcom/gravatar/AvatarQueryOptions;ZLandroidx/compose/runtime/Composer;II)V
 	public static final fun Avatar-EUb7tLY (Lcom/gravatar/ui/components/ComponentState;FLandroidx/compose/ui/Modifier;Lcom/gravatar/AvatarQueryOptions;ZLandroidx/compose/runtime/Composer;II)V
+	public static final fun Avatar-uFdPcIQ (Lcom/gravatar/ui/components/ComponentState;FLandroidx/compose/ui/Modifier;Landroidx/compose/runtime/Composer;II)V
 }
 
 public final class com/gravatar/ui/components/atomic/ComposableSingletons$AboutMeKt {

gravatar-ui/src/main/java/com/gravatar/ui/components/ComponentState.kt

@@ -42,6 +42,23 @@ public sealed class ComponentState<out T> {
     public data object Empty : ComponentState<Nothing>()
 }
 
+/**
+ * Transforms the loaded value of a [ComponentState] using the provided [transform] function.
+ *
+ * @receiver The [ComponentState] to apply the transformation
+ * @param T The type of the loaded value
+ * @param R The type of the transformed value
+ * @param transform The transformation function
+ * @return A new [ComponentState] with the transformed value
+ */
+public inline fun <T, R> ComponentState<T>.transform(transform: T.() -> R): ComponentState<R> {
+    return when (this) {
+        is Loading -> Loading
+        is Loaded -> Loaded(this.loadedValue.transform())
+        is ComponentState.Empty -> ComponentState.Empty
+    }
+}
+
 @Preview
 @Composable
 internal fun LoadingToLoadedProfileStatePreview(composable: @Composable (state: ComponentState<Profile>) -> Unit = {}) {

gravatar-ui/src/main/java/com/gravatar/ui/components/atomic/Avatar.kt

@@ -94,6 +94,30 @@ public fun Avatar(
     }
 }
 
+/**
+ * [Avatar] is a composable that displays a user's avatar.
+ *
+ * @param state The state of the avatar, when loaded it should contain the Avatar URL
+ * @param size The size of the avatar
+ * @param modifier Composable modifier
+ */
+@Composable
+public fun Avatar(state: ComponentState<String>, size: Dp, modifier: Modifier = Modifier) {
+    when (state) {
+        is ComponentState.Loading -> SkeletonAvatar(size = size, modifier = modifier)
+
+        is ComponentState.Loaded -> {
+            Avatar(
+                model = state.loadedValue,
+                size = size,
+                modifier = modifier,
+            )
+        }
+
+        ComponentState.Empty -> EmptyAvatar(size = size, modifier = modifier)
+    }
+}
+
 @Composable
 private fun SkeletonAvatar(size: Dp, modifier: Modifier = Modifier) {
     Box(