From 964fa6c19b485dfa86a719a372d9b6f36ff6a4f2 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 16 Dec 2020 14:21:05 +0100 Subject: [PATCH 1/8] [FEATURE] manifestBundler: Add support for sap.app/i18n/enhanceWith --- lib/processors/bundlers/manifestBundler.js | 34 +++++++--- .../processors/bundlers/manifestBundler.js | 62 +++++++++++++++++++ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index 03359d53b..dab0d4771 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -57,20 +57,35 @@ class I18nResourceList { * @returns {Promise} Promise resolving with manifest bundle resources */ module.exports = ({resources, options: {namespace, bundleName, propertiesExtension, descriptor}}) => { - function getDescriptorI18nInfo(manifest) { + function addDescriptorI18nInfos(descriptorI18nInfos, manifest) { + function addI18nInfo(i18nPath) { + descriptorI18nInfos.set( + path.join(path.dirname(manifest.path), path.dirname(i18nPath)), + path.basename(i18nPath, propertiesExtension) + ); + } + const content = JSON.parse(manifest.content); - let i18nFullPath = content["sap.app"]["i18n"]; + const appI18n = content["sap.app"]["i18n"]; + let i18nFullPath; // i18n section in sap.app can be either a string or an object with bundleUrl - if (typeof i18nFullPath === "object") { - i18nFullPath = i18nFullPath.bundleUrl; + if (typeof appI18n === "object") { + i18nFullPath = appI18n.bundleUrl; } if (!i18nFullPath) { i18nFullPath = "i18n/i18n.properties"; } - return { - path: path.join(path.dirname(manifest.path), path.dirname(i18nFullPath)), - rootName: path.basename(i18nFullPath, propertiesExtension) - }; + addI18nInfo(i18nFullPath); + + if (typeof appI18n === "object") { + if (appI18n.enhanceWith) { + appI18n.enhanceWith.forEach((enhanceWithEntry) => { + if (enhanceWithEntry.bundleUrl) { + addI18nInfo(enhanceWithEntry.bundleUrl); + } + }); + } + } } return Promise.all(resources.map((resource) => @@ -90,8 +105,7 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi resources.forEach((resource) => { if (resource.isManifest) { - const descriptorI18nInfo = getDescriptorI18nInfo(resource); - descriptorI18nInfos.set(descriptorI18nInfo.path, descriptorI18nInfo.rootName); + addDescriptorI18nInfos(descriptorI18nInfos, resource); archiveContent.set(resource.path, resource.content); } else { const directory = path.dirname(resource.path); diff --git a/test/lib/processors/bundlers/manifestBundler.js b/test/lib/processors/bundlers/manifestBundler.js index 1f607d476..21ad079f9 100644 --- a/test/lib/processors/bundlers/manifestBundler.js +++ b/test/lib/processors/bundlers/manifestBundler.js @@ -145,3 +145,65 @@ test.serial("manifestBundler with manifest with i18n object", async (t) => { t.deepEqual(t.context.addBufferSpy.getCall(2).args, ["A=C", "i18n/i18n_en.properties"], "should be called with correct arguments"); }); + +test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => { + const resources = []; + const manifestString = JSON.stringify({ + "sap.app": { + "i18n": { + "bundleUrl": "i18n/i18n.properties", + "enhanceWith": [ + { + "bundleUrl": "enhancement1/i18n.properties" + }, + { + "bundleUrl": "enhancement2/i18n.properties" + } + ] + } + } + }); + resources.push({ + name: "manifest.json", + getPath: () => "/resources/pony/manifest.json", + getBuffer: async () => manifestString + }); + resources.push({ + name: "i18n_de.properties", + getPath: () => "/resources/pony/i18n/i18n_de.properties", + getBuffer: async () => "A=B" + }); + resources.push({ + name: "i18n_en.properties", + getPath: () => "/resources/pony/i18n/i18n_en.properties", + getBuffer: async () => "A=C" + }); + resources.push({ + name: "i18n.properties", + getPath: () => "/resources/pony/enhancement1/i18n.properties", + getBuffer: async () => "A=enhancement1" + }); + resources.push({ + name: "i18n.properties", + getPath: () => "/resources/pony/enhancement2/i18n.properties", + getBuffer: async () => "A=enhancement2" + }); + const options = { + descriptor: "manifest.json", + namespace: "pony", + propertiesExtension: ".properties" + }; + await manifestBundler({resources, options}); + t.deepEqual(t.context.addBufferSpy.callCount, 5, "should be called 3 times"); + t.deepEqual(t.context.logVerboseSpy.callCount, 0, "should not be called"); + t.deepEqual(t.context.addBufferSpy.getCall(0).args, [manifestString, "manifest.json"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(1).args, ["A=B", "i18n/i18n_de.properties"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(2).args, ["A=C", "i18n/i18n_en.properties"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(3).args, ["A=enhancement1", "enhancement1/i18n.properties"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(4).args, ["A=enhancement2", "enhancement2/i18n.properties"], + "should be called with correct arguments"); +}); From f50c2042e070aad24a4559bc356941b2ab6ad273 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 16 Dec 2020 14:44:40 +0100 Subject: [PATCH 2/8] Support bundleName --- lib/processors/bundlers/manifestBundler.js | 18 ++++++++++++++++-- .../lib/processors/bundlers/manifestBundler.js | 3 ++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index dab0d4771..20bda0ece 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -57,6 +57,14 @@ class I18nResourceList { * @returns {Promise} Promise resolving with manifest bundle resources */ module.exports = ({resources, options: {namespace, bundleName, propertiesExtension, descriptor}}) => { + function bundleNameToUrl(bundleName, appId) { + if (!bundleName.startsWith(appId)) { + return null; + } + const relativeBundleName = bundleName.substring(appId.length + 1); + return relativeBundleName.replace(/\./g, "/") + propertiesExtension; + } + function addDescriptorI18nInfos(descriptorI18nInfos, manifest) { function addI18nInfo(i18nPath) { descriptorI18nInfos.set( @@ -78,10 +86,16 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi addI18nInfo(i18nFullPath); if (typeof appI18n === "object") { - if (appI18n.enhanceWith) { + if (Array.isArray(appI18n.enhanceWith)) { appI18n.enhanceWith.forEach((enhanceWithEntry) => { + let bundleUrl; if (enhanceWithEntry.bundleUrl) { - addI18nInfo(enhanceWithEntry.bundleUrl); + bundleUrl = enhanceWithEntry.bundleUrl; + } else if (enhanceWithEntry.bundleName) { + bundleUrl = bundleNameToUrl(enhanceWithEntry.bundleName, content["sap.app"]["id"]); + } + if (bundleUrl) { + addI18nInfo(bundleUrl); } }); } diff --git a/test/lib/processors/bundlers/manifestBundler.js b/test/lib/processors/bundlers/manifestBundler.js index 21ad079f9..b4cdcdf8f 100644 --- a/test/lib/processors/bundlers/manifestBundler.js +++ b/test/lib/processors/bundlers/manifestBundler.js @@ -150,6 +150,7 @@ test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => const resources = []; const manifestString = JSON.stringify({ "sap.app": { + "id": "pony", "i18n": { "bundleUrl": "i18n/i18n.properties", "enhanceWith": [ @@ -157,7 +158,7 @@ test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => "bundleUrl": "enhancement1/i18n.properties" }, { - "bundleUrl": "enhancement2/i18n.properties" + "bundleName": "pony.enhancement2.i18n" } ] } From ed74c66bfb01e41c92af6eb0b2f37c38caa982bd Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 18 Dec 2020 09:30:45 +0100 Subject: [PATCH 3/8] Log error when ui5:// protocol is used --- lib/processors/bundlers/manifestBundler.js | 3 + .../processors/bundlers/manifestBundler.js | 138 +++++++++++++++--- 2 files changed, 124 insertions(+), 17 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index 20bda0ece..dd7e657c9 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -67,6 +67,9 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi function addDescriptorI18nInfos(descriptorI18nInfos, manifest) { function addI18nInfo(i18nPath) { + if (i18nPath.startsWith("ui5:")) { + log.error(`Using the ui5:// protocol for i18n bundles is currently not supported ('${i18nPath}' in ${manifest.path})`); + } descriptorI18nInfos.set( path.join(path.dirname(manifest.path), path.dirname(i18nPath)), path.basename(i18nPath, propertiesExtension) diff --git a/test/lib/processors/bundlers/manifestBundler.js b/test/lib/processors/bundlers/manifestBundler.js index b4cdcdf8f..8767ed3c6 100644 --- a/test/lib/processors/bundlers/manifestBundler.js +++ b/test/lib/processors/bundlers/manifestBundler.js @@ -7,14 +7,15 @@ const mock = require("mock-require"); let manifestBundler = require("../../../../lib/processors/bundlers/manifestBundler"); test.beforeEach((t) => { - // Spying logger of processors/bundlers/manifestBundler + // Stubbing logger of processors/bundlers/manifestBundler const log = require("@ui5/logger"); const loggerInstance = log.getLogger("builder:processors:bundlers:manifestBundler"); mock("@ui5/logger", { getLogger: () => loggerInstance }); mock.reRequire("@ui5/logger"); - t.context.logVerboseSpy = sinon.spy(loggerInstance, "verbose"); + t.context.logVerboseSpy = sinon.stub(loggerInstance, "verbose"); + t.context.logErrorSpy = sinon.stub(loggerInstance, "error"); // Re-require tested module manifestBundler = mock.reRequire("../../../../lib/processors/bundlers/manifestBundler"); @@ -25,19 +26,18 @@ test.beforeEach((t) => { t.context.yazlZipFile = sinon.stub(yazl, "ZipFile").returns(zip); }); -test.afterEach.always((t) => { - mock.stop("@ui5/logger"); - t.context.logVerboseSpy.restore(); - t.context.yazlZipFile.restore(); - t.context.addBufferSpy.restore(); +test.afterEach.always(() => { + mock.stopAll(); + sinon.restore(); }); test.serial("manifestBundler with empty resources", async (t) => { const resources = []; const options = {}; await manifestBundler({resources, options}); - t.deepEqual(t.context.addBufferSpy.callCount, 0, "should not be called"); - t.deepEqual(t.context.logVerboseSpy.callCount, 0, "should not be called"); + t.is(t.context.addBufferSpy.callCount, 0); + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); }); test.serial("manifestBundler with manifest path not starting with '/resources'", async (t) => { @@ -53,12 +53,16 @@ test.serial("manifestBundler with manifest path not starting with '/resources'", descriptor: "manifest.json", namespace: "pony" }; + await manifestBundler({resources, options}); + t.deepEqual(t.context.addBufferSpy.callCount, 0, "should not be called"); - t.deepEqual(t.context.logVerboseSpy.callCount, 1, "should be called once"); + + t.is(t.context.logVerboseSpy.callCount, 1); t.deepEqual(t.context.logVerboseSpy.getCall(0).args, ["Not bundling resource with path pony/manifest.json since it is not based on path /resources/pony/"], "should be called with correct arguments"); + t.is(t.context.logErrorSpy.callCount, 0); }); test.serial("manifestBundler with manifest without i18n section in sap.app", async (t) => { @@ -74,11 +78,15 @@ test.serial("manifestBundler with manifest without i18n section in sap.app", asy descriptor: "manifest.json", namespace: "pony" }; + await manifestBundler({resources, options}); - t.deepEqual(t.context.addBufferSpy.callCount, 1, "should be called once"); + + t.is(t.context.addBufferSpy.callCount, 1, "should be called once"); t.deepEqual(t.context.addBufferSpy.getCall(0).args, ["{\"sap.app\":{}}", "manifest.json"], "should be called with correct arguments"); - t.deepEqual(t.context.logVerboseSpy.callCount, 0, "should not be called"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); }); test.serial("manifestBundler with manifest with i18n string", async (t) => { @@ -96,12 +104,16 @@ test.serial("manifestBundler with manifest with i18n string", async (t) => { descriptor: "manifest.json", namespace: "pony" }; + await manifestBundler({resources, options}); + t.deepEqual(t.context.addBufferSpy.callCount, 1, "should be called once"); t.deepEqual(t.context.addBufferSpy.getCall(0).args, ["{\"sap.app\":{\"i18n\":\"i18n/i18n.properties\"}}", "manifest.json"], "should be called with correct arguments"); - t.deepEqual(t.context.logVerboseSpy.callCount, 0, "should not be called"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); }); test.serial("manifestBundler with manifest with i18n object", async (t) => { @@ -135,15 +147,19 @@ test.serial("manifestBundler with manifest with i18n object", async (t) => { namespace: "pony", propertiesExtension: ".properties" }; + await manifestBundler({resources, options}); - t.deepEqual(t.context.addBufferSpy.callCount, 3, "should be called 3 times"); - t.deepEqual(t.context.logVerboseSpy.callCount, 0, "should not be called"); + + t.is(t.context.addBufferSpy.callCount, 3, "should be called 3 times"); t.deepEqual(t.context.addBufferSpy.getCall(0).args, [manifestString, "manifest.json"], "should be called with correct arguments"); t.deepEqual(t.context.addBufferSpy.getCall(1).args, ["A=B", "i18n/i18n_de.properties"], "should be called with correct arguments"); t.deepEqual(t.context.addBufferSpy.getCall(2).args, ["A=C", "i18n/i18n_en.properties"], "should be called with correct arguments"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); }); test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => { @@ -194,9 +210,10 @@ test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => namespace: "pony", propertiesExtension: ".properties" }; + await manifestBundler({resources, options}); - t.deepEqual(t.context.addBufferSpy.callCount, 5, "should be called 3 times"); - t.deepEqual(t.context.logVerboseSpy.callCount, 0, "should not be called"); + + t.is(t.context.addBufferSpy.callCount, 5, "should be called 5 times"); t.deepEqual(t.context.addBufferSpy.getCall(0).args, [manifestString, "manifest.json"], "should be called with correct arguments"); t.deepEqual(t.context.addBufferSpy.getCall(1).args, ["A=B", "i18n/i18n_de.properties"], @@ -207,4 +224,91 @@ test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => "should be called with correct arguments"); t.deepEqual(t.context.addBufferSpy.getCall(4).args, ["A=enhancement2", "enhancement2/i18n.properties"], "should be called with correct arguments"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); +}); + +test.serial("manifestBundler with manifest with missing i18n files", async (t) => { + const resources = []; + const manifestString = JSON.stringify({ + "sap.app": { + "id": "pony", + "i18n": { + "bundleUrl": "i18n/i18n.properties", + "enhanceWith": [ + { + "bundleUrl": "enhancement1/i18n.properties" + }, + { + "bundleName": "pony.enhancement2.i18n" + } + ] + } + } + }); + resources.push({ + name: "manifest.json", + getPath: () => "/resources/pony/manifest.json", + getBuffer: async () => manifestString + }); + const options = { + descriptor: "manifest.json", + namespace: "pony", + propertiesExtension: ".properties" + }; + + await manifestBundler({resources, options}); + + t.is(t.context.addBufferSpy.callCount, 1, "should be called 1 time"); + t.deepEqual(t.context.addBufferSpy.getCall(0).args, [manifestString, "manifest.json"], + "should be called with correct arguments"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); +}); + +test.serial("manifestBundler with manifest with ui5:// url", async (t) => { + const resources = []; + const manifestString = JSON.stringify({ + "sap.app": { + "id": "pony", + "i18n": { + "bundleUrl": "ui5://pony/i18n/i18n.properties" + } + } + }); + resources.push({ + name: "manifest.json", + getPath: () => "/resources/pony/manifest.json", + getBuffer: async () => manifestString + }); + resources.push({ + name: "i18n_de.properties", + getPath: () => "/resources/pony/i18n/i18n_de.properties", + getBuffer: async () => "A=B" + }); + resources.push({ + name: "i18n_en.properties", + getPath: () => "/resources/pony/i18n/i18n_en.properties", + getBuffer: async () => "A=C" + }); + const options = { + descriptor: "manifest.json", + namespace: "pony", + propertiesExtension: ".properties" + }; + + await manifestBundler({resources, options}); + + t.is(t.context.addBufferSpy.callCount, 1, "should be called 1 time"); + t.deepEqual(t.context.addBufferSpy.getCall(0).args, [manifestString, "manifest.json"], + "should be called with correct arguments"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 1); + t.deepEqual(t.context.logErrorSpy.getCall(0).args, [ + `Using the ui5:// protocol for i18n bundles is currently not supported ` + + `('ui5://pony/i18n/i18n.properties' in /resources/pony/manifest.json)` + ]); }); From 4ff4356e69c0fca777e125af881395d842ac377d Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 18 Dec 2020 10:28:25 +0100 Subject: [PATCH 4/8] Change log to warn --- lib/processors/bundlers/manifestBundler.js | 2 +- test/lib/processors/bundlers/manifestBundler.js | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index dd7e657c9..46bfb0c1b 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -68,7 +68,7 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi function addDescriptorI18nInfos(descriptorI18nInfos, manifest) { function addI18nInfo(i18nPath) { if (i18nPath.startsWith("ui5:")) { - log.error(`Using the ui5:// protocol for i18n bundles is currently not supported ('${i18nPath}' in ${manifest.path})`); + log.warn(`Using the ui5:// protocol for i18n bundles is currently not supported ('${i18nPath}' in ${manifest.path})`); } descriptorI18nInfos.set( path.join(path.dirname(manifest.path), path.dirname(i18nPath)), diff --git a/test/lib/processors/bundlers/manifestBundler.js b/test/lib/processors/bundlers/manifestBundler.js index 8767ed3c6..9d507066c 100644 --- a/test/lib/processors/bundlers/manifestBundler.js +++ b/test/lib/processors/bundlers/manifestBundler.js @@ -15,6 +15,7 @@ test.beforeEach((t) => { }); mock.reRequire("@ui5/logger"); t.context.logVerboseSpy = sinon.stub(loggerInstance, "verbose"); + t.context.logWarnSpy = sinon.stub(loggerInstance, "warn"); t.context.logErrorSpy = sinon.stub(loggerInstance, "error"); // Re-require tested module @@ -37,6 +38,7 @@ test.serial("manifestBundler with empty resources", async (t) => { await manifestBundler({resources, options}); t.is(t.context.addBufferSpy.callCount, 0); t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -62,6 +64,7 @@ test.serial("manifestBundler with manifest path not starting with '/resources'", t.deepEqual(t.context.logVerboseSpy.getCall(0).args, ["Not bundling resource with path pony/manifest.json since it is not based on path /resources/pony/"], "should be called with correct arguments"); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -86,6 +89,7 @@ test.serial("manifestBundler with manifest without i18n section in sap.app", asy "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -113,6 +117,7 @@ test.serial("manifestBundler with manifest with i18n string", async (t) => { "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -159,6 +164,7 @@ test.serial("manifestBundler with manifest with i18n object", async (t) => { "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -226,6 +232,7 @@ test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -265,6 +272,7 @@ test.serial("manifestBundler with manifest with missing i18n files", async (t) = "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -306,9 +314,10 @@ test.serial("manifestBundler with manifest with ui5:// url", async (t) => { "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); - t.is(t.context.logErrorSpy.callCount, 1); - t.deepEqual(t.context.logErrorSpy.getCall(0).args, [ + t.is(t.context.logWarnSpy.callCount, 1); + t.deepEqual(t.context.logWarnSpy.getCall(0).args, [ `Using the ui5:// protocol for i18n bundles is currently not supported ` + `('ui5://pony/i18n/i18n.properties' in /resources/pony/manifest.json)` ]); + t.is(t.context.logErrorSpy.callCount, 0); }); From 5ad70f2c2f1a9b453555720851254f2481476f7c Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 18 Dec 2020 11:48:44 +0100 Subject: [PATCH 5/8] Add warning for missing i18n files / fix bug --- lib/processors/bundlers/manifestBundler.js | 14 ++++++-- .../processors/bundlers/manifestBundler.js | 34 +++++++++++++++---- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index 46bfb0c1b..135f0d4f6 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -69,6 +69,7 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi function addI18nInfo(i18nPath) { if (i18nPath.startsWith("ui5:")) { log.warn(`Using the ui5:// protocol for i18n bundles is currently not supported ('${i18nPath}' in ${manifest.path})`); + return; } descriptorI18nInfos.set( path.join(path.dirname(manifest.path), path.dirname(i18nPath)), @@ -82,6 +83,8 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi // i18n section in sap.app can be either a string or an object with bundleUrl if (typeof appI18n === "object") { i18nFullPath = appI18n.bundleUrl; + } else { + i18nFullPath = appI18n; } if (!i18nFullPath) { i18nFullPath = "i18n/i18n.properties"; @@ -131,9 +134,14 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi }); descriptorI18nInfos.forEach((rootName, directory) => { - i18nResourceList.get(directory) - .filter((resource) => resource.name.startsWith(rootName)) - .forEach((resource) => archiveContent.set(resource.path, resource.content)); + const i18nResources = i18nResourceList.get(directory) + .filter((resource) => resource.name.startsWith(rootName)); + + if (i18nResources.length) { + i18nResources.forEach((resource) => archiveContent.set(resource.path, resource.content)); + } else { + log.warn(`Could not find any resources for i18n bundle '${directory}'`); + } }); return archiveContent; diff --git a/test/lib/processors/bundlers/manifestBundler.js b/test/lib/processors/bundlers/manifestBundler.js index 9d507066c..ce24589e4 100644 --- a/test/lib/processors/bundlers/manifestBundler.js +++ b/test/lib/processors/bundlers/manifestBundler.js @@ -64,7 +64,9 @@ test.serial("manifestBundler with manifest path not starting with '/resources'", t.deepEqual(t.context.logVerboseSpy.getCall(0).args, ["Not bundling resource with path pony/manifest.json since it is not based on path /resources/pony/"], "should be called with correct arguments"); - t.is(t.context.logWarnSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 1); + t.deepEqual(t.context.logWarnSpy.getCall(0).args, + ["Could not find any resources for i18n bundle 'pony/i18n'"]); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -89,7 +91,9 @@ test.serial("manifestBundler with manifest without i18n section in sap.app", asy "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); - t.is(t.context.logWarnSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 1); + t.deepEqual(t.context.logWarnSpy.getCall(0).args, + ["Could not find any resources for i18n bundle '/resources/pony/i18n'"]); t.is(t.context.logErrorSpy.callCount, 0); }); @@ -100,10 +104,15 @@ test.serial("manifestBundler with manifest with i18n string", async (t) => { getPath: () => "/resources/pony/manifest.json", getBuffer: async () => JSON.stringify({ "sap.app": { - "i18n": "i18n/i18n.properties" + "i18n": "i18n-bundle/i18n.properties" } }) }); + resources.push({ + name: "i18n.properties", + getPath: () => "/resources/pony/i18n-bundle/i18n.properties", + getBuffer: async () => "A=B" + }); const options = { descriptor: "manifest.json", namespace: "pony" @@ -111,9 +120,12 @@ test.serial("manifestBundler with manifest with i18n string", async (t) => { await manifestBundler({resources, options}); - t.deepEqual(t.context.addBufferSpy.callCount, 1, "should be called once"); + t.deepEqual(t.context.addBufferSpy.callCount, 2); t.deepEqual(t.context.addBufferSpy.getCall(0).args, - ["{\"sap.app\":{\"i18n\":\"i18n/i18n.properties\"}}", "manifest.json"], + ["{\"sap.app\":{\"i18n\":\"i18n-bundle/i18n.properties\"}}", "manifest.json"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(1).args, + ["A=B", "i18n-bundle/i18n.properties"], "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); @@ -272,7 +284,17 @@ test.serial("manifestBundler with manifest with missing i18n files", async (t) = "should be called with correct arguments"); t.is(t.context.logVerboseSpy.callCount, 0); - t.is(t.context.logWarnSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 3); + t.deepEqual(t.context.logWarnSpy.getCall(0).args, [ + `Could not find any resources for i18n bundle '/resources/pony/i18n'` + ]); + t.deepEqual(t.context.logWarnSpy.getCall(1).args, [ + `Could not find any resources for i18n bundle '/resources/pony/enhancement1'` + ]); + t.deepEqual(t.context.logWarnSpy.getCall(2).args, [ + `Could not find any resources for i18n bundle '/resources/pony/enhancement2'` + ]); + t.is(t.context.logErrorSpy.callCount, 0); }); From 67938907d3650fc9fb6e4995b0ecb60fd8874aec Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 18 Dec 2020 12:42:10 +0100 Subject: [PATCH 6/8] Support i18n/bundleName --- lib/processors/bundlers/manifestBundler.js | 15 ++++-- .../processors/bundlers/manifestBundler.js | 50 ++++++++++++++++++- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index 135f0d4f6..a82164d37 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -82,14 +82,19 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi let i18nFullPath; // i18n section in sap.app can be either a string or an object with bundleUrl if (typeof appI18n === "object") { - i18nFullPath = appI18n.bundleUrl; - } else { + if (appI18n.bundleUrl) { + i18nFullPath = appI18n.bundleUrl; + } else if (appI18n.bundleName) { + i18nFullPath = bundleNameToUrl(appI18n.bundleName, content["sap.app"]["id"]); + } + } else if (typeof appI18n === "string") { i18nFullPath = appI18n; - } - if (!i18nFullPath) { + } else { i18nFullPath = "i18n/i18n.properties"; } - addI18nInfo(i18nFullPath); + if (i18nFullPath) { + addI18nInfo(i18nFullPath); + } if (typeof appI18n === "object") { if (Array.isArray(appI18n.enhanceWith)) { diff --git a/test/lib/processors/bundlers/manifestBundler.js b/test/lib/processors/bundlers/manifestBundler.js index ce24589e4..0081803b6 100644 --- a/test/lib/processors/bundlers/manifestBundler.js +++ b/test/lib/processors/bundlers/manifestBundler.js @@ -133,7 +133,7 @@ test.serial("manifestBundler with manifest with i18n string", async (t) => { t.is(t.context.logErrorSpy.callCount, 0); }); -test.serial("manifestBundler with manifest with i18n object", async (t) => { +test.serial("manifestBundler with manifest with i18n object (bundleUrl)", async (t) => { const resources = []; const manifestString = JSON.stringify({ "sap.app": { @@ -180,6 +180,54 @@ test.serial("manifestBundler with manifest with i18n object", async (t) => { t.is(t.context.logErrorSpy.callCount, 0); }); +test.serial("manifestBundler with manifest with i18n object (bundleName)", async (t) => { + const resources = []; + const manifestString = JSON.stringify({ + "sap.app": { + "id": "pony", + "i18n": { + "bundleName": "pony.i18n.i18n", + "supportedLocales": ["en", "de"], + "fallbackLocale": "en" + } + } + }); + resources.push({ + name: "manifest.json", + getPath: () => "/resources/pony/manifest.json", + getBuffer: async () => manifestString + }); + resources.push({ + name: "i18n_de.properties", + getPath: () => "/resources/pony/i18n/i18n_de.properties", + getBuffer: async () => "A=B" + }); + resources.push({ + name: "i18n_en.properties", + getPath: () => "/resources/pony/i18n/i18n_en.properties", + getBuffer: async () => "A=C" + }); + const options = { + descriptor: "manifest.json", + namespace: "pony", + propertiesExtension: ".properties" + }; + + await manifestBundler({resources, options}); + + t.is(t.context.addBufferSpy.callCount, 3, "should be called 3 times"); + t.deepEqual(t.context.addBufferSpy.getCall(0).args, [manifestString, "manifest.json"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(1).args, ["A=B", "i18n/i18n_de.properties"], + "should be called with correct arguments"); + t.deepEqual(t.context.addBufferSpy.getCall(2).args, ["A=C", "i18n/i18n_en.properties"], + "should be called with correct arguments"); + + t.is(t.context.logVerboseSpy.callCount, 0); + t.is(t.context.logWarnSpy.callCount, 0); + t.is(t.context.logErrorSpy.callCount, 0); +}); + test.serial("manifestBundler with manifest with i18n enhanceWith", async (t) => { const resources = []; const manifestString = JSON.stringify({ From 7bfa12f8470ece58b12f5eb179c1cae57132e22b Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 18 Dec 2020 12:44:42 +0100 Subject: [PATCH 7/8] path -> path.posix --- lib/processors/bundlers/manifestBundler.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index a82164d37..d5ea4251c 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -1,4 +1,4 @@ -const path = require("path"); +const posixPath = require("path").posix; const yazl = require("yazl"); const resourceFactory = require("@ui5/fs").resourceFactory; const log = require("@ui5/logger").getLogger("builder:processors:bundlers:manifestBundler"); @@ -23,7 +23,7 @@ class I18nResourceList { * @param {module:@ui5/fs.Resource} resource i18n resource */ add(directory, resource) { - const normalizedDirectory = path.normalize(directory); + const normalizedDirectory = posixPath.normalize(directory); if (!this.propertyFiles.has(normalizedDirectory)) { this.propertyFiles.set(normalizedDirectory, [resource]); } else { @@ -38,7 +38,7 @@ class I18nResourceList { * @returns {Array} Array of resources files */ get(directory) { - return this.propertyFiles.get(path.normalize(directory)) || []; + return this.propertyFiles.get(posixPath.normalize(directory)) || []; } } @@ -72,8 +72,8 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi return; } descriptorI18nInfos.set( - path.join(path.dirname(manifest.path), path.dirname(i18nPath)), - path.basename(i18nPath, propertiesExtension) + posixPath.join(posixPath.dirname(manifest.path), posixPath.dirname(i18nPath)), + posixPath.basename(i18nPath, propertiesExtension) ); } @@ -115,7 +115,7 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi return Promise.all(resources.map((resource) => resource.getBuffer().then((content) => { - const basename = path.basename(resource.getPath()); + const basename = posixPath.basename(resource.getPath()); return { name: basename, isManifest: basename === descriptor, @@ -133,7 +133,7 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi addDescriptorI18nInfos(descriptorI18nInfos, resource); archiveContent.set(resource.path, resource.content); } else { - const directory = path.dirname(resource.path); + const directory = posixPath.dirname(resource.path); i18nResourceList.add(directory, resource); } }); From 2a4bb8e3afab62a73ddc98407c4af1b4a2075cf3 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 18 Dec 2020 15:29:15 +0100 Subject: [PATCH 8/8] Review feedback --- lib/processors/bundlers/manifestBundler.js | 40 ++++++++++------------ 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/lib/processors/bundlers/manifestBundler.js b/lib/processors/bundlers/manifestBundler.js index d5ea4251c..9453f5c10 100644 --- a/lib/processors/bundlers/manifestBundler.js +++ b/lib/processors/bundlers/manifestBundler.js @@ -79,37 +79,35 @@ module.exports = ({resources, options: {namespace, bundleName, propertiesExtensi const content = JSON.parse(manifest.content); const appI18n = content["sap.app"]["i18n"]; - let i18nFullPath; + let bundleUrl; // i18n section in sap.app can be either a string or an object with bundleUrl if (typeof appI18n === "object") { if (appI18n.bundleUrl) { - i18nFullPath = appI18n.bundleUrl; + bundleUrl = appI18n.bundleUrl; } else if (appI18n.bundleName) { - i18nFullPath = bundleNameToUrl(appI18n.bundleName, content["sap.app"]["id"]); + bundleUrl = bundleNameToUrl(appI18n.bundleName, content["sap.app"]["id"]); } } else if (typeof appI18n === "string") { - i18nFullPath = appI18n; + bundleUrl = appI18n; } else { - i18nFullPath = "i18n/i18n.properties"; + bundleUrl = "i18n/i18n.properties"; } - if (i18nFullPath) { - addI18nInfo(i18nFullPath); + if (bundleUrl) { + addI18nInfo(bundleUrl); } - if (typeof appI18n === "object") { - if (Array.isArray(appI18n.enhanceWith)) { - appI18n.enhanceWith.forEach((enhanceWithEntry) => { - let bundleUrl; - if (enhanceWithEntry.bundleUrl) { - bundleUrl = enhanceWithEntry.bundleUrl; - } else if (enhanceWithEntry.bundleName) { - bundleUrl = bundleNameToUrl(enhanceWithEntry.bundleName, content["sap.app"]["id"]); - } - if (bundleUrl) { - addI18nInfo(bundleUrl); - } - }); - } + if (typeof appI18n === "object" && Array.isArray(appI18n.enhanceWith)) { + appI18n.enhanceWith.forEach((enhanceWithEntry) => { + let bundleUrl; + if (enhanceWithEntry.bundleUrl) { + bundleUrl = enhanceWithEntry.bundleUrl; + } else if (enhanceWithEntry.bundleName) { + bundleUrl = bundleNameToUrl(enhanceWithEntry.bundleName, content["sap.app"]["id"]); + } + if (bundleUrl) { + addI18nInfo(bundleUrl); + } + }); } }