Deduplicate entries in domain inversion's simplify

issue11248
review397301002

CHANGELOG

@@ -1,3 +1,4 @@
+* Deduplicate entries in domain inversion's simplify
 * Prevent to create and delete singleton
 * Allow CORS on root path
 * Allow button access to be deactivated

trytond/tests/test_tools.py

@@ -533,7 +533,7 @@ class DomainInversionTestCase(unittest.TestCase):
             domain_inversion(domain, 'x', {'y': 7, 'z': 'abc'}), True)
         self.assertEqual(
             domain_inversion(domain, 'x', {'y': 4, 'z': 'b'}),
-            ['OR', [['x', '=', 3]], [['x', '=', 2]]])
+            ['OR', ['x', '=', 3], ['x', '=', 2]])
 
     def test_parse(self):
         domain = parse([['x', '=', 5]])
@@ -555,16 +555,34 @@ class DomainInversionTestCase(unittest.TestCase):
         domain = [[['x', '=', 3]]]
         self.assertEqual(simplify(domain), [['x', '=', 3]])
 
+        domain = [[['x', '=', 3], ['y', '=', 4]]]
+        self.assertEqual(simplify(domain), [['x', '=', 3], ['y', '=', 4]])
+
         domain = ['OR', ['x', '=', 3]]
         self.assertEqual(simplify(domain), [['x', '=', 3]])
 
         domain = ['OR', [['x', '=', 3]], [['y', '=', 5]]]
         self.assertEqual(
-            simplify(domain), ['OR', [['x', '=', 3]], [['y', '=', 5]]])
+            simplify(domain), ['OR', ['x', '=', 3], ['y', '=', 5]])
 
         domain = ['OR', ['x', '=', 3], ['AND', ['y', '=', 5]]]
         self.assertEqual(
-            simplify(domain), ['OR', ['x', '=', 3], [['y', '=', 5]]])
+            simplify(domain), ['OR', ['x', '=', 3], ['y', '=', 5]])
+
+        domain = [['x', '=', 3], ['OR']]
+        self.assertEqual(simplify(domain), [['x', '=', 3]])
+
+        domain = ['OR', ['x', '=', 3], []]
+        self.assertEqual(simplify(domain), [])
+
+        domain = ['OR', ['x', '=', 3], ['OR']]
+        self.assertEqual(simplify(domain), [])
+
+        domain = [['x', '=', 3], []]
+        self.assertEqual(simplify(domain), [['x', '=', 3]])
+
+        domain = [['x', '=', 3], ['AND']]
+        self.assertEqual(simplify(domain), [['x', '=', 3]])
 
         domain = ['AND']
         self.assertEqual(simplify(domain), [])
@@ -572,6 +590,35 @@ class DomainInversionTestCase(unittest.TestCase):
         domain = ['OR']
         self.assertEqual(simplify(domain), [])
 
+    def test_simplify_deduplicate(self):
+        "Test deduplicate"
+        clause = ('x', '=', 'x')
+        another = ('y', '=', 'y')
+        third = ('z', '=', 'z')
+        tests = [
+            ([], []),
+            (['OR', []], []),
+            (['AND', []], []),
+            ([clause], [clause]),
+            (['OR', clause], [clause]),
+            ([clause, clause], [clause]),
+            (['OR', clause, clause], [clause]),
+            ([clause, [clause, clause]], [clause]),
+            ([clause, another], [clause, another]),
+            (['OR', clause, another], ['OR', clause, another]),
+            ([clause, clause, another], [clause, another]),
+            ([clause, [clause, clause], another], [clause, another]),
+            ([clause, clause, another, another], [clause, another]),
+            ([clause, another, clause, another], [clause, another]),
+            (
+                ['AND', ['OR', clause, another], third],
+                ['AND', ['OR', clause, another], third]),
+            ]
+
+        for input, expected in tests:
+            with self.subTest(input=input):
+                self.assertEqual(simplify(input), expected)
+
     def test_merge(self):
         domain = [['x', '=', 6], ['y', '=', 7]]
         self.assertEqual(merge(domain), ['AND', ['x', '=', 6], ['y', '=', 7]])
@@ -621,7 +668,7 @@ class DomainInversionTestCase(unittest.TestCase):
         self.assertEqual(concat([], []), [])
         self.assertEqual(
             concat(domain1, domain2, domoperator='OR'),
-            ['OR', [['a', '=', 1]], [['b', '=', 2]]])
+            ['OR', ['a', '=', 1], ['b', '=', 2]])
 
     def test_unique_value(self):
         domain = [['a', '=', 1]]

trytond/tools/domain_inversion.py

@@ -284,22 +284,65 @@ def localize_domain(domain, field_name=None, strip_target=False):
             for part in domain]
 
 
-def simplify(domain):
-    "remove unused domain delimiter"
-    if is_leaf(domain):
-        return domain
-    elif domain in ('OR', 'AND'):
-        return domain
-    elif domain in (['OR'], ['AND']):
+def bool_operator(domain):
+    "Returns the boolean operator used by a domain"
+    bool_op = 'AND'
+    if domain and domain[0] in ['AND', 'OR']:
+        bool_op = domain[0]
+    return bool_op
+
+
+def simplify_nested(domain):
+    """Simplify extra domain markers"""
+    if not domain:
         return []
-    elif (isinstance(domain, list) and len(domain) == 1
-            and not is_leaf(domain[0])):
-        return simplify(domain[0])
-    elif (isinstance(domain, list) and len(domain) == 2
-            and domain[0] in ('AND', 'OR')):
-        return [simplify(domain[1])]
+    elif is_leaf(domain):
+        return [domain]
+    elif domain in ['OR', 'AND']:
+        return [domain]
+    elif isinstance(domain, list) and len(domain) == 1:
+        return simplify_nested(domain[0])
     else:
-        return [simplify(branch) for branch in domain]
+        simplified = []
+        domain_op = bool_operator(domain)
+        for branch in domain:
+            simplified_branch = simplify_nested(branch)
+            if (bool_operator(branch) == domain_op
+                    or len(simplified_branch) == 1):
+                simplified.extend(simplified_branch)
+            else:
+                simplified.append(simplified_branch)
+        return simplified
+
+
+def simplify_duplicate(domain):
+    """Remove duplicates subdomain from domain"""
+    dedup_branches = []
+    bool_op = None
+    if domain[0] in ['AND', 'OR']:
+        bool_op, *domain = domain
+    for branch in domain:
+        simplified_branch = simplify(branch)
+        if not simplified_branch:
+            if bool_op == 'OR':
+                return []
+            else:
+                continue
+        elif simplified_branch not in dedup_branches:
+            dedup_branches.append(simplified_branch)
+    if bool_op and len(dedup_branches) > 1:
+        dedup_branches.insert(0, bool_op)
+    return dedup_branches
+
+
+def simplify(domain):
+    """Remove duplicate expressions and useless OR/AND"""
+    if is_leaf(domain):
+        return [domain]
+    elif not domain:
+        return domain
+    else:
+        return simplify_nested(simplify_duplicate(domain))
 
 
 def merge(domain, domoperator=None):