From 3f16d018eb3c2b125eac5beecf4a03bd127b562a Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Dec 08 2017 04:03:07 +0000
Subject: [PATCH 1/2] Mark ErrataAdvisoryRPMsSignedEvent COMPLETE


When all builds are in FAILED or DONE, event
ErrataAdvisoryRPMsSignedEvent will be marked as COMPLETE.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/freshmaker/handlers/brew/container_task_state_change.py b/freshmaker/handlers/brew/container_task_state_change.py
index 79b2987..7b5c0d2 100644
--- a/freshmaker/handlers/brew/container_task_state_change.py
+++ b/freshmaker/handlers/brew/container_task_state_change.py
@@ -25,7 +25,7 @@ from freshmaker.events import BrewContainerTaskStateChangeEvent
 from freshmaker.models import ArtifactBuild
 from freshmaker.handlers import (
     ContainerBuildHandler, fail_event_on_handler_exception)
-from freshmaker.types import ArtifactType, ArtifactBuildState
+from freshmaker.types import ArtifactType, ArtifactBuildState, EventState
 
 
 class BrewContainerTaskStateChangeHandler(ContainerBuildHandler):
@@ -73,3 +73,29 @@ class BrewContainerTaskStateChangeHandler(ContainerBuildHandler):
                     build.build_id = self.build_image_artifact_build(build, repo_urls)
                     build.state = ArtifactBuildState.BUILD.value
                 db.session.commit()
+
+            # Finally, we check if all builds scheduled by event
+            # found_build.event (ErrataAdvisoryRPMsSignedEvent) have been
+            # switched to FAILED or COMPLETE. If yes, mark the event COMPLETE.
+            self._mark_event_complete_when_all_builds_done(found_build.event)
+
+    def _mark_event_complete_when_all_builds_done(self, db_event):
+        """Mark ErrataAdvisoryRPMsSignedEvent COMPLETE
+
+        As we know that docker images are scheduled to be rebuilt by hanlding
+        event ErrataAdvisoryRPMsSignedEvent. When all those builds are done,
+        the event should be marked as COMPLETE accordingly. If not all finish,
+        nothing change to the state.
+
+        :param Event db_event: instance of Event that represents an event
+            ErrataAdvisoryRPMsSignedEvent.
+        """
+        build_complete_states = (
+            ArtifactBuildState.FAILED.value,
+            ArtifactBuildState.DONE.value
+        )
+        all_builds_done = all((build.state in build_complete_states
+                               for build in db_event.builds))
+        if all_builds_done:
+            db_event.transition(
+                EventState.COMPLETE, 'All docker images have been rebuilt.')
diff --git a/tests/test_brew_container_task_state_change_handler.py b/tests/test_brew_container_task_state_change_handler.py
index 51310e3..68b1782 100644
--- a/tests/test_brew_container_task_state_change_handler.py
+++ b/tests/test_brew_container_task_state_change_handler.py
@@ -30,7 +30,7 @@ from tests import get_fedmsg, helpers
 from freshmaker import db, events, models
 from freshmaker.parsers.brew import BrewTaskStateChangeParser
 from freshmaker.handlers.brew import BrewContainerTaskStateChangeHandler
-from freshmaker.types import ArtifactType, ArtifactBuildState
+from freshmaker.types import ArtifactType, ArtifactBuildState, EventState
 
 
 class TestBrewContainerTaskStateChangeHandler(helpers.FreshmakerTestCase):
@@ -115,6 +115,92 @@ class TestBrewContainerTaskStateChangeHandler(helpers.FreshmakerTestCase):
         self.assertEqual(base_build.state, ArtifactBuildState.FAILED.value)
         build_image.assert_not_called()
 
