From 61569d0f5e40d0a69cdba95a14335f21079c0678 Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Tue, 3 Aug 2021 12:47:40 -0400
Subject: [PATCH 1/7] [AOPS-60] Refactor and update RedisDedupe before new
 additions

---
 .gitignore                        |  1 +
 lib/redis_dedupe.rb               | 64 ---------------------
 lib/redis_dedupe/helpers.rb       | 44 +++++++++++++++
 lib/redis_dedupe/set.rb           | 69 +++++++++++++++++++++++
 redis_dedupe.gemspec              |  4 +-
 spec/redis_dedupe/helpers_spec.rb | 46 +++++++++++++++
 spec/redis_dedupe/set_spec.rb     | 94 +++++++++++++++++++++++++++++++
 spec/redis_dedupe_spec.rb         | 56 ------------------
 8 files changed, 257 insertions(+), 121 deletions(-)
 delete mode 100644 lib/redis_dedupe.rb
 create mode 100644 lib/redis_dedupe/helpers.rb
 create mode 100644 lib/redis_dedupe/set.rb
 create mode 100644 spec/redis_dedupe/helpers_spec.rb
 create mode 100644 spec/redis_dedupe/set_spec.rb
 delete mode 100644 spec/redis_dedupe_spec.rb

diff --git a/.gitignore b/.gitignore
index 31cafb5..92a5b23 100644
--- a/.gitignore
+++ b/.gitignore
@@ -20,3 +20,4 @@ tmp
 *.o
 *.a
 mkmf.log
+vendor/*
diff --git a/lib/redis_dedupe.rb b/lib/redis_dedupe.rb
deleted file mode 100644
index a706668..0000000
--- a/lib/redis_dedupe.rb
+++ /dev/null
@@ -1,64 +0,0 @@
-require 'redis_dedupe/version'
-
-module RedisDedupe
-  class<<self
-    attr_accessor :client
-  end
-
-  class Set
-    SEVEN_DAYS = 7 * 24 * 60 * 60
-
-    attr_reader :key, :expires_in
-
-    def initialize(redis, key, expires_in = SEVEN_DAYS)
-      @redis      = redis
-      @key        = key
-      @expires_in = expires_in
-    end
-
-    def check(member)
-      results = redis.pipelined do
-        redis.sadd(key, member)
-        redis.expire(key, expires_in)
-      end
-
-      if results[0]
-        yield
-      end
-    end
-
-    def finish
-      redis.del(key)
-    end
-
-    private
-
-    def redis
-      @redis
-    end
-  end
-
-  module Helpers
-    private
-
-    def dedupe
-      @dedupe ||= RedisDedupe::Set.new(RedisDedupe.client, [dedupe_namespace, dedupe_id].join(':'))
-    end
-
-    # Implement in class, should return an integer or string:
-    #
-    # Ex.
-    #
-    #   def dedupe_id
-    #     @announcement.id # => 42
-    #   end
-    #
-    def dedupe_id
-      raise NotImplementedError
-    end
-
-    def dedupe_namespace
-      self.class.name
-    end
-  end
-end
diff --git a/lib/redis_dedupe/helpers.rb b/lib/redis_dedupe/helpers.rb
new file mode 100644
index 0000000..de977a7
--- /dev/null
+++ b/lib/redis_dedupe/helpers.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module RedisDedupe
+  #
+  # Include `RedisDedupe::Helpers` to use +RedisDedupe::Set+
+  #
+  # class MyClass
+  #   include RedisDedupe::Helpers
+  #
+  #   private
+  #
+  #   def dedupe_id
+  #     "my_unique_set_key"
+  #   end
+  # end
+  #
+  module Helpers
+    private
+
+    def dedupe
+      @dedupe ||= RedisDedupe::Set.new(RedisDedupe.client, key)
+    end
+
+    def key
+      [dedupe_namespace, dedupe_id].join(":")
+    end
+
+    # Implement in class, should return an integer or string:
+    #
+    # Ex.
+    #
+    #   def dedupe_id
+    #     @announcement.id # => 42
+    #   end
+    #
+    def dedupe_id
+      raise NotImplementedError
+    end
+
+    def dedupe_namespace
+      self.class.name
+    end
+  end
+end
diff --git a/lib/redis_dedupe/set.rb b/lib/redis_dedupe/set.rb
new file mode 100644
index 0000000..55ed18e
--- /dev/null
+++ b/lib/redis_dedupe/set.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module RedisDedupe
+  class << self
+    attr_accessor :client
+  end
+
+  # A mechanism to make sure that a block of code will only be called once for each specified identifier (or +member+)
+  # even if the calling process dies or restarts, as long the datastore is +Redis+-backed.
+  #
+  # @example Keep the set around for 7 days, letting Redis handle its own memory cleanup after that time
+  #   ```
+  #   dedupe = RedisDedupe::Set.new($redis, "send_payment_due_emails")
+  #   Account.all do |account|
+  #     dedupe.check(account.id) do
+  #       mail(to: account.billing_email, subject: "PAY US... NOW!!!")
+  #     end
+  #   end
+  #   ```
+  #
+  # @example If you want to be able to repeat the process at any time immediately following this method
+  #   ```
+  #   dedupe = RedisDedupe::Set.new($redis, "send_welcome_emails")
+  #   Account.all.pluck(:email) do |email|
+  #     dedupe.check(email) { mail(to: email, subject: "Hello!") }
+  #   end
+  #   dedupe.finish
+  #   ```
+  #
+   class Set
+    SEVEN_DAYS = 7 * 24 * 60 * 60
+
+    attr_reader :key, :expires_in
+
+    def initialize(redis, key, expires_in = SEVEN_DAYS)
+      @redis      = redis
+      @key        = key
+      @expires_in = expires_in
+    end
+
+    # Ensures that a block of code will only be run if the +member+ is not already contained in Redis.
+    # ie: the code block has not already run for the specified +member+.
+    #
+    # @param [String, Integer] member identifiying value to make sure the given block only runs once
+    #
+    # @return `nil` if the block was not run, otherwise the result of the yielded block
+    #
+    def check(member)
+      results = redis.pipelined do
+        redis.sadd(key, member)
+        redis.expire(key, expires_in)
+      end
+
+      return nil unless block_given? && results[0] # `results` will be `[true]` or `[false]`
+
+      yield
+    end
+
+    def finish
+      redis.del(key)
+    end
+
+    private
+
+    def redis
+      @redis
+    end
+  end
+end
diff --git a/redis_dedupe.gemspec b/redis_dedupe.gemspec
index fcce7a4..ac32c9c 100644
--- a/redis_dedupe.gemspec
+++ b/redis_dedupe.gemspec
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 # coding: utf-8
 lib = File.expand_path('../lib', __FILE__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
@@ -18,7 +19,8 @@ Gem::Specification.new do |spec|
   spec.require_paths = ["lib"]
 
   spec.add_development_dependency "bundler", "~> 1.6"
+  spec.add_development_dependency "mock_redis"
   spec.add_development_dependency "rake"
   spec.add_development_dependency "rspec"
-  spec.add_development_dependency "mock_redis"
+  spec.add_development_dependency "timecop"
 end
diff --git a/spec/redis_dedupe/helpers_spec.rb b/spec/redis_dedupe/helpers_spec.rb
new file mode 100644
index 0000000..316675f
--- /dev/null
+++ b/spec/redis_dedupe/helpers_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require "mock_redis"
+require "spec_helper"
+
+require "redis_dedupe/helpers"
+
+RSpec.describe RedisDedupe::Helpers do
+  class MyRedisDedupeTestClass
+    include RedisDedupe::Helpers
+
+    attr_reader :counter
+
+    def initialize
+      @counter = 0
+    end
+
+    def test_call
+      dedupe.check(5) { @counter += 1 }
+      dedupe.check(5) { @counter += 1 }
+      dedupe.check(7) { @counter += 1 }
+    end
+
+    def dedupe_id
+      "just_a_test"
+    end
+  end
+
+  let(:redis)     { MockRedis.new }
+  let(:instance)  { MyRedisDedupeTestClass.new }
+
+  before do
+    allow(RedisDedupe).to receive(:client).and_return(redis)
+  end
+
+  describe "#dedupe" do
+    subject { instance.test_call }
+
+    it { expect { subject }.to change(instance, :counter).from(0).to(2) }
+
+    it "uses the correct redis key" do
+      subject
+      expect(redis.smembers("MyRedisDedupeTestClass:just_a_test")).to match_array(["5", "7"])
+    end
+  end
+end
diff --git a/spec/redis_dedupe/set_spec.rb b/spec/redis_dedupe/set_spec.rb
new file mode 100644
index 0000000..ce8bc46
--- /dev/null
+++ b/spec/redis_dedupe/set_spec.rb
@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+require "mock_redis"
+require "spec_helper"
+require "timecop"
+
+require "redis_dedupe/set"
+
+RSpec.describe RedisDedupe::Set do
+  let(:redis) { MockRedis.new }
+  let(:key)   { "Module::Class::#{Time.now.utc.to_i}" }
+
+  let(:member) do
+    [
+      [1, 2, 3].sample,
+      %w[key1 key2 key3].sample,
+      %i[key1 key2 key3].sample
+    ].sample
+  end
+
+  describe "#initialize #new" do
+    subject(:dedupe) { described_class.new(redis, key) }
+
+    it { expect(dedupe.key).to eq(key) }
+    it { expect(dedupe.expires_in.to_i).to eq(7 * 24 * 60 * 60) } # 7 days
+
+    context "when given the optional arg :expires_in" do
+      subject(:dedupe) { described_class.new(redis, key, expires_in) }
+
+      # 30s, 1m, 2m, 3m, 5m, 1d, 3d, 30d
+      let(:expires_in) { [30, 60, 120, 180, 300, 86_400, 259_200, 2_592_000].sample }
+
+      it { expect(dedupe.key).to eq(key) }
+      it { expect(dedupe.expires_in.to_i).to eq(expires_in) }
+    end
+  end
+
+  describe "#check" do
+    let(:dedupe1) { described_class.new(redis, key, 120) }
+    let(:dedupe2) { described_class.new(redis, "Some::Other::Key") }
+    let(:dedupe3) { described_class.new(redis, "Another::Strange::Key") }
+    let(:results) { [] }
+
+    context "when calling multiple times for the same member" do
+      before do
+        dedupe1.check(member) { results << "A" }
+        dedupe2.check(member) { results << "B" }
+        dedupe1.check(member) { results << "C" }
+        dedupe3.check(member) { results << "D" }
+        dedupe2.check(member) { results << "E" }
+
+        Timecop.travel(Time.now + 110)
+      end
+
+      after do
+        Timecop.return
+      end
+
+      it "only calls the block once per member" do
+        expect(results).to eq(%w[A B D])
+      end
+
+      it "resets the redis ttl" do
+        expect(redis.ttl(key)).to be_within(1).of(10) # 120 - 110 => 10
+        dedupe1.check("another_member")
+        expect(redis.ttl(key)).to be_within(1).of(120)
+      end
+    end
+
+    context "when the yielded block raises an error" do
+      subject do
+        dedupe1.check(member) do
+          raise "foobar"
+        end
+      end
+
+      it "does not remove the member from Redis and re-raises the error" do
+        expect { subject }.to raise_error(RuntimeError, "foobar")
+        expect(redis.smembers(key)).to include(member.to_s)
+      end
+    end
+  end
+
+  describe "#finish" do
+    let(:dedupe) { described_class.new(redis, key) }
+
+    it "removes the entire set for the specified key" do
+      dedupe.check(member)
+      expect(redis.exists?(key)).to eq(true)
+      dedupe.finish
+      expect(redis.exists?(key)).to eq(false)
+    end
+  end
+end
diff --git a/spec/redis_dedupe_spec.rb b/spec/redis_dedupe_spec.rb
deleted file mode 100644
index 7544211..0000000
--- a/spec/redis_dedupe_spec.rb
+++ /dev/null
@@ -1,56 +0,0 @@
-require 'redis_dedupe'
-require 'mock_redis'
-require 'spec_helper'
-
-describe RedisDedupe::Set do
-  it "is initialized with a redis client and key" do
-    dedupe = RedisDedupe::Set.new(:redis, :key)
-    expect(dedupe.key).to eq(:key)
-  end
-
-  it "defaults expires_in to 7 days" do
-    dedupe = RedisDedupe::Set.new(:redis, :key)
-    expect(dedupe.expires_in.to_i).to eq(7 * 24 * 60 * 60)
-  end
-
-  it "optionally receives an expires_in seconds value" do
-    dedupe = RedisDedupe::Set.new(:redis, :key, 60)
-    expect(dedupe.expires_in.to_i).to eq(60)
-  end
-end
-
-describe RedisDedupe::Set, "#check" do
-  it "prevents a block from yielding multiple times for the same member" do
-    dedupe1 = RedisDedupe::Set.new(MockRedis.new, 'spec_key:1')
-    dedupe2 = RedisDedupe::Set.new(MockRedis.new, 'spec_key:2')
-
-    @results = []
-
-    dedupe1.check('1') { @results << 'A' }
-    dedupe1.check('1') { @results << 'B' }
-    dedupe2.check('1') { @results << 'C' }
-
-    expect(@results).to eq(['A', 'C'])
-  end
-
-  it "sets the set to expire so it cleans up if the process never completes" do
-    redis  = MockRedis.new
-    dedupe = RedisDedupe::Set.new(redis, 'spec_key:1', 10)
-
-    dedupe.check('1') {  }
-
-    expect(redis.ttl 'spec_key:1').to be_within(1).of(10)
-  end
-end
-
-describe RedisDedupe::Set, "#finish" do
-  it "removes the set to free up memory" do
-    redis  = MockRedis.new
-    dedupe = RedisDedupe::Set.new(redis, 'spec_key:1')
-
-    dedupe.check('1') {  }
-    dedupe.finish
-
-    expect(redis.exists 'spec_key:1').to be(false)
-  end
-end

From 9821ac61b1a713b916970eba3b405242233812a6 Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Tue, 3 Aug 2021 12:50:14 -0400
Subject: [PATCH 2/7] [AOPS-60] Add RedisDedupe::Set#max_member

---
 lib/redis_dedupe/set.rb       | 21 +++++++++++++++++++++
 spec/redis_dedupe/set_spec.rb | 30 ++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/lib/redis_dedupe/set.rb b/lib/redis_dedupe/set.rb
index 55ed18e..074005a 100644
--- a/lib/redis_dedupe/set.rb
+++ b/lib/redis_dedupe/set.rb
@@ -60,6 +60,27 @@ def finish
       redis.del(key)
     end
 
+    # Retrieves the member in the set with the largest value.
+    #
+    # This will work on String and Integers, but really meant for Integer
+    # If used for String, make sure it's really doing what you want and expect
+    #
+    # @example with Integers
+    # redis.smembers("foo") => [1, 2, 3, 4, 5]
+    # max_member => 5
+    #
+    # @example with String
+    # redis.smembers("foo") => ["abc", "xyz", "lmn"]
+    # max_member => "xyz"
+    #
+    # @see Array#max
+    #
+    # @return [Integer, String] the member in the set with the largest value
+    #
+    def max_member
+      redis.smembers(key).max
+    end
+
     private
 
     def redis
diff --git a/spec/redis_dedupe/set_spec.rb b/spec/redis_dedupe/set_spec.rb
index ce8bc46..45464d1 100644
--- a/spec/redis_dedupe/set_spec.rb
+++ b/spec/redis_dedupe/set_spec.rb
@@ -91,4 +91,34 @@
       expect(redis.exists?(key)).to eq(false)
     end
   end
+
+  describe "#max_member" do
+    subject { dedupe.max_member }
+
+    let(:dedupe) { described_class.new(redis, key) }
+
+    context "when member is input as an integer" do
+      before do
+        dedupe.check(3)
+        dedupe.check(5)
+        dedupe.check(4)
+        dedupe.check(2)
+        dedupe.check(1)
+      end
+
+      it { is_expected.to eq("5") }
+    end
+
+    context "when member is input as a string representation of an integer" do
+      before do
+        dedupe.check("3")
+        dedupe.check("5")
+        dedupe.check("4")
+        dedupe.check("2")
+        dedupe.check("1")
+      end
+
+      it { is_expected.to eq("5") }
+    end
+  end
 end

From 500fba3afe13d5f75bce0b32f6e8fff9545006d3 Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Tue, 3 Aug 2021 12:51:59 -0400
Subject: [PATCH 3/7] [AOPS-60] Remove member from set when error is raised in
 given block

---
 lib/redis_dedupe/set.rb       | 13 ++++++++++++-
 spec/redis_dedupe/set_spec.rb |  4 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/redis_dedupe/set.rb b/lib/redis_dedupe/set.rb
index 074005a..e89ad74 100644
--- a/lib/redis_dedupe/set.rb
+++ b/lib/redis_dedupe/set.rb
@@ -41,6 +41,8 @@ def initialize(redis, key, expires_in = SEVEN_DAYS)
     # Ensures that a block of code will only be run if the +member+ is not already contained in Redis.
     # ie: the code block has not already run for the specified +member+.
     #
+    # Note that if the given block raises an error, the +member+ will not remain in the +Set+ and may be tried again.
+    #
     # @param [String, Integer] member identifiying value to make sure the given block only runs once
     #
     # @return `nil` if the block was not run, otherwise the result of the yielded block
@@ -53,7 +55,16 @@ def check(member)
 
       return nil unless block_given? && results[0] # `results` will be `[true]` or `[false]`
 
-      yield
+      # If it didn't get processed... we can retry it again
+      # Unless it it's an error that's ALWAYS going to occur, in which case we might not want to do this.
+      # Should we track/handle this differently?
+      # But as long as the calling code doesn't error in the yielded block, all should be fine.
+      begin
+        yield
+      rescue => e
+        redis.del(key, member)
+        raise e
+      end
     end
 
     def finish
diff --git a/spec/redis_dedupe/set_spec.rb b/spec/redis_dedupe/set_spec.rb
index 45464d1..a517422 100644
--- a/spec/redis_dedupe/set_spec.rb
+++ b/spec/redis_dedupe/set_spec.rb
@@ -74,9 +74,9 @@
         end
       end
 
-      it "does not remove the member from Redis and re-raises the error" do
+      it "removes the member from Redis and re-raises the error" do
         expect { subject }.to raise_error(RuntimeError, "foobar")
-        expect(redis.smembers(key)).to include(member.to_s)
+        expect(redis.smembers(key)).not_to include(member)
       end
     end
   end

From 2e98171eb09317b141af2df760c84ce4d87158b0 Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Tue, 3 Aug 2021 14:51:52 -0400
Subject: [PATCH 4/7] [AOPS-60] Bump version and require set and helpers by
 default

---
 lib/redis_dedupe.rb         | 2 ++
 lib/redis_dedupe/version.rb | 2 +-
 redis_dedupe.gemspec        | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)
 create mode 100644 lib/redis_dedupe.rb

diff --git a/lib/redis_dedupe.rb b/lib/redis_dedupe.rb
new file mode 100644
index 0000000..c1fa560
--- /dev/null
+++ b/lib/redis_dedupe.rb
@@ -0,0 +1,2 @@
+require 'redis_dedupe/set'
+require 'redis_dedupe/helpers'
diff --git a/lib/redis_dedupe/version.rb b/lib/redis_dedupe/version.rb
index 9711216..24fd429 100644
--- a/lib/redis_dedupe/version.rb
+++ b/lib/redis_dedupe/version.rb
@@ -1,3 +1,3 @@
 module RedisDedupe
-  VERSION = "0.0.3"
+  VERSION = "0.0.4"
 end
diff --git a/redis_dedupe.gemspec b/redis_dedupe.gemspec
index ac32c9c..ef26378 100644
--- a/redis_dedupe.gemspec
+++ b/redis_dedupe.gemspec
@@ -14,7 +14,7 @@ Gem::Specification.new do |spec|
   spec.homepage      = ""
   spec.license       = "MIT"
 
-  spec.files         = `git ls-files -z`.split("\x0")
+  spec.files         = Dir['lib/**/*.rb'] + ['lib/redis_dedupe.rb']
   spec.executables   = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
   spec.require_paths = ["lib"]
 

