Refactor: split internal/imgcache into focused packages #39

New Issue

clawbot · 2026-03-18T04:34:17+01:00

clawbot commented

2026-03-18 04:34:17 +01:00

Per PR #37 discussion, internal/imgcache/ currently bundles many distinct concerns into a single package. This issue tracks splitting it into focused, independently testable packages.

Already Done

imageprocessor → internal/imageprocessor/ — Image format conversion and resizing (extracted in PR #37)

Easily Separable

These have minimal coupling to the core cache/service layer and can be extracted with straightforward interface boundaries:

fetcher (fetcher.go) → internal/httpfetcher/
- HTTPFetcher, FetcherConfig, DefaultFetcherConfig, SSRF-safe dialer, rate limiting, content-type validation
- Only coupled via the Fetcher interface and FetchResult type in imgcache.go
- mock_fetcher.go can move with it or stay as a test helper
signature (signature.go) → internal/signature/
- Signer, NewSigner, Sign, Verify, GenerateSignedURL, ParseSignatureParams
- Only uses ImageRequest for building the HMAC payload
magic (magic.go) → internal/magic/
- MIMEType, DetectFormat, ValidateMagicBytes, PeekAndValidate, IsSupportedMIMEType, MIMEToImageFormat, ImageFormatToMIME
- Only uses ImageFormat type constants
whitelist (whitelist.go) → internal/whitelist/
- HostWhitelist, NewHostWhitelist, IsWhitelisted, IsEmpty, Count
- Completely self-contained — only depends on net/url and strings
urlparser (urlparser.go) → internal/urlparser/
- ParsedURL, ParseImagePath, ParseImageURL, path traversal validation, size/format parsing
- Uses ImageRequest, Size, ImageFormat, FitMode — would need to import shared types

Tightly Coupled (keep together)

These share significant state and data flow with the service orchestrator. Splitting them further would require significant interface surgery and may not be worth it:

cache (cache.go) — Cache lookup, variant storage, negative cache, stats. Tightly coupled with service.go and storage.go.
storage (storage.go) — Content-addressed ContentStorage and MetadataStorage. Created and managed by Cache.
service (service.go) — The core orchestrator that wires cache, fetcher, and processor together.
types/interfaces (imgcache.go) — ImageRequest, ImageResponse, ImageFormat, FitMode, Size, etc. These are the shared vocabulary used across all subpackages.

Approach

Each extraction should:

Create the new package under internal/
Move types and functions, defining standalone types where needed to avoid circular imports
Update all import sites
Ensure docker build . passes (fmt, lint, test, build)
One PR per package extraction to keep reviews manageable

Per [PR #37 discussion](https://git.eeqj.de/sneak/pixa/pulls/37#issuecomment-1173), `internal/imgcache/` currently bundles many distinct concerns into a single package. This issue tracks splitting it into focused, independently testable packages. ## Already Done - [x] **imageprocessor** → `internal/imageprocessor/` — Image format conversion and resizing (extracted in [PR #37](https://git.eeqj.de/sneak/pixa/pulls/37)) ## Easily Separable These have minimal coupling to the core cache/service layer and can be extracted with straightforward interface boundaries: - [ ] **fetcher** (`fetcher.go`) → `internal/httpfetcher/` - `HTTPFetcher`, `FetcherConfig`, `DefaultFetcherConfig`, SSRF-safe dialer, rate limiting, content-type validation - Only coupled via the `Fetcher` interface and `FetchResult` type in `imgcache.go` - `mock_fetcher.go` can move with it or stay as a test helper - [ ] **signature** (`signature.go`) → `internal/signature/` - `Signer`, `NewSigner`, `Sign`, `Verify`, `GenerateSignedURL`, `ParseSignatureParams` - Only uses `ImageRequest` for building the HMAC payload - [ ] **magic** (`magic.go`) → `internal/magic/` - `MIMEType`, `DetectFormat`, `ValidateMagicBytes`, `PeekAndValidate`, `IsSupportedMIMEType`, `MIMEToImageFormat`, `ImageFormatToMIME` - Only uses `ImageFormat` type constants - [ ] **whitelist** (`whitelist.go`) → `internal/whitelist/` - `HostWhitelist`, `NewHostWhitelist`, `IsWhitelisted`, `IsEmpty`, `Count` - Completely self-contained — only depends on `net/url` and `strings` - [ ] **urlparser** (`urlparser.go`) → `internal/urlparser/` - `ParsedURL`, `ParseImagePath`, `ParseImageURL`, path traversal validation, size/format parsing - Uses `ImageRequest`, `Size`, `ImageFormat`, `FitMode` — would need to import shared types ## Tightly Coupled (keep together) These share significant state and data flow with the service orchestrator. Splitting them further would require significant interface surgery and may not be worth it: - **cache** (`cache.go`) — Cache lookup, variant storage, negative cache, stats. Tightly coupled with `service.go` and `storage.go`. - **storage** (`storage.go`) — Content-addressed `ContentStorage` and `MetadataStorage`. Created and managed by `Cache`. - **service** (`service.go`) — The core orchestrator that wires cache, fetcher, and processor together. - **types/interfaces** (`imgcache.go`) — `ImageRequest`, `ImageResponse`, `ImageFormat`, `FitMode`, `Size`, etc. These are the shared vocabulary used across all subpackages. ## Approach Each extraction should: 1. Create the new package under `internal/` 2. Move types and functions, defining standalone types where needed to avoid circular imports 3. Update all import sites 4. Ensure `docker build .` passes (fmt, lint, test, build) 5. One PR per package extraction to keep reviews manageable

clawbot referenced this issue

2026-03-18 04:34:34 +01:00

Bound imageprocessor.Process input read to prevent unbounded memory use #37

clawbot referenced this issue

2026-03-18 04:40:46 +01:00

Bound imageprocessor.Process input read to prevent unbounded memory use #37

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/pixa#39