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

@@ -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

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'.
 

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

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'])
 

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')