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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] projectPreprocessor: Log warning when using a deprecated or restricted dependency #268

Merged
merged 2 commits into from
Mar 9, 2020
Merged

[FEATURE] projectPreprocessor: Log warning when using a deprecated or restricted dependency #268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b15aae3
[FEATURE] projectPreprocessor: Log warning when using a deprecated or…
RandomByte Mar 5, 2020
3888f71
[INTERNAL] projectPreprocessor tests: Use mock.stopAll()
RandomByte Mar 9, 2020
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
33 changes: 25 additions & 8 deletions lib/projectPreprocessor.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also use mock.stopAll() within test.afterEach.always to ensure a proper cleanup

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

});

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");
});