Altinn / Altinn/altinn-authentication
Make IPartiesClient/IAccessManagementClient return types nullable + handle null in 22 callers (#488 follow-up)
- Dominant language
- C#
- Stars
- 8
- Forks
- 6
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 21
Description
## Summary
To finish the #488 nullable ratchet for the **Integration** project (and lock it with `TreatWarningsAsErrors`), three client interface methods that genuinely return `null` need their contracts made nullable — a change that ripples to their callers and so was deferred out of #2108 for review.
## What needs changing
- **`IPartiesClient`** — `GetPartyAsync`, `GetPartyByOrgNo`, `GetPartyByUuId` are typed `Task` but return `null` on non-OK responses. Make them `Task` (impl already returns null).
- **`IAccessManagementClient`** — `GetAccessPackage`: the implementation already returns `Task`, but the interface declares `Task` (a `CS8613` mismatch). Align the interface to `Task`.
## Caller impact (the reason this is separate)
All **22 production callers** of the party methods do `Party party = await _partiesClient.GetPartyByOrgNo(...)` — assigning to a **non-nullable** local and then dereferencing without a null check:
- `AuthenticationController` (1), `SystemUserClientDelegationController` (5)
- `ChangeRequestSystemUserService` (4), `RequestSystemUserService` (4), `SystemUserService` (8)
Making the return nullable surfaces a `CS8600`/`CS8602` at each — i.e. **latent NRE risk that exists today**. Each site needs a real decision: what should happen when the party isn't found? (return a Problem, throw, skip, etc.) This is the "don't change a shared contract without reviewing all downstream callers" guidance from prior review.
## Tasks
- [ ] Make `IPartiesClient.GetParty*` return `Task` and `IAccessManagementClient.GetAccessPackage` return `Task`.
- [ ] Review each of the 22 party callers and add appropriate null handling (not just `!`).
- [ ] Enable `` on the Integration project (it will then be at 0 warnings) — completing its #488 ratchet.
## Context
- Follow-up to #2108 (Integration 60→7 warnings).
- Part of #488.
- Related: the `HandleResponse` 403/500 issue #2101 (same client).
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the IPartiesClient and IAccessManagementClient declarations and their existing implementations, then inspect the 22 callers named in AuthenticationController, SystemUserClientDelegationController, ChangeRequestSystemUserService, RequestSystemUserService, and SystemUserService. Decide and implement explicit null handling at each call site, then enable TreatWarningsAsErrors in the Integration project and verify the build has zero warnings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- authentication, backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100