From db33d9415725e8ce8cc189e1b8a49b6e1187f706 Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Tue, 3 Aug 2021 15:05:59 -0400
Subject: [PATCH 5/7] [AOPS-60] Add Rubocop

---
 .rubocop.yml                      |  8 ++++++
 .rubocop_todo.yml                 |  0
 Gemfile                           |  4 ++-
 Rakefile                          |  5 ++--
 lib/redis_dedupe.rb               |  6 +++--
 lib/redis_dedupe/helpers.rb       |  1 +
 lib/redis_dedupe/set.rb           | 23 +++++++++-------
 lib/redis_dedupe/version.rb       |  3 +++
 redis_dedupe.gemspec              | 38 +++++++++++++++-----------
 spec/redis_dedupe/helpers_spec.rb | 45 ++++++++++++++++---------------
 spec/spec_helper.rb               | 20 ++++++++++++++
 tasks/rspec.rake                  |  4 ++-
 12 files changed, 104 insertions(+), 53 deletions(-)
 create mode 100644 .rubocop.yml
 create mode 100644 .rubocop_todo.yml

diff --git a/.rubocop.yml b/.rubocop.yml
new file mode 100644
index 0000000..8882b88
--- /dev/null
+++ b/.rubocop.yml
@@ -0,0 +1,8 @@
+inherit_from: .rubocop_todo.yml
+
+Style/StringLiterals:
+  EnforcedStyle: "double_quotes"
+
+Metrics/BlockLength:
+  Exclude:
+    - "spec/**/*.rb"
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
new file mode 100644
index 0000000..e69de29
diff --git a/Gemfile b/Gemfile
index 0581497..e8ef368 100644
--- a/Gemfile
+++ b/Gemfile
@@ -1,4 +1,6 @@
-source 'https://rubygems.org'
+# frozen_string_literal: true
+
+source "https://rubygems.org"
 
 # Specify your gem's dependencies in redis_dedupe.gemspec
 gemspec
diff --git a/Rakefile b/Rakefile
index 416672a..53a945c 100644
--- a/Rakefile
+++ b/Rakefile
@@ -1,3 +1,4 @@
-require "bundler/gem_tasks"
-Dir.glob('tasks/**/*.rake').each(&method(:import))
+# frozen_string_literal: true
 
+require "bundler/gem_tasks"
+Dir.glob("tasks/**/*.rake").each(&method(:import))
diff --git a/lib/redis_dedupe.rb b/lib/redis_dedupe.rb
index c1fa560..42cdfa8 100644
--- a/lib/redis_dedupe.rb
+++ b/lib/redis_dedupe.rb
@@ -1,2 +1,4 @@
-require 'redis_dedupe/set'
-require 'redis_dedupe/helpers'
+# frozen_string_literal: true
+
+require "redis_dedupe/set"
+require "redis_dedupe/helpers"
diff --git a/lib/redis_dedupe/helpers.rb b/lib/redis_dedupe/helpers.rb
index de977a7..d6f2ad9 100644
--- a/lib/redis_dedupe/helpers.rb
+++ b/lib/redis_dedupe/helpers.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+# :nodoc:
 module RedisDedupe
   #
   # Include `RedisDedupe::Helpers` to use +RedisDedupe::Set+
diff --git a/lib/redis_dedupe/set.rb b/lib/redis_dedupe/set.rb
index e89ad74..06f98ab 100644
--- a/lib/redis_dedupe/set.rb
+++ b/lib/redis_dedupe/set.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+# :nodoc:
 module RedisDedupe
   class << self
     attr_accessor :client
