fix: trap when a mediaUploader is set without site credentials - #632
Draft
jkmassel wants to merge 1 commit into
Draft
fix: trap when a mediaUploader is set without site credentials#632jkmassel wants to merge 1 commit into
jkmassel wants to merge 1 commit into
Conversation
Setting a `mediaUploader` means the host is taking over uploads. With no site credentials the server would previously just not start, silently dropping the uploader — and its media deletes still need the internal media client to reach the configured site, since every attachment lives there no matter who delivered it. Starting anyway would give a server whose every delete 500s. So the behavior forks by intent. A `mediaProcessor` with no credentials leaves the server down and uploads fall to the default WebView path — there is nothing to deliver through, so nothing to process. A `mediaUploader` with no credentials is a configuration error and fails fast: `precondition` on iOS, `check` on Android. The iOS policy lives in `MediaServerCredentials` rather than `EditorViewController`, which is `#if canImport(UIKit)` and therefore absent from the macOS host — the one platform that can run Swift Testing's exit tests. Living outside the gate, the trap itself is testable, not just the predicate: two exit tests run it in a child process, and neutering the precondition fails both. Android's `check` is covered through `GutenbergView`, and neutering it fails those two as well.
4 tasks
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/632")Built from 9ee12ea |
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 #631. Ninth of ten PRs splitting #621.
What?
Setting a
mediaUploaderwithout site credentials is a configuration error, and now fails fast instead of silently doing nothing.Why?
Setting a
mediaUploadermeans the host is taking over uploads. With no site credentials the server would previously just not start, silently dropping the uploader — and its media deletes still need the internal media client to reach the configured site, since every attachment lives there no matter who delivered it. Starting anyway would give a server whose every delete 500s.How?
The behavior forks by intent:
mediaProcessorwith no credentials leaves the server down and uploads fall to the default WebView path — there is nothing to deliver through, so nothing to process.mediaUploaderwith no credentials traps:preconditionon iOS,checkon Android.The iOS policy lives in
MediaServerCredentials(added in #624) rather thanEditorViewController, which is#if canImport(UIKit)and therefore absent from the macOS host — the one platform that can run Swift Testing's exit tests. Living outside the gate, the trap itself is testable, not just the predicate.Testing Instructions
preconditionfails bothGutenbergView— the two uploader arms and the processor arm; neutering thecheckfails the two uploader testsswift test— host suite green:Gutenberg:testDebugUnitTestgreenxcodebuild