ADORSYS-GIS / ADORSYS-GIS/lightbridge-authz

[Ticket]: derive Default for Oauth2 — a new config field currently edits ~30 construction sites

Aberta
#643 1 comentário 0 reações 0 responsáveis Ver no GitHub
ticket
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

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.