From 6094d17718e12e873fbafc2b9a19b2abdced7047 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Nov 2018 18:40:47 +0100 Subject: [PATCH] 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()); + } +}