[FIX] osv.expression: normalize domains in combine

Before this rev:

* Take 2 domains that contain at least two leaves, and at least one of
them must use the implicit `&` operator

e.g.:
    d1 = [('so_line', 'in', [91]), ('amount', '<=', 0.0)]
    d2 = ['&', ('so_line', 'in', []), ('project_id', '!=', False)]

* Perform osv.expression.OR() between both domains

Expected result:
    d3 = ['|', d1, d2]

Actual result (after normalization):
    d3 = ['&', '|', d1, d2]

This is because, since the `&` is implicit for the first domain, when we
OR it, we give it an explicit `|` operator, so when we pass this domain
through the normalize_domain function, d1 no longer contains an implicit
`&` operator but instead the implicit operator is the one between d1 and
d2, therefore giving us a completely wrong domain.

The `combine` function states that it only accepts normalized domains,
however neither the OR nor AND functions do, this leads to a lot of
developers putting non-normalized domains into these functions, and
there's no error checking or anything that obviously indicates that the
domain is incorrect, so we might as well normalize all domains being
passed since it's already pretty optimized.
This commit is contained in:
Adrian Torres
2018-04-06 07:45:25 +02:00
committed by GitHub
parent 5f06926093
commit 2875b94e03
2 changed files with 15 additions and 3 deletions
+11
View File
@@ -611,6 +611,17 @@ class TestExpression(TransactionCase):
not_be = Partner.with_context(lang='fr_FR').search([('country_id', '!=', 'Belgique')])
self.assertNotIn(agrolait, not_be)
def test_or_with_implicit_and(self):
# Check that when using expression.OR on a list of domains with at least one
# implicit '&' the returned domain is the expected result.
# from #24038
d1 = [('foo', '=', 1), ('bar', '=', 1)]
d2 = ['&', ('foo', '=', 2), ('bar', '=', 2)]
expected = ['|', '&', ('foo', '=', 1), ('bar', '=', 1),
'&', ('foo', '=', 2), ('bar', '=', 2)]
self.assertEqual(expression.OR([d1, d2]), expected)
class TestAutoJoin(TransactionCase):
+4 -3
View File
@@ -232,8 +232,9 @@ def is_false(model, domain):
def combine(operator, unit, zero, domains):
"""Returns a new domain expression where all domain components from ``domains``
have been added together using the binary operator ``operator``. The given
domains must be normalized.
have been added together using the binary operator ``operator``.
It is guaranteed to return a normalized domain.
:param unit: the identity element of the domains "set" with regard to the operation
performed by ``operator``, i.e the domain component ``i`` which, when
@@ -255,7 +256,7 @@ def combine(operator, unit, zero, domains):
if domain == zero:
return zero
if domain:
result += domain
result += normalize_domain(domain)
count += 1
result = [operator] * (count - 1) + result
return result