Skip to content

Commit 9637541

Browse files
committed
honor optional non-nullable in deserialize_properties
`class_members.mustache` makes the Dart getter `String?` whenever `isNullable || !required` (the only sane Dart mapping: an optional field can always be observably `null`), but `deserialize_properties.mustache` only honored `isNullable`. The two templates therefore disagreed: getter was `String?` but the deserializer cast the value as non-nullable `String`, throwing `type 'Null' is not a subtype of type 'String'` the moment the API returned the field as `null`. The throw bubbled up through any enclosing container, so a single null leaf could tank the entire parent payload -- and most call paths swallowed the error silently. Fix: in `deserialize_properties.mustache` the cast and the FullType now key on the same condition as the getter: nullable when `isNullable || !required`. The null-skip guard (`if (valueDes == null) continue;`) is also extended to optional non-nullable properties so we never reach the builder assignment with a null on the wire. Required + non-nullable, required + explicitly nullable, and optional + explicitly nullable all keep their existing behavior -- only the previously-broken optional non-nullable path changes. A new fixture `built_value_optional_nullable.yaml` exercises every shape, and a new test `DartDioClientCodegenTest.testOptionalNonNullablePropertyDeserializesAsNullable` asserts the generated `watch_provider_entry.dart` contains the expected lines for each. Full Dart suite: 115 tests, 0 failures, 0 regressions.
1 parent 8a65919 commit 9637541

3 files changed

Lines changed: 131 additions & 3 deletions

File tree

