From d567e4ef1990d3de76b3c024c978f886bb8de6dd Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Dec 21 2018 06:16:48 +0000 Subject: Remove hardcoded subject_types When submitting a waiver (via CLI or via API) you must specify a subject_type from a predefined list. Subject types are growing and evolving, so this shouldn't be hardcoded anymore. --- diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py index f7e28b3..ca14aef 100644 --- a/tests/test_api_v10.py +++ b/tests/test_api_v10.py @@ -757,3 +757,27 @@ def test_create_multiple_waivers_rollback_on_error(mocked_get_user, client, sess # Transaction was rolled back. assert session.query(Waiver).count() == 0 + + +@patch('waiverdb.auth.get_user', return_value=('foo', {})) +def test_create_waiver_with_arbitrary_subject_type(mocked_get_user, client, session): + data = { + 'subject_type': 'kind-of-magic', + 'subject_identifier': 'glibc-2.26-27.fc27', + 'testcase': 'testcase1', + 'product_version': 'fool-1', + 'waived': True, + 'comment': 'it broke', + } + r = client.post('/api/v1.0/waivers/', data=json.dumps(data), + content_type='application/json') + res_data = json.loads(r.get_data(as_text=True)) + assert r.status_code == 201 + assert res_data['username'] == 'foo' + assert res_data['subject'] == {'type': 'kind-of-magic', 'item': 'glibc-2.26-27.fc27'} + assert res_data['subject_type'] == 'kind-of-magic' + assert res_data['subject_identifier'] == 'glibc-2.26-27.fc27' + assert res_data['testcase'] == 'testcase1' + assert res_data['product_version'] == 'fool-1' + assert res_data['waived'] is True + assert res_data['comment'] == 'it broke' diff --git a/tests/test_cli.py b/tests/test_cli.py index 5610beb..ce5226f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -456,20 +456,36 @@ koji_base_url=https://koji.fedoraproject.org/kojihub '-t', 'test.testcase', '-c', "This is fine"] result = runner.invoke(waiverdb_cli, args) assert result.exit_code != 0 - assert 'Error: Please specify correct subject type' in result.output + assert 'Error: Please specify subject_type' in result.output -def test_create_waiver_invalid_subject_type(tmpdir): - p = tmpdir.join('client.conf') - p.write(""" +def test_submit_waiver_with_arbitrary_subject_type(tmpdir): + with patch('requests.request') as mock_request: + mock_rv = Mock() + mock_rv.json.return_value = [{ + "comment": "This is fine", + "id": 15, + "product_version": "Parrot", + "subject_type": "some-kind-of-magic", + "subject_identifier": "setup-2.8.71-7.el7_4", + "testcase": "test.testcase", + "timestamp": "2017-010-16T17:42:04.209638", + "username": "foo", + "waived": True + }] + mock_request.return_value = mock_rv + p = tmpdir.join('client.conf') + p.write(""" [waiverdb] auth_method=dummy api_url=http://localhost:5004/api/v1.0 -koji_base_url=https://koji.fedoraproject.org/kojihub - """) - runner = CliRunner() - args = ['-C', p.strpath, '-T', 'brew-build', '-i', 'setup-2.8.71-7.el7_4', - '-t', 'test.testcase', '-c', "This is fine"] - result = runner.invoke(waiverdb_cli, args) - assert result.exit_code != 0 - assert 'invalid choice: brew-build' in result.output + """) + runner = CliRunner() + args = ['-C', p.strpath, '-p', 'Parrot', + '-s', '{"type": "some-kind-of-magic", "item": "setup-2.8.71-7.el7_4"}', + '-t', 'test.testcase', '-c', "This is fine"] + result = runner.invoke(waiverdb_cli, args) + mock_request.assert_called() + assert result.output == ('Created waiver 15 for result with subject type ' + 'some-kind-of-magic, identifier setup-2.8.71-7.el7_4 and ' + 'testcase test.testcase\n') diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py index 4a71145..771753e 100644 --- a/waiverdb/api_v1.py +++ b/waiverdb/api_v1.py @@ -289,8 +289,7 @@ class WaiversResource(Resource): "proxied_by": null } - :json string subject_type: The type of thing which this waiver is for - ("koji_build", "bodhi_update", "compose"). + :json string subject_type: The type of thing which this waiver is for. :json string subject_identifier: The identifier of the thing this waiver is for. The semantics of this identifier depend on the "subject_type". For example, Koji builds are identified by their NVR. @@ -351,8 +350,8 @@ class WaiversResource(Resource): elif 'type' in result_data and result_data['type'][0] in ['koji_build', 'brew-build']: args['subject_type'] = 'koji_build' args['subject_identifier'] = result_data['item'][0] - elif 'type' in result_data and result_data['type'][0] == 'bodhi_update': - args['subject_type'] = 'bodhi_update' + elif 'type' in result_data: + args['subject_type'] = result_data['type'][0] args['subject_identifier'] = result_data['item'][0] else: raise BadRequest('It is not possible to submit a waiver by ' diff --git a/waiverdb/cli.py b/waiverdb/cli.py index 4a292d4..59b867d 100644 --- a/waiverdb/cli.py +++ b/waiverdb/cli.py @@ -11,8 +11,6 @@ from xmlrpc import client requests_session = requests.Session() -SUBJECT_TYPES = ("koji_build", "bodhi_update", "compose") - class OldJSONSubject(click.ParamType): """ @@ -128,7 +126,7 @@ def guess_product_version(toparse, koji_build=False): 'Subject for a result to waive.')) @click.option('--subject-identifier', '-i', help='Subject identifier for a result to waive.') -@click.option('--subject-type', '-T', type=click.Choice(SUBJECT_TYPES), +@click.option('--subject-type', '-T', help='Subject type for a result to waive.') @click.option('--testcase', '-t', help='Specify a testcase for the subject') @@ -182,9 +180,8 @@ def cli(comment, waived, product_version, testcase, subject, subject_identifier, raise click.ClickException('Please specify subject-identifier') if not result_ids and not testcase: raise click.ClickException('Please specify testcase') - if not result_ids and subject_type not in SUBJECT_TYPES: - raise click.ClickException( - 'Please specify correct subject type {!r}.'.format(SUBJECT_TYPES)) + if not result_ids and not subject_type: + raise click.ClickException('Please specify subject_type') if not product_version and not result_ids: # trying to guess the product_version diff --git a/waiverdb/models/waivers.py b/waiverdb/models/waivers.py index 4150390..e59005e 100644 --- a/waiverdb/models/waivers.py +++ b/waiverdb/models/waivers.py @@ -11,11 +11,8 @@ def subject_dict_to_type_identifier(subject): Now we expect a specific type and identifier. This maps from the old style to the new, for backwards compatibility. """ - if (subject.get('type') == 'bodhi_update' and - 'item' in subject and - isinstance(subject['item'], str)): - return ('bodhi_update', subject['item']) - elif (subject.get('type') in ['koji_build', 'brew-build'] and + # handling the special cases... + if (subject.get('type') in ['koji_build', 'brew-build'] and 'item' in subject and isinstance(subject['item'], str)): return ('koji_build', subject['item']) @@ -23,8 +20,11 @@ def subject_dict_to_type_identifier(subject): return ('koji_build', subject['original_spec_nvr']) elif 'productmd.compose.id' in subject and isinstance(subject['productmd.compose.id'], str): return ('compose', subject['productmd.compose.id']) + # then handling the general case... + elif 'item' in subject and isinstance(subject['item'], str): + return (subject.get('type'), subject['item']) else: - raise ValueError('Unrecognised subject type: %r' % subject) + raise ValueError('Subject type should be non empty string, actual value is: %r' % subject) def subject_type_identifier_to_dict(subject_type, subject_identifier): @@ -32,14 +32,13 @@ def subject_type_identifier_to_dict(subject_type, subject_identifier): Inverse of the above function. This is for backwards compatibility in *responses*. """ - if subject_type == 'bodhi_update': - return {'type': 'bodhi_update', 'item': subject_identifier} - elif subject_type == 'koji_build': - return {'type': 'koji_build', 'item': subject_identifier} - elif subject_type == 'compose': + if subject_type == 'compose': return {'productmd.compose.id': subject_identifier} + elif subject_type and isinstance(subject_type, str): + return {'type': subject_type, 'item': subject_identifier} else: - raise ValueError('Unrecognised subject type: %s' % subject_type) + raise ValueError(('Subject type should be non empty string, ' + 'actual value is: %r') % subject_type) class Waiver(db.Model):