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

Introduce Operations to v1.2 #1254

Closed
wants to merge 2 commits into from
Closed

Introduce Operations to v1.2 #1254

wants to merge 2 commits into from

Conversation

@dgeb
Copy link
Member

@dgeb dgeb commented Feb 9, 2018

We've long discussed the problem of needing to support multiple operations in a
single request (#795 attempted to synthesize requirements from a number of
issues).

I had originally planned to propose "jsonapi-operations" as either an extension
to the base spec that requires negotiation, or as a separate media type
entirely. However, this PR instead introduces the concept directly into the base
spec. The rationale for my change in direction is that, in my experience, the
lack of these facilities in the base spec makes it unsuitable for a wide array
of fairly common scenarios. Some of these scenarios include:

  • Creating multiple resources of the same or different types together and then
    creating relationships between them. For example, creating an invoice
    together with one or more invoice items.

  • Editing memberships of very large relationships, assuming that edits include
    additions and deletions. For example, removing two members of an organization
    at the same time two other members are added.

  • Clearing relationships and explicitly deleting the related resources as well.
    For example, deleting an author and some of the articles associated with that
    author.

  • Sending a general "batch" of changes that must succeed or fail together. For
    example, queueing up an array of operations while offline and then applying
    them together.

I believe this proposal is a complete and thorough solution to these problems
and more.

I really appreciate the work done by others that takes a more incremental
approach to tackling these problems. For instance, @beauby's sideposting draft
(#1197) is a great example of this. There is a nice symmetry with compound
documents that makes it feel like a natural fit for the spec. My primary
concerns with this proposal are:

  • It doesn't allow a request to specify order of changes, which can make
    implementation on the server quite difficult to generalize. There can be
    cross-dependencies and dependencies several layers deep which the server
    must be prepared to unwind and create an execution plan.

  • Discussion regarding extension to accommodate updating and deletion of
    resources and relationships has led to very operation-like proposals such as
    method being added to individual resources.

Given these concerns, I thought it would be worthwhile to consider operations
as the single answer in the base spec.

Note: This PR builds off the Local IDs PR, which are key for linking resources which have not yet been assigned permanent IDs.


Last but not least, I want to say thank you to @ebryn and his company Prototypal for funding much of the time I spent working on the operations proposal. Almost all the time I spend on JSON:API is my own, and it has taken quite a bit over the years, so Erik's assistance was a big boost to helping me push this forward.

With that said, we are all trying to do what's right to move the spec forward, and I'm not trying to make unilateral decisions. I do want to see operations through in one form or another (as part of the base spec or separately), and this is my first attempt. I definitely want to hear what others are thinking.

Copy link
Contributor

@gabesullice gabesullice left a comment

Very interesting proposal @dgeb!

I've completely read through it once this morning. So I haven't really sat down and thought through it completely yet.

My first reaction is that the "identities" key could use a little more elaboration. I'm least clear about its intended purpose.

For example, in the final example, both an "lid" and "id" are included in the newly created "authors" resource, which I think would serve the same purpose as an identity. I'm probably missing the use case that you're trying to support here and a little handholding in the spec might help elucidate that case.

Copy link

@courajs courajs left a comment

I love it! I have a few comments, but overall I think this would be a huge win to include in the base spec (though keeping it optional - "a server MAY support usage..." is good).

Every [resource object][resource objects] **MUST** contain an `id` member and a `type` member.
The values of the `id` and `type` members **MUST** be strings.
Every [resource object][resource objects] **MUST** contain an `id` member and a
`type` member. As noted above, the only exception is that the `id` member is not

This comment has been minimized.

@courajs

courajs Feb 10, 2018

I'm not sure this is noted above - I think this is the first mention of it

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

The exception is spelled out above as follows:

A resource object MUST contain at least the following top-level members:

  • id
  • type

Exception: The id member is not required when the resource object originates at
the client and represents a new resource to be created on the server.

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

BTW, I'm open to rewording this in both the 1.0 and 1.1 specs so that the exception clause is integrated in the requirement instead of standing on its own.

Every [resource object][resource objects] **MUST** contain an `id` member and a `type` member.
The values of the `id` and `type` members **MUST** be strings.
Every [resource object][resource objects] **MUST** contain an `id` member and a
`type` member. As noted above, the only exception is that the `id` member is not

This comment has been minimized.

@courajs

courajs Feb 10, 2018

I think this is confusing phrasing. It means we've got a sentence that's false on its own, and needs to be taken together with another sentence. Can we find something that doesn't have to contradict itself later? Maybe something like:

Every resource object MUST contain a type member. Every resource object MUST contain an id member, except when the resource object originates at the client and represents a new resource.

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

It means we've got a sentence that's false on its own, and needs to be taken together with another sentence. Can we find something that doesn't have to contradict itself later?

I agree this is a best practice and will reword. Thanks for the suggestion!

@@ -361,9 +381,19 @@ A "resource identifier object" is an object that identifies an individual
resource.

A "resource identifier object" **MUST** contain `type` and `id` members.
The only exception is that the `id` member is not required when the resource
identifier object contains a `lid` that identifies it as matching a new resource
to be created on the server.

This comment has been minimized.

@courajs

courajs Feb 10, 2018

Similar to the comment on the exception above - can we make every sentence true on its own? Maybe:

A "resource identifier object" MUST contain a type member. A resource identifier object must also contain an id member, except when it contains a lid that identifies it as matching a new resource to be created on the server.

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

Yes, I think that is an improvement.

[resource identifier objects][resource identifier object].

The purpose of the `identities` array is to correlate `lid`s in a request with
`id`s in a response.

This comment has been minimized.

@courajs

courajs Feb 10, 2018

Like @gabesullice, I'm a little fuzzy on why identities is strictly necessary - I think you could construct the same information by crawling the ops list. But, maybe its purpose is just to simplify that task? That would be valid, but it might be nice to have phrasing to that effect. Maybe:

The purpose of the identities array is to make it simple to correlate lids in a request with ids in a response.

requests. The values `"get"`, `"add"`, `"replace"`, and `"remove"` align with
the HTTP methods `GET`, `POST`, `PATCH`, and `DELETE` used for singular
requests. The specific `op` codes are inspired by [RFC
6902](https://tools.ietf.org/html/rfc6902).

This comment has been minimized.

@courajs

courajs Feb 10, 2018

I don't love the choice of replace here. In the referenced JSON-patch spec, replace is only ever applied to a single field, replacing it entirely. In this spec, it's applied to a document, but has the semantics of an update - keys not mentioned in the replace op are not modified. replace seems to imply the whole record would be replaced with only the new values.
Could we change to patch or update? I think otherwise it will cause confusion for people learning the spec.

This comment has been minimized.

@gabesullice

gabesullice Feb 10, 2018
Contributor

I'll second that sentiment. Until @courajs just pointed out the update semantics, I fell into the trap thinking "replace" was like a PUT.

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

I agree that replace could cause confusion and will probably change it to update. I'm trying to avoid usage of HTTP methods, since operations exist at a different layer, so I'm reluctant to use patch.

Without perfect alignment, I should probably do away with the reference to RFC 6902 as well.


An operation that creates a resource **MUST** target a resource collection
either through the request endpoint or the operation's `ref` object. The
operation **MUST** include an `op` code of `"add"`.

This comment has been minimized.

@courajs

courajs Feb 10, 2018

Here, it says the operation must target either through the endpoint or the ref object, but the example does neither. Instead, it targets via properties of the data member.

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

@courajs thanks - good catch!


An operation that updates a resource **MUST** target that resource
either through the request endpoint or the operation's `ref` object. The
operation **MUST** include an `op` code of `"replace"`.

This comment has been minimized.

@courajs

courajs Feb 10, 2018

Same as above comment - this targets via properties of data, but it's stated that ops must use ref.

This comment has been minimized.

@dgeb

dgeb Feb 10, 2018
Author Member

@courajs again - good catch!

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 10, 2018

@gabesullice @courajs thanks for reviewing!

My first reaction is that the "identities" key could use a little more elaboration. I'm least clear about its intended purpose.

The identities key was introduced in #1244 as a means to ensure that every lid that is passed in a request can be aligned with an id in the response, regardless of the shape of that response. This concept was born out of thinking through some exceptions in #1197 in which a response might take a shape that doesn't include all the created resources. However, as you point out, it's not strictly necessary with operations. I'm going to remove the concept from both #1244 and this PR to keep them both as lean as possible. Thanks for questioning this.

A client may include a "local ID" as a `lid` member in a resource object to
uniquely identify the resource within the request document. Every representation
of that resource, whether as a resource object or resource identifier object,
must then include the matching `lid` member and value.

When a server receives a request document that contains resources with local
IDs, the server must include the matching `lid` member and value in every
representation of that resource or resource identifier in the response document.

This addition paves the way for requests of all kinds that may need to establish
linkage between resources that have not yet been assigned a server-generated ID.
@dgeb dgeb force-pushed the operations branch from f85de2d to 6a7ae6f Feb 10, 2018
@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 10, 2018

Ok, the identities array has been removed from both here and #1244, and the other suggestions / corrections have been incorporated.

One aspect that perhaps should be fleshed out further with examples is the usage of params in operations. I am reluctant to provide completely redundant examples with the query param examples, but I think some examples could be clarifying.

@dgeb dgeb changed the title Introduce operations to the base 1.1 spec Introduce Operations to v1.1 Feb 10, 2018
Introduces the concept of operations to the base 1.1 spec.

Operations can be used to perform one or more fetch or mutation operations serially and transactionally.
@remmeier
Copy link
Contributor

@remmeier remmeier commented Feb 12, 2018

has http://jsonpatch.com/ also been considered as a basis? Everything here looks great and jsonpatch.com would give just a slight different envelope. An additional benefit of this would be the support of other, non-JSONAPI calls.

one minor other thing, "op" could potentially retain a more HTTP-style naming: GET, POST, PATCH, DELETE.

### <a href="#processing-operations" id="processing-operations" class="headerlink"></a> Processing Operations

A server **MUST** perform operations in the order they appear in the

This comment has been minimized.

@gabesullice

gabesullice Feb 12, 2018
Contributor

I keep going back and forth on this. On the one hand, a server can't possibly know the dependencies of every operation. On the other, it seems like it exposes too much of the server implementation to the client.

Take the example of a client which wants to replace a resource. If the operations happen out of order, then the server will generate a collision error (when the add happens before the remove).

If the order of operations given by the client must be followed, then it's the client's responsibility to order these correctly. That makes sense because it would be far too difficult for a server to infer the correct order in every possible scenario.

However, take the example of creating a resource which has a relationship to another resource. There are 3 possible orderings that could be valid or invalid dependent on the storage implementation of the server. (1) If the server stores relationships separately from resources, then both resources need to be saved before the relationship is created. (2) If the server stores relationships on the referencing resource, then the the referenced resource needs to be created first. (3) Finally, if the relationship is stored on the referenced resource, then the opposite is true and the referencing resource must be saved first.

It seems incorrect for this to be the client's responsibility to handle. Generic clients will need to switch behavior based on something not defined in the spec.

After writing that out, I wonder if the way to handle this is with an opt-in dependency tree. That is, allow operations to be identified and also allow operations to specify operations on which they depend. When operations have no dependency or when multiple operations depend on the same operation, the server may perform those operations in an arbitrary order. That would allow a server to make a best-effort at ordering the operations appropriately (e.g. it could order operations according to relationships) but it would also allow the client to express order when it would otherwise be too ambiguous for the server to determine.

This comment has been minimized.

@gabesullice

gabesullice Feb 12, 2018
Contributor

Here's an example of the replace example from above with dependencies on other operations.

PATCH /bulk
{
  "operations": [{
    "op": "add",
    "deps": ["remove_some_resource"],
    "data": {"not": "shown"}},
  {
    "op": "remove",
    "id": "remove_some_resource",
    "ref": {"not": "shown"}}
  ]
}
@richmolj
Copy link
Contributor

@richmolj richmolj commented Feb 12, 2018

It's great to finally see this area get momentum @dgeb ❤️ ! I strongly agree with your bullet-points making the case that this should be part of the base spec - the scenarios are too common to overlook. I also think this proposal is more cohesive than what I've seen in the past; very much appreciate the work you have put in here.

I do have two main concerns:

  • A. We're introducing a completely new payload, but all these concepts can be expressed with minor additions to the existing payload.
  • B. Rather than allowing the client to specify the order of operations, it requires the client to do so.

Based on the PR description, I think we agree on A except for order of operations. Is this the case? Here's the "sideposting" doppleganger that would add/remove/update comments from an article, using op and lid keys from this PR:

PATCH /articles/1

{
  data: {
    id: "1" 
    type: "articles",
    relationships: {
      comments: {
        data: [
          { type: "comments", lid: "a", op: "add" },
          { type: "comments", id: 1, op: "remove" },
          { type: "comments", id: 2, op: "update" }
        ] 
      }
    }
  },
  included: [
    {
      type: "comments",
      lid: "a",
      attributes: { text: "I love JSONAPI!" }
    },
    {
      type: "comments",
      id: "2",
      attributes: { text: "An updated comment" }
    }
  ]
}

Note that this also supports creating 2 articles at the same time through sideposting to /bulk.

The benefits of a symmetrical payload seem so great to me, point B above (ordering operations) must be overwhelming if we are to introduce a new payload. But I'm not sure that's the case.

Consider a SQL database - you would, 99% of the time, want to persist the parent (e.g. article) before persisting the child (e.g. comments) (since we need the parent id before we can populate the foreign key). In these cases, forcing the client to specify order of operations seems nothing more than cruft (though I'm certainly open to allowing order of operations for the 1% use case).

There's a more important reason than cruft, though. Let's say I coded all my clients to persist the parent before the children while using a SQL database, then switched to a NoSQL store where this logic no longer applies. I would now want to persist these entities differently - but all my clients have already coded the logic to first persist the parent, then the child. I fear there are a number of examples like this, largely summarized by the statement:

Order of operations is a server implementation detail that is not the responsibility of the client.

Which is similar to the concern and examples raised by @gabesullice here.

I'd love to see order of operations supported optionally instead of required, both because it is almost always not needed and often seems not desirable. One way to support order of operations optionally is to take the above sideposting payload and add another key:

...
relationships: {
  comments: {
    data: [
      { type: "comments", lid: "a", op: "add", order: 1 }
      { type: "comments", lid: "b", op: "add", order: 2 }
  }
}
...

We'd end up mirroring the existing sideloading spec, with the addition of three total keys - lid and op, which are in this PR as well, and an optional order (which could even be an extension rather than base spec at that point).

I have some additional comments on the specifics of this PR but would love to get your thoughts on the above first. Again, thanks for the work here, it is awesome to see JSONAPI attempting to address this problem area.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 12, 2018

has http://jsonpatch.com/ also been considered as a basis? Everything here looks great and jsonpatch.com would give just a slight different envelope. An additional benefit of this would be the support of other, non-JSONAPI calls.

@remmeier yes, this proposal was strongly inspired by JSON Patch. One significant difference is that update is used instead of replace to be consistent with the PATCH (instead of PUT) semantics. Another difference is that ref is used instead of a path pointer to acknowledge that the spec is targeting resources and resource collections, instead of a singular JSON document. Given these differences, I don't feel it's right to try to shoehorn exact compliance with RFC 6902 into this proposal, but it's definitely appropriate to call it out as an inspiration :)

one minor other thing, "op" could potentially retain a more HTTP-style naming: GET, POST, PATCH, DELETE.

Just like in JSON Patch, operations occur at a layer distinct from HTTP, and I didn't want to muddy the waters by pretending otherwise.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 12, 2018

@gabesullice @richmolj thanks for considering this carefully and for your feedback!

There are a few aspects of this proposal about ordering and complexity that I'd like to address:

  • Every operation has complete fidelity with a singular request. Everything possible with singular requests can be completed with operations, and vice versa. The only additional complexity is the usage of lids to bind together inputs and outputs if necessary. And the only additional requirement is for servers to process all the operations in a request transactionally.

  • There is no additional knowledge required by clients of how a server persists resources beyond what is required by the existing spec. For instance, a relationship can't be created between an existing resource and one that has not yet been created yet. If request A can follow request B, then operation A can follow operation B.

  • If order is not important, then I would strongly recommend making multiple singular requests in parallel. By using operations, the client is saying that order is important.

  • If order can be elided by the server because of some knowledge it has, then I think it is free to do so. However, there should be no "evidence" in the response, because that breaks the contract with the client. If this allowance requires some tweaking of the language used in this proposal, I'm open to it.


Note: edited for clarity, but then reverted edits after I realized others had commented based on the original text.

@gabesullice
Copy link
Contributor

@gabesullice gabesullice commented Feb 12, 2018

thanks for considering this carefully and for your feedback!

Thanks for taking the time to address each concern so thoroughly!

Everything possible with singular requests can be completed with operations, and vice versa... There is no additional knowledge required by clients of how a server persists resources beyond what is required by the existing spec.

Thanks for clarifying this @dgeb, that's a subtlety that I didn't pick up on.

Perhaps this is something that could be stated explicitly in the operations introduction or as a note.

Something like, "Individual operations in a request are semantically equivalent to singular requests performed serially with a transactional guarantee."

Perhaps that could be left to the recommendations page of the website too :)

If order can be elided by the server because of some knowledge it has, then I think it is free to do so. However, there should be no "evidence" in the response, because that breaks the contract with the client. If this allowance requires some tweaking of the language used in this proposal, I'm open to it.

Does this suggest that the imperative ought to be SHOULD over MUST? I'm actually unclear on this aspect of specification-writing. As long as there's no "evidence," as you said, does the imperative matter?

@richmolj
Copy link
Contributor

@richmolj richmolj commented Feb 12, 2018

Thanks for taking the time to address concerns @dgeb ❤️

Everything possible with singular requests can be completed with operations, and vice versa.

This is why I favor mirroring sideloading payload - why not mimic what we do in singular requests (RESTfully pair a resource with a verb) all the way down, instead of introducing a new payload?

  • There is no additional knowledge required by clients of how a server persists resources
  • If order can be elided by the server because of some knowledge it has, then I think it is free to do so.

I'm not sure how these statements are reconciled with the statement in the PR "A server MUST perform operations in the order they appear in the operations array". Doesn't this mean the client now needs to know the correct order to persist resources? Or is this just a matter of updating the PR language?

If order is not important, then I would strongly recommend making multiple singular requests in parallel. By using operations, the client is saying that order is important.

This is often not possible, for instance when things need to run in the same transaction but the order doesn't matter - most nested web forms with validation errors would fall into this category. Time-consuming ETLs is another use case I have personally run into.

In fact, none of the examples given in the PR seem order-dependent (at least not for the >90% use case):

  • Creating an invoice together with one or more invoice items.

Most servers would create the invoice, then create the items, then roll back if anything is invalid.

  • For example, removing two members of an organization at the same time two other members are added.

Note sure why the order would matter for the default use case.

  • Deleting an author and some of the articles associated with that
    author.

Most servers would delete the author, then the articles, within a transaction.

Considering this PR makes a great case for supporting common use cases as part of the base spec, and ordering of operations is not a common use case, wouldn't it make more sense to have "sideposting" in the base spec and operations as an extension? Assuming we don't want to go with the option of simply adding an order key.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 12, 2018

Sorry, I rushed my comment as I was going out the door and realized I should retract this statement:

If order is not important, then I would strongly recommend making multiple singular requests in parallel. By using operations, the client is saying that order is important.

This would be better stated:

If order and atomicity are not important, then I would strongly recommend making multiple singular requests in parallel. By using operations, the client is saying that order and / or atomicity are important.

@richmolj Thanks for checking me on this point. I'll respond to your other points soon.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 12, 2018

This is why I favor mirroring sideloading payload - why not mimic what we do in singular requests (RESTfully pair a resource with a verb) all the way down, instead of introducing a new payload?

In the side-posting issue you have proposed modifications to the document structure to support layering capabilities on top of side loading. For instance, you proposed adding op or method to individual resources so that you can, for example, delete some resources as you add others. You have to be extremely careful with HTTP compliance here - only PATCH allows for such flexibility for media types.

I'm not sure how these statements are reconciled with the statement in the PR "A server MUST perform operations in the order they appear in the operations array". Doesn't this mean the client now needs to know the correct order to persist resources? Or is this just a matter of updating the PR language?

I said above that I am open to tweaking the PR language. The important thing (to me) is that the client not be able to distinguish that a server has chosen to parallelize processing some operations.

In the current spec, clients already need to understand order dependencies to some extent because servers are free to impose any order dependencies they want. Some servers will require that invoices be created before invoice items, for instance. The specification of allowed ordering of operations seems orthogonal to this debate and is probably best solved with a capabilities specification (something we should definitely tackle).

I should also point out that sideposting can have related concerns. A server may allow sideposted invoice items together with an invoice as primary data, but not the other way around. Ultimately, as a specification, we have to allow servers to control of what's allowed at each endpoint. Again, I think this can be best communicated with a capabilities specification.

@richmolj
Copy link
Contributor

@richmolj richmolj commented Feb 12, 2018

@dgeb I actually think we agree on most things.

The specification of allowed ordering of operations seems orthogonal to this debate and is probably best solved with a capabilities specification (something we should definitely tackle).

I agree! Let's put ordering to the side for now

The important thing (to me) is that the client not be able to distinguish that a server has chosen to parallelize processing some operations.

Agree the client should be agnostic in this regard

only PATCH allows for such flexibility for media types.

While I am not sure I agree with this interpretation, I'm happy to agree on PATCH being the only verb used for operations/sideposting

A server may allow sideposted invoice items together with an invoice as primary data, but not the other way around. Ultimately, as a specification, we have to allow servers to control of what's allowed at each endpoint.

Agree on this was well - just as an invoice can sideload invoice items, but not the other way around, there should be no such expectation for a sidepost.

lid

Yep, sounds like we need this no matter what

Given all the above, it looks like we have two choices to solve this problem:

  • Mirror the existing sideload payload, adding a single key ("sideposting")
  • Introduce a completely new payload ("operations")

Since we seem to agree on so much, I'm wondering how we are reaching different conclusions on this final choice. What am I missing?

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 12, 2018

@richmolj Just want to say that I love your emphasis on consensus <3

I'll get back to you soon with some examples that I hope will clarify any differences that remain.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 13, 2018

@richmolj Ok, below is my attempt to distinguish between sideposting, as I believe you're proposing it, and operations as proposed in this PR. Please feel free to correct any misunderstandings about your proposal.


I'm pretty sure it's an oversimplification to say that sideposting just involves "Mirror the existing sideload payload, adding a single key". I think what you're proposing is allowing an op member to be associated with any resource and resource identifier in a compound document. I'm not sure "sideposting" is an accurate name for your counterproposal - maybe "graph mutations" would be more accurate? Anyway, I'll call it sideposting for the sake of discussion.

I believe that sideposting would involve the following changes to the spec at a minimum:

  • If only PATCH is used for sideposting, then every resource and resource identifier in the compound document request would need to include the op member to understand its role in the request. Or there would need to be some concept of a "default" op for the request.

  • We would need to make this breaking for 1.0 servers which do not understand sideposting. This could be handled via version or feature negotiation or somehow choosing a structural distinciton that would make these requests invalid for 1.0 servers. Keep in mind that 1.0 servers are supposed to simply ignore the op member and wouldn't be expecting anything in included in a request.

  • New rules would be needed for responses to guarantee that every lid sent in a request can be mapped to an id in the response. Something along the lines of the identities array would be needed as I explored for a bit in #1244 (before removing it). Without this guaranteed mapping, clients risk never being able to map local ids to server ids.

In addition, to fulfill some of the other use cases solved by operations, the spec would need to expand in additional ways:

  • In order to allow multiple resources of the same type to be created / updated together, we'd have to expand the definition of primary data to allow it to be an array of resources, and not just a single resource.

And then there are some general implementation complexities:

  • Since the graph of resources could be arbitrarily complex (but ultimately limited by the server), a server would need to generate execution plans for each allowed graph based upon the operation associated with each resource or resource identifier. For instance, the server might need to delete some resources before updating others. Cross references between resources would need to be fully accounted for. Basically, the full graph of primary and included resources would need to be traversed before an execution plan could be generated.

  • Clients would need to track all mutations based upon relationships. If a mutation could not be related in one way or another, they could not be included in the same request. There could be substantial bookkeeping on the client.

Åfter all this, sideposting still wouldn't be able to handle use cases like the following:

  • Create three articles and then fetch the first ten articles sorted by title in a single request.

  • Collect mutations which may or may not be related and perform them together transactionally (e.g. while a client is offline).

To contrast, the operations proposal does introduce a new wrapper object, the operation object, but the rules for processing each operation, and the capabilities allowed for each, are identical to the rules for singular requests. The only additional requirements are accounting for local IDs and transactionality (but these requirements are also present in the sideposting proposal).

In sum, I consider the operations proposal to be the smallest delta from the current spec, while also providing the greatest range of new functionality.

@richmolj
Copy link
Contributor

@richmolj richmolj commented Feb 13, 2018

Thanks for taking the time to give a detailed breakdown @dgeb, I really appreciate it ❤️ Though I'll still make the case for sideposting below, I think this will lead to some constructive edits and a healthier PR at a minimum.

Re: Sideposting

We would need to make this breaking for 1.0 servers

As the changes are purely additive (and the server should ignore the additions, as you say), how is this breaking? Seems in-line with a minor version release to me.

every lid sent in a request can be mapped to an id in the response

Because this is a mirrored payload, the lid would exist in the response at the same level (in the relationship resource identifier and next to the server-minted id). This is how clients map lid to server-generated ids.

allow multiple resources of the same type to be created / updated together

I think this is an independent issue. "sideposting" would be compatible with an array of data, for instance, which is something that could be merged independently of this discussion. Relatedly: see the "Operations: B" note below, where I note this might be a better fit for an extension than the base spec.

As a minor note (don't want to get held up on this point), sideposting would support this even without that independent PR like so:

PATCH /bulk

{
  data: {
    type: "bulk_updates"
    relationships: {
      articles: [
        { lid: "a", op: "create", type: "articles" },
        { lid: "b", op: "create", type: "articles" }
    }
  }
  included: [...article data...]
}

server would need to generate execution plans for each allowed graph based upon the operation associated with each resource or resource identifier

I believe that this is a feature, not a bug. Baking this logic into the client means coupling the client to server implementation. Taking the example "For instance, the server might need to delete some resources before updating others." - what if this was true in January, but the reverse is true in December after I have swapped my datastore? I should be able to keep the same API without requiring all clients to be updated.

I think this comes back to the "A server MUST perform operations in the order they appear in the operations array" language in the PR that @gabesullice and I commented on above, and is my chief concern with operations. If we removed the "Must" here it seems both solutions would have that same server-side challenge, if we keep the "Must" it seems like it would lead to client/server coupling.

Clients would need to track all mutations based upon relationships

I'm unsure what you mean by this, could you give an example? How is this different from sideloading, which is also based on available relationships?

Again, I think this might be an independent issue regarding single-request writes of unrelated data.

Re: Operations

Create three articles and then fetch the first ten articles sorted by title in a single request.

Collect mutations which may or may not be related and perform them together transactionally (e.g. while a client is offline).

On the first, I agree, sideposting would not support this. On the second, I somewhat agree but as noted above I think this is a separate PR possibly unrelated to operations or sideposting.

In any case, I'm guessing you separated these out into a separate section because they are less common scenarios. If these two points are what separates the abilities of operations from the abilities of sideposting, it would make sense to

A) Update the PR to explicitly call this out as a requirement we are trying to solve
B) Move operations to an extension and sideposting into the base spec

The reason I arrive at B: per the PR description, this was originally an extension or separate media type, but the scenarios were too common to ignore. I actually strongly agree with this when we are talking about updating primary data alongside relationships in a single request (as "sideposting" solves)...but disagree when we are talking about blending reads and writes into a single request (which "operations" solves, amongst other important but less-common use cases).

As an aside: the benefit of "sideposting" is the intuitive mirroring of "sideloading", which we generally expect from RESTful APIs that are built to transfer the same state representation back and forth to the server. There is no question that "operations" is a more powerful paradigm - but in my opinion it diverges enough from expectations that it makes more sense as an official extension.

in [Errors].

> Note: An empty object (`{}`) is a valid operation object, and thus can be
returned as a result to an operation request.

This comment has been minimized.

@EndangeredMassa

EndangeredMassa Feb 13, 2018

Allowing an empty object to be a valid operations object makes sense in a response, but not a request, right? Should the spec make it clear that this is the case or am I missing something here?

Also, should this be more than a "Note"? This seems like an important part of the spec. The phrase "can be" in this note also seems out of place when we clearly use the imperatives everywhere else. Should it be **MAY** be?

This comment has been minimized.

@dgeb

dgeb Feb 13, 2018
Author Member

This is a note and not a normative portion of the spec because it does not introduce any new requirements or information. In other words, the fact that an empty object can be a valid operation object can be deduced from the description of the operation object. Since it's not normative, we shouldn't use language like **MAY**.

@@ -164,6 +169,8 @@ In addition, a resource object **MAY** contain any of these top-level members:
* `links`: a [links object][links] containing links related to the resource.
* `meta`: a [meta object][meta] containing non-standard meta-information about a
resource that can not be represented as an attribute or relationship.
* `lid`: a local identifier that, together with `type`, uniquely
identifies a resource within the document.

This comment has been minimized.

@EndangeredMassa

EndangeredMassa Feb 13, 2018

Should it be in the scope of the spec to note the value of UUIDs in solving this problem? More specifically, generating a UUID locally and posting that to a server allows you to ensure that the id on the server is the same value without any real chance of conflict.

This comment has been minimized.

@dgeb

dgeb Feb 13, 2018
Author Member

I agree that this would make a good non-normative note 👍 I think we may mention client-generated IDs in a couple places, but mentioning it near lids makes sense as well.

This comment has been minimized.

@wimleers

wimleers Feb 14, 2018
Contributor

Thanks for asking this, @EndangeredMassa, it made me ask a question at #1244: #1244 (comment)

This comment has been minimized.

@krainboltgreene

krainboltgreene Apr 13, 2018
Contributor

IMO this should just be called local, jsonapi doesn't truncate words anywhere else.

This comment has been minimized.

@les2

les2 Feb 23, 2019

@krainboltgreene id is truncated from identifier

i could see using localId over lid though ...

This comment has been minimized.

@ahx

ahx Feb 23, 2019
Contributor

@les2 Uh, let’s avoid using camel “localId” or snake case “local_id” in the spec. People will go crazy about these things. 😉 lid is fine, IMO.

> Note: `op` codes are used instead of the HTTP methods prescribed for singular
requests. The values `"get"`, `"add"`, `"update"`, and `"remove"` align with
the HTTP methods `GET`, `POST`, `PATCH`, and `DELETE` used for singular
requests.

This comment has been minimized.

@EndangeredMassa

EndangeredMassa Feb 13, 2018

I understand why you don't want to clash meaning with the http verbs and op values, but the logical alternative to me is "create", "read", "update", "delete" (CRUD). Was this considered when naming these operation types?

This comment has been minimized.

@dgeb

dgeb Feb 13, 2018
Author Member

Haha ... I just had this thought this morning. The original intent was to align with RFC 6902, but since we're now using update and not replace, we may just want to use the more memorable CRUD theme.

@EndangeredMassa
Copy link

@EndangeredMassa EndangeredMassa commented Feb 13, 2018

The lack of bulk operations (mostly bulk updates) severely hinders adaptability of JSON API in conversations I've had about standardizing API wire formats. I'm excited to see progress made on this front and I very much agree that something like this should go in the base spec.


I like that this doesn't really add much in terms of functionality. It appears to be a small superset of the existing functionality, right? It's essentially a tool for optimizing for less round trips to the server. It has me wondering: if operations can essentially do anything the rest of the spec can do and one of the huge benefits to a standard wire format is that standard tools on each end can understand it, what need do we have for the spec's top-level document structure definition keys (data, included) or the use of HTTP VERBS at all?

For my use of JSON API, it's an explicit goal that I rarely had to read the actual request/response because the tooling on each side does such a good job abstracting that away. That tooling could be simplified if it always used operations, right?

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 13, 2018

@richmolj I'm not sure whether to keep this conversation going here. Maybe a separate issue would be better to allow operations to be refined here? But anyway, I appreciate your responses and will follow up as much as time allows:

As the changes are purely additive (and the server should ignore the additions, as you say), how is this breaking?

We would want 1.0 servers to treat sideposted requests as breaking because they would attempt to understand the payload with a PATCH method as a resource update request, and they would ignore the op code in the resource. In other words, 1.0 servers would misinterpret sideposting because they have no concept of this new feature. We don't want clients to get a false "success".

Because this is a mirrored payload

Are include or sort params allowed to customize the response? Or is it a hard rule that the response directly mirrors the request?

I think this comes back to the "A server MUST perform operations in the order they appear in the operations array" language in the PR that @gabesullice and I commented on above, and is my chief concern with operations. If we removed the "Must" here it seems both solutions would have that same server-side challenge, if we keep the "Must" it seems like it would lead to client/server coupling.

I've responded above that I'm fine to tweak the MUST language: "The important thing (to me) is that the client not be able to distinguish that a server has chosen to parallelize processing some operations." Note that this would allow servers to create an optimized execution plan, but not require it.

I'm unsure what you mean by this, could you give an example?

Clients would need to track dirtying of record graphs instead of tracking a queue of changes (i.e. operations).

I'm guessing you separated these out into a separate section because they are less common scenarios.

The other sections were related to adjustments that would be required to fulfill sideposting as you're describing it. I separated these items out because I have not heard any proposals related to sideposting that attempt to address these scenarios.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Feb 13, 2018

@EndangeredMassa Thanks very much for your feedback!

I like that this doesn't really add much in terms of functionality. It appears to be a small superset of the existing functionality, right?

That's correct.

That tooling could be simplified if it always used operations, right?

Yes, tooling could use operations exclusively without losing any functionality from the current spec.

@mydea
Copy link

@mydea mydea commented Aug 28, 2018

What's the status on this? We are very excited about this and would love to see it landing!

@mydea
Copy link

@mydea mydea commented Dec 3, 2018

Really sad to not see this in 1.1 :( What's the reason for this? What is open to get this (Operations) finalized? And what is the (rough) timetable for 1.2? This has been open for a pretty long time now, so it would be really great to finally have these capabilities in JSON:API.

@dgeb
Copy link
Member Author

@dgeb dgeb commented Dec 3, 2018

Really sad to not see this in 1.1 :( What's the reason for this? What is open to get this (Operations) finalized? And what is the (rough) timetable for 1.2? This has been open for a pretty long time now, so it would be really great to finally have these capabilities in JSON:API.

@mydea from my perspective, operations fills one of the most pressing gaps in the spec, so I understand your feelings. Simply put, we decided to ship 1.1 with a smaller set of features rather than delay it further, since profiles will bring a lot of value on their own. We will resume discussing 1.2 this week with the understanding that operations are now the highest priority. I think it's possible that we could ship a 1.2 RC within a few months of 1.1 final.

@donnysim
Copy link

@donnysim donnysim commented Dec 7, 2018

I've skimmed through this a bit, and wonder how would this look with something like "belongs to" relation? If the client is creating a Place and in the same window/form he can create a City (through a modal, like a + button near city select etc.), so Place belongs to City and both should be persisted on save. In this scenario City must be created before Place, but if this is handled on the client side, then the client needs to know about what kind of relationship this is (and all the others) and make implementations/execution plan based on it. From developers standpoint, this would be a "disgusting" API to work with? What would be the go to in this scenario? From my point of view, the server should be the one to build the execution plan, otherwise this leaves API's frustrating to use if anything.

@marijnz0r
Copy link

@marijnz0r marijnz0r commented Feb 20, 2019

Hi @dgeb,

Could you provide us with an update regarding this pull request and/or version 1.2?

Kind regards

@krainboltgreene
Copy link
Contributor

@krainboltgreene krainboltgreene commented Feb 24, 2019

Copy link
Contributor

@gabesullice gabesullice left a comment

I started reviewing and working on an existing implementation of this and here are some of the questions/thoughts that came up while I was doing that.

My general feeling is that the spec needs to provide a little more rigidity.

The ref being optional because it can be inherited from the endpoint is a little strange. I'm struggling to think of a good example of when that would be useful.

I also think it might be good to consider having separate operation objects and result objects to come from the client and server, respectively. That would help in defining the allowed members in each case.


An operation object **MAY** contain any of the following members:

* `op`: an operation code, expressed as a string, that indicates the type of

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

Does this member need to be required? If not, does it have a default?

* `ref`: the resource or resource collection that is the target of the
operation. This member serves to disambiguate the target at a particular
endpoint if that endpoint represents multiple resources. `ref` **MUST** be
expressed as an object that **MAY** contain any of the following members:

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

I think the intent here is actually that a ref MUST contain an type and id, but MAY contain relationship. @dgeb, is your intent that any combination of these members should be allowed?

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

Ah, I think I have it: it's possible for the client to send a get operation and expect a collection. In that case, they'd just send type on its own. Still, for that case, relationship ought to be forbidden right?

expressed as an object that **MAY** contain any of the following members:
- `type`: resource `type`
- `id` or `lid`: resource identifier or local identifier
- `relationship`: the name of the relationship

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

Nit: the field name of the relationship

An operation object **MAY** contain any of the following members:

* `op`: an operation code, expressed as a string, that indicates the type of
operation to perform. Valid values include:

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

Is there a reason the list of valid operations isn't a "MUST be one of the following" case? Do you for this list to be extended?

endpoint if that endpoint represents multiple resources. `ref` **MUST** be
expressed as an object that **MAY** contain any of the following members:
- `type`: resource `type`
- `id` or `lid`: resource identifier or local identifier

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

"resource identifier" here is confusing because of the "resource identifier object" which is typically a type and id pair.

- `sort`
- `filter`
- `page`
- `fields`

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

@dgeb, I think this needs more clarification too. I think a reference to our definition of a query parameter name and a link would be enough.

If you wanted to go even further you might consider language like:

A params object value MUST be either a string, a number, or an array of strings or numbers.

@jaredcnance, I agree that that would be pretty neat, but I also feat that parsing query parameters is so poorly defined (and often programming language/framework-specific) that this will cause headaches. Version 1.1 of this spec expands the definition of a query parameter and I think we should illustrate that understanding here with an example like:

{
  "foo[bar]": "baz",
  "foo[qux]": "quux"
}

That encoding is easy to put back together as a string so the implementation can then parse it just like a URL query string.

meta-information about the operation.

Different members are required for processing different types of operations, as
described below.

This comment has been minimized.

@gabesullice

gabesullice May 15, 2019
Contributor

I don't think included should be permitted in a request document; nor should a params or ref member be permitted in response documents. This made me think I would find that below, but I don't see anything to that effect. Am I missing it?

@tembra
Copy link

@tembra tembra commented May 21, 2019

Oh god! This specification would solve my problems and make me sleep very well!

But I guess that we are too far from that, because this is just for v1.2 and the v1.1 that was planned for January 31, 2019 is not even finished 😕

@mydea
Copy link

@mydea mydea commented Jul 3, 2019

Is there any timeline on this? Frankly, this has been long overdue and is pending without any clear timeline for 1,5 years. We are using JSON:API heavily, but this issue is massively holding us back, which is driving us to consider other API designs. Which is a shame, since we really like the general idea of JSON:API.

I would just like to have a proper understanding of where this is going. If we cannot, realistically, consider this to be finalized e.g. in the next year, then we'll be forced to look for other options to solve this (IMHO probably pretty common) issue of transactionality.

@marctich
Copy link

@marctich marctich commented Jul 26, 2019

Any update as to when this will be implemented?

@dlikhten
Copy link

@dlikhten dlikhten commented Jan 6, 2020

Going to just poke at this again. Any comments from the maintainers? Is this even being discussed?

@ebryn
Copy link

@ebryn ebryn commented Jan 6, 2020

@dgeb
Copy link
Member Author

@dgeb dgeb commented Jun 11, 2020

I'm closing this PR in favor of #1437, which introduces operations as a new v1.1 "extension" (see #1435). The atomic operations extension is technically very close to this PR, with the main exception being that it covers mutations only and not get operations. It will likely be the only official extension launched with 1.1, since local IDs have already been merged into the 1.1 base spec.

Thanks to everyone for the discussions and focus on this over the years. We're very close to being able to use operations "officially" now.

@dgeb dgeb closed this Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet