Rack::Client::Parser's not working as expected

[benschwarz]

I couldn't get it to work, so I thought I'd write a spec that validated that fact.

Hopefully this helps you either correct the operation or you can let me know if I've misinterpreted how this should exactly work.

Either way, I figure a pull request is a good start.

[benschwarz]

Through reading all of the code and writing further tests, I discovered that the body is parsed, but it is kept on a header called rack-client.body_collection. Its stored in an array (I assume this is because responses could be chunked? Or Multiget?)

Either way, were you planning on writing this back to the body accessor? Or to a special parsed_body method?

[benburkert]

The parser code was a spike that never really got finished, and probably shouldn't have been merged into master. It's no where near ready for use. I have been manually writing middleware's for dumping & loading json requests & responses.

The reason for the 'rack-client.body_collection' header was to keep the parser middleware compliant with the rack spec, which states that "The Body must respond to each and must only yield String values". The idea was for the Base adapter to follow the rack spec to the letter, but also allow for more adapters which implemented extensions to the rack spec. For example, the Simple adapter could be updated to look for the 'rack-client.body_collection' header when each is called on the body, similar to the way the collapsed body works.

[benschwarz]

I'm cool to implement something because I actually want to use this functionality—

If you wrote some failing tests I'd be happy to implement it from there…

[benburkert]

awesome.

I'll try adding some tests this weekend, although i don't usually write tests up front. I might end up doing a reference implementation for YAML or something.

[benschwarz]

Thinking about this recently—

I think that the parsers (if included within your "stack") should alter the "body" of the response, here is the rationale:

• These parsers are not themselves rack middlewares, they cannot be used outside the context of rack-client. (It simply does not make sense)
• If a response yields a Content-Type of application/json, I'd expect it to become a native ruby hash after being run through the parser. It feels as a logical step from including it within my "http client stack".

Now—Having said that, I think that including it along side the Rack middlewares feels strange. I'm not sure if the syntax should be altered to include something like this, but I'd be interested to hear how halorgium feels about these thoughts.

[halorgium]

I believe this parser should be a middleware even though it is potentially meant to be returning something not like a Rack response.

The thing about a decorator is that the post-conditions must not be weakened. So making the Parser.new(Client.new).call(request) now return a non-Rack response is not valid. The reason for this is that it makes this client impossible to be extended further. This does raise the question of whether this is part of the default client stack or as a final post-process.

Using the callbacks for capturing the output of the parser does seem to be sensible though. I have been unsure how to sanely do things like have a middleware which converts the hash which is from the JSON into some Domain models.

Hope this provides some perspective of what I've been thinking.

[benschwarz]

Thanks for your perspective Tim.

I think you're spot on with your comments about weakening the chain—This is why I think that a parser (which wouldn't/couldn't be used as a regular rack-middleware) should be a presenter that is either persistant in rack-client, or perhaps even explicitly required. (Change the way that we interface with it, as for a design, I'm not so sure…)

Being the way that this discussion has played out, I'm not certain who will be making the next strike here…