Skip to content

feat!: remove MediaUploadDelegate.uploadFile #629

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

Open
jkmassel wants to merge 2 commits into from
Open

feat!: remove MediaUploadDelegate.uploadFile #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bcc210
feat!: remove MediaUploadDelegate.uploadFile
jkmassel Sep 5, 2026
d8b85e2
docs: correct the handlesFile contract and the delegate's stale uploa…
jkmassel Sep 9, 2026
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
10 changes: 6 additions & 4 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 @@ -113,8 +113,10 @@ class GutenbergView : FrameLayout {
var requestInterceptor: GutenbergRequestInterceptor = DefaultGutenbergRequestInterceptor()

/**
* Optional delegate for customizing media upload behavior (resize, transcode,
* custom upload).
* Optional delegate for transforming media before upload (resize, transcode,
* strip EXIF).
*
* To perform the upload yourself, set [mediaUploader] instead.
*
* Provide this **before the editor loads** — typically right after
* construction (e.g. in the `AndroidView` factory). It is captured once, when
Expand All @@ -136,8 +138,8 @@ class GutenbergView : FrameLayout {
* 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.
* A [mediaUploadDelegate] can still transform the file first; only delivery moves
* to the uploader.
*/
var mediaUploader: MediaUploader? = null
set(value) {
Expand Down
67 changes: 23 additions & 44 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 @@ -33,8 +33,11 @@ import okio.source
* so every consumer — image sub-sizes, attachment links, error notices —
* behaves identically to a non-native upload.
*/
class MediaUploadResponse(
/** The HTTP status code WordPress (or the host's upload service) returned. */
internal class MediaUploadResponse(
/**
* The HTTP status code WordPress returned, or 201 for an upload a
* [MediaUploader] delivered.
*/
val statusCode: Int,
/**
* The raw response body — a WordPress REST attachment on success, or a
Expand Down Expand Up @@ -70,23 +73,30 @@ sealed class ProcessedProxyFile {
}

/**
* Interface for customizing media upload behavior.
* Transforms media before GutenbergKit delivers it.
*
* A delegate only changes *bytes* — GutenbergKit still uploads the result to the
* configured site and owns the whole lifecycle (retries, cleanup). Because it never
* performs the upload itself, it cannot deliver media to the wrong place. Set
* [GutenbergView.mediaUploadDelegate] to resize images, transcode video, strip EXIF,
* etc.
*
* The native host app can provide an implementation to resize images,
* transcode video, or use its own upload service.
* This is the safe, common extension point: most hosts want only this. To perform the
* upload yourself, implement [MediaUploader] instead.
*/
interface MediaUploadDelegate {
/**
* Whether this delegate might handle a file with the given metadata — either
* processing it ([processFile]) or uploading it itself ([uploadFile]).
* Whether this delegate might transform a file with the given metadata.
*
* A cheap, metadata-only gate the server consults *before* materializing the
* upload to a temp file. Return false to decline a file by type — e.g. an
* image-only delegate returning false for a video — so the server forwards
* the original upload to WordPress without first copying a file the delegate
* won't touch. Because it gates the temp-file copy needed by *both*
* [processFile] and [uploadFile], return true for any file the delegate will
* either process or upload itself.
* won't touch.
*
* With a [MediaUploader] set this can't decline the upload itself — an uploader
* delivers every file, so there is no passthrough to fall to — but it still gates
* [processFile]: a declined file reaches the uploader unprocessed.
*
* Defaults to true: every file is materialized and the full pipeline runs. A
* true here is not a commitment — [processFile] may still return
Expand All @@ -103,30 +113,6 @@ interface MediaUploadDelegate {
* stores it with the correct extension and type.
*/
suspend fun processFile(file: File, mimeType: String, filename: String): ProcessedProxyFile = ProcessedProxyFile.Original

/**
* 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 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.
*/
// No ReplaceWith: it takes a replacement *expression* the IDE substitutes for the
// call, and there is none that means "implement a different interface" — the
// quick-fix would drop the arguments and leave a type name where a
// MediaUploadResponse? was expected. The message carries the guidance instead.
@Deprecated(
"Implement MediaUploader instead — it owns the upload's retries and receives the editor's form fields."
)
suspend fun uploadFile(file: File, mimeType: String, filename: String): MediaUploadResponse? = null
}

/**
Expand Down Expand Up @@ -380,8 +366,8 @@ internal class MediaUploadServer(

// 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).
// 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 and the gate can't decline the upload outright. It
// still decides whether processFile runs, though — a declined file is handed to
Expand Down Expand Up @@ -594,21 +580,14 @@ internal class MediaUploadServer(
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)
}

// Unmodified — forward the original request body directly, skipping
// multipart re-encoding.
if (processed is ProcessedProxyFile.Original) {
return UploadResult.Passthrough
}

val result = internalClient?.upload(targetFile, targetMimeType, targetFilename, extraParts, query)
?: error("No upload delegate or internal media client configured")
?: error("No media uploader 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
49 changes: 13 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 @@ -245,11 +245,11 @@ class MediaUploadServerTest {
}

@Test
fun `an uploader takes precedence over the deprecated uploadFile hook`() {
// Both set: the uploader owns delivery and the deprecated hook must not run.
// The delegate still processes — only delivery moves to the uploader.
fun `a delegate still processes the file an uploader delivers`() {
// With both set, the delegate still processes — only delivery moves to
// the uploader.
val uploader = RecordingUploader()
val delegate = MockUploadDelegate()
val delegate = ProcessOnlyDelegate()
val client = MockInternalMediaClient()
server.stop()
server = MediaUploadServer(
Expand All @@ -270,7 +270,6 @@ class MediaUploadServerTest {
)

assertNotNull(uploader.received)
assertFalse(delegate.uploadFileCalled)
assertTrue(delegate.processFileCalled)
assertFalse(client.uploadCalled)
}
Expand Down Expand Up @@ -343,10 +342,11 @@ class MediaUploadServerTest {
// MARK: - Upload with delegate

@Test
fun `calls delegate processFile and uploadFile`() {
val delegate = MockUploadDelegate()
fun `processes with the delegate, then delivers through the internal client`() {
val delegate = TranscodingDelegate()
val client = MockInternalMediaClient()
server.stop()
server = MediaUploadServer(uploadDelegate = delegate, internalClient = null, cacheDir = tempFolder.root)
server = MediaUploadServer(uploadDelegate = delegate, internalClient = client, cacheDir = tempFolder.root)

val boundary = "test-boundary-123"
val body = buildMultipartBody(boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray())
Expand All @@ -362,16 +362,14 @@ class MediaUploadServerTest {
)

assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201"))
assertTrue(delegate.processFileCalled)
assertTrue(delegate.uploadFileCalled)
assertEquals("image/jpeg", delegate.lastMimeType)
assertEquals("photo.jpg", delegate.lastFilename)
// The delegate only transforms; GutenbergKit performs the upload.
assertTrue(client.uploadCalled)

// The server relays WordPress's raw response body verbatim.
val json = JsonParser.parseString(response.body).asJsonObject
assertEquals(42, json.get("id").asInt)
assertEquals("https://example.com/photo.jpg", json.get("source_url").asString)
assertEquals("image", json.get("media_type").asString)
assertEquals(99, json.get("id").asInt)
assertEquals("https://example.com/doc.pdf", json.get("source_url").asString)
assertEquals("file", json.get("media_type").asString)
}

@Test
Expand Down Expand Up @@ -896,27 +894,6 @@ class MediaUploadServerTest {

// MARK: - Mocks

private class MockUploadDelegate : MediaUploadDelegate {
@Volatile var processFileCalled = false
@Volatile var uploadFileCalled = false
@Volatile var lastMimeType: String? = null
@Volatile var lastFilename: String? = null

override suspend fun processFile(file: File, mimeType: String, filename: String): ProcessedProxyFile {
processFileCalled = true
lastMimeType = mimeType
return ProcessedProxyFile.Original
}

@Suppress("OVERRIDE_DEPRECATION")
override suspend fun uploadFile(file: File, mimeType: String, filename: String): MediaUploadResponse? {
uploadFileCalled = true
lastFilename = filename
val json = """{"id":42,"source_url":"https://example.com/photo.jpg","media_type":"image"}"""
return MediaUploadResponse(201, json.toByteArray())
}
}

private class ProcessOnlyDelegate : MediaUploadDelegate {
@Volatile var processFileCalled = false

Expand Down
2 changes: 1 addition & 1 deletion android/app/src/main/java/com/example/gutenbergkit/DemoMediaUploadDelegate.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import java.io.IOException
/**
* Demo media upload delegate that resizes images to a maximum dimension of 2000px.
*
* Only overrides [processFile] — [uploadFile] returns null so the default uploader is used.
* Only transforms the file; GutenbergKit performs the upload.
*/
class DemoMediaUploadDelegate : MediaUploadDelegate {
companion object {
Expand Down
8 changes: 5 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 @@ -109,7 +109,9 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro
/// take effect, so its setter traps if written.
private var hasStartedLoading = false

/// Delegate for customizing media file processing and upload behavior.
/// Delegate for transforming media before upload — resize, transcode, strip EXIF.
///
/// To perform the upload yourself, set ``mediaUploader`` instead.
///
/// Provide this **before the editor loads** — typically right after `init`, the
/// same way the rest of the editor configuration is supplied. It is captured
Expand Down Expand Up @@ -147,8 +149,8 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro
/// 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.
/// A ``mediaUploadDelegate`` can still transform the file first; only delivery
/// moves to the uploader.
public var mediaUploader: (any MediaUploader)? {
didSet {
precondition(!hasStartedLoading, Self.lateMediaAssignmentMessage("mediaUploader"))
Expand Down
60 changes: 22 additions & 38 deletions ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import Foundation
/// or WordPress REST error object (on failure) it would get from a direct
/// upload, so every consumer — image sub-sizes, attachment links, error notices —
/// behaves identically to a non-native upload.
public struct MediaUploadResponse: Sendable {
/// The HTTP status code WordPress (or the host's upload service) returned.
public let statusCode: Int
struct MediaUploadResponse: Sendable {
/// The HTTP status code WordPress returned, or 201 for an upload a
/// ``MediaUploader`` delivered.
let statusCode: Int

/// The raw response body — a WordPress REST attachment on success, or a
/// WordPress REST error object (`{ "code", "message", "data" }`) on failure.
public let body: Data
let body: Data

/// The response headers to relay to the editor.
///
Expand All @@ -22,9 +23,9 @@ public struct MediaUploadResponse: Sendable {
/// metadata generation fataled, and the editor's api-fetch middleware reads
/// it to retry `post-process` and clean up the orphan. Dropping it turns a
/// recoverable upload into a permanent failure.
public let headers: [String: String]
let headers: [String: String]

public init(statusCode: Int, body: Data, headers: [String: String] = [:]) {
init(statusCode: Int, body: Data, headers: [String: String] = [:]) {
self.statusCode = statusCode
self.body = body
self.headers = headers
Expand All @@ -44,23 +45,28 @@ public enum ProcessedProxyFile: Sendable {
case processed(URL, mimeType: String, filename: String)
}

/// Protocol for customizing media upload behavior.
/// Transforms media before GutenbergKit delivers it.
///
/// The native host app can provide an implementation to resize images,
/// transcode video, or use its own upload service. Default implementations
/// pass files through unchanged and upload via the WordPress REST API.
/// A delegate only changes *bytes* — GutenbergKit still uploads the result to the
/// configured site and owns the whole lifecycle (retries, cleanup). Because it never
/// performs the upload itself, it cannot deliver media to the wrong place. Set
/// ``EditorViewController/mediaUploadDelegate`` to resize images, transcode video,
/// strip EXIF, etc.
///
/// This is the safe, common extension point: most hosts want only this. To perform
/// the upload yourself, conform to ``MediaUploader`` instead.
public protocol MediaUploadDelegate: AnyObject, Sendable {
/// Whether this delegate might handle a file with the given metadata — either
/// processing it (``processFile(at:mimeType:filename:)``) or uploading it
/// itself (``uploadFile(at:mimeType:filename:)``).
/// Whether this delegate might transform a file with the given metadata.
///
/// A cheap, metadata-only gate the server consults *before* materializing the
/// upload to a temp file. Return `false` to decline a file by type — e.g. an
/// image-only delegate returning `false` for a video — so the server forwards
/// the original upload to WordPress without first copying a file the delegate
/// won't touch. Because it gates the temp-file copy needed by *both*
/// `processFile` and `uploadFile`, return `true` for any file the delegate
/// will either process or upload itself.
/// won't touch.
///
/// With a ``MediaUploader`` set this can't decline the upload itself — an
/// uploader delivers every file, so there is no passthrough to fall to — but it
/// still gates `processFile`: a declined file reaches the uploader unprocessed.
///
/// Defaults to `true`: every file is materialized and the full pipeline runs.
/// A `true` here is not a commitment — `processFile` may still return
Expand All @@ -74,23 +80,6 @@ public protocol MediaUploadDelegate: AnyObject, Sendable {
/// file and its metadata. When the format changes, report the new mimeType
/// and filename so WordPress stores it with the correct extension and type.
func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile

/// 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 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?
}

/// Default implementations.
Expand All @@ -102,11 +91,6 @@ extension MediaUploadDelegate {
public func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile {
.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.
Expand Down
Loading
Loading