Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

Commit 5f93109

Browse files
schmidt-sebastianschmidt-sebastian
committed
Addressing Feedback
1 parent e55174c commit 5f93109

12 files changed

Lines changed: 102 additions & 95 deletions

File tree

‎dev/conformance/runner.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import * as proto from '../protos/firestore_proto_api';
2626
import {DocumentChange, DocumentSnapshot, FieldPath, FieldValue, Firestore, Query, QueryDocumentSnapshot, QuerySnapshot, Timestamp} from '../src';
2727
import {fieldsFromJson} from '../src/convert';
2828
import {DocumentChangeType} from '../src/document-change';
29-
import {ResourcePath} from '../src/path';
29+
import {QualifiedResourcePath} from '../src/path';
3030
import {DocumentData} from '../src/types';
3131
import {isObject} from '../src/util';
3232
import {ApiOverride, createInstance as createInstanceHelper} from '../test/util/helpers';
@@ -68,13 +68,13 @@ const COMMIT_REQUEST_TYPE =
6868
let firestore: Firestore;
6969

7070
const docRef = (path: string) => {
71-
const relativePath = ResourcePath.fromSlashSeparatedString(path).relativeName;
72-
return firestore.doc(relativePath);
71+
const resourcePath = QualifiedResourcePath.fromSlashSeparatedString(path);
72+
return firestore.doc(resourcePath.relativeName);
7373
};
7474

7575
const collRef = (path: string) => {
76-
const relativePath = ResourcePath.fromSlashSeparatedString(path).relativeName;
77-
return firestore.collection(relativePath);
76+
const resourcePath = QualifiedResourcePath.fromSlashSeparatedString(path);
77+
return firestore.collection(resourcePath.relativeName);
7878
};
7979

