Skip to content

refactor(deps): inject OpenRegister instead of looking it up (ADR-083) - #515

Draft
rubenvdlinde wants to merge 1 commit into
developmentfrom
refactor/adr-083-inject-openregister
Draft

refactor(deps): inject OpenRegister instead of looking it up (ADR-083)#515
rubenvdlinde wants to merge 1 commit into
developmentfrom
refactor/adr-083-inject-openregister

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

5 file(s) reached OpenRegister through $this->container->get(...) on an unconditional path — no availability check, no degrading catch. The dependency was announced nowhere: not in the constructor, not in the use block, not in any type. It appeared mid-method, as a string.

Now constructor-injected and typed. Behaviour is unchanged — same object, same container, resolved at construction instead of at first use. ContainerInterface is dropped only where nothing else used it.

Deliberately not converted, because they are correct as written (ADR-083 rule 1's exception): lookups behind isInstalled()/getInstalledApps(), and lookups whose catch degrades rather than rethrows. Converting those would make the service unconstructable without OpenRegister and turn a clean message into a 500.

Verified per file: php -l clean, and gate-66's lookup check reports zero remaining findings for each file changed.

gate-66 for this app: 23 → 8.

5 file(s) reached OpenRegister through $this->container->get(...) on an
UNCONDITIONAL path — no availability check, no degrading catch. The
dependency was announced nowhere: not in the constructor, not in the use
block, not in any type. It appeared mid-method, as a string.

Now constructor-injected and typed, so the dependency is visible to a
reader and to tooling. Behaviour is unchanged: the same object, from the
same container, resolved at construction instead of at first use.

ContainerInterface is dropped only where nothing else used it.

Deliberately NOT converted, because they are correct as written (ADR-083
rule 1's exception): lookups behind isInstalled()/getInstalledApps(), and
lookups whose catch degrades rather than rethrows.

Verified per file: php -l clean, and gate-66's lookup check reports zero
remaining findings for each file changed.

gate-66 for this app: 23 -> 8.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ cfbbdd1

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue-demi
test-l10n
format
composer ✅ 130/130
npm ✅ 704/704
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-14 19:05 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde marked this pull request as draft August 14, 2026 19:12
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Marked draft — not mergeable as it stands, same cause as decidesk/pipelinq/shillinq.

Removing ContainerInterface shifts every positional argument after it, so unit tests constructing these services hand the wrong mock to the next parameter. CI shows the PHPUnit matrix red across all six cells.

7 test file(s) here construct the classes this PR changes and need updating to the new signature first.

The lib/ side is complete and verified — php -l clean per file, gate-66 lookup findings zero for every file changed. Kept open so that work is not lost; docudesk #461 shows the fix shape on a 2-file example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant