-
Notifications
You must be signed in to change notification settings - Fork 944
Add SimpleQueryEngine #2160
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
Add SimpleQueryEngine #2160
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/** | ||
* @license | ||
* Copyright 2019 Google Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import { Query } from '../core/query'; | ||
import { SnapshotVersion } from '../core/snapshot_version'; | ||
import { DocumentMap } from '../model/collections'; | ||
import { LocalDocumentsView } from './local_documents_view'; | ||
import { PersistenceTransaction } from './persistence'; | ||
import { PersistencePromise } from './persistence_promise'; | ||
|
||
/** | ||
* Represents a query engine capable of performing queries over the local | ||
* document cache. You must call `setLocalDocumentsView()` before using. | ||
*/ | ||
export interface QueryEngine { | ||
/** Sets the document view to query against. */ | ||
setLocalDocumentsView(localDocuments: LocalDocumentsView): void; | ||
|
||
/** Returns all local documents matching the specified query. */ | ||
getDocumentsMatchingQuery( | ||
transaction: PersistenceTransaction, | ||
query: Query, | ||
sinceReadTime: SnapshotVersion | ||
): PersistencePromise<DocumentMap>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/** | ||
* @license | ||
* Copyright 2019 Google Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import { SnapshotVersion } from '../core/snapshot_version'; | ||
import { Query } from '../core/query'; | ||
import { DocumentMap } from '../model/collections'; | ||
import { assert } from '../util/assert'; | ||
import { LocalDocumentsView } from './local_documents_view'; | ||
import { QueryEngine } from './query_engine'; | ||
import { PersistenceTransaction } from './persistence'; | ||
import { PersistencePromise } from './persistence_promise'; | ||
|
||
/** | ||
* A naive implementation of QueryEngine that just loads all the documents in | ||
* the queried collection and then filters them in memory. | ||
*/ | ||
export class SimpleQueryEngine implements QueryEngine { | ||
private localDocumentsView: LocalDocumentsView | undefined; | ||
|
||
setLocalDocumentsView(localDocuments: LocalDocumentsView): void { | ||
this.localDocumentsView = localDocuments; | ||
} | ||
|
||
/** Returns all local documents matching the specified query. */ | ||
getDocumentsMatchingQuery( | ||
transaction: PersistenceTransaction, | ||
query: Query, | ||
sinceReadTime: SnapshotVersion | ||
): PersistencePromise<DocumentMap> { | ||
assert( | ||
this.localDocumentsView !== undefined, | ||
'setLocalDocumentsView() not called' | ||
); | ||
|
||
// TODO: Once LocalDocumentsView provides a getCollectionDocuments() | ||
// method, we should call that here and then filter the results. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll drop it in the next PR. |
||
return this.localDocumentsView!.getDocumentsMatchingQuery( | ||
transaction, | ||
query, | ||
SnapshotVersion.MIN | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It allows to parameterize the tests for the LocalStore. Fun times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.