From 0339d4a327025329a0e144ba61026eae0766eec3 Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 04:32:58 +0000
Subject: [PATCH 1/7] tests: use realistic values for "subject"


---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index eb8441a..a5cdf06 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -15,7 +15,7 @@ from waiverdb.models import Waiver
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_waiver(mocked_get_user, client, session):
     data = {
-        'subject': {'subject.test': 'subject'},
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
@@ -26,7 +26,7 @@ def test_create_waiver(mocked_get_user, client, session):
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 201
     assert res_data['username'] == 'foo'
-    assert res_data['subject'] == {'subject.test': 'subject'}
+    assert res_data['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
     assert res_data['testcase'] == 'testcase1'
     assert res_data['product_version'] == 'fool-1'
     assert res_data['waived'] is True
@@ -134,7 +134,7 @@ def test_create_waiver_with_unknown_result_id(mocked_get_user, mocked_resultsdb,
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_waiver_with_no_testcase(mocked_get_user, client):
     data = {
-        'subject': {'foo': 'bar'},
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'waived': True,
         'product_version': 'the-best',
     }
@@ -164,7 +164,7 @@ def test_create_waiver_with_malformed_subject(mocked_get_user, client):
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_non_superuser_cannot_create_waiver_for_other_users(mocked_get_user, client):
     data = {
-        'subject': {'subject.test': 'subject'},
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
@@ -181,7 +181,7 @@ def test_non_superuser_cannot_create_waiver_for_other_users(mocked_get_user, cli
 @patch('waiverdb.auth.get_user', return_value=('bodhi', {}))
 def test_superuser_can_create_waiver_for_other_users(mocked_get_user, client, session):
     data = {
-        'subject': {'subject.test': 'subject'},
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
@@ -199,7 +199,7 @@ def test_superuser_can_create_waiver_for_other_users(mocked_get_user, client, se
 
 def test_get_waiver(client, session):
     # create a new waiver
-    waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                            testcase='testcase1', username='foo',
                            product_version='foo-1', comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/%s' % waiver.id)
@@ -233,7 +233,7 @@ def test_500(mocked, client, session):
 
 def test_get_waivers(client, session):
     for i in range(0, 10):
-        create_waiver(session, subject={"subject%d" % i: "%d" % i},
+        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/')
@@ -244,7 +244,7 @@ def test_get_waivers(client, session):
 
 def test_pagination_waivers(client, session):
     for i in range(0, 30):
-        create_waiver(session, subject={"subject%d" % i: "%d" % i},
+        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/?page=2')
@@ -258,10 +258,10 @@ def test_pagination_waivers(client, session):
 
 
 def test_obsolete_waivers_are_excluded_by_default(client, session):
-    create_waiver(session, subject={'subject.test': 'subject'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo',
                   product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='foo',
                                product_version='foo-1', waived=False)
     r = client.get('/api/v1.0/waivers/')
@@ -273,10 +273,10 @@ def test_obsolete_waivers_are_excluded_by_default(client, session):
 
 
 def test_get_obsolete_waivers(client, session):
-    old_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    old_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='foo',
                                product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='foo',
                                product_version='foo-1', waived=False)
     r = client.get('/api/v1.0/waivers/?include_obsolete=1')
@@ -288,10 +288,10 @@ def test_get_obsolete_waivers(client, session):
 
 
 def test_obsolete_waivers_with_different_product_version(client, session):
-    old_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    old_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='foo',
                                product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='foo',
                                product_version='foo-2')
     r = client.get('/api/v1.0/waivers/?include_obsolete=0')
@@ -303,10 +303,10 @@ def test_obsolete_waivers_with_different_product_version(client, session):
 
 
 def test_obsolete_waivers_with_different_username(client, session):
-    old_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    old_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='foo',
                                product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'subject.test': 'subject'},
+    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                                testcase='testcase1', username='bar',
                                product_version='foo-1')
     r = client.get('/api/v1.0/waivers/?include_obsolete=0')
@@ -318,16 +318,16 @@ def test_obsolete_waivers_with_different_username(client, session):
 
 
 def test_filtering_waivers_by_subject(client, session):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase', username='foo-1', product_version='foo-1')
-    create_waiver(session, subject={'subject.test2': 'subject2'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
                   testcase='testcase', username='foo-2', product_version='foo-1')
 
-    r = client.get('/api/v1.0/waivers/?subject=%s' % json.dumps({'subject.test1': 'subject1'}))
+    r = client.get('/api/v1.0/waivers/?subject=%s' % json.dumps({'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}))
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'subject.test1': 'subject1'}
+    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
 
 
 def test_filtering_waivers_with_invalid_json_subject(client, session):
@@ -339,9 +339,9 @@ def test_filtering_waivers_with_invalid_json_subject(client, session):
 
 
 def test_filtering_waivers_by_testcase(client, session):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo-1', product_version='foo-1')
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase2', username='foo-2', product_version='foo-1')
 
     r = client.get('/api/v1.0/waivers/?testcase=testcase1')
@@ -352,9 +352,9 @@ def test_filtering_waivers_by_testcase(client, session):
 
 
 def test_filtering_waivers_by_product_version(client, session):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo-1', product_version='release-1')
-    create_waiver(session, subject={'subject.test2': 'subject2'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
                   testcase='testcase2', username='foo-1', product_version='release-2')
     r = client.get('/api/v1.0/waivers/?product_version=release-1')
     res_data = json.loads(r.get_data(as_text=True))
@@ -364,9 +364,9 @@ def test_filtering_waivers_by_product_version(client, session):
 
 
 def test_filtering_waivers_by_username(client, session):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo', product_version='foo-1')
-    create_waiver(session, subject={'subject.test2': 'subject2'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
                   testcase='testcase2', username='bar', product_version='foo-2')
     r = client.get('/api/v1.0/waivers/?username=foo')
     res_data = json.loads(r.get_data(as_text=True))
@@ -379,20 +379,20 @@ def test_filtering_waivers_by_since(client, session):
     before1 = (datetime.datetime.utcnow() - datetime.timedelta(seconds=100)).isoformat()
     before2 = (datetime.datetime.utcnow() - datetime.timedelta(seconds=99)).isoformat()
     after = (datetime.datetime.utcnow() + datetime.timedelta(seconds=100)).isoformat()
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo', product_version='foo-1')
     r = client.get('/api/v1.0/waivers/?since=%s' % before1)
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'subject.test1': 'subject1'}
+    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
     assert res_data['data'][0]['testcase'] == 'testcase1'
 
     r = client.get('/api/v1.0/waivers/?since=%s,%s' % (before1, after))
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'subject.test1': 'subject1'}
+    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
     assert res_data['data'][0]['testcase'] == 'testcase1'
 
     r = client.get('/api/v1.0/waivers/?since=%s' % (after))
@@ -428,21 +428,21 @@ def test_filtering_waivers_by_malformed_since(client, session):
 
 
 def test_filtering_waivers_by_proxied_by(client, session):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo-1', product_version='foo-1',
                   proxied_by='bodhi')
-    create_waiver(session, subject={'subject.test2': 'subject2'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
                   testcase='testcase2', username='foo-2', product_version='foo-1')
     r = client.get('/api/v1.0/waivers/?proxied_by=bodhi')
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'subject.test1': 'subject1'}
+    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
     assert res_data['data'][0]['testcase'] == 'testcase1'
 
 
 def test_jsonp(client, session):
-    waiver = create_waiver(session, subject={'subject.test1': 'subject1'},
+    waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                            testcase='testcase1', username='foo', product_version='foo-1')
     r = client.get('/api/v1.0/waivers/%s?callback=jsonpcallback' % waiver.id)
     assert r.mimetype == 'application/javascript'
@@ -462,9 +462,9 @@ def test_get_waivers_with_post_request(client, session):
     """
     results = []
     for i in range(1, 51):
-        results.append({'subject': {'subject%d' % i: '%d' % i},
+        results.append({'subject': {'type': 'koji_build', 'item': '%d' % i},
                         'testcase': 'case %d' % i})
-        create_waiver(session, subject={"subject%d" % i: "%d" % i},
+        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     data = {
@@ -478,7 +478,7 @@ def test_get_waivers_with_post_request(client, session):
     subjects = []
     testcases = []
     for i in reversed(range(1, 51)):
-        subjects.append({'subject%d' % i: '%d' % i})
+        subjects.append({'type': 'koji_build', 'item': '%d' % i})
         testcases.append('case %d' % i)
     assert [w['subject'] for w in res_data['data']] == subjects
     assert set([w['testcase'] for w in res_data['data']]) == set(testcases)
@@ -505,7 +505,7 @@ def test_filtering_waivers_with_bad_key(client, session, results):
     [{}],
 ])
 def test_filtering_waivers_with_empty_results(client, session, results):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo-1', product_version='foo-1')
     data = {'results': results}
     r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
@@ -516,7 +516,7 @@ def test_filtering_waivers_with_empty_results(client, session, results):
 
 
 def test_get_waivers_with_post_malformed_since(client, session):
-    create_waiver(session, subject={'subject.test1': 'subject1'},
+    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
                   testcase='testcase1', username='foo', product_version='foo-1')
     data = {
         'since': 123
@@ -567,7 +567,7 @@ def test_no_cors_about(client, session):
 @pytest.mark.usefixtures('enable_cors')
 def test_cors_waivers(client, session):
     for i in range(0, 3):
-        create_waiver(session, subject={"subject%d" % i: "%d" % i},
+        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/')
@@ -586,7 +586,7 @@ def test_cors_waivers(client, session):
 
 def test_no_cors_waivers(client, session):
     for i in range(0, 3):
-        create_waiver(session, subject={"subject%d" % i: "%d" % i},
+        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/')
@@ -603,14 +603,14 @@ def test_no_cors_waivers(client, session):
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_multiple_waivers(mocked_get_user, client, session):
     item1 = {
-        'subject': {'subject.test': 'subject1'},
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
         'comment': 'it broke',
     }
     item2 = {
-        'subject': {'subject.test': 'subject2'},
+        'subject': {'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
         'testcase': 'testcase2',
         'product_version': 'fool-2',
         'waived': False,
@@ -638,7 +638,7 @@ def test_create_multiple_waivers(mocked_get_user, client, session):
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_multiple_waivers_rollback_on_error(mocked_get_user, client, session):
     item1 = {
-        'subject': {'subject.test': 'subject1'},
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
diff --git a/tests/test_auth.py b/tests/test_auth.py
index 918be59..ad479b1 100644
--- a/tests/test_auth.py
+++ b/tests/test_auth.py
@@ -28,7 +28,7 @@ class TestGSSAPIAuthentication(object):
     def test_authorized(self, client, monkeypatch):
         monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')
         data = {
-            'subject': {'subject.test': 'subject'},
+            'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
             'testcase': 'testcase1',
             'product_version': 'fool-1',
             'waived': True,
diff --git a/tests/test_events.py b/tests/test_events.py
index 2b201bd..0a64414 100644
--- a/tests/test_events.py
+++ b/tests/test_events.py
@@ -9,7 +9,7 @@ from waiverdb.models import Waiver
 @mock.patch('waiverdb.events.fedmsg')
 def test_publish_new_waiver_with_fedmsg(mock_fedmsg, session):
     waiver = Waiver(
-        subject={'subject.test': 'subject'},
+        subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         testcase='testcase1',
         username='jcline',
         product_version='something',
@@ -23,7 +23,7 @@ def test_publish_new_waiver_with_fedmsg(mock_fedmsg, session):
         topic='waiver.new',
         msg={
             'id': waiver.id,
-            'subject': {'subject.test': 'subject'},
+            'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
             'testcase': 'testcase1',
             'username': 'jcline',
             'proxied_by': None,
@@ -38,7 +38,7 @@ def test_publish_new_waiver_with_fedmsg(mock_fedmsg, session):
 @mock.patch('waiverdb.events.fedmsg')
 def test_publish_new_waiver_with_fedmsg_for_proxy_user(mock_fedmsg, session):
     waiver = Waiver(
-        subject={'subject.test': 'subject'},
+        subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         testcase='testcase1',
         username='jcline',
         product_version='something',
@@ -53,7 +53,7 @@ def test_publish_new_waiver_with_fedmsg_for_proxy_user(mock_fedmsg, session):
         topic='waiver.new',
         msg={
             'id': waiver.id,
-            'subject': {'subject.test': 'subject'},
+            'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
             'testcase': 'testcase1',
             'username': 'jcline',
             'proxied_by': 'bodhi',

From ba38f181582f4d2bf3ce6ebd2135aaf140b07b30 Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 04:33:01 +0000
Subject: [PATCH 2/7] also use RequestParser for POST /api/v1.0/waivers/+by-subjects-and-testcases


This way the error handling matches the other HTTP endpoints.

---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index a5cdf06..0a06869 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -496,8 +496,8 @@ def test_filtering_waivers_with_bad_key(client, session, results):
                     content_type='application/json')
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 400
-    assert "'results' parameter should be a list of dictionaries with subject and testcase" \
-        in res_data.get('message')
+    assert res_data['message']['results'] == \
+        'Must be a list of dictionaries with "subject" and "testcase"'
 
 
 @pytest.mark.parametrize("results", [
@@ -516,16 +516,20 @@ def test_filtering_waivers_with_empty_results(client, session, results):
 
 
 def test_get_waivers_with_post_malformed_since(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
-                  testcase='testcase1', username='foo', product_version='foo-1')
-    data = {
-        'since': 123
-    }
+    data = {'since': 123}
+    r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
+                    content_type='application/json')
+    res_data = json.loads(r.get_data(as_text=True))
+    assert r.status_code == 400
+    assert res_data['message']['since'] == "argument of type 'int' is not iterable"
+
+    data = {'since': 'asdf'}
     r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
                     content_type='application/json')
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 400
-    assert res_data['message'] == "'since' parameter not in ISO8601 format"
+    assert res_data['message']['since'] == \
+        "time data 'asdf' does not match format '%Y-%m-%dT%H:%M:%S.%f'"
 
 
 def test_about_endpoint(client):
diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py
index 745679f..3e666ee 100644
--- a/waiverdb/api_v1.py
+++ b/waiverdb/api_v1.py
@@ -6,7 +6,7 @@ import datetime
 import requests
 from flask import Blueprint, request, current_app
 from flask_restful import Resource, Api, reqparse, marshal_with, marshal
-from werkzeug.exceptions import BadRequest, UnsupportedMediaType, Forbidden, ServiceUnavailable
+from werkzeug.exceptions import BadRequest, Forbidden, ServiceUnavailable
 from sqlalchemy.sql.expression import func, cast
 
 from waiverdb import __version__
@@ -45,7 +45,7 @@ def get_resultsdb_result(result_id):
     return response.json()
 
 
-def _validate_results_filter(results):
+def valid_results_list(results):
     expected = {
         'subject': dict,
         'testcase': str,
@@ -53,9 +53,8 @@ def _validate_results_filter(results):
     for item in results:
         for k, v in item.items():
             if not (k in expected and isinstance(v, expected[k])):
-                raise BadRequest(
-                    ("'results' parameter should be a list of dictionaries with"
-                     " subject and testcase"))
+                raise ValueError('Must be a list of dictionaries with "subject" and "testcase"')
+    return results
 
 
 def reqparse_since(since):
@@ -126,6 +125,15 @@ RP['get_waivers'].add_argument('page', default=1, type=int, location='args')
 RP['get_waivers'].add_argument('limit', default=10, type=int, location='args')
 RP['get_waivers'].add_argument('proxied_by', location='args')
 
+RP['get_waivers_by_subjects_and_testcase'] = rp = reqparse.RequestParser()
+rp.add_argument('results', type=valid_results_list, location='json')
+rp.add_argument('testcase', type=str, location='json')
+rp.add_argument('product_version', type=str, location='json')
+rp.add_argument('username', type=str, location='json')
+rp.add_argument('proxied_by', location='json')
+rp.add_argument('since', type=reqparse_since, location='json')
+rp.add_argument('include_obsolete', type=bool, default=False, location='json')
+
 
 class DummyJsonRequest(object):
     """
@@ -456,32 +464,23 @@ class GetWaiversBySubjectsAndTestcases(Resource):
         :statuscode 200: If the query was valid and no problems were encountered.
             Note that the response may still contain 0 waivers.
         """
-        if not request.get_json():
-            raise UnsupportedMediaType('No JSON payload in request')
-        data = request.get_json()
+        args = RP['get_waivers_by_subjects_and_testcase'].parse_args()
         query = Waiver.query.order_by(Waiver.timestamp.desc())
-        if 'results' in data:
-            results = data['results']
-            _validate_results_filter(results)
-            query = Waiver.by_results(query, results)
-        if 'product_version' in data:
-            query = query.filter(Waiver.product_version == data['product_version'])
-        if 'username' in data:
-            query = query.filter(Waiver.username == data['username'])
-        if 'proxied_by' in data:
-            query = query.filter(Waiver.proxied_by == data['proxied_by'])
-        if 'since' in data:
-            try:
-                since_start, since_end = reqparse_since(data['since'])
-            except ValueError:
-                raise BadRequest("'since' parameter not in ISO8601 format")
-            except TypeError:
-                raise BadRequest("'since' parameter not in ISO8601 format")
+        if args['results']:
+            query = Waiver.by_results(query, args['results'])
+        if args['product_version']:
+            query = query.filter(Waiver.product_version == args['product_version'])
+        if args['username']:
+            query = query.filter(Waiver.username == args['username'])
+        if args['proxied_by']:
+            query = query.filter(Waiver.proxied_by == args['proxied_by'])
+        if args['since']:
+            since_start, since_end = args['since']
             if since_start:
                 query = query.filter(Waiver.timestamp >= since_start)
             if since_end:
                 query = query.filter(Waiver.timestamp <= since_end)
-        if not data.get('include_obsolete', False):
+        if not args['include_obsolete']:
             query = _filter_out_obsolete_waivers(query)
 
         query = query.order_by(Waiver.timestamp.desc())

From 89c0f61f01ff9b811b704e32234e50b2c7ef8111 Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 04:33:01 +0000
Subject: [PATCH 3/7] also use RequestParser for making 'comment' required


---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index 0a06869..cbbf5d4 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -112,7 +112,7 @@ def test_create_waiver_without_comment(mocked_get_user, mocked_resultsdb, client
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 400
     res_data = json.loads(r.get_data(as_text=True))
-    assert res_data['message'] == 'Comment is a required argument.'
+    assert res_data['message']['comment'] == 'Missing required parameter in the JSON body'
 
 
 @patch('waiverdb.api_v1.get_resultsdb_result', side_effect=HTTPError(response=Mock(status=404)))
@@ -137,6 +137,7 @@ def test_create_waiver_with_no_testcase(mocked_get_user, client):
         'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
         'waived': True,
         'product_version': 'the-best',
+        'comment': 'it broke',
     }
     r = client.post('/api/v1.0/waivers/', data=json.dumps(data),
                     content_type='application/json')
diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py
index 3e666ee..2a3ca6d 100644
--- a/waiverdb/api_v1.py
+++ b/waiverdb/api_v1.py
@@ -109,7 +109,7 @@ RP['create_waiver'].add_argument('testcase', type=str, location='json')
 RP['create_waiver'].add_argument('result_id', type=int, location='json')
 RP['create_waiver'].add_argument('waived', type=bool, required=True, location='json')
 RP['create_waiver'].add_argument('product_version', type=str, required=True, location='json')
-RP['create_waiver'].add_argument('comment', type=str, default=None, location='json')
+RP['create_waiver'].add_argument('comment', type=str, required=True, location='json')
 RP['create_waiver'].add_argument('username', type=str, default=None, location='json')
 
 RP['get_waivers'] = reqparse.RequestParser()
@@ -346,8 +346,6 @@ class WaiversResource(Resource):
         if not args['subject'] or not args['testcase']:
             raise BadRequest('Either result_id or subject/testcase '
                              'are required arguments.')
-        if not args['comment']:
-            raise BadRequest('Comment is a required argument.')
 
         return Waiver(
             args['subject'],

From 9faf3820a635f6005d3643654ca087724821a81d Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 04:33:01 +0000
Subject: [PATCH 4/7] replace 'subject' with 'subject_type' and 'subject_identifier'


This is for Greenwave's API v2: https://pagure.io/greenwave/issue/126

---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index cbbf5d4..e5fd51a 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -15,7 +15,8 @@ from waiverdb.models import Waiver
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_waiver(mocked_get_user, client, session):
     data = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
@@ -27,15 +28,42 @@ def test_create_waiver(mocked_get_user, client, session):
     assert r.status_code == 201
     assert res_data['username'] == 'foo'
     assert res_data['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
+    assert res_data['subject_type'] == 'koji_build'
+    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'
 
 
+@patch('waiverdb.auth.get_user', return_value=('foo', {}))
+def test_create_waiver_with_subject(mocked_get_user, client, session):
+    # 'subject' key was the API in Waiverdb < 0.11
+    data = {
+        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_identifier': 'glibc-2.26-27.fc27',
+        'testcase': 'dist.rpmdeplint',
+        'product_version': 'fedora-27',
+        'waived': True,
+        'comment': 'it really 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': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
+    assert res_data['subject_type'] == 'koji_build'
+    assert res_data['subject_identifier'] == 'glibc-2.26-27.fc27'
+    assert res_data['testcase'] == 'dist.rpmdeplint'
+    assert res_data['product_version'] == 'fedora-27'
+    assert res_data['waived'] is True
+    assert res_data['comment'] == 'it really broke'
+
+
 @patch('waiverdb.api_v1.get_resultsdb_result')
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
-def test_create_waiver_legacy(mocked_get_user, mocked_resultsdb, client, session):
+def test_create_waiver_with_result_id(mocked_get_user, mocked_resultsdb, client, session):
     mocked_resultsdb.return_value = {
         'data': {
             'type': ['koji_build'],
@@ -44,6 +72,7 @@ def test_create_waiver_legacy(mocked_get_user, mocked_resultsdb, client, session
         'testcase': {'name': 'sometest'}
     }
 
+    # 'result_id' key was the API in Waiverdb < 0.6
     data = {
         'result_id': 123,
         'product_version': 'fool-1',
@@ -56,6 +85,8 @@ def test_create_waiver_legacy(mocked_get_user, mocked_resultsdb, client, session
     assert r.status_code == 201
     assert res_data['username'] == 'foo'
     assert res_data['subject'] == {'type': 'koji_build', 'item': 'somebuild'}
+    assert res_data['subject_type'] == 'koji_build'
+    assert res_data['subject_identifier'] == 'somebuild'
     assert res_data['testcase'] == 'sometest'
     assert res_data['product_version'] == 'fool-1'
     assert res_data['waived'] is True
@@ -64,8 +95,8 @@ def test_create_waiver_legacy(mocked_get_user, mocked_resultsdb, client, session
 
 @patch('waiverdb.api_v1.get_resultsdb_result')
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
-def test_create_waiver_with_original_spec_nvr_subject(mocked_get_user, mocked_resultsdb, client,
-                                                      session):
+def test_create_waiver_with_result_for_original_spec_nvr(
+        mocked_get_user, mocked_resultsdb, client, session):
     mocked_resultsdb.return_value = {
         'data': {
             'original_spec_nvr': ['somedata'],
@@ -84,26 +115,20 @@ def test_create_waiver_with_original_spec_nvr_subject(mocked_get_user, mocked_re
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 201
     assert res_data['username'] == 'foo'
-    assert res_data['subject'] == {'original_spec_nvr': 'somedata'}
+    assert res_data['subject'] == {'type': 'koji_build', 'item': 'somedata'}
+    assert res_data['subject_type'] == 'koji_build'
+    assert res_data['subject_identifier'] == 'somedata'
     assert res_data['testcase'] == 'sometest'
     assert res_data['product_version'] == 'fool-1'
     assert res_data['waived'] is True
     assert res_data['comment'] == 'it broke'
 
 
-@patch('waiverdb.api_v1.get_resultsdb_result')
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
-def test_create_waiver_without_comment(mocked_get_user, mocked_resultsdb, client,
-                                       session):
-    mocked_resultsdb.return_value = {
-        'data': {
-            'original_spec_nvr': ['somedata'],
-        },
-        'testcase': {'name': 'sometest'}
-    }
-
+def test_create_waiver_without_comment(mocked_get_user, client, session):
     data = {
-        'result_id': 123,
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'product_version': 'fool-1',
         'waived': True,
     }
@@ -134,7 +159,8 @@ def test_create_waiver_with_unknown_result_id(mocked_get_user, mocked_resultsdb,
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_waiver_with_no_testcase(mocked_get_user, client):
     data = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'waived': True,
         'product_version': 'the-best',
         'comment': 'it broke',
@@ -143,10 +169,7 @@ def test_create_waiver_with_no_testcase(mocked_get_user, client):
                     content_type='application/json')
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 400
-    # XXX - when we ditch result_id and make subject and testcase required
-    # again, this assertion will change back to the original.
-#   assert 'Missing required parameter in the JSON body' in res_data['message']['testcase']
-    assert 'Either result_id or subject/testcase are required arguments.' in res_data['message']
+    assert 'Missing required parameter in the JSON body' in res_data['message']['testcase']
 
 
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
@@ -165,7 +188,8 @@ def test_create_waiver_with_malformed_subject(mocked_get_user, client):
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_non_superuser_cannot_create_waiver_for_other_users(mocked_get_user, client):
     data = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
@@ -182,7 +206,8 @@ def test_non_superuser_cannot_create_waiver_for_other_users(mocked_get_user, cli
 @patch('waiverdb.auth.get_user', return_value=('bodhi', {}))
 def test_superuser_can_create_waiver_for_other_users(mocked_get_user, client, session):
     data = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
@@ -200,14 +225,17 @@ def test_superuser_can_create_waiver_for_other_users(mocked_get_user, client, se
 
 def test_get_waiver(client, session):
     # create a new waiver
-    waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    waiver = create_waiver(session, subject_type='koji_build',
+                           subject_identifier='glibc-2.26-27.fc27',
                            testcase='testcase1', username='foo',
                            product_version='foo-1', comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/%s' % waiver.id)
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert res_data['username'] == waiver.username
-    assert res_data['subject'] == waiver.subject
+    assert res_data['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
+    assert res_data['subject_type'] == waiver.subject_type
+    assert res_data['subject_identifier'] == waiver.subject_identifier
     assert res_data['testcase'] == waiver.testcase
     assert res_data['product_version'] == waiver.product_version
     assert res_data['waived'] is True
@@ -234,7 +262,7 @@ def test_500(mocked, client, session):
 
 def test_get_waivers(client, session):
     for i in range(0, 10):
-        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
+        create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/')
@@ -245,7 +273,7 @@ def test_get_waivers(client, session):
 
 def test_pagination_waivers(client, session):
     for i in range(0, 30):
-        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
+        create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/?page=2')
@@ -259,10 +287,12 @@ def test_pagination_waivers(client, session):
 
 
 def test_obsolete_waivers_are_excluded_by_default(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build',
+                  subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo',
                   product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    new_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='foo',
                                product_version='foo-1', waived=False)
     r = client.get('/api/v1.0/waivers/')
@@ -274,10 +304,12 @@ def test_obsolete_waivers_are_excluded_by_default(client, session):
 
 
 def test_get_obsolete_waivers(client, session):
-    old_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    old_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='foo',
                                product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    new_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='foo',
                                product_version='foo-1', waived=False)
     r = client.get('/api/v1.0/waivers/?include_obsolete=1')
@@ -289,10 +321,12 @@ def test_get_obsolete_waivers(client, session):
 
 
 def test_obsolete_waivers_with_different_product_version(client, session):
-    old_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    old_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='foo',
                                product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    new_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='foo',
                                product_version='foo-2')
     r = client.get('/api/v1.0/waivers/?include_obsolete=0')
@@ -304,10 +338,12 @@ def test_obsolete_waivers_with_different_product_version(client, session):
 
 
 def test_obsolete_waivers_with_different_username(client, session):
-    old_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    old_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='foo',
                                product_version='foo-1')
-    new_waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    new_waiver = create_waiver(session, subject_type='koji_build',
+                               subject_identifier='glibc-2.26-27.fc27',
                                testcase='testcase1', username='bar',
                                product_version='foo-1')
     r = client.get('/api/v1.0/waivers/?include_obsolete=0')
@@ -318,31 +354,36 @@ def test_obsolete_waivers_with_different_username(client, session):
     assert res_data['data'][1]['id'] == old_waiver.id
 
 
-def test_filtering_waivers_by_subject(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+def test_filtering_waivers_by_subject_type(client, session):
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase', username='foo-1', product_version='foo-1')
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
+    create_waiver(session, subject_type='bodhi_update', subject_identifier='FEDORA-2017-7e594f96bb',
                   testcase='testcase', username='foo-2', product_version='foo-1')
 
-    r = client.get('/api/v1.0/waivers/?subject=%s' % json.dumps({'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}))
+    r = client.get('/api/v1.0/waivers/?subject_type=bodhi_update')
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
+    assert res_data['data'][0]['subject_type'] == 'bodhi_update'
 
 
-def test_filtering_waivers_with_invalid_json_subject(client, session):
-    r = client.get('/api/v1.0/waivers/?subject=[')
-    assert r.status_code == 400
+def test_filtering_waivers_by_subject_identifier(client, session):
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
+                  testcase='testcase', username='foo-1', product_version='foo-1')
+    create_waiver(session, subject_type='koji_build', subject_identifier='kernel-4.15.17-300.fc27',
+                  testcase='testcase', username='foo-2', product_version='foo-1')
+
+    r = client.get('/api/v1.0/waivers/?subject_identifier=glibc-2.26-27.fc27')
     res_data = json.loads(r.get_data(as_text=True))
-    assert res_data['message']['subject'] == \
-        'Invalid JSON: Expecting value: line 1 column 2 (char 1)'
+    assert r.status_code == 200
+    assert len(res_data['data']) == 1
+    assert res_data['data'][0]['subject_identifier'] == 'glibc-2.26-27.fc27'
 
 
 def test_filtering_waivers_by_testcase(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo-1', product_version='foo-1')
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase2', username='foo-2', product_version='foo-1')
 
     r = client.get('/api/v1.0/waivers/?testcase=testcase1')
@@ -353,9 +394,9 @@ def test_filtering_waivers_by_testcase(client, session):
 
 
 def test_filtering_waivers_by_product_version(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo-1', product_version='release-1')
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='kernel-4.15.17-300.fc27',
                   testcase='testcase2', username='foo-1', product_version='release-2')
     r = client.get('/api/v1.0/waivers/?product_version=release-1')
     res_data = json.loads(r.get_data(as_text=True))
@@ -365,9 +406,9 @@ def test_filtering_waivers_by_product_version(client, session):
 
 
 def test_filtering_waivers_by_username(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo', product_version='foo-1')
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='kernel-4.15.17-300.fc27',
                   testcase='testcase2', username='bar', product_version='foo-2')
     r = client.get('/api/v1.0/waivers/?username=foo')
     res_data = json.loads(r.get_data(as_text=True))
@@ -380,20 +421,20 @@ def test_filtering_waivers_by_since(client, session):
     before1 = (datetime.datetime.utcnow() - datetime.timedelta(seconds=100)).isoformat()
     before2 = (datetime.datetime.utcnow() - datetime.timedelta(seconds=99)).isoformat()
     after = (datetime.datetime.utcnow() + datetime.timedelta(seconds=100)).isoformat()
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo', product_version='foo-1')
     r = client.get('/api/v1.0/waivers/?since=%s' % before1)
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
+    assert res_data['data'][0]['subject_identifier'] == 'glibc-2.26-27.fc27'
     assert res_data['data'][0]['testcase'] == 'testcase1'
 
     r = client.get('/api/v1.0/waivers/?since=%s,%s' % (before1, after))
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 1
-    assert res_data['data'][0]['subject'] == {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'}
+    assert res_data['data'][0]['subject_identifier'] == 'glibc-2.26-27.fc27'
     assert res_data['data'][0]['testcase'] == 'testcase1'
 
     r = client.get('/api/v1.0/waivers/?since=%s' % (after))
@@ -429,10 +470,10 @@ def test_filtering_waivers_by_malformed_since(client, session):
 
 
 def test_filtering_waivers_by_proxied_by(client, session):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo-1', product_version='foo-1',
                   proxied_by='bodhi')
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='kernel-4.15.17-300.fc27',
                   testcase='testcase2', username='foo-2', product_version='foo-1')
     r = client.get('/api/v1.0/waivers/?proxied_by=bodhi')
     res_data = json.loads(r.get_data(as_text=True))
@@ -443,7 +484,8 @@ def test_filtering_waivers_by_proxied_by(client, session):
 
 
 def test_jsonp(client, session):
-    waiver = create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    waiver = create_waiver(session, subject_type='koji_build',
+                           subject_identifier='glibc-2.26-27.fc27',
                            testcase='testcase1', username='foo', product_version='foo-1')
     r = client.get('/api/v1.0/waivers/%s?callback=jsonpcallback' % waiver.id)
     assert r.mimetype == 'application/javascript'
@@ -465,7 +507,7 @@ def test_get_waivers_with_post_request(client, session):
     for i in range(1, 51):
         results.append({'subject': {'type': 'koji_build', 'item': '%d' % i},
                         'testcase': 'case %d' % i})
-        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
+        create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     data = {
@@ -476,12 +518,12 @@ def test_get_waivers_with_post_request(client, session):
     res_data = json.loads(r.get_data(as_text=True))
     assert r.status_code == 200
     assert len(res_data['data']) == 50
-    subjects = []
+    subject_identifiers = []
     testcases = []
     for i in reversed(range(1, 51)):
-        subjects.append({'type': 'koji_build', 'item': '%d' % i})
+        subject_identifiers.append('%d' % i)
         testcases.append('case %d' % i)
-    assert [w['subject'] for w in res_data['data']] == subjects
+    assert [w['subject_identifier'] for w in res_data['data']] == subject_identifiers
     assert set([w['testcase'] for w in res_data['data']]) == set(testcases)
     assert all(w['username'].startswith('foo') for w in res_data['data'])
     assert all(w['product_version'].startswith('foo-') for w in res_data['data'])
@@ -506,7 +548,7 @@ def test_filtering_waivers_with_bad_key(client, session, results):
     [{}],
 ])
 def test_filtering_waivers_with_empty_results(client, session, results):
-    create_waiver(session, subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+    create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo-1', product_version='foo-1')
     data = {'results': results}
     r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
@@ -572,7 +614,7 @@ def test_no_cors_about(client, session):
 @pytest.mark.usefixtures('enable_cors')
 def test_cors_waivers(client, session):
     for i in range(0, 3):
-        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
+        create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/')
@@ -591,7 +633,7 @@ def test_cors_waivers(client, session):
 
 def test_no_cors_waivers(client, session):
     for i in range(0, 3):
-        create_waiver(session, subject={'type': 'koji_build', 'item': "%d" % i},
+        create_waiver(session, subject_type='koji_build', subject_identifier="%d" % i,
                       testcase="case %d" % i, username='foo %d' % i,
                       product_version='foo-%d' % i, comment='bla bla bla')
     r = client.get('/api/v1.0/waivers/')
@@ -608,14 +650,16 @@ def test_no_cors_waivers(client, session):
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_multiple_waivers(mocked_get_user, client, session):
     item1 = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
         'comment': 'it broke',
     }
     item2 = {
-        'subject': {'type': 'koji_build', 'item': 'kernel-4.15.17-300.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'kernel-4.15.17-300.fc27',
         'testcase': 'testcase2',
         'product_version': 'fool-2',
         'waived': False,
@@ -643,7 +687,8 @@ def test_create_multiple_waivers(mocked_get_user, client, session):
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_multiple_waivers_rollback_on_error(mocked_get_user, client, session):
     item1 = {
-        'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        'subject_type': 'koji_build',
+        'subject_identifier': 'glibc-2.26-27.fc27',
         'testcase': 'testcase1',
         'product_version': 'fool-1',
         'waived': True,
diff --git a/tests/test_events.py b/tests/test_events.py
index 0a64414..f248572 100644
--- a/tests/test_events.py
+++ b/tests/test_events.py
@@ -9,7 +9,8 @@ from waiverdb.models import Waiver
 @mock.patch('waiverdb.events.fedmsg')
 def test_publish_new_waiver_with_fedmsg(mock_fedmsg, session):
     waiver = Waiver(
-        subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        subject_type='koji_build',
+        subject_identifier='glibc-2.26-27.fc27',
         testcase='testcase1',
         username='jcline',
         product_version='something',
@@ -23,6 +24,8 @@ def test_publish_new_waiver_with_fedmsg(mock_fedmsg, session):
         topic='waiver.new',
         msg={
             'id': waiver.id,
+            'subject_type': 'koji_build',
+            'subject_identifier': 'glibc-2.26-27.fc27',
             'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
             'testcase': 'testcase1',
             'username': 'jcline',
@@ -38,7 +41,8 @@ def test_publish_new_waiver_with_fedmsg(mock_fedmsg, session):
 @mock.patch('waiverdb.events.fedmsg')
 def test_publish_new_waiver_with_fedmsg_for_proxy_user(mock_fedmsg, session):
     waiver = Waiver(
-        subject={'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+        subject_type='koji_build',
+        subject_identifier='glibc-2.26-27.fc27',
         testcase='testcase1',
         username='jcline',
         product_version='something',
@@ -53,6 +57,8 @@ def test_publish_new_waiver_with_fedmsg_for_proxy_user(mock_fedmsg, session):
         topic='waiver.new',
         msg={
             'id': waiver.id,
+            'subject_type': 'koji_build',
+            'subject_identifier': 'glibc-2.26-27.fc27',
             'subject': {'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
             'testcase': 'testcase1',
             'username': 'jcline',
diff --git a/tests/test_model.py b/tests/test_model.py
deleted file mode 100644
index ffab473..0000000
--- a/tests/test_model.py
+++ /dev/null
@@ -1,29 +0,0 @@
-
-# SPDX-License-Identifier: GPL-2.0+
-
-from collections import OrderedDict
-from waiverdb.models import Waiver
-
-
-# https://pagure.io/waiverdb/issue/134
-def test_waiver_subject_json_serialization_is_consistent(db):
-    # With the default json.dumps serializer, the order of keys in the
-    # serialized JSON object could vary because dict keys are iterated in
-    # arbitrary order. Hence we have a custom serializer to guarantee the
-    # serialization is predictable, regardless of iteration order.
-    # In this test we therefore use OrderedDict and not dict, specifically so
-    # we can guarantee that the iteration order is *not* the same in each
-    # object.
-    nvr = 'python-requests-1.2.3-1.fc26'
-    subject = OrderedDict([('item', nvr), ('type', 'koji_build')])
-    equivalent_subject = OrderedDict([('type', 'koji_build'), ('item', nvr)])
-    waiver = Waiver(username='dcallagh', waived=True, comment='test', product_version='fedora-26',
-                    testcase='dist.rpmlint', subject=subject)
-    db.session.add(waiver)
-    waiver = Waiver(username='dcallagh', waived=True, comment='test', product_version='fedora-26',
-                    testcase='dist.rpmlint', subject=equivalent_subject)
-    db.session.add(waiver)
-    db.session.flush()
-    query = Waiver.query.filter(Waiver.subject == {'item': nvr, 'type': 'koji_build'})
-    # The query should select *both* the waivers above
-    assert query.count() == 2
diff --git a/tests/utils.py b/tests/utils.py
index ef8d9a2..52dbda2 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -3,10 +3,10 @@
 from waiverdb.models import Waiver
 
 
-def create_waiver(session, subject, testcase, username, product_version, waived=True,
-                  comment=None, proxied_by=None):
-    waiver = Waiver(subject, testcase, username, product_version, waived, comment,
-                    proxied_by)
+def create_waiver(session, subject_type, subject_identifier, testcase, username, product_version,
+                  waived=True, comment=None, proxied_by=None):
+    waiver = Waiver(subject_type, subject_identifier, testcase, username, product_version,
+                    waived, comment, proxied_by)
     session.add(waiver)
     session.flush()
     return waiver
diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py
index 2a3ca6d..01b2ed8 100644
--- a/waiverdb/api_v1.py
+++ b/waiverdb/api_v1.py
@@ -1,16 +1,16 @@
 # SPDX-License-Identifier: GPL-2.0+
 
-import json
 import datetime
 
 import requests
 from flask import Blueprint, request, current_app
 from flask_restful import Resource, Api, reqparse, marshal_with, marshal
 from werkzeug.exceptions import BadRequest, Forbidden, ServiceUnavailable
-from sqlalchemy.sql.expression import func, cast
+from sqlalchemy.sql.expression import func
 
 from waiverdb import __version__
-from waiverdb.models import db, Waiver
+from waiverdb.models import db
+from waiverdb.models.waivers import Waiver, subject_dict_to_type_identifier
 from waiverdb.utils import json_collection, jsonp
 from waiverdb.fields import waiver_fields
 import waiverdb.auth
@@ -26,16 +26,6 @@ def valid_dict(value):
     return value
 
 
-def json_dict(value):
-    try:
-        value = json.loads(value)
-    except ValueError as e:
-        raise ValueError("Invalid JSON: %s" % e)
-    if not isinstance(value, dict):
-        raise ValueError("Must be a valid dict, not %r" % value)
-    return value
-
-
 def get_resultsdb_result(result_id):
     response = requests_session.request('GET', '{0}/results/{1}'.format(
         current_app.config['RESULTSDB_API_URL'], result_id),
@@ -92,7 +82,8 @@ def _filter_out_obsolete_waivers(query):
     same subject, test case name, username and product_version.
     """
     subquery = db.session.query(func.max(Waiver.id)).group_by(
-        cast(Waiver.subject, db.Text),
+        Waiver.subject_type,
+        Waiver.subject_identifier,
         Waiver.testcase,
         Waiver.username,
         Waiver.product_version,
@@ -104,8 +95,11 @@ def _filter_out_obsolete_waivers(query):
 #    Parsers are added in each 'resource section' for better readability
 RP = {}
 RP['create_waiver'] = reqparse.RequestParser()
-RP['create_waiver'].add_argument('subject', type=valid_dict, location='json')
+RP['create_waiver'].add_argument('subject_type', type=str, location='json')
+RP['create_waiver'].add_argument('subject_identifier', type=str, location='json')
 RP['create_waiver'].add_argument('testcase', type=str, location='json')
+# These are accepted for backwards compatibility
+RP['create_waiver'].add_argument('subject', type=valid_dict, location='json')
 RP['create_waiver'].add_argument('result_id', type=int, location='json')
 RP['create_waiver'].add_argument('waived', type=bool, required=True, location='json')
 RP['create_waiver'].add_argument('product_version', type=str, required=True, location='json')
@@ -113,7 +107,8 @@ RP['create_waiver'].add_argument('comment', type=str, required=True, location='j
 RP['create_waiver'].add_argument('username', type=str, default=None, location='json')
 
 RP['get_waivers'] = reqparse.RequestParser()
-RP['get_waivers'].add_argument('subject', type=json_dict, location='args')
+RP['get_waivers'].add_argument('subject_type', location='args')
+RP['get_waivers'].add_argument('subject_identifier', location='args')
 RP['get_waivers'].add_argument('testcase', location='args')
 RP['get_waivers'].add_argument('product_version', location='args')
 RP['get_waivers'].add_argument('username', location='args')
@@ -180,7 +175,8 @@ class WaiversResource(Resource):
 
         :query int page: The page to get.
         :query int limit: Limit the number of items returned.
-        :query dict subject: Only include waivers for the given subject.
+        :query string subject_type: Only include waivers for the given subject type.
+        :query string subject_identifier: Only include waivers for the given subject identifier.
         :query string testcase: Only include waivers for the given test case name.
         :query string product_version: Only include waivers for the given
             product version.
@@ -200,8 +196,10 @@ class WaiversResource(Resource):
         args = RP['get_waivers'].parse_args()
         query = Waiver.query.order_by(Waiver.timestamp.desc())
 
-        if args['subject']:
-            query = query.filter(Waiver.subject == args['subject'])
+        if args['subject_type']:
+            query = query.filter(Waiver.subject_type == args['subject_type'])
+        if args['subject_identifier']:
+            query = query.filter(Waiver.subject_identifier == args['subject_identifier'])
         if args['testcase']:
             query = query.filter(Waiver.testcase == args['testcase'])
         if args['product_version']:
@@ -245,7 +243,8 @@ class WaiversResource(Resource):
            Content-Length: 91
 
            {
-               "subject": {"productmd.compose.id": "Fedora-9000-19700101.n.18"},
+               "subject_type": "compose",
+               "subject_identifier": "Fedora-9000-19700101.n.18",
                "testcase": "compose.install_no_user",
                "waived": false,
                "product_version": "Parrot",
@@ -276,7 +275,11 @@ class WaiversResource(Resource):
                "proxied_by": null
            }
 
-        :json object subject: The result subject for the waiver.
+        :json string subject_type: The type of thing which this waiver is for
+            ("koji_build", "bodhi_update", "compose").
+        :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.
         :json string testcase: The result testcase for the waiver.
         :json boolean waived: Whether or not the result is waived.
         :json string product_version: The product version string.
@@ -313,13 +316,13 @@ class WaiversResource(Resource):
             proxied_by = user
             user = args['username']
 
-        # XXX - remove this in a future release (it was for temp backwards compat)
+        # WaiverDB < 0.6
         if args['result_id']:
             if args['subject'] or args['testcase']:
                 raise BadRequest('Only result_id or subject and '
                                  'testcase are allowed.  Not both.')
             try:
-                result = get_resultsdb_result(args['result_id'])
+                result = get_resultsdb_result(args.pop('result_id'))
             except requests.HTTPError as e:
                 if e.response.status_code == 404:
                     raise BadRequest('Result id not found in Resultsdb')
@@ -327,28 +330,41 @@ class WaiversResource(Resource):
                     raise ServiceUnavailable('Failed looking up result in Resultsdb: %s' % e)
             except Exception as e:
                 raise ServiceUnavailable('Failed looking up result in Resultsdb: %s' % e)
-            if 'original_spec_nvr' in result['data']:
-                subject = {'original_spec_nvr': result['data']['original_spec_nvr'][0]}
+            result_data = result['data']  # ResultsDB "extra data" for the given result
+            if 'original_spec_nvr' in result_data:
+                args['subject_type'] = 'koji_build'
+                args['subject_identifier'] = result_data['original_spec_nvr'][0]
+            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'
+                args['subject_identifier'] = result_data['item'][0]
             else:
-                if result['data']['type'][0] == 'koji_build' or \
-                   result['data']['type'][0] == 'brew-build' or \
-                   result['data']['type'][0] == 'bodhi_update':
-                    SUBJECT_KEYS = ['item', 'type']
-                    subject = dict([(k, v[0]) for k, v in result['data'].items()
-                                    if k in SUBJECT_KEYS])
-                else:
-                    raise BadRequest('It is not possible to submit a waiver by '
-                                     'id for this result. Please try again specifying '
-                                     'a subject and a testcase.')
-            args['subject'] = subject
+                raise BadRequest('It is not possible to submit a waiver by '
+                                 'id for this result. Please try again specifying '
+                                 'a subject and a testcase.')
             args['testcase'] = result['testcase']['name']
 
-        if not args['subject'] or not args['testcase']:
-            raise BadRequest('Either result_id or subject/testcase '
-                             'are required arguments.')
+        # WaiverDB < 0.11
+        if args['subject']:
+            args['subject_type'], args['subject_identifier'] = \
+                subject_dict_to_type_identifier(args.pop('subject'))
+
+        # These are not marked required in the RequestParser, because they may
+        # be absent in the request but filled in by the backwards
+        # compatibility logic above. So we check explicitly here, and give
+        # back an error matching what RequestParser would do.
+        if not args['subject_type']:
+            raise BadRequest({'subject_type': 'Missing required parameter in the JSON body'})
+        if not args['subject_identifier']:
+            raise BadRequest({'subject_identifier': 'Missing required parameter in the JSON body'})
+        if not args['testcase']:
+            raise BadRequest({'testcase': 'Missing required parameter in the JSON body'})
 
         return Waiver(
-            args['subject'],
+            args['subject_type'],
+            args['subject_identifier'],
             args['testcase'],
             user,
             args['product_version'],
diff --git a/waiverdb/fields.py b/waiverdb/fields.py
index baf7198..ff1d73d 100644
--- a/waiverdb/fields.py
+++ b/waiverdb/fields.py
@@ -2,10 +2,21 @@
 
 from flask_restful import fields
 
+from waiverdb.models.waivers import subject_type_identifier_to_dict
+
+
+class BackwardsCompatibleSubjectField(fields.Raw):
+
+    def output(self, key, obj):
+        waiver = obj
+        return subject_type_identifier_to_dict(waiver.subject_type, waiver.subject_identifier)
+
 
 waiver_fields = {
     'id': fields.Integer,
-    'subject': fields.Raw,
+    'subject_type': fields.String,
+    'subject_identifier': fields.String,
+    'subject': BackwardsCompatibleSubjectField,
     'testcase': fields.String,
     'username': fields.String,
     'proxied_by': fields.String,
diff --git a/waiverdb/migrations/versions/f6bc296ba966_subject_dict_to_type_identifier.py b/waiverdb/migrations/versions/f6bc296ba966_subject_dict_to_type_identifier.py
new file mode 100644
index 0000000..d742520
--- /dev/null
+++ b/waiverdb/migrations/versions/f6bc296ba966_subject_dict_to_type_identifier.py
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+"""Replace waiver.subject dict with subject_type and subject_identifier
+
+Revision ID: f6bc296ba966
+Revises: ce8a1351ecdc
+Create Date: 2018-04-24 16:21:42.017640
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'f6bc296ba966'
+down_revision = 'ce8a1351ecdc'
+
+from alembic import op
+from sqlalchemy import Column, Text, Integer, null
+# These are the "lightweight" SQL expression versions (not using metadata):
+from sqlalchemy.sql.expression import table, column, select, update
+from waiverdb.models.base import EqualityComparableJSONType
+from waiverdb.models.waivers import subject_dict_to_type_identifier, \
+    subject_type_identifier_to_dict
+
+def upgrade():
+    # Create the columns NULLable first, so that we can populate them
+    op.add_column('waiver', Column('subject_identifier', Text, nullable=True))
+    op.add_column('waiver', Column('subject_type', Text, nullable=True))
+
+    # Lightweight table definition for producing queries:
+    waiver_table = table('waiver',
+                         column('id', type_=Integer),
+                         column('subject', type_=EqualityComparableJSONType),
+                         column('subject_type', type_=Text),
+                         column('subject_identifier', type_=Text))
+
+    # Fill in values for the new columns
+    connection = op.get_bind()
+    rows = connection.execute(select([waiver_table.c.id, waiver_table.c.subject]))
+    for waiver_id, subject in rows:
+        subject_type, subject_identifier = subject_dict_to_type_identifier(subject)
+        connection.execute(update(waiver_table)
+                           .where(waiver_table.c.id == waiver_id)
+                           .values(subject_type=subject_type,
+                                   subject_identifier=subject_identifier))
+
+    # Now make the columns non-NULLable, and populate indexes
+    op.alter_column('waiver', 'subject_type', nullable=False)
+    op.alter_column('waiver', 'subject_identifier', nullable=False)
+    op.create_index('ix_waiver_subject_identifier', 'waiver', ['subject_identifier'])
+    op.create_index('ix_waiver_subject_type', 'waiver', ['subject_type'])
+    op.create_index('ix_waiver_subject_type_identifier', 'waiver', ['subject_type', 'subject_identifier'])
+
+    # Make the old column NULLable for newly inserted rows
+    # (we keep it around in case of downgrade though)
+    op.alter_column('waiver', 'subject', nullable=True)
+
+
+def downgrade():
+    # Lightweight table definition for producing queries:
+    waiver_table = table('waiver',
+                         column('id', type_=Integer),
+                         column('subject', type_=EqualityComparableJSONType),
+                         column('subject_type', type_=Text),
+                         column('subject_identifier', type_=Text))
+
+    # Fill in the old column for any waivers inserted since the upgrade
+    connection = op.get_bind()
+    rows = connection.execute(
+            select([waiver_table.c.id, waiver_table.c.subject_type, waiver_table.c.subject_identifier])
+            .where(waiver_table.c.subject.is_(null())))
+    for waiver_id, subject_type, subject_identifier in rows:
+        subject = subject_type_identifier_to_dict(subject_type, subject_identifier)
+        connection.execute(update(waiver_table)
+                           .where(waiver_table.c.id == waiver_id)
+                           .values(subject=subject))
+
+    # Drop the new columns
+    op.drop_index('ix_waiver_subject_type_identifier', table_name='waiver')
+    op.drop_index('ix_waiver_subject_type', table_name='waiver')
+    op.drop_index('ix_waiver_subject_identifier', table_name='waiver')
+    op.drop_column('waiver', 'subject_type')
+    op.drop_column('waiver', 'subject_identifier')
+
+    # Make the old column non-NULLable again
+    op.alter_column('waiver', 'subject', nullable=False)
diff --git a/waiverdb/models/waivers.py b/waiverdb/models/waivers.py
index 59381b4..0754fe4 100644
--- a/waiverdb/models/waivers.py
+++ b/waiverdb/models/waivers.py
@@ -1,13 +1,47 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 import datetime
-from .base import db, EqualityComparableJSONType
-from sqlalchemy import or_, and_, cast
+from .base import db
+from sqlalchemy import or_, and_
+
+
+def subject_dict_to_type_identifier(subject):
+    """
+    WaiverDB < 0.11 accepted an arbitrary dict for the '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:
+        return ('bodhi_update', subject['item'])
+    elif subject.get('type') == 'koji_build' and 'item' in subject:
+        return ('koji_build', subject['item'])
+    elif 'original_spec_nvr' in subject:
+        return ('koji_build', subject['original_spec_nvr'])
+    elif 'productmd.compose.id' in subject:
+        return ('compose', subject['productmd.compose.id'])
+    else:
+        raise ValueError('Unrecognised subject type: %r' % subject)
+
+
+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':
+        return {'productmd.compose.id': subject_identifier}
+    else:
+        raise ValueError('Unrecognised subject type: %s' % subject_type)
 
 
 class Waiver(db.Model):
     id = db.Column(db.Integer, primary_key=True)
-    subject = db.Column(EqualityComparableJSONType, nullable=False)
+    subject_type = db.Column(db.Text, nullable=False, index=True)
+    subject_identifier = db.Column(db.Text, nullable=False, index=True)
     testcase = db.Column(db.Text, nullable=False, index=True)
     username = db.Column(db.String(255), nullable=False)
     proxied_by = db.Column(db.String(255))
@@ -16,12 +50,13 @@ class Waiver(db.Model):
     comment = db.Column(db.Text)
     timestamp = db.Column(db.DateTime, default=datetime.datetime.utcnow)
     __table_args__ = (
-        db.Index('ix_waiver_subject', cast(subject, db.Text)),
+        db.Index('ix_waiver_subject_type_identifier', subject_type, subject_identifier),
     )
 
-    def __init__(self, subject, testcase, username, product_version, waived=False,
-                 comment=None, proxied_by=None):
-        self.subject = subject
+    def __init__(self, subject_type, subject_identifier, testcase, username, product_version,
+                 waived=False, comment=None, proxied_by=None):
+        self.subject_type = subject_type
+        self.subject_identifier = subject_identifier
         self.testcase = testcase
         self.username = username
         self.product_version = product_version
@@ -30,9 +65,10 @@ class Waiver(db.Model):
         self.proxied_by = proxied_by
 
     def __repr__(self):
-        return '%s(subject=%r, testcase=%r, username=%r, product_version=%r, waived=%r)' % (
-            self.__class__.__name__, self.subject, self.testcase, self.username,
-            self.product_version, self.waived)
+        return ('%s(subject_type=%r, subject_identifier=%r, testcase=%r, username=%r, '
+                'product_version=%r, waived=%r)'
+                % (self.__class__.__name__, self.subject_type, self.subject_identifier,
+                   self.testcase, self.username, self.product_version, self.waived))
 
     @classmethod
     def by_results(cls, query, results):
@@ -53,10 +89,15 @@ class Waiver(db.Model):
         for result in results:
             subject = result.get('subject', None)
             testcase = result.get('testcase', None)
-            if subject or testcase:
-                clauses.append(and_(
-                    not subject or cls.subject == subject,
-                    not testcase or cls.testcase == testcase
-                ))
+            if not subject and not testcase:
+                continue
+            inner_clauses = []
+            if subject:
+                subject_type, subject_identifier = subject_dict_to_type_identifier(subject)
+                inner_clauses.append(cls.subject_type == subject_type)
+                inner_clauses.append(cls.subject_identifier == subject_identifier)
+            if testcase:
+                inner_clauses.append(cls.testcase == testcase)
+            clauses.append(and_(*inner_clauses))
 
         return query.filter(or_(*clauses))

From 2499af4a7d6461db529f98e9c2eda7ad4cce604e Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 04:33:01 +0000
Subject: [PATCH 5/7] new POST /api/v1.0/waivers/+filtered endpoint


The POST /api/v1.0/waivers/+by-subjects-and-testcases endpoint has
proven confusing and difficult to use in practice.

This deprecates that endpoint in favour of a general "filtering" POST
endpoint. It should be easier to grok since the filter parameters work
the same way as they do for GET /api/v1.0/waivers/.

---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index e5fd51a..4382bb1 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -498,7 +498,40 @@ def test_healthcheck(client):
     assert r.get_data(as_text=True) == 'Health check OK'
 
 
-def test_get_waivers_with_post_request(client, session):
+def test_filtering_waivers_with_post(client, session):
+    filters = []
+    for i in range(1, 51):
+        filters.append({'subject_type': 'koji_build',
+                        'subject_identifier': 'python2-2.7.14-%d.fc27' % i,
+                        'testcase': 'case %d' % i})
+        create_waiver(session, subject_type='koji_build',
+                      subject_identifier='python2-2.7.14-%d.fc27' % i,
+                      testcase='case %d' % i, username='person',
+                      product_version='fedora-27', comment='bla bla bla')
+    # Unrelated waiver which should not be included
+    create_waiver(session, subject_type='koji_build',
+                  subject_identifier='glibc-2.26-27.fc27',
+                  testcase='dist.rpmdeplint', username='person',
+                  product_version='fedora-27', comment='bla bla bla')
+    r = client.post('/api/v1.0/waivers/+filtered',
+                    data=json.dumps({'filters': filters}),
+                    content_type='application/json')
+    res_data = json.loads(r.get_data(as_text=True))
+    assert r.status_code == 200
+    assert len(res_data['data']) == 50
+    assert all(w['subject_identifier'].startswith('python2-2.7.14') for w in res_data['data'])
+
+
+def test_filtering_with_missing_filter(client, session):
+    r = client.post('/api/v1.0/waivers/+filtered',
+                    data=json.dumps({'somethingelse': 'what'}),
+                    content_type='application/json')
+    res_data = json.loads(r.get_data(as_text=True))
+    assert r.status_code == 400
+    assert res_data['message']['filters'] == 'Missing required parameter in the JSON body'
+
+
+def test_waivers_by_subjects_and_testcases(client, session):
     """
     This tests that users can get waivers by sending a POST request with a long
     list of result subject/testcase.
@@ -533,7 +566,7 @@ def test_get_waivers_with_post_request(client, session):
     [{'item': {'subject.test1': 'subject1'}}],  # Unexpected key
     [{'subject': 'subject1'}],  # Unexpected key type
 ])
-def test_filtering_waivers_with_bad_key(client, session, results):
+def test_waivers_by_subjects_and_testcases_with_bad_results_parameter(client, session, results):
     data = {'results': results}
     r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
                     content_type='application/json')
@@ -547,7 +580,7 @@ def test_filtering_waivers_with_bad_key(client, session, results):
     [],
     [{}],
 ])
-def test_filtering_waivers_with_empty_results(client, session, results):
+def test_waivers_by_subjects_and_testcases_with_empty_results_parameter(client, session, results):
     create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
                   testcase='testcase1', username='foo-1', product_version='foo-1')
     data = {'results': results}
@@ -558,7 +591,7 @@ def test_filtering_waivers_with_empty_results(client, session, results):
     assert len(res_data['data']) == 1
 
 
-def test_get_waivers_with_post_malformed_since(client, session):
+def test_waivers_by_subjects_and_testcases_with_malformed_since(client, session):
     data = {'since': 123}
     r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
                     content_type='application/json')
diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py
index 01b2ed8..bd6e409 100644
--- a/waiverdb/api_v1.py
+++ b/waiverdb/api_v1.py
@@ -6,7 +6,7 @@ import requests
 from flask import Blueprint, request, current_app
 from flask_restful import Resource, Api, reqparse, marshal_with, marshal
 from werkzeug.exceptions import BadRequest, Forbidden, ServiceUnavailable
-from sqlalchemy.sql.expression import func
+from sqlalchemy.sql.expression import func, and_, or_
 
 from waiverdb import __version__
 from waiverdb.models import db
@@ -47,6 +47,15 @@ def valid_results_list(results):
     return results
 
 
+def valid_filter_list(filters):
+    if not filters:
+        raise ValueError('Must be a list of non-empty dictionaries')
+    for item in filters:
+        if not isinstance(item, dict) or not item:
+            raise ValueError('Must be a list of non-empty dictionaries')
+    return filters
+
+
 def reqparse_since(since):
     """
     Parses the 'since' query parameter, which is expected to be either a
@@ -120,6 +129,10 @@ RP['get_waivers'].add_argument('page', default=1, type=int, location='args')
 RP['get_waivers'].add_argument('limit', default=10, type=int, location='args')
 RP['get_waivers'].add_argument('proxied_by', location='args')
 
+RP['filter_waivers'] = reqparse.RequestParser()
+RP['filter_waivers'].add_argument('filters', type=valid_filter_list, required=True, location='json')
+RP['filter_waivers'].add_argument('include_obsolete', type=bool, default=False, location='json')
+
 RP['get_waivers_by_subjects_and_testcase'] = rp = reqparse.RequestParser()
 rp.add_argument('results', type=valid_results_list, location='json')
 rp.add_argument('testcase', type=str, location='json')
@@ -391,37 +404,38 @@ class WaiverResource(Resource):
             raise type(NotFound)('Waiver not found')
 
 
-class GetWaiversBySubjectsAndTestcases(Resource):
-    @jsonp
+class FilteredWaiversResource(Resource):
+
+    @marshal_with(waiver_fields, envelope='data')
     def post(self):
         """
-        Return a list of waivers by filtering the waivers with a
-        list of result subjects and testcases.
-        This accepts POST requests in order to handle a special
-        case where a GET /waivers/ request has a long query string with many
-        result subjects/testcases that could cause 413 errors.
+        Get waiver records, filtered by some criteria.
+
+        This API behaves the same way as :http:get:`/api/v1.0/waivers/`, but it
+        allows for longer or more complex filter criteria that cannot be
+        expressed in the query string.
+
+        Note that the response is not paginated (that is, *all* waivers are
+        returned in the 'data' key, even if there is a large number of them).
 
         **Sample request**:
 
         .. sourcecode:: http
 
-           POST /api/v1.0/waivers/+by-subjects-and-testcases HTTP/1.1
-           Host: localhost:5004
-           Accept-Encoding: gzip, deflate
+           POST /api/v1.0/waivers/+filtered HTTP/1.1
            Accept: application/json
-           Connection: keep-alive
-           User-Agent: HTTPie/0.9.4
            Content-Type: application/json
-           Content-Length: 40
 
            {
-                "results": [
+                "filters": [
                     {
-                        "subject": {"productmd.compose.id": "Fedora-9000-19700101.n.18"},
+                        "subject_type": "compose",
+                        "subject_identifier": "Fedora-9000-19700101.n.18",
                         "testcase": "compose.install_no_user"
                     },
                     {
-                        "subject": {"item": "gzip-1.9-1.fc28", "type": "koji_build"},
+                        "subject_type": "koji_build",
+                        "subject_identifier": "gzip-1.9-1.fc28",
                         "testcase": "dist.rpmlint"
                     }
                 ]
@@ -429,54 +443,119 @@ class GetWaiversBySubjectsAndTestcases(Resource):
 
         **Sample response**:
 
-        .. sourcecode:: http
+        .. sourcecode:: none
 
-            HTTP/1.0 200 OK
-            Content-Length: 562
-            Content-Type: application/json
-            Date: Thu, 21 Sep 2017 04:58:37 GMT
-            Server: Werkzeug/0.11.10 Python/2.7.13
+           HTTP/1.1 200 OK
+           Content-Type: application/json
 
-            {
+           {
                 "data": [
                     {
-                        "comment": "This is fine",
-                        "id": 5,
-                        "product_version": "Parrot",
-                        "subject": {"productmd.compose.id": "Fedora-9000-19700101.n.18"},
+                        "id": 15,
+                        "comment": "The tests broke",
+                        "product_version": "fedora-27",
+                        "subject_type": "compose",
+                        "subject_identifier": "Fedora-9000-19700101.n.18",
                         "testcase": "compose.install_no_user",
-                        "timestamp": "2017-09-21T04:55:53.343368",
-                        "username": "dummy",
+                        "timestamp": "2017-03-16T17:42:04.209638",
+                        "username": "jcline",
                         "waived": true,
                         "proxied_by": null
+                    }
+                ]
+           }
+
+        :json list filters: List of filter dicts. If the list contains
+            multiple filter dicts, they are combined with logical OR. Within
+            each filter dict, the criteria are combined with logical AND. Keys
+            within the filter dict are the same as the filtering
+            parameters accepted by :http:get:`/api/v1.0/waivers/`.
+        :json boolean include_obsolete: If true, obsolete waivers will be included.
+        :statuscode 200: Returns matching waivers, if any.
+        :statuscode 400: The request was malformed (invalid filter critera).
+        """
+        args = RP['filter_waivers'].parse_args()
+        query = Waiver.query.order_by(Waiver.timestamp.desc())
+        clauses = []
+        for filter_ in args['filters']:
+            inner_clauses = []
+            if 'subject_type' in filter_:
+                inner_clauses.append(Waiver.subject_type == filter_['subject_type'])
+            if 'subject_identifier' in filter_:
+                inner_clauses.append(Waiver.subject_identifier == filter_['subject_identifier'])
+            if 'testcase' in filter_:
+                inner_clauses.append(Waiver.testcase == filter_['testcase'])
+            if 'product_version' in filter_:
+                inner_clauses.append(Waiver.product_version == filter_['product_version'])
+            if 'username' in filter_:
+                inner_clauses.append(Waiver.username == filter_['username'])
+            if 'proxied_by' in filter_:
+                inner_clauses.append(Waiver.proxied_by == filter_['proxied_by'])
+            if 'since' in filter_:
+                try:
+                    since_start, since_end = reqparse_since(filter_['since'])
+                except ValueError as e:
+                    raise BadRequest({'since': str(e)})
+                if since_start:
+                    inner_clauses.append(Waiver.timestamp >= since_start)
+                if since_end:
+                    inner_clauses.append(Waiver.timestamp <= since_end)
+            clauses.append(and_(*inner_clauses))
+        query = query.filter(or_(*clauses))
+        if not args['include_obsolete']:
+            subquery = db.session.query(func.max(Waiver.id))\
+                .group_by(Waiver.subject_type, Waiver.subject_identifier, Waiver.testcase)
+            query = query.filter(Waiver.id.in_(subquery))
+        return query.all()
+
+
+class GetWaiversBySubjectsAndTestcases(Resource):
+    @jsonp
+    def post(self):
+        """
+        **Deprecated.** Use :http:post:`/api/v1.0/waivers/+filtered` instead.
+
+        Instead of making a deprecated request like this:
+
+        .. sourcecode:: http
+
+           POST /api/v1.0/waivers/+by-subjects-and-testcases HTTP/1.1
+           Content-Type: application/json
+
+           {
+                "results": [
+                    {
+                        "subject": {"productmd.compose.id": "Fedora-9000-19700101.n.18"},
+                        "testcase": "compose.install_no_user"
                     },
                     {
-                        "comment": "This is fine",
-                        "id": 4,
-                        "product_version": "Parrot",
                         "subject": {"item": "gzip-1.9-1.fc28", "type": "koji_build"},
-                        "testcase": "dist.rpmlint",
-                        "timestamp": "2017-09-21T04:55:51.936658",
-                        "username": "dummy",
-                        "waived": true,
-                        "proxied_by": null
+                        "testcase": "dist.rpmlint"
                     }
                 ]
-            }
-
-        :jsonparam array results: Filter the waivers by a list of dictionaries
-            with result subjects and testcase.
-        :jsonparam string product_version: Filter the waivers by product version.
-        :jsonparam string username: Filter the waivers by username.
-        :jsonparam string proxied_by: Filter the waivers by the users who are
-            allowed to create waivers on behalf of other users.
-        :jsonparam string since: An ISO 8601 formatted datetime (e.g. 2017-03-16T13:40:05+00:00)
-            to filter results by. Optionally provide a second ISO 8601 datetime separated
-            by a comma to retrieve a range (e.g. 2017-03-16T13:40:05+00:00,
-            2017-03-16T13:40:15+00:00)
-        :jsonparam boolean include_obsolete: If true, obsolete waivers will be included.
-        :statuscode 200: If the query was valid and no problems were encountered.
-            Note that the response may still contain 0 waivers.
+           }
+
+        make the following equivalent request:
+
+        .. sourcecode:: http
+
+           POST /api/v1.0/waivers/+filtered HTTP/1.1
+           Content-Type: application/json
+
+           {
+                "filters": [
+                    {
+                        "subject_type": "compose",
+                        "subject_identifier": "Fedora-9000-19700101.n.18",
+                        "testcase": "compose.install_no_user"
+                    },
+                    {
+                        "subject_type": "koji_build",
+                        "subject_identifier": "gzip-1.9-1.fc28",
+                        "testcase": "dist.rpmlint"
+                    }
+                ]
+           }
         """
         args = RP['get_waivers_by_subjects_and_testcase'].parse_args()
         query = Waiver.query.order_by(Waiver.timestamp.desc())
@@ -531,5 +610,6 @@ class AboutResource(Resource):
 # set up the Api resource routing here
 api.add_resource(WaiversResource, '/waivers/')
 api.add_resource(WaiverResource, '/waivers/<int:waiver_id>')
+api.add_resource(FilteredWaiversResource, '/waivers/+filtered')
 api.add_resource(GetWaiversBySubjectsAndTestcases, '/waivers/+by-subjects-and-testcases')
 api.add_resource(AboutResource, '/about')

From 45346ab17aa9976d81a48ae43221245dcd75bf83 Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 04:33:01 +0000
Subject: [PATCH 6/7] map 'brew-build' subjects to koji_build subject type


---

diff --git a/tests/test_models.py b/tests/test_models.py
new file mode 100644
index 0000000..19eee06
--- /dev/null
+++ b/tests/test_models.py
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+import pytest
+
+from waiverdb.models.waivers import subject_dict_to_type_identifier
+
+
+@pytest.mark.parametrize('subject,expected_type,expected_identifier', [
+    ({'type': 'bodhi_update', 'item': 'FEDORA-2017-7e594f96bb'},
+     'bodhi_update', 'FEDORA-2017-7e594f96bb'),
+    ({'type': 'koji_build', 'item': 'glibc-2.26-27.fc27'},
+     'koji_build', 'glibc-2.26-27.fc27'),
+    # The 'brew-build' type is used internally within Red Hat.
+    # We treat it as the 'koji_build' subject type.
+    ({'type': 'brew-build', 'item': 'nss-softokn-3.36.1-1.0.el8+5'},
+     'koji_build', 'nss-softokn-3.36.1-1.0.el8+5'),
+    ({'original_spec_nvr': 'glibc-2.26-27.fc27'},
+     'koji_build', 'glibc-2.26-27.fc27'),
+    ({'productmd.compose.id': 'Fedora-Rawhide-20170508.n.0'},
+     'compose', 'Fedora-Rawhide-20170508.n.0'),
+])
+def test_subject_dict_to_type_identifier(subject, expected_type, expected_identifier):
+    subject_type, subject_identifier = subject_dict_to_type_identifier(subject)
+    assert subject_type == expected_type
+    assert subject_identifier == expected_identifier
diff --git a/waiverdb/models/waivers.py b/waiverdb/models/waivers.py
index 0754fe4..69e9c1e 100644
--- a/waiverdb/models/waivers.py
+++ b/waiverdb/models/waivers.py
@@ -13,7 +13,7 @@ def subject_dict_to_type_identifier(subject):
     """
     if subject.get('type') == 'bodhi_update' and 'item' in subject:
         return ('bodhi_update', subject['item'])
-    elif subject.get('type') == 'koji_build' and 'item' in subject:
+    elif subject.get('type') in ['koji_build', 'brew-build'] and 'item' in subject:
         return ('koji_build', subject['item'])
     elif 'original_spec_nvr' in subject:
         return ('koji_build', subject['original_spec_nvr'])

From 19db92548803d5b152f7c057abbf7d71633a927f Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: May 31 2018 05:03:55 +0000
Subject: [PATCH 7/7] accept unrecognised subject types in /api/v1.0/waivers/+by-subjects-and-testcases


---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index 4382bb1..f7e28b3 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -576,6 +576,26 @@ def test_waivers_by_subjects_and_testcases_with_bad_results_parameter(client, se
         'Must be a list of dictionaries with "subject" and "testcase"'
 
 
+def test_waivers_by_subjects_and_testcases_with_unrecognized_subject_type(client, session):
+    create_waiver(session, subject_type='koji_build',
+                  subject_identifier='python3-flask-0.12.2-1.fc29',
+                  testcase='dist.rpmdeplint', username='person',
+                  product_version='fedora-29', comment='bla bla bla')
+    # This doesn't match any of the known subject types which we understand
+    # for backwards compatibility. So if you tried to submit a waiver with a
+    # subject like this, Waiverdb would reject it. But if the caller is just
+    # *searching* and not *submitting* then instead of giving back an error
+    # Waiverdb should just return an empty result set.
+    data = {'results': [
+        {'subject': {'item': 'python3-flask-0.12.2-1.fc29'}, 'testcase': 'dist.rpmdeplint'},
+    ]}
+    r = client.post('/api/v1.0/waivers/+by-subjects-and-testcases', data=json.dumps(data),
+                    content_type='application/json')
+    res_data = json.loads(r.get_data(as_text=True))
+    assert r.status_code == 200
+    assert res_data['data'] == []
+
+
 @pytest.mark.parametrize("results", [
     [],
     [{}],
diff --git a/waiverdb/models/waivers.py b/waiverdb/models/waivers.py
index 69e9c1e..00bd4a5 100644
--- a/waiverdb/models/waivers.py
+++ b/waiverdb/models/waivers.py
@@ -2,7 +2,7 @@
 
 import datetime
 from .base import db
-from sqlalchemy import or_, and_
+from sqlalchemy import or_, and_, false
 
 
 def subject_dict_to_type_identifier(subject):
@@ -93,9 +93,13 @@ class Waiver(db.Model):
                 continue
             inner_clauses = []
             if subject:
-                subject_type, subject_identifier = subject_dict_to_type_identifier(subject)
-                inner_clauses.append(cls.subject_type == subject_type)
-                inner_clauses.append(cls.subject_identifier == subject_identifier)
+                try:
+                    subject_type, subject_identifier = subject_dict_to_type_identifier(subject)
+                except ValueError:
+                    inner_clauses.append(false())
+                else:
+                    inner_clauses.append(cls.subject_type == subject_type)
+                    inner_clauses.append(cls.subject_identifier == subject_identifier)
             if testcase:
                 inner_clauses.append(cls.testcase == testcase)
             clauses.append(and_(*inner_clauses))