From c9c5aab2c21b3df450ed38b6384b6868151f2d2f Mon Sep 17 00:00:00 2001 From: Akira Yoshiyama Date: Wed, 2 Dec 2015 11:55:05 +0900 Subject: [PATCH] Fixes combined "and" and "or" rule handling The text parser handles rules like below: - A or B or C [or D...] - A and B and C [and D...] But it doesn't ones below: - A or B and C - A and B or C So, this patch fixes them with: - for "A and B or C": adds @reducer('and_expr', 'or', 'check') to _make_or_expr(). - for "A or B and C": adds _mix_or_and_expr() method. It pops the last check (B) from OrCheck rule list [A, B] and append AndCheck with rule list [B, C] to the Or Check rule list. So, finally we will get "OrCheck[A, AndCheck[B, C]]". Change-Id: Iaaee4864356411374ee7e7c5c0c05b98889e0f4e Closes-Bug: #1523030 --- oslo_policy/_checks.py | 10 +++ oslo_policy/_parser.py | 13 ++++ oslo_policy/policy.py | 16 ++++ oslo_policy/tests/test_checks.py | 7 ++ oslo_policy/tests/test_parser.py | 129 +++++++++++++++++++++++++++++++ 5 files changed, 175 insertions(+) diff --git a/oslo_policy/_checks.py b/oslo_policy/_checks.py index 07fcfd9f..2daf37fe 100644 --- a/oslo_policy/_checks.py +++ b/oslo_policy/_checks.py @@ -171,6 +171,16 @@ class OrCheck(BaseCheck): self.rules.append(rule) return self + def pop_check(self): + """Pops the last check from the list and returns them + + :returns: self, the popped check + :rtype: :class:`.OrCheck`, class:`.Check` + """ + + check = self.rules.pop() + return self, check + def register(name, func=None): # Perform the actual decoration by registering the function or diff --git a/oslo_policy/_parser.py b/oslo_policy/_parser.py index 29be1b49..2225c2df 100644 --- a/oslo_policy/_parser.py +++ b/oslo_policy/_parser.py @@ -159,6 +159,18 @@ class ParseState(object): return [('and_expr', _checks.AndCheck([check1, check2]))] + @reducer('or_expr', 'and', 'check') + def _mix_or_and_expr(self, or_expr, _and, check): + """Modify the case 'A or B and C'""" + + or_expr, check1 = or_expr.pop_check() + if isinstance(check1, _checks.AndCheck): + and_expr = check1 + and_expr.add_check(check) + else: + and_expr = _checks.AndCheck([check1, check]) + return [('or_expr', or_expr.add_check(and_expr))] + @reducer('and_expr', 'and', 'check') def _extend_and_expr(self, and_expr, _and, check): """Extend an 'and_expr' by adding one more check.""" @@ -166,6 +178,7 @@ class ParseState(object): return [('and_expr', and_expr.add_check(check))] @reducer('check', 'or', 'check') + @reducer('and_expr', 'or', 'check') def _make_or_expr(self, check1, _or, check2): """Create an 'or_expr'. diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 735b8220..d870db93 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -70,6 +70,22 @@ policy rule:: "project_id:%(project_id)s and not role:dunce" +Operator precedence is below: + + +------------+-------------+-------------+ + | PRECEDENCE | TYPE | EXPRESSION | + +============+=============+=============+ + | 4 | Grouping | (...) | + +------------+-------------+-------------+ + | 3 | Logical NOT | not ... | + +------------+-------------+-------------+ + | 2 | Logical AND | ... and ... | + +------------+-------------+-------------+ + | 1 | Logical OR | ... or ... | + +------------+-------------+-------------+ + +Operator with larger precedence number precedes others with smaller numbers. + In the list-of-lists representation, each check inside the innermost list is combined as with an "and" conjunction -- for that check to pass, all the specified checks must pass. These innermost lists are then diff --git a/oslo_policy/tests/test_checks.py b/oslo_policy/tests/test_checks.py index 6a886de6..e685f622 100644 --- a/oslo_policy/tests/test_checks.py +++ b/oslo_policy/tests/test_checks.py @@ -389,6 +389,13 @@ class OrCheckTestCase(test_base.BaseTestCase): self.assertEqual(['rule1', 'rule2', 'rule3'], check.rules) + def test_pop_check(self): + check = _checks.OrCheck(['rule1', 'rule2', 'rule3']) + rules, check1 = check.pop_check() + + self.assertEqual(['rule1', 'rule2'], check.rules) + self.assertEqual('rule3', check1) + def test_str(self): check = _checks.OrCheck(['rule1', 'rule2']) diff --git a/oslo_policy/tests/test_parser.py b/oslo_policy/tests/test_parser.py index 554278e2..16bb1702 100644 --- a/oslo_policy/tests/test_parser.py +++ b/oslo_policy/tests/test_parser.py @@ -381,6 +381,135 @@ class ParseTextRuleTestCase(test_base.BaseTestCase): self.assertTrue(isinstance(result, _checks.FalseCheck)) mock_parse_tokenize.assert_called_once_with('test rule') + def test_A_or_B_or_C(self): + result = _parser._parse_text_rule('@ or ! or @') + self.assertEqual('(@ or ! or @)', str(result)) + + def test_A_or_B_and_C(self): + result = _parser._parse_text_rule('@ or ! and @') + self.assertEqual('(@ or (! and @))', str(result)) + + def test_A_and_B_or_C(self): + result = _parser._parse_text_rule('@ and ! or @') + self.assertEqual('((@ and !) or @)', str(result)) + + def test_A_and_B_and_C(self): + result = _parser._parse_text_rule('@ and ! and @') + self.assertEqual('(@ and ! and @)', str(result)) + + def test_A_or_B_or_C_or_D(self): + result = _parser._parse_text_rule('@ or ! or @ or !') + self.assertEqual('(@ or ! or @ or !)', str(result)) + + def test_A_or_B_or_C_and_D(self): + result = _parser._parse_text_rule('@ or ! or @ and !') + self.assertEqual('(@ or ! or (@ and !))', str(result)) + + def test_A_or_B_and_C_or_D(self): + result = _parser._parse_text_rule('@ or ! and @ or !') + self.assertEqual('(@ or (! and @) or !)', str(result)) + + def test_A_or_B_and_C_and_D(self): + result = _parser._parse_text_rule('@ or ! and @ and !') + self.assertEqual('(@ or (! and @ and !))', str(result)) + + def test_A_and_B_or_C_or_D(self): + result = _parser._parse_text_rule('@ and ! or @ or !') + self.assertEqual('((@ and !) or @ or !)', str(result)) + + def test_A_and_B_or_C_and_D(self): + result = _parser._parse_text_rule('@ and ! or @ and !') + self.assertEqual('((@ and !) or (@ and !))', str(result)) + + def test_A_and_B_and_C_or_D(self): + result = _parser._parse_text_rule('@ and ! and @ or !') + self.assertEqual('((@ and ! and @) or !)', str(result)) + + def test_A_and_B_and_C_and_D(self): + result = _parser._parse_text_rule('@ and ! and @ and !') + self.assertEqual('(@ and ! and @ and !)', str(result)) + + def test_A_and_B_or_C_with_not_1(self): + result = _parser._parse_text_rule('not @ and ! or @') + self.assertEqual('((not @ and !) or @)', str(result)) + + def test_A_and_B_or_C_with_not_2(self): + result = _parser._parse_text_rule('@ and not ! or @') + self.assertEqual('((@ and not !) or @)', str(result)) + + def test_A_and_B_or_C_with_not_3(self): + result = _parser._parse_text_rule('@ and ! or not @') + self.assertEqual('((@ and !) or not @)', str(result)) + + def test_A_and_B_or_C_with_group_1(self): + for expression in ['( @ ) and ! or @', + '@ and ( ! ) or @', + '@ and ! or ( @ )', + '( @ ) and ! or ( @ )', + '@ and ( ! ) or ( @ )', + '( @ ) and ( ! ) or ( @ )', + '( @ and ! ) or @', + '( ( @ ) and ! ) or @', + '( @ and ( ! ) ) or @', + '( ( @ and ! ) ) or @', + '( @ and ! or @ )']: + result = _parser._parse_text_rule(expression) + self.assertEqual('((@ and !) or @)', str(result)) + + def test_A_and_B_or_C_with_group_2(self): + result = _parser._parse_text_rule('@ and ( ! or @ )') + self.assertEqual('(@ and (! or @))', str(result)) + + def test_A_and_B_or_C_with_group_and_not_1(self): + for expression in ['not ( @ ) and ! or @', + 'not @ and ( ! ) or @', + 'not @ and ! or ( @ )', + '( not @ ) and ! or @', + '( not @ and ! ) or @', + '( not @ and ! or @ )']: + result = _parser._parse_text_rule(expression) + self.assertEqual('((not @ and !) or @)', str(result)) + + def test_A_and_B_or_C_with_group_and_not_2(self): + result = _parser._parse_text_rule('not @ and ( ! or @ )') + self.assertEqual('(not @ and (! or @))', str(result)) + + def test_A_and_B_or_C_with_group_and_not_3(self): + result = _parser._parse_text_rule('not ( @ and ! or @ )') + self.assertEqual('not ((@ and !) or @)', str(result)) + + def test_A_and_B_or_C_with_group_and_not_4(self): + for expression in ['( @ ) and not ! or @', + '@ and ( not ! ) or @', + '@ and not ( ! ) or @', + '@ and not ! or ( @ )', + '( @ and not ! ) or @', + '( @ and not ! or @ )']: + result = _parser._parse_text_rule(expression) + self.assertEqual('((@ and not !) or @)', str(result)) + + def test_A_and_B_or_C_with_group_and_not_5(self): + result = _parser._parse_text_rule('@ and ( not ! or @ )') + self.assertEqual('(@ and (not ! or @))', str(result)) + + def test_A_and_B_or_C_with_group_and_not_6(self): + result = _parser._parse_text_rule('@ and not ( ! or @ )') + self.assertEqual('(@ and not (! or @))', str(result)) + + def test_A_and_B_or_C_with_group_and_not_7(self): + for expression in ['( @ ) and ! or not @', + '@ and ( ! ) or not @', + '@ and ! or not ( @ )', + '@ and ! or ( not @ )', + '( @ and ! ) or not @', + '( @ and ! or not @ )']: + result = _parser._parse_text_rule(expression) + self.assertEqual('((@ and !) or not @)', str(result)) + + def test_A_and_B_or_C_with_group_and_not_8(self): + result = _parser._parse_text_rule('@ and ( ! or not @ )') + self.assertEqual('(@ and (! or not @))', str(result)) + class ParseRuleTestCase(test_base.BaseTestCase): @mock.patch.object(_parser, '_parse_text_rule', return_value='text rule')