Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] byGlob: Re-included exclude patterns do not add resources to the result set themselves #193

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 38 additions & 29 deletions lib/adapters/AbstractAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,43 +44,52 @@ class AbstractAdapter extends AbstractReaderWriter {
* @param {module:@ui5/fs.tracing.Trace} trace Trace instance
* @returns {Promise<module:@ui5/fs.Resource[]>} 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);
});
}

/**
Expand Down
14 changes: 12 additions & 2 deletions lib/adapters/FileSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<module:@ui5/fs.Resource[]>} 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) => {
Expand Down
19 changes: 16 additions & 3 deletions lib/adapters/Memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<module:@ui5/fs.Resource[]>} 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({
Expand All @@ -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];
Expand All @@ -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];
Expand Down
14 changes: 14 additions & 0 deletions test/lib/adapters/FileSystem_read.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
14 changes: 14 additions & 0 deletions test/lib/adapters/Memory_read.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/",
Expand Down