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

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

Open
jonmarozick opened this issue Jan 9, 2024 · 5 comments

Comments

@jonmarozick
Copy link

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
Copy link
Owner

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

@vvdb-architecture
Copy link

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

@paulomorgado
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Author

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; }
    }

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

No branches or pull requests

4 participants