Added feature to inject custom HTTP clients

[SammyK]

Take a deep breath...! The very much needed injectable HTTP client handler PR is here! :) This would close #4.

Usage

You can inject your own HTTP client in two steps:

1. Code to the Facebook\FacebookHttpable interface.
2. Inject your implementation like so:

FacebookRequest::setHttpClientHandler(new MyCustomHttpClient());

I've posted examples of Guzzle and stream wrapper implementations.

Considerations

• I really tried to decouple the code in the FacebookRequest object in this refactor. The execute() method was especially getting out of hand. It's not perfect yet, but it's much better than before.
• I killed the IPv6 workaround since it was broken anyways - even with the latest fix 0a34085. And it really shouldn't be the responsibility of the SDK to fix people's misconfigured servers. :)
• This merge is going to cause some serious conflicts with Fall back to use http stream when Curl module is not enabled #75. But I hope you can see why we don't need Fall back to use http stream when Curl module is not enabled #75 with this PR since the stream implementation can be easily injected by the developer.
• If you'd like to include the Guzzle and stream wrapper implementations in the SDK, that would make it easy for a developer to just inject a new implementation with just two lines of code without even having to write anything new on their own:

use Facebook\FacebookStreamHttpClient;
FacebookRequest::setHttpClientHandler(new FacebookStreamHttpClient());

• There is a new dev dependency in the composer.json file. It pulls in mockery so that we can mock dependancies. The test suite doesn't test in isolation enough and mockery will make it easier to do that going forward. So make sure to run composer update when you pull this one in to test.

The end

Let me know if you have any questions on anything!

[ahsanity]

Great work! Love the idea of injectable HTTP client handler :)

Here's my 2cents:

1. the class FacebookStreamHttpClient in your examples should also be part of this PR (good to see some of the code from Fall back to use http stream when Curl module is not enabled #75 in this class!)
2. getHttpClientHandler() method in FacebookRequest should fall back to use FacebookStreamHttpClient if curl is not enabled in the environment (i.e. of the user didn't set her own custom class)

Reasoning:
I know your main design philosophy is that the developer can add her own custom http client class depending on the platform/environment. This is great but I believe both the curl http client and stream http client class should be added by default as this covers most of the use cases for php environments. That way the average joe developer (like me) don't need to bother with adding their own implementation as with these two classes it covers most php environments. Only when a dev has some special need (or want to use some other 3rd party lib) they can implement their own class.

Hope this is useful and makes sense :)

Cheers!

[gfosco]

I'm supportive of this and the Stream client inclusion and fallback. Great stuff Sammy.

[yguedidi]

Missing type hint

[yguedidi]

Whaoo great work ! Much better than what I'd send ;)
Just a question, why add mockery ? You can mock with PHPUnit.
I'd also propose to move FacebookCurl to the end of FacebookCurlHtttpClient. Because it's act like a private class, should not be instanciated by the dev.

[SammyK]

Thanks for the feedback guys! :)

@ahsanity & @gfosco Sweet - I'll add the Guzzle and stream implementations in another PR with the stream implementation fallback. That way this PR doesn't get too out of scope. :)

@yguedidi Thanks for the code review! I'll def tweak some of your callouts.

On moving FacebookCurl into the same file as FacebookCurlHtttpClient - that's funny you should mention that because that's how I had it at first! :) But when both classes are in the same file, the FacebookCurl class will be declared twice when you try to mock the FacebookCurl object for testing thus throwing an error.

On mockery - I included it because it has really become the de facto standard in mocking objects in PHPUnit and has a lot more power and flexibility when mocking an object than PHPUnit's mocking capabilities.

[SammyK]

Done! This commit should merge without conflicts.

@yguedidi - On moving the curl bug fix to FacebookCurl, I really like that idea, but unfortunately FacebookCurl isn't tested as-is since its only reason for existing is so that we can mock the curl functionality for the tests in the implementation. It's just a procedural-to-object conversion and nothing more. And I want to be able to test the bug fix. So it makes more sense for testing purposes to keep the bug fix in FacebookCurlHtttpClient. :)

[gfosco]

Onward and upward.

[SammyK]

W00t, w00t! 👍