Skip to content

[Multipart] Add a frames -> raw parts parsing sequence #74

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
merged 6 commits into from
Nov 17, 2023

Conversation

czechboy0
Copy link
Contributor

@czechboy0 czechboy0 commented Nov 16, 2023

Motivation

Next sequence, parses MultipartRawPart values from a sequence of MultipartFrame values.

Modifications

Moved the MultipartParser/Serializer into the files where their respective sequences live, it's better organized this way. But it's just a move so doesn't need to be re-reviewed.

This sequence is a bit unique, as this is where we need to handle the two sequences pulling from a common source of truth.

On the consumer side, this allows you to do:

for try await part in partsSequence {
    for try await chunk in part.body {
        // process chunk of this part
    }
}

The outer sequence has the element of MultipartRawType, which has header fields, and an HTTPBody (which itself is an async sequence, of byte chunks). This sequence vends MultipartRawType values, and needs to demultiplex the frames appropriately to the right async sequence (either the outer one vending parts, or the inner body one of the last-vended part).

It is a requirement that the part's body chunks (inner sequence) must be fully consumed before the outer sequence is asked for the next part. The state machine enforces this.

The solution is by using an actor that both iterators (outer and each of the inner ones) have a reference to and can ask for frames.

Also, had to disable strict concurrency checking on 5.10, because they added a warning when you iterate an async sequence from an actor; it's known, no workaround AFAIK. The alternative here would be to turn off warnings-as-errors for 5.10.

Result

A parsing sequence that turns a sequence of frames into a sequence of parts, each of which has a nested sequence of body chunks.

Test Plan

Unit tests added at all 3 layers:

  • sequence
  • shared iterator actor
  • state machine of the shared iterator

@czechboy0 czechboy0 marked this pull request as ready for review November 16, 2023 14:33
Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

This looks good to me. Did you want more eyes on the state machine logic?

@czechboy0
Copy link
Contributor Author

Happy to get feedback even after landing this, but the unit tests cover at least the common codepaths.

@czechboy0 czechboy0 merged commit 927f930 into apple:main Nov 17, 2023
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Nov 27, 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.

2 participants