🧹 Code Cleanup & Modularization#9
Merged
Merged
Conversation
Improve code organization by splitting large module files:
- Extract 40+ API data models from client/mod.rs (1,086 lines) into
src/client/models/ with 13 domain-specific files (app, scan, finding,
user, policy, repo, etc.)
- Extract CLI argument types from cli/mod.rs (471 lines) into
src/cli/args/ with common.rs, filters.rs, and pagination.rs
- Update all imports to use explicit paths instead of re-exports,
making dependencies clearer and reducing coupling
This is Phase 1 of the code quality improvement plan, addressing the
organization review findings about oversized module files.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Break up the 1,919-line display.rs monolith into focused modules: - src/models/display/app.rs - AppDisplay - src/models/display/audit.rs - AuditDisplay + audit helpers - src/models/display/common.rs - Shared utilities (truncate_string, etc.) - src/models/display/config.rs - ConfigDisplay - src/models/display/finding.rs - Alert* display types - src/models/display/oas.rs - OASDisplay - src/models/display/org.rs - OrgDisplay - src/models/display/policy.rs - PolicyDisplay - src/models/display/repo.rs - RepoDisplay - src/models/display/scan.rs - ScanDisplay + formatting helpers - src/models/display/secret.rs - SecretDisplay - src/models/display/user.rs - UserDisplay, TeamDisplay Each file is now self-contained with its struct, From impls, helpers, and unit tests. This is Phase 2 task 1.3 of the code quality plan. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive test coverage for CLI command helper functions: - scan.rs: 29 tests covering format_duration_seconds, format_scan_status, matches_status, get_new_findings, apply_status_filter, apply_sort, and ScanContext creation - app.rs: 8 tests for filter_by_type including edge cases for cloud/standard filtering, case insensitivity, and default type handling - audit.rs: 14 tests for parse_date_to_millis (relative dates, ISO formats) and build_filter_params (all filter options, limit capping) Test count increased from 102 to 155 (+53 new tests). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract common list command pattern into reusable handler:
- Create src/cli/handlers/list.rs with run_list_command<T, D>()
- Refactor user, team, repo, oas, config commands to use it
- Reduces ~53 lines of duplicated boilerplate code
Each list command now follows a declarative pattern:
run_list_command::<ApiType, DisplayType, _, _>(
format, org, config, pagination, no_cache, "resource_name",
|client, org_id, params| async move { client.list_X(...) }
)
Policy and secret commands remain unchanged due to their
unique patterns (dual fetch and no org scope respectively).
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Split the monolithic 23-method StackHawkApi trait into three focused sub-traits for better modularity and testability: - AuthApi: Authentication operations (1 method) - ListingApi: Collection listing operations (17 methods) - ScanDetailApi: Scan drill-down operations (5 methods) The original StackHawkApi remains as a super-trait combining all three via blanket implementation. This enables: - Better separation of concerns - Easier mocking for specific test scenarios - Clearer API contracts per domain Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The super-trait was only used by CachedStackHawkClient and added no real value since we control all the code. Replaced with explicit trait bounds (AuthApi + ListingApi + ScanDetailApi). Net reduction: 17 lines. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Cache operations now use tokio::task::spawn_blocking to avoid blocking the async executor during SQLite reads/writes. This prevents thread starvation during parallel API fetches. Changes: - get_cached: async, awaited (need result before continuing) - set_cached: fire-and-forget spawn_blocking (don't delay response) - Cache now wrapped in Arc<Mutex<>> for Send + Clone Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
429 responses now retry with exponential backoff and jitter: - Max 3 retries before returning RateLimited error - Backoff: base_wait * 2^attempt (base from retry-after header) - Jitter: 0-1000ms random offset to prevent thundering herd Example wait times (base=1s): 1s, 2s, 4s before giving up. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create src/output/formatters.rs with reusable utilities: - format_timestamp_local(): Unix millis to local date/time - format_duration_seconds(): seconds to human-readable duration - offset_to_tz_abbrev(): UTC offset to timezone abbreviation - Add comprehensive doc comments explaining API quirks: - deserialize_string_to_usize: handles API's inconsistent number formats - matches_status: maps API statuses to user-friendly display values - Remove duplicate code and tests from src/cli/scan.rs - Part of 3.x Polish phase Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create src/client/fixtures.rs with builder patterns for: - OrganizationBuilder: build test orgs with name, user_count, app_count - ApplicationBuilder: build test apps with env, risk_level, org_id - ScanResultBuilder: build scans with status, findings, duration - UserBuilder: build users with email, name - Add convenience functions: test_org(), test_app(), test_scan(), test_user() - Comprehensive tests for all builders and their options - Part of 4.x Testing infrastructure phase Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add advanced testing features to the mock client: - rate_limit_after(n): Simulate rate limiting after N total API calls Returns RateLimited error once threshold is exceeded - captured_requests(): Record and retrieve all API requests for assertions Captures method name, org_id, page number, and page size - with_app_pages(pages): Support page-specific responses for pagination tests Each page index returns corresponding app list from the vector - CallCounts.total(): Sum all API call counts for threshold checks Includes 6 new tests verifying: - Rate limiting triggers after specified call count - Rate limiting works across different API methods - Request capture records org_id and pagination params - Paginated responses return correct page data - Total count reflects all pages in paged response Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add 6 new integration tests for error handling: - missing_config_shows_helpful_error: Verifies actionable error when config is missing - invalid_org_id_returns_not_found: Verifies 404 includes org ID and suggests fix - unauthorized_error_suggests_init: Verifies 401 suggests running hawkop init - rate_limit_shows_retry_message: Verifies 429 shows rate limit message - server_error_shows_helpful_message: Verifies 500 shows server error message - connection_error_shows_network_message: Verifies connection failure message Also fixes existing tests: - Add --no-cache flag to all HTTP tests to prevent cache pollution - Add match_query(Matcher::Any) to app_list test for query param matching - Fix auth mock to use GET instead of POST for /auth/login endpoint Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- README.md: Streamlined for new users with value proposition, GitHub Releases installation, and high-level command overview - PLANNING.md: Technical reference with architecture, caching design, rate limiting, completed code quality plan, and roadmap - CONTRIBUTING.md: Updated for Rust 2024 edition, added integration test patterns, and documented test fixtures module Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Major refactoring to improve code organization, testability, and maintainability. This PR splits monolithic modules into focused, domain-specific components while adding comprehensive testing infrastructure.
68 files changed | +6,394 | -4,038
What Changed
🏗️ Architecture Refactoring
client/mod.rs(1000+ lines) into focused sub-modules:client/api/- Trait definitions (auth, listing, scan_detail)client/models/- API data models by domain (scan, app, finding, etc.)models/display.rs(1900+ lines) into domain-specific display modulescli/args/module (common, pagination, filters)🧪 Testing Infrastructure
client/fixtures.rs) for easy test data creationMockStackHawkClientwith configurable simulation capabilitiestests/cli.rs)⚡ Performance & Reliability
📚 Documentation
Testing
Notes
This is a refactoring-focused PR with no user-facing behavior changes. The codebase is now more modular and significantly easier to test and maintain.
🤖 Generated with Claude Code