-
Notifications
You must be signed in to change notification settings - Fork 884
Attributes of relationships #415 #431
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
Attributes of relationships #415 #431
Conversation
@hhware Thanks for working up this PR in time for consideration. I personally don't agree with this blurring of lines between resources and relationships. In my opinion, this adds a lot of complexity for little benefit. If relationships are allowed to have attributes, then the spec will need to provide a means to edit those attributes (PATCH I suppose). Furthermore, it will create dissonance for API designers who need to decide between creating resources vs. "complex relationships". |
Moving to 1.0 milestone. @wycats @steveklabnik are either of you in favor? |
@dgeb, thanks for reviewing it!
It is addressed by this PR
IMHO, this "blurring" can be viewed as an intrinsic feature of some real-life systems, which have objects which are "in between" resources and links, but much closer to links. As for the "dissonance", it could indeed be a dissonance to some, but longed-for flexibility to others. There is always a balance between rigidity and flexibility, and I think it would in the spirit of JSON API to be more on a rigid side of things, but not entirely rigid. In my view, allowing simple attributes of relationships is 90% of rigidity and 10% of flexibility. Let me elaborate more with two examples. First one extends the filesystem example from #415. Imagine a CMS-like system (this is the case which I personally care about), consisting of a tree of containers, which can contain either some objects or other containers. A container with all its content can be referred to from another container (to allow people share content without copying it). The difference from the filesystem example is that there is more than one types of such references (to allow different approach to sharing, either with read-only rights, with some admin rights, etc.) Surely, each of this types of sharing links can be represented as an object of its own, necessitating several types of such objects and complexity associated with that, but it would be very convenient for the API designer to hide this complexity and simplify life for himself and for the API consumers by abstracting this as a relationship with just one attribute (type of the link). I wish JSON API allowed to do that. Second one is some modification of audit-like example from #415. Suppose some endpoints of an API represent some committees, and other endpoints of that API represent people, who are members of these committees. People move from one committee to another over time, which can be reflected by Last but not least, |
@hhware Would you be willing to mock up a set of example requests/payloads showing how you'd accomplish your desired interaction within the constraints of json-api now, and a companion set within the constraints you are proposing? I think it would be very helpful to see them side by side. |
This change is additive and can be considered post 1.0. I'm going to pull it from the milestone. |
@tkellen Definitely, I will. |
ac5423d
to
b0f0231
Compare
@hhware Are you still willing to provide examples for this? If so, is there any chance you could do so this week? We are planning on doing a group review in the very near term and I would love to have them as a reference so we can consider this carefully. |
b0f0231
to
7aca000
Compare
@tkellen, yes, I have been working on this PR and examples for the last few weeks, time permitting. I guess it's finally time to let it go :) @dgeb, @tkellen, I very much appreciate your time and interest in reviewing my earlier posts on this topic. I reworked and significantly simplified this proposal to account for the discussion. @lsylvester, I would appreciate your opinion on whether this satisfies your expectations wrt attributes of relationships. Structure of this PRThis PR is split into several commits for convenience of reviewers:
The 4th commit is the example, it can be viewed in readable form here. Key pointsI think the PR now articulates the goals and limitations of the proposed changes much better than before. In particular, I would like to emphasize the following points:
MilestoningI am well aware that the official position is to postpone additive changes until post 1.0. While some additive changes are mere optimizations with available workarounds (e.g., #477), I think it would be appropriate to consider this one for 1.0 because it modifies one of the core concepts of the spec. If relationship attributes were proposed post 1.0 and then accepted, then obviosuly implementors of libraries would have to deal with them as with additive change. As @steveklabnik put: "I can appreciate frustration with changes, we are making them now, so that we can not make them in the future" -- I think it absolutely applies to the particular change proposed here. Another reason to consider this now is that this change is additive for the JSON API spec and its generic implementations, but it will not be additive for specific JSON-API-compliant APIs which could make use of this feature. They will rarely be able to afford a luxury of switching from relationships-as-entities to relationships-with-attributes because it is a backward-incompatible change for an API. Implementors of such APIs make choices on API design and then stick to them. Even if this feature becomes availabe in JSON API 1.X, APIs implemented before that would not be able just to "add" it, the way the spec and libraries can. Obviously, some people are waiting for 1.0 to start using JSON API or to consider it "for real" because of stability expectations -- it is not unreasonable to expect that after 2 years of preparations for 1.0, the core concepts of the spec will remain stable for a while. Commit "substance" is what I believe has to be considered for 1.0. Commit "terminology" would be useful in 1.0 too, but the main reason I provide it here is to avoid rejection of the proposal because of misunderstandings of terminology (the commits are non-overlapping though, so some terminology changes have leaked into other commits as well). Commit "optional" is not blocking for 1.0, but provided here to complete the picture. ExamplesTwo alternatives to relationship attributes have been suggested in discussions in #415 and here:
After having a closer look, I realized that I do not even understand how the first option can be implemented. But even if it could be, I think it is not a workable solution for the following reasons:
So I skip the first option and compare the second one to the proposed solution here. If these examples are unclear or need improvement, please let me know, so I could update them. For completeness sake, other alternatives also exists: define an extension or simply handle these attributes within |
@hhware For my purposes, I wanted to embed information about the linked resource without having to include the linked resource, so the information wasn't particular to the association. The reason for this was to be able to invalidated locally cached resources. I think for my use case, the best approach would be to allow for |
Updated on 4/7/15: added the following: > Linkage objects **MUST** reserve `"links"` member for future use.
7aca000
to
51bb65a
Compare
Made a minor update, left a note in the message of the corresponding commit (to preserve PR structure). |
Ultimately, this is my thought as well. I've read through these tickets (sorry for the delay, work is getting tough) but I don't understand why relationships need attributes. As soon as they do, they become resources in their own right, and don't need special casing. |
We sincerely appreciate your effort here, @hhware. However, after consideration and discussion, @tkellen, @steveklabnik, and myself share the belief that relationships with attributes should become resources in their own right. This has been a tough call as we struggle to balance complexity and utility. An edge case we should consider is the allowance of Thanks again for all your work, @hhware. |
/reffing #794, as the extension system should make something like this possible |
Resolves: #415