refactor: migrate to DDD layered architecture

Split the monolithic app.rs (762 lines) into four DDD layers: - domain/: models, navigation enums (Tab, ModalKind), conversions, aggregates - application/: App state, LibraryState, NotificationManager, event handlers - infrastructure/: config, gRPC client, system utilities - presentation/: all render functions, widgets, views, modals Original modules (app, data, ui, config, grpc) preserved as thin re-export facades for backward compatibility. All 13 insta snapshot tests pass without modification.
2026-05-09 12:25:10 +02:00
parent 5bee7092d3
commit c1205e5fb0
45 changed files with 3983 additions and 2248 deletions
@@ -0,0 +1,494 @@
+# DDD Migration Plan — ui-agregator
+
+## Hard Constraint
+
+Existing insta snapshot tests in `tests/ui_snapshots.rs` MUST pass without any changes to the test file or snapshot content. All current import paths must continue to resolve to the same types and functions producing identical rendering output.
+
+### Frozen Test Import Paths
+
+```rust
+use ui_agregator::app::Tab;
+use ui_agregator::data::{Album, AlbumStatus, Artist, MonitorState};
+use ui_agregator::ui::library::{LibraryFocus, LibraryState, render_library};
+use ui_agregator::ui::modals::render_help_modal;
+use ui_agregator::ui::progress_bar::progress_bar;
+use ui_agregator::ui::topbar::render_topbar;
+```
+
+## Strategy: Facade Re-exports
+
+Create new DDD directories (`domain/`, `application/`, `infrastructure/`, `presentation/`), move code there verbatim, then turn existing `app.rs`, `data/`, and `ui/` into thin re-export facades. Tests import from unchanged paths while actual code lives in proper DDD layers.
+
+---
+
+## Target Architecture
+
+```
+src/
+├── lib.rs                          ← Module declarations + new layers
+├── main.rs                         ← Entry point (unchanged)
+│
+├── domain/                         ← Pure business logic, zero UI deps
+│   ├── mod.rs
+│   ├── models.rs                   ← from data/models.rs
+│   ├── navigation.rs               ← Tab enum (from app.rs) — shared by application + presentation
+│   ├── conversions.rs              ← from app.rs (convert_artist, convert_album, convert_track)
+│   └── aggregates.rs               ← from ui/library.rs (artist_status)
+│
+├── application/                    ← State management, orchestration
+│   ├── mod.rs
+│   ├── app_state.rs                ← App struct (from app.rs) — Tab moved to domain
+│   ├── library_state.rs            ← LibraryState + LibraryFocus (from ui/library.rs)
+│   ├── notification_state.rs       ← NotificationManager, NotifKind, Notification (from ui/notifications.rs)
+│   └── handlers.rs                 ← All handle_* methods (from app.rs)
+│
+├── infrastructure/                 ← External concerns
+│   ├── mod.rs
+│   ├── config.rs                   ← from src/config.rs
+│   ├── grpc/                       ← from src/grpc/
+│   │   └── mod.rs
+│   └── system.rs                   ← get_free_space() from statusbar.rs
+│
+├── presentation/                   ← UI rendering only
+│   ├── mod.rs
+│   ├── library.rs                  ← render_library + render helpers (from ui/library.rs)
+│   ├── notifications.rs            ← render + render_notification_item (from ui/notifications.rs)
+│   ├── topbar.rs                   ← from ui/topbar.rs
+│   ├── statusbar.rs                ← render only, syscall moved out
+│   ├── pane.rs                     ← from ui/pane.rs
+│   ├── progress_bar.rs             ← from ui/progress_bar.rs
+│   ├── modals/                     ← from ui/modals/
+│   └── views/                      ← from ui/views/
+│
+├── theme.rs                        ← Unchanged (leaf node)
+│
+├── app.rs                          ← FACADE: re-exports from application
+├── data/                           ← FACADE: re-exports from domain
+│   └── mod.rs
+├── config.rs                       ← FACADE: re-exports from infrastructure
+├── grpc/                           ← FACADE: re-exports from infrastructure
+│   └── mod.rs
+└── ui/                             ← FACADE: re-exports from presentation + application
+    ├── mod.rs
+    ├── library.rs                  ← re-exports LibraryFocus, LibraryState, render_library
+    ├── modals/
+    │   └── mod.rs                  ← re-exports from presentation/modals
+    ├── progress_bar.rs             ← re-exports progress_bar
+    ├── topbar.rs                   ← re-exports render_topbar
+    ├── notifications.rs            ← re-exports from application + presentation
+    ├── statusbar.rs                ← re-exports from presentation
+    └── pane.rs                     ← re-exports from presentation
+```
+
+---
+
+## Bounded Contexts (Lightweight)
+
+For a single TUI binary, use sub-domains within the domain layer:
+
+| Sub-domain     | Entities                          | Value Objects             |
+|----------------|-----------------------------------|---------------------------|
+| **Library**    | Artist, Album, Track              | AlbumStatus, MonitorState |
+| **Acquisition**| WantedEntry, QueueEntry           | —                         |
+| **Activity**   | HistoryEntry, CalendarEntry, Notification | NotifKind           |
+
+All live in `domain/models.rs` for now — separate into sub-modules only if they grow significantly.
+
+---
+
+## Phase 1: Infrastructure Extraction (Safest)
+
+**Goal:** Move external concerns. Zero test path impact.
+
+### Moves
+
+| Source | Destination | Items |
+|--------|-------------|-------|
+| `src/config.rs` | `src/infrastructure/config.rs` | Entire file (Config, ServerConfig) |
+| `src/grpc/mod.rs` | `src/infrastructure/grpc/mod.rs` | Entire file (GrpcClient, GrpcRequest, GrpcResponse, spawn_grpc_worker) |
+| `src/ui/statusbar.rs` line 12-25 | `src/infrastructure/system.rs` | `get_free_space()` only |
+
+### Facade Files
+
+```rust
+// src/config.rs → facade
+pub use crate::infrastructure::config::*;
+
+// src/grpc/mod.rs → facade  
+pub use crate::infrastructure::grpc::*;
+```
+
+### Risk: Minimal
+No test paths affected. `config` and `grpc` are only used by `main.rs`.
+
+### Verification
+`cargo build && cargo test`
+
+---
+
+## Phase 2: Domain Layer Creation
+
+**Goal:** Extract pure domain models, shared navigation types, and business logic.
+
+### Moves
+
+| Source | Destination | Items |
+|--------|-------------|-------|
+| `src/data/models.rs` | `src/domain/models.rs` | All types: AlbumStatus, MonitorState, Artist, Album, Track, WantedEntry, QueueEntry, HistoryEntry, CalendarEntry |
+| `src/app.rs` Tab enum + impls (lines 26-68) | `src/domain/navigation.rs` | `Tab` enum (see Risk Resolution 1) |
+| `src/ui/modals/mod.rs` ModalKind enum (lines 7-11) | `src/domain/navigation.rs` | `ModalKind` enum (see Risk Resolution 6) |
+| `src/app.rs` (convert_artist, convert_album, convert_track, parse_year, format_duration) | `src/domain/conversions.rs` | 5 functions (~90 lines) |
+| `src/ui/library.rs` (artist_status fn) | `src/domain/aggregates.rs` | `artist_status()` (~12 lines) |
+
+### Facade Files
+
+```rust
+// src/data/mod.rs → facade
+pub use crate::domain::models::*;
+
+// src/app.rs must re-export Tab for test compat
+pub use crate::domain::navigation::Tab;
+
+// src/domain/mod.rs
+pub mod models;
+pub mod navigation;
+pub mod conversions;
+pub mod aggregates;
+```
+
+### Risk: Low
+`data::*` path preserved via re-export. `app::Tab` path preserved via re-export. Domain models are leaf nodes with zero internal deps.
+
+### Verification
+`cargo test` — all `ui_agregator::data::*` and `ui_agregator::app::Tab` imports still resolve.
+
+---
+
+## Phase 3: Application Layer — State Types
+
+**Goal:** Extract state management types and logic.
+
+### Moves
+
+| Source | Destination | Items |
+|--------|-------------|-------|
+| `src/app.rs` lines 70-131 | `src/application/app_state.rs` | `App` struct + Default impl + `App::new()` (Tab already moved to domain in Phase 2) |
+| `src/ui/library.rs` lines 18-198 + 593-650 | `src/application/library_state.rs` | `LibraryFocus`, `LibraryState` (all fields + all methods: new, selected_artist, selected_album, move_up/down, focus_left/right, cycle_focus, cache_tracks, needs_fetch, clear_cache, etc.) |
+| `src/ui/notifications.rs` lines 15-111 + 189-196 + 252-260 | `src/application/notification_state.rs` | `NotifKind`, `Notification`, `NotificationManager` (all fields + push, tick, history, active_count, history_count), `format_elapsed` |
+
+### Pre-requisite: Add `active()` getter to NotificationManager
+
+Before moving, add this public getter (see Risk Resolution 2):
+```rust
+pub fn active(&self) -> &[Notification] {
+    &self.active
+}
+```
+
+This allows Phase 4 to convert `render()` to a free function without accessing private fields.
+
+### Critical Re-exports (preserve test paths)
+
+```rust
+// src/app.rs → facade
+pub use crate::domain::navigation::Tab;
+pub use crate::application::app_state::App;
+
+// src/ui/library.rs → facade
+pub use crate::application::library_state::{LibraryFocus, LibraryState};
+pub use crate::presentation::library::render_library;
+
+// src/ui/notifications.rs → facade
+pub use crate::application::notification_state::*;
+pub use crate::presentation::notifications::render_notification_item;
+```
+
+### Risk: Medium
+Most refactoring happens here. LibraryState has private fields (tracks_cache, pending_album_id) — same crate so visibility is fine.
+
+### Mitigation
+Move types first, verify tests, then move methods, verify again.
+
+### Verification
+`cargo test` — snapshot tests must pass unchanged.
+
+---
+
+## Phase 4: Presentation Layer — Render Functions
+
+**Goal:** Move all rendering code.
+
+### Moves
+
+| Source | Destination | Items |
+|--------|-------------|-------|
+| `src/ui/library.rs` lines 200-591 | `src/presentation/library.rs` | `render_library`, `render_artists_pane`, `render_detail_pane`, `render_artist_header`, `render_albums_list`, `render_tracks_list`, `status_icon`, `monitor_state_icon`, `track_icon`, `fmt_size` |
+| `src/ui/notifications.rs` lines 113-250 | `src/presentation/notifications.rs` | `NotificationManager::render` (as free fn taking &NotificationManager), `render_notification_item` |
+| `src/ui/topbar.rs` | `src/presentation/topbar.rs` | Entire file (render_topbar, TopbarAreas) |
+| `src/ui/progress_bar.rs` | `src/presentation/progress_bar.rs` | Entire file (progress_bar fn) |
+| `src/ui/pane.rs` | `src/presentation/pane.rs` | Entire file (Pane, section_divider) |
+| `src/ui/statusbar.rs` | `src/presentation/statusbar.rs` | render_statusbar (with get_free_space imported from infrastructure) |
+| `src/ui/modals/` | `src/presentation/modals/` | Entire directory (ModalKind, render_help_modal, render_quit_modal) |
+| `src/ui/views/` | `src/presentation/views/` | Entire directory (render_calendar, render_history, render_queue, render_settings, render_wanted) |
+
+### Facade Files
+
+```rust
+// src/ui/topbar.rs → facade
+pub use crate::presentation::topbar::*;
+
+// src/ui/progress_bar.rs → facade
+pub use crate::presentation::progress_bar::*;
+
+// src/ui/modals/mod.rs → facade
+pub use crate::presentation::modals::*;
+
+// src/ui/pane.rs → facade
+pub use crate::presentation::pane::*;
+
+// src/ui/statusbar.rs → facade
+pub use crate::presentation::statusbar::*;
+```
+
+### Risk: Low-Medium
+Mostly file moves. Rendering code moves verbatim — no logic changes means no snapshot changes.
+
+### Verification
+`cargo test` — all 13 tests pass, snapshots unchanged.
+
+---
+
+## Phase 5: Application Layer — Event Handlers
+
+**Goal:** Extract event handling from app.rs.
+
+### Moves
+
+| Source | Destination | Items |
+|--------|-------------|-------|
+| `src/app.rs` handle_escape, handle_click, handle_modal_click, handle_topbar_click, handle_notification_click, handle_main_click, handle_library_click, select_list_item, handle_scroll, scroll_library_list, handle_tick, set_error, handle_grpc_response, pending_album_fetch | `src/application/handlers.rs` | All event handler methods |
+| `src/app.rs` scroll_list_state | `src/application/handlers.rs` | Free function |
+
+### Implementation
+
+Use split impl blocks (Rust allows impl blocks in multiple files within the same crate):
+
+```rust
+// src/application/handlers.rs
+use crate::application::app_state::App;
+
+impl App {
+    pub fn handle_escape(&mut self) { ... }
+    pub fn handle_click(&mut self, x: u16, y: u16, button: MouseButton) { ... }
+    // ...all other handlers
+}
+```
+
+### Risk: Medium
+Must manage imports carefully. All handler methods access App fields directly.
+
+### Verification
+`cargo test && cargo clippy`, manual TUI smoke test.
+
+---
+
+## Phase 6: Final app.rs Cleanup + draw() Extraction
+
+**Goal:** Move rendering orchestration out of App, reduce app.rs to pure facade.
+
+### Moves
+
+| Source | Destination | Items |
+|--------|-------------|-------|
+| `src/app.rs` App::draw, render_main_content, render_notifications_dropdown, get_position | `src/presentation/app_renderer.rs` | All draw/render methods |
+
+### Final app.rs
+
+```rust
+pub use crate::domain::navigation::Tab;
+pub use crate::application::app_state::App;
+```
+
+### Final lib.rs
+
+```rust
+pub mod domain;
+pub mod application;
+pub mod infrastructure;
+pub mod presentation;
+pub mod theme;
+
+// Facade modules (backward compat)
+pub mod app;
+pub mod config;
+pub mod data;
+pub mod grpc;
+pub mod proto;
+pub mod ui;
+```
+
+### Risk: Highest
+Final wiring — all re-exports must be correct.
+
+### Verification
+`cargo test`, `cargo clippy`, run the actual TUI end-to-end.
+
+---
+
+---
+
+## Risk Resolutions
+
+### Risk 1: Tab Circular Dependency — RESOLVED
+
+**Problem:** `presentation/topbar.rs` imports `Tab` from `app.rs`. If Tab lives in `application/`, then presentation depends on application, and application's `App::draw()` depends on presentation — creating a cycle.
+
+**Investigation:** Tab is used in exactly 3 locations:
+- `src/app.rs` — definition + usage in App struct, event handlers, render routing
+- `src/ui/topbar.rs` — imported as `use crate::app::Tab`, used to build tab list and compare active tab
+- `tests/ui_snapshots.rs` — imported as `use ui_agregator::app::Tab`, used in test data
+
+Tab has no business logic dependencies. It's a pure enum with display helpers (`label()`, `index()`). No method on Tab calls into application or presentation code.
+
+**Solution:** Move Tab to `domain/navigation.rs`. Both application and presentation import from domain — dependency flows downward only:
+
+```
+domain/navigation.rs  (defines Tab)
+       ↑                    ↑
+application/app_state.rs   presentation/topbar.rs
+       (App.tab field)      (renders tabs)
+```
+
+Re-export chain for test compatibility:
+```rust
+// src/domain/navigation.rs — canonical location
+pub enum Tab { Library, Wanted, Queue, History, Calendar, Settings }
+
+// src/app.rs — facade re-export
+pub use crate::domain::navigation::Tab;
+
+// src/presentation/topbar.rs — imports from domain
+use crate::domain::navigation::Tab;
+```
+
+Test path `ui_agregator::app::Tab` continues to work via the facade.
+
+---
+
+### Risk 2: NotificationManager::render() Split — RESOLVED
+
+**Problem:** `NotificationManager::render()` is an `impl` method. When the type moves to `application/` and render moves to `presentation/`, the presentation layer can't add impl blocks to application types without creating a dependency cycle.
+
+**Investigation:** `render()` accesses exactly ONE private field: `self.active` (the Vec of active notifications). All `Notification` fields are already public. `render_notification_item()` is already a free function — good precedent.
+
+From app.rs, `NotificationManager` is used via these public methods only:
+- `.history_count()` — already pub
+- `.history()` — already pub, returns `&[Notification]`
+- `.render(frame, area)` — the method to split
+- `.tick()` — already pub
+- `.push(...)` — already pub
+
+**Solution:** Add one pub getter, convert render to free function:
+
+```rust
+// Step 1: Add getter to NotificationManager (in Phase 3, before the split)
+pub fn active(&self) -> &[Notification] {
+    &self.active
+}
+
+// Step 2: Convert render() to free function (in Phase 4)
+// src/presentation/notifications.rs
+pub fn render_notifications(frame: &mut Frame, area: Rect, active: &[Notification]) {
+    // Identical logic, `self.active` becomes `active` parameter
+}
+
+// Step 3: Update call site in app.rs draw() (Phase 6)
+// Before:  self.notifications.render(frame, area);
+// After:   render_notifications(frame, area, self.notifications.active());
+```
+
+No snapshot impact — render logic is identical, just parameterized differently.
+
+---
+
+### Risk 3: Private Fields Across Layers — RESOLVED
+
+**Problem:** `LibraryState` has private fields (`tracks_cache`, `pending_album_id`). When type moves to `application/` and render code moves to `presentation/`, can presentation still work?
+
+**Investigation:** Verified that presentation/library.rs render functions access LibraryState through these paths only:
+- `state.artists` — **pub** field
+- `state.tracks` — **pub** field
+- `state.focus` — **pub** field
+- `state.artist_state` / `state.album_state` / `state.track_state` — **pub** fields (ListState for stateful widgets)
+- `state.selected_artist()` — **pub** method
+- `state.selected_album()` — **pub** method
+
+The private fields (`tracks_cache`, `pending_album_id`) are only accessed by `LibraryState`'s own methods (`cache_tracks`, `needs_fetch`, `load_tracks_from_cache`, `clear_cache`) — all of which stay with the type in `application/library_state.rs`.
+
+**Conclusion:** No new getters needed. The existing public API is sufficient for the presentation layer.
+
+---
+
+### Risk 4: impl Block Splitting — RESOLVED
+
+**Problem:** After migration, `App` has impl blocks in 3 files: `application/app_state.rs`, `application/handlers.rs`, `presentation/app_renderer.rs`. Will Rust allow this?
+
+**Investigation:** Rust allows multiple impl blocks for a type anywhere within the same crate. The only requirement is that the type is accessible via `use`. Since all three files are in the same crate, this works:
+
+```rust
+// application/app_state.rs — struct definition + new()
+pub struct App { ... }
+impl App { pub fn new() -> Self { ... } }
+
+// application/handlers.rs — event handlers
+use crate::application::app_state::App;
+impl App { pub fn handle_click(&mut self, ...) { ... } }
+
+// presentation/app_renderer.rs — draw methods
+use crate::application::app_state::App;
+impl App { pub fn draw(&mut self, frame: &mut Frame) { ... } }
+```
+
+**Caveat:** `presentation/app_renderer.rs` adds impl blocks to an `application` type. This means presentation depends on application — which is architecturally correct (presentation calls down to application, not up). But it does mean `draw()` can access private fields of `App`, which is acceptable since it needs `self.tab`, `self.modal`, `self.queue`, etc.
+
+**Conclusion:** No issue. This is standard Rust practice for organizing large impl blocks.
+
+---
+
+### Risk 5: `use crate::` Path Updates — RESOLVED
+
+**Problem:** Every moved file has `use crate::data::*`, `use crate::ui::*` etc. These must be updated.
+
+**Solution:** Two options:
+
+**Option A (Recommended): Import from facade modules.** Moved files keep importing `use crate::data::*`, `use crate::ui::pane::*` etc. Since facades re-export everything, these paths still work. No import changes needed in moved files.
+
+**Option B: Import from canonical locations.** Update all imports to `use crate::domain::*`, `use crate::presentation::pane::*` etc. Cleaner but more churn.
+
+**Recommendation:** Use Option A during migration (zero import changes in moved files), then optionally do a cleanup pass later with Option B. This eliminates an entire class of errors during the migration.
+
+---
+
+### Risk 6: ModalKind Placement — RESOLVED
+
+**Problem:** `ModalKind` enum is defined in `ui/modals/mod.rs` and used in `app.rs` (matching on `self.modal`). After migration, ModalKind moves to `presentation/modals/` but App (application layer) matches on it — application depends on presentation.
+
+**Investigation:** `ModalKind` is used in:
+- `src/ui/modals/mod.rs` — definition
+- `src/app.rs` line 180-184 — `match modal { ModalKind::Help => ..., ModalKind::Quit => ... }`
+
+**Solution:** Move `ModalKind` to `domain/navigation.rs` alongside `Tab` — both are simple navigation/state enums shared across layers:
+
+```rust
+// src/domain/navigation.rs
+pub enum Tab { Library, Wanted, Queue, History, Calendar, Settings }
+pub enum ModalKind { Help, Quit }
+```
+
+Re-export from `ui/modals/mod.rs` facade for backward compatibility:
+```rust
+pub use crate::domain::navigation::ModalKind;
+pub use crate::presentation::modals::{render_help_modal, render_quit_modal};
+```
+
+Application layer imports `ModalKind` from domain. Presentation layer imports it from domain. No cross-layer dependency.