Split the media delegate into MediaProcessor and MediaUploader - #621
Draft
jkmassel wants to merge 1 commit into
Draft
Split the media delegate into MediaProcessor and MediaUploader#621jkmassel wants to merge 1 commit into
MediaProcessor and MediaUploader#621jkmassel wants to merge 1 commit into
Conversation
XCFramework BuildThis PR's XCFramework is available for testing. Add the following to your .package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/621")Built from 1247d09 |
jkmassel
force-pushed
the
refactor/media-processor-uploader
branch
4 times, most recently
from
September 3, 2026 21:50
9fe481e to
9f00177
Compare
Make performing a media upload -- and retrying it -- a single, all-or-nothing responsibility: either GutenbergKit performs the upload and owns its retries, or the host does (say, to run it through its own networking so it can log every request). Both go to the same configured site; the only difference is who executes the requests. There is no in-between where the host performs the upload but GutenbergKit retries it. When an upload fatals in server-side post-processing it has to be retried: core retries POST .../post-process up to 5x, and cleans up the orphan if that fails. The old `MediaUploadDelegate.uploadFile` let a host perform the upload itself by returning the raw response it received -- which split one upload's HTTP across two owners: the host performed the POST /wp/v2/media, then core, reading that raw response, drove the post-process retries (and the orphan cleanup) behind it. A host that took over uploads to run them through its own stack still didn't own the retries; those went out through the browser, not the host. Delivery and its retries were owned by different parties. Make the upload and its retries one unit with one owner: - `MediaProcessor` (handlesFile, processFile) only transforms the file. It never performs the upload, so GutenbergKit performs it and owns the retries -- the extension point almost every host wants. - `MediaUploader` (upload) performs the upload on the host's own stack (its networking, logging, retry policy, a background session) and owns the whole lifecycle. `upload` returns the finished attachment or throws: there's no raw response for core to retry behind it, so the host drives its own post-process recovery and force-deletes its own orphan on terminal failure. All-or-nothing: the host performs the upload and its retries, or GutenbergKit does -- never a split. An uploader and GutenbergKit's built-in default both target the same configured site; the choice is only who executes the requests. Media deletes always relay to the default uploader (the configured site): every attachment lives there, even one a host uploader delivered, so there is no per-host delete path. The relay is left unscoped -- core issues its cleanup DELETE there, but the relay can't tell it from any other DELETE the WebView sends, so a client-side-compromised editor holding the loopback token could force-delete media on the site. Accepted -- such a script already has broad write access, and a server-side compromise deletes media directly without the editor. An earlier revision carried a per-session ledger to scope the relay; dropped as not worth the cost for a client-side-only threat. `EditorViewController`/`GutenbergView` expose `mediaProcessor` + `mediaUploader` in place of `mediaUploadDelegate`; the server starts if either is set and builds a default uploader whenever site credentials are present (it delivers GutenbergKit's own uploads and relays every media delete). `MediaUploadResponse` drops to internal -- it is no longer on any public API. Both demos and all tests move to the new protocols. Breaking change: hosts must migrate `mediaUploadDelegate` (WordPress-iOS/Android, Jetpack). iOS and Android suites green; SwiftLint and Detekt clean.
jkmassel
force-pushed
the
refactor/media-processor-uploader
branch
from
September 3, 2026 22:21
9f00177 to
1247d09
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #594. Makes performing a media upload — and retrying it — a single, all-or-nothing responsibility: either GutenbergKit performs the upload and owns its retries, or the host does (say, to run it through its own networking so it can log every request). Both go to the same configured site; the only difference is who executes the requests. There's no in-between where the host performs the upload but GutenbergKit retries it.
Summary
Two protocols replace the single
MediaUploadDelegate:MediaProcessor— transform the file before upload; GutenbergKit performs the upload and owns its retries. The common, safe extension point.MediaUploader— perform the upload yourself (your own networking, logging, retry policy, a background session) and own its whole lifecycle: retries, recovery, and cleanup.Breaking: hosts migrate
mediaUploadDelegate→mediaProcessor/mediaUploader.The problem with the old design
When an upload fatals in server-side post-processing, it has to be retried: core retries
POST …/post-processup to 5×, and cleans up the orphan if that fails.MediaUploadDelegate.uploadFilelet a host perform the upload itself by returning the raw response it received. But that split one upload's HTTP across two owners: the host performed thePOST /wp/v2/media, then core — reading that raw response — drove thepost-processretries (and the orphan cleanup) behind it. A host that took over uploads to run them through its own stack still didn't own the retries; those went out through the browser, not the host. Delivery and its retries were owned by different parties.The fix
Make the upload and its retries one unit with one owner:
MediaProcessor(handlesFile,processFile) only transforms the file. It never performs the upload, so GutenbergKit performs it and owns the retries. The extension point almost every host wants.MediaUploader(upload) performs the upload on the host's own stack and owns the whole lifecycle.uploadreturns the finished attachment or throws — there's no raw response for core to retry behind it, so the host drives its ownpost-processrecovery and force-deletes its own orphan on terminal failure.So it's all-or-nothing: the host performs the upload and its retries, or GutenbergKit does — never a split. An uploader and GutenbergKit's built-in default both target the same configured site; the choice is only who executes the requests.
Media deletes always relay to the default uploader (the configured site) — every attachment lives there, even one a host uploader delivered, so there's no per-host delete path.
EditorViewController/GutenbergViewexposemediaProcessor+mediaUploader; the server starts if either is set and builds a default uploader whenever site credentials are present (it delivers GutenbergKit's own uploads and relays every delete).MediaUploadResponseis nowinternal— it's no longer on any public API.Accepted Risk / Out of Scope
DELETEto the configured site, but the relay can't distinguish it from any otherDELETEthe WebView sends — a client-side-compromised editor (a supply-chain-tampered JS bundle, or editor XSS) holding the loopback token could force-delete arbitrary media there. We accept this: such a script already has broad write access via allowed methods, and a server-side compromise (a malicious plugin) deletes media directly without the editor. An earlier revision carried a per-session ledger to scope the relay; it's dropped as not worth the cost for a client-side-only threat.Test Plan
MediaUploadServer,GutenbergViewxcodebuild, Xcode 26.4.1); Android demo builds (Detekt compiles it)mediaUploadDelegatein WordPress-iOS / WordPress-Android / JetpackRelated