Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pretty print for policy rules #63

Merged
merged 2 commits into from
Mar 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
## master

- Added `Policy#pp(rule)` method to print annotated rule source code. ([@palkan][])

Example (debugging):

```ruby
def edit?
binding.pry # rubocop:disable Lint/Debugger
(user.name == "John") && (admin? || access_feed?)
end
```

```sh
pry> pp :edit?
MyPolicy#edit?
↳ (
user.name == "John" #=> false
)
AND
(
admin? #=> false
OR
access_feed? #=> true
)
)
```

See [PR#63](https://github.com/palkan/action_policy/pull/63)

- Added ability to provide additional failure reasons details. ([@palkan][])

Example:
Expand Down
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ gemspec

gem "pry-byebug", platform: :mri

gem "method_source"
gem "unparser"

gem 'sqlite3', "~> 1.3.0", platform: :mri
gem 'activerecord-jdbcsqlite3-adapter', '~> 50.0', platform: :jruby
gem 'jdbc-sqlite3', platform: :jruby
Expand Down
1 change: 1 addition & 0 deletions docs/_sidebar.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* [Failure Reasons](reasons.md)
* [Instrumentation](instrumentation.md)
* [I18n Support](i18n.md)
* [Debugging](debugging.md)
* Tips & Tricks
* [From Pundit to Action Policy](./pundit_migration.md)
* [Dealing with Decorators](./decorators.md)
Expand Down
55 changes: 55 additions & 0 deletions docs/debugging.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Debug Helpers

**NOTE:** this functionality requires two additional gems to be available in the app:
- [unparser](https://github.com/mbj/unparser)
- [method_source](https://github.com/banister/method_source).

We usually describe policy rules using _boolean expressions_ (e.g. `A or (B and C)` where each of `A`, `B` and `C` is a simple boolean expression or predicate method).

When dealing with complex policies, it could be hard to figure out which predicate/check made policy to fail.

The `Policy#pp(rule)` method aims to help debug such situations.

Consider a (synthetic) example:

```ruby
def feed?
(admin? || allowed_to?(:access_feed?)) &&
(user.name == "Jack" || user.name == "Kate")
end

```

Suppose that you want to debug this rule ("Why does it return false?").
You can drop a [`binding.pry`](https://github.com/deivid-rodriguez/pry-byebug) (or `binding.irb`) right at the beginning of the method:

```ruby
def feed?
binding.pry # rubocop:disable Lint/Debugger
#...
end
```

Now, run your code and trigger the breakpoint (i.e., run the method):

```
# now you can preview the execution of the rule using the `pp` method (defined on the policy instance)
pry> pp :feed?
MyPolicy#feed?
↳ (
admin? #=> false
OR
allowed_to?(:access_feed?) #=> true
)
AND
(
user.name == "Jack" #=> false
OR
user.name == "Kate" #=> true
)

# you can also check other rules or methods as well
pry> pp :admin?
MyPolicy#admin?
↳ user.admin? #=> false
```
24 changes: 24 additions & 0 deletions docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,30 @@ describe PostPolicy do
end
```

If test failed the exception message includes the result and [failure reasons](reasons) (if any):

```
1) PostPolucy#show? when post is draft
Failure/Error: ...

Expected to fail but succeed:
<PostPolicy#show?: true (reasons: ...)>
```

If you have [debugging utils](debugging) installed the message also includes the _annotated_
source code of the policy rule:

```
1) UserPolucy#manage? when post is draft
Failure/Error: ...

Expected to fail but succeed:
<PostPolicy#show?: true (reasons: ...)>
↳ user.admin? #=> true
OR
!record.draft? #=> false
```

**NOTE:** DSL for focusing or skipping examples and groups is also available (e.g. `xdescribe_rule`, `fsucceed`, etc.).

**NOTE:** the DSL is included only to example with the tag `type: :policy` or in the `spec/policies` folder. If you want to add this DSL to other examples, add `include ActionPolicy::RSpec::PolicyExampleGroup`.
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/rails42.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@ source "https://rubygems.org"

gem "sqlite3", "~> 1.3.0"
gem "rails", "~> 4.2"
gem "method_source"
gem "unparser"

gemspec path: ".."
4 changes: 3 additions & 1 deletion gemfiles/rails6.gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
source "https://rubygems.org"

gem "sqlite3"
gem "rails", "6.0.0.beta2"
gem "rails", "6.0.0.beta3"
gem "method_source"
gem "unparser"

gemspec path: ".."
21 changes: 21 additions & 0 deletions lib/action_policy/policy/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require "action_policy/behaviours/policy_for"
require "action_policy/policy/execution_result"
require "action_policy/utils/suggest_message"
require "action_policy/utils/pretty_print"

unless "".respond_to?(:underscore)
require "action_policy/ext/string_underscore"
Expand Down Expand Up @@ -120,6 +121,26 @@ def resolve_rule(activity)
respond_to?(activity)
activity
end

# Return annotated source code for the rule
# NOTE: require "method_source" and "unparser" gems to be installed.
# Otherwise returns empty string.
def inspect_rule(rule)
PrettyPrint.print_method(self, rule)
end

# Helper for printing the annotated rule source.
# Useful for debugging: type `pp :show?` within the context of the policy
# to preview the rule.
def pp(rule)
with_clean_result do
# We need result to exist for `allowed_to?` to work correctly
@result = self.class.result_class.new(self.class, rule)
header = "#{self.class.name}##{rule}"
source = inspect_rule(rule)
$stdout.puts "#{header}\n#{source}"
end
end
end
end
end
8 changes: 6 additions & 2 deletions lib/action_policy/rspec/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def #{prefix}succeed(*args, **kwargs)
context(*args) do
instance_eval(&Proc.new) if block_given?
#{prefix}it "succeeds", kwargs do
is_expected.to be_success, "Expected succeed but failed: \#{policy.result.inspect}"
is_expected.to be_success, "Expected succeed but failed:\n\#{formatted_policy(policy)}"
end
end
end
Expand All @@ -35,7 +35,7 @@ def #{prefix}failed(*args, **kwargs)
context(*args) do
instance_eval(&Proc.new) if block_given?
#{prefix}it "fails", kwargs do
is_expected.to be_fail, "Expected to fail but succeed: \#{policy.result.inspect}"
is_expected.to be_fail, "Expected to fail but succeed:\n\#{formatted_policy(policy)}"
end
end
end
Expand All @@ -51,6 +51,10 @@ def self.included(base)
base.let(:policy) { described_class.new(record, context) }
super
end

def formatted_policy(policy)
"#{policy.result.inspect}\n#{policy.inspect_rule(policy.result.rule)}"
end
end
end
end
Expand Down
128 changes: 128 additions & 0 deletions lib/action_policy/utils/pretty_print.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# frozen_string_literal: true

begin
require "method_source"
require "parser/current"
require "unparser"
rescue LoadError
# do nothing
end

module ActionPolicy
require "action_policy/ext/yield_self_then"
using ActionPolicy::Ext::YieldSelfThen

# Takes the object and a method name,
# and returns the "annotated" source code for the method:
# code is split into parts by logical operators and each
# part is evaluated separately.
#
# Example:
#
# class MyClass
# def access?
# admin? && access_feed?
# end
# end
#
# puts PrettyPrint.format_method(MyClass.new, :access?)
#
# #=> MyClass#access?
# #=> ↳ admin? #=> false
# #=> AND
# #=> access_feed? #=> true
module PrettyPrint
class Visitor
attr_reader :lines, :object
attr_accessor :indent

def initialize(object)
@object = object
end

def collect(ast)
@lines = []
@indent = 0

visit_node(ast)

lines.join("\n")
end

def visit_node(ast)
if respond_to?("visit_#{ast.type}")
send("visit_#{ast.type}", ast)
else
visit_missing ast
end
end

def expression_with_result(sexp)
expression = Unparser.unparse(sexp)
"#{expression} #=> #{eval_exp(expression)}"
end

def eval_exp(exp)
object.instance_eval(exp)
rescue => e
"Failed: #{e.message}"
end

def visit_and(ast)
visit_node(ast.children[0])
lines << indented("AND")
visit_node(ast.children[1])
end

def visit_or(ast)
visit_node(ast.children[0])
lines << indented("OR")
visit_node(ast.children[1])
end

# Parens
def visit_begin(ast)
lines << indented("(")
self.indent += 2
visit_node(ast.children[0])
self.indent -= 2
lines << indented(")")
end

def visit_missing(ast)
lines << indented(expression_with_result(ast))
end

def indented(str)
"#{indent.zero? ? "↳ " : ""}#{" " * indent}#{str}".tap do
# increase indent after the first expression
self.indent += 2 if indent.zero?
end
end
end

class << self
if defined?(::Unparser) && defined?(::MethodSource)
def available?
true
end

def print_method(object, method_name)
ast = object.method(method_name).source.then(&Unparser.method(:parse))
# outer node is a method definition itself
body = ast.children[2]

Visitor.new(object).collect(body)
end
else
def available?
false
end

def print_method(_, _)
""
end
end
end
end
end
15 changes: 15 additions & 0 deletions spec/action_policy/dsl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,20 @@ def manage?
let(:record) { User.new("admin") }
end
end

context "test errors" do
after do |ex|
msg = ex.exception.message
# mark as not failed
ex.remove_instance_variable(:@exception)

expect(msg).to include("<UserPolicy#manage?: true>")
expect(msg).to include("↳ user.admin? #=> true") if ActionPolicy::PrettyPrint.available?
end

failed do
let(:user) { admin }
end
end
end
end
Loading