I'm using Pundit gem for my authorization classes, where each controller action is checked against the model policy, to see if action is allowed by the user.
These methods are sometimes becoming quite bloated and unreadable, because I'm checking quite some stuff for some objects.
Now I'm thinking to refactor those methods, and place every "validation" in it's own method:
Previous:
class PostPolicy < ApplicationPolicy
def update
return true if @user.has_role? :admin
return true if @object.owner == user
return true if 'some other reason'
false
end
end
Now ideally, I want to refactor this in something like:
class PostPolicy < ApplicationPolicy
def update
allow_if_user_is_admin
allow_if_user_owns_record
allow_for_some_other_reason
false
end
private
def allow_if_user_is_admin
# this would go in the parent class, as the logic is the same for other objects
return true if @user.has_role? :admin
end
end
The problem now is, that the mane update
method will keep on going, even if the user is admin, as there's no return. If I would inlcude a return, then the other methods will never be evalutaed. Is there a way in ruby to do kind of a "superreturn", so that when the user is an admin, the main update
method would stop evaluting?
Thanks!
CodePudding user response:
Given your example and this comment: "...no native way to do kind of a 'super return' in Ruby? It feels like kind of a "raise" but then with a positive outcome... could I use that perhaps?".
While there are usually other ways to solve the issue that could be considered "more idiomatic", ruby does have a Kernel#throw
and Kernel#catch
implementation that can be very useful for control flow when navigating through numerous and possibly disparate methods and operations.
The throw
and corresponding catch
will short circuit the result of the block which appears to be the syntax you are looking for.
VERY Basic Example:
class PostPolicy
def initialize(n)
@n = n
end
def update
catch(:success) do
allow_if_user_is_admin
allow_if_user_owns_record
allow_for_some_other_reason
false
end
end
private
def allow_if_user_is_admin
puts "Is User Admin?"
throw(:success, true) if @n == 1
end
def allow_if_user_owns_record
puts "Is User Owner?"
throw(:success,true) if @n == 2
end
def allow_for_some_other_reason
puts "Is User Special?"
throw(:success,true) if @n == 3
end
end
Example Output:
PostPolicy.new(1).update
# Is User Admin?
#=> true
PostPolicy.new(2).update
# Is User Admin?
# Is User Owner?
#=> true
PostPolicy.new(3).update
# Is User Admin?
# Is User Owner?
# Is User Special?
#=> true
PostPolicy.new(4).update
# Is User Admin?
# Is User Owner?
# Is User Special?
#=> false
CodePudding user response:
What you can do is chain &&
operators.
As soon as one is false
, ruby will not evaluate the others (And the update method will return false
).
class PostPolicy < ApplicationPolicy
def update
allow_if_user_is_admin &&
allow_if_user_owns_record &&
allow_for_some_other_reason &&
end
private
def allow_if_user_is_admin
# this would go in the parent class, as the logic is the same for other objects
return true if @user.has_role? :admin
end
end
CodePudding user response:
It seems that this would achieve your aim and be more idiomatic:
class PostPolicy < ApplicationPolicy
def update
user_has_admin_role? ||
user_owns_object? ||
some_other_reason?
end
private
def user_has_admin_role?
@user.has_role? :admin
end
def user_owns_object?
@object.owner == user
end
def some_other_reason?
'some other reason'
end
end
CodePudding user response:
You can short-circuit boolean syntax, there me be cases where long chaining would look like bad style, here's an alternative, but basically same idea using Enumerable#all? See this answer for how it short-circuits
class PostPolicy < ApplicationPolicy
def update
permit?
end
private
def permit?
[
user_is_admin?,
user_owns_record,
allow_for_some_other_reason?,
thing1?,
thing2?
].all?
end
def user_is_admin?
@user.has_role? :admin
end
def user_owns_record?
@user.owns_record?
end
def allow_for_some_other_reason?
@user.has_cheezebuerger?
end
def thing1?
@user.thing1
end
def thing2?
@user.thing2
end
end