refactor period incrementing, move appropriate tests to counter_test

lib/von.rb

@@ -21,9 +21,7 @@ def self.configure
 
   def self.increment(field)
     counter = Counter.new(field)
-
     counter.increment
-    counter.periods.each { |key, period| counter.increment(period) }
   end
 
   def self.count(field, period = nil)

lib/von/counter.rb

@@ -28,30 +28,24 @@ def hash_key
     end
 
     # Increment the Redis count for this Counter.
-    # If the field is a Period, we increment the Period.
-    #
-    # field - the Redis field containing the count
-    def increment(field = 'total')
-      if field.is_a?(Period)
-        increment_period(field)
-      else
-        Von.connection.hincrby(hash_key, field, 1)
-        increment_parents
-      end
+    def increment
+      Von.connection.hincrby(hash_key, 'total', 1)
+      increment_periods
+      increment_parents
     end
 
-    # Increment the Redis count for a Period
-    #
-    # period - The Period to increment
-    def increment_period(period)
-      Von.connection.hincrby(period.hash_key, period.field, 1)
-      unless Von.connection.lrange(period.list_key, 0, -1).include?(period.field)
-        Von.connection.rpush(period.list_key, period.field)
-      end
+    # Increment the Redis count for the associated Periods
+    def increment_periods
+      periods.each do |key, period|
+        Von.connection.hincrby(period.hash_key, period.field, 1)
+        unless Von.connection.lrange(period.list_key, 0, -1).include?(period.field)
+          Von.connection.rpush(period.list_key, period.field)
+        end
 
-      if Von.connection.llen(period.list_key) > period.length
-        expired_counter = Von.connection.lpop(period.list_key)
-        Von.connection.hdel(period.hash_key, expired_counter)
+        if Von.connection.llen(period.list_key) > period.length
+          expired_counter = Von.connection.lpop(period.list_key)
+          Von.connection.hdel(period.hash_key, expired_counter)
+        end
       end
     end
 

lib/von/period.rb

@@ -4,7 +4,7 @@ class Period
 
     attr_reader :counter
     attr_reader :length
-    
+
     # Initialize a Period object
     #
     # counter - the field name for the counter
@@ -38,7 +38,7 @@ def time_unit
     def hours?
       @period == :hourly
     end
-    
+
     # Returns the String DateTime format
     def format
       @format ||= Von.config.send(:"#{@period}_format")
@@ -53,7 +53,7 @@ def hash_key
     def list_key
       @list ||= "#{Von.config.namespace}:lists:#{@counter}:#{@period}"
     end
-    
+
     # Returns the Redis field representation used for storing the count value
     def field
       @now.strftime(format)

test/counter_test.rb

@@ -0,0 +1,99 @@
+require 'test_helper'
+
+describe Von::Counter do
+  Counter = Von::Counter
+
+  before :each do
+    Timecop.freeze(Time.local(2013, 01))
+    Von.config.reset!
+    connection = Von::TestConnection.new
+    @store     = connection.store
+    Von.expects(:connection).returns(connection).at_least_once
+  end
+
+  it "increments the total counter if given a single key" do
+    Counter.new('foo').increment
+
+    @store.has_key?('von:foo').must_equal true
+    @store['von:foo']['total'].must_equal 1
+
+    Counter.new('foo').increment
+    @store['von:foo']['total'].must_equal 2
+  end
+
+  it "increments the total counter for a key and it's parent keys" do
+    Counter.new('foo:bar').increment
+
+    @store.has_key?('von:foo').must_equal true
+    @store['von:foo']['total'].must_equal 1
+    @store.has_key?('von:foo:bar').must_equal true
+    @store['von:foo:bar']['total'].must_equal 1
+
+    Counter.new('foo:bar').increment
+    @store['von:foo']['total'].must_equal 2
+    @store['von:foo:bar']['total'].must_equal 2
+  end
+
+  it "increments a month counter" do
+    Von.configure do |config|
+      config.counter 'foo', :monthly => 1
+    end
+
+    Counter.new('foo').increment
+    Counter.new('foo').increment
+
+    @store.has_key?('von:foo').must_equal true
+    @store.has_key?('von:foo:monthly').must_equal true
+    @store['von:foo']['total'].must_equal 2
+    @store['von:foo:monthly']['2013-01'].must_equal 2
+    @store['von:lists:foo:monthly'].size.must_equal 1
+  end
+
+  it "expires counters past the limit" do
+    Von.configure do |config|
+      config.counter 'foo', :monthly => 1
+    end
+
+    Counter.new('foo').increment
+    Timecop.freeze(Time.local(2013, 02))
+    Counter.new('foo').increment
+
+    @store.has_key?('von:foo').must_equal true
+    @store.has_key?('von:foo:monthly').must_equal true
+    @store['von:foo']['total'].must_equal 2
+    @store['von:foo:monthly'].has_key?('2013-02').must_equal true
+    @store['von:lists:foo:monthly'].size.must_equal 1
+  end
+
+  it "gets a total count for a counter" do
+    Counter.new('foo').increment
+    Counter.new('foo').increment
+    Counter.new('foo').increment
+
+    Von.count('foo').must_equal 3
+  end
+
+  it "gets a count for a time period and 0s missing entries" do
+    Von.configure do |config|
+      config.counter 'foo', :monthly => 1, :hourly => 6
+    end
+
+    Timecop.freeze(Time.local(2013, 02, 01, 05))
+    Counter.new('foo').increment
+    Timecop.freeze(Time.local(2013, 02, 01, 07))
+    Counter.new('foo').increment
+
+    Von.count('foo').must_equal 2
+
+    Von.count('foo', :monthly).must_equal [{"2013-02" => 2}]
+    Von.count('foo', :hourly).must_equal [
+      {"2013-02-01 02:00"=>0},
+      {"2013-02-01 03:00"=>0},
+      {"2013-02-01 04:00"=>0},
+      {"2013-02-01 05:00"=>1},
+      {"2013-02-01 06:00"=>0},
+      {"2013-02-01 07:00"=>1}
+    ]
+  end
+
+end

test/period_test.rb

@@ -11,47 +11,47 @@
     period.counter.must_equal 'foo'
     period.length.must_equal 6
   end
-  
+
   it "checks if the period is an hourly period" do
     Period.new('foo', :hourly, 6).must_be :hours?
     Period.new('foo', :daily, 6).wont_be :hours?
     Period.new('foo', :weekly, 6).wont_be :hours?
     Period.new('foo', :monthly, 6).wont_be :hours?
     Period.new('foo', :yearly, 6).wont_be :hours?
   end
-  
+
   it "knows what time unit it is" do
     Period.new('foo', :hourly, 6).time_unit.must_equal :hour
     Period.new('foo', :daily, 6).time_unit.must_equal :day
     Period.new('foo', :weekly, 6).time_unit.must_equal :week
     Period.new('foo', :monthly, 6).time_unit.must_equal :month
     Period.new('foo', :yearly, 6).time_unit.must_equal :year
   end
-  
+
   it "pulls a time format from config options" do
     Period.new('foo', :hourly, 6).format.must_equal Von.config.hourly_format
     Period.new('foo', :daily, 6).format.must_equal Von.config.daily_format
     Period.new('foo', :weekly, 6).format.must_equal Von.config.weekly_format
     Period.new('foo', :monthly, 6).format.must_equal Von.config.monthly_format
     Period.new('foo', :yearly, 6).format.must_equal Von.config.yearly_format
   end
-  
+
   it "builds a redis hash key string" do
     field  = 'foo'
     period = :hourly
     period_obj = Period.new(field, period, 6)
-    
+
     period_obj.hash_key.must_equal "#{@config.namespace}:#{field}:#{period}"
   end
-  
+
   it "builds a redis list key string" do
     field  = 'foo'
     period = :hourly
     period_obj = Period.new(field, period, 6)
