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

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

Open
bradjones1 opened this issue Apr 21, 2022 · 6 comments · May be fixed by #962
Open

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

bradjones1 opened this issue Apr 21, 2022 · 6 comments · May be fixed by #962

Comments

@bradjones1
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Update: rolled my own, changed to

relationshipChanged = !equalRecordIdentities(currentData, data) || !!data?.meta;

@dgeb
Copy link
Member

dgeb commented Apr 24, 2022

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
Copy link
Contributor Author

@bradjones1
Copy link
Contributor Author

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
Copy link
Contributor Author

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?

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 a pull request may close this issue.

2 participants