AzureAD/microsoft-identity-abstractions-for-dotnet

[Bug] Improve the serializability of DownstreamApiOptions

jmprieur opened this issue · 2 comments

Which version of Microsoft Identity Abstractions for dotnet are you using?
4.1.0

Is this a new or an existing app?
This is an app that I'm trying to compile with AoT

Repro

<Project Sdk="Microsoft.NET.Sdk">
	<PropertyGroup>
		<OutputType>Exe</OutputType>
		<TargetFramework>net8.0</TargetFramework>
		<ImplicitUsings>enable</ImplicitUsings>
		<Nullable>enable</Nullable>
		<PublishAot>true</PublishAot>
		<InvariantGlobalization>true</InvariantGlobalization>
		<EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
		<LangVersion>latest</LangVersion>
		<Features>InterceptorsPreview</Features>
		<AdditionalCompilerArguments>-outputgeneratedcode:GeneratedCode</AdditionalCompilerArguments>
	</PropertyGroup>

	<ItemGroup>
	  <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-rc.1.23419.4" />
	  <PackageReference Include="Microsoft.Identity.Abstractions" Version="4.1.0" />
	</ItemGroup>

</Project>

And the code:

#pragma warning disable SYSLIB1100 // Did not generate binding logic for a type
#pragma warning disable SYSLIB1101 // Did not generate binding logic for a property on a type
            builder.Configuration.GetSection("DownstreamApi").Bind(downstreamApiOptions);
#pragma warning restore SYSLIB1101 // Did not generate binding logic for a property on a type
#pragma warning restore SYSLIB1100 // Did not generate binding logic for a type
            downstreamApiOptions.HttpMethod = HttpMethod.Parse(builder.Configuration.GetSection("DownstreamApi")["HttpMethod"]);

Expected behavior
No issues. The code generator can produce the binding on .NET 8 (we won't even try on lower versions of .NET, as we don't want to force a new dependency for these)

Actual behavior
The following properties issue warnings

  • Method (System.Net.HttpMethod), is not bound/de-serialized.
  • Serializer, Deserializer, CustomizeHttpRequestMessage should not be serializable (they are code-only configuration). Add a JsonIgnore attribute.

Possible solution

  • Direct the serialization better with attribute and a type converter

The warnings for Serializer, Deserializer, CustomizeHttpRequestMessage are actually not harmful, and it's not obvious how to remove them (JSonIgnore is not taken into account).

For the HttpMethod, would a TypeConverter be enough? Bind does not use JSon serialization.

closing, as #98 will cover it.