`and`ed expressions evaluate different objects unless parenthesized

[Saviq]

Bug Description

According to the documentation:

It’s not possible to refer to two different resources, from the same resource group, in one resource expression.

Consider this test:

    def test_evaluate_same_object(self):
        resource_map = {
            'a': [Resource({'foo': 1, 'baz': 'b'}), Resource({'foo': 2, 'baz': 'a'})],
        }
        expr = ResourceExpression("a.foo == 1 and a.baz != 'b'")
        self.assertFalse(expr.evaluate(
            resource_map['a'],
            resource_map=resource_map
        ))

And it failing:

FAIL: test_evaluate_same_object (plainbox.impl.test_resource.ResourceExpressionTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/michal/dev/canonical/checkbox/checkbox-ng/plainbox/impl/test_resource.py", line 426, in test_evaluate_same_object
    self.assertFalse(expr.evaluate(
AssertionError: True is not false

There is not a single resource whose foo is 1, and baz is not b. But the expression succeeds. Wrapping the first expression in parentheses makes it work as intended.

To Reproduce

Run the above test.

Environment

N/A

Relevant log output

No response

Additional context

No response

[Saviq]

The problematic piece of code seems to be:

checkbox/checkbox-ng/plainbox/impl/resource.py

Lines 639 to 649 in 8d1089f

	if not '(' in self._text:
	or_pos = self._text.rfind(' or ')
	if or_pos > 0:
	lhs, rhs = self._split_and_evaluate(' or ', resource_map)
	return lhs or rhs
	and_pos = self._text.rfind(' and ')
	if and_pos > 0:
	lhs, rhs = self._split_and_evaluate(' and ', resource_map)
	return lhs and rhs
	# there are no conjuctions, so let's do a simple evaluation

If there are no parens in the expression, it's evaluating "simply", not splitting by and and or.

From a skim, the resource product should be made before ever recursing, since you only ever want to work on a single object per resource.

[kissiel]

Without those parens the evaluation can take a few days per job to complete on some devices.

[Saviq]

Without those parens the evaluation can take a few days per job to complete on some devices.

Don't get it. Just make the code work as if there are parens around the first expression, always?

I mean, this should be equivalent, no?

foo.bar == 1 and bar.baz == 2
(foo.bar == 1) and bar.baz == 2

[kissiel]

So parenthesis allow us to divide and conquer. Without them checkbox creates a HUMONGOUS matrix with all the combinations of resources so for instance there can be relationship like yours where multiple fields have to be evaluate with values from the same record.
Natural solution -> let's imply parens around X and Y, but what about X and Y and Z? :D
So this is becoming an AST story with some smart code that would feed the values and "solve" the python expression.

[Saviq]

OK I get what you mean, -ish. But let's not focus on the parens.

The expression shouldn't pass regardless of parens :)

[Hook25]

We have decided to clean up our issue tracker to systematize our approach to backlog grooming and planning. If this issue is still relevant to you, please reopen it and tag one of the maintainers or reach out on ~Checkbox on Mattermost. Going forward all issues that have received no activity for more than a year will be marked as 'stale' and closed after one week

[syncronize-issues-to-jira]

Thank you for reporting your feedback to us!

The internal ticket has been created: https://warthogs.atlassian.net/browse/CHECKBOX-2123.

This message was autogenerated