From 29e29ba9a0f3d7ff095d781e0ce5aef1f2a73e6d Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Fri, 22 Nov 2019 14:27:26 +0100 Subject: [PATCH] [FIX] byGlob: Re-included exclude patterns do not add resources to the result set themselves The current handling of glob exclude patterns may lead to resources being added to a result set even though they do not match the provided glob patterns. This can happen if a negated exclude pattern is provided. Resolves https://github.com/SAP/ui5-builder/issues/367 --- lib/adapters/AbstractAdapter.js | 67 ++++++++++++++++------------ lib/adapters/FileSystem.js | 14 +++++- lib/adapters/Memory.js | 19 ++++++-- test/lib/adapters/FileSystem_read.js | 14 ++++++ test/lib/adapters/Memory_read.js | 14 ++++++ 5 files changed, 94 insertions(+), 34 deletions(-) diff --git a/lib/adapters/AbstractAdapter.js b/lib/adapters/AbstractAdapter.js index e2696289..fce552be 100644 --- a/lib/adapters/AbstractAdapter.js +++ b/lib/adapters/AbstractAdapter.js @@ -44,43 +44,52 @@ class AbstractAdapter extends AbstractReaderWriter { * @param {module:@ui5/fs.tracing.Trace} trace Trace instance * @returns {Promise} Promise resolving to list of resources */ - _byGlob(virPattern, options = {nodir: true}, trace) { - const excludes = this._excludesNegated; - + async _byGlob(virPattern, options = {nodir: true}, trace) { if (!(virPattern instanceof Array)) { virPattern = [virPattern]; } - // Append static exclude patterns - virPattern = Array.prototype.concat.apply(virPattern, excludes); + let patterns = await Promise.all(virPattern.map(this._normalizePattern, this)); + patterns = Array.prototype.concat.apply([], patterns); + if (patterns.length === 0) { + return []; + } - return Promise.all(virPattern.map(this._normalizePattern, this)).then((patterns) => { - patterns = Array.prototype.concat.apply([], patterns); - if (patterns.length === 0) { - return []; - } + const excludePatterns = await this._getExcludePattern(); - if (!options.nodir) { - for (let i = patterns.length - 1; i >= 0; i--) { - const idx = this._virBaseDir.indexOf(patterns[i]); - if (patterns[i] && idx !== -1 && idx < this._virBaseDir.length) { - const subPath = patterns[i]; - return Promise.resolve([ - new Resource({ - project: this.project, - statInfo: { // TODO: make closer to fs stat info - isDirectory: function() { - return true; - } - }, - path: subPath - }) - ]); - } + if (!options.nodir) { + for (let i = patterns.length - 1; i >= 0; i--) { + const idx = this._virBaseDir.indexOf(patterns[i]); + if (patterns[i] && idx !== -1 && idx < this._virBaseDir.length) { + const subPath = patterns[i]; + return Promise.resolve([ + new Resource({ + project: this.project, + statInfo: { // TODO: make closer to fs stat info + isDirectory: function() { + return true; + } + }, + path: subPath + }) + ]); } } - return this._runGlob(patterns, options, trace); - }); + } + return this._runGlob(patterns, { + nodir: options.nodir, + ignore: excludePatterns + }, trace); + } + + async _getExcludePattern() { + if (this._normalizedExcludes) { + return this._normalizedExcludes; + } + return this._normalizedExcludes = Promise.all(this._excludes.map(this._normalizePattern, this)) + .then((excludePatterns) => { + return Array.prototype.concat.apply([], excludePatterns); + }); } /** diff --git a/lib/adapters/FileSystem.js b/lib/adapters/FileSystem.js index 0664955f..605ea9e3 100644 --- a/lib/adapters/FileSystem.js +++ b/lib/adapters/FileSystem.js @@ -35,18 +35,28 @@ class FileSystem extends AbstractAdapter { * @param {Array} patterns Array of glob patterns * @param {Object} [options={}] glob options * @param {boolean} [options.nodir=true] Do not match directories + * @param {boolean} [options.ignore=[]] An array of glob patterns to exclude matches. * @param {module:@ui5/fs.tracing.Trace} trace Trace instance * @returns {Promise} Promise resolving to list of resources */ - async _runGlob(patterns, options = {nodir: true}, trace) { + async _runGlob(patterns, options = {}, trace) { + if (options.nodir === undefined) { + options.nodir = true; + } + if (options.ignore === undefined) { + options.ignore = []; + } const opt = { cwd: this._fsBasePath, dot: true, onlyFiles: options.nodir, + ignore: options.ignore, followSymbolicLinks: false }; trace.globCall(); - + if (log.isLevelEnabled("verbose")) { + log.verbose(`Glob with patterns ${patterns.join(", ")} and excludes ${options.ignore.join(", ")}`); + } const promises = []; if (!opt.onlyFiles && patterns.includes("")) { // Match physical root directory promises.push(new Promise((resolve, reject) => { diff --git a/lib/adapters/Memory.js b/lib/adapters/Memory.js index 2c679fe4..dc4887b6 100644 --- a/lib/adapters/Memory.js +++ b/lib/adapters/Memory.js @@ -32,10 +32,21 @@ class Memory extends AbstractAdapter { * @param {Array} patterns array of glob patterns * @param {Object} [options={}] glob options * @param {boolean} [options.nodir=true] Do not match directories + * @param {boolean} [options.ignore=[]] An array of glob patterns to exclude matches. * @param {module:@ui5/fs.tracing.Trace} trace Trace instance * @returns {Promise} Promise resolving to list of resources */ - async _runGlob(patterns, options = {nodir: true}, trace) { + async _runGlob(patterns, options = {}, trace) { + if (options.nodir === undefined) { + options.nodir = true; + } + if (options.ignore === undefined) { + options.ignore = []; + } + trace.globCall(); + if (log.isLevelEnabled("verbose")) { + log.verbose(`Glob with patterns ${patterns.join(", ")} and excludes ${options.ignore.join(", ")}`); + } if (patterns[0] === "" && !options.nodir) { // Match virtual root directory return [ new Resource({ @@ -52,7 +63,8 @@ class Memory extends AbstractAdapter { const filePaths = Object.keys(this._virFiles); const matchedFilePaths = micromatch(filePaths, patterns, { - dot: true + dot: true, + ignore: options.ignore }); let matchedResources = matchedFilePaths.map((virPath) => { return this._virFiles[virPath]; @@ -61,7 +73,8 @@ class Memory extends AbstractAdapter { if (!options.nodir) { const dirPaths = Object.keys(this._virDirs); const matchedDirs = micromatch(dirPaths, patterns, { - dot: true + dot: true, + ignore: options.ignore }); matchedResources = matchedResources.concat(matchedDirs.map((virPath) => { return this._virDirs[virPath]; diff --git a/test/lib/adapters/FileSystem_read.js b/test/lib/adapters/FileSystem_read.js index f762c3ba..ff2ceab4 100644 --- a/test/lib/adapters/FileSystem_read.js +++ b/test/lib/adapters/FileSystem_read.js @@ -242,6 +242,20 @@ test("static excludes: glob with negated directory exclude, not excluding resour t.deepEqual(resources.length, 2, "Found two resources"); }); +test("static excludes: glob not existing resource should not match any re-included excludes", async (t) => { + const srcReader = resourceFactory.createAdapter({ + fsBasePath: "./test/fixtures/library.l/src", + virBasePath: "/resources/", + excludes: [ + "**", + "!**/some.js" + ] + }); + + const resources = await srcReader.byGlob("/resources/**/does-not-exist", {nodir: true}); + t.deepEqual(resources.length, 0, "Found no resources"); +}); + test("static excludes: byPath exclude everything in sub-directory", async (t) => { const readerWriter = resourceFactory.createAdapter({ fsBasePath: "./test/fixtures/application.a/webapp", diff --git a/test/lib/adapters/Memory_read.js b/test/lib/adapters/Memory_read.js index d3da89ae..5a6ac1ab 100644 --- a/test/lib/adapters/Memory_read.js +++ b/test/lib/adapters/Memory_read.js @@ -416,6 +416,20 @@ test("static excludes: glob with negated directory exclude, not excluding resour t.deepEqual(resources.length, 2, "Found two resources"); }); +test("static excludes: glob not existing resource should not match any re-included excludes", async (t) => { + const srcReader = resourceFactory.createAdapter({ + fsBasePath: "./test/fixtures/library.l/src", + virBasePath: "/resources/", + excludes: [ + "**", + "!**/some.js" + ] + }); + + const resources = await srcReader.byGlob("/resources/**/does-not-exist", {nodir: true}); + t.deepEqual(resources.length, 0, "Found no resources"); +}); + test("static excludes: byPath exclude everything in sub-directory", async (t) => { const readerWriter = resourceFactory.createAdapter({ virBasePath: "/resources/app/",