Skip to content

v4: change IFieldConfiguration.Validators from concrete List<> to an interface type #155

@phmatray

Description

@phmatray

Context

IFieldConfiguration<TModel, TValue>.Validators is declared as the concrete List<IFieldValidator<TModel, TValue>>. Because List<> members are not virtual, the object-typed view (FieldConfigurationWrapper) cannot return a forwarding collection — so in #151 (v3.1.0) we had to settle for a cached materialized list plus an additive AddValidator(...) member, instead of making config.Fields[i].Validators.Add(...) itself work.

The result is a residual API trap: .Add() on the object-typed Validators list still mutates a snapshot that never affects validation. It compiles, runs, and silently does nothing — exactly the failure mode the v3.0 audit flagged (finding #33).

Proposal (breaking — v4)

Change the property type on IFieldConfiguration<TModel, TValue> (and the object-typed projection) to an interface:

  • Option A — IReadOnlyList<IFieldValidator<TModel, TValue>> (recommended): mutation through the property becomes a compile error; AddValidator(...) (already shipped in v3.1) becomes the single documented mutation path. Honest and simple.
  • Option B — IList<IFieldValidator<TModel, TValue>>: lets the wrapper return a true forwarding list so .Add() works through the object-typed view too. More convenient, but keeps two mutation paths and requires careful wrapper unwrapping of ValidatorWrapper.

Either way:

  • FieldConfiguration keeps a private List<> backing field.
  • Audit the codebase for other concrete-collection leaks on public interfaces while at it (Dependencies, AdditionalAttributes, FieldDependencies dictionary on IFormConfiguration) and decide each deliberately — same class of problem.
  • Migration note for the v4 changelog: replace field.Validators.Add(v) with field.AddValidator(v) (works since 3.1.0, so consumers can migrate before upgrading).

Acceptance criteria

  • Mutating validators through the object-typed view either works (B) or cannot compile (A) — no silent no-op path remains.
  • All existing tests pass with mechanical adjustments only.
  • v4 migration table entry documenting the change.

Refs: #151, v3.0 audit finding #33.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:mediumFix when possiblestatus:triagedClassified and ready for analysis/worktype:refactorCode refactoring without behavior change

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions