Skip to content

Refactor model_generator.type_converter to reduce complexity and improve composite schema handling #98

@dougborg

Description

@dougborg

Summary

model_generator.type_converter has accumulated complexity: deeply nested conditionals for allOf / oneOf / anyOf, repeated UUID/date-time format handling, manual and brittle import aggregation, and intertwined responsibilities (reference resolution, primitive mapping, optional wrapping, composite flattening). Recent composite edge tests (added in my fork) revealed an IndexError in allOf import aggregation (now patched). A structured refactor will improve maintainability and safety for future OpenAPI 3.1 feature expansions.

Pain Points

  1. High cyclomatic complexity (C901 territory) makes reasoning and extending risky.
  2. Repeated logic for UUID and date-time handling across multiple branches.
  3. Fragile import_types collection previously used direct indexing (i.import_types[0]).
  4. Self-referential model handling (quoted forward refs) is scattered.
  5. Mixed responsibilities (composite assembly + primitive format mapping) in one long function.
  6. Hard to introduce new format/constraint mappings without duplication.

Goals

  • Isolate responsibilities via helper functions.
  • Centralize format handling (UUID variants, date-time) with orjson flag awareness.
  • Robust import aggregation (ordered unique, no blind indexing).
  • Preserve current generated output (unless import ordering normalization explicitly chosen).
  • Improve testability of composite branches (nested allOf/oneOf/anyOf combinations, self refs, reference-only composites).

Proposed Approach

  1. Extract helpers:
    • _normalize_schema_type(schema) -> str | list[str]
    • _convert_primitive(schema, required, model_name)
    • _convert_composite(kind, subs, required, model_name) where kind in {allOf, anyOf, oneOf}
    • _wrap_optional(type_str, required)
    • _collect_imports(conversions) returns ordered unique imports
  2. First mechanical refactor (no behavioral changes) guarded by snapshot-style tests of current composite outputs.
  3. Consolidate UUID/date-time logic into a single helper invoked from primitive + list-type branches.
  4. Expand regression tests (already drafted in fork) to fix output expectations: nested composites, only references, self-ref single element, anyOf/oneOf single collapse, mixed ref + array.
  5. Document unsupported OpenAPI 3.1 JSON Schema features (boolean schemas, unevaluatedProperties=false, items=false) for clarity.

Non-Goals

  • Adding new 3.1 features not currently parsed by upstream library.
  • Changing public API or altering generated model signatures.

Risks & Mitigations

Risk Mitigation
Output ordering (union/tuple member order) shifts Snapshot current outputs & assert equality in tests
Import list ordering differences Decide: preserve legacy order or normalize (document decision)
Hidden regression in edge composite cases Comprehensive composite test matrix + forward-ref cases

Acceptance Criteria

  • All existing tests pass with identical generated outputs (aside from intentionally normalized import ordering if chosen).
  • type_converter significantly reduced in size; main function orchestrates helpers.
  • No direct indexing of possibly empty import lists.
  • Composite test suite covers previously missed branches (improved coverage numbers for model_generator.py).

Follow-Up (Separate Issues)

  • Potential mypy-friendly wrapper types for Schema/Reference to reduce repeated isinstance checks.
  • Introduce feature flags for future 3.1 JSON Schema enhancements once library support lands.

References

  • Patch fixing IndexError in allOf import aggregation (fork history).
  • New composite edge tests (tests/test_model_generator_edges.py in fork) demonstrating current behavior.

Feedback welcome: especially on whether to normalize import ordering now or defer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions