Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions lib/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,18 @@ public function getCount(INotification $notification): int {
* @since 8.2.0
*/
public function markProcessed(INotification $notification): void {
$deleted = $this->handler->delete($notification);
$notifications = $this->handler->delete($notification);

$isAlreadyDeferring = $this->push->isDeferring();
if (!$isAlreadyDeferring) {
$this->push->deferPayloads();
}
foreach ($deleted as $user => $notifications) {
foreach ($notifications as $data) {
$this->push->pushDeleteToDevice((string)$user, [$data['id']], $data['app']);
}
foreach ($notifications as $notificationId => $notification) {
$this->push->pushDeleteToDevice(
$notification->getUser(),
[ $notificationId ],
$notification->getApp()
);
}
if (!$isAlreadyDeferring) {
$this->push->flushPayloads();
Expand Down
20 changes: 8 additions & 12 deletions lib/Controller/EndpointController.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,9 @@ public function deleteNotification(int $id): DataResponse {

try {
$notification = $this->handler->getById($id, $this->getCurrentUser());
$deleted = $this->handler->deleteById($id, $this->getCurrentUser(), $notification);

if ($deleted) {
$this->push->pushDeleteToDevice($this->getCurrentUser(), [$id], $notification->getApp());
}
// markProcessed() does the actual deletion
$this->manager->markProcessed($notification);
} catch (NotificationNotFoundException) {
}

Expand All @@ -251,15 +249,13 @@ public function deleteAllNotifications(): DataResponse {
return new DataResponse(null, Http::STATUS_FORBIDDEN);
}

$shouldFlush = $this->manager->defer();

$deletedSomething = $this->handler->deleteByUser($this->getCurrentUser());
if ($deletedSomething) {
$this->push->pushDeleteToDevice($this->getCurrentUser(), null);
}
try {
$notification = $this->manager->createNotification();
$notification->setUser($this->getCurrentUser());

if ($shouldFlush) {
$this->manager->flush();
// markProcessed() does the actual deletion
$this->manager->markProcessed($notification);
} catch (NotificationNotFoundException) {
}

return new DataResponse();
Expand Down
81 changes: 12 additions & 69 deletions lib/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,28 +59,7 @@ public function count(INotification $notification): int {
* @return array A Map with all deleted notifications [user => [notifications]]
*/
public function delete(INotification $notification): array {
$sql = $this->connection->getQueryBuilder();
$sql->select('*')
->from('notifications');

$this->sqlWhere($sql, $notification);
$statement = $sql->executeQuery();

$deleted = [];
$notifications = [];
while ($row = $statement->fetch()) {
if (!isset($deleted[$row['user']])) {
$deleted[$row['user']] = [];
}

$deleted[$row['user']][] = [
'id' => (int)$row['notification_id'],
'app' => $row['app'],
];
$notifications[(int)$row['notification_id']] = $this->notificationFromRow($row);
}
$statement->closeCursor();

$notifications = $this->get($notification, PHP_INT_MAX);
if (count($notifications) === 0) {
return [];
}
Expand All @@ -93,9 +72,17 @@ public function delete(INotification $notification): array {
$this->manager->dismissNotification($n);
}

// delete notifications from db in chunks
$notificationIds = array_keys($notifications);
foreach (array_chunk($notificationIds, 1000) as $chunk) {
$this->deleteIds($chunk);
foreach (array_chunk($notificationIds, 1000) as $ids) {
$sql = $this->connection->getQueryBuilder();
$sql->delete('notifications')
->where($sql->expr()->in(
'notification_id',
$sql->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)
)
);
$sql->executeStatement();
}

if ($shouldFlush) {
Expand All @@ -107,51 +94,7 @@ public function delete(INotification $notification): array {
}
$this->connection->commit();

return $deleted;
}

/**
* Delete the notification of a given user
*/
public function deleteByUser(string $user): bool {
$notification = $this->manager->createNotification();
try {
$notification->setUser($user);
} catch (\InvalidArgumentException) {
return false;
}
return !empty($this->delete($notification));
}

/**
* Delete the notification matching the given id
*
* @throws NotificationNotFoundException
*/
public function deleteById(int $id, string $user, ?INotification $notification = null): bool {
if (!$notification instanceof INotification) {
$notification = $this->getById($id, $user);
}

$this->manager->dismissNotification($notification);

$sql = $this->connection->getQueryBuilder();
$sql->delete('notifications')
->where($sql->expr()->eq('notification_id', $sql->createNamedParameter($id)))
->andWhere($sql->expr()->eq('user', $sql->createNamedParameter($user)));
return (bool)$sql->executeStatement();
}

/**
* Delete the notification matching the given ids
*
* @param int[] $ids
*/
public function deleteIds(array $ids): void {
$sql = $this->connection->getQueryBuilder();
$sql->delete('notifications')
->where($sql->expr()->in('notification_id', $sql->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
$sql->executeStatement();
return $notifications;
}

/**
Expand Down
10 changes: 7 additions & 3 deletions lib/Listener/UserDeletedListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace OCA\Notifications\Listener;

use OCA\Notifications\Handler;
use OCP\Notification\IManager as INotificationManager;
use OCA\Notifications\Model\SettingsMapper;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
Expand All @@ -20,7 +20,7 @@
*/
class UserDeletedListener implements IEventListener {
public function __construct(
private Handler $handler,
private INotificationManager $notificationManager,
private SettingsMapper $settingsMapper,
) {
}
Expand All @@ -32,7 +32,11 @@ public function handle(Event $event): void {
}

$user = $event->getUser();
$this->handler->deleteByUser($user->getUID());

$notification = $this->notificationManager->createNotification();
$notification->setUser($user->getUID());
$this->notificationManager->markProcessed($notification);

$this->settingsMapper->deleteSettingsByUser($user->getUID());
}
}
46 changes: 0 additions & 46 deletions tests/Unit/Controller/EndpointControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -376,52 +376,6 @@ public static function dataDeleteNotification(): array {
];
}

/**
* @dataProvider dataDeleteNotification
*/
public function testDeleteNotification(int $id, string $username): void {
$controller = $this->getController([], $username);

$this->handler->expects($this->once())
->method('deleteById')
->with($id, $username);

$response = $controller->deleteNotification($id);
$this->assertInstanceOf(DataResponse::class, $response);
$this->assertSame(Http::STATUS_OK, $response->getStatus());
}

public function testDeleteNotificationNoId(): void {
$controller = $this->getController();

$this->handler->expects($this->never())
->method('deleteById');

$response = $controller->deleteNotification(0);
$this->assertInstanceOf(DataResponse::class, $response);
$this->assertSame(Http::STATUS_NOT_FOUND, $response->getStatus());
}

/**
* @dataProvider dataDeleteNotification
*/
public function testDeleteAllNotifications(int $_, string $username): void {
$controller = $this->getController([], $username);

$this->handler->expects($this->once())
->method('deleteByUser')
->with($username);
$this->manager->expects($this->once())
->method('defer')
->willReturn(true);
$this->manager->expects($this->once())
->method('flush');

$response = $controller->deleteAllNotifications();
$this->assertInstanceOf(DataResponse::class, $response);
$this->assertSame(Http::STATUS_OK, $response->getStatus());
}

public static function dataNotificationToArray(): array {
return [
['v1', 42, 'app1', 'user1', 1234, 'type1', '42', 'subject1', '', [], 'message1', 'richMessage 1', ['richMessage param'], 'link1', 'icon1', 0, []],
Expand Down
62 changes: 0 additions & 62 deletions tests/Unit/HandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,68 +153,6 @@ public function testFullEmptyMessageForOracle(): void {
$this->assertSame(0, $this->handler->count($limitedNotification2), 'Wrong notification count for user2 after deleting');
}

public function testDeleteById(): void {
$notification = $this->getNotification([
'getApp' => 'testing_notifications',
'getUser' => 'test_user1',
'getDateTime' => new \DateTime(),
'getObjectType' => 'notification',
'getObjectId' => '1337',
'getSubject' => 'subject',
'getSubjectParameters' => [],
'getMessage' => 'message',
'getMessageParameters' => [],
'getLink' => 'link',
'getIcon' => 'icon',
'getActions' => [
[
'getLabel' => 'action_label',
'getLink' => 'action_link',
'getRequestType' => 'GET',
'isPrimary' => true,
]
],
]);
$limitedNotification = $this->getNotification([
'getApp' => 'testing_notifications',
'getUser' => 'test_user1',
]);

// Make sure there is no notification
$this->assertSame(0, $this->handler->count($limitedNotification));
$notifications = $this->handler->get($limitedNotification);
$this->assertCount(0, $notifications);

// Add and count
$this->handler->add($notification);
$this->assertSame(1, $this->handler->count($limitedNotification));

// Get and count
$notifications = $this->handler->get($limitedNotification);
$this->assertCount(1, $notifications);
$notificationId = array_key_first($notifications);

// Get with wrong user
try {
$this->handler->getById($notificationId, 'test_user2');
$this->fail('Exception of type NotificationNotFoundException expected');
} catch (\Exception $e) {
$this->assertInstanceOf(NotificationNotFoundException::class, $e);
}

// Delete with wrong user
$this->handler->deleteById($notificationId, 'test_user2', $notification);
$this->assertSame(1, $this->handler->count($limitedNotification), 'Wrong notification count for user1 after trying to delete for user2');

// Get with correct user
$getNotification = $this->handler->getById($notificationId, 'test_user1');
$this->assertInstanceOf(INotification::class, $getNotification);

// Delete and count
$this->handler->deleteById($notificationId, 'test_user1');
$this->assertSame(0, $this->handler->count($limitedNotification), 'Wrong notification count for user1 after deleting');
}

protected function getNotification(array $values = []): INotification&MockObject {
$notification = $this->getMockBuilder(INotification::class)
->getMock();
Expand Down