Refactoring

stfndamjanovic · Jan 12, 2024 · df6d5c3 · df6d5c3
1 parent baf2b41
commit df6d5c3
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 80 deletions.
diff --git a/README.md b/README.md
@@ -20,12 +20,9 @@ composer require stfndamjanovic/php-circuit-breaker
 use Stfn\CircuitBreaker\CircuitBreaker;
 use Stfn\CircuitBreaker\Exceptions\CircuitHalfOpenFailException;
 
-$factory = CircuitBreaker::factory()
-        ->for('3rd-party-api')
+$factory = CircuitBreaker::for('3rd-party-service')
         ->failWhen(function ($result) {
-            if ($result->status > 400) {
-                throw new Exception();
-            }
+            return $result->status > 400;
         })
         ->skipFailure(function (Exception $exception) {
             return $exception instanceof CircuitHalfOpenFailException;

diff --git a/src/CircuitBreaker.php b/src/CircuitBreaker.php
@@ -12,6 +12,7 @@
 
 class CircuitBreaker
 {
+    public string $name;
     /**
      * @var Config
      */
@@ -41,8 +42,9 @@ class CircuitBreaker
      * @param Config|null $config
      * @param CircuitBreakerStorage|null $storage
      */
-    public function __construct(Config $config = null, CircuitBreakerStorage $storage = null)
+    public function __construct(string $name, Config $config = null, CircuitBreakerStorage $storage = null)
     {
+        $this->name = $name;
         $this->config = $config ?: new Config();
         $this->storage = $storage ?: new InMemoryStorage();
     }
@@ -55,6 +57,8 @@ public function __construct(Config $config = null, CircuitBreakerStorage $storag
      */
     public function call(\Closure $action, ...$args)
     {
+        $this->storage->init($this->name);
+
         /** @var StateHandler $stateHandler */
         $stateHandler = $this->makeStateHandler();
 
@@ -116,10 +120,11 @@ public function addListener(CircuitBreakerListener $listener)
     }
 
     /**
+     * @param string $service
      * @return CircuitBreakerFactory
      */
-    public static function factory()
+    public static function for(string $service)
     {
-        return new CircuitBreakerFactory(new self());
+        return new CircuitBreakerFactory(new self($service));
     }
 }
diff --git a/src/CircuitBreakerFactory.php b/src/CircuitBreakerFactory.php
@@ -15,7 +15,7 @@ public function __construct(CircuitBreaker $breaker)
 
     public function for(string $service)
     {
-        $this->breaker->storage->setService($service);
+        $this->breaker->config->name = $service;
 
         return $this;
     }
@@ -61,4 +61,9 @@ public function call(\Closure $action, ...$args)
     {
         return $this->breaker->call($action, $args);
     }
+
+    public function getCircuitBreaker()
+    {
+        return $this->breaker;
+    }
 }
diff --git a/src/Exceptions/FailOnSuccessException.php b/src/Exceptions/FailOnSuccessException.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace Stfn\CircuitBreaker\Exceptions;
+
+class FailOnSuccessException extends \Exception
+{
+    public static function make()
+    {
+        return new self("Circuit breaker failed on success.");
+    }
+}
diff --git a/src/StateHandlers/ClosedStateHandler.php b/src/StateHandlers/ClosedStateHandler.php
@@ -22,7 +22,7 @@ public function onFailure(\Exception $exception)
         if ($failuresCount >= $this->breaker->config->failureThreshold) {
             $this->breaker->openCircuit();
 
-            throw CircuitOpenException::make($storage->getService());
+            throw CircuitOpenException::make($this->breaker->name);
         }
     }
 }
diff --git a/src/StateHandlers/OpenStateHandler.php b/src/StateHandlers/OpenStateHandler.php
@@ -27,6 +27,6 @@ public function beforeCall(\Closure $action, ...$args)
             return;
         }
 
-        throw CircuitOpenException::make($storage->getService());
+        throw CircuitOpenException::make($this->breaker->name);
     }
 }
diff --git a/src/StateHandlers/StateHandler.php b/src/StateHandlers/StateHandler.php
@@ -3,6 +3,8 @@
 namespace Stfn\CircuitBreaker\StateHandlers;
 
 use Stfn\CircuitBreaker\CircuitBreaker;
