Change cancel build API to be a PATCH instead

module_build_service/models.py

@@ -38,9 +38,6 @@ import module_build_service.messaging
 
 from sqlalchemy.orm import lazyload
 
-from flask import url_for
-
-
 # Just like koji.BUILD_STATES, except our own codes for modules.
 BUILD_STATES = {
     # When you parse the modulemd file and know the nvr and you create a
@@ -239,7 +236,7 @@ class ModuleBuild(RidaBase):
             'state': self.state,
             'state_name': INVERSE_BUILD_STATES[self.state],
             'state_reason': self.state_reason,
-            'state_url': get_url_for('module_build_query', id=self.id),
+            'state_url': get_url_for('module_build', id=self.id),
             'scmurl': self.scmurl,
             'owner': self.owner,
             'time_submitted': self.time_submitted,

module_build_service/views.py

@@ -41,29 +41,23 @@ from module_build_service.errors import (
     ValidationError, Unauthorized, NotFound)
 
 api_v1 = {
-    'module_build_submit': {
+    'module_builds': {
         'url': '/module-build-service/1/module-builds/',
         'options': {
             'methods': ['POST'],
         }
     },
-    'module_build_list': {
+    'module_builds_list': {
         'url': '/module-build-service/1/module-builds/',
         'options': {
             'defaults': {'id': None},
             'methods': ['GET'],
         }
     },
-    'module_build_query': {
+    'module_build': {
         'url': '/module-build-service/1/module-builds/<int:id>',
         'options': {
-            'methods': ['GET'],
-        }
-    },
-    'module_build_cancel': {
-        'url': '/module-build-service/1/module-builds/cancel/<int:id>',
-        'options': {
-            'methods': ['PUT']
+            'methods': ['GET', 'PATCH'],
         }
     },
 }
@@ -132,33 +126,51 @@ class ModuleBuildAPI(MethodView):
         module = submit_module_build(username, url, allow_local_url=False)
         return jsonify(module.json()), 201
 
-    def put(self, id):
+    def patch(self, id):
         username = module_build_service.auth.get_username(request.environ)
 
         if conf.require_packager:
-            module_build_service.auth.assert_is_packager(username, fas_kwargs=dict(
-                base_url=conf.fas_url,
-                username=conf.fas_username,
-                password=conf.fas_password))
-
-        if id is None:
-            raise NotFound('You must provide module build id.')
+            module_build_service.auth.assert_is_packager(
+                username,
+                fas_kwargs=dict(
+                    base_url=conf.fas_url,
+                    username=conf.fas_username,
+                    password=conf.fas_password
+                )
+            )
 
         module = models.ModuleBuild.query.filter_by(id=id).first()
         if not module:
             raise NotFound('No such module found.')
 
         if module.owner != username:
-            raise Unauthorized("You are not owner of this build and "
-                "therefore cannot cancel it.")
+            raise Unauthorized('You are not owner of this build and '
+                               'therefore cannot modify it.')
 
-        module.transition(conf, models.BUILD_STATES["failed"],
-                          "Canceled by %s." % username)
+        try:
+            r = json.loads(request.get_data().decode("utf-8"))
+        except:
+            log.error('Invalid JSON submitted')
+            raise ValidationError('Invalid JSON submitted')
+
+        if not r.get('state'):
+            log.error('Invalid JSON submitted')
+            raise ValidationError('Invalid JSON submitted')
+
+        if r['state'] == 'failed' \
+                or r['state'] == str(models.BUILD_STATES['failed']):
+            module.transition(conf, models.BUILD_STATES["failed"],
+                              "Canceled by %s." % username)
+        else:
+            log.error('The provided state change of "{}" is not supported'
+                      .format(r['state']))
+            raise ValidationError('The provided state change is not supported')
         db.session.add(module)
         db.session.commit()
 
         return jsonify(module.api_json()), 200
 
+
 def register_api_v1():
     """ Registers version 1 of Rida API. """
     module_view = ModuleBuildAPI.as_view('module_builds')

tests/test_build/test_build.py

@@ -246,7 +246,9 @@ class TestBuild(unittest.TestCase):
         # method. We just cancel the build here using the web API to simulate
         # user cancelling the build in the middle of building.
         def on_build_cb(cls, artifact_name, source):
-            self.client.put('/module-build-service/1/module-builds/cancel/' + str(module_build_id))
+            self.client.patch(
+                '/module-build-service/1/module-builds/' + str(module_build_id),
+                data=json.dumps({'state': 'failed'}))
 
         cancelled_tasks = []
         def on_cancel_cb(cls, task_id):

tests/test_views/test_views.py

@@ -405,7 +405,8 @@ class TestViews(unittest.TestCase):
     @patch('module_build_service.auth.assert_is_packager')
     def test_cancel_build(self, mocked_assert_is_packager,
                           mocked_get_username):
-        rv = self.client.put('/module-build-service/1/module-builds/cancel/30')
+        rv = self.client.patch('/module-build-service/1/module-builds/30',
+                               data=json.dumps({'state': 'failed'}))
         data = json.loads(rv.data)
 
         self.assertEquals(data['state'], 4)
@@ -416,8 +417,35 @@ class TestViews(unittest.TestCase):
     @patch('module_build_service.auth.assert_is_packager')
     def test_cancel_build_unauthorized(self, mocked_assert_is_packager,
                           mocked_get_username):
-        rv = self.client.put('/module-build-service/1/module-builds/cancel/30')
+        rv = self.client.patch('/module-build-service/1/module-builds/30',
+                               data=json.dumps({'state': 'failed'}))
         data = json.loads(rv.data)
 
         self.assertEquals(data['status'], 401)
         self.assertEquals(data['error'], 'Unauthorized')
+
+    @patch('module_build_service.auth.get_username', return_value='some_other_user')
+    @patch('module_build_service.auth.assert_is_packager')
+    def test_cancel_build_wrong_param(self, mocked_assert_is_packager,
+                                      mocked_get_username):
+        rv = self.client.patch('/module-build-service/1/module-builds/30',
+                               data=json.dumps({'some_param': 'value'}))
+        data = json.loads(rv.data)
+
+        self.assertEquals(data['status'], 400)
+        self.assertEquals(data['error'], 'Bad Request')
+        self.assertEquals(
+            data['message'], 'Invalid JSON submitted')
+
+    @patch('module_build_service.auth.get_username', return_value='some_other_user')
+    @patch('module_build_service.auth.assert_is_packager')
+    def test_cancel_build_wrong_state(self, mocked_assert_is_packager,
+                          mocked_get_username):
+        rv = self.client.patch('/module-build-service/1/module-builds/30',
+                               data=json.dumps({'state': 'some_state'}))
+        data = json.loads(rv.data)
+
+        self.assertEquals(data['status'], 400)
+        self.assertEquals(data['error'], 'Bad Request')
+        self.assertEquals(
+            data['message'], 'The provided state change is not supported')