refactor(archimate): delete the unreachable private getVoorzieningenConfig duplicate (gate-50 17 → 14) - #468
Merged
Conversation
…onfig duplicate `ArchiMateService::getVoorzieningenConfig()` is `private` and has zero `$this->` call sites in its own file — the only thing that can reach a private method. It is not reflected into either: the whole of `lib/` contains exactly one `ReflectionMethod` call site (`SettingsService:4644`) and it targets `getAmefConfig`, not this method. It cannot run. It is also a STALE duplicate. `SettingsService::getVoorzieningenConfig()` is the live resolver — 13 references across the app — and it ends with `normalizeVoorzieningenConfig()`, which this copy never had. Anything that had been wired to the copy would have received un-normalised config. Removing it removes three of gate-50's seventeen unsafe config reads (`voorzieningen_register`, `voorzieningen_organisatie_schema`, `voorzieningen_contactpersoon_schema`) by removing code that cannot execute, not by moving a guard into the checker's window. Can-fail proof: restoring the file from origin/development puts gate-50 back to 17; with the deletion it reports 14. phpcs clean, psalm clean, phpstan clean, phpmd clean against the repo baseline, unit suite 512 tests green.
Contributor
Quality Report — ConductionNL/softwarecatalog @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue-demi | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 128/128 | |||
| npm | ✅ | ✅ 718/718 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-09 14:58 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
hydra-gates
48c88ba1e0d049f8f38538c33e790d3e603c55d0. gate-50 security-config-fail-mode: 17 → 14.What was deleted
ArchiMateService::getVoorzieningenConfig()isprivateand has zero$this->call sites in its own file — the only thing that can reach a private method.It is not reflected into either. The whole of
lib/contains exactly oneReflectionMethodcall site (SettingsService:4644) and it targetsgetAmefConfig. So this method cannot execute.It is also a stale duplicate:
SettingsService::getVoorzieningenConfig()is the live resolver (13 references across the app) and it ends withnormalizeVoorzieningenConfig(), which this copy never had. Anything wired to the copy would have received un-normalised config.Three of gate-50's seventeen findings go away because the code that made them cannot run — not because a guard was moved into the checker's 10-line window.
The near-miss worth recording
The obvious read of this file is that both
ArchiMateService::getAmefConfig()andgetVoorzieningenConfig()are dead — neither has a->getAmefConfig()/->getVoorzieningenConfig()call site on anArchiMateServiceinstance anywhere inlib/,src/,tests/orappinfo/routes.php.That is wrong for
getAmefConfig().SettingsService::getAmefConfig()reaches it by reflection:No grep for a method call finds that.
getAmefConfig()is live and heavily used; deleting it would have broken every AMEF consumer. Only the private sibling, which nothing reflects into, is dead.The other 14 findings are left RED, deliberately
They are the two legacy-fallback blocks (7 in
ArchiMateService, 7 inArchiMateImportService). The fail-mode is already handled — outside the gate's 10-line window:getAmefRegisterId():is_numeric((string) $raw) === trueand$id > 0, elsereturn null.getAmefSchemaIdForType(): same two conditions on every candidate, elsereturn null.ArchiMateService:1342:throw new \InvalidArgumentException('AMEF register ID is not configured…').Those are stricter than the empty-string comparison gate-50 looks for. Restructuring working code so the guard lands within ten lines of the read would be satisfying a window heuristic, not fixing a defect.
And deleting the reads was checked, and rejected
getAmefSchemaIdForType()reads singular candidate keys (element_schema,view_schema,model_schema…) while the legacy fallback produces plural ones (elements_schema,views_schema…) — measured overlap between producer and consumer: empty set. That looked like six structurally unreachable reads per file.It is not.
src/store/modules/settings.js:942readsac.elements_schemaas a fallback, andSettingsService:5723reads$amefConfig['elements_schema']for the configuration-status readout. Deleting them would have broken the admin settings UI's schema pre-selection and the status panel — the same shape as commit651a055f, which satisfied PHPMD by deleting sixteenifbodies. Left alone.The plural/singular split between what the fallback writes and what
getAmefSchemaIdForType()reads is a real inconsistency and deserves its own issue; it is not something to resolve inside a quality-gate change.Measurements
lib/Service/ArchiMateService.phpfromorigin/developmentputs gate-50 back to 17; with the deletion it reports 14.