Skip to content

Commit

Permalink
Merge pull request #438 from bf4/fix_cached_tag_columns_check
Browse files Browse the repository at this point in the history
[#432][#403][#433][#422] Fix checking for column cache until db connects
  • Loading branch information
bf4 committed Dec 22, 2013
2 parents 6ad1eb6 + b433baa commit 5f87304
Showing 1 changed file with 48 additions and 19 deletions.
67 changes: 48 additions & 19 deletions lib/acts_as_taggable_on/acts_as_taggable_on/cache.rb
Original file line number Diff line number Diff line change
@@ -1,41 +1,69 @@
module ActsAsTaggableOn::Taggable
module Cache
def self.included(base)
# Skip adding caching capabilities if table not exists or no cache columns exist
return unless base.connected? && base.table_exists? && base.tag_types.any? { |context| base.column_names.include?("cached_#{context.to_s.singularize}_list") }

base.send :include, ActsAsTaggableOn::Taggable::Cache::InstanceMethods
base.extend ActsAsTaggableOn::Taggable::Cache::ClassMethods

base.class_eval do
before_save :save_cached_tag_list
# When included, conditionally adds tag caching methods when the model
# has any "cached_#{tag_type}_list" column
base.instance_eval do
# @private
def _has_acts_as_taggable_on_cache_columns?(db_columns)

This comment has been minimized.

Copy link
@seuros

seuros Dec 23, 2013

Collaborator

Method name is too long

This comment has been minimized.

Copy link
@bf4

bf4 Dec 23, 2013

Author Collaborator

suggestion?

db_column_names = db_columns.map(&:name)
tag_types.any? {|context|
db_column_names.include?("cached_#{context.to_s.singularize}_list")
}
end

# @private
def _add_acts_as_taggable_on_caching_methods

This comment has been minimized.

Copy link
@seuros

seuros Dec 23, 2013

Collaborator

Method name is too long

This comment has been minimized.

Copy link
@bf4

bf4 Dec 23, 2013

Author Collaborator

suggestion?

send :include, ActsAsTaggableOn::Taggable::Cache::InstanceMethods
extend ActsAsTaggableOn::Taggable::Cache::ClassMethods

before_save :save_cached_tag_list

initialize_acts_as_taggable_on_cache
end

# ActiveRecord::Base.columns makes a database connection and caches the calculated
# columns hash for the record as @columns. Since we don't want to add caching
# methods until we confirm the presence of a caching column, and we don't
# want to force opening a database connection when the class is loaded,
# here we intercept and cache the call to :columns as @acts_as_taggable_on_columns
# to mimic the underlying behavior. While processing this first call to columns,
# we do the caching column check and dynamically add the class and instance methods
def columns
@acts_as_taggable_on_columns ||= begin
db_columns = super
if _has_acts_as_taggable_on_cache_columns?(db_columns)
_add_acts_as_taggable_on_caching_methods
end
db_columns
end
end

end

base.initialize_acts_as_taggable_on_cache
end

module ClassMethods
def initialize_acts_as_taggable_on_cache
def initialize_acts_as_taggable_on_cache

This comment has been minimized.

Copy link
@seuros

seuros Dec 23, 2013

Collaborator

Method name is too long

This comment has been minimized.

Copy link
@bf4

bf4 Dec 23, 2013

Author Collaborator

suggestion? (comments below okay)

tag_types.map(&:to_s).each do |tag_type|
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def self.caching_#{tag_type.singularize}_list?
caching_tag_list_on?("#{tag_type}")
end
end
RUBY
end
end
end

def acts_as_taggable_on(*args)
super(*args)
initialize_acts_as_taggable_on_cache
end

def caching_tag_list_on?(context)
column_names.include?("cached_#{context.to_s.singularize}_list")
end
end
module InstanceMethods

module InstanceMethods
def save_cached_tag_list
tag_types.map(&:to_s).each do |tag_type|
if self.class.send("caching_#{tag_type.singularize}_list?")
Expand All @@ -45,9 +73,10 @@ def save_cached_tag_list
end
end
end

true
end
end

end
end

5 comments on commit 5f87304

@seuros
Copy link
Collaborator

@seuros seuros commented on 5f87304 Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could change the names to :

   _has_tags_cache_columns? 
   _add_tags_caching_methods 
   initialize_tags_cache

@bf4
Copy link
Collaborator Author

@bf4 bf4 commented on 5f87304 Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to namespace it to prevent possible conflicts with other libraries....

@seuros
Copy link
Collaborator

@seuros seuros commented on 5f87304 Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nobody will/should include 2 tagging libraries in the same model.

@bf4
Copy link
Collaborator Author

@bf4 bf4 commented on 5f87304 Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR?

@seuros
Copy link
Collaborator

@seuros seuros commented on 5f87304 Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, i forgot to send it : #442

Please sign in to comment.