|
| 1 | +# Add Buffering Control |
| 2 | + |
| 3 | +**Issue**: [#138](https://github.com/torrust/torrust-tracker-deployer/issues/138) |
| 4 | +**Parent Epic**: [#102](https://github.com/torrust/torrust-tracker-deployer/issues/102) - User Output Architecture Improvements |
| 5 | +**Related**: |
| 6 | + |
| 7 | +- Refactoring Plan: [docs/refactors/plans/user-output-architecture-improvements.md](../refactors/plans/user-output-architecture-improvements.md) |
| 8 | +- Proposal #6 (Dependency): Type-Safe Channel Routing ([#135](https://github.com/torrust/torrust-tracker-deployer/issues/135)) |
| 9 | + |
| 10 | +## Overview |
| 11 | + |
| 12 | +This task adds explicit buffering control to the `UserOutput` module by providing a `flush()` method. While writes are typically line-buffered by default, explicit flush control ensures output appears immediately when needed and provides better testing capabilities. |
| 13 | + |
| 14 | +**Current State**: The codebase has implemented proposals 0-6, including: |
| 15 | + |
| 16 | +- `Theme` support (Proposal #2, Issue #124) |
| 17 | +- `OutputMessage` trait with concrete message types (Proposal #3, Issue #127) |
| 18 | +- Type-safe channel routing with `StdoutWriter` and `StderrWriter` newtypes (Proposal #6, Issue #135) |
| 19 | + |
| 20 | +Output is written immediately through the typed writer wrappers, but there's no explicit flush control available to callers. |
| 21 | + |
| 22 | +## Goals |
| 23 | + |
| 24 | +- [ ] Add `flush()` method to `UserOutput` for explicit buffer control |
| 25 | +- [ ] Document buffering behavior in module documentation |
| 26 | +- [ ] Add tests demonstrating flush behavior |
| 27 | +- [ ] Maintain backward compatibility with existing API |
| 28 | +- [ ] Follow Rust standard patterns from `Write` trait |
| 29 | + |
| 30 | +## 🏗️ Architecture Requirements |
| 31 | + |
| 32 | +**DDD Layer**: Presentation |
| 33 | +**Module Path**: `src/presentation/user_output.rs` |
| 34 | +**Pattern**: Standard Rust `Write` trait pattern for buffering control |
| 35 | + |
| 36 | +### Module Structure Requirements |
| 37 | + |
| 38 | +- [ ] Follow DDD layer separation (see [docs/codebase-architecture.md](../codebase-architecture.md)) |
| 39 | +- [ ] Keep presentation logic in presentation layer (no domain concerns) |
| 40 | +- [ ] Use appropriate module organization (see [docs/contributing/module-organization.md](../contributing/module-organization.md)) |
| 41 | + |
| 42 | +### Architectural Constraints |
| 43 | + |
| 44 | +- [ ] Must work with existing typed writer wrappers (`StdoutWriter`, `StderrWriter`) |
| 45 | +- [ ] Must be safe to call multiple times without side effects |
| 46 | +- [ ] Error handling follows project conventions (see [docs/contributing/error-handling.md](../contributing/error-handling.md)) |
| 47 | +- [ ] Testing covers successful flush and error scenarios |
| 48 | + |
| 49 | +### Anti-Patterns to Avoid |
| 50 | + |
| 51 | +- ❌ Automatic flushing after every write (defeats purpose of buffering) |
| 52 | +- ❌ Silent error suppression (flush errors should be propagated or logged) |
| 53 | +- ❌ Complex buffering strategies (keep it simple) |
| 54 | +- ❌ Breaking existing behavior (flushing should be opt-in) |
| 55 | + |
| 56 | +## Specifications |
| 57 | + |
| 58 | +### Flush Method Implementation |
| 59 | + |
| 60 | +Add an explicit `flush()` method that flushes both stdout and stderr writers: |
| 61 | + |
| 62 | +````rust |
| 63 | +impl UserOutput { |
| 64 | + /// Flush all pending output to stdout and stderr |
| 65 | + /// |
| 66 | + /// This is typically not needed as writes are line-buffered by default, |
| 67 | + /// but can be useful for ensuring output appears immediately. |
| 68 | + /// |
| 69 | + /// # Errors |
| 70 | + /// |
| 71 | + /// Returns an error if flushing either stream fails. |
| 72 | + /// |
| 73 | + /// # Examples |
| 74 | + /// |
| 75 | + /// ```rust,ignore |
| 76 | + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; |
| 77 | + /// |
| 78 | + /// let mut output = UserOutput::new(VerbosityLevel::Normal); |
| 79 | + /// output.progress("Processing..."); |
| 80 | + /// output.flush().expect("Failed to flush output"); |
| 81 | + /// ``` |
| 82 | + pub fn flush(&mut self) -> std::io::Result<()> { |
| 83 | + self.stdout.0.flush()?; |
| 84 | + self.stderr.0.flush()?; |
| 85 | + Ok(()) |
| 86 | + } |
| 87 | +} |
| 88 | +```` |
| 89 | + |
| 90 | +### Documentation Updates |
| 91 | + |
| 92 | +Update module-level documentation to explain buffering behavior: |
| 93 | + |
| 94 | +````rust |
| 95 | +//! ## Buffering Behavior |
| 96 | +//! |
| 97 | +//! Output is line-buffered by default. Messages are typically flushed automatically |
| 98 | +//! after each newline. For cases where immediate output is critical (e.g., before |
| 99 | +//! long-running operations), call `flush()` explicitly: |
| 100 | +//! |
| 101 | +//! ```rust,ignore |
| 102 | +//! output.progress("Starting long operation..."); |
| 103 | +//! output.flush()?; // Ensure message appears before operation starts |
| 104 | +//! perform_long_operation(); |
| 105 | +//! ``` |
| 106 | +```` |
| 107 | + |
| 108 | +### Testing Strategy |
| 109 | + |
| 110 | +Add tests to verify flush behavior: |
| 111 | + |
| 112 | +```rust |
| 113 | +#[cfg(test)] |
| 114 | +mod buffering_tests { |
| 115 | + use super::*; |
| 116 | + |
| 117 | + #[test] |
| 118 | + fn it_should_flush_all_writers() { |
| 119 | + let mut output = create_test_output(VerbosityLevel::Normal); |
| 120 | + output.output().progress("Test"); |
| 121 | + output.output().flush().expect("Flush should succeed"); |
| 122 | + |
| 123 | + // Verify output is present (flushed) |
| 124 | + assert!(!output.stderr().is_empty()); |
| 125 | + } |
| 126 | + |
| 127 | + #[test] |
| 128 | + fn it_should_be_safe_to_flush_multiple_times() { |
| 129 | + let mut output = create_test_output(VerbosityLevel::Normal); |
| 130 | + output.output().progress("Test"); |
| 131 | + |
| 132 | + // Multiple flushes should be safe |
| 133 | + output.output().flush().expect("First flush should succeed"); |
| 134 | + output.output().flush().expect("Second flush should succeed"); |
| 135 | + output.output().flush().expect("Third flush should succeed"); |
| 136 | + } |
| 137 | + |
| 138 | + #[test] |
| 139 | + fn it_should_flush_empty_buffers_safely() { |
| 140 | + let mut output = create_test_output(VerbosityLevel::Normal); |
| 141 | + |
| 142 | + // Flushing with no output should be safe |
| 143 | + output.output().flush().expect("Flushing empty buffers should succeed"); |
| 144 | + } |
| 145 | +} |
| 146 | +``` |
| 147 | + |
| 148 | +## Implementation Plan |
| 149 | + |
| 150 | +### Phase 1: Core Implementation (30 minutes) |
| 151 | + |
| 152 | +- [ ] Add `flush()` method to `UserOutput` |
| 153 | +- [ ] Implement flush for both `stdout` and `stderr` writers |
| 154 | +- [ ] Add rustdoc documentation for the method |
| 155 | +- [ ] Verify method signature matches Rust conventions |
| 156 | + |
| 157 | +### Phase 2: Documentation (20 minutes) |
| 158 | + |
| 159 | +- [ ] Update module-level documentation with buffering behavior section |
| 160 | +- [ ] Add usage examples showing when to use `flush()` |
| 161 | +- [ ] Document error handling for flush failures |
| 162 | +- [ ] Update related documentation if needed |
| 163 | + |
| 164 | +### Phase 3: Testing (30 minutes) |
| 165 | + |
| 166 | +- [ ] Add unit tests for successful flush |
| 167 | +- [ ] Add tests for multiple flush calls |
| 168 | +- [ ] Add tests for empty buffer flushing |
| 169 | +- [ ] Verify all existing tests still pass |
| 170 | + |
| 171 | +### Phase 4: Quality Assurance (20 minutes) |
| 172 | + |
| 173 | +- [ ] Run `./scripts/pre-commit.sh` and fix any issues |
| 174 | +- [ ] Verify all linters pass |
| 175 | +- [ ] Check code coverage for new code |
| 176 | +- [ ] Review for adherence to project conventions |
| 177 | + |
| 178 | +**Total Estimated Time**: 2 hours |
| 179 | + |
| 180 | +## Acceptance Criteria |
| 181 | + |
| 182 | +### Functional Requirements |
| 183 | + |
| 184 | +- [ ] `UserOutput::flush()` method flushes both stdout and stderr |
| 185 | +- [ ] Flush can be called multiple times safely |
| 186 | +- [ ] Flush returns `std::io::Result<()>` for error handling |
| 187 | +- [ ] Existing functionality is not affected by the addition |
| 188 | + |
| 189 | +### Documentation Requirements |
| 190 | + |
| 191 | +- [ ] Method has comprehensive rustdoc comments |
| 192 | +- [ ] Module documentation includes buffering behavior section |
| 193 | +- [ ] Usage examples demonstrate when to call `flush()` |
| 194 | +- [ ] Error handling is documented |
| 195 | + |
| 196 | +### Testing Requirements |
| 197 | + |
| 198 | +- [ ] Unit tests cover successful flush scenarios |
| 199 | +- [ ] Tests verify idempotency (multiple flushes are safe) |
| 200 | +- [ ] Tests verify empty buffer handling |
| 201 | +- [ ] All existing tests continue to pass |
| 202 | + |
| 203 | +### Quality Requirements (applies to every commit and PR) |
| 204 | + |
| 205 | +- [ ] Pre-commit checks pass: `./scripts/pre-commit.sh` |
| 206 | +- [ ] Code follows project conventions (see [docs/contributing/module-organization.md](../contributing/module-organization.md)) |
| 207 | +- [ ] Error handling follows project patterns (see [docs/contributing/error-handling.md](../contributing/error-handling.md)) |
| 208 | +- [ ] Changes are documented in code comments and rustdoc |
| 209 | + |
| 210 | +**Note for Contributors**: These criteria define what the PR reviewer will check. Use this as your pre-review checklist before submitting the PR to minimize back-and-forth iterations. |
| 211 | + |
| 212 | +## Related Documentation |
| 213 | + |
| 214 | +- [Development Principles](../development-principles.md) - Core principles including testability |
| 215 | +- [Module Organization](../contributing/module-organization.md) - Code organization conventions |
| 216 | +- [Error Handling Guide](../contributing/error-handling.md) - Error handling patterns |
| 217 | +- [User Output Research](../research/UX/console-app-output-patterns.md) - Background on output patterns |
| 218 | + |
| 219 | +## Notes |
| 220 | + |
| 221 | +### Why This Matters |
| 222 | + |
| 223 | +- **Testing**: Explicit flush control makes test assertions more reliable |
| 224 | +- **Immediate Feedback**: Ensures critical messages appear before long operations |
| 225 | +- **Standard Pattern**: Follows Rust's `Write` trait conventions |
| 226 | +- **Minimal Impact**: Low-risk enhancement with clear benefits |
| 227 | + |
| 228 | +### Implementation Considerations |
| 229 | + |
| 230 | +- Line-buffered output typically flushes after newlines automatically |
| 231 | +- Explicit flush is mainly useful before long-running operations or in tests |
| 232 | +- Error handling should propagate flush failures to callers |
| 233 | +- No need for automatic flushing after every write (would defeat buffering purpose) |
| 234 | + |
| 235 | +### Future Enhancements |
| 236 | + |
| 237 | +- Consider adding `auto_flush` configuration option if needed |
| 238 | +- Could add per-channel flush methods (`flush_stdout()`, `flush_stderr()`) if use case emerges |
| 239 | +- May want to add flush timing metrics for observability (future work) |
| 240 | + |
| 241 | +--- |
| 242 | + |
| 243 | +**Created**: November 4, 2025 |
| 244 | +**Status**: 📋 Not Started |
| 245 | +**Priority**: P2 (Phase 2 - Polish & Extensions) |
| 246 | +**Estimated Effort**: 2 hours |
0 commit comments