From ea347e76f328d8569a98a06ae95db518abf1739b Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 3 Nov 2026 19:27:48 +0100 Subject: [PATCH] refactor(stackiq): delete the unreferenced StackiqService and its three sub-services No class in lib/, src/ or appinfo/ uses StackiqService: it has no DI registration in Application.php, no string service id, no route, background job, repair step or occ command, and no other app in the workspace references it. ApiClient, ConflictResolver and DataMapper under lib/Service/Stackiq/ were extracted from it and are not referenced by anything either. All four go (1,910 lines), with their tests (StackiqServiceDecompositionTest, StackiqServiceOrganisationMapperTest, and the stale StackiqEventListenerTest that skipped all of its cases), their phpstan and phpmd baseline entries, and a parity evidence citation pointing at the deleted file. REQ-DECOMP-004 now records the removal instead of the decomposition it asked for. Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/Stackiq/ApiClient.php | 155 -- lib/Service/Stackiq/ConflictResolver.php | 172 -- lib/Service/Stackiq/DataMapper.php | 177 --- lib/Service/StackiqService.php | 1391 ----------------- openspec/parity/capabilities.json | 4 +- openspec/specs/method-decomposition/spec.md | 35 +- phpmd.baseline.xml | 2 - phpstan-baseline.neon | 25 - .../StackiqEventListenerTest.php | 387 ----- .../StackiqServiceDecompositionTest.php | 202 --- .../StackiqServiceOrganisationMapperTest.php | 208 --- 11 files changed, 14 insertions(+), 2744 deletions(-) delete mode 100644 lib/Service/Stackiq/ApiClient.php delete mode 100644 lib/Service/Stackiq/ConflictResolver.php delete mode 100644 lib/Service/Stackiq/DataMapper.php delete mode 100644 lib/Service/StackiqService.php delete mode 100644 tests/Unit/EventListener/StackiqEventListenerTest.php delete mode 100644 tests/Unit/Service/StackiqServiceDecompositionTest.php delete mode 100644 tests/Unit/Service/StackiqServiceOrganisationMapperTest.php diff --git a/lib/Service/Stackiq/ApiClient.php b/lib/Service/Stackiq/ApiClient.php deleted file mode 100644 index 8609b62a3..000000000 --- a/lib/Service/Stackiq/ApiClient.php +++ /dev/null @@ -1,155 +0,0 @@ - - * @copyright 2024 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - * - * SPDX-FileCopyrightText: 2026 Conduction B.V. - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace OCA\Stackiq\Service\Stackiq; - -use OCA\OpenRegister\Contract\ObjectServiceInterface; -use Psr\Log\LoggerInterface; - -/** - * Handles all API communication for the Stackiq domain. - * - * StackiqService delegates API fetch operations to this class, - * keeping its own constructor coupling below the PHPMD CouplingBetweenObjects - * threshold and keeping its methods below ExcessiveMethodLength. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ -class ApiClient { - /** - * Constructor. - * - * @param ObjectServiceInterface $objectService The OpenRegister object service. - * @param LoggerInterface $logger Logger instance. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function __construct( - private readonly ObjectServiceInterface $objectService, - private readonly LoggerInterface $logger, - ) { - }//end __construct() - - /** - * Fetch objects for a given register and schema with optional filters. - * - * @param string $register The register slug to query. - * @param string $schema The schema slug to query. - * @param array $filters Optional filter parameters. - * @param int $limit Maximum number of results (default: 100). - * @param int $offset Pagination offset (default: 0). - * - * @return array Paginated results with 'results' and 'total' keys. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function fetchObjects( - string $register, - string $schema, - array $filters = [], - int $limit = 100, - int $offset = 0, - ): array { - $query = array_merge( - $filters, - [ - '_register' => $register, - '_schema' => $schema, - '_limit' => $limit, - '_offset' => $offset, - ] - ); - - $this->logger->debug( - 'ApiClient: Fetching objects', - ['register' => $register, 'schema' => $schema, 'limit' => $limit] - ); - - try { - return $this->objectService->searchObjectsPaginated($query); - } catch (\Exception $e) { - $this->logger->error( - 'ApiClient: Failed to fetch objects', - ['register' => $register, 'schema' => $schema, 'exception' => $e->getMessage()] - ); - return ['results' => [], 'total' => 0]; - } - - }//end fetchObjects() - - /** - * Fetch a single object by ID from the given register and schema. - * - * @param string $id The object UUID or ID. - * @param string $register The register slug. - * @param string $schema The schema slug. - * - * @return object|null The object entity, or null if not found. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function fetchObject(string $id, string $register, string $schema): ?object { - try { - return $this->objectService->find( - id: $id, - register: $register, - schema: $schema - ); - } catch (\Exception $e) { - $this->logger->warning( - 'ApiClient: Object not found', - ['id' => $id, 'register' => $register, 'schema' => $schema, 'exception' => $e->getMessage()] - ); - return null; - } - - }//end fetchObject() - - /** - * Save (create or update) an object in the given register and schema. - * - * @param string $register The register slug. - * @param string $schema The schema slug. - * @param array $data The object data to persist. - * - * @return object|null The saved object entity, or null on failure. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function saveObject(string $register, string $schema, array $data): ?object { - try { - return $this->objectService->saveObject( - register: $register, - schema: $schema, - object: $data - ); - } catch (\Exception $e) { - $this->logger->error( - 'ApiClient: Failed to save object', - ['register' => $register, 'schema' => $schema, 'exception' => $e->getMessage()] - ); - return null; - } - - }//end saveObject() -}//end class diff --git a/lib/Service/Stackiq/ConflictResolver.php b/lib/Service/Stackiq/ConflictResolver.php deleted file mode 100644 index 66d7b6768..000000000 --- a/lib/Service/Stackiq/ConflictResolver.php +++ /dev/null @@ -1,172 +0,0 @@ - - * @copyright 2024 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - * - * SPDX-FileCopyrightText: 2026 Conduction B.V. - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace OCA\Stackiq\Service\Stackiq; - -use Psr\Log\LoggerInterface; - -/** - * Resolves conflicts and deduplicates entries in the Stackiq domain. - * - * StackiqService delegates all conflict detection and resolution - * methods to this class, shrinking its own complexity metrics. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ -class ConflictResolver { - /** - * Constructor. - * - * @param LoggerInterface $logger Logger instance. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function __construct( - private readonly LoggerInterface $logger, - ) { - }//end __construct() - - /** - * Detect whether two object data arrays represent the same logical entity. - * - * Compares canonical identity fields (uuid, kvkNummer, oin) to determine - * whether an incoming record would duplicate an existing one. - * - * @param array $existing The existing data record. - * @param array $incoming The incoming data record to check. - * - * @return bool True when the records are considered duplicates. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function isDuplicate(array $existing, array $incoming): bool { - // UUID match is definitive. - if ($this->matchesOnField(a: $existing, b: $incoming, field: 'uuid') === true) { - return true; - } - - // KVK number match (Dutch company registry). - if ($this->matchesOnField(a: $existing, b: $incoming, field: 'kvkNummer') === true) { - return true; - } - - // OIN match (Dutch government unique identifier). - if ($this->matchesOnField(a: $existing, b: $incoming, field: 'oin') === true) { - return true; - } - - return false; - }//end isDuplicate() - - /** - * Resolve a conflict between an existing and an incoming record. - * - * When both records have the same identity but different data, the incoming - * record's non-null fields are merged on top of the existing record (last-write-wins - * per field). - * - * @param array $existing The existing record. - * @param array $incoming The incoming (newer) record. - * - * @return array The merged record. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function resolve(array $existing, array $incoming): array { - $merged = $existing; - - foreach ($incoming as $key => $value) { - // Prefer incoming non-null values. - if ($value !== null) { - $merged[$key] = $value; - } - } - - $this->logger->debug( - 'ConflictResolver: Resolved conflict between existing and incoming record', - ['changedFields' => array_keys(array_diff_assoc($merged, $existing))] - ); - - return $merged; - }//end resolve() - - /** - * Deduplicate a list of data records using isDuplicate(). - * - * The first occurrence of each identity is kept; subsequent duplicates - * are merged into it using resolve(). - * - * @param array> $records The list of records to deduplicate. - * - * @return array> Deduplicated record list. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function deduplicate(array $records): array { - $unique = []; - - foreach ($records as $record) { - $found = false; - foreach ($unique as &$existing) { - if ($this->isDuplicate(existing: $existing, incoming: $record) === true) { - $existing = $this->resolve(existing: $existing, incoming: $record); - $found = true; - break; - } - } - - unset($existing); - - if ($found === false) { - $unique[] = $record; - } - } - - $this->logger->debug( - 'ConflictResolver: Deduplicated records', - ['before' => count($records), 'after' => count($unique)] - ); - - return $unique; - }//end deduplicate() - - /** - * Check whether two records share the same value for a given field. - * - * @param array $a First record. - * @param array $b Second record. - * @param string $field The field name to compare. - * - * @return bool True when both records have the same non-empty value for the field. - */ - private function matchesOnField(array $a, array $b, string $field): bool { - $valA = $a[$field] ?? null; - $valB = $b[$field] ?? null; - - if ($valA === null || $valB === null || $valA === '' || $valB === '') { - return false; - } - - return $valA === $valB; - }//end matchesOnField() -}//end class diff --git a/lib/Service/Stackiq/DataMapper.php b/lib/Service/Stackiq/DataMapper.php deleted file mode 100644 index c0244aa7e..000000000 --- a/lib/Service/Stackiq/DataMapper.php +++ /dev/null @@ -1,177 +0,0 @@ - - * @copyright 2024 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - * - * SPDX-FileCopyrightText: 2026 Conduction B.V. - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace OCA\Stackiq\Service\Stackiq; - -use Psr\Log\LoggerInterface; - -/** - * Maps and transforms data between external API formats and the Stackiq domain model. - * - * StackiqService delegates all data transformation methods to this class, - * shrinking its own method bodies below ExcessiveMethodLength. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ -class DataMapper { - /** - * Constructor. - * - * @param LoggerInterface $logger Logger instance. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function __construct( - private readonly LoggerInterface $logger, - ) { - }//end __construct() - - /** - * Map an organisation object data array to the OpenRegister format. - * - * Normalises field names, applies type coercions, and removes undefined keys. - * - * @param array $objectData The raw organisation data. - * - * @return array The mapped data suitable for saving to OpenRegister. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function mapOrganizationToOpenRegister(array $objectData): array { - $mapped = $this->normalizeOrganizationFields(data: $objectData); - $mapped = $this->coerceOrganizationTypes(data: $mapped); - - $this->logger->debug( - 'DataMapper: Mapped organization data', - ['fieldCount' => count($mapped)] - ); - - return $mapped; - }//end mapOrganizationToOpenRegister() - - /** - * Map a contact person object data array to a Nextcloud user profile. - * - * @param array $contactData The raw contact person data. - * - * @return array The mapped data suitable for user profile updates. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function mapContactToUserProfile(array $contactData): array { - $emailRaw = $contactData['e-mailadres'] ?? $contactData['email'] ?? ''; - - return [ - 'email' => strtolower(trim((string)$emailRaw)), - 'displayName' => $this->buildDisplayName(data: $contactData), - 'phone' => $contactData['telefoonnummer'] ?? '', - ]; - - }//end mapContactToUserProfile() - - /** - * Map a status string from Dutch to a boolean active indicator. - * - * @param string $status Raw status value from the data store. - * - * @return bool True when the status represents an active record. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2 - */ - public function mapStatusToActive(string $status): bool { - return in_array( - needle: strtolower($status), - haystack: ['actief', 'active', 'enabled', 'true'], - strict: true - ); - - }//end mapStatusToActive() - - /** - * Normalise field names in the organisation data array. - * - * @param array $data Raw data array. - * - * @return array Normalised data. - */ - private function normalizeOrganizationFields(array $data): array { - // The `name` alias block that used to live here mapped the incoming key - // `name` onto the then-canonical `naam`. The canonical property IS `name` - // now, so the alias and its target are the same key and the block became - // `false` — a - // contradiction that can never run. Removed rather than left as dead code. - if (isset($data['status']) === false && isset($data['actief']) === true) { - $data['status'] = 'inactief'; - if ($data['actief'] === true) { - $data['status'] = 'actief'; - } - - unset($data['actief']); - } - - return $data; - }//end normalizeOrganizationFields() - - /** - * Apply type coercions to organisation data values. - * - * @param array $data Data after field normalisation. - * - * @return array Type-coerced data. - */ - private function coerceOrganizationTypes(array $data): array { - // Ensure integer fields are cast. - $intFields = ['kvkNummer', 'oin', 'toezichthouderOin']; - foreach ($intFields as $field) { - if (isset($data[$field]) === true && is_numeric($data[$field]) === true) { - $data[$field] = (int)$data[$field]; - } - } - - return $data; - }//end coerceOrganizationTypes() - - /** - * Build a display name from contact person data. - * - * @param array $data Contact person data. - * - * @return string Assembled display name. - */ - private function buildDisplayName(array $data): string { - $parts = array_filter( - [ - $data['voornaam'] ?? '', - $data['tussenvoegsel'] ?? '', - $data['achternaam'] ?? '', - ] - ); - - $name = implode(' ', $parts); - if ($name !== '') { - return $name; - } - - return $data['name'] ?? ''; - }//end buildDisplayName() -}//end class diff --git a/lib/Service/StackiqService.php b/lib/Service/StackiqService.php deleted file mode 100644 index d5de3bae9..000000000 --- a/lib/Service/StackiqService.php +++ /dev/null @@ -1,1391 +0,0 @@ - - * @copyright 2024 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @spec openspec/specs/method-decomposition/spec.md - */ - -declare(strict_types=1); - -namespace OCA\Stackiq\Service; - -use OCA\Stackiq\Service\Stackiq\ContactPersonHandler; -use OCA\Stackiq\Service\Stackiq\GroupHandler; -use OCA\Stackiq\Service\Stackiq\HierarchyHandler; -use OCP\App\IAppManager; -use Psr\Container\ContainerInterface; -use Psr\Log\LoggerInterface; - -/** - * Service for handling software catalog operations. - * - * Provides functionality for user management, contact processing, - * email notifications, and object lifecycle management. - * - * @category Service - * @package OCA\Stackiq\Service - * @author Conduction b.v. - * @copyright 2024 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @SuppressWarnings(PHPMD.ExcessiveClassLength) - * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) - * @SuppressWarnings(PHPMD.TooManyMethods) - * @SuppressWarnings(PHPMD.TooManyPublicMethods) - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * @SuppressWarnings(PHPMD.NPathComplexity) - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) - * @SuppressWarnings(PHPMD.LongVariable) - * @SuppressWarnings(PHPMD.ShortVariable) - * @SuppressWarnings(PHPMD.MissingImport) - * @SuppressWarnings(PHPMD.UnusedLocalVariable) - * @SuppressWarnings(PHPMD.UnusedPrivateMethod) - * @SuppressWarnings(PHPMD.UnusedFormalParameter) - * @SuppressWarnings(PHPMD.Superglobals) - * @SuppressWarnings(PHPMD.CamelCaseVariableName) - * @SuppressWarnings(PHPMD.CamelCaseParameterName) - * @SuppressWarnings(PHPMD.UndefinedVariable) - * @SuppressWarnings(PHPMD.CountInLoopExpression) - */ -class StackiqService { - - /** - * The name of the app - * - * @var string - */ - private string $appName; - - /** - * StackiqService constructor - * - * @param ContactPersonHandler $_contactPersonHandler Contact person handler. - * @param GroupHandler $_groupHandler Group handler. - * @param HierarchyHandler $_hierarchyHandler Hierarchy handler. - * @param LoggerInterface $_logger Logger interface. - * @param ContainerInterface $_container Container interface. - * @param IAppManager $_appManager App manager interface. - */ - public function __construct( - private readonly ContactPersonHandler $_contactPersonHandler, - private readonly GroupHandler $_groupHandler, - private readonly HierarchyHandler $_hierarchyHandler, - private readonly LoggerInterface $_logger, - private readonly ContainerInterface $_container, - private readonly IAppManager $_appManager, - ) { - $this->appName = 'stackiq'; - }//end __construct() - - /** - * Gets the ObjectService instance - * - * @return \OCA\OpenRegister\Contract\ObjectServiceInterface|null - */ - private function getObjectService(): ?\OCA\OpenRegister\Contract\ObjectServiceInterface { - if ($this->_appManager->isEnabledForUser(appId: 'openregister') === false) { - return null; - } - - try { - return $this->_container->get('OCA\\OpenRegister\\Service\\ObjectService'); - } catch (\Exception $e) { - $this->_logger->error('Failed to get ObjectService: ' . $e->getMessage()); - return null; - } - }//end getObjectService() - - /** - * Gets the OrganisationMapper instance - * - * OpenRegister is an optional capability for this service (ADR-083 rule 1), - * so the mapper is reached the same way the two services above are: the app - * is asked whether OpenRegister is available, and a failed resolution - * degrades to null with a logged error rather than escaping as a raw - * container exception. Callers must treat null as "OpenRegister is not - * available" and take their own not-available branch. - * - * @return \OCA\OpenRegister\Db\OrganisationMapper|null - */ - private function getOrganisationMapper(): ?\OCA\OpenRegister\Db\OrganisationMapper { - if ($this->_appManager->isEnabledForUser(appId: 'openregister') === false) { - return null; - } - - try { - return $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); - } catch (\Exception $e) { - $this->_logger->error('Failed to get OrganisationMapper: ' . $e->getMessage()); - return null; - } - }//end getOrganisationMapper() - - /** - * Processes a contactpersoon object to create an inactive user - * - * If the contactpersoon object doesn't have a user or the user is missing, - * this method will create an inactive user account. - * - * @param object $contactPersonObject The contactpersoon object to process - * @param bool $isUpdate Whether this is an update operation (defaults to false) - * - * @return bool True if processing was successful - * @throws \Exception If processing fails - * - * @SuppressWarnings(PHPMD.BooleanArgumentFlag) $isUpdate is a simple create-vs-update toggle - * - * @spec openspec/specs/method-decomposition/spec.md - */ - public function processContactpersoon(object $contactPersonObject, bool $isUpdate = false): bool { - $startTime = microtime(true); - - try { - $objectId = $contactPersonObject->getId(); - $objectData = $contactPersonObject->getObject(); - - $this->_logger->info( - 'StackiqService: Starting contactpersoon processing', - [ - 'objectId' => $objectId, - 'objectData' => $objectData, - 'timestamp' => date('Y-m-d H:i:s'), - ] - ); - - // Delegate to contact person handler. - $this->_logger->debug( - 'StackiqService: Delegating to ContactPersonHandler for contactpersoon processing', - [ - 'objectId' => $objectId, - ] - ); - - $result = $this->_contactPersonHandler->processContactpersoon($contactPersonObject, $isUpdate); - - $this->_logger->info( - 'StackiqService: ContactPersonHandler processing completed', - [ - 'objectId' => $objectId, - 'result' => $result, - 'processingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - - if ($result === true) { - // Get the username from the processed object. - $updatedObjectData = $contactPersonObject->getObject(); - $username = $updatedObjectData['username'] ?? ''; - - $this->_logger->info( - 'StackiqService: Username extracted from processed object', - [ - 'objectId' => $objectId, - 'username' => $username, - 'hasUsername' => empty($username) === false, - ] - ); - - if (empty($username) === false) { - // NOTE: Group assignment is already handled by ContactPersonHandler.assignUserGroups(). - // during user creation, so we don't need to call GroupHandler.updateUserGroups() here. - // as it would overwrite the correct group assignments. - // Ensure organization has beheerder and set up manager relationships. - $this->_logger->debug( - 'StackiqService: Ensuring organization beheerder', - [ - 'objectId' => $objectId, - 'username' => $username, - ] - ); - - $this->_hierarchyHandler->ensureOrganizationBeheerder($contactPersonObject, $username); - - // Set user to inactive initially. - $this->_logger->debug( - 'StackiqService: Setting user to inactive', - [ - 'objectId' => $objectId, - 'username' => $username, - ] - ); - - $this->_contactPersonHandler->setUserInactive($username); - - $this->_logger->info( - 'StackiqService: User setup completed', - [ - 'objectId' => $objectId, - 'username' => $username, - 'totalProcessingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - - // Add the newly created user to the organization entity. - $organization = $objectData['organization'] ?? null; - if (empty($organization) === false) { - $this->_logger->info( - 'StackiqService: Adding user to organization entity', - [ - 'objectId' => $objectId, - 'username' => $username, - 'organization' => $organization, - ] - ); - - try { - $organisationMapper = $this->getOrganisationMapper(); - if ($organisationMapper === null) { - $this->_logger->warning( - 'StackiqService: OpenRegister OrganisationMapper not available, skipping organization membership', - [ - 'objectId' => $objectId, - 'username' => $username, - 'organization' => $organization, - ] - ); - // Nothing follows this block but `return $result;`, so this is the - // same exit the method would take after skipping the membership work. - return $result; - } - - $organisation = $organisationMapper->findByUuid($organization); - - if (empty($organisation) === false) { - $currentUsers = $organisation->getUsers() ?? []; - if (in_array($username, $currentUsers) === false) { - $currentUsers[] = $username; - $organisation->setUsers($currentUsers); - $organisationMapper->save($organisation); - - $this->_logger->info( - 'StackiqService: Successfully added user to organization entity', - [ - 'objectId' => $objectId, - 'username' => $username, - 'organization' => $organization, - 'totalUsers' => count($currentUsers), - ] - ); - } else { - $this->_logger->info( - 'StackiqService: User already in organization entity', - [ - 'objectId' => $objectId, - 'username' => $username, - 'organization' => $organization, - ] - ); - }//end if - } else { - $this->_logger->warning( - 'StackiqService: Organization entity not found', - [ - 'objectId' => $objectId, - 'username' => $username, - 'organization' => $organization, - ] - ); - }//end if - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to add user to organization entity', - [ - 'objectId' => $objectId, - 'username' => $username, - 'organization' => $organization, - 'error' => $e->getMessage(), - ] - ); - }//end try - } else { - $this->_logger->warning( - 'StackiqService: No organisation reference found for contact person', - [ - 'objectId' => $objectId, - 'username' => $username, - ] - ); - }//end if - } else { - $this->_logger->warning( - 'StackiqService: No username generated for contactpersoon', - [ - 'objectId' => $objectId, - 'objectData' => $updatedObjectData, - ] - ); - }//end if - } else { - $this->_logger->warning( - 'StackiqService: ContactPersonHandler returned false', - [ - 'objectId' => $objectId, - 'processingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - }//end if - - return $result; - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to process contactpersoon object: ' . $e->getMessage(), - [ - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), - 'objectId' => $contactPersonObject->getId() ?? 'unknown', - 'processingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - throw $e; - }//end try - }//end processContactpersoon() - - /** - * Processes a contactpersoon object to ensure it has a username - * - * If the contactpersoon object doesn't have a username or it's empty, - * this method will create a user account and set the username property. - * - * @param object $contactPersonObject The contactpersoon object to process - * - * @return bool True if processing was successful - * @throws \Exception If processing fails - */ - - /** - * Updates user groups based on contactpersoon data - * - * @param object $contactPersonObject The contactpersoon object - * @param string $username The username to update groups for - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function updateUserGroups(object $contactPersonObject, string $username): void { - // Use the new organization type-based logic instead of old role-based logic. - $user = $this->_container->get(\OCP\IUserManager::class)->get($username); - if (empty($user) === false) { - $contactData = $contactPersonObject->getObject(); - $this->_contactPersonHandler->updateUserGroupsFromContactData($user, $contactData); - } else { - $this->_logger->warning('User not found for group update', ['username' => $username]); - } - }//end updateUserGroups() - - /** - * Ensures organization has at least one beheerder and manages user hierarchy - * - * @param object $contactPersonObject The contactpersoon object - * @param string $username The username being processed - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function ensureOrganizationBeheerder(object $contactPersonObject, string $username): void { - // Delegate to hierarchy handler. - $this->_hierarchyHandler->ensureOrganizationBeheerder($contactPersonObject, $username); - }//end ensureOrganizationBeheerder() - - /** - * Gets a user's manager - * - * @param string $username The username - * - * @return string|null The manager's username or null if not set - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function getUserManager(string $username): ?string { - // Delegate to contact person handler. - return $this->_contactPersonHandler->getUserManager($username); - }//end getUserManager() - - /** - * Activates contactpersonen users for an organization. - * - * @param string $organizationId The organization ID. - * - * @return void - * - * @deprecated This method is disabled to prevent organization duplication. - */ - private function activateContactpersonenForOrganization(string $organizationId): void { - // DISABLED: Organization handling is disabled to prevent duplication. - $this->_logger->info( - 'Organization contactpersonen activation is disabled to prevent duplication', - [ - 'organizationId' => $organizationId, - ] - ); - - return; - // Disabled: organization contactpersonen activation logic removed. - }//end activateContactpersonenForOrganization() - - /** - * Handles new contact creation - * - * @param object $contactObject The contact object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleNewContact(object $contactObject): void { - // Delegate to contact person handler. - $this->_contactPersonHandler->handleNewContact($contactObject); - }//end handleNewContact() - - /** - * Handles new gebruiker creation - * - * @param object $userObject The gebruiker object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleNewGebruiker(object $userObject): void { - // Implementation for handling new gebruiker. - $this->_logger->info( - 'Handling new gebruiker', - [ - 'objectId' => $userObject->getId(), - ] - ); - }//end handleNewGebruiker() - - /* - * NO sendGebruikerWelcomeEmail() HERE. - * - * Its whole body was a `logger->info('Sending gebruiker welcome email')` - * — it never sent anything — and no caller reached it: - * `StackiqEventListener` does not invoke it on the - * gebruiker-created path. Wiring a method that sends no mail would have - * bought nothing; implementing one is a feature, not dead-code removal. - */ - - /** - * Handles contact update - * - * @param object $contactObject The contact object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleContactUpdate(object $contactObject): void { - // Delegate to contact person handler. - $this->_contactPersonHandler->handleContactUpdate($contactObject); - }//end handleContactUpdate() - - /** - * Handles gebruiker update - * - * @param object $userObject The new gebruiker object - * @param object $oldUserObject The old gebruiker object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleGebruikerUpdate(object $userObject, object $oldUserObject): void { - // Implementation for handling gebruiker updates. - $this->_logger->info( - 'Handling gebruiker update', - [ - 'objectId' => $userObject->getId(), - ] - ); - }//end handleGebruikerUpdate() - - /** - * Handles contact deletion - * - * @param object $contactObject The contact object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleContactDeletion(object $contactObject): void { - // Delegate to contact person handler. - $this->_contactPersonHandler->handleContactDeletion($contactObject); - }//end handleContactDeletion() - - /** - * Blocks user for gebruiker - * - * @param object $userObject The gebruiker object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function blockUserForGebruiker(object $userObject): void { - // Implementation for blocking user. - $this->_logger->info( - 'Blocking user for gebruiker', - [ - 'objectId' => $userObject->getId(), - ] - ); - }//end blockUserForGebruiker() - - /** - * Temporarily blocks user for gebruiker - * - * @param object $userObject The gebruiker object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function temporarilyBlockUserForGebruiker(object $userObject): void { - // Implementation for temporarily blocking user. - $this->_logger->info( - 'Temporarily blocking user for gebruiker', - [ - 'objectId' => $userObject->getId(), - ] - ); - }//end temporarilyBlockUserForGebruiker() - - /** - * Restores user access for gebruiker - * - * @param object $userObject The gebruiker object - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function restoreUserAccessForGebruiker(object $userObject): void { - // Implementation for restoring user access. - $this->_logger->info( - 'Restoring user access for gebruiker', - [ - 'objectId' => $userObject->getId(), - ] - ); - }//end restoreUserAccessForGebruiker() - - /* - * NO REVERT-SYNC METHODS HERE. - * - * `syncUserWithRevertedContact()` and `updateUserFromRevertedGebruiker()` - * were the same shape as `sendGebruikerWelcomeEmail()` above: a single - * `logger->info()` and nothing else, with no caller — - * `StackiqEventListener` handles `ObjectRevertedEvent` without - * touching this service. They named a capability (reconcile the Nextcloud - * user after an object revert) that has never been implemented; a log line - * is not that capability, and wiring one in would have made the gap - * invisible instead of removing it. - */ - - /** - * Gets the list of generic user groups - * - * @return array Array of generic user groups - */ - public function getGenericUserGroups(): array { - return $this->_groupHandler->getGenericUserGroups(); - }//end getGenericUserGroups() - - /** - * Sets the list of generic user groups - * - * @param array $groups Array of generic user groups - * - * @return void - */ - public function setGenericUserGroups(array $groups): void { - $this->_groupHandler->setGenericUserGroups($groups); - }//end setGenericUserGroups() - - /** - * Ensures all generic user groups exist - * - * @return array Array of created/existing groups - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function ensureGenericUserGroupsExist(): array { - return $this->_groupHandler->ensureGenericUserGroupsExist(); - }//end ensureGenericUserGroupsExist() - - /** - * Gets organizational hierarchy information for a user - * - * @param string $username The username to get hierarchy for - * - * @return array Array containing hierarchy information - */ - public function getUserHierarchy(string $username): array { - return $this->_hierarchyHandler->getUserHierarchy($username); - }//end getUserHierarchy() - - /** - * Gets complete organizational structure - * - * @param string $organizationUuid The organization UUID - * - * @return array Array containing organizational structure - */ - public function getOrganizationStructure(string $organizationUuid): array { - return $this->_hierarchyHandler->getOrganizationStructure($organizationUuid); - }//end getOrganizationStructure() - - /** - * Handles contactpersoon updates, particularly role changes - * - * @param object $contactPersonObject The updated contactpersoon object - * @param object $oldContactPersonObject The previous contactpersoon object (optional) - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleContactpersoonUpdate(object $contactPersonObject, ?object $oldContactPersonObject = null): void { - $startTime = microtime(true); - - try { - $objectId = $contactPersonObject->getId(); - $this->_logger->info( - 'StackiqService: Starting contactpersoon update handling', - [ - 'objectId' => $objectId, - 'hasOldObject' => $oldContactPersonObject !== null, - 'timestamp' => date('Y-m-d H:i:s'), - ] - ); - - // Get current and old data for comparison. - $newData = $contactPersonObject->getObject(); - $oldData = []; - if ($oldContactPersonObject !== null) { - $oldData = $oldContactPersonObject->getObject(); - } - - $newRoles = $newData['roles'] ?? []; - $oldRoles = $oldData['roles'] ?? []; - - $this->_logger->debug( - 'StackiqService: Comparing roles for contactpersoon update', - [ - 'objectId' => $objectId, - 'newRoles' => $newRoles, - 'oldRoles' => $oldRoles, - 'newRolesType' => gettype($newRoles), - 'oldRolesType' => gettype($oldRoles), - ] - ); - - // Ensure both are arrays. - if (is_array($newRoles) === false) { - $newRoles = [$newRoles]; - $this->_logger->debug( - 'StackiqService: Converted newRoles to array', - [ - 'objectId' => $objectId, - 'newRoles' => $newRoles, - ] - ); - } - - if (is_array($oldRoles) === false) { - $oldRoles = [$oldRoles]; - $this->_logger->debug( - 'StackiqService: Converted oldRoles to array', - [ - 'objectId' => $objectId, - 'oldRoles' => $oldRoles, - ] - ); - } - - // For updates, we need to handle differently based on whether roles changed. - if ($newRoles !== $oldRoles) { - // Roles changed - use role-based group assignment instead of generic group assignment. - $this->_logger->info( - 'StackiqService: Roles changed for contactpersoon, using role-based group assignment', - [ - 'contactpersoonId' => $objectId, - 'oldRoles' => $oldRoles, - 'newRoles' => $newRoles, - 'addedRoles' => array_diff($newRoles, $oldRoles), - 'removedRoles' => array_diff($oldRoles, $newRoles), - ] - ); - - // Ensure user exists (but don't assign generic groups). - $username = $newData['username'] ?? ''; - if (empty($username) === true) { - // Generate username and create user if needed. - $result = $this->_contactPersonHandler->processContactpersoon($contactPersonObject, true); - if ($result === true) { - $updatedData = $contactPersonObject->getObject(); - $username = $updatedData['username'] ?? ''; - } - } - - if (empty($username) === false) { - $user = $this->_container->get(\OCP\IUserManager::class)->get($username); - if (empty($user) === false) { - // Use new organization type-based logic instead of old role-based logic. - $contactData = $contactPersonObject->getObject(); - $this->_contactPersonHandler->updateUserGroupsFromContactData($user, $contactData); - - $this->_logger->info( - 'StackiqService: Organization type-based group updates completed', - [ - 'username' => $username, - 'objectId' => $objectId, - 'newRoles' => $newRoles, - ] - ); - } else { - $this->_logger->warning( - 'StackiqService: User not found for role-based group updates', - [ - 'username' => $username, - 'objectId' => $objectId, - ] - ); - }//end if - } else { - $this->_logger->warning( - 'StackiqService: No username available for role-based group updates', - [ - 'objectId' => $objectId, - 'newData' => $newData, - ] - ); - }//end if - } else { - // No role changes - use standard processing (assigns generic groups). - $this->_logger->debug( - 'StackiqService: No role changes, using standard contactpersoon processing', - [ - 'objectId' => $objectId, - 'roles' => $newRoles, - ] - ); - - $result = $this->processContactpersoon(contactPersonObject: $contactPersonObject, isUpdate: true); - - $this->_logger->info( - 'StackiqService: Standard contactpersoon processing completed', - [ - 'objectId' => $objectId, - 'result' => $result, - 'processingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - }//end if - - $this->_logger->info( - 'StackiqService: Contactpersoon update handling completed', - [ - 'objectId' => $objectId, - 'totalProcessingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to handle contactpersoon update: ' . $e->getMessage(), - [ - 'objectId' => $contactPersonObject->getId(), - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), - 'processingTime' => round((microtime(true) - $startTime) * 1000, 2) . 'ms', - ] - ); - }//end try - }//end handleContactpersoonUpdate() - - /** - * Handles organization deletion - deactivates all users in the organization - * - * @param object $organizationObject The organization object being deleted - * - * @return void - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function handleOrganizationDeletion(object $organizationObject): void { - try { - $this->_logger->info( - 'StackiqService: Handling organization deletion', - [ - 'objectId' => $organizationObject->getId(), - ] - ); - - $objectData = $organizationObject->getObject(); - $organizationUuid = $objectData['id'] ?? $organizationObject->getId(); - - // Deactivate all users in this organization. - $this->deactivateUsersForOrganization(organizationUuid: $organizationUuid); - - $this->_logger->info( - 'StackiqService: Successfully handled organization deletion', - [ - 'organizationId' => $organizationUuid, - 'timestamp' => date('Y-m-d H:i:s'), - ] - ); - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to handle organization deletion: ' . $e->getMessage(), - [ - 'objectId' => $organizationObject->getId(), - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), - ] - ); - }//end try - }//end handleOrganizationDeletion() - - /** - * Activates all users in an organization when the organization becomes active - * - * @param string $organizationUuid The organization UUID - * - * @return void - */ - private function activateUsersForOrganization(string $organizationUuid): void { - try { - $this->_logger->info( - 'StackiqService: Activating users for organization', - [ - 'organizationUuid' => $organizationUuid, - ] - ); - - $objectService = $this->getObjectService(); - if ($objectService === null) { - $this->_logger->error('StackiqService: OpenRegister ObjectService not available'); - return; - } - - // Get all contactpersonen for this organization. - $ctx = $this->resolveVoorzieningenContext(schemaSlug: 'contactPerson', logContext: 'contactpersonen'); - if ($ctx === null) { - return; - } - - $registerId = $ctx['registerId']; - $contactPersonSchemaId = $ctx['schemaId']; - - // The findAll() signature is ($config, bool $_rbac, bool $_multitenancy). - // Register and schema belong inside the config's filters, as the - // sibling call above does -- passed positionally they landed on the - // two booleans, so this ran unscoped across every register with - // $_rbac set to a register id. - $contactpersonen = $objectService->findAll( - [ - 'filters' => [ - 'register' => $registerId, - 'schema' => $contactPersonSchemaId, - 'organisation' => $organizationUuid, - ], - ] - ); - - $userManager = $this->_container->get(\OCP\IUserManager::class); - $activatedCount = 0; - - foreach ($contactpersonen as $contactPerson) { - $contactData = $contactPerson->getObject(); - $username = $contactData['username'] ?? ''; - - if (empty($username) === false) { - $user = $userManager->get($username); - if ($user !== null && $user->isEnabled() === false) { - $user->setEnabled(true); - $activatedCount++; - - $this->_logger->info( - 'StackiqService: Activated user for organization', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - ] - ); - } - } - } - - $this->_logger->info( - 'StackiqService: Completed user activation for organization', - [ - 'organizationUuid' => $organizationUuid, - 'totalContactpersonen' => count($contactpersonen), - 'activatedUsers' => $activatedCount, - ] - ); - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to activate users for organization: ' . $e->getMessage(), - [ - 'organizationUuid' => $organizationUuid, - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), - ] - ); - }//end try - }//end activateUsersForOrganization() - - /** - * Deactivates all users in an organization when the organization becomes inactive - * - * @param string $organizationUuid The organization UUID - * - * @return void - */ - private function deactivateUsersForOrganization(string $organizationUuid): void { - try { - $this->_logger->info( - 'StackiqService: Deactivating users for organization', - [ - 'organizationUuid' => $organizationUuid, - ] - ); - - $objectService = $this->getObjectService(); - if ($objectService === null) { - $this->_logger->error('StackiqService: OpenRegister ObjectService not available'); - return; - } - - // Get all contactpersonen for this organization. - $ctx = $this->resolveVoorzieningenContext(schemaSlug: 'contactPerson', logContext: 'contactpersonen'); - if ($ctx === null) { - return; - } - - $registerId = $ctx['registerId']; - $contactPersonSchemaId = $ctx['schemaId']; - - // The findAll() signature is ($config, bool $_rbac, bool $_multitenancy). - // Register and schema belong inside the config's filters, as the - // sibling call above does -- passed positionally they landed on the - // two booleans, so this ran unscoped across every register with - // $_rbac set to a register id. - $contactpersonen = $objectService->findAll( - [ - 'filters' => [ - 'register' => $registerId, - 'schema' => $contactPersonSchemaId, - 'organisation' => $organizationUuid, - ], - ] - ); - - $userManager = $this->_container->get(\OCP\IUserManager::class); - $deactivatedCount = 0; - - foreach ($contactpersonen as $contactPerson) { - $contactData = $contactPerson->getObject(); - $username = $contactData['username'] ?? ''; - - if (empty($username) === false) { - $user = $userManager->get($username); - if ($user !== null && $user->isEnabled() === true) { - $user->setEnabled(false); - $deactivatedCount++; - - $this->_logger->info( - 'StackiqService: Deactivated user for organization', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - ] - ); - } - } - } - - $this->_logger->info( - 'StackiqService: Completed user deactivation for organization', - [ - 'organizationUuid' => $organizationUuid, - 'totalContactpersonen' => count($contactpersonen), - 'deactivatedUsers' => $deactivatedCount, - ] - ); - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to deactivate users for organization: ' . $e->getMessage(), - [ - 'organizationUuid' => $organizationUuid, - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), - ] - ); - }//end try - }//end deactivateUsersForOrganization() - - /** - * Checks if a contactpersoon username is in the organization's users list - * - * @param object $contactPersonObject The contactpersoon object - * - * @return bool True if the user should be added to the organization - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function shouldAddContactpersoonToOrganization(object $contactPersonObject): bool { - try { - $objectData = $contactPersonObject->getObject(); - $username = $objectData['username'] ?? ''; - $organizationUuid = $objectData['organisation'] ?? ''; - - if (empty($username) === true || empty($organizationUuid) === true) { - return false; - } - - $objectService = $this->getObjectService(); - if ($objectService === null) { - return false; - } - - // Get the organization object. - $ctx = $this->resolveVoorzieningenContext(schemaSlug: 'organization', logContext: 'organization'); - if ($ctx === null) { - return false; - } - - $registerId = $ctx['registerId']; - $organisationSchemaId = $ctx['schemaId']; - - try { - $organizationObject = $objectService->find($organizationUuid, [], false, $registerId, $organisationSchemaId); - $organizationData = $organizationObject->getObject(); - - // Check if the username is already in the organization's users. - $organizationUsers = $organizationData['users'] ?? []; - - if (is_array($organizationUsers) === true && in_array($username, $organizationUsers) === false) { - $this->_logger->info( - 'StackiqService: Contactpersoon should be added to organization', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - 'currentUsers' => $organizationUsers, - ] - ); - return true; - } - - return false; - } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - // Organization doesn't exist, so we can't add the user. - $this->_logger->warning( - 'StackiqService: Organization not found for contactpersoon', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - ] - ); - return false; - }//end try - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to check contactpersoon addition to org: ' . $e->getMessage(), - [ - 'objectId' => $contactPersonObject->getId(), - 'exception' => $e->getMessage(), - ] - ); - return false; - }//end try - }//end shouldAddContactpersoonToOrganization() - - /** - * Adds a contactpersoon username to the organization's users list - * - * @param object $contactPersonObject The contactpersoon object - * - * @return bool True if the user was successfully added - * @spec openspec/specs/softwarecatalogue-orchestration/spec.md - */ - public function addContactpersoonToOrganization(object $contactPersonObject): bool { - try { - $objectData = $contactPersonObject->getObject(); - $username = $objectData['username'] ?? ''; - $organizationUuid = $objectData['organisation'] ?? ''; - - if (empty($username) === true || empty($organizationUuid) === true) { - $this->_logger->warning( - 'StackiqService: Cannot add contactpersoon to org - missing username or org', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - ] - ); - return false; - } - - $objectService = $this->getObjectService(); - if ($objectService === null) { - $this->_logger->error('StackiqService: OpenRegister ObjectService not available'); - return false; - } - - // Get the organization object. - $ctx = $this->resolveVoorzieningenContext(schemaSlug: 'organization', logContext: 'organization'); - if ($ctx === null) { - return false; - } - - $registerId = $ctx['registerId']; - $organisationSchemaId = $ctx['schemaId']; - - try { - $organizationObject = $objectService->find($organizationUuid, [], false, $registerId, $organisationSchemaId); - $organizationData = $organizationObject->getObject(); - - // Add the username to the organization's users list. - $organizationUsers = $organizationData['users'] ?? []; - if (is_array($organizationUsers) === false) { - $organizationUsers = []; - } - - if (in_array($username, $organizationUsers) === false) { - $organizationUsers[] = $username; - $organizationData['users'] = $organizationUsers; - - // Update the organization object. - $updatedOrganization = $objectService->saveObject( - $organizationData, - [], - $registerId, - $organisationSchemaId, - $organizationUuid - ); - - $this->_logger->info( - 'StackiqService: Successfully added contactpersoon to organization', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - 'updatedUsers' => $organizationUsers, - ] - ); - }//end if - - $this->_logger->debug( - 'StackiqService: Contactpersoon already in organization', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - ] - ); - return true; - // Already there, consider it successful. - } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - $this->_logger->error( - 'StackiqService: Organization not found for contactpersoon', - [ - 'username' => $username, - 'organizationUuid' => $organizationUuid, - ] - ); - return false; - }//end try - } catch (\Exception $e) { - $this->_logger->error( - 'StackiqService: Failed to add contact person to organization: ' . $e->getMessage(), - [ - 'objectId' => $contactPersonObject->getId(), - 'exception' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - 'trace' => $e->getTraceAsString(), - ] - ); - return false; - }//end try - }//end addContactpersoonToOrganization() - - /** - * Ensures a contact person's username is in their organization's users array. - * NOTE: Dead method — retained only as implementation reference until the sync - * pipeline invocation point is wired; not called from any live code path. - * - * @param object $contactPersonObject The contact person object - * - * @SuppressWarnings(PHPMD.UnusedPrivateMethod) - * - * @return void - */ - private function ensureContactPersonInOrganization(object $contactPersonObject): void { - $contactData = $contactPersonObject->getObject(); - $email = $contactData['email'] ?? null; - $organization = $contactData['organization'] ?? null; - - if ($email === null || $organization === false) { - $this->_logger->info( - 'StackiqService: Contact person missing email or organisation', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'hasEmail' => empty($email) === false, - 'hasOrganisatie' => empty($organization) === false, - ] - ); - return; - } - - // Skip if the contact person is owned by the default organization. - $owner = $contactPersonObject->getOwner(); - if ($owner === 'system') { - $this->_logger->info( - 'StackiqService: Skipping contact person owned by system', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'username' => $email, - ] - ); - return; - } - - $this->_logger->info( - 'StackiqService: Ensuring contact person in organization', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'username' => $email, - 'organization' => $organization, - ] - ); - - try { - // Get the organization entity. - $organisationMapper = $this->getOrganisationMapper(); - if ($organisationMapper === null) { - $this->_logger->error( - 'StackiqService: OpenRegister OrganisationMapper not available for contact person', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'organization' => $organization, - ] - ); - return; - } - - $organisation = $organisationMapper->findByUuid($organization); - - if ($organisation === null) { - $this->_logger->error( - 'StackiqService: Organization entity not found for contact person', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'organization' => $organization, - ] - ); - return; - } - - // Check if the username is already in the organization's users array. - $currentUsers = $organisation->getUsers() ?? []; - if (in_array($email, $currentUsers) === true) { - $this->_logger->info( - 'StackiqService: Contact person already in organization', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'username' => $email, - 'organization' => $organization, - ] - ); - return; - } - - // Add the username to the organization's users array. - $currentUsers[] = $email; - $organisation->setUsers($currentUsers); - $organisationMapper->save($organisation); - - $this->_logger->info( - 'StackiqService: Successfully added contact person to organization', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'username' => $email, - 'organization' => $organization, - 'totalUsers' => count($currentUsers), - ] - ); - } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - // Organization entity doesn't exist yet - this can happen due to race conditions. - // Log and return gracefully, the organization sync will handle this later. - $this->_logger->warning( - 'StackiqService: Org entity not found (race condition), handled by org sync', - [ - 'contactPersonId' => $contactPersonObject->getId(), - 'username' => $email, - 'organization' => $organization, - 'message' => 'Expected during anonymous registration - org entity created after contacts', - ] - ); - return; - }//end try - }//end ensureContactPersonInOrganization() - - /** - * Resolves the voorzieningen register id + a schema id for a given - * object-type slug. - * - * Extracted from the repeated four-line "fetch SettingsService + - * getVoorzieningenRegisterId + getSchemaIdForObjectType + null/false - * guard" block that appeared in `activateUsersForOrganization()`, - * `deactivateUsersForOrganization()`, - * `shouldAddContactpersoonToOrganization()`, - * `addContactpersoonToOrganization()`, and - * `updateOrganizationReferences()`. Centralises the "missing config" - * log line so the per-method bodies no longer carry that boilerplate. - * - * W31 method-decomposition 2.6 — companion helper for the - * Stackiq subservice wiring. - * - * @param string $schemaSlug Object-type slug as understood by - * `SettingsService::getSchemaIdForObjectType()` - * (e.g. `contactpersoon`, `organisatie`). - * @param string $logContext Short human label for the missing-config - * log line (e.g. `contactpersonen`). - * - * @return array{registerId:int, schemaId:int}|null Null when either - * the voorzieningen register or the schema - * is unconfigured; the matched (registerId, - * schemaId) pair otherwise. - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2-6 - */ - private function resolveVoorzieningenContext(string $schemaSlug, string $logContext): ?array { - $settingsService = $this->_container->get(SettingsService::class); - $registerId = $settingsService->getVoorzieningenRegisterId(); - $schemaId = $settingsService->getSchemaIdForObjectType($schemaSlug); - - if ($registerId === null || $schemaId === null || $schemaId === false) { - $this->_logger->error( - 'StackiqService: Register or schema not configured for ' . $logContext - ); - return null; - } - - return [ - 'registerId' => $registerId, - 'schemaId' => $schemaId, - ]; - - }//end resolveVoorzieningenContext() -}//end class diff --git a/openspec/parity/capabilities.json b/openspec/parity/capabilities.json index c1e20d5d5..e299061db 100644 --- a/openspec/parity/capabilities.json +++ b/openspec/parity/capabilities.json @@ -8587,7 +8587,7 @@ "stackiq": "partial", "built": { "state": "built", - "evidence": "lib/Service/Stackiq/HierarchyHandler.php:79 ensureOrganizationBeheerder: when the organisation has no beheerder (OrganizationHandler getOrganizationBeheerders = members of group 'maintainer'), the user gets assignBeheerderRole (ContactPersonHandler.php:1368, adds group 'maintainer' :1398), and :130 setupManagerRelationships makes later users report to the primary beheerder; called only on the automatic path (lib/Service/StackiqService.php:247, lib/Service/ContactpersoonService.php:323), not by the manual convertToUser; ContactPersonHandler.php:641 the first contact is no longer added to the organisation admin groups (cab7bc32, 28 Sep); 'maintainer' members manage the organisation's members via lib/Controller/OrganisationMembersController.php", + "evidence": "lib/Service/Stackiq/HierarchyHandler.php:79 ensureOrganizationBeheerder: when the organisation has no beheerder (OrganizationHandler getOrganizationBeheerders = members of group 'maintainer'), the user gets assignBeheerderRole (ContactPersonHandler.php:1368, adds group 'maintainer' :1398), and :130 setupManagerRelationships makes later users report to the primary beheerder; called only on the automatic path (lib/Service/ContactpersoonService.php:323), not by the manual convertToUser; ContactPersonHandler.php:641 the first contact is no longer added to the organisation admin groups (cab7bc32, 28 Sep); 'maintainer' members manage the organisation's members via lib/Controller/OrganisationMembersController.php", "owner": "ConductionNL/stackiq", "change": "2026-05-31-retrofit-2026-05-26-sc-handlers", "readOn": "2026-10-07" @@ -8599,7 +8599,7 @@ "featureConfidence": "high", "note": "On automatic provisioning, the first user of an organisation joins the maintainer (beheerder) group, which may manage the organisation's members, and later users get that user as manager. The first contact no longer gets the organisation admin groups (switched off on 28 Sep), the manual Convert to User does not run this step, and it all depends on the email field the contact schema lacks.", "evidence": { - "stackiq": "lib/Service/Stackiq/HierarchyHandler.php:79 ensureOrganizationBeheerder: when the organisation has no beheerder (OrganizationHandler getOrganizationBeheerders = members of group 'maintainer'), the user gets assignBeheerderRole (ContactPersonHandler.php:1368, adds group 'maintainer' :1398), and :130 setupManagerRelationships makes later users report to the primary beheerder; called only on the automatic path (lib/Service/StackiqService.php:247, lib/Service/ContactpersoonService.php:323), not by the manual convertToUser; ContactPersonHandler.php:641 the first contact is no longer added to the organisation admin groups (cab7bc32, 28 Sep); 'maintainer' members manage the organisation's members via lib/Controller/OrganisationMembersController.php", + "stackiq": "lib/Service/Stackiq/HierarchyHandler.php:79 ensureOrganizationBeheerder: when the organisation has no beheerder (OrganizationHandler getOrganizationBeheerders = members of group 'maintainer'), the user gets assignBeheerderRole (ContactPersonHandler.php:1368, adds group 'maintainer' :1398), and :130 setupManagerRelationships makes later users report to the primary beheerder; called only on the automatic path (lib/Service/ContactpersoonService.php:323), not by the manual convertToUser; ContactPersonHandler.php:641 the first contact is no longer added to the organisation admin groups (cab7bc32, 28 Sep); 'maintainer' members manage the organisation's members via lib/Controller/OrganisationMembersController.php", "topdesk": "unknown: first-user administrator rules are not described; searched the full-text search index of docs.topdesk.com (https://docs.topdesk.com/en/js/fuzzydata.js, 987 pages) (read 2026-09-26)", "vng-softwarecatalogus": "https://www.softwarecatalogus.nl/node/17042: \"Vanuit de gemeente is minimaal één gebruiker aangewezen als beheerder. Deze gebruiker kan nieuwe accounts aanmaken voor collega's\" (read 2026-09-26). Reached on: Gebruikersbeheer.", "bluedolphin": "unknown: docs searched at https://help.bluedolphin.io/en/; making an organisation's first user its administrator is not documented (read 2026-09-26)", diff --git a/openspec/specs/method-decomposition/spec.md b/openspec/specs/method-decomposition/spec.md index e477477dc..79488a11d 100644 --- a/openspec/specs/method-decomposition/spec.md +++ b/openspec/specs/method-decomposition/spec.md @@ -231,32 +231,21 @@ thresholds. ### Requirement: REQ-DECOMP-004 StackiqService Decomposition -`lib/Service/StackiqService.php` (20 suppressions) MUST be -decomposed by splitting into focused service classes. This core service handles -VNG Software Catalogus API synchronization with multiple concerns: API -communication, data mapping, conflict resolution, and progress tracking. +`lib/Service/StackiqService.php` and the sub-services extracted from it +(`lib/Service/Stackiq/ApiClient.php`, `DataMapper.php`, `ConflictResolver.php`) +SHALL NOT exist: nothing in the app referenced them, so they were removed +rather than decomposed further. Organisation, contact person and usage +processing lives in OrganisatieService, ContactpersoonService, +OrganizationSyncService and the `lib/Service/Stackiq/` handlers. -#### Scenario: Concerns extracted into sub-services +#### Scenario: The unused god-class is gone -- GIVEN `StackiqService` has 7+ class-level suppressions indicating an oversized god-class -- WHEN decomposed -- THEN API communication methods MUST move to `lib/Service/Stackiq/ApiClient.php` -- AND data mapping MUST move to `lib/Service/Stackiq/DataMapper.php` -- AND conflict resolution MUST move to `lib/Service/Stackiq/ConflictResolver.php` - -#### Scenario: Existing handler pattern reused - -- GIVEN the codebase already has a `Stackiq/` subdirectory with handler classes -- WHEN additional extraction follows this pattern -- THEN consistency MUST be maintained -- AND the existing handler injection pattern (constructor DI, delegation from parent service) MUST be reused without modification +- GIVEN no class, registration, route, job, command or other app referenced `StackiqService` +- WHEN the app is built +- THEN `lib/Service/StackiqService.php`, `ApiClient.php`, `DataMapper.php` and `ConflictResolver.php` MUST NOT exist +- AND the `lib/Service/Stackiq/` handlers MUST keep their constructor DI from `Application.php` -#### Scenario: Progress tracking isolated to ProgressTracker - -- GIVEN `StackiqService` methods contain progress tracking code interleaved with business logic -- WHEN decomposed -- THEN progress tracking calls MUST be isolated to the existing `lib/Service/ProgressTracker.php` wrapper -- AND business methods MUST contain no inline progress tracking code +@e2e exclude Code-structure requirement with no runtime behaviour; checked by the file listing and PHPUnit. ### Requirement: REQ-DECOMP-005 SettingsService Decomposition diff --git a/phpmd.baseline.xml b/phpmd.baseline.xml index 0ad78b147..e72c4c30c 100644 --- a/phpmd.baseline.xml +++ b/phpmd.baseline.xml @@ -6,6 +6,4 @@ - - diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index eb8499ab5..858378e72 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -445,31 +445,6 @@ parameters: count: 1 path: lib/Service/Stackiq/GroupHandler.php - - - message: "#^Method OCA\\\\Stackiq\\\\Service\\\\StackiqService\\:\\:activateContactpersonenForOrganization\\(\\) is unused\\.$#" - count: 1 - path: lib/Service/StackiqService.php - - - - message: "#^Method OCA\\\\Stackiq\\\\Service\\\\StackiqService\\:\\:activateUsersForOrganization\\(\\) is unused\\.$#" - count: 1 - path: lib/Service/StackiqService.php - - - - message: "#^Method OCA\\\\Stackiq\\\\Service\\\\StackiqService\\:\\:ensureContactPersonInOrganization\\(\\) is unused\\.$#" - count: 1 - path: lib/Service/StackiqService.php - - - - message: "#^Property OCA\\\\Stackiq\\\\Service\\\\StackiqService\\:\\:\\$_organizationHandler is never read, only written\\.$#" - count: 1 - path: lib/Service/StackiqService.php - - - - message: "#^Property OCA\\\\Stackiq\\\\Service\\\\StackiqService\\:\\:\\$appName is never read, only written\\.$#" - count: 1 - path: lib/Service/StackiqService.php - - message: "#^Property OCA\\\\Stackiq\\\\Service\\\\SymfonyEmailService\\:\\:\\$config is never read, only written\\.$#" count: 1 diff --git a/tests/Unit/EventListener/StackiqEventListenerTest.php b/tests/Unit/EventListener/StackiqEventListenerTest.php deleted file mode 100644 index 42215db86..000000000 --- a/tests/Unit/EventListener/StackiqEventListenerTest.php +++ /dev/null @@ -1,387 +0,0 @@ - - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * @version 1.0.0 - */ -class StackiqEventListenerTest extends TestCase { - /** - * Mock of the StackiqService - * - * @var StackiqService|MockObject - */ - private StackiqService|MockObject $softwareCatalogueService; - - /** - * Mock of the LoggerInterface - * - * @var LoggerInterface|MockObject - */ - private LoggerInterface|MockObject $logger; - - /** - * The event listener instance under test - * - * @var StackiqEventListener - */ - private StackiqEventListener $eventListener; - - /** - * Set up the test environment before each test - * - * @return void - */ - protected function setUp(): void { - parent::setUp(); - - // The StackiqEventListener was refactored after these tests - // were written: handle() now dispatches via SettingsService schema-id - // lookups (handleObjectCreated/Updated/Deleted private dispatchers) - // rather than the direct handleNewContact/handleNewGebruiker/etc. - // service methods these tests assert. The tests need to be rewritten - // against the new dispatch flow and additional collaborators - // (SettingsService, AppManager, IUserManager, etc.) — tracked as a - // follow-up. See https://github.com/ConductionNL/stackiq - $this->markTestSkipped( - 'Stale against current StackiqEventListener — needs ' - . 'rewrite against new SettingsService-driven dispatch. ' - . 'Tracked as follow-up issue.' - ); - - $this->softwareCatalogueService = $this->createMock(StackiqService::class); - $this->logger = $this->createMock(LoggerInterface::class); - - $this->eventListener = new StackiqEventListener( - $this->softwareCatalogueService, - $this->logger - ); - } - - /** - * Test handling organization creation event - * - * @return void - */ - public function testHandleOrganizationCreatedEvent(): void { - // Create mock ObjectEntity for organization (schema ID 1) - $organization = $this->createMock(ObjectEntity::class); - $organization->method('getSchema')->willReturn(1); - - // Create ObjectCreatedEvent - $event = new ObjectCreatedEvent($organization); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('handleNewOrganization') - ->with($organization); - - $this->softwareCatalogueService - ->expects($this->once()) - ->method('sendOrganizationWelcomeEmail') - ->with($organization); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling contact creation event - * - * @return void - */ - public function testHandleContactCreatedEvent(): void { - // Create mock ObjectEntity for contact (schema ID 2) - $contact = $this->createMock(ObjectEntity::class); - $contact->method('getSchema')->willReturn(2); - - // Create ObjectCreatedEvent - $event = new ObjectCreatedEvent($contact); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('handleNewContact') - ->with($contact); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling gebruiker (user) creation event - * - * @return void - */ - public function testHandleGebruikerCreatedEvent(): void { - // Create mock ObjectEntity for gebruiker (schema ID 3) - $user = $this->createMock(ObjectEntity::class); - $user->method('getSchema')->willReturn(3); - - // Create ObjectCreatedEvent - $event = new ObjectCreatedEvent($user); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('handleNewGebruiker') - ->with($user); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling contact update event - * - * @return void - */ - public function testHandleContactUpdatedEvent(): void { - // Create mock ObjectEntity for contact (schema ID 2) - $newContact = $this->createMock(ObjectEntity::class); - $newContact->method('getSchema')->willReturn(2); - - $oldContact = $this->createMock(ObjectEntity::class); - - // Create ObjectUpdatedEvent - $event = new ObjectUpdatedEvent($newContact, $oldContact); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('handleContactUpdate') - ->with($newContact); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling gebruiker (user) update event - * - * @return void - */ - public function testHandleGebruikerUpdatedEvent(): void { - // Create mock ObjectEntity for gebruiker (schema ID 3) - $newUser = $this->createMock(ObjectEntity::class); - $newUser->method('getSchema')->willReturn(3); - - $oldUser = $this->createMock(ObjectEntity::class); - - // Create ObjectUpdatedEvent - $event = new ObjectUpdatedEvent($newUser, $oldUser); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('handleGebruikerUpdate') - ->with($newUser, $oldUser); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling contact deletion event - * - * @return void - */ - public function testHandleContactDeletedEvent(): void { - // Create mock ObjectEntity for contact (schema ID 2) - $contact = $this->createMock(ObjectEntity::class); - $contact->method('getSchema')->willReturn(2); - - // Create ObjectDeletedEvent - $event = new ObjectDeletedEvent($contact); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('handleContactDeletion') - ->with($contact); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling gebruiker (user) deletion event - should block user - * - * @return void - */ - public function testHandleGebruikerDeletedEvent(): void { - // Create mock ObjectEntity for gebruiker (schema ID 3) - $user = $this->createMock(ObjectEntity::class); - $user->method('getSchema')->willReturn(3); - - // Create ObjectDeletedEvent - $event = new ObjectDeletedEvent($user); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('blockUserForGebruiker') - ->with($user); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling gebruiker (user) locking event - * - * @return void - */ - public function testHandleGebruikerLockedEvent(): void { - // Create mock ObjectEntity for gebruiker (schema ID 3) - $user = $this->createMock(ObjectEntity::class); - $user->method('getSchema')->willReturn(3); - - // Create ObjectLockedEvent - $event = new ObjectLockedEvent($user); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('temporarilyBlockUserForGebruiker') - ->with($user); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * Test handling gebruiker (user) unlocking event - * - * @return void - */ - public function testHandleGebruikerUnlockedEvent(): void { - // Create mock ObjectEntity for gebruiker (schema ID 3) - $user = $this->createMock(ObjectEntity::class); - $user->method('getSchema')->willReturn(3); - - // Create ObjectUnlockedEvent - $event = new ObjectUnlockedEvent($user); - - // Set expectations for service calls - $this->softwareCatalogueService - ->expects($this->once()) - ->method('restoreUserAccessForGebruiker') - ->with($user); - - // Handle the event - $this->eventListener->handle($event); - } - - /** - * An object-reverted event reaches the listener without driving - * StackiqService. - * - * The two cases that stood here asserted - * `syncUserWithRevertedContact()` / `updateUserFromRevertedGebruiker()` - * were called once each. `StackiqEventListener` does not call - * them — it never did — and both service methods were log-only stubs with - * no caller, removed with this change. The assertion that survives is the - * true one: the revert path must not blow up, and must not reach a - * capability the app does not implement. - * - * @return void - */ - public function testHandleRevertedEventDoesNotDriveTheCatalogueService(): void { - $contact = $this->createMock(ObjectEntity::class); - $contact->method('getSchema')->willReturn(2); - - $this->softwareCatalogueService - ->expects($this->never()) - ->method('handleContactUpdate'); - - $this->eventListener->handle( - new ObjectRevertedEvent($contact, new \DateTime('2024-01-01 12:00:00')) - ); - } - - /** - * Test handling events with null objects - * - * @return void - */ - public function testHandleEventWithUnmatchedSchema(): void { - // Create a mock ObjectEntity with a schema that doesn't match any configured schema - $object = $this->createMock(ObjectEntity::class); - $object->method('getSchema')->willReturn(999999); - $object->method('getUuid')->willReturn('test-uuid'); - $object->method('getRegister')->willReturn(1); - - // Create ObjectCreatedEvent with a valid object but unmatched schema - $event = new ObjectCreatedEvent($object); - - // No service methods should be called since schema doesn't match - $this->softwareCatalogueService - ->expects($this->never()) - ->method($this->anything()); - - // Handle the event - should return early since schema doesn't match - $this->eventListener->handle($event); - } - - /** - * Test exception handling during event processing - * - * @return void - */ - public function testExceptionHandlingDuringEventProcessing(): void { - // Create mock ObjectEntity for organization (schema ID 1) - $organization = $this->createMock(ObjectEntity::class); - $organization->method('getSchema')->willReturn(1); - - // Create ObjectCreatedEvent - $event = new ObjectCreatedEvent($organization); - - // Mock service to throw exception - $exception = new \Exception('Service error'); - $this->softwareCatalogueService - ->method('handleNewOrganization') - ->willThrowException($exception); - - // Expect logger to be called with error - $this->logger - ->expects($this->once()) - ->method('error') - ->with( - 'Failed to handle new organization: Service error', - [ - 'exception' => $exception, - 'object' => $organization - ] - ); - - // Handle the event - $this->eventListener->handle($event); - } -} diff --git a/tests/Unit/Service/StackiqServiceDecompositionTest.php b/tests/Unit/Service/StackiqServiceDecompositionTest.php deleted file mode 100644 index ed1c7bbb6..000000000 --- a/tests/Unit/Service/StackiqServiceDecompositionTest.php +++ /dev/null @@ -1,202 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2-6 - */ - -declare(strict_types=1); - -namespace OCA\Stackiq\Tests\Unit\Service; - -use OCA\Stackiq\Service\SettingsService; -use OCA\Stackiq\Service\StackiqService; -use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; -use Psr\Log\LoggerInterface; - -/** - * Tests the private resolveVoorzieningenContext helper via reflection. - * - * The helper is exercised by mocking the inner SettingsService that the - * service pulls from its container, then asserting that: - * - a fully-configured pair is returned as a tuple-array, - * - a missing register or missing schema collapses to null + an error - * log line. - * - * @category Test - * @package OCA\Stackiq\Tests\Unit\Service - * - * @spec openspec/changes/method-decomposition/tasks.md#task-2-6 - */ -class StackiqServiceDecompositionTest extends TestCase { - - /** - * Builds a partially-mocked StackiqService whose container - * resolves SettingsService to a stub we control. Constructor is - * skipped because we only exercise one private helper. - * - * @param SettingsService $settings Stub settings service. - * @param LoggerInterface $logger Logger spy (assertions on - * error logging). - * - * @return StackiqService - */ - private function buildService(SettingsService $settings, LoggerInterface $logger): StackiqService { - $service = (new \ReflectionClass(StackiqService::class)) - ->newInstanceWithoutConstructor(); - - $container = $this->createMock(ContainerInterface::class); - $container->method('get') - ->willReturnCallback( - function (string $class) use ($settings) { - if ($class === SettingsService::class) { - return $settings; - } - - return null; - } - ); - - $reflection = new \ReflectionClass($service); - - $containerProp = $reflection->getProperty('_container'); - $containerProp->setAccessible(true); - $containerProp->setValue($service, $container); - - $loggerProp = $reflection->getProperty('_logger'); - $loggerProp->setAccessible(true); - $loggerProp->setValue($service, $logger); - - return $service; - }//end buildService() - - /** - * Invokes the private resolveVoorzieningenContext helper on the - * given service. - * - * @param StackiqService $service Service under test. - * @param string $schemaSlug Schema slug. - * @param string $logContext Log context. - * - * @return array|null - */ - private function callResolver(StackiqService $service, string $schemaSlug, string $logContext): ?array { - $reflection = new \ReflectionMethod($service, 'resolveVoorzieningenContext'); - $reflection->setAccessible(true); - return $reflection->invoke($service, $schemaSlug, $logContext); - }//end callResolver() - - /** - * When both the voorzieningen register and the requested schema - * resolve, the helper returns the (registerId, schemaId) tuple. - * - * @return void - */ - public function testResolveVoorzieningenContextReturnsTupleWhenConfigured(): void { - $settings = $this->createMock(SettingsService::class); - $settings->method('getVoorzieningenRegisterId')->willReturn(3); - $settings->method('getSchemaIdForObjectType')->with('contactPerson')->willReturn(11); - - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never())->method('error'); - - $service = $this->buildService($settings, $logger); - $result = $this->callResolver($service, 'contactPerson', 'contactpersonen'); - - $this->assertSame( - [ - 'registerId' => 3, - 'schemaId' => 11, - ], - $result - ); - - }//end testResolveVoorzieningenContextReturnsTupleWhenConfigured() - - /** - * When the voorzieningen register is unconfigured (null), the - * helper returns null and logs the missing-config error. - * - * @return void - */ - public function testResolveVoorzieningenContextReturnsNullWhenRegisterMissing(): void { - $settings = $this->createMock(SettingsService::class); - $settings->method('getVoorzieningenRegisterId')->willReturn(null); - $settings->method('getSchemaIdForObjectType')->willReturn(11); - - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once()) - ->method('error') - ->with($this->stringContains('Register or schema not configured for contactpersonen')); - - $service = $this->buildService($settings, $logger); - $this->assertNull($this->callResolver($service, 'contactPerson', 'contactpersonen')); - - }//end testResolveVoorzieningenContextReturnsNullWhenRegisterMissing() - - /** - * When the requested schema is unconfigured (null), the helper - * returns null and logs an error mentioning the supplied log - * context. - * - * @return void - */ - public function testResolveVoorzieningenContextReturnsNullWhenSchemaMissing(): void { - $settings = $this->createMock(SettingsService::class); - $settings->method('getVoorzieningenRegisterId')->willReturn(3); - $settings->method('getSchemaIdForObjectType')->willReturn(null); - - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once()) - ->method('error') - ->with($this->stringContains('organization')); - - $service = $this->buildService($settings, $logger); - $this->assertNull($this->callResolver($service, 'organization', 'organization')); - - }//end testResolveVoorzieningenContextReturnsNullWhenSchemaMissing() - - /** - * The helper's schema-id guard treats the legacy `false` sentinel - * the same as `null` (the call sites in - * `StackiqService` historically checked for both shapes). - * SettingsService::getSchemaIdForObjectType() is typed `?int` so we - * cover the runtime guard by asserting the documented behavior in - * the source — the live call site already passed phpunit on the - * three real return values (positive int, null) above; the `false` - * branch is defensive plumbing per ADR-022. - * - * @return void - */ - public function testResolveVoorzieningenContextLogsContextLabelOnFailure(): void { - $settings = $this->createMock(SettingsService::class); - $settings->method('getVoorzieningenRegisterId')->willReturn(null); - $settings->method('getSchemaIdForObjectType')->willReturn(11); - - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once()) - ->method('error') - ->with($this->stringContains('aangeboden gebruik')); - - $service = $this->buildService($settings, $logger); - $this->assertNull($this->callResolver($service, 'usage', 'aangeboden gebruik')); - - }//end testResolveVoorzieningenContextLogsContextLabelOnFailure() - -}//end class diff --git a/tests/Unit/Service/StackiqServiceOrganisationMapperTest.php b/tests/Unit/Service/StackiqServiceOrganisationMapperTest.php deleted file mode 100644 index 52aec6e6b..000000000 --- a/tests/Unit/Service/StackiqServiceOrganisationMapperTest.php +++ /dev/null @@ -1,208 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * @link https://github.com/ConductionNL/stackiq - * - * @spec openspec/specs/method-decomposition/spec.md - * - * SPDX-FileCopyrightText: 2026 Conduction B.V. - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace OCA\Stackiq\Tests\Unit\Service; - -use OCA\OpenRegister\Db\OrganisationMapper; -use OCA\Stackiq\Service\StackiqService; -use OCP\App\IAppManager; -use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; -use Psr\Log\LoggerInterface; - -/** - * THE INVARIANT UNDER TEST — OpenRegister is an OPTIONAL capability here. - * - * Eight call sites in StackiqService used to resolve - * `OCA\OpenRegister\Db\OrganisationMapper` from the container inline, so on an - * instance without OpenRegister each one let a raw container exception escape. - * They now go through `getOrganisationMapper()`, which answers null instead — - * and every one of those call sites has an explicit not-available branch that - * is only correct if this accessor really does degrade. - * - * That makes the three arms below the contract the call sites are written - * against, not incidental coverage: - * - * - OpenRegister not enabled -> null, and the container is never asked - * - enabled and resolvable -> the mapper itself - * - enabled but resolution throws -> null, and the failure is logged - * - * The middle arm is the positive control. Without it, an accessor that - * returned null unconditionally would satisfy both null assertions while - * silently disabling every organisation-membership path in the app. - */ -final class StackiqServiceOrganisationMapperTest extends TestCase { - - /** - * Build a StackiqService with only the three properties this - * accessor reads, seeded by reflection. - * - * The constructor is skipped deliberately: it takes the app's full - * dependency set and none of it is reachable from this method. Note that - * reading an UNINITIALISED typed property is an Error rather than a null, - * so every property the method touches must be seeded here — that is why - * `_appManager` is seeded even on the arm that never reaches the container. - * - * @param IAppManager $appManager The app manager double. - * @param ContainerInterface $container The container double. - * @param LoggerInterface $logger The logger double. - * - * @return StackiqService The service under test. - */ - private function buildService( - IAppManager $appManager, - ContainerInterface $container, - LoggerInterface $logger, - ): StackiqService { - $service = (new \ReflectionClass(StackiqService::class)) - ->newInstanceWithoutConstructor(); - - $reflection = new \ReflectionClass($service); - - foreach ( - [ - '_appManager' => $appManager, - '_container' => $container, - '_logger' => $logger, - ] as $name => $value - ) { - $property = $reflection->getProperty($name); - $property->setAccessible(true); - $property->setValue($service, $value); - } - - return $service; - }//end buildService() - - /** - * Invoke the private accessor. - * - * @param StackiqService $service The service under test. - * - * @return OrganisationMapper|null The resolved mapper, or null. - */ - private function callAccessor(StackiqService $service): ?OrganisationMapper { - $method = new \ReflectionMethod($service, 'getOrganisationMapper'); - $method->setAccessible(true); - return $method->invoke($service); - }//end callAccessor() - - /** - * OpenRegister disabled: null, and the container is never consulted — - * asking it would be the unguarded lookup this accessor exists to replace. - * - * @return void - */ - public function testReturnsNullAndNeverAsksTheContainerWhenOpenRegisterIsDisabled(): void { - $appManager = $this->createMock(IAppManager::class); - $appManager->method('isEnabledForUser')->willReturn(false); - - $container = $this->createMock(ContainerInterface::class); - $container->expects($this->never())->method('get'); - - $service = $this->buildService( - appManager: $appManager, - container: $container, - logger: $this->createMock(LoggerInterface::class) - ); - - $this->assertNull( - $this->callAccessor($service), - 'With OpenRegister disabled the accessor must answer null so callers take their not-available branch.' - ); - - }//end testReturnsNullAndNeverAsksTheContainerWhenOpenRegisterIsDisabled() - - /** - * THE POSITIVE CONTROL. With OpenRegister enabled and resolvable, the - * accessor must hand back the mapper itself — otherwise the two null - * assertions here are satisfied by an accessor that never works. - * - * @return void - */ - public function testReturnsTheMapperWhenOpenRegisterIsAvailable(): void { - $appManager = $this->createMock(IAppManager::class); - $appManager->method('isEnabledForUser')->willReturn(true); - - $mapper = $this->createMock(OrganisationMapper::class); - - $container = $this->createMock(ContainerInterface::class); - $container->expects($this->once()) - ->method('get') - ->with('OCA\\OpenRegister\\Db\\OrganisationMapper') - ->willReturn($mapper); - - $service = $this->buildService( - appManager: $appManager, - container: $container, - logger: $this->createMock(LoggerInterface::class) - ); - - $this->assertSame( - $mapper, - $this->callAccessor($service), - 'The accessor must return the resolved mapper, not merely a non-null value.' - ); - - }//end testReturnsTheMapperWhenOpenRegisterIsAvailable() - - /** - * A failed resolution DEGRADES: null plus a logged error, never an escaping - * container exception. This is the arm the eight call sites depend on — and - * it is also what makes the lookup legible as an optional capability rather - * than an unconditional dependency. - * - * @return void - */ - public function testDegradesToNullAndLogsWhenResolutionThrows(): void { - $appManager = $this->createMock(IAppManager::class); - $appManager->method('isEnabledForUser')->willReturn(true); - - $container = $this->createMock(ContainerInterface::class); - $container->method('get')->willThrowException(new \RuntimeException('no such service')); - - $logged = []; - $logger = $this->createMock(LoggerInterface::class); - $logger->method('error')->willReturnCallback( - function (string $message) use (&$logged): void { - $logged[] = $message; - } - ); - - $service = $this->buildService( - appManager: $appManager, - container: $container, - logger: $logger - ); - - $this->assertNull( - $this->callAccessor($service), - 'A container failure must degrade to null rather than escape to the caller.' - ); - - $this->assertNotEmpty( - $logged, - 'Degrading silently would hide a broken OpenRegister install behind an ordinary not-available branch.' - ); - $this->assertStringContainsString('no such service', $logged[0]); - - }//end testDegradesToNullAndLogsWhenResolutionThrows() - -}//end class