[Policy API] Response when returning a single object is not clear

[fractalf]

Ref: https://github.com/openmobilityfoundation/mobility-data-specification/blob/main/policy/README.md#rest-endpoints

I think the specs should be more clear about how the exact response should be when it comes to returning a single object vs multiple. In fact, each of these cases should have their own little section:

• /policies
• /policies/{id}
• /geographies
• /geographies/{id}

When "bundling" the docs on single/multiple endpoints, confusion and conflicts arise. An example: id is listed as a query parameter along with start_date and end_date. Though this is not really a query parameter, but part of the endpoint.

I studied the code in mds-core and found some undocumented responses when it comes to single objects

/policies

        res.status(200).send({ version: res.locals.version, data: { policies: active } })

=> Would produce expected result according to docs

/policies/{id}

        res.status(200).send({ version: res.locals.version, data: { policy } })

=> Would produce something not expected or documentet, the data object would contain a policy property

/geographies

        return res.status(200).send({ version: res.locals.version, data: { geographies } })

=> Would produce expected result according to docs

/geographies/{id}

        return res.status(200).send({ version: res.locals.version, data: { geography } })

=> Would produce something not expected or documentet, the data object would contain a geography property

Thank you

[pxlrbt]

The spec also varies between the documents:

policy/README.md defines all responses in plural form:

{
    "data": {
        "policies": [
         ]
    }
}

JSON Schema and Examples define all responses in singular form, but as an array:

{
    "data": {
        "policy": [
        ]
    }
}

MDS Core implementation is using singular form for single items and returns an object vs. plural form for multiple items returning an array.

{
    "data": {
        "policies": [
         ]
    }
}

{
    "data": {
        "policy": {
        }
    }
}

In accordance with JSON:API spec it should be neither of those.
It states the following structure for collections:

{
  "data": [{
    "type": "policies",
    "attributes": {
     }
  }]
},

And this structure for single resources:
It states the following structure for collections:

{
  "data": {
    "type": "policies",
    "attributes": {
     }
  }
},

[thekaveman]

@fractalf @pxlrbt these are great finds, and I'm kicking myself for not noticing these differences - esp. between the README/examples/schema (all of which I have touched in the recent past!)

@schnuerle this feels like something that should be cleaned up as part of the forthcoming 1.1 release, but theoretically could break somebody since there is a mismatch between the docs and schema.

@quicklywilliam @karcass I think you all have been working on the implementation side the most, any thoughts?

[schnuerle]

Ok I'm going to mark it as 1.2.0 for now. This may be more of a bug than a change to be made that is breaking. If it's a bug, then I think we can include it in 1.2.0 just like it was a hotfix, and we can make a clear note of this in the release notes.

[pxlrbt]

If it's a bug, then I think we can include it in 1.2.0 just like it was a hotfix, and we can make a clear note of this in the release notes.

Probably a hard decision. I'd consider it a breaking change when the mds-core version is used. But I can also imagine, that many people refer to mds-core for clarification, and chaging mds-core could probably result in more breakage.

[schnuerle]

Can anyone create a pull request to fix this in the next week or two? Then it could included in the 1.2.0 release which is wrapping up.

[marie-x]

@schnuerle either I will do it, or I'll find a colleague to do it.

[schnuerle]

Can you get a PR made for this before Thursday's WG meeting, @marie-x? We are at the end of the 1.2 release now.

[avatarneil]

We propose that we'll just fix the JSON Schema definitions & examples, and have it so regardless of the plurality the return payload is data: { policies: [...] }. This means that we can make this change in a non-breaking fashion for 1.2. I'll have a PR up by Thursday for this, unless anyone objects!

For 2.0, we may want to consider not having the policies property inside of a data object, and instead return data: [/* 1 or many policies */] in order to better conform with the JSONAPI spec.

[schnuerle]

Per our working group meeting this fix in PR #693 was agreed upon. We will merge to dev by the end of the week.

@avatarneil if you'd like to open a new separate issue to capture what you are proposing for 2.0, that would be helpful.

[schnuerle]

Fixed with #693.