From 6e9266c18f8bcee586be871a6f7017f621fe6b54 Mon Sep 17 00:00:00 2001
From: Giulia Naponiello <gnaponie@redhat.com>
Date: Aug 28 2019 07:51:49 +0000
Subject: Merge #481 `Use brew_task_id in ResultsDB messages to guess product version`


---

diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py
index 1b0375d..8f622b1 100644
--- a/greenwave/consumers/resultsdb.py
+++ b/greenwave/consumers/resultsdb.py
@@ -10,69 +10,35 @@ to the message bus about the newly satisfied/unsatisfied policy.
 """
 
 import logging
-import re
 
 from greenwave.consumers.consumer import Consumer
+from greenwave.product_versions import subject_product_version
 
 import xmlrpc.client
 
 log = logging.getLogger(__name__)
 
 
-def _guess_product_version(toparse, koji_build=False):
-    if toparse == 'rawhide' or toparse.startswith('Fedora-Rawhide'):
-        return 'fedora-rawhide'
-
-    product_version = None
-    if toparse.startswith('f') and koji_build:
-        product_version = 'fedora-'
-    elif toparse.startswith('epel'):
-        product_version = 'epel-'
-    elif toparse.startswith('el') and len(toparse) > 2 and toparse[2].isdigit():
-        product_version = 'rhel-'
-    elif toparse.startswith('fc') or toparse.startswith('Fedora'):
-        product_version = 'fedora-'
-
-    if product_version:
-        # seperate the prefix from the number
-        result = list(filter(None, '-'.join(re.split(r'(\d+)', toparse)).split('-')))
-        if len(result) >= 2:
-            try:
-                int(result[1])
-                product_version += result[1]
-                return product_version
-            except ValueError:
-                pass
-
-    log.error("It wasn't possible to guess the product version")
-    return None
-
-
-def _subject_product_version(subject_identifier, subject_type, koji_proxy=None):
-    if subject_type == 'koji_build':
-        try:
-            _, _, release = subject_identifier.rsplit('-', 2)
-            _, short_prod_version = release.rsplit('.', 1)
-            return _guess_product_version(short_prod_version, koji_build=True)
-        except (KeyError, ValueError):
-            pass
+def _unpack_value(value):
+    """
+    If value is list with single element, returns the element, otherwise
+    returns the value.
+    """
+    if isinstance(value, list) and len(value) == 1:
+        value = value[0]
+    return value
 
-    if subject_type == "compose":
-        return _guess_product_version(subject_identifier)
 
-    if subject_type == "redhat-module":
-        return "rhel-8"
+def _get_brew_task_id(msg):
+    data = msg.get('data')
+    if not data:
+        return None
 
-    if koji_proxy:
-        try:
-            build = koji_proxy.getBuild(subject_identifier)
-            if build:
-                target = koji_proxy.getTaskRequest(build['task_id'])[1]
-                return _guess_product_version(target, koji_build=True)
-        except KeyError:
-            pass
-        except xmlrpc.client.Fault:
-            pass
+    task_id = _unpack_value(data.get('brew_task_id'))
+    try:
+        return int(task_id)
+    except (ValueError, TypeError):
+        return None
 
 
 class ResultsDBHandler(Consumer):
@@ -112,13 +78,7 @@ class ResultsDBHandler(Consumer):
         except KeyError:
             data = message['msg']['task']  # Old format
 
-        def _decode(value):
-            """ Decode either a string or a list of strings. """
-            if value and len(value) == 1:
-                value = value[0]
-            return value
-
-        _type = _decode(data.get('type'))
+        _type = _unpack_value(data.get('type'))
         # note: it is *intentional* that we do not handle old format
         # compose-type messages, because it is impossible to reliably
         # produce a decision from these. compose decisions can only be
@@ -129,20 +89,20 @@ class ResultsDBHandler(Consumer):
         # https://pagure.io/taskotron/resultsdb/pull-request/101
         # https://pagure.io/greenwave/pull-request/262#comment-70350
         if 'productmd.compose.id' in data:
-            yield ('compose', _decode(data['productmd.compose.id']))
+            yield ('compose', _unpack_value(data['productmd.compose.id']))
         elif _type == 'compose':
             pass
         elif 'original_spec_nvr' in data:
-            nvr = _decode(data['original_spec_nvr'])
+            nvr = _unpack_value(data['original_spec_nvr'])
             # when the pipeline ignores a package, which happens
             # *a lot*, we get a message with an 'original_spec_nvr'
             # key with an empty value; let's not try and handle this
             if nvr:
                 yield ('koji_build', nvr)
         elif _type == 'brew-build':
-            yield ('koji_build', _decode(data['item']))
+            yield ('koji_build', _unpack_value(data['item']))
         elif 'item' in data and _type:
-            yield (_type, _decode(data['item']))
+            yield (_type, _unpack_value(data['item']))
 
     def _consume_message(self, message):
         msg = message['msg']
@@ -157,11 +117,17 @@ class ResultsDBHandler(Consumer):
         except KeyError:
             submit_time = msg['result']['submit_time']
 
+        brew_task_id = _get_brew_task_id(msg)
+
         for subject_type, subject_identifier in self.announcement_subjects(message):
             log.debug('Considering subject %s: %r', subject_type, subject_identifier)
 
-            product_version = _subject_product_version(
-                subject_identifier, subject_type, self.koji_proxy)
+            product_version = subject_product_version(
+                subject_identifier,
+                subject_type,
+                self.koji_proxy,
+                brew_task_id,
+            )
 
             self._publish_decision_change(
                 submit_time=submit_time,
diff --git a/greenwave/product_versions.py b/greenwave/product_versions.py
new file mode 100644
index 0000000..f83dec6
--- /dev/null
+++ b/greenwave/product_versions.py
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0+
+"""
+Product version guessing for subject identifiers
+"""
+
+import logging
+import re
+
+import xmlrpc.client
+
+log = logging.getLogger(__name__)
+
+
+def _guess_product_version(toparse, koji_build=False):
+    if toparse == 'rawhide' or toparse.startswith('Fedora-Rawhide'):
+        return 'fedora-rawhide'
+
+    product_version = None
+    if toparse.startswith('f') and koji_build:
+        product_version = 'fedora-'
+    elif toparse.startswith('epel'):
+        product_version = 'epel-'
+    elif toparse.startswith('el') and len(toparse) > 2 and toparse[2].isdigit():
+        product_version = 'rhel-'
+    elif toparse.startswith('fc') or toparse.startswith('Fedora'):
+        product_version = 'fedora-'
+
+    if product_version:
+        # seperate the prefix from the number
+        result = list(filter(None, '-'.join(re.split(r'(\d+)', toparse)).split('-')))
+        if len(result) >= 2:
+            try:
+                int(result[1])
+                product_version += result[1]
+                return product_version
+            except ValueError:
+                pass
+
+    log.error("It wasn't possible to guess the product version")
+    return None
+
+
+def _guess_koji_build_product_version(
+        subject_identifier, koji_proxy, koji_task_id=None):
+    try:
+        if not koji_task_id:
+            build = koji_proxy.getBuild(subject_identifier) or {}
+            koji_task_id = build.get('task_id')
+            if not koji_task_id:
+                return None
+
+        target = koji_proxy.getTaskRequest(koji_task_id)[1]
+        return _guess_product_version(target, koji_build=True)
+    except xmlrpc.client.Fault:
+        pass
+
+
+def subject_product_version(
+        subject_identifier,
+        subject_type,
+        koji_proxy=None,
+        koji_task_id=None):
+    if subject_type == 'koji_build':
+        try:
+            _, _, release = subject_identifier.rsplit('-', 2)
+            _, short_prod_version = release.rsplit('.', 1)
+            return _guess_product_version(short_prod_version, koji_build=True)
+        except (KeyError, ValueError):
+            pass
+
+    if subject_type == "compose":
+        return _guess_product_version(subject_identifier)
+
+    if subject_type == "redhat-module":
+        return "rhel-8"
+
+    if koji_proxy:
+        return _guess_koji_build_product_version(
+            subject_identifier, koji_proxy, koji_task_id)
diff --git a/greenwave/tests/test_resultsdb_consumer.py b/greenwave/tests/test_resultsdb_consumer.py
index fe7fdf7..21669ae 100644
--- a/greenwave/tests/test_resultsdb_consumer.py
+++ b/greenwave/tests/test_resultsdb_consumer.py
@@ -7,6 +7,7 @@ from textwrap import dedent
 
 import greenwave.app_factory
 import greenwave.consumers.resultsdb
+from greenwave.product_versions import subject_product_version
 from greenwave.policies import Policy
 
 
@@ -313,7 +314,6 @@ def test_remote_rule_decision_change_not_matching(
 
 
 def test_guess_product_version():
-    # pylint: disable=protected-access
     hub = mock.MagicMock()
     hub.config = {
         'environment': 'environment',
@@ -321,32 +321,39 @@ def test_guess_product_version():
     }
     handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub)
     with handler.flask_app.app_context():
-        product_version = greenwave.consumers.resultsdb._subject_product_version(
+        product_version = subject_product_version(
             'release-e2e-test-1.0.1685-1.el5', 'koji_build')
         assert product_version == 'rhel-5'
 
-        product_version = greenwave.consumers.resultsdb._subject_product_version(
+        product_version = subject_product_version(
             'rust-toolset-rhel8-20181010170614.b09eea91', 'redhat-module')
         assert product_version == 'rhel-8'
 
 
 def test_guess_product_version_with_koji():
-    # pylint: disable=protected-access,unused-variable
-    class DummyKojiProxy():
-        def getBuild(self, subject_identifier):
-            assert 'fake_koji_build' == subject_identifier
-
     koji_proxy = mock.MagicMock()
     koji_proxy.getBuild.return_value = {'task_id': 666}
     koji_proxy.getTaskRequest.return_value = ['git://example.com/project', 'rawhide', {}]
 
-    product_version = greenwave.consumers.resultsdb._subject_product_version(
+    product_version = subject_product_version(
         'fake_koji_build', 'container-build', koji_proxy)
     koji_proxy.getBuild.assert_called_once_with('fake_koji_build')
     koji_proxy.getTaskRequest.assert_called_once_with(666)
     assert product_version == 'fedora-rawhide'
 
 
+def test_guess_product_version_with_koji_without_task_id():
+    koji_proxy = mock.MagicMock()
+    koji_proxy.getBuild.return_value = {'task_id': None}
+
+    product_version = subject_product_version(
+        'fake_koji_build', 'container-build', koji_proxy)
+
+    koji_proxy.getBuild.assert_called_once_with('fake_koji_build')
+    koji_proxy.getTaskRequest.assert_not_called()
+    assert product_version is None
+
+
 @pytest.mark.parametrize('nvr', (
     'badnvr.elastic-1-228',
     'badnvr-1.2-1.elastic8',
@@ -354,8 +361,7 @@ def test_guess_product_version_with_koji():
     'badnvr-1.2.f30',
 ))
 def test_guess_product_version_failure(nvr):
-    # pylint: disable=protected-access
-    product_version = greenwave.consumers.resultsdb._subject_product_version(nvr, 'koji_build')
+    product_version = subject_product_version(nvr, 'koji_build')
     assert product_version is None
 
 
@@ -594,3 +600,91 @@ def test_real_fedora_messaging_msg(
                 'subject_identifier': 'FEDORA-2019-9244c8b209',
                 'previous': None,
             }
+
+
+@mock.patch('greenwave.resources.ResultsRetriever.retrieve')
+@mock.patch('greenwave.resources.retrieve_decision')
+def test_container_brew_build(
+        mock_retrieve_decision,
+        mock_retrieve_results):
+    message = {
+        'msg': {
+            'submit_time': '2019-08-27T13:57:53.490376',
+            'testcase': {
+                'name': 'example_test',
+            },
+            'data': {
+                'brew_task_id': ['666'],
+                'type': ['brew-build'],
+                'item': ['example-container'],
+            }
+        }
+    }
+
+    policies = dedent("""
+        --- !Policy
+        id: test_policy
+        product_versions: [example_product_version]
+        decision_context: test_context
+        subject_type: koji_build
+        rules:
+          - !PassingTestCaseRule {test_case_name: example_test}
+    """)
+
+    config = 'fedora-messaging'
+    publish = 'greenwave.consumers.consumer.fedora_messaging.api.publish'
+
+    with mock.patch('greenwave.config.Config.MESSAGING', config):
+        with mock.patch(publish) as mock_fedmsg:
+            result = {
+                'id': 1,
+                'testcase': {'name': 'example_test'},
+                'outcome': 'PASSED',
+                'data': {'item': 'example-container', 'type': 'koji_build'},
+                'submit_time': '2019-04-24 13:06:12.135146'
+            }
+            mock_retrieve_results.return_value = [result]
+
+            def retrieve_decision(url, data):
+                #pylint: disable=unused-argument
+                if 'when' in data:
+                    return None
+                return {}
+            mock_retrieve_decision.side_effect = retrieve_decision
+
+            hub = mock.MagicMock()
+            hub.config = {
+                'environment': 'environment',
+                'topic_prefix': 'topic_prefix',
+            }
+            handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub)
+
+            koji_proxy = mock.MagicMock()
+            koji_proxy.getBuild.return_value = None
+            koji_proxy.getTaskRequest.return_value = [
+                'git://example.com/project', 'example_product_version', {}]
+            handler.koji_proxy = koji_proxy
+
+            handler.flask_app.config['policies'] = Policy.safe_load_all(policies)
+            with handler.flask_app.app_context():
+                handler.consume(message)
+
+            koji_proxy.getBuild.assert_not_called()
+            koji_proxy.getTaskRequest.assert_called_once_with(666)
+
+            assert len(mock_fedmsg.mock_calls) == 1
+
+            mock_call = mock_fedmsg.mock_calls[0][1][0]
+            assert mock_call.topic == 'greenwave.decision.update'
+            actual_msgs_sent = mock_call.body
+
+            assert actual_msgs_sent == {
+                'decision_context': 'test_context',
+                'product_version': 'example_product_version',
+                'subject': [
+                    {'item': 'example-container', 'type': 'koji_build'},
+                ],
+                'subject_type': 'koji_build',
+                'subject_identifier': 'example-container',
+                'previous': None,
+            }