From 2d2e31edfd94283a4ab024de9d48b39e252d2cd4 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 30 Nov 2020 11:04:03 +0100 Subject: [PATCH 1/5] [FIX] manifestCreator: Only list components with corresponding 'embeddedBy' --- lib/processors/manifestCreator.js | 84 +++++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 10 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index c903ee321..42f1d27b2 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -155,24 +155,83 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in // return undefined } - function createSapApp() { - function findComponentPaths() { + async function createSapApp() { + async function isEmbeddedByLibrary(componentPath, libraryPathPrefix) { + const manifestPath = componentPath + "/manifest.json"; + + const manifestResource = libBundle.findResource(manifestPath.substring(libraryPathPrefix.length)); + if ( manifestResource == null ) { + 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"] && typeof manifest["sap.app"]["embeddedBy"] !== "undefined") { + embeddedBy = manifest["sap.app"]["embeddedBy"]; + } + if (embeddedBy == null) { + 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'", 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'" + ); + return false; + } + let resolvedEmbeddedBy = resolvePath(embeddedBy, componentPath); + 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; + } + } + + async function findEmbeddedComponents() { const result = []; const prefix = libraryResource.getPath().slice(0, - ".library".length); const components = libBundle.getResources(/^\/(?:[^/]+\/)*Component\.js$/); - components.forEach((comp) => { - const relativePath = comp.getPath().slice(prefix.length); + for (const comp of components) { + const componentPath = comp.getPath().slice(0, - "Component.js".length - 1); + log.verbose("checking component at %s", componentPath); + const relativePath = componentPath.slice(prefix.length); if ( relativePath.lastIndexOf("/") >= 0 ) { - result.push( relativePath.slice(0, relativePath.lastIndexOf("/")) ); + if (await isEmbeddedByLibrary(componentPath, prefix)) { + result.push( relativePath.slice(0, relativePath.lastIndexOf("/")) ); + } } else if ( prefix === "/resources/sap/apf/" ) { log.verbose("Package %s contains both '*.library' and 'Component.js'. " + - "This is a known issue but can't be solved due to backward compatibility.", comp.getPath()); + "This is a known issue but can't be solved due to backward compatibility.", componentPath); } else if ( prefix !== "/resources/sap/ui/core/" ) { log.error("Package %s contains both '*.library' and 'Component.js'. " + "This is not supported by manifests, therefore the component won't be " + - "listed in the library's manifest.", comp.getPath()); + "listed in the library's manifest.", componentPath); } - }); + } return result.sort(); } @@ -266,11 +325,16 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in return osComponents.length > 0 ? osComponents : undefined; } + function resolvePath(relativePath, basePath) { + const posixPath = require("path").posix; + return posixPath.resolve(basePath, relativePath); + } + const sapApp = { _version: sectionVersion(APP_DESCRIPTOR_V3_SECTION_SAP_APP), id: library.getName(), type: "library", - embeds: findComponentPaths(), + embeds: await findEmbeddedComponents(), i18n: getChildTextContent(manifestAppData, "i18n"), applicationVersion: { version: isValid(library.getVersion()) ? library.getVersion() : getProjectVersion() @@ -544,7 +608,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in return { "_version": descriptorVersion.toString(), - "sap.app": createSapApp(), + "sap.app": await createSapApp(), "sap.ui": createSapUi(), "sap.ui5": createSapUI5(), "sap.fiori": createSapFiori(), From 7db8304c2b3d9bb861666f9518d7e42ae7e00542 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 30 Nov 2020 11:04:19 +0100 Subject: [PATCH 2/5] [INTERNAL] WIP: Adopt tests --- test/lib/processors/manifestCreator.js | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index a11ea275c..9160a818d 100644 --- a/test/lib/processors/manifestCreator.js +++ b/test/lib/processors/manifestCreator.js @@ -116,3 +116,89 @@ test.serial("manifest creation for sap/apf", async (t) => { "This is a known issue but can't be solved due to backward compatibility."); t.is(verboseLogStub.firstCall.args[1], "/resources/sap/apf/Component.js"); }); + +test.serial("manifest creation with embedded component", async (t) => { + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + // const logger = require("@ui5/logger"); + // const verboseLogStub = sinon.stub(); + // const myLoggerInstance = { + // verbose: verboseLogStub + // }; + // sinon.stub(logger, "getLogger").returns(myLoggerInstance); + const manifestCreatorWithStub = mock.reRequire("../../../lib/processors/manifestCreator"); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return JSON.stringify({ + "sap.app": { + "embeddedBy": "../" + } + }); + } + }; + + const result = await manifestCreatorWithStub({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + // t.is(verboseLogStub.callCount, 8); +}); From f678c3437414bbc715c7b566a64cf4e3f2b42672 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 30 Nov 2020 12:01:25 +0100 Subject: [PATCH 3/5] [INTERNAL] Fix tests --- lib/processors/manifestCreator.js | 9 ++--- .../resources/library/h/manifest.json | 7 +--- .../resources/library/h/resources.json | 2 +- .../dest/resources/library/h/manifest.json | 7 +--- test/lib/processors/manifestCreator.js | 34 +++++++++++++------ 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index 42f1d27b2..40464dcd6 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -218,15 +218,12 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in for (const comp of components) { const componentPath = comp.getPath().slice(0, - "Component.js".length - 1); log.verbose("checking component at %s", componentPath); - const relativePath = componentPath.slice(prefix.length); - if ( relativePath.lastIndexOf("/") >= 0 ) { - if (await isEmbeddedByLibrary(componentPath, prefix)) { - result.push( relativePath.slice(0, relativePath.lastIndexOf("/")) ); - } + if ( componentPath.startsWith(prefix) && await isEmbeddedByLibrary(componentPath, prefix) ) { + result.push( componentPath.substring(prefix.length) ); } else if ( prefix === "/resources/sap/apf/" ) { log.verbose("Package %s contains both '*.library' and 'Component.js'. " + "This is a known issue but can't be solved due to backward compatibility.", componentPath); - } else if ( prefix !== "/resources/sap/ui/core/" ) { + } else if ( prefix === (componentPath + "/") && prefix !== "/resources/sap/ui/core/" ) { log.error("Package %s contains both '*.library' and 'Component.js'. " + "This is not supported by manifests, therefore the component won't be " + "listed in the library's manifest.", componentPath); diff --git a/test/expected/build/library.h/dest-resources-json/resources/library/h/manifest.json b/test/expected/build/library.h/dest-resources-json/resources/library/h/manifest.json index 2b6c457ad..de1590b80 100644 --- a/test/expected/build/library.h/dest-resources-json/resources/library/h/manifest.json +++ b/test/expected/build/library.h/dest-resources-json/resources/library/h/manifest.json @@ -3,12 +3,7 @@ "sap.app": { "id": "library.h", "type": "library", - "embeds": [ - "components", - "components/subcomponent1", - "components/subcomponent2", - "components/subcomponent3" - ], + "embeds": [], "applicationVersion": { "version": "1.0.0" }, diff --git a/test/expected/build/library.h/dest-resources-json/resources/library/h/resources.json b/test/expected/build/library.h/dest-resources-json/resources/library/h/resources.json index d0820dcf4..f3e80a739 100644 --- a/test/expected/build/library.h/dest-resources-json/resources/library/h/resources.json +++ b/test/expected/build/library.h/dest-resources-json/resources/library/h/resources.json @@ -133,7 +133,7 @@ { "name": "manifest.json", "module": "library/h/manifest.json", - "size": 739 + "size": 613 }, { "name": "not.js", diff --git a/test/expected/build/library.h/dest/resources/library/h/manifest.json b/test/expected/build/library.h/dest/resources/library/h/manifest.json index 2b6c457ad..de1590b80 100644 --- a/test/expected/build/library.h/dest/resources/library/h/manifest.json +++ b/test/expected/build/library.h/dest/resources/library/h/manifest.json @@ -3,12 +3,7 @@ "sap.app": { "id": "library.h", "type": "library", - "embeds": [ - "components", - "components/subcomponent1", - "components/subcomponent2", - "components/subcomponent3" - ], + "embeds": [], "applicationVersion": { "version": "1.0.0" }, diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index 9160a818d..0eb9c60f6 100644 --- a/test/lib/processors/manifestCreator.js +++ b/test/lib/processors/manifestCreator.js @@ -110,11 +110,11 @@ test.serial("manifest creation for sap/apf", async (t) => { const result = await manifestCreatorWithStub({libraryResource, resources: [componentResource], options: {}}); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); - t.is(verboseLogStub.callCount, 8); - t.is(verboseLogStub.firstCall.args[0], + t.is(verboseLogStub.callCount, 9); + t.is(verboseLogStub.getCall(1).args[0], "Package %s contains both '*.library' and 'Component.js'. " + "This is a known issue but can't be solved due to backward compatibility."); - t.is(verboseLogStub.firstCall.args[1], "/resources/sap/apf/Component.js"); + t.is(verboseLogStub.getCall(1).args[1], "/resources/sap/apf"); }); test.serial("manifest creation with embedded component", async (t) => { @@ -123,7 +123,9 @@ test.serial("manifest creation with embedded component", async (t) => { "sap.app": { "id": "sap.lib1", "type": "library", - "embeds": [], + "embeds": [ + "component1" + ], "applicationVersion": { "version": "1.0.0" }, @@ -145,12 +147,14 @@ test.serial("manifest creation with embedded component", async (t) => { } }, null, 2); - // const logger = require("@ui5/logger"); - // const verboseLogStub = sinon.stub(); - // const myLoggerInstance = { - // verbose: verboseLogStub - // }; - // sinon.stub(logger, "getLogger").returns(myLoggerInstance); + const logger = require("@ui5/logger"); + const verboseLogStub = sinon.stub(); + const errorLogStub = sinon.stub(); + const myLoggerInstance = { + verbose: verboseLogStub, + error: errorLogStub + }; + sinon.stub(logger, "getLogger").returns(myLoggerInstance); const manifestCreatorWithStub = mock.reRequire("../../../lib/processors/manifestCreator"); const libraryResource = { @@ -200,5 +204,13 @@ test.serial("manifest creation with embedded component", async (t) => { }); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); - // t.is(verboseLogStub.callCount, 8); + t.is(errorLogStub.callCount, 0); + + t.true(verboseLogStub.callCount >= 2, "There should be at least 2 verbose log calls"); + t.deepEqual(verboseLogStub.getCall(0).args, [ + "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'" + ]); }); From 7c8573aad35f0fcd3edf8f38fd0b4b664420c8c8 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 30 Nov 2020 13:50:15 +0100 Subject: [PATCH 4/5] [INTERNAL] Add more test cases --- lib/processors/manifestCreator.js | 8 +- test/lib/processors/manifestCreator.js | 779 ++++++++++++++++++++++++- 2 files changed, 758 insertions(+), 29 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index 40464dcd6..a5ccbc60b 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -1,5 +1,6 @@ "use strict"; +const posixPath = require("path").posix; const {SemVer: Version} = require("semver"); const log = require("@ui5/logger").getLogger("builder:processors:manifestCreator"); const EvoResource = require("@ui5/fs").Resource; @@ -196,7 +197,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in ); return false; } - let resolvedEmbeddedBy = resolvePath(embeddedBy, componentPath); + let resolvedEmbeddedBy = posixPath.resolve(componentPath, embeddedBy); if ( resolvedEmbeddedBy && !resolvedEmbeddedBy.endsWith("/") ) { resolvedEmbeddedBy = resolvedEmbeddedBy + "/"; } @@ -322,11 +323,6 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in return osComponents.length > 0 ? osComponents : undefined; } - function resolvePath(relativePath, basePath) { - const posixPath = require("path").posix; - return posixPath.resolve(basePath, relativePath); - } - const sapApp = { _version: sectionVersion(APP_DESCRIPTOR_V3_SECTION_SAP_APP), id: library.getName(), diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index 0eb9c60f6..1cdc41fd0 100644 --- a/test/lib/processors/manifestCreator.js +++ b/test/lib/processors/manifestCreator.js @@ -1,8 +1,7 @@ const test = require("ava"); const sinon = require("sinon"); const mock = require("mock-require"); - -const manifestCreator = require("../../../lib/processors/manifestCreator"); +const logger = require("@ui5/logger"); const libraryContent = ` @@ -49,12 +48,24 @@ const expectedManifestContent = `{ } }`; +test.beforeEach((t) => { + t.context.verboseLogStub = sinon.stub(); + t.context.errorLogStub = sinon.stub(); + sinon.stub(logger, "getLogger").returns({ + verbose: t.context.verboseLogStub, + error: t.context.errorLogStub + }); + t.context.manifestCreator = mock.reRequire("../../../lib/processors/manifestCreator"); +}); + test.afterEach.always((t) => { mock.stopAll(); sinon.restore(); }); -test("default manifest creation", async (t) => { +test.serial("default manifest creation", async (t) => { + const {manifestCreator, errorLogStub} = t.context; + const libraryResource = { getPath: () => { return "/resources/sap/ui/mine/.library"; @@ -71,19 +82,14 @@ test("default manifest creation", async (t) => { } }; + t.is(errorLogStub.callCount, 0); + const result = await manifestCreator({libraryResource, resources: [], options: {}}); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); }); test.serial("manifest creation for sap/apf", async (t) => { - const logger = require("@ui5/logger"); - const verboseLogStub = sinon.stub(); - const myLoggerInstance = { - verbose: verboseLogStub - }; - sinon.stub(logger, "getLogger").returns(myLoggerInstance); - const manifestCreatorWithStub = mock.reRequire("../../../lib/processors/manifestCreator"); - + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; const libraryResource = { getPath: () => { @@ -107,9 +113,11 @@ test.serial("manifest creation for sap/apf", async (t) => { } }; - const result = await manifestCreatorWithStub({libraryResource, resources: [componentResource], options: {}}); + const result = await manifestCreator({libraryResource, resources: [componentResource], options: {}}); t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + t.is(errorLogStub.callCount, 0); + t.is(verboseLogStub.callCount, 9); t.is(verboseLogStub.getCall(1).args[0], "Package %s contains both '*.library' and 'Component.js'. " + @@ -117,7 +125,147 @@ test.serial("manifest creation for sap/apf", async (t) => { t.is(verboseLogStub.getCall(1).args[1], "/resources/sap/apf"); }); +test.serial("manifest creation for sap/ui/core", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.ui.core", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.ui.core", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/ui/core/.library"; + }, + getString: async () => { + return ` + + sap.ui.core + 1.0.0 + `; + }, + _project: { + metadata: { + name: "sap.ui.core" + } + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/ui/core/Component.js"; + } + }; + + const result = await manifestCreator({libraryResource, resources: [componentResource], options: {}}); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + t.is(errorLogStub.callCount, 0); + + t.is(verboseLogStub.callCount, 8); + t.is(verboseLogStub.getCall(1).args[0], + " sap.app/id taken from .library: '%s'"); + t.is(verboseLogStub.getCall(1).args[1], "sap.ui.core"); +}); + +test.serial("manifest creation with .library / Component.js at same namespace", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/Component.js"; + } + }; + + const result = await manifestCreator({libraryResource, resources: [componentResource], options: {}}); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + t.is(errorLogStub.callCount, 1); + t.deepEqual(errorLogStub.getCall(0).args, [ + "Package %s contains both '*.library' and 'Component.js'. " + + "This is not supported by manifests, therefore the component won't be " + + "listed in the library's manifest.", + "/resources/sap/lib1" + ]); + + t.is(verboseLogStub.callCount, 8); + t.is(verboseLogStub.getCall(1).args[0], + " sap.app/id taken from .library: '%s'"); + t.is(verboseLogStub.getCall(1).args[1], "sap.lib1"); +}); + test.serial("manifest creation with embedded component", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + const expectedManifestContent = JSON.stringify({ "_version": "1.9.0", "sap.app": { @@ -147,16 +295,6 @@ test.serial("manifest creation with embedded component", async (t) => { } }, null, 2); - const logger = require("@ui5/logger"); - const verboseLogStub = sinon.stub(); - const errorLogStub = sinon.stub(); - const myLoggerInstance = { - verbose: verboseLogStub, - error: errorLogStub - }; - sinon.stub(logger, "getLogger").returns(myLoggerInstance); - const manifestCreatorWithStub = mock.reRequire("../../../lib/processors/manifestCreator"); - const libraryResource = { getPath: () => { return "/resources/sap/lib1/.library"; @@ -195,7 +333,7 @@ test.serial("manifest creation with embedded component", async (t) => { } }; - const result = await manifestCreatorWithStub({ + const result = await manifestCreator({ libraryResource, resources: [ componentResource, @@ -214,3 +352,598 @@ test.serial("manifest creation with embedded component", async (t) => { " component's 'sap.app/embeddedBy' property points to library, list it as 'embedded'" ]); }); + +test.serial("manifest creation with embedded component (Missing 'embeddedBy')", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return JSON.stringify({ + "sap.app": {} + }); + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + t.is(errorLogStub.callCount, 0); + + t.true(verboseLogStub.callCount >= 2, "There should be at least 2 verbose log calls"); + t.deepEqual(verboseLogStub.getCall(0).args, [ + "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'" + ]); +}); + +test.serial("manifest creation with embedded component ('embeddedBy' doesn't point to library)", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return JSON.stringify({ + "sap.app": { + "embeddedBy": "../foo/" + } + }); + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + t.is(errorLogStub.callCount, 0); + + t.true(verboseLogStub.callCount >= 2, "There should be at least 2 verbose log calls"); + t.deepEqual(verboseLogStub.getCall(0).args, [ + "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/" + ]); +}); + +test.serial("manifest creation with embedded component ('embeddedBy' absolute path)", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return JSON.stringify({ + "sap.app": { + "embeddedBy": "/" + } + }); + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + t.is(errorLogStub.callCount, 0); + + t.true(verboseLogStub.callCount >= 2, "There should be at least 2 verbose log calls"); + t.deepEqual(verboseLogStub.getCall(0).args, [ + "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'", + "/" + ]); +}); + +test.serial("manifest creation with embedded component ('embeddedBy' empty string)", async (t) => { + const {manifestCreator, errorLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return JSON.stringify({ + "sap.app": { + "embeddedBy": "" + } + }); + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + 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'" + ]); +}); + +test.serial("manifest creation with embedded component ('embeddedBy' object)", async (t) => { + const {manifestCreator, errorLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return JSON.stringify({ + "sap.app": { + "embeddedBy": { + "foo": "bar" + } + } + }); + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + 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'", + "object" + ]); +}); + +test.serial("manifest creation with embedded component (no manifest.json)", async (t) => { + const {manifestCreator, errorLogStub, verboseLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource + ] + }); + t.is(await result.getString(), expectedManifestContent, "Correct result returned"); + + t.is(errorLogStub.callCount, 0); + + t.true(verboseLogStub.callCount >= 2, "There should be at least 2 verbose log calls"); + t.deepEqual(verboseLogStub.getCall(0).args, [ + "checking component at %s", + "/resources/sap/lib1/component1" + ]); + t.deepEqual(verboseLogStub.getCall(1).args, [ + " component has no accompanying manifest.json, don't list it as 'embedded'" + ]); +}); + +test.serial("manifest creation with embedded component (invalid manifest.json)", async (t) => { + const {manifestCreator, errorLogStub} = t.context; + + const expectedManifestContent = JSON.stringify({ + "_version": "1.9.0", + "sap.app": { + "id": "sap.lib1", + "type": "library", + "embeds": [], + "applicationVersion": { + "version": "1.0.0" + }, + "title": "sap.lib1", + "resources": "resources.json", + "offline": true + }, + "sap.ui": { + "technology": "UI5", + "supportedThemes": [] + }, + "sap.ui5": { + "dependencies": { + "libs": {} + }, + "library": { + "i18n": false + } + } + }, null, 2); + + const libraryResource = { + getPath: () => { + return "/resources/sap/lib1/.library"; + }, + getString: async () => { + return ` + + sap.lib1 + 1.0.0 + `; + }, + _project: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + } + }; + + const componentResource = { + getPath: () => { + return "/resources/sap/lib1/component1/Component.js"; + } + }; + const componentManifestResource = { + getPath: () => { + return "/resources/sap/lib1/component1/manifest.json"; + }, + getString: async () => { + return "{invalid}"; + } + }; + + const result = await manifestCreator({ + libraryResource, + resources: [ + componentResource, + componentManifestResource + ] + }); + 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")); +}); From 75377ac0ae0c62bc1c145defee8fb38abfcd0963 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 30 Nov 2020 15:32:43 +0100 Subject: [PATCH 5/5] [INTERNAL] Fix embeddedBy check --- lib/processors/manifestCreator.js | 4 ++-- test/lib/processors/manifestCreator.js | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js index a5ccbc60b..1d627d3d9 100644 --- a/lib/processors/manifestCreator.js +++ b/lib/processors/manifestCreator.js @@ -176,10 +176,10 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in return false; } let embeddedBy; - if (manifest && manifest["sap.app"] && typeof manifest["sap.app"]["embeddedBy"] !== "undefined") { + if (manifest && manifest["sap.app"]) { embeddedBy = manifest["sap.app"]["embeddedBy"]; } - if (embeddedBy == null) { + if (typeof embeddedBy === "undefined") { log.verbose(" component doesn't declare 'sap.app/embeddedBy', don't list it as 'embedded'"); return false; } diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js index 1cdc41fd0..d1b36f512 100644 --- a/test/lib/processors/manifestCreator.js +++ b/test/lib/processors/manifestCreator.js @@ -413,9 +413,7 @@ test.serial("manifest creation with embedded component (Missing 'embeddedBy')", return "/resources/sap/lib1/component1/manifest.json"; }, getString: async () => { - return JSON.stringify({ - "sap.app": {} - }); + return JSON.stringify({}); } };