Implement live query on @orbit/record-cache

[tchak]

This a new proposition to introduce query observers. This one is not relying on any known interface such as RX or async iterator. It uses orbit events. It does expose an LiveQuery interface, but it is simply an object with a subscribe method with returns a subscription with an unsubscribe method.

The current one has a downside of looking like an RX/Zen Observable but not being one. I am worried it might confuse folks. One option I considered is to pass the callback directly to liveQuery method, but it makes it for a complex signature because of optional options and id arguments...

What do you think @dgeb?

[tchak]

I changed QueryObserver interface from

interface QueryObserverSubscription {
  unsubscribe(): void;
}

interface QueryObserver {
  subscribe(listener: Listener): QueryObserverSubscription
}

to

interface LiveQuery {
  on(listener: (result: QueryResult) => void): () => void
}

so it is more in line with orbit vocabulary and less prompt to create confusion with an actual observable

[tchak]

I am not sure about change event. Maybe I should just use recordOperationChange in the QueryObserver implementation.

[tchak]

Right now I added bits all around code base. Maybe it all should be just part of a new @obit/query-observer package used by @orbit/memory to expose observeQuery.

[tchak]

Also maybe observeQuery should live in the record-cache package

[selvagsz]

@tchak @dgeb sorry for being too naive here. But, what is a "live query" or "query observer" ?

[tchak]

@selvagsz it is a way to observe a query result. Example:

memory.cache.liveQuery(q => q.findRecords('planet')).on((planets => {
  console.log('>', planets);
});

memory.update(t => t.addRecord({ type: 'planet', id: 'earth' }));
memory.update(t => t.addRecord({ type: 'planet', id: 'mars' }));

// will print:
// > []
// > [{ type: 'planet', id: 'earth' }]
// > [{ type: 'planet', id: 'earth' }, { type: 'planet', id: 'mars' }]

[tchak]

I went ahead and moved everything to @orbit/record-cache

[tchak]

I am getting quite happy with this. The main concern I have is error handling. I am dealing with RecordNotFound exceptions in a meaningful way by returning empty data. I think this is what one might expect on a live query. But all other errors are just thrown into the void. We might want to add a second error callback.

I guess I am just a bit unhappy that we are basically reinventing Observable interface...

[selvagsz]

@tchak where can I find the motivation or use case for this feature ?

[tchak]

For prior art you can check:

• ember-orbit has liveQueryhttps://github.com/orbitjs/ember-orbit/blob/03c03026c75314766afaaa2b986106989a7623ba/addon/-private/cache.ts#L234
• apollo has watchQuery https://www.apollographql.com/docs/react/api/apollo-client/#apolloclient-functions
• In ember-data all collections are reactive

If we want to implement any reactive functionality on top of orbit (like a react hook) we would need this.

[tchak]

@selvagsz I can see you have implemented some react hooks: https://github.com/selvagsz/react-orbit-example/blob/master/src/hooks/useQuery.js
This addition will make such implementations easier and more reliable. Subscribing to transform is a way too low level api.

[tchak]

added proper error handling

[selvagsz]

@tchak I was re-implementing the react orbit hooks on top of this PR. I hit 2 issues,

1. When the findRecords query is resolved, the liveQuery listener is called for each transform operation. For eg., if the response resulted in 50 records, the listener is called 50 times because of listening to the patch event

2. When the findRecords response contains side-loaded (included) records, source.cache.query(t => t.findRelatedRecords) results in RecordNotFoundException even though the record being present in the included key in the response.

[selvagsz]

@tchak this impl. is 🔥🔥🔥 Was able to get an optimal implementation of react-orbit hook with this record change detection on the source's transform event instead of cache's patch event.

[tchak]

Thank you for giving it a go!

I definitely remember @dgeb explaining in some issue why listening to transform is probably not what we want. I think we might miss some changes.

When the findRecords query is resolved, the liveQuery listener is called for each transform operation. For eg., if the response resulted in 50 records, the listener is called 50 times because of listening to the patch event

Yeah, this is going to be a problem... I think we can fix this with some sort of async coalescing. I am also pretty sure that the whole "dirty check" logic in LiveQuery can be made better.

When the findRecords response contains side-loaded (included) records, source.cache.query(t => t.findRelatedRecords) results in RecordNotFoundException even though the record being present in the included key in the response.

Not sure what's going on here

[tchak]

Ideally I think a QueryExpression should be serialisable as some sort of cache key. It would make "dirty checking" and coalescing easier.

[tchak]

@selvagsz I think I fixed the over fetching issue. I will add more tests tomorrow to be sure :)

[selvagsz]

@tchak Can confirm. Over-firing of the listener is fixed with the tick change. The other issue related to RecordNotFoundException in the listener of liveQuery also got fixed 🎉

[selvagsz]

@tchak any reason to immediately execute the listener ? Does it make sense to add an api for this similar to ember debounce method debounce (target, method, args*, wait, immediate)

[tchak]

I made another change to the interface. So first execution needs to be explicit now.

const subscription = store.cache
  .liveQuery(q => q.findRecords('planet'))
  .subscribe(fn);

subscription.execute();

subscribe(): LiveQuerySubscription;

interface LiveQuerySubscription {
  unsubscribe(): void;
  execute(): void;
}

[brumm]

I'm curious about the store.cache usage here, does this mean that liveQuery is only available on the local cache, not the actual store? And would this mean that a coordinator strategy that auto-pulls data from a remote would not be part of this liveQuery process?

[tchak]

liveQuery as defined in this PR is a cache feature not a source feature. I would like to have a conversation about remote subscriptions, but it is a quite different topic. Regarding a setup with a remote source – if your remote source is syncing back to memory source 'liveQuery' will pick up changes.

[brumm]

Ah, I see, thanks for explaining! Our team is currently looking at orbitjs as a backing store for our json:api connected react application. One thing I'd like to solve is auto-pull of missing data, together with a reactive connection to the query that triggered the pull. It seems like this is not quite there yet - We'll continue to watch this space :)

[tchak]

so would it help you if in the query/pull flow there was a subscribe/unsubscribe event on the source? Or something similar. What is your server implementation like? Is your server able to push diff operations for a registered query?

[tchak]

For example in one app I am trying to use something like this : https://gist.github.com/tchak/2a23843c35617a265e75c136afc99b77

[dgeb]

@tchak great work! I really like the new interfaces we settled on.

IMO this just needs some meta and links handling, but that doesn't have to happen in this PR (your choice).

[dgeb]

I think we'll also need to track changes to meta and links (which are currently only possible via addRecord / updateRecord ops).

[dgeb]

Check for record.meta and record.links.

[tchak]

I am realizing that current implementation of change matching is not using attributes or keys (nor meta or links). Right now the algorithm is just to check if the identity or relationship identity is the same, we consider change relevant. So in practice we could only expose identity, relationships, and removed in the RecordChange interface. But we also may improuve the algorith in the future and use extra information.