diff --git a/CHANGELOG.md b/CHANGELOG.md index 766f87bc..3bff8681 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,33 @@ # CHANGELOG +## 1.10.2 - 2018-08-04 + +#### Added + +* *Nothing* + +#### Changed + +* *Nothing* + +#### Deprecated + +* *Nothing* + +#### Removed + +* *Nothing* + +#### Fixed + +* [#177](https://github.com/shlinkio/shlink/issues/177) Fixed `[GET] /short-codes` endpoint returning a 500 status code when trying to filter by `tags` and `searchTerm` at the same time. +* [#175](https://github.com/shlinkio/shlink/issues/175) Fixed error introduced in previous version, where you could end up banned from the service used to resolve IP address locations. + + In order to fix that, just fill [this form](http://ip-api.com/docs/unban) including your server's IP address and your server should be unbanned. + + In order to prevent this, after resolving 150 IP addresses, shlink now waits 1 minute before trying to resolve any more addresses. + + ## 1.10.1 - 2018-08-02 #### Added diff --git a/docs/swagger/paths/v1_short-codes.json b/docs/swagger/paths/v1_short-codes.json index 9d52f798..f377e67c 100644 --- a/docs/swagger/paths/v1_short-codes.json +++ b/docs/swagger/paths/v1_short-codes.json @@ -25,7 +25,7 @@ } }, { - "name": "tags", + "name": "tags[]", "in": "query", "description": "A list of tags used to filter the resultset. Only short URLs tagged with at least one of the provided tags will be returned. (Since v1.3.0)", "required": false, diff --git a/module/CLI/lang/es.mo b/module/CLI/lang/es.mo index 2b8b5135..fb888fbf 100644 Binary files a/module/CLI/lang/es.mo and b/module/CLI/lang/es.mo differ diff --git a/module/CLI/lang/es.po b/module/CLI/lang/es.po index fe82aebe..1555bf1e 100644 --- a/module/CLI/lang/es.po +++ b/module/CLI/lang/es.po @@ -1,15 +1,15 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2018-01-21 09:36+0100\n" -"PO-Revision-Date: 2018-01-21 09:39+0100\n" +"POT-Creation-Date: 2018-08-04 16:35+0200\n" +"PO-Revision-Date: 2018-08-04 16:37+0200\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"X-Generator: Poedit 2.0.4\n" +"X-Generator: Poedit 2.0.6\n" "X-Poedit-Basepath: ..\n" "Plural-Forms: nplurals=2; plural=(n != 1);\n" "X-Poedit-SourceCharset: UTF-8\n" @@ -317,6 +317,13 @@ msgstr "Ignorada IP de localhost" msgid "Address located at \"%s\"" msgstr "Dirección localizada en \"%s\"" +msgid "An error occurred while locating IP" +msgstr "Se produjo un error al localizar la IP" + +#, php-format +msgid "IP location resolver limit reached. Waiting %s seconds..." +msgstr "Limite del localizador de IPs alcanzado. Esperando %s segundos..." + msgid "Finished processing all IPs" msgstr "Finalizado el procesado de todas las IPs" diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 81200e22..b858546d 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -55,6 +55,7 @@ class ProcessVisitsCommand extends Command $io = new SymfonyStyle($input, $output); $visits = $this->visitService->getUnlocatedVisits(); + $count = 0; foreach ($visits as $visit) { $ipAddr = $visit->getRemoteAddr(); $io->write(\sprintf('%s %s', $this->translator->translate('Processing IP'), $ipAddr)); @@ -65,6 +66,7 @@ class ProcessVisitsCommand extends Command continue; } + $count++; try { $result = $this->ipLocationResolver->resolveIpLocation($ipAddr); @@ -85,6 +87,16 @@ class ProcessVisitsCommand extends Command $this->getApplication()->renderException($e, $output); } } + + if ($count === $this->ipLocationResolver->getApiLimit()) { + $count = 0; + $seconds = $this->ipLocationResolver->getApiInterval(); + $io->note(\sprintf( + $this->translator->translate('IP location resolver limit reached. Waiting %s seconds...'), + $seconds + )); + \sleep($seconds); + } } $io->success($this->translator->translate('Finished processing all IPs')); diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index bc62124f..5f843fa4 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -33,6 +33,8 @@ class ProcessVisitsCommandTest extends TestCase { $this->visitService = $this->prophesize(VisitService::class); $this->ipResolver = $this->prophesize(IpApiLocationResolver::class); + $this->ipResolver->getApiLimit()->willReturn(10000000000); + $command = new ProcessVisitsCommand( $this->visitService->reveal(), $this->ipResolver->reveal(), @@ -95,4 +97,41 @@ class ProcessVisitsCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertTrue(strpos($output, 'Ignored localhost address') > 0); } + + /** + * @test + */ + public function sleepsEveryTimeTheApiLimitIsReached() + { + $visits = [ + (new Visit())->setRemoteAddr('1.2.3.4'), + (new Visit())->setRemoteAddr('4.3.2.1'), + (new Visit())->setRemoteAddr('12.34.56.78'), + (new Visit())->setRemoteAddr('1.2.3.4'), + (new Visit())->setRemoteAddr('4.3.2.1'), + (new Visit())->setRemoteAddr('12.34.56.78'), + (new Visit())->setRemoteAddr('1.2.3.4'), + (new Visit())->setRemoteAddr('4.3.2.1'), + (new Visit())->setRemoteAddr('12.34.56.78'), + (new Visit())->setRemoteAddr('4.3.2.1'), + ]; + $apiLimit = 3; + + $this->visitService->getUnlocatedVisits()->willReturn($visits); + $this->visitService->saveVisit(Argument::any())->will(function () { + }); + + $getApiLimit = $this->ipResolver->getApiLimit()->willReturn($apiLimit); + $getApiInterval = $this->ipResolver->getApiInterval()->willReturn(0); + $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]) + ->shouldBeCalledTimes(count($visits)); + + $this->commandTester->execute([ + 'command' => 'visit:process', + ]); + + $getApiLimit->shouldHaveBeenCalledTimes(\count($visits)); + $getApiInterval->shouldHaveBeenCalledTimes(\round(\count($visits) / $apiLimit)); + $resolveIpLocation->shouldHaveBeenCalledTimes(\count($visits)); + } } diff --git a/module/Common/src/Service/IpApiLocationResolver.php b/module/Common/src/Service/IpApiLocationResolver.php index 9787770c..d801a2e8 100644 --- a/module/Common/src/Service/IpApiLocationResolver.php +++ b/module/Common/src/Service/IpApiLocationResolver.php @@ -48,4 +48,24 @@ class IpApiLocationResolver implements IpLocationResolverInterface 'time_zone' => $entry['timezone'] ?? '', ]; } + + /** + * Returns the interval in seconds that needs to be waited when the API limit is reached + * + * @return int + */ + public function getApiInterval(): int + { + return 65; // ip-api interval is 1 minute. Return 5 extra seconds just in case + } + + /** + * Returns the limit of requests that can be performed to the API in a specific interval, or null if no limit exists + * + * @return int|null + */ + public function getApiLimit(): ?int + { + return 145; // ip-api limit is 150 requests per minute. Leave 5 less requests just in case + } } diff --git a/module/Common/src/Service/IpLocationResolverInterface.php b/module/Common/src/Service/IpLocationResolverInterface.php index fc1f07a3..98099946 100644 --- a/module/Common/src/Service/IpLocationResolverInterface.php +++ b/module/Common/src/Service/IpLocationResolverInterface.php @@ -13,4 +13,18 @@ interface IpLocationResolverInterface * @throws WrongIpException */ public function resolveIpLocation(string $ipAddress): array; + + /** + * Returns the interval in seconds that needs to be waited when the API limit is reached + * + * @return int + */ + public function getApiInterval(): int; + + /** + * Returns the limit of requests that can be performed to the API in a specific interval, or null if no limit exists + * + * @return int|null + */ + public function getApiLimit(): ?int; } diff --git a/module/Common/test/Service/IpApiLocationResolverTest.php b/module/Common/test/Service/IpApiLocationResolverTest.php index 1d36ce9a..84012e73 100644 --- a/module/Common/test/Service/IpApiLocationResolverTest.php +++ b/module/Common/test/Service/IpApiLocationResolverTest.php @@ -65,4 +65,20 @@ class IpApiLocationResolverTest extends TestCase ->shouldBeCalledTimes(1); $this->ipResolver->resolveIpLocation('1.2.3.4'); } + + /** + * @test + */ + public function getApiIntervalReturnsExpectedValue() + { + $this->assertEquals(65, $this->ipResolver->getApiInterval()); + } + + /** + * @test + */ + public function getApiLimitReturnsExpectedValue() + { + $this->assertEquals(145, $this->ipResolver->getApiLimit()); + } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 04a5b6e6..42b2c73e 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -93,7 +93,10 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI // Apply search term to every searchable field if not empty if (! empty($searchTerm)) { - $qb->leftJoin('s.tags', 't'); + // Left join with tags only if no tags were provided. In case of tags, an inner join will be done later + if (empty($tags)) { + $qb->leftJoin('s.tags', 't'); + } $conditions = [ $qb->expr()->like('s.originalUrl', ':searchPattern'), diff --git a/module/Core/test-func/Repository/ShortUrlRepositoryTest.php b/module/Core/test-func/Repository/ShortUrlRepositoryTest.php index 184ca054..1a0cf1e6 100644 --- a/module/Core/test-func/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-func/Repository/ShortUrlRepositoryTest.php @@ -5,15 +5,17 @@ namespace ShlinkioTest\Shlink\Core\Repository; use Doctrine\Common\Collections\ArrayCollection; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use ShlinkioTest\Shlink\Common\DbUnit\DatabaseTestCase; class ShortUrlRepositoryTest extends DatabaseTestCase { - const ENTITIES_TO_EMPTY = [ + protected const ENTITIES_TO_EMPTY = [ ShortUrl::class, Visit::class, + Tag::class, ]; /** @@ -79,4 +81,35 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertEquals($count, $this->repo->countList()); } + + /** + * @test + */ + public function findListProperlyFiltersByTagAndSearchTerm() + { + $tag = new Tag('bar'); + $this->getEntityManager()->persist($tag); + + $foo = new ShortUrl(); + $foo->setOriginalUrl('foo') + ->setShortCode('foo') + ->setTags(new ArrayCollection([$tag])); + $this->getEntityManager()->persist($foo); + + $bar = new ShortUrl(); + $bar->setOriginalUrl('bar') + ->setShortCode('bar_very_long_text'); + $this->getEntityManager()->persist($bar); + + $foo2 = new ShortUrl(); + $foo2->setOriginalUrl('foo_2') + ->setShortCode('foo_2'); + $this->getEntityManager()->persist($foo2); + + $this->getEntityManager()->flush(); + + $result = $this->repo->findList(null, null, 'foo', ['bar']); + $this->assertCount(1, $result); + $this->assertSame($foo, $result[0]); + } } diff --git a/module/Core/test-func/Repository/TagRepositoryTest.php b/module/Core/test-func/Repository/TagRepositoryTest.php index b6a96d85..1fce379f 100644 --- a/module/Core/test-func/Repository/TagRepositoryTest.php +++ b/module/Core/test-func/Repository/TagRepositoryTest.php @@ -9,7 +9,7 @@ use ShlinkioTest\Shlink\Common\DbUnit\DatabaseTestCase; class TagRepositoryTest extends DatabaseTestCase { - const ENTITIES_TO_EMPTY = [ + protected const ENTITIES_TO_EMPTY = [ Tag::class, ]; diff --git a/module/Core/test-func/Repository/VisitRepositoryTest.php b/module/Core/test-func/Repository/VisitRepositoryTest.php index b13e4053..00c2eaf3 100644 --- a/module/Core/test-func/Repository/VisitRepositoryTest.php +++ b/module/Core/test-func/Repository/VisitRepositoryTest.php @@ -12,7 +12,7 @@ use ShlinkioTest\Shlink\Common\DbUnit\DatabaseTestCase; class VisitRepositoryTest extends DatabaseTestCase { - const ENTITIES_TO_EMPTY = [ + protected const ENTITIES_TO_EMPTY = [ VisitLocation::class, Visit::class, ShortUrl::class,