-
Notifications
You must be signed in to change notification settings - Fork 741
feat: introduce Body type class and some Body types for HTTP
#12144
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
base: sofia/async-http-uri
Are you sure you want to change the base?
Conversation
9882dba to
c77b3ee
Compare
2c034a9 to
2359476
Compare
|
Mathlib CI status (docs):
|
|
Reference manual CI status:
|
c77b3ee to
bad70e3
Compare
2437c9f to
f8ad249
Compare
| -/ | ||
| def size? (_ : Empty) : Async (Option Body.Length) := | ||
| pure (some (.fixed 0)) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring says "Returns none" but the implementation returns some (.fixed 0).
| c[0]'(Nat.le_of_eq h) | ||
| else | ||
| c.data.foldl (· ++ ·) (.emptyWithCapacity c.size) | ||
| c.foldl (· ++ ·) (.emptyWithCapacity c.size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After let c := c.data.toArray, the name c is shadowed. Now c.size in .emptyWithCapacity c.size is the number of ByteArrays in the queue, not the total byte count. The original code used the struct's size field (total bytes) for pre-allocation.
Should this be something like:
def toByteArray (cb : ChunkedBuffer) : ByteArray :=
let arr := cb.data.toArray
if h : 1 = arr.size then
arr[0]'(Nat.le_of_eq h)
else
arr.foldl (· ++ ·) (.emptyWithCapacity cb.size)| -/ | ||
| @[inline] | ||
| def append (buffer : ChunkedBuffer) (data : ChunkedBuffer) : ChunkedBuffer := | ||
| { data := buffer.data.enqueueAll data.data.toArray.toList.reverse, size := buffer.size + data.size } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .reverse here is correct but non-obvious—I initially thought it was a bug. Queue.enqueueAll prepends to eList, so reversing is needed to maintain FIFO order after the internal reversal during toArray/dequeue?.
Would a brief comment help future readers? Something like:
-- Queue.enqueueAll prepends to eList, so reverse to maintain FIFO order
This PR introduces the
Bodytype class, theChunkStreamandFulltypes that are used to represent streaming bodies of Requests and Responses.This contains the same code as #10478, divided into separate pieces to facilitate easier review.
The pieces of this feature are:
Headersdata type for HTTP #12127URIdata type for HTTP #12128Bodytype class and some Body types for HTTP #12144