Skip to content

Commit

Permalink
fix(circles): Use probeCircles() instead of getCircles() if possible
Browse files Browse the repository at this point in the history
`probeCircles()` is lighter and more performant.

We can only use `probeCircles()` on newer Nextcloud versions though:
* Nextcloud 31+
* Nextcloud 30 starting with 30.0.3
* Nextcloud 29 starting with 29.0.10
* Nextcloud 28 starting with 28.0.13

Fixes: #498

Signed-off-by: Jonas <jonas@freesources.org>
  • Loading branch information
mejo- committed Dec 4, 2024
1 parent d60b077 commit 39c9587
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 13 deletions.
32 changes: 28 additions & 4 deletions lib/Service/CircleHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
use OCA\Circles\Model\FederatedUser;
use OCA\Circles\Model\Member;
use OCA\Circles\Model\Probes\CircleProbe;
use OCA\Circles\Model\Probes\DataProbe;
use OCA\Circles\Tools\Exceptions\InvalidItemException;
use OCP\AppFramework\QueryException;
use OCP\AutoloadNotAllowedException;
use OCP\Util;
use Psr\Container\ContainerInterface;

class CircleHelper {
Expand All @@ -36,6 +38,22 @@ public function __construct(ContainerInterface $appContainer) {
}
}

/**
* Use `probeCircles()` on
* - Nextcloud 31+
* - Nextcloud 30 starting with 30.0.3
* - Nextcloud 29 starting with 29.0.10
* - Nextcloud 28 starting with 28.0.13
*/
private static function useProbeCircles(): bool {
[$major, $minor, $micro] = Util::getVersion();
$version = $major . '.' . $minor . '.' . $micro;
return $major >= 31
|| ($major === 30 && version_compare($version, '30.0.3', '>='))
|| ($major === 29 && version_compare($version, '29.0.10', '>='))
|| ($major === 28 && version_compare($version, '28.0.13', '>='));
}

/**
* @throws NotFoundException
* @throws NotPermittedException
Expand Down Expand Up @@ -88,9 +106,13 @@ private function startSuperSession(): void {
public function getCircles(?string $userId = null): array {
try {
$this->startSession($userId);
$probe = new CircleProbe();
$probe->mustBeMember();
$circles = $this->circlesManager->getCircles($probe, true);
$circleProbe = new CircleProbe();
$circleProbe->mustBeMember();
$dataProbe = new DataProbe();
$dataProbe->add(DataProbe::INITIATOR);
$circles = self::useProbeCircles()
? $this->circlesManager->probeCircles($circleProbe, $dataProbe)
: $this->circlesManager->getCircles($circleProbe, true);
} catch (RequestBuilderException|
FederatedItemException $e) {
throw new NotPermittedException($e->getMessage(), 0, $e);
Expand Down Expand Up @@ -149,7 +171,9 @@ public function findCircle(string $name, string $userId, int $level = Member::LE
private function existsCircle(string $name): bool {
$this->circlesManager->startSuperSession();
try {
$circles = $this->circlesManager->getCircles();
$circles = self::useProbeCircles()
? $this->circlesManager->probeCircles()
: $this->circlesManager->getCircles();
} catch (InitiatorNotFoundException|RequestBuilderException $e) {
throw new NotPermittedException($e->getMessage(), 0, $e);
}
Expand Down
9 changes: 5 additions & 4 deletions lib/Service/CollectiveHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function __construct(
*/
public function getCollectivesForUser(string $userId, bool $getLevel = true, bool $getUserSettings = true): array {
$circles = $this->circleHelper->getCircles($userId);
$cids = array_map(fn ($circle) => $circle->getSingleId(), $circles);
$cids = array_map(static fn ($circle) => $circle->getSingleId(), $circles);
$circles = array_combine($cids, $circles);
/** @var Collective[] $collectives */
$collectives = $this->collectiveMapper->findByCircleIds($cids);
Expand All @@ -55,13 +55,14 @@ public function getCollectivesForUser(string $userId, bool $getLevel = true, boo
*/
public function getCollectivesTrashForUser(string $userId): array {
$circles = $this->circleHelper->getCircles($userId);
$cids = array_map(fn ($circle) => $circle->getSingleId(), $circles);
$cids = array_map(static fn ($circle) => $circle->getSingleId(), $circles);
$circles = array_combine($cids, $circles);
$collectives = $this->collectiveMapper->findTrashByCircleIdsAndUser($cids, $userId);
foreach ($collectives as $c) {
$cid = $c->getCircleId();
$c->setName($circles[$cid]->getSanitizedName());
$c->setLevel($this->circleHelper->getLevel($cid, $userId));
$circle = $circles[$cid];
$c->setName($circle->getSanitizedName());
$c->setLevel($circle->getInitiator()->getLevel());
}
return $collectives;
}
Expand Down
16 changes: 11 additions & 5 deletions tests/stub.phpstub
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,10 @@ namespace OCA\Circles\Model\Probes {
class CircleProbe {
public function mustBeMember(bool $must = true): self {}
}
class DataProbe {
public const INITIATOR = 'h';
public function add(string $key, array $path = []): self {}
}
}

namespace OCA\Circles\Events {
Expand Down Expand Up @@ -689,16 +693,18 @@ namespace OCA\Circles {
use OCA\Circles\Model\Circle;
use OCA\Circles\Model\FederatedUser;
use OCA\Circles\Model\Probes\CircleProbe;
use OCA\Circles\Model\Probes\DataProbe;
class CirclesManager {
public function startSuperSession(): void {}
public function startSession(?FederatedUser $federatedUser = null): void {}
public function getCircles(?CircleProbe $probe = null): array {}
public function getCircle(string $singleId, ?CircleProbe $probe = null): Circle {}
public function flagAsAppManaged(string $circleId, bool $enabled = true): void {}
public function getFederatedUser(string $federatedId, int $type = Member::TYPE_SINGLE): FederatedUser {}
public function startSession(?FederatedUser $federatedUser = null): void {}
public function startSuperSession(): void {}
public function stopSession(): void {}
public function createCircle(string $name, ?FederatedUser $owner = null, bool $personal = false, bool $local = false): Circle {}
public function destroyCircle(string $singleId): void {}
public function getCircles(?CircleProbe $probe = null): array {}
public function getCircle(string $singleId, ?CircleProbe $probe = null): Circle {}
public function flagAsAppManaged(string $circleId, bool $enabled = true): void {}
public function probeCircles(?CircleProbe $circleProbe = null, ?DataProbe $dataProbe = null): array {}
}
}

Expand Down

0 comments on commit 39c9587

Please sign in to comment.