Phase 1 complete (2/2 plans): - Plan 01-01: Extract AllegroTokenManager — OAuth token logic centralized from 4 classes into dedicated manager class - Plan 01-02: Extract StringHelper — nullableString/normalizeDateTime/ normalizeColorHex extracted from 15+ classes into App\Core\Support\StringHelper; removed 19 duplicate private methods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
14 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous
| phase | plan | type | wave | depends_on | files_modified | autonomous | |||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 01-tech-debt | 01 | execute | 1 |
|
false |
Purpose
The same ~65-line block (resolveAccessToken + forceRefreshToken + requireOAuthData) exists verbatim in 4 classes. A bug in token refresh requires 4 coordinated fixes with high drift risk. This is the highest-severity tech debt item per CONCERNS.md.
Output
- New class:
src/Modules/Settings/AllegroTokenManager.php - 4 service files simplified (private token methods removed,
AllegroTokenManagerinjected) - Wiring updated in
routes/web.phpandsrc/Core/Application.php - CONCERNS.md entry for this issue removed
- ARCHITECTURE.md updated with new class
Affected Files
@src/Modules/Settings/AllegroOrderImportService.php @src/Modules/Settings/AllegroOrdersSyncService.php @src/Modules/Settings/AllegroStatusDiscoveryService.php @src/Modules/Shipments/AllegroShipmentService.php @routes/web.php @src/Core/Application.php
## Required Skills (from SPECIAL-FLOWS.md)| Skill | Priority | When to Invoke | Loaded? | |-------|----------|----------------|---------|| | /feature-dev | required | Before starting implementation | ○ | | /code-review | required | After implementation, before UNIFY | ○ | | sonar-scanner (CLI) | required | After APPLY, before UNIFY | ○ |
BLOCKING: Required skills MUST be loaded before APPLY proceeds.
Skill Invocation Checklist
- /feature-dev loaded
- /code-review loaded (run after implementation)
- sonar-scanner run (run after implementation)
<acceptance_criteria>
AC-1: AllegroTokenManager extracting token resolution
Given the AllegroIntegrationRepository has valid OAuth credentials
When AllegroTokenManager::resolveToken() is called
Then it returns [accessToken: string, environment: string]
AND if the token is within 5 minutes of expiry it first calls refreshAccessToken()
AND if the token is absent it calls refreshAccessToken()
AND after refresh it saves updated tokens via integrationRepository->saveTokens()
AC-2: No private token methods in the 4 consumer classes
Given the refactor is complete
When the 4 service files are read
Then none of them contain resolveAccessToken(), forceRefreshToken(), or requireOAuthData() methods
AND each has AllegroTokenManager injected in its constructor
AC-3: Services retain correct constructor dependencies
Given the refactor is complete
When AllegroOrderImportService and AllegroOrdersSyncService are instantiated
Then they still inject AllegroIntegrationRepository (used for getActiveIntegrationId, getSettings)
AND also inject AllegroTokenManager
When AllegroStatusDiscoveryService and AllegroShipmentService are instantiated
Then they inject AllegroTokenManager instead of AllegroIntegrationRepository + AllegroOAuthClient separately
AC-4: Wiring updated in both places
Given routes/web.php and src/Core/Application.php are read
When searching for instantiation of the 4 services
Then each instantiation passes an AllegroTokenManager instance
AND AllegroTokenManager is constructed with (allegroIntegrationRepository, allegroOAuthClient)
AC-5: App boots without errors
Given the refactor is complete
When the application is loaded in a browser (or PHP -l is run on all changed files)
Then no PHP parse errors or fatal errors occur
</acceptance_criteria>
Before implementing, verify the required /feature-dev skill is loaded. Run /feature-dev in this conversation before proceeding. Type "feature-dev loaded" to continue with implementation Task 1: Create AllegroTokenManager src/Modules/Settings/AllegroTokenManager.php Create `src/Modules/Settings/AllegroTokenManager.php` in namespace `App\Modules\Settings`.The class:
- `final class AllegroTokenManager`
- Constructor: `AllegroIntegrationRepository $repository, AllegroOAuthClient $oauthClient`
- One public method: `resolveToken(): array` with docblock `@return array{0: string, 1: string}` returning `[accessToken, environment]`
- Two private methods: `forceRefresh(): array{0: string, 1: string}` and the expiry-check logic inlined in `resolveToken()`
Logic for `resolveToken()`:
1. Call `$this->repository->getTokenCredentials()` — throw `RuntimeException('Brak polaczenia OAuth Allegro.')` if null
2. Extract `$accessToken`, `$tokenExpiresAt`, `$env` from the oauth array
3. If `$accessToken === ''` → call `forceRefresh()` and return
4. If `$tokenExpiresAt !== ''`:
- Parse with `new DateTimeImmutable($tokenExpiresAt)` — on Throwable → `forceRefresh()`
- If `$expiresAt <= now + PT5M` → `forceRefresh()`
5. Return `[$accessToken, $env]`
Logic for `forceRefresh()`:
1. Load oauth from `$this->repository->getTokenCredentials()` — throw if null
2. Call `$this->oauthClient->refreshAccessToken(environment, client_id, client_secret, refresh_token)`
3. Calculate `$expiresAt` (null if `expires_in` is 0)
4. Preserve old refresh_token if new one is empty
5. Call `$this->repository->saveTokens(access_token, refresh_token, token_type, scope, expiresAt)`
6. Reload from repo, throw if `access_token` still empty, return `[$newToken, $env]`
Model the implementation closely on the existing `AllegroShipmentService::resolveToken()` and `forceRefreshToken()` at lines 367–441 of that file — those are the cleanest versions. Use `declare(strict_types=1)`, proper imports, no unnecessary comments.
Avoid: introducing a public `forceRefresh()` method — keep it private.
Run: php -l "src/Modules/Settings/AllegroTokenManager.php" — should output "No syntax errors detected"
AC-1 satisfied: AllegroTokenManager created with correct resolve/refresh logic
Task 2: Refactor 4 service classes to use AllegroTokenManager
src/Modules/Settings/AllegroOrderImportService.php,
src/Modules/Settings/AllegroOrdersSyncService.php,
src/Modules/Settings/AllegroStatusDiscoveryService.php,
src/Modules/Shipments/AllegroShipmentService.php
For each of the 4 files:
**AllegroOrderImportService** (src/Modules/Settings/AllegroOrderImportService.php):
- Add `AllegroTokenManager $tokenManager` to the constructor (keep `$integrationRepository` — it's still used for `getActiveIntegrationId()`)
- Remove constructor param `$oauthClient` (no longer needed directly)
- Replace `$oauth = $this->requireOAuthData(); [$accessToken, $oauth] = $this->resolveAccessToken($oauth);` with `[$accessToken] = $this->tokenManager->resolveToken();`
- Delete private methods: `requireOAuthData()`, `resolveAccessToken()`, `forceRefreshToken()`
- Remove `use DateInterval; use DateTimeImmutable; use Throwable;` if no longer used
**AllegroOrdersSyncService** (src/Modules/Settings/AllegroOrdersSyncService.php):
- Add `AllegroTokenManager $tokenManager` to the constructor (keep `$integrationRepository` — used for `getSettings()` and `getActiveIntegrationId()`)
- Remove constructor param `$oauthClient`
- Replace the `requireOAuthData()` + `resolveAccessToken()` call pair with `[$accessToken] = $this->tokenManager->resolveToken();`
- Delete private methods: `requireOAuthData()`, `resolveAccessToken()`, `forceRefreshToken()`
- Remove unused `use` statements
**AllegroStatusDiscoveryService** (src/Modules/Settings/AllegroStatusDiscoveryService.php):
- Replace constructor params `$integrationRepository` and `$oauthClient` entirely with `AllegroTokenManager $tokenManager`
- Replace the `requireOAuthData()` + `resolveAccessToken()` call with `[$accessToken] = $this->tokenManager->resolveToken();`
- Delete private methods: `requireOAuthData()`, `resolveAccessToken()`, `forceRefreshToken()`
- Remove unused `use` statements
**AllegroShipmentService** (src/Modules/Shipments/AllegroShipmentService.php):
- Replace constructor params `$integrationRepository` and `$oauthClient` with `AllegroTokenManager $tokenManager`
- Replace `[$token, $env] = $this->resolveToken();` with `[$token, $env] = $this->tokenManager->resolveToken();`
- Delete private methods: `resolveToken()`, `forceRefreshToken()`
- Remove unused `use` statements (DateInterval, DateTimeImmutable, Throwable if not used elsewhere)
- Add `use App\Modules\Settings\AllegroTokenManager;` import
Run: php -l on each of the 4 files. All should report "No syntax errors detected"
AC-2 and AC-3 satisfied: private token methods gone, AllegroTokenManager injected correctly
Task 3: Update wiring and remove concern entry
routes/web.php,
src/Core/Application.php,
.paul/codebase/CONCERNS.md,
DOCS/ARCHITECTURE.md
**routes/web.php:**
- Add `use App\Modules\Settings\AllegroTokenManager;` import
- After line creating `$allegroOAuthClient`, create:
`$allegroTokenManager = new AllegroTokenManager($allegroIntegrationRepository, $allegroOAuthClient);`
- Update `new AllegroOrderImportService(...)`:
- Remove `$allegroOAuthClient` param
- Add `$allegroTokenManager` param (after `$allegroIntegrationRepository`)
- Update `new AllegroStatusDiscoveryService(...)`:
- Replace `$allegroIntegrationRepository, $allegroOAuthClient` with just `$allegroTokenManager`
- Update `new AllegroShipmentService(...)`:
- Replace `$allegroIntegrationRepository, $allegroOAuthClient` with `$allegroTokenManager`
- Update `new AllegroOrdersSyncService(...)` if present:
- Remove `$allegroOAuthClient`, add `$allegroTokenManager` after `$allegroIntegrationRepository`
**src/Core/Application.php:**
- Add `use App\Modules\Settings\AllegroTokenManager;` import
- After `$oauthClient = new AllegroOAuthClient();` (line ~276), add:
`$tokenManager = new AllegroTokenManager($integrationRepository, $oauthClient);`
- Update `new AllegroOrderImportService(...)`:
- Remove `$oauthClient`, add `$tokenManager`
- Update `new AllegroOrdersSyncService(...)`:
- Remove `$oauthClient`, add `$tokenManager`
**.paul/codebase/CONCERNS.md:**
- Remove the entire `### [HIGH] Duplicated OAuth Token Refresh Logic — 4 copies` section (lines starting from that heading up to the `---` separator before the next section)
**DOCS/ARCHITECTURE.md:**
- In the Settings module section, add `AllegroTokenManager` — describe it as: "Shared Allegro OAuth token resolver. Checks expiry and refreshes via AllegroOAuthClient when needed. Injected into all Allegro service classes."
1. Run: php -l routes/web.php — no syntax errors
2. Run: php -l src/Core/Application.php — no syntax errors
3. Grep: grep -r "resolveAccessToken\|forceRefreshToken\|requireOAuthData" src/Modules/Settings/ src/Modules/Shipments/ — should return 0 results
AC-4 satisfied: wiring updated. Concern removed from CONCERNS.md.
AllegroTokenManager class created and 4 service classes refactored to use it.
Wiring updated in routes/web.php and Application.php.
1. Open the application in a browser — verify it loads without a 500 error
2. Navigate to Settings > Allegro — verify the page loads
3. Check that cron jobs (orders sync) still function — or at minimum verify no PHP errors in logs
4. Run: grep -r "resolveAccessToken\|forceRefreshToken\|requireOAuthData" src/ — should return 0 results
Type "approved" if working, or describe the error if something broke
DO NOT CHANGE
src/Modules/Cron/AllegroTokenRefreshHandler.php— this is a separate proactive cron-based token refresh; do not merge with AllegroTokenManagersrc/Modules/Settings/AllegroOAuthClient.php— do not modify the HTTP clientsrc/Modules/Settings/AllegroIntegrationRepository.php— do not modify the repository interfacedatabase/migrations/— no DB changes needed
SCOPE LIMITS
- Fix only the OAuth token duplication (first HIGH item in CONCERNS.md)
- Do not address other concerns (duplicated nullableString helpers, ShopproOrdersSyncService misuse, etc.)
- Do not refactor any logic — only move existing code; do not change behavior
- Do not add tests in this plan
<success_criteria>
- All tasks completed
- All verification checks pass
- No behavior change — only structural extraction
- CONCERNS.md HIGH item #1 removed </success_criteria>