From 863aea7a6bffff36a4a68bab31ff8a480d35ea14 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Thu, 31 Aug 2023 13:56:34 +0000 Subject: [PATCH 01/14] Add sorting spec parameters --- mass/adapters/inbound/fastapi_/models.py | 6 +++++- mass/core/models.py | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/mass/adapters/inbound/fastapi_/models.py b/mass/adapters/inbound/fastapi_/models.py index e05b064..73ba5c8 100644 --- a/mass/adapters/inbound/fastapi_/models.py +++ b/mass/adapters/inbound/fastapi_/models.py @@ -18,7 +18,7 @@ from pydantic import BaseModel, Field -from mass.core.models import Filter +from mass.core.models import Filter, SortingParameter, SortOrder class SearchParameters(BaseModel): @@ -37,3 +37,7 @@ class SearchParameters(BaseModel): limit: Optional[int] = Field( default=None, description="Limit the results to this number" ) + sorting_parameters: list[SortingParameter] = Field( + default=[SortingParameter(sort_field="id_", sort_order=SortOrder.ASCENDING)], + description=("Collection of sorting parameters used to refine search results"), + ) diff --git a/mass/core/models.py b/mass/core/models.py index d039c88..68c6ffb 100644 --- a/mass/core/models.py +++ b/mass/core/models.py @@ -14,6 +14,8 @@ # limitations under the License. """Defines dataclasses for holding business-logic data""" +from enum import IntEnum + from hexkit.custom_types import JsonObject from pydantic import BaseModel, Field @@ -71,3 +73,22 @@ class QueryResults(BaseModel): facets: list[Facet] = Field(default=[], description="Contains the faceted fields") count: int = Field(default=0, description="The number of results found") hits: list[Resource] = Field(default=[], description="The search results") + + +class SortOrder(IntEnum): + """Represents the possible sorting orders""" + + ASCENDING = 1 + DESCENDING = -1 + + +class SortingParameter(BaseModel): + """Represents a combination of a field to sort and the sort order""" + + sort_field: str = Field( + ..., + description=("Which field to sort results by."), + ) + sort_order: SortOrder = Field( + default=SortOrder.ASCENDING, description="Sort order to apply to sort_field" + ) From be1826660f9202de77b7fc39b4a4d401503f094f Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Thu, 31 Aug 2023 14:04:53 +0000 Subject: [PATCH 02/14] Pass sorting parameters through service chain --- mass/adapters/inbound/fastapi_/routes.py | 1 + mass/adapters/outbound/aggregator.py | 2 ++ mass/adapters/outbound/utils.py | 3 +++ mass/core/query_handler.py | 2 ++ mass/ports/inbound/query_handler.py | 1 + mass/ports/outbound/aggregator.py | 1 + 6 files changed, 10 insertions(+) diff --git a/mass/adapters/inbound/fastapi_/routes.py b/mass/adapters/inbound/fastapi_/routes.py index b23dbbd..eb60ffa 100644 --- a/mass/adapters/inbound/fastapi_/routes.py +++ b/mass/adapters/inbound/fastapi_/routes.py @@ -69,6 +69,7 @@ async def search( filters=parameters.filters, skip=parameters.skip, limit=parameters.limit, + sorting_parameters=parameters.sorting_parameters, ) except query_handler.ClassNotConfiguredError as err: raise HTTPException( diff --git a/mass/adapters/outbound/aggregator.py b/mass/adapters/outbound/aggregator.py index f4d44ed..4fea938 100644 --- a/mass/adapters/outbound/aggregator.py +++ b/mass/adapters/outbound/aggregator.py @@ -46,6 +46,7 @@ async def aggregate( facet_fields: list[models.FacetLabel], skip: int = 0, limit: Optional[int] = None, + sorting_parameters: list[models.SortingParameter], ) -> JsonObject: # don't carry out aggregation if the collection is empty if not await self._collection.find_one(): @@ -58,6 +59,7 @@ async def aggregate( facet_fields=facet_fields, skip=skip, limit=limit, + sorting_parameters=sorting_parameters, ) try: diff --git a/mass/adapters/outbound/utils.py b/mass/adapters/outbound/utils.py index 9aa91f5..17d1fb7 100644 --- a/mass/adapters/outbound/utils.py +++ b/mass/adapters/outbound/utils.py @@ -115,6 +115,9 @@ def build_pipeline( facet_fields: list[models.FacetLabel], skip: int = 0, limit: Optional[int] = None, + sorting_parameters: list[ # pylint: disable=unused-argument + models.SortingParameter + ], ) -> list[JsonObject]: """Build aggregation pipeline based on query""" pipeline: list[JsonObject] = [] diff --git a/mass/core/query_handler.py b/mass/core/query_handler.py index 245de5c..c4d9a7a 100644 --- a/mass/core/query_handler.py +++ b/mass/core/query_handler.py @@ -71,6 +71,7 @@ async def handle_query( filters: list[models.Filter], skip: int = 0, limit: Optional[int] = None, + sorting_parameters: list[models.SortingParameter], ) -> models.QueryResults: # get configured facet fields for given resource class try: @@ -89,6 +90,7 @@ async def handle_query( facet_fields=facet_fields, skip=skip, limit=limit, + sorting_parameters=sorting_parameters, ) except AggregationError as exc: raise self.SearchError() from exc diff --git a/mass/ports/inbound/query_handler.py b/mass/ports/inbound/query_handler.py index 1e7fe17..206c992 100644 --- a/mass/ports/inbound/query_handler.py +++ b/mass/ports/inbound/query_handler.py @@ -77,6 +77,7 @@ async def handle_query( filters: list[models.Filter], skip: int = 0, limit: Optional[int] = None, + sorting_parameters: list[models.SortingParameter], ) -> models.QueryResults: """Processes a query diff --git a/mass/ports/outbound/aggregator.py b/mass/ports/outbound/aggregator.py index c0357a9..c50e974 100644 --- a/mass/ports/outbound/aggregator.py +++ b/mass/ports/outbound/aggregator.py @@ -43,6 +43,7 @@ async def aggregate( facet_fields: list[models.FacetLabel], skip: int = 0, limit: Optional[int] = None, + sorting_parameters: list[models.SortingParameter], ) -> JsonObject: """Applies an aggregation pipeline to a mongodb collection""" ... From 1b211cb7c227cae0d5f14d78c76787a6cd48a764 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Thu, 31 Aug 2023 14:07:33 +0000 Subject: [PATCH 03/14] Apply sorting in pipeline --- mass/adapters/outbound/utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mass/adapters/outbound/utils.py b/mass/adapters/outbound/utils.py index 17d1fb7..4d8913f 100644 --- a/mass/adapters/outbound/utils.py +++ b/mass/adapters/outbound/utils.py @@ -115,9 +115,7 @@ def build_pipeline( facet_fields: list[models.FacetLabel], skip: int = 0, limit: Optional[int] = None, - sorting_parameters: list[ # pylint: disable=unused-argument - models.SortingParameter - ], + sorting_parameters: list[models.SortingParameter], ) -> list[JsonObject]: """Build aggregation pipeline based on query""" pipeline: list[JsonObject] = [] @@ -128,7 +126,9 @@ def build_pipeline( pipeline.append(pipeline_match_text_search(query=query)) # sort initial results - pipeline.append({"$sort": {"_id": 1}}) + pipeline.append( + {"$sort": {param.sort_field: param.sort_order for param in sorting_parameters}} + ) # apply filters if filters: From 63389477fa3efd29ae985725d320cce4ab5c4704 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Thu, 31 Aug 2023 14:12:03 +0000 Subject: [PATCH 04/14] Always include id_ in sorting parameters --- mass/core/query_handler.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mass/core/query_handler.py b/mass/core/query_handler.py index c4d9a7a..97110f3 100644 --- a/mass/core/query_handler.py +++ b/mass/core/query_handler.py @@ -73,6 +73,14 @@ async def handle_query( limit: Optional[int] = None, sorting_parameters: list[models.SortingParameter], ) -> models.QueryResults: + # if id_ is not in sorting_parameters, add to end + if "id_" not in [param.sort_field for param in sorting_parameters]: + sorting_parameters.append( + models.SortingParameter( + sort_field="id_", sort_order=models.SortOrder.ASCENDING + ) + ) + # get configured facet fields for given resource class try: facet_fields: list[models.FacetLabel] = self._config.searchable_classes[ From 04e3bc759a4c024926254678059fe505569b1a6c Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Thu, 31 Aug 2023 14:33:23 +0000 Subject: [PATCH 05/14] Make sorting_parameters optional in QueryHandler --- mass/core/query_handler.py | 6 +++++- mass/ports/inbound/query_handler.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/mass/core/query_handler.py b/mass/core/query_handler.py index 97110f3..7975b27 100644 --- a/mass/core/query_handler.py +++ b/mass/core/query_handler.py @@ -71,8 +71,12 @@ async def handle_query( filters: list[models.Filter], skip: int = 0, limit: Optional[int] = None, - sorting_parameters: list[models.SortingParameter], + sorting_parameters: Optional[list[models.SortingParameter]] = None, ) -> models.QueryResults: + # set empty list if not provided + if sorting_parameters is None: + sorting_parameters = [] + # if id_ is not in sorting_parameters, add to end if "id_" not in [param.sort_field for param in sorting_parameters]: sorting_parameters.append( diff --git a/mass/ports/inbound/query_handler.py b/mass/ports/inbound/query_handler.py index 206c992..f7aa2eb 100644 --- a/mass/ports/inbound/query_handler.py +++ b/mass/ports/inbound/query_handler.py @@ -77,7 +77,7 @@ async def handle_query( filters: list[models.Filter], skip: int = 0, limit: Optional[int] = None, - sorting_parameters: list[models.SortingParameter], + sorting_parameters: Optional[list[models.SortingParameter]] = None, ) -> models.QueryResults: """Processes a query From 6ef6c4f4da213a42802143112bf64d54f83645cf Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Fri, 1 Sep 2023 10:33:41 +0000 Subject: [PATCH 06/14] Rename facet stage and run it every time --- mass/adapters/outbound/utils.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/mass/adapters/outbound/utils.py b/mass/adapters/outbound/utils.py index 4d8913f..7a54177 100644 --- a/mass/adapters/outbound/utils.py +++ b/mass/adapters/outbound/utils.py @@ -53,8 +53,12 @@ def pipeline_match_filters_stage(*, filters: list[models.Filter]) -> JsonObject: return {"$match": segment} -def pipeline_apply_facets( - *, facet_fields: list[models.FacetLabel], skip: int, limit: Optional[int] = None +def pipeline_facet_sort_and_paginate( + *, + facet_fields: list[models.FacetLabel], + skip: int, + limit: Optional[int] = None, + sorts: JsonObject, ): """Uses a list of facetable property names to build the subquery for faceting""" segment: dict[str, list[JsonObject]] = {} @@ -80,9 +84,9 @@ def pipeline_apply_facets( # sort by ID, then rename the ID field to id_ to match our model segment["hits"] = [ - {"$sort": {"_id": 1}}, {"$addFields": {"id_": "$_id"}}, {"$unset": "_id"}, + {"$sort": sorts}, ] # apply skip and limit for pagination @@ -125,20 +129,22 @@ def build_pipeline( if query: pipeline.append(pipeline_match_text_search(query=query)) - # sort initial results - pipeline.append( - {"$sort": {param.sort_field: param.sort_order for param in sorting_parameters}} - ) - # apply filters if filters: pipeline.append(pipeline_match_filters_stage(filters=filters)) + # turn the sorting parameters into a formatted pipeline $sort + sorts = {param.sort_field: param.sort_order for param in sorting_parameters} + # define facets from preliminary results and reshape data - if facet_fields: - pipeline.append( - pipeline_apply_facets(facet_fields=facet_fields, skip=skip, limit=limit) + pipeline.append( + pipeline_facet_sort_and_paginate( + facet_fields=facet_fields, + skip=skip, + limit=limit, + sorts=sorts, ) + ) # transform data one more time to match models pipeline.append(pipeline_project(facet_fields=facet_fields)) From 82a7e7d8a3ecb0542ec95f7a4d1b1f86b86aca51 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Fri, 1 Sep 2023 13:36:17 +0000 Subject: [PATCH 07/14] Add validator to prevent duplicate sort fields --- mass/adapters/inbound/fastapi_/models.py | 10 +++++++- openapi.yaml | 32 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/mass/adapters/inbound/fastapi_/models.py b/mass/adapters/inbound/fastapi_/models.py index 73ba5c8..e42dc2f 100644 --- a/mass/adapters/inbound/fastapi_/models.py +++ b/mass/adapters/inbound/fastapi_/models.py @@ -16,7 +16,7 @@ """Models only used by the API""" from typing import Optional -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, validator from mass.core.models import Filter, SortingParameter, SortOrder @@ -41,3 +41,11 @@ class SearchParameters(BaseModel): default=[SortingParameter(sort_field="id_", sort_order=SortOrder.ASCENDING)], description=("Collection of sorting parameters used to refine search results"), ) + + @validator("sorting_parameters") + @classmethod + def no_duplicate_fields(cls, parameters: list[SortingParameter]): + """Check for duplicate fields in sorting parameters""" + all_sort_fields = [param.sort_field for param in parameters] + if len(set(all_sort_fields)) < len(all_sort_fields): + raise ValueError("Sorting parameters cannot contain duplicate fields") diff --git a/openapi.yaml b/openapi.yaml index 465c20f..da5e1a5 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -159,6 +159,15 @@ components: description: The number of results to skip for pagination title: Skip type: integer + sorting_parameters: + default: + - sort_field: id_ + sort_order: 1 + description: Collection of sorting parameters used to refine search results + items: + $ref: '#/components/schemas/SortingParameter' + title: Sorting Parameters + type: array required: - class_name title: SearchParameters @@ -181,6 +190,29 @@ components: - facetable_properties title: SearchableClass type: object + SortOrder: + description: Represents the possible sorting orders + enum: + - 1 + - -1 + title: SortOrder + type: integer + SortingParameter: + description: Represents a combination of a field to sort and the sort order + properties: + sort_field: + description: Which field to sort results by. + title: Sort Field + type: string + sort_order: + allOf: + - $ref: '#/components/schemas/SortOrder' + default: 1 + description: Sort order to apply to sort_field + required: + - sort_field + title: SortingParameter + type: object ValidationError: properties: loc: From c7d11373666a2095fb22d2e1c41bded585304d2e Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Fri, 1 Sep 2023 14:32:26 +0000 Subject: [PATCH 08/14] Add tests for the sorting parameters --- tests/fixtures/test_config.yaml | 5 + tests/fixtures/test_data/SortingTests.json | 28 ++++ tests/test_sorting.py | 183 +++++++++++++++++++++ 3 files changed, 216 insertions(+) create mode 100644 tests/fixtures/test_data/SortingTests.json create mode 100644 tests/test_sorting.py diff --git a/tests/fixtures/test_config.yaml b/tests/fixtures/test_config.yaml index 9219c8d..3ed84d8 100644 --- a/tests/fixtures/test_config.yaml +++ b/tests/fixtures/test_config.yaml @@ -31,6 +31,11 @@ searchable_classes: facetable_properties: - key: fun_fact name: Fun Fact + SortingTests: + description: Data for testing sorting functionality. + facetable_properties: + - key: field + name: Field resource_change_event_topic: searchable_resources resource_deletion_event_type: searchable_resource_deleted resource_upsertion_event_type: searchable_resource_upserted diff --git a/tests/fixtures/test_data/SortingTests.json b/tests/fixtures/test_data/SortingTests.json new file mode 100644 index 0000000..4206851 --- /dev/null +++ b/tests/fixtures/test_data/SortingTests.json @@ -0,0 +1,28 @@ +{ + "items": [ + { + "field": "some data", + "id_": "i2" + }, + { + "field": "some data", + "id_": "i1" + }, + { + "field": "some data", + "id_": "i3" + }, + { + "field": "some data", + "id_": "i5" + }, + { + "field": "some data", + "id_": "i6" + }, + { + "field": "some data", + "id_": "i4" + } + ] +} diff --git a/tests/test_sorting.py b/tests/test_sorting.py new file mode 100644 index 0000000..f529617 --- /dev/null +++ b/tests/test_sorting.py @@ -0,0 +1,183 @@ +# Copyright 2021 - 2023 Universität Tübingen, DKFZ, EMBL, and Universität zu Köln +# for the German Human Genome-Phenome Archive (GHGA) +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Tests concerning the sorting functionality""" + +import pytest +from hexkit.custom_types import JsonObject + +from mass.core import models +from tests.fixtures.joint import JointFixture + +CLASS_NAME = "SortingTests" +BASIC_SORT_PARAMETERS = [ + models.SortingParameter(sort_field="id_", sort_order=models.SortOrder.ASCENDING) +] + + +def sort_resources( + resources: list[models.Resource], sorts: list[models.SortingParameter] +) -> list[models.Resource]: + """Convenience function to sort a list of resources for comparison""" + sorted_list = resources.copy() + + for parameter in sorts: + reverse = True if parameter.sort_order == -1 else False + + if parameter.sort_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.sort_field], + reverse=reverse, + ) + + return sorted_list + + +@pytest.mark.asyncio +async def test_api_without_search_parameters(joint_fixture: JointFixture): + """Make sure default Pydantic model parameter works as expected""" + + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + } + + results = await joint_fixture.call_search_endpoint( + search_parameters=search_parameters + ) + assert results.count >= 0 + expected = sort_resources(results.hits, BASIC_SORT_PARAMETERS) + assert results.hits == expected + + +@pytest.mark.asyncio +async def test_sort_with_id_not_last(joint_fixture: JointFixture): + """Test sorting parameters that contain id_, but id_ is not final sorting field. + + Since we modify sorting parameters based on presence of id_, make sure there aren't + any bugs that will break the sort or query process. + """ + sorts = [ + {"sort_field": "id_", "sort_order": 1}, + {"sort_field": "field", "sort_order": -1}, + ] + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + "sorting_parameters": sorts, + } + + sorts_in_model_form = [models.SortingParameter(**param) for param in sorts] + results = await joint_fixture.call_search_endpoint(search_parameters) + assert results.hits == sort_resources(results.hits, sorts_in_model_form) + + +@pytest.mark.asyncio +async def test_sort_with_params_but_not_id(joint_fixture: JointFixture): + """Test supplying sorting parameters but omitting id_. + + In order to provide consistent sorting, id_ should always be included. If it's not + explicitly included, it will be added as the final sorting field in order to break + any tie between otherwise equivalent keys. If it is included but is not the final + field, then we should not modify the parameters. + """ + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + "sorting_parameters": [{"sort_field": "field", "sort_order": 1}], + } + + results = await joint_fixture.call_search_endpoint(search_parameters) + assert results.hits == sort_resources(results.hits, BASIC_SORT_PARAMETERS) + + +@pytest.mark.asyncio +async def test_sort_with_invalid_field(joint_fixture: JointFixture): + """Test supplying an invalid field name as a sort field. + + MongoDB treats any documents without a given sort field as if they had a `null` + value for it. If we sort with a truly invalid field, it should have no impact on the + resulting sort order. + """ + + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + "sorting_parameters": [{"sort_field": "some_bogus_field", "sort_order": 1}], + } + + results = await joint_fixture.call_search_endpoint(search_parameters) + assert results.hits == sort_resources(results.hits, BASIC_SORT_PARAMETERS) + + +@pytest.mark.parametrize("sort_order", [-7, 17, "some_string"]) +@pytest.mark.asyncio +async def test_sort_with_invalid_sort_order(joint_fixture: JointFixture, sort_order): + """Test supplying an invalid value for the sort order""" + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + "sorting_parameters": [{"sort_field": "field", "sort_order": sort_order}], + } + + response = await joint_fixture.rest_client.post( + url="/rpc/search", json=search_parameters + ) + assert response.status_code == 422 + + +@pytest.mark.asyncio +async def test_sort_with_invalid_field_and_sort_order(joint_fixture: JointFixture): + """Test with both invalid field name and invalid sort order.""" + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + "sorting_parameters": [{"sort_field": "some_bogus_field", "sort_order": -7}], + } + + response = await joint_fixture.rest_client.post( + url="/rpc/search", json=search_parameters + ) + assert response.status_code == 422 + + +@pytest.mark.asyncio +async def test_sort_with_duplicate_field(joint_fixture: JointFixture): + """Supply sorting parameters with two instances of the same sort field. + + This should be prevented by the pydantic model validator and raise an HTTP error. + """ + search_parameters: JsonObject = { + "class_name": CLASS_NAME, + "query": "", + "filters": [], + "sorting_parameters": [ + {"sort_field": "field", "sort_order": 1}, + {"sort_field": "field", "sort_order": 1}, + ], + } + response = await joint_fixture.rest_client.post( + url="/rpc/search", json=search_parameters + ) + assert response.status_code == 422 From 88ef706422c0fe763c2a3812ed8341ac66658a61 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Fri, 1 Sep 2023 14:33:20 +0000 Subject: [PATCH 09/14] Bump version from 0.3.0 -> 0.3.1 --- README.md | 6 +++--- mass/__init__.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a29a000..4b86443 100644 --- a/README.md +++ b/README.md @@ -32,13 +32,13 @@ We recommend using the provided Docker container. A pre-build version is available at [docker hub](https://hub.docker.com/repository/docker/ghga/mass): ```bash -docker pull ghga/mass:0.3.0 +docker pull ghga/mass:0.3.1 ``` Or you can build the container yourself from the [`./Dockerfile`](./Dockerfile): ```bash # Execute in the repo's root dir: -docker build -t ghga/mass:0.3.0 . +docker build -t ghga/mass:0.3.1 . ``` For production-ready deployment, we recommend using Kubernetes, however, @@ -46,7 +46,7 @@ for simple use cases, you could execute the service using docker on a single server: ```bash # The entrypoint is preconfigured: -docker run -p 8080:8080 ghga/mass:0.3.0 --help +docker run -p 8080:8080 ghga/mass:0.3.1 --help ``` If you prefer not to use containers, you may install the service from source: diff --git a/mass/__init__.py b/mass/__init__.py index 857a430..6c42764 100644 --- a/mass/__init__.py +++ b/mass/__init__.py @@ -15,4 +15,4 @@ """A service for searching metadata artifacts and filtering results.""" -__version__ = "0.3.0" +__version__ = "0.3.1" From 3e9a93436ce4e0a464645cf9222cd6c456bb9993 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Mon, 4 Sep 2023 13:43:22 +0000 Subject: [PATCH 10/14] Change intenum to string enum --- mass/adapters/outbound/utils.py | 2 ++ mass/core/models.py | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/mass/adapters/outbound/utils.py b/mass/adapters/outbound/utils.py index 7a54177..1028e86 100644 --- a/mass/adapters/outbound/utils.py +++ b/mass/adapters/outbound/utils.py @@ -22,6 +22,8 @@ from mass.core import models +SORT_ORDER_CONVERSION = {"ascending": 1, "descending": -1} + def pipeline_match_text_search(*, query: str) -> JsonObject: """Build text search segment of aggregation pipeline""" diff --git a/mass/core/models.py b/mass/core/models.py index 68c6ffb..39aaacf 100644 --- a/mass/core/models.py +++ b/mass/core/models.py @@ -14,7 +14,7 @@ # limitations under the License. """Defines dataclasses for holding business-logic data""" -from enum import IntEnum +from enum import Enum from hexkit.custom_types import JsonObject from pydantic import BaseModel, Field @@ -75,11 +75,11 @@ class QueryResults(BaseModel): hits: list[Resource] = Field(default=[], description="The search results") -class SortOrder(IntEnum): +class SortOrder(Enum): """Represents the possible sorting orders""" - ASCENDING = 1 - DESCENDING = -1 + ASCENDING = "ascending" + DESCENDING = "descending" class SortingParameter(BaseModel): From b855678bdac5fe9e35a30b24e894ca833e3b3778 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Mon, 4 Sep 2023 13:44:43 +0000 Subject: [PATCH 11/14] Change sort_field/sort_order to field/order --- mass/adapters/inbound/fastapi_/models.py | 4 ++-- mass/adapters/outbound/utils.py | 5 ++++- mass/core/models.py | 4 ++-- mass/core/query_handler.py | 6 ++---- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/mass/adapters/inbound/fastapi_/models.py b/mass/adapters/inbound/fastapi_/models.py index e42dc2f..d5cd390 100644 --- a/mass/adapters/inbound/fastapi_/models.py +++ b/mass/adapters/inbound/fastapi_/models.py @@ -38,7 +38,7 @@ class SearchParameters(BaseModel): default=None, description="Limit the results to this number" ) sorting_parameters: list[SortingParameter] = Field( - default=[SortingParameter(sort_field="id_", sort_order=SortOrder.ASCENDING)], + default=[SortingParameter(field="id_", order=SortOrder.ASCENDING)], description=("Collection of sorting parameters used to refine search results"), ) @@ -46,6 +46,6 @@ class SearchParameters(BaseModel): @classmethod def no_duplicate_fields(cls, parameters: list[SortingParameter]): """Check for duplicate fields in sorting parameters""" - all_sort_fields = [param.sort_field for param in parameters] + all_sort_fields = [param.field for param in parameters] if len(set(all_sort_fields)) < len(all_sort_fields): raise ValueError("Sorting parameters cannot contain duplicate fields") diff --git a/mass/adapters/outbound/utils.py b/mass/adapters/outbound/utils.py index 1028e86..9d03fa6 100644 --- a/mass/adapters/outbound/utils.py +++ b/mass/adapters/outbound/utils.py @@ -136,7 +136,10 @@ def build_pipeline( pipeline.append(pipeline_match_filters_stage(filters=filters)) # turn the sorting parameters into a formatted pipeline $sort - sorts = {param.sort_field: param.sort_order for param in sorting_parameters} + sorts = {} + for param in sorting_parameters: + sort_order = SORT_ORDER_CONVERSION[param.order.value] + sorts[param.field] = sort_order # define facets from preliminary results and reshape data pipeline.append( diff --git a/mass/core/models.py b/mass/core/models.py index 39aaacf..039da56 100644 --- a/mass/core/models.py +++ b/mass/core/models.py @@ -85,10 +85,10 @@ class SortOrder(Enum): class SortingParameter(BaseModel): """Represents a combination of a field to sort and the sort order""" - sort_field: str = Field( + field: str = Field( ..., description=("Which field to sort results by."), ) - sort_order: SortOrder = Field( + order: SortOrder = Field( default=SortOrder.ASCENDING, description="Sort order to apply to sort_field" ) diff --git a/mass/core/query_handler.py b/mass/core/query_handler.py index 7975b27..7e64888 100644 --- a/mass/core/query_handler.py +++ b/mass/core/query_handler.py @@ -78,11 +78,9 @@ async def handle_query( sorting_parameters = [] # if id_ is not in sorting_parameters, add to end - if "id_" not in [param.sort_field for param in sorting_parameters]: + if "id_" not in [param.field for param in sorting_parameters]: sorting_parameters.append( - models.SortingParameter( - sort_field="id_", sort_order=models.SortOrder.ASCENDING - ) + models.SortingParameter(field="id_", order=models.SortOrder.ASCENDING) ) # get configured facet fields for given resource class From 1eeb1c6be17ff78a521d01f9c309038a6b21e59a Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Mon, 4 Sep 2023 13:45:04 +0000 Subject: [PATCH 12/14] Update openapi --- openapi.yaml | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index da5e1a5..d9375e9 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -161,8 +161,8 @@ components: type: integer sorting_parameters: default: - - sort_field: id_ - sort_order: 1 + - field: id_ + order: ascending description: Collection of sorting parameters used to refine search results items: $ref: '#/components/schemas/SortingParameter' @@ -193,24 +193,23 @@ components: SortOrder: description: Represents the possible sorting orders enum: - - 1 - - -1 + - ascending + - descending title: SortOrder - type: integer SortingParameter: description: Represents a combination of a field to sort and the sort order properties: - sort_field: + field: description: Which field to sort results by. - title: Sort Field + title: Field type: string - sort_order: + order: allOf: - $ref: '#/components/schemas/SortOrder' - default: 1 + default: ascending description: Sort order to apply to sort_field required: - - sort_field + - field title: SortingParameter type: object ValidationError: From 4cb22d378aab6e08859e86176d8bb317cbc38cbb Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Mon, 4 Sep 2023 13:48:03 +0000 Subject: [PATCH 13/14] Update tests --- tests/test_sorting.py | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/tests/test_sorting.py b/tests/test_sorting.py index f529617..7f94af9 100644 --- a/tests/test_sorting.py +++ b/tests/test_sorting.py @@ -23,7 +23,7 @@ CLASS_NAME = "SortingTests" BASIC_SORT_PARAMETERS = [ - models.SortingParameter(sort_field="id_", sort_order=models.SortOrder.ASCENDING) + models.SortingParameter(field="id_", order=models.SortOrder.ASCENDING) ] @@ -34,14 +34,14 @@ def sort_resources( sorted_list = resources.copy() for parameter in sorts: - reverse = True if parameter.sort_order == -1 else False + reverse = True if parameter.order == models.SortOrder.DESCENDING else False - if parameter.sort_field == "id_": + 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.sort_field], + key=lambda resource: resource.dict()["content"][parameter.field], reverse=reverse, ) @@ -74,8 +74,8 @@ async def test_sort_with_id_not_last(joint_fixture: JointFixture): any bugs that will break the sort or query process. """ sorts = [ - {"sort_field": "id_", "sort_order": 1}, - {"sort_field": "field", "sort_order": -1}, + {"field": "id_", "order": models.SortOrder.ASCENDING.value}, + {"field": "field", "order": models.SortOrder.DESCENDING.value}, ] search_parameters: JsonObject = { "class_name": CLASS_NAME, @@ -102,7 +102,9 @@ async def test_sort_with_params_but_not_id(joint_fixture: JointFixture): "class_name": CLASS_NAME, "query": "", "filters": [], - "sorting_parameters": [{"sort_field": "field", "sort_order": 1}], + "sorting_parameters": [ + {"field": "field", "order": models.SortOrder.ASCENDING.value} + ], } results = await joint_fixture.call_search_endpoint(search_parameters) @@ -122,22 +124,27 @@ async def test_sort_with_invalid_field(joint_fixture: JointFixture): "class_name": CLASS_NAME, "query": "", "filters": [], - "sorting_parameters": [{"sort_field": "some_bogus_field", "sort_order": 1}], + "sorting_parameters": [ + { + "field": "some_bogus_field", + "order": models.SortOrder.ASCENDING.value, + } + ], } results = await joint_fixture.call_search_endpoint(search_parameters) assert results.hits == sort_resources(results.hits, BASIC_SORT_PARAMETERS) -@pytest.mark.parametrize("sort_order", [-7, 17, "some_string"]) +@pytest.mark.parametrize("order", [-7, 17, "some_string"]) @pytest.mark.asyncio -async def test_sort_with_invalid_sort_order(joint_fixture: JointFixture, sort_order): +async def test_sort_with_invalid_sort_order(joint_fixture: JointFixture, order): """Test supplying an invalid value for the sort order""" search_parameters: JsonObject = { "class_name": CLASS_NAME, "query": "", "filters": [], - "sorting_parameters": [{"sort_field": "field", "sort_order": sort_order}], + "sorting_parameters": [{"field": "field", "order": order}], } response = await joint_fixture.rest_client.post( @@ -153,7 +160,7 @@ async def test_sort_with_invalid_field_and_sort_order(joint_fixture: JointFixtur "class_name": CLASS_NAME, "query": "", "filters": [], - "sorting_parameters": [{"sort_field": "some_bogus_field", "sort_order": -7}], + "sorting_parameters": [{"field": "some_bogus_field", "order": -7}], } response = await joint_fixture.rest_client.post( @@ -173,8 +180,8 @@ async def test_sort_with_duplicate_field(joint_fixture: JointFixture): "query": "", "filters": [], "sorting_parameters": [ - {"sort_field": "field", "sort_order": 1}, - {"sort_field": "field", "sort_order": 1}, + {"field": "field", "order": models.SortOrder.ASCENDING.value}, + {"field": "field", "order": models.SortOrder.DESCENDING.value}, ], } response = await joint_fixture.rest_client.post( From aba8f1fe5d021037307efe252f80428a80e3229d Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Mon, 4 Sep 2023 13:57:17 +0000 Subject: [PATCH 14/14] Tweak model creation in tests --- tests/test_sorting.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_sorting.py b/tests/test_sorting.py index 7f94af9..0a71772 100644 --- a/tests/test_sorting.py +++ b/tests/test_sorting.py @@ -74,8 +74,8 @@ async def test_sort_with_id_not_last(joint_fixture: JointFixture): any bugs that will break the sort or query process. """ sorts = [ - {"field": "id_", "order": models.SortOrder.ASCENDING.value}, - {"field": "field", "order": models.SortOrder.DESCENDING.value}, + {"field": "id_", "order": "ascending"}, + {"field": "field", "order": "descending"}, ] search_parameters: JsonObject = { "class_name": CLASS_NAME, @@ -84,7 +84,12 @@ async def test_sort_with_id_not_last(joint_fixture: JointFixture): "sorting_parameters": sorts, } - sorts_in_model_form = [models.SortingParameter(**param) for param in sorts] + sorts_in_model_form = [ + models.SortingParameter( + field=param["field"], order=models.SortOrder(param["order"]) + ) + for param in sorts + ] results = await joint_fixture.call_search_endpoint(search_parameters) assert results.hits == sort_resources(results.hits, sorts_in_model_form)