Merge pull request #79 from ackintosh/evict-outdated-memcahed-items

Fix: Memcached items keep growing
ackintosh · Dec 13, 2020 · a225863 · a225863
2 parents de17208 + 8e9983a
commit a225863
Show file tree

Hide file tree

Showing 17 changed files with 249 additions and 59 deletions.
diff --git a/src/Ganesha/Configuration.php b/src/Ganesha/Configuration.php
@@ -29,11 +29,6 @@ public function __construct($params)
         $this->params = $params;
     }
 
-    public function adapter(): AdapterInterface
-    {
-        return $this->params[self::ADAPTER];
-    }
-
     public function timeWindow(): int
     {
         return $this->params[self::TIME_WINDOW];
@@ -65,16 +60,17 @@ public function storageKeys(): StorageKeysInterface
     }
 
     /**
+     * @param array $params
      * @throws \InvalidArgumentException
      */
-    public function validate(): void
+    public static function validate(array $params): void
     {
-        if (isset($this->params[self::ADAPTER]) && !$this->params[self::ADAPTER] instanceof AdapterInterface) {
-            throw new \InvalidArgumentException(get_class($this->params[self::ADAPTER]) . ' should be an instance of AdapterInterface');
+        if (isset($params[self::ADAPTER]) && !$params[self::ADAPTER] instanceof AdapterInterface) {
+            throw new \InvalidArgumentException(get_class($params[self::ADAPTER]) . ' should be an instance of AdapterInterface');
         }
 
-        if (isset($this->params[self::STORAGE_KEYS]) && !$this->params[self::STORAGE_KEYS] instanceof StorageKeysInterface) {
-            throw new \InvalidArgumentException(get_class($this->params[self::STORAGE_KEYS]) . ' should be an instance of StorageKeysInterface');
+        if (isset($params[self::STORAGE_KEYS]) && !$params[self::STORAGE_KEYS] instanceof StorageKeysInterface) {
+            throw new \InvalidArgumentException(get_class($params[self::STORAGE_KEYS]) . ' should be an instance of StorageKeysInterface');
         }
 
         foreach ([
@@ -84,15 +80,15 @@ public function validate(): void
                 self::MINIMUM_REQUESTS,
                 self::INTERVAL_TO_HALF_OPEN
             ] as $name) {
-            if (isset($this->params[$name])) {
-                $v = $this->params[$name];
+            if (isset($params[$name])) {
+                $v = $params[$name];
                 if (!is_int($v) || $v < 1) {
                     throw new \InvalidArgumentException($name . ' should be an positive integer');
                 }
             }
         }
 
-        if (isset($this->params[self::FAILURE_RATE_THRESHOLD]) && $this->params[self::FAILURE_RATE_THRESHOLD] > 100) {
+        if (isset($params[self::FAILURE_RATE_THRESHOLD]) && $params[self::FAILURE_RATE_THRESHOLD] > 100) {
             throw new \InvalidArgumentException(self::FAILURE_RATE_THRESHOLD . ' should be equal or less than 100');
         }
     }

diff --git a/src/Ganesha/Context.php b/src/Ganesha/Context.php
@@ -0,0 +1,79 @@
+<?php
+namespace Ackintosh\Ganesha;
+
+
+use Ackintosh\Ganesha\Storage\Adapter\SlidingTimeWindowInterface;
+use Ackintosh\Ganesha\Storage\Adapter\TumblingTimeWindowInterface;
+use Ackintosh\Ganesha\Storage\AdapterInterface;
+use Ackintosh\Ganesha\Strategy\Count;
+use Ackintosh\Ganesha\Strategy\Rate;
+
+class Context
+{
+    /**
+     * @var string
+     */
+    const STRATEGY_COUNT = 'count';
+
+    /**
+     * @var string
+     */
+    const STRATEGY_RATE_TUMBLING_TIME_WINDOW = 'rate_tumbling_time_window';
+
+    /**
+     * @var string
+     */
+    const STRATEGY_RATE_SLIDING_TIME_WINDOW = 'rate_sliding_time_window';
+
+    /**
+     * @var string
+     */
+    private $strategy;
+
+    /**
+     * @var Configuration
+     */
+    private $configuration;
+
+    /**
+     * @param string $strategyClass
+     * @param AdapterInterface $adapter
+     * @param Configuration $configuration
+     */
+    public function __construct(string $strategyClass, AdapterInterface $adapter, Configuration $configuration)
+    {
+        $this->strategy = $this->determineStrategyContext($strategyClass, $adapter);
+        $this->configuration = $configuration;
+    }
+
+    public function strategy(): string
+    {
+        return $this->strategy;
+    }
+
+    public function configuration(): Configuration
+    {
+        return $this->configuration;
+    }
+
+    private function determineStrategyContext(string $strategyClass, AdapterInterface $adapter): string
+    {
+        switch ($strategyClass) {
+            case Count::class:
+                return self::STRATEGY_COUNT;
+                break;
+            case Rate::class:
+                if ($adapter instanceof SlidingTimeWindowInterface) {
+                    return self::STRATEGY_RATE_SLIDING_TIME_WINDOW;
+                } elseif ($adapter instanceof TumblingTimeWindowInterface) {
+                    return self::STRATEGY_RATE_TUMBLING_TIME_WINDOW;
+                } else {
+                    throw new \InvalidArgumentException('Adapter should implement SlidingTimeWindowInterface or TumblingTimeWindowInterface');
+                }
+                break;
+            default:
+                throw new \InvalidArgumentException('Unknown strategy class name: ' . $strategyClass);
+                break;
+        }
+    }
+}
diff --git a/src/Ganesha/Storage/Adapter/Apcu.php b/src/Ganesha/Storage/Adapter/Apcu.php
@@ -42,12 +42,12 @@ public function supportRateStrategy(): bool
     }
 
     /**
-     * @param Configuration $configuration
+     * @param Ganesha\Context $context
      * @return void
      */
-    public function setConfiguration(Configuration $configuration): void
+    public function setContext(Ganesha\Context $context): void
     {
-        $this->storageKeys = $configuration->storageKeys();
+        $this->storageKeys = $context->configuration()->storageKeys();
     }
 
     /**

diff --git a/src/Ganesha/Storage/Adapter/Memcached.php b/src/Ganesha/Storage/Adapter/Memcached.php
@@ -15,6 +15,11 @@ class Memcached implements AdapterInterface, TumblingTimeWindowInterface
      */
     private $memcached;
 
+    /**
+     * @var Ganesha\Context
+     */
+    private $context;
+
     /**
      * Memcached constructor.
      * @param \Memcached $memcached
@@ -43,13 +48,13 @@ public function supportRateStrategy(): bool
     }
 
     /**
-     * @param Configuration $configuration
+     * @param Ganesha\Context $context
      * @return void
      * @codeCoverageIgnore
      */
-    public function setConfiguration(Configuration $configuration): void
+    public function setContext(Ganesha\Context $context): void
     {
-        // This adapter doesn't use the configuration.
+        $this->context = $context;
     }
 
     /**
@@ -84,8 +89,14 @@ public function save(string $service, int $count): void
      */
     public function increment(string $service): void
     {
+        $expiry = 0;
+        if ($this->context->strategy() === Ganesha\Context::STRATEGY_RATE_TUMBLING_TIME_WINDOW) {
+            // Set the expiry time to make ensure outdated items of TumblingTimeWindow will be removed.
+            $expiry = $this->context->configuration()->timeWindow() * 10;
+        }
+
         // requires \Memcached::OPT_BINARY_PROTOCOL
-        if ($this->memcached->increment($service, 1, 1) === false) {
+        if ($this->memcached->increment($service, 1, 1, $expiry) === false) {
             throw new StorageException('failed to increment failure count : ' . $this->memcached->getResultMessage());
         }
     }

diff --git a/src/Ganesha/Storage/Adapter/MongoDB.php b/src/Ganesha/Storage/Adapter/MongoDB.php
@@ -55,14 +55,13 @@ public function supportRateStrategy(): bool
     }
 
     /**
-     * @param Configuration $configuration
+     * @param Ganesha\Context $context
      * @return void
-     * @throws \Exception
      * @codeCoverageIgnore
      */
-    public function setConfiguration(Configuration $configuration): void
+    public function setContext(Ganesha\Context $context): void
     {
-        // This adapter doesn't use the configuration.
+        // This adapter doesn't use the context.
     }
 
     /**

diff --git a/src/Ganesha/Storage/Adapter/Redis.php b/src/Ganesha/Storage/Adapter/Redis.php
@@ -48,13 +48,12 @@ public function supportRateStrategy(): bool
     }
 
     /**
-     * @param Configuration $configuration
-     *
+     * @param Ganesha\Context $context
      * @return void
      */
-    public function setConfiguration(Configuration $configuration): void
+    public function setContext(Ganesha\Context $context): void
     {
-        $this->configuration = $configuration;
+        $this->configuration = $context->configuration();
     }
 
     /**

diff --git a/src/Ganesha/Storage/AdapterInterface.php b/src/Ganesha/Storage/AdapterInterface.php
@@ -1,7 +1,7 @@
 <?php
 namespace Ackintosh\Ganesha\Storage;
 
-use Ackintosh\Ganesha\Configuration;
+use Ackintosh\Ganesha\Context;
 
 interface AdapterInterface
 {
@@ -18,10 +18,10 @@ public function supportCountStrategy(): bool ;
     public function supportRateStrategy(): bool ;
 
     /**
-     * @param Configuration $configuration
+     * @param Context $context
      * @return void
      */
-    public function setConfiguration(Configuration $configuration): void;
+    public function setContext(Context $context): void;
 
     /**
      * @param  string $service

diff --git a/src/Ganesha/Strategy/Count.php b/src/Ganesha/Strategy/Count.php
@@ -30,15 +30,16 @@ private function __construct(Configuration $configuration, Storage $storage)
     }
 
     /**
+     * @param Storage\AdapterInterface $adapter
      * @param Configuration $configuration
      * @return Count
      */
-    public static function create(Configuration $configuration): StrategyInterface
+    public static function create(Storage\AdapterInterface $adapter, Configuration $configuration): StrategyInterface
     {
         return new self(
             $configuration,
             new Storage(
-                $configuration->adapter(),
+                $adapter,
                 $configuration->storageKeys(),
                 null
             )

diff --git a/src/Ganesha/Strategy/Count/Builder.php b/src/Ganesha/Strategy/Count/Builder.php
@@ -20,7 +20,7 @@ class Builder
     ];
 
     /** @var string */
-    private static $strategyClass = '\Ackintosh\Ganesha\Strategy\Count';
+    private static $strategyClass = 'Ackintosh\Ganesha\Strategy\Count';
 
     /** @var string */
     private static $adapterRequirement = 'supportCountStrategy';
@@ -64,4 +64,4 @@ public function storageKeys(Ganesha\Storage\StorageKeysInterface $storageKeys):
         $this->params[Configuration::STORAGE_KEYS] = $storageKeys;
         return $this;
     }
-}
+}
diff --git a/src/Ganesha/Strategy/Rate.php b/src/Ganesha/Strategy/Rate.php
@@ -59,13 +59,12 @@ public static function validate(array $params): void
     }
 
     /**
+     * @param Storage\AdapterInterface $adapter
      * @param Configuration $configuration
      * @return Rate
      */
