Fix for BaseUrl in C# base class not being used

[soligen2010]

Fix to use the BaseUrl from the base class when there is a base class and GenerateBaseUrlProperty is not set.

See issue #4764

Edit: This is for C# code generation

[soligen2010]

Added new commit to fix the If statement and added a new test case for when there is a BaseClass.

[soligen2010]

Can someone please help me understand why this PR is failing? Looking at the errors in the latest run it doesn't seem to me to be related to something I changed. Is there something more I need to do?

[soligen2010]

I finally got my local installation working properly and found that I had 4 extra spaces added to a line in the generated client which caused the file to not match the verified. Hopefully this time the workflow works.

[soligen2010]

Everything runs successfully on my local machine. The workflow error is

"You must install or update .NET to run this application."

I don't understand how anything I did could cause this, and I can't do anything about what is installed on the workflow server.

Can someone please advise?

Thanks

[Numpsy]

"You must install or update .NET to run this application."

Looks like it's trying to run the tests as .NET 6.0, and the build agent only has 7.0/8.0 installed.

Possibly because the CI build is set to use macos-latest and that recently became macOS 14 running on ARM cpus? (so nothing specific to do with the change here)

[franklixuefei]

does this PR have any recent tractions?

[soligen2010]

I don't know what you mean by "recent tractions" This PR addresses an issue that was first introduced in 14.0.1, possibly introduced by PR #4691. There were further changes in this area after that PR, but they didn't fix this issue.

I believe this PR fixes issues #4764 and #4705.

This PR is needed by anyone setting BaseUrl in a base class. For anyone who does this, the recent versions are not usable - they cant upgrade to the most recent version.

[akeijzer11]

When will this issue be resolved? Since 14.0.1 i tried every new release if the issue was fixed but setting the BaseUrl in a base class is still not possible.

[soligen2010]

@RicoSuter Can you please provide some feedback on acceptance of this pull request. This issue is affecting a number of people who are waiting for it. I know the MacOS checks failed, but this appears to be an issue with the build server, which I can't fix, and I don't see a way I can re-run the checks to see if the problem has been resolved.

Thanks

[Numpsy]

I don't see a way I can re-run the checks to see if the problem has been resolved.

Maybe update or rebase your branch on top of the latest master and push that to trigger the CI again?

[RicoSuter]

You know that you can overwrite templates on your side without changing NSwag itself?

I'm super careful changing these templates because in the past PRs like this broke a lot of stuff and I had to work days to fix all issues introduced by untested PRs...

[soligen2010]

@RicoSuter thanks for coming into the discussion. I am willing to help with anything I can do to resolve this issue.

I think the issue is that Base Url has been removed even when using a base class to set the Base Url, and there are a number of us (There are a couple of issue threads out there concerning this) that have base classes that do this. In the template, I have set the IF conditions so that behavior is only changed if a Base Class is used and GenerateBaseUrlProperty is false. In these cases the Base URL previously was controlled by the Base Class, and I think all of us affected would agree that we would like this capability restored.

If there is another way we should be now be doing this same thing, then please let me know and I will be happy to try it. I really would prefer to not maintain a fork with customized templates as I think that would cause issues in my organization for version upgrades.

To my mind, this change simply restores the ability to control the Base Url in the base class. If this pull request isn’t acceptable, how else should we change the base URL dynamically? If this capability is intentionally permanently removed, what is your recommendation on how we should do this?

For background, I just changed from Swagger/AutoRest to all NSwag in January. I implemented using a base class because then I could have a similar constructor that minimizes the code remediation in the projects that used the old AutoRest clients (BaseUrl came in on the constructor in our AutoRest clients). About a month after changing to NSwag, this stopped working (It partially broke in 14.0.2 (I found a work around). It fully broke in 14.0.3. Maybe there is a alternate way to do things, but it would likely be a lot of work to re-structure the existing code base to work differently.

Please let me know how to proceed. I am committed to helping resolve this issue.

[cculver]

Thanks @soligen2010, your comment echoes my use case and I am eagerly awaiting a solution to this problem. I am also happy to adjust if there is a way.

[RicoSuter]

This whole BaseUrl is now super messed up and very hard to understand... My personal recommendation is to not use BaseURL at all and instead set the base URL on the injected HttpClient... But will still look into fixing this.

[soligen2010]

This whole BaseUrl is now super messed up and very hard to understand... My personal recommendation is to not use BaseURL at all and instead set the base URL on the injected HttpClient... But will still look into fixing this.

That was one of the first things I tried, but doesn't work with our code base - it probably should work if things are done "right", but for me right now, that would involve a lot of remediation in the code base. Looking through things again, perhaps it can be set using PrepareRequest. I'll give it a try and report back.

[soligen2010]

Looking at this again, I agree there are issues with the base url logic. In the current release (14.0.8) when a Base Class is used and GenerateBaseUrlProperty is false, this following is generated.

#pragma warning disable 8618
private string _baseUrl;
#pragma warning restore 8618

And it is used in each endpoint like this.

if (!string.IsNullOrEmpty(_baseUrl)) urlBuilder_.Append(_baseUrl);

However, there is no way to actually set a value for _baseUrl, so these lines of code never do anything useful. This pull request removes _baseUrl, and instead uses BaseUrl from the base class instead so it seems to me it wont break other use cases.

As for using PrepareRequest, That is looking viable as a work around, but I would prefer the BaseUrl solution. I will post more on that after some testing.

[soligen2010]

The PrepareRequest work around worked. I prefer the BaseUrl solution because PrepareRequest is a partial method, so this code would need to be copied into every client. Having it all in BaseClass allows me to distribute the solution via our internal NuGet server.

Another alternative would be to add a virtual method like PrepareRequest in the template so that the BaseClass can override it.

Below is the workaround I came up with using PrepareRequest. It must be in a file using the same namespace as the generated client class. @RicoSuter please comment on how you would like to proceed in resolving this issue.

    public partial class MyApi : MyNamespace.MyClientBase, IMyApi
    {
        partial void PrepareRequest(System.Net.Http.HttpClient client, System.Net.Http.HttpRequestMessage request, System.Text.StringBuilder urlBuilder)
        {
            if (!string.IsNullOrWhiteSpace(this.BaseUrl) && client?.BaseAddress is null)
            {
                var testUri = new Uri(urlBuilder.ToString(), UriKind.RelativeOrAbsolute);
                if (!testUri.IsAbsoluteUri)
                {
                    urlBuilder.Insert(0, this.BaseUrl);
                }
            }
        }