Skip to content

Commit

Permalink
BREAKING: Error if destination wrong type for ensureLink/Symlink
Browse files Browse the repository at this point in the history 
Fixes #786
  • Loading branch information
@RyanZim
RyanZim committed Jul 29, 2020
1 parent 6bffcd8 commit eae5c33
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 108 deletions.
35 changes: 1 addition & 34 deletions lib/ensure/__tests__/link.test.js
View file
Expand Up @@ -34,7 +34,7 @@ describe('fse-ensure-link', () => {
[['../foo.txt', './link.txt'], 'file-error', 'file-error'],
[['../dir-foo/foo.txt', './link.txt'], 'file-error', 'file-error'],
// error is thrown if destination path exists
[['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-dest-exists'],
[['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-error'],
[[path.resolve(path.join(TEST_DIR, './foo.txt')), './link.txt'], 'file-success', 'file-success'],
[[path.resolve(path.join(TEST_DIR, './dir-foo/foo.txt')), './link.txt'], 'file-success', 'file-success'],
[[path.resolve(path.join(TEST_DIR, './missing.txt')), './link.txt'], 'file-error', 'file-error'],
Expand Down Expand Up @@ -107,23 +107,6 @@ describe('fse-ensure-link', () => {
})
}

function fileDestExists (args, fn) {
const srcpath = args[0]
const dstpath = args[1]

it(`should do nothing using src ${srcpath} and dst ${dstpath}`, done => {
const destinationContentBefore = fs.readFileSync(dstpath, 'utf8')
const callback = err => {
if (err) return done(err)
const destinationContentAfter = fs.readFileSync(dstpath, 'utf8')
assert.strictEqual(destinationContentBefore, destinationContentAfter)
return done()
}
args.push(callback)
return fn(...args)
})
}

function fileSuccessSync (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
Expand Down Expand Up @@ -161,18 +144,6 @@ describe('fse-ensure-link', () => {
})
}

function fileDestExistsSync (args, fn) {
const srcpath = args[0]
const dstpath = args[1]

it(`should do nothing using src ${srcpath} and dst ${dstpath}`, () => {
const destinationContentBefore = fs.readFileSync(dstpath, 'utf8')
fn(...args)
const destinationContentAfter = fs.readFileSync(dstpath, 'utf8')
assert.strictEqual(destinationContentBefore, destinationContentAfter)
})
}

describe('fs.link()', () => {
const fn = fs.link
tests.forEach(test => {
Expand All @@ -181,7 +152,6 @@ describe('fse-ensure-link', () => {
// const newBehavior = test[2]
if (nativeBehavior === 'file-success') fileSuccess(args, fn)
if (nativeBehavior === 'file-error') fileError(args, fn)
if (nativeBehavior === 'file-dest-exists') fileDestExists(args, fn)
})
})

Expand All @@ -193,7 +163,6 @@ describe('fse-ensure-link', () => {
const newBehavior = test[2]
if (newBehavior === 'file-success') fileSuccess(args, fn)
if (newBehavior === 'file-error') fileError(args, fn)
if (newBehavior === 'file-dest-exists') fileDestExists(args, fn)
})
})

Expand Down Expand Up @@ -229,7 +198,6 @@ describe('fse-ensure-link', () => {
// const newBehavior = test[2]
if (nativeBehavior === 'file-success') fileSuccessSync(args, fn)
if (nativeBehavior === 'file-error') fileErrorSync(args, fn)
if (nativeBehavior === 'file-dest-exists') fileDestExists(args, fn)
})
})

Expand All @@ -241,7 +209,6 @@ describe('fse-ensure-link', () => {
const newBehavior = test[2]
if (newBehavior === 'file-success') fileSuccessSync(args, fn)
if (newBehavior === 'file-error') fileErrorSync(args, fn)
if (newBehavior === 'file-dest-exists') fileDestExistsSync(args, fn)
})
})
})
66 changes: 2 additions & 64 deletions lib/ensure/__tests__/symlink.test.js
View file
Expand Up @@ -35,7 +35,7 @@ describe('fse-ensure-symlink', () => {
[['./missing.txt', './symlink.txt'], 'file-broken', 'file-error'],
[['./missing.txt', './missing-dir/symlink.txt'], 'file-error', 'file-error'],
// error is thrown if destination path exists
[['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-dest-exists'],
[['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-error'],
[['./dir-foo', './symlink-dir-foo'], 'dir-success', 'dir-success'],
[['../dir-bar', './dir-foo/symlink-dir-bar'], 'dir-success', 'dir-success'],
[['./dir-bar', './dir-foo/symlink-dir-bar'], 'dir-broken', 'dir-success'],
Expand All @@ -48,7 +48,7 @@ describe('fse-ensure-symlink', () => {
[['./dir-foo', './alpha/beta/gamma/dir-foo'], 'dir-error', 'dir-success'],
[['./missing', './dir-foo/symlink-dir-missing'], 'dir-broken', 'dir-error'],
// error is thrown if destination path exists
[['./dir-foo', './real-alpha/real-beta'], 'dir-error', 'dir-dest-exists'],
[['./dir-foo', './real-alpha/real-beta'], 'dir-error', 'dir-error'],
[[path.resolve(path.join(TEST_DIR, './foo.txt')), './symlink.txt'], 'file-success', 'file-success'],
[[path.resolve(path.join(TEST_DIR, './dir-foo/foo.txt')), './symlink.txt'], 'file-success', 'file-success'],
[[path.resolve(path.join(TEST_DIR, './missing.txt')), './symlink.txt'], 'file-broken', 'file-error'],
Expand Down Expand Up @@ -138,22 +138,6 @@ describe('fse-ensure-symlink', () => {
})
}

function fileDestExists (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
it(`should do nothing using src ${srcpath} and dst ${dstpath}`, done => {
const destinationContentBefore = fs.readFileSync(dstpath, 'utf8')
const callback = err => {
if (err) return done(err)
const destinationContentAfter = fs.readFileSync(dstpath, 'utf8')
assert.strictEqual(destinationContentBefore, destinationContentAfter)
return done()
}
args.push(callback)
return fn(...args)
})
}

function dirSuccess (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
Expand Down Expand Up @@ -214,22 +198,6 @@ describe('fse-ensure-symlink', () => {
})
}

function dirDestExists (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
it(`should do nothing using src ${srcpath} and dst ${dstpath}`, done => {
const destinationContentBefore = fs.readdirSync(dstpath)
const callback = err => {
if (err) return done(err)
const destinationContentAfter = fs.readdirSync(dstpath)
assert.deepStrictEqual(destinationContentBefore, destinationContentAfter)
return done()
}
args.push(callback)
return fn(...args)
})
}

function fileSuccessSync (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
Expand Down Expand Up @@ -280,17 +248,6 @@ describe('fse-ensure-symlink', () => {
})
}

function fileDestExistsSync (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
it(`should do nothing using src ${srcpath} and dst ${dstpath}`, () => {
const destinationContentBefore = fs.readFileSync(dstpath, 'utf8')
fn(...args)
const destinationContentAfter = fs.readFileSync(dstpath, 'utf8')
assert.strictEqual(destinationContentBefore, destinationContentAfter)
})
}

function dirSuccessSync (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
Expand Down Expand Up @@ -341,17 +298,6 @@ describe('fse-ensure-symlink', () => {
})
}

function dirDestExistsSync (args, fn) {
const srcpath = args[0]
const dstpath = args[1]
it(`should do nothing using src ${srcpath} and dst ${dstpath}`, () => {
const destinationContentBefore = fs.readdirSync(dstpath)
fn(...args)
const destinationContentAfter = fs.readdirSync(dstpath)
assert.deepStrictEqual(destinationContentBefore, destinationContentAfter)
})
}

describe('fs.symlink()', () => {
const fn = fs.symlink
tests.forEach(test => {
Expand All @@ -361,12 +307,10 @@ describe('fse-ensure-symlink', () => {
if (nativeBehavior === 'file-success') fileSuccess(args, fn)
if (nativeBehavior === 'file-broken') fileBroken(args, fn)
if (nativeBehavior === 'file-error') fileError(args, fn)
if (nativeBehavior === 'file-dest-exists') fileDestExists(args, fn)
args.push('dir')
if (nativeBehavior === 'dir-success') dirSuccess(args, fn)
if (nativeBehavior === 'dir-broken') dirBroken(args, fn)
if (nativeBehavior === 'dir-error') dirError(args, fn)
if (nativeBehavior === 'dir-dest-exists') dirDestExists(args, fn)
})
})

Expand All @@ -379,11 +323,9 @@ describe('fse-ensure-symlink', () => {
if (newBehavior === 'file-success') fileSuccess(args, fn)
if (newBehavior === 'file-broken') fileBroken(args, fn)
if (newBehavior === 'file-error') fileError(args, fn)
if (newBehavior === 'file-dest-exists') fileDestExists(args, fn)
if (newBehavior === 'dir-success') dirSuccess(args, fn)
if (newBehavior === 'dir-broken') dirBroken(args, fn)
if (newBehavior === 'dir-error') dirError(args, fn)
if (newBehavior === 'dir-dest-exists') dirDestExists(args, fn)
})
})

Expand Down Expand Up @@ -419,12 +361,10 @@ describe('fse-ensure-symlink', () => {
if (nativeBehavior === 'file-success') fileSuccessSync(args, fn)
if (nativeBehavior === 'file-broken') fileBrokenSync(args, fn)
if (nativeBehavior === 'file-error') fileErrorSync(args, fn)
if (nativeBehavior === 'file-dest-exists') fileDestExistsSync(args, fn)
args.push('dir')
if (nativeBehavior === 'dir-success') dirSuccessSync(args, fn)
if (nativeBehavior === 'dir-broken') dirBrokenSync(args, fn)
if (nativeBehavior === 'dir-error') dirErrorSync(args, fn)
if (nativeBehavior === 'dir-dest-exists') dirDestExistsSync(args, fn)
})
})

Expand All @@ -437,11 +377,9 @@ describe('fse-ensure-symlink', () => {
if (newBehavior === 'file-success') fileSuccessSync(args, fn)
if (newBehavior === 'file-broken') fileBrokenSync(args, fn)
if (newBehavior === 'file-error') fileErrorSync(args, fn)
if (newBehavior === 'file-dest-exists') fileDestExistsSync(args, fn)
if (newBehavior === 'dir-success') dirSuccessSync(args, fn)
if (newBehavior === 'dir-broken') dirBrokenSync(args, fn)
if (newBehavior === 'dir-error') dirErrorSync(args, fn)
if (newBehavior === 'dir-dest-exists') dirDestExistsSync(args, fn)
})
})
})
13 changes: 8 additions & 5 deletions lib/ensure/link.js
View file
Expand Up @@ -14,9 +14,9 @@ function createLink (srcpath, dstpath, callback) {
})
}

pathExists(dstpath, (err, destinationExists) => {
if (err) return callback(err)
if (destinationExists) return callback(null)
fs.lstat(dstpath, (err, stats) => {
if (!err && stats.isSymbolicLink()) return callback(null)

fs.lstat(srcpath, (err) => {
if (err) {
err.message = err.message.replace('lstat', 'ensureLink')
Expand All @@ -37,8 +37,11 @@ function createLink (srcpath, dstpath, callback) {
}

function createLinkSync (srcpath, dstpath) {
const destinationExists = fs.existsSync(dstpath)
if (destinationExists) return undefined
let stats
try {
stats = fs.lstatSync(dstpath)
} catch {}
if (stats && stats.isSymbolicLink()) return

try {
fs.lstatSync(srcpath)
Expand Down
12 changes: 7 additions & 5 deletions lib/ensure/symlink.js
View file
Expand Up @@ -21,9 +21,8 @@ function createSymlink (srcpath, dstpath, type, callback) {
callback = (typeof type === 'function') ? type : callback
type = (typeof type === 'function') ? false : type

pathExists(dstpath, (err, destinationExists) => {
if (err) return callback(err)
if (destinationExists) return callback(null)
fs.lstat(dstpath, (err, stats) => {
if (!err && stats.isSymbolicLink()) return callback(null)
symlinkPaths(srcpath, dstpath, (err, relative) => {
if (err) return callback(err)
srcpath = relative.toDst
Expand All @@ -44,8 +43,11 @@ function createSymlink (srcpath, dstpath, type, callback) {
}

function createSymlinkSync (srcpath, dstpath, type) {
const destinationExists = fs.existsSync(dstpath)
if (destinationExists) return undefined
let stats
try {
stats = fs.lstatSync(dstpath)
} catch {}
if (stats && stats.isSymbolicLink()) return

const relative = symlinkPathsSync(srcpath, dstpath)
srcpath = relative.toDst
Expand Down

0 comments on commit eae5c33

Please sign in to comment.