diff --git a/app/models/impressionist/impressionable.rb b/app/models/impressionist/impressionable.rb index 4b23b4b..2402e66 100644 --- a/app/models/impressionist/impressionable.rb +++ b/app/models/impressionist/impressionable.rb @@ -12,7 +12,7 @@ module Impressionist @impressionist_cache_options.reverse_merge!(DEFAULT_CACHE) end - # true or false + # asks impressionable entity whether or not it is counter_caching def impressionist_counter_caching? impressionist_counter_cache_options[:counter_cache] end @@ -22,7 +22,7 @@ module Impressionist impressionist_counter_caching? end - end + end # end of ClassMethods # ------------------------------------------ # TODO: CLEAN UP, make it HUMAN readable diff --git a/lib/impressionist/counter_cache.rb b/lib/impressionist/counter_cache.rb index d1d2898..73ef9af 100644 --- a/lib/impressionist/counter_cache.rb +++ b/lib/impressionist/counter_cache.rb @@ -1,55 +1,34 @@ +# Note +# It is only updatable if +# impressionist_id && impressionist_type(class) are present && +# impressionable_class(which is imp_type.constantize) is counter_caching +# Defined like so +# is_impressionable :counter_cache => true + module Impressionist module CounterCache attr_reader :impressionable_class, :entity private - LOG_MESSAGE = "Can't find impressionable_type or impressionable_id. Will not update_counters!" # if updatable returns true, it must be qualified to update_counters - # Therefore there's no need to validate again # impressionable_class instance var is set when updatable? is called def impressionable_counter_cache_updatable? - updatable? ? update_impression_cache : impressionist_log(LOG_MESSAGE) + updatable? && impressionable_try end - def update_impression_cache - impressionable_find - impressionable_try - end - - # asks imp_id whether it's present or not - # also expect imp_class to be true - # all should be true, so that it is updatable + # asks imp_id && imp_class whether it's present or not + # so that it is updatable def updatable? @impressionable_class = impressionable_class_set impressionable_valid? end - # imps_type == nil, constantize returns Object - # Therefore it attemps to return false so it won't be updatable - # calls to_s otherwise it would try to constantize nil - # and it would raise an exeception.. - def impressionable_class_set - _type_ = self.impressionable_type.to_s.constantize - ((_type_.to_s !~ /Object/) && _type_.impressionist_counter_caching? ? _type_ : false) - end - - # Either true or false - # needs true to be updatable def impressionable_valid? - (self.impressionable_id.present? && impressionable_class && impressionable_find) + (self.impressionable_id.present? && impressionable_class) end - # Logs to log file, expects a message to be passed - - # default mode is ERROR - # ruby 1.8.7 support - def impressionist_log(str, mode=:error) - Rails.logger.send(mode.to_s, str) - end - - # read it out and LOUD def impressionable_find exeception_rescuer { @entity = impressionable_class.find(self.impressionable_id) @@ -58,10 +37,27 @@ module Impressionist end + # imps_type == nil, constantize returns Object + # It attemps to return false so it won't be updatable + # calls to_s otherwise it would try to constantize nil + # and it would raise an exeception + # Must be !~ Object and be counter_caching to be qualified as updatable + def impressionable_class_set + klass = self.impressionable_type.to_s.constantize + (klass.to_s !~ /Object/) && klass.impressionist_counter_caching? ? klass : false + end + + # default mode is ERROR + # ruby 1.8.7 support + def impressionist_log(str, mode=:error) + Rails.logger.send(mode.to_s, str) + end + # receives an entity(instance of a Model) and then tries to update # counter_cache column - # entity is a impressionable_model + # entity is a impressionable instance model def impressionable_try + impressionable_find entity.try(:update_impressionist_counter_cache) end diff --git a/lib/impressionist/is_impressionable.rb b/lib/impressionist/is_impressionable.rb new file mode 100644 index 0000000..9fdf550 --- /dev/null +++ b/lib/impressionist/is_impressionable.rb @@ -0,0 +1,23 @@ +module Impressionist + module IsImpressionable + extend ActiveSupport::Concern + + module ClassMethods + def is_impressionable(options={}) + define_association + @impressionist_cache_options = options + + true + end + + private + + def define_association + has_many(:impressions, + :as => :impressionable, + :dependent => :destroy) + end + end + + end +end diff --git a/lib/impressionist/load.rb b/lib/impressionist/load.rb index d046ba6..c97de48 100644 --- a/lib/impressionist/load.rb +++ b/lib/impressionist/load.rb @@ -1,5 +1,3 @@ -require 'impressionist/engine' - require 'impressionist/setup_association' require 'impressionist/counter_cache' @@ -7,3 +5,7 @@ require 'impressionist/counter_cache' require 'impressionist/update_counters' require 'impressionist/rails_toggle' + +require 'impressionist/is_impressionable' + +require 'impressionist/engine' diff --git a/lib/impressionist/models/active_record/impression.rb b/lib/impressionist/models/active_record/impression.rb index 252a9cc..b32a8b8 100644 --- a/lib/impressionist/models/active_record/impression.rb +++ b/lib/impressionist/models/active_record/impression.rb @@ -1,14 +1,12 @@ # Responsability -## See CounterCache TODO: extract it into a class -# * be able to update_counters -# * log an error if imps_id and imps_type can not be found +# * logs an error if imps_id and imps_type can not be found # * asks updatable? whether it may or may not be updated -# FIX exeception raising when no imps_id is found class Impression < ActiveRecord::Base include Impressionist::CounterCache - # sets belongs_to and attr_accessible + + # sets belongs_to and attr_accessible depending on Rails version Impressionist::SetupAssociation.new(self).set after_save :impressionable_counter_cache_updatable? diff --git a/lib/impressionist/models/active_record/impressionist/impressionable.rb b/lib/impressionist/models/active_record/impressionist/impressionable.rb index 053281c..345e64b 100644 --- a/lib/impressionist/models/active_record/impressionist/impressionable.rb +++ b/lib/impressionist/models/active_record/impressionist/impressionable.rb @@ -1,26 +1,12 @@ -ActiveRecord::Base.send(:include, Impressionist::Impressionable) - module Impressionist + module Impressionable - extend ActiveSupport::Concern - - module ClassMethods - - def is_impressionable(options={}) - define_association - - @impressionist_cache_options = options - end - -private - def define_association - has_many(:impressions, - :as => :impressionable, - :dependent => :destroy) - end - - end + # extends AS::Concern + include Impressionist::IsImpressionable end end + +ActiveRecord::Base. +send(:include, Impressionist::Impressionable) diff --git a/lib/impressionist/models/mongoid/impressionist/impressionable.rb b/lib/impressionist/models/mongoid/impressionist/impressionable.rb index 4802e0a..2bacfd1 100644 --- a/lib/impressionist/models/mongoid/impressionist/impressionable.rb +++ b/lib/impressionist/models/mongoid/impressionist/impressionable.rb @@ -1,31 +1,13 @@ -# TODO: Refactor this Entity -# There's a lot of duplication -Mongoid::Document.send(:include, Impressionist::Impressionable) - module Impressionist module Impressionable - extend ActiveSupport::Concern - module ClassMethods + # extends AS::Concern + include Impressionist::IsImpressionable - def is_impressionable(options={}) - define_association - @impressionist_cache_options = options + ## TODO: Make it readable - true - end - - private - def define_association - has_many(:impressions, - :as => :impressionable, - :dependent => :destroy) - end - - end - - ## - # Overides active_record impressionist_count + # Overides impressionist_count in order to provied + # mongoid compability def impressionist_count(options={}) options.reverse_merge!(:filter=>:request_hash, :start_date=>nil, :end_date=>Time.now) imps = options[:start_date].blank? ? impressions : impressions.between(created_at: options[:start_date]..options[:end_date]) @@ -34,5 +16,7 @@ module Impressionist end end - end + +Mongoid::Document. +send(:include, Impressionist::Impressionable) diff --git a/lib/impressionist/update_counters.rb b/lib/impressionist/update_counters.rb index 3b4a445..bdfaeb5 100644 --- a/lib/impressionist/update_counters.rb +++ b/lib/impressionist/update_counters.rb @@ -1,62 +1,68 @@ +# Note +# If impressionist_counter_cache_options[:counter_cache] is false(default) +# it won't event run this class module Impressionist + class UpdateCounters - attr_reader :receiver, :master + attr_reader :receiver, :klass def initialize(receiver) @receiver = receiver - @master = receiver.class + @klass = receiver.class end def update - result = (impressions_total - impressions_cached) - - master. + klass. update_counters(id, column_name => result) end private - def id - receiver.id - end - # if unique == true then uses it - # otherwise just count all impressions - # using filter: :all - def impressions_total - receiver.impressionist_count filter(unique_filter) - end + def result + impressions_total - impressions_cached + end - # from a given db column - # default should be impressions_count - def impressions_cached - receiver.send(column_name) || 0 - end + # Count impressions based on unique_filter + # default is :ip_address when unique: true + def impressions_total + receiver.impressionist_count filter + end - def column_name - cache_options[:column_name].to_sym - end + # Fetch impressions from a receiver's column + def impressions_cached + receiver.send(column_name) || 0 + end - def cache_options - master. - impressionist_counter_cache_options - end + def filter + {:filter => unique_filter} + end - # default is ip_address if what_is_unique is TRUE or FALSE - def unique_filter - what_is_unique? ? :ip_address : cache_options[:unique] - end + # :filter gets assigned to :ip_address as default + # One could do + # is_impressionable :counter_cache => true, + # :unique => :any_other_filter + def unique_filter + Symbol === unique ? + unique : + :ip_address + end - # Either true or false - # :filter gets assigned to :ip_address as default - # One could do - # is_impressionable :counter_cache => true, :unique => :any_other_colum - def what_is_unique? - cache_options[:unique].to_s =~ /true|false/ - end + def unique + cache_options[:unique] + end - def filter(filter_will_be) - {:filter => filter_will_be.to_sym} - end + def column_name + cache_options[:column_name].to_s + end + + def cache_options + klass. + impressionist_counter_cache_options + end + + def id + receiver.id + end end diff --git a/tests/test_app/config/initializers/impression.rb b/tests/test_app/config/initializers/impression.rb index 5de89e4..34d8140 100644 --- a/tests/test_app/config/initializers/impression.rb +++ b/tests/test_app/config/initializers/impression.rb @@ -1,5 +1,8 @@ # Use this hook to configure impressionist parameters -Impressionist.setup do |config| +#Impressionist.setup do |config| # Define ORM. Could be :active_record (default), :mongo_mapper or :mongoid # config.orm = :active_record -end +#end + + +Impressionist.orm = :active_record diff --git a/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb b/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb new file mode 100644 index 0000000..c874863 --- /dev/null +++ b/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb @@ -0,0 +1,30 @@ +class CreateImpressionsTable < ActiveRecord::Migration + def self.up + create_table :impressions, :force => true do |t| + t.string :impressionable_type + t.integer :impressionable_id + t.integer :user_id + t.string :controller_name + t.string :action_name + t.string :view_name + t.string :request_hash + t.string :ip_address + t.string :session_hash + t.text :message + t.text :referrer + t.timestamps + end + add_index :impressions, [:impressionable_type, :message, :impressionable_id], :name => "impressionable_type_message_index", :unique => false, :length => {:message => 255 } + add_index :impressions, [:impressionable_type, :impressionable_id, :request_hash], :name => "poly_request_index", :unique => false + add_index :impressions, [:impressionable_type, :impressionable_id, :ip_address], :name => "poly_ip_index", :unique => false + add_index :impressions, [:impressionable_type, :impressionable_id, :session_hash], :name => "poly_session_index", :unique => false + add_index :impressions, [:controller_name,:action_name,:request_hash], :name => "controlleraction_request_index", :unique => false + add_index :impressions, [:controller_name,:action_name,:ip_address], :name => "controlleraction_ip_index", :unique => false + add_index :impressions, [:controller_name,:action_name,:session_hash], :name => "controlleraction_session_index", :unique => false + add_index :impressions, :user_id + end + + def self.down + drop_table :impressions + end +end