refactor parent/period incrementing

lib/von.rb

@@ -20,13 +20,11 @@ def self.configure
   end
 
   def self.increment(field)
-    counter = Counter.new(field)
-    counter.increment
+    Counter.increment(field)
   end
 
   def self.count(field, period = nil)
-    counter = Counter.new(field)
-    counter.count(period)
+    Counter.count(field, period)
   end
 
   config.init!

lib/von/config.rb

@@ -11,8 +11,11 @@ module Config
     attr_accessor :daily_format
     attr_accessor :hourly_format
 
+    attr_reader  :periods
+
     def init!
       @counter_options = {}
+      @periods         = {}
       # all keys are prefixed with this namespace
       self.namespace = 'von'
       # 2013
@@ -34,15 +37,27 @@ def redis=(arg)
         @redis = Redis.new(arg)
       end
     end
-
     def redis
-      @redis
+      @redis ||= Redis.new
     end
 
     def counter(field, options = {})
+      options.each do |key, value|
+        if Period::AVAILABLE_PERIODS.include?(key)
+          @periods[field.to_sym] ||= {}
+          @periods[field.to_sym][key.to_sym] = Period.new(field, key, value)
+          options.delete(key)
+        end
+      end
+
       @counter_options[field.to_sym] = options
     end
 
+    def period_defined_for?(key, period)
+      @periods.has_key?(key) && @periods[key].has_key?(period)
+    end
+
+    # TODO: rename
     def counter_options(field)
       @counter_options[field.to_sym] ||= {}
     end

lib/von/counter.rb

@@ -1,6 +1,5 @@
 module Von
   class Counter
-    CHILD_REGEX  = /:[^:]+\z/
     PARENT_REGEX = /:?[^:]+\z/
 
     # Initialize a new Counter
@@ -15,28 +14,28 @@ def options
       @options ||= Von.config.counter_options(@field)
     end
 
-    # Returns periods specified in config for this Counter
-    def periods
-      @periods ||= options.select { |k|
-        Period::AVAILABLE_PERIODS.include?(k)
-      }.inject({}) { |h, (p, l)| h[p] = Period.new(@field, p, l); h }
-    end
-
     # Returns the Redis hash key used for storing counts for this Counter
     def hash_key
       @hash_key ||= "#{Von.config.namespace}:#{@field}"
     end
 
-    # Increment the Redis count for this Counter.
+    # Increment the total count for this Counter
+    # If the key has time periods specified, increment those.
+    #
+    # Returns the Integer total for the key
     def increment
-      Von.connection.hincrby(hash_key, 'total', 1)
+      total = Von.connection.hincrby(hash_key, 'total', 1)
+
       increment_periods
-      increment_parents
+
+      total
     end
 
-    # Increment the Redis count for the associated Periods
+    # Increment periods associated with this key
     def increment_periods
-      periods.each do |key, period|
+      return unless Von.config.periods.has_key?(@field.to_sym)
+
+      Von.config.periods[@field.to_sym].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)
@@ -49,19 +48,47 @@ def increment_periods
       end
     end
 
-    # Increment the parent keys of this Counter.
-    # Example: increment('something:foo:bar') would increment
-    # 'something:foo:bar', 'something:foo', 'something'
-    def increment_parents
-      field = @field.to_s
-      return if field !~ CHILD_REGEX
-
-      parents = field.sub(CHILD_REGEX, '')
+    # Increment the Redis count for this Counter.
+    # If the key has parents, increment them as well.
+    #
+    # Returns the Integer total for the key
+    def self.increment(field)
+      total   = Counter.new(field).increment
+      parents = field.sub(PARENT_REGEX, '')
 
       until parents.empty? do
-        Von.connection.hincrby("#{Von.config.namespace}:#{parents}", 'total', 1)
+        Counter.new(parents).increment
         parents.sub!(PARENT_REGEX, '')
       end
+
+      total
+    end
+
+    # Count the "total" field for this Counter.
+    #
+    # Returns an Integer count
+    def count
+      Von.connection.hget(hash_key, 'total')
+    end
+
+    # Count the fields for the given time period for this Counter.
+    #
+    # Returns an Array of Hashes representing the count
+    def count_period(period)
+      return unless Von.config.period_defined_for?(@field, period)
+
+      _counts   = []
+      _period   = Von.config.periods[@field][period]
+      now       = DateTime.now.beginning_of_hour
+
+      _period.length.times do
+        this_period = now.strftime(_period.format)
+        _counts.unshift(this_period)
+        now = _period.hours? ? now.ago(3600) : now.send(:"prev_#{_period.time_unit}")
+      end
+
+      keys = Von.connection.hgetall("#{hash_key}:#{period}")
+      _counts.map { |date| { date => keys.fetch(date, 0) }}
     end
 
     # Lookup the count for this Counter in Redis.
@@ -72,22 +99,13 @@ def increment_parents
     # period - A Period to lookup
     #
     # Returns an Integer representing the count or an Array of counts.
