From 44827e37c0a2e9f69feae6a741a223bf19e49685 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Jun 2021 19:53:18 +0200 Subject: [PATCH] allow interacting with IAccountPropertyCollections - in fact the API could be done in a nicer way and it might be possible to work without IAccountPropertyCollection, but only with the IAccountProperties. - To keep it simple at first and not overengineer the blunt attempt is followed - If necessary helpful in the further cause of development adjustements or extensions can be done quickly with this base Signed-off-by: Arthur Schiwon --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Accounts/Account.php | 58 +++++++++++++++++-- lib/private/Accounts/AccountManager.php | 9 +-- .../Accounts/AccountPropertyCollection.php | 4 ++ lib/private/Accounts/TAccountsHelper.php | 41 +++++++++++++ lib/public/Accounts/IAccount.php | 37 +++++++++++- .../Accounts/IAccountPropertyCollection.php | 7 +++ tests/lib/Accounts/AccountTest.php | 27 ++++++++- 9 files changed, 166 insertions(+), 19 deletions(-) create mode 100644 lib/private/Accounts/TAccountsHelper.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0912eb5396..ef7085cd5e 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -584,6 +584,7 @@ return array( 'OC\\Accounts\\AccountProperty' => $baseDir . '/lib/private/Accounts/AccountProperty.php', 'OC\\Accounts\\AccountPropertyCollection' => $baseDir . '/lib/private/Accounts/AccountPropertyCollection.php', 'OC\\Accounts\\Hooks' => $baseDir . '/lib/private/Accounts/Hooks.php', + 'OC\\Accounts\\TAccountsHelper' => $baseDir . '/lib/private/Accounts/TAccountsHelper.php', 'OC\\Activity\\ActivitySettingsAdapter' => $baseDir . '/lib/private/Activity/ActivitySettingsAdapter.php', 'OC\\Activity\\Event' => $baseDir . '/lib/private/Activity/Event.php', 'OC\\Activity\\EventMerger' => $baseDir . '/lib/private/Activity/EventMerger.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 04adffc410..b89068acbe 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -613,6 +613,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Accounts\\AccountProperty' => __DIR__ . '/../../..' . '/lib/private/Accounts/AccountProperty.php', 'OC\\Accounts\\AccountPropertyCollection' => __DIR__ . '/../../..' . '/lib/private/Accounts/AccountPropertyCollection.php', 'OC\\Accounts\\Hooks' => __DIR__ . '/../../..' . '/lib/private/Accounts/Hooks.php', + 'OC\\Accounts\\TAccountsHelper' => __DIR__ . '/../../..' . '/lib/private/Accounts/TAccountsHelper.php', 'OC\\Activity\\ActivitySettingsAdapter' => __DIR__ . '/../../..' . '/lib/private/Activity/ActivitySettingsAdapter.php', 'OC\\Activity\\Event' => __DIR__ . '/../../..' . '/lib/private/Activity/Event.php', 'OC\\Activity\\EventMerger' => __DIR__ . '/../../..' . '/lib/private/Activity/EventMerger.php', diff --git a/lib/private/Accounts/Account.php b/lib/private/Accounts/Account.php index 109691641f..ff9b78dae4 100644 --- a/lib/private/Accounts/Account.php +++ b/lib/private/Accounts/Account.php @@ -27,12 +27,15 @@ declare(strict_types=1); namespace OC\Accounts; +use Generator; use OCP\Accounts\IAccount; use OCP\Accounts\IAccountProperty; +use OCP\Accounts\IAccountPropertyCollection; use OCP\Accounts\PropertyDoesNotExistException; use OCP\IUser; class Account implements IAccount { + use TAccountsHelper; /** @var IAccountProperty[] */ private $properties = []; @@ -45,11 +48,17 @@ class Account implements IAccount { } public function setProperty(string $property, string $value, string $scope, string $verified, string $verificationData = ''): IAccount { + if ($this->isCollection($property)) { + throw new \InvalidArgumentException('setProperty cannot set an IAccountsPropertyCollection'); + } $this->properties[$property] = new AccountProperty($property, $value, $scope, $verified, $verificationData); return $this; } public function getProperty(string $property): IAccountProperty { + if ($this->isCollection($property)) { + throw new \InvalidArgumentException('getProperty cannot retrieve an IAccountsPropertyCollection'); + } if (!array_key_exists($property, $this->properties)) { throw new PropertyDoesNotExistException($property); } @@ -57,19 +66,41 @@ class Account implements IAccount { } public function getProperties(): array { - return $this->properties; + return array_filter($this->properties, function ($obj) { + return $obj instanceof IAccountProperty; + }); + } + + public function getAllProperties(): Generator { + foreach ($this->properties as $propertyObject) { + if ($propertyObject instanceof IAccountProperty) { + yield $propertyObject; + } else if ($propertyObject instanceof IAccountPropertyCollection) { + foreach ($propertyObject->getProperties() as $property) { + yield $property; + } + } + } } public function getFilteredProperties(string $scope = null, string $verified = null): array { - return \array_filter($this->properties, function (IAccountProperty $obj) use ($scope, $verified) { + $result = $incrementals = []; + /** @var IAccountProperty $obj */ + foreach ($this->getAllProperties() as $obj) { if ($scope !== null && $scope !== $obj->getScope()) { - return false; + continue; } if ($verified !== null && $verified !== $obj->getVerified()) { - return false; + continue; } - return true; - }); + $index = $obj->getName(); + if ($this->isCollection($index)) { + $incrementals[$index] = ($incrementals[$index] ?? -1) + 1; + $index .= '#' . $incrementals[$index]; + } + $result[$index] = $obj; + } + return $result; } public function jsonSerialize() { @@ -79,4 +110,19 @@ class Account implements IAccount { public function getUser(): IUser { return $this->user; } + + public function setPropertyCollection(IAccountPropertyCollection $propertyCollection): IAccount { + $this->properties[$propertyCollection->getName()] = $propertyCollection; + return $this; + } + + public function getPropertyCollection(string $propertyCollection): IAccountPropertyCollection { + if (!array_key_exists($propertyCollection, $this->properties)) { + throw new PropertyDoesNotExistException($propertyCollection); + } + if (!$this->properties[$propertyCollection] instanceof IAccountPropertyCollection) { + throw new \RuntimeException('Requested collection is not an IAccountPropertyCollection'); + } + return $this->properties[$propertyCollection]; + } } diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 3241ad27c6..3b3117d8be 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -58,6 +58,7 @@ use function json_last_error; * @package OC\Accounts */ class AccountManager implements IAccountManager { + use TAccountsHelper; /** @var IDBConnection database connection */ private $connection; @@ -607,12 +608,4 @@ class AccountManager implements IAccountManager { $this->updateUser($account->getUser(), $data, true); } - - protected function isCollection(string $propertyName): bool { - return in_array($propertyName, - [ - IAccountManager::COLLECTION_EMAIL, - ] - ); - } } diff --git a/lib/private/Accounts/AccountPropertyCollection.php b/lib/private/Accounts/AccountPropertyCollection.php index 8e74d45d52..84e10e6a50 100644 --- a/lib/private/Accounts/AccountPropertyCollection.php +++ b/lib/private/Accounts/AccountPropertyCollection.php @@ -83,4 +83,8 @@ class AccountPropertyCollection implements IAccountPropertyCollection { public function jsonSerialize() { return [$this->collectionName => $this->properties]; } + + public function getName(): string { + return $this->collectionName; + } } diff --git a/lib/private/Accounts/TAccountsHelper.php b/lib/private/Accounts/TAccountsHelper.php new file mode 100644 index 0000000000..09cc44c87c --- /dev/null +++ b/lib/private/Accounts/TAccountsHelper.php @@ -0,0 +1,41 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Accounts; + + +use OCP\Accounts\IAccountManager; + +trait TAccountsHelper { + protected function isCollection(string $propertyName): bool { + return in_array($propertyName, + [ + IAccountManager::COLLECTION_EMAIL, + ], + true + ); + } +} diff --git a/lib/public/Accounts/IAccount.php b/lib/public/Accounts/IAccount.php index 7397eb4fea..1d882b5515 100644 --- a/lib/public/Accounts/IAccount.php +++ b/lib/public/Accounts/IAccount.php @@ -26,6 +26,7 @@ declare(strict_types=1); namespace OCP\Accounts; +use Generator; use OCP\IUser; /** @@ -62,16 +63,48 @@ interface IAccount extends \JsonSerializable { /** * Get all properties of an account. Array indices are property names. + * Values from IAccountPropertyCollections are not included in the return + * array. * * @since 15.0.0 - * - * @return IAccountProperty[] + * @deprecated 22.0.0 use getAllProperties() */ public function getProperties(): array; + /** + * Get all properties of an account. Array indices are numeric. To get + * the property name, call getName() against the value. + * + * IAccountPropertyCollections are being flattened into an IAccountProperty + * for each value. + * + * @since 22.0.0 + * + * @return Generator + */ + public function getAllProperties(): Generator; + + /** + * Set a property collection (multi-value properties) + * + * @since 22.0.0 + */ + public function setPropertyCollection(IAccountPropertyCollection $propertyCollection): IAccount; + + /** + * Returns the requestes propery collection (multi-value properties) + * + * @since 22.0.0 + */ + public function getPropertyCollection(string $propertyCollection): IAccountPropertyCollection; + /** * Get all properties that match the provided filters for scope and verification status * + * Since 22.0.0 values from IAccountPropertyCollection are included, but also + * as IAccountProperty instances. They for properties of IAccountPropertyCollection are + * suffixed incrementally, i.e. #0, #1 ... #n – the numbers have no further meaning. + * * @since 15.0.0 * * @param string $scope Must be one of the VISIBILITY_ prefixed constants of \OCP\Accounts\IAccountManager diff --git a/lib/public/Accounts/IAccountPropertyCollection.php b/lib/public/Accounts/IAccountPropertyCollection.php index b67fa5646f..9e026f4ce5 100644 --- a/lib/public/Accounts/IAccountPropertyCollection.php +++ b/lib/public/Accounts/IAccountPropertyCollection.php @@ -38,6 +38,13 @@ use JsonSerializable; */ interface IAccountPropertyCollection extends JsonSerializable { + /** + * retuns the collection name + * + * @since 22.0.0 + */ + public function getName(): string; + /** * set properties of this collection * diff --git a/tests/lib/Accounts/AccountTest.php b/tests/lib/Accounts/AccountTest.php index 9c2a5333d2..0e0c42804e 100644 --- a/tests/lib/Accounts/AccountTest.php +++ b/tests/lib/Accounts/AccountTest.php @@ -25,6 +25,7 @@ namespace Test\Accounts; use OC\Accounts\Account; use OC\Accounts\AccountProperty; +use OC\Accounts\AccountPropertyCollection; use OCP\Accounts\IAccountManager; use OCP\IUser; use Test\TestCase; @@ -49,7 +50,7 @@ class AccountTest extends TestCase { $this->assertEquals($property, $account->getProperty(IAccountManager::PROPERTY_WEBSITE)); } - public function testGetProperties() { + public function testGetAndGetAllProperties() { $user = $this->createMock(IUser::class); $properties = [ IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, ''), @@ -59,7 +60,14 @@ class AccountTest extends TestCase { $account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED); $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED); + $col = new AccountPropertyCollection(IAccountManager::COLLECTION_EMAIL); + $additionalProperty = new AccountProperty($col->getName(), 'second@example.org', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, ''); + $col->addProperty($additionalProperty); + $account->setPropertyCollection($col); + $this->assertEquals($properties, $account->getProperties()); + $properties[] = $additionalProperty; + $this->assertEquals(array_values($properties), \iterator_to_array($account->getAllProperties())); } public function testGetFilteredProperties() { @@ -74,11 +82,20 @@ class AccountTest extends TestCase { $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED); $account->setProperty(IAccountManager::PROPERTY_PHONE, '123456', IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED); + $col = new AccountPropertyCollection(IAccountManager::COLLECTION_EMAIL); + $additionalProperty1 = new AccountProperty($col->getName(), 'second@example.org', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, ''); + $additionalProperty2 = new AccountProperty($col->getName(), 'third@example.org', IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED, ''); + $col->addProperty($additionalProperty1); + $col->addProperty($additionalProperty2); + $account->setPropertyCollection($col); + $this->assertEquals( [ IAccountManager::PROPERTY_WEBSITE => $properties[IAccountManager::PROPERTY_WEBSITE], IAccountManager::PROPERTY_PHONE => $properties[IAccountManager::PROPERTY_PHONE], + IAccountManager::COLLECTION_EMAIL . '#0' => $additionalProperty1, + IAccountManager::COLLECTION_EMAIL . '#1' => $additionalProperty2, ], $account->getFilteredProperties(IAccountManager::SCOPE_PUBLISHED) ); @@ -86,12 +103,16 @@ class AccountTest extends TestCase { [ IAccountManager::PROPERTY_EMAIL => $properties[IAccountManager::PROPERTY_EMAIL], IAccountManager::PROPERTY_PHONE => $properties[IAccountManager::PROPERTY_PHONE], + IAccountManager::COLLECTION_EMAIL . '#0' => $additionalProperty2, ], $account->getFilteredProperties(null, IAccountManager::VERIFIED) ); $this->assertEquals( - [IAccountManager::PROPERTY_PHONE => $properties[IAccountManager::PROPERTY_PHONE]], - $account->getFilteredProperties(IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED) + [ + IAccountManager::PROPERTY_PHONE => $properties[IAccountManager::PROPERTY_PHONE], + IAccountManager::COLLECTION_EMAIL . '#0' => $additionalProperty2, + ], + $account->getFilteredProperties(IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED), ); }