[FIX] manifestCreator: supportedThemes should only list relevant themes

Only themes within a "themes" directory in the library namespace (i.e. where the .library / library.js is located) should be taken into account. JIRA: CPOUI5FOUNDATION-323
SAP · Oct 8, 2021 · 01f3859 · 01f3859
1 parent e7ea5d3
commit 01f3859
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 7 deletions.
diff --git a/lib/processors/manifestCreator.js b/lib/processors/manifestCreator.js
@@ -161,6 +161,8 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
 	// create a Library wrapper around the .library XML
 	const library = await Library.from(libraryResource);
 
+	const libraryPathPrefix = posixPath.dirname(libraryResource.getPath()) + "/";
+
 	// collect information from library.js file
 	const libraryJSInfo = await analyzeLibraryJS(libBundle.findResource("library.js"));
 	const includeSupportedLocalesInformation = descriptorVersion.compare(APP_DESCRIPTOR_V22) >= 0;
@@ -183,7 +185,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
 	}
 
 	function createSapApp() {
-		function hasManifest(componentPath, libraryPathPrefix) {
+		function hasManifest(componentPath) {
 			const manifestPath = componentPath + "/manifest.json";
 
 			const manifestResource = libBundle.findResource(manifestPath.substring(libraryPathPrefix.length));
@@ -196,17 +198,19 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
 
 		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) && hasManifest(componentPath, prefix) ) {
-					result.push( componentPath.substring(prefix.length) );
-				} else if ( prefix === "/resources/sap/apf/" ) {
+				if ( componentPath.startsWith(libraryPathPrefix) && hasManifest(componentPath) ) {
+					result.push( componentPath.substring(libraryPathPrefix.length) );
+				} else if ( libraryPathPrefix === "/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 === (componentPath + "/") && prefix !== "/resources/sap/ui/core/" ) {
+				} else if (
+					libraryPathPrefix === (componentPath + "/") &&
+					libraryPathPrefix !== "/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);
@@ -353,13 +357,17 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
 
 			// find theme resources and determine theme names from their paths
 			libBundle.getResources(/(?:[^/]+\/)*themes\//).forEach((res) => {
+				if ( !res.getPath().startsWith(libraryPathPrefix + "themes/") ) {
+					// only consider themes within direct "themes" sub-directory
+					return;
+				}
 				const match = /\/themes\/([^/]+)\//.exec(res.getPath());
 				if ( match ) {
 					themes[match[1]] = true;
 				}
 			});
 
-			// merge with supporteTheme info from .library file
+			// merge with supportedTheme info from .library file
 			const elems = findChildren(manifestAppData, "supportedTheme");
 			if ( elems ) {
 				elems.forEach((elem) => {

diff --git a/test/lib/processors/manifestCreator.js b/test/lib/processors/manifestCreator.js
@@ -454,6 +454,90 @@ test.serial("default manifest creation with special characters very small app de
 	t.deepEqual(JSON.parse(sResult), expectedManifestContentSmallVersion, "Correct result returned");
 });
 
+test.serial("manifest creation with themes", async (t) => {
+	const {manifestCreator, errorLogStub, verboseLogStub} = t.context;
+
+	const prefix = "/resources/sap/ui/test/";
+
+	const expectedManifestContent = JSON.stringify({
+		"_version": "1.21.0",
+		"sap.app": {
+			"id": "sap.ui.test",
+			"type": "library",
+			"embeds": [],
+			"applicationVersion": {
+				"version": "1.0.0"
+			},
+			"title": "sap.ui.test",
+			"resources": "resources.json",
+			"offline": true
+		},
+		"sap.ui": {
+			"technology": "UI5",
+			"supportedThemes": [
+				"base", "sap_foo"
+			]
+		},
+		"sap.ui5": {
+			"dependencies": {
+				"libs": {}
+			},
+			"library": {
+				"i18n": false
+			}
+		}
+	}, null, 2);
+
+	const libraryResource = {
+		getPath: () => {
+			return prefix + ".library";
+		},
+		getString: async () => {
+			return `<?xml version="1.0" encoding="UTF-8" ?>
+			<library xmlns="http://www.sap.com/sap.ui.library.xsd" >
+				<name>sap.ui.test</name>
+				<version>1.0.0</version>
+			</library>`;
+		},
+		_project: {
+			dependencies: [{
+				metadata: {
+					name: "sap.ui.core"
+				}
+			}]
+		}
+	};
+
+	const resources = [];
+	["base", "sap_foo"].forEach((name) => {
+		resources.push({
+			getPath: () => {
+				return `${prefix}themes/${name}/some.less`;
+			}
+		});
+		resources.push({
+			getPath: () => {
+				return `${prefix}themes/${name}/library.source.less`;
+			}
+		});
+	});
+	resources.push({
+		getPath: () => {
+			return `${prefix}js/lib/themes/invalid/some.css`;
+		}
+	});
+
+	const result = await manifestCreator({libraryResource, resources, options: {}});
+	t.is(await result.getString(), expectedManifestContent, "Correct result returned");
+
+	t.is(errorLogStub.callCount, 0);
+	t.is(verboseLogStub.callCount, 7);
+	t.deepEqual(verboseLogStub.getCall(4).args, [
+		"  sap.ui/supportedThemes determined from resources: '%s'",
+		["base", "sap_foo"]
+	]);
+});
+
 test.serial("manifest creation for sap/apf", async (t) => {
 	const {manifestCreator, errorLogStub, verboseLogStub} = t.context;