refactor(storage): adopt swift-openapi-generator for the HTTP layer#1093
Draft
grdsdev wants to merge 32 commits into
Draft
refactor(storage): adopt swift-openapi-generator for the HTTP layer#1093grdsdev wants to merge 32 commits into
grdsdev wants to merge 32 commits into
Conversation
Design for replacing hand-written HTTP request/response building in StorageBucketApi/StorageFileApi with a generated OpenAPI client, sourced from supabase/storage#1215's spec-quality fixes, kept fully internal behind the existing public API.
…s 1-2) Bite-sized task plan covering spec vendoring, codegen tooling, the StorageOpenAPITransport bridge, and migrating all six StorageBucketApi methods to the generated client. Milestones 3-4 (JSON object endpoints, multipart/binary) get their own follow-up plans per the design doc.
…dored spec The docs:export output declares openapi: 3.0.3 but used OpenAPI 3.1's type-array nullable syntax (type: ["null", "integer"]) in 4 places (bucketSchema.file_size_limit, .allowed_mime_types, and two properties in the object/sign response schema). That's invalid for a 3.0.3 document and swift-openapi-generator correctly rejects it. Converted to the equivalent valid 3.0.3 form (type + nullable: true) with no semantic change.
Living tracking doc for spec bugs found while adopting swift-openapi-generator, to file back upstream once confirmed against master.
The previous fix converted the 4 type-array occurrences but missed a
second OpenAPI 3.1-only pattern: 12 properties used
anyOf: [<schema>, {"type": "null"}], which is also invalid under the
document's declared 3.0.3 version. swift-openapi-generator failed with
"Cannot initialize JSONType from invalid String value null" on
objectSchema.properties.id and the /object/copy response schema.
Converted each to the equivalent 3.0.3 form: merge the non-null
branch's keywords onto the parent and set nullable: true. No semantic
change.
Adds an isolated tools/openapi-generator package (kept out of the main
graph to avoid pulling swift-openapi-generator's own dependencies into
consumer builds) plus scripts/generate-storage-openapi.sh to regenerate
Sources/Storage/Generated/{Types,Client}.swift from the vendored spec.
Wires swift-openapi-runtime into the root Package.swift as a real
Storage dependency since the generated code imports it.
Renamed the existing hand-written Sources/Storage/Types.swift to
StorageTypes.swift: SwiftPM names build products by file basename
regardless of directory, so it collided with the generator's
Generated/Types.swift (a reserved output filename that can't be
renamed via config) and failed the build with "multiple producers".
Generated Components.Schemas.bucketSchema spells the JSON `public` and
`file_size_limit` fields as `_public: Swift.Bool?` and
`file_size_limit: Swift.Int?`.
anyOf-with-null nullable syntax (12 sites) and the bucketUpdate required/properties mismatch warning, both found running Task 2 of the storage OpenAPI generator plan.
Task 2 renamed Sources/Storage/Types.swift to StorageTypes.swift to resolve a SwiftPM basename collision with the generator's reserved Types.swift output. Task 5's instructions still referenced the old path.
Adds Bucket.init(fromGenerated:) bridging Components.Schemas.bucketSchema to the existing public Bucket type, and switches listBuckets() to call openAPIClient.bucketList(...) instead of hand-built HTTP request/response code. Also strips the Accept header that swift-openapi-generator's client always sets in StorageOpenAPITransport, since none of the hand-written implementations being migrated ever sent one and wire behavior must stay byte-identical (testListBuckets' curl snapshot would otherwise gain an extra header).
Bare ISO8601DateFormatter() (no fractional-seconds support) fails to parse the Storage API's millisecond-precision timestamps, silently falling back to the 1970 epoch for createdAt/updatedAt on every real response. Reuse the existing String.date helper (Helpers module), which already handles both fractional and non-fractional ISO8601.
…API client file_size_limit is generated as a value1(Int)/value2(String) struct rather than an anyOf enum; only value2 is ever used since BucketOptions.fileSizeLimit is already a plain String. Configures the generated client's JSON encoder with sortedKeys so request bodies stay deterministic across migrated operations. Also excludes Sources/Storage/Generated from scripts/format.sh, since that directory is machine-generated and must not be hand-reformatted.
The generated OpenAPI client appends `; charset=utf-8` to Content-Type on every JSON-body request, but the hand-written path (and every other migrated bucket method) always sent plain `application/json`. StorageOpenAPITransport already normalizes the generator-injected Accept header for wire compatibility during the migration; apply the same treatment to Content-Type instead of accepting the drift in snapshots.
…body
PUT /bucket/{bucketId}'s request body declared a top-level anyOf
(at-least-one-of public/file_size_limit/allowed_mime_types) alongside
its properties and minProperties: 1 — already fully implied by
minProperties: 1 given the schema has no other fields. Because a
top-level anyOf makes swift-openapi-generator treat the schema as
anyOf-driven, it silently dropped the co-located properties, producing
a generated body type with zero usable fields. Deleting the redundant
anyOf (keeping minProperties + properties) regenerates a normal,
populated body type with no semantic change server-side.
Found while attempting Task 10 of
docs/superpowers/plans/2026-07-08-storage-openapi-generator.md.
RealtimeIntegrationTests was missing the XCTSkipUnless(INTEGRATION_TESTS) guard present in every sibling integration test file, so a plain swift test ran it unconditionally. Most tests failed fast with no server, but testPostgresAllChanges hung forever: it drives a manual TestClock that backs withTimeout()'s subscribe-timeout logic but never advances it for this test's subscribe call, so with no live server and a clock that never moves on its own, the subscribe never resolves and never times out. Restoring the guard exposed a second bug: tearDown() force-unwrapped client/client2, which are nil when setUp() skips before assigning them, crashing every run. Switched to optional chaining so tearDown() is a no-op when setUp() skipped.
…API client Mirrors createBucket's Task 9 pattern, mapping BucketOptions onto the now-populated Operations.bucketUpdate.Input.Body.jsonPayload (unblocked by 7970393's spec fix). testUpdateBucket's snapshot needed two corrections, both pre-existing inaccuracies exposed by the generated client actually honoring the spec: - Request body: the old hand-written client sent id/name on update, fields the real PUT /bucket/{bucketId} endpoint never accepts (only public/file_size_limit/allowed_mime_types, per the vendored spec). The generated body type has no id/name fields to set, so the request now correctly sends only {"public":true}. - Mock response: the endpoint's actual 200 response is {"message": "..."}, not a full bucket object. The old client ignored the response body entirely, so the mock's inaccurate fixture never mattered; the generated client decodes it per spec and requires the real shape. Also removed BucketParameters, dead now that both createBucket and updateBucket use the generated client.
…nses StorageOpenAPITransport's execute closure reused StorageApi.executeRequest, which throws StorageError/HTTPError directly on non-2xx. OpenAPIRuntime's UniversalClient.send wraps any non-ClientError thrown by the transport into a ClientError, so bucket API errors regressed from bare StorageError to ClientError(underlyingError: StorageError). Give the OpenAPI transport path its own executeRequestWithoutStatusCheck that shares header-merging with executeRequest but never throws on non-2xx, matching the design doc's "transport doesn't throw, facade maps errors" architecture. StorageBucketApi's six migrated methods now switch on the generated Output's .forbidden/.clientError/.undocumented cases and build a StorageError from the real decoded errorSchema body (or raw bytes for undocumented statuses), instead of a generic placeholder message. Adds a regression test proving the thrown error is a bare StorageError (not ClientError) with the real statusCode/message/error, plus two small StorageOpenAPITransport tests for the Accept-header-drop and Content-Type-charset-normalization branches.
Updated after the error-handling fix (f1e0aef) changed this transport's contract from throwing on non-2xx to always returning the raw response.
Milestone 4 of the storage OpenAPI generator adoption. Requires patching the vendored spec to add a multipart requestBody (currently undeclared) before the generated client can send anything for these operations — flagged as a de-risking spike before any facade changes.
…PI client Covers upload/update/uploadToSignedURL (Milestone 4, multipart write path). Grounded in a throwaway codegen spike confirming swift-openapi-generator handles the multipart shape needed once the vendored spec's undeclared request body is patched. download() (binary response) stays out of scope for its own follow-up plan.
objectUpload/objectUploadUpdate/objectUploadSigned had no declared request body at all (Fastify's multipart handling isn't schema- validated the same way as JSON bodies). Added a multipart/form-data requestBody (cacheControl, optional metadata, file) to all three so swift-openapi-generator produces a usable typed Input.body. Confirmed the generated client hardcodes Content-Type: application/octet-stream for the typed .file case with no override — the file part must be built via the .undocumented(MultipartRawPart) escape hatch to send the real MIME type, per the implementation plan.
Migrates StorageFileApi._uploadOrUpdate to call the generated objectUpload/objectUploadUpdate operations instead of the hand-written MultipartFormData/execute() path. Adds a StorageApi task-local (extraHeadersForCurrentRequest) so per-call headers (x-upsert, Duplex, Cache-Control, options.headers) can flow through the generated Client, which only exposes `accept` on its per-operation Input.headers. Also wires the OpenAPI client's multipart boundary generator to the existing DEBUG-only testingBoundary hook so snapshot tests stay deterministic.
Migrates StorageFileApi._uploadToSignedURL to call the generated objectUploadSigned operation instead of the hand-written MultipartFormData/execute() path, reusing the shared file-part builder and StorageApi.extraHeadersForCurrentRequest task-local established for upload/update. The generated success response only has a Key field (no Id), matching SignedURLUploadResponse's existing shape, and this operation has no forbidden case in its Output (only ok, clientError, undocumented).
…OpenAPI client MultipartFormData's only callers (upload/update and uploadToSignedURL) were migrated to OpenAPIRuntime.MultipartRawPart in prior tasks. Also removes two now-dead code paths that referenced it: FileUpload.encode(to:withPath:options:) and the Helpers.HTTPRequest(formData:) initializer, neither of which had any remaining call sites.
…rtFormData The deprecated FormData type's availability attribute pointed consumers to MultipartFormData via renamed:, but that type was deleted in cea2c1b (superseded by the generated OpenAPI client's multipart support). renamed: is a string literal, not compiler-validated, so the build stayed clean, but the message pointed at a symbol that no longer exists in the package.
_uploadToSignedURL dropped the Cache-Control header during the OpenAPI migration, unlike _uploadOrUpdate which correctly preserved it via _uploadExtraHeaders. Pre-migration, both paths shared the same header logic. Also adds missing coverage: a POST upload() test (previously only update()/uploadToSignedURL() had snapshot tests) and a concurrency test proving the TaskLocal used to thread per-call headers through the OpenAPI transport doesn't leak between concurrent calls.
The @available(renamed:) attribute pointed to a nonexistent updateToSignedURL(_:token:data:options:); the real method is uploadToSignedURL.
Coverage Report for CI Build 28956968023Coverage decreased (-36.9%) to 46.475%Details
Uncovered Changes
Coverage Regressions20 previously-covered lines in 2 files lost coverage.
Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces hand-written HTTP request/response plumbing in
StorageBucketApiand the multipart-bearing methods ofStorageFileApiwith a generated OpenAPI client, sourced from supabase/storage#1215's spec-quality fixes. Zero public API or behavior change.Design docs / plans:
What changed
Milestones 1-2 — bucket API + plumbing:
docs/superpowers/storage-openapi-upstream-issues.mdfor filing upstream)tools/openapi-generator,scripts/generate-storage-openapi.sh) that keepsswift-openapi-generatoritself out of the main dependency graphStorageOpenAPITransportbridges the generated client onto the existingStorageHTTPSession/header-merging pipelineStorageBucketApimethods (listBuckets,getBucket,createBucket,updateBucket,emptyBucket,deleteBucket) migratedClientError-wrapped errors instead of bareStorageErrorMilestone 4 — upload/update/uploadToSignedURL (multipart):
multipart/form-datarequest body (previously undeclared entirely) for the three upload operationsupload/update/uploadToSignedURLoff the hand-writtenMultipartFormDataencoder (now deleted, ~900 lines) onto the generated client's multipart supportTaskLocal-scoped per-call header mechanism for headers the generated client'sInputdoesn't expose a slot forOut of scope, follow-ups filed separately:
download()(binary response streaming) — its own follow-up planTest plan
xcodebuild test, all Storage/Supabase suites green)