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

Firestore - optimistic locking #171

Merged
merged 9 commits into from
Dec 30, 2020
Merged

Conversation

dmitry-s
Copy link
Contributor

@dmitry-s dmitry-s commented Dec 10, 2020

@meltsufin meltsufin changed the title optimistic locking Enable optimistic locking for Firestore Dec 10, 2020
@dmitry-s dmitry-s changed the title Enable optimistic locking for Firestore Firestore - optimistic locking Dec 10, 2020
Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Documentation and perhaps updates to the sample seems to be missing.

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

Reviewed in person,

Copy link
Contributor

@dzou dzou left a comment

Choose a reason for hiding this comment

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

Based on the in person review it looks good on my end.

However it looks like the integration tests may have broken; the error appears related to this feature:

Error:  writeReadDeleteTest  Time elapsed: 3.249 s  <<< ERROR!
io.grpc.StatusRuntimeException: FAILED_PRECONDITION: the stored version (0) does not match the required base version (1607632501741884)

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Remaining items:

  • documentation
  • check transaction error message

this.firestoreTemplate.saveAll(Flux.just(bob)).as(operator::transactional).collectList()
.as(operator::transactional)
.as(StepVerifier::create)
.expectError()
Copy link
Member

Choose a reason for hiding this comment

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

Need to check the cause such that it is due to violation of optimistic locking.

Copy link
Member

@meltsufin meltsufin Dec 30, 2020

Choose a reason for hiding this comment

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

Created #187 to track this.

@sonarcloud
Copy link

sonarcloud bot commented Dec 30, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

93.8% 93.8% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firestore Feature Request - Optimistic locking with Firestore
4 participants