-    
+
     period_obj.list_key.must_equal "#{@config.namespace}:lists:#{field}:#{period}"
   end
-  
+
   it "builds a redis field for the given period and current time" do
     Timecop.freeze(Time.local(2013, 02, 01, 05))
     Period.new('foo', :hourly, 6).field.must_equal '2013-02-01 05:00'
@@ -60,5 +60,5 @@
     Period.new('foo', :monthly, 6).field.must_equal '2013-02'
     Period.new('foo', :yearly, 6).field.must_equal '2013'
   end
-  
+
 end

test/von_test.rb

@@ -1,100 +1,18 @@
 require 'test_helper'
 
-
-# TODO: benchmark parent checking
 describe Von do
 
   before :each do
-    Timecop.freeze(Time.local(2013, 01))
     Von.config.reset!
     connection = Von::TestConnection.new
     @store     = connection.store
     Von.expects(:connection).returns(connection).at_least_once
   end
 
-  it "increments the total counter if given a single key" do
-    Von.increment('foo')
-
-    @store.has_key?('von:foo').must_equal true
-    @store['von:foo']['total'].must_equal 1
-
-    Von.increment('foo')
-    @store['von:foo']['total'].must_equal 2
-  end
-
-  it "increments the total counter for a key and it's parent keys" do
-    Von.increment('foo:bar')
-
-    @store.has_key?('von:foo').must_equal true
-    @store['von:foo']['total'].must_equal 1
-    @store.has_key?('von:foo:bar').must_equal true
-    @store['von:foo:bar']['total'].must_equal 1
-
-    Von.increment('foo:bar')
-    @store['von:foo']['total'].must_equal 2
-    @store['von:foo:bar']['total'].must_equal 2
-  end
-
-  it "increments a month counter" do
-    Von.configure do |config|
-      config.counter 'foo', :monthly => 1
-    end
-
-    Von.increment('foo')
-    Von.increment('foo')
-
-    @store.has_key?('von:foo').must_equal true
-    @store.has_key?('von:foo:monthly').must_equal true
-    @store['von:foo']['total'].must_equal 2
-    @store['von:foo:monthly']['2013-01'].must_equal 2
-    @store['von:lists:foo:monthly'].size.must_equal 1
-  end
-
-  it "expires counters past the limit" do
-    Von.configure do |config|
-      config.counter 'foo', :monthly => 1
-    end
-
-    Von.increment('foo')
-    Timecop.freeze(Time.local(2013, 02))
-    Von.increment('foo')
-
-    @store.has_key?('von:foo').must_equal true
-    @store.has_key?('von:foo:monthly').must_equal true
-    @store['von:foo']['total'].must_equal 2
-    @store['von:foo:monthly'].has_key?('2013-02').must_equal true
-    @store['von:lists:foo:monthly'].size.must_equal 1
-  end
-
-  it "gets a total count for a counter" do
-    Von.increment('foo')
-    Von.increment('foo')
-    Von.increment('foo')
-
-    Von.count('foo').must_equal 3
-  end
-
-  it "gets a count for a time period and 0s missing entries" do
-    Von.configure do |config|
-      config.counter 'foo', :monthly => 1, :hourly => 6
-    end
-
-    Timecop.freeze(Time.local(2013, 02, 01, 05))
+  it "increments a counter and counts it" do
     Von.increment('foo')
-    Timecop.freeze(Time.local(2013, 02, 01, 07))
     Von.increment('foo')
-
     Von.count('foo').must_equal 2
-
-    Von.count('foo', :monthly).must_equal [{"2013-02" => 2}]
-    Von.count('foo', :hourly).must_equal [
-      {"2013-02-01 02:00"=>0},
-      {"2013-02-01 03:00"=>0},
-      {"2013-02-01 04:00"=>0},
-      {"2013-02-01 05:00"=>1},
-      {"2013-02-01 06:00"=>0},
-      {"2013-02-01 07:00"=>1}
-    ]
   end
 
 end