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

Generated client code does not replace/set route parameters with 14.0.0-preview010 #4587

Open
vas6ili opened this issue Nov 17, 2023 · 9 comments

Comments

@vas6ili
Copy link

vas6ili commented Nov 17, 2023

For a path template like /odata/QueueRetention({key}) generated client code does not replace {key} url route parameter. Used net80/dotnet-nswag.exe from https://github.com/RicoSuter/NSwag/releases/tag/v14.0.0-preview010 with net8 (which is the reason I'm upgrading to 14)

Generated client code snippet

var urlBuilder_ = new System.Text.StringBuilder();

urlBuilder_.Append("odata");
urlBuilder_.Append('/');
urlBuilder_.Append("QueueRetention({key})");

PrepareRequest(client_, request_, urlBuilder_);

var url_ = urlBuilder_.ToString();
request_.RequestUri = new System.Uri(url_, System.UriKind.RelativeOrAbsolute);

PrepareRequest(client_, request_, url_);

API definition snippet

    "/odata/QueueRetention({key})": {
        "delete": {
            "description": "",
            "operationId": "QueueRetention_Delete",
            "parameters": [
                {
                    "in": "path",
                    "name": "key",
                    "required": true,
                    "schema": {
                        "format": "int64",
                        "type": "integer"
                    },
                    "x-position": 1
                },
                {
                    "in": "header",
                    "name": "api-version",
                    "schema": {
                        "nullable": true,
                        "type": "string"
                    },
                    "x-position": 2
                }
            ],
            "responses": {
                "204": {
                    "description": ""
                }
            },
            "tags": [
                "QueueRetention"
            ]
        }
    }

test-swag.json
NswagClient.zip

Probably related to #4579 @paulomorgado

@paulomorgado
Copy link
Contributor

OOPS! I'll work on that tomorrow.

@RicoSuter, what's the best way to add a unit test for this?

@lahma
Copy link
Collaborator

lahma commented Nov 17, 2023

NJsonSchema uses Verify whici is IMO a nice way to test as snapshots are produced and VCS controlled and you can see differences in pull request when output changes. I think this hasn't been taken into use (yet) on NSwag's side though.

@paulomorgado
Copy link
Contributor

I've added corrections to my draft PR: #4585.

Please, checkout if that solves the issue.

@vas6ili
Copy link
Author

vas6ili commented Nov 17, 2023

It did solve the issue. I'm inspecting a quite large 8 MB generated client from a 2.5 MB swagger file and haven't found any other issue yet.

@RicoSuter
Copy link
Owner

Can you confirm that the latest preview works fine for you?

@vas6ili
Copy link
Author

vas6ili commented Dec 12, 2023

Can confirm with latest RC3 the issue no longer reproduces

@bzwieratinnovadis
Copy link

bzwieratinnovadis commented Jan 4, 2024

@RicoSuter @vas6ili I'm having a similar issue with route parameters with the stable release (v14.0.0) in a .NET 8 project.
I'm using NSwag.MSBuild to generate a CSharp client.

I have the following controller action:

[HttpGet("confirm/{batch}")]
public Task<MessageBoxConfirmBatchModel> ConfirmAsync(Guid batch) => _batchService.ConfirmAsync(batch);

OpenAPI definition:

"/api/message-box-batch/confirm/{batch}": {
  "get": {
    "tags": [
      "MessageBoxBatch"
    ],
    "parameters": [
      {
        "name": "OrganizationReference",
        "in": "header",
        "required": true,
        "schema": {
          "type": "string",
          "format": "uuid"
        }
      },
      {
        "name": "batch",
        "in": "path",
        "required": true,
        "schema": {
          "type": "string",
          "format": "uuid"
        }
      }
    ],
    "responses": {
      "200": {
        "description": "Success",
        "content": {
          "text/plain": {
            "schema": {
              "$ref": "#/components/schemas/MessageBoxConfirmBatchModel"
            }
          },
          "application/json": {
            "schema": {
              "$ref": "#/components/schemas/MessageBoxConfirmBatchModel"
            }
          },
          "text/json": {
            "schema": {
              "$ref": "#/components/schemas/MessageBoxConfirmBatchModel"
            }
          }
        }
      },
      "204": {
        "description": "No Content"
      },
      "400": {
        "description": "Bad Request",
        "content": {
          "application/json": {
            "schema": {
              "$ref": "#/components/schemas/ValidationProblemDetails"
            }
          }
        }
      },
      "500": {
        "description": "Internal Server Error",
        "content": {
          "application/json": {
            "schema": {
              "$ref": "#/components/schemas/ProblemDetails"
            }
          }
        }
      }
    }
  }
}

The resulting client:

var urlBuilder_ = new System.Text.StringBuilder();
urlBuilder_.Append("api/message-box-batch/confirm/{batch}");
urlBuilder_.Append(System.Uri.EscapeDataString(ConvertToString(batch, System.Globalization.CultureInfo.InvariantCulture)));

The request URI ends up being: api/message-box-batch/confirm/{batch}462995b7-433b-4382-b4d7-5f84599cf732

When the client is called the API returns a 400 Bad Request response due to the malformed URI.

In v13.20.0 the client was generated correctly:

var urlBuilder_ = new System.Text.StringBuilder();
urlBuilder_.Append("api/message-box-batch/confirm/{batch}");
urlBuilder_.Replace("{batch}", System.Uri.EscapeDataString(ConvertToString(batch, System.Globalization.CultureInfo.InvariantCulture)));

Is there some workaround in terms of configuration? Or should I wait for the next minor/patch version?

@paulomorgado
Copy link
Contributor

Thanks for the concrete example, @bzwieratinnovadis.

Which tooling and version are you using?

I just tried that example on NSwagStudio v14.0.0.0 and got this generated code:

var urlBuilder_ = new System.Text.StringBuilder();
if (!string.IsNullOrEmpty(BaseUrl)) urlBuilder_.Append(BaseUrl);
// Operation Path: "api/message-box-batch/confirm/{batch}"
urlBuilder_.Append("api/message-box-batch/confirm/");
urlBuilder_.Append(System.Uri.EscapeDataString(ConvertToString(batch, System.Globalization.CultureInfo.InvariantCulture)));

@bzwieratinnovadis
Copy link

bzwieratinnovadis commented Jan 5, 2024

Thanks for the concrete example, @bzwieratinnovadis.

Which tooling and version are you using?

I just tried that example on NSwagStudio v14.0.0.0 and got this generated code:

var urlBuilder_ = new System.Text.StringBuilder();
if (!string.IsNullOrEmpty(BaseUrl)) urlBuilder_.Append(BaseUrl);
// Operation Path: "api/message-box-batch/confirm/{batch}"
urlBuilder_.Append("api/message-box-batch/confirm/");
urlBuilder_.Append(System.Uri.EscapeDataString(ConvertToString(batch, System.Globalization.CultureInfo.InvariantCulture)));

So I've been messing around with NSwagStudio to reproduce the issue and figured out why the code was being generated incorrecly. Apparently there was a template being used that causes the issue because without it everything is fine and path/route parameters are handled correctly.

I closed related #4683

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants