RicoSuter / NSwag

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

V14 BaseUrl property now needs to be defined twice (where as V13 you only specified it once) #4737

Closed xantari closed 6 months ago

xantari commented 7 months ago

There seems to be a difference in how V14 generates the base URL when the "useBaseUrl": true flag is turned on within the nswag.json configuration.

Now it uses both _baseUrl (in the constructor of the generated client), and BaseUrl (in the methods of the generated client).

This now means that I need to create my base class with both properties as shown here:

        protected string _baseUrl = "https://api.somewhere.com/api";

        public string BaseUrl
        {
            get { return _baseUrl; }
            set { _baseUrl = value; }
        }

In V13 all that was needed was the following:

    public string BaseUrl { get; set; } = "http://api.somewhere.com/api"

Is there a reason for this change in defining base url with both a internal _baseUrl parameter and a BaseUrl parameter?

It seems like you could get rid of _baseUrl and just set the constructor version to use BaseUrl like in V13 to get rid of the extra field that was introduced in V14?

@paulomorgado @RicoSuter Thoughts on this?

paulomorgado commented 7 months ago

@xantari, is this the same issue as #4705

xantari commented 7 months ago

@paulomorgado That looks a bit different, but may be related. In my case I have useBaseUrl: true, it sounds like #4705 is an issue where it is false.

paulomorgado commented 7 months ago

@xantari, can you provide a sample swagger document and the settings used for generation?

xantari commented 7 months ago

@paulomorgado

YML spec:

https://api.galaxydigital.com/docs

Swagger Json config:

{
  "runtime": "Net60",
  "defaultVariables": null,
  "documentGenerator": {
    "fromDocument": {
      "json": "",
      "url": "galaxydigital_1.5.3.yml", //URL method doesn't work due to bug in nswag: https://api.galaxydigital.com/docs/api.yml
      "output": null,
      "newLineBehavior": "Auto"
    }
  },
  "codeGenerators": {
    "openApiToCSharpClient": {
      "clientBaseClass": "ClientBase", //name of your client base class
      "configurationClass": "IGalaxyDigitalApiConfiguration",
      "useBaseUrl": true,
      "className": "GalaxyDigitalClient",
      "generateBaseUrlProperty": false,
      "generateClientClasses": true,
      "generateClientInterfaces": true,
      "useHttpClientCreationMethod": true,
      "generateSyncMethods": true,
      "disposeHttpClient": true,
      "httpClientType": "System.Net.Http.HttpClient",
      "useHttpRequestMessageCreationMethod": true,
      "injectHttpClient": true,
      "clientClassAccessModifier": "public",
      "typeAccessModifier": "public", //make your models and client interfaces public
      "generateContractsOutput": true, //generate contacts in a separte file
      "contractsNamespace": "GalaxyDigital.Api.Client.Contracts", //contracts namespace
      "contractsOutputFilePath": "Contracts.g.cs",
      "namespace": "GalaxyDigital.Api.Client", //clients namespace
      "output": "Client.g.cs",
      "datetimetype": "System.DateTime",
      "requiredPropertiesMustBeDefined": false,
      "generateOptionalPropertiesAsNullable": true,
      "generateNullableReferenceTypes": false,
      "generateDefaultValues": true,
      "generateOptionalParameters": true
    }
  }
}

Notice how in V14 it generates a _baseUrl property in the constructor of the client, but in V13 it never did that. Instead it used BaseUrl exclusively.

paulomorgado commented 7 months ago

@xantari,

Yes, it's the same issue.

RicoSuter commented 6 months ago

Hopefully this is fixed with this commit? https://github.com/RicoSuter/NSwag/commit/be42e58d1e2f9c4e86b9c302b0c6e82cc5c36d45

xantari commented 6 months ago

@RicoSuter Looks like that would fix it. From the looks of it that changes everything from BaseUrl (used in V13 and prior), to using a new property _baseUrl in V14 and beyond. Might want to put that as a breaking change for those who used the BaseUrl property in V13 and earlier that they will need to change their base url to _baseUrl...

Trapulo commented 6 months ago

I still have problems after upgrade from 13 to 14. [System.CodeDom.Compiler.GeneratedCode("NSwag", "14.0.3.0 (NJsonSchema v11.0.0.0 (Newtonsoft.Json v13.0.0.0))")]

The generated code is:

public partial class CitiesClient CoreWebApiClientBase
 {
     #pragma warning disable 8618
     private string _baseUrl;
     #pragma warning restore 8618

private static System.Lazy<Newtonsoft.Json.JsonSerializerSettings> _settings = new System.Lazy<Newtonsoft.Json.JsonSerializerSettings>(CreateSerializerSettings, true);

  public CitiesClient(CoreWebApiClientConfiguration configuration) : base(configuration)
  {
      _baseUrl = "https://xXXXXXX/";
  }

where xxx is the url where I pointed NSWAG Studio to download the OpenAPI specification. CoreWebApiClientBase is my base class where I defined BaseUrl as property in b13, now as:

public string BaseUrl
 {
     get { return _baseUrl; }
     set { _baseUrl = value; }
 }
 protected string _baseUrl;

So constructor is overriding every setting my base class has defined using a fixed value and also overrides _baseUrl variable where my property saves right value.

anbuDW commented 5 months ago

I ran into the same issue. It seems that it is currently (14.0.3.) not possible to use the clientBaseClass and configurationClass property together with useBaseUrl. useBaseUrl will generate a _baseUrl variable that overrides the _baseUrl in my clientBase class and which will always be null. The only way to fill _baseUrl is by not using a configurationClass, which will make the baseUrl parameter in the client constructor available. But that way I will loose all my other configurations for my clientBase class.

xantari commented 5 months ago

@anbuDW FYI if you revert back to 14.0.2 you can get around the baseUrl issue... 14.0.3 I ran into the same issue as you and had to revert back.

kemmis commented 4 months ago

Same issue here. Had to revert back to 14.0.2. :(