Skip to content

Commit 3627331

Browse files
AlexDainesAlexDaines
committedMar 6, 2025
Fix RestJson serialization of sparse null map values
1 parent 149475e commit 3627331

File tree

17 files changed

+989
-470
lines changed

17 files changed

+989
-470
lines changed
 

‎generator/.DevConfigs/363f8b3b-9377-41b9-9ea0-63cc82b25cbeB.json

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"core": {
3+
"changeLogMessages": [
4+
"Fixed protocol test RestJsonSerializesSparseNullMapValues by adding proper null value handling in sparse map serialization"
5+
],
6+
"type": "patch",
7+
"updateMinimum": false
8+
}
9+
}

‎generator/ProtocolTestsGenerator/smithy-dotnet-codegen/src/main/java/software/amazon/smithy/dotnet/codegen/customizations/ProtocolTestCustomizations.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ private ProtocolTestCustomizations() {
100100
);
101101
public static final List<String> VNextTests = Arrays.asList(
102102
//These are the tests that are failing in v4 after updating to 1.54.0 and artifacts 1.0.3004.0. Each one needs to be investigated.
103-
"RestJsonSerializesSparseNullMapValues"
103+
"RestJsonNullAndEmptyHeaders",
104+
"NullAndEmptyHeaders"
104105
);
105106
}

‎generator/ServiceClientGeneratorLib/Generators/Marshallers/JsonRPCRequestMarshaller.cs

+44-44
Large diffs are not rendered by default.

‎generator/ServiceClientGeneratorLib/Generators/Marshallers/JsonRPCStructureMarshaller.cs

+590-230
Large diffs are not rendered by default.

‎generator/ServiceClientGeneratorLib/Generators/Marshallers/JsonRPCStructureMarshaller.tt

+88-3
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ namespace <#= this.Config.Namespace #>.Model.Internal.MarshallTransformat
4949
<#=new string(' ', level * 4)#> {
5050
<#=new string(' ', level * 4)#> context.Writer.WritePropertyName("<#=member.MarshallName#>");
5151
<#+
52-
string memberProperty = variableName + "." + member.PropertyName + (member.IsNullable ? ".Value" : string.Empty);
52+
string memberProperty = variableName + "." + member.PropertyName;
53+
// Only append .Value for nullable types that aren't already complex types
54+
if (member.IsNullable && !member.IsStructure && !member.IsList && !member.IsMap)
55+
{
56+
memberProperty += ".Value";
57+
}
5358
if(member.IsStructure || member.IsList || member.IsMap)
5459
{
5560
this.ProcessStructure(level + 1, variableName + "." + member.PropertyName, member.Shape);
@@ -160,8 +165,76 @@ namespace <#= this.Config.Namespace #>.Model.Internal.MarshallTransformat
160165
<#=new string(' ', level * 4)#> context.Writer.WritePropertyName(<#=flatVariableName#>Kvp.Key);
161166
<#=new string(' ', level * 4)#> var <#=flatVariableName#>Value = <#=flatVariableName#>Kvp.Value;
162167

163-
<#+
164-
ProcessStructure(level + 1, flatVariableName + "Value", structure.ValueShape);
168+
<#+ // Check for null values - only null checks for sparse maps as defined in customizations
169+
170+
bool isNullableMap = false;
171+
if (this.Structure != null)
172+
{
173+
var memberWithMap = this.Structure.Members.FirstOrDefault(m => m.PropertyName == variableName.Split('.').Last());
174+
isNullableMap = (memberWithMap != null && memberWithMap.UseNullable);
175+
}
176+
177+
if (isNullableMap)
178+
{
179+
#>
180+
<#=new string(' ', level * 4)#> if(<#=flatVariableName#>Value == null)
181+
<#=new string(' ', level * 4)#> {
182+
<#=new string(' ', level * 4)#> context.Writer.WriteNullValue();
183+
<#=new string(' ', level * 4)#> }
184+
<#=new string(' ', level * 4)#> else
185+
<#=new string(' ', level * 4)#> {
186+
<#+
187+
}
188+
189+
if(structure.ValueShape.IsBoolean)
190+
{
191+
if(isNullableMap)
192+
{
193+
#>
194+
<#=new string(' ', level * 4)#> context.Writer.WriteBooleanValue(<#=flatVariableName#>Value.Value);
195+
<#+
196+
}
197+
else
198+
{
199+
#>
200+
<#=new string(' ', level * 4)#> context.Writer.WriteBooleanValue(<#=flatVariableName#>Value);
201+
<#+
202+
}
203+
}
204+
else if(structure.ValueShape.IsInt || structure.ValueShape.IsLong || structure.ValueShape.IsFloat || structure.ValueShape.IsDouble)
205+
{
206+
if(isNullableMap)
207+
{
208+
#>
209+
<#=new string(' ', level * 4)#> context.Writer.WriteNumberValue(<#=flatVariableName#>Value.Value);
210+
<#+
211+
}
212+
else
213+
{
214+
#>
215+
<#=new string(' ', level * 4)#> context.Writer.WriteNumberValue(<#=flatVariableName#>Value);
216+
<#+
217+
}
218+
}
219+
else
220+
{
221+
if(isNullableMap)
222+
{
223+
ProcessStructure(level + 2, flatVariableName + "Value", structure.ValueShape);
224+
}
225+
else
226+
{
227+
ProcessStructure(level + 1, flatVariableName + "Value", structure.ValueShape);
228+
}
229+
}
230+
231+
// Close the else block for nullable maps
232+
if (isNullableMap)
233+
{
234+
#>
235+
<#=new string(' ', level * 4)#> }
236+
<#+
237+
}
165238
#>
166239
<#=new string(' ', level * 4)#> }
167240
<#=new string(' ', level * 4)#> context.Writer.WriteEndObject();
@@ -256,9 +329,21 @@ namespace <#= this.Config.Namespace #>.Model.Internal.MarshallTransformat
256329
}
257330
else if (shape.IsInt || shape.IsLong || shape.IsFloat || shape.IsDouble)
258331
{
332+
// Handle integer enums with special care to support both nullable and non-nullable types
333+
if (memberProperty.EndsWith(".Value") && shape.IsInt)
334+
{
335+
// Already has .Value appended, use as is for nullable types
259336
#>
260337
<#=new string(' ', level * 4)#> context.Writer.WriteNumberValue(<#=memberProperty#>);
261338
<#+
339+
}
340+
else
341+
{
342+
// For non-nullable types, don't try to access .Value
343+
#>
344+
<#=new string(' ', level * 4)#> context.Writer.WriteNumberValue(<#=memberProperty#>);
345+
<#+
346+
}
262347
}
263348
else if (shape.IsBoolean)
264349
{

‎generator/TestServiceModels/restjson-tests-client/rest-json-protocol-2019-12-16.normal.json

+46
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,17 @@
389389
"output":{"shape":"JsonMapsInputOutput"},
390390
"documentation":"<p>The example tests basic map serialization.</p>"
391391
},
392+
"SparseJsonMaps":{
393+
"name":"SparseJsonMaps",
394+
"http":{
395+
"method":"POST",
396+
"requestUri":"/SparseJsonMaps",
397+
"responseCode":200
398+
},
399+
"input":{"shape":"SparseJsonMapsInputOutput"},
400+
"output":{"shape":"SparseJsonMapsInputOutput"},
401+
"documentation":"<p>The example tests serialization of JSON map values in sparse maps.</p>"
402+
},
392403
"JsonTimestamps":{
393404
"name":"JsonTimestamps",
394405
"http":{
@@ -1466,6 +1477,16 @@
14661477
"denseSetMap":{"shape":"DenseSetMap"}
14671478
}
14681479
},
1480+
"SparseJsonMapsInputOutput":{
1481+
"type":"structure",
1482+
"members":{
1483+
"sparseStructMap":{"shape":"SparseStructMap"},
1484+
"sparseNumberMap":{"shape":"SparseNumberMap"},
1485+
"sparseBooleanMap":{"shape":"SparseBooleanMap"},
1486+
"sparseStringMap":{"shape":"SparseStringMap"},
1487+
"sparseSetMap":{"shape":"SparseSetMap"}
1488+
}
1489+
},
14691490
"JsonTimestampsInputOutput":{
14701491
"type":"structure",
14711492
"members":{
@@ -1929,6 +1950,31 @@
19291950
}
19301951
},
19311952
"union":true
1953+
},
1954+
"SparseBooleanMap":{
1955+
"type":"map",
1956+
"key":{"shape":"String"},
1957+
"value":{"shape":"Boolean"}
1958+
},
1959+
"SparseNumberMap":{
1960+
"type":"map",
1961+
"key":{"shape":"String"},
1962+
"value":{"shape":"Integer"}
1963+
},
1964+
"SparseSetMap":{
1965+
"type":"map",
1966+
"key":{"shape":"String"},
1967+
"value":{"shape":"StringList"}
1968+
},
1969+
"SparseStringMap":{
1970+
"type":"map",
1971+
"key":{"shape":"String"},
1972+
"value":{"shape":"String"}
1973+
},
1974+
"SparseStructMap":{
1975+
"type":"map",
1976+
"key":{"shape":"String"},
1977+
"value":{"shape":"GreetingStruct"}
19321978
}
19331979
},
19341980
"documentation":"<p>A REST JSON service that sends JSON requests and responses.</p>"

‎sdk/test/ProtocolTests/Generated/RestJsonProtocol/dotnet-protocol-test-codegen/SparseJsonMaps.cs

-6
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,6 @@ public void RestJsonSparseJsonMapsRequest()
8686
/// <summary>
8787
/// Serializes JSON map values in sparse maps
8888
/// </summary>
89-
/*
90-
* This test either requires a breaking change and will be addressed
91-
* in V4, or has a backlog item to be fixed in the future. Please
92-
* refer to the VNextTests list to see which it is.
93-
* */
94-
[Ignore]
9589
[TestMethod]
9690
[TestCategory("ProtocolTest")]
9791
[TestCategory("RequestTest")]

‎sdk/test/Services/RestJsonProtocol/Custom/_netstandard/AmazonRestJsonProtocolClient.cs

-76
This file was deleted.

‎sdk/test/Services/RestJsonProtocol/Custom/_netstandard/IAmazonRestJsonProtocol.cs

-56
This file was deleted.

0 commit comments

Comments
 (0)
Failed to load comments.