From a705ef21a9084a3f4d2c0736d01189908f344a9d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 17:36:22 +0100 Subject: [PATCH 1/6] Increased MSI to 61% --- composer.json | 6 ++++- infection.json | 2 +- .../CLI/src/Command/Api/ListKeysCommand.php | 3 +-- .../Command/Api/DisableKeyCommandTest.php | 9 +++++-- .../Command/Api/GenerateKeyCommandTest.php | 8 ++++-- .../test/Command/Api/ListKeysCommandTest.php | 26 +++++++++++++++---- 6 files changed, 41 insertions(+), 13 deletions(-) diff --git a/composer.json b/composer.json index 1f5db2ec..90fdb87b 100644 --- a/composer.json +++ b/composer.json @@ -125,7 +125,11 @@ "infect": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered", "infect:ci": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --coverage=build", - "infect:show": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --show-mutations" + "infect:show": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --show-mutations", + "infect:test": [ + "@test:unit:ci", + "@infect:ci" + ] }, "scripts-descriptions": { "check": "Alias for \"cs\", \"stan\", \"test\" and \"infect\"", diff --git a/infection.json b/infection.json index a4579d63..44fdf228 100644 --- a/infection.json +++ b/infection.json @@ -4,7 +4,7 @@ "module/*/src" ] }, - "timeout": 10, + "timeout": 5, "logs": { "text": "build/infection/infection-log.txt", "summary": "build/infection/summary-log.txt", diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index ef1c5c29..aabad7ec 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -57,12 +57,11 @@ class ListKeysCommand extends Command $enabledOnly = $input->getOption('enabledOnly'); $rows = array_map(function (ApiKey $apiKey) use ($enabledOnly) { - $key = (string) $apiKey; $expiration = $apiKey->getExpirationDate(); $messagePattern = $this->determineMessagePattern($apiKey); // Set columns for this row - $rowData = [sprintf($messagePattern, $key)]; + $rowData = [sprintf($messagePattern, $apiKey)]; if (! $enabledOnly) { $rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey)); } diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index c910ea85..dacfcf91 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -39,10 +39,14 @@ class DisableKeyCommandTest extends TestCase { $apiKey = 'abcd1234'; $this->apiKeyService->disable($apiKey)->shouldBeCalledOnce(); + $this->commandTester->execute([ 'command' => 'api-key:disable', 'apiKey' => $apiKey, ]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('API key "abcd1234" properly disabled', $output); } /** @@ -51,14 +55,15 @@ class DisableKeyCommandTest extends TestCase public function errorIsReturnedIfServiceThrowsException() { $apiKey = 'abcd1234'; - $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class) - ->shouldBeCalledOnce(); + $disable = $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class); $this->commandTester->execute([ 'command' => 'api-key:disable', 'apiKey' => $apiKey, ]); $output = $this->commandTester->getDisplay(); + $this->assertContains('API key "abcd1234" does not exist.', $output); + $disable->shouldHaveBeenCalledOnce(); } } diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index 78fcf4d5..104b6e70 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -39,11 +39,15 @@ class GenerateKeyCommandTest extends TestCase */ public function noExpirationDateIsDefinedIfNotProvided() { - $this->apiKeyService->create(null)->shouldBeCalledOnce() - ->willReturn(new ApiKey()); + $create = $this->apiKeyService->create(null)->willReturn(new ApiKey()); + $this->commandTester->execute([ 'command' => 'api-key:generate', ]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Generated API key: ', $output); + $create->shouldHaveBeenCalledOnce(); } /** diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index 72485ebe..da5908ef 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -35,30 +35,46 @@ class ListKeysCommandTest extends TestCase /** * @test */ - public function ifEnabledOnlyIsNotProvidedEverythingIsListed() + public function everythingIsListedIfEnabledOnlyIsNotProvided() { $this->apiKeyService->listKeys(false)->willReturn([ new ApiKey(), new ApiKey(), new ApiKey(), ])->shouldBeCalledOnce(); + $this->commandTester->execute([ - 'command' => 'api-key:list', + 'command' => ListKeysCommand::NAME, ]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Key', $output); + $this->assertContains('Is enabled', $output); + $this->assertContains(' +++ ', $output); + $this->assertNotContains(' --- ', $output); + $this->assertContains('Expiration date', $output); } /** * @test */ - public function ifEnabledOnlyIsProvidedOnlyThoseKeysAreListed() + public function onlyEnabledKeysAreListedIfEnabledOnlyIsProvided() { $this->apiKeyService->listKeys(true)->willReturn([ - new ApiKey(), + (new ApiKey())->disable(), new ApiKey(), ])->shouldBeCalledOnce(); + $this->commandTester->execute([ - 'command' => 'api-key:list', + 'command' => ListKeysCommand::NAME, '--enabledOnly' => true, ]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Key', $output); + $this->assertNotContains('Is enabled', $output); + $this->assertNotContains(' +++ ', $output); + $this->assertNotContains(' --- ', $output); + $this->assertContains('Expiration date', $output); } } From d2ed7d64170a9aa9e610b5e01feca0895f6939d2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 18:06:06 +0100 Subject: [PATCH 2/6] Increased MSI to 62% --- .../Command/Visit/ProcessVisitsCommand.php | 2 +- .../ShortUrl/GeneratePreviewCommandTest.php | 15 +++++-- ...st.php => GenerateShortUrlCommandTest.php} | 43 +++++++++++++++---- ...dTest.php => ListShortUrlsCommandTest.php} | 18 +++++++- .../Common/src/Service/PreviewGenerator.php | 4 +- .../src/Service/PreviewGeneratorInterface.php | 4 +- 6 files changed, 66 insertions(+), 20 deletions(-) rename module/CLI/test/Command/ShortUrl/{GenerateShortcodeCommandTest.php => GenerateShortUrlCommandTest.php} (61%) rename module/CLI/test/Command/ShortUrl/{ListShortcodesCommandTest.php => ListShortUrlsCommandTest.php} (82%) diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 860e3277..128ed9b7 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -126,7 +126,7 @@ class ProcessVisitsCommand extends Command $this->getApplication()->renderException($e, $this->output); } - throw new IpCannotBeLocatedException('An error occurred while locating IP', 0, $e); + throw new IpCannotBeLocatedException('An error occurred while locating IP', $e->getCode(), $e); } } } diff --git a/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php b/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php index 8469d24d..62b9496a 100644 --- a/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php @@ -62,13 +62,22 @@ class GeneratePreviewCommandTest extends TestCase ]); $this->shortUrlService->listShortUrls(1)->willReturn($paginator)->shouldBeCalledOnce(); - $this->previewGenerator->generatePreview('http://foo.com')->shouldBeCalledOnce(); - $this->previewGenerator->generatePreview('https://bar.com')->shouldBeCalledOnce(); - $this->previewGenerator->generatePreview('http://baz.com/something')->shouldBeCalledOnce(); + $generatePreview1 = $this->previewGenerator->generatePreview('http://foo.com')->willReturn(''); + $generatePreview2 = $this->previewGenerator->generatePreview('https://bar.com')->willReturn(''); + $generatePreview3 = $this->previewGenerator->generatePreview('http://baz.com/something')->willReturn(''); $this->commandTester->execute([ 'command' => 'shortcode:process-previews', ]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Processing URL http://foo.com', $output); + $this->assertContains('Processing URL https://bar.com', $output); + $this->assertContains('Processing URL http://baz.com/something', $output); + $this->assertContains('Finished processing all URLs', $output); + $generatePreview1->shouldHaveBeenCalledOnce(); + $generatePreview2->shouldHaveBeenCalledOnce(); + $generatePreview3->shouldHaveBeenCalledOnce(); } /** diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php similarity index 61% rename from module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php rename to module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php index 34d7e34a..7f3a339e 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php @@ -3,9 +3,11 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\ShortUrl; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\CLI\Command\ShortUrl\GenerateShortUrlCommand; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; @@ -13,9 +15,8 @@ use Shlinkio\Shlink\Core\Service\UrlShortener; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; use Zend\I18n\Translator\Translator; -use function strpos; -class GenerateShortcodeCommandTest extends TestCase +class GenerateShortUrlCommandTest extends TestCase { /** * @var CommandTester @@ -43,18 +44,19 @@ class GenerateShortcodeCommandTest extends TestCase */ public function properShortCodeIsCreatedIfLongUrlIsCorrect() { - $this->urlShortener->urlToShortCode(Argument::cetera()) - ->willReturn( - (new ShortUrl(''))->setShortCode('abc123') - ) - ->shouldBeCalledOnce(); + $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn( + (new ShortUrl(''))->setShortCode('abc123') + ); $this->commandTester->execute([ 'command' => 'shortcode:generate', 'longUrl' => 'http://domain.com/foo/bar', + '--maxVisits' => '3', ]); $output = $this->commandTester->getDisplay(); - $this->assertTrue(strpos($output, 'http://foo.com/abc123') > 0); + + $this->assertContains('http://foo.com/abc123', $output); + $urlToShortCode->shouldHaveBeenCalledOnce(); } /** @@ -75,4 +77,29 @@ class GenerateShortcodeCommandTest extends TestCase $output ); } + + /** + * @test + */ + public function properlyProcessesProvidedTags() + { + $urlToShortCode = $this->urlShortener->urlToShortCode( + Argument::type(UriInterface::class), + Argument::that(function (array $tags) { + Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $tags); + return $tags; + }), + Argument::cetera() + )->willReturn((new ShortUrl(''))->setShortCode('abc123')); + + $this->commandTester->execute([ + 'command' => 'shortcode:generate', + 'longUrl' => 'http://domain.com/foo/bar', + '--tags' => ['foo,bar', 'baz', 'boo,zar'], + ]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('http://foo.com/abc123', $output); + $urlToShortCode->shouldHaveBeenCalledOnce(); + } } diff --git a/module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php similarity index 82% rename from module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php rename to module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 3480a289..bbde52fd 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -15,7 +15,7 @@ use Zend\I18n\Translator\Translator; use Zend\Paginator\Adapter\ArrayAdapter; use Zend\Paginator\Paginator; -class ListShortcodesCommandTest extends TestCase +class ListShortUrlsCommandTest extends TestCase { /** * @var CommandTester @@ -52,7 +52,7 @@ class ListShortcodesCommandTest extends TestCase */ public function loadingMorePagesCallsListMoreTimes() { - // The paginator will return more than one page for the first 3 times + // The paginator will return more than one page $data = []; for ($i = 0; $i < 50; $i++) { $data[] = new ShortUrl('url_' . $i); @@ -64,6 +64,11 @@ class ListShortcodesCommandTest extends TestCase $this->commandTester->setInputs(['y', 'y', 'n']); $this->commandTester->execute(['command' => 'shortcode:list']); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Continue with page 2?', $output); + $this->assertContains('Continue with page 3?', $output); + $this->assertContains('Continue with page 4?', $output); } /** @@ -82,6 +87,15 @@ class ListShortcodesCommandTest extends TestCase $this->commandTester->setInputs(['n']); $this->commandTester->execute(['command' => 'shortcode:list']); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('url_1', $output); + $this->assertContains('url_9', $output); + $this->assertNotContains('url_10', $output); + $this->assertNotContains('url_20', $output); + $this->assertNotContains('url_30', $output); + $this->assertContains('Continue with page 2?', $output); + $this->assertNotContains('Continue with page 3?', $output); } /** diff --git a/module/Common/src/Service/PreviewGenerator.php b/module/Common/src/Service/PreviewGenerator.php index bc084171..26ff32fc 100644 --- a/module/Common/src/Service/PreviewGenerator.php +++ b/module/Common/src/Service/PreviewGenerator.php @@ -35,11 +35,9 @@ class PreviewGenerator implements PreviewGeneratorInterface /** * Generates and stores preview for provided website and returns the path to the image file * - * @param string $url - * @return string * @throws PreviewGenerationException */ - public function generatePreview($url) + public function generatePreview(string $url): string { /** @var Image $image */ $image = $this->imageBuilder->build(Image::class, ['url' => $url]); diff --git a/module/Common/src/Service/PreviewGeneratorInterface.php b/module/Common/src/Service/PreviewGeneratorInterface.php index f9e5dcae..3fff0158 100644 --- a/module/Common/src/Service/PreviewGeneratorInterface.php +++ b/module/Common/src/Service/PreviewGeneratorInterface.php @@ -10,9 +10,7 @@ interface PreviewGeneratorInterface /** * Generates and stores preview for provided website and returns the path to the image file * - * @param string $url - * @return string * @throws PreviewGenerationException */ - public function generatePreview($url); + public function generatePreview(string $url): string; } From 6094d17718e12e873fbafc2b9a19b2abdced7047 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 18:40:47 +0100 Subject: [PATCH 3/6] Increased MSI to 64% --- .../test/Factory/ApplicationFactoryTest.php | 36 ++++++++++++++----- module/Common/src/Factory/CacheFactory.php | 12 ++----- module/Common/src/Image/ImageFactory.php | 4 +-- .../src/Middleware/LocaleMiddleware.php | 6 +--- module/Common/src/Response/PixelResponse.php | 2 +- module/Common/src/Util/ResponseUtilsTrait.php | 17 ++------- .../test/Exception/WrongIpExceptionTest.php | 35 ++++++++++++++++++ .../IpGeolocation/GeoLite2/DbUpdaterTest.php | 3 ++ .../GeoLite2LocationResolverTest.php | 1 + .../test/Response/PixelResponseTest.php | 29 +++++++++++++++ 10 files changed, 103 insertions(+), 42 deletions(-) create mode 100644 module/Common/test/Exception/WrongIpExceptionTest.php create mode 100644 module/Common/test/Response/PixelResponseTest.php diff --git a/module/CLI/test/Factory/ApplicationFactoryTest.php b/module/CLI/test/Factory/ApplicationFactoryTest.php index a5d6614b..fdbd56c6 100644 --- a/module/CLI/test/Factory/ApplicationFactoryTest.php +++ b/module/CLI/test/Factory/ApplicationFactoryTest.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Factory; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; +use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Factory\ApplicationFactory; use Shlinkio\Shlink\Core\Options\AppOptions; use Symfony\Component\Console\Application; @@ -29,7 +31,7 @@ class ApplicationFactoryTest extends TestCase */ public function serviceIsCreated() { - $instance = $this->factory->__invoke($this->createServiceManager(), ''); + $instance = ($this->factory)($this->createServiceManager(), ''); $this->assertInstanceOf(Application::class, $instance); } @@ -40,21 +42,24 @@ class ApplicationFactoryTest extends TestCase { $sm = $this->createServiceManager([ 'commands' => [ - 'foo', - 'bar', - 'baz', + 'foo' => 'foo', + 'bar' => 'bar', + 'baz' => 'baz', ], ]); - $sm->setService('foo', $this->prophesize(Command::class)->reveal()); - $sm->setService('baz', $this->prophesize(Command::class)->reveal()); + $sm->setService('foo', $this->createCommandMock('foo')->reveal()); + $sm->setService('bar', $this->createCommandMock('bar')->reveal()); /** @var Application $instance */ - $instance = $this->factory->__invoke($sm, ''); + $instance = ($this->factory)($sm, ''); $this->assertInstanceOf(Application::class, $instance); - $this->assertCount(2, $instance->all()); + + $this->assertTrue($instance->has('foo')); + $this->assertTrue($instance->has('bar')); + $this->assertFalse($instance->has('baz')); } - protected function createServiceManager($config = []) + private function createServiceManager(array $config = []): ServiceManager { return new ServiceManager(['services' => [ 'config' => [ @@ -64,4 +69,17 @@ class ApplicationFactoryTest extends TestCase Translator::class => Translator::factory([]), ]]); } + + private function createCommandMock(string $name): ObjectProphecy + { + $command = $this->prophesize(Command::class); + $command->getName()->willReturn($name); + $command->getDefinition()->willReturn($name); + $command->isEnabled()->willReturn(true); + $command->getAliases()->willReturn([]); + $command->setApplication(Argument::type(Application::class))->willReturn(function () { + }); + + return $command; + } } diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index 92f63c61..bd0eea3f 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -45,11 +45,7 @@ class CacheFactory implements FactoryInterface return $adapter; } - /** - * @param ContainerInterface $container - * @return Cache\CacheProvider - */ - protected function getAdapter(ContainerInterface $container) + private function getAdapter(ContainerInterface $container): Cache\CacheProvider { // Try to get the adapter from config $config = $container->get('config'); @@ -61,11 +57,7 @@ class CacheFactory implements FactoryInterface return env('APP_ENV', 'pro') === 'pro' ? new Cache\ApcuCache() : new Cache\ArrayCache(); } - /** - * @param array $cacheConfig - * @return Cache\CacheProvider - */ - protected function resolveCacheAdapter(array $cacheConfig) + private function resolveCacheAdapter(array $cacheConfig): Cache\CacheProvider { switch ($cacheConfig['adapter']) { case Cache\ArrayCache::class: diff --git a/module/Common/src/Image/ImageFactory.php b/module/Common/src/Image/ImageFactory.php index 95d0e998..0629ce2c 100644 --- a/module/Common/src/Image/ImageFactory.php +++ b/module/Common/src/Image/ImageFactory.php @@ -27,9 +27,9 @@ class ImageFactory implements FactoryInterface public function __invoke(ContainerInterface $container, $requestedName, array $options = null) { $config = $container->get('config')['phpwkhtmltopdf']; - $image = new Image(isset($config['images']) ? $config['images'] : null); + $image = new Image($config['images'] ?? null); - if (isset($options) && isset($options['url'])) { + if ($options['url'] ?? null) { $image->setPage($options['url']); } diff --git a/module/Common/src/Middleware/LocaleMiddleware.php b/module/Common/src/Middleware/LocaleMiddleware.php index 066cacf2..6970f803 100644 --- a/module/Common/src/Middleware/LocaleMiddleware.php +++ b/module/Common/src/Middleware/LocaleMiddleware.php @@ -45,11 +45,7 @@ class LocaleMiddleware implements MiddlewareInterface return $delegate->handle($request); } - /** - * @param string $locale - * @return string - */ - protected function normalizeLocale($locale) + private function normalizeLocale(string $locale): string { $parts = explode('_', $locale); if (count($parts) > 1) { diff --git a/module/Common/src/Response/PixelResponse.php b/module/Common/src/Response/PixelResponse.php index 37b3bbbd..e2554bb5 100644 --- a/module/Common/src/Response/PixelResponse.php +++ b/module/Common/src/Response/PixelResponse.php @@ -27,7 +27,7 @@ class PixelResponse extends Response private function createBody(): StreamInterface { $body = new Stream('php://temp', 'wb+'); - $body->write((string) base64_decode(self::BASE_64_IMAGE)); + $body->write(base64_decode(self::BASE_64_IMAGE)); $body->rewind(); return $body; } diff --git a/module/Common/src/Util/ResponseUtilsTrait.php b/module/Common/src/Util/ResponseUtilsTrait.php index 998b0af4..e8ad8bf0 100644 --- a/module/Common/src/Util/ResponseUtilsTrait.php +++ b/module/Common/src/Util/ResponseUtilsTrait.php @@ -9,28 +9,15 @@ use Zend\Diactoros\Response; use Zend\Diactoros\Stream; use Zend\Stdlib\ArrayUtils; use const FILEINFO_MIME; -use function basename; trait ResponseUtilsTrait { - protected function generateDownloadFileResponse(string $filePath): ResponseInterface - { - return $this->generateBinaryResponse($filePath, [ - 'Content-Disposition' => 'attachment; filename=' . basename($filePath), - 'Content-Transfer-Encoding' => 'Binary', - 'Content-Description' => 'File Transfer', - 'Pragma' => 'public', - 'Expires' => '0', - 'Cache-Control' => 'must-revalidate', - ]); - } - - protected function generateImageResponse(string $imagePath): ResponseInterface + private function generateImageResponse(string $imagePath): ResponseInterface { return $this->generateBinaryResponse($imagePath); } - protected function generateBinaryResponse(string $path, array $extraHeaders = []): ResponseInterface + private function generateBinaryResponse(string $path, array $extraHeaders = []): ResponseInterface { $body = new Stream($path); return new Response($body, 200, ArrayUtils::merge([ diff --git a/module/Common/test/Exception/WrongIpExceptionTest.php b/module/Common/test/Exception/WrongIpExceptionTest.php new file mode 100644 index 00000000..1b6d0d08 --- /dev/null +++ b/module/Common/test/Exception/WrongIpExceptionTest.php @@ -0,0 +1,35 @@ +assertEquals('Provided IP "1.2.3.4" is invalid', $e->getMessage()); + $this->assertEquals(0, $e->getCode()); + $this->assertNull($e->getPrevious()); + } + /** + * @test + */ + public function fromIpAddressProperlyCreatesExceptionWithPrev() + { + $prev = new Exception('Previous error'); + $e = WrongIpException::fromIpAddress('1.2.3.4', $prev); + + $this->assertEquals('Provided IP "1.2.3.4" is invalid', $e->getMessage()); + $this->assertEquals(0, $e->getCode()); + $this->assertSame($prev, $e->getPrevious()); + } +} diff --git a/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php b/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php index c5c7c861..116cf5f2 100644 --- a/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php +++ b/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php @@ -55,6 +55,7 @@ class DbUpdaterTest extends TestCase $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class); $this->expectException(RuntimeException::class); + $this->expectExceptionCode(0); $this->expectExceptionMessage( 'An error occurred while trying to download a fresh copy of the GeoLite2 database' ); @@ -73,6 +74,7 @@ class DbUpdaterTest extends TestCase $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); $this->expectException(RuntimeException::class); + $this->expectExceptionCode(0); $this->expectExceptionMessage( 'An error occurred while trying to extract the GeoLite2 database from __invalid__/GeoLite2-City.tar.gz' ); @@ -91,6 +93,7 @@ class DbUpdaterTest extends TestCase $copy = $this->filesystem->copy(Argument::cetera())->willThrow($e); $this->expectException(RuntimeException::class); + $this->expectExceptionCode(0); $this->expectExceptionMessage('An error occurred while trying to copy GeoLite2 db file to destination'); $request->shouldBeCalledOnce(); $copy->shouldBeCalledOnce(); diff --git a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php index b643bf0b..65aedaf1 100644 --- a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php +++ b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php @@ -41,6 +41,7 @@ class GeoLite2LocationResolverTest extends TestCase $this->expectException(WrongIpException::class); $this->expectExceptionMessage($message); + $this->expectExceptionCode(0); $cityMethod->shouldBeCalledOnce(); $this->resolver->resolveIpLocation($ipAddress); diff --git a/module/Common/test/Response/PixelResponseTest.php b/module/Common/test/Response/PixelResponseTest.php new file mode 100644 index 00000000..5813a7dc --- /dev/null +++ b/module/Common/test/Response/PixelResponseTest.php @@ -0,0 +1,29 @@ +resp = new PixelResponse(); + } + + /** + * @test + */ + public function responseHasGifTypeAndIsNotEmpty() + { + $this->assertEquals('image/gif', $this->resp->getHeaderLine('Content-Type')); + $this->assertNotEmpty((string) $this->resp->getBody()); + } +} From 79b2a0839f4c4903c115ecde1bd867d647568ea2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 19:23:25 +0100 Subject: [PATCH 4/6] Increased MSI to 65% --- composer.json | 6 +-- module/Common/src/Response/PixelResponse.php | 3 +- module/Common/src/Response/QrCodeResponse.php | 11 ++--- module/Common/src/Util/ResponseUtilsTrait.php | 3 +- module/Common/src/Util/StringUtilsTrait.php | 2 +- .../test/Response/QrCodeResponseTest.php | 23 ++++++++++ module/Common/test/Util/DateRangeTest.php | 20 +++++++++ .../Common/test/Util/StringUtilsTraitTest.php | 45 +++++++++++++++++++ module/Core/src/Entity/Visit.php | 8 ++++ .../Exception/InvalidShortCodeException.php | 4 +- .../src/Exception/InvalidUrlException.php | 2 +- module/Core/test/Entity/VisitTest.php | 40 +++++++++++++++++ .../Exception/DeleteShortUrlExceptionTest.php | 1 + .../InvalidShortCodeExceptionTest.php | 43 ++++++++++++++++++ .../Exception/InvalidUrlExceptionTest.php | 33 ++++++++++++++ 15 files changed, 227 insertions(+), 17 deletions(-) create mode 100644 module/Common/test/Response/QrCodeResponseTest.php create mode 100644 module/Common/test/Util/StringUtilsTraitTest.php create mode 100644 module/Core/test/Entity/VisitTest.php create mode 100644 module/Core/test/Exception/InvalidShortCodeExceptionTest.php create mode 100644 module/Core/test/Exception/InvalidUrlExceptionTest.php diff --git a/composer.json b/composer.json index 90fdb87b..3d325b03 100644 --- a/composer.json +++ b/composer.json @@ -123,9 +123,9 @@ ], "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --coverage-html build/coverage --order-by=random", - "infect": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered", - "infect:ci": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --coverage=build", - "infect:show": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --show-mutations", + "infect": "infection --threads=4 --min-msi=65 --log-verbosity=2 --only-covered", + "infect:ci": "infection --threads=4 --min-msi=65 --log-verbosity=2 --only-covered --coverage=build", + "infect:show": "infection --threads=4 --min-msi=65 --log-verbosity=2 --only-covered --show-mutations", "infect:test": [ "@test:unit:ci", "@infect:ci" diff --git a/module/Common/src/Response/PixelResponse.php b/module/Common/src/Response/PixelResponse.php index e2554bb5..87a10dca 100644 --- a/module/Common/src/Response/PixelResponse.php +++ b/module/Common/src/Response/PixelResponse.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Response; +use Fig\Http\Message\StatusCodeInterface as StatusCode; use Psr\Http\Message\StreamInterface; use Zend\Diactoros\Response; use Zend\Diactoros\Stream; @@ -13,7 +14,7 @@ class PixelResponse extends Response private const BASE_64_IMAGE = 'R0lGODlhAQABAJAAAP8AAAAAACH5BAUQAAAALAAAAAABAAEAAAICBAEAOw=='; private const CONTENT_TYPE = 'image/gif'; - public function __construct(int $status = 200, array $headers = []) + public function __construct(int $status = StatusCode::STATUS_OK, array $headers = []) { $headers['content-type'] = self::CONTENT_TYPE; parent::__construct($this->createBody(), $status, $headers); diff --git a/module/Common/src/Response/QrCodeResponse.php b/module/Common/src/Response/QrCodeResponse.php index b207fa0d..230ae08f 100644 --- a/module/Common/src/Response/QrCodeResponse.php +++ b/module/Common/src/Response/QrCodeResponse.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Response; use Endroid\QrCode\QrCode; +use Fig\Http\Message\StatusCodeInterface as StatusCode; use Psr\Http\Message\StreamInterface; use Zend\Diactoros\Response; use Zend\Diactoros\Stream; @@ -12,7 +13,7 @@ class QrCodeResponse extends Response { use Response\InjectContentTypeTrait; - public function __construct(QrCode $qrCode, $status = 200, array $headers = []) + public function __construct(QrCode $qrCode, int $status = StatusCode::STATUS_OK, array $headers = []) { parent::__construct( $this->createBody($qrCode), @@ -21,13 +22,7 @@ class QrCodeResponse extends Response ); } - /** - * Create the message body. - * - * @param QrCode $qrCode - * @return StreamInterface - */ - private function createBody(QrCode $qrCode) + private function createBody(QrCode $qrCode): StreamInterface { $body = new Stream('php://temp', 'wb+'); $body->write($qrCode->get()); diff --git a/module/Common/src/Util/ResponseUtilsTrait.php b/module/Common/src/Util/ResponseUtilsTrait.php index e8ad8bf0..25a884f9 100644 --- a/module/Common/src/Util/ResponseUtilsTrait.php +++ b/module/Common/src/Util/ResponseUtilsTrait.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Util; +use Fig\Http\Message\StatusCodeInterface as StatusCode; use finfo; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\Response; @@ -20,7 +21,7 @@ trait ResponseUtilsTrait private function generateBinaryResponse(string $path, array $extraHeaders = []): ResponseInterface { $body = new Stream($path); - return new Response($body, 200, ArrayUtils::merge([ + return new Response($body, StatusCode::STATUS_OK, ArrayUtils::merge([ 'Content-Type' => (new finfo(FILEINFO_MIME))->file($path), 'Content-Length' => (string) $body->getSize(), ], $extraHeaders)); diff --git a/module/Common/src/Util/StringUtilsTrait.php b/module/Common/src/Util/StringUtilsTrait.php index 87b77a2a..853ba7b8 100644 --- a/module/Common/src/Util/StringUtilsTrait.php +++ b/module/Common/src/Util/StringUtilsTrait.php @@ -9,7 +9,7 @@ use function strlen; trait StringUtilsTrait { - private function generateRandomString($length = 10): string + private function generateRandomString(int $length = 10): string { $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; $charactersLength = strlen($characters); diff --git a/module/Common/test/Response/QrCodeResponseTest.php b/module/Common/test/Response/QrCodeResponseTest.php new file mode 100644 index 00000000..36d6fada --- /dev/null +++ b/module/Common/test/Response/QrCodeResponseTest.php @@ -0,0 +1,23 @@ +assertEquals($qrCode->getContentType(), $resp->getHeaderLine('Content-Type')); + $this->assertEquals($qrCode->get(), (string) $resp->getBody()); + } +} diff --git a/module/Common/test/Util/DateRangeTest.php b/module/Common/test/Util/DateRangeTest.php index e5e48d46..ec5602a2 100644 --- a/module/Common/test/Util/DateRangeTest.php +++ b/module/Common/test/Util/DateRangeTest.php @@ -32,4 +32,24 @@ class DateRangeTest extends TestCase $this->assertSame($endDate, $range->getEndDate()); $this->assertFalse($range->isEmpty()); } + + /** + * @test + * @dataProvider provideDates + */ + public function isConsideredEmptyOnlyIfNoneOfTheDatesIsSet(?Chronos $startDate, ?Chronos $endDate, bool $isEmpty) + { + $range = new DateRange($startDate, $endDate); + $this->assertEquals($isEmpty, $range->isEmpty()); + } + + public function provideDates(): array + { + return [ + [null, null, true], + [null, Chronos::now(), false], + [Chronos::now(), null, false], + [Chronos::now(), Chronos::now(), false], + ]; + } } diff --git a/module/Common/test/Util/StringUtilsTraitTest.php b/module/Common/test/Util/StringUtilsTraitTest.php new file mode 100644 index 00000000..b64f4738 --- /dev/null +++ b/module/Common/test/Util/StringUtilsTraitTest.php @@ -0,0 +1,45 @@ +assertEquals($length, strlen($this->generateRandomString($length))); + } + + public function provideLengths(): array + { + return [ + [1], + [10], + [15], + ]; + } + + /** + * @test + */ + public function generatesUuidV4() + { + $uuidPattern = '/[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}/'; + + $this->assertRegExp($uuidPattern, $this->generateV4Uuid()); + $this->assertRegExp($uuidPattern, $this->generateV4Uuid()); + $this->assertRegExp($uuidPattern, $this->generateV4Uuid()); + $this->assertRegExp($uuidPattern, $this->generateV4Uuid()); + $this->assertRegExp($uuidPattern, $this->generateV4Uuid()); + } +} diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 3fd98db0..d7c44ff7 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -118,4 +118,12 @@ class Visit extends AbstractEntity implements JsonSerializable 'remoteAddr' => null, ]; } + + /** + * @internal + */ + public function getDate(): Chronos + { + return $this->date; + } } diff --git a/module/Core/src/Exception/InvalidShortCodeException.php b/module/Core/src/Exception/InvalidShortCodeException.php index a4e68cea..59a361bb 100644 --- a/module/Core/src/Exception/InvalidShortCodeException.php +++ b/module/Core/src/Exception/InvalidShortCodeException.php @@ -8,7 +8,7 @@ use function sprintf; class InvalidShortCodeException extends RuntimeException { - public static function fromCharset($shortCode, $charSet, Exception $previous = null) + public static function fromCharset(string $shortCode, string $charSet, Exception $previous = null): self { $code = $previous !== null ? $previous->getCode() : -1; return new static( @@ -18,7 +18,7 @@ class InvalidShortCodeException extends RuntimeException ); } - public static function fromNotFoundShortCode($shortCode) + public static function fromNotFoundShortCode(string $shortCode): self { return new static(sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); } diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 65f87e4e..6a94b548 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -10,7 +10,7 @@ class InvalidUrlException extends RuntimeException { public static function fromUrl($url, Throwable $previous = null) { - $code = isset($previous) ? $previous->getCode() : -1; + $code = $previous !== null ? $previous->getCode() : -1; return new static(sprintf('Provided URL "%s" is not an existing and valid URL', $url), $code, $previous); } } diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php new file mode 100644 index 00000000..c5daaad6 --- /dev/null +++ b/module/Core/test/Entity/VisitTest.php @@ -0,0 +1,40 @@ +assertEquals([ + 'referer' => 'some site', + 'date' => ($date ?? $visit->getDate())->toAtomString(), + 'userAgent' => 'Chrome', + 'visitLocation' => null, + + // Deprecated + 'remoteAddr' => null, + ], $visit->jsonSerialize()); + } + + public function provideDates(): array + { + return [ + [null], + [Chronos::now()->subDays(10)], + ]; + } +} diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php index 80c5ff10..f5c1ef80 100644 --- a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php +++ b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php @@ -21,6 +21,7 @@ class DeleteShortUrlExceptionTest extends TestCase ) { $e = DeleteShortUrlException::fromVisitsThreshold($threshold, $shortCode); $this->assertEquals($expectedMessage, $e->getMessage()); + $this->assertEquals(0, $e->getCode()); } public function provideMessages(): array diff --git a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php new file mode 100644 index 00000000..4dca8e07 --- /dev/null +++ b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php @@ -0,0 +1,43 @@ +assertEquals('Provided short code "abc123" does not match the char set "def456"', $e->getMessage()); + $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode()); + $this->assertEquals($prev, $e->getPrevious()); + } + + public function providePrevious(): array + { + return [ + [null], + [new Exception('Previos error', 10)], + ]; + } + + /** + * @test + */ + public function properlyCreatesExceptionFromNotFoundShortCode() + { + $e = InvalidShortCodeException::fromNotFoundShortCode('abc123'); + + $this->assertEquals('Provided short code "abc123" does not belong to a short URL', $e->getMessage()); + } +} diff --git a/module/Core/test/Exception/InvalidUrlExceptionTest.php b/module/Core/test/Exception/InvalidUrlExceptionTest.php new file mode 100644 index 00000000..67a12601 --- /dev/null +++ b/module/Core/test/Exception/InvalidUrlExceptionTest.php @@ -0,0 +1,33 @@ +assertEquals('Provided URL "http://the_url.com" is not an existing and valid URL', $e->getMessage()); + $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode()); + $this->assertEquals($prev, $e->getPrevious()); + } + + public function providePrevious(): array + { + return [ + [null], + [new Exception('Previos error', 10)], + ]; + } +} From f48f98f4d7a5ee6c368f5808f5bbf17cf7f104f7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 19:26:07 +0100 Subject: [PATCH 5/6] Updated changelog for v1.14.1 --- CHANGELOG.md | 4 ++-- module/Core/test/Entity/VisitTest.php | 2 +- module/Core/test/Exception/InvalidShortCodeExceptionTest.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5de19a2e..6a9673f1 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/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 1.14.1 - 2018-11-17 #### Added @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Changed -* *Nothing* +* [#260](https://github.com/shlinkio/shlink/issues/260) Increased mutation score to 65%. #### Deprecated diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index c5daaad6..0687131a 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -4,9 +4,9 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Entity; use Cake\Chronos\Chronos; +use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; -use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Model\Visitor; class VisitTest extends TestCase diff --git a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php index 4dca8e07..65fb858e 100644 --- a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php +++ b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Exception; use Exception; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Throwable; class InvalidShortCodeExceptionTest extends TestCase From b8faa6714ae0bd86b11a51c03389bad4ee071d19 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 19:32:31 +0100 Subject: [PATCH 6/6] Increased MSI to 65% (for sure this time) --- module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php | 4 ++-- module/Core/src/Exception/ValidationException.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index bbde52fd..4edd20f8 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -82,8 +82,8 @@ class ListShortUrlsCommandTest extends TestCase $data[] = new ShortUrl('url_' . $i); } - $this->shortUrlService->listShortUrls(Argument::cetera())->willReturn(new Paginator(new ArrayAdapter($data))) - ->shouldBeCalledOnce(); + $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter($data))) + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['n']); $this->commandTester->execute(['command' => 'shortcode:list']); diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index 5220005a..fc362303 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -42,7 +42,7 @@ class ValidationException extends RuntimeException * @param \Throwable|null $prev * @return ValidationException */ - public static function fromArray(array $invalidData, Throwable $prev = null): self + private static function fromArray(array $invalidData, Throwable $prev = null): self { return new self( sprintf(