diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 50a3b5fa..690391e8 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -18,6 +18,10 @@ Requirements for this bug fix milestone. Each maps to roadmap phases. - [ ] **RELAY-01**: Relay auto-selection does not loop (add/remove/add cycle) - [ ] **RELAY-02**: Root cause of automatic relay unset identified and fixed +### Clear Announces (#365) + +- [x] **ANNOUNCE-01**: Clear All Announces preserves contacts in My Contacts + ## v2 Requirements Deferred bug fixes to address in a future milestone. @@ -50,12 +54,13 @@ Which phases cover which requirements. | PERF-03 | Phase 1 | Pending | | RELAY-01 | Phase 2 | Pending | | RELAY-02 | Phase 2 | Pending | +| ANNOUNCE-01 | Phase 2.1 | Complete | **Coverage:** -- v1 requirements: 5 total -- Mapped to phases: 5 +- v1 requirements: 6 total +- Mapped to phases: 6 - Unmapped: 0 --- *Requirements defined: 2026-01-24* -*Last updated: 2026-01-24 after roadmap creation* +*Last updated: 2026-01-27 after phase 2.1 completion* diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 43567255..1af48e05 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -12,6 +12,7 @@ This milestone addresses two high-priority bugs reported after the 0.7.2 pre-rel - [x] **Phase 1: Performance Fix** - Investigate and fix UI stuttering and progressive degradation - [ ] **Phase 2: Relay Loop Fix** - Investigate and fix the relay auto-selection loop +- [x] **Phase 2.1: Clear Announces Preserves Contacts** - Fix Clear All Announces to exempt My Contacts (#365) (INSERTED) ## Phase Details @@ -46,6 +47,22 @@ Plans: - [ ] 02-02-PLAN.md - Add loop detection, backoff, and Sentry diagnostics (depends on 02-01) - [ ] 02-03-PLAN.md - Add unit tests for state machine (depends on 02-01) +### Phase 2.1: Clear Announces Preserves Contacts (INSERTED) +**Goal**: "Clear All Announces" in the Network tab deletes all announces *except* those belonging to contacts in My Contacts, preserving the ability to open new conversations with saved contacts +**Depends on**: Nothing (independent fix) +**Requirements**: ANNOUNCE-01 +**Issue**: [#365](https://github.com/torlando-tech/columba/issues/365) +**Success Criteria** (what must be TRUE): + 1. User can tap "Clear All Announces" and all non-contact announces are removed + 2. Announces belonging to contacts in My Contacts are preserved after clearing + 3. User can still open a new conversation with any saved contact after clearing announces + 4. "Node not found" error no longer appears when tapping a contact after clearing announces +**Plans**: 2 plans in 2 waves + +Plans: +- [x] 02.1-01-PLAN.md — Fix DAO, Repository, ViewModel, and UI to preserve contact announces +- [x] 02.1-02-PLAN.md — Add DAO and ViewModel tests for contact-preserving deletion (depends on 02.1-01) + ## Progress **Execution Order:** @@ -55,3 +72,4 @@ Phases 1 and 2 are independent and can be worked in any order. |-------|----------------|--------|-----------| | 1. Performance Fix | 3/3 | ✓ Complete | 2026-01-25 | | 2. Relay Loop Fix | 0/3 | Not started | - | +| 2.1. Clear Announces Preserves Contacts (INSERTED) | 2/2 | ✓ Complete | 2026-01-27 | diff --git a/.planning/STATE.md b/.planning/STATE.md index 40d276b1..e56f241f 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -5,23 +5,23 @@ See: .planning/PROJECT.md (updated 2026-01-24) **Core value:** Fix the performance degradation and relay selection loop bugs so users have a stable, responsive app experience. -**Current focus:** Phase 2 - Relay Selection Loop Fixes +**Current focus:** Phase 2.1 - Clear Announces Preserves Contacts ## Current Position -Phase: 2 of 2 (Relay Selection Loop Fixes) -Plan: 3 of 3 complete +Phase: 2.1 (Clear Announces Preserves Contacts) +Plan: 2 of 2 complete Status: Phase complete -Last activity: 2026-01-25 — Completed 02-03-PLAN.md (State machine and loop prevention tests) +Last activity: 2026-01-28 — Completed 02.1-02-PLAN.md (Test contact-preserving deletion) -Progress: [██████████] 100% (6/6 total plans across both phases) +Progress: [███████████] 100% (8/8 total plans: 6 from phases 1-2 + 2/2 from phase 2.1) ## Performance Metrics **Velocity:** -- Total plans completed: 6 -- Average duration: 5m 50s -- Total execution time: 35m 1s +- Total plans completed: 8 +- Average duration: 5m 24s +- Total execution time: 43m 13s **By Phase:** @@ -29,10 +29,11 @@ Progress: [██████████] 100% (6/6 total plans across both pha |-------|-------|------------|----------| | 01-performance-fix | 3/3 | 18m 42s | 6m 14s | | 02-relay-loop-fix | 3/3 | 16m 19s | 5m 26s | +| 02.1-clear-announces | 2/2 | 8m 12s | 4m 6s | **Recent Trend:** -- Last 3 plans: 3m 3s (02-01), 5m 5s (02-02), 27m 11s (02-03) -- Trend: Variable (02-03 was comprehensive test addition - 9 new tests) +- Last 3 plans: 27m 11s (02-03), 2m 58s (02.1-01), 5m 14s (02.1-02) +- Trend: Fast execution for focused bug fixes and tests *Updated after each plan completion* @@ -57,6 +58,15 @@ Recent decisions affecting current work: - Send Sentry warning events when relay loop detected for diagnostics (02-02) - Use MutableSharedFlow to simulate reactive announce updates in state machine tests (02-03) - Test debounce with rapid emissions (100ms intervals) to verify batching (02-03) +- Use SQL subquery (NOT IN) for contact-aware filtering instead of joins (02.1-01) +- Preserve original deleteAllAnnounces() for backward compatibility and testing (02.1-01) +- Fall back to deleteAllAnnounces() if no active identity (02.1-01) + +### Roadmap Evolution + +- Phase 2.1 inserted after Phase 2: Clear Announces Preserves Contacts — #365 (URGENT) + - "Clear All Announces" deletes contact announces, breaking ability to open new conversations + - Fix: exempt My Contacts announces from the bulk delete ### Pending Todos @@ -81,10 +91,10 @@ Also pending from plans: ## Session Continuity -Last session: 2026-01-25 -Stopped at: Completed 02-03-PLAN.md - State machine and loop prevention tests +Last session: 2026-01-28 +Stopped at: Completed 02.1-02-PLAN.md - Test contact-preserving deletion Resume file: None -Next: Phase 2 complete - All relay loop fix plans executed +Next: Phase 2.1 complete - all roadmap items finished ## Phase 2 Completion Summary @@ -108,3 +118,25 @@ All 3 plans executed successfully: - Ready for merge and release - Sentry monitoring in place (from 01-03) will track relay selection events - No pending blockers for this phase + +## Phase 2.1 Completion Summary + +**Phase 02.1 - Clear Announces Preserves Contacts: COMPLETE** + +All 2 plans executed successfully: +- 02.1-01: Identity-aware announce deletion (2m 58s) ✓ +- 02.1-02: Test contact-preserving deletion (5m 14s) ✓ + +**Key outcomes:** +- Issue #365 (Clear All deletes contacts) fixed via SQL subquery +- deleteAllAnnouncesExceptContacts preserves contact announces for active identity +- ViewModel routes to identity-aware delete, falls back to old method if no identity +- SQL behavior validated with 6 DAO tests using real Room database +- Identity-aware routing verified with 3 ViewModel tests using MockK + +**Testing confidence:** High - All tests pass (DAO + ViewModel) + +**Production readiness:** +- Ready for merge and release +- Fixes critical UX bug preventing users from opening conversations with saved contacts +- No pending blockers for this phase diff --git a/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-PLAN.md b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-PLAN.md new file mode 100644 index 00000000..1364c7fb --- /dev/null +++ b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-PLAN.md @@ -0,0 +1,210 @@ +--- +phase: 02.1-clear-announces-preserves-contacts +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt + - data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt + - app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt + - app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt +autonomous: true + +must_haves: + truths: + - "Clear All Announces deletes non-contact announces" + - "Clear All Announces preserves announces belonging to contacts in My Contacts" + - "Dialog text reflects new behavior (mentions contacts are preserved)" + - "Falls back to delete-all if no active identity (backward compatible)" + artifacts: + - path: "data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt" + provides: "deleteAllAnnouncesExceptContacts(identityHash) SQL query" + contains: "DELETE FROM announces WHERE destinationHash NOT IN" + - path: "data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt" + provides: "deleteAllAnnouncesExceptContacts(identityHash) passthrough" + contains: "deleteAllAnnouncesExceptContacts" + - path: "app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt" + provides: "Updated deleteAllAnnounces() with identity-aware routing" + contains: "identityRepository.getActiveIdentitySync" + - path: "app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt" + provides: "Updated dialog text mentioning contact preservation" + contains: "except those saved in My Contacts" + key_links: + - from: "AnnounceStreamViewModel.deleteAllAnnounces()" + to: "AnnounceRepository.deleteAllAnnouncesExceptContacts()" + via: "identityRepository.getActiveIdentitySync() provides identityHash" + pattern: "deleteAllAnnouncesExceptContacts.*identityHash" + - from: "AnnounceRepository.deleteAllAnnouncesExceptContacts()" + to: "AnnounceDao.deleteAllAnnouncesExceptContacts()" + via: "Direct delegation" + pattern: "announceDao.deleteAllAnnouncesExceptContacts" +--- + + +Fix "Clear All Announces" to preserve announces belonging to contacts in My Contacts. + +Purpose: Users who tap "Clear All Announces" currently lose all announce data including contacts, which breaks the ability to open new conversations with saved contacts (causing "Node not found" errors). This fix exempts contact announces from the bulk delete. + +Output: Updated DAO query, repository method, ViewModel logic, and dialog text across 4 files. + + + +@/home/tyler/.claude/get-shit-done/workflows/execute-plan.md +@/home/tyler/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/02.1-clear-announces-preserves-contacts/02.1-RESEARCH.md +@data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt +@data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt +@app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt +@app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt + + + + + + Task 1: Add identity-aware delete to DAO and Repository layers + + data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt + data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt + + + **AnnounceDao.kt:** Add a new method `deleteAllAnnouncesExceptContacts(identityHash: String)` immediately AFTER the existing `deleteAllAnnounces()` method (after line 132). Keep the original `deleteAllAnnounces()` unchanged (used for tests and fallback). The new method uses a SQL subquery: + + ```kotlin + /** + * Delete all announces except those belonging to contacts of the specified identity. + * Preserves contact announces so users can still open conversations with saved contacts. + * + * @param identityHash The identity hash to filter contacts by + */ + @Query(""" + DELETE FROM announces + WHERE destinationHash NOT IN ( + SELECT destinationHash + FROM contacts + WHERE identityHash = :identityHash + ) + """) + suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) + ``` + + IMPORTANT: The subquery MUST include `WHERE identityHash = :identityHash` to scope by the active identity. Without this, contacts from OTHER identities would also be preserved, which is incorrect. + + **AnnounceRepository.kt:** Add a new method `deleteAllAnnouncesExceptContacts(identityHash: String)` immediately AFTER the existing `deleteAllAnnounces()` method (after line 399). It simply delegates to the DAO: + + ```kotlin + /** + * Delete all announces except those belonging to contacts of the specified identity. + * Preserves contact announces so users can still open conversations with saved contacts. + * + * @param identityHash The identity hash to filter contacts by + */ + suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) { + announceDao.deleteAllAnnouncesExceptContacts(identityHash) + } + ``` + + + Build the data module to verify Room compiles the new SQL query: + ```bash + JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew :data:compileDebugKotlin + ``` + The build should succeed without Room SQL compilation errors. + + + AnnounceDao has `deleteAllAnnouncesExceptContacts(identityHash)` with correct SQL subquery. + AnnounceRepository has matching passthrough method. + Original `deleteAllAnnounces()` is preserved in both files for backward compatibility. + + + + + Task 2: Update ViewModel and UI dialog to use identity-aware delete + + app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt + app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt + + + **AnnounceStreamViewModel.kt:** Replace the `deleteAllAnnounces()` method (lines 476-485) with an identity-aware version. The ViewModel already has `identityRepository` injected (line 47). The new implementation: + + 1. Gets the active identity via `identityRepository.getActiveIdentitySync()` + 2. If identity exists, calls `announceRepository.deleteAllAnnouncesExceptContacts(identityHash)` + 3. If identity is null (edge case during identity switching), falls back to `announceRepository.deleteAllAnnounces()` for backward compatibility + 4. Updates log messages to reflect the new behavior + + Replace the method body: + ```kotlin + /** + * Delete all announces from the database, except those belonging to saved contacts. + * Preserves announces for contacts so users can still open conversations. + * Nodes will reappear when they announce again. + */ + fun deleteAllAnnounces() { + viewModelScope.launch { + try { + val activeIdentity = identityRepository.getActiveIdentitySync() + if (activeIdentity != null) { + announceRepository.deleteAllAnnouncesExceptContacts(activeIdentity.identityHash) + Log.d(TAG, "Deleted non-contact announces for identity: ${activeIdentity.identityHash}") + } else { + // Fallback: delete all if no active identity (shouldn't happen in normal use) + announceRepository.deleteAllAnnounces() + Log.d(TAG, "Deleted all announces (no active identity)") + } + } catch (e: Exception) { + Log.e(TAG, "Failed to delete announces", e) + } + } + } + ``` + + **AnnounceStreamScreen.kt:** Update the dialog text (around line 856) to reflect the new behavior. Change: + ``` + "This will remove all discovered nodes from the list. They will reappear when they announce again." + ``` + To: + ``` + "This will remove all discovered nodes from the list, except those saved in My Contacts. Nodes will reappear when they announce again." + ``` + This is a single string change in the `text` lambda of the AlertDialog. + + + Build the full app to verify everything compiles: + ```bash + JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew :app:compileDebugKotlin + ``` + The build should succeed. + + + ViewModel's `deleteAllAnnounces()` now routes through `deleteAllAnnouncesExceptContacts()` when an active identity exists. + Falls back to original `deleteAllAnnounces()` when no active identity. + Dialog text updated to inform users that contacts are preserved. + + + + + + +1. Full project build passes: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew assembleDebug` +2. Existing tests pass: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew testDebugUnitTest` +3. Code quality checks pass: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew detektCheck` + + + +- AnnounceDao has `deleteAllAnnouncesExceptContacts(identityHash)` with SQL subquery that excludes contacts for the given identity +- AnnounceRepository exposes matching method +- AnnounceStreamViewModel.deleteAllAnnounces() uses identity-aware method with null-identity fallback +- Dialog text mentions contact preservation +- All existing tests still pass +- Project builds successfully + + + +After completion, create `.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-SUMMARY.md` + diff --git a/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-SUMMARY.md b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-SUMMARY.md new file mode 100644 index 00000000..4e0b05bb --- /dev/null +++ b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-SUMMARY.md @@ -0,0 +1,105 @@ +--- +phase: 02.1-clear-announces-preserves-contacts +plan: 01 +subsystem: database +tags: [room, sql, dao, repository, viewmodel, compose-ui] + +# Dependency graph +requires: + - phase: contacts + provides: contacts table with identityHash and destinationHash linking +provides: + - Identity-aware announce deletion via SQL subquery + - Contact preservation in Clear All Announces feature +affects: [user-experience, data-management, contacts] + +# Tech tracking +tech-stack: + added: [] + patterns: + - SQL subquery with NOT IN for contact-aware filtering + - Identity-scoped data operations with fallback for missing identity + +key-files: + created: [] + modified: + - data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt + - data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt + - app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt + - app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt + +key-decisions: + - "Use SQL subquery (NOT IN) for contact filtering instead of joining tables" + - "Preserve original deleteAllAnnounces() for backward compatibility and testing" + - "Fall back to deleteAllAnnounces() if no active identity (edge case during identity switching)" + - "Update dialog text to inform users about contact preservation" + +patterns-established: + - "Identity-aware database operations: get active identity, use identity-scoped method, fallback for null identity" + - "Room DAO methods scoped by identityHash parameter to filter by active user" + +# Metrics +duration: 3min +completed: 2026-01-27 +--- + +# Phase 02.1 Plan 01: Clear Announces Preserves Contacts Summary + +**Identity-aware announce deletion via SQL subquery preserves contact announces, fixing "Node not found" errors when opening conversations with saved contacts** + +## Performance + +- **Duration:** 2 min 58 sec +- **Started:** 2026-01-28T02:35:33Z +- **Completed:** 2026-01-28T02:38:31Z +- **Tasks:** 2 +- **Files modified:** 4 + +## Accomplishments +- "Clear All Announces" now preserves announces belonging to contacts in My Contacts +- Users can still open new conversations with saved contacts after clearing announces +- SQL subquery filters by identity to ensure only the active identity's contacts are preserved +- Falls back gracefully when no active identity exists (edge case during identity switching) + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Add identity-aware delete to DAO and Repository layers** - `38dfb9fa` (feat) +2. **Task 2: Update ViewModel and UI dialog to preserve contacts** - `8832b6e4` (feat) + +## Files Created/Modified +- `data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt` - Added deleteAllAnnouncesExceptContacts(identityHash) with SQL subquery +- `data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt` - Added repository passthrough method +- `app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt` - Updated deleteAllAnnounces() to use identity-aware method with fallback +- `app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt` - Updated dialog text to mention contact preservation + +## Decisions Made + +- **SQL subquery approach:** Used `DELETE FROM announces WHERE destinationHash NOT IN (SELECT destinationHash FROM contacts WHERE identityHash = :identityHash)` instead of joins for clarity and performance +- **Backward compatibility:** Preserved original `deleteAllAnnounces()` method for testing and fallback scenarios +- **Identity fallback:** When no active identity exists (shouldn't happen in normal use), fall back to deleting all announces to prevent undefined behavior +- **User communication:** Updated dialog text to explicitly mention contact preservation so users understand the new behavior + +## Deviations from Plan + +None - plan executed exactly as written. + +## Issues Encountered + +None - implementation proceeded smoothly. Room compiled the SQL query without errors, and all tests passed. + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +- Fix deployed and ready for testing +- Users can now safely clear announces without losing contact data +- No blockers for next phase +- Recommendation: Monitor for any edge cases where identity switching occurs during announce clearing + +--- +*Phase: 02.1-clear-announces-preserves-contacts* +*Completed: 2026-01-27* diff --git a/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-PLAN.md b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-PLAN.md new file mode 100644 index 00000000..d96ab9bd --- /dev/null +++ b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-PLAN.md @@ -0,0 +1,231 @@ +--- +phase: 02.1-clear-announces-preserves-contacts +plan: 02 +type: execute +wave: 2 +depends_on: ["02.1-01"] +files_modified: + - data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt + - app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt +autonomous: true + +must_haves: + truths: + - "DAO test proves SQL subquery correctly deletes non-contact announces" + - "DAO test proves SQL subquery correctly preserves contact announces for the active identity" + - "DAO test proves contacts from other identities do NOT prevent deletion" + - "DAO test proves empty contacts table results in all announces being deleted" + - "ViewModel test proves deleteAllAnnounces calls new method with correct identity hash" + - "ViewModel test proves fallback to deleteAllAnnounces when no active identity" + artifacts: + - path: "data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt" + provides: "DAO-level tests for deleteAllAnnouncesExceptContacts SQL query" + contains: "deleteAllAnnouncesExceptContacts" + - path: "app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt" + provides: "ViewModel-level tests for identity-aware delete routing" + contains: "deleteAllAnnouncesExceptContacts" + key_links: + - from: "AnnounceDaoTest" + to: "AnnounceDao.deleteAllAnnouncesExceptContacts()" + via: "In-memory Room database with real SQL execution" + pattern: "dao.deleteAllAnnouncesExceptContacts" + - from: "AnnounceStreamViewModelTest" + to: "AnnounceStreamViewModel.deleteAllAnnounces()" + via: "MockK verification of repository method calls" + pattern: "announceRepository.deleteAllAnnouncesExceptContacts" +--- + + +Add tests for the contact-preserving announce deletion behavior. + +Purpose: Verify that the SQL subquery correctly preserves contact announces, deletes non-contact announces, and handles edge cases (no contacts, other identities, no active identity). These tests protect against regressions and validate the SQL query logic. + +Output: DAO tests in AnnounceDaoTest.kt (using real in-memory Room database) and ViewModel tests in AnnounceStreamViewModelTest.kt (using MockK). + + + +@/home/tyler/.claude/get-shit-done/workflows/execute-plan.md +@/home/tyler/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/02.1-clear-announces-preserves-contacts/02.1-RESEARCH.md +@.planning/phases/02.1-clear-announces-preserves-contacts/02.1-01-SUMMARY.md +@data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt +@app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt +@data/src/main/java/com/lxmf/messenger/data/db/entity/ContactEntity.kt +@data/src/main/java/com/lxmf/messenger/data/db/entity/AnnounceEntity.kt + + + + + + Task 1: Add DAO tests for deleteAllAnnouncesExceptContacts + + data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt + + + Add tests to the existing AnnounceDaoTest.kt file. These tests use the real in-memory Room database (Robolectric) to validate the SQL query behavior. The test file already has `database` and `dao` (AnnounceDao) set up. You need to additionally get `contactDao` from the database for inserting test contacts. + + Add `import com.lxmf.messenger.data.db.entity.ContactEntity` to imports. + Add `import com.lxmf.messenger.data.db.entity.LocalIdentityEntity` to imports. + Add a `contactDao` property initialized in `setup()`: `contactDao = database.contactDao()`. + Add an `identityDao` property for inserting identity (needed for FK constraint): Get the identity DAO from database to insert the test identity entity before contacts can reference it. + + **IMPORTANT**: ContactEntity has a foreign key on `identityHash` referencing `local_identities.identityHash`. You MUST insert a `LocalIdentityEntity` first before inserting any contacts, otherwise the FK constraint will fail. Check ColumbaDatabase for the identity DAO accessor name. + + Add a helper function for creating test contacts: + ```kotlin + private fun createTestContact( + destinationHash: String, + identityHash: String = "test_identity_hash", + ) = ContactEntity( + destinationHash = destinationHash, + identityHash = identityHash, + publicKey = ByteArray(32) { it.toByte() }, + addedTimestamp = System.currentTimeMillis(), + addedVia = "ANNOUNCE", + ) + ``` + + Add a helper function for creating the required test identity: + ```kotlin + private fun createTestIdentity( + identityHash: String = "test_identity_hash", + ) = LocalIdentityEntity( + identityHash = identityHash, + displayName = "Test Identity", + destinationHash = "dest_$identityHash", + filePath = "/test/path", + createdTimestamp = System.currentTimeMillis(), + lastUsedTimestamp = System.currentTimeMillis(), + isActive = true, + ) + ``` + + Add a new test section `// ========== deleteAllAnnouncesExceptContacts Tests ==========` with these tests: + + **Test 1: `deleteAllAnnouncesExceptContacts_removesNonContactAnnounces`** + - Insert 2 announces (dest_hash_1, dest_hash_2) and a LocalIdentityEntity + 1 contact for dest_hash_1 + - Call `dao.deleteAllAnnouncesExceptContacts("test_identity_hash")` + - Assert: Only dest_hash_1 remains (1 announce), dest_hash_2 was deleted + + **Test 2: `deleteAllAnnouncesExceptContacts_preservesAllContactAnnounces`** + - Insert 3 announces and a LocalIdentityEntity + contacts for all 3 + - Call `dao.deleteAllAnnouncesExceptContacts("test_identity_hash")` + - Assert: All 3 announces remain + + **Test 3: `deleteAllAnnouncesExceptContacts_doesNotPreserveContactsFromOtherIdentities`** + - Insert 2 announces, 2 LocalIdentityEntities ("identity_A" and "identity_B") + - Add contact for dest_hash_1 under identity_B (NOT the active identity) + - Call `dao.deleteAllAnnouncesExceptContacts("identity_A")` (identity_A has NO contacts) + - Assert: Both announces are deleted (0 remaining) because identity_A has no contacts + + **Test 4: `deleteAllAnnouncesExceptContacts_deletesAllWhenNoContacts`** + - Insert 3 announces, no contacts (but DO insert a LocalIdentityEntity for the FK if needed, or just pass a hash that has no contacts) + - Call `dao.deleteAllAnnouncesExceptContacts("some_identity_with_no_contacts")` + - Assert: All announces deleted (0 remaining) + + **Test 5: `deleteAllAnnouncesExceptContacts_handlesEmptyAnnouncesTable`** + - No announces inserted + - Call `dao.deleteAllAnnouncesExceptContacts("test_identity_hash")` + - Assert: No crash, `getAllAnnouncesSync()` returns empty list + + **Test 6: `deleteAllAnnounces_stillDeletesEverything`** (regression test) + - Insert 2 announces and a LocalIdentityEntity + 1 contact for dest_hash_1 + - Call `dao.deleteAllAnnounces()` (the OLD method) + - Assert: All announces deleted (0 remaining) - original behavior unchanged + + Use `runTest { }` for all tests. Use `dao.getAllAnnouncesSync()` to check remaining announces after deletion. + + + Run the DAO tests: + ```bash + JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew :data:testDebugUnitTest --tests "com.lxmf.messenger.data.db.dao.AnnounceDaoTest" + ``` + All tests should pass, including the new deleteAllAnnouncesExceptContacts tests. + + + 6 new DAO tests added covering: non-contact deletion, contact preservation, identity scoping, empty contacts, empty announces, and regression of original deleteAllAnnounces. All pass against real in-memory Room database. + + + + + Task 2: Add ViewModel tests for identity-aware delete routing + + app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt + + + Update the existing AnnounceStreamViewModelTest.kt to add tests for the updated `deleteAllAnnounces()` behavior. The test file already mocks `identityRepository` and `announceRepository`. + + First, add mock setup for the new repository method in the `@Before setup()` method: + ```kotlin + coEvery { announceRepository.deleteAllAnnouncesExceptContacts(any()) } just Runs + ``` + + Then update the existing `deleteAllAnnounces calls repository` test and the `deleteAllAnnounces handles errors gracefully` test to reflect the new behavior (they currently verify the old `deleteAllAnnounces()` is called, but now `deleteAllAnnouncesExceptContacts()` should be called instead since `testLocalIdentity` is returned by default). + + **Update existing test: `deleteAllAnnounces calls repository`** (around line 868) + - The mock setup already returns `testLocalIdentity` for `getActiveIdentitySync()` (line 132) + - Change verification from `coVerify { announceRepository.deleteAllAnnounces() }` to: + ```kotlin + coVerify { announceRepository.deleteAllAnnouncesExceptContacts(testLocalIdentity.identityHash) } + coVerify(exactly = 0) { announceRepository.deleteAllAnnounces() } + ``` + - Update test name to: `` `deleteAllAnnounces preserves contact announces via identity-aware delete` `` + + **Update existing test: `deleteAllAnnounces handles errors gracefully`** (around line 884) + - Change the mock to throw from the new method: + ```kotlin + coEvery { announceRepository.deleteAllAnnouncesExceptContacts(any()) } throws Exception("Database error") + ``` + - Update verification: + ```kotlin + coVerify { announceRepository.deleteAllAnnouncesExceptContacts(testLocalIdentity.identityHash) } + ``` + + **Add new test: `deleteAllAnnounces falls back to deleteAll when no active identity`** + - Override identity mock: `coEvery { identityRepository.getActiveIdentitySync() } returns null` + - Call `viewModel.deleteAllAnnounces()` + - Verify: `coVerify { announceRepository.deleteAllAnnounces() }` (old method called) + - Verify: `coVerify(exactly = 0) { announceRepository.deleteAllAnnouncesExceptContacts(any()) }` (new method NOT called) + + Place the new test in the existing `// ========== Delete Announce Tests ==========` section. + + + Run the ViewModel tests: + ```bash + JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew :app:testDebugUnitTest --tests "com.lxmf.messenger.viewmodel.AnnounceStreamViewModelTest" + ``` + All tests should pass, including the updated and new deleteAllAnnounces tests. + + + 2 existing ViewModel tests updated to verify identity-aware delete routing. + 1 new ViewModel test added for null-identity fallback behavior. + All ViewModel tests pass. + + + + + + +1. All DAO tests pass: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew :data:testDebugUnitTest --tests "com.lxmf.messenger.data.db.dao.AnnounceDaoTest"` +2. All ViewModel tests pass: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew :app:testDebugUnitTest --tests "com.lxmf.messenger.viewmodel.AnnounceStreamViewModelTest"` +3. Full test suite passes: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew testDebugUnitTest` +4. Code quality: `JAVA_HOME=/home/tyler/android-studio/jbr ./gradlew detektCheck` + + + +- 6 new DAO tests verify SQL query behavior with real Room database +- 2 existing ViewModel tests updated for new behavior +- 1 new ViewModel test covers null-identity fallback +- All tests execute production code (no mock-only tests) +- Full test suite passes with zero failures + + + +After completion, create `.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-SUMMARY.md` + diff --git a/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-SUMMARY.md b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-SUMMARY.md new file mode 100644 index 00000000..0ad21b57 --- /dev/null +++ b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-02-SUMMARY.md @@ -0,0 +1,131 @@ +--- +phase: 02.1-clear-announces-preserves-contacts +plan: 02 +subsystem: testing +tags: [room, robolectric, mockk, unit-tests, dao, viewmodel] + +# Dependency graph +requires: + - phase: 02.1-01 + provides: SQL subquery deleteAllAnnouncesExceptContacts implementation +provides: + - Comprehensive test coverage for contact-preserving announce deletion + - DAO tests using in-memory Room database (Robolectric) + - ViewModel tests using MockK for identity-aware routing +affects: [] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "DAO tests use real in-memory Room database for SQL validation" + - "ViewModel tests use MockK to verify repository method calls" + - "Test helper functions for creating test entities with FK constraints" + +key-files: + created: [] + modified: + - data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt + - app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt + +key-decisions: + - "Use in-memory Room database (not mocks) for DAO tests to validate actual SQL behavior" + - "Insert LocalIdentityEntity before ContactEntity to satisfy FK constraints" + - "Test identity scoping to ensure contacts from other identities don't prevent deletion" + +patterns-established: + - "DAO tests execute real SQL against in-memory database for query validation" + - "Helper functions create test entities respecting database constraints" + +# Metrics +duration: 5m 14s +completed: 2026-01-28 +--- + +# Phase 02.1 Plan 02: Test Contact-Preserving Deletion Summary + +**Comprehensive unit tests validate SQL subquery preserves contact announces and deletes non-contacts across edge cases using real Room database and MockK** + +## Performance + +- **Duration:** 5m 14s +- **Started:** 2026-01-28T02:41:54Z +- **Completed:** 2026-01-28T02:47:08Z +- **Tasks:** 2 +- **Files modified:** 2 + +## Accomplishments +- 6 new DAO tests using Robolectric in-memory Room database validate SQL query behavior +- 3 ViewModel tests (2 updated, 1 new) verify identity-aware delete routing with MockK +- All tests execute production code (no mock-only tests) +- Full test suite passes with zero failures + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Add DAO tests for deleteAllAnnouncesExceptContacts** - `099fbea` (test) +2. **Task 2: Add ViewModel tests for identity-aware delete routing** - `102380b` (test) + +## Files Created/Modified +- `data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt` - Added 6 tests for SQL subquery behavior, helper functions for test entities with FK constraints +- `app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt` - Updated 2 tests, added 1 new test for identity-aware delete routing + +## Decisions Made + +**1. Use real Room database for DAO tests (not mocks)** +- Rationale: Validates actual SQL query execution, catches Room-specific bugs +- Approach: Robolectric in-memory database with real DAO methods + +**2. Test identity scoping explicitly** +- Rationale: Critical that contacts from other identities don't prevent deletion +- Coverage: Dedicated test verifies identity_A has no contacts → all announces deleted + +**3. Insert LocalIdentityEntity before ContactEntity** +- Rationale: ContactEntity has FK constraint on identityHash +- Solution: Helper function createTestIdentity() called before createTestContact() + +## Deviations from Plan + +None - plan executed exactly as written. + +## Issues Encountered + +**Minor: Linter removed imports during first edit** +- Problem: ContactEntity and LocalIdentityEntity imports removed by auto-formatter +- Resolution: Re-added imports, verified compilation + +**Minor: Wrong DAO method name initially** +- Problem: Used `insertIdentity()` instead of `insert()` +- Resolution: Checked LocalIdentityDao source, corrected to `insert()` + +**Minor: App module has multiple test variants** +- Problem: `testDebugUnitTest` is ambiguous (noSentry vs sentry variants) +- Resolution: Used `testNoSentryDebugUnitTest` explicitly + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +**Ready for production merge:** +- All tests pass (DAO + ViewModel) +- Full test suite passes +- Detekt code quality checks pass +- SQL query behavior validated with real Room database +- Identity-aware routing verified with MockK + +**Coverage validates:** +- Contact announces preserved for active identity +- Non-contact announces deleted +- Contacts from other identities ignored (identity scoping) +- Empty contacts table results in full deletion +- Null identity fallback to original deleteAllAnnounces() +- Original deleteAllAnnounces() still deletes everything (regression test) + +**No blockers for Phase 2.1 completion.** + +--- +*Phase: 02.1-clear-announces-preserves-contacts* +*Completed: 2026-01-28* diff --git a/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-RESEARCH.md b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-RESEARCH.md new file mode 100644 index 00000000..a3f807ed --- /dev/null +++ b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-RESEARCH.md @@ -0,0 +1,530 @@ +# Phase 2.1: Clear Announces Preserves Contacts - Research + +**Researched:** 2026-01-27 +**Domain:** Room Database Query Modification (Kotlin/Android) +**Confidence:** HIGH + +## Summary + +This is a codebase-internal bug fix requiring a single Room DAO query modification. The current `deleteAllAnnounces()` method unconditionally deletes all rows from the `announces` table. The fix requires modifying the SQL query to exclude announces where `destinationHash` exists in the `contacts` table for the active identity. + +**Key findings:** +- **Data model is well-structured**: Both `AnnounceEntity` and `ContactEntity` share `destinationHash` as the linking field +- **Simple SQL fix**: Need to add a `WHERE` clause with a `NOT IN` subquery to exclude contact-owned announces +- **Identity context exists**: Contact table is scoped by `identityHash`, so we need to filter by the active identity +- **Call chain is clear**: UI → ViewModel → Repository → DAO (4 layers, all identified) + +**Primary recommendation:** Modify `AnnounceDao.deleteAllAnnounces()` to accept an `identityHash` parameter and use a SQL subquery to exclude contact announces. Update the call chain to pass the active identity hash from ViewModel to DAO. + +## Standard Stack + +This phase uses existing project infrastructure - no new libraries needed. + +### Core +| Component | Purpose | Already In Project | +|-----------|---------|-------------------| +| Room | SQLite ORM for database operations | Yes, v2.6.1 | +| Kotlin Coroutines | Async database operations | Yes, v1.9.0 | +| Hilt | Dependency injection | Yes, v2.51.1 | +| JUnit 4 + MockK | Testing framework | Yes | + +### Architecture Pattern Used +- **Repository Pattern**: Data layer abstraction (already in use) +- **DAO Pattern**: Direct database access (Room) +- **Unidirectional Data Flow**: ViewModel → Repository → DAO → Database + +## Architecture Patterns + +### Current Structure (Identified in Codebase) + +``` +UI Layer: +├── AnnounceStreamScreen.kt (line 380: viewModel.deleteAllAnnounces()) +└── ContactsScreen.kt (line 153: showClearAllAnnouncesDialog trigger) + +ViewModel Layer: +├── AnnounceStreamViewModel.kt (line 476-485: deleteAllAnnounces() function) +└── identityRepository: IdentityRepository (available for getActiveIdentitySync()) + +Repository Layer: +├── AnnounceRepository.kt (line 397-399: deleteAllAnnounces() function) +└── ContactRepository.kt (not used in this fix, but contacts defined here) + +DAO Layer: +├── AnnounceDao.kt (line 131-132: deleteAllAnnounces() - THE FIX LOCATION) +└── ContactDao.kt (shows pattern for identity-scoped queries) + +Database: +├── announces table (PK: destinationHash) +└── contacts table (PK: destinationHash + identityHash) +``` + +### Pattern 1: Room DAO Query with Subquery Exclusion + +**What:** Use SQL `WHERE NOT IN (SELECT ...)` to exclude announces owned by contacts + +**When to use:** When deleting records that should exclude items matching a foreign relationship + +**Example from ContactDao (line 37-78):** +```kotlin +@Query(""" + SELECT c.* + FROM contacts c + LEFT JOIN announces a ON c.destinationHash = a.destinationHash + WHERE c.identityHash = :identityHash +""") +fun getEnrichedContacts(identityHash: String): Flow> +``` + +**Pattern for this fix:** +```kotlin +@Query(""" + DELETE FROM announces + WHERE destinationHash NOT IN ( + SELECT destinationHash + FROM contacts + WHERE identityHash = :identityHash + ) +""") +suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) +``` + +### Pattern 2: Identity-Scoped Operations + +**What:** Operations that filter by active identity to maintain data isolation + +**When to use:** Any operation involving user-specific data (contacts are identity-scoped) + +**Example from AnnounceStreamViewModel (line 361):** +```kotlin +val displayName = identityRepository.getActiveIdentitySync()?.displayName ?: "Unknown" +``` + +**Pattern for this fix:** +```kotlin +fun deleteAllAnnounces() { + viewModelScope.launch { + try { + val activeIdentity = identityRepository.getActiveIdentitySync() + if (activeIdentity != null) { + announceRepository.deleteAllAnnouncesExceptContacts(activeIdentity.identityHash) + } else { + // Fallback: delete all if no active identity (shouldn't happen) + announceRepository.deleteAllAnnounces() + } + Log.d(TAG, "Deleted non-contact announces") + } catch (e: Exception) { + Log.e(TAG, "Failed to delete announces", e) + } + } +} +``` + +### Anti-Patterns to Avoid + +- **Don't fetch all contacts to filter in Kotlin**: The WHERE clause in SQL is far more efficient than loading all data into memory +- **Don't iterate and delete individually**: A single SQL DELETE with subquery is atomic and faster +- **Don't forget the identity context**: Contacts are scoped to identities, so must filter by `identityHash` + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Identity resolution | Manual identity fetching in multiple places | Use existing `identityRepository.getActiveIdentitySync()` | Already tested and handles edge cases | +| Contact-announce linking | Custom join logic | Room's native SQL support with subqueries | Atomic, efficient, and type-safe | +| Error handling | Custom try-catch everywhere | Existing ViewModel pattern (already logs errors) | Consistent error handling across app | + +**Key insight:** This is a SQL query change, not a new feature. Use Room's @Query annotation capabilities rather than building custom logic. + +## Common Pitfalls + +### Pitfall 1: Forgetting Identity Context + +**What goes wrong:** Deleting announces that match ANY contact, not just contacts for the active identity + +**Why it happens:** The `contacts` table has composite primary key `(destinationHash, identityHash)`, meaning the same destinationHash can appear for multiple identities + +**How to avoid:** Always include `WHERE identityHash = :identityHash` in the contact subquery + +**Example of WRONG query:** +```kotlin +@Query(""" + DELETE FROM announces + WHERE destinationHash NOT IN (SELECT destinationHash FROM contacts) +""") +``` +This would preserve announces for contacts belonging to OTHER identities too! + +**Example of CORRECT query:** +```kotlin +@Query(""" + DELETE FROM announces + WHERE destinationHash NOT IN ( + SELECT destinationHash FROM contacts WHERE identityHash = :identityHash + ) +""") +suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) +``` + +**Warning signs:** Test with multiple identities. If Identity A's contacts prevent deletion but Identity B is active, the query is wrong. + +### Pitfall 2: Breaking the Existing deleteAllAnnounces() + +**What goes wrong:** Tests or other code paths that call `deleteAllAnnounces()` for legitimate reasons (like test cleanup) break + +**Why it happens:** Changing the signature of existing method without checking all call sites + +**How to avoid:** +- Option A: Keep old `deleteAllAnnounces()` and add new `deleteAllAnnouncesExceptContacts(identityHash)` +- Option B: Change `deleteAllAnnounces()` to optionally take `identityHash` parameter with default null +- Option C: Keep old method for tests, rename production call to new method + +**Recommendation:** Use Option A for clarity - two separate methods with clear intent + +**Warning signs:** Run test suite after change. If AnnounceStreamViewModelTest fails, you broke the old method. + +### Pitfall 3: Race Condition with Contact Deletion + +**What goes wrong:** User deletes a contact while "Clear All Announces" is executing + +**Why it happens:** Two database operations in separate transactions + +**How to avoid:** This is acceptable behavior - Room transactions are atomic. If contact is deleted before clear executes, its announce will be deleted. If deleted after, announce is preserved. Both outcomes are correct. + +**Warning signs:** None - this is not a bug, it's expected eventual consistency. + +### Pitfall 4: Null Active Identity + +**What goes wrong:** User has no active identity (edge case during identity switching or first run) + +**Why it happens:** `getActiveIdentitySync()` can return null + +**How to avoid:** Add null check in ViewModel and decide behavior: + - Option A: Delete nothing (safer) + - Option B: Delete all (original behavior) + - Option C: Show error message + +**Recommendation:** Option B (delete all) for backward compatibility. No active identity means no contacts are relevant anyway. + +**Warning signs:** Test with identity switching and fresh app install. + +## Code Examples + +### Change 1: DAO Layer (AnnounceDao.kt) + +**Current implementation (line 131-132):** +```kotlin +/** + * Delete all announces (for testing/debugging) + */ +@Query("DELETE FROM announces") +suspend fun deleteAllAnnounces() +``` + +**New implementation:** +```kotlin +/** + * Delete all announces (for testing/debugging) + */ +@Query("DELETE FROM announces") +suspend fun deleteAllAnnounces() + +/** + * Delete all announces except those belonging to contacts of the specified identity. + * Preserves contact announces so users can still open conversations with saved contacts. + * + * @param identityHash The identity hash to filter contacts by + */ +@Query(""" + DELETE FROM announces + WHERE destinationHash NOT IN ( + SELECT destinationHash + FROM contacts + WHERE identityHash = :identityHash + ) +""") +suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) +``` + +**Rationale:** Keep original method for test compatibility, add new method for production use. + +### Change 2: Repository Layer (AnnounceRepository.kt) + +**Current implementation (line 397-399):** +```kotlin +/** + * Delete all announces (for testing/debugging) + */ +suspend fun deleteAllAnnounces() { + announceDao.deleteAllAnnounces() +} +``` + +**New implementation:** +```kotlin +/** + * Delete all announces (for testing/debugging) + */ +suspend fun deleteAllAnnounces() { + announceDao.deleteAllAnnounces() +} + +/** + * Delete all announces except those belonging to contacts of the specified identity. + * Preserves contact announces so users can still open conversations with saved contacts. + * + * @param identityHash The identity hash to filter contacts by + */ +suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) { + announceDao.deleteAllAnnouncesExceptContacts(identityHash) +} +``` + +### Change 3: ViewModel Layer (AnnounceStreamViewModel.kt) + +**Current implementation (line 476-485):** +```kotlin +/** + * Delete all announces from the database. + * Nodes will reappear when they announce again. + */ +fun deleteAllAnnounces() { + viewModelScope.launch { + try { + announceRepository.deleteAllAnnounces() + Log.d(TAG, "Deleted all announces") + } catch (e: Exception) { + Log.e(TAG, "Failed to delete all announces", e) + } + } +} +``` + +**New implementation:** +```kotlin +/** + * Delete all announces from the database, except those belonging to saved contacts. + * Preserves announces for contacts so users can still open conversations. + * Nodes will reappear when they announce again. + */ +fun deleteAllAnnounces() { + viewModelScope.launch { + try { + val activeIdentity = identityRepository.getActiveIdentitySync() + if (activeIdentity != null) { + announceRepository.deleteAllAnnouncesExceptContacts(activeIdentity.identityHash) + Log.d(TAG, "Deleted non-contact announces for identity: ${activeIdentity.identityHash}") + } else { + // Fallback: delete all if no active identity (shouldn't happen in normal use) + announceRepository.deleteAllAnnounces() + Log.d(TAG, "Deleted all announces (no active identity)") + } + } catch (e: Exception) { + Log.e(TAG, "Failed to delete announces", e) + } + } +} +``` + +**Rationale:** Use new method when active identity exists, fallback to old behavior if not. + +### Change 4: UI Layer (Optional - Improve Dialog Text) + +**Current implementation (AnnounceStreamScreen.kt line 853-856):** +```kotlin +title = { + Text("Clear All Announces?") +}, +text = { + Text("This will remove all discovered nodes from the list. They will reappear when they announce again.") +}, +``` + +**Improved implementation:** +```kotlin +title = { + Text("Clear All Announces?") +}, +text = { + Text("This will remove all discovered nodes from the list, except those saved in My Contacts. Nodes will reappear when they announce again.") +}, +``` + +**Rationale:** Update user-facing text to reflect the new behavior (preserves contact announces). + +## Test Strategy + +### Unit Tests to Add + +**File:** `data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt` (new file) + +**Test cases:** +1. `deleteAllAnnouncesExceptContacts removes non-contact announces` +2. `deleteAllAnnouncesExceptContacts preserves contact announces for active identity` +3. `deleteAllAnnouncesExceptContacts does not preserve contacts from other identities` +4. `deleteAllAnnouncesExceptContacts handles empty contacts table` +5. `deleteAllAnnouncesExceptContacts handles empty announces table` +6. `deleteAllAnnounces still works for test cleanup` (regression test) + +**Test pattern (from TESTING.md):** +```kotlin +@OptIn(ExperimentalCoroutinesApi::class) +class AnnounceDaoTest { + @get:Rule + val instantExecutorRule = InstantTaskExecutorRule() + + private lateinit var database: ColumbaDatabase + private lateinit var announceDao: AnnounceDao + private lateinit var contactDao: ContactDao + + private val testIdentityHash = "identity123" + private val testDestHash1 = "dest_hash_1" + private val testDestHash2 = "dest_hash_2" + + @Before + fun setup() { + // Setup in-memory database for testing + val context = ApplicationProvider.getApplicationContext() + database = Room.inMemoryDatabaseBuilder(context, ColumbaDatabase::class.java) + .allowMainThreadQueries() + .build() + announceDao = database.announceDao() + contactDao = database.contactDao() + } + + @After + fun tearDown() { + database.close() + } + + @Test + fun `deleteAllAnnouncesExceptContacts preserves contact announces`() = runTest { + // Arrange: Insert 2 announces + val announce1 = AnnounceEntity( + destinationHash = testDestHash1, + peerName = "Contact Node", + publicKey = ByteArray(32) { 1 }, + appData = null, + hops = 1, + lastSeenTimestamp = System.currentTimeMillis(), + nodeType = "PEER", + ) + val announce2 = AnnounceEntity( + destinationHash = testDestHash2, + peerName = "Random Node", + publicKey = ByteArray(32) { 2 }, + appData = null, + hops = 2, + lastSeenTimestamp = System.currentTimeMillis(), + nodeType = "PEER", + ) + announceDao.upsertAnnounce(announce1) + announceDao.upsertAnnounce(announce2) + + // Add contact for announce1 only + val contact = ContactEntity( + destinationHash = testDestHash1, + identityHash = testIdentityHash, + publicKey = ByteArray(32) { 1 }, + addedTimestamp = System.currentTimeMillis(), + addedVia = "ANNOUNCE", + ) + contactDao.insertContact(contact) + + // Act: Delete non-contact announces + announceDao.deleteAllAnnouncesExceptContacts(testIdentityHash) + + // Assert: Only contact announce remains + val remaining = announceDao.getAllAnnouncesSync() + assertEquals(1, remaining.size) + assertEquals(testDestHash1, remaining[0].destinationHash) + } +} +``` + +### Integration Test Pattern + +**File:** `app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt` (add to existing) + +**Test case:** +```kotlin +@Test +fun `deleteAllAnnounces preserves contact announces`() = runTest { + // Setup: Mock active identity and repository responses + val testIdentityHash = "test_identity" + coEvery { identityRepository.getActiveIdentitySync() } returns testLocalIdentity.copy(identityHash = testIdentityHash) + coEvery { announceRepository.deleteAllAnnouncesExceptContacts(testIdentityHash) } just Runs + + // Create ViewModel + viewModel = createViewModel() + advanceUntilIdle() + + // Act: Call delete + viewModel.deleteAllAnnounces() + advanceUntilIdle() + + // Assert: Verify new method was called with correct identity + coVerify { announceRepository.deleteAllAnnouncesExceptContacts(testIdentityHash) } + coVerify(exactly = 0) { announceRepository.deleteAllAnnounces() } +} +``` + +## State of the Art + +| Old Approach | Current Approach | Impact | +|--------------|------------------|--------| +| Unconditional DELETE FROM announces | DELETE with WHERE NOT IN subquery | Preserves contact announces | +| No identity context | Identity-scoped filtering | Multi-identity support | +| Simple deletion | Conditional deletion | Better UX (no "Node not found" errors) | + +**No deprecated patterns involved** - this is a straightforward SQL enhancement to existing code. + +## Open Questions + +1. **Should pinned/favorited announces also be preserved?** + - What we know: Current requirement only mentions contacts + - What's unclear: User might expect favorited nodes to persist too + - Recommendation: Stick to requirement (contacts only). Favoriting can be Phase 2.2 if needed. + +2. **Should there be UI feedback showing how many announces were deleted?** + - What we know: Current dialog just says "deleted all" + - What's unclear: Whether users care about the count + - Recommendation: Not required for this phase. Log it for debugging, but no UI change needed. + +3. **What happens if a contact is in PENDING_IDENTITY state (no public key)?** + - What we know: ContactEntity can have null publicKey (line 63 of ContactEntity.kt) + - What's unclear: Does the announce still need to be preserved? + - Recommendation: Yes, preserve it. The destinationHash still matters for identity resolution. + +## Sources + +### Primary (HIGH confidence) +- `/home/tyler/repos/public/columba/data/src/main/java/com/lxmf/messenger/data/db/entity/AnnounceEntity.kt` - Data model +- `/home/tyler/repos/public/columba/data/src/main/java/com/lxmf/messenger/data/db/entity/ContactEntity.kt` - Data model +- `/home/tyler/repos/public/columba/data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt` - DAO methods +- `/home/tyler/repos/public/columba/data/src/main/java/com/lxmf/messenger/data/db/dao/ContactDao.kt` - Join pattern examples +- `/home/tyler/repos/public/columba/data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt` - Repository layer +- `/home/tyler/repos/public/columba/app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt` - ViewModel layer +- `/home/tyler/repos/public/columba/app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt` - UI trigger +- `/home/tyler/repos/public/columba/.planning/codebase/TESTING.md` - Test patterns + +### Secondary (MEDIUM confidence) +- GitHub Issue #365 - User-reported bug describing the problem +- Room documentation - Subquery patterns (implicit from existing code patterns) + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH - All dependencies already in project +- Architecture: HIGH - Existing code clearly shows pattern +- Pitfalls: HIGH - Based on Room's known behavior and multi-identity setup +- Test strategy: HIGH - Test infrastructure exists and patterns documented + +**Research date:** 2026-01-27 +**Valid until:** 2026-02-27 (30 days - stable domain, no framework changes expected) + +**Notes:** +- This is a codebase-internal fix, not dependent on external library updates +- Room 2.6.1 is stable and well-tested with subqueries +- The fix is low-risk: if subquery fails, it defaults to no deletion (safe behavior) +- All change points identified with exact file paths and line numbers diff --git a/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-VERIFICATION.md b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-VERIFICATION.md new file mode 100644 index 00000000..53779ca0 --- /dev/null +++ b/.planning/phases/02.1-clear-announces-preserves-contacts/02.1-VERIFICATION.md @@ -0,0 +1,128 @@ +--- +phase: 02.1-clear-announces-preserves-contacts +verified: 2026-01-28T02:47:00Z +status: passed +score: 10/10 must-haves verified +re_verification: false +--- + +# Phase 2.1: Clear Announces Preserves Contacts Verification Report + +**Phase Goal:** "Clear All Announces" in the Network tab deletes all announces *except* those belonging to contacts in My Contacts, preserving the ability to open new conversations with saved contacts + +**Verified:** 2026-01-28T02:47:00Z +**Status:** passed +**Re-verification:** No — initial verification + +## Goal Achievement + +### Observable Truths + +| # | Truth | Status | Evidence | +|---|-------|--------|----------| +| 1 | Clear All Announces deletes non-contact announces | VERIFIED | DAO SQL query uses `WHERE destinationHash NOT IN (SELECT destinationHash FROM contacts WHERE identityHash = :identityHash)` (line 145-150) + Test `deleteAllAnnouncesExceptContacts_removesNonContactAnnounces` PASSED | +| 2 | Clear All Announces preserves announces belonging to contacts in My Contacts | VERIFIED | Same SQL subquery + Test `deleteAllAnnouncesExceptContacts_preservesAllContactAnnounces` PASSED | +| 3 | Dialog text reflects new behavior (mentions contacts are preserved) | VERIFIED | Dialog text at line 854: "except those saved in My Contacts" | +| 4 | Falls back to delete-all if no active identity (backward compatible) | VERIFIED | ViewModel line 481 `else { deleteAllAnnounces() }` + Test `deleteAllAnnounces falls back to deleteAll when no active identity` PASSED | +| 5 | DAO test proves SQL subquery correctly deletes non-contact announces | VERIFIED | Test `deleteAllAnnouncesExceptContacts_removesNonContactAnnounces` PASSED | +| 6 | DAO test proves SQL subquery correctly preserves contact announces for the active identity | VERIFIED | Test `deleteAllAnnouncesExceptContacts_preservesAllContactAnnounces` PASSED | +| 7 | DAO test proves contacts from other identities do NOT prevent deletion | VERIFIED | Test `deleteAllAnnouncesExceptContacts_doesNotPreserveContactsFromOtherIdentities` PASSED | +| 8 | DAO test proves empty contacts table results in all announces being deleted | VERIFIED | Test `deleteAllAnnouncesExceptContacts_deletesAllWhenNoContacts` PASSED | +| 9 | ViewModel test proves deleteAllAnnounces calls new method with correct identity hash | VERIFIED | Test `deleteAllAnnounces preserves contact announces via identity-aware delete` PASSED | +| 10 | ViewModel test proves fallback to deleteAllAnnounces when no active identity | VERIFIED | Test `deleteAllAnnounces falls back to deleteAll when no active identity` PASSED | + +**Score:** 10/10 truths verified + +### Required Artifacts + +| Artifact | Expected | Status | Details | +|----------|----------|--------|---------| +| `data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt` | deleteAllAnnouncesExceptContacts(identityHash) SQL query | VERIFIED | Lines 143-153: SQL DELETE with NOT IN subquery against contacts table | +| `data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt` | deleteAllAnnouncesExceptContacts(identityHash) passthrough | VERIFIED | Lines 389-391: Method exists, calls DAO correctly | +| `app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt` | Updated deleteAllAnnounces() with identity-aware routing | VERIFIED | Lines 472-488: Gets active identity, routes to new method, falls back to old method if no identity | +| `app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt` | Updated dialog text mentioning contact preservation | VERIFIED | Line 854: "except those saved in My Contacts" | +| `data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt` | DAO-level tests for deleteAllAnnouncesExceptContacts SQL query | VERIFIED | Lines 335-457: 6 tests covering all edge cases, all PASSED | +| `app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt` | ViewModel-level tests for identity-aware delete routing | VERIFIED | Lines 869-924: 3 tests covering routing logic, all PASSED | + +### Key Link Verification + +| From | To | Via | Status | Details | +|------|----|----|--------|---------| +| UI Dialog | ViewModel.deleteAllAnnounces() | onClick handler | WIRED | Line 380: `viewModel.deleteAllAnnounces()` in confirm button | +| ViewModel.deleteAllAnnounces() | IdentityRepository.getActiveIdentitySync() | Direct call | WIRED | Line 475: Fetches active identity synchronously | +| ViewModel.deleteAllAnnounces() | AnnounceRepository.deleteAllAnnouncesExceptContacts() | Conditional routing | WIRED | Line 477: Called with `activeIdentity.identityHash` if identity exists | +| ViewModel.deleteAllAnnounces() | AnnounceRepository.deleteAllAnnounces() | Fallback routing | WIRED | Line 481: Called when no active identity (backward compatible) | +| Repository.deleteAllAnnouncesExceptContacts() | DAO.deleteAllAnnouncesExceptContacts() | Direct passthrough | WIRED | Line 390: Suspending call to DAO | +| DAO.deleteAllAnnouncesExceptContacts() | Database SQL | Room @Query annotation | WIRED | Lines 143-153: SQL DELETE with NOT IN subquery filtering by identityHash | + +### Requirements Coverage + +No REQUIREMENTS.md file maps to this phase (inserted phase). Phase was added to fix issue #365. + +### Anti-Patterns Found + +None. Code quality is excellent: +- No TODO/FIXME comments in implementation +- No placeholder or stub implementations +- All methods have real implementations +- Tests have good coverage (17 DAO tests + 3 ViewModel tests) + +### Human Verification Required + +**1. End-to-End User Flow Test** + +**Test:** +1. Add some contacts to My Contacts from the Network tab (star button) +2. Note which announces are contacts vs non-contacts +3. Tap overflow menu → "Clear All Announces" +4. Confirm in dialog +5. Verify contact announces remain, non-contact announces deleted +6. Tap a contact card +7. Verify conversation opens (no "Node not found" error) + +**Expected:** +- Contact announces remain visible in Network tab +- Non-contact announces are removed +- Opening conversation with contact works without error + +**Why human:** +- Requires visual verification of UI state changes +- Involves multiple screen interactions +- Tests actual user experience, not just logic + +**2. Multi-Identity Isolation Test** + +**Test:** +1. Create/switch to Identity A +2. Add some contacts +3. Switch to Identity B +4. Add different contacts +5. Switch back to Identity A +6. Tap "Clear All Announces" +7. Verify only Identity A's contacts are preserved + +**Expected:** +- Only contacts belonging to active identity are preserved +- Contacts from other identities do NOT prevent deletion + +**Why human:** +- Requires multi-identity setup +- Tests cross-identity isolation (critical security/privacy feature) +- Validates identity-scoped SQL query works correctly in practice + +### Gaps Summary + +No gaps found. All must-haves verified: +- Implementation artifacts exist and are substantive (not stubs) +- All key links are wired correctly +- DAO SQL query correctly filters by identityHash +- Repository passes identity hash correctly +- ViewModel routes to correct method based on identity availability +- Fallback to old behavior when no active identity +- Dialog text updated to reflect new behavior +- All 20 unit tests pass (17 DAO + 3 ViewModel) + +--- + +_Verified: 2026-01-28T02:47:00Z_ +_Verifier: Claude (gsd-verifier)_ diff --git a/app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt b/app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt index 227514a8..9281fd48 100644 --- a/app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt +++ b/app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt @@ -439,8 +439,7 @@ fun NodeTypeFilterDialog( } else { tempSelection + nodeType } - } - .padding(vertical = 4.dp), + }.padding(vertical = 4.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp), ) { @@ -486,8 +485,7 @@ fun NodeTypeFilterDialog( .fillMaxWidth() .clickable { tempShowAudio = !tempShowAudio - } - .padding(vertical = 4.dp), + }.padding(vertical = 4.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp), ) { @@ -853,7 +851,7 @@ fun ClearAllAnnouncesDialog( Text("Clear All Announces?") }, text = { - Text("This will remove all discovered nodes from the list. They will reappear when they announce again.") + Text("This will remove all discovered nodes from the list, except those saved in My Contacts. Nodes will reappear when they announce again.") }, confirmButton = { TextButton( diff --git a/app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt b/app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt index 639cde09..1e7dbdf4 100644 --- a/app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt +++ b/app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt @@ -20,7 +20,6 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.withContext import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -32,6 +31,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeoutOrNull import javax.inject.Inject @@ -66,7 +66,8 @@ class AnnounceStreamViewModel // Total announce count for tab label val announceCount: StateFlow = - announceRepository.getAnnounceCountFlow() + announceRepository + .getAnnounceCountFlow() .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5000L), @@ -98,24 +99,25 @@ class AnnounceStreamViewModel flowOf(PagingData.empty()) } else { // Get paginated announces from repository - announceRepository.getAnnouncesPaged( - nodeTypes = typeStrings, - searchQuery = query.trim(), - ).map { pagingData -> - // Apply in-memory filters for nodeType and audio aspect - pagingData.filter { announce -> - // Filter by nodeType - // (exclude PEER if user didn't select it and we only added it for audio) - val matchesNodeType = - selectedTypes.map { it.name }.contains(announce.nodeType) - val isAudioAnnounce = announce.aspect == "call.audio" - - // Show announce if: - // - It matches selected nodeType AND (showAudio OR not audio announce) - // - OR it's audio announce AND showAudio is true - (matchesNodeType && (showAudio || !isAudioAnnounce)) || (isAudioAnnounce && showAudio) + announceRepository + .getAnnouncesPaged( + nodeTypes = typeStrings, + searchQuery = query.trim(), + ).map { pagingData -> + // Apply in-memory filters for nodeType and audio aspect + pagingData.filter { announce -> + // Filter by nodeType + // (exclude PEER if user didn't select it and we only added it for audio) + val matchesNodeType = + selectedTypes.map { it.name }.contains(announce.nodeType) + val isAudioAnnounce = announce.aspect == "call.audio" + + // Show announce if: + // - It matches selected nodeType AND (showAudio OR not audio announce) + // - OR it's audio announce AND showAudio is true + (matchesNodeType && (showAudio || !isAudioAnnounce)) || (isAudioAnnounce && showAudio) + } } - } } }.cachedIn(viewModelScope) @@ -168,9 +170,10 @@ class AnnounceStreamViewModel try { // Get path table hashes from RNS (Python call - must be off main thread) - val pathTableHashes = withContext(Dispatchers.IO) { - reticulumProtocol.getPathTableHashes() - } + val pathTableHashes = + withContext(Dispatchers.IO) { + reticulumProtocol.getPathTableHashes() + } // Count announces that match the path table (database query - already on IO dispatcher in repository) val count = announceRepository.countReachableAnnounces(pathTableHashes) @@ -317,23 +320,17 @@ class AnnounceStreamViewModel /** * Check if an announce is already a contact (for star button state) */ - suspend fun isContact(destinationHash: String): Boolean { - return contactRepository.hasContact(destinationHash) - } + suspend fun isContact(destinationHash: String): Boolean = contactRepository.hasContact(destinationHash) /** * Observe a specific announce reactively */ - fun getAnnounceFlow(destinationHash: String): Flow { - return announceRepository.getAnnounceFlow(destinationHash) - } + fun getAnnounceFlow(destinationHash: String): Flow = announceRepository.getAnnounceFlow(destinationHash) /** * Observe contact status reactively */ - fun isContactFlow(destinationHash: String): Flow { - return contactRepository.hasContactFlow(destinationHash) - } + fun isContactFlow(destinationHash: String): Flow = contactRepository.hasContactFlow(destinationHash) /** * Update the selected node types filter @@ -415,9 +412,7 @@ class AnnounceStreamViewModel /** * Observe whether a destination is the user's current relay. */ - fun isMyRelayFlow(destinationHash: String): Flow { - return contactRepository.isMyRelayFlow(destinationHash) - } + fun isMyRelayFlow(destinationHash: String): Flow = contactRepository.isMyRelayFlow(destinationHash) /** * Set a propagation node as the user's relay. @@ -470,16 +465,24 @@ class AnnounceStreamViewModel } /** - * Delete all announces from the database. + * Delete all announces from the database, except those belonging to saved contacts. + * Preserves announces for contacts so users can still open conversations. * Nodes will reappear when they announce again. */ fun deleteAllAnnounces() { viewModelScope.launch { try { - announceRepository.deleteAllAnnounces() - Log.d(TAG, "Deleted all announces") + val activeIdentity = identityRepository.getActiveIdentitySync() + if (activeIdentity != null) { + announceRepository.deleteAllAnnouncesExceptContacts(activeIdentity.identityHash) + Log.d(TAG, "Deleted non-contact announces for identity: ${activeIdentity.identityHash}") + } else { + // Fallback: delete all if no active identity (shouldn't happen in normal use) + announceRepository.deleteAllAnnounces() + Log.d(TAG, "Deleted all announces (no active identity)") + } } catch (e: Exception) { - Log.e(TAG, "Failed to delete all announces", e) + Log.e(TAG, "Failed to delete announces", e) } } } diff --git a/app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt b/app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt index 6e639a1c..e2e024af 100644 --- a/app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt +++ b/app/src/test/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModelTest.kt @@ -120,6 +120,7 @@ class AnnounceStreamViewModelTest { coEvery { announceRepository.saveAnnounce(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()) } just Runs coEvery { announceRepository.getAnnounceCount() } returns 0 coEvery { announceRepository.countReachableAnnounces(any()) } returns 0 + coEvery { announceRepository.deleteAllAnnouncesExceptContacts(any()) } just Runs coEvery { reticulumProtocol.shutdown() } returns Result.success(Unit) coEvery { reticulumProtocol.getPathTableHashes() } returns emptyList() coEvery { announceRepository.setFavorite(any(), any()) } just Runs @@ -865,7 +866,7 @@ class AnnounceStreamViewModelTest { } @Test - fun `deleteAllAnnounces calls repository`() = + fun `deleteAllAnnounces preserves contact announces via identity-aware delete`() = runTest { networkStatusFlow.value = NetworkStatus.READY coEvery { announceRepository.deleteAllAnnounces() } just Runs @@ -877,15 +878,16 @@ class AnnounceStreamViewModelTest { viewModel.deleteAllAnnounces() advanceUntilIdle() - // Verify repository was called - coVerify { announceRepository.deleteAllAnnounces() } + // Verify new identity-aware method was called (testLocalIdentity is returned by default) + coVerify { announceRepository.deleteAllAnnouncesExceptContacts(testLocalIdentity.identityHash) } + coVerify(exactly = 0) { announceRepository.deleteAllAnnounces() } } @Test fun `deleteAllAnnounces handles errors gracefully`() = runTest { networkStatusFlow.value = NetworkStatus.READY - coEvery { announceRepository.deleteAllAnnounces() } throws Exception("Database error") + coEvery { announceRepository.deleteAllAnnouncesExceptContacts(any()) } throws Exception("Database error") viewModel = AnnounceStreamViewModel(reticulumProtocol, announceRepository, contactRepository, propagationNodeManager, identityRepository) advanceUntilIdle() @@ -894,10 +896,30 @@ class AnnounceStreamViewModelTest { viewModel.deleteAllAnnounces() advanceUntilIdle() - // Verify delete was attempted - coVerify { announceRepository.deleteAllAnnounces() } + // Verify delete was attempted with identity-aware method + coVerify { announceRepository.deleteAllAnnouncesExceptContacts(testLocalIdentity.identityHash) } // ViewModel should still be functioning assertNotNull(viewModel) } + + @Test + fun `deleteAllAnnounces falls back to deleteAll when no active identity`() = + runTest { + networkStatusFlow.value = NetworkStatus.READY + coEvery { identityRepository.getActiveIdentitySync() } returns null + coEvery { announceRepository.deleteAllAnnounces() } just Runs + + viewModel = AnnounceStreamViewModel(reticulumProtocol, announceRepository, contactRepository, propagationNodeManager, identityRepository) + advanceUntilIdle() + + // Delete all announces + viewModel.deleteAllAnnounces() + advanceUntilIdle() + + // Verify old method was called (fallback behavior) + coVerify { announceRepository.deleteAllAnnounces() } + // Verify new method was NOT called + coVerify(exactly = 0) { announceRepository.deleteAllAnnouncesExceptContacts(any()) } + } } diff --git a/data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt b/data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt index 9af7a4e8..2fb169f7 100644 --- a/data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt +++ b/data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt @@ -36,13 +36,16 @@ interface AnnounceDao { /** * Get announces in batches to prevent OOM when loading large amounts of data. * Used for identity restoration with pagination. - * + * * @param limit Number of announces to return in this batch * @param offset Number of announces to skip (for pagination) * @return List of announces sorted by most recently seen */ @Query("SELECT * FROM announces ORDER BY lastSeenTimestamp DESC LIMIT :limit OFFSET :offset") - suspend fun getAnnouncesBatch(limit: Int, offset: Int): List + suspend fun getAnnouncesBatch( + limit: Int, + offset: Int, + ): List /** * Insert multiple announces at once (for import). @@ -131,6 +134,24 @@ interface AnnounceDao { @Query("DELETE FROM announces") suspend fun deleteAllAnnounces() + /** + * Delete all announces except those belonging to contacts of the specified identity. + * Preserves contact announces so users can still open conversations with saved contacts. + * + * @param identityHash The identity hash to filter contacts by + */ + @Query( + """ + DELETE FROM announces + WHERE destinationHash NOT IN ( + SELECT destinationHash + FROM contacts + WHERE identityHash = :identityHash + ) + """, + ) + suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) + /** * Get count of all announces */ diff --git a/data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt b/data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt index 753cd527..ce68a86d 100644 --- a/data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt +++ b/data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt @@ -111,22 +111,20 @@ class AnnounceRepository * Automatically updates UI when announces are added or updated. * Includes icon data from peer_icons table (LXMF message appearances). */ - fun getAnnounces(): Flow> { - return announceDao.getEnrichedAnnounces().map { enriched -> + fun getAnnounces(): Flow> = + announceDao.getEnrichedAnnounces().map { enriched -> enriched.map { it.toAnnounce() } } - } /** * Search announces by peer name or destination hash. * Automatically updates UI when matching announces are added or updated. * Includes icon data from peer_icons table. */ - fun searchAnnounces(query: String): Flow> { - return announceDao.searchEnrichedAnnounces(query).map { enriched -> + fun searchAnnounces(query: String): Flow> = + announceDao.searchEnrichedAnnounces(query).map { enriched -> enriched.map { it.toAnnounce() } } - } /** * Get announces filtered by node types as a Flow, sorted by most recently seen. @@ -134,11 +132,10 @@ class AnnounceRepository * Includes icon data from peer_icons table. * @param nodeTypes List of node types to include (e.g., ["PEER", "NODE"]) */ - fun getAnnouncesByTypes(nodeTypes: List): Flow> { - return announceDao.getEnrichedAnnouncesByTypes(nodeTypes).map { enriched -> + fun getAnnouncesByTypes(nodeTypes: List): Flow> = + announceDao.getEnrichedAnnouncesByTypes(nodeTypes).map { enriched -> enriched.map { it.toAnnounce() } } - } /** * Get top propagation nodes sorted by hop count (ascending). @@ -148,11 +145,10 @@ class AnnounceRepository * @param limit Maximum number of nodes to return (default 10) * @return Flow of propagation node announces sorted by nearest first */ - fun getTopPropagationNodes(limit: Int = 10): Flow> { - return announceDao.getEnrichedTopPropagationNodes(limit).map { enriched -> + fun getTopPropagationNodes(limit: Int = 10): Flow> = + announceDao.getEnrichedTopPropagationNodes(limit).map { enriched -> enriched.map { it.toAnnounce() } } - } /** * Get announces with pagination support. Combines node type filtering and search query. @@ -166,8 +162,8 @@ class AnnounceRepository fun getAnnouncesPaged( nodeTypes: List, searchQuery: String, - ): Flow> { - return Pager( + ): Flow> = + Pager( config = PagingConfig( pageSize = 30, @@ -194,14 +190,11 @@ class AnnounceRepository ).flow.map { pagingData -> pagingData.map { enriched -> enriched.toAnnounce() } } - } /** * Get a specific announce by destination hash */ - suspend fun getAnnounce(destinationHash: String): Announce? { - return announceDao.getAnnounce(destinationHash)?.toAnnounce() - } + suspend fun getAnnounce(destinationHash: String): Announce? = announceDao.getAnnounce(destinationHash)?.toAnnounce() /** * Find an announce by identity hash. @@ -297,23 +290,17 @@ class AnnounceRepository /** * Check if an announce exists */ - suspend fun announceExists(destinationHash: String): Boolean { - return announceDao.announceExists(destinationHash) - } + suspend fun announceExists(destinationHash: String): Boolean = announceDao.announceExists(destinationHash) /** * Get total count of announces */ - suspend fun getAnnounceCount(): Int { - return announceDao.getAnnounceCount() - } + suspend fun getAnnounceCount(): Int = announceDao.getAnnounceCount() /** * Get total count of announces as a Flow for reactive UI updates. */ - fun getAnnounceCountFlow(): Flow { - return announceDao.getAnnounceCountFlow() - } + fun getAnnounceCountFlow(): Flow = announceDao.getAnnounceCountFlow() /** * Count announces that match the given path table hashes. @@ -334,22 +321,20 @@ class AnnounceRepository * Automatically updates UI when favorites are added or removed. * Includes icon data from peer_icons table. */ - fun getFavoriteAnnounces(): Flow> { - return announceDao.getEnrichedFavoriteAnnounces().map { enriched -> + fun getFavoriteAnnounces(): Flow> = + announceDao.getEnrichedFavoriteAnnounces().map { enriched -> enriched.map { it.toAnnounce() } } - } /** * Search favorite announces by peer name or destination hash. * Automatically updates UI when matching favorites are added or removed. * Includes icon data from peer_icons table. */ - fun searchFavoriteAnnounces(query: String): Flow> { - return announceDao.searchEnrichedFavoriteAnnounces(query).map { enriched -> + fun searchFavoriteAnnounces(query: String): Flow> = + announceDao.searchEnrichedFavoriteAnnounces(query).map { enriched -> enriched.map { it.toAnnounce() } } - } /** * Toggle favorite status for an announce. @@ -377,19 +362,16 @@ class AnnounceRepository /** * Get count of favorite announces as a Flow. */ - fun getFavoriteCount(): Flow { - return announceDao.getFavoriteCount() - } + fun getFavoriteCount(): Flow = announceDao.getFavoriteCount() /** * Get a specific announce as Flow (for observing favorite status changes). * Includes icon data from peer_icons table. */ - fun getAnnounceFlow(destinationHash: String): Flow { - return announceDao.getEnrichedAnnounceFlow(destinationHash).map { enriched -> + fun getAnnounceFlow(destinationHash: String): Flow = + announceDao.getEnrichedAnnounceFlow(destinationHash).map { enriched -> enriched?.toAnnounce() } - } /** * Delete all announces (for testing/debugging) @@ -398,14 +380,22 @@ class AnnounceRepository announceDao.deleteAllAnnounces() } + /** + * Delete all announces except those belonging to contacts of the specified identity. + * Preserves contact announces so users can still open conversations with saved contacts. + * + * @param identityHash The identity hash to filter contacts by + */ + suspend fun deleteAllAnnouncesExceptContacts(identityHash: String) { + announceDao.deleteAllAnnouncesExceptContacts(identityHash) + } + /** * Get count of announces grouped by nodeType. * Used for debugging relay selection issues. * Returns list of (nodeType, count) pairs. */ - suspend fun getNodeTypeCounts(): List> { - return announceDao.getNodeTypeCounts().map { it.nodeType to it.count } - } + suspend fun getNodeTypeCounts(): List> = announceDao.getNodeTypeCounts().map { it.nodeType to it.count } // Note: This mapping is only used for non-UI operations (export, toggle favorite, etc.) // For UI display, use enriched queries that join peer_icons for icon data diff --git a/data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt b/data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt index 5bf006cf..d5b31ee0 100644 --- a/data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt +++ b/data/src/test/java/com/lxmf/messenger/data/db/dao/AnnounceDaoTest.kt @@ -7,6 +7,8 @@ import androidx.test.core.app.ApplicationProvider import app.cash.turbine.test import com.lxmf.messenger.data.db.ColumbaDatabase import com.lxmf.messenger.data.db.entity.AnnounceEntity +import com.lxmf.messenger.data.db.entity.ContactEntity +import com.lxmf.messenger.data.db.entity.LocalIdentityEntity import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals @@ -26,15 +28,20 @@ import org.robolectric.annotation.Config class AnnounceDaoTest { private lateinit var database: ColumbaDatabase private lateinit var dao: AnnounceDao + private lateinit var contactDao: ContactDao + private lateinit var identityDao: LocalIdentityDao @Before fun setup() { val context = ApplicationProvider.getApplicationContext() database = - Room.inMemoryDatabaseBuilder(context, ColumbaDatabase::class.java) + Room + .inMemoryDatabaseBuilder(context, ColumbaDatabase::class.java) .allowMainThreadQueries() .build() dao = database.announceDao() + contactDao = database.contactDao() + identityDao = database.localIdentityDao() } @After @@ -73,6 +80,28 @@ class AnnounceDaoTest { peeringCost = if (stampCostFlexibility != null) 18 else null, ) + private fun createTestContact( + destinationHash: String, + identityHash: String = "test_identity_hash", + ) = ContactEntity( + destinationHash = destinationHash, + identityHash = identityHash, + publicKey = ByteArray(32) { it.toByte() }, + addedTimestamp = System.currentTimeMillis(), + addedVia = "ANNOUNCE", + ) + + private fun createTestIdentity(identityHash: String = "test_identity_hash") = + LocalIdentityEntity( + identityHash = identityHash, + displayName = "Test Identity", + destinationHash = "dest_$identityHash", + filePath = "/test/path", + createdTimestamp = System.currentTimeMillis(), + lastUsedTimestamp = System.currentTimeMillis(), + isActive = true, + ) + // ========== getTopPropagationNodes Tests ========== @Test @@ -302,4 +331,128 @@ class AnnounceDaoTest { assertEquals(3, counts.find { it.nodeType == "PEER" }?.count) assertEquals(2, counts.find { it.nodeType == "NODE" }?.count) } + + // ========== deleteAllAnnouncesExceptContacts Tests ========== + + @Test + fun deleteAllAnnouncesExceptContacts_removesNonContactAnnounces() = + runTest { + // Given - 2 announces, 1 contact for dest_hash_1 + val identity = createTestIdentity("test_identity_hash") + identityDao.insert(identity) + + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_1")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_2")) + + val contact = createTestContact("dest_hash_1", "test_identity_hash") + contactDao.insertContact(contact) + + // When + dao.deleteAllAnnouncesExceptContacts("test_identity_hash") + + // Then - Only dest_hash_1 remains + val remaining = dao.getAllAnnouncesSync() + assertEquals(1, remaining.size) + assertEquals("dest_hash_1", remaining[0].destinationHash) + } + + @Test + fun deleteAllAnnouncesExceptContacts_preservesAllContactAnnounces() = + runTest { + // Given - 3 announces, all are contacts + val identity = createTestIdentity("test_identity_hash") + identityDao.insert(identity) + + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_1")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_2")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_3")) + + contactDao.insertContact(createTestContact("dest_hash_1", "test_identity_hash")) + contactDao.insertContact(createTestContact("dest_hash_2", "test_identity_hash")) + contactDao.insertContact(createTestContact("dest_hash_3", "test_identity_hash")) + + // When + dao.deleteAllAnnouncesExceptContacts("test_identity_hash") + + // Then - All 3 announces remain + val remaining = dao.getAllAnnouncesSync() + assertEquals(3, remaining.size) + } + + @Test + fun deleteAllAnnouncesExceptContacts_doesNotPreserveContactsFromOtherIdentities() = + runTest { + // Given - 2 announces, contact for dest_hash_1 under identity_B (NOT active identity_A) + val identityA = createTestIdentity("identity_A") + val identityB = createTestIdentity("identity_B") + identityDao.insert(identityA) + identityDao.insert(identityB) + + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_1")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_2")) + + // Contact belongs to identity_B, not identity_A + contactDao.insertContact(createTestContact("dest_hash_1", "identity_B")) + + // When - Delete using identity_A (which has NO contacts) + dao.deleteAllAnnouncesExceptContacts("identity_A") + + // Then - Both announces deleted (identity_A has no contacts) + val remaining = dao.getAllAnnouncesSync() + assertEquals(0, remaining.size) + } + + @Test + fun deleteAllAnnouncesExceptContacts_deletesAllWhenNoContacts() = + runTest { + // Given - 3 announces, no contacts + val identity = createTestIdentity("some_identity_with_no_contacts") + identityDao.insert(identity) + + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_1")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_2")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_3")) + + // When + dao.deleteAllAnnouncesExceptContacts("some_identity_with_no_contacts") + + // Then - All announces deleted + val remaining = dao.getAllAnnouncesSync() + assertEquals(0, remaining.size) + } + + @Test + fun deleteAllAnnouncesExceptContacts_handlesEmptyAnnouncesTable() = + runTest { + // Given - No announces + val identity = createTestIdentity("test_identity_hash") + identityDao.insert(identity) + + // When - Should not crash + dao.deleteAllAnnouncesExceptContacts("test_identity_hash") + + // Then - Empty list + val remaining = dao.getAllAnnouncesSync() + assertEquals(0, remaining.size) + } + + @Test + fun deleteAllAnnounces_stillDeletesEverything() = + runTest { + // Given - 2 announces and 1 contact for dest_hash_1 + val identity = createTestIdentity("test_identity_hash") + identityDao.insert(identity) + + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_1")) + dao.upsertAnnounce(createTestAnnounce(destinationHash = "dest_hash_2")) + + contactDao.insertContact(createTestContact("dest_hash_1", "test_identity_hash")) + + // When - Use OLD deleteAllAnnounces (not the new contact-preserving version) + dao.deleteAllAnnounces() + + // Then - All announces deleted (original behavior unchanged) + val remaining = dao.getAllAnnouncesSync() + assertEquals(0, remaining.size) + } }