Skip to content

refactor: rename DefaultMediaUploader to InternalMediaClient - #627

Open
jkmassel wants to merge 1 commit into
fix/media-upload-cancellation-checkfrom
refactor/internal-media-client
Open

refactor: rename DefaultMediaUploader to InternalMediaClient#627
jkmassel wants to merge 1 commit into
fix/media-upload-cancellation-checkfrom
refactor/internal-media-client

Conversation

@jkmassel

@jkmassel jkmassel commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Stacked on #626. Fourth of ten PRs splitting #621. No behavior change beyond error-message text.

What?

DefaultMediaUploaderInternalMediaClient on both platforms, and the defaultUploader parameters and properties that carry it.

Why?

The name reads as an implementation of a host-facing protocol — the "default" one, as against a host's. It is not. It is GutenbergKit's own HTTP client for the configured site: it performs the uploads no host took over, and it relays every media delete, because the editor only ever asks to delete /wp/v2/media/<id> on the configured site.

The name gets actively confusing in #628, which adds a MediaUploader protocol. Renaming first keeps that PR's diff about the new API rather than about disambiguating two similar names.

How?

Mechanical, ~21 sites on trunk plus tests, plus a sweep of the prose and error strings that used the retired vocabulary — including the UploadContext doc header and Android's three media-client messages. The host-facing docs still say "the default uploader" as a role: InternalMediaClient is internal on both platforms, so naming it in prose a host reads would be worse.

On iOS it also narrows two signatures: passthroughResponse and handleDelete took the whole UploadContext and touched only the client, so they now take it directly.

On the delete path that is more than tidiness — a deletion always relays to the configured site, never to a delegate. That was a convention the signature let you break; now the type won't. The three functions that keep the context genuinely need every field.

Android's server holds the client as a constructor property rather than threading a context, so it needs the rename only — and because its handleDelete is an instance method with the delegate in scope, the delete-path convention stays a convention there. The type-level guarantee is iOS-only.

Testing Instructions

  • swift test — host suite green (969 tests, 57 suites)
  • Android :Gutenberg:testDebugUnitTest green
  • iOS Simulator xcodebuild
  • SwiftLint + Detekt clean

@jkmassel jkmassel added [Type] Task Issues or PRs that have been broken down into an individual action to take Android iOS labels Sep 5, 2026
@jkmassel jkmassel self-assigned this Sep 5, 2026
@wpmobilebot

wpmobilebot commented Sep 5, 2026

Copy link
Copy Markdown

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/627")

Built from 23e3c85

@jkmassel
jkmassel force-pushed the refactor/internal-media-client branch 2 times, most recently from 1c97d57 to 56d21ec Compare September 8, 2026 20:55
@jkmassel
jkmassel marked this pull request as ready for review September 8, 2026 21:11
`DefaultMediaUploader` reads as an implementation of a host-facing
protocol — the "default" one, as against a host's. It is not. It is
GutenbergKit's own HTTP client for the configured site: it performs the
uploads no host took over, and it relays every media delete, because the
editor only ever asks to delete `/wp/v2/media/<id>` on the configured
site.

Rename it, and the `defaultUploader` parameters and properties that carry
it, on both platforms. Sweep the prose and error strings that used the
retired vocabulary too, including the `UploadContext` doc header and
Android's three media-client messages. The host-facing docs still say
"the default uploader" as a role: `InternalMediaClient` is internal on
both platforms, so naming it in prose a host reads would be worse.

On iOS this also narrows two signatures. `passthroughResponse` and
`handleDelete` took the whole `UploadContext` and touched only the client.
Pass it directly. On the delete path that is more than tidiness: a
deletion always relays to the configured site, never to a delegate. That
was a convention the signature let you break; now the type won't. The
three functions that keep the context genuinely need every field.

Android's server holds the client as a constructor property rather than
threading a context, so it needs the rename only — and because its
`handleDelete` is an instance method with the delegate in scope, the
delete-path convention stays a convention there. The type-level guarantee
is iOS-only.
@jkmassel
jkmassel force-pushed the refactor/internal-media-client branch from 56d21ec to 23e3c85 Compare September 8, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android iOS [Type] Task Issues or PRs that have been broken down into an individual action to take

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants