Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: COUNT Queries #1774

Merged
merged 33 commits into from Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f70e121
firestore.d.ts: COUNT API added
dconeybe Jun 20, 2022
76cd8e7
Work in progress
tom-andersen Sep 2, 2022
d9f5170
Implement count with tests.
tom-andersen Sep 12, 2022
6ddf4e9
Fix tests with better termination
tom-andersen Sep 14, 2022
eb422f9
Copy Denvers API changes
tom-andersen Sep 14, 2022
0d936e5
Implement COUNT API changes.
tom-andersen Sep 15, 2022
b1747cc
Add comments
tom-andersen Sep 15, 2022
70b5659
Fix linting errors
tom-andersen Sep 15, 2022
a06f110
Revert manual proto change
tom-andersen Sep 16, 2022
0ecf768
Add comments
tom-andersen Sep 16, 2022
f396bda
Fix types
tom-andersen Sep 16, 2022
349ca1e
Implement retry
tom-andersen Sep 16, 2022
f162f12
Pass transaction
tom-andersen Sep 16, 2022
63fa8d8
Comment out test
tom-andersen Sep 16, 2022
8871496
Fix test
tom-andersen Sep 16, 2022
12aaf16
Remove demo file
tom-andersen Sep 16, 2022
999b910
Run integration test
tom-andersen Sep 16, 2022
4d2ee83
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 16, 2022
401b843
Remove future aggregates
tom-andersen Sep 19, 2022
63ed8bb
Fix emulator
tom-andersen Sep 19, 2022
e21e3f9
Merge branch 'tomandersen/count' of https://github.com/googleapis/nod…
tom-andersen Sep 19, 2022
c58bad7
Enable test
tom-andersen Sep 19, 2022
dc8c3a5
Prettier
tom-andersen Sep 19, 2022
dbf9d1a
Cleanup
tom-andersen Sep 20, 2022
046399c
Add tests
tom-andersen Sep 20, 2022
c519991
Changes from PR review
tom-andersen Sep 20, 2022
d12c82a
Fix PR based on comments
tom-andersen Sep 22, 2022
80e5044
Merge remote-tracking branch 'origin/main' into tomandersen/count
dconeybe Sep 23, 2022
54b8bd7
Merge remote-tracking branch 'origin/main' into tomandersen/count
dconeybe Sep 29, 2022
daf127e
Merge remote-tracking branch 'origin/main' into tomandersen/count
dconeybe Sep 30, 2022
e3136b8
system-test/firestore.ts: remove unconditional call to setLogFunction…
dconeybe Sep 30, 2022
996d86e
docs: Write javadocs for COUNT API (#1783)
dconeybe Oct 3, 2022
38bc42b
Merge branch 'main' into tomandersen/count
tom-andersen Oct 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 6 additions & 18 deletions dev/src/reference.ts
Expand Up @@ -1606,7 +1606,7 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
* result set.
*/
count(): AggregateQuery<{count: firestore.AggregateField<number>}> {
return this.aggregate({count: AggregateField.count()});
return this._aggregate({count: AggregateField.count()});
}

/**
Expand All @@ -1615,7 +1615,7 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
* @param aggregates the aggregations to perform.
* @return an `AggregateQuery` that performs the given aggregations.
*/
aggregate<T extends firestore.AggregateSpec>(
_aggregate<T extends firestore.AggregateSpec>(
tom-andersen marked this conversation as resolved.
Show resolved Hide resolved
aggregates: T
): AggregateQuery<T> {
return new AggregateQuery(this, aggregates);
Expand Down Expand Up @@ -2881,10 +2881,6 @@ export class AggregateQuery<T extends firestore.AggregateSpec>
return this._query;
}

get aggregates(): T {
return this._aggregates;
}

get(): Promise<AggregateQuerySnapshot<T>> {
return this._get();
}
Expand Down Expand Up @@ -2938,7 +2934,7 @@ export class AggregateQuery<T extends firestore.AggregateSpec>
new AggregateQuerySnapshot<T>(this, readTime, data)
);
} else {
callback(Error('unexpected'));
callback(Error('RunAggregationQueryResponse is missing result'));
}
},
});
Expand Down Expand Up @@ -2968,11 +2964,11 @@ export class AggregateQuery<T extends firestore.AggregateSpec>
backendStream.unpipe(stream);
// If a non-transactional query failed, attempt to restart.
// Transactional queries are retried via the transaction runner.
if (!transactionId && !isPermanentRpcError(err, 'runQuery')) {
if (!transactionId && !isPermanentRpcError(err, 'runAggregationQuery')) {
logger(
'Query._stream',
'AggregateQuery._stream',
tag,
'Query failed with retryable stream error:',
'AggregateQuery failed with retryable stream error:',
err
);
streamActive.resolve(/* active= */ true);
Expand Down Expand Up @@ -3093,14 +3089,6 @@ export class AggregateQuerySnapshot<T extends firestore.AggregateSpec>
return this._readTime;
}

getCount(): number {
const count = this._data.count;
if (typeof count === 'number') {
return count;
}
throw new Error('Unexpected count is ' + typeof count);
}

isEqual(other: firestore.AggregateQuerySnapshot<T>): boolean {
if (this === other) {
return true;
Expand Down
14 changes: 7 additions & 7 deletions dev/system-test/firestore.ts
Expand Up @@ -2155,21 +2155,21 @@ describe.only('Aggregates', () => {
it('counts 0 document from non-existent collection', async () => {
const count = randomCol.count();
const res = await firestore.runTransaction(f => f.get(count));
expect(res.getCount()).to.equal(0);
expect(res.data().count).to.equal(0);
});

it('counts 0 document from filtered empty collection', async () => {
await randomCol.doc('doc').set({foo: 'bar'});
const count = randomCol.where('foo', '==', 'notbar').count();
const res = await firestore.runTransaction(f => f.get(count));
expect(res.getCount()).to.equal(0);
expect(res.data().count).to.equal(0);
});

it('counts 1 document', async () => {
await randomCol.doc('doc').set({foo: 'bar'});
const count = randomCol.count();
const res = await firestore.runTransaction(f => f.get(count));
expect(res.getCount()).to.equal(1);
expect(res.data().count).to.equal(1);
});

it('counts multiple documents with filter', async () => {
Expand All @@ -2179,7 +2179,7 @@ describe.only('Aggregates', () => {
await randomCol.doc('doc3').set({notfoo: 'bar'});
const count = randomCol.where('foo', '==', 'bar').count();
const res = await firestore.runTransaction(f => f.get(count));
expect(res.getCount()).to.equal(2);
expect(res.data().count).to.equal(2);
});

it('counts up to limit', async () => {
Expand All @@ -2193,7 +2193,7 @@ describe.only('Aggregates', () => {
await randomCol.doc('doc8').set({foo: 'bar'});
const count = randomCol.limit(5).count();
const res = await firestore.runTransaction(f => f.get(count));
expect(res.getCount()).to.equal(5);
expect(res.data().count).to.equal(5);
});

it('counts with orderBy', async () => {
Expand All @@ -2208,11 +2208,11 @@ describe.only('Aggregates', () => {

const count1 = randomCol.orderBy('foo2').count();
const res1 = await firestore.runTransaction(f => f.get(count1));
expect(res1.getCount()).to.equal(3);
expect(res1.data().count).to.equal(3);

const count2 = randomCol.orderBy('foo3').count();
const res2 = await firestore.runTransaction(f => f.get(count2));
expect(res2.getCount()).to.equal(0);
expect(res2.data().count).to.equal(0);
});
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
});

Expand Down
10 changes: 0 additions & 10 deletions types/firestore.d.ts
Expand Up @@ -1713,14 +1713,6 @@ declare namespace FirebaseFirestore {
*/
count(): AggregateQuery<{count: AggregateField<number>}>;

/**
* Returns `AggregateQuery` for given aggregates based on this `Query`.
*
* @param aggregates Specify aliases with aggregate functions.
* @return An AggregateQuery that contains given aggregates.
*/
aggregate<T extends AggregateSpec>(aggregates: T): AggregateQuery<T>;

/**
* Returns true if this `Query` is equal to the provided one.
*
Expand Down Expand Up @@ -2092,8 +2084,6 @@ declare namespace FirebaseFirestore {

readonly query: Query<unknown>;

readonly aggregates: T;

get(): Promise<AggregateQuerySnapshot<T>>;

/**
Expand Down