Fix PR review issues: head.html test content, misleading test name, and missing try/finally

lib/BrandResolver.php

@@ -0,0 +1,88 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\NcwMailtemplate;
+
+use OCP\IConfig;
+
+/**
+ * Resolves brand-specific template and image paths with fallback to the default brand.
+ *
+ * Reads the `ncw.brand` system config value to determine the active brand.
+ * For each template file or image, it first checks if a brand-specific version exists.
+ * If not, it falls back to the default brand ('ionos').
+ */
+class BrandResolver {
+	public const DEFAULT_BRAND = 'ionos';
+
+	private string $brand;
+	private string $templatesBasePath;
+	private string $imgBasePath;
+
+	public function __construct(IConfig $config) {
+		$brand = $config->getSystemValueString('ncw.brand', self::DEFAULT_BRAND);
+
+		// Sanitize: only allow alphanumeric, dash, and underscore to prevent path traversal
+		if (!preg_match('/^[a-zA-Z0-9_-]+$/', $brand)) {
+			$brand = self::DEFAULT_BRAND;
+		}
+
+		// Lowercase to match folder names on disk regardless of config casing
+		$brand = strtolower($brand);
+
+		$this->brand = $brand;
+		$this->templatesBasePath = __DIR__ . '/templates/email';
+		$this->imgBasePath = __DIR__ . '/../img';
+	}
+
+	/**
+	 * Get the active brand identifier.
+	 */
+	public function getBrand(): string {
+		return $this->brand;
+	}
+
+	/**
+	 * Resolve a template file path with brand fallback.
+	 *
+	 * If the brand-specific file exists, return its path.
+	 * Otherwise, fall back to the default brand's version.
+	 *
+	 * @param string $fileName The template file name (e.g. 'header.html')
+	 * @return string The resolved absolute file path
+	 */
+	public function resolveTemplatePath(string $fileName): string {
+		if ($this->brand !== self::DEFAULT_BRAND) {
+			$brandPath = $this->templatesBasePath . '/' . $this->brand . '/' . $fileName;
+			if (file_exists($brandPath)) {
+				return $brandPath;
+			}
+		}
+
+		return $this->templatesBasePath . '/' . self::DEFAULT_BRAND . '/' . $fileName;
+	}
+
+	/**
+	 * Resolve an image name with brand fallback.
+	 *
+	 * Returns the app-relative image path (e.g. 'ionos/logo.png') for use
+	 * with IURLGenerator::imagePath().
+	 *
+	 * If the brand-specific image exists on disk, return '<brand>/<imageName>'.
+	 * Otherwise, fall back to '<DEFAULT_BRAND>/<imageName>'.
+	 *
+	 * @param string $imageName The image file name (e.g. 'logo.png')
+	 * @return string The resolved app-relative image path
+	 */
+	public function resolveImageName(string $imageName): string {
+		if ($this->brand !== self::DEFAULT_BRAND) {
+			$brandImagePath = $this->imgBasePath . '/' . $this->brand . '/' . $imageName;
+			if (file_exists($brandImagePath)) {
+				return $this->brand . '/' . $imageName;
+			}
+		}
+
+		return self::DEFAULT_BRAND . '/' . $imageName;
+	}
+}

lib/EMailTemplate.php

