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 d893265f4..accb34f03 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -681,7 +681,7 @@ class GutenbergView : FrameLayout { // fall to the default WebView path. (Matches iOS.) if (mediaUploadDelegate == null) return - // The native upload server relays through DefaultMediaUploader, which needs a + // The native upload server relays through InternalMediaClient, which needs a // site root and an auth header (every host provides one — the editor injects // it because the WebView has no auth cookies). Without both there is nothing // to upload through, so leave the server down and let uploads fall to the @@ -706,7 +706,7 @@ class GutenbergView : FrameLayout { } try { - val defaultUploader = DefaultMediaUploader( + val internalClient = InternalMediaClient( httpClient = uploadHttpClient, siteApiRoot = configuration.siteApiRoot, authHeader = configuration.authHeader, @@ -714,7 +714,7 @@ class GutenbergView : FrameLayout { ) uploadServer = MediaUploadServer( uploadDelegate = mediaUploadDelegate, - defaultUploader = defaultUploader, + internalClient = internalClient, 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 6ce861d4b..defbe3b38 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -127,7 +127,7 @@ interface MediaUploadDelegate { */ internal class MediaUploadServer( private val uploadDelegate: MediaUploadDelegate?, - private val defaultUploader: DefaultMediaUploader?, + private val internalClient: InternalMediaClient?, cacheDir: File? = null, scope: CoroutineScope? = null, ioDispatcher: CoroutineDispatcher = Dispatchers.IO @@ -264,7 +264,7 @@ internal class MediaUploadServer( * browser blocks it at preflight. Relaying it here lets the cleanup run. */ private suspend fun handleDelete(attachmentId: String, query: String): HttpResponse { - val uploader = defaultUploader ?: return errorResponse(500, "No uploader configured") + val uploader = internalClient ?: return errorResponse(500, "No internal media client configured") return try { relayResponse(uploader.deleteMedia(attachmentId, query)) } catch (e: IOException) { @@ -407,11 +407,12 @@ internal class MediaUploadServer( throw e // Never swallow coroutine cancellation. } catch (e: Exception) { // Any other failure — IOException from the upload call, JSON parse - // errors, a throwing host delegate, or "no uploader configured" — - // must still be answered WITH CORS headers. Otherwise it escapes to - // HttpServer's header-less 500 fallback and the browser rejects the - // preflighted cross-origin fetch with an opaque "Failed to fetch", - // hiding the real error from the editor (mirrors the iOS catch-all). + // errors, a throwing host delegate, or "no internal media client + // configured" — must still be answered WITH CORS headers. Otherwise + // it escapes to HttpServer's header-less 500 fallback and the browser + // rejects the preflighted cross-origin fetch with an opaque "Failed to + // fetch", hiding the real error from the editor (mirrors the iOS + // catch-all). Log.e(TAG, "Upload failed", e) return errorResponse(500, e.message ?: "Upload failed") } finally { @@ -429,9 +430,9 @@ internal class MediaUploadServer( private suspend fun performPassthroughUpload(request: HttpRequest, query: String): MediaUploadResponse { val body = request.body val contentType = request.header("Content-Type") - val uploader = defaultUploader + val uploader = internalClient if (body == null || contentType == null || uploader == null) { - throw MediaUploadException("Passthrough upload requires a request body, Content-Type, and default uploader") + throw MediaUploadException("Passthrough upload requires a request body, Content-Type, and internal media client") } return uploader.passthroughUpload(body, contentType, query) } @@ -472,8 +473,8 @@ internal class MediaUploadServer( return UploadResult.Passthrough } - val result = defaultUploader?.upload(targetFile, targetMimeType, targetFilename, extraParts, query) - ?: error("No upload delegate or default uploader configured") + val result = internalClient?.upload(targetFile, targetMimeType, targetFilename, extraParts, query) + ?: 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 @@ -528,9 +529,15 @@ internal class MediaUploadServer( internal class MediaUploadException(message: String, cause: Throwable? = null) : Exception(message, cause) /** - * Uploads files to the WordPress REST API using OkHttp. + * GutenbergKit's own client for the configured site, built from the site credentials + * in the editor configuration. + * + * Not an implementation of any host-facing interface — it is the thing that actually + * performs GutenbergKit's media requests. It delivers uploads the host did not take + * over, and relays the editor's media deletes: the editor only ever asks to delete + * `/wp/v2/media/` on the configured site, so that is where the relay sends it. */ -internal open class DefaultMediaUploader( +internal open class InternalMediaClient( private val httpClient: okhttp3.OkHttpClient, private val siteApiRoot: String, private val authHeader: String, 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 e7d10e942..ba8690037 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -33,7 +33,7 @@ class MediaUploadServerTest { @Before fun setUp() { - server = MediaUploadServer(uploadDelegate = null, defaultUploader = null, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = null, internalClient = null, cacheDir = tempFolder.root) } @After @@ -53,7 +53,7 @@ class MediaUploadServerTest { fun `stop cancels an internally-created scope but leaves a caller-supplied one alone`() { // No scope supplied → the server owns one, which stop() must cancel. val owningServer = - MediaUploadServer(uploadDelegate = null, defaultUploader = null, cacheDir = tempFolder.root) + MediaUploadServer(uploadDelegate = null, internalClient = null, cacheDir = tempFolder.root) val ownedScope = ownedScopeOf(owningServer) assertNotNull("server should own a scope when none is supplied", ownedScope) assertTrue(ownedScope!!.isActive) @@ -64,7 +64,7 @@ class MediaUploadServerTest { val callerScope = CoroutineScope(Dispatchers.IO) val borrowingServer = MediaUploadServer( uploadDelegate = null, - defaultUploader = null, + internalClient = null, cacheDir = tempFolder.root, scope = callerScope ) @@ -150,9 +150,9 @@ class MediaUploadServerTest { // Exercised through the delete relay because every response relayResponse // handles — WordPress's own included — carries a `Content-Type`, so this is // the ordinary path rather than an edge case. - val uploader = ContentTypeDeleteUploader() + val uploader = ContentTypeDeleteClient() server.stop() - server = MediaUploadServer(uploadDelegate = null, defaultUploader = uploader, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = null, internalClient = uploader, cacheDir = tempFolder.root) val response = sendRawRequest( method = "DELETE", @@ -171,9 +171,9 @@ class MediaUploadServerTest { @Test fun `routes upload with a query string and relays the query`() { val delegate = ProcessOnlyDelegate() - val mockUploader = MockDefaultUploader() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = delegate, internalClient = mockUploader, cacheDir = tempFolder.root) // `@wordpress/media-utils` uploads to `/wp/v2/media?_embed=wp:featuredmedia`, // so the middleware forwards that query on to the native server. Routing must @@ -206,7 +206,7 @@ class MediaUploadServerTest { fun `calls delegate processFile and uploadFile`() { val delegate = MockUploadDelegate() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = null, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = delegate, internalClient = null, cacheDir = tempFolder.root) val boundary = "test-boundary-123" val body = buildMultipartBody(boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray()) @@ -237,9 +237,9 @@ class MediaUploadServerTest { @Test fun `forwards the delegate's processed metadata to the uploader`() { val delegate = TranscodingDelegate() - val mockUploader = MockDefaultUploader() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = delegate, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-meta" val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "movie".toByteArray()) @@ -264,9 +264,9 @@ class MediaUploadServerTest { @Test fun `deletes the delegate's processed file after upload`() { val delegate = TranscodingDelegate() - val mockUploader = MockDefaultUploader() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = delegate, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-cleanup" val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "movie".toByteArray()) @@ -305,7 +305,7 @@ class MediaUploadServerTest { server.stop() server = MediaUploadServer( uploadDelegate = null, - defaultUploader = null, + internalClient = null, cacheDir = tempFolder.root, ioDispatcher = Dispatchers.Unconfined ) @@ -314,15 +314,15 @@ class MediaUploadServerTest { assertTrue("Fresh temp should be preserved", fresh.exists()) } - // MARK: - Fallback to default uploader + // MARK: - Fallback to the internal media client @Test fun `uses passthrough when delegate does not modify file`() { val delegate = ProcessOnlyDelegate() - val mockUploader = MockDefaultUploader() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = delegate, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-456" val body = buildMultipartBody(boundary, "doc.pdf", "application/pdf", "fake pdf data".toByteArray()) @@ -350,10 +350,10 @@ class MediaUploadServerTest { @Test fun `skips processing and the temp copy when the delegate declines by metadata`() { val delegate = DeclineByMetadataDelegate() - val mockUploader = MockDefaultUploader() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(uploadDelegate = delegate, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-decline" val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "fake movie".toByteArray()) @@ -376,10 +376,10 @@ class MediaUploadServerTest { assertFalse(mockUploader.uploadCalled) } - // MARK: - DefaultMediaUploader + // MARK: - InternalMediaClient @Test - fun `DefaultMediaUploader relays the WordPress response`() { + fun `InternalMediaClient relays the WordPress response`() { val mockWpServer = MockWebServer() val wpBody = """{"id":1,"source_url":"https://example.com/u.jpg","media_type":"image"}""" @@ -392,7 +392,7 @@ class MediaUploadServerTest { mockWpServer.start() val wpBaseUrl = mockWpServer.url("/wp-json/").toString() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = wpBaseUrl, authHeader = "Bearer test-token" @@ -417,13 +417,13 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader relays a WordPress error response instead of throwing`() { + fun `InternalMediaClient relays a WordPress error response instead of throwing`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(500).setBody("Internal error")) mockWpServer.start() val wpBaseUrl = mockWpServer.url("/wp-json/").toString() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = wpBaseUrl, authHeader = "Bearer test-token" @@ -442,7 +442,7 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader relays the upload attachment ID header`() { + fun `InternalMediaClient relays the upload attachment ID header`() { // WordPress sets this header on an upload whose attachment row was // created before metadata generation fataled. The editor reads it to // retry post-process and clean up the orphan, so it must survive the @@ -457,7 +457,7 @@ class MediaUploadServerTest { ) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -475,12 +475,12 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader deletes an attachment carrying namespace and force query`() { + fun `InternalMediaClient deletes an attachment carrying namespace and force query`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(200).setBody("""{"deleted":true}""")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token", @@ -498,12 +498,12 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader normalizes an unslashed root and namespace`() { + fun `InternalMediaClient normalizes an unslashed root and namespace`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(201).setBody("{}")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json").toString(), // no trailing slash authHeader = "Bearer test-token", @@ -535,7 +535,7 @@ class MediaUploadServerTest { ) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -563,12 +563,12 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader re-encode preserves extra parts and query`() { + fun `InternalMediaClient re-encode preserves extra parts and query`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(201).setBody("{}")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -603,7 +603,7 @@ class MediaUploadServerTest { mockWpServer.enqueue(MockResponse().setResponseCode(201).setBody("{}")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -814,11 +814,11 @@ class MediaUploadServerTest { } /** - * A default uploader whose delete response carries its own `Content-Type`, + * An internal media client whose delete response carries its own `Content-Type`, * lowercased, so the relay must override the JSON default rather than emit * the header twice. */ - private class ContentTypeDeleteUploader : DefaultMediaUploader( + private class ContentTypeDeleteClient : InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = "https://example.com/wp-json/", authHeader = "Bearer mock" @@ -830,7 +830,7 @@ class MediaUploadServerTest { ) } - private class MockDefaultUploader : DefaultMediaUploader( + private class MockInternalMediaClient : InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = "https://example.com/wp-json/", authHeader = "Bearer mock" diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index 1f01737d8..269d21b72 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -564,7 +564,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro return } - // The native upload server relays through DefaultMediaUploader, which needs a + // The native upload server relays through InternalMediaClient, which needs a // site root and an auth header (every host provides one — the editor injects // it because the WebView has no auth cookies). Without both there is nothing // to upload through, so leave the server down and let uploads fall to the @@ -579,7 +579,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro return } - let defaultUploader = DefaultMediaUploader( + let internalClient = InternalMediaClient( httpClient: httpClient.uploadClient(), siteApiRoot: configuration.siteApiRoot, siteApiNamespace: configuration.siteApiNamespace @@ -588,7 +588,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro do { let server = try await MediaUploadServer.start( uploadDelegate: mediaUploadDelegate, - defaultUploader: defaultUploader + internalClient: internalClient ) // `stopMediaHandling()` can land while the bind is in flight: it is a diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift index a8ce4ee51..f4e48c8f3 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift @@ -7,10 +7,10 @@ import Foundation /// this check, which already diverged silently between iOS and Android once. Living /// here, it is reachable from the host test suite. enum MediaServerCredentials { - /// Whether a ``DefaultMediaUploader`` built from this configuration could actually + /// Whether an ``InternalMediaClient`` built from this configuration could actually /// reach the site. /// - /// Both fields are required. The uploader delivers GutenbergKit's uploads to the + /// Both fields are required. The client delivers GutenbergKit's uploads to the /// configured site, so it needs somewhere to send them and credentials to be /// accepted; with either missing, every media request it makes fails. /// diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 6d0561416..ed58ad69e 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -30,12 +30,13 @@ final class MediaUploadServer: Sendable { /// /// - Parameters: /// - uploadDelegate: Optional delegate for customizing file processing and upload. - /// - defaultUploader: Fallback uploader used when no delegate provides `uploadFile`. + /// - internalClient: GutenbergKit's own client for the configured site. Delivers + /// uploads when no delegate provides `uploadFile`, and every media delete. /// - 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, - defaultUploader: DefaultMediaUploader? = nil, + internalClient: InternalMediaClient? = nil, maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize ) async throws -> MediaUploadServer { // Sweep temp files orphaned by a prior crash, off the editor-startup @@ -45,7 +46,7 @@ final class MediaUploadServer: Sendable { cleanOrphanedUploads() } - let context = UploadContext(uploadDelegate: uploadDelegate, defaultUploader: defaultUploader) + let context = UploadContext(uploadDelegate: uploadDelegate, 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 @@ -153,7 +154,7 @@ final class MediaUploadServer: Sendable { } if method == "DELETE", let attachmentId = attachmentId(fromPath: parsed.path) { - return await handleDelete(attachmentId, query: parsed.query, context: context) + return await handleDelete(attachmentId, query: parsed.query, internalClient: context.internalClient) } return errorResponse(status: 404, message: "Not found") @@ -187,7 +188,7 @@ final class MediaUploadServer: Sendable { // upload (e.g. a video handed to an image-only delegate). guard context.uploadDelegate?.handlesFile(ofType: mimeType, named: filename) ?? false else { do { - return try await passthroughResponse(request, query: query, context: context) + return try await passthroughResponse(request, query: query, internalClient: context.internalClient) } catch { return uploadErrorResponse(error) } @@ -227,7 +228,7 @@ final class MediaUploadServer: Sendable { case .passthrough: // Delegate didn't modify the file — forward the original request // body to WordPress without re-encoding. - return try await passthroughResponse(request, query: query, context: context) + return try await passthroughResponse(request, query: query, internalClient: context.internalClient) } } catch { return uploadErrorResponse(error) @@ -239,7 +240,7 @@ final class MediaUploadServer: Sendable { /// the file — it declined by metadata (`handlesFile` returned false) or /// `processFile` returned `.original`. private static func passthroughResponse( - _ request: HTTPServer.Request, query: String, context: UploadContext + _ request: HTTPServer.Request, query: String, internalClient: InternalMediaClient? ) async throws -> HTTPResponse { // As in `processAndUpload`: don't put bytes on the wire for a torn-down // editor, regardless of whether the HTTP client honors cancellation. @@ -248,10 +249,10 @@ final class MediaUploadServer: Sendable { Logger.uploadServer.debug("Passthrough: forwarding original request body to WordPress") guard let body = request.parsed.body, let contentType = request.parsed.header("Content-Type"), - let defaultUploader = context.defaultUploader else { + let internalClient else { return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) } - let response = try await defaultUploader.passthroughUpload(body: body, contentType: contentType, query: query) + let response = try await internalClient.passthroughUpload(body: body, contentType: contentType, query: query) return relayResponse(response) } @@ -275,13 +276,13 @@ final class MediaUploadServer: Sendable { /// `X-HTTP-Method-Override`, which core's CORS allow-list omits, so the /// browser blocks it at preflight. Relaying it here lets the cleanup run. private static func handleDelete( - _ attachmentId: String, query: String, context: UploadContext + _ attachmentId: String, query: String, internalClient: InternalMediaClient? ) async -> HTTPResponse { - guard let defaultUploader = context.defaultUploader else { + guard let internalClient else { return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) } do { - let response = try await defaultUploader.deleteMedia(attachmentId: attachmentId, query: query) + let response = try await internalClient.deleteMedia(attachmentId: attachmentId, query: query) return relayResponse(response) } catch { return uploadErrorResponse(error) @@ -327,7 +328,7 @@ final class MediaUploadServer: Sendable { /// Result of the delegate processing + upload pipeline. private enum UploadResult { - /// The delegate (or default uploader) completed the upload; carries the + /// The delegate (or the internal media client) completed the upload; carries the /// raw WordPress response to relay. case uploaded(MediaUploadResponse) /// The delegate didn't modify the file and `uploadFile` returned nil. @@ -383,13 +384,13 @@ final class MediaUploadServer: Sendable { if let delegate = context.uploadDelegate, let result = try await delegate.uploadFile(at: uploadURL, mimeType: uploadMimeType, filename: uploadFilename) { return .uploaded(result) - } else if let defaultUploader = context.defaultUploader { + } else if let internalClient = context.internalClient { // Unmodified — forward the original request body directly, skipping // multipart re-encoding. if case .original = processed { return .passthrough } - let result = try await defaultUploader.upload(fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, extraParts: extraParts, query: query) + let result = try await internalClient.upload(fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, extraParts: extraParts, query: query) return .uploaded(result) } else { throw UploadError.noUploader @@ -509,7 +510,7 @@ enum UploadError: Error, LocalizedError { var errorDescription: String? { switch self { - case .noUploader: "No upload delegate or default uploader configured" + case .noUploader: "No upload delegate or internal media client configured" case .streamReadFailed: "Failed to read upload stream" case .streamWriteFailed: "Failed to write upload to disk" } @@ -518,13 +519,16 @@ enum UploadError: Error, LocalizedError { // MARK: - Upload Context -/// Container for the upload delegate and default uploader, captured by the +/// Container for the upload delegate and the internal media client, captured by the /// HTTPServer handler closure and read on each request. /// /// Both are held **strongly**, so a delegate that admitted a file for processing /// will process it — the three reads within a request can't disagree, and an -/// in-flight upload keeps the host's delegate alive until it unwinds. This matches -/// Android, which holds its `uploadDelegate` as a plain `val` for the same reason. +/// in-flight upload keeps the host's delegate alive until it unwinds. That lifetime +/// comes from the handler closure, which the listener retains for the server's +/// lifetime; it therefore holds just as well on the paths that take the client +/// alone rather than the whole context. This matches Android, which holds its +/// `uploadDelegate` as a plain `val` for the same reason. /// /// Strong is safe *given* `EditorViewController` now owns `mediaUploadDelegate` /// strongly too — but be exact about what that trades away. Weak here did break one @@ -536,16 +540,22 @@ enum UploadError: Error, LocalizedError { /// vanishing mid-request — which is the failure that was actually being hit. /// /// A `struct`, so it is implicitly `Sendable`: `MediaUploadDelegate` is a `Sendable` -/// protocol and `DefaultMediaUploader` is `@unchecked Sendable`. +/// protocol and `InternalMediaClient` is `@unchecked Sendable`. private struct UploadContext: Sendable { let uploadDelegate: (any MediaUploadDelegate)? - let defaultUploader: DefaultMediaUploader? + let internalClient: InternalMediaClient? } -// MARK: - Default Media Uploader +// MARK: - Internal Media Client -/// Uploads files to the WordPress REST API using site credentials from EditorConfiguration. -class DefaultMediaUploader: @unchecked Sendable { +/// GutenbergKit's own client for the configured site, built from the site credentials +/// in `EditorConfiguration`. +/// +/// Not an implementation of any host-facing protocol — it is the thing that actually +/// performs GutenbergKit's media requests. It delivers uploads the host did not take +/// over, and relays the editor's media deletes: the editor only ever asks to delete +/// `/wp/v2/media/` on the configured site, so that is where the relay sends it. +class InternalMediaClient: @unchecked Sendable { private let httpClient: EditorHTTPClientProtocol private let siteApiRoot: URL private let siteApiNamespace: String? diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 6e2c6af1d..0a3294326 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -103,8 +103,8 @@ struct MediaUploadServerTests { @Test("routes /upload with a query string and relays the query") func uploadWithQueryString() async throws { let delegate = ProcessOnlyDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, internalClient: mockUploader) defer { server.stop() } // `@wordpress/media-utils` uploads to `/wp/v2/media?_embed=wp:featuredmedia`, @@ -141,8 +141,8 @@ struct MediaUploadServerTests { // Exercised through the delete relay because every response `relayResponse` // handles — WordPress's own included — carries a `Content-Type`, so this is // the ordinary path rather than an edge case. - let uploader = ContentTypeDeleteUploader() - let server = try await MediaUploadServer.start(defaultUploader: uploader) + let uploader = ContentTypeDeleteClient() + let server = try await MediaUploadServer.start(internalClient: uploader) defer { server.stop() } let url = URL(string: "http://127.0.0.1:\(server.port)/media/42?force=true")! @@ -194,8 +194,8 @@ struct MediaUploadServerTests { @Test("uses passthrough when delegate does not modify file") func delegatePassthrough() async throws { let delegate = ProcessOnlyDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -227,8 +227,8 @@ struct MediaUploadServerTests { @Test("skips processing and the temp copy when the delegate declines by metadata") func delegateDeclinesByMetadata() async throws { let delegate = DeclineByMetadataDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -255,8 +255,8 @@ struct MediaUploadServerTests { @Test("forwards the delegate's processed metadata to the uploader") func processedMetadataForwarded() async throws { let delegate = ResizingDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -281,8 +281,8 @@ struct MediaUploadServerTests { @Test("deletes the delegate's processed file after upload") func deletesProcessedFile() async throws { let delegate = ResizingDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploadDelegate: delegate, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -455,10 +455,10 @@ struct MediaUploadServerTests { // Held weakly, a host that dropped its reference changed the answer between // those reads: a file admitted for processing was forwarded unprocessed. The // host dropping it before the request is the same condition, deterministically. - let mockUploader = MockDefaultUploader() + let mockUploader = MockInternalMediaClient() var delegate: TranscodingDelegate? = TranscodingDelegate() weak let weakDelegate = delegate - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let server = try await MediaUploadServer.start(uploadDelegate: delegate, internalClient: mockUploader) defer { server.stop() } // Drop the host's only strong reference. Under the documented contract the @@ -498,7 +498,7 @@ struct MediaUploadServerTests { // MARK: - Streaming Multipart Body Tests -@Suite("DefaultMediaUploader streaming multipart body") +@Suite("InternalMediaClient streaming multipart body") struct MultipartBodyStreamTests { @Test("streaming output matches in-memory multipart format") @@ -521,7 +521,7 @@ struct MultipartBodyStreamTests { expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) // Build streaming output. - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType, extraFields: [] ) #expect(contentLength == expected.count) @@ -538,7 +538,7 @@ struct MultipartBodyStreamTests { // Craft a filename, field name, and MIME type that each try to smuggle a CRLF // and a fake header into the body relayed to WordPress. - let (stream, _) = try DefaultMediaUploader.multipartBodyStream( + let (stream, _) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: "boundary", filename: "evil\"\r\nX-Injected-File: 1.jpg", @@ -573,7 +573,7 @@ struct MultipartBodyStreamTests { expected.append(fileContent) expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType, extraFields: [("post", Data("123".utf8))] ) @@ -605,7 +605,7 @@ struct MultipartBodyStreamTests { expected.append(fileContent) expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType, extraFields: [("blob", binaryValue)] ) @@ -621,7 +621,7 @@ struct MultipartBodyStreamTests { try fileContent.write(to: tempFile) defer { try? FileManager.default.removeItem(at: tempFile) } - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: "boundary", filename: "big.bin", mimeType: "application/octet-stream", extraFields: [] ) @@ -645,7 +645,7 @@ struct MultipartBodyStreamTests { let preamble = Data("PREAMBLE".utf8) let epilogue = Data("EPILOGUE".utf8) - let ok = DefaultMediaUploader.writeMultipartBody( + let ok = InternalMediaClient.writeMultipartBody( fileHandle: fileHandle, fileSize: fileContent.count, preamble: preamble, epilogue: epilogue, to: output ) @@ -672,7 +672,7 @@ struct MultipartBodyStreamTests { let preamble = Data("PREAMBLE".utf8) let epilogue = Data("EPILOGUE".utf8) // Claim the file is larger than it is, as if it shrank after being measured. - let ok = DefaultMediaUploader.writeMultipartBody( + let ok = InternalMediaClient.writeMultipartBody( fileHandle: fileHandle, fileSize: fileContent.count + 100, preamble: preamble, epilogue: epilogue, to: output ) @@ -685,17 +685,17 @@ struct MultipartBodyStreamTests { } } -// MARK: - DefaultMediaUploader Relay Tests +// MARK: - InternalMediaClient Relay Tests -@Suite("DefaultMediaUploader relay") -struct DefaultMediaUploaderRelayTests { +@Suite("InternalMediaClient relay") +struct InternalMediaClientRelayTests { @Test("relays a non-2xx WordPress response instead of throwing") func relaysErrorResponseVerbatim() async throws { // A WordPress REST error body, returned with a non-2xx status. let errorBody = Data(#"{"code":"rest_cannot_create","message":"Sorry, you are not allowed to upload this file type."}"#.utf8) let client = RelayStubHTTPClient(statusCode: 403, body: errorBody) - let uploader = DefaultMediaUploader(httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) + let uploader = InternalMediaClient(httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("relay-\(UUID().uuidString).jpg") try Data("fake image".utf8).write(to: tempFile) @@ -722,7 +722,7 @@ struct DefaultMediaUploaderRelayTests { body: Data(#"{"code":"rest_upload_error"}"#.utf8), headerFields: ["x-wp-upload-attachment-id": "4242"] ) - let uploader = DefaultMediaUploader( + let uploader = InternalMediaClient( httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent( @@ -745,7 +745,7 @@ struct DefaultMediaUploaderRelayTests { body: Data("{}".utf8), headerFields: ["X-Powered-By": "PHP/8.2", "Set-Cookie": "session=secret"] ) - let uploader = DefaultMediaUploader( + let uploader = InternalMediaClient( httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent( @@ -763,7 +763,7 @@ struct DefaultMediaUploaderRelayTests { @Test("deletes an attachment, carrying the namespace and force query") func deletesAttachment() async throws { let client = URLCapturingHTTPClient() - let uploader = DefaultMediaUploader( + let uploader = InternalMediaClient( httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json")!, siteApiNamespace: ["sites/123"] @@ -779,7 +779,7 @@ struct DefaultMediaUploaderRelayTests { @Test("carries the namespace and request query through to the media endpoint") func forwardsNamespaceAndQuery() async throws { let client = URLCapturingHTTPClient() - let uploader = DefaultMediaUploader( + let uploader = InternalMediaClient( httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json")!, siteApiNamespace: ["sites/123"] @@ -802,7 +802,7 @@ struct DefaultMediaUploaderRelayTests { /// An HTTP client whose `performRaw` relays a canned response without validating /// status, while `perform` throws on a non-2xx — mirroring the real -/// `EditorHTTPClient`. Lets a test prove `DefaultMediaUploader` routes uploads +/// `EditorHTTPClient`. Lets a test prove `InternalMediaClient` routes uploads /// through `performRaw` (relay) rather than `perform` (throw). private struct RelayStubHTTPClient: EditorHTTPClientProtocol { let statusCode: Int @@ -962,7 +962,7 @@ private final class ResizingDelegate: MediaUploadDelegate, @unchecked Sendable { } } -private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendable { +private final class MockInternalMediaClient: InternalMediaClient, @unchecked Sendable { private let lock = NSLock() private var _uploadCalled = false private var _passthroughUploadCalled = false @@ -1004,9 +1004,9 @@ private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendab } } -/// A default uploader whose delete response carries its own `Content-Type`, so the +/// An internal media client whose delete response carries its own `Content-Type`, so the /// relay must override the JSON default rather than emit the header twice. -private final class ContentTypeDeleteUploader: DefaultMediaUploader, @unchecked Sendable { +private final class ContentTypeDeleteClient: InternalMediaClient, @unchecked Sendable { init() { super.init(httpClient: MockHTTPClient(), siteApiRoot: URL(string: "https://example.com/wp-json/")!) }