diff --git a/lib/cancan/can_definition.rb b/lib/cancan/can_definition.rb index 32b6e12..7cb56f1 100644 --- a/lib/cancan/can_definition.rb +++ b/lib/cancan/can_definition.rb @@ -3,9 +3,7 @@ module CanCan # it holds the information about a "can" call made on Ability and provides # helpful methods to determine permission checking and conditions hash generation. class CanDefinition # :nodoc: - attr_reader :conditions, :block, :base_behavior - attr_reader :block - attr_reader :actions + attr_reader :base_behavior, :actions attr_writer :expanded_actions # The first argument when initializing is the base_behavior which is a true/false @@ -55,20 +53,12 @@ module CanCan @conditions == {} || @conditions.nil? end - def association_joins(conditions = @conditions) - return nil unless conditions.kind_of?(Hash) - joins = [] - conditions.each do |name, value| - if value.kind_of? Hash - nested = association_joins(value) - if nested - joins << {name => nested} - else - joins << {name => []} - end - end + def associations_hash(conditions = @conditions) + hash = {} + conditions.map do |name, value| + hash[name] = associations_hash(value) if value.kind_of? Hash end - joins unless joins.empty? + hash end private diff --git a/lib/cancan/query.rb b/lib/cancan/query.rb index 8411a63..acfc044 100644 --- a/lib/cancan/query.rb +++ b/lib/cancan/query.rb @@ -32,9 +32,11 @@ module CanCan # Returns the associations used in conditions for the :joins option of a search # See ActiveRecordAdditions#accessible_by for use in Active Record. def joins - unless @can_definitions.empty? - collect_association_joins(@can_definitions) + joins_hash = {} + @can_definitions.each do |can_definition| + merge_joins(joins_hash, can_definition.associations_hash) end + clean_joins(joins_hash) unless joins_hash.empty? end private @@ -67,31 +69,22 @@ module CanCan @sanitizer.sanitize_sql(conditions) end - def collect_association_joins(can_definitions) - joins = [] - @can_definitions.each do |can_definition| - merge_association_joins(joins, can_definition.association_joins || []) - end - joins = clear_association_joins(joins) - joins unless joins.empty? - end - - def merge_association_joins(what, with) - with.each do |join| - name, nested = join.each_pair.first - if at = what.detect{|h| h.has_key?(name) } - at[name] = merge_association_joins(at[name], nested) + def merge_joins(base, add) + add.each do |name, nested| + if base[name].is_a?(Hash) && !nested.empty? + merge_joins(base[name], nested) else - what << join + base[name] = nested end end end - def clear_association_joins(joins) - joins.map do |join| - name, nested = join.each_pair.first - nested.empty? ? name : {name => clear_association_joins(nested)} + def clean_joins(joins_hash) + joins = [] + joins_hash.each do |name, nested| + joins << (nested.empty? ? name : {name => clean_joins(nested)}) end + joins end end end diff --git a/spec/cancan/active_record_additions_spec.rb b/spec/cancan/active_record_additions_spec.rb index 275f90d..c50b7ab 100644 --- a/spec/cancan/active_record_additions_spec.rb +++ b/spec/cancan/active_record_additions_spec.rb @@ -45,7 +45,7 @@ describe CanCan::ActiveRecordAdditions do end end # @ability.conditions(:read, @model_class).should == '(too.car=1 AND too.far.bar=1) OR (foo.bar=1)' - # @ability.association_joins(:read, @model_class).should == [{:too => [:far]}, :foo] + # @ability.associations_hash(:read, @model_class).should == [{:too => [:far]}, :foo] @model_class.accessible_by(@ability).should == :found_records end end diff --git a/spec/cancan/can_definition_spec.rb b/spec/cancan/can_definition_spec.rb index 7ef74dc..a11efb8 100644 --- a/spec/cancan/can_definition_spec.rb +++ b/spec/cancan/can_definition_spec.rb @@ -7,38 +7,38 @@ describe CanCan::CanDefinition do end it "should return no association joins if none exist" do - @can.association_joins.should be_nil + @can.associations_hash.should == {} end it "should return no association for joins if just attributes" do @conditions[:foo] = :bar - @can.association_joins.should be_nil + @can.associations_hash.should == {} end it "should return single association for joins" do @conditions[:foo] = {:bar => 1} - @can.association_joins.should == [{:foo=>[]}] + @can.associations_hash.should == {:foo => {}} end it "should return multiple associations for joins" do @conditions[:foo] = {:bar => 1} @conditions[:test] = {1 => 2} - @can.association_joins.map(&:to_s).sort.should == [:foo, :test].map(&:to_s).sort + @can.associations_hash.should == {:foo => {}, :test => {}} end it "should return nested associations for joins" do @conditions[:foo] = {:bar => {1 => 2}} - @can.association_joins.should == [{:foo => [{:bar=>[]}]}] + @can.associations_hash.should == {:foo => {:bar => {}}} end it "should return table names in conditions for association joins" do @conditions[:foo] = {:bar => 1} @conditions[:test] = 1 - @can.tableized_conditions.should == { :foos => { :bar => 1}, :test => 1 } + @can.tableized_conditions.should == {:foos => {:bar => 1}, :test => 1} end it "should return no association joins if conditions is nil" do can = CanCan::CanDefinition.new(true, :read, Integer, nil, nil) - can.association_joins.should be_nil + can.associations_hash.should == {} end end diff --git a/spec/cancan/query_spec.rb b/spec/cancan/query_spec.rb index 56878ac..746e8dc 100644 --- a/spec/cancan/query_spec.rb +++ b/spec/cancan/query_spec.rb @@ -41,35 +41,30 @@ describe CanCan::Query do it "should return SQL for single `can` definition in front of default `cannot` condition" do @ability.cannot :read, Person @ability.can :read, Person, :blocked => false, :user_id => 1 - - result = @ability.query(:read, Person).conditions.should orderlessly_match("blocked=false AND user_id=1") + @ability.query(:read, Person).conditions.should orderlessly_match("blocked=false AND user_id=1") end it "should return true condition for single `can` definition in front of default `can` condition" do @ability.can :read, Person @ability.can :read, Person, :blocked => false, :user_id => 1 - @ability.query(:read, Person).conditions.should == 'true=true' end it "should return false condition for single `cannot` definition" do @ability.cannot :read, Person, :blocked => true, :user_id => 1 - @ability.query(:read, Person).conditions.should == 'true=false' end it "should return `false condition` for single `cannot` definition in front of default `cannot` condition" do @ability.cannot :read, Person @ability.cannot :read, Person, :blocked => true, :user_id => 1 - @ability.query(:read, Person).conditions.should == 'true=false' end it "should return `not (sql)` for single `cannot` definition in front of default `can` condition" do @ability.can :read, Person @ability.cannot :read, Person, :blocked => true, :user_id => 1 - - result = @ability.query(:read, Person).conditions.should orderlessly_match("not (blocked=true AND user_id=1)") + @ability.query(:read, Person).conditions.should orderlessly_match("not (blocked=true AND user_id=1)") end it "should return appropriate sql conditions in complex case" do @@ -77,9 +72,36 @@ describe CanCan::Query do @ability.can :manage, Person, :id => 1 @ability.can :update, Person, :manager_id => 1 @ability.cannot :update, Person, :self_managed => true - @ability.query(:update, Person).conditions.should == 'not (self_managed=true) AND ((manager_id=1) OR (id=1))' @ability.query(:manage, Person).conditions.should == {:id=>1} @ability.query(:read, Person).conditions.should == 'true=true' end + + it "should have nil joins if no can definitions" do + @ability.query(:read, Person).joins.should be_nil + end + + it "should have nil joins if no nested hashes specified in conditions" do + @ability.can :read, Person, :blocked => false + @ability.can :read, Person, :admin => true + @ability.query(:read, Person).joins.should be_nil + end + + it "should merge separate joins into a single array" do + @ability.can :read, Person, :project => { :blocked => false } + @ability.can :read, Person, :company => { :admin => true } + @ability.query(:read, Person).joins.inspect.should orderlessly_match([:company, :project].inspect) + end + + it "should merge same joins into a single array" do + @ability.can :read, Person, :project => { :blocked => false } + @ability.can :read, Person, :project => { :admin => true } + @ability.query(:read, Person).joins.should == [:project] + end + + it "should merge complex, nested joins" do + @ability.can :read, Person, :project => { :bar => {:test => true} }, :company => { :bar => {:test => true} } + @ability.can :read, Person, :project => { :foo => {:bar => true}, :bar => {:zip => :zap} } + @ability.query(:read, Person).joins.inspect.should orderlessly_match([{:project => [:bar, :foo]}, {:company => [:bar]}].inspect) + end end