From 7eca1c1f66e655b610d97cd71e8084669c6c39b1 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Mon, 1 Apr 2024 21:32:01 +0700 Subject: [PATCH 1/4] feat: Write critical operations to the audit log Signed-off-by: Hoang Pham --- lib/AppInfo/Application.php | 16 +++++++++ lib/Event/RowDeletedEvent.php | 26 ++++++++++++++ lib/Event/TableDeletedEvent.php | 26 ++++++++++++++ lib/Event/TableOwnershipTransferredEvent.php | 31 +++++++++++++++++ lib/Event/ViewDeletedEvent.php | 26 ++++++++++++++ .../WhenRowDeletedAuditLogListener.php | 33 ++++++++++++++++++ .../WhenTableDeletedAuditLogListener.php | 32 +++++++++++++++++ .../WhenTableTransferredAuditLogListener.php | 34 +++++++++++++++++++ .../WhenViewDeletedAuditLogListener.php | 32 +++++++++++++++++ lib/Service/RowService.php | 29 ++++++++++++++-- .../Support/AuditLogServiceInterface.php | 10 ++++++ .../Support/DefaultAuditLogService.php | 22 ++++++++++++ lib/Service/TableService.php | 31 +++++++++++++++-- lib/Service/ViewService.php | 29 ++++++++++++++-- 14 files changed, 368 insertions(+), 9 deletions(-) create mode 100644 lib/Event/RowDeletedEvent.php create mode 100644 lib/Event/TableDeletedEvent.php create mode 100644 lib/Event/TableOwnershipTransferredEvent.php create mode 100644 lib/Event/ViewDeletedEvent.php create mode 100644 lib/Listener/WhenRowDeletedAuditLogListener.php create mode 100644 lib/Listener/WhenTableDeletedAuditLogListener.php create mode 100644 lib/Listener/WhenTableTransferredAuditLogListener.php create mode 100644 lib/Listener/WhenViewDeletedAuditLogListener.php create mode 100644 lib/Service/Support/AuditLogServiceInterface.php create mode 100644 lib/Service/Support/DefaultAuditLogService.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 4d1ecec19..4515e1761 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -5,15 +5,25 @@ use Exception; use OCA\Analytics\Datasource\DatasourceEvent; use OCA\Tables\Capabilities; +use OCA\Tables\Event\RowDeletedEvent; +use OCA\Tables\Event\TableDeletedEvent; +use OCA\Tables\Event\TableOwnershipTransferredEvent; +use OCA\Tables\Event\ViewDeletedEvent; use OCA\Tables\Listener\AnalyticsDatasourceListener; use OCA\Tables\Listener\BeforeTemplateRenderedListener; use OCA\Tables\Listener\LoadAdditionalListener; use OCA\Tables\Listener\TablesReferenceListener; use OCA\Tables\Listener\UserDeletedListener; +use OCA\Tables\Listener\WhenRowDeletedAuditLogListener; +use OCA\Tables\Listener\WhenTableDeletedAuditLogListener; +use OCA\Tables\Listener\WhenTableTransferredAuditLogListener; +use OCA\Tables\Listener\WhenViewDeletedAuditLogListener; use OCA\Tables\Middleware\PermissionMiddleware; use OCA\Tables\Reference\ContentReferenceProvider; use OCA\Tables\Reference\ReferenceProvider; use OCA\Tables\Search\SearchTablesProvider; +use OCA\Tables\Service\Support\AuditLogServiceInterface; +use OCA\Tables\Service\Support\DefaultAuditLogService; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; @@ -47,11 +57,17 @@ public function register(IRegistrationContext $context): void { throw new Exception('Cannot include autoload. Did you run install dependencies using composer?'); } + $context->registerService(AuditLogServiceInterface::class, fn($c) => $c->query(DefaultAuditLogService::class)); + $context->registerEventListener(BeforeUserDeletedEvent::class, UserDeletedListener::class); $context->registerEventListener(DatasourceEvent::class, AnalyticsDatasourceListener::class); $context->registerEventListener(RenderReferenceEvent::class, TablesReferenceListener::class); $context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); $context->registerEventListener(LoadAdditionalScriptsEvent::class, LoadAdditionalListener::class); + $context->registerEventListener(TableDeletedEvent::class, WhenTableDeletedAuditLogListener::class); + $context->registerEventListener(ViewDeletedEvent::class, WhenViewDeletedAuditLogListener::class); + $context->registerEventListener(RowDeletedEvent::class, WhenRowDeletedAuditLogListener::class); + $context->registerEventListener(TableOwnershipTransferredEvent::class, WhenTableTransferredAuditLogListener::class); $context->registerSearchProvider(SearchTablesProvider::class); diff --git a/lib/Event/RowDeletedEvent.php b/lib/Event/RowDeletedEvent.php new file mode 100644 index 000000000..a3a894b51 --- /dev/null +++ b/lib/Event/RowDeletedEvent.php @@ -0,0 +1,26 @@ +row; + } + + public function getUserId(): string + { + return $this->userId; + } +} \ No newline at end of file diff --git a/lib/Event/TableDeletedEvent.php b/lib/Event/TableDeletedEvent.php new file mode 100644 index 000000000..4acac95df --- /dev/null +++ b/lib/Event/TableDeletedEvent.php @@ -0,0 +1,26 @@ +table; + } + + public function getUserId(): string + { + return $this->userId; + } +} \ No newline at end of file diff --git a/lib/Event/TableOwnershipTransferredEvent.php b/lib/Event/TableOwnershipTransferredEvent.php new file mode 100644 index 000000000..96bd44c69 --- /dev/null +++ b/lib/Event/TableOwnershipTransferredEvent.php @@ -0,0 +1,31 @@ +table; + } + + public function getFromUserId(): string + { + return $this->fromUserId; + } + + public function getToUserId(): string + { + return $this->toUserId; + } +} \ No newline at end of file diff --git a/lib/Event/ViewDeletedEvent.php b/lib/Event/ViewDeletedEvent.php new file mode 100644 index 000000000..b49512053 --- /dev/null +++ b/lib/Event/ViewDeletedEvent.php @@ -0,0 +1,26 @@ +view; + } + + public function getUserId(): string + { + return $this->userId; + } +} \ No newline at end of file diff --git a/lib/Listener/WhenRowDeletedAuditLogListener.php b/lib/Listener/WhenRowDeletedAuditLogListener.php new file mode 100644 index 000000000..2fba386fb --- /dev/null +++ b/lib/Listener/WhenRowDeletedAuditLogListener.php @@ -0,0 +1,33 @@ +getRow(); + $userId = $event->getUserId(); + $rowId = $row->getId(); + + $this->auditLogService->log("Row with ID: $rowId was deleted by user with ID: $userId", [ + 'row' => $row->jsonSerialize(), + 'userId' => $userId, + ]); + } +} \ No newline at end of file diff --git a/lib/Listener/WhenTableDeletedAuditLogListener.php b/lib/Listener/WhenTableDeletedAuditLogListener.php new file mode 100644 index 000000000..5994eaa17 --- /dev/null +++ b/lib/Listener/WhenTableDeletedAuditLogListener.php @@ -0,0 +1,32 @@ +getTable(); + $userId = $event->getUserId(); + + $this->auditLogService->log("Table with ID: $table->id was deleted by user with ID: $userId", [ + 'table' => $table->jsonSerialize(), + 'userId' => $userId, + ]); + } +} \ No newline at end of file diff --git a/lib/Listener/WhenTableTransferredAuditLogListener.php b/lib/Listener/WhenTableTransferredAuditLogListener.php new file mode 100644 index 000000000..fb23553a3 --- /dev/null +++ b/lib/Listener/WhenTableTransferredAuditLogListener.php @@ -0,0 +1,34 @@ +getTable(); + $fromUserId = $event->getFromUserId(); + $toUserId = $event->getToUserId(); + + $this->auditLogService->log("Table with ID: $table->id was transferred from user with ID: $fromUserId to user with ID: $toUserId", [ + 'table' => $table->jsonSerialize(), + 'fromUserId' => $fromUserId, + 'toUserId' => $toUserId, + ]); + } +} \ No newline at end of file diff --git a/lib/Listener/WhenViewDeletedAuditLogListener.php b/lib/Listener/WhenViewDeletedAuditLogListener.php new file mode 100644 index 000000000..8aad0df7b --- /dev/null +++ b/lib/Listener/WhenViewDeletedAuditLogListener.php @@ -0,0 +1,32 @@ +getView(); + $userId = $event->getUserId(); + + $this->auditLogService->log("View with ID: $view->id was deleted by user with ID: $userId", [ + 'view' => $view->jsonSerialize(), + 'userId' => $userId, + ]); + } +} \ No newline at end of file diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index e8d3ca416..b399bc93f 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -13,11 +13,13 @@ use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Event\RowDeletedEvent; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnTypes\IColumnTypeBusiness; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Server; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; @@ -33,13 +35,25 @@ class RowService extends SuperService { private Row2Mapper $row2Mapper; private array $tmpRows = []; // holds already loaded rows as a small cache - public function __construct(PermissionsService $permissionsService, LoggerInterface $logger, ?string $userId, - ColumnMapper $columnMapper, ViewMapper $viewMapper, TableMapper $tableMapper, Row2Mapper $row2Mapper) { + protected IEventDispatcher $eventDispatcher; + + public function __construct( + PermissionsService $permissionsService, + LoggerInterface $logger, + ?string $userId, + ColumnMapper $columnMapper, + ViewMapper $viewMapper, + TableMapper $tableMapper, + Row2Mapper $row2Mapper, + IEventDispatcher $eventDispatcher + ) + { parent::__construct($logger, $userId, $permissionsService); $this->columnMapper = $columnMapper; $this->viewMapper = $viewMapper; $this->tableMapper = $tableMapper; $this->row2Mapper = $row2Mapper; + $this->eventDispatcher = $eventDispatcher; } /** @@ -450,7 +464,16 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { } try { - return $this->filterRowResult($view ?? null, $this->row2Mapper->delete($item)); + $deletedRow = $this->row2Mapper->delete($item); + + $event = new RowDeletedEvent( + row: $item, + userId: $userId + ); + + $this->eventDispatcher->dispatchTyped($event); + + return $this->filterRowResult($view ?? null, $deletedRow); } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); diff --git a/lib/Service/Support/AuditLogServiceInterface.php b/lib/Service/Support/AuditLogServiceInterface.php new file mode 100644 index 000000000..ec761f1de --- /dev/null +++ b/lib/Service/Support/AuditLogServiceInterface.php @@ -0,0 +1,10 @@ +eventDispatcher->dispatchTyped($auditEvent); + } +} \ No newline at end of file diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index e28fe5fcb..c6b3725e5 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -5,19 +5,20 @@ namespace OCA\Tables\Service; use DateTime; - use OCA\Tables\AppInfo\Application; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Event\TableDeletedEvent; +use OCA\Tables\Event\TableOwnershipTransferredEvent; use OCA\Tables\Helper\UserHelper; - use OCA\Tables\ResponseDefinitions; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception as OcpDbException; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IL10N; use Psr\Log\LoggerInterface; @@ -44,6 +45,8 @@ class TableService extends SuperService { protected IL10N $l; + protected IEventDispatcher $eventDispatcher; + public function __construct( PermissionsService $permissionsService, LoggerInterface $logger, @@ -56,7 +59,8 @@ public function __construct( ShareService $shareService, UserHelper $userHelper, FavoritesService $favoritesService, - IL10N $l + IL10N $l, + IEventDispatcher $eventDispatcher ) { parent::__construct($logger, $userId, $permissionsService); $this->mapper = $mapper; @@ -68,6 +72,7 @@ public function __construct( $this->userHelper = $userHelper; $this->favoritesService = $favoritesService; $this->l = $l; + $this->eventDispatcher = $eventDispatcher; } /** @@ -343,6 +348,14 @@ public function setOwner(int $id, string $newOwnerUserId, ?string $userId = null throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } + $event = new TableOwnershipTransferredEvent( + table: $table, + toUserId: $newOwnerUserId, + fromUserId: $userId + ); + + $this->eventDispatcher->dispatchTyped($event); + return $table; } @@ -417,6 +430,18 @@ public function delete(int $id, ?string $userId = null): Table { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } + + /* + * Decouple the side logic from the core business logic + * (logging, mail sending, etc.), called handling side effects and most of them can be done asynchronously via queue. + */ + $event = new TableDeletedEvent( + table: $item, + userId: $userId + ); + + $this->eventDispatcher->dispatchTyped($event); + return $item; } diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 4593086a0..ce7cb4e85 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -16,10 +16,12 @@ use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Event\ViewDeletedEvent; use OCA\Tables\Helper\UserHelper; use OCA\Tables\ResponseDefinitions; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IL10N; use Psr\Log\LoggerInterface; @@ -39,6 +41,8 @@ class ViewService extends SuperService { protected IL10N $l; + protected IEventDispatcher $eventDispatcher; + public function __construct( PermissionsService $permissionsService, LoggerInterface $logger, @@ -48,7 +52,8 @@ public function __construct( RowService $rowService, UserHelper $userHelper, FavoritesService $favoritesService, - IL10N $l + IL10N $l, + IEventDispatcher $eventDispatcher ) { parent::__construct($logger, $userId, $permissionsService); $this->l = $l; @@ -57,6 +62,7 @@ public function __construct( $this->rowService = $rowService; $this->userHelper = $userHelper; $this->favoritesService = $favoritesService; + $this->eventDispatcher = $eventDispatcher; } @@ -274,7 +280,16 @@ public function delete(int $id, ?string $userId = null): View { $this->shareService->deleteAllForView($view); try { - return $this->mapper->delete($view); + $deletedView = $this->mapper->delete($view); + + $event = new ViewDeletedEvent( + view: $view, + userId: $userId + ); + + $this->eventDispatcher->dispatchTyped($event); + + return $deletedView; } catch (\OCP\DB\Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); @@ -301,7 +316,15 @@ public function deleteByObject(View $view, ?string $userId = null): View { $this->shareService->deleteAllForView($view); $this->mapper->delete($view); - return $view; + + $event = new ViewDeletedEvent( + view: $view, + userId: $userId + ); + + $this->eventDispatcher->dispatchTyped($event); + + return $view; } catch (Exception $e) { $this->logger->error($e->getMessage()); throw new InternalError($e->getMessage()); From 3b936ff0418ac56e0f5a8afc5e7db3c130cb29d6 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Tue, 2 Apr 2024 20:18:03 +0700 Subject: [PATCH 2/4] Fix linting, psalm Signed-off-by: Hoang Pham --- lib/AppInfo/Application.php | 11 ++--- lib/Db/LegacyRowMapper.php | 14 ++++++- lib/Db/RowCellSuper.php | 5 ++- lib/Event/RowDeletedEvent.php | 26 +++++------- lib/Event/TableDeletedEvent.php | 26 +++++------- lib/Event/TableOwnershipTransferredEvent.php | 33 +++++++-------- lib/Event/ViewDeletedEvent.php | 26 +++++------- .../WhenRowDeletedAuditLogListener.php | 38 +++++++++--------- .../WhenTableDeletedAuditLogListener.php | 36 ++++++++--------- .../WhenTableTransferredAuditLogListener.php | 40 +++++++++---------- .../WhenViewDeletedAuditLogListener.php | 36 ++++++++--------- lib/Service/RowService.php | 33 ++++++++------- .../Support/AuditLogServiceInterface.php | 7 ++-- .../Support/DefaultAuditLogService.php | 19 ++++----- lib/Service/TableService.php | 36 ++++++++--------- lib/Service/ViewService.php | 30 +++++++------- 16 files changed, 204 insertions(+), 212 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 4515e1761..268f8e777 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -29,6 +29,7 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; +use OCP\AppFramework\IAppContainer; use OCP\Collaboration\Reference\RenderReferenceEvent; use OCP\Collaboration\Resources\LoadAdditionalScriptsEvent; use OCP\User\Events\BeforeUserDeletedEvent; @@ -57,17 +58,17 @@ public function register(IRegistrationContext $context): void { throw new Exception('Cannot include autoload. Did you run install dependencies using composer?'); } - $context->registerService(AuditLogServiceInterface::class, fn($c) => $c->query(DefaultAuditLogService::class)); + $context->registerService(AuditLogServiceInterface::class, fn (IAppContainer $c) => $c->query(DefaultAuditLogService::class)); $context->registerEventListener(BeforeUserDeletedEvent::class, UserDeletedListener::class); $context->registerEventListener(DatasourceEvent::class, AnalyticsDatasourceListener::class); $context->registerEventListener(RenderReferenceEvent::class, TablesReferenceListener::class); $context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); $context->registerEventListener(LoadAdditionalScriptsEvent::class, LoadAdditionalListener::class); - $context->registerEventListener(TableDeletedEvent::class, WhenTableDeletedAuditLogListener::class); - $context->registerEventListener(ViewDeletedEvent::class, WhenViewDeletedAuditLogListener::class); - $context->registerEventListener(RowDeletedEvent::class, WhenRowDeletedAuditLogListener::class); - $context->registerEventListener(TableOwnershipTransferredEvent::class, WhenTableTransferredAuditLogListener::class); + $context->registerEventListener(TableDeletedEvent::class, WhenTableDeletedAuditLogListener::class); + $context->registerEventListener(ViewDeletedEvent::class, WhenViewDeletedAuditLogListener::class); + $context->registerEventListener(RowDeletedEvent::class, WhenRowDeletedAuditLogListener::class); + $context->registerEventListener(TableOwnershipTransferredEvent::class, WhenTableTransferredAuditLogListener::class); $context->registerSearchProvider(SearchTablesProvider::class); diff --git a/lib/Db/LegacyRowMapper.php b/lib/Db/LegacyRowMapper.php index c3a4a5db2..e7a164b7f 100644 --- a/lib/Db/LegacyRowMapper.php +++ b/lib/Db/LegacyRowMapper.php @@ -116,7 +116,12 @@ private function getInnerFilterExpressions($qb, $filterGroup, int $groupIndex): return $innerFilterExpressions; } - private function getFilterGroups($qb, $filters): array { + /** + * @param (float|int|string)[][][] $filters + * + * @psalm-param non-empty-list> $filters + */ + private function getFilterGroups(IQueryBuilder $qb, array $filters): array { $filterGroups = []; foreach ($filters as $groupIndex => $filterGroup) { $filterGroups[] = $qb->expr()->andX(...$this->getInnerFilterExpressions($qb, $filterGroup, $groupIndex)); @@ -149,7 +154,12 @@ private function resolveSearchValue(string $unresolvedSearchValue, string $userI } } - private function addOrderByRules(IQueryBuilder $qb, $sortArray) { + /** + * @param (int|string)[][] $sortArray + * + * @psalm-param list $sortArray + */ + private function addOrderByRules(IQueryBuilder $qb, array $sortArray) { foreach ($sortArray as $index => $sortRule) { $sortMode = $sortRule['mode']; if (!in_array($sortMode, ['ASC', 'DESC'])) { diff --git a/lib/Db/RowCellSuper.php b/lib/Db/RowCellSuper.php index 87a232b3e..e2d5962ec 100644 --- a/lib/Db/RowCellSuper.php +++ b/lib/Db/RowCellSuper.php @@ -36,7 +36,10 @@ public function __construct() { $this->addType('rowId', 'integer'); } - public function jsonSerializePreparation($value): array { + /** + * @param float|null|string $value + */ + public function jsonSerializePreparation(string|float|null $value): array { return [ 'id' => $this->id, 'columnId' => $this->columnId, diff --git a/lib/Event/RowDeletedEvent.php b/lib/Event/RowDeletedEvent.php index a3a894b51..e0566ef31 100644 --- a/lib/Event/RowDeletedEvent.php +++ b/lib/Event/RowDeletedEvent.php @@ -7,20 +7,16 @@ use OCA\Tables\Db\Row2; use OCP\EventDispatcher\Event; -final class RowDeletedEvent extends Event -{ - public function __construct(protected Row2 $row, protected string $userId) - { - parent::__construct(); - } +final class RowDeletedEvent extends Event { + public function __construct(protected Row2 $row, protected string $userId) { + parent::__construct(); + } - public function getRow(): Row2 - { - return $this->row; - } + public function getRow(): Row2 { + return $this->row; + } - public function getUserId(): string - { - return $this->userId; - } -} \ No newline at end of file + public function getUserId(): string { + return $this->userId; + } +} diff --git a/lib/Event/TableDeletedEvent.php b/lib/Event/TableDeletedEvent.php index 4acac95df..7b30915cb 100644 --- a/lib/Event/TableDeletedEvent.php +++ b/lib/Event/TableDeletedEvent.php @@ -7,20 +7,16 @@ use OCA\Tables\Db\Table; use OCP\EventDispatcher\Event; -final class TableDeletedEvent extends Event -{ - public function __construct(protected Table $table, protected string $userId) - { - parent::__construct(); - } +final class TableDeletedEvent extends Event { + public function __construct(protected Table $table, protected string $userId) { + parent::__construct(); + } - public function getTable(): Table - { - return $this->table; - } + public function getTable(): Table { + return $this->table; + } - public function getUserId(): string - { - return $this->userId; - } -} \ No newline at end of file + public function getUserId(): string { + return $this->userId; + } +} diff --git a/lib/Event/TableOwnershipTransferredEvent.php b/lib/Event/TableOwnershipTransferredEvent.php index 96bd44c69..182b65541 100644 --- a/lib/Event/TableOwnershipTransferredEvent.php +++ b/lib/Event/TableOwnershipTransferredEvent.php @@ -7,25 +7,20 @@ use OCA\Tables\Db\Table; use OCP\EventDispatcher\Event; -final class TableOwnershipTransferredEvent extends Event -{ - public function __construct(protected Table $table, protected string $toUserId, protected ?string $fromUserId = null) - { - parent::__construct(); - } +final class TableOwnershipTransferredEvent extends Event { + public function __construct(protected Table $table, protected string $toUserId, protected ?string $fromUserId = null) { + parent::__construct(); + } - public function getTable(): Table - { - return $this->table; - } + public function getTable(): Table { + return $this->table; + } - public function getFromUserId(): string - { - return $this->fromUserId; - } + public function getFromUserId(): string|null { + return $this->fromUserId; + } - public function getToUserId(): string - { - return $this->toUserId; - } -} \ No newline at end of file + public function getToUserId(): string { + return $this->toUserId; + } +} diff --git a/lib/Event/ViewDeletedEvent.php b/lib/Event/ViewDeletedEvent.php index b49512053..bcc461024 100644 --- a/lib/Event/ViewDeletedEvent.php +++ b/lib/Event/ViewDeletedEvent.php @@ -7,20 +7,16 @@ use OCA\Tables\Db\View; use OCP\EventDispatcher\Event; -final class ViewDeletedEvent extends Event -{ - public function __construct(protected View $view, protected string $userId) - { - parent::__construct(); - } +final class ViewDeletedEvent extends Event { + public function __construct(protected View $view, protected string $userId) { + parent::__construct(); + } - public function getView(): View - { - return $this->view; - } + public function getView(): View { + return $this->view; + } - public function getUserId(): string - { - return $this->userId; - } -} \ No newline at end of file + public function getUserId(): string { + return $this->userId; + } +} diff --git a/lib/Listener/WhenRowDeletedAuditLogListener.php b/lib/Listener/WhenRowDeletedAuditLogListener.php index 2fba386fb..a176ed927 100644 --- a/lib/Listener/WhenRowDeletedAuditLogListener.php +++ b/lib/Listener/WhenRowDeletedAuditLogListener.php @@ -9,25 +9,25 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -final class WhenRowDeletedAuditLogListener implements IEventListener -{ - public function __construct(protected AuditLogServiceInterface $auditLogService) - { - } +/** + * @template-implements IEventListener + */ +final class WhenRowDeletedAuditLogListener implements IEventListener { + public function __construct(protected AuditLogServiceInterface $auditLogService) { + } - public function handle(Event $event): void - { - if (!($event instanceof RowDeletedEvent)) { - return; - } + public function handle(Event $event): void { + if (!($event instanceof RowDeletedEvent)) { + return; + } - $row = $event->getRow(); - $userId = $event->getUserId(); - $rowId = $row->getId(); + $row = $event->getRow(); + $userId = $event->getUserId(); + $rowId = $row->getId(); - $this->auditLogService->log("Row with ID: $rowId was deleted by user with ID: $userId", [ - 'row' => $row->jsonSerialize(), - 'userId' => $userId, - ]); - } -} \ No newline at end of file + $this->auditLogService->log("Row with ID: $rowId was deleted by user with ID: $userId", [ + 'row' => $row->jsonSerialize(), + 'userId' => $userId, + ]); + } +} diff --git a/lib/Listener/WhenTableDeletedAuditLogListener.php b/lib/Listener/WhenTableDeletedAuditLogListener.php index 5994eaa17..c85b85230 100644 --- a/lib/Listener/WhenTableDeletedAuditLogListener.php +++ b/lib/Listener/WhenTableDeletedAuditLogListener.php @@ -9,24 +9,24 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -final class WhenTableDeletedAuditLogListener implements IEventListener -{ - public function __construct(protected AuditLogServiceInterface $auditLogService) - { - } +/** + * @template-implements IEventListener + */ +final class WhenTableDeletedAuditLogListener implements IEventListener { + public function __construct(protected AuditLogServiceInterface $auditLogService) { + } - public function handle(Event $event): void - { - if (!($event instanceof TableDeletedEvent)) { - return; - } + public function handle(Event $event): void { + if (!($event instanceof TableDeletedEvent)) { + return; + } - $table = $event->getTable(); - $userId = $event->getUserId(); + $table = $event->getTable(); + $userId = $event->getUserId(); - $this->auditLogService->log("Table with ID: $table->id was deleted by user with ID: $userId", [ - 'table' => $table->jsonSerialize(), - 'userId' => $userId, - ]); - } -} \ No newline at end of file + $this->auditLogService->log("Table with ID: $table->id was deleted by user with ID: $userId", [ + 'table' => $table->jsonSerialize(), + 'userId' => $userId, + ]); + } +} diff --git a/lib/Listener/WhenTableTransferredAuditLogListener.php b/lib/Listener/WhenTableTransferredAuditLogListener.php index fb23553a3..b4b90ff98 100644 --- a/lib/Listener/WhenTableTransferredAuditLogListener.php +++ b/lib/Listener/WhenTableTransferredAuditLogListener.php @@ -9,26 +9,26 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -final class WhenTableTransferredAuditLogListener implements IEventListener -{ - public function __construct(protected AuditLogServiceInterface $auditLogService) - { - } +/** + * @template-implements IEventListener + */ +final class WhenTableTransferredAuditLogListener implements IEventListener { + public function __construct(protected AuditLogServiceInterface $auditLogService) { + } - public function handle(Event $event): void - { - if (!($event instanceof TableOwnershipTransferredEvent)) { - return; - } + public function handle(Event $event): void { + if (!($event instanceof TableOwnershipTransferredEvent)) { + return; + } - $table = $event->getTable(); - $fromUserId = $event->getFromUserId(); - $toUserId = $event->getToUserId(); + $table = $event->getTable(); + $fromUserId = $event->getFromUserId(); + $toUserId = $event->getToUserId(); - $this->auditLogService->log("Table with ID: $table->id was transferred from user with ID: $fromUserId to user with ID: $toUserId", [ - 'table' => $table->jsonSerialize(), - 'fromUserId' => $fromUserId, - 'toUserId' => $toUserId, - ]); - } -} \ No newline at end of file + $this->auditLogService->log("Table with ID: $table->id was transferred from user with ID: $fromUserId to user with ID: $toUserId", [ + 'table' => $table->jsonSerialize(), + 'fromUserId' => $fromUserId, + 'toUserId' => $toUserId, + ]); + } +} diff --git a/lib/Listener/WhenViewDeletedAuditLogListener.php b/lib/Listener/WhenViewDeletedAuditLogListener.php index 8aad0df7b..f89a69244 100644 --- a/lib/Listener/WhenViewDeletedAuditLogListener.php +++ b/lib/Listener/WhenViewDeletedAuditLogListener.php @@ -9,24 +9,24 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -final class WhenViewDeletedAuditLogListener implements IEventListener -{ - public function __construct(protected AuditLogServiceInterface $auditLogService) - { - } +/** + * @template-implements IEventListener + */ +final class WhenViewDeletedAuditLogListener implements IEventListener { + public function __construct(protected AuditLogServiceInterface $auditLogService) { + } - public function handle(Event $event): void - { - if (!($event instanceof ViewDeletedEvent)) { - return; - } + public function handle(Event $event): void { + if (!($event instanceof ViewDeletedEvent)) { + return; + } - $view = $event->getView(); - $userId = $event->getUserId(); + $view = $event->getView(); + $userId = $event->getUserId(); - $this->auditLogService->log("View with ID: $view->id was deleted by user with ID: $userId", [ - 'view' => $view->jsonSerialize(), - 'userId' => $userId, - ]); - } -} \ No newline at end of file + $this->auditLogService->log("View with ID: $view->id was deleted by user with ID: $userId", [ + 'view' => $view->jsonSerialize(), + 'userId' => $userId, + ]); + } +} diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index b399bc93f..a88a7f2ca 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -35,25 +35,24 @@ class RowService extends SuperService { private Row2Mapper $row2Mapper; private array $tmpRows = []; // holds already loaded rows as a small cache - protected IEventDispatcher $eventDispatcher; + protected IEventDispatcher $eventDispatcher; public function __construct( - PermissionsService $permissionsService, - LoggerInterface $logger, - ?string $userId, + PermissionsService $permissionsService, + LoggerInterface $logger, + ?string $userId, ColumnMapper $columnMapper, - ViewMapper $viewMapper, - TableMapper $tableMapper, - Row2Mapper $row2Mapper, - IEventDispatcher $eventDispatcher - ) - { + ViewMapper $viewMapper, + TableMapper $tableMapper, + Row2Mapper $row2Mapper, + IEventDispatcher $eventDispatcher + ) { parent::__construct($logger, $userId, $permissionsService); $this->columnMapper = $columnMapper; $this->viewMapper = $viewMapper; $this->tableMapper = $tableMapper; $this->row2Mapper = $row2Mapper; - $this->eventDispatcher = $eventDispatcher; + $this->eventDispatcher = $eventDispatcher; } /** @@ -464,14 +463,14 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { } try { - $deletedRow = $this->row2Mapper->delete($item); + $deletedRow = $this->row2Mapper->delete($item); - $event = new RowDeletedEvent( - row: $item, - userId: $userId - ); + $event = new RowDeletedEvent( + row: $item, + userId: $userId + ); - $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped($event); return $this->filterRowResult($view ?? null, $deletedRow); } catch (Exception $e) { diff --git a/lib/Service/Support/AuditLogServiceInterface.php b/lib/Service/Support/AuditLogServiceInterface.php index ec761f1de..93eeaa962 100644 --- a/lib/Service/Support/AuditLogServiceInterface.php +++ b/lib/Service/Support/AuditLogServiceInterface.php @@ -4,7 +4,6 @@ namespace OCA\Tables\Service\Support; -interface AuditLogServiceInterface -{ - public function log(string $message, array $context): void; -} \ No newline at end of file +interface AuditLogServiceInterface { + public function log(string $message, array $context): void; +} diff --git a/lib/Service/Support/DefaultAuditLogService.php b/lib/Service/Support/DefaultAuditLogService.php index 686cbef74..e4a523719 100644 --- a/lib/Service/Support/DefaultAuditLogService.php +++ b/lib/Service/Support/DefaultAuditLogService.php @@ -7,16 +7,13 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Log\Audit\CriticalActionPerformedEvent; -final class DefaultAuditLogService implements AuditLogServiceInterface -{ - public function __construct(private IEventDispatcher $eventDispatcher) - { - } +final class DefaultAuditLogService implements AuditLogServiceInterface { + public function __construct(private IEventDispatcher $eventDispatcher) { + } - public function log(string $message, array $context): void - { - $auditEvent = new CriticalActionPerformedEvent($message, $context); + public function log(string $message, array $context): void { + $auditEvent = new CriticalActionPerformedEvent($message, $context); - $this->eventDispatcher->dispatchTyped($auditEvent); - } -} \ No newline at end of file + $this->eventDispatcher->dispatchTyped($auditEvent); + } +} diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index c6b3725e5..796390df0 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -45,7 +45,7 @@ class TableService extends SuperService { protected IL10N $l; - protected IEventDispatcher $eventDispatcher; + protected IEventDispatcher $eventDispatcher; public function __construct( PermissionsService $permissionsService, @@ -60,7 +60,7 @@ public function __construct( UserHelper $userHelper, FavoritesService $favoritesService, IL10N $l, - IEventDispatcher $eventDispatcher + IEventDispatcher $eventDispatcher ) { parent::__construct($logger, $userId, $permissionsService); $this->mapper = $mapper; @@ -72,7 +72,7 @@ public function __construct( $this->userHelper = $userHelper; $this->favoritesService = $favoritesService; $this->l = $l; - $this->eventDispatcher = $eventDispatcher; + $this->eventDispatcher = $eventDispatcher; } /** @@ -348,13 +348,13 @@ public function setOwner(int $id, string $newOwnerUserId, ?string $userId = null throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } - $event = new TableOwnershipTransferredEvent( - table: $table, - toUserId: $newOwnerUserId, - fromUserId: $userId - ); + $event = new TableOwnershipTransferredEvent( + table: $table, + toUserId: $newOwnerUserId, + fromUserId: $userId + ); - $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped($event); return $table; } @@ -431,16 +431,16 @@ public function delete(int $id, ?string $userId = null): Table { throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } - /* - * Decouple the side logic from the core business logic - * (logging, mail sending, etc.), called handling side effects and most of them can be done asynchronously via queue. - */ - $event = new TableDeletedEvent( - table: $item, - userId: $userId - ); + /* + * Decouple the side logic from the core business logic + * (logging, mail sending, etc.), called handling side effects and most of them can be done asynchronously via queue. + */ + $event = new TableDeletedEvent( + table: $item, + userId: $userId + ); - $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped($event); return $item; } diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index ce7cb4e85..ac8845881 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -41,7 +41,7 @@ class ViewService extends SuperService { protected IL10N $l; - protected IEventDispatcher $eventDispatcher; + protected IEventDispatcher $eventDispatcher; public function __construct( PermissionsService $permissionsService, @@ -53,7 +53,7 @@ public function __construct( UserHelper $userHelper, FavoritesService $favoritesService, IL10N $l, - IEventDispatcher $eventDispatcher + IEventDispatcher $eventDispatcher ) { parent::__construct($logger, $userId, $permissionsService); $this->l = $l; @@ -62,7 +62,7 @@ public function __construct( $this->rowService = $rowService; $this->userHelper = $userHelper; $this->favoritesService = $favoritesService; - $this->eventDispatcher = $eventDispatcher; + $this->eventDispatcher = $eventDispatcher; } @@ -282,14 +282,14 @@ public function delete(int $id, ?string $userId = null): View { try { $deletedView = $this->mapper->delete($view); - $event = new ViewDeletedEvent( - view: $view, - userId: $userId - ); + $event = new ViewDeletedEvent( + view: $view, + userId: $userId + ); - $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped($event); - return $deletedView; + return $deletedView; } catch (\OCP\DB\Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); @@ -317,14 +317,14 @@ public function deleteByObject(View $view, ?string $userId = null): View { $this->mapper->delete($view); - $event = new ViewDeletedEvent( - view: $view, - userId: $userId - ); + $event = new ViewDeletedEvent( + view: $view, + userId: $userId + ); - $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped($event); - return $view; + return $view; } catch (Exception $e) { $this->logger->error($e->getMessage()); throw new InternalError($e->getMessage()); From ae59bc279ab4f9e213481bfc0c42b176c9af1df6 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Tue, 2 Apr 2024 20:50:33 +0700 Subject: [PATCH 3/4] Add unit tests Signed-off-by: Hoang Pham --- .../Listener/WhenRowDeletedAuditLogTest.php | 42 ++++++++++++++++++ .../Listener/WhenTableDeletedAuditLogTest.php | 42 ++++++++++++++++++ .../WhenTableOwnerTransferredAuditLogTest.php | 44 +++++++++++++++++++ .../Listener/WhenViewDeletedAuditLogTest.php | 42 ++++++++++++++++++ .../Support/DefaultAuditLogServiceTest.php | 34 ++++++++++++++ 5 files changed, 204 insertions(+) create mode 100644 tests/unit/Listener/WhenRowDeletedAuditLogTest.php create mode 100644 tests/unit/Listener/WhenTableDeletedAuditLogTest.php create mode 100644 tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php create mode 100644 tests/unit/Listener/WhenViewDeletedAuditLogTest.php create mode 100644 tests/unit/Service/Support/DefaultAuditLogServiceTest.php diff --git a/tests/unit/Listener/WhenRowDeletedAuditLogTest.php b/tests/unit/Listener/WhenRowDeletedAuditLogTest.php new file mode 100644 index 000000000..56efe9d1b --- /dev/null +++ b/tests/unit/Listener/WhenRowDeletedAuditLogTest.php @@ -0,0 +1,42 @@ +auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->listener = new WhenRowDeletedAuditLogListener($this->auditLogService); + } + + public function testHandle(): void { + $row = new Row2(); + $row->setId(1); + $userId = 'user1'; + + $event = new RowDeletedEvent($row, $userId); + + $this->auditLogService + ->expects($this->once()) + ->method('log') + ->with( + $this->equalTo("Row with ID: {$row->getId()} was deleted by user with ID: $userId"), + $this->equalTo([ + 'row' => $row->jsonSerialize(), + 'userId' => $userId, + ]) + ); + + $this->listener->handle($event); + } +} diff --git a/tests/unit/Listener/WhenTableDeletedAuditLogTest.php b/tests/unit/Listener/WhenTableDeletedAuditLogTest.php new file mode 100644 index 000000000..5fa2f7dd2 --- /dev/null +++ b/tests/unit/Listener/WhenTableDeletedAuditLogTest.php @@ -0,0 +1,42 @@ +auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->listener = new WhenTableDeletedAuditLogListener($this->auditLogService); + } + + public function testHandle(): void { + $table = new Table(); + $table->id = 1; + $userId = 'user1'; + + $event = new TableDeletedEvent($table, $userId); + + $this->auditLogService + ->expects($this->once()) + ->method('log') + ->with( + $this->equalTo("Table with ID: {$table->id} was deleted by user with ID: $userId"), + $this->equalTo([ + 'table' => $table->jsonSerialize(), + 'userId' => $userId, + ]) + ); + + $this->listener->handle($event); + } +} diff --git a/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php b/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php new file mode 100644 index 000000000..ae9f956e1 --- /dev/null +++ b/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php @@ -0,0 +1,44 @@ +auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->listener = new WhenTableTransferredAuditLogListener($this->auditLogService); + } + + public function testHandle(): void { + $table = new Table(); + $table->id = 1; + $fromUserId = 'user1'; + $toUserId = 'user2'; + + $event = new TableOwnershipTransferredEvent($table, $toUserId, $fromUserId); + + $this->auditLogService + ->expects($this->once()) + ->method('log') + ->with( + $this->equalTo("Table with ID: {$table->id} was transferred from user with ID: $fromUserId to user with ID: $toUserId"), + $this->equalTo([ + 'table' => $table->jsonSerialize(), + 'fromUserId' => $fromUserId, + 'toUserId' => $toUserId, + ]) + ); + + $this->listener->handle($event); + } +} diff --git a/tests/unit/Listener/WhenViewDeletedAuditLogTest.php b/tests/unit/Listener/WhenViewDeletedAuditLogTest.php new file mode 100644 index 000000000..e7e0edd07 --- /dev/null +++ b/tests/unit/Listener/WhenViewDeletedAuditLogTest.php @@ -0,0 +1,42 @@ +auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->listener = new WhenViewDeletedAuditLogListener($this->auditLogService); + } + + public function testHandle(): void { + $view = new View(); + $view->id = 1; + $userId = 'user1'; + + $event = new ViewDeletedEvent($view, $userId); + + $this->auditLogService + ->expects($this->once()) + ->method('log') + ->with( + $this->equalTo("View with ID: {$view->id} was deleted by user with ID: $userId"), + $this->equalTo([ + 'view' => $view->jsonSerialize(), + 'userId' => $userId, + ]) + ); + + $this->listener->handle($event); + } +} diff --git a/tests/unit/Service/Support/DefaultAuditLogServiceTest.php b/tests/unit/Service/Support/DefaultAuditLogServiceTest.php new file mode 100644 index 000000000..152b99fe5 --- /dev/null +++ b/tests/unit/Service/Support/DefaultAuditLogServiceTest.php @@ -0,0 +1,34 @@ +eventDispatcher = $this->createMock(IEventDispatcher::class); + + $this->service = new DefaultAuditLogService($this->eventDispatcher); + } + + public function testLog(): void { + $message = 'Test message'; + $context = ['key' => 'value']; + + $this->eventDispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event) use ($message, $context) { + return $event->getLogMessage() === $message && $event->getParameters() === $context; + })); + + $this->service->log($message, $context); + } +} From f82b6df8abb4abb7e8900d4e94bf069d02088bd2 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Thu, 11 Apr 2024 17:35:10 +0700 Subject: [PATCH 4/4] Remove unneccessary user id Signed-off-by: Hoang Pham --- lib/Db/LegacyRowMapper.php | 2 +- lib/Event/RowDeletedEvent.php | 6 +----- lib/Event/TableDeletedEvent.php | 6 +----- lib/Event/ViewDeletedEvent.php | 6 +----- lib/Listener/WhenRowDeletedAuditLogListener.php | 4 +--- lib/Listener/WhenTableDeletedAuditLogListener.php | 4 +--- lib/Listener/WhenViewDeletedAuditLogListener.php | 4 +--- lib/Service/RowService.php | 5 +---- lib/Service/TableService.php | 9 +-------- lib/Service/ViewService.php | 10 ++-------- tests/unit/Listener/WhenRowDeletedAuditLogTest.php | 6 ++---- tests/unit/Listener/WhenTableDeletedAuditLogTest.php | 6 ++---- tests/unit/Listener/WhenViewDeletedAuditLogTest.php | 6 ++---- 13 files changed, 17 insertions(+), 57 deletions(-) diff --git a/lib/Db/LegacyRowMapper.php b/lib/Db/LegacyRowMapper.php index e7a164b7f..ae7b3996b 100644 --- a/lib/Db/LegacyRowMapper.php +++ b/lib/Db/LegacyRowMapper.php @@ -157,7 +157,7 @@ private function resolveSearchValue(string $unresolvedSearchValue, string $userI /** * @param (int|string)[][] $sortArray * - * @psalm-param list $sortArray + * @psalm-param list $sortArray */ private function addOrderByRules(IQueryBuilder $qb, array $sortArray) { foreach ($sortArray as $index => $sortRule) { diff --git a/lib/Event/RowDeletedEvent.php b/lib/Event/RowDeletedEvent.php index e0566ef31..afcfaa715 100644 --- a/lib/Event/RowDeletedEvent.php +++ b/lib/Event/RowDeletedEvent.php @@ -8,15 +8,11 @@ use OCP\EventDispatcher\Event; final class RowDeletedEvent extends Event { - public function __construct(protected Row2 $row, protected string $userId) { + public function __construct(protected Row2 $row) { parent::__construct(); } public function getRow(): Row2 { return $this->row; } - - public function getUserId(): string { - return $this->userId; - } } diff --git a/lib/Event/TableDeletedEvent.php b/lib/Event/TableDeletedEvent.php index 7b30915cb..b40ed1372 100644 --- a/lib/Event/TableDeletedEvent.php +++ b/lib/Event/TableDeletedEvent.php @@ -8,15 +8,11 @@ use OCP\EventDispatcher\Event; final class TableDeletedEvent extends Event { - public function __construct(protected Table $table, protected string $userId) { + public function __construct(protected Table $table) { parent::__construct(); } public function getTable(): Table { return $this->table; } - - public function getUserId(): string { - return $this->userId; - } } diff --git a/lib/Event/ViewDeletedEvent.php b/lib/Event/ViewDeletedEvent.php index bcc461024..563370a19 100644 --- a/lib/Event/ViewDeletedEvent.php +++ b/lib/Event/ViewDeletedEvent.php @@ -8,15 +8,11 @@ use OCP\EventDispatcher\Event; final class ViewDeletedEvent extends Event { - public function __construct(protected View $view, protected string $userId) { + public function __construct(protected View $view) { parent::__construct(); } public function getView(): View { return $this->view; } - - public function getUserId(): string { - return $this->userId; - } } diff --git a/lib/Listener/WhenRowDeletedAuditLogListener.php b/lib/Listener/WhenRowDeletedAuditLogListener.php index a176ed927..b9ca7bde5 100644 --- a/lib/Listener/WhenRowDeletedAuditLogListener.php +++ b/lib/Listener/WhenRowDeletedAuditLogListener.php @@ -22,12 +22,10 @@ public function handle(Event $event): void { } $row = $event->getRow(); - $userId = $event->getUserId(); $rowId = $row->getId(); - $this->auditLogService->log("Row with ID: $rowId was deleted by user with ID: $userId", [ + $this->auditLogService->log("Row with ID: $rowId was deleted", [ 'row' => $row->jsonSerialize(), - 'userId' => $userId, ]); } } diff --git a/lib/Listener/WhenTableDeletedAuditLogListener.php b/lib/Listener/WhenTableDeletedAuditLogListener.php index c85b85230..aa3a1a548 100644 --- a/lib/Listener/WhenTableDeletedAuditLogListener.php +++ b/lib/Listener/WhenTableDeletedAuditLogListener.php @@ -22,11 +22,9 @@ public function handle(Event $event): void { } $table = $event->getTable(); - $userId = $event->getUserId(); - $this->auditLogService->log("Table with ID: $table->id was deleted by user with ID: $userId", [ + $this->auditLogService->log("Table with ID: $table->id was deleted", [ 'table' => $table->jsonSerialize(), - 'userId' => $userId, ]); } } diff --git a/lib/Listener/WhenViewDeletedAuditLogListener.php b/lib/Listener/WhenViewDeletedAuditLogListener.php index f89a69244..755cf07d7 100644 --- a/lib/Listener/WhenViewDeletedAuditLogListener.php +++ b/lib/Listener/WhenViewDeletedAuditLogListener.php @@ -22,11 +22,9 @@ public function handle(Event $event): void { } $view = $event->getView(); - $userId = $event->getUserId(); - $this->auditLogService->log("View with ID: $view->id was deleted by user with ID: $userId", [ + $this->auditLogService->log("View with ID: $view->id was deleted", [ 'view' => $view->jsonSerialize(), - 'userId' => $userId, ]); } } diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index a88a7f2ca..fda2ed6bb 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -465,10 +465,7 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { try { $deletedRow = $this->row2Mapper->delete($item); - $event = new RowDeletedEvent( - row: $item, - userId: $userId - ); + $event = new RowDeletedEvent(row: $item); $this->eventDispatcher->dispatchTyped($event); diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index 796390df0..100b380f0 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -431,14 +431,7 @@ public function delete(int $id, ?string $userId = null): Table { throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } - /* - * Decouple the side logic from the core business logic - * (logging, mail sending, etc.), called handling side effects and most of them can be done asynchronously via queue. - */ - $event = new TableDeletedEvent( - table: $item, - userId: $userId - ); + $event = new TableDeletedEvent(table: $item); $this->eventDispatcher->dispatchTyped($event); diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index ac8845881..6d79baaf4 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -282,10 +282,7 @@ public function delete(int $id, ?string $userId = null): View { try { $deletedView = $this->mapper->delete($view); - $event = new ViewDeletedEvent( - view: $view, - userId: $userId - ); + $event = new ViewDeletedEvent(view: $view); $this->eventDispatcher->dispatchTyped($event); @@ -317,10 +314,7 @@ public function deleteByObject(View $view, ?string $userId = null): View { $this->mapper->delete($view); - $event = new ViewDeletedEvent( - view: $view, - userId: $userId - ); + $event = new ViewDeletedEvent(view: $view); $this->eventDispatcher->dispatchTyped($event); diff --git a/tests/unit/Listener/WhenRowDeletedAuditLogTest.php b/tests/unit/Listener/WhenRowDeletedAuditLogTest.php index 56efe9d1b..7afa7726a 100644 --- a/tests/unit/Listener/WhenRowDeletedAuditLogTest.php +++ b/tests/unit/Listener/WhenRowDeletedAuditLogTest.php @@ -22,18 +22,16 @@ protected function setUp(): void { public function testHandle(): void { $row = new Row2(); $row->setId(1); - $userId = 'user1'; - $event = new RowDeletedEvent($row, $userId); + $event = new RowDeletedEvent(row: $row); $this->auditLogService ->expects($this->once()) ->method('log') ->with( - $this->equalTo("Row with ID: {$row->getId()} was deleted by user with ID: $userId"), + $this->equalTo("Row with ID: {$row->getId()} was deleted"), $this->equalTo([ 'row' => $row->jsonSerialize(), - 'userId' => $userId, ]) ); diff --git a/tests/unit/Listener/WhenTableDeletedAuditLogTest.php b/tests/unit/Listener/WhenTableDeletedAuditLogTest.php index 5fa2f7dd2..643a0d10e 100644 --- a/tests/unit/Listener/WhenTableDeletedAuditLogTest.php +++ b/tests/unit/Listener/WhenTableDeletedAuditLogTest.php @@ -22,18 +22,16 @@ protected function setUp(): void { public function testHandle(): void { $table = new Table(); $table->id = 1; - $userId = 'user1'; - $event = new TableDeletedEvent($table, $userId); + $event = new TableDeletedEvent($table); $this->auditLogService ->expects($this->once()) ->method('log') ->with( - $this->equalTo("Table with ID: {$table->id} was deleted by user with ID: $userId"), + $this->equalTo("Table with ID: {$table->id} was deleted"), $this->equalTo([ 'table' => $table->jsonSerialize(), - 'userId' => $userId, ]) ); diff --git a/tests/unit/Listener/WhenViewDeletedAuditLogTest.php b/tests/unit/Listener/WhenViewDeletedAuditLogTest.php index e7e0edd07..d15be7352 100644 --- a/tests/unit/Listener/WhenViewDeletedAuditLogTest.php +++ b/tests/unit/Listener/WhenViewDeletedAuditLogTest.php @@ -22,18 +22,16 @@ protected function setUp(): void { public function testHandle(): void { $view = new View(); $view->id = 1; - $userId = 'user1'; - $event = new ViewDeletedEvent($view, $userId); + $event = new ViewDeletedEvent(view: $view); $this->auditLogService ->expects($this->once()) ->method('log') ->with( - $this->equalTo("View with ID: {$view->id} was deleted by user with ID: $userId"), + $this->equalTo("View with ID: {$view->id} was deleted"), $this->equalTo([ 'view' => $view->jsonSerialize(), - 'userId' => $userId, ]) );