Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The current spec (inadvertently?) allows non-standard links, which are used in the wild #1656

Closed
cmeeren opened this issue Aug 17, 2022 · 4 comments

Comments

@cmeeren
Copy link
Contributor

cmeeren commented Aug 17, 2022

Both the 1.0 and 1.1 specs say:

The optional links member within each resource object contains links related to the resource.

If present, this links object MAY contain a self link that identifies the resource represented by the resource object.

Specifically, this wording does not prohibit additional members in the links object. (For that, the spec would have to either say something like "MAY ONLY contain", or add a sentence saying that the links object "MAY NOT contain additional members".) However, in #1019 (comment), @ethanresnick stated that this is forbidden.

I have implemented the now two-and-a-half year old JSON:API framework Felicity. Based on my understanding of the spec, it has always allowed custom links, which we use in many of our APIs. I have essentially used custom links in cases where representing state transitions via resource creation or normal PATCH update seemed too heavy-handed. For example, in an API representing a directory tree, folder resources have a makeAllDescendantsInheritAccess link that supports POST to make all child files/folders inherit its access rules. Its presence or absence indicates whether the operation is currently allowed by the requesting user. We also have a download link on file resources that serves the actual file contents (i.e., does not return a JSON:API response). I have many more examples if need be.

I just wanted to let you know that custom links are easily interpreted as allowed by the current spec, and that these exist in the wild. The spec may need clarification on this point if you really want them to be forbidden (which may cause breaking changes in APIs in the wild, preventing them from targeting a new JSON:API version and still be spec compliant).

@jelhan
Copy link
Contributor

jelhan commented Aug 17, 2022

Both v1.0 and v1.1 state in addition:

Unless otherwise noted, objects defined by this specification MUST NOT contain any additional members. Client and server implementations MUST ignore members not recognized by this specification.

This forbids any additional members in the links object.

In v1.1 your use case is supported using an extension, which defines additional members for links object of a resource.

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 17, 2022

Both v1.0 and v1.1 state in addition:

Unless otherwise noted, objects defined by this specification MUST NOT contain any additional members. Client and server implementations MUST ignore members not recognized by this specification.

This forbids any additional members in the links object.

My bad. I totally missed the connection between these two. Thank you for clarifying this.

In v1.1 your use case is supported using an extension, which defines additional members for links object of a resource.

Wouldn't the extension have to add a new myNamespace:links member? Or at the very least have all links prefixed with a namespace? That would then be a breaking change for the existing APIs anyway.

@jelhan
Copy link
Contributor

jelhan commented Aug 17, 2022

Wouldn't the extension have to add a new myNamespace:links member? Or at the very least have all links prefixed with a namespace? That would then be a breaking change for the existing APIs anyway.

An extension is also allowed to define additional members in the links object itself. Of course every member must be prefixed by extension namespace. It would look like this:

{
  "data": {
    "type": "folder",
    "id": "foo",
    "links": {
      "self": "/folder/foo"
      "fs:makeAllDescendantsInheritAccess": "/folder/foo/make-all-descendants-inherit-access"
    }
  }
}

In this case I assume that the extension is about file systems. It uses the fs namespace. As an alternative you could also have a less-domain specific extension for action links in general using an actions namespace.

If I got it right, your additional members in links object are not prefixed with extension namespace yet. In that case you would either need to accept a breaking change for existing API or continue to violate the specification in that regard. To be honest in some cases the latter might also be acceptable.

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 17, 2022

If I got it right, your additional members in links object are not prefixed with extension namespace yet. In that case you would either need to accept a breaking change for existing API or continue to violate the specification in that regard. To be honest in some cases the latter might also be acceptable.

Yes, that is probably what I'll have to do. That also means that my JSON:API framework, Felicity, will have to continue supporting this feature.

Thanks for the clarifications!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants