Skip to content

Make View processing logic optional #3561

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 24 commits into from
Aug 27, 2020
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
534 changes: 252 additions & 282 deletions packages/firestore/exp/dependencies.json

Large diffs are not rendered by default.

21 changes: 17 additions & 4 deletions packages/firestore/exp/src/api/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ import {
import { handleUserChange, LocalStore } from '../../../src/local/local_store';
import { Deferred } from '../../../src/util/promise';
import { logDebug } from '../../../src/util/log';
import { SyncEngine } from '../../../src/core/sync_engine';
import {
SyncEngine,
syncEngineListen,
syncEngineUnlisten
} from '../../../src/core/sync_engine';
import { RemoteStore } from '../../../src/remote/remote_store';
import { Persistence } from '../../../src/local/persistence';
import { EventManager } from '../../../src/core/event_manager';
Expand Down Expand Up @@ -140,6 +144,9 @@ function verifyNotTerminated(firestore: Firestore): void {
}
}

// Note: These functions cannot be `async` since we want to throw an exception
// when Firestore is terminated (via `getOnlineComponentProvider()`).

export function getSyncEngine(firestore: Firestore): Promise<SyncEngine> {
return getOnlineComponentProvider(firestore).then(
components => components.syncEngine
Expand All @@ -153,9 +160,15 @@ export function getRemoteStore(firestore: Firestore): Promise<RemoteStore> {
}

export function getEventManager(firestore: Firestore): Promise<EventManager> {
return getOnlineComponentProvider(firestore).then(
components => components.eventManager
);
return getOnlineComponentProvider(firestore).then(components => {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to address this here, necessarily, but why not write these with async/await?

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 don't remember why I did it this way, but "await" here is much better, especially now that the function has some nested logic. Change is pretty easy and only in 5 places so I did it just now.

const eventManager = components.eventManager;
eventManager.onListen = syncEngineListen.bind(null, components.syncEngine);
eventManager.onUnlisten = syncEngineUnlisten.bind(
null,
components.syncEngine
);
return eventManager;
});
}

export function getPersistence(firestore: Firestore): Promise<Persistence> {
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/exp/test/shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,8 @@ export class DocumentChange<T = legacy.DocumentData>
readonly newIndex = this._delegate.oldIndex;
}

export class CollectionReference<T = legacy.DocumentData> extends Query<T>
export class CollectionReference<T = legacy.DocumentData>
extends Query<T>
implements legacy.CollectionReference<T> {
constructor(
firestore: FirebaseFirestore,
Expand Down
5 changes: 4 additions & 1 deletion packages/firestore/src/api/blob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ function assertBase64Available(): void {

/**
* Immutable class holding a blob (binary data).
* This class is directly exposed in the public API.
*
* This class is directly exposed in the public API. It extends the Bytes class
* of the firestore-exp API to support `instanceof Bytes` checks during user
* data conversion.
*
* Note that while you can't hide the constructor in JavaScript code, we are
* using the hack above to make sure no-one outside this module can call it.
Expand Down
9 changes: 6 additions & 3 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,8 @@ export class WriteBatch implements PublicWriteBatch {
/**
* A reference to a particular document in a collection in the database.
*/
export class DocumentReference<T = DocumentData> extends DocumentKeyReference<T>
export class DocumentReference<T = DocumentData>
extends DocumentKeyReference<T>
implements PublicDocumentReference<T> {
private _firestoreClient: FirestoreClient;

Expand Down Expand Up @@ -1440,7 +1441,8 @@ export class DocumentSnapshot<T = DocumentData>
}
}

export class QueryDocumentSnapshot<T = DocumentData> extends DocumentSnapshot<T>
export class QueryDocumentSnapshot<T = DocumentData>
extends DocumentSnapshot<T>
implements PublicQueryDocumentSnapshot<T> {
data(options?: SnapshotOptions): T {
const data = super.data(options);
Expand Down Expand Up @@ -2306,7 +2308,8 @@ export class QuerySnapshot<T = DocumentData> implements PublicQuerySnapshot<T> {
}
}

export class CollectionReference<T = DocumentData> extends Query<T>
export class CollectionReference<T = DocumentData>
extends Query<T>
implements PublicCollectionReference<T> {
constructor(
readonly _path: ResourcePath,
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/src/api/field_value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ export class NumericIncrementFieldValueImpl extends SerializableFieldValue {
}

/** The public FieldValue class of the lite API. */
export abstract class FieldValue extends SerializableFieldValue
export abstract class FieldValue
extends SerializableFieldValue
implements PublicFieldValue {
protected constructor() {
super();
Expand Down
18 changes: 13 additions & 5 deletions packages/firestore/src/core/component_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ import {
import {
applyActiveTargetsChange,
applyBatchState,
applyOnlineStateChange,
applyPrimaryState,
applyTargetState,
getActiveClients,
handleCredentialChange,
newSyncEngine,
SyncEngine
} from './sync_engine';
Expand Down Expand Up @@ -332,20 +334,26 @@ export class OnlineComponentProvider {
this.syncEngine = this.createSyncEngine(cfg);
this.eventManager = this.createEventManager(cfg);

this.syncEngine.subscribe(this.eventManager);

this.sharedClientState.onlineStateHandler = onlineState =>
this.syncEngine.applyOnlineStateChange(
applyOnlineStateChange(
this.syncEngine,
onlineState,
OnlineStateSource.SharedClientState
);

this.remoteStore.syncEngine = this.syncEngine;
this.remoteStore.remoteSyncer.handleCredentialChange = handleCredentialChange.bind(
null,
this.syncEngine
);

await this.remoteStore.start();
await this.remoteStore.applyPrimaryState(this.syncEngine.isPrimaryClient);
}

createEventManager(cfg: ComponentConfiguration): EventManager {
return new EventManager(this.syncEngine);
return new EventManager();
}

createDatastore(cfg: ComponentConfiguration): Datastore {
Expand All @@ -360,7 +368,8 @@ export class OnlineComponentProvider {
this.datastore,
cfg.asyncQueue,
onlineState =>
this.syncEngine.applyOnlineStateChange(
applyOnlineStateChange(
this.syncEngine,
onlineState,
OnlineStateSource.RemoteStore
),
Expand All @@ -372,7 +381,6 @@ export class OnlineComponentProvider {
return newSyncEngine(
this.localStore,
this.remoteStore,
this.datastore,
this.sharedClientState,
cfg.initialUser,
cfg.maxConcurrentLimboResolutions,
Expand Down
19 changes: 13 additions & 6 deletions packages/firestore/src/core/event_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { debugAssert } from '../util/assert';
import { EventHandler } from '../util/misc';
import { ObjectMap } from '../util/obj_map';
import { canonifyQuery, Query, queryEquals, stringifyQuery } from './query';
import { SyncEngine, SyncEngineListener } from './sync_engine';
import { SyncEngineListener } from './sync_engine';
import { OnlineState } from './types';
import { ChangeType, DocumentViewChange, ViewSnapshot } from './view_snapshot';
import { wrapInUserErrorIfRecoverable } from '../util/async_queue';
Expand All @@ -45,6 +45,10 @@ export interface Observer<T> {
* EventManager is responsible for mapping queries to query event emitters.
* It handles "fan-out". -- Identical queries will re-use the same watch on the
* backend.
*
* PORTING NOTE: On Web, EventManager `onListen` and `onUnlisten` need to be
* assigned to SyncEngine's `listen()` and `unlisten()` API before usage. This
* allows users to tree-shake the Watch logic.
*/
export class EventManager implements SyncEngineListener {
private queries = new ObjectMap<Query, QueryListenersInfo>(
Expand All @@ -56,11 +60,13 @@ export class EventManager implements SyncEngineListener {

private snapshotsInSyncListeners: Set<Observer<void>> = new Set();

constructor(private syncEngine: SyncEngine) {
this.syncEngine.subscribe(this);
}
/** Callback invoked when a Query is first listen to. */
onListen?: (query: Query) => Promise<ViewSnapshot>;
/** Callback invoked once all listeners to a Query are removed. */
onUnlisten?: (query: Query) => Promise<void>;

async listen(listener: QueryListener): Promise<void> {
debugAssert(!!this.onListen, 'onListen not set');
const query = listener.query;
let firstListen = false;

Expand All @@ -72,7 +78,7 @@ export class EventManager implements SyncEngineListener {

if (firstListen) {
try {
queryInfo.viewSnap = await this.syncEngine.listen(query);
queryInfo.viewSnap = await this.onListen(query);
} catch (e) {
const firestoreError = wrapInUserErrorIfRecoverable(
e,
Expand Down Expand Up @@ -102,6 +108,7 @@ export class EventManager implements SyncEngineListener {
}

async unlisten(listener: QueryListener): Promise<void> {
debugAssert(!!this.onUnlisten, 'onUnlisten not set');
const query = listener.query;
let lastListen = false;

Expand All @@ -116,7 +123,7 @@ export class EventManager implements SyncEngineListener {

if (lastListen) {
this.queries.delete(query);
return this.syncEngine.unlisten(query);
return this.onUnlisten(query);
}
}

Expand Down
29 changes: 20 additions & 9 deletions packages/firestore/src/core/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ import {
Observer,
QueryListener
} from './event_manager';
import { SyncEngine } from './sync_engine';
import {
registerPendingWritesCallback,
SyncEngine,
syncEngineListen,
syncEngineUnlisten,
syncEngineWrite
} from './sync_engine';
import { View } from './view';
import { SharedClientState } from '../local/shared_client_state';
import { AutoId } from '../util/misc';
Expand Down Expand Up @@ -277,6 +283,9 @@ export class FirestoreClient {
this.syncEngine = onlineComponentProvider.syncEngine;
this.eventMgr = onlineComponentProvider.eventManager;

this.eventMgr.onListen = syncEngineListen.bind(null, this.syncEngine);
this.eventMgr.onUnlisten = syncEngineUnlisten.bind(null, this.syncEngine);

// When a user calls clearPersistence() in one client, all other clients
// need to be terminated to allow the delete to succeed.
this.persistence.setDatabaseDeletedListener(async () => {
Expand Down Expand Up @@ -405,9 +414,9 @@ export class FirestoreClient {
this.verifyNotTerminated();

const deferred = new Deferred<void>();
this.asyncQueue.enqueueAndForget(() => {
return this.syncEngine.registerPendingWritesCallback(deferred);
});
this.asyncQueue.enqueueAndForget(() =>
registerPendingWritesCallback(this.syncEngine, deferred)
);
return deferred.promise;
}

Expand Down Expand Up @@ -480,7 +489,7 @@ export class FirestoreClient {
this.verifyNotTerminated();
const deferred = new Deferred<void>();
this.asyncQueue.enqueueAndForget(() =>
this.syncEngine.write(mutations, deferred)
syncEngineWrite(this.syncEngine, mutations, deferred)
);
return deferred.promise;
}
Expand Down Expand Up @@ -549,7 +558,9 @@ export function enqueueWrite(
mutations: Mutation[]
): Promise<void> {
const deferred = new Deferred<void>();
asyncQueue.enqueueAndForget(() => syncEngine.write(mutations, deferred));
asyncQueue.enqueueAndForget(() =>
syncEngineWrite(syncEngine, mutations, deferred)
);
return deferred.promise;
}

Expand All @@ -570,9 +581,9 @@ export function enqueueWaitForPendingWrites(
syncEngine: SyncEngine
): Promise<void> {
const deferred = new Deferred<void>();
asyncQueue.enqueueAndForget(() => {
return syncEngine.registerPendingWritesCallback(deferred);
});
asyncQueue.enqueueAndForget(() =>
registerPendingWritesCallback(syncEngine, deferred)
);
return deferred.promise;
}

Expand Down
Loading