Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caching on first save for an unused model #928

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ As such, _Breaking Changes_ are major. _Features_ would map to either major or m
* [@mizukami234 @junmoka Make table names configurable](https://github.com/mbleigh/acts-as-taggable-on/pull/910)
* Fixes
* [@tonyta Avoid overriding user-defined columns cache methods](https://github.com/mbleigh/acts-as-taggable-on/pull/911)
* [@kratob Fix caching on first save for an unused model](https://github.com/mbleigh/acts-as-taggable-on/pull/928)
* Misc
* [@gssbzn Remove legacy code for an empty query and replace it with ` ActiveRecord::none`](https://github.com/mbleigh/acts-as-taggable-on/pull/906)
* Documentation
Expand Down
40 changes: 14 additions & 26 deletions lib/acts_as_taggable_on/taggable/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,28 @@ module Cache
def self.included(base)
# When included, conditionally adds tag caching methods when the model
# has any "cached_#{tag_type}_list" column
base.extend Columns
base.extend LoadSchema
end

module Columns
# 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_cache_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
# FIXME: this method cannot compile in rubinius
def columns
@acts_as_taggable_on_cache_columns ||= begin
db_columns = super
_add_tags_caching_methods if _has_tags_cache_columns?(db_columns)
db_columns
end
end
module LoadSchema
private

def reset_column_information
# @private
# ActiveRecord::Base.load_schema! makes a database connection and caches the
# calculated columns hash for the record as @column_hashs. 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,
# we override load_schema!, do the caching column check and dynamically
# add the class and instance methods.
def load_schema!
super
@acts_as_taggable_on_cache_columns = nil
_add_tags_caching_methods if _has_tags_cache_columns?
end

private

# @private
def _has_tags_cache_columns?(db_columns)
db_column_names = db_columns.map(&:name)
def _has_tags_cache_columns?
tag_types.any? do |context|
db_column_names.include?("cached_#{context.to_s.singularize}_list")
@columns_hash.has_key?("cached_#{context.to_s.singularize}_list")
end
end

Expand Down
8 changes: 8 additions & 0 deletions spec/acts_as_taggable_on/caching_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@
expect(@another_taggable.cached_language_list).to eq('ruby, .net')
end

it 'should cache tags when the model is freshly loaded' do
taggable = SingleUseCachedModel.new
taggable.tag_list = 'awesome, epic'
taggable.save!

expect(taggable.cached_tag_list).to eq('awesome, epic')
end

it 'should keep the cache' do
@taggable.update_attributes(tag_list: 'awesome, epic')
@taggable = CachedModel.find(@taggable.id)
Expand Down
5 changes: 5 additions & 0 deletions spec/internal/app/models/single_use_cached_model.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# This is used in a spec that expects this model not to have been used before.
class SingleUseCachedModel < ActiveRecord::Base
self.table_name = 'cached_models'
acts_as_taggable
end