Skip auto data dir migration if target dir exists and is non-empty

This commit is contained in:
Dan Stillman 2016-11-30 01:50:49 -05:00
parent cba2c0c58b
commit 1b67ed071e
3 changed files with 46 additions and 11 deletions

View file

@ -428,6 +428,9 @@ Zotero.DataDirectory = {
},
/**
* Determine if current data directory is in a legacy location
*/
canMigrate: function () {
// If (not default location) && (not useDataDir or within legacy location)
var currentDir = this.dir;
@ -489,6 +492,12 @@ Zotero.DataDirectory = {
else {
return false;
}
// Skip automatic migration if there's a non-empty directory at the new location
if ((yield OS.File.exists(newDir)) && !(yield Zotero.File.directoryIsEmpty(newDir))) {
Zotero.debug(`${newDir} exists and is non-empty -- skipping migration`);
return false;
}
}
// Check for an existing pipe from other running versions of Zotero pointing at the same data

View file

@ -486,6 +486,17 @@ Zotero.File = new function(){
});
}
/**
* If directories can be moved at once, instead of recursively creating directories and moving files
*
* Currently this means using /bin/mv, which only works on macOS and Linux
*/
this.canMoveDirectoryAtomic = Zotero.lazy(function () {
var cmd = "/bin/mv";
return !Zotero.isWin && this.pathToFile(cmd).exists();
});
/**
* Move directory (using mv on macOS/Linux, recursively on Windows)
*
@ -497,7 +508,7 @@ Zotero.File = new function(){
this.moveDirectory = Zotero.Promise.coroutine(function* (oldDir, newDir, options = {}) {
var maxDepth = options.maxDepth || 10;
var cmd = "/bin/mv";
var useCmd = !Zotero.isWin && (yield OS.File.exists(cmd));
var useCmd = this.canMoveDirectoryAtomic();
if (!options.allowExistingTarget && (yield OS.File.exists(newDir))) {
throw new Error(newDir + " exists");

View file

@ -62,18 +62,17 @@ describe("Zotero.DataDirectory", function () {
var disableCommandMode = function () {
// Force non-mv mode
var origFunc = OS.File.exists;
stubs.fileExists = sinon.stub(OS.File, "exists", function (path) {
if (path == '/bin/mv') {
return Zotero.Promise.resolve(false);
}
else {
return origFunc(path);
}
});
if (!stubs.canMoveDirectoryAtomic) {
stubs.canMoveDirectoryAtomic = sinon.stub(Zotero.File, "canMoveDirectoryAtomic")
.returns(false);
}
};
var resetCommandMode = function () {
stubs.fileExists.restore();
if (stubs.canMoveDirectoryAtomic) {
stubs.canMoveDirectoryAtomic.restore();
stubs.canMoveDirectoryAtomic = undefined;
}
};
var populateDataDirectory = Zotero.Promise.coroutine(function* (dir, srcDir, automatic = false) {
@ -143,7 +142,7 @@ describe("Zotero.DataDirectory", function () {
describe("#checkForMigration()", function () {
let fileMoveStub;
before(function () {
beforeEach(function () {
disableCommandMode();
});
@ -156,6 +155,22 @@ describe("Zotero.DataDirectory", function () {
tests.push([desc, fn]);
}
it("should skip automatic migration if target directory exists and is non-empty", function* () {
resetCommandMode();
// No automatic migration without atomic directory move
if (!Zotero.File.canMoveDirectoryAtomic()) {
this.skip();
}
yield populateDataDirectory(oldDir);
yield OS.File.remove(oldMigrationMarker);
yield OS.File.makeDir(newDir, { unixMode: 0o755 });
yield Zotero.File.putContentsAsync(OS.Path.join(newDir, 'a'), '');
yield assert.eventually.isFalse(Zotero.DataDirectory.checkForMigration(oldDir, newDir));
});
add("should show error on partial failure", function (automatic) {
return function* () {
yield populateDataDirectory(oldDir, null, automatic);