fix(gate-66): an availability-guarded lookup is correct, not a violation - #458
Merged
Merged
Conversation
This is what stopped a fleet-wide script from running over 1,263 call
sites, and it would have been a regression in every one of the 106 it
touched.
Measured on portaliq:
if ($this->isOpenRegisterAvailable() === false) {
return ['success' => false,
'message' => 'OpenRegister is not installed or enabled.'];
}
try { $cfg = $this->container->get('OCA\OpenRegister\Service\ConfigurationService'); }
Injecting that dependency makes the whole service unconstructable on an
instance without OpenRegister and turns the clean message into a 500 —
the exact failure ADR-083 rule 3 exists to prevent. The app was already
right; the gate was wrong to report it.
So rule 1 governs UNCONDITIONAL dependencies. Where a class reaches for
OpenRegister only after establishing it is there, deferring construction
IS the point and the lookup stays. The finding text now says so, and
offers both remedies rather than only injection.
File-scoped on purpose: per-method scoping would flag the helper that does
the reaching while clearing the caller that does the checking.
Fleet split, measured: 106 guarded, 1000 unguarded. Effect —
portaliq/scholiq/doriath/hermiq lookup findings -> 0
pipelinq 192, decidesk 120, opencatalogi 57 unchanged
16 tests. Both directions pinned: the guarded shape passes, and the SAME
shape with the availability check removed still fails, so the clause
cannot go quiet. clean/ gains OptionalCapabilityService.php, which carries
the exact construct planted/ is reported for — if the availability clause
is ever lost, that arm goes red.
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.
This is what stopped a fleet-wide script from running over 1,263 call sites — and it would have been a regression in every one of the 106 it touched.
Measured on portaliq:
Injecting that dependency makes the service unconstructable on an instance without OpenRegister and turns the clean message into a 500 — the exact failure ADR-083 rule 3 exists to prevent. The app was already right; the gate was wrong to report it.
The rule, refined
Rule 1 governs unconditional dependencies. Where a class reaches for OpenRegister only after establishing it is there, deferring construction is the point and the lookup stays. The finding text now offers both remedies instead of only injection.
File-scoped deliberately: per-method scoping would flag the helper that does the reaching while clearing the caller that does the checking.
Measured effect
Fleet split: 106 guarded, 1,000 unguarded.
Tests
16, both directions pinned: the guarded shape passes, and the same shape with the availability check removed still fails, so the clause cannot go quiet.
clean/gainsOptionalCapabilityService.php, carrying the exact constructplanted/is reported for — if the availability clause is ever lost, that arm goes red. Acceptance matrix: 146 passed, 0 failed.