Merge pull request #445 from acelaya/feature/redis-missings

Feature/redis missings

config/autoload/locks.global.php

@@ -2,6 +2,7 @@
 declare(strict_types=1);
 
 use Shlinkio\Shlink\Common\Cache\RedisFactory;
+use Shlinkio\Shlink\Common\Lock\RetryLockStoreDelegatorFactory;
 use Symfony\Component\Lock;
 use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 
@@ -22,6 +23,11 @@ return [
             'lock_store' => Lock\Store\FlockStore::class,
             'redis_lock_store' => Lock\Store\RedisStore::class,
         ],
+        'delegators' => [
+            Lock\Store\RedisStore::class => [
+                RetryLockStoreDelegatorFactory::class,
+            ],
+        ],
     ],
 
     ConfigAbstractFactory::class => [

config/config.php

@@ -26,5 +26,5 @@ return (new ConfigAggregator\ConfigAggregator([
         ? new ConfigAggregator\PhpFileProvider('config/test/*.global.php')
         : new ConfigAggregator\ZendConfigProvider('config/params/{generated_config.php,*.config.{php,json}}'),
 ], 'data/cache/app_config.php', [
-    Core\ConfigPostProcessor::class,
+    Core\SimplifiedConfigParser::class,
 ]))->getMergedConfig();

module/Common/src/Cache/RedisFactory.php

@@ -6,9 +6,8 @@ namespace Shlinkio\Shlink\Common\Cache;
 use Predis\Client as PredisClient;
 use Psr\Container\ContainerInterface;
 
-use function array_shift;
 use function count;
-use function is_array;
+use function explode;
 use function is_string;
 
 class RedisFactory
@@ -18,13 +17,11 @@ class RedisFactory
     public function __invoke(ContainerInterface $container): PredisClient
     {
         $redisConfig = $container->get('config')['redis'] ?? [];
+
         $servers = $redisConfig['servers'] ?? [];
+        $servers = is_string($servers) ? explode(',', $servers) : $servers;
+        $options = count($servers) <= 1 ? null : ['cluster' => 'redis'];
 
-        if (is_array($servers) && count($servers) === 1) {
-            $servers = array_shift($servers);
-        }
-
-        $options = is_string($servers) || count($servers) < 1 ? null : ['cluster' => 'redis'];
         return new PredisClient($servers, $options);
     }
 }

module/Common/src/Lock/RetryLockStoreDelegatorFactory.php

@@ -0,0 +1,18 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common\Lock;
+
+use Interop\Container\ContainerInterface;
+use Symfony\Component\Lock\Store\RetryTillSaveStore;
+use Symfony\Component\Lock\StoreInterface;
+
+class RetryLockStoreDelegatorFactory
+{
+    public function __invoke(ContainerInterface $container, $name, callable $callback): RetryTillSaveStore
+    {
+        /** @var StoreInterface $originalStore */
+        $originalStore = $callback();
+        return new RetryTillSaveStore($originalStore);
+    }
+}

module/Common/test/Cache/RedisFactoryTest.php

@@ -54,5 +54,8 @@ class RedisFactoryTest extends TestCase
         yield 'empty cluster of servers' => [[
             'servers' => [],
         ], PredisCluster::class];
+        yield 'cluster of servers as string' => [[
+            'servers' => 'tcp://1.1.1.1:6379,tcp://2.2.2.2:6379',
+        ], RedisCluster::class];
     }
 }

module/Common/test/Lock/RetryLockStoreDelegatorFactoryTest.php

@@ -0,0 +1,41 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\Lock;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\Prophecy\ObjectProphecy;
+use ReflectionObject;
+use Shlinkio\Shlink\Common\Lock\RetryLockStoreDelegatorFactory;
+use Symfony\Component\Lock\StoreInterface;
+use Zend\ServiceManager\ServiceManager;
+
+class RetryLockStoreDelegatorFactoryTest extends TestCase
+{
+    /** @var RetryLockStoreDelegatorFactory */
+    private $delegator;
+    /** @var ObjectProphecy */
+    private $originalStore;
+
+    public function setUp(): void
+    {
+        $this->originalStore = $this->prophesize(StoreInterface::class)->reveal();
+        $this->delegator = new RetryLockStoreDelegatorFactory();
+    }
+
+    /** @test */
+    public function originalStoreIsWrappedInRetryStore(): void
+    {
+        $callback = function () {
+            return $this->originalStore;
+        };
+
+        $result = ($this->delegator)(new ServiceManager(), '', $callback);
+
+        $ref = new ReflectionObject($result);
+        $prop = $ref->getProperty('decorated');
+        $prop->setAccessible(true);
+
+        $this->assertSame($this->originalStore, $prop->getValue($result));
+    }
+}

