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

Use support dir instead of document dir #746

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jmshrv
Copy link

@jmshrv jmshrv commented Jul 30, 2021

Currently, Hive uses getApplicationDocumentsDir as a base directory to store files. On Android, this is fine since the documents dir is hidden from the user, but on iOS and Desktop, the documents dir is in a user-readable location. On iOS, it returns a folder in the user's "On My iPhone" section, and on Desktop it just returns the user's home or documents directory. This could confuse the user and may cause them to lose data by deleting the files. On iOS, the documents directory is also backed up to iCloud by default, which could annoy the user.

I have changed initFlutter to use getApplicationSupportDir instead. This returns a hidden location on all platforms. To handle DBs in old locations, the old location is looped through and all .hive/ files (and the accompanying .lock files) are copied to the new location. The vast majority of apps using Hive shouldn't have to do anything to migrate to this version, but this would probably require a major version bump since some developers may depend on interacting with the DB files directly for some reason.

@jmshrv jmshrv changed the title Support dir Use support dir instead of document dir Jul 30, 2021
@themisir
Copy link
Contributor

First of all thanks for your effort to contribute into Hive project 😊

This change is fine, unless the developer automatically updates packages and tries to read Hive file from document directory manually, since hive is widely used, there might be some use cases that this change will going to break.

Also hive does supports initializing in custom directories (by calling Hive.init rather than Hive.initFlutter) so anyone is free to use which directory they want. But I'm concerned about breaking current users' use case. I noticed that your code contains migration from previous directory, but again, there might be some use case that we're not aware of that might be broken after this update.

Maybe we could keep old initFlutter around and call your implementation as something like initFlutterOnApplicationSupportDir(), or we can add optional parameter to initFlutter({ bool? useSupportDir }) to let users choose which directory to store hive files without forcing them.

@jmshrv
Copy link
Author

jmshrv commented Aug 1, 2021

Using a separate function could be a good idea, it could also include something like bool migrateFromDocuments = false as an argument for any developers that want to move their Hive data to the support directory. The migration code should work for the vast majority of cases (unless there are other files that Hive writes that I'm not aware of), but I agree that there's probably some people that are relying on boxes being in the documents directory.

I won't be able to work on this for a few days but I'll probably make another function for the support dir when I can :)

@jmshrv
Copy link
Author

jmshrv commented Aug 1, 2021

Another note: I didn't realise that iOS hides the documents path by default, I had UISupportsDocumentBrowser enabled. This PR is still relevant for desktop/iOS apps that use the documents library.

@jmshrv
Copy link
Author

jmshrv commented Sep 3, 2021

Just pushed a new commit to make the documents dir the default. If developers want to migrate to the support dir, they should set useSupportDir and migrateFromDocuments to true. I had to make subDir a named argument, since I don't think you can have optional parameters and named parameters in the same function

@Merrit
Copy link

Merrit commented Mar 31, 2022

I am going to advocate for a breaking change, and use getApplicationSupportDir. Anyone doing something non-standard should read the breaking change to account for it if necessary.

I ended up here after wondering why I had Hive files appearing in my home directory (because I disabled the xdg 'Documents' directory). Even without that though, I consider ~/Documents a garbage directory that is sometimes created by rogue apps, which I could and do delete on a whim. For Linux especially, this sort of data should really be in ~/.local/share/<app>.

I can manually initialize with an appropriate directory now that I know what is going on, but that doesn't help every new developer who tries to use Hive.

@themisir
Copy link
Contributor

I am going to advocate for a breaking change, and use getApplicationSupportDir. Anyone doing something non-standard should read the breaking change to account for it if necessary.

I ended up here after wondering why I had Hive files appearing in my home directory (because I disabled the xdg 'Documents' directory). Even without that though, I consider ~/Documents a garbage directory that is sometimes created by rogue apps, which I could and do delete on a whim. For Linux especially, this sort of data should really be in ~/.local/share/<app>.

I can manually initialize with an appropriate directory now that I know what is going on, but that doesn't help every new developer who tries to use Hive.

https://xkcd.com/1172/

Merrit added a commit to Merrit/desktop_widgets that referenced this pull request Mar 31, 2022
It defaults to ~/Documents, until it is fixed we specify
~/.local/share/<app>

isar/hive#746
@Merrit
Copy link

Merrit commented Mar 31, 2022

I am not certain about other platforms, but at least for Linux the current Hive implementation goes against specs:

$XDG_DATA_HOME defines the base directory relative to which user-specific data files should be stored. If $XDG_DATA_HOME is either not set or empty, a default equal to $HOME/.local/share should be used.

XDG_DATA_HOME is correctly where getApplicationSupportDir() on Linux points to.

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

Successfully merging this pull request may close these issues.

3 participants