Skip to content

refactor: rename DefaultMediaUploader to InternalMediaClient #627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jkmassel wants to merge 1 commit into from
Closed

refactor: rename DefaultMediaUploader to InternalMediaClient #627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -706,15 +706,15 @@ class GutenbergView : FrameLayout {
}

try {
val defaultUploader = DefaultMediaUploader(
val internalClient = InternalMediaClient(
httpClient = uploadHttpClient,
siteApiRoot = configuration.siteApiRoot,
authHeader = configuration.authHeader,
siteApiNamespace = configuration.siteApiNamespace.toList()
)
uploadServer = MediaUploadServer(
uploadDelegate = mediaUploadDelegate,
defaultUploader = defaultUploader,
internalClient = internalClient,
cacheDir = context.cacheDir,
scope = coroutineScope
)
Expand Down
33 changes: 20 additions & 13 deletions android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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/<id>` 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,
Expand Down
72 changes: 36 additions & 36 deletions android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
)
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -305,7 +305,7 @@ class MediaUploadServerTest {
server.stop()
server = MediaUploadServer(
uploadDelegate = null,
defaultUploader = null,
internalClient = null,
cacheDir = tempFolder.root,
ioDispatcher = Dispatchers.Unconfined
)
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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"}"""
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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"
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions ios/Sources/GutenbergKit/Sources/EditorViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading
Loading