From 199ec9f2c28bf14e7da62d131ced80f1255d5f79 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 9 Nov 2017 13:22:59 -0800 Subject: [PATCH] Apply filter before creating parent dir in copy --- .../copy-prevent-copying-identical.test.js | 8 +-- .../copy-prevent-copying-into-itself.test.js | 66 +++++++++---------- lib/copy/__tests__/copy.test.js | 19 +++++- lib/copy/copy.js | 2 + 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/lib/copy/__tests__/copy-prevent-copying-identical.test.js b/lib/copy/__tests__/copy-prevent-copying-identical.test.js index 64357105..615645b5 100644 --- a/lib/copy/__tests__/copy-prevent-copying-identical.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-identical.test.js @@ -8,21 +8,21 @@ const klawSync = require('klaw-sync') /* global beforeEach, afterEach, describe, it */ -describe('+ copySync() - prevent copying identical files and dirs', () => { +describe('+ copy() - prevent copying identical files and dirs', () => { let TEST_DIR = '' let src = '' let dest = '' beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-identical') + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-identical') fs.emptyDir(TEST_DIR, done) }) afterEach(done => fs.remove(TEST_DIR, done)) it('should return an error if src and dest are the same', done => { - const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync') - const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync') + const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy') + const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy') fs.copy(fileSrc, fileDest, err => { assert.equal(err.message, 'Source and destination must not be the same.') diff --git a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js index d759a1a5..9fe0ef0e 100644 --- a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js +++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js @@ -21,43 +21,11 @@ const dat1 = 'file1' const dat2 = 'file2' const dat3 = 'file3' -function testSuccess (src, dest, done) { - const srclen = klawSync(src).length - assert(srclen > 2) - fs.copy(src, dest, err => { - assert.ifError(err) - - const destlen = klawSync(dest).length - - assert.strictEqual(destlen, srclen) - - FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) - - const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') - const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') - const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') - const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') - - assert.strictEqual(o0, dat0, 'file contents matched') - assert.strictEqual(o1, dat1, 'file contents matched') - assert.strictEqual(o2, dat2, 'file contents matched') - assert.strictEqual(o3, dat3, 'file contents matched') - done() - }) -} - -function testError (src, dest, done) { - fs.copy(src, dest, err => { - assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) - done() - }) -} - describe('+ copy() - prevent copying into itself', () => { let TEST_DIR, src beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself-4') + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself') src = path.join(TEST_DIR, 'src') fs.mkdirpSync(src) @@ -370,3 +338,35 @@ describe('+ copy() - prevent copying into itself', () => { }) }) }) + +function testSuccess (src, dest, done) { + const srclen = klawSync(src).length + assert(srclen > 2) + fs.copy(src, dest, err => { + assert.ifError(err) + + const destlen = klawSync(dest).length + + assert.strictEqual(destlen, srclen) + + FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied')) + + const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8') + const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8') + const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8') + const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8') + + assert.strictEqual(o0, dat0, 'file contents matched') + assert.strictEqual(o1, dat1, 'file contents matched') + assert.strictEqual(o2, dat2, 'file contents matched') + assert.strictEqual(o3, dat3, 'file contents matched') + done() + }) +} + +function testError (src, dest, done) { + fs.copy(src, dest, err => { + assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`) + done() + }) +} diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js index 4787bfd8..d8e38f63 100644 --- a/lib/copy/__tests__/copy.test.js +++ b/lib/copy/__tests__/copy.test.js @@ -208,6 +208,21 @@ describe('fs-extra', () => { }) describe('> when filter is used', () => { + it('should do nothing if filter fails', done => { + const srcDir = path.join(TEST_DIR, 'src') + const srcFile = path.join(srcDir, 'srcfile.css') + fse.outputFileSync(srcFile, 'src contents') + const destDir = path.join(TEST_DIR, 'dest') + const destFile = path.join(destDir, 'destfile.css') + const filter = s => path.extname(s) !== '.css' && !fs.statSync(s).isDirectory() + + fse.copy(srcFile, destFile, filter, err => { + assert.ifError(err) + assert(!fs.existsSync(destDir)) + done() + }) + }) + it('should only copy files allowed by filter fn', done => { const srcFile1 = path.join(TEST_DIR, '1.css') fs.writeFileSync(srcFile1, '') @@ -234,7 +249,7 @@ describe('fs-extra', () => { }) }) - it('should should apply filter recursively', done => { + it('should apply filter recursively', done => { const FILES = 2 // Don't match anything that ends with a digit higher than 0: const filter = s => /(0|\D)$/i.test(s) @@ -280,7 +295,7 @@ describe('fs-extra', () => { }) }) - it('should apply the filter to directory names', done => { + it('should apply filter to directory names', done => { const IGNORE = 'ignore' const filter = p => !~p.indexOf(IGNORE) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 76aaa23c..f270c1d7 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -35,6 +35,8 @@ function copy (src, dest, opts, cb) { // don't allow src and dest to be the same if (src === dest) return cb(new Error('Source and destination must not be the same.')) + if (opts.filter && !opts.filter(src, dest)) return cb() + const destParent = path.dirname(dest) pathExists(destParent, (err, dirExists) => { if (err) return cb(err)