Migrate LevelDB data out of ~/Documents #4609
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The spec tests aren't actually leaking, but they accumulate enough garbage without clearing the autorelease pool that Instruments complains.
ARC does not release temporaries when an Objective-C exception is thrown. Use C++ equivalents directly to avoid leaks as detected by Instruments.
Any previous value must be freed before the next one is assigned.
The single by-value implementation wasn't preventing the compiler generated standard versions from being generated which was causing leaks.
It's possible for the Executor to immediately execute the given block, at which point the block might observe the async queue in a not-shutting-down state.
After shutdown, the AsyncQueue silently drops Enqueue requests which prevents these blocks from being deleted.
This avoids races between Shutdown and Complete and prevents ownership ambiguity.
Use util::LogIsLoggable(util::kLogLevelDebug) directly.
* Make methods const where possible
* Make key() and value() return const std::string&, allowing callers
to create their own const references
iOS: from Documents/firestore to Library/Application Support/firestore. macOS: .firestore to Library/Application Support/firestore.
wilhuff
added a commit
that referenced
this pull request
Jan 9, 2020
This change does not yet change all the classes that use the filesystem to take Filesystem as a dependency. Those changes will come in follow-up PRs. This change is motivated by feedback in #4609. There I had created a complicated interface to LevelDbOpener just so that different migration scenarios could be tested, when in reality, I just needed to be able to plumb through different values for the result of AppDataDir and related functions.
wilhuff
added a commit
that referenced
this pull request
Jan 9, 2020
This change does not yet change all the classes that use the filesystem to take Filesystem as a dependency. Those changes will come in follow-up PRs. This change is motivated by feedback in #4609. There I had created a complicated interface to LevelDbOpener just so that different migration scenarios could be tested, when in reality, I just needed to be able to plumb through different values for the result of AppDataDir and related functions.
wilhuff
added a commit
that referenced
this pull request
Jan 10, 2020
This change does not yet change all the classes that use the filesystem to take Filesystem as a dependency. Those changes will come in follow-up PRs. This change is motivated by feedback in #4609. There I had created a complicated interface to LevelDbOpener just so that different migration scenarios could be tested, when in reality, I just needed to be able to plumb through different values for the result of AppDataDir and related functions.
wilhuff
force-pushed
the
wilhuff/02-leveldb-migration
branch
from
2432d88 to
6783f11
Compare
mikelehen
approved these changes
Jan 10, 2020
Firestore/core/src/firebase/firestore/local/leveldb_persistence.cc
Outdated
Show resolved
Hide resolved
Firestore/core/test/firebase/firestore/local/leveldb_opener_test.cc
Outdated
Show resolved
Hide resolved
mikelehen
approved these changes
Jan 12, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves iOS and macOS user data from
NSDocumentDirectorytoNSApplicationSupportDirectory, adding a pre-open migration step that:Note that other platforms don't need a migration:
NSCachesDirectorySee individual commits for details. Most notably this adds some additional filesystem manipulation routines to our (sadly) growing library.
Fixes #843.