Merge pull request #8 from tarunsai/master

Remove caching for reached_failure_threshold and add spec for the same

Author: vasuadari

lib/circuit_breaker-ruby/config.rb

@@ -20,5 +20,11 @@ def initialize
       self.invocation_timeout = INVOCATION_TIMEOUT
       self.retry_timeout = RETRY_TIMEOUT
     end
+
+    def self.update(klass, options)
+      (UPDATABLE & options.keys).each do |variable|
+        klass.instance_variable_set("@#{variable}", options[variable])
+      end
+    end
   end
 end

lib/circuit_breaker-ruby/shield.rb

@@ -16,10 +16,10 @@ module States
     def initialize(**options)
       @failure_count = 0
       @total_count = 0
-      @failure_threshold = options[:failure_threshold] || config.failure_threshold
-      @failure_threshold_percentage = options[:failure_threshold_percentage] || config.failure_threshold_percentage
-      @invocation_timeout = options[:invocation_timeout] || config.invocation_timeout
-      @retry_timeout = options[:retry_timeout] || config.retry_timeout
+      @failure_threshold = options.fetch(:failure_threshold, config.failure_threshold)
+      @failure_threshold_percentage = options.fetch(:failure_threshold_percentage, config.failure_threshold_percentage)
+      @invocation_timeout = options.fetch(:invocation_timeout, config.invocation_timeout)
+      @retry_timeout = options.fetch(:retry_timeout, config.retry_timeout)
       @callback = options[:callback]
     end
 
@@ -28,17 +28,15 @@ def config
     end
 
     def update_config!(options)
-      (CircuitBreaker::Config::UPDATABLE & options.keys).each do |variable|
-        instance_variable_set("@#{variable}", options[variable])
-      end
+      CircuitBreaker::Config.update(self, options)
     end
 
     def protect(options = {}, &block)
       update_config!(options)
 
       case prev_state = state
       when States::CLOSED, States::HALF_OPEN
-        connect(prev_state, &block)
+        connect(&block)
       when States::OPEN
         raise CircuitBreaker::Open
       end
@@ -47,21 +45,21 @@ def protect(options = {}, &block)
     private
 
     def state
-      if reached_failure_threshold? && reached_retry_timeout?
+      if reached_failure_threshold? && reached_failure_threshold_percentage? && reached_retry_timeout?
         States::HALF_OPEN
-      elsif reached_failure_threshold?
+      elsif reached_failure_threshold? && reached_failure_threshold_percentage?
         States::OPEN
       else
         States::CLOSED
       end
     end
 
     def reached_failure_threshold?
-      @_reached_failure_threshold ||= begin
-        (failure_count >= failure_threshold) &&
-          (total_count != 0 &&
-            (failure_count.to_f / total_count.to_f) >= failure_threshold_percentage)
-      end
+      (failure_count >= failure_threshold)
+    end
+
+    def reached_failure_threshold_percentage?
+      (total_count.nonzero? && (failure_count.to_f / total_count.to_f) >= failure_threshold_percentage)
     end
 
     def reached_retry_timeout?
@@ -74,7 +72,7 @@ def reset
       @state = States::CLOSED
     end
 
-    def connect(prev_state, &block)
+    def connect(&block)
       begin
         result = nil
         ::Timeout::timeout(invocation_timeout) do
@@ -89,13 +87,13 @@ def connect(prev_state, &block)
         invoke_callback
         raise CircuitBreaker::TimeoutError
       ensure
-        increment_total_count(prev_state)
+        increment_total_count
       end
 
       result
     end
 
-    def increment_total_count(state)
+    def increment_total_count
       @total_count += 1
     end
 

spec/circuit_breaker/shield_spec.rb

@@ -118,6 +118,44 @@
         end
       end
     end
+
+    context 'when circuit is in open state and goes to closed state' do
+      it 'remains in closed state until it reaches failure_threshold' do
+        retry_timeout = 3
+
+        circuit_breaker_shield = CircuitBreaker::Shield.new(
+          invocation_timeout: 1,
+          retry_timeout: retry_timeout,
+          failure_threshold: 2
+        )
+
+        circuit_breaker_shield.protect { sleep(0.1) } # succeed once
+        expect(circuit_breaker_shield.total_count).to eql(1)
+        expect(circuit_breaker_shield.failure_count).to eql(0)
+
+        expect {circuit_breaker_shield.protect { sleep(1.1) }}.to raise_error(CircuitBreaker::TimeoutError) # fail once
+        expect(circuit_breaker_shield.total_count).to eql(2)
+        expect(circuit_breaker_shield.failure_count).to eql(1)
+        expect(circuit_breaker_shield.send(:state)).to be(CircuitBreaker::Shield::States::CLOSED)
+
+        expect {circuit_breaker_shield.protect { sleep(1.1) }}.to raise_error(CircuitBreaker::TimeoutError) # fail twice
+        expect(circuit_breaker_shield.total_count).to eql(3)
+        expect(circuit_breaker_shield.failure_count).to eql(2)
+        expect(circuit_breaker_shield.send(:state)).to be(CircuitBreaker::Shield::States::OPEN)
+
+        Timecop.freeze(Time.now + retry_timeout) do
+          circuit_breaker_shield.protect { sleep(0.1) } # succeed once
+          expect(circuit_breaker_shield.total_count).to eql(1)
+          expect(circuit_breaker_shield.failure_count).to eql(0)
+          expect(circuit_breaker_shield.send(:state)).to be(CircuitBreaker::Shield::States::CLOSED)
+
+          expect {circuit_breaker_shield.protect { sleep(1.1) }}.to raise_error(CircuitBreaker::TimeoutError) # fail once
+          expect(circuit_breaker_shield.total_count).to eql(2)
+          expect(circuit_breaker_shield.failure_count).to eql(1)
+          expect(circuit_breaker_shield.send(:state)).to be(CircuitBreaker::Shield::States::CLOSED)
+        end
+      end
+    end
   end
 
   context '#protect' do