Skip to content

Support base64-encoded data #326

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.

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

Merged

Conversation

rnro
Copy link
Contributor

@rnro rnro commented Oct 6, 2023

This change accompanies apple/swift-openapi-runtime#55 and relies on it for the OpenAPIRuntime.Base64EncodedData type.

Motivation

OpenAPI supports base64-encoded data but to this point Swift OpenAPI Generator has not (#11).

Modifications

A data type specified as type: string, format: byte will now result in a generated type which is Codable and backed by a OpenAPIRuntime.Base64EncodedData type which knows how to encode and decode base64 data.

Result

Users will be able to specify request/response payloads as base64-encoded data which will be encoded and decoded transparently

Test Plan

Unit tested locally.

@rnro rnro changed the title Motivation Support base64-encoded data Oct 6, 2023
Motivation

OpenAPI supports base64-encoded data but to this point Swift OpenAPI Generator has not (apple#11).

Modifications

A data type specified as `type: string, format: byte` will now result in a generated type which is `Codable` and backed by a `OpenAPIRuntime.Base64EncodedData` type which knows how to encode and decode base64 data.

Result

Users will be able to specify request/response payloads as base64-encoded data which will be encoded and decoded transparently

Test Plan

Unit tested locally.
@rnro rnro force-pushed the base64_encoded_data_type_string_format_byte branch from ada956b to 6b59d6a Compare October 6, 2023 09:20
@czechboy0
Copy link
Contributor

@swift-server-bot add to allowlist

rnro added 2 commits October 9, 2023 13:36
`base64DataEncodingDecoding` enables interpretation of `type: string, format: byte` as base64-encoded data.
@czechboy0
Copy link
Contributor

@rnro As a last thing, can you add a base64 property to an object of your choice in the file-based reference tests? I'm a little paranoid that this PR is passing even though we haven't released the update runtime yet, which means we're not testing that it all builds and works together. Add a property, and make sure PetstoreConsumerTests are updated to also populate and verify the value both on client and server side. Otherwise lgtm.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last ask about testing, see my other comment.

@rnro
Copy link
Contributor Author

rnro commented Oct 10, 2023

Thanks for the review, I've added an endpoint to upload audio of your pet in base64-encoded binary format. I'm not sure if that's the best API design for an actual pet app but I think it's plausible to get the new code in there.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add tests to Test_Client and Test_Server that set and verify the value in Pet.genome, just to make sure it works end-to-end.

@czechboy0
Copy link
Contributor

Also, you can now update to 0.3.2 of the runtime library in Package.swift of swift-openapi-generator, all that should hopefully make the CI green on this PR.

@rnro rnro force-pushed the base64_encoded_data_type_string_format_byte branch from 6e36455 to 84ef6c6 Compare October 10, 2023 14:02
@rnro rnro force-pushed the base64_encoded_data_type_string_format_byte branch from 84ef6c6 to ab6dd25 Compare October 10, 2023 14:06
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you @rnro!

@czechboy0 czechboy0 enabled auto-merge (squash) October 10, 2023 14:24
@czechboy0 czechboy0 linked an issue Oct 10, 2023 that may be closed by this pull request
@czechboy0 czechboy0 merged commit a8db309 into apple:main Oct 10, 2023
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support base64 encoded data (type: string + format: byte)
2 participants