Skip to content

refactor(ios): host the upload route and the REST relay on an editor local server - #619

Draft
dcalhoun wants to merge 7 commits into
task/stabilize-rest-request-relayfrom
refactor/editor-local-server-routes
Draft

refactor(ios): host the upload route and the REST relay on an editor local server#619
dcalhoun wants to merge 7 commits into
task/stabilize-rest-request-relayfrom
refactor/editor-local-server-routes

Conversation

@dcalhoun

@dcalhoun dcalhoun commented Sep 3, 2026

Copy link
Copy Markdown
Member

What?

Replaces MediaUploadServer with an EditorLocalServer that takes registered routes. The upload handler becomes a MediaUploadRoute, and RestRelay conforms to the same LocalServerRoute protocol instead of occupying an optional slot on the upload server.

Stacked on #611. A mechanical refactor with no behavior change: the JS contract (nativeUploadPort, nativeUploadToken, networkProxy), the relay's /proxy route, the server name and limits, and every test assertion are unchanged.

Why?

#611 bolted the REST relay onto a media-specific server: an optional restRelay constructor parameter, a path-prefix check ahead of the upload route, two booleans on the view controller mirroring what it passed in, and ternaries in buildEditorConfiguration reading them. Each further route (asset cache, oEmbed proxy, the any-origin transport) would add one of each. Once the relay accepts any origin it is the editor's general transport and the upload route is one client of it, so a media server hosting the transport is inverted. Raised by the /code-review run on #611.

How?

  • LocalServerRoute: handles(_:) and handle(_:). The relay conforms with a one-line forward to its existing static predicate.
  • MediaUploadRoute: the upload handler and everything it calls, owning the delegate (weakly, as before) and the default uploader, plus the orphan sweep and its awaitable cleanupTask.
  • EditorLocalServer.start(routes:maxRequestBodySize:): owns the listener configuration that was already relay-aware, hands each request to the first route that claims it, and answers hosts(_:). The media-upload name is kept so an upgrade does not strand the temp directory.
  • EditorViewController builds the route list from the same two conditions it already evaluated and advertises to JavaScript only what the server hosts. The two booleans and their reset on a failed start are gone.
  • Files move under Sources/LocalServer/ and Tests/LocalServer/; the combined server test file splits by type.

Each commit leaves the suite green. Deferred: the server's 404 and parse-error refusals still carry the code upload_error, and the 413 message is upload-worded. Changing those is behavior, so it belongs with the any-origin work.

Testing Instructions

  1. make test-swift-library — 1013 tests, 61 suites.
  2. In the demo app, with an upload delegate set, upload an image: it goes through the native route as before.
  3. With Lockdown Mode on (or the debug override), load a post and save it: REST traffic is relayed as on fix(ios): route editor REST requests through a native relay under Lockdown Mode #611.

Accessibility Testing Instructions

No UI change.

Screenshots or screencast

None.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB

dcalhoun and others added 7 commits September 3, 2026 11:36
The relay already has the two members a route needs: a predicate for
whether a request is its own and an async handler for it. Naming that
shape as a protocol is the first step toward a local server that takes
registered routes instead of an optional relay slot; the upload route
conforms next.

The static `handles(_:)` stays for the callers that hold no relay, and
the instance requirement forwards to it. No caller changes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
Moved verbatim out of MediaUploadServer.swift ahead of extracting the
upload route from the server, so that diff shows only the extraction.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
The upload handler and everything it calls move into MediaUploadRoute,
a LocalServerRoute that owns the upload delegate and default uploader.
The route holds the delegate weakly, as the upload context did, for the
same retain-cycle reason.

The server becomes a shell: it builds the route list — relay first, so
its prefix is checked ahead of the upload route's exact path — and its
handler hands each request to the first route that claims it. The
constructor, the server-level refusals, and every test are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
The server takes the routes it hosts instead of an optional delegate,
uploader, and relay, and answers `hosts(_:)` so a caller can ask what is
registered rather than remembering what it passed in. Nothing about
the listener changes: the same name, limits, CORS policy, and refusals.

The upload route's startup sweep is the route's, so `cleanupTask` moves
there; the one test that awaits it builds the route directly. The view
controller builds the route list from the same two conditions it already
evaluates; deriving the advertised fields from `hosts(_:)` is next.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
`buildEditorConfiguration` asks the server what it hosts instead of
reading two view-controller flags that mirrored what was passed to the
constructor, so the flags and their reset on a failed start go away. A
server that failed to start is nil, and nothing is advertised.

The route conditions stay where they were, now each guarding the route
it enables. The property and method lose the "upload" in their names,
since the server hosts more than uploads, and the start failure logs
under its own category.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
Neither is a media concern: the relay is a route on the editor's local
server, and the WordPress-shaped error body serves every route and the
server's own refusals.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
MediaUploadServerTests.swift held the server's admission tests, the
upload route's tests, and the uploader's, plus the helpers they shared.
Each now sits with its type: the server tests move under LocalServer
with the relay's, the route and uploader tests stay under Media, and
the bind probe, multipart builder, and browser-origin helper they share
become a support file. Assertions are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB
@dcalhoun dcalhoun added the [Type] Task Issues or PRs that have been broken down into an individual action to take label Sep 3, 2026
@wpmobilebot

Copy link
Copy Markdown

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/619")

Built from 963882b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Task Issues or PRs that have been broken down into an individual action to take

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants