diff --git a/CHANGELOG.md b/CHANGELOG.md index 722e69ab..5395a394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 1.18.0 - 2019-08-08 #### Added @@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#430](https://github.com/shlinkio/shlink/issues/430) Updated to [shlinkio/php-coding-standard](https://github.com/shlinkio/php-coding-standard) 1.2.2 * [#305](https://github.com/shlinkio/shlink/issues/305) Implemented changes which will allow Shlink to be truly clusterizable. +* [#262](https://github.com/shlinkio/shlink/issues/262) Increased mutation score to 75%. ### Deprecated diff --git a/composer.json b/composer.json index b067f8ed..89d25ece 100644 --- a/composer.json +++ b/composer.json @@ -137,9 +137,9 @@ ], "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --coverage-html build/coverage --order-by=random", - "infect": "infection --threads=4 --min-msi=70 --log-verbosity=default --only-covered", - "infect:ci": "infection --threads=4 --min-msi=70 --log-verbosity=default --only-covered --coverage=build", - "infect:show": "infection --threads=4 --min-msi=70 --log-verbosity=default --only-covered --show-mutations", + "infect": "infection --threads=4 --min-msi=75 --log-verbosity=default --only-covered", + "infect:ci": "infection --threads=4 --min-msi=75 --log-verbosity=default --only-covered --coverage=build", + "infect:show": "infection --threads=4 --min-msi=75 --log-verbosity=default --only-covered --show-mutations", "infect:test": [ "@test:unit:ci", "@infect:ci" diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index 6058c6a6..f8d14c55 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Exception; use Throwable; use Zend\InputFilter\InputFilterInterface; +use function Functional\reduce_left; use function is_array; use function print_r; use function sprintf; @@ -27,21 +28,11 @@ class ValidationException extends RuntimeException parent::__construct($message, $code, $previous); } - /** - * @param InputFilterInterface $inputFilter - * @param \Throwable|null $prev - * @return ValidationException - */ public static function fromInputFilter(InputFilterInterface $inputFilter, ?Throwable $prev = null): self { return static::fromArray($inputFilter->getMessages(), $prev); } - /** - * @param array $invalidData - * @param \Throwable|null $prev - * @return ValidationException - */ private static function fromArray(array $invalidData, ?Throwable $prev = null): self { return new self( @@ -57,23 +48,17 @@ class ValidationException extends RuntimeException ); } - private static function formMessagesToString(array $messages = []) + private static function formMessagesToString(array $messages = []): string { - $text = ''; - foreach ($messages as $name => $messageSet) { - $text .= sprintf( - "\n\t'%s' => %s", + return reduce_left($messages, function ($messageSet, $name, $_, string $acc) { + return $acc . sprintf( + "\n '%s' => %s", $name, is_array($messageSet) ? print_r($messageSet, true) : $messageSet ); - } - - return $text; + }, ''); } - /** - * @return array - */ public function getInvalidElements(): array { return $this->invalidElements; diff --git a/module/Core/test/Exception/ValidationExceptionTest.php b/module/Core/test/Exception/ValidationExceptionTest.php new file mode 100644 index 00000000..4b7af6d2 --- /dev/null +++ b/module/Core/test/Exception/ValidationExceptionTest.php @@ -0,0 +1,81 @@ +assertEquals($expectedMessage, $e->getMessage()); + $this->assertEquals($expectedInvalidElements, $e->getInvalidElements()); + $this->assertEquals($expectedCode, $e->getCode()); + $this->assertEquals($expectedPrev, $e->getPrevious()); + } + + public function provideExceptionData(): iterable + { + yield 'empty args' => [[], '', [], 0, null]; + yield 'with message' => [['something'], 'something', [], 0, null]; + yield 'with elements' => [['something_else', [1, 2, 3]], 'something_else', [1, 2, 3], 0, null]; + yield 'with code' => [['foo', [], $foo = random_int(-100, 100)], 'foo', [], $foo, null]; + yield 'with prev' => [['bar', [], 8, $e = new RuntimeException()], 'bar', [], 8, $e]; + } + + /** + * @test + * @dataProvider provideExceptions + */ + public function createsExceptionFromInputFilter(?Throwable $prev): void + { + $invalidData = [ + 'foo' => 'bar', + 'something' => ['baz', 'foo'], + ]; + $barValue = print_r(['baz', 'foo'], true); + $expectedMessage = << bar + 'something' => {$barValue} + +EOT; + + $inputFilter = $this->prophesize(InputFilterInterface::class); + $getMessages = $inputFilter->getMessages()->willReturn($invalidData); + + $e = ValidationException::fromInputFilter($inputFilter->reveal()); + + $this->assertEquals($invalidData, $e->getInvalidElements()); + $this->assertEquals($expectedMessage, $e->getMessage()); + $this->assertEquals(-1, $e->getCode()); + $this->assertEquals($prev, $e->getPrevious()); + $getMessages->shouldHaveBeenCalledOnce(); + } + + public function provideExceptions(): iterable + { + return [[null, new RuntimeException(), new LogicException()]]; + } +} diff --git a/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php b/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php index ae697a00..5326eee4 100644 --- a/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php +++ b/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php @@ -4,28 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Authentication; use Interop\Container\ContainerInterface; -use Interop\Container\Exception\ContainerException; -use Zend\ServiceManager\Exception\ServiceNotCreatedException; -use Zend\ServiceManager\Exception\ServiceNotFoundException; -use Zend\ServiceManager\Factory\FactoryInterface; -class AuthenticationPluginManagerFactory implements FactoryInterface +class AuthenticationPluginManagerFactory { - /** - * Create an object - * - * @param ContainerInterface $container - * @param string $requestedName - * @param null|array $options - * @return object - * @throws ServiceNotFoundException if unable to resolve the service. - * @throws ServiceNotCreatedException if an exception is raised when - * creating a service. - * @throws ContainerException if any other error occurs - */ - public function __invoke(ContainerInterface $container, $requestedName, ?array $options = null) + public function __invoke(ContainerInterface $container): AuthenticationPluginManager { - $config = $container->get('config') ?? []; + $config = $container->has('config') ? $container->get('config') : []; return new AuthenticationPluginManager($container, $config['auth']['plugins'] ?? []); } } diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 65f2a3bb..1c413235 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -6,8 +6,10 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; use Exception; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\ListShortUrlsAction; +use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequest; use Zend\Paginator\Adapter\ArrayAdapter; use Zend\Paginator\Paginator; @@ -18,39 +20,84 @@ class ListShortUrlsActionTest extends TestCase private $action; /** @var ObjectProphecy */ private $service; + /** @var ObjectProphecy */ + private $logger; public function setUp(): void { $this->service = $this->prophesize(ShortUrlService::class); + $this->logger = $this->prophesize(LoggerInterface::class); + $this->action = new ListShortUrlsAction($this->service->reveal(), [ 'hostname' => 'doma.in', 'schema' => 'https', - ]); + ], $this->logger->reveal()); } - /** @test */ - public function properListReturnsSuccessResponse() - { - $page = 3; - $this->service->listShortUrls($page, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledOnce(); + /** + * @test + * @dataProvider provideFilteringData + */ + public function properListReturnsSuccessResponse( + array $query, + int $expectedPage, + ?string $expectedSearchTerm, + array $expectedTags, + ?string $expectedOrderBy + ): void { + $listShortUrls = $this->service->listShortUrls( + $expectedPage, + $expectedSearchTerm, + $expectedTags, + $expectedOrderBy + )->willReturn(new Paginator(new ArrayAdapter())); - $response = $this->action->handle((new ServerRequest())->withQueryParams([ - 'page' => $page, - ])); + /** @var JsonResponse $response */ + $response = $this->action->handle((new ServerRequest())->withQueryParams($query)); + $payload = $response->getPayload(); + + $this->assertArrayHasKey('shortUrls', $payload); + $this->assertArrayHasKey('data', $payload['shortUrls']); + $this->assertEquals([], $payload['shortUrls']['data']); $this->assertEquals(200, $response->getStatusCode()); + $listShortUrls->shouldHaveBeenCalledOnce(); + } + + public function provideFilteringData(): iterable + { + yield [[], 1, null, [], null]; + yield [['page' => 10], 10, null, [], null]; + yield [['page' => null], 1, null, [], null]; + yield [['page' => '8'], 8, null, [], null]; + yield [['searchTerm' => $searchTerm = 'foo'], 1, $searchTerm, [], null]; + yield [['tags' => $tags = ['foo','bar']], 1, null, $tags, null]; + yield [['orderBy' => $orderBy = 'something'], 1, null, [], $orderBy]; + yield [[ + 'page' => '2', + 'orderBy' => $orderBy = 'something', + 'tags' => $tags = ['one', 'two'], + ], 2, null, $tags, $orderBy]; } /** @test */ - public function anExceptionsReturnsErrorResponse() + public function anExceptionReturnsErrorResponse(): void { $page = 3; - $this->service->listShortUrls($page, null, [], null)->willThrow(Exception::class) + $e = new Exception(); + + $this->service->listShortUrls($page, null, [], null)->willThrow($e) ->shouldBeCalledOnce(); + $logError = $this->logger->error( + 'Unexpected error while listing short URLs. {e}', + ['e' => $e] + )->will(function () { + }); $response = $this->action->handle((new ServerRequest())->withQueryParams([ 'page' => $page, ])); + $this->assertEquals(500, $response->getStatusCode()); + $logError->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php b/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php index 04f46211..6880cbd3 100644 --- a/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php +++ b/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Rest\Authentication; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManager; use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerFactory; +use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; use Zend\ServiceManager\ServiceManager; class AuthenticationPluginManagerFactoryTest extends TestCase @@ -18,12 +19,41 @@ class AuthenticationPluginManagerFactoryTest extends TestCase $this->factory = new AuthenticationPluginManagerFactory(); } - /** @test */ - public function serviceIsProperlyCreated() + /** + * @test + * @dataProvider provideConfigs + */ + public function serviceIsProperlyCreatedWithExpectedPlugins(?array $config, array $expectedPlugins): void { - $instance = $this->factory->__invoke(new ServiceManager(['services' => [ - 'config' => [], - ]]), ''); - $this->assertInstanceOf(AuthenticationPluginManager::class, $instance); + $instance = ($this->factory)(new ServiceManager(['services' => [ + 'config' => $config, + ]])); + + $this->assertEquals($expectedPlugins, $this->getPlugins($instance)); + } + + private function getPlugins(AuthenticationPluginManager $pluginManager): array + { + return (function () { + return $this->services; + })->call($pluginManager); + } + + public function provideConfigs(): iterable + { + yield [null, []]; + yield [[], []]; + yield [['auth' => []], []]; + yield [['auth' => [ + 'plugins' => [], + ]], []]; + yield [['auth' => [ + 'plugins' => [ + 'services' => $plugins = [ + 'foo' => $this->prophesize(AuthenticationPluginInterface::class)->reveal(), + 'bar' => $this->prophesize(AuthenticationPluginInterface::class)->reveal(), + ], + ], + ]], $plugins]; } } diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index 94e9b4d7..cd4ab889 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -13,6 +13,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; @@ -37,14 +38,19 @@ class AuthenticationMiddlewareTest extends TestCase private $middleware; /** @var ObjectProphecy */ private $requestToPlugin; - - /** @var callable */ - private $dummyMiddleware; + /** @var ObjectProphecy */ + private $logger; public function setUp(): void { $this->requestToPlugin = $this->prophesize(RequestToHttpAuthPluginInterface::class); - $this->middleware = new AuthenticationMiddleware($this->requestToPlugin->reveal(), [AuthenticateAction::class]); + $this->logger = $this->prophesize(LoggerInterface::class); + + $this->middleware = new AuthenticationMiddleware( + $this->requestToPlugin->reveal(), + [AuthenticateAction::class], + $this->logger->reveal() + ); } /** @@ -97,6 +103,10 @@ class AuthenticationMiddlewareTest extends TestCase RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) ); $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willThrow($e); + $logWarning = $this->logger->warning('Invalid or no authentication provided. {e}', ['e' => $e])->will( + function () { + } + ); /** @var Response\JsonResponse $response */ $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); @@ -108,6 +118,7 @@ class AuthenticationMiddlewareTest extends TestCase implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) ), $payload['message']); $fromRequest->shouldHaveBeenCalledOnce(); + $logWarning->shouldHaveBeenCalledOnce(); } public function provideExceptions(): iterable @@ -124,12 +135,15 @@ class AuthenticationMiddlewareTest extends TestCase RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) ); + $e = VerifyAuthenticationException::withError('the_error', 'the_message'); $plugin = $this->prophesize(AuthenticationPluginInterface::class); - $verify = $plugin->verify($request)->willThrow( - VerifyAuthenticationException::withError('the_error', 'the_message') - ); + $verify = $plugin->verify($request)->willThrow($e); $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn($plugin->reveal()); + $logWarning = $this->logger->warning('Authentication verification failed. {e}', ['e' => $e])->will( + function () { + } + ); /** @var Response\JsonResponse $response */ $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); @@ -139,6 +153,7 @@ class AuthenticationMiddlewareTest extends TestCase $this->assertEquals('the_message', $payload['message']); $verify->shouldHaveBeenCalledOnce(); $fromRequest->shouldHaveBeenCalledOnce(); + $logWarning->shouldHaveBeenCalledOnce(); } /** @test */