fix: Fix errors spotted by reviewers, fix `@throws` annotations

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

diff --git a/apps/webhooks/lib/BackgroundJobs/WebhookCall.php b/apps/webhooks/lib/BackgroundJobs/WebhookCall.php
index 4c0def3f69de5288b04f7a559e1a85ddeab36bda..06c3a4cf5d72a3fe210590489dfdb59670957cb1 100644 (file)
--- a/apps/webhooks/lib/BackgroundJobs/WebhookCall.php
+++ b/apps/webhooks/lib/BackgroundJobs/WebhookCall.php
@@ -45,9 +45,9 @@ class WebhookCall extends QueuedJob {
                        $response = $client->request($webhookListener->getHttpMethod(), $webhookListener->getUri(), $options);
                        $statusCode = $response->getStatusCode();
                        if ($statusCode >= 200 && $statusCode < 300) {
-                               $this->logger->warning('Webhook returned unexpected status code '.$statusCode, ['body' => $response->getBody()]);
-                       } else {
                                $this->logger->debug('Webhook returned status code '.$statusCode, ['body' => $response->getBody()]);
+                       } else {
+                               $this->logger->warning('Webhook returned unexpected status code '.$statusCode, ['body' => $response->getBody()]);
                        }
                } catch (\Exception $e) {
                        $this->logger->error('Webhook call failed: '.$e->getMessage(), ['exception' => $e]);

diff --git a/apps/webhooks/lib/Controller/WebhooksController.php b/apps/webhooks/lib/Controller/WebhooksController.php
index 832f22727340b64840ccf4f04cdd15b13faa7b3b..f61e458430597c11edabf17df1e5e66c8ae8d0ee 100644 (file)
--- a/apps/webhooks/lib/Controller/WebhooksController.php
+++ b/apps/webhooks/lib/Controller/WebhooksController.php
@@ -9,9 +9,9 @@ declare(strict_types=1);
 
 namespace OCA\Webhooks\Controller;
 
-use Doctrine\DBAL\Exception;
 use OCA\Webhooks\Db\AuthMethod;
 use OCA\Webhooks\Db\WebhookListenerMapper;
+use OCA\Webhooks\ResponseDefinitions;
 use OCP\AppFramework\Http\Attribute\ApiRoute;
 use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
@@ -210,7 +210,7 @@ class WebhooksController extends OCSController {
                        throw new OCSBadRequestException($e->getMessage(), $e);
                } catch (\DomainException $e) {
                        throw new OCSForbiddenException($e->getMessage(), $e);
-               } catch (Exception $e) {
+               } catch (\Exception $e) {
                        $this->logger->error('Error when deleting flow with id ' . $id, ['exception' => $e]);
                        throw new OCSException('An internal error occurred', $e->getCode(), $e);
                }

diff --git a/apps/webhooks/lib/Db/WebhookListenerMapper.php b/apps/webhooks/lib/Db/WebhookListenerMapper.php
index 8f07a413c7f7ad609f51354e3ed7e22c4ec722e3..a4a43a882f5929d0c8e44badcd12e58bbe8435f6 100644 (file)
--- a/apps/webhooks/lib/Db/WebhookListenerMapper.php
+++ b/apps/webhooks/lib/Db/WebhookListenerMapper.php
@@ -42,8 +42,6 @@ class WebhookListenerMapper extends QBMapper {
        }
 
        /**
-        * @throws DoesNotExistException
-        * @throws MultipleObjectsReturnedException
         * @throws Exception
         * @return WebhookListener[]
         */
@@ -56,6 +54,9 @@ class WebhookListenerMapper extends QBMapper {
                return $this->findEntities($qb);
        }
 
+       /**
+        * @throws Exception
+        */
        public function addWebhookListener(
                ?string $appId,
                string $userId,
@@ -66,7 +67,7 @@ class WebhookListenerMapper extends QBMapper {
                ?array $headers,
                AuthMethod $authMethod,
                ?array $authData,
-       ) {
+       ): WebhookListener {
                $webhookListener = WebhookListener::fromParams(
                        [
                                'appId' => $appId,
@@ -83,6 +84,9 @@ class WebhookListenerMapper extends QBMapper {
                return $this->insert($webhookListener);
        }
 
+       /**
+        * @throws Exception
+        */
        public function updateWebhookListener(
                int $id,
                ?string $appId,
@@ -94,7 +98,7 @@ class WebhookListenerMapper extends QBMapper {
                ?array $headers,
                AuthMethod $authMethod,
                ?array $authData,
-       ) {
+       ): WebhookListener {
                $webhookListener = WebhookListener::fromParams(
                        [
                                'id' => $id,
@@ -113,8 +117,6 @@ class WebhookListenerMapper extends QBMapper {
        }
 
        /**
-        * @throws DoesNotExistException
-        * @throws MultipleObjectsReturnedException
         * @throws Exception
         */
        public function deleteById(int $id): bool {
@@ -127,6 +129,7 @@ class WebhookListenerMapper extends QBMapper {
        }
 
        /**
+        * @throws Exception
         * @return list<string>
         * TODO cache
         */
@@ -147,6 +150,9 @@ class WebhookListenerMapper extends QBMapper {
                return $configuredEvents;
        }
 
+       /**
+        * @throws Exception
+        */
        public function getByEvent(string $event): array {
                $qb = $this->db->getQueryBuilder();
 

diff --git a/apps/webhooks/lib/Listener/WebhooksEventListener.php b/apps/webhooks/lib/Listener/WebhooksEventListener.php
index 5ea0012d4e9cddd31dd15e0e7140c8890ba0a188..afe53595e75dc9a94a975802bbfceb5efd2cf2f2 100644 (file)
--- a/apps/webhooks/lib/Listener/WebhooksEventListener.php
+++ b/apps/webhooks/lib/Listener/WebhooksEventListener.php
@@ -34,14 +34,13 @@ class WebhooksEventListener implements IEventListener {
 
        public function handle(Event $event): void {
                $webhookListeners = $this->mapper->getByEvent($event::class);
-               /** @var IUser */
                $user = $this->userSession->getUser();
 
                foreach ($webhookListeners as $webhookListener) {
                        // TODO add group membership to be able to filter on it
                        $data = [
                                'event' => $this->serializeEvent($event),
-                               'user' => JsonSerializer::serializeUser($user),
+                               'user' => (is_null($user) ? null : JsonSerializer::serializeUser($user)),
                                'time' => time(),
                        ];
                        if ($this->filterMatch($webhookListener->getEventFilter(), $data)) {

diff --git a/apps/webhooks/lib/Migration/Version1000Date20240527153425.php b/apps/webhooks/lib/Migration/Version1000Date20240527153425.php
index f2d722687de983be4a59fe26fd6aca5fa6377c21..7c6f66314e4685a24e7b585d737c6bf557602c79 100755 (executable)
--- a/apps/webhooks/lib/Migration/Version1000Date20240527153425.php
+++ b/apps/webhooks/lib/Migration/Version1000Date20240527153425.php
@@ -26,7 +26,7 @@ class Version1000Date20240527153425 extends SimpleMigrationStep {
 
                if (!$schema->hasTable(WebhookListenerMapper::TABLE_NAME)) {
                        $table = $schema->createTable(WebhookListenerMapper::TABLE_NAME);
-                       $table->addColumn('id', Types::INTEGER, [
+                       $table->addColumn('id', Types::BIGINT, [
                                'autoincrement' => true,
                                'notnull' => true,
                                'length' => 4,
@@ -45,7 +45,7 @@ class Version1000Date20240527153425 extends SimpleMigrationStep {
                        ]);
                        $table->addColumn('uri', Types::STRING, [
                                'notnull' => true,
-                               'length' => 256,
+                               'length' => 4096,
                        ]);
                        $table->addColumn('event', Types::TEXT, [
                                'notnull' => true,