8080
const watchQuery = () => {

‎dev/src/index.ts‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ import {google} from '../protos/firestore_proto_api';
2222
import {fieldsFromJson, timestampFromJson} from './convert';
2323
import {DocumentSnapshot, DocumentSnapshotBuilder, QueryDocumentSnapshot} from './document';
2424
import {logger, setLibVersion} from './logger';
25-
import {DATABASE_ID, FieldPath, RelativePath, validateResourcePath} from './path';
26-
import {ResourcePath} from './path';
25+
import {DEFAULT_DATABASE_ID, FieldPath, QualifiedResourcePath, ResourcePath, validateResourcePath} from './path';
2726
import {ClientPool} from './pool';
2827
import {CollectionReference} from './reference';
2928
import {DocumentReference} from './reference';
@@ -369,8 +368,11 @@ export class Firestore {
369368
* @private
370369
*/
371370
get projectId(): string {
372-
this.ensureClientInitialized();
373-
return this._projectId!;
371+
if (this._projectId === undefined) {
372+
throw new Error(
373+
'INTERNAL ERROR: Client is not yet ready to issue requests.');
374+
}
375+
return this._projectId;
374376
}
375377

376378
/**
@@ -380,8 +382,7 @@ export class Firestore {
380382
* @private
381383
*/
382384
get formattedName(): string {
383-
this.ensureClientInitialized();
384-
return `projects/${this.projectId}/databases/${DATABASE_ID}`;
385+
return `projects/${this.projectId}/databases/${DEFAULT_DATABASE_ID}`;
385386
}
386387

387388
/**
@@ -399,7 +400,7 @@ export class Firestore {
399400
doc(documentPath: string): DocumentReference {
400401
validateResourcePath('documentPath', documentPath);
401402

402-
const path = RelativePath.EMPTY.append(documentPath);
403+
const path = ResourcePath.EMPTY.append(documentPath);
403404
if (!path.isDocument) {
404405
throw new Error(`Argument "documentPath" must point to a document, but was "${
405406
documentPath}". Your path does not contain an even number of components.`);
@@ -427,7 +428,7 @@ export class Firestore {
427428
collection(collectionPath: string): CollectionReference {
428429
validateResourcePath('collectionPath', collectionPath);
429430

430-
const path = RelativePath.EMPTY.append(collectionPath);
431+
const path = ResourcePath.EMPTY.append(collectionPath);
431432
if (!path.isCollection) {
432433
throw new Error(`Argument "collectionPath" must point to a collection, but was "${
433434
collectionPath}". Your path does not contain an odd number of components.`);
@@ -518,11 +519,12 @@ export class Firestore {
518519

519520
if (typeof documentOrName === 'string') {
520521
document.ref = new DocumentReference(
521-
this, ResourcePath.fromSlashSeparatedString(documentOrName));
522+
this, QualifiedResourcePath.fromSlashSeparatedString(documentOrName));
522523
} else {
523524
document.ref = new DocumentReference(
524525
this,
525-
ResourcePath.fromSlashSeparatedString(documentOrName.name as string));
526+
QualifiedResourcePath.fromSlashSeparatedString(
527+
documentOrName.name as string));
526528
document.fieldsProto = documentOrName.fields ?
527529
convertFields(documentOrName.fields as ApiMapValue) :
528530
{};
@@ -671,7 +673,7 @@ export class Firestore {
671673
* });
672674
*/
673675
listCollections() {
674-
const rootDocument = new DocumentReference(this, RelativePath.EMPTY);
676+
const rootDocument = new DocumentReference(this, ResourcePath.EMPTY);
675677
return rootDocument.listCollections();
676678
}
677679

@@ -881,18 +883,6 @@ export class Firestore {
881883
}
882884
}
883885

884-
/**
885-
* Ensures that the client has been fully initialized;
886-
* @private
887-
*/
888-
private ensureClientInitialized(): void {
889-
if (this._projectId === undefined) {
890-
// Note that we do not mention `initializeIfNeeded()` here to not leak the
891-
// private API.
892-
throw new Error('INTERNAL ERROR: Client is yet ready to issue requests.');
893-
}
894-
}
895-
896886
/**
897887
* Returns GAX call options that set the cloud resource header.
898888
* @private

‎dev/src/order.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
import {google} from '../protos/firestore_proto_api';
1818
import {detectValueType} from './convert';
19-
import {ResourcePath} from './path';
19+
import {QualifiedResourcePath} from './path';
2020
import {ApiMapValue} from './types';
2121

2222
import api = google.firestore.v1;
@@ -151,9 +151,10 @@ function compareBlobs(left: Uint8Array, right: Uint8Array): number {
151151
* @private
152152
*/
153153
function compareReferenceProtos(left: api.IValue, right: api.IValue): number {
154-
const leftPath = ResourcePath.fromSlashSeparatedString(left.referenceValue!);
154+
const leftPath =
155+
QualifiedResourcePath.fromSlashSeparatedString(left.referenceValue!);
155156
const rightPath =
156-
ResourcePath.fromSlashSeparatedString(right.referenceValue!);
157+
QualifiedResourcePath.fromSlashSeparatedString(right.referenceValue!);
157158
return leftPath.compareTo(rightPath);
158159
}
159160

‎dev/src/path.ts‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ import {customObjectMessage, invalidArgumentMessage, validateMinNumberOfArgument
2121

2222
import api = google.firestore.v1;
2323

24+
/*!
25+
* The default database ID for this Firestore client. We do not yet expose the
26+
* ability to use different databases.
27+
*/
28+
export const DEFAULT_DATABASE_ID = '(default)';
29+
2430
/*!
2531
* A regular expression to verify an absolute Resource Path in Firestore. It
2632
* extracts the project ID, the database name and the relative resource path
@@ -49,12 +55,6 @@ const UNESCAPED_FIELD_NAME_RE = /^[_a-zA-Z][_a-zA-Z0-9]*$/;
4955
*/
5056
const FIELD_PATH_RE = /^[^*~/[\]]+$/;
5157

52-
/*!
53-
* The default database ID for this Firestore client. We do not yet expose the
54-
* ability to use different databases.
55-
*/
56-
export const DATABASE_ID = '(default)';
57-
5858
/**
5959
* An abstract class representing a Firestore path.
6060
*
@@ -190,12 +190,12 @@ abstract class Path<T> {
190190
*
191191
* @private
192192
*/
193-
export class RelativePath extends Path<RelativePath> {
193+
export class ResourcePath extends Path<ResourcePath> {
194194
/** A default instance pointing to the root collection. */
195-
static EMPTY = new RelativePath();
195+
static EMPTY = new ResourcePath();
196196

197197
/**
198-
* Constructs a RelativePath.
198+
* Constructs a ResourcePath.
199199
*
200200
* @param segments Sequence of names of the parts of the path.
201201
*/
@@ -238,11 +238,11 @@ export class RelativePath extends Path<RelativePath> {
238238
/**
239239
* Constructs a new instance of RelativePath.
240240
*
241-
* @param segments Sequence of names of the parts of the path.
241+
* @param segments Sequence of ResourcePath of the parts of the path.
242242
* @returns The newly created RelativePath.
243243
*/
244-
construct(segments: string[]): RelativePath {
245-
return new RelativePath(...segments);
244+
construct(segments: string[]): ResourcePath {
245+
return new ResourcePath(...segments);
246246
}
247247

248248
/**
@@ -263,18 +263,19 @@ export class RelativePath extends Path<RelativePath> {
263263
* @param databaseId The database ID of the new path.
264264
* @return A fully-qualified resource path pointing to the same element.
265265
*/
266-
toResourcePath(projectId: string): ResourcePath {
267-
return new ResourcePath(projectId, DATABASE_ID, ...this.segments);
266+
toQualifiedResourcePath(projectId: string): QualifiedResourcePath {
267+
return new QualifiedResourcePath(
268+
projectId, DEFAULT_DATABASE_ID, ...this.segments);
268269
}
269270
}
270271

271272
/**
272-
* A slash-separated path for navigating resources (documents and collections)
273-
* within any Firestore project.
273+
* A slash-separated path that includes a project and database ID for referring
274+
* to resources in any Firestore project.
274275
*
275276
* @private
276277
*/
277-
export class ResourcePath extends RelativePath {
278+
export class QualifiedResourcePath extends ResourcePath {
278279
/**
279280
* The project ID of this path.
280281
*/
@@ -312,14 +313,14 @@ export class ResourcePath extends RelativePath {
312313
* @param absolutePath A string representation of a Resource Path.
313314
* @returns The new ResourcePath.
314315
*/
315-
static fromSlashSeparatedString(absolutePath: string): ResourcePath {
316+
static fromSlashSeparatedString(absolutePath: string): QualifiedResourcePath {
316317
const elements = RESOURCE_PATH_RE.exec(absolutePath);
317318

318319
if (elements) {
319320
const project = elements[1];
320321
const database = elements[2];
321322
const path = elements[3];
322-
return new ResourcePath(project, database).append(path);
323+
return new QualifiedResourcePath(project, database).append(path);
323324
}
324325

325326
throw new Error(`Resource name '${absolutePath}' is not valid.`);
@@ -331,8 +332,8 @@ export class ResourcePath extends RelativePath {
331332
* @param relativePath Relative path to append to the current path.
332333
* @returns The new path.
333334
*/
334-
append(relativePath: RelativePath|string): ResourcePath {
335-
return super.append(relativePath) as ResourcePath;
335+
append(relativePath: ResourcePath|string): QualifiedResourcePath {
336+
return super.append(relativePath) as QualifiedResourcePath;
336337
}
337338

338339

@@ -341,8 +342,8 @@ export class ResourcePath extends RelativePath {
341342
*
342343
* @returns The new path.
343344
*/
344-
parent(): ResourcePath|null {
345-
return super.parent() as ResourcePath | null;
345+
parent(): QualifiedResourcePath|null {
346+
return super.parent() as QualifiedResourcePath | null;
346347
}
347348

348349
/**
@@ -364,17 +365,18 @@ export class ResourcePath extends RelativePath {
364365
* methods.
365366
*
366367
* @param segments Sequence of names of the parts of the path.
367-
* @returns {ResourcePath} The newly created ResourcePath.
368+
* @returns The newly created QualifiedResourcePath.
368369
*/
369-
construct(segments: string[]): ResourcePath {
370-
return new ResourcePath(this.projectId, this.databaseId, ...segments);
370+
construct(segments: string[]): QualifiedResourcePath {
371+
return new QualifiedResourcePath(
372+
this.projectId, this.databaseId, ...segments);
371373
}
372374

373375
/**
374-
* Convenience method to match the RelativePath API. This method always
376+
* Convenience method to match the ResourcePath API. This method always
375377
* returns the current instance. The arguments is ignored.
376378
*/
377-
toResourcePath(projectId: string): ResourcePath {
379+
toQualifiedResourcePath(projectId: string): QualifiedResourcePath {
378380
return this;
379381
}
380382

@@ -384,8 +386,8 @@ export class ResourcePath extends RelativePath {
384386
* @param other The path to compare to.
385387
* @returns -1 if current < other, 1 if current > other, 0 if equal
386388
*/
387-
compareTo(other: RelativePath): number {
388-
if (other instanceof ResourcePath) {
389+
compareTo(other: ResourcePath): number {
390+
if (other instanceof QualifiedResourcePath) {
389391
if (this.projectId < other.projectId) {
390392
return -1;
391393
}
@@ -557,8 +559,8 @@ export class FieldPath extends Path<FieldPath> {
557559
*
558560
* @private
559561
* @override
560-
* @param {Array.<string>} segments Sequence of field names.
561-
* @returns {ResourcePath} The newly created FieldPath.
562+
* @param segments Sequence of field names.
563+
* @returns The newly created FieldPath.
562564
*/
563565
construct(segments: string[]) {
564566
return new FieldPath(...segments);

‎dev/src/reference.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {DocumentChange} from './document-change';
2727
import {Firestore} from './index';
2828
import {logger} from './logger';
2929
import {compare} from './order';
30-
import {FieldPath, RelativePath, ResourcePath, validateFieldPath, validateResourcePath} from './path';
30+
import {FieldPath, QualifiedResourcePath, ResourcePath, validateFieldPath, validateResourcePath} from './path';
3131
import {Serializable, Serializer, validateUserInput} from './serializer';
3232
import {Timestamp} from './timestamp';
3333
import {DocumentData, OrderByDirection, Precondition, SetOptions, UpdateData, WhereFilterOp} from './types';
@@ -105,7 +105,7 @@ export class DocumentReference implements Serializable {
105105
* @param _path The Path of this reference.
106106
*/
107107
constructor(
108-
private readonly _firestore: Firestore, readonly _path: RelativePath) {}
108+
private readonly _firestore: Firestore, readonly _path: ResourcePath) {}
109109

110110

111111
/**
@@ -116,7 +116,7 @@ export class DocumentReference implements Serializable {
116116
*/
117117
get formattedName(): string {
118118
const projectId = this.firestore.projectId;
119-
return this._path.toResourcePath(projectId).formattedName;
119+
return this._path.toQualifiedResourcePath(projectId).formattedName;
120120
}
121121

122122
/**
@@ -900,7 +900,7 @@ export class Query {
900900
* @param _queryOptions Additional query options.
901901
*/
902902
constructor(
903-
private readonly _firestore: Firestore, readonly _path: RelativePath,
903+
private readonly _firestore: Firestore, readonly _path: ResourcePath,
904904
private readonly _fieldFilters: FieldFilter[] = [],
905905
private readonly _fieldOrders: FieldOrder[] = [],
906906
private readonly _queryOptions: QueryOptions = {}) {
@@ -1579,7 +1579,7 @@ export class Query {
15791579
*/
15801580
toProto(transactionId?: Uint8Array): api.IRunQueryRequest {
15811581
const projectId = this.firestore.projectId;
1582-
const parentPath = this._path.parent()!.toResourcePath(projectId);
1582+
const parentPath = this._path.parent()!.toQualifiedResourcePath(projectId);
15831583
const reqOpts: api.IRunQueryRequest = {
15841584
parent: parentPath.formattedName,
15851585
structuredQuery: {
@@ -1769,7 +1769,7 @@ export class CollectionReference extends Query {
17691769
* @param firestore The Firestore Database client.
17701770
* @param path The Path of this collection.
17711771
*/
1772-
constructor(firestore: Firestore, path: RelativePath) {
1772+
constructor(firestore: Firestore, path: ResourcePath) {
17731773
super(firestore, path);
17741774
}
17751775

@@ -1850,7 +1850,8 @@ export class CollectionReference extends Query {
18501850
*/
18511851
listDocuments(): Promise<DocumentReference[]> {
18521852
return this.firestore.initializeIfNeeded().then(() => {
1853-
const resourcePath = this._path.toResourcePath(this.firestore.projectId);
1853+
const resourcePath =
1854+
this._path.toQualifiedResourcePath(this.firestore.projectId);
18541855
const parentPath = resourcePath.parent()!;
18551856

18561857
const request: api.IListDocumentsRequest = {
@@ -1867,7 +1868,8 @@ export class CollectionReference extends Query {
18671868
// Note that the backend already orders these documents by name,
18681869
// so we do not need to manually sort them.
18691870
return documents.map(doc => {
1870-
const path = ResourcePath.fromSlashSeparatedString(doc.name!);
1871+
const path =
1872+
QualifiedResourcePath.fromSlashSeparatedString(doc.name!);
18711873
return this.doc(path.id!);
18721874
});
18731875
});

0 commit comments

Comments
 (0)