perf(files_reminders): Reduce db queries on propfind

Signed-off-by: Christopher Ng <chrng8@gmail.com>
This commit is contained in:
Christopher Ng 2025-01-24 14:27:53 -08:00 committed by provokateurin
parent e694f65e92
commit e969d277b6
No known key found for this signature in database
4 changed files with 92 additions and 36 deletions

View file

@ -14,8 +14,8 @@ use DateTimeInterface;
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;
@ -53,15 +53,14 @@ class ApiController extends OCSController {
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 (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);
}
}
@ -125,7 +124,7 @@ class ApiController extends OCSController {
try {
$this->reminderService->remove($user, $fileId);
return new DataResponse([], Http::STATUS_OK);
} catch (DoesNotExistException $e) {
} catch (NodeNotFoundException|ReminderNotFoundException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}
}

View file

@ -12,7 +12,6 @@ namespace OCA\FilesReminders\Dav;
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;
@ -52,9 +51,8 @@ class PropFindPlugin extends ServerPlugin {
}
$fileId = $node->getId();
try {
$reminder = $this->reminderService->getDueForUser($user, $fileId);
} catch (DoesNotExistException $e) {
$reminder = $this->reminderService->getDueForUser($user, $fileId);
if ($reminder === null) {
return '';
}

View file

@ -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 {
}

View file

@ -15,11 +15,14 @@ use OCA\FilesReminders\AppInfo\Application;
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;
@ -28,6 +31,9 @@ use Psr\Log\LoggerInterface;
use Throwable;
class ReminderService {
private ICache $cache;
public function __construct(
protected IUserManager $userManager,
protected IURLGenerator $urlGenerator,
@ -35,7 +41,9 @@ class ReminderService {
protected ReminderMapper $reminderMapper,
protected IRootFolder $root,
protected LoggerInterface $logger,
protected ICacheFactory $cacheFactory,
) {
$this->cache = $this->cacheFactory->createDistributed('files_reminders');
}
/**
@ -47,11 +55,27 @@ class ReminderService {
}
/**
* @throws DoesNotExistException
* @throws NodeNotFoundException
*/
public function getDueForUser(IUser $user, int $fileId): RichReminder {
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
return new RichReminder($reminder, $this->root);
public function getDueForUser(IUser $user, int $fileId): ?RichReminder {
$this->checkNode($user, $fileId);
/** @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;
}
}
/**
@ -74,18 +98,9 @@ class ReminderService {
*/
public function createOrUpdate(IUser $user, int $fileId, DateTime $dueDate): bool {
$now = new DateTime('now', new DateTimeZone('UTC'));
try {
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
$reminder->setDueDate($dueDate);
$reminder->setUpdatedAt($now);
$this->reminderMapper->update($reminder);
return false;
} catch (DoesNotExistException $e) {
$node = $this->root->getUserFolder($user->getUID())->getFirstNodeById($fileId);
if (!$node) {
throw new NodeNotFoundException();
}
// Create new reminder if no reminder is found
$this->checkNode($user, $fileId);
$reminder = $this->getDueForUser($user, $fileId);
if ($reminder === null) {
$reminder = new Reminder();
$reminder->setUserId($user->getUID());
$reminder->setFileId($fileId);
@ -93,29 +108,40 @@ class ReminderService {
$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 DoesNotExistException
* @throws NodeNotFoundException
* @throws ReminderNotFoundException
*/
public function remove(IUser $user, int $fileId): void {
$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
$this->reminderMapper->delete($reminder);
$this->checkNode($user, $fileId);
$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);
}
}
@ -147,6 +173,7 @@ class ReminderService {
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());
}
@ -158,7 +185,24 @@ class ReminderService {
->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
*/
private function checkNode(IUser $user, int $fileId): void {
$userFolder = $this->root->getUserFolder($user->getUID());
$node = $userFolder->getFirstNodeById($fileId);
if ($node === null) {
throw new NodeNotFoundException();
}
}
}