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

[BREAKING] Make namespace mandatory for application and library projects #430

Merged
merged 9 commits into from
Mar 20, 2020
8 changes: 1 addition & 7 deletions lib/types/application/ApplicationFormatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,7 @@ class ApplicationFormatter extends AbstractUi5Formatter {
"/": project.resources.configuration.paths.webapp
};

try {
project.metadata.namespace = await this.getNamespace();
} catch (err) {
// Catch error because namespace is optional
// TODO 2.0: Make namespace mandatory and just let the error throw
log.warn(err.message);
}
project.metadata.namespace = await this.getNamespace();
}

/**
Expand Down
8 changes: 1 addition & 7 deletions lib/types/library/LibraryFormatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,7 @@ class LibraryFormatter extends AbstractUi5Formatter {
"Either no setting was provided or the path not found.");
}

try {
project.metadata.namespace = await this.getNamespace();
} catch (err) {
// Catch error because namespace is optional
// TODO 2.0: Make namespace mandatory and just let the error throw
log.warn(err.message);
}
project.metadata.namespace = await this.getNamespace();

try {
project.metadata.copyright = await this.getCopyright();
Expand Down
30 changes: 19 additions & 11 deletions test/lib/types/application/ApplicationFormatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,24 @@ function createMockProject() {
};
}

test("getSourceBasePath: posix", async (t) => {
const myProject = clone(applicationBTree);
myProject.path = "my/pony";
const applicationFormatter = new ApplicationFormatter({project: myProject});

const sourceBasePath = applicationFormatter.getSourceBasePath(true);
t.is(sourceBasePath, "my/pony/webapp", "correct path");
});

test("format: No 'sap.app' configuration found", async (t) => {
const project = createMockProject();
const applicationFormatter = new ApplicationFormatter({project});
sinon.stub(applicationFormatter, "validate").resolves();
sinon.stub(applicationFormatter, "getManifest").resolves({content: {}, fsPath: {}});

await applicationFormatter.format();
t.deepEqual(project.resources.pathMappings["/"], "webapp", "path mappings is set");
t.falsy(project.metadata.namespace,
"namespace is falsy since getManifest resolves with an empty object");
const error = await t.throwsAsync(applicationFormatter.format());
t.deepEqual(error.message, "No \"sap.app\" ID configuration found in manifest.json of project projectName",
"Rejected with correct error message");
});

test("format: No application id in 'sap.app' configuration found", async (t) => {
Expand All @@ -148,10 +156,9 @@ test("format: No application id in 'sap.app' configuration found", async (t) =>
sinon.stub(applicationFormatter, "validate").resolves();
sinon.stub(applicationFormatter, "getManifest").resolves({content: {"sap.app": {}}});

await applicationFormatter.format();
const error = await t.throwsAsync(applicationFormatter.format());
t.deepEqual(error.message, "No \"sap.app\" ID configuration found in manifest.json of project projectName");
t.deepEqual(project.resources.pathMappings["/"], "webapp", "path mappings is set");
t.falsy(project.metadata.namespace,
"namespace is falsy since getManifest resolves with an empty object");
});

test("format: set namespace to id", async (t) => {
Expand Down Expand Up @@ -220,11 +227,12 @@ test.serial("getManifest: result is cached", async (t) => {

const ApplicationFormatter = mock.reRequire("../../../../lib/types/application/ApplicationFormatter");
const libraryFormatter = new ApplicationFormatter({project: myProject});

const expectedPath = path.join(applicationBPath, "webapp", "manifest.json");

const {content, fsPath} = await libraryFormatter.getManifest();
t.deepEqual(content, {pony: "no unicorn"}, "Correct result on first call");
t.deepEqual(fsPath, expectedPath, "Correct manifest.json path returned on first call");

const {content: content2, fsPath: fsPath2} = await libraryFormatter.getManifest();
t.deepEqual(content2, {pony: "no unicorn"}, "Correct result on second call");
t.deepEqual(fsPath2, expectedPath, "Correct manifest.json path returned on second call");
Expand Down Expand Up @@ -278,7 +286,7 @@ test("namespace: detect namespace from pom.xml via ${appId} from properties", as
myProject.resources.configuration.paths.webapp = "webapp-properties.appId";
const applicationFormatter = new ApplicationFormatter({project: myProject});

await applicationFormatter.format();
t.falsy(myProject.metadata.namespace,
"namespace is falsy since getManifest resolves with an empty object");
const error = await t.throwsAsync(applicationFormatter.format());
t.deepEqual(error.message, "Failed to resolve namespace of project application.h: \"${appId}\"" +
" couldn't be resolved from maven property \"appId\" of pom.xml of project application.h");
});
92 changes: 80 additions & 12 deletions test/lib/types/library/LibraryFormatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,34 @@ test("format: copyright already configured", async (t) => {
t.deepEqual(myProject.metadata.copyright, libraryETree.metadata.copyright, "Copyright was not altered");
});

test.serial("format: copyright retrieval fails", async (t) => {
const myProject = clone(libraryETree);

const log = require("@ui5/logger");
const loggerInstance = log.getLogger("types:library:LibraryFormatter");

mock("@ui5/logger", {
getLogger: () => loggerInstance
});
mock.reRequire("@ui5/logger");
const loggerVerboseSpy = sinon.spy(loggerInstance, "verbose");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});
sinon.stub(libraryFormatter, "validate").resolves();
sinon.stub(libraryFormatter, "getCopyright").rejects(Error("my-pony"));

await libraryFormatter.format();
t.deepEqual(myProject.metadata.copyright, libraryETree.metadata.copyright, "Copyright was not altered");


t.is(loggerVerboseSpy.callCount, 4, "calls to verbose");
t.is(loggerVerboseSpy.getCall(3).args[0], "my-pony", "message from rejection");

mock.stop("@ui5/logger");
});

test("format: formats correctly", async (t) => {
const myProject = clone(libraryETree);
myProject.metadata.copyright = undefined;
Expand Down Expand Up @@ -194,20 +222,22 @@ test.serial("format: namespace resolution fails", async (t) => {
});
mock.reRequire("@ui5/logger");
const loggerVerboseSpy = sinon.spy(loggerInstance, "verbose");
const loggerWarnSpy = sinon.spy(loggerInstance, "warn");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});
sinon.stub(libraryFormatter, "validate").resolves();


await libraryFormatter.format();
const error = await t.throwsAsync(libraryFormatter.format());
t.deepEqual(error.message, "Failed to detect namespace or namespace is empty for project library.e." +
" Check verbose log for details.");

t.deepEqual(globbyStub.callCount, 3, "globby got called three times");
t.deepEqual(globbyStub.getCall(0).args[0], "**/manifest.json", "First glob is for manifest.json files");
t.deepEqual(globbyStub.getCall(1).args[0], "**/.library", "Second glob is for .library files");
t.deepEqual(globbyStub.getCall(2).args[0], "**/library.js", "Third glob for library.js files");
t.deepEqual(loggerVerboseSpy.callCount, 7, "7 calls to log.verbose should be done");
t.deepEqual(loggerVerboseSpy.callCount, 6, "7 calls to log.verbose should be done");
const logVerboseCalls = loggerVerboseSpy.getCalls().map((call) => call.args[0]);

t.true(logVerboseCalls.includes(
Expand All @@ -225,12 +255,6 @@ test.serial("format: namespace resolution fails", async (t) => {
"Could not find library.js file for project library.e"),
"should contain message for missing library.js");

t.deepEqual(loggerWarnSpy.callCount, 1, "1 calls to log.warn should be done");
const logWarnCalls = loggerWarnSpy.getCalls().map((call) => call.args[0]);
t.true(logWarnCalls.includes(
"Failed to detect namespace or namespace is empty for project library.e. Check verbose log for details."),
"should contain message for .library");

mock.stop("globby");
mock.stop("@ui5/logger");
});
Expand Down Expand Up @@ -313,7 +337,6 @@ test.serial("getDotLibrary: result is cached", async (t) => {
mock.reRequire("globby");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});

const {content, fsPath} = await libraryFormatter.getDotLibrary();
Expand All @@ -322,6 +345,13 @@ test.serial("getDotLibrary: result is cached", async (t) => {
myProject.resources.configuration.paths.src, "library", "e", ".library");
t.deepEqual(fsPath, expectedPath, ".library fsPath is correct");

const {content: contentSecondCall, fsPath: fsPathSecondCall} = await libraryFormatter.getDotLibrary();
t.deepEqual(contentSecondCall.library.name, "library.e", ".library content has been read," +
"but should be cached now.");
const expectedPathSecondCall = path.join(myProject.path,
myProject.resources.configuration.paths.src, "library", "e", ".library");
t.deepEqual(fsPathSecondCall, expectedPathSecondCall, ".library fsPath is correct");

t.deepEqual(globbySpy.callCount, 1,
"globby got called exactly once (and then cached)");
mock.stop("globby");
Expand Down Expand Up @@ -402,6 +432,11 @@ test.serial("getLibraryJsPath: result is cached", async (t) => {
myProject.resources.configuration.paths.src, "library", "e", "library.js");
t.deepEqual(fsPath, expectedPath, ".library fsPath is correct");

const fsPathSecondCall = await libraryFormatter.getLibraryJsPath();
const expectedPathSecondCall = path.join(myProject.path,
myProject.resources.configuration.paths.src, "library", "e", "library.js");
t.deepEqual(fsPathSecondCall, expectedPathSecondCall, ".library fsPath is correct");

t.deepEqual(globbySpy.callCount, 1,
"globby got called exactly once (and then cached)");
mock.stop("globby");
Expand Down Expand Up @@ -497,8 +532,41 @@ test("getNamespace: from manifest.json with not matching file path", async (t) =
const err = await t.throwsAsync(libraryFormatter.getNamespace());

t.deepEqual(err.message, `Detected namespace "mani-pony" does not match detected directory structure ` +
`"different/namespace" for project library.e`,
"Rejected with correct error message");
`"different/namespace" for project library.e`, "Rejected with correct error message");
});

test.serial("getNamespace: from manifest.json without sap.app id", async (t) => {
const myProject = clone(libraryETree);

const log = require("@ui5/logger");
const loggerInstance = log.getLogger("types:library:LibraryFormatter");

mock("@ui5/logger", {
getLogger: () => loggerInstance
});
mock.reRequire("@ui5/logger");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});
sinon.stub(libraryFormatter, "getManifest").resolves({
content: {
"sap.app": {
}
},
fsPath: path.normalize("/some/path/different/namespace/manifest.json") // normalize for windows
});
sinon.stub(libraryFormatter, "getSourceBasePath").returns("/some/path/");

const loggerSpy = sinon.spy(loggerInstance, "verbose");
const err = await t.throwsAsync(libraryFormatter.getNamespace());

t.deepEqual(err.message, `Failed to detect namespace or namespace is empty for project library.e. Check verbose log for details.`, "Rejected with correct error message");
t.is(loggerSpy.callCount, 4, "calls to verbose");


t.is(loggerSpy.getCall(0).args[0], "No \"sap.app\" ID configuration found in manifest.json of project library.e", "correct verbose message");
mock.stop("@ui5/logger");
});

test("getNamespace: from .library", async (t) => {
Expand Down