-
-
Notifications
You must be signed in to change notification settings - Fork 64
Optional parameters with default values #803
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 4 commits
935cfb9
d4435c4
181f74e
8091fe4
284bdc8
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 | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -86,7 +86,7 @@ public static IEnumerable<string> GetParameters( | |||||||||||||||||||||||||||||||||||||||||||||
| parameters.AddRange(formParameters); | ||||||||||||||||||||||||||||||||||||||||||||||
| parameters.AddRange(binaryBodyParameters); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| parameters = ReOrderNullableParameters(parameters, settings); | ||||||||||||||||||||||||||||||||||||||||||||||
| parameters = ReOrderNullableParameters(parameters, settings, operationModel.Parameters); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (settings.ApizrSettings?.WithRequestOptions == true) | ||||||||||||||||||||||||||||||||||||||||||||||
| parameters.Add("[RequestOptions] IApizrRequestOptions options"); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -116,7 +116,8 @@ private static string ReplaceUnsafeCharacters( | |||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| private static List<string> ReOrderNullableParameters( | ||||||||||||||||||||||||||||||||||||||||||||||
| List<string> parameters, | ||||||||||||||||||||||||||||||||||||||||||||||
| RefitGeneratorSettings settings) | ||||||||||||||||||||||||||||||||||||||||||||||
| RefitGeneratorSettings settings, | ||||||||||||||||||||||||||||||||||||||||||||||
| ICollection<CSharpParameterModel> parameterModels) | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (!settings.OptionalParameters || settings.ApizrSettings?.WithRequestOptions == true) | ||||||||||||||||||||||||||||||||||||||||||||||
| return parameters; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -125,12 +126,54 @@ private static List<string> ReOrderNullableParameters( | |||||||||||||||||||||||||||||||||||||||||||||
| for (int index = 0; index < parameters.Count; index++) | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (parameters[index].Contains("?")) | ||||||||||||||||||||||||||||||||||||||||||||||
| parameters[index] += " = default"; | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| var parameterString = parameters[index]; | ||||||||||||||||||||||||||||||||||||||||||||||
| var defaultValue = GetDefaultValueForParameter(parameterString, parameterModels); | ||||||||||||||||||||||||||||||||||||||||||||||
| parameters[index] = parameterString + " = " + defaultValue; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| return parameters; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| private static string GetDefaultValueForParameter(string parameterString, ICollection<CSharpParameterModel> parameterModels) | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| var parts = parameterString.Split([' '], StringSplitOptions.RemoveEmptyEntries); | ||||||||||||||||||||||||||||||||||||||||||||||
| if (parts.Length == 0) | ||||||||||||||||||||||||||||||||||||||||||||||
| return "default"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| var variableName = parts[parts.Length - 1].TrimEnd(';', ','); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| var parameterModel = parameterModels.FirstOrDefault(p => p.VariableName == variableName); | ||||||||||||||||||||||||||||||||||||||||||||||
| return parameterModel?.Schema?.Default != null | ||||||||||||||||||||||||||||||||||||||||||||||
| ? FormatDefaultValue(parameterModel.Schema.Default, parameterModel.Type) | ||||||||||||||||||||||||||||||||||||||||||||||
| : "default"; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| private static string FormatDefaultValue(object? defaultValue, string parameterType) | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (defaultValue == null) | ||||||||||||||||||||||||||||||||||||||||||||||
| return "default"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| var type = parameterType.TrimEnd('?').Trim(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| return type switch | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| "bool" => defaultValue.ToString()?.ToLowerInvariant() ?? "default", | ||||||||||||||||||||||||||||||||||||||||||||||
| "string" => $"\"{defaultValue}\"", | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| _ when IsNumericType(type) => defaultValue.ToString() ?? "default", | ||||||||||||||||||||||||||||||||||||||||||||||
| _ => "default" | ||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+153
to
+167
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix string escaping to prevent invalid generated code. Line 163 wraps string default values in quotes but doesn't escape existing quotes or special characters. If the OpenAPI schema specifies a default like string? filter = "my"test" // Invalid C#
string? path = "path\to\file" // Invalid escape sequenceApply this diff to add proper string escaping: - "string" => $"\"{defaultValue}\"",
+ "string" => $"\"{defaultValue.ToString()?.Replace("\\", "\\\\").Replace("\"", "\\\"")}\"",Alternatively, use C#'s verbatim string literals or 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+164
to
+168
|
||||||||||||||||||||||||||||||||||||||||||||||
| _ when IsNumericType(type) => defaultValue.ToString() ?? "default", | |
| _ => "default" | |
| }; | |
| } | |
| _ when IsNumericType(type) => FormatNumericValue(defaultValue, type), | |
| _ => "default" | |
| }; | |
| } | |
| private static string FormatNumericValue(object defaultValue, string type) | |
| { | |
| var numericString = defaultValue is IFormattable formattable | |
| ? formattable.ToString(null, CultureInfo.InvariantCulture) | |
| : (defaultValue.ToString() ?? "default"); | |
| return type switch | |
| { | |
| "float" or "Single" => $"{numericString}f", | |
| "decimal" or "Decimal" => $"{numericString}m", | |
| _ => numericString | |
| }; | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| using FluentAssertions; | ||
| using Refitter.Core; | ||
| using Refitter.Tests.Build; | ||
| using Refitter.Tests.TestUtilities; | ||
| using Xunit; | ||
|
|
||
| namespace Refitter.Tests.Examples; | ||
|
|
||
| public class OptionalParametersWithDefaultValuesTests | ||
| { | ||
| private const string OpenApiSpec = | ||
| """ | ||
| { | ||
| "openapi": "3.0.1", | ||
| "info": { | ||
| "title": "Test API", | ||
| "version": "v1" | ||
| }, | ||
| "paths": { | ||
| "/api/schedule/list": { | ||
| "get": { | ||
| "tags": ["Schedule"], | ||
| "operationId": "List", | ||
| "parameters": [ | ||
| { | ||
| "name": "start", | ||
| "in": "query", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string", | ||
| "format": "date" | ||
| } | ||
| }, | ||
| { | ||
| "name": "end", | ||
| "in": "query", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string", | ||
| "format": "date" | ||
| } | ||
| }, | ||
| { | ||
| "name": "includeCancelled", | ||
| "in": "query", | ||
| "required": false, | ||
| "schema": { | ||
| "type": "boolean", | ||
| "default": true | ||
| } | ||
| }, | ||
| { | ||
| "name": "pageSize", | ||
| "in": "query", | ||
| "required": false, | ||
| "schema": { | ||
| "type": "integer", | ||
| "format": "int32", | ||
| "default": 10 | ||
| } | ||
| }, | ||
| { | ||
| "name": "filter", | ||
| "in": "query", | ||
| "required": false, | ||
| "schema": { | ||
| "type": "string", | ||
| "default": "active" | ||
| } | ||
| } | ||
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "Success", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| """; | ||
|
|
||
| [Fact] | ||
| public async Task Can_Generate_Code() | ||
| { | ||
| string generatedCode = await GenerateCode(); | ||
| generatedCode.Should().NotBeNullOrWhiteSpace(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Generated_Code_Should_Have_Optional_Parameters_With_Default_Values() | ||
| { | ||
| string generatedCode = await GenerateCode(); | ||
| generatedCode.Should().Contain("bool? includeCancelled = true"); | ||
| generatedCode.Should().Contain("int? pageSize = 10"); | ||
| generatedCode.Should().Contain("string? filter = \"active\""); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Generated_Code_Should_Have_Required_Parameters_Without_Defaults() | ||
| { | ||
| string generatedCode = await GenerateCode(); | ||
| generatedCode.Should().Contain("[Query] System.DateTimeOffset start"); | ||
| generatedCode.Should().Contain("[Query] System.DateTimeOffset end"); | ||
| generatedCode.Should().NotContain("System.DateTimeOffset start ="); | ||
| generatedCode.Should().NotContain("System.DateTimeOffset end ="); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Can_Build_Generated_Code() | ||
| { | ||
| string generatedCode = await GenerateCode(); | ||
| BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); | ||
| } | ||
|
|
||
| private static async Task<string> GenerateCode() | ||
| { | ||
| var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(OpenApiSpec); | ||
| var settings = new RefitGeneratorSettings | ||
| { | ||
| OpenApiPath = swaggerFile, | ||
| OptionalParameters = true | ||
| }; | ||
|
|
||
| var sut = await RefitGenerator.CreateAsync(settings); | ||
| var code = sut.Generate(); | ||
| return code; | ||
| } | ||
| } |
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 code doesn't handle the case where
parameterModel.Typecould be null or empty. While this is unlikely given that it comes from NSwag's code generation, it would be safer to add a null check to prevent potentialNullReferenceException.Suggestion: