Skip to content

Commit 889e81f

Browse files
milaGGLmilaGGL
committed
resolve comments
1 parent c00b9a8 commit 889e81f

File tree

4 files changed

+72
-97
lines changed

4 files changed

+72
-97
lines changed

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -512,11 +512,6 @@ export function firestoreClientRunAggregationQuery(
512512
client.asyncQueue.enqueueAndForget(async () => {
513513
const remoteStore = await getRemoteStore(client);
514514
if (!canUseNetwork(remoteStore)) {
515-
logDebug(
516-
LOG_TAG,
517-
'The network is disabled. The task returned by ' +
518-
"'getAggregateFromServerDirect()' will not complete until the network is enabled."
519-
);
520515
deferred.reject(
521516
new FirestoreError(
522517
Code.UNAVAILABLE,

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

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import {
3232
apiDescribe,
3333
postConverter,
34+
withEmptyTestCollection,
3435
withTestCollection,
3536
withTestDb
3637
} from '../util/helpers';
@@ -41,31 +42,24 @@ apiDescribe('Aggregation query', (persistence: boolean) => {
4142
a: { author: 'authorA', title: 'titleA' },
4243
b: { author: 'authorB', title: 'titleB' }
4344
};
44-
return withTestCollection(persistence, testDocs, async collection => {
45-
const countQuery_ = countQuery(collection);
45+
return withTestCollection(persistence, testDocs, async coll => {
46+
const countQuery_ = countQuery(coll);
4647
const snapshot = await getAggregateFromServerDirect(countQuery_);
4748
expect(snapshot.getCount()).to.equal(2);
4849
});
4950
});
5051

5152
it('aggregateQuery.query equals to original query', () => {
52-
const testDocs = {
53-
a: { author: 'authorA', title: 'titleA' }
54-
};
55-
return withTestCollection(persistence, testDocs, async collection => {
56-
const query_ = query(collection, where('author', '==', 'authorA'));
53+
return withEmptyTestCollection(persistence, async coll => {
54+
const query_ = query(coll);
5755
const aggregateQuery_ = countQuery(query_);
5856
expect(aggregateQuery_.query).to.be.equal(query_);
5957
});
6058
});
6159

6260
it('aggregateQuerySnapshot.query equals to aggregateQuery', () => {
63-
const testDocs = {
64-
a: { author: 'authorA', title: 'titleA' }
65-
};
66-
return withTestCollection(persistence, testDocs, async collection => {
67-
const query_ = query(collection, where('author', '==', 'authorA'));
68-
const aggregateQuery_ = countQuery(query_);
61+
return withEmptyTestCollection(persistence, async coll => {
62+
const aggregateQuery_ = countQuery(coll);
6963
const snapshot = await getAggregateFromServerDirect(aggregateQuery_);
7064
expect(snapshot.query).to.be.equal(aggregateQuery_);
7165
});
@@ -76,9 +70,9 @@ apiDescribe('Aggregation query', (persistence: boolean) => {
7670
a: { author: 'authorA', title: 'titleA' },
7771
b: { author: 'authorB', title: 'titleB' }
7872
};
79-
return withTestCollection(persistence, testDocs, async collection => {
73+
return withTestCollection(persistence, testDocs, async coll => {
8074
const query_ = query(
81-
collection,
75+
coll,
8276
where('author', '==', 'authorA')
8377
).withConverter(postConverter);
8478
const countQuery_ = countQuery(query_);
@@ -109,34 +103,20 @@ apiDescribe('Aggregation query', (persistence: boolean) => {
109103
});
110104

111105
it('aggregate query fails if firestore is terminated', () => {
112-
const testDocs = {
113-
a: { author: 'authorA', title: 'titleA' }
114-
};
115-
return withTestCollection(
116-
persistence,
117-
testDocs,
118-
async (collection, firestore) => {
119-
await terminate(firestore);
120-
const countQuery_ = countQuery(collection);
121-
expect(() => getAggregateFromServerDirect(countQuery_)).to.throw(
122-
'The client has already been terminated.'
123-
);
124-
}
125-
);
106+
return withEmptyTestCollection(persistence, async (coll, firestore) => {
107+
await terminate(firestore);
108+
const countQuery_ = countQuery(coll);
109+
expect(() => getAggregateFromServerDirect(countQuery_)).to.throw(
110+
'The client has already been terminated.'
111+
);
112+
});
126113
});
127114

128-
it("terminate doesn't crash when there is flying aggregate query", () => {
129-
const testDocs = {
130-
a: { author: 'authorA', title: 'titleA' }
131-
};
132-
return withTestCollection(
133-
persistence,
134-
testDocs,
135-
async (collection, firestore) => {
136-
const countQuery_ = countQuery(collection);
137-
getAggregateFromServerDirect(countQuery_).then();
138-
await terminate(firestore);
139-
}
140-
);
115+
it("terminate doesn't crash when there is aggregate query in flight", () => {
116+
return withEmptyTestCollection(persistence, async (coll, firestore) => {
117+
const countQuery_ = countQuery(coll);
118+
void getAggregateFromServerDirect(countQuery_);
119+
await terminate(firestore);
120+
});
141121
});
142122
});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,14 @@ export function withTestCollection(
255255
return withTestCollectionSettings(persistence, DEFAULT_SETTINGS, docs, fn);
256256
}
257257

258+
export function withEmptyTestCollection(
259+
persistence: boolean,
260+
fn: (collection: CollectionReference, db: Firestore) => Promise<void>
261+
): Promise<void> {
262+
return withTestDb(persistence, db => {
263+
return withTestCollectionSettings(persistence, DEFAULT_SETTINGS, {}, fn);
264+
});
265+
}
258266
// TODO(mikelehen): Once we wipe the database between tests, we can probably
259267
// return the same collection every time.
260268
export function withTestCollectionSettings(

‎packages/firestore/test/lite/integration.test.ts

Lines changed: 42 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2047,8 +2047,8 @@ describe('withConverter() support', () => {
20472047

20482048
describe('countQuery()', () => {
20492049
it('AggregateQuery and AggregateQuerySnapshot inherits the original query', () => {
2050-
return withTestCollection(async collection => {
2051-
const query_ = query(collection);
2050+
return withTestCollection(async coll => {
2051+
const query_ = query(coll);
20522052
const countQuery_ = countQuery(query_);
20532053
const snapshot = await getAggregate(countQuery_);
20542054
expect(countQuery_.query).to.equal(query_);
@@ -2058,8 +2058,8 @@ describe('countQuery()', () => {
20582058
});
20592059

20602060
it('empty test collection count', () => {
2061-
return withTestCollection(async collection => {
2062-
const countQuery_ = countQuery(collection);
2061+
return withTestCollection(async coll => {
2062+
const countQuery_ = countQuery(coll);
20632063
const snapshot = await getAggregate(countQuery_);
20642064
expect(snapshot.getCount()).to.equal(0);
20652065
});
@@ -2071,8 +2071,8 @@ describe('countQuery()', () => {
20712071
{ author: 'authorA', title: 'titleB' },
20722072
{ author: 'authorB', title: 'titleC' }
20732073
];
2074-
return withTestCollectionAndInitialData(testDocs, async collection => {
2075-
const countQuery_ = countQuery(collection);
2074+
return withTestCollectionAndInitialData(testDocs, async coll => {
2075+
const countQuery_ = countQuery(coll);
20762076
const snapshot = await getAggregate(countQuery_);
20772077
expect(snapshot.getCount()).to.equal(3);
20782078
});
@@ -2084,8 +2084,8 @@ describe('countQuery()', () => {
20842084
{ author: 'authorA', title: 'titleB' },
20852085
{ author: 'authorB', title: 'titleC' }
20862086
];
2087-
return withTestCollectionAndInitialData(testDocs, async collection => {
2088-
const query_ = query(collection, where('author', '==', 'authorA'));
2087+
return withTestCollectionAndInitialData(testDocs, async coll => {
2088+
const query_ = query(coll, where('author', '==', 'authorA'));
20892089
const countQuery_ = countQuery(query_);
20902090
const snapshot = await getAggregate(countQuery_);
20912091
expect(snapshot.getCount()).to.equal(2);
@@ -2098,12 +2098,8 @@ describe('countQuery()', () => {
20982098
{ author: 'authorA', title: 'titleB' },
20992099
{ author: 'authorB', title: 'titleC' }
21002100
];
2101-
return withTestCollectionAndInitialData(testDocs, async collection => {
2102-
const query_ = query(
2103-
collection,
2104-
where('author', '==', 'authorA'),
2105-
limit(1)
2106-
);
2101+
return withTestCollectionAndInitialData(testDocs, async coll => {
2102+
const query_ = query(coll, where('author', '==', 'authorA'), limit(1));
21072103
const countQuery_ = countQuery(query_);
21082104
const snapshot = await getAggregate(countQuery_);
21092105
expect(snapshot.getCount()).to.equal(1);
@@ -2116,12 +2112,8 @@ describe('countQuery()', () => {
21162112
{ author: 'authorA', title: 'titleB' },
21172113
{ author: 'authorB', title: 'titleC' }
21182114
];
2119-
return withTestCollectionAndInitialData(testDocs, async collection => {
2120-
const query_ = query(
2121-
collection,
2122-
where('author', '==', 'authorA'),
2123-
limit(3)
2124-
);
2115+
return withTestCollectionAndInitialData(testDocs, async coll => {
2116+
const query_ = query(coll, where('author', '==', 'authorA'), limit(3));
21252117
const countQuery_ = countQuery(query_);
21262118
const snapshot = await getAggregate(countQuery_);
21272119
expect(snapshot.getCount()).to.equal(2);
@@ -2135,8 +2127,8 @@ describe('countQuery()', () => {
21352127
{ author: 'authorB', title: null },
21362128
{ author: 'authorB' }
21372129
];
2138-
return withTestCollectionAndInitialData(testDocs, async collection => {
2139-
const query_ = query(collection, orderBy('title'));
2130+
return withTestCollectionAndInitialData(testDocs, async coll => {
2131+
const query_ = query(coll, orderBy('title'));
21402132
const countQuery_ = countQuery(query_);
21412133
const snapshot = await getAggregate(countQuery_);
21422134
expect(snapshot.getCount()).to.equal(3);
@@ -2150,8 +2142,8 @@ describe('countQuery()', () => {
21502142
{ id: 2, author: 'authorB', title: 'titleC' },
21512143
{ id: null, author: 'authorB', title: 'titleD' }
21522144
];
2153-
return withTestCollectionAndInitialData(testDocs, async collection => {
2154-
const query_ = query(collection, orderBy('id'), startAt(2));
2145+
return withTestCollectionAndInitialData(testDocs, async coll => {
2146+
const query_ = query(coll, orderBy('id'), startAt(2));
21552147
const countQuery_ = countQuery(query_);
21562148
const snapshot = await getAggregate(countQuery_);
21572149
expect(snapshot.getCount()).to.equal(2);
@@ -2165,8 +2157,8 @@ describe('countQuery()', () => {
21652157
{ id: 2, author: 'authorB', title: 'titleC' },
21662158
{ id: null, author: 'authorB', title: 'titleD' }
21672159
];
2168-
return withTestCollectionAndInitialData(testDocs, async collection => {
2169-
const query_ = query(collection, orderBy('id'), startAfter(2));
2160+
return withTestCollectionAndInitialData(testDocs, async coll => {
2161+
const query_ = query(coll, orderBy('id'), startAfter(2));
21702162
const countQuery_ = countQuery(query_);
21712163
const snapshot = await getAggregate(countQuery_);
21722164
expect(snapshot.getCount()).to.equal(1);
@@ -2180,8 +2172,8 @@ describe('countQuery()', () => {
21802172
{ id: 2, author: 'authorB', title: 'titleC' },
21812173
{ id: null, author: 'authorB', title: 'titleD' }
21822174
];
2183-
return withTestCollectionAndInitialData(testDocs, async collection => {
2184-
const query_ = query(collection, orderBy('id'), startAt(1), endAt(2));
2175+
return withTestCollectionAndInitialData(testDocs, async coll => {
2176+
const query_ = query(coll, orderBy('id'), startAt(1), endAt(2));
21852177
const countQuery_ = countQuery(query_);
21862178
const snapshot = await getAggregate(countQuery_);
21872179
expect(snapshot.getCount()).to.equal(2);
@@ -2195,8 +2187,8 @@ describe('countQuery()', () => {
21952187
{ id: 2, author: 'authorB', title: 'titleC' },
21962188
{ id: null, author: 'authorB', title: 'titleD' }
21972189
];
2198-
return withTestCollectionAndInitialData(testDocs, async collection => {
2199-
const query_ = query(collection, orderBy('id'), startAt(1), endBefore(2));
2190+
return withTestCollectionAndInitialData(testDocs, async coll => {
2191+
const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2));
22002192
const countQuery_ = countQuery(query_);
22012193
const snapshot = await getAggregate(countQuery_);
22022194
expect(snapshot.getCount()).to.equal(1);
@@ -2209,9 +2201,9 @@ describe('countQuery()', () => {
22092201
{ author: 'authorA', title: 'titleB' },
22102202
{ author: 'authorB', title: 'titleC' }
22112203
];
2212-
return withTestCollectionAndInitialData(testDocs, async collection => {
2204+
return withTestCollectionAndInitialData(testDocs, async coll => {
22132205
const query_ = query(
2214-
collection,
2206+
coll,
22152207
where('author', '==', 'authorA')
22162208
).withConverter(postConverter);
22172209
const countQuery_ = countQuery(query_);
@@ -2247,9 +2239,9 @@ describe('countQuery()', () => {
22472239
{ author: 'authorA', title: 'titleB' },
22482240
{ author: 'authorB', title: 'titleC' }
22492241
];
2250-
return withTestCollectionAndInitialData(testDocs, async collection => {
2251-
const query1 = query(collection, where('author', '==', 'authorA'));
2252-
const query2 = query(collection, where('author', '==', 'authorA'));
2242+
return withTestCollectionAndInitialData(testDocs, async coll => {
2243+
const query1 = query(coll, where('author', '==', 'authorA'));
2244+
const query2 = query(coll, where('author', '==', 'authorA'));
22532245
const countQuery1 = countQuery(query1);
22542246
const countQuery2 = countQuery(query2);
22552247
expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.true;
@@ -2262,9 +2254,9 @@ describe('countQuery()', () => {
22622254
{ author: 'authorA', title: 'titleB' },
22632255
{ author: 'authorB', title: 'titleC' }
22642256
];
2265-
return withTestCollectionAndInitialData(testDocs, async collection => {
2266-
const query1 = query(collection, where('author', '==', 'authorA'));
2267-
const query2 = query(collection, where('author', '==', 'authorB'));
2257+
return withTestCollectionAndInitialData(testDocs, async coll => {
2258+
const query1 = query(coll, where('author', '==', 'authorA'));
2259+
const query2 = query(coll, where('author', '==', 'authorB'));
22682260
const countQuery1 = countQuery(query1);
22692261
const countQuery2 = countQuery(query2);
22702262
expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.false;
@@ -2277,9 +2269,9 @@ describe('countQuery()', () => {
22772269
{ author: 'authorA', title: 'titleB' },
22782270
{ author: 'authorB', title: 'titleC' }
22792271
];
2280-
return withTestCollectionAndInitialData(testDocs, async collection => {
2281-
const query1 = query(collection, where('author', '==', 'authorA'));
2282-
const query2 = query(collection, where('author', '==', 'authorA'));
2272+
return withTestCollectionAndInitialData(testDocs, async coll => {
2273+
const query1 = query(coll, where('author', '==', 'authorA'));
2274+
const query2 = query(coll, where('author', '==', 'authorA'));
22832275
const countQuery1A = countQuery(query1);
22842276
const countQuery1B = countQuery(query1);
22852277
const countQuery2 = countQuery(query2);
@@ -2297,9 +2289,9 @@ describe('countQuery()', () => {
22972289
{ author: 'authorA', title: 'titleB' },
22982290
{ author: 'authorB', title: 'titleC' }
22992291
];
2300-
return withTestCollectionAndInitialData(testDocs, async collection => {
2301-
const query1 = query(collection, where('author', '==', 'authorA'));
2302-
const query2 = query(collection, where('author', '==', 'authorB'));
2292+
return withTestCollectionAndInitialData(testDocs, async coll => {
2293+
const query1 = query(coll, where('author', '==', 'authorA'));
2294+
const query2 = query(coll, where('author', '==', 'authorB'));
23032295
const countQuery1 = countQuery(query1);
23042296
const countQuery2 = countQuery(query2);
23052297
const snapshot1 = await getAggregate(countQuery1);
@@ -2309,9 +2301,9 @@ describe('countQuery()', () => {
23092301
});
23102302

23112303
it('count query on a terminated Firestore', () => {
2312-
return withTestCollection(async collection => {
2313-
await terminate(collection.firestore);
2314-
const countQuery_ = countQuery(collection);
2304+
return withTestCollection(async coll => {
2305+
await terminate(coll.firestore);
2306+
const countQuery_ = countQuery(coll);
23152307
expect(() => getAggregate(countQuery_)).to.throw(
23162308
'The client has already been terminated.'
23172309
);
@@ -2324,10 +2316,10 @@ describe('countQuery()', () => {
23242316
{ author: 'authorA', title: 'titleB' },
23252317
{ author: 'authorB', title: 'titleC' }
23262318
];
2327-
return withTestCollectionAndInitialData(testDocs, async collection => {
2328-
const countQuery_ = countQuery(collection);
2319+
return withTestCollectionAndInitialData(testDocs, async coll => {
2320+
const countQuery_ = countQuery(coll);
23292321
const promise = getAggregate(countQuery_);
2330-
await terminate(collection.firestore);
2322+
await terminate(coll.firestore);
23312323
const snapshot = await promise;
23322324
expect(snapshot.getCount()).to.equal(3);
23332325
});

0 commit comments

Comments
 (0)