From 8f86a81039d874916091c74d6e69a8b272ef4716 Mon Sep 17 00:00:00 2001 From: Michael Chouinard Date: Fri, 6 Sep 2024 12:10:59 -0400 Subject: [PATCH 1/4] [Issue #188] More filters in search schema --- .../opportunities_v1/opportunity_schemas.py | 44 ++++- .../api/schemas/extension/field_validators.py | 2 + api/src/api/schemas/search_schema.py | 62 ++++++- .../src/api/opportunities_v1/conftest.py | 24 +++ .../test_opportunity_route_search.py | 166 +++++++++++++++++- 5 files changed, 287 insertions(+), 11 deletions(-) diff --git a/api/src/api/opportunities_v1/opportunity_schemas.py b/api/src/api/opportunities_v1/opportunity_schemas.py index e4392e96a..9de3bf40d 100644 --- a/api/src/api/opportunities_v1/opportunity_schemas.py +++ b/api/src/api/opportunities_v1/opportunity_schemas.py @@ -2,7 +2,12 @@ from src.api.schemas.extension import Schema, fields, validators from src.api.schemas.response_schema import AbstractResponseSchema, PaginationMixinSchema -from src.api.schemas.search_schema import DateSearchSchemaBuilder, StrSearchSchemaBuilder +from src.api.schemas.search_schema import ( + BoolSearchSchemaBuilder, + DateSearchSchemaBuilder, + IntegerSearchSchemaBuilder, + StrSearchSchemaBuilder, +) from src.constants.lookup_constants import ( ApplicantType, FundingCategory, @@ -320,6 +325,43 @@ class OpportunitySearchFilterV1Schema(Schema): .with_one_of(example="USAID", minimum_length=2) .build() ) + assistance_listing_number = fields.Nested( + StrSearchSchemaBuilder("AssistanceListingNumberFilterV1Schema") + .with_one_of( + example="45.149", pattern=r"^\d{2}\.\d{2,3}$" + ) # Always of the format ##.## or ##.### + .build() + ) + is_cost_sharing = fields.Nested( + BoolSearchSchemaBuilder("IsCostSharingFilterV1Schema").with_one_of(example=True).build() + ) + expected_number_of_awards = fields.Nested( + IntegerSearchSchemaBuilder("ExpectedNumberAwardsFilterV1Schema") + .with_minimum_value(example=0) + .with_maximum_value(example=25) + .build() + ) + + award_floor = fields.Nested( + IntegerSearchSchemaBuilder("AwardFloorFilterV1Schema") + .with_minimum_value(example=0) + .with_maximum_value(example=10_000) + .build() + ) + + award_ceiling = fields.Nested( + IntegerSearchSchemaBuilder("AwardCeilingFilterV1Schema") + .with_minimum_value(example=0) + .with_maximum_value(example=10_000_000) + .build() + ) + + estimated_total_program_funding = fields.Nested( + IntegerSearchSchemaBuilder("EstimatedTotalProgramFundingFilterV1Schema") + .with_minimum_value(example=0) + .with_maximum_value(example=10_000_000) + .build() + ) post_date = fields.Nested( DateSearchSchemaBuilder("PostDateFilterV1Schema").with_start_date().with_end_date().build() diff --git a/api/src/api/schemas/extension/field_validators.py b/api/src/api/schemas/extension/field_validators.py index 98be35e62..43f87a506 100644 --- a/api/src/api/schemas/extension/field_validators.py +++ b/api/src/api/schemas/extension/field_validators.py @@ -7,6 +7,8 @@ from src.api.schemas.extension.schema_common import MarshmallowErrorContainer from src.validation.validation_constants import ValidationErrorType +Validator = validators.Validator # re-export + class Regexp(validators.Regexp): REGEX_ERROR = MarshmallowErrorContainer( diff --git a/api/src/api/schemas/search_schema.py b/api/src/api/schemas/search_schema.py index 8047ff1ee..ee80e88e5 100644 --- a/api/src/api/schemas/search_schema.py +++ b/api/src/api/schemas/search_schema.py @@ -1,5 +1,5 @@ from enum import StrEnum -from typing import Any, Type +from typing import Any, Pattern, Type from marshmallow import ValidationError, validates_schema @@ -84,9 +84,13 @@ def with_one_of( self, *, allowed_values: Type[StrEnum] | None = None, + pattern: str | Pattern | None = None, example: str | None = None, minimum_length: int | None = None ) -> "StrSearchSchemaBuilder": + if pattern is not None and allowed_values is not None: + raise Exception("Cannot specify both a pattern and allowed_values") + metadata = {} if example: metadata["example"] = example @@ -94,8 +98,16 @@ def with_one_of( # We assume it's just a list of strings if allowed_values is None: params: dict = {"metadata": metadata} + + field_validators: list[validators.Validator] = [] if minimum_length is not None: - params["validate"] = [validators.Length(min=2)] + field_validators.append(validators.Length(min=minimum_length)) + + if pattern is not None: + field_validators.append(validators.Regexp(regex=pattern)) + + if len(field_validators) > 0: + params["validate"] = field_validators list_type: fields.MixinField = fields.String(**params) @@ -109,6 +121,52 @@ def with_one_of( return self +class IntegerSearchSchemaBuilder(BaseSearchSchemaBuilder): + def with_minimum_value( + self, example: int | None = None, positive_only: bool = True + ) -> "IntegerSearchSchemaBuilder": + metadata = {} + if example is not None: + metadata["example"] = example + + field_validators = [] + if positive_only: + field_validators.append(validators.Range(min=0)) + + self.schema_fields["min"] = fields.Integer( + allow_none=True, metadata=metadata, validate=field_validators + ) + return self + + def with_maximum_value( + self, example: int | None = None, positive_only: bool = True + ) -> "IntegerSearchSchemaBuilder": + metadata = {} + if example is not None: + metadata["example"] = example + + field_validators = [] + if positive_only: + field_validators.append(validators.Range(min=0)) + + self.schema_fields["max"] = fields.Integer( + allow_none=True, metadata=metadata, validate=field_validators + ) + return self + + +class BoolSearchSchemaBuilder(BaseSearchSchemaBuilder): + def with_one_of(self, example: bool | None = None) -> "BoolSearchSchemaBuilder": + metadata = {} + if example is not None: + metadata["example"] = example + + self.schema_fields["one_of"] = fields.List( + fields.Boolean(metadata=metadata), allow_none=True + ) + return self + + class DateSearchSchemaBuilder(BaseSearchSchemaBuilder): """ Builder for setting up a filter for a range of dates in the search endpoint schema. diff --git a/api/tests/src/api/opportunities_v1/conftest.py b/api/tests/src/api/opportunities_v1/conftest.py index 8b5e4c977..c71675cf0 100644 --- a/api/tests/src/api/opportunities_v1/conftest.py +++ b/api/tests/src/api/opportunities_v1/conftest.py @@ -37,6 +37,12 @@ def get_search_request( applicant_type_one_of: list[ApplicantType] | None = None, opportunity_status_one_of: list[OpportunityStatus] | None = None, agency_one_of: list[str] | None = None, + assistance_listing_one_of: list[str] = None, + is_cost_sharing_one_of: list[bool | str] | None = None, + expected_number_of_awards: dict | None = None, + award_floor: dict | None = None, + award_ceiling: dict | None = None, + estimated_total_program_funding: dict | None = None, post_date: dict | None = None, close_date: dict | None = None, format: str | None = None, @@ -67,6 +73,24 @@ def get_search_request( if agency_one_of is not None: filters["agency"] = {"one_of": agency_one_of} + if assistance_listing_one_of is not None: + filters["assistance_listing_number"] = {"one_of": assistance_listing_one_of} + + if is_cost_sharing_one_of is not None: + filters["is_cost_sharing"] = {"one_of": is_cost_sharing_one_of} + + if expected_number_of_awards is not None: + filters["expected_number_of_awards"] = expected_number_of_awards + + if award_floor is not None: + filters["award_floor"] = award_floor + + if award_ceiling is not None: + filters["award_ceiling"] = award_ceiling + + if estimated_total_program_funding is not None: + filters["estimated_total_program_funding"] = estimated_total_program_funding + if post_date is not None: filters["post_date"] = post_date diff --git a/api/tests/src/api/opportunities_v1/test_opportunity_route_search.py b/api/tests/src/api/opportunities_v1/test_opportunity_route_search.py index 045fb096c..c1661e431 100644 --- a/api/tests/src/api/opportunities_v1/test_opportunity_route_search.py +++ b/api/tests/src/api/opportunities_v1/test_opportunity_route_search.py @@ -290,7 +290,7 @@ def search_scenario_id_fnc(val): class TestOpportunityRouteSearch(BaseTestClass): - @pytest.fixture(scope="class") + @pytest.fixture(scope="class", autouse=True) def setup_search_data(self, opportunity_index, opportunity_index_alias, search_client): # Load into the search index schema = OpportunityV1Schema() @@ -489,7 +489,7 @@ def setup_search_data(self, opportunity_index, opportunity_index_alias, search_c ids=search_scenario_id_fnc, ) def test_sorting_and_pagination_200( - self, client, api_auth_token, setup_search_data, search_request, expected_results + self, client, api_auth_token, search_request, expected_results ): call_search_and_validate(client, api_auth_token, search_request, expected_results) @@ -705,9 +705,7 @@ def test_sorting_and_pagination_200( ], ids=search_scenario_id_fnc, ) - def test_search_filters_200( - self, client, api_auth_token, setup_search_data, search_request, expected_results - ): + def test_search_filters_200(self, client, api_auth_token, search_request, expected_results): call_search_and_validate(client, api_auth_token, search_request, expected_results) @pytest.mark.parametrize( @@ -773,6 +771,160 @@ def test_search_validate_date_filters_422(self, client, api_auth_token, search_r assert json["message"] == "Validation error" assert error["message"] == "Not a valid date." + @pytest.mark.parametrize( + "search_request", + [ + get_search_request(assistance_listing_one_of=["12.345", "67.89"]), + get_search_request(assistance_listing_one_of=["98.765"]), + get_search_request(assistance_listing_one_of=["67.89", "54.24", "12.345", "86.753"]), + ], + ) + def test_search_validate_assistance_listing_filters_200( + self, client, api_auth_token, search_request + ): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + assert resp.status_code == 200 + + @pytest.mark.parametrize( + "search_request", + [ + get_search_request(assistance_listing_one_of=["12.345", "675.89"]), + get_search_request(assistance_listing_one_of=["hello"]), + get_search_request(assistance_listing_one_of=["67.89", "54.2412"]), + get_search_request(assistance_listing_one_of=["1.1"]), + get_search_request(assistance_listing_one_of=["12.hello"]), + get_search_request(assistance_listing_one_of=["fourfive.sixseveneight"]), + get_search_request(assistance_listing_one_of=["11..11"]), + ], + ) + def test_search_validate_assistance_listing_filters_422( + self, client, api_auth_token, search_request + ): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + assert resp.status_code == 422 + + json = resp.get_json() + error = json["errors"][0] + assert json["message"] == "Validation error" + assert error["message"] == "String does not match expected pattern." + + @pytest.mark.parametrize( + "search_request", + [ + get_search_request(is_cost_sharing_one_of=[True, False]), + get_search_request(is_cost_sharing_one_of=["1", "0"]), + get_search_request(is_cost_sharing_one_of=["t", "f"]), + get_search_request(is_cost_sharing_one_of=["true", "false"]), + get_search_request(is_cost_sharing_one_of=["on", "off"]), + get_search_request(is_cost_sharing_one_of=["yes", "no"]), + ], + ) + def test_search_validate_is_cost_sharing_200(self, client, api_auth_token, search_request): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + assert resp.status_code == 200 + + @pytest.mark.parametrize( + "search_request", + [ + get_search_request(is_cost_sharing_one_of=["hello"]), + get_search_request(is_cost_sharing_one_of=[True, "definitely"]), + get_search_request(is_cost_sharing_one_of=[5, 6]), + get_search_request(is_cost_sharing_one_of=["2024-01-01"]), + get_search_request(is_cost_sharing_one_of=[{}]), + ], + ) + def test_search_validate_is_cost_sharing_filters_422( + self, client, api_auth_token, search_request + ): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + assert resp.status_code == 422 + + json = resp.get_json() + error = json["errors"][0] + assert json["message"] == "Validation error" + assert error["message"] == "Not a valid boolean." + + @pytest.mark.parametrize( + "search_request", + [ + get_search_request( + expected_number_of_awards={"min": 0}, + award_floor={"max": 35}, + award_ceiling={"max": "10000000"}, + estimated_total_program_funding={"min": "123456"}, + ), + get_search_request( + expected_number_of_awards={"min": 1, "max": 2}, + award_floor={"min": 0, "max": 1000}, + award_ceiling={"min": 10000, "max": 10000000}, + estimated_total_program_funding={"min": 123456, "max": 345678}, + ), + get_search_request(expected_number_of_awards={"min": 1, "max": 2}), + get_search_request(award_floor={"min": 0, "max": 1000}), + get_search_request(award_ceiling={"min": "10000", "max": 10000000}), + get_search_request(estimated_total_program_funding={"min": 123456, "max": "345678"}), + ], + ) + def test_search_validate_award_values_200(self, client, api_auth_token, search_request): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + assert resp.status_code == 200 + + @pytest.mark.parametrize( + "search_request", + [ + get_search_request(estimated_total_program_funding={"min": "hello", "max": "345678"}), + get_search_request(award_floor={"min": "one"}), + get_search_request(award_ceiling={"min": {}, "max": "123e4f5"}), + ], + ) + def test_search_validate_award_values_422(self, client, api_auth_token, search_request): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + assert resp.status_code == 422 + + json = resp.get_json() + assert json["message"] == "Validation error" + for error in json["errors"]: + assert error["message"] == "Not a valid integer." + + @pytest.mark.parametrize( + "search_request", + [ + get_search_request( + expected_number_of_awards={"min": -1}, + award_floor={"max": -2}, + award_ceiling={"max": "-10000000"}, + estimated_total_program_funding={"min": "-123456"}, + ), + get_search_request(expected_number_of_awards={"min": -1, "max": 10000000}), + get_search_request( + estimated_total_program_funding={"max": "-5"}, award_floor={"max": "-9"} + ), + ], + ) + def test_search_validate_award_values_negative_422( + self, client, api_auth_token, search_request + ): + resp = client.post( + "/v1/opportunities/search", json=search_request, headers={"X-Auth": api_auth_token} + ) + + json = resp.get_json() + assert json["message"] == "Validation error" + for error in json["errors"]: + assert error["message"] == "Must be greater than or equal to 0." + @pytest.mark.parametrize( "search_request, expected_results", [ @@ -831,9 +983,7 @@ def test_search_validate_date_filters_422(self, client, api_auth_token, search_r ], ids=search_scenario_id_fnc, ) - def test_search_query_200( - self, client, api_auth_token, setup_search_data, search_request, expected_results - ): + def test_search_query_200(self, client, api_auth_token, search_request, expected_results): # This test isn't looking to validate opensearch behavior, just that we've connected fields properly and # results being returned are as expected. call_search_and_validate(client, api_auth_token, search_request, expected_results) From 51f2eb0666784b1c01369caba0a05d677cdd9036 Mon Sep 17 00:00:00 2001 From: Michael Chouinard Date: Fri, 6 Sep 2024 12:33:39 -0400 Subject: [PATCH 2/4] Add examples --- .../opportunities_v1/opportunity_routes.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/api/src/api/opportunities_v1/opportunity_routes.py b/api/src/api/opportunities_v1/opportunity_routes.py index 306272847..678bf2a88 100644 --- a/api/src/api/opportunities_v1/opportunity_routes.py +++ b/api/src/api/opportunities_v1/opportunity_routes.py @@ -98,6 +98,37 @@ }, }, }, + "example5": { + "summary": "Filter by award fields", + "value": { + "filters": { + "expected_number_of_awards": {"min": 5}, + "award_floor": {"min": 10000}, + "award_ceiling": {"max": 1000000}, + "estimated_total_program_funding": {"min": 100000, "max": 250000}, + }, + "pagination": { + "order_by": "opportunity_id", + "page_offset": 1, + "page_size": 25, + "sort_direction": "descending", + }, + }, + }, + "example6": { + "summary": "FIlter by assistance listing numbers", + "value": { + "filters": { + "assistance_listing_number": {"one_of": ["43.001", "47.049"]}, + }, + "pagination": { + "order_by": "opportunity_id", + "page_offset": 1, + "page_size": 25, + "sort_direction": "descending", + }, + }, + }, } From 43137f072412a5df0ab1a860715746184fb6a6be Mon Sep 17 00:00:00 2001 From: Michael Chouinard Date: Fri, 6 Sep 2024 12:41:16 -0400 Subject: [PATCH 3/4] Adding some docs --- api/src/api/schemas/search_schema.py | 63 ++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/api/src/api/schemas/search_schema.py b/api/src/api/schemas/search_schema.py index ee80e88e5..35be5a1b6 100644 --- a/api/src/api/schemas/search_schema.py +++ b/api/src/api/schemas/search_schema.py @@ -122,6 +122,37 @@ def with_one_of( class IntegerSearchSchemaBuilder(BaseSearchSchemaBuilder): + """ + Builder for setting up a filter in a search endpoint schema for an integer. + + Our schemas are setup to look like: + + { + "filters": { + "field": { + "min": 1, + "max": 5 + } + } + } + + This helps generate the filters for a given field. At the moment, + only a min and max filter are implemented, and can be used to filter + on a range of values. + + Usage:: + + # In a search request schema, you would use it like so + + class OpportunitySearchFilterSchema(Schema): + example_int_field = fields.Nested( + IntegerSearchSchemaBuilder("ExampleIntFieldSchema") + .with_minimum_value(example=1) + .with_maximum_value(example=25) + .build() + ) + """ + def with_minimum_value( self, example: int | None = None, positive_only: bool = True ) -> "IntegerSearchSchemaBuilder": @@ -156,6 +187,38 @@ def with_maximum_value( class BoolSearchSchemaBuilder(BaseSearchSchemaBuilder): + """ + Builder for setting up a filter in a search endpoint schema. + + Our schemas are setup to look like: + + { + "filters": { + "field": { + "one_of": ["True", "False"] + } + } + } + + This helps generate the filters for a given field. At the moment, + only a one_of filter is implemented - note that any truthy value + as determined by Marshmallow is accepted (including "yes", "y", 1 - for true) + + While it doesn't quite make sense to filter by multiple boolean values in most cases, + we err on the side of consistency with the structure of the query to match other types. + + Usage:: + + # In a search request schema, you would use it like so + + class OpportunitySearchFilterSchema(Schema): + example_bool_field = fields.Nested( + BoolSearchSchemaBuilder("ExampleBoolFieldSchema") + .with_one_of(example=True) + .build() + ) + """ + def with_one_of(self, example: bool | None = None) -> "BoolSearchSchemaBuilder": metadata = {} if example is not None: From 9ae06399db914890cd42a62fd0ad0a9df56f8d78 Mon Sep 17 00:00:00 2001 From: nava-platform-bot Date: Fri, 6 Sep 2024 16:45:32 +0000 Subject: [PATCH 4/4] Update OpenAPI spec --- api/openapi.generated.yml | 141 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml index e6f64c7e0..1dcab55f4 100644 --- a/api/openapi.generated.yml +++ b/api/openapi.generated.yml @@ -220,6 +220,37 @@ paths: page_offset: 1 page_size: 100 sort_direction: ascending + example5: + summary: Filter by award fields + value: + filters: + expected_number_of_awards: + min: 5 + award_floor: + min: 10000 + award_ceiling: + max: 1000000 + estimated_total_program_funding: + min: 100000 + max: 250000 + pagination: + order_by: opportunity_id + page_offset: 1 + page_size: 25 + sort_direction: descending + example6: + summary: FIlter by assistance listing numbers + value: + filters: + assistance_listing_number: + one_of: + - '43.001' + - '47.049' + pagination: + order_by: opportunity_id + page_offset: 1 + page_size: 25 + sort_direction: descending security: - ApiKeyAuth: [] /v0.1/opportunities/search: @@ -831,6 +862,86 @@ components: type: string minLength: 2 example: USAID + AssistanceListingNumberFilterV1: + type: object + properties: + one_of: + type: array + minItems: 1 + items: + type: string + pattern: ^\d{2}\.\d{2,3}$ + example: '45.149' + IsCostSharingFilterV1: + type: object + properties: + one_of: + type: + - array + - 'null' + items: + type: boolean + example: true + ExpectedNumberAwardsFilterV1: + type: object + properties: + min: + type: + - integer + - 'null' + minimum: 0 + example: 0 + max: + type: + - integer + - 'null' + minimum: 0 + example: 25 + AwardFloorFilterV1: + type: object + properties: + min: + type: + - integer + - 'null' + minimum: 0 + example: 0 + max: + type: + - integer + - 'null' + minimum: 0 + example: 10000 + AwardCeilingFilterV1: + type: object + properties: + min: + type: + - integer + - 'null' + minimum: 0 + example: 0 + max: + type: + - integer + - 'null' + minimum: 0 + example: 10000000 + EstimatedTotalProgramFundingFilterV1: + type: object + properties: + min: + type: + - integer + - 'null' + minimum: 0 + example: 0 + max: + type: + - integer + - 'null' + minimum: 0 + example: 10000000 PostDateFilterV1: type: object properties: @@ -885,6 +996,36 @@ components: - object allOf: - $ref: '#/components/schemas/AgencyFilterV1' + assistance_listing_number: + type: + - object + allOf: + - $ref: '#/components/schemas/AssistanceListingNumberFilterV1' + is_cost_sharing: + type: + - object + allOf: + - $ref: '#/components/schemas/IsCostSharingFilterV1' + expected_number_of_awards: + type: + - object + allOf: + - $ref: '#/components/schemas/ExpectedNumberAwardsFilterV1' + award_floor: + type: + - object + allOf: + - $ref: '#/components/schemas/AwardFloorFilterV1' + award_ceiling: + type: + - object + allOf: + - $ref: '#/components/schemas/AwardCeilingFilterV1' + estimated_total_program_funding: + type: + - object + allOf: + - $ref: '#/components/schemas/EstimatedTotalProgramFundingFilterV1' post_date: type: - object