Skip to content

Commit 29e29ba

Browse files
RandomByteRandomByte
committed
[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
1 parent 1f11762 commit 29e29ba

File tree

5 files changed

+94
-34
lines changed

5 files changed

+94
-34
lines changed

‎lib/adapters/AbstractAdapter.js

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -44,43 +44,52 @@ class AbstractAdapter extends AbstractReaderWriter {
4444
* @param {module:@ui5/fs.tracing.Trace} trace Trace instance
4545
* @returns {Promise<module:@ui5/fs.Resource[]>} Promise resolving to list of resources
4646
*/
47-
_byGlob(virPattern, options = {nodir: true}, trace) {
48-
const excludes = this._excludesNegated;
49-
47+
async _byGlob(virPattern, options = {nodir: true}, trace) {
5048
if (!(virPattern instanceof Array)) {
5149
virPattern = [virPattern];
5250
}
5351

54-
// Append static exclude patterns
55-
virPattern = Array.prototype.concat.apply(virPattern, excludes);
52+
let patterns = await Promise.all(virPattern.map(this._normalizePattern, this));
53+
patterns = Array.prototype.concat.apply([], patterns);
54+
if (patterns.length === 0) {
55+
return [];
56+
}
5657

57-
return Promise.all(virPattern.map(this._normalizePattern, this)).then((patterns) => {
58-
patterns = Array.prototype.concat.apply([], patterns);
59-
if (patterns.length === 0) {
60-
return [];
61-
}
58+
const excludePatterns = await this._getExcludePattern();
6259

63-
if (!options.nodir) {
64-
for (let i = patterns.length - 1; i >= 0; i--) {
65-
const idx = this._virBaseDir.indexOf(patterns[i]);
66-
if (patterns[i] && idx !== -1 && idx < this._virBaseDir.length) {
67-
const subPath = patterns[i];
68-
return Promise.resolve([
69-
new Resource({
70-
project: this.project,
71-
statInfo: { // TODO: make closer to fs stat info
72-
isDirectory: function() {
73-
return true;
74-
}
75-
},
76-
path: subPath
77-
})
78-
]);
79-
}
60+
if (!options.nodir) {
61+
for (let i = patterns.length - 1; i >= 0; i--) {
62+
const idx = this._virBaseDir.indexOf(patterns[i]);
63+
if (patterns[i] && idx !== -1 && idx < this._virBaseDir.length) {
64+
const subPath = patterns[i];
65+
return Promise.resolve([
66+
new Resource({
67+
project: this.project,
68+
statInfo: { // TODO: make closer to fs stat info
69+
isDirectory: function() {
70+
return true;
71+
}
72+
},
73+
path: subPath
74+
})
75+
]);
8076
}
8177
}
82-
return this._runGlob(patterns, options, trace);
83-
});
78+
}
79+
return this._runGlob(patterns, {
80+
nodir: options.nodir,
81+
ignore: excludePatterns
82+
}, trace);
83+
}
84+
85+
async _getExcludePattern() {
86+
if (this._normalizedExcludes) {
87+
return this._normalizedExcludes;
88+
}
89+
return this._normalizedExcludes = Promise.all(this._excludes.map(this._normalizePattern, this))
90+
.then((excludePatterns) => {
91+
return Array.prototype.concat.apply([], excludePatterns);
92+
});
8493
}
8594

8695
/**

‎lib/adapters/FileSystem.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,28 @@ class FileSystem extends AbstractAdapter {
3535
* @param {Array} patterns Array of glob patterns
3636
* @param {Object} [options={}] glob options
3737
* @param {boolean} [options.nodir=true] Do not match directories
38+
* @param {boolean} [options.ignore=[]] An array of glob patterns to exclude matches.
3839
* @param {module:@ui5/fs.tracing.Trace} trace Trace instance
3940
* @returns {Promise<module:@ui5/fs.Resource[]>} Promise resolving to list of resources
4041
*/
41-
async _runGlob(patterns, options = {nodir: true}, trace) {
42+
async _runGlob(patterns, options = {}, trace) {
43+
if (options.nodir === undefined) {
44+
options.nodir = true;
45+
}
46+
if (options.ignore === undefined) {
47+
options.ignore = [];
48+
}
4249
const opt = {
4350
cwd: this._fsBasePath,
4451
dot: true,
4552
onlyFiles: options.nodir,
53+
ignore: options.ignore,
4654
followSymbolicLinks: false
4755
};
4856
trace.globCall();
49-
57+
if (log.isLevelEnabled("verbose")) {
58+
log.verbose(`Glob with patterns ${patterns.join(", ")} and excludes ${options.ignore.join(", ")}`);
59+
}
5060
const promises = [];
5161
if (!opt.onlyFiles && patterns.includes("")) { // Match physical root directory
5262
promises.push(new Promise((resolve, reject) => {

‎lib/adapters/Memory.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,21 @@ class Memory extends AbstractAdapter {
3232
* @param {Array} patterns array of glob patterns
3333
* @param {Object} [options={}] glob options
3434
* @param {boolean} [options.nodir=true] Do not match directories
35+
* @param {boolean} [options.ignore=[]] An array of glob patterns to exclude matches.
3536
* @param {module:@ui5/fs.tracing.Trace} trace Trace instance
3637
* @returns {Promise<module:@ui5/fs.Resource[]>} Promise resolving to list of resources
3738
*/
38-
async _runGlob(patterns, options = {nodir: true}, trace) {
39+
async _runGlob(patterns, options = {}, trace) {
40+
if (options.nodir === undefined) {
41+
options.nodir = true;
42+
}
43+
if (options.ignore === undefined) {
44+
options.ignore = [];
45+
}
46+
trace.globCall();
47+
if (log.isLevelEnabled("verbose")) {
48+
log.verbose(`Glob with patterns ${patterns.join(", ")} and excludes ${options.ignore.join(", ")}`);
49+
}
3950
if (patterns[0] === "" && !options.nodir) { // Match virtual root directory
4051
return [
4152
new Resource({
@@ -52,7 +63,8 @@ class Memory extends AbstractAdapter {
5263

5364
const filePaths = Object.keys(this._virFiles);
5465
const matchedFilePaths = micromatch(filePaths, patterns, {
55-
dot: true
66+
dot: true,
67+
ignore: options.ignore
5668
});
5769
let matchedResources = matchedFilePaths.map((virPath) => {
5870
return this._virFiles[virPath];
@@ -61,7 +73,8 @@ class Memory extends AbstractAdapter {
6173
if (!options.nodir) {
6274
const dirPaths = Object.keys(this._virDirs);
6375
const matchedDirs = micromatch(dirPaths, patterns, {
64-
dot: true
76+
dot: true,
77+
ignore: options.ignore
6578
});
6679
matchedResources = matchedResources.concat(matchedDirs.map((virPath) => {
6780
return this._virDirs[virPath];

‎test/lib/adapters/FileSystem_read.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,20 @@ test("static excludes: glob with negated directory exclude, not excluding resour
242242
t.deepEqual(resources.length, 2, "Found two resources");
243243
});
244244

245+
test("static excludes: glob not existing resource should not match any re-included excludes", async (t) => {
246+
const srcReader = resourceFactory.createAdapter({
247+
fsBasePath: "./test/fixtures/library.l/src",
248+
virBasePath: "/resources/",
249+
excludes: [
250+
"**",
251+
"!**/some.js"
252+
]
253+
});
254+
255+
const resources = await srcReader.byGlob("/resources/**/does-not-exist", {nodir: true});
256+
t.deepEqual(resources.length, 0, "Found no resources");
257+
});
258+
245259
test("static excludes: byPath exclude everything in sub-directory", async (t) => {
246260
const readerWriter = resourceFactory.createAdapter({
247261
fsBasePath: "./test/fixtures/application.a/webapp",

‎test/lib/adapters/Memory_read.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,20 @@ test("static excludes: glob with negated directory exclude, not excluding resour
416416
t.deepEqual(resources.length, 2, "Found two resources");
417417
});
418418

419+
test("static excludes: glob not existing resource should not match any re-included excludes", async (t) => {
420+
const srcReader = resourceFactory.createAdapter({
421+
fsBasePath: "./test/fixtures/library.l/src",
422+
virBasePath: "/resources/",
423+
excludes: [
424+
"**",
425+
"!**/some.js"
426+
]
427+
});
428+
429+
const resources = await srcReader.byGlob("/resources/**/does-not-exist", {nodir: true});
430+
t.deepEqual(resources.length, 0, "Found no resources");
431+
});
432+
419433
test("static excludes: byPath exclude everything in sub-directory", async (t) => {
420434
const readerWriter = resourceFactory.createAdapter({
421435
virBasePath: "/resources/app/",

0 commit comments

Comments
 (0)