Skip to content

Commit

Permalink
fix(webhooks): Fix last psalm and openapi problems with the API
Browse files Browse the repository at this point in the history 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
  • Loading branch information
@come-nc
come-nc committed Jun 11, 2024
1 parent d5b53be commit bff7d3c
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 11 deletions.
53 changes: 43 additions & 10 deletions apps/webhook_listeners/lib/Controller/WebhooksController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
namespace OCA\WebhookListeners\Controller;

use OCA\WebhookListeners\Db\AuthMethod;
use OCA\WebhookListeners\Db\WebhookListener;
use OCA\WebhookListeners\Db\WebhookListenerMapper;
use OCA\WebhookListeners\ResponseDefinitions;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\ApiRoute;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
Expand All @@ -20,6 +22,7 @@
use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCSController;
use OCP\IRequest;
use OCP\ISession;
Expand All @@ -45,15 +48,26 @@ public function __construct(
* List registered webhooks
*
* @return DataResponse<Http::STATUS_OK, WebhookListenersWebhookInfo[], array{}>
* @throws OCSException Other internal error
*
* 200: Webhook registrations returned
*/
#[ApiRoute(verb: 'GET', url: '/api/v1/webhooks')]
#[AuthorizedAdminSetting(settings:'OCA\WebhookListeners\Settings\Admin')]

Check notice

Code scanning / Psalm

ArgumentTypeCoercion Note

Argument 1 of OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting::__construct expects class-string<OCP\Settings\IDelegatedSettings>, but parent type 'OCA\\WebhookListeners\\Settings\\Admin' provided
public function index(): DataResponse {
$webhookListeners = $this->mapper->getAll();
try {
$webhookListeners = $this->mapper->getAll();

return new DataResponse($webhookListeners);
return new DataResponse(
array_map(
fn (WebhookListener $listener): array => $listener->jsonSerialize(),
$webhookListeners
)
);
} catch (\Exception $e) {
$this->logger->error('Error when listing webhooks', ['exception' => $e]);
throw new OCSException('An internal error occurred', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
}
}

/**
Expand All @@ -62,13 +76,22 @@ public function index(): DataResponse {
* @param int $id id of the webhook
*
* @return DataResponse<Http::STATUS_OK, WebhookListenersWebhookInfo, array{}>
* @throws OCSNotFoundException Webhook not found
* @throws OCSException Other internal error
*
* 200: Webhook registration returned
*/
#[ApiRoute(verb: 'GET', url: '/api/v1/webhooks/{id}')]
#[AuthorizedAdminSetting(settings:'OCA\WebhookListeners\Settings\Admin')]

Check notice

Code scanning / Psalm

ArgumentTypeCoercion Note

Argument 1 of OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting::__construct expects class-string<OCP\Settings\IDelegatedSettings>, but parent type 'OCA\\WebhookListeners\\Settings\\Admin' provided
public function show(int $id): DataResponse {
return new DataResponse($this->mapper->getById($id));
try {
return new DataResponse($this->mapper->getById($id)->jsonSerialize());
} catch (DoesNotExistException $e) {
throw new OCSNotFoundException($e->getMessage(), $e);
} catch (\Exception $e) {
$this->logger->error('Error when getting webhook', ['exception' => $e]);
throw new OCSException('An internal error occurred', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
}
}

/**
Expand Down Expand Up @@ -106,6 +129,11 @@ public function create(
if ($this->session->get('app_api') === true) {
$appId = $this->request->getHeader('EX-APP-ID');
}
try {
$authMethod = AuthMethod::from($authMethod ?? AuthMethod::None->value);
} catch (\ValueError $e) {
throw new OCSBadRequestException('This auth method does not exist');
}
try {
$webhookListener = $this->mapper->addWebhookListener(
$appId,
Expand All @@ -115,17 +143,17 @@ public function create(
$event,
$eventFilter,
$headers,
AuthMethod::from($authMethod ?? AuthMethod::None->value),
$authMethod,
$authData,
);
return new DataResponse($webhookListener);
return new DataResponse($webhookListener->jsonSerialize());
} catch (\UnexpectedValueException $e) {
throw new OCSBadRequestException($e->getMessage(), $e);
} catch (\DomainException $e) {
throw new OCSForbiddenException($e->getMessage(), $e);
} catch (\Exception $e) {
$this->logger->error('Error when inserting webhook', ['exception' => $e]);
throw new OCSException('An internal error occurred', $e->getCode(), $e);
throw new OCSException('An internal error occurred', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
}
}

Expand Down Expand Up @@ -166,6 +194,11 @@ public function update(
if ($this->session->get('app_api') === true) {
$appId = $this->request->getHeader('EX-APP-ID');
}
try {
$authMethod = AuthMethod::from($authMethod ?? AuthMethod::None->value);
} catch (\ValueError $e) {
throw new OCSBadRequestException('This auth method does not exist');
}
try {
$webhookListener = $this->mapper->updateWebhookListener(
$id,
Expand All @@ -176,17 +209,17 @@ public function update(
$event,
$eventFilter,
$headers,
AuthMethod::from($authMethod ?? AuthMethod::None->value),
$authMethod,
$authData,
);
return new DataResponse($webhookListener);
return new DataResponse($webhookListener->jsonSerialize());
} catch (\UnexpectedValueException $e) {
throw new OCSBadRequestException($e->getMessage(), $e);
} catch (\DomainException $e) {
throw new OCSForbiddenException($e->getMessage(), $e);
} catch (\Exception $e) {
$this->logger->error('Error when updating flow with id ' . $id, ['exception' => $e]);
throw new OCSException('An internal error occurred', $e->getCode(), $e);
throw new OCSException('An internal error occurred', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
}
}

Expand Down Expand Up @@ -215,7 +248,7 @@ public function destroy(int $id): DataResponse {
throw new OCSForbiddenException($e->getMessage(), $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);
throw new OCSException('An internal error occurred', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
}
}
}
4 changes: 3 additions & 1 deletion apps/webhook_listeners/lib/Settings/Admin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace OCA\WebhookListeners\Settings;

use OCP\AppFramework\Http;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IL10N;
use OCP\Settings\IDelegatedSettings;
Expand All @@ -28,7 +29,8 @@ public function __construct(
* Empty template response
*/
public function getForm(): TemplateResponse {
return new class($this->appName, '') extends TemplateResponse {

return new /** @template-extends TemplateResponse<Http::STATUS_OK, array{}> */ class($this->appName, '') extends TemplateResponse {
public function render(): string {
return '';
}
Expand Down
28 changes: 28 additions & 0 deletions apps/webhook_listeners/openapi.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,34 @@
}
}
}
},
"404": {
"description": "Webhook not found",
"content": {
"application/json": {
"schema": {
"type": "object",
"required": [
"ocs"
],
"properties": {
"ocs": {
"type": "object",
"required": [
"meta",
"data"
],
"properties": {
"meta": {
"$ref": "#/components/schemas/OCSMeta"
},
"data": {}
}
}
}
}
}
}
}
}
},
Expand Down

0 comments on commit bff7d3c

Please sign in to comment.