From 904166c01c585bd27e3f4a5c01ea2722566f3581 Mon Sep 17 00:00:00 2001 From: Michal Horejsek Date: Mon, 22 Feb 2021 10:04:07 +0100 Subject: [PATCH] GODT-247 Cache and update files moved from user's cache to config --- internal/app/base/base.go | 1 + internal/app/base/migration.go | 68 ++++++++++++++++++++++------ internal/locations/locations.go | 39 ++++++++++++++-- internal/locations/locations_test.go | 8 +++- 4 files changed, 96 insertions(+), 20 deletions(-) diff --git a/internal/app/base/base.go b/internal/app/base/base.go index 11a7254d..b0c9cd0e 100644 --- a/internal/app/base/base.go +++ b/internal/app/base/base.go @@ -136,6 +136,7 @@ func New( // nolint[funlen] if err := logging.Init(logsPath); err != nil { return nil, err } + logging.SetLevel("debug") // Proper level is set later in run. crashHandler.AddRecoveryAction(logging.DumpStackTrace(logsPath)) if err := migrateFiles(configName); err != nil { diff --git a/internal/app/base/migration.go b/internal/app/base/migration.go index 7280d594..203dd7a0 100644 --- a/internal/app/base/migration.go +++ b/internal/app/base/migration.go @@ -29,10 +29,12 @@ import ( // migrateFiles migrates files from their old (pre-refactor) locations to their new locations. // We can remove this eventually. // -// | entity | old location | new location | -// |--------|-------------------------------------------|----------------------------------------| -// | prefs | ~/.cache/protonmail//c11/prefs.json | ~/.config/protonmail//prefs.json | -// | c11 | ~/.cache/protonmail//c11 | ~/.cache/protonmail//cache/c11 | +// | entity | old location | new location | +// |-----------|-------------------------------------------|----------------------------------------| +// | prefs | ~/.cache/protonmail//c11/prefs.json | ~/.config/protonmail//prefs.json | +// | c11 1.5.x | ~/.cache/protonmail//c11 | ~/.cache/protonmail//cache/c11 | +// | c11 1.6.x | ~/.cache/protonmail//cache/c11 | ~/.config/protonmail//cache/c11 | +// | updates | ~/.cache/protonmail//updates | ~/.config/protonmail//updates | func migrateFiles(configName string) error { locationsProvider, err := locations.NewDefaultProvider(filepath.Join(constants.VendorName, configName)) if err != nil { @@ -41,43 +43,81 @@ func migrateFiles(configName string) error { locations := locations.New(locationsProvider, configName) userCacheDir := locationsProvider.UserCache() + + if err := migratePrefsFrom15x(locations, userCacheDir); err != nil { + return err + } + if err := migrateCacheFromBoth15xAnd16x(locations, userCacheDir); err != nil { + return err + } + if err := migrateUpdatesFrom16x(locations); err != nil { + return err + } + return nil +} + +func migratePrefsFrom15x(locations *locations.Locations, userCacheDir string) error { newSettingsDir, err := locations.ProvideSettingsPath() if err != nil { return err } - if err := moveIfExists( + return moveIfExists( filepath.Join(userCacheDir, "c11", "prefs.json"), filepath.Join(newSettingsDir, "prefs.json"), - ); err != nil { - return err - } + ) +} - newCacheDir, err := locations.ProvideCachePath() +func migrateCacheFromBoth15xAnd16x(locations *locations.Locations, userCacheDir string) error { + olderCacheDir := userCacheDir + newerCacheDir := locations.GetOldCachePath() + latestCacheDir, err := locations.ProvideCachePath() if err != nil { return err } + // Migration for versions before 1.6.x. if err := moveIfExists( - filepath.Join(userCacheDir, "c11"), - filepath.Join(newCacheDir, "c11"), + filepath.Join(olderCacheDir, "c11"), + filepath.Join(latestCacheDir, "c11"), ); err != nil { return err } - return nil + // Migration for versions 1.6.x. + return moveIfExists( + filepath.Join(newerCacheDir, "c11"), + filepath.Join(latestCacheDir, "c11"), + ) +} + +func migrateUpdatesFrom16x(locations *locations.Locations) error { + oldUpdatesPath := locations.GetOldUpdatesPath() + // Do not use ProvideUpdatesPath, that creates dir right away. + newUpdatesPath := locations.GetUpdatesPath() + + return moveIfExists(oldUpdatesPath, newUpdatesPath) } func moveIfExists(source, destination string) error { + l := logrus.WithField("source", source).WithField("destination", destination) + if _, err := os.Stat(source); os.IsNotExist(err) { - logrus.WithField("source", source).WithField("destination", destination).Debug("No need to migrate file") + l.Debug("No need to migrate file, source doesn't exist") return nil } if _, err := os.Stat(destination); !os.IsNotExist(err) { - logrus.WithField("source", source).WithField("destination", destination).Debug("No need to migrate file") + // Once migrated, files should not stay in source anymore. Therefore + // if some files are still in source location but target already exist, + // it's suspicious. Could happen by installing new version, then the + // old one because of some reason, and then the new one again. + // Good to see as warning because it could be a reason why Bridge is + // behaving weirdly, like wrong configuration, or db re-sync and so on. + l.Warn("No need to migrate file, target already exists") return nil } + l.Info("Migrating files") return os.Rename(source, destination) } diff --git a/internal/locations/locations.go b/internal/locations/locations.go index bc851a2d..317ecf3b 100644 --- a/internal/locations/locations.go +++ b/internal/locations/locations.go @@ -32,8 +32,8 @@ import ( // On linux: // - settings: ~/.config/protonmail/ // - logs: ~/.cache/protonmail//logs -// - cache: ~/.cache/protonmail//cache -// - updates: ~/.cache/protonmail//updates +// - cache: ~/.config/protonmail//cache +// - updates: ~/.config/protonmail//updates // - lockfile: ~/.cache/protonmail//.lock type Locations struct { userConfig, userCache string @@ -129,7 +129,7 @@ func (l *Locations) ProvideLogsPath() (string, error) { return l.getLogsPath(), nil } -// ProvideCachePath returns a location for user cache dirs (e.g. ~/.cache///cache). +// ProvideCachePath returns a location for user cache dirs (e.g. ~/.config///cache). // It creates it if it doesn't already exist. func (l *Locations) ProvideCachePath() (string, error) { if err := os.MkdirAll(l.getCachePath(), 0700); err != nil { @@ -139,6 +139,11 @@ func (l *Locations) ProvideCachePath() (string, error) { return l.getCachePath(), nil } +// GetOldCachePath returns a former location for user cache dirs used for migration scripts only. +func (l *Locations) GetOldCachePath() string { + return filepath.Join(l.userCache, "cache") +} + // ProvideUpdatesPath returns a location for update files (e.g. ~/.cache///updates). // It creates it if it doesn't already exist. func (l *Locations) ProvideUpdatesPath() (string, error) { @@ -149,6 +154,16 @@ func (l *Locations) ProvideUpdatesPath() (string, error) { return l.getUpdatesPath(), nil } +// GetUpdatesPath returns a new location for update files used for migration scripts only. +func (l *Locations) GetUpdatesPath() string { + return l.getUpdatesPath() +} + +// GetOldUpdatesPath returns a former location for update files used for migration scripts only. +func (l *Locations) GetOldUpdatesPath() string { + return filepath.Join(l.userCache, "updates") +} + func (l *Locations) getSettingsPath() string { return l.userConfig } @@ -158,11 +173,25 @@ func (l *Locations) getLogsPath() string { } func (l *Locations) getCachePath() string { - return filepath.Join(l.userCache, "cache") + // Bridge cache is not a typical cache which can be deleted with only + // downside that the app has to download everything again. + // Cache for bridge is database with IMAP UIDs and UIDVALIDITY, and also + // other IMAP setup. Deleting such data leads to either re-sync of client, + // or mix of headers and bodies. Both is caused because of need of re-sync + // between Bridge and API which will happen in different order than before. + // In the first case, UIDVALIDITY is also changed and causes the better + // outcome to "just" re-sync everything; in the later, UIDVALIDITY stays + // the same, causing the client to not re-sync but UIDs in the client does + // not match UIDs in Bridge. + // Because users might use tools to regularly clear caches, Bridge cache + // cannot be located in a standard cache folder. + return filepath.Join(l.userConfig, "cache") } func (l *Locations) getUpdatesPath() string { - return filepath.Join(l.userCache, "updates") + // Users might use tools to regularly clear caches, which would mean always + // removing updates, therefore Bridge updates have to be somewhere else. + return filepath.Join(l.userConfig, "updates") } // Clear removes everything except the lock and update files. diff --git a/internal/locations/locations_test.go b/internal/locations/locations_test.go index 1340794a..7aaa6947 100644 --- a/internal/locations/locations_test.go +++ b/internal/locations/locations_test.go @@ -45,7 +45,8 @@ func TestClearRemovesEverythingExceptLockAndUpdateFiles(t *testing.T) { assert.NoError(t, l.Clear()) assert.FileExists(t, l.GetLockFile()) - assert.NoDirExists(t, l.getSettingsPath()) + assert.DirExists(t, l.getSettingsPath()) + assert.NoFileExists(t, filepath.Join(l.getSettingsPath(), "prefs.json")) assert.NoDirExists(t, l.getLogsPath()) assert.NoDirExists(t, l.getCachePath()) assert.DirExists(t, l.getUpdatesPath()) @@ -58,6 +59,7 @@ func TestClearUpdateFiles(t *testing.T) { assert.FileExists(t, l.GetLockFile()) assert.DirExists(t, l.getSettingsPath()) + assert.FileExists(t, filepath.Join(l.getSettingsPath(), "prefs.json")) assert.DirExists(t, l.getLogsPath()) assert.DirExists(t, l.getCachePath()) assert.NoDirExists(t, l.getUpdatesPath()) @@ -75,6 +77,7 @@ func TestCleanLeavesStandardLocationsUntouched(t *testing.T) { assert.FileExists(t, l.GetLockFile()) assert.DirExists(t, l.getSettingsPath()) + assert.FileExists(t, filepath.Join(l.getSettingsPath(), "prefs.json")) assert.DirExists(t, l.getLogsPath()) assert.FileExists(t, filepath.Join(l.getLogsPath(), "log1.txt")) assert.FileExists(t, filepath.Join(l.getLogsPath(), "log2.txt")) @@ -138,6 +141,9 @@ func newTestLocations(t *testing.T) *Locations { require.NoError(t, err) require.DirExists(t, settings) + createFilesInDir(t, settings, "prefs.json") + require.FileExists(t, filepath.Join(settings, "prefs.json")) + logs, err := l.ProvideLogsPath() require.NoError(t, err) require.DirExists(t, logs)