KEMBAR78
feat: Transaction.get(AggregateQuery) added by dconeybe · Pull Request #1029 · googleapis/java-firestore · GitHub
Skip to content

Conversation

@dconeybe
Copy link
Contributor

No description provided.

@dconeybe dconeybe added the api: firestore Issues related to the googleapis/java-firestore API. label Aug 31, 2022
@dconeybe dconeybe self-assigned this Aug 31, 2022
@dconeybe dconeybe requested a review from a team as a code owner August 31, 2022 20:20
@conventional-commit-lint-gcf
Copy link

conventional-commit-lint-gcf bot commented Aug 31, 2022

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@dconeybe dconeybe force-pushed the dconeybe/CountTransactionSupport branch from 5bb721e to b6a2556 Compare August 31, 2022 20:22
@dconeybe dconeybe changed the title Add transaction support to COUNT queries feat: Transaction.get(AggregateQuery) added Aug 31, 2022
@dconeybe dconeybe requested review from ehsannas and wu-hui August 31, 2022 20:22
Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

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

Great! One minor fix needed pointed out below.

RunAggregationQueryResponse.newBuilder()
.setResult(
AggregationResult.newBuilder()
.putAggregateFields("count", Value.newBuilder().setIntegerValue(42).build())
Copy link
Contributor

Choose a reason for hiding this comment

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

s/42/count ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep!

}

@Test
public void aggregateQueryInATransactionShouldLockTheCountedDocuments() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this test :)

@dconeybe dconeybe merged commit 23a1557 into count Sep 8, 2022
@dconeybe dconeybe deleted the dconeybe/CountTransactionSupport branch September 8, 2022 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the googleapis/java-firestore API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants