Skip to content

Commit

Permalink
[FEATURE] projectPreprocessor: Log warning when using a deprecated or…
Browse files Browse the repository at this point in the history 
… restricted depdency

Projects in the tree that are not the root project may trigger a warning
log if they configure any of the following metadata flags:
- deprecated: true
- sapInternal: true

The latter may not trigger a warning if its parent project defines a
metadata flag "allowSapInternal: true".
  • Loading branch information
@RandomByte
RandomByte committed Mar 5, 2020
1 parent f132eb5 commit efcd106
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 20 deletions.
33 changes: 25 additions & 8 deletions lib/projectPreprocessor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ const parseYaml = require("js-yaml").safeLoadAll;
const typeRepository = require("@ui5/builder").types.typeRepository;

class ProjectPreprocessor {
constructor() {
constructor({tree}) {
this.tree = tree;
this.processedProjects = {};
this.configShims = {};
this.collections = {};
Expand All @@ -19,9 +20,9 @@ class ProjectPreprocessor {
- Replace duplicate projects further away from the root with those closer to the root
- Add configuration to projects
*/
async processTree(tree) {
async processTree() {
const queue = [{
projects: [tree],
projects: [this.tree],
parent: null,
level: 0
}];
Expand Down Expand Up @@ -65,7 +66,8 @@ class ProjectPreprocessor {
// Do a dependency lookahead to apply any extensions that might affect this project
await this.dependencyLookahead(project, project.dependencies);
} else {
// When using the static translator for instance dependencies is not defined and will fail later access calls to it
// When using the static translator for instance, dependencies is not defined and will
// fail later access calls to it
project.dependencies = [];
}

Expand All @@ -81,14 +83,15 @@ class ProjectPreprocessor {
this.applyShims(project);
if (this.isConfigValid(project)) {
await this.applyType(project);
this.checkProjectMetadata(parent, project);
queue.push({
// copy array, so that the queue is stable while ignored project dependencies are removed
projects: [...project.dependencies],
parent: project,
level: level + 1
});
} else {
if (project === tree) {
if (project === this.tree) {
throw new Error(`Failed to configure root project "${project.id}". Please check verbose log for details.`);
}
// No config available
Expand All @@ -110,7 +113,7 @@ class ProjectPreprocessor {
const timeDiff = process.hrtime(startTime);
log.verbose(`Processed ${Object.keys(this.processedProjects).length} projects in ${prettyHrtime(timeDiff)}`);
}
return tree;
return this.tree;
});
}

Expand Down Expand Up @@ -322,6 +325,20 @@ class ProjectPreprocessor {
await type.format(project);
}

checkProjectMetadata(parent, project) {
if (parent && project.metadata.deprecated) {
// Do not warn for deprecated root project
log.warn(`Dependency ${project.metadata.name} is deprecated and should not be used for new projects!`);
}

if (parent && project.metadata.sapInternal && !parent.metadata.allowSapInternal) {
// Do not warn for sapInternal root project
log.warn(`Dependency ${project.metadata.name} is restricted for use by SAP internal projects only! ` +
`If the project ${parent.metadata.name} is an SAP internal project, add the attribute ` +
`"allowSapInternal: true" to its metadata configuration`);
}
}

async applyExtension(extension) {
if (!extension.metadata || !extension.metadata.name) {
throw new Error(`metadata.name configuration is missing for extension ${extension.id}`);
Expand Down Expand Up @@ -534,7 +551,7 @@ module.exports = {
* @returns {Promise<Object>} Promise resolving with the dependency tree and enriched project configuration
*/
processTree: function(tree) {
return new ProjectPreprocessor().processTree(tree);
return new ProjectPreprocessor({tree}).processTree();
},
ProjectPreprocessor
_ProjectPreprocessor: ProjectPreprocessor
};
12 changes: 6 additions & 6 deletions test/lib/extensions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const test = require("ava");
const path = require("path");
const sinon = require("sinon");
const projectPreprocessor = require("../..").projectPreprocessor;
const Preprocessor = require("../..").projectPreprocessor.ProjectPreprocessor;
const Preprocessor = require("../..").projectPreprocessor._ProjectPreprocessor;
const applicationAPath = path.join(__dirname, "..", "fixtures", "application.a");
const legacyLibraryAPath = path.join(__dirname, "..", "fixtures", "legacy.library.a");
const legacyLibraryBPath = path.join(__dirname, "..", "fixtures", "legacy.library.b");
Expand Down Expand Up @@ -673,7 +673,7 @@ test("specVersion: Missing version", async (t) => {
},
shims: {}
};
const preprocessor = new Preprocessor();
const preprocessor = new Preprocessor({});
await t.throwsAsync(preprocessor.applyExtension(extension),
"No specification version defined for extension shims.a",
"Rejected with error");
Expand All @@ -693,7 +693,7 @@ test("specVersion: Extension with invalid version", async (t) => {
},
shims: {}
};
const preprocessor = new Preprocessor();
const preprocessor = new Preprocessor({});
await t.throwsAsync(preprocessor.applyExtension(extension),
"Unsupported specification version 0.9 defined for extension shims.a. " +
"Your UI5 CLI installation might be outdated. For details see " +
Expand All @@ -715,7 +715,7 @@ test("specVersion: Extension with valid version 0.1", async (t) => {
},
shims: {}
};
const preprocessor = new Preprocessor();
const preprocessor = new Preprocessor({});
const handleShimStub = sinon.stub(preprocessor, "handleShim");
await preprocessor.applyExtension(extension);
t.deepEqual(handleShimStub.getCall(0).args[0].specVersion, "0.1", "Correct spec version");
Expand All @@ -735,7 +735,7 @@ test("specVersion: Extension with valid version 1.0", async (t) => {
},
shims: {}
};
const preprocessor = new Preprocessor();
const preprocessor = new Preprocessor({});
const handleShimStub = sinon.stub(preprocessor, "handleShim");
await preprocessor.applyExtension(extension);
t.deepEqual(handleShimStub.getCall(0).args[0].specVersion, "1.0", "Correct spec version");
Expand All @@ -755,7 +755,7 @@ test("specVersion: Extension with valid version 1.1", async (t) => {
},
shims: {}
};
const preprocessor = new Preprocessor();
const preprocessor = new Preprocessor({});
const handleShimStub = sinon.stub(preprocessor, "handleShim");
await preprocessor.applyExtension(extension);
t.deepEqual(handleShimStub.getCall(0).args[0].specVersion, "1.1", "Correct spec version");
Expand Down
162 changes: 156 additions & 6 deletions test/lib/projectPreprocessor.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const test = require("ava");
const sinon = require("sinon");
const mock = require("mock-require");
const path = require("path");
const projectPreprocessor = require("../..").projectPreprocessor;
const projectPreprocessor = require("../../lib/projectPreprocessor");
const applicationAPath = path.join(__dirname, "..", "fixtures", "application.a");
const applicationBPath = path.join(__dirname, "..", "fixtures", "application.b");
const applicationCPath = path.join(__dirname, "..", "fixtures", "application.c");
Expand All @@ -11,6 +13,10 @@ const libraryDPath = path.join(__dirname, "..", "fixtures", "library.d");
const cycleDepsBasePath = path.join(__dirname, "..", "fixtures", "cyclic-deps", "node_modules");
const pathToInvalidModule = path.join(__dirname, "..", "fixtures", "invalidModule");

test.afterEach.always((t) => {
sinon.restore();
});

test("Project with inline configuration", (t) => {
const tree = {
id: "application.a",
Expand Down Expand Up @@ -1651,7 +1657,7 @@ test("specVersion: Project with valid version 1.1", async (t) => {
});

test("isBeingProcessed: Is not being processed", (t) => {
const preprocessor = new projectPreprocessor.ProjectPreprocessor();
const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

preprocessor.processedProjects = {};

Expand All @@ -1668,7 +1674,7 @@ test("isBeingProcessed: Is not being processed", (t) => {
});

test("isBeingProcessed: Is being processed", (t) => {
const preprocessor = new projectPreprocessor.ProjectPreprocessor();
const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

const alreadyProcessedProject = {
project: {
Expand Down Expand Up @@ -1697,7 +1703,7 @@ test("isBeingProcessed: Is being processed", (t) => {
});

test("isBeingProcessed: Processed project is ignored", (t) => {
const preprocessor = new projectPreprocessor.ProjectPreprocessor();
const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

const alreadyProcessedProject = {
project: {
Expand Down Expand Up @@ -1725,7 +1731,7 @@ test("isBeingProcessed: Processed project is ignored", (t) => {
});

test("isBeingProcessed: Processed project is ignored but already removed from parent", (t) => {
const preprocessor = new projectPreprocessor.ProjectPreprocessor();
const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

const alreadyProcessedProject = {
project: {
Expand Down Expand Up @@ -1758,7 +1764,7 @@ test("isBeingProcessed: Processed project is ignored but already removed from pa
});

test("isBeingProcessed: Deduped project is being ignored", (t) => {
const preprocessor = new projectPreprocessor.ProjectPreprocessor();
const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

preprocessor.processedProjects = {};

Expand All @@ -1770,3 +1776,147 @@ test("isBeingProcessed: Deduped project is being ignored", (t) => {
const res = preprocessor.isBeingProcessed(parent, project);
t.deepEqual(res, true, "Project is being ignored");
});


test.serial("applyType", async (t) => {
const formatStub = sinon.stub();
const getTypeStub = sinon.stub(require("@ui5/builder").types.typeRepository, "getType")
.returns({
format: formatStub
});

const project = {
type: "pony",
metadata: {}
};

const preprocessor = new projectPreprocessor._ProjectPreprocessor({});
await preprocessor.applyType(project);

t.is(getTypeStub.callCount, 1, "getType got called once");
t.deepEqual(getTypeStub.getCall(0).args[0], "pony", "getType got called with correct type");

t.is(formatStub.callCount, 1, "format got called once");
t.is(formatStub.getCall(0).args[0], project, "format got called with correct project");
});

test.serial("checkProjectMetadata: Warning logged for deprecated dependencies", async (t) => {
const log = require("@ui5/logger");
const loggerInstance = log.getLogger("pony");
mock("@ui5/logger", {
getLogger: () => loggerInstance
});
const logWarnSpy = sinon.spy(loggerInstance, "warn");

// Re-require tested module
const projectPreprocessor = mock.reRequire("../../lib/projectPreprocessor");

const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

const project1 = {
metadata: {
name: "root.project",
deprecated: true
}
};

// no warning should be logged for root level project
await preprocessor.checkProjectMetadata(null, project1);

const project2 = {
_level: 1,
metadata: {
name: "my.project",
deprecated: true
}
};

// one warning should be logged for deprecated dependency
await preprocessor.checkProjectMetadata(project1, project2);

t.is(logWarnSpy.callCount, 1, "One warning got logged");
t.deepEqual(logWarnSpy.getCall(0).args[0],
"Dependency my.project is deprecated and should not be used for new projects!",
"Logged expected warning message");
mock.stop("@ui5/logger");
});

test.serial("checkProjectMetadata: Warning logged for SAP internal dependencies", async (t) => {
const log = require("@ui5/logger");
const loggerInstance = log.getLogger("pony");
mock("@ui5/logger", {
getLogger: () => loggerInstance
});
const logWarnSpy = sinon.spy(loggerInstance, "warn");

// Re-require tested module
const projectPreprocessor = mock.reRequire("../../lib/projectPreprocessor");

const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

const project1 = {
metadata: {
name: "root.project",
sapInternal: true
}
};

// no warning should be logged for root level project
await preprocessor.checkProjectMetadata(null, project1);

const project2 = {
_level: 1,
metadata: {
name: "my.project",
sapInternal: true
}
};

// one warning should be logged for internal dependency
await preprocessor.checkProjectMetadata(project1, project2);

t.is(logWarnSpy.callCount, 1, "One warning got logged");
t.deepEqual(logWarnSpy.getCall(0).args[0],
`Dependency my.project is restricted for use by SAP internal projects only! ` +
`If the project root.project is an SAP internal project, add the attribute ` +
`"allowSapInternal: true" to its metadata configuration`,
"Logged expected warning message");
mock.stop("@ui5/logger");
});

test.serial("checkProjectMetadata: No warning logged for allowed SAP internal libraries", async (t) => {
sinon.stub(require("@ui5/builder").types.typeRepository, "getType")
.returns({format: () => {}});

// Spying logger of processors/bootstrapHtmlTransformer
const log = require("@ui5/logger");
const loggerInstance = log.getLogger("pony");
mock("@ui5/logger", {
getLogger: () => loggerInstance
});
const logWarnSpy = sinon.spy(loggerInstance, "warn");

// Re-require tested module
const projectPreprocessor = mock.reRequire("../../lib/projectPreprocessor");

const preprocessor = new projectPreprocessor._ProjectPreprocessor({});

const parent = {
metadata: {
name: "parent.project",
allowSapInternal: true // parent project allows sap internal project use
}
};

const project = {
metadata: {
name: "my.project",
sapInternal: true
}
};

await preprocessor.checkProjectMetadata(parent, project);

t.is(logWarnSpy.callCount, 0, "No warning got logged");
mock.stop("@ui5/logger");
});

0 comments on commit efcd106

Please sign in to comment.