Skip to content

Commit d7b5ec3

Browse files
committed
resolve comments
1 parent 654075e commit d7b5ec3

File tree

2 files changed

+30
-34
lines changed

2 files changed

+30
-34
lines changed

packages/firestore/src/lite-api/aggregate.ts

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,18 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { CompositeFilterOpEnum } from '../protos/firestore_proto_api';
18+
import { Value } from '../protos/firestore_proto_api';
1919
import { invokeRunAggregationQueryRpc } from '../remote/datastore';
2020
import { hardAssert } from '../util/assert';
2121
import { cast } from '../util/input_validation';
22+
2223
import { getDatastore } from './components';
2324
import { Firestore } from './database';
2425
import { Query, queryEqual } from './reference';
2526
import { LiteUserDataWriter } from './reference_impl';
2627

2728
/**
28-
* A `AggregateQuery` computes some aggregation statistics from the result set of
29+
* An `AggregateQuery` computes some aggregation statistics from the result set of
2930
* a base `Query`.
3031
*/
3132
export class AggregateQuery {
@@ -44,7 +45,7 @@ export class AggregateQuery {
4445
}
4546

4647
/**
47-
* A `AggregateQuerySnapshot` contains results of a `AggregateQuery`.
48+
* An `AggregateQuerySnapshot` contains results of a `AggregateQuery`.
4849
*/
4950
export class AggregateQuerySnapshot {
5051
readonly type = 'AggregateQuerySnapshot';
@@ -56,7 +57,7 @@ export class AggregateQuerySnapshot {
5657
}
5758

5859
/**
59-
* @return The result of a document count aggregation. Returns null if no count aggregation is
60+
* @returns The result of a document count aggregation. Returns null if no count aggregation is
6061
* available in the result.
6162
*/
6263
getCount(): number | null {
@@ -67,43 +68,42 @@ export class AggregateQuerySnapshot {
6768
/**
6869
* Creates an `AggregateQuery` counting the number of documents matching this query.
6970
*
70-
* @return An `AggregateQuery` object that can be used to count the number of documents in
71+
* @returns An `AggregateQuery` object that can be used to count the number of documents in
7172
* the result set of this query.
7273
*/
7374
export function countQuery(query: Query<unknown>): AggregateQuery {
7475
return new AggregateQuery(query);
7576
}
7677

7778
export function getAggregateFromServerDirect(
78-
aggregateQuery: AggregateQuery
79+
query: AggregateQuery
7980
): Promise<AggregateQuerySnapshot> {
80-
const firestore = cast(aggregateQuery.query.firestore, Firestore);
81+
const firestore = cast(query.query.firestore, Firestore);
8182
const datastore = getDatastore(firestore);
8283
const userDataWriter = new LiteUserDataWriter(firestore);
8384

84-
return invokeRunAggregationQueryRpc(datastore, aggregateQuery).then(
85-
result => {
86-
hardAssert(
87-
result[0] !== undefined,
88-
'Aggregation fields are missing from result.'
89-
);
85+
return invokeRunAggregationQueryRpc(datastore, query).then(result => {
86+
hardAssert(
87+
result[0] !== undefined,
88+
'Aggregation fields are missing from result.'
89+
);
9090

91-
const countField = (result[0] as any).count_alias;
92-
hardAssert(
93-
countField !== undefined,
94-
'Count field is missing from result.'
95-
);
91+
const counts = Object.entries(result[0])
92+
.filter(([key, value]) => key === 'count_alias')
93+
.map(([key, value]) => userDataWriter.convertValue(value as Value));
9694

97-
const countAggregateResult = userDataWriter.convertValue(countField);
98-
hardAssert(
99-
typeof countAggregateResult === 'number',
100-
'Count aggeragte field is not a number: ' + countAggregateResult
101-
);
102-
return Promise.resolve(
103-
new AggregateQuerySnapshot(aggregateQuery, countAggregateResult)
104-
);
105-
}
106-
);
95+
const count = counts[0];
96+
hardAssert(
97+
count !== undefined,
98+
'count_alias property is missing from result.'
99+
);
100+
hardAssert(
101+
typeof count === 'number',
102+
'Count aggeragte field is not a number: ' + count
103+
);
104+
105+
return Promise.resolve(new AggregateQuerySnapshot(query, count));
106+
});
107107
}
108108

109109
export function aggregateQueryEqual(

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ import {
8383
import { Timestamp } from '../../src/lite-api/timestamp';
8484
import { runTransaction } from '../../src/lite-api/transaction';
8585
import { writeBatch } from '../../src/lite-api/write_batch';
86-
8786
import {
8887
DEFAULT_PROJECT_ID,
8988
DEFAULT_SETTINGS
@@ -2058,26 +2057,23 @@ describe('countQuery()', () => {
20582057
it('AggregateQuery and AggregateQuerySnapshot inherits the original query', () => {
20592058
return withTestCollection(async coll => {
20602059
const query_ = query(coll);
2061-
20622060
const countQuery_ = countQuery(query_);
2063-
expect(countQuery_.type).to.equal('AggregateQuery');
20642061
expect(countQuery_.query).to.equal(query_);
2065-
20662062
const snapshot = await getAggregateFromServerDirect(countQuery_);
2063+
expect(snapshot.query).to.equal(countQuery_);
20672064
expect(snapshot.query.query).to.equal(query_);
20682065
});
20692066
});
20702067

20712068
it('empty test collection count', () => {
20722069
return withTestCollection(async coll => {
20732070
const countQuery_ = countQuery(query(coll));
2074-
20752071
const snapshot = await getAggregateFromServerDirect(countQuery_);
20762072
expect(snapshot.getCount()).to.equal(0);
20772073
});
20782074
});
20792075

2080-
it('test collection count with sample docs ', () => {
2076+
it('test collection count with 5 docs', () => {
20812077
return withTestCollectionAndInitialData(testDocs, async collection => {
20822078
const countQuery_ = countQuery(query(collection));
20832079
const snapshot = await getAggregateFromServerDirect(countQuery_);

0 commit comments

Comments
 (0)