fix(robot-server): Fix None ambiguity in labware offset filtering

[SyntaxColoring]

Overview

This fixes an unreleased bug in the /labwareOffsets API.

This API lets you filter on a bunch of independent fields. Internally, we were representing the default condition, where there is no filter on a field, as None. But that's ambiguous for the location.moduleModel and location.definitionUri fields, where None corresponds to JSON null, which is a possible legitimate value that you might want to filter for.

So:

• Internally to our Python, use a sentinel value, distinct from None, to represent the default "do not filter" condition.
  • Do a little bit of FastAPI trickery to stop the sentinel from messing up the docs.
• At the HTTP level, expect these filter query parameters to be JSON-encoded. The pattern is:
  • ?someField=null to filter for JSON "someField": null
  • ?someField="foo" to filter for JSON "someField": "foo"
  • ?someField="null" to filter for JSON "someField": "null"
  • omit the someField query param to not filter on someField at all

Test Plan and Hands on Testing

• Run a couple of quick requests in Postman to make sure the FastAPI trickery doesn't totally break things.
• Add integration tests.
• Check the generated docs and make sure they render reasonably.

Review requests

Is this a good way to implement this?

If you provide a completely invalid value, like this:

GET /labwareOffsets?locationModuleModel=123

Then the internal sentinel leaks out and causes a confusing error message:

{
    "errors": [
        {
            "id": "InvalidRequest",
            "title": "Invalid Request",
            "detail": "Input should be 'temperatureModuleV1', 'temperatureModuleV2', 'magneticModuleV1', 'magneticModuleV2', 'thermocyclerModuleV1', 'thermocyclerModuleV2', 'heaterShakerModuleV1', 'magneticBlockV1' or 'absorbanceReaderV1'",
            "source": {
                "parameter": "locationModuleModel"
            },
            "errorCode": "4000"
        },
        {
            "id": "InvalidRequest",
            "title": "Invalid Request",
            "detail": "Input should be a subclass of DO_NOT_FILTER",
            "source": {
                "parameter": "locationModuleModel"
            },
            "errorCode": "4000"
        }
    ]
}

Which is unfortunate, but I couldn't find a way to fix it.

Risk assessment

Low. This code is currently unused.

[sfoster1]

Interesting. Looks like a good way to solve the problem. I guess we can't use normal pydantic queries about whether the field was set because they're fastapi url params?

[TamarZanzouri]

Love it! Thank you!

[SyntaxColoring]

I guess we can't use normal pydantic queries about whether the field was set because they're fastapi url params?

Yeah exactly.

Although, as I look at it again, I might have been able to get that to work if I used a query parameter model. [Edit: Not available in our FastAPI version.]

[SyntaxColoring]

OK it turns out this is a little more complicated. FastAPI+Pydantic do not let clients request ?someQueryParam=null to create a value of None on the server. This makes sense—it's a query string, not necessarily JSON. If we want to accept null, we need to special-case Literal["null"]. That highlights that the string "null" could never be used as one of these values, which is fine in this case, but is icky enough to make me second-guess the API pattern. The alternative would be to say that the query param needs to be JSON-encoded, like ?someQueryParam=null vs. ?someQueryParam="aString", which is gross in other ways. Re-drafting to look into it and add some tests. Went with the JSON-encoded option after feedback from @mjhuff. PR description updated.

[mjhuff]

Looks great! Thank you!