orbitjs / orbit

Composable data framework for ambitious web applications.
https://orbitjs.com
MIT License
2.33k stars 133 forks source link

Remotely generate IDs #849

Closed ef4 closed 3 years ago

ef4 commented 3 years ago

The orbit docs say:

Applications can take one of the following approaches to managing identity:

  1. Auto-generate IDs, typically as v4 UUIDs, and then use the same IDs locally and remotely.
  2. Auto-generate IDs locally and map those IDs to canonical IDs (or “keys”) generated remotely.
  3. Remotely generate IDs and don’t reference records until those IDs have been assigned.

From what I have discovered so far, option 1 is the default.

And I seem to be able to achieve option 2 by extending JSONAPISerializer with:

resourceKey(type) {
  return 'remoteId';
}

But I have not been able to achieve option 3. It's distinct from option 2 because I need myModel.id to reflect the server's id. The use case is porting existing read-only models from ember-data, where the IDs are used in URLs that I would rather not break.

My direct deps are @orbit/jsonapi 0.16.7 and ember-orbit 0.16.9.

dgeb commented 3 years ago

You are correct about options 1 and 2. And option 3 should be possible if all your records are created server-side and none in your client. This might be the case if you are only querying records from the server for instance.

If you are creating records client-side, then Orbit's transform builder will verify that IDs are assigned to those records and, if not, auto-generate an id. This is important if transforms are sync'd across more than one source or records are referenced by more than one operation before reaching the server (e.g. addRecord followed by replaceRelatedRecords which both reference the same record, before it can be assigned an ID by the server). If Orbit didn't generate an id locally, it couldn't track the record.

v0.17 beta simplifies the process of using server-generated URLs of any kind for individual records, since URLs that come in a links object should be maintained as a record is sync'd between sources. You can also easily use the url option in a findRecord query (or query expression) to request that URL be used by the JSONAPISource. For instance:

let latestRecord = memory.query(q => q.findRecord(record), { url: record.links.self });

v0.17 also makes it a bit easier to work with remote keys. You should just be able to define a key for each model that uses it in your schema, and then the JSONAPISource will recognize and use that key (no need to customize JSONAPISerializer). Furthermore, you can easily query by key, since you can use type / key / value wherever you can use type / id:

let latestRecord = memory.query(q => q.findRecord({ type, key, value });

With that said, I'm just now wrapping up a PR to bring some of this goodness to ember-orbit, but I can keep you posted.

ef4 commented 3 years ago

I guess what I'm finding surprising is that remote IDs are relegated to a behind-the-scenes concern while local IDs are front-and-center. I would have thought that local IDs are of interest only to Orbit and its sources, while app code almost always wants to talk about remote IDs.

From the app author's perspective, it's a bug to render a local ID in a user-facing URL:

<a href="/posts/{{this.model.id}}/detail">

What am I supposed to say instead in this spot?

Likewise for queries, it would be a bug to ever pass a local ID to findRecord, if there's any chance that the request will be sent to the server:

model({ post_id }) {
  return this.store.query(q => q.findRecord({ type: 'price', id: post_id }));
}

So instead all queries needs to be against the remote ID. How does one spell that? I don't understand your type/key/value comment, do you mean something like this?

model({ post_id }) {
  return this.store.query(q => q.findRecord({ type: 'price', key: 'remoteId', value: post_id }));
}
SafaAlfulaij commented 3 years ago

As I understood it, it is actually a good idea to do operations on local ids. For url generation, we can use the remote id directly (this.modal.remoteId), and use the third snippet to query from a given url, providing type-key-value. Otherwise, you can just do all operations with local ids, and the normalizer/mapper will map the local ids to the server ids, and save the server id (remoteId) in the map for future use.

Remote ids will be used for url generation, for queries from a url, for displaying the id to the user, and I believe that's it.

dgeb commented 3 years ago

Remote ids will be used for url generation, for queries from a url, for displaying the id to the user, and I believe that's it.

@SafaAlfulaij yes, thanks for concisely summarizing this.

I guess what I'm finding surprising is that remote IDs are relegated to a behind-the-scenes concern while local IDs are front-and-center.

@ef4 As you point out in the issue, option 1 (a single id, which can be generated on the client or server) is the default approach assumed by Orbit. I chose this default because it is simplest and works well for both online and offline apps. This default nudges developers toward accepting client-generated IDs in greenfield apps. Of course, this is not appropriate for every application and often the frontend developer does not have the luxury of choice, but it is a simple yet powerful approach in my experience.

With that said, I realize that this can feel a bit inverted and prone to error coming from an exclusively server-generated ID world, especially in v0.16. I've worked to improve the ergonomics with ID management recently, so that things are simpler and can Just Work at any layer in v0.17 regardless of your ID strategy. For instance, querying by key or id, improved normalizers, serializers, etc.

Arguably, the flexible keys system is unnecessarily flexible (and thus complex) and should be deprecated prior to v1.0. I've only ever seen applications use keys to define a single remote ID per type. Given this, perhaps the most straightforward simplification Orbit could make would be to deprecate keys and recognize remoteId as a sibling to id and keep everything else working the same. This would allow for queries such as:

model({ post_id }) {
-  return this.store.query(q => q.findRecord({ type: 'price', key: 'remoteId', value: post_id }));
+  return this.store.query(q => q.findRecord({ type: 'price', remoteId: post_id }));
}

An alternative approach that might seem more intuitive for server-generated-id scenarios would be to rename id -> lid (for "local ID", as used in json:api) and reserve id for remote IDs. However, it gets a bit more awkward for client-generated-id scenarios in that we would have to figure out whether it's important to duplicate lid to id.

One last consideration in my internal debates here is a simplification of relationship data tracking -- if an application can guarantee that IDs are universally unique across types, then we can track normalized relationship data with a single ID instead of a type / id pair. For example:

{
  type: 'planet',
  id: '4d6ad6eb-3a63-433c-a06f-93ba78c8ad1a',
  attributes: {
    name: 'Jupiter'
  },
  relationships: {
    moons: {
      data: [
-        { type: 'moon', id: '53e02af4-ac24-47e1-8f8f-885ca7dc95be' },
-        { type: 'moon', id: '1c72c694-8f5e-4a4e-a446-963e12db8175' }
+        '53e02af4-ac24-47e1-8f8f-885ca7dc95be', 
+        '1c72c694-8f5e-4a4e-a446-963e12db8175'
      ]
    }
  }
}

I'm just laying all this out there as it's all related to Orbit's rationale for record identity and the remaining questions are perhaps the most important ones to settle prior to v1.0. Of course, feel free to ignore most of this for now while you focus on getting your app up and running with the current approach :)

ef4 commented 3 years ago

For url generation, we can use the remote id directly (this.modal.remoteId)

This was the first thing I tried, but maybe I am missing some setup step because my models don't have this.model.remoteId.

What is the appropriate extension point to make that happen?

ef4 commented 3 years ago

Also, I'm hearing the subtext "don't use 0.16", which is fine but consider making that discoverable somehow. Everything currently pushes adopters to 0.16.

SafaAlfulaij commented 3 years ago

For url generation, we can use the remote id directly (this.modal.remoteId)

This was the first thing I tried, but maybe I am missing some setup step because my models don't have this.model.remoteId.

What is the appropriate extension point to make that happen?

Oops, it's this.model.keys.remoteId, sorry. I have a proxy around the model data and got confused...

ef4 commented 3 years ago

I also tried keys.remoteId, I also don’t have that.

On Sat, Jun 12, 2021 at 10:38 AM Safa Alfulaij @.***> wrote:

For url generation, we can use the remote id directly (this.modal.remoteId )

This was the first thing I tried, but maybe I am missing some setup step because my models don't have this.model.remoteId.

What is the appropriate extension point to make that happen?

Oops, it's this.model.keys.remoteId, sorry. I have a proxy around the model data and got confused...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/orbitjs/orbit/issues/849#issuecomment-860062260, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACN6MQQTFNLXMHDG4RLEWLTSNWQFANCNFSM46RT43XA .

dgeb commented 3 years ago

What is the appropriate extension point to make that happen?

To define keys for your models in ember-orbit, declare them with the @key decorator:

import { Model, attr, key } from 'ember-orbit';

export class Planet extends Model {
  @key() remoteId;
  @attr('string') name;
}

This is equivalent to the following Orbit schema:

import { RecordSchema } from '@orbit/records';

const schema = new RecordSchema({
  models: {
    planet: {
      attributes: {
        name: { type: 'string' }
      },
      keys: {
        remoteId: {}
      }
  }
});

ember-orbit flattens all fields, including keys, to be directly accessible properties on the model, so you should be able to access this key via planet.remoteId. As @SafaAlfulaij mentioned, the raw Orbit data itself is structured so the remoteId is inside a keys object.

Also, I'm hearing the subtext "don't use 0.16", which is fine but consider making that discoverable somehow. Everything currently pushes adopters to 0.16.

Sorry to confuse, I'm only pointing out awkward areas with 0.16 that have been smoothed over for 0.17. I still use 0.16 myself in some projects and it's very stable in my experience, and is recommended as it's stable and has full docs.

ef4 commented 3 years ago

Thanks, adding @key lets me read model.remoteId.

I still don't have an answer to the question: how do I query by remoteId on 0.16?

Things I have tried:

// all of these seem to silently ignore everything but `type` and 
// construct the URL `/prices` and return the whole set. Why 
// does singular `findRecord` ever give me an array? I would hope 
// that even if the server returns plural `data` it would be an error.
qb.findRecord({ type: 'price', id: remoteId });
qb.findRecord({ type: 'price', key: 'remoteId', value: remoteId });
qb.findRecord({ type: 'price', keys: { remoteId: remoteId } });

// this was just a guess and it throws
qb.findRecords('price').filter({ key: 'remoteId', value: remoteId })

// this constructs the URL /prices?filter[remoteId]=xxxx, which is not what we need
qb.findRecords('price').filter({ attribute: 'remoteId', value: remoteId })
dgeb commented 3 years ago

In v0.16, you can query the store with the async findRecordByKey:

let record = await store.findRecordByKey('price', 'remoteId', remoteId);

And you can query the cache with the sync peekRecordByKey:

let record = store.cache.peekRecordByKey('price', 'remoteId', remoteId);

I plan to deprecate these in v0.17 (but they'll still be available) in favor of the general purpose query as discussed previously. Sorry to muddy the waters re: v0.16 vs. v0.17.

ef4 commented 3 years ago

Thanks, I think that gives me the pieces I need to port this model to orbit.