Appwrite/appwrite
1
0
Fork 0
mirror of https://github.com/appwrite/appwrite.git synced 2026-05-26 13:51:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address notifications review comments

Browse Source
This commit is contained in:
Jake Barnby
2026-05-16 19:37:21 +12:00
parent bc9e67dc5b
commit 70df121e6f
12 changed files with 382 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/config/collections/platform.php
+42 -3
View File
@@ -1048,6 +1048,17 @@ $platformCollections = [
'array' => false,
'filters' => [],
],
[
'$id' => ID::custom('recipientHash'),
'type' => Database::VAR_STRING,
'format' => '',
'size' => 64,
'signed' => true,
'required' => true,
'default' => null,
'array' => false,
'filters' => [],
],
[
'$id' => ID::custom('type'),
'type' => Database::VAR_STRING,
@@ -1148,9 +1159,9 @@ $platformCollections = [
[
'$id' => ID::custom('_key_recipient'),
'type' => Database::INDEX_UNIQUE,
'attributes' => ['messageId', 'channel', 'userId', 'teamId'],
'lengths' => [Database::LENGTH_KEY, 64, Database::LENGTH_KEY, Database::LENGTH_KEY],
'orders' => [Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC],
'attributes' => ['messageId', 'channel', 'recipientHash'],
'lengths' => [Database::LENGTH_KEY, 64, 64],
'orders' => [Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC],
],
[
'$id' => ID::custom('_key_userId_read'),
@@ -1173,6 +1184,34 @@ $platformCollections = [
'lengths' => [Database::LENGTH_KEY],
'orders' => [Database::ORDER_ASC],
],
[
'$id' => ID::custom('_key_userId_type'),
'type' => Database::INDEX_KEY,
'attributes' => ['userId', 'type'],
'lengths' => [Database::LENGTH_KEY, 64],
'orders' => [Database::ORDER_ASC, Database::ORDER_ASC],
],
[
'$id' => ID::custom('_key_userId_channel'),
'type' => Database::INDEX_KEY,
'attributes' => ['userId', 'channel'],
'lengths' => [Database::LENGTH_KEY, 64],
'orders' => [Database::ORDER_ASC, Database::ORDER_ASC],
],
[
'$id' => ID::custom('_key_userId_messageId'),
'type' => Database::INDEX_KEY,
'attributes' => ['userId', 'messageId'],
'lengths' => [Database::LENGTH_KEY, Database::LENGTH_KEY],
'orders' => [Database::ORDER_ASC, Database::ORDER_ASC],
],
[
'$id' => ID::custom('_key_userId_projectId'),
'type' => Database::INDEX_KEY,
'attributes' => ['userId', 'projectId'],
'lengths' => [Database::LENGTH_KEY, Database::LENGTH_KEY],
'orders' => [Database::ORDER_ASC, Database::ORDER_ASC],
],
],
],
app/init/constants.php
+4 -4
View File
@@ -263,12 +263,12 @@ const MESSAGE_TYPE_EMAIL = 'email';
const MESSAGE_TYPE_SMS = 'sms';
const MESSAGE_TYPE_PUSH = 'push';
// Notification types
const NOTIFICATION_TYPE_EMAIL = 'email';
const NOTIFICATION_TYPE_SMS = 'sms';
const NOTIFICATION_TYPE_PUSH = 'push';
const NOTIFICATION_TYPE_EMAIL = MESSAGE_TYPE_EMAIL;
const NOTIFICATION_TYPE_SMS = MESSAGE_TYPE_SMS;
const NOTIFICATION_TYPE_PUSH = MESSAGE_TYPE_PUSH;
const NOTIFICATION_TYPE_CONSOLE = 'console';
const NOTIFICATION_TYPE_WEBHOOK = 'webhook';
const RESOURCE_TYPE_ALERTS = 'alerts';
const ALERT_TRACKING_JWT_TTL = 60 * 60 * 24 * 7;
// API key types
const API_KEY_STANDARD = 'standard';
const API_KEY_EPHEMERAL = 'ephemeral';
src/Appwrite/Event/Notification.php
+7 -1
View File
@@ -20,7 +20,7 @@ class Notification extends Event
/**
* Recipients to deliver the notification to.
*
* Each entry has an `address` (channel-specific identifier — email,
* Each entry has an `address` (channel-specific identifier: email,
* userId, or webhook URL) and a `channel`. Webhook recipients may
* additionally carry an optional `signatureKey`; when set, the
* webhook adapter signs the request body with HMAC-SHA256 and adds
@@ -267,7 +267,13 @@ class Notification extends Event
public function reset(): self
{
parent::reset();
$this->project = null;
$this->context = [];
$this->platform = [];
$this->user = null;
$this->userId = null;
$this->recipient = '';
$this->name = '';
$this->subject = '';
src/Appwrite/Platform/Modules/Account/Http/Alerts/Track/Get.php
+1 -1
View File
@@ -66,7 +66,7 @@ class Get extends Action
if ($secret !== '' && $jwt !== '') {
try {
$decoder = new JWT($secret, 'HS256', 2592000, 0);
$decoder = new JWT($secret, 'HS256', ALERT_TRACKING_JWT_TTL, 0);
$decoded = $decoder->decode($jwt);
if (
src/Appwrite/Platform/Workers/Notifications.php
+95 -71
View File
@@ -16,7 +16,7 @@ use Utopia\Database\Document;
use Utopia\Database\Exception\Duplicate as DuplicateException;
use Utopia\Database\Helpers\Permission;
use Utopia\Database\Helpers\Role;
use Utopia\Database\Query;
use Utopia\Database\Validator\UID;
use Utopia\Logger\Log;
use Utopia\Messaging\Adapter\Email as EmailAdapter;
use Utopia\Messaging\Adapter\Email\SMTP;
@@ -32,11 +32,6 @@ class Notifications extends Action
protected int $previewMaxLen = 150;
protected string $whitespaceCodes = ' ‌​‍‎‏';
/**
* Tracking pixel JWT lifetime: 30 days.
*/
private const TRACKING_JWT_TTL = 2592000;
/**
* @var array<string, string>
*/
@@ -76,18 +71,21 @@ class Notifications extends Action
$deduplicationKey = $payload['deduplicationKey'] ?? '';
$messageId = $deduplicationKey !== '' ? \md5($deduplicationKey) : '';
if ($messageId !== '' && $this->alreadyDelivered($dbForPlatform, $messageId)) {
$log->addTag('dedup', 'hit');
return;
}
$recipients = $this->resolveRecipients($payload);
if (empty($recipients)) {
throw new Exception('No recipients in payload');
}
foreach ($recipients as $recipient) {
$recipient = $this->normalizeRecipient($recipient);
$channel = $recipient['channel'];
if ($messageId !== '' && $this->alreadyDelivered($dbForPlatform, self::buildAlertId($messageId, $recipient))) {
$log->addTag('dedup', 'hit');
$log->addTag('channel', $channel);
continue;
}
try {
$alertId = $this->dispatch($recipient, $messageId, $payload, $project, $register, $dbForPlatform, $log);
if ($messageId !== '' && $channel === NOTIFICATION_TYPE_WEBHOOK && $alertId === null) {
@@ -120,23 +118,28 @@ class Notifications extends Action
}
/**
* Look up an existing alert by the indexed `messageId` attribute.
*
* Greptile P1 #1: persistAlert and the Console adapter both write
* compound `$id`s (messageId + recipient hash), so a direct
* `getDocument($messageId)` would always miss. Query the attribute.
* @param array{address: string, channel: string, signatureKey?: string, userId?: string, teamId?: string} $recipient
* @return array{address: string, channel: string, signatureKey?: string, userId?: string, teamId?: string}
*/
private function alreadyDelivered(Database $dbForPlatform, string $messageId): bool
private function normalizeRecipient(array $recipient): array
{
try {
$matches = $dbForPlatform->find('alerts', [
Query::equal('messageId', [$messageId]),
Query::limit(1),
]);
return !empty($matches);
} catch (Throwable) {
return false;
$recipient['userId'] = $recipient['userId'] ?? '';
$recipient['teamId'] = $recipient['teamId'] ?? '';
if (
$recipient['channel'] === NOTIFICATION_TYPE_CONSOLE
&& $recipient['userId'] === ''
&& $recipient['teamId'] === ''
) {
$recipient['userId'] = $recipient['address'];
}
return $recipient;
}
private function alreadyDelivered(Database $dbForPlatform, string $alertId): bool
{
return !$dbForPlatform->getDocument('alerts', $alertId)->isEmpty();
}
/**
@@ -183,7 +186,8 @@ class Notifications extends Action
$smtp = $this->resolveSmtpConfig($project);
if (empty($smtp) && empty(System::getEnv('_APP_SMTP_HOST'))) {
throw new Exception('Skipped email notification. No SMTP configuration has been set.');
$log->addTag('email_skipped', 'no_smtp');
return null;
}
$type = empty($smtp) ? 'cloud' : 'smtp';
@@ -247,16 +251,10 @@ class Notifications extends Action
}
$subject = \strip_tags($subjectTemplate->render());
// Pre-compute the alertId WITHOUT persisting so the tracking pixel
// URL is stable, but defer the database write until after a
// successful adapter send. Persisting first risks a dedup-by-attribute
// hit on retry if SMTP throws — the email would be permanently lost.
$deterministicAlertId = $messageId !== ''
? self::buildAlertId($messageId, $recipient)
: null;
// Tracking pixel: only injectable when we have a userId AND a
// deterministic alertId AND a signing key.
$userId = $recipient['userId'] ?? '';
$opensslKey = System::getEnv('_APP_OPENSSL_KEY_V1');
if ($deterministicAlertId !== null && $userId !== '' && !empty($opensslKey)) {
@@ -320,16 +318,9 @@ class Notifications extends Action
try {
$adapter->send($emailMessage);
} catch (Throwable $error) {
// Use 500 for any SMTP delivery failure: timeouts, DNS, refused
// connections, and authentication errors are all infrastructure
// problems, not HTTP-style 401s. The queue infrastructure handles
// retries based on the throw, not the code.
throw new Exception('Error sending notification: ' . $error->getMessage(), 500);
}
// Persist the alert ONLY after a successful send. The Track endpoint
// tolerates a missing alert row, so a tracking pixel fetched between
// send and persistence is harmless.
if ($messageId !== '') {
return $this->persistAlert($dbForPlatform, $messageId, $recipient, $payload);
}
@@ -342,22 +333,22 @@ class Notifications extends Action
*/
protected function dispatchConsole(array $recipient, string $messageId, array $payload, Database $dbForPlatform): ?string
{
$this->validateConsoleRecipient($recipient);
$project = $payload['project'] ?? null;
$projectId = \is_array($project) ? ($project['$id'] ?? null) : null;
$title = $payload['subject'] ?? '';
$body = $payload['body'] ?? '';
$params = $payload['templateParams'] ?? ($payload['variables'] ?? []);
if ($title !== '' && !empty($params)) {
$rendered = Template::fromString($title);
foreach ($params as $key => $value) {
$rendered->setParam('{{' . $key . '}}', (string) $value);
}
$title = \strip_tags($rendered->render());
$title = self::renderText($payload['subject'] ?? '', $params);
$body = self::renderText($payload['preview'] ?? '', $params);
if ($body === '') {
$body = self::renderText($payload['body'] ?? '', $params);
}
$userId = $recipient['userId'] ?? $recipient['address'];
$teamId = $recipient['teamId'] ?? '';
$alertId = $messageId !== '' ? self::buildAlertId($messageId, $recipient) : null;
$recipientHash = $messageId !== '' ? self::buildRecipientHash($recipient) : null;
$consoleRecipient = [];
if ($userId !== '') {
@@ -366,6 +357,10 @@ class Notifications extends Action
if ($teamId !== '') {
$consoleRecipient['teamId'] = $teamId;
}
if ($alertId !== null) {
$consoleRecipient['alertId'] = $alertId;
$consoleRecipient['recipientHash'] = $recipientHash;
}
$consoleMessage = new ConsoleMessage(
recipients: [$consoleRecipient],
@@ -379,18 +374,12 @@ class Notifications extends Action
$adapter = new ConsoleAdapter($dbForPlatform);
$result = $adapter->send($consoleMessage);
// Greptile P1 #4: surface adapter failures. The Console adapter
// catches per-recipient exceptions and reports zero deliveries via
// `deliveredTo`; without this throw the worker would silently
// succeed on a hard write failure.
if (($result['deliveredTo'] ?? 0) === 0) {
$error = $result['results'][0]['error'] ?? 'unknown error';
throw new Exception('Console alert delivery failed: ' . $error);
}
// Adapter persisted the alert, so the action loop must NOT
// call persistAlert again (Greptile P1 #3).
return null;
return $alertId;
}
/**
@@ -444,39 +433,41 @@ class Notifications extends Action
*/
protected function persistAlert(Database $dbForPlatform, string $messageId, array $recipient, array $payload): string
{
$recipient = $this->normalizeRecipient($recipient);
$project = $payload['project'] ?? null;
$projectId = \is_array($project) ? ($project['$id'] ?? null) : null;
$channel = $recipient['channel'];
$address = $recipient['address'];
$userId = $recipient['userId'] ?? '';
$teamId = $recipient['teamId'] ?? '';
// Console alerts derive userId from address when no explicit
// userId is supplied (matches Console adapter's own bookkeeping).
if ($channel === NOTIFICATION_TYPE_CONSOLE && $userId === '' && $teamId === '') {
$userId = $address;
$recipient['userId'] = $userId;
}
$alertId = self::buildAlertId($messageId, $recipient);
$recipientHash = self::buildRecipientHash($recipient);
$permissions = $this->buildAlertPermissions($userId, $teamId);
if (empty($permissions)) {
$permissions = $payload['permissions'] ?? [];
}
$params = $payload['templateParams'] ?? ($payload['variables'] ?? []);
$body = self::renderText($payload['preview'] ?? '', $params);
if ($body === '') {
$body = self::renderText($payload['body'] ?? '', $params);
}
$document = new Document([
'$id' => $alertId,
'$permissions' => $permissions,
'messageId' => $messageId,
'recipientHash' => $recipientHash,
'type' => $payload['type'] ?? 'info',
'channel' => $channel,
'userId' => $userId !== '' ? $userId : null,
'teamId' => $teamId !== '' ? $teamId : null,
'userId' => $userId,
'teamId' => $teamId,
'projectId' => $projectId,
'title' => $payload['subject'] ?? '',
'body' => $payload['body'] ?? '',
'title' => self::renderText($payload['subject'] ?? '', $params),
'body' => $body,
'read' => false,
]);
@@ -490,21 +481,29 @@ class Notifications extends Action
}
/**
* Build the deterministic alertId composed of the messageId plus an
* 8-char hash over the recipient identity. Single source of truth used
* Build the deterministic alertId composed of the messageId plus a
* recipient hash. Single source of truth used
* by both `persistAlert()` and `dispatchEmail()` so the tracking pixel
* URL matches the eventually-persisted row exactly.
*
* @param array{address: string, channel: string, signatureKey?: string, userId?: string, teamId?: string} $recipient
*/
private static function buildAlertId(string $messageId, array $recipient): string
{
return \substr($messageId, 0, 19) . '_' . self::buildRecipientHash($recipient);
}
/**
* @param array{address: string, channel: string, signatureKey?: string, userId?: string, teamId?: string} $recipient
*/
private static function buildRecipientHash(array $recipient): string
{
$channel = $recipient['channel'];
$address = $recipient['address'];
$userId = $recipient['userId'] ?? '';
$teamId = $recipient['teamId'] ?? '';
return $messageId . '_' . \substr(\md5($channel . ':' . $address . ':' . $userId . ':' . $teamId), 0, 8);
return \substr(\md5($channel . ':' . $address . ':' . $userId . ':' . $teamId), 0, 16);
}
/**
@@ -555,16 +554,41 @@ class Notifications extends Action
];
}
private function validateConsoleRecipient(array $recipient): void
{
$validator = new UID();
foreach (['userId', 'teamId'] as $key) {
$value = $recipient[$key] ?? '';
if ($value !== '' && !$validator->isValid($value)) {
throw new Exception('Invalid console alert ' . $key . ': ' . $validator->getDescription());
}
}
}
private static function renderText(string $value, array $params): string
{
if ($value !== '' && !empty($params)) {
$template = Template::fromString($value);
foreach ($params as $key => $param) {
$template->setParam('{{' . $key . '}}', (string) $param);
}
$value = $template->render();
}
return \trim(\strip_tags($value));
}
/**
* Splice a 1x1 tracking pixel before the last `</body>` tag (or
* append at the end if the body has no closing tag). The pixel
* carries a 30-day JWT identifying the alert and user, which the
* carries a signed JWT identifying the alert and user, which the
* `/v1/account/alerts/:alertId/track` endpoint verifies before
* marking the alert as read.
*/
private function injectTrackingPixel(string $body, string $alertId, string $userId, string $opensslKey): string
{
$jwt = (new JWT($opensslKey, 'HS256', self::TRACKING_JWT_TTL, 0))
$jwt = (new JWT($opensslKey, 'HS256', ALERT_TRACKING_JWT_TTL, 0))
->encode([
'alertId' => $alertId,
'userId' => $userId,
src/Appwrite/Platform/Workers/Webhooks.php
+20 -2
View File
@@ -219,10 +219,9 @@ class Webhooks extends Action
Query::limit(APP_LIMIT_SUBQUERY)
]);
// Webhook-paused alerts go only to project owners — non-owner team members do not receive them.
$ownerMemberships = \array_filter(
$memberships,
fn (Document $membership) => \in_array('owner', $membership->getAttribute('roles', []), true)
fn (Document $membership) => self::hasOwnerRole($membership)
);
if (empty($ownerMemberships)) {
@@ -310,4 +309,23 @@ class Webhooks extends Action
$queueForNotifications->trigger();
}
private static function hasOwnerRole(Document $membership): bool
{
$roles = $membership->getAttribute('roles', []);
if (\is_string($roles)) {
$roles = \array_map('trim', \explode(',', $roles));
}
if (!\is_array($roles)) {
return false;
}
foreach ($roles as $role) {
if (\is_string($role) && \strtolower($role) === 'owner') {
return true;
}
}
return false;
}
}
src/Appwrite/Utopia/Messaging/Adapter/Console.php
+5 -3
View File
@@ -64,8 +64,9 @@ class Console extends Adapter
$messageId = $message->getMessageId();
$recipientKey = $userId !== '' ? 'user:' . $userId : 'team:' . $teamId;
$recipientHash = $recipient['recipientHash'] ?? \substr(\md5($recipientKey), 0, 16);
$documentId = $messageId !== null
? $messageId . '_' . \substr(\md5($recipientKey), 0, 8)
? ($recipient['alertId'] ?? \substr($messageId, 0, 19) . '_' . $recipientHash)
: ID::unique();
try {
@@ -73,10 +74,11 @@ class Console extends Adapter
'$id' => $documentId,
'$permissions' => $this->buildPermissions($userId, $teamId),
'messageId' => $messageId,
'recipientHash' => $recipientHash,
'type' => $message->getType(),
'channel' => self::TYPE,
'userId' => $userId !== '' ? $userId : null,
'teamId' => $teamId !== '' ? $teamId : null,
'userId' => $userId,
'teamId' => $teamId,
'projectId' => $message->getProjectId(),
'title' => $message->getTitle(),
'body' => $message->getBody(),
src/Appwrite/Utopia/Messaging/Messages/Console.php
+3 -3
View File
@@ -7,7 +7,7 @@ use Utopia\Messaging\Message;
class Console implements Message
{
/**
* @param array<int, array{userId?: string, teamId?: string}> $recipients
* @param array<int, array{userId?: string, teamId?: string, alertId?: string, recipientHash?: string}> $recipients
*/
public function __construct(
protected array $recipients,
@@ -20,7 +20,7 @@ class Console implements Message
}
/**
* @return array<int, array{userId?: string, teamId?: string}>
* @return array<int, array{userId?: string, teamId?: string, alertId?: string, recipientHash?: string}>
*/
public function getRecipients(): array
{
@@ -28,7 +28,7 @@ class Console implements Message
}
/**
* @return array<int, array{userId?: string, teamId?: string}>
* @return array<int, array{userId?: string, teamId?: string, alertId?: string, recipientHash?: string}>
*/
public function getTo(): array
{
tests/e2e/Services/Notifications/NotificationsBase.php
+6 -6
View File
@@ -12,7 +12,7 @@ use Utopia\System\System;
* account-alerts user-facing API.
*
* The notification worker itself is exercised in unit tests with a pinned
* queue payload — the server side cannot deterministically inject a
* queue payload; the server side cannot deterministically inject a
* Notification onto the live queue without an admin endpoint, so the health
* portion validates the public contract that ops and KEDA scale on:
*
@@ -70,7 +70,7 @@ trait NotificationsBase
{
// Always read alerts as the console-authenticated owner of the team.
// The /v1/account/alerts endpoint is platform-scoped (dbForPlatform) and
// requires a session — server-mode API keys do not satisfy it.
// requires a session; server-mode API keys do not satisfy it.
$response = $this->client->call(Client::METHOD_GET, '/account/alerts', $this->getConsoleAlertHeaders());
$this->assertSame(200, $response['headers']['status-code']);
@@ -197,7 +197,7 @@ trait NotificationsBase
// Track endpoint requires `purpose: 'alert_track'` — see C/M7 in
// PR #12195 review. Other claim purposes are silently ignored (which
// testTrackingPixelRejectsJwtWithoutPurposeClaim covers).
$jwt = (new JWT($secret, 'HS256', 2592000, 0))->encode([
$jwt = (new JWT($secret, 'HS256', ALERT_TRACKING_JWT_TTL, 0))->encode([
'alertId' => $alertId,
'userId' => $userId,
'purpose' => 'alert_track',
@@ -252,7 +252,7 @@ trait NotificationsBase
$userId = $this->getRoot()['$id'];
// Mint a JWT with valid alertId/userId but NO purpose claim.
$jwtNoPurpose = (new JWT($secret, 'HS256', 2592000, 0))->encode([
$jwtNoPurpose = (new JWT($secret, 'HS256', ALERT_TRACKING_JWT_TTL, 0))->encode([
'alertId' => $alertId,
'userId' => $userId,
]);
@@ -278,8 +278,8 @@ trait NotificationsBase
$this->assertNotNull($found);
$this->assertFalse($found['read'], 'JWT without purpose claim must not flip the read flag');
// Mint a JWT with a wrong purpose value — same expectation: silently rejected.
$jwtWrongPurpose = (new JWT($secret, 'HS256', 2592000, 0))->encode([
// Mint a JWT with a wrong purpose value: same expectation, silently rejected.
$jwtWrongPurpose = (new JWT($secret, 'HS256', ALERT_TRACKING_JWT_TTL, 0))->encode([
'alertId' => $alertId,
'userId' => $userId,
'purpose' => 'something_else',
tests/unit/Platform/Workers/NotificationsTest.php
+155 -44
View File
@@ -177,6 +177,7 @@ class NotificationsTest extends TestCase
false,
);
$this->database->createAttribute('alerts', 'messageId', Database::VAR_STRING, 255, false);
$this->database->createAttribute('alerts', 'recipientHash', Database::VAR_STRING, 64, true);
$this->database->createAttribute('alerts', 'type', Database::VAR_STRING, 64, false, 'info');
$this->database->createAttribute('alerts', 'channel', Database::VAR_STRING, 64, true);
$this->database->createAttribute('alerts', 'userId', Database::VAR_STRING, 255, false);
@@ -187,15 +188,15 @@ class NotificationsTest extends TestCase
$this->database->createAttribute('alerts', 'read', Database::VAR_BOOLEAN, 0, false, false);
// Mirror the production `_key_recipient` UNIQUE composite index so the
// duplicate-handling branch in persistAlert (catch DuplicateException →
// duplicate-handling branch in persistAlert (catch DuplicateException ->
// return existing alertId) is actually exercised by tests.
$this->database->createIndex(
'alerts',
'_key_recipient',
Database::INDEX_UNIQUE,
['messageId', 'channel', 'userId', 'teamId'],
[Database::LENGTH_KEY, 64, Database::LENGTH_KEY, Database::LENGTH_KEY],
[Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC],
['messageId', 'channel', 'recipientHash'],
[Database::LENGTH_KEY, 64, 64],
[Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC],
);
$this->registry = new Registry();
@@ -219,6 +220,16 @@ class NotificationsTest extends TestCase
]);
}
private function recipientHash(string $channel, string $address, string $userId = '', string $teamId = ''): string
{
return \substr(\md5($channel . ':' . $address . ':' . $userId . ':' . $teamId), 0, 16);
}
private function alertId(string $messageId, string $channel, string $address, string $userId = '', string $teamId = ''): string
{
return \substr($messageId, 0, 19) . '_' . $this->recipientHash($channel, $address, $userId, $teamId);
}
public function testDispatchesPerChannelToCorrectAdapter(): void
{
$worker = new SpyNotifications();
@@ -284,21 +295,9 @@ class NotificationsTest extends TestCase
'deduplicationKey' => 'dup-key',
];
// First run delivers and persists.
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
$this->assertCount(1, $worker->dispatched);
// Manually insert a row with the dedup messageId so alreadyDelivered() returns true.
$messageId = \md5('dup-key');
$this->database->createDocument('alerts', new Document([
'$id' => $messageId,
'$permissions' => [Permission::read(Role::any())],
'messageId' => $messageId,
'channel' => 'console',
'title' => 'x',
'body' => 'y',
]));
$worker->dispatched = [];
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
$this->assertCount(0, $worker->dispatched, 'second invocation must short-circuit on dedup hit');
@@ -391,37 +390,46 @@ class NotificationsTest extends TestCase
$this->assertCount(0, $rows, 'failed dispatch must not persist alert');
}
public function testDedupQueriesByAttributeNotById(): void
public function testDedupSkipsOnlyDeliveredRecipientAfterPartialFanoutFailure(): void
{
$worker = new SpyNotifications();
$worker->throwOn[NOTIFICATION_TYPE_WEBHOOK] = new \RuntimeException('webhook down');
// 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',
]));
$messageId = \md5('partial-key');
$payload = [
'project' => ['$id' => 'project-x'],
'recipients' => [['address' => 'user-1', 'channel' => NOTIFICATION_TYPE_CONSOLE]],
'recipients' => [
['address' => 'user-1', 'channel' => NOTIFICATION_TYPE_CONSOLE, 'userId' => 'user-1'],
['address' => 'https://hooks.example.test/in', 'channel' => NOTIFICATION_TYPE_WEBHOOK],
],
'subject' => 'Sub',
'body' => 'B',
'deduplicationKey' => 'dup-key',
'deduplicationKey' => 'partial-key',
];
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
try {
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
$this->fail('expected webhook failure to propagate');
} catch (\RuntimeException $error) {
$this->assertSame('webhook down', $error->getMessage());
}
$this->assertCount(0, $worker->dispatched, 'attribute-keyed seed must trigger dedup short-circuit');
$this->assertSame('hit', $this->log->getTags()['dedup'] ?? null);
$rows = $this->database->find('alerts', [
\Utopia\Database\Query::equal('messageId', [$messageId]),
]);
$this->assertCount(1, $rows, 'first attempt should persist only the successful console recipient');
$this->assertSame(NOTIFICATION_TYPE_CONSOLE, $rows[0]->getAttribute('channel'));
$retry = new SpyNotifications();
$retry->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
$this->assertCount(1, $retry->dispatched, 'retry should dispatch only the previously undelivered webhook');
$this->assertSame(NOTIFICATION_TYPE_WEBHOOK, $retry->dispatched[0]['channel']);
$rows = $this->database->find('alerts', [
\Utopia\Database\Query::equal('messageId', [$messageId]),
]);
$this->assertCount(2, $rows, 'retry must complete the missing recipient without duplicating console');
}
public function testConsoleChannelSkipsPersistAlert(): void
@@ -539,8 +547,12 @@ class NotificationsTest extends TestCase
// provide an OpenSSL key so injectTrackingPixel actually runs.
$previousSmtpHost = \getenv('_APP_SMTP_HOST');
$previousOpensslKey = \getenv('_APP_OPENSSL_KEY_V1');
$previousDomain = \getenv('_APP_DOMAIN');
$previousConsoleDomain = \getenv('_APP_CONSOLE_DOMAIN');
\putenv('_APP_SMTP_HOST=spy.smtp.test');
\putenv('_APP_OPENSSL_KEY_V1=test-key-32bytes-min-aaaaaaaaaaaaaa');
\putenv('_APP_DOMAIN=api.example.test');
\putenv('_APP_CONSOLE_DOMAIN=console.example.test');
try {
$worker = new Notifications();
@@ -563,6 +575,8 @@ class NotificationsTest extends TestCase
} finally {
\putenv($previousSmtpHost === false ? '_APP_SMTP_HOST' : '_APP_SMTP_HOST=' . $previousSmtpHost);
\putenv($previousOpensslKey === false ? '_APP_OPENSSL_KEY_V1' : '_APP_OPENSSL_KEY_V1=' . $previousOpensslKey);
\putenv($previousDomain === false ? '_APP_DOMAIN' : '_APP_DOMAIN=' . $previousDomain);
\putenv($previousConsoleDomain === false ? '_APP_CONSOLE_DOMAIN' : '_APP_CONSOLE_DOMAIN=' . $previousConsoleDomain);
}
$this->assertNotNull($spy->captured, 'SpyEmailAdapter must capture exactly one EmailMessage');
@@ -571,6 +585,8 @@ class NotificationsTest extends TestCase
$this->assertStringContainsString('<img src=', $body, 'tracking pixel <img> must be present');
$this->assertStringContainsString('/v1/account/alerts/', $body);
$this->assertStringContainsString('/track?jwt=', $body);
$this->assertStringContainsString('http://api.example.test/v1/account/alerts/', \html_entity_decode($body));
$this->assertStringNotContainsString('console.example.test/v1/account/alerts/', \html_entity_decode($body));
// The pixel must sit BEFORE the last </body>.
$lastBodyClose = \strripos($body, '</body>');
@@ -593,7 +609,7 @@ class NotificationsTest extends TestCase
// All four unique-index fields populated so the
// `_key_recipient` UNIQUE composite (messageId, channel,
// userId, teamId) actually fires — SQL UNIQUE semantics treat
// userId, teamId) actually fires. SQL UNIQUE semantics treat
// NULL as not-equal, so any null in the tuple disables it.
$payload = [
'project' => ['$id' => 'project-x'],
@@ -629,7 +645,7 @@ class NotificationsTest extends TestCase
// action loop's alreadyDelivered() check short-circuits before
// persistAlert, so call persistAlert directly to actually hit
// the duplicate branch. The deterministic $id collides on the
// primary key → DuplicateException → branch returns the
// primary key -> DuplicateException -> branch returns the
// existing alertId without throwing.
$reflection = new \ReflectionMethod($worker, 'persistAlert');
$secondAlertId = $reflection->invoke($worker, $this->database, $messageId, $recipient, $payload);
@@ -638,13 +654,14 @@ class NotificationsTest extends TestCase
// Third write: bypass the deterministic $id path and use a
// distinct $id with the same recipient tuple. The
// `_key_recipient` UNIQUE composite must reject it — proving
// `_key_recipient` UNIQUE composite must reject it, proving
// the unique-index (not just primary-key) is what backstops the
// duplicate-handling branch.
$sameTupleDoc = new Document([
'$id' => 'sibling-id-' . \uniqid(),
'$permissions' => [Permission::read(Role::any())],
'messageId' => $messageId,
'recipientHash' => $this->recipientHash(NOTIFICATION_TYPE_EMAIL, 'user@example.test', 'user-7', 'team-7'),
'channel' => NOTIFICATION_TYPE_EMAIL,
'userId' => 'user-7',
'teamId' => 'team-7',
@@ -814,7 +831,14 @@ class NotificationsTest extends TestCase
->setTemplate('template-id')
->setTemplateParams(['x' => 1])
->setDeduplicationKey('dedup')
->setPermissions([Permission::read(Role::any())]);
->setPermissions([Permission::read(Role::any())])
->setEvent('databases.*.documents.*.create')
->setParam('databaseId', 'database-1')
->setPayload(['before' => 'reset'], ['before'])
->setUser(new Document(['$id' => 'user-before']))
->setUserId('user-before')
->setPlatform(['name' => 'platform-before'])
->setContext('actor', new Document(['$id' => 'actor-before']));
$event->reset();
@@ -833,6 +857,13 @@ class NotificationsTest extends TestCase
$this->assertSame('', $event->getDeduplicationKey(), 'deduplicationKey must reset to empty string');
$this->assertSame([], $event->getPermissions(), 'permissions must reset to empty array');
$this->assertNull($event->getProject(), 'project must reset to null');
$this->assertSame('', $event->getEvent(), 'inherited event must reset to empty string');
$this->assertSame([], $event->getParams(), 'inherited params must reset to empty array');
$this->assertSame([], $event->getPayload(), 'inherited payload must reset to empty array');
$this->assertNull($event->getUser(), 'inherited user must reset to null');
$this->assertNull($event->getUserId(), 'inherited userId must reset to null');
$this->assertSame([], $event->getPlatform(), 'inherited platform must reset to empty array');
$this->assertNull($event->getContext('actor'), 'inherited context must reset');
}
/**
@@ -976,6 +1007,7 @@ class NotificationsTest extends TestCase
// dispatchEmail's returned alertId must match the row $id (used by the
// tracking pixel URL).
$this->assertSame($worker->persistedIds[0], $row->getId());
$this->assertLessThanOrEqual(36, \strlen($row->getId()), 'alert ids must pass the UID route validator');
}
/**
@@ -1017,11 +1049,10 @@ class NotificationsTest extends TestCase
$this->assertSame(NOTIFICATION_TYPE_CONSOLE, $row->getAttribute('channel'));
$this->assertFalse($row->getAttribute('read'));
// Per-recipient suffix scheme used by the Console adapter is
// `messageId . '_' . substr(md5('user:' . userId), 0, 8)`.
$messageId = \md5('happy-console');
$expectedId = $messageId . '_' . \substr(\md5('user:u1'), 0, 8);
$expectedId = $this->alertId($messageId, NOTIFICATION_TYPE_CONSOLE, 'console-recipient', 'u1', 't1');
$this->assertSame($expectedId, $row->getId(), 'row $id must match adapter suffix scheme');
$this->assertLessThanOrEqual(36, \strlen($row->getId()), 'alert ids must pass the UID route validator');
$permissions = $row->getPermissions();
$this->assertContains(Permission::read(Role::user('u1')), $permissions);
@@ -1034,6 +1065,86 @@ class NotificationsTest extends TestCase
$this->assertSame(0, $worker->persistAlertCalls, 'console channel must NOT trigger action-loop persistAlert');
}
public function testConsoleChannelUsesPreviewBodyInsteadOfRenderedEmailHtml(): void
{
$worker = new CountingPersistAlertNotifications();
$payload = [
'project' => ['$id' => 'project-x'],
'recipients' => [
[
'address' => 'user-preview',
'channel' => NOTIFICATION_TYPE_CONSOLE,
'userId' => 'user-preview',
],
],
'subject' => 'Webhook {{name}} paused',
'preview' => 'Plain alert for {{name}}.',
'body' => '<!DOCTYPE html><html><head><style>body{}</style></head><body><strong>Email-only HTML</strong></body></html>',
'templateParams' => ['name' => 'orders'],
'deduplicationKey' => 'console-preview',
];
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
$rows = $this->database->find('alerts');
$this->assertCount(1, $rows);
$this->assertSame('Webhook orders paused', $rows[0]->getAttribute('title'));
$this->assertSame('Plain alert for orders.', $rows[0]->getAttribute('body'));
$this->assertStringNotContainsString('<!DOCTYPE html>', $rows[0]->getAttribute('body'));
}
public function testEmailChannelSkipsWhenSmtpIsNotConfigured(): void
{
$previousSmtpHost = \getenv('_APP_SMTP_HOST');
\putenv('_APP_SMTP_HOST=');
try {
$worker = new CountingPersistAlertNotifications();
$payload = [
'project' => ['$id' => 'project-x'],
'recipients' => [
[
'address' => 'missing-smtp@example.test',
'channel' => NOTIFICATION_TYPE_EMAIL,
'userId' => 'user-smtp',
],
],
'subject' => 'No SMTP',
'body' => 'Body',
'deduplicationKey' => 'missing-smtp',
];
$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(0, $worker->persistAlertCalls);
$this->assertCount(0, $this->database->find('alerts'));
$this->assertSame('no_smtp', $this->log->getTags()['email_skipped'] ?? null);
}
public function testConsoleChannelRejectsInvalidImplicitUserId(): void
{
$worker = new CountingPersistAlertNotifications();
$payload = [
'project' => ['$id' => 'project-x'],
'recipients' => [
['address' => 'not an appwrite user id', 'channel' => NOTIFICATION_TYPE_CONSOLE],
],
'subject' => 'Invalid',
'body' => 'Body',
'deduplicationKey' => 'invalid-console-user',
];
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Invalid console alert userId');
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
}
/**
* Worker happy-path: webhook channel.
*
tests/unit/Platform/Workers/WebhooksTest.php
+36
View File
@@ -0,0 +1,36 @@
<?php
namespace Tests\Unit\Platform\Workers;
use Appwrite\Platform\Workers\Webhooks;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Utopia\Database\Document;
require_once __DIR__ . '/../../../../app/init.php';
class WebhooksTest extends TestCase
{
#[DataProvider('ownerRoleProvider')]
public function testOwnerRoleDetectionAcceptsArrayAndLegacyStringRoles(mixed $roles, bool $expected): void
{
$method = new \ReflectionMethod(Webhooks::class, 'hasOwnerRole');
$membership = new Document([
'$id' => 'membership-1',
'roles' => $roles,
]);
$this->assertSame($expected, $method->invoke(null, $membership));
}
public static function ownerRoleProvider(): array
{
return [
'array owner' => [['owner'], true],
'array mixed case owner' => [['Owner'], true],
'comma string owner' => ['developer, owner', true],
'non owner' => [['developer'], false],
'invalid roles' => [null, false],
];
}
}
tests/unit/Utopia/Messaging/Adapter/ConsoleTest.php
+8 -7
View File
@@ -33,6 +33,7 @@ class ConsoleTest extends TestCase
$this->database->create();
$this->database->createCollection('alerts', [], [], [Permission::create(Role::any()), Permission::read(Role::any())], false);
$this->database->createAttribute('alerts', 'messageId', Database::VAR_STRING, 255, false);
$this->database->createAttribute('alerts', 'recipientHash', Database::VAR_STRING, 64, true);
$this->database->createAttribute('alerts', 'type', Database::VAR_STRING, 64, false, 'info');
$this->database->createAttribute('alerts', 'channel', Database::VAR_STRING, 64, true);
$this->database->createAttribute('alerts', 'userId', Database::VAR_STRING, 255, false);
@@ -55,7 +56,7 @@ class ConsoleTest extends TestCase
private function alertId(string $messageId, string $userId = '', string $teamId = ''): string
{
$key = $userId !== '' ? 'user:' . $userId : 'team:' . $teamId;
return $messageId . '_' . \substr(\md5($key), 0, 8);
return \substr($messageId, 0, 19) . '_' . \substr(\md5($key), 0, 16);
}
public function testWritesAlertWithCorrectSchema(): void
@@ -155,9 +156,8 @@ class ConsoleTest extends TestCase
$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);
$this->assertMatchesRegularExpression('/^same-msg_[0-9a-f]{16}$/', $idA);
$this->assertMatchesRegularExpression('/^same-msg_[0-9a-f]{16}$/', $idB);
}
public function testRejectsForeignMessageType(): void
@@ -166,7 +166,7 @@ class ConsoleTest extends TestCase
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Invalid message type.');
// ConsoleMessage extends nothing — pass an unrelated Message implementation
// ConsoleMessage extends nothing; pass an unrelated Message implementation
$adapter->send(new \Appwrite\Utopia\Messaging\Messages\Webhook(urls: ['https://example.test'], payload: []));
}
@@ -174,7 +174,7 @@ class ConsoleTest extends TestCase
* Reviewer C4: a `DuplicateException` thrown by `createDocument` must be
* treated as a SUCCESSFUL delivery, not a failure. The adapter previously
* lumped Duplicate into the generic Throwable catch, which surfaced as a
* per-recipient `error` and caused the worker to throw — re-queueing the
* per-recipient `error` and caused the worker to throw, re-queueing the
* notification and never marking the duplicate as delivered.
*/
public function testConsoleAdapterTreatsDuplicateAsDelivered(): void
@@ -190,6 +190,7 @@ class ConsoleTest extends TestCase
'$id' => $documentId,
'$permissions' => [Permission::read(Role::any())],
'messageId' => $messageId,
'recipientHash' => \substr(\md5('user:' . $userId), 0, 16),
'channel' => 'console',
'userId' => $userId,
'title' => 'pre-existing',
@@ -212,7 +213,7 @@ class ConsoleTest extends TestCase
$this->assertSame('success', $result['results'][0]['status'] ?? '', 'duplicate must report success status');
$this->assertSame('', $result['results'][0]['error'] ?? 'unset', 'duplicate must not surface a per-recipient error');
// Still exactly ONE row — the pre-existing one. The adapter must not
// Still exactly ONE row: the pre-existing one. The adapter must not
// overwrite it nor create a sibling.
$rows = $this->database->find('alerts');
$this->assertCount(1, $rows);