@@ -27,7 +28,7 @@ class << self
   #   dedupe.finish
   #   ```
   #
-   class Set
+  class Set
     SEVEN_DAYS = 7 * 24 * 60 * 60
 
     attr_reader :key, :expires_in
@@ -48,12 +49,7 @@ def initialize(redis, key, expires_in = SEVEN_DAYS)
     # @return `nil` if the block was not run, otherwise the result of the yielded block
     #
     def check(member)
-      results = redis.pipelined do
-        redis.sadd(key, member)
-        redis.expire(key, expires_in)
-      end
-
-      return nil unless block_given? && results[0] # `results` will be `[true]` or `[false]`
+      return unless execute_block_for_member?(member)
 
       # If it didn't get processed... we can retry it again
       # Unless it it's an error that's ALWAYS going to occur, in which case we might not want to do this.
@@ -61,7 +57,7 @@ def check(member)
       # But as long as the calling code doesn't error in the yielded block, all should be fine.
       begin
         yield
-      rescue => e
+      rescue StandardError => e
         redis.del(key, member)
         raise e
       end
@@ -94,8 +90,15 @@ def max_member
 
     private
 
-    def redis
-      @redis
+    attr_reader :redis
+
+    def execute_block_for_member?(member)
+      results = redis.pipelined do
+        redis.sadd(key, member)
+        redis.expire(key, expires_in)
+      end
+
+      block_given? && results[0] # `results` will be `[true]` or `[false]`
     end
   end
 end
