Merge pull request #448 from acelaya/feature/improve-msi

Feature/improve msi

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
 

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"

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;

module/Core/test/Exception/ValidationExceptionTest.php

@@ -0,0 +1,81 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Exception;
+
+use LogicException;
+use PHPUnit\Framework\TestCase;
+use RuntimeException;
+use Shlinkio\Shlink\Core\Exception\ValidationException;
+use Throwable;
+use Zend\InputFilter\InputFilterInterface;
+
+use function print_r;
+use function random_int;
+
+class ValidationExceptionTest extends TestCase
+{
+    /**
+     * @test
+     * @dataProvider provideExceptionData
+     */
+    public function createsExceptionWrappingExpectedData(
+        array $args,
+        string $expectedMessage,
+        array $expectedInvalidElements,
+        int $expectedCode,
+        ?Throwable $expectedPrev
+    ): void {
+        $e = new ValidationException(...$args);
+
+        $this->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 = <<<EOT
+Provided data is not valid. These are the messages:
+
+    'foo' => 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()]];
+    }
+}

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'] ?? []);
     }
 }

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();
     }
 }

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];
     }
 }

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 */