From af4c66d40abb8beffcd99837b5ca2a16cdb8a619 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Jul 2018 18:42:53 +0200 Subject: [PATCH 01/13] Added version placeholder in place of hardcoded version in config --- config/autoload/app_options.global.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/autoload/app_options.global.php b/config/autoload/app_options.global.php index 2a32f31b6..12e22c0f0 100644 --- a/config/autoload/app_options.global.php +++ b/config/autoload/app_options.global.php @@ -7,7 +7,7 @@ 'app_options' => [ 'name' => 'Shlink', - 'version' => '1.7.0', + 'version' => '%SHLINK_VERSION%', 'secret_key' => env('SECRET_KEY'), ], From 82f41de87b993142e3f982478c28ed879bd89bb0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Jul 2018 18:44:04 +0200 Subject: [PATCH 02/13] Added build step which sets shlink's version --- build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build.sh b/build.sh index de9102d6b..ff26fef46 100755 --- a/build.sh +++ b/build.sh @@ -46,6 +46,11 @@ rm -rf data/{cache,log,proxies}/{*,.gitignore} rm -rf config/params/{*,.gitignore} rm -rf config/autoload/{{,*.}local.php{,.dist},.gitignore} +# Update shlink version in config +latestShlinkVersion=$(git tag -l --sort=-v:refname | head -n 1) +computedVersion=${latestShlinkVersion:1:${#latestShlinkVersion}} +sed -i "s/%SHLINK_VERSION%/${computedVersion}/g" config/autoload/app_options.global.php + # Compressing file rm -f "${projectdir}"/build/shlink_${version}_dist.zip zip -ry "${projectdir}"/build/shlink_${version}_dist.zip "../shlink_${version}_dist" From b2a63f734afa0b57b5d41616bbdad9e4957d9158 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Jul 2018 20:35:02 +0200 Subject: [PATCH 03/13] Simplified how built shlink version is found out --- build.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.sh b/build.sh index ff26fef46..65e4b2918 100755 --- a/build.sh +++ b/build.sh @@ -47,9 +47,7 @@ rm -rf config/params/{*,.gitignore} rm -rf config/autoload/{{,*.}local.php{,.dist},.gitignore} # Update shlink version in config -latestShlinkVersion=$(git tag -l --sort=-v:refname | head -n 1) -computedVersion=${latestShlinkVersion:1:${#latestShlinkVersion}} -sed -i "s/%SHLINK_VERSION%/${computedVersion}/g" config/autoload/app_options.global.php +sed -i "s/%SHLINK_VERSION%/${version}/g" config/autoload/app_options.global.php # Compressing file rm -f "${projectdir}"/build/shlink_${version}_dist.zip From d5b78f2a7e30bf636ceec672c2019ad17b287ea3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Jul 2018 18:57:24 +0200 Subject: [PATCH 04/13] Fixed date fields not properly parsed depending if originally they were datetimes or strings --- module/Core/src/Model/ShortUrlMeta.php | 27 ++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index d9acf6b92..b7e0275a5 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -78,19 +78,34 @@ private function validate(array $data): void throw ValidationException::fromInputFilter($inputFilter); } - $this->validSince = $inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE); - $this->validSince = $this->validSince !== null ? new \DateTime($this->validSince) : null; - $this->validUntil = $inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL); - $this->validUntil = $this->validUntil !== null ? new \DateTime($this->validUntil) : null; + $this->validSince = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); + $this->validUntil = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG); $this->maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS); $this->maxVisits = $this->maxVisits !== null ? (int) $this->maxVisits : null; } /** + * @param string|\DateTime|null $date * @return \DateTime|null */ - public function getValidSince() + private function parseDateField($date): ?\DateTime + { + if ($date === null || $date instanceof \DateTime) { + return $date; + } + + if (\is_string($date)) { + return new \DateTime($date); + } + + return null; + } + + /** + * @return \DateTime|null + */ + public function getValidSince(): ?\DateTime { return $this->validSince; } @@ -103,7 +118,7 @@ public function hasValidSince(): bool /** * @return \DateTime|null */ - public function getValidUntil() + public function getValidUntil(): ?\DateTime { return $this->validUntil; } From 39d79366a3c8bf581e722becd2db2a441755d35a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 30 Jul 2018 20:28:41 +0200 Subject: [PATCH 05/13] Documented date range params for visits endpoint --- .../v1_short-codes_{shortCode}_visits.json | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/swagger/paths/v1_short-codes_{shortCode}_visits.json b/docs/swagger/paths/v1_short-codes_{shortCode}_visits.json index ff390fce2..f431fc001 100644 --- a/docs/swagger/paths/v1_short-codes_{shortCode}_visits.json +++ b/docs/swagger/paths/v1_short-codes_{shortCode}_visits.json @@ -15,6 +15,24 @@ "schema": { "type": "string" } + }, + { + "name": "startDate", + "in": "query", + "description": "The date (in ISO-8601 format) from which we want to get visits.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "endDate", + "in": "query", + "description": "The date (in ISO-8601 format) until which we want to get visits.", + "required": false, + "schema": { + "type": "string" + } } ], "security": [ From 0b8e305533e88f8e107841eede02636a19ed3968 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 31 Jul 2018 19:42:33 +0200 Subject: [PATCH 06/13] Improved error management in process visits command --- .../src/Command/Visit/ProcessVisitsCommand.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index f3e174d63..81200e229 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -15,8 +15,8 @@ class ProcessVisitsCommand extends Command { - const LOCALHOST = '127.0.0.1'; - const NAME = 'visit:process'; + private const LOCALHOST = '127.0.0.1'; + public const NAME = 'visit:process'; /** * @var VisitServiceInterface @@ -57,10 +57,10 @@ public function execute(InputInterface $input, OutputInterface $output) foreach ($visits as $visit) { $ipAddr = $visit->getRemoteAddr(); - $io->write(sprintf('%s %s', $this->translator->translate('Processing IP'), $ipAddr)); + $io->write(\sprintf('%s %s', $this->translator->translate('Processing IP'), $ipAddr)); if ($ipAddr === self::LOCALHOST) { $io->writeln( - sprintf(' (%s)', $this->translator->translate('Ignored localhost address')) + \sprintf(' (%s)', $this->translator->translate('Ignored localhost address')) ); continue; } @@ -73,12 +73,17 @@ public function execute(InputInterface $input, OutputInterface $output) $visit->setVisitLocation($location); $this->visitService->saveVisit($visit); - $io->writeln(sprintf( + $io->writeln(\sprintf( ' (' . $this->translator->translate('Address located at "%s"') . ')', $location->getCityName() )); } catch (WrongIpException $e) { - continue; + $io->writeln( + \sprintf(' %s', $this->translator->translate('An error occurred while locating IP')) + ); + if ($io->isVerbose()) { + $this->getApplication()->renderException($e, $output); + } } } From 5be5e0bc60b554105e82d6e6b4f9490cec18d2bb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 31 Jul 2018 19:53:59 +0200 Subject: [PATCH 07/13] Fixed coding styles --- module/Core/src/Service/Tag/TagService.php | 12 +++++++----- .../Core/src/Service/Tag/TagServiceInterface.php | 8 ++++---- module/Rest/src/Service/ApiKeyService.php | 14 ++++++++------ module/Rest/src/Service/ApiKeyServiceInterface.php | 10 +++++----- phpstan.neon | 1 - 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Service/Tag/TagService.php index 65ae733ab..5369769d8 100644 --- a/module/Core/src/Service/Tag/TagService.php +++ b/module/Core/src/Service/Tag/TagService.php @@ -28,16 +28,18 @@ public function __construct(ORM\EntityManagerInterface $em) * @return Tag[] * @throws \UnexpectedValueException */ - public function listTags() + public function listTags(): array { - return $this->em->getRepository(Tag::class)->findBy([], ['name' => 'ASC']); + /** @var Tag[] $tags */ + $tags = $this->em->getRepository(Tag::class)->findBy([], ['name' => 'ASC']); + return $tags; } /** * @param array $tagNames * @return void */ - public function deleteTags(array $tagNames) + public function deleteTags(array $tagNames): void { /** @var TagRepository $repo */ $repo = $this->em->getRepository(Tag::class); @@ -50,7 +52,7 @@ public function deleteTags(array $tagNames) * @param string[] $tagNames * @return Collection|Tag[] */ - public function createTags(array $tagNames) + public function createTags(array $tagNames): Collection { $tags = $this->tagNamesToEntities($this->em, $tagNames); $this->em->flush(); @@ -65,7 +67,7 @@ public function createTags(array $tagNames) * @throws EntityDoesNotExistException * @throws ORM\OptimisticLockException */ - public function renameTag($oldName, $newName) + public function renameTag($oldName, $newName): Tag { $criteria = ['name' => $oldName]; /** @var Tag|null $tag */ diff --git a/module/Core/src/Service/Tag/TagServiceInterface.php b/module/Core/src/Service/Tag/TagServiceInterface.php index 7ee57bda2..a70b9e489 100644 --- a/module/Core/src/Service/Tag/TagServiceInterface.php +++ b/module/Core/src/Service/Tag/TagServiceInterface.php @@ -12,13 +12,13 @@ interface TagServiceInterface /** * @return Tag[] */ - public function listTags(); + public function listTags(): array; /** * @param string[] $tagNames * @return void */ - public function deleteTags(array $tagNames); + public function deleteTags(array $tagNames): void; /** * Provided a list of tag names, creates all that do not exist yet @@ -26,7 +26,7 @@ public function deleteTags(array $tagNames); * @param string[] $tagNames * @return Collection|Tag[] */ - public function createTags(array $tagNames); + public function createTags(array $tagNames): Collection; /** * @param string $oldName @@ -34,5 +34,5 @@ public function createTags(array $tagNames); * @return Tag * @throws EntityDoesNotExistException */ - public function renameTag($oldName, $newName); + public function renameTag($oldName, $newName): Tag; } diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index e886bcf19..815bbed35 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -25,7 +25,7 @@ public function __construct(EntityManagerInterface $em) * @param \DateTime $expirationDate * @return ApiKey */ - public function create(\DateTime $expirationDate = null) + public function create(\DateTime $expirationDate = null): ApiKey { $key = new ApiKey(); if ($expirationDate !== null) { @@ -44,7 +44,7 @@ public function create(\DateTime $expirationDate = null) * @param string $key * @return bool */ - public function check(string $key) + public function check(string $key): bool { /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); @@ -58,7 +58,7 @@ public function check(string $key) * @return ApiKey * @throws InvalidArgumentException */ - public function disable(string $key) + public function disable(string $key): ApiKey { /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); @@ -77,10 +77,12 @@ public function disable(string $key) * @param bool $enabledOnly Tells if only enabled keys should be returned * @return ApiKey[] */ - public function listKeys(bool $enabledOnly = false) + public function listKeys(bool $enabledOnly = false): array { $conditions = $enabledOnly ? ['enabled' => true] : []; - return $this->em->getRepository(ApiKey::class)->findBy($conditions); + /** @var ApiKey[] $apiKeys */ + $apiKeys = $this->em->getRepository(ApiKey::class)->findBy($conditions); + return $apiKeys; } /** @@ -89,7 +91,7 @@ public function listKeys(bool $enabledOnly = false) * @param string $key * @return ApiKey|null */ - public function getByKey(string $key) + public function getByKey(string $key): ?ApiKey { /** @var ApiKey|null $apiKey */ $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index eab38503a..3d62adbee 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -14,7 +14,7 @@ interface ApiKeyServiceInterface * @param \DateTime $expirationDate * @return ApiKey */ - public function create(\DateTime $expirationDate = null); + public function create(\DateTime $expirationDate = null): ApiKey; /** * Checks if provided key is a valid api key @@ -22,7 +22,7 @@ public function create(\DateTime $expirationDate = null); * @param string $key * @return bool */ - public function check(string $key); + public function check(string $key): bool; /** * Disables provided api key @@ -31,7 +31,7 @@ public function check(string $key); * @return ApiKey * @throws InvalidArgumentException */ - public function disable(string $key); + public function disable(string $key): ApiKey; /** * Lists all existing api keys @@ -39,7 +39,7 @@ public function disable(string $key); * @param bool $enabledOnly Tells if only enabled keys should be returned * @return ApiKey[] */ - public function listKeys(bool $enabledOnly = false); + public function listKeys(bool $enabledOnly = false): array; /** * Tries to find one API key by its key string @@ -47,5 +47,5 @@ public function listKeys(bool $enabledOnly = false); * @param string $key * @return ApiKey|null */ - public function getByKey(string $key); + public function getByKey(string $key): ?ApiKey; } diff --git a/phpstan.neon b/phpstan.neon index f1c8ab6d3..c67d210be 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,4 +4,3 @@ parameters: - module/Rest/src/Util/RestUtils.php ignoreErrors: - '#is not subtype of Throwable#' - - '#Cannot access offset#' From 863803b61403884d5382d016f2a9f2cea2476fad Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 31 Jul 2018 19:59:41 +0200 Subject: [PATCH 08/13] Fixed tests failing with new typehints --- module/CLI/test/Command/Api/GenerateKeyCommandTest.php | 9 ++++++--- module/CLI/test/Command/Tag/CreateTagCommandTest.php | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index 7c9482f70..88a44ab82 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -7,6 +7,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -37,7 +38,8 @@ public function setUp() */ public function noExpirationDateIsDefinedIfNotProvided() { - $this->apiKeyService->create(null)->shouldBeCalledTimes(1); + $this->apiKeyService->create(null)->shouldBeCalledTimes(1) + ->willReturn(new ApiKey()); $this->commandTester->execute([ 'command' => 'api-key:generate', ]); @@ -46,9 +48,10 @@ public function noExpirationDateIsDefinedIfNotProvided() /** * @test */ - public function expirationDateIsDefinedIfWhenProvided() + public function expirationDateIsDefinedIfProvided() { - $this->apiKeyService->create(Argument::type(\DateTime::class))->shouldBeCalledTimes(1); + $this->apiKeyService->create(Argument::type(\DateTime::class))->shouldBeCalledTimes(1) + ->willReturn(new ApiKey()); $this->commandTester->execute([ 'command' => 'api-key:generate', '--expirationDate' => '2016-01-01', diff --git a/module/CLI/test/Command/Tag/CreateTagCommandTest.php b/module/CLI/test/Command/Tag/CreateTagCommandTest.php index a81a23833..39b6f59fa 100644 --- a/module/CLI/test/Command/Tag/CreateTagCommandTest.php +++ b/module/CLI/test/Command/Tag/CreateTagCommandTest.php @@ -3,6 +3,7 @@ namespace ShlinkioTest\Shlink\CLI\Command\Tag; +use Doctrine\Common\Collections\ArrayCollection; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; @@ -52,7 +53,7 @@ public function serviceIsInvokedOnSuccess() { $tagNames = ['foo', 'bar']; /** @var MethodProphecy $createTags */ - $createTags = $this->tagService->createTags($tagNames)->willReturn([]); + $createTags = $this->tagService->createTags($tagNames)->willReturn(new ArrayCollection()); $this->commandTester->execute([ '--name' => $tagNames, From 899771cc2e10de36c3ffe301111a35c2fe9b0f4e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 31 Jul 2018 20:24:13 +0200 Subject: [PATCH 09/13] Fixed geolocation by switching to different API --- module/CLI/config/dependencies.config.php | 4 +- .../Visit/ProcessVisitsCommandTest.php | 4 +- module/Common/config/dependencies.config.php | 4 +- .../src/Service/IpApiLocationResolver.php | 51 +++++++++++++++++++ .../Common/src/Service/IpLocationResolver.php | 38 -------------- ...Test.php => IpApiLocationResolverTest.php} | 32 ++++++++---- 6 files changed, 78 insertions(+), 55 deletions(-) create mode 100644 module/Common/src/Service/IpApiLocationResolver.php delete mode 100644 module/Common/src/Service/IpLocationResolver.php rename module/Common/test/Service/{IpLocationResolverTest.php => IpApiLocationResolverTest.php} (50%) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 6f692af72..4dd461bec 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,7 +3,7 @@ use Shlinkio\Shlink\CLI\Command; use Shlinkio\Shlink\CLI\Factory\ApplicationFactory; -use Shlinkio\Shlink\Common\Service\IpLocationResolver; +use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -51,7 +51,7 @@ ], Command\Visit\ProcessVisitsCommand::class => [ Service\VisitService::class, - IpLocationResolver::class, + IpApiLocationResolver::class, 'translator', ], Command\Config\GenerateCharsetCommand::class => ['translator'], diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index c80f52230..bc62124ff 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -7,7 +7,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand; -use Shlinkio\Shlink\Common\Service\IpLocationResolver; +use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Service\VisitService; use Symfony\Component\Console\Application; @@ -32,7 +32,7 @@ class ProcessVisitsCommandTest extends TestCase public function setUp() { $this->visitService = $this->prophesize(VisitService::class); - $this->ipResolver = $this->prophesize(IpLocationResolver::class); + $this->ipResolver = $this->prophesize(IpApiLocationResolver::class); $command = new ProcessVisitsCommand( $this->visitService->reveal(), $this->ipResolver->reveal(), diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 19093549a..e9422d381 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -32,7 +32,7 @@ Image\ImageBuilder::class => Image\ImageBuilderFactory::class, - Service\IpLocationResolver::class => ConfigAbstractFactory::class, + Service\IpApiLocationResolver::class => ConfigAbstractFactory::class, Service\PreviewGenerator::class => ConfigAbstractFactory::class, ], 'aliases' => [ @@ -51,7 +51,7 @@ ConfigAbstractFactory::class => [ TranslatorExtension::class => ['translator'], LocaleMiddleware::class => ['translator'], - Service\IpLocationResolver::class => ['httpClient'], + Service\IpApiLocationResolver::class => ['httpClient'], Service\PreviewGenerator::class => [ ImageBuilder::class, Filesystem::class, diff --git a/module/Common/src/Service/IpApiLocationResolver.php b/module/Common/src/Service/IpApiLocationResolver.php new file mode 100644 index 000000000..9787770c8 --- /dev/null +++ b/module/Common/src/Service/IpApiLocationResolver.php @@ -0,0 +1,51 @@ +httpClient = $httpClient; + } + + /** + * @param string $ipAddress + * @return array + * @throws WrongIpException + */ + public function resolveIpLocation(string $ipAddress): array + { + try { + $response = $this->httpClient->get(\sprintf(self::SERVICE_PATTERN, $ipAddress)); + return $this->mapFields(\json_decode((string) $response->getBody(), true)); + } catch (GuzzleException $e) { + throw WrongIpException::fromIpAddress($ipAddress, $e); + } + } + + private function mapFields(array $entry): array + { + 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'] ?? '', + ]; + } +} diff --git a/module/Common/src/Service/IpLocationResolver.php b/module/Common/src/Service/IpLocationResolver.php deleted file mode 100644 index 017bf89cf..000000000 --- a/module/Common/src/Service/IpLocationResolver.php +++ /dev/null @@ -1,38 +0,0 @@ -httpClient = $httpClient; - } - - /** - * @param string $ipAddress - * @return array - * @throws WrongIpException - */ - public function resolveIpLocation(string $ipAddress): array - { - try { - $response = $this->httpClient->get(sprintf(self::SERVICE_PATTERN, $ipAddress)); - return json_decode((string) $response->getBody(), true); - } catch (GuzzleException $e) { - throw WrongIpException::fromIpAddress($ipAddress, $e); - } - } -} diff --git a/module/Common/test/Service/IpLocationResolverTest.php b/module/Common/test/Service/IpApiLocationResolverTest.php similarity index 50% rename from module/Common/test/Service/IpLocationResolverTest.php rename to module/Common/test/Service/IpApiLocationResolverTest.php index f5fb162c2..1d36ce9ac 100644 --- a/module/Common/test/Service/IpLocationResolverTest.php +++ b/module/Common/test/Service/IpApiLocationResolverTest.php @@ -8,12 +8,12 @@ use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Service\IpLocationResolver; +use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; -class IpLocationResolverTest extends TestCase +class IpApiLocationResolverTest extends TestCase { /** - * @var IpLocationResolver + * @var IpApiLocationResolver */ protected $ipResolver; /** @@ -24,7 +24,7 @@ class IpLocationResolverTest extends TestCase public function setUp() { $this->client = $this->prophesize(Client::class); - $this->ipResolver = new IpLocationResolver($this->client->reveal()); + $this->ipResolver = new IpApiLocationResolver($this->client->reveal()); } /** @@ -32,16 +32,26 @@ public function setUp() */ public function correctIpReturnsDecodedInfo() { + $actual = [ + 'countryCode' => 'bar', + 'lat' => 5, + 'lon' => 10, + ]; $expected = [ - 'foo' => 'bar', - 'baz' => 'foo', + 'country_code' => 'bar', + 'country_name' => '', + 'region_name' => '', + 'city' => '', + 'latitude' => 5, + 'longitude' => 10, + 'time_zone' => '', ]; $response = new Response(); - $response->getBody()->write(json_encode($expected)); + $response->getBody()->write(\json_encode($actual)); $response->getBody()->rewind(); - $this->client->get('http://freegeoip.net/json/1.2.3.4')->willReturn($response) - ->shouldBeCalledTimes(1); + $this->client->get('http://ip-api.com/json/1.2.3.4')->willReturn($response) + ->shouldBeCalledTimes(1); $this->assertEquals($expected, $this->ipResolver->resolveIpLocation('1.2.3.4')); } @@ -51,8 +61,8 @@ public function correctIpReturnsDecodedInfo() */ public function guzzleExceptionThrowsShlinkException() { - $this->client->get('http://freegeoip.net/json/1.2.3.4')->willThrow(new TransferException()) - ->shouldBeCalledTimes(1); + $this->client->get('http://ip-api.com/json/1.2.3.4')->willThrow(new TransferException()) + ->shouldBeCalledTimes(1); $this->ipResolver->resolveIpLocation('1.2.3.4'); } } From f4b569c245c78be83540554a4ccb431faf54d100 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 1 Aug 2018 20:28:05 +0200 Subject: [PATCH 10/13] Improved code --- .../Common/src/Factory/EntityManagerFactory.php | 15 +++++++-------- module/Core/src/Repository/ShortUrlRepository.php | 5 +++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/module/Common/src/Factory/EntityManagerFactory.php b/module/Common/src/Factory/EntityManagerFactory.php index d86221212..a219f2e67 100644 --- a/module/Common/src/Factory/EntityManagerFactory.php +++ b/module/Common/src/Factory/EntityManagerFactory.php @@ -23,8 +23,7 @@ class EntityManagerFactory implements FactoryInterface * @param null|array $options * @return object * @throws ServiceNotFoundException if unable to resolve the service. - * @throws ServiceNotCreatedException if an exception is raised when - * creating a service. + * @throws ServiceNotCreatedException if an exception is raised when creating a service. * @throws ContainerException if any other error occurs */ public function __invoke(ContainerInterface $container, $requestedName, array $options = null) @@ -32,14 +31,14 @@ public function __invoke(ContainerInterface $container, $requestedName, array $o $globalConfig = $container->get('config'); $isDevMode = isset($globalConfig['debug']) ? ((bool) $globalConfig['debug']) : false; $cache = $container->has(Cache::class) ? $container->get(Cache::class) : new ArrayCache(); - $emConfig = isset($globalConfig['entity_manager']) ? $globalConfig['entity_manager'] : []; - $connecitonConfig = isset($emConfig['connection']) ? $emConfig['connection'] : []; - $ormConfig = isset($emConfig['orm']) ? $emConfig['orm'] : []; + $emConfig = $globalConfig['entity_manager'] ?? []; + $connectionConfig = $emConfig['connection'] ?? []; + $ormConfig = $emConfig['orm'] ?? []; - return EntityManager::create($connecitonConfig, Setup::createAnnotationMetadataConfiguration( - isset($ormConfig['entities_paths']) ? $ormConfig['entities_paths'] : [], + return EntityManager::create($connectionConfig, Setup::createAnnotationMetadataConfiguration( + $ormConfig['entities_paths'] ?? [], $isDevMode, - isset($ormConfig['proxies_dir']) ? $ormConfig['proxies_dir'] : null, + $ormConfig['proxies_dir'] ?? null, $cache, false )); diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index a959f818b..3919402f8 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -57,10 +57,11 @@ protected function processOrderByForList(QueryBuilder $qb, $orderBy) ->orderBy('totalVisits', $order); return \array_column($qb->getQuery()->getResult(), 0); - } elseif (\in_array($fieldName, ['originalUrl', 'shortCode', 'dateCreated'], true)) { - $qb->orderBy('s.' . $fieldName, $order); } + if (\in_array($fieldName, ['originalUrl', 'shortCode', 'dateCreated'], true)) { + $qb->orderBy('s.' . $fieldName, $order); + } return $qb->getQuery()->getResult(); } From a79c1f580e3058546cc5d13b87d1f5e98ff7a542 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 1 Aug 2018 20:31:54 +0200 Subject: [PATCH 11/13] Fixed visits count multiplied by the number of tags when ordering and filtering by text --- module/Core/src/Repository/ShortUrlRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 3919402f8..04a5b6e67 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -51,7 +51,7 @@ protected function processOrderByForList(QueryBuilder $qb, $orderBy) $order = \is_array($orderBy) ? $orderBy[$fieldName] : 'ASC'; if (\in_array($fieldName, ['visits', 'visitsCount', 'visitCount'], true)) { - $qb->addSelect('COUNT(v) AS totalVisits') + $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') ->leftJoin('s.visits', 'v') ->groupBy('s') ->orderBy('totalVisits', $order); From 3883ed15c45c07fd7cad939bcaa3e53515302012 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 1 Aug 2018 20:40:24 +0200 Subject: [PATCH 12/13] Fixed short codes DB length too short --- data/migrations/Version20180801183328.php | 44 +++++++++++++++++++ module/Core/src/Entity/ShortUrl.php | 2 +- .../Repository/ShortUrlRepositoryTest.php | 2 +- 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 data/migrations/Version20180801183328.php diff --git a/data/migrations/Version20180801183328.php b/data/migrations/Version20180801183328.php new file mode 100644 index 000000000..de967de7e --- /dev/null +++ b/data/migrations/Version20180801183328.php @@ -0,0 +1,44 @@ +setSize($schema, self::NEW_SIZE); + } + + /** + * @param Schema $schema + * @throws SchemaException + */ + public function down(Schema $schema) : void + { + $this->setSize($schema, self::OLD_SIZE); + } + + /** + * @param Schema $schema + * @param int $size + * @throws SchemaException + */ + private function setSize(Schema $schema, int $size): void + { + $schema->getTable('short_urls')->getColumn('short_code')->setLength($size); + } +} diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index b1ffe60f3..ff21c47bb 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -29,7 +29,7 @@ class ShortUrl extends AbstractEntity implements \JsonSerializable * name="short_code", * type="string", * nullable=false, - * length=10, + * length=255, * unique=true * ) */ diff --git a/module/Core/test-func/Repository/ShortUrlRepositoryTest.php b/module/Core/test-func/Repository/ShortUrlRepositoryTest.php index 4616f6c8c..184ca054f 100644 --- a/module/Core/test-func/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-func/Repository/ShortUrlRepositoryTest.php @@ -38,7 +38,7 @@ public function findOneByShortCodeReturnsProperData() $bar = new ShortUrl(); $bar->setOriginalUrl('bar') - ->setShortCode('bar') + ->setShortCode('bar_very_long_text') ->setValidSince((new \DateTime())->add(new \DateInterval('P1M'))); $this->getEntityManager()->persist($bar); From 8063e643a37be4d14104003cb553d600b597b010 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 1 Aug 2018 20:58:48 +0200 Subject: [PATCH 13/13] Updated changelog with version 1.10.1 --- CHANGELOG.md | 30 +++++++++++++++++++++++ data/migrations/Version20180801183328.php | 7 +++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6e866bb..766f87bc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,35 @@ # CHANGELOG +## 1.10.1 - 2018-08-02 + +#### Added + +* *Nothing* + +#### Changed + +* [#167](https://github.com/shlinkio/shlink/issues/167) Shlink version is now set at build time to avoid older version numbers to be kept in newer builds. + +#### Deprecated + +* *Nothing* + +#### Removed + +* *Nothing* + +#### Fixed + +* [#165](https://github.com/shlinkio/shlink/issues/165) Fixed custom slugs failing when they are longer than 10 characters. +* [#166](https://github.com/shlinkio/shlink/issues/166) Fixed unusual edge case in which visits were not properly counted when ordering by visit and filtering by search term in `[GET] /short-codes` API endpoint. +* [#174](https://github.com/shlinkio/shlink/issues/174) Fixed geolocation not working due to a deprecation on used service. +* [#172](https://github.com/shlinkio/shlink/issues/172) Documented missing filtering params for `[GET] /short-codes/{shortCode}/visits` API endpoint, which allow the list to be filtered by date range. + + For example: `https://doma.in/rest/v1/short-urls/abc123/visits?startDate=2017-05-23&endDate=2017-10-05` + +* [#169](https://github.com/shlinkio/shlink/issues/169) Fixed unhandled error when parsing `ShortUrlMeta` and date fields are already `DateTime` instances. + + ## 1.10.0 - 2018-07-09 #### Added diff --git a/data/migrations/Version20180801183328.php b/data/migrations/Version20180801183328.php index de967de7e..b87415f63 100644 --- a/data/migrations/Version20180801183328.php +++ b/data/migrations/Version20180801183328.php @@ -1,4 +1,5 @@ -setSize($schema, self::NEW_SIZE); } @@ -27,7 +28,7 @@ public function up(Schema $schema) : void * @param Schema $schema * @throws SchemaException */ - public function down(Schema $schema) : void + public function down(Schema $schema): void { $this->setSize($schema, self::OLD_SIZE); }