From 6e1837762f0d60edc9b6c2d92b5a9435f404173f Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sat, 17 May 2014 20:03:43 +0000 Subject: [PATCH] Deprecate ActsAsTaggableOn::Utils --- .travis.yml | 1 + .../acts_as_taggable_on/core.rb | 2 +- lib/acts_as_taggable_on/utils.rb | 28 +---- spec/acts_as_taggable_on/tag_spec.rb | 62 +++++------ spec/acts_as_taggable_on/taggable_spec.rb | 104 ++++++++---------- .../app/models/cached_model_with_array.rb | 2 +- spec/internal/app/models/models.rb | 4 +- spec/internal/db/schema.rb | 4 +- spec/support/0-helpers.rb | 32 ++++++ 9 files changed, 117 insertions(+), 122 deletions(-) create mode 100644 spec/support/0-helpers.rb diff --git a/.travis.yml b/.travis.yml index f6f86b919..009ca345a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,6 +25,7 @@ matrix: allow_failures: - gemfile: gemfiles/activerecord_edge.gemfile - rvm: rbx-2 + - rvm: ruby-head exclude: - rvm: 1.9.3 gemfile: gemfiles/activerecord_4.0.gemfile diff --git a/lib/acts_as_taggable_on/acts_as_taggable_on/core.rb b/lib/acts_as_taggable_on/acts_as_taggable_on/core.rb index 3a65f5ba3..42b65404e 100644 --- a/lib/acts_as_taggable_on/acts_as_taggable_on/core.rb +++ b/lib/acts_as_taggable_on/acts_as_taggable_on/core.rb @@ -383,7 +383,7 @@ def save_tags # Destroy old taggings: if old_tags.present? - self.taggings.not_owned.by_context(context).destroy_all(tag_id: old_tags) + taggings.not_owned.by_context(context).destroy_all(tag_id: old_tags) end # Create new taggings: diff --git a/lib/acts_as_taggable_on/utils.rb b/lib/acts_as_taggable_on/utils.rb index d4853cc11..e81f9f937 100644 --- a/lib/acts_as_taggable_on/utils.rb +++ b/lib/acts_as_taggable_on/utils.rb @@ -1,3 +1,5 @@ +# This module is deprecated and will be removed in the incoming versions + module ActsAsTaggableOn module Utils class << self @@ -10,33 +12,11 @@ def using_postgresql? connection && connection.adapter_name == 'PostgreSQL' end - def postgresql_version - if using_postgresql? - connection.execute('SHOW SERVER_VERSION').first['server_version'].to_f - end - end - - def postgresql_support_json? - postgresql_version >= 9.2 - end - - def using_sqlite? - connection && connection.adapter_name == 'SQLite' - end - def using_mysql? #We should probably use regex for mysql to support prehistoric adapters connection && connection.adapter_name == 'Mysql2' end - def using_case_insensitive_collation? - using_mysql? && connection.collation =~ /_ci\Z/ - end - - def supports_concurrency? - !using_sqlite? - end - def sha_prefix(string) Digest::SHA1.hexdigest("#{string}#{rand}")[0..6] end @@ -45,10 +25,6 @@ def active_record4? ::ActiveRecord::VERSION::MAJOR == 4 end - def active_record42? - active_record4? && ::ActiveRecord::VERSION::MINOR >= 2 - end - def like_operator using_postgresql? ? 'ILIKE' : 'LIKE' end diff --git a/spec/acts_as_taggable_on/tag_spec.rb b/spec/acts_as_taggable_on/tag_spec.rb index 2dba070b5..4826ab049 100644 --- a/spec/acts_as_taggable_on/tag_spec.rb +++ b/spec/acts_as_taggable_on/tag_spec.rb @@ -18,21 +18,19 @@ describe 'named like any' do - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? - context 'case insensitive collation and unique index on tag name' do - before(:each) do - ActsAsTaggableOn::Tag.create(name: 'Awesome') - ActsAsTaggableOn::Tag.create(name: 'epic') - end - - it 'should find both tags' do - expect(ActsAsTaggableOn::Tag.named_like_any(%w(awesome epic)).count).to eq(2) - end + context 'case insensitive collation and unique index on tag name', if: using_case_insensitive_collation? do + before(:each) do + ActsAsTaggableOn::Tag.create(name: 'Awesome') + ActsAsTaggableOn::Tag.create(name: 'epic') + end + + it 'should find both tags' do + expect(ActsAsTaggableOn::Tag.named_like_any(%w(awesome epic)).count).to eq(2) end end context 'case insensitive collation without indexes or case sensitive collation with indexes' do - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? + if using_case_insensitive_collation? include_context 'without unique index' end @@ -69,28 +67,24 @@ end end - unless ActsAsTaggableOn::Utils.using_sqlite? - describe 'find or create by unicode name' do - before(:each) do - @tag.name = 'привет' - @tag.save - end + describe 'find or create by unicode name', unless: using_sqlite? do + before(:each) do + @tag.name = 'привет' + @tag.save + end - it 'should find by name' do - expect(ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('привет')).to eq(@tag) - end + it 'should find by name' do + expect(ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('привет')).to eq(@tag) + end - it 'should find by name case insensitive' do - expect(ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('ПРИВЕТ')).to eq(@tag) - end + it 'should find by name case insensitive' do + expect(ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('ПРИВЕТ')).to eq(@tag) + end - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? - it 'should find by name accent insensitive' do - @tag.name = 'inupiat' - @tag.save - expect(ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('Iñupiat')).to eq(@tag) - end - end + it 'should find by name accent insensitive', if: using_case_insensitive_collation? do + @tag.name = 'inupiat' + @tag.save + expect(ActsAsTaggableOn::Tag.find_or_create_with_like_by_name('Iñupiat')).to eq(@tag) end end @@ -109,7 +103,7 @@ end context 'case sensitive' do - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? + if using_case_insensitive_collation? include_context 'without unique index' end @@ -128,7 +122,7 @@ end context 'case sensitive' do - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? + if using_case_insensitive_collation? include_context 'without unique index' end @@ -228,7 +222,7 @@ end context 'case sensitive' do - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? + if using_case_insensitive_collation? include_context 'without unique index' end @@ -242,7 +236,7 @@ end context 'case sensitive' do - if ActsAsTaggableOn::Utils.using_case_insensitive_collation? + if using_case_insensitive_collation? include_context 'without unique index' end diff --git a/spec/acts_as_taggable_on/taggable_spec.rb b/spec/acts_as_taggable_on/taggable_spec.rb index 13f48c590..2b50ebefb 100644 --- a/spec/acts_as_taggable_on/taggable_spec.rb +++ b/spec/acts_as_taggable_on/taggable_spec.rb @@ -255,16 +255,14 @@ expect(TaggableModel.select('distinct(taggable_models.id), taggable_models.*').joins(:untaggable_models).tagged_with(['rails', 'ruby'], :any => false).to_a.sort).to eq([bob, frank].sort) end - unless ActsAsTaggableOn::Utils.using_sqlite? - it 'should not care about case for unicode names' do - ActsAsTaggableOn.strict_case_match = false - TaggableModel.create(name: 'Anya', tag_list: 'ПРИВЕТ') - TaggableModel.create(name: 'Igor', tag_list: 'привет') - TaggableModel.create(name: 'Katia', tag_list: 'ПРИВЕТ') - - expect(ActsAsTaggableOn::Tag.all.size).to eq(1) - expect(TaggableModel.tagged_with('привет').to_a).to eq(TaggableModel.tagged_with('ПРИВЕТ').to_a) - end + it 'should not care about case for unicode names', unless: using_sqlite? do + ActsAsTaggableOn.strict_case_match = false + TaggableModel.create(name: 'Anya', tag_list: 'ПРИВЕТ') + TaggableModel.create(name: 'Igor', tag_list: 'привет') + TaggableModel.create(name: 'Katia', tag_list: 'ПРИВЕТ') + + expect(ActsAsTaggableOn::Tag.all.size).to eq(1) + expect(TaggableModel.tagged_with('привет').to_a).to eq(TaggableModel.tagged_with('ПРИВЕТ').to_a) end context 'should be able to create and find tags in languages without capitalization :' do @@ -586,28 +584,26 @@ end - if ActsAsTaggableOn::Utils.supports_concurrency? - xit 'should not duplicate tags added on different threads' do - #TODO, try with more threads and fix deadlock - thread_count = 4 - barrier = Barrier.new thread_count - - expect { - thread_count.times.map do |idx| - Thread.start do - connor = TaggableModel.first_or_create(name: 'Connor') - connor.tag_list = 'There, can, be, only, one' - barrier.wait - begin - connor.save - rescue ActsAsTaggableOn::DuplicateTagError - # second save should succeed - connor.save - end + xit 'should not duplicate tags added on different threads', if: supports_concurrency?, skip: 'FIXME, Deadlocks in travis' do + #TODO, try with more threads and fix deadlock + thread_count = 4 + barrier = Barrier.new thread_count + + expect { + thread_count.times.map do |idx| + Thread.start do + connor = TaggableModel.first_or_create(name: 'Connor') + connor.tag_list = 'There, can, be, only, one' + barrier.wait + begin + connor.save + rescue ActsAsTaggableOn::DuplicateTagError + # second save should succeed + connor.save end - end.map(&:join) - }.to change(ActsAsTaggableOn::Tag, :count).by(5) - end + end + end.map(&:join) + }.to change(ActsAsTaggableOn::Tag, :count).by(5) end end @@ -853,34 +849,30 @@ end end -if ActsAsTaggableOn::Utils.using_postgresql? - if ActsAsTaggableOn::Utils.postgresql_support_json? - describe 'Taggable model with json columns' do - before(:each) do - @taggable = TaggableModelWithJson.new(:name => 'Bob Jones') - @taggables = [@taggable, TaggableModelWithJson.new(:name => 'John Doe')] - end +describe 'Taggable model with json columns', if: postgresql_support_json? do + before(:each) do + @taggable = TaggableModelWithJson.new(:name => 'Bob Jones') + @taggables = [@taggable, TaggableModelWithJson.new(:name => 'John Doe')] + end - it 'should be able to find by tag with context' do - @taggable.skill_list = 'ruby, rails, css' - @taggable.tag_list = 'bob, charlie' - @taggable.save + it 'should be able to find by tag with context' do + @taggable.skill_list = 'ruby, rails, css' + @taggable.tag_list = 'bob, charlie' + @taggable.save - expect(TaggableModelWithJson.tagged_with('ruby').first).to eq(@taggable) - expect(TaggableModelWithJson.tagged_with('ruby, css').first).to eq(@taggable) - expect(TaggableModelWithJson.tagged_with('bob', :on => :skills).first).to_not eq(@taggable) - expect(TaggableModelWithJson.tagged_with('bob', :on => :tags).first).to eq(@taggable) - end + expect(TaggableModelWithJson.tagged_with('ruby').first).to eq(@taggable) + expect(TaggableModelWithJson.tagged_with('ruby, css').first).to eq(@taggable) + expect(TaggableModelWithJson.tagged_with('bob', :on => :skills).first).to_not eq(@taggable) + expect(TaggableModelWithJson.tagged_with('bob', :on => :tags).first).to eq(@taggable) + end - it 'should be able to find tagged with any tag' do - bob = TaggableModelWithJson.create(:name => 'Bob', :tag_list => 'fitter, happier, more productive', :skill_list => 'ruby, rails, css') - frank = TaggableModelWithJson.create(:name => 'Frank', :tag_list => 'weaker, depressed, inefficient', :skill_list => 'ruby, rails, css') - steve = TaggableModelWithJson.create(:name => 'Steve', :tag_list => 'fitter, happier, more productive', :skill_list => 'c++, java, ruby') + it 'should be able to find tagged with any tag' do + bob = TaggableModelWithJson.create(:name => 'Bob', :tag_list => 'fitter, happier, more productive', :skill_list => 'ruby, rails, css') + frank = TaggableModelWithJson.create(:name => 'Frank', :tag_list => 'weaker, depressed, inefficient', :skill_list => 'ruby, rails, css') + steve = TaggableModelWithJson.create(:name => 'Steve', :tag_list => 'fitter, happier, more productive', :skill_list => 'c++, java, ruby') - expect(TaggableModelWithJson.tagged_with(%w(ruby java), :order => 'taggable_model_with_jsons.name', :any => true).to_a).to eq([bob, frank, steve]) - expect(TaggableModelWithJson.tagged_with(%w(c++ fitter), :order => 'taggable_model_with_jsons.name', :any => true).to_a).to eq([bob, steve]) - expect(TaggableModelWithJson.tagged_with(%w(depressed css), :order => 'taggable_model_with_jsons.name', :any => true).to_a).to eq([bob, frank]) - end - end + expect(TaggableModelWithJson.tagged_with(%w(ruby java), :order => 'taggable_model_with_jsons.name', :any => true).to_a).to eq([bob, frank, steve]) + expect(TaggableModelWithJson.tagged_with(%w(c++ fitter), :order => 'taggable_model_with_jsons.name', :any => true).to_a).to eq([bob, steve]) + expect(TaggableModelWithJson.tagged_with(%w(depressed css), :order => 'taggable_model_with_jsons.name', :any => true).to_a).to eq([bob, frank]) end end diff --git a/spec/internal/app/models/cached_model_with_array.rb b/spec/internal/app/models/cached_model_with_array.rb index c19e1dd5b..116cf0020 100644 --- a/spec/internal/app/models/cached_model_with_array.rb +++ b/spec/internal/app/models/cached_model_with_array.rb @@ -1,4 +1,4 @@ -if ActsAsTaggableOn::Utils.using_postgresql? +if using_postgresql? class CachedModelWithArray < ActiveRecord::Base acts_as_taggable end diff --git a/spec/internal/app/models/models.rb b/spec/internal/app/models/models.rb index d28b1f24d..74927c980 100644 --- a/spec/internal/app/models/models.rb +++ b/spec/internal/app/models/models.rb @@ -77,11 +77,11 @@ class OrderedTaggableModel < ActiveRecord::Base acts_as_ordered_taggable_on :colours end -if ActsAsTaggableOn::Utils.using_postgresql? +if using_postgresql? class CachedModelWithArray < ActiveRecord::Base acts_as_taggable end - if ActsAsTaggableOn::Utils.postgresql_support_json? + if postgresql_support_json? class TaggableModelWithJson < ActiveRecord::Base acts_as_taggable acts_as_taggable_on :skills diff --git a/spec/internal/db/schema.rb b/spec/internal/db/schema.rb index f8e656df8..e44fd5e43 100644 --- a/spec/internal/db/schema.rb +++ b/spec/internal/db/schema.rb @@ -76,7 +76,7 @@ # Special cases for postgresql - if ActsAsTaggableOn::Utils.using_postgresql? + if using_postgresql? create_table :other_cached_with_array_models, force: true do |t| t.column :name, :string @@ -86,7 +86,7 @@ t.column :cached_glass_list, :string, array: true end - if ActsAsTaggableOn::Utils.postgresql_support_json? + if postgresql_support_json? create_table :taggable_model_with_jsons, :force => true do |t| t.column :name, :string t.column :type, :string diff --git a/spec/support/0-helpers.rb b/spec/support/0-helpers.rb new file mode 100644 index 000000000..abbf5db1c --- /dev/null +++ b/spec/support/0-helpers.rb @@ -0,0 +1,32 @@ +def using_sqlite? + ActsAsTaggableOn::Utils.connection && ActsAsTaggableOn::Utils.connection.adapter_name == 'SQLite' +end + +def supports_concurrency? + !using_sqlite? +end + +def using_postgresql? + ActsAsTaggableOn::Utils.using_postgresql? +end + +def postgresql_version + if using_postgresql? + ActsAsTaggableOn::Utils.connection.execute('SHOW SERVER_VERSION').first['server_version'].to_f + else + 0.0 + end +end + +def postgresql_support_json? + postgresql_version >= 9.2 +end + + +def using_mysql? + ActsAsTaggableOn::Utils.using_mysql? +end + +def using_case_insensitive_collation? + using_mysql? && ActsAsTaggableOn::Utils.connection.collation =~ /_ci\Z/ +end