From 534123cd87737994c23a531cc9dd4e26114d1d89 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 May 2026 15:10:38 +1200 Subject: [PATCH] test(notifications): add unit tests for dedup, fanout, console skip, zero-delivery, recipient struct, and tracking pixel Cover the new worker behaviors introduced in waves 1-3: - testDedupQueriesByAttributeNotById: prove alreadyDelivered() queries the messageId attribute, not getDocument($messageId), by seeding a row with a non-matching $id but matching messageId. - testConsoleChannelSkipsPersistAlert: confirm the action loop does NOT call persistAlert for console recipients (the adapter persists). - testConsoleZeroDeliveryThrows: surface adapter failures via the worker. - testMultiRecipientFanoutNoCollision: same messageId across recipients must produce distinct $id values. - testRecipientStructRoundtripsUserIdAndTeamId: persisted alert carries recipient userId/teamId. - testTrackingPixelInjectedIntoEmailHtml: verify the tracking pixel is spliced before the last tag with the expected URL shape. - testPersistAlertReturnsAlertIdAndStoresUserId: dispatchEmail's persistAlert returns a resolvable id and stores userId + read=false. ConsoleTest: add testMultiRecipientWithSameMessageIdGeneratesDistinctIds and update existing tests to use the post-ST2 compound `$id` form (messageId + 8-hex md5 suffix). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Platform/Workers/NotificationsTest.php | 298 ++++++++++++++++++ .../Utopia/Messaging/Adapter/ConsoleTest.php | 53 +++- 2 files changed, 348 insertions(+), 3 deletions(-) diff --git a/tests/unit/Platform/Workers/NotificationsTest.php b/tests/unit/Platform/Workers/NotificationsTest.php index 423e0846ad..3b77cc7c24 100644 --- a/tests/unit/Platform/Workers/NotificationsTest.php +++ b/tests/unit/Platform/Workers/NotificationsTest.php @@ -13,6 +13,8 @@ use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; use Utopia\Database\Validator\Authorization; use Utopia\Logger\Log; +use Utopia\Messaging\Adapter\Email as EmailAdapter; +use Utopia\Messaging\Messages\Email as EmailMessage; use Utopia\Queue\Message; use Utopia\Registry\Registry; @@ -67,6 +69,72 @@ class SpyNotifications extends Notifications } } +/** + * Worker that runs the real `dispatchConsole` (so the ConsoleAdapter writes + * the alert row) but counts how many times the action loop calls + * `persistAlert`. Used to assert that the loop does NOT double-persist for + * console recipients (Greptile P1 #3). + */ +class CountingPersistAlertNotifications extends Notifications +{ + public int $persistAlertCalls = 0; + + /** @var array */ + public array $persistedIds = []; + + protected function persistAlert(Database $database, string $messageId, array $recipient, array $payload): string + { + $this->persistAlertCalls++; + $alertId = parent::persistAlert($database, $messageId, $recipient, $payload); + $this->persistedIds[] = $alertId; + return $alertId; + } +} + +/** + * Worker that simulates a console-adapter zero-delivery outcome (e.g. + * permission denied on createDocument) without depending on a real adapter + * failure mode. + */ +class ZeroDeliveryConsoleNotifications extends Notifications +{ + protected function dispatchConsole(array $recipient, string $messageId, array $payload, Database $database): ?string + { + // Simulate the Console adapter swallowing a per-recipient + // exception and reporting zero deliveries. + throw new \Exception('Console alert delivery failed: permission denied'); + } +} + +/** + * Captures the EmailMessage handed to the SMTP adapter so tests can assert + * on the rendered HTML body without touching a real mail server. + */ +class SpyEmailAdapter extends EmailAdapter +{ + public ?EmailMessage $captured = null; + + public function getName(): string + { + return 'SpySMTP'; + } + + public function getMaxMessagesPerRequest(): int + { + return 1000; + } + + protected function process(EmailMessage $message): array + { + $this->captured = $message; + return [ + 'deliveredTo' => 1, + 'type' => $this->getType(), + 'results' => [['recipient' => $message->getTo()[0]['email'] ?? '', 'status' => 'sent']], + ]; + } +} + class NotificationsTest extends TestCase { private Database $database; @@ -296,4 +364,234 @@ class NotificationsTest extends TestCase $rows = $this->database->find('alerts'); $this->assertCount(0, $rows, 'failed dispatch must not persist alert'); } + + public function testDedupQueriesByAttributeNotById(): void + { + $worker = new SpyNotifications(); + + // Seed an alert row with an arbitrary $id but the matching dedup + // messageId attribute. If alreadyDelivered() short-circuits via + // getDocument($messageId) it will miss this seed and dispatch + // anyway. Querying by the `messageId` attribute is the only way to + // see the seed. + $messageId = \md5('dup-key'); + $this->database->createDocument('alerts', new Document([ + '$id' => 'random-id-123', + '$permissions' => [Permission::read(Role::any())], + 'messageId' => $messageId, + 'channel' => 'console', + 'title' => 'seed', + 'body' => 'seed', + ])); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [['address' => 'user-1', 'channel' => NOTIFICATION_TYPE_CONSOLE]], + 'subject' => 'Sub', + 'body' => 'B', + 'deduplicationKey' => 'dup-key', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + $this->assertCount(0, $worker->dispatched, 'attribute-keyed seed must trigger dedup short-circuit'); + $this->assertSame('hit', $this->log->getTags()['dedup'] ?? null); + } + + public function testConsoleChannelSkipsPersistAlert(): void + { + $worker = new CountingPersistAlertNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + ['address' => 'user-1', 'channel' => NOTIFICATION_TYPE_CONSOLE, 'userId' => 'user-1'], + ], + 'subject' => 'Heads up', + 'body' => 'console body', + 'deduplicationKey' => 'console-skip', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + // The Console adapter wrote exactly one alert; the action loop + // must NOT have called persistAlert (otherwise we'd see 2 rows or + // a duplicate-key swallow plus a non-zero counter). + $consoleRows = $this->database->find('alerts', [ + \Utopia\Database\Query::equal('channel', ['console']), + ]); + $this->assertCount(1, $consoleRows); + $this->assertSame(0, $worker->persistAlertCalls, 'console channel must NOT trigger action-loop persistAlert'); + } + + public function testConsoleZeroDeliveryThrows(): void + { + $worker = new ZeroDeliveryConsoleNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + ['address' => 'user-1', 'channel' => NOTIFICATION_TYPE_CONSOLE, 'userId' => 'user-1'], + ], + 'subject' => 'Heads up', + 'body' => 'console body', + 'deduplicationKey' => 'console-fail', + ]; + + try { + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + $this->fail('expected console zero-delivery to throw'); + } catch (\Throwable $error) { + $this->assertStringContainsString('Console alert delivery failed', $error->getMessage()); + } + + $this->assertCount(0, $this->database->find('alerts'), 'failed console dispatch must not persist alert'); + } + + public function testMultiRecipientFanoutNoCollision(): void + { + $worker = new CountingPersistAlertNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + ['address' => 'user-1', 'channel' => NOTIFICATION_TYPE_CONSOLE, 'userId' => 'user-1'], + ['address' => 'user-2', 'channel' => NOTIFICATION_TYPE_CONSOLE, 'userId' => 'user-2'], + ], + 'subject' => 'Heads up', + 'body' => 'console body', + 'deduplicationKey' => 'fanout', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + $rows = $this->database->find('alerts'); + $this->assertCount(2, $rows, 'two recipients must produce two distinct alert rows'); + + $messageId = \md5('fanout'); + $ids = []; + foreach ($rows as $row) { + $this->assertSame($messageId, $row->getAttribute('messageId'), 'all rows share the dedup messageId'); + $ids[] = $row->getId(); + } + $this->assertCount(2, \array_unique($ids), 'recipient suffixes must keep $id values distinct'); + } + + public function testRecipientStructRoundtripsUserIdAndTeamId(): void + { + $worker = new CountingPersistAlertNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'console-recipient', + 'channel' => NOTIFICATION_TYPE_CONSOLE, + 'userId' => 'u1', + 'teamId' => 't1', + ], + ], + 'subject' => 'Heads up', + 'body' => 'b', + 'deduplicationKey' => 'roundtrip', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + + $rows = $this->database->find('alerts'); + $this->assertCount(1, $rows); + $this->assertSame('u1', $rows[0]->getAttribute('userId')); + $this->assertSame('t1', $rows[0]->getAttribute('teamId')); + } + + public function testTrackingPixelInjectedIntoEmailHtml(): void + { + $spy = new SpyEmailAdapter(); + $this->registry->set('smtp', static fn () => $spy); + + // Force the cloud SMTP branch (project has no smtp config) and + // provide an OpenSSL key so injectTrackingPixel actually runs. + $previousSmtpHost = \getenv('_APP_SMTP_HOST'); + $previousOpensslKey = \getenv('_APP_OPENSSL_KEY_V1'); + \putenv('_APP_SMTP_HOST=spy.smtp.test'); + \putenv('_APP_OPENSSL_KEY_V1=test-key-32bytes-min-aaaaaaaaaaaaaa'); + + try { + $worker = new Notifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'user@example.test', + 'channel' => NOTIFICATION_TYPE_EMAIL, + 'userId' => 'user-7', + ], + ], + 'subject' => 'Heads up', + 'body' => 'plain body', + 'deduplicationKey' => 'pixel-key', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + } finally { + \putenv($previousSmtpHost === false ? '_APP_SMTP_HOST' : '_APP_SMTP_HOST=' . $previousSmtpHost); + \putenv($previousOpensslKey === false ? '_APP_OPENSSL_KEY_V1' : '_APP_OPENSSL_KEY_V1=' . $previousOpensslKey); + } + + $this->assertNotNull($spy->captured, 'SpyEmailAdapter must capture exactly one EmailMessage'); + + $body = $spy->captured->getContent(); + $this->assertStringContainsString(' must be present'); + $this->assertStringContainsString('/v1/account/alerts/', $body); + $this->assertStringContainsString('/track?jwt=', $body); + + // The pixel must sit BEFORE the last . + $lastBodyClose = \strripos($body, ''); + $pixelPosition = \strripos($body, ''); + $this->assertNotFalse($pixelPosition); + $this->assertLessThan($lastBodyClose, $pixelPosition, 'pixel must be spliced before the final '); + } + + public function testPersistAlertReturnsAlertIdAndStoresUserId(): void + { + $spy = new SpyEmailAdapter(); + $this->registry->set('smtp', static fn () => $spy); + + $previousSmtpHost = \getenv('_APP_SMTP_HOST'); + \putenv('_APP_SMTP_HOST=spy.smtp.test'); + + try { + $worker = new CountingPersistAlertNotifications(); + + $payload = [ + 'project' => ['$id' => 'project-x'], + 'recipients' => [ + [ + 'address' => 'user@example.test', + 'channel' => NOTIFICATION_TYPE_EMAIL, + 'userId' => 'user-7', + ], + ], + 'subject' => 'Heads up', + 'body' => 'b', + 'deduplicationKey' => 'persist-email', + ]; + + $worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log); + } finally { + \putenv($previousSmtpHost === false ? '_APP_SMTP_HOST' : '_APP_SMTP_HOST=' . $previousSmtpHost); + } + + $this->assertSame(1, $worker->persistAlertCalls, 'email channel must persist exactly once'); + $this->assertCount(1, $worker->persistedIds); + + $alertId = $worker->persistedIds[0]; + $row = $this->database->getDocument('alerts', $alertId); + $this->assertFalse($row->isEmpty(), 'persistAlert must return an id resolvable via getDocument'); + $this->assertSame('user-7', $row->getAttribute('userId')); + $this->assertFalse($row->getAttribute('read'), 'new alerts default to unread'); + $this->assertSame(\md5('persist-email'), $row->getAttribute('messageId')); + } } diff --git a/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php b/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php index 283fd17819..d1165fa002 100644 --- a/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php +++ b/tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php @@ -48,6 +48,16 @@ class ConsoleTest extends TestCase $this->authorization->addRole(Role::any()->toString()); } + /** + * Mirrors the per-recipient `$id` derivation in + * Appwrite\Utopia\Messaging\Adapter\Console::process(). + */ + private function alertId(string $messageId, string $userId = '', string $teamId = ''): string + { + $key = $userId !== '' ? 'user:' . $userId : 'team:' . $teamId; + return $messageId . '_' . \substr(\md5($key), 0, 8); + } + public function testWritesAlertWithCorrectSchema(): void { $message = new ConsoleMessage( @@ -64,7 +74,7 @@ class ConsoleTest extends TestCase $this->assertSame(1, $result['deliveredTo']); - $stored = $this->database->getDocument('alerts', 'msg-aaa'); + $stored = $this->database->getDocument('alerts', $this->alertId('msg-aaa', userId: 'user-1')); $this->assertFalse($stored->isEmpty()); $this->assertSame('msg-aaa', $stored->getAttribute('messageId')); $this->assertSame('console', $stored->getAttribute('channel')); @@ -86,7 +96,7 @@ class ConsoleTest extends TestCase (new Console($this->database))->send($message); - $stored = $this->database->getDocument('alerts', 'msg-perms-user'); + $stored = $this->database->getDocument('alerts', $this->alertId('msg-perms-user', userId: 'user-2')); $permissions = $stored->getPermissions(); $this->assertContains(Permission::read(Role::user('user-2')), $permissions); @@ -105,7 +115,7 @@ class ConsoleTest extends TestCase (new Console($this->database))->send($message); - $stored = $this->database->getDocument('alerts', 'msg-team'); + $stored = $this->database->getDocument('alerts', $this->alertId('msg-team', teamId: 'team-9')); $permissions = $stored->getPermissions(); $this->assertContains(Permission::read(Role::team('team-9')), $permissions); @@ -113,6 +123,43 @@ class ConsoleTest extends TestCase $this->assertContains(Permission::delete(Role::team('team-9', 'owner')), $permissions); } + public function testMultiRecipientWithSameMessageIdGeneratesDistinctIds(): void + { + $message = new ConsoleMessage( + recipients: [ + ['userId' => 'a'], + ['userId' => 'b'], + ], + title: 'Heads up', + body: 'multi', + messageId: ID::custom('same-msg'), + ); + + $adapter = new Console($this->database); + $result = $adapter->send($message); + + $this->assertSame(2, $result['deliveredTo']); + + $idA = $this->alertId('same-msg', userId: 'a'); + $idB = $this->alertId('same-msg', userId: 'b'); + + $this->assertNotSame($idA, $idB, 'recipient suffixes must be distinct'); + + $rowA = $this->database->getDocument('alerts', $idA); + $rowB = $this->database->getDocument('alerts', $idB); + $this->assertFalse($rowA->isEmpty(), 'first recipient row must exist'); + $this->assertFalse($rowB->isEmpty(), 'second recipient row must exist'); + + $this->assertSame('same-msg', $rowA->getAttribute('messageId')); + $this->assertSame('same-msg', $rowB->getAttribute('messageId')); + $this->assertSame('a', $rowA->getAttribute('userId')); + $this->assertSame('b', $rowB->getAttribute('userId')); + + // $id values must be `messageId_<8-hex>` and the suffixes differ. + $this->assertMatchesRegularExpression('/^same-msg_[0-9a-f]{8}$/', $idA); + $this->assertMatchesRegularExpression('/^same-msg_[0-9a-f]{8}$/', $idB); + } + public function testRejectsForeignMessageType(): void { $adapter = new Console($this->database);