-
Notifications
You must be signed in to change notification settings - Fork 125
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
[Generator] Integrate the new URI and String coders #226
Merged
Conversation
This file contains 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
… and properties can now be documented
czechboy0
added a commit
to apple/swift-openapi-runtime
that referenced
this pull request
Aug 30, 2023
[Runtime] Integrate the new URI and String coders ### Motivation See details in the generator PR on motivation (apple/swift-openapi-generator#226). ### Modifications - Deprecated `_AutoLosslessStringConvertible` and `_StringConvertible` 🎉 - Deprecated all `*Text` helper functions on `Converter`, it was a convoluted mix of `URI` and `String`, so they all need to go away. - Added `*URI` and `*String` helper functions for the appropriate parameters/bodies. Those use the new URI/String coders under the hood. - Note: we now manipulate the `Request.query` property more directly, instead of going through `URLQueryItem`. ### Result Now we fully rely on `Codable` and a combination of system and custom encoder/decoders, which allows us to much more easily support Foundation types (such as Date), generated types (such as string enums), and primitive types - all the same way. While this might not be fully evident in this PR, this refactoring actually makes it possible for us to support much more of the OpenAPI specification. ### Test Plan Deprecated tests of the `*Text` helpers, introduced new tests for the new helpers. Reviewed by: glbrntt Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (api breakage) - Build finished. ✔︎ pull request validation (docc test) - Build finished. ✔︎ pull request validation (integration test) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. #45
glbrntt
approved these changes
Aug 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Depends on runtime changes from apple/swift-openapi-runtime#45.
Up until now, we relied on a series of marker and helper protocols
_StringConvertible
and_AutoLosslessStringConvertible
to handle converting between various types and their string representation.This has been very manual and required a non-trivial amount of work to support any extra type, especially
Date
and generated string enums.Well, turns out this was an unnecessarily difficult way to approach the problem - we had a better solution available for a long time -
Codable
.Since all the generated types and all the built-in types we reference are already
Codable
, there is no need to reinvent a way to serialize and deserialize types, and we should just embrace it.While a JSON encoder and decoder already exists in Foundation, we didn't have one handy for encoding to and from URIs (used by headers, query and path parameters), and raw string representation (using
LosslessStringConvertible
). We created those in the runtime library in PRs apple/swift-openapi-runtime#44 and apple/swift-openapi-runtime#41, and integrated them into our helper functions (which got significantly simplified this way) in apple/swift-openapi-runtime#45.Out of scope of this PR, but this also opens the door to supporting URL form encoded bodies (#182), multipart (#36), and base64 (#11).
While this should be mostly invisible to our adopters, this refactoring creates space for implementing more complex features and overall simplifies our serialization story.
Modifications
Codable
.lineLength
to 120 on the formatter configuration, it was inconsistent with our.swift-format
file, and lead to the soundness script trying to update the reference files, but then the reference tests were failing. Since we're planning to sync these in Use the same .swift-format in the repo and in the generated code #40, this is a step closer to it, but it means that it's probably best to review this PR's diff with whitespace ignored.Result
Now the generated code uses the new helper functions, allowing us to delete all the deprecated helpers in 0.2.0.
Test Plan
Updated file-based reference, snippet, and unit tests.