Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

Reel::RequestBody #75

Merged
merged 2 commits into from
Aug 11, 2013
Merged

Reel::RequestBody #75

merged 2 commits into from
Aug 11, 2013

Conversation

tarcieri
Copy link
Member

Encapsulate request bodies in their own object. This allows easy conversion to a
string in cases where streaming isn't desired, or streaming as an IO-alike
through #read and #readpartial

This is a breaking change from the previous API. People who were using
request.body will now need to use request.body.to_s. The previous callback-based
API for reading the body is completely removed.

/cc @adstage-david @raggi @halorgium

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 76055f3 on request-body-objects into f7eb96a on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 924576a on request-body-objects into f7eb96a on master.

Encapsulate request bodies in their own object. This allows easy conversion to a
string in cases where streaming isn't desired, or streaming as an IO-alike
through #read and #readpartial

This is a breaking change from the previous API. People who were using
request.body will now need to use request.body.to_s. The previous callback-based
API for reading the body is completely removed.
@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 0724c9b on request-body-objects into f7eb96a on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 0724c9b on request-body-objects into f7eb96a on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 0724c9b on request-body-objects into f7eb96a on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 0724c9b on request-body-objects into f7eb96a on master.

end

# Eagerly consume the entire body as a string
def to_s
Copy link
Contributor

Choose a reason for hiding this comment

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

I ponder whether this might be better called string as in StringIO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps. That said I'm trying to allow this body to passed directly through in e.g. Webmachine, and they probably want a #to_s style API. Still waiting to hear back from @seancribbs on that

Copy link
Contributor

Choose a reason for hiding this comment

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

Either could be solved with an alias.

@adstage-david
Copy link

This looks good to me - getting rid of that callback for the body block is nice, that code felt kind of ugly.

@tarcieri
Copy link
Member Author

There's a bug somewhere in this code. I'm getting RequestBody#readpartial returning an empty string for whatever reason, which it seems to be explicitly designed not to do.

Will continue investigating... and hopefully capture this case in a test

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 9ed107e on request-body-objects into f7eb96a on master.

tarcieri added a commit that referenced this pull request Aug 11, 2013
@tarcieri tarcieri merged commit 2fe4947 into master Aug 11, 2013
@tarcieri tarcieri deleted the request-body-objects branch August 11, 2013 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants