RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.69k stars 1.24k forks source link

UrlBuilder no longer ensures the BaseUrl and path are concatenated with a slash #4697

Open jonmarozick opened 8 months ago

jonmarozick commented 8 months ago

When upgrading from version 13.20 to 14.0 the following generated C# code no longer prepends a slash to the path.

v13.20 code

    [System.CodeDom.Compiler.GeneratedCode("NSwag", "13.20.0.0 (NJsonSchema v10.9.0.0 (Newtonsoft.Json v13.0.0.0))")]
    public partial class DomainIdentityApiClient : ApiClientBase, IDomainIdentityApiClient
    {
        // Removed code for brevity

        public virtual async System.Threading.Tasks.Task AuthAsync(Credential body = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
        {
            var urlBuilder_ = new System.Text.StringBuilder();
            urlBuilder_.Append(BaseUrl != null ? BaseUrl.TrimEnd('/') : "").Append("/v1/auth");

            var client_ = _httpClient;
        }

v14.0 code

    [System.CodeDom.Compiler.GeneratedCode("NSwag", "14.0.0.0 (NJsonSchema v11.0.0.0 (Newtonsoft.Json v13.0.0.0))")]
    public partial class DomainIdentityApiClient : ApiClientBase, IDomainIdentityApiClient
    {
        // Removed code for brevity

        public virtual async System.Threading.Tasks.Task AuthAsync(Credential body = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
        {
            var client_ = _httpClient;
            var disposeClient_ = false;
            try
            {
                using (var request_ = new System.Net.Http.HttpRequestMessage())
                {
                    var json_ = Newtonsoft.Json.JsonConvert.SerializeObject(body, _settings.Value);
                    var content_ = new System.Net.Http.StringContent(json_);
                    content_.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse("application/json-patch+json");
                    request_.Content = content_;
                    request_.Method = new System.Net.Http.HttpMethod("POST");

                    var urlBuilder_ = new System.Text.StringBuilder();
                    if (!string.IsNullOrEmpty(BaseUrl)) urlBuilder_.Append(BaseUrl);
                    // Operation Path: "v1/auth"
                    urlBuilder_.Append("v1/auth"); <------------------------------------ Path is appended without the leading slash

                    PrepareRequest(client_, request_, urlBuilder_);

This change was not listed on the breaking changes section. Was this intentional?

I was able to work-around this by adding code the base class constructor, but it was a surprise.

RicoSuter commented 8 months ago

@paulomorgado is this a known regression? Should we fix that?

vvdb-architecture commented 8 months ago

Woulnd't it be better to ensure that BaseUrl always ends with a '/' when its value is assigned?

paulomorgado commented 8 months ago

@RicoSuter,

The strategy is now to guarantee that the base URL has the trailing /.

Maybe we should do the other way around and remove the trailing / from the base URL and force on the path.

Or maybe we should just use the Uri class for that. But that would be a breaking change.

@jonmarozick,

What changes did you have to make in the constructor? Does this PR (#4691) fix it?

paulomorgado commented 8 months ago

@vvdb-architecture

Woulnd't it be better to ensure that BaseUrl always ends with a '/' when its value is assigned?

That's the strategy now.

jonmarozick commented 8 months ago

This is my base class. I just ensured that the base url has a trialing slash. PR #4691 looks like it will fix it. Thanks.

    public class ApiClientBase
    {
        /// <summary>
        /// Initializes a new instance of the <see cref="ApiClientBase"/> class.
        /// </summary>
        /// <param name="configuration">The configuration.</param>
        public ApiClientBase(DomainIdentityApiClientConfiguration configuration)
        {
            if (configuration == null)
            {
                throw new ArgumentNullException(nameof(configuration));
            }

            // this.BaseUrl = configuration.BaseUrl; <----- original code
            this.BaseUrl = configuration.BaseUrl != null ? configuration.BaseUrl.TrimEnd('/') + "/" : string.Empty;
        }

        /// <summary>
        /// Gets or sets the base URL.
        /// </summary>
        protected string BaseUrl { get; set; }
    }