From ef3e09858620cdcb9914c16cb3d48b10de32f2a2 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 22 Nov 2016 01:40:39 -0500 Subject: [PATCH] Migrate data directory automatically on macOS and Linux If data directory is within the profile directory and we can move the subdirectories instantaneously with /bin/mv, just do it silently at startup. --- .../preferences/preferences_advanced.js | 4 +- chrome/content/zotero/xpcom/zotero.js | 57 ++++-- chrome/content/zotero/zoteroPane.js | 5 +- chrome/locale/en-US/zotero/zotero.properties | 9 +- test/tests/zoteroTest.js | 175 +++++++++++------- 5 files changed, 164 insertions(+), 86 deletions(-) diff --git a/chrome/content/zotero/preferences/preferences_advanced.js b/chrome/content/zotero/preferences/preferences_advanced.js index d670d8f76..2afa4d139 100644 --- a/chrome/content/zotero/preferences/preferences_advanced.js +++ b/chrome/content/zotero/preferences/preferences_advanced.js @@ -65,9 +65,7 @@ Zotero_Preferences.Advanced = { ); if (index == 0) { - yield Zotero.File.putContentsAsync( - OS.Path.join(currentDir, Zotero.DATA_DIR_MIGRATION_MARKER), currentDir - ); + yield Zotero.markDataDirectoryForMigration(currentDir); Zotero.Utilities.Internal.quitZotero(true); } }), diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index 574f3ff7b..49975001c 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -318,6 +318,12 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); } if (!Zotero.isConnector) { + // On macOS and Linux, migrate the data directory automatically + if (this.canMigrateDataDirectory(dataDir.path) + // Should match check in Zotero.File.moveDirectory() + && !Zotero.isWin && (yield OS.File.exists("/bin/mv"))) { + yield this.markDataDirectoryForMigration(dataDir.path, true); + } yield Zotero.checkForDataDirectoryMigration(dataDir.path, this.getDefaultDataDir()); if (this.skipLoading) { return; @@ -975,13 +981,14 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); let dbFile = OS.Path.join(prefVal, this.getDatabaseFilename()); if (Zotero.File.pathToFile(migrationMarker).exists() && !(Zotero.File.pathToFile(dbFile).exists())) { - let fileStr = Zotero.File.getContents(migrationMarker); + let contents = Zotero.File.getContents(migrationMarker); try { - file = Zotero.File.pathToFile(fileStr); + let { sourceDir } = JSON.parse(contents); + file = Zotero.File.pathToFile(sourceDir); } catch (e) { Zotero.logError(e); - Zotero.debug(`Invalid path '${fileStr}' in marker file`, 1); + Zotero.debug(`Invalid marker file:\n\n${contents}`, 1); e = { name: "NS_ERROR_FILE_NOT_FOUND" }; throw e; } @@ -1435,6 +1442,17 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); this.DATA_DIR_MIGRATION_MARKER = 'migrate-dir'; + this.markDataDirectoryForMigration = function (dir, automatic = false) { + return Zotero.File.putContentsAsync( + OS.Path.join(dir, this.DATA_DIR_MIGRATION_MARKER), + JSON.stringify({ + sourceDir: dir, + automatic + }) + ); + }; + + /** * Migrate data directory if necessary and show any errors * @@ -1443,7 +1461,7 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); * the default data directory */ this.checkForDataDirectoryMigration = Zotero.Promise.coroutine(function* (dataDir, newDir) { - let migrationMarker = OS.Path.join(dataDir, Zotero.DATA_DIR_MIGRATION_MARKER); + let migrationMarker = OS.Path.join(dataDir, this.DATA_DIR_MIGRATION_MARKER); try { var exists = yield OS.File.exists(migrationMarker) } @@ -1457,6 +1475,20 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); let oldDir; let partial = false; + // Check whether this is an automatic or manual migration + let contents; + try { + contents = yield Zotero.File.getContentsAsync(migrationMarker); + var { sourceDir, automatic } = JSON.parse(contents); + } + catch (e) { + if (contents !== undefined) { + Zotero.debug(contents, 1); + } + Zotero.logError(e); + return false; + } + // Not set to the default directory, so use current as old directory if (dataDir != newDir) { oldDir = dataDir; @@ -1464,13 +1496,7 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); // Unfinished migration -- already using new directory, so get path to previous // directory from the migration marker else { - try { - oldDir = yield Zotero.File.getContentsAsync(migrationMarker); - } - catch (e) { - Zotero.logError(e); - return false; - } + oldDir = sourceDir; partial = true; } @@ -1480,6 +1506,7 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); }.bind(this); let errors; + let mode = automatic ? 'automatic' : 'manual'; try { this.showZoteroPaneProgressMeter(Zotero.getString("dataDir.migration.inProgress")); errors = yield Zotero.migrateDataDirectory(oldDir, newDir, partial, progressHandler); @@ -1494,11 +1521,11 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING); let index = ps.confirmEx(null, Zotero.getString('dataDir.migration.failure.title'), - Zotero.getString('dataDir.migration.failure.full.text1') + Zotero.getString(`dataDir.migration.failure.full.${mode}.text1`, ZOTERO_CONFIG.CLIENT_NAME) + "\n\n" + e + "\n\n" - + Zotero.getString('dataDir.migration.failure.full.text2', Zotero.appName) + + Zotero.getString(`dataDir.migration.failure.full.${mode}.text2`, Zotero.appName) + "\n\n" + Zotero.getString('dataDir.migration.failure.full.current', oldDir) + "\n\n" @@ -1532,7 +1559,7 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); + (ps.BUTTON_POS_2) * (ps.BUTTON_TITLE_IS_STRING); let index = ps.confirmEx(null, Zotero.getString('dataDir.migration.failure.title'), - Zotero.getString('dataDir.migration.failure.partial.text', + Zotero.getString(`dataDir.migration.failure.partial.${mode}.text`, [ZOTERO_CONFIG.CLIENT_NAME, Zotero.appName]) + "\n\n" + Zotero.getString('dataDir.migration.failure.partial.old', oldDir) @@ -1714,6 +1741,8 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); let newMigrationMarker = OS.Path.join(newDir, Zotero.DATA_DIR_MIGRATION_MARKER); Zotero.debug("Removing " + newMigrationMarker); yield OS.File.remove(newMigrationMarker); + + Zotero.debug("Migration successful"); } catch (e) { addError(e); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 24156703b..d09a8f0d0 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -461,8 +461,9 @@ var ZoteroPane = new function() function isShowing() { var zoteroPane = document.getElementById('zotero-pane-stack'); - return zoteroPane.getAttribute('hidden') != 'true' && - zoteroPane.getAttribute('collapsed') != 'true'; + return zoteroPane + && zoteroPane.getAttribute('hidden') != 'true' + && zoteroPane.getAttribute('collapsed') != 'true'; } function isFullScreen() { diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index 98759e574..abd2d75d7 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -136,12 +136,15 @@ dataDir.incompatibleDbVersion.title = Incompatible Database Version dataDir.incompatibleDbVersion.text = The currently selected data directory is not compatible with Zotero Standalone, which can share a database only with Zotero for Firefox 2.1b3 or later.\n\nUpgrade to the latest version of Zotero for Firefox first or select a different data directory for use with Zotero Standalone. dataDir.migration.inProgress = Migration in progress — do not interrupt… dataDir.migration.failure.title = Data Directory Migration Error -dataDir.migration.failure.partial.text = Some files in your old %1$S data directory could not be transferred to the new location. Close any open attachment files and try again. You can also quit %2$S and attempt to move the remaining files manually. +dataDir.migration.failure.partial.automatic.text = %1$S attempted to move your data directory to a new default location, but some files could not be transferred. Close any open attachment files and try again. You can also quit %2$S and attempt to move the remaining files manually. +dataDir.migration.failure.partial.manual.text = Some files in your %1$S data directory could not be transferred to the new location. Close any open attachment files and try again. You can also quit %2$S and attempt to move the remaining files manually. dataDir.migration.failure.partial.old = Old directory: %S dataDir.migration.failure.partial.new = New directory: %S dataDir.migration.failure.partial.showDirectoriesAndQuit = Show Directories and Quit -dataDir.migration.failure.full.text1 = Your data directory could not be migrated. -dataDir.migration.failure.full.text2 = It is recommended that you close %S and manually move your data directory to the new default location. +dataDir.migration.failure.full.automatic.text1 = %S attempted to move your data directory to a new default location, but the migration could not be completed. +dataDir.migration.failure.full.automatic.text2 = It is recommended that you close %S and move your data directory manually. +dataDir.migration.failure.full.manual.text1 = Your %S data directory could not be migrated. +dataDir.migration.failure.full.manual.text2 = It is recommended that you close %S and manually move your data directory to the new default location. dataDir.migration.failure.full.current = Current location: %S dataDir.migration.failure.full.recommended = Recommended location: %S dataDir.migration.failure.full.showCurrentDirectoryAndQuit = Show Current Directory and Quit diff --git a/test/tests/zoteroTest.js b/test/tests/zoteroTest.js index 8a8aad504..1b57f684c 100644 --- a/test/tests/zoteroTest.js +++ b/test/tests/zoteroTest.js @@ -68,7 +68,7 @@ describe("Zotero Core Functions", function () { stub2.restore(); }; - var populateDataDirectory = Zotero.Promise.coroutine(function* (dir, srcDir) { + var populateDataDirectory = Zotero.Promise.coroutine(function* (dir, srcDir, automatic = false) { yield OS.File.makeDir(dir, { unixMode: 0o755 }); let storageDir = OS.Path.join(dir, 'storage'); let storageDir1 = OS.Path.join(storageDir, 'AAAAAAAA'); @@ -92,7 +92,13 @@ describe("Zotero Core Functions", function () { yield Zotero.File.putContentsAsync(OS.Path.join(translatorsDir, translatorName1), str4); yield Zotero.File.putContentsAsync(OS.Path.join(translatorsDir, translatorName2), str5); // Migration marker - yield Zotero.File.putContentsAsync(migrationMarker, srcDir || dir); + yield Zotero.File.putContentsAsync( + migrationMarker, + JSON.stringify({ + sourceDir: srcDir || dir, + automatic + }) + ); }); var checkMigration = Zotero.Promise.coroutine(function* (options = {}) { @@ -137,74 +143,115 @@ describe("Zotero Core Functions", function () { resetCommandMode(); }); - it("should show error on partial failure", function* () { - Zotero.Debug.init(true); - yield populateDataDirectory(oldDir); - - let origFunc = OS.File.move; - let stub3 = sinon.stub(OS.File, "move", function () { - if (OS.Path.basename(arguments[0]) == storageFile1) { - return Zotero.Promise.reject(new Error("Error")); - } - else { - let args; - if (Zotero.platformMajorVersion < 46) { - args = Array.from(arguments); + var tests = []; + function add(desc, fn) { + tests.push([desc, fn]); + } + + add("should show error on partial failure", function (automatic) { + return function* () { + Zotero.Debug.init(true); + yield populateDataDirectory(oldDir, null, automatic); + + let origFunc = OS.File.move; + let stub3 = sinon.stub(OS.File, "move", function () { + if (OS.Path.basename(arguments[0]) == storageFile1) { + return Zotero.Promise.reject(new Error("Error")); } else { - args = arguments; + let args; + if (Zotero.platformMajorVersion < 46) { + args = Array.from(arguments); + } + else { + args = arguments; + } + return origFunc(...args); } - return origFunc(...args); - } - }); - let stub4 = sinon.stub(Zotero.File, "reveal").returns(Zotero.Promise.resolve()); - let stub5 = sinon.stub(Zotero.Utilities.Internal, "quitZotero"); - - var promise2; - // Click "Try Again" the first time, and then "Show Directories and Quit Zotero" - var promise = waitForDialog(function () { - promise2 = waitForDialog(null, 'extra1'); - }); - yield Zotero.checkForDataDirectoryMigration(oldDir, newDir); - yield promise; - yield promise2; - - assert.isTrue(stub4.calledTwice); - assert.isTrue(stub4.getCall(0).calledWith(oldStorageDir)); - assert.isTrue(stub4.getCall(1).calledWith(newDBFile)); - assert.isTrue(stub5.called); - - stub3.restore(); - stub4.restore(); - stub5.restore(); + }); + let stub4 = sinon.stub(Zotero.File, "reveal").returns(Zotero.Promise.resolve()); + let stub5 = sinon.stub(Zotero.Utilities.Internal, "quitZotero"); + + var promise2; + // Click "Try Again" the first time, and then "Show Directories and Quit Zotero" + var promise = waitForDialog(function (dialog) { + promise2 = waitForDialog(null, 'extra1'); + + // Make sure we're displaying the right message for this mode (automatic or manual) + Components.utils.import("resource://zotero/config.js"); + assert.include( + dialog.document.documentElement.textContent, + Zotero.getString( + `dataDir.migration.failure.partial.${automatic ? 'automatic' : 'manual'}.text`, + [ZOTERO_CONFIG.CLIENT_NAME, Zotero.appName] + ) + ); + }); + yield Zotero.checkForDataDirectoryMigration(oldDir, newDir); + yield promise; + yield promise2; + + assert.isTrue(stub4.calledTwice); + assert.isTrue(stub4.getCall(0).calledWith(oldStorageDir)); + assert.isTrue(stub4.getCall(1).calledWith(newDBFile)); + assert.isTrue(stub5.called); + + stub3.restore(); + stub4.restore(); + stub5.restore(); + }; }); - it("should show error on full failure", function* () { - yield populateDataDirectory(oldDir); - - let origFunc = OS.File.move; - let stub3 = sinon.stub(OS.File, "move", function () { - if (OS.Path.basename(arguments[0]) == dbFilename) { - return Zotero.Promise.reject(new Error("Error")); - } - else { - return origFunc(...arguments); - } + add("should show error on full failure", function (automatic) { + return function* () { + yield populateDataDirectory(oldDir, null, automatic); + + let origFunc = OS.File.move; + let stub3 = sinon.stub(OS.File, "move", function () { + if (OS.Path.basename(arguments[0]) == dbFilename) { + return Zotero.Promise.reject(new Error("Error")); + } + else { + return origFunc(...arguments); + } + }); + let stub4 = sinon.stub(Zotero.File, "reveal").returns(Zotero.Promise.resolve()); + let stub5 = sinon.stub(Zotero.Utilities.Internal, "quitZotero"); + + var promise = waitForDialog(function (dialog) { + // Make sure we're displaying the right message for this mode (automatic or manual) + Components.utils.import("resource://zotero/config.js"); + assert.include( + dialog.document.documentElement.textContent, + Zotero.getString( + `dataDir.migration.failure.full.${automatic ? 'automatic' : 'manual'}.text1`, + ZOTERO_CONFIG.CLIENT_NAME + ) + ); + }); + yield Zotero.checkForDataDirectoryMigration(oldDir, newDir); + yield promise; + + assert.isTrue(stub4.calledOnce); + assert.isTrue(stub4.calledWith(oldDir)); + assert.isTrue(stub5.called); + + stub3.restore(); + stub4.restore(); + stub5.restore(); + }; + }); + + describe("automatic mode", function () { + tests.forEach(arr => { + it(arr[0], arr[1](true)); + }); + }); + + describe("manual mode", function () { + tests.forEach(arr => { + it(arr[0], arr[1](false)); }); - let stub4 = sinon.stub(Zotero.File, "reveal").returns(Zotero.Promise.resolve()); - let stub5 = sinon.stub(Zotero.Utilities.Internal, "quitZotero"); - - var promise = waitForDialog(); - yield Zotero.checkForDataDirectoryMigration(oldDir, newDir); - yield promise; - - assert.isTrue(stub4.calledOnce); - assert.isTrue(stub4.calledWith(oldDir)); - assert.isTrue(stub5.called); - - stub3.restore(); - stub4.restore(); - stub5.restore(); }); it("should remove marker if old directory doesn't exist", function* () {