Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Compat class for DocumentSnapshot #4051

Merged
merged 6 commits into from
Nov 11, 2020
Merged

Compat class for DocumentSnapshot #4051

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
30c25d2
Compat class for DocumentSnapshot
schmidt-sebastian Nov 9, 2020
04a0d04
Introduce different UserDataWriters
schmidt-sebastian Nov 10, 2020
c21e02b
Create fast-impalas-tickle.md
schmidt-sebastian Nov 11, 2020
084c347
Merge branch 'master' into mrschmidt/documentsnapshot-compat-v2
schmidt-sebastian Nov 11, 2020
5e93783
Merge branch 'master' into mrschmidt/documentsnapshot-compat-v2
schmidt-sebastian Nov 11, 2020
6f8e9a3
Debug
schmidt-sebastian Nov 11, 2020
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
20 changes: 16 additions & 4 deletions packages/firestore/exp/src/api/reference.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
CollectionReference,
doc,
DocumentReference,
newExpUserDataWriter,
newUserDataReader,
Query,
SetOptions,
Expand Down Expand Up @@ -140,6 +141,7 @@ export function getDocFromCache<T>(
): Promise<DocumentSnapshot<T>> {
const firestore = cast(reference.firestore, FirebaseFirestore);
const client = ensureFirestoreConfigured(firestore);
const userDataWriter = newExpUserDataWriter(firestore, reference._converter);

const deferred = new Deferred<Document | null>();
firestore._queue.enqueueAndForget(async () => {
Expand All @@ -150,6 +152,7 @@ export function getDocFromCache<T>(
doc =>
new DocumentSnapshot(
firestore,
userDataWriter,
reference._key,
doc,
new SnapshotMetadata(
Expand Down Expand Up @@ -203,6 +206,7 @@ export function getDocFromServer<T>(
export function getDocs<T>(query: Query<T>): Promise<QuerySnapshot<T>> {
const firestore = cast(query.firestore, FirebaseFirestore);
const client = ensureFirestoreConfigured(firestore);
const userDataWriter = newExpUserDataWriter(firestore, query._converter);

validateHasExplicitOrderByForLimitToLast(query._query);

Expand All @@ -218,7 +222,7 @@ export function getDocs<T>(query: Query<T>): Promise<QuerySnapshot<T>> {
);
});
return deferred.promise.then(
snapshot => new QuerySnapshot(firestore, query, snapshot)
snapshot => new QuerySnapshot(firestore, userDataWriter, query, snapshot)
);
}

Expand All @@ -233,14 +237,15 @@ export function getDocsFromCache<T>(
): Promise<QuerySnapshot<T>> {
const firestore = cast(query.firestore, FirebaseFirestore);
const client = ensureFirestoreConfigured(firestore);
const userDataWriter = newExpUserDataWriter(firestore, query._converter);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: you could save a little bit of memory by deferring the creation of the userDataWriter until you need it just as you're creating the QuerySnapshot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I slightly prefer the current arrangement as I can use the same pattern across all functions. It also keeps the assignment in the Lambda shorter, which makes it fit one in line.


const deferred = new Deferred<ViewSnapshot>();
firestore._queue.enqueueAndForget(async () => {
const localStore = await getLocalStore(client);
await executeQueryFromCache(localStore, query._query, deferred);
});
return deferred.promise.then(
snapshot => new QuerySnapshot(firestore, query, snapshot)
snapshot => new QuerySnapshot(firestore, userDataWriter, query, snapshot)
);
}

Expand All @@ -255,6 +260,7 @@ export function getDocsFromServer<T>(
): Promise<QuerySnapshot<T>> {
const firestore = cast(query.firestore, FirebaseFirestore);
const client = ensureFirestoreConfigured(firestore);
const userDataWriter = newExpUserDataWriter(firestore, query._converter);

const deferred = new Deferred<ViewSnapshot>();
firestore._queue.enqueueAndForget(async () => {
Expand All @@ -268,7 +274,7 @@ export function getDocsFromServer<T>(
);
});
return deferred.promise.then(
snapshot => new QuerySnapshot(firestore, query, snapshot)
snapshot => new QuerySnapshot(firestore, userDataWriter, query, snapshot)
);
}

Expand Down Expand Up @@ -701,12 +707,16 @@ export function onSnapshot<T>(
} else {
firestore = cast(reference.firestore, FirebaseFirestore);
internalQuery = reference._query;
const userDataWriter = newExpUserDataWriter(
firestore,
reference._converter
);

observer = {
next: snapshot => {
if (args[currArg]) {
(args[currArg] as NextFn<QuerySnapshot<T>>)(
new QuerySnapshot(firestore, reference, snapshot)
new QuerySnapshot(firestore, userDataWriter, reference, snapshot)
);
}
},
Expand Down Expand Up @@ -836,8 +846,10 @@ function convertToDocSnapshot<T>(
);
const doc = snapshot.docs.get(ref._key);

const userDataWriter = newExpUserDataWriter(firestore, ref._converter);
return new DocumentSnapshot(
firestore,
userDataWriter,
ref._key,
doc,
new SnapshotMetadata(snapshot.hasPendingWrites, snapshot.fromCache),
Expand Down
41 changes: 16 additions & 25 deletions packages/firestore/exp/src/api/snapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,14 @@

import { DocumentKey } from '../../../src/model/document_key';
import { Document } from '../../../src/model/document';
import {
ServerTimestampBehavior,
UserDataWriter
} from '../../../src/api/user_data_writer';
import { UserDataWriter } from '../../../src/api/user_data_writer';
import {
DocumentSnapshot as LiteDocumentSnapshot,
fieldPathFromArgument
} from '../../../lite/src/api/snapshot';
import { FirebaseFirestore } from './database';
import {
DocumentData,
DocumentReference,
Query,
queryEqual,
SetOptions
Expand All @@ -41,9 +37,7 @@ import { Code, FirestoreError } from '../../../src/util/error';
import { ViewSnapshot } from '../../../src/core/view_snapshot';
import { FieldPath } from '../../../lite/src/api/field_path';
import { SnapshotListenOptions } from './reference';
import { Bytes } from '../../../lite/src/api/bytes';

const DEFAULT_SERVER_TIMESTAMP_BEHAVIOR: ServerTimestampBehavior = 'none';
import { UntypedFirestoreDataConverter } from '../../../src/api/user_data_reader';

/**
* Converter used by `withConverter()` to transform user objects of type `T`
Expand Down Expand Up @@ -187,12 +181,13 @@ export class DocumentSnapshot<T = DocumentData> extends LiteDocumentSnapshot<

constructor(
readonly _firestore: FirebaseFirestore,
userDataWriter: UserDataWriter,
key: DocumentKey,
document: Document | null,
metadata: SnapshotMetadata,
converter: FirestoreDataConverter<T> | null
converter: UntypedFirestoreDataConverter<T> | null
) {
super(_firestore, key, document, converter);
super(_firestore, userDataWriter, key, document, converter);
this._firestoreImpl = _firestore;
this.metadata = metadata;
}
Expand All @@ -219,29 +214,26 @@ export class DocumentSnapshot<T = DocumentData> extends LiteDocumentSnapshot<
* @return An `Object` containing all fields in the document or `undefined` if
* the document doesn't exist.
*/
data(options?: SnapshotOptions): T | undefined {
data(options: SnapshotOptions = {}): T | undefined {
if (!this._document) {
return undefined;
} else if (this._converter) {
// We only want to use the converter and create a new DocumentSnapshot
// if a converter has been provided.
const snapshot = new QueryDocumentSnapshot(
this._firestore,
this._userDataWriter,
this._key,
this._document,
this.metadata,
/* converter= */ null
);
return this._converter.fromFirestore(snapshot, options);
} else {
const userDataWriter = new UserDataWriter(
this._firestoreImpl._databaseId,
options?.serverTimestamps || DEFAULT_SERVER_TIMESTAMP_BEHAVIOR,
key =>
new DocumentReference(this._firestore, /* converter= */ null, key),
bytes => new Bytes(bytes)
);
return userDataWriter.convertValue(this._document.toProto()) as T;
return this._userDataWriter.convertValue(
this._document.toProto(),
options.serverTimestamps
) as T;
}
}

Expand Down Expand Up @@ -269,13 +261,10 @@ export class DocumentSnapshot<T = DocumentData> extends LiteDocumentSnapshot<
.data()
.field(fieldPathFromArgument('DocumentSnapshot.get', fieldPath));
if (value !== null) {
const userDataWriter = new UserDataWriter(
this._firestoreImpl._databaseId,
options.serverTimestamps || DEFAULT_SERVER_TIMESTAMP_BEHAVIOR,
key => new DocumentReference(this._firestore, this._converter, key),
bytes => new Bytes(bytes)
return this._userDataWriter.convertValue(
value,
options.serverTimestamps
);
return userDataWriter.convertValue(value);
}
}
return undefined;
Expand Down Expand Up @@ -339,6 +328,7 @@ export class QuerySnapshot<T = DocumentData> {

constructor(
readonly _firestore: FirebaseFirestore,
readonly _userDataWriter: UserDataWriter,
query: Query<T>,
readonly _snapshot: ViewSnapshot
) {
Expand Down Expand Up @@ -431,6 +421,7 @@ export class QuerySnapshot<T = DocumentData> {
): QueryDocumentSnapshot<T> {
return new QueryDocumentSnapshot<T>(
this._firestore,
this._userDataWriter,
doc.key,
doc,
new SnapshotMetadata(hasPendingWrites, fromCache),
Expand Down
8 changes: 8 additions & 0 deletions packages/firestore/exp/src/api/transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import { Transaction as InternalTransaction } from '../../../src/core/transactio
import { validateReference } from '../../../lite/src/api/write_batch';
import { getDatastore } from '../../../lite/src/api/components';
import { DocumentReference } from '../../../lite/src/api/reference';
import { UserDataWriter } from '../../../src/api/user_data_writer';
import { Bytes } from '../../../lite/src/api/bytes';

/**
* A reference to a transaction.
Expand Down Expand Up @@ -56,12 +58,18 @@ export class Transaction extends LiteTransaction {
*/
get<T>(documentRef: DocumentReference<T>): Promise<DocumentSnapshot<T>> {
const ref = validateReference<T>(documentRef, this._firestore);
const userDataWriter = new UserDataWriter(
this._firestore._databaseId,
key => new DocumentReference(this._firestore, ref._converter, key),
bytes => new Bytes(bytes)
);
return super
.get(documentRef)
.then(
liteDocumentSnapshot =>
new DocumentSnapshot(
this._firestore,
userDataWriter,
ref._key,
liteDocumentSnapshot._document,
new SnapshotMetadata(
Expand Down
68 changes: 2 additions & 66 deletions packages/firestore/exp/test/shim.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ import { Compat } from '../../src/compat/compat';
import {
Firestore,
DocumentReference,
DocumentSnapshot,
QueryDocumentSnapshot,
wrapObserver,
extractSnapshotOptions
} from '../../src/api/database';
Expand Down Expand Up @@ -184,48 +186,6 @@ export class WriteBatch
}
}

export class DocumentSnapshot<T = legacy.DocumentData>
extends Compat<exp.DocumentSnapshot<T>>
implements legacy.DocumentSnapshot<T> {
constructor(
private readonly _firestore: Firestore,
delegate: exp.DocumentSnapshot<T>
) {
super(delegate);
}

readonly ref = new DocumentReference<T>(this._firestore, this._delegate.ref);
readonly id = this._delegate.id;
readonly metadata = this._delegate.metadata;

get exists(): boolean {
return this._delegate.exists();
}

data(options?: legacy.SnapshotOptions): T | undefined {
return wrap(this._firestore, this._delegate.data(options));
}

get(fieldPath: string | FieldPath, options?: legacy.SnapshotOptions): any {
return wrap(
this._firestore,
this._delegate.get(unwrap(fieldPath), options)
);
}

isEqual(other: DocumentSnapshot<T>): boolean {
return snapshotEqual(this._delegate, other._delegate);
}
}

export class QueryDocumentSnapshot<T = legacy.DocumentData>
extends DocumentSnapshot<T>
implements legacy.QueryDocumentSnapshot<T> {
data(options?: legacy.SnapshotOptions): T {
return this._delegate.data(options)!;
}
}

export class Query<T = legacy.DocumentData>
extends Compat<exp.Query<T>>
implements legacy.Query<T> {
Expand Down Expand Up @@ -496,30 +456,6 @@ export class Blob extends Compat<BytesExp> implements legacy.Blob {
}
}

/**
* Takes document data that uses the firestore-exp API types and replaces them
* with the API types defined in this shim.
*/
function wrap(firestore: Firestore, value: any): any {
if (Array.isArray(value)) {
return value.map(v => wrap(firestore, v));
} else if (value instanceof FieldPathExp) {
return new FieldPath(...value._internalPath.toArray());
} else if (value instanceof BytesExp) {
return new Blob(value);
} else if (isPlainObject(value)) {
const obj: any = {};
for (const key in value) {
if (value.hasOwnProperty(key)) {
obj[key] = wrap(firestore, value[key]);
}
}
return obj;
} else {
return value;
}
}

/**
* Takes user data that uses API types from this shim and replaces them
* with the the firestore-exp API types.
Expand Down
Loading