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

Change parse local database (sembast) path in IOS to LibraryDirectory rather then ApplicationDocumentsDirectory. #791

Closed
3 tasks done
Nidal-Bakir opened this issue Sep 29, 2022 · 7 comments · Fixed by #826
Labels
type:feature New feature or improvement of existing feature

Comments

@Nidal-Bakir
Copy link
Member

Nidal-Bakir commented Sep 29, 2022

New Feature / Enhancement Checklist

Current Limitation

The database (sembast) file "parse.db" can be viewed by the regular user and can be deleted.
that's not good in some cases where the app will store some images, videos, and documents in the app directory, so the user will open the app directory to browse these files and see the "parse.db" database file!!
Or in some cases where the user suddenly decided to delete the app directory the (sembast) "parse.db" file will be deleted and the user will open the app to be surprised that his login and any saved data in the database were deleted.

Feature / Enhancement Description

I suggest using LibraryDirectory as a path for the database file in IOS.
Rather than the ApplicationDocumentsDirectory path.

We can do something like this:

class CoreStoreDirectory {
  Future<String> getDatabaseDirectory() async {
     if (Platform.isIOS) {
        return (await getLibraryDirectory()).path;
    } else {
        return (await getApplicationDocumentsDirectory()).path;
    }
  }

here:

class CoreStoreDirectory {
Future<String> getDatabaseDirectory() async {
return (await getApplicationDocumentsDirectory()).path;
}

Example Use Case

Selection_091

Alternatives / Workarounds

@parse-github-assistant
Copy link

parse-github-assistant bot commented Sep 29, 2022

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mtrezza
Copy link
Member

mtrezza commented Feb 26, 2023

Moving the DB file path makes this is a breaking change. Can we add a simple migration algorithm that looks for the existing file at the old path and move it to the new path if it exists?

@mtrezza mtrezza added the state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message label Feb 26, 2023
@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented Feb 27, 2023

@mtrezza yes, I am well aware of this! I point it out in my PR description here

First solution:

This solution will return the old path (ApplicationDocumentsDirectory) if a database file exists in it. otherwise, it will return the new path (LibraryDirectory).

This is a breaking change for ios platform because the SDK now will look for the "parse.db" in the LibraryDirectory rather than the ApplicationDocumentsDirectory.

We can support the old directory path by doing something like:

  Future<String> getDatabaseDirectory() async {
    if (Platform.isIOS) {
      if(await _isDatabaseFileExistsInOldPath()){
        return (await path_provider.getApplicationDocumentsDirectory()).path;
      }
      return (await path_provider.getLibraryDirectory()).path;
    } else {
      return (await path_provider.getApplicationDocumentsDirectory()).path;
    }
  }

  Future<bool> _isDatabaseFileExistsInOldPath() async {
    final oldIosDatabaseDirPath =
        (await path_provider.getApplicationDocumentsDirectory()).path;
    final oldDatabaseFile =
        path.join('$oldIosDatabaseDirPath/parse', 'parse.db');

    if (await File(oldDatabaseFile).exists()) {
      return true;
    }
    return false;
  }

copy solution:

  Future<String> getDatabaseDirectory() async {
    if (Platform.isIOS) {
      await _migrateDBFileToLibraryDirectory();

      return (await path_provider.getLibraryDirectory()).path;
    }

    return (await path_provider.getApplicationDocumentsDirectory()).path;
  }

  Future<void> _migrateDBFileToLibraryDirectory() async {
    final dbFile = await _getDBFileIfExistsInAppDocDir();

    if (dbFile != null) {
      await _moveDatabaseFileToLibraryDirectory(dbFile);
    }
  }

  Future<File?> _getDBFileIfExistsInAppDocDir() async {
    final appDocDirPath =
        (await path_provider.getApplicationDocumentsDirectory()).path;

    final databaseFilePath = path.join(
      appDocDirPath,
      'parse',
      'parse.db',
    );

    final dbFile = File(databaseFilePath);

    if (await dbFile.exists()) {
      return dbFile;
    }

    return null;
  }

  Future<void> _moveDatabaseFileToLibraryDirectory(
    File databaseFileToMove,
  ) async {
    final libraryDirectoryPath =
        (await path_provider.getLibraryDirectory()).path;

    final libraryDirectoryDatabaseFilePath = path.join(
      libraryDirectoryPath,
      'parse',
      'parse.db',
    );

    await databaseFileToMove.copy(libraryDirectoryDatabaseFilePath);
    await databaseFileToMove.delete();
  }

@Nidal-Bakir
Copy link
Member Author

Currently, I do not have access to any mac device, so I can not test this!
It will be helpful if anyone can test the code.
thanks.

@mtrezza
Copy link
Member

mtrezza commented Feb 27, 2023

I think the "copy" solution makes most sense. It essentially migrates existing clients over time to move their DB file. In the future (some years from now), we can then remove that migration algorithm from the SDK. The migration is something we could bake into the SDK however, as a convenience for developers since this may not be a trivial task for some and a custom solution could break with future changes of the SDK.

Do you think you could write a test for a function that moves the file? Then the OS wouldn't matter. You would just place a dummy file, call the function and see it being moved to a new location. Maybe you could mock the paths if necessary.

@Nidal-Bakir
Copy link
Member Author

@mtrezza I agree with you the "copy" solution is the right thing to do.

Do you think you could write a test for a function that moves the file? Then the OS wouldn't matter. You would just place a dummy file, call the function and see it being moved to a new location. Maybe you could mock the paths if necessary.

working on it!

@Nidal-Bakir
Copy link
Member Author

@mtrezza
Done!
Can you review the PR?

@mtrezza mtrezza removed the state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
2 participants