From 5a9b1f0c241c7d7ebb85a94abee081c3da0180ef Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Thu, 26 Oct 2023 19:44:06 +0530 Subject: [PATCH 1/2] removes provider from topics --- app/config/collections.php | 72 ------------ app/controllers/api/account.php | 4 - app/controllers/api/messaging.php | 62 ++-------- composer.lock | 12 +- src/Appwrite/Platform/Workers/Messaging.php | 107 ++++++++++-------- .../Utopia/Response/Model/Message.php | 6 - src/Appwrite/Utopia/Response/Model/Topic.php | 6 - tests/e2e/Services/GraphQL/Base.php | 28 ++--- tests/e2e/Services/GraphQL/MessagingTest.php | 63 ++++------- .../e2e/Services/Messaging/MessagingBase.php | 40 +++---- 10 files changed, 121 insertions(+), 279 deletions(-) diff --git a/app/config/collections.php b/app/config/collections.php index 4f492471d6..54b6b3bec3 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -1523,28 +1523,6 @@ $commonCollections = [ '$id' => ID::custom('messages'), 'name' => 'Messages', 'attributes' => [ - [ - '$id' => ID::custom('providerId'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => Database::LENGTH_KEY, - 'signed' => true, - 'required' => true, - 'default' => null, - 'array' => false, - 'filters' => [], - ], - [ - '$id' => ID::custom('providerInternalId'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => Database::LENGTH_KEY, - 'signed' => true, - 'required' => true, - 'default' => null, - 'array' => false, - 'filters' => [], - ], [ '$id' => ID::custom('description'), 'type' => Database::VAR_STRING, @@ -1646,20 +1624,6 @@ $commonCollections = [ ], ], 'indexes' => [ - [ - '$id' => ID::custom('_key_providerId'), - 'type' => Database::INDEX_KEY, - 'attributes' => ['providerId'], - 'lengths' => [], - 'orders' => [Database::ORDER_ASC], - ], - [ - '$id' => ID::custom('_key_providerInternalId'), - 'type' => Database::INDEX_KEY, - 'attributes' => ['providerInternalId'], - 'lengths' => [], - 'orders' => [Database::ORDER_ASC], - ], [ '$id' => ID::custom('_key_search'), 'type' => Database::INDEX_FULLTEXT, @@ -1675,28 +1639,6 @@ $commonCollections = [ '$id' => ID::custom('topics'), 'name' => 'Topics', 'attributes' => [ - [ - '$id' => ID::custom('providerId'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => Database::LENGTH_KEY, - 'signed' => true, - 'required' => true, - 'default' => null, - 'array' => false, - 'filters' => [], - ], - [ - '$id' => ID::custom('providerInternalId'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => Database::LENGTH_KEY, - 'signed' => true, - 'required' => true, - 'default' => null, - 'array' => false, - 'filters' => [], - ], [ '$id' => ID::custom('name'), 'type' => Database::VAR_STRING, @@ -1754,20 +1696,6 @@ $commonCollections = [ ], ], 'indexes' => [ - [ - '$id' => ID::custom('_key_providerId'), - 'type' => Database::INDEX_KEY, - 'attributes' => ['providerId'], - 'lengths' => [], - 'orders' => [Database::ORDER_ASC], - ], - [ - '$id' => ID::custom('_key_providerInternalId'), - 'type' => Database::INDEX_KEY, - 'attributes' => ['providerInternalId'], - 'lengths' => [], - 'orders' => [Database::ORDER_ASC], - ], [ '$id' => ID::custom('_key_name'), 'type' => Database::INDEX_FULLTEXT, diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index f119d33414..4c7411d978 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1351,8 +1351,6 @@ App::post('/v1/account/sessions/phone') 'data' => [ 'content' => $message, ], - 'providerId' => $provider->getId(), - 'providerInternalId' => $provider->getInternalId(), ])); $queueForMessaging @@ -2986,8 +2984,6 @@ App::post('/v1/account/verification/phone') 'data' => [ 'content' => $message, ], - 'providerId' => $provider->getId(), - 'providerInternalId' => $provider->getInternalId(), ])); $queueForMessaging diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index e1057a0885..b9bfbefdb7 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -1260,23 +1260,15 @@ App::post('/v1/messaging/topics') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_TOPIC) ->param('topicId', '', new CustomId(), 'Topic ID. Choose a custom Topic ID or a new Topic ID.') - ->param('providerId', '', new UID(), 'Provider ID.') ->param('name', '', new Text(128), 'Topic Name.') ->param('description', '', new Text(2048), 'Topic Description.', true) ->inject('dbForProject') ->inject('response') - ->action(function (string $topicId, string $providerId, string $name, string $description, Database $dbForProject, Response $response) { + ->action(function (string $topicId, string $name, string $description, Database $dbForProject, Response $response) { $topicId = $topicId == 'unique()' ? ID::unique() : $topicId; - $provider = $dbForProject->getDocument('providers', $providerId); - - if ($provider->isEmpty()) { - throw new Exception(Exception::PROVIDER_NOT_FOUND); - } $topic = new Document([ '$id' => $topicId, - 'providerId' => $providerId, - 'providerInternalId' => $provider->getInternalId(), 'name' => $name, ]); @@ -1628,7 +1620,6 @@ App::post('/v1/messaging/messages/email') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new CustomId(), 'Message ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') - ->param('providerId', '', new UID(), 'Email Provider ID.') ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.') ->param('subject', '', new Text(998), 'Email Subject.') ->param('content', '', new Text(64230), 'Email Content.') @@ -1640,23 +1631,11 @@ App::post('/v1/messaging/messages/email') ->inject('project') ->inject('queueForMessaging') ->inject('response') - ->action(function (string $messageId, string $providerId, array $to, string $subject, string $content, string $description, string $status, bool $html, ?string $deliveryTime, Database $dbForProject, Document $project, Messaging $queueForMessaging, Response $response) { + ->action(function (string $messageId, array $to, string $subject, string $content, string $description, string $status, bool $html, ?string $deliveryTime, Database $dbForProject, Document $project, Messaging $queueForMessaging, Response $response) { $messageId = $messageId == 'unique()' ? ID::unique() : $messageId; - $provider = $dbForProject->getDocument('providers', $providerId); - - if ($provider->isEmpty()) { - throw new Exception(Exception::PROVIDER_NOT_FOUND); - } - - if ($provider->getAttribute('type') !== 'email') { - throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); - } - $message = $dbForProject->createDocument('messages', new Document([ '$id' => $messageId, - 'providerId' => $provider->getId(), - 'providerInternalId' => $provider->getInternalId(), 'to' => $to, 'description' => $description, 'data' => [ @@ -1665,7 +1644,7 @@ App::post('/v1/messaging/messages/email') 'html' => $html, ], 'status' => $status, - 'search' => $messageId . ' ' . $description . ' ' . $subject . ' ' . $providerId, + 'search' => $messageId . ' ' . $description . ' ' . $subject, ])); if ($status === 'processing') { @@ -1694,7 +1673,6 @@ App::post('/v1/messaging/messages/sms') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new CustomId(), 'Message ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') - ->param('providerId', '', new UID(), 'SMS Provider ID.') ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.') ->param('content', '', new Text(64230), 'SMS Content.') ->param('description', '', new Text(256), 'Description for Message.', true) @@ -1704,31 +1682,18 @@ App::post('/v1/messaging/messages/sms') ->inject('project') ->inject('queueForMessaging') ->inject('response') - ->action(function (string $messageId, string $providerId, array $to, string $content, string $description, string $status, ?string $deliveryTime, Database $dbForProject, Document $project, Messaging $queueForMessaging, Response $response) { + ->action(function (string $messageId, array $to, string $content, string $description, string $status, ?string $deliveryTime, Database $dbForProject, Document $project, Messaging $queueForMessaging, Response $response) { $messageId = $messageId == 'unique()' ? ID::unique() : $messageId; - $provider = $dbForProject->getDocument('providers', $providerId); - - if ($provider->isEmpty()) { - throw new Exception(Exception::PROVIDER_NOT_FOUND); - } - - if ($provider->getAttribute('type') !== 'sms') { - throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); - } - $message = $dbForProject->createDocument('messages', new Document([ '$id' => $messageId, - 'providerId' => $provider->getId(), - 'providerInternalId' => $provider->getInternalId(), 'to' => $to, 'description' => $description, - 'deliveryTime' => $deliveryTime, 'data' => [ 'content' => $content, ], 'status' => $status, - 'search' => $messageId . ' ' . $description . ' ' . $providerId, + 'search' => $messageId . ' ' . $description, ])); if ($status === 'processing') { @@ -1757,7 +1722,6 @@ App::post('/v1/messaging/messages/push') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new CustomId(), 'Message ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') - ->param('providerId', '', new UID(), 'Push Provider ID.') ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.') ->param('title', '', new Text(256), 'Title for push notification.') ->param('body', '', new Text(64230), 'Body for push notification.') @@ -1775,19 +1739,9 @@ App::post('/v1/messaging/messages/push') ->inject('project') ->inject('queueForMessaging') ->inject('response') - ->action(function (string $messageId, string $providerId, array $to, string $title, string $body, string $description, ?array $data, string $action, string $icon, string $sound, string $color, string $tag, string $badge, string $status, ?string $deliveryTime, Database $dbForProject, Document $project, Messaging $queueForMessaging, Response $response) { + ->action(function (string $messageId, array $to, string $title, string $body, string $description, ?array $data, string $action, string $icon, string $sound, string $color, string $tag, string $badge, string $status, ?string $deliveryTime, Database $dbForProject, Document $project, Messaging $queueForMessaging, Response $response) { $messageId = $messageId == 'unique()' ? ID::unique() : $messageId; - $provider = $dbForProject->getDocument('providers', $providerId); - - if ($provider->isEmpty()) { - throw new Exception(Exception::PROVIDER_NOT_FOUND); - } - - if ($provider->getAttribute('type') !== 'push') { - throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); - } - $pushData = [ 'title' => $title, 'body' => $body, @@ -1823,14 +1777,12 @@ App::post('/v1/messaging/messages/push') $message = $dbForProject->createDocument('messages', new Document([ '$id' => $messageId, - 'providerId' => $provider->getId(), - 'providerInternalId' => $provider->getInternalId(), 'to' => $to, 'description' => $description, 'deliveryTime' => $deliveryTime, 'data' => $pushData, 'status' => $status, - 'search' => $messageId . ' ' . $description . ' ' . $title . ' ' . $providerId, + 'search' => $messageId . ' ' . $description . ' ' . $title, ])); if ($status === 'processing') { diff --git a/composer.lock b/composer.lock index 9ef16ead55..66e5e5a11b 100644 --- a/composer.lock +++ b/composer.lock @@ -1906,16 +1906,16 @@ }, { "name": "utopia-php/database", - "version": "0.44.3", + "version": "0.44.4", "source": { "type": "git", "url": "https://github.com/utopia-php/database.git", - "reference": "b2d403c25a77506e03db5736335b0cae52bcc18a" + "reference": "b0c3fd8ecfedc3646d7780f2d6b38955a66baf48" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/database/zipball/b2d403c25a77506e03db5736335b0cae52bcc18a", - "reference": "b2d403c25a77506e03db5736335b0cae52bcc18a", + "url": "https://api.github.com/repos/utopia-php/database/zipball/b0c3fd8ecfedc3646d7780f2d6b38955a66baf48", + "reference": "b0c3fd8ecfedc3646d7780f2d6b38955a66baf48", "shasum": "" }, "require": { @@ -1956,9 +1956,9 @@ ], "support": { "issues": "https://github.com/utopia-php/database/issues", - "source": "https://github.com/utopia-php/database/tree/0.44.3" + "source": "https://github.com/utopia-php/database/tree/0.44.4" }, - "time": "2023-10-24T10:13:48+00:00" + "time": "2023-10-26T07:08:12+00:00" }, { "name": "utopia-php/domains", diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index b542dccaf2..0c101b26e0 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -36,7 +36,6 @@ class Messaging extends Action return "messaging"; } - /** * @throws Exception */ @@ -64,24 +63,19 @@ class Messaging extends Action return; } - $message = $dbForProject->getDocument('messages', $payload['messageId']); - $provider = $dbForProject->getDocument('providers', $message->getAttribute('providerId')); - - $this->processMessage($dbForProject, $message, $provider); + $this->processMessage($dbForProject, $message); } - private function processMessage(Database $dbForProject, Document $message, Document $provider): void + private function processMessage(Database $dbForProject, Document $message): void { - $adapter = match ($provider->getAttribute('type')) { - 'sms' => $this->sms($provider), - 'push' => $this->push($provider), - 'email' => $this->email($provider), - default => throw new Exception(Exception::PROVIDER_INCORRECT_TYPE) - }; + $recipientsId = $message->getAttribute('to', []); - $recipientsId = $message->getAttribute('to'); + if (\count($recipientsId) < 1) { + Console::error('Recipients not found'); + return; + } /** * @var Document[] $recipients @@ -100,44 +94,65 @@ class Messaging extends Action $targets = $dbForProject->find('targets', [Query::equal('$id', $recipientsId)]); $recipients = \array_merge($recipients, $targets); - $recipients = \array_filter($recipients, function (Document $recipient) use ($provider) { - return $recipient->getAttribute('providerId') === $provider->getId(); - }); - $identifiers = \array_map(function (Document $recipient) { - return $recipient->getAttribute('identifier'); - }, $recipients); + $providers = []; + foreach ($recipients as $recipient) { + $providerId = $recipient->getAttribute('providerId'); + if (!isset($providers[$providerId])) { + $providers[$providerId] = []; + } + $providers[$providerId][] = $recipient->getAttribute('identifier'); + } - $maxBatchSize = $adapter->getMaxMessagesPerRequest(); - $batches = \array_chunk($identifiers, $maxBatchSize); - $batchIndex = 0; - - $results = batch(\array_map(function ($batch) use ($message, $provider, $adapter, $batchIndex) { - return function () use ($batch, $message, $provider, $adapter, $batchIndex) { - $deliveredTo = 0; - $deliveryErrors = []; - $messageData = clone $message; - $messageData->setAttribute('to', $batch); - $data = match ($provider->getAttribute('type')) { - 'sms' => $this->buildSMSMessage($messageData, $provider), - 'push' => $this->buildPushMessage($messageData), - 'email' => $this->buildEmailMessage($messageData, $provider), + /** + * @var array[] $results + */ + $results = batch(\array_map(function ($providerId) use ($providers, $message, $dbForProject) { + return function () use ($providerId, $providers, $message, $dbForProject) { + $provider = $dbForProject->getDocument('providers', $providerId); + $identifiers = $providers[$providerId]; + $adapter = match ($provider->getAttribute('type')) { + 'sms' => $this->sms($provider), + 'push' => $this->push($provider), + 'email' => $this->email($provider), default => throw new Exception(Exception::PROVIDER_INCORRECT_TYPE) }; - try { - $adapter->send($data); - $deliveredTo += \count($batch); - } catch (\Exception $e) { - $deliveryErrors[] = 'Failed sending to targets ' . $batchIndex + 1 . '-' . \count($batch) . ' with error: ' . $e->getMessage(); - } finally { - $batchIndex++; - return [ - 'deliveredTo' => $deliveredTo, - 'deliveryErrors' => $deliveryErrors, - ]; - } + $maxBatchSize = $adapter->getMaxMessagesPerRequest(); + $batches = \array_chunk($identifiers, $maxBatchSize); + $batchIndex = 0; + + $results = batch(\array_map(function ($batch) use ($message, $provider, $adapter, $batchIndex) { + return function () use ($batch, $message, $provider, $adapter, $batchIndex) { + $deliveredTo = 0; + $deliveryErrors = []; + $messageData = clone $message; + $messageData->setAttribute('to', $batch); + $data = match ($provider->getAttribute('type')) { + 'sms' => $this->buildSMSMessage($messageData, $provider), + 'push' => $this->buildPushMessage($messageData), + 'email' => $this->buildEmailMessage($messageData, $provider), + default => throw new Exception(Exception::PROVIDER_INCORRECT_TYPE) + }; + try { + $adapter->send($data); + $deliveredTo += \count($batch); + } catch (\Exception $e) { + $deliveryErrors[] = 'Failed sending to targets ' . $batchIndex + 1 . '-' . \count($batch) . ' with error: ' . $e->getMessage(); + } finally { + $batchIndex++; + return [ + 'deliveredTo' => $deliveredTo, + 'deliveryErrors' => $deliveryErrors, + ]; + } + }; + }, $batches)); + + return $results; }; - }, $batches)); + }, \array_keys($providers))); + + $results = array_merge(...$results); $deliveredTo = 0; $deliveryErrors = []; diff --git a/src/Appwrite/Utopia/Response/Model/Message.php b/src/Appwrite/Utopia/Response/Model/Message.php index 0cadc378d9..609aa4022e 100644 --- a/src/Appwrite/Utopia/Response/Model/Message.php +++ b/src/Appwrite/Utopia/Response/Model/Message.php @@ -17,12 +17,6 @@ class Message extends Any 'default' => '', 'example' => '5e5ea5c16897e', ]) - ->addRule('providerId', [ - 'type' => self::TYPE_STRING, - 'description' => 'Provider ID for the message.', - 'default' => '', - 'example' => '5e5ea5c16897e', - ]) ->addRule('to', [ 'type' => self::TYPE_STRING, 'description' => 'Message recipients.', diff --git a/src/Appwrite/Utopia/Response/Model/Topic.php b/src/Appwrite/Utopia/Response/Model/Topic.php index 6d76faf6f4..d46d26597d 100644 --- a/src/Appwrite/Utopia/Response/Model/Topic.php +++ b/src/Appwrite/Utopia/Response/Model/Topic.php @@ -16,12 +16,6 @@ class Topic extends Model 'default' => '', 'example' => '259125845563242502', ]) - ->addRule('providerId', [ - 'type' => self::TYPE_STRING, - 'description' => 'Provider ID.', - 'default' => '', - 'example' => '259125845563242502', - ]) ->addRule('name', [ 'type' => self::TYPE_STRING, 'description' => 'The name of the topic.', diff --git a/tests/e2e/Services/GraphQL/Base.php b/tests/e2e/Services/GraphQL/Base.php index 745a64c0e7..bf6ae84f7f 100644 --- a/tests/e2e/Services/GraphQL/Base.php +++ b/tests/e2e/Services/GraphQL/Base.php @@ -2013,11 +2013,10 @@ trait Base } }'; case self::$CREATE_TOPIC: - return 'mutation createTopic($providerId: String!, $topicId: String!, $name: String!, $description: String!) { - messagingCreateTopic(providerId: $providerId, topicId: $topicId, name: $name, description: $description) { + return 'mutation createTopic($topicId: String!, $name: String!, $description: String!) { + messagingCreateTopic(topicId: $topicId, name: $name, description: $description) { _id name - providerId description } }'; @@ -2028,7 +2027,6 @@ trait Base topics { _id name - providerId description } } @@ -2038,7 +2036,6 @@ trait Base messagingGetTopic(topicId: $topicId) { _id name - providerId description } }'; @@ -2047,7 +2044,6 @@ trait Base messagingUpdateTopic(topicId: $topicId, name: $name, description: $description) { _id name - providerId description } }'; @@ -2091,10 +2087,9 @@ trait Base } }'; case self::$CREATE_EMAIL: - return 'mutation createEmail($messageId: String!, $providerId: String!, $to: [String!]!, $subject: String!, $content: String!, $status: String, $description: String, $html: Boolean, $deliveryTime: String) { - messagingCreateEmail(messageId: $messageId, providerId: $providerId, to: $to, subject: $subject, content: $content, status: $status, description: $description, html: $html, deliveryTime: $deliveryTime) { + return 'mutation createEmail($messageId: String!, $to: [String!]!, $subject: String!, $content: String!, $status: String, $description: String, $html: Boolean, $deliveryTime: String) { + messagingCreateEmail(messageId: $messageId, to: $to, subject: $subject, content: $content, status: $status, description: $description, html: $html, deliveryTime: $deliveryTime) { _id - providerId to deliveryTime deliveredAt @@ -2105,10 +2100,9 @@ trait Base } }'; case self::$CREATE_SMS: - return 'mutation createSMS($messageId: String!, $providerId: String!, $to: [String!]!, $content: String!, $status: String, $description: String, $deliveryTime: String) { - messagingCreateSMS(messageId: $messageId, providerId: $providerId, to: $to, content: $content, status: $status, description: $description, deliveryTime: $deliveryTime) { + return 'mutation createSMS($messageId: String!, $to: [String!]!, $content: String!, $status: String, $description: String, $deliveryTime: String) { + messagingCreateSMS(messageId: $messageId, to: $to, content: $content, status: $status, description: $description, deliveryTime: $deliveryTime) { _id - providerId to deliveryTime deliveredAt @@ -2119,10 +2113,9 @@ trait Base } }'; case self::$CREATE_PUSH_NOTIFICATION: - return 'mutation createPushNotification($messageId: String!, $providerId: String!, $to: [String!]!, $title: String!, $body: String!, $data: Json, $action: String, $icon: String, $sound: String, $color: String, $tag: String, $badge: String, $status: String, $description: String, $deliveryTime: String) { - messagingCreatePushNotification(messageId: $messageId, providerId: $providerId, to: $to, title: $title, body: $body, data: $data, action: $action, icon: $icon, sound: $sound, color: $color, tag: $tag, badge: $badge, status: $status, description: $description, deliveryTime: $deliveryTime) { + return 'mutation createPushNotification($messageId: String!, $to: [String!]!, $title: String!, $body: String!, $data: Json, $action: String, $icon: String, $sound: String, $color: String, $tag: String, $badge: String, $status: String, $description: String, $deliveryTime: String) { + messagingCreatePushNotification(messageId: $messageId, to: $to, title: $title, body: $body, data: $data, action: $action, icon: $icon, sound: $sound, color: $color, tag: $tag, badge: $badge, status: $status, description: $description, deliveryTime: $deliveryTime) { _id - providerId to deliveryTime deliveredAt @@ -2138,7 +2131,6 @@ trait Base total messages { _id - providerId to deliveryTime deliveredAt @@ -2153,7 +2145,6 @@ trait Base return 'query getMessage($messageId: String!) { messagingGetMessage(messageId: $messageId) { _id - providerId to deliveryTime deliveredAt @@ -2167,7 +2158,6 @@ trait Base return 'mutation updateEmail($messageId: String!, $to: [String!], $subject: String, $content: String, $status: String, $description: String, $html: Boolean, $deliveryTime: String) { messagingUpdateEmail(messageId: $messageId, to: $to, subject: $subject, content: $content, status: $status, description: $description, html: $html, deliveryTime: $deliveryTime) { _id - providerId to deliveryTime deliveredAt @@ -2181,7 +2171,6 @@ trait Base return 'mutation updateSMS($messageId: String!, $to: [String!], $content: String, $status: String, $description: String, $deliveryTime: String) { messagingUpdateSMS(messageId: $messageId, to: $to, content: $content, status: $status, description: $description, deliveryTime: $deliveryTime) { _id - providerId to deliveryTime deliveredAt @@ -2195,7 +2184,6 @@ trait Base return 'mutation updatePushNotification($messageId: String!, $to: [String!], $title: String, $body: String, $data: Json, $action: String, $icon: String, $sound: String, $color: String, $tag: String, $badge: String, $status: String, $description: String, $deliveryTime: String) { messagingUpdatePushNotification(messageId: $messageId, to: $to, title: $title, body: $body, data: $data, action: $action, icon: $icon, sound: $sound, color: $color, tag: $tag, badge: $badge, status: $status, description: $description, deliveryTime: $deliveryTime) { _id - providerId to deliveryTime deliveredAt diff --git a/tests/e2e/Services/GraphQL/MessagingTest.php b/tests/e2e/Services/GraphQL/MessagingTest.php index 4478a4d75b..82f697f7ec 100644 --- a/tests/e2e/Services/GraphQL/MessagingTest.php +++ b/tests/e2e/Services/GraphQL/MessagingTest.php @@ -96,7 +96,6 @@ class MessagingTest extends Scope 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ]), $graphQLPayload); - var_dump($response['body']); \array_push($providers, $response['body']['data']['messagingCreate' . $key . 'Provider']); $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals($providersParams[$key]['name'], $response['body']['data']['messagingCreate' . $key . 'Provider']['name']); @@ -268,32 +267,10 @@ class MessagingTest extends Scope public function testCreateTopic() { - $providerParam = [ - 'sendgrid' => [ - 'providerId' => ID::unique(), - 'name' => 'Sengrid1', - 'apiKey' => 'my-apikey', - 'from' => 'sender-email@my-domain.com', - ] - ]; - $query = $this->getQuery(self::$CREATE_SENDGRID_PROVIDER); - $graphQLPayload = [ - 'query' => $query, - 'variables' => $providerParam['sendgrid'], - ]; - $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'], - ]), $graphQLPayload); - - $providerId = $response['body']['data']['messagingCreateSendgridProvider']['_id']; - $query = $this->getQuery(self::$CREATE_TOPIC); $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -391,13 +368,34 @@ class MessagingTest extends Scope $userId = $this->getUser()['$id']; + $providerParam = [ + 'sendgrid' => [ + 'providerId' => ID::unique(), + 'name' => 'Sengrid1', + 'apiKey' => 'my-apikey', + 'from' => 'sender-email@my-domain.com', + ] + ]; + $query = $this->getQuery(self::$CREATE_SENDGRID_PROVIDER); + $graphQLPayload = [ + 'query' => $query, + 'variables' => $providerParam['sendgrid'], + ]; + $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ]), $graphQLPayload); + + $providerId = $response['body']['data']['messagingCreateSendgridProvider']['_id']; + $query = $this->getQuery(self::$CREATE_USER_TARGET); $graphQLPayload = [ 'query' => $query, 'variables' => [ 'targetId' => ID::unique(), 'userId' => $userId, - 'providerId' => $topic['providerId'], + 'providerId' => $providerId, 'identifier' => 'token', ], ]; @@ -546,7 +544,7 @@ class MessagingTest extends Scope 'apiKey' => $apiKey, 'domain' => $domain, 'from' => $from, - 'isEuRegion' => $isEuRegion, + 'isEuRegion' => filter_var($isEuRegion, FILTER_VALIDATE_BOOLEAN), ], ]; $provider = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ @@ -563,7 +561,6 @@ class MessagingTest extends Scope $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -634,7 +631,6 @@ class MessagingTest extends Scope 'query' => $query, 'variables' => [ 'messageId' => ID::unique(), - 'providerId' => $providerId, 'to' => [$topic['body']['data']['messagingCreateTopic']['_id']], 'subject' => 'Khali beats Undertaker', 'content' => 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', @@ -693,7 +689,7 @@ class MessagingTest extends Scope 'apiKey' => $apiKey, 'domain' => $domain, 'from' => $from, - 'isEuRegion' => $isEuRegion, + 'isEuRegion' => filter_var($isEuRegion, FILTER_VALIDATE_BOOLEAN), ], ]; $provider = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ @@ -710,7 +706,6 @@ class MessagingTest extends Scope $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -781,7 +776,6 @@ class MessagingTest extends Scope 'query' => $query, 'variables' => [ 'messageId' => ID::unique(), - 'providerId' => $providerId, 'status' => 'draft', 'to' => [$topic['body']['data']['messagingCreateTopic']['_id']], 'subject' => 'Khali beats Undertaker', @@ -867,7 +861,6 @@ class MessagingTest extends Scope $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -938,7 +931,6 @@ class MessagingTest extends Scope 'query' => $query, 'variables' => [ 'messageId' => ID::unique(), - 'providerId' => $providerId, 'to' => [$topic['body']['data']['messagingCreateTopic']['_id']], 'content' => '454665', ], @@ -1010,7 +1002,6 @@ class MessagingTest extends Scope $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -1081,7 +1072,6 @@ class MessagingTest extends Scope 'query' => $query, 'variables' => [ 'messageId' => ID::unique(), - 'providerId' => $providerId, 'status' => 'draft', 'to' => [$topic['body']['data']['messagingCreateTopic']['_id']], 'content' => '345463', @@ -1162,7 +1152,6 @@ class MessagingTest extends Scope $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -1233,7 +1222,6 @@ class MessagingTest extends Scope 'query' => $query, 'variables' => [ 'messageId' => ID::unique(), - 'providerId' => $providerId, 'to' => [$topic['body']['data']['messagingCreateTopic']['_id']], 'title' => 'Push Notification Title', 'body' => 'Push Notifiaction Body', @@ -1296,14 +1284,12 @@ class MessagingTest extends Scope ]), $graphQLPayload); $this->assertEquals(200, $provider['headers']['status-code']); - var_dump($provider['body']); $providerId = $provider['body']['data']['messagingCreateFcmProvider']['_id']; $query = $this->getQuery(self::$CREATE_TOPIC); $graphQLPayload = [ 'query' => $query, 'variables' => [ - 'providerId' => $providerId, 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Active users', @@ -1374,7 +1360,6 @@ class MessagingTest extends Scope 'query' => $query, 'variables' => [ 'messageId' => ID::unique(), - 'providerId' => $providerId, 'status' => 'draft', 'to' => [$topic['body']['data']['messagingCreateTopic']['_id']], 'title' => 'Push Notification Title', diff --git a/tests/e2e/Services/Messaging/MessagingBase.php b/tests/e2e/Services/Messaging/MessagingBase.php index 765bd91ede..01138c6412 100644 --- a/tests/e2e/Services/Messaging/MessagingBase.php +++ b/tests/e2e/Services/Messaging/MessagingBase.php @@ -218,23 +218,11 @@ trait MessagingBase public function testCreateTopic(): array { - $provider = $this->client->call(Client::METHOD_POST, '/messaging/providers/sendgrid', \array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'], - ]), [ - 'providerId' => 'unique()', - 'name' => 'Sendgrid1', - 'apiKey' => 'my-apikey', - 'from' => 'sender-email@my-domain.com', - ]); - $this->assertEquals(201, $provider['headers']['status-code']); $response = $this->client->call(Client::METHOD_POST, '/messaging/topics', [ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => 'unique()', 'name' => 'my-app', 'description' => 'web app' @@ -297,13 +285,27 @@ trait MessagingBase public function testCreateSubscriber(array $topic) { $userId = $this->getUser()['$id']; + + $provider = $this->client->call(Client::METHOD_POST, '/messaging/providers/sendgrid', \array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ]), [ + 'providerId' => ID::unique(), + 'name' => 'Sendgrid1', + 'apiKey' => 'my-apikey', + 'from' => 'sender-email@my-domain.com', + ]); + + $this->assertEquals(201, $provider['headers']['status-code']); + $target = $this->client->call(Client::METHOD_POST, '/users/' . $userId . '/targets', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ]), [ 'targetId' => ID::unique(), - 'providerId' => $topic['providerId'], + 'providerId' => $provider['body']['$id'], 'identifier' => 'my-token', ]); $this->assertEquals(201, $target['headers']['status-code']); @@ -433,7 +435,6 @@ trait MessagingBase 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Test Topic' @@ -485,7 +486,6 @@ trait MessagingBase 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ 'messageId' => ID::unique(), - 'providerId' => $provider['body']['$id'], 'to' => [$topic['body']['$id']], 'subject' => 'Khali beats Undertaker', 'content' => 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', @@ -553,7 +553,6 @@ trait MessagingBase 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Test Topic' @@ -605,7 +604,6 @@ trait MessagingBase 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ 'messageId' => ID::unique(), - 'providerId' => $provider['body']['$id'], 'status' => 'draft', 'to' => [$topic['body']['$id']], 'subject' => 'Khali beats Undertaker', @@ -668,7 +666,6 @@ trait MessagingBase 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Test Topic' @@ -720,7 +717,6 @@ trait MessagingBase 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ 'messageId' => ID::unique(), - 'providerId' => $provider['body']['$id'], 'to' => [$topic['body']['$id']], 'content' => '064763', ]); @@ -785,7 +781,6 @@ trait MessagingBase 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Test Topic' @@ -837,7 +832,6 @@ trait MessagingBase 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ 'messageId' => ID::unique(), - 'providerId' => $provider['body']['$id'], 'status' => 'draft', 'to' => [$topic['body']['$id']], 'content' => '047487', @@ -895,7 +889,6 @@ trait MessagingBase 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Test Topic' @@ -947,7 +940,6 @@ trait MessagingBase 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ 'messageId' => ID::unique(), - 'providerId' => $provider['body']['$id'], 'to' => [$topic['body']['$id']], 'title' => 'Test-Notification', 'body' => 'Test-Notification-Body', @@ -1009,7 +1001,6 @@ trait MessagingBase 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ - 'providerId' => $provider['body']['$id'], 'topicId' => ID::unique(), 'name' => 'topic1', 'description' => 'Test Topic' @@ -1061,7 +1052,6 @@ trait MessagingBase 'x-appwrite-key' => $this->getProject()['apiKey'], ], [ 'messageId' => ID::unique(), - 'providerId' => $provider['body']['$id'], 'status' => 'draft', 'to' => [$topic['body']['$id']], 'title' => 'Test-Notification', From bc6df4bab71bbe79e035ce3e81e9a9e6c9343646 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Fri, 27 Oct 2023 13:42:27 +0530 Subject: [PATCH 2/2] add recipients array check in API instead of worker --- app/controllers/api/messaging.php | 12 ++++++------ src/Appwrite/Platform/Workers/Messaging.php | 5 ----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index b9bfbefdb7..1dbb371a7f 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -1620,7 +1620,7 @@ App::post('/v1/messaging/messages/email') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new CustomId(), 'Message ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') - ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.') + ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY), 1), 'List of Topic IDs or List of User IDs or List of Target IDs.') ->param('subject', '', new Text(998), 'Email Subject.') ->param('content', '', new Text(64230), 'Email Content.') ->param('description', '', new Text(256), 'Description for message.', true) @@ -1673,7 +1673,7 @@ App::post('/v1/messaging/messages/sms') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new CustomId(), 'Message ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') - ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.') + ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY), 1), 'List of Topic IDs or List of User IDs or List of Target IDs.') ->param('content', '', new Text(64230), 'SMS Content.') ->param('description', '', new Text(256), 'Description for Message.', true) ->param('status', 'processing', new WhiteList(['draft', 'processing']), 'Message Status. Value must be either draft or processing.', true) @@ -1722,7 +1722,7 @@ App::post('/v1/messaging/messages/push') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new CustomId(), 'Message ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') - ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.') + ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY), 1), 'List of Topic IDs or List of User IDs or List of Target IDs.') ->param('title', '', new Text(256), 'Title for push notification.') ->param('body', '', new Text(64230), 'Body for push notification.') ->param('description', '', new Text(256), 'Description for Message.', true) @@ -1878,7 +1878,7 @@ App::patch('/v1/messaging/messages/email/:messageId') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new UID(), 'Message ID.') - ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.', true) + ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY), 1), 'List of Topic IDs or List of User IDs or List of Target IDs.', true) ->param('subject', '', new Text(998), 'Email Subject.', true) ->param('description', '', new Text(256), 'Description for Message.', true) ->param('content', '', new Text(64230), 'Email Content.', true) @@ -1965,7 +1965,7 @@ App::patch('/v1/messaging/messages/sms/:messageId') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new UID(), 'Message ID.') - ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.', true) + ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY), 1), 'List of Topic IDs or List of User IDs or List of Target IDs.', true) ->param('description', '', new Text(256), 'Description for Message.', true) ->param('content', '', new Text(64230), 'Email Content.', true) ->param('status', '', new WhiteList(['draft', 'processing']), 'Message Status. Value must be either draft or processing.', true) @@ -2042,7 +2042,7 @@ App::patch('/v1/messaging/messages/push/:messageId') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MESSAGE) ->param('messageId', '', new UID(), 'Message ID.') - ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY)), 'List of Topic IDs or List of User IDs or List of Target IDs.', true) + ->param('to', [], new ArrayList(new Text(Database::LENGTH_KEY), 1), 'List of Topic IDs or List of User IDs or List of Target IDs.', true) ->param('description', '', new Text(256), 'Description for Message.', true) ->param('title', '', new Text(256), 'Title for push notification.', true) ->param('body', '', new Text(64230), 'Body for push notification.', true) diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index 0c101b26e0..5ca5ebfc65 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -72,11 +72,6 @@ class Messaging extends Action { $recipientsId = $message->getAttribute('to', []); - if (\count($recipientsId) < 1) { - Console::error('Recipients not found'); - return; - } - /** * @var Document[] $recipients */