From 98db5837d852aa0fadb77134aaf81a6712c3029a Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Wed, 29 May 2019 15:24:52 +0200 Subject: [PATCH] Integrate code review - Rename isExcludedPath to isPathExcluded - Also resolve with null instead of undefined if a resource has been excluded - Rename GLOB to glob as already decided earlier --- lib/adapters/AbstractAdapter.js | 4 ++-- lib/adapters/FileSystem.js | 8 +++++--- lib/adapters/Memory.js | 6 +++--- lib/resourceFactory.js | 10 +++++----- test/lib/glob.js | 6 +++++- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/adapters/AbstractAdapter.js b/lib/adapters/AbstractAdapter.js index 9f9aa21e..e2696289 100644 --- a/lib/adapters/AbstractAdapter.js +++ b/lib/adapters/AbstractAdapter.js @@ -19,7 +19,7 @@ class AbstractAdapter extends AbstractReaderWriter { * @public * @param {Object} parameters Parameters * @param {string} parameters.virBasePath Virtual base path - * @param {string[]} [parameters.excludes] List of GLOB patterns to exclude + * @param {string[]} [parameters.excludes] List of glob patterns to exclude */ constructor({virBasePath, excludes = [], project}) { if (new.target === AbstractAdapter) { @@ -89,7 +89,7 @@ class AbstractAdapter extends AbstractReaderWriter { * @param {string} virPath Virtual Path * @returns {boolean} True if path is excluded, otherwise false */ - isExcludedPath(virPath) { + isPathExcluded(virPath) { return micromatch(virPath, this._excludes).length > 0; } diff --git a/lib/adapters/FileSystem.js b/lib/adapters/FileSystem.js index 2997cb3b..64430f7c 100644 --- a/lib/adapters/FileSystem.js +++ b/lib/adapters/FileSystem.js @@ -21,7 +21,7 @@ class FileSystem extends AbstractAdapter { * @param {Object} parameters Parameters * @param {string} parameters.virBasePath Virtual base path * @param {string} parameters.fsBasePath (Physical) File system path - * @param {string[]} [parameters.excludes] List of GLOB patterns to exclude + * @param {string[]} [parameters.excludes] List of glob patterns to exclude */ constructor({virBasePath, project, fsBasePath, excludes}) { super({virBasePath, project, excludes}); @@ -66,6 +66,8 @@ class FileSystem extends AbstractAdapter { }); })); } + // Only glob if the only pattern is an empty string. + // Starting with globby v8 or v9 empty glob patterns "" act like "**" if (patterns.length > 1 || patterns[0] !== "") { const matches = await glob(patterns, opt); for (let i = matches.length - 1; i >= 0; i--) { @@ -107,8 +109,8 @@ class FileSystem extends AbstractAdapter { * @returns {Promise} Promise resolving to a single resource or null if not found */ _byPath(virPath, options, trace) { - if (this.isExcludedPath(virPath)) { - return Promise.resolve(); + if (this.isPathExcluded(virPath)) { + return Promise.resolve(null); } return new Promise((resolve, reject) => { diff --git a/lib/adapters/Memory.js b/lib/adapters/Memory.js index 178e13f3..2c679fe4 100644 --- a/lib/adapters/Memory.js +++ b/lib/adapters/Memory.js @@ -17,7 +17,7 @@ class Memory extends AbstractAdapter { * @public * @param {Object} parameters Parameters * @param {string} parameters.virBasePath Virtual base path - * @param {string[]} [parameters.excludes] List of GLOB patterns to exclude + * @param {string[]} [parameters.excludes] List of glob patterns to exclude */ constructor({virBasePath, project, excludes}) { super({virBasePath, project, excludes}); @@ -81,8 +81,8 @@ class Memory extends AbstractAdapter { * @returns {Promise} Promise resolving to a single resource */ _byPath(virPath, options, trace) { - if (this.isExcludedPath(virPath)) { - return Promise.resolve(); + if (this.isPathExcluded(virPath)) { + return Promise.resolve(null); } return new Promise((resolve, reject) => { if (!virPath.startsWith(this._virBasePath) && virPath !== this._virBaseDir) { diff --git a/lib/resourceFactory.js b/lib/resourceFactory.js index 2a7cb9b0..dc0a05af 100644 --- a/lib/resourceFactory.js +++ b/lib/resourceFactory.js @@ -21,7 +21,7 @@ const resourceFactory = { * @public * @callback module:@ui5/fs.resourceFactory~getProjectExcludes * @param {Object} Project - * @returns {string[]} List of GLOB patterns to exclude + * @returns {string[]} List of glob patterns to exclude */ /** @@ -50,7 +50,7 @@ const resourceFactory = { * @param {boolean} [parameters.useNamespaces=false] Use project namespaces as path prefixes * DEPRECATED: Use parameter getVirtualBasePathPrefix instead. * @param {module:@ui5/fs.resourceFactory~getProjectExcludes} [parameters.getProjectExcludes] - * Callback to retrieve the exclude GLOBs of a project + * Callback to retrieve the exclude globs of a project * @param {module:@ui5/fs.resourceFactory~getVirtualBasePathPrefix} [parameters.getVirtualBasePathPrefix] * Callback to retrieve a prefix for a given virtual base path of a project if required * @returns {Object} Object containing source and dependencies resource readers @@ -167,9 +167,9 @@ const resourceFactory = { * @param {string} parameters.virBasePath Virtual base path to create the adapter for * @param {boolean} parameters.useNamespaces Use project namespace as path prefix * @param {module:@ui5/fs.resourceFactory~getProjectExcludes} [parameters.getProjectExcludes] - * Callback to retrieve the exclude GLOB of a project + * Callback to retrieve the exclude glob of a project * @param {module:@ui5/fs.resourceFactory~getVirtualBasePathPrefix} [parameters.getVirtualBasePathPrefix] - * Callback to retrieve the exclude GLOB of a project + * Callback to retrieve the exclude glob of a project * @returns {Promise} Promise resolving to list of normalized glob patterns */ _createFsAdapterForVirtualBasePath({ @@ -251,7 +251,7 @@ const resourceFactory = { * @param {Object} parameters Parameters * @param {string} parameters.virBasePath Virtual base path * @param {string} [parameters.fsBasePath] File system base path - * @param {string[]} [parameters.excludes] List of GLOB patterns to exclude + * @param {string[]} [parameters.excludes] List of glob patterns to exclude * @returns {module:@ui5/fs.adapters.FileSystem|module:@ui5/fs.adapters.Memory} File System- or Virtual Adapter */ createAdapter({fsBasePath, virBasePath, project, excludes}) { diff --git a/test/lib/glob.js b/test/lib/glob.js index b4d091d9..6c24fc10 100644 --- a/test/lib/glob.js +++ b/test/lib/glob.js @@ -209,7 +209,11 @@ test("glob with multiple patterns with static exclude", (t) => { }); }); -/* Generic Micromatch tests for understanding GLOB exclude behavior*/ +/* + Generic Micromatch tests for understanding glob exclude behavior. + Not active since they only test external code and our adapter-tests should + already test for any incompatible changes. +*/ // test("micromatch exclude test", async (t) => { // const micromatch = require("micromatch");