6
\$\begingroup\$

This code review request relates to this code review which covers the basic single REST call use case in this REST client library.

This code review covers the classes and unit tests for the multiple parallel REST call functionality provided by the library, which leverage the PHP cURL extension's curl_multi_* functionality.

You might find it useful to read the library's README file before performing this review for further background and usage examples, which I have omitted here for brevity and to allow the question to focus on the code.

RestMultiClient class

<?php

namespace MikeBrant\RestClientLib;

/**
* @desc Class which extendd RestClient to provide curl_multi capabilities, allowing for multiple REST calls to be made in parallel.
*/
class RestMultiClient extends RestClient
{
    /**
     * Store array of curl handles for multi_exec
     * 
     * @var array
     */
    private $curlHandles = array();
    
    /**
     * Stores curl multi handle to which individual handles in curlHandles are added
     * 
     * @var mixed
     */
    private $curlMultiHandle = null;
    
    /**
     * Variable to store the maximum number of handles to be used for curl_multi_exec
     * 
     * @var integer
     */
    private $maxHandles = 10;
    
    /**
     * Variable to store an array of request headers sent in a multi_exec request
     * 
     * @var array
     */
    private $requestHeaders = array();
    
    /**
     * Variable to store an array of request data sent for multi_exec POST/PUT requests.
     * 
     * @var array
     */
    private $requestDataArray = array();
     
    /**
     * Variable to store CurlMultiHttpResponse object
     * 
     * @var CurlMultiHttpResponse
     */
    private $curlMultiHttpResponse = null;
    
    /**
     * Constructor method. Currently there is no instantiation logic.
     * 
     * @return void
     */
    public function __construct() {}
    
    /**
     * Method to perform multiple GET actions using curl_multi_exec.
     * 
     * @param array $actions
     * @param integer $maxHandles
     * @return RestMultiClient
     * @throws \Exception
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    public function get($actions) {
        $this->validateActionArray($actions);
        
        // set up curl handles
        $this->curlMultiSetup(count($actions));
        $this->setRequestUrls($actions);
        foreach($this->curlHandles as $curl) {
            curl_setopt($curl, CURLOPT_HTTPGET, true); // explicitly set the method to GET    
        }
        $this->curlMultiExec();
        
        return $this->curlMultiHttpResponse;
    }
    
    /**
     * Method to perform multiple POST actions using curl_multi_exec.
     * 
     * @param array $actions
     * @param array $data
     * @return RestMultiClient
     * @throws \Exception
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    public function post($actions, $data) {
        $this->validateActionArray($actions);
        $this->validateDataArray($data);
        // verify that the number of data elements matches the number of action elements
        if (count($actions) !== count($data)) {
            throw new \LengthException('The number of actions requested does not match the number of data elements provided.'); 
        }
        
        // set up curl handles
        $this->curlMultiSetup(count($actions));
        $this->setRequestUrls($actions);
        $this->setRequestDataArray($data);
        foreach($this->curlHandles as $curl) {
            curl_setopt($curl, CURLOPT_POST, true); // explicitly set the method to POST 
        }
        $this->curlMultiExec();
        
        return $this->curlMultiHttpResponse;
    }
    
    /**
     * Method to perform multiple PUT actions using curl_multi_exec.
     * 
     * @param array $actions
     * @param array $data
     * @return RestMultiClient
     * @throws \Exception
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    public function put($actions, $data) {
        $this->validateActionArray($actions);
        $this->validateDataArray($data);
        // verify that the number of data elements matches the number of action elements
        if (count($actions) !== count($data)) {
            throw new \LengthException('The number of actions requested does not match the number of data elements provided.'); 
        }
        
        // set up curl handles
        $this->curlMultiSetup(count($actions));
        $this->setRequestUrls($actions);
        $this->setRequestDataArray($data);
        foreach($this->curlHandles as $curl) {
            curl_setopt($curl, CURLOPT_CUSTOMREQUEST, 'PUT'); // explicitly set the method to PUT 
        }
        $this->curlMultiExec();
        
        return $this->curlMultiHttpResponse;
    }
    
    /**
     * Method to perform multiple DELETE actions using curl_multi_exec.
     * 
     * @param array $actions
     * @param integer $maxHandles
     * @return RestMultiClient
     * @throws \Exception
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    public function delete($actions) {
        $this->validateActionArray($actions);
        
        // set up curl handles
        $this->curlMultiSetup(count($actions));
        $this->setRequestUrls($actions);
        foreach($this->curlHandles as $curl) {
            curl_setopt($curl, CURLOPT_CUSTOMREQUEST, 'DELETE'); // explicitly set the method to DELETE
        }
        $this->curlMultiExec();
        
        return $this->curlMultiHttpResponse;
    }
    
    /**
     * Method to perform multiple HEAD actions using curl_multi_exec.
     * 
     * @param array $actions
     * @return RestMultiClient
     * @throws \Exception
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    public function head($actions) {
        $this->validateActionArray($actions);
        
        // set up curl handles
        $this->curlMultiSetup(count($actions));
        $this->setRequestUrls($actions);
        foreach($this->curlHandles as $curl) {
            curl_setopt($curl, CURLOPT_CUSTOMREQUEST, 'HEAD');
            curl_setopt($curl, CURLOPT_NOBODY, true);
        }
        $this->curlMultiExec();
        
        return $this->curlMultiHttpResponse;
    }
    
    /**
     * Sets maximum number of handles that will be instantiated for curl_multi_exec calls
     * 
     * @param integer $maxHandles
     * @return RestMultiClient
     * @throws \InvalidArgumentException
     */
    public function setMaxHandles($maxHandles) {
        if (!is_integer($maxHandles) || $maxHandles <= 0) {
            throw new \InvalidArgumentException('A non-integer value was passed for max_handles parameter.');     
        }
        $this->maxHandles = $maxHandles;
        
        return $this->curlMultiHttpResponse;
    }
    
    /**
     * Getter for maxHandles setting
     * 
     * @return integer
     */
    public function getMaxHandles() {
        return $this->maxHandles;
    }
    
    /**
     * Method to set up a given number of curl handles for use with curl_multi_exec
     * 
     * @param integer $handlesNeeded
     * @return void
     * @throws \Exception
     */
    private function curlMultiSetup($handlesNeeded) {
        $multiCurl = curl_multi_init();
        if($multiCurl === false) {
            throw new \Exception('multi_curl handle failed to initialize.');
        }
        $this->curlMultiHandle = $multiCurl;
        
        for($i = 0; $i < $handlesNeeded; $i++) {
            $curl = $this->curlInit();
            $this->curlHandles[$i] = $curl;
            curl_multi_add_handle($this->curlMultiHandle, $curl);
        }
    }
    
    /**
     * Method to reset the curlMultiHandle and all individual curlHandles related to it.
     * 
     * @return void
     */
    private function curlMultiTeardown() {
        foreach ($this->curlHandles as $curl) {
            curl_multi_remove_handle($this->curlMultiHandle, $curl);
            $this->curlClose($curl);
        }
        curl_multi_close($this->curlMultiHandle);
        $this->curlHandles = array();
        $this->curlMultiHandle = null;
    }
    
    /**
     * Method to execute curl_multi call
     * 
     * @return void
     * @throws \Exception
     */
    private function curlMultiExec() {
        // start multi_exec execution
        do {
            $status = curl_multi_exec($this->curlMultiHandle, $active);
        } while ($status === CURLM_CALL_MULTI_PERFORM || $active);
        
        // see if there are any errors on the multi_exec call as a whole
        if($status !== CURLM_OK) {
            throw new \Exception('curl_multi_exec failed with status "' . $status . '"');
        }
        
        // process the results. Note there could be individual errors on specific calls
        $this->curlMultiHttpResponse = new CurlMultiHttpResponse();
        foreach($this->curlHandles as $i => $curl) {
            try {
                $response = new CurlHttpResponse(
                    curl_multi_getcontent($curl),
                    curl_getinfo($curl)
                );
            } catch (\InvalidArgumentException $e) {
                $this->curlMultiTeardown();
                throw new \Exception(
                   'Unable to instantiate CurlHttpResponse. Message: "' . $e->getMessage() . '"',
                   $e->getCode(),
                   $e
                );
            }
            $this->curlMultiHttpResponse->addResponse($response);
        }
        $this->curlMultiTeardown();
    }
    
    /**
     * Method to reset all properties specific to a particular request/response sequence.
     * 
     * @return void
     */
    protected function resetRequestResponseProperties() {
        $this->$curlMultiHttpResponse = null;
        $this->requestHeaders = array();
        $this->requestDataArray = array();
    }
    
    /**
     * Method to set the urls for  multi_exec action
     * 
     * @param array $actions
     * @return void
     */
    private function setRequestUrls(array $actions) {
        for ($i = 0; $i < count($actions); $i++) {
            $url = $this->buildUrl($actions[$i]);
            $this->requestUrls[$i] = $url;
            curl_setopt($this->curlHandles[$i], CURLOPT_URL, $url);
        }   
    }
    
    /**
     * Method to set array of data to be sent along with multi_exec POST/PUT requests
     * 
     * @param array $data
     * @return void
     */
    private function setRequestDataArray(array $data) {
        for ($i = 0; $i < count($data); $i++) {
            $data = $data[$i];
            $this->requestDataArray[$i] = $data;
            curl_setopt($this->curlHandles[$i], CURLOPT_POSTFIELDS, $data);
        }
    }
    
    /**
     * Method to provide common validation for action array parameters
     * 
     * @param array $actions
     * @return void
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    private function validateActionArray(array $actions) {
        if(empty($actions)) {
            throw new \InvalidArgumentException('An empty array was passed for actions parameter.');
        }
        if(count($actions) > $this->maxHandles) {
            throw new \LengthException('Length of actions array exceeds maxHandles setting.');
        }
        foreach($actions as $action) {
            $this->validateAction($action);
        }
    }
    
    /**
     * Method to provide common validation for data array parameters
     * 
     * @param array $data
     * @return void
     * @throws \InvalidArgumentException
     * @throws \LengthException
     */
    private function validateDataArray(array $data) {
        if(empty($data)) {
            throw new \InvalidArgumentException('An empty array was passed for data parameter');
        }
        if(count($data) > $this->maxHandles) {
            throw new \LengthException('Length of data array exceeds maxHandles setting.');
        }
        foreach($data as $item) {
            $this->validateData($item);
        }
    }
}

RestMultiClient unit tests

<?php

namespace MikeBrant\RestClientLib;

use PHPUnit\Framework\TestCase;

/**
 * Mock for curl_multi_init global function
 * 
 * @return mixed
 */
function curl_multi_init() {
    if (!is_null(RestMultiClientTest::$curlMultiInitResponse)) {
        return RestMultiClientTest::$curlMultiInitResponse;
    }
    return \curl_multi_init();
}

/**
 * Mock for curl_multi_exec global function
 * 
 * @param resource curl_multi handle
 * @param integer flag indicating if there are still active handles.
 * @return integer
 */
function curl_multi_exec($multiCurl, &$active) {
    if (is_null(RestMultiClientTest::$curlMultiExecResponse)) {
        return \curl_multi_exec($multiCurl, $active);
    }
    $active = 0;
    return RestMultiClientTest::$curlMultiExecResponse;
}

/**
 * Mock for curl_multi_getcontent global function
 * 
 * @param resource curl handle
 * @return string
 */
function curl_multi_getcontent($curl) {
    if (!is_null(RestMultiClientTest::$curlMultiGetcontentResponse)) {
        return RestMultiClientTest::$curlMultiGetcontentResponse;
    }
    return \curl_multi_getcontent($curl);
}

/**
 * This is hacky workaround for avoiding double definition of this global method override
 * when running full test suite on this library.
 */
if(!function_exists('\MikeBrant\RestClientLib\curl_getinfo')) {

    /**
     * Mock for curl_getinfo function
     * 
     * @param resource curl handle
     * @return mixed
     */
    function curl_getinfo($curl) {
        $backtrace = debug_backtrace();
        $testClass = $backtrace[1]['class'] . 'Test';
        if (!is_null($testClass::$curlGetinfoResponse)) {
            return $testClass::$curlGetinfoResponse;
        }
        return \curl_getinfo($curl);
    }
}

class RestMultiClientTest extends TestCase
{
    public static $curlMultiInitResponse = null;
    
    public static $curlMultiExecResponse = null;
    
    public static $curlMultiGetcontentResponse = null;
    
    public static $curlGetinfoResponse = null;
    
    protected $client = null;
    
    protected $curlMultiExecFailedResponse = CURLM_INTERNAL_ERROR;
    
    protected $curlMultiExecCompleteResponse = CURLM_OK;
    
    protected $curlGetinfoMockResponse = array(
        'url' => 'http://google.com/',
        'content_type' => 'text/html; charset=UTF-8',
        'http_code' => 200,
        'header_size' => 321,
        'request_size' => 49,
        'filetime' => -1,
        'ssl_verify_result' => 0,
        'redirect_count' => 0,
        'total_time' => 1.123264,
        'namelookup_time' => 1.045272,
        'connect_time' => 1.070183,
        'pretransfer_time' => 1.071139,
        'size_upload' => 0,
        'size_download' => 219,
        'speed_download' => 194,
        'speed_upload' => 0,
        'download_content_length' => 219,
        'upload_content_length' => -1,
        'starttransfer_time' => 1.122377,
        'redirect_time' => 0,
        'redirect_url' => 'http://www.google.com/',
        'primary_ip' => '216.58.194.142',
        'certinfo' => array(),
        'primary_port' => 80,
        'local_ip' => '192.168.1.74',
        'local_port' => 59733,
        'request_header' => "GET / HTTP/1.1\nHost: google.com\nAccept: */*",
    );
    
    protected function setUp() {
        self::$curlMultiInitResponse = null;
        self::$curlMultiExecResponse = null;
        self::$curlMultiGetcontentResponse = null;
        self::$curlGetinfoResponse = null;
        $this->client = new RestMultiClient();
    }
    
    protected function tearDown() {
        $this->client = null;
    }
    
    /**
     * @expectedException \InvalidArgumentException
     * @covers MikeBrant\RestClientLib\RestMultiClient::validateActionArray
     */
    public function testValidateActionArrayThrowsExceptionOnEmptyArray() {
        $this->client->get(array());
    }
    
    /**
     * @expectedException \LengthException
     * @covers MikeBrant\RestClientLib\RestMultiClient::validateActionArray
     */
    public function testValidateActionArrayThrowsExceptionOnOversizedArray() {
        $maxHandles = $this->client->getMaxHandles();
        $this->client->get(
            array_fill(0, $maxHandles + 1, 'action')
        );
    }
    
    /**
     * @expectedException \InvalidArgumentException
     * @covers MikeBrant\RestClientLib\RestMultiClient::validateDataArray
     */
    public function testValidateDataArrayThrowsExceptionOnEmptyArray() {
        $this->client->get(array());
    }
    
    /**
     * @expectedException \LengthException
     * @covers MikeBrant\RestClientLib\RestMultiClient::validateDataArray
     */
    public function testValidateDataArrayThrowsExceptionOnOversizedArray() {
        $maxHandles = $this->client->getMaxHandles();
        $this->client->post(
            array_fill(0, $maxHandles, 'action'),
            array_fill(0, $maxHandles + 1, 'data')
        );
    }
    
    /**
     * @expectedException \Exception
     * @covers MikeBrant\RestClientLib\RestMultiClient::curlMultiSetup
     */
    public function testCurlMultiSetupThrowsExceptionOnCurlMultiInitFailure() {
        self::$curlMultiInitResponse = false;
        $this->client->get(
            array_fill(0, 2, 'action')
        );
    }
    /**
     * @expectedException \Exception
     * @covers MikeBrant\RestClientLib\RestMultiClient::curlMultiExec
     */
    public function testCurlMultiExecThrowsExceptionOnMultiCurlFailure() {
        self::$curlMultiExecResponse = $this->curlMultiExecFailedResponse;
        $this->client->get(
            array_fill(0, 2, 'action')
        );
    }
    
    /**
     * @expectedException \Exception
     * @covers MikeBrant\RestClientLib\RestMultiClient::curlMultiExec
     */
    public function testCurlMultiExecThrowsExceptionOnMalformedCurlHttpResponse() {
        self::$curlMultiExecResponse = $this->curlMultiExecCompleteResponse;
        self::$curlMultiGetcontentResponse = 'test';
        self::$curlGetinfoResponse = array();
        $this->client->get(
            array_fill(0, 2, 'action')
        );
    }
    /**
     * @covers MikeBrant\RestClientLib\RestMultiClient::get
     * @covers MikeBrant\RestClientLib\RestMultiClient::validateActionArray
     * @covers MikeBrant\RestClientLib\RestMultiClient::curlMultiSetup
     * @covers MikeBrant\RestClientLib\RestMultiClient::resetRequestResponseProperties
     * @covers MikeBrant\RestClientLib\RestMultiClient::setRequestUrls
     * @covers MikeBrant\RestClientLib\RestMultiClient::curlMultiExec
     * @covers MikeBrant\RestClientLib\RestMultiClient::curlMultiTeardown
     */
    public function testGet() {
        self::$curlMultiExecResponse = $this->curlMultiExecCompleteResponse;
        self::$curlMultiGetcontentResponse = 'test';
        self::$curlGetinfoResponse = $this->curlGetinfoMockResponse;
        $response = $this->client->get(
            array_fill(0, 2, 'action')
        );
        $this->assertInstanceOf(CurlMultiHttpResponse::class, $response);
        $this->assertAttributeEquals(null, 'curlMultiHandle', $this->client);
    }
    
    /**
     * @expectedException \LengthException
     * @covers MikeBrant\RestClientLib\RestMultiClient::post
     */
    public function testPostThrowsExceptionOnArraySizeMismatch() {
        $maxHandles = $this->client->getMaxHandles();
        $this->client->post(
            array_fill(0, $maxHandles, 'action'),
            array_fill(0, $maxHandles - 1, 'data')
        );
    }
    
    /**
     * @covers MikeBrant\RestClientLib\RestMultiClient::post
     * @covers MikeBrant\RestClientLib\RestMultiClient::validateData
     * @covers MikeBrant\RestClientLib\RestMultiClient::setRequestData
     */
    public function testPost() {
        self::$curlMultiExecResponse = $this->curlMultiExecCompleteResponse;
        self::$curlMultiGetcontentResponse = 'test';
        self::$curlGetinfoResponse = $this->curlGetinfoMockResponse;
        $response = $this->client->post(
            array_fill(0, 2, 'action'),
            array_fill(0, 2, 'data')
        );
        $this->assertInstanceOf(CurlMultiHttpResponse::class, $response);
        $this->assertAttributeEquals(null, 'curlMultiHandle', $this->client);
   }
    
    /**
     * @expectedException \LengthException
     * @covers MikeBrant\RestClientLib\RestMultiClient::put
     */
    public function testPutThrowsExceptionOnArraySizeMismatch() {
        $maxHandles = $this->client->getMaxHandles();
        $this->client->put(
            array_fill(0, $maxHandles, 'action'),
            array_fill(0, $maxHandles - 1, 'data')
        );
    }
    
    /**
     * @covers MikeBrant\RestClientLib\RestMultiClient::put
     */
    public function testPut() {
        self::$curlMultiExecResponse = $this->curlMultiExecCompleteResponse;
        self::$curlMultiGetcontentResponse = 'test';
        self::$curlGetinfoResponse = $this->curlGetinfoMockResponse;
        $response = $this->client->put(
            array_fill(0, 2, 'action'),
            array_fill(0, 2, 'data')
        );
        $this->assertInstanceOf(CurlMultiHttpResponse::class, $response);
        $this->assertAttributeEquals(null, 'curlMultiHandle', $this->client);
    }
    
    /**
     * @covers MikeBrant\RestClientLib\RestMultiClient::delete
     */
    public function testDelete() {
        self::$curlMultiExecResponse = $this->curlMultiExecCompleteResponse;
        self::$curlMultiGetcontentResponse = 'test';
        self::$curlGetinfoResponse = $this->curlGetinfoMockResponse;
        $response = $this->client->delete(
            array_fill(0, 2, 'action')
        );
        $this->assertInstanceOf(CurlMultiHttpResponse::class, $response);
        $this->assertAttributeEquals(null, 'curlMultiHandle', $this->client);
    }
    
    /**
     * @covers MikeBrant\RestClientLib\RestMultiClient::head
     */
    public function testHead() {
        self::$curlMultiExecResponse = $this->curlMultiExecCompleteResponse;
        self::$curlMultiGetcontentResponse = 'test';
        self::$curlGetinfoResponse = $this->curlGetinfoMockResponse;
        $response = $this->client->head(
            array_fill(0, 2, 'action')
        );
        $this->assertInstanceOf(CurlMultiHttpResponse::class, $response);
        $this->assertAttributeEquals(null, 'curlMultiHandle', $this->client);
    }
}

CurlMultiHttpResponse class

<?php

namespace MikeBrant\RestClientLib;

class CurlMultiHttpResponse
{
    /**
     * Variable to store individual CurlHttpResponse objects from curl_multi call
     * 
     * @var array
     */
    protected $curlHttpResponses = array();
    
    /**
     * Constructor method. Currently there is no instantiation logic.
     */
    public function __construct() {}
    
    /**
     * Method to add CurlHttpResponse object to collection
     * 
     * @param CurlHttpResponse $response
     * @return void
     */
    public function addResponse(CurlHttpResponse $response) {
        $this->curlHttpResponses[] = $response;
    }
    
    /**
     * Returns array of all CurlHttpResponse objects in collection.
     * 
     * @return array
     */
    public function getCurlHttpResponses() {
        return $this->curlHttpResponses;
    }
    
    /**
     * Alias for getCurlHttpResponses
     * 
     * @return array
     */
    public function getAll() {
        return $this->getCurlHttpResponses();
    }
    
    /**
     * Returns array of response bodies for each response in collection.
     * 
     * @return array
     */
    public function getResponseBodies() {
        return array_map(
            function(CurlHttpResponse $value) {
                return $value->getBody();
            },
            $this->curlHttpResponses
        );
    }
    
    /**
     * Returns array of response codes for each response in collection.
     * 
     * @return array
     */
    public function getHttpCodes() {
        return array_map(
            function(CurlHttpResponse $value) {
                return $value->getHttpCode();
            },
            $this->curlHttpResponses
        );
    }
    
    /**
     * Returns array of URL's used for each response in collectoin as returned via curl_getinfo.
     * 
     * @return array
     */
    public function getRequestUrls() {
        return array_map(
            function(CurlHttpResponse $value) {
                return $value->getRequestUrl();
            },
            $this->curlHttpResponses
        );
    }
    
    /**
     * Returns array of request headers for each response in collection as returned via curl_getinfo.
     * 
     * @return array
     */
    public function getRequestHeaders() {
        return array_map(
            function(CurlHttpResponse $value) {
                return $value->getRequestHeader();
            },
            $this->curlHttpResponses
        );
    }
    
    /**
     * Returns array of curl_getinfo arrays for each response in collection.
     * See documentation at http://php.net/manual/en/function.curl-getinfo.php for expected format for each array element.
     * 
     * @return array
     */
    public function getCurlGetinfoArrays() {
        return array_map(
            function(CurlHttpResponse $value) {
                return $value->getCurlGetinfo();
            },
            $this->curlHttpResponses
        );
    }
}

CurlMultiHttpResponse unit tests

<?php

namespace MikeBrant\RestClientLib;

use PHPUnit\Framework\TestCase;

class CurlMultiHttpResponseTest extends TestCase
{
    protected $curlMultiHttpResponse = null;
    
    protected $curlExecMockResponse = 'Test Response';
    
    protected $curlGetinfoMockResponse = array(
        'url' => 'http://google.com/',
        'content_type' => 'text/html; charset=UTF-8',
        'http_code' => 200,
        'header_size' => 321,
        'request_size' => 49,
        'filetime' => -1,
        'ssl_verify_result' => 0,
        'redirect_count' => 0,
        'total_time' => 1.123264,
        'namelookup_time' => 1.045272,
        'connect_time' => 1.070183,
        'pretransfer_time' => 1.071139,
        'size_upload' => 0,
        'size_download' => 219,
        'speed_download' => 194,
        'speed_upload' => 0,
        'download_content_length' => 219,
        'upload_content_length' => -1,
        'starttransfer_time' => 1.122377,
        'redirect_time' => 0,
        'redirect_url' => 'http://www.google.com/',
        'primary_ip' => '216.58.194.142',
        'certinfo' => array(),
        'primary_port' => 80,
        'local_ip' => '192.168.1.74',
        'local_port' => 59733,
        'request_header' => "GET / HTTP/1.1\nHost: google.com\nAccept: */*",
    );
    
    protected function setUp() {
        $this->curlMultiHttpResponse = new CurlMultiHttpResponse();
    }
    
    public function curlHttpResponseProvider() {
        return array(
            array(
                new CurlHttpResponse($this->curlExecMockResponse, $this->curlGetinfoMockResponse)
            )
        );
    }
    
    /**
     * @dataProvider curlHttpResponseProvider
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::addResponse
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getCurlHttpResponses
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getAll
     */
    public function testAddResponse($curlHttpResponse) {
        $responseArray = array_fill(0, 5, $curlHttpResponse);
        for ($i = 0; $i < count($responseArray); $i++) {
            $this->curlMultiHttpResponse->addResponse($curlHttpResponse);
        }
        $this->assertEquals($responseArray, $this->curlMultiHttpResponse->getCurlHttpResponses());
        $this->assertEquals($responseArray, $this->curlMultiHttpResponse->getAll());
    }
    
    /**
     * @dataProvider curlHttpResponseProvider
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getResponseBodies
     */
    public function testGetRepsonseBodies($curlHttpResponse) {
        $responseArray = array_fill(0, 5, $curlHttpResponse);
        for ($i = 0; $i < count($responseArray); $i++) {
            $this->curlMultiHttpResponse->addResponse($curlHttpResponse);
        }
        $responseBodies = array_map(
            function($val) {
                return $val->getBody();
            },
            $responseArray
        );
        $this->assertEquals($responseBodies, $this->curlMultiHttpResponse->getResponseBodies());
    }
    
    /**
     * @dataProvider curlHttpResponseProvider
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getHttpCodes
     */
    public function testgetHttpCodes($curlHttpResponse) {
        $responseArray = array_fill(0, 5, $curlHttpResponse);
        for ($i = 0; $i < count($responseArray); $i++) {
            $this->curlMultiHttpResponse->addResponse($curlHttpResponse);
        }
        $responseCodes = array_map(
            function($val) {
                return $val->getHttpCode();
            },
            $responseArray
        );
        $this->assertEquals($responseCodes, $this->curlMultiHttpResponse->getHttpCodes());
    }
    
    /**
     * @dataProvider curlHttpResponseProvider
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getRequestUrls
     */
    public function testGetRequestUrls($curlHttpResponse) {
        $responseArray = array_fill(0, 5, $curlHttpResponse);
        for ($i = 0; $i < count($responseArray); $i++) {
            $this->curlMultiHttpResponse->addResponse($curlHttpResponse);
        }
        $requestUrls = array_map(
            function($val) {
                return $val->getRequestUrl();
            },
            $responseArray
        );
        $this->assertEquals($requestUrls, $this->curlMultiHttpResponse->getRequestUrls());
    }
    
    /**
     * @dataProvider curlHttpResponseProvider
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getRequestHeaders
     */
    public function testGetRequestHeaders($curlHttpResponse) {
        $responseArray = array_fill(0, 5, $curlHttpResponse);
        for ($i = 0; $i < count($responseArray); $i++) {
            $this->curlMultiHttpResponse->addResponse($curlHttpResponse);
        }
        $requestHeaders = array_map(
            function($val) {
                return $val->getRequestHeader();
            },
            $responseArray
        );
        $this->assertEquals($requestHeaders, $this->curlMultiHttpResponse->getRequestHeaders());
    }
    
    /**
     * @dataProvider curlHttpResponseProvider
     * @covers MikeBrant\RestClientLib\CurlMultiHttpResponse::getCurlGetinfoArrays
     */
    public function testGetCurlGetinfoArrays($curlHttpResponse) {
        $responseArray = array_fill(0, 5, $curlHttpResponse);
        for ($i = 0; $i < count($responseArray); $i++) {
            $this->curlMultiHttpResponse->addResponse($curlHttpResponse);
        }
        $requestInfoArrays = array_map(
            function($val) {
                return $val->getCurlGetinfo();
            },
            $responseArray
        );
        $this->assertEquals($requestInfoArrays, $this->curlMultiHttpResponse->getCurlGetinfoArrays());
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Review

Just as is the case with the code in rounds one and two, this code looks very thorough, easy-to-read, well-tested and well-documented. There aren't many suggestions I have to offer but please take a look and see if you like them.

Suggestions

Mapping method return on array elements

As noted in my review of the round 2 code the methods corresponding to the HTTP verbs are a bit repetitive. The methods in class CurlMultiHttpResponse to call getter methods on each request are also quite redundant. As you likely know array_map() doesn't have a straight-forward way of specifying the callback be a method of the instances in the array.

In the class CurlMultiHttpResponse a method could be created to return a function that will accept a method name as a parameter and return a function that calls the method on an item:


public static function mapMethod(string $method) 

{
    
    return function($item) use ($method)
    {
        
        return $item->$method();
    
    };

}

or for PHP 7.4 or later an arrow function can be used:


public static function mapMethod(string $method) 

{
    
    return fn ($item) => $item->$method();


}

This can be used in conjunction with array_map() . For example, instead of

public function getHttpCodes() {
       return array_map(
           function(CurlHttpResponse $value) {
               return $value->getHttpCode();
           },
           $this->curlHttpResponses
       );
   }   

It can be as simple as:

public function getHttpCodes()
{
 
    return array_map(self::mapMethod('getHttpCode'),
 $this->curlHttpResponses
);

}

Though that depends on the method to map the method calls being defined on the class. Instead of doing that, the __call() method could be defined like below. This allows the removal of method definitions like getCurlGetinfoArrays(), getHttpCodes(), getRequestHeaders(), getRequestUrls(), getResponseBodies()

private const METHOD_MAPPING = [ 
   
    'getCurlGetinfoArrays' => 'getCurlGetinfo',
    
    'getResponseBodies' => 'getBody'

];

public function __call($method, $arguments) 
{

    $mapMethod = substr($method, 0, -1);

    if (isset(self::METHOD_MAPPING[$method])) {

        $mapMethod = self::METHOD_MAPPING[$method];

    }

    if (!method_exists(CurlHttpResponse::class, $mapMethod)) {

        throw new \BadFunctionCallException("method $mapMethod does not exist on response class");

    }

    return array_map(function($item) use ($mapMethod)
 {

        return $item->$mapMethod();

    },
    $this->curlHttpResponses);

}

Or for PHP 7.4 or later the arrow function can be used also for the return statement:

    return array_map(fn ($item) => $item->$mapMethod(), $this->curlHttpResponses);
}

Properties vs. Constants

Some properties are not re-assigned -e.g. in CurlMultiHttpResponseTest property $curlExecMockResponse so those could be made constants.

Nit-pick: typo in doc-block

There is a typo on the third word in the doc-block for class RestMultiClient:

/**
* @desc Class which extendd RestClient to provide curl_multi capabilities, allowing for multiple REST calls to be made in parallel.
*/
class RestMultiClient extends RestClient
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.