added curl transport #2
Conversation
|
Thanks for your contribution! Could you add CurlTransport to the integration test? The integration tests run against a node.js XML/RPC server, so all you need is node.js installed and than run the ./vendor.sh script. Adding it to the integration test suite will show two things: a) the check for the response is too generic, as it does not check for the HTTP response code and throws a specific exception there and b) there is not really a need for a specific unit test. The integration test will test your transport against every possible combination of serializers and parsers, so if this works you can be really certain it’s good :) Besides that I would suggest creating the curl handle in the constructor and just setting the URL as a parameter in send(). That way you could reuse the handle with the second request. |
| public function send($uri, $payload) | ||
| { | ||
| curl_setopt($this->handle, CURLOPT_URL, $uri); | ||
| curl_setopt($this->handle, CURLOPT_HTTPHEADER, array('Content-Type: text/xml')); |
There was a problem hiding this comment.
Every curl_setopt() call besides CURLOPT_URL could be done in the constructor.
There was a problem hiding this comment.
sorry but i disagree. think of curl_multi_exec. this is request specific.
There was a problem hiding this comment.
It’s more a really small optimization. FXMLRPC reuses client instances. So you save a few cycles by only doing the general setopt calls once.
There was a problem hiding this comment.
really don't like to do work in constructor... but here it is :)
|
integration tests) missed this. cannot run tests as i have none of this transports :( try to get travis green now :) |
|
|
||
| $response = curl_exec($this->handle); | ||
| $code = curl_getinfo($this->handle, CURLINFO_HTTP_CODE); | ||
| if (substr($code, 0, 1) != 2 || $response === false || strlen($response) < 1) { |
There was a problem hiding this comment.
You can check for exactly 200 here, as everything else is not allowed by XML/RPC.
|
Thanks again. See the rest of my comments. |
|
travis passed... enjoy :) |
|
Thanks for your patience! |
|
it was my pleasure :) |
cannot test as http://validator.xmlrpc.com/ is down :(