From ac88f7249dc8c0f7915495cc47c15868f18133f7 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Wed, 27 Jan 2021 13:47:28 +0100 Subject: [PATCH 1/5] [FEATURE] manifestCreator: embeds field embeds field in created manifest.json now contains all nested manifests independent from the reverse field embeddedBy, which points from an embedded manifest to the library's manifest. --- lib/processors/manifestCreator.js | 54 +++-------------------- test/lib/processors/manifestCreator.js | 59 +++++++++++--------------- 2 files changed, 30 insertions(+), 83 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index 92696b323..7aa1ede39 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -183,7 +183,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in } async function createSapApp() { - async function isEmbeddedByLibrary(componentPath, libraryPathPrefix) { + async function hasManifest(componentPath, libraryPathPrefix) { const manifestPath = componentPath + "/manifest.json"; const manifestResource = libBundle.findResource(manifestPath.substring(libraryPathPrefix.length)); @@ -191,61 +191,17 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in log.verbose(" component has no accompanying manifest.json, don't list it as 'embedded'"); return false; } - const manifestString = await manifestResource.getString(); - let manifest; - try { - manifest = JSON.parse(manifestString); - } catch (err) { - log.error( - " component '%s': failed to read the component's manifest.json, " + - "it won't be listed as 'embedded'.\nError details: %s", componentPath, err.stack); - return false; - } - let embeddedBy; - if (manifest && manifest["sap.app"]) { - embeddedBy = manifest["sap.app"]["embeddedBy"]; - } - if (typeof embeddedBy === "undefined") { - log.verbose(" component doesn't declare 'sap.app/embeddedBy', don't list it as 'embedded'"); - return false; - } - if (typeof embeddedBy !== "string") { - log.error( - " component '%s': property 'sap.app/embeddedBy' is of type '%s' (expected 'string'), " + - "it won't be listed as 'embedded'", componentPath, typeof embeddedBy - ); - return false; - } - if ( !embeddedBy.length ) { - log.error( - " component '%s': property 'sap.app/embeddedBy' has an empty string value (which is invalid), " + - "it won't be listed as 'embedded'", componentPath - ); - return false; - } - let resolvedEmbeddedBy = posixPath.resolve(componentPath, embeddedBy); - if ( resolvedEmbeddedBy && !resolvedEmbeddedBy.endsWith("/") ) { - resolvedEmbeddedBy = resolvedEmbeddedBy + "/"; - } - if ( libraryPathPrefix === resolvedEmbeddedBy ) { - log.verbose(" component's 'sap.app/embeddedBy' property points to library, list it as 'embedded'"); - return true; - } else { - log.verbose( - " component's 'sap.app/embeddedBy' points to '%s', don't list it as 'embedded'", resolvedEmbeddedBy - ); - return false; - } + return true; } async function findEmbeddedComponents() { const result = []; - const prefix = libraryResource.getPath().slice(0, - ".library".length); + const prefix = posixPath.dirname(libraryResource.getPath()) + "/"; const components = libBundle.getResources(/^\/(?:[^/]+\/)*Component\.js$/); for (const comp of components) { - const componentPath = comp.getPath().slice(0, - "Component.js".length - 1); + const componentPath = posixPath.dirname(comp.getPath()); log.verbose("checking component at %s", componentPath); - if ( componentPath.startsWith(prefix) && await isEmbeddedByLibrary(componentPath, prefix) ) { + if ( componentPath.startsWith(prefix) && await hasManifest(componentPath, prefix) ) { result.push( componentPath.substring(prefix.length) ); } else if ( prefix === "/resources/sap/apf/" ) { log.verbose("Package %s contains both '*.library' and 'Component.js'. " + diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index 9e853085b..5bf26dae3 100644 --- a/test/lib/processors/manifestCreator.js +++ b/test/lib/processors/manifestCreator.js @@ -696,7 +696,7 @@ test.serial("manifest creation with embedded component", async (t) => { "checking component at %s", "/resources/sap/lib1/component1" ]); t.deepEqual(verboseLogStub.getCall(1).args, [ - " component's 'sap.app/embeddedBy' property points to library, list it as 'embedded'" + " sap.app/id taken from .library: '%s'", "sap.lib1" ]); }); @@ -708,7 +708,9 @@ test.serial("manifest creation with embedded component (Missing 'embeddedBy')", "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -780,7 +782,7 @@ test.serial("manifest creation with embedded component (Missing 'embeddedBy')", "checking component at %s", "/resources/sap/lib1/component1" ]); t.deepEqual(verboseLogStub.getCall(1).args, [ - " component doesn't declare 'sap.app/embeddedBy', don't list it as 'embedded'" + " sap.app/id taken from .library: '%s'", "sap.lib1" ]); }); @@ -792,7 +794,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' doesn't poi "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -868,8 +872,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' doesn't poi "checking component at %s", "/resources/sap/lib1/component1" ]); t.deepEqual(verboseLogStub.getCall(1).args, [ - " component's 'sap.app/embeddedBy' points to '%s', don't list it as 'embedded'", - "/resources/sap/lib1/foo/" + " sap.app/id taken from .library: '%s'", "sap.lib1" ]); }); @@ -881,7 +884,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' absolute pa "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -957,8 +962,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' absolute pa "checking component at %s", "/resources/sap/lib1/component1" ]); t.deepEqual(verboseLogStub.getCall(1).args, [ - " component's 'sap.app/embeddedBy' points to '%s', don't list it as 'embedded'", - "/" + " sap.app/id taken from .library: '%s'", "sap.lib1" ]); }); @@ -970,7 +974,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' empty strin "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -1039,12 +1045,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' empty strin }); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); - t.is(errorLogStub.callCount, 1); - t.deepEqual(errorLogStub.getCall(0).args, [ - " component '%s': property 'sap.app/embeddedBy' has an empty string value (which is invalid), " + - "it won't be listed as 'embedded'", - "/resources/sap/lib1/component1" - ]); + t.is(errorLogStub.callCount, 0); }); test.serial("manifest creation with embedded component ('embeddedBy' object)", async (t) => { @@ -1055,7 +1056,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' object)", a "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -1126,13 +1129,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' object)", a }); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); - t.is(errorLogStub.callCount, 1); - t.deepEqual(errorLogStub.getCall(0).args, [ - " component '%s': property 'sap.app/embeddedBy' is of type '%s' (expected 'string'), " + - "it won't be listed as 'embedded'", - "/resources/sap/lib1/component1", - "object" - ]); + t.is(errorLogStub.callCount, 0); }); test.serial("manifest creation with embedded component (no manifest.json)", async (t) => { @@ -1219,7 +1216,9 @@ test.serial("manifest creation with embedded component (invalid manifest.json)", "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -1284,15 +1283,7 @@ test.serial("manifest creation with embedded component (invalid manifest.json)", }); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); - t.is(errorLogStub.callCount, 1); - t.is(errorLogStub.getCall(0).args.length, 3); - t.deepEqual(errorLogStub.getCall(0).args.slice(0, 2), [ - " component '%s': failed to read the component's manifest.json, " + - "it won't be listed as 'embedded'.\n" + - "Error details: %s", - "/resources/sap/lib1/component1" - ]); - t.true(errorLogStub.getCall(0).args[2].startsWith("SyntaxError: Unexpected token")); + t.is(errorLogStub.callCount, 0); }); test.serial("manifest creation for invalid .library content", async (t) => { From fed9475dea355f7ac99814e239fd5830b17b41ef Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Tue, 2 Feb 2021 11:27:23 +0100 Subject: [PATCH 2/5] Added test for smaller descriptor version Removed invalid code in sapui5 version section (#getUI5Version) Fixed version in sections: use a version string instead of Semver version object --- lib/processors/manifestCreator.js | 7 +----- test/lib/processors/manifestCreator.js | 32 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index 7aa1ede39..762269edd 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -177,7 +177,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in function sectionVersion(candidateVersion) { // _version property for nested sections became optional with AppDescriptor V5 if ( descriptorVersion.compare(APP_DESCRIPTOR_V5) < 0 ) { - return candidateVersion; + return candidateVersion.toString(); } // return undefined } @@ -385,11 +385,6 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in function createSapUI5() { function getUI5Version() { - let ui5Version; - if ( ui5Version != null ) { - return ui5Version; - } - const dummy = new Dependency({ libraryName: [{ _: "sap.ui.core" diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index 5bf26dae3..4ee617714 100644 --- a/test/lib/processors/manifestCreator.js +++ b/test/lib/processors/manifestCreator.js @@ -422,6 +422,38 @@ test.serial("default manifest creation with special characters small app descrip t.is(await result.getString(), expectedManifestContentSmallVersionString, "Correct result returned"); }); +test.serial("default manifest creation with special characters very small app descriptor version", async (t) => { + const {manifestCreator, errorLogStub} = t.context; + const prefix = "/resources/sap/ui/mine/"; + const libraryResource = { + getPath: () => { + return prefix + ".library"; + }, + getString: async () => { + return libraryContent; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + t.is(errorLogStub.callCount, 0); + + const options = {descriptorVersion: new Version("1.1.0")}; + const result = await manifestCreator({libraryResource, resources: [], options}); + const expectedManifestContentSmallVersion = expectedManifestContentObject(); + expectedManifestContentSmallVersion["_version"] = "1.1.0"; + expectedManifestContentSmallVersion["sap.app"]["_version"] = "1.2.0"; + expectedManifestContentSmallVersion["sap.ui"]["_version"] = "1.1.0"; + expectedManifestContentSmallVersion["sap.ui5"]["_version"] = "1.1.0"; + expectedManifestContentSmallVersion["sap.app"]["i18n"] = "i18n/i18n.properties"; + const sResult = await result.getString(); + t.deepEqual(JSON.parse(sResult), expectedManifestContentSmallVersion, "Correct result returned"); +}); + test.serial("manifest creation for sap/apf", async (t) => { const {manifestCreator, errorLogStub, verboseLogStub} = t.context; From d56dd1df9c1f2bc92c1a9381754383e1a7b679f7 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Tue, 16 Feb 2021 11:55:07 +0100 Subject: [PATCH 3/5] remove async method signature for methods which do not require it --- lib/processors/manifestCreator.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index 762269edd..eb00646dc 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -183,7 +183,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in } async function createSapApp() { - async function hasManifest(componentPath, libraryPathPrefix) { + function hasManifest(componentPath, libraryPathPrefix) { const manifestPath = componentPath + "/manifest.json"; const manifestResource = libBundle.findResource(manifestPath.substring(libraryPathPrefix.length)); @@ -194,14 +194,14 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in return true; } - async function findEmbeddedComponents() { + function findEmbeddedComponents() { const result = []; const prefix = posixPath.dirname(libraryResource.getPath()) + "/"; const components = libBundle.getResources(/^\/(?:[^/]+\/)*Component\.js$/); for (const comp of components) { const componentPath = posixPath.dirname(comp.getPath()); log.verbose("checking component at %s", componentPath); - if ( componentPath.startsWith(prefix) && await hasManifest(componentPath, prefix) ) { + if ( componentPath.startsWith(prefix) && hasManifest(componentPath, prefix) ) { result.push( componentPath.substring(prefix.length) ); } else if ( prefix === "/resources/sap/apf/" ) { log.verbose("Package %s contains both '*.library' and 'Component.js'. " + @@ -314,7 +314,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in _version: sectionVersion(APP_DESCRIPTOR_V3_SECTION_SAP_APP), id: library.getName(), type: "library", - embeds: await findEmbeddedComponents(), + embeds: findEmbeddedComponents(), i18n, applicationVersion: { version: isValid(library.getVersion()) ? library.getVersion() : getProjectVersion() From f7ae8bc51970767dbf362801b8d32798162ee06f Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Tue, 16 Feb 2021 11:57:09 +0100 Subject: [PATCH 4/5] remove async method signature for methods which do not require it --- lib/processors/manifestCreator.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index eb00646dc..c92621c40 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -182,7 +182,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in // return undefined } - async function createSapApp() { + function createSapApp() { function hasManifest(componentPath, libraryPathPrefix) { const manifestPath = componentPath + "/manifest.json"; @@ -640,7 +640,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in return { "_version": descriptorVersion.toString(), - "sap.app": await createSapApp(), + "sap.app": createSapApp(), "sap.ui": createSapUi(), "sap.ui5": createSapUI5(), "sap.fiori": createSapFiori(), From 9cd131397c4ec44821980cba40da9fbc963f44fe Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Thu, 25 Feb 2021 10:35:30 +0100 Subject: [PATCH 5/5] Added TODO for descriptorVersion parameter The descriptorVersion parameter is not public API, but it makes sense to make the parameter of type string and create a SemVer object inside. --- lib/processors/manifestCreator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index c92621c40..46e0981da 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -652,7 +652,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in module.exports = function({libraryResource, resources, options}) { // merge options with defaults options = Object.assign({ - descriptorVersion: APP_DESCRIPTOR_V22, + descriptorVersion: APP_DESCRIPTOR_V22, // TODO change this to type string instead of a semver object include3rdParty: true, prettyPrint: true }, options);