@@ -12,7 +12,7 @@ namespace :db do user_group.name = Populator.words(1..2).titleize user_group.active = [true, false] User.populate 0..150 do |user| - user.user_group_id = user_group.id + user.user_group_id = [nil, user_group.id] user.first_name = Faker::Name.first_name user.last_name = Faker::Name.last_name user.email = "#{user.first_name.downcase}.#{user.last_name.downcase}@#{Faker::Internet.domain_name}" lib/tasks/populate.rake @@ -1,13 +1,27 @@ -== 1.3.2 released 2008-10-02 +== 1.3.4 released 2008-10-03 + +* Fixed method names creation for conditions with modifiers +* Create local column conditions, incase there are method conflicts. Such as "id". +* All joins are switched to left outer joins. When search with *any* conditions, inner joins return incorrect results. Also when ordering by an association that allows for blanks, inner joins exclude the records with blank values. Only option is left outer joins. +* If joins is manually set with a string, treat it as SQL, just like ActiveRecord + +== 1.3.3 released 2008-10-03 + +* Fixed modifiers being double escaped on equals condition +* Fixed bug when merging conditions with array substitutions +* Updated blank condition to check if the values is false or not +* Fixed type for the year modifier in the mysql adapter + +== 1.3.2 released 2008-10-03 * Fixed condition to type cast all values in an array * Fixed bug to properly set the type for the type cast column * Fixed bug to allow conditions on local columns * Apparently ActiveRecord 2.2 doesnt remove duplicates on all joins, need to enforce this for ALL versions of ActiveRecord * Fixed bug when passing a symbol as the SQL type for condition return types -* Moved duplicate removal to only apply when searching or calculating from a search object. Anything done in a normal ActiveRecord method should remain untouched and let ActiveRecord handle it. Just like in ActiveRecord if you do a User.all search and pass some :joins, you might get duplicate records. -* Discard meaningless array values -* Delete blank strings from mass assignments on conditions +* Moved duplicate removal to only apply when searching or calculating from a search object. Anything done in a normal ActiveRecord method should remain untouched and let ActiveRecord handle it. Just like in ActiveRecord if you do a User.all search and pass some :joins, you might get duplicate records. Searchgasm should not change that behavior, unless called from a searchgasm object. +* Discard meaningless array values when setting a condition. +* Delete blank strings from mass assignments on conditions. == 1.3.1 released 2008-10-02 vendor/plugins/searchgasm/CHANGELOG.rdoc @@ -44,7 +44,7 @@ module Searchgasm end def year_sql(column_name) - "MONTH(#{column_name})" + "YEAR(#{column_name})" end # String functions vendor/plugins/searchgasm/lib/searchgasm/active_record/connection_adapters/mysql_adapter.rb @@ -11,9 +11,9 @@ module Searchgasm def to_conditions(value) if value == true - "#{column_sql} is NULL or #{column_sql} = ''" + "#{column_sql} is NULL or #{column_sql} = '' or #{column_sql} = false" elsif value == false - "#{column_sql} is NOT NULL and #{column_sql} != ''" + "#{column_sql} is NOT NULL and #{column_sql} != '' and #{column_sql} != false" end end end vendor/plugins/searchgasm/lib/searchgasm/condition/blank.rb @@ -12,7 +12,15 @@ module Searchgasm def to_conditions(value) # Let ActiveRecord handle this - klass.send(:sanitize_sql_hash_for_conditions, {column.name => value}) + args = [] + case value + when Range + args = [value.first, value.last] + else + args << value + end + + ["#{column_sql} #{klass.send(:attribute_condition, value)}", *args] end end end vendor/plugins/searchgasm/lib/searchgasm/condition/equals.rb @@ -15,12 +15,12 @@ module Searchgasm condition = Equals.new(klass, column) condition.value = value - sql = condition.sanitize - sql.gsub!(/ IS /, " IS NOT ") - sql.gsub!(/ BETWEEN /, " NOT BETWEEN ") - sql.gsub!(/ IN /, " NOT IN ") - sql.gsub!(/=/, "!=") - sql + conditions_array = condition.sanitize + conditions_array.first.gsub!(/ IS /, " IS NOT ") + conditions_array.first.gsub!(/ BETWEEN /, " NOT BETWEEN ") + conditions_array.first.gsub!(/ IN /, " NOT IN ") + conditions_array.first.gsub!(/=/, "!=") + conditions_array end end end vendor/plugins/searchgasm/lib/searchgasm/condition/not_equal.rb @@ -11,7 +11,7 @@ module Searchgasm attr_accessor :any, :relationship_name class << self - attr_accessor :added_klass_conditions, :added_column_conditions, :added_associations + attr_accessor :added_klass_conditions, :added_column_equals_conditions, :added_associations def column_details # :nodoc: return @column_details if @column_details @@ -149,6 +149,7 @@ module Searchgasm def initialize(init_conditions = {}) add_associations! + add_column_equals_conditions! self.conditions = init_conditions end @@ -208,6 +209,7 @@ module Searchgasm new_condition_value = [] condition_value.each { |v| new_condition_value << v unless v == "" } next if new_condition_value.size == 0 + new_condition_value = new_condition_value.first if new_condition_value.size == 1 else next if condition_value == "" new_condition_value = condition_value @@ -265,6 +267,12 @@ module Searchgasm self.class.added_associations = true end + def add_column_equals_conditions! + return true if self.class.added_column_equals_conditions + klass.column_names.each { |name| setup_condition(name) } + self.class.added_column_equals_conditions = true + end + def extract_column_and_condition_from_method_name(name) name_parts = name.gsub("=", "").split("_") @@ -333,42 +341,56 @@ module Searchgasm modifier_name_parts = [] modifier_klasses.each { |modifier_klass| modifier_name_parts << modifier_klass.modifier_names.first } method_name_parts = [] - method_name_parts << modifier_name_parts.join("_of_") unless modifier_name_parts.blank? + method_name_parts << modifier_name_parts.join("_of_") + "_of" unless modifier_name_parts.blank? method_name_parts << column_name - method_name_parts << condition_name + method_name_parts << condition_name unless condition_name.blank? method_name_parts.join("_") end def method_missing(name, *args, &block) + if setup_condition(name) + send(name, *args, &block) + else + super + end + end + + def setup_condition(name) modifier_klasses, column_detail, condition_klass = breakdown_method_name(name.to_s) if !column_detail.nil? && !condition_klass.nil? method_name = build_method_name(modifier_klasses, column_detail[:column].name, condition_klass.condition_names_for_column.first) - column_type = column_sql = nil - if !modifier_klasses.blank? - # Find the column type - column_type = modifier_klasses.first.return_type + + if !added_condition?(method_name) + column_type = column_sql = nil + if !modifier_klasses.blank? + # Find the column type + column_type = modifier_klasses.first.return_type - # Build the column sql - column_sql = "#{klass.connection.quote_table_name(klass.table_name)}.#{klass.connection.quote_column_name(column_detail[:column].name)}" - modifier_klasses.each do |modifier_klass| - next unless klass.connection.respond_to?(modifier_klass.adapter_method_name) - column_sql = klass.connection.send(modifier_klass.adapter_method_name, column_sql) + # Build the column sql + column_sql = "#{klass.connection.quote_table_name(klass.table_name)}.#{klass.connection.quote_column_name(column_detail[:column].name)}" + modifier_klasses.each do |modifier_klass| + next unless klass.connection.respond_to?(modifier_klass.adapter_method_name) + column_sql = klass.connection.send(modifier_klass.adapter_method_name, column_sql) + end end - end - add_condition!(condition_klass, method_name, column_detail[:column], column_type, column_sql) + add_condition!(condition_klass, method_name, column_detail[:column], column_type, column_sql) - ([column_detail[:column].name] + column_detail[:aliases]).each do |column_name| - condition_klass.condition_names_for_column.each do |condition_name| - alias_method_name = build_method_name(modifier_klasses, column_name, condition_name) - add_condition_alias!(alias_method_name, method_name) unless alias_method_name == method_name + ([column_detail[:column].name] + column_detail[:aliases]).each do |column_name| + condition_klass.condition_names_for_column.each do |condition_name| + alias_method_name = build_method_name(modifier_klasses, column_name, condition_name) + add_condition_alias!(alias_method_name, method_name) unless added_condition?(alias_method_name) + end end end - send(method_name + (name.to_s =~ /=$/ ? "=" : ""), *args, &block) - else - super + alias_method_name = name.to_s.gsub("=", "") + add_condition_alias!(alias_method_name, method_name) unless added_condition?(alias_method_name) + + return true end + + false end def add_condition!(condition, name, column = nil, column_type = nil, column_sql = nil) @@ -396,6 +418,10 @@ module Searchgasm end_eval end + def added_condition?(name) + respond_to?("#{name}_object") && respond_to?(name) && respond_to?("#{name}=") && respond_to?("reset_#{name}!") + end + def add_condition_alias!(alias_name, name) self.class.condition_names << alias_name vendor/plugins/searchgasm/lib/searchgasm/conditions/base.rb @@ -1,6 +1,6 @@ module Searchgasm module Helpers #:nodoc: - module Utilities # :nodoc: + module Utilities # Builds a hash of params for creating a url and preserves any existing params. You can pass this into url_for and build your url. Although most rails helpers accept a hash. # # Let's take the page_link helper. Here is the code behind that helper: @@ -84,6 +84,7 @@ module Searchgasm # * <tt>:params</tt> -- default: nil, Additional params to add to the url, must be a hash # * <tt>:exclude_params</tt> -- default: nil, params you want to exclude. This is nifty because it does a "deep delete". So you can pass {:param1 => {:param2 => :param3}} and it will make sure param3 does not get include. param1 and param2 will not be touched. This also accepts an array or just a symbol or string. # * <tt>:search_params</tt> -- default: nil, Additional search params to add to the url, must be a hash. Adds the options into the :params_scope. + # * <tt>:literal_search_params</tt> -- default: nil, Additional search params to add to the url, but are not escaped. So you can add javascript into the URL: :literal_search_params => {:per_page => "' + escape(this.value) + '"} # * <tt>:exclude_search_params</tt> -- default: nil, Same as :exclude_params but for the :search_params. def searchgasm_url(options = {}) search_params = searchgasm_params(options) vendor/plugins/searchgasm/lib/searchgasm/helpers/utilities.rb @@ -79,8 +79,27 @@ module Searchgasm #:nodoc: "#<#{klass}Search #{current_find_options.inspect}>" end + # Searchgasm requires that all joins be left outer joins for conditions and ordering to work properly def joins - merge_joins(@joins, auto_joins) + joins_sql = "" + all_joins = auto_joins + + case @joins + when String + joins_sql += @joins + else + all_joins = merge_joins(@joins, all_joins) + end + + return if joins_sql.blank? && all_joins.blank? + + unless all_joins.blank? + join_dependency = ::ActiveRecord::Associations::ClassMethods::JoinDependency.new(klass, all_joins, nil) + joins_sql += " " unless joins_sql.blank? + joins_sql += join_dependency.join_associations.collect { |assoc| assoc.association_join }.join + end + + joins_sql end def limit=(value) vendor/plugins/searchgasm/lib/searchgasm/search/base.rb @@ -11,7 +11,6 @@ module Searchgasm alias_method_chain :conditions=, :conditions alias_method_chain :conditions, :conditions alias_method_chain :auto_joins, :conditions - alias_method_chain :joins, :conditions alias_method_chain :sanitize, :conditions end end @@ -57,16 +56,6 @@ module Searchgasm @memoized_auto_joins ||= merge_joins(auto_joins_without_conditions, conditions.auto_joins) end - # Changes joins to use left outer joins if conditions.any == true. - def joins_with_conditions - if conditions.any? - join_dependency = ::ActiveRecord::Associations::ClassMethods::JoinDependency.new(klass, joins_without_conditions, nil) - join_dependency.join_associations.collect { |assoc| assoc.association_join }.join - else - joins_without_conditions - end - end - def sanitize_with_conditions(searching = true) # :nodoc: find_options = sanitize_without_conditions(searching) if conditions_obj = find_options.delete(:conditions) vendor/plugins/searchgasm/lib/searchgasm/search/conditions.rb @@ -13,7 +13,7 @@ module Searchgasm conditions.each do |condition| next if condition.blank? - arr_condition = [condition].flatten + arr_condition = condition.is_a?(Array) ? condition : [condition] conditions_strs << arr_condition.first conditions_subs += arr_condition[1..-1] end vendor/plugins/searchgasm/lib/searchgasm/shared/utilities.rb @@ -67,7 +67,7 @@ module Searchgasm MAJOR = 1 MINOR = 3 - TINY = 1 + TINY = 4 # The current version as a Version instance CURRENT = new(MAJOR, MINOR, TINY) vendor/plugins/searchgasm/lib/searchgasm/version.rb @@ -24,4 +24,13 @@ jennifer: last_name: Hopkins active: false bio: Totally not awesome at all! + +tren: + id: 4 + account_id: + parent_id: + first_name: Tren + last_name: Garfield + active: false + bio: Rocks a little too hard \ No newline at end of file vendor/plugins/searchgasm/test/fixtures/users.yml @@ -8,19 +8,19 @@ class TestConditionTypes < Test::Unit::TestCase condition = Searchgasm::Condition::Blank.new(Account, Account.columns_hash["id"]) condition.value = "true" - assert_equal "\"accounts\".\"id\" is NULL or \"accounts\".\"id\" = ''", condition.sanitize + assert_equal "\"accounts\".\"id\" is NULL or \"accounts\".\"id\" = '' or \"accounts\".\"id\" = false", condition.sanitize condition = Searchgasm::Condition::Blank.new(Account, Account.columns_hash["id"]) condition.value = "false" - assert_equal "\"accounts\".\"id\" is NOT NULL and \"accounts\".\"id\" != ''", condition.sanitize + assert_equal "\"accounts\".\"id\" is NOT NULL and \"accounts\".\"id\" != '' and \"accounts\".\"id\" != false", condition.sanitize condition = Searchgasm::Condition::Blank.new(Account, Account.columns_hash["id"]) condition.value = true - assert_equal "\"accounts\".\"id\" is NULL or \"accounts\".\"id\" = ''", condition.sanitize + assert_equal "\"accounts\".\"id\" is NULL or \"accounts\".\"id\" = '' or \"accounts\".\"id\" = false", condition.sanitize condition = Searchgasm::Condition::Blank.new(Account, Account.columns_hash["id"]) condition.value = false - assert_equal "\"accounts\".\"id\" is NOT NULL and \"accounts\".\"id\" != ''", condition.sanitize + assert_equal "\"accounts\".\"id\" is NOT NULL and \"accounts\".\"id\" != '' and \"accounts\".\"id\" != false", condition.sanitize condition = Searchgasm::Condition::Blank.new(Account, Account.columns_hash["id"]) condition.value = nil @@ -48,15 +48,15 @@ class TestConditionTypes < Test::Unit::TestCase condition = Searchgasm::Condition::Equals.new(Account, Account.columns_hash["id"]) condition.value = 12 - assert_equal "\"accounts\".\"id\" = 12", condition.sanitize + assert_equal ["\"accounts\".\"id\" = ?", 12], condition.sanitize condition = Searchgasm::Condition::Equals.new(Account, Account.columns_hash["id"]) condition.value = [1,2,3,4] - assert_equal "\"accounts\".\"id\" IN (1,2,3,4)", condition.sanitize + assert_equal ["\"accounts\".\"id\" IN (?)", [1, 2, 3, 4]], condition.sanitize condition = Searchgasm::Condition::Equals.new(Account, Account.columns_hash["id"]) condition.value = (1..10) - assert_equal "\"accounts\".\"id\" BETWEEN 1 AND 10", condition.sanitize + assert_equal ["\"accounts\".\"id\" BETWEEN ? AND ?", 1, 10], condition.sanitize condition = Searchgasm::Condition::GreaterThan.new(Account, Account.columns_hash["id"]) condition.value = 2 @@ -100,15 +100,15 @@ class TestConditionTypes < Test::Unit::TestCase condition = Searchgasm::Condition::NotEqual.new(Account, Account.columns_hash["id"]) condition.value = 12 - assert_equal "\"accounts\".\"id\" != 12", condition.sanitize + assert_equal ["\"accounts\".\"id\" != ?", 12], condition.sanitize condition = Searchgasm::Condition::NotEqual.new(Account, Account.columns_hash["id"]) condition.value = [1,2,3,4] - assert_equal "\"accounts\".\"id\" NOT IN (1,2,3,4)", condition.sanitize + assert_equal ["\"accounts\".\"id\" NOT IN (?)", [1, 2, 3, 4]], condition.sanitize condition = Searchgasm::Condition::NotEqual.new(Account, Account.columns_hash["id"]) condition.value = (1..10) - assert_equal "\"accounts\".\"id\" NOT BETWEEN 1 AND 10", condition.sanitize + assert_equal ["\"accounts\".\"id\" NOT BETWEEN ? AND ?", 1, 10], condition.sanitize condition = Searchgasm::Condition::Keywords.new(Account, Account.columns_hash["name"]) condition.value = "freedom yeah, freedom YEAH right" vendor/plugins/searchgasm/test/test_condition_types.rb @@ -44,11 +44,11 @@ class TestConditionsBase < Test::Unit::TestCase conditions.name_contains = "Binary" assert_equal ["\"accounts\".\"name\" LIKE ?", "%Binary%"], conditions.sanitize conditions.id = 1 - assert_equal ["(\"accounts\".\"id\" = 1) AND (\"accounts\".\"name\" LIKE ?)", "%Binary%"], conditions.sanitize + assert_equal ["(\"accounts\".\"id\" = ?) AND (\"accounts\".\"name\" LIKE ?)", 1, "%Binary%"], conditions.sanitize conditions.any = true - assert_equal ["(\"accounts\".\"id\" = 1) OR (\"accounts\".\"name\" LIKE ?)", "%Binary%"], conditions.sanitize + assert_equal ["(\"accounts\".\"id\" = ?) OR (\"accounts\".\"name\" LIKE ?)", 1, "%Binary%"], conditions.sanitize conditions.any = false - assert_equal ["(\"accounts\".\"id\" = 1) AND (\"accounts\".\"name\" LIKE ?)", "%Binary%"], conditions.sanitize + assert_equal ["(\"accounts\".\"id\" = ?) AND (\"accounts\".\"name\" LIKE ?)", 1, "%Binary%"], conditions.sanitize end def test_auto_joins @@ -117,8 +117,9 @@ class TestConditionsBase < Test::Unit::TestCase conditions.name_contains = "awesome" assert_equal({:name_like => "awesome"}, conditions.conditions) - conditions.conditions = {:id_gt => "", :id => "", :name => ["", "", ""], :name_starts_with => "Ben"} - assert_equal({:name_like => "awesome", :name_begins_with => "Ben"}, conditions.conditions) + now = Time.now + conditions.conditions = {:id_gt => "", :id => "", :name => ["", "", ""], :created_at => ["", now], :name_starts_with => "Ben"} + assert_equal({:name_like => "awesome", :name_begins_with => "Ben", :created_at_equals => now}, conditions.conditions) end # Other general usage tests, need to clean these up @@ -212,4 +213,9 @@ class TestConditionsBase < Test::Unit::TestCase conditions.reset_users! assert_equal({}, conditions.send(:objects)) end + + def test_method_conflicts + conditions = Searchgasm::Cache::AccountConditions.new + assert_equal nil, conditions.id + end end vendor/plugins/searchgasm/test/test_conditions_base.rb @@ -28,9 +28,9 @@ class TestSearchBase < Test::Unit::TestCase end def test_setting_first_level_options - search = Account.new_search!(:include => :users, :joins => :test, :offset => 5, :limit => 20, :order => "name ASC", :select => "name", :readonly => true, :group => "name", :from => "accounts", :lock => true) + search = Account.new_search!(:include => :users, :joins => :users, :offset => 5, :limit => 20, :order => "name ASC", :select => "name", :readonly => true, :group => "name", :from => "accounts", :lock => true) assert_equal :users, search.include - assert_equal :test, search.joins + assert_equal " LEFT OUTER JOIN \"users\" ON users.account_id = accounts.id ", search.joins assert_equal 5, search.offset assert_equal 20, search.limit assert_equal "name ASC", search.order @@ -44,9 +44,10 @@ class TestSearchBase < Test::Unit::TestCase search.include = :users assert_equal :users, search.include - - search.joins = "test" - assert_equal "test", search.joins + + # treat it like SQL, just like AR + search.joins = "users" + assert_equal "users", search.joins search.page = 5 assert_equal 1, search.page @@ -140,7 +141,7 @@ class TestSearchBase < Test::Unit::TestCase search.conditions.users.id_greater_than = 2 search.page = 3 search.readonly = true - assert_equal({:joins => :users, :offset => 4, :readonly => true, :conditions => ["(\"accounts\".\"name\" LIKE ?) AND (\"users\".\"id\" > ?)", "%Binary%", 2], :limit => 2 }, search.sanitize) + assert_equal({:joins => " LEFT OUTER JOIN \"users\" ON users.account_id = accounts.id ", :offset => 4, :readonly => true, :conditions => ["(\"accounts\".\"name\" LIKE ?) AND (\"users\".\"id\" > ?)", "%Binary%", 2], :limit => 2 }, search.sanitize) end def test_scope vendor/plugins/searchgasm/test/test_search_base.rb @@ -23,16 +23,6 @@ class TestSearchConditions < Test::Unit::TestCase assert_equal nil, search.auto_joins end - def test_joins - search = Account.new_search - search.conditions = {:id_lte => 2, :users => {:first_name_like => "Ben"}} - assert_equal :users, search.joins - assert_equal [Account.find(1)], search.all - search.conditions.any = true - assert_equal " LEFT OUTER JOIN \"users\" ON users.account_id = accounts.id ", search.joins - assert_equal Account.find(1, 2), search.all - end - def test_sanitize # This is tested in test_search_base end vendor/plugins/searchgasm/test/test_search_conditions.rb @@ -157,4 +157,10 @@ class TestSearchOrdering < Test::Unit::TestCase def test_sanitize # tested in test_priority_order_by end + + def test_ordering_includes_blank + search = User.new_search + search.order_by = {:account => :name} + assert_equal 4, search.count + end end vendor/plugins/searchgasm/test/test_search_ordering.rb 3938198db3eb8ad6556b3aeaf503fe5dfbb04a2d binarylogic bjohnson@binarylogic.com http://github.com/binarylogic/searchlogic_example/commit/2e406572083a5a63c76b1a62d881fc13f1aff0e1 2e406572083a5a63c76b1a62d881fc13f1aff0e1 2008-10-06T21:38:23-07:00 2008-10-06T21:38:23-07:00 Updated searchgasm 6d6adfe9541db36a5141634e3af0e85a27a694c3 binarylogic bjohnson@binarylogic.com