From 0d7cf68771ee8430e26032d3b6032d00c26b999b Mon Sep 17 00:00:00 2001 From: ArthurErlich Date: Sun, 12 Jul 2026 12:43:33 +0200 Subject: [PATCH] chore(todo): checked php stan instalation and configuration. added document php stan erros as well to update readme docs --- TODO.md | 272 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 136 insertions(+), 136 deletions(-) diff --git a/TODO.md b/TODO.md index c007d5e..4ac298a 100644 --- a/TODO.md +++ b/TODO.md @@ -7,33 +7,32 @@ Order matters — later steps assume earlier ones are done. ## 1. Audit cleanup - [x] **Remove `eightpoints/guzzle-bundle` + `idci/graphql-client-bundle`.** - They exist only to build one near-static GitHub GraphQL query string - (`GitHubProvider.php:59-74`); the bundle's own HTTP transport is already - bypassed (comment at `GitHubProvider.php:76-77`). - - [x] Replace - `$graphqlClient->buildQuery(...)->getGraphQLQuery()` with a plain - `sprintf`/heredoc query string. Delete the two packages from - `composer.json`, `config/bundles.php`, and delete - `config/packages/eight_points_guzzle.yaml` + - `config/packages/idci_graphql_client.yaml`. Run `composer update` and - commit the regenerated `composer.lock`. - -- [x] **Dedupe `baseUrl` normalization.** `GitLabProvider.php` (lines 41, - 53) and `GiteaProvider.php` (lines 42, 54) each call - `rtrim($this->baseUrl..., '/')` twice — once in `ping()`, once in - `fetch()`. Compute it once as a `private readonly string $baseUrl` in - the constructor instead. + They exist only to build one near-static GitHub GraphQL query string + (`GitHubProvider.php:59-74`); the bundle's own HTTP transport is already + bypassed (comment at `GitHubProvider.php:76-77`). + - [x] Replace + `$graphqlClient->buildQuery(...)->getGraphQLQuery()` with a plain + `sprintf`/heredoc query string. Delete the two packages from + `composer.json`, `config/bundles.php`, and delete + `config/packages/eight_points_guzzle.yaml` + + `config/packages/idci_graphql_client.yaml`. Run `composer update` and + commit the regenerated `composer.lock`. + +- [x] **Dedupe `baseUrl` normalization.** `GitLabProvider.php` (lines 41, 53) and `GiteaProvider.php` (lines 42, 54) each call + `rtrim($this->baseUrl..., '/')` twice — once in `ping()`, once in + `fetch()`. Compute it once as a `private readonly string $baseUrl` in + the constructor instead. ## 2. Fix stale `config/services.yaml` (found during exploration, blocks step 1 & 4) - [x] `services.yaml` still binds `$username`/`$token`/`$baseUrl` to the - pre-refactor FQCNs (`App\Service\GitHubProvider` etc.) and - `_instanceof: App\Service\ProviderInterface`, left over from the - `Service/` → `Provider/`+`Renderer/` namespace reorg. Update all of - these to `App\Service\Provider\...`. Currently these bindings silently - no-op, and scalar constructor args can't autowire without them — any - edit to these constructors (steps 1, 4) needs this fixed first, or the - container fails to compile. + pre-refactor FQCNs (`App\Service\GitHubProvider` etc.) and + `_instanceof: App\Service\ProviderInterface`, left over from the + `Service/` → `Provider/`+`Renderer/` namespace reorg. Update all of + these to `App\Service\Provider\...`. Currently these bindings silently + no-op, and scalar constructor args can't autowire without them — any + edit to these constructors (steps 1, 4) needs this fixed first, or the + container fails to compile. ## 3. `ContributionStore` (SQLite via native PDO) @@ -46,10 +45,10 @@ Order matters — later steps assume earlier ones are done. > [SQLite datatypes (why `date` is `INTEGER`/unixtime, not `TEXT`)](https://www.sqlite.org/datatype3.html) - [x] New `src/Service/ContributionStore.php`. PDO SQLite, DB file at - `%kernel.project_dir%/var/data/contributions.db` (configurable - constructor arg), table created lazily. **Schema note:** `date` is - stored as a Unix timestamp (`INTEGER`), not `TEXT` — matches the - current implementation: + `%kernel.project_dir%/var/data/contributions.db` (configurable + constructor arg), table created lazily. **Schema note:** `date` is + stored as a Unix timestamp (`INTEGER`), not `TEXT` — matches the + current implementation: ```sql CREATE TABLE IF NOT EXISTS contributions ( provider TEXT NOT NULL, @@ -59,56 +58,56 @@ Order matters — later steps assume earlier ones are done. ) WITHOUT ROWID, STRICT; ``` - [x] `add(string $provider, int $unixtime, int $count): void` — plain - insert (done, needs test coverage — see below). + insert (done, needs test coverage — see below). - [x] Fix `add()`: currently a plain `INSERT`, so re-adding an existing - `(provider, date)` throws a unique-constraint violation instead of - upserting. Switch to - `INSERT ... ON CONFLICT(provider, date) DO UPDATE SET count = excluded.count` - (see SQLite upsert doc above), or keep `add()` insert-only and add a - separate `merge()` for the upsert case used by step 5. + `(provider, date)` throws a unique-constraint violation instead of + upserting. Switch to + `INSERT ... ON CONFLICT(provider, date) DO UPDATE SET count = excluded.count` + (see SQLite upsert doc above), or keep `add()` insert-only and add a + separate `merge()` for the upsert case used by step 5. - [x] `remove(string $provider, int $unixtime): void` — started, has a - bug: `DELETE contributions WHERE ...` is invalid SQL, missing the - `FROM` keyword (must be `DELETE FROM contributions WHERE ...`); also - drop the `LIKE` on `provider` (exact match, use `=`) since it's an - unindexed wildcard scan for what should be an exact key lookup. + bug: `DELETE contributions WHERE ...` is invalid SQL, missing the + `FROM` keyword (must be `DELETE FROM contributions WHERE ...`); also + drop the `LIKE` on `provider` (exact match, use `=`) since it's an + unindexed wildcard scan for what should be an exact key lookup. - [x] **`Contribution` entity.** Small immutable value object - (`src/Entity/Contribution.php`) wrapping one row: `provider` (string), - `date` (unix timestamp int, or `\DateTimeImmutable` — pick one and - use it consistently everywhere, including `add()`/`all()`), `count` - (int). Gives `ContributionStore::all()` and the aggregator something - typed to pass around instead of raw arrays/tuples. + (`src/Entity/Contribution.php`) wrapping one row: `provider` (string), + `date` (unix timestamp int, or `\DateTimeImmutable` — pick one and + use it consistently everywhere, including `add()`/`all()`), `count` + (int). Gives `ContributionStore::all()` and the aggregator something + typed to pass around instead of raw arrays/tuples. - [x] **`ContributionCollection`.** Typed collection - (`src/Entity/ContributionCollection.php`) wrapping - `array` — implement `IteratorAggregate` + `Countable` - at minimum ([`IteratorAggregate`](https://www.php.net/manual/en/class.iteratoraggregate.php), - [`Countable`](https://www.php.net/manual/en/class.countable.php)) so - it can be `foreach`'d and `count()`'d like a normal array. This is - what `ContributionStore::all()` should return instead of a bare - `date => count` array. + (`src/Entity/ContributionCollection.php`) wrapping + `array` — implement `IteratorAggregate` + `Countable` + at minimum ([`IteratorAggregate`](https://www.php.net/manual/en/class.iteratoraggregate.php), + [`Countable`](https://www.php.net/manual/en/class.countable.php)) so + it can be `foreach`'d and `count()`'d like a normal array. This is + what `ContributionStore::all()` should return instead of a bare + `date => count` array. - [x] `latestDate(string $provider): ?int` — `SELECT MAX(date) WHERE provider = ?` - (returns a unix timestamp, not a string, per the schema above). + (returns a unix timestamp, not a string, per the schema above). - [x] `merge(string $provider, array $dateCounts): void` — upsert via - `INSERT ... ON CONFLICT(provider, date) DO UPDATE SET count = excluded.count`; - `$dateCounts` keyed by unix timestamp. + `INSERT ... ON CONFLICT(provider, date) DO UPDATE SET count = excluded.count`; + `$dateCounts` keyed by unix timestamp. - [x] `all(string $provider, ?int $sinceDays = null): ContributionCollection` — - fetch all rows for a provider, or since a given range. `null` returns - full history (no arbitrary cutoff), a value filters to rows where - `date >= (now - sinceDays * 86400)`. **Bug:** current stub in - `ContributionStore.php:58` has invalid PHP (`$provider == null` instead - of `?string $provider = null` — this won't parse) and returns `void` - instead of `ContributionCollection`; fix the signature when implementing. + fetch all rows for a provider, or since a given range. `null` returns + full history (no arbitrary cutoff), a value filters to rows where + `date >= (now - sinceDays * 86400)`. **Bug:** current stub in + `ContributionStore.php:58` has invalid PHP (`$provider == null` instead + of `?string $provider = null` — this won't parse) and returns `void` + instead of `ContributionCollection`; fix the signature when implementing. - [x] `prune(): void` — `DELETE FROM contributions WHERE date < ?` using - the configured retention window (a unix timestamp cutoff); no-ops if - retention is unset/0. + the configured retention window (a unix timestamp cutoff); no-ops if + retention is unset/0. - [x] Constructor takes `?int $retentionDays` bound from new env var - `CONTRIBUTIONS_RETENTION_DAYS` (empty/default = keep forever). *(constructor - param wired; the env binding itself is step 6's job.)* + `CONTRIBUTIONS_RETENTION_DAYS` (empty/default = keep forever). _(constructor + param wired; the env binding itself is step 6's job.)_ - [x] Tests: `tests/Unit/Service/ContributionStoreTest.php` — store & - retrieve, upsert overwrites existing date, `sinceDays` filtering, - `prune()` no-ops when retention unset, `prune()` deletes rows older - than the window when set. + retrieve, upsert overwrites existing date, `sinceDays` filtering, + `prune()` no-ops when retention unset, `prune()` deletes rows older + than the window when set. - [x] Tests: `tests/Unit/Entity/ContributionTest.php` and - `ContributionCollectionTest.php` — construction, iteration, `count()`. + `ContributionCollectionTest.php` — construction, iteration, `count()`. ## 4. Wire `$since`/`$until` through providers @@ -120,70 +119,70 @@ PHP `max_execution_time` fatal can't be caught by `try/catch` at all, so "catch it better" was never on the table. - [ ] `ProviderInterface::startFetch()` → `startFetch(?\DateTimeImmutable $since = null, ?\DateTimeImmutable $until = null): mixed` - (interface already split into `startFetch`/`resolveFetch` for - concurrency — this doc previously said `fetch()`, which predates that - split). + (interface already split into `startFetch`/`resolveFetch` for + concurrency — this doc previously said `fetch()`, which predates that + split). - [ ] `GitHubProvider::startFetch()` — already builds explicit `from`/`to` - GraphQL args (`GitHubProvider.php:51-58`); swap the hardcoded - `-365 days`/`now` for `$since ?? -365 days` / `$until ?? now`. + GraphQL args (`GitHubProvider.php:51-58`); swap the hardcoded + `-365 days`/`now` for `$since ?? -365 days` / `$until ?? now`. - [ ] `GitLabProvider::startFetch()` — add a `before` query param alongside - the existing `after` (`GitLabProvider.php:77-84`), fed by `$until`; - `$since` already flows into `after` (this is what actually shrinks the - pagination loop). + the existing `after` (`GitLabProvider.php:77-84`), fed by `$until`; + `$since` already flows into `after` (this is what actually shrinks the + pagination loop). - [ ] `GiteaProvider::resolveFetch()` — heatmap endpoint has no query - params (always returns full history); add an `$until` upper-bound - filter alongside the existing `$cutoff` lower bound. + params (always returns full history); add an `$until` upper-bound + filter alongside the existing `$cutoff` lower bound. - [ ] Update `GitHubProviderTest.php`, `GitLabProviderTest.php`, - `GiteaProviderTest.php` — add cases asserting a passed `$since`/`$until` - narrows the request window/query params. + `GiteaProviderTest.php` — add cases asserting a passed `$since`/`$until` + narrows the request window/query params. ## 5. Wire `ContributionStore` into `ContributionAggregator` - [ ] Fix `ContributionStore` wiring in `config/services.yaml` first — - it's currently dead code (nothing calls it). The constructor default - `$dbPath` string (`%kernel.project_dir%/var/data/contributions.db`) is - a plain PHP default, not a resolved container parameter; bind it - explicitly (same pattern as the provider bindings): + it's currently dead code (nothing calls it). The constructor default + `$dbPath` string (`%kernel.project_dir%/var/data/contributions.db`) is + a plain PHP default, not a resolved container parameter; bind it + explicitly (same pattern as the provider bindings): ```yaml App\Service\ContributionStore: - arguments: - $dbPath: '%kernel.project_dir%/var/data/contributions.db' - $retentionDays: '%env(int:CONTRIBUTIONS_RETENTION_DAYS)%' + arguments: + $dbPath: "%kernel.project_dir%/var/data/contributions.db" + $retentionDays: "%env(int:CONTRIBUTIONS_RETENTION_DAYS)%" ``` Add `env(CONTRIBUTIONS_RETENTION_DAYS): ''` to `parameters:` (empty → casts to `0` → `prune()`'s existing `retentionDays === 0` check already treats that as "keep forever"). Document the var in `.env`. - [ ] Inject `ContributionStore` into `ContributionAggregator`. - [ ] Per configured provider: `$latest = $store->latestDate($name)` → - `$since = $latest !== null ? (new \DateTimeImmutable('@' . $latest))->modify('-3 days') : null` - (3-day overlap for late corrections — old stored days are immutable and - never re-fetched, only this trailing window + anything new hits the - network), else `null` (first run, provider's own default lookback). - `$until = null` (always "up to now" on the normal request path). + `$since = $latest !== null ? (new \DateTimeImmutable('@' . $latest))->modify('-3 days') : null` + (3-day overlap for late corrections — old stored days are immutable and + never re-fetched, only this trailing window + anything new hits the + network), else `null` (first run, provider's own default lookback). + `$until = null` (always "up to now" on the normal request path). - [ ] `startFetch($since, $until)` / `resolveFetch()` as today, - `$store->merge($name, $fresh)` on success, then read back - `$store->all($name, sinceDays: 371)` for the render window - (53 weeks × 7 days) — a `ContributionCollection` — and merge its - contributions into the returned array (the store becomes the source - of truth for what gets rendered, not the fresh fetch alone). + `$store->merge($name, $fresh)` on success, then read back + `$store->all($name, sinceDays: 371)` for the render window + (53 weeks × 7 days) — a `ContributionCollection` — and merge its + contributions into the returned array (the store becomes the source + of truth for what gets rendered, not the fresh fetch alone). - [ ] Call `$store->prune()` once per `aggregate()` call, after all - providers have merged. + providers have merged. - [ ] Keep the existing try/catch-and-log-per-provider behavior — a - provider failure leaves its DB history stale, doesn't break the render. + provider failure leaves its DB history stale, doesn't break the render. - [ ] Update `ContributionAggregatorTest.php` with store-interaction - (`latestDate` consulted, `merge` called, `all()` feeds the result, - `prune()` runs once) and a case confirming a provider failure leaves - other providers' stored data intact. + (`latestDate` consulted, `merge` called, `all()` feeds the result, + `prune()` runs once) and a case confirming a provider failure leaves + other providers' stored data intact. ## 6. Docker / env - [ ] `docker-compose.yml` — add a `data` named volume mounted at - `/app/var/data` (same pattern as `cache`/`logs`), and pass through - `CONTRIBUTIONS_RETENTION_DAYS: "${CONTRIBUTIONS_RETENTION_DAYS:-}"`. + `/app/var/data` (same pattern as `cache`/`logs`), and pass through + `CONTRIBUTIONS_RETENTION_DAYS: "${CONTRIBUTIONS_RETENTION_DAYS:-}"`. - [ ] `Dockerfile` — add `var/data` to the `mkdir -p` in the `final` stage - alongside `var/cache/prod/pools var/log`, owned by `app`. + alongside `var/cache/prod/pools var/log`, owned by `app`. - [ ] `.env` — document `CONTRIBUTIONS_RETENTION_DAYS` (empty by default), - same style as the existing `ALLOWED_HOSTS` comment. + same style as the existing `ALLOWED_HOSTS` comment. ## 9. `app:contributions:refetch` console command @@ -192,55 +191,56 @@ adding a new host, or if the store needs rebuilding) — bypasses step 5's incremental `latestDate()` window entirely for the providers/range given. - [ ] `src/Command/RefetchContributionsCommand.php`, `#[AsCommand]` + - constructor-promoted readonly properties (matches this codebase's - attribute-first style), using `SymfonyStyle` - ([console styling guide](https://symfony.com/doc/current/console/style.html)). - Inject `iterable $providers` (`#[AutowireIterator('app.provider')]`, - same as `ContributionAggregator`) and `ContributionStore`. + constructor-promoted readonly properties (matches this codebase's + attribute-first style), using `SymfonyStyle` + ([console styling guide](https://symfony.com/doc/current/console/style.html)). + Inject `iterable $providers` (`#[AutowireIterator('app.provider')]`, + same as `ContributionAggregator`) and `ContributionStore`. - [ ] Options: `--provider=github,gitlab` (repeatable/comma-split, - restricts to named provider(s), default = all configured; unknown name - → `$io->error()` + `Command::FAILURE`), `--from=YYYY-MM-DD` (default - today − 365 days), `--to=YYYY-MM-DD` (default today), `--all` (shorthand - for `--from` far enough back — e.g. 2005-01-01 — to mean "existing full - history"; combinable with `--provider`). + restricts to named provider(s), default = all configured; unknown name + → `$io->error()` + `Command::FAILURE`), `--from=YYYY-MM-DD` (default + today − 365 days), `--to=YYYY-MM-DD` (default today), `--all` (shorthand + for `--from` far enough back — e.g. 2005-01-01 — to mean "existing full + history"; combinable with `--provider`). - [ ] **Batch by range**: split `[from, to]` into ≤365-day chunks - (GitHub's GraphQL `contributionsCollection` rejects windows over a - year; chunking also caps GitLab's pagination per call, so a big - `--all` re-fetch can't reintroduce the original runaway-pagination - timeout). Iterate chunks oldest-first. + (GitHub's GraphQL `contributionsCollection` rejects windows over a + year; chunking also caps GitLab's pagination per call, so a big + `--all` re-fetch can't reintroduce the original runaway-pagination + timeout). Iterate chunks oldest-first. - [ ] Per provider, per chunk: `$io->section(...)`, `startFetch($chunkFrom, $chunkTo)` - / `resolveFetch()`, `$store->merge($name, $result)` immediately (don't - accumulate all chunks in memory), `ProgressBar` across chunks. + / `resolveFetch()`, `$store->merge($name, $result)` immediately (don't + accumulate all chunks in memory), `ProgressBar` across chunks. - [ ] Catch per-provider/per-chunk `\Throwable` → `$io->warning(...)`, - continue with remaining chunks/providers (same graceful-degradation - philosophy as `ContributionAggregator`). + continue with remaining chunks/providers (same graceful-degradation + philosophy as `ContributionAggregator`). - [ ] Finish with `$io->table(...)` summary (provider, days written, - chunks fetched, any errors) and `$io->success()`/`$io->error()`; exit - `Command::SUCCESS` if at least one provider fully succeeded, else - `Command::FAILURE`. + chunks fetched, any errors) and `$io->success()`/`$io->error()`; exit + `Command::SUCCESS` if at least one provider fully succeeded, else + `Command::FAILURE`. - [ ] Tests: `tests/Unit/Command/RefetchContributionsCommandTest.php` - using `CommandTester` with fake `ProviderInterface` stubs and a - `:memory:` `ContributionStore` — assert chunking count for a >365-day - range, store ends up populated, unknown `--provider` name fails - cleanly, a provider throwing doesn't abort the others. + using `CommandTester` with fake `ProviderInterface` stubs and a + `:memory:` `ContributionStore` — assert chunking count for a >365-day + range, store ends up populated, unknown `--provider` name fails + cleanly, a provider throwing doesn't abort the others. ## 10. PHPStan -- [ ] `composer require --dev phpstan/phpstan` (plain PHPStan — no - Symfony extension needed for this app's size). -- [ ] Add `phpstan.neon`: `paths: [src, tests]`, `level: 8`. +- [x] `composer require --dev phpstan/phpstan` (plain PHPStan — no + Symfony extension needed for this app's size). +- [x] Add `phpstan.neon`: `paths: [src, tests]`, `level: 8`. - [ ] Add composer script `"phpstan": "phpstan analyse"`. -- [ ] Fix whatever level-8 flags on first run. +- [ ] Update README.md docs with new command and php stan static lintin. +- [ ] Document in PHP-Stan-Errors.md whatever level-8 flags on first run. ## 11. `CLAUDE.md` update - [ ] Architecture section: add the `ContributionStore` (SQLite) tier - between providers and the renderer; note the two-tier cache (1h SVG - cache → SQLite raw-data store → provider APIs). + between providers and the renderer; note the two-tier cache (1h SVG + cache → SQLite raw-data store → provider APIs). - [ ] Environment variables table: add `CONTRIBUTIONS_RETENTION_DAYS`. - [ ] Document `app:contributions:refetch` (options, batching behavior). - [ ] Development section: add `composer phpstan` next to the existing - `vendor/bin/phpunit` commands. + `vendor/bin/phpunit` commands. - [ ] Remove any remaining mentions of the two deleted bundles. ## Verification