diff --git a/lib/ransack/adapters/active_record.rb b/lib/ransack/adapters/active_record.rb index 1a4369d59..0ffbf303c 100644 --- a/lib/ransack/adapters/active_record.rb +++ b/lib/ransack/adapters/active_record.rb @@ -1,6 +1,8 @@ require 'ransack/adapters/active_record/base' ActiveRecord::Base.extend Ransack::Adapters::ActiveRecord::Base +require 'ransack/adapters/active_record/context' + case ActiveRecord::VERSION::STRING when /^3\.0\./ require 'ransack/adapters/active_record/3.0/context' @@ -8,6 +10,4 @@ require 'ransack/adapters/active_record/3.1/context' when /^3\.2\./ require 'ransack/adapters/active_record/3.2/context' -else - require 'ransack/adapters/active_record/context' end diff --git a/lib/ransack/adapters/active_record/3.0/context.rb b/lib/ransack/adapters/active_record/3.0/context.rb index f9ad0f726..e725a2cdf 100644 --- a/lib/ransack/adapters/active_record/3.0/context.rb +++ b/lib/ransack/adapters/active_record/3.0/context.rb @@ -7,9 +7,11 @@ module Ransack module Adapters module ActiveRecord class Context < ::Ransack::Context + # Because the AR::Associations namespace is insane - JoinDependency = ::ActiveRecord::Associations::ClassMethods::JoinDependency - JoinBase = JoinDependency::JoinBase + if defined? ::ActiveRecord::Associations::ClassMethods::JoinDependency + JoinDependency = ::ActiveRecord::Associations::ClassMethods::JoinDependency + end # Redefine a few things for ActiveRecord 3.0. diff --git a/lib/ransack/adapters/active_record/3.1/context.rb b/lib/ransack/adapters/active_record/3.1/context.rb index 2a718d2e8..8a9cbdc07 100644 --- a/lib/ransack/adapters/active_record/3.1/context.rb +++ b/lib/ransack/adapters/active_record/3.1/context.rb @@ -6,9 +6,6 @@ module Ransack module Adapters module ActiveRecord class Context < ::Ransack::Context - # Because the AR::Associations namespace is insane - JoinDependency = ::ActiveRecord::Associations::JoinDependency - JoinPart = JoinDependency::JoinPart # Redefine a few things for ActiveRecord 3.1. diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb index 4f68d6502..390664a4d 100644 --- a/lib/ransack/adapters/active_record/context.rb +++ b/lib/ransack/adapters/active_record/context.rb @@ -8,8 +8,9 @@ module ActiveRecord class Context < ::Ransack::Context # Because the AR::Associations namespace is insane - JoinDependency = ::ActiveRecord::Associations::JoinDependency - JoinPart = JoinDependency::JoinPart + if defined? ::ActiveRecord::Associations::JoinDependency + JoinDependency = ::ActiveRecord::Associations::JoinDependency + end def initialize(object, options = {}) super @@ -137,6 +138,64 @@ def alias_tracker @join_dependency.alias_tracker end + def lock_association(association) + @lock_associations << association + end + + if ::ActiveRecord::VERSION::STRING >= Constants::RAILS_4_1 + def remove_association(association) + return if @lock_associations.include?(association) + @join_dependency.join_root.children.delete_if { |stashed| + stashed.eql?(association) + } + @object.joins_values.delete_if { |jd| + jd.join_root.children.map(&:object_id) == [association.object_id] + } + end + else + def remove_association(association) + return if @lock_associations.include?(association) + @join_dependency.join_parts.delete(association) + @object.joins_values.delete(association) + end + end + + # Build an Arel subquery that selects keys for the top query, + # drawn from the first join association's foreign_key. + # + # Example: for an Article that has_and_belongs_to_many Tags + # + # context = Article.search.context + # attribute = Attribute.new(context, "tags_name").tap do |a| + # context.bind(a, a.name) + # end + # context.build_correlated_subquery(attribute.parent).to_sql + # + # # SELECT "articles_tags"."article_id" FROM "articles_tags" + # # INNER JOIN "tags" ON "tags"."id" = "articles_tags"."tag_id" + # # WHERE "articles_tags"."article_id" = "articles"."id" + # + # The WHERE condition on this query makes it invalid by itself, + # because it is correlated to the primary key on the outer query. + # + def build_correlated_subquery(association) + join_constraints = extract_joins(association) + join_root = join_constraints.shift + join_table = join_root.left + correlated_key = join_root.right.expr.left + subquery = Arel::SelectManager.new(association.base_klass) + subquery.from(join_root.left) + subquery.project(correlated_key) + join_constraints.each do |j| + subquery.join_sources << Arel::Nodes::InnerJoin.new(j.left, j.right) + end + subquery.where(correlated_key.eq(primary_key)) + end + + def primary_key + @object.table[@object.primary_key] + end + private def database_table_exists? @@ -242,65 +301,86 @@ def convert_join_strings_to_ast(table, joins) .map { |join| table.create_string_join(Arel.sql(join)) } end + def build_or_find_association(name, parent = @base, klass = nil) + find_association(name, parent, klass) or build_association(name, parent, klass) + end + if ::ActiveRecord::VERSION::STRING >= Constants::RAILS_4_1 - def build_or_find_association(name, parent = @base, klass = nil) - found_association = @join_dependency.join_root.children - .detect do |assoc| + def find_association(name, parent = @base, klass = nil) + @join_dependency.join_root.children.detect do |assoc| assoc.reflection.name == name && - (@associations_pot.nil? || @associations_pot[assoc] == parent) && + (@associations_pot.empty? || @associations_pot[assoc] == parent) && (!klass || assoc.reflection.klass == klass) end + end - unless found_association - jd = JoinDependency.new( - parent.base_klass, - Polyamorous::Join.new(name, @join_type, klass), - [] + def build_association(name, parent = @base, klass = nil) + jd = JoinDependency.new( + parent.base_klass, + Polyamorous::Join.new(name, @join_type, klass), + [] + ) + found_association = jd.join_root.children.last + @associations_pot[found_association] = parent + + # TODO maybe we dont need to push associations here, we could loop + # through the @associations_pot instead + @join_dependency.join_root.children.push found_association + + # Builds the arel nodes properly for this association + @join_dependency.send( + :construct_tables!, jd.join_root, found_association ) - found_association = jd.join_root.children.last - associations found_association, parent - # TODO maybe we dont need to push associations here, we could loop - # through the @associations_pot instead - @join_dependency.join_root.children.push found_association - - # Builds the arel nodes properly for this association - @join_dependency.send( - :construct_tables!, jd.join_root, found_association - ) + # Leverage the stashed association functionality in AR + @object = @object.joins(jd) - # Leverage the stashed association functionality in AR - @object = @object.joins(jd) - end found_association end - def associations(assoc, parent) - @associations_pot ||= {} - @associations_pot[assoc] = parent + def extract_joins(association) + parent = @join_dependency.join_root + reflection = association.reflection + join_constraints = association.join_constraints( + parent.table, + parent.base_klass, + association, + Arel::Nodes::OuterJoin, + association.tables, + reflection.scope_chain, + reflection.chain + ) + join_constraints.to_a.flatten end else - def build_or_find_association(name, parent = @base, klass = nil) + def build_association(name, parent = @base, klass = nil) + @join_dependency.send( + :build, + Polyamorous::Join.new(name, @join_type, klass), + parent + ) + found_association = @join_dependency.join_associations.last + # Leverage the stashed association functionality in AR + @object = @object.joins(found_association) + + found_association + end + + def extract_joins(association) + query = Arel::SelectManager.new(association.base_klass, association.table) + association.join_to(query).join_sources + end + + def find_association(name, parent = @base, klass = nil) found_association = @join_dependency.join_associations .detect do |assoc| assoc.reflection.name == name && assoc.parent == parent && (!klass || assoc.reflection.klass == klass) end - unless found_association - @join_dependency.send( - :build, - Polyamorous::Join.new(name, @join_type, klass), - parent - ) - found_association = @join_dependency.join_associations.last - # Leverage the stashed association functionality in AR - @object = @object.joins(found_association) - end - found_association end end diff --git a/lib/ransack/adapters/active_record/ransack/context.rb b/lib/ransack/adapters/active_record/ransack/context.rb index eba9539ab..708efeaa2 100644 --- a/lib/ransack/adapters/active_record/ransack/context.rb +++ b/lib/ransack/adapters/active_record/ransack/context.rb @@ -27,6 +27,8 @@ def initialize(object, options = {}) @join_dependency = join_dependency(@object) @join_type = options[:join_type] || Polyamorous::OuterJoin @search_key = options[:search_key] || Ransack.options[:search_key] + @associations_pot = {} + @lock_associations = [] if ::ActiveRecord::VERSION::STRING >= Constants::RAILS_4_1 @base = @join_dependency.join_root @@ -40,7 +42,7 @@ def initialize(object, options = {}) @base.table_name, as: @base.aliased_table_name, type_caster: self ) @bind_pairs = Hash.new do |hash, key| - parent, attr_name = get_parent_and_attribute_name(key.to_s) + parent, attr_name = get_parent_and_attribute_name(key) if parent && attr_name hash[key] = [parent, attr_name] end diff --git a/lib/ransack/adapters/active_record/ransack/nodes/condition.rb b/lib/ransack/adapters/active_record/ransack/nodes/condition.rb index a45eba7f0..e97ac5def 100644 --- a/lib/ransack/adapters/active_record/ransack/nodes/condition.rb +++ b/lib/ransack/adapters/active_record/ransack/nodes/condition.rb @@ -3,41 +3,33 @@ module Nodes class Condition def arel_predicate - arel_predicate_for(attributes_array) + attributes.map { |attribute| + association = attribute.parent + if negative? && attribute.associated_collection? + query = context.build_correlated_subquery(association) + query.where(format_predicate(attribute).not) + context.remove_association(association) + Arel::Nodes::NotIn.new(context.primary_key, Arel.sql(query.to_sql)) + else + format_predicate(attribute) + end + }.reduce(combinator_method) end private - def attributes_array - attributes.map do |a| - a.attr.send( - arel_predicate_for_attribute(a), formatted_values_for_attribute(a) - ) - end - end - - def arel_predicate_for(predicates) - if predicates.size > 1 - combinator_for(predicates) - else - format_predicate(predicates.first) - end - end - - def combinator_for(predicates) - if combinator === Constants::AND - Arel::Nodes::Grouping.new(Arel::Nodes::And.new(predicates)) - elsif combinator === Constants::OR - predicates.inject(&:or) - end + def combinator_method + combinator === Constants::OR ? :or : :and end - def format_predicate(predicate) - predicate.tap do - if casted_array_with_in_predicate?(predicate) - predicate.right[0] = format_values_for(predicate.right[0]) - end + def format_predicate(attribute) + arel_pred = arel_predicate_for_attribute(attribute) + arel_values = formatted_values_for_attribute(attribute) + predicate = attribute.attr.public_send(arel_pred, arel_values) + if casted_array_with_in_predicate?(predicate) + predicate.right[0] = format_values_for(predicate.right[0]) end + predicate end def casted_array_with_in_predicate?(predicate) diff --git a/lib/ransack/adapters/mongoid/context.rb b/lib/ransack/adapters/mongoid/context.rb index 1847f5d84..f1a1db41f 100644 --- a/lib/ransack/adapters/mongoid/context.rb +++ b/lib/ransack/adapters/mongoid/context.rb @@ -6,10 +6,6 @@ module Adapters module Mongoid class Context < ::Ransack::Context - # Because the AR::Associations namespace is insane - # JoinDependency = ::Mongoid::Associations::JoinDependency - # JoinPart = JoinDependency::JoinPart - def initialize(object, options = {}) super # @arel_visitor = @engine.connection.visitor @@ -100,6 +96,14 @@ def klassify(obj) end end + def lock_association(association) + warn "lock_association is not implemented for Ransack mongoid adapter" if $DEBUG + end + + def remove_association(association) + warn "remove_association is not implemented for Ransack mongoid adapter" if $DEBUG + end + private def get_parent_and_attribute_name(str, parent = @base) diff --git a/lib/ransack/context.rb b/lib/ransack/context.rb index 8be64d627..5f012bfba 100644 --- a/lib/ransack/context.rb +++ b/lib/ransack/context.rb @@ -58,6 +58,7 @@ def scope_arity(scope) end def bind(object, str) + return nil unless str object.parent, object.attr_name = @bind_pairs[str] end diff --git a/lib/ransack/nodes/attribute.rb b/lib/ransack/nodes/attribute.rb index 0b2fc3b1f..0eccd46a9 100644 --- a/lib/ransack/nodes/attribute.rb +++ b/lib/ransack/nodes/attribute.rb @@ -16,7 +16,6 @@ def initialize(context, name = nil, ransacker_args = []) def name=(name) @name = name - context.bind(self, name) unless name.blank? end def valid? @@ -25,6 +24,10 @@ def valid? .include?(attr_name.split('.').last) end + def associated_collection? + parent.respond_to?(:reflection) && parent.reflection.collection? + end + def type if ransacker return ransacker.type diff --git a/lib/ransack/nodes/condition.rb b/lib/ransack/nodes/condition.rb index 5bd3982af..09eb8c198 100644 --- a/lib/ransack/nodes/condition.rb +++ b/lib/ransack/nodes/condition.rb @@ -79,14 +79,12 @@ def attributes def attributes=(args) case args when Array - args.each do |attr| - attr = Attribute.new(@context, attr) - self.attributes << attr if attr.valid? + args.each do |name| + build_attribute(name) end when Hash args.each do |index, attrs| - attr = Attribute.new(@context, attrs[:name], attrs[:ransacker_args]) - self.attributes << attr if attr.valid? + build_attribute(attrs[:name], attrs[:ransacker_args]) end else raise ArgumentError, @@ -129,9 +127,13 @@ def combinator=(val) alias :m= :combinator= alias :m :combinator - def build_attribute(name = nil) - Attribute.new(@context, name).tap do |attribute| - self.attributes << attribute + def build_attribute(name = nil, ransacker_args = []) + Attribute.new(@context, name, ransacker_args).tap do |attribute| + @context.bind(attribute, attribute.name) + self.attributes << attribute if attribute.valid? + if predicate && !negative? + @context.lock_association(attribute.parent) + end end end @@ -183,6 +185,10 @@ def hash def predicate_name=(name) self.predicate = Predicate.named(name) + unless negative? + attributes.each { |a| context.lock_association(a.parent) } + end + @predicate end alias :p= :predicate_name= @@ -248,6 +254,10 @@ def inspect "Condition <#{data}>" end + def negative? + predicate.negative? + end + private def valid_combinator? diff --git a/lib/ransack/nodes/sort.rb b/lib/ransack/nodes/sort.rb index 0a700df86..244aeec74 100644 --- a/lib/ransack/nodes/sort.rb +++ b/lib/ransack/nodes/sort.rb @@ -32,7 +32,7 @@ def valid? def name=(name) @name = name - context.bind(self, name) unless name.blank? + context.bind(self, name) end def dir=(dir) diff --git a/lib/ransack/predicate.rb b/lib/ransack/predicate.rb index 2b5bd5da5..935729469 100644 --- a/lib/ransack/predicate.rb +++ b/lib/ransack/predicate.rb @@ -74,5 +74,9 @@ def validate(vals, type = @type) vals.any? { |v| validator.call(type ? v.cast(type) : v.value) } end + def negative? + @name.include?("not_".freeze) + end + end end diff --git a/spec/ransack/adapters/active_record/base_spec.rb b/spec/ransack/adapters/active_record/base_spec.rb index c9d3e97c7..8220746f6 100644 --- a/spec/ransack/adapters/active_record/base_spec.rb +++ b/spec/ransack/adapters/active_record/base_spec.rb @@ -94,6 +94,76 @@ module ActiveRecord end end + context 'negative conditions on HABTM associations' do + let(:medieval) { Tag.create!(name: 'Medieval') } + let(:fantasy) { Tag.create!(name: 'Fantasy') } + let(:arthur) { Article.create!(title: 'King Arthur') } + let(:marco) { Article.create!(title: 'Marco Polo') } + + before do + marco.tags << medieval + arthur.tags << medieval + arthur.tags << fantasy + end + + it 'removes redundant joins from top query' do + s = Article.ransack(tags_name_not_eq: "Fantasy") + sql = s.result.to_sql + + expect(sql).to_not include('LEFT OUTER JOIN') + end + + it 'handles != for single values' do + s = Article.ransack(tags_name_not_eq: "Fantasy") + articles = s.result.to_a + + expect(articles).to include marco + expect(articles).to_not include arthur + end + + it 'handles NOT IN for multiple attributes' do + s = Article.ransack(tags_name_not_in: ["Fantasy", "Scifi"]) + articles = s.result.to_a + + expect(articles).to include marco + expect(articles).to_not include arthur + end + end + + context 'negative conditions on self-referenced associations' do + let(:pop) { Person.create!(name: 'Grandpa') } + let(:dad) { Person.create!(name: 'Father') } + let(:mom) { Person.create!(name: 'Mother') } + let(:son) { Person.create!(name: 'Grandchild') } + + before do + son.parent = dad + dad.parent = pop + dad.children << son + mom.children << son + pop.children << dad + son.save! && dad.save! && mom.save! && pop.save! + end + + it 'handles multiple associations and aliases' do + s = Person.ransack( + c: { + '0' => { a: ['name'], p: 'not_eq', v: ['Father'] }, + '1' => { + a: ['children_name', 'parent_name'], + p: 'not_eq', v: ['Father'], m: 'or' + }, + '2' => { a: ['children_salary'], p: 'eq', v: [nil] } + }) + people = s.result + + expect(people.to_a).to include son + expect(people.to_a).to include mom + expect(people.to_a).to_not include dad # rule '0': 'name' + expect(people.to_a).to_not include pop # rule '1': 'children_name' + end + end + describe '#ransack_alias' do it 'translates an alias to the correct attributes' do p = Person.create!(name: 'Meatloaf', email: 'babies@example.com') diff --git a/spec/ransack/helpers/form_helper_spec.rb b/spec/ransack/helpers/form_helper_spec.rb index 7dc28d112..c0af3a0cb 100644 --- a/spec/ransack/helpers/form_helper_spec.rb +++ b/spec/ransack/helpers/form_helper_spec.rb @@ -312,7 +312,7 @@ module Helpers context 'view has existing parameters' do before do - @controller.view_context.params.merge!({ exist: 'existing' }) + @controller.view_context.params[:exist] = 'existing' end describe '#sort_link should not remove existing params' do subject { @controller.view_context diff --git a/spec/ransack/nodes/condition_spec.rb b/spec/ransack/nodes/condition_spec.rb index 6cb728ddc..8520fd882 100644 --- a/spec/ransack/nodes/condition_spec.rb +++ b/spec/ransack/nodes/condition_spec.rb @@ -29,6 +29,15 @@ module Nodes specify { expect(subject.values.size).to eq(2) } end + describe '#negative?' do + let(:context) { Context.for(Person) } + let(:eq) { Condition.extract(context, 'name_eq', 'A') } + let(:not_eq) { Condition.extract(context, 'name_not_eq', 'A') } + + specify { expect(not_eq.negative?).to be true } + specify { expect(eq.negative?).to be false } + end + context 'with an invalid predicate' do subject { Condition.extract(