Skip to content

Introduce Local Identities extension #1436

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

Closed
wants to merge 4 commits into from
Closed

Introduce Local Identities extension #1436

wants to merge 4 commits into from

Conversation

dgeb
Copy link
Member

@dgeb dgeb commented Oct 8, 2019

This is a proposal for an official extension to the JSON:API spec, as described in #1435. This proposal is based upon #1244, and supersedes that PR.

This extension provides a means to uniquely identify resources "local" to a specific document. It uses the namespace local.

Resources can be assigned a "local identity" (local:id) that can be used to reference each other before they have been assigned a permanent id.

@richmolj
Copy link
Contributor

richmolj commented Oct 8, 2019

Curious about this requirement:

The server MUST NOT include local:id members in a response document.

One of the prime use cases for local IDs is to sync records in JS memory with the server, which this prevents. IOW let's say I have 3 Posts in JS memory, and I use something like Operations to save all 3 at once. If the server doesn't respond with the local id, I'd have no way to match up the persisted server records with the ones in JS memory.

@dgeb
Copy link
Member Author

dgeb commented Oct 8, 2019

@richmolj We discussed different means to return a local/server mapping of IDs, but were unable to justify the additional complexity for the known use cases.

The position of resources in a response document should be sufficient to make the association between local IDs and server-generated IDs in every scenario in both the base spec and the proposed atomic operations extension (#1437). Clients should be able to await the response document to perform mappings for server-generated IDs.

@dgeb dgeb mentioned this pull request Oct 8, 2019
Co-Authored-By: Gabe Sullice <[email protected]>
@jugaadi
Copy link

jugaadi commented Oct 8, 2019

@richmolj Won't a profile help in this case? LocalID can be provided in the meta object of each resource.

@tobyzerner
Copy link
Contributor

tobyzerner commented Oct 9, 2019

@jugaadi My understanding is that while a local ID could indeed be defined in a resource's meta object via a profile, referencing a local ID in relationship data requires an addition to the base spec which can only be achieved by an extension.

@sandstrom
Copy link
Contributor

I have the same comment as @richmolj, that afaik most people that want local IDs need them to sync JS state with server-state.

The server MUST NOT include local:id members in a response document.

Some examples:

@dgeb
Copy link
Member Author

dgeb commented Oct 9, 2019

The challenge here is that we are now essentially discussing making accommodations for extensions that have not been written and formalized. As I said above, the position of resources in a response document should be sufficient to make the association between local IDs and server-generated IDs in every scenario in both the base spec and the proposed atomic operations extension.

Let's say that someone decides to formalize the conventions used by Ember's embedded record mixin as a community extension. There are a couple paths we could take:

  1. If the "embedded" extension needs an identities map to make sense of its document for server-generated IDs, then the extension itself could include such a map in its own specification. For instance, it might require that servers output a root element such as embedded:identities that contains a map between local identities and server-assigned identities.

  2. We could foresee such a need as general enough to be included in the local identities extension. For instance, if a client passes an array of identities in local:ids, then the server MUST respond with an identities map at its root.

There are pros / cons to each approach.

With the first approach, the "pro" is that we keep "official" extensions as lean as possible, while encouraging other extensions to meet their own needs. The "con" is that the embedded extension may not share the same logic as another community extension that attempts to solve the same problem.

With the second approach, the "pro" is that the solution is probably general enough to meet the future needs of any extension that needs a local identity map. The "con" is that it adds complexity to the local identities extension and every implementation that supports it so that some as-yet-unspecified extension can benefit.

@dgeb
Copy link
Member Author

dgeb commented Oct 9, 2019

A related point I should make here is that extensions themselves can evolve, as long as they evolve in a compatible way.

For example, we could start with the simple form of the local identities extension (as currently defined). If the need arises over time across multiple extensions to have an identities map, we could specify that if clients include an array of local:ids, then servers MUST respond with an identities map.

@sandstrom
Copy link
Contributor

sandstrom commented Oct 10, 2019

@dgeb I see your point!

Position in the document is indeed enough (theoretically), however it feels somewhat brittle and I guess that's what some people (me included) have against it.

Also, maybe there are scenarios where the server won't respond with all embedded records in the payload? You know the spec better than me, but it seems like it may allow returning only the primary document.

I think your suggestion about making it optional depending on whether the client included an array of local:ids in the request or not could be a good middle ground.

Also, although I understand your concern about making accommodations for something somewhat specific, I still think there is a difference between the main spec and the extensions. I think it's natural that extensions are slightly more opinionated/narrow than the base spec, because they are catering to specific use-cases. Handling of side-posting for embedded records has been an known issue for several years and this would be an awesome way to fix it.

It could look something like this:

[first part of extension]…

The client **MAY** include the member `local:ids` in the top level document, 
with an array of local identity members. If such a key is present, 
the server **MUST** must include a map of IDs to local IDs under 
the key `local:map` at the root level of the response.

A request/response pair could look like this:
POST /people HTTP/1.1
Content-Type: application/vnd.api+json;ext="https://jsonapi.org/ext/local"
Accept: application/vnd.api+json;ext="https://jsonapi.org/ext/local"
{
  "data": {
    "type": "people",
    "attributes": {
      "firstName": "John",
      "lastName": "Doe"
    },
    "relationships": {
      "addresses": [{
        "data": {
          "local:id": "a",
          "type": "addresses"
        }
      }, {
        "data": {
          "local:id": "b",
          "type": "addresses"
        }
      }]
    }
  },
  "included": [{
    "data": {
      "local:id": "a",
      "type": "addresses",
      "attributes": {
        "label": "Work",
        "street": "10 Downing Street",
        "zip": "3819ZY",
        "city": "London"
      }
    }
  }, {
    "data": {
      "local:id": "b",
      "type": "addresses",
      "attributes": {
        "label": "Home",
        "street": "5 Kensington Gardens",
        "zip": "4213ZV",
        "city": "London"
      }
    }
  }],
  "local:ids": ["a", "b"]
}
HTTP/1.1 201 Created
Location: http://example.com/people/550e8400-e29b-41d4-a716-446655440000
Content-Type: application/vnd.api+json;ext="https://jsonapi.org/ext/local"

{
  "data": {
    "id": "550e8400-e29b-41d4-a716-446655440000",
    "type": "people",
    "attributes": {
      "firstName": "John",
      "lastName": "Doe"
    },
    "relationships": {
      "addresses": [{
        "data": {
          "local:id": "4ceb5ea7-8f2f-4836-9294-f235f3e506b4",
          "type": "addresses"
        }
      }, {
        "data": {
          "local:id": "9dc4ec66-4c2b-483d-b15f-151a61d91bf9",
          "type": "addresses"
        }
      }]
    }
  },
  "included": [{
    "data": {
      "id": "4ceb5ea7-8f2f-4836-9294-f235f3e506b4",
      "type": "addresses",
      "attributes": {
        "label": "Work",
        "street": "10 Downing Street",
        "zip": "3819ZY",
        "city": "London"
      }
    }
  }, {
    "data": {
      "id": "9dc4ec66-4c2b-483d-b15f-151a61d91bf9",
      "type": "addresses",
      "attributes": {
        "label": "Home",
        "street": "5 Kensington Gardens",
        "zip": "4213ZV",
        "city": "London"
      }
    }
  }],
  "local:map": {
    "a": "4ceb5ea7-8f2f-4836-9294-f235f3e506b4",
    "b": "9dc4ec66-4c2b-483d-b15f-151a61d91bf9"
  }
}

Regardless of where this discussion ends up, my main desire is for this to move forward! 😄

@jugaadi
Copy link

jugaadi commented Oct 10, 2019

@jugaadi My understanding is that while a local ID could indeed be defined in a resource's meta object via a profile, referencing a local ID in relationship data requires an addition to the base spec which can only be achieved by an extension.

I'm not quite sure I understand it. I was only referring to the root meta object. I guess the best way forward would be to evolve this extension based on the community feedback as @dgeb suggested.

@jugaadi
Copy link

jugaadi commented Dec 2, 2019

Any updates?

@gabesullice
Copy link
Contributor

I think I like this better than the change described in the older lids branch.

I'm curious about why the example request includes an Accept header with the extension, rather than simply application/vnd.api+json, since the extension specifies the server must not include local:id values in the response. Is there ever a reason a client would ask for a response with this extension specifically, rather than the base application/vnd.api+json?

Great observation @freddrake, you helped @dgeb and me understand that the spec is missing some nuance in its content negotiation responsibilities (see #1461).

I think you're right about the example too. We'll get that fixed. Thanks!

@sandstrom
Copy link
Contributor

@dgeb @gabesullice I've updated my comment with a more fleshed-out example.

Let me know what you think!

@dgeb
Copy link
Member Author

dgeb commented Mar 4, 2020

I think you're right about the example too. We'll get that fixed. Thanks!

@freddrake @gabesullice I've removed the extension from both the Accept header of the example request and the Content-Type header of the example request. Thanks again for pointing this out!

@dgeb
Copy link
Member Author

dgeb commented Mar 4, 2020

@sandstrom Thank you for the continued discussion.

Position in the document is indeed enough (theoretically), however it feels somewhat brittle and I guess that's what some people (me included) have against it.

You'll have to provide an example of brittleness for me to agree here. I believe you're confusing brittleness with inability to fully support potential future extensions, not inability to support the base spec or the proposed operations extension.

Handling of side-posting for embedded records has been an known issue for several years and this would be an awesome way to fix it.

We are not seriously considering side-posting support in the base spec, since there are many limitations and edge cases that it does not support.

However, let's say that someone creates a "side-posting" extension in the future. To support server-generated IDs, that extension may rely upon this local identities extension to declare relationships among resources. The side-posting extension could then declare how to request and return a map of those local identities. That map would need to be under the sidepost (or similar) namespace (not the local namespace), which seems appropriate to me. I'd much prefer to allow that extension to absorb the complexity needed for the identity map rather than every implementation of this local identities extension.

@dgeb
Copy link
Member Author

dgeb commented Mar 4, 2020

At this point, the biggest debate I'm having re: this extension is whether lid should just be added to the base spec in v1.1 rather than introduced via an extension. The reason for my internal debate is that the concept of local identity fills a narrow edge case in the base spec when POSTing a resource with a relationship to itself (see the very example in this extension).

Should that hole be filled directly in the base spec or via extension?

@freddrake
Copy link
Contributor

@dgeb: Here's a question for you; perhaps you've thought of this use case already, but I think it's a fairly common situation.

If an application provides an "issue tracker" functionality (similar to github, gitlab, etc.), where there's an issue resource and associated change-event resources, where change-event resources contain a record of both changes to the issue and additional values (comment text, etc.), the request to make a change needs to be atomic. Should this be a PATCH for the issue, a POST to a related collection of change-event resources, or... something slightly different still?

I can imagine handling this as a PATCH to the issue resource, with an update to the relationship containing change-events, adding a new change-event resource identified by a local id, and that would need to be created as well. A POST to create a change-event first doesn't really make sense, since that's not going to be atomic.

I started asking about modeling this in the forum (https://discuss.jsonapi.org/t/post-one-resource-type-to-create-another/1830), but discussion didn't get anywhere (possibly because my explanation wasn't clear). I'm really not sure of the best way to model this in a proper REST API.


Since local ids are really only present in requests, and responses won't have them, I don't see a problem with making it an extension. What a service implementation needs to do to handle them correctly can be fairly significant, to making it clear in the headers that the additional concerns are in play might be valuable. For applications that want to take advantage of other 1.1 enhancements, but that don't need this, knowing a request can be quickly rejected might be worthwhile.

@sandstrom
Copy link
Contributor

The position of resources in a response document should be sufficient to make the association between local IDs and server-generated IDs in every scenario [dgeb]

@dgeb I agree with your statement above, it's certainly true.

Brittle is the wrong word. Perhaps implicit/obtuse is better. For example, if you rely on a serialization library it may be difficult to modify it to ensure a correct order of the returned resources, and possibly easier with something like local:map + more explicit (when you look at the payload it's more obvious than inferring from position).


However, let's say that someone creates a "side-posting" extension in the future. To support server-generated IDs, that extension may rely upon this local identities extension to declare relationships among resources. The side-posting extension could then declare how to request and return a map of those local identities. That map would need to be under the sidepost (or similar) namespace (not the local namespace), which seems appropriate to me. I'd much prefer to allow that extension to absorb the complexity needed for the identity map rather than every implementation of this local identities extension.

The self-reference use-case is a valid one. For us it's a much lesser concern than the problem with embedded records, specifically in Ember data. But what you propose (adding it to the 1.1. base spec) sounds reasonable!


For us specifically, our main pain point is around side-posting and embedded records, and how to reference them such that the client can map the embedded records it sent in, with the response it got back.

I guess there are several routes that can take us there:

  • A larger scope for lid, including a local:map
  • A narrow scope for lid + (in a next step) something more specific around side-posting

You're in a better position to determine which course of actions is best to take, so I'd be in favour of both! 😄 If I'd have to guess, maybe smaller, narrow increments are easier to make progress on.

@dgeb
Copy link
Member Author

dgeb commented Apr 16, 2020

The local identities extension proposal is being withdrawn in favor of the original lid proposal to be adopted in the base spec. Please see: #1244 (comment)

@dgeb dgeb closed this Apr 16, 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
Development

Successfully merging this pull request may close these issues.

7 participants