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

Backup and restore settings, closes #839 #1244

Merged
merged 66 commits into from Apr 16, 2021

Conversation

harshad1
Copy link
Collaborator

@harshad1 harshad1 commented Feb 17, 2021

This PR addresses #839. I have liberally borrowed from OpenLauncher/BackupHelper

This PR now does not export a zip of the shared preference XML files (as openlauncher does). Instead, the preferences are exported as a JSON.

@harshad1
Copy link
Collaborator Author

@gsantner This works, but we need to review strings etc

@gsantner
Copy link
Owner

gsantner commented Feb 18, 2021

what do you think of adding SearchOrCustomDialog after file selection?

Data / Data +Settings / Settings

and for data, also select & add second folder recursive to zip.

I made impression users are interested in data backup too, even more than settings backup.

@harshad1
Copy link
Collaborator Author

harshad1 commented Feb 18, 2021 via email

@harshad1 harshad1 changed the title Backup and restore settings WIP: Backup and restore settings Feb 19, 2021
@harshad1 harshad1 changed the title WIP: Backup and restore settings Backup and restore settings Feb 19, 2021
@gsantner
Copy link
Owner

gsantner commented Feb 24, 2021

btw, the password is not contained in the export right (->keystore)? If so, we should strip that one

--->

ok just tried it, yes al credentials are exported. hmm not really good I would say.

Screenshot_20210225-011500_Screenshot_Tile_(NoRoot)

@harshad1
Copy link
Collaborator Author

btw, the password is not contained in the export right (->keystore)? If so, we should strip that one

Ah good catch. I will address this

@gsantner
Copy link
Owner

gsantner commented Feb 25, 2021

basically we need a file::readLines().grep(!.encryption.)

@opensource21
Is it enough to decrypt with these two values? Just recall we (try to) store something in android's key storage, outside of app files/prefs.

@gsantner gsantner added this to the v2.6 milestone Feb 25, 2021
@gsantner gsantner linked an issue Feb 25, 2021 that may be closed by this pull request
@gsantner gsantner changed the title Backup and restore settings Backup and restore settings, closes #839 Feb 25, 2021
@harshad1
Copy link
Collaborator Author

@gsantner Actually I am not sure why this is happening

As far as I can tell, the password should be removed from plaintext as soon as it is written. i.e. it should never be in plaintext

@harshad1
Copy link
Collaborator Author

IMO, a much cleaner solution would be to use a regular dialog to set the password. That way it is never stored in plaintext and we don't have to do this delete after storing in password manager thing.

@harshad1
Copy link
Collaborator Author

@gsantner @opensource21 I think we are good to go :)

@gsantner gsantner self-requested a review March 28, 2021 23:21
Copy link
Owner

@gsantner gsantner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry that I have to block this still, but it does not work. Tried it.

  • Set 22 as value just above the backup settings. exited app to make sure it saved the 22.
  • then made backup
  • Changed to 10, exited again to make sure it's saved and not pending.
  • Then restored the backup. Checked settings and it's still at 10 - it should be 22 now.
  • I also exited / restarted the app multiple times after loading the backup - the value is still at 10, so backup-restore didn't do anything.

https://user-images.githubusercontent.com/6735650/112771640-1da1c080-902d-11eb-9e0e-28bf076ee997.mp4

a.mp4

Screenshot_20210329-011009 Screenshot_20210329-011137

This is probably the price of converting back-and-forth formats, potential loss of information.

@harshad1
Copy link
Collaborator Author

For some reason, the .apply() does not appear to consistently apply the results before exit() after the recent changes. I have reverted to .commit()

@gsantner could you check importing now?

@harshad1
Copy link
Collaborator Author

harshad1 commented Apr 1, 2021

This is probably the price of converting back-and-forth formats, potential loss of information.

To be clear, the issue was not data loss (all data appears to be preserved with 100% fidelity), but that .apply() would not finish writing to disk before sys.exit()

This is fixed now

@harshad1 harshad1 mentioned this pull request Apr 1, 2021
@harshad1
Copy link
Collaborator Author

harshad1 commented Apr 2, 2021

It appears we may not even need the system exit now that we are using system APIs to restore the settings. I have removed the exit, and everything seems to work great. Could you test and confirm?

@harshad1 harshad1 changed the title Backup and restore settings, closes #839 WIP: Backup and restore settings, closes #839 Apr 7, 2021
@harshad1
Copy link
Collaborator Author

harshad1 commented Apr 7, 2021

After much testing, I have decided that I agree with @gsantner and that we should store the file type as strings rather than indices. I will be making these changes later today.

@gsantner
Copy link
Owner

gsantner commented Apr 7, 2021

OK as you prefer, if you think indices is good enough, also OK if it works

case R.string.action_format_keyvalue:
case R.string.action_format_todotxt:
case R.string.action_format_plaintext:
case R.string.action_format_markdown: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched all type formats to strings

@harshad1 harshad1 changed the title WIP: Backup and restore settings, closes #839 Backup and restore settings, closes #839 Apr 8, 2021
@gsantner
Copy link
Owner

Note, this is how the format is saved now in the end:

grafik

@gsantner gsantner merged commit 841c83b into gsantner:master Apr 16, 2021
@harshad1 harshad1 deleted the backup_and_restore branch April 11, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App Settings backup & restore
3 participants