From c9be89647c1547d08d589bce332cbbc08584f9de Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Aug 2019 17:12:22 +0200 Subject: [PATCH 01/16] Updated RedisFactory so that it loads redis config from cache.redis too --- module/Common/src/Cache/RedisFactory.php | 3 ++- module/Common/test/Cache/RedisFactoryTest.php | 20 ++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/module/Common/src/Cache/RedisFactory.php b/module/Common/src/Cache/RedisFactory.php index 42118767..f1a2d693 100644 --- a/module/Common/src/Cache/RedisFactory.php +++ b/module/Common/src/Cache/RedisFactory.php @@ -16,7 +16,8 @@ class RedisFactory public function __invoke(ContainerInterface $container): PredisClient { - $redisConfig = $container->get('config')['redis'] ?? []; + $config = $container->get('config'); + $redisConfig = $config['cache']['redis'] ?? $config['redis'] ?? []; $servers = $redisConfig['servers'] ?? []; $servers = is_string($servers) ? explode(',', $servers) : $servers; diff --git a/module/Common/test/Cache/RedisFactoryTest.php b/module/Common/test/Cache/RedisFactoryTest.php index dbc1c9b8..67acba2c 100644 --- a/module/Common/test/Cache/RedisFactoryTest.php +++ b/module/Common/test/Cache/RedisFactoryTest.php @@ -27,7 +27,7 @@ class RedisFactoryTest extends TestCase * @test * @dataProvider provideRedisConfig */ - public function createsRedisClientBasedOnConfig(?array $config, string $expectedCluster): void + public function createsRedisClientBasedOnRedisConfig(?array $config, string $expectedCluster): void { $getConfig = $this->container->get('config')->willReturn([ 'redis' => $config, @@ -39,6 +39,24 @@ class RedisFactoryTest extends TestCase $this->assertInstanceOf($expectedCluster, $client->getOptions()->cluster); } + /** + * @test + * @dataProvider provideRedisConfig + */ + public function createsRedisClientBasedOnCacheConfig(?array $config, string $expectedCluster): void + { + $getConfig = $this->container->get('config')->willReturn([ + 'cache' => [ + 'redis' => $config, + ], + ]); + + $client = ($this->factory)($this->container->reveal()); + + $getConfig->shouldHaveBeenCalledOnce(); + $this->assertInstanceOf($expectedCluster, $client->getOptions()->cluster); + } + public function provideRedisConfig(): iterable { yield 'no config' => [null, PredisCluster::class]; From 16653d60ed49e2515c9934f64e26bae2ed2446c7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Aug 2019 17:44:09 +0200 Subject: [PATCH 02/16] Enhanced CacheFactory to support redis and allow optional APCu --- module/Common/README.md | 40 +++++++++++ module/Common/src/Cache/CacheFactory.php | 48 ++++++++++--- module/Common/test/Cache/CacheFactoryTest.php | 72 +++++++++++-------- 3 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 module/Common/README.md diff --git a/module/Common/README.md b/module/Common/README.md new file mode 100644 index 00000000..3b903f41 --- /dev/null +++ b/module/Common/README.md @@ -0,0 +1,40 @@ +# Shlink Common + +This library provides some utils and conventions for web apps. It's main purpose is to be used on [Shlink](https://github.com/shlinkio/shlink) project, but any PHP project can take advantage. + +Most of the elements it provides require a [PSR-11] container, and it's easy to integrate on [expressive] applications thanks to the `ConfigProvider` it includes. + +## Cache + +A [doctrine cache] adapter is registered, which returns different instances depending on your configuration: + + * An `ArrayCache` instance when the `debug` config is set to true. + * A `PredisCache` instance when the `cache.redis` config is defined. + * An `ArrayCache` instance when no `cache.redis` is defined and the APCu extension is not installed. + * An `ApcuCache`instance when no `cache.redis` is defined and the APCu extension is installed. + + Any of the adapters will use the namespace defined in `cache.namespace` config entry. + + ```php + false, + + 'cache' => [ + 'namespace' => 'my_namespace', + 'redis' => [ + 'servers' => [ + 'tcp://1.1.1.1:6379', + 'tcp://2.2.2.2:6379', + 'tcp://3.3.3.3:6379', + ], + ], + ], + +]; +``` + +When the `cache.redis` config is provided, a set of servers is expected. If only one server is provided, this library will treat it as a regular server, but if several servers are defined, it will treat them as a redis cluster and expect the servers to be configured as such. diff --git a/module/Common/src/Cache/CacheFactory.php b/module/Common/src/Cache/CacheFactory.php index 907621ea..12676dd8 100644 --- a/module/Common/src/Cache/CacheFactory.php +++ b/module/Common/src/Cache/CacheFactory.php @@ -4,22 +4,48 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Cache; use Doctrine\Common\Cache; -use Interop\Container\ContainerInterface; -use Shlinkio\Shlink\Core\Options\AppOptions; -use Zend\ServiceManager\Factory\FactoryInterface; +use Predis\Client as PredisClient; +use Psr\Container\ContainerInterface; -use function Shlinkio\Shlink\Common\env; +use function extension_loaded; -class CacheFactory implements FactoryInterface +class CacheFactory { - public function __invoke(ContainerInterface $container, $requestedName, ?array $options = null): Cache\Cache - { - // TODO Make use of the redis cache via RedisFactory when possible + /** @var callable|null */ + private $apcuEnabled; - $appOptions = $container->get(AppOptions::class); - $adapter = env('APP_ENV', 'pro') === 'pro' ? new Cache\ApcuCache() : new Cache\ArrayCache(); - $adapter->setNamespace((string) $appOptions); + public function __construct(?callable $apcuEnabled = null) + { + $this->apcuEnabled = $apcuEnabled ?? function () { + return extension_loaded('apcu'); + }; + } + + public function __invoke(ContainerInterface $container): Cache\CacheProvider + { + $config = $container->get('config'); + $adapter = $this->buildAdapter($config, $container); + $adapter->setNamespace($config['cache']['namespace'] ?? ''); return $adapter; } + + private function buildAdapter(array $config, ContainerInterface $container): Cache\CacheProvider + { + $isDebug = (bool) ($config['debug'] ?? false); + $redisConfig = $config['cache']['redis'] ?? null; + $apcuEnabled = ($this->apcuEnabled)(); + + if ($isDebug || (! $apcuEnabled && $redisConfig === null)) { + return new Cache\ArrayCache(); + } + + if ($redisConfig === null) { + return new Cache\ApcuCache(); + } + + /** @var PredisClient $predis */ + $predis = $container->get(RedisFactory::SERVICE_NAME); + return new Cache\PredisCache($predis); + } } diff --git a/module/Common/test/Cache/CacheFactoryTest.php b/module/Common/test/Cache/CacheFactoryTest.php index d83d56b7..9575befc 100644 --- a/module/Common/test/Cache/CacheFactoryTest.php +++ b/module/Common/test/Cache/CacheFactoryTest.php @@ -3,48 +3,60 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Common\Cache; -use Doctrine\Common\Cache\ApcuCache; -use Doctrine\Common\Cache\ArrayCache; +use Doctrine\Common\Cache; use PHPUnit\Framework\TestCase; +use Predis\ClientInterface; +use Prophecy\Prophecy\ObjectProphecy; +use Psr\Container\ContainerInterface; use Shlinkio\Shlink\Common\Cache\CacheFactory; -use Shlinkio\Shlink\Core\Options\AppOptions; -use Zend\ServiceManager\ServiceManager; - -use function putenv; +use Shlinkio\Shlink\Common\Cache\RedisFactory; class CacheFactoryTest extends TestCase { - /** @var CacheFactory */ - private $factory; - /** @var ServiceManager */ - private $sm; + /** @var ObjectProphecy */ + private $container; public function setUp(): void { - $this->factory = new CacheFactory(); - $this->sm = new ServiceManager(['services' => [ - AppOptions::class => new AppOptions(), - ]]); + $this->container = $this->prophesize(ContainerInterface::class); } - public static function tearDownAfterClass(): void - { - putenv('APP_ENV'); + /** + * @test + * @dataProvider provideCacheConfig + */ + public function expectedCacheAdapterIsReturned( + array $config, + string $expectedAdapterClass, + string $expectedNamespace, + ?callable $apcuEnabled = null + ): void { + $factory = new CacheFactory($apcuEnabled); + + $getConfig = $this->container->get('config')->willReturn($config); + $getRedis = $this->container->get(RedisFactory::SERVICE_NAME)->willReturn( + $this->prophesize(ClientInterface::class)->reveal() + ); + + $cache = $factory($this->container->reveal()); + + $this->assertInstanceOf($expectedAdapterClass, $cache); + $this->assertEquals($expectedNamespace, $cache->getNamespace()); + $getConfig->shouldHaveBeenCalledOnce(); + $getRedis->shouldHaveBeenCalledTimes($expectedAdapterClass === Cache\PredisCache::class ? 1 :0); } - /** @test */ - public function productionReturnsApcAdapter(): void + public function provideCacheConfig(): iterable { - putenv('APP_ENV=pro'); - $instance = ($this->factory)($this->sm, ''); - $this->assertInstanceOf(ApcuCache::class, $instance); - } - - /** @test */ - public function developmentReturnsArrayAdapter(): void - { - putenv('APP_ENV=dev'); - $instance = ($this->factory)($this->sm, ''); - $this->assertInstanceOf(ArrayCache::class, $instance); + yield 'debug true' => [['debug' => true], Cache\ArrayCache::class, '']; + yield 'debug false' => [['debug' => false], Cache\ApcuCache::class, '']; + yield 'no debug' => [[], Cache\ApcuCache::class, '']; + yield 'with redis' => [['cache' => [ + 'namespace' => $namespace = 'some_namespace', + 'redis' => [], + ]], Cache\PredisCache::class, $namespace]; + yield 'debug false and no apcu' => [['debug' => false], Cache\ArrayCache::class, '', function () { + return false; + }]; } } From 4aed8e6b5987e65172c3fda370cd8ea3e92c7464 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Aug 2019 23:16:34 +0200 Subject: [PATCH 03/16] Moved ShlinkTable class to CLI module --- module/CLI/src/Command/Api/ListKeysCommand.php | 2 +- module/CLI/src/Command/ShortUrl/GetVisitsCommand.php | 2 +- module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php | 2 +- module/CLI/src/Command/Tag/ListTagsCommand.php | 2 +- .../{Common/src/Console => CLI/src/Util}/ShlinkTable.php | 2 +- .../test/Console => CLI/test/Util}/ShlinkTableTest.php | 8 ++++---- 6 files changed, 9 insertions(+), 9 deletions(-) rename module/{Common/src/Console => CLI/src/Util}/ShlinkTable.php (96%) rename module/{Common/test/Console => CLI/test/Util}/ShlinkTableTest.php (93%) diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index c49af8e2..22a85b6f 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Console\ShlinkTable; +use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index e557757a..aa8eeca5 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -5,7 +5,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Cake\Chronos\Chronos; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Console\ShlinkTable; +use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\VisitsParams; diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index d44ce109..329676c5 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Console\ShlinkTable; +use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Adapter\PaginableRepositoryAdapter; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 152f9d43..5086ac3e 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Console\ShlinkTable; +use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; diff --git a/module/Common/src/Console/ShlinkTable.php b/module/CLI/src/Util/ShlinkTable.php similarity index 96% rename from module/Common/src/Console/ShlinkTable.php rename to module/CLI/src/Util/ShlinkTable.php index 1ded8334..44a93bbe 100644 --- a/module/Common/src/Console/ShlinkTable.php +++ b/module/CLI/src/Util/ShlinkTable.php @@ -1,7 +1,7 @@ prophesize(OutputInterface::class)->reveal()); From 53243d17642391749996b86ab5b70242044a7ccd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Aug 2019 23:26:39 +0200 Subject: [PATCH 04/16] Moved WrongIpException to IpGeolocation module --- data/migrations/Version20180913205455.php | 2 +- module/CLI/src/Command/Visit/LocateVisitsCommand.php | 2 +- .../CLI/test/Command/Visit/LocateVisitsCommandTest.php | 2 +- module/Common/src/Util/IpAddress.php | 2 +- module/Core/src/Entity/Visit.php | 2 +- .../Core/src/EventDispatcher/LocateShortUrlVisit.php | 2 +- .../test/EventDispatcher/LocateShortUrlVisitTest.php | 2 +- .../IpGeolocation/src/Exception/ExceptionInterface.php | 10 ++++++++++ .../src/Exception/WrongIpException.php | 5 +++-- .../src/Resolver/ChainIpLocationResolver.php | 3 +-- .../src/Resolver/EmptyIpLocationResolver.php | 3 +-- .../src/Resolver/GeoLite2LocationResolver.php | 3 +-- .../src/Resolver/IpApiLocationResolver.php | 3 +-- .../src/Resolver/IpLocationResolverInterface.php | 2 +- .../test/Exception/WrongIpExceptionTest.php | 9 +++++---- .../test/Resolver/ChainIpLocationResolverTest.php | 2 +- .../test/Resolver/GeoLite2LocationResolverTest.php | 2 +- .../test/Resolver/IpApiLocationResolverTest.php | 2 +- module/Rest/test/Util/RestUtilsTest.php | 2 +- 19 files changed, 34 insertions(+), 26 deletions(-) create mode 100644 module/IpGeolocation/src/Exception/ExceptionInterface.php rename module/{Common => IpGeolocation}/src/Exception/WrongIpException.php (58%) rename module/{Common => IpGeolocation}/test/Exception/WrongIpExceptionTest.php (85%) diff --git a/data/migrations/Version20180913205455.php b/data/migrations/Version20180913205455.php index 0b012ca1..0b2b60d9 100644 --- a/data/migrations/Version20180913205455.php +++ b/data/migrations/Version20180913205455.php @@ -7,8 +7,8 @@ use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Schema\Schema; use Doctrine\Migrations\AbstractMigration; use PDO; -use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\Util\IpAddress; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; /** * Auto-generated Migration: Please modify to your needs! diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index cbaf2118..d59ead2a 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -9,12 +9,12 @@ use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Service\VisitServiceInterface; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Console\Helper\ProgressBar; diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 0ce1a5bf..41a1a8fa 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -9,13 +9,13 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\LocateVisitsCommand; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\Common\Exception\WrongIpException; 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\Model\Visitor; use Shlinkio\Shlink\Core\Service\VisitService; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpApiLocationResolver; use Symfony\Component\Console\Application; diff --git a/module/Common/src/Util/IpAddress.php b/module/Common/src/Util/IpAddress.php index 8521f9be..c237cd15 100644 --- a/module/Common/src/Util/IpAddress.php +++ b/module/Common/src/Util/IpAddress.php @@ -3,7 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Util; -use Shlinkio\Shlink\Common\Exception\WrongIpException; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use function count; use function explode; diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 58401088..a6e87bad 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -6,11 +6,11 @@ namespace Shlinkio\Shlink\Core\Entity; use Cake\Chronos\Chronos; use JsonSerializable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; -use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; class Visit extends AbstractEntity implements JsonSerializable { diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php index e0a0f274..b0eb5107 100644 --- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php +++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php @@ -7,9 +7,9 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php index 5c10406c..966c02a8 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -10,7 +10,6 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; @@ -19,6 +18,7 @@ use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; diff --git a/module/IpGeolocation/src/Exception/ExceptionInterface.php b/module/IpGeolocation/src/Exception/ExceptionInterface.php new file mode 100644 index 00000000..12b2ccdc --- /dev/null +++ b/module/IpGeolocation/src/Exception/ExceptionInterface.php @@ -0,0 +1,10 @@ +assertEquals(0, $e->getCode()); $this->assertNull($e->getPrevious()); } + /** @test */ - public function fromIpAddressProperlyCreatesExceptionWithPrev() + public function fromIpAddressProperlyCreatesExceptionWithPrev(): void { $prev = new Exception('Previous error'); $e = WrongIpException::fromIpAddress('1.2.3.4', $prev); diff --git a/module/IpGeolocation/test/Resolver/ChainIpLocationResolverTest.php b/module/IpGeolocation/test/Resolver/ChainIpLocationResolverTest.php index 34e5100c..a95b60c0 100644 --- a/module/IpGeolocation/test/Resolver/ChainIpLocationResolverTest.php +++ b/module/IpGeolocation/test/Resolver/ChainIpLocationResolverTest.php @@ -5,7 +5,7 @@ namespace ShlinkioTest\Shlink\IpGeolocation\Resolver; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Exception\WrongIpException; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\ChainIpLocationResolver; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; diff --git a/module/IpGeolocation/test/Resolver/GeoLite2LocationResolverTest.php b/module/IpGeolocation/test/Resolver/GeoLite2LocationResolverTest.php index 66122796..806f2759 100644 --- a/module/IpGeolocation/test/Resolver/GeoLite2LocationResolverTest.php +++ b/module/IpGeolocation/test/Resolver/GeoLite2LocationResolverTest.php @@ -9,7 +9,7 @@ use GeoIp2\Model\City; use MaxMind\Db\Reader\InvalidDatabaseException; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Exception\WrongIpException; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\GeoLite2LocationResolver; diff --git a/module/IpGeolocation/test/Resolver/IpApiLocationResolverTest.php b/module/IpGeolocation/test/Resolver/IpApiLocationResolverTest.php index 7e0425de..a87edf98 100644 --- a/module/IpGeolocation/test/Resolver/IpApiLocationResolverTest.php +++ b/module/IpGeolocation/test/Resolver/IpApiLocationResolverTest.php @@ -8,7 +8,7 @@ use GuzzleHttp\Exception\TransferException; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Exception\WrongIpException; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpApiLocationResolver; diff --git a/module/Rest/test/Util/RestUtilsTest.php b/module/Rest/test/Util/RestUtilsTest.php index 0f3ab4ff..fd5578ca 100644 --- a/module/Rest/test/Util/RestUtilsTest.php +++ b/module/Rest/test/Util/RestUtilsTest.php @@ -5,9 +5,9 @@ namespace ShlinkioTest\Shlink\Rest\Util; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; -use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\Rest\Exception\AuthenticationException; use Shlinkio\Shlink\Rest\Util\RestUtils; From 986c165815490c5267bd9c0768467796ae681895 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Aug 2019 23:30:47 +0200 Subject: [PATCH 05/16] Moved RuntimeException to IpGeolocation module --- module/CLI/src/Command/Visit/UpdateDbCommand.php | 2 +- module/CLI/src/Util/GeolocationDbUpdater.php | 2 +- module/CLI/test/Command/Visit/UpdateDbCommandTest.php | 2 +- module/CLI/test/Util/GeolocationDbUpdaterTest.php | 2 +- module/Common/src/Exception/PreviewGenerationException.php | 2 ++ .../src/Exception/RuntimeException.php | 2 +- module/IpGeolocation/src/Exception/WrongIpException.php | 2 +- module/IpGeolocation/src/GeoLite2/DbUpdater.php | 2 +- module/IpGeolocation/src/GeoLite2/DbUpdaterInterface.php | 2 +- module/IpGeolocation/test/GeoLite2/DbUpdaterTest.php | 2 +- 10 files changed, 11 insertions(+), 9 deletions(-) rename module/{Common => IpGeolocation}/src/Exception/RuntimeException.php (76%) diff --git a/module/CLI/src/Command/Visit/UpdateDbCommand.php b/module/CLI/src/Command/Visit/UpdateDbCommand.php index 938c24b7..0e1cc223 100644 --- a/module/CLI/src/Command/Visit/UpdateDbCommand.php +++ b/module/CLI/src/Command/Visit/UpdateDbCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/Util/GeolocationDbUpdater.php index 48f958ca..1a2c5715 100644 --- a/module/CLI/src/Util/GeolocationDbUpdater.php +++ b/module/CLI/src/Util/GeolocationDbUpdater.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Util; use Cake\Chronos\Chronos; use GeoIp2\Database\Reader; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\Common\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock\Factory as Locker; use Throwable; diff --git a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php index ea4e0861..2ebd6a38 100644 --- a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php +++ b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php @@ -8,7 +8,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\UpdateDbCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; diff --git a/module/CLI/test/Util/GeolocationDbUpdaterTest.php b/module/CLI/test/Util/GeolocationDbUpdaterTest.php index 137e70de..6a8dcceb 100644 --- a/module/CLI/test/Util/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/Util/GeolocationDbUpdaterTest.php @@ -11,7 +11,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; -use Shlinkio\Shlink\Common\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock; use Throwable; diff --git a/module/Common/src/Exception/PreviewGenerationException.php b/module/Common/src/Exception/PreviewGenerationException.php index 726e60fa..a889474a 100644 --- a/module/Common/src/Exception/PreviewGenerationException.php +++ b/module/Common/src/Exception/PreviewGenerationException.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Exception; +use RuntimeException; + use function sprintf; /** @deprecated */ diff --git a/module/Common/src/Exception/RuntimeException.php b/module/IpGeolocation/src/Exception/RuntimeException.php similarity index 76% rename from module/Common/src/Exception/RuntimeException.php rename to module/IpGeolocation/src/Exception/RuntimeException.php index 6fad16aa..d55a5af9 100644 --- a/module/Common/src/Exception/RuntimeException.php +++ b/module/IpGeolocation/src/Exception/RuntimeException.php @@ -1,7 +1,7 @@ Date: Sat, 10 Aug 2019 23:58:21 +0200 Subject: [PATCH 06/16] Moved some elements in Common module to more proper locations --- composer.json | 6 ++-- config/autoload/dependencies.global.php | 6 ---- config/config.php | 1 + module/Common/config/dependencies.config.php | 20 +---------- .../DottedAccessConfigAbstractFactory.php | 4 +-- ...sponseImplicitOptionsMiddlewareFactory.php | 34 ------------------ .../Common/src/Factory/TranslatorFactory.php | 32 ----------------- module/Common/src/I18n/TranslatorFactory.php | 16 +++++++++ .../TranslatorFactoryTest.php | 10 +++--- .../config/event_dispatcher.config.php | 2 ++ .../config/dependencies.config.php | 16 +++++++++ module/Integrations/src/ConfigProvider.php | 14 ++++++++ ...sponseImplicitOptionsMiddlewareFactory.php | 17 +++++++++ ...seImplicitOptionsMiddlewareFactoryTest.php | 13 ++++--- module/IpGeolocation/README.md | 18 ++++++++++ .../IpGeolocation/config/geolite2.config.php | 35 +++++++++++++++++++ phpunit.xml.dist | 3 ++ 17 files changed, 139 insertions(+), 108 deletions(-) delete mode 100644 module/Common/src/Factory/EmptyResponseImplicitOptionsMiddlewareFactory.php delete mode 100644 module/Common/src/Factory/TranslatorFactory.php create mode 100644 module/Common/src/I18n/TranslatorFactory.php rename module/Common/test/{Factory => I18n}/TranslatorFactoryTest.php (66%) create mode 100644 module/Integrations/config/dependencies.config.php create mode 100644 module/Integrations/src/ConfigProvider.php create mode 100644 module/Integrations/src/Middleware/EmptyResponseImplicitOptionsMiddlewareFactory.php rename module/{Common/test/Factory => Integrations/test/Middleware}/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php (67%) create mode 100644 module/IpGeolocation/README.md create mode 100644 module/IpGeolocation/config/geolite2.config.php diff --git a/composer.json b/composer.json index bedf3f9a..2ed59701 100644 --- a/composer.json +++ b/composer.json @@ -75,7 +75,8 @@ "Shlinkio\\Shlink\\Core\\": "module/Core/src", "Shlinkio\\Shlink\\Common\\": "module/Common/src", "Shlinkio\\Shlink\\EventDispatcher\\": "module/EventDispatcher/src", - "Shlinkio\\Shlink\\IpGeolocation\\": "module/IpGeolocation/src/" + "Shlinkio\\Shlink\\IpGeolocation\\": "module/IpGeolocation/src/", + "Shlinkio\\Shlink\\Integrations\\": "module/Integrations/src/" }, "files": [ "module/Common/functions/functions.php", @@ -96,7 +97,8 @@ "module/Common/test-db" ], "ShlinkioTest\\Shlink\\EventDispatcher\\": "module/EventDispatcher/test", - "ShlinkioTest\\Shlink\\IpGeolocation\\": "module/IpGeolocation/test" + "ShlinkioTest\\Shlink\\IpGeolocation\\": "module/IpGeolocation/test", + "ShlinkioTest\\Shlink\\Integrations\\": "module/Integrations/test" } }, "scripts": { diff --git a/config/autoload/dependencies.global.php b/config/autoload/dependencies.global.php index 2dc0de9b..3577f3d4 100644 --- a/config/autoload/dependencies.global.php +++ b/config/autoload/dependencies.global.php @@ -1,18 +1,12 @@ [ - 'factories' => [ - ImplicitOptionsMiddleware::class => EmptyResponseImplicitOptionsMiddlewareFactory::class, - ], - 'delegators' => [ Expressive\Application::class => [ Container\ApplicationConfigInjectionDelegator::class, diff --git a/config/config.php b/config/config.php index d5776eef..f6297fd8 100644 --- a/config/config.php +++ b/config/config.php @@ -18,6 +18,7 @@ return (new ConfigAggregator\ConfigAggregator([ ExpressiveErrorHandler\ConfigProvider::class, Common\ConfigProvider::class, IpGeolocation\ConfigProvider::class, + Integrations\ConfigProvider::class, Core\ConfigProvider::class, CLI\ConfigProvider::class, Rest\ConfigProvider::class, diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 34e37d6e..fec932e1 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -3,7 +3,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common; -use GeoIp2\Database\Reader; use GuzzleHttp\Client as GuzzleClient; use Monolog\Logger; use Psr\Log\LoggerInterface; @@ -12,7 +11,6 @@ use Symfony\Component\Filesystem\Filesystem; use Zend\I18n\Translator\Translator; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Zend\ServiceManager\Factory\InvokableFactory; -use Zend\ServiceManager\Proxy\LazyServiceFactory; return [ @@ -20,9 +18,8 @@ return [ 'factories' => [ GuzzleClient::class => InvokableFactory::class, Filesystem::class => InvokableFactory::class, - Reader::class => ConfigAbstractFactory::class, - Translator::class => Factory\TranslatorFactory::class, + Translator::class => I18n\TranslatorFactory::class, Template\Extension\TranslatorExtension::class => ConfigAbstractFactory::class, Middleware\LocaleMiddleware::class => ConfigAbstractFactory::class, @@ -44,24 +41,9 @@ return [ 'abstract_factories' => [ Factory\DottedAccessConfigAbstractFactory::class, ], - 'delegators' => [ - // The GeoLite2 db reader has to be lazy so that it does not try to load the DB file at app bootstrapping. - // By doing so, it would fail the first time shlink tries to download it. - Reader::class => [ - LazyServiceFactory::class, - ], - ], - - 'lazy_services' => [ - 'class_map' => [ - Reader::class => Reader::class, - ], - ], ], ConfigAbstractFactory::class => [ - Reader::class => ['config.geolite2.db_location'], - Template\Extension\TranslatorExtension::class => ['translator'], Middleware\LocaleMiddleware::class => ['translator'], Middleware\CloseDbConnectionMiddleware::class => ['em'], diff --git a/module/Common/src/Factory/DottedAccessConfigAbstractFactory.php b/module/Common/src/Factory/DottedAccessConfigAbstractFactory.php index cfe01eef..64c4d2f8 100644 --- a/module/Common/src/Factory/DottedAccessConfigAbstractFactory.php +++ b/module/Common/src/Factory/DottedAccessConfigAbstractFactory.php @@ -22,11 +22,9 @@ class DottedAccessConfigAbstractFactory implements AbstractFactoryInterface /** * Can the factory create an instance for the service? * - * @param ContainerInterface $container * @param string $requestedName - * @return bool */ - public function canCreate(ContainerInterface $container, $requestedName) + public function canCreate(ContainerInterface $container, $requestedName): bool { return substr_count($requestedName, '.') > 0; } diff --git a/module/Common/src/Factory/EmptyResponseImplicitOptionsMiddlewareFactory.php b/module/Common/src/Factory/EmptyResponseImplicitOptionsMiddlewareFactory.php deleted file mode 100644 index 793a2699..00000000 --- a/module/Common/src/Factory/EmptyResponseImplicitOptionsMiddlewareFactory.php +++ /dev/null @@ -1,34 +0,0 @@ -get('config'); - return Translator::factory($config['translator'] ?? []); - } -} diff --git a/module/Common/src/I18n/TranslatorFactory.php b/module/Common/src/I18n/TranslatorFactory.php new file mode 100644 index 00000000..942dad07 --- /dev/null +++ b/module/Common/src/I18n/TranslatorFactory.php @@ -0,0 +1,16 @@ +get('config'); + return Translator::factory($config['translator'] ?? []); + } +} diff --git a/module/Common/test/Factory/TranslatorFactoryTest.php b/module/Common/test/I18n/TranslatorFactoryTest.php similarity index 66% rename from module/Common/test/Factory/TranslatorFactoryTest.php rename to module/Common/test/I18n/TranslatorFactoryTest.php index 07cbfd5b..756a3991 100644 --- a/module/Common/test/Factory/TranslatorFactoryTest.php +++ b/module/Common/test/I18n/TranslatorFactoryTest.php @@ -1,10 +1,10 @@ factory->__invoke(new ServiceManager(['services' => [ + $instance = ($this->factory)(new ServiceManager(['services' => [ 'config' => [], - ]]), ''); + ]])); $this->assertInstanceOf(Translator::class, $instance); } } diff --git a/module/EventDispatcher/config/event_dispatcher.config.php b/module/EventDispatcher/config/event_dispatcher.config.php index ed932155..8941443f 100644 --- a/module/EventDispatcher/config/event_dispatcher.config.php +++ b/module/EventDispatcher/config/event_dispatcher.config.php @@ -23,6 +23,8 @@ return [ Psr\EventDispatcherInterface::class => Phly\EventDispatcher::class, ], 'delegators' => [ + // The listener provider has to be lazy, because it uses the Swoole server to generate AsyncEventListeners + // Without making this lazy, CLI commands which depend on the EventDispatcher fail Psr\ListenerProviderInterface::class => [ LazyServiceFactory::class, ], diff --git a/module/Integrations/config/dependencies.config.php b/module/Integrations/config/dependencies.config.php new file mode 100644 index 00000000..d2944e54 --- /dev/null +++ b/module/Integrations/config/dependencies.config.php @@ -0,0 +1,16 @@ + [ + 'factories' => [ + ImplicitOptionsMiddleware::class => Middleware\EmptyResponseImplicitOptionsMiddlewareFactory::class, + ], + ], + +]; diff --git a/module/Integrations/src/ConfigProvider.php b/module/Integrations/src/ConfigProvider.php new file mode 100644 index 00000000..bcbab248 --- /dev/null +++ b/module/Integrations/src/ConfigProvider.php @@ -0,0 +1,14 @@ +factory->__invoke(new ServiceManager(), ''); + $instance = ($this->factory)(); $this->assertInstanceOf(ImplicitOptionsMiddleware::class, $instance); } /** @test */ - public function responsePrototypeIsEmptyResponse() + public function responsePrototypeIsEmptyResponse(): void { - $instance = $this->factory->__invoke(new ServiceManager(), ''); + $instance = ($this->factory)(); $ref = new ReflectionObject($instance); $prop = $ref->getProperty('responseFactory'); diff --git a/module/IpGeolocation/README.md b/module/IpGeolocation/README.md new file mode 100644 index 00000000..feed5025 --- /dev/null +++ b/module/IpGeolocation/README.md @@ -0,0 +1,18 @@ +# IP Address Geolocation module + +Shlink module with tools to locate an IP address suing different strategies. + +```php + [ + 'db_location' => __DIR__ . '/../../data/GeoLite2-City.mmdb', + 'temp_dir' => sys_get_temp_dir(), + // 'download_from' => 'http://geolite.maxmind.com/download/geoip/database/GeoLite2-City.tar.gz', + ], + +]; +``` diff --git a/module/IpGeolocation/config/geolite2.config.php b/module/IpGeolocation/config/geolite2.config.php new file mode 100644 index 00000000..aa910a50 --- /dev/null +++ b/module/IpGeolocation/config/geolite2.config.php @@ -0,0 +1,35 @@ + [ + 'factories' => [ + Reader::class => ConfigAbstractFactory::class, + ], + 'delegators' => [ + // The GeoLite2 db reader has to be lazy so that it does not try to load the DB file at app bootstrapping. + // By doing so, it would fail the first time shlink tries to download it. + Reader::class => [ + LazyServiceFactory::class, + ], + ], + + 'lazy_services' => [ + 'class_map' => [ + Reader::class => Reader::class, + ], + ], + ], + + ConfigAbstractFactory::class => [ + Reader::class => ['config.geolite2.db_location'], + ], + +]; diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 910cdc99..4787de5d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -24,6 +24,9 @@ ./module/IpGeolocation/test + + ./module/Integrations/test + From 0323e0d17d30ed9bd9507ef156eaf21970b28afb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 10:22:19 +0200 Subject: [PATCH 07/16] Simplified IpAddressMiddlewareFactory and decoupled from Core module --- module/Common/src/Image/ImageBuilder.php | 1 + .../Common/src/Image/ImageBuilderFactory.php | 1 + .../src/Image/ImageBuilderInterface.php | 1 + module/Common/src/Image/ImageFactory.php | 1 + module/Common/src/Logger/LoggerFactory.php | 21 +++-------------- .../Middleware/IpAddressMiddlewareFactory.php | 23 +++++-------------- .../IpAddressMiddlewareFactoryTest.php | 5 ++-- module/Core/src/Model/Visitor.php | 5 ++-- 8 files changed, 17 insertions(+), 41 deletions(-) diff --git a/module/Common/src/Image/ImageBuilder.php b/module/Common/src/Image/ImageBuilder.php index 2dc3b942..06940327 100644 --- a/module/Common/src/Image/ImageBuilder.php +++ b/module/Common/src/Image/ImageBuilder.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Common\Image; use mikehaertl\wkhtmlto\Image; use Zend\ServiceManager\AbstractPluginManager; +/** @deprecated */ class ImageBuilder extends AbstractPluginManager implements ImageBuilderInterface { protected $instanceOf = Image::class; diff --git a/module/Common/src/Image/ImageBuilderFactory.php b/module/Common/src/Image/ImageBuilderFactory.php index a330666f..93c369bb 100644 --- a/module/Common/src/Image/ImageBuilderFactory.php +++ b/module/Common/src/Image/ImageBuilderFactory.php @@ -10,6 +10,7 @@ use Zend\ServiceManager\Exception\ServiceNotCreatedException; use Zend\ServiceManager\Exception\ServiceNotFoundException; use Zend\ServiceManager\Factory\FactoryInterface; +/** @deprecated */ class ImageBuilderFactory implements FactoryInterface { /** diff --git a/module/Common/src/Image/ImageBuilderInterface.php b/module/Common/src/Image/ImageBuilderInterface.php index ff4baeeb..c2d3c820 100644 --- a/module/Common/src/Image/ImageBuilderInterface.php +++ b/module/Common/src/Image/ImageBuilderInterface.php @@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Common\Image; use Zend\ServiceManager\ServiceLocatorInterface; +/** @deprecated */ interface ImageBuilderInterface extends ServiceLocatorInterface { } diff --git a/module/Common/src/Image/ImageFactory.php b/module/Common/src/Image/ImageFactory.php index 1305d16d..b02cde04 100644 --- a/module/Common/src/Image/ImageFactory.php +++ b/module/Common/src/Image/ImageFactory.php @@ -10,6 +10,7 @@ use Zend\ServiceManager\Exception\ServiceNotCreatedException; use Zend\ServiceManager\Exception\ServiceNotFoundException; use Zend\ServiceManager\Factory\FactoryInterface; +/** @deprecated */ class ImageFactory implements FactoryInterface { /** diff --git a/module/Common/src/Logger/LoggerFactory.php b/module/Common/src/Logger/LoggerFactory.php index 0671bbcc..7e896108 100644 --- a/module/Common/src/Logger/LoggerFactory.php +++ b/module/Common/src/Logger/LoggerFactory.php @@ -5,29 +5,14 @@ namespace Shlinkio\Shlink\Common\Logger; use Cascade\Cascade; use Interop\Container\ContainerInterface; -use Interop\Container\Exception\ContainerException; -use Zend\ServiceManager\Exception\ServiceNotCreatedException; -use Zend\ServiceManager\Exception\ServiceNotFoundException; -use Zend\ServiceManager\Factory\FactoryInterface; +use Monolog\Logger; use function count; use function explode; -class LoggerFactory implements FactoryInterface +class LoggerFactory { - /** - * Create an object - * - * @param ContainerInterface $container - * @param string $requestedName - * @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 ContainerException if any other error occurs - */ - public function __invoke(ContainerInterface $container, $requestedName, ?array $options = null) + public function __invoke(ContainerInterface $container, string $requestedName, ?array $options = null): Logger { $config = $container->has('config') ? $container->get('config') : []; Cascade::fileConfig($config['logger'] ?? ['loggers' => []]); diff --git a/module/Common/src/Middleware/IpAddressMiddlewareFactory.php b/module/Common/src/Middleware/IpAddressMiddlewareFactory.php index 67063c28..73d643b4 100644 --- a/module/Common/src/Middleware/IpAddressMiddlewareFactory.php +++ b/module/Common/src/Middleware/IpAddressMiddlewareFactory.php @@ -3,28 +3,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Middleware; -use Interop\Container\ContainerInterface; +use Psr\Container\ContainerInterface; use RKA\Middleware\IpAddress; -use Shlinkio\Shlink\Core\Model\Visitor; -use Zend\ServiceManager\Exception\ServiceNotCreatedException; -use Zend\ServiceManager\Exception\ServiceNotFoundException; -use Zend\ServiceManager\Factory\FactoryInterface; -class IpAddressMiddlewareFactory implements FactoryInterface +class IpAddressMiddlewareFactory { - /** - * Create an object - * - * @param ContainerInterface $container - * @param string $requestedName - * @param null|array $options - * @throws ServiceNotFoundException if unable to resolve the service. - * @throws ServiceNotCreatedException if an exception is raised when creating a service. - */ - public function __invoke(ContainerInterface $container, $requestedName, ?array $options = null): IpAddress + public const REQUEST_ATTR = 'remote_address'; + + public function __invoke(ContainerInterface $container): IpAddress { $config = $container->get('config'); $headersToInspect = $config['ip_address_resolution']['headers_to_inspect'] ?? []; - return new IpAddress(true, [], Visitor::REMOTE_ADDRESS_ATTR, $headersToInspect); + return new IpAddress(true, [], self::REQUEST_ATTR, $headersToInspect); } } diff --git a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php index 4a3361bd..fdf81a35 100644 --- a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php +++ b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php @@ -6,7 +6,6 @@ namespace ShlinkioTest\Shlink\Common\Middleware; use PHPUnit\Framework\TestCase; use ReflectionObject; use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; -use Shlinkio\Shlink\Core\Model\Visitor; use Zend\ServiceManager\ServiceManager; class IpAddressMiddlewareFactoryTest extends TestCase @@ -26,7 +25,7 @@ class IpAddressMiddlewareFactoryTest extends TestCase { $instance = ($this->factory)(new ServiceManager(['services' => [ 'config' => $config, - ]]), ''); + ]])); $ref = new ReflectionObject($instance); $checkProxyHeaders = $ref->getProperty('checkProxyHeaders'); @@ -40,7 +39,7 @@ class IpAddressMiddlewareFactoryTest extends TestCase $this->assertTrue($checkProxyHeaders->getValue($instance)); $this->assertEquals([], $trustedProxies->getValue($instance)); - $this->assertEquals(Visitor::REMOTE_ADDRESS_ATTR, $attributeName->getValue($instance)); + $this->assertEquals(IpAddressMiddlewareFactory::REQUEST_ATTR, $attributeName->getValue($instance)); $this->assertEquals($expectedHeadersToInspect, $headersToInspect->getValue($instance)); } diff --git a/module/Core/src/Model/Visitor.php b/module/Core/src/Model/Visitor.php index 8a8107cf..1a4506b7 100644 --- a/module/Core/src/Model/Visitor.php +++ b/module/Core/src/Model/Visitor.php @@ -4,11 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Model; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; final class Visitor { - public const REMOTE_ADDRESS_ATTR = 'remote_address'; - /** @var string */ private $userAgent; /** @var string */ @@ -28,7 +27,7 @@ final class Visitor return new self( $request->getHeaderLine('User-Agent'), $request->getHeaderLine('Referer'), - $request->getAttribute(self::REMOTE_ADDRESS_ATTR) + $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR) ); } From 15bd83994065edb48b05f908c561a30ff801380e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 13:06:10 +0200 Subject: [PATCH 08/16] Improved README files --- module/Common/README.md | 55 ++++++++++++++++++- .../src/Middleware/LocaleMiddleware.php | 2 - module/IpGeolocation/README.md | 14 ++++- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/module/Common/README.md b/module/Common/README.md index 3b903f41..0804f249 100644 --- a/module/Common/README.md +++ b/module/Common/README.md @@ -4,14 +4,21 @@ This library provides some utils and conventions for web apps. It's main purpose Most of the elements it provides require a [PSR-11] container, and it's easy to integrate on [expressive] applications thanks to the `ConfigProvider` it includes. +## Install + +Install this library using composer + + composer require shlinkio/shlink-common + +> This library is also an expressive module which provides its own `ConfigProvider`. Add it to your configuration to get everything automatically set up. + ## Cache A [doctrine cache] adapter is registered, which returns different instances depending on your configuration: - * An `ArrayCache` instance when the `debug` config is set to true. - * A `PredisCache` instance when the `cache.redis` config is defined. - * An `ArrayCache` instance when no `cache.redis` is defined and the APCu extension is not installed. + * An `ArrayCache` instance when the `debug` config is set to true or when the APUc extension is not installed and the `cache.redis` config is not defined. * An `ApcuCache`instance when no `cache.redis` is defined and the APCu extension is installed. + * A `PredisCache` instance when the `cache.redis` config is defined. Any of the adapters will use the namespace defined in `cache.namespace` config entry. @@ -38,3 +45,45 @@ return [ ``` When the `cache.redis` config is provided, a set of servers is expected. If only one server is provided, this library will treat it as a regular server, but if several servers are defined, it will treat them as a redis cluster and expect the servers to be configured as such. + +## Middlewares + +This module provides a set of useful middlewares, all registered as services in the container: + +* **CloseDatabaseConnectionMiddleware**: + + Should be an early middleware in the pipeline. It makes use of the EntityManager that ensure the database connection is closed at the end of the request. + + It should be used when serving an app with a non-blocking IO server (like Swoole or ReactPHP), which persist services between requests. + +* **LocaleMiddleware**: + + Sets the locale in the translator, based on the `Accapt-Language` header. + +* **IpAddress** (from [akrabat/ip-address-middleware] package): + + Improves detection of the remote IP address. + + The set of headers which are inspected in order to search for the address can be customized using this configuration: + + ```php + [ + 'headers_to_inspect' => [ + 'CF-Connecting-IP', + 'True-Client-IP', + 'X-Real-IP', + 'Forwarded', + 'X-Forwarded-For', + 'X-Forwarded', + 'X-Cluster-Client-Ip', + 'Client-Ip', + ], + ], + + ]; + ``` diff --git a/module/Common/src/Middleware/LocaleMiddleware.php b/module/Common/src/Middleware/LocaleMiddleware.php index 39cde1c4..12ba084b 100644 --- a/module/Common/src/Middleware/LocaleMiddleware.php +++ b/module/Common/src/Middleware/LocaleMiddleware.php @@ -24,8 +24,6 @@ class LocaleMiddleware implements MiddlewareInterface $this->translator = $translator; } - - /** * Process an incoming server request and return a response, optionally delegating * to the next middleware component to create the response. diff --git a/module/IpGeolocation/README.md b/module/IpGeolocation/README.md index feed5025..a3d02cf1 100644 --- a/module/IpGeolocation/README.md +++ b/module/IpGeolocation/README.md @@ -1,6 +1,16 @@ -# IP Address Geolocation module +# Shlink IP Address Geolocation module -Shlink module with tools to locate an IP address suing different strategies. +Shlink module with tools to geolocate an IP address using different strategies. + +## Install + +Install this library using composer + + composer require shlinkio/shlink-ip-geolocation + +> This library is also an expressive module which provides its own `ConfigProvider`. Add it to your configuration to get everything automatically set up. + +## *TODO* ```php Date: Sun, 11 Aug 2019 13:20:18 +0200 Subject: [PATCH 09/16] Dropped Integrations module and created LICENSE files for new modules --- composer.json | 6 ++---- config/config.php | 1 - module/Common/LICENSE | 21 +++++++++++++++++++ module/Common/README.md | 2 +- module/EventDispatcher/LICENSE | 21 +++++++++++++++++++ module/EventDispatcher/README.md | 13 ++++++++++++ .../config/dependencies.config.php | 16 -------------- module/Integrations/src/ConfigProvider.php | 14 ------------- module/IpGeolocation/LICENSE | 21 +++++++++++++++++++ module/IpGeolocation/README.md | 4 +++- ...sponseImplicitOptionsMiddlewareFactory.php | 2 +- ...seImplicitOptionsMiddlewareFactoryTest.php | 4 ++-- phpunit.xml.dist | 3 --- 13 files changed, 85 insertions(+), 43 deletions(-) create mode 100644 module/Common/LICENSE create mode 100644 module/EventDispatcher/LICENSE create mode 100644 module/EventDispatcher/README.md delete mode 100644 module/Integrations/config/dependencies.config.php delete mode 100644 module/Integrations/src/ConfigProvider.php create mode 100644 module/IpGeolocation/LICENSE rename module/{Integrations => Rest}/src/Middleware/EmptyResponseImplicitOptionsMiddlewareFactory.php (87%) rename module/{Integrations => Rest}/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php (87%) diff --git a/composer.json b/composer.json index 2ed59701..bedf3f9a 100644 --- a/composer.json +++ b/composer.json @@ -75,8 +75,7 @@ "Shlinkio\\Shlink\\Core\\": "module/Core/src", "Shlinkio\\Shlink\\Common\\": "module/Common/src", "Shlinkio\\Shlink\\EventDispatcher\\": "module/EventDispatcher/src", - "Shlinkio\\Shlink\\IpGeolocation\\": "module/IpGeolocation/src/", - "Shlinkio\\Shlink\\Integrations\\": "module/Integrations/src/" + "Shlinkio\\Shlink\\IpGeolocation\\": "module/IpGeolocation/src/" }, "files": [ "module/Common/functions/functions.php", @@ -97,8 +96,7 @@ "module/Common/test-db" ], "ShlinkioTest\\Shlink\\EventDispatcher\\": "module/EventDispatcher/test", - "ShlinkioTest\\Shlink\\IpGeolocation\\": "module/IpGeolocation/test", - "ShlinkioTest\\Shlink\\Integrations\\": "module/Integrations/test" + "ShlinkioTest\\Shlink\\IpGeolocation\\": "module/IpGeolocation/test" } }, "scripts": { diff --git a/config/config.php b/config/config.php index f6297fd8..d5776eef 100644 --- a/config/config.php +++ b/config/config.php @@ -18,7 +18,6 @@ return (new ConfigAggregator\ConfigAggregator([ ExpressiveErrorHandler\ConfigProvider::class, Common\ConfigProvider::class, IpGeolocation\ConfigProvider::class, - Integrations\ConfigProvider::class, Core\ConfigProvider::class, CLI\ConfigProvider::class, Rest\ConfigProvider::class, diff --git a/module/Common/LICENSE b/module/Common/LICENSE new file mode 100644 index 00000000..31778387 --- /dev/null +++ b/module/Common/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2019 Alejandro Celaya + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/module/Common/README.md b/module/Common/README.md index 0804f249..d4dc4815 100644 --- a/module/Common/README.md +++ b/module/Common/README.md @@ -6,7 +6,7 @@ Most of the elements it provides require a [PSR-11] container, and it's easy to ## Install -Install this library using composer +Install this library using composer: composer require shlinkio/shlink-common diff --git a/module/EventDispatcher/LICENSE b/module/EventDispatcher/LICENSE new file mode 100644 index 00000000..31778387 --- /dev/null +++ b/module/EventDispatcher/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2019 Alejandro Celaya + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/module/EventDispatcher/README.md b/module/EventDispatcher/README.md new file mode 100644 index 00000000..47938c12 --- /dev/null +++ b/module/EventDispatcher/README.md @@ -0,0 +1,13 @@ +# Shlink Event Dispatcher + +This library provides a PSR-14 EventDispatcher which is capable of dispatching both regular listeners and async listeners which are run using [swoole]'s task system. + +Most of the elements it provides require a [PSR-11] container, and it's easy to integrate on [expressive] applications thanks to the `ConfigProvider` it includes. + +## Install + +Install this library using composer: + + composer require shlinkio/shlink-event-dispatcher + +> This library is also an expressive module which provides its own `ConfigProvider`. Add it to your configuration to get everything automatically set up. diff --git a/module/Integrations/config/dependencies.config.php b/module/Integrations/config/dependencies.config.php deleted file mode 100644 index d2944e54..00000000 --- a/module/Integrations/config/dependencies.config.php +++ /dev/null @@ -1,16 +0,0 @@ - [ - 'factories' => [ - ImplicitOptionsMiddleware::class => Middleware\EmptyResponseImplicitOptionsMiddlewareFactory::class, - ], - ], - -]; diff --git a/module/Integrations/src/ConfigProvider.php b/module/Integrations/src/ConfigProvider.php deleted file mode 100644 index bcbab248..00000000 --- a/module/Integrations/src/ConfigProvider.php +++ /dev/null @@ -1,14 +0,0 @@ - ./module/IpGeolocation/test - - ./module/Integrations/test - From 848d574f68da0292602fb58ea7a7d8f2d65bafac Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 13:33:42 +0200 Subject: [PATCH 10/16] Moved too concrete class from Common to Core --- .../Command/ShortUrl/ListShortUrlsCommand.php | 4 +- module/Common/src/Entity/AbstractEntity.php | 9 +---- .../Paginator/Util/PaginatorUtilsTrait.php | 6 --- .../PaginableRepositoryInterface.php | 34 ----------------- .../PaginableRepositoryAdapterTest.php | 37 ------------------- .../Adapter/ShortUrlRepositoryAdapter.php} | 18 ++++----- .../ShortUrlRepositoryInterface.php | 21 ++++++++++- module/Core/src/Service/ShortUrlService.php | 6 +-- .../Adapter/ShortUrlRepositoryAdapterTest.php | 37 +++++++++++++++++++ 9 files changed, 71 insertions(+), 101 deletions(-) delete mode 100644 module/Common/src/Repository/PaginableRepositoryInterface.php delete mode 100644 module/Common/test/Paginator/Adapter/PaginableRepositoryAdapterTest.php rename module/{Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php => Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php} (71%) create mode 100644 module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 329676c5..ba602fbc 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -5,9 +5,9 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; -use Shlinkio\Shlink\Common\Paginator\Adapter\PaginableRepositoryAdapter; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; +use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Symfony\Component\Console\Command\Command; @@ -62,7 +62,7 @@ class ListShortUrlsCommand extends Command 'page', 'p', InputOption::VALUE_OPTIONAL, - sprintf('The first page to list (%s items per page)', PaginableRepositoryAdapter::ITEMS_PER_PAGE), + sprintf('The first page to list (%s items per page)', ShortUrlRepositoryAdapter::ITEMS_PER_PAGE), '1' ) ->addOption( diff --git a/module/Common/src/Entity/AbstractEntity.php b/module/Common/src/Entity/AbstractEntity.php index 9358d2c5..dc3b84bc 100644 --- a/module/Common/src/Entity/AbstractEntity.php +++ b/module/Common/src/Entity/AbstractEntity.php @@ -3,16 +3,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Entity; -use Doctrine\ORM\Mapping as ORM; - abstract class AbstractEntity { - /** - * @var string - * @ORM\Id - * @ORM\GeneratedValue(strategy="IDENTITY") - * @ORM\Column(name="id", type="bigint", options={"unsigned"=true}) - */ + /** @var string */ protected $id; public function getId(): string diff --git a/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php b/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php index 7157a21d..2009164d 100644 --- a/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php +++ b/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php @@ -31,12 +31,6 @@ trait PaginatorUtilsTrait return $transformer === null ? $items : array_map([$transformer, 'transform'], $items); } - /** - * Checks if provided paginator is in last page - * - * @param Paginator $paginator - * @return bool - */ private function isLastPage(Paginator $paginator): bool { return $paginator->getCurrentPageNumber() >= $paginator->count(); diff --git a/module/Common/src/Repository/PaginableRepositoryInterface.php b/module/Common/src/Repository/PaginableRepositoryInterface.php deleted file mode 100644 index 7de324e9..00000000 --- a/module/Common/src/Repository/PaginableRepositoryInterface.php +++ /dev/null @@ -1,34 +0,0 @@ -repo = $this->prophesize(PaginableRepositoryInterface::class); - $this->adapter = new PaginableRepositoryAdapter($this->repo->reveal(), 'search', ['foo', 'bar'], 'order'); - } - - /** @test */ - public function getItemsFallbacksToFindList() - { - $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order')->shouldBeCalledOnce(); - $this->adapter->getItems(5, 10); - } - - /** @test */ - public function countFallbacksToCountList() - { - $this->repo->countList('search', ['foo', 'bar'])->shouldBeCalledOnce(); - $this->adapter->count(); - } -} diff --git a/module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php similarity index 71% rename from module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php rename to module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index 32511d52..d93063f0 100644 --- a/module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -1,20 +1,20 @@ paginableRepository = $paginableRepository; + $this->repository = $repository; $this->searchTerm = $searchTerm !== null ? trim(strip_tags($searchTerm)) : null; $this->orderBy = $orderBy; $this->tags = $tags; @@ -43,7 +43,7 @@ class PaginableRepositoryAdapter implements AdapterInterface */ public function getItems($offset, $itemCountPerPage): array { - return $this->paginableRepository->findList( + return $this->repository->findList( $itemCountPerPage, $offset, $this->searchTerm, @@ -63,6 +63,6 @@ class PaginableRepositoryAdapter implements AdapterInterface */ public function count(): int { - return $this->paginableRepository->countList($this->searchTerm, $this->tags); + return $this->repository->countList($this->searchTerm, $this->tags); } } diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index b1edab1b..fcec73e4 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -4,10 +4,27 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Common\Persistence\ObjectRepository; -use Shlinkio\Shlink\Common\Repository\PaginableRepositoryInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; -interface ShortUrlRepositoryInterface extends ObjectRepository, PaginableRepositoryInterface +interface ShortUrlRepositoryInterface extends ObjectRepository { + /** + * Gets a list of elements using provided filtering data + * + * @param string|array|null $orderBy + */ + public function findList( + ?int $limit = null, + ?int $offset = null, + ?string $searchTerm = null, + array $tags = [], + $orderBy = null + ): array; + + /** + * Counts the number of elements in a list using provided filtering data + */ + public function countList(?string $searchTerm = null, array $tags = []): int; + public function findOneByShortCode(string $shortCode): ?ShortUrl; } diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 1b95ea34..09ecdc10 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -4,10 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM; -use Shlinkio\Shlink\Common\Paginator\Adapter\PaginableRepositoryAdapter; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\FindShortCodeTrait; use Shlinkio\Shlink\Core\Util\TagManagerTrait; @@ -35,8 +35,8 @@ class ShortUrlService implements ShortUrlServiceInterface { /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); - $paginator = new Paginator(new PaginableRepositoryAdapter($repo, $searchQuery, $tags, $orderBy)); - $paginator->setItemCountPerPage(PaginableRepositoryAdapter::ITEMS_PER_PAGE) + $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $searchQuery, $tags, $orderBy)); + $paginator->setItemCountPerPage(ShortUrlRepositoryAdapter::ITEMS_PER_PAGE) ->setCurrentPageNumber($page); return $paginator; diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php new file mode 100644 index 00000000..4236fa69 --- /dev/null +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -0,0 +1,37 @@ +repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $this->adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), 'search', ['foo', 'bar'], 'order'); + } + + /** @test */ + public function getItemsFallbacksToFindList(): void + { + $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order')->shouldBeCalledOnce(); + $this->adapter->getItems(5, 10); + } + + /** @test */ + public function countFallbacksToCountList(): void + { + $this->repo->countList('search', ['foo', 'bar'])->shouldBeCalledOnce(); + $this->adapter->count(); + } +} From 334cc231dc92bbc01a48a505f841cd0326ce63ae Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 13:44:42 +0200 Subject: [PATCH 11/16] Final changes done on Common module --- module/Common/config/doctrine.config.php | 2 +- module/Common/src/{ => Doctrine}/Type/ChronosDateTimeType.php | 2 +- module/Common/test/Doctrine/EntityManagerFactoryTest.php | 2 +- .../test/{ => Doctrine}/Type/ChronosDateTimeTypeTest.php | 4 ++-- .../Shlinkio.Shlink.Core.Entity.ShortUrl.php | 2 +- .../entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php | 2 +- .../entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKey.php | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) rename module/Common/src/{ => Doctrine}/Type/ChronosDateTimeType.php (96%) rename module/Common/test/{ => Doctrine}/Type/ChronosDateTimeTypeTest.php (95%) diff --git a/module/Common/config/doctrine.config.php b/module/Common/config/doctrine.config.php index 0b569043..d85ef8e8 100644 --- a/module/Common/config/doctrine.config.php +++ b/module/Common/config/doctrine.config.php @@ -11,7 +11,7 @@ return [ 'entity_manager' => [ 'orm' => [ 'types' => [ - Type\ChronosDateTimeType::CHRONOS_DATETIME => Type\ChronosDateTimeType::class, + Doctrine\Type\ChronosDateTimeType::CHRONOS_DATETIME => Doctrine\Type\ChronosDateTimeType::class, ], ], ], diff --git a/module/Common/src/Type/ChronosDateTimeType.php b/module/Common/src/Doctrine/Type/ChronosDateTimeType.php similarity index 96% rename from module/Common/src/Type/ChronosDateTimeType.php rename to module/Common/src/Doctrine/Type/ChronosDateTimeType.php index a400106e..210eb03a 100644 --- a/module/Common/src/Type/ChronosDateTimeType.php +++ b/module/Common/src/Doctrine/Type/ChronosDateTimeType.php @@ -1,7 +1,7 @@ Date: Sun, 11 Aug 2019 13:47:42 +0200 Subject: [PATCH 12/16] Moved ResponseUtilsTrait to Response subnamespace --- module/Common/src/{Util => Response}/ResponseUtilsTrait.php | 2 +- module/Core/src/Action/PreviewAction.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename module/Common/src/{Util => Response}/ResponseUtilsTrait.php (95%) diff --git a/module/Common/src/Util/ResponseUtilsTrait.php b/module/Common/src/Response/ResponseUtilsTrait.php similarity index 95% rename from module/Common/src/Util/ResponseUtilsTrait.php rename to module/Common/src/Response/ResponseUtilsTrait.php index cc5a48a5..dee426fc 100644 --- a/module/Common/src/Util/ResponseUtilsTrait.php +++ b/module/Common/src/Response/ResponseUtilsTrait.php @@ -1,7 +1,7 @@ Date: Sun, 11 Aug 2019 13:54:21 +0200 Subject: [PATCH 13/16] Created SluggerFilterTest --- .../test/Validation/SluggerFilterTest.php | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 module/Common/test/Validation/SluggerFilterTest.php diff --git a/module/Common/test/Validation/SluggerFilterTest.php b/module/Common/test/Validation/SluggerFilterTest.php new file mode 100644 index 00000000..0c02982f --- /dev/null +++ b/module/Common/test/Validation/SluggerFilterTest.php @@ -0,0 +1,44 @@ +slugger = $this->prophesize(SlugifyInterface::class); + $this->filter = new SluggerFilter($this->slugger->reveal()); + } + + /** + * @test + * @dataProvider provideValuesToFilter + */ + public function providedValueIsFilteredAsExpected($providedValue, $expectedValue): void + { + $slugify = $this->slugger->slugify($providedValue)->willReturn('slug'); + + $result = $this->filter->filter($providedValue); + + $this->assertEquals($expectedValue, $result); + $slugify->shouldHaveBeenCalledTimes($expectedValue !== null ? 1 : 0); + } + + public function provideValuesToFilter(): iterable + { + yield 'null' => [null, null]; + yield 'empty string' => ['', null]; + yield 'not empty string' => ['foo', 'slug']; + } +} From 24e708b7e160a8009acebb0816b92b1f344b4e90 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 14:02:25 +0200 Subject: [PATCH 14/16] Removered registered options middleware --- module/Rest/config/dependencies.config.php | 2 ++ .../Rest/src/Middleware/ShortUrl/ShortCodePathMiddleware.php | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 27e86a35..e15788d2 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -7,6 +7,7 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Rest\Service\ApiKeyService; +use Zend\Expressive\Router\Middleware\ImplicitOptionsMiddleware; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Zend\ServiceManager\Factory\InvokableFactory; @@ -32,6 +33,7 @@ return [ Action\Tag\CreateTagsAction::class => ConfigAbstractFactory::class, Action\Tag\UpdateTagAction::class => ConfigAbstractFactory::class, + ImplicitOptionsMiddleware::class => Middleware\EmptyResponseImplicitOptionsMiddlewareFactory::class, Middleware\BodyParserMiddleware::class => InvokableFactory::class, Middleware\CrossDomainMiddleware::class => InvokableFactory::class, Middleware\PathVersionMiddleware::class => InvokableFactory::class, diff --git a/module/Rest/src/Middleware/ShortUrl/ShortCodePathMiddleware.php b/module/Rest/src/Middleware/ShortUrl/ShortCodePathMiddleware.php index 80045097..e79bcd7d 100644 --- a/module/Rest/src/Middleware/ShortUrl/ShortCodePathMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/ShortCodePathMiddleware.php @@ -10,9 +10,10 @@ use Psr\Http\Server\RequestHandlerInterface; use function str_replace; +/** @deprecated */ class ShortCodePathMiddleware implements MiddlewareInterface { - private const OLD_PATH_PREFIX = '/short-codes'; + private const OLD_PATH_PREFIX = '/short-codes'; // Old path is deprecated. Remove this middleware on v2 private const NEW_PATH_PREFIX = '/short-urls'; /** From 97a362617d99dedc04a7d2f8e63faa021cb01f18 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 14:21:35 +0200 Subject: [PATCH 15/16] Added new API test for Options requests --- .../test-api/Action/ListShortUrlsTest.php | 2 +- .../test-api/Action/OptionsRequestTest.php | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 module/Rest/test-api/Action/OptionsRequestTest.php diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 77966214..eb1e605b 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -8,7 +8,7 @@ use ShlinkioTest\Shlink\Common\ApiTest\ApiTestCase; class ListShortUrlsTest extends ApiTestCase { /** @test */ - public function shortUrlsAreProperlyListed() + public function shortUrlsAreProperlyListed(): void { $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls'); $respPayload = $this->getJsonResponsePayload($resp); diff --git a/module/Rest/test-api/Action/OptionsRequestTest.php b/module/Rest/test-api/Action/OptionsRequestTest.php new file mode 100644 index 00000000..121e9328 --- /dev/null +++ b/module/Rest/test-api/Action/OptionsRequestTest.php @@ -0,0 +1,31 @@ +callApi(self::METHOD_OPTIONS, '/short-urls'); + + $this->assertEquals(self::STATUS_NO_CONTENT, $resp->getStatusCode()); + $this->assertEmpty((string) $resp->getBody()); + } + + /** @test */ + public function optionsRequestsReturnAllowedMethodsForEndpoint(): void + { + $resp = $this->callApi(self::METHOD_OPTIONS, '/short-urls'); + $allowedMethods = $resp->getHeaderLine('Allow'); + + $this->assertEquals([ + self::METHOD_GET, + self::METHOD_POST, + ], explode(',', $allowedMethods)); + } +} From cb715c0877162158d7712af4776ad0dbb17077e9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2019 14:29:22 +0200 Subject: [PATCH 16/16] Decoupled Common module from any other module --- data/migrations/Version20180913205455.php | 4 ++-- module/Common/src/Util/IpAddress.php | 7 ++++--- module/Common/test-db/ApiTest/ApiTestCase.php | 3 +-- module/Core/src/Entity/Visit.php | 4 ++-- module/Rest/test-api/Action/OptionsRequestTest.php | 1 + 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/data/migrations/Version20180913205455.php b/data/migrations/Version20180913205455.php index 0b2b60d9..5de96361 100644 --- a/data/migrations/Version20180913205455.php +++ b/data/migrations/Version20180913205455.php @@ -7,8 +7,8 @@ use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Schema\Schema; use Doctrine\Migrations\AbstractMigration; use PDO; +use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\IpAddress; -use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; /** * Auto-generated Migration: Please modify to your needs! @@ -60,7 +60,7 @@ final class Version20180913205455 extends AbstractMigration try { return (string) IpAddress::fromString($addr)->getObfuscatedCopy(); - } catch (WrongIpException $e) { + } catch (InvalidArgumentException $e) { return null; } } diff --git a/module/Common/src/Util/IpAddress.php b/module/Common/src/Util/IpAddress.php index c237cd15..a71bac14 100644 --- a/module/Common/src/Util/IpAddress.php +++ b/module/Common/src/Util/IpAddress.php @@ -3,11 +3,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common\Util; -use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; +use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use function count; use function explode; use function implode; +use function sprintf; use function trim; final class IpAddress @@ -36,14 +37,14 @@ final class IpAddress /** * @param string $address * @return IpAddress - * @throws WrongIpException + * @throws InvalidArgumentException */ public static function fromString(string $address): self { $address = trim($address); $parts = explode('.', $address); if (count($parts) !== self::IPV4_PARTS_COUNT) { - throw WrongIpException::fromIpAddress($address); + throw new InvalidArgumentException(sprintf('Provided IP "%s" is invalid', $address)); } return new self(...$parts); diff --git a/module/Common/test-db/ApiTest/ApiTestCase.php b/module/Common/test-db/ApiTest/ApiTestCase.php index 1c6e3d0d..160ed2cc 100644 --- a/module/Common/test-db/ApiTest/ApiTestCase.php +++ b/module/Common/test-db/ApiTest/ApiTestCase.php @@ -9,7 +9,6 @@ use GuzzleHttp\ClientInterface; use GuzzleHttp\RequestOptions; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; -use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin; use function Shlinkio\Shlink\Common\json_decode; use function sprintf; @@ -48,7 +47,7 @@ abstract class ApiTestCase extends TestCase implements StatusCodeInterface, Requ protected function callApiWithKey(string $method, string $uri, array $options = []): ResponseInterface { $headers = $options[RequestOptions::HEADERS] ?? []; - $headers[ApiKeyHeaderPlugin::HEADER_NAME] = 'valid_api_key'; + $headers['X-Api-Key'] = 'valid_api_key'; $options[RequestOptions::HEADERS] = $headers; return $this->callApi($method, $uri, $options); diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index a6e87bad..f035f2ae 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -6,11 +6,11 @@ namespace Shlinkio\Shlink\Core\Entity; use Cake\Chronos\Chronos; use JsonSerializable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; -use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; class Visit extends AbstractEntity implements JsonSerializable { @@ -45,7 +45,7 @@ class Visit extends AbstractEntity implements JsonSerializable try { return (string) IpAddress::fromString($address)->getObfuscatedCopy(); - } catch (WrongIpException $e) { + } catch (InvalidArgumentException $e) { return null; } } diff --git a/module/Rest/test-api/Action/OptionsRequestTest.php b/module/Rest/test-api/Action/OptionsRequestTest.php index 121e9328..1be10ee0 100644 --- a/module/Rest/test-api/Action/OptionsRequestTest.php +++ b/module/Rest/test-api/Action/OptionsRequestTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; use ShlinkioTest\Shlink\Common\ApiTest\ApiTestCase; + use function explode; class OptionsRequestTest extends ApiTestCase