diff --git a/appinfo/info.xml b/appinfo/info.xml index 1f48c5e704..3fd175ad83 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -352,6 +352,7 @@ Vrij en open source onder de EUPL-licentie. OCA\OpenRegister\Command\EncryptFieldCommand OCA\OpenRegister\Command\DedupeRegistersCommand + OCA\OpenRegister\Command\AdoptLeafOrganisationsCommand OCA\OpenRegister\Command\PruneRetiredSchemasCommand OCA\OpenRegister\Command\RelinkRegisterSchemasCommand OCA\OpenRegister\Command\ReconcileMagicTablesCommand diff --git a/lib/Command/AdoptLeafOrganisationsCommand.php b/lib/Command/AdoptLeafOrganisationsCommand.php new file mode 100644 index 0000000000..53d2aa4d1e --- /dev/null +++ b/lib/Command/AdoptLeafOrganisationsCommand.php @@ -0,0 +1,591 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://OpenRegister.app + * + * SPDX-License-Identifier: EUPL-1.2 + * SPDX-FileCopyrightText: 2026 Conduction B.V. + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Command; + +use DateTime; +use OCA\OpenRegister\Db\Organisation; +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\OpenRegister\Service\ObjectService; +use Psr\Log\LoggerInterface; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; +use Throwable; + +/** + * Adopt a leaf app's organisation objects into OpenRegister's Organisation. + * + * @spec openspec/changes/consolidate-organisation-on-or/tasks.md#5-leaf-app-consolidation + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) The spread is the point: each + * branch is one rule about what an adoption keeps, merges or drops, and the + * class is at 53 against a threshold of 50. Collapsing branches to satisfy the + * number would hide the rules rather than simplify them, and every one of them + * is pinned by its own test. + */ +class AdoptLeafOrganisationsCommand extends Command { + /** + * The legal identifiers a match may be made on, in precedence order. + * + * A name is deliberately absent. Two organisations sharing a name are + * routine; two sharing an OIN are the same body. + * + * @var array + */ + private const LEGAL_IDENTIFIERS = ['oin', 'rsin', 'kvk']; + + /** + * Leaf property name to Organisation setter suffix. + * + * Only properties Organisation actually declares are listed. Anything the + * leaf schema carries beyond these is reported rather than dropped quietly, + * because a property OR does not declare is a property OR discards. + * + * @var array + */ + private const FIELD_MAP = [ + 'name' => 'Name', + 'summary' => 'Summary', + 'description' => 'Description', + 'oin' => 'Oin', + 'tooi' => 'Tooi', + 'rsin' => 'Rsin', + 'kvk' => 'Kvk', + 'pki' => 'Pki', + 'image' => 'Image', + 'type' => 'Type', + 'status' => 'Status', + 'registrationStatus' => 'RegistrationStatus', + ]; + + /** + * Wire the mappers and the object reader. + * + * @param OrganisationMapper $organisationMapper The Organisation mapper. + * @param ObjectService $objectService Reader for the leaf objects. + * @param LoggerInterface $logger Logger. + * + * @return void + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public function __construct( + private readonly OrganisationMapper $organisationMapper, + private readonly ObjectService $objectService, + private readonly LoggerInterface $logger, + ) { + parent::__construct(); + }//end __construct() + + /** + * Define command name, description, and options. + * + * @return void + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + protected function configure(): void { + $this->setName(name: 'openregister:organisations:adopt') + ->setDescription( + 'Adopt a leaf app\'s own organisation objects into OpenRegister\'s Organisation, ' + . 'preserving each uuid and recording a merge where the same legal entity already exists.' + ) + ->addOption( + 'register', + null, + InputOption::VALUE_REQUIRED, + 'The register slug holding the leaf organisation objects (for example `publication`).' + ) + ->addOption( + 'schema', + null, + InputOption::VALUE_REQUIRED, + 'The leaf schema slug to adopt from. Defaults to `organization`.', + 'organization' + ) + ->addOption( + 'apply', + null, + InputOption::VALUE_NONE, + 'Actually write. Without this flag the command reports what it WOULD do.' + ); + }//end configure() + + /** + * Read the leaf rows and adopt each one. + * + * @param InputInterface $input Console input. + * @param OutputInterface $output Console output stream. + * + * @return int Symfony command exit code. + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.NPathComplexity) + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + protected function execute(InputInterface $input, OutputInterface $output): int { + $registerSlug = (string)$input->getOption('register'); + $schemaSlug = (string)$input->getOption('schema'); + $dryRun = ((bool)$input->getOption('apply') === false); + + if ($registerSlug === '') { + $output->writeln('--register is required.'); + return Command::FAILURE; + } + + if ($dryRun === true) { + $output->writeln( + 'Running in DRY-RUN mode — nothing will be written. ' + . 'Re-run with --apply to adopt.' + ); + } + + try { + $rows = $this->objectService->searchObjectsBySlug( + $registerSlug, + $schemaSlug, + ['_limit' => 5000], + false, + false + ); + } catch (Throwable $e) { + $output->writeln(sprintf('Could not read %s/%s: %s', $registerSlug, $schemaSlug, $e->getMessage())); + return Command::FAILURE; + } + + if (is_array($rows) === false) { + $output->writeln('The object reader returned a count rather than rows.'); + return Command::FAILURE; + } + + $existing = $this->existingOrganisations(); + $adopted = 0; + $merged = 0; + $skipped = 0; + $failed = 0; + + foreach ($rows as $row) { + $outcome = $this->adoptRow( + row: $row, + existing: $existing, + dryRun: $dryRun, + output: $output + ); + + $adopted += $outcome['adopted']; + $merged += $outcome['merged']; + $skipped += $outcome['skipped']; + $failed += $outcome['failed']; + }//end foreach + + $suffix = ''; + if ($dryRun === true) { + $suffix = ' (dry run — nothing written)'; + } + + $output->writeln( + sprintf( + 'Done. Adopted=%d (of which merged=%d), skipped=%d, failed=%d%s', + $adopted, + $merged, + $skipped, + $failed, + $suffix + ) + ); + + $this->logger->info( + 'OpenRegister: adopted leaf organisations', + [ + 'register' => $registerSlug, + 'schema' => $schemaSlug, + 'adopted' => $adopted, + 'merged' => $merged, + 'skipped' => $skipped, + 'failed' => $failed, + 'dryRun' => $dryRun, + ] + ); + + if ($failed > 0) { + return Command::FAILURE; + } + + return Command::SUCCESS; + }//end execute() + + /** + * Adopt one leaf row, reporting what happened to it. + * + * @param mixed $row The row as the reader returned it. + * @param array> $existing Organisations already on the instance, + * keyed by uuid. It grows as rows are + * adopted, so a legal identifier appearing + * twice within one run merges the second + * occurrence too. + * @param bool $dryRun Whether to report rather than write. + * @param OutputInterface $output Console output stream. + * + * @return array{adopted:int, merged:int, skipped:int, failed:int} The tally for this row. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + private function adoptRow(mixed $row, array &$existing, bool $dryRun, OutputInterface $output): array { + $none = ['adopted' => 0, 'merged' => 0, 'skipped' => 0, 'failed' => 0]; + + $fields = self::toFields(row: $row); + $uuid = (string)($fields['uuid'] ?? ''); + + if ($uuid === '') { + $output->writeln(' SKIP a row with no uuid: it has no idempotency key.'); + return array_merge($none, ['skipped' => 1]); + } + + if (isset($existing[$uuid]) === true) { + $output->writeln(sprintf(' SKIP %s: already adopted.', $uuid)); + return array_merge($none, ['skipped' => 1]); + } + + $target = self::findMergeTarget(row: $fields, existing: array_values($existing)); + + $mergeNote = ''; + $mergedCount = 0; + if ($target !== null) { + $mergeNote = sprintf(' -> merges into %s', $target['uuid']); + $mergedCount = 1; + } + + $output->writeln( + sprintf('%s (%s)%s', $uuid, (string)($fields['name'] ?? 'unnamed'), $mergeNote) + ); + + $this->reportUndeclared(fields: $fields, output: $output); + + if ($dryRun === true) { + $output->writeln(' WOULD ADOPT'); + return $none; + } + + try { + $saved = $this->organisationMapper->insert( + self::buildOrganisation(fields: $fields, mergeTarget: $target) + ); + } catch (Throwable $e) { + $output->writeln(sprintf(' FAILED: %s', $e->getMessage())); + return array_merge($none, ['failed' => 1]); + } + + $existing[$uuid] = self::toCandidate(organisation: $saved); + $output->writeln(' ADOPTED'); + + return array_merge($none, ['adopted' => 1, 'merged' => $mergedCount]); + }//end adoptRow() + + /** + * Name the properties this adoption will not carry over. + * + * @param array $fields The leaf row's fields. + * @param OutputInterface $output Console output stream. + * + * @return void + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + private function reportUndeclared(array $fields, OutputInterface $output): void { + $dropped = self::undeclaredProperties(row: $fields); + if ($dropped === []) { + return; + } + + $phrase = sprintf('%d properties have', count($dropped)); + if (count($dropped) === 1) { + $phrase = '1 property has'; + } + + $output->writeln( + sprintf( + ' %s no column on Organisation and will NOT be carried over: %s', + $phrase, + implode(', ', $dropped) + ) + ); + }//end reportUndeclared() + + /** + * Normalise a legal identifier for comparison. + * + * The same OIN is written with and without spaces and dots depending on who + * typed it, so a literal comparison misses matches that are plainly the same + * body. Everything that is not a letter or a digit is dropped. + * + * @param mixed $value The stored identifier. + * + * @return string The comparable form, or '' when there is nothing to compare. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public static function normaliseIdentifier(mixed $value): string { + if (is_string($value) === false && is_int($value) === false) { + return ''; + } + + $stripped = preg_replace('/[^a-z0-9]/i', '', (string)$value); + if (is_string($stripped) === false) { + return ''; + } + + return strtolower($stripped); + }//end normaliseIdentifier() + + /** + * Find the organisation this row is the same legal entity as, if any. + * + * Matching runs on OIN, then RSIN, then KVK, and stops at the first + * identifier the row actually carries. A name is never matched on: two + * organisations sharing a name are routine, and collapsing them would + * destroy data that no later step can recover. + * + * Among several matches the LOWEST id is canonical, so a repeated run + * chooses the same survivor. A candidate that was itself merged away is + * deprioritised rather than excluded: pointing at it still resolves, + * because the resolver walks the chain. + * + * @param array $row The leaf row's fields. + * @param array> $existing Candidate organisations. + * + * @return array|null The organisation to merge into, or null. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public static function findMergeTarget(array $row, array $existing): ?array { + foreach (self::LEGAL_IDENTIFIERS as $field) { + $needle = self::normaliseIdentifier(value: ($row[$field] ?? null)); + if ($needle === '') { + continue; + } + + $matches = []; + foreach ($existing as $candidate) { + if (self::normaliseIdentifier(value: ($candidate[$field] ?? null)) === $needle) { + $matches[] = $candidate; + } + } + + if ($matches === []) { + continue; + } + + usort( + $matches, + static function (array $a, array $b) { + $aMerged = 0; + if (($a['mergedInto'] ?? null) !== null) { + $aMerged = 1; + } + + $bMerged = 0; + if (($b['mergedInto'] ?? null) !== null) { + $bMerged = 1; + } + + if ($aMerged !== $bMerged) { + return ($aMerged <=> $bMerged); + } + + return ((int)($a['id'] ?? 0) <=> (int)($b['id'] ?? 0)); + } + ); + + return $matches[0]; + }//end foreach + + return null; + }//end findMergeTarget() + + /** + * The leaf properties Organisation has nowhere to put. + * + * OpenRegister discards a property its schema does not declare, and it does + * so with a 200 and the object back, so an adoption that loses fields looks + * exactly like one that did not. Naming them is the whole point. + * + * @param array $row The leaf row's fields. + * + * @return array The property names that will not be carried over. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public static function undeclaredProperties(array $row): array { + $dropped = []; + foreach (array_keys($row) as $property) { + if (str_starts_with($property, '@') === true || $property === 'id' || $property === 'uuid') { + continue; + } + + if (isset(self::FIELD_MAP[$property]) === true) { + continue; + } + + $dropped[] = $property; + } + + sort($dropped); + + return $dropped; + }//end undeclaredProperties() + + /** + * Flatten an object row into a plain field map. + * + * @param mixed $row The row as the object reader returned it. + * + * @return array The fields, with the uuid resolved. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public static function toFields(mixed $row): array { + if (is_object($row) === true && method_exists($row, 'jsonSerialize') === true) { + $row = $row->jsonSerialize(); + } + + if (is_array($row) === false) { + return []; + } + + $self = ($row['@self'] ?? []); + $uuid = ''; + if (is_array($self) === true) { + $uuid = (string)($self['uuid'] ?? ($self['id'] ?? '')); + } + + if ($uuid === '') { + $uuid = (string)($row['uuid'] ?? ($row['id'] ?? '')); + } + + $row['uuid'] = $uuid; + + return $row; + }//end toFields() + + /** + * Build the Organisation to insert for one leaf row. + * + * @param array $fields The leaf row's fields. + * @param array|null $mergeTarget The organisation to merge into, if any. + * + * @return Organisation The organisation, not yet persisted. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public static function buildOrganisation(array $fields, ?array $mergeTarget = null): Organisation { + $organisation = new Organisation(); + $organisation->setUuid((string)$fields['uuid']); + + foreach (self::FIELD_MAP as $property => $suffix) { + $value = ($fields[$property] ?? null); + if ($value === null || $value === '') { + continue; + } + + if (is_scalar($value) === false) { + continue; + } + + $organisation->{'set' . $suffix}((string)$value); + } + + // A slug is derived from the uuid rather than the name. Two adopted rows + // can legitimately share a name, and a name-derived slug would collide. + $organisation->setSlug('adopted-' . substr((string)$fields['uuid'], 0, 36)); + + if ($mergeTarget !== null) { + $organisation->setMergedInto((string)$mergeTarget['uuid']); + $organisation->setMergedAt(new DateTime()); + } + + return $organisation; + }//end buildOrganisation() + + /** + * Reduce an Organisation to the fields matching needs. + * + * @param Organisation $organisation The organisation. + * + * @return array The candidate record. + * + * @spec openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md#requirement-a-leaf-apps-organisations-are-adopted-not-re-created-req-org-106 + */ + public static function toCandidate(Organisation $organisation): array { + return [ + 'id' => (int)$organisation->getId(), + 'uuid' => (string)$organisation->getUuid(), + 'oin' => $organisation->getOin(), + 'rsin' => $organisation->getRsin(), + 'kvk' => $organisation->getKvk(), + 'mergedInto' => $organisation->getMergedInto(), + ]; + }//end toCandidate() + + /** + * Every organisation already on the instance, keyed by uuid. + * + * @return array> The candidates. + */ + private function existingOrganisations(): array { + $candidates = []; + foreach ($this->organisationMapper->findAll(limit: 10000, offset: 0, filters: []) as $organisation) { + $candidates[(string)$organisation->getUuid()] = self::toCandidate(organisation: $organisation); + } + + return $candidates; + }//end existingOrganisations() +}//end class diff --git a/openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md b/openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md index 802cac906c..56b6334ada 100644 --- a/openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md +++ b/openspec/changes/consolidate-organisation-on-or/specs/consolidated-organisation/spec.md @@ -133,3 +133,60 @@ uuid/slug rather than minting new ones. - **GIVEN** an instance where the migration already ran - **WHEN** it runs again - **THEN** no column or index is added a second time and no row is modified. + +### Requirement: A leaf app's organisations are adopted, not re-created (REQ-ORG-106) + +Adopting a leaf app's organisation objects into the OpenRegister Organisation +MUST preserve each row's existing uuid, because references to it are stored in +places no migration can reach. + +The idempotency key MUST be the uuid, never the slug and never the name. A leaf +row is free to carry no slug at all, and two rows sharing a name are routine, so +a name-derived key would skip the second row as already migrated and silently +merge two distinct legal entities. + +Where the same legal entity already exists in OpenRegister under a different +uuid, the rows MUST NOT be collapsed into one. The adopted row is created and +pointed at the existing one through `mergedInto`, so both uuids keep resolving +and the merge is a fact recorded on a row rather than data thrown away. Matching +MUST be on a legal identifier, in the order OIN, RSIN, KVK, and MUST NOT be on a +name. Among several matches the lowest id is canonical, so a repeated run +chooses the same survivor. + +A leaf property the Organisation entity does not declare MUST be reported before +the write. OpenRegister discards an undeclared property and answers 200 with the +object, so an adoption that loses fields is indistinguishable from one that did +not. + +#### Scenario: An adopted organisation keeps its uuid + +- **GIVEN** a leaf organisation object with uuid `abc` +- **WHEN** it is adopted +- **THEN** the resulting Organisation carries uuid `abc`. + +#### Scenario: A second run adopts nothing + +- **GIVEN** an instance where the adoption already ran +- **WHEN** it runs again +- **THEN** every row is skipped as already adopted and nothing is written. + +#### Scenario: The same OIN records a merge rather than collapsing + +- **GIVEN** an existing organisation carrying OIN `00000001002220647000` +- **AND** a leaf row carrying the same OIN under a different uuid +- **WHEN** the leaf row is adopted +- **THEN** it is created with its own uuid and `mergedInto` set to the existing + organisation's uuid. + +#### Scenario: Two organisations sharing only a name are not merged + +- **GIVEN** two organisations with the same name and no shared legal identifier +- **WHEN** one is adopted +- **THEN** no merge is recorded. + +#### Scenario: Properties with no column are named before the write + +- **GIVEN** a leaf schema carrying a property the Organisation entity does not + declare +- **WHEN** the adoption runs +- **THEN** that property is reported as one that will not be carried over. diff --git a/openspec/changes/consolidate-organisation-on-or/tasks.md b/openspec/changes/consolidate-organisation-on-or/tasks.md index 01fd6bf504..ef57e1dec7 100644 --- a/openspec/changes/consolidate-organisation-on-or/tasks.md +++ b/openspec/changes/consolidate-organisation-on-or/tasks.md @@ -34,11 +34,49 @@ ## 5. Leaf-app consolidation -- [ ] 5.1 DECISION REQUIRED: the migration path for existing leaf-app - organisation data. Adding the columns makes reuse possible; it does not - move OpenCatalogi's publisher rows or Stackiq's vendor rows into them. - The backfill must preserve the existing uuid/slug, and it needs a ruling - on what happens when the same legal entity exists in BOTH leaf apps under - different UUIDs. Until then the leaf apps keep their own records and OR's - new columns stay empty. -- [ ] 5.2 Point the leaf apps at the OR organisation once 5.1 is decided. +- [x] 5.1 DECIDED and built: `openregister:organisations:adopt`. The uuid is + the idempotency key and is preserved, following the rule dossiq's + `migrate-partners` arrived at — a leaf row may carry no slug, and two + rows sharing a name are routine, so a name-derived key would skip the + second as "already migrated" and silently merge two legal entities. + Where the same entity already exists under a different uuid the rows are + NOT collapsed: the adopted row is created and pointed at the existing one + through `mergedInto`, so both uuids keep resolving. Matching is on OIN, + then RSIN, then KVK, normalised for punctuation, and never on a name. + Lowest id is canonical; a merged-away candidate loses to a live one. + Properties Organisation has no column for are NAMED before the write, + because OpenRegister discards an undeclared property and answers 200. + Dry-run by default. Proven live on the dev instance including the + negative control (no shared identifier, no merge reported). +- [ ] 5.2 Point the leaf apps at the OR organisation, then retire their + schemas. Measured 2026-09-02, and the measurement changed the shape: + + **opencatalogi** maps 9-for-9 onto Organisation apart from + `tooiIdentifier`. 22 code sites name the slug, and most are + `catalog['organization']`, a stored REFERENCE that keeps resolving once + the uuid is preserved. Roughly 8 real UI sites do + `getCollection('organization')` and need repointing at the Organisation + API. Tractable as one change. + + **stackiq** carries 21 properties, 9 of which have no column on + Organisation. The plan was to map what maps and rehome the rest. Reading + the entity rather than assuming, NOTHING maps: + + - `contactpersonen` -> `contacts` is a different thing. `contacts` is + linked Nextcloud Contacts app data, serialised as `_contacts`. + - `participants` / `deelnames` -> `children` would be lost. The setter + says it plainly: "Children are not stored in the database, only loaded + on demand." A written value is dropped and then recomputed. + - `samenwerkingtype` -> `type` collides. `type` has a closed vocabulary + (organisation, government, vendor, collaboration, department), and a + collaboration SUBtype is a different axis from it. + + So all 9 belong on a stackiq-owned schema under a non-colliding slug, + and none on Organisation. 235 code sites name the slug, because the app + treats `organization` as a first-class object type throughout + (`objectStore.getCollection('organization')`). That is its own change + with its own spec, not a step in this one. + + A later option worth recording: `participants` could be modelled through + OR's `parent`, which IS stored, by setting it on each participant rather + than listing them on the collaboration. diff --git a/tests/Unit/Command/AdoptLeafOrganisationsCommandTest.php b/tests/Unit/Command/AdoptLeafOrganisationsCommandTest.php new file mode 100644 index 0000000000..2205a497aa --- /dev/null +++ b/tests/Unit/Command/AdoptLeafOrganisationsCommandTest.php @@ -0,0 +1,554 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://OpenRegister.app + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Tests\Unit\Command; + +use OCA\OpenRegister\Command\AdoptLeafOrganisationsCommand; +use OCA\OpenRegister\Db\Organisation; +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\OpenRegister\Service\ObjectService; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Tester\CommandTester; + +/** + * Locks the rules that decide what an adoption keeps, merges and drops. + */ +class AdoptLeafOrganisationsCommandTest extends TestCase { + + /** + * Build a candidate organisation record. + * + * @param int $id The row id. + * @param string $uuid The uuid. + * @param string|null $oin The OIN, if any. + * @param string|null $mergedInto The uuid this row was merged into, if any. + * + * @return array The candidate. + */ + private function candidate(int $id, string $uuid, ?string $oin = null, ?string $mergedInto = null): array { + return [ + 'id' => $id, + 'uuid' => $uuid, + 'oin' => $oin, + 'rsin' => null, + 'kvk' => null, + 'mergedInto' => $mergedInto, + ]; + + }//end candidate() + + /** + * The same OIN written with and without punctuation is the same body. + * + * @return void + */ + public function testPunctuationDoesNotHideAMatch(): void { + $this->assertSame( + AdoptLeafOrganisationsCommand::normaliseIdentifier(value: '0000 0001-0022.2064 7000'), + AdoptLeafOrganisationsCommand::normaliseIdentifier(value: '00000001002220647000') + ); + + }//end testPunctuationDoesNotHideAMatch() + + /** + * A value that is not a scalar identifier compares as nothing, rather than + * as an empty string that would match every other empty one. + * + * @return void + */ + public function testANonScalarIdentifierIsNotComparable(): void { + $this->assertSame('', AdoptLeafOrganisationsCommand::normaliseIdentifier(value: ['00000001'])); + $this->assertSame('', AdoptLeafOrganisationsCommand::normaliseIdentifier(value: null)); + + }//end testANonScalarIdentifierIsNotComparable() + + /** + * A shared OIN identifies the same legal entity. + * + * @return void + */ + public function testASharedOinFindsTheMergeTarget(): void { + $target = AdoptLeafOrganisationsCommand::findMergeTarget( + row: ['oin' => '00000001002220647000'], + existing: [$this->candidate(id: 7, uuid: 'existing', oin: '00000001002220647000')] + ); + + $this->assertSame('existing', $target['uuid']); + + }//end testASharedOinFindsTheMergeTarget() + + /** + * Two organisations sharing only a name are two organisations. Merging them + * would destroy data no later step could recover. + * + * @return void + */ + public function testASharedNameNeverMerges(): void { + $this->assertNull( + AdoptLeafOrganisationsCommand::findMergeTarget( + row: ['name' => 'Gemeente Utrecht'], + existing: [ + [ + 'id' => 7, + 'uuid' => 'existing', + 'name' => 'Gemeente Utrecht', + 'oin' => null, + 'rsin' => null, + 'kvk' => null, + 'mergedInto' => null, + ], + ] + ) + ); + + }//end testASharedNameNeverMerges() + + /** + * An empty identifier on either side is not a match: otherwise every row + * carrying no OIN would merge into the first other row carrying none. + * + * @return void + */ + public function testAnEmptyIdentifierIsNotAMatch(): void { + $this->assertNull( + AdoptLeafOrganisationsCommand::findMergeTarget( + row: ['oin' => ''], + existing: [$this->candidate(id: 7, uuid: 'existing', oin: '')] + ) + ); + + }//end testAnEmptyIdentifierIsNotAMatch() + + /** + * Among several matches the lowest id is canonical, so a repeated run + * chooses the same survivor rather than whichever row came back first. + * + * @return void + */ + public function testTheLowestIdIsCanonical(): void { + $target = AdoptLeafOrganisationsCommand::findMergeTarget( + row: ['oin' => '123'], + existing: [ + $this->candidate(id: 9, uuid: 'later', oin: '123'), + $this->candidate(id: 4, uuid: 'earlier', oin: '123'), + ] + ); + + $this->assertSame('earlier', $target['uuid']); + + }//end testTheLowestIdIsCanonical() + + /** + * A candidate that was itself merged away loses to a live one, so the + * adoption points at a row that is still a usable tenant. + * + * @return void + */ + public function testALiveCandidateBeatsAMergedAwayOne(): void { + $target = AdoptLeafOrganisationsCommand::findMergeTarget( + row: ['oin' => '123'], + existing: [ + $this->candidate(id: 2, uuid: 'merged-away', oin: '123', mergedInto: 'somewhere'), + $this->candidate(id: 8, uuid: 'live', oin: '123'), + ] + ); + + $this->assertSame('live', $target['uuid']); + + }//end testALiveCandidateBeatsAMergedAwayOne() + + /** + * OIN is tried before RSIN, so a row carrying both matches on the stronger + * identifier. + * + * @return void + */ + public function testOinIsTriedBeforeRsin(): void { + $target = AdoptLeafOrganisationsCommand::findMergeTarget( + row: ['oin' => '111', 'rsin' => '222'], + existing: [ + ['id' => 3, 'uuid' => 'by-rsin', 'oin' => null, 'rsin' => '222', 'kvk' => null, 'mergedInto' => null], + ['id' => 9, 'uuid' => 'by-oin', 'oin' => '111', 'rsin' => null, 'kvk' => null, 'mergedInto' => null], + ] + ); + + $this->assertSame('by-oin', $target['uuid']); + + }//end testOinIsTriedBeforeRsin() + + /** + * The uuid is preserved: references to it are stored where no migration can + * reach them. + * + * @return void + */ + public function testTheUuidIsPreserved(): void { + $organisation = AdoptLeafOrganisationsCommand::buildOrganisation( + fields: ['uuid' => 'leaf-uuid-123', 'name' => 'Gemeente Utrecht'] + ); + + $this->assertSame('leaf-uuid-123', $organisation->getUuid()); + $this->assertSame('Gemeente Utrecht', $organisation->getName()); + + }//end testTheUuidIsPreserved() + + /** + * The derived slug comes from the uuid, not the name: two adopted rows can + * legitimately share a name and a name-derived slug would collide. + * + * @return void + */ + public function testTheSlugIsDerivedFromTheUuidNotTheName(): void { + $first = AdoptLeafOrganisationsCommand::buildOrganisation( + fields: ['uuid' => 'uuid-one', 'name' => 'Gemeente Utrecht'] + ); + $second = AdoptLeafOrganisationsCommand::buildOrganisation( + fields: ['uuid' => 'uuid-two', 'name' => 'Gemeente Utrecht'] + ); + + $this->assertNotSame($first->getSlug(), $second->getSlug()); + + }//end testTheSlugIsDerivedFromTheUuidNotTheName() + + /** + * A merge target is recorded on the adopted row, so both uuids keep + * resolving. + * + * @return void + */ + public function testTheMergeIsRecordedOnTheAdoptedRow(): void { + $organisation = AdoptLeafOrganisationsCommand::buildOrganisation( + fields: ['uuid' => 'leaf-uuid', 'name' => 'Gemeente Utrecht'], + mergeTarget: ['id' => 4, 'uuid' => 'survivor-uuid'] + ); + + $this->assertSame('leaf-uuid', $organisation->getUuid()); + $this->assertSame('survivor-uuid', $organisation->getMergedInto()); + $this->assertNotNull($organisation->getMergedAt()); + + }//end testTheMergeIsRecordedOnTheAdoptedRow() + + /** + * Properties the entity has no column for are named. OpenRegister discards + * an undeclared property and answers 200 with the object, so an adoption + * that loses fields is otherwise indistinguishable from one that did not. + * + * @return void + */ + public function testUndeclaredPropertiesAreNamed(): void { + $this->assertSame( + ['contactpersonen', 'deelnames', 'xml'], + AdoptLeafOrganisationsCommand::undeclaredProperties( + row: [ + 'uuid' => 'leaf', + '@self' => ['id' => 1], + 'name' => 'Gemeente Utrecht', + 'oin' => '123', + 'xml' => '', + 'deelnames' => [], + 'contactpersonen' => [], + ] + ) + ); + + }//end testUndeclaredPropertiesAreNamed() + + /** + * The uuid is read from the `@self` metadata block, which is where the + * object reader puts it. + * + * @return void + */ + public function testTheUuidIsReadFromTheSelfBlock(): void { + $fields = AdoptLeafOrganisationsCommand::toFields( + row: ['@self' => ['uuid' => 'from-self'], 'name' => 'Gemeente Utrecht'] + ); + + $this->assertSame('from-self', $fields['uuid']); + + }//end testTheUuidIsReadFromTheSelfBlock() + + /** + * A row with no identifier anywhere yields an empty uuid, which the command + * treats as a row it cannot key on rather than minting one. + * + * @return void + */ + public function testARowWithNoIdentifierYieldsNoUuid(): void { + $this->assertSame('', AdoptLeafOrganisationsCommand::toFields(row: ['name' => 'Nameless'])['uuid']); + + }//end testARowWithNoIdentifierYieldsNoUuid() + + /** + * A non-scalar value is not forced into a string column: an array cast to + * string is the word "Array", which is worse than not carrying it. + * + * @return void + */ + public function testANonScalarValueIsNotWrittenToAStringColumn(): void { + $organisation = AdoptLeafOrganisationsCommand::buildOrganisation( + fields: ['uuid' => 'leaf', 'name' => ['nested' => 'value']] + ); + + $this->assertNull($organisation->getName()); + + }//end testANonScalarValueIsNotWrittenToAStringColumn() + + /** + * Build a command over mocked collaborators. + * + * @param array> $rows The leaf rows the reader returns. + * @param array $existing Organisations already on the instance. + * + * @return array{tester: CommandTester, mapper: OrganisationMapper&MockObject} The tester and the mapper. + */ + private function commandOver(array $rows, array $existing = []): array { + $objectService = $this->createMock(ObjectService::class); + $objectService->method('searchObjectsBySlug')->willReturn($rows); + + $mapper = $this->createMock(OrganisationMapper::class); + $mapper->method('findAll')->willReturn($existing); + $mapper->method('insert')->willReturnArgument(0); + + $command = new AdoptLeafOrganisationsCommand( + organisationMapper: $mapper, + objectService: $objectService, + logger: $this->createMock(LoggerInterface::class) + ); + + return ['tester' => new CommandTester($command), 'mapper' => $mapper]; + + }//end commandOver() + + /** + * Build an existing organisation. + * + * @param string $uuid The uuid. + * @param string|null $oin The OIN, if any. + * + * @return Organisation The organisation. + */ + private function existing(string $uuid, ?string $oin = null): Organisation { + $organisation = new Organisation(); + $organisation->setId(4); + $organisation->setUuid($uuid); + $organisation->setOin($oin); + + return $organisation; + + }//end existing() + + /** + * Without --register there is nothing to read from, and the command says so + * rather than reading whatever it can find. + * + * @return void + */ + public function testTheRegisterOptionIsRequired(): void { + $run = $this->commandOver(rows: []); + $this->assertSame(Command::FAILURE, $run['tester']->execute([])); + $this->assertStringContainsString('--register is required', $run['tester']->getDisplay()); + + }//end testTheRegisterOptionIsRequired() + + /** + * The default is a dry run, because the alternative default is a command + * that writes to every organisation on the instance the first time someone + * types its name to see what it does. + * + * @return void + */ + public function testItIsADryRunByDefault(): void { + $run = $this->commandOver(rows: [['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht']]); + $run['mapper']->expects($this->never())->method('insert'); + + $this->assertSame(Command::SUCCESS, $run['tester']->execute(['--register' => 'publication'])); + $display = $run['tester']->getDisplay(); + $this->assertStringContainsString('DRY-RUN', $display); + $this->assertStringContainsString('WOULD ADOPT', $display); + $this->assertStringContainsString('nothing written', $display); + + }//end testItIsADryRunByDefault() + + /** + * With --apply the row is inserted, keeping its uuid. + * + * @return void + */ + public function testApplyInsertsTheRowKeepingItsUuid(): void { + $run = $this->commandOver(rows: [['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht']]); + $run['mapper']->expects($this->once()) + ->method('insert') + ->with( + $this->callback( + static function ($organisation) { + return ((string) $organisation->getUuid() === 'leaf-1'); + } + ) + ) + ->willReturnArgument(0); + + $this->assertSame( + Command::SUCCESS, + $run['tester']->execute(['--register' => 'publication', '--apply' => true]) + ); + $this->assertStringContainsString('Adopted=1', $run['tester']->getDisplay()); + + }//end testApplyInsertsTheRowKeepingItsUuid() + + /** + * A row whose uuid is already an organisation is skipped, which is what + * makes a second run of the command a no-op. + * + * @return void + */ + public function testAnAlreadyAdoptedRowIsSkipped(): void { + $run = $this->commandOver( + rows: [['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht']], + existing: [$this->existing(uuid: 'leaf-1')] + ); + $run['mapper']->expects($this->never())->method('insert'); + + $run['tester']->execute(['--register' => 'publication', '--apply' => true]); + $display = $run['tester']->getDisplay(); + $this->assertStringContainsString('already adopted', $display); + $this->assertStringContainsString('skipped=1', $display); + + }//end testAnAlreadyAdoptedRowIsSkipped() + + /** + * A row with no uuid has no idempotency key, so adopting it would create a + * duplicate on every run. It is skipped and reported. + * + * @return void + */ + public function testARowWithNoUuidIsSkipped(): void { + $run = $this->commandOver(rows: [['name' => 'Nameless']]); + $run['mapper']->expects($this->never())->method('insert'); + + $run['tester']->execute(['--register' => 'publication', '--apply' => true]); + $this->assertStringContainsString('no uuid', $run['tester']->getDisplay()); + + }//end testARowWithNoUuidIsSkipped() + + /** + * A matching legal identifier is reported as a merge and recorded on the + * adopted row, so both uuids keep resolving. + * + * @return void + */ + public function testAMatchingIdentifierRecordsAMerge(): void { + $run = $this->commandOver( + rows: [['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht', 'oin' => '0000-0001']], + existing: [$this->existing(uuid: 'survivor', oin: '00000001')] + ); + + $run['tester']->execute(['--register' => 'publication', '--apply' => true]); + $display = $run['tester']->getDisplay(); + $this->assertStringContainsString('merges into survivor', $display); + $this->assertStringContainsString('merged=1', $display); + + }//end testAMatchingIdentifierRecordsAMerge() + + /** + * Properties Organisation has no column for are named before the write. + * This is the whole reason the report exists: OpenRegister discards an + * undeclared property and answers 200, so a lossy adoption is otherwise + * indistinguishable from a clean one. + * + * @return void + */ + public function testUndeclaredPropertiesAreReportedBeforeTheWrite(): void { + $run = $this->commandOver( + rows: [ + [ + '@self' => ['uuid' => 'leaf-1'], + 'name' => 'Gemeente Utrecht', + 'xml' => '', + 'deelnames' => [], + ], + ] + ); + + $run['tester']->execute(['--register' => 'publication']); + $display = $run['tester']->getDisplay(); + $this->assertStringContainsString('2 properties have no column', $display); + $this->assertStringContainsString('deelnames, xml', $display); + + }//end testUndeclaredPropertiesAreReportedBeforeTheWrite() + + /** + * One undeclared property reads as one, not as "1 property have". + * + * @return void + */ + public function testTheSingularReportReadsAsASingular(): void { + $run = $this->commandOver( + rows: [['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht', 'xml' => '']] + ); + + $run['tester']->execute(['--register' => 'publication']); + $this->assertStringContainsString('1 property has no column', $run['tester']->getDisplay()); + + }//end testTheSingularReportReadsAsASingular() + + /** + * A row that fails to insert is counted as failed and the command exits + * non-zero, so a partial adoption does not report success. + * + * @return void + */ + public function testAFailedInsertMakesTheCommandFail(): void { + $run = $this->commandOver(rows: [['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht']]); + $run['mapper']->method('insert')->willThrowException(new \RuntimeException('constraint')); + + $this->assertSame( + Command::FAILURE, + $run['tester']->execute(['--register' => 'publication', '--apply' => true]) + ); + $display = $run['tester']->getDisplay(); + $this->assertStringContainsString('FAILED: constraint', $display); + $this->assertStringContainsString('failed=1', $display); + + }//end testAFailedInsertMakesTheCommandFail() + + /** + * A second row sharing the first's legal identifier merges into it within + * the same run, because the candidate set grows as rows are adopted. + * + * @return void + */ + public function testARunMergesADuplicateItAdoptedItself(): void { + $run = $this->commandOver( + rows: [ + ['@self' => ['uuid' => 'leaf-1'], 'name' => 'Gemeente Utrecht', 'oin' => '111'], + ['@self' => ['uuid' => 'leaf-2'], 'name' => 'Gemeente Utrecht', 'oin' => '111'], + ] + ); + + $run['tester']->execute(['--register' => 'publication', '--apply' => true]); + $display = $run['tester']->getDisplay(); + $this->assertStringContainsString('merges into leaf-1', $display); + $this->assertStringContainsString('Adopted=2 (of which merged=1)', $display); + + }//end testARunMergesADuplicateItAdoptedItself() +}//end class