Skip to content

Commit d3f2507

Browse files
schmidt-sebastianschmidt-sebastian
authored
Make newConnection() synchronous (#3569)
1 parent 90203eb commit d3f2507

File tree

16 files changed

+97
-115
lines changed

16 files changed

+97
-115
lines changed

‎packages/firestore/exp/src/api/transaction.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,14 @@ export function runTransaction<T>(
6868
updateFunction: (transaction: firestore.Transaction) => Promise<T>
6969
): Promise<T> {
7070
const firestoreClient = cast(firestore, Firestore);
71-
return getDatastore(firestoreClient).then(async datastore => {
72-
const deferred = new Deferred<T>();
73-
new TransactionRunner<T>(
74-
new AsyncQueue(),
75-
datastore,
76-
internalTransaction =>
77-
updateFunction(new Transaction(firestoreClient, internalTransaction)),
78-
deferred
79-
).run();
80-
return deferred.promise;
81-
});
71+
const datastore = getDatastore(firestoreClient);
72+
const deferred = new Deferred<T>();
73+
new TransactionRunner<T>(
74+
new AsyncQueue(),
75+
datastore,
76+
internalTransaction =>
77+
updateFunction(new Transaction(firestoreClient, internalTransaction)),
78+
deferred
79+
).run();
80+
return deferred.promise;
8281
}

‎packages/firestore/lite/src/api/components.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,14 @@ export const DEFAULT_SSL = true;
3737
* An instance map that ensures only one Datastore exists per Firestore
3838
* instance.
3939
*/
40-
const datastoreInstances = new Map<Firestore, Promise<Datastore>>();
40+
const datastoreInstances = new Map<Firestore, Datastore>();
4141

4242
/**
4343
* Returns an initialized and started Datastore for the given Firestore
4444
* instance. Callers must invoke removeDatastore() when the Firestore
4545
* instance is terminated.
4646
*/
47-
export function getDatastore(firestore: Firestore): Promise<Datastore> {
47+
export function getDatastore(firestore: Firestore): Datastore {
4848
if (firestore._terminated) {
4949
throw new FirestoreError(
5050
Code.FAILED_PRECONDITION,
@@ -61,13 +61,16 @@ export function getDatastore(firestore: Firestore): Promise<Datastore> {
6161
settings.ssl ?? DEFAULT_SSL,
6262
/* forceLongPolling= */ false
6363
);
64-
const datastorePromise = newConnection(databaseInfo).then(connection => {
65-
const serializer = newSerializer(databaseInfo.databaseId);
66-
const datastore = newDatastore(firestore._credentials, serializer);
67-
datastore.start(connection);
68-
return datastore;
69-
});
70-
datastoreInstances.set(firestore, datastorePromise);
64+
65+
const connection = newConnection(databaseInfo);
66+
const serializer = newSerializer(databaseInfo.databaseId);
67+
const datastore = newDatastore(
68+
firestore._credentials,
69+
connection,
70+
serializer
71+
);
72+
73+
datastoreInstances.set(firestore, datastore);
7174
}
7275
return datastoreInstances.get(firestore)!;
7376
}
@@ -76,11 +79,11 @@ export function getDatastore(firestore: Firestore): Promise<Datastore> {
7679
* Removes all components associated with the provided instance. Must be called
7780
* when the Firestore instance is terminated.
7881
*/
79-
export async function removeComponents(firestore: Firestore): Promise<void> {
80-
const datastorePromise = await datastoreInstances.get(firestore);
81-
if (datastorePromise) {
82+
export function removeComponents(firestore: Firestore): void {
83+
const datastore = datastoreInstances.get(firestore);
84+
if (datastore) {
8285
logDebug(LOG_TAG, 'Removing Datastore');
8386
datastoreInstances.delete(firestore);
84-
return (await datastorePromise).termiate();
87+
datastore.terminate();
8588
}
8689
}

‎packages/firestore/lite/src/api/database.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ export class Firestore
110110
* Only ever called once.
111111
*/
112112
protected _terminate(): Promise<void> {
113-
return removeComponents(this);
113+
removeComponents(this);
114+
return Promise.resolve();
114115
}
115116

116117
// TODO(firestoreexp): `deleteApp()` should call the delete method above,

‎packages/firestore/lite/src/api/reference.ts

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -547,8 +547,9 @@ export function getDoc<T>(
547547
reference: firestore.DocumentReference<T>
548548
): Promise<firestore.DocumentSnapshot<T>> {
549549
const ref = cast<DocumentReference<T>>(reference, DocumentReference);
550-
return getDatastore(ref.firestore).then(async datastore => {
551-
const result = await invokeBatchGetDocumentsRpc(datastore, [ref._key]);
550+
551+
const datastore = getDatastore(ref.firestore);
552+
return invokeBatchGetDocumentsRpc(datastore, [ref._key]).then(result => {
552553
hardAssert(result.length === 1, 'Expected a single document result');
553554
const maybeDocument = result[0];
554555
return new DocumentSnapshot<T>(
@@ -563,21 +564,22 @@ export function getDoc<T>(
563564
export function getDocs<T>(
564565
query: firestore.Query<T>
565566
): Promise<firestore.QuerySnapshot<T>> {
566-
const internalQuery = cast<Query<T>>(query, Query);
567-
validateHasExplicitOrderByForLimitToLast(internalQuery._query);
568-
return getDatastore(internalQuery.firestore).then(async datastore => {
569-
const result = await invokeRunQueryRpc(datastore, internalQuery._query);
567+
const queryImpl = cast<Query<T>>(query, Query);
568+
validateHasExplicitOrderByForLimitToLast(queryImpl._query);
569+
570+
const datastore = getDatastore(queryImpl.firestore);
571+
return invokeRunQueryRpc(datastore, queryImpl._query).then(result => {
570572
const docs = result.map(
571573
doc =>
572574
new QueryDocumentSnapshot<T>(
573-
internalQuery.firestore,
575+
queryImpl.firestore,
574576
doc.key,
575577
doc,
576-
internalQuery.converter
578+
queryImpl.converter
577579
)
578580
);
579581

580-
if (internalQuery._query.hasLimitToLast()) {
582+
if (queryImpl._query.hasLimitToLast()) {
581583
// Limit to last queries reverse the orderBy constraint that was
582584
// specified by the user. As such, we need to reverse the order of the
583585
// results to return the documents in the expected order.
@@ -619,11 +621,10 @@ export function setDoc<T>(
619621
options
620622
);
621623

622-
return getDatastore(ref.firestore).then(datastore =>
623-
invokeCommitRpc(
624-
datastore,
625-
parsed.toMutations(ref._key, Precondition.none())
626-
)
624+
const datastore = getDatastore(ref.firestore);
625+
return invokeCommitRpc(
626+
datastore,
627+
parsed.toMutations(ref._key, Precondition.none())
627628
);
628629
}
629630

@@ -668,23 +669,21 @@ export function updateDoc(
668669
);
669670
}
670671

671-
return getDatastore(ref.firestore).then(datastore =>
672-
invokeCommitRpc(
673-
datastore,
674-
parsed.toMutations(ref._key, Precondition.exists(true))
675-
)
672+
const datastore = getDatastore(ref.firestore);
673+
return invokeCommitRpc(
674+
datastore,
675+
parsed.toMutations(ref._key, Precondition.exists(true))
676676
);
677677
}
678678

679679
export function deleteDoc(
680680
reference: firestore.DocumentReference
681681
): Promise<void> {
682682
const ref = cast<DocumentReference<unknown>>(reference, DocumentReference);
683-
return getDatastore(ref.firestore).then(datastore =>
684-
invokeCommitRpc(datastore, [
685-
new DeleteMutation(ref._key, Precondition.none())
686-
])
687-
);
683+
const datastore = getDatastore(ref.firestore);
684+
return invokeCommitRpc(datastore, [
685+
new DeleteMutation(ref._key, Precondition.none())
686+
]);
688687
}
689688

690689
export function addDoc<T>(
@@ -706,14 +705,11 @@ export function addDoc<T>(
706705
{}
707706
);
708707

709-
return getDatastore(collRef.firestore)
710-
.then(datastore =>
711-
invokeCommitRpc(
712-
datastore,
713-
parsed.toMutations(docRef._key, Precondition.exists(false))
714-
)
715-
)
716-
.then(() => docRef);
708+
const datastore = getDatastore(collRef.firestore);
709+
return invokeCommitRpc(
710+
datastore,
711+
parsed.toMutations(docRef._key, Precondition.exists(false))
712+
).then(() => docRef);
717713
}
718714

719715
export function refEqual<T>(

‎packages/firestore/lite/src/api/transaction.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -178,15 +178,14 @@ export function runTransaction<T>(
178178
updateFunction: (transaction: firestore.Transaction) => Promise<T>
179179
): Promise<T> {
180180
const firestoreClient = cast(firestore, Firestore);
181-
return getDatastore(firestoreClient).then(async datastore => {
182-
const deferred = new Deferred<T>();
183-
new TransactionRunner<T>(
184-
new AsyncQueue(),
185-
datastore,
186-
internalTransaction =>
187-
updateFunction(new Transaction(firestoreClient, internalTransaction)),
188-
deferred
189-
).run();
190-
return deferred.promise;
191-
});
181+
const datastore = getDatastore(firestoreClient);
182+
const deferred = new Deferred<T>();
183+
new TransactionRunner<T>(
184+
new AsyncQueue(),
185+
datastore,
186+
internalTransaction =>
187+
updateFunction(new Transaction(firestoreClient, internalTransaction)),
188+
deferred
189+
).run();
190+
return deferred.promise;
192191
}

‎packages/firestore/lite/src/api/write_batch.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,8 @@ export function writeBatch(
181181
firestore: firestore.FirebaseFirestore
182182
): firestore.WriteBatch {
183183
const firestoreImpl = cast(firestore, Firestore);
184+
const datastore = getDatastore(firestoreImpl);
184185
return new WriteBatch(firestoreImpl, writes =>
185-
getDatastore(firestoreImpl).then(datastore =>
186-
invokeCommitRpc(datastore, writes)
187-
)
186+
invokeCommitRpc(datastore, writes)
188187
);
189188
}

‎packages/firestore/src/core/component_provider.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ import { newConnection, newConnectivityMonitor } from '../platform/connection';
6161
import { newSerializer } from '../platform/serializer';
6262
import { getDocument, getWindow } from '../platform/dom';
6363
import { CredentialsProvider } from '../api/credentials';
64-
import { Connection } from '../remote/connection';
6564

6665
const MEMORY_ONLY_PERSISTENCE_ERROR_MESSAGE =
6766
'You are using the memory-only build of Firestore. Persistence support is ' +
@@ -329,9 +328,6 @@ export class OnlineComponentProvider {
329328
this.localStore = offlineComponentProvider.localStore;
330329
this.sharedClientState = offlineComponentProvider.sharedClientState;
331330
this.datastore = this.createDatastore(cfg);
332-
const connection = await this.loadConnection(cfg);
333-
this.datastore.start(connection);
334-
335331
this.remoteStore = this.createRemoteStore(cfg);
336332
this.syncEngine = this.createSyncEngine(cfg);
337333
this.eventManager = this.createEventManager(cfg);
@@ -348,17 +344,14 @@ export class OnlineComponentProvider {
348344
await this.remoteStore.applyPrimaryState(this.syncEngine.isPrimaryClient);
349345
}
350346

351-
protected loadConnection(cfg: ComponentConfiguration): Promise<Connection> {
352-
return newConnection(cfg.databaseInfo);
353-
}
354-
355347
createEventManager(cfg: ComponentConfiguration): EventManager {
356348
return new EventManager(this.syncEngine);
357349
}
358350

359351
createDatastore(cfg: ComponentConfiguration): Datastore {
360352
const serializer = newSerializer(cfg.databaseInfo.databaseId);
361-
return newDatastore(cfg.credentials, serializer);
353+
const connection = newConnection(cfg.databaseInfo);
354+
return newDatastore(cfg.credentials, connection, serializer);
362355
}
363356

364357
createRemoteStore(cfg: ComponentConfiguration): RemoteStore {

‎packages/firestore/src/platform/browser/connection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import { BrowserConnectivityMonitor } from './connectivity_monitor';
2323
import { NoopConnectivityMonitor } from '../../remote/connectivity_monitor_noop';
2424

2525
/** Initializes the WebChannelConnection for the browser. */
26-
export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
27-
return Promise.resolve(new WebChannelConnection(databaseInfo));
26+
export function newConnection(databaseInfo: DatabaseInfo): Connection {
27+
return new WebChannelConnection(databaseInfo);
2828
}
2929

3030
/** Return the Platform-specific connectivity monitor. */

‎packages/firestore/src/platform/browser_lite/connection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,6 @@ import { FetchConnection } from './fetch_connection';
2222
export { newConnectivityMonitor } from '../browser/connection';
2323

2424
/** Initializes the HTTP connection for the REST API. */
25-
export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
26-
return Promise.resolve(new FetchConnection(databaseInfo, fetch.bind(null)));
25+
export function newConnection(databaseInfo: DatabaseInfo): Connection {
26+
return new FetchConnection(databaseInfo, fetch.bind(null));
2727
}

‎packages/firestore/src/platform/connection.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ export function newConnectivityMonitor(): ConnectivityMonitor {
2626
return platform.newConnectivityMonitor();
2727
}
2828

29-
// TODO(firestorexp): This doesn't need to return a Promise
30-
export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
29+
export function newConnection(databaseInfo: DatabaseInfo): Connection {
3130
return platform.newConnection(databaseInfo);
3231
}

‎packages/firestore/src/platform/node/connection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ import { Connection } from '../../remote/connection';
2323
import { NoopConnectivityMonitor } from '../../remote/connectivity_monitor_noop';
2424

2525
/** Loads the GRPC stack */
26-
export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
26+
export function newConnection(databaseInfo: DatabaseInfo): Connection {
2727
const protos = loadProtos();
28-
return Promise.resolve(new GrpcConnection(protos, databaseInfo));
28+
return new GrpcConnection(protos, databaseInfo);
2929
}
3030

3131
/** Return the Platform-specific connectivity monitor. */

‎packages/firestore/src/platform/node_lite/connection.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ import { Connection } from '../../remote/connection';
2424
export { newConnectivityMonitor } from '../browser/connection';
2525

2626
/** Initializes the HTTP connection for the REST API. */
27-
export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
28-
// node-fetch is meant to be API compatible with `fetch`, but its type don't
27+
export function newConnection(databaseInfo: DatabaseInfo): Connection {
28+
// node-fetch is meant to be API compatible with `fetch`, but its type doesn't
2929
// match 100%.
3030
// eslint-disable-next-line @typescript-eslint/no-explicit-any
31-
return Promise.resolve(new FetchConnection(databaseInfo, nodeFetch as any));
31+
return new FetchConnection(databaseInfo, nodeFetch as any);
3232
}

‎packages/firestore/src/remote/datastore.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,19 @@ import { Query, queryToTarget } from '../core/query';
4747
* for the rest of the client SDK architecture to consume.
4848
*/
4949
export abstract class Datastore {
50-
abstract start(connection: Connection): void;
51-
abstract termiate(): Promise<void>;
50+
abstract terminate(): void;
5251
}
5352

5453
/**
5554
* An implementation of Datastore that exposes additional state for internal
5655
* consumption.
5756
*/
5857
class DatastoreImpl extends Datastore {
59-
connection!: Connection;
6058
terminated = false;
6159

6260
constructor(
6361
readonly credentials: CredentialsProvider,
62+
readonly connection: Connection,
6463
readonly serializer: JsonProtoSerializer
6564
) {
6665
super();
@@ -76,11 +75,6 @@ class DatastoreImpl extends Datastore {
7675
}
7776
}
7877

79-
start(connection: Connection): void {
80-
debugAssert(!this.connection, 'Datastore.start() already called');
81-
this.connection = connection;
82-
}
83-
8478
/** Gets an auth token and invokes the provided RPC. */
8579
invokeRPC<Req, Resp>(
8680
rpcName: string,
@@ -131,7 +125,7 @@ class DatastoreImpl extends Datastore {
131125
});
132126
}
133127

134-
async termiate(): Promise<void> {
128+
terminate(): void {
135129
this.terminated = false;
136130
}
137131
}
@@ -140,9 +134,10 @@ class DatastoreImpl extends Datastore {
140134
// firestore-exp client.
141135
export function newDatastore(
142136
credentials: CredentialsProvider,
137+
connection: Connection,
143138
serializer: JsonProtoSerializer
144139
): Datastore {
145-
return new DatastoreImpl(credentials, serializer);
140+
return new DatastoreImpl(credentials, connection, serializer);
146141
}
147142

148143
export async function invokeCommitRpc(

0 commit comments

Comments
 (0)