From 103e94a0c02f33013590cfadf2b34d2d8e0fdff6 Mon Sep 17 00:00:00 2001 From: David Kohr Date: Thu, 17 Sep 2026 15:44:51 +0200 Subject: [PATCH 1/3] [BUGFIX] Throw a speaking exception on missing summary mail settings ConfigurationService::getTypoScriptSettingsByPath() returns an empty string when the given path cannot be resolved. SendSummaryService used that return value as an array, which fails with TypeError: Cannot access offset of type string on string The error points at SendSummaryService instead of the actual cause: the static TypoScript of EXT:lux is not part of the TypoScript template of the site the command runs against. Multi-site installations run into this easily, because ExtbaseCommandTrait binds the request to SiteService::getDefaultSite(), which is the first site returned by the SiteFinder and not necessarily the site lux is configured for. The configuration is now resolved once, validated, and reused by getSender(), getSubject() and getMailTemplate(). A missing configuration raises a ConfigurationException naming the TypoScript path that has to be included. --- .../Service/Email/SendSummaryService.php | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/Classes/Domain/Service/Email/SendSummaryService.php b/Classes/Domain/Service/Email/SendSummaryService.php index c8ec6619..d56b8ea1 100644 --- a/Classes/Domain/Service/Email/SendSummaryService.php +++ b/Classes/Domain/Service/Email/SendSummaryService.php @@ -54,33 +54,57 @@ public function send(array $emails): bool /** * @return array + * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ protected function getSender(): array { - $configuration = $this->configurationService->getTypoScriptSettingsByPath('commandControllers.summaryMail'); + $configuration = $this->getSummaryMailConfiguration(); return [$configuration['fromEmail'] => $configuration['fromName']]; } /** * @return string + * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ protected function getSubject(): string { - return $this->configurationService->getTypoScriptSettingsByPath('commandControllers.summaryMail.subject'); + return $this->getSummaryMailConfiguration()['subject'] ?? ''; + } + + /** + * ConfigurationService::getTypoScriptSettingsByPath() returns an empty string if the path could not be + * resolved. Accessing that string with an array offset leads to a TypeError that does not tell the + * integrator anything about the actual problem, so fail with a speaking exception instead. + * + * @return array + * @throws ConfigurationException + * @throws InvalidConfigurationTypeException + */ + protected function getSummaryMailConfiguration(): array + { + $configuration = $this->configurationService->getTypoScriptSettingsByPath('commandControllers.summaryMail'); + if (is_array($configuration) === false) { + throw new ConfigurationException( + 'TypoScript setting plugin.tx_lux_fe.settings.commandControllers.summaryMail could not be ' + . 'resolved. Please add the static TypoScript of EXT:lux to the TypoScript template of the ' + . 'site that is used by this command.', + 1789652586 + ); + } + return $configuration; } /** * @param array $assignment * @return string + * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ protected function getMailTemplate(array $assignment = []): string { - $mailTemplatePath = $this->configurationService->getTypoScriptSettingsByPath( - 'commandControllers.summaryMail.mailTemplate' - ); + $mailTemplatePath = $this->getSummaryMailConfiguration()['mailTemplate'] ?? ''; $view = GeneralUtility::makeInstance(ViewFactoryInterface::class)->create(new ViewFactoryData( templatePathAndFilename: GeneralUtility::getFileAbsFileName($mailTemplatePath), )); From c31dac2c8f8010e99f2bd5d52c768dd8c05e9bad Mon Sep 17 00:00:00 2001 From: Bastien Lutz Date: Fri, 18 Sep 2026 15:09:54 +0200 Subject: [PATCH 2/3] [TASK] Add unit tests for SendSummaryService --- .../Service/Email/SendSummaryServiceTest.php | 205 ++++++++++++++++++ .../Email/SendSummaryServiceFixture.php | 58 +++++ 2 files changed, 263 insertions(+) create mode 100644 Tests/Unit/Domain/Service/Email/SendSummaryServiceTest.php create mode 100644 Tests/Unit/Fixtures/Domain/Service/Email/SendSummaryServiceFixture.php diff --git a/Tests/Unit/Domain/Service/Email/SendSummaryServiceTest.php b/Tests/Unit/Domain/Service/Email/SendSummaryServiceTest.php new file mode 100644 index 00000000..751b0fe7 --- /dev/null +++ b/Tests/Unit/Domain/Service/Email/SendSummaryServiceTest.php @@ -0,0 +1,205 @@ + 'sender@domain.org', + 'fromName' => 'Sender Name', + 'subject' => 'Your lead summary', + 'mailTemplate' => 'EXT:lux/Resources/Private/Templates/Mail/SummaryMail.html', + ]; + + protected bool $resetSingletonInstances = true; + + public function setUp(): void + { + parent::setUp(); + TestingHelper::setDefaultConstants(); + } + + #[Test] + public function testGetSummaryMailConfigurationReturnsConfiguration(): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION); + self::assertSame(self::VALID_CONFIGURATION, $service->getSummaryMailConfigurationPublic()); + } + + #[Test] + public function testGetSenderReturnsEmailAndName(): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION); + self::assertSame(['sender@domain.org' => 'Sender Name'], $service->getSenderPublic()); + } + + #[Test] + public function testGetSubjectReturnsSubject(): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION); + self::assertSame('Your lead summary', $service->getSubjectPublic()); + } + + #[Test] + public function testGetSubjectReturnsEmptyStringOnMissingSubject(): void + { + $configuration = self::VALID_CONFIGURATION; + unset($configuration['subject']); + $service = $this->getServiceFixture($configuration); + self::assertSame('', $service->getSubjectPublic()); + } + + public static function missingConfigurationDataProvider(): array + { + return [ + 'empty string from unresolvable typoscript path' => [''], + 'string instead of array' => ['summaryMail'], + 'null' => [null], + 'integer' => [0], + ]; + } + + #[Test] + #[DataProvider('missingConfigurationDataProvider')] + public function testGetSummaryMailConfigurationThrowsExceptionOnMissingConfiguration( + mixed $configuration + ): void { + $service = $this->getServiceFixture($configuration); + $this->expectException(ConfigurationException::class); + $this->expectExceptionCode(self::MISSING_CONFIGURATION_EXCEPTION_CODE); + $service->getSummaryMailConfigurationPublic(); + } + + #[Test] + public function testGetSummaryMailConfigurationExceptionMessageNamesTypoScriptPath(): void + { + $service = $this->getServiceFixture(''); + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessageMatches('~plugin\.tx_lux_fe\.settings\.commandControllers\.summaryMail~'); + $service->getSummaryMailConfigurationPublic(); + } + + #[Test] + public function testGetSenderThrowsExceptionOnMissingConfiguration(): void + { + $service = $this->getServiceFixture(''); + $this->expectException(ConfigurationException::class); + $this->expectExceptionCode(self::MISSING_CONFIGURATION_EXCEPTION_CODE); + $service->getSenderPublic(); + } + + #[Test] + public function testGetSubjectThrowsExceptionOnMissingConfiguration(): void + { + $service = $this->getServiceFixture(''); + $this->expectException(ConfigurationException::class); + $this->expectExceptionCode(self::MISSING_CONFIGURATION_EXCEPTION_CODE); + $service->getSubjectPublic(); + } + + #[Test] + public function testGetMailTemplateThrowsExceptionOnMissingConfiguration(): void + { + $service = $this->getServiceFixture(''); + $this->expectException(ConfigurationException::class); + $this->expectExceptionCode(self::MISSING_CONFIGURATION_EXCEPTION_CODE); + $service->getMailTemplatePublic(); + } + + #[Test] + public function testConfigurationIsAlwaysResolvedFromTheSameTypoScriptPath(): void + { + $configurationService = $this->createMock(ConfigurationService::class); + $configurationService + ->expects(self::atLeastOnce()) + ->method('getTypoScriptSettingsByPath') + ->with('commandControllers.summaryMail') + ->willReturn(self::VALID_CONFIGURATION); + GeneralUtility::setSingletonInstance(ConfigurationService::class, $configurationService); + + $service = new SendSummaryServiceFixture([new Visitor()]); + $service->getSenderPublic(); + $service->getSubjectPublic(); + } + + #[Test] + public function testCheckPropertiesThrowsExceptionOnEmptyEmails(): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION); + $this->expectException(ConfigurationException::class); + $this->expectExceptionCode(1524299754); + $service->checkPropertiesPublic([]); + } + + public static function invalidEmailDataProvider(): array + { + return [ + 'no email at all' => ['receiver'], + 'missing domain' => ['receiver@'], + 'missing local part' => ['@domain.org'], + 'whitespace' => ['receiver @domain.org'], + ]; + } + + #[Test] + #[DataProvider('invalidEmailDataProvider')] + public function testCheckPropertiesThrowsExceptionOnInvalidEmail(string $email): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION); + $this->expectException(EmailValidationException::class); + $this->expectExceptionCode(1524299869); + $service->checkPropertiesPublic([$email]); + } + + #[Test] + public function testCheckPropertiesThrowsExceptionOnMissingVisitors(): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION, []); + $this->expectException(ConfigurationException::class); + $this->expectExceptionCode(1524300114); + $service->checkPropertiesPublic(['receiver@domain.org']); + } + + #[Test] + public function testCheckPropertiesPassesWithValidProperties(): void + { + $service = $this->getServiceFixture(self::VALID_CONFIGURATION); + $this->expectNotToPerformAssertions(); + $service->checkPropertiesPublic(['receiver@domain.org', 'receiver2@domain.org']); + } + + protected function getServiceFixture(mixed $configuration, ?array $visitors = null): SendSummaryServiceFixture + { + $configurationService = self::createStub(ConfigurationService::class); + $configurationService + ->method('getTypoScriptSettingsByPath') + ->willReturn($configuration); + GeneralUtility::setSingletonInstance(ConfigurationService::class, $configurationService); + return new SendSummaryServiceFixture($visitors ?? [new Visitor()]); + } +} diff --git a/Tests/Unit/Fixtures/Domain/Service/Email/SendSummaryServiceFixture.php b/Tests/Unit/Fixtures/Domain/Service/Email/SendSummaryServiceFixture.php new file mode 100644 index 00000000..5cdbb60d --- /dev/null +++ b/Tests/Unit/Fixtures/Domain/Service/Email/SendSummaryServiceFixture.php @@ -0,0 +1,58 @@ +getSummaryMailConfiguration(); + } + + /** + * @throws ConfigurationException + * @throws InvalidConfigurationTypeException + */ + public function getSenderPublic(): array + { + return $this->getSender(); + } + + /** + * @throws ConfigurationException + * @throws InvalidConfigurationTypeException + */ + public function getSubjectPublic(): string + { + return $this->getSubject(); + } + + /** + * @throws ConfigurationException + * @throws InvalidConfigurationTypeException + */ + public function getMailTemplatePublic(array $assignment = []): string + { + return $this->getMailTemplate($assignment); + } + + /** + * @throws ConfigurationException + * @throws EmailValidationException + */ + public function checkPropertiesPublic(array $emails): void + { + $this->checkProperties($emails); + } +} From 579273bac24cb139606e8269e6a6e87cb5b83616 Mon Sep 17 00:00:00 2001 From: Bastien Lutz Date: Fri, 18 Sep 2026 15:09:54 +0200 Subject: [PATCH 3/3] [TASK] Clean up docblocks in SendSummaryService --- .../Service/Email/SendSummaryService.php | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/Classes/Domain/Service/Email/SendSummaryService.php b/Classes/Domain/Service/Email/SendSummaryService.php index d56b8ea1..023d3871 100644 --- a/Classes/Domain/Service/Email/SendSummaryService.php +++ b/Classes/Domain/Service/Email/SendSummaryService.php @@ -22,9 +22,6 @@ class SendSummaryService protected array $visitors; protected ?ConfigurationService $configurationService = null; - /** - * @param array $visitors - */ public function __construct(array $visitors) { $this->visitors = $visitors; @@ -32,8 +29,6 @@ public function __construct(array $visitors) } /** - * @param array $emails - * @return bool * @throws ConfigurationException * @throws EmailValidationException * @throws InvalidConfigurationTypeException @@ -53,7 +48,6 @@ public function send(array $emails): bool } /** - * @return array * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ @@ -64,7 +58,6 @@ protected function getSender(): array } /** - * @return string * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ @@ -74,11 +67,6 @@ protected function getSubject(): string } /** - * ConfigurationService::getTypoScriptSettingsByPath() returns an empty string if the path could not be - * resolved. Accessing that string with an array offset leads to a TypeError that does not tell the - * integrator anything about the actual problem, so fail with a speaking exception instead. - * - * @return array * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ @@ -97,8 +85,6 @@ protected function getSummaryMailConfiguration(): array } /** - * @param array $assignment - * @return string * @throws ConfigurationException * @throws InvalidConfigurationTypeException */ @@ -113,12 +99,10 @@ protected function getMailTemplate(array $assignment = []): string } /** - * @param array $emails - * @return void * @throws EmailValidationException * @throws ConfigurationException */ - protected function checkProperties(array $emails) + protected function checkProperties(array $emails): void { if ($emails === []) { throw new ConfigurationException('No emails to send given', 1524299754);