-
-
Notifications
You must be signed in to change notification settings - Fork 64
Move [JsonConverter] from enum properties to enum types #938
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,170 @@ | ||||||||||||||
| using FluentAssertions; | ||||||||||||||
| using FluentAssertions.Execution; | ||||||||||||||
| using Refitter.Core; | ||||||||||||||
| using Refitter.Tests.Build; | ||||||||||||||
| using Refitter.Tests.TestUtilities; | ||||||||||||||
| using TUnit.Core; | ||||||||||||||
|
|
||||||||||||||
| namespace Refitter.Tests.Examples; | ||||||||||||||
|
|
||||||||||||||
| /// <summary> | ||||||||||||||
| /// Tests for enum types with hyphenated or otherwise special-character values (issue #300). | ||||||||||||||
| /// When enum values contain characters that require [EnumMember(Value = "...")] attributes | ||||||||||||||
| /// (e.g. "allegro-pl", "AC_1_PHASE"), JsonStringEnumConverter cannot deserialize them because | ||||||||||||||
| /// it uses the C# identifier name, not the [EnumMember] value. | ||||||||||||||
| /// The fix is to place [JsonConverter(typeof(JsonStringEnumConverter))] on the enum TYPE | ||||||||||||||
| /// so users can override it via JsonSerializerOptions.Converters with a converter that | ||||||||||||||
| /// respects [EnumMember] (e.g. JsonStringEnumMemberConverter). | ||||||||||||||
| /// </summary> | ||||||||||||||
| public class HyphenatedEnumValuesTests | ||||||||||||||
| { | ||||||||||||||
| private const string OpenApiSpec = @" | ||||||||||||||
| { | ||||||||||||||
| ""swagger"": ""2.0"", | ||||||||||||||
| ""info"": { | ||||||||||||||
| ""title"": ""Hyphenated Enum API"", | ||||||||||||||
| ""version"": ""1.0.0"" | ||||||||||||||
| }, | ||||||||||||||
| ""host"": ""example.com"", | ||||||||||||||
| ""basePath"": ""/"", | ||||||||||||||
| ""schemes"": [ | ||||||||||||||
| ""https"" | ||||||||||||||
| ], | ||||||||||||||
| ""paths"": { | ||||||||||||||
| ""/api/offers"": { | ||||||||||||||
| ""get"": { | ||||||||||||||
| ""summary"": ""Get offers"", | ||||||||||||||
| ""operationId"": ""getOffers"", | ||||||||||||||
| ""parameters"": [ | ||||||||||||||
| { | ||||||||||||||
| ""name"": ""marketplaceId"", | ||||||||||||||
| ""in"": ""query"", | ||||||||||||||
| ""required"": false, | ||||||||||||||
| ""type"": ""string"", | ||||||||||||||
| ""enum"": [""allegro-pl"", ""allegro-cz""] | ||||||||||||||
| } | ||||||||||||||
| ], | ||||||||||||||
| ""responses"": { | ||||||||||||||
| ""200"": { | ||||||||||||||
| ""description"": ""Success"", | ||||||||||||||
| ""schema"": { | ||||||||||||||
| ""$ref"": ""#/definitions/MarketplaceReference"" | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| }, | ||||||||||||||
| ""definitions"": { | ||||||||||||||
| ""MarketplaceReference"": { | ||||||||||||||
| ""type"": ""object"", | ||||||||||||||
| ""properties"": { | ||||||||||||||
| ""id"": { | ||||||||||||||
| ""type"": ""string"", | ||||||||||||||
| ""enum"": [""allegro-pl"", ""allegro-cz""] | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| "; | ||||||||||||||
|
|
||||||||||||||
| [Test] | ||||||||||||||
| public async Task Can_Generate_Code() | ||||||||||||||
| { | ||||||||||||||
| string generatedCode = await GenerateCode(); | ||||||||||||||
| generatedCode.Should().NotBeNullOrWhiteSpace(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [Test] | ||||||||||||||
| public async Task Can_Build_Generated_Code() | ||||||||||||||
| { | ||||||||||||||
| string generatedCode = await GenerateCode(); | ||||||||||||||
| BuildHelper | ||||||||||||||
| .BuildCSharp(generatedCode) | ||||||||||||||
| .Should() | ||||||||||||||
| .BeTrue(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [Test] | ||||||||||||||
| public async Task Generated_Code_Places_JsonConverter_On_Enum_Type_Not_Property() | ||||||||||||||
| { | ||||||||||||||
| string generatedCode = await GenerateCode(); | ||||||||||||||
|
|
||||||||||||||
| using (new AssertionScope()) | ||||||||||||||
| { | ||||||||||||||
| // [JsonConverter] should appear immediately before the enum type declaration | ||||||||||||||
| generatedCode.Should().MatchRegex( | ||||||||||||||
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter\)\)\][\r\n\s]+public enum"); | ||||||||||||||
| // Enum properties should NOT have [JsonConverter] directly on them | ||||||||||||||
| generatedCode.Should().NotMatchRegex( | ||||||||||||||
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter[^)]*\)\)\][\r\n\s]+public \w+Id\b"); | ||||||||||||||
|
Comment on lines
+99
to
+101
|
||||||||||||||
| // Enum properties should NOT have [JsonConverter] directly on them | |
| generatedCode.Should().NotMatchRegex( | |
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter[^)]*\)\)\][\r\n\s]+public \w+Id\b"); | |
| // Enum properties should NOT have [JsonConverter] applied to them (only enum types may have it) | |
| generatedCode.Should().NotMatchRegex( | |
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter[^)]*\)\)\)[\s\S]*?public\s+(?!enum)\w+\s+\w*Id\b"); |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -94,11 +94,28 @@ public async Task Generated_Code_Contains_JsonConverter_By_Default() | |||||||||||||
|
|
||||||||||||||
| using (new AssertionScope()) | ||||||||||||||
| { | ||||||||||||||
| generatedCode.Should().Contain("[JsonConverter(typeof(JsonStringEnumConverter"); | ||||||||||||||
| generatedCode.Should().Contain("[JsonConverter(typeof(JsonStringEnumConverter))]"); | ||||||||||||||
| generatedCode.Should().Contain("public enum PetStatus"); | ||||||||||||||
| generatedCode.Should().Contain("Status { get; set; }"); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [Test] | ||||||||||||||
| public async Task Generated_Code_Places_JsonConverter_On_Enum_Type_Not_Property() | ||||||||||||||
| { | ||||||||||||||
| string generatedCode = await GenerateCode(inlineJsonConverters: true); | ||||||||||||||
|
|
||||||||||||||
| using (new AssertionScope()) | ||||||||||||||
| { | ||||||||||||||
| // [JsonConverter] should appear immediately before the enum type declaration | ||||||||||||||
| generatedCode.Should().MatchRegex( | ||||||||||||||
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter\)\)\][\r\n\s]+public enum PetStatus"); | ||||||||||||||
| // The property line itself should NOT be preceded by [JsonConverter] | ||||||||||||||
| generatedCode.Should().NotMatchRegex( | ||||||||||||||
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter[^)]*\)\)\][\r\n\s]+public PetStatus"); | ||||||||||||||
|
Comment on lines
+113
to
+115
|
||||||||||||||
| // The property line itself should NOT be preceded by [JsonConverter] | |
| generatedCode.Should().NotMatchRegex( | |
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter[^)]*\)\)\][\r\n\s]+public PetStatus"); | |
| // The property line itself should NOT be preceded by [JsonConverter], even if other attributes are present | |
| generatedCode.Should().NotMatchRegex( | |
| @"\[JsonConverter\(typeof\(JsonStringEnumConverter[^)]*\)\)\](?:[\r\n\s]*\[[^\]]*\])*[\r\n\s]*public PetStatus"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enum-attribute insertion uses a hard-coded "\n" and a fully-qualified attribute name. This can introduce mixed line endings in otherwise CRLF output, and it also makes contracts-only / GenerateMultipleFiles output noisier because the later namespace-stripping step doesn’t run there. Consider emitting the attribute using the same line ending as the input (or Environment.NewLine) and using the non-qualified attribute/type names when the relevant using is already present.