diff --git a/module_build_service/__init__.py b/module_build_service/__init__.py index 42642216..7a8f20df 100644 --- a/module_build_service/__init__.py +++ b/module_build_service/__init__.py @@ -62,6 +62,7 @@ try: version = pkg_resources.get_distribution('module-build-service').version except pkg_resources.DistributionNotFound: version = 'unknown' +api_version = 2 app = Flask(__name__) app.wsgi_app = ReverseProxy(app.wsgi_app) diff --git a/module_build_service/models.py b/module_build_service/models.py index 6bb87f49..ff16dbda 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -553,17 +553,18 @@ class ModuleBuild(MBSBase): }) return json - def extended_json(self, show_state_url=False): + def extended_json(self, show_state_url=False, api_version=1): """ :kwarg show_state_url: this will determine if `get_url_for` should be run to determine what the `state_url` is. This should be set to `False` when extended_json is called from the backend because it forces an app context to be created, which causes issues with SQLAlchemy sessions. + :kwarg api_version: the API version to use when building the state URL """ json = self.json() state_url = None if show_state_url: - state_url = get_url_for('module_build', id=self.id) + state_url = get_url_for('module_build', api_version=api_version, id=self.id) json.update({ 'component_builds': [build.id for build in self.component_builds], 'build_context': self.build_context, @@ -722,17 +723,18 @@ class ComponentBuild(MBSBase): return retval - def extended_json(self, show_state_url=False): + def extended_json(self, show_state_url=False, api_version=1): """ :kwarg show_state_url: this will determine if `get_url_for` should be run to determine what the `state_url` is. This should be set to `False` when extended_json is called from the backend because it forces an app context to be created, which causes issues with SQLAlchemy sessions. + :kwarg api_version: the API version to use when building the state URL """ json = self.json() state_url = None if show_state_url: - state_url = get_url_for('component_build', id=self.id) + state_url = get_url_for('component_build', api_version=api_version, id=self.id) json.update({ 'state_trace': [{'time': _utc_datetime_to_iso(record.state_time), 'state': record.state, diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 292f3e24..f46a851c 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -38,9 +38,9 @@ from flask import request, url_for, Response from datetime import datetime from sqlalchemy.sql.sqltypes import Boolean as sqlalchemy_boolean -from module_build_service import log, models, Modulemd +from module_build_service import log, models, Modulemd, api_version from module_build_service.errors import (ValidationError, UnprocessableEntity, - ProgrammingError) + ProgrammingError, NotFound) from module_build_service import conf, db from module_build_service.errors import (Forbidden, Conflict) import module_build_service.messaging @@ -359,11 +359,12 @@ def start_next_batch_build(config, module, session, builder, components=None): config, module, session, builder, unbuilt_components_after_reuse) -def pagination_metadata(p_query, request_args): +def pagination_metadata(p_query, api_version, request_args): """ Returns a dictionary containing metadata about the paginated query. This must be run as part of a Flask request. :param p_query: flask_sqlalchemy.Pagination object + :param api_version: an int of the API version :param request_args: a dictionary of the arguments that were part of the Flask request :return: a dictionary containing metadata about the paginated query @@ -386,21 +387,21 @@ def pagination_metadata(p_query, request_args): 'prev': None, 'next': None, 'total': p_query.total, - 'first': url_for(request.endpoint, page=1, per_page=p_query.per_page, - _external=True, **request_args_wo_page), - 'last': url_for(request.endpoint, page=p_query.pages, + 'first': url_for(request.endpoint, api_version=api_version, page=1, + per_page=p_query.per_page, _external=True, **request_args_wo_page), + 'last': url_for(request.endpoint, api_version=api_version, page=p_query.pages, per_page=p_query.per_page, _external=True, **request_args_wo_page) } if p_query.has_prev: - pagination_data['prev'] = url_for(request.endpoint, page=p_query.prev_num, - per_page=p_query.per_page, _external=True, - **request_args_wo_page) + pagination_data['prev'] = url_for(request.endpoint, api_version=api_version, + page=p_query.prev_num, per_page=p_query.per_page, + _external=True, **request_args_wo_page) if p_query.has_next: - pagination_data['next'] = url_for(request.endpoint, page=p_query.next_num, - per_page=p_query.per_page, _external=True, - **request_args_wo_page) + pagination_data['next'] = url_for(request.endpoint, api_version=api_version, + page=p_query.next_num, per_page=p_query.per_page, + _external=True, **request_args_wo_page) return pagination_data @@ -1105,6 +1106,7 @@ def submit_module_build(username, url, mmd, scm, optional_params=None): validate_mmd(mmd) mmds = generate_expanded_mmds(db.session, mmd) + modules = [] for mmd in mmds: log.debug('Checking whether module build already exists: %s.', @@ -1159,9 +1161,10 @@ def submit_module_build(username, url, mmd, scm, optional_params=None): db.session.add(module) db.session.commit() + modules.append(module) log.info("%s submitted build of %s, stream=%s, version=%s, context=%s", username, mmd.get_name(), mmd.get_stream(), mmd.get_version(), mmd.get_context()) - return module + return modules def scm_url_schemes(terse=False): @@ -1798,6 +1801,21 @@ def cors_header(allow='*'): return decorator +def validate_api_version(): + """ + A decorator that validates the requested API version on a route + """ + def decorator(func): + @wraps(func) + def wrapper(*args, **kwargs): + req_api_version = kwargs.get('api_version', 1) + if req_api_version > api_version or req_api_version < 1: + raise NotFound('The requested API version is not available') + return func(*args, **kwargs) + return wrapper + return decorator + + def import_mmd(session, mmd): """ Imports new module build defined by `mmd` to MBS database using `session`. diff --git a/module_build_service/views.py b/module_build_service/views.py index 45b1050a..8ded0df9 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -32,55 +32,55 @@ from flask import request, jsonify, url_for from flask.views import MethodView from builtins import str -from module_build_service import app, conf, log, models, db, version +from module_build_service import app, conf, log, models, db, version, api_version as max_api_version from module_build_service.utils import ( pagination_metadata, filter_module_builds, filter_component_builds, submit_module_build_from_scm, submit_module_build_from_yaml, - get_scm_url_re, cors_header) + get_scm_url_re, cors_header, validate_api_version) from module_build_service.errors import ( ValidationError, Forbidden, NotFound, ProgrammingError) -api_v1 = { +api_routes = { 'module_builds': { - 'url': '/module-build-service/1/module-builds/', + 'url': '/module-build-service//module-builds/', 'options': { 'methods': ['POST'], } }, 'module_builds_list': { - 'url': '/module-build-service/1/module-builds/', + 'url': '/module-build-service//module-builds/', 'options': { 'defaults': {'id': None}, 'methods': ['GET'], } }, 'module_build': { - 'url': '/module-build-service/1/module-builds/', + 'url': '/module-build-service//module-builds/', 'options': { 'methods': ['GET', 'PATCH'], } }, 'component_builds_list': { - 'url': '/module-build-service/1/component-builds/', + 'url': '/module-build-service//component-builds/', 'options': { 'defaults': {'id': None}, 'methods': ['GET'], } }, 'component_build': { - 'url': '/module-build-service/1/component-builds/', + 'url': '/module-build-service//component-builds/', 'options': { 'methods': ['GET'], } }, 'about': { - 'url': '/module-build-service/1/about/', + 'url': '/module-build-service//about/', 'options': { 'methods': ['GET'] } }, 'rebuild_strategies_list': { - 'url': '/module-build-service/1/rebuild-strategies/', + 'url': '/module-build-service//rebuild-strategies/', 'options': { 'methods': ['GET'] } @@ -92,13 +92,14 @@ class AbstractQueryableBuildAPI(MethodView): """ An abstract class, housing some common functionality. """ @cors_header() - def get(self, id): + @validate_api_version() + def get(self, api_version, id): id_flag = request.args.get('id') if id_flag: endpoint = request.endpoint.split('s_list')[0] raise ValidationError( 'The "id" query option is invalid. Did you mean to go to "{0}"?'.format( - url_for(endpoint, id=id_flag))) + url_for(endpoint, api_version=api_version, id=id_flag))) verbose_flag = request.args.get('verbose', 'false').lower() short_flag = request.args.get('short', 'false').lower() json_func_kwargs = {} @@ -107,14 +108,14 @@ class AbstractQueryableBuildAPI(MethodView): if id is None: # Lists all tracked builds p_query = self.query_filter(request) - json_data = { - 'meta': pagination_metadata(p_query, request.args) + 'meta': pagination_metadata(p_query, api_version, request.args) } if verbose_flag == 'true' or verbose_flag == '1': json_func_name = 'extended_json' json_func_kwargs['show_state_url'] = True + json_func_kwargs['api_version'] = api_version elif short_flag == 'true' or short_flag == '1': if hasattr(p_query.items[0], 'short_json'): json_func_name = 'short_json' @@ -129,6 +130,7 @@ class AbstractQueryableBuildAPI(MethodView): if verbose_flag == 'true' or verbose_flag == '1': json_func_name = 'extended_json' json_func_kwargs['show_state_url'] = True + json_func_kwargs['api_version'] = api_version elif short_flag == 'true' or short_flag == '1': if getattr(instance, 'short_json', None): json_func_name = 'short_json' @@ -149,8 +151,8 @@ class ModuleBuildAPI(AbstractQueryableBuildAPI): model = models.ModuleBuild # Additional POST and DELETE handlers for modules follow. - - def post(self): + @validate_api_version() + def post(self, api_version): if "multipart/form-data" in request.headers.get("Content-Type", ""): handler = YAMLFileHandler(request) else: @@ -164,10 +166,16 @@ class ModuleBuildAPI(AbstractQueryableBuildAPI): handler.username, conf.allowed_groups, handler.groups)) handler.validate() - module = handler.post() - return jsonify(module.extended_json(True)), 201 + modules = handler.post() + if api_version == 1: + # Only show the first module build for backwards-compatibility + rv = modules[0].extended_json(True, api_version) + else: + rv = [module.extended_json(True, api_version) for module in modules] + return jsonify(rv), 201 - def patch(self, id): + @validate_api_version() + def patch(self, api_version, id): username, groups = module_build_service.auth.get_user(request) try: @@ -213,13 +221,14 @@ class ModuleBuildAPI(AbstractQueryableBuildAPI): db.session.add(module) db.session.commit() - return jsonify(module.extended_json(True)), 200 + return jsonify(module.extended_json(True, api_version)), 200 class AboutAPI(MethodView): @cors_header() - def get(self): - json = {'version': version} + @validate_api_version() + def get(self, api_version): + json = {'version': version, 'api_version': max_api_version} config_items = ['auth_method'] for item in config_items: config_item = getattr(conf, item) @@ -233,7 +242,8 @@ class AboutAPI(MethodView): class RebuildStrategies(MethodView): @cors_header() - def get(self): + @validate_api_version() + def get(self, api_version): items = [] # Sort the items list by name for strategy in sorted(models.ModuleBuild.rebuild_strategies.keys()): @@ -357,13 +367,13 @@ class YAMLFileHandler(BaseHandler): optional_params=self.optional_params) -def register_api_v1(): - """ Registers version 1 of MBS API. """ +def register_api(): + """ Registers the MBS API. """ module_view = ModuleBuildAPI.as_view('module_builds') component_view = ComponentBuildAPI.as_view('component_builds') about_view = AboutAPI.as_view('about') rebuild_strategies_view = RebuildStrategies.as_view('rebuild_strategies') - for key, val in api_v1.items(): + for key, val in api_routes.items(): if key.startswith('component_build'): app.add_url_rule(val['url'], endpoint=key, @@ -388,4 +398,4 @@ def register_api_v1(): raise NotImplementedError("Unhandled api key.") -register_api_v1() +register_api() diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index ea1c054c..ba21c794 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -29,6 +29,7 @@ from shutil import copyfile from os import path, mkdir from os.path import dirname import hashlib +import pytest from tests import app, init_data from module_build_service.errors import UnprocessableEntity @@ -145,6 +146,14 @@ class TestViews: assert data['rebuild_strategy'] == 'changed-and-after' assert data['version'] == '2' + @pytest.mark.parametrize('api_version', [0, 99]) + def test_query_builds_invalid_api_version(self, api_version): + rv = self.client.get('/module-build-service/{0}/module-builds/'.format(api_version)) + data = json.loads(rv.data) + assert data['error'] == 'Not Found' + assert data['message'] == 'The requested API version is not available' + assert data['status'] == 404 + def test_query_build_short(self): rv = self.client.get('/module-build-service/1/module-builds/2?short=True') data = json.loads(rv.data) @@ -504,17 +513,24 @@ class TestViews: assert data['error'] == 'Bad Request' assert data['message'] == 'An invalid order_by or order_desc_by key was supplied' + @pytest.mark.parametrize('api_version', [1, 2]) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build(self, mocked_scm, mocked_get_user): + def test_submit_build(self, mocked_scm, mocked_get_user, api_version): FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') - rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + post_url = '/module-build-service/{0}/module-builds/'.format(api_version) + rv = self.client.post(post_url, data=json.dumps( {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) + if api_version >= 2: + assert isinstance(data, list) + assert len(data) == 1 + data = data[0] + assert 'component_builds' in data, data assert data['component_builds'] == [] assert data['name'] == 'testmodule' @@ -529,7 +545,7 @@ class TestViews: assert data['id'] == 8 assert data['rebuild_strategy'] == 'changed-and-after' assert data['state_name'] == 'init' - assert data['state_url'] == '/module-build-service/1/module-builds/8' + assert data['state_url'] == '/module-build-service/{0}/module-builds/8'.format(api_version) assert len(data['state_trace']) == 1 assert data['state_trace'][0]['state'] == 0 assert data['tasks'] == {} @@ -912,7 +928,7 @@ class TestViews: rv = self.client.get('/module-build-service/1/about/') data = json.loads(rv.data) assert rv.status_code == 200 - assert data == {'auth_method': 'kerberos', 'version': version} + assert data == {'auth_method': 'kerberos', 'api_version': 2, 'version': version} def test_rebuild_strategy_api(self): rv = self.client.get('/module-build-service/1/rebuild-strategies/')