|
| 1 | +# Extract Verbosity Filtering Logic |
| 2 | + |
| 3 | +**Issue**: [#103](https://github.com/torrust/torrust-tracker-deployer/issues/103) |
| 4 | +**Parent Epic**: [#102](https://github.com/torrust/torrust-tracker-deployer/issues/102) - User Output Architecture Improvements |
| 5 | +**Related**: [Refactoring Plan - Proposal #0](../refactors/plans/user-output-architecture-improvements.md#proposal-0-extract-verbosity-filtering-logic) |
| 6 | + |
| 7 | +## Overview |
| 8 | + |
| 9 | +Extract verbosity checking logic from `UserOutput` into a dedicated `VerbosityFilter` struct. This eliminates code duplication across all output methods and makes verbosity rules testable independently. |
| 10 | + |
| 11 | +## Goals |
| 12 | + |
| 13 | +- [ ] Create `VerbosityFilter` struct that encapsulates verbosity logic |
| 14 | +- [ ] Replace inline verbosity checks with filter method calls |
| 15 | +- [ ] Make verbosity rules independently testable |
| 16 | +- [ ] Maintain all existing functionality and test coverage |
| 17 | + |
| 18 | +## 🏗️ Architecture Requirements |
| 19 | + |
| 20 | +**DDD Layer**: Presentation |
| 21 | +**Module Path**: `src/presentation/user_output.rs` |
| 22 | +**Pattern**: Extract to struct pattern (internal refactoring within module) |
| 23 | + |
| 24 | +### Module Structure Requirements |
| 25 | + |
| 26 | +- [ ] Keep changes within existing `user_output.rs` module |
| 27 | +- [ ] Follow module organization conventions (see [docs/contributing/module-organization.md](../docs/contributing/module-organization.md)) |
| 28 | +- [ ] Maintain backward compatibility with existing API |
| 29 | + |
| 30 | +### Architectural Constraints |
| 31 | + |
| 32 | +- [ ] No breaking changes to public `UserOutput` API |
| 33 | +- [ ] Error handling follows project conventions (see [docs/contributing/error-handling.md](../docs/contributing/error-handling.md)) |
| 34 | +- [ ] Follow testing conventions (see [docs/contributing/testing/](../docs/contributing/testing/)) |
| 35 | + |
| 36 | +### Anti-Patterns to Avoid |
| 37 | + |
| 38 | +- ❌ Exposing `VerbosityFilter` struct as public (it's an internal implementation detail, not part of the public API) |
| 39 | +- ❌ Making `verbosity_filter` field public (users should only interact through existing methods) |
| 40 | +- ❌ Complex verbosity logic - keep methods simple and self-documenting |
| 41 | +- ❌ Breaking existing tests or requiring extensive test rewrites |
| 42 | + |
| 43 | +**Note**: `VerbosityLevel` enum remains public (it's already part of the public API). Users pass `VerbosityLevel` to `UserOutput` constructors, and `UserOutput` creates the `VerbosityFilter` internally. |
| 44 | + |
| 45 | +## Problem |
| 46 | + |
| 47 | +Verbosity checks are duplicated across all output methods, violating DRY principle: |
| 48 | + |
| 49 | +```rust |
| 50 | +pub fn progress(&mut self, message: &str) { |
| 51 | + if self.verbosity >= VerbosityLevel::Normal { |
| 52 | + writeln!(self.stderr_writer, "⏳ {message}").ok(); |
| 53 | + } |
| 54 | +} |
| 55 | + |
| 56 | +pub fn success(&mut self, message: &str) { |
| 57 | + if self.verbosity >= VerbosityLevel::Normal { |
| 58 | + writeln!(self.stderr_writer, "✅ {message}").ok(); |
| 59 | + } |
| 60 | +} |
| 61 | +``` |
| 62 | + |
| 63 | +This makes the code harder to maintain and test, and violates the Single Responsibility Principle. |
| 64 | + |
| 65 | +## Specifications |
| 66 | + |
| 67 | +### VerbosityFilter Struct |
| 68 | + |
| 69 | +Create a private struct that encapsulates verbosity logic: |
| 70 | + |
| 71 | +```rust |
| 72 | +/// Determines what messages should be displayed based on verbosity level |
| 73 | +struct VerbosityFilter { |
| 74 | + level: VerbosityLevel, |
| 75 | +} |
| 76 | + |
| 77 | +impl VerbosityFilter { |
| 78 | + fn new(level: VerbosityLevel) -> Self { |
| 79 | + Self { level } |
| 80 | + } |
| 81 | + |
| 82 | + /// Check if messages at the given level should be shown |
| 83 | + fn should_show(&self, required_level: VerbosityLevel) -> bool { |
| 84 | + self.level >= required_level |
| 85 | + } |
| 86 | + |
| 87 | + /// Progress messages require Normal level |
| 88 | + fn should_show_progress(&self) -> bool { |
| 89 | + self.should_show(VerbosityLevel::Normal) |
| 90 | + } |
| 91 | + |
| 92 | + /// Errors are always shown |
| 93 | + fn should_show_errors(&self) -> bool { |
| 94 | + true |
| 95 | + } |
| 96 | +} |
| 97 | +``` |
| 98 | + |
| 99 | +### UserOutput Integration |
| 100 | + |
| 101 | +Update `UserOutput` to use the filter as a **private field**: |
| 102 | + |
| 103 | +```rust |
| 104 | +pub struct UserOutput { |
| 105 | + verbosity_filter: VerbosityFilter, // Private field - no `pub` keyword |
| 106 | + stdout_writer: Box<dyn Write + Send + Sync>, |
| 107 | + stderr_writer: Box<dyn Write + Send + Sync>, |
| 108 | +} |
| 109 | + |
| 110 | +impl UserOutput { |
| 111 | + pub fn new(verbosity: VerbosityLevel) -> Self { |
| 112 | + Self::with_writers( |
| 113 | + verbosity, |
| 114 | + Box::new(std::io::stdout()), |
| 115 | + Box::new(std::io::stderr()), |
| 116 | + ) |
| 117 | + } |
| 118 | + |
| 119 | + pub fn with_writers( |
| 120 | + verbosity: VerbosityLevel, |
| 121 | + stdout_writer: Box<dyn Write + Send + Sync>, |
| 122 | + stderr_writer: Box<dyn Write + Send + Sync>, |
| 123 | + ) -> Self { |
| 124 | + Self { |
| 125 | + verbosity_filter: VerbosityFilter::new(verbosity), // Create filter internally |
| 126 | + stdout_writer, |
| 127 | + stderr_writer, |
| 128 | + } |
| 129 | + } |
| 130 | + |
| 131 | + pub fn progress(&mut self, message: &str) { |
| 132 | + if self.verbosity_filter.should_show_progress() { |
| 133 | + writeln!(self.stderr_writer, "⏳ {message}").ok(); |
| 134 | + } |
| 135 | + } |
| 136 | +} |
| 137 | +``` |
| 138 | + |
| 139 | +**Key Points**: |
| 140 | + |
| 141 | +- **`VerbosityLevel` enum**: Remains public (already part of the API) - users pass this to constructors |
| 142 | +- **`VerbosityFilter` struct**: Module-private (no `pub` keyword) - internal implementation detail |
| 143 | +- **`verbosity_filter` field**: Private field in `UserOutput` - not accessible from outside |
| 144 | +- **Public API unchanged**: Users continue to pass `VerbosityLevel::Normal`, `VerbosityLevel::Quiet`, etc. to constructors |
| 145 | +- **Internal creation**: `UserOutput` creates the `VerbosityFilter` internally using `VerbosityFilter::new(verbosity)` |
| 146 | + |
| 147 | +This design allows future CLI arguments like `--verbosity=quiet` to control output, while keeping the internal filtering mechanism private. |
| 148 | + |
| 149 | +## Implementation Plan |
| 150 | + |
| 151 | +### Phase 1: Create VerbosityFilter (30 minutes) |
| 152 | + |
| 153 | +- [ ] Create `VerbosityFilter` struct with `level` field |
| 154 | +- [ ] Implement `new()` constructor |
| 155 | +- [ ] Implement `should_show()` base method |
| 156 | +- [ ] Add convenience methods: `should_show_progress()`, `should_show_errors()`, etc. |
| 157 | + |
| 158 | +### Phase 2: Add Unit Tests (45 minutes) |
| 159 | + |
| 160 | +- [ ] Test `should_show_progress()` at different verbosity levels |
| 161 | +- [ ] Test `should_show_errors()` always returns true |
| 162 | +- [ ] Test general `should_show()` method |
| 163 | +- [ ] Verify edge cases and boundary conditions |
| 164 | + |
| 165 | +### Phase 3: Integrate with UserOutput (1 hour) |
| 166 | + |
| 167 | +- [ ] Replace `verbosity: VerbosityLevel` field with `verbosity_filter: VerbosityFilter` |
| 168 | +- [ ] Update `UserOutput::new()` constructor |
| 169 | +- [ ] Update `UserOutput::with_writers()` constructor |
| 170 | +- [ ] Update all output methods to use `verbosity_filter.should_show_X()` |
| 171 | + |
| 172 | +### Phase 4: Verify & Clean Up (30 minutes) |
| 173 | + |
| 174 | +- [ ] Run all existing tests and verify they pass |
| 175 | +- [ ] Run linters: `cargo run --bin linter all` |
| 176 | +- [ ] Review code for any remaining inline verbosity checks |
| 177 | +- [ ] Update module documentation if needed |
| 178 | + |
| 179 | +## Acceptance Criteria |
| 180 | + |
| 181 | +**Quality Checks**: |
| 182 | + |
| 183 | +- [ ] Pre-commit checks pass: `./scripts/pre-commit.sh` |
| 184 | + |
| 185 | +**Task-Specific Criteria**: |
| 186 | + |
| 187 | +- [ ] `VerbosityFilter` struct created with all required methods |
| 188 | +- [ ] All inline verbosity checks replaced with filter method calls |
| 189 | +- [ ] Unit tests added for `VerbosityFilter` behavior |
| 190 | +- [ ] All existing tests still pass without modification |
| 191 | +- [ ] No breaking changes to public `UserOutput` API |
| 192 | +- [ ] Module documentation updated if needed |
| 193 | + |
| 194 | +## Related Documentation |
| 195 | + |
| 196 | +- [Refactoring Plan](../refactors/plans/user-output-architecture-improvements.md) - Complete refactoring plan with all proposals |
| 197 | +- [Development Principles](../development-principles.md) - Core principles including testability and maintainability |
| 198 | +- [Module Organization](../contributing/module-organization.md) - Code organization conventions |
| 199 | +- [Testing Conventions](../contributing/testing/) - Testing best practices |
| 200 | + |
| 201 | +## Notes |
| 202 | + |
| 203 | +### Rationale |
| 204 | + |
| 205 | +- **DRY Principle**: Eliminates repeated verbosity checks across multiple methods |
| 206 | +- **Testability**: Verbosity logic can now be tested independently from output formatting |
| 207 | +- **Clarity**: Named methods like `should_show_progress()` are self-documenting |
| 208 | +- **Extensibility**: Easy to add complex filtering rules in one place |
| 209 | + |
| 210 | +### Benefits |
| 211 | + |
| 212 | +- ✅ Removes code duplication across all output methods |
| 213 | +- ✅ Makes verbosity rules testable independently |
| 214 | +- ✅ Self-documenting code with named filter methods |
| 215 | +- ✅ Single source of truth for verbosity logic |
| 216 | +- ✅ Easy to extend with more complex filtering rules |
| 217 | + |
| 218 | +### Estimated Time |
| 219 | + |
| 220 | +**Total**: ~3 hours (conservative estimate with testing and verification) |
0 commit comments