stapi-spec / stapi-fastapi

Spatio Temporal Asset Tasking with FastAPI
MIT License
12 stars 4 forks source link

Discrepancy between product parameters and opportunities properties models #75

Open jkeifer opened 1 month ago

jkeifer commented 1 month ago

To preface this issue, I fully recognize the spec is currently vague regarding the proper behavior here. I am optimistic we will be able to clarify what is intended at the upcoming sprint to better inform the resolution of this issue. Potentially-related spec issues: https://github.com/stapi-spec/stapi-spec/issues/201, https://github.com/stapi-spec/stapi-spec/issues/199.

Anyway, it seems that the current implementation does not properly handle the difference in Product parameter definitions and how those parameters are represented on opportunities. It is my understanding that the spec expects parameters on products to generally be specified as single, flat values. A reference I've been using is the Blacksky standard product example from the spec repo: https://github.com/stapi-spec/stapi-spec/blob/main/product/examples/ProductCollectionBlackSky.json. Specifically, here is a portion of the parameters object on that product:

{
    "$schema": "https://json-schema.org/draft-07/schema",
    "type": "object",
    "$defs": {
        ...
    },
    "properties": {
        ...
        "view:off_nadir": {
            "type": "integer",
            "description": "Angle between the sub-satellite point, satellite, and target (degrees)",
            "minimum": 0,
            "maximum": 30
        },
        ...
    }
}

Notice that the continuous parameter values on that product are expressed as a single value. Compare to the example product parameters from the TLE backend, as currently returned from the API:

{
    "$defs": {
        "OffNadirRange": {
            "properties": {
                "minimum": {
                    "maximum": 45,
                    "minimum": 0,
                    "title": "Minimum",
                    "type": "number"
                },
                "maximum": {
                    "maximum": 45,
                    "minimum": 0,
                    "title": "Maximum",
                    "type": "number"
                }
            },
            "required": [
                "minimum",
                "maximum"
            ],
            "title": "OffNadirRange",
            "type": "object"
        }
    },
    "properties": {
        "off_nadir": {
            "$ref": "#/$defs/OffNadirRange",
            "default": {
                "minimum": 0,
                "maximum": 30
            }
        }
    },
    "title": "Parameters",
    "type": "object"
}

Notice how the Blacksky off nadir is a single value within the specified range, whereas the TLE backend parameters currently represent the off nadir angle as an object with both a minimum value and a maximum value?

Of course, it seems that the spec intends that opportunities models represent a parameter such as the off nadir angle, or any other values with some measure of uncertainty, as a range property with a minimum and a maximum. In other words, the TLE model for off nadir does appear to correctly represent how that parameter should be structured within an opportunity's properties object.

I am struggling with this issue at the moment. It certainly seems that the solution requires having two models for each "range" parameter: one to represent it in the product parameters jsonschema, and another to validate it within returned opportunities' properties. But the redundancy in implementing two different models per parameter seem so onerous to be a non-starter.

If we can define a way to discriminate range parameters from non-range parameters we might be able to create a solution that dynamically constructs the proper models to validate opportunity properties correctly. This said, I am concerned that such a solution would not allow proper documentation of types for the opportunities endpoints (based on a previous failure trying to derive types for FastAPI endpoints at runtime), such that we would only ever be able to document the most liberal STAPI-compliant opportunity model. This restriction may not be a problem with the current structure of the API where /opportunities cannot be specific to any single product anyway, but if the suggestion from https://github.com/stapi-spec/stapi-spec/issues/198 is adopted then we would definitely want a way to specify the model of a product-specific opportunities endpoint.

c-wygoda commented 1 month ago

What if the intent of the opportunity is to return the requested "limitations" as is? Then that would be just the same model for the base, and we could define an extension/conformance that opportunity responses can include calculated values, something like view:off-nadir which is the actual expected off-nadir at time of opportunity?