From 324da664d7c529a768632fb03face08bf6eeaaae Mon Sep 17 00:00:00 2001 From: Michael Totschnig Date: Mon, 28 Oct 2024 17:37:41 +0100 Subject: [PATCH] Fixes #1591: We no longer depend on flattenMerge's concurrency limit We make sure that budgets are consistently updated --- .../totschnig/myexpenses/db2/Repository.kt | 44 ++++++---- .../myexpenses/fragment/BudgetList.kt | 71 ++++++++------- .../viewmodel/BudgetEditViewModel.kt | 17 ++++ .../viewmodel/BudgetListViewModel.kt | 77 ++++++++++++++++ .../myexpenses/viewmodel/BudgetViewModel.kt | 88 ++----------------- .../myexpenses/viewmodel/data/Budget.kt | 13 --- 6 files changed, 164 insertions(+), 146 deletions(-) create mode 100644 myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetListViewModel.kt diff --git a/myExpenses/src/main/java/org/totschnig/myexpenses/db2/Repository.kt b/myExpenses/src/main/java/org/totschnig/myexpenses/db2/Repository.kt index c1048fc899..69a95eed5a 100644 --- a/myExpenses/src/main/java/org/totschnig/myexpenses/db2/Repository.kt +++ b/myExpenses/src/main/java/org/totschnig/myexpenses/db2/Repository.kt @@ -12,9 +12,18 @@ import androidx.datastore.preferences.core.Preferences import org.totschnig.myexpenses.model.CurrencyContext import org.totschnig.myexpenses.model.Grouping import org.totschnig.myexpenses.preference.PrefHandler -import org.totschnig.myexpenses.provider.DatabaseConstants import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ACCOUNTID +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ACCOUNT_LABEL import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_CATID +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_COLOR +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_CURRENCY +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_DESCRIPTION +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_END +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_GROUPING +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_IS_DEFAULT +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ROWID +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_START +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_TITLE import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_UUID import org.totschnig.myexpenses.provider.TransactionProvider import org.totschnig.myexpenses.provider.TransactionProvider.AUTOFILL_URI @@ -123,23 +132,22 @@ open class Repository @Inject constructor( } ?: 0L val budgetCreatorFunction: (Cursor) -> Budget = { cursor -> - val currency = cursor.getString(DatabaseConstants.KEY_CURRENCY) - val budgetId = cursor.getLong(DatabaseConstants.KEY_ROWID) - val accountId = cursor.getLong(KEY_ACCOUNTID) - val grouping = cursor.getEnum(DatabaseConstants.KEY_GROUPING, Grouping.NONE) - Budget( - id = budgetId, - accountId = accountId, - title = cursor.getString(DatabaseConstants.KEY_TITLE), - description = cursor.getString(DatabaseConstants.KEY_DESCRIPTION), - currency = currency, - grouping = grouping, - color = cursor.getInt(DatabaseConstants.KEY_COLOR), - start = if (grouping == Grouping.NONE) cursor.getString(DatabaseConstants.KEY_START) else null, - end = if (grouping == Grouping.NONE) cursor.getString(DatabaseConstants.KEY_END) else null, - accountName = cursor.getStringOrNull(DatabaseConstants.KEY_ACCOUNT_LABEL), - default = cursor.getBoolean(DatabaseConstants.KEY_IS_DEFAULT) - ) + with(cursor) { + val grouping = getEnum(KEY_GROUPING, Grouping.NONE) + Budget( + id = getLong(KEY_ROWID), + accountId = getLong(KEY_ACCOUNTID), + title = getString(KEY_TITLE), + description = getString(KEY_DESCRIPTION), + currency = getString(KEY_CURRENCY), + grouping = grouping, + color = getInt(KEY_COLOR), + start = if (grouping == Grouping.NONE) getString(KEY_START) else null, + end = if (grouping == Grouping.NONE) getString(KEY_END) else null, + accountName = getStringOrNull(KEY_ACCOUNT_LABEL), + default = getBoolean(KEY_IS_DEFAULT) + ) + } } } diff --git a/myExpenses/src/main/java/org/totschnig/myexpenses/fragment/BudgetList.kt b/myExpenses/src/main/java/org/totschnig/myexpenses/fragment/BudgetList.kt index a1fd1cb774..21e6284bac 100644 --- a/myExpenses/src/main/java/org/totschnig/myexpenses/fragment/BudgetList.kt +++ b/myExpenses/src/main/java/org/totschnig/myexpenses/fragment/BudgetList.kt @@ -13,6 +13,7 @@ import androidx.fragment.app.activityViewModels import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle +import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.DividerItemDecoration import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.ListAdapter @@ -31,17 +32,15 @@ import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ROWID import org.totschnig.myexpenses.provider.filter.FilterPersistence import org.totschnig.myexpenses.util.ICurrencyFormatter import org.totschnig.myexpenses.util.ui.addChipsBulk -import org.totschnig.myexpenses.util.crashreporting.CrashHandler +import org.totschnig.myexpenses.viewmodel.BudgetListViewModel +import org.totschnig.myexpenses.viewmodel.BudgetListViewModel.BudgetViewItem import org.totschnig.myexpenses.viewmodel.BudgetViewModel -import org.totschnig.myexpenses.viewmodel.data.Budget -import org.totschnig.myexpenses.viewmodel.data.Budget.Companion.DIFF_CALLBACK import javax.inject.Inject class BudgetList : Fragment() { private var _binding: BudgetsBinding? = null private val binding get() = _binding!! - private val viewModel: BudgetViewModel by activityViewModels() - private var budgetAmounts: MutableMap?> = mutableMapOf() + private val viewModel: BudgetListViewModel by activityViewModels() @Inject lateinit var currencyFormatter: ICurrencyFormatter @@ -75,11 +74,13 @@ class BudgetList : Fragment() { StateSaver.saveInstanceState(this, outState) } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - val adapter = requireActivity().let { - it.injector.inject(viewModel) - BudgetsAdapter(it) - } + injector.inject(viewModel) + viewModel.init() + val activity = requireActivity() + val adapter = BudgetsAdapter(activity) with(binding.recyclerView) { LinearLayoutManager(activity).also { layoutManager = it @@ -89,7 +90,7 @@ class BudgetList : Fragment() { with(viewLifecycleOwner) { lifecycleScope.launch { repeatOnLifecycle(Lifecycle.State.STARTED) { - viewModel.data.collect { + viewModel.enrichedData.collect { adapter.submitList(it) binding.empty.isVisible = it.isEmpty() binding.recyclerView.isVisible = it.isNotEmpty() @@ -98,24 +99,6 @@ class BudgetList : Fragment() { } } - with(viewLifecycleOwner) { - lifecycleScope.launch { - repeatOnLifecycle(Lifecycle.State.CREATED) { - viewModel.amounts.collect { (position, id, spent, allocated) -> - budgetAmounts[id] = spent to allocated - if (binding.recyclerView.isComputingLayout) { - CrashHandler.report(Exception("Budget amount received while recyclerView is computing layout")) - binding.recyclerView.post { - adapter.notifyItemChanged(position) - } - } else { - adapter.notifyItemChanged(position) - } - } - } - } - } - binding.recyclerView.adapter = adapter } @@ -134,8 +117,18 @@ class BudgetList : Fragment() { } } + val diffCallback = object : DiffUtil.ItemCallback() { + override fun areItemsTheSame(oldItem: BudgetViewItem, newItem: BudgetViewItem): Boolean { + return oldItem.budget.id == newItem.budget.id + } + + override fun areContentsTheSame(oldItem: BudgetViewItem, newItem: BudgetViewItem): Boolean { + return oldItem == newItem + } + } + inner class BudgetsAdapter(val context: Context) : - ListAdapter(DIFF_CALLBACK) { + ListAdapter(diffCallback) { init { setHasStableIds(true) @@ -147,14 +140,20 @@ class BudgetList : Fragment() { } override fun onBindViewHolder(holder: BudgetViewHolder, position: Int) { - getItem(position).let { budget -> + getItem(position).let { (budget, info) -> + if (info == null) { + viewModel.loadBudgetAmounts(budget) + } with(holder.binding) { Title.text = budget.titleComplete(context) - val (spent, allocated) = budgetAmounts[budget.id] ?: run { - viewModel.loadBudgetAmounts(position, budget) - 0L to 0L - } - budgetSummary.bind(budget, currencyContext[budget.currency], -spent, allocated, currencyFormatter) + + budgetSummary.bind( + budget, + currencyContext[budget.currency], + info?.spent?.unaryMinus() ?: 0L, + info?.allocated ?: 0L, + currencyFormatter + ) filter.addChipsBulk(buildList { add(budget.label(requireContext())) @@ -180,7 +179,7 @@ class BudgetList : Fragment() { } } - override fun getItemId(position: Int): Long = getItem(position).id + override fun getItemId(position: Int): Long = getItem(position).budget.id } } diff --git a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetEditViewModel.kt b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetEditViewModel.kt index a16eb5f5e9..efea2a7e47 100644 --- a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetEditViewModel.kt +++ b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetEditViewModel.kt @@ -3,6 +3,9 @@ package org.totschnig.myexpenses.viewmodel import android.app.Application import android.content.ContentUris import android.net.Uri +import androidx.lifecycle.MutableLiveData +import androidx.lifecycle.liveData +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ROWID import org.totschnig.myexpenses.provider.TransactionProvider import org.totschnig.myexpenses.provider.filter.FilterPersistence import org.totschnig.myexpenses.provider.filter.WhereFilter @@ -11,6 +14,20 @@ import org.totschnig.myexpenses.viewmodel.data.Budget class BudgetEditViewModel(application: Application) : BudgetViewModel(application) { private val databaseHandler: DatabaseHandler = DatabaseHandler(application.contentResolver) + /** + * provides id of budget on success, -1 on error + */ + val databaseResult = MutableLiveData() + + fun budget(budgetId: Long) = liveData(context = coroutineContext()) { + contentResolver.query( + TransactionProvider.BUDGETS_URI, + PROJECTION, "${q(KEY_ROWID)} = ?", arrayOf(budgetId.toString()), null + )?.use { + if (it.moveToFirst()) emit((repository.budgetCreatorFunction(it))) + } + } + fun saveBudget(budget: Budget, initialAmount: Long?, whereFilter: WhereFilter) { val contentValues = budget.toContentValues(initialAmount) if (budget.id == 0L) { diff --git a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetListViewModel.kt b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetListViewModel.kt new file mode 100644 index 0000000000..a616fa2f6b --- /dev/null +++ b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetListViewModel.kt @@ -0,0 +1,77 @@ +package org.totschnig.myexpenses.viewmodel + +import android.app.Application +import androidx.lifecycle.viewModelScope +import app.cash.copper.flow.mapToOne +import app.cash.copper.flow.observeQuery +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch +import org.totschnig.myexpenses.db2.budgetAllocationQueryUri +import org.totschnig.myexpenses.db2.sumLoaderForBudget +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_BUDGET +import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_SUM_EXPENSES +import org.totschnig.myexpenses.provider.DatabaseConstants.THIS_YEAR +import org.totschnig.myexpenses.provider.getLong +import org.totschnig.myexpenses.viewmodel.BudgetViewModel2.Companion.aggregateNeutralPrefKey +import org.totschnig.myexpenses.viewmodel.data.Budget +import timber.log.Timber + +class BudgetListViewModel(application: Application) : BudgetViewModel(application) { + + data class BudgetViewItem(val budget: Budget, val budgetInfo: BudgetInfo?) + data class BudgetInfo(val allocated: Long, val spent: Long) + + fun init() { + viewModelScope.launch { + data.collect { + _enrichedData.value = it.map { BudgetViewItem(it, null) } + } + } + } + + private val _enrichedData = MutableStateFlow>(emptyList()) + val enrichedData: StateFlow> = _enrichedData + + @OptIn(ExperimentalCoroutinesApi::class) + fun loadBudgetAmounts(budget: Budget) { + Timber.d("Debugg: Loading, %d", budget.id) + viewModelScope.launch { + dataStore.data.map { + it[aggregateNeutralPrefKey(budget.id)] == true + }.flatMapLatest { aggregateNeutral -> + val (sumUri, sumSelection, sumSelectionArguments) = repository.sumLoaderForBudget( + budget, aggregateNeutral, null + ) + + val allocationUri = budgetAllocationQueryUri( + budget.id, + 0, + budget.grouping, + THIS_YEAR, + budget.grouping.queryArgumentForThisSecond + ) + + combine( + contentResolver.observeQuery( + sumUri, + arrayOf(KEY_SUM_EXPENSES), sumSelection, sumSelectionArguments, null, true + ) + .mapToOne { it.getLong(KEY_SUM_EXPENSES) }, + contentResolver.observeQuery(allocationUri) + .mapToOne(0) { it.getLong(KEY_BUDGET) } + ) { spent, allocated -> Triple(budget.id, spent, allocated) } + }.collect { (id, spent, allocated) -> + _enrichedData.value = _enrichedData.value.map { + if (it.budget.id == id) + BudgetViewItem(it.budget, BudgetInfo(allocated, spent)) + else it + } + } + } + } +} \ No newline at end of file diff --git a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetViewModel.kt b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetViewModel.kt index 4157bf08fc..d930c05685 100644 --- a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetViewModel.kt +++ b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/BudgetViewModel.kt @@ -3,24 +3,14 @@ package org.totschnig.myexpenses.viewmodel import android.app.Application import androidx.lifecycle.MutableLiveData import androidx.lifecycle.liveData -import androidx.lifecycle.viewModelScope import app.cash.copper.flow.mapToList -import app.cash.copper.flow.mapToOne import app.cash.copper.flow.observeQuery -import arrow.core.Tuple4 -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.flattenMerge -import kotlinx.coroutines.flow.map -import kotlinx.coroutines.launch -import org.totschnig.myexpenses.db2.budgetAllocationQueryUri -import org.totschnig.myexpenses.db2.sumLoaderForBudget +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.onEach import org.totschnig.myexpenses.provider.DataBaseAccount.Companion.HOME_AGGREGATE_ID import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ACCOUNTID import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ACCOUNT_LABEL -import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_BUDGET import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_CODE import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_COLOR import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_CURRENCY @@ -31,86 +21,26 @@ import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_IS_DEFAULT import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_LABEL import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_ROWID import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_START -import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_SUM_EXPENSES import org.totschnig.myexpenses.provider.DatabaseConstants.KEY_TITLE import org.totschnig.myexpenses.provider.DatabaseConstants.TABLE_ACCOUNTS import org.totschnig.myexpenses.provider.DatabaseConstants.TABLE_BUDGETS import org.totschnig.myexpenses.provider.DatabaseConstants.TABLE_CURRENCIES -import org.totschnig.myexpenses.provider.DatabaseConstants.THIS_YEAR import org.totschnig.myexpenses.provider.TransactionProvider -import org.totschnig.myexpenses.provider.getLong -import org.totschnig.myexpenses.viewmodel.BudgetViewModel2.Companion.aggregateNeutralPrefKey import org.totschnig.myexpenses.viewmodel.data.Budget +import timber.log.Timber import java.util.Locale open class BudgetViewModel(application: Application) : ContentResolvingAndroidViewModel(application) { - val data by lazy { + val data: Flow> by lazy { contentResolver.observeQuery( uri = TransactionProvider.BUDGETS_URI, - projection = PROJECTION - ).mapToList(mapper = repository.budgetCreatorFunction) - } - - /** - * provides id of budget on success, -1 on error - */ - val databaseResult = MutableLiveData() - - /** - * pair of position to budget - */ - private val budgetLoaderFlow = MutableSharedFlow>() - - - fun budget(budgetId: Long) = liveData(context = coroutineContext()) { - contentResolver.query( - TransactionProvider.BUDGETS_URI, - PROJECTION, "${q(KEY_ROWID)} = ?", arrayOf(budgetId.toString()), null - )?.use { - if (it.moveToFirst()) emit((repository.budgetCreatorFunction(it))) - } - } - - @OptIn(ExperimentalCoroutinesApi::class) - val amounts: Flow> by lazy { - budgetLoaderFlow.map { budgetPair -> - dataStore.data.map { - Triple( - budgetPair.first, - budgetPair.second, - it[aggregateNeutralPrefKey(budgetPair.second.id)] == true - ) - } - }.flattenMerge().map { (position, budget, aggregateNeutral) -> - val (sumUri, sumSelection, sumSelectionArguments) = repository.sumLoaderForBudget( - budget, aggregateNeutral, null - ) - - val allocationUri = budgetAllocationQueryUri( - budget.id, - 0, - budget.grouping, - THIS_YEAR, - budget.grouping.queryArgumentForThisSecond - ) - - combine( - contentResolver.observeQuery( - sumUri, - arrayOf(KEY_SUM_EXPENSES), sumSelection, sumSelectionArguments, null, true - ) - .mapToOne { it.getLong(KEY_SUM_EXPENSES) }, - contentResolver.observeQuery(allocationUri) - .mapToOne(0) { it.getLong(KEY_BUDGET) } - ) { spent, allocated -> Tuple4(position, budget.id, spent, allocated) } - }.flattenMerge() - } - - fun loadBudgetAmounts(position: Int, budget: Budget) { - viewModelScope.launch { - budgetLoaderFlow.emit(position to budget) + projection = PROJECTION, + notifyForDescendants = true + ).onEach { + Timber.i("budget data received") } + .mapToList(mapper = repository.budgetCreatorFunction) } companion object { diff --git a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/data/Budget.kt b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/data/Budget.kt index 160d204579..d038fcf4e6 100644 --- a/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/data/Budget.kt +++ b/myExpenses/src/main/java/org/totschnig/myexpenses/viewmodel/data/Budget.kt @@ -2,7 +2,6 @@ package org.totschnig.myexpenses.viewmodel.data import android.content.ContentValues import android.content.Context -import androidx.recyclerview.widget.DiffUtil import org.totschnig.myexpenses.R import org.totschnig.myexpenses.model.Grouping import org.totschnig.myexpenses.provider.DataBaseAccount.Companion.HOME_AGGREGATE_ID @@ -120,18 +119,6 @@ data class Budget( else -> context.getString(grouping.getLabelForBudgetType()) } })" - - companion object { - val DIFF_CALLBACK = object : DiffUtil.ItemCallback() { - override fun areItemsTheSame(oldItem: Budget, newItem: Budget): Boolean { - return oldItem.id == newItem.id - } - - override fun areContentsTheSame(oldItem: Budget, newItem: Budget): Boolean { - return oldItem == newItem - } - } - } } fun Grouping.getLabelForBudgetType() = when (this) {