From 11f823a77e72cfa4c096e7e8f4277a6a6b9400b8 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 26 Feb 2021 09:00:30 +0100 Subject: [PATCH] [FIX] manifestCreator: 'embeds' should list all components (#575) Field embeds in created manifest.json file now contains all nested manifests, independent from the reverse field embeddedBy (embeddedBy points from an embedded manifest to the library's manifest). Remove unreachable code (in createSapUI5() -> getUI5Version() function) . Use path.dirname method to get directory name in findEmbeddedComponents() function. Use string for version value in manifest (in sectionVersion() function). e.g. "1.2.3" instead of the semver object's string representation. It reverts changes which were introduced with #555 Successor of #554 --- lib/processors/manifestCreator.js | 71 ++++---------------- test/lib/processors/manifestCreator.js | 91 ++++++++++++++++---------- 2 files changed, 68 insertions(+), 94 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index e1275318c..35563f4da 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -177,13 +177,13 @@ 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 } - async function createSapApp() { - async function isEmbeddedByLibrary(componentPath, libraryPathPrefix) { + function createSapApp() { + 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() { + 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) && 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'. " + @@ -358,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() @@ -429,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" @@ -689,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(), @@ -701,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); diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index 9e853085b..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; @@ -696,7 +728,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 +740,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 +814,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 +826,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 +904,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 +916,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 +994,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 +1006,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 +1077,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 +1088,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 +1161,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 +1248,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 +1315,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) => {