opentripmodel / otm5-change-requests

Tracking and reporting bugs and change requests of the OTM5 specification.
5 stars 1 forks source link

geoReference vs geoReferences #86

Open chrissvo opened 7 months ago

chrissvo commented 7 months ago

Type of bug

Describe the bug In most of the spec when we need geolocation we create an object called geoReference. Only in routes we call this object geoReferences. Is that just a typo?

The geographical representation of a route may be a list of coordinates, but that is still a singular geographical reference. Even the spec says it is one of a number of possible geographical representations.

"geoReferences": {
  "description": "Geographic representation of this route.",
  "oneOf": [
    {
      "$ref": "#/components/schemas/latLonPointGeoReference"
    },
    {
      "$ref": "#/components/schemas/latLonArrayGeoReference"
    },
    {
      "$ref": "#/components/schemas/Feature"
    },
    {
      "$ref": "#/components/schemas/openLRGeoReference"
    },
    {
      "$ref": "#/components/schemas/tmcLocationGeoReference"
    },
    {
      "$ref": "#/components/schemas/tmcArrayGeoReference"
    },
    {
      "$ref": "#/components/schemas/addressGeoReference"
    }
  ]
}

To Reproduce https://github.com/opentripmodel/otm5-change-requests/blob/09b28ebfd1473a8a1c21a94ba2c9b96ca2b3d294/otm5_openapi_specification.json#L2595

Expected behavior I would expect that a route has a property called geoReference (like locations and events).

Screenshots CleanShot 2023-11-24 at 19 30 29@2x

CleanShot 2023-11-24 at 19 36 13@2x

Additional context I build a few OTM API's and realised that in one case I implemented this response for a geographical area:

"geoReferences": [{}] // with just a single object

And this response in another to describe a location:

"geoReference": {}

And this response in yet another to describe a route:

"geoReferences": {}

I think the first one is just wrong, but the last one conforms to the spec, yet feels wrong ;)

cc @robbertjanssen85

bmeesters commented 7 months ago

Hello @chrissvo, thanks for your contribution. You are correct that this is inconsistent and should not be done this way. The problem is that parties might lean on the 'typo' and therefore we cannot just change it. However what we can do is introduce a new field geoReference and deprecate geoReferences. It does mean that both fields will be part of the specification, but OpenAPI can clearly indicate which one is deprecated. Would that work for you?

chrissvo commented 7 months ago

Works for me, thanks a lot!