-    def count(period = nil)
+    def self.count(field, period = nil)
+      counter = Counter.new(field)
+
       if period.nil?
-        Von.connection.hget(hash_key, 'total')
+        counter.count
       else
-        _count   = []
-        _period  = periods[period]
-        now      = DateTime.now.beginning_of_hour
-
-        _period.length.times do
-          this_period = now.strftime(_period.format)
-          _count.unshift(this_period)
-          now = _period.hours? ? now.ago(3600) : now.send(:"prev_#{_period.time_unit}")
-        end
-
-        keys = Von.connection.hgetall("#{hash_key}:#{period}")
-        _count.map { |date| { date => keys.fetch(date, 0) }}
+        counter.count_period(period.to_sym)
       end
     end
 

lib/von/period.rb

@@ -2,19 +2,20 @@ module Von
   class Period
     AVAILABLE_PERIODS = [ :hourly, :daily, :weekly, :monthly, :yearly ]
 
-    attr_reader :counter
+    attr_reader :counter_key
     attr_reader :length
+    attr_reader :format
 
     # Initialize a Period object
     #
     # counter - the field name for the counter
     # period - the time period one of AVAILABLE_PERIODS
     # length - length of period
-    def initialize(counter, period, length)
-      @counter = counter
-      @period  = period
-      @length  = length
-      @now     = Time.now
+    def initialize(counter_key, period, length)
+      @counter_key = counter_key
+      @period      = period
+      @length      = length
+      @format      = Von.config.send(:"#{@period}_format")
     end
 
     # Returns a Symbol representing the time unit
@@ -39,24 +40,19 @@ def hours?
       @period == :hourly
     end
 
-    # Returns the String DateTime format
-    def format
-      @format ||= Von.config.send(:"#{@period}_format")
-    end
-
     # Returns the Redis hash key used for storing counts for this Period
     def hash_key
-      @hash ||= "#{Von.config.namespace}:#{@counter}:#{@period}"
+      @hash ||= "#{Von.config.namespace}:#{@counter_key}:#{@period}"
     end
 
     # Returns the Redis list key used for storing current "active" counters
     def list_key
-      @list ||= "#{Von.config.namespace}:lists:#{@counter}:#{@period}"
+      @list ||= "#{Von.config.namespace}:lists:#{@counter_key}:#{@period}"
     end
 
     # Returns the Redis field representation used for storing the count value
     def field
-      @now.strftime(format)
+      Time.now.strftime(format)
     end
   end
 end

test/counter_test.rb

@@ -12,24 +12,24 @@
   end
 
   it "increments the total counter if given a single key" do
-    Counter.new('foo').increment
+    Counter.increment('foo')
 
     @store.has_key?('von:foo').must_equal true
     @store['von:foo']['total'].must_equal 1
 
-    Counter.new('foo').increment
+    Counter.increment('foo')
     @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
+    Counter.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
 
-    Counter.new('foo:bar').increment
+    Counter.increment('foo:bar')
     @store['von:foo']['total'].must_equal 2
     @store['von:foo:bar']['total'].must_equal 2
   end
@@ -39,8 +39,8 @@
       config.counter 'foo', :monthly => 1
     end
 
-    Counter.new('foo').increment
-    Counter.new('foo').increment
+    Counter.increment('foo')
+    Counter.increment('foo')
 
     @store.has_key?('von:foo').must_equal true
     @store.has_key?('von:foo:monthly').must_equal true
@@ -54,9 +54,9 @@
       config.counter 'foo', :monthly => 1
     end
 
-    Counter.new('foo').increment
+    Counter.increment('foo')
     Timecop.freeze(Time.local(2013, 02))
-    Counter.new('foo').increment
+    Counter.increment('foo')
 
     @store.has_key?('von:foo').must_equal true
     @store.has_key?('von:foo:monthly').must_equal true
@@ -66,9 +66,9 @@
   end
 
   it "gets a total count for a counter" do
-    Counter.new('foo').increment
-    Counter.new('foo').increment
-    Counter.new('foo').increment
+    Counter.increment('foo')
+    Counter.increment('foo')
+    Counter.increment('foo')
 
     Von.count('foo').must_equal 3
   end
@@ -79,9 +79,9 @@
     end
 
     Timecop.freeze(Time.local(2013, 02, 01, 05))
-    Counter.new('foo').increment
+    Counter.increment('foo')
     Timecop.freeze(Time.local(2013, 02, 01, 07))
-    Counter.new('foo').increment
+    Counter.increment('foo')
 
     Von.count('foo').must_equal 2
 

test/period_test.rb

@@ -8,8 +8,9 @@
 
   it "intiializes given a counter, period, and length" do
     period = Period.new('foo', :monthly, 6)
-    period.counter.must_equal 'foo'
+    period.counter_key.must_equal 'foo'
     period.length.must_equal 6
+    period.format.must_equal '%Y-%m'
   end
 
   it "checks if the period is an hourly period" do