-
Notifications
You must be signed in to change notification settings - Fork 63
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 (not available for use yet) #1033
Conversation
…ogleapis-gen at HEAD, 3445f74bc2b52dc4df2b33a97a6a92026019e4fa
…ds, which were deleted by the import from googleapis-gen but are needed.
This comment was marked as outdated.
This comment was marked as outdated.
…ackage This fixes the following warning from `mvn -B dependency:analyze -DfailOnWarning=true` ``` [WARNING] Used undeclared dependencies found: [WARNING] com.google.http-client:google-http-client:jar:1.42.2:compile [WARNING] Non-test scoped test only dependencies found: [WARNING] com.google.http-client:google-http-client:jar:1.42.2:compile ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testing is very thorough. Love it.
There are a few things that need to be addressed.
1- comments for the class and methods of AggregateQuery
2- comments for the class and methods of AggregateQuerySnapshot
3- Moving the integration test to it/
I'd say land this PR as is and address my comments in the next PR.
|
||
@Override | ||
public void onResponse(RunAggregationQueryResponse response) { | ||
if (!isFutureNotified.compareAndSet(false, true)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be good to add a comment that we expect only one response and why. Please do as part of the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: #1041
@@ -1846,6 +1846,12 @@ private boolean isRetryableError(Throwable throwable) { | |||
return false; | |||
} | |||
|
|||
// TODO(count) Make this method public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also need a comment for the method. Please do as part of the next PR.
Thanks for the review, @ehsannas! Regarding your comments...
|
🤖 I have created a release *beep* *boop* --- ## [3.6.0](https://togithub.com/googleapis/java-firestore/compare/v3.5.0...v3.6.0) (2022-10-04) ### Features * Count queries (not available for use yet) ([#1033](https://togithub.com/googleapis/java-firestore/issues/1033)) ([8b60612](https://togithub.com/googleapis/java-firestore/commit/8b60612f1922a4c377fac357ee7f4304362622f3)) * Make count queries publicly available for use ([#1042](https://togithub.com/googleapis/java-firestore/issues/1042)) ([1c8d242](https://togithub.com/googleapis/java-firestore/commit/1c8d2428d94ab8b3c18a8cad14daa2a1e39af369)) ### Documentation * AggregateQuery.java: describe why we ignore subsequent responses ([#1041](https://togithub.com/googleapis/java-firestore/issues/1041)) ([8150544](https://togithub.com/googleapis/java-firestore/commit/8150544aee0aa04a5e6239dbc4994ddd056393ba)) ### Dependencies * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.4 ([#1053](https://togithub.com/googleapis/java-firestore/issues/1053)) ([9b4c26e](https://togithub.com/googleapis/java-firestore/commit/9b4c26eac062fa73c8b275978f35c5bfb8cdfc21)) * Update dependency org.graalvm.buildtools:junit-platform-native to v0.9.14 ([#1045](https://togithub.com/googleapis/java-firestore/issues/1045)) ([04b3861](https://togithub.com/googleapis/java-firestore/commit/04b3861d874dcd63713e62bd7717097ca8f68a3c)) * Update dependency org.graalvm.buildtools:native-maven-plugin to v0.9.14 ([#1046](https://togithub.com/googleapis/java-firestore/issues/1046)) ([6631a58](https://togithub.com/googleapis/java-firestore/commit/6631a58553a12e1f48d060d0b87cfa8683492b94)) * Update dependency org.junit.vintage:junit-vintage-engine to v5.9.1 ([#1039](https://togithub.com/googleapis/java-firestore/issues/1039)) ([84423f4](https://togithub.com/googleapis/java-firestore/commit/84423f4db6e35f9d30a34a5bc2d29050380840ec)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [3.6.0](https://togithub.com/googleapis/java-firestore/compare/v3.5.0...v3.6.0) (2022-10-04) ### Features * Count queries (not available for use yet) ([#1033](https://togithub.com/googleapis/java-firestore/issues/1033)) ([8b60612](https://togithub.com/googleapis/java-firestore/commit/8b60612f1922a4c377fac357ee7f4304362622f3)) * Make count queries publicly available for use ([#1042](https://togithub.com/googleapis/java-firestore/issues/1042)) ([1c8d242](https://togithub.com/googleapis/java-firestore/commit/1c8d2428d94ab8b3c18a8cad14daa2a1e39af369)) ### Documentation * AggregateQuery.java: describe why we ignore subsequent responses ([#1041](https://togithub.com/googleapis/java-firestore/issues/1041)) ([8150544](https://togithub.com/googleapis/java-firestore/commit/8150544aee0aa04a5e6239dbc4994ddd056393ba)) ### Dependencies * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.4 ([#1053](https://togithub.com/googleapis/java-firestore/issues/1053)) ([9b4c26e](https://togithub.com/googleapis/java-firestore/commit/9b4c26eac062fa73c8b275978f35c5bfb8cdfc21)) * Update dependency org.graalvm.buildtools:junit-platform-native to v0.9.14 ([#1045](https://togithub.com/googleapis/java-firestore/issues/1045)) ([04b3861](https://togithub.com/googleapis/java-firestore/commit/04b3861d874dcd63713e62bd7717097ca8f68a3c)) * Update dependency org.graalvm.buildtools:native-maven-plugin to v0.9.14 ([#1046](https://togithub.com/googleapis/java-firestore/issues/1046)) ([6631a58](https://togithub.com/googleapis/java-firestore/commit/6631a58553a12e1f48d060d0b87cfa8683492b94)) * Update dependency org.junit.vintage:junit-vintage-engine to v5.9.1 ([#1039](https://togithub.com/googleapis/java-firestore/issues/1039)) ([84423f4](https://togithub.com/googleapis/java-firestore/commit/84423f4db6e35f9d30a34a5bc2d29050380840ec)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Add support for "count" queries; that is, a query that gets the number of documents in the result set without actually downloading the documents.
This PR implements all of the logic but does not expose the API. A follow-up PR will change the visibility of the related classes and methods to
public
.