Skip to content

Add timeout options and some maintenance stuff. #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ php:
- 5.4
- 5.5
- 5.6
- hhvm
- 7.0
- 7.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prep. for upcoming 7.1 release

- hhvm
- hhvm-nightly

matrix:
allow_failures:
- php: 7.0
- php: 7.1
- php: hhvm-nightly

script: phpunit --bootstrap autoload.php tests/
script: phpunit --verbose
24 changes: 24 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>

<phpunit backupGlobals="false"
backupStaticAttributes="false"
colors="true"
convertErrorsToExceptions="true"
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
processIsolation="false"
stopOnFailure="false"
syntaxCheck="false"
bootstrap="autoload.php"
>
<testsuites>
<testsuite name="php-rest-api">
<directory>./tests/</directory>
</testsuite>
</testsuites>
<filter>
<whitelist>
<directory suffix=".php">./src/</directory>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allows for PHPUnit to generate a coverage report (and easy PHPUnit runs from the commandline, i.e. just phpunit)

</whitelist>
</filter>
</phpunit>
13 changes: 7 additions & 6 deletions src/MessageBird/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
*/
class Client
{

const ENDPOINT = 'https://rest.messagebird.com';
const CHATAPI_ENDPOINT = 'https://chat.messagebird.com/1';

Expand Down Expand Up @@ -59,14 +58,17 @@ class Client
* @var Resources\Chat\Message
*/
public $chatMessages;

/**
* @var Resources\Chat\Channel
*/
public $chatChannels;

/**
* @var Resources\Chat\Platform
*/
public $chatPlatforms;

/**
* @var Resources\Chat\Contact
*/
Expand All @@ -88,7 +90,7 @@ class Client
*/
public function __construct($accessKey = null, Common\HttpClient $httpClient = null)
{
if ($httpClient == null) {
if ($httpClient === null) {
$this->ChatAPIHttpClient = new Common\HttpClient(self::CHATAPI_ENDPOINT);
$this->HttpClient = new Common\HttpClient(self::ENDPOINT);
} else {
Expand Down Expand Up @@ -117,7 +119,6 @@ public function __construct($accessKey = null, Common\HttpClient $httpClient = n
$this->chatChannels = new Resources\Chat\Channel($this->ChatAPIHttpClient);
$this->chatPlatforms = new Resources\Chat\Platform($this->ChatAPIHttpClient);
$this->chatContacts = new Resources\Chat\Contact($this->ChatAPIHttpClient);

}

/**
Expand All @@ -138,9 +139,9 @@ private function getPhpVersion()
{
if (!defined('PHP_VERSION_ID')) {
$version = explode('.', PHP_VERSION);
define('PHP_VERSION_ID', ($version[0] * 10000 + $version[1] * 100 + $version[2]));
define('PHP_VERSION_ID', $version[0] * 10000 + $version[1] * 100 + $version[2]);
}
return "PHP/" . PHP_VERSION_ID;
}

return 'PHP/' . PHP_VERSION_ID;
}
}
75 changes: 54 additions & 21 deletions src/MessageBird/Common/HttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
*/
class HttpClient
{
const REQUEST_GET = "GET";
const REQUEST_POST = "POST";
const REQUEST_DELETE = "DELETE";
const REQUEST_PUT = "PUT";
const REQUEST_GET = 'GET';
const REQUEST_POST = 'POST';
const REQUEST_DELETE = 'DELETE';
const REQUEST_PUT = 'PUT';

const HTTP_NO_CONTENT = 204;

Expand All @@ -27,19 +27,51 @@ class HttpClient
/**
* @var array
*/
protected $userAgent = array ();
protected $userAgent = array();

/**
* @var Common\Authentication
*/
protected $Authentication;

/**
* @var int
*/
private $timeout = 10;

/**
* @var int
*/
private $connectionTimeout = 2;

/**
* @param string $endpoint
* @param int $timeout > 0
* @param int $connectionTimeout >= 0
*
* @throws \InvalidArgumentException if timeout settings are invalid
*/
public function __construct($endpoint)
public function __construct($endpoint, $timeout = 10, $connectionTimeout = 2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added timeout options without BC breaks

{
$this->endpoint = $endpoint;

if (!is_int($timeout) || $timeout < 1) {
throw new \InvalidArgumentException(sprintf(
'Timeout must be an int > 0, got "%s".',
is_object($timeout) ? get_class($timeout) : gettype($timeout).' '.var_export($timeout, true))
);
}

$this->timeout = $timeout;

if (!is_int($connectionTimeout) || $connectionTimeout < 0) {
throw new \InvalidArgumentException(sprintf(
'Connection timeout must be an int >= 0, got "%s".',
is_object($connectionTimeout) ? get_class($connectionTimeout) : gettype($connectionTimeout).' '.var_export($connectionTimeout, true))
);
}

$this->connectionTimeout = $connectionTimeout;
}

/**
Expand All @@ -59,8 +91,8 @@ public function setAuthentication(Common\Authentication $Authentication)
}

/**
* @param $resourceName
* @param $query
* @param string $resourceName
* @param mixed $query
*
* @return string
*/
Expand All @@ -73,14 +105,15 @@ public function getRequestUrl($resourceName, $query)
}
$requestUrl .= '?' . $query;
}

return $requestUrl;
}

/**
* @param $method
* @param $resourceName
* @param null $query
* @param null $body
* @param string $method
* @param string $resourceName
* @param mixed $query
* @param string|null $body
*
* @return array
* @throws Exceptions\HttpException
Expand All @@ -92,17 +125,17 @@ public function performHttpRequest($method, $resourceName, $query = null, $body
$headers = array (
'User-agent: ' . implode(' ', $this->userAgent),
'Accepts: application/json',
"Content-Type: application/json",
"Accept-Charset: utf-8",
sprintf("Authorization: AccessKey %s", $this->Authentication->accessKey)
'Content-Type: application/json',
'Accept-Charset: utf-8',
sprintf('Authorization: AccessKey %s', $this->Authentication->accessKey)
);

curl_setopt($curl, CURLOPT_HTTPHEADER, $headers);
curl_setopt($curl, CURLOPT_HEADER, true);
curl_setopt($curl, CURLOPT_URL, $this->getRequestUrl($resourceName, $query));
curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
curl_setopt($curl, CURLOPT_TIMEOUT, 10);
curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, 2);
curl_setopt($curl, CURLOPT_TIMEOUT, $this->timeout);
curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, $this->connectionTimeout);

if ($method === self::REQUEST_GET) {
curl_setopt($curl, CURLOPT_HTTPGET, true);
Expand All @@ -117,10 +150,11 @@ public function performHttpRequest($method, $resourceName, $query = null, $body
}

// Some servers have outdated or incorrect certificates, Use the included CA-bundle
$caFile = realpath(dirname(__FILE__) . "/../ca-bundle.crt");
$caFile = realpath(__DIR__ . '/../ca-bundle.crt');
if (!file_exists($caFile)) {
throw new Exceptions\HttpException('Unable to find CA-bundle file: ' . $caFile);
throw new Exceptions\HttpException(sprintf('Unable to find CA-bundle file "%s".', __DIR__ . '/../ca-bundle.crt'));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the file does not exists realpath returns false so printing the var. $caFile did not help

}

curl_setopt($curl, CURLOPT_CAINFO, $caFile);

$response = curl_exec($curl);
Expand All @@ -133,11 +167,10 @@ public function performHttpRequest($method, $resourceName, $query = null, $body

// Split the header and body
$parts = explode("\r\n\r\n", $response, 3);
list($responseHeader, $responseBody) = ($parts[0] == 'HTTP/1.1 100 Continue') ? array ($parts[1], $parts[2]) : array ($parts[0], $parts[1]);
list($responseHeader, $responseBody) = ($parts[0] === 'HTTP/1.1 100 Continue') ? array ($parts[1], $parts[2]) : array ($parts[0], $parts[1]);

curl_close($curl);

return array ($responseStatus, $responseHeader, $responseBody);
}

}
21 changes: 10 additions & 11 deletions src/MessageBird/Resources/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
*/
class Base
{

/**
* @var \MessageBird\Common\HttpClient
*/
Expand Down Expand Up @@ -85,7 +84,6 @@ public function create($object, $query = null)
return $this->processRequest($body);
}


public function getList($parameters = array ())
{
list($status, , $body) = $this->HttpClient->performHttpRequest(Common\HttpClient::REQUEST_GET, $this->resourceName, $parameters);
Expand All @@ -108,9 +106,9 @@ public function getList($parameters = array ())
$BaseList->items[] = $Message;
}
return $BaseList;
} else {
return $this->processRequest($body);
}

return $this->processRequest($body);
}

/**
Expand Down Expand Up @@ -143,15 +141,16 @@ public function delete($id)

if ($status === Common\HttpClient::HTTP_NO_CONTENT) {
return true;
} else {
return $this->processRequest($body);
}

return $this->processRequest($body);
}

/**
* @param $body
* @param string $body
*
* @return $this
*
* @throws \MessageBird\Exceptions\RequestException
* @throws \MessageBird\Exceptions\ServerException
*/
Expand All @@ -174,22 +173,22 @@ public function processRequest($body)
/**
* @param $object
* @param $id
*
* @return $this ->Object
* @internal param array $parameters
*
* @internal param array $parameters
*/
public function update($object, $id)
{

$objVars = get_object_vars($object);
$body = array();
foreach ($objVars as $key => $value) {
if (!is_null($value)) {
if (null !== $value) {
$body[$key] = $value;
}
}

$ResourceName = $this->resourceName . (($id) ? '/' . $id : null);
$ResourceName = $this->resourceName . ($id ? '/' . $id : null);
$body = json_encode($body);

list(, , $body) = $this->HttpClient->performHttpRequest(Common\HttpClient::REQUEST_PUT, $ResourceName, false, $body);
Expand Down
35 changes: 35 additions & 0 deletions tests/integration/HttpClientTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php
use MessageBird\Client;
use MessageBird\Common\HttpClient;

class HttpClientTest extends PHPUnit_Framework_TestCase
{
public function testHttpClient()
{
$client = new HttpClient(Client::ENDPOINT);

$url = $client->getRequestUrl('a', null);
$this->assertSame(Client::ENDPOINT.'/a', $url);

$url = $client->getRequestUrl('a', array('b' => 1));
$this->assertSame(Client::ENDPOINT.'/a?b=1', $url);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessageRegExp #^Timeout must be an int > 0, got "integer 0".$#
*/
public function testHttpClientInvalidTimeout()
{
new HttpClient(Client::ENDPOINT, 0);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessageRegExp #^Connection timeout must be an int >= 0, got "stdClass".$#
*/
public function testHttpClientInvalidConnectionTimeout()
{
new HttpClient(Client::ENDPOINT, 10, new \stdClass());
}
}
2 changes: 1 addition & 1 deletion tests/integration/chat/ChatTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class ChatTest extends BaseTest
{
public function setUp()
{
parent::setup();
parent::setUp();
$this->client = new \MessageBird\Client('YOUR_ACCESS_KEY', $this->mockClient);
}

Expand Down