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 build #430

Merged
merged 8 commits into from
Dec 10, 2013
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 @@ -5,3 +5,4 @@ env:
- DB=sqlite3
- DB=mysql
- DB=postgresql
cache: bundler
19 changes: 7 additions & 12 deletions lib/acts-as-taggable-on.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
require "active_record"
require "active_record/version"
require "action_view"
require 'active_support/all'

require "digest/sha1"

$LOAD_PATH.unshift(File.dirname(__FILE__))

module ActsAsTaggableOn
mattr_accessor :delimiter
@@delimiter = ','
Expand Down Expand Up @@ -50,16 +49,12 @@ 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
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

34 changes: 27 additions & 7 deletions lib/acts_as_taggable_on/tag.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# coding: utf-8
module ActsAsTaggableOn
class Tag < ::ActiveRecord::Base
include ActsAsTaggableOn::Utils
Expand Down Expand Up @@ -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:
Expand All @@ -56,15 +68,15 @@ 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?

existing_tags = Tag.named_any(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
Expand All @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQLITE3 queries require 8 bit ascii, but mb_chars seems to occasionally render 'one' as some invalid utf-8 'o', which crashes sqlite3. This fixes that.

end
end
end
end
end
7 changes: 4 additions & 3 deletions spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# coding: utf-8
require 'spec_helper'

describe "Acts As Taggable On" do
Expand All @@ -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!
Expand Down Expand Up @@ -46,15 +47,15 @@
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

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
Expand Down
38 changes: 19 additions & 19 deletions spec/acts_as_taggable_on/acts_as_tagger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
before(:each) do
clean_database!
end

describe "Tagger Method Generation" do
before(:each) do
@tagger = User.new
Expand All @@ -13,68 +13,68 @@
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)
@taggable.tag_list.should == %w(ruby python)
@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
Expand All @@ -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
Expand Down
46 changes: 25 additions & 21 deletions spec/schema.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
$LOAD_PATH << "." unless $LOAD_PATH.include?(".")
$LOAD_PATH.unshift(File.expand_path('../../lib', __FILE__))
require 'logger'

begin
Expand Down