SonarQube reports GitLfsCacheHandler has 14 constructor parameters against an allowed 7 (GitLfsCacheHandler.cs:43).
Logged rather than fixed alongside the other complexity findings, because the honest fix is a design change and not something to do while silencing an analyzer.
Why it grew
The handler was designed around a genuine shared preamble — resolve the upstream, validate the token, decide whether bytes come from the store or upstream — and its own doc comment argues that splitting it across five endpoint classes would mean five copies of that preamble. That argument held when it handled batch, transfers and verify.
It has since taken on the repository allow-list, lock listing, lock snapshot invalidation, and lock fan-out. Those last three share almost none of the original preamble: they need the upstream and the allow-list check, and nothing about tokens, the store, or public URL resolution.
What not to do
Bundling the dependencies into an options object to get the count under 7. That would leave one class doing two unrelated jobs and make the coupling harder to see rather than smaller.
Suggested shape
Split the locks routes into a LockRouteHandler holding LockListService, ILockSnapshotStore and LockFanOut. The dispatch preamble that is genuinely shared — route parsing, upstream resolution, the allow-list check — stays where it is and calls into whichever handler the route kind selects.
That leaves the object handler with the token and store dependencies it was designed around, and the lock handler with the three it actually uses.
Notes
- Not urgent. The current code is correct and tested; this is about keeping it changeable.
- Worth doing before anything else is added to the handler, because the next feature will make the split more expensive rather than less.
- Related: the metadata-only mode already routes batch, transfer and verify to the relay, so the two groups are already behaving as separate concerns at dispatch time.
Context: #2, #3
SonarQube reports
GitLfsCacheHandlerhas 14 constructor parameters against an allowed 7 (GitLfsCacheHandler.cs:43).Logged rather than fixed alongside the other complexity findings, because the honest fix is a design change and not something to do while silencing an analyzer.
Why it grew
The handler was designed around a genuine shared preamble — resolve the upstream, validate the token, decide whether bytes come from the store or upstream — and its own doc comment argues that splitting it across five endpoint classes would mean five copies of that preamble. That argument held when it handled batch, transfers and verify.
It has since taken on the repository allow-list, lock listing, lock snapshot invalidation, and lock fan-out. Those last three share almost none of the original preamble: they need the upstream and the allow-list check, and nothing about tokens, the store, or public URL resolution.
What not to do
Bundling the dependencies into an options object to get the count under 7. That would leave one class doing two unrelated jobs and make the coupling harder to see rather than smaller.
Suggested shape
Split the locks routes into a
LockRouteHandlerholdingLockListService,ILockSnapshotStoreandLockFanOut. The dispatch preamble that is genuinely shared — route parsing, upstream resolution, the allow-list check — stays where it is and calls into whichever handler the route kind selects.That leaves the object handler with the token and store dependencies it was designed around, and the lock handler with the three it actually uses.
Notes
Context: #2, #3