Skip to content

Introduce NIODecodedAsyncSequence for easy decoding of async sequences #3407

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Oct 16, 2025
Merged

Introduce NIODecodedAsyncSequence for easy decoding of async sequences #3407

merged 17 commits into from
Oct 16, 2025

Conversation

@MahdiBM
Copy link
Contributor

@MahdiBM MahdiBM commented Oct 14, 2025 •
edited
Loading

Previous PR: #3405

Add an API on top of AsyncSequence<ByteBuffer> which can dynamically decode values.
Add an API on top of the new AsyncSequence<ByteBuffer> APIs which splits the file based on its content.

Motivation:

Provides a nice API to decode files, instead of users having to go though manually handling BufferedReader.read(while:).
I struggled with this, as documented in https://swift-open-source.slack.com/archives/C9MMT6VGB/p1760115481607159

Modifications:

Add NIODecodedAsyncSequence + functions on AsyncSequence<ByteBuffer> to create such a sequence.
Add NIOSplitMessageDecoder + stdlib-like functions on AsyncSequence<ByteBuffer> to create such a sequence.

Result:

Users can decode an async sequence of ByteBuffers easier.
Users can easily split files based on their content.

Checklist

See this comment for a checklist of the remaining things to do: #3407 (comment)

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Oct 14, 2025

@glbrntt
I can split the NIOSplitMessageDecoder into another PR if you want.
I haven't added docs or unit tests to most stuff. I'll add those after I know this implementation looks fine to you.
I just converted the integration tests from the other PR to integration tests for this PR, so you can trust the implementation does work at the very least.

MahdiBM
MahdiBM commented Oct 14, 2025
View reviewed changes
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Oct 14, 2025

Also cc @Lukasa. Since the PR is in "draft" mode it doesn't ping maintainers for reviews, so I thought I can do it manually.

@MahdiBM MahdiBM force-pushed the mmbm-split-take-2 branch 3 times, most recently from 2ab34fe to 5ca14cb Compare October 14, 2025 14:34
@MahdiBM MahdiBM requested a review from glbrntt October 14, 2025 14:36
glbrntt
glbrntt reviewed Oct 14, 2025
View reviewed changes
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

The overall shape looks good! I left a few high-level comments inline.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Oct 14, 2025 •
edited
Loading

Checklist of the remaining things:

  • Unit tests
  • More docs
  • Move NIOSplitMessageDecoder to another PR

I'll request a review when these are done.

@MahdiBM MahdiBM force-pushed the mmbm-split-take-2 branch 4 times, most recently from 9f88b1b to 0d8cbc2 Compare October 15, 2025 08:16
@MahdiBM MahdiBM force-pushed the mmbm-split-take-2 branch 2 times, most recently from 08a664d to 758008b Compare October 15, 2025 08:27
@MahdiBM MahdiBM marked this pull request as ready for review October 15, 2025 08:28
@MahdiBM MahdiBM requested a review from glbrntt October 15, 2025 08:28
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Oct 15, 2025 •
edited
Loading

@glbrntt The PR should be ready. I'm open to better namings or more test ideas or nit picks etc...

MahdiBM
MahdiBM commented Oct 15, 2025
View reviewed changes
glbrntt
glbrntt reviewed Oct 15, 2025
View reviewed changes
@MahdiBM MahdiBM requested a review from glbrntt October 15, 2025 10:24
@MahdiBM MahdiBM changed the title Introduce NIODecodedAsyncSequence + NIOSplitMessageDecoder to decode ByteBuffers and split content Introduce NIODecodedAsyncSequence for easy decoding of async sequences Oct 15, 2025
glbrntt
glbrntt requested changes Oct 15, 2025
View reviewed changes
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

This is looking a lot better!

@glbrntt glbrntt added the 🆕 semver/minor Adds new public API. label Oct 15, 2025
@MahdiBM MahdiBM requested a review from glbrntt October 15, 2025 12:59
@MahdiBM MahdiBM force-pushed the mmbm-split-take-2 branch 2 times, most recently from f6618d8 to 9430c32 Compare October 15, 2025 13:02
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Oct 15, 2025

@glbrntt should be good to go assuming I've satisfied the docs soundness CI.
Other failures look unrelated to this PR. Most of them complaining that swift-nio has moved to Swift 6.0 and doesn't support 5.10 anymore.

glbrntt
glbrntt approved these changes Oct 16, 2025
View reviewed changes
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @MahdiBM!

@glbrntt glbrntt enabled auto-merge (squash) October 16, 2025 09:39
@glbrntt glbrntt merged commit 86c5ead into apple:main Oct 16, 2025
57 of 62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glbrntt glbrntt glbrntt approved these changes

Assignees

No one assigned

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MahdiBM @glbrntt