ocaml-cohttp

ocaml-cohttp copied to clipboard

From @rgrinberg:

Also what about the Body.t refactor? The current code has the nasty side effect of making the users think that the body value is persistent and calling things like to_string multiple times on it. My original suggestion was to split body into 2 types:

1. A type that makes it easy for users to construct requests using base type they might have (strings, bytes, lists of strings, pipes, etc.). (String_builder.t perhaps?)

2. Another type that we hand over to users that makes it clear that they are responsible for persisting the body somewhere if they intend to reuse it.

At this point I'd really like to gather some data points from other http libraries/frameworks/toolkits. Also, I'm not insisting that this should block 1.0 but it cannot be avoided in the long run either.

If I understand this right, String_builder.t would essentially be the current Body.t and become a 'one shot' function to generate or parse a Body.

We would then need a memoized Body.t that could survive repeated calls of to_string or similar. Wouldn't this just be a Cohttp.Body.t without the Async/Lwt additional functions? The portable library is always safe to repeatedly call functions like to_string on.

Before talking about implementation details let me just state the goals I'm trying to achieve here:

• I want users to be able to construct response bodies as conveniently as possible (strings, buffers, lists of strings, various streaming abstractions, backend specific abstractions, etc.)
• We should not do any unnecessary copying of the inputs the user provides us
• We should not do any memoization or buffering of responses as it introduces hidden memory costs and punt this to the user. Alternatively, we could do the memoization but we must be very explicit with regards to memory consumption.

If I understand this right, String_builder.t would essentially be the current Body.t and become a 'one shot' function to generate or parse a Body.

Yeah that sounds right. Although I'm not sure what you mean by "one shot".

We would then need a memoized Body.t that could survive repeated calls of to_string or similar. Wouldn't this just be a Cohttp.Body.t without the Async/Lwt additional functions? The portable library is always safe to repeatedly call functions like to_string on.

Instead of doing being the scenes memoization here we couldjust be making the conversion to string explicit (call it drain_to_string perhaps) and raise on a user's attempt to call to_string twice. Of course this has its own disadvantages. This is another one of those things where we need to look at other http toolkits for inspiration.

The portable library is always safe to repeatedly call functions like to_string on.

This is true but there could be value in letting a user choose a backend agnostic stream implementation so this might not always hold.