modules/openapi-generator/src/main/resources/dart/libraries/dio/serialization/built_value/deserialize_properties.mustache

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,16 @@
1414
case r'{{baseName}}':
1515
final valueDes = serializers.deserialize(
1616
value,
17-
specifiedType: const {{>serialization/built_value/variable_serializer_type}},
18-
) as {{>serialization/built_value/variable_type}};
17+
specifiedType: const FullType{{#isNullable}}.nullable{{/isNullable}}{{^isNullable}}{{^required}}.nullable{{/required}}{{/isNullable}}({{#isContainer}}{{baseType}}, [{{#isMap}}FullType(String), {{/isMap}}{{#items}}{{>serialization/built_value/variable_serializer_type}}{{/items}}]{{/isContainer}}{{^isContainer}}{{{datatypeWithEnum}}}{{/isContainer}}),
18+
) as {{#isContainer}}{{baseType}}<{{#isMap}}String, {{/isMap}}{{#items}}{{>serialization/built_value/variable_type}}{{/items}}>{{/isContainer}}{{^isContainer}}{{{datatypeWithEnum}}}{{/isContainer}}{{#isNullable}}?{{/isNullable}}{{^isNullable}}{{^required}}?{{/required}}{{/isNullable}};
1919
{{#isNullable}}
2020
if (valueDes == null) continue;
2121
{{/isNullable}}
22+
{{^isNullable}}
23+
{{^required}}
24+
if (valueDes == null) continue;
25+
{{/required}}
26+
{{/isNullable}}
2227
{{#isContainer}}
2328
result.{{{name}}}.replace(valueDes);
2429
{{/isContainer}}
@@ -54,4 +59,4 @@
5459
break;
5560
}
5661
}
57-
}
62+
}

modules/openapi-generator/src/test/java/org/openapitools/codegen/dart/dio/DartDioClientCodegenTest.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,79 @@ public void testImportMappingsInSerializersAndBarrelFile() throws IOException {
142142
"package:my_api/src/model/custom_address.dart");
143143
}
144144

145+
/**
146+
* Regression test for the dart-dio built_value generator's handling
147+
* of optional non-nullable model properties.
148+
*
149+
* Before the fix, the generator emitted a Dart getter typed as
150+
* {@code String?} (because in Dart, an optional field can always be
151+
* absent, which is observably {@code null}) but the matching
152+
* deserializer used {@code FullType(String)} and cast the result
153+
* {@code as String}. As a consequence, the moment the JSON payload
154+
* carried the field as an explicit {@code null} the cast threw and
155+
* the entire enclosing object failed to deserialize -- silently in
156+
* many call paths.
157+
*
158+
* The fix: in {@code deserialize_properties.mustache} the cast
159+
* follows the same rule that {@code class_members.mustache} already
160+
* uses for the getter type, i.e. nullable when
161+
* {@code isNullable || !required}. This test asserts that.
162+
*/
163+
@Test
164+
public void testOptionalNonNullablePropertyDeserializesAsNullable() throws IOException {
165+
File output = Files.createTempDirectory("test").toFile();
166+
output.deleteOnExit();
167+
168+
final CodegenConfigurator configurator = new CodegenConfigurator()
169+
.setGeneratorName("dart-dio")
170+
.setInputSpec("src/test/resources/3_0/dart-dio/built_value_optional_nullable.yaml")
171+
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));
172+
173+
ClientOptInput opts = configurator.toClientOptInput();
174+
Generator generator = new DefaultGenerator().opts(opts);
175+
List<File> files = generator.generate();
176+
files.forEach(File::deleteOnExit);
177+
178+
Path widget = output.toPath().resolve("lib/src/model/widget.dart");
179+
180+
// Required fields stay strict: cast as a non-nullable type with a
181+
// non-nullable FullType. Otherwise the existing behaviour for
182+
// required fields would change.
183+
TestUtils.assertFileContains(widget,
184+
"case r'id':",
185+
"specifiedType: const FullType(int),",
186+
"as int;");
187+
TestUtils.assertFileContains(widget,
188+
"case r'name':",
189+
"specifiedType: const FullType(String),",
190+
"as String;");
191+
192+
// Optional non-nullable: getter is `String?` (existing), and the
193+
// deserializer now matches -- FullType.nullable + cast as `T?`
194+
// + skip-on-null guard so we never reach `result.x = valueDes`
195+
// with a null.
196+
TestUtils.assertFileContains(widget, "String? get iconUrl;");
197+
TestUtils.assertFileContains(widget,
198+
"case r'iconUrl':",
199+
"specifiedType: const FullType.nullable(String),",
200+
"as String?;",
201+
"if (valueDes == null) continue;");
202+
203+
TestUtils.assertFileContains(widget, "int? get priority;");
204+
TestUtils.assertFileContains(widget,
205+
"case r'priority':",
206+
"specifiedType: const FullType.nullable(int),",
207+
"as int?;",
208+
"if (valueDes == null) continue;");
209+
210+
// Explicitly nullable still works (regression guard).
211+
TestUtils.assertFileContains(widget,
212+
"case r'explicitlyNullable':",
213+
"specifiedType: const FullType.nullable(String),",
214+
"as String?;",
215+
"if (valueDes == null) continue;");
216+
}
217+
145218
@Test
146219
public void verifyDartDioGeneratorRuns() throws IOException {
147220
File output = Files.createTempDirectory("test").toFile();
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
openapi: "3.0.0"
2+
info:
3+
title: Built-value optional / nullable property test
4+
version: "1.0.0"
5+
paths:
6+
/widget/{id}:
7+
get:
8+
operationId: getWidget
9+
parameters:
10+
- name: id
11+
in: path
12+
required: true
13+
schema: { type: integer }
14+
responses:
15+
"200":
16+
description: OK
17+
content:
18+
application/json:
19+
schema:
20+
$ref: "#/components/schemas/Widget"
21+
components:
22+
schemas:
23+
Widget:
24+
type: object
25+
required:
26+
- id
27+
- name
28+
properties:
29+
# required + non-nullable: deserialize as `int`.
30+
id:
31+
type: integer
32+
# required + non-nullable: deserialize as `String`.
33+
name:
34+
type: string
35+
# optional + non-nullable in OpenAPI: dart getter is `String?`
36+
# because Dart can't distinguish "missing" from "null". The
37+
# deserializer must therefore ALSO accept null and skip the
38+
# assignment, otherwise the cast `as String` throws when the
39+
# API returns the field as null.
40+
iconUrl:
41+
type: string
42+
# optional + non-nullable, integer flavor: same shape as iconUrl
43+
# but exercises FullType(int) instead of FullType(String).
44+
priority:
45+
type: integer
46+
# optional + explicitly nullable: existing behavior, kept as a
47+
# regression guard so the fix doesn't break it.
48+
explicitlyNullable:
49+
type: string
50+
nullable: true

0 commit comments

Comments
 (0)