admin/ledgerr
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Implement recurring interactors (#3) #10

Merged
admin merged 2 commits from feat/3-implement-recurring-interactors into main 2026-06-28 09:56:39 +00:00
Conversation 1 Commits 2 Files Changed 5
+28 -26
2 changed files with 28 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7bb65025a2 - Show all commits
app/src/main/java/dev/achmad/ledgerr/di/DomainModule.kt
+1 -1
View File
@@ -22,5 +22,5 @@ val domainModule = module {
factory { GetRecurringExpenses(get()) }
admin commented 2026-06-28 09:45:23 +00:00
Review
Copy Link
Copy Source

This is single-DAO, but docs/02-interfaces.md:107 and the original issue #3 step 3 both specify GetRecurringExpenses(get(), get()) with a CategoryDao. The simplification is correct — RecurringExpenseDao.subscribeAll() uses @Relation to join the category in one query (RecurringExpenseDao.kt:27-29), and awaitOne only needs the recurring entity — so a second CategoryDao would be redundant. GetExpenses keeps its two-DAO shape because ExpenseDao.getById / search do not use @Relation, so the asymmetry between the two interactors is justified by the DAO design, not a doc bug.

Suggested follow-up (out of scope here per the no-implementation rule)

Open a follow-up issue to update docs/02-interfaces.md:107 (and the original issue #3 if it is still open) to match the actual signature. Optionally add a one-line note in the doc explaining why GetRecurringExpenses is one-DAO while GetExpenses is two-DAO.

This is single-DAO, but `docs/02-interfaces.md:107` and the original issue #3 step 3 both specify `GetRecurringExpenses(get(), get())` with a `CategoryDao`. The simplification is correct — `RecurringExpenseDao.subscribeAll()` uses `@Relation` to join the category in one query (`RecurringExpenseDao.kt:27-29`), and `awaitOne` only needs the recurring entity — so a second `CategoryDao` would be redundant. `GetExpenses` keeps its two-DAO shape because `ExpenseDao.getById` / `search` do not use `@Relation`, so the asymmetry between the two interactors is justified by the DAO design, not a doc bug. ### Suggested follow-up (out of scope here per the no-implementation rule) Open a follow-up issue to update `docs/02-interfaces.md:107` (and the original issue #3 if it is still open) to match the actual signature. Optionally add a one-line note in the doc explaining *why* `GetRecurringExpenses` is one-DAO while `GetExpenses` is two-DAO.
factory { UpsertRecurringExpense(get()) }
factory { DeleteRecurringExpense(get()) }
factory { ProcessDueRecurringExpenses(get(), get()) }
factory { ProcessDueRecurringExpenses(get()) }
}
app/src/main/java/dev/achmad/ledgerr/domain/recurring/interactor/ProcessDueRecurringExpenses.kt
+27 -25
View File
@@ -1,36 +1,38 @@
package dev.achmad.ledgerr.domain.recurring.interactor
import dev.achmad.ledgerr.data.local.dao.ExpenseDao
import dev.achmad.ledgerr.data.local.dao.RecurringExpenseDao
import androidx.room.withTransaction
import dev.achmad.ledgerr.data.local.AppDatabase
import dev.achmad.ledgerr.data.local.mapper.toEntity
import dev.achmad.ledgerr.data.local.mapper.toModel
import dev.achmad.ledgerr.domain.expense.model.Expense
import java.time.LocalDate
class ProcessDueRecurringExpenses(
private val recurringDao: RecurringExpenseDao,
private val expenseDao: ExpenseDao,
private val database: AppDatabase,
) {
suspend fun await(today: LocalDate = LocalDate.now()): List<Expense> {
val dueTemplates = recurringDao.getDue(today.toEpochDay())
if (dueTemplates.isEmpty()) return emptyList()
val created = mutableListOf<Expense>()
for (templateEntity in dueTemplates) {
val template = templateEntity.toModel()
val expense = Expense(
amount = template.amount,
categoryId = template.categoryId,
date = template.nextDueDate,
note = template.note,
recurringExpenseId = template.id,
)
val newId = expenseDao.insert(expense.toEntity())
created += expense.copy(id = newId)
val advanced = template.copy(
nextDueDate = template.interval.advance(template.nextDueDate),
)
recurringDao.update(advanced.toEntity())
suspend fun await(today: LocalDate = LocalDate.now()): List<Expense> =
database.withTransaction {
val recurringDao = database.recurringExpenseDao()
val expenseDao = database.expenseDao()
val dueTemplates = recurringDao.getDue(today.toEpochDay())
if (dueTemplates.isEmpty()) return@withTransaction emptyList()
val created = mutableListOf<Expense>()
for (templateEntity in dueTemplates) {
val template = templateEntity.toModel()
val expense = Expense(
amount = template.amount,
categoryId = template.categoryId,
date = template.nextDueDate,
note = template.note,
recurringExpenseId = template.id,
)
val newId = expenseDao.insert(expense.toEntity())
created += expense.copy(id = newId)
val advanced = template.copy(
nextDueDate = template.interval.advance(template.nextDueDate),
admin commented 2026-06-28 09:45:23 +00:00
Outdated
Review
Copy Link
Copy Source

The insert + advance pair is not atomic. If the process is killed (or a coroutine is cancelled) between expenseDao.insert and recurringDao.update, the template's nextDueDate is not advanced and the next call will re-process the same template → duplicate expense with no idempotency key (the only link back to the template is recurringExpenseId, which is not unique).

The codebase already uses database.withTransaction { } for ClearAllData (per docs/03-function-todos.md:181), so the pattern is established.

Suggested fix

Inject AppDatabase and wrap the per-row loop in database.withTransaction { ... }. The cleanest shape drops the two DAO constructor params and resolves them from the database inside the transaction block:

class ProcessDueRecurringExpenses(
    private val database: AppDatabase,
) {
    suspend fun await(today: LocalDate = LocalDate.now()): List<Expense> =
        database.withTransaction {
            val recurringDao = database.recurringExpenseDao()
            val expenseDao = database.expenseDao()
            // ... existing loop body, unchanged ...
        }
}

Then DomainModule becomes factory { ProcessDueRecurringExpenses(get()) } (the AppDatabase is already a single in DataModule).

Trade-off

Per AGENTS.md, interactors normally take DAOs and not the database. ClearAllData is the existing exception because it also needs cross-DAO atomicity. Same justification applies here.

The insert + advance pair is **not atomic**. If the process is killed (or a coroutine is cancelled) between `expenseDao.insert` and `recurringDao.update`, the template's `nextDueDate` is not advanced and the next call will re-process the same template → **duplicate expense** with no idempotency key (the only link back to the template is `recurringExpenseId`, which is not unique). The codebase already uses `database.withTransaction { }` for `ClearAllData` (per `docs/03-function-todos.md:181`), so the pattern is established. ### Suggested fix Inject `AppDatabase` and wrap the per-row loop in `database.withTransaction { ... }`. The cleanest shape drops the two DAO constructor params and resolves them from the database inside the transaction block: ```kotlin class ProcessDueRecurringExpenses( private val database: AppDatabase, ) { suspend fun await(today: LocalDate = LocalDate.now()): List<Expense> = database.withTransaction { val recurringDao = database.recurringExpenseDao() val expenseDao = database.expenseDao() // ... existing loop body, unchanged ... } } ``` Then `DomainModule` becomes `factory { ProcessDueRecurringExpenses(get()) }` (the `AppDatabase` is already a `single` in `DataModule`). ### Trade-off Per AGENTS.md, interactors normally take DAOs and not the database. `ClearAllData` is the existing exception because it also needs cross-DAO atomicity. Same justification applies here.
)
recurringDao.update(advanced.toEntity())
}
created
}
return created
}
}