From 5e5778b06480ac8e11215569db6201d974c655b1 Mon Sep 17 00:00:00 2001
From: Stefan Damjanovic <damjanovicstefan0@gmail.com>
Date: Wed, 24 Jan 2024 19:18:16 +0100
Subject: [PATCH] Simplify circuit breaker configuration

---
 README.md                                |  7 +--
 src/Config.php                           | 26 ++------
 src/Counter.php                          | 58 ------------------
 src/StateHandlers/ClosedStateHandler.php | 12 +---
 src/Storage/CircuitBreakerStorage.php    |  9 +--
 src/Storage/InMemoryStorage.php          | 20 +------
 src/Storage/RedisStorage.php             | 24 +++-----
 tests/CircuitBreakerTest.php             | 75 +++---------------------
 tests/ConfigTest.php                     | 14 ++---
 tests/CounterTest.php                    | 23 --------
 10 files changed, 36 insertions(+), 232 deletions(-)
 delete mode 100644 src/Counter.php
 delete mode 100644 tests/CounterTest.php

diff --git a/README.md b/README.md
index 72a3add..070f101 100644
--- a/README.md
+++ b/README.md
@@ -86,10 +86,9 @@ use Stfn\CircuitBreaker\CircuitBreaker;
 
 $breaker = CircuitBreaker::for('3rd-party-service')
     ->withOptions([
-        'failure_ratio' => 0.5,
-        'minimum_throughput' => 10,
-        'recovery_time' => 60,
-        'sample_duration' => 120
+        'failure_threshold' => 10,
+        'recovery_time' => 120,
+        'sample_duration' => 60,
     ]);
 ```
 
diff --git a/src/Config.php b/src/Config.php
index 2f8087f..76bfaf7 100644
--- a/src/Config.php
+++ b/src/Config.php
@@ -9,12 +9,7 @@ class Config
     /**
      * @var float
      */
-    public float $failureRatio;
-
-    /**
-     * @var int
-     */
-    public int $minimumThroughput;
+    public float $failureThreshold;
 
     /**
      * @var int
@@ -27,23 +22,16 @@ class Config
     public int $sampleDuration;
 
     /**
-     * @param float $failureRatio
-     * @param int $minimumThroughput
+     * @param float $failureThreshold
      * @param int $recoveryTime
      * @param int $sampleDuration
      */
     public function __construct(
-        float $failureRatio = 0.5,
-        int $minimumThroughput = 10,
+        float $failureThreshold = 5,
         int $recoveryTime = 60,
         int $sampleDuration = 120
     ) {
-        if ($failureRatio < 0 || $failureRatio > 1) {
-            throw new \InvalidArgumentException("Failure ratio must be between 0 and 1");
-        }
-
-        $this->failureRatio = $failureRatio;
-        $this->minimumThroughput = $minimumThroughput;
+        $this->failureThreshold = $failureThreshold;
         $this->recoveryTime = $recoveryTime;
         $this->sampleDuration = $sampleDuration;
     }
@@ -55,8 +43,7 @@ public function __construct(
     public static function fromArray(array $config = []): Config
     {
         return new Config(
-            $config['failure_ratio'] ?? 0.5,
-            $config['minimum_throughput'] ?? 10,
+            $config['failure_threshold'] ?? 5,
             $config['recovery_time'] ?? 60,
             $config['sample_duration'] ?? 120
         );
@@ -68,8 +55,7 @@ public static function fromArray(array $config = []): Config
     public function toArray()
     {
         return [
-            'failure_ratio' => $this->failureRatio,
-            'minimum_throughput' => $this->minimumThroughput,
+            'failure_ratio' => $this->failureThreshold,
             'recovery_time' => $this->recoveryTime,
             'sample_duration' => $this->sampleDuration,
         ];
diff --git a/src/Counter.php b/src/Counter.php
deleted file mode 100644
index dcdc396..0000000
--- a/src/Counter.php
+++ /dev/null
@@ -1,58 +0,0 @@
-<?php
-
-namespace Stfn\CircuitBreaker;
-
-class Counter
-{
-    /**
-     * @var int
-     */
-    protected int $success;
-
-    /**
-     * @var int
-     */
-    protected int $failures;
-
-    /**
-     * @param $failures
-     * @param $success
-     */
-    public function __construct($failures, $success)
-    {
-        $this->failures = $failures;
-        $this->success = $success;
-    }
-
-    /**
-     * @return int
-     */
-    public function getNumberOfSuccess()
-    {
-        return $this->success;
-    }
-
-    /**
-     * @return int
-     */
-    public function getNumberOfFailures()
-    {
-        return $this->failures;
-    }
-
-    /**
-     * @return float|int
-     */
-    public function failureRatio()
-    {
-        return $this->failures / $this->total();
-    }
-
-    /**
-     * @return int
-     */
-    public function total()
-    {
-        return $this->failures + $this->success;
-    }
-}
diff --git a/src/StateHandlers/ClosedStateHandler.php b/src/StateHandlers/ClosedStateHandler.php
index 908176b..12c7e79 100644
--- a/src/StateHandlers/ClosedStateHandler.php
+++ b/src/StateHandlers/ClosedStateHandler.php
@@ -6,14 +6,6 @@
 
 class ClosedStateHandler extends StateHandler
 {
-    /**
-     * @return void
-     */
-    public function onSucess()
-    {
-        $this->breaker->getStorage()->incrementSuccess();
-    }
-
     /**
      * @param \Exception $exception
      * @return void
@@ -27,9 +19,7 @@ public function onFailure(\Exception $exception)
 
         $config = $this->breaker->getConfig();
 
-        $counter = $storage->getCounter();
-
-        if ($counter->total() >= $config->minimumThroughput && $counter->failureRatio() >= $config->failureRatio) {
+        if ($storage->getNumberOfFailures() >= $config->failureThreshold) {
             $this->breaker->openCircuit();
 
             throw CircuitOpenException::make($this->breaker->getName());
diff --git a/src/Storage/CircuitBreakerStorage.php b/src/Storage/CircuitBreakerStorage.php
index 0c3fb85..6792a1a 100644
--- a/src/Storage/CircuitBreakerStorage.php
+++ b/src/Storage/CircuitBreakerStorage.php
@@ -48,20 +48,15 @@ abstract public function close(): void;
      */
     abstract public function incrementFailure(): void;
 
-    /**
-     * @return void
-     */
-    abstract public function incrementSuccess(): void;
-
     /**
      * @return void
      */
     abstract public function resetCounter(): void;
 
     /**
-     * @return Counter
+     * @return int
      */
-    abstract public function getCounter(): Counter;
+    abstract public function getNumberOfFailures(): int;
 
     /**
      * @return int
diff --git a/src/Storage/InMemoryStorage.php b/src/Storage/InMemoryStorage.php
index 9a74990..9732ed2 100644
--- a/src/Storage/InMemoryStorage.php
+++ b/src/Storage/InMemoryStorage.php
@@ -3,7 +3,6 @@
 namespace Stfn\CircuitBreaker\Storage;
 
 use Stfn\CircuitBreaker\CircuitState;
-use Stfn\CircuitBreaker\Counter;
 
 class InMemoryStorage extends CircuitBreakerStorage
 {
@@ -17,11 +16,6 @@ class InMemoryStorage extends CircuitBreakerStorage
      */
     protected int $failCount = 0;
 
-    /**
-     * @var int
-     */
-    protected int $successCount = 0;
-
     /**
      * @var int|null
      */
@@ -52,14 +46,6 @@ public function incrementFailure(): void
         $this->failCount++;
     }
 
-    /**
-     * @return void
-     */
-    public function incrementSuccess(): void
-    {
-        $this->successCount++;
-    }
-
     /**
      * @return void
      */
@@ -69,11 +55,11 @@ public function resetCounter(): void
     }
 
     /**
-     * @return Counter
+     * @return int
      */
-    public function getCounter(): Counter
+    public function getNumberOfFailures(): int
     {
-        return new Counter($this->failCount, $this->successCount);
+        return $this->failCount;
     }
 
     /**
diff --git a/src/Storage/RedisStorage.php b/src/Storage/RedisStorage.php
index 71bb309..09fd9df 100644
--- a/src/Storage/RedisStorage.php
+++ b/src/Storage/RedisStorage.php
@@ -11,7 +11,6 @@ class RedisStorage extends CircuitBreakerStorage
     public const BASE_NAMESPACE = "stfn_php_circuit_breaker";
     public const STATE_KEY = "state";
     public const FAIL_COUNT_KEY = "fail_count";
-    public const SUCCESS_COUNT_KEY = "success_count";
     public const OPENED_AT_KEY = "opened_at";
 
     /**
@@ -72,16 +71,10 @@ public function setState(CircuitState $state): void
      */
     public function incrementFailure(): void
     {
-        $this->incrementOrCreate($this->getNamespace(self::FAIL_COUNT_KEY), $this->breaker->getConfig()->sampleDuration);
-    }
-
-    /**
-     * @return void
-     * @throws \RedisException
-     */
-    public function incrementSuccess(): void
-    {
-        $this->incrementOrCreate($this->getNamespace(self::SUCCESS_COUNT_KEY), $this->breaker->getConfig()->sampleDuration);
+        $this->incrementOrCreate(
+            $this->getNamespace(self::FAIL_COUNT_KEY),
+            $this->breaker->getConfig()->sampleDuration
+        );
     }
 
     /**
@@ -110,15 +103,12 @@ public function resetCounter(): void
     }
 
     /**
-     * @return Counter
+     * @return int
      * @throws \RedisException
      */
-    public function getCounter(): Counter
+    public function getNumberOfFailures(): int
     {
-        $failuresCount = (int) $this->redis->get($this->getNamespace(self::FAIL_COUNT_KEY));
-        $successCount = (int) $this->redis->get($this->getNamespace(self::SUCCESS_COUNT_KEY));
-
-        return new Counter($failuresCount, $successCount);
+        return (int) $this->redis->get($this->getNamespace(self::FAIL_COUNT_KEY));
     }
 
     /**
diff --git a/tests/CircuitBreakerTest.php b/tests/CircuitBreakerTest.php
index 93712bc..9332f78 100644
--- a/tests/CircuitBreakerTest.php
+++ b/tests/CircuitBreakerTest.php
@@ -47,8 +47,7 @@ public function test_if_it_will_record_every_failure()
     {
         $breaker = CircuitBreaker::for('test-service')
             ->withOptions([
-                'failure_ratio' => 1,
-                'minimum_throughput' => 4,
+                'failure_threshold' => 4
             ]);
 
         $fail = function () {
@@ -65,15 +64,14 @@ public function test_if_it_will_record_every_failure()
             }
         }
 
-        $this->assertEquals($tries, $breaker->getStorage()->getCounter()->getNumberOfFailures());
+        $this->assertEquals($tries, $breaker->getStorage()->getNumberOfFailures());
     }
 
-    public function test_if_it_will_not_open_breaker_when_minimum_throughput_is_not_reached()
+    public function test_if_it_will_open_circuit_when_failure_threshold_is_reached()
     {
         $breaker = CircuitBreaker::for('test-service')
             ->withOptions([
-                'failure_ratio' => 1,
-                'minimum_throughput' => 4,
+                'failure_threshold' => 3
             ]);
 
         $fail = function () {
@@ -88,64 +86,8 @@ public function test_if_it_will_not_open_breaker_when_minimum_throughput_is_not_
             }
         }
 
-        $this->assertTrue($breaker->isClosed());
-    }
-
-    public function test_if_it_will_not_open_breaker_if_ratio_is_not_reached()
-    {
-        $breaker = CircuitBreaker::for('test-service')
-            ->withOptions([
-                'failure_ratio' => 0.51,
-                'minimum_throughput' => 4,
-            ]);
-
-        $success = function () {
-            return true;
-        };
-
-        $fail = function () {
-            throw new \Exception();
-        };
-
-        $breaker->call($success);
-        $breaker->call($success);
-
-        foreach (range(1, 2) as $i) {
-            try {
-                $breaker->call($fail);
-            } catch (\Exception) {
-
-            }
-        }
-
-        $this->assertTrue($breaker->isClosed());
-    }
-
-    public function test_if_it_will_open_circuit_when_failure_ratio_is_reached()
-    {
-        $breaker = CircuitBreaker::for('test-service')
-            ->withOptions([
-                'failure_ratio' => 0.5,
-                'minimum_throughput' => 4,
-            ]);
-
-        $breaker->call(fn () => true);
-        $breaker->call(fn () => true);
-
-        $fail = function () {
-            throw new \Exception();
-        };
-
-        foreach (range(1, 2) as $i) {
-            try {
-                $breaker->call($fail);
-            } catch (\Exception) {
-
-            }
-        }
-
         $this->assertTrue($breaker->isOpen());
-        $this->assertEquals(0, $breaker->getStorage()->getCounter()->getNumberOfFailures());
+        $this->assertEquals(0, $breaker->getStorage()->getNumberOfFailures());
     }
 
     public function test_if_it_will_close_circuit_after_success_call()
@@ -252,15 +194,14 @@ public function test_if_it_can_skip_some_exception()
             throw $testException;
         });
 
-        $this->assertEquals(0, $breaker->getStorage()->getCounter()->getNumberOfFailures());
+        $this->assertEquals(0, $breaker->getStorage()->getNumberOfFailures());
     }
 
     public function test_if_it_can_fail_even_without_exception()
     {
         $breaker = CircuitBreaker::for('test-service')
             ->withOptions([
-                'failure_ratio' => 1,
-                'minimum_throughput' => 3,
+                'failure_threshold' => 3
             ])
             ->failWhen(function ($result) {
                 return $result instanceof \stdClass;
@@ -275,7 +216,7 @@ public function test_if_it_can_fail_even_without_exception()
         }
 
         // Make sure that number of failures is reset to zero
-        $this->assertEquals(0, $breaker->getStorage()->getCounter()->getNumberOfFailures());
+        $this->assertEquals(0, $breaker->getStorage()->getNumberOfFailures());
         $this->assertTrue($breaker->isOpen());
     }
 
diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php
index 27da672..838c5ff 100644
--- a/tests/ConfigTest.php
+++ b/tests/ConfigTest.php
@@ -9,24 +9,22 @@ class ConfigTest extends TestCase
 {
     public function test_if_it_will_throw_an_exception_on_invalid_failure_ratio()
     {
-        $this->expectException(\InvalidArgumentException::class);
+        $this->expectException(\TypeError::class);
 
-        Config::fromArray(['failure_ratio' => 1.01]);
+        Config::fromArray(['failure_threshold' => "1"]);
     }
 
     public function test_if_it_can_set_valid_config()
     {
         $setup = [
-            'failure_ratio' => 0.2,
-            'minimum_throughput' => 10,
-            'recovery_time' => 120,
-            'sample_duration' => 120,
+            'failure_threshold' => 20,
+            'recovery_time' => 200,
+            'sample_duration' => 20,
         ];
 
         $config = Config::fromArray($setup);
 
-        $this->assertEquals($setup['failure_ratio'], $config->failureRatio);
-        $this->assertEquals($setup['minimum_throughput'], $config->minimumThroughput);
+        $this->assertEquals($setup['failure_threshold'], $config->failureThreshold);
         $this->assertEquals($setup['recovery_time'], $config->recoveryTime);
         $this->assertEquals($setup['sample_duration'], $config->sampleDuration);
     }
diff --git a/tests/CounterTest.php b/tests/CounterTest.php
deleted file mode 100644
index f1840c4..0000000
--- a/tests/CounterTest.php
+++ /dev/null
@@ -1,23 +0,0 @@
-<?php
-
-namespace Stfn\CircuitBreaker\Tests;
-
-use PHPUnit\Framework\TestCase;
-use Stfn\CircuitBreaker\Counter;
-
-class CounterTest extends TestCase
-{
-    public function test_if_it_can_calculate_total_number_of_requests()
-    {
-        $counter = new Counter(2, 3);
-
-        $this->assertEquals(5, $counter->total());
-    }
-
-    public function test_if_it_can_calculate_failure_ratio()
-    {
-        $counter = new Counter(2, 8);
-
-        $this->assertEquals(0.2, $counter->failureRatio());
-    }
-}