Skip to content

Remove redundant list-coercion validators (pydantic v2 already coerces list items) #35

Description

@TimEvci

Summary

Every model defines a @field_validator (ensure_payments, ensure_details, ensure_contact_people, ensure_financial_mutations) delegating to the ensure_list_of helper (model.py:10). These validators are fully redundant and can be deleted with zero behavior change — verified empirically, details below.

Details (verified)

Two facts make the validators dead weight:

  1. Pydantic v2 coerces natively. For a field typed list[Payment], pydantic constructs Payment from a dict and preserves already-constructed instances — before any @field_validator runs. Proof in-repo: ExternalSalesInvoice.details has no validator and coerces dicts correctly today.

  2. The None guards never execute on the non-optional fields. @field_validator defaults to mode="after", which runs after type validation. For non-optional fields (SalesInvoice.payments/details, Document.payments/details, FinancialStatement.financial_mutations), a None input fails the list[...] type check before the validator is ever called. Verified: SalesInvoice(payments=None) and FinancialStatement(financial_mutations=None) raise ValidationError today — the if value is None: return [] branches are unreachable.

For the optional fields (Contact.contact_people, ExternalSalesInvoice.payments, both typed list[...] | None), the validator receives None and returns None — identical to having no validator at all.

So in every case, deleting the validator changes nothing.

⚠️ Do NOT normalize None to [] as part of this change

An earlier draft suggested a BeforeValidator(lambda v: v or []). That is not behavior-preserving:

  • For Contact.contact_people and ExternalSalesInvoice.payments, it would turn None into [] (today None is preserved).
  • For the non-optional fields, it would make a null-from-API start succeeding where it currently raises ValidationError.

Both may be desirable, but they are behavior changes and belong to a separate decision (see below), not this cleanup.

Suggested fix (scoped)

  1. Delete all ensure_* field validators listed above.
  2. Delete ensure_list_of from model.py.
  3. No other changes.

Follow-up decision (out of scope here)

Whether the library should tolerate null collections from the API (mapping them to []) is a real question — the original validators appear to have intended it, but it never worked for the non-optional fields. If desired, that is a BeforeValidator/validate_default-style change and pairs with making ExternalSalesInvoice's collections non-optional (tracked in the detail/payment de-duplication issue).

Acceptance criteria

  • The ensure_* validators and ensure_list_of are removed; no replacement added.
  • Dicts still become typed models; existing instances preserved; optional fields still preserve None.
  • Full test suite passes with no test modifications.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions