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

Generator for dotnet System.Text.Json.JsonConverter produces code that does not match case settings and does not compile. #1448

Closed
tomwolanski opened this issue Jul 13, 2023 · 5 comments · Fixed by #1462
Labels
bug Something isn't working released

Comments

@tomwolanski
Copy link

Describe the bug

When generating a model the property name setting was kept to default value (PascalCase for dotnet). An option to generate json converter was also set. The output code does not compile due to property case mismatch.

How to Reproduce

Reproduced in modelina playground:

The sample api documment:

{
    "asyncapi": "2.5.0",
    "info": {
        "title": "Streetlights API",
        "version": "1.0.0"
    },
    "channels": {
        "light/measured": {
            "publish": {
                "summary": "Inform about environmental lighting conditions for a particular streetlight.",
                "operationId": "onLightMeasured",
                "message": {
                    "name": "LightMeasuredEvent",
                    "payload": {
                        "type": "object",
                        "$id": "LightMeasuredEvent",
                        "required": [ "id" ],
                        "properties": {
                            "id": {                                        // <-- camel case in naming
                                "type": "integer",
                                "minimum": 10
                            },
                            "sentAt": {
                                "type": "string",
                                "format": "date-time"
                            }
                        }
                    }
                }
            }
        }
    }
}

with code generator options:

const generator = new CSharpGenerator({
  indentation: {
    type: IndentationTypes.SPACES
  },
  collectionType: 'Array',
  autoImplementedProperties: true,
  namespace: 'asyncapi.models',
  nullable: true,
 presets: [
  CSHARP_JSON_SERIALIZER_PRESET
]
});

produces model code with expected property format:

[JsonConverter(typeof(LightMeasuredEventConverter))]
public class LightMeasuredEvent
{
	public int Id { get; set; }                             // <-- pascal case in model
	public string? SentAt { get; set; }
	public Dictionary<string, dynamic>? AdditionalProperties { get; set; }
}

the conventer, however uses camel case naming to access those properties:

// ...
if (value.id != null)
{
	// write property name and let the serializer serialize the value itself
	writer.WritePropertyName("id");
	JsonSerializer.Serialize(writer, value.id, options);   // <-- camel case in converter,  should be "value.Id"
}
if (value.sentAt != null)
{
	// write property name and let the serializer serialize the value itself
	writer.WritePropertyName("sentAt");
	JsonSerializer.Serialize(writer, value.sentAt, options);   // <-- camel case in converter, should be "value.SentAt"
}
// ..

Expected behavior

The json converter code produced by the generator should be valid and possible to compile.

@tomwolanski tomwolanski added the bug Something isn't working label Jul 13, 2023
@github-actions
Copy link
Contributor

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@jonaslagoni
Copy link
Member

You are right, thanks for the issue @tomwolanski, nice description!

Yea, the code tries to use the property name instead of the accessor name, which is always pascal case:

const formattedAccessorName = pascalCase(property.propertyName);

So this line here:

const modelInstanceVariable = `value.${propertyName}`;

Needs to change to pascal case as well, at least that's what I currently think is wrong, feel free to correct me @tomwolanski 🙂

Wanna provide this change @tomwolanski?

@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 1.8.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jonaslagoni
Copy link
Member

@all-contributors please add @tomwolanski for bug

@allcontributors
Copy link
Contributor

@jonaslagoni

I've put up a pull request to add @tomwolanski! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants