From 8087580069e4b0de7807d58ecf8b024e8419a767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Tue, 8 Jan 2019 15:47:29 +0100 Subject: [PATCH 1/4] Fixed all errors from phpqa tools --- .gitignore | 1 + .php_cs.dist | 1 + .travis.yml | 4 +- src/Clients/GuzzleClient.php | 6 +- src/Contracts/CircuitBreaker.php | 12 ++- src/Contracts/Client.php | 6 +- src/Contracts/Factory.php | 4 +- src/Contracts/Place.php | 6 +- src/Contracts/Storage.php | 6 ++ src/Contracts/Transaction.php | 10 +-- src/Exceptions/InvalidPlace.php | 4 +- src/Exceptions/InvalidTransaction.php | 4 +- src/Places/AbstractPlace.php | 67 ++++++++------ src/SimpleCircuitBreaker.php | 95 ++++++++++++-------- src/SimpleCircuitBreakerFactory.php | 7 +- src/States.php | 2 +- src/Storages/SimpleArray.php | 11 ++- src/Storages/SymfonyCache.php | 14 +-- src/Transactions/SimpleTransaction.php | 59 ++++++------ src/Transitions.php | 2 +- src/Utils/Assert.php | 12 +-- src/Utils/ErrorFormatter.php | 6 +- tests/Clients/GuzzleClientTest.php | 2 +- tests/Exceptions/InvalidPlaceTest.php | 5 +- tests/Exceptions/InvalidTransactionTest.php | 5 +- tests/Places/ClosedPlaceTest.php | 10 ++- tests/Places/HalfOpenPlaceTest.php | 10 ++- tests/Places/OpenPlaceTest.php | 10 ++- tests/Places/PlaceTestCase.php | 6 ++ tests/SimpleCircuitBreakerFactoryTest.php | 4 +- tests/SimpleCircuitBreakerTest.php | 6 +- tests/Storages/SimpleArrayTest.php | 35 +++++--- tests/Storages/SymfonyCacheTest.php | 51 +++++------ tests/Transactions/SimpleTransactionTest.php | 10 +-- tests/Utils/AssertTest.php | 15 +++- 35 files changed, 318 insertions(+), 190 deletions(-) diff --git a/.gitignore b/.gitignore index 2a35621..d73941b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +build/ vendor/ composer.lock .php_cs.cache diff --git a/.php_cs.dist b/.php_cs.dist index 60c72e8..f4a32e8 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -32,6 +32,7 @@ return PhpCsFixer\Config::create() 'self_accessor' => false, 'yoda_style' => null, 'non_printable_character' => true, + 'phpdoc_no_empty_return' => false, ]) ->setFinder($finder) ->setCacheFile(__DIR__.'/.php_cs.cache') diff --git a/.travis.yml b/.travis.yml index b46eb0a..fda02ae 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,6 +32,8 @@ jobs: - stage: PHPStan Code quality script: - composer require --dev "phpstan/phpstan:^0.10" --ignore-platform-reqs && composer phpstan - + - stage: PHPQA Full analysis + script: + - docker run --rm -u $UID -v $PWD:/app eko3alpha/docker-phpqa --report --ignoredDirs vendor after_success: - bash <(curl -s https://codecov.io/bash) diff --git a/src/Clients/GuzzleClient.php b/src/Clients/GuzzleClient.php index 3690ca1..b86681d 100644 --- a/src/Clients/GuzzleClient.php +++ b/src/Clients/GuzzleClient.php @@ -2,10 +2,10 @@ namespace PrestaShop\CircuitBreaker\Clients; -use PrestaShop\CircuitBreaker\Exceptions\UnavailableService; -use PrestaShop\CircuitBreaker\Contracts\Client; -use GuzzleHttp\Client as OriginalGuzzleClient; use Exception; +use GuzzleHttp\Client as OriginalGuzzleClient; +use PrestaShop\CircuitBreaker\Contracts\Client; +use PrestaShop\CircuitBreaker\Exceptions\UnavailableService; /** * Guzzle implementation of client. diff --git a/src/Contracts/CircuitBreaker.php b/src/Contracts/CircuitBreaker.php index 82dc004..a0a8297 100644 --- a/src/Contracts/CircuitBreaker.php +++ b/src/Contracts/CircuitBreaker.php @@ -10,7 +10,7 @@ interface CircuitBreaker { /** - * @var string the circuit breaker state + * @return string the circuit breaker state */ public function getState(); @@ -19,21 +19,25 @@ public function getState(); * * @var string the function to call * @var callable $fallback if the service is unavailable, rely on the fallback + * + * @return string + * + * @param mixed $service */ public function call($service, callable $fallback); /** - * @var bool checks if the circuit breaker is open + * @return bool checks if the circuit breaker is open */ public function isOpened(); /** - * @var bool checks if the circuit breaker is half open + * @return bool checks if the circuit breaker is half open */ public function isHalfOpened(); /** - * @var bool checks if the circuit breaker is closed + * @return bool checks if the circuit breaker is closed */ public function isClosed(); } diff --git a/src/Contracts/Client.php b/src/Contracts/Client.php index c7d4a9c..f5a007e 100644 --- a/src/Contracts/Client.php +++ b/src/Contracts/Client.php @@ -9,8 +9,10 @@ interface Client { /** - * @var string The URI of the service to be reached - * @var array $options the options if needed + * @param string $resource the URI of the service to be reached + * @param array $options the options if needed + * + * @return string */ public function request($resource, array $options); } diff --git a/src/Contracts/Factory.php b/src/Contracts/Factory.php index 08bc084..367d279 100644 --- a/src/Contracts/Factory.php +++ b/src/Contracts/Factory.php @@ -8,7 +8,9 @@ interface Factory { /** - * @var array the settings for the Places + * @param array the settings for the Places + * + * @return CircuitBreaker */ public function create(array $settings); } diff --git a/src/Contracts/Place.php b/src/Contracts/Place.php index 1b7b161..efffd49 100644 --- a/src/Contracts/Place.php +++ b/src/Contracts/Place.php @@ -17,17 +17,17 @@ interface Place public function getState(); /** - * @var int the number of failures + * @return int the number of failures */ public function getFailures(); /** - * @var int the allowed number of trials + * @return int the allowed number of trials */ public function getThreshold(); /** - * @var int the allowed timeout + * @return float the allowed timeout */ public function getTimeout(); } diff --git a/src/Contracts/Storage.php b/src/Contracts/Storage.php index 4fcafee..043afbd 100644 --- a/src/Contracts/Storage.php +++ b/src/Contracts/Storage.php @@ -17,6 +17,8 @@ interface Storage * @var Transaction $transaction the transaction * * @return bool + * + * @param mixed $service */ public function saveTransaction($service, Transaction $transaction); @@ -28,6 +30,8 @@ public function saveTransaction($service, Transaction $transaction); * @throws TransactionNotFound * * @return Transaction + * + * @param mixed $service */ public function getTransaction($service); @@ -37,6 +41,8 @@ public function getTransaction($service); * @var string the service name * * @return bool + * + * @param mixed $service */ public function hasTransaction($service); diff --git a/src/Contracts/Transaction.php b/src/Contracts/Transaction.php index c4022eb..f903bf8 100644 --- a/src/Contracts/Transaction.php +++ b/src/Contracts/Transaction.php @@ -11,23 +11,23 @@ interface Transaction { /** - * @var string the service name + * @return string the service name */ public function getService(); /** - * @var int the number of failures to call the service + * @return int the number of failures to call the service */ public function getFailures(); /** - * @var string the current state of the Circuit Breaker + * @return string the current state of the Circuit Breaker */ public function getState(); /** - * @var DateTime the time when the circuit breaker move - * from open to half open state + * @return DateTime the time when the circuit breaker move + * from open to half open state */ public function getThresholdDateTime(); diff --git a/src/Exceptions/InvalidPlace.php b/src/Exceptions/InvalidPlace.php index 6cb1175..015e4c2 100644 --- a/src/Exceptions/InvalidPlace.php +++ b/src/Exceptions/InvalidPlace.php @@ -2,8 +2,8 @@ namespace PrestaShop\CircuitBreaker\Exceptions; -use PrestaShop\CircuitBreaker\Utils\ErrorFormatter; use Exception; +use PrestaShop\CircuitBreaker\Utils\ErrorFormatter; final class InvalidPlace extends Exception { @@ -11,6 +11,8 @@ final class InvalidPlace extends Exception * @param mixed $failures the failures * @param mixed $timeout the timeout * @param mixed $threshold the threshold + * + * @return self */ public static function invalidSettings($failures, $timeout, $threshold) { diff --git a/src/Exceptions/InvalidTransaction.php b/src/Exceptions/InvalidTransaction.php index 82e3391..7d16249 100644 --- a/src/Exceptions/InvalidTransaction.php +++ b/src/Exceptions/InvalidTransaction.php @@ -2,8 +2,8 @@ namespace PrestaShop\CircuitBreaker\Exceptions; -use PrestaShop\CircuitBreaker\Utils\ErrorFormatter; use Exception; +use PrestaShop\CircuitBreaker\Utils\ErrorFormatter; final class InvalidTransaction extends Exception { @@ -12,6 +12,8 @@ final class InvalidTransaction extends Exception * @param mixed $failures the failures * @param mixed $state the Circuit Breaker * @param mixed $threshold the threshold + * + * @return self */ public static function invalidParameters($service, $failures, $state, $threshold) { diff --git a/src/Places/AbstractPlace.php b/src/Places/AbstractPlace.php index be145ce..7044fa7 100644 --- a/src/Places/AbstractPlace.php +++ b/src/Places/AbstractPlace.php @@ -2,23 +2,39 @@ namespace PrestaShop\CircuitBreaker\Places; -use PrestaShop\CircuitBreaker\Exceptions\InvalidPlace; use PrestaShop\CircuitBreaker\Contracts\Place; +use PrestaShop\CircuitBreaker\Exceptions\InvalidPlace; use PrestaShop\CircuitBreaker\Utils\Assert; abstract class AbstractPlace implements Place { + /** + * @var int the Place failures + */ private $failures; + + /** + * @var float the Place timeout + */ private $timeout; + + /** + * @var int the Place threshold + */ private $threshold; + /** + * @param int $failures the Place failures + * @param float $timeout the Place timeout + * @param int $threshold the Place threshold + */ public function __construct($failures, $timeout, $threshold) { - if ($this->validate($failures, $timeout, $threshold)) { - $this->failures = $failures; - $this->timeout = $timeout; - $this->threshold = $threshold; - } + $this->validate($failures, $timeout, $threshold); + + $this->failures = $failures; + $this->timeout = $timeout; + $this->threshold = $threshold; } /** @@ -51,10 +67,22 @@ public function getThreshold() } /** - * Ensure the place is valid (PHP5 is permissive) + * Helper: create a Place from an array. + * + * @var array the failures, timeout and treshold + * + * @return self + */ + public static function fromArray(array $settings) + { + return new static($settings[0], $settings[1], $settings[2]); + } + + /** + * Ensure the place is valid (PHP5 is permissive). * * @param int $failures the failures should be a positive value - * @param int $timeout the timeout should be a positive value + * @param float $timeout the timeout should be a positive value * @param int $threshold the threshold should be a positive value * * @throws InvalidPlace @@ -63,26 +91,15 @@ public function getThreshold() */ private function validate($failures, $timeout, $threshold) { - if ( - Assert::isPositiveInteger($failures) && - Assert::isPositiveValue($timeout) && - Assert::isPositiveInteger($threshold) - ) { + $assertionsAreValid = Assert::isPositiveInteger($failures) + && Assert::isPositiveValue($timeout) + && Assert::isPositiveInteger($threshold) + ; + + if ($assertionsAreValid) { return true; } throw InvalidPlace::invalidSettings($failures, $timeout, $threshold); } - - /** - * Helper: create a Place from an array - * - * @var array the failures, timeout and treshold - * - * @return self - */ - public static function fromArray(array $settings) - { - return new static($settings[0], $settings[1], $settings[2]); - } } diff --git a/src/SimpleCircuitBreaker.php b/src/SimpleCircuitBreaker.php index e7f120b..b6e1124 100644 --- a/src/SimpleCircuitBreaker.php +++ b/src/SimpleCircuitBreaker.php @@ -2,15 +2,15 @@ namespace PrestaShop\CircuitBreaker; -use PrestaShop\CircuitBreaker\Transactions\SimpleTransaction; -use PrestaShop\CircuitBreaker\Exceptions\UnavailableService; +use DateTime; +use PrestaShop\CircuitBreaker\Clients\GuzzleClient; use PrestaShop\CircuitBreaker\Contracts\CircuitBreaker; +use PrestaShop\CircuitBreaker\Contracts\Place; +use PrestaShop\CircuitBreaker\Contracts\Storage; use PrestaShop\CircuitBreaker\Contracts\Transaction; -use PrestaShop\CircuitBreaker\Clients\GuzzleClient; +use PrestaShop\CircuitBreaker\Exceptions\UnavailableService; use PrestaShop\CircuitBreaker\Storages\SimpleArray; -use PrestaShop\CircuitBreaker\Contracts\Storage; -use PrestaShop\CircuitBreaker\Contracts\Place; -use DateTime; +use PrestaShop\CircuitBreaker\Transactions\SimpleTransaction; /** * Main implementation of Circuit Breaker. @@ -27,24 +27,19 @@ final class SimpleCircuitBreaker implements CircuitBreaker */ private $places = []; - /** - * @var Transaction the Circuit Breaker transaction - */ - private $transaction; - /** * @var Storage the Circuit Breaker storage */ private $storage; /** - * Constructor + * Constructor. */ public function __construct( Place $openPlace, Place $halfOpenPlace, Place $closedPlace - ) { + ) { $this->currentPlace = $closedPlace; $this->places = [ States::CLOSED_STATE => $closedPlace, @@ -68,31 +63,31 @@ public function getState() */ public function call($service, callable $fallback) { - $this->initTransaction($service); + $transaction = $this->initTransaction($service); // implement the right workflow with a machine state. // see schema. try { if ($this->isOpened()) { - if ($this->canAccessService()) { - $this->moveStateTo(States::HALF_OPEN_STATE, $service); + if ($this->canAccessService($transaction)) { + $this->moveStateTo(States::HALF_OPEN_STATE, $transaction, $service); } - return call_user_func($fallback); + return \call_user_func($fallback); } $response = $this->tryExecute($service); - $this->moveStateTo(States::CLOSED_STATE, $service); + $this->moveStateTo(States::CLOSED_STATE, $transaction, $service); return $response; } catch (UnavailableService $exception) { - $this->transaction->incrementFailures(); - $this->storage->saveTransaction($service, $this->transaction); + $transaction->incrementFailures(); + $this->storage->saveTransaction($service, $transaction); - if (!$this->isAllowedToRetry()) { - $this->moveStateTo(States::OPEN_STATE, $service); + if (!$this->isAllowedToRetry($transaction)) { + $this->moveStateTo(States::OPEN_STATE, $transaction, $service); - return call_user_func($fallback); + return \call_user_func($fallback); } return $this->call($service, $fallback); @@ -104,7 +99,7 @@ public function call($service, callable $fallback) */ public function isOpened() { - return $this->currentPlace->getState() === States::OPEN_STATE; + return States::OPEN_STATE === $this->currentPlace->getState(); } /** @@ -112,7 +107,7 @@ public function isOpened() */ public function isHalfOpened() { - return $this->currentPlace->getState() === States::HALF_OPEN_STATE; + return States::HALF_OPEN_STATE === $this->currentPlace->getState(); } /** @@ -120,46 +115,74 @@ public function isHalfOpened() */ public function isClosed() { - return $this->currentPlace->getState() === States::CLOSED_STATE; + return States::CLOSED_STATE === $this->currentPlace->getState(); } - private function moveStateTo($state, $service) + /** + * @param string $state the Place state + * @param Transaction $transaction the Circuit Breaker transaction + * @param string $service the service URI + * + * @return bool + */ + private function moveStateTo($state, Transaction $transaction, $service) { $this->currentPlace = $this->places[$state]; - $this->transaction = SimpleTransaction::createFromPlace( + $transaction = SimpleTransaction::createFromPlace( $this->currentPlace, $service ); - $this->storage->saveTransaction($service, $this->transaction); + return $this->storage->saveTransaction($service, $transaction); } + /** + * @param string $service the service URI + * + * @return Transaction + */ private function initTransaction($service) { if ($this->storage->hasTransaction($service)) { - $this->transaction = $this->storage->getTransaction($service); + $transaction = $this->storage->getTransaction($service); } else { - $this->transaction = SimpleTransaction::createFromPlace( + $transaction = SimpleTransaction::createFromPlace( $this->currentPlace, $service ); - $this->storage->saveTransaction($service, $this->transaction); + $this->storage->saveTransaction($service, $transaction); } + + return $transaction; } - private function isAllowedToRetry() + /** + * @param Transaction $transaction the Transaction + * + * @return bool + */ + private function isAllowedToRetry(Transaction $transaction) { - return $this->transaction->getFailures() < $this->currentPlace->getFailures(); + return $transaction->getFailures() < $this->currentPlace->getFailures(); } - private function canAccessService() + /** + * @param Transaction $transaction the Transaction + * + * @return bool + */ + private function canAccessService(Transaction $transaction) { - return $this->transaction->getThresholdDateTime() < new DateTime(); + return $transaction->getThresholdDateTime() < new DateTime(); } /** * @todo should be moved in its own class maybe? + * + * @param string $service the service URI + * + * @return string */ private function tryExecute($service) { diff --git a/src/SimpleCircuitBreakerFactory.php b/src/SimpleCircuitBreakerFactory.php index bf602af..821ae74 100644 --- a/src/SimpleCircuitBreakerFactory.php +++ b/src/SimpleCircuitBreakerFactory.php @@ -2,9 +2,9 @@ namespace PrestaShop\CircuitBreaker; -use PrestaShop\CircuitBreaker\Places\HalfOpenPlace; -use PrestaShop\CircuitBreaker\Places\ClosedPlace; use PrestaShop\CircuitBreaker\Contracts\Factory; +use PrestaShop\CircuitBreaker\Places\ClosedPlace; +use PrestaShop\CircuitBreaker\Places\HalfOpenPlace; use PrestaShop\CircuitBreaker\Places\OpenPlace; /** @@ -13,6 +13,9 @@ */ final class SimpleCircuitBreakerFactory implements Factory { + /** + * {@inheritdoc} + */ public function create(array $settings) { $openPlace = OpenPlace::fromArray($settings['open']); diff --git a/src/States.php b/src/States.php index 49d0430..fe508dc 100644 --- a/src/States.php +++ b/src/States.php @@ -3,7 +3,7 @@ namespace PrestaShop\CircuitBreaker; /** - * Define the available states of the Circuit Breaker; + * Define the available states of the Circuit Breaker;. */ final class States { diff --git a/src/Storages/SimpleArray.php b/src/Storages/SimpleArray.php index 13819d4..ff8b7c8 100644 --- a/src/Storages/SimpleArray.php +++ b/src/Storages/SimpleArray.php @@ -2,15 +2,18 @@ namespace PrestaShop\CircuitBreaker\Storages; -use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; -use PrestaShop\CircuitBreaker\Contracts\Transaction; use PrestaShop\CircuitBreaker\Contracts\Storage; +use PrestaShop\CircuitBreaker\Contracts\Transaction; +use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; /** * Very simple implementation of Storage using a simple PHP array. */ final class SimpleArray implements Storage { + /** + * @var array the circuit breaker transactions + */ public static $transactions = []; /** @@ -55,10 +58,12 @@ public function hasTransaction($service) public function clear() { self::$transactions = []; + + return true; } /** - * Helper method to properly store the transaction + * Helper method to properly store the transaction. * * @param string $service the service URI * diff --git a/src/Storages/SymfonyCache.php b/src/Storages/SymfonyCache.php index ee55b79..cdd5864 100644 --- a/src/Storages/SymfonyCache.php +++ b/src/Storages/SymfonyCache.php @@ -2,10 +2,10 @@ namespace PrestaShop\CircuitBreaker\Storages; +use PrestaShop\CircuitBreaker\Contracts\Storage; +use PrestaShop\CircuitBreaker\Contracts\Transaction; use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; use Symfony\Component\Cache\Simple\AbstractCache; -use PrestaShop\CircuitBreaker\Contracts\Transaction; -use PrestaShop\CircuitBreaker\Contracts\Storage; /** * Implementation of Storage using the Symfony Cache Component. @@ -17,15 +17,9 @@ final class SymfonyCache implements Storage */ private $symfonyCache; - /** - * @var string the Symfony Cache namespace - */ - private $namespace; - - public function __construct(AbstractCache $symfonyCache, $namespace) + public function __construct(AbstractCache $symfonyCache) { $this->symfonyCache = $symfonyCache; - $this->namespace = $namespace; } /** @@ -71,7 +65,7 @@ public function clear() } /** - * Helper method to properly store the transaction + * Helper method to properly store the transaction. * * @param string $service the service URI * diff --git a/src/Transactions/SimpleTransaction.php b/src/Transactions/SimpleTransaction.php index a243b83..654e787 100644 --- a/src/Transactions/SimpleTransaction.php +++ b/src/Transactions/SimpleTransaction.php @@ -2,11 +2,11 @@ namespace PrestaShop\CircuitBreaker\Transactions; -use PrestaShop\CircuitBreaker\Exceptions\InvalidTransaction; -use PrestaShop\CircuitBreaker\Contracts\Transaction; +use DateTime; use PrestaShop\CircuitBreaker\Contracts\Place; +use PrestaShop\CircuitBreaker\Contracts\Transaction; +use PrestaShop\CircuitBreaker\Exceptions\InvalidTransaction; use PrestaShop\CircuitBreaker\Utils\Assert; -use DateTime; /** * Main implementation of Circuit Breaker transaction. @@ -33,6 +33,12 @@ final class SimpleTransaction implements Transaction */ private $thresholdDateTime; + /** + * @param string $service the service URI + * @param int $failures the allowed failures + * @param string $state the circuit breaker state/place + * @param int $threshold the place threshold + */ public function __construct($service, $failures, $state, $threshold) { $this->validate($service, $failures, $state, $threshold); @@ -75,32 +81,21 @@ public function getThresholdDateTime() return $this->thresholdDateTime; } - /** - * Set the right DateTime from the threshold value. - * - * @param int $threshold the Transaction threshold - */ - private function initThresholdDateTime($threshold) - { - $thresholdDateTime = new DateTime(); - $thresholdDateTime->modify("+$threshold second"); - - $this->thresholdDateTime = $thresholdDateTime; - } - /** * {@inheritdoc} */ public function incrementFailures() { ++$this->failures; + + return true; } /** * Helper to create a transaction from the Place. * - * @var Place the Circuit Breaker place - * @var string $service the service URI + * @param Place the Circuit Breaker place + * @param string $service the service URI * * @return self */ @@ -117,7 +112,20 @@ public static function createFromPlace(Place $place, $service) } /** - * Ensure the transaction is valid (PHP5 is permissive) + * Set the right DateTime from the threshold value. + * + * @param int $threshold the Transaction threshold + */ + private function initThresholdDateTime($threshold) + { + $thresholdDateTime = new DateTime(); + $thresholdDateTime->modify("+$threshold second"); + + $this->thresholdDateTime = $thresholdDateTime; + } + + /** + * Ensure the transaction is valid (PHP5 is permissive). * * @param string $service the service URI * @param int $failures the failures should be a positive value @@ -130,12 +138,13 @@ public static function createFromPlace(Place $place, $service) */ private function validate($service, $failures, $state, $threshold) { - if ( - Assert::isURI($service) && - Assert::isPositiveInteger($failures) && - Assert::isString($state) && - Assert::isPositiveInteger($threshold) - ) { + $assertionsAreValid = Assert::isURI($service) + && Assert::isPositiveInteger($failures) + && Assert::isString($state) + && Assert::isPositiveInteger($threshold) + ; + + if ($assertionsAreValid) { return true; } diff --git a/src/Transitions.php b/src/Transitions.php index 46883fd..ee60491 100644 --- a/src/Transitions.php +++ b/src/Transitions.php @@ -3,7 +3,7 @@ namespace PrestaShop\CircuitBreaker; /** - * Define the available transitions of the Circuit Breaker; + * Define the available transitions of the Circuit Breaker;. */ final class Transitions { diff --git a/src/Utils/Assert.php b/src/Utils/Assert.php index de2230d..a3fdb42 100644 --- a/src/Utils/Assert.php +++ b/src/Utils/Assert.php @@ -16,7 +16,7 @@ final class Assert */ public static function isPositiveValue($value) { - return !is_string($value) && is_numeric($value) && $value >= 0; + return !\is_string($value) && is_numeric($value) && $value >= 0; } /** @@ -26,7 +26,7 @@ public static function isPositiveValue($value) */ public static function isPositiveInteger($value) { - return self::isPositiveValue($value) && is_int($value); + return self::isPositiveValue($value) && \is_int($value); } /** @@ -36,10 +36,10 @@ public static function isPositiveInteger($value) */ public static function isURI($value) { - return !is_null($value) + return null !== $value && !is_numeric($value) - && !is_bool($value) - && filter_var($value, FILTER_SANITIZE_URL) !== false + && !\is_bool($value) + && false !== filter_var($value, FILTER_SANITIZE_URL) ; } @@ -50,6 +50,6 @@ public static function isURI($value) */ public static function isString($value) { - return !empty($value) && is_string($value); + return !empty($value) && \is_string($value); } } diff --git a/src/Utils/ErrorFormatter.php b/src/Utils/ErrorFormatter.php index 03e8227..dc15cab 100644 --- a/src/Utils/ErrorFormatter.php +++ b/src/Utils/ErrorFormatter.php @@ -10,7 +10,7 @@ final class ErrorFormatter { /** - * Format error message + * Format error message. * * @param string $parameter the parameter to evaluate * @param mixed $value the value to format @@ -23,8 +23,8 @@ public static function format($parameter, $value, $function, $expectedType) { $errorMessage = ''; $isValid = Assert::$function($value); - $type = gettype($value); - $hasStringValue = in_array($type, ['integer', 'float', 'string']); + $type = \gettype($value); + $hasStringValue = \in_array($type, ['integer', 'float', 'string'], true); if (!$isValid) { $errorMessage = sprintf( diff --git a/tests/Clients/GuzzleClientTest.php b/tests/Clients/GuzzleClientTest.php index 7595a9f..5ae10f2 100644 --- a/tests/Clients/GuzzleClientTest.php +++ b/tests/Clients/GuzzleClientTest.php @@ -2,9 +2,9 @@ namespace Tests\PrestaShop\CircuitBreaker\Clients; +use PHPUnit\Framework\TestCase; use PrestaShop\CircuitBreaker\Clients\GuzzleClient; use PrestaShop\CircuitBreaker\Exceptions\UnavailableService; -use PHPUnit\Framework\TestCase; class GuzzleClientTest extends TestCase { diff --git a/tests/Exceptions/InvalidPlaceTest.php b/tests/Exceptions/InvalidPlaceTest.php index 1ea0f36..e232c85 100644 --- a/tests/Exceptions/InvalidPlaceTest.php +++ b/tests/Exceptions/InvalidPlaceTest.php @@ -2,8 +2,8 @@ namespace Tests\PrestaShop\CircuitBreaker\Exceptions; -use PrestaShop\CircuitBreaker\Exceptions\InvalidPlace; use PHPUnit\Framework\TestCase; +use PrestaShop\CircuitBreaker\Exceptions\InvalidPlace; class InvalidPlaceTest extends TestCase { @@ -31,6 +31,9 @@ public function testInvalidSettings($settings, $expectedExceptionMessage) $this->assertSame($invalidPlace->getMessage(), $expectedExceptionMessage); } + /** + * @return array + */ public function getSettings() { return [ diff --git a/tests/Exceptions/InvalidTransactionTest.php b/tests/Exceptions/InvalidTransactionTest.php index 4b90620..97d187b 100644 --- a/tests/Exceptions/InvalidTransactionTest.php +++ b/tests/Exceptions/InvalidTransactionTest.php @@ -2,8 +2,8 @@ namespace Tests\PrestaShop\CircuitBreaker\Exceptions; -use PrestaShop\CircuitBreaker\Exceptions\InvalidTransaction; use PHPUnit\Framework\TestCase; +use PrestaShop\CircuitBreaker\Exceptions\InvalidTransaction; class InvalidTransactionTest extends TestCase { @@ -32,6 +32,9 @@ public function testInvalidParameters($parameters, $expectedExceptionMessage) $this->assertSame($invalidPlace->getMessage(), $expectedExceptionMessage); } + /** + * @return array + */ public function getParameters() { return [ diff --git a/tests/Places/ClosedPlaceTest.php b/tests/Places/ClosedPlaceTest.php index 911636a..904c196 100644 --- a/tests/Places/ClosedPlaceTest.php +++ b/tests/Places/ClosedPlaceTest.php @@ -10,6 +10,10 @@ class ClosedPlaceTest extends PlaceTestCase { /** * @dataProvider getFixtures + * + * @param mixed $failures + * @param mixed $timeout + * @param mixed $threshold */ public function testCreationWith($failures, $timeout, $threshold) { @@ -22,12 +26,16 @@ public function testCreationWith($failures, $timeout, $threshold) /** * @dataProvider getInvalidFixtures + * + * @param mixed $failures + * @param mixed $timeout + * @param mixed $threshold */ public function testCreationWithInvalidValues($failures, $timeout, $threshold) { $this->expectException(InvalidPlace::class); - $closedPlace = new ClosedPlace($failures, $timeout, $threshold); + new ClosedPlace($failures, $timeout, $threshold); } public function testGetExpectedState() diff --git a/tests/Places/HalfOpenPlaceTest.php b/tests/Places/HalfOpenPlaceTest.php index 2750adb..bd797ee 100644 --- a/tests/Places/HalfOpenPlaceTest.php +++ b/tests/Places/HalfOpenPlaceTest.php @@ -10,6 +10,10 @@ class HalfOpenPlaceTest extends PlaceTestCase { /** * @dataProvider getFixtures + * + * @param mixed $failures + * @param mixed $timeout + * @param mixed $threshold */ public function testCreationWith($failures, $timeout, $threshold) { @@ -22,12 +26,16 @@ public function testCreationWith($failures, $timeout, $threshold) /** * @dataProvider getInvalidFixtures + * + * @param mixed $failures + * @param mixed $timeout + * @param mixed $threshold */ public function testCreationWithInvalidValues($failures, $timeout, $threshold) { $this->expectException(InvalidPlace::class); - $closedPlace = new HalfOpenPlace($failures, $timeout, $threshold); + new HalfOpenPlace($failures, $timeout, $threshold); } public function testGetExpectedState() diff --git a/tests/Places/OpenPlaceTest.php b/tests/Places/OpenPlaceTest.php index e9be2af..dcb7c1f 100644 --- a/tests/Places/OpenPlaceTest.php +++ b/tests/Places/OpenPlaceTest.php @@ -10,6 +10,10 @@ class OpenPlaceTest extends PlaceTestCase { /** * @dataProvider getFixtures + * + * @param mixed $failures + * @param mixed $timeout + * @param mixed $threshold */ public function testCreationWith($failures, $timeout, $threshold) { @@ -22,12 +26,16 @@ public function testCreationWith($failures, $timeout, $threshold) /** * @dataProvider getInvalidFixtures + * + * @param mixed $failures + * @param mixed $timeout + * @param mixed $threshold */ public function testCreationWithInvalidValues($failures, $timeout, $threshold) { $this->expectException(InvalidPlace::class); - $closedPlace = new OpenPlace($failures, $timeout, $threshold); + new OpenPlace($failures, $timeout, $threshold); } public function testGetExpectedState() diff --git a/tests/Places/PlaceTestCase.php b/tests/Places/PlaceTestCase.php index ae4d907..2fea53d 100644 --- a/tests/Places/PlaceTestCase.php +++ b/tests/Places/PlaceTestCase.php @@ -9,6 +9,9 @@ */ class PlaceTestCase extends TestCase { + /** + * @return array + */ public function getFixtures() { return [ @@ -17,6 +20,9 @@ public function getFixtures() ]; } + /** + * @return array + */ public function getInvalidFixtures() { return [ diff --git a/tests/SimpleCircuitBreakerFactoryTest.php b/tests/SimpleCircuitBreakerFactoryTest.php index 6614297..53b8128 100644 --- a/tests/SimpleCircuitBreakerFactoryTest.php +++ b/tests/SimpleCircuitBreakerFactoryTest.php @@ -2,9 +2,9 @@ namespace Tests\PrestaShop\CircuitBreaker; -use PrestaShop\CircuitBreaker\SimpleCircuitBreakerFactory; -use PrestaShop\CircuitBreaker\SimpleCircuitBreaker; use PHPUnit\Framework\TestCase; +use PrestaShop\CircuitBreaker\SimpleCircuitBreaker; +use PrestaShop\CircuitBreaker\SimpleCircuitBreakerFactory; class SimpleCircuitBreakerFactoryTest extends TestCase { diff --git a/tests/SimpleCircuitBreakerTest.php b/tests/SimpleCircuitBreakerTest.php index 109946e..c000ef3 100644 --- a/tests/SimpleCircuitBreakerTest.php +++ b/tests/SimpleCircuitBreakerTest.php @@ -2,12 +2,12 @@ namespace Tests\PrestaShop\CircuitBreaker; -use PrestaShop\CircuitBreaker\SimpleCircuitBreaker; -use PrestaShop\CircuitBreaker\Places\HalfOpenPlace; +use PHPUnit\Framework\TestCase; use PrestaShop\CircuitBreaker\Places\ClosedPlace; +use PrestaShop\CircuitBreaker\Places\HalfOpenPlace; use PrestaShop\CircuitBreaker\Places\OpenPlace; +use PrestaShop\CircuitBreaker\SimpleCircuitBreaker; use PrestaShop\CircuitBreaker\States; -use PHPUnit\Framework\TestCase; /** * @todo: needs tools to emulate a service unreachable diff --git a/tests/Storages/SimpleArrayTest.php b/tests/Storages/SimpleArrayTest.php index bcb1ce7..4aec508 100644 --- a/tests/Storages/SimpleArrayTest.php +++ b/tests/Storages/SimpleArrayTest.php @@ -2,13 +2,25 @@ namespace Tests\PrestaShop\CircuitBreaker\Storages; -use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; +use PHPUnit\Framework\TestCase; use PrestaShop\CircuitBreaker\Contracts\Transaction; +use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; use PrestaShop\CircuitBreaker\Storages\SimpleArray; -use PHPUnit\Framework\TestCase; class SimpleArrayTest extends TestCase { + /** + * {@inheritdoc} + */ + protected function setUp() + { + $simpleArray = new SimpleArray(); + $simpleArray::$transactions = []; + } + + /** + * @return void + */ public function testCreation() { $simpleArray = new SimpleArray(); @@ -19,6 +31,8 @@ public function testCreation() /** * @depends testCreation + * + * @return void */ public function testSaveTransaction() { @@ -34,6 +48,8 @@ public function testSaveTransaction() /** * @depends testCreation * @depends testSaveTransaction + * + * @return void */ public function testHasTransaction() { @@ -47,6 +63,8 @@ public function testHasTransaction() * @depends testCreation * @depends testSaveTransaction * @depends testHasTransaction + * + * @return void */ public function testGetTransaction() { @@ -63,6 +81,8 @@ public function testGetTransaction() * @depends testCreation * @depends testGetTransaction * @depends testHasTransaction + * + * @return void */ public function testGetNotFoundTransactionThrowsAnException() { @@ -75,6 +95,8 @@ public function testGetNotFoundTransactionThrowsAnException() /** * @depends testSaveTransaction * @depends testGetTransaction + * + * @return void */ public function testClear() { @@ -88,13 +110,4 @@ public function testClear() $transactions = $simpleArray::$transactions; $this->assertEmpty($transactions); } - - /** - * {@inheritdoc} - */ - protected function setUp() - { - $simpleArray = new SimpleArray(); - $simpleArray::$transactions = []; - } } diff --git a/tests/Storages/SymfonyCacheTest.php b/tests/Storages/SymfonyCacheTest.php index 06b29ad..e9285e0 100644 --- a/tests/Storages/SymfonyCacheTest.php +++ b/tests/Storages/SymfonyCacheTest.php @@ -2,11 +2,11 @@ namespace Tests\PrestaShop\CircuitBreaker\Storages; -use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; +use PHPUnit\Framework\TestCase; use PrestaShop\CircuitBreaker\Contracts\Transaction; +use PrestaShop\CircuitBreaker\Exceptions\TransactionNotFound; use PrestaShop\CircuitBreaker\Storages\SymfonyCache; use Symfony\Component\Cache\Simple\FilesystemCache; -use PHPUnit\Framework\TestCase; class SymfonyCacheTest extends TestCase { @@ -15,13 +15,29 @@ class SymfonyCacheTest extends TestCase */ private $symfonyCache; - public function testCreation() + /** + * {@inheritdoc} + */ + protected function setUp() { - $namespace = 'ps__circuit_breaker'; + $this->symfonyCache = new SymfonyCache( + new FilesystemCache('ps__circuit_breaker', 20) + ); + } + + /** + * {@inheritdoc} + */ + protected function tearDown() + { + $filesystemAdapter = new FilesystemCache('ps__circuit_breaker', 20); + $filesystemAdapter->clear(); + } + public function testCreation() + { $symfonyCache = new SymfonyCache( - new FilesystemCache($namespace), - $namespace + new FilesystemCache('ps__circuit_breaker') ); $this->assertInstanceOf(SymfonyCache::class, $symfonyCache); @@ -63,7 +79,7 @@ public function testGetTransaction() $transaction = $this->symfonyCache->getTransaction('http://test.com'); - $this->assertEquals($transaction, $translationStub); + $this->assertSame($transaction, $translationStub); } /** @@ -94,25 +110,4 @@ public function testClear() $this->symfonyCache->getTransaction('http://a.com'); } - - /** - * {@inheritdoc} - */ - protected function setUp() - { - $namespace = 'ps__circuit_breaker'; - $this->symfonyCache = new SymfonyCache( - new FilesystemCache($namespace, 20), - $namespace - ); - } - - /** - * {@inheritdoc} - */ - protected function tearDown() - { - $filesystemAdapter = new FilesystemCache('ps__circuit_breaker', 20); - $filesystemAdapter->clear(); - } } diff --git a/tests/Transactions/SimpleTransactionTest.php b/tests/Transactions/SimpleTransactionTest.php index 782e4ef..f9c4177 100644 --- a/tests/Transactions/SimpleTransactionTest.php +++ b/tests/Transactions/SimpleTransactionTest.php @@ -2,11 +2,11 @@ namespace Tests\PrestaShop\CircuitBreaker\Transactions; -use PrestaShop\CircuitBreaker\Transactions\SimpleTransaction; -use PrestaShop\CircuitBreaker\Contracts\Place; -use PHPUnit_Framework_MockObject_MockObject; -use PHPUnit\Framework\TestCase; use DateTime; +use PHPUnit\Framework\TestCase; +use PHPUnit_Framework_MockObject_MockObject; +use PrestaShop\CircuitBreaker\Contracts\Place; +use PrestaShop\CircuitBreaker\Transactions\SimpleTransaction; class SimpleTransactionTest extends TestCase { @@ -101,7 +101,7 @@ public function testCreationFromPlaceHelper() /** * Returns an instance of SimpleTransaction for tests. * - * @return SimpleTransaction + * @return SimpleTransaction&PHPUnit_Framework_MockObject_MockObject */ private function createSimpleTransaction() { diff --git a/tests/Utils/AssertTest.php b/tests/Utils/AssertTest.php index c0291f2..5f7ddef 100644 --- a/tests/Utils/AssertTest.php +++ b/tests/Utils/AssertTest.php @@ -2,8 +2,8 @@ namespace Tests\PrestaShop\CircuitBreaker\Utils; -use PrestaShop\CircuitBreaker\Utils\Assert; use PHPUnit\Framework\TestCase; +use PrestaShop\CircuitBreaker\Utils\Assert; class AssertTest extends TestCase { @@ -40,6 +40,9 @@ public function testIsString($value, $expected) $this->assertSame($expected, Assert::isString($value)); } + /** + * @return array + */ public function getValues() { return [ @@ -47,12 +50,20 @@ public function getValues() 'str_0' => ['0', false], 'float' => [0.1, true], 'stdclass' => [new \stdClass(), false], - 'callable' => [function () { return 0; }, false], + 'callable' => [ + function () { + return 0; + }, + false, + ], 'negative' => [-1, false], 'bool' => [false, false], ]; } + /** + * @return array + */ public function getURIs() { return [ From 3de33e04013770f6a65f5fe50a0898d9ced4a2ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Tue, 8 Jan 2019 15:49:55 +0100 Subject: [PATCH 2/4] Tried to use phpqa tools on Travis --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index fda02ae..f7c89aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,8 @@ language: php +services: + - docker + notifications: email: on_success: never From 7b1df8061f3d77caece3f4bebc5e296c50928ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Tue, 8 Jan 2019 15:56:36 +0100 Subject: [PATCH 3/4] fix tests --- tests/Storages/SymfonyCacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Storages/SymfonyCacheTest.php b/tests/Storages/SymfonyCacheTest.php index e9285e0..7a11b34 100644 --- a/tests/Storages/SymfonyCacheTest.php +++ b/tests/Storages/SymfonyCacheTest.php @@ -79,7 +79,7 @@ public function testGetTransaction() $transaction = $this->symfonyCache->getTransaction('http://test.com'); - $this->assertSame($transaction, $translationStub); + $this->assertEquals($transaction, $translationStub); } /** From f8516bafbfb76dcec6eb79156cc8ba78e98972c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Tue, 8 Jan 2019 16:06:05 +0100 Subject: [PATCH 4/4] Fixed PHPStan --- src/Contracts/Factory.php | 2 +- src/Transactions/SimpleTransaction.php | 2 +- tests/Transactions/SimpleTransactionTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Contracts/Factory.php b/src/Contracts/Factory.php index 367d279..813d336 100644 --- a/src/Contracts/Factory.php +++ b/src/Contracts/Factory.php @@ -8,7 +8,7 @@ interface Factory { /** - * @param array the settings for the Places + * @param array $settings the settings for the Places * * @return CircuitBreaker */ diff --git a/src/Transactions/SimpleTransaction.php b/src/Transactions/SimpleTransaction.php index 654e787..97c5161 100644 --- a/src/Transactions/SimpleTransaction.php +++ b/src/Transactions/SimpleTransaction.php @@ -94,7 +94,7 @@ public function incrementFailures() /** * Helper to create a transaction from the Place. * - * @param Place the Circuit Breaker place + * @param Place $place the Circuit Breaker place * @param string $service the service URI * * @return self diff --git a/tests/Transactions/SimpleTransactionTest.php b/tests/Transactions/SimpleTransactionTest.php index f9c4177..6b3e782 100644 --- a/tests/Transactions/SimpleTransactionTest.php +++ b/tests/Transactions/SimpleTransactionTest.php @@ -101,7 +101,7 @@ public function testCreationFromPlaceHelper() /** * Returns an instance of SimpleTransaction for tests. * - * @return SimpleTransaction&PHPUnit_Framework_MockObject_MockObject + * @return SimpleTransaction */ private function createSimpleTransaction() {