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

FR: Add precondition support to 'set' methods #1286

Open
eranl opened this issue May 5, 2023 · 4 comments
Open

FR: Add precondition support to 'set' methods #1286

eranl opened this issue May 5, 2023 · 4 comments
Assignees
Labels
api: firestore Issues related to the googleapis/java-firestore API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@eranl
Copy link
Contributor

eranl commented May 5, 2023

The 'update' methods of UpdateBuilder and DocumentReference support a Precondition parameter, while 'set' methods do not, even though they use the same underlying server API, which supports preconditions.

I suggest adding precondition support to 'set' methods as well, as that would be useful for cases where you want to replace a document, unless it has been modified elsewhere since you loaded it.

I have implemented a POC of this feature, and tested that it works. I would be happy to submit a PR.

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/java-firestore API. label May 5, 2023
@dconeybe dconeybe self-assigned this May 5, 2023
@dconeybe
Copy link
Contributor

dconeybe commented May 5, 2023

Thanks for the suggestion. It makes sense to me. We have a fairly rigorous process to go through when making changes to the API surface so, despite the simplicity of the requested change, it would take some time to get it through. We'd also want to add this feature to https://github.com/googleapis/nodejs-firestore, for consistency. Finally, we have competing priorities that would bump this feature low on our todo list. But I will bring it up with the team to discuss.

Feel free to open a PR; however, it's not necessary at this point. I assume you're just suggesting adding "precondition" to SetOptions. Is that what you were thinking?

@eranl
Copy link
Contributor Author

eranl commented May 6, 2023

Thanks for the response. I was actually thinking of a separate precondition parameter, similarly to the 'update' methods, but adding it to SetOptions makes sense too.

I would appreciate updates here as you go through the process.

@dconeybe
Copy link
Contributor

dconeybe commented May 9, 2023

After discussing with the team, we remembered that this same feature was requested last year: googleapis/nodejs-firestore#1697. Unfortunately, we still do not have the time to prioritize it now but do think that it is a reasonable request. We will keep you posted if/when we start to work on this.

@dconeybe dconeybe changed the title Add precondition support to 'set' methods FR: Add precondition support to 'set' methods May 9, 2023
@dconeybe dconeybe added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label May 9, 2023
@eranl
Copy link
Contributor Author

eranl commented May 9, 2023

Thanks for the update! Once you're ready for implementation, let me know if I can help with coding or testing.

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. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants