Skip to content

[dart-dio][built_value] Register BuilderFactory for nested additionalProperties shapes#23662

Open
Homegan wants to merge 2 commits intoOpenAPITools:masterfrom
Homegan:fix/dart-dio-built-value-additional-properties-factory
Open

[dart-dio][built_value] Register BuilderFactory for nested additionalProperties shapes#23662
Homegan wants to merge 2 commits intoOpenAPITools:masterfrom
Homegan:fix/dart-dio-built-value-additional-properties-factory

Conversation

@Homegan
Copy link
Copy Markdown

@Homegan Homegan commented Apr 30, 2026

[dart-dio][built_value] Register BuilderFactory for nested additionalProperties shapes

PR checklist

  • Read the contribution guidelines.
  • Pull request title is prefixed with the affected generator(s): [dart-dio][built_value].
  • File the PR against the correct branch: master.
  • Copy the technical committee to review the pull request: @gibahjoe @ahmednfwela.

Summary

For a property like { additionalProperties: { type: array, items: { $ref: ... } } } (a "map of array of $ref" — a very common shape: category → list of items, locale → list of strings, tag → list of IDs), the dart-dio built_value generator emits a Dart property of type BuiltMap<String, BuiltList<X>> but never registers the BuiltList<X> BuilderFactory in serializers.dart. built_value then fails at runtime with:

Bad state: No builder factory for BuiltList<X>.

The code compiles fine — the failure only surfaces on the first deserialization that touches the property.

Reproduction

WidgetsByCategory:
  type: object
  additionalProperties:
    type: array
    items: { $ref: '#/components/schemas/Widget' }

Catalog:
  type: object
  required: [id]
  properties:
    id: { type: integer }
    widgetsByCategory: { $ref: '#/components/schemas/WidgetsByCategory' }

Generated serializers.dart before this PR:

Serializers serializers = (_$serializers.toBuilder()
      // ...factories for direct return / param container types only...
      ..add(const OneOfSerializer())
      ..add(const AnyOfSerializer())
    ).build();

No factory for BuiltList<Widget>.

Root cause

DartDioClientCodegen.postProcessModelProperty already had a branch for additionalProperties, but it only fired when items.getAdditionalProperties() itself was non-null — i.e. nested Map<String, Map<String, X>>. The much more common shape Map<String, List<X>> (a category map of arrays of $ref) was missed entirely:

if (property.isContainer) {
    final CodegenProperty items = property.items;
    if (items.getAdditionalProperties() != null) {       // ← misses Map<String, List<X>>
        addBuiltValueSerializer(new BuiltValueSerializer(
                items.isArray, items.getUniqueItems(), items.isMap,
                items.items.isNullable,
                items.getAdditionalProperties().dataType));
    }
}

Worse, even if the right tree was being walked, the existing BuiltValueSerializer model only carries a single dataType string — physically can't represent something like BuiltMap<String, BuiltList<X>>, because the FullType argument is recursive (FullType(BuiltMap, [FullType(String), FullType(BuiltList, [FullType(X)])])) and isn't expressible with (isArray | isMap, dataType).

Fix

Two pieces:

1. Recursive walk in Java

postProcessModelProperty now also calls registerNestedBuilderFactories(property), which walks the property's items tree top-down and registers a factory for every container layer. Three small helpers compute the corresponding strings for arbitrary nesting:

Shape Registered factories
Map<String, List<X>> BuiltList<X>, BuiltMap<String, BuiltList<X>>
List<Map<String, X>> BuiltMap<String, X>, BuiltList<BuiltMap<String, X>>
Map<String, Map<String, X>> BuiltMap<String, X>, BuiltMap<String, BuiltMap<String, X>>
List<List<X>> BuiltList<X>, BuiltList<BuiltList<X>>
Set<...> variants swap BuiltList/ListBuilder for BuiltSet/SetBuilder

Recursion handles deeper trees naturally.

2. Composite mode for BuiltValueSerializer

A new constructor BuiltValueSerializer.composite(fullTypeArgs, builderInstantiation) carries the pre-rendered FullType(...) arguments and XBuilder<...> instantiation. serializers.mustache gets a new branch that emits them verbatim:

{{#fullTypeArgs}}
        const FullType({{{fullTypeArgs}}}),
        () => {{{builderInstantiation}}}(),
{{/fullTypeArgs}}
{{^fullTypeArgs}}
        {{! existing isArray / isMap dispatch — unchanged }}
{{/fullTypeArgs}}

equals / hashCode are extended so composite serializers dedup on (fullTypeArgs, builderInstantiation) and never collide with simple ones.

After this PR:

Serializers serializers = (_$serializers.toBuilder()
      // ...
      ..addBuilderFactory(
        const FullType(BuiltList, [FullType(Widget)]),
        () => ListBuilder<Widget>(),
      )
      ..addBuilderFactory(
        const FullType(BuiltMap, [FullType(String), FullType(BuiltList, [FullType(Widget)])]),
        () => MapBuilder<String, BuiltList<Widget>>(),
      )
      ..add(const OneOfSerializer())
      ..add(const AnyOfSerializer())
    ).build();

The existing single-level cases continue to flow through the original (isArray | isMap, dataType) dispatch — no behavior change for them, just fewer "No builder factory" errors at the edges.

Tests

A new fixture src/test/resources/3_0/dart-dio/built_value_additional_properties_factory.yaml exercises the canonical Map<String, List<$ref>> shape on a minimal Widget / WidgetsByCategory / Catalog schema. A new test DartDioClientCodegenTest.testNestedAdditionalPropertiesGetBuilderFactories asserts both the inner BuiltList<Widget> and the outer BuiltMap<String, BuiltList<Widget>> factories appear in serializers.dart.

$ ./mvnw -pl :openapi-generator -am test -Dtest='org.openapitools.codegen.dart.**'

[INFO] Tests run:  2, Failures: 0, Errors: 0, Skipped: 0 -- DartClientOptionsTest
[INFO] Tests run: 78, Failures: 0, Errors: 0, Skipped: 0 -- DartModelTest
[INFO] Tests run:  8, Failures: 0, Errors: 0, Skipped: 0 -- DartDioClientCodegenTest   ← +1 new
[INFO] Tests run:  8, Failures: 0, Errors: 0, Skipped: 0 -- DartClientCodegenTest
[INFO] Tests run:  2, Failures: 0, Errors: 0, Skipped: 0 -- DartDioClientOptionsTest
[INFO] Tests run: 17, Failures: 0, Errors: 0, Skipped: 0 -- DartDioModelTest
[INFO] Tests run: 115, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Files changed

modules/openapi-generator/src/main/java/.../DartDioClientCodegen.java                                       +151 -0
modules/openapi-generator/src/main/resources/dart/libraries/dio/serialization/built_value/serializers.mustache +6   -0
modules/openapi-generator/src/test/java/.../DartDioClientCodegenTest.java                                   +44  -0
modules/openapi-generator/src/test/resources/3_0/dart-dio/built_value_additional_properties_factory.yaml    +51  -0  (new)

4 files, +252 / 0.

Compatibility

  • BuiltValueSerializer public surface: only additive (a new constructor, two new fields). The existing constructor and fields are unchanged. No change to the simple isArray | isMap template dispatch.
  • Generated code that did not exercise the broken path: byte-identical output. The new ..addBuilderFactory(...) lines only appear when a property's container tree is nested (rare today because of this very bug — most users avoid the shape).
  • OpenAPI 3.0 vs 3.1: works for both.

Related

#23645 ([BUG][DART-DIO] Invalid generics for additionalProperties without type) is a different, adjacent bug in the same area — it's about the BuiltMap<String> (single type parameter) compile error when the schema has additionalProperties but no type: object. That one is at the schema-classification layer (setTypeProperties / getPrimitiveType); this PR is at the factory-registration layer in DartDioClientCodegen. Both can land independently. Worth keeping an eye on while reviewing.


Summary by cubic

Fixes missing and duplicate BuilderFactory registrations for nested additionalProperties in the dart-dio built_value generator. Ensures serializers.dart includes the correct factories for shapes like Map<String, List<Widget>>, preventing “Bad state: No builder factory for BuiltList” at runtime.

  • Bug Fixes
    • Recursively registers factories for each nested container (e.g., BuiltList<Widget> and BuiltMap<String, BuiltList<Widget>>).
    • Adds a composite mode to BuiltValueSerializer to emit nested FullType(...)/XBuilder pairs; updates serializers.mustache and deduplicates simple/composite entries to avoid duplicate addBuilderFactory lines.
    • No changes to simple cases; output only changes when nested containers are present.
    • Adds a fixture and test to verify inner and outer factories; regenerates the petstore sample to reflect new factories.

Written for commit 70521cd. Summary will update on new commits. Review in cubic

For a property like
`{ additionalProperties: { type: array, items: { $ref: ... } } }` the
generator emits `Map<String, BuiltList<X>>` in the Dart class but never
registers the `BuiltList<X>` BuilderFactory. The only
factory-registration path that ran for additionalProperties looked at
`items.getAdditionalProperties()`, which is null for this very common
shape (a region map of arrays of $ref). built_value then fails at
runtime with `Bad state: No builder factory for BuiltList<X>` on the
first deserialization that touches the property.

Fix:
- `postProcessModelProperty` now also calls `registerNestedBuilderFactories`,
  which walks the property's `items` tree top-down and registers a
  factory for every container layer. Three small helpers
  (`renderInnerFullType`, `renderDartType`, `renderBuilderFactory`)
  compute the corresponding `FullType(...)` argument list and the
  matching `XBuilder<...>` instantiation for arbitrary nesting:
  `Map<String, List<X>>`, `List<Map<String, X>>`,
  `Map<String, Map<String, X>>`, `List<List<X>>`, `Set<...>`, etc.
- `BuiltValueSerializer` gets a new `composite(fullTypeArgs,
  builderInstantiation)` constructor that carries the pre-rendered
  expressions. The existing `(isArray, uniqueItems, isMap,
  isNullable, dataType)` form is unchanged -- needed because the
  original model can't represent something like
  `BuiltMap<String, BuiltList<X>>` (the `FullType` argument is
  recursive and isn't expressible with a single `dataType` string).
  `equals`/`hashCode` are extended so composite serializers dedup on
  `(fullTypeArgs, builderInstantiation)` and never collide with simple
  ones.
- `serializers.mustache` gets a new branch that emits the composite
  fields verbatim when present; otherwise the existing
  `isArray`/`isMap` dispatch runs unchanged.

Existing simple cases (direct return / parameter container types,
single-level additionalProperties already handled by the prior
branch) keep producing byte-identical output.

A new fixture `built_value_additional_properties_factory.yaml`
exercises the canonical `Map<String, List<$ref>>` shape, and a new
test
`DartDioClientCodegenTest.testNestedAdditionalPropertiesGetBuilderFactories`
asserts both the inner `BuiltList<WatchProviderEntry>` and the outer
`BuiltMap<String, BuiltList<WatchProviderEntry>>` factories appear in
the generated `serializers.dart`.

Full Dart suite: 115 tests, 0 failures, 0 regressions.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants