diff --git a/robot-server/robot_server/labware_offsets/router.py b/robot-server/robot_server/labware_offsets/router.py index 6aca03e4b18..97b237064a1 100644 --- a/robot-server/robot_server/labware_offsets/router.py +++ b/robot-server/robot_server/labware_offsets/router.py @@ -3,9 +3,11 @@ from datetime import datetime import textwrap -from typing import Annotated, Literal +from typing import Annotated, Literal, Type import fastapi +from pydantic import Json +from pydantic.json_schema import SkipJsonSchema from server_utils.fastapi_utils.light_router import LightRouter from opentrons.protocol_engine import LabwareOffset, LabwareOffsetCreate, ModuleModel @@ -22,7 +24,7 @@ SimpleMultiBody, ) -from .store import LabwareOffsetNotFoundError, LabwareOffsetStore +from .store import DO_NOT_FILTER, LabwareOffsetNotFoundError, LabwareOffsetStore from .fastapi_dependencies import get_labware_offset_store @@ -76,11 +78,11 @@ async def post_labware_offset( # noqa: D103 async def get_labware_offsets( # noqa: D103 store: Annotated[LabwareOffsetStore, fastapi.Depends(get_labware_offset_store)], id: Annotated[ - str | None, + Json[str] | SkipJsonSchema[Type[DO_NOT_FILTER]], fastapi.Query(description="Filter for exact matches on the `id` field."), - ] = None, + ] = DO_NOT_FILTER, definition_uri: Annotated[ - str | None, + Json[str] | SkipJsonSchema[Type[DO_NOT_FILTER]], fastapi.Query( alias="definitionUri", description=( @@ -88,23 +90,23 @@ async def get_labware_offsets( # noqa: D103 " (Not to be confused with `location.definitionUri`.)" ), ), - ] = None, + ] = DO_NOT_FILTER, location_slot_name: Annotated[ - DeckSlotName | None, + Json[DeckSlotName] | SkipJsonSchema[Type[DO_NOT_FILTER]], fastapi.Query( alias="locationSlotName", description="Filter for exact matches on the `location.slotName` field.", ), - ] = None, + ] = DO_NOT_FILTER, location_module_model: Annotated[ - ModuleModel | None, + Json[ModuleModel | None] | SkipJsonSchema[Type[DO_NOT_FILTER]], fastapi.Query( alias="locationModuleModel", description="Filter for exact matches on the `location.moduleModel` field.", ), - ] = None, + ] = DO_NOT_FILTER, location_definition_uri: Annotated[ - str | None, + Json[str | None] | SkipJsonSchema[Type[DO_NOT_FILTER]], fastapi.Query( alias="locationDefinitionUri", description=( @@ -112,9 +114,9 @@ async def get_labware_offsets( # noqa: D103 " (Not to be confused with just `definitionUri`.)" ), ), - ] = None, + ] = DO_NOT_FILTER, cursor: Annotated[ - int | None, + int | SkipJsonSchema[None], fastapi.Query( description=( "The first index to return out of the overall filtered result list." diff --git a/robot-server/robot_server/labware_offsets/store.py b/robot-server/robot_server/labware_offsets/store.py index 9f7e76cfd57..93968a30986 100644 --- a/robot-server/robot_server/labware_offsets/store.py +++ b/robot-server/robot_server/labware_offsets/store.py @@ -1,9 +1,21 @@ # noqa: D100 +from typing import Type + from opentrons.protocol_engine import LabwareOffset, ModuleModel from opentrons.types import DeckSlotName +class DO_NOT_FILTER: + """A sentinel value for when a filter should not be applied. + + This is different from filtering on `None`, which returns only entries where the + value is equal to `None`. + """ + + pass + + # todo(mm, 2024-12-06): Convert to be SQL-based and persistent instead of in-memory. # https://opentrons.atlassian.net/browse/EXEC-1015 class LabwareOffsetStore: @@ -19,11 +31,15 @@ def add(self, offset: LabwareOffset) -> None: def search( self, - id_filter: str | None, - definition_uri_filter: str | None, - location_slot_name_filter: DeckSlotName | None, - location_module_model_filter: ModuleModel | None, - location_definition_uri_filter: str | None, + id_filter: str | Type[DO_NOT_FILTER] = DO_NOT_FILTER, + definition_uri_filter: str | Type[DO_NOT_FILTER] = DO_NOT_FILTER, + location_slot_name_filter: DeckSlotName | Type[DO_NOT_FILTER] = DO_NOT_FILTER, + location_module_model_filter: ModuleModel + | None + | Type[DO_NOT_FILTER] = DO_NOT_FILTER, + location_definition_uri_filter: str + | None + | Type[DO_NOT_FILTER] = DO_NOT_FILTER, # todo(mm, 2024-12-06): Support pagination (cursor & pageLength query params). # The logic for that is currently duplicated across several places in # robot-server and api. We should try to clean that up, or at least avoid @@ -33,13 +49,14 @@ def search( def is_match(candidate: LabwareOffset) -> bool: return ( - id_filter in (None, candidate.id) - and definition_uri_filter in (None, candidate.definitionUri) - and location_slot_name_filter in (None, candidate.location.slotName) + id_filter in (DO_NOT_FILTER, candidate.id) + and definition_uri_filter in (DO_NOT_FILTER, candidate.definitionUri) + and location_slot_name_filter + in (DO_NOT_FILTER, candidate.location.slotName) and location_module_model_filter - in (None, candidate.location.moduleModel) + in (DO_NOT_FILTER, candidate.location.moduleModel) and location_definition_uri_filter - in (None, candidate.location.definitionUri) + in (DO_NOT_FILTER, candidate.location.definitionUri) ) return [ diff --git a/robot-server/tests/integration/http_api/test_labware_offsets.tavern.yaml b/robot-server/tests/integration/http_api/test_labware_offsets.tavern.yaml index d9ff35d7136..f84e5b15d56 100644 --- a/robot-server/tests/integration/http_api/test_labware_offsets.tavern.yaml +++ b/robot-server/tests/integration/http_api/test_labware_offsets.tavern.yaml @@ -83,7 +83,7 @@ stages: # Just a basic test here. More complicated tests for the filters belong in the unit tests. - name: Test getting labware offsets with a filter request: - url: '{ot3_server_base_url}/labwareOffsets?locationSlotName=A2' + url: '{ot3_server_base_url}/labwareOffsets?locationSlotName="A2"' method: GET response: json: @@ -129,3 +129,122 @@ stages: meta: cursor: 0 totalLength: 0 + +--- +# Some of the filter query parameters can have `null` values or be omitted, +# with different semantics between the two. That distinction takes a bit of care to +# preserve across our code, so here we test it specifically. +test_name: Test null vs. omitted filter query parameters +marks: + - usefixtures: + - ot3_server_base_url +stages: + - name: POST test offset 1 + request: + method: POST + url: '{ot3_server_base_url}/labwareOffsets' + json: + data: + definitionUri: testNamespace/loadName1/1 + location: + slotName: A1 + # No moduleModel + # No definitionUri + vector: + x: 1 + y: 2 + z: 3 + response: + status_code: 201 + save: + json: + offset_1_data: data + - name: POST test offset 2 + request: + method: POST + url: '{ot3_server_base_url}/labwareOffsets' + json: + data: + definitionUri: testNamespace/loadName2/1 + location: + slotName: A1 + moduleModel: temperatureModuleV2 + # No definitionUri + vector: + x: 1 + y: 2 + z: 3 + response: + status_code: 201 + save: + json: + offset_2_data: data + - name: POST test offset 3 + request: + method: POST + url: '{ot3_server_base_url}/labwareOffsets' + json: + data: + definitionUri: testNamespace/loadName2/1 + location: + slotName: A1 + # no moduleModel + definitionUri: testNamespace/adapterLoadName/1 + vector: + x: 1 + y: 2 + z: 3 + response: + status_code: 201 + save: + json: + offset_3_data: data + - name: POST test offset 4 + request: + method: POST + url: '{ot3_server_base_url}/labwareOffsets' + json: + data: + definitionUri: testNamespace/loadName3/1 + location: + slotName: A1 + moduleModel: temperatureModuleV2 + definitionUri: testNamespace/adapterLoadName/1 + vector: + x: 1 + y: 2 + z: 3 + response: + status_code: 201 + save: + json: + offset_4_data: data + - name: Test no filters + request: + url: '{ot3_server_base_url}/labwareOffsets' + response: + json: + data: + - !force_format_include '{offset_1_data}' + - !force_format_include '{offset_2_data}' + - !force_format_include '{offset_3_data}' + - !force_format_include '{offset_4_data}' + meta: !anydict + - name: Test filtering on locationModuleModel=null + request: + url: '{ot3_server_base_url}/labwareOffsets?locationModuleModel=null' + response: + json: + data: + - !force_format_include '{offset_1_data}' + - !force_format_include '{offset_3_data}' + meta: !anydict + - name: Test filtering on locationDefinitionUri=null + request: + url: '{ot3_server_base_url}/labwareOffsets?locationDefinitionUri=null' + response: + json: + data: + - !force_format_include '{offset_1_data}' + - !force_format_include '{offset_2_data}' + meta: !anydict diff --git a/robot-server/tests/labware_offsets/test_store.py b/robot-server/tests/labware_offsets/test_store.py index 0f28f2e6825..9f122e2930a 100644 --- a/robot-server/tests/labware_offsets/test_store.py +++ b/robot-server/tests/labware_offsets/test_store.py @@ -16,13 +16,7 @@ def _get_all(store: LabwareOffsetStore) -> list[LabwareOffset]: - return store.search( - id_filter=None, - definition_uri_filter=None, - location_definition_uri_filter=None, - location_module_model_filter=None, - location_slot_name_filter=None, - ) + return store.search() def test_filters() -> None: @@ -52,25 +46,10 @@ def test_filters() -> None: subject.add(labware_offset) # No filters: - assert ( - subject.search( - id_filter=None, - definition_uri_filter=None, - location_definition_uri_filter=None, - location_module_model_filter=None, - location_slot_name_filter=None, - ) - == labware_offsets - ) + assert subject.search() == labware_offsets # Filter on one thing: - result = subject.search( - id_filter=None, - definition_uri_filter="definition-uri-b", - location_definition_uri_filter=None, - location_module_model_filter=None, - location_slot_name_filter=None, - ) + result = subject.search(definition_uri_filter="definition-uri-b") assert len(result) == 3 assert result == [ entry for entry in labware_offsets if entry.definitionUri == "definition-uri-b" @@ -80,9 +59,6 @@ def test_filters() -> None: result = subject.search( id_filter="id-2", definition_uri_filter="definition-uri-b", - location_definition_uri_filter=None, - location_module_model_filter=None, - location_slot_name_filter=None, ) assert result == [labware_offsets[1]] @@ -90,9 +66,6 @@ def test_filters() -> None: result = subject.search( id_filter="id-1", definition_uri_filter="definition-uri-b", - location_definition_uri_filter=None, - location_module_model_filter=None, - location_slot_name_filter=None, ) assert result == []