From 8abe9bbe13d584745f67bb4aed091d466b1c275f Mon Sep 17 00:00:00 2001 From: Lukas Schaefer Date: Tue, 27 Jan 2026 09:56:15 -0500 Subject: [PATCH] Use new preloadCollection event to fetch all data for a directory Signed-off-by: Lukas Schaefer --- lib/Dav/ApprovalPlugin.php | 49 +++++++++++++++++++++++---- lib/Service/ApprovalService.php | 59 ++++++++++++++++++++++----------- psalm.xml | 2 ++ tests/psalm-baseline.xml | 10 +++++- 4 files changed, 93 insertions(+), 27 deletions(-) diff --git a/lib/Dav/ApprovalPlugin.php b/lib/Dav/ApprovalPlugin.php index 309b9d21..e1921fb3 100644 --- a/lib/Dav/ApprovalPlugin.php +++ b/lib/Dav/ApprovalPlugin.php @@ -11,15 +11,19 @@ use OCA\Approval\AppInfo\Application; use OCA\Approval\Service\ApprovalService; +use OCA\DAV\Connector\Sabre\Directory as SabreDirectory; +use OCA\DAV\Connector\Sabre\Node as SabreNode; +use Sabre\DAV\ICollection; use Sabre\DAV\INode; use Sabre\DAV\PropFind; - use Sabre\DAV\Server; use Sabre\DAV\ServerPlugin; class ApprovalPlugin extends ServerPlugin { /** @var Server */ protected $server; + protected ApprovalService $approvalService; + private array $cachedDirectories; /** * Initializes the plugin and registers event handlers @@ -29,7 +33,9 @@ class ApprovalPlugin extends ServerPlugin { */ public function initialize(Server $server) { $this->server = $server; - $server->on('propFind', [$this, 'getApprovalState']); + $this->approvalService = \OC::$server->get(ApprovalService::class); + $server->on('propFind', [$this, 'propFind']); + $server->on('preloadCollection', $this->preloadCollection(...)); } @@ -37,10 +43,16 @@ public function initialize(Server $server) { * @param PropFind $propFind * @param INode $node */ - public function getApprovalState(PropFind $propFind, INode $node) { - // we instantiate the ApprovalService here to make sure sabre auth backend was triggered - $approvalService = \OC::$server->get(ApprovalService::class); - $approvalService->propFind($propFind, $node); + public function propFind(PropFind $propFind, INode $node) { + if (!$node instanceof SabreNode) { + return; + } + $nodeId = $node->getId(); + $propFind->handle( + Application::DAV_PROPERTY_APPROVAL_STATE, function () use ($nodeId) { + return $this->approvalService->propFind($nodeId); + } + ); } /** @@ -72,4 +84,29 @@ public function getPluginInfo(): array { 'description' => 'Provides approval state in PROPFIND WebDav requests', ]; } + + /** + * @param PropFind $propFind + * @param ICollection $collection + * @return void + */ + public function preloadCollection(PropFind $propFind, ICollection $collection): void { + if (!($collection instanceof SabreNode)) { + return; + } + // need prefetch ? + if ($collection instanceof SabreDirectory + && !isset($this->cachedDirectories[$collection->getPath()]) + && (!is_null($propFind->getStatus(Application::DAV_PROPERTY_APPROVAL_STATE)) + )) { + // note: pre-fetching only supported for depth <= 1 + $folderContent = $collection->getChildren(); + $fileIds = [(int)$collection->getId()]; + foreach ($folderContent as $info) { + $fileIds[] = (int)$info->getId(); + } + $this->approvalService->preloadApprovalStates($fileIds); + $this->cachedDirectories[$collection->getPath()] = true; + } + } } diff --git a/lib/Service/ApprovalService.php b/lib/Service/ApprovalService.php index 4aae1ed9..c0d953e9 100644 --- a/lib/Service/ApprovalService.php +++ b/lib/Service/ApprovalService.php @@ -11,7 +11,6 @@ use OCA\Approval\Activity\ActivityManager; use OCA\Approval\AppInfo\Application; use OCA\Approval\Exceptions\OutdatedEtagException; -use OCA\DAV\Connector\Sabre\Node as SabreNode; use OCP\App\IAppManager; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; @@ -26,13 +25,12 @@ use OCP\SystemTag\ISystemTagObjectMapper; use OCP\SystemTag\TagNotFoundException; use Psr\Log\LoggerInterface; -use Sabre\DAV\INode; -use Sabre\DAV\PropFind; /** * @psalm-import-type Rule from RuleService */ class ApprovalService { + private array $cachedStates; public function __construct( string $appName, @@ -288,22 +286,32 @@ public function getEtag(int $fileId): string { * @param int $fileId * @param string|null $userId * @param bool $userHasAccessChecked whether we already checked if a user has access + * @param array|null $tags preloaded tags * @return array state and rule id */ - public function getApprovalState(int $fileId, ?string $userId, bool $userHasAccessChecked = false): array { + public function getApprovalState(int $fileId, ?string $userId, bool $userHasAccessChecked = false, ?array $tags = null): array { + if (isset($this->cachedStates[$userId][$fileId])) { + return $this->cachedStates[$userId][$fileId]; + } if (is_null($userId) || !($userHasAccessChecked || $this->utilsService->userHasAccessTo($fileId, $userId))) { return ['state' => Application::STATE_NOTHING]; } - $rules = $this->ruleService->getRules(); - - // Get all tags a file has to prevent needing to check for each tag in every rule - $tags = $this->tagObjectMapper->getTagIdsForObjects([(string)$fileId], 'files'); + // Get all tags a file has to prevent needing to check for each tag in every rule and uses preloaded tags when possible + if (is_null($tags)) { + $tags = $this->tagObjectMapper->getTagIdsForObjects([(string)$fileId], 'files'); + } if (!array_key_exists((string)$fileId, $tags)) { return ['state' => Application::STATE_NOTHING]; } $tags = array_map(static fn ($tag): string => (string)$tag, $tags[(string)$fileId]); + if (empty($tags)) { + return ['state' => Application::STATE_NOTHING]; + } + + $rules = $this->ruleService->getRules(); + // first check if it's approvable foreach ($rules as $id => $rule) { if (in_array($rule['tagPending'], $tags, true) @@ -828,22 +836,33 @@ public function sendRequestNotification(int $fileId, array $rule, string $reques /** * Get approval state as a WebDav attribute * - * @param PropFind $propFind - * @param INode $node + * @param int $nodeId + * @return int + */ + public function propFind(int $nodeId): int { + $state = $this->getApprovalState($nodeId, $this->userId, true); + return $state['state']; + } + + + /** + * Get approval state for multiple files and loads all the tags at once + * + * @param array $fileIds * @return void */ - public function propFind(PropFind $propFind, INode $node): void { - if (!$node instanceof SabreNode) { + public function preloadApprovalStates(array $fileIds): void { + $userId = $this->userId; + if (is_null($userId)) { return; } - $nodeId = $node->getId(); - - $propFind->handle( - Application::DAV_PROPERTY_APPROVAL_STATE, function () use ($nodeId) { - $state = $this->getApprovalState($nodeId, $this->userId, true); - return $state['state']; - } - ); + $tags = $this->tagObjectMapper->getTagIdsForObjects($fileIds, 'files'); + if (!isset($this->cachedStates[$userId])) { + $this->cachedStates[$userId] = []; + } + foreach ($fileIds as $fileId) { + $this->cachedStates[$userId][$fileId] = $this->getApprovalState($fileId, $userId, true, $tags); + } } /** diff --git a/psalm.xml b/psalm.xml index 4fa9a49b..3f4c4ff8 100644 --- a/psalm.xml +++ b/psalm.xml @@ -41,7 +41,9 @@ + + diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 5a9a7d82..8e301cf5 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,3 +1,11 @@ - + + + + + + + + +