From efcd106efafe6f6a1f553a7dabe76952fc4f9653 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Thu, 5 Mar 2020 15:24:13 +0100 Subject: [PATCH] [FEATURE] projectPreprocessor: Log warning when using a deprecated or 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". --- lib/projectPreprocessor.js | 33 +++++-- test/lib/extensions.js | 12 +-- test/lib/projectPreprocessor.js | 162 ++++++++++++++++++++++++++++++-- 3 files changed, 187 insertions(+), 20 deletions(-) diff --git a/lib/projectPreprocessor.js b/lib/projectPreprocessor.js index 82625e979..5a59f85ac 100644 --- a/lib/projectPreprocessor.js +++ b/lib/projectPreprocessor.js @@ -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 = {}; @@ -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 }]; @@ -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 = []; } @@ -81,6 +83,7 @@ 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], @@ -88,7 +91,7 @@ class ProjectPreprocessor { 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 @@ -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; }); } @@ -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}`); @@ -534,7 +551,7 @@ module.exports = { * @returns {Promise} 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 }; diff --git a/test/lib/extensions.js b/test/lib/extensions.js index 7138cbd45..a1c283505 100644 --- a/test/lib/extensions.js +++ b/test/lib/extensions.js @@ -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"); @@ -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"); @@ -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 " + @@ -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"); @@ -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"); @@ -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"); diff --git a/test/lib/projectPreprocessor.js b/test/lib/projectPreprocessor.js index d88b8e20f..ba338a65c 100644 --- a/test/lib/projectPreprocessor.js +++ b/test/lib/projectPreprocessor.js @@ -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"); @@ -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", @@ -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 = {}; @@ -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: { @@ -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: { @@ -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: { @@ -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 = {}; @@ -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"); +});