-    public static function create(Configuration $configuration): StrategyInterface
+    public static function create(Storage\AdapterInterface $adapter, Configuration $configuration): StrategyInterface
     {
-        $adapter = $configuration->adapter();
-        $adapter->setConfiguration($configuration);
         $serviceNameDecorator = $adapter instanceof Storage\Adapter\TumblingTimeWindowInterface ? self::serviceNameDecorator($configuration->timeWindow()) : null;
 
         return new self(
@@ -141,7 +140,7 @@ public function isAvailable(string $service): bool
     /**
      * @param  string $service
      * @return bool
-     * @throws StorageException 
+     * @throws StorageException
      * @throws \LogicException
      */
     private function isClosed(string $service): bool

diff --git a/src/Ganesha/Strategy/Rate/Builder.php b/src/Ganesha/Strategy/Rate/Builder.php
@@ -25,7 +25,7 @@ class Builder
     ];
 
     /** @var string */
-    private static $strategyClass = '\Ackintosh\Ganesha\Strategy\Rate';
+    private static $strategyClass = 'Ackintosh\Ganesha\Strategy\Rate';
 
     /** @var string */
     private static $adapterRequirement = 'supportRateStrategy';
@@ -89,4 +89,4 @@ public function timeWindow(int $timeWindow): self
         $this->params[Configuration::TIME_WINDOW] = $timeWindow;
         return $this;
     }
