diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 848a540c..25006cc9 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; @@ -61,7 +62,7 @@ class ProcessVisitsCommand extends Command } try { - $this->visitService->locateVisits( + $this->visitService->locateUnlocatedVisits( [$this, 'getGeolocationDataForVisit'], function (VisitLocation $location) use ($output) { $output->writeln(sprintf(' [Address located at "%s"]', $location->getCountryName())); @@ -75,7 +76,7 @@ class ProcessVisitsCommand extends Command } } - public function getGeolocationDataForVisit(Visit $visit): array + public function getGeolocationDataForVisit(Visit $visit): Location { if (! $visit->hasRemoteAddr()) { $this->output->writeln( diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index a63b94b6..35e1024a 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -27,9 +27,7 @@ class DisableKeyCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function providedApiKeyIsDisabled() { $apiKey = 'abcd1234'; @@ -44,9 +42,7 @@ class DisableKeyCommandTest extends TestCase $this->assertStringContainsString('API key "abcd1234" properly disabled', $output); } - /** - * @test - */ + /** @test */ public function errorIsReturnedIfServiceThrowsException() { $apiKey = 'abcd1234'; diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index 1ffee677..a298198e 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -29,9 +29,7 @@ class GenerateKeyCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function noExpirationDateIsDefinedIfNotProvided() { $create = $this->apiKeyService->create(null)->willReturn(new ApiKey()); @@ -45,9 +43,7 @@ class GenerateKeyCommandTest extends TestCase $create->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function expirationDateIsDefinedIfProvided() { $this->apiKeyService->create(Argument::type(Chronos::class))->shouldBeCalledOnce() diff --git a/module/CLI/test/Command/Config/GenerateCharsetCommandTest.php b/module/CLI/test/Command/Config/GenerateCharsetCommandTest.php index 941ff0e8..b0378f62 100644 --- a/module/CLI/test/Command/Config/GenerateCharsetCommandTest.php +++ b/module/CLI/test/Command/Config/GenerateCharsetCommandTest.php @@ -25,9 +25,7 @@ class GenerateCharsetCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function charactersAreGeneratedFromDefault() { $prefix = 'Character set: '; diff --git a/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php b/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php index aaf0a5e1..abf76e4b 100644 --- a/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php @@ -39,9 +39,7 @@ class GeneratePreviewCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function previewsForEveryUrlAreGenerated() { $paginator = $this->createPaginator([ @@ -69,9 +67,7 @@ class GeneratePreviewCommandTest extends TestCase $generatePreview3->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function exceptionWillOutputError() { $items = [ diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php index 36125fa1..d120b20f 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php @@ -34,9 +34,7 @@ class GenerateShortUrlCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function properShortCodeIsCreatedIfLongUrlIsCorrect() { $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn( @@ -54,9 +52,7 @@ class GenerateShortUrlCommandTest extends TestCase $urlToShortCode->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function exceptionWhileParsingLongUrlOutputsError() { $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(new InvalidUrlException()) @@ -73,9 +69,7 @@ class GenerateShortUrlCommandTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function properlyProcessesProvidedTags() { $urlToShortCode = $this->urlShortener->urlToShortCode( diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index 6e0f830d..b2e1e01f 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\GetVisitsCommand; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; @@ -36,9 +37,7 @@ class GetVisitsCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function noDateFlagsTriesToListWithoutDateRange() { $shortCode = 'abc123'; @@ -52,9 +51,7 @@ class GetVisitsCommandTest extends TestCase ]); } - /** - * @test - */ + /** @test */ public function providingDateFlagsTheListGetsFiltered() { $shortCode = 'abc123'; @@ -75,16 +72,14 @@ class GetVisitsCommandTest extends TestCase ]); } - /** - * @test - */ - public function outputIsProperlyGenerated() + /** @test */ + public function outputIsProperlyGenerated(): void { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, Argument::any())->willReturn( new Paginator(new ArrayAdapter([ (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->locate( - new VisitLocation(['country_name' => 'Spain']) + new VisitLocation(new Location('', 'Spain', '', '', 0, 0, '')) ), ])) )->shouldBeCalledOnce(); diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index f8b3a920..f7faae5c 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -30,9 +30,7 @@ class ListShortUrlsCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function noInputCallsListJustOnce() { $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) @@ -42,9 +40,7 @@ class ListShortUrlsCommandTest extends TestCase $this->commandTester->execute(['command' => 'shortcode:list']); } - /** - * @test - */ + /** @test */ public function loadingMorePagesCallsListMoreTimes() { // The paginator will return more than one page @@ -66,9 +62,7 @@ class ListShortUrlsCommandTest extends TestCase $this->assertStringContainsString('Continue with page 4?', $output); } - /** - * @test - */ + /** @test */ public function havingMorePagesButAnsweringNoCallsListJustOnce() { // The paginator will return more than one page @@ -93,9 +87,7 @@ class ListShortUrlsCommandTest extends TestCase $this->assertStringNotContainsString('Continue with page 3?', $output); } - /** - * @test - */ + /** @test */ public function passingPageWillMakeListStartOnThatPage() { $page = 5; @@ -109,9 +101,7 @@ class ListShortUrlsCommandTest extends TestCase ]); } - /** - * @test - */ + /** @test */ public function ifTagsFlagIsProvidedTagsColumnIsIncluded() { $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index fa9acdab..2997352a 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -31,9 +31,7 @@ class ResolveUrlCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function correctShortCodeResolvesUrl() { $shortCode = 'abc123'; @@ -50,9 +48,7 @@ class ResolveUrlCommandTest extends TestCase $this->assertEquals('Long URL: ' . $expectedUrl . PHP_EOL, $output); } - /** - * @test - */ + /** @test */ public function incorrectShortCodeOutputsErrorMessage() { $shortCode = 'abc123'; @@ -67,9 +63,7 @@ class ResolveUrlCommandTest extends TestCase $this->assertStringContainsString('Provided short code "' . $shortCode . '" could not be found.', $output); } - /** - * @test - */ + /** @test */ public function wrongShortCodeFormatOutputsErrorMessage() { $shortCode = 'abc123'; diff --git a/module/CLI/test/Command/Tag/CreateTagCommandTest.php b/module/CLI/test/Command/Tag/CreateTagCommandTest.php index 84efc922..7b482478 100644 --- a/module/CLI/test/Command/Tag/CreateTagCommandTest.php +++ b/module/CLI/test/Command/Tag/CreateTagCommandTest.php @@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\CLI\Command\Tag; use Doctrine\Common\Collections\ArrayCollection; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\CreateTagCommand; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; @@ -30,9 +29,7 @@ class CreateTagCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function errorIsReturnedWhenNoTagsAreProvided() { $this->commandTester->execute([]); @@ -41,13 +38,10 @@ class CreateTagCommandTest extends TestCase $this->assertStringContainsString('You have to provide at least one tag name', $output); } - /** - * @test - */ + /** @test */ public function serviceIsInvokedOnSuccess() { $tagNames = ['foo', 'bar']; - /** @var MethodProphecy $createTags */ $createTags = $this->tagService->createTags($tagNames)->willReturn(new ArrayCollection()); $this->commandTester->execute([ diff --git a/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php b/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php index 8169941c..81d89953 100644 --- a/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\Tag; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\DeleteTagsCommand; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; @@ -31,9 +30,7 @@ class DeleteTagsCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function errorIsReturnedWhenNoTagsAreProvided() { $this->commandTester->execute([]); @@ -42,13 +39,10 @@ class DeleteTagsCommandTest extends TestCase $this->assertStringContainsString('You have to provide at least one tag name', $output); } - /** - * @test - */ + /** @test */ public function serviceIsInvokedOnSuccess() { $tagNames = ['foo', 'bar']; - /** @var MethodProphecy $deleteTags */ $deleteTags = $this->tagService->deleteTags($tagNames)->will(function () { }); diff --git a/module/CLI/test/Command/Tag/ListTagsCommandTest.php b/module/CLI/test/Command/Tag/ListTagsCommandTest.php index 2f88a861..b651db09 100644 --- a/module/CLI/test/Command/Tag/ListTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/ListTagsCommandTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\Tag; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\ListTagsCommand; use Shlinkio\Shlink\Core\Entity\Tag; @@ -32,12 +31,9 @@ class ListTagsCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function noTagsPrintsEmptyMessage() { - /** @var MethodProphecy $listTags */ $listTags = $this->tagService->listTags()->willReturn([]); $this->commandTester->execute([]); @@ -47,12 +43,9 @@ class ListTagsCommandTest extends TestCase $listTags->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function listOfTagsIsPrinted() { - /** @var MethodProphecy $listTags */ $listTags = $this->tagService->listTags()->willReturn([ new Tag('foo'), new Tag('bar'), diff --git a/module/CLI/test/Command/Tag/RenameTagCommandTest.php b/module/CLI/test/Command/Tag/RenameTagCommandTest.php index c6c8815b..f2dba891 100644 --- a/module/CLI/test/Command/Tag/RenameTagCommandTest.php +++ b/module/CLI/test/Command/Tag/RenameTagCommandTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\Tag; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\RenameTagCommand; use Shlinkio\Shlink\Core\Entity\Tag; @@ -33,14 +32,11 @@ class RenameTagCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function errorIsPrintedIfExceptionIsThrown() { $oldName = 'foo'; $newName = 'bar'; - /** @var MethodProphecy $renameTag */ $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(EntityDoesNotExistException::class); $this->commandTester->execute([ @@ -53,14 +49,11 @@ class RenameTagCommandTest extends TestCase $renameTag->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function successIsPrintedIfNoErrorOccurs() { $oldName = 'foo'; $newName = 'bar'; - /** @var MethodProphecy $renameTag */ $renameTag = $this->tagService->renameTag($oldName, $newName)->willReturn(new Tag($newName)); $this->commandTester->execute([ diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index cd438637..dbb311f2 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -9,18 +9,17 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; -use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Service\VisitService; use Symfony\Component\Console\Application; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; use Symfony\Component\Lock; -use Throwable; use function array_shift; use function sprintf; @@ -60,15 +59,13 @@ class ProcessVisitsCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ - public function allPendingVisitsAreProcessed() + /** @test */ + public function allPendingVisitsAreProcessed(): void { $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4')); - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); - $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will( + $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( function (array $args) use ($visit, $location) { $firstCallback = array_shift($args); $firstCallback($visit); @@ -77,7 +74,9 @@ class ProcessVisitsCommandTest extends TestCase $secondCallback($location, $visit); } ); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]); + $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn( + Location::emptyInstance() + ); $this->commandTester->execute([ 'command' => 'visit:process', @@ -93,12 +92,12 @@ class ProcessVisitsCommandTest extends TestCase * @test * @dataProvider provideIgnoredAddresses */ - public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message) + public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void { $visit = new Visit(new ShortUrl(''), new Visitor('', '', $address)); - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); - $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will( + $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( function (array $args) use ($visit, $location) { $firstCallback = array_shift($args); $firstCallback($visit); @@ -107,41 +106,34 @@ class ProcessVisitsCommandTest extends TestCase $secondCallback($location, $visit); } ); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]); + $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn( + Location::emptyInstance() + ); - try { - $this->commandTester->execute([ - 'command' => 'visit:process', - ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - } catch (Throwable $e) { - $output = $this->commandTester->getDisplay(); + $this->commandTester->execute([ + 'command' => 'visit:process', + ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - $this->assertInstanceOf(IpCannotBeLocatedException::class, $e); - - $this->assertStringContainsString($message, $output); - $locateVisits->shouldHaveBeenCalledOnce(); - $resolveIpLocation->shouldNotHaveBeenCalled(); - } + $output = $this->commandTester->getDisplay(); + $this->assertStringContainsString($message, $output); + $locateVisits->shouldHaveBeenCalledOnce(); + $resolveIpLocation->shouldNotHaveBeenCalled(); } - public function provideIgnoredAddresses(): array + public function provideIgnoredAddresses(): iterable { - return [ - ['', 'Ignored visit with no IP address'], - [null, 'Ignored visit with no IP address'], - [IpAddress::LOCALHOST, 'Ignored localhost address'], - ]; + yield 'with empty address' => ['', 'Ignored visit with no IP address']; + yield 'with null address' => [null, 'Ignored visit with no IP address']; + yield 'with localhost address' => [IpAddress::LOCALHOST, 'Ignored localhost address']; } - /** - * @test - */ - public function errorWhileLocatingIpIsDisplayed() + /** @test */ + public function errorWhileLocatingIpIsDisplayed(): void { $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4')); - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); - $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will( + $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( function (array $args) use ($visit, $location) { $firstCallback = array_shift($args); $firstCallback($visit); @@ -152,29 +144,23 @@ class ProcessVisitsCommandTest extends TestCase ); $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class); - try { - $this->commandTester->execute([ - 'command' => 'visit:process', - ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - } catch (Throwable $e) { - $output = $this->commandTester->getDisplay(); + $this->commandTester->execute([ + 'command' => 'visit:process', + ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - $this->assertInstanceOf(IpCannotBeLocatedException::class, $e); + $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('An error occurred while locating IP. Skipped', $output); - $locateVisits->shouldHaveBeenCalledOnce(); - $resolveIpLocation->shouldHaveBeenCalledOnce(); - } + $this->assertStringContainsString('An error occurred while locating IP. Skipped', $output); + $locateVisits->shouldHaveBeenCalledOnce(); + $resolveIpLocation->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function noActionIsPerformedIfLockIsAcquired() { $this->lock->acquire()->willReturn(false); - $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(function () { + $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(function () { }); $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]); diff --git a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php index b6d3caeb..80579513 100644 --- a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php +++ b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php @@ -30,9 +30,7 @@ class UpdateDbCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ + /** @test */ public function successMessageIsPrintedIfEverythingWorks() { $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->will(function () { @@ -45,9 +43,7 @@ class UpdateDbCommandTest extends TestCase $download->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function errorMessageIsPrintedIfAnExceptionIsThrown() { $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->willThrow(RuntimeException::class); diff --git a/module/CLI/test/ConfigProviderTest.php b/module/CLI/test/ConfigProviderTest.php index 8d937df2..3149f07b 100644 --- a/module/CLI/test/ConfigProviderTest.php +++ b/module/CLI/test/ConfigProviderTest.php @@ -16,9 +16,7 @@ class ConfigProviderTest extends TestCase $this->configProvider = new ConfigProvider(); } - /** - * @test - */ + /** @test */ public function confiIsProperlyReturned() { $config = ($this->configProvider)(); diff --git a/module/CLI/test/Factory/ApplicationFactoryTest.php b/module/CLI/test/Factory/ApplicationFactoryTest.php index 85d875fc..a533ed09 100644 --- a/module/CLI/test/Factory/ApplicationFactoryTest.php +++ b/module/CLI/test/Factory/ApplicationFactoryTest.php @@ -23,18 +23,14 @@ class ApplicationFactoryTest extends TestCase $this->factory = new ApplicationFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreated() { $instance = ($this->factory)($this->createServiceManager(), ''); $this->assertInstanceOf(Application::class, $instance); } - /** - * @test - */ + /** @test */ public function allCommandsWhichAreServicesAreAdded() { $sm = $this->createServiceManager([ diff --git a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php index 6484b577..6d8659fa 100644 --- a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php +++ b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php @@ -18,7 +18,7 @@ class ChainIpLocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { $error = null; diff --git a/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php b/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php index c9e1f4a4..11aa9995 100644 --- a/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php +++ b/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php @@ -10,16 +10,8 @@ class EmptyIpLocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { - return [ - 'country_code' => '', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => '', - 'longitude' => '', - 'time_zone' => '', - ]; + return Model\Location::emptyInstance(); } } diff --git a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php index c824f903..4aa4e868 100644 --- a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php +++ b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php @@ -24,7 +24,7 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { try { $city = $this->geoLiteDbReader->city($ipAddress); @@ -36,19 +36,19 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface } } - private function mapFields(City $city): array + private function mapFields(City $city): Model\Location { /** @var Subdivision $region */ $region = first($city->subdivisions); - return [ - 'country_code' => $city->country->isoCode ?? '', - 'country_name' => $city->country->name ?? '', - 'region_name' => $region->name ?? '', - 'city' => $city->city->name ?? '', - 'latitude' => $city->location->latitude ?? '', - 'longitude' => $city->location->longitude ?? '', - 'time_zone' => $city->location->timeZone ?? '', - ]; + return new Model\Location( + $city->country->isoCode ?? '', + $city->country->name ?? '', + $region->name ?? '', + $city->city->name ?? '', + (float) ($city->location->latitude ?? ''), + (float) ($city->location->longitude ?? ''), + $city->location->timeZone ?? '' + ); } } diff --git a/module/Common/src/IpGeolocation/IpApiLocationResolver.php b/module/Common/src/IpGeolocation/IpApiLocationResolver.php index 25b6f774..0f9fc9a3 100644 --- a/module/Common/src/IpGeolocation/IpApiLocationResolver.php +++ b/module/Common/src/IpGeolocation/IpApiLocationResolver.php @@ -25,7 +25,7 @@ class IpApiLocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { try { $response = $this->httpClient->get(sprintf(self::SERVICE_PATTERN, $ipAddress)); @@ -37,16 +37,16 @@ class IpApiLocationResolver implements IpLocationResolverInterface } } - private function mapFields(array $entry): array + private function mapFields(array $entry): Model\Location { - return [ - 'country_code' => $entry['countryCode'] ?? '', - 'country_name' => $entry['country'] ?? '', - 'region_name' => $entry['regionName'] ?? '', - 'city' => $entry['city'] ?? '', - 'latitude' => $entry['lat'] ?? '', - 'longitude' => $entry['lon'] ?? '', - 'time_zone' => $entry['timezone'] ?? '', - ]; + return new Model\Location( + (string) ($entry['countryCode'] ?? ''), + (string) ($entry['country'] ?? ''), + (string) ($entry['regionName'] ?? ''), + (string) ($entry['city'] ?? ''), + (float) ($entry['lat'] ?? 0.0), + (float) ($entry['lon'] ?? 0.0), + (string) ($entry['timezone'] ?? '') + ); } } diff --git a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php index 6017db61..ed73c0db 100644 --- a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php +++ b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php @@ -10,5 +10,5 @@ interface IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array; + public function resolveIpLocation(string $ipAddress): Model\Location; } diff --git a/module/Common/src/IpGeolocation/Model/Location.php b/module/Common/src/IpGeolocation/Model/Location.php new file mode 100644 index 00000000..4ddb706e --- /dev/null +++ b/module/Common/src/IpGeolocation/Model/Location.php @@ -0,0 +1,80 @@ +countryCode = $countryCode; + $this->countryName = $countryName; + $this->regionName = $regionName; + $this->city = $city; + $this->latitude = $latitude; + $this->longitude = $longitude; + $this->timeZone = $timeZone; + } + + public static function emptyInstance(): self + { + return new self('', '', '', '', 0.0, 0.0, ''); + } + + public function countryCode(): string + { + return $this->countryCode; + } + + public function countryName(): string + { + return $this->countryName; + } + + public function regionName(): string + { + return $this->regionName; + } + + public function city(): string + { + return $this->city; + } + + public function latitude(): float + { + return $this->latitude; + } + + public function longitude(): float + { + return $this->longitude; + } + + public function timeZone(): string + { + return $this->timeZone; + } +} diff --git a/module/Common/test/ConfigProviderTest.php b/module/Common/test/ConfigProviderTest.php index e98a772a..c253483e 100644 --- a/module/Common/test/ConfigProviderTest.php +++ b/module/Common/test/ConfigProviderTest.php @@ -16,9 +16,7 @@ class ConfigProviderTest extends TestCase $this->configProvider = new ConfigProvider(); } - /** - * @test - */ + /** @test */ public function configIsReturned() { $config = $this->configProvider->__invoke(); diff --git a/module/Common/test/Console/ShlinkTableTest.php b/module/Common/test/Console/ShlinkTableTest.php index 7a4c7135..dc75293a 100644 --- a/module/Common/test/Console/ShlinkTableTest.php +++ b/module/Common/test/Console/ShlinkTableTest.php @@ -25,9 +25,7 @@ class ShlinkTableTest extends TestCase $this->shlinkTable = new ShlinkTable($this->baseTable->reveal()); } - /** - * @test - */ + /** @test */ public function renderMakesTableToBeRenderedWithProvidedInfo() { $headers = []; @@ -54,9 +52,7 @@ class ShlinkTableTest extends TestCase $render->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function newTableIsCreatedForFactoryMethod() { $instance = ShlinkTable::fromOutput($this->prophesize(OutputInterface::class)->reveal()); diff --git a/module/Common/test/Exception/WrongIpExceptionTest.php b/module/Common/test/Exception/WrongIpExceptionTest.php index 1b6d0d08..2b7b3531 100644 --- a/module/Common/test/Exception/WrongIpExceptionTest.php +++ b/module/Common/test/Exception/WrongIpExceptionTest.php @@ -9,9 +9,7 @@ use Shlinkio\Shlink\Common\Exception\WrongIpException; class WrongIpExceptionTest extends TestCase { - /** - * @test - */ + /** @test */ public function fromIpAddressProperlyCreatesExceptionWithoutPrev() { $e = WrongIpException::fromIpAddress('1.2.3.4'); @@ -20,9 +18,7 @@ class WrongIpExceptionTest extends TestCase $this->assertEquals(0, $e->getCode()); $this->assertNull($e->getPrevious()); } - /** - * @test - */ + /** @test */ public function fromIpAddressProperlyCreatesExceptionWithPrev() { $prev = new Exception('Previous error'); diff --git a/module/Common/test/Factory/CacheFactoryTest.php b/module/Common/test/Factory/CacheFactoryTest.php index 16d143ca..614a787f 100644 --- a/module/Common/test/Factory/CacheFactoryTest.php +++ b/module/Common/test/Factory/CacheFactoryTest.php @@ -32,9 +32,7 @@ class CacheFactoryTest extends TestCase putenv('APP_ENV'); } - /** - * @test - */ + /** @test */ public function productionReturnsApcAdapter() { putenv('APP_ENV=pro'); @@ -42,9 +40,7 @@ class CacheFactoryTest extends TestCase $this->assertInstanceOf(ApcuCache::class, $instance); } - /** - * @test - */ + /** @test */ public function developmentReturnsArrayAdapter() { putenv('APP_ENV=dev'); @@ -52,9 +48,7 @@ class CacheFactoryTest extends TestCase $this->assertInstanceOf(ArrayCache::class, $instance); } - /** - * @test - */ + /** @test */ public function adapterDefinedInConfigIgnoresEnvironment() { putenv('APP_ENV=pro'); @@ -62,9 +56,7 @@ class CacheFactoryTest extends TestCase $this->assertInstanceOf(ArrayCache::class, $instance); } - /** - * @test - */ + /** @test */ public function invalidAdapterDefinedInConfigFallbacksToEnvironment() { putenv('APP_ENV=pro'); @@ -72,9 +64,7 @@ class CacheFactoryTest extends TestCase $this->assertInstanceOf(ApcuCache::class, $instance); } - /** - * @test - */ + /** @test */ public function filesystemCacheAdaptersReadDirOption() { $dir = realpath(sys_get_temp_dir()); @@ -84,9 +74,7 @@ class CacheFactoryTest extends TestCase $this->assertEquals($dir, $instance->getDirectory()); } - /** - * @test - */ + /** @test */ public function memcachedCacheAdaptersReadServersOption() { $servers = [ diff --git a/module/Common/test/Factory/DottedAccessConfigAbstractFactoryTest.php b/module/Common/test/Factory/DottedAccessConfigAbstractFactoryTest.php index f5a3a6e1..f9d2c80c 100644 --- a/module/Common/test/Factory/DottedAccessConfigAbstractFactoryTest.php +++ b/module/Common/test/Factory/DottedAccessConfigAbstractFactoryTest.php @@ -20,30 +20,23 @@ class DottedAccessConfigAbstractFactoryTest extends TestCase } /** - * @param string $serviceName - * @param bool $canCreate - * * @test * @dataProvider provideDotNames */ - public function canCreateOnlyServicesWithDot(string $serviceName, bool $canCreate) + public function canCreateOnlyServicesWithDot(string $serviceName, bool $canCreate): void { $this->assertEquals($canCreate, $this->factory->canCreate(new ServiceManager(), $serviceName)); } - public function provideDotNames(): array + public function provideDotNames(): iterable { - return [ - ['foo.bar', true], - ['config.something', true], - ['config_something', false], - ['foo', false], - ]; + yield 'with a valid service' => ['foo.bar', true]; + yield 'with another valid service' => ['config.something', true]; + yield 'with an invalid service' => ['config_something', false]; + yield 'with another invalid service' => ['foo', false]; } - /** - * @test - */ + /** @test */ public function throwsExceptionWhenFirstPartOfTheServiceIsNotRegistered() { $this->expectException(ServiceNotCreatedException::class); @@ -54,9 +47,7 @@ class DottedAccessConfigAbstractFactoryTest extends TestCase $this->factory->__invoke(new ServiceManager(), 'foo.bar'); } - /** - * @test - */ + /** @test */ public function dottedNotationIsRecursivelyResolvedUntilLastValueIsFoundAndReturned() { $expected = 'this is the result'; @@ -70,9 +61,7 @@ class DottedAccessConfigAbstractFactoryTest extends TestCase $this->assertEquals($expected, $result); } - /** - * @test - */ + /** @test */ public function exceptionIsThrownIfAnyStepCannotBeResolved() { $this->expectException(InvalidArgumentException::class); diff --git a/module/Common/test/Factory/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php b/module/Common/test/Factory/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php index 3d21f261..e6b42c09 100644 --- a/module/Common/test/Factory/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php +++ b/module/Common/test/Factory/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php @@ -20,18 +20,14 @@ class EmptyResponseImplicitOptionsMiddlewareFactoryTest extends TestCase $this->factory = new EmptyResponseImplicitOptionsMiddlewareFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreated() { $instance = $this->factory->__invoke(new ServiceManager(), ''); $this->assertInstanceOf(ImplicitOptionsMiddleware::class, $instance); } - /** - * @test - */ + /** @test */ public function responsePrototypeIsEmptyResponse() { $instance = $this->factory->__invoke(new ServiceManager(), ''); diff --git a/module/Common/test/Factory/EntityManagerFactoryTest.php b/module/Common/test/Factory/EntityManagerFactoryTest.php index ec70f59e..70a969f9 100644 --- a/module/Common/test/Factory/EntityManagerFactoryTest.php +++ b/module/Common/test/Factory/EntityManagerFactoryTest.php @@ -18,9 +18,7 @@ class EntityManagerFactoryTest extends TestCase $this->factory = new EntityManagerFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreated() { $sm = new ServiceManager(['services' => [ diff --git a/module/Common/test/Factory/LoggerFactoryTest.php b/module/Common/test/Factory/LoggerFactoryTest.php index adcb5865..9f4a8a3c 100644 --- a/module/Common/test/Factory/LoggerFactoryTest.php +++ b/module/Common/test/Factory/LoggerFactoryTest.php @@ -19,9 +19,7 @@ class LoggerFactoryTest extends TestCase $this->factory = new LoggerFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreated() { /** @var Logger $instance */ @@ -30,9 +28,7 @@ class LoggerFactoryTest extends TestCase $this->assertEquals('Logger', $instance->getName()); } - /** - * @test - */ + /** @test */ public function nameIsSetFromOptions() { /** @var Logger $instance */ @@ -41,9 +37,7 @@ class LoggerFactoryTest extends TestCase $this->assertEquals('Foo', $instance->getName()); } - /** - * @test - */ + /** @test */ public function serviceNameOverwritesOptionsLoggerName() { /** @var Logger $instance */ diff --git a/module/Common/test/Factory/TranslatorFactoryTest.php b/module/Common/test/Factory/TranslatorFactoryTest.php index 4b099d25..07cbfd5b 100644 --- a/module/Common/test/Factory/TranslatorFactoryTest.php +++ b/module/Common/test/Factory/TranslatorFactoryTest.php @@ -18,9 +18,7 @@ class TranslatorFactoryTest extends TestCase $this->factory = new TranslatorFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreated() { $instance = $this->factory->__invoke(new ServiceManager(['services' => [ diff --git a/module/Common/test/Image/ImageBuilderFactoryTest.php b/module/Common/test/Image/ImageBuilderFactoryTest.php index 9cfbb2e5..8a5aaab1 100644 --- a/module/Common/test/Image/ImageBuilderFactoryTest.php +++ b/module/Common/test/Image/ImageBuilderFactoryTest.php @@ -18,9 +18,7 @@ class ImageBuilderFactoryTest extends TestCase $this->factory = new ImageBuilderFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreated() { $instance = $this->factory->__invoke(new ServiceManager(), ''); diff --git a/module/Common/test/Image/ImageFactoryTest.php b/module/Common/test/Image/ImageFactoryTest.php index f94221b6..eb0e4af0 100644 --- a/module/Common/test/Image/ImageFactoryTest.php +++ b/module/Common/test/Image/ImageFactoryTest.php @@ -19,9 +19,7 @@ class ImageFactoryTest extends TestCase $this->factory = new ImageFactory(); } - /** - * @test - */ + /** @test */ public function noPageIsSetWhenOptionsAreNotProvided() { /** @var Image $image */ @@ -36,9 +34,7 @@ class ImageFactoryTest extends TestCase $this->assertNull($page->getValue($image)); } - /** - * @test - */ + /** @test */ public function aPageIsSetWhenOptionsIncludeTheUrl() { $expectedPage = 'foo/bar.html'; diff --git a/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php b/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php index 49bb0873..36d3e6b5 100644 --- a/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php @@ -8,6 +8,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\ChainIpLocationResolver; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; class ChainIpLocationResolverTest extends TestCase { @@ -29,9 +30,7 @@ class ChainIpLocationResolverTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function throwsExceptionWhenNoInnerResolverCanHandleTheResolution() { $ipAddress = '1.2.3.4'; @@ -46,14 +45,12 @@ class ChainIpLocationResolverTest extends TestCase $this->resolver->resolveIpLocation($ipAddress); } - /** - * @test - */ - public function returnsResultOfFirstInnerResolver() + /** @test */ + public function returnsResultOfFirstInnerResolver(): void { $ipAddress = '1.2.3.4'; - $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willReturn([]); + $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willReturn(Location::emptyInstance()); $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); $this->resolver->resolveIpLocation($ipAddress); @@ -62,15 +59,15 @@ class ChainIpLocationResolverTest extends TestCase $secondResolve->shouldNotHaveBeenCalled(); } - /** - * @test - */ - public function returnsResultOfSecondInnerResolver() + /** @test */ + public function returnsResultOfSecondInnerResolver(): void { $ipAddress = '1.2.3.4'; $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); - $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willReturn([]); + $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willReturn( + Location::emptyInstance() + ); $this->resolver->resolveIpLocation($ipAddress); diff --git a/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php b/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php index 2c2962bb..da5e0f98 100644 --- a/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php @@ -5,6 +5,7 @@ namespace ShlinkioTest\Shlink\Common\IpGeolocation; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\IpGeolocation\EmptyIpLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use function Functional\map; use function range; @@ -13,16 +14,6 @@ class EmptyIpLocationResolverTest extends TestCase { use StringUtilsTrait; - private const EMPTY_RESP = [ - 'country_code' => '', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => '', - 'longitude' => '', - 'time_zone' => '', - ]; - /** @var EmptyIpLocationResolver */ private $resolver; @@ -35,15 +26,15 @@ class EmptyIpLocationResolverTest extends TestCase * @test * @dataProvider provideEmptyResponses */ - public function alwaysReturnsAnEmptyResponse(array $expected, string $ipAddress) + public function alwaysReturnsAnEmptyLocation(string $ipAddress): void { - $this->assertEquals($expected, $this->resolver->resolveIpLocation($ipAddress)); + $this->assertEquals(Location::emptyInstance(), $this->resolver->resolveIpLocation($ipAddress)); } public function provideEmptyResponses(): array { return map(range(0, 5), function () { - return [self::EMPTY_RESP, $this->generateRandomString(10)]; + return [$this->generateRandomString(15)]; }); } } diff --git a/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php b/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php index 05a9a68f..d2c9cc5e 100644 --- a/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php +++ b/module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php @@ -39,10 +39,8 @@ class DbUpdaterTest extends TestCase $this->dbUpdater = new DbUpdater($this->httpClient->reveal(), $this->filesystem->reveal(), $this->options); } - /** - * @test - */ - public function anExceptionIsThrownIfFreshDbCannotBeDownloaded() + /** @test */ + public function anExceptionIsThrownIfFreshDbCannotBeDownloaded(): void { $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class); @@ -56,10 +54,8 @@ class DbUpdaterTest extends TestCase $this->dbUpdater->downloadFreshCopy(); } - /** - * @test - */ - public function anExceptionIsThrownIfFreshDbCannotBeExtracted() + /** @test */ + public function anExceptionIsThrownIfFreshDbCannotBeExtracted(): void { $this->options->tempDir = '__invalid__'; @@ -79,7 +75,7 @@ class DbUpdaterTest extends TestCase * @test * @dataProvider provideFilesystemExceptions */ - public function anExceptionIsThrownIfFreshDbCannotBeCopiedToDestination(string $e) + public function anExceptionIsThrownIfFreshDbCannotBeCopiedToDestination(string $e): void { $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); $copy = $this->filesystem->copy(Argument::cetera())->willThrow($e); @@ -93,18 +89,14 @@ class DbUpdaterTest extends TestCase $this->dbUpdater->downloadFreshCopy(); } - public function provideFilesystemExceptions(): array + public function provideFilesystemExceptions(): iterable { - return [ - [FilesystemException\FileNotFoundException::class], - [FilesystemException\IOException::class], - ]; + yield 'file not found' => [FilesystemException\FileNotFoundException::class]; + yield 'IO error' => [FilesystemException\IOException::class]; } - /** - * @test - */ - public function noExceptionsAreThrownIfEverythingWorksFine() + /** @test */ + public function noExceptionsAreThrownIfEverythingWorksFine(): void { $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); $copy = $this->filesystem->copy(Argument::cetera())->will(function () { diff --git a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php index 2c5209f0..0f58515b 100644 --- a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php +++ b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php @@ -11,6 +11,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2LocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; class GeoLite2LocationResolverTest extends TestCase { @@ -29,7 +30,7 @@ class GeoLite2LocationResolverTest extends TestCase * @test * @dataProvider provideReaderExceptions */ - public function exceptionIsThrownIfReaderThrowsException(string $e, string $message) + public function exceptionIsThrownIfReaderThrowsException(string $e, string $message): void { $ipAddress = '1.2.3.4'; @@ -43,18 +44,14 @@ class GeoLite2LocationResolverTest extends TestCase $this->resolver->resolveIpLocation($ipAddress); } - public function provideReaderExceptions(): array + public function provideReaderExceptions(): iterable { - return [ - [AddressNotFoundException::class, 'Provided IP "1.2.3.4" is invalid'], - [InvalidDatabaseException::class, 'Provided GeoLite2 db file is invalid'], - ]; + yield 'invalid IP address' => [AddressNotFoundException::class, 'Provided IP "1.2.3.4" is invalid']; + yield 'invalid geolite DB' => [InvalidDatabaseException::class, 'Provided GeoLite2 db file is invalid']; } - /** - * @test - */ - public function resolvedCityIsProperlyMapped() + /** @test */ + public function resolvedCityIsProperlyMapped(): void { $ipAddress = '1.2.3.4'; $city = new City([]); @@ -63,15 +60,7 @@ class GeoLite2LocationResolverTest extends TestCase $result = $this->resolver->resolveIpLocation($ipAddress); - $this->assertEquals([ - 'country_code' => '', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => '', - 'longitude' => '', - 'time_zone' => '', - ], $result); + $this->assertEquals(Location::emptyInstance(), $result); $cityMethod->shouldHaveBeenCalledOnce(); } } diff --git a/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php b/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php index c830fc19..cc0c29b3 100644 --- a/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use function json_encode; class IpApiLocationResolverTest extends TestCase @@ -25,25 +26,15 @@ class IpApiLocationResolverTest extends TestCase $this->ipResolver = new IpApiLocationResolver($this->client->reveal()); } - /** - * @test - */ - public function correctIpReturnsDecodedInfo() + /** @test */ + public function correctIpReturnsDecodedInfo(): void { $actual = [ 'countryCode' => 'bar', 'lat' => 5, 'lon' => 10, ]; - $expected = [ - 'country_code' => 'bar', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => 5, - 'longitude' => 10, - 'time_zone' => '', - ]; + $expected = new Location('bar', '', '', '', 5, 10, ''); $response = new Response(); $response->getBody()->write(json_encode($actual)); $response->getBody()->rewind(); @@ -54,7 +45,7 @@ class IpApiLocationResolverTest extends TestCase } /** @test */ - public function guzzleExceptionThrowsShlinkException() + public function guzzleExceptionThrowsShlinkException(): void { $this->client->get('http://ip-api.com/json/1.2.3.4')->willThrow(new TransferException()) ->shouldBeCalledOnce(); diff --git a/module/Common/test/Logger/Processor/ExceptionWithNewLineProcessorTest.php b/module/Common/test/Logger/Processor/ExceptionWithNewLineProcessorTest.php index 182dc235..96520dcf 100644 --- a/module/Common/test/Logger/Processor/ExceptionWithNewLineProcessorTest.php +++ b/module/Common/test/Logger/Processor/ExceptionWithNewLineProcessorTest.php @@ -5,10 +5,15 @@ namespace ShlinkioTest\Shlink\Common\Logger\Processor; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Logger\Processor\ExceptionWithNewLineProcessor; +use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use const PHP_EOL; +use function Functional\map; +use function range; class ExceptionWithNewLineProcessorTest extends TestCase { + use StringUtilsTrait; + /** @var ExceptionWithNewLineProcessor */ private $processor; @@ -21,44 +26,40 @@ class ExceptionWithNewLineProcessorTest extends TestCase * @test * @dataProvider provideNoPlaceholderRecords */ - public function keepsRecordAsIsWhenNoPlaceholderExists(array $record) + public function keepsRecordAsIsWhenNoPlaceholderExists(array $record): void { $this->assertSame($record, ($this->processor)($record)); } - public function provideNoPlaceholderRecords(): array + public function provideNoPlaceholderRecords(): iterable { - return [ - [['message' => 'Hello World']], - [['message' => 'Shlink']], - [['message' => 'Foo bar']], - ]; + return map(range(1, 5), function () { + return [['message' => $this->generateRandomString()]]; + }); } /** * @test * @dataProvider providePlaceholderRecords */ - public function properlyReplacesExceptionPlaceholderAddingNewLine(array $record, array $expected) + public function properlyReplacesExceptionPlaceholderAddingNewLine(array $record, array $expected): void { $this->assertEquals($expected, ($this->processor)($record)); } - public function providePlaceholderRecords(): array + public function providePlaceholderRecords(): iterable { - return [ - [ - ['message' => 'Hello World with placeholder {e}'], - ['message' => 'Hello World with placeholder ' . PHP_EOL . '{e}'], - ], - [ - ['message' => '{e} Shlink'], - ['message' => PHP_EOL . '{e} Shlink'], - ], - [ - ['message' => 'Foo {e} bar'], - ['message' => 'Foo ' . PHP_EOL . '{e} bar'], - ], + yield [ + ['message' => 'Hello World with placeholder {e}'], + ['message' => 'Hello World with placeholder ' . PHP_EOL . '{e}'], + ]; + yield [ + ['message' => '{e} Shlink'], + ['message' => PHP_EOL . '{e} Shlink'], + ]; + yield [ + ['message' => 'Foo {e} bar'], + ['message' => 'Foo ' . PHP_EOL . '{e} bar'], ]; } } diff --git a/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php b/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php index f187559d..cd95b26d 100644 --- a/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php +++ b/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php @@ -29,9 +29,7 @@ class CloseDbConnectionMiddlewareTest extends TestCase $this->middleware = new CloseDbConnectionMiddleware($this->em->reveal()); } - /** - * @test - */ + /** @test */ public function connectionIsClosedWhenMiddlewareIsProcessed() { $req = new ServerRequest(); diff --git a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php index bb93593e..41fbafbb 100644 --- a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php +++ b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php @@ -18,9 +18,7 @@ class IpAddressMiddlewareFactoryTest extends TestCase $this->factory = new IpAddressMiddlewareFactory(); } - /** - * @test - */ + /** @test */ public function returnedInstanceIsProperlyConfigured() { $instance = $this->factory->__invoke(new ServiceManager(), ''); diff --git a/module/Common/test/Middleware/LocaleMiddlewareTest.php b/module/Common/test/Middleware/LocaleMiddlewareTest.php index 0cac4546..e9145cba 100644 --- a/module/Common/test/Middleware/LocaleMiddlewareTest.php +++ b/module/Common/test/Middleware/LocaleMiddlewareTest.php @@ -22,20 +22,16 @@ class LocaleMiddlewareTest extends TestCase $this->middleware = new LocaleMiddleware($this->translator); } - /** - * @test - */ - public function whenNoHeaderIsPresentLocaleIsNotChanged() + /** @test */ + public function whenNoHeaderIsPresentLocaleIsNotChanged(): void { $this->assertEquals('ru', $this->translator->getLocale()); $this->middleware->process(new ServerRequest(), TestUtils::createReqHandlerMock()->reveal()); $this->assertEquals('ru', $this->translator->getLocale()); } - /** - * @test - */ - public function whenTheHeaderIsPresentLocaleIsChanged() + /** @test */ + public function whenTheHeaderIsPresentLocaleIsChanged(): void { $this->assertEquals('ru', $this->translator->getLocale()); $request = (new ServerRequest())->withHeader('Accept-Language', 'es'); @@ -47,7 +43,7 @@ class LocaleMiddlewareTest extends TestCase * @test * @dataProvider provideLanguages */ - public function localeGetsNormalized(string $lang, string $expected) + public function localeGetsNormalized(string $lang, string $expected): void { $handler = TestUtils::createReqHandlerMock(); @@ -58,12 +54,10 @@ class LocaleMiddlewareTest extends TestCase $this->assertEquals($expected, $this->translator->getLocale()); } - public function provideLanguages(): array + public function provideLanguages(): iterable { - return [ - ['ru', 'ru'], - ['es_ES', 'es'], - ['en-US', 'en'], - ]; + yield 'language only' => ['ru', 'ru']; + yield 'country and language with underscore' => ['es_ES', 'es']; + yield 'country and language with dash' => ['en-US', 'en']; } } diff --git a/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php b/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php index b39b3573..4e63ba78 100644 --- a/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php +++ b/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php @@ -21,18 +21,14 @@ class PaginableRepositoryAdapterTest extends TestCase $this->adapter = new PaginableRepositoryAdapter($this->repo->reveal(), 'search', ['foo', 'bar'], 'order'); } - /** - * @test - */ + /** @test */ public function getItemsFallbacksToFindList() { $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order')->shouldBeCalledOnce(); $this->adapter->getItems(5, 10); } - /** - * @test - */ + /** @test */ public function countFallbacksToCountList() { $this->repo->countList('search', ['foo', 'bar'])->shouldBeCalledOnce(); diff --git a/module/Common/test/Response/PixelResponseTest.php b/module/Common/test/Response/PixelResponseTest.php index 62e83f0d..ae580472 100644 --- a/module/Common/test/Response/PixelResponseTest.php +++ b/module/Common/test/Response/PixelResponseTest.php @@ -16,9 +16,7 @@ class PixelResponseTest extends TestCase $this->resp = new PixelResponse(); } - /** - * @test - */ + /** @test */ public function responseHasGifTypeAndIsNotEmpty() { $this->assertEquals('image/gif', $this->resp->getHeaderLine('Content-Type')); diff --git a/module/Common/test/Response/QrCodeResponseTest.php b/module/Common/test/Response/QrCodeResponseTest.php index 36d6fada..7b413bc4 100644 --- a/module/Common/test/Response/QrCodeResponseTest.php +++ b/module/Common/test/Response/QrCodeResponseTest.php @@ -9,9 +9,7 @@ use Shlinkio\Shlink\Common\Response\QrCodeResponse; class QrCodeResponseTest extends TestCase { - /** - * @test - */ + /** @test */ public function providedQrCoideIsSetAsBody() { $qrCode = new QrCode('Hello'); diff --git a/module/Common/test/Service/PreviewGeneratorTest.php b/module/Common/test/Service/PreviewGeneratorTest.php index ad961ca3..470a6eae 100644 --- a/module/Common/test/Service/PreviewGeneratorTest.php +++ b/module/Common/test/Service/PreviewGeneratorTest.php @@ -38,9 +38,7 @@ class PreviewGeneratorTest extends TestCase ]), $this->filesystem->reveal(), 'dir'); } - /** - * @test - */ + /** @test */ public function alreadyProcessedElementsAreNotProcessed() { $url = 'http://foo.com'; @@ -50,9 +48,7 @@ class PreviewGeneratorTest extends TestCase $this->assertEquals(sprintf('dir/preview_%s.png', urlencode($url)), $this->generator->generatePreview($url)); } - /** - * @test - */ + /** @test */ public function nonProcessedElementsAreProcessed() { $url = 'http://foo.com'; diff --git a/module/Common/test/Template/Extension/TranslatorExtensionTest.php b/module/Common/test/Template/Extension/TranslatorExtensionTest.php index 06deaa21..690daabf 100644 --- a/module/Common/test/Template/Extension/TranslatorExtensionTest.php +++ b/module/Common/test/Template/Extension/TranslatorExtensionTest.php @@ -19,9 +19,7 @@ class TranslatorExtensionTest extends TestCase $this->extension = new TranslatorExtension($this->prophesize(Translator::class)->reveal()); } - /** - * @test - */ + /** @test */ public function properFunctionsAreReturned() { $engine = $this->prophesize(Engine::class); diff --git a/module/Common/test/Type/ChronosDateTimeTypeTest.php b/module/Common/test/Type/ChronosDateTimeTypeTest.php index e3744d82..34c98486 100644 --- a/module/Common/test/Type/ChronosDateTimeTypeTest.php +++ b/module/Common/test/Type/ChronosDateTimeTypeTest.php @@ -28,10 +28,8 @@ class ChronosDateTimeTypeTest extends TestCase $this->type = Type::getType(ChronosDateTimeType::CHRONOS_DATETIME); } - /** - * @test - */ - public function nameIsReturned() + /** @test */ + public function nameIsReturned(): void { $this->assertEquals(ChronosDateTimeType::CHRONOS_DATETIME, $this->type->getName()); } @@ -40,7 +38,7 @@ class ChronosDateTimeTypeTest extends TestCase * @test * @dataProvider provideValues */ - public function valueIsConverted(?string $value, ?string $expected) + public function valueIsConverted(?string $value, ?string $expected): void { $platform = $this->prophesize(AbstractPlatform::class); $platform->getDateTimeFormatString()->willReturn('Y-m-d H:i:s'); @@ -54,20 +52,18 @@ class ChronosDateTimeTypeTest extends TestCase } } - public function provideValues(): array + public function provideValues(): iterable { - return [ - [null, null], - ['now', Chronos::class], - ['2017-01-01', Chronos::class], - ]; + yield 'null date' => [null, null]; + yield 'human friendly date' => ['now', Chronos::class]; + yield 'numeric date' => ['2017-01-01', Chronos::class]; } /** * @test * @dataProvider providePhpValues */ - public function valueIsConvertedToDatabaseFormat(?DateTimeInterface $value, ?string $expected) + public function valueIsConvertedToDatabaseFormat(?DateTimeInterface $value, ?string $expected): void { $platform = $this->prophesize(AbstractPlatform::class); $platform->getDateTimeFormatString()->willReturn('Y-m-d'); @@ -75,20 +71,16 @@ class ChronosDateTimeTypeTest extends TestCase $this->assertEquals($expected, $this->type->convertToDatabaseValue($value, $platform->reveal())); } - public function providePhpValues(): array + public function providePhpValues(): iterable { - return [ - [null, null], - [new DateTimeImmutable('2017-01-01'), '2017-01-01'], - [Chronos::parse('2017-02-01'), '2017-02-01'], - [new DateTime('2017-03-01'), '2017-03-01'], - ]; + yield 'null date' => [null, null]; + yield 'DateTimeImmutable date' => [new DateTimeImmutable('2017-01-01'), '2017-01-01']; + yield 'Chronos date' => [Chronos::parse('2017-02-01'), '2017-02-01']; + yield 'DateTime date' => [new DateTime('2017-03-01'), '2017-03-01']; } - /** - * @test - */ - public function exceptionIsThrownIfInvalidValueIsParsedToDatabase() + /** @test */ + public function exceptionIsThrownIfInvalidValueIsParsedToDatabase(): void { $this->expectException(ConversionException::class); $this->type->convertToDatabaseValue(new stdClass(), $this->prophesize(AbstractPlatform::class)->reveal()); diff --git a/module/Common/test/Util/DateRangeTest.php b/module/Common/test/Util/DateRangeTest.php index ec5602a2..d1716e8a 100644 --- a/module/Common/test/Util/DateRangeTest.php +++ b/module/Common/test/Util/DateRangeTest.php @@ -9,9 +9,7 @@ use Shlinkio\Shlink\Common\Util\DateRange; class DateRangeTest extends TestCase { - /** - * @test - */ + /** @test */ public function defaultConstructorSetDatesToNull() { $range = new DateRange(); @@ -20,9 +18,7 @@ class DateRangeTest extends TestCase $this->assertTrue($range->isEmpty()); } - /** - * @test - */ + /** @test */ public function providedDatesAreSet() { $startDate = Chronos::now(); @@ -37,19 +33,20 @@ class DateRangeTest extends TestCase * @test * @dataProvider provideDates */ - public function isConsideredEmptyOnlyIfNoneOfTheDatesIsSet(?Chronos $startDate, ?Chronos $endDate, bool $isEmpty) - { + public function isConsideredEmptyOnlyIfNoneOfTheDatesIsSet( + ?Chronos $startDate, + ?Chronos $endDate, + bool $isEmpty + ): void { $range = new DateRange($startDate, $endDate); $this->assertEquals($isEmpty, $range->isEmpty()); } - public function provideDates(): array + public function provideDates(): iterable { - return [ - [null, null, true], - [null, Chronos::now(), false], - [Chronos::now(), null, false], - [Chronos::now(), Chronos::now(), false], - ]; + yield 'both are null' => [null, null, true]; + yield 'start is null' => [null, Chronos::now(), false]; + yield 'end is null' => [Chronos::now(), null, false]; + yield 'none are null' => [Chronos::now(), Chronos::now(), false]; } } diff --git a/module/Common/test/Util/StringUtilsTraitTest.php b/module/Common/test/Util/StringUtilsTraitTest.php index b64f4738..0d05a1fd 100644 --- a/module/Common/test/Util/StringUtilsTraitTest.php +++ b/module/Common/test/Util/StringUtilsTraitTest.php @@ -5,6 +5,8 @@ namespace ShlinkioTest\Shlink\Common\Util; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Util\StringUtilsTrait; +use function Functional\map; +use function range; use function strlen; class StringUtilsTraitTest extends TestCase @@ -15,23 +17,19 @@ class StringUtilsTraitTest extends TestCase * @test * @dataProvider provideLengths */ - public function generateRandomStringGeneratesStringOfProvidedLength(int $length) + public function generateRandomStringGeneratesStringOfProvidedLength(int $length): void { $this->assertEquals($length, strlen($this->generateRandomString($length))); } public function provideLengths(): array { - return [ - [1], - [10], - [15], - ]; + return map(range(10, 50, 5), function (int $i) { + return [$i]; + }); } - /** - * @test - */ + /** @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}/'; diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 928a243a..8910ae1d 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -82,7 +82,7 @@ class Visit extends AbstractEntity implements JsonSerializable { return [ 'referer' => $this->referer, - 'date' => isset($this->date) ? $this->date->toAtomString() : null, + 'date' => $this->date !== null ? $this->date->toAtomString() : null, 'userAgent' => $this->userAgent, 'visitLocation' => $this->visitLocation, diff --git a/module/Core/src/Entity/VisitLocation.php b/module/Core/src/Entity/VisitLocation.php index ad68117b..b8376c01 100644 --- a/module/Core/src/Entity/VisitLocation.php +++ b/module/Core/src/Entity/VisitLocation.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Entity; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; -use function array_key_exists; class VisitLocation extends AbstractEntity implements VisitLocationInterface { @@ -24,9 +24,9 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface /** @var string */ private $timezone; - public function __construct(array $locationInfo) + public function __construct(Location $location) { - $this->exchangeArray($locationInfo); + $this->exchangeLocationInfo($location); } public function getCountryName(): string @@ -49,32 +49,15 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface return $this->cityName ?? ''; } - /** - * Exchange internal values from provided array - */ - private function exchangeArray(array $array): void + private function exchangeLocationInfo(Location $info): void { - if (array_key_exists('country_code', $array)) { - $this->countryCode = (string) $array['country_code']; - } - if (array_key_exists('country_name', $array)) { - $this->countryName = (string) $array['country_name']; - } - if (array_key_exists('region_name', $array)) { - $this->regionName = (string) $array['region_name']; - } - if (array_key_exists('city', $array)) { - $this->cityName = (string) $array['city']; - } - if (array_key_exists('latitude', $array)) { - $this->latitude = (string) $array['latitude']; - } - if (array_key_exists('longitude', $array)) { - $this->longitude = (string) $array['longitude']; - } - if (array_key_exists('time_zone', $array)) { - $this->timezone = (string) $array['time_zone']; - } + $this->countryCode = $info->countryCode(); + $this->countryName = $info->countryName(); + $this->regionName = $info->regionName(); + $this->cityName = $info->city(); + $this->latitude = (string) $info->latitude(); + $this->longitude = (string) $info->longitude(); + $this->timezone = $info->timeZone(); } public function jsonSerialize(): array diff --git a/module/Core/src/Exception/InvalidShortCodeException.php b/module/Core/src/Exception/InvalidShortCodeException.php index 59a361bb..504bbc88 100644 --- a/module/Core/src/Exception/InvalidShortCodeException.php +++ b/module/Core/src/Exception/InvalidShortCodeException.php @@ -3,12 +3,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; -use Exception; +use Throwable; use function sprintf; class InvalidShortCodeException extends RuntimeException { - public static function fromCharset(string $shortCode, string $charSet, Exception $previous = null): self + public static function fromCharset(string $shortCode, string $charSet, ?Throwable $previous = null): self { $code = $previous !== null ? $previous->getCode() : -1; return new static( diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 4291f0ec..1a1c2d3a 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -8,7 +8,7 @@ use function sprintf; class InvalidUrlException extends RuntimeException { - public static function fromUrl(string $url, Throwable $previous = null) + public static function fromUrl(string $url, Throwable $previous = null): self { $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/src/Service/VisitService.php b/module/Core/src/Service/VisitService.php index a9952e02..705dab59 100644 --- a/module/Core/src/Service/VisitService.php +++ b/module/Core/src/Service/VisitService.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; @@ -19,7 +20,7 @@ class VisitService implements VisitServiceInterface $this->em = $em; } - public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void + public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void { /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); @@ -27,26 +28,27 @@ class VisitService implements VisitServiceInterface foreach ($results as [$visit]) { try { - $locationData = $getGeolocationData($visit); + /** @var Location $location */ + $location = $geolocateVisit($visit); } catch (IpCannotBeLocatedException $e) { // Skip if the visit's IP could not be located continue; } - $location = new VisitLocation($locationData); - $this->locateVisit($visit, $location, $locatedVisit); + $location = new VisitLocation($location); + $this->locateVisit($visit, $location, $notifyVisitWithLocation); } } - private function locateVisit(Visit $visit, VisitLocation $location, ?callable $locatedVisit): void + private function locateVisit(Visit $visit, VisitLocation $location, ?callable $notifyVisitWithLocation): void { $visit->locate($location); $this->em->persist($visit); $this->em->flush(); - if ($locatedVisit !== null) { - $locatedVisit($location, $visit); + if ($notifyVisitWithLocation !== null) { + $notifyVisitWithLocation($location, $visit); } $this->em->clear(); diff --git a/module/Core/src/Service/VisitServiceInterface.php b/module/Core/src/Service/VisitServiceInterface.php index 907729fa..089567d2 100644 --- a/module/Core/src/Service/VisitServiceInterface.php +++ b/module/Core/src/Service/VisitServiceInterface.php @@ -5,5 +5,5 @@ namespace Shlinkio\Shlink\Core\Service; interface VisitServiceInterface { - public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void; + public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void; } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index fdca0e87..b623cacd 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -30,9 +30,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->repo = $this->getEntityManager()->getRepository(ShortUrl::class); } - /** - * @test - */ + /** @test */ public function findOneByShortCodeReturnsProperData() { $foo = new ShortUrl('foo'); @@ -62,9 +60,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertNull($this->repo->findOneByShortCode($baz->getShortCode())); } - /** - * @test - */ + /** @test */ public function countListReturnsProperNumberOfResults() { $count = 5; @@ -78,9 +74,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertEquals($count, $this->repo->countList()); } - /** - * @test - */ + /** @test */ public function findListProperlyFiltersByTagAndSearchTerm() { $tag = new Tag('bar'); @@ -125,9 +119,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertSame($bar, $result[0]); } - /** - * @test - */ + /** @test */ public function findListProperlyMapsFieldNamesToColumnNamesWhenOrdering() { $urls = ['a', 'z', 'c', 'b']; diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index 9f25cabd..1896bc9d 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -21,17 +21,13 @@ class TagRepositoryTest extends DatabaseTestCase $this->repo = $this->getEntityManager()->getRepository(Tag::class); } - /** - * @test - */ + /** @test */ public function deleteByNameDoesNothingWhenEmptyListIsProvided() { $this->assertEquals(0, $this->repo->deleteByName([])); } - /** - * @test - */ + /** @test */ public function allTagsWhichMatchNameAreDeleted() { $names = ['foo', 'bar', 'baz']; diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index a9ee6f35..db797920 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; @@ -29,10 +30,8 @@ class VisitRepositoryTest extends DatabaseTestCase $this->repo = $this->getEntityManager()->getRepository(Visit::class); } - /** - * @test - */ - public function findUnlocatedVisitsReturnsProperVisits() + /** @test */ + public function findUnlocatedVisitsReturnsProperVisits(): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); @@ -41,7 +40,7 @@ class VisitRepositoryTest extends DatabaseTestCase $visit = new Visit($shortUrl, Visitor::emptyInstance()); if ($i % 2 === 0) { - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); $this->getEntityManager()->persist($location); $visit->locate($location); } @@ -59,10 +58,8 @@ class VisitRepositoryTest extends DatabaseTestCase $this->assertEquals(3, $resultsCount); } - /** - * @test - */ - public function findVisitsByShortCodeReturnsProperData() + /** @test */ + public function findVisitsByShortCodeReturnsProperData(): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); @@ -86,10 +83,8 @@ class VisitRepositoryTest extends DatabaseTestCase $this->assertCount(2, $this->repo->findVisitsByShortCode($shortUrl->getShortCode(), null, 5, 4)); } - /** - * @test - */ - public function countVisitsByShortCodeReturnsProperData() + /** @test */ + public function countVisitsByShortCodeReturnsProperData(): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index 7914a2e9..483f8e4f 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -37,9 +37,7 @@ class PixelActionTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function imageIsReturned() { $shortCode = 'abc123'; diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index 56536a38..6bf227ac 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -6,7 +6,6 @@ namespace ShlinkioTest\Shlink\Core\Action; use finfo; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; @@ -37,9 +36,7 @@ class PreviewActionTest extends TestCase $this->action = new PreviewAction($this->previewGenerator->reveal(), $this->urlShortener->reveal()); } - /** - * @test - */ + /** @test */ public function invalidShortCodeFallsBackToNextMiddleware() { $shortCode = 'abc123'; @@ -52,9 +49,7 @@ class PreviewActionTest extends TestCase $this->action->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate->reveal()); } - /** - * @test - */ + /** @test */ public function correctShortCodeReturnsImageResponse() { $shortCode = 'abc123'; @@ -73,16 +68,13 @@ class PreviewActionTest extends TestCase $this->assertEquals((new finfo(FILEINFO_MIME))->file($path), $resp->getHeaderLine('Content-type')); } - /** - * @test - */ + /** @test */ public function invalidShortCodeExceptionFallsBackToNextMiddleware() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ $process = $delegate->handle(Argument::any())->willReturn(new Response()); $this->action->process( diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index a118451d..12e294ef 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\Core\Action; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\QrCodeResponse; @@ -35,9 +34,7 @@ class QrCodeActionTest extends TestCase $this->action = new QrCodeAction($router->reveal(), $this->urlShortener->reveal()); } - /** - * @test - */ + /** @test */ public function aNotFoundShortCodeWillDelegateIntoNextMiddleware() { $shortCode = 'abc123'; @@ -51,16 +48,13 @@ class QrCodeActionTest extends TestCase $process->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function anInvalidShortCodeWillReturnNotFoundResponse() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ $process = $delegate->handle(Argument::any())->willReturn(new Response()); $this->action->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate->reveal()); @@ -68,9 +62,7 @@ class QrCodeActionTest extends TestCase $process->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function aCorrectRequestReturnsTheQrCodeResponse() { $shortCode = 'abc123'; diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index a35550f6..9fc13906 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -42,9 +42,7 @@ class RedirectActionTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function redirectionIsPerformedToLongUrl() { $shortCode = 'abc123'; @@ -63,9 +61,7 @@ class RedirectActionTest extends TestCase $this->assertEquals($expectedUrl, $response->getHeaderLine('Location')); } - /** - * @test - */ + /** @test */ public function nextMiddlewareIsInvokedIfLongUrlIsNotFound() { $shortCode = 'abc123'; @@ -82,9 +78,7 @@ class RedirectActionTest extends TestCase $handle->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function redirectToCustomUrlIsReturnedIfConfiguredSoAndShortUrlIsNotFound() { $shortCode = 'abc123'; @@ -107,9 +101,7 @@ class RedirectActionTest extends TestCase $handle->shouldNotHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function visitIsNotTrackedIfDisableParamIsProvided() { $shortCode = 'abc123'; diff --git a/module/Core/test/ConfigProviderTest.php b/module/Core/test/ConfigProviderTest.php index d271e929..44c4e308 100644 --- a/module/Core/test/ConfigProviderTest.php +++ b/module/Core/test/ConfigProviderTest.php @@ -16,9 +16,7 @@ class ConfigProviderTest extends TestCase $this->configProvider = new ConfigProvider(); } - /** - * @test - */ + /** @test */ public function properConfigIsReturned() { $config = $this->configProvider->__invoke(); diff --git a/module/Core/test/Entity/TagTest.php b/module/Core/test/Entity/TagTest.php index f3a31d14..f9acfc29 100644 --- a/module/Core/test/Entity/TagTest.php +++ b/module/Core/test/Entity/TagTest.php @@ -8,9 +8,7 @@ use Shlinkio\Shlink\Core\Entity\Tag; class TagTest extends TestCase { - /** - * @test - */ + /** @test */ public function jsonSerializationOfTagsReturnsItsStringRepresentation() { $tag = new Tag('This is my name'); diff --git a/module/Core/test/Entity/VisitLocationTest.php b/module/Core/test/Entity/VisitLocationTest.php index 17d4045e..6e008e15 100644 --- a/module/Core/test/Entity/VisitLocationTest.php +++ b/module/Core/test/Entity/VisitLocationTest.php @@ -4,20 +4,15 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Entity; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\VisitLocation; class VisitLocationTest extends TestCase { - /** - * @test - */ - public function valuesFoundWhenExchangingArrayAreCastToString() + /** @test */ + public function valuesFoundWhenExchangingArrayAreCastToString(): void { - $payload = [ - 'latitude' => 1000.7, - 'longitude' => -2000.4, - ]; - + $payload = new Location('', '', '', '', 1000.7, -2000.4, ''); $location = new VisitLocation($payload); $this->assertSame('1000.7', $location->getLatitude()); diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index 0687131a..728bfdd2 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -15,7 +15,7 @@ class VisitTest extends TestCase * @test * @dataProvider provideDates */ - public function isProperlyJsonSerialized(?Chronos $date) + public function isProperlyJsonSerialized(?Chronos $date): void { $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', '1.2.3.4'), $date); @@ -30,11 +30,9 @@ class VisitTest extends TestCase ], $visit->jsonSerialize()); } - public function provideDates(): array + public function provideDates(): iterable { - return [ - [null], - [Chronos::now()->subDays(10)], - ]; + yield 'null date' => [null]; + yield 'not null date' => [Chronos::now()->subDays(10)]; } } diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php index f5c1ef80..afbfd6bc 100644 --- a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php +++ b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php @@ -4,61 +4,56 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Exception; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException; use function Functional\map; use function range; +use function sprintf; class DeleteShortUrlExceptionTest extends TestCase { + use StringUtilsTrait; + /** * @test - * @dataProvider provideMessages + * @dataProvider provideThresholds */ public function fromVisitsThresholdGeneratesMessageProperly( int $threshold, string $shortCode, string $expectedMessage - ) { + ): void { $e = DeleteShortUrlException::fromVisitsThreshold($threshold, $shortCode); + + $this->assertEquals($threshold, $e->getVisitsThreshold()); $this->assertEquals($expectedMessage, $e->getMessage()); $this->assertEquals(0, $e->getCode()); - } - - public function provideMessages(): array - { - return [ - [ - 50, - 'abc123', - 'Impossible to delete short URL with short code "abc123" since it has more than "50" visits.', - ], - [ - 33, - 'def456', - 'Impossible to delete short URL with short code "def456" since it has more than "33" visits.', - ], - [ - 5713, - 'foobar', - 'Impossible to delete short URL with short code "foobar" since it has more than "5713" visits.', - ], - ]; + $this->assertNull($e->getPrevious()); } /** * @test * @dataProvider provideThresholds */ - public function visitsThresholdIsProperlyReturned(int $threshold) + public function visitsThresholdIsProperlyReturned(int $threshold): void { $e = new DeleteShortUrlException($threshold); + $this->assertEquals($threshold, $e->getVisitsThreshold()); + $this->assertEquals('', $e->getMessage()); + $this->assertEquals(0, $e->getCode()); + $this->assertNull($e->getPrevious()); } public function provideThresholds(): array { return map(range(5, 50, 5), function (int $number) { - return [$number]; + $shortCode = $this->generateRandomString(6); + return [$number, $shortCode, sprintf( + 'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.', + $shortCode, + $number + )]; }); } } diff --git a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php index 65fb858e..e6ac4738 100644 --- a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php +++ b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php @@ -14,7 +14,7 @@ class InvalidShortCodeExceptionTest extends TestCase * @test * @dataProvider providePrevious */ - public function properlyCreatesExceptionFromCharset(?Throwable $prev) + public function properlyCreatesExceptionFromCharset(?Throwable $prev): void { $e = InvalidShortCodeException::fromCharset('abc123', 'def456', $prev); @@ -23,18 +23,14 @@ class InvalidShortCodeExceptionTest extends TestCase $this->assertEquals($prev, $e->getPrevious()); } - public function providePrevious(): array + public function providePrevious(): iterable { - return [ - [null], - [new Exception('Previos error', 10)], - ]; + yield 'null previous' => [null]; + yield 'instance previous' => [new Exception('Previous error', 10)]; } - /** - * @test - */ - public function properlyCreatesExceptionFromNotFoundShortCode() + /** @test */ + public function properlyCreatesExceptionFromNotFoundShortCode(): void { $e = InvalidShortCodeException::fromNotFoundShortCode('abc123'); diff --git a/module/Core/test/Exception/InvalidUrlExceptionTest.php b/module/Core/test/Exception/InvalidUrlExceptionTest.php index 67a12601..743d89c1 100644 --- a/module/Core/test/Exception/InvalidUrlExceptionTest.php +++ b/module/Core/test/Exception/InvalidUrlExceptionTest.php @@ -14,7 +14,7 @@ class InvalidUrlExceptionTest extends TestCase * @test * @dataProvider providePrevious */ - public function properlyCreatesExceptionFromUrl(?Throwable $prev) + public function properlyCreatesExceptionFromUrl(?Throwable $prev): void { $e = InvalidUrlException::fromUrl('http://the_url.com', $prev); @@ -23,11 +23,9 @@ class InvalidUrlExceptionTest extends TestCase $this->assertEquals($prev, $e->getPrevious()); } - public function providePrevious(): array + public function providePrevious(): iterable { - return [ - [null], - [new Exception('Previos error', 10)], - ]; + yield 'null previous' => [null]; + yield 'instance previous' => [new Exception('Previous error', 10)]; } } diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php index 5b7bd00c..f4d26759 100644 --- a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php +++ b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php @@ -26,9 +26,7 @@ class QrCodeCacheMiddlewareTest extends TestCase $this->middleware = new QrCodeCacheMiddleware($this->cache); } - /** - * @test - */ + /** @test */ public function noCachedPathFallsBackToNextMiddleware() { $delegate = $this->prophesize(RequestHandlerInterface::class); @@ -39,9 +37,7 @@ class QrCodeCacheMiddlewareTest extends TestCase $this->assertTrue($this->cache->contains('/foo/bar')); } - /** - * @test - */ + /** @test */ public function cachedPathReturnsCacheContent() { $isCalled = false; diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php index 26a574d3..165aa3bf 100644 --- a/module/Core/test/Model/ShortUrlMetaTest.php +++ b/module/Core/test/Model/ShortUrlMetaTest.php @@ -16,32 +16,28 @@ class ShortUrlMetaTest extends TestCase * @test * @dataProvider provideInvalidData */ - public function exceptionIsThrownIfProvidedDataIsInvalid(array $data) + public function exceptionIsThrownIfProvidedDataIsInvalid(array $data): void { $this->expectException(ValidationException::class); ShortUrlMeta::createFromRawData($data); } - public function provideInvalidData(): array + public function provideInvalidData(): iterable { - return [ - [[ - ShortUrlMetaInputFilter::VALID_SINCE => '', - ShortUrlMetaInputFilter::VALID_UNTIL => '', - ShortUrlMetaInputFilter::CUSTOM_SLUG => 'foobar', - ShortUrlMetaInputFilter::MAX_VISITS => 'invalid', - ]], - [[ - ShortUrlMetaInputFilter::VALID_SINCE => '2017', - ShortUrlMetaInputFilter::MAX_VISITS => 5, - ]], - ]; + yield [[ + ShortUrlMetaInputFilter::VALID_SINCE => '', + ShortUrlMetaInputFilter::VALID_UNTIL => '', + ShortUrlMetaInputFilter::CUSTOM_SLUG => 'foobar', + ShortUrlMetaInputFilter::MAX_VISITS => 'invalid', + ]]; + yield [[ + ShortUrlMetaInputFilter::VALID_SINCE => '2017', + ShortUrlMetaInputFilter::MAX_VISITS => 5, + ]]; } - /** - * @test - */ - public function properlyCreatedInstanceReturnsValues() + /** @test */ + public function properlyCreatedInstanceReturnsValues(): void { $meta = ShortUrlMeta::createFromParams(Chronos::parse('2015-01-01')->toAtomString(), null, 'foobar'); diff --git a/module/Core/test/Response/NotFoundHandlerTest.php b/module/Core/test/Response/NotFoundHandlerTest.php index 5c32fb0f..3ed8fec0 100644 --- a/module/Core/test/Response/NotFoundHandlerTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\Core\Response; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Zend\Diactoros\Response; @@ -26,17 +25,12 @@ class NotFoundHandlerTest extends TestCase } /** - * @param string $expectedResponse - * @param string $accept - * @param int $renderCalls - * * @test * @dataProvider provideResponses */ - public function properResponseTypeIsReturned(string $expectedResponse, string $accept, int $renderCalls) + public function properResponseTypeIsReturned(string $expectedResponse, string $accept, int $renderCalls): void { $request = (new ServerRequest())->withHeader('Accept', $accept); - /** @var MethodProphecy $render */ $render = $this->renderer->render(Argument::cetera())->willReturn(''); $resp = $this->delegate->handle($request); @@ -45,13 +39,11 @@ class NotFoundHandlerTest extends TestCase $render->shouldHaveBeenCalledTimes($renderCalls); } - public function provideResponses(): array + public function provideResponses(): iterable { - return [ - [Response\JsonResponse::class, 'application/json', 0], - [Response\JsonResponse::class, 'text/json', 0], - [Response\JsonResponse::class, 'application/x-json', 0], - [Response\HtmlResponse::class, 'text/html', 1], - ]; + yield 'application/json' => [Response\JsonResponse::class, 'application/json', 0]; + yield 'text/json' => [Response\JsonResponse::class, 'text/json', 0]; + yield 'application/x-json' => [Response\JsonResponse::class, 'application/x-json', 0]; + yield 'text/html' => [Response\HtmlResponse::class, 'text/html', 1]; } } diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index ee6178b1..3e59f535 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -39,9 +39,7 @@ class DeleteShortUrlServiceTest extends TestCase $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); } - /** - * @test - */ + /** @test */ public function deleteByShortCodeThrowsExceptionWhenThresholdIsReached() { $service = $this->createService(); @@ -54,9 +52,7 @@ class DeleteShortUrlServiceTest extends TestCase $service->deleteByShortCode('abc123'); } - /** - * @test - */ + /** @test */ public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButExplicitlyIgnored() { $service = $this->createService(); @@ -70,9 +66,7 @@ class DeleteShortUrlServiceTest extends TestCase $flush->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButCheckIsDisabled() { $service = $this->createService(false); @@ -86,9 +80,7 @@ class DeleteShortUrlServiceTest extends TestCase $flush->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function deleteByShortCodeDeletesUrlWhenThresholdIsNotReached() { $service = $this->createService(true, 100); diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 88391a83..bd186bda 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -32,9 +32,7 @@ class ShortUrlServiceTest extends TestCase $this->service = new ShortUrlService($this->em->reveal()); } - /** - * @test - */ + /** @test */ public function listedUrlsAreReturnedFromEntityManager() { $list = [ @@ -53,9 +51,7 @@ class ShortUrlServiceTest extends TestCase $this->assertEquals(4, $list->getCurrentItemCount()); } - /** - * @test - */ + /** @test */ public function exceptionIsThrownWhenSettingTagsOnInvalidShortcode() { $shortCode = 'abc123'; @@ -68,9 +64,7 @@ class ShortUrlServiceTest extends TestCase $this->service->setTagsByShortCode($shortCode); } - /** - * @test - */ + /** @test */ public function providedTagsAreGetFromRepoAndSetToTheShortUrl() { $shortUrl = $this->prophesize(ShortUrl::class); @@ -89,9 +83,7 @@ class ShortUrlServiceTest extends TestCase $this->service->setTagsByShortCode($shortCode, ['foo', 'bar']); } - /** - * @test - */ + /** @test */ public function updateMetadataByShortCodeUpdatesProvidedData() { $shortUrl = new ShortUrl(''); diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index 32f3e036..e3c907e8 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -7,7 +7,6 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; @@ -27,17 +26,13 @@ class TagServiceTest extends TestCase $this->service = new TagService($this->em->reveal()); } - /** - * @test - */ + /** @test */ public function listTagsDelegatesOnRepository() { $expected = [new Tag('foo'), new Tag('bar')]; $repo = $this->prophesize(EntityRepository::class); - /** @var MethodProphecy $find */ $find = $repo->findBy(Argument::cetera())->willReturn($expected); - /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); $result = $this->service->listTags(); @@ -47,15 +42,11 @@ class TagServiceTest extends TestCase $getRepo->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function deleteTagsDelegatesOnRepository() { $repo = $this->prophesize(TagRepository::class); - /** @var MethodProphecy $delete */ $delete = $repo->deleteByName(['foo', 'bar'])->willReturn(4); - /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); $this->service->deleteTags(['foo', 'bar']); @@ -64,19 +55,13 @@ class TagServiceTest extends TestCase $getRepo->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function createTagsPersistsEntities() { $repo = $this->prophesize(TagRepository::class); - /** @var MethodProphecy $find */ $find = $repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo')); - /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); - /** @var MethodProphecy $persist */ $persist = $this->em->persist(Argument::type(Tag::class))->willReturn(null); - /** @var MethodProphecy $flush */ $flush = $this->em->flush()->willReturn(null); $result = $this->service->createTags(['foo', 'bar']); @@ -88,15 +73,11 @@ class TagServiceTest extends TestCase $flush->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function renameInvalidTagThrowsException() { $repo = $this->prophesize(TagRepository::class); - /** @var MethodProphecy $find */ $find = $repo->findOneBy(Argument::cetera())->willReturn(null); - /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); $find->shouldBeCalled(); @@ -106,19 +87,14 @@ class TagServiceTest extends TestCase $this->service->renameTag('foo', 'bar'); } - /** - * @test - */ + /** @test */ public function renameValidTagChangesItsName() { $expected = new Tag('foo'); $repo = $this->prophesize(TagRepository::class); - /** @var MethodProphecy $find */ $find = $repo->findOneBy(Argument::cetera())->willReturn($expected); - /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); - /** @var MethodProphecy $flush */ $flush = $this->em->flush($expected)->willReturn(null); $tag = $this->service->renameTag('foo', 'bar'); diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 83c3c223..bc91dcb8 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -13,7 +13,6 @@ use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Psr7\Request; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; @@ -60,7 +59,7 @@ class UrlShortenerTest extends TestCase $this->setUrlShortener(false); } - public function setUrlShortener(bool $urlValidationEnabled): void + private function setUrlShortener(bool $urlValidationEnabled): void { $this->urlShortener = new UrlShortener( $this->httpClient->reveal(), @@ -69,9 +68,7 @@ class UrlShortenerTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function urlIsProperlyShortened(): void { // 10 -> 0Q1Y @@ -119,15 +116,12 @@ class UrlShortenerTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void { $repo = $this->prophesize(ShortUrlRepository::class); $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1); $repo->findOneBy(Argument::cetera())->willReturn(null); - /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $countBySlug->shouldBeCalledOnce(); @@ -143,7 +137,7 @@ class UrlShortenerTest extends TestCase /** * @test - * @dataProvider provideExsitingShortUrls + * @dataProvider provideExistingShortUrls */ public function existingShortUrlIsReturnedWhenRequested( string $url, @@ -162,58 +156,54 @@ class UrlShortenerTest extends TestCase $getRepo->shouldHaveBeenCalledOnce(); } - public function provideExsitingShortUrls(): array + public function provideExistingShortUrls(): iterable { $url = 'http://foo.com'; - return [ - [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), new ShortUrl($url)], - [$url, [], ShortUrlMeta::createFromRawData( - ['findIfExists' => true, 'customSlug' => 'foo'] - ), new ShortUrl($url)], - [ - $url, - ['foo', 'bar'], - ShortUrlMeta::createFromRawData(['findIfExists' => true]), - (new ShortUrl($url))->setTags(new ArrayCollection([new Tag('bar'), new Tag('foo')])), - ], - [ - $url, - [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'maxVisits' => 3]), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['maxVisits' => 3])), - ], - [ - $url, - [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01')]), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['validSince' => Chronos::parse('2017-01-01')])), - ], - [ - $url, - [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01')]), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['validUntil' => Chronos::parse('2017-01-01')])), - ], - [ - $url, - ['baz', 'foo', 'bar'], - ShortUrlMeta::createFromRawData([ - 'findIfExists' => true, - 'validUntil' => Chronos::parse('2017-01-01'), - 'maxVisits' => 4, - ]), - (new ShortUrl($url, ShortUrlMeta::createFromRawData([ - 'validUntil' => Chronos::parse('2017-01-01'), - 'maxVisits' => 4, - ])))->setTags(new ArrayCollection([new Tag('foo'), new Tag('bar'), new Tag('baz')])), - ], + yield [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), new ShortUrl($url)]; + yield [$url, [], ShortUrlMeta::createFromRawData( + ['findIfExists' => true, 'customSlug' => 'foo'] + ), new ShortUrl($url)]; + yield [ + $url, + ['foo', 'bar'], + ShortUrlMeta::createFromRawData(['findIfExists' => true]), + (new ShortUrl($url))->setTags(new ArrayCollection([new Tag('bar'), new Tag('foo')])), + ]; + yield [ + $url, + [], + ShortUrlMeta::createFromRawData(['findIfExists' => true, 'maxVisits' => 3]), + new ShortUrl($url, ShortUrlMeta::createFromRawData(['maxVisits' => 3])), + ]; + yield [ + $url, + [], + ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01')]), + new ShortUrl($url, ShortUrlMeta::createFromRawData(['validSince' => Chronos::parse('2017-01-01')])), + ]; + yield [ + $url, + [], + ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01')]), + new ShortUrl($url, ShortUrlMeta::createFromRawData(['validUntil' => Chronos::parse('2017-01-01')])), + ]; + yield [ + $url, + ['baz', 'foo', 'bar'], + ShortUrlMeta::createFromRawData([ + 'findIfExists' => true, + 'validUntil' => Chronos::parse('2017-01-01'), + 'maxVisits' => 4, + ]), + (new ShortUrl($url, ShortUrlMeta::createFromRawData([ + 'validUntil' => Chronos::parse('2017-01-01'), + 'maxVisits' => 4, + ])))->setTags(new ArrayCollection([new Tag('foo'), new Tag('bar'), new Tag('baz')])), ]; } - /** - * @test - */ + /** @test */ public function shortCodeIsProperlyParsed(): void { $shortCode = '12C1c'; diff --git a/module/Core/test/Service/VisitServiceTest.php b/module/Core/test/Service/VisitServiceTest.php index 8d8753f0..618e4781 100644 --- a/module/Core/test/Service/VisitServiceTest.php +++ b/module/Core/test/Service/VisitServiceTest.php @@ -7,6 +7,7 @@ use Doctrine\ORM\EntityManager; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; @@ -31,10 +32,8 @@ class VisitServiceTest extends TestCase $this->visitService = new VisitService($this->em->reveal()); } - /** - * @test - */ - public function locateVisitsIteratesAndLocatesUnlocatedVisits() + /** @test */ + public function locateVisitsIteratesAndLocatesUnlocatedVisits(): void { $unlocatedVisits = [ [new Visit(new ShortUrl('foo'), Visitor::emptyInstance())], @@ -52,8 +51,8 @@ class VisitServiceTest extends TestCase $clear = $this->em->clear()->will(function () { }); - $this->visitService->locateVisits(function () { - return []; + $this->visitService->locateUnlocatedVisits(function () { + return Location::emptyInstance(); }, function () { $args = func_get_args(); @@ -68,9 +67,7 @@ class VisitServiceTest extends TestCase $clear->shouldHaveBeenCalledTimes(count($unlocatedVisits)); } - /** - * @test - */ + /** @test */ public function visitsWhichCannotBeLocatedAreIgnored() { $unlocatedVisits = [ @@ -88,7 +85,7 @@ class VisitServiceTest extends TestCase $clear = $this->em->clear()->will(function () { }); - $this->visitService->locateVisits(function () { + $this->visitService->locateUnlocatedVisits(function () { throw new IpCannotBeLocatedException('Cannot be located'); }); diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index a39b4c82..c76bfc1f 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -31,9 +31,7 @@ class VisitsTrackerTest extends TestCase $this->visitsTracker = new VisitsTracker($this->em->reveal()); } - /** - * @test - */ + /** @test */ public function trackPersistsVisit() { $shortCode = '123ABC'; @@ -47,9 +45,7 @@ class VisitsTrackerTest extends TestCase $this->visitsTracker->track($shortCode, Visitor::emptyInstance()); } - /** - * @test - */ + /** @test */ public function trackedIpAddressGetsObfuscated() { $shortCode = '123ABC'; @@ -67,9 +63,7 @@ class VisitsTrackerTest extends TestCase $this->visitsTracker->track($shortCode, new Visitor('', '', '4.3.2.1')); } - /** - * @test - */ + /** @test */ public function infoReturnsVisistForCertainShortCode() { $shortCode = '123ABC'; diff --git a/module/Rest/test-api/Action/CreateShortUrlActionTest.php b/module/Rest/test-api/Action/CreateShortUrlActionTest.php index aefb8d6f..a04e31f5 100644 --- a/module/Rest/test-api/Action/CreateShortUrlActionTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlActionTest.php @@ -6,12 +6,12 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Cake\Chronos\Chronos; use GuzzleHttp\RequestOptions; use ShlinkioTest\Shlink\Common\ApiTest\ApiTestCase; +use function Functional\map; +use function range; class CreateShortUrlActionTest extends ApiTestCase { - /** - * @test - */ + /** @test */ public function createsNewShortUrlWhenOnlyLongUrlIsProvided(): void { $expectedKeys = ['shortCode', 'shortUrl', 'longUrl', 'dateCreated', 'visitsCount', 'tags']; @@ -23,9 +23,7 @@ class CreateShortUrlActionTest extends ApiTestCase } } - /** - * @test - */ + /** @test */ public function createsNewShortUrlWithCustomSlug(): void { [$statusCode, $payload] = $this->createShortUrl(['customSlug' => 'my cool slug']); @@ -34,9 +32,7 @@ class CreateShortUrlActionTest extends ApiTestCase $this->assertEquals('my-cool-slug', $payload['shortCode']); } - /** - * @test - */ + /** @test */ public function createsNewShortUrlWithTags(): void { [$statusCode, $payload] = $this->createShortUrl(['tags' => ['foo', 'bar', 'baz']]); @@ -65,16 +61,12 @@ class CreateShortUrlActionTest extends ApiTestCase public function provideMaxVisits(): array { - return [ - [1], - [5], - [3], - ]; + return map(range(1, 20), function (int $i) { + return [$i]; + }); } - /** - * @test - */ + /** @test */ public function createsShortUrlWithValidSince(): void { [$statusCode, ['shortCode' => $shortCode]] = $this->createShortUrl([ @@ -88,9 +80,7 @@ class CreateShortUrlActionTest extends ApiTestCase $this->assertEquals(self::STATUS_NOT_FOUND, $lastResp->getStatusCode()); } - /** - * @test - */ + /** @test */ public function createsShortUrlWithValidUntil(): void { [$statusCode, ['shortCode' => $shortCode]] = $this->createShortUrl([ @@ -110,7 +100,6 @@ class CreateShortUrlActionTest extends ApiTestCase */ public function returnsAnExistingShortUrlWhenRequested(array $body): void { - [$firstStatusCode, ['shortCode' => $firstShortCode]] = $this->createShortUrl($body); $body['findIfExists'] = true; @@ -121,26 +110,22 @@ class CreateShortUrlActionTest extends ApiTestCase $this->assertEquals($firstShortCode, $secondShortCode); } - public function provideMatchingBodies(): array + public function provideMatchingBodies(): iterable { $longUrl = 'https://www.alejandrocelaya.com'; - return [ - 'only long URL' => [['longUrl' => $longUrl]], - 'long URL and tags' => [['longUrl' => $longUrl, 'tags' => ['boo', 'far']]], - 'long URL custom slug' => [['longUrl' => $longUrl, 'customSlug' => 'my cool slug']], - 'several params' => [[ - 'longUrl' => $longUrl, - 'tags' => ['boo', 'far'], - 'validSince' => Chronos::now()->toAtomString(), - 'maxVisits' => 7, - ]], - ]; + yield 'only long URL' => [['longUrl' => $longUrl]]; + yield 'long URL and tags' => [['longUrl' => $longUrl, 'tags' => ['boo', 'far']]]; + yield 'long URL and custom slug' => [['longUrl' => $longUrl, 'customSlug' => 'my cool slug']]; + yield 'several params' => [[ + 'longUrl' => $longUrl, + 'tags' => ['boo', 'far'], + 'validSince' => Chronos::now()->toAtomString(), + 'maxVisits' => 7, + ]]; } - /** - * @test - */ + /** @test */ public function returnsErrorWhenRequestingReturnExistingButCustomSlugIsInUse(): void { $longUrl = 'https://www.alejandrocelaya.com'; @@ -156,9 +141,7 @@ class CreateShortUrlActionTest extends ApiTestCase $this->assertEquals(self::STATUS_BAD_REQUEST, $secondStatusCode); } - /** - * @test - */ + /** @test */ public function createsNewShortUrlIfRequestedToFindButThereIsNoMatch(): void { [$firstStatusCode, ['shortCode' => $firstShortCode]] = $this->createShortUrl([ diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 3e63da8b..5d6acc8a 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -7,9 +7,7 @@ use ShlinkioTest\Shlink\Common\ApiTest\ApiTestCase; class ListShortUrlsTest extends ApiTestCase { - /** - * @test - */ + /** @test */ public function shortUrlsAreProperlyListed() { $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls'); diff --git a/module/Rest/test-api/Middleware/AuthenticationTest.php b/module/Rest/test-api/Middleware/AuthenticationTest.php index 09ea3506..6b766614 100644 --- a/module/Rest/test-api/Middleware/AuthenticationTest.php +++ b/module/Rest/test-api/Middleware/AuthenticationTest.php @@ -12,10 +12,8 @@ use function sprintf; class AuthenticationTest extends ApiTestCase { - /** - * @test - */ - public function authorizationErrorIsReturnedIfNoApiKeyIsSent() + /** @test */ + public function authorizationErrorIsReturnedIfNoApiKeyIsSent(): void { $resp = $this->callApi(self::METHOD_GET, '/short-codes'); ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); @@ -35,7 +33,7 @@ class AuthenticationTest extends ApiTestCase * @test * @dataProvider provideInvalidApiKeys */ - public function apiKeyErrorIsReturnedWhenProvidedApiKeyIsInvalid(string $apiKey) + public function apiKeyErrorIsReturnedWhenProvidedApiKeyIsInvalid(string $apiKey): void { $resp = $this->callApi(self::METHOD_GET, '/short-codes', [ 'headers' => [ @@ -49,12 +47,10 @@ class AuthenticationTest extends ApiTestCase $this->assertEquals('Provided API key does not exist or is invalid.', $message); } - public function provideInvalidApiKeys(): array + public function provideInvalidApiKeys(): iterable { - return [ - 'key which does not exist' => ['invalid'], - 'key which is expired' => ['expired_api_key'], - 'key which is disabled' => ['disabled_api_key'], - ]; + yield 'key which does not exist' => ['invalid']; + yield 'key which is expired' => ['expired_api_key']; + yield 'key which is disabled' => ['disabled_api_key']; } } diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 5f10f4b1..7a6f53cb 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -31,18 +31,14 @@ class AuthenticateActionTest extends TestCase $this->action = new AuthenticateAction($this->apiKeyService->reveal(), $this->jwtService->reveal()); } - /** - * @test - */ + /** @test */ public function notProvidingAuthDataReturnsError() { $resp = $this->action->handle(new ServerRequest()); $this->assertEquals(400, $resp->getStatusCode()); } - /** - * @test - */ + /** @test */ public function properApiKeyReturnsTokenInResponse() { $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->setId('5')) @@ -58,9 +54,7 @@ class AuthenticateActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), '"token"') > 0); } - /** - * @test - */ + /** @test */ public function invalidApiKeyReturnsErrorResponse() { $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->disable()) diff --git a/module/Rest/test/Action/HealthActionFactoryTest.php b/module/Rest/test/Action/HealthActionFactoryTest.php index c16bb4c5..933cc43d 100644 --- a/module/Rest/test/Action/HealthActionFactoryTest.php +++ b/module/Rest/test/Action/HealthActionFactoryTest.php @@ -21,9 +21,7 @@ class HealthActionFactoryTest extends TestCase $this->factory = new Action\HealthActionFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsCreatedExtractingConnectionFromEntityManager() { $em = $this->prophesize(EntityManager::class); diff --git a/module/Rest/test/Action/HealthActionTest.php b/module/Rest/test/Action/HealthActionTest.php index 2e9a5e91..fcc069b8 100644 --- a/module/Rest/test/Action/HealthActionTest.php +++ b/module/Rest/test/Action/HealthActionTest.php @@ -25,9 +25,7 @@ class HealthActionTest extends TestCase $this->action = new HealthAction($this->conn->reveal(), new AppOptions(['version' => '1.2.3'])); } - /** - * @test - */ + /** @test */ public function passResponseIsReturnedWhenConnectionSucceeds() { $ping = $this->conn->ping()->willReturn(true); @@ -47,9 +45,7 @@ class HealthActionTest extends TestCase $ping->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function failResponseIsReturnedWhenConnectionFails() { $ping = $this->conn->ping()->willReturn(false); @@ -69,9 +65,7 @@ class HealthActionTest extends TestCase $ping->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function failResponseIsReturnedWhenConnectionThrowsException() { $ping = $this->conn->ping()->willThrow(Exception::class); diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index edc64fb0..cb4ca40e 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -34,18 +34,14 @@ class CreateShortUrlActionTest extends TestCase ]); } - /** - * @test - */ + /** @test */ public function missingLongUrlParamReturnsError() { $response = $this->action->handle(new ServerRequest()); $this->assertEquals(400, $response->getStatusCode()); } - /** - * @test - */ + /** @test */ public function properShortcodeConversionReturnsData() { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) @@ -62,9 +58,7 @@ class CreateShortUrlActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), 'http://foo.com/abc123') > 0); } - /** - * @test - */ + /** @test */ public function anInvalidUrlReturnsError() { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) @@ -79,9 +73,7 @@ class CreateShortUrlActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_URL_ERROR) > 0); } - /** - * @test - */ + /** @test */ public function nonUniqueSlugReturnsError() { $this->urlShortener->urlToShortCode( @@ -100,9 +92,7 @@ class CreateShortUrlActionTest extends TestCase $this->assertStringContainsString(RestUtils::INVALID_SLUG_ERROR, (string) $response->getBody()); } - /** - * @test - */ + /** @test */ public function aGenericExceptionWillReturnError() { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php index dc928cb8..357621e5 100644 --- a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php @@ -27,10 +27,8 @@ class DeleteShortUrlActionTest extends TestCase $this->action = new DeleteShortUrlAction($this->service->reveal()); } - /** - * @test - */ - public function emptyResponseIsReturnedIfProperlyDeleted() + /** @test */ + public function emptyResponseIsReturnedIfProperlyDeleted(): void { $deleteByShortCode = $this->service->deleteByShortCode(Argument::any())->will(function () { }); @@ -45,7 +43,7 @@ class DeleteShortUrlActionTest extends TestCase * @test * @dataProvider provideExceptions */ - public function returnsErrorResponseInCaseOfException(Throwable $e, string $error, int $statusCode) + public function returnsErrorResponseInCaseOfException(Throwable $e, string $error, int $statusCode): void { $deleteByShortCode = $this->service->deleteByShortCode(Argument::any())->willThrow($e); @@ -58,11 +56,13 @@ class DeleteShortUrlActionTest extends TestCase $deleteByShortCode->shouldHaveBeenCalledOnce(); } - public function provideExceptions(): array + public function provideExceptions(): iterable { - return [ - [new Exception\InvalidShortCodeException(), RestUtils::INVALID_SHORTCODE_ERROR, 404], - [new Exception\DeleteShortUrlException(5), RestUtils::INVALID_SHORTCODE_DELETION_ERROR, 400], + yield 'not found' => [new Exception\InvalidShortCodeException(), RestUtils::INVALID_SHORTCODE_ERROR, 404]; + yield 'bad request' => [ + new Exception\DeleteShortUrlException(5), + RestUtils::INVALID_SHORTCODE_DELETION_ERROR, + 400, ]; } } diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php index 05c94f40..171f364a 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php @@ -27,9 +27,7 @@ class EditShortUrlActionTest extends TestCase $this->action = new EditShortUrlAction($this->shortUrlService->reveal()); } - /** - * @test - */ + /** @test */ public function invalidDataReturnsError() { $request = (new ServerRequest())->withParsedBody([ @@ -45,9 +43,7 @@ class EditShortUrlActionTest extends TestCase $this->assertEquals('Provided data is invalid.', $payload['message']); } - /** - * @test - */ + /** @test */ public function incorrectShortCodeReturnsError() { $request = (new ServerRequest())->withAttribute('shortCode', 'abc123') @@ -68,9 +64,7 @@ class EditShortUrlActionTest extends TestCase $updateMeta->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function correctShortCodeReturnsSuccess() { $request = (new ServerRequest())->withAttribute('shortCode', 'abc123') diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php index bd815c76..1cfa679c 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php @@ -24,18 +24,14 @@ class EditShortUrlTagsActionTest extends TestCase $this->action = new EditShortUrlTagsAction($this->shortUrlService->reveal()); } - /** - * @test - */ + /** @test */ public function notProvidingTagsReturnsError() { $response = $this->action->handle((new ServerRequest())->withAttribute('shortCode', 'abc123')); $this->assertEquals(400, $response->getStatusCode()); } - /** - * @test - */ + /** @test */ public function anInvalidShortCodeReturnsNotFound() { $shortCode = 'abc123'; @@ -49,9 +45,7 @@ class EditShortUrlTagsActionTest extends TestCase $this->assertEquals(404, $response->getStatusCode()); } - /** - * @test - */ + /** @test */ public function tagsListIsReturnedIfCorrectShortCodeIsProvided() { $shortCode = 'abc123'; diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 953c6277..65f2a3bb 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -28,9 +28,7 @@ class ListShortUrlsActionTest extends TestCase ]); } - /** - * @test - */ + /** @test */ public function properListReturnsSuccessResponse() { $page = 3; @@ -43,9 +41,7 @@ class ListShortUrlsActionTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); } - /** - * @test - */ + /** @test */ public function anExceptionsReturnsErrorResponse() { $page = 3; diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index c5692456..1d5ab309 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -28,9 +28,7 @@ class ResolveShortUrlActionTest extends TestCase $this->action = new ResolveShortUrlAction($this->urlShortener->reveal(), []); } - /** - * @test - */ + /** @test */ public function incorrectShortCodeReturnsError() { $shortCode = 'abc123'; @@ -43,9 +41,7 @@ class ResolveShortUrlActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_ARGUMENT_ERROR) > 0); } - /** - * @test - */ + /** @test */ public function correctShortCodeReturnsSuccess() { $shortCode = 'abc123'; @@ -59,9 +55,7 @@ class ResolveShortUrlActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), 'http://domain.com/foo/bar') > 0); } - /** - * @test - */ + /** @test */ public function invalidShortCodeExceptionReturnsError() { $shortCode = 'abc123'; @@ -74,9 +68,7 @@ class ResolveShortUrlActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_SHORTCODE_ERROR) > 0); } - /** - * @test - */ + /** @test */ public function unexpectedExceptionWillReturnError() { $shortCode = 'abc123'; diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index 673c8e29..f0322543 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -40,9 +40,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase ); } - /** - * @test - */ + /** @test */ public function errorResponseIsReturnedIfInvalidApiKeyIsProvided() { $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']); @@ -58,9 +56,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase $findApiKey->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function errorResponseIsReturnedIfNoUrlIsProvided() { $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']); @@ -76,9 +72,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase $findApiKey->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function properDataIsPassedWhenGeneratingShortCode() { $request = (new ServerRequest())->withQueryParams([ diff --git a/module/Rest/test/Action/Tag/CreateTagsActionTest.php b/module/Rest/test/Action/Tag/CreateTagsActionTest.php index 78907a68..8e2dabcd 100644 --- a/module/Rest/test/Action/Tag/CreateTagsActionTest.php +++ b/module/Rest/test/Action/Tag/CreateTagsActionTest.php @@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag; use Doctrine\Common\Collections\ArrayCollection; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\CreateTagsAction; @@ -27,12 +26,10 @@ class CreateTagsActionTest extends TestCase /** * @test * @dataProvider provideTags - * @param array|null $tags */ - public function processDelegatesIntoService($tags) + public function processDelegatesIntoService(?array $tags): void { $request = (new ServerRequest())->withParsedBody(['tags' => $tags]); - /** @var MethodProphecy $deleteTags */ $deleteTags = $this->tagService->createTags($tags ?: [])->willReturn(new ArrayCollection()); $response = $this->action->handle($request); @@ -41,13 +38,11 @@ class CreateTagsActionTest extends TestCase $deleteTags->shouldHaveBeenCalled(); } - public function provideTags() + public function provideTags(): iterable { - return [ - [['foo', 'bar', 'baz']], - [['some', 'thing']], - [null], - [[]], - ]; + yield 'three tags' => [['foo', 'bar', 'baz']]; + yield 'two tags' => [['some', 'thing']]; + yield 'null tags' => [null]; + yield 'empty tags' => [[]]; } } diff --git a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php index 8eaac2ee..0fbcba80 100644 --- a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php +++ b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\Tag; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\DeleteTagsAction; @@ -26,12 +25,10 @@ class DeleteTagsActionTest extends TestCase /** * @test * @dataProvider provideTags - * @param array|null $tags */ - public function processDelegatesIntoService($tags) + public function processDelegatesIntoService(?array $tags): void { $request = (new ServerRequest())->withQueryParams(['tags' => $tags]); - /** @var MethodProphecy $deleteTags */ $deleteTags = $this->tagService->deleteTags($tags ?: []); $response = $this->action->handle($request); @@ -40,13 +37,11 @@ class DeleteTagsActionTest extends TestCase $deleteTags->shouldHaveBeenCalled(); } - public function provideTags() + public function provideTags(): iterable { - return [ - [['foo', 'bar', 'baz']], - [['some', 'thing']], - [null], - [[]], - ]; + yield 'three tags' => [['foo', 'bar', 'baz']]; + yield 'two tags' => [['some', 'thing']]; + yield 'null tags' => [null]; + yield 'empty tags' => [[]]; } } diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index 0221931a..cc8dc93c 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\Tag; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; @@ -25,12 +24,9 @@ class ListTagsActionTest extends TestCase $this->action = new ListTagsAction($this->tagService->reveal()); } - /** - * @test - */ + /** @test */ public function returnsDataFromService() { - /** @var MethodProphecy $listTags */ $listTags = $this->tagService->listTags()->willReturn([new Tag('foo'), new Tag('bar')]); $resp = $this->action->handle(new ServerRequest()); diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index 6819539c..d39b81f1 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/module/Rest/test/Action/Tag/UpdateTagActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\Tag; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; @@ -28,9 +27,8 @@ class UpdateTagActionTest extends TestCase /** * @test * @dataProvider provideParams - * @param array $bodyParams */ - public function whenInvalidParamsAreProvidedAnErrorIsReturned(array $bodyParams) + public function whenInvalidParamsAreProvidedAnErrorIsReturned(array $bodyParams): void { $request = (new ServerRequest())->withParsedBody($bodyParams); $resp = $this->action->handle($request); @@ -38,25 +36,20 @@ class UpdateTagActionTest extends TestCase $this->assertEquals(400, $resp->getStatusCode()); } - public function provideParams() + public function provideParams(): iterable { - return [ - [['oldName' => 'foo']], - [['newName' => 'foo']], - [[]], - ]; + yield 'old name only' => [['oldName' => 'foo']]; + yield 'new name only' => [['newName' => 'foo']]; + yield 'no params' => [[]]; } - /** - * @test - */ - public function requestingInvalidTagReturnsError() + /** @test */ + public function requestingInvalidTagReturnsError(): void { $request = (new ServerRequest())->withParsedBody([ 'oldName' => 'foo', 'newName' => 'bar', ]); - /** @var MethodProphecy $rename */ $rename = $this->tagService->renameTag('foo', 'bar')->willThrow(EntityDoesNotExistException::class); $resp = $this->action->handle($request); @@ -65,16 +58,13 @@ class UpdateTagActionTest extends TestCase $rename->shouldHaveBeenCalled(); } - /** - * @test - */ - public function correctInvocationRenamesTag() + /** @test */ + public function correctInvocationRenamesTag(): void { $request = (new ServerRequest())->withParsedBody([ 'oldName' => 'foo', 'newName' => 'bar', ]); - /** @var MethodProphecy $rename */ $rename = $this->tagService->renameTag('foo', 'bar')->willReturn(new Tag('bar')); $resp = $this->action->handle($request); diff --git a/module/Rest/test/Action/Visit/GetVisitsActionTest.php b/module/Rest/test/Action/Visit/GetVisitsActionTest.php index 067118de..489cdfc5 100644 --- a/module/Rest/test/Action/Visit/GetVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/GetVisitsActionTest.php @@ -29,9 +29,7 @@ class GetVisitsActionTest extends TestCase $this->action = new GetVisitsAction($this->visitsTracker->reveal()); } - /** - * @test - */ + /** @test */ public function providingCorrectShortCodeReturnsVisits() { $shortCode = 'abc123'; @@ -43,9 +41,7 @@ class GetVisitsActionTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); } - /** - * @test - */ + /** @test */ public function providingInvalidShortCodeReturnsError() { $shortCode = 'abc123'; @@ -57,9 +53,7 @@ class GetVisitsActionTest extends TestCase $this->assertEquals(404, $response->getStatusCode()); } - /** - * @test - */ + /** @test */ public function paramsAreReadFromQuery() { $shortCode = 'abc123'; diff --git a/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php b/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php index 45c07fe6..04f46211 100644 --- a/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php +++ b/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php @@ -18,9 +18,7 @@ class AuthenticationPluginManagerFactoryTest extends TestCase $this->factory = new AuthenticationPluginManagerFactory(); } - /** - * @test - */ + /** @test */ public function serviceIsProperlyCreated() { $instance = $this->factory->__invoke(new ServiceManager(['services' => [ diff --git a/module/Rest/test/Authentication/JWTServiceTest.php b/module/Rest/test/Authentication/JWTServiceTest.php index 8f21a14c..bfce6c74 100644 --- a/module/Rest/test/Authentication/JWTServiceTest.php +++ b/module/Rest/test/Authentication/JWTServiceTest.php @@ -25,9 +25,7 @@ class JWTServiceTest extends TestCase ])); } - /** - * @test - */ + /** @test */ public function tokenIsProperlyCreated() { $id = '34'; @@ -40,9 +38,7 @@ class JWTServiceTest extends TestCase $this->assertEquals('ShlinkTest:v10000.3.1', $payload['iss']); } - /** - * @test - */ + /** @test */ public function refreshIncreasesExpiration() { $originalLifetime = 10; @@ -55,25 +51,19 @@ class JWTServiceTest extends TestCase $this->assertGreaterThan($originalPayload['exp'], $newPayload['exp']); } - /** - * @test - */ + /** @test */ public function verifyReturnsTrueWhenTheTokenIsCorrect() { $this->assertTrue($this->service->verify(JWT::encode([], 'foo'))); } - /** - * @test - */ + /** @test */ public function verifyReturnsFalseWhenTheTokenIsCorrect() { $this->assertFalse($this->service->verify('invalidToken')); } - /** - * @test - */ + /** @test */ public function getPayloadWorksWithCorrectTokens() { $originalPayload = [ diff --git a/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php b/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php index df200601..aabba2e8 100644 --- a/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php +++ b/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php @@ -25,9 +25,7 @@ class ApiKeyHeaderPluginTest extends TestCase $this->plugin = new ApiKeyHeaderPlugin($this->apiKeyService->reveal()); } - /** - * @test - */ + /** @test */ public function verifyThrowsExceptionWhenApiKeyIsNotValid() { $apiKey = 'abc-ABC'; @@ -40,9 +38,7 @@ class ApiKeyHeaderPluginTest extends TestCase $this->plugin->verify($this->createRequest($apiKey)); } - /** - * @test - */ + /** @test */ public function verifyDoesNotThrowExceptionWhenApiKeyIsValid() { $apiKey = 'abc-ABC'; @@ -53,9 +49,7 @@ class ApiKeyHeaderPluginTest extends TestCase $check->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function updateReturnsResponseAsIs() { $apiKey = 'abc-ABC'; diff --git a/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php b/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php index 6f76f595..461f325a 100644 --- a/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php +++ b/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php @@ -25,9 +25,7 @@ class AuthorizationHeaderPluginTest extends TestCase $this->plugin = new AuthorizationHeaderPlugin($this->jwtService->reveal()); } - /** - * @test - */ + /** @test */ public function verifyAnAuthorizationWithoutBearerTypeThrowsException() { $authToken = 'ABC-abc'; @@ -45,9 +43,7 @@ class AuthorizationHeaderPluginTest extends TestCase $this->plugin->verify($request); } - /** - * @test - */ + /** @test */ public function verifyAnAuthorizationWithWrongTypeThrowsException() { $authToken = 'Basic ABC-abc'; @@ -64,9 +60,7 @@ class AuthorizationHeaderPluginTest extends TestCase $this->plugin->verify($request); } - /** - * @test - */ + /** @test */ public function verifyAnExpiredTokenThrowsException() { $authToken = 'Bearer ABC-abc'; @@ -88,9 +82,7 @@ class AuthorizationHeaderPluginTest extends TestCase $jwtVerify->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function verifyValidTokenDoesNotThrowException() { $authToken = 'Bearer ABC-abc'; @@ -105,9 +97,7 @@ class AuthorizationHeaderPluginTest extends TestCase $jwtVerify->shouldHaveBeenCalledOnce(); } - /** - * @test - */ + /** @test */ public function updateReturnsAnUpdatedResponseWithNewJwt() { $authToken = 'Bearer ABC-abc'; diff --git a/module/Rest/test/Authentication/RequestToAuthPluginTest.php b/module/Rest/test/Authentication/RequestToAuthPluginTest.php index 61832eb1..9c3ae9b5 100644 --- a/module/Rest/test/Authentication/RequestToAuthPluginTest.php +++ b/module/Rest/test/Authentication/RequestToAuthPluginTest.php @@ -28,10 +28,8 @@ class RequestToAuthPluginTest extends TestCase $this->requestToPlugin = new RequestToHttpAuthPlugin($this->pluginManager->reveal()); } - /** - * @test - */ - public function exceptionIsFoundWhenNoneOfTheSupportedMethodsIsFound() + /** @test */ + public function exceptionIsFoundWhenNoneOfTheSupportedMethodsIsFound(): void { $request = new ServerRequest(); @@ -48,7 +46,7 @@ class RequestToAuthPluginTest extends TestCase * @test * @dataProvider provideHeaders */ - public function properPluginIsFetchedWhenAnyAuthTypeIsFound(array $headers, string $expectedHeader) + public function properPluginIsFetchedWhenAnyAuthTypeIsFound(array $headers, string $expectedHeader): void { $request = new ServerRequest(); foreach ($headers as $header => $value) { @@ -63,19 +61,17 @@ class RequestToAuthPluginTest extends TestCase $getPlugin->shouldHaveBeenCalledOnce(); } - public function provideHeaders(): array + public function provideHeaders(): iterable { - return [ - 'API key header only' => [[ - ApiKeyHeaderPlugin::HEADER_NAME => 'foobar', - ], ApiKeyHeaderPlugin::HEADER_NAME], - 'Authorization header only' => [[ - AuthorizationHeaderPlugin::HEADER_NAME => 'foobar', - ], AuthorizationHeaderPlugin::HEADER_NAME], - 'Both headers' => [[ - AuthorizationHeaderPlugin::HEADER_NAME => 'foobar', - ApiKeyHeaderPlugin::HEADER_NAME => 'foobar', - ], ApiKeyHeaderPlugin::HEADER_NAME], - ]; + yield 'API key header only' => [[ + ApiKeyHeaderPlugin::HEADER_NAME => 'foobar', + ], ApiKeyHeaderPlugin::HEADER_NAME]; + yield 'Authorization header only' => [[ + AuthorizationHeaderPlugin::HEADER_NAME => 'foobar', + ], AuthorizationHeaderPlugin::HEADER_NAME]; + yield 'Both headers' => [[ + AuthorizationHeaderPlugin::HEADER_NAME => 'foobar', + ApiKeyHeaderPlugin::HEADER_NAME => 'foobar', + ], ApiKeyHeaderPlugin::HEADER_NAME]; } } diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 9fafc839..83bb53f3 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -16,9 +16,7 @@ class ConfigProviderTest extends TestCase $this->configProvider = new ConfigProvider(); } - /** - * @test - */ + /** @test */ public function properConfigIsReturned() { $config = $this->configProvider->__invoke(); diff --git a/module/Rest/test/Exception/AuthenticationExceptionTest.php b/module/Rest/test/Exception/AuthenticationExceptionTest.php index f953d40e..231a51cd 100644 --- a/module/Rest/test/Exception/AuthenticationExceptionTest.php +++ b/module/Rest/test/Exception/AuthenticationExceptionTest.php @@ -23,7 +23,7 @@ class AuthenticationExceptionTest extends TestCase $this->assertEquals('The token has expired.', $e->getMessage()); } - public function providePrev() + public function providePrev(): iterable { yield 'with previous exception' => [new Exception('Prev')]; yield 'without previous exception' => [null]; diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index 9e079a76..eadbd978 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -21,6 +21,7 @@ use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; use Shlinkio\Shlink\Rest\Util\RestUtils; +use Throwable; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; use Zend\Expressive\Router\Route; @@ -49,7 +50,7 @@ class AuthenticationMiddlewareTest extends TestCase * @test * @dataProvider provideWhitelistedRequests */ - public function someWhiteListedSituationsFallbackToNextMiddleware(ServerRequestInterface $request) + public function someWhiteListedSituationsFallbackToNextMiddleware(ServerRequestInterface $request): void { $handler = $this->prophesize(RequestHandlerInterface::class); $handle = $handler->handle($request)->willReturn(new Response()); @@ -63,34 +64,32 @@ class AuthenticationMiddlewareTest extends TestCase $fromRequest->shouldNotHaveBeenCalled(); } - public function provideWhitelistedRequests(): array + public function provideWhitelistedRequests(): iterable { $dummyMiddleware = $this->getDummyMiddleware(); - return [ - 'with no route result' => [new ServerRequest()], - 'with failure route result' => [(new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRouteFailure([RequestMethodInterface::METHOD_GET]) - )], - 'with whitelisted route' => [(new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRoute( - new Route('foo', $dummyMiddleware, Route::HTTP_METHOD_ANY, AuthenticateAction::class) - ) - )], - 'with OPTIONS method' => [(new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $dummyMiddleware), []) - )->withMethod(RequestMethodInterface::METHOD_OPTIONS)], - ]; + yield 'with no route result' => [new ServerRequest()]; + yield 'with failure route result' => [(new ServerRequest())->withAttribute( + RouteResult::class, + RouteResult::fromRouteFailure([RequestMethodInterface::METHOD_GET]) + )]; + yield 'with whitelisted route' => [(new ServerRequest())->withAttribute( + RouteResult::class, + RouteResult::fromRoute( + new Route('foo', $dummyMiddleware, Route::HTTP_METHOD_ANY, AuthenticateAction::class) + ) + )]; + yield 'with OPTIONS method' => [(new ServerRequest())->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('bar', $dummyMiddleware), []) + )->withMethod(RequestMethodInterface::METHOD_OPTIONS)]; } /** * @test * @dataProvider provideExceptions */ - public function errorIsReturnedWhenNoValidAuthIsProvided($e) + public function errorIsReturnedWhenNoValidAuthIsProvided(Throwable $e): void { $request = (new ServerRequest())->withAttribute( RouteResult::class, @@ -110,19 +109,15 @@ class AuthenticationMiddlewareTest extends TestCase $fromRequest->shouldHaveBeenCalledOnce(); } - public function provideExceptions(): array + public function provideExceptions(): iterable { - return [ - [new class extends Exception implements ContainerExceptionInterface { - }], - [NoAuthenticationException::fromExpectedTypes([])], - ]; + yield 'container exception' => [new class extends Exception implements ContainerExceptionInterface { + }]; + yield 'authentication exception' => [NoAuthenticationException::fromExpectedTypes([])]; } - /** - * @test - */ - public function errorIsReturnedWhenVerificationFails() + /** @test */ + public function errorIsReturnedWhenVerificationFails(): void { $request = (new ServerRequest())->withAttribute( RouteResult::class, @@ -145,10 +140,8 @@ class AuthenticationMiddlewareTest extends TestCase $fromRequest->shouldHaveBeenCalledOnce(); } - /** - * @test - */ - public function updatedResponseIsReturnedWhenVerificationPasses() + /** @test */ + public function updatedResponseIsReturnedWhenVerificationPasses(): void { $newResponse = new Response(); $request = (new ServerRequest())->withAttribute( diff --git a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php index b127fd8b..33c1aab2 100644 --- a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php +++ b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php @@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\Rest\Middleware; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Middleware\BodyParserMiddleware; @@ -36,9 +35,9 @@ class BodyParserMiddlewareTest extends TestCase public function provideIgnoredRequestMethods(): iterable { - yield 'with GET' => ['GET']; - yield 'with HEAD' => ['HEAD']; - yield 'with OPTIONS' => ['OPTIONS']; + yield 'GET' => ['GET']; + yield 'HEAD' => ['HEAD']; + yield 'OPTIONS' => ['OPTIONS']; } /** @test */ @@ -68,7 +67,6 @@ class BodyParserMiddlewareTest extends TestCase ->withBody($body) ->withHeader('content-type', 'application/json'); $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ $process = $delegate->handle(Argument::type(ServerRequestInterface::class))->will( function (array $args) use ($test) { /** @var ServerRequestInterface $req */ @@ -97,7 +95,6 @@ class BodyParserMiddlewareTest extends TestCase $request = (new ServerRequest())->withMethod('PUT') ->withBody($body); $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ $process = $delegate->handle(Argument::type(ServerRequestInterface::class))->will( function (array $args) use ($test) { /** @var ServerRequestInterface $req */ diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 23e0fc01..17539f86 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -24,9 +24,7 @@ class CrossDomainMiddlewareTest extends TestCase $this->delegate = $this->prophesize(RequestHandlerInterface::class); } - /** - * @test - */ + /** @test */ public function nonCrossDomainRequestsAreNotAffected() { $originalResponse = new Response(); @@ -40,9 +38,7 @@ class CrossDomainMiddlewareTest extends TestCase $this->assertArrayNotHasKey('Access-Control-Allow-Headers', $headers); } - /** - * @test - */ + /** @test */ public function anyRequestIncludesTheAllowAccessHeader() { $originalResponse = new Response(); @@ -59,9 +55,7 @@ class CrossDomainMiddlewareTest extends TestCase $this->assertArrayNotHasKey('Access-Control-Allow-Headers', $headers); } - /** - * @test - */ + /** @test */ public function optionsRequestIncludesMoreHeaders() { $originalResponse = new Response(); diff --git a/module/Rest/test/Middleware/PathVersionMiddlewareTest.php b/module/Rest/test/Middleware/PathVersionMiddlewareTest.php index 22d2e96d..cef65b1c 100644 --- a/module/Rest/test/Middleware/PathVersionMiddlewareTest.php +++ b/module/Rest/test/Middleware/PathVersionMiddlewareTest.php @@ -24,9 +24,7 @@ class PathVersionMiddlewareTest extends TestCase $this->middleware = new PathVersionMiddleware(); } - /** - * @test - */ + /** @test */ public function whenVersionIsProvidedRequestRemainsUnchanged() { $request = (new ServerRequest())->withUri(new Uri('/v2/foo')); @@ -39,9 +37,7 @@ class PathVersionMiddlewareTest extends TestCase $process->shouldHaveBeenCalled(); } - /** - * @test - */ + /** @test */ public function versionOneIsPrependedWhenNoVersionIsDefined() { $request = (new ServerRequest())->withUri(new Uri('/bar/baz')); diff --git a/module/Rest/test/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddlewareTest.php b/module/Rest/test/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddlewareTest.php index 1a97914d..be4e6e89 100644 --- a/module/Rest/test/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddlewareTest.php @@ -25,10 +25,8 @@ class CreateShortUrlContentNegotiationMiddlewareTest extends TestCase $this->requestHandler = $this->prophesize(RequestHandlerInterface::class); } - /** - * @test - */ - public function whenNoJsonResponseIsReturnedNoFurtherOperationsArePerformed() + /** @test */ + public function whenNoJsonResponseIsReturnedNoFurtherOperationsArePerformed(): void { $expectedResp = new Response(); $this->requestHandler->handle(Argument::type(ServerRequestInterface::class))->willReturn($expectedResp); @@ -43,7 +41,7 @@ class CreateShortUrlContentNegotiationMiddlewareTest extends TestCase * @dataProvider provideData * @param array $query */ - public function properResponseIsReturned(?string $accept, array $query, string $expectedContentType) + public function properResponseIsReturned(?string $accept, array $query, string $expectedContentType): void { $request = (new ServerRequest())->withQueryParams($query); if ($accept !== null) { @@ -60,18 +58,16 @@ class CreateShortUrlContentNegotiationMiddlewareTest extends TestCase $handle->shouldHaveBeenCalled(); } - public function provideData(): array + public function provideData(): iterable { - return [ - [null, [], 'application/json'], - [null, ['format' => 'json'], 'application/json'], - [null, ['format' => 'invalid'], 'application/json'], - [null, ['format' => 'txt'], 'text/plain'], - ['application/json', [], 'application/json'], - ['application/xml', [], 'application/json'], - ['text/plain', [], 'text/plain'], - ['application/json', ['format' => 'txt'], 'text/plain'], - ]; + yield [null, [], 'application/json']; + yield [null, ['format' => 'json'], 'application/json']; + yield [null, ['format' => 'invalid'], 'application/json']; + yield [null, ['format' => 'txt'], 'text/plain']; + yield ['application/json', [], 'application/json']; + yield ['application/xml', [], 'application/json']; + yield ['text/plain', [], 'text/plain']; + yield ['application/json', ['format' => 'txt'], 'text/plain']; } /** @@ -79,7 +75,7 @@ class CreateShortUrlContentNegotiationMiddlewareTest extends TestCase * @dataProvider provideTextBodies * @param array $json */ - public function properBodyIsReturnedInPlainTextResponses(array $json, string $expectedBody) + public function properBodyIsReturnedInPlainTextResponses(array $json, string $expectedBody): void { $request = (new ServerRequest())->withQueryParams(['format' => 'txt']); @@ -93,12 +89,10 @@ class CreateShortUrlContentNegotiationMiddlewareTest extends TestCase $handle->shouldHaveBeenCalled(); } - public function provideTextBodies(): array + public function provideTextBodies(): iterable { - return [ - [['shortUrl' => 'foobar'], 'foobar'], - [['error' => 'FOO_BAR'], 'FOO_BAR'], - [[], ''], - ]; + yield 'shortUrl key' => [['shortUrl' => 'foobar'], 'foobar']; + yield 'error key' => [['error' => 'FOO_BAR'], 'FOO_BAR']; + yield 'no shortUrl or error keys' => [[], '']; } } diff --git a/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php b/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php index cc71c8f6..50efd691 100644 --- a/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php @@ -25,9 +25,7 @@ class ShortCodePathMiddlewareTest extends TestCase $this->requestHandler->handle(Argument::type(ServerRequestInterface::class))->willReturn(new Response()); } - /** - * @test - */ + /** @test */ public function properlyReplacesTheOldPathByTheNewOne() { $uri = new Uri('/short-codes/foo'); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 61335856..b5cf734c 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -26,9 +26,7 @@ class ApiKeyServiceTest extends TestCase $this->service = new ApiKeyService($this->em->reveal()); } - /** - * @test - */ + /** @test */ public function keyIsProperlyCreated() { $this->em->flush()->shouldBeCalledOnce(); @@ -38,9 +36,7 @@ class ApiKeyServiceTest extends TestCase $this->assertNull($key->getExpirationDate()); } - /** - * @test - */ + /** @test */ public function keyIsProperlyCreatedWithExpirationDate() { $this->em->flush()->shouldBeCalledOnce(); @@ -51,9 +47,7 @@ class ApiKeyServiceTest extends TestCase $this->assertSame($date, $key->getExpirationDate()); } - /** - * @test - */ + /** @test */ public function checkReturnsFalseWhenKeyIsInvalid() { $repo = $this->prophesize(EntityRepository::class); @@ -64,9 +58,7 @@ class ApiKeyServiceTest extends TestCase $this->assertFalse($this->service->check('12345')); } - /** - * @test - */ + /** @test */ public function checkReturnsFalseWhenKeyIsDisabled() { $key = new ApiKey(); @@ -79,9 +71,7 @@ class ApiKeyServiceTest extends TestCase $this->assertFalse($this->service->check('12345')); } - /** - * @test - */ + /** @test */ public function checkReturnsFalseWhenKeyIsExpired() { $key = new ApiKey(Chronos::now()->subDay()); @@ -93,9 +83,7 @@ class ApiKeyServiceTest extends TestCase $this->assertFalse($this->service->check('12345')); } - /** - * @test - */ + /** @test */ public function checkReturnsTrueWhenConditionsAreFavorable() { $repo = $this->prophesize(EntityRepository::class); @@ -118,9 +106,7 @@ class ApiKeyServiceTest extends TestCase $this->service->disable('12345'); } - /** - * @test - */ + /** @test */ public function disableReturnsDisabledKeyWhenFOund() { $key = new ApiKey(); @@ -137,9 +123,7 @@ class ApiKeyServiceTest extends TestCase $this->assertSame($key, $returnedKey); } - /** - * @test - */ + /** @test */ public function listFindsAllApiKeys() { $repo = $this->prophesize(EntityRepository::class); @@ -150,9 +134,7 @@ class ApiKeyServiceTest extends TestCase $this->service->listKeys(); } - /** - * @test - */ + /** @test */ public function listEnabledFindsOnlyEnabledApiKeys() { $repo = $this->prophesize(EntityRepository::class); diff --git a/module/Rest/test/Util/RestUtilsTest.php b/module/Rest/test/Util/RestUtilsTest.php index 73752e81..0f3ab4ff 100644 --- a/module/Rest/test/Util/RestUtilsTest.php +++ b/module/Rest/test/Util/RestUtilsTest.php @@ -13,9 +13,7 @@ use Shlinkio\Shlink\Rest\Util\RestUtils; class RestUtilsTest extends TestCase { - /** - * @test - */ + /** @test */ public function correctCodeIsReturnedFromException() { $this->assertEquals(