From a562bc661d0a20749f423ff727bcc9dd79ddf728 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Thu, 6 Dec 2018 21:05:11 +0100
Subject: [PATCH] Improved CacheFactory class

---
 module/Common/src/Factory/CacheFactory.php | 42 ++++++++++------
 module/Core/src/Options/AppOptions.php     | 58 ++++++----------------
 2 files changed, 42 insertions(+), 58 deletions(-)

diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php
index bd0eea3f..2efde447 100644
--- a/module/Common/src/Factory/CacheFactory.php
+++ b/module/Common/src/Factory/CacheFactory.php
@@ -13,6 +13,7 @@ use Zend\ServiceManager\Exception\ServiceNotFoundException;
 use Zend\ServiceManager\Factory\FactoryInterface;
 use function Functional\contains;
 use function Shlinkio\Shlink\Common\env;
+use function sys_get_temp_dir;
 
 class CacheFactory implements FactoryInterface
 {
@@ -23,6 +24,7 @@ class CacheFactory implements FactoryInterface
         Cache\PhpFileCache::class,
         Cache\MemcachedCache::class,
     ];
+    private const DEFAULT_MEMCACHED_PORT = 11211;
 
     /**
      * Create an object
@@ -40,7 +42,7 @@ class CacheFactory implements FactoryInterface
     {
         $appOptions = $container->get(AppOptions::class);
         $adapter = $this->getAdapter($container);
-        $adapter->setNamespace($appOptions->__toString());
+        $adapter->setNamespace((string) $appOptions);
 
         return $adapter;
     }
@@ -65,25 +67,35 @@ class CacheFactory implements FactoryInterface
                 return new $cacheConfig['adapter']();
             case Cache\FilesystemCache::class:
             case Cache\PhpFileCache::class:
-                return new $cacheConfig['adapter']($cacheConfig['options']['dir']);
+                return new $cacheConfig['adapter']($cacheConfig['options']['dir'] ?? sys_get_temp_dir());
             case Cache\MemcachedCache::class:
-                $memcached = new Memcached();
-                $servers = $cacheConfig['options']['servers'] ?? [];
-
-                foreach ($servers as $server) {
-                    if (! isset($server['host'])) {
-                        continue;
-                    }
-                    $port = isset($server['port']) ? (int) $server['port'] : 11211;
-
-                    $memcached->addServer($server['host'], $port);
-                }
-
                 $cache = new Cache\MemcachedCache();
-                $cache->setMemcached($memcached);
+                $cache->setMemcached($this->buildMemcached($cacheConfig));
                 return $cache;
             default:
                 return new Cache\ArrayCache();
         }
     }
+
+    private function buildMemcached(array $cacheConfig): Memcached
+    {
+        $memcached = new Memcached();
+        $servers = $cacheConfig['options']['servers'] ?? [];
+
+        foreach ($servers as $server) {
+            $this->addMemcachedServer($memcached, $server);
+        }
+
+        return $memcached;
+    }
+
+    private function addMemcachedServer(Memcached $memcached, array $server): void
+    {
+        if (! isset($server['host'])) {
+            return;
+        }
+        $port = (int) ($server['port'] ?? self::DEFAULT_MEMCACHED_PORT);
+
+        $memcached->addServer($server['host'], $port);
+    }
 }
diff --git a/module/Core/src/Options/AppOptions.php b/module/Core/src/Options/AppOptions.php
index b448b8c0..341d5df5 100644
--- a/module/Core/src/Options/AppOptions.php
+++ b/module/Core/src/Options/AppOptions.php
@@ -15,69 +15,48 @@ class AppOptions extends AbstractOptions
     private $name = '';
     /** @var string */
     private $version = '1.0';
-    /** @var string */
+    /**
+     * @var string
+     * @deprecated
+     */
     private $secretKey = '';
     /** @var string|null */
     private $disableTrackParam;
 
-    /**
-     * AppOptions constructor.
-     * @param array|null|\Traversable $options
-     */
-    public function __construct($options = null)
-    {
-        parent::__construct($options);
-    }
-
-    /**
-     * @return string
-     */
-    public function getName()
+    public function getName(): string
     {
         return $this->name;
     }
 
-    /**
-     * @param string $name
-     * @return $this
-     */
-    protected function setName($name)
+    protected function setName(string $name): self
     {
         $this->name = $name;
         return $this;
     }
 
-    /**
-     * @return string
-     */
-    public function getVersion()
+    public function getVersion(): string
     {
         return $this->version;
     }
 
-    /**
-     * @param string $version
-     * @return $this
-     */
-    protected function setVersion($version)
+    protected function setVersion(string $version): self
     {
         $this->version = $version;
         return $this;
     }
 
     /**
-     * @return mixed
+     * @deprecated
      */
-    public function getSecretKey()
+    public function getSecretKey(): string
     {
         return $this->secretKey;
     }
 
     /**
-     * @param mixed $secretKey
-     * @return $this
+     * @deprecated
      */
-    protected function setSecretKey($secretKey)
+    protected function setSecretKey(string $secretKey): self
     {
         $this->secretKey = $secretKey;
         return $this;
@@ -86,25 +65,18 @@ class AppOptions extends AbstractOptions
     /**
      * @return string|null
      */
-    public function getDisableTrackParam()
+    public function getDisableTrackParam(): ?string
     {
         return $this->disableTrackParam;
     }
 
-    /**
-     * @param string|null $disableTrackParam
-     * @return $this|self
-     */
-    protected function setDisableTrackParam($disableTrackParam): self
+    protected function setDisableTrackParam(?string $disableTrackParam): self
     {
         $this->disableTrackParam = $disableTrackParam;
         return $this;
     }
 
-    /**
-     * @return string
-     */
-    public function __toString()
+    public function __toString(): string
     {
         return sprintf('%s:v%s', $this->name, $this->version);
     }