Skip to content

Make Documents mutable #4548

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

Merged
merged 16 commits into from
Mar 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/firestore/src/core/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

import { LoadBundleTaskProgress } from '@firebase/firestore-types';

import { MaybeDocumentMap } from '../model/collections';
import { MaybeDocument } from '../model/document';
import { DocumentMap } from '../model/collections';
import { MutableDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { BundledDocumentMetadata as ProtoBundledDocumentMetadata } from '../protos/firestore_bundle_proto';
import {
Expand Down Expand Up @@ -46,7 +46,7 @@ export type BundledDocuments = BundledDocument[];
export class BundleLoadResult {
constructor(
readonly progress: LoadBundleTaskProgress,
readonly changedDocs: MaybeDocumentMap
readonly changedDocs: DocumentMap
) {}
}

Expand Down Expand Up @@ -89,9 +89,9 @@ export interface BundleConverter {
toDocumentKey(name: string): DocumentKey;

/**
* Converts a BundleDocument to a MaybeDocument.
* Converts a BundleDocument to a MutableDocument.
*/
toMaybeDocument(bundledDoc: BundledDocument): MaybeDocument;
toMutableDocument(bundledDoc: BundledDocument): MutableDocument;

toSnapshotVersion(time: ApiTimestamp): SnapshotVersion;
}
8 changes: 4 additions & 4 deletions packages/firestore/src/core/bundle_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
localStoreSaveNamedQuery
} from '../local/local_store_impl';
import { documentKeySet, DocumentKeySet } from '../model/collections';
import { MaybeDocument, NoDocument } from '../model/document';
import { MutableDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import {
BundleMetadata as ProtoBundleMetadata,
Expand Down Expand Up @@ -58,17 +58,17 @@ export class BundleConverterImpl implements BundleConverter {
}

/**
* Converts a BundleDocument to a MaybeDocument.
* Converts a BundleDocument to a MutableDocument.
*/
toMaybeDocument(bundledDoc: BundledDocument): MaybeDocument {
toMutableDocument(bundledDoc: BundledDocument): MutableDocument {
if (bundledDoc.metadata.exists) {
debugAssert(
!!bundledDoc.document,
'Document is undefined when metadata.exist is true.'
);
return fromDocument(this.serializer, bundledDoc.document!, false);
} else {
return new NoDocument(
return MutableDocument.newNoDocument(
this.toDocumentKey(bundledDoc.metadata.name!),
this.toSnapshotVersion(bundledDoc.metadata.readTime!)
);
Expand Down
10 changes: 5 additions & 5 deletions packages/firestore/src/core/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
localStoreReadDocument
} from '../local/local_store_impl';
import { Persistence } from '../local/persistence';
import { Document, NoDocument } from '../model/document';
import { Document } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { Mutation } from '../model/mutation';
import { toByteStreamReader } from '../platform/byte_stream_reader';
Expand Down Expand Up @@ -498,10 +498,10 @@ async function readDocumentFromCache(
result: Deferred<Document | null>
): Promise<void> {
try {
const maybeDoc = await localStoreReadDocument(localStore, docKey);
if (maybeDoc instanceof Document) {
result.resolve(maybeDoc);
} else if (maybeDoc instanceof NoDocument) {
const document = await localStoreReadDocument(localStore, docKey);
if (document.isFoundDocument()) {
result.resolve(document);
} else if (document.isNoDocument()) {
result.resolve(null);
} else {
result.reject(
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/src/core/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ export function stringifyQuery(query: Query): string {
/** Returns whether `doc` matches the constraints of `query`. */
export function queryMatches(query: Query, doc: Document): boolean {
return (
doc.isFoundDocument() &&
queryMatchesPathAndCollectionGroup(query, doc) &&
queryMatchesOrderBy(query, doc) &&
queryMatchesFilters(query, doc) &&
Expand Down Expand Up @@ -491,7 +492,7 @@ function queryMatchesPathAndCollectionGroup(
function queryMatchesOrderBy(query: Query, doc: Document): boolean {
for (const orderBy of query.explicitOrderBy) {
// order by key always matches
if (!orderBy.field.isKeyField() && doc.field(orderBy.field) === null) {
if (!orderBy.field.isKeyField() && doc.data.field(orderBy.field) === null) {
return false;
}
}
Expand Down
14 changes: 7 additions & 7 deletions packages/firestore/src/core/sync_engine_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ import { TargetData, TargetPurpose } from '../local/target_data';
import {
DocumentKeySet,
documentKeySet,
MaybeDocumentMap
DocumentMap
} from '../model/collections';
import { MaybeDocument, NoDocument } from '../model/document';
import { MutableDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { Mutation } from '../model/mutation';
import { MutationBatchResult } from '../model/mutation_batch';
Expand Down Expand Up @@ -161,7 +161,7 @@ class LimboResolution {
*/
type ApplyDocChangesHandler = (
queryView: QueryView,
changes: MaybeDocumentMap,
changes: DocumentMap,
remoteEvent?: RemoteEvent
) => Promise<ViewSnapshot | undefined>;

Expand Down Expand Up @@ -618,12 +618,12 @@ export async function syncEngineRejectListen(
// This is kind of a hack. Ideally, we would have a method in the local
// store to purge a document. However, it would be tricky to keep all of
// the local store's invariants with another method.
let documentUpdates = new SortedMap<DocumentKey, MaybeDocument>(
let documentUpdates = new SortedMap<DocumentKey, MutableDocument>(
DocumentKey.comparator
);
documentUpdates = documentUpdates.insert(
limboKey,
new NoDocument(limboKey, SnapshotVersion.min())
MutableDocument.newNoDocument(limboKey, SnapshotVersion.min())
);
const resolvedLimboDocuments = documentKeySet().add(limboKey);
const event = new RemoteEvent(
Expand Down Expand Up @@ -1001,7 +1001,7 @@ export function syncEngineGetEnqueuedLimboDocumentResolutions(

export async function syncEngineEmitNewSnapsAndNotifyLocalStore(
syncEngine: SyncEngine,
changes: MaybeDocumentMap,
changes: DocumentMap,
remoteEvent?: RemoteEvent
): Promise<void> {
const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl);
Expand Down Expand Up @@ -1052,7 +1052,7 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore(
async function applyDocChanges(
syncEngineImpl: SyncEngineImpl,
queryView: QueryView,
changes: MaybeDocumentMap,
changes: DocumentMap,
remoteEvent?: RemoteEvent
): Promise<ViewSnapshot | undefined> {
let viewDocChanges = queryView.view.computeDocChanges(changes);
Expand Down
12 changes: 6 additions & 6 deletions packages/firestore/src/core/target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export class FieldFilter extends Filter {
}

matches(doc: Document): boolean {
const other = doc.field(this.field);
const other = doc.data.field(this.field);
// Types do not have to match in NOT_EQUAL filters.
if (this.op === Operator.NOT_EQUAL) {
return (
Expand Down Expand Up @@ -459,7 +459,7 @@ export class ArrayContainsFilter extends FieldFilter {
}

matches(doc: Document): boolean {
const other = doc.field(this.field);
const other = doc.data.field(this.field);
return isArray(other) && arrayValueContains(other.arrayValue, this.value);
}
}
Expand All @@ -472,7 +472,7 @@ export class InFilter extends FieldFilter {
}

matches(doc: Document): boolean {
const other = doc.field(this.field);
const other = doc.data.field(this.field);
return other !== null && arrayValueContains(this.value.arrayValue!, other);
}
}
Expand All @@ -490,7 +490,7 @@ export class NotInFilter extends FieldFilter {
) {
return false;
}
const other = doc.field(this.field);
const other = doc.data.field(this.field);
return other !== null && !arrayValueContains(this.value.arrayValue!, other);
}
}
Expand All @@ -503,7 +503,7 @@ export class ArrayContainsAnyFilter extends FieldFilter {
}

matches(doc: Document): boolean {
const other = doc.field(this.field);
const other = doc.data.field(this.field);
if (!isArray(other) || !other.arrayValue.values) {
return false;
}
Expand Down Expand Up @@ -588,7 +588,7 @@ export function sortsBeforeDocument(
doc.key
);
} else {
const docValue = doc.field(orderByComponent.field);
const docValue = doc.data.field(orderByComponent.field);
debugAssert(
docValue !== null,
'Field should exist since document matched the orderBy already.'
Expand Down
18 changes: 6 additions & 12 deletions packages/firestore/src/core/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { ParsedSetData, ParsedUpdateData } from '../lite/user_data_reader';
import { Document, MaybeDocument, NoDocument } from '../model/document';
import { Document } from '../model/document';
import { DocumentKey } from '../model/document_key';
import {
DeleteMutation,
Expand Down Expand Up @@ -60,7 +60,7 @@ export class Transaction {

constructor(private datastore: Datastore) {}

async lookup(keys: DocumentKey[]): Promise<MaybeDocument[]> {
async lookup(keys: DocumentKey[]): Promise<Document[]> {
this.ensureCommitNotCalled();

if (this.mutations.length > 0) {
Expand All @@ -70,13 +70,7 @@ export class Transaction {
);
}
const docs = await invokeBatchGetDocumentsRpc(this.datastore, keys);
docs.forEach(doc => {
if (doc instanceof NoDocument || doc instanceof Document) {
this.recordVersion(doc);
} else {
fail('Document in a transaction was a ' + doc.constructor.name);
}
});
docs.forEach(doc => this.recordVersion(doc));
return docs;
}

Expand Down Expand Up @@ -120,12 +114,12 @@ export class Transaction {
this.committed = true;
}

private recordVersion(doc: MaybeDocument): void {
private recordVersion(doc: Document): void {
let docVersion: SnapshotVersion;

if (doc instanceof Document) {
if (doc.isFoundDocument()) {
docVersion = doc.version;
} else if (doc instanceof NoDocument) {
} else if (doc.isNoDocument()) {
// For deleted docs, we must use baseVersion 0 when we overwrite them.
docVersion = SnapshotVersion.min();
} else {
Expand Down
Loading