From 44db0cf3f7c864feafc387f0f28f87d2f6536884 Mon Sep 17 00:00:00 2001 From: Gabriel Hilal Date: Thu, 5 Nov 2015 11:20:37 -0200 Subject: [PATCH] Implements the has_cached_role? and has_strict_cached_role? methods. ```ruby @user.add_role :admin, Forum @user.add_role :member, Forum users = User.with_role(:admin, Forum).preload(:roles) users.each do |user| user.has_cached_role?(:member, Forum) # no extra queries end ``` This method should be used with caution. If you don't preload the roles, the `has_cached_role?` might return `false`. In the above example, it would return `false` for `@user.has_cached_role?(:member, Forum)`, because `User.with_role(:admin, Forum)` will load only the `:admin` roles. --- README.md | 14 ++++ .../adapters/active_record/role_adapter.rb | 22 +++++++ lib/rolify/adapters/mongoid/role_adapter.rb | 22 +++++++ lib/rolify/role.rb | 9 +++ spec/rolify/resource_spec.rb | 5 ++ .../shared_examples_for_has_role.rb | 65 +++++++++++++++++++ 6 files changed, 137 insertions(+) diff --git a/README.md b/README.md index a2efb96c..0e9a768e 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,20 @@ end ``` I.e. you get true only on a role that you manually add. +### Cached Roles (to avoid N+1 issue) + +```ruby +@user.add_role :admin, Forum +@user.add_role :member, Forum + +users = User.with_role(:admin, Forum).preload(:roles) +users.each do |user| + user.has_cached_role?(:member, Forum) # no extra queries +end +``` + +This method should be used with caution. If you don't preload the roles, the `has_cached_role?` might return `false`. In the above example, it would return `false` for `@user.has_cached_role?(:member, Forum)`, because `User.with_role(:admin, Forum)` will load only the `:admin` roles. + ## Resources * [Wiki](https://github.com/RolifyCommunity/rolify/wiki) diff --git a/lib/rolify/adapters/active_record/role_adapter.rb b/lib/rolify/adapters/active_record/role_adapter.rb index d72d68e5..71a3f99a 100644 --- a/lib/rolify/adapters/active_record/role_adapter.rb +++ b/lib/rolify/adapters/active_record/role_adapter.rb @@ -18,6 +18,28 @@ def where_strict(relation, args) relation.where(:name => args[:name], :resource_type => resource[:class], :resource_id => resource[:id]) end + def find_cached(relation, args) + resource_id = (args[:resource].nil? || args[:resource].is_a?(Class) || args[:resource] == :any) ? nil : args[:resource].id + resource_type = args[:resource].is_a?(Class) ? args[:resource].to_s : args[:resource].class.name + + return relation.find_all { |role| role.name == args[:name].to_s } if args[:resource] == :any + + relation.find_all do |role| + (role.name == args[:name].to_s && role.resource_type == nil && role.resource_id == nil) || + (role.name == args[:name].to_s && role.resource_type == resource_type && role.resource_id == nil) || + (role.name == args[:name].to_s && role.resource_type == resource_type && role.resource_id == resource_id) + end + end + + def find_cached_strict(relation, args) + resource_id = (args[:resource].nil? || args[:resource].is_a?(Class)) ? nil : args[:resource].id + resource_type = args[:resource].is_a?(Class) ? args[:resource].to_s : args[:resource].class.name + + relation.find_all do |role| + role.resource_id == resource_id && role.resource_type == resource_type && role.name == args[:name].to_s + end + end + def find_or_create_by(role_name, resource_type = nil, resource_id = nil) role_class.where(:name => role_name, :resource_type => resource_type, :resource_id => resource_id).first_or_create end diff --git a/lib/rolify/adapters/mongoid/role_adapter.rb b/lib/rolify/adapters/mongoid/role_adapter.rb index 9f77c0ce..01bbd632 100644 --- a/lib/rolify/adapters/mongoid/role_adapter.rb +++ b/lib/rolify/adapters/mongoid/role_adapter.rb @@ -18,6 +18,28 @@ def where_strict(relation, args) relation.where(:name => args[:name], :resource_type => resource[:class], :resource_id => resource[:id]) end + def find_cached(relation, args) + resource_id = (args[:resource].nil? || args[:resource].is_a?(Class) || args[:resource] == :any) ? nil : args[:resource].id + resource_type = args[:resource].is_a?(Class) ? args[:resource].to_s : args[:resource].class.name + + return relation.find_all { |role| role.name == args[:name].to_s } if args[:resource] == :any + + relation.find_all do |role| + (role.name == args[:name].to_s && role.resource_type == nil && role.resource_id == nil) || + (role.name == args[:name].to_s && role.resource_type == resource_type && role.resource_id == nil) || + (role.name == args[:name].to_s && role.resource_type == resource_type && role.resource_id == resource_id) + end + end + + def find_cached_strict(relation, args) + resource_id = (args[:resource].nil? || args[:resource].is_a?(Class)) ? nil : args[:resource].id + resource_type = args[:resource].is_a?(Class) ? args[:resource].to_s : args[:resource].class.name + + relation.find_all do |role| + role.resource_id == resource_id && role.resource_type == resource_type && role.name == args[:name].to_s + end + end + def find_or_create_by(role_name, resource_type = nil, resource_id = nil) self.role_class.find_or_create_by(:name => role_name, :resource_type => resource_type, diff --git a/lib/rolify/role.rb b/lib/rolify/role.rb index 0fba4a83..bb05f8d6 100644 --- a/lib/rolify/role.rb +++ b/lib/rolify/role.rb @@ -44,6 +44,15 @@ def has_strict_role?(role_name, resource) self.class.adapter.where_strict(self.roles, name: role_name, resource: resource).any? end + def has_cached_role?(role_name, resource = nil) + return has_strict_cached_role?(role_name, resource) if self.class.strict_rolify and resource and resource != :any + self.class.adapter.find_cached(self.roles, name: role_name, resource: resource).any? + end + + def has_strict_cached_role?(role_name, resource = nil) + self.class.adapter.find_cached_strict(self.roles, name: role_name, resource: resource).any? + end + def has_all_roles?(*args) args.each do |arg| if arg.is_a? Hash diff --git a/spec/rolify/resource_spec.rb b/spec/rolify/resource_spec.rb index 076c6d1a..3e82bfa1 100644 --- a/spec/rolify/resource_spec.rb +++ b/spec/rolify/resource_spec.rb @@ -548,23 +548,28 @@ it "should return only strict forum" do @strict_user.has_role?(:forum, Forum.first).should be true + @strict_user.has_cached_role?(:forum, Forum.first).should be true end it "should return false on strict another forum" do @strict_user.has_role?(:forum, Forum.last).should be false + @strict_user.has_cached_role?(:forum, Forum.last).should be false end it "should return true if user has role on Forum model" do @strict_user.has_role?(:forum, Forum).should be true + @strict_user.has_cached_role?(:forum, Forum).should be true end it "should return true if user has role any forum name" do @strict_user.has_role?(:forum, :any).should be true + @strict_user.has_cached_role?(:forum, :any).should be true end it "should return false when deleted role on Forum model" do @strict_user.remove_role(:forum, Forum) @strict_user.has_role?(:forum, Forum).should be false + @strict_user.has_cached_role?(:forum, Forum).should be false end end end diff --git a/spec/rolify/shared_examples/shared_examples_for_has_role.rb b/spec/rolify/shared_examples/shared_examples_for_has_role.rb index b1c23c14..3311c591 100644 --- a/spec/rolify/shared_examples/shared_examples_for_has_role.rb +++ b/spec/rolify/shared_examples/shared_examples_for_has_role.rb @@ -3,10 +3,16 @@ context "with a global role", :scope => :global do it { subject.has_role?("admin".send(param_method)).should be_truthy } + it { subject.has_cached_role?("admin".send(param_method)).should be_truthy } + context "on resource request" do it { subject.has_role?("admin".send(param_method), Forum.first).should be_truthy } it { subject.has_role?("admin".send(param_method), Forum).should be_truthy } it { subject.has_role?("admin".send(param_method), :any).should be_truthy } + + it { subject.has_cached_role?("admin".send(param_method), Forum.first).should be_truthy } + it { subject.has_cached_role?("admin".send(param_method), Forum).should be_truthy } + it { subject.has_cached_role?("admin".send(param_method), :any).should be_truthy } end context "with another global role" do @@ -14,19 +20,29 @@ it { subject.has_role?("global".send(param_method)).should be_falsey } it { subject.has_role?("global".send(param_method), :any).should be_falsey } + + it { subject.has_cached_role?("global".send(param_method)).should be_falsey } + it { subject.has_cached_role?("global".send(param_method), :any).should be_falsey } end it "should not get an instance scoped role" do subject.has_role?("moderator".send(param_method), Group.first).should be_falsey + + subject.has_cached_role?("moderator".send(param_method), Group.first).should be_falsey end it "should not get a class scoped role" do subject.has_role?("manager".send(param_method), Forum).should be_falsey + + subject.has_cached_role?("manager".send(param_method), Forum).should be_falsey end context "using inexisting role" do it { subject.has_role?("dummy".send(param_method)).should be_falsey } it { subject.has_role?("dumber".send(param_method), Forum.first).should be_falsey } + + it { subject.has_cached_role?("dummy".send(param_method)).should be_falsey } + it { subject.has_cached_role?("dumber".send(param_method), Forum.first).should be_falsey } end end @@ -35,15 +51,23 @@ it { subject.has_role?("manager".send(param_method), Forum).should be_truthy } it { subject.has_role?("manager".send(param_method), Forum.first).should be_truthy } it { subject.has_role?("manager".send(param_method), :any).should be_truthy } + + it { subject.has_cached_role?("manager".send(param_method), Forum).should be_truthy } + it { subject.has_cached_role?("manager".send(param_method), Forum.first).should be_truthy } + it { subject.has_cached_role?("manager".send(param_method), :any).should be_truthy } end it "should not get a scoped role when asking for a global" do subject.has_role?("manager".send(param_method)).should be_falsey + + subject.has_cached_role?("manager".send(param_method)).should be_falsey end it "should not get a global role" do role_class.create(:name => "admin") subject.has_role?("admin".send(param_method)).should be_falsey + + subject.has_cached_role?("admin".send(param_method)).should be_falsey end context "with another class scoped role" do @@ -52,6 +76,9 @@ it { subject.has_role?("member".send(param_method), Forum).should be_falsey } it { subject.has_role?("member".send(param_method), :any).should be_falsey } + + it { subject.has_cached_role?("member".send(param_method), Forum).should be_falsey } + it { subject.has_cached_role?("member".send(param_method), :any).should be_falsey } end context "on another resource with the same name" do @@ -59,6 +86,9 @@ it { subject.has_role?("manager".send(param_method), Group).should be_falsey } it { subject.has_role?("manager".send(param_method), :any).should be_truthy } + + it { subject.has_cached_role?("manager".send(param_method), Group).should be_falsey } + it { subject.has_cached_role?("manager".send(param_method), :any).should be_truthy } end context "on another resource with another name" do @@ -66,12 +96,18 @@ it { subject.has_role?("defenders".send(param_method), Group).should be_falsey } it { subject.has_role?("defenders".send(param_method), :any).should be_falsey } + + it { subject.has_cached_role?("defenders".send(param_method), Group).should be_falsey } + it { subject.has_cached_role?("defenders".send(param_method), :any).should be_falsey } end end context "using inexisting role" do it { subject.has_role?("dummy".send(param_method), Forum).should be_falsey } it { subject.has_role?("dumber".send(param_method)).should be_falsey } + + it { subject.has_cached_role?("dummy".send(param_method), Forum).should be_falsey } + it { subject.has_cached_role?("dumber".send(param_method)).should be_falsey } end end @@ -84,19 +120,33 @@ m.add_role("moderator", Forum.first) m.has_role?("moderator".send(param_method), :any).should be_truthy } + + it { subject.has_cached_role?("moderator".send(param_method), Forum.first).should be_truthy } + it { subject.has_cached_role?("moderator".send(param_method), :any).should be_truthy } + it { + m = subject.class.new + m.add_role("moderator", Forum.first) + m.has_cached_role?("moderator".send(param_method), :any).should be_truthy + } end it "should not get an instance scoped role when asking for a global" do subject.has_role?("moderator".send(param_method)).should be_falsey + + subject.has_cached_role?("moderator".send(param_method)).should be_falsey end it "should not get an instance scoped role when asking for a class scoped" do subject.has_role?("moderator".send(param_method), Forum).should be_falsey + + subject.has_cached_role?("moderator".send(param_method), Forum).should be_falsey end it "should not get a global role" do role_class.create(:name => "admin") subject.has_role?("admin".send(param_method)).should be_falsey + + subject.has_cached_role?("admin".send(param_method)).should be_falsey end context "with another instance scoped role" do @@ -105,6 +155,9 @@ it { subject.has_role?("member".send(param_method), Forum.first).should be_falsey } it { subject.has_role?("member".send(param_method), :any).should be_falsey } + + it { subject.has_cached_role?("member".send(param_method), Forum.first).should be_falsey } + it { subject.has_cached_role?("member".send(param_method), :any).should be_falsey } end context "on another resource of the same type but with the same role name" do @@ -112,6 +165,9 @@ it { subject.has_role?("moderator".send(param_method), Forum.last).should be_falsey } it { subject.has_role?("moderator".send(param_method), :any).should be_truthy } + + it { subject.has_cached_role?("moderator".send(param_method), Forum.last).should be_falsey } + it { subject.has_cached_role?("moderator".send(param_method), :any).should be_truthy } end context "on another resource of different type but with the same role name" do @@ -119,6 +175,9 @@ it { subject.has_role?("moderator".send(param_method), Group.last).should be_falsey } it { subject.has_role?("moderator".send(param_method), :any).should be_truthy } + + it { subject.has_cached_role?("moderator".send(param_method), Group.last).should be_falsey } + it { subject.has_cached_role?("moderator".send(param_method), :any).should be_truthy } end context "on another resource of the same type and with another role name" do @@ -126,6 +185,9 @@ it { subject.has_role?("member".send(param_method), Forum.last).should be_falsey } it { subject.has_role?("member".send(param_method), :any).should be_falsey } + + it { subject.has_cached_role?("member".send(param_method), Forum.last).should be_falsey } + it { subject.has_cached_role?("member".send(param_method), :any).should be_falsey } end context "on another resource of different type and with another role name" do @@ -133,6 +195,9 @@ it { subject.has_role?("member".send(param_method), Group.first).should be_falsey } it { subject.has_role?("member".send(param_method), :any).should be_falsey } + + it { subject.has_cached_role?("member".send(param_method), Group.first).should be_falsey } + it { subject.has_cached_role?("member".send(param_method), :any).should be_falsey } end end end