@@ -17,6 +17,7 @@
 class EMailTemplate extends ParentTemplate {
 	private IL10N $l;
 	private ?IUser $user = null;
+	private BrandResolver $brandResolver;
 
 	// Generated asset URLs (filled in constructor)
 	private string $spacerUrl = '';
@@ -58,28 +59,31 @@ public function __construct(
 		?int $logoHeight,
 		string $emailId,
 		array $data = [],
-		) {
+	) {
 		// Initialize parent first to set up basic properties
 		parent::__construct($defaults, $urlGenerator, $l10nFactory, $logoWidth, $logoHeight, $emailId, $data);
-
+
+		// Initialize the brand resolver from server config
+		$config = \OC::$server->get(IConfig::class);
+		$this->brandResolver = new BrandResolver($config);
+
 		// Try to get user from various sources (for recipient's language)
 		$this->user = $this->determineUser($this->data);
-		
+
 		// Get language: user's preference, or system default
 		if ($this->user) {
 			$lang = $this->l10nFactory->getUserLanguage($this->user);
 		} else {
-			$config = \OC::$server->get(IConfig::class);
 			$lang = $config->getSystemValue('default_language', 'en');
 		}
 		$this->l = $this->l10nFactory->get(Application::APP_ID, $lang);
 
-		// Generate URLs for template assets
+		// Generate URLs for template assets (brand-aware)
 		$this->generateTemplateAssetUrls($urlGenerator);
 
 		// Load all HTML template files - this will override parent's head and tail
 		$this->loadHtmlTemplateFiles();
-		
+
 		// Replace the parent's htmlBody that was set with the parent's head
 		// with our custom head
 		$this->htmlBody = $this->head;
@@ -154,24 +158,34 @@ private function determineUser(array $data): ?IUser {
 	/**
 	 * Generate URLs for template assets (images, etc.)
 	 *
+	 * Uses BrandResolver to pick brand-specific images with fallback to the
+	 * default brand.
+	 *
 	 * @param IURLGenerator $urlGenerator
 	 */
 	private function generateTemplateAssetUrls(IURLGenerator $urlGenerator): void {
-		// store on the instance so we can inject into templates
-		$this->spacerUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, 'spacer.png'));
-		$this->logoUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, 'ionos_logo_de.png'));
-		$this->emailIconUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, 'email.png'));
-		$this->listItemIconUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, 'list-item-icon.png'));
+		$spacerImage = $this->brandResolver->resolveImageName('spacer.png');
+		$logoImage = $this->brandResolver->resolveImageName('logo.png');
+		$emailIconImage = $this->brandResolver->resolveImageName('email.png');
+		$listItemIconImage = $this->brandResolver->resolveImageName('list-item-icon.png');
+
+		$this->spacerUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, $spacerImage));
+		$this->logoUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, $logoImage));
+		$this->emailIconUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, $emailIconImage));
+		$this->listItemIconUrl = $urlGenerator->getAbsoluteURL($urlGenerator->imagePath(Application::APP_ID, $listItemIconImage));
 	}
 
 	/**
 	 * Load HTML template files for email components
 	 *
+	 * Uses BrandResolver to pick brand-specific template files with fallback
+	 * to the default brand.
+	 *
 	 * @return void
 	 */
 	private function loadHtmlTemplateFiles(): void {
 		foreach (self::HTML_TEMPLATE_FILES as $property => $file) {
-			$templatePath = __DIR__ . '/templates/email/' . $file;
+			$templatePath = $this->brandResolver->resolveTemplatePath($file);
 			if (!file_exists($templatePath)) {
 				continue;
 			}

lib/templates/email/head.html

@@ -1,15 +1,2 @@
-<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
-<html xmlns="http://www.w3.org/1999/xhtml" lang="en" xml:lang="en" style="-webkit-font-smoothing:antialiased;background:#fff!important">
-<head>
-	<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
-	<meta name="viewport" content="width=device-width">
-	<title></title>
-	<style type="text/css">@media only screen{html{min-height:100%;background:#fff}}@media only screen and (max-width:610px){table.body img{width:auto;height:auto}table.body center{min-width:0!important}table.body .container{width:95%!important}table.body .columns{height:auto!important;-moz-box-sizing:border-box;-webkit-box-sizing:border-box;box-sizing:border-box;padding-left:30px!important;padding-right:30px!important}th.small-12{display:inline-block!important;width:100%!important}table.menu{width:100%!important}table.menu td,table.menu th{width:auto!important;display:inline-block!important}table.menu.vertical td,table.menu.vertical th{display:block!important}table.menu[align=center]{width:auto!important}}</style>
-</head>
-<body style="-moz-box-sizing:border-box;-ms-text-size-adjust:100%;-webkit-box-sizing:border-box;-webkit-font-smoothing:antialiased;-webkit-text-size-adjust:100%;margin:0;background:#fff!important;box-sizing:border-box;color:#0a0a0a;font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Oxygen-Sans,Ubuntu,Cantarell,'Helvetica Neue',Arial,sans-serif;font-size:16px;font-weight:400;line-height:1.3;margin:0;min-width:100%;padding:0;text-align:left;width:100%!important">
-	<span class="preheader" style="color:#F5F5F5;display:none!important;font-size:1px;line-height:1px;max-height:0;max-width:0;mso-hide:all!important;opacity:0;overflow:hidden;visibility:hidden">
-	</span>
-	<table class="body" style="-webkit-font-smoothing:antialiased;margin:0;background:#fff;border-collapse:collapse;border-spacing:0;color:#0a0a0a;font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Oxygen-Sans,Ubuntu,Cantarell,'Helvetica Neue',Arial,sans-serif;font-size:16px;font-weight:400;line-height:1.3;margin:0;padding:0;text-align:left;vertical-align:top;width:100%">
-		<tr style="padding:0;text-align:left;vertical-align:top">
-			<td class="center" align="center" valign="top" style="-moz-hyphens:auto;-webkit-hyphens:auto;Margin:0;border-collapse:collapse!important;color:#0a0a0a;font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Oxygen-Sans,Ubuntu,Cantarell,'Helvetica Neue',Arial,sans-serif;font-size:16px;font-weight:400;hyphens:auto;line-height:1.3;margin:0;padding:0;text-align:left;vertical-align:top;word-wrap:break-word">
-				<center data-parsed="" style="width:100%;max-width:740px;margin:auto">
+<!-- Legacy email head template retained for backward compatibility.
+     Intentionally left blank; use brand-specific templates instead. -->

lib/templates/email/ionos/head.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" lang="en" xml:lang="en" style="-webkit-font-smoothing:antialiased;background:#fff!important">
+<head>
+	<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+	<meta name="viewport" content="width=device-width">
+	<title></title>
+	<style type="text/css">@media only screen{html{min-height:100%;background:#fff}}@media only screen and (max-width:610px){table.body img{width:auto;height:auto}table.body center{min-width:0!important}table.body .container{width:95%!important}table.body .columns{height:auto!important;-moz-box-sizing:border-box;-webkit-box-sizing:border-box;box-sizing:border-box;padding-left:30px!important;padding-right:30px!important}th.small-12{display:inline-block!important;width:100%!important}table.menu{width:100%!important}table.menu td,table.menu th{width:auto!important;display:inline-block!important}table.menu.vertical td,table.menu.vertical th{display:block!important}table.menu[align=center]{width:auto!important}}</style>
+</head>
+<body style="-moz-box-sizing:border-box;-ms-text-size-adjust:100%;-webkit-box-sizing:border-box;-webkit-font-smoothing:antialiased;-webkit-text-size-adjust:100%;margin:0;background:#fff!important;box-sizing:border-box;color:#0a0a0a;font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Oxygen-Sans,Ubuntu,Cantarell,'Helvetica Neue',Arial,sans-serif;font-size:16px;font-weight:400;line-height:1.3;margin:0;min-width:100%;padding:0;text-align:left;width:100%!important">
+	<span class="preheader" style="color:#F5F5F5;display:none!important;font-size:1px;line-height:1px;max-height:0;max-width:0;mso-hide:all!important;opacity:0;overflow:hidden;visibility:hidden">
+	</span>
+	<table class="body" style="-webkit-font-smoothing:antialiased;margin:0;background:#fff;border-collapse:collapse;border-spacing:0;color:#0a0a0a;font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Oxygen-Sans,Ubuntu,Cantarell,'Helvetica Neue',Arial,sans-serif;font-size:16px;font-weight:400;line-height:1.3;margin:0;padding:0;text-align:left;vertical-align:top;width:100%">
+		<tr style="padding:0;text-align:left;vertical-align:top">
+			<td class="center" align="center" valign="top" style="-moz-hyphens:auto;-webkit-hyphens:auto;Margin:0;border-collapse:collapse!important;color:#0a0a0a;font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Oxygen-Sans,Ubuntu,Cantarell,'Helvetica Neue',Arial,sans-serif;font-size:16px;font-weight:400;hyphens:auto;line-height:1.3;margin:0;padding:0;text-align:left;vertical-align:top;word-wrap:break-word">
+				<center data-parsed="" style="width:100%;max-width:740px;margin:auto">

tests/lib/EMailTemplateTest.php

@@ -16,31 +16,35 @@
 class EMailTemplateTest extends TestCase {
 	public function testIncludeTemplateFileIsCovered(): void {
 		// Setup: create a real template file for 'head.html'
-		$templateDir = __DIR__ . '/../../lib/templates/email';
-		if (!is_dir($templateDir)) {
-			mkdir($templateDir, 0777, true);
-		}
+		$templateDir = __DIR__ . '/../../lib/templates/email/ionos';
 		$templateFile = $templateDir . '/head.html';
+		$originalContent = file_exists($templateFile) ? file_get_contents($templateFile) : null;
 		$expectedContent = '<div>Test Head Template</div>';
 		file_put_contents($templateFile, $expectedContent);
 
-		// Reset the property to empty string
-		$reflection = new \ReflectionClass($this->emailTemplate);
-		$prop = $reflection->getProperty('head');
-		$prop->setAccessible(true);
-		$prop->setValue($this->emailTemplate, '');
-
-		// Call the private method via reflection
-		$method = $reflection->getMethod('loadHtmlTemplateFiles');
-		$method->setAccessible(true);
-		$method->invoke($this->emailTemplate);
-
-		// Assert the property now contains the expected content
-		$value = $prop->getValue($this->emailTemplate);
-		$this->assertStringContainsString($expectedContent, $value);
-
-		// Cleanup
-		unlink($templateFile);
+		try {
+			// Reset the property to empty string
+			$reflection = new \ReflectionClass($this->emailTemplate);
+			$prop = $reflection->getProperty('head');
+			$prop->setAccessible(true);
+			$prop->setValue($this->emailTemplate, '');
+
+			// Call the private method via reflection
+			$method = $reflection->getMethod('loadHtmlTemplateFiles');
+			$method->setAccessible(true);
+			$method->invoke($this->emailTemplate);
+
+			// Assert the property now contains the expected content
+			$value = $prop->getValue($this->emailTemplate);
+			$this->assertStringContainsString($expectedContent, $value);
+		} finally {
+			// Cleanup: restore original content or remove file
+			if ($originalContent !== null) {
+				file_put_contents($templateFile, $originalContent);
+			} else {
+				unlink($templateFile);
+			}
+		}
 	}
 	public function testLoadHtmlTemplateFilesMethodIsCovered(): void {
 		// Reset template properties to empty string to verify method effect
@@ -103,7 +107,7 @@ public function testAssetUrlsAreGenerated(): void {
 		$this->assertStringStartsWith('https://example.org', $this->getPrivateProperty('spacerUrl'));
 		$this->assertStringContainsString('spacer.png', $this->getPrivateProperty('spacerUrl'));
 		$this->assertStringStartsWith('https://example.org', $this->getPrivateProperty('logoUrl'));
-		$this->assertStringContainsString('ionos_logo_de.png', $this->getPrivateProperty('logoUrl'));
+		$this->assertStringContainsString('ionos/logo.png', $this->getPrivateProperty('logoUrl'));
 		$this->assertStringStartsWith('https://example.org', $this->getPrivateProperty('emailIconUrl'));
 		$this->assertStringContainsString('email.png', $this->getPrivateProperty('emailIconUrl'));
 		$this->assertStringStartsWith('https://example.org', $this->getPrivateProperty('listItemIconUrl'));