Skip to content

Use new preloadCollection event to fetch all data for a directory #371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lukasdotcom wants to merge 1 commit into
base: main
Choose a base branch
from
+93 −27
Open

Use new preloadCollection event to fetch all data for a directory #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 43 additions & 6 deletions lib/Dav/ApprovalPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,18 +33,26 @@ 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(...));
}


/**
* @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);
}
);
}

/**
Expand Down Expand Up @@ -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;
}
}
}
59 changes: 39 additions & 20 deletions lib/Service/ApprovalService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down
2 changes: 2 additions & 0 deletions psalm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@
<referencedClass name="Sabre\DAV\ServerPlugin" />
<referencedClass name="Sabre\DAV\Server" />
<referencedClass name="OCA\DAV\Connector\Sabre\Node" />
<referencedClass name="OCA\DAV\Connector\Sabre\Directory" />
<referencedClass name="OC\Files\Node\Node" />
<referencedClass name="Sabre\DAV\ICollection" />
</errorLevel>
</UndefinedClass>
<UndefinedDocblockClass>
Expand Down
10 changes: 9 additions & 1 deletion tests/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="5.17.0@c620f6e80d0abfca532b00bda366062aaedf6e5d">
<files psalm-version="6.14.3@d0b040a91f280f071c1abcb1b77ce3822058725a">
<file src="lib/Dav/ApprovalPlugin.php">
<RedundantCondition>
<code><![CDATA[$collection instanceof SabreNode]]></code>
</RedundantCondition>
<TypeDoesNotContainType>
<code><![CDATA[$collection instanceof SabreDirectory]]></code>
</TypeDoesNotContainType>
</file>
</files>
Loading