Merge pull request #541 from seuros/deprecate-utils

Deprecate ActsAsTaggableOn::Utils

.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

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:

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

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
 

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

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

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

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

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