-
Notifications
You must be signed in to change notification settings - Fork 945
Implement bundle features in local store. #3200
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
Changes from 21 commits
9602712
c5e783e
5e7fb89
1ee1615
18f0be1
aa455bf
78248cd
83160a1
24e10cb
9d6edc5
4cbe608
4313e51
296cfc4
fff3d36
fb762de
1ec4182
cd3ab7a
d991c75
af097c5
8f0d5da
7ea8335
5a38f0a
9340d6a
6fc7f4a
bdbd70a
8a94ed5
6fc92ee
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 | ||||
---|---|---|---|---|---|---|
|
@@ -17,6 +17,12 @@ | |||||
|
||||||
import { Query } from './query'; | ||||||
import { SnapshotVersion } from './snapshot_version'; | ||||||
import { JsonProtoSerializer } from '../remote/serializer'; | ||||||
import * as bundleProto from '../protos/firestore_bundle_proto'; | ||||||
import * as api from '../protos/firestore_proto_api'; | ||||||
import { DocumentKey } from '../model/document_key'; | ||||||
import { MaybeDocument, NoDocument } from '../model/document'; | ||||||
import { debugAssert } from '../util/assert'; | ||||||
|
||||||
/** | ||||||
* Represents a Firestore bundle saved by the SDK in its local storage. | ||||||
|
@@ -40,3 +46,43 @@ export interface NamedQuery { | |||||
/** The time at which the results for this query were read. */ | ||||||
readonly readTime: SnapshotVersion; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Represents the [metadata, document] pairs from the bundles in an array. | ||||||
*/ | ||||||
export type BundledDocuments = Array< | ||||||
[bundleProto.BundledDocumentMetadata, api.Document | undefined] | ||||||
>; | ||||||
|
||||||
/** | ||||||
* Helper to convert objects from bundles to model objects in the SDK. | ||||||
*/ | ||||||
export class BundleConverter { | ||||||
constructor(private serializer: JsonProtoSerializer) {} | ||||||
|
||||||
toDocumentKey(name: string): DocumentKey { | ||||||
schmidt-sebastian marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return this.serializer.fromName(name); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Converts a [metadata, document] pair to a MaybeDocument. | ||||||
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.
Suggested change
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. Done. |
||||||
*/ | ||||||
toMaybeDocument( | ||||||
metadata: bundleProto.BundledDocumentMetadata, | ||||||
doc: api.Document | undefined | ||||||
): MaybeDocument { | ||||||
if (metadata.exists) { | ||||||
debugAssert(!!doc, 'Document is undefined when metadata.exist is true.'); | ||||||
return this.serializer.fromDocument(doc!, false); | ||||||
} else { | ||||||
return new NoDocument( | ||||||
this.toDocumentKey(metadata.name!), | ||||||
this.toSnapshotVersion(metadata.readTime!) | ||||||
); | ||||||
} | ||||||
} | ||||||
|
||||||
toSnapshotVersion(time: api.Timestamp): SnapshotVersion { | ||||||
return this.serializer.fromVersion(time); | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,7 @@ import { | |
MemoryEagerDelegate, | ||
MemoryPersistence | ||
} from '../local/memory_persistence'; | ||
import { BundleConverter } from './bundle'; | ||
|
||
const MEMORY_ONLY_PERSISTENCE_ERROR_MESSAGE = | ||
'You are using the memory-only build of Firestore. Persistence support is ' + | ||
|
@@ -125,10 +126,12 @@ export class MemoryComponentProvider implements ComponentProvider { | |
} | ||
|
||
createLocalStore(cfg: ComponentConfiguration): LocalStore { | ||
const serializer = cfg.platform.newSerializer(cfg.databaseInfo.databaseId); | ||
return new LocalStore( | ||
this.persistence, | ||
new IndexFreeQueryEngine(), | ||
cfg.initialUser | ||
cfg.initialUser, | ||
new BundleConverter(serializer) | ||
); | ||
} | ||
|
||
|
@@ -209,10 +212,12 @@ export class IndexedDbComponentProvider extends MemoryComponentProvider { | |
} | ||
|
||
createLocalStore(cfg: ComponentConfiguration): LocalStore { | ||
const serializer = cfg.platform.newSerializer(cfg.databaseInfo.databaseId); | ||
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. Consider sharing this serializer instance with 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. Done. |
||
return new MultiTabLocalStore( | ||
this.persistence, | ||
new IndexFreeQueryEngine(), | ||
cfg.initialUser | ||
cfg.initialUser, | ||
new BundleConverter(serializer) | ||
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. You could also just pass the Serializer or Database ID and handle initializing BundleConverter inside LocalStore, which would greatly increase encapsulation. 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. Done. |
||
); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -445,20 +445,20 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { | |
primitiveComparator(l.canonicalString(), r.canonicalString()) | ||
); | ||
|
||
this.changes.forEach((key, maybeDocument) => { | ||
this.changes.forEach((key, documentChange) => { | ||
const previousSize = this.documentSizes.get(key); | ||
debugAssert( | ||
previousSize !== undefined, | ||
`Cannot modify a document that wasn't read (for ${key})` | ||
); | ||
if (maybeDocument) { | ||
if (documentChange.maybeDoc) { | ||
debugAssert( | ||
!this.readTime.isEqual(SnapshotVersion.min()), | ||
!this.getReadTime(key).isEqual(SnapshotVersion.min()), | ||
'Cannot add a document with a read time of zero' | ||
); | ||
const doc = this.documentCache.serializer.toDbRemoteDocument( | ||
maybeDocument, | ||
this.readTime | ||
documentChange.maybeDoc!, | ||
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. Drop bang here. 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. Done. |
||
this.getReadTime(key) | ||
); | ||
collectionParents = collectionParents.add(key.path.popLast()); | ||
|
||
|
@@ -474,7 +474,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { | |
// preserved in `getNewDocumentChanges()`. | ||
const deletedDoc = this.documentCache.serializer.toDbRemoteDocument( | ||
new NoDocument(key, SnapshotVersion.min()), | ||
this.readTime | ||
this.getReadTime(key) | ||
); | ||
promises.push( | ||
this.documentCache.addEntry(transaction, key, deletedDoc) | ||
|
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.
I would prefer if you a nested object here:
Array<{metadata: ..., document?: }>
. Index-based typing is very hard to parse for readers of the code.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.
Done.