From 0ab9ee52fdc9fab82c23cdb38e3b560a71dc8e78 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:33:03 -0600 Subject: [PATCH] feat: add MediaUploader, for a host that owns the whole upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Performing a media upload — and retrying it — should be a single, all-or-nothing responsibility: either GutenbergKit performs the upload and owns its retries, or the host does. Both go to the same configured site; the only difference is who executes the requests. `MediaUploadDelegate.uploadFile` doesn't offer that. A host performs the `POST /wp/v2/media` and returns the raw response it received — then the editor, reading that response, drives the `post-process` retries and the orphan cleanup behind it, through the WebView rather than the host's stack. A host that took over uploads to run them through its own networking still didn't own the retries. It also receives no form fields, so an attachment it uploads lands unattached to its post. Add `MediaUploader`, which owns the upload end to end: - `upload(_:)` returns the finished attachment or throws. There is no raw response left for the editor to retry behind it, so the host drives its own post-process recovery and force-deletes its own orphan on terminal failure. - It receives a `MediaUpload` carrying the file, its metadata, the editor's non-file form fields (`post`, additionalData) and the request query (`?_embed`) — everything needed to reproduce a native request. - Fields are a `MediaUploadField` list rather than a dictionary, so repeated names (a `field[]` array) survive verbatim and in order. Purely additive. `uploadFile` still works and is marked deprecated, pointing hosts at the replacement; an uploader takes precedence when both are set. GutenbergKit's own build keeps one deprecation warning at the call site that supports the old hook — the marker exists to tell hosts to migrate, and supporting the hook until it is removed means calling it. With an uploader set, the delegate's metadata gate can no longer decline a file: the gate exists to skip a temp copy for a file the delegate won't touch, but an uploader takes over delivery for *every* file, so passing through would silently bypass it. Covered on both platforms. `MediaUploadServerTest` crosses Detekt's LargeClass threshold; baselined rather than split, which is its own change. --- android/Gutenberg/detekt-baseline.xml | 1 + .../org/wordpress/gutenberg/GutenbergView.kt | 37 +++- .../wordpress/gutenberg/MediaUploadServer.kt | 123 ++++++++++++- .../gutenberg/MediaUploadServerTest.kt | 120 ++++++++++++ .../Sources/EditorViewController.swift | 58 +++--- .../Sources/Media/MediaUploadDelegate.swift | 100 +++++++++- .../Sources/Media/MediaUploadServer.swift | 63 ++++++- .../Media/MediaUploadServerTests.swift | 171 ++++++++++++++++++ 8 files changed, 629 insertions(+), 44 deletions(-) diff --git a/android/Gutenberg/detekt-baseline.xml b/android/Gutenberg/detekt-baseline.xml index 4f6c96915..ce3b4481a 100644 --- a/android/Gutenberg/detekt-baseline.xml +++ b/android/Gutenberg/detekt-baseline.xml @@ -11,6 +11,7 @@ ExplicitItLambdaParameter:EditorAssetsLibrary.kt$EditorAssetsLibrary${ str, it -> str + "%02x".format(it) } FunctionNaming:EditorURLCache.kt$EditorURLCache$private fun __store( response: EditorURLResponse, url: String, httpMethod: EditorHttpMethod, currentDate: Date ) LargeClass:GutenbergView.kt$GutenbergView : FrameLayout + LargeClass:MediaUploadServerTest.kt$MediaUploadServerTest LongMethod:FixtureTests.kt$FixtureTests$@Test fun `request parsing - all basic cases pass`() LongMethod:FixtureTests.kt$FixtureTests$@Test fun `request parsing - all incremental cases pass`() LongMethod:HTTPRequestParser.kt$HTTPRequestParser$fun append(data: ByteArray): Unit diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index d32c4547a..eadb760ff 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -123,14 +123,32 @@ class GutenbergView : FrameLayout { */ var mediaUploadDelegate: MediaUploadDelegate? = null set(value) { - check(!hasStartedLoading) { - "mediaUploadDelegate must be set before the editor loads (e.g. right " + - "after construction). It is captured when the page begins loading; " + - "setting it afterward has no effect." - } + check(!hasStartedLoading) { lateMediaAssignmentMessage("mediaUploadDelegate") } + field = value + } + + /** + * Takes over media upload on the host's own stack (background service, offline + * queue, resumable transport). Setting it makes the host own every upload and its + * whole lifecycle; GutenbergKit stays out of the network entirely for media. + * + * Same lifecycle rules as [mediaUploadDelegate]: set it before the editor loads, + * and this view owns it for its lifetime — so you needn't retain it yourself, just + * don't strongly retain this [GutenbergView] from your uploader. + * + * Takes precedence over the deprecated [MediaUploadDelegate.uploadFile]: with an + * uploader set, that hook is never called. + */ + var mediaUploader: MediaUploader? = null + set(value) { + check(!hasStartedLoading) { lateMediaAssignmentMessage("mediaUploader") } field = value } + private fun lateMediaAssignmentMessage(name: String) = + "$name must be set before the editor loads (e.g. right after construction). " + + "It is captured when the page begins loading; setting it afterward has no effect." + @Volatile private var uploadServer: MediaUploadServer? = null /** @@ -671,10 +689,10 @@ class GutenbergView : FrameLayout { } private fun startUploadServer() { - // No delegate means nothing wants to customize uploads, so there's no reason - // to route them through the native server — leave it down and let uploads - // fall to the default WebView path. (Matches iOS.) - if (mediaUploadDelegate == null) return + // Nothing to route through the native server unless the host provided a + // delegate or an uploader — leave it down and let uploads fall to the default + // WebView path. (Matches iOS.) + if (mediaUploadDelegate == null && mediaUploader == null) return // The native upload server relays through InternalMediaClient, which needs a // site root and an auth header (every host provides one — the editor injects @@ -710,6 +728,7 @@ class GutenbergView : FrameLayout { uploadServer = MediaUploadServer( uploadDelegate = mediaUploadDelegate, internalClient = internalClient, + uploader = mediaUploader, cacheDir = context.cacheDir, scope = coroutineScope ) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt index 0e51067ee..095e426b9 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -106,13 +106,97 @@ interface MediaUploadDelegate { * Upload a processed file to the remote WordPress site. * * Return the raw WordPress response (status code + body), which GutenbergKit - * relays to the editor unchanged, or null to use the default uploader. A host - * that uploads to WordPress should return the exact response it received so + * relays to the editor unchanged, or null to use the internal media client. A + * host that uploads to WordPress should return the exact response it received so * the editor sees a complete attachment object. + * + * Returning a raw response splits one upload's HTTP across two owners: you + * perform the POST, but the editor drives the `post-process` retries and orphan + * cleanup behind it, through the WebView rather than your stack. It also receives + * no form fields, so an attachment uploaded this way lands unattached to its post. + * Implement [MediaUploader] instead — it owns the upload end-to-end and receives a + * [MediaUpload] carrying the fields. */ + @Deprecated( + "Implement MediaUploader instead — it owns the upload's retries and receives the editor's form fields.", + ReplaceWith("MediaUploader") + ) suspend fun uploadFile(file: File, mimeType: String, filename: String): MediaUploadResponse? = null } +/** + * One of the editor's non-file form fields, as sent with a media upload. + * + * A named type rather than a pair so the field's meaning is legible at every call + * site, and so the type can gain members without a source break for every host. + * + * @property name The field name, e.g. `post`. Not unique — a `field[]` array repeats it. + * @property value The field's value, decoded as UTF-8. + */ +data class MediaUploadField(val name: String, val value: String) + +/** + * Everything a [MediaUploader] needs to reproduce a native upload: the file to send, + * its metadata, the editor's non-file form fields, and the request's query. + * + * @property file The file to upload — already processed, if a [MediaUploadDelegate] ran. + * @property mimeType The file's MIME type. + * @property filename The file's name. + * @property fields The editor's non-file form fields, in order, each decoded as UTF-8 — + * most importantly `post`, the parent post's ID, without which the attachment is + * created unattached. A list, not a map, so repeated field names (e.g. a `field[]` + * array) survive verbatim. Send each as a form part on your `POST /wp/v2/media`, in + * the given order. + * @property query The request's query string (leading `?`, e.g. `?_embed=wp:featuredmedia`), + * or empty. Carry it on your request so the editor gets the response it expects. + */ +data class MediaUpload( + val file: File, + val mimeType: String, + val filename: String, + val fields: List, + val query: String +) + +/** + * Takes over *performing* a media upload — on the host's own stack: its own + * networking (say, to log every request), a background service, an offline queue, a + * resumable transport, its own retry policy. + * + * This is a choice of *who executes the requests*, not where they go: an uploader and + * GutenbergKit's internal media client both target the same configured site. Setting + * [GutenbergView.mediaUploader] makes the host own that upload end-to-end — the + * request, its own retries, and its recovery and cleanup — with GutenbergKit out of + * the network entirely. Because the host does the retries itself, there's no raw + * response left for the editor to retry behind it. + */ +interface MediaUploader { + /** + * Upload a (possibly processed) file and return the finished WordPress attachment + * JSON the editor inserts — the same object a direct `POST /wp/v2/media` returns. + * Return only once the upload is genuinely done, or throw on terminal failure: a + * returned value is taken as a completed attachment, and there is no GutenbergKit + * recovery behind you. + * + * The [MediaUpload] carries the file plus the editor's form fields (e.g. `post`) + * and query — send them all so the created attachment matches a native upload + * rather than landing as an unattached orphan. + * + * That recovery is yours to run. When `POST /wp/v2/media` fatals in server-side + * post-processing it returns a 5xx carrying the attachment's ID in + * `x-wp-upload-attachment-id` — the attachment exists but is unfinished. Don't + * re-upload; drive `POST /wp/v2/media//post-process` to completion, the way + * core recovers its own uploads (up to 5 attempts), then return the finished + * attachment. + * + * Owning the upload means owning cleanup on the server too: if post-process can't + * be recovered, force-delete the orphan (`DELETE /wp/v2/media/?force=true`) + * before you throw, or it stays on the site — neither GutenbergKit nor the editor + * cleans up behind you. + */ + suspend fun upload(upload: MediaUpload): ByteArray +} + /** * A local HTTP server that receives file uploads from the WebView and routes * them through the native media processing pipeline. @@ -128,6 +212,7 @@ interface MediaUploadDelegate { internal class MediaUploadServer( private val uploadDelegate: MediaUploadDelegate?, private val internalClient: InternalMediaClient?, + private val uploader: MediaUploader? = null, cacheDir: File? = null, scope: CoroutineScope? = null, ioDispatcher: CoroutineDispatcher = Dispatchers.IO @@ -247,6 +332,15 @@ internal class MediaUploadServer( * Deliberately narrow: this server relays media operations, not arbitrary * REST requests, so only a numeric attachment ID under `/media/` matches. */ + /** + * The editor's non-file form parts as ordered, UTF-8-decoded fields. + * + * A list rather than a map so repeated names (e.g. a `field[]` array) survive + * verbatim, in the order the editor sent them. + */ + private fun formFields(parts: List): List = + parts.map { MediaUploadField(it.name, String(it.body.readBytes(), Charsets.UTF_8)) } + private fun attachmentIdFromPath(path: String): String? { val components = path.split("/").filter { it.isNotEmpty() } if (components.size != 2 || components[0] != "media") return null @@ -290,7 +384,10 @@ internal class MediaUploadServer( // like this. If not, forward the original upload to WordPress directly, // skipping a full temp-file copy of a file the delegate won't process or // upload (e.g. a video handed to an image-only delegate). - if (uploadDelegate?.handlesFile(mimeType, filename) != true) { + // An uploader takes over delivery for *every* file, so with one set there is no + // passthrough to fall to: only the delegate's metadata gate can decline a file, + // and only when no uploader is configured. + if (uploader == null && uploadDelegate?.handlesFile(mimeType, filename) != true) { return passthroughResponse(request, query) } @@ -461,7 +558,23 @@ internal class MediaUploadServer( } try { - // If the delegate provided its own upload, use that. + // An uploader owns delivery on the host's own stack and returns the finished + // attachment JSON (or throws); GutenbergKit relays that as a success and + // never runs its own recovery behind it. + uploader?.let { hostUploader -> + val upload = MediaUpload( + file = targetFile, + mimeType = targetMimeType, + filename = targetFilename, + fields = formFields(extraParts), + query = query + ) + return UploadResult.Uploaded(MediaUploadResponse(201, hostUploader.upload(upload))) + } + + // The deprecated delegate path: the host performs the POST but returns the + // raw response, leaving the editor to drive post-process recovery behind it. + @Suppress("DEPRECATION") uploadDelegate?.uploadFile(targetFile, targetMimeType, targetFilename)?.let { return UploadResult.Uploaded(it) } @@ -473,7 +586,7 @@ internal class MediaUploadServer( } val result = internalClient?.upload(targetFile, targetMimeType, targetFilename, extraParts, query) - ?: error("No upload delegate or default uploader configured") + ?: error("No upload delegate or internal media client configured") return UploadResult.Uploaded(result) } finally { // The processed file (if the delegate produced a new one) is ours to diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt index 855a8cc1a..436be1614 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -168,6 +168,111 @@ class MediaUploadServerTest { assertEquals(listOf("text/plain"), response.rawHeaderValues("content-type")) } + @Test + fun `an uploader performs the upload and its result is relayed`() { + val uploader = RecordingUploader() + val client = MockInternalMediaClient() + server.stop() + server = MediaUploadServer( + uploadDelegate = null, internalClient = client, uploader = uploader, cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-uploader" + val body = buildMultipartBody(boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray()) + val response = sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) + assertTrue(response.body.contains("\"id\":7")) + // GutenbergKit stays out of the network when a host uploader is set. + assertFalse(client.uploadCalled) + assertFalse(client.passthroughUploadCalled) + assertEquals("photo.jpg", uploader.received?.filename) + assertEquals("image/jpeg", uploader.received?.mimeType) + } + + @Test + fun `an uploader receives the editor's form fields in order, and the query`() { + // Without `post` the attachment is created unattached, and repeated names (a + // `field[]` array) must survive as repeats rather than collapse into a map. + val uploader = RecordingUploader() + server.stop() + server = MediaUploadServer( + uploadDelegate = null, internalClient = MockInternalMediaClient(), uploader = uploader, + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-fields" + val body = java.io.ByteArrayOutputStream().apply { + for ((name, value) in listOf("post" to "42", "tags[]" to "a", "tags[]" to "b")) { + write("--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"$name\"\r\n\r\n".toByteArray()) + write("$value\r\n".toByteArray()) + } + write("--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n".toByteArray()) + write("Content-Type: image/jpeg\r\n\r\n".toByteArray()) + write("fake image data".toByteArray()) + write("\r\n--$boundary--\r\n".toByteArray()) + }.toByteArray() + + sendRawRequest( + method = "POST", + path = "/upload?_embed=wp:featuredmedia", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertEquals( + listOf( + MediaUploadField("post", "42"), + MediaUploadField("tags[]", "a"), + MediaUploadField("tags[]", "b") + ), + uploader.received?.fields + ) + assertEquals("?_embed=wp:featuredmedia", uploader.received?.query) + } + + @Test + fun `an uploader sees a file the delegate's metadata gate would have declined`() { + // The gate exists to skip a temp copy for a file the delegate won't touch. An + // uploader takes over delivery for every file, so passing through here would + // silently bypass it. + val uploader = RecordingUploader() + val client = MockInternalMediaClient() + server.stop() + server = MediaUploadServer( + uploadDelegate = DecliningDelegate(), internalClient = client, uploader = uploader, + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-declined" + val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "movie".toByteArray()) + sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertEquals("clip.mov", uploader.received?.filename) + assertFalse(client.passthroughUploadCalled) + } + @Test fun `routes upload with a query string and relays the query`() { val delegate = ProcessOnlyDelegate() @@ -830,6 +935,21 @@ class MediaUploadServerTest { ) } + /** Records the [MediaUpload] it is handed, and returns a finished attachment. */ + private class RecordingUploader : MediaUploader { + @Volatile var received: MediaUpload? = null + + override suspend fun upload(upload: MediaUpload): ByteArray { + received = upload + return """{"id":7,"source_url":"https://example.com/photo.jpg","media_type":"image"}""".toByteArray() + } + } + + /** A delegate that declines every file by metadata. */ + private class DecliningDelegate : MediaUploadDelegate { + override fun handlesFile(mimeType: String, filename: String) = false + } + private class MockInternalMediaClient : InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = "https://example.com/wp-json/", diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index e4327a28b..0daeb6d76 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -120,6 +120,37 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro /// don't need to keep a reference after assigning it. The one rule: your delegate /// must not strongly retain this `EditorViewController` in return, or the two form /// a retain cycle and neither is freed. + /// Takes over media upload on the host's own stack (background session, offline + /// queue, resumable transport). Setting it makes the host own every upload and its + /// whole lifecycle; GutenbergKit stays out of the network entirely for media. + /// + /// Same lifecycle rules as ``mediaUploadDelegate``: set it before the editor loads. + /// The editor owns it for its lifetime (releasing it on `deinit`), so you needn't + /// retain it yourself — just don't strongly retain this `EditorViewController` + /// from your uploader. + /// + /// Takes precedence over the deprecated ``MediaUploadDelegate/uploadFile(at:mimeType:filename:)``: + /// with an uploader set, that hook is never called. + public var mediaUploader: (any MediaUploader)? { + didSet { + precondition(!hasStartedLoading, Self.lateMediaAssignmentMessage("mediaUploader")) + } + } + + /// Message for the fail-fast when media handling is assigned too late. + /// + /// Deliberate fail-fast, not a defensive check: the handler is captured into the + /// page's initial configuration when the editor begins loading, so one assigned + /// afterward would silently never take effect. `hasStartedLoading` flips at the + /// start of the async load, which runs at or after `viewDidLoad`, so a host that + /// follows the contract (set right after `init`) can never race it. Do not soften + /// this to a no-op or a log — silently dropping the host's media handling is + /// exactly the failure this catches. + private static func lateMediaAssignmentMessage(_ name: String) -> String { + "\(name) must be set before the editor loads (e.g. right after init). " + + "It is captured into the editor configuration at load; setting it afterward has no effect." + } + // Ownership here is the point: the editor holds this for its lifetime so an // in-flight upload can't lose the delegate mid-request. The cycle `weak_delegate` // guards against runs the other way (a delegate retaining the editor), which this @@ -127,23 +158,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro // swiftlint:disable:next weak_delegate public var mediaUploadDelegate: (any MediaUploadDelegate)? { didSet { - // Deliberate fail-fast, not a defensive check. The delegate is captured - // into the page's initial configuration when the editor begins loading, - // so a delegate assigned afterward would silently never take effect; - // trapping surfaces that misuse loudly instead of failing quietly. - // - // `hasStartedLoading` flips at the start of the async load (see - // `loadEditor`), which runs at or after `viewDidLoad` — so this only - // *widens* the safe window versus a synchronous flip. A host that - // follows the documented contract (set right after `init`, before - // presenting) can never race it; the trap fires only on a genuinely - // late assignment. Do not soften this to a no-op or a log — silently - // dropping the delegate is exactly the failure this is here to catch. - precondition( - !hasStartedLoading, - "mediaUploadDelegate must be set before the editor loads (e.g. right after init). " - + "It is captured into the editor configuration at load; setting it afterward has no effect." - ) + precondition(!hasStartedLoading, Self.lateMediaAssignmentMessage("mediaUploadDelegate")) } } @@ -448,10 +463,10 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro /// because `nativeUploadPort` will be nil in GBKit). private func startUploadServer() async { // Nothing to route through the native server unless the host provided a - // delegate. The editor owns it — `mediaUploadDelegate` is strong — so there's - // no released-before-load case to guard against; it lives as long as the - // editor does. - guard mediaUploadDelegate != nil else { + // delegate or an uploader. The editor owns whichever it was given — both + // properties are strong — so there's no released-before-load case to guard + // against; they live as long as it does. + guard mediaUploadDelegate != nil || mediaUploader != nil else { return } @@ -479,6 +494,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro do { self.uploadServer = try await MediaUploadServer.start( uploadDelegate: mediaUploadDelegate, + uploader: mediaUploader, internalClient: internalClient ) } catch { diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift index 73752166b..8c7f791c6 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift @@ -78,9 +78,18 @@ public protocol MediaUploadDelegate: AnyObject, Sendable { /// Upload a processed file to the remote WordPress site. /// /// Return the raw WordPress response (status code + body), which GutenbergKit - /// relays to the editor unchanged, or `nil` to use the default uploader. A + /// relays to the editor unchanged, or `nil` to use the internal media client. A /// host that uploads to WordPress should return the exact response it /// received so the editor sees a complete attachment object. + /// + /// - Warning: Returning a raw response splits one upload's HTTP across two + /// owners — you perform the `POST`, but GutenbergKit's editor drives the + /// `post-process` retries and orphan cleanup behind it, through the WebView + /// rather than your stack. It also receives no form fields, so an attachment + /// uploaded this way lands unattached to its post. Conform to ``MediaUploader`` + /// instead: it owns the upload end-to-end and receives a ``MediaUpload`` + /// carrying the fields. + @available(*, deprecated, message: "Conform to MediaUploader instead — it owns the upload's retries and receives the editor's form fields.") func uploadFile(at url: URL, mimeType: String, filename: String) async throws -> MediaUploadResponse? } @@ -94,7 +103,96 @@ extension MediaUploadDelegate { .original } + @available(*, deprecated, message: "Conform to MediaUploader instead — it owns the upload's retries and receives the editor's form fields.") public func uploadFile(at url: URL, mimeType: String, filename: String) async throws -> MediaUploadResponse? { nil } } + +/// One of the editor's non-file form fields, as sent with a media upload. +/// +/// A named type rather than a `(name, value)` tuple: tuples are not nominal, so a +/// tuple-typed property would permanently block `Equatable`/`Hashable`/`Codable` +/// synthesis on ``MediaUpload`` — including inside GutenbergKit, and not fixable +/// later without a source break for every host. +public struct MediaUploadField: Sendable, Hashable, Codable { + /// The field name, e.g. `post`. Not unique — a `field[]` array repeats it. + public let name: String + + /// The field's value, decoded as UTF-8. + public let value: String + + public init(name: String, value: String) { + self.name = name + self.value = value + } +} + +/// Everything a ``MediaUploader`` needs to reproduce a native upload: the file to +/// send, its metadata, the editor's non-file form fields, and the request's query. +public struct MediaUpload: Sendable { + /// The file to upload — already processed, if a ``MediaUploadDelegate`` ran. + public let fileURL: URL + + /// The file's MIME type. + public let mimeType: String + + /// The file's name. + public let filename: String + + /// The editor's non-file form fields, in order, each decoded as UTF-8 — most + /// importantly `post`, the parent post's ID, without which the attachment is + /// created unattached. A list, not a dictionary, so repeated field names (e.g. a + /// `field[]` array) survive verbatim. Send each as a form part on your + /// `POST /wp/v2/media`, in the given order. + public let fields: [MediaUploadField] + + /// The request's query string (leading `?`, e.g. `?_embed=wp:featuredmedia`), or + /// empty. Carry it on your request so the editor gets the response it expects. + public let query: String + + public init(fileURL: URL, mimeType: String, filename: String, fields: [MediaUploadField], query: String) { + self.fileURL = fileURL + self.mimeType = mimeType + self.filename = filename + self.fields = fields + self.query = query + } +} + +/// Takes over *performing* a media upload — on the host's own stack: its own +/// networking (say, to log every request), a background session, an offline queue, +/// a resumable transport, its own retry policy. +/// +/// This is a choice of *who executes the requests*, not where they go: an uploader +/// and GutenbergKit's internal media client both target the same configured site. +/// Setting ``EditorViewController/mediaUploader`` makes the host own that upload +/// end-to-end — the request, its own retries, and its recovery and cleanup — with +/// GutenbergKit out of the network entirely. Because the host does the retries +/// itself, there's no raw response left for the editor to retry behind it. The +/// attachment you return lives on that same configured site, where the editor reads +/// and updates it by ID. +public protocol MediaUploader: AnyObject, Sendable { + /// Upload a (possibly processed) file and return the finished WordPress + /// attachment JSON the editor inserts — the same object a direct + /// `POST /wp/v2/media` returns. Return only once the upload is genuinely done, + /// or `throw` on terminal failure: a returned value is taken as a completed + /// attachment, and there is no GutenbergKit recovery behind you. + /// + /// The ``MediaUpload`` carries the file plus the editor's form fields (e.g. + /// `post`) and query — send them all so the created attachment matches a native + /// upload rather than landing as an unattached orphan. + /// + /// That recovery is yours to run. When `POST /wp/v2/media` fatals in server-side + /// post-processing it returns a 5xx carrying the attachment's ID in + /// `x-wp-upload-attachment-id` — the attachment exists but is unfinished. Don't + /// re-upload; drive `POST /wp/v2/media//post-process` to completion, the way + /// core recovers its own uploads (up to 5 attempts), then return the finished + /// attachment. + /// + /// Owning the upload means owning cleanup on the server too: if post-process + /// can't be recovered, force-delete the orphan + /// (`DELETE /wp/v2/media/?force=true`) before you `throw`, or it stays on the + /// site — neither GutenbergKit nor the editor cleans up behind you. + func upload(_ upload: MediaUpload) async throws -> Data +} diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index ad192928b..1e0fb16ac 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -30,11 +30,13 @@ final class MediaUploadServer: Sendable { /// /// - Parameters: /// - uploadDelegate: Optional delegate for customizing file processing and upload. + /// - uploader: Optional host uploader that performs the upload on its own stack. /// - internalClient: Fallback uploader used when no delegate provides `uploadFile`. /// - maxRequestBodySize: The maximum allowed request body size in bytes. /// Requests exceeding this limit receive a 413 response. Defaults to 4 GB. static func start( uploadDelegate: (any MediaUploadDelegate)? = nil, + uploader: (any MediaUploader)? = nil, internalClient: InternalMediaClient? = nil, maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize ) async throws -> MediaUploadServer { @@ -45,7 +47,7 @@ final class MediaUploadServer: Sendable { cleanOrphanedUploads() } - let context = UploadContext(uploadDelegate: uploadDelegate, internalClient: internalClient) + let context = UploadContext(uploadDelegate: uploadDelegate, uploader: uploader, internalClient: internalClient) // A generous ceiling for receiving the upload body. The body read is // primarily bounded by the per-read idle timeout (which reaps a stalled @@ -126,11 +128,16 @@ final class MediaUploadServer: Sendable { let filename = filePart.filename ?? "upload" let mimeType = filePart.contentType - // Ask the delegate — from metadata alone — whether it will touch a file - // like this. If not, forward the original upload to WordPress directly, - // skipping a full temp-file copy of a file the delegate won't process or - // upload (e.g. a video handed to an image-only delegate). - guard context.uploadDelegate?.handlesFile(ofType: mimeType, named: filename) ?? false else { + // Ask the delegate — from metadata alone — whether it will touch a file like + // this. If not, forward the original upload to WordPress directly, skipping a + // full temp-file copy of a file the delegate won't process (e.g. a video handed + // to an image-only delegate). + // + // An uploader takes over delivery for *every* file, so with one set there is + // no passthrough to fall to: only the delegate's metadata gate can decline a + // file, and only when no uploader is configured. + let delegateWantsFile = context.uploadDelegate?.handlesFile(ofType: mimeType, named: filename) ?? false + guard context.uploader != nil || delegateWantsFile else { do { return try await passthroughResponse(request, query: query, internalClient: context.internalClient) } catch { @@ -204,6 +211,29 @@ final class MediaUploadServer: Sendable { /// /// Deliberately narrow: this server relays media operations, not arbitrary /// REST requests, so only a numeric attachment ID under `/media/` matches. + /// The editor's non-file form parts as ordered, UTF-8-decoded fields. + /// + /// A list rather than a dictionary so repeated names (e.g. a `field[]` array) + /// survive verbatim, in the order the editor sent them. + private static func formFields(from parts: [MultipartPart]) async throws -> [MediaUploadField] { + var fields: [MediaUploadField] = [] + for part in parts { + fields.append(MediaUploadField(name: part.name, value: String(decoding: try await part.body.data, as: UTF8.self))) + } + return fields + } + + /// Calls the deprecated `uploadFile` hook from one place. + /// + /// This deliberately leaves one deprecation warning in GutenbergKit's own build: + /// the marker exists to tell *hosts* to migrate, and supporting the hook until it + /// is removed means calling it. The warning marks the code that goes with it. + private static func deprecatedUploadFile( + _ delegate: any MediaUploadDelegate, _ url: URL, _ mimeType: String, _ filename: String + ) async throws -> MediaUploadResponse? { + try await delegate.uploadFile(at: url, mimeType: mimeType, filename: filename) + } + private static func attachmentId(fromPath path: String) -> String? { let components = path.split(separator: "/", omittingEmptySubsequences: true) guard components.count == 2, components[0] == "media" else { return nil } @@ -324,9 +354,25 @@ final class MediaUploadServer: Sendable { // keeps this true for a host-injected `URLSessionProtocol` that doesn't. try Task.checkCancellation() - // Step 2: Upload to remote WordPress + // Step 2: deliver. An uploader owns delivery on the host's own stack and + // returns the finished attachment JSON (or throws); GutenbergKit relays that + // as a success and never runs its own recovery behind it. + if let uploader = context.uploader { + let upload = MediaUpload( + fileURL: uploadURL, + mimeType: uploadMimeType, + filename: uploadFilename, + fields: try await formFields(from: extraParts), + query: query + ) + let attachment = try await uploader.upload(upload) + return .uploaded(MediaUploadResponse(statusCode: 201, body: attachment)) + } + + // The deprecated delegate path: the host performs the POST but returns the raw + // response, leaving the editor to drive post-process recovery behind it. if let delegate = context.uploadDelegate, - let result = try await delegate.uploadFile(at: uploadURL, mimeType: uploadMimeType, filename: uploadFilename) { + let result = try await deprecatedUploadFile(delegate, uploadURL, uploadMimeType, uploadFilename) { return .uploaded(result) } else if let internalClient = context.internalClient { // Unmodified — forward the original request body directly, skipping @@ -481,6 +527,7 @@ enum UploadError: Error, LocalizedError { /// protocol and `InternalMediaClient` is `@unchecked Sendable`. private struct UploadContext: Sendable { let uploadDelegate: (any MediaUploadDelegate)? + let uploader: (any MediaUploader)? let internalClient: InternalMediaClient? } diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 20aa7899e..e1183fcf6 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -385,6 +385,147 @@ struct MediaUploadServerTests { #expect(FileManager.default.fileExists(atPath: fresh.path(percentEncoded: false))) } + @Test("an uploader performs the upload and its result is relayed") + func uploaderPerformsUpload() async throws { + let uploader = RecordingUploader() + let internalClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: internalClient) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody(boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", data: Data("fake image data".utf8)) + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + let (data, response) = try await URLSession.shared.data(for: request) + let httpResponse = try #require(response as? HTTPURLResponse) + + #expect(httpResponse.statusCode == 201) + #expect(String(decoding: data, as: UTF8.self).contains("\"id\":7")) + // GutenbergKit stays out of the network when a host uploader is set. + #expect(!internalClient.uploadCalled) + #expect(!internalClient.passthroughUploadCalled) + #expect(uploader.received?.filename == "photo.jpg") + #expect(uploader.received?.mimeType == "image/jpeg") + } + + @Test("an uploader receives the editor's form fields in order, and the query") + func uploaderReceivesFieldsAndQuery() async throws { + // Without `post` the attachment is created unattached, and repeated names (a + // `field[]` array) must survive as repeats rather than collapse into a dictionary. + let uploader = RecordingUploader() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: MockInternalMediaClient()) + defer { server.stop() } + + let boundary = UUID().uuidString + var body = Data() + for (name, value) in [("post", "42"), ("tags[]", "a"), ("tags[]", "b")] { + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"\(name)\"\r\n\r\n") + body.append("\(value)\r\n") + } + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n") + body.append("Content-Type: image/jpeg\r\n\r\n") + body.append(Data("fake image data".utf8)) + body.append("\r\n--\(boundary)--\r\n") + + let url = URL(string: "http://127.0.0.1:\(server.port)/upload?_embed=wp:featuredmedia")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + _ = try await URLSession.shared.data(for: request) + + let received = try #require(uploader.received) + #expect(received.fields == [ + MediaUploadField(name: "post", value: "42"), + MediaUploadField(name: "tags[]", value: "a"), + MediaUploadField(name: "tags[]", value: "b"), + ]) + #expect(received.query == "?_embed=wp:featuredmedia") + } + + @Test("an uploader takes precedence over the deprecated uploadFile hook") + func uploaderWinsOverDeprecatedHook() async throws { + let delegate = MockUploadDelegate() + let uploader = RecordingUploader() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, uploader: uploader, internalClient: MockInternalMediaClient()) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody(boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", data: Data("fake image data".utf8)) + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + _ = try await URLSession.shared.data(for: request) + + // The delegate still processes; only delivery moves to the uploader. + #expect(delegate.processFileCalled) + #expect(!delegate.uploadFileCalled) + #expect(uploader.received != nil) + } + + @Test("an uploader sees a file the delegate's metadata gate would have declined") + func uploaderSeesDeclinedFile() async throws { + // The gate exists to skip a temp copy for a file the delegate won't touch. An + // uploader takes over delivery for every file, so passing through here would + // silently bypass it. + let delegate = DecliningDelegate() + let uploader = RecordingUploader() + let internalClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, uploader: uploader, internalClient: internalClient) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody(boundary: boundary, filename: "clip.mov", mimeType: "video/quicktime", data: Data("movie".utf8)) + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + _ = try await URLSession.shared.data(for: request) + + #expect(uploader.received?.filename == "clip.mov") + #expect(!internalClient.passthroughUploadCalled) + } + + @Test("an uploader that throws surfaces as a failure, with no GutenbergKit retry") + func uploaderThrowSurfaces() async throws { + let internalClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploader: ThrowingUploader(), internalClient: internalClient) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody(boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", data: Data("fake image data".utf8)) + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + let (_, response) = try await URLSession.shared.data(for: request) + let httpResponse = try #require(response as? HTTPURLResponse) + + #expect(httpResponse.statusCode == 500) + // Recovery is the uploader's, not GutenbergKit's — it must not re-deliver. + #expect(!internalClient.uploadCalled) + #expect(!internalClient.passthroughUploadCalled) + } + @Test("retains the delegate for the server's lifetime, and releases it after") func retainsDelegateForServerLifetime() async throws { weak var weakDelegate: MockUploadDelegate? @@ -842,6 +983,36 @@ private func readAllFromStream(_ stream: InputStream) -> Data { // MARK: - Mocks +/// Records the ``MediaUpload`` it is handed, and returns a finished attachment. +private final class RecordingUploader: MediaUploader, @unchecked Sendable { + private let lock = NSLock() + private var _received: MediaUpload? + + var received: MediaUpload? { lock.withLock { _received } } + + func upload(_ upload: MediaUpload) async throws -> Data { + lock.withLock { _received = upload } + return Data(#"{"id":7,"source_url":"https://example.com/photo.jpg","media_type":"image"}"#.utf8) + } +} + +/// An uploader whose delivery fails terminally, as one would after exhausting its own +/// post-process recovery and force-deleting the orphan. +private final class ThrowingUploader: MediaUploader, @unchecked Sendable { + struct Failure: Error {} + + func upload(_ upload: MediaUpload) async throws -> Data { + throw Failure() + } +} + +/// A delegate that declines every file by metadata. +private final class DecliningDelegate: MediaUploadDelegate, @unchecked Sendable { + func handlesFile(ofType mimeType: String, named filename: String) -> Bool { + false + } +} + /// A delegate that transcodes, used to check the server holds it across the whole /// request rather than re-reading a reference the host may have dropped. private final class TranscodingDelegate: MediaUploadDelegate, @unchecked Sendable {