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 {