From d567e4ef1990d3de76b3c024c978f886bb8de6dd Mon Sep 17 00:00:00 2001
From: Giulia Naponiello <gnaponie@redhat.com>
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):

