Skip to content

Commit

Permalink
waivers/+filtered API endpoint: handle scenarios (#431)
Browse files Browse the repository at this point in the history
The FilteredWaiversResource method which backs this endpoint
tries to find only 'current' waivers by grouping all waivers
by subject_type, subject_identifier and testcase, then taking
only the highest-numbered waiver from each group. This breaks if
we have multiple waivers for the same subject and testcase but
with different scenarios - the endpoint will only return one
waiver, the one with the highest ID, and will not return the
valid and current waivers for other scenarios as it should.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
  • Loading branch information
AdamWill committed Aug 8, 2023
1 parent d985336 commit ef89179
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
33 changes: 33 additions & 0 deletions tests/test_api_v10.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,39 @@ def test_filtering_waivers_with_post(client, session):
assert all(w['subject_identifier'].startswith('python2-2.7.14') for w in res_data['data'])


def test_filtering_scenario_waivers_with_post(client, session):
filters = [{'subject_type': 'koji_build',
'subject_identifier': 'python2-2.7.14-1.fc27',
'testcase': 'case'}]
for i in range(1, 11):
create_waiver(session, subject_type='koji_build',
subject_identifier='python2-2.7.14-1.fc27',
testcase='case', username='person', scenario='scenario.%d' % i,
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')
# looking for all waivers for this subject/testcase should find all 10
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']) == 10
assert all(w['subject_identifier'].startswith('python2-2.7.14-1') for w in res_data['data'])
# now test finding waivers by scenario, should find only 1
filters[0]['scenario'] = 'scenario.5'
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']) == 1
assert all(w['subject_identifier'].startswith('python2-2.7.14-1') 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'}),
Expand Down
8 changes: 6 additions & 2 deletions waiverdb/api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,12 @@ def post(self):
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)
subquery = db.session.query(func.max(Waiver.id)).group_by(
Waiver.subject_type,
Waiver.subject_identifier,
Waiver.testcase,
Waiver.scenario
)
query = query.filter(Waiver.id.in_(subquery))
return query.all()

Expand Down

0 comments on commit ef89179

Please sign in to comment.