From 06ba1a9fdce55c64f1be98345a954aa203272ae7 Mon Sep 17 00:00:00 2001 From: AnirudhBhat Date: Wed, 4 Dec 2024 18:50:07 +0530 Subject: [PATCH 1/2] Use in memory cache --- .../ui/woopos/home/items/WooPosItemsList.kt | 31 +- .../items/variations/VariationsLRUCache.kt | 48 ++ .../variations/WooPosVariationsDataSource.kt | 57 +- .../variations/WooPosVariationsViewModel.kt | 131 ++-- .../WooPosVariationsViewModelTest.kt | 644 ++++-------------- 5 files changed, 340 insertions(+), 571 deletions(-) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/VariationsLRUCache.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt index 71b1ece84fa..4240fdfec42 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt @@ -96,28 +96,23 @@ fun WooPosItemList( } } - item { - Box( - modifier = Modifier - .fillMaxWidth() - .height(104.dp), - contentAlignment = Alignment.Center - ) { - when (state.paginationState) { - PaginationState.Error -> { - onErrorWhilePaginating() - } - PaginationState.Loading -> { - ItemsLoadingItem() - } - PaginationState.None -> { - Spacer(modifier = Modifier.height(0.dp)) - } + when (state.paginationState) { + PaginationState.Error -> { + item { + onErrorWhilePaginating() } } + PaginationState.Loading -> { + item { + ItemsLoadingItem() + } + } + PaginationState.None -> { + } } + item { - Spacer(modifier = Modifier.height(54.dp)) + Spacer(modifier = Modifier.height(104.dp)) } } InfiniteListHandler(listState, state) { diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/VariationsLRUCache.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/VariationsLRUCache.kt new file mode 100644 index 00000000000..a13ffbf5878 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/VariationsLRUCache.kt @@ -0,0 +1,48 @@ +package com.woocommerce.android.ui.woopos.home.items.variations + +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock + +class VariationsLRUCache(private val maxSize: Int) { + + companion object { + private const val LOAD_FACTOR = 0.75f + } + private val cache = object : LinkedHashMap(maxSize, LOAD_FACTOR, true) { + override fun removeEldestEntry(eldest: Map.Entry): Boolean { + return size > maxSize + } + } + + private val mutex = Mutex() + + suspend fun get(key: K): V? { + return mutex.withLock { + cache[key] + } + } + + suspend fun put(key: K, value: V) { + mutex.withLock { + cache[key] = value + } + } + + suspend fun remove(key: K) { + mutex.withLock { + cache.remove(key) + } + } + + suspend fun clear() { + mutex.withLock { + cache.clear() + } + } + + suspend fun containsKey(key: K): Boolean { + return mutex.withLock { + cache.containsKey(key) + } + } +} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt index c1e33c1888e..cfd28e8a392 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt @@ -5,6 +5,11 @@ import com.woocommerce.android.ui.products.variations.selector.VariationListHand import com.woocommerce.android.util.WooLog import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import javax.inject.Inject import javax.inject.Singleton @@ -13,6 +18,19 @@ import javax.inject.Singleton class WooPosVariationsDataSource @Inject constructor( private val handler: VariationListHandler ) { + private val variationCache = VariationsLRUCache>(maxSize = 50) + private val cacheMutex = kotlinx.coroutines.sync.Mutex() + + private suspend fun getCachedVariations(productId: Long): List { + return cacheMutex.withLock { variationCache.get(productId) ?: emptyList() } + } + + private suspend fun updateCache(productId: Long, variations: List) { + cacheMutex.withLock { + variationCache.put(productId, variations) + } + } + fun getVariationsFlow(productId: Long): Flow> { return handler.getVariationsFlow(productId) } @@ -21,6 +39,35 @@ class WooPosVariationsDataSource @Inject constructor( return handler.canLoadMore() } + fun fetchFirstPage( + productId: Long, + forceRefresh: Boolean = true + ): Flow>> = flow { + if (forceRefresh) { + updateCache(productId, emptyList()) + } + + val cachedVariations = getCachedVariations(productId) + if (cachedVariations.isNotEmpty()) { + emit(FetchResult.Cached(cachedVariations)) + } + + val result = handler.fetchVariations(productId, forceRefresh = true) + if (result.isSuccess) { + val remoteVariations = handler.getVariationsFlow(productId).firstOrNull() ?: emptyList() + updateCache(productId, remoteVariations) + emit(FetchResult.Remote(Result.success(remoteVariations))) + } else { + emit( + FetchResult.Remote( + Result.failure( + result.exceptionOrNull() ?: Exception("Unknown error while fetching variations") + ) + ) + ) + } + }.flowOn(Dispatchers.IO) + suspend fun fetchVariations(productId: Long, forceRefresh: Boolean = true): Result { val result = handler.fetchVariations(productId, forceRefresh = forceRefresh) return if (result.isSuccess) { @@ -33,10 +80,11 @@ class WooPosVariationsDataSource @Inject constructor( } } - suspend fun loadMore(productId: Long): Result = withContext(Dispatchers.IO) { + suspend fun loadMore(productId: Long): Result> = withContext(Dispatchers.IO) { val result = handler.loadMore(productId) if (result.isSuccess) { - Result.success(Unit) + val fetchedVariations = handler.getVariationsFlow(productId).first() + Result.success(fetchedVariations) } else { result.logFailure() Result.failure( @@ -55,3 +103,8 @@ private fun Result.logFailure() { val errorMessage = error?.message ?: "Unknown error" WooLog.e(WooLog.T.POS, "Loading variations failed - $errorMessage", error) } + +sealed class FetchResult { + data class Cached(val data: T) : FetchResult() + data class Remote(val result: Result) : FetchResult() +} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt index 058505b1556..149bb707a4b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.home.items.variations import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.home.ChildToParentEvent import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender @@ -15,9 +16,7 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn -import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import javax.inject.Inject @@ -39,60 +38,88 @@ class WooPosVariationsViewModel @Inject constructor( ) private var flowJob: Job? = null - private var fetchJob: Job? = null private var loadMoreJob: Job? = null fun init(productId: Long, withPullToRefresh: Boolean = false, withCart: Boolean = true) { resetState() - startCollectingVariationsFlow(productId) - fetchVariations(productId = productId, withPullToRefresh = withPullToRefresh, withCart = withCart) + loadVariations( + productId = productId, + withPullToRefresh = withPullToRefresh, + withCart = withCart, + forceRefresh = false + ) } private fun resetState() { flowJob?.cancel() variationsDataSource.resetLoadMoreState() - _viewState.value = WooPosVariationsViewState.Loading(withCart = true) } - private fun startCollectingVariationsFlow(productId: Long) { - flowJob?.cancel() - flowJob = viewModelScope.launch { - variationsDataSource.getVariationsFlow(productId) - .map { variationList -> - if (variationList.isEmpty() && fetchJob?.isActive == true) { - WooPosVariationsViewState.Loading(withCart = true) - } else if (variationList.isEmpty()) { - WooPosVariationsViewState.Error() - } else { - WooPosVariationsViewState.Content( - items = variationList.filter { it.price != null }.map { - WooPosItem.Variation( - id = it.remoteVariationId, - name = it.getName(getProductById(productId)), - productId = it.remoteProductId, - price = priceFormat(it.price), - imageUrl = it.image?.source - ) + private fun loadVariations( + productId: Long, + forceRefresh: Boolean, + withPullToRefresh: Boolean, + withCart: Boolean, + ) { + viewModelScope.launch { + _viewState.value = if (withPullToRefresh) { + buildProductsReloadingState() + } else { + WooPosVariationsViewState.Loading(withCart = withCart) + } + + variationsDataSource.fetchFirstPage(productId, forceRefresh = forceRefresh).collect { result -> + when (result) { + is FetchResult.Cached -> { + if (result.data.isNotEmpty()) { + updateViewStateWithVariations(result.data, productId) + } + } + + is FetchResult.Remote -> { + _viewState.value = when { + result.result.isSuccess -> { + val variations = result.result.getOrThrow() + if (variations.isNotEmpty()) { + WooPosVariationsViewState.Content( + items = variations.filter { it.price != null }.map { + WooPosItem.Variation( + id = it.remoteVariationId, + name = it.getName(getProductById(productId)), + productId = it.remoteProductId, + price = priceFormat(it.price), + imageUrl = it.image?.source + ) + } + ) + } else { + WooPosVariationsViewState.Empty() + } } - ) + + else -> WooPosVariationsViewState.Error() + } } } - .collect { state -> - _viewState.value = state - } + } } } - private fun fetchVariations(productId: Long, withPullToRefresh: Boolean, withCart: Boolean) { - _viewState.value = if (withPullToRefresh) { - buildProductsReloadingState() + private suspend fun updateViewStateWithVariations(variations: List, productId: Long) { + if (variations.isEmpty()) { + _viewState.value = WooPosVariationsViewState.Empty() } else { - WooPosVariationsViewState.Loading(withCart = withCart) - } - fetchJob?.cancel() - - fetchJob = viewModelScope.launch { - variationsDataSource.fetchVariations(productId, forceRefresh = true) + _viewState.value = WooPosVariationsViewState.Content( + items = variations.filter { it.price != null }.map { + WooPosItem.Variation( + id = it.remoteVariationId, + name = it.getName(getProductById(productId)), + productId = it.remoteProductId, + price = priceFormat(it.price), + imageUrl = it.image?.source + ) + } + ) } } @@ -119,20 +146,24 @@ class WooPosVariationsViewModel @Inject constructor( loadMoreJob?.cancel() loadMoreJob = viewModelScope.launch { val result = variationsDataSource.loadMore(productId) - _viewState.update { current -> - if (current is WooPosVariationsViewState.Content) { - current.copy(paginationState = determinePaginationState(result)) - } else { - current - } + _viewState.value = if (result.isSuccess) { + WooPosVariationsViewState.Content( + items = result.getOrThrow().filter { it.price != null }.map { + WooPosItem.Variation( + id = it.remoteVariationId, + name = it.getName(getProductById(productId)), + productId = it.remoteProductId, + price = priceFormat(it.price), + imageUrl = it.image?.source + ) + } + ) + } else { + currentState.copy(paginationState = PaginationState.Error) } } } - private fun determinePaginationState(result: Result<*>): PaginationState { - return if (result.isSuccess) PaginationState.None else PaginationState.Error - } - fun onUIEvent(event: WooPosVariationsUIEvents) { when (event) { is WooPosVariationsUIEvents.EndOfItemsListReached -> { @@ -140,11 +171,11 @@ class WooPosVariationsViewModel @Inject constructor( } is WooPosVariationsUIEvents.PullToRefreshTriggered -> { - fetchVariations(event.productId, withPullToRefresh = true, withCart = false) + loadVariations(event.productId, forceRefresh = true, withPullToRefresh = true, withCart = false) } is WooPosVariationsUIEvents.VariationsLoadingErrorRetryButtonClicked -> { - init(event.productId, withPullToRefresh = false, withCart = false) + loadVariations(event.productId, forceRefresh = true, withPullToRefresh = false, withCart = false) } is WooPosVariationsUIEvents.OnItemClicked -> { diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt index 409ef184760..185fee8481b 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt @@ -1,8 +1,5 @@ package com.woocommerce.android.ui.woopos.home.variations -import android.annotation.SuppressLint -import androidx.arch.core.executor.testing.InstantTaskExecutorRule -import androidx.lifecycle.asLiveData import app.cash.turbine.test import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById @@ -11,587 +8,232 @@ import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender import com.woocommerce.android.ui.woopos.home.items.PaginationState import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel import com.woocommerce.android.ui.woopos.home.items.WooPosVariationsViewState +import com.woocommerce.android.ui.woopos.home.items.variations.FetchResult import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsDataSource import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsUIEvents import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsViewModel import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import com.woocommerce.android.ui.woopos.util.format.WooPosFormatPrice import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat import org.junit.Rule -import org.junit.Test -import org.mockito.ArgumentMatchers.eq import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import kotlin.test.assertFalse -import kotlin.test.assertTrue +import kotlin.test.Test @ExperimentalCoroutinesApi class WooPosVariationsViewModelTest { - @Rule @JvmField - val rule = InstantTaskExecutorRule() - @Rule @JvmField val coroutinesTestRule = WooPosCoroutineTestRule() private val getProductById: WooPosGetProductById = mock() - private val childrenToParentEventSender: WooPosChildrenToParentEventSender = mock() private val variationsDataSource: WooPosVariationsDataSource = mock() + private val fromChildToParentEventSender: WooPosChildrenToParentEventSender = mock() private val priceFormat: WooPosFormatPrice = mock { onBlocking { invoke(any()) }.thenReturn("$10.0") } - private lateinit var wooPosVariationsViewModel: WooPosVariationsViewModel - - @Test - fun `given view model init, then loading state is displayed`() = runTest { - whenever(variationsDataSource.getVariationsFlow(1L)).thenReturn(emptyFlow()) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - - assertThat( - wooPosVariationsViewModel.viewState.value - ).isEqualTo( - WooPosVariationsViewState.Loading(withCart = true) - ) - } @Test - fun `given view model init, then API call is made to fetch product`() = runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - } + fun `given variations from data source, when view model created, then view state updated correctly`() = runTest { + // GIVEN + val variations = listOf( + ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ProductTestUtils.generateProductVariation(2, 1, "20.0") ) - - whenever(variationsDataSource.fetchVariations(any(), any())).thenReturn( - Result.success(Unit) + whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( + flowOf(FetchResult.Remote(Result.success(variations))) ) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) + // WHEN + val viewModel = createViewModel() + viewModel.init(1L) - verify(getProductById, times(3)).invoke(1L) + viewModel.viewState.test { + // THEN + val state = awaitItem() as WooPosVariationsViewState.Content + assertThat(state.items).hasSize(2) + assertThat(state.items[0].id).isEqualTo(1) + assertThat(state.items[0].price).isEqualTo("$10.0") + } } @Test - fun `given view model init, then API call is made to fetch variation`() = runTest { - whenever(variationsDataSource.getVariationsFlow(1L)).thenReturn(emptyFlow()) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat + fun `given empty variations list returned, when view model created, then view state is empty`() = runTest { + // GIVEN + whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( + flowOf(FetchResult.Remote(Result.success(emptyList()))) ) - wooPosVariationsViewModel.init(1L) - verify(variationsDataSource).fetchVariations(eq(1L), any()) + // WHEN + val viewModel = createViewModel() + viewModel.init(1L) + viewModel.viewState.test { + // THEN + assertThat(awaitItem()).isEqualTo(WooPosVariationsViewState.Empty()) + } } @Test - fun `given view model init, then API call is made to fetch variation with forceRefresh set to true`() = runTest { - whenever(variationsDataSource.getVariationsFlow(1L)).thenReturn(emptyFlow()) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat + fun `given error fetching variations, when view model created, then view state is error`() = runTest { + // GIVEN + whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( + flowOf(FetchResult.Remote(Result.failure(Exception("Fetch error")))) ) - wooPosVariationsViewModel.init(1L) - - verify(variationsDataSource).fetchVariations(1L, forceRefresh = true) - } - - @Test - fun `given view model init, when variation fetched successfully, then view state is updated with variation content`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - } - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - advanceUntilIdle() - - wooPosVariationsViewModel.viewState.test { - // THEN - val value = awaitItem() as WooPosVariationsViewState.Content - assertThat(value).isInstanceOf(WooPosVariationsViewState.Content::class.java) - } - } - - @Test - fun `given view model init, when variation fetched successfully, then filter out variations with price null`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L, amount = ""), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - } - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - advanceUntilIdle() - - wooPosVariationsViewModel.viewState.test { - // THEN - val value = awaitItem() as WooPosVariationsViewState.Content - assertThat(value.items.size).isEqualTo(2) - } - } - - @Test - fun `given view model init, when variation fetched successfully, then view state is updated with proper variation content`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - } - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - advanceUntilIdle() - - wooPosVariationsViewModel.viewState.test { - // THEN - val value = awaitItem() as WooPosVariationsViewState.Content - assertThat(value.items.size).isEqualTo(3) - assertThat(value.items[0].id).isEqualTo(2) - assertThat(value.items[1].id).isEqualTo(3) - assertThat(value.items[2].id).isEqualTo(4) - assertThat(value.paginationState).isEqualTo(PaginationState.None) - assertFalse(value.reloadingProductsWithPullToRefresh) - } - } - - @Test - fun `given variation fetch fails, when retry clicked, then fetch variations called`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - emptyFlow() - ) - whenever(variationsDataSource.fetchVariations(any(), any())).thenReturn( - Result.failure(Throwable()) - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - - wooPosVariationsViewModel.init(1L) - wooPosVariationsViewModel.onUIEvent( - WooPosVariationsUIEvents.VariationsLoadingErrorRetryButtonClicked(1L) - ) - - verify(variationsDataSource, times(2)).fetchVariations(1L) - } - @SuppressLint("CheckResult") - @Test - fun `given view state is content, when pull to refreshed, then view state contains Content state with pull to refresh set to true`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flowOf( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - val states: MutableList = mutableListOf() - wooPosVariationsViewModel.viewState.asLiveData().observeForever { - states.add(it) - } - - wooPosVariationsViewModel.init(1L) - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.PullToRefreshTriggered(1L)) + // WHEN + val viewModel = createViewModel() + viewModel.init(1L) - assertThat( - states.any { (it as? WooPosVariationsViewState.Content)?.reloadingProductsWithPullToRefresh == true } - ) + viewModel.viewState.test { + // THEN + assertThat(awaitItem()).isEqualTo(WooPosVariationsViewState.Error()) } + } @Test - fun `given view state is Loading, when pull to refreshed, then view state contains Loading state with pull to refresh set to true`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(1L)).thenReturn( - emptyFlow() - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) + fun `given variations, when pull to refresh triggered, then fetchFirstPage is called`() = runTest { + // GIVEN + val variations = listOf( + ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ProductTestUtils.generateProductVariation(2, 1, "20.0") + ) + whenever(variationsDataSource.fetchFirstPage(any(), eq(true))).thenReturn( + flowOf(FetchResult.Remote(Result.success(variations))) + ) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.PullToRefreshTriggered(1L)) + // WHEN + val viewModel = createViewModel() + viewModel.onUIEvent(WooPosVariationsUIEvents.PullToRefreshTriggered(123L)) - wooPosVariationsViewModel.viewState.test { - // THEN - val value = awaitItem() as WooPosVariationsViewState.Loading - assertTrue(value.reloadingProductsWithPullToRefresh) - } - } + // THEN + verify(variationsDataSource).fetchFirstPage(eq(123L), eq(true)) + } @Test - fun `given view state is Error, when fetch variations, then view state is updated with error state`() = runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - delay(100) - emit(emptyList()) - } + fun `given no more variations, when end of list reached, then pagination state is none`() = runTest { + // GIVEN + val variations = listOf( + ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ProductTestUtils.generateProductVariation(2, 1, "20.0") ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - whenever(variationsDataSource.fetchVariations(any(), any())).thenReturn( - Result.failure(Throwable()) + whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( + flowOf(FetchResult.Remote(Result.success(variations))) ) + whenever(variationsDataSource.canLoadMore()).thenReturn(false) + whenever(variationsDataSource.loadMore(any())).thenReturn(Result.success(emptyList())) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) + val viewModel = createViewModel() + viewModel.init(1L) advanceUntilIdle() - wooPosVariationsViewModel.viewState.test { - // THEN - val value = awaitItem() as WooPosVariationsViewState.Error - assertThat(value).isInstanceOf(WooPosVariationsViewState.Error::class.java) + // WHEN + viewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(123L)) + advanceUntilIdle() + // THEN + viewModel.viewState.test { + val state = awaitItem() as WooPosVariationsViewState.Content + assertThat(state.paginationState).isEqualTo(PaginationState.None) } } @Test - fun `given view state is Error, when pull to refreshed, then view state is updated with proper variation content`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), + fun `given variations, when load more succeeds, then pagination state is updated`() = runTest { + // GIVEN + val variations = listOf( + ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ) + whenever(variationsDataSource.loadMore(any())).thenReturn(Result.success(variations)) + whenever(variationsDataSource.canLoadMore()).thenReturn(true) + whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( + flow { + emit( + FetchResult.Remote( + Result.success( + listOf( + ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ProductTestUtils.generateProductVariation(2, 1, "20.0") + ) ) ) - } - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - whenever(variationsDataSource.fetchVariations(any(), any())).thenReturn( - Result.failure(Throwable()) - ) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - val states: MutableList = mutableListOf() - wooPosVariationsViewModel.viewState.asLiveData().observeForever { - states.add(it) + ) } + ) - wooPosVariationsViewModel.init(1L) - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.PullToRefreshTriggered(1L)) + val viewModel = createViewModel() + viewModel.init(1L) + viewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(123L)) - assertThat( - states.any { (it as? WooPosVariationsViewState.Error)?.reloadingProductsWithPullToRefresh == true } - ) + // THEN + viewModel.viewState.test { + val state = awaitItem() as WooPosVariationsViewState.Content + assertThat(state.items).hasSize(1) + assertThat(state.items[0].id).isEqualTo(1) } + } @Test - fun `when end of list reached, then load more called`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), + fun `given load more fails, when end of list reached, then pagination state is error`() = runTest { + // GIVEN + whenever(variationsDataSource.loadMore(any())).thenReturn(Result.failure(Exception())) + whenever(variationsDataSource.canLoadMore()).thenReturn(true) + whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( + flow { + emit( + FetchResult.Remote( + Result.success( + listOf( + ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ProductTestUtils.generateProductVariation(2, 1, "20.0") + ) ) ) - } - ) - - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - whenever(variationsDataSource.fetchVariations(any(), any())).thenReturn( - Result.success(Unit) - ) - whenever(variationsDataSource.canLoadMore()).thenReturn(true) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - advanceUntilIdle() - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(1L)) - - verify(variationsDataSource).loadMore(1L) - } - - @Test - fun `given view state that is not Content, when load more is called, then return with doing nothing`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(1L)).thenReturn( - emptyFlow() - ) - whenever(variationsDataSource.loadMore(any())).thenReturn( - Result.failure(Throwable()) - ) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(1L)) - - verify(variationsDataSource, never()).loadMore(1L) - } - - @Test - fun `given no more items to load, when load more is called, then return with doing nothing`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(1L)).thenReturn( - emptyFlow() - ) - whenever(variationsDataSource.loadMore(any())).thenReturn( - Result.failure(Throwable()) - ) - whenever(variationsDataSource.canLoadMore()).thenReturn(false) + ) + } + ) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(1L)) + val viewModel = createViewModel() + viewModel.init(1L) + advanceUntilIdle() + viewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(123L)) + advanceUntilIdle() - verify(variationsDataSource, never()).loadMore(1L) + // THEN + viewModel.viewState.test { + val state = awaitItem() as WooPosVariationsViewState.Content + assertThat(state.paginationState).isEqualTo(PaginationState.Error) } + } @Test - fun `given more items to load, when load more is success, then pagination state is updated to None `() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - } - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - whenever(variationsDataSource.loadMore(any())).thenReturn( - Result.success(Unit) - ) - whenever(variationsDataSource.canLoadMore()).thenReturn(true) - - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat - ) - wooPosVariationsViewModel.init(1L) - advanceUntilIdle() - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(1L)) - val states: MutableList = mutableListOf() - wooPosVariationsViewModel.viewState.asLiveData().observeForever { - states.add(it) - } - - assertThat((states[1] as WooPosVariationsViewState.Content).paginationState).isEqualTo( - PaginationState.None - ) - } + fun `given variation clicked, when item clicked, then send event to parent`() = runTest { + // GIVEN + val viewModel = createViewModel() - @Test - fun `given load more call fails, when load more is called, then pagination state is updated to Error`() = - runTest { - whenever(variationsDataSource.getVariationsFlow(any())).thenReturn( - flow { - emit(emptyList()) - delay(100) - emit( - listOf( - ProductTestUtils.generateProductVariation(1L, 2L), - ProductTestUtils.generateProductVariation(1L, 3L), - ProductTestUtils.generateProductVariation(1L, 4L), - ) - ) - } - ) - whenever(getProductById.invoke(any())).thenReturn( - ProductTestUtils.generateProduct(1L, isVariable = true, productType = "variable") - ) - whenever(variationsDataSource.loadMore(any())).thenReturn( - Result.failure(Throwable()) - ) - whenever(variationsDataSource.canLoadMore()).thenReturn(true) + // WHEN + viewModel.onUIEvent(WooPosVariationsUIEvents.OnItemClicked(123L, 1L)) - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, - getProductById, - variationsDataSource, - priceFormat + // THEN + verify(fromChildToParentEventSender).sendToParent( + ChildToParentEvent.ItemClickedInProductSelector( + WooPosItemsViewModel.ItemClickedData.Variation(123L, 1L) ) - wooPosVariationsViewModel.init(1L) - advanceUntilIdle() - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(1L)) - - wooPosVariationsViewModel.viewState.test { - val value = awaitItem() as WooPosVariationsViewState.Content - assertThat(value.paginationState).isEqualTo( - PaginationState.Error - ) - } - } + ) + } - @Test - fun `given OnItemClicked event, when onUIEvent is called, then onVariationClicked is triggered`() = runTest { - // Arrange - val productId = 1L - val variationId = 2L - wooPosVariationsViewModel = WooPosVariationsViewModel( - childrenToParentEventSender, + private fun createViewModel() = + WooPosVariationsViewModel( + fromChildToParentEventSender, getProductById, variationsDataSource, priceFormat ) - - // Act - wooPosVariationsViewModel.onUIEvent(WooPosVariationsUIEvents.OnItemClicked(productId, variationId)) - - // Assert - verify(childrenToParentEventSender).sendToParent( - ChildToParentEvent.ItemClickedInProductSelector( - WooPosItemsViewModel.ItemClickedData.Variation(productId, variationId) - ) - ) - } } From 2664527fd8c26757cdc14474793457a92b81fe54 Mon Sep 17 00:00:00 2001 From: AnirudhBhat Date: Wed, 4 Dec 2024 18:51:06 +0530 Subject: [PATCH 2/2] Remove unnecessary code --- .../variations/WooPosVariationsDataSource.kt | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt index cfd28e8a392..b27e477c28e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt @@ -31,10 +31,6 @@ class WooPosVariationsDataSource @Inject constructor( } } - fun getVariationsFlow(productId: Long): Flow> { - return handler.getVariationsFlow(productId) - } - fun canLoadMore(): Boolean { return handler.canLoadMore() } @@ -68,18 +64,6 @@ class WooPosVariationsDataSource @Inject constructor( } }.flowOn(Dispatchers.IO) - suspend fun fetchVariations(productId: Long, forceRefresh: Boolean = true): Result { - val result = handler.fetchVariations(productId, forceRefresh = forceRefresh) - return if (result.isSuccess) { - Result.success(Unit) - } else { - result.logFailure() - Result.failure( - result.exceptionOrNull() ?: Exception("Unknown error while loading more variations") - ) - } - } - suspend fun loadMore(productId: Long): Result> = withContext(Dispatchers.IO) { val result = handler.loadMore(productId) if (result.isSuccess) {