Protocol tests issue 30 #3266
Protocol tests issue 30 #3266
Conversation
| /// For map shapes the KeyXmlName represents the "locationName" on the map shape's key member. This can be changed | ||
| /// via the @xmlName trait. This ensures that we don't check for a hardcoded "key" value, if the key's name was changed. | ||
| /// </summary> | ||
| public string KeyXmlName |
There was a problem hiding this comment.
nit: these could be public string KeyXmlName { get; set; } without needing member variables. Even if you set the default you can do so with the easier syntax.
There was a problem hiding this comment.
used auto properties
| private string valueXmlName = ""; | ||
| private string keyXmlName = ""; |
There was a problem hiding this comment.
Should the defaults be key and value? This would ensure the class would at least work for the default case out of the box without setting values for the properties.
| @@ -72,6 +72,8 @@ namespace <#=this.Config.Namespace #>.Model.Internal.MarshallTransformations | |||
| if (member.IsMap || member.IsList) | |||
| { | |||
| #> | |||
| context.KeyXmlName = "<#=member.Shape.KeyMarshallName#>"; | |||
There was a problem hiding this comment.
This mechanism of storing the current key and value name for the shape being marshalled to the context seems like it would be broken if we had values that were maps as well that had different KeyMarshallName. When we made the recursive call to to the child map being marshalled the context would be changed and then when we process the next element in the parent map we would have different KeyXmlName or ValueXmlName.
There was a problem hiding this comment.
Going to have to update this, to handle nested xmlNames on key/values. Will push an update with that fix later
|
Nested maps with xml names need more investigation. closing |
Description
This PR fixes DOTNET-7357 issue 30.
Issue 30 (Related to issue 29) Incorrectly unmarshalling responses which used xmlName on the requestA little more context: This specifically fixes the case where xml name is used on a key or value of a map. This makes it so that we don't search for a hardcoded "key" or "value" and instead pass that down from the model.
MERGE into
protocol-testsfeature branch onlyBuilds on prior PR: #3265
Merge order: 111 (I'm going to start mine from 100 and increment to keep separate from Bo's)
Ticket: DOTNET-7357
Motivation and Context
Testing
N/A - the test cases are the protocol tests. A final dry-run will be done on the
protocol-testsfeature branch once everything is merged into the feature branch.Screenshots (if appropriate)
Types of changes
Checklist
License