-}
+}
diff --git a/src/Ganesha/StrategyInterface.php b/src/Ganesha/StrategyInterface.php
@@ -1,15 +1,17 @@
 <?php
 namespace Ackintosh\Ganesha;
 
+use Ackintosh\Ganesha\Storage\AdapterInterface;
 use LogicException;
 
 interface StrategyInterface
 {
     /**
+     * @param AdapterInterface $adapter
      * @param Configuration $configuration
      * @return mixed
      */
-    public static function create(Configuration $configuration): StrategyInterface;
+    public static function create(AdapterInterface $adapter, Configuration $configuration): StrategyInterface;
 
     /**
      * @param string $service

diff --git a/src/Ganesha/Traits/BuildGanesha.php b/src/Ganesha/Traits/BuildGanesha.php
@@ -30,10 +30,17 @@ public function build(): Ganesha
     {
         // Strategy specific validation
         $this->validate();
+        // Validate the params
+        Configuration::validate($this->params);
+
+        // Unset `adapter` key from configuration params to avoid circular reference.
+        $adapter = $this->params[Configuration::ADAPTER];
+        unset($this->params[Configuration::ADAPTER]);
 
         $configuration = new Configuration($this->params);
-        $configuration->validate();
+        $context = new Ganesha\Context(self::$strategyClass, $adapter, $configuration);
+        $adapter->setContext($context);
 
-        return new Ganesha(self::$strategyClass::create($configuration));
+        return new Ganesha(self::$strategyClass::create($adapter, $configuration));
     }
-}
+}