diff --git a/lib/redis_dedupe/version.rb b/lib/redis_dedupe/version.rb
index 24fd429..e8c5056 100644
--- a/lib/redis_dedupe/version.rb
+++ b/lib/redis_dedupe/version.rb
@@ -1,3 +1,6 @@
+# frozen_string_literal: true
+
+# :nodoc:
 module RedisDedupe
   VERSION = "0.0.4"
 end
diff --git a/redis_dedupe.gemspec b/redis_dedupe.gemspec
index ef26378..c9b210a 100644
--- a/redis_dedupe.gemspec
+++ b/redis_dedupe.gemspec
@@ -1,26 +1,34 @@
 # frozen_string_literal: true
-# coding: utf-8
-lib = File.expand_path('../lib', __FILE__)
+
+lib = File.expand_path("lib", __dir__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
-require 'redis_dedupe/version'
+require "redis_dedupe/version"
 
 Gem::Specification.new do |spec|
-  spec.name          = "redis_dedupe"
-  spec.version       = RedisDedupe::VERSION
-  spec.authors       = ["Andy Huynh"]
-  spec.email         = ["andy4thehuynh@gmail.com"]
-  spec.summary       = %q{ A weak deduper to make things like bulk email run safer. }
-  spec.description   = %q{ This is a weak deduper to make things like bulk email run safer. It is not a lock safe for financial/security needs because it uses a weak redis locking pattern that can have race conditions. However, imagine a bulk email job that loops over 100 users, and enqueues a background email for each user. If the job fails at iteration 50, a retry would enqueue all the users again and many will receive dupes. This would continue multiple times as the parent job continued to rerun. By marking that a subjob has been enqueued, we can let that isolated job handle its own failures, and the batch enqueue job can run multiple times without re-enqueueing the same subjobs. }
-  spec.homepage      = ""
-  spec.license       = "MIT"
-
-  spec.files         = Dir['lib/**/*.rb'] + ['lib/redis_dedupe.rb']
-  spec.executables   = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
-  spec.require_paths = ["lib"]
+  spec.name                   = "redis_dedupe"
+  spec.version                = RedisDedupe::VERSION
+  spec.required_ruby_version  = ">= 2.6.0"
+  spec.authors                = ["Andy Huynh"]
+  spec.email                  = ["andy4thehuynh@gmail.com"]
+  spec.summary                = "A weak deduper to make things like bulk email run safer."
+  spec.homepage               = ""
+  spec.license                = "MIT"
+  spec.files                  = Dir["lib/**/*.rb"] + ["lib/redis_dedupe.rb"]
+  spec.executables            = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
+  spec.require_paths          = ["lib"]
+  spec.description            = <<~EO_DESC
+    This is a weak deduper to make things like bulk email run safer. It is not a lock safe for financial/security needs
+    because it uses a weak redis locking pattern that can have race conditions. However, imagine a bulk email job that
+    loops over 100 users, and enqueues a background email for each user. If the job fails at iteration 50, a retry
+    would enqueue all the users again and many will receive dupes. This would continue multiple times as the parent
+    job continued to rerun. By marking that a subjob has been enqueued, we can let that isolated job handle its own
+    failures, and the batch enqueue job can run multiple times without re-enqueueing the same subjobs.
+  EO_DESC
 
   spec.add_development_dependency "bundler", "~> 1.6"
   spec.add_development_dependency "mock_redis"
   spec.add_development_dependency "rake"
   spec.add_development_dependency "rspec"
+  spec.add_development_dependency "rubocop"
   spec.add_development_dependency "timecop"
 end
diff --git a/spec/redis_dedupe/helpers_spec.rb b/spec/redis_dedupe/helpers_spec.rb
index 316675f..dd0e716 100644
--- a/spec/redis_dedupe/helpers_spec.rb
+++ b/spec/redis_dedupe/helpers_spec.rb
@@ -6,28 +6,8 @@
 require "redis_dedupe/helpers"
 
 RSpec.describe RedisDedupe::Helpers do
-  class MyRedisDedupeTestClass
-    include RedisDedupe::Helpers
-
-    attr_reader :counter
-
-    def initialize
-      @counter = 0
-    end
-
-    def test_call
-      dedupe.check(5) { @counter += 1 }
-      dedupe.check(5) { @counter += 1 }
-      dedupe.check(7) { @counter += 1 }
-    end
-
-    def dedupe_id
-      "just_a_test"
-    end
-  end
-
   let(:redis)     { MockRedis.new }
-  let(:instance)  { MyRedisDedupeTestClass.new }
+  let(:instance)  { RedisDedupeSpecStubbedClass.new }
 
   before do
     allow(RedisDedupe).to receive(:client).and_return(redis)
@@ -40,7 +20,28 @@ def dedupe_id
 
     it "uses the correct redis key" do
       subject
-      expect(redis.smembers("MyRedisDedupeTestClass:just_a_test")).to match_array(["5", "7"])
+      expect(redis.smembers("RedisDedupeSpecStubbedClass:just_a_test")).to match_array(%w[5 7])
     end
   end
 end
+
+# :nodoc:
+class RedisDedupeSpecStubbedClass
+  include RedisDedupe::Helpers
+
+  attr_reader :counter
+
+  def initialize
+    @counter = 0
+  end
+
+  def test_call
+    dedupe.check(5) { @counter += 1 }
+    dedupe.check(5) { @counter += 1 }
+    dedupe.check(7) { @counter += 1 }
+  end
+
+  def dedupe_id
+    "just_a_test"
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index e69de29..f88db8e 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+RSpec.configure do |config|
+  # rspec-mocks config goes here. You can use an alternate test double
+  # library (such as bogus or mocha) by changing the `mock_with` option here.
+  config.mock_with :rspec do |mocks|
+    # Prevents you from mocking or stubbing a method that does not exist on
+    # a real object. This is generally recommended, and will default to
+    # `true` in RSpec 4.
+    mocks.verify_partial_doubles = true
+  end
+  # Limits the available syntax to the non-monkey patched syntax that is
+  # recommended. For more details, see:
+  #   - http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax
+  #   - http://teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/
+  #   - http://myronmars.to/n/dev-blog/2014/05/notable-changes-in-rspec-3#new__config_option_to_disable_rspeccore_monkey_patching
+  config.disable_monkey_patching!
+
+  config.order = :random
+end
diff --git a/tasks/rspec.rake b/tasks/rspec.rake
index f7c787b..1f1889a 100644
--- a/tasks/rspec.rake
+++ b/tasks/rspec.rake
@@ -1,3 +1,5 @@
-require 'rspec/core/rake_task'
+# frozen_string_literal: true
+
+require "rspec/core/rake_task"
 
 RSpec::Core::RakeTask.new(:spec)

From 3aac507fe7d517ca30645539a519dbe3edd0ee4f Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Wed, 4 Aug 2021 14:38:40 -0400
Subject: [PATCH 6/7] [AOPS-60] Update `check` to require a block, even if
 empty

---
 lib/redis_dedupe/set.rb           | 15 +++++++--------
 spec/redis_dedupe/helpers_spec.rb | 16 +++++++---------
 spec/redis_dedupe/set_spec.rb     | 32 +++++++++++++++++++------------
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/lib/redis_dedupe/set.rb b/lib/redis_dedupe/set.rb
index 06f98ab..f3d5529 100644
--- a/lib/redis_dedupe/set.rb
+++ b/lib/redis_dedupe/set.rb
@@ -46,17 +46,16 @@ def initialize(redis, key, expires_in = SEVEN_DAYS)
     #
     # @param [String, Integer] member identifiying value to make sure the given block only runs once
     #
+    # @yield block to run for the specified +member+, which should only be run once for any particular member
+    #
     # @return `nil` if the block was not run, otherwise the result of the yielded block
     #
-    def check(member)
-      return unless execute_block_for_member?(member)
+    def check(member, &block)
+      raise ArgumentError, "passing a block is required" if block.nil?
+      return nil unless execute_block_for_member?(member)
 
-      # If it didn't get processed... we can retry it again
-      # Unless it it's an error that's ALWAYS going to occur, in which case we might not want to do this.
-      # Should we track/handle this differently?
-      # But as long as the calling code doesn't error in the yielded block, all should be fine.
       begin
-        yield
+        block.call
       rescue StandardError => e
         redis.del(key, member)
         raise e
@@ -98,7 +97,7 @@ def execute_block_for_member?(member)
         redis.expire(key, expires_in)
       end
 
-      block_given? && results[0] # `results` will be `[true]` or `[false]`
+      results[0] # `results` will be `[true]` or `[false]`
     end
   end
 end
diff --git a/spec/redis_dedupe/helpers_spec.rb b/spec/redis_dedupe/helpers_spec.rb
index dd0e716..fb27217 100644
--- a/spec/redis_dedupe/helpers_spec.rb
+++ b/spec/redis_dedupe/helpers_spec.rb
@@ -16,7 +16,7 @@
   describe "#dedupe" do
     subject { instance.test_call }
 
-    it { expect { subject }.to change(instance, :counter).from(0).to(2) }
+    it { expect(subject).to eq(2) }
 
     it "uses the correct redis key" do
       subject
@@ -29,16 +29,14 @@
 class RedisDedupeSpecStubbedClass
   include RedisDedupe::Helpers
 
-  attr_reader :counter
+  def test_call
+    counter = 0
 
-  def initialize
-    @counter = 0
-  end
+    dedupe.check(5) { counter += 1 }
+    dedupe.check(5) { counter += 1 }
+    dedupe.check(7) { counter += 1 }
 
-  def test_call
-    dedupe.check(5) { @counter += 1 }
-    dedupe.check(5) { @counter += 1 }
-    dedupe.check(7) { @counter += 1 }
+    counter
   end
 
   def dedupe_id
diff --git a/spec/redis_dedupe/set_spec.rb b/spec/redis_dedupe/set_spec.rb
index a517422..3403d54 100644
--- a/spec/redis_dedupe/set_spec.rb
+++ b/spec/redis_dedupe/set_spec.rb
@@ -41,6 +41,14 @@
     let(:dedupe3) { described_class.new(redis, "Another::Strange::Key") }
     let(:results) { [] }
 
+    context "when no block is given" do
+      it { expect { dedupe1.check(member) }.to raise_error(ArgumentError, "passing a block is required") }
+    end
+
+    context "when an empty block is given" do
+      it { expect(dedupe1.check(member) {}).to eq(nil) }
+    end
+
     context "when calling multiple times for the same member" do
       before do
         dedupe1.check(member) { results << "A" }
@@ -62,7 +70,7 @@
 
       it "resets the redis ttl" do
         expect(redis.ttl(key)).to be_within(1).of(10) # 120 - 110 => 10
-        dedupe1.check("another_member")
+        dedupe1.check("another_member") {}
         expect(redis.ttl(key)).to be_within(1).of(120)
       end
     end
@@ -85,7 +93,7 @@
     let(:dedupe) { described_class.new(redis, key) }
 
     it "removes the entire set for the specified key" do
-      dedupe.check(member)
+      dedupe.check(member) {}
       expect(redis.exists?(key)).to eq(true)
       dedupe.finish
       expect(redis.exists?(key)).to eq(false)
@@ -99,11 +107,11 @@
 
     context "when member is input as an integer" do
       before do
-        dedupe.check(3)
-        dedupe.check(5)
-        dedupe.check(4)
-        dedupe.check(2)
-        dedupe.check(1)
+        dedupe.check(3) {}
+        dedupe.check(5) {}
+        dedupe.check(4) {}
+        dedupe.check(2) {}
+        dedupe.check(1) {}
       end
 
       it { is_expected.to eq("5") }
@@ -111,11 +119,11 @@
 
     context "when member is input as a string representation of an integer" do
       before do
-        dedupe.check("3")
-        dedupe.check("5")
-        dedupe.check("4")
-        dedupe.check("2")
-        dedupe.check("1")
+        dedupe.check("3") {}
+        dedupe.check("5") {}
+        dedupe.check("4") {}
+        dedupe.check("2") {}
+        dedupe.check("1") {}
       end
 
       it { is_expected.to eq("5") }

From a2f3906253089b9888aa9f4fc981a87a641af4b1 Mon Sep 17 00:00:00 2001
From: "Brian H. Castle" <bhcastle@gmail.com>
Date: Wed, 4 Aug 2021 14:56:04 -0400
Subject: [PATCH 7/7] [AOPS-60] Fix removal of single member instead of
 deleting entire key

---
 lib/redis_dedupe/set.rb       | 2 +-
 spec/redis_dedupe/set_spec.rb | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/redis_dedupe/set.rb b/lib/redis_dedupe/set.rb
index f3d5529..b6792a8 100644
--- a/lib/redis_dedupe/set.rb
+++ b/lib/redis_dedupe/set.rb
@@ -57,7 +57,7 @@ def check(member, &block)
       begin
         block.call
       rescue StandardError => e
-        redis.del(key, member)
+        redis.srem(key, member)
         raise e
       end
     end
diff --git a/spec/redis_dedupe/set_spec.rb b/spec/redis_dedupe/set_spec.rb
index 3403d54..ab3279c 100644
--- a/spec/redis_dedupe/set_spec.rb
+++ b/spec/redis_dedupe/set_spec.rb
@@ -77,14 +77,14 @@
 
     context "when the yielded block raises an error" do
       subject do
-        dedupe1.check(member) do
-          raise "foobar"
-        end
+        dedupe1.check("a") {}
+        dedupe1.check("b") {}
+        dedupe1.check("c") { raise "foobar" }
       end
 
       it "removes the member from Redis and re-raises the error" do
         expect { subject }.to raise_error(RuntimeError, "foobar")
-        expect(redis.smembers(key)).not_to include(member)
+        expect(redis.smembers(key)).to match_array(%w[a b])
       end
     end
   end