extend Body with reader/writer semantics, split Chunked_body#972
Closed
bikallem wants to merge 3 commits into
Closed
extend Body with reader/writer semantics, split Chunked_body#972bikallem wants to merge 3 commits into
bikallem wants to merge 3 commits into
Conversation
Rwer module mostly served as miscellaneous dump bag of various readers and writers. The name Rwer always felt a bit incoherent even when it was initially added. However, I couldn't quite pin the correct module name for it. Recently, while working on another PR, it dawned on me that the readers and writers can be logically split into Buf_read and Buf_write modules by extending Eio Buf_read and Buf_write modules respectively.
Extend Body module with writer/reader semantics.
Splits Chunked_body module from Body module. Add reader/writer for HTTP chunked encoding.
Contributor
Author
|
Closing this in support of discussion happening at #969 |
This file contains hidden or 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
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.
This PR is split off from #969 and builds on top of #971. Therefore only the last 2 commits belong to this PR. The first commit should go away with rebase to master after #971 is merged.
The PR improves the abstraction mechanism to define/use HTTP request/response body. A HTTP request/response body needs to also define Headers that go along with it. For e.g. a fixed body needs at least a Content-Type and Content-Length header, a chunked encoding needs
Transfer-Encodingheaders and possibly a Trailer header and trailers, A JSON body needs a different Content-Type value and so on. The current abstraction is leaky since it leaves an important requirement for the user of the library to devise/figure out and makes reuse of writers a bit less useful and convenient.The current Body.t type also lacks a precise/formal notion of reading a body, for e.g. we have
Custom buf_writefor writing a body but we lack similar mechanism for reading. From the types (in the Body) it we get a sense that they are mostly for writing the body and not for reading the body.The
reader/writerabstraction aims to address the shortcomings identified above. We no longer need the brittleBody.t. However, for the purposes of keeping the trunk/master buildable, tests passing and the CI green, we still retain the "legacy" code in this PR.Body.twill be cleaned up in a future PR after Server/Client PRs (future PRs) are merged.Body.writerabstracts HTTP request/response body that can be written.Body.readerabstracts HTTP request/response body that can be read. The PR includes implementations of two common writers/readers,content_writer/readerandform_values_writer/readerin Body module.content_writer/readerencapsulates previousFixedbody semantics andform_values_writer/readeris a new body that reusescontent_writer/readerto implement HTTP form body writing/reading. Notice the convenience with which we reuse writers/readers. Future PRs should add json and xml reader/writer by again reusing the content_writer/reader.Chunked encoding is split into separate module
Chunked_bodyand now follows the reader/writer semantics. However, as mentioned above theBodymodule still includes chunked body functionality for now so as to keep the ci/trunk green. This duplication will be cleaned up once the relevant future PRs are merged./cc @patricoferris @mseri