From b439900e273113818d7c3f4ae47cfa33abf39e90 Mon Sep 17 00:00:00 2001 From: Derek Kniffin Date: Fri, 3 Mar 2017 13:42:56 -0500 Subject: [PATCH 1/6] Change with_any_role to return an AR relation instead of an Array Fixes #292 --- .../adapters/active_record/role_adapter.rb | 9 ++-- lib/rolify/finders.rb | 47 ++++++++++++------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/rolify/adapters/active_record/role_adapter.rb b/lib/rolify/adapters/active_record/role_adapter.rb index 54c385d3..4d2930e5 100644 --- a/lib/rolify/adapters/active_record/role_adapter.rb +++ b/lib/rolify/adapters/active_record/role_adapter.rb @@ -104,15 +104,16 @@ def build_conditions(relation, args) end def build_query(role, resource = nil) - return [ "#{role_table}.name = ?", [ role ] ] if resource == :any - query = "((#{role_table}.name = ?) AND (#{role_table}.resource_type IS NULL) AND (#{role_table}.resource_id IS NULL))" + role = [role].flatten + return [ "#{role_table}.name IN (?)", [ role ] ] if resource == :any + query = "((#{role_table}.name IN (?)) AND (#{role_table}.resource_type IS NULL) AND (#{role_table}.resource_id IS NULL))" values = [ role ] if resource query.insert(0, "(") - query += " OR ((#{role_table}.name = ?) AND (#{role_table}.resource_type = ?) AND (#{role_table}.resource_id IS NULL))" + query += " OR ((#{role_table}.name IN (?)) AND (#{role_table}.resource_type = ?) AND (#{role_table}.resource_id IS NULL))" values << role << (resource.is_a?(Class) ? resource.to_s : resource.class.name) if !resource.is_a? Class - query += " OR ((#{role_table}.name = ?) AND (#{role_table}.resource_type = ?) AND (#{role_table}.resource_id = ?))" + query += " OR ((#{role_table}.name IN (?)) AND (#{role_table}.resource_type = ?) AND (#{role_table}.resource_id = ?))" values << role << resource.class.name << resource.id end query += ")" diff --git a/lib/rolify/finders.rb b/lib/rolify/finders.rb index b17557d5..e6f904ab 100644 --- a/lib/rolify/finders.rb +++ b/lib/rolify/finders.rb @@ -10,7 +10,7 @@ def without_role(role_name, resource = nil) def with_all_roles(*args) users = [] - parse_args(args, users) do |users_to_add| + parse_args(args) do |users_to_add| users = users_to_add if users.empty? users &= users_to_add return [] if users.empty? @@ -19,26 +19,37 @@ def with_all_roles(*args) end def with_any_role(*args) - users = [] - parse_args(args, users) do |users_to_add| - users += users_to_add - end - users.uniq + union_ars(parse_args(args)) end end - + private - - def parse_args(args, users, &block) - args.each do |arg| - if arg.is_a? Hash - users_to_add = self.with_role(arg[:name], arg[:resource]) - elsif arg.is_a?(String) || arg.is_a?(Symbol) - users_to_add = self.with_role(arg) - else - raise ArgumentError, "Invalid argument type: only hash or string or symbol allowed" + + def parse_args(args, &block) + normalize_args(args).map do |arg| + self.with_role(arg[:name], arg[:resource]).tap do |users_to_add| + block.call(users_to_add) if block end - block.call(users_to_add) end end -end \ No newline at end of file + + # In: [:a, "b", { name: :c, resource: :d }] + # Out: [{ name: [:a, "b"] }, { name: :c, resource: :d }] + def normalize_args(args) + groups = args.group_by(&:class) + unless groups.keys.all? { |type| [Hash, Symbol, String].include?(type) } + raise ArgumentError, "Invalid argument type: only hash or string or symbol allowed" + end + + normalized = [groups[Hash]] + sym_str_args = [groups[Symbol], groups[String]].flatten.compact + normalized += [{ name: sym_str_args }] unless sym_str_args.empty? + + normalized.flatten.compact + end + + def union_ars(ars) + query = ars.map(&:to_sql).join(" UNION ") + from("(#{query}) AS #{table_name}") + end +end From 5358ff326e221c75a87e545dfae79465d269ee2b Mon Sep 17 00:00:00 2001 From: Derek Kniffin Date: Fri, 3 Mar 2017 17:05:51 -0500 Subject: [PATCH 2/6] Add tests to ensure with_any_role returns an AR relation --- .../shared_examples/shared_examples_for_finders.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/spec/rolify/shared_examples/shared_examples_for_finders.rb b/spec/rolify/shared_examples/shared_examples_for_finders.rb index 0c9194c7..6fcdffdd 100644 --- a/spec/rolify/shared_examples/shared_examples_for_finders.rb +++ b/spec/rolify/shared_examples/shared_examples_for_finders.rb @@ -97,7 +97,7 @@ end end end - + describe ".with_all_roles" do it { should respond_to(:with_all_roles) } @@ -126,6 +126,11 @@ it { subject.with_any_role({ :name => "visitor".send(param_method), :resource => Forum.last }, { :name => "moderator".send(param_method), :resource => Group }).should =~ [ root, visitor ] } it { subject.with_any_role({ :name => "visitor".send(param_method), :resource => Group.first }, { :name => "moderator".send(param_method), :resource => Forum }).should eq([ modo ]) } it { subject.with_any_role({ :name => "visitor".send(param_method), :resource => :any }, { :name => "moderator".send(param_method), :resource => :any }).should =~ [ root, modo, visitor ] } + + it 'should return an AR relation' do + subject.with_any_role("admin".send(params_method), :staff, { :name => "moderator".send(param_method), :resource => Group }).should be_a(ActiveRecord::Relation) + subject.with_any_role({ :name => "visitor".send(param_method), :resource => :any }, { :name => "moderator".send(param_method), :resource => :any }).should be_a(ActiveRecord::Relation) + end end end -end \ No newline at end of file +end From 388326b5d893ed4d7e33a9d6e64c594e8bd3ad7c Mon Sep 17 00:00:00 2001 From: Derek Kniffin Date: Fri, 3 Mar 2017 17:06:01 -0500 Subject: [PATCH 3/6] Uniq the results --- lib/rolify/finders.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rolify/finders.rb b/lib/rolify/finders.rb index e6f904ab..81a9d735 100644 --- a/lib/rolify/finders.rb +++ b/lib/rolify/finders.rb @@ -19,7 +19,7 @@ def with_all_roles(*args) end def with_any_role(*args) - union_ars(parse_args(args)) + union_ars(parse_args(args)).uniq end end From f2a15861b1332cd27ccd85ef8a3daaafa2e39b83 Mon Sep 17 00:00:00 2001 From: Derek Kniffin Date: Fri, 3 Mar 2017 17:27:00 -0500 Subject: [PATCH 4/6] Intersect for with_all_roles --- lib/rolify/finders.rb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/rolify/finders.rb b/lib/rolify/finders.rb index 81a9d735..aa6aa549 100644 --- a/lib/rolify/finders.rb +++ b/lib/rolify/finders.rb @@ -9,13 +9,7 @@ def without_role(role_name, resource = nil) end def with_all_roles(*args) - users = [] - parse_args(args) do |users_to_add| - users = users_to_add if users.empty? - users &= users_to_add - return [] if users.empty? - end - users + intersect_ars(parse_args(args)).uniq end def with_any_role(*args) @@ -48,6 +42,13 @@ def normalize_args(args) normalized.flatten.compact end + + def intersect_ars(ars) + query = ars.map(&:to_sql).join(" INTERSECT ") + from("(#{query}) AS #{table_name}") + end + + # http://stackoverflow.com/a/16868735/1202488 def union_ars(ars) query = ars.map(&:to_sql).join(" UNION ") from("(#{query}) AS #{table_name}") From b1ec451ac3a33d429415f6ee90c5afe6bfc2ea25 Mon Sep 17 00:00:00 2001 From: Derek Kniffin Date: Fri, 3 Mar 2017 17:28:59 -0500 Subject: [PATCH 5/6] Add tests to ensure with_all_roles returns an AR relation --- spec/rolify/shared_examples/shared_examples_for_finders.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/rolify/shared_examples/shared_examples_for_finders.rb b/spec/rolify/shared_examples/shared_examples_for_finders.rb index 6fcdffdd..f6fb7727 100644 --- a/spec/rolify/shared_examples/shared_examples_for_finders.rb +++ b/spec/rolify/shared_examples/shared_examples_for_finders.rb @@ -111,6 +111,11 @@ it { subject.with_all_roles({ :name => "visitor".send(param_method), :resource => Forum.last }, { :name => "moderator".send(param_method), :resource => Group }).should eq([ root ]) } it { subject.with_all_roles({ :name => "visitor".send(param_method), :resource => Group.first }, { :name => "moderator".send(param_method), :resource => Forum }).should eq([ modo ]) } it { subject.with_all_roles({ :name => "visitor".send(param_method), :resource => :any }, { :name => "moderator".send(param_method), :resource => :any }).should =~ [ root, modo ] } + + it 'should return an AR relation' do + subject.with_all_roles("admin".send(param_method), :staff).should be_a(ActiveRecord::Relation) + subject.with_all_roles({ :name => "visitor".send(param_method), :resource => :any }, { :name => "moderator".send(param_method), :resource => :any }).should be_a(ActiveRecord::Relation) + end end describe ".with_any_role" do From d4276aad67373050098c5ec9501c7b92340df01e Mon Sep 17 00:00:00 2001 From: Derek Kniffin Date: Fri, 3 Mar 2017 18:18:02 -0500 Subject: [PATCH 6/6] Fix some failing test cases --- lib/rolify/finders.rb | 17 +++++++++++------ .../shared_examples_for_finders.rb | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/rolify/finders.rb b/lib/rolify/finders.rb index aa6aa549..8ae2b53a 100644 --- a/lib/rolify/finders.rb +++ b/lib/rolify/finders.rb @@ -9,18 +9,18 @@ def without_role(role_name, resource = nil) end def with_all_roles(*args) - intersect_ars(parse_args(args)).uniq + intersect_ars(parse_args(args, :split)).uniq end def with_any_role(*args) - union_ars(parse_args(args)).uniq + union_ars(parse_args(args, :join)).uniq end end private - def parse_args(args, &block) - normalize_args(args).map do |arg| + def parse_args(args, mode, &block) + normalize_args(args, mode).map do |arg| self.with_role(arg[:name], arg[:resource]).tap do |users_to_add| block.call(users_to_add) if block end @@ -29,7 +29,7 @@ def parse_args(args, &block) # In: [:a, "b", { name: :c, resource: :d }] # Out: [{ name: [:a, "b"] }, { name: :c, resource: :d }] - def normalize_args(args) + def normalize_args(args, mode) groups = args.group_by(&:class) unless groups.keys.all? { |type| [Hash, Symbol, String].include?(type) } raise ArgumentError, "Invalid argument type: only hash or string or symbol allowed" @@ -37,7 +37,12 @@ def normalize_args(args) normalized = [groups[Hash]] sym_str_args = [groups[Symbol], groups[String]].flatten.compact - normalized += [{ name: sym_str_args }] unless sym_str_args.empty? + case mode + when :join + normalized += [{ name: sym_str_args }] unless sym_str_args.empty? + when :split + normalized += sym_str_args.map { |arg| { name: arg } } + end normalized.flatten.compact end diff --git a/spec/rolify/shared_examples/shared_examples_for_finders.rb b/spec/rolify/shared_examples/shared_examples_for_finders.rb index f6fb7727..13cb75f2 100644 --- a/spec/rolify/shared_examples/shared_examples_for_finders.rb +++ b/spec/rolify/shared_examples/shared_examples_for_finders.rb @@ -133,7 +133,7 @@ it { subject.with_any_role({ :name => "visitor".send(param_method), :resource => :any }, { :name => "moderator".send(param_method), :resource => :any }).should =~ [ root, modo, visitor ] } it 'should return an AR relation' do - subject.with_any_role("admin".send(params_method), :staff, { :name => "moderator".send(param_method), :resource => Group }).should be_a(ActiveRecord::Relation) + subject.with_any_role("admin".send(param_method), :staff, { :name => "moderator".send(param_method), :resource => Group }).should be_a(ActiveRecord::Relation) subject.with_any_role({ :name => "visitor".send(param_method), :resource => :any }, { :name => "moderator".send(param_method), :resource => :any }).should be_a(ActiveRecord::Relation) end end