From ddd07f2782a4c456c7fd4e9ba982b4db54c8e8e3 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 24 Jun 2021 11:52:11 -0100 Subject: [PATCH] cleaner version of config check for memberships Signed-off-by: Maxence Lange --- lib/Command/CirclesMaintenance.php | 3 +- lib/Db/CoreQueryBuilder.php | 129 +++++++++++++----- lib/Db/CoreRequestBuilder.php | 1 - lib/Db/MembershipRequest.php | 4 +- lib/Listeners/Files/MembershipsRemoved.php | 15 +- .../Version0022Date20220526113601.php | 9 +- .../Version0022Date20220623224231.php | 16 +-- lib/Model/Membership.php | 19 ++- 8 files changed, 115 insertions(+), 81 deletions(-) diff --git a/lib/Command/CirclesMaintenance.php b/lib/Command/CirclesMaintenance.php index d788f19b3..e3b8de848 100644 --- a/lib/Command/CirclesMaintenance.php +++ b/lib/Command/CirclesMaintenance.php @@ -131,8 +131,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln(''); $output->writeln('WARNING! This operation is not reversible.'); - - + $question = new Question( 'Please confirm this destructive operation by typing \'' . $action . '\': ', '' diff --git a/lib/Db/CoreQueryBuilder.php b/lib/Db/CoreQueryBuilder.php index cc6a51f27..3f11ffd50 100644 --- a/lib/Db/CoreQueryBuilder.php +++ b/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(); diff --git a/lib/Db/CoreRequestBuilder.php b/lib/Db/CoreRequestBuilder.php index 16bc00187..ca4cc4a22 100644 --- a/lib/Db/CoreRequestBuilder.php +++ b/lib/Db/CoreRequestBuilder.php @@ -106,7 +106,6 @@ class CoreRequestBuilder { 'single_id', 'circle_id', 'level', - 'config', 'inheritance_first', 'inheritance_last', 'inheritance_path', diff --git a/lib/Db/MembershipRequest.php b/lib/Db/MembershipRequest.php index 54bcf1141..1ec14b691 100644 --- a/lib/Db/MembershipRequest.php +++ b/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(); diff --git a/lib/Listeners/Files/MembershipsRemoved.php b/lib/Listeners/Files/MembershipsRemoved.php index d746179df..e0fea493c 100644 --- a/lib/Listeners/Files/MembershipsRemoved.php +++ b/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() + ); + } } } diff --git a/lib/Migration/Version0022Date20220526113601.php b/lib/Migration/Version0022Date20220526113601.php index 944d86498..308cd8baa 100644 --- a/lib/Migration/Version0022Date20220526113601.php +++ b/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' diff --git a/lib/Migration/Version0022Date20220623224231.php b/lib/Migration/Version0022Date20220623224231.php index a6f424103..e4e220aaa 100644 --- a/lib/Migration/Version0022Date20220623224231.php +++ b/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')) { diff --git a/lib/Model/Membership.php b/lib/Model/Membership.php index a105e0f28..d866470bd 100644 --- a/lib/Model/Membership.php +++ b/lib/Model/Membership.php @@ -57,7 +57,7 @@ class Membership extends ManagedModel implements IDeserializable, INC22QueryRow, private $circleId = ''; /** @var int */ - private $config = 0; + private $circleConfig = 0; /** @var int */ private $level = 0; @@ -99,7 +99,6 @@ public function __construct( $this->setCircleId($circle->getSingleId()); $this->setInheritanceFirst($member->getSingleId()); $this->setInheritanceLast($inheritanceLast === '' ? $member->getCircleId() : $inheritanceLast); - $this->setConfig($circle->getConfig()); $this->setLevel($member->getLevel()); } @@ -143,12 +142,12 @@ public function getCircleId(): string { /** - * @param int $config + * @param int $circleConfig * * @return Membership */ - public function setConfig(int $config): self { - $this->config = $config; + public function setCircleConfig(int $circleConfig): self { + $this->circleConfig = $circleConfig; return $this; } @@ -156,8 +155,8 @@ public function setConfig(int $config): self { /** * @return int */ - public function getConfig(): int { - return $this->config; + public function getCircleConfig(): int { + return $this->circleConfig; } @@ -288,8 +287,8 @@ public function import(array $data): IDeserializable { $this->setSingleId($this->get('singleId', $data)); $this->setCircleId($this->get('circleId', $data)); + $this->setCircleConfig($this->getInt('circleConfig', $data)); $this->setLevel($this->getInt('level', $data)); - $this->setConfig($this->getInt('config', $data)); $this->setInheritanceFirst($this->get('inheritanceFirst', $data)); $this->setInheritanceLast($this->get('inheritanceLast', $data)); $this->setInheritancePath($this->getArray('inheritancePath', $data)); @@ -313,7 +312,7 @@ public function importFromDatabase(array $data, string $prefix = ''): INC22Query $this->setSingleId($this->get($prefix . 'single_id', $data)); $this->setCircleId($this->get($prefix . 'circle_id', $data)); $this->setLevel($this->getInt($prefix . 'level', $data)); - $this->setConfig($this->getInt($prefix . 'config', $data)); + $this->setCircleConfig($this->getInt($prefix . 'circle_config', $data)); $this->setInheritanceFirst($this->get($prefix . 'inheritance_first', $data)); $this->setInheritanceLast($this->get($prefix . 'inheritance_last', $data)); $this->setInheritancePath($this->getArray($prefix . 'inheritance_path', $data)); @@ -330,7 +329,7 @@ public function jsonSerialize(): array { $result = [ 'singleId' => $this->getSingleId(), 'circleId' => $this->getCircleId(), - 'config' => $this->getConfig(), + 'circleConfig' => $this->getCircleConfig(), 'level' => $this->getLevel(), 'inheritanceFirst' => $this->getInheritanceFirst(), 'inheritanceLast' => $this->getInheritanceLast(),