Merge pull request #2 from prysmex/v0.3.3

cleaner api, ensure jobs run after failing

Author: patodevilla

.rubocop.yml

@@ -18,7 +18,7 @@ require:
 
 AllCops:
   NewCops: enable
-  # TargetRubyVersion: 2.7.8
+  TargetRubyVersion: 3.2.2
   # TargetRailsVersion: 6.1.4
   # Exclude:
   #   - 'Gemfile.lock'

.rubocop_todo.yml

@@ -1,24 +1,23 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-03-19 18:55:09 UTC using RuboCop version 1.62.1.
+# on 2024-03-22 01:08:54 UTC using RuboCop version 1.62.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
+# Offense count: 1
+# Configuration parameters: Severity, Include.
+# Include: **/*.gemspec
+Gemspec/RequiredRubyVersion:
+  Exclude:
+    - 'sidekiq-bouncer.gemspec'
+
 # Offense count: 1
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
   Max: 14
 
-# Offense count: 1
-RSpec/BeforeAfterAll:
-  Exclude:
-    - '**/spec/spec_helper.rb'
-    - '**/spec/rails_helper.rb'
-    - '**/spec/support/**/*.rb'
-    - 'spec/sidekiq_bouncer/bouncer_spec.rb'
-
 # Offense count: 2
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
@@ -29,7 +28,7 @@ RSpec/ExampleLength:
 RSpec/NestedGroups:
   Max: 4
 
-# Offense count: 1
+# Offense count: 2
 RSpec/SubjectStub:
   Exclude:
     - 'spec/sidekiq_bouncer/bouncer_spec.rb'

Gemfile

@@ -9,6 +9,7 @@ gemspec
 gem 'debug', '>= 1.0.0'
 gem 'rake', '~> 13.1'
 gem 'rspec', '~> 3.9'
+gem 'timecop'
 
 # rubocop
 gem 'rubocop', '~> 1.62'

README.md

@@ -61,9 +61,7 @@ class FooWorker
   # The default delay is 60 seconds. You can optionally override it.
   register_bouncer(delay: optional_delay_override, delay_buffer: optional_delay_buffer_override)
 
-  def perform(param1, param2, debounce_key = nil)
-    return unless self.class.bouncer.let_in?(debounce_key) # last argument is added as debounce_key
-
+  def perform(param1, param2)
     # Do your thing.
   end
 end

lib/sidekiq_bouncer/bounceable.rb

@@ -4,23 +4,26 @@ module SidekiqBouncer
   module Bounceable
 
     def self.included(base)
-      base.include InstanceMethods
+      base.prepend InstanceMethods
       base.extend ClassMethods
     end
 
     module ClassMethods
-      # creates and sets a +SidekiqBouncer::Bouncer+
-      def register_bouncer(**kwargs)
-        @bouncer = SidekiqBouncer::Bouncer.new(self, **kwargs)
-      end
-
       # @retrun [SidekiqBouncer::Bouncer]
-      def bouncer
-        @bouncer
+      attr_reader :bouncer
+
+      # creates and sets a +SidekiqBouncer::Bouncer+
+      def register_bouncer(**)
+        @bouncer = SidekiqBouncer::Bouncer.new(self, **)
       end
     end
 
     module InstanceMethods
+      def perform(*, debounce_key, **)
+        self.class.bouncer.run(debounce_key) do
+          super(*, **)
+        end
+      end
     end
 
   end

lib/sidekiq_bouncer/bouncer.rb

@@ -54,6 +54,16 @@ def debounce(*params, key_or_args_indices:)
 
     # Checks if job should be excecuted
     #
+    # @param [NilClass|String] key
+    # @return [False|*] true if should be excecuted
+    def run(key)
+      return false unless let_in?(key)
+
+      yield.tap do
+        redis.call('DEL', key)
+      end
+    end
+
     # @param [NilClass|String] key
     # @return [Boolean] true if should be excecuted
     def let_in?(key)
@@ -67,18 +77,16 @@ def let_in?(key)
       timestamp = redis.call('GET', key)
       return false if timestamp.nil? || now_i < timestamp.to_i
 
-      redis.call('DEL', key)
-
       true
     end
 
+    private
+
     # @return [RedisClient::Pooled]
     def redis
       SidekiqBouncer.config.redis
     end
 
-    private
-
     # Builds a key based on arguments
     #
     # @param [Array] redis_params

lib/sidekiq_bouncer/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module SidekiqBouncer
-  VERSION = '0.3.2'
+  VERSION = '0.3.3'
 end

spec/sidekiq_bouncer/bouncer_spec.rb

@@ -1,36 +1,33 @@
 # frozen_string_literal: true
 
 # Mock
-module RedisMock
+class RedisMock # rubocop:disable Lint/EmptyClass
 end
 
 # Mock
 class WorkerMock
   def self.bouncer
     SidekiqBouncer::Bouncer.new(self)
   end
-
-  # def self.perform_at(*args); end
 end
 
 # Tests
 describe SidekiqBouncer::Bouncer do
   # careful, the bouncer instance is generally cached on the worker model
   subject(:bouncer) { WorkerMock.bouncer }
 
-  before :all do
+  let(:redis) { SidekiqBouncer.config.redis }
+  let(:worker_klass) { WorkerMock }
+  let(:now) { Time.now.to_i }
+
+  before do
     SidekiqBouncer.configure do |config|
-      config.redis = RedisMock
+      config.redis = RedisMock.new
     end
-  end
 
-  let(:redis) { RedisMock }
-  let(:worker_klass) { WorkerMock }
-  let(:now) { 100 }
+    Timecop.freeze(Time.now)
 
-  before do
     # stubbing
-    allow(bouncer).to receive(:now_i) { now }
     allow(redis).to receive(:call)
     allow(worker_klass).to receive(:perform_at)
   end
@@ -43,7 +40,6 @@ def self.bouncer
     it { expect(bouncer).to respond_to(:delay_buffer=) }
     it { expect(bouncer).to respond_to(:debounce) }
     it { expect(bouncer).to respond_to(:let_in?) }
-    it { expect(bouncer).to respond_to(:redis) }
   end
 
   describe '.new' do
@@ -82,7 +78,7 @@ def self.bouncer
     it 'sets scoped_key to Redis with delayed timestamp' do
       bouncer.debounce('test_param_1', 'test_param_2', key_or_args_indices: [0, 1])
 
-      expect(SidekiqBouncer.config.redis)
+      expect(redis)
         .to have_received(:call)
         .with('SET', 'WorkerMock:test_param_1,test_param_2', now + bouncer.delay)
     end
@@ -101,7 +97,7 @@ def self.bouncer
       it 'sets scoped_key to Redis with delayed timestamp' do
         bouncer.debounce('test_param_1', 'test_param_2', key_or_args_indices: [0])
 
-        expect(SidekiqBouncer.config.redis)
+        expect(redis)
           .to have_received(:call)
           .with('SET', 'WorkerMock:test_param_1', now + bouncer.delay)
       end
@@ -121,7 +117,7 @@ def self.bouncer
   describe '#let_in?' do
     context 'when key is nil' do
       it 'does not call redis' do
-        expect(SidekiqBouncer.config.redis).not_to have_received(:call)
+        expect(redis).not_to have_received(:call)
       end
 
       it 'returns true' do
@@ -135,7 +131,7 @@ def self.bouncer
       it 'exec call on redis with GET' do
         bouncer.let_in?(key)
 
-        expect(SidekiqBouncer.config.redis)
+        expect(redis)
           .to have_received(:call)
           .with('GET', key)
       end
@@ -145,14 +141,6 @@ def self.bouncer
           allow(redis).to receive(:call).with('GET', anything).and_return(now - 10)
         end
 
-        it 'exec call on redis with DEL' do
-          bouncer.let_in?(key)
-
-          expect(SidekiqBouncer.config.redis)
-            .to have_received(:call)
-            .with('DEL', key)
-        end
-
         it 'returns true' do
           expect(bouncer.let_in?(key)).to be(true)
         end
@@ -163,14 +151,6 @@ def self.bouncer
           allow(redis).to receive(:call).with('GET', anything).and_return(Time.now + 10)
         end
 
-        it 'does not exec call on redis with DEL' do
-          bouncer.let_in?(key)
-
-          expect(SidekiqBouncer.config.redis)
-            .not_to have_received(:call)
-            .with('DEL', anything)
-        end
-
         it 'returns false' do
           expect(bouncer.let_in?(key)).to be(false)
         end
@@ -181,18 +161,72 @@ def self.bouncer
           allow(redis).to receive(:call).with('GET', anything).and_return(nil)
         end
 
-        it 'does not exec call on redis with DEL' do
-          bouncer.let_in?(key)
-
-          expect(SidekiqBouncer.config.redis)
-            .not_to have_received(:call)
-            .with('DEL', anything)
-        end
-
         it 'returns false' do
           expect(bouncer.let_in?(key)).to be(false)
         end
       end
     end
   end
+
+  describe '#run' do
+    before do
+      # stubbing
+      allow(bouncer).to receive(:let_in?).with('do').and_return(true)
+      allow(bouncer).to receive(:let_in?).with('do_not').and_return(false)
+    end
+
+    context 'when let_in? returns false' do
+      it 'returns false' do
+        expect(bouncer.run('do_not')).to be(false)
+      end
+
+      it 'does not yield' do
+        expect { |b| bouncer.run('do_not', &b) }.not_to yield_control
+      end
+
+      it 'does not exec call on redis with DEL' do
+        bouncer.run('do_not') { '__test__' }
+
+        expect(redis)
+          .not_to have_received(:call)
+          .with('DEL', anything)
+      end
+    end
+
+    context 'when let_in? returns true' do
+      it 'returns yield return' do
+        expect(bouncer.run('do') { '__test__' }).to be('__test__')
+      end
+
+      it 'yields' do
+        expect { |b| bouncer.run('do', &b) }.to yield_control
+      end
+
+      it 'exec call on redis with DEL' do
+        bouncer.run('do') { '__test__' }
+
+        expect(redis)
+          .to have_received(:call)
+          .with('DEL', 'do')
+      end
+    end
+  end
+
+  describe '#now_i' do
+    it 'returns now as integer' do
+      expect(bouncer.send(:now_i)).to be(now)
+    end
+  end
+
+  describe '#redis' do
+    it 'returns' do
+      expect(bouncer.send(:redis)).to be_a(RedisMock)
+    end
+  end
+
+  describe '#redis_key' do
+    it 'returns now as integer' do
+      expect(bouncer.send(:redis_key, 'test_key')).to eql('WorkerMock:test_key')
+    end
+  end
 end

spec/spec_helper.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
+$LOAD_PATH.unshift File.expand_path('../lib', __dir__)
 require 'bundler/setup'
 require 'sidekiq_bouncer'
+require 'timecop'
 require 'debug'
 
 RSpec.configure do |config|