Skip to content

Commit

Permalink
Fix/replace flawed sorter
Browse files Browse the repository at this point in the history
  • Loading branch information
TheByronHimes committed Sep 7, 2023
1 parent 0d2e93b commit 04e4dcc
Showing 1 changed file with 41 additions and 18 deletions.
59 changes: 41 additions & 18 deletions tests/test_sorting.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,52 @@
]


def sort_resources(
def multi_column_sort(
resources: list[models.Resource], sorts: list[models.SortingParameter]
) -> list[models.Resource]:
"""Convenience function to sort a list of resources for comparison"""
"""This is equivalent to nested sorted() calls.
This uses the same approach as the sorting function in test_relevance, but the
difference is that this function uses Resource models and doesn't work with the
relevance sorting parameter. There's no spot for a top-level text score parameter in
the resource model, which is why the relevance tests use a slightly different version
of this function.
The sorting parameters are supplied in order of most significant to least significant,
so we take them off the front and apply sorted(). If there are more parameters to
apply (more sorts), we recurse until we apply the final parameter. The sorted lists
are passed back up the call chain.
"""
sorted_list = resources.copy()
sorts = sorts.copy()

parameter = sorts[0]
del sorts[0]

for parameter in sorts:
reverse = True if parameter.order == models.SortOrder.DESCENDING else False
# sort descending for DESCENDING and RELEVANCE
reverse = parameter.order != models.SortOrder.ASCENDING

if parameter.field == "id_":
sorted_list.sort(key=lambda resource: resource.id_, reverse=reverse)
else:
# all other fields will be contained within 'content'.
sorted_list.sort(
key=lambda resource: resource.dict()["content"][parameter.field],
reverse=reverse,
)
if len(sorts) > 0:
# if there are more sorting parameters, recurse to nest the sorts
sorted_list = multi_column_sort(sorted_list, sorts)

return sorted_list
if parameter.field == "id_":
return sorted(
sorted_list,
key=lambda result: result.dict()[parameter.field],
reverse=reverse,
)
else:
# the only top-level fields is "_id" -- all else is in "content"
return sorted(
sorted_list,
key=lambda result: result.dict()["content"][parameter.field],
reverse=reverse,
)


@pytest.mark.asyncio
async def test_api_without_search_parameters(joint_fixture: JointFixture):
async def test_api_without_sort_parameters(joint_fixture: JointFixture):
"""Make sure default Pydantic model parameter works as expected"""

search_parameters: JsonObject = {
Expand All @@ -62,7 +85,7 @@ async def test_api_without_search_parameters(joint_fixture: JointFixture):
search_parameters=search_parameters
)
assert results.count > 0
expected = sort_resources(results.hits, BASIC_SORT_PARAMETERS)
expected = multi_column_sort(results.hits, BASIC_SORT_PARAMETERS)
assert results.hits == expected


Expand Down Expand Up @@ -91,7 +114,7 @@ async def test_sort_with_id_not_last(joint_fixture: JointFixture):
for param in sorts
]
results = await joint_fixture.call_search_endpoint(search_parameters)
assert results.hits == sort_resources(results.hits, sorts_in_model_form)
assert results.hits == multi_column_sort(results.hits, sorts_in_model_form)


@pytest.mark.asyncio
Expand All @@ -113,7 +136,7 @@ async def test_sort_with_params_but_not_id(joint_fixture: JointFixture):
}

results = await joint_fixture.call_search_endpoint(search_parameters)
assert results.hits == sort_resources(results.hits, BASIC_SORT_PARAMETERS)
assert results.hits == multi_column_sort(results.hits, BASIC_SORT_PARAMETERS)


@pytest.mark.asyncio
Expand All @@ -138,7 +161,7 @@ async def test_sort_with_invalid_field(joint_fixture: JointFixture):
}

results = await joint_fixture.call_search_endpoint(search_parameters)
assert results.hits == sort_resources(results.hits, BASIC_SORT_PARAMETERS)
assert results.hits == multi_column_sort(results.hits, BASIC_SORT_PARAMETERS)


@pytest.mark.parametrize("order", [-7, 17, "some_string"])
Expand Down

0 comments on commit 04e4dcc

Please sign in to comment.