[dart-dio][built_value] Honor optional non-nullable properties in deserialize_properties.mustache

[Homegan]

[dart-dio][built_value] Honor optional non-nullable properties in deserialize_properties.mustache

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

The dart-dio built_value generator emits a Dart getter typed String? (nullable) for any property that is optional in OpenAPI — even when the schema doesn't say nullable: true. This is the only sane Dart mapping: an absent JSON key is observably null in Dart code.

But the matching deserializer in deserialize_properties.mustache only honored OpenAPI nullable: true and emitted a non-nullable FullType(String) + cast as String. The two templates therefore disagree, and the cast throws type 'Null' is not a subtype of type 'String' the moment the API returns the field as null.

Because the throw bubbles up through every enclosing container, a single null leaf can tank the entire parent payload — and most call paths swallow the error silently.

Reproduction

Widget:
  type: object
  required: [id, name]
  properties:
    id:      { type: integer }
    name:    { type: string }
    iconUrl: { type: string }   # optional, NOT nullable: true

Generated widget.dart before this PR:

@BuiltValueField(wireName: r'iconUrl')
String? get iconUrl;        //  ←  getter is nullable
...
case r'iconUrl':
  final valueDes = serializers.deserialize(
    value,
    specifiedType: const FullType(String),     //  ←  but FullType is non-nullable
  ) as String;                                 //  ←  and the cast is non-nullable
  result.iconUrl = valueDes;
  break;

Send { "iconUrl": null } and the cast throws.

Root cause

class_members.mustache already emits a nullable getter when the Dart side has to admit null:

{{>variable_type}}{{^isNullable}}{{^required}}?{{/required}}{{/isNullable}} get {{name}};

i.e. nullable iff isNullable || !required. But deserialize_properties.mustache only keys on isNullable. So the getter is String? and the deserializer cast is as String — internally inconsistent.

Fix

In deserialize_properties.mustache, the cast and the FullType now follow the same condition as the getter, and the existing null-skip guard is extended to the new branch:

specifiedType: const FullType{{#isNullable}}.nullable{{/isNullable}}{{^isNullable}}{{^required}}.nullable{{/required}}{{/isNullable}}({{...}}),
) as {{...}}{{#isNullable}}?{{/isNullable}}{{^isNullable}}{{^required}}?{{/required}}{{/isNullable}};
{{#isNullable}}
if (valueDes == null) continue;
{{/isNullable}}
{{^isNullable}}
{{^required}}
if (valueDes == null) continue;
{{/required}}
{{/isNullable}}

After this PR:

case r'iconUrl':
  final valueDes = serializers.deserialize(
    value,
    specifiedType: const FullType.nullable(String),
  ) as String?;
  if (valueDes == null) continue;
  result.iconUrl = valueDes;
  break;

Shape	Before	After
Required + non-nullable	FullType(X) / as X	unchanged — FullType(X) / as X
Required + nullable: true	FullType.nullable(X) / as X? + skip-on-null	unchanged
Optional + non-nullable	FullType(X) / as X (BUG)	FullType.nullable(X) / as X? + skip-on-null
Optional + nullable: true	FullType.nullable(X) / as X? + skip-on-null	unchanged

Only the third row — the previously broken case — changes.

Tests

A new fixture src/test/resources/3_0/dart-dio/built_value_optional_nullable.yaml covers all four rows of the table on a minimal Widget schema (id, name required; iconUrl, priority optional non-nullable; explicitlyNullable optional + nullable: true). A new test DartDioClientCodegenTest.testOptionalNonNullablePropertyDeserializesAsNullable asserts the generated widget.dart contains the expected lines for each.

$ ./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/resources/dart/libraries/dio/serialization/built_value/
  deserialize_properties.mustache                                                            +9   -2
modules/openapi-generator/src/test/java/.../DartDioClientCodegenTest.java                    +73  -0
modules/openapi-generator/src/test/resources/3_0/dart-dio/built_value_optional_nullable.yaml +50  -0  (new)

3 files, +131 / −2.

Compatibility

• Generated code that exercised the broken path: the runtime cast exception is gone; null on the wire is now correctly accepted and skipped.
• Generated code that did not exercise the broken path: byte-identical output for required + non-nullable, required + explicitly nullable, and optional + explicitly nullable properties.
• OpenAPI 3.0 vs 3.1: works for both — keyed on the codegen flags isNullable and required, not on the spec dialect.

Side effect, intentional

Models that used to silently lose data (when a parent payload contained any null leaf on an optional non-nullable field) will start surfacing the value correctly. If a downstream consumer was relying on the field being missing rather than null, they'll now see null. This is the correct observable behavior — the previous behavior was the throw + a silent catch-and-drop somewhere up the stack.

Related

There's a separate, independent bug in the dart-dio built_value generator where types reachable only through a model property's additionalProperties never get a BuilderFactory registered (Bad state: No builder factory for BuiltList<X>). I'm filing it as a separate PR — the two fixes touch different files and have different risk profiles, and either can land without the other.

---

Summary by cubic

Fixes deserialization of optional non-nullable properties in the dart-dio + built_value generator by accepting nulls on the wire and skipping null assignments to prevent cast crashes. Regenerates the petstore sample and adds a regression test + fixture to cover this behavior.

• Bug Fixes
  • Use FullType.nullable(...) and cast as T? when isNullable || !required in deserialize_properties.mustache.
  • Add a null-guard to skip assignment when valueDes == null for optional fields.
  • Regenerated petstore_client_lib_fake and added built_value_optional_nullable.yaml + DartDioClientCodegenTest.testOptionalNonNullablePropertyDeserializesAsNullable.

^{Written for commit 63babf6. Summary will update on new commits. Review in cubic}

[cubic-dev-ai]

No issues found across 3 files