From 1cde22e76c79a4f017a0b5bb07a1eed86a83be1c Mon Sep 17 00:00:00 2001
From: Charlton Austin <charlton.austin@gmail.com>
Date: Fri, 21 Oct 2016 10:05:17 -0400
Subject: [PATCH] Making some refactors to make it easier to cancel the build
 at any time.

---
 buildman/jobutil/buildstatus.py |  5 +--
 data/model/build.py             | 43 ++++++++++++++++++++------
 test/test_queries.py            | 54 +++++++++++++++++++++++++++++++--
 3 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/buildman/jobutil/buildstatus.py b/buildman/jobutil/buildstatus.py
index 0b65f7934..d7b1af1bc 100644
--- a/buildman/jobutil/buildstatus.py
+++ b/buildman/jobutil/buildstatus.py
@@ -63,10 +63,7 @@ class StatusHandler(object):
     self._append_log_message(phase, self._build_logs.PHASE, extra_data)
 
     # Update the repository build with the new phase
-    repo_build = model.build.get_repository_build(self._uuid)
-    repo_build.phase = phase
-    repo_build.save()
-    return True
+    return model.build.update_phase(self._uuid, phase)
 
   def __enter__(self):
     return self._status
diff --git a/data/model/build.py b/data/model/build.py
index af6c9e846..5a43859ee 100644
--- a/data/model/build.py
+++ b/data/model/build.py
@@ -143,24 +143,47 @@ def get_pull_robot_name(trigger):
   return trigger.pull_robot.username
 
 
-def cancel_repository_build(build, work_queue):
+def update_phase(build_uuid, phase):
+  """ A function to change the phase of a build """
+  with db_transaction():
+    try:
+      build = get_repository_build(build_uuid)
+      build.phase = phase
+      build.save()
+      return True
+    except InvalidRepositoryBuildException:
+      return False
+
+
+def create_cancel_build_in_queue(build, build_queue):
+  """ A function to cancel a build before it leaves the queue """
+  def cancel_build():
+    if build.phase != BUILD_PHASE.WAITING or not build.queue_id:
+      return False
+
+    return build_queue.cancel(build.queue_id)
+
+  return cancel_build
+
+
+def cancel_repository_build(build, build_queue):
+  """ This tries to cancel the build returns true if request is successful false if it can't be cancelled """
   with db_transaction():
     # Reload the build for update.
+    # We are loading the build for update so checks should be as quick as possible.
     try:
       build = db_for_update(RepositoryBuild.select().where(RepositoryBuild.id == build.id)).get()
     except RepositoryBuild.DoesNotExist:
       return False
 
-    if build.phase != BUILD_PHASE.WAITING or not build.queue_id:
-      return False
+    cancel_builds = [create_cancel_build_in_queue(build, build_queue), ]
+    for cancelled in cancel_builds:
+      if cancelled():
+        # Delete the build row.
+        build.delete_instance()
+        return True
 
-    # Try to cancel the queue item.
-    if not work_queue.cancel(build.queue_id):
-      return False
-
-    # Delete the build row.
-    build.delete_instance()
-    return True
+    return False
 
 
 def get_archivable_build():
diff --git a/test/test_queries.py b/test/test_queries.py
index 17c383028..6c4367e30 100644
--- a/test/test_queries.py
+++ b/test/test_queries.py
@@ -1,13 +1,16 @@
 import unittest
+from unittest import TestCase
 
 from app import app
+from data.model.build import get_repository_build, update_phase, create_repository_build
 from initdb import setup_database_for_testing, finished_database_for_testing
 from data import model
-from data.database import RepositoryBuild, Repository, Image, ImageStorage
+from data.database import RepositoryBuild, Image, ImageStorage, BUILD_PHASE
 
 ADMIN_ACCESS_USER = 'devtable'
 SIMPLE_REPO = 'simple'
 
+
 class TestSpecificQueries(unittest.TestCase):
   def setUp(self):
     setup_database_for_testing(self)
@@ -63,6 +66,53 @@ class TestSpecificQueries(unittest.TestCase):
     self.fail('Expected BlobDoesNotExist exception')
 
 
+class TestUpdatePhase(unittest.TestCase):
+  def setUp(self):
+    setup_database_for_testing(self)
+    self.app = app.test_client()
+    self.ctx = app.test_request_context()
+    self.ctx.__enter__()
+
+  def tearDown(self):
+    finished_database_for_testing(self)
+    self.ctx.__exit__(True, None, None)
+
+  def testUpdatePhase(self):
+    build = self.create_build(model.repository.get_repository("devtable", "building"))
+
+    repo_build = get_repository_build(build.uuid)
+
+    self.assertEqual(repo_build.phase, BUILD_PHASE.WAITING)
+    self.assertTrue(update_phase(build.uuid, BUILD_PHASE.COMPLETE))
+
+    repo_build = get_repository_build(build.uuid)
+
+    self.assertEqual(repo_build.phase, BUILD_PHASE.COMPLETE)
+
+    repo_build.delete_instance()
+
+    self.assertFalse(update_phase(repo_build.uuid, BUILD_PHASE.PULLING))
+
+  @staticmethod
+  def create_build(repository):
+    new_token = model.token.create_access_token(repository, 'write', 'build-worker')
+    repo = 'ci.devtable.com:5000/%s/%s' % (repository.namespace_user.username, repository.name)
+    job_config = {
+      'repository': repo,
+      'docker_tags': ['latest'],
+      'build_subdir': '',
+      'trigger_metadata': {
+        'commit': '3482adc5822c498e8f7db2e361e8d57b3d77ddd9',
+        'ref': 'refs/heads/master',
+        'default_branch': 'master'
+      }
+    }
+    build = create_repository_build(repository, new_token, job_config,
+                                    '68daeebd-a5b9-457f-80a0-4363b882f8ea',
+                                    "build_name")
+    build.save()
+    return build
+
 
 if __name__ == '__main__':
-  unittest.main()
\ No newline at end of file
+  unittest.main()