C# code generation with /GenerateBaseUrlProperty:false option is broken in v14.0.1

[olegd-superoffice]

NSwag generates invalid C# code when /GenerateBaseUrlProperty:false option is used. It looks like it is a regression in v14.0.1 possibly caused by #4691 Here's a simple project which demonstrates the regression: GenerateBaseUrlPropertyFalse.zip. It will download and use example petstore OpenAPI document. Just run dotnet build. It works with NSwag.ApiDescription.Client 13.20.0 and 14.0.0 but fails with 14.0.1

@paulomorgado

[paulomorgado]

@olegd-superoffice, please, explain in what way it doesn't work.

[paulomorgado]

@RicoSuter,

I think I'm getting something wrong here.

Is the _baseUrl field supposed to be used anywhere other than inside the BaseUrl property?

HasBaseUrl indicates that the base URL from the API definition is to be used, right?

UseBaseUrl indicates that a base URL should be used, right?

GenerateBaseUrlProperty indicates that a BaseUrl property is to be generated, right?

InjectHttpClient indicates that an HttpClient property is to be injected, right?

The difference between 14.0.1 and 14.0.0 and 13.20.0 is that 14.0.1 assigns the _baseUrl field without declaring it, but all versions use the baseUrl property without declaring it when /GenerateBaseUrlProperty:false is specified.

My reasoning how this should be is:

• if UseBaseUrl is true, _baseUrl or BaseUrl should be used, depending on BaseUrl being generated or not.
  • Or always use BaseUrl and make it public only if /GenerateBaseUrlProperty:true.
• if UseBaseUrl is true
  • if HasBaseUrl is true , set either _baseUrl or BaseUrl to the sanitzed BaseUrl.
  • Or always use BaseUrl and make it public only if /GenerateBaseUrlProperty:true.
  • if HasBaseUrl is false , add a baseUrl parameter to the constructor and set either _baseUrl or BaseUrl to the sanitzed BaseUrl.
  • Or always use BaseUrl and make it public only if /GenerateBaseUrlProperty:true.

Is that how it should be?

[paulomorgado]

@RicoSuter,

Seems like the change is to never initialize the _baseUrlfield and initialize BaseUrl property instead.

And there's a breaking change for implementers of the BaseUrl property that need to provide a / terminated URL.

[W0GER]

But why force implementers to provide a /terminated URL. This seems like a potential spot for bugs.

If the BaseUrl has a value, version 13.20 would check to see if it had a trailing / before concatenating with the rest of the URL and query strings.

[paulomorgado]

I changed it as a performance optimization, but I'm rethinking it.

[RicoSuter]

Should be fixed with this commit: https://github.com/RicoSuter/NSwag/commit/be42e58d1e2f9c4e86b9c302b0c6e82cc5c36d45

Sorry for all the inconveniences...

[olegd-superoffice]

@RicoSuter The 14.0.3 generated code is getting compiled without errors now but the logic of how /GenerateBaseUrlProperty:false flag works is different now.

• Up to (and including) 14.0.0 it meant that BaseUrl property was not generated but still was used and it was responsibility of base class to declare and assign a BaseUrl value (usually from configuration).
• In 14.0.1 and 14.0.2 it was broken and code generated with /GenerateBaseUrlProperty:false flag didn't compile.
• In 14.0.3 code is getting compiled again but works differently - BaseUrl property is not generated and is not used at all. Instead private string _baseUrl field is assigned from a host value defined in OpenAPI document. The only workaround I could find to change _baseUrl value is to put some weird logic into override of PrepareRequest method.

It looks like it is still a breaking change between 14.0.0 and 14.0.3.

[paulomorgado]

I think I was addressing all issues with https://github.com/paulomorgado/NSwag/commit/744940b6c5b90a06c3c64f98e8ed247de34a5b26, but it's now failing this recently added test:

[Fact]
public async Task WhenUsingBaseUrl_ButNoProperty_ThenPropertyIsNotUsedAndFieldIsGenerated()
{
    // Arrange
    var swaggerGenerator = new WebApiOpenApiDocumentGenerator(new WebApiOpenApiDocumentGeneratorSettings
    {
        SchemaSettings = new NewtonsoftJsonSchemaGeneratorSettings()
    });
    var document = await swaggerGenerator.GenerateForControllerAsync<FooController>();
    var generator = new CSharpClientGenerator(document, new CSharpClientGeneratorSettings
    {
        UseBaseUrl = true,
        GenerateBaseUrlProperty = false
    });
    // Act
    var code = generator.GenerateFile();
    // Assert
    Assert.DoesNotContain("BaseUrl", code);
    Assert.Contains("string _baseUrl", code);
}

I still don't have a full grasp of the usage of -baseUrl and BaseUrl.

[alexcheveau]

I can confirm the impact on my project as well.

My use case is: useBaseUrl=true generateBaseUrlProperty=false

I have a base class that implements BaseURL and return the urlto be used in urlBuilder but now it's ignored. The proxy is using directly _baseUrl instead of BaseURL

[Trapulo]

I also have this problem. I cannot assign baseUrl in base class because generated classes use locally defined _baseUrl ignoring external settings.

[soligen2010]

For me, 14.0.1 works as expected using BaseUrl from the base class 14.0.2 works as long as the swagger.json does not have a BasePath. If it has a BasePath I get a compile error because _baseUrl is not defined. 14.0.3 compiles, but the BasePath in the base class gets ignored, so things don't work.

I am running with UseBaseUrl = true and GenerateBaseUrlProperty = false.

For my application, I need to be able to set the base Url at run time, even if a Base Path is specified in swagger.json. Is this going to be changed back so the base class can control the base Url, or do I need to find another way to do this?

For now I have to stay on 14.0.1

Thanks

[franklixuefei]

14.0.7 still doesn't work. It is complaining and the BaseUrl property defined in the base class is now not referenced at all. What's the new logic of setting the base url?

[patophasselt]

When will this issue be fixed? The code should use the BaseUrl from the ClientBase instead of hardcoding the original URL in the _baseUrl. Please also revert the logic to trim a trailing '/'. As mentioned above, this potentially introduces bugs.