fix(firestore,windows): honor persistenceEnabled: false - #18662
fix(firestore,windows): honor persistenceEnabled: false#18662SelaseKay wants to merge 2 commits into
Conversation
…e settings The Pigeon getter returns const bool*; assigning it to a bool tested pointer presence rather than the value, so an explicit false read as true. The plugin also never called set_persistence_enabled, so the native default (enabled) always won. Fixes #18659
…rvive terminate Adds a Windows-only e2e that writes, terminates, then reads Source.cache so disk persistence is observable, covering #18659.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
Description
On Windows,
Settings(persistenceEnabled: false)never reached the C++ Firestore client.GetFirestoreFromPigeonassignspigeonApp.settings().persistence_enabled()(aconst bool*) to abool, so the assignment tests pointer presence rather than the value — an explicitfalseis read astrue. The plugin also never callssettings.set_persistence_enabled(...), and the C++ SDK defaults persistence to on, so even a correct dereference would not disable it.This dereferences the pointer and forwards the value. A Windows-only e2e writes a doc, terminates the client, then
get(source: cache): disk cache is the only state that survives terminate. A persistence-on control must hit cache; the disabled instance must miss.Related Issues
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?