cleaner version of config check for memberships

lib/Command/CirclesMaintenance.php

@@ -131,8 +131,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
 			$output->writeln('');
 			$output->writeln('<error>WARNING! This operation is not reversible.</error>');
-
-
+
 			$question = new Question(
 				'<comment>Please confirm this destructive operation by typing \'' . $action
 				. '\'</comment>: ', ''

lib/Db/CoreQueryBuilder.php

@@ -69,6 +69,7 @@ class CoreQueryBuilder extends NC22ExtendedQueryBuilder {
 	const BASED_ON = 'basedon';
 	const INITIATOR = 'initiator';
 	const MEMBERSHIPS = 'memberships';
+	const CONFIG = 'config';
 	const UPSTREAM_MEMBERSHIPS = 'upstreammemberships';
 	const INHERITANCE_FROM = 'inheritancefrom';
 	const INHERITED_BY = 'inheritedby';
@@ -83,43 +84,47 @@ class CoreQueryBuilder extends NC22ExtendedQueryBuilder {
 
 
 	public static $SQL_PATH = [
-		self::SINGLE => [
+		self::SINGLE      => [
 			self::MEMBER
 		],
-		self::CIRCLE => [
-			self::OPTIONS   => [
-				'getPersonalCircleAsAdmin'     => true,
-				'filterPersonalCircleAsMember' => true
+		self::CIRCLE      => [
+			self::OPTIONS     => [
 			],
 			self::MEMBER,
 			self::MEMBER_COUNT,
-			self::OWNER     => [
+			self::OWNER       => [
 				self::BASED_ON
 			],
-			self::MEMBERSHIPS,
-			self::INITIATOR => [
+			self::MEMBERSHIPS => [
+				self::CONFIG
+			],
+			self::INITIATOR   => [
 				self::BASED_ON,
 				self::INHERITED_BY => [
 					self::MEMBERSHIPS
 				]
 			],
-			self::REMOTE    => [
+			self::REMOTE      => [
 				self::MEMBER,
 				self::CIRCLE => [
 					self::OWNER
 				]
 			]
 		],
-		self::MEMBER => [
-			self::MEMBERSHIPS,
+		self::MEMBER      => [
+			self::MEMBERSHIPS => [
+				self::CONFIG
+			],
 			self::INHERITANCE_FROM,
-			self::CIRCLE     => [
-				self::OPTIONS   => [
+			self::CIRCLE      => [
+				self::OPTIONS     => [
 					'getData' => true
 				],
 				self::OWNER,
-				self::MEMBERSHIPS,
-				self::INITIATOR => [
+				self::MEMBERSHIPS => [
+					self::CONFIG
+				],
+				self::INITIATOR   => [
 					self::OPTIONS      => [
 						'mustBeMember' => true,
 						'canBeVisitor' => false
@@ -134,7 +139,7 @@ class CoreQueryBuilder extends NC22ExtendedQueryBuilder {
 					]
 				]
 			],
-			self::BASED_ON   => [
+			self::BASED_ON    => [
 				self::OWNER,
 				self::MEMBERSHIPS,
 				self::INITIATOR => [
@@ -144,18 +149,21 @@ class CoreQueryBuilder extends NC22ExtendedQueryBuilder {
 					]
 				]
 			],
-			self::REMOTE     => [
+			self::REMOTE      => [
 				self::MEMBER,
 				self::CIRCLE => [
 					self::OWNER
 				]
 			],
-			self::INVITED_BY => [
+			self::INVITED_BY  => [
 				self::OWNER,
 				self::BASED_ON
 			]
 		],
-		self::SHARE  => [
+		self::MEMBERSHIPS => [
+			self::CONFIG
+		],
+		self::SHARE       => [
 			self::SHARE,
 			self::FILE_CACHE           => [
 				self::STORAGES
@@ -167,7 +175,9 @@ class CoreQueryBuilder extends NC22ExtendedQueryBuilder {
 				],
 				self::SHARE,
 			],
-			self::MEMBERSHIPS,
+			self::MEMBERSHIPS          => [
+				self::CONFIG
+			],
 			self::INHERITANCE_FROM,
 			self::INHERITED_BY         => [
 				self::BASED_ON
@@ -182,29 +192,33 @@ class CoreQueryBuilder extends NC22ExtendedQueryBuilder {
 				]
 			]
 		],
-		self::REMOTE => [
+		self::REMOTE      => [
 			self::MEMBER
 		],
-		self::MOUNT  => [
-			self::MEMBER    => [
+		self::MOUNT       => [
+			self::MEMBER      => [
 				self::REMOTE
 			],
-			self::INITIATOR => [
+			self::INITIATOR   => [
 				self::INHERITED_BY => [
 					self::MEMBERSHIPS
 				]
 			],
 			self::MOUNTPOINT,
-			self::MEMBERSHIPS
+			self::MEMBERSHIPS => [
+				self::CONFIG
+			]
 		],
-		self::HELPER => [
-			self::MEMBERSHIPS,
-			self::INITIATOR => [
+		self::HELPER      => [
+			self::MEMBERSHIPS => [
+				self::CONFIG
+			],
+			self::INITIATOR   => [
 				self::INHERITED_BY => [
 					self::MEMBERSHIPS
 				]
 			],
-			self::CIRCLE    => [
+			self::CIRCLE      => [
 				self::OPTIONS => [
 				],
 				self::MEMBER,
@@ -1049,6 +1063,28 @@ public function limitToInitiator(
 	}
 
 
+	/**
+	 * @param string $alias
+	 */
+	public function leftJoinCircleConfig(string $alias): void {
+		$expr = $this->expr();
+		try {
+			$aliasConfig = $this->generateAlias($alias, self::CONFIG, $options);
+			$this->selectAlias(
+				$aliasConfig . '.config',
+				(($alias !== $this->getDefaultSelectAlias()) ? $alias . '_' : '') . 'circle_config'
+			);
+			$this->leftJoin(
+				$alias,
+				CoreRequestBuilder::TABLE_CIRCLE,
+				$aliasConfig,
+				$expr->eq($alias . '.circle_id', $aliasConfig . '.unique_id')
+			);
+		} catch (RequestBuilderException $e) {
+		}
+	}
+
+
 	/**
 	 * Left join members to filter userId as initiator.
 	 *
@@ -1084,6 +1120,17 @@ public function leftJoinInitiator(
 			)
 		);
 
+		try {
+			$aliasMembershipCircle = $this->generateAlias($aliasMembership, self::CONFIG, $options);
+			$this->leftJoin(
+				$aliasMembership,
+				CoreRequestBuilder::TABLE_CIRCLE,
+				$aliasMembershipCircle,
+				$expr->eq($aliasMembership . '.circle_id', $aliasMembershipCircle . '.unique_id')
+			);
+		} catch (RequestBuilderException $e) {
+		}
+
 		if (!$this->getBool('getData', $options, false)) {
 			return;
 		}
@@ -1135,8 +1182,9 @@ public function leftJoinInitiator(
 	 */
 	protected function limitInitiatorVisibility(string $alias): ICompositeExpression {
 		$aliasMembership = $this->generateAlias($alias, self::MEMBERSHIPS, $options);
-		$getPersonalCircleAsAdmin = $this->getBool('getPersonalCircleAsAdmin', $options, false);
-		$filterPersonalCircleAsMember = $this->getBool('filterPersonalCircleAsMember', $options, false);
+		$aliasMembershipCircle = $this->generateAlias($aliasMembership, self::CONFIG, $options);
+
+		$filterPersonalCircle = $this->getBool('filterPersonalCircle', $options, true);
 
 		$expr = $this->expr();
 
@@ -1145,10 +1193,10 @@ protected function limitInitiatorVisibility(string $alias): ICompositeExpression
 		// - 2 (Personal), if initiator is owner)
 		// - 4 (Visible to everyone)
 		$orX = $expr->orX();
-		if ($getPersonalCircleAsAdmin) {
+		if ($filterPersonalCircle) {
 			$orX->add(
 				$expr->andX(
-					$this->exprLimitBitwise('config', Circle::CFG_PERSONAL, $aliasMembership),
+					$this->exprLimitBitwise('config', Circle::CFG_PERSONAL, $aliasMembershipCircle),
 					$expr->eq($aliasMembership . '.level', $this->createNamedParameter(Member::LEVEL_OWNER))
 				)
 			);
@@ -1158,8 +1206,10 @@ protected function limitInitiatorVisibility(string $alias): ICompositeExpression
 		$andXMember->add(
 			$expr->gte($aliasMembership . '.level', $this->createNamedParameter(Member::LEVEL_MEMBER))
 		);
-		if ($filterPersonalCircleAsMember) {
-			$andXMember->add($this->exprFilterBitwise('config', Circle::CFG_PERSONAL, $aliasMembership));
+		if ($filterPersonalCircle) {
+			$andXMember->add(
+				$this->exprFilterBitwise('config', Circle::CFG_PERSONAL, $aliasMembershipCircle)
+			);
 		}
 		$orX->add($andXMember);
 
@@ -1168,7 +1218,14 @@ protected function limitInitiatorVisibility(string $alias): ICompositeExpression
 		}
 		if ($this->getBool('canBeVisitor', $options, false)) {
 			// TODO: should find a better way, also filter on remote initiator on non-federated ?
-			$orX->add($expr->gte($alias . '.config', $this->createNamedParameter(0)));
+			$andXVisitor = $expr->andX();
+			$andXVisitor->add($expr->gte($alias . '.config', $this->createNamedParameter(0)));
+			if ($filterPersonalCircle) {
+				$andXVisitor->add(
+					$this->exprFilterBitwise('config', Circle::CFG_PERSONAL, $aliasMembershipCircle)
+				);
+			}
+			$orX->add($andXVisitor);
 		}
 		if ($this->getBool('canBeVisitorOnOpen', $options, false)) {
 			$andOpen = $expr->andX();

lib/Db/CoreRequestBuilder.php

@@ -106,7 +106,6 @@ class CoreRequestBuilder {
 			'single_id',
 			'circle_id',
 			'level',
-			'config',
 			'inheritance_first',
 			'inheritance_last',
 			'inheritance_path',

lib/Db/MembershipRequest.php

@@ -52,7 +52,6 @@ class MembershipRequest extends MembershipRequestBuilder {
 	public function insert(Membership $membership) {
 		$qb = $this->getMembershipInsertSql();
 		$qb->setValue('circle_id', $qb->createNamedParameter($membership->getCircleId()));
-		$qb->setValue('config', $qb->createNamedParameter($membership->getConfig()));
 		$qb->setValue('single_id', $qb->createNamedParameter($membership->getSingleId()));
 		$qb->setValue('level', $qb->createNamedParameter($membership->getLevel()));
 		$qb->setValue('inheritance_first', $qb->createNamedParameter($membership->getInheritanceFirst()));
@@ -99,6 +98,7 @@ public function getMembership(string $circleId, string $singleId): Membership {
 		$qb = $this->getMembershipSelectSql();
 		$qb->limitToCircleId($circleId);
 		$qb->limitToSingleId($singleId);
+		$qb->leftJoinCircleConfig(self::TABLE_MEMBERSHIP);
 
 		return $this->getItemFromRequest($qb);
 	}
@@ -112,6 +112,7 @@ public function getMembership(string $circleId, string $singleId): Membership {
 	public function getMemberships(string $singleId): array {
 		$qb = $this->getMembershipSelectSql();
 		$qb->limitToSingleId($singleId);
+		$qb->leftJoinCircleConfig(CoreQueryBuilder::MEMBERSHIPS);
 
 		return $this->getItemsFromRequest($qb);
 	}
@@ -126,6 +127,7 @@ public function getMemberships(string $singleId): array {
 	public function getInherited(string $singleId, int $level = 0): array {
 		$qb = $this->getMembershipSelectSql();
 		$qb->limitToCircleId($singleId);
+		$qb->leftJoinCircleConfig(self::TABLE_MEMBERSHIP);
 
 		if ($level > 1) {
 			$expr = $qb->expr();

lib/Listeners/Files/MembershipsRemoved.php

@@ -125,14 +125,13 @@ public function handle(Event $event): void {
 			 * // $this->shareWrapperRequest->removeByMembership($membership);
 			 */
 			$federatedUser = $this->circlesManager->getFederatedUser($membership->getSingleId());
-			// TODO confirm that members is really removed from the Circle
-//			if ($federatedUser->getUserType() === Member::TYPE_USER
-//				&& $federatedUser->isLocal()) {
-//				$this->shareWrapperRequest->removeByInitiatorAndShareWith(
-//					$federatedUser->getUserId(),
-//					$membership->getCircleId()
-//				);
-//			}
+			if ($federatedUser->getUserType() === Member::TYPE_USER
+				&& $federatedUser->isLocal()) {
+				$this->shareWrapperRequest->removeByInitiatorAndShareWith(
+					$federatedUser->getUserId(),
+					$membership->getCircleId()
+				);
+			}
 		}
 	}
 

lib/Migration/Version0022Date20220526113601.php

@@ -417,13 +417,6 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 							   'length'  => 31,
 						   ]
 			);
-			$table->addColumn(
-				'config', 'integer', [
-							'notnull'  => false,
-							'length'   => 11,
-							'unsigned' => true
-						]
-			);
 			$table->addColumn(
 				'single_id', 'string', [
 							   'notnull' => true,
@@ -462,7 +455,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 								  ]
 			);
 
-			$table->addIndex(['single_id', 'config']);
+			$table->addIndex(['single_id']);
 			$table->addUniqueIndex(['single_id', 'circle_id']);
 			$table->addIndex(
 				['inheritance_first', 'inheritance_last', 'circle_id'], 'circles_membership_ifilci'

lib/Migration/Version0022Date20220623224231.php

@@ -90,21 +90,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 			}
 		}
 
-		if ($schema->hasTable('circles_membership')) {
-			$table = $schema->getTable('circles_membership');
-			if (!$table->hasColumn('config')) {
-				$table->addColumn(
-					'config', 'integer', [
-								'notnull'  => false,
-								'length'   => 11,
-								'unsigned' => true
-							]
-				);
-				$table->addIndex(['single_id', 'config']);
-			}
-
-		}
-
+
 		if ($schema->hasTable('circles_circle')) {
 			$table = $schema->getTable('circles_circle');
 			if (!$table->hasColumn('sanitized_name')) {