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

"Unrecognised arguments are present" after updating from 14.0.8 to 14.1.0 #4938

Open
mus65 opened this issue Jul 18, 2024 · 5 comments
Open

Comments

@mus65
Copy link

mus65 commented Jul 18, 2024

Regression #4891 @bkoelman .

If /generateResponseClasses:false is already specified for the API on the Options attribute of <OpenApiReference> in the csproj, this change appears to cause it to be appended again.

We simply removed /generateResponseClasses:false from the <OpenApiReference> as a workaround.

  GenerateNSwagCSharp:
    dotnet --roll-forward-on-no-candidate-fx 2 "/root/.nuget/packages/nswag.msbuild/14.1.0/buildTransitive/../tools/Net80//dotnet-nswag.dll" openapi2csclient /className:<redacted> /namespace:ApiReferences.<redacted> /generateExceptionClasses:false /input:"<redacted>.json" /output:"obj/<redacted.cs" /generateClientClasses:false /generateBaseUrlProperty:false /generateUpdateJsonSerializerSettingsMethod:false /generateJsonMethods:false /generateResponseClasses:false /additionalNamespaceUsages:ApiReferences.Exceptions /generateResponseClasses:false
  NSwag command line tool for .NET Core Net80, toolchain v14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))
  Visit http://nswag.org/ for more information.
  NSwag bin directory: /root/.nuget/packages/nswag.msbuild/14.1.0/tools/Net80
  NConsole.UnusedArgumentException: Unrecognised arguments are present: []
     at NConsole.CommandLineProcessor.ProcessSingleAsync(String[] args, Object input)
     at NConsole.CommandLineProcessor.ProcessAsync(String[] args, Object input)
     at NSwag.Commands.NSwagCommandProcessor.ProcessAsync(String[] args) in /_/src/NSwag.Commands/NSwagCommandProcessor.cs:line 65
@bkoelman
Copy link
Contributor

Hi @mus65, thanks for reporting this.

Some background on what's happening here: When a project has multiple OpenAPI references without any settings, each client generates its own ApiException class. That's usually fine, except when no separate namespaces are configured (the default) because it leads to duplicate ApiException class definitions (and the project doesn't compile). The MSBuild properties (NSwagGenerateExceptionClasses, NSwagGenerateResponseClasses, and others) were introduced in v14. Additionally, it was decided to smoothen the getting-started experience by adding logic that when NSwagGenerateExceptionClasses isn't configured, only the first OpenAPI reference generates the ApiException class. But that logic wasn't implemented properly, so it became impossible to configure multiple clients in separate namespaces, each with their own ApiExplorer class. My PR fixes that. That PR also makes NSwagGenerateResponseClasses behave similarly because the problem is similar.

The error you're experiencing is because I made NSwagGenerateResponseClasses behave similarly to NSwagGenerateExceptionClasses. You would have hit this issue in earlier v14 versions when passing /generateExceptionClasses instead.

Unfortunately, the MSBuild properties can't reliably know which command-line switches are directly set in <Options /> and take that into account (because <Options /> could be set/changed later). What makes it worse, is that the generator executable fails on duplicate or conflicting command-line switches.

I'm very happy with the introduction of MSBuild properties, because it's a huge enabler. Not only does it make the project file easier to read (no more long <Options>...</Options> lines, it enables to control settings from various places. For example, a NuGet package can include a .props file that sets them based on company-wide conventions, and then each OpenAPI reference can override them as needed. Or simply define the properties that apply to all your references in a global PropertyGroup, to reduce duplication.

Personally, I'm not a fan of the magic introduced for the getting-started experience. But it was already there, so I just made it behave consistently.

I would recommend everyone to favor the MSBuild properties over setting <Options /> directly, though I understand getting this error is annoying during an update. It would help if the generator executable wouldn't choke on duplicate command-line switches to improve the experience. A bigger question is how the executable should handle conflicting switches. Assuming the first one wins, what's defined in <Options /> before evaluating NSwagGenerateResponseClasses wins. But if <Options /> is set after NSwagGenerateResponseClasses is evaluated, the switches set in <Options /> would be discarded. So there's no correct answer on whether the first or last switch should win. Properties should not depend on each other, and it's unfortunate that we now have two ways of configuring /generateExceptionClasses.

I'm not sure what's the best approach here. Theoretically, it should be possible to introduce an internal <EvaluatedOptions /> where the information from both sources (MSBuild properties and parsed <Options />) gets merged, and then use <EvaluatedOptions /> with the generator executable.

@RicoSuter any thoughts?

@mus65
Copy link
Author

mus65 commented Jul 18, 2024

Some background on what's happening here: When a project has multiple OpenAPI references without any settings, each client generates its own ApiException class. That's usually fine, except when no separate namespaces are configured (the default) because it leads to duplicate ApiException class definitions (and the project doesn't compile). The MSBuild properties (NSwagGenerateExceptionClasses, NSwagGenerateResponseClasses, and others) were introduced in v14. Additionally, it was decided to smoothen the getting-started experience by adding logic that when NSwagGenerateExceptionClasses isn't configured, only the first OpenAPI reference generates the ApiException class.

The way we are (still) working around this is by having a copy of the ApiException class in our own namespace and setting the following in the csproj. Obviously having our own copy of the class is not great, but this workaround was good enough for us.

  <PropertyGroup>
    <NSwagGenerateExceptionClasses>false</NSwagGenerateExceptionClasses>
    <NSwagAdditionalNamespaceUsages><OurNamespace>.Exceptions</NSwagAdditionalNamespaceUsages>
  </PropertyGroup>

But that logic wasn't implemented properly, so it became impossible to configure multiple clients in separate namespaces, each with their own ApiExplorer class. My PR fixes that. That PR also makes NSwagGenerateResponseClasses behave similarly because the problem is similar.

I don't quite understand how this is supposed to work. I just tried this by removing the workaround. We have every client in a separate namespace, so now the ApiException is generated in namespace of Client1, but all other clients fail to compile because
they can't find the ApiException class (even if they could, I would find it weird if all other Clients use the ApiException class of Client1). Maybe I'm misunderstanding something....

Why doesn't NSwag always generate the ApiException class in its own separate namespace (NSwag.*) and all generated clients use that one? This would work regardless of the namespaces of the client. (I assume there is a reason this doesn't work either, I'm just asking. 😄 ).

@bkoelman
Copy link
Contributor

There's no need for a workaround anymore. Below are some scenarios, hope that helps.

<Project Sdk="Microsoft.NET.Sdk">

	<PropertyGroup>
		<OutputType>Exe</OutputType>
		<TargetFramework>net8.0</TargetFramework>
		<ImplicitUsings>enable</ImplicitUsings>
		<Nullable>enable</Nullable>
	</PropertyGroup>

	<ItemGroup>
		<PackageReference Include="Microsoft.Extensions.ApiDescription.Client" Version="8.0.7" PrivateAssets="all" />
		<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
		<PackageReference Include="NSwag.ApiDescription.Client" Version="14.1.0" PrivateAssets="all" />
	</ItemGroup>

	<!-- Example 1: Generate ApiException in the first file (using a namespace is optional). -->
	<!--
	<ItemGroup>
		<OpenApiReference Include="..\openapi1.json" CodeGenerator="NSwagCSharp" Link="OpenAPIs\openapi1.json" ClassName="ApiClient1" Namespace="ExampleNamespace" />
		<OpenApiReference Include="..\openapi2.json" CodeGenerator="NSwagCSharp" Link="OpenAPIs\openapi2.json" ClassName="ApiClient2" Namespace="ExampleNamespace" />
	</ItemGroup>
	-->

	<!-- Example 2: Use your own shared ApiException class (ApiException is never generated). -->
	<!--
	<ItemGroup>
		<OpenApiReference Include="..\openapi1.json" CodeGenerator="NSwagCSharp" Link="OpenAPIs\openapi1.json" ClassName="ApiClient1" Namespace="ExampleNamespace">
			<NSwagGenerateExceptionClasses>false</NSwagGenerateExceptionClasses>
			<NSwagAdditionalNamespaceUsages>NSwagMultiClientDemo.Shared</NSwagAdditionalNamespaceUsages>
		</OpenApiReference>
		<OpenApiReference Include="..\openapi2.json" CodeGenerator="NSwagCSharp" Link="OpenAPIs\openapi2.json" ClassName="ApiClient2" Namespace="ExampleNamespace">
			<NSwagGenerateExceptionClasses>false</NSwagGenerateExceptionClasses>
			<NSwagAdditionalNamespaceUsages>NSwagMultiClientDemo.Shared</NSwagAdditionalNamespaceUsages>
		</OpenApiReference>
	</ItemGroup>
	-->

	<!-- Example 3: Every client gets its own generated ApiException class (in different namespaces). -->
	<!--
	<ItemGroup>
		<OpenApiReference Include="..\openapi1.json" CodeGenerator="NSwagCSharp" Link="OpenAPIs\openapi1.json" ClassName="ApiClient1" Namespace="ExampleNamespace1">
			<NSwagGenerateExceptionClasses>true</NSwagGenerateExceptionClasses>
		</OpenApiReference>
		<OpenApiReference Include="..\openapi2.json" CodeGenerator="NSwagCSharp" Link="OpenAPIs\openapi2.json" ClassName="ApiClient2" Namespace="ExampleNamespace2">
			<NSwagGenerateExceptionClasses>true</NSwagGenerateExceptionClasses>
		</OpenApiReference>
	</ItemGroup>
	-->

</Project>

So depending on your needs, choose one of the following:

  • If you'd like to have a single ApiException class, use example 1 or 2.
    • If all paths/schemas are uniquely named over all openapi.json files, example 1 is sufficient. Otherwise, use example 2 with your own shared copy of ApiException.
  • If you'd like every client to have its own ApiException, use example 3.

Caution

There's one gotcha to be aware of though: if you change your project file, the client generator doesn't rerun. Delete the .obj directory manually, then rebuild, to observe the effects of your changes.

Below I've attached a fully runnable sample to try these out, which I created using the Add Service Reference dialog in Visual Studio.

NSwagMultiClientDemo.zip

I would find it weird if all other Clients use the ApiException class of Client1)
Why doesn't NSwag always generate the ApiException class in its own separate namespace

This is how NSwag worked before I started looking into this. I can only guess about the motivations. What matters to me most is that it's possible to configure it as needed.

@mus65
Copy link
Author

mus65 commented Jul 18, 2024

Thanks, Example 3 works well for our case. Setting NSwagGenerateExceptionClasses globally instead of for every reference also seems to work.

I still find the default "only generate for the first reference" behaviour a little surprising, but I realize this is a non-trivial problem and this is still a much simpler workaround than before.

@krtek2k
Copy link

krtek2k commented Jul 18, 2024

thanks for this update. It works, as Example 3 suggests

also thanks for this global property settings suggestion
<PropertyGroup> <NSwagGenerateExceptionClasses>false</NSwagGenerateExceptionClasses> </PropertyGroup>
works like a charm

What happens, when you have mixed namespaces in references, some references sharing the same, some having mixed namespaces. You then have to individually manage this option for each one.

Shared namespaces must use the Example 1 or Example 2. Based on what you prefer.
Unique namespaces can use the Example 2 or Example 3.

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

3 participants