diff --git a/module_build_service/models.py b/module_build_service/models.py index 1c17e065..905d332d 100644 --- a/module_build_service/models.py +++ b/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, diff --git a/module_build_service/views.py b/module_build_service/views.py index c710ad1e..1463e4ed 100644 --- a/module_build_service/views.py +++ b/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/', 'options': { - 'methods': ['GET'], - } - }, - 'module_build_cancel': { - 'url': '/module-build-service/1/module-builds/cancel/', - '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') diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 94145546..734168f7 100644 --- a/tests/test_build/test_build.py +++ b/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): diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index f9fa0c90..2e8142a3 100644 --- a/tests/test_views/test_views.py +++ b/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')