ADORSYS-GIS / ADORSYS-GIS/lightbridge-authz
[Ticket]: derive Default for Oauth2 — a new config field currently edits ~30 construction sites
- Linguagem predominante
- Rust
- Estrelas
- 0
- Forks
- 1
- Merge médio
- 6h 42min
- PRs com merge (30d)
- 246
Descrição
## Type
Task (maintainability / removes a recurring LoC-gate cost)
## Summary
`Oauth2` does not derive `Default`, so **every one of its 30 struct-literal constructions must name every field**. Adding a single config field therefore edits ~30 sites and grows several files at once — which is why the last two config additions each had to raise multiple LoC-gate baselines.
## Intent
Make adding a config field cost one line in one file, instead of ~30 lines across six.
## Source of truth (links)
- #637 — added two `OauthClient` fields; raised `config/mod.rs` 2173 -> 2193
- #642 — added `oauth2.jwks_ca_bundle_path`; raised **six** baselines: `config/mod.rs` 2193 -> 2207, `relying_party.rs`, `rest/lib.rs`, `handlers/mod.rs`, `mcp.rs`, `usage/lib.rs`
Two consecutive PRs, both routine config additions, both forced to move the gate.
## Current Behavior
```rust
#[derive(Debug, Clone, Deserialize)]
pub struct Oauth2 { ... }
```
No `Default`. So a new field means every literal gains a line:
```rust
+ jwks_ca_bundle_path: None,
+ jwks_ca_bundle_path: None,
+ jwks_ca_bundle_path: None,
+ jwks_ca_bundle_path: None,
```
That is the entire diff of `handlers/mod.rs` in #642 — four lines of pure ceremony, and a baseline bump to accommodate them.
## Expected Behavior
`Oauth2` derives (or hand-implements) `Default`, and construction sites that only care about a few fields use struct-update syntax:
```rust
Oauth2 { jwks_url: "...".into(), ..Default::default() }
```
A new field then touches `config/mod.rs` and nothing else.
## Acceptance Criteria
- [ ] `Oauth2` implements `Default`, with values matching the existing `serde` defaults **exactly** — a `Default` that disagrees with what deserialization produces is worse than none, because the two paths would silently diverge.
- [ ] Construction sites migrate to `..Default::default()` where they do not deliberately pin every field.
- [ ] **Sites that assert on the full struct keep naming every field.** Anywhere the point of the test is "these are exactly the fields", `..Default::default()` would hide a future field from that assertion. Judgement per site, not a blanket sed.
- [ ] A test pins that `Oauth2::default()` equals what `serde_yaml` produces from a minimal config, so the two definitions of "default" cannot drift.
- [ ] Net effect demonstrated: adding a throwaway field touches only `config/mod.rs`.
## Out of Scope
Splitting `config/mod.rs` — that is the sibling ticket. `OauthClient` and the other config structs may want the same treatment, but do `Oauth2` first and see whether the pattern is worth generalising.
## Technical Context
`type` (`OauthType`) has no obvious default and is `Required, no default` at the config layer (`config/mod.rs:396-399` hard-errors when missing). Decide deliberately whether `Default` picks `self` or whether `Oauth2` needs a hand-written impl that makes the choice explicit — do not let `#[derive(Default)]` silently choose the first enum variant.
## Risks
Low, but real: `..Default::default()` applied indiscriminately **weakens tests** that exist to assert the full shape of a config. The migration is per-site judgement.
## Test Plan
The drift test above, plus the existing config suite. Then add a scratch field locally and confirm only `config/mod.rs` changes.
## Verification evidence
`grep -rn "Oauth2 {" --include='*.rs' crates/ app/ | wc -l` -> **30** construction sites. `config/mod.rs:885` shows `#[derive(Debug, Clone, Deserialize)]` with no `Default`. #642's `handlers/mod.rs` diff is four identical `jwks_ca_bundle_path: None,` lines.
Guia de contribuição
Avaliação
Esta issue ainda não foi avaliada.