Skip to content

Commit 7fda4c4

Browse files
wu-huiwu-hui
committed
Address comments
1 parent 9d0d0a5 commit 7fda4c4

File tree

4 files changed

+69
-69
lines changed

4 files changed

+69
-69
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -535,9 +535,9 @@ export class FirestoreClient {
535535
);
536536
this.asyncQueue.enqueueAndForget(async () => {
537537
loadBundle(this.syncEngine, reader, resultTask);
538-
return resultTask.catch(e => {
539-
logWarn(LOG_TAG, `Loading bundle failed with ${e}`);
540-
});
538+
});
539+
resultTask.catch(e => {
540+
logWarn(LOG_TAG, `Loading bundle failed with ${e}`);
541541
});
542542
}
543543

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,8 @@ export function fromName(
341341
const resource = fromResourceName(name);
342342

343343
if (resource.get(1) !== serializer.databaseId.projectId) {
344-
throw new Error(
344+
throw new FirestoreError(
345+
Code.INVALID_ARGUMENT,
345346
'Tried to deserialize key from different project: ' +
346347
resource.get(1) +
347348
' vs ' +
@@ -350,7 +351,8 @@ export function fromName(
350351
}
351352

352353
if (resource.get(3) !== serializer.databaseId.database) {
353-
throw new Error(
354+
throw new FirestoreError(
355+
Code.INVALID_ARGUMENT,
354356
'Tried to deserialize key from different database: ' +
355357
resource.get(3) +
356358
' vs ' +

‎packages/firestore/test/integration/api/bundle.test.ts

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -45,71 +45,9 @@ function verifyInProgress(
4545
expect(p.documentsLoaded).to.equal(expectedDocuments);
4646
}
4747

48-
/**
49-
* Returns a testing bundle string for the given projectId.
50-
*
51-
* The function is commented out, but kept for documentation purpose. It accesses SDK
52-
* internals, which is not available in test:minified.
53-
*
54-
* The tests uses `BUNDLE_TEMPLATE` as test data instead, which is generated from this function
55-
* and replaced with different project IDs when required. To update `BUNDLE_TEMPALTE`, you
56-
* need to uncomment the function, and copy/paste from the console output to `BUNDLE_TEMPALTE`.
57-
* It is manual, but should be required only rarely.
58-
*/
59-
/*
60-
function bundleWithTestDocsAndQueries(projectId: string): string {
61-
const testDocs: { [key: string]: firestore.DocumentData } = {
62-
a: { k: { stringValue: 'a' }, bar: { integerValue: 1 } },
63-
b: { k: { stringValue: 'b' }, bar: { integerValue: 2 } }
64-
};
65-
66-
const a = key('coll-1/a');
67-
const b = key('coll-1/b');
68-
const builder = new TestBundleBuilder(new DatabaseId(projectId));
69-
70-
builder.addNamedQuery(
71-
'limit',
72-
{ seconds: 1000, nanos: 9999 },
73-
(collectionReference('coll-1')
74-
.orderBy('bar', 'desc')
75-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
76-
.limit(1) as any)._query
77-
);
78-
builder.addNamedQuery(
79-
'limit-to-last',
80-
{ seconds: 1000, nanos: 9999 },
81-
(collectionReference('coll-1')
82-
.orderBy('bar', 'desc')
83-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
84-
.limitToLast(1) as any)._query
85-
);
86-
87-
builder.addDocumentMetadata(a, { seconds: 1000, nanos: 9999 }, true);
88-
builder.addDocument(
89-
a,
90-
{ seconds: 1, nanos: 9 },
91-
{ seconds: 1, nanos: 9 },
92-
testDocs.a
93-
);
94-
builder.addDocumentMetadata(b, { seconds: 1000, nanos: 9999 }, true);
95-
builder.addDocument(
96-
b,
97-
{ seconds: 1, nanos: 9 },
98-
{ seconds: 1, nanos: 9 },
99-
testDocs.b
100-
);
101-
102-
return builder
103-
.build('test-bundle', { seconds: 1001, nanos: 9999 })
104-
.toString();
105-
}
106-
107-
console.log(
108-
`${bundleWithTestDocsAndQueries('test-project')}`
109-
);
110-
*/
111-
11248
const TEMPLATE_PROJECT_ID = 'test-project';
49+
// This template is generated from bundleWithTestDocsAndQueries in '../util/internal_helpsers.ts',
50+
// and manually copied here.
11351
const BUNDLE_TEMPLATE =
11452
'125{"metadata":{"id":"test-bundle","createTime":{"seconds":1001,"nanos":9999},"version":1,"totalDocuments":2,"totalBytes":1503}}374{"namedQuery":{"name":"limit","readTime":{"seconds":1000,"nanos":9999},"bundledQuery":{"parent":"projects/test-project/databases/(default)/documents","structuredQuery":{"from":[{"collectionId":"coll-1"}],"orderBy":[{"field":{"fieldPath":"bar"},"direction":"DESCENDING"},{"field":{"fieldPath":"__name__"},"direction":"DESCENDING"}],"limit":{"value":1}},"limitType":"FIRST"}}}381{"namedQuery":{"name":"limit-to-last","readTime":{"seconds":1000,"nanos":9999},"bundledQuery":{"parent":"projects/test-project/databases/(default)/documents","structuredQuery":{"from":[{"collectionId":"coll-1"}],"orderBy":[{"field":{"fieldPath":"bar"},"direction":"DESCENDING"},{"field":{"fieldPath":"__name__"},"direction":"DESCENDING"}],"limit":{"value":1}},"limitType":"LAST"}}}147{"documentMetadata":{"name":"projects/test-project/databases/(default)/documents/coll-1/a","readTime":{"seconds":1000,"nanos":9999},"exists":true}}218{"document":{"name":"projects/test-project/databases/(default)/documents/coll-1/a","createTime":{"seconds":1,"nanos":9},"updateTime":{"seconds":1,"nanos":9},"fields":{"k":{"stringValue":"a"},"bar":{"integerValue":1}}}}147{"documentMetadata":{"name":"projects/test-project/databases/(default)/documents/coll-1/b","readTime":{"seconds":1000,"nanos":9999},"exists":true}}218{"document":{"name":"projects/test-project/databases/(default)/documents/coll-1/b","createTime":{"seconds":1,"nanos":9},"updateTime":{"seconds":1,"nanos":9},"fields":{"k":{"stringValue":"b"},"bar":{"integerValue":2}}}}';
11553

‎packages/firestore/test/integration/util/internal_helpers.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ import { User } from '../../../src/auth/user';
3232
import { DEFAULT_PROJECT_ID, DEFAULT_SETTINGS } from './settings';
3333
import { newConnection } from '../../../src/platform/connection';
3434
import { newSerializer } from '../../../src/platform/serializer';
35+
import { key } from '../../util/helpers';
36+
import { TestBundleBuilder } from '../../unit/util/bundle_data';
37+
import { collectionReference } from '../../util/api_helpers';
3538

3639
/** Helper to retrieve the AsyncQueue for a give FirebaseFirestore instance. */
3740
export function asyncQueue(db: firestore.FirebaseFirestore): AsyncQueue {
@@ -95,3 +98,60 @@ export function withMockCredentialProviderTestDb(
9598
}
9699
);
97100
}
101+
102+
/**
103+
* Returns a testing bundle string for the given projectId.
104+
*
105+
* The function is not referenced by bundle.test.ts, instead the bundle string used there
106+
* is generated by this function and copied over there. The reason is this function accesses
107+
* SDK internals, which is not available in test:minified.
108+
*/
109+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
110+
function bundleWithTestDocsAndQueries(
111+
projectId: string = 'test-project'
112+
): string {
113+
const testDocs: { [key: string]: firestore.DocumentData } = {
114+
a: { k: { stringValue: 'a' }, bar: { integerValue: 1 } },
115+
b: { k: { stringValue: 'b' }, bar: { integerValue: 2 } }
116+
};
117+
118+
const a = key('coll-1/a');
119+
const b = key('coll-1/b');
120+
const builder = new TestBundleBuilder(new DatabaseId(projectId));
121+
122+
builder.addNamedQuery(
123+
'limit',
124+
{ seconds: 1000, nanos: 9999 },
125+
(collectionReference('coll-1')
126+
.orderBy('bar', 'desc')
127+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
128+
.limit(1) as any)._query
129+
);
130+
builder.addNamedQuery(
131+
'limit-to-last',
132+
{ seconds: 1000, nanos: 9999 },
133+
(collectionReference('coll-1')
134+
.orderBy('bar', 'desc')
135+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
136+
.limitToLast(1) as any)._query
137+
);
138+
139+
builder.addDocumentMetadata(a, { seconds: 1000, nanos: 9999 }, true);
140+
builder.addDocument(
141+
a,
142+
{ seconds: 1, nanos: 9 },
143+
{ seconds: 1, nanos: 9 },
144+
testDocs.a
145+
);
146+
builder.addDocumentMetadata(b, { seconds: 1000, nanos: 9999 }, true);
147+
builder.addDocument(
148+
b,
149+
{ seconds: 1, nanos: 9 },
150+
{ seconds: 1, nanos: 9 },
151+
testDocs.b
152+
);
153+
154+
return builder
155+
.build('test-bundle', { seconds: 1001, nanos: 9999 })
156+
.toString();
157+
}

0 commit comments

Comments
 (0)