Skip to content

refactor(arr): extract AbstractArrClient (F2a, dedup, no behavior change)#16

Merged
detain merged 1 commit into
masterfrom
fix/shared-f2a
Jun 28, 2026
Merged

refactor(arr): extract AbstractArrClient (F2a, dedup, no behavior change)#16
detain merged 1 commit into
masterfrom
fix/shared-f2a

Conversation

@detain

@detain detain commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Step F2a — Extract AbstractArrClient

Findings: CQ2, CQ5 (enabler for B1/F2b). From findings/plan_phlix-shared.md Phase 2.

What

The four near-identical *arr clients (RadarrClient, SonarrClient, ProwlarrClient, BazarrClient) now extend a new abstract class AbstractArrClient that owns:

  • the constructor (baseUrl/apiKey/logger/timeout),
  • buildHeaders(),
  • the GET/POST/PUT/DELETE cURL methods (folded into one private request()),
  • the per-status-code error mapping.

Each subclass keeps only its endpoint-specific methods plus protected function vendorName(): string used in error messages ("Radarr API error: HTTP 500", etc.).

No behavior change

  • Still blocking cURL — intentional for F2a. F2b swaps the transport behind an injected seam, and the single private request() in the base is now the one place it changes.
  • Public class names, method signatures, and thrown exceptions/messages are byte-identical.
  • Diff: +245 / -779 lines (4×~250-500 collapse to one 213-line base + thin subclasses).

Visibility note

The shared baseUrl/apiKey/timeout/logger properties are protected (not private) because the existing constructor tests reflect on the concrete subclass via ReflectionClass($client)->getProperty('baseUrl'), which does not report a parent's private properties. protected keeps reflection working with zero test edits.

How verified (actual output)

  • composer testOK (349 tests, 2838 assertions) — same count, no test files changed
  • composer stan[OK] No errors (PHPStan L9, no baseline)
  • composer cs → clean (PSR-12), exit 0
  • composer psalmNo errors found!

Consumer impact

None — internal refactor, public surface preserved. No version bump; CHANGELOG [Unreleased] ### Changed note added.

Coupling

This is the enabler for F2b (async transport via injected ArrTransportInterface). Do not merge ahead of coordination if F2b is staged to build on this branch.

🤖 Generated with Claude Code

…ts (F2a)

Pull the near-identical constructor, buildHeaders(), GET/POST/PUT/DELETE cURL
methods, and per-status-code error mapping out of RadarrClient/SonarrClient/
ProwlarrClient/BazarrClient into a shared abstract AbstractArrClient. Subclasses
keep only endpoint-specific methods plus a protected vendorName() used in error
messages.

No behavior change: still blocking cURL, identical public class names/methods,
identical thrown exceptions and messages. Properties are protected (not private)
so the existing reflection-based constructor tests pass unchanged. This is the
structural enabler for F2b's injected async transport — the single private
request() in the base is the one place F2b will swap.

Findings: CQ2, CQ5 (enabler for B1). Internal refactor; no consumer impact.

Gate: composer test (349 OK, 2838 assertions), stan (L9 no baseline, OK),
cs (PSR-12 clean), psalm (no errors).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 6.94444% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.38%. Comparing base (17ed149) to head (2352982).

Files with missing lines Patch % Lines
src/Arr/AbstractArrClient.php 7.81% 59 Missing ⚠️
src/Arr/BazarrClient.php 0.00% 2 Missing ⚠️
src/Arr/ProwlarrClient.php 0.00% 2 Missing ⚠️
src/Arr/RadarrClient.php 0.00% 2 Missing ⚠️
src/Arr/SonarrClient.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master      #16       +/-   ##
=============================================
+ Coverage     58.81%   75.38%   +16.57%     
+ Complexity      424      345       -79     
=============================================
  Files            38       39        +1     
  Lines          1265      967      -298     
=============================================
- Hits            744      729       -15     
+ Misses          521      238      -283     
Flag Coverage Δ
phpunit 75.38% <6.94%> (+16.57%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 4 medium · 2 minor

Alerts:
⚠ 6 issues (≤ 0 issues of at least minor severity)

Results:
6 new issues

Category Results
Comprehensibility 1 minor
CodeStyle 1 minor
Complexity 4 medium

View in Codacy

🟢 Metrics -75 complexity · -20 duplication

Metric Results
Complexity -75
Duplication -20

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@detain detain merged commit 6336d08 into master Jun 28, 2026
7 of 9 checks passed
@detain detain deleted the fix/shared-f2a branch June 28, 2026 16:39
detain added a commit that referenced this pull request Jun 28, 2026
Bump Version::VERSION to 0.11.0 and promote the [Unreleased] CHANGELOG
section to [0.11.0] - 2026-06-28, rolling up the four changes merged
since v0.10.1:

- #14 S1: harden RelayHttpRequest with path/method gate + forbidden-header
  helpers (isForbiddenHeader/withoutForbiddenHeaders/assertSafe)
- #15 JwtClaims: strict-aud variant (fromPayloadStrict) + verification/
  round-trip docs (S4+B4)
- #16 refactor(arr): extract AbstractArrClient to dedup the four *arr
  clients (F2a)
- #17 feat(arr): inject async HTTP transport via ArrTransportInterface
  (F2b/B1/P1)

composer.json carries no hardcoded version field (tags drive version),
so it is left unchanged. The git tag is applied by the coordinator
after merge.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant