Skip to content

Commit

Permalink
[BREAKING] Make namespace mandatory for application and library proje…
Browse files Browse the repository at this point in the history 
…cts (#430)

[BREAKING] Make namespace mandatory for application and library projects
BREAKING CHANGE:
UI5 Project must be able to determine the project's namespace,
otherwise an error is thrown.
  • Loading branch information
@svbender
svbender authored Mar 20, 2020
1 parent 3f31a5d commit ee96c00
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 37 deletions.
8 changes: 1 addition & 7 deletions lib/types/application/ApplicationFormatter.js
View file
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
View file
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
View file
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
View file
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

0 comments on commit ee96c00

Please sign in to comment.