module/Core/src/SimplifiedConfigParser.php

@@ -11,7 +11,7 @@ use function array_key_exists;
 use function Functional\contains;
 use function Functional\reduce_left;
 
-class ConfigPostProcessor
+class SimplifiedConfigParser
 {
     private const SIMPLIFIED_CONFIG_MAPPING = [
         'disable_track_param' => ['app_options', 'disable_track_param'],
@@ -22,11 +22,21 @@ class ConfigPostProcessor
         'db_config' => ['entity_manager', 'connection'],
         'delete_short_url_threshold' => ['delete_short_urls', 'visits_threshold'],
         'locale' => ['translator', 'locale'],
-        'lock_store' => ['dependencies', 'aliases', 'lock_store'],
+        'redis_servers' => ['redis', 'servers'],
     ];
-    private const SIMPLIFIED_CONFIG_TOGGLES = [
-        'not_found_redirect_to' => ['url_shortener', 'not_found_short_url', 'enable_redirection'],
-        'delete_short_url_threshold' => ['delete_short_urls', 'check_visits_threshold'],
+    private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [
+        'not_found_redirect_to' => [
+            'path' => ['url_shortener', 'not_found_short_url', 'enable_redirection'],
+            'value' => true,
+        ],
+        'delete_short_url_threshold' => [
+            'path' => ['delete_short_urls', 'check_visits_threshold'],
+            'value' => true,
+        ],
+        'redis_servers' => [
+            'path' => ['dependencies', 'aliases', 'lock_store'],
+            'value' => 'redis_lock_store',
+        ],
     ];
     private const SIMPLIFIED_MERGEABLE_CONFIG = ['db_config'];
 
@@ -41,8 +51,9 @@ class ConfigPostProcessor
             }
 
             $collection->setValueInPath($value, $path);
-            if (array_key_exists($key, self::SIMPLIFIED_CONFIG_TOGGLES)) {
-                $collection->setValueInPath(true, self::SIMPLIFIED_CONFIG_TOGGLES[$key]);
+            if (array_key_exists($key, self::SIMPLIFIED_CONFIG_SIDE_EFFECTS)) {
+                ['path' => $sideEffectPath, 'value' => $sideEffectValue] = self::SIMPLIFIED_CONFIG_SIDE_EFFECTS[$key];
+                $collection->setValueInPath($sideEffectValue, $sideEffectPath);
             }
 
             return $collection;

module/Core/test/SimplifiedConfigParserTest.php

@@ -4,17 +4,17 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Core;
 
 use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\Core\ConfigPostProcessor;
+use Shlinkio\Shlink\Core\SimplifiedConfigParser;
 
 use function array_merge;
 
-class ConfigPostProcessorTest extends TestCase
+class SimplifiedConfigParserTest extends TestCase
 {
     private $postProcessor;
 
     public function setUp(): void
     {
-        $this->postProcessor = new ConfigPostProcessor();
+        $this->postProcessor = new SimplifiedConfigParser();
     }
 
     /** @test */
@@ -41,7 +41,10 @@ class ConfigPostProcessorTest extends TestCase
             'delete_short_url_threshold' => 50,
             'locale' => 'es',
             'not_found_redirect_to' => 'foobar.com',
-            'lock_store' => 'redis_lock_store',
+            'redis_servers' => [
+                'tcp://1.1.1.1:1111',
+                'tcp://1.2.2.2:2222',
+            ],
             'db_config' => [
                 'dbname' => 'shlink',
                 'user' => 'foo',
@@ -91,6 +94,13 @@ class ConfigPostProcessorTest extends TestCase
                     'lock_store' => 'redis_lock_store',
                 ],
             ],
+
+            'redis' => [
+                'servers' => [
+                    'tcp://1.1.1.1:1111',
+                    'tcp://1.2.2.2:2222',
+                ],
+            ],
         ];
 
         $result = ($this->postProcessor)(array_merge($config, $simplified));