Skip to content

Commit e989b74

Browse files
authored
Cleanup for 8.0.0: comment audit, drop unneeded files, restructure AGENTS.md (#324)
* Cleanup for 8.0.0: comment audit + drop unneeded files - Comment audit across Sources: the codebase is well-tended (comments are overwhelmingly non-obvious why, external facts, gotchas, and @unchecked/ fatalError justifications). Cut one restatement comment on Duration.seconds. - Drop the duplicate LICENSE.md (older 2023 copy; keep the standard 2024 LICENSE) and repoint the README link. - Drop three unreferenced test fixtures: invalid.json, simple_array.json, simple_dictionary.json. * Drop the roadmap planning doc; CHANGELOG is the release record The roadmap was internal planning; for the release, CHANGELOG.md carries the user-facing history and serves as the v8 release notes. Drop docs/roadmap.md and remove AGENTS.md's stale Roadmap section + its '(roadmap #8)' reference, pointing release history at CHANGELOG.md. CHANGELOG left as-is — already accurate and release-notes-ready (covers the actor migration, typed bodies, error model, events, interceptors, cache lifecycle, and the statusCodeType/composedURL deltas). * Restructure AGENTS.md per best-practice guidelines Apply the AGENTS.md guidance from GitHub's 2,500-repo study (and peers): commands early, a scannable source map with purpose annotations, tight cross-file invariants instead of paragraph-heavy essays, a three-tier Always/Ask/Never boundaries block, and progressive disclosure — the deep rationale now lives in the (just-audited) code comments rather than being duplicated here. Notes it's a refinement layer over the parent AGENTS.md files; no persona (the walk-up doesn't use one).
1 parent 82a605b commit e989b74

8 files changed

Lines changed: 53 additions & 130 deletions

File tree

AGENTS.md

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,64 @@
11
# Networking — repo notes
22

33
Async HTTP client for Apple platforms. **iOS 18+ / Swift 6** (`swift-tools 6.2`, language mode `[.v6]`).
4+
The iOS-shared and global conventions in the parent `AGENTS.md` files still apply; this layer adds only
5+
what's specific to this repo. Release history and the v8 (major) notes live in `CHANGELOG.md`.
46

57
## Build & test
68

7-
- `make test` — spins up go-httpbin in Docker, runs the full suite against it, tears it down. **Requires Docker.**
8-
- `make httpbin` / `make httpbin-stop` — run go-httpbin on `:8080` while you iterate with plain `swift test`.
9-
- Integration tests need a server speaking the httpbin protocol; `TestConfig.httpbinBaseURL` defaults to `http://127.0.0.1:8080` (CI sets the same). The offline / `fake*` suites need no server. Without go-httpbin running, integration tests fail fast (connection refused) — that's intended, not a flake.
10-
- CI: `macos-15` / Xcode 26.3 / Swift 6.2.x.
9+
- `make test` — spins up go-httpbin in Docker, runs the full suite, tears it down. **Requires Docker.**
10+
- `make httpbin` / `make httpbin-stop` — run go-httpbin on `:8080` to iterate with plain `swift test`.
11+
- `swift build` — library only; CI also fails on any first-party `warning:`.
12+
- Integration tests need go-httpbin (`TestConfig.httpbinBaseURL`, default `http://127.0.0.1:8080`). Without it
13+
they fail fast (connection refused) — intended, not a flake. The offline / `fake*` suites need no server.
14+
- CI: `macos-15` / Xcode 26.3 / Swift 6.2.x (stricter region-isolation than local 6.3 — CI is the gate).
1115

12-
## Architecture (current)
16+
## Source map
1317

14-
- **`Networking` is a `public actor`.** Safe to share one instance across tasks; isolated members are accessed with `await`. Config is via async setters — `setAuthorizationHeader`, `setHeaderFields`, `setInterceptors`, `setRedactedHeaderFields`, `setLogLevel`, `setLogFileURL`, `setCacheTTL` (actors forbid external property mutation). No subclassing.
15-
- **Verbs:** `get`/`post`/`put`/`patch`/`delete``Result<T, NetworkingError>`. Pick `T`: any `Decodable` model, `Data`, `Void`, or `JSONResponse` (`{ statusCode, headers, body }`) when you want metadata.
16-
- **Typed bodies — no `Any`.** The encoding is chosen by the method, not an untyped `parameters:`/`parameterType:` pair (`Networking+HTTPRequests.swift`): `post`/`put`/`patch` take `body:` (any `Encodable` → JSON, ISO-8601 dates), `form:` (any flat `Encodable` → url-encoded), `parts:`+`fields:` (multipart), or `data:contentType:` (raw); `get`/`delete` take `query:` (a `[URLQueryItem]` for ordering/dupes, or any flat `Encodable`). `form:`/`query:` flatten an `Encodable` to `[String: String]` via `formFields` in `Networking+FormEncoding.swift` (JSON bridge + scalar re-decode so `Bool`→"true", not NSNumber "1"). Bodies flow through the typed `RequestBody` enum in `Networking+New.swift`.
17-
- **Downloads:** `downloadImage`/`downloadData``Result<T, NetworkingError>` where `T` is the payload (`Image`/`Data`) or an envelope (`ImageResponse`/`DataResponse`).
18-
- **Cache lifecycle (`CacheStore.swift`, `CacheExpiry.swift`):** the whole two-tier cache subsystem lives in **`CacheStore`** — a non-actor `@unchecked Sendable` keyed by an already-resolved resource string (composing a key from baseURL + path stays the networking layer's job; `Networking.cacheResource(for:cacheName:)` does it and `destinationURL`/`objectFromCache`/`cacheOrPurgeData`/`cacheOrPurgeImage` are now thin nonisolated shims that delegate). The store is an in-memory `NSCache` (the **warm** tier, pressure-evicted by iOS, no exposed limits — assume it can be wiped at any time) over on-disk files; `Networking` holds the same `NSCache` by reference so it stays injectable/inspectable. **Reads are pure** (`CacheStore.object`): `.memory` serves the warm tier only and `.none` reports a miss — neither touches disk, so a read can't destroy a durable `.memoryAndFile` copy (purging is the write path's and `clearCache`'s job). Cleanup is symmetric: `clearCache()` → `cacheStore.clear()` empties **both** tiers (the old disk-only static `deleteCachedFiles()` is gone — it left memory serving deleted data); `reset()` = `clearCache()` + wipe credentials/headers/fakes. On-disk entries carry a **sliding TTL** (`cacheTTL`, default 7 days) whose clock is the **file's modification date** — persisted, no in-memory map, no manifest. The store drops a disk entry whose mtime is older than the TTL (`CacheExpiry.isExpired(fileDate:)`); a disk hit (memory miss) re-warms by bumping the mtime — and since `NSCache` absorbs repeat reads, that touch is ~once per entry per launch, so no explicit debounce is needed. (Memory hits deliberately don't re-stamp the disk file, so an entry kept warm *only* in memory past `cacheTTL` can read as cold once evicted — accepted; normal use cycles through disk hits that re-warm it.) **Sharded from the start:** `CacheStore.destinationURL(forResource:)` lays files out under `domain/<shard>/<file>` where the shard is one hex nibble of the key hash (`shardName`, `shardCount` = 16); a detached `CacheStore.sweepExpired` runs once per init and sweeps **one shard per launch** (rotated via a `.sweep-shard` cursor file), so per-launch work is O(N / shardCount) and a full pass takes `shardCount` launches (it also clears pre-sharding strays at the domain root). Sweep and `clear` serialize on `CacheStore.mutationLock` (a static, shared across instances) so the background sweep can't race a clear. `destinationURL` also hashes the filename component past the filesystem's 255-byte limit (readable prefix + SHA-256 suffix, `filesystemSafeComponent`).
19-
- **Fakes (testing):** `fakeGET`/`fakePOST`/… take `response:` over any `Encodable` (encoded to JSON), a no-body overload for status-only fakes, or `fileName:` for bundled raw `Data`; `FakeRequest` holds a typed `Payload` enum.
20-
- **Errors (`NetworkingError.swift`):** categorized by where the failure happened — `invalidRequest(InvalidRequestReason)`, `transport(URLError)`, `http(HTTPError)`, `decoding(DecodingError, ResponseMetadata)`, `validation(reason:ResponseMetadata)` (a 2xx rejected by a `ResponseValidatorInterceptor`), `invalidResponse`, `cancelled`. `HTTPError` carries `statusCode`/`isClientError`/`isServerError`/`metadata`; cross-cutting `statusCode`, `responseMetadata`, and conservative `isRetryable` (transient transport + 408/429/5xx). **The core makes no assumption about the error body's shape**`ResponseMetadata` retains the **full** `body: Data` (with `bodySnippet` a derived, truncated log excerpt and `decode(_:)` a convenience over `body`), so a caller decodes its own error envelope; there's no `serverMessage`/in-core parse. `ResponseMetadata(response:body:)` is the single source for metadata extraction. Construction lives in `Networking+New.swift` (`handleResponse`/`handleSuccessfulResponse`/`mapThrownError`).
21-
- **Observability (`NetworkingEvent.swift`):** no `print`, no closure observer. One consumer hook — `events()`, an `AsyncStream<NetworkingEvent>` (multi-consumer, `for await` accumulation without `Box`/`@unchecked`; continuations registry + `onTermination` cleanup on the actor). Every request emits `.started(RequestContext)` then `.completed(RequestContext, outcome:duration:metrics:)` — verbs via `handle`/`emitPreflightFailure`, downloads via `handleDataRequest`/`handleImageRequest` (all share `complete`/`makeContext` in `Networking+New.swift`; downloads inline the emit/complete rather than a closure, to satisfy Swift 6.2 region isolation). `RequestContext`: per-request id + the real request headers (raw — redaction is a log-path concern); `TransactionMetrics` distills `URLSessionTaskMetrics` (via `MetricsCollector`, a lock-synchronized per-task delegate).
22-
- **Interceptors / middleware (`HTTPInterceptor.swift`):** an async `HTTPInterceptor.intercept(_:next:)` seam (the swift-openapi `ClientMiddleware` onion shape, not Alamofire's adapt/retry split) wrapping every *verb* request. It sits **below** the generic `Result<T>` decode layer — operating on a raw `HTTPExchange` (`Data` + `HTTPURLResponse`), so one interceptor serves all response types. Registered outermost-first via `setInterceptors`; folded around the real `URLSession` call in `perform` (`Networking+New.swift`), which reads `session`/`collector` into locals first so the `@Sendable` chain captures no actor state (the Swift 6.2 region-isolation trap). Calling `next` again replays the request. A thrown `NetworkingError` from an interceptor passes through `mapThrownError` intact. Built-in interceptors: **`AuthRefreshInterceptor`** (replaced the old `unauthorizedRequestCallback`) — on 401/403 refreshes the credential and replays once, concurrent refreshes deduped through a `RefreshCoordinator` actor (single-flight, vs the token-refresh stampede); **`RetryInterceptor`** — retries transient transport failures + `retryableStatusCodes` (default `NetworkingError.retryableStatusCodes` = 408/429/5xx) with exponential backoff + full jitter, honoring `Retry-After` (seconds or HTTP-date), and **only idempotent methods** (`retryableMethods`, default GET/HEAD/PUT/DELETE/OPTIONS/TRACE) so a retry can't duplicate a POST/PATCH side effect — opt non-idempotent methods in explicitly. The retryability predicates (`retryableStatusCodes`, `isRetryableTransport`) live on `NetworkingError` as the single source of truth shared with `isRetryable`. **`ResponseValidatorInterceptor`** — runs a `(HTTPExchange) -> .valid/.invalid(reason:)` check on **2xx only** (non-2xx flows through to stay `.http`); a rejection throws `.validation(reason:metadata:)`. Register outermost (before retry) to validate the final response. **Downloads route through the chain too** — `requestData` (`Networking+Private.swift`) calls `perform`, so retry/auth apply to `downloadImage`/`downloadData` as well. **The cache is the innermost layer:** a GET cache hit is fed into `perform(…, cached:)` as the base result rather than bypassing it, so it flows back out through the chain (validators see it; retry/auth are no-ops on a success). Errors that were 401/403-only callback sites are gone from `handleResponse`/fake/download paths.
23-
- **Built-in logging** runs synchronously in the completion funnel (`complete` → `logCompletion`), gated by `logLevel` (`Networking.LogLevel`: `.none` / `.failures` default / `.all`, `setLogLevel`). `.failures` logs every failure; `.all` also logs successes; both with `✗`/`✓ METHOD url [id] …` line + request & response headers + request & response bodies (truncated). The request body and success response metadata aren't on the event, so they're threaded into `complete`; downloads pass no response metadata (binary payload) so they log line + request headers only. **Log redaction:** `redactsLogs` (`setRedactsLogs`) replaces both the request/response **body lines** *and* the `redactedHeaderFields` header values with `<redacted>` in the logs; defaults via `#if DEBUG` (shown in debug, redacted in release). Redaction lives in the log path (`logCompletion`), not `makeContext` — so `events()` `RequestContext.headers` carries the **real** values (your own request data). The one thing it doesn't cover: `events()` (raw by design). `record(_:level:)` writes to `os.Logger` and, when `logFileURL` is set (`setLogFileURL` or `NETWORKING_LOG_FILE`), mirrors to a text file — how a CLI/agent reads logs, since `os.Logger` isn't in stdout.
18+
- `Networking.swift` — the `public actor`: config (auth/headers/interceptors/log/`cacheTTL` via async
19+
setters), `events()`, URL composition, public cache entry points (`clearCache`/`reset`/`destinationURL`).
20+
- `Networking+HTTPRequests.swift` — the public surface: verb overloads (`get`/`post`/…), `downloadImage`/
21+
`downloadData`, and the `fake*` helpers.
22+
- `Networking+New.swift` — request execution: the `RequestBody` enum, `createRequest`, the interceptor fold
23+
(`perform`), response→`Result` mapping, the completion funnel (`complete`/`logCompletion`), `mapThrownError`.
24+
- `Networking+Private.swift` — download handlers + the cache shims that delegate to `CacheStore`.
25+
- `Networking+FormEncoding.swift` — flat-`Encodable``[String: String]` for forms/queries.
26+
- `CacheStore.swift` / `CacheExpiry.swift` — the two-tier disk cache (layout, sharding, sweep, TTL).
27+
- `HTTPInterceptor.swift` — the middleware seam + `AuthRefreshInterceptor` / `RetryInterceptor` /
28+
`ResponseValidatorInterceptor`.
29+
- `NetworkingError.swift` — the categorized error model + `ResponseMetadata`.
30+
- `NetworkingEvent.swift` — the `events()` types (`NetworkingEvent` / `RequestContext` / `Outcome` /
31+
`TransactionMetrics`).
32+
- `JSONResponse` · `DownloadResponse` · `FakeRequest` · `FormDataPart` · `Image` · `Helpers` — supporting types.
2433

25-
## Roadmap
34+
## Architecture invariants
2635

27-
`docs/roadmap.md`. The async/`Result`/Swift-6 modernization is done; #1 (typed request API — fully off `Any`), #2 (richer error model), #3 (structured observability — no `print`), and #4a/#4b/#4c/#4e (interceptor seam + auth-refresh + retry/backoff + response validators, with downloads routed through the chain) have landed. Next up within #4: timeout policy (#4d).
36+
Cross-file contracts an agent must hold; the *why* lives in each file's comments.
2837

29-
## Conventions
38+
- **Actor isolation.** `Networking` is an actor — isolated members need `await`, config is async setters
39+
(no external property mutation), no subclassing.
40+
- **Typed bodies, no `Any`.** The method picks the encoding (`body:` JSON / `form:` url-encoded /
41+
`parts:`+`fields:` multipart / `data:contentType:` raw; `query:` for get/delete), via the `RequestBody`
42+
enum. `T` ∈ any `Decodable` · `Data` · `Void` · `JSONResponse`.
43+
- **Errors say *where* it failed**, never a stringified catch-all; the core never parses the error body —
44+
`ResponseMetadata.body` is the full bytes, the caller decodes its own envelope.
45+
- **Interceptors are an onion *below* the decode layer**, over a raw `HTTPExchange`; registered
46+
outermost-first, `next` replays. Downloads route through too; a GET cache hit flows back out through the
47+
chain (the cache is the innermost layer).
48+
- **Cache reads are pure.** `.memory`/`.none` never touch disk, so a read can't destroy a durable
49+
`.memoryAndFile` copy — purging belongs to the write path and `clearCache`. Sliding TTL keyed on file
50+
mtime; sharded layout; one-shard-per-launch background sweep. The `NSCache` warm tier can be evicted by
51+
iOS at any time — disk is the durable fallback.
52+
- **Observability is one hook:** `events()` (multi-consumer `AsyncStream`). Built-in logging is separate,
53+
synchronous, gated by `logLevel`; redaction is log-path only, so `events()` carries the real headers.
54+
- **Region-isolation trap (Swift 6.2):** before building the `@Sendable` interceptor chain, read actor
55+
state (`session`/`collector`) into locals so the closure captures none.
3056

31-
- **Public-API change → update the README in the same PR.** Its snippets are copy-paste docs and must compile under the actor (isolated members need `await`).
32-
- These were breaking changes vs the last release (7.0.0); the next tag is a major bump (roadmap #8).
57+
## Boundaries
58+
59+
- **Always** run `make test` (Docker) to green and keep the build warning-free before claiming done.
60+
- **Always** update `README.md` and `CHANGELOG.md` in the same PR as any public-API change — the README
61+
snippets are copy-paste docs and must compile under the actor (`await`).
62+
- **Ask first** before adding a third-party dependency — the core is intentionally dependency-free.
63+
- **Never** put cache purging back on the read path: `.memory`/`.none` reads must stay pure, or an
64+
evicted warm tier turns a recoverable miss into permanent loss.

LICENSE.md

Lines changed: 0 additions & 22 deletions
This file was deleted.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,7 @@ This library was made with love by [@3lvis](https://twitter.com/3lvis).
665665

666666
## License
667667

668-
**Networking** is available under the MIT license. See the [LICENSE file](https://github.com/3lvis/Networking/blob/master/LICENSE.md) for more info.
668+
**Networking** is available under the MIT license. See the [LICENSE file](https://github.com/3lvis/Networking/blob/master/LICENSE) for more info.
669669

670670

671671
## Attribution

Sources/Networking/Helpers.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import Foundation
22

33
extension Duration {
4-
// Whole + fractional seconds as a Double, for TimeInterval / age arithmetic.
54
var seconds: Double {
65
let (wholeSeconds, attoseconds) = components
76
return Double(wholeSeconds) + Double(attoseconds) / 1e18

Tests/NetworkingTests/Resources/invalid.json

Lines changed: 0 additions & 1 deletion
This file was deleted.

Tests/NetworkingTests/Resources/simple_array.json

Lines changed: 0 additions & 6 deletions
This file was deleted.

Tests/NetworkingTests/Resources/simple_dictionary.json

Lines changed: 0 additions & 4 deletions
This file was deleted.

0 commit comments

Comments
 (0)