docs(spec): resolve open questions — fold in upload budget check, keep fetcher namespace
refs #511
This commit is contained in:
@@ -123,10 +123,15 @@ Therefore the save path must validate before caching:
|
||||
the save path both calling it. The retention budget (`EnsureScaledFramesAffordable`) stays in
|
||||
`ImageElementBase` — it depends on render-time scale and has no meaning at save time.
|
||||
|
||||
**Pre-existing hole, flagged for a decision:** direct **uploads** (`UploadArtworkHandler`) are not
|
||||
budget-checked either. Once URL logos become uploads, they inherit that gap on any subsequent
|
||||
re-upload. Applying the same validation to `UploadArtworkHandler` is a small addition using the same
|
||||
component and is *recommended*, but it is a scope increase and is called out rather than assumed.
|
||||
**Uploads are budget-checked too (decided: fold in).** Direct **uploads** (`UploadArtworkHandler`)
|
||||
are not budget-checked today. Once URL logos become uploads, they inherit that gap on any subsequent
|
||||
re-upload — an inconsistency this change would *create* (same bytes, same cache, enforcement
|
||||
depending only on arrival path). The `RemoteImageDecodeBudget` component is being built regardless, so
|
||||
`UploadArtworkHandler` calls it too. One consistent rule: **anything entering the logo cache is
|
||||
budget-checked, however it arrived.** A budget failure returns a `400` from the upload endpoint the
|
||||
same way it does from the channel save. (Risk is admin-only, like #511's SSRF stance, but the failure
|
||||
mode — cache succeeds, render OOMs concurrent streams later — is exactly the fail-late pattern this
|
||||
redesign exists to kill, so it is closed here rather than deferred.)
|
||||
|
||||
### Migration of existing rows
|
||||
|
||||
@@ -204,12 +209,12 @@ selected again.
|
||||
- `ArtworkController.RedirectArtwork` (`ArtworkController.cs:37-57`) builds `"/iptv/logos/" + Path`
|
||||
unconditionally, producing a malformed redirect when `Path` is a URL. Pre-existing, unrelated to
|
||||
this change, and largely mooted by it for logos — **file separately**.
|
||||
- Applying decode budgets to `UploadArtworkHandler` (see *Decode validation*).
|
||||
- #1 (generated-initials `localhost`) and #510 (deco path) remain untouched.
|
||||
|
||||
## Open questions
|
||||
## Resolved decisions
|
||||
|
||||
1. Should `UploadArtworkHandler` get the same decode validation in this PR, or a follow-up?
|
||||
2. Should `IRemoteImageFetcher` move from `Core/Interfaces/Streaming/` to `Core/Interfaces/Images/`
|
||||
now that its primary consumer is the save path rather than the streaming path? Cosmetic; a
|
||||
namespace move only.
|
||||
1. **`UploadArtworkHandler` decode validation is folded into this PR**, not deferred — see *Decode
|
||||
validation*. The component exists either way and the inconsistency is created by this change.
|
||||
2. **`IRemoteImageFetcher` stays in `Core/Interfaces/Streaming/`.** It is still used by the streaming
|
||||
path (YAML image elements), and a namespace move is churn against `git blame` for weak
|
||||
naming-accuracy benefit. Trivial standalone rename if ever wanted.
|
||||
|
||||
Reference in New Issue
Block a user