refactor(ios): host the upload route and the REST relay on an editor local server - #619
Draft
dcalhoun wants to merge 7 commits into
Draft
Conversation
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
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/619")Built from 963882b |
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.
What?
Replaces
MediaUploadServerwith anEditorLocalServerthat takes registered routes. The upload handler becomes aMediaUploadRoute, andRestRelayconforms to the sameLocalServerRouteprotocol 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/proxyroute, the server name and limits, and every test assertion are unchanged.Why?
#611 bolted the REST relay onto a media-specific server: an optional
restRelayconstructor parameter, a path-prefix check ahead of the upload route, two booleans on the view controller mirroring what it passed in, and ternaries inbuildEditorConfigurationreading 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-reviewrun on #611.How?
LocalServerRoute:handles(_:)andhandle(_:). 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 awaitablecleanupTask.EditorLocalServer.start(routes:maxRequestBodySize:): owns the listener configuration that was already relay-aware, hands each request to the first route that claims it, and answershosts(_:). Themedia-uploadname is kept so an upgrade does not strand the temp directory.EditorViewControllerbuilds 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.Sources/LocalServer/andTests/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
make test-swift-library— 1013 tests, 61 suites.Accessibility Testing Instructions
No UI change.
Screenshots or screencast
None.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PfwJZw19TxukV3iLkbVECB