fix(CoreQueryBuilder): Use correct member entry for circle as initiator

lib/CirclesManager.php

@@ -69,6 +69,7 @@ class CirclesManager {
 	/** @var CirclesQueryHelper */
 	private $circlesQueryHelper;
 
+	private bool $forceSync = false;
 
 	/**
 	 * CirclesManager constructor.
@@ -152,7 +153,8 @@ public function getLocalFederatedUser(string $userId): FederatedUser {
 	 * @throws InvalidIdException
 	 * @throws FederatedUserException
 	 */
-	public function startSession(?FederatedUser $federatedUser = null): void {
+	public function startSession(?FederatedUser $federatedUser = null, bool $forceSync = false): void {
+		$this->forceSync = $forceSync;
 		if (is_null($federatedUser)) {
 			$this->federatedUserService->initCurrentUser();
 		} else {
@@ -163,7 +165,8 @@ public function startSession(?FederatedUser $federatedUser = null): void {
 	/**
 	 *
 	 */
-	public function startSuperSession(): void {
+	public function startSuperSession(bool $forceSync = false): void {
+		$this->forceSync = $forceSync;
 		$this->federatedUserService->unsetCurrentUser();
 		$this->federatedUserService->bypassCurrentUserCondition(true);
 	}
@@ -224,6 +227,7 @@ public function startOccSession(
 	public function stopSession(): void {
 		$this->federatedUserService->unsetCurrentUser();
 		$this->federatedUserService->bypassCurrentUserCondition(false);
+		$this->forceSync = false;
 	}
 
 
@@ -298,7 +302,7 @@ public function createCircle(
 	 * @throws UnknownRemoteException
 	 */
 	public function destroyCircle(string $singleId): void {
-		$this->circleService->destroy($singleId);
+		$this->circleService->destroy($singleId, $this->forceSync);
 	}
 
 
@@ -426,7 +430,7 @@ public function flagAsAppManaged(string $circleId, bool $enabled = true): void {
 	 * @throws UnknownRemoteException
 	 */
 	public function addMember(string $circleId, FederatedUser $federatedUser): Member {
-		$outcome = $this->memberService->addMember($circleId, $federatedUser);
+		$outcome = $this->memberService->addMember($circleId, $federatedUser, $this->forceSync);
 		$member = new Member();
 		$member->import($outcome);
 
@@ -475,7 +479,7 @@ public function levelMember(string $memberId, int $level): Member {
 	 * @throws UnknownRemoteException
 	 */
 	public function removeMember(string $memberId): void {
-		$this->memberService->removeMember($memberId);
+		$this->memberService->removeMember($memberId, $this->forceSync);
 	}
 
 

lib/Db/CoreQueryBuilder.php

@@ -1307,8 +1307,8 @@ public function completeProbeWithInitiator(
 			->leftJoin(
 				$alias, CoreRequestBuilder::TABLE_MEMBER, $aliasInitiator,
 				$expr->andX(
-					$expr->eq($aliasInitiator . '.circle_id', $helperAlias . '.' . $field),
-					$this->exprLimitInt('level', Member::LEVEL_OWNER, $aliasInitiator)
+					$expr->eq($aliasInitiator . '.circle_id', $alias . '.unique_id'),
+					$expr->eq($aliasInitiator . '.' . $field, $helperAlias . '.inheritance_first'),
 				)
 			);
 		//

lib/Model/Federated/FederatedEvent.php

@@ -94,6 +94,8 @@ class FederatedEvent implements JsonSerializable {
 	/** @var int */
 	private $bypass = 0;
 
+	private bool $forceSync = false;
+
 
 	/**
 	 * FederatedEvent constructor.
@@ -552,6 +554,15 @@ public function canBypass(int $flag): bool {
 		return (($this->bypass & $flag) !== 0);
 	}
 
+	public function forceSync(bool $forceSync): self {
+		$this->forceSync = $forceSync;
+		return $this;
+	}
+
+	public function isForceSync(): bool {
+		return $this->forceSync;
+	}
+
 
 	/**
 	 * @param array $data

lib/Service/CircleService.php

@@ -221,6 +221,7 @@ public function create(
 
 	/**
 	 * @param string $circleId
+	 * @param bool $forceSync
 	 *
 	 * @return array
 	 * @throws CircleNotFoundException
@@ -235,13 +236,14 @@ public function create(
 	 * @throws RequestBuilderException
 	 * @throws UnknownRemoteException
 	 */
-	public function destroy(string $circleId): array {
+	public function destroy(string $circleId, bool $forceSync = false): array {
 		$this->federatedUserService->mustHaveCurrentUser();
 
 		$circle = $this->getCircle($circleId);
 
 		$event = new FederatedEvent(CircleDestroy::class);
 		$event->setCircle($circle);
+		$event->forceSync($forceSync);
 		$this->federatedEventService->newEvent($event);
 
 		return $event->getOutcome();

lib/Service/FederatedEventService.php

@@ -364,7 +364,7 @@ private function confirmSharedItem(FederatedEvent $event, IFederatedItem $item):
 	 * @param IFederatedItem $item
 	 */
 	private function configureEvent(FederatedEvent $event, IFederatedItem $item) {
-		if ($item instanceof IFederatedItemAsyncProcess) {
+		if ($item instanceof IFederatedItemAsyncProcess && !$event->isForceSync()) {
 			$event->setAsync(true);
 		}
 		if ($item instanceof IFederatedItemLimitedToInstanceWithMembership) {

lib/Service/MemberService.php

@@ -192,7 +192,7 @@ public function getMembers(string $circleId): array {
 	 * @throws InvalidIdException
 	 * @throws SingleCircleNotFoundException
 	 */
-	public function addMember(string $circleId, FederatedUser $federatedUser): array {
+	public function addMember(string $circleId, FederatedUser $federatedUser, bool $forceSync = false): array {
 		$this->federatedUserService->mustHaveCurrentUser();
 		$circle = $this->circleRequest->getCircle($circleId, $this->federatedUserService->getCurrentUser());
 
@@ -204,6 +204,7 @@ public function addMember(string $circleId, FederatedUser $federatedUser): array
 		$event = new FederatedEvent(SingleMemberAdd::class);
 		$event->setCircle($circle);
 		$event->setMember($member);
+		$event->forceSync($forceSync);
 
 		$this->federatedEventService->newEvent($event);
 
@@ -261,6 +262,7 @@ function (FederatedUser $federatedUser) use ($patron) {
 
 	/**
 	 * @param string $memberId
+	 * @param bool $forceSync
 	 *
 	 * @return array
 	 * @throws FederatedEventException
@@ -274,7 +276,7 @@ function (FederatedUser $federatedUser) use ($patron) {
 	 * @throws UnknownRemoteException
 	 * @throws RequestBuilderException
 	 */
-	public function removeMember(string $memberId): array {
+	public function removeMember(string $memberId, bool $forceSync = false): array {
 		$this->federatedUserService->mustHaveCurrentUser();
 		$member = $this->memberRequest->getMemberById(
 			$memberId,
@@ -284,6 +286,7 @@ public function removeMember(string $memberId): array {
 		$event = new FederatedEvent(MemberRemove::class);
 		$event->setCircle($member->getCircle());
 		$event->setMember($member);
+		$event->forceSync($forceSync);
 
 		$this->federatedEventService->newEvent($event);
 

tests/phpunit.xml

@@ -1,24 +1,19 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="utf-8"?>
 <!--
   - SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
   - SPDX-License-Identifier: AGPL-3.0-or-later
 -->
-<phpunit bootstrap="bootstrap.php" convertDeprecationsToExceptions="true">
-	<testsuite name="circles">
-		<directory suffix='Test.php'>.</directory>
-	</testsuite>
-	<filter>
-		<whitelist>
-			<directory suffix=".php">../appinfo</directory>
-			<directory suffix=".php">../lib</directory>
-		</whitelist>
-	</filter>
-	<logging>
-		<log type="coverage-clover" target="clover.xml"/>
-		<!--<log type="coverage-text" target="php://stdout" showUncoveredFiles="false"/>-->
-	</logging>
-	<listeners>
-		<listener class="OCA\Circles\Tests\Env" file="TestSuiteListener.php"/>
-	</listeners>
-
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="bootstrap.php" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd" cacheDirectory=".phpunit.cache">
+  <testsuite name="circles">
+    <directory suffix="Test.php">.</directory>
+  </testsuite>
+  <logging>
+    <!--<log type="coverage-text" target="php://stdout" showUncoveredFiles="false"/>-->
+  </logging>
+  <source>
+    <include>
+      <directory suffix=".php">../appinfo</directory>
+      <directory suffix=".php">../lib</directory>
+    </include>
+  </source>
 </phpunit>

tests/unit/CirclesManagerTest.php

@@ -0,0 +1,162 @@
+<?php
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Circles\Tests;
+
+use OCA\Circles\CirclesManager;
+use OCA\Circles\Model\Circle;
+use OCA\Circles\Model\FederatedUser;
+use OCA\Circles\Model\Member;
+use OCA\Circles\Model\Probes\DataProbe;
+use OCP\IGroupManager;
+use OCP\IUserManager;
+use Test\TestCase;
+
+/**
+ * @group DB
+ */
+class CirclesManagerTest extends TestCase {
+	private CirclesManager $circlesManager;
+	private string $userId = 'circles-testuser';
+	private string $groupId = 'circles-testgroup';
+	private string $circleName;
+
+	public function setUp(): void {
+		parent::setUp();
+
+		// Random circle name
+		$this->circleName = sha1(uniqId(mt_rand(), true));
+
+		// Create test user
+		$userManager = \OC::$server->get(IUserManager::class);
+		if (!$userManager->userExists($this->userId)) {
+			$user = $userManager->createUser($this->userId, $this->userId);
+		} else {
+			$user = $userManager->get($this->userId);
+		}
+
+		// Create test group and add user
+		$groupManager = \OC::$server->get(IGroupManager::class);
+		if (!$groupManager->groupExists($this->groupId)) {
+			$group = $groupManager->createGroup($this->groupId);
+			$group->addUser($user);
+		}
+
+		$this->circlesManager = \OC::$server->get(CirclesManager::class);
+
+	}
+
+	// Start user session as user (default: test user)
+	private function startSession(?string $userId = null): FederatedUser {
+		if (!$userId) {
+			$userId = $this->userId;
+		}
+		$federatedUser = $this->circlesManager->getLocalFederatedUser($userId);
+		$this->circlesManager->startSession($federatedUser, true);
+		return $federatedUser;
+	}
+
+	public function testCreateCircle(): void {
+		$federatedUser = $this->startSession();
+
+		// Created circle has properties
+		$circle = $this->circlesManager->createCircle($this->circleName);
+		$this->assertEquals($this->circleName, $circle->getName());
+		$this->assertEquals($this->circleName, $circle->getDisplayName());
+		$this->assertEquals($this->circleName, $circle->getSanitizedName());
+		$this->assertEquals($federatedUser->getSingleId(), $circle->getOwner()->getSingleId());
+		$this->assertEquals($federatedUser->getSingleId(), $circle->getInitiator()->getSingleId());
+
+		// Created circle returned by probeCircle()
+		$circles = $this->circlesManager->probeCircles();
+		$this->assertCount(1, array_filter($circles, function (Circle $c) { return $c->getName() === $this->circleName; }));
+
+		// Destroyed circle not returned by probeCircle()
+		$this->circlesManager->destroyCircle($circle->getSingleId());
+		$circles = $this->circlesManager->probeCircles();
+		$this->assertCount(0, array_filter($circles, function (Circle $c) { return $c->getName() === $this->circleName; }));
+	}
+
+	public function testGetCirclesWithInitiator(): void {
+		// Create circle as user 'admin' and add test user as member
+		$this->startSession('admin');
+		$adminCircle = $this->circlesManager->createCircle($this->circleName);
+		$this->circlesManager->addMember($adminCircle->getSingleId(), $this->circlesManager->getLocalFederatedUser($this->userId));
+
+		// Get circles as test user
+		$federatedUser = $this->startSession();
+		$circles = $this->circlesManager->getCircles();
+		$circle = null;
+		foreach ($circles as $c) {
+			if ($c->getSingleId() === $adminCircle->getSingleId()) {
+				$circle = $c;
+			}
+		}
+
+		// Initiator of probed circle has correct properties
+		$this->assertEquals($federatedUser->getSingleId(), $circle->getInitiator()->getSingleId());
+		$this->assertEquals(1, $circle->getInitiator()->getLevel());
+
+		// Destroy circle as user 'admin'
+		$this->startSession('admin');
+		$this->circlesManager->destroyCircle($adminCircle->getSingleId());
+	}
+
+	public function testProbeCirclesWithInitiator(): void {
+		// Create circle as user 'admin' and add test user as member
+		$this->startSession('admin');
+		$adminCircle = $this->circlesManager->createCircle($this->circleName);
+		$this->circlesManager->addMember($adminCircle->getSingleId(), $this->circlesManager->getLocalFederatedUser($this->userId));
+
+		// Probe circles as test user
+		$federatedUser = $this->startSession();
+		$dataProbe = new DataProbe();
+		$dataProbe->add(DataProbe::INITIATOR);
+		$circles = $this->circlesManager->probeCircles(null, $dataProbe);
+		$circle = null;
+		foreach ($circles as $c) {
+			if ($c->getSingleId() === $adminCircle->getSingleId()) {
+				$circle = $c;
+			}
+		}
+
+		// Initiator of probed circle has correct properties
+		$this->assertEquals($federatedUser->getSingleId(), $circle->getInitiator()->getSingleId());
+		$this->assertEquals(1, $circle->getInitiator()->getLevel());
+
+		// Destroy circle as user 'admin'
+		$this->startSession('admin');
+		$this->circlesManager->destroyCircle($adminCircle->getSingleId());
+	}
+
+	public function testProbeCirclesWithInitiatorAsGroupMember(): void {
+		// Create circle as user 'admin' and add test group as member
+		$this->startSession('admin');
+		$adminCircle = $this->circlesManager->createCircle($this->circleName);
+		$federatedGroup = $this->circlesManager->getFederatedUser($this->groupId, Member::TYPE_GROUP);
+		$this->circlesManager->addMember($adminCircle->getSingleId(), $federatedGroup);
+
+		// Probe circles as test user
+		$federatedUser = $this->startSession();
+		$dataProbe = new DataProbe();
+		$dataProbe->add(DataProbe::INITIATOR);
+		$circles = $this->circlesManager->probeCircles(null, $dataProbe);
+		$circle = null;
+		foreach ($circles as $c) {
+			if ($c->getSingleId() === $adminCircle->getSingleId()) {
+				$circle = $c;
+			}
+		}
+
+		// Initiator of probed circle has correct properties
+		$this->assertEquals($federatedGroup->getSingleId(), $circle->getInitiator()->getSingleId());
+		$this->assertEquals(1, $circle->getInitiator()->getLevel());
+
+		// Destroy circle as user 'admin'
+		$this->startSession('admin');
+		$this->circlesManager->destroyCircle($adminCircle->getSingleId());
+	}
+}

tests/unit/lib/Controller/AdminControllerTest.php

@@ -108,7 +108,7 @@ public function testCirclesList(int $limit, int $offset): void {
 		$this->assertEquals($response, $this->adminController->circles('an-user-id', $limit, $offset));
 	}
 
-	public function dataForCirclesList(): array {
+	public static function dataForCirclesList(): array {
 		return [
 			[-1, 0],
 			[1, 1]

tests/unit/lib/Controller/LocalControllerTest.php

@@ -117,7 +117,7 @@ public function testCirclesList(int $limit, int $offset): void {
 		$this->assertEquals($response, $this->localController->circles($limit, $offset));
 	}
 
-	public function dataForCirclesList(): array {
+	public static function dataForCirclesList(): array {
 		return [
 			[-1, 0],
 			[1, 1]