From a6381336212e667f9ee055e7f877c36df2360f56 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 13 Nov 2022 14:58:04 +0100 Subject: [PATCH 1/4] Increase page size to improve load speed --- .../kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt index 97ab134..97f9b74 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt @@ -25,7 +25,7 @@ class RecipeRepoImpl @Inject constructor( override fun createPager(): Pager { logger.v { "createPager() called" } - val pagingConfig = PagingConfig(pageSize = 5, enablePlaceholders = true) + val pagingConfig = PagingConfig(pageSize = LOAD_PAGE_SIZE, enablePlaceholders = true) return Pager( config = pagingConfig, remoteMediator = mediator, @@ -58,4 +58,8 @@ class RecipeRepoImpl @Inject constructor( logger.v { "updateNameQuery() called with: name = $name" } pagingSourceFactory.setQuery(name) } + + companion object { + private const val LOAD_PAGE_SIZE = 50 + } } \ No newline at end of file From 5c662478431c4080508a842e95e9b51b14fcf209 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 13 Nov 2022 15:03:13 +0100 Subject: [PATCH 2/4] Rename RecipesFragment to RecipesListFragment --- .../kirmanak/mealient/ui/activity/MainActivity.kt | 13 ++++++++----- .../mealient/ui/activity/MainActivityViewModel.kt | 2 +- .../mealient/ui/baseurl/BaseURLFragment.kt | 3 ++- .../mealient/ui/disclaimer/DisclaimerFragment.kt | 3 ++- .../{RecipesFragment.kt => RecipesListFragment.kt} | 11 ++++++----- ...{RecipeViewModel.kt => RecipesListViewModel.kt} | 2 +- ...gment_recipes.xml => fragment_recipes_list.xml} | 2 +- app/src/main/res/navigation/nav_graph.xml | 14 +++++++------- ...iewModelTest.kt => RecipesListViewModelTest.kt} | 4 ++-- 9 files changed, 30 insertions(+), 24 deletions(-) rename app/src/main/java/gq/kirmanak/mealient/ui/recipes/{RecipesFragment.kt => RecipesListFragment.kt} (93%) rename app/src/main/java/gq/kirmanak/mealient/ui/recipes/{RecipeViewModel.kt => RecipesListViewModel.kt} (96%) rename app/src/main/res/layout/{fragment_recipes.xml => fragment_recipes_list.xml} (95%) rename app/src/test/java/gq/kirmanak/mealient/ui/recipes/{RecipeViewModelTest.kt => RecipesListViewModelTest.kt} (95%) diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt b/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt index 3fedf11..39dc6bb 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt @@ -16,7 +16,10 @@ import by.kirich1409.viewbindingdelegate.viewBinding import com.google.android.material.shape.CornerFamily import com.google.android.material.shape.MaterialShapeDrawable import dagger.hilt.android.AndroidEntryPoint -import gq.kirmanak.mealient.NavGraphDirections +import gq.kirmanak.mealient.NavGraphDirections.Companion.actionGlobalAddRecipeFragment +import gq.kirmanak.mealient.NavGraphDirections.Companion.actionGlobalAuthenticationFragment +import gq.kirmanak.mealient.NavGraphDirections.Companion.actionGlobalBaseURLFragment +import gq.kirmanak.mealient.NavGraphDirections.Companion.actionGlobalRecipesListFragment import gq.kirmanak.mealient.R import gq.kirmanak.mealient.databinding.MainActivityBinding import gq.kirmanak.mealient.extensions.observeOnce @@ -69,9 +72,9 @@ class MainActivity : AppCompatActivity(R.layout.main_activity) { logger.v { "onNavigationItemSelected() called with: menuItem = $menuItem" } menuItem.isChecked = true val directions = when (menuItem.itemId) { - R.id.add_recipe -> NavGraphDirections.actionGlobalAddRecipeFragment() - R.id.recipes_list -> NavGraphDirections.actionGlobalRecipesFragment() - R.id.change_url -> NavGraphDirections.actionGlobalBaseURLFragment() + R.id.add_recipe -> actionGlobalAddRecipeFragment() + R.id.recipes_list -> actionGlobalRecipesListFragment() + R.id.change_url -> actionGlobalBaseURLFragment() else -> throw IllegalArgumentException("Unknown menu item id: ${menuItem.itemId}") } navigateTo(directions) @@ -142,7 +145,7 @@ class MainActivity : AppCompatActivity(R.layout.main_activity) { logger.v { "onOptionsItemSelected() called with: item = $item" } val result = when (item.itemId) { R.id.login -> { - navigateTo(NavGraphDirections.actionGlobalAuthenticationFragment()) + navigateTo(actionGlobalAuthenticationFragment()) true } R.id.logout -> { diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivityViewModel.kt b/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivityViewModel.kt index 95d6dd7..73ab467 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivityViewModel.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivityViewModel.kt @@ -41,7 +41,7 @@ class MainActivityViewModel @Inject constructor( _startDestination.value = when { !disclaimerStorage.isDisclaimerAccepted() -> R.id.disclaimerFragment serverInfoRepo.getUrl() == null -> R.id.baseURLFragment - else -> R.id.recipesFragment + else -> R.id.recipesListFragment } } } diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLFragment.kt b/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLFragment.kt index 846e04d..31b1bfa 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLFragment.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLFragment.kt @@ -15,6 +15,7 @@ import gq.kirmanak.mealient.extensions.checkIfInputIsEmpty import gq.kirmanak.mealient.logging.Logger import gq.kirmanak.mealient.ui.OperationUiState import gq.kirmanak.mealient.ui.activity.MainActivityViewModel +import gq.kirmanak.mealient.ui.baseurl.BaseURLFragmentDirections.Companion.actionBaseURLFragmentToRecipesListFragment import javax.inject.Inject @AndroidEntryPoint @@ -56,7 +57,7 @@ class BaseURLFragment : Fragment(R.layout.fragment_base_url) { private fun onUiStateChange(uiState: OperationUiState) = with(binding) { logger.v { "onUiStateChange() called with: uiState = $uiState" } if (uiState.isSuccess) { - findNavController().navigate(BaseURLFragmentDirections.actionBaseURLFragmentToRecipesFragment()) + findNavController().navigate(actionBaseURLFragmentToRecipesListFragment()) return } urlInputLayout.error = when (val exception = uiState.exceptionOrNull) { diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/disclaimer/DisclaimerFragment.kt b/app/src/main/java/gq/kirmanak/mealient/ui/disclaimer/DisclaimerFragment.kt index b1bea90..bd024e4 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/disclaimer/DisclaimerFragment.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/disclaimer/DisclaimerFragment.kt @@ -12,6 +12,7 @@ import gq.kirmanak.mealient.R import gq.kirmanak.mealient.databinding.FragmentDisclaimerBinding import gq.kirmanak.mealient.logging.Logger import gq.kirmanak.mealient.ui.activity.MainActivityViewModel +import gq.kirmanak.mealient.ui.disclaimer.DisclaimerFragmentDirections.Companion.actionDisclaimerFragmentToBaseURLFragment import javax.inject.Inject @AndroidEntryPoint @@ -37,7 +38,7 @@ class DisclaimerFragment : Fragment(R.layout.fragment_disclaimer) { private fun navigateNext() { logger.v { "navigateNext() called" } - findNavController().navigate(DisclaimerFragmentDirections.actionDisclaimerFragmentToBaseURLFragment()) + findNavController().navigate(actionDisclaimerFragmentToBaseURLFragment()) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesFragment.kt b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt similarity index 93% rename from app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesFragment.kt rename to app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt index b715a5d..a9d044c 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesFragment.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt @@ -15,7 +15,7 @@ import by.kirich1409.viewbindingdelegate.viewBinding import dagger.hilt.android.AndroidEntryPoint import gq.kirmanak.mealient.R import gq.kirmanak.mealient.database.recipe.entity.RecipeSummaryEntity -import gq.kirmanak.mealient.databinding.FragmentRecipesBinding +import gq.kirmanak.mealient.databinding.FragmentRecipesListBinding import gq.kirmanak.mealient.datasource.NetworkError import gq.kirmanak.mealient.extensions.collectWhenViewResumed import gq.kirmanak.mealient.extensions.refreshRequestFlow @@ -23,6 +23,7 @@ import gq.kirmanak.mealient.extensions.showLongToast import gq.kirmanak.mealient.extensions.valueUpdatesOnly import gq.kirmanak.mealient.logging.Logger import gq.kirmanak.mealient.ui.activity.MainActivityViewModel +import gq.kirmanak.mealient.ui.recipes.RecipesListFragmentDirections.Companion.actionRecipesFragmentToRecipeInfoFragment import gq.kirmanak.mealient.ui.recipes.images.RecipePreloaderFactory import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.filter @@ -31,10 +32,10 @@ import kotlinx.coroutines.flow.map import javax.inject.Inject @AndroidEntryPoint -class RecipesFragment : Fragment(R.layout.fragment_recipes) { +class RecipesListFragment : Fragment(R.layout.fragment_recipes_list) { - private val binding by viewBinding(FragmentRecipesBinding::bind) - private val viewModel by viewModels() + private val binding by viewBinding(FragmentRecipesListBinding::bind) + private val viewModel by viewModels() private val activityViewModel by activityViewModels() @Inject @@ -62,7 +63,7 @@ class RecipesFragment : Fragment(R.layout.fragment_recipes) { private fun navigateToRecipeInfo(id: String) { logger.v { "navigateToRecipeInfo() called with: id = $id" } - val directions = RecipesFragmentDirections.actionRecipesFragmentToRecipeInfoFragment(id) + val directions = actionRecipesFragmentToRecipeInfoFragment(id) findNavController().navigate(directions) } diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipeViewModel.kt b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt similarity index 96% rename from app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipeViewModel.kt rename to app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt index 7760d28..1a75853 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipeViewModel.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt @@ -12,7 +12,7 @@ import kotlinx.coroutines.flow.onEach import javax.inject.Inject @HiltViewModel -class RecipeViewModel @Inject constructor( +class RecipesListViewModel @Inject constructor( private val recipeRepo: RecipeRepo, authRepo: AuthRepo, private val logger: Logger, diff --git a/app/src/main/res/layout/fragment_recipes.xml b/app/src/main/res/layout/fragment_recipes_list.xml similarity index 95% rename from app/src/main/res/layout/fragment_recipes.xml rename to app/src/main/res/layout/fragment_recipes_list.xml index 3a9f114..4d59c87 100644 --- a/app/src/main/res/layout/fragment_recipes.xml +++ b/app/src/main/res/layout/fragment_recipes_list.xml @@ -4,7 +4,7 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="match_parent" - tools:context=".ui.recipes.RecipesFragment"> + tools:context=".ui.recipes.RecipesListFragment"> + tools:layout="@layout/fragment_recipes_list"> @@ -49,8 +49,8 @@ android:label="fragment_base_url" tools:layout="@layout/fragment_base_url"> @@ -66,8 +66,8 @@ app:destination="@id/authenticationFragment" /> + android:id="@+id/action_global_recipesListFragment" + app:destination="@id/recipesListFragment" /> Date: Sun, 13 Nov 2022 15:29:44 +0100 Subject: [PATCH 3/4] Disable swipe refresh when data is refreshing --- .../ui/recipes/RecipesListFragment.kt | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt index a9d044c..64c67a7 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt @@ -110,6 +110,11 @@ class RecipesListFragment : Fragment(R.layout.fragment_recipes_list) { showLongToast(R.string.fragment_recipes_last_page_loaded_toast) } + collectWhenViewResumed(recipesAdapter.sourceIsRefreshing()) { disableSwipeRefresh -> + logger.v { "setupRecipeAdapter: changing refresher enabled state to ${!disableSwipeRefresh}" } + binding.refresher.isEnabled = !disableSwipeRefresh + } + collectWhenViewResumed(recipesAdapter.refreshErrors()) { onLoadFailure(it) } @@ -157,11 +162,21 @@ private fun Throwable.toLoadErrorReasonText(): Int? = when (this) { } private fun PagingDataAdapter.refreshErrors(): Flow { - return loadStateFlow.map { it.refresh }.valueUpdatesOnly().filterIsInstance() + return loadStateFlow + .map { it.refresh } + .valueUpdatesOnly() + .filterIsInstance() .map { it.error } } private fun PagingDataAdapter.appendPaginationEnd(): Flow { - return loadStateFlow.map { it.append.endOfPaginationReached }.valueUpdatesOnly().filter { it } + return loadStateFlow + .map { it.append.endOfPaginationReached } + .valueUpdatesOnly() + .filter { it } .map { } } + +private fun PagingDataAdapter.sourceIsRefreshing(): Flow { + return loadStateFlow.map { it.source.refresh !is LoadState.NotLoading }.valueUpdatesOnly() +} From 32d366b8fd0876ae12f467e647c229f8590c6fb0 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 13 Nov 2022 15:38:45 +0100 Subject: [PATCH 4/4] Fix absent recipe refresh on authorization --- .../mealient/data/recipes/RecipeRepo.kt | 2 ++ .../data/recipes/impl/RecipeRepoImpl.kt | 16 ++++++++++++- .../ui/recipes/RecipesListFragment.kt | 9 -------- .../ui/recipes/RecipesListViewModel.kt | 19 ++++++--------- .../ui/recipes/RecipesListViewModelTest.kt | 23 ++++++++----------- 5 files changed, 33 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/gq/kirmanak/mealient/data/recipes/RecipeRepo.kt b/app/src/main/java/gq/kirmanak/mealient/data/recipes/RecipeRepo.kt index afb786c..b7c1266 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/recipes/RecipeRepo.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/recipes/RecipeRepo.kt @@ -15,4 +15,6 @@ interface RecipeRepo { suspend fun loadRecipeInfo(recipeId: String): FullRecipeEntity? fun updateNameQuery(name: String?) + + suspend fun refreshRecipes() } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt index 97f9b74..542cd18 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipeRepoImpl.kt @@ -25,7 +25,11 @@ class RecipeRepoImpl @Inject constructor( override fun createPager(): Pager { logger.v { "createPager() called" } - val pagingConfig = PagingConfig(pageSize = LOAD_PAGE_SIZE, enablePlaceholders = true) + val pagingConfig = PagingConfig( + pageSize = LOAD_PAGE_SIZE, + enablePlaceholders = true, + initialLoadSize = INITIAL_LOAD_PAGE_SIZE, + ) return Pager( config = pagingConfig, remoteMediator = mediator, @@ -59,7 +63,17 @@ class RecipeRepoImpl @Inject constructor( pagingSourceFactory.setQuery(name) } + override suspend fun refreshRecipes() { + logger.v { "refreshRecipes() called" } + runCatchingExceptCancel { + storage.refreshAll(dataSource.requestRecipes(0, INITIAL_LOAD_PAGE_SIZE)) + }.onFailure { + logger.e(it) { "Can't refresh recipes" } + } + } + companion object { private const val LOAD_PAGE_SIZE = 50 + private const val INITIAL_LOAD_PAGE_SIZE = LOAD_PAGE_SIZE * 3 } } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt index 64c67a7..019d099 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListFragment.kt @@ -123,15 +123,6 @@ class RecipesListFragment : Fragment(R.layout.fragment_recipes_list) { logger.v { "setupRecipeAdapter: received refresh request" } recipesAdapter.refresh() } - - viewModel.isAuthorized.observe(viewLifecycleOwner) { isAuthorized -> - logger.v { "setupRecipeAdapter: isAuthorized changed to $isAuthorized" } - if (isAuthorized != null) { - if (isAuthorized) recipesAdapter.refresh() - // else is ignored to avoid the removal of the non-public recipes - viewModel.onAuthorizationChangeHandled() - } - } } private fun onLoadFailure(error: Throwable) { diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt index 1a75853..2536f8a 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModel.kt @@ -1,6 +1,9 @@ package gq.kirmanak.mealient.ui.recipes -import androidx.lifecycle.* +import androidx.lifecycle.LiveData +import androidx.lifecycle.ViewModel +import androidx.lifecycle.liveData +import androidx.lifecycle.viewModelScope import androidx.paging.cachedIn import dagger.hilt.android.lifecycle.HiltViewModel import gq.kirmanak.mealient.data.auth.AuthRepo @@ -20,21 +23,13 @@ class RecipesListViewModel @Inject constructor( val pagingData = recipeRepo.createPager().flow.cachedIn(viewModelScope) - private val _isAuthorized = MutableLiveData(null) - val isAuthorized: LiveData = _isAuthorized - init { - authRepo.isAuthorizedFlow.valueUpdatesOnly().onEach { - logger.v { "Authorization state changed to $it" } - _isAuthorized.postValue(it) + authRepo.isAuthorizedFlow.valueUpdatesOnly().onEach { hasAuthorized -> + logger.v { "Authorization state changed to $hasAuthorized" } + if (hasAuthorized) recipeRepo.refreshRecipes() }.launchIn(viewModelScope) } - fun onAuthorizationChangeHandled() { - logger.v { "onAuthorizationSuccessHandled() called" } - _isAuthorized.postValue(null) - } - fun refreshRecipeInfo(recipeSlug: String): LiveData> { logger.v { "refreshRecipeInfo called with: recipeSlug = $recipeSlug" } return liveData { diff --git a/app/src/test/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModelTest.kt b/app/src/test/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModelTest.kt index ee9ddcd..3d9ccbb 100644 --- a/app/src/test/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModelTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/ui/recipes/RecipesListViewModelTest.kt @@ -25,29 +25,24 @@ class RecipesListViewModelTest : BaseUnitTest() { lateinit var recipeRepo: RecipeRepo @Test - fun `when authRepo isAuthorized changes to true expect isAuthorized update`() { + fun `when authRepo isAuthorized changes to true expect that recipes are refreshed`() { every { authRepo.isAuthorizedFlow } returns flowOf(false, true) - assertThat(createSubject().isAuthorized.value).isTrue() + createSubject() + coVerify { recipeRepo.refreshRecipes() } } @Test - fun `when authRepo isAuthorized changes to false expect isAuthorized update`() { + fun `when authRepo isAuthorized changes to false expect that recipes are not refreshed`() { every { authRepo.isAuthorizedFlow } returns flowOf(true, false) - assertThat(createSubject().isAuthorized.value).isFalse() + createSubject() + coVerify(inverse = true) { recipeRepo.refreshRecipes() } } @Test - fun `when authRepo isAuthorized doesn't change expect isAuthorized null`() { + fun `when authRepo isAuthorized doesn't change expect that recipes are not refreshed`() { every { authRepo.isAuthorizedFlow } returns flowOf(true) - assertThat(createSubject().isAuthorized.value).isNull() - } - - @Test - fun `when isAuthorization change is handled expect isAuthorized null`() { - every { authRepo.isAuthorizedFlow } returns flowOf(true, false) - val subject = createSubject() - subject.onAuthorizationChangeHandled() - assertThat(subject.isAuthorized.value).isNull() + createSubject() + coVerify(inverse = true) { recipeRepo.refreshRecipes() } } @Test