Add SimpleQueryEngine #2160
Add SimpleQueryEngine #2160
Conversation
schmidt-sebastian
changed the base branch from
master
to
mrschmidt/indexfree-master
schmidt-sebastian
force-pushed
the
mrschmidt/simplequeryengine
branch
from
005936a to
f574c0b
Compare
| @@ -175,6 +177,7 @@ export class LocalStore { | |||
| this.mutationQueue, | |||
| this.persistence.getIndexManager() | |||
| ); | |||
| this.queryEngine.setLocalDocumentsView(this.localDocuments); | |||
There was a problem hiding this comment.
How come we construct the QueryEngine in FirestoreClient and pass it into LocalStore, rather than just construct it here in LocalStore? If we could do that, then we wouldn't need this awkward setLocalDocumentsView() method that must be called before use...
There was a problem hiding this comment.
It allows to parameterize the tests for the LocalStore. Fun times.
There was a problem hiding this comment.
It's tempting to suggest that we just have a useIndexFreeQueryEngine boolean argument or something that tests can use... But I don't care that much.
| ); | ||
|
|
||
| // TODO: Once LocalDocumentsView provides a getCollectionDocuments() | ||
| // method, we should call that here and then filter the results. |
There was a problem hiding this comment.
Hrm. What's the context for this TODO (other than the fact that it exists in Android too)? Why do we expect LocalDocumentsView is going to provide a getCollectionDocuments() method in the future?
There was a problem hiding this comment.
Hm. I don't know. I ported it cause it was there an Android. Unfortunately, all traces of who added it are gone by now. I could invest some research and dig this up on Codesearch, but I suspect you would not like the outcome :)
There was a problem hiding this comment.
Ohhh... GRRRR... I forgot about that. I suspect it used to be TODO(indexing) which would imply it doesn't apply to web. So I would prefer if you don't port it. But that means you have to run CI again, so maybe not worth removing (it's friday, YOLO, etc.)
There was a problem hiding this comment.
I'll drop it in the next PR.
|
Let me know if you want me to remove the TODO, otherwise I will merge once CI goes green. |
Adds SimpleQueryEngine and wires it up.
Android: https://github.com/firebase/firebase-android-sdk/blob/715f3f03a96681e8dfd816b2645276b2cd58b8d7/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SimpleQueryEngine.java
https://github.com/firebase/firebase-android-sdk/blob/715f3f03a96681e8dfd816b2645276b2cd58b8d7/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java