From ed859767a816726a68e1b3914c61e7b49cfacb7e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 2 Aug 2018 23:02:48 +0200 Subject: [PATCH 1/6] Updated IpLocation resolver to be able to provide limits in order to apply sleeps --- .../Command/Visit/ProcessVisitsCommand.php | 12 +++++++++++ .../Visit/ProcessVisitsCommandTest.php | 2 ++ .../src/Service/IpApiLocationResolver.php | 20 +++++++++++++++++++ .../Service/IpLocationResolverInterface.php | 14 +++++++++++++ 4 files changed, 48 insertions(+) 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..ae36de22 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(), 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; } From 110e8cb78d067c50a9ce547eea8ab14fe287773b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Aug 2018 15:50:02 +0200 Subject: [PATCH 2/6] Added test to cover new IP resolution API limits --- .../Visit/ProcessVisitsCommandTest.php | 37 +++++++++++++++++++ .../Service/IpApiLocationResolverTest.php | 16 ++++++++ 2 files changed, 53 insertions(+) diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index ae36de22..5f843fa4 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -97,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/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()); + } } From c7239aaca2516e42c87608d151b8fc88e4df1b2d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Aug 2018 16:15:09 +0200 Subject: [PATCH 3/6] Fixed duplicated join with same table performed while filtering short codes by search term and tags --- docs/swagger/paths/v1_short-codes.json | 2 +- module/Core/src/Repository/ShortUrlRepository.php | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) 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/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'), From 080965e166fbbd62af9dccc6fba170eab5e7485d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Aug 2018 16:21:01 +0200 Subject: [PATCH 4/6] Improved ShortUrlRepositoryTest covering listing case with filter by tag and search term at the same time --- .../Repository/ShortUrlRepositoryTest.php | 35 ++++++++++++++++++- .../Repository/TagRepositoryTest.php | 2 +- .../Repository/VisitRepositoryTest.php | 2 +- 3 files changed, 36 insertions(+), 3 deletions(-) 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, From 6b968a6843066c36f5311837962e3532dd28dd0b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Aug 2018 16:28:12 +0200 Subject: [PATCH 5/6] Updated changelog including v1.10.2 --- CHANGELOG.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) 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 From 416c56dee2c19f22f86c40c2aff5a5a052ce6c4d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Aug 2018 16:37:54 +0200 Subject: [PATCH 6/6] Added new spanish translations --- module/CLI/lang/es.mo | Bin 8799 -> 9046 bytes module/CLI/lang/es.po | 13 ++++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/module/CLI/lang/es.mo b/module/CLI/lang/es.mo index 2b8b5135bfcaddef812ad4f6718194f4c0444224..fb888fbf936bd243d618df6a9be5ce8132f5bd52 100644 GIT binary patch delta 2068 zcmY+^TWl0n9LMqh7HFxJ7P=LzmombFRb*+G+6$uHrI*-RmjYf<6UW_2yUn^wce|31 zG^-(kF@{7DV7a75|M|>iXU_ke z^PidT{jR@!p?>KxqwOJXA!aJf#&KvF8`{b$v(>l?*I+ZYU>qO8aqPo$*n_pp&DP;= z)bmQ%gs-9QKaB~TL;hd3nrgH4oY;wBEMh$##zuS#Z^Ri~ieKRM_;ql79ve8mjLNz8 zIs6;Thy-ud?wUgdH5u;9a4oX4B+Pt<*3POA7?k=(2Ub$&2#G`M~!xc&-iNlum7(TL8W zmf{ksm}=?m7_LJlFpCf2JZeO3^q#;M@B#c7xz*};2(xd^sD!#u)!&Vp;U`feKa85; z@;kvF&LFd7U!tb`H&pRm!Yo$t6iwj-Qh#<9b$t%wcp-2t!&cEfjgR9LR^k<8*)2o? zI2guVdjH$lY30Nr)W~O19ej=&`A?{|50SQBrwD2UQ8qfC#7#JkJMaW@i~WiR@o#(# z4?2t*&*OubreNCj{-0roWwhVX?}*I?jw8sRY!KUV1iSDgY9v3P*7^@rqJNl^nVI%fl@Dv!nt1 zzwx#8n?68l`f#ak%G8bDl(`CI_u`>nlp2-VE~3-lTl~;agZ|7g;D1{mdlkXa7QBbh zY;7m_T-betn%*j2F8{6LO)L9fG+j{JLBt4ExQa#vrgkqu9WGWjYWi7Le;u`S_Y*4G zIH6ilWwQ#`G!=IbF-$}W)!LxH@a(h^y@c9;Hq$>=_E)S=cab+V#!I5?HhIR&qn&pziBoNVUV%mjySYSc?dok!h_f6IoV zQ}j~#T)G&IMyJPWdaG7+Zf3^jpd!v z)l}T9-&Zzsz@-mYDb%g`pdvA(?mQ>${dYZ0=4p?A6dgC4a&rgVbUy0z758}sH7+&!Eu&mCY;2}B<0AiZh7DUbZ7svK zKXYT|%#Ca^%ncic7z;Cu85084 zqqI}AspmXqy;w4l1LaM;*>rr1nfMEXn3`a=9viR;Ph%sFV;<)DV$W;BIh=1n{r(U( z;2Grph`pqdK}SxaSvr>EOl-y(xCtlVUYvvn-RlE5mGkqcxn9R9IEu6IA! z#qQ6=NX|&KWj+$9vGK1p|)I_c#m+U@zaSS!#mzay+P!maGc8p~OsPiCl z$(oQqYv-Wfb>n8-g{6#dPiUl|mxJ>ltmM1^ORyUk;sCbc2ohyWnrz0OrE}0kbFm(4 zaRKhfR6K`Tz%^84MsXUx!iYlq-R=14nnE0v!+b2mX4D?oi+wnVija?|>wFEa!FyPQ znQ5^B!l>MLy7r+`aSCnqfK{fMD2c)C7vK5L@v>9D|`&+D(`g`aP&k zcNRC|IIhNOmZu3G#WH+=IjB_xz>l-BZ94JS%zNnQz#FImQkkYUUlC?uBkBj;sLgf& znTs8E`-fel?)69R^)XZ`KBE?r!9$gTI@Dh1iqIIQ(TfG>W%PAegjz`iSK)o+7MsCr zl+zGuB4N}nUx5ngR@6!dQ4zi3ULQe%V9!wz{)z<4B7bP~&`4!k3Ryq0zw8d`hR3)J zKe{epG;Ol&*oBuc9=(KzELsx!F&o>l4nuenwc=Z-=RZXj9II0}CiRg=;9wv^gH zT|`|@{a04fAnHyU>fKJ|w3f=8{wpn<`KZdV%G?+>-&JUzETJlTO;nXg1qbDcZQy)l z!1=gnv+31Q@jJ&@-jr)aLtnywrIdzJ7j!%7P`h2FoLcF$VjnA|LPaTEVYbckX^^u2 zigx%~YM82As<2I*{{yR?v)F6PcgU7e+o(!e2~{ccQ1x~=>B7cnVnVYgx;wGe6aAeW oPDlv_%0m8NbxnD7aWFKrGGi>RBAPq3-y6M~dBYREoi*h93l@2p`2YX_ 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"