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

Deprecate ActsAsTaggableOn::Utils #541

Merged
merged 1 commit into from
May 18, 2014
Merged
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 .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/acts_as_taggable_on/acts_as_taggable_on/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
28 changes: 2 additions & 26 deletions lib/acts_as_taggable_on/utils.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# This module is deprecated and will be removed in the incoming versions

module ActsAsTaggableOn
module Utils
class << self
Expand All @@ -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
Expand All @@ -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
Expand Down
62 changes: 28 additions & 34 deletions spec/acts_as_taggable_on/tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
104 changes: 48 additions & 56 deletions spec/acts_as_taggable_on/taggable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion spec/internal/app/models/cached_model_with_array.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
if ActsAsTaggableOn::Utils.using_postgresql?
if using_postgresql?
class CachedModelWithArray < ActiveRecord::Base
acts_as_taggable
end
Expand Down
4 changes: 2 additions & 2 deletions spec/internal/app/models/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/internal/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
32 changes: 32 additions & 0 deletions spec/support/0-helpers.rb
Original file line number Diff line number Diff line change
@@ -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