feat: handle unknown enum values gracefully

[galesky-a]

• Add SafeStringEnumConverter to return null for unknown enum values
• Register converter globally in JsonOperation.Deserialize()
• Update template to make all enums nullable (not just non-required)
• Add comprehensive test suite for enum deserialization
• Ensures forward compatibility when API adds new enum values

Description

Tested scenarios

Fixed issue:

[gemini-code-assist]

Summary of Changes

Hello @galesky-a, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of JSON deserialization by implementing a strategy to gracefully handle unknown enum values. By introducing a custom SafeStringEnumConverter and registering it globally, the application will now assign null to nullable enum properties when an unrecognized enum string is encountered in the JSON, rather than throwing an exception. This change, coupled with an update to the code generation template to make all enum properties nullable, significantly improves the client's forward compatibility with API changes that might introduce new enum members.

Highlights

• Graceful Enum Deserialization: Introduced SafeStringEnumConverter to prevent deserialization failures when encountering unknown enum values in JSON payloads, returning null instead.
• Global Converter Registration: The new SafeStringEnumConverter is now globally registered within JsonOperation.Deserialize(), ensuring consistent behavior across the application.
• Nullable Enum Properties: Updated the C# model generation template (modelGeneric.mustache) to make all enum properties nullable by default, improving forward compatibility with evolving APIs.
• Comprehensive Test Coverage: Added new test suites (NewtonsoftEnumBehaviorTest.cs, UnknownEnumDeserializationTest.cs) to validate the behavior of both Newtonsoft.Json and the custom converter with various enum deserialization scenarios.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[galesky-a]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a SafeStringEnumConverter to gracefully handle unknown enum values during JSON deserialization by converting them to null instead of throwing an exception. This is a great improvement for forward compatibility with the API. The changes include updating the JSON deserialization logic, modifying the model generation template to make all enums nullable, and adding a comprehensive suite of tests.

My review includes a few suggestions to improve the implementation and tests:

• A small refinement in the SafeStringEnumConverter for better code style.
• An improvement to one of the new tests to make it more efficient.
• A recommendation to make another test more accurately reflect its purpose.

[gemini-code-assist]

This test deserializes the JSON string twice if no exception is thrown. This is inefficient and can be avoided by capturing the result of the deserialization within the Record.Exception lambda.

        {
            // Test if Newtonsoft.Json handles unknown enum values gracefully with nullable enums
            var json = @"{\"enumField\": \"UnknownValue\"}";
            
            // This will tell us if we need a custom converter or not
            TestModel result = null;
            var exception = Record.Exception(() => result = JsonConvert.DeserializeObject<TestModel>(json));
            
            if (exception == null)
            {
                Assert.NotNull(result);
                Assert.Null(result.EnumField);
            }
            else
            {
                // If this throws, we need a custom converter
                Assert.IsType<JsonSerializationException>(exception);
            }
        }

[gemini-code-assist]

The name of this test, TestWebhookHandlerWithUnknownEnumValues, is misleading as the JSON payload used does not contain any unknown enum values. It only tests that a minimal webhook notification can be deserialized without errors.

To make this test more valuable and align with its name, consider adding a notification item with an unknown enum value to the JSON payload. For example (assuming eventCode is an enum property):

{
    "live": "false",
    "notificationItems": [
        {
            "NotificationRequestItem": {
                "eventCode": "SOME_UNKNOWN_EVENT_CODE",
                "success": "true"
                // ... other properties
            }
        }
    ]
}

This would properly test that the webhook handler can gracefully handle new, unknown enum values from the Adyen platform.

[gemini-code-assist]

Code Review

This pull request introduces a SafeStringEnumConverter to handle unknown enum values gracefully during JSON deserialization, preventing exceptions and instead returning null for nullable enum properties. This change improves forward compatibility with API changes. The converter is registered globally, and model generation templates have been updated to make all enum properties nullable. New tests have been added to cover this new behavior.

My review focuses on improving the new tests for clarity and completeness, and suggests a minor refinement in the converter's implementation for better readability and adherence to C# idioms. Overall, this is a valuable improvement for the library's robustness.

[gemini-code-assist]

This test's logic is a bit confusing as it handles two possible outcomes (throwing an exception or returning null). A test should ideally be deterministic and verify a single, specific behavior.

Since the purpose of this test is to demonstrate why SafeStringEnumConverter is needed, it should explicitly assert that deserializing an unknown enum with the standard StringEnumConverter throws a JsonSerializationException.

This would make the test's purpose clearer and avoid ambiguity. Also, the repeated deserialization call inside the if (exception == null) block is inefficient.

[gemini-code-assist]

This test is named TestWebhookHandlerWithUnknownEnumValues, but it doesn't actually test the deserialization of an unknown enum value within a webhook notification. The provided JSON for notificationItems is an empty array, so no enum properties are being deserialized.

To make this test effective and match its name, you should provide a JSON payload that includes a notification item with an enum property set to an unknown value, and then assert that this property is deserialized as null. For example, you could add a notification item with a success field set to an unknown value, and then assert that the corresponding property on the deserialized object is null.