[Proposal] SOAR-0004: Streaming request and response bodies #254

czechboy0 · 2023-09-08T12:59:30Z

Motivation

See the proposal at https://github.com/czechboy0/swift-openapi-generator/blob/hd-soar-0004/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0004.md.

Modifications

N/A

Result

N/A

Test Plan

N/A

gjcairo · 2023-09-08T13:23:10Z

This proposal is now In Review, which will run until 15th September - feel free to leave your feedback either on this PR or on the corresponding Swift Forums post.

Thanks, and thanks @czechboy0 for the write-up!

…erator into hd-soar-0004

gjcairo · 2023-09-12T15:01:30Z

Overall really like the shape of things here, great job Honza 🥳 I've only got two pretty minor comments:

As a general rule, we tend to favour extending Bar with init(foo: Foo) when Foo can be converted into a Bar, instead of having Foo.asBar() methods. I wonder if we should make the following changes - have you thought about this?

HTTPBody.collectAsString(upTo maxBytes: Int) async throws -> String -> String.init(collecting httpBody: HTTPBody, upTo maxBytes: Int) async throws
HTTPBody.collectAsData(upTo maxBytes: Int) async throws -> Data -> Data.init(collecting httpBody: HTTPBody, upTo maxBytes: Int) async throws
StringProtocol's extension to add asBodyChunk -> HTTPBody.ByteChunk.init(_ string: String)

This one really is about the proposal itself and quite minor, but if collect(upTo: .max) is discouraged (as stated in the text), should we use it in examples in the proposal? Maybe we should steer clear of what we deem bad practices in sample code.

czechboy0 · 2023-09-12T15:07:30Z

Overall really like the shape of things here, great job Honza 🥳 I've only got two pretty minor comments:

As a general rule, we tend to favour extending Bar with init(foo: Foo) when Foo can be converted into a Bar, instead of having Foo.asBar() methods. I wonder if we should make the following changes - have you thought about this?

HTTPBody.collectAsString(upTo maxBytes: Int) async throws -> String -> String.init(collecting httpBody: HTTPBody, upTo maxBytes: Int) async throws

HTTPBody.collectAsData(upTo maxBytes: Int) async throws -> Data -> Data.init(collecting httpBody: HTTPBody, upTo maxBytes: Int) async throws

I quite like that. I guess it's less common for async initializers, but that shouldn't stop us. While it's a little less discoverable, I think the consistency with general Swift guidelines is more important here. I'll make that change.

StringProtocol's extension to add asBodyChunk -> HTTPBody.ByteChunk.init(_ string: String)

This is an internal, not public, method, it just showed up there as it's inlinable. I'll see later about changing it, but probably not before the review ends.

This one really is about the proposal itself and quite minor, but if collect(upTo: .max) is discouraged (as stated in the text), should we use it in examples in the proposal? Maybe we should steer clear of what we deem bad practices in sample code.

Ha - keeping me honest. Yes, I'll update it 🙂

Thanks, @gjcairo! 🙏

czechboy0 · 2023-09-12T20:56:35Z

@gjcairo I applied all your feedback, please take a look at v1.3: https://github.com/czechboy0/swift-openapi-generator/blob/hd-soar-0004/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0004.md

…names.

Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0004.md

gjcairo · 2023-09-15T09:59:25Z

The review period is now over, and the proposal has been accepted. SOAR-0004 is now Ready for Implementation.

czechboy0 · 2023-09-26T07:42:47Z

Landing the proposals to avoid more conflicts, will update their status once the implementation lands.

[Runtime] Async bodies + swift-http-types adoption ### Motivation Runtime changes of the approved proposals apple/swift-openapi-generator#255 and apple/swift-openapi-generator#254. ### Blockers of merging this - [x] 1.0 of swift-http-types ### Modifications ⚠️ Contains breaking changes, this will land to main and then 0.3.0 will be tagged, so not backwards compatible with 0.2.0. - add a dependency on https://github.com/apple/swift-http-types - remove our currency types `Request`, `Response`, `HeaderField` - replace them with the types provided by `HTTPTypes` - remove `...AsString` and the whole string-based serialization strategy, which was only ever used by bodies, but with streaming, we can't safely stream string chunks, only byte chunks, so we instead provide utils on `HTTPBody` to create it from string, and to collect it into a string. This means that the underlying type for the `text/plain` content type is now `HTTPBody` (a sequence of byte chunks) as opposed to `String` - adapted Converter extensions to work with the new types - added some internal utils for working with the query section of a path, as `HTTPTypes` doesn't provide that, the `path` property contains both the path and the query components (in `setEscapedQueryItem`) - adapted error types - adapted printing of request/response types, now no bytes of the body are printed, as they cannot be assumed to be consumable more than once - adjusted the transport and middleware protocols, as described in the proposal - removed the `queryParameters` property from `ServerRequestMetadata`, as now we parse the full query ourselves, instead of relying on the server transport to do it for us - removed `RouterPathComponent` as now we pass the full path string to the server transport in the `register` function, allowing transport with more flexible routers to handle mixed path components, e.g. `/foo/{bar}.zip`. - introduced `HTTPBody`, as described by the proposal - adjusted UniversalClient and UniversalServer - moved from String to Substring in a few types, to allow more passthrough of parsed string data without copying ### Result SOAR-0004 and SOAR-0005 implemented. ### Test Plan Introduced and adjusted tests for all of the above. Reviewed by: Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (docc test) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. ✖︎ pull request validation (api breakage) - Build finished. ✖︎ pull request validation (integration test) - Build finished. #47

[AHC Transport] Async bodies + swift-http-types adoption ### Motivation AHC transport changes of the approved proposals apple/swift-openapi-generator#255 and apple/swift-openapi-generator#254. ### Modifications - Adapts to the runtime changes, depends on HTTPTypes now. - Both request and response streaming works. ### Result Transport works with the 0.3.0 runtime API of. ### Test Plan Adapted tests. Reviewed by: dnadoba, simonjbeaumont Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. #16

[URLSession Transport] Async bodies + swift-http-types adoption ### Motivation URLSession transport changes of the approved proposals apple/swift-openapi-generator#255 and apple/swift-openapi-generator#254. ### Modifications - Adapts to the runtime changes, depends on HTTPTypes now. - Doesn't do streaming yet, we'll addressed that separately, continues to buffer for now (apple/swift-openapi-generator#301) ### Result Transport works with the 0.3.0 runtime API of. ### Test Plan Adapted tests. Reviewed by: simonjbeaumont Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. #15

[Generator] Async bodies + swift-http-types adoption ### Motivation Generator changes of the approved proposals #255 and #254. ### Modifications - Adapts to the runtime changes. - Most changes are tests updating to the new generated structure. - As usual, easiest to start with the diff to the file-based reference tests to understand the individual changes, and then review the rest of the PR. - To see how to use the generated code, check out some streaming examples in https://github.com/apple/swift-openapi-generator/pull/245/files#diff-2be042f4d1d5896dc213e3a5e451b168bd1f0143e76753f4a5be466a455255eb ### Result Generator works with the 0.3.0 runtime API of. ### Test Plan Adapted tests. Reviewed by: simonjbeaumont Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (compatibility test) - Build finished. ✔︎ pull request validation (docc test) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. ✖︎ pull request validation (integration test) - Build finished. #245

czechboy0 added 2 commits September 8, 2023 14:59

[Proposal] SOAR-0004: Streaming request and response bodies

b71df6a

Add links to SOAR-0005

95cca7d

czechboy0 added 4 commits September 8, 2023 16:12

Feedback: v1.1

9fb104c

Update SOAR-0004.md

fb397f6

Feedback: all the async sequences need to be Sendable

ff717b6

Merge branch 'hd-soar-0004' of github.com:czechboy0/swift-openapi-gen…

5432a22

…erator into hd-soar-0004

v1.3

c3f25d8

czechboy0 added 3 commits September 12, 2023 23:26

Add a mention of RFC 7464 and JSON Sequence

114b656

Review feedback: use opaque parameter types

4a24315

A further more Sendable annotations and cleanup of generic parameter …

37f3deb

…names.

gjcairo reviewed Sep 13, 2023

View reviewed changes

Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0004.md Show resolved Hide resolved

Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0004.md Outdated Show resolved Hide resolved

Wording improvements

f1cabbe

Update SOAR-0004.md

5aa3746

czechboy0 merged commit c8b0008 into apple:main Sep 26, 2023

czechboy0 deleted the hd-soar-0004 branch September 26, 2023 07:42

czechboy0 added the semver/none No version bump required. label Oct 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Proposal] SOAR-0004: Streaming request and response bodies #254

[Proposal] SOAR-0004: Streaming request and response bodies #254

czechboy0 commented Sep 8, 2023 •

edited

Loading

gjcairo commented Sep 8, 2023

gjcairo commented Sep 12, 2023

czechboy0 commented Sep 12, 2023

czechboy0 commented Sep 12, 2023

gjcairo commented Sep 15, 2023

czechboy0 commented Sep 26, 2023

[Proposal] SOAR-0004: Streaming request and response bodies #254

[Proposal] SOAR-0004: Streaming request and response bodies #254

Conversation

czechboy0 commented Sep 8, 2023 • edited Loading

Motivation

Modifications

Result

Test Plan

gjcairo commented Sep 8, 2023

gjcairo commented Sep 12, 2023

czechboy0 commented Sep 12, 2023

czechboy0 commented Sep 12, 2023

gjcairo commented Sep 15, 2023

czechboy0 commented Sep 26, 2023

czechboy0 commented Sep 8, 2023 •

edited

Loading