Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model FieldDeserializers key is always first letter lowercase - CSharp #1830

Closed
MarkusBux opened this issue Sep 9, 2022 · 2 comments · Fixed by #1832
Closed

Model FieldDeserializers key is always first letter lowercase - CSharp #1830

MarkusBux opened this issue Sep 9, 2022 · 2 comments · Fixed by #1832
Assignees
Labels
Csharp Pull requests that update .net code type:bug A broken experience

Comments

@MarkusBux
Copy link

I'm currently playing around with kiota and have the problem that the models generated field deserializer dictionary writes the properties always with first letter in lowercase.

In case the API send uppercase property names, it does not match and therefore the properties of the model are not populated.

Is there a way to do the match case-insensitive or is there a configuration available to do not lower-case the first letter while creating the model?

        //...
        /// <summary>
        /// The deserialization information for the current model
        /// </summary>
        public IDictionary<string, Action<IParseNode>> GetFieldDeserializers()
        {
            return new Dictionary<string, Action<IParseNode>> {
                {"architectureKey", n => { ArchitectureKey = n.GetIntValue(); } },
                {"clientEdition", n => { ClientEdition = n.GetIntValue(); } },
                {"clientType", n => { ClientType = n.GetIntValue(); } },
                {"clientVersion", n => { ClientVersion = n.GetStringValue(); } },
                {"domain", n => { Domain = n.GetStringValue(); } },
                {"isActive", n => { IsActive = n.GetIntValue(); } },
                {"isAOACCapable", n => { IsAOACCapable = n.GetBoolValue(); } },
                {"isObsolete", n => { IsObsolete = n.GetIntValue(); } },
                {"isVirtualMachine", n => { IsVirtualMachine = n.GetBoolValue(); } },
                {"machineId", n => { MachineId = n.GetIntValue(); } },
                {"name", n => { Name = n.GetStringValue(); } },
                {"siteCode", n => { SiteCode = n.GetStringValue(); } },
                {"sMSID", n => { SMSID = n.GetStringValue(); } },
               //...
            };
        }
        //...

API response

{
    "@odata.context": "https://l1cm01/AdminService/v1.0/$metadata#Device/$entity",
    "MachineId": 16787281,
    "ArchitectureKey": 5,
    "Name": "L2WKS02",
    "SMSID": "GUID:E6320265-8298-4367-AA66-2ADEC2A13271",
    "SiteCode": "LAB",
    "Domain": "LAB1",
    "ClientEdition": 0,
    "ClientType": 1,
    "ClientVersion": "5.00.9088.1007",
    "IsObsolete": 0,
    "IsActive": 1,
    "IsVirtualMachine": true,
    "IsAOACCapable": false
    //...
}

image

@ghost ghost added the Needs: Triage 🔍 label Sep 9, 2022
@MarkusBux MarkusBux changed the title Model FieldDeserializers key is always first letter lowercase Model FieldDeserializers key is always first letter lowercase - CSharp Sep 9, 2022
@baywet baywet self-assigned this Sep 9, 2022
@baywet baywet added type:bug A broken experience Csharp Pull requests that update .net code and removed Needs: Triage 🔍 labels Sep 9, 2022
@baywet
Copy link
Member

baywet commented Sep 9, 2022

Hi @MarkusBux
Thanks for trying out Kiota and for reaching out.
I don't think making the collection case insensitive would be the right thing to do since {case:10, Case:10} is valid JSON and would translate to an object with two properties.
The fix should probably be to set the right casing to begin with during the generation, we've made a wrong assumption here because of conventions on Microsoft Graph.

The code lives here for deserialization.

writer.WriteLine($"{{\"{otherProp.SerializationName ?? otherProp.Name.ToFirstCharacterLowerCase()}\", n => {{ {otherProp.Name.ToFirstCharacterUpperCase()} = n.{GetDeserializationMethodName(otherProp.Type, codeElement)}; }} }},");

And there for serialization

writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type, method)}(\"{otherProp.SerializationName ?? otherProp.Name.ToFirstCharacterLowerCase()}\", {otherProp.Name.ToFirstCharacterUpperCase()});");

(and we have equivalents for other languages)

Is this something you'd be willing to submit a pull request for?
Thanks!

@MarkusBux
Copy link
Author

Hi @baywet

thanks for pointing to the corresponding lines within the code. I will try to create a PR, but I think there may arise another issue.
As you pointed out {case:10, Case:10} is valid json.
Using the example of the kiota integration test, the following definition should be valid:

components:
  schemas:
    todo:
      title: Todo
      type: object
      properties:
        id:
          type: string
        subject:
          type: string
        Subject:
          type: string

Using this definition will create a model like

    public class Todo : IAdditionalDataHolder, IParsable {
        /// <summary>Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well.</summary>
        public IDictionary<string, object> AdditionalData { get; set; }
        /// <summary>The id property</summary>
        public string Id { get; set; }
        /// <summary>The subject property</summary>
        public string Subject { get; set; }
        /// <summary>
        /// Instantiates a new todo and sets the default values.
        /// </summary>
        public Todo() {
            AdditionalData = new Dictionary<string, object>();
        }
        /// <summary>
        /// Creates a new instance of the appropriate class based on discriminator value
        /// <param name="parseNode">The parse node to use to read the discriminator value and create the object</param>
        /// </summary>
        public static Todo CreateFromDiscriminatorValue(IParseNode parseNode) {
            _ = parseNode ?? throw new ArgumentNullException(nameof(parseNode));
            return new Todo();
        }
        /// <summary>
        /// The deserialization information for the current model
        /// </summary>
        public IDictionary<string, Action<IParseNode>> GetFieldDeserializers() {
            return new Dictionary<string, Action<IParseNode>> {
                {"id", n => { Id = n.GetStringValue(); } },
                {"subject", n => { Subject = n.GetStringValue(); } },
            };
        }
        /// <summary>
        /// Serializes information the current object
        /// <param name="writer">Serialization writer to use to serialize this model</param>
        /// </summary>
        public void Serialize(ISerializationWriter writer) {
            _ = writer ?? throw new ArgumentNullException(nameof(writer));
            writer.WriteStringValue("id", Id);
            writer.WriteStringValue("subject", Subject);
            writer.WriteAdditionalData(AdditionalData);
        }
    }

As you can see, there is only one property on the model for Subject. If this should be ignored as of now, I can submit a PR for using the PropertyName as provided in the definition and still use the uppercase property name in the model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Csharp Pull requests that update .net code type:bug A broken experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants