Skip to content

Commit 153a7e5

Browse files
rubenvdlindeConduction Release Bot
andauthored
refactor(softwarecatalog): translate the stored enum values, and migrate them (#520)
* refactor(softwarecatalog): translate the stored enum values, and migrate them Values are the other half of this programme, and the quieter half. Renaming one in the schema changes the DECLARATION; every row already written still holds the Dutch string, and a filter on the new value then returns NULL rather than an error — so the feature reports "nothing found" instead of failing. `RenameDutchCatalogValues` rewrites the stored rows, scoped by COLUMN and idempotent. Scoped by column, never by the string alone: `intern` is a connection's integration type here and a statutory ZGW confidentiality value elsewhere, and `Concept` is a lifecycle state on one column and an ordinary word on the next. Held back, each for a reason rather than a hunch: - `roles` — its members MIRROR NEXTCLOUD GROUP NAMES that ContactpersonenController checks with `isInGroup('gebruik-beheerder')`. Renaming the enum while the groups keep their names desynchronises the two, and the groups are instance data an administrator created. - `digikoppeling`, licence names, and the abbreviations (SLA, DVO, IaaS, BBN1-3) — proper names. Only the Dutch WORDS inside a licence move: `Licentie` -> `License`, `versie` -> `version`. - `samenwerkingtype` as a member of its own enum — the property name leaked into the value list. That is a data defect, not a translation. Two tooling defects this shook out, both found by tests rather than by reading: - The enum rewriter matched its property block with a regex that tolerated ONE level of nested braces, so any property nesting deeper was silently skipped — `service.type` was. The test asserting the old value is gone caught it; nothing else would have. Now brace-matched. - The assignment rewriter built its replacement and then rewrote the first quote character, emitting `'value"` and breaking seven PHP files. Verified against a control: PHPUnit 696 vs 684 (+12 new), the single remaining error identical on development (Symfony HeaderUtils missing under the unit bootstrap). phpstan, psalm, phpmd clean; 0 phpcs errors on the new files; jest 120; vitest 226; l10n and prettier clean; no duplicate JSON keys. Dutch enum values 45 -> 8. * test(softwarecatalog): cover the decisions class the value test uses testPropertiesSnakeToRealColumnNames calls RenameDutchCatalogDecisions to check the map's properties snake down to the columns the migration will UPDATE. PHPUnit's strict coverage marks a test RISKY when it executes a class @Covers does not name, and one risky test fails the whole cell. Fixed before CI reported it — procest #849 hit exactly this an hour ago, and softwarecatalog #516 before that. It only ever shows up in CI: there is no coverage driver in the container. * test(softwarecatalog): plan the value rewrites where they can be tested The coverage guard failed stable34 by 0.03%: the value step is DB code a unit suite cannot reach. Working out WHICH rewrites a table needs is not DB code though — it is a decision, and it moves to the collaborator with the rest of them. It earns the test on its own merits. Shard tables are per-schema, so most carry only a few of the mapped columns, and an UPDATE against a column the table lacks is an error rather than a no-op. 698 tests (was 684 on development), the single remaining error identical there. phpstan and psalm clean. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
1 parent c15e6fd commit 153a7e5

43 files changed

Lines changed: 783 additions & 243 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

appinfo/info.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ Vrij en open source onder de EUPL-licentie.
126126
Idempotent, non-destructive, and refuses ambiguous renames.
127127
-->
128128
<step>OCA\SoftwareCatalog\Repair\RenameDutchCatalogColumns</step>
129+
<!--
130+
After the column rename, and late for the same reason: the schema
131+
edit changes the DECLARATION, but every row already written still
132+
holds the Dutch string. A filter on the new value then returns
133+
null rather than an error, so the feature reports "nothing found"
134+
instead of failing.
135+
-->
136+
<step>OCA\SoftwareCatalog\Repair\RenameDutchCatalogValues</step>
129137
</post-migration>
130138
<!--
131139
FRESH INSTALL. Nextcloud does NOT run post-migration on a first install:

lib/Portal/PortalContributionProvider.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
* Catalog behaves exactly as before.
1414
*
1515
* It declares — for the `vendor-org` (software supplier / organisatie.type
16-
* "Leverancier") and `participant-org` (municipality / "Gemeente",
17-
* "Samenwerking", "Community") audiences — the OpenRegister collections a portal
16+
* "Supplier") and `participant-org` (municipality / "Municipality",
17+
* "Collaboration", "Community") audiences — the OpenRegister collections a portal
1818
* subject may READ, each scoped by the subject's organisatie UUID (claim
1919
* `organisationId`). Some collections reach that organisatie UUID through a
2020
* single `via` one-hop join (contract → dienst/gebruik, compliancy → module)
@@ -83,9 +83,9 @@ class PortalContributionProvider {
8383
* The audiences this provider contributes to (contract v2, preferred).
8484
*
8585
* The registry probes for this method first. Software Catalog serves
86-
* software suppliers (`vendor-org`, organisatie.type "Leverancier") and the
86+
* software suppliers (`vendor-org`, organisatie.type "Supplier") and the
8787
* municipalities/collaborations that consume that software (`participant-org`,
88-
* organisatie.type "Gemeente" / "Samenwerking" / "Community"). The two
88+
* organisatie.type "Municipality" / "Collaboration" / "Community"). The two
8989
* audiences exist because the same `gebruik` object is scoped by a DIFFERENT
9090
* property for each side (`aanbieder` vs `afnemer`).
9191
*

lib/Repair/RenameDutchCatalogDecisions.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,4 +127,40 @@ public function firstSafeTarget(string $old, array $candidates, array $declared)
127127
return null;
128128
}//end firstSafeTarget()
129129

130+
/**
131+
* Work out which value rewrites a table actually needs.
132+
*
133+
* Pure, and separated for the same reason as the rest of this class: the
134+
* UPDATE that follows needs a database, the decision about WHICH updates to
135+
* run does not.
136+
*
137+
* A property whose column the table does not have is skipped — shard tables
138+
* are per-schema, so most tables carry only a few of the mapped columns, and
139+
* issuing an UPDATE against a missing column is an error rather than a no-op.
140+
*
141+
* @param array<string, array<string, string>> $valueMap Property => old => new.
142+
* @param array<int, string> $columns Columns the table has.
143+
*
144+
* @return array<int, array{column: string, old: string, new: string}>
145+
*/
146+
public function plannedRewrites(array $valueMap, array $columns): array {
147+
$planned = [];
148+
149+
foreach ($valueMap as $property => $values) {
150+
$column = $this->sanitizeColumnName(name: $property);
151+
if (in_array($column, $columns, true) === false) {
152+
continue;
153+
}
154+
155+
foreach ($values as $old => $new) {
156+
$planned[] = [
157+
'column' => $column,
158+
'old' => (string)$old,
159+
'new' => $new,
160+
];
161+
}
162+
}
163+
164+
return $planned;
165+
}//end plannedRewrites()
130166
}//end class
Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
<?php
2+
3+
/**
4+
* Translates the Dutch ENUM VALUES stored in this app's shard tables.
5+
*
6+
* Renaming a value in the schema is only half the job, and the quieter half.
7+
* The declaration changes, but every row already written still holds the old
8+
* string — and a filter on the new one then returns NULL rather than an error,
9+
* so the feature reports "nothing found" instead of failing. That is the shape
10+
* of every value migration: the code looks right and the data disagrees.
11+
*
12+
* Scoped by COLUMN, never by value alone. `intern` is a connection's
13+
* integration type here and a statutory ZGW confidentiality value elsewhere;
14+
* `Concept` is a lifecycle state on one column and an ordinary word on the
15+
* next. A value migration that matches on the string alone corrupts every
16+
* column that happens to share it.
17+
*
18+
* Idempotent: an already-migrated row simply matches no WHERE clause.
19+
*
20+
* @category Repair
21+
* @package OCA\SoftwareCatalog\Repair
22+
* @author Conduction B.V. <info@conduction.nl>
23+
* @copyright 2026 Conduction B.V.
24+
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
25+
* @link https://www.conduction.nl
26+
*
27+
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
28+
* SPDX-License-Identifier: EUPL-1.2
29+
*/
30+
31+
declare(strict_types=1);
32+
33+
namespace OCA\SoftwareCatalog\Repair;
34+
35+
use OCP\DB\Exception;
36+
use OCP\IDBConnection;
37+
use OCP\Migration\IOutput;
38+
use OCP\Migration\IRepairStep;
39+
use Psr\Log\LoggerInterface;
40+
41+
/**
42+
* Migrates stored Dutch enum values to their English spelling.
43+
*/
44+
class RenameDutchCatalogValues implements IRepairStep {
45+
46+
/**
47+
* Property name => old value => new value.
48+
*
49+
* Keyed by the PROPERTY as declared in the schema; the column is derived
50+
* with MagicMapper's own rule (see RenameDutchCatalogDecisions).
51+
*
52+
* `roles` is absent on purpose: its members mirror Nextcloud GROUP names
53+
* that `ContactpersonenController` checks with `isInGroup()`, and renaming
54+
* the stored value while the group keeps its name desynchronises the two.
55+
*
56+
* @var array<string, array<string, string>>
57+
*/
58+
public const VALUE_MAP = [
59+
'type' => [
60+
'Functioneel beheer' => 'Functional management',
61+
'Applicatiebeheer' => 'Application management',
62+
'Technisch beheer' => 'Technical management',
63+
'Implementatieondersteuning' => 'Implementation support',
64+
'Opleidingen' => 'Training',
65+
'Licentiereseller' => 'Licence reseller',
66+
'Gemeente' => 'Municipality',
67+
'Leverancier' => 'Supplier',
68+
'Samenwerking' => 'Collaboration',
69+
'Applicatie' => 'Application',
70+
'Systeemsoftware' => 'System software',
71+
'n.v.t.' => 'n/a',
72+
'bestandsoverdracht' => 'file transfer',
73+
'upload naar portaal' => 'upload to portal',
74+
],
75+
'status' => [
76+
'Concept' => 'Draft',
77+
'Actief' => 'Active',
78+
'Deactief' => 'Inactive',
79+
'samengevoegd' => 'merged',
80+
'Verwerving' => 'Acquisition',
81+
'Gepland' => 'Planned',
82+
'In productie' => 'In production',
83+
'Uit te faseren' => 'To be phased out',
84+
'Uitgefaseerd' => 'Phased out',
85+
'Verlopen' => 'Expired',
86+
'In onderhandeling' => 'In negotiation',
87+
'in ontwikkeling' => 'in development',
88+
'in gebruik' => 'in use',
89+
'einde ondersteuning' => 'end of support',
90+
'teruggetrokken' => 'withdrawn',
91+
],
92+
'registeredBy' => [
93+
'Gemeente' => 'Municipality',
94+
'Applicatie' => 'Application',
95+
'Samenwerking' => 'Collaboration',
96+
'Leverancier' => 'Supplier',
97+
],
98+
'samenwerkingtype' => [
99+
'Uitvoeringsorganisatie' => 'Implementing organisation',
100+
'Sociaal Domein samenwerking' => 'Social Domain collaboration',
101+
'Omgevingsdienst' => 'Environmental agency',
102+
'ICT (bijvoorbeeld Shared Service Center)' => 'ICT (for example Shared Service Center)',
103+
'Gemeentelijke herindeling (gepland)' => 'Municipal reorganisation (planned)',
104+
'Gemeenschappelijke Regeling (samenwerking meerdere domeinen)' => 'Joint Arrangement (collaboration across multiple domains)',
105+
'Gemeenschappelijke Regeling' => 'Joint Arrangement',
106+
'Centrumgemeenteregeling' => 'Central municipality arrangement',
107+
'Belastingsamenwerking' => 'Tax collaboration',
108+
'Bedrijfsvoeringsorganisatie' => 'Operations organisation',
109+
'Archiefdienst (regionaal)' => 'Archive service (regional)',
110+
'Ambtelijke fusie' => 'Administrative merger',
111+
],
112+
'contractType' => [
113+
'Licentie' => 'Licence',
114+
'Onderhoud' => 'Maintenance',
115+
],
116+
'costPeriod' => [
117+
'Maandelijks' => 'Monthly',
118+
'Jaarlijks' => 'Annually',
119+
'Eenmalig' => 'One-off',
120+
],
121+
'dataExchangeDirection' => [
122+
'AnaarB' => 'AtoB',
123+
'BnaarA' => 'BtoA',
124+
'bi-directioneel' => 'bi-directional',
125+
],
126+
'integrationType' => [
127+
'extern' => 'external',
128+
'intern' => 'internal',
129+
],
130+
'hostingJurisdiction' => ['Elders' => 'Elsewhere'],
131+
'hostingLocation' => ['Elders' => 'Elsewhere'],
132+
'licence' => [
133+
'BSD Licentie (Berkeley Software Distribution)' => 'BSD License (Berkeley Software Distribution)',
134+
'European Union Public Licence (EUPL), versie 1.2' => 'European Union Public Licence (EUPL), version 1.2',
135+
],
136+
];
137+
138+
/**
139+
* Constructor.
140+
*
141+
* @param IDBConnection $db Database connection.
142+
* @param LoggerInterface $logger Logger.
143+
* @param RenameDutchCatalogDecisions $decisions Column-name predicates.
144+
*/
145+
public function __construct(
146+
private readonly IDBConnection $db,
147+
private readonly LoggerInterface $logger,
148+
private readonly RenameDutchCatalogDecisions $decisions = new RenameDutchCatalogDecisions(),
149+
) {
150+
}//end __construct()
151+
152+
/**
153+
* Step name shown by `occ maintenance:repair`.
154+
*
155+
* @return string
156+
*/
157+
public function getName(): string {
158+
return 'Translate stored Dutch SoftwareCatalog enum values';
159+
}//end getName()
160+
161+
/**
162+
* Rewrite the stored values, one column at a time.
163+
*
164+
* @param IOutput $output Repair output.
165+
*
166+
* @return void
167+
*/
168+
public function run(IOutput $output): void {
169+
$tables = $this->shardTables();
170+
if ($tables === []) {
171+
$output->info('RenameDutchCatalogValues: no SoftwareCatalog shard tables on this install; nothing to do.');
172+
return;
173+
}
174+
175+
$updated = 0;
176+
foreach ($tables as $table) {
177+
$planned = $this->decisions->plannedRewrites(
178+
valueMap: self::VALUE_MAP,
179+
columns: $this->columnsOf(table: $table)
180+
);
181+
182+
foreach ($planned as $job) {
183+
$updated += $this->rewrite(
184+
table: $table,
185+
column: $job['column'],
186+
old: $job['old'],
187+
new: $job['new']
188+
);
189+
}
190+
}
191+
192+
$output->info(sprintf('RenameDutchCatalogValues: %d row value(s) translated.', $updated));
193+
}//end run()
194+
195+
/**
196+
* Rewrite one value in one column.
197+
*
198+
* @param string $table Shard table.
199+
* @param string $column Column name.
200+
* @param string $old Stored Dutch value.
201+
* @param string $new English replacement.
202+
*
203+
* @return int Rows affected.
204+
*/
205+
private function rewrite(string $table, string $column, string $old, string $new): int {
206+
$sql = 'UPDATE ' . $this->quote(identifier: $table)
207+
. ' SET ' . $this->quote(identifier: $column) . ' = ?'
208+
. ' WHERE ' . $this->quote(identifier: $column) . ' = ?';
209+
210+
try {
211+
return $this->db->executeStatement($sql, [$new, $old]);
212+
} catch (Exception $e) {
213+
$this->logger->warning(
214+
'RenameDutchCatalogValues: value rewrite failed.',
215+
['table' => $table, 'column' => $column, 'exception' => $e->getMessage()]
216+
);
217+
return 0;
218+
}
219+
}//end rewrite()
220+
221+
/**
222+
* Discover this app's shard tables.
223+
*
224+
* Anchors on the `openregister_table_` marker rather than a computed
225+
* prefix: OCP\IDBConnection exposes neither getSchema() nor getPrefix(),
226+
* and calling them is a runtime fatal that only phpstan catches.
227+
*
228+
* @return array<int, string>
229+
*/
230+
private function shardTables(): array {
231+
try {
232+
$stmt = $this->db->prepare(
233+
'SELECT table_name FROM information_schema.tables WHERE table_name LIKE :pattern'
234+
);
235+
$stmt->bindValue('pattern', '%openregister\_table\_%');
236+
$stmt->execute();
237+
$rows = $stmt->fetchAll();
238+
} catch (\Throwable $e) {
239+
$this->logger->warning(
240+
'RenameDutchCatalogValues: could not list tables; skipping.',
241+
['exception' => $e->getMessage()]
242+
);
243+
return [];
244+
}
245+
246+
return array_map(static fn (array $r): string => (string)$r['table_name'], $rows);
247+
}//end shardTables()
248+
249+
/**
250+
* Read a table's column names.
251+
*
252+
* @param string $table Table name.
253+
*
254+
* @return array<int, string>
255+
*/
256+
private function columnsOf(string $table): array {
257+
try {
258+
$stmt = $this->db->prepare(
259+
'SELECT column_name FROM information_schema.columns WHERE table_name = :table'
260+
);
261+
$stmt->bindValue('table', $table);
262+
$stmt->execute();
263+
$rows = $stmt->fetchAll();
264+
} catch (\Throwable $e) {
265+
$this->logger->warning(
266+
'RenameDutchCatalogValues: could not read columns; skipping table.',
267+
['table' => $table, 'exception' => $e->getMessage()]
268+
);
269+
return [];
270+
}
271+
272+
return array_map(static fn (array $r): string => (string)$r['column_name'], $rows);
273+
}//end columnsOf()
274+
275+
/**
276+
* Quote an identifier for the active platform.
277+
*
278+
* @param string $identifier Table or column name.
279+
*
280+
* @return string
281+
*/
282+
private function quote(string $identifier): string {
283+
return $this->db->getDatabasePlatform()->quoteSingleIdentifier($identifier);
284+
}//end quote()
285+
}//end class

lib/Service/AanbodService.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,9 @@ private function addQueryFilters(array $baseQuery, array $options): array {
735735
* Mapping from organisatie.type to registeredBy value.
736736
*/
737737
private const TYPE_MAP = [
738-
'Gemeente' => 'Gemeente',
739-
'Leverancier' => 'Leverancier',
740-
'Samenwerking' => 'Samenwerking',
738+
'Municipality' => 'Municipality',
739+
'Supplier' => 'Supplier',
740+
'Collaboration' => 'Collaboration',
741741
'Community' => 'Community',
742742
];
743743

lib/Service/AangebodenGebruikService.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,9 +1520,9 @@ private function checkOrganisationOwnership(
15201520
* Mapping from organisatie.type to registeredBy value
15211521
*/
15221522
private const TYPE_MAP = [
1523-
'Gemeente' => 'Gemeente',
1524-
'Leverancier' => 'Leverancier',
1525-
'Samenwerking' => 'Samenwerking',
1523+
'Municipality' => 'Municipality',
1524+
'Supplier' => 'Supplier',
1525+
'Collaboration' => 'Collaboration',
15261526
'Community' => 'Community',
15271527
];
15281528

0 commit comments

Comments
 (0)