From 62cc63a7df10d185169c067161f33f4965cd6fd3 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 5 Aug 2013 09:15:28 -0500 Subject: [PATCH 1/8] Rails needs to be loaded to meet all gem dependencies --- lib/acts-as-taggable-on.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/acts-as-taggable-on.rb b/lib/acts-as-taggable-on.rb index 7a6e116c2..7deaafc45 100644 --- a/lib/acts-as-taggable-on.rb +++ b/lib/acts-as-taggable-on.rb @@ -1,6 +1,7 @@ require "active_record" require "active_record/version" require "action_view" +require 'rails' require "digest/sha1" From 41c16f05eede168904d02a6a1406da2d31e1a46f Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 5 Aug 2013 09:19:09 -0500 Subject: [PATCH 2/8] No need to monkey with load path in gem, add to tests --- lib/acts-as-taggable-on.rb | 4 ---- spec/spec_helper.rb | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/acts-as-taggable-on.rb b/lib/acts-as-taggable-on.rb index 7deaafc45..6f722b3c4 100644 --- a/lib/acts-as-taggable-on.rb +++ b/lib/acts-as-taggable-on.rb @@ -5,8 +5,6 @@ require "digest/sha1" -$LOAD_PATH.unshift(File.dirname(__FILE__)) - module ActsAsTaggableOn mattr_accessor :delimiter @@delimiter = ',' @@ -51,8 +49,6 @@ def self.setup require "acts_as_taggable_on/tags_helper" require "acts_as_taggable_on/tagging" -$LOAD_PATH.shift - if defined?(ActiveRecord::Base) ActiveRecord::Base.extend ActsAsTaggableOn::Compatibility diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 02b1f1dbc..493538bb1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,4 +1,5 @@ $LOAD_PATH << "." unless $LOAD_PATH.include?(".") +$LOAD_PATH.unshift(File.expand_path('../../lib', __FILE__)) require 'logger' begin From 4c77ac6828e2554911f3ecbb5f9ca54e7e2afa02 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 5 Aug 2013 09:20:27 -0500 Subject: [PATCH 3/8] Use ActiveSupport hooks to lazy-extend/include rails classes --- lib/acts-as-taggable-on.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/acts-as-taggable-on.rb b/lib/acts-as-taggable-on.rb index 6f722b3c4..0e22128d1 100644 --- a/lib/acts-as-taggable-on.rb +++ b/lib/acts-as-taggable-on.rb @@ -49,14 +49,12 @@ def self.setup require "acts_as_taggable_on/tags_helper" require "acts_as_taggable_on/tagging" - -if defined?(ActiveRecord::Base) - ActiveRecord::Base.extend ActsAsTaggableOn::Compatibility - ActiveRecord::Base.extend ActsAsTaggableOn::Taggable - ActiveRecord::Base.send :include, ActsAsTaggableOn::Tagger +ActiveSupport.on_load(:active_record) do + extend ActsAsTaggableOn::Compatibility + extend ActsAsTaggableOn::Taggable + include ActsAsTaggableOn::Tagger end - -if defined?(ActionView::Base) - ActionView::Base.send :include, ActsAsTaggableOn::TagsHelper +ActiveSupport.on_load(:action_view) do + include ActsAsTaggableOn::TagsHelper end From c8dd916118ba05a4b5c1b07ff9d29f4a9ba4ffc5 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 9 Dec 2013 17:15:21 -0600 Subject: [PATCH 4/8] Only ActiveSupport actually required, not Rails gem --- lib/acts-as-taggable-on.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/acts-as-taggable-on.rb b/lib/acts-as-taggable-on.rb index 0e22128d1..25eb9dd5b 100644 --- a/lib/acts-as-taggable-on.rb +++ b/lib/acts-as-taggable-on.rb @@ -1,7 +1,7 @@ require "active_record" require "active_record/version" require "action_view" -require 'rails' +require 'active_support/all' require "digest/sha1" From d074e203849ed30f29dcad94556b28a722e18ab9 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 9 Dec 2013 17:38:04 -0600 Subject: [PATCH 5/8] Update test schema.rb to match current migration However, set force: true and didn't set indices Fixes test failures for postgresql:: No integer type has byte size 11. Use a numeric with precision 0 instead. https://travis-ci.org/mbleigh/acts-as-taggable-on/jobs/15196579 see https://github.com/rails/rails/issues/6272 --- spec/schema.rb | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/spec/schema.rb b/spec/schema.rb index 4949f0107..2d2e27453 100644 --- a/spec/schema.rb +++ b/spec/schema.rb @@ -1,59 +1,63 @@ ActiveRecord::Schema.define :version => 0 do - create_table "taggings", :force => true do |t| - t.integer "tag_id", :limit => 11 - t.integer "taggable_id", :limit => 11 - t.string "taggable_type" - t.string "context" - t.datetime "created_at" - t.integer "tagger_id", :limit => 11 - t.string "tagger_type" + create_table :tags, :force => true do |t| + t.string :name end - add_index "taggings", ["tag_id"], :name => "index_taggings_on_tag_id" - add_index "taggings", ["taggable_id", "taggable_type", "context"], :name => "index_taggings_on_taggable_id_and_taggable_type_and_context" + create_table :taggings, :force => true do |t| + t.references :tag - create_table "tags", :force => true do |t| - t.string "name" + # You should make sure that the column created is + # long enough to store the required class names. + t.references :taggable, :polymorphic => true + t.references :tagger, :polymorphic => true + + # Limit is created to prevent MySQL error on index + # length for MyISAM table type: http://bit.ly/vgW2Ql + t.string :context, :limit => 128 + + t.datetime :created_at end - + # above copied from + # generators/acts_as_taggable_on/migration/migration_generator + create_table :taggable_models, :force => true do |t| t.column :name, :string t.column :type, :string end - + create_table :non_standard_id_taggable_models, :primary_key => "an_id", :force => true do |t| t.column :name, :string t.column :type, :string end - + create_table :untaggable_models, :force => true do |t| t.column :taggable_model_id, :integer t.column :name, :string end - + create_table :cached_models, :force => true do |t| t.column :name, :string t.column :type, :string t.column :cached_tag_list, :string end - + create_table :other_cached_models, :force => true do |t| t.column :name, :string t.column :type, :string - t.column :cached_language_list, :string + t.column :cached_language_list, :string t.column :cached_status_list, :string t.column :cached_glass_list, :string end - + create_table :users, :force => true do |t| t.column :name, :string end - + create_table :other_taggable_models, :force => true do |t| t.column :name, :string t.column :type, :string end - + create_table :ordered_taggable_models, :force => true do |t| t.column :name, :string t.column :type, :string From 1c34142738cd847966053a87c96e599588d09a10 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 9 Dec 2013 17:43:47 -0600 Subject: [PATCH 6/8] Add .travis.yml bundle cache --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 64c9be4f0..65369cc8a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,3 +5,4 @@ env: - DB=sqlite3 - DB=mysql - DB=postgresql +cache: bundler From 801e1530a87b81042cdecb0a88e94e53b707534a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 9 Dec 2013 22:16:10 -0600 Subject: [PATCH 7/8] Use #detect in Rails to avoid conflicts with AR.find Fixes https://travis-ci.org/mbleigh/acts-as-taggable-on/jobs/15206376 --- lib/acts_as_taggable_on/tag.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/acts_as_taggable_on/tag.rb b/lib/acts_as_taggable_on/tag.rb index 07bbb6ca7..eb3949542 100644 --- a/lib/acts_as_taggable_on/tag.rb +++ b/lib/acts_as_taggable_on/tag.rb @@ -64,7 +64,7 @@ def self.find_or_create_all_with_like_by_name(*list) list.map do |tag_name| comparable_tag_name = comparable_name(tag_name) - existing_tag = existing_tags.find { |tag| comparable_name(tag.name) == comparable_tag_name } + existing_tag = existing_tags.detect { |tag| comparable_name(tag.name) == comparable_tag_name } existing_tag || Tag.create(:name => tag_name) end From 83a365624520a5fddb51a2006eaf0fd57ea10c64 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 9 Dec 2013 22:40:10 -0600 Subject: [PATCH 8/8] Refactoring; Ensure queried tag is 8bit ASCII Fixes SQLITE3 failures --- lib/acts_as_taggable_on/tag.rb | 32 +++++++++++++--- .../acts_as_taggable_on_spec.rb | 7 ++-- .../acts_as_tagger_spec.rb | 38 +++++++++---------- 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/lib/acts_as_taggable_on/tag.rb b/lib/acts_as_taggable_on/tag.rb index eb3949542..336f8ba9f 100644 --- a/lib/acts_as_taggable_on/tag.rb +++ b/lib/acts_as_taggable_on/tag.rb @@ -1,3 +1,4 @@ +# coding: utf-8 module ActsAsTaggableOn class Tag < ::ActiveRecord::Base include ActsAsTaggableOn::Utils @@ -31,18 +32,29 @@ def self.named(name) def self.named_any(list) if ActsAsTaggableOn.strict_case_match - where(list.map { |tag| sanitize_sql(["name = #{binary}?", tag.to_s.mb_chars]) }.join(" OR ")) + clause = list.map { |tag| + sanitize_sql(["name = #{binary}?", as_8bit_ascii(tag)]) + }.join(" OR ") + where(clause) else - where(list.map { |tag| sanitize_sql(["lower(name) = ?", tag.to_s.mb_chars.downcase]) }.join(" OR ")) + clause = list.map { |tag| + lowercase_ascii_tag = as_8bit_ascii(tag).downcase + sanitize_sql(["lower(name) = ?", lowercase_ascii_tag]) + }.join(" OR ") + where(clause) end end def self.named_like(name) - where(["name #{like_operator} ? ESCAPE '!'", "%#{escape_like(name)}%"]) + clause = ["name #{like_operator} ? ESCAPE '!'", "%#{escape_like(name)}%"] + where(clause) end def self.named_like_any(list) - where(list.map { |tag| sanitize_sql(["name #{like_operator} ? ESCAPE '!'", "%#{escape_like(tag.to_s)}%"]) }.join(" OR ")) + clause = list.map { |tag| + sanitize_sql(["name #{like_operator} ? ESCAPE '!'", "%#{escape_like(tag.to_s)}%"]) + }.join(" OR ") + where(clause) end ### CLASS METHODS: @@ -56,7 +68,7 @@ def self.find_or_create_with_like_by_name(name) end def self.find_or_create_all_with_like_by_name(*list) - list = [list].flatten + list = Array(list).flatten return [] if list.empty? @@ -88,12 +100,20 @@ class << self private def comparable_name(str) - str.mb_chars.downcase.to_s + as_8bit_ascii(str).downcase end def binary /mysql/ === ActiveRecord::Base.connection_config[:adapter] ? "BINARY " : nil end + + def as_8bit_ascii(string) + if defined?(Encoding) + string.to_s.force_encoding('BINARY') + else + string.to_s.mb_chars + end + end end end end diff --git a/spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb b/spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb index 3c9dc2495..3c1ab3205 100644 --- a/spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb +++ b/spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 require 'spec_helper' describe "Acts As Taggable On" do @@ -8,7 +9,7 @@ it "should provide a class method 'taggable?' that is false for untaggable models" do UntaggableModel.should_not be_taggable end - + describe "Taggable Method Generation To Preserve Order" do before(:each) do clean_database! @@ -46,7 +47,7 @@ it "should have all tag types" do @taggable.tag_types.should == [:tags, :languages, :skills, :needs, :offerings] end - + it "should create a class attribute for preserve tag order" do @taggable.class.should respond_to(:preserve_tag_order?) end @@ -54,7 +55,7 @@ it "should create an instance attribute for preserve tag order" do @taggable.should respond_to(:preserve_tag_order?) end - + it "should respond 'false' to preserve_tag_order?" do @taggable.class.preserve_tag_order?.should be_false end diff --git a/spec/acts_as_taggable_on/acts_as_tagger_spec.rb b/spec/acts_as_taggable_on/acts_as_tagger_spec.rb index 9591fcba3..22e93067a 100644 --- a/spec/acts_as_taggable_on/acts_as_tagger_spec.rb +++ b/spec/acts_as_taggable_on/acts_as_tagger_spec.rb @@ -4,7 +4,7 @@ before(:each) do clean_database! end - + describe "Tagger Method Generation" do before(:each) do @tagger = User.new @@ -13,55 +13,55 @@ it "should add #is_tagger? query method to the class-side" do User.should respond_to(:is_tagger?) end - + it "should return true from the class-side #is_tagger?" do User.is_tagger?.should be_true end - + it "should return false from the base #is_tagger?" do ActiveRecord::Base.is_tagger?.should be_false end - + it "should add #is_tagger? query method to the singleton" do @tagger.should respond_to(:is_tagger?) end - + it "should add #tag method on the instance-side" do @tagger.should respond_to(:tag) end - + it "should generate an association for #owned_taggings and #owned_tags" do @tagger.should respond_to(:owned_taggings, :owned_tags) end end - + describe "#tag" do context 'when called with a non-existent tag context' do before(:each) do @tagger = User.new @taggable = TaggableModel.new(:name=>"Richard Prior") end - + it "should by default not throw an exception " do @taggable.tag_list_on(:foo).should be_empty lambda { @tagger.tag(@taggable, :with=>'this, and, that', :on=>:foo) }.should_not raise_error end - + it 'should by default create the tag context on-the-fly' do @taggable.tag_list_on(:here_ond_now).should be_empty @tagger.tag(@taggable, :with=>'that', :on => :here_ond_now) @taggable.tag_list_on(:here_ond_now).should_not include('that') @taggable.all_tags_list_on(:here_ond_now).should include('that') end - + it "should show all the tag list when both public and owned tags exist" do @taggable.tag_list = 'ruby, python' @tagger.tag(@taggable, :with => 'java, lisp', :on => :tags) @taggable.all_tags_on(:tags).map(&:name).sort.should == %w(ruby python java lisp).sort end - + it "should not add owned tags to the common list" do @taggable.tag_list = 'ruby, python' @tagger.tag(@taggable, :with => 'java, lisp', :on => :tags) @@ -69,12 +69,12 @@ @tagger.tag(@taggable, :with => '', :on => :tags) @taggable.tag_list.should == %w(ruby python) end - + it "should throw an exception when the default is over-ridden" do @taggable.tag_list_on(:foo_boo).should be_empty lambda { @tagger.tag(@taggable, :with=>'this, and, that', :on=>:foo_boo, :force=>false) - }.should raise_error + }.should raise_error end it "should not create the tag context on-the-fly when the default is over-ridden" do @@ -83,28 +83,28 @@ @taggable.tag_list_on(:foo_boo).should be_empty end end - + describe "when called by multiple tagger's" do before(:each) do @user_x = User.create(:name => "User X") @user_y = User.create(:name => "User Y") @taggable = TaggableModel.create(:name => 'acts_as_taggable_on', :tag_list => 'plugin') - + @user_x.tag(@taggable, :with => 'ruby, rails', :on => :tags) @user_y.tag(@taggable, :with => 'ruby, plugin', :on => :tags) @user_y.tag(@taggable, :with => '', :on => :tags) @user_y.tag(@taggable, :with => '', :on => :tags) end - - it "should delete owned tags" do + + it "should delete owned tags" do @user_y.owned_tags.should == [] end - + it "should not delete other taggers tags" do @user_x.owned_tags.should have(2).items end - + it "should not delete original tags" do @taggable.all_tags_list_on(:tags).should include('plugin') end