+use Stfn\CircuitBreaker\Exceptions\FailOnSuccessException;
+use Stfn\CircuitBreaker\Exceptions\FailWithoutException;
 
 class StateHandler
 {
@@ -76,12 +78,18 @@ public function handleFailure(\Exception $exception)
     }
 
     /**
+     * @param $result
      * @return void
+     * @throws FailOnSuccessException
      */
     public function handleSucess($result)
     {
         if (is_callable($this->breaker->failWhenCallback)) {
-            call_user_func($this->breaker->failWhenCallback, $result);
+            $shouldFail = call_user_func($this->breaker->failWhenCallback, $result);
+
+            if ($shouldFail) {
+                throw FailOnSuccessException::make();
+            }
         }
 
         $this->onSucess();

diff --git a/src/Storage/CircuitBreakerStorage.php b/src/Storage/CircuitBreakerStorage.php
@@ -9,15 +9,7 @@ abstract class CircuitBreakerStorage
     /**
      * @var string
      */
-    protected string $service;
-
-    /**
-     * @param string $service
-     */
-    public function __construct(string $service)
-    {
-        $this->service = $service;
-    }
+    protected string $service = '';
 
     /**
      * @return string
@@ -27,15 +19,6 @@ public function getService(): string
         return $this->service;
     }
 
-    /**
-     * @param string $service
-     * @return void
-     */
-    public function setService(string $service)
-    {
-        $this->service = $service;
-    }
-
     /**
      * @return CircuitState
      */
@@ -47,6 +30,15 @@ abstract public function getState(): CircuitState;
      */
     abstract public function setState(CircuitState $state): void;
 
+    /**
+     * @param string $service
+     * @return void
+     */
+    public function init(string $service): void
+    {
+
+    }
+
     /**
      * @return void
      */

diff --git a/src/Storage/InMemoryStorage.php b/src/Storage/InMemoryStorage.php
@@ -19,17 +19,7 @@ class InMemoryStorage extends CircuitBreakerStorage
     /**
      * @var int|null
      */
-    protected null|int $openedAt;
-
-    /**
-     *
-     */
-    public function __construct()
-    {
-        parent::__construct("in_memory");
-
-        $this->openedAt = null;
-    }
+    protected null|int $openedAt = null;
 
     /**
      * @return CircuitState

diff --git a/src/Storage/RedisStorage.php b/src/Storage/RedisStorage.php
@@ -21,21 +21,20 @@ class RedisStorage extends CircuitBreakerStorage
      * @param \Redis $redis
      * @throws \RedisException
      */
-    public function __construct(\Redis $redis, string $service)
+    public function __construct(\Redis $redis)
     {
-        parent::__construct($service);
-
         $this->redis = $redis;
-
-        $this->initState();
     }
 
     /**
+     * @param string $service
      * @return void
      * @throws \RedisException
      */
-    protected function initState()
+    public function init(string $service): void
     {
+        $this->service = $service;
+
         $this->redis->setnx($this->getNamespace(self::STATE_KEY), CircuitState::Closed->value);
         $this->redis->setnx($this->getNamespace(self::FAIL_COUNT_KEY), 0);
     }

diff --git a/tests/CircuitBreakerTest.php b/tests/CircuitBreakerTest.php
@@ -6,15 +6,14 @@
 use Stfn\CircuitBreaker\CircuitBreaker;
 use Stfn\CircuitBreaker\CircuitBreakerListener;
 use Stfn\CircuitBreaker\CircuitState;
-use Stfn\CircuitBreaker\Config;
 use Stfn\CircuitBreaker\Exceptions\CircuitHalfOpenFailException;
 use Stfn\CircuitBreaker\Exceptions\CircuitOpenException;
 
 class CircuitBreakerTest extends TestCase
 {
     public function test_if_it_can_handle_function_success()
     {
-        $breaker = new CircuitBreaker();
+        $breaker = CircuitBreaker::for('test-service')->getCircuitBreaker();
 
         $result = $breaker->call(function () {
             return true;
@@ -33,7 +32,7 @@ public function test_if_it_can_handle_function_success()
 
     public function test_if_it_will_throw_an_exception_if_circuit_breaker_is_open()
     {
-        $breaker = new CircuitBreaker();
+        $breaker = CircuitBreaker::for('test-service')->getCircuitBreaker();
         $breaker->openCircuit();
 
         $this->expectException(CircuitOpenException::class);
@@ -45,7 +44,7 @@ public function test_if_it_will_throw_an_exception_if_circuit_breaker_is_open()
 
     public function test_if_it_will_record_every_success()
     {
-        $breaker = new CircuitBreaker();
+        $breaker = CircuitBreaker::for('test-service')->getCircuitBreaker();
 
         $success = function () {
             return true;
@@ -62,11 +61,9 @@ public function test_if_it_will_record_every_success()
 
     public function test_if_it_will_record_every_failure()
     {
-        $config = Config::make([
-            'failure_threshold' => 4,
-        ]);
-
-        $breaker = new CircuitBreaker($config);
+        $breaker = CircuitBreaker::for('test-service')
+            ->withOptions(['failure_threshold' => 4])
+            ->getCircuitBreaker();
 
         $fail = function () {
             throw new \Exception("test");
@@ -87,11 +84,9 @@ public function test_if_it_will_record_every_failure()
 
     public function test_if_it_will_open_circuit_after_failure_threshold()
     {
-        $config = Config::make([
-            'failure_threshold' => 3,
-        ]);
-
-        $breaker = new CircuitBreaker($config);
+        $breaker = CircuitBreaker::for('test-service')
+            ->withOptions(['failure_threshold' => 3])
+            ->getCircuitBreaker();
 
         $fail = function () {
             throw new \Exception();
@@ -102,7 +97,7 @@ public function test_if_it_will_open_circuit_after_failure_threshold()
         foreach (range(1, $tries) as $i) {
             try {
                 $breaker->call($fail);
-            } catch (\Exception $exception) {
+            } catch (\Exception) {
 
             }
         }
@@ -112,11 +107,9 @@ public function test_if_it_will_open_circuit_after_failure_threshold()
 
     public function test_if_counter_is_reset_after_circuit_change_state_from_close_to_open()
     {
-        $config = Config::make([
-            'failure_threshold' => 3,
-        ]);
-
-        $breaker = new CircuitBreaker($config);
+        $breaker = CircuitBreaker::for('test-service')
+            ->withOptions(['failure_threshold' => 3])
+            ->getCircuitBreaker();
 
         $fail = function () {
             throw new \Exception();
@@ -127,7 +120,7 @@ public function test_if_counter_is_reset_after_circuit_change_state_from_close_t
         foreach (range(1, $tries) as $i) {
             try {
                 $breaker->call($fail);
-            } catch (\Exception $exception) {
+            } catch (\Exception) {
 
             }
         }
@@ -137,7 +130,7 @@ public function test_if_counter_is_reset_after_circuit_change_state_from_close_t
 
     public function test_if_it_will_close_circuit_after_success_call()
     {
-        $breaker = new CircuitBreaker();
+        $breaker = CircuitBreaker::for('test-service')->getCircuitBreaker();
         $breaker->storage->setState(CircuitState::HalfOpen);
 
         $success = function () {
@@ -151,7 +144,7 @@ public function test_if_it_will_close_circuit_after_success_call()
 
     public function test_if_it_will_transit_back_to_open_state_after_first_fail()
     {
-        $breaker = new CircuitBreaker();
+        $breaker = CircuitBreaker::for('test-service')->getCircuitBreaker();
 
         $breaker->storage->setState(CircuitState::HalfOpen);
 
@@ -183,7 +176,7 @@ public function onFail($exception): void
             }
         };
 
-        $factory = CircuitBreaker::factory()
+        $factory = CircuitBreaker::for('test-service')
             ->withOptions(['failure_threshold' => 10])
             ->withListeners([$object]);
 
@@ -212,7 +205,7 @@ public function test_if_it_can_skip_some_exception()
     {
         $testException = new class () extends \Exception {};
 
-        $factory = CircuitBreaker::factory()
+        $factory = CircuitBreaker::for('test-service')
             ->skipFailure(function (\Exception $exception) use ($testException) {
                 return $exception instanceof $testException;
             });
@@ -226,20 +219,23 @@ public function test_if_it_can_skip_some_exception()
 
     public function test_if_it_can_fail_even_without_exception()
     {
-        $factory = CircuitBreaker::factory()
+        $factory = CircuitBreaker::for('test-service')
+            ->withOptions(['failure_threshold' => 2])
             ->failWhen(function ($result) {
-                if ($result instanceof \stdClass) {
-                    throw new \Exception();
-                }
+                return $result instanceof \stdClass;
             });
 
-        try {
-            $factory->call(fn () => new \stdClass());
-        } catch (\Exception) {
+        foreach (range(1, 3) as $i) {
+            try {
+                $factory->call(fn () => new \stdClass());
+            } catch (\Exception) {
 
+            }
         }
 
-        $this->assertEquals(1, $factory->breaker->storage->getFailuresCount());
+        // Make sure that number of failures is reset to zero
+        $this->assertEquals(0, $factory->breaker->storage->getFailuresCount());
+        $this->assertTrue($factory->breaker->isOpen());
     }
 
     //    public function test_if_redis_work()