From 10e8c098b1aceddae463123d8bd4d3a9bd81d03f Mon Sep 17 00:00:00 2001 From: Juan Claude Date: Wed, 11 Nov 2026 18:48:38 +0000 Subject: [PATCH 1/2] test(profile-fields): pin ProfileFieldService visibility, search, saves and definitions before the split Assisted-by: ClaudeCode:claude-opus-5-5 --- .../ProfileFieldServiceAccessGoldenTest.php | 308 ++++++++++++++++++ 1 file changed, 308 insertions(+) create mode 100644 tests/Unit/Service/ProfileFieldServiceAccessGoldenTest.php diff --git a/tests/Unit/Service/ProfileFieldServiceAccessGoldenTest.php b/tests/Unit/Service/ProfileFieldServiceAccessGoldenTest.php new file mode 100644 index 000000000..64fe1a975 --- /dev/null +++ b/tests/Unit/Service/ProfileFieldServiceAccessGoldenTest.php @@ -0,0 +1,308 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace Unit\Service; + +use InvalidArgumentException; +use OCA\LaunchPad\Db\AdminSettingMapper; +use OCA\LaunchPad\Db\ProfileFieldValues; +use OCA\LaunchPad\Service\AdminTemplateService; +use OCA\LaunchPad\Service\ProfileFieldService; +use OCA\LaunchPad\Service\ProfileFieldTools; +use OCA\LaunchPad\Service\ProfileSources; +use OCP\Accounts\IAccountManager; +use OCP\IUser; +use OCP\LDAP\ILDAPProvider; +use OCP\LDAP\ILDAPProviderFactory; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use RuntimeException; +use Unit\Support\InMemoryProfileValues; + +class ProfileFieldServiceAccessGoldenTest extends TestCase { + /** @var array */ + private array $settings = []; + + /** @var array */ + private array $groups = ['ann' => ['sales'], 'bob' => ['sales'], 'cat' => ['hr']]; + + private InMemoryProfileValues $values; + private ILDAPProviderFactory $ldapFactory; + private LoggerInterface $logger; + private ProfileFieldService $service; + + protected function setUp(): void { + $settingMapper = $this->createMock(AdminSettingMapper::class); + $settingMapper->method('getValue')->willReturnCallback(fn (string $key, mixed $default = null): mixed => $this->settings[$key] ?? $default); + $settingMapper->method('setSetting')->willReturnCallback(function (string $key, mixed $value) { + $this->settings[$key] = $value; + return new \OCA\LaunchPad\Db\AdminSetting(); + }); + $templates = $this->createMock(AdminTemplateService::class); + $templates->method('getUserGroupIdsFor')->willReturnCallback(fn (string $userId): array => $this->groups[$userId] ?? []); + $this->values = new InMemoryProfileValues(); + $this->ldapFactory = $this->createMock(ILDAPProviderFactory::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->service = new ProfileFieldService( + $settingMapper, + $this->values, + new ProfileSources($this->createMock(IAccountManager::class), $this->ldapFactory), + new ProfileFieldTools($templates, $this->logger), + ); + } + + private function define(): void { + $this->service->saveDefinitions(raw: [ + ['key' => 'office', 'label' => 'Office', 'type' => 'text', 'source' => 'ldap', 'ldapAttribute' => 'roomNumber'], + ['key' => 'cost', 'label' => 'Cost centre', 'type' => 'text', 'source' => 'self', 'searchable' => true, 'visibility' => 'groups'], + ['key' => 'skills', 'label' => 'Skills', 'type' => 'tags', 'source' => 'self', 'searchable' => true], + ['key' => 'hidden', 'label' => 'Hidden', 'type' => 'text', 'source' => 'self', 'shownInWidget' => false, 'searchable' => true], + ]); + } + + private function seed(string $userId, string $key, array $values, ?string $scope = null): void { + $this->values->replaceValues(userId: $userId, field: new ProfileFieldValues(fieldKey: $key, values: $values, source: 'self', scope: $scope)); + } + + public function testNormalisedDefinition(): void { + $saved = $this->service->saveDefinitions(raw: ['x' => ['key' => 'k1', 'label' => ' Label ', 'ldapAttribute' => 'ignored', 'searchable' => 1, 'shownInWidget' => 'yes']]); + + $this->assertSame([[ + 'key' => 'k1', + 'label' => 'Label', + 'type' => 'text', + 'source' => 'self', + 'ldapAttribute' => '', + 'searchable' => false, + 'shownInWidget' => false, + 'visibility' => 'everyone', + ]], $saved); + } + + public function testLdapDefinitionKeepsItsAttribute(): void { + $saved = $this->service->saveDefinitions(raw: [['key' => 'o', 'label' => 'O', 'source' => 'ldap', 'ldapAttribute' => ' room-Number2 ', 'type' => 'tags', 'visibility' => 'groups', 'searchable' => true]]); + + $this->assertSame(['room-Number2', 'tags', 'groups', true, true], [$saved[0]['ldapAttribute'], $saved[0]['type'], $saved[0]['visibility'], $saved[0]['searchable'], $saved[0]['shownInWidget']]); + } + + /** + * Invalid definitions and their message. + * + * @return array + */ + public static function invalidDefinitions(): array { + return [ + 'not an object' => ['text', 'A profile field must be an object'], + 'bad key' => [['key' => 'Bad'], 'Field key must be lower case letters, digits or _: Bad'], + 'key too long' => [['key' => 'a' . str_repeat('b', 32)], 'Field key must be lower case letters, digits or _: a' . str_repeat('b', 32)], + 'key before label' => [['key' => '', 'label' => ''], 'Field key must be lower case letters, digits or _: '], + 'no label' => [['key' => 'k', 'label' => ' '], 'Field label must be 1 to 64 characters: k'], + 'label too long' => [['key' => 'k', 'label' => str_repeat('é', 65)], 'Field label must be 1 to 64 characters: k'], + 'label before type' => [['key' => 'k', 'label' => '', 'type' => 'x'], 'Field label must be 1 to 64 characters: k'], + 'bad type' => [['key' => 'k', 'label' => 'L', 'type' => 'number', 'source' => 'x'], 'Field type must be one of '], + 'bad source' => [['key' => 'k', 'label' => 'L', 'source' => 'x', 'visibility' => 'x'], 'Field source must be one of '], + 'bad visibility' => [['key' => 'k', 'label' => 'L', 'visibility' => 'x', 'source' => 'ldap'], 'Field visibility must be one of '], + 'ldap no attribute' => [['key' => 'k', 'label' => 'L', 'source' => 'ldap'], 'An LDAP field needs an LDAP attribute name: k'], + 'ldap bad attr' => [['key' => 'k', 'label' => 'L', 'source' => 'ldap', 'ldapAttribute' => '9x'], 'An LDAP field needs an LDAP attribute name: k'], + 'type not string' => [['key' => 'k', 'label' => 'L', 'type' => 1], 'Field type must be one of '], + ]; + } + + /** + * @dataProvider invalidDefinitions + */ + public function testInvalidDefinition(mixed $entry, string $message): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + + $this->service->saveDefinitions(raw: [$entry]); + } + + public function testSavedOwnValuesAreCleanedAndOthersSkipped(): void { + $this->define(); + + $fields = $this->service->saveOwnValues(userId: 'ann', values: [ + 'office' => 'ignored, read only', + 'cost' => ' 4711 ', + 'skills' => [' PHP ', 'php', null, '', 'Vue'], + ]); + + $byKey = array_column($fields, 'values', 'key'); + $this->assertSame([[], ['4711'], ['PHP', 'Vue'], []], [$byKey['office'], $byKey['cost'], $byKey['skills'], $byKey['hidden']]); + } + + public function testEmptyValuesClearAField(): void { + $this->define(); + $this->seed('ann', 'cost', ['1']); + + $fields = $this->service->saveOwnValues(userId: 'ann', values: ['cost' => null, 'skills' => []]); + + $this->assertSame([], array_column($fields, 'values', 'key')['cost']); + } + + /** + * Invalid own values and their message; nothing is written. + * + * @return array,1:string}> + */ + public static function invalidValues(): array { + return [ + 'unknown field' => [['cost' => '1', 'ghost' => 'x'], 'Unknown profile field: ghost'], + 'tags not list' => [['skills' => 'php'], 'Tags must be a list: skills'], + 'not text' => [['cost' => 5], 'A value must be text: cost'], + 'tag not text' => [['skills' => ['a', ['b']]], 'A value must be text: skills'], + 'too long' => [['cost' => str_repeat('x', 256)], 'A value is longer than 255 characters: cost'], + 'too many tags' => [['skills' => array_map('strval', range(1, 26))], 'At most 25 tags: skills'], + 'unknown first' => [['ghost' => 'x', 'cost' => 5], 'Unknown profile field: ghost'], + ]; + } + + /** + * @dataProvider invalidValues + */ + public function testInvalidOwnValues(array $values, string $message): void { + $this->define(); + $this->seed('ann', 'cost', ['keep']); + + try { + $this->service->saveOwnValues(userId: 'ann', values: $values); + $this->fail('expected a refusal'); + } catch (InvalidArgumentException $e) { + $this->assertSame($message, $e->getMessage()); + } + + $this->assertSame(['keep'], array_column($this->service->getOwnFields(userId: 'ann'), 'values', 'key')['cost']); + } + + public function testVisibleFieldsPerViewer(): void { + $this->define(); + $this->seed('ann', 'cost', ['4711']); + $this->seed('ann', 'skills', ['PHP']); + $this->seed('ann', 'hidden', ['secret']); + $this->seed('cat', 'skills', ['HR law']); + $this->seed('dan', 'cost', ['9']); + + $all = ['ann', 'cat', 'dan', 'eve']; + $asBob = $this->service->visibleFieldsFor(viewerId: 'bob', userIds: $all); + $asCat = $this->service->visibleFieldsFor(viewerId: 'cat', userIds: $all); + $asDan = $this->service->visibleFieldsFor(viewerId: 'dan', userIds: $all); + $anonymous = $this->service->visibleFieldsFor(viewerId: null, userIds: $all); + + $this->assertSame([ + 'ann' => [ + ['key' => 'cost', 'label' => 'Cost centre', 'type' => 'text', 'values' => ['4711']], + ['key' => 'skills', 'label' => 'Skills', 'type' => 'tags', 'values' => ['PHP']], + ], + 'cat' => [['key' => 'skills', 'label' => 'Skills', 'type' => 'tags', 'values' => ['HR law']]], + ], $asBob); + $this->assertSame(['ann', 'cat'], array_keys($asCat)); + $this->assertSame(['skills'], array_column($asCat['ann'], 'key')); + $this->assertSame(['ann', 'cat', 'dan'], array_keys($asDan)); + $this->assertSame([], $anonymous); + $this->assertSame([], $this->service->visibleFieldsFor(viewerId: 'bob', userIds: [])); + } + + public function testNoWidgetFieldsShowsNothing(): void { + $this->service->saveDefinitions(raw: [['key' => 'k', 'label' => 'K', 'shownInWidget' => false]]); + $this->seed('ann', 'k', ['v']); + + $this->assertSame([], $this->service->visibleFieldsFor(viewerId: 'ann', userIds: ['ann'])); + } + + public function testSearchHonoursVisibilityAndScopes(): void { + $this->define(); + $this->seed('ann', 'cost', ['4711 north']); + $this->seed('cat', 'cost', ['north 9']); + $this->seed('dan', 'skills', ['north star']); + $this->seed('eve', 'office', ['north wing']); + $this->seed('fay', 'nc:role', ['north lead'], IAccountManager::SCOPE_PRIVATE); + $this->seed('gus', 'nc:role', ['north lead'], IAccountManager::SCOPE_FEDERATED); + $this->seed('hal', 'hidden', ['north']); + + $this->assertSame(['ann', 'dan', 'gus', 'hal'], $this->service->findMatchingUserIds(viewerId: 'bob', query: ' NORTH ')); + $this->assertSame(['dan', 'fay', 'gus', 'hal'], $this->service->findMatchingUserIds(viewerId: 'fay', query: 'north')); + $this->assertSame([], $this->service->findMatchingUserIds(viewerId: null, query: 'north')); + $this->assertSame([], $this->service->findMatchingUserIds(viewerId: 'bob', query: ' ')); + } + + /** + * Scope cases: scope, viewer, owner, allowed. + * + * @return array + */ + public static function scopes(): array { + return [ + 'owner private' => [IAccountManager::SCOPE_PRIVATE, 'ann', 'ann', true], + 'other private' => [IAccountManager::SCOPE_PRIVATE, 'bob', 'ann', false], + 'other local' => [IAccountManager::SCOPE_LOCAL, 'bob', 'ann', true], + 'other no scope' => [null, 'bob', 'ann', true], + 'anonymous public' => [IAccountManager::SCOPE_PUBLISHED, null, 'ann', false], + 'anonymous owner' => [null, null, '', false], + ]; + } + + /** + * @dataProvider scopes + */ + public function testScopeAllows(?string $scope, ?string $viewerId, string $ownerId, bool $allowed): void { + $this->assertSame($allowed, $this->service->scopeAllows(scope: $scope, viewerId: $viewerId, ownerId: $ownerId)); + } + + private function ldapUser(string $backend = 'LDAP'): IUser { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('ann'); + $user->method('getBackendClassName')->willReturn($backend); + return $user; + } + + public function testLdapSyncOutcomes(): void { + $this->assertSame(0, $this->service->syncLdapFields(user: $this->ldapUser())); + + $this->service->saveDefinitions(raw: [ + ['key' => 'office', 'label' => 'Office', 'source' => 'ldap', 'ldapAttribute' => 'roomNumber'], + ['key' => 'langs', 'label' => 'Languages', 'type' => 'tags', 'source' => 'ldap', 'ldapAttribute' => 'lang'], + ]); + $this->ldapFactory->method('isAvailable')->willReturnOnConsecutiveCalls(false, true, true); + $provider = $this->createMock(ILDAPProvider::class); + $calls = 0; + $provider->method('getMultiValueUserAttribute')->willReturnCallback(static function (string $uid, string $attribute) use (&$calls): array { + $calls++; + if ($calls === 4) { + throw new RuntimeException('ldap gone'); + } + + return $attribute === 'roomNumber' ? [' A1 ', 'B2'] : ['nl', 'NL', 'en']; + }); + $this->ldapFactory->method('getLDAPProvider')->willReturn($provider); + $this->logger->expects($this->once())->method('warning')->with('Profile LDAP sync failed for ann: ldap gone'); + + $this->assertSame(0, $this->service->syncLdapFields(user: $this->ldapUser('Database'))); + $this->assertSame(0, $this->service->syncLdapFields(user: $this->ldapUser())); + $this->assertSame(2, $this->service->syncLdapFields(user: $this->ldapUser())); + $this->assertSame(1, $this->service->syncLdapFields(user: $this->ldapUser())); + + $byKey = array_column($this->service->getOwnFields(userId: 'ann'), 'values', 'key'); + $this->assertSame([['A1'], ['nl', 'en']], [$byKey['office'], $byKey['langs']]); + } +}//end class From 728c630edc950255f9d74689bca89b9f2900ab1e Mon Sep 17 00:00:00 2001 From: Juan Claude Date: Wed, 11 Nov 2026 18:49:55 +0000 Subject: [PATCH 2/2] refactor(profile-fields): split ProfileFieldService definitions, own values, visibility, search and LDAP sync into named steps Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/ProfileFieldListing.php | 93 ++++++++ lib/Service/ProfileFieldService.php | 321 ++++++++++++++++++++-------- 2 files changed, 324 insertions(+), 90 deletions(-) create mode 100644 lib/Service/ProfileFieldListing.php diff --git a/lib/Service/ProfileFieldListing.php b/lib/Service/ProfileFieldListing.php new file mode 100644 index 000000000..9ae4008c7 --- /dev/null +++ b/lib/Service/ProfileFieldListing.php @@ -0,0 +1,93 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @version GIT:auto + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * @spec openspec/specs/people-widget/spec.md + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Service; + +use Closure; + +/** + * Lists the profile fields one viewer may see, per person. + * + * Built by {@see ProfileFieldService::visibleFieldsFor()} for one listing; + * never shared. + * + * @spec openspec/specs/people-widget/spec.md + */ +final class ProfileFieldListing { + /** + * Constructor. + * + * @param array> $definitions The widget's field definitions. + * @param Closure $canSee `(string $ownerId, array $definition): bool`. + */ + public function __construct( + private readonly array $definitions, + private readonly Closure $canSee, + ) { + }//end __construct() + + /** + * The visible fields of every person that has at least one. + * + * @param array> $grouped Values by person, then field key. + * + * @return array> + * + * @spec openspec/specs/people-widget/spec.md + */ + public function fieldsByPerson(array $grouped): array { + $result = []; + foreach ($grouped as $userId => $byKey) { + $fields = $this->fieldsOf(ownerId: (string)$userId, byKey: $byKey); + if ($fields !== []) { + $result[$userId] = $fields; + } + } + + return $result; + }//end fieldsByPerson() + + /** + * One person's fields that have values and that the viewer may see, + * in definition order. + * + * @param string $ownerId The person. + * @param array $byKey Their values by field key. + * + * @return list + */ + private function fieldsOf(string $ownerId, array $byKey): array { + $fields = []; + foreach ($this->definitions as $definition) { + $values = $byKey[$definition['key']] ?? []; + if ($values === [] || ($this->canSee)($ownerId, $definition) === false) { + continue; + } + + $fields[] = ['key' => $definition['key'], 'label' => $definition['label'], 'type' => $definition['type'], 'values' => $values]; + } + + return $fields; + }//end fieldsOf() +}//end class diff --git a/lib/Service/ProfileFieldService.php b/lib/Service/ProfileFieldService.php index 6d14aaa6d..a39fc09b1 100644 --- a/lib/Service/ProfileFieldService.php +++ b/lib/Service/ProfileFieldService.php @@ -195,40 +195,96 @@ private function validateDefinition(mixed $entry): array { throw new InvalidArgumentException(message: 'A profile field must be an object'); } + $definition = $this->definitionFrom(entry: $entry, key: self::validKey(entry: $entry)); + + return self::withCheckedLdapAttribute(definition: $definition); + }//end validateDefinition() + + /** + * The definition's key, checked: lower case letters, digits or `_`, + * starting with a letter, at most 32 characters. + * + * @param array $entry Raw definition. + * + * @return string The key. + * + * @throws InvalidArgumentException When the key is malformed. + */ + private static function validKey(array $entry): string { $key = (string)($entry['key'] ?? ''); if (preg_match(pattern: '/^[a-z][a-z0-9_]{0,31}$/', subject: $key) !== 1) { throw new InvalidArgumentException(message: 'Field key must be lower case letters, digits or _: ' . $key); } + return $key; + }//end validKey() + + /** + * The definition's trimmed label, checked: 1 to 64 characters. + * + * @param array $entry Raw definition. + * @param string $key The checked key (for the message). + * + * @return string The label. + * + * @throws InvalidArgumentException When the label is empty or too long. + */ + private static function validLabel(array $entry, string $key): string { $label = trim(string: (string)($entry['label'] ?? '')); if ($label === '' || mb_strlen(string: $label) > 64) { throw new InvalidArgumentException(message: 'Field label must be 1 to 64 characters: ' . $key); } - $type = $this->oneOf(value: $entry['type'] ?? 'text', allowed: self::TYPES, name: 'type'); - $source = $this->oneOf(value: $entry['source'] ?? 'self', allowed: self::SOURCES, name: 'source'); - $visibility = $this->oneOf(value: $entry['visibility'] ?? 'everyone', allowed: self::VISIBILITIES, name: 'visibility'); - - $ldapAttribute = trim(string: (string)($entry['ldapAttribute'] ?? '')); - if ($source === 'ldap' && preg_match(pattern: '/^[A-Za-z][A-Za-z0-9-]{0,63}$/', subject: $ldapAttribute) !== 1) { - throw new InvalidArgumentException(message: 'An LDAP field needs an LDAP attribute name: ' . $key); - } - - if ($source !== 'ldap') { - $ldapAttribute = ''; - } + return $label; + }//end validLabel() + /** + * The normalised definition: label, then type, source and visibility + * checked in that order; flags default to not searchable and shown. + * + * @param array $entry Raw definition. + * @param string $key The checked key. + * + * @return array The definition, LDAP attribute unchecked. + * + * @throws InvalidArgumentException When a field is invalid. + */ + private function definitionFrom(array $entry, string $key): array { return [ 'key' => $key, - 'label' => $label, - 'type' => $type, - 'source' => $source, - 'ldapAttribute' => $ldapAttribute, + 'label' => self::validLabel(entry: $entry, key: $key), + 'type' => $this->oneOf(value: $entry['type'] ?? 'text', allowed: self::TYPES, name: 'type'), + 'source' => $this->oneOf(value: $entry['source'] ?? 'self', allowed: self::SOURCES, name: 'source'), + 'ldapAttribute' => trim(string: (string)($entry['ldapAttribute'] ?? '')), 'searchable' => ($entry['searchable'] ?? false) === true, 'shownInWidget' => ($entry['shownInWidget'] ?? true) === true, - 'visibility' => $visibility, + 'visibility' => $this->oneOf(value: $entry['visibility'] ?? 'everyone', allowed: self::VISIBILITIES, name: 'visibility'), ]; - }//end validateDefinition() + }//end definitionFrom() + + /** + * An LDAP field must name a valid LDAP attribute; any other field keeps + * none. + * + * @param array $definition The normalised definition. + * + * @return array The definition. + * + * @throws InvalidArgumentException When an LDAP field has no valid attribute. + */ + private static function withCheckedLdapAttribute(array $definition): array { + if ($definition['source'] === 'ldap' + && preg_match(pattern: '/^[A-Za-z][A-Za-z0-9-]{0,63}$/', subject: $definition['ldapAttribute']) !== 1 + ) { + throw new InvalidArgumentException(message: 'An LDAP field needs an LDAP attribute name: ' . $definition['key']); + } + + if ($definition['source'] !== 'ldap') { + $definition['ldapAttribute'] = ''; + } + + return $definition; + }//end withCheckedLdapAttribute() /** * Check that a value is one of the allowed strings. @@ -289,11 +345,41 @@ public function getOwnFields(string $userId): array { * @spec openspec/specs/people-widget/spec.md */ public function saveOwnValues(string $userId, array $values): array { + $clean = $this->cleanOwnValues(definitions: $this->definitionsByKey(), values: $values); + + foreach ($clean as $key => $list) { + $this->valueMapper->replaceValues(userId: $userId, field: new ProfileFieldValues(fieldKey: (string)$key, values: $list, source: 'self')); + } + + return $this->getOwnFields(userId: $userId); + }//end saveOwnValues() + + /** + * The definitions keyed by field key. + * + * @return array> + */ + private function definitionsByKey(): array { $definitions = []; foreach ($this->getDefinitions() as $definition) { $definitions[$definition['key']] = $definition; } + return $definitions; + }//end definitionsByKey() + + /** + * Check every submitted value before anything is written; values of + * fields the user does not own (LDAP, Nextcloud) are left out. + * + * @param array> $definitions The definitions by key. + * @param array $values Key → value (string or list). + * + * @return array The cleaned values by key. + * + * @throws InvalidArgumentException On an unknown field or invalid value. + */ + private function cleanOwnValues(array $definitions, array $values): array { $clean = []; foreach ($values as $key => $value) { $definition = $definitions[$key] ?? null; @@ -301,19 +387,13 @@ public function saveOwnValues(string $userId, array $values): array { throw new InvalidArgumentException(message: 'Unknown profile field: ' . $key); } - if ($definition['source'] !== 'self') { - continue; + if ($definition['source'] === 'self') { + $clean[$key] = $this->cleanValues(definition: $definition, value: $value); } - - $clean[$key] = $this->cleanValues(definition: $definition, value: $value); } - foreach ($clean as $key => $list) { - $this->valueMapper->replaceValues(userId: $userId, field: new ProfileFieldValues(fieldKey: (string)$key, values: $list, source: 'self')); - } - - return $this->getOwnFields(userId: $userId); - }//end saveOwnValues() + return $clean; + }//end cleanOwnValues() /** * Turn a submitted value into a clean list of strings. @@ -326,44 +406,89 @@ public function saveOwnValues(string $userId, array $values): array { * @throws InvalidArgumentException When the value does not fit the type. */ private function cleanValues(array $definition, mixed $value): array { - $items = [$value]; - if ($definition['type'] === 'tags') { - if (is_array(value: $value) === false) { - throw new InvalidArgumentException(message: 'Tags must be a list: ' . $definition['key']); + $result = []; + foreach (self::valueItems(definition: $definition, value: $value) as $item) { + $text = self::cleanItem(definition: $definition, item: $item); + if ($text !== null) { + // The first spelling of a tag wins. + $result[mb_strtolower(string: $text)] ??= $text; } + } - $items = $value; + if (count(value: $result) > self::MAX_TAGS) { + throw new InvalidArgumentException(message: 'At most ' . self::MAX_TAGS . ' tags: ' . $definition['key']); } - $result = []; - foreach ($items as $item) { - if ($item === null) { - continue; - } + return array_values(array: $result); + }//end cleanValues() - if (is_string(value: $item) === false) { - throw new InvalidArgumentException(message: 'A value must be text: ' . $definition['key']); - } + /** + * The submitted items of a field: a tags field takes a list, any other + * field one value. + * + * @param array $definition The field definition. + * @param mixed $value The submitted value. + * + * @return array The items. + * + * @throws InvalidArgumentException When a tags field gets no list. + */ + private static function valueItems(array $definition, mixed $value): array { + if ($definition['type'] !== 'tags') { + return [$value]; + } - $item = trim(string: $item); - if ($item === '') { - continue; - } + if (is_array(value: $value) === false) { + throw new InvalidArgumentException(message: 'Tags must be a list: ' . $definition['key']); + } - if (mb_strlen(string: $item) > self::MAX_VALUE_LENGTH) { - throw new InvalidArgumentException(message: 'A value is longer than ' . self::MAX_VALUE_LENGTH . ' characters: ' . $definition['key']); - } + return $value; + }//end valueItems() - // The first spelling of a tag wins. - $result[mb_strtolower(string: $item)] ??= $item; + /** + * One submitted item, trimmed; null for a null or blank item. + * + * @param array $definition The field definition. + * @param mixed $item The submitted item. + * + * @return string|null The text, or null when the item is skipped. + * + * @throws InvalidArgumentException When the item is not text or too long. + */ + private static function cleanItem(array $definition, mixed $item): ?string { + if ($item === null) { + return null; } - if (count(value: $result) > self::MAX_TAGS) { - throw new InvalidArgumentException(message: 'At most ' . self::MAX_TAGS . ' tags: ' . $definition['key']); + $text = self::itemText(definition: $definition, item: $item); + if ($text === '') { + return null; } - return array_values(array: $result); - }//end cleanValues() + if (mb_strlen(string: $text) > self::MAX_VALUE_LENGTH) { + throw new InvalidArgumentException(message: 'A value is longer than ' . self::MAX_VALUE_LENGTH . ' characters: ' . $definition['key']); + } + + return $text; + }//end cleanItem() + + /** + * A submitted item as trimmed text. + * + * @param array $definition The field definition. + * @param mixed $item The submitted, non-null item. + * + * @return string The trimmed text. + * + * @throws InvalidArgumentException When the item is not text. + */ + private static function itemText(array $definition, mixed $item): string { + if (is_string(value: $item) === false) { + throw new InvalidArgumentException(message: 'A value must be text: ' . $definition['key']); + } + + return trim(string: $item); + }//end itemText() /** * Custom fields shown in the widget that the viewer may see (REQ-PEX-004). @@ -384,26 +509,12 @@ public function visibleFieldsFor(?string $viewerId, array $userIds): array { return []; } - $grouped = $this->groupValues(rows: $this->valueMapper->findByUsers(userIds: $userIds)); - - $result = []; - foreach ($grouped as $userId => $byKey) { - foreach ($definitions as $definition) { - $values = $byKey[$definition['key']] ?? []; - if ($values === [] || $this->canSee(viewerId: $viewerId, ownerId: (string)$userId, definition: $definition) === false) { - continue; - } - - $result[$userId][] = [ - 'key' => $definition['key'], - 'label' => $definition['label'], - 'type' => $definition['type'], - 'values' => $values, - ]; - } - } + $listing = new ProfileFieldListing( + definitions: $definitions, + canSee: fn (string $ownerId, array $definition): bool => $this->canSee(viewerId: $viewerId, ownerId: $ownerId, definition: $definition) + ); - return $result; + return $listing->fieldsByPerson(grouped: $this->groupValues(rows: $this->valueMapper->findByUsers(userIds: $userIds))); }//end visibleFieldsFor() /** @@ -423,12 +534,7 @@ public function findMatchingUserIds(?string $viewerId, string $query): array { return []; } - $definitions = []; - foreach ($this->getDefinitions() as $definition) { - if ($definition['searchable'] === true) { - $definitions[$definition['key']] = $definition; - } - } + $definitions = $this->searchableDefinitions(); $matches = []; foreach ($this->valueMapper->search(needle: $needle) as $row) { @@ -441,6 +547,22 @@ public function findMatchingUserIds(?string $viewerId, string $query): array { return array_keys(array: $matches); }//end findMatchingUserIds() + /** + * The searchable definitions keyed by field key. + * + * @return array> + */ + private function searchableDefinitions(): array { + $definitions = []; + foreach ($this->getDefinitions() as $definition) { + if ($definition['searchable'] === true) { + $definitions[$definition['key']] = $definition; + } + } + + return $definitions; + }//end searchableDefinitions() + /** * Whether a matching row may count for this viewer. * @@ -480,15 +602,11 @@ private function rowMatchesFor(?string $viewerId, ProfileValue $row, array $defi * @spec openspec/specs/people-widget/spec.md */ public function scopeAllows(?string $scope, ?string $viewerId, string $ownerId): bool { - if ($viewerId !== null && $viewerId === $ownerId) { - return true; - } - if ($viewerId === null) { return false; } - return $scope !== IAccountManager::SCOPE_PRIVATE; + return $viewerId === $ownerId || $scope !== IAccountManager::SCOPE_PRIVATE; }//end scopeAllows() /** @@ -613,16 +731,25 @@ public function syncLdapFields(IUser $user): int { return 0; } + return $this->syncFromLdap(user: $user, definitions: $definitions); + }//end syncLdapFields() + + /** + * Copy each LDAP field's attribute values from the directory; a + * directory failure is logged and ends the sync. + * + * @param IUser $user The LDAP user. + * @param array> $definitions The LDAP field definitions. + * + * @return int The number of fields synced before any failure. + */ + private function syncFromLdap(IUser $user, array $definitions): int { $synced = 0; try { $provider = $this->sources->ldapProviderFactory->getLDAPProvider(); foreach ($definitions as $definition) { $raw = $provider->getMultiValueUserAttribute(uid: $user->getUID(), attribute: $definition['ldapAttribute']); - $values = $this->cleanLdapValues(definition: $definition, raw: $raw); - $this->valueMapper->replaceValues( - userId: $user->getUID(), - field: new ProfileFieldValues(fieldKey: $definition['key'], values: $values, source: 'ldap') - ); + $this->valueMapper->replaceValues(userId: $user->getUID(), field: $this->ldapFieldValues(definition: $definition, raw: $raw)); $synced++; } } catch (Exception $e) { @@ -630,7 +757,21 @@ public function syncLdapFields(IUser $user): int { } return $synced; - }//end syncLdapFields() + }//end syncFromLdap() + + /** + * The cleaned LDAP values of one field, ready to replace the stored ones. + * + * @param array $definition The LDAP field definition. + * @param array $raw The directory's attribute values. + * + * @return ProfileFieldValues The field's new values. + */ + private function ldapFieldValues(array $definition, array $raw): ProfileFieldValues { + $values = $this->cleanLdapValues(definition: $definition, raw: $raw); + + return new ProfileFieldValues(fieldKey: $definition['key'], values: $values, source: 'ldap'); + }//end ldapFieldValues() /** * Clean the values read from LDAP: a text field keeps its first value.