+    @mock.patch('freshmaker.models.messaging.publish')
+    def test_mark_event_COMPLETE_if_all_builds_done(self, publish):
+        self.db_advisory_rpm_signed_event = models.Event.create(
+            db.session, 'msg-id-123', '12345',
+            events.ErrataAdvisoryStateChangedEvent,
+            state=EventState.BUILDING.value)
+
+        self.image_a_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-a-0.1-1', ArtifactType.IMAGE,
+            state=ArtifactBuildState.DONE.value)
+
+        self.image_b_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-b-0.1-1', ArtifactType.IMAGE,
+            dep_on=self.image_a_build,
+            state=ArtifactBuildState.DONE.value)
+
+        self.image_c_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-c-0.1-1', ArtifactType.IMAGE,
+            dep_on=self.image_b_build,
+            state=ArtifactBuildState.FAILED.value)
+
+        self.image_d_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-d-0.1-1', ArtifactType.IMAGE,
+            dep_on=self.image_a_build,
+            build_id=12345,
+            state=ArtifactBuildState.BUILD.value)
+
+        db.session.commit()
+
+        state_changed_event = events.BrewContainerTaskStateChangeEvent(
+            'msg-id-890', 'image-d', 'branch', 'target', 12345,
+            'BUILD', 'CLOSED')
+
+        handler = BrewContainerTaskStateChangeHandler()
+        handler.handle(state_changed_event)
+
+        self.assertEqual(EventState.COMPLETE.value,
+                         self.db_advisory_rpm_signed_event.state)
+
+    @mock.patch('freshmaker.handlers.ContainerBuildHandler.build_image_artifact_build')
+    @mock.patch('freshmaker.handlers.ContainerBuildHandler.get_repo_urls')
+    def test_not_change_state_if_not_all_builds_done(
+            self, get_repo_urls, build_image_artifact_build):
+        build_image_artifact_build.return_value = 67890
+
+        self.db_advisory_rpm_signed_event = models.Event.create(
+            db.session, 'msg-id-123', '12345',
+            events.ErrataAdvisoryStateChangedEvent,
+            state=EventState.BUILDING.value)
+
+        self.image_a_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-a-0.1-1', ArtifactType.IMAGE,
+            build_id=12345,
+            state=ArtifactBuildState.BUILD.value)
+
+        self.image_b_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-b-0.1-1', ArtifactType.IMAGE,
+            dep_on=self.image_a_build,
+            state=ArtifactBuildState.PLANNED.value)
+
+        self.image_c_build = models.ArtifactBuild.create(
+            db.session, self.db_advisory_rpm_signed_event,
+            'image-c-0.1-1', ArtifactType.IMAGE,
+            dep_on=self.image_b_build,
+            state=ArtifactBuildState.FAILED.value)
+
+        db.session.commit()
+
+        state_changed_event = events.BrewContainerTaskStateChangeEvent(
+            'msg-id-890', 'image-a', 'branch', 'target', 12345,
+            'BUILD', 'CLOSED')
+
+        handler = BrewContainerTaskStateChangeHandler()
+        handler.handle(state_changed_event)
+
+        # As self.image_b_build starts to be rebuilt, not all images are
+        # rebuilt yet.
+        self.assertEqual(EventState.BUILDING.value,
+                         self.db_advisory_rpm_signed_event.state)
+
 
 if __name__ == '__main__':
     unittest.main()

From 782b5b177d49766350527fcba40191428644ca16 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Dec 08 2017 04:22:58 +0000
Subject: [PATCH 2/2] Use SQLAlchemy false() instead of comparing with False


Original code ``cls.released == False`` was detected by flake8 as an
error E712: Comparison to False should be "if cond is False:" or "if not
cond:".

Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/freshmaker/models.py b/freshmaker/models.py
index d5edc94..ca567b6 100644
--- a/freshmaker/models.py
+++ b/freshmaker/models.py
@@ -28,6 +28,7 @@ import json
 
 from datetime import datetime
 from sqlalchemy.orm import (validates, relationship)
+from sqlalchemy.sql.expression import false
 
 from flask_login import UserMixin
 
@@ -222,7 +223,7 @@ class Event(FreshmakerBase):
                 state.value if isinstance(state, EventState) else state for
                 state in states
             ]
-        return session.query(cls).filter(cls.released == False,
+        return session.query(cls).filter(cls.released == false(),
                                          cls.state.in_(states)).all()
 
     @property

