From ffeb3f8fcf1a8f954ad4ced75e057ffa1489ab72 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Tue, 17 Oct 2023 22:53:26 +0530 Subject: [PATCH] review work --- app/controllers/api/messaging.php | 45 ++-- app/init.php | 1 + app/workers/deletes.php | 22 ++ .../Database/Validator/Queries/Messages.php | 3 +- .../Database/Validator/Queries/Providers.php | 2 - tests/e2e/Services/GraphQL/MessagingTest.php | 198 +++++++++--------- 6 files changed, 146 insertions(+), 125 deletions(-) diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index b3deaa409f..9a2f5b9856 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -1,5 +1,6 @@ getValue(); - $cursorDocument = Authorization::skip(fn () => $dbForProject->findOne('providers', [ - Query::equal('$id', [$providerId]), - ])); + $cursorDocument = Authorization::skip(fn () => $dbForProject->getDocument('providers', $providerId)); - if ($cursorDocument === false || $cursorDocument->isEmpty()) { + if ($cursorDocument->isEmpty()) { throw new Exception(Exception::GENERAL_CURSOR_NOT_FOUND, "Provider '{$providerId}' for the 'cursor' value not found."); } @@ -1242,8 +1241,8 @@ App::post('/v1/messaging/topics') } $response - ->setStatusCode(Response::STATUS_CODE_CREATED) - ->dynamic($topic, Response::MODEL_TOPIC); + ->setStatusCode(Response::STATUS_CODE_CREATED) + ->dynamic($topic, Response::MODEL_TOPIC); }); App::get('/v1/messaging/topics') @@ -1269,12 +1268,9 @@ App::get('/v1/messaging/topics') if ($cursor) { $topicId = $cursor->getValue(); - $cursorDocument = Authorization::skip(fn () => $dbForProject->find('topics', [ - Query::equal('$id', [$topicId]), - Query::limit(1), - ])); + $cursorDocument = Authorization::skip(fn () => $dbForProject->getDocument('topics', $topicId)); - if (empty($cursorDocument) || $cursorDocument[0]->isEmpty()) { + if ($cursorDocument->isEmpty()) { throw new Exception(Exception::GENERAL_CURSOR_NOT_FOUND, "Topic '{$topicId}' for the 'cursor' value not found."); } @@ -1330,7 +1326,7 @@ App::patch('/v1/messaging/topics/:topicId') ->label('sdk.response.model', Response::MODEL_TOPIC) ->param('topicId', '', new UID(), 'Topic ID.') ->param('name', '', new Text(128), 'Topic Name.', true) - ->param('description', null, new Text(2048), 'Topic Description.', true) + ->param('description', '', new Text(2048), 'Topic Description.', true) ->inject('dbForProject') ->inject('response') ->action(function (string $topicId, string $name, string $description, Database $dbForProject, Response $response) { @@ -1340,11 +1336,11 @@ App::patch('/v1/messaging/topics/:topicId') throw new Exception(Exception::TOPIC_NOT_FOUND); } - if ($name) { + if (!empty($name)) { $topic->setAttribute('name', $name); } - if ($description) { + if (!empty($description)) { $topic->setAttribute('description', $description); } @@ -1369,15 +1365,20 @@ App::delete('/v1/messaging/topics/:topicId') ->label('sdk.response.model', Response::MODEL_NONE) ->param('topicId', '', new UID(), 'Topic ID.') ->inject('dbForProject') + ->inject('deletes') ->inject('response') - ->action(function (string $topicId, Database $dbForProject, Response $response) { + ->action(function (string $topicId, Database $dbForProject, Delete $deletes, Response $response) { $topic = $dbForProject->getDocument('topics', $topicId); if ($topic->isEmpty()) { throw new Exception(Exception::TOPIC_NOT_FOUND); } - $topic = $dbForProject->deleteDocument('topics', $topicId); + $dbForProject->deleteDocument('topics', $topicId); + + $deletes + ->setType(DELETE_TYPE_SUBSCRIBERS) + ->setDocument($topic); $response ->setStatusCode(Response::STATUS_CODE_NOCONTENT) @@ -1437,8 +1438,8 @@ App::post('/v1/messaging/topics/:topicId/subscribers') } $response - ->setStatusCode(Response::STATUS_CODE_CREATED) - ->dynamic($subscriber, Response::MODEL_SUBSCRIBER); + ->setStatusCode(Response::STATUS_CODE_CREATED) + ->dynamic($subscriber, Response::MODEL_SUBSCRIBER); }); App::get('/v1/messaging/topics/:topicId/subscribers') @@ -1563,7 +1564,7 @@ App::post('/v1/messaging/messages/email') ->param('content', '', new Text(64230), 'Email Content.') ->param('status', 'processing', new WhiteList(['draft', 'processing']), 'Message Status.', true) ->param('html', false, new Boolean(), 'Is content of type HTML', true) - ->param('deliveryTime', null, new DatetimeValidator(), 'Delivery time for message.', true) + ->param('deliveryTime', null, new DatetimeValidator(false), 'Delivery time for message.', true) ->inject('dbForProject') ->inject('project') ->inject('messaging') @@ -1634,11 +1635,9 @@ App::get('/v1/messaging/messages') if ($cursor) { $messageId = $cursor->getValue(); - $cursorDocument = Authorization::skip(fn () => $dbForProject->findOne('messages', [ - Query::equal('$id', [$messageId]), - ])); + $cursorDocument = Authorization::skip(fn () => $dbForProject->getDocument('messages', $messageId)); - if ($cursorDocument === false || $cursorDocument->isEmpty()) { + if ($cursorDocument->isEmpty()) { throw new Exception(Exception::GENERAL_CURSOR_NOT_FOUND, "Message '{$messageId}' for the 'cursor' value not found."); } diff --git a/app/init.php b/app/init.php index 5b40ad386c..cc1c77c6ea 100644 --- a/app/init.php +++ b/app/init.php @@ -169,6 +169,7 @@ const DELETE_TYPE_SESSIONS = 'sessions'; const DELETE_TYPE_CACHE_BY_TIMESTAMP = 'cacheByTimeStamp'; const DELETE_TYPE_CACHE_BY_RESOURCE = 'cacheByResource'; const DELETE_TYPE_SCHEDULES = 'schedules'; +const DELETE_TYPE_SUBSCRIBERS = 'subscribers'; // Compression type const COMPRESSION_TYPE_NONE = 'none'; const COMPRESSION_TYPE_GZIP = 'gzip'; diff --git a/app/workers/deletes.php b/app/workers/deletes.php index f831c1df3f..d5e319a662 100644 --- a/app/workers/deletes.php +++ b/app/workers/deletes.php @@ -127,6 +127,10 @@ class DeletesV1 extends Worker case DELETE_TYPE_SCHEDULES: $this->deleteSchedules($this->args['datetime']); break; + case DELETE_TYPE_SUBSCRIBERS: + $topic = new Document($this->args['document'] ?? []); + $this->deleteSubscribers($project, $topic); + break; default: Console::error('No delete operation for type: ' . $type); break; @@ -170,6 +174,24 @@ class DeletesV1 extends Worker ); } + /** + * @param Document $project + * @param Document $topic + * @throws Exception + */ + protected function deleteSubscribers(Document $project, Document $topic) + { + if ($topic->isEmpty()) { + Console::error('Failed to delete subscribers. Topic not found'); + return; + } + $dbForProject = $this->getProjectDB($project); + + $this->deleteByGroup('subscribers', [ + Query::equal('topicInternalId', [$topic->getInternalId()]) + ], $dbForProject); + } + /** * @param Document $project * @param string $resource diff --git a/src/Appwrite/Utopia/Database/Validator/Queries/Messages.php b/src/Appwrite/Utopia/Database/Validator/Queries/Messages.php index 5fa391e7f6..4bff13ae19 100644 --- a/src/Appwrite/Utopia/Database/Validator/Queries/Messages.php +++ b/src/Appwrite/Utopia/Database/Validator/Queries/Messages.php @@ -11,7 +11,8 @@ class Messages extends Base 'deliveredTo', 'deliveryErrors', 'status', - 'description' + 'description', + 'data' ]; /** diff --git a/src/Appwrite/Utopia/Database/Validator/Queries/Providers.php b/src/Appwrite/Utopia/Database/Validator/Queries/Providers.php index ad0e08403c..1fd6c9e9f8 100644 --- a/src/Appwrite/Utopia/Database/Validator/Queries/Providers.php +++ b/src/Appwrite/Utopia/Database/Validator/Queries/Providers.php @@ -10,8 +10,6 @@ class Providers extends Base 'type', 'default', 'enabled', - 'credentials', - 'options' ]; /** diff --git a/tests/e2e/Services/GraphQL/MessagingTest.php b/tests/e2e/Services/GraphQL/MessagingTest.php index 178b9a6a6c..8c3e4970e7 100644 --- a/tests/e2e/Services/GraphQL/MessagingTest.php +++ b/tests/e2e/Services/GraphQL/MessagingTest.php @@ -270,8 +270,8 @@ class MessagingTest extends Scope ]; $query = $this->getQuery(self::$CREATE_SENDGRID_PROVIDER); $graphQLPayload = [ - 'query' => $query, - 'variables' => $providerParam['sendgrid'], + 'query' => $query, + 'variables' => $providerParam['sendgrid'], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -283,13 +283,13 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_TOPIC); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'providerId' => $providerId, - 'topicId' => ID::unique(), - 'name' => 'topic1', - 'description' => 'Active users', - ], + 'query' => $query, + 'variables' => [ + 'providerId' => $providerId, + 'topicId' => ID::unique(), + 'name' => 'topic1', + 'description' => 'Active users', + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -312,12 +312,12 @@ class MessagingTest extends Scope $topicId = $topic['_id']; $query = $this->getQuery(self::$UPDATE_TOPIC); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'topicId' => $topicId, - 'name' => 'topic2', - 'description' => 'Inactive users', - ], + 'query' => $query, + 'variables' => [ + 'topicId' => $topicId, + 'name' => 'topic2', + 'description' => 'Inactive users', + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -339,7 +339,7 @@ class MessagingTest extends Scope { $query = $this->getQuery(self::$LIST_TOPICS); $graphQLPayload = [ - 'query' => $query, + 'query' => $query, ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -358,10 +358,10 @@ class MessagingTest extends Scope { $query = $this->getQuery(self::$GET_TOPIC); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'topicId' => $topicId, - ], + 'query' => $query, + 'variables' => [ + 'topicId' => $topicId, + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -385,13 +385,13 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_USER_TARGET); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'targetId' => ID::unique(), - 'userId' => $userId, - 'providerId' => $topic['providerId'], - 'identifier' => 'token', - ], + 'query' => $query, + 'variables' => [ + 'targetId' => ID::unique(), + 'userId' => $userId, + 'providerId' => $topic['providerId'], + 'identifier' => 'token', + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -407,12 +407,12 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_SUBSCRIBER); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'subscriberId' => ID::unique(), - 'topicId' => $topicId, - 'targetId' => $targetId, - ], + 'query' => $query, + 'variables' => [ + 'subscriberId' => ID::unique(), + 'topicId' => $topicId, + 'targetId' => $targetId, + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -431,10 +431,10 @@ class MessagingTest extends Scope { $query = $this->getQuery(self::$LIST_SUBSCRIBERS); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'topicId' => $topicId, - ], + 'query' => $query, + 'variables' => [ + 'topicId' => $topicId, + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -456,11 +456,11 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$GET_SUBSCRIBER); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'topicId' => $topicId, - 'subscriberId' => $subscriberId, - ], + 'query' => $query, + 'variables' => [ + 'topicId' => $topicId, + 'subscriberId' => $subscriberId, + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ @@ -483,11 +483,11 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$DELETE_SUBSCRIBER); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'topicId' => $topicId, - 'subscriberId' => $subscriberId, - ], + 'query' => $query, + 'variables' => [ + 'topicId' => $topicId, + 'subscriberId' => $subscriberId, + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ @@ -504,10 +504,10 @@ class MessagingTest extends Scope { $query = $this->getQuery(self::$DELETE_TOPIC); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'topicId' => $topicId, - ], + 'query' => $query, + 'variables' => [ + 'topicId' => $topicId, + ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -531,15 +531,15 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_MAILGUN_PROVIDER); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'providerId' => ID::unique(), - 'name' => 'Mailgun1', - 'apiKey' => $apiKey, - 'domain' => $domain, - 'from' => $from, - 'isEuRegion' => $isEuRegion, - ], + 'query' => $query, + 'variables' => [ + 'providerId' => ID::unique(), + 'name' => 'Mailgun1', + 'apiKey' => $apiKey, + 'domain' => $domain, + 'from' => $from, + 'isEuRegion' => $isEuRegion, + ], ]; $provider = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -553,13 +553,13 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_TOPIC); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'providerId' => $providerId, - 'topicId' => ID::unique(), - 'name' => 'topic1', - 'description' => 'Active users', - ], + 'query' => $query, + 'variables' => [ + 'providerId' => $providerId, + 'topicId' => ID::unique(), + 'name' => 'topic1', + 'description' => 'Active users', + ], ]; $topic = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -571,13 +571,13 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_USER); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'userId' => ID::custom('test-user'), - 'email' => $to, - 'password' => 'password', - 'name' => 'Messaging User', - ] + 'query' => $query, + 'variables' => [ + 'userId' => ID::custom('test-user'), + 'email' => $to, + 'password' => 'password', + 'name' => 'Messaging User', + ] ]; $user = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -589,13 +589,13 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_USER_TARGET); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'targetId' => ID::unique(), - 'userId' => $user['body']['data']['usersCreate']['_id'], - 'providerId' => $providerId, - 'identifier' => $to, - ], + 'query' => $query, + 'variables' => [ + 'targetId' => ID::unique(), + 'userId' => $user['body']['data']['usersCreate']['_id'], + 'providerId' => $providerId, + 'identifier' => $to, + ], ]; $target = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -607,12 +607,12 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_SUBSCRIBER); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'subscriberId' => ID::unique(), - 'topicId' => $topic['body']['data']['messagingCreateTopic']['_id'], - 'targetId' => $target['body']['data']['usersCreateTarget']['_id'], - ], + 'query' => $query, + 'variables' => [ + 'subscriberId' => ID::unique(), + 'topicId' => $topic['body']['data']['messagingCreateTopic']['_id'], + 'targetId' => $target['body']['data']['usersCreateTarget']['_id'], + ], ]; $subscriber = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -623,14 +623,14 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$CREATE_EMAIL); $graphQLPayload = [ - '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', - ], + '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', + ], ]; $email = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json', @@ -644,10 +644,10 @@ class MessagingTest extends Scope $query = $this->getQuery(self::$GET_MESSAGE); $graphQLPayload = [ - 'query' => $query, - 'variables' => [ - 'messageId' => $email['body']['data']['messagingCreateEmail']['_id'], - ], + 'query' => $query, + 'variables' => [ + 'messageId' => $email['body']['data']['messagingCreateEmail']['_id'], + ], ]; $message = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ 'content-type' => 'application/json',