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

docs: Write javadocs for COUNT API #1783

Merged
merged 13 commits into from
Oct 3, 2022
239 changes: 157 additions & 82 deletions dev/src/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1599,26 +1599,24 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
}

/**
* Returns an `AggregateQuery` that counts the number of documents in the
* result set.
* Returns a query that counts the documents in the result set of this
* query.
*
* @return an `AggregateQuery` that counts the number of documents in the
* result set.
*/
count(): AggregateQuery<{count: firestore.AggregateField<number>}> {
return this._aggregate({count: AggregateField.count()});
}

/**
* Returns an `AggregateQuery` that performs the given aggregations.
* The returned query, when executed, counts the documents in the result set
* of this query without actually downloading the documents.
*
* Using the returned query to count the documents is efficient because only
* the final count, not the documents' data, is downloaded. The returned
* query can even count the documents if the result set would be
* prohibitively large to download entirely (e.g. thousands of documents).
*
* @param aggregates the aggregations to perform.
* @return an `AggregateQuery` that performs the given aggregations.
* @return a query that counts the documents in the result set of this
* query. The count can be retrieved from `snapshot.data().count`, where
* `snapshot` is the `AggregateQuerySnapshot` resulting from running the
* returned query.
*/
private _aggregate<T extends firestore.AggregateSpec>(
aggregates: T
): AggregateQuery<T> {
return new AggregateQuery(this, aggregates);
count(): AggregateQuery<{count: firestore.AggregateField<number>}> {
return new AggregateQuery(this, {count: {}});
}

/**
Expand Down Expand Up @@ -2854,33 +2852,36 @@ export class CollectionReference<T = firestore.DocumentData>
}
}

export class AggregateField<T> implements firestore.AggregateField<T> {
private constructor() {}

static count(): AggregateField<number> {
return new AggregateField<number>();
}

isEqual(other: firestore.AggregateField<T>): boolean {
return this === other || other instanceof AggregateField;
}
}

/**
* A query that calculates aggregations over an underlying query.
*/
export class AggregateQuery<T extends firestore.AggregateSpec>
implements firestore.AggregateQuery<T>
{
private readonly _query: Query<unknown>;
private readonly _aggregates: T;

constructor(query: Query<any>, aggregates: T) {
this._query = query;
this._aggregates = aggregates;
}
/**
* @private
* @internal
*
* @param _query The query whose aggregations will be calculated by this
* object.
* @param _aggregates The aggregations that will be performed by this query.
*/
constructor(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private readonly _query: Query<any>,
private readonly _aggregates: T
) {}

get query(): firestore.Query<unknown> {
/** The query whose aggregations will be calculated by this object. */
get query(): Query<unknown> {
return this._query;
}

/**
* Executes this query.
*
* @return A promise that will be resolved with the results of the query.
*/
get(): Promise<AggregateQuerySnapshot<T>> {
return this._get();
}
Expand Down Expand Up @@ -2914,14 +2915,14 @@ export class AggregateQuery<T extends firestore.AggregateSpec>
/**
* Internal streaming method that accepts an optional transaction ID.
*
* @param transactionId A transaction ID.
* @private
* @internal
* @param transactionId A transaction ID.
* @returns A stream of document results.
*/
_stream(transactionId?: Uint8Array): Readable {
const tag = requestTag();
const firestore = this._query.firestore;
const firestore = this.query.firestore;

const stream: Transform = new Transform({
objectMode: true,
Expand Down Expand Up @@ -2997,17 +2998,16 @@ export class AggregateQuery<T extends firestore.AggregateSpec>

/**
* Internal method to decode values within result.
*
* @param proto
* @private
*/
private decodeResult(
proto: api.IAggregationResult
): firestore.AggregateSpecData<T> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const data: any = {};
const fields = proto.aggregateFields;
if (fields) {
const serializer = this._query.firestore._serializer!;
const serializer = this.query.firestore._serializer!;
for (const prop of Object.keys(fields)) {
if (this._aggregates[prop] === undefined) {
throw new Error(
Expand All @@ -3029,8 +3029,9 @@ export class AggregateQuery<T extends firestore.AggregateSpec>
* @returns Serialized JSON for the query.
*/
toProto(transactionId?: Uint8Array): api.IRunAggregationQueryRequest {
const queryProto = this._query.toProto();
//TODO(tomandersen) inspect _query to build request - this is just hard coded count right now.
const queryProto = this.query.toProto();
//TODO(tomandersen) inspect _query to build request - this is just hard
// coded count right now.
const runQueryRequest: api.IRunAggregationQueryRequest = {
parent: queryProto.parent,
structuredAggregationQuery: {
Expand All @@ -3051,76 +3052,127 @@ export class AggregateQuery<T extends firestore.AggregateSpec>
return runQueryRequest;
}

/**
* Compares this object with the given object for equality.
*
* This object is considered "equal" to the other object if and only if
* `other` performs the same aggregations as this `AggregateQuery` and
* the underlying `Query` of `other` compares equal to that of this object.
*
* @param other The object to compare to this object for equality.
* @return `true` if this object is "equal" to the given object, as
* defined above, or `false` otherwise.
*/
isEqual(other: firestore.AggregateQuery<T>): boolean {
if (this === other) {
return true;
}
if (other instanceof AggregateQuery) {
if (!this._query.isEqual(other._query)) {
return false;
}

const thisAggregates: [string, AggregateField<unknown>][] =
Object.entries(this._aggregates);
const otherAggregates = other._aggregates;
if (!(other instanceof AggregateQuery)) {
return false;
}
if (!this.query.isEqual(other.query)) {
return false;
}

return (
thisAggregates.length === Object.keys(otherAggregates).length &&
thisAggregates.every(([alias, field]) =>
field.isEqual(otherAggregates[alias])
)
);
// Verify that this object has the same aggregation keys as `other`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this without sort? Maybe there exists or we can create a helper method?

The consequence is likely small since we are likely only dealing with maps of one element. So, maybe we can just have a TODO to revisit this when other aggregate types are implement.

https://stackoverflow.com/a/35951373

Copy link
Contributor Author

@dconeybe dconeybe Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked a little harder (I must have missed this while coding late last night!) and there already exists a deepEqual() function that employs the logic you suggested. So I've modified AggregateQuery.isEqual() and AggregateQuerySnapshot.isEqual() to just use deepEqual().

If interested, here is the raw source code of deepEqual(): https://github.com/epoberezkin/fast-deep-equal/blob/master/src/index.jst, and here is the compiled JavaScript version: https://gist.github.com/dconeybe/c2654f622b87fc0d4c75e5d58364491f

const thisAggregateKeys = Object.keys(this._aggregates).sort();
const otherAggregateKeys = Object.keys(other._aggregates).sort();
if (!isPrimitiveArrayEqual(thisAggregateKeys, otherAggregateKeys)) {
return false;
}
return false;

// TODO: Also compare the `AggregateField` instances from `this._aggregates`
// to their counterparts in `other._aggregates` once `AggregateField` gains
// an `isEqual()` method, which will happen once we support more than one
// type of `AggregateField` (currently, we only support "count").
return true;
}
}

/**
* The results of executing an aggregation query.
*/
export class AggregateQuerySnapshot<T extends firestore.AggregateSpec>
implements firestore.AggregateQuerySnapshot<T>
{
/**
* @private
* @internal
*
* @param _query The query that was executed to produce this result.
* @param _readTime The time this snapshot was read.
* @param _data The results of the aggregations performed over the underlying
* query.
*/
constructor(
private readonly _query: AggregateQuery<T>,
private readonly _readTime: Timestamp,
private readonly _data: firestore.AggregateSpecData<T>
) {}

get query(): firestore.AggregateQuery<T> {
/** The query that was executed to produce this result. */
get query(): AggregateQuery<T> {
return this._query;
}

get readTime(): firestore.Timestamp {
/** The time this snapshot was read. */
get readTime(): Timestamp {
return this._readTime;
}

/**
* Returns the results of the aggregations performed over the underlying
* query.
*
* The keys of the returned object will be the same as those of the
* `AggregateSpec` object specified to the aggregation method, and the
* values will be the corresponding aggregation result.
*
* @returns The results of the aggregations performed over the underlying
* query.
*/
data(): firestore.AggregateSpecData<T> {
return this._data;
}

/**
* Compares this object with the given object for equality.
*
* Two `AggregateQuerySnapshot` instances are considered "equal" if they
* have the read time, the same data, and underlying queries that compare
* "equal" using `AggregateQuery.isEqual()`.
*
* @param other The object to compare to this object for equality.
* @return `true` if this object is "equal" to the given object, as
* defined above, or `false` otherwise.
*/
isEqual(other: firestore.AggregateQuerySnapshot<T>): boolean {
if (this === other) {
return true;
}
if (other instanceof AggregateQuerySnapshot) {
if (!this._query.isEqual(other._query)) {
return false;
}

const thisData = this._data;
const thisDataKeys: string[] = Object.keys(thisData);
if (!(other instanceof AggregateQuerySnapshot)) {
return false;
}
if (!this.readTime.isEqual(other.readTime)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isEquals method for QuerySnapshot does not include timestamp. Are you sure this is correct behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. DocumentSnapshot is actually a better class for comparison than QuerySnapshot, and it indeed explicitly excludes checking the read time:

// Since the read time is different on every document read, we explicitly
// ignore all document metadata in this comparison.

I've removed the comparison of readTime, as suggested, to match the behavior of DocumentSnapshot.isEqual(). I've also updated the documentation, and will make the same fix in the java-firestore SDK. Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, here is the corresponding fix in the java-firestore SDK: googleapis/java-firestore#1051

return false;
}
if (!this.query.isEqual(other.query)) {
return false;
}

const otherData = other._data;
const otherDataKeys: string[] = Object.keys(otherData);
// Verify that this object has the same aggregation keys as `other`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this better than previous map comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably not better. I had mistakenly thought that sorting the keys is required. I've changed it to use deepEqual() now.

const thisAggregateKeys = Object.keys(this._data).sort();
const otherAggregateKeys = Object.keys(other._data).sort();
if (!isPrimitiveArrayEqual(thisAggregateKeys, otherAggregateKeys)) {
return false;
}

return (
thisDataKeys.length === otherDataKeys.length &&
thisDataKeys.every(
alias =>
Object.prototype.hasOwnProperty.call(otherData, alias) &&
thisData[alias] === otherData[alias]
)
);
// Verify that this object has the same aggregation results as `other`.
if (!thisAggregateKeys.every(key => this._data[key] === other._data[key])) {
return false;
}
return false;
}

data(): firestore.AggregateSpecData<T> {
return this._data;
return true;
}
}

Expand Down Expand Up @@ -3255,6 +3307,29 @@ function isArrayEqual<T extends {isEqual: (t: T) => boolean}>(
return true;
}

/**
* Verifies equality for an array of primitives using the `===` operator.
*
* @private
* @internal
* @param left Array of primitives.
* @param right Array of primitives.
* @return True if arrays are equal.
*/
function isPrimitiveArrayEqual<T>(left: T[], right: T[]): boolean {
if (left.length !== right.length) {
return false;
}

for (let i = 0; i < left.length; ++i) {
if (left[i] !== right[i]) {
return false;
}
}

return true;
}

/**
* Returns the first non-undefined value or `undefined` if no such value exists.
* @private
Expand Down
Loading