Hey everyone,
I was wondering what the possability was of changing these lines:
body = request.raw_post
Hash.from_xml(body)
or
data = ActiveSupport::JSON.decode(body)
to instead be handed the env['rack.input'] IO itself?
Being as though both calls (from_xml and decode) are handled by rails
elsewhere, we could move the responsibility of parsing of the request
body IO to the swappable XML/JSON backends.
I'm asking because I'm looking to integrate yajl-ruby as one of the
JSON backends for ActiveSupport in Rails 3. It's capable of parsing
JSON right off the IO as a stream, and this change would allow it to
perform it's best (and keep memory usage very low for large request
bodies).
As for doing it for the XML backends as well, it opens up the
possibility of doing the type of stream parsing for XML.
The problem is that the ParamsParser reads the entire body into a
string (from what I can tell by the line "body = request.raw_post")
before handing it to the parsing backend. My suggestion is that it
hand the IO object (env['rack.input'] I assume?) to the parser
instead. This way the parser can either read the entire string, then
parse - OR if it supports parsing as a stream, start doing so directly
off the IO.
I guess the real patch would allow "ActiveSupport::JSON.decode" to
accept an IO object as well. Then we could just pass that directly in.
yeah, ideally both the JSON and XML parsers would accept an IO, and
the 'read into a string' logic would live in the implementations which
don't support streaming.
Ok I forked and patched the ParamsParser to just pass request.body to
the parsers. I did it for JSON, XML and YAML (using YAML.load_stream
instead of just load).
I also patched the XmlMini and JSON decoders for parsing from an IO,
in addition to a string. And as a result (like you said), I didn't
have to refactor any tests. Just added the ones regarding parsing from
an IO.
The one test I haven't figured out how to write, is the "that the
ParamsParser middleware doesn't read the entire stream into a string
itself." test. I can imagine how I might do it using rspec/mocha but
no idea using Test::Unit.
Thanks for committing that, but what was the reason you removed the
yajl JSON backend and updated JSON test?
I should have noted somewhere that the JSON test and yajl.rb backend
were originally written by Rick Olson, and I made modifications to
support the ability to be passed an IO.
I've updated the LH ticket with another patch - finishing off the
changes to actually parse from the IO (instead of converting to a
string first) for the rexml, libxml and nokogiri backends.
Should this be in another ticket?
