Add tests for client classes

Author: Alexander Obuhovich

.travis.yml

@@ -17,5 +17,18 @@ php:
 install:
   - composer install --prefer-dist
 
+before_script:
+  - echo "always_populate_raw_post_data = -1" >> /home/travis/.phpenv/versions/5.6.5/etc/php.ini
+  - ~/.phpenv/versions/5.6/bin/php --ini
+  - ~/.phpenv/versions/5.6/bin/php -S localhost:8000 -t $(pwd) > /dev/null 2> /tmp/webserver_output.txt &
+  - export REPO_URL=http://localhost:8000/
+  - curl -I http://localhost:8000/tests/debug_response.php/http_code/201/response_mode/empty/? || true
+  - curl -I http://localhost:8000/tests/debug_response.php/http_code/201/response_mode/empty/ || true
+  - curl -I http://localhost:8000/tests/debug_response.php/http_code/201/ || true
+  - curl -I http://localhost:8000/tests/debug_response.php/ || true
+
 script:
   - ./vendor/bin/phpunit
+
+after_failure:
+    - cat /tmp/webserver_output.txt

CONTRIBUTING.md

@@ -15,7 +15,13 @@ JIRA REST API Client is an open source, community-driven project. If you'd like
 7. Create Pull Request.
 
 ## Running the Tests
-Make sure that you don't break anything with your changes by running:
+
+1. make sure repository is located in web server document root (or it's sub-folder)
+2. copy `phpunit.xml.dist` file into `phpunit.xml` file
+3. in the `phpunit.xml` file:
+ * uncomment part, where `REPO_URL` environment variable is defined
+ * set `REPO_URL` environment variable value to URL, from where repository can be accessed (e.g. `http://localhost/path/to/repository/`)
+4. run following command to make sure that you don't break anything with your changes:
 
 ```bash
 $> phpunit

phpunit.xml.dist

@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<phpunit backupGlobals="true"
+<phpunit bootstrap="vendor/autoload.php"
+         backupGlobals="true"
          backupStaticAttributes="false"
          colors="true"
          convertErrorsToExceptions="true"
@@ -13,13 +14,17 @@
          syntaxCheck="true"
          strict="true"
          verbose="true">
-    <php>
-        <ini name="date.timezone" value="Asia/Tokyo" />
-    </php>
+
     <testsuites>
         <testsuite name="Jira_Api Test Suite">
             <directory>tests</directory>
         </testsuite>
     </testsuites>
 
-</phpunit>
+    <!--
+        <php>
+            <server name="REPO_URL" value="http://localhost/"/>
+        </php>
+    -->
+
+</phpunit>

src/Jira/Api/Client/ClientInterface.php

@@ -26,6 +26,8 @@
 
 
 use chobie\Jira\Api\Authentication\AuthenticationInterface;
+use chobie\Jira\Api\Exception;
+use chobie\Jira\Api\UnauthorizedException;
 
 interface ClientInterface
 {
@@ -42,6 +44,11 @@ interface ClientInterface
 	 * @param boolean                 $debug      Debug this request.
 	 *
 	 * @return array|string
+	 * @throws \InvalidArgumentException When non-supported implementation of AuthenticationInterface is given.
+	 * @throws \InvalidArgumentException When data is not an array and http method is GET.
+	 * @throws Exception When request failed due communication error.
+	 * @throws UnauthorizedException When request failed, because user can't be authorized properly.
+	 * @throws Exception When there was empty response instead of needed data.
 	 */
 	public function sendRequest(
 		$method,

src/Jira/Api/Client/CurlClient.php

@@ -53,11 +53,11 @@ public function __construct()
 	 * @param boolean                 $debug      Debug this request.
 	 *
 	 * @return array|string
-	 * @throws \Exception When non-supported implementation of AuthenticationInterface is given.
-	 * @throws Exception When request failed due CURL error.
+	 * @throws \InvalidArgumentException When non-supported implementation of AuthenticationInterface is given.
+	 * @throws \InvalidArgumentException When data is not an array and http method is GET.
+	 * @throws Exception When request failed due communication error.
 	 * @throws UnauthorizedException When request failed, because user can't be authorized properly.
 	 * @throws Exception When there was empty response instead of needed data.
-	 * @throws \InvalidArgumentException When data is not an array and http method is GET.
 	 */
 	public function sendRequest(
 		$method,
@@ -69,17 +69,20 @@ public function sendRequest(
 		$debug = false
 	) {
 		if ( !($credential instanceof Basic) && !($credential instanceof Anonymous) ) {
-			throw new \Exception(sprintf('CurlClient does not support %s authentication.', get_class($credential)));
+			throw new \InvalidArgumentException(sprintf(
+				'CurlClient does not support %s authentication.',
+				get_class($credential)
+			));
 		}
 
 		$curl = curl_init();
 
 		if ( $method == 'GET' ) {
-			$url .= '?' . http_build_query($data);
-
 			if ( !is_array($data) ) {
 				throw new \InvalidArgumentException('Data must be an array.');
 			}
+
+			$url .= '?' . http_build_query($data);
 		}
 
 		curl_setopt($curl, CURLOPT_URL, $endpoint . $url);
@@ -96,7 +99,7 @@ public function sendRequest(
 		curl_setopt($curl, CURLOPT_VERBOSE, $debug);
 
 		if ( $is_file ) {
-			if ( defined('CURLOPT_SAFE_UPLOAD') ) {
+			if ( defined('CURLOPT_SAFE_UPLOAD') && PHP_VERSION_ID < 70000 ) {
 				curl_setopt($curl, CURLOPT_SAFE_UPLOAD, false);
 			}
 
@@ -117,12 +120,12 @@ public function sendRequest(
 				curl_setopt($curl, CURLOPT_POSTFIELDS, json_encode($data));
 			}
 		}
-		elseif ( $method == 'PUT' ) {
-			curl_setopt($curl, CURLOPT_CUSTOMREQUEST, 'PUT');
+		elseif ( $method == 'PUT' || $method == 'DELETE' ) {
+			curl_setopt($curl, CURLOPT_CUSTOMREQUEST, $method);
 			curl_setopt($curl, CURLOPT_POSTFIELDS, json_encode($data));
 		}
 
-		$data = curl_exec($curl);
+		$response = curl_exec($curl);
 
 		$error_number = curl_errno($curl);
 
@@ -139,15 +142,17 @@ public function sendRequest(
 			throw new UnauthorizedException('Unauthorized');
 		}
 
-		if ( $data === '' && !in_array($http_code, array(201, 204)) ) {
+		if ( $response === '' && !in_array($http_code, array(201, 204)) ) {
 			throw new Exception('JIRA Rest server returns unexpected result.');
 		}
 
-		if ( is_null($data) ) {
+		// @codeCoverageIgnoreStart
+		if ( is_null($response) ) {
 			throw new Exception('JIRA Rest server returns unexpected result.');
 		}
+		// @codeCoverageIgnoreEnd
 
-		return $data;
+		return $response;
 	}
 
 	/**
@@ -160,10 +165,10 @@ public function sendRequest(
 	protected function getCurlValue($file_string)
 	{
 		if ( !function_exists('curl_file_create') ) {
-			return $file_string . '; filename=' . basename($file_string);
+			return $file_string . ';filename=' . basename($file_string);
 		}
 
-		return curl_file_create(substr($file_string, 1), null, basename($file_string));
+		return curl_file_create(substr($file_string, 1), '', basename($file_string));
 	}
 
 }

src/Jira/Api/Client/PHPClient.php

@@ -29,6 +29,7 @@
 use chobie\Jira\Api\Authentication\AuthenticationInterface;
 use chobie\Jira\Api\Authentication\Basic;
 use chobie\Jira\Api\Exception;
+use chobie\Jira\Api\UnauthorizedException;
 
 class PHPClient implements ClientInterface
 {
@@ -40,6 +41,13 @@ class PHPClient implements ClientInterface
 	 */
 	protected $httpsSupport = false;
 
+	/**
+	 * Last error message.
+	 *
+	 * @var string
+	 */
+	private $_lastErrorMessage = '';
+
 	/**
 	 * Create a traditional php client.
 	 */
@@ -57,6 +65,8 @@ public function __construct()
 	 * Returns status of HTTP support.
 	 *
 	 * @return boolean
+	 *
+	 * @codeCoverageIgnore
 	 */
 	protected function isHttpsSupported()
 	{
@@ -75,7 +85,12 @@ protected function isHttpsSupported()
 	 * @param boolean                 $debug      Debug this request.
 	 *
 	 * @return array|string
-	 * @throws \Exception When non-supported implementation of AuthenticationInterface is given.
+	 * @throws \InvalidArgumentException When non-supported implementation of AuthenticationInterface is given.
+	 * @throws \InvalidArgumentException When data is not an array and http method is GET.
+	 * @throws Exception When request failed due communication error.
+	 * @throws UnauthorizedException When request failed, because user can't be authorized properly.
+	 * @throws Exception When there was empty response instead of needed data.
+	 * @throws \InvalidArgumentException When "https" wrapper is not available, but http:// is requested.
 	 */
 	public function sendRequest(
 		$method,
@@ -87,7 +102,10 @@ public function sendRequest(
 		$debug = false
 	) {
 		if ( !($credential instanceof Basic) && !($credential instanceof Anonymous) ) {
-			throw new \Exception(sprintf('PHPClient does not support %s authentication.', get_class($credential)));
+			throw new \InvalidArgumentException(sprintf(
+				'PHPClient does not support %s authentication.',
+				get_class($credential)
+			));
 		}
 
 		$header = array();
@@ -96,18 +114,18 @@ public function sendRequest(
 			$header[] = 'Authorization: Basic ' . $credential->getCredential();
 		}
 
+		if ( !$is_file ) {
+			$header[] = 'Content-Type: application/json;charset=UTF-8';
+		}
+
 		$context = array(
 			'http' => array(
 				'method' => $method,
 				'header' => implode("\r\n", $header),
 			),
 		);
 
-		if ( !$is_file ) {
-			$header[] = 'Content-Type: application/json;charset=UTF-8';
-		}
-
-		if ( $method == 'POST' || $method == 'PUT' ) {
+		if ( $method == 'POST' || $method == 'PUT' || $method == 'DELETE' ) {
 			if ( $is_file ) {
 				$filename = preg_replace('/^@/', '', $data['file']);
 				$boundary = '--------------------------' . microtime(true);
@@ -129,41 +147,84 @@ public function sendRequest(
 			$context['http']['header'] = implode("\r\n", $header);
 			$context['http']['content'] = $__data;
 		}
-		else {
+		elseif ( $method == 'GET' ) {
+			if ( !is_array($data) ) {
+				throw new \InvalidArgumentException('Data must be an array.');
+			}
+
 			$url .= '?' . http_build_query($data);
 		}
 
+		// @codeCoverageIgnoreStart
 		if ( strpos($endpoint, 'https://') === 0 && !$this->isHttpsSupported() ) {
-			throw new \Exception('does not support https wrapper. please enable openssl extension');
+			throw new \InvalidArgumentException('does not support https wrapper. please enable openssl extension');
+		}
+		// @codeCoverageIgnoreEnd
+
+		list ($http_code, $response, $error_message) = $this->doSendRequest($endpoint . $url, $context);
+
+		// Check for 401 code before "$error_message" checking, because it's considered as an error.
+		if ( $http_code == 401 ) {
+			throw new UnauthorizedException('Unauthorized');
+		}
+
+		if ( !empty($error_message) ) {
+			throw new Exception(
+				sprintf('Jira request failed: "%s"', $error_message)
+			);
+		}
+
+		if ( $response === '' && !in_array($http_code, array(201, 204)) ) {
+			throw new Exception('JIRA Rest server returns unexpected result.');
 		}
 
+		// @codeCoverageIgnoreStart
+		if ( is_null($response) ) {
+			throw new Exception('JIRA Rest server returns unexpected result.');
+		}
+		// @codeCoverageIgnoreEnd
+
+		return $response;
+	}
+
+	/**
+	 * Sends the request.
+	 *
+	 * @param string $url     URL.
+	 * @param array  $context Context.
+	 *
+	 * @return array
+	 */
+	protected function doSendRequest($url, array $context)
+	{
+		$this->_lastErrorMessage = '';
+
 		set_error_handler(array($this, 'errorHandler'));
-		$data = file_get_contents(
-			$endpoint . $url,
-			false,
-			stream_context_create($context)
-		);
+		$response = file_get_contents($url, false, stream_context_create($context));
 		restore_error_handler();
 
-		if ( is_null($data) ) {
-			throw new \Exception('JIRA Rest server returns unexpected result.');
+		if ( isset($http_response_header) ) {
+			preg_match('#HTTP/\d+\.\d+ (\d+)#', $http_response_header[0], $matches);
+			$http_code = $matches[1];
+		}
+		else {
+			$http_code = 0;
 		}
 
-		return $data;
+		return array($http_code, $response, $this->_lastErrorMessage);
 	}
 
 	/**
-	 * Throws exception on error.
+	 * Remembers last error.
 	 *
 	 * @param integer $errno  Error number.
 	 * @param string  $errstr Error message.
 	 *
 	 * @return void
-	 * @throws \Exception Always.
 	 */
 	public function errorHandler($errno, $errstr)
 	{
-		throw new \Exception($errstr);
+		$this->_lastErrorMessage = $errstr;
 	}
 
 }