Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
perf(files_reminders): Reduce db queries on propfind
Signed-off-by: Christopher Ng <chrng8@gmail.com>
  • Loading branch information
Pytal committed Feb 21, 2025
commit 802886d592547a66028b842cfb27662d879b639e
19 changes: 9 additions & 10 deletions apps/files_reminders/lib/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
use DateTimeZone;
use Exception;
use OCA\FilesReminders\Exception\NodeNotFoundException;
use OCA\FilesReminders\Exception\ReminderNotFoundException;
use OCA\FilesReminders\Service\ReminderService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\DataResponse;
Expand Down Expand Up @@ -53,15 +53,14 @@ public function get(int $fileId): DataResponse {

try {
$reminder = $this->reminderService->getDueForUser($user, $fileId);
$reminderData = [
if ($reminder === null) {
return new DataResponse(['dueDate' => null], Http::STATUS_OK);
}
return new DataResponse([
'dueDate' => $reminder->getDueDate()->format(DateTimeInterface::ATOM), // ISO 8601
];
return new DataResponse($reminderData, Http::STATUS_OK);
} catch (NodeNotFoundException|DoesNotExistException $e) {
$reminderData = [
'dueDate' => null,
];
return new DataResponse($reminderData, Http::STATUS_OK);
], Http::STATUS_OK);
} catch (NodeNotFoundException $e) {
return new DataResponse(['dueDate' => null], Http::STATUS_OK);
}
}

Expand Down Expand Up @@ -125,7 +124,7 @@ public function remove(int $fileId): DataResponse {
try {
$this->reminderService->remove($user, $fileId);
return new DataResponse([], Http::STATUS_OK);
} catch (NodeNotFoundException|DoesNotExistException $e) {
} catch (NodeNotFoundException|ReminderNotFoundException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}
}
Expand Down
6 changes: 2 additions & 4 deletions apps/files_reminders/lib/Dav/PropFindPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use DateTimeInterface;
use OCA\DAV\Connector\Sabre\Node;
use OCA\FilesReminders\Service\ReminderService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IUser;
use OCP\IUserSession;
use Sabre\DAV\INode;
Expand Down Expand Up @@ -52,9 +51,8 @@ function () use ($node) {
}

$fileId = $node->getId();
try {
$reminder = $this->reminderService->getDueForUser($user, $fileId);
} catch (DoesNotExistException $e) {
$reminder = $this->reminderService->getDueForUser($user, $fileId);
if ($reminder === null) {
return '';
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\FilesReminders\Exception;

use Exception;

class ReminderNotFoundException extends Exception {
}
68 changes: 50 additions & 18 deletions apps/files_reminders/lib/Service/ReminderService.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
use OCA\FilesReminders\Db\Reminder;
use OCA\FilesReminders\Db\ReminderMapper;
use OCA\FilesReminders\Exception\NodeNotFoundException;
use OCA\FilesReminders\Exception\ReminderNotFoundException;
use OCA\FilesReminders\Exception\UserNotFoundException;
use OCA\FilesReminders\Model\RichReminder;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
Expand All @@ -28,14 +31,19 @@
use Throwable;

class ReminderService {

private ICache $cache;

public function __construct(
protected IUserManager $userManager,
protected IURLGenerator $urlGenerator,
protected INotificationManager $notificationManager,
protected ReminderMapper $reminderMapper,
protected IRootFolder $root,
protected LoggerInterface $logger,
protected ICacheFactory $cacheFactory,
) {
$this->cache = $this->cacheFactory->createDistributed('files_reminders');
}

/**
Expand All @@ -48,12 +56,26 @@ public function get(int $id): RichReminder {

/**
* @throws NodeNotFoundException
* @throws DoesNotExistException
*/
public function getDueForUser(IUser $user, int $fileId): RichReminder {
public function getDueForUser(IUser $user, int $fileId): ?RichReminder {
$this->checkNode($user, $fileId);
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
return new RichReminder($reminder, $this->root);
/** @var null|false|Reminder $cachedReminder */
$cachedReminder = $this->cache->get("{$user->getUID()}-$fileId");
if ($cachedReminder === false) {
return null;
}
if ($cachedReminder instanceof Reminder) {
return new RichReminder($cachedReminder, $this->root);
}

try {
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
$this->cache->set("{$user->getUID()}-$fileId", $reminder);
return new RichReminder($reminder, $this->root);
} catch (DoesNotExistException $e) {
$this->cache->set("{$user->getUID()}-$fileId", false);
return null;
}
}

/**
Expand All @@ -77,46 +99,49 @@ public function getAll(?IUser $user = null) {
public function createOrUpdate(IUser $user, int $fileId, DateTime $dueDate): bool {
$now = new DateTime('now', new DateTimeZone('UTC'));
$this->checkNode($user, $fileId);
try {
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
$reminder->setDueDate($dueDate);
$reminder->setUpdatedAt($now);
$this->reminderMapper->update($reminder);
return false;
} catch (DoesNotExistException $e) {
// Create new reminder if no reminder is found
$reminder = $this->getDueForUser($user, $fileId);
if ($reminder === null) {
$reminder = new Reminder();
$reminder->setUserId($user->getUID());
$reminder->setFileId($fileId);
$reminder->setDueDate($dueDate);
$reminder->setUpdatedAt($now);
$reminder->setCreatedAt($now);
$this->reminderMapper->insert($reminder);
$this->cache->set("{$user->getUID()}-$fileId", $reminder);
return true;
}
$reminder->setDueDate($dueDate);
$reminder->setUpdatedAt($now);
$this->reminderMapper->update($reminder);
$this->cache->set("{$user->getUID()}-$fileId", $reminder);
return false;
}

/**
* @throws NodeNotFoundException
* @throws DoesNotExistException
* @throws ReminderNotFoundException
*/
public function remove(IUser $user, int $fileId): void {
$this->checkNode($user, $fileId);
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
$this->reminderMapper->delete($reminder);
$reminder = $this->getDueForUser($user, $fileId);
if ($reminder === null) {
throw new ReminderNotFoundException();
}
$this->deleteReminder($reminder);
}

public function removeAllForNode(Node $node): void {
$reminders = $this->reminderMapper->findAllForNode($node);
foreach ($reminders as $reminder) {
$this->reminderMapper->delete($reminder);
$this->deleteReminder($reminder);
}
}

public function removeAllForUser(IUser $user): void {
$reminders = $this->reminderMapper->findAllForUser($user);
foreach ($reminders as $reminder) {
$this->reminderMapper->delete($reminder);
$this->deleteReminder($reminder);
}
}

Expand Down Expand Up @@ -148,6 +173,7 @@ public function send(Reminder $reminder): void {
try {
$this->notificationManager->notify($notification);
$this->reminderMapper->markNotified($reminder);
$this->cache->set("{$user->getUID()}-{$reminder->getFileId()}", $reminder);
} catch (Throwable $th) {
$this->logger->error($th->getMessage(), $th->getTrace());
}
Expand All @@ -159,10 +185,16 @@ public function cleanUp(?int $limit = null): void {
->modify('-1 day');
$reminders = $this->reminderMapper->findNotified($buffer, $limit);
foreach ($reminders as $reminder) {
$this->reminderMapper->delete($reminder);
$this->deleteReminder($reminder);
}
}

private function deleteReminder(Reminder $reminder): void {
$this->reminderMapper->delete($reminder);
$this->cache->set("{$reminder->getUserId()}-{$reminder->getFileId()}", false);
}


/**
* @throws NodeNotFoundException
*/
Expand Down