Replace standalone C# enums with forward-compatible IEnum class pattern#1680
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
572eec7 to
115781c
Compare
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request implements a new IEnum pattern for generated C# models to improve forward compatibility and round-trip safety by preserving unknown enum values as raw strings. The changes involve significant updates to mustache templates, JSON converters, and numerous generated model files, along with the addition of comprehensive tests. A review comment identifies an improvement opportunity to make the Value property on the generated IEnum classes readonly, ensuring immutability and preventing potential bugs caused by consumers modifying enum instances.
galesky-a
left a comment
There was a problem hiding this comment.
LGTM, best to tackle those asap so we have the same pattern for all enums moving forward. Left a single comment on null behavior but it's just a sanity check
Description
Fix #1650 #1612
Problem
There are 2 types of enum values in the .NET library:
inner enums: they define anIEnumclass that it is forward-compatible, new (unknown) values are handledstandalone enums: they define standalonepublic enums(with int backing values), new (unknown) values throw an exceptionSolution
Replaces all standalone C# enum types generated by
modelEnum.mustachewith anIEnumclass pattern, matching the existing inner-enum approach. Unknown API values are now preserved as-is instead of throwingJsonExceptionor causingNullReferenceException(fixes #1612).The PR updates the Mustache templates and generates Checkout models (to create additional tests for the different type of enums) and BalancePlatform models (to fix a failing test).
Important
👍 Handling of standalone enum values has been improved: new enum values do not thrown an error, they instead retain the string value found in the JSON payload. This was the goal of the PR.
null, but they instead retain the string value found in the JSON payload.This is an improvement but also a breaking change: existing code that relies on null checks to identify unknown enum values will no longer work.
Changes
modelEnum.mustache: override default template and generateIEnumclasses for standalone enums withFromStringOrDefault,ToJsonValue, implicit string conversions, and a nestedJsonConverterTested scenarios
Result(standalone) andPixPayByBankDetails(inner TypeEnum) with real generated classesAffected Standalone Enums (18 files)
18 standalone enums listed above change from
enum(value type) toclass : IEnum(reference type).LimitStatus,AssociationStatus,Scope,ScaDeviceType,ScaExemption,MandateStatus,MandateType,SettingType,ScaStatus,ScaEntityType,TransferTypeCALocalBankAccountType,AdditionalBankIdentificationTypes,FundsCollectionType,USLocalBankAccountTypeResultResourceType,ProductTypeBEFORE (current SDK):
AFTER (new SDK):
Summary of breaking changes
enum->classswitch/caseno longer worksif/==or pattern matching.HasValue/.Valueon nullable removed!= nullinstead of.HasValuedefaultchanges from0tonull(int)cast removed.Valuestring propertyEnum.Parse/Enum.GetValuesremovedFromStringOrDefault/ static fields