From 9d3845a93174c3083aa23e05ae531b8c73a030ab Mon Sep 17 00:00:00 2001
From: Qixiang Wan <qwan@redhat.com>
Date: May 15 2017 12:29:22 +0000
Subject: [PATCH 1/3] Add db entries when module/container build is submitted


---

diff --git a/freshmaker/events.py b/freshmaker/events.py
index 33cb0fa..3131d90 100644
--- a/freshmaker/events.py
+++ b/freshmaker/events.py
@@ -143,9 +143,10 @@ class ModuleMetadataUpdated(BaseEvent):
     :param scm_url: SCM URL of a updated module.
     :param branch: Branch of updated module.
     """
-    def __init__(self, msg_id, scm_url, branch):
+    def __init__(self, msg_id, scm_url, name, branch):
         super(ModuleMetadataUpdated, self).__init__(msg_id)
         self.scm_url = scm_url
+        self.name = name
         self.branch = branch
 
 
diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py
index 071abf9..9d352ac 100644
--- a/freshmaker/handlers/__init__.py
+++ b/freshmaker/handlers/__init__.py
@@ -24,7 +24,7 @@
 import abc
 import fedmsg.utils
 
-from freshmaker import conf
+from freshmaker import conf, db, models
 
 
 def load_handlers():
@@ -59,3 +59,17 @@ class BaseHandler(object):
         generate internal events for other handlers in Freshmaker.
         """
         raise NotImplementedError()
+
+    def record_build(self, event, name, type, build_id, dep_of=None):
+        """
+        Record build in db.
+
+        :param event: instance of an event.
+        :param name: name of the artifact.
+        :param type: type of the artifact, can be 'rpm', 'image' or module.
+        :param build_id: id of the build in build system.
+        :param def_of: the artifact which this one depends on.
+        """
+        ev = models.Event.get_or_create(db.session, event.msg_id)
+        models.ArtifactBuild.create(db.session, ev, name, type, build_id, dep_of)
+        db.session.commit()
diff --git a/freshmaker/handlers/image_builder.py b/freshmaker/handlers/image_builder.py
index 5503b40..9aa2436 100644
--- a/freshmaker/handlers/image_builder.py
+++ b/freshmaker/handlers/image_builder.py
@@ -49,10 +49,13 @@ class DockerImageRebuildHandler(BaseHandler):
         log.info('Start to rebuild docker image %s', event.repo)
 
         try:
-            self.build_image(repo_url=event.repo_url,
-                             rev=event.rev,
-                             branch=event.branch,
-                             namespace=event.namespace)
+            task_id = self.build_image(repo_url=event.repo_url,
+                                       rev=event.rev,
+                                       branch=event.branch,
+                                       namespace=event.namespace)
+
+            self.record_build(event, event.repo, 'image', task_id)
+
         except koji.krbV.Krb5Error as e:
             log.exception('Failed to login Koji via Kerberos using GSSAPI. %s', e.args[1])
         except:
@@ -98,7 +101,8 @@ class DockerImageRebuildHandlerForBodhi(DockerImageRebuildHandler):
 
         for container in containers:
             try:
-                self.handle_image_build(container)
+                task_id = self.handle_image_build(container)
+                self.record_build(event, container['name'], 'image', task_id)
             except:
                 log.exception('Error when rebuild %s', container)
 
diff --git a/freshmaker/handlers/mbs.py b/freshmaker/handlers/mbs.py
index 8092fb0..1c4a01c 100644
--- a/freshmaker/handlers/mbs.py
+++ b/freshmaker/handlers/mbs.py
@@ -82,11 +82,13 @@ class MBS(BaseHandler):
 
         if commitid is not None:
             scm_url = conf.git_base_url + '/modules/%s.git?#%s' % (name, commitid)
-            self.rebuild_module(scm_url, branch)
+            return self.rebuild_module(scm_url, branch)
 
     def handle_metadata_update(self, event):
         log.info("Triggering rebuild of %s, metadata updated", event.scm_url)
-        self.rebuild_module(event.scm_url, event.branch)
+        build_id = self.rebuild_module(event.scm_url, event.branch)
+        if build_id is not None:
+            self.record_build(event, event.name, 'module', build_id)
 
         return []
 
@@ -108,11 +110,12 @@ class MBS(BaseHandler):
                                          build_dep_stream=module_stream,
                                          active='true')
         for mod in modules:
+            name = mod['variant_name']
+            version = mod['variant_version']
             commit_msg = "Bump to rebuild because of %s update" % module_name
-            self.bump_and_rebuild_module(mod['variant_name'],
-                                         mod['variant_version'],
-                                         commit_msg=commit_msg)
-        return []
+            build_id = self.bump_and_rebuild_module(name, version, commit_msg=commit_msg)
+            if build_id is not None:
+                self.record_build(event, name, 'module', build_id)
 
     def handle_rpm_spec_updated(self, event):
         """
@@ -135,7 +138,9 @@ class MBS(BaseHandler):
             module_branch = mod['variant_version']
             log.info("Going to rebuild module '%s:%s'.", module_name, module_branch)
             commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev)
-            self.bump_and_rebuild_module(module_name, module_branch, commit_msg=commit_msg)
+            build_id = self.bump_and_rebuild_module(module_name, module_branch, commit_msg=commit_msg)
+            if build_id is not None:
+                self.record_build(event, module_name, 'module', build_id)
 
         return []
 
diff --git a/freshmaker/models.py b/freshmaker/models.py
index cfbdc82..a1287be 100644
--- a/freshmaker/models.py
+++ b/freshmaker/models.py
@@ -24,15 +24,10 @@
 """ SQLAlchemy Database models for the Flask app
 """
 
-import contextlib
-
 from datetime import datetime
-from sqlalchemy import engine_from_config
-from sqlalchemy.orm import (validates, scoped_session, sessionmaker,
-                            relationship)
-from sqlalchemy.ext.declarative import declarative_base
+from sqlalchemy.orm import (validates, relationship)
 
-from freshmaker import db, log
+from freshmaker import db
 
 # BUILD_STATES for the builds submitted by Freshmaker
 BUILD_STATES = {
@@ -56,9 +51,11 @@ ARTIFACT_TYPES = {
 
 INVERSE_ARTIFACT_TYPES = {v: k for k, v in ARTIFACT_TYPES.items()}
 
+
 class FreshmakerBase(db.Model):
     __abstract__ = True
 
+
 class Event(FreshmakerBase):
     __tablename__ = "events"
     id = db.Column(db.Integer, primary_key=True)
@@ -75,6 +72,17 @@ class Event(FreshmakerBase):
         session.add(event)
         return event
 
+    @classmethod
+    def get_or_create(cls, session, message_id):
+        instance = session.query(cls).filter_by(message_id=message_id).first()
+        if instance:
+            return instance
+        return cls.create(session, message_id)
+
+    def __repr__(self):
+        return "<Event %s>" % (self.message_id)
+
+
 class ArtifactBuild(FreshmakerBase):
     __tablename__ = "artifact_builds"
     id = db.Column(db.Integer, primary_key=True)
@@ -131,4 +139,3 @@ class ArtifactBuild(FreshmakerBase):
         return "<ArtifactBuild %s, type %s, state %s, event %s>" % (
             self.name, INVERSE_ARTIFACT_TYPES[self.type],
             INVERSE_BUILD_STATES[self.state], self.event.message_id)
-
diff --git a/freshmaker/parsers/gitreceive.py b/freshmaker/parsers/gitreceive.py
index 6982577..da6811c 100644
--- a/freshmaker/parsers/gitreceive.py
+++ b/freshmaker/parsers/gitreceive.py
@@ -67,7 +67,7 @@ class GitReceiveParser(BaseParser):
             scm_url = "%s/%s/%s.git?#%s" % (conf.git_base_url, namespace, repo, rev)
             log.debug("Parsed ModuleMetadataUpdated fedmsg, scm_url=%s, "
                       "branch=%s", scm_url, branch)
-            return ModuleMetadataUpdated(msg_id, scm_url, branch)
+            return ModuleMetadataUpdated(msg_id, scm_url, repo, branch)
 
         elif namespace == 'container':
             changed_files = msg['msg']['commit']['stats']['files']
diff --git a/tests/handlers/test_image_builder.py b/tests/handlers/test_image_builder.py
index 6dfc7fe..4f5296c 100644
--- a/tests/handlers/test_image_builder.py
+++ b/tests/handlers/test_image_builder.py
@@ -20,7 +20,6 @@
 #
 # Written by Chenxiong Qi <cqi@redhat.com>
 
-import shutil
 import tempfile
 import unittest
 
@@ -32,13 +31,23 @@ from mock import patch
 from mock import MagicMock
 from mock import call
 
-from freshmaker import conf
+from freshmaker import conf, db, models
 from freshmaker.consumer import FreshmakerConsumer
 from freshmaker.handlers.image_builder import DockerImageRebuildHandlerForBodhi
 from tests import get_fedmsg
 
 
 class BaseTestCase(unittest.TestCase):
+    def setUp(self):
+        db.session.remove()
+        db.drop_all()
+        db.create_all()
+        db.session.commit()
+
+    def tearDown(self):
+        db.session.remove()
+        db.drop_all()
+        db.session.commit()
 
     def create_consumer(self):
         hub = MagicMock()
@@ -65,9 +74,11 @@ class TestImageBuilderHandler(BaseTestCase):
             'weburl': 'https://localhost/koji',
         }
 
-        self.consume_fedmsg(get_fedmsg('git_receive_dockerfile_changed'))
-
         mock_session = ClientSession.return_value
+        mock_session.buildContainer.return_value = 123
+        msg = get_fedmsg('git_receive_dockerfile_changed')
+        self.consume_fedmsg(msg)
+
         mock_session.krb_login.assert_called_once_with(proxyuser=None)
         mock_session.buildContainer.assert_called_once_with(
             'git://pkgs.fedoraproject.org/container/testimage.git?#e1f39d43471fc37ec82616f76a119da4eddec787',
@@ -75,6 +86,15 @@ class TestImageBuilderHandler(BaseTestCase):
             {'scratch': True, 'git_branch': 'master'})
         mock_session.logout.assert_called_once()
 
+        events = models.Event.query.all()
+        self.assertEquals(len(events), 1)
+        self.assertEquals(events[0].message_id, msg['body']['msg_id'])
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 1)
+        self.assertEquals(builds[0].name, 'testimage')
+        self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['image'])
+        self.assertEquals(builds[0].build_id, 123)
+
     @patch('freshmaker.handlers.image_builder.DockerImageRebuildHandler.build_image')
     def test_not_rebuild_if_Dockerfile_not_changed(self, build_image):
         self.consume_fedmsg(get_fedmsg('git_receive_dockerfile_not_changed'))
@@ -157,6 +177,7 @@ mock_release_components = {
     }
 }
 
+
 def mock_get_release_component(pdc_session, id):
     return mock_release_components[id]
 
@@ -165,6 +186,7 @@ def mock_get_release_component(pdc_session, id):
 class TestRebuildWhenBodhiUpdateStable(BaseTestCase):
 
     def setUp(self):
+        super(TestRebuildWhenBodhiUpdateStable, self).setUp()
         # Use to return a temporary directory from temp_dir method. So, no need
         # to delete this directory, since temp_dir ensures to do that.
         self.working_dir = tempfile.mkdtemp(prefix='test-image-rebuild-')
@@ -199,7 +221,11 @@ class TestRebuildWhenBodhiUpdateStable(BaseTestCase):
 
         get_containers_including_rpms.return_value = mock_found_containers
 
-        self.consume_fedmsg(get_fedmsg('bodhi_update_stable'))
+        session = ClientSession.return_value
+        session.buildContainer.side_effect = [123, 456]
+
+        msg = get_fedmsg('bodhi_update_stable')
+        self.consume_fedmsg(msg)
 
         self.assertEqual(2, _run_command.call_count)
 
@@ -212,8 +238,6 @@ class TestRebuildWhenBodhiUpdateStable(BaseTestCase):
                  rundir=self.working_dir)
         ])
 
-        session = ClientSession.return_value
-
         self.assertEqual(2, session.krb_login.call_count)
 
         buildContainer = session.buildContainer
@@ -232,6 +256,18 @@ class TestRebuildWhenBodhiUpdateStable(BaseTestCase):
             ],
             any_order=True)
 
+        events = models.Event.query.all()
+        self.assertEquals(len(events), 1)
+        self.assertEquals(events[0].message_id, msg['body']['msg_id'])
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 2)
+        self.assertEquals(builds[0].name, 'testimage1')
+        self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['image'])
+        self.assertEquals(builds[0].build_id, 123)
+        self.assertEquals(builds[1].name, 'testimage2')
+        self.assertEquals(builds[1].type, models.ARTIFACT_TYPES['image'])
+        self.assertEquals(builds[1].build_id, 456)
+
 
 class TestContainersIncludingRPMs(unittest.TestCase):
 
diff --git a/tests/test_mbs_handler.py b/tests/test_mbs_handler.py
index 17e4a67..a5bf068 100644
--- a/tests/test_mbs_handler.py
+++ b/tests/test_mbs_handler.py
@@ -26,7 +26,7 @@ import mock
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 from tests import helpers
 
-from freshmaker import events
+from freshmaker import events, db, models
 from freshmaker.handlers.mbs import MBS
 from freshmaker.parsers.mbsmodule import MBSModuleParser
 from freshmaker.parsers.gitreceive import GitReceiveParser
@@ -34,9 +34,19 @@ from freshmaker.parsers.gitreceive import GitReceiveParser
 
 class MBSHandlerTest(unittest.TestCase):
     def setUp(self):
+        db.session.remove()
+        db.drop_all()
+        db.create_all()
+        db.session.commit()
+
         events.BaseEvent.register_parser(MBSModuleParser)
         events.BaseEvent.register_parser(GitReceiveParser)
 
+    def tearDown(self):
+        db.session.remove()
+        db.drop_all()
+        db.session.commit()
+
     def _get_event(self, message):
         event = events.BaseEvent.from_fedmsg(message['body']['topic'], message['body'])
         return event
@@ -103,12 +113,25 @@ class MBSHandlerTest(unittest.TestCase):
             "43ec03000d249231bc7135b11b810afc96e90efb",
         ]
         handler.rebuild_module = mock.Mock()
+        handler.rebuild_module.side_effect = [123, 456]
         handler.handle_module_built(event)
 
         self.assertEqual(handler.rebuild_module.call_args_list,
                          [mock.call(u'git://pkgs.fedoraproject.org/modules/testmodule2.git?#fae7848fa47a854f25b782aa64441040a6d86544', u'master'),
                           mock.call(u'git://pkgs.fedoraproject.org/modules/testmodule3.git?#43ec03000d249231bc7135b11b810afc96e90efb', u'master')])
 
+        event_list = models.Event.query.all()
+        self.assertEquals(len(event_list), 1)
+        self.assertEquals(event_list[0].message_id, event.msg_id)
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 2)
+        self.assertEquals(builds[0].name, mod2_r1['variant_name'])
+        self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['module'])
+        self.assertEquals(builds[0].build_id, 123)
+        self.assertEquals(builds[1].name, mod3_r1['variant_name'])
+        self.assertEquals(builds[1].build_id, 456)
+        self.assertEquals(builds[1].type, models.ARTIFACT_TYPES['module'])
+
     @mock.patch('freshmaker.pdc.get_modules')
     @mock.patch('freshmaker.handlers.mbs.utils')
     @mock.patch('freshmaker.handlers.mbs.conf')
@@ -154,11 +177,21 @@ class MBSHandlerTest(unittest.TestCase):
             "43ec03000d249231bc7135b11b810afc96e90efb",
         ]
         handler.rebuild_module = mock.Mock()
+        handler.rebuild_module.return_value = 123
         handler.handle_module_built(event)
 
         self.assertEqual(handler.rebuild_module.call_args_list,
                          [mock.call(u'git://pkgs.fedoraproject.org/modules/testmodule2.git?#fae7848fa47a854f25b782aa64441040a6d86544', u'master')])
 
+        event_list = models.Event.query.all()
+        self.assertEquals(len(event_list), 1)
+        self.assertEquals(event_list[0].message_id, event.msg_id)
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 1)
+        self.assertEquals(builds[0].name, mod2_r1['variant_name'])
+        self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['module'])
+        self.assertEquals(builds[0].build_id, 123)
+
     def test_can_handle_rpm_spec_updated_event(self):
         """
         Tests MBS handler can handle rpm spec updated event
@@ -213,10 +246,19 @@ class MBSHandlerTest(unittest.TestCase):
 
         handler = MBS()
         handler.rebuild_module = mock.Mock()
+        handler.rebuild_module.return_value = 123
         handler.handle(event)
         self.assertEqual(handler.rebuild_module.call_args_list,
                          [mock.call('git://pkgs.fedoraproject.org/modules/testmodule.git?#%s' % commitid, 'master')])
 
+        event_list = models.Event.query.all()
+        self.assertEquals(len(event_list), 1)
+        self.assertEquals(event_list[0].message_id, event.msg_id)
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 1)
+        self.assertEquals(builds[0].name, 'testmodule')
+        self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['module'])
+        self.assertEquals(builds[0].build_id, 123)
 
 if __name__ == '__main__':
     unittest.main()

From 135aaeb999facd7d90731de953ab9c52d4e3306f Mon Sep 17 00:00:00 2001
From: Qixiang Wan <qwan@redhat.com>
Date: May 15 2017 12:32:16 +0000
Subject: [PATCH 2/3] Update build state when receives module build state change message


When receives a module build state change message, if the build is
submitted by freshmaker, which means we can find it in db, update
the its state in db.

---

diff --git a/freshmaker/handlers/mbs.py b/freshmaker/handlers/mbs.py
index 1c4a01c..e7f3c35 100644
--- a/freshmaker/handlers/mbs.py
+++ b/freshmaker/handlers/mbs.py
@@ -23,20 +23,28 @@
 
 import requests
 
-from freshmaker import log, conf, utils, pdc
+from freshmaker import log, conf, utils, pdc, db, models
 from freshmaker.handlers import BaseHandler
 from freshmaker.events import ModuleBuilt, ModuleMetadataUpdated, RPMSpecUpdated
 
 
+MBS_BUILD_STATES = {
+    "init": 0,
+    "wait": 1,
+    "build": 2,
+    "done": 3,
+    "failed": 4,
+    "ready": 5,
+}
+
+
 class MBS(BaseHandler):
     name = "MBS"
 
     def can_handle(self, event):
-        # Handle only "ready" state of ModuleBuilt.
         # TODO: Handle only when something depends on
         # this module.
-        if (isinstance(event, ModuleBuilt) and
-                event.module_build_state == 5):
+        if isinstance(event, ModuleBuilt):
             return True
 
         if isinstance(event, ModuleMetadataUpdated):
@@ -100,22 +108,41 @@ class MBS(BaseHandler):
         """
         module_name = event.module_name
         module_stream = event.module_stream
-
-        log.info("Triggering rebuild of modules depending on %s:%s "
-                 "in MBS", module_name, module_stream)
-
-        pdc_session = pdc.get_client_session(conf)
-        modules = pdc.get_latest_modules(pdc_session,
-                                         build_dep_name=module_name,
-                                         build_dep_stream=module_stream,
-                                         active='true')
-        for mod in modules:
-            name = mod['variant_name']
-            version = mod['variant_version']
-            commit_msg = "Bump to rebuild because of %s update" % module_name
-            build_id = self.bump_and_rebuild_module(name, version, commit_msg=commit_msg)
-            if build_id is not None:
-                self.record_build(event, name, 'module', build_id)
+        build_id = event.module_build_id
+        build_state = event.module_build_state
+
+        # update build state if the build is submitted by Freshmaker
+        builds = db.session.query(models.ArtifactBuild).filter_by(build_id=build_id,
+                                                                  type=models.ARTIFACT_TYPES['module']).all()
+        if len(builds) > 1:
+            raise RuntimeError("Found duplicate module build '%s' in db" % build_id)
+        if len(builds) == 1:
+            build = builds[0]
+            if build_state in [MBS_BUILD_STATES['ready'], MBS_BUILD_STATES['failed']]:
+                log.info("Module build '%s' state changed in MBS, updating it in db.", build_id)
+            if build_state == MBS_BUILD_STATES['ready']:
+                build.state = models.BUILD_STATES['done']
+            if build_state == MBS_BUILD_STATES['failed']:
+                build.state = models.BUILD_STATES['failed']
+            db.session.commit()
+
+        # Rebuild depending modules when state of ModuleBuilt is 'ready'
+        if build_state == MBS_BUILD_STATES['ready']:
+            log.info("Triggering rebuild of modules depending on %s:%s "
+                     "in MBS", module_name, module_stream)
+
+            pdc_session = pdc.get_client_session(conf)
+            modules = pdc.get_latest_modules(pdc_session,
+                                             build_dep_name=module_name,
+                                             build_dep_stream=module_stream,
+                                             active='true')
+            for mod in modules:
+                name = mod['variant_name']
+                version = mod['variant_version']
+                commit_msg = "Bump to rebuild because of %s update" % module_name
+                build_id = self.bump_and_rebuild_module(name, version, commit_msg=commit_msg)
+                if build_id is not None:
+                    self.record_build(event, name, 'module', build_id)
 
     def handle_rpm_spec_updated(self, event):
         """
diff --git a/tests/test_mbs_handler.py b/tests/test_mbs_handler.py
index a5bf068..7e89f46 100644
--- a/tests/test_mbs_handler.py
+++ b/tests/test_mbs_handler.py
@@ -51,28 +51,16 @@ class MBSHandlerTest(unittest.TestCase):
         event = events.BaseEvent.from_fedmsg(message['body']['topic'], message['body'])
         return event
 
-    def test_can_handle_module_built_ready_event(self):
+    def test_can_handle_module_built_event(self):
         """
-        Tests MBS handler can handle modult build ready message
+        Tests MBS handler can handle module built message
         """
-
-        msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='ready').produce()
-        event = self._get_event(msg)
-
-        handler = MBS()
-        self.assertTrue(handler.can_handle(event))
-
-    def test_can_not_handle_module_built_non_ready_event(self):
-        """
-        Tests MBS handler cannot handle modult build message which is not with
-        'ready' state.
-        """
-        for s in ['init', 'wait', 'build', 'done', 'failed']:
-            msg = helpers.ModuleBuiltMessage('testmodule', 'master', state=s).produce()
+        for state in ['init', 'wait', 'build', 'done', 'failed', 'ready']:
+            msg = helpers.ModuleBuiltMessage('testmodule', 'master', state=state).produce()
             event = self._get_event(msg)
 
             handler = MBS()
-            self.assertFalse(handler.can_handle(event))
+            self.assertTrue(handler.can_handle(event))
 
     @mock.patch('freshmaker.pdc.get_modules')
     @mock.patch('freshmaker.handlers.mbs.utils')
@@ -260,5 +248,68 @@ class MBSHandlerTest(unittest.TestCase):
         self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['module'])
         self.assertEquals(builds[0].build_id, 123)
 
+    @mock.patch('freshmaker.handlers.mbs.utils')
+    @mock.patch('freshmaker.handlers.mbs.pdc')
+    @mock.patch('freshmaker.handlers.mbs.conf')
+    def test_update_build_state_in_db(self, conf, pdc, utils):
+        """
+        Test build state in db will be updated when receives module build
+        state change message.
+        """
+
+        # trigger a build on rpm spec updated event first
+        conf.git_base_url = "git://pkgs.fedoraproject.org"
+
+        m = helpers.DistGitMessage('rpms', 'bash', 'master', '123')
+        m.add_changed_file('bash.spec', 1, 1)
+        msg = m.produce()
+
+        event = self._get_event(msg)
+
+        mod_info = helpers.PDCModuleInfo('testmodule', 'master', '20170412010101')
+        mod_info.add_rpm("bash-1.2.3-4.f26.rpm")
+        mod = mod_info.produce()
+        pdc.get_latest_modules.return_value = [mod]
+
+        commitid = '9287eb8eb4c4c60f73b4a59f228a673846d940c6'
+        utils.get_commit_hash.return_value = commitid
+
+        handler = MBS()
+        handler.rebuild_module = mock.Mock()
+        handler.rebuild_module.return_value = 123
+        handler.handle(event)
+        self.assertEqual(handler.rebuild_module.call_args_list,
+                         [mock.call('git://pkgs.fedoraproject.org/modules/testmodule.git?#%s' % commitid, 'master')])
+
+        event_list = models.Event.query.all()
+        self.assertEquals(len(event_list), 1)
+        self.assertEquals(event_list[0].message_id, event.msg_id)
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 1)
+        self.assertEquals(builds[0].name, 'testmodule')
+        self.assertEquals(builds[0].type, models.ARTIFACT_TYPES['module'])
+        self.assertEquals(builds[0].build_id, 123)
+        self.assertEquals(builds[0].state, models.BUILD_STATES['build'])
+
+        # update build state when receive module built messages
+        # build is failed
+        msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='failed', build_id=123).produce()
+        event = self._get_event(msg)
+        handler.handle(event)
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 1)
+        # build state updated to 'failed'
+        self.assertEquals(builds[0].state, models.BUILD_STATES['failed'])
+
+        # build is ready
+        pdc.get_latest_modules.return_value = []
+        msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='ready', build_id=123).produce()
+        event = self._get_event(msg)
+        handler.handle(event)
+        builds = models.ArtifactBuild.query.all()
+        self.assertEquals(len(builds), 1)
+        # build state updated to 'done'
+        self.assertEquals(builds[0].state, models.BUILD_STATES['done'])
+
 if __name__ == '__main__':
     unittest.main()

From 28e0f6b1335d10f3f75c3e7c65ba425e511fa34c Mon Sep 17 00:00:00 2001
From: Qixiang Wan <qwan@redhat.com>
Date: May 15 2017 12:33:37 +0000
Subject: [PATCH 3/3] Update build state when receives koji task state change message


When reveives a koji task state change message, and the koji task
id exists in db as a image build, update build state in db.

---

diff --git a/conf/config.py b/conf/config.py
index 4dd572c..45283cf 100644
--- a/conf/config.py
+++ b/conf/config.py
@@ -45,6 +45,7 @@ class BaseConfiguration(object):
         "freshmaker.handlers.mbs:MBS",  # Module Build Service
         "freshmaker.handlers.image_builder:DockerImageRebuildHandler",
         "freshmaker.handlers.image_builder:DockerImageRebuildHandlerForBodhi",
+        "freshmaker.handlers.buildsys:BuildsysHandler",
     ]
 
     # Base URL of git repository with source artifacts.
@@ -76,6 +77,7 @@ class BaseConfiguration(object):
 
     SSL_ENABLED = False
 
+
 class DevConfiguration(BaseConfiguration):
     DEBUG = True
     LOG_BACKEND = 'console'
diff --git a/freshmaker/consumer.py b/freshmaker/consumer.py
index b2ef291..0e7ef38 100644
--- a/freshmaker/consumer.py
+++ b/freshmaker/consumer.py
@@ -31,6 +31,7 @@ import freshmaker.handlers
 import freshmaker.parsers.mbsmodule
 import freshmaker.parsers.gitreceive
 import freshmaker.parsers.bodhiupdate
+import freshmaker.parsers.buildsys
 
 from freshmaker import log, conf, messaging, events
 
@@ -65,6 +66,7 @@ class FreshmakerConsumer(fedmsg.consumers.FedmsgConsumer):
         events.BaseEvent.register_parser(freshmaker.parsers.mbsmodule.MBSModuleParser)
         events.BaseEvent.register_parser(freshmaker.parsers.gitreceive.GitReceiveParser)
         events.BaseEvent.register_parser(freshmaker.parsers.bodhiupdate.UpdateCompleteStableParser)
+        events.BaseEvent.register_parser(freshmaker.parsers.buildsys.BuildsysParser)
         log.debug("Parser classes: %r", events.BaseEvent._parsers)
 
         self.topic = events.BaseEvent.get_parsed_topics()
diff --git a/freshmaker/events.py b/freshmaker/events.py
index 3131d90..1bc3535 100644
--- a/freshmaker/events.py
+++ b/freshmaker/events.py
@@ -211,3 +211,13 @@ class BodhiUpdateCompleteStable(BaseEvent):
         self.update_id = update_id
         self.builds = builds
         self.release = release
+
+
+class KojiTaskStateChanged(BaseEvent):
+    """
+    Provides an event object for "the state of task changed in koji"
+    """
+    def __init__(self, msg_id, task_id, task_state):
+        super(KojiTaskStateChanged, self).__init__(msg_id)
+        self.task_id = task_id
+        self.task_state = task_state
diff --git a/freshmaker/handlers/buildsys.py b/freshmaker/handlers/buildsys.py
new file mode 100644
index 0000000..f17937c
--- /dev/null
+++ b/freshmaker/handlers/buildsys.py
@@ -0,0 +1,62 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2017  Red Hat, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in all
+# copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+from freshmaker import log, db, models
+from freshmaker.handlers import BaseHandler
+from freshmaker.events import KojiTaskStateChanged
+
+
+class BuildsysHandler(BaseHandler):
+    name = "BuildsysHandler"
+
+    def can_handle(self, event):
+        if isinstance(event, KojiTaskStateChanged):
+            return True
+
+        return False
+
+    def handle_koji_task_state_changed(self, event):
+        task_id = event.task_id
+        task_state = event.task_state
+
+        # check whether the task exists in db as image build
+        builds = db.session.query(models.ArtifactBuild).filter_by(build_id=task_id,
+                                                                  type=models.ARTIFACT_TYPES['image']).all()
+        if len(builds) > 1:
+            raise RuntimeError("Found duplicate image build '%s' in db" % task_id)
+        if len(builds) == 1:
+            build = builds[0]
+            if task_state in ['CLOSED', 'FAILED']:
+                log.info("Image build '%s' state changed in koji, updating it in db.", task_id)
+            if task_state == 'CLOSED':
+                build.state = models.BUILD_STATES['done']
+                db.session.commit()
+            if task_state == 'FAILED':
+                build.state = models.BUILD_STATES['failed']
+                db.session.commit()
+
+        return []
+
+    def handle(self, event):
+        if isinstance(event, KojiTaskStateChanged):
+            return self.handle_koji_task_state_changed(event)
+
+        return []
diff --git a/freshmaker/parsers/buildsys.py b/freshmaker/parsers/buildsys.py
new file mode 100644
index 0000000..b301fc2
--- /dev/null
+++ b/freshmaker/parsers/buildsys.py
@@ -0,0 +1,53 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2017  Red Hat, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in all
+# copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+from freshmaker import log
+from freshmaker.parsers import BaseParser
+from freshmaker.events import KojiTaskStateChanged
+
+
+class BuildsysParser(BaseParser):
+    """
+    Parser parsing task state change message from buildsys (koji), generating
+    KojiTaskStateChanged event.
+    """
+    name = "BuildsysParser"
+    topic_suffixes = ["buildsys.task.state.change"]
+
+    def can_parse(self, topic, msg):
+        log.debug(topic)
+        if not any([topic.endswith(s) for s in self.topic_suffixes]):
+            return False
+        return True
+
+    def parse(self, topic, msg):
+        msg_id = msg.get('msg_id')
+        msg_inner_msg = msg.get('msg')
+
+        # If there isn't a msg dict in msg then this message can be skipped
+        if not msg_inner_msg:
+            log.debug(('Skipping message without any content with the '
+                      'topic "{0}"').format(topic))
+            return None
+
+        return KojiTaskStateChanged(msg_id,
+                                    msg_inner_msg.get('id'),
+                                    msg_inner_msg.get('new'))
diff --git a/tests/helpers.py b/tests/helpers.py
index 2c60c96..8235874 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -23,6 +23,9 @@ import six
 import string
 import time
 import uuid
+import unittest
+
+from freshmaker import events
 
 
 BUILD_STATES = {
@@ -35,13 +38,19 @@ BUILD_STATES = {
 }
 
 
+class FreshmakerTestCase(unittest.TestCase):
+    def get_event_from_msg(self, message):
+        event = events.BaseEvent.from_fedmsg(message['body']['topic'], message['body'])
+        return event
+
+
 class FedMsgFactory(object):
     def __init__(self, *args, **kwargs):
         self.msg_id = "%s-%s" % (time.strftime("%Y"), uuid.uuid4())
         self.msg = {}
         self.signature = '123'
-        self.source_name = 'unittest',
-        self.source_version = '0.1.1',
+        self.source_name = 'unittest'
+        self.source_version = '0.1.1'
         self.timestamp = time.time()
         self.topic = ''
         self.username = 'freshmaker'
@@ -152,6 +161,29 @@ class DistGitMessage(FedMsgFactory):
         self.stats['total']['lines'] += self.stats['files'][filename]['lines']
 
 
+class BuildsysTaskStateChangeMessage(FedMsgFactory):
+    def __init__(self, task_id, old_state, new_state, *args, **kwargs):
+        super(BuildsysTaskStateChangeMessage, self).__init__(*args, **kwargs)
+        self.topic = 'org.fedoraproject.prod.buildsys.task.state.change'
+        self.attribute = 'state'
+        self.task_id = task_id
+        self.old_state = old_state
+        self.new_state = new_state
+        self.owner = 'freshmaker'
+        self.method = 'build'
+
+    @property
+    def inner_msg(self):
+        return {
+            'attribute': self.attribute,
+            'id': self.task_id,
+            'method': self.method,
+            'new': self.new_state,
+            'old': self.old_state,
+            'owner': self.owner,
+        }
+
+
 class PDCModuleInfoFactory(object):
     def __init__(self, name, version, release, active=True):
         self.variant_name = name
diff --git a/tests/test_buildsys_handler.py b/tests/test_buildsys_handler.py
new file mode 100644
index 0000000..a33be7e
--- /dev/null
+++ b/tests/test_buildsys_handler.py
@@ -0,0 +1,91 @@
+# Copyright (c) 2017  Red Hat, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in all
+# copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+import os
+import sys
+import unittest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+from tests import helpers
+
+from freshmaker import events, db, models
+from freshmaker.handlers.buildsys import BuildsysHandler
+from freshmaker.parsers.buildsys import BuildsysParser
+
+
+class BuildsysHandlerTest(helpers.FreshmakerTestCase):
+    def setUp(self):
+        db.session.remove()
+        db.drop_all()
+        db.create_all()
+        db.session.commit()
+
+        events.BaseEvent.register_parser(BuildsysParser)
+
+    def tearDown(self):
+        db.session.remove()
+        db.drop_all()
+        db.session.commit()
+
+    def test_can_handle_koji_task_state_changed_event(self):
+        """
+        Tests buildsys handler can handle koji task state changed message
+        """
+        m = helpers.BuildsysTaskStateChangeMessage(123, 'OPEN', 'FAILED')
+        msg = m.produce()
+        event = self.get_event_from_msg(msg)
+        handler = BuildsysHandler()
+        self.assertTrue(handler.can_handle(event))
+
+    def test_update_build_state_on_koji_task_state_changed_event(self):
+        """
+        Tests build state will be updated when receives koji task state changed message
+        """
+        task_id = 123
+        ev = models.Event.create(db.session, 'test_msg_id')
+        build = models.ArtifactBuild.create(db.session,
+                                            ev,
+                                            'testimage',
+                                            models.ARTIFACT_TYPES['image'],
+                                            task_id)
+        db.session.add(ev)
+        db.session.add(build)
+        db.session.commit()
+
+        m = helpers.BuildsysTaskStateChangeMessage(task_id, 'OPEN', 'FAILED')
+        msg = m.produce()
+        event = self.get_event_from_msg(msg)
+
+        handler = BuildsysHandler()
+        handler.handle(event)
+        build = models.ArtifactBuild.query.all()[0]
+        self.assertEqual(build.state, models.BUILD_STATES['failed'])
+
+        m = helpers.BuildsysTaskStateChangeMessage(task_id, 'OPEN', 'CLOSED')
+        msg = m.produce()
+        event = self.get_event_from_msg(msg)
+
+        handler = BuildsysHandler()
+        handler.handle(event)
+        build = models.ArtifactBuild.query.all()[0]
+        self.assertEqual(build.state, models.BUILD_STATES['done'])
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_mbs_handler.py b/tests/test_mbs_handler.py
index 7e89f46..ce3a697 100644
--- a/tests/test_mbs_handler.py
+++ b/tests/test_mbs_handler.py
@@ -32,7 +32,7 @@ from freshmaker.parsers.mbsmodule import MBSModuleParser
 from freshmaker.parsers.gitreceive import GitReceiveParser
 
 
-class MBSHandlerTest(unittest.TestCase):
+class MBSHandlerTest(helpers.FreshmakerTestCase):
     def setUp(self):
         db.session.remove()
         db.drop_all()
@@ -47,17 +47,13 @@ class MBSHandlerTest(unittest.TestCase):
         db.drop_all()
         db.session.commit()
 
-    def _get_event(self, message):
-        event = events.BaseEvent.from_fedmsg(message['body']['topic'], message['body'])
-        return event
-
     def test_can_handle_module_built_event(self):
         """
         Tests MBS handler can handle module built message
         """
         for state in ['init', 'wait', 'build', 'done', 'failed', 'ready']:
             msg = helpers.ModuleBuiltMessage('testmodule', 'master', state=state).produce()
-            event = self._get_event(msg)
+            event = self.get_event_from_msg(msg)
 
             handler = MBS()
             self.assertTrue(handler.can_handle(event))
@@ -71,7 +67,7 @@ class MBSHandlerTest(unittest.TestCase):
         in module built event.
         """
         msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='ready').produce()
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
 
         handler = MBS()
 
@@ -129,7 +125,7 @@ class MBSHandlerTest(unittest.TestCase):
         module only has old release depends on the module, it won't be rebuilt.
         """
         msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='ready').produce()
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
 
         handler = MBS()
 
@@ -188,7 +184,7 @@ class MBSHandlerTest(unittest.TestCase):
         m.add_changed_file('bash.spec', 1, 1)
         msg = m.produce()
 
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
 
         handler = MBS()
         self.assertTrue(handler.can_handle(event))
@@ -203,7 +199,7 @@ class MBSHandlerTest(unittest.TestCase):
         m.add_changed_file('test.c', 1, 1)
         msg = m.produce()
 
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
 
         handler = MBS()
         self.assertFalse(handler.can_handle(event))
@@ -222,7 +218,7 @@ class MBSHandlerTest(unittest.TestCase):
         m.add_changed_file('bash.spec', 1, 1)
         msg = m.produce()
 
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
 
         mod_info = helpers.PDCModuleInfo('testmodule', 'master', '20170412010101')
         mod_info.add_rpm("bash-1.2.3-4.f26.rpm")
@@ -264,7 +260,7 @@ class MBSHandlerTest(unittest.TestCase):
         m.add_changed_file('bash.spec', 1, 1)
         msg = m.produce()
 
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
 
         mod_info = helpers.PDCModuleInfo('testmodule', 'master', '20170412010101')
         mod_info.add_rpm("bash-1.2.3-4.f26.rpm")
@@ -294,7 +290,7 @@ class MBSHandlerTest(unittest.TestCase):
         # update build state when receive module built messages
         # build is failed
         msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='failed', build_id=123).produce()
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
         handler.handle(event)
         builds = models.ArtifactBuild.query.all()
         self.assertEquals(len(builds), 1)
@@ -304,7 +300,7 @@ class MBSHandlerTest(unittest.TestCase):
         # build is ready
         pdc.get_latest_modules.return_value = []
         msg = helpers.ModuleBuiltMessage('testmodule', 'master', state='ready', build_id=123).produce()
-        event = self._get_event(msg)
+        event = self.get_event_from_msg(msg)
         handler.handle(event)
         builds = models.ArtifactBuild.query.all()
         self.assertEquals(len(builds), 1)

