orbitjs / orbit

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

Memory Cache won't update relationships' meta if identities match existing record #953

Open bradjones1 opened 2 years ago

bradjones1 commented 2 years ago

I'm not sure if this is a bug, a documentation issue or simply "tough luck, Brad" but here's what I've spent my afternoon on:

I have a Drupal json:api backend that serves up rather vanilla resources in compliance with the spec. Orbit has been working really well for me in that regard, with some tweaks to the initial client setup like overriding the URL builder, etc.

Here's the rub: I am using Drupal's consumer_image_styles module to add a meta member to the resource identifier object, which contains links to what Drupal calls "image derivatives," or basically URLs for different sizes and transformations of the referenced image.

(Yes, I'd say it's better to put this in the meta member of the resource linkage object, but the way Drupal implements hooks to alter the output of the field, it's much easier to put this inside of the data resource identifier object, and it's still valid. If not entirely semantically perfect.)

In my case, I make my life more difficult by signing these derivative URLs, because they reference access-controlled content. The URLs might also change due to attributes of the user accessing them (which is a whole other ball of wax.) The point is, though, that the data contained inside the meta member of the resource linkage may change, while the resource identities will not.

Problem is, Orbit's memory source throws these changes away, because the relationship identities do not change. (They're compared to match type and id of the current record.) This is in the "Inverse transform operator", but I will confess I'm not entirely sure what an inverse transform is yet.

It seems like in theory this could be overridden during setup, but it raises the questions:

  1. Is this the desired behavior? Could we perhaps always overwrite if we have a meta object in the resource identifier, or broaden the test to compare the contents of the two resource identifier objects more generally?
  2. If this isn't a bug, what's the "right" way to work around this? It looks like I'd need to... pass settings.inverseTransformOperators to a customized transform buffer on my MemoryCache instance?
bradjones1 commented 2 years ago

I understand of course that some of this is a little json:api-specific, which may play into the answer. (And, come to think of it, is why this works the way it does, because while Orbit's data model looks very json:api-esque, the data model is not json:api itself.)

bradjones1 commented 2 years ago

Update: rolled my own, changed to

relationshipChanged = !equalRecordIdentities(currentData, data) || !!data?.meta;
dgeb commented 2 years ago

Since Orbit's record relationships do support meta members, I agree that changes to these fields should be captured. This was an oversight that should be corrected. I suppose we should do a deep equality check on the meta objects.

Thanks for raising this!

bradjones1 commented 2 years ago

Related: https://www.drupal.org/project/consumer_image_styles/issues/3277529#comment-14496957

bradjones1 commented 2 years ago

Related, I just discovered that the meta data is not persisted in the cache, at all. This may be a separate issue but I think it's related in spirit, anyway.

  1. Create a live query for records of a certain type.
  2. Run an update query that creates a record of said type remotely (with a coordinator that will sync it to the memory source/cache.)
  3. The record that is available in the live query subscriber via update.query() does not have any of the relationship's meta info.
bradjones1 commented 2 years ago

I suppose we should do a deep equality check on the meta objects.

Linked PR just checks if there is meta, and if so, let's update it anyway. I imagine that should be sufficient, or is that not performant enough?