diff --git a/Cargo.lock b/Cargo.lock index 1e65070..6574a9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "aho-corasick" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "1.0.0" @@ -58,6 +67,16 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "assert-json-diff" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47e4f2b81832e72834d7518d8487a0396a28cc408186a2e8854c0f98011faf12" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "atomic-waker" version = "1.1.2" @@ -171,6 +190,15 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "colored" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" +dependencies = [ + "windows-sys 0.61.2", +] + [[package]] name = "core-foundation" version = "0.9.4" @@ -510,6 +538,12 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" +[[package]] +name = "httpdate" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" + [[package]] name = "hyper" version = "1.9.0" @@ -524,6 +558,7 @@ dependencies = [ "http", "http-body", "httparse", + "httpdate", "itoa", "pin-project-lite", "smallvec", @@ -800,6 +835,31 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "mockito" +version = "1.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "90820618712cab19cfc46b274c6c22546a82affcb3c3bdf0f29e3db8e1bb92c0" +dependencies = [ + "assert-json-diff", + "bytes", + "colored", + "futures-core", + "http", + "http-body", + "http-body-util", + "hyper", + "hyper-util", + "log", + "pin-project-lite", + "rand", + "regex", + "serde_json", + "serde_urlencoded", + "similar", + "tokio", +] + [[package]] name = "native-tls" version = "0.2.18" @@ -881,6 +941,16 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + [[package]] name = "parking_lot_core" version = "0.9.12" @@ -1042,6 +1112,29 @@ dependencies = [ "bitflags", ] +[[package]] +name = "regex" +version = "1.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + [[package]] name = "regex-syntax" version = "0.8.10" @@ -1277,6 +1370,12 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "similar" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" + [[package]] name = "slab" version = "0.4.12" @@ -1421,6 +1520,7 @@ dependencies = [ "bytes", "libc", "mio", + "parking_lot", "pin-project-lite", "socket2", "tokio-macros", @@ -1478,6 +1578,7 @@ dependencies = [ "clap", "dashmap", "futures", + "mockito", "proptest", "reqwest", "serde", @@ -1485,6 +1586,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", + "top_drives", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index a2de455..2b8f724 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,12 @@ clap = { version = "4", features = ["derive"] } thiserror = "2" dashmap = "6" +[features] +default = [] +test-utils = [] + [dev-dependencies] +top_drives = { path = ".", features = ["test-utils"] } tempfile = "3.10.1" proptest = "1" +mockito = "1" diff --git a/docs/API.md b/docs/API.md index 70428fe..ac80cfa 100644 --- a/docs/API.md +++ b/docs/API.md @@ -8,7 +8,14 @@ This document provides comprehensive API documentation for the Top Drives Record ``` src/ -├── lib.rs # Shared library: types, utilities, error handling +├── lib.rs # Thin re-export module: declares sub-modules, re-exports public types +├── types.rs # Core types: AppError, Car, BestCarRecord, TripletKey +├── context.rs # Analysis state: AnalysisContext, CarCounts, compute_car_counts +├── user_data.rs # User data: MyCarsData, load_my_cars_data +├── translate.rs # Translation: translate_track_name, translate_track_type +├── http.rs # HTTP: get_file (async download with retry) +├── csv_util.rs # CSV utilities: compare_cars, escape_csv_field +├── test_utils.rs # Test helpers: make_record, make_car ├── records.rs # Binary entry point: CLI definition and dispatch ├── commands/ │ ├── mod.rs # Command dispatch, run_analysis(), load_my_cars_or_warn() @@ -30,7 +37,7 @@ src/ ## Shared Library (`src/lib.rs`) -Common types and utilities shared across the project. +Thin re-export module that declares 6 focused sub-modules and re-exports all public types for backward compatibility. External code can use `top_drives::Car` without knowing the internal module layout. ### Public Types @@ -176,7 +183,7 @@ pub enum AppError { - Sorts by: RQ → Brand → Model name -#### `compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> Result` +#### `compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> CarCounts` **Aggregates per-car win counts from analysis context** - Iterates all entries in the `DashMap` diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fd834b8..a7a6870 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -8,17 +8,19 @@ The Top Drives Records Analyzer is a Rust CLI tool for analyzing car performance ### 1. Shared Library (`src/lib.rs`) -**Common types and utilities** shared across the project. +**Thin re-export module** that declares 6 focused sub-modules and re-exports all public types for backward compatibility. External code can use `top_drives::Car` without knowing the internal module layout. -#### Key Responsibilities -- **Data Types**: Defines `Car`, `BestCarRecord`, `AnalysisContext`, `CarCounts`, `MyCarsData`, `AppError` -- **Track Name Translation**: Converts track codes to human-readable names via `translate_track_name()` -- **Track Type Translation**: Converts condition codes to surface descriptions via `translate_track_type()` -- **File Download**: Async file download utility with retry logic via `get_file()` -- **User Data Loading**: Parses `my_cars.json` via `load_my_cars_data()` -- **Car Comparison**: Sorting comparator via `compare_cars()` -- **Count Aggregation**: Per-car win count computation via `compute_car_counts()` -- **CSV Escaping**: Safe CSV field formatting via `escape_csv_field()` +#### Sub-modules + +| Module | File | Key Exports | +|--------|------|-------------| +| `types` | `src/types.rs` | `AppError`, `BestCarRecord`, `Car`, `TripletKey` | +| `context` | `src/context.rs` | `AnalysisContext`, `CarCounts`, `compute_car_counts` | +| `user_data` | `src/user_data.rs` | `MyCarsData`, `load_my_cars_data` | +| `translate` | `src/translate.rs` | `translate_track_name`, `translate_track_type` | +| `http` | `src/http.rs` | `get_file` | +| `csv_util` | `src/csv_util.rs` | `compare_cars`, `escape_csv_field` | +| `test_utils` | `src/test_utils.rs` | `make_record`, `make_car` (test helpers) | #### Error Handling All public functions return `Result` where `AppError` is a unified error type: @@ -42,6 +44,7 @@ All public functions return `Result` where `AppError` is a unified - `--cars-db` — Path to car database file (default: `cars_repo.json`) - `--times-dir` — Directory for per-car timing JSON files (default: `times`) - `--tracks-db` — Path to tracks database file (default: `tracks_repo.json`) +- `--output-dir` — Directory for output CSV files (default: `.`) ### 3. Commands Module (`src/commands/`) diff --git a/docs/DATA_STRUCTURES.md b/docs/DATA_STRUCTURES.md index b51bad7..71abdc9 100644 --- a/docs/DATA_STRUCTURES.md +++ b/docs/DATA_STRUCTURES.md @@ -199,7 +199,7 @@ This document describes the JSON data structures used by the Top Drives Records | *value* | String | Human-readable track name (e.g., `"Laguna Seca"`) | **Usage in Application**: -- Loaded by `translate_track_name()` in `src/lib.rs` +- Loaded by `translate_track_name()` in `src/translate.rs` - Compiled into the binary at build time - Fallback: Returns original track code if no mapping found diff --git a/plans/code-review-v2.md b/plans/code-review-v2.md new file mode 100644 index 0000000..6127025 --- /dev/null +++ b/plans/code-review-v2.md @@ -0,0 +1,394 @@ +# Code Review — Top Drives Records Analyzer (v0.3.0) + +**Date**: 2026-05-08 +**Reviewer**: Architect Mode +**Scope**: Full codebase review after round-1 refactoring + +--- + +## Executive Summary + +The codebase has improved significantly since the initial review. The module split is clean, error handling is consistent, and test coverage is solid. The remaining issues are **design-level improvements** — not bugs or correctness problems. They focus on reducing coupling, improving maintainability, and making the codebase more idiomatic Rust. + +**Overall Assessment**: ✅ Good — well-structured, well-documented, well-tested. The suggestions below are quality refinements. + +--- + +## 1. Architecture & Design + +### 1.1 ✅ Strengths + +- **Clean module separation**: `commands/` → `parsing/` → `output/` pipeline is well-defined +- **Proper error propagation**: `AppError` enum with `thiserror` is used consistently +- **Good concurrency model**: `DashMap` + `Semaphore` + `spawn_blocking` is the right approach +- **Zero global mutable state**: `AnalysisContext` is created in `main()` and threaded through +- **Excellent documentation**: Module-level doc comments, function docs, and separate docs/ directory + +### 1.2 🔧 `lib.rs` is a God Module (Medium) + +[`src/lib.rs`](src/lib.rs) at ~650 lines contains **all** shared types and utilities: `AppError`, `Car`, `BestCarRecord`, `AnalysisContext`, `CarCounts`, `MyCarsData`, `compute_car_counts`, `load_my_cars_data`, `compare_cars`, `get_file`, `translate_track_name`, `translate_track_type`, `escape_csv_field`, plus 200 lines of tests. + +**Problem**: Any change to any type requires recompiling all downstream modules. The file has 6+ distinct responsibilities. + +**Suggestion**: Split into focused sub-modules within `lib.rs`: + +``` +src/lib.rs → re-exports only +src/types.rs → Car, BestCarRecord, TripletKey, AppError +src/context.rs → AnalysisContext, CarCounts, compute_car_counts +src/user_data.rs → MyCarsData, load_my_cars_data +src/translate.rs → translate_track_name, translate_track_type +src/http.rs → get_file +src/csv_util.rs → escape_csv_field, compare_cars +``` + +### 1.3 🔧 `output/mod.rs` Has Mixed Responsibilities (Low-Medium) + +[`src/output/mod.rs`](src/output/mod.rs) contains: +- Generic CSV infrastructure (`write_sorted_csv`, `format_counted_row`) +- Specific output commands (`write_best_csv`, `write_trash_csv`, `write_details_csv`, `write_wishlist_files`) +- Business logic (`compute_ideal_wishlist`) +- Stdout output (`print_best_car`) +- 340 lines of tests + +**Suggestion**: Split into: +- `output/csv.rs` — generic CSV infrastructure + `format_counted_row` +- `output/wishlist.rs` — wishlist-specific logic (`compute_ideal_wishlist`, `write_wishlist_files`) +- `output/track_query.rs` — `print_best_car` (stdout output is conceptually different from CSV files) + +### 1.4 🔧 `DashMap>` — Double Arc Wrapping (Low) + +In [`src/lib.rs:127`](src/lib.rs:127): +```rust +pub best_cars: Arc>>, +``` + +Then in [`src/parsing/mod.rs:252`](src/parsing/mod.rs:252): +```rust +let best_cars = Arc::clone(&ctx.best_cars); +``` + +`DashMap` already uses `Arc` internally for its shard locks. Wrapping it in another `Arc` is redundant — `DashMap` itself is `Clone + Send + Sync` and can be shared by cloning the `DashMap` handle directly. + +**Suggestion**: Change to: +```rust +pub best_cars: DashMap>, +``` + +And share via `ctx.best_cars.clone()` (which clones only the Arc handle, not the data). + +--- + +## 2. Code Quality + +### 2.1 🔧 `update_best_car` String Allocation in Hot Loop (Medium) + +In [`src/parsing/mod.rs:184-185`](src/parsing/mod.rs:184): +```rust +let track_id_owned = track_id.to_string(); +let surface_owned = surface.to_string(); + +for rq in car_rq..=MAX_RQ { + let key = (track_id_owned.clone(), rq, surface_owned.clone()); +``` + +For a car with RQ 10, this loop runs **104 times**, cloning two strings per iteration. The `track_id_owned` and `surface_owned` are only needed to construct keys. + +**Suggestion**: Use `Cow` or restructure to avoid per-iteration clones: +```rust +for rq in car_rq..=MAX_RQ { + let key = (track_id.to_string(), rq, surface.to_string()); +``` +This is the same number of allocations but avoids the intermediate owned strings. Or, since `TripletKey = (String, u8, String)`, consider changing the key type to `(Arc, u8, Arc)` to make cloning O(1). + +### 2.2 🔧 `compute_car_counts` Returns `Result` But Never Fails (Low) + +In [`src/lib.rs:156`](src/lib.rs:156): +```rust +pub fn compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> Result { +``` + +This function only does `HashMap` inserts and `DashMap` iteration — it **cannot fail**. The `Result` return is misleading and forces all callers to `.unwrap()`. + +**Suggestion**: Return `CarCounts` directly: +```rust +pub fn compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> CarCounts { +``` + +### 2.3 🔧 Hardcoded Output File Paths (Low-Medium) + +All CSV output functions hardcode their filenames: +- [`write_best_csv`](src/output/mod.rs:78) → `"best.csv"` +- [`write_very_best_csv`](src/output/mod.rs:91) → `"very_best.csv"` +- [`write_trash_csv`](src/output/mod.rs:110) → `"trash.csv"` +- [`write_details_csv`](src/output/mod.rs:148) → `"details.csv"` +- [`write_wishlist_files`](src/output/mod.rs:205) → `"wishlist.csv"`, `"add.csv"`, `"remove.csv"` + +**Problem**: Users cannot specify an output directory. All files are written to CWD. + +**Suggestion**: Add an `--output-dir` CLI option and pass the output directory through to the write functions. Or at minimum, extract the filenames as constants. + +### 2.4 🔧 Magic Number `80` in Trash Filter (Low) + +In [`src/output/mod.rs:114`](src/output/mod.rs:114): +```rust +.filter(|car| !car.prize && car.rq < 80) +``` + +The value `80` is a business rule (minimum RQ for keeping a non-best car) but is not named. + +**Suggestion**: Extract as a named constant: +```rust +const TRASH_MIN_RQ: u8 = 80; +``` + +### 2.5 🔧 `write_details_csv` Doesn't Use `write_sorted_csv` (Low) + +[`write_details_csv`](src/output/mod.rs:148) manually creates a file and iterates, while all other output functions use the generic [`write_sorted_csv`](src/output/mod.rs:35). This is because `details.csv` has a different row format, but the pattern could still be unified. + +**Suggestion**: Either refactor `write_sorted_csv` to be more flexible, or add a comment explaining why this function is different. + +### 2.6 💡 `#[allow(clippy::new_without_default)]` — Just Implement Default (Low) + +In [`src/lib.rs:132`](src/lib.rs:132): +```rust +#[allow(clippy::new_without_default)] +pub fn new() -> Self { +``` + +Since `AnalysisContext` only contains `Arc>` which implements `Default`, the derive is trivial: + +**Suggestion**: +```rust +#[derive(Default)] +pub struct AnalysisContext { ... } +``` +Then remove the manual `new()` and the allow attribute. + +--- + +## 3. Error Handling + +### 3.1 ✅ Strengths + +- `AppError` with `thiserror` is clean and comprehensive +- `load_my_cars_or_warn` correctly distinguishes `MissingFile` from real errors +- `fetch_times` returns the first error encountered +- `parse_all_times` detects total failure and propagates + +### 3.2 🔧 `writeln!` Errors Silently Propagated via `?` in `write_sorted_csv` (Low) + +In [`src/output/mod.rs:49-52`](src/output/mod.rs:49): +```rust +writeln!(file, "{header}")?; +for i in indices { + format_row(&entries[i], &mut file)?; +} +``` + +The `?` operator converts `std::io::Error` → `AppError::Io` via the `From` impl. This is correct but means a partial CSV file may be left on disk if writing fails mid-way. + +**Suggestion**: Consider writing to a temporary file and renaming on success (same pattern as [`get_file`](src/lib.rs:279)), or at minimum document that partial writes are possible. + +--- + +## 4. Performance + +### 4.1 🔧 `parse_all_times` Clones Every Car (Medium) + +In [`src/parsing/mod.rs:257`](src/parsing/mod.rs:257): +```rust +let car = car.clone(); +``` + +Every car in the database is cloned to move into the async task. For ~8000+ cars, each with 5+ string fields, this is significant. + +**Suggestion**: Use `Arc` or pass only the needed fields (rid, rq, guid, brand, only_name, year, prize) as a smaller struct. + +### 4.2 🔧 `times_dir` Cloned Per Car (Low) + +In [`src/parsing/mod.rs:259`](src/parsing/mod.rs:259): +```rust +let times_dir = times_dir.to_string(); +``` + +This allocates a new String for every car. + +**Suggestion**: Use `Arc` or pass as `&str` by restructuring the spawn. + +### 4.3 💡 Consider `rayon` for `compute_car_counts` (Low) + +[`compute_car_counts`](src/lib.rs:156) iterates all `DashMap` entries sequentially. For large datasets, this could benefit from parallel iteration. + +--- + +## 5. Test Coverage + +### 5.1 ✅ Strengths + +- **Unit tests** in every module (`lib.rs`, `parsing/mod.rs`, `parsing/car_list.rs`, `output/mod.rs`) +- **Integration tests** in `tests/integration_tests.rs` covering full pipelines +- **Property-based tests** in `tests/proptest_tests.rs` for sorting invariants +- **Edge case tests**: empty input, missing files, downvoted times, Test Bowl inversion +- Good use of `tempfile::tempdir()` for file-based tests + +### 5.2 🔧 Duplicated Test Helpers (Medium) + +The `make_record()` and `make_car()` helper functions are duplicated across: +- [`src/parsing/mod.rs:369`](src/parsing/mod.rs:369) +- [`src/output/mod.rs:319`](src/output/mod.rs:319) +- [`tests/integration_tests.rs:17`](tests/integration_tests.rs:17) + +**Suggestion**: Create `tests/common/mod.rs` or a `test_utils` module in `lib.rs` with shared helpers: + +```rust +// src/test_utils.rs (gated behind #[cfg(test)]) +pub fn make_record(car_rid: &str, rq: u8, time: f64, guid: &str) -> BestCarRecord { ... } +pub fn make_car(rid: &str, rq: u8, guid: &str) -> Car { ... } +``` + +### 5.3 🔧 Missing Test Coverage for `get_file` (Medium) + +[`get_file`](src/lib.rs:275) is the most complex function in `lib.rs` (~75 lines with retry logic, temp file handling, empty response detection). It has **zero tests**. + +**Suggestion**: Add tests using a mock HTTP server (e.g., `mockito` crate) or at minimum test the retry/backoff logic. + +### 5.4 🔧 Missing Test Coverage for `fetch_times` and `fetch_car` (Medium) + +[`fetch_times`](src/parsing/mod.rs:317) and [`fetch_car`](src/parsing/mod.rs:351) have no tests. These contain important concurrency and error-handling logic. + +### 5.5 🔧 No Tests for Command Handlers (Low) + +None of the command handlers in `src/commands/` have dedicated tests. While they're thin wrappers, testing `load_my_cars_or_warn`'s graceful degradation and `run_analysis`'s orchestration would be valuable. + +### 5.6 💡 `proptest` Only Tests `compare_cars` and `escape_csv_field` (Low) + +The property-based test suite could be expanded to cover: +- `update_best_car` — properties like "inserting the same car twice doesn't change the result" +- `compute_car_counts` — "count is always >= 1 for any car in `best_cars`" +- `translate_track_name` — "output is never empty for known tracks" + +--- + +## 6. Documentation + +### 6.1 ✅ Strengths + +- All public functions have doc comments with purpose, arguments, and return values +- Module-level `//!` doc comments explain each module's role +- `docs/` directory covers API, architecture, data structures, setup, and user guide +- `CHANGELOG.md` tracks version history + +### 6.2 🔧 `docs/DATA_STRUCTURES.md` May Be Outdated (Low) + +Need to verify that `DATA_STRUCTURES.md` reflects the current struct definitions, especially `AnalysisContext` with `DashMap` and the new `CarCounts` struct. + +### 6.3 🔧 `docs/DEVELOPMENT_SETUP.md` — Missing `mockito` or Test Dependencies (Low) + +If `mockito` or other test dependencies are added, they should be documented. + +### 6.4 💡 No `CONTRIBUTING.md` (Low) + +The README mentions "Contributing" in the table of contents but there's no contributing guide. + +--- + +## 7. Dependencies + +### 7.1 ✅ Strengths + +- Tokio features are properly narrowed +- `thiserror v2`, `dashmap v6`, `clap v4` are all current +- `proptest` and `tempfile` are correctly in `[dev-dependencies]` + +### 7.2 🔧 Consider Adding `mockito` (Low) + +For testing `get_file`'s retry logic and HTTP error handling. + +### 7.3 💡 Consider Replacing `futures::future::join_all` with `tokio::join!` or `futures::join!` (Low) + +In [`src/parsing/mod.rs:274`](src/parsing/mod.rs:274): +```rust +let results = join_all(handles).await; +``` + +`join_all` buffers all futures in memory. For ~8000 cars this means ~8000 `JoinHandle`s allocated upfront. This is fine for the current scale but could be replaced with `FuturesUnordered` for streaming results. + +--- + +## 8. Summary of Recommended Improvements + +### Priority 1 — Design Improvements + +| # | Issue | File | Impact | +|---|-------|------|--------| +| 1 | Split `lib.rs` into sub-modules | `src/lib.rs` | Reduces coupling, faster compile times | +| 2 | Remove outer `Arc` from `DashMap` field | `src/lib.rs:127` | Removes unnecessary indirection | +| 3 | Deduplicate test helpers | Multiple | Reduces maintenance burden | +| 4 | Add `mockito` + tests for `get_file` | `src/lib.rs` | Covers untested critical path | + +### Priority 2 — Code Quality + +| # | Issue | File | Impact | +|---|-------|------|--------| +| 5 | `compute_car_counts` should return `CarCounts` not `Result` | `src/lib.rs:156` | Removes misleading API | +| 6 | Extract `TRASH_MIN_RQ` constant | `src/output/mod.rs:114` | Names business rule | +| 7 | Implement `Default` for `AnalysisContext` | `src/lib.rs:132` | Removes clippy allow | +| 8 | Add `--output-dir` CLI option | `src/records.rs` | User configurability | +| 9 | Atomic CSV writes via temp+rename | `src/output/mod.rs` | Prevents partial files | + +### Priority 3 — Performance & Polish + +| # | Issue | File | Impact | +|---|-------|------|--------| +| 10 | Reduce string cloning in `update_best_car` loop | `src/parsing/mod.rs:184` | Performance | +| 11 | Use `Arc` or slim struct in `parse_all_times` | `src/parsing/mod.rs:257` | Reduces allocations | +| 12 | Split `output/mod.rs` into sub-modules | `src/output/` | Maintainability | +| 13 | Expand proptest coverage | `tests/proptest_tests.rs` | Test confidence | +| 14 | Add command handler tests | `src/commands/` | Test coverage | + +--- + +## Architecture Diagram (Current) + +```mermaid +graph TD + CLI[src/records.rs - CLI Entry Point] --> CMD[src/commands/ - Command Handlers] + CMD --> PARSE[src/parsing/ - Core Parsing] + CMD --> OUT[src/output/ - CSV Output] + PARSE --> LIB[src/lib.rs - Shared Library] + CMD --> LIB + OUT --> LIB + + LIB --> TYPES[Types: Car, BestCarRecord, AppError] + LIB --> CTX[AnalysisContext with DashMap] + LIB --> UTILS[Utilities: translate_*, get_file, escape_csv_field] + + PARSE --> |spawn_blocking| PARSE + PARSE --> |Semaphore limited| FETCH[fetch_times - HTTP Downloads] + + CMD --> |run_analysis| PARSE + CMD --> |compute_car_counts| LIB +``` + +## Proposed Architecture (After Refactoring) + +```mermaid +graph TD + CLI[src/records.rs - CLI Entry Point] --> CMD[src/commands/ - Command Handlers] + CMD --> PARSE[src/parsing/ - Core Parsing] + CMD --> OUT[src/output/ - CSV Output] + PARSE --> LIB[src/lib.rs - Re-exports only] + CMD --> LIB + OUT --> LIB + + LIB --> TYPES[src/types.rs - Car, BestCarRecord, AppError] + LIB --> CTX[src/context.rs - AnalysisContext, CarCounts] + LIB --> USER[src/user_data.rs - MyCarsData] + LIB --> TRANS[src/translate.rs - Track name/type] + LIB --> HTTP[src/http.rs - get_file with retry] + LIB --> CSV[src/csv_util.rs - escape_csv_field, compare_cars] + + OUT --> WISHLIST[src/output/wishlist.rs] + OUT --> TRACK_Q[src/output/track_query.rs] +``` diff --git a/src/commands/all.rs b/src/commands/all.rs index 5c516c6..e1bfcba 100644 --- a/src/commands/all.rs +++ b/src/commands/all.rs @@ -13,17 +13,18 @@ pub async fn run_all( my_cars_path: &str, cars_db_path: &str, times_dir: &str, + output_dir: &str, min_count: usize, max_wishlist: usize, ) -> Result<(), AppError> { let my_data = load_my_cars_or_warn(my_cars_path)?; let (ctx, counts) = run_analysis(&my_data.owned, cars_db_path, times_dir).await?; - output::write_best_csv(&counts)?; - output::write_very_best_csv(&counts, min_count)?; - output::write_trash_csv(&counts, &my_data.owned)?; - output::write_details_csv(&ctx)?; + output::write_best_csv(output_dir, &counts)?; + output::write_very_best_csv(output_dir, &counts, min_count)?; + output::write_trash_csv(output_dir, &counts, &my_data.owned)?; + output::write_details_csv(output_dir, &ctx)?; output::write_wishlist_files( - &counts, &my_data.owned, &my_data.wishlist, min_count, max_wishlist, + output_dir, &counts, &my_data.owned, &my_data.wishlist, min_count, max_wishlist, )?; println!( "Written: best.csv, very_best.csv, trash.csv, details.csv, wishlist.csv, add.csv, remove.csv" diff --git a/src/commands/best.rs b/src/commands/best.rs index 0d5690e..8fafdb3 100644 --- a/src/commands/best.rs +++ b/src/commands/best.rs @@ -9,11 +9,11 @@ use crate::commands::{load_my_cars_or_warn, run_analysis}; use crate::output::{write_best_csv, write_very_best_csv}; /// Executes the best command: generates best.csv and very_best.csv. -pub async fn run_best(my_cars_path: &str, cars_db_path: &str, times_dir: &str, min_count: usize) -> Result<(), AppError> { +pub async fn run_best(my_cars_path: &str, cars_db_path: &str, times_dir: &str, output_dir: &str, min_count: usize) -> Result<(), AppError> { let my_data = load_my_cars_or_warn(my_cars_path)?; let (_ctx, counts) = run_analysis(&my_data.owned, cars_db_path, times_dir).await?; - write_best_csv(&counts)?; - write_very_best_csv(&counts, min_count)?; + write_best_csv(output_dir, &counts)?; + write_very_best_csv(output_dir, &counts, min_count)?; println!("Written: best.csv, very_best.csv"); Ok(()) } diff --git a/src/commands/details.rs b/src/commands/details.rs index 7dcf49a..87f59af 100644 --- a/src/commands/details.rs +++ b/src/commands/details.rs @@ -9,10 +9,10 @@ use crate::commands::{load_my_cars_or_warn, run_analysis}; use crate::output::write_details_csv; /// Executes the details command: generates details.csv. -pub async fn run_details(my_cars_path: &str, cars_db_path: &str, times_dir: &str) -> Result<(), AppError> { +pub async fn run_details(my_cars_path: &str, cars_db_path: &str, times_dir: &str, output_dir: &str) -> Result<(), AppError> { let my_data = load_my_cars_or_warn(my_cars_path)?; let (ctx, _counts) = run_analysis(&my_data.owned, cars_db_path, times_dir).await?; - write_details_csv(&ctx)?; + write_details_csv(output_dir, &ctx)?; println!("Written: details.csv"); Ok(()) } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index e6501d7..1be57a6 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -58,6 +58,6 @@ pub async fn run_analysis( let ctx = AnalysisContext::new(); let owned = Arc::new(owned.clone()); let car_list = parse_all_times(owned, &ctx, cars_db_path, times_dir).await?; - let counts = compute_car_counts(&ctx, car_list)?; + let counts = compute_car_counts(&ctx, car_list); Ok((ctx, counts)) } diff --git a/src/commands/track.rs b/src/commands/track.rs index f4eabe4..30771b3 100644 --- a/src/commands/track.rs +++ b/src/commands/track.rs @@ -8,10 +8,15 @@ use crate::commands::{load_my_cars_or_warn, run_analysis}; use crate::output::print_best_car; /// Executes the track command: shows best car for a specific track/condition/RQ. +/// +/// Note: `_output_dir` is unused because the track command prints to stdout +/// rather than writing a CSV file. It exists for API consistency with other +/// command signatures so the caller doesn't need per-command branching. pub async fn run_track( my_cars_path: &str, cars_db_path: &str, times_dir: &str, + _output_dir: &str, track: &str, condition: &str, rq: u8, diff --git a/src/commands/trash.rs b/src/commands/trash.rs index 98c35be..f86271c 100644 --- a/src/commands/trash.rs +++ b/src/commands/trash.rs @@ -8,10 +8,10 @@ use crate::commands::{load_my_cars_or_warn, run_analysis}; use crate::output::write_trash_csv; /// Executes the trash command: generates trash.csv. -pub async fn run_trash(my_cars_path: &str, cars_db_path: &str, times_dir: &str) -> Result<(), AppError> { +pub async fn run_trash(my_cars_path: &str, cars_db_path: &str, times_dir: &str, output_dir: &str) -> Result<(), AppError> { let my_data = load_my_cars_or_warn(my_cars_path)?; let (_ctx, counts) = run_analysis(&my_data.owned, cars_db_path, times_dir).await?; - write_trash_csv(&counts, &my_data.owned)?; + write_trash_csv(output_dir, &counts, &my_data.owned)?; println!("Written: trash.csv"); Ok(()) } diff --git a/src/commands/wishlist.rs b/src/commands/wishlist.rs index 77feb41..8d0d0d8 100644 --- a/src/commands/wishlist.rs +++ b/src/commands/wishlist.rs @@ -12,13 +12,14 @@ pub async fn run_wishlist( my_cars_path: &str, cars_db_path: &str, times_dir: &str, + output_dir: &str, min_count: usize, max_wishlist: usize, ) -> Result<(), AppError> { let my_data = load_my_cars_or_warn(my_cars_path)?; let (_ctx, counts) = run_analysis(&my_data.owned, cars_db_path, times_dir).await?; write_wishlist_files( - &counts, &my_data.owned, &my_data.wishlist, min_count, max_wishlist, + output_dir, &counts, &my_data.owned, &my_data.wishlist, min_count, max_wishlist, )?; println!("Written: wishlist.csv, add.csv, remove.csv"); Ok(()) diff --git a/src/context.rs b/src/context.rs new file mode 100644 index 0000000..b22275e --- /dev/null +++ b/src/context.rs @@ -0,0 +1,189 @@ +//! Analysis context and aggregated car counts. +//! +//! Contains the mutable analysis state (`AnalysisContext`) and the +//! aggregated per-car win counts (`CarCounts`) computed from it. + +use std::collections::HashMap; +use std::sync::Arc; + +use dashmap::DashMap; + +use crate::types::{BestCarRecord, Car, TripletKey}; + +/// Holds the mutable analysis state for a records analysis run. +/// +/// Replaces the former global `static BEST_CARS`. Created in `main()` and +/// passed through the call chain so that no global mutable state is needed. +/// +/// Uses `Arc` for concurrent access from spawned tasks. +/// `Arc::clone` only increments the reference count, so all tasks share +/// the same underlying map. +pub struct AnalysisContext { + /// Best car records for each track/RQ/surface combination. + /// Wrapped in `Arc` so it can be cheaply shared across async tasks. + pub best_cars: Arc>>, +} + +impl Default for AnalysisContext { + fn default() -> Self { + Self::new() + } +} + +impl AnalysisContext { + /// Create a new, empty analysis context. + pub fn new() -> Self { + AnalysisContext { + best_cars: Arc::new(DashMap::new()), + } + } +} + +/// Aggregated per-car win counts across all track/RQ/condition combinations. +/// Computed once from `AnalysisContext` and shared across all output functions. +pub struct CarCounts { + /// Map from car_rid → (BestCarRecord, count of tracks where this car is best) + pub by_rid: HashMap, + /// Map from guid → car_rid (for O(1) ownership/wishlist lookups) + pub guid_to_rid: HashMap, + /// Map from guid → Car (for O(1) lookups of car metadata by GUID). + /// Built once in `compute_car_counts()` and reused by trash/wishlist output. + pub car_map: HashMap, + /// Full car list from the database (kept for any callers that need iteration). + pub car_list: Vec, +} + +/// Computes per-car win counts from the analysis context. +/// +/// Returns a `CarCounts` struct that can be shared across all output functions. +/// This function cannot fail — it only performs `DashMap` iteration and +/// `HashMap` inserts. +pub fn compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> CarCounts { + let mut by_rid: HashMap = HashMap::new(); + let mut guid_to_rid: HashMap = HashMap::new(); + + for entry in ctx.best_cars.iter() { + for best_car in entry.value() { + let rid_entry = by_rid + .entry(best_car.car_rid.clone()) + .or_insert_with(|| (best_car.clone(), 0usize)); + rid_entry.1 += 1; + // Keep the record with the best (lowest) time for this car. + if best_car.time < rid_entry.0.time { + rid_entry.0 = best_car.clone(); + } + // HashMap iteration order is non-deterministic, but car_rid and guid + // have a 1:1 relationship in the data model, so any mapping is correct. + guid_to_rid.entry(best_car.guid.clone()) + .or_insert_with(|| best_car.car_rid.clone()); + } + } + + // Build guid → Car lookup once, reused by trash/wishlist output functions. + let car_map: HashMap = car_list.iter() + .map(|c| (c.guid.clone(), c.clone())) + .collect(); + + CarCounts { by_rid, guid_to_rid, car_map, car_list } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_compute_car_counts_basic() { + let ctx = AnalysisContext::new(); + let record = BestCarRecord { + car_rid: "Test_Car".to_string(), + brand: "TestBrand".to_string(), + only_name: "TestModel".to_string(), + year: 2020, + rq: 85, + config: "111".to_string(), + time: 100.0, + owned: true, + prize: false, + guid: "guid_test".to_string(), + }; + + // Manually insert into best_cars + ctx.best_cars.insert( + ("Laguna Seca".to_string(), 85, "Dry".to_string()), + vec![record.clone()], + ); + ctx.best_cars.insert( + ("Monaco Streets".to_string(), 85, "Dry".to_string()), + vec![record.clone()], + ); + + let car_list = vec![Car { + rid: "Test_Car".to_string(), + brand: "TestBrand".to_string(), + only_name: "TestModel".to_string(), + year: 2020, + rq: 85, + prize: false, + guid: "guid_test".to_string(), + }]; + + let counts = compute_car_counts(&ctx, car_list); + + // Should have exactly 1 entry in by_rid + assert_eq!(counts.by_rid.len(), 1); + let (rec, count) = counts.by_rid.get("Test_Car").unwrap(); + assert_eq!(count, &2); // best on 2 tracks + assert_eq!(rec.car_rid, "Test_Car"); + + // guid_to_rid should map guid → rid + assert_eq!(counts.guid_to_rid.get("guid_test"), Some(&"Test_Car".to_string())); + + // car_map should have the car + assert!(counts.car_map.contains_key("guid_test")); + assert_eq!(counts.car_list.len(), 1); + } + + #[test] + fn test_compute_car_counts_keeps_best_time() { + let ctx = AnalysisContext::new(); + + let record_fast = BestCarRecord { + car_rid: "Car_A".to_string(), + brand: "Brand".to_string(), + only_name: "Model".to_string(), + year: 2020, + rq: 85, + config: "111".to_string(), + time: 90.0, + owned: false, + prize: false, + guid: "guid_a".to_string(), + }; + let record_slow = BestCarRecord { + car_rid: "Car_A".to_string(), + brand: "Brand".to_string(), + only_name: "Model".to_string(), + year: 2020, + rq: 85, + config: "233".to_string(), + time: 110.0, + owned: false, + prize: false, + guid: "guid_a".to_string(), + }; + + ctx.best_cars.insert( + ("Track1".to_string(), 85, "Dry".to_string()), + vec![record_fast], + ); + ctx.best_cars.insert( + ("Track2".to_string(), 85, "Dry".to_string()), + vec![record_slow], + ); + + let counts = compute_car_counts(&ctx, vec![]); + let (rec, count) = counts.by_rid.get("Car_A").unwrap(); + assert_eq!(*count, 2); + assert_eq!(rec.time, 90.0); // Should keep the faster time + } +} diff --git a/src/csv_util.rs b/src/csv_util.rs new file mode 100644 index 0000000..e621c3a --- /dev/null +++ b/src/csv_util.rs @@ -0,0 +1,66 @@ +//! CSV escaping and car comparison utilities. +//! +//! Provides shared helper functions used across multiple output modules. + +use crate::types::Car; + +/// Escapes a field for safe inclusion in a CSV file. +/// +/// If the field contains a comma, quote, newline, or carriage return, +/// it is wrapped in double-quotes with internal quotes doubled. +pub fn escape_csv_field(field: &str) -> String { + if field.contains(',') || field.contains('"') || field.contains('\n') || field.contains('\r') { + format!("\"{}\"", field.replace('"', "\"\"")) + } else { + field.to_string() + } +} + +/// Comparison function for sorting cars by RQ, then brand, then model name. +/// +/// Used as a shared comparator to ensure consistent ordering across output functions. +pub fn compare_cars(a: &Car, b: &Car) -> std::cmp::Ordering { + a.rq.cmp(&b.rq) + .then(a.brand.cmp(&b.brand)) + .then(a.only_name.cmp(&b.only_name)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_escape_csv_field() { + // Simple field — no escaping needed + assert_eq!(escape_csv_field("hello"), "hello"); + // Field with comma + assert_eq!(escape_csv_field("hello,world"), "\"hello,world\""); + // Field with quote + assert_eq!(escape_csv_field("say \"hi\""), "\"say \"\"hi\"\"\""); + // Field with newline + assert_eq!(escape_csv_field("line1\nline2"), "\"line1\nline2\""); + } + + #[test] + fn test_compare_cars_ordering() { + let car_a = Car { + rid: "A".to_string(), brand: "BMW".to_string(), only_name: "M3".to_string(), + year: 2020, rq: 80, prize: false, guid: "guid_a".to_string(), + }; + let car_b = Car { + rid: "B".to_string(), brand: "Audi".to_string(), only_name: "RS6".to_string(), + year: 2021, rq: 90, prize: false, guid: "guid_b".to_string(), + }; + let car_c = Car { + rid: "C".to_string(), brand: "BMW".to_string(), only_name: "M5".to_string(), + year: 2022, rq: 80, prize: false, guid: "guid_c".to_string(), + }; + + // Lower RQ sorts first + assert_eq!(compare_cars(&car_a, &car_b), std::cmp::Ordering::Less); + // Same RQ, same brand, then model alphabetically (M3 < M5) + assert_eq!(compare_cars(&car_a, &car_c), std::cmp::Ordering::Less); + // Higher RQ sorts last + assert_eq!(compare_cars(&car_b, &car_a), std::cmp::Ordering::Greater); + } +} diff --git a/src/http.rs b/src/http.rs new file mode 100644 index 0000000..39bfffb --- /dev/null +++ b/src/http.rs @@ -0,0 +1,103 @@ +//! HTTP file download utility with retry logic. +//! +//! Provides the [`get_file`] function for downloading files from remote URLs +//! with automatic retry on transient failures. + +use std::fs; + +use reqwest::get; + +use crate::types::AppError; + +/// Downloads a file from a URL and saves it locally. +/// +/// Returns `Ok(())` on success, or an error describing what went wrong. +/// +/// Writes to a temporary file first, then renames on success, so a failed +/// download never deletes the existing file. +/// +/// Retries up to `MAX_FETCH_RETRIES` times on transient failures (server errors, +/// connection errors, or empty responses). Permanent HTTP errors (4xx) fail immediately. +pub async fn get_file(file_url: &str, local_file_path: &str) -> Result<(), AppError> { + /// Maximum number of retries when the server returns an empty response. + const MAX_FETCH_RETRIES: usize = 3; + + // Parse the URL once so malformed URLs are caught before any network I/O. + // Store as string so we avoid cloning a `Url` on each retry attempt. + let url = reqwest::Url::parse(file_url) + .map_err(|e| AppError::Network(format!("Invalid URL {file_url}: {e}")))?; + let url_str = url.as_str(); + + let tmp_path = format!("{local_file_path}.tmp"); + let _ = fs::remove_file(&tmp_path); // clean stale temp file, OK to ignore + + let mut attempt = 0usize; + loop { + match get(url_str).await { + Ok(response) => { + let status = response.status(); + if !status.is_success() { + // Only retry on server errors (5xx); client errors (4xx) are permanent. + if status.is_server_error() && attempt < MAX_FETCH_RETRIES { + eprintln!( + "Warning: HTTP {} from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...", + status, + attempt + 1, + ); + tokio::time::sleep( + std::time::Duration::from_millis(500 * (attempt + 1) as u64) + ).await; + attempt += 1; + continue; + } + return Err(AppError::Network( + format!("HTTP {status} from {file_url}") + )); + } + match response.text().await { + Ok(content) => { + if content.is_empty() { + if attempt < MAX_FETCH_RETRIES { + eprintln!( + "Warning: empty response from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...", + attempt + 1, + ); + tokio::time::sleep( + std::time::Duration::from_millis(500 * (attempt + 1) as u64) + ).await; + attempt += 1; + continue; + } + return Err(AppError::Network( + format!("Empty response from {file_url} after {MAX_FETCH_RETRIES} retries") + )); + } + fs::write(&tmp_path, &content).map_err(AppError::Io)?; + return fs::rename(&tmp_path, local_file_path).map_err(AppError::Io); + } + Err(e) => { + return Err(AppError::Network( + format!("Error reading response from {file_url}: {e}") + )); + } + } + } + Err(error) => { + if attempt < MAX_FETCH_RETRIES { + eprintln!( + "Warning: failed to fetch {file_url} (attempt {}/{MAX_FETCH_RETRIES}): {error}", + attempt + 1, + ); + tokio::time::sleep( + std::time::Duration::from_millis(500 * (attempt + 1) as u64) + ).await; + attempt += 1; + continue; + } + return Err(AppError::Network( + format!("Failed to fetch {file_url} after {MAX_FETCH_RETRIES} retries: {error}") + )); + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 2599267..adf540d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,637 +17,33 @@ //! - **`load_my_cars_data()`** — loads and parses `my_cars.json` //! - **`compare_cars()`** — sorting comparator for `Car` //! - **`compute_car_counts()`** — aggregates per-car win counts from analysis context +//! - **`escape_csv_field()`** — CSV-safe field escaping -use std::collections::{HashMap, HashSet}; -use std::fs; -use std::fs::File; -use std::io::BufReader; -use std::sync::{Arc, OnceLock}; +// Sub-modules +pub mod context; +pub mod csv_util; +pub mod http; +pub mod translate; +pub mod types; +pub mod user_data; -use dashmap::DashMap; -use reqwest::get; -use serde::Deserialize; -use serde_json::Value; - -/// Custom error type for the Top Drives application. +/// Shared test utilities. /// -/// Provides a unified error handling strategy across all modules, -/// replacing the mix of `Result<(), String>`, `std::io::Result`, and -/// silent `eprintln!` + early return patterns. -#[derive(Debug, thiserror::Error)] -pub enum AppError { - /// An I/O error occurred (file not found, permission denied, etc.) - #[error("I/O error: {0}")] - Io(#[from] std::io::Error), - - /// A network error occurred during data fetching - #[error("Network error: {0}")] - Network(String), - - /// A JSON parsing error in a specific file - #[error("Parse error in {file}: {reason}")] - Parse { - /// The file that failed to parse - file: String, - /// The reason for the parse failure - reason: String, - }, - - /// A required data file is missing - #[error("Missing data file: {0}")] - MissingFile(String), - - /// An internal runtime error (e.g., semaphore closed, task join failure) - #[error("Internal error: {0}")] - Internal(String), -} - -/// Core car information structure containing basic car metadata. -/// Used throughout the application to represent car entities. -#[derive(Debug, Clone, Deserialize)] -pub struct Car { - /// Unique car identifier used in the records database - pub rid: String, - /// Car manufacturer/brand name - pub brand: String, - /// Car model name (without brand) - #[serde(rename = "onlyName")] - pub only_name: String, - /// Car model year - pub year: u16, - /// Rarity level (10-113, affects performance brackets) - pub rq: u8, - /// Whether this car is only available as a prize car - #[serde(default)] - pub prize: bool, - /// Game-specific unique identifier for the car - pub guid: String, -} - -/// Represents the best performing car record for a specific track, RQ, and condition. -/// Contains all necessary information to compare car performance. -#[derive(Debug, Clone)] -pub struct BestCarRecord { - /// Car identifier from the records database - pub car_rid: String, - /// Car manufacturer/brand name - pub brand: String, - /// Car model name (without brand) - pub only_name: String, - /// Car model year - pub year: u16, - /// Rarity level of this specific car configuration - pub rq: u8, - /// Tune configuration string (e.g., "111", "233", "323") - pub config: String, - /// Track time in seconds (lower is better) - pub time: f64, - /// Whether the current user owns this car - pub owned: bool, - /// Whether this car is only available as a prize car - pub prize: bool, - /// Game-specific unique identifier for the car - pub guid: String, -} - -/// Type alias for the composite key used to identify unique track conditions. -/// Consists of: (Track ID, RQ Level, Surface Condition) -pub type TripletKey = (String, u8, String); - -/// Holds the mutable analysis state for a records analysis run. +/// Intended for use in unit and integration tests. Provides helper constructors +/// for [`BestCarRecord`] and [`Car`] to reduce boilerplate in test code. /// -/// Replaces the former global `static BEST_CARS`. Created in `main()` and -/// passed through the call chain so that no global mutable state is needed. -/// -/// Uses `Arc` for concurrent access from spawned tasks. -/// `DashMap` provides sharded internal locking for high-concurrency access. -pub struct AnalysisContext { - /// Best car records for each track/RQ/surface combination. - /// Wrapped in `Arc` so it can be shared across async tasks. - pub best_cars: Arc>>, -} +/// Enabled by the `test-utils` feature flag. This keeps test helpers out of +/// the library's semver public API while remaining accessible to downstream +/// test crates via `top_drives = { features = ["test-utils"] }`. +#[cfg(feature = "test-utils")] +#[doc(hidden)] +pub mod test_utils; -impl AnalysisContext { - /// Create a new, empty analysis context. - #[allow(clippy::new_without_default)] - pub fn new() -> Self { - AnalysisContext { - best_cars: Arc::new(DashMap::new()), - } - } -} - -/// Aggregated per-car win counts across all track/RQ/condition combinations. -/// Computed once from `AnalysisContext` and shared across all output functions. -pub struct CarCounts { - /// Map from car_rid → (BestCarRecord, count of tracks where this car is best) - pub by_rid: HashMap, - /// Map from guid → car_rid (for O(1) ownership/wishlist lookups) - pub guid_to_rid: HashMap, - /// Map from guid → Car (for O(1) lookups of car metadata by GUID). - /// Built once in `compute_car_counts()` and reused by trash/wishlist output. - pub car_map: HashMap, - /// Full car list from the database (kept for any callers that need iteration). - pub car_list: Vec, -} - -/// Computes per-car win counts from the analysis context. -/// Returns a `CarCounts` struct that can be shared across all output functions. -pub fn compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> Result { - let mut by_rid: HashMap = HashMap::new(); - let mut guid_to_rid: HashMap = HashMap::new(); - - for entry in ctx.best_cars.iter() { - for best_car in entry.value() { - let rid_entry = by_rid - .entry(best_car.car_rid.clone()) - .or_insert_with(|| (best_car.clone(), 0usize)); - rid_entry.1 += 1; - // Keep the record with the best (lowest) time for this car. - if best_car.time < rid_entry.0.time { - rid_entry.0 = best_car.clone(); - } - // HashMap iteration order is non-deterministic, but car_rid and guid - // have a 1:1 relationship in the data model, so any mapping is correct. - guid_to_rid.entry(best_car.guid.clone()) - .or_insert_with(|| best_car.car_rid.clone()); - } - } - - // Build guid → Car lookup once, reused by trash/wishlist output functions. - let car_map: HashMap = car_list.iter() - .map(|c| (c.guid.clone(), c.clone())) - .collect(); - - Ok(CarCounts { by_rid, guid_to_rid, car_map, car_list }) -} - -/// Parsed user data from `my_cars.json` -/// -/// Holds both the owned car GUIDs and wishlist GUIDs extracted from a single -/// file read, avoiding duplicate I/O. -#[derive(Debug, Default)] -pub struct MyCarsData { - /// GUIDs of cars the user owns (from `playerDeck[].cardId` where `locked == true`) - pub owned: HashSet, - /// GUIDs of wishlisted cars (from `wishlists[0].cards`) - pub wishlist: HashSet, -} - -/// Loads and parses `my_cars.json` once, extracting both owned and wishlist data. -/// -/// Opens the game export file a single time and extracts: -/// - **Owned cars** from `playerDeck[].cardId` (filtered by `locked == true`) -/// - **Wishlist** from `wishlists[0].cards` -/// -/// # File Source -/// -/// * **File**: `my_cars.json` (exported from Top Drives game) -/// -/// # Error Handling -/// -/// Returns an `AppError` if the file cannot be opened or parsed. -/// Use `MyCarsData::default()` for graceful degradation when the file is optional. -pub fn load_my_cars_data(path: &str) -> Result { - let file = File::open(path).map_err(|e| { - if e.kind() == std::io::ErrorKind::NotFound { - AppError::MissingFile(format!("{path}: {e}")) - } else { - AppError::Io(e) - } - })?; - let reader = BufReader::new(file); - let json: Value = serde_json::from_reader(reader) - .map_err(|e| AppError::Parse { file: path.to_string(), reason: e.to_string() })?; - - // Extract owned car GUIDs from playerDeck - let owned: HashSet = json - .get("playerDeck") - .and_then(|d| d.as_array()) - .map(|deck| { - deck.iter() - .filter_map(|card| { - if card.get("locked").and_then(|v| v.as_bool()).unwrap_or(false) { - card.get("cardId").and_then(|v| v.as_str()).map(|s| s.to_string()) - } else { - None - } - }) - .collect() - }) - .unwrap_or_default(); - - // Extract wishlist GUIDs from wishlists[0].cards - let wishlist: HashSet = json - .get("wishlists") - .and_then(|w| w.get(0)) - .and_then(|w| w.get("cards")) - .and_then(|c| c.as_array()) - .map(|cards| { - cards - .iter() - .filter_map(|c| c.as_str().map(|s| s.to_string())) - .collect() - }) - .unwrap_or_default(); - - Ok(MyCarsData { owned, wishlist }) -} - -/// Comparison function for sorting cars by RQ, then brand, then model name. -/// -/// Used as a shared comparator to ensure consistent ordering across output functions. -pub fn compare_cars(a: &Car, b: &Car) -> std::cmp::Ordering { - a.rq.cmp(&b.rq) - .then(a.brand.cmp(&b.brand)) - .then(a.only_name.cmp(&b.only_name)) -} - -/// Downloads a file from a URL and saves it locally. -/// -/// Returns `Ok(())` on success, or an error describing what went wrong. -/// -/// Writes to a temporary file first, then renames on success, so a failed -/// download never deletes the existing file. -/// -/// Retries up to `MAX_FETCH_RETRIES` times on transient failures (server errors, -/// connection errors, or empty responses). Permanent HTTP errors (4xx) fail immediately. -pub async fn get_file(file_url: &str, local_file_path: &str) -> Result<(), AppError> { - /// Maximum number of retries when the server returns an empty response. - const MAX_FETCH_RETRIES: usize = 3; - - let tmp_path = format!("{local_file_path}.tmp"); - let _ = fs::remove_file(&tmp_path); // clean stale temp file, OK to ignore - - let mut attempt = 0usize; - loop { - match get(file_url).await { - Ok(response) => { - let status = response.status(); - if !status.is_success() { - // Only retry on server errors (5xx); client errors (4xx) are permanent. - if status.is_server_error() && attempt < MAX_FETCH_RETRIES { - eprintln!( - "Warning: HTTP {} from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...", - status, - attempt + 1, - ); - tokio::time::sleep( - std::time::Duration::from_millis(500 * (attempt + 1) as u64) - ).await; - attempt += 1; - continue; - } - return Err(AppError::Network( - format!("HTTP {status} from {file_url}") - )); - } - match response.text().await { - Ok(content) => { - if content.is_empty() { - if attempt < MAX_FETCH_RETRIES { - eprintln!( - "Warning: empty response from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...", - attempt + 1, - ); - tokio::time::sleep( - std::time::Duration::from_millis(500 * (attempt + 1) as u64) - ).await; - attempt += 1; - continue; - } - return Err(AppError::Network( - format!("Empty response from {file_url} after {MAX_FETCH_RETRIES} retries") - )); - } - fs::write(&tmp_path, &content).map_err(AppError::Io)?; - return fs::rename(&tmp_path, local_file_path).map_err(AppError::Io); - } - Err(e) => { - return Err(AppError::Network( - format!("Error reading response from {file_url}: {e}") - )); - } - } - } - Err(error) => { - if attempt < MAX_FETCH_RETRIES { - eprintln!( - "Warning: failed to fetch {file_url} (attempt {}/{MAX_FETCH_RETRIES}): {error}", - attempt + 1, - ); - tokio::time::sleep( - std::time::Duration::from_millis(500 * (attempt + 1) as u64) - ).await; - attempt += 1; - continue; - } - return Err(AppError::Network( - format!("Failed to fetch {file_url} after {MAX_FETCH_RETRIES} retries: {error}") - )); - } - } - } -} - -/// Translates track condition codes to human-readable surface condition names. -/// -/// Converts internal track condition codes (like "00", "01", "10") to descriptive -/// names that represent the actual racing surface conditions in Top Drives. -/// -/// # Arguments -/// -/// * `track_code` - The internal condition code from the timing data -/// -/// # Returns -/// -/// Returns a human-readable string describing the track surface condition: -/// - "00" → "Dry" (standard asphalt) -/// - "01" → "Wet" (rain-affected) -/// - "10" → "Dirt" (off-road conditions) -/// - And many more specialized conditions -/// -/// # Coverage -/// -/// Supports all known track condition codes used in the Top Drives game, -/// including special combinations like "Asphalt Dirt Wet" and "Snow Dirt Wet". -pub fn translate_track_type(track_code: &str) -> &str { - match track_code { - "00" => "Dry", - "01" => "Wet", - "10" => "Dirt", - "11" => "Dirt Wet", - "20" => "Gravel", - "30" => "Ice", - "40" => "Asphalt Dirt", - "41" => "Asphalt Dirt Wet", - "50" => "Sand", - "60" => "Snow", - "70" => "Grass", - "71" => "Grass Wet", - "b0" => "Asphalt Gravel", - "c0" => "Asphalt Sand", - "c1" => "Asphalt Sand Wet", - "d0" => "Asphalt Snow", - "e0" => "Sand Dirt", - "f0" => "Asphalt Grass Dirt", - "g0" => "Ice Snow", - "h1" => "Snow Dirt Wet", - _ => track_code, - } -} - -/// Translates internal track identifiers to human-readable track names. -/// -/// Converts compact internal track codes (like "laguna", "monaco", "testBowl") -/// to their full, descriptive names that appear in the Top Drives game interface. -/// -/// # Arguments -/// -/// * `track_code` - The internal track identifier from the timing data -/// -/// # Returns -/// -/// Returns the full track name as displayed in the game: -/// - "laguna" → "Laguna Seca" -/// - "monaco" → "Monaco Streets" -/// - "testBowl" → "Test Bowl" -/// - And hundreds more track mappings -/// -/// # Coverage -/// -/// Contains mappings for all known tracks in the Top Drives game, including -/// special tracks like drag strips, rally stages, and circuit races. -/// -/// # Note -/// -/// If an unknown track code is provided, the original code is returned -/// unchanged, allowing the system to gracefully handle new tracks. -pub fn translate_track_name(track_code: &str) -> String { - static TRACK_NAMES: OnceLock> = OnceLock::new(); - - let map = TRACK_NAMES.get_or_init(|| { - let json_str = include_str!("../track_names.json"); - let raw: HashMap = - serde_json::from_str(json_str).expect("track_names.json should be valid JSON"); - raw - }); - - map.get(track_code) - .cloned() - .unwrap_or_else(|| track_code.to_string()) -} - -/// Escapes a field for safe inclusion in a CSV file. -/// -/// If the field contains a comma, quote, newline, or carriage return, -/// it is wrapped in double-quotes with internal quotes doubled. -pub fn escape_csv_field(field: &str) -> String { - if field.contains(',') || field.contains('"') || field.contains('\n') || field.contains('\r') { - format!("\"{}\"", field.replace('"', "\"\"")) - } else { - field.to_string() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_translate_track_name() { - assert_eq!(translate_track_name("laguna"), "Laguna Seca"); - assert_eq!(translate_track_name("mnCity"), "Monaco Streets"); - assert_eq!(translate_track_name("testBowl"), "Test Bowl"); - assert_eq!(translate_track_name("unknown_track"), "unknown_track"); - assert_eq!(translate_track_name("drag100"), "0-100mph"); - assert_eq!(translate_track_name("mile4"), "1/4 Mile Drag"); - } - - #[test] - fn test_translate_track_type() { - assert_eq!(translate_track_type("00"), "Dry"); - assert_eq!(translate_track_type("01"), "Wet"); - assert_eq!(translate_track_type("10"), "Dirt"); - assert_eq!(translate_track_type("11"), "Dirt Wet"); - assert_eq!(translate_track_type("30"), "Ice"); - assert_eq!(translate_track_type("60"), "Snow"); - assert_eq!(translate_track_type("99"), "99"); - } - - #[test] - fn test_track_name_completeness() { - let test_tracks = vec![ - "laguna", - "mnCity", - "testBowl", - "drag100", - "mile4", - "airplaneHangars", - "dealsGap", - "northloop", - ]; - - for track in test_tracks { - let name = translate_track_name(track); - assert_ne!(name, track, "Track {} should have a proper name mapping", track); - assert!(!name.is_empty(), "Track name should not be empty"); - } - } - - #[test] - fn test_condition_completeness() { - let test_conditions = vec!["00", "01", "10", "11", "20", "30", "40", "41", "50", "60"]; - - for condition in test_conditions { - let name = translate_track_type(condition); - assert_ne!(name, condition, "Condition {} should have a proper name mapping", condition); - assert!(!name.is_empty(), "Condition name should not be empty"); - } - } - - #[test] - fn test_compare_cars_ordering() { - let car_a = Car { - rid: "A".to_string(), brand: "BMW".to_string(), only_name: "M3".to_string(), - year: 2020, rq: 80, prize: false, guid: "guid_a".to_string(), - }; - let car_b = Car { - rid: "B".to_string(), brand: "Audi".to_string(), only_name: "RS6".to_string(), - year: 2021, rq: 90, prize: false, guid: "guid_b".to_string(), - }; - let car_c = Car { - rid: "C".to_string(), brand: "BMW".to_string(), only_name: "M5".to_string(), - year: 2022, rq: 80, prize: false, guid: "guid_c".to_string(), - }; - - // Lower RQ sorts first - assert_eq!(compare_cars(&car_a, &car_b), std::cmp::Ordering::Less); - // Same RQ, same brand, then model alphabetically (M3 < M5) - assert_eq!(compare_cars(&car_a, &car_c), std::cmp::Ordering::Less); - // Higher RQ sorts last - assert_eq!(compare_cars(&car_b, &car_a), std::cmp::Ordering::Greater); - } - - #[test] - fn test_compute_car_counts_basic() { - let ctx = AnalysisContext::new(); - let record = BestCarRecord { - car_rid: "Test_Car".to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq: 85, - config: "111".to_string(), - time: 100.0, - owned: true, - prize: false, - guid: "guid_test".to_string(), - }; - - // Manually insert into best_cars - { - ctx.best_cars.insert( - ("Laguna Seca".to_string(), 85, "Dry".to_string()), - vec![record.clone()], - ); - ctx.best_cars.insert( - ("Monaco Streets".to_string(), 85, "Dry".to_string()), - vec![record.clone()], - ); - } - - let car_list = vec![Car { - rid: "Test_Car".to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq: 85, - prize: false, - guid: "guid_test".to_string(), - }]; - - let counts = compute_car_counts(&ctx, car_list).unwrap(); - - // Should have exactly 1 entry in by_rid - assert_eq!(counts.by_rid.len(), 1); - let (rec, count) = counts.by_rid.get("Test_Car").unwrap(); - assert_eq!(count, &2); // best on 2 tracks - assert_eq!(rec.car_rid, "Test_Car"); - - // guid_to_rid should map guid → rid - assert_eq!(counts.guid_to_rid.get("guid_test"), Some(&"Test_Car".to_string())); - - // car_map should have the car - assert!(counts.car_map.contains_key("guid_test")); - assert_eq!(counts.car_list.len(), 1); - } - - #[test] - fn test_compute_car_counts_keeps_best_time() { - let ctx = AnalysisContext::new(); - - let record_fast = BestCarRecord { - car_rid: "Car_A".to_string(), - brand: "Brand".to_string(), - only_name: "Model".to_string(), - year: 2020, - rq: 85, - config: "111".to_string(), - time: 90.0, - owned: false, - prize: false, - guid: "guid_a".to_string(), - }; - let record_slow = BestCarRecord { - car_rid: "Car_A".to_string(), - brand: "Brand".to_string(), - only_name: "Model".to_string(), - year: 2020, - rq: 85, - config: "233".to_string(), - time: 110.0, - owned: false, - prize: false, - guid: "guid_a".to_string(), - }; - - { - ctx.best_cars.insert( - ("Track1".to_string(), 85, "Dry".to_string()), - vec![record_fast], - ); - ctx.best_cars.insert( - ("Track2".to_string(), 85, "Dry".to_string()), - vec![record_slow], - ); - } - - let counts = compute_car_counts(&ctx, vec![]).unwrap(); - let (rec, count) = counts.by_rid.get("Car_A").unwrap(); - assert_eq!(*count, 2); - assert_eq!(rec.time, 90.0); // Should keep the faster time - } - - #[test] - fn test_escape_csv_field() { - // Simple field — no escaping needed - assert_eq!(escape_csv_field("hello"), "hello"); - // Field with comma - assert_eq!(escape_csv_field("hello,world"), "\"hello,world\""); - // Field with quote - assert_eq!(escape_csv_field("say \"hi\""), "\"say \"\"hi\"\"\""); - // Field with newline - assert_eq!(escape_csv_field("line1\nline2"), "\"line1\nline2\""); - } - - #[test] - fn test_load_my_cars_data_missing_file() { - let result = load_my_cars_data("nonexistent_file.json"); - assert!(result.is_err()); - match result.unwrap_err() { - AppError::MissingFile(msg) => assert!(msg.contains("nonexistent_file.json")), - other => panic!("Expected MissingFile error, got: {other}"), - } - } -} +// Re-export all public types and functions for backward compatibility. +// External code can use `top_drives::Car` etc. without knowing the internal module layout. +pub use context::{AnalysisContext, CarCounts, compute_car_counts}; +pub use csv_util::{compare_cars, escape_csv_field}; +pub use http::get_file; +pub use translate::{translate_track_name, translate_track_type}; +pub use types::{AppError, BestCarRecord, Car, TripletKey}; +pub use user_data::{MyCarsData, load_my_cars_data}; diff --git a/src/output/mod.rs b/src/output/mod.rs index eb0bbef..c11ff14 100644 --- a/src/output/mod.rs +++ b/src/output/mod.rs @@ -6,9 +6,18 @@ use std::collections::HashSet; use std::fs; use std::io::Write; +use std::path::PathBuf; use top_drives::{BestCarRecord, CarCounts, TripletKey, AnalysisContext, AppError, compare_cars, escape_csv_field}; +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/// Minimum RQ level for keeping a non-best car. Cars below this RQ that are +/// not best on any track and are not prize cars are considered "trash". +const TRASH_MIN_RQ: u8 = 80; + // --------------------------------------------------------------------------- // Sorting helpers // --------------------------------------------------------------------------- @@ -23,6 +32,33 @@ pub fn compare_records(a: &BestCarRecord, b: &BestCarRecord) -> std::cmp::Orderi .then(a.only_name.cmp(&b.only_name)) } +// --------------------------------------------------------------------------- +// Atomic write helper +// --------------------------------------------------------------------------- + +/// Writes to a temporary file first, then renames on success. +/// +/// If the write callback returns an error, the temporary file is cleaned up +/// and no partial file is left at the target path. +fn atomic_write( + path: &PathBuf, + write_fn: impl FnOnce(&mut fs::File) -> Result<(), AppError>, +) -> Result<(), AppError> { + let tmp_path = PathBuf::from(format!("{}.tmp", path.display())); + let _ = fs::remove_file(&tmp_path); // clean stale temp file + + let result: Result<(), AppError> = (|| { + let mut file = fs::File::create(&tmp_path).map_err(AppError::Io)?; + write_fn(&mut file)?; + drop(file); // ensure file is closed before rename + fs::rename(&tmp_path, path).map_err(AppError::Io) + })(); + if result.is_err() { + let _ = fs::remove_file(&tmp_path); // clean up temp file on failure + } + result +} + // --------------------------------------------------------------------------- // CSV output helpers // --------------------------------------------------------------------------- @@ -32,8 +68,12 @@ pub fn compare_records(a: &BestCarRecord, b: &BestCarRecord) -> std::cmp::Orderi /// Generic helper that sorts entries by index (avoiding cloning), writes the header, /// and formats each row. Returns `std::io::Result<()>` so callers can propagate errors /// instead of panicking. +/// +/// Uses [`atomic_write`] internally so a failed write never leaves a partially-written +/// CSV file. pub fn write_sorted_csv( - path: &str, + output_dir: &str, + filename: &str, header: &str, entries: &[T], cmp: impl Fn(&T, &T) -> std::cmp::Ordering, @@ -45,12 +85,14 @@ where let mut indices: Vec = (0..entries.len()).collect(); indices.sort_by(|&a, &b| cmp(&entries[a], &entries[b])); - let mut file = fs::File::create(path).map_err(AppError::Io)?; - writeln!(file, "{header}")?; - for i in indices { - format_row(&entries[i], &mut file)?; - } - Ok(()) + let path = PathBuf::from(output_dir).join(filename); + atomic_write(&path, |file| { + writeln!(file, "{header}")?; + for i in indices { + format_row(&entries[i], file)?; + } + Ok(()) + }) } /// Formats a `(BestCarRecord, usize)` row as `RQ,Brand,Model,Year,Count`. @@ -75,9 +117,10 @@ pub fn format_counted_row(entry: &(BestCarRecord, usize), file: &mut fs::File) - /// Writes best.csv — all cars that are best on at least one track, sorted by RQ. /// Format: RQ,Brand,Model,Year,Count -pub fn write_best_csv(counts: &CarCounts) -> Result<(), AppError> { +pub fn write_best_csv(output_dir: &str, counts: &CarCounts) -> Result<(), AppError> { let entries: Vec<_> = counts.by_rid.values().collect(); write_sorted_csv( + output_dir, "best.csv", "RQ,Brand,Model,Year,Count", &entries, @@ -88,11 +131,12 @@ pub fn write_best_csv(counts: &CarCounts) -> Result<(), AppError> { /// Writes very_best.csv — cars best on >= min_count tracks, sorted by RQ. /// Format: RQ,Brand,Model,Year,Count -pub fn write_very_best_csv(counts: &CarCounts, min_count: usize) -> Result<(), AppError> { +pub fn write_very_best_csv(output_dir: &str, counts: &CarCounts, min_count: usize) -> Result<(), AppError> { let entries: Vec<_> = counts.by_rid.values() .filter(|(_, count)| *count >= min_count) .collect(); write_sorted_csv( + output_dir, "very_best.csv", "RQ,Brand,Model,Year,Count", &entries, @@ -105,16 +149,17 @@ pub fn write_very_best_csv(counts: &CarCounts, min_count: usize) -> Result<(), A // CSV output: trash // --------------------------------------------------------------------------- -/// Writes trash.csv — owned cars not best on any track, RQ < 80, not prize. +/// Writes trash.csv — owned cars not best on any track, RQ < TRASH_MIN_RQ, not prize. /// Format: RQ,Brand,Model,Year -pub fn write_trash_csv(counts: &CarCounts, owned: &HashSet) -> Result<(), AppError> { +pub fn write_trash_csv(output_dir: &str, counts: &CarCounts, owned: &HashSet) -> Result<(), AppError> { let trash: Vec<_> = owned.iter() .filter(|guid| !counts.guid_to_rid.contains_key(*guid)) .filter_map(|guid| counts.car_map.get(guid)) - .filter(|car| !car.prize && car.rq < 80) + .filter(|car| !car.prize && car.rq < TRASH_MIN_RQ) .collect(); write_sorted_csv( + output_dir, "trash.csv", "RQ,Brand,Model,Year", &trash, @@ -145,7 +190,7 @@ pub fn write_trash_csv(counts: &CarCounts, owned: &HashSet) -> Result<() /// RQ,TrackID,Condition,CarRQ,Brand,Model,Year,Config,Time /// 85,laguna,Dry,85,McLaren,F1,1993,233,112.45 /// ``` -pub fn write_details_csv(ctx: &AnalysisContext) -> Result<(), AppError> { +pub fn write_details_csv(output_dir: &str, ctx: &AnalysisContext) -> Result<(), AppError> { // Collect data from DashMap — no global lock needed let mut sorted_data: Vec<(TripletKey, Vec)> = ctx .best_cars @@ -156,26 +201,28 @@ pub fn write_details_csv(ctx: &AnalysisContext) -> Result<(), AppError> { // Sort by RQ then Track ID for consistent output sorted_data.sort_by(|a, b| a.0 .1.cmp(&b.0 .1).then(a.0 .0.cmp(&b.0 .0))); - let mut file = fs::File::create("details.csv")?; - writeln!(file, "RQ,TrackID,Condition,CarRQ,Brand,Model,Year,Config,Time")?; - for (key, records) in sorted_data { - for best_car in records { - writeln!( - file, - "{},{},{},{},{},{},{},{},{}", - key.1, // RQ level - escape_csv_field(&key.0), // Track ID - escape_csv_field(&key.2), // Surface condition - best_car.rq, // Car's RQ - escape_csv_field(&best_car.brand), // Car brand - escape_csv_field(&best_car.only_name), // Car model - best_car.year, // Car year - best_car.config, // Tune configuration - best_car.time // Track time - )?; + let path = PathBuf::from(output_dir).join("details.csv"); + atomic_write(&path, |file| { + writeln!(file, "RQ,TrackID,Condition,CarRQ,Brand,Model,Year,Config,Time")?; + for (key, records) in sorted_data { + for best_car in records { + writeln!( + file, + "{},{},{},{},{},{},{},{},{}", + key.1, // RQ level + escape_csv_field(&key.0), // Track ID + escape_csv_field(&key.2), // Surface condition + best_car.rq, // Car's RQ + escape_csv_field(&best_car.brand), // Car brand + escape_csv_field(&best_car.only_name), // Car model + best_car.year, // Car year + best_car.config, // Tune configuration + best_car.time // Track time + )?; + } } - } - Ok(()) + Ok(()) + }) } // --------------------------------------------------------------------------- @@ -203,6 +250,7 @@ pub fn compute_ideal_wishlist( /// Computes the ideal wishlist once and derives all three output files from it, /// avoiding redundant computation. pub fn write_wishlist_files( + output_dir: &str, counts: &CarCounts, owned: &HashSet, wishlist: &HashSet, @@ -214,6 +262,7 @@ pub fn write_wishlist_files( // --- wishlist.csv --- write_sorted_csv( + output_dir, "wishlist.csv", "RQ,Brand,Model,Year,Count", &ideal, @@ -227,6 +276,7 @@ pub fn write_wishlist_files( .collect(); write_sorted_csv( + output_dir, "add.csv", "RQ,Brand,Model,Year,Count", &to_add, @@ -255,6 +305,7 @@ pub fn write_wishlist_files( } write_sorted_csv( + output_dir, "remove.csv", "RQ,Brand,Model,Year,Count", &to_remove, @@ -314,35 +365,7 @@ mod tests { use super::*; use std::collections::HashSet; use top_drives::{Car, compute_car_counts}; - - // -- Helper: create a test BestCarRecord -- - fn make_record(car_rid: &str, rq: u8, time: f64, guid: &str) -> BestCarRecord { - BestCarRecord { - car_rid: car_rid.to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq, - config: "111".to_string(), - time, - owned: false, - prize: false, - guid: guid.to_string(), - } - } - - // -- Helper: create a test Car -- - fn make_car(rid: &str, rq: u8, guid: &str) -> Car { - Car { - rid: rid.to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq, - prize: false, - guid: guid.to_string(), - } - } + use top_drives::test_utils::{make_record, make_car}; // ======================================================================= // compute_ideal_wishlist tests @@ -363,7 +386,7 @@ mod tests { } let car_list = vec![make_car("Car_A", 85, "guid_a")]; - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); let owned: HashSet = HashSet::new(); let wishlist = compute_ideal_wishlist(&counts, &owned, 10, 300); @@ -388,7 +411,7 @@ mod tests { } let car_list = vec![make_car("Car_A", 85, "guid_a")]; - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); let owned: HashSet = vec!["guid_a".to_string()].into_iter().collect(); let wishlist = compute_ideal_wishlist(&counts, &owned, 10, 300); @@ -420,7 +443,7 @@ mod tests { let car_list: Vec = (0..5u8) .map(|j| make_car(&format!("Car_{j}"), 80 + j, &format!("guid_{j}"))) .collect(); - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); let owned: HashSet = HashSet::new(); let wishlist = compute_ideal_wishlist(&counts, &owned, 10, 3); @@ -435,19 +458,20 @@ mod tests { #[test] fn test_write_sorted_csv_basic() { let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("test.csv"); - let path_str = path.to_str().unwrap(); + let output_dir = dir.path().to_str().unwrap(); let entries = vec![("beta", 2), ("alpha", 1), ("gamma", 3)]; write_sorted_csv( - path_str, + output_dir, + "test.csv", "Name,Value", &entries, |a, b| a.0.cmp(b.0), |entry, file| writeln!(file, "{},{}", entry.0, entry.1), ).unwrap(); + let path = dir.path().join("test.csv"); let content = fs::read_to_string(path).unwrap(); let lines: Vec<&str> = content.lines().collect(); assert_eq!(lines[0], "Name,Value"); @@ -473,17 +497,18 @@ mod tests { let entry = (rec, 5); let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("test.csv"); - let path_str = path.to_str().unwrap(); + let output_dir = dir.path().to_str().unwrap(); write_sorted_csv( - path_str, + output_dir, + "test.csv", "RQ,Brand,Model,Year,Count", &[entry], |_, _| std::cmp::Ordering::Equal, |e, file| format_counted_row(e, file), ).unwrap(); + let path = dir.path().join("test.csv"); let content = fs::read_to_string(path).unwrap(); let lines: Vec<&str> = content.lines().collect(); assert_eq!(lines[1], "85,\"Brand, Inc.\",\"Model \"\"Sport\"\"\",2020,5"); @@ -513,7 +538,7 @@ mod tests { car_d, // not best, prize car — not trash ]; - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); let owned: HashSet = vec![ "guid_a".to_string(), @@ -526,7 +551,7 @@ mod tests { let trash: Vec<_> = owned.iter() .filter(|guid| !counts.guid_to_rid.contains_key(*guid)) .filter_map(|guid| counts.car_map.get(guid)) - .filter(|car| !car.prize && car.rq < 80) + .filter(|car| !car.prize && car.rq < TRASH_MIN_RQ) .collect(); assert_eq!(trash.len(), 1, "Only Car_B should be trash"); @@ -581,7 +606,7 @@ mod tests { #[test] fn test_empty_analysis_context() { let ctx = AnalysisContext::new(); - let counts = compute_car_counts(&ctx, vec![]).unwrap(); + let counts = compute_car_counts(&ctx, vec![]); assert!(counts.by_rid.is_empty()); assert!(counts.guid_to_rid.is_empty()); assert!(counts.car_map.is_empty()); @@ -599,7 +624,7 @@ mod tests { ); let car = make_car("Only_Car", 90, "guid_only"); - let counts = compute_car_counts(&ctx, vec![car]).unwrap(); + let counts = compute_car_counts(&ctx, vec![car]); assert_eq!(counts.by_rid.len(), 1); let (rec, count) = counts.by_rid.get("Only_Car").unwrap(); @@ -620,7 +645,7 @@ mod tests { } let car = make_car("Car_A", 85, "guid_a"); - let counts = compute_car_counts(&ctx, vec![car]).unwrap(); + let counts = compute_car_counts(&ctx, vec![car]); let all_owned: HashSet = vec!["guid_a".to_string()].into_iter().collect(); let wishlist = compute_ideal_wishlist(&counts, &all_owned, 10, 300); @@ -640,7 +665,7 @@ mod tests { } let car = make_car("Car_A", 85, "guid_a"); - let counts = compute_car_counts(&ctx, vec![car]).unwrap(); + let counts = compute_car_counts(&ctx, vec![car]); let no_owned: HashSet = HashSet::new(); let wishlist = compute_ideal_wishlist(&counts, &no_owned, 10, 300); diff --git a/src/parsing/mod.rs b/src/parsing/mod.rs index 468de6d..9a5fde2 100644 --- a/src/parsing/mod.rs +++ b/src/parsing/mod.rs @@ -364,22 +364,7 @@ async fn fetch_car(rid: &str, times_dir: &str) -> Result<(), AppError> { mod tests { use super::*; use std::collections::HashSet; - - // -- Helper: create a test BestCarRecord -- - fn make_record(car_rid: &str, rq: u8, time: f64, guid: &str) -> BestCarRecord { - BestCarRecord { - car_rid: car_rid.to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq, - config: "111".to_string(), - time, - owned: false, - prize: false, - guid: guid.to_string(), - } - } + use top_drives::test_utils::make_record; // ======================================================================= // update_best_car tests diff --git a/src/records.rs b/src/records.rs index 311327c..48a6f8c 100644 --- a/src/records.rs +++ b/src/records.rs @@ -58,6 +58,9 @@ const DEFAULT_TIMES_DIR: &str = "times"; /// Default path for the tracks database file. const DEFAULT_TRACKS_DB_PATH: &str = "tracks_repo.json"; +/// Default directory for output CSV files. +const DEFAULT_OUTPUT_DIR: &str = "."; + // --------------------------------------------------------------------------- // CLI definition // --------------------------------------------------------------------------- @@ -82,6 +85,10 @@ struct Cli { #[arg(long, default_value = DEFAULT_TRACKS_DB_PATH)] tracks_db: String, + /// Directory for output CSV files + #[arg(long, default_value = DEFAULT_OUTPUT_DIR)] + output_dir: String, + #[command(subcommand)] command: Commands, } @@ -154,32 +161,37 @@ async fn main() { async fn run() -> Result<(), AppError> { let cli = Cli::parse(); + // Ensure the output directory exists before dispatching any command. + // create_dir_all(".") is a no-op, so no special-casing needed. + std::fs::create_dir_all(&cli.output_dir)?; + match cli.command { Commands::Fetch => { commands::fetch::run_fetch(&cli.cars_db, &cli.tracks_db, &cli.times_dir).await?; } Commands::Best { min_count } => { - commands::best::run_best(&cli.my_cars, &cli.cars_db, &cli.times_dir, min_count).await?; + commands::best::run_best(&cli.my_cars, &cli.cars_db, &cli.times_dir, &cli.output_dir, min_count).await?; } Commands::Trash => { - commands::trash::run_trash(&cli.my_cars, &cli.cars_db, &cli.times_dir).await?; + commands::trash::run_trash(&cli.my_cars, &cli.cars_db, &cli.times_dir, &cli.output_dir).await?; } Commands::Wishlist { min_count, max_wishlist } => { commands::wishlist::run_wishlist( - &cli.my_cars, &cli.cars_db, &cli.times_dir, min_count, max_wishlist, + &cli.my_cars, &cli.cars_db, &cli.times_dir, &cli.output_dir, min_count, max_wishlist, ).await?; } Commands::All { min_count, max_wishlist } => { commands::all::run_all( - &cli.my_cars, &cli.cars_db, &cli.times_dir, min_count, max_wishlist, + &cli.my_cars, &cli.cars_db, &cli.times_dir, &cli.output_dir, min_count, max_wishlist, ).await?; } Commands::Details => { - commands::details::run_details(&cli.my_cars, &cli.cars_db, &cli.times_dir).await?; + commands::details::run_details(&cli.my_cars, &cli.cars_db, &cli.times_dir, &cli.output_dir).await?; } Commands::Track { track, condition, rq } => { commands::track::run_track( - &cli.my_cars, &cli.cars_db, &cli.times_dir, &track, &condition, rq, + &cli.my_cars, &cli.cars_db, &cli.times_dir, &cli.output_dir, + &track, &condition, rq, ).await?; } } diff --git a/src/test_utils.rs b/src/test_utils.rs new file mode 100644 index 0000000..c75e87f --- /dev/null +++ b/src/test_utils.rs @@ -0,0 +1,41 @@ +//! Shared test utilities for unit and integration tests. +//! +//! Provides helper constructors for [`BestCarRecord`] and [`Car`] to avoid +//! duplication across test modules. + +use crate::{BestCarRecord, Car}; + +/// Creates a test [`BestCarRecord`] with sensible defaults. +/// +/// All string fields use `"TestBrand"` / `"TestModel"` / `"111"` defaults. +/// Only the distinguishing fields (`car_rid`, `rq`, `time`, `guid`) are parameterised. +pub fn make_record(car_rid: &str, rq: u8, time: f64, guid: &str) -> BestCarRecord { + BestCarRecord { + car_rid: car_rid.to_string(), + brand: "TestBrand".to_string(), + only_name: "TestModel".to_string(), + year: 2020, + rq, + config: "111".to_string(), + time, + owned: false, + prize: false, + guid: guid.to_string(), + } +} + +/// Creates a test [`Car`] with sensible defaults. +/// +/// All string fields use `"TestBrand"` / `"TestModel"` defaults. +/// Only the distinguishing fields (`rid`, `rq`, `guid`) are parameterised. +pub fn make_car(rid: &str, rq: u8, guid: &str) -> Car { + Car { + rid: rid.to_string(), + brand: "TestBrand".to_string(), + only_name: "TestModel".to_string(), + year: 2020, + rq, + prize: false, + guid: guid.to_string(), + } +} diff --git a/src/translate.rs b/src/translate.rs new file mode 100644 index 0000000..db4bdbd --- /dev/null +++ b/src/translate.rs @@ -0,0 +1,152 @@ +//! Track name and condition code translation. +//! +//! Converts internal game codes to human-readable track names and +//! surface condition descriptions. + +use std::collections::HashMap; +use std::sync::OnceLock; + +/// Translates track condition codes to human-readable surface condition names. +/// +/// Converts internal track condition codes (like "00", "01", "10") to descriptive +/// names that represent the actual racing surface conditions in Top Drives. +/// +/// # Arguments +/// +/// * `track_code` - The internal condition code from the timing data +/// +/// # Returns +/// +/// Returns a human-readable string describing the track surface condition: +/// - "00" → "Dry" (standard asphalt) +/// - "01" → "Wet" (rain-affected) +/// - "10" → "Dirt" (off-road conditions) +/// - And many more specialized conditions +/// +/// # Coverage +/// +/// Supports all known track condition codes used in the Top Drives game, +/// including special combinations like "Asphalt Dirt Wet" and "Snow Dirt Wet". +pub fn translate_track_type(track_code: &str) -> &str { + match track_code { + "00" => "Dry", + "01" => "Wet", + "10" => "Dirt", + "11" => "Dirt Wet", + "20" => "Gravel", + "30" => "Ice", + "40" => "Asphalt Dirt", + "41" => "Asphalt Dirt Wet", + "50" => "Sand", + "60" => "Snow", + "70" => "Grass", + "71" => "Grass Wet", + "b0" => "Asphalt Gravel", + "c0" => "Asphalt Sand", + "c1" => "Asphalt Sand Wet", + "d0" => "Asphalt Snow", + "e0" => "Sand Dirt", + "f0" => "Asphalt Grass Dirt", + "g0" => "Ice Snow", + "h1" => "Snow Dirt Wet", + _ => track_code, + } +} + +/// Translates internal track identifiers to human-readable track names. +/// +/// Converts compact internal track codes (like "laguna", "monaco", "testBowl") +/// to their full, descriptive names that appear in the Top Drives game interface. +/// +/// # Arguments +/// +/// * `track_code` - The internal track identifier from the timing data +/// +/// # Returns +/// +/// Returns the full track name as displayed in the game: +/// - "laguna" → "Laguna Seca" +/// - "monaco" → "Monaco Streets" +/// - "testBowl" → "Test Bowl" +/// - And hundreds more track mappings +/// +/// # Coverage +/// +/// Contains mappings for all known tracks in the Top Drives game, including +/// special tracks like drag strips, rally stages, and circuit races. +/// +/// # Note +/// +/// If an unknown track code is provided, the original code is returned +/// unchanged, allowing the system to gracefully handle new tracks. +pub fn translate_track_name(track_code: &str) -> String { + static TRACK_NAMES: OnceLock> = OnceLock::new(); + + let map = TRACK_NAMES.get_or_init(|| { + let json_str = include_str!("../track_names.json"); + let raw: HashMap = + serde_json::from_str(json_str).expect("track_names.json should be valid JSON"); + raw + }); + + map.get(track_code) + .cloned() + .unwrap_or_else(|| track_code.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_translate_track_name() { + assert_eq!(translate_track_name("laguna"), "Laguna Seca"); + assert_eq!(translate_track_name("mnCity"), "Monaco Streets"); + assert_eq!(translate_track_name("testBowl"), "Test Bowl"); + assert_eq!(translate_track_name("unknown_track"), "unknown_track"); + assert_eq!(translate_track_name("drag100"), "0-100mph"); + assert_eq!(translate_track_name("mile4"), "1/4 Mile Drag"); + } + + #[test] + fn test_translate_track_type() { + assert_eq!(translate_track_type("00"), "Dry"); + assert_eq!(translate_track_type("01"), "Wet"); + assert_eq!(translate_track_type("10"), "Dirt"); + assert_eq!(translate_track_type("11"), "Dirt Wet"); + assert_eq!(translate_track_type("30"), "Ice"); + assert_eq!(translate_track_type("60"), "Snow"); + assert_eq!(translate_track_type("99"), "99"); + } + + #[test] + fn test_track_name_completeness() { + let test_tracks = vec![ + "laguna", + "mnCity", + "testBowl", + "drag100", + "mile4", + "airplaneHangars", + "dealsGap", + "northloop", + ]; + + for track in test_tracks { + let name = translate_track_name(track); + assert_ne!(name, track, "Track {} should have a proper name mapping", track); + assert!(!name.is_empty(), "Track name should not be empty"); + } + } + + #[test] + fn test_condition_completeness() { + let test_conditions = vec!["00", "01", "10", "11", "20", "30", "40", "41", "50", "60"]; + + for condition in test_conditions { + let name = translate_track_type(condition); + assert_ne!(name, condition, "Condition {} should have a proper name mapping", condition); + assert!(!name.is_empty(), "Condition name should not be empty"); + } + } +} diff --git a/src/types.rs b/src/types.rs new file mode 100644 index 0000000..8d298a7 --- /dev/null +++ b/src/types.rs @@ -0,0 +1,91 @@ +//! Core data types and error definitions. +//! +//! Contains the fundamental types shared across the entire application: +//! car records, error types, and composite key definitions. + +use serde::Deserialize; + +/// Custom error type for the Top Drives application. +/// +/// Provides a unified error handling strategy across all modules, +/// replacing the mix of `Result<(), String>`, `std::io::Result`, and +/// silent `eprintln!` + early return patterns. +#[derive(Debug, thiserror::Error)] +pub enum AppError { + /// An I/O error occurred (file not found, permission denied, etc.) + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), + + /// A network error occurred during data fetching + #[error("Network error: {0}")] + Network(String), + + /// A JSON parsing error in a specific file + #[error("Parse error in {file}: {reason}")] + Parse { + /// The file that failed to parse + file: String, + /// The reason for the parse failure + reason: String, + }, + + /// A required data file is missing + #[error("Missing data file: {0}")] + MissingFile(String), + + /// An internal runtime error (e.g., semaphore closed, task join failure) + #[error("Internal error: {0}")] + Internal(String), +} + +/// Core car information structure containing basic car metadata. +/// Used throughout the application to represent car entities. +#[derive(Debug, Clone, Deserialize)] +pub struct Car { + /// Unique car identifier used in the records database + pub rid: String, + /// Car manufacturer/brand name + pub brand: String, + /// Car model name (without brand) + #[serde(rename = "onlyName")] + pub only_name: String, + /// Car model year + pub year: u16, + /// Rarity level (10-113, affects performance brackets) + pub rq: u8, + /// Whether this car is only available as a prize car + #[serde(default)] + pub prize: bool, + /// Game-specific unique identifier for the car + pub guid: String, +} + +/// Represents the best performing car record for a specific track, RQ, and condition. +/// Contains all necessary information to compare car performance. +#[derive(Debug, Clone)] +pub struct BestCarRecord { + /// Car identifier from the records database + pub car_rid: String, + /// Car manufacturer/brand name + pub brand: String, + /// Car model name (without brand) + pub only_name: String, + /// Car model year + pub year: u16, + /// Rarity level of this specific car configuration + pub rq: u8, + /// Tune configuration string (e.g., "111", "233", "323") + pub config: String, + /// Track time in seconds (lower is better) + pub time: f64, + /// Whether the current user owns this car + pub owned: bool, + /// Whether this car is only available as a prize car + pub prize: bool, + /// Game-specific unique identifier for the car + pub guid: String, +} + +/// Type alias for the composite key used to identify unique track conditions. +/// Consists of: (Track ID, RQ Level, Surface Condition) +pub type TripletKey = (String, u8, String); diff --git a/src/user_data.rs b/src/user_data.rs new file mode 100644 index 0000000..f061126 --- /dev/null +++ b/src/user_data.rs @@ -0,0 +1,99 @@ +//! User data loading from `my_cars.json`. +//! +//! Handles parsing the game export file to extract owned car GUIDs +//! and wishlist entries. + +use std::collections::HashSet; +use std::fs::File; +use std::io::BufReader; + +use serde_json::Value; + +use crate::types::AppError; + +/// Parsed user data from `my_cars.json` +/// +/// Holds both the owned car GUIDs and wishlist GUIDs extracted from a single +/// file read, avoiding duplicate I/O. +#[derive(Debug, Default)] +pub struct MyCarsData { + /// GUIDs of cars the user owns (from `playerDeck[].cardId` where `locked == true`) + pub owned: HashSet, + /// GUIDs of wishlisted cars (from `wishlists[0].cards`) + pub wishlist: HashSet, +} + +/// Loads and parses `my_cars.json` once, extracting both owned and wishlist data. +/// +/// Opens the game export file a single time and extracts: +/// - **Owned cars** from `playerDeck[].cardId` (filtered by `locked == true`) +/// - **Wishlist** from `wishlists[0].cards` +/// +/// # File Source +/// +/// * **File**: `my_cars.json` (exported from Top Drives game) +/// +/// # Error Handling +/// +/// Returns an `AppError` if the file cannot be opened or parsed. +/// Use `MyCarsData::default()` for graceful degradation when the file is optional. +pub fn load_my_cars_data(path: &str) -> Result { + let file = File::open(path).map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + AppError::MissingFile(format!("{path}: {e}")) + } else { + AppError::Io(e) + } + })?; + let reader = BufReader::new(file); + let json: Value = serde_json::from_reader(reader) + .map_err(|e| AppError::Parse { file: path.to_string(), reason: e.to_string() })?; + + // Extract owned car GUIDs from playerDeck + let owned: HashSet = json + .get("playerDeck") + .and_then(|d| d.as_array()) + .map(|deck| { + deck.iter() + .filter_map(|card| { + if card.get("locked").and_then(|v| v.as_bool()).unwrap_or(false) { + card.get("cardId").and_then(|v| v.as_str()).map(|s| s.to_string()) + } else { + None + } + }) + .collect() + }) + .unwrap_or_default(); + + // Extract wishlist GUIDs from wishlists[0].cards + let wishlist: HashSet = json + .get("wishlists") + .and_then(|w| w.get(0)) + .and_then(|w| w.get("cards")) + .and_then(|c| c.as_array()) + .map(|cards| { + cards + .iter() + .filter_map(|c| c.as_str().map(|s| s.to_string())) + .collect() + }) + .unwrap_or_default(); + + Ok(MyCarsData { owned, wishlist }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_load_my_cars_data_missing_file() { + let result = load_my_cars_data("nonexistent_file.json"); + assert!(result.is_err()); + match result.unwrap_err() { + AppError::MissingFile(msg) => assert!(msg.contains("nonexistent_file.json")), + other => panic!("Expected MissingFile error, got: {other}"), + } + } +} diff --git a/tests/http_tests.rs b/tests/http_tests.rs new file mode 100644 index 0000000..c18f644 --- /dev/null +++ b/tests/http_tests.rs @@ -0,0 +1,130 @@ +//! Tests for the HTTP file download utility with mockito mock server. + +use std::fs; + +use mockito::{Server, ServerGuard, Mock}; +use top_drives::get_file; + +/// Helper: create a mock that returns a successful response with the given body. +fn mock_success(server: &mut ServerGuard, body: &str) -> Mock { + server.mock("GET", "/data.json") + .with_status(200) + .with_body(body) + .create() +} + +/// Helper: create a mock that returns the given HTTP status. +fn mock_status(server: &mut ServerGuard, status: usize) -> Mock { + server.mock("GET", "/data.json") + .with_status(status) + .create() +} + +#[tokio::test] +async fn test_get_file_success() { + let mut server = Server::new_async().await; + let m = mock_success(&mut server, "hello world"); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + get_file(&url, out_path.to_str().unwrap()).await.unwrap(); + + assert_eq!(fs::read_to_string(&out_path).unwrap(), "hello world"); + m.assert(); +} + +#[tokio::test] +async fn test_get_file_empty_response_retries_and_fails() { + let mut server = Server::new_async().await; + // Return 200 with empty body — triggers retry, then fails + let m = server.mock("GET", "/data.json") + .with_status(200) + .with_body("") + .expect(4) // 1 initial + 3 retries + .create(); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + let result = get_file(&url, out_path.to_str().unwrap()).await; + assert!(result.is_err(), "Empty response should fail after retries"); + m.assert(); +} + +#[tokio::test] +async fn test_get_file_server_error_retries() { + let mut server = Server::new_async().await; + // Return 500 three times, then 200 + let m_fail = server.mock("GET", "/data.json") + .with_status(500) + .expect(3) + .create(); + let m_ok = mock_success(&mut server, "recovered"); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + get_file(&url, out_path.to_str().unwrap()).await.unwrap(); + + assert_eq!(fs::read_to_string(&out_path).unwrap(), "recovered"); + m_fail.assert(); + m_ok.assert(); +} + +#[tokio::test] +async fn test_get_file_client_error_fails_immediately() { + let mut server = Server::new_async().await; + // 404 should fail immediately without retries + let m = mock_status(&mut server, 404).expect(1).create(); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + let result = get_file(&url, out_path.to_str().unwrap()).await; + assert!(result.is_err(), "404 should fail immediately"); + let err_msg = format!("{}", result.unwrap_err()); + assert!(err_msg.contains("404"), "Error should mention HTTP 404: {err_msg}"); + m.assert(); +} + +#[tokio::test] +async fn test_get_file_persistent_server_error_fails() { + let mut server = Server::new_async().await; + // 503 every time — should retry 3 times then fail + let m = server.mock("GET", "/data.json") + .with_status(503) + .expect(4) // 1 initial + 3 retries + .create(); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + let result = get_file(&url, out_path.to_str().unwrap()).await; + assert!(result.is_err(), "Persistent 503 should fail after retries"); + let err_msg = format!("{}", result.unwrap_err()); + assert!(err_msg.contains("503"), "Error should mention HTTP 503: {err_msg}"); + m.assert(); +} + +#[tokio::test] +async fn test_get_file_atomic_write_no_partial_on_error() { + let mut server = Server::new_async().await; + // Return 404 — immediate failure, no file written + let m = mock_status(&mut server, 404).create(); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + let result = get_file(&url, out_path.to_str().unwrap()).await; + assert!(result.is_err()); + // The output file should NOT exist (atomic write via temp + rename) + assert!(!out_path.exists(), "No partial file should be left on failure"); + m.assert(); +} diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 919eaa5..b057abd 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -11,35 +11,7 @@ use top_drives::{ compare_cars, compute_car_counts, escape_csv_field, load_my_cars_data, translate_track_name, translate_track_type, }; - -// -- Helpers -- - -fn make_record(car_rid: &str, rq: u8, time: f64, guid: &str) -> BestCarRecord { - BestCarRecord { - car_rid: car_rid.to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq, - config: "111".to_string(), - time, - owned: false, - prize: false, - guid: guid.to_string(), - } -} - -fn make_car(rid: &str, rq: u8, guid: &str) -> Car { - Car { - rid: rid.to_string(), - brand: "TestBrand".to_string(), - only_name: "TestModel".to_string(), - year: 2020, - rq, - prize: false, - guid: guid.to_string(), - } -} +use top_drives::test_utils::{make_record, make_car}; // ======================================================================= // Integration: AnalysisContext → CarCounts pipeline @@ -49,7 +21,7 @@ fn make_car(rid: &str, rq: u8, guid: &str) -> Car { fn test_full_pipeline_empty_input() { let ctx = AnalysisContext::new(); let car_list: Vec = vec![]; - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); assert!(counts.by_rid.is_empty()); assert!(counts.guid_to_rid.is_empty()); @@ -70,7 +42,7 @@ fn test_full_pipeline_single_car_multiple_tracks() { } let car = make_car("Ferrari_F40", 95, "guid_f40"); - let counts = compute_car_counts(&ctx, vec![car]).unwrap(); + let counts = compute_car_counts(&ctx, vec![car]); assert_eq!(counts.by_rid.len(), 1); let (rec, count) = counts.by_rid.get("Ferrari_F40").unwrap(); @@ -124,7 +96,7 @@ fn test_full_pipeline_multiple_cars_different_rq() { make_car("Car_C", 100, "guid_c"), ]; - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); assert_eq!(counts.by_rid.len(), 3); @@ -265,7 +237,7 @@ fn test_ownership_filtering_in_counts() { make_car("Unowned_Car", 90, "guid_unowned"), ]; - let counts = compute_car_counts(&ctx, car_list).unwrap(); + let counts = compute_car_counts(&ctx, car_list); // Both should be in by_rid assert_eq!(counts.by_rid.len(), 2); diff --git a/tests/proptest_tests.rs b/tests/proptest_tests.rs index a17d0dc..76efb34 100644 --- a/tests/proptest_tests.rs +++ b/tests/proptest_tests.rs @@ -2,7 +2,8 @@ use proptest::prelude::*; -use top_drives::{compare_cars, Car}; +use top_drives::{compare_cars, compute_car_counts, AnalysisContext, Car}; +use top_drives::test_utils::{make_car, make_record}; /// Generate arbitrary Car instances for property testing. fn car_strategy() -> impl Strategy { @@ -166,4 +167,82 @@ proptest! { assert!(escaped.starts_with('"')); assert!(escaped.ends_with('"')); } + + // ======================================================================= + // compute_car_counts: empty context always yields zero counts + // ======================================================================= + + #[test] + fn compute_car_counts_empty_ctx( + cars in proptest::collection::vec(car_strategy(), 0..20) + ) { + let ctx = AnalysisContext::default(); + let counts = compute_car_counts(&ctx, cars.clone()); + // No best_cars entries → every car has 0 wins → by_rid is empty + assert_eq!(counts.by_rid.len(), 0); + // guid_to_rid is also empty since no car won any track + assert_eq!(counts.guid_to_rid.len(), 0); + // But car_map contains ALL cars from the list + assert_eq!(counts.car_map.len(), cars.len()); + for car in &cars { + assert!(counts.car_map.contains_key(&car.guid)); + } + } + + // ======================================================================= + // compute_car_counts: car with wins has correct count + // ======================================================================= + + #[test] + fn compute_car_counts_reflects_wins( + n_tracks in 1usize..30, + rid in "[a-zA-Z]{3,8}", + guid in "[a-zA-Z0-9_]{3,10}", + rq in 10u8..113u8, + ) { + let ctx = AnalysisContext::default(); + let rec = make_record(&rid, rq, 100.0, &guid); + + // Insert the car as best on n_tracks tracks + for i in 0..n_tracks { + ctx.best_cars.insert( + (format!("track_{i}"), rq, "Dry".to_string()), + vec![rec.clone()], + ); + } + + let car_list = vec![make_car(&rid, rq, &guid)]; + let counts = compute_car_counts(&ctx, car_list); + + let entry = counts.by_rid.get(&rid); + assert!(entry.is_some(), "car should appear in by_rid"); + let (_, count) = entry.unwrap(); + assert_eq!(*count, n_tracks, "count should match number of tracks"); + } + + // ======================================================================= + // compute_car_counts: ownership flag propagates correctly + // ======================================================================= + + #[test] + fn compute_car_counts_ownership( + rid in "[a-zA-Z]{3,8}", + guid in "[a-zA-Z0-9_]{3,10}", + rq in 10u8..113u8, + owned in any::(), + ) { + let ctx = AnalysisContext::default(); + let mut rec = make_record(&rid, rq, 100.0, &guid); + rec.owned = owned; + ctx.best_cars.insert( + ("track_x".to_string(), rq, "Dry".to_string()), + vec![rec], + ); + + let car_list = vec![make_car(&rid, rq, &guid)]; + let counts = compute_car_counts(&ctx, car_list); + + let (record, _) = counts.by_rid.get(&rid).unwrap(); + assert_eq!(record.owned, owned, "owned flag should propagate"); + } }