patternphpMinor
cURL based REST client library
Viewed 0 times
restclientlibrarybasedcurl
Problem
I have recently refactored a REST client library and was hoping to get review for both the class itself and the unit tests that cover it.
This code is developed against PHP 7.1, but tested against PHP 5.6, 7.0 7.1, HipHop VM, and PHP nightly builds.
The full library can be seen on GitHub
RestClient.php
```
validateAction($action);
$this->curlSetup();
$this->setRequestUrl($action);
curl_setopt($this->curl, CURLOPT_HTTPGET, true);
// execute call. Can throw \Exception.
$this->curlExec();
return $this;
}
/**
* Method to exexute POST on server
*
* @param mixed $action
* @param mixed $data
* @return RestClient
* @throws \InvalidArgumentException
* @throws \Exception
*/
public function post($action = null, $data = null) {
$this->validateAction($action);
$this->validateData($data);
$this->curlSetup();
$this->setRequestUrl($action);
$this->setRequestData($data);
curl_setopt($this->curl, CURLOPT_POST, true);
// execute call. Can throw \Exception.
$this->curlExec();
return $this;
}
/**
* Method to execute PUT on server
*
* @param string $action
* @param mixed $data
* @return RestClient
* @throws \InvalidArgumentException
* @throws \Exception
*/
public function put($action = null, $data = null) {
$this->validateAction($action);
$this->validateData($data);
$this->curlSetup();
$this->setRequestUrl($action);
$this->setRequestData($data);
curl_setopt($this->curl, CURLOPT_CUSTOMREQUEST, 'PUT');
// execute call. Can throw \Exception.
$this->curlExec();
return $this;
}
/**
* Method to execute DELETE on server
*
* @param string $action
* @return RestClient
* @throws \InvalidArgumentException
* @throws \Exception
*/
public function dele
This code is developed against PHP 7.1, but tested against PHP 5.6, 7.0 7.1, HipHop VM, and PHP nightly builds.
The full library can be seen on GitHub
RestClient.php
```
validateAction($action);
$this->curlSetup();
$this->setRequestUrl($action);
curl_setopt($this->curl, CURLOPT_HTTPGET, true);
// execute call. Can throw \Exception.
$this->curlExec();
return $this;
}
/**
* Method to exexute POST on server
*
* @param mixed $action
* @param mixed $data
* @return RestClient
* @throws \InvalidArgumentException
* @throws \Exception
*/
public function post($action = null, $data = null) {
$this->validateAction($action);
$this->validateData($data);
$this->curlSetup();
$this->setRequestUrl($action);
$this->setRequestData($data);
curl_setopt($this->curl, CURLOPT_POST, true);
// execute call. Can throw \Exception.
$this->curlExec();
return $this;
}
/**
* Method to execute PUT on server
*
* @param string $action
* @param mixed $data
* @return RestClient
* @throws \InvalidArgumentException
* @throws \Exception
*/
public function put($action = null, $data = null) {
$this->validateAction($action);
$this->validateData($data);
$this->curlSetup();
$this->setRequestUrl($action);
$this->setRequestData($data);
curl_setopt($this->curl, CURLOPT_CUSTOMREQUEST, 'PUT');
// execute call. Can throw \Exception.
$this->curlExec();
return $this;
}
/**
* Method to execute DELETE on server
*
* @param string $action
* @return RestClient
* @throws \InvalidArgumentException
* @throws \Exception
*/
public function dele
Solution
When I look at the RestClient class, what is required to make a request?
At first glance, this appears all I need to do, right? But then I see method like
I think the confusion comes from this class doing too much. I see this actually requiring three classes:
You could even consider having the
This gives you a few more options when handling error conditions:
Mike Brant commented:
... I think exception-response concept is interesting and certainly something similar to what may be seen in other libraries of this sort. My initial intention was to not impose response interpretation in this class.
This is a good point. In fact, I would argue that throwing exceptions AND just returning a response with no exception is perfectly valid. It depends on the use case.
If you can gracefully handle 4xx and 5xx HTTP responses, then there is no need for an exception.
If your code absolutely must work and you can't handle the 4xx or 5xx responses, exceptions are the way to go.
Both are useful, and that might be a good option to include in the
The code above says "If a non 200 response is received, no big deal. We'll show a message to the user. Now consider the following code:
Here, a POST is modifying or creating data on the server. No error handling is done, because we really can't handle server problems when POST-ing data (creating a Blog Post). An exception here is useful because seeing "Blog post created successfully" on screen doesn't make sense if it wasn't created - and your PHP script can't do anything about it.
There is a use case for both kinds of error handling: Error codes and exceptions.
$client = new RestClient();
$client->get("http://codereview.stackexchange.com");At first glance, this appears all I need to do, right? But then I see method like
setRequestUrl and setRequestData and I'm not so sure. I also see an empty constructor.I think the confusion comes from this class doing too much. I see this actually requiring three classes:
RestClient- A wrapper for RESTful calls
get($url, $params = null): HttpResponse
post($url, $params = null): HttpResponse
- And
put,delete, andheadmethods
- Settings defining things like authentication, SSL, timeouts, etc.
HttpRequest- A wrapper class defining an HTTP request
- HTTP verb (GET, PUT, POST, DELETE, HEAD)
- Request headers
- Request body
- Query string params
HttpResponse- A wrapper class for the response
- Response status
- Response headers
- Response body
- A reference to the
HttpRequestobject that generated this response maybe?
You could even consider having the
RestClient throw specific Exceptions when non 200 or 201 HTTP status codes are returned:class RestResponseException extends Exception
{
private $response;
public function __construct(HttpResponse $response, int $httpStatusCode) {
$this->response = $response;
}
public function getResponse() {
return $this->response;
}
}
// 404 Not Found
class ResourceNotFoundException extends RestResponseException
{
public const STATUS_CODE = 404;
public function __construct(HttpResponse $response) {
parent::__construct($response, STATUS_CODE);
}
}This gives you a few more options when handling error conditions:
$client = new RestClient();
try {
$response = $client->get("...");
} catch (ResourceNotFoundException $notFoundError) {
echo "Couldn't find X";
} catch (RestResponseException $responseError) {
echo "Something went wrong, try again."
} catch (Exception $ex) {
// Log the exception, something outside the REST request/response went wrong
}Mike Brant commented:
... I think exception-response concept is interesting and certainly something similar to what may be seen in other libraries of this sort. My initial intention was to not impose response interpretation in this class.
This is a good point. In fact, I would argue that throwing exceptions AND just returning a response with no exception is perfectly valid. It depends on the use case.
If you can gracefully handle 4xx and 5xx HTTP responses, then there is no need for an exception.
If your code absolutely must work and you can't handle the 4xx or 5xx responses, exceptions are the way to go.
Both are useful, and that might be a good option to include in the
RestClient - whether or not 4xx and 5xx responses should throw exceptions.$response = $client->get("...");
if ($response->getStatus() != 200) {
// Show gracefull error message
}The code above says "If a non 200 response is received, no big deal. We'll show a message to the user. Now consider the following code:
$response = $client->post("...", data);
echo "Blog post created successfully";Here, a POST is modifying or creating data on the server. No error handling is done, because we really can't handle server problems when POST-ing data (creating a Blog Post). An exception here is useful because seeing "Blog post created successfully" on screen doesn't make sense if it wasn't created - and your PHP script can't do anything about it.
There is a use case for both kinds of error handling: Error codes and exceptions.
Code Snippets
$client = new RestClient();
$client->get("http://codereview.stackexchange.com");class RestResponseException extends Exception
{
private $response;
public function __construct(HttpResponse $response, int $httpStatusCode) {
$this->response = $response;
}
public function getResponse() {
return $this->response;
}
}
// 404 Not Found
class ResourceNotFoundException extends RestResponseException
{
public const STATUS_CODE = 404;
public function __construct(HttpResponse $response) {
parent::__construct($response, STATUS_CODE);
}
}$client = new RestClient();
try {
$response = $client->get("...");
} catch (ResourceNotFoundException $notFoundError) {
echo "Couldn't find X";
} catch (RestResponseException $responseError) {
echo "Something went wrong, try again."
} catch (Exception $ex) {
// Log the exception, something outside the REST request/response went wrong
}$response = $client->get("...");
if ($response->getStatus() != 200) {
// Show gracefull error message
}$response = $client->post("...", data);
echo "Blog post created successfully";Context
StackExchange Code Review Q#156714, answer score: 4
Revisions (0)
No revisions yet.