Skip to content

Commit

Permalink
Merge pull request #29 from hirak/feature/improve-scrutinizer-score
Browse files Browse the repository at this point in the history 
Improve scrutinizer score
  • Loading branch information
@hirak
hirak committed Jan 31, 2016
2 parents 290b17c + 42384b2 commit ca95d41
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 52 deletions.
31 changes: 16 additions & 15 deletions src/Aspects/HttpGetRequest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,24 @@
*/
class HttpGetRequest
{
public $origin
, $scheme = 'http'
, $host = 'example.com'
, $port = 80
, $path = '/'
public $origin;
public $scheme = 'http';
public $host = 'example.com';
public $port = 80;
public $path = '/';

, $special = null
public $special = null;

, $query = array()
, $headers = array()
public $query = array();
public $headers = array();

, $curlOpts = array()
public $curlOpts = array();

, $username = null
, $password = null
public $username = null;
public $password = null;

, $maybePublic = true
, $verbose = false
;
public $maybePublic = true;
public $verbose = false;

/**
* normalize url and authentication info
Expand Down Expand Up @@ -63,9 +62,11 @@ public function __construct($origin, $url, IO\IOInterface $io)
public function importURL($url)
{
$struct = parse_url($url);
// @codeCoverageIgnoreStart
if (! $struct) {
throw new \InvalidArgumentException("$url is not valid URL");
}
// @codeCoverageIgnoreEnd

$this->scheme = self::setOr($struct, 'scheme', $this->scheme);
$this->host = self::setOr($struct, 'host', $this->host);
Expand Down Expand Up @@ -105,7 +106,7 @@ public function getCurlOpts()
if ($this->username && $this->password) {
$curlOpts[CURLOPT_USERPWD] = "$this->username:$this->password";
} else {
$curlOpts[CURLOPT_USERPWD] = null;
unset($curlOpts[CURLOPT_USERPWD]);
}

$curlOpts[CURLOPT_URL] = $this->getUrl();
Expand Down
7 changes: 3 additions & 4 deletions src/Aspects/HttpGetResponse.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@

class HttpGetResponse
{
public $errno
, $error
, $info
;
public $errno;
public $error;
public $info;

protected $needAuth = false;

Expand Down
24 changes: 8 additions & 16 deletions src/CurlRemoteFilesystem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class CurlRemoteFilesystem extends Util\RemoteFilesystem
* @param Config $config
* @param array $options
*/
public function __construct(IO\IOInterface $io, Config $config = null, array $options = array())
public function __construct(IO\IOInterface $io, Config $config, array $options = array())
{
$this->io = $io;
$this->config = $config;
Expand Down Expand Up @@ -73,7 +73,7 @@ public function copy($origin, $fileUrl, $fileName, $progress=true, $options=arra
curl_setopt($ch, CURLOPT_RETURNTRANSFER, false);
curl_setopt($ch, CURLOPT_FILE, $outputFile->getPointer());

list($execStatus, $response) = $result = $that->exec($ch, $request);
list(, $response) = $result = $that->exec($ch, $request);

curl_setopt($ch, CURLOPT_FILE, STDOUT);

Expand All @@ -88,7 +88,7 @@ public function copy($origin, $fileUrl, $fileName, $progress=true, $options=arra
/**
* Get the content.
*
* @param string $originUrl The origin URL
* @param string $origin The origin URL
* @param string $fileUrl The file URL
* @param bool $progress Display the progression
* @param array $options Additional context options
Expand Down Expand Up @@ -149,9 +149,6 @@ protected function fetch($origin, $fileUrl, $progress, $options, $exec)
$this->onPreDownload->notify();

$opts = $request->getCurlOpts();
if (empty($opts[CURLOPT_USERPWD])) {
unset($opts[CURLOPT_USERPWD]);
}
$ch = Factory::getConnection($origin, isset($opts[CURLOPT_USERPWD]));

if ($this->pluginConfig['insecure']) {
Expand All @@ -163,7 +160,7 @@ protected function fetch($origin, $fileUrl, $progress, $options, $exec)

curl_setopt_array($ch, $opts);

list($execStatus, $response) = $exec($ch, $request);
list($execStatus, ) = $exec($ch, $request);
} while ($this->_retry);

if ($progress) {
Expand Down Expand Up @@ -195,9 +192,9 @@ public function getLastHeaders()

/**
* @internal
* @param resource<curl> $ch
* @param resource $ch
* @param Aspects\HttpGetRequest $request
* @return array(int, Aspects\HttpGetResponse)
* @return array {int, Aspects\HttpGetResponse}
*/
public function exec($ch, $request)
{
Expand All @@ -222,19 +219,14 @@ public function exec($ch, $request)

/**
* @internal
* @param resource $ch
* @param int $downBytesMax
* @param int $downBytes
* @param int $upBytesMax
* @param int $upBytes
*/
public function progress()
{
// @codeCoverageIgnoreStart
if (PHP_VERSION_ID >= 50500) {
list($ch, $downBytesMax, $downBytes, $upBytesMax, $upBytes) = func_get_args();
list(, $downBytesMax, $downBytes, , ) = func_get_args();
} else {
list($downBytesMax, $downBytes, $upBytesMax, $upBytes) = func_get_args();
list($downBytesMax, $downBytes, , ) = func_get_args();
}
// @codeCoverageIgnoreEnd

Expand Down
30 changes: 14 additions & 16 deletions src/ParallelDownloader.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function download(array $packages, array $pluginConfig)
$this->io->write(" Prefetch start: <comment>success: $this->successCnt, failure: $this->failureCnt, total: $this->totalCnt</comment>");
do {
// prepare curl resources
while ($unused && $packages) {
while (count($unused) > 0 && count($packages) > 0) {
$package = array_pop($packages);
$filepath = $cachedir . DIRECTORY_SEPARATOR . static::getCacheKey($package);
if (file_exists($filepath)) {
Expand All @@ -89,12 +89,13 @@ public function download(array $packages, array $pluginConfig)

// make url
$url = $package->getDistUrl();
$request = new Aspects\HttpGetRequest(parse_url($url, PHP_URL_HOST), $url, $this->io);
$host = parse_url($url, PHP_URL_HOST) ?: '';
$request = new Aspects\HttpGetRequest($host, $url, $this->io);
$request->verbose = $pluginConfig['verbose'];
if (in_array($package->getName(), $pluginConfig['privatePackages'])) {
$request->maybePublic = false;
} else {
$request->maybePublic = preg_match('%^(?:https|git)://github\.com%', $package->getSourceUrl());
$request->maybePublic = (bool)preg_match('%^(?:https|git)://github\.com%', $package->getSourceUrl());
}
$onPreDownload = Factory::getPreEvent($request);
$onPreDownload->notify();
Expand All @@ -113,20 +114,17 @@ public function download(array $packages, array $pluginConfig)
curl_multi_add_handle($mh, $ch);
}

// start multi download
do {
$stat = curl_multi_exec($mh, $running);
} while ($stat === CURLM_CALL_MULTI_PERFORM);

// wait for any event
do {
// start multi download
do {
$stat = curl_multi_exec($mh, $running);
} while ($stat === CURLM_CALL_MULTI_PERFORM);

switch (curl_multi_select($mh, 5)) {
case -1:
usleep(10);
do {
$stat = curl_multi_exec($mh, $running);
} while ($stat === CURLM_CALL_MULTI_PERFORM);
continue 2;
usleep(250);
// fall through
case 0:
continue 2;
default:
Expand Down Expand Up @@ -154,14 +152,14 @@ public function download(array $packages, array $pluginConfig)
curl_multi_remove_handle($mh, $ch);
$unused[] = $ch;
}
} while ($remains);
} while ($remains > 0);

if ($packages) {
if (count($packages) > 0) {
break 2;
}
}
} while ($running);
} while ($packages);
} while (count($packages) > 0);
$this->io->write(" Finished: <comment>success: $this->successCnt, failure: $this->failureCnt, total: $this->totalCnt</comment>");

foreach ($unused as $ch) {
Expand Down
1 change: 0 additions & 1 deletion tests/unit/Aspects/HttpGetRequestTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public function testGetCurlOpts()
CURLOPT_MAXREDIRS => 20,
CURLOPT_ENCODING => 'gzip',
CURLOPT_HTTPHEADER => array(),
CURLOPT_USERPWD => '',
CURLOPT_VERBOSE => false,
);
$curlOpts = $req->getCurlOpts();
Expand Down

0 comments on commit ca95d41

Please sign in to comment.