Make ProcessDueRecurringExpenses atomic with withTransaction #12

Closed
opened 2026-06-28 09:42:28 +00:00 by admin · 1 comment
Owner

Context

Review of PR #10 (feat/3-implement-recurring-interactors) flagged that ProcessDueRecurringExpenses.await performs expenseDao.insert(...) and recurringDao.update(...) outside a @Transaction boundary (app/src/main/java/dev/achmad/ledgerr/domain/recurring/interactor/ProcessDueRecurringExpenses.kt:27-32).

If the process is killed (or a coroutine is cancelled) between those two calls, the template's nextDueDate is not advanced and the next call will re-process the same template → duplicate expense in the user's list. There is no idempotency key on the generated Expense (the only link back to the template is recurringExpenseId), so the duplicate is invisible and persists.

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

What to do

  1. Inject AppDatabase into ProcessDueRecurringExpenses. The cleanest shape is to drop the two DAO constructor params and resolve 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 ...
            }
    }
    
  2. Import androidx.room.withTransaction.
  3. Update app/src/main/java/dev/achmad/ledgerr/di/DomainModule.kt to pass the database: factory { ProcessDueRecurringExpenses(get()) } (the AppDatabase is already a single in DataModule).
  4. Verify ./gradlew assembleDebug succeeds.

Trade-off

  • Pro: closes the duplicate-expense gap; aligns with the pattern used by ClearAllData.
  • Con: ProcessDueRecurringExpenses no longer takes DAOs directly. Per AGENTS.md, interactors normally take DAOs and not the database — ClearAllData is the existing exception. Acceptable for an interactor that needs cross-DAO atomicity.

Acceptance

  • ProcessDueRecurringExpenses.await runs the per-row insert + advance loop inside database.withTransaction { }
  • di/DomainModule.kt factory updated accordingly
  • ./gradlew assembleDebug succeeds

Implementation rule

Per AGENTS.md — do not start implementation without explicit user sign-off on this issue. When working, check for related issues in the remote repo first.

## Context Review of PR #10 (`feat/3-implement-recurring-interactors`) flagged that `ProcessDueRecurringExpenses.await` performs `expenseDao.insert(...)` and `recurringDao.update(...)` outside a `@Transaction` boundary (`app/src/main/java/dev/achmad/ledgerr/domain/recurring/interactor/ProcessDueRecurringExpenses.kt:27-32`). If the process is killed (or a coroutine is cancelled) between those two calls, the template's `nextDueDate` is not advanced and the next call will re-process the same template → **duplicate expense** in the user's list. There is no idempotency key on the generated `Expense` (the only link back to the template is `recurringExpenseId`), so the duplicate is invisible and persists. The codebase already uses `database.withTransaction { }` for `ClearAllData` (per `docs/03-function-todos.md:181`), so the pattern is established. ## What to do 1. Inject `AppDatabase` into `ProcessDueRecurringExpenses`. The cleanest shape is to drop the two DAO constructor params and resolve 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 ... } } ``` 2. Import `androidx.room.withTransaction`. 3. Update `app/src/main/java/dev/achmad/ledgerr/di/DomainModule.kt` to pass the database: `factory { ProcessDueRecurringExpenses(get()) }` (the `AppDatabase` is already a `single` in `DataModule`). 4. Verify `./gradlew assembleDebug` succeeds. ## Trade-off - **Pro:** closes the duplicate-expense gap; aligns with the pattern used by `ClearAllData`. - **Con:** `ProcessDueRecurringExpenses` no longer takes DAOs directly. Per AGENTS.md, interactors normally take DAOs and not the database — `ClearAllData` is the existing exception. Acceptable for an interactor that needs cross-DAO atomicity. ## Acceptance - [ ] `ProcessDueRecurringExpenses.await` runs the per-row insert + advance loop inside `database.withTransaction { }` - [ ] `di/DomainModule.kt` factory updated accordingly - [ ] `./gradlew assembleDebug` succeeds ## Implementation rule Per `AGENTS.md` — do not start implementation without explicit user sign-off on this issue. When working, check for related issues in the remote repo first.
Author
Owner

Closing — addressed as a review comment on PR #10 (#10) instead of a separate issue.

Closing — addressed as a review comment on PR #10 (https://git.achmad.dev/admin/ledgerr/pulls/10) instead of a separate issue.
admin closed this issue 2026-06-28 09:44:46 +00:00
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/ledgerr#12