From d867698c0057bfe07c67c29fa71a75e03a2580fd Mon Sep 17 00:00:00 2001
From: Lukas Holecek <hluk@email.cz>
Date: Nov 13 2018 11:38:31 +0000
Subject: Refactor getting matching policies and rules


Adds Policy.matches() and Rule.matches() so as to simplify getting
applicable policies for a changed test case result.

Signed-off-by: Lukas Holecek <hluk@email.cz>

---

diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py
index 19e5c7f..4aaae81 100644
--- a/greenwave/api_v1.py
+++ b/greenwave/api_v1.py
@@ -328,7 +328,12 @@ def make_decision():
     for subject_type, subject_identifier in _decision_subjects_for_request(data):
         subject_policies = [
             policy for policy in current_app.config['policies']
-            if policy.applies_to(decision_context, product_version, subject_type)]
+            if policy.matches(
+                decision_context=decision_context,
+                product_version=product_version,
+                subject_type=subject_type)
+        ]
+
         if not subject_policies:
             # Ignore non-existent policy for Bodhi updates.
             if subject_type == 'bodhi_update':
diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py
index a70f78a..ef4388f 100644
--- a/greenwave/consumers/resultsdb.py
+++ b/greenwave/consumers/resultsdb.py
@@ -21,8 +21,7 @@ import greenwave.app_factory
 import greenwave.resources
 from greenwave.api_v1 import subject_type_identifier_to_list
 from greenwave.monitoring import publish_decision_exceptions_result_counter
-from greenwave.policies import RemoteRule
-from greenwave.safe_yaml import SafeYAMLError
+from greenwave.policies import applicable_decision_context_product_version_pairs
 
 import xmlrpc.client
 
@@ -211,94 +210,50 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer):
             result_id (int): A result ID to ignore for comparison.
             testcase (munch.Munch): The name of a testcase to consider.
         """
-        # Also need to apply policies for each build in the update.
-        if subject_type == 'bodhi_update':
-            subject_types = set([subject_type, 'koji_build'])
-        else:
-            subject_types = set([subject_type])
-
-        # Build a set of all policies which might apply to this new results
-        applicable_policies = set()
-        remote_policies = set()
-        for policy in current_app.config['policies']:
-            for rule in policy.rules:
-                if isinstance(rule, RemoteRule):
-                    try:
-                        for remote_policy in rule.get_policies(policy, subject_identifier):
-                            remote_policy.product_versions = set(
-                                remote_policy.product_versions).intersection(set(
-                                    policy.product_versions))
-                            if (remote_policy.product_versions and
-                                    remote_policy.decision_context == policy.decision_context):
-                                remote_policies = remote_policies.union(set([remote_policy]))
-                    except SafeYAMLError as e:
-                        pass
-
-        tmp_policies = remote_policies.union(current_app.config['policies'])
-        for policy in tmp_policies:
-            if policy.subject_type in subject_types:
-                testcases = (
-                    getattr(rule, 'test_case_name', None)
-                    for rule in policy.rules)
-
-                if testcase in testcases:
-                    applicable_policies.add(policy)
-
-        log.debug("messaging: found %i applicable policies of %i for testcase %r",
-                  len(applicable_policies), len(current_app.config['policies']), testcase)
-
-        # Given all of our applicable policies, build a map of all decision
-        # context we know about, and which product versions they relate to.
-        decision_contexts = collections.defaultdict(set)
         product_version = _subject_product_version(subject_identifier, subject_type)
-        for policy in applicable_policies:
-            if not product_version or policy.applies_to_product_version(product_version):
-                if product_version:
-                    versions = set([product_version])
-                else:
-                    versions = set(policy.product_versions)
-                decision_contexts[policy.decision_context].update(versions)
-        log.debug("messaging: found %i decision contexts", len(decision_contexts))
-
-        # For every context X version combination, ask greenwave if this new
-        # result pushes any decisions over a threshold.
-        for decision_context in sorted(decision_contexts.keys()):
-            product_versions = decision_contexts[decision_context]
-            for product_version in sorted(product_versions):
-                greenwave_url = self.fedmsg_config['greenwave_api_url'] + '/decision'
-
-                data = {
-                    'decision_context': decision_context,
-                    'product_version': product_version,
+        policies = self.flask_app.config['policies']
+        contexts_product_versions = applicable_decision_context_product_version_pairs(
+            policies,
+            subject_type=subject_type,
+            subject_identifier=subject_identifier,
+            testcase=testcase,
+            product_version=product_version)
+
+        for decision_context, product_version in sorted(contexts_product_versions):
+            greenwave_url = self.fedmsg_config['greenwave_api_url'] + '/decision'
+
+            data = {
+                'decision_context': decision_context,
+                'product_version': product_version,
+                'subject_type': subject_type,
+                'subject_identifier': subject_identifier,
+            }
+
+            try:
+                decision = greenwave.resources.retrieve_decision(greenwave_url, data)
+
+                # get old decision
+                data.update({
+                    'ignore_result': [result_id],
+                })
+                old_decision = greenwave.resources.retrieve_decision(greenwave_url, data)
+            except requests.exceptions.HTTPError as e:
+                log.exception('Failed to retrieve decision for data=%s, error: %s', data, e)
+                continue
+
+            if decision == old_decision:
+                log.debug('Skipped emitting fedmsg, decision did not change: %s', decision)
+            else:
+                decision.update({
                     'subject_type': subject_type,
                     'subject_identifier': subject_identifier,
-                }
-
-                try:
-                    decision = greenwave.resources.retrieve_decision(greenwave_url, data)
-
-                    # get old decision
-                    data.update({
-                        'ignore_result': [result_id],
-                    })
-                    old_decision = greenwave.resources.retrieve_decision(greenwave_url, data)
-                except requests.exceptions.HTTPError as e:
-                    log.exception('Failed to retrieve decision for data=%s, error: %s', data, e)
-                    continue
-
-                if decision == old_decision:
-                    log.debug('Skipped emitting fedmsg, decision did not change: %s', decision)
-                else:
-                    decision.update({
-                        'subject_type': subject_type,
-                        'subject_identifier': subject_identifier,
-                        # subject is for backwards compatibility only:
-                        'subject': subject_type_identifier_to_list(subject_type,
-                                                                   subject_identifier),
-                        'decision_context': decision_context,
-                        'product_version': product_version,
-                        'previous': old_decision,
-                    })
-                    log.debug('Emitted a fedmsg, %r, on the "%s" topic', decision,
-                              'greenwave.decision.update')
-                    fedmsg.publish(topic='decision.update', msg=decision)
+                    # subject is for backwards compatibility only:
+                    'subject': subject_type_identifier_to_list(subject_type,
+                                                               subject_identifier),
+                    'decision_context': decision_context,
+                    'product_version': product_version,
+                    'previous': old_decision,
+                })
+                log.debug('Emitted a fedmsg, %r, on the "%s" topic', decision,
+                          'greenwave.decision.update')
+                fedmsg.publish(topic='decision.update', msg=decision)
diff --git a/greenwave/consumers/waiverdb.py b/greenwave/consumers/waiverdb.py
index a7180d5..808cf64 100644
--- a/greenwave/consumers/waiverdb.py
+++ b/greenwave/consumers/waiverdb.py
@@ -18,6 +18,7 @@ import requests
 import greenwave.app_factory
 from greenwave.api_v1 import subject_type_identifier_to_list
 from greenwave.monitoring import publish_decision_exceptions_waiver_counter
+from greenwave.policies import applicable_decision_context_product_version_pairs
 
 requests_session = requests.Session()
 
@@ -78,54 +79,60 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer):
     @publish_decision_exceptions_waiver_counter.count_exceptions()
     def _publish_decision_changes(self, subject_type, subject_identifier, waiver_id,
                                   product_version, testcase):
-        for policy in self.flask_app.config['policies']:
-            for rule in policy.rules:
-                if getattr(rule, 'test_case_name', None) == testcase:
-                    data = {
-                        'decision_context': policy.decision_context,
-                        'product_version': product_version,
-                        'subject_type': subject_type,
-                        'subject_identifier': subject_identifier,
-                    }
-                    response = requests_session.post(
-                        self.fedmsg_config['greenwave_api_url'] + '/decision',
-                        headers={'Content-Type': 'application/json'},
-                        data=json.dumps(data))
-
-                    if not response.ok:
-                        log.error(response.text)
-                        continue
-
-                    decision = response.json()
-
-                    # get old decision
-                    data.update({
-                        'ignore_waiver': [waiver_id],
-                    })
-                    response = requests_session.post(
-                        self.fedmsg_config['greenwave_api_url'] + '/decision',
-                        headers={'Content-Type': 'application/json'},
-                        data=json.dumps(data))
-
-                    if not response.ok:
-                        log.error(response.text)
-                        continue
-
-                    old_decision = response.json()
-
-                    if decision != old_decision:
-                        msg = decision
-                        decision.update({
-                            'subject_type': subject_type,
-                            'subject_identifier': subject_identifier,
-                            # subject is for backwards compatibility only:
-                            'subject': subject_type_identifier_to_list(subject_type,
-                                                                       subject_identifier),
-                            'testcase': testcase,
-                            'decision_context': policy.decision_context,
-                            'product_version': product_version,
-                            'previous': old_decision,
-                        })
-                        log.debug('Emitted a fedmsg, %r, on the "%s" topic', msg,
-                                  'greenwave.decision.update')
-                        fedmsg.publish(topic='decision.update', msg=msg)
+        policies = self.flask_app.config['policies']
+        contexts_product_versions = applicable_decision_context_product_version_pairs(
+            policies,
+            subject_type=subject_type,
+            subject_identifier=subject_identifier,
+            testcase=testcase,
+            product_version=product_version)
+
+        for decision_context, product_version in sorted(contexts_product_versions):
+            data = {
+                'decision_context': decision_context,
+                'product_version': product_version,
+                'subject_type': subject_type,
+                'subject_identifier': subject_identifier,
+            }
+            response = requests_session.post(
+                self.fedmsg_config['greenwave_api_url'] + '/decision',
+                headers={'Content-Type': 'application/json'},
+                data=json.dumps(data))
+
+            if not response.ok:
+                log.error(response.text)
+                continue
+
+            decision = response.json()
+
+            # get old decision
+            data.update({
+                'ignore_waiver': [waiver_id],
+            })
+            response = requests_session.post(
+                self.fedmsg_config['greenwave_api_url'] + '/decision',
+                headers={'Content-Type': 'application/json'},
+                data=json.dumps(data))
+
+            if not response.ok:
+                log.error(response.text)
+                continue
+
+            old_decision = response.json()
+
+            if decision != old_decision:
+                msg = decision
+                decision.update({
+                    'subject_type': subject_type,
+                    'subject_identifier': subject_identifier,
+                    # subject is for backwards compatibility only:
+                    'subject': subject_type_identifier_to_list(subject_type,
+                                                               subject_identifier),
+                    'testcase': testcase,
+                    'decision_context': decision_context,
+                    'product_version': product_version,
+                    'previous': old_decision,
+                })
+                log.debug('Emitted a fedmsg, %r, on the "%s" topic', msg,
+                          'greenwave.decision.update')
+                fedmsg.publish(topic='decision.update', msg=msg)
diff --git a/greenwave/policies.py b/greenwave/policies.py
index c67db6f..cde1dec 100644
--- a/greenwave/policies.py
+++ b/greenwave/policies.py
@@ -284,6 +284,18 @@ class Rule(SafeYAMLObject):
         """
         raise NotImplementedError()
 
+    def matches(self, policy, **attributes):
+        """
+        Same as Policy.matches() for a rule attributes.
+
+        Args:
+            policy (Policy): Parent policy of the rule
+
+        Returns:
+            bool: True only if provided attributes matches the rule
+        """
+        return True
+
 
 def waives_invalid_gating_yaml(waiver, subject_type, subject_identifier):
     return (waiver['testcase'] == 'invalid-gating-yaml' and
@@ -295,7 +307,7 @@ class RemoteRule(Rule):
     yaml_tag = '!RemoteRule'
     safe_yaml_attributes = {}
 
-    def get_policies(self, policy, subject_identifier):
+    def _get_sub_policies(self, policy, subject_identifier):
         if policy.subject_type != 'koji_build':
             return []
 
@@ -313,12 +325,15 @@ class RemoteRule(Rule):
             return []
 
         policies = RemotePolicy.safe_load_all(response)
-        return policies
+        return [
+            sub_policy for sub_policy in policies
+            if sub_policy.decision_context == policy.decision_context
+        ]
 
     def check(self, policy, product_version, subject_identifier, results_retriever, waivers):
 
         try:
-            policies = self.get_policies(policy, subject_identifier)
+            policies = self._get_sub_policies(policy, subject_identifier)
         except SafeYAMLError as e:
             if any(waives_invalid_gating_yaml(waiver, policy.subject_type, subject_identifier)
                     for waiver in waivers):
@@ -330,8 +345,7 @@ class RemoteRule(Rule):
 
         answers = []
         for remote_policy in policies:
-            if remote_policy.applies_to(
-                    policy.decision_context, product_version, policy.subject_type):
+            if remote_policy.matches_product_version(product_version):
                 response = remote_policy.check(
                     product_version, subject_identifier, results_retriever, waivers)
 
@@ -342,6 +356,23 @@ class RemoteRule(Rule):
 
         return answers
 
+    def matches(self, policy, **attributes):
+        subject_identifier = attributes.get('subject_identifier')
+        if not subject_identifier:
+            return True
+
+        sub_policies = []
+        try:
+            sub_policies = self._get_sub_policies(policy, subject_identifier)
+        except SafeYAMLError:
+            logging.exception(
+                'Failed to parse policies for %r', subject_identifier)
+        except Exception:
+            logging.exception(
+                'Failed to retrieve policies for %r', subject_identifier)
+
+        return any(sub_policy.matches(**attributes) for sub_policy in sub_policies)
+
     def to_json(self):
         return {
             'rule': self.__class__.__name__,
@@ -409,6 +440,10 @@ class PassingTestCaseRule(Rule):
         return self._answer_for_result(
             latest_result, waivers, policy.subject_type, subject_identifier)
 
+    def matches(self, policy, **attributes):
+        testcase = attributes.get('testcase')
+        return not testcase or testcase == self.test_case_name
+
     def to_json(self):
         return {
             'rule': self.__class__.__name__,
@@ -488,6 +523,10 @@ class PackageSpecificRule(Rule):
         rule.test_case_name = self.test_case_name
         return rule.check(policy, product_version, subject_identifier, results_retriever, waivers)
 
+    def matches(self, policy, **attributes):
+        testcase = attributes.get('testcase')
+        return not testcase or testcase == self.test_case_name
+
     def to_json(self):
         return {
             'rule': self.__class__.__name__,
@@ -520,10 +559,29 @@ class Policy(SafeYAMLObject):
         'relevance_value': SafeYAMLString(optional=True),
     }
 
-    def applies_to(self, decision_context, product_version, subject_type):
-        return (decision_context == self.decision_context and
-                self.applies_to_product_version(product_version) and
-                subject_type == self.subject_type)
+    def matches(self, **attributes):
+        """
+        Returns True only if policy matches provided attributes.
+
+        If an attribute to match is missing it's treated as irrelevant, i.e."match anything".
+
+        Unknown attributes are ignored.
+
+        There must be at least one matching rule or no rules in the policy.
+        """
+        decision_context = attributes.get('decision_context')
+        if decision_context and decision_context != self.decision_context:
+            return False
+
+        product_version = attributes.get('product_version')
+        if product_version and not self.matches_product_version(product_version):
+            return False
+
+        subject_type = attributes.get('subject_type')
+        if subject_type and subject_type != self.subject_type:
+            return False
+
+        return not self.rules or any(rule.matches(self, **attributes) for rule in self.rules)
 
     def check(self, product_version, subject_identifier, results_retriever, waivers):
         # If an item is about a package and it is in the blacklist, return RuleSatisfied()
@@ -541,7 +599,7 @@ class Policy(SafeYAMLObject):
                 answers.append(response)
         return answers
 
-    def applies_to_product_version(self, product_version):
+    def matches_product_version(self, product_version):
         return any(fnmatch(product_version, version) for version in self.product_versions)
 
     @property
@@ -567,3 +625,31 @@ class RemotePolicy(Policy):
             if isinstance(rule, RemoteRule):
                 raise SafeYAMLError('RemoteRule is not allowed in remote policies')
         super().validate()
+
+
+def _applicable_decision_context_product_version_pairs(policies, **attributes):
+    applicable_policies = [
+        policy for policy in policies if policy.matches(**attributes)
+    ]
+
+    log.debug("found %i applicable policies of %i for: %r",
+              len(applicable_policies), len(policies), attributes)
+
+    product_version = attributes.get('product_version')
+    if product_version:
+        for policy in applicable_policies:
+            yield policy.decision_context, product_version
+    else:
+        for policy in applicable_policies:
+            # FIXME: This can returns product version patterns like 'fedora-*'.
+            for product_version in policy.product_versions:
+                yield policy.decision_context, product_version
+
+
+def applicable_decision_context_product_version_pairs(policies, **attributes):
+    contexts_product_versions = sorted(set(
+        _applicable_decision_context_product_version_pairs(
+            policies, **attributes)))
+
+    log.debug("found %i decision contexts", len(contexts_product_versions))
+    return contexts_product_versions
diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py
index 794ac05..335406e 100644
--- a/greenwave/tests/test_policies.py
+++ b/greenwave/tests/test_policies.py
@@ -316,7 +316,12 @@ rules:
     load_policies(tmpdir.strpath)
 
 
-def test_product_versions_pattern(tmpdir):
+@pytest.mark.parametrize(('product_version', 'applies'), [
+    ('fedora-27', True),
+    ('fedora-28', True),
+    ('epel-7', False),
+])
+def test_product_versions_pattern(product_version, applies, tmpdir):
     p = tmpdir.join('fedora.yaml')
     p.write("""
 --- !Policy
@@ -325,14 +330,16 @@ product_versions:
   - fedora-*
 decision_context: dummy_context
 subject_type: bodhi_update
-rules: []
+rules:
+  - !PassingTestCaseRule {test_case_name: test}
         """)
     policies = load_policies(tmpdir.strpath)
     policy = policies[0]
 
-    assert policy.applies_to('dummy_context', 'fedora-27', 'bodhi_update')
-    assert policy.applies_to('dummy_context', 'fedora-28', 'bodhi_update')
-    assert not policy.applies_to('dummy_context', 'epel-7', 'bodhi_update')
+    assert applies == policy.matches(
+        decision_context='dummy_context',
+        product_version=product_version,
+        subject_type='bodhi_update')
 
 
 def test_remote_rule_policy(tmpdir):
diff --git a/greenwave/tests/test_rules.py b/greenwave/tests/test_rules.py
new file mode 100644
index 0000000..afab240
--- /dev/null
+++ b/greenwave/tests/test_rules.py
@@ -0,0 +1,98 @@
+import mock
+
+from textwrap import dedent
+
+from greenwave.app_factory import create_app
+from greenwave.policies import Policy
+from greenwave.safe_yaml import SafeYAMLError
+
+
+def test_match_passing_test_case_rule():
+    policy_yaml = dedent("""
+        --- !Policy
+        id: "some_policy"
+        product_versions: [rhel-9000]
+        decision_context: bodhi_update_push_stable
+        subject_type: koji_build
+        rules:
+          - !PassingTestCaseRule {test_case_name: some_test_case}
+    """)
+    policies = Policy.safe_load_all(policy_yaml)
+    assert len(policies) == 1
+
+    policy = policies[0]
+    assert len(policy.rules) == 1
+
+    rule = policy.rules[0]
+    assert rule.matches(policy)
+    assert rule.matches(policy, testcase='some_test_case')
+    assert not rule.matches(policy, testcase='other_test_case')
+
+
+def test_match_package_specific_rule():
+    policy_yaml = dedent("""
+        --- !Policy
+        id: "some_policy"
+        product_versions: [rhel-9000]
+        decision_context: bodhi_update_push_stable
+        subject_type: koji_build
+        rules:
+          - !PackageSpecificBuild {test_case_name: some_test_case, repos: [nethack]}
+    """)
+    policies = Policy.safe_load_all(policy_yaml)
+    assert len(policies) == 1
+
+    policy = policies[0]
+    assert len(policy.rules) == 1
+
+    rule = policy.rules[0]
+    assert rule.matches(policy)
+    assert rule.matches(policy, testcase='some_test_case')
+    assert not rule.matches(policy, testcase='other_test_case')
+
+
+@mock.patch('greenwave.resources.retrieve_yaml_remote_rule')
+@mock.patch('greenwave.resources.retrieve_scm_from_koji')
+def test_match_remote_rule(mock_retrieve_scm_from_koji, mock_retrieve_yaml_remote_rule):
+    policy_yaml = dedent("""
+        --- !Policy
+        id: "some_policy"
+        product_versions: [rhel-9000]
+        decision_context: bodhi_update_push_stable
+        subject_type: koji_build
+        rules:
+          - !RemoteRule {}
+    """)
+    mock_retrieve_yaml_remote_rule.return_value = dedent("""
+        --- !Policy
+        product_versions: [rhel-*]
+        decision_context: bodhi_update_push_stable
+        rules:
+          - !PassingTestCaseRule {test_case_name: some_test_case}
+    """)
+    nvr = 'nethack-1.2.3-1.el9000'
+    mock_retrieve_scm_from_koji.return_value = ('rpms', nvr, '123')
+
+    app = create_app('greenwave.config.TestingConfig')
+    with app.app_context():
+        policies = Policy.safe_load_all(policy_yaml)
+        assert len(policies) == 1
+
+        policy = policies[0]
+        assert len(policy.rules) == 1
+
+        rule = policy.rules[0]
+        assert rule.matches(policy)
+        assert rule.matches(policy, subject_identifier=nvr)
+        assert rule.matches(policy, subject_identifier=nvr, testcase='some_test_case')
+        assert not rule.matches(policy, subject_identifier=nvr, testcase='other_test_case')
+
+        # Simulate invalid gating.yaml file.
+        def raiseYamlError(*args):
+            raise SafeYAMLError()
+        mock_retrieve_yaml_remote_rule.side_effect = raiseYamlError
+
+        assert rule.matches(policy)
+        assert not rule.matches(policy, subject_identifier=nvr)
+        assert not rule.matches(policy, subject_identifier=nvr, testcase='some_test_case')
+        assert not rule.matches(policy, subject_identifier=nvr, testcase='other_test_case')