dagolden / HTTP-Tiny-UA

Higher-level UA features for HTTP::Tiny
https://metacpan.org/author/DAGOLDEN
4 stars 8 forks source link

SPDY support #1

Open dagolden opened 10 years ago

dagolden commented 10 years ago

This is a placeholder issue to discuss @odyniec's SPDY variant to HTTP::Tiny and how best to merge it in.

My inclination is to put HTTP::TIny::Handle::SPDY into it's own .pm file. (The original was bundled to make fatpacking HTTP::TIny easier).

I'd put the modified _request method into the main HTTP::Tiny::UA module rather than having it as a subclass.. My goal is to avoid fragmented subclasses that can't work together or require people to compose new classes. If it starts getting unwieldy, maybe a plugin/mixin system will work.

Actually, as I think about it, maybe it would be better to have a _request_spdy method that does all the hard work, and then just make an overloaded _request method that either calls _request_spdy or calls SUPER::_request. That would avoid duplicating a lot of the code, I think.

If there's a way to cleanly refactor the _request call in HTTP::Tiny itself so make swapping in SPDY require less duplicate code, I might be open to that as well.

Anyway, those are my thoughts so far. :-)

odyniec commented 10 years ago

Refactoring HTTP::Tiny's _request method might be the best solution. There are essentially three things that need to be done differently with SPDY -- establishing the connection, writing the request, and reading the response. Connecting and writing can already be implemented in subclasses in a rather clean manner by overriding the HTTP::Tiny::Handle::connect and HTTP::Tiny::Handle::write_request methods (and I did just that in HTTP::Tiny::SPDY), so we need to also allow that for reading the response.

For a quick proof of concept, I modified HTTP::Tiny a bit so that it allows its subclasses to define the handle class to instantiate, and to provide a custom method to read the response. See the diff here: https://github.com/odyniec/p5-http-tiny/compare/chansen:master...master

And these are the changes in HTTP::Tiny::SPDY (most of the duplicated code is removed): https://github.com/odyniec/p5-HTTP-Tiny-SPDY/compare/read-response

dagolden commented 10 years ago

I haven't forgotten about this. I'm just completely swamped. Sorry.