Set ApiParameterDescription.Type to string for Simple Types

[brunolins16]

Contributes #44677

This change updates how the ApiParameterDescription.Type is defined for MVC Simple Types (with TypeConverter or TryParse) that are not primitives. Basically, setting the Type as string

It is a similar approach that is used here

https://github.com/brunolins16/aspnetcore/blob/7518259777aa22268843401cfa0dfdfaba37eff3/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs#L291

But in addition to the simple IsPrimitive check this PR includes some additional known types that should be handled by OpenAPI spec.

• DateTime
• DateTimeOffset
• DateOnly
• TimeOnly
• decimal
• Guid
• Uri

Obs.: NSwag will not requires any change to reflect it correctly, however, Swashbuckle will need a simple change since it is using Metadata.ModelType instead of ApiParameterDescription.Type

[captainsafia]

Do we have versions of these tests in EndpointApiDescriptionProvider?

[brunolins16]

Those tests give us a similar coverage:

aspnetcore/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs

Lines 273 to 327 in c8d252d

	[Fact]
	public void AddsFromRouteParameterAsPathWithCustomClassWithTryParse()
	{
	static void AssertPathParameter(ApiDescription apiDescription)
	{
	var param = Assert.Single(apiDescription.ParameterDescriptions);
	Assert.Equal(typeof(TryParseStringRecord), param.Type);
	Assert.Equal(typeof(string), param.ModelMetadata.ModelType);
	Assert.Equal(BindingSource.Path, param.Source);
	}
	AssertPathParameter(GetApiDescription((TryParseStringRecord foo) => { }, "/{foo}"));
	}
	[Fact]
	public void AddsFromRouteParameterAsPathWithPrimitiveType()
	{
	static void AssertPathParameter(ApiDescription apiDescription)
	{
	var param = Assert.Single(apiDescription.ParameterDescriptions);
	Assert.Equal(typeof(int), param.Type);
	Assert.Equal(typeof(int), param.ModelMetadata.ModelType);
	Assert.Equal(BindingSource.Path, param.Source);
	}
	AssertPathParameter(GetApiDescription((int foo) => { }, "/{foo}"));
	}
	[Fact]
	public void AddsFromRouteParameterAsPathWithNullablePrimitiveType()
	{
	static void AssertPathParameter(ApiDescription apiDescription)
	{
	var param = Assert.Single(apiDescription.ParameterDescriptions);
	Assert.Equal(typeof(int?), param.Type);
	Assert.Equal(typeof(int?), param.ModelMetadata.ModelType);
	Assert.Equal(BindingSource.Path, param.Source);
	}
	AssertPathParameter(GetApiDescription((int? foo) => { }, "/{foo}"));
	}
	[Fact]
	public void AddsFromRouteParameterAsPathWithStructTypeWithTryParse()
	{
	static void AssertPathParameter(ApiDescription apiDescription)
	{
	var param = Assert.Single(apiDescription.ParameterDescriptions);
	Assert.Equal(typeof(TryParseStringRecordStruct), param.Type);
	Assert.Equal(typeof(string), param.ModelMetadata.ModelType);
	Assert.Equal(BindingSource.Path, param.Source);
	}
	AssertPathParameter(GetApiDescription((TryParseStringRecordStruct foo) => { }, "/{foo}"));
	}

[halter73]

It'd be nice to have regression tests for all the types we're special-casing like Guid.

[brunolins16]

Added AddsFromRouteParameterAsPathWithSpecialPrimitiveType and AddsFromRouteParameterAsPathWithNullableSpecialPrimitiveType

[davidfowl]

Should we check for IParsable?

[halter73]

Should we check for IParsable?

I talked to @brunolins16 offline about this @davidfowl. He told me that !metadata.IsComplexType is checking for IParsable effectively. That and TypeConverter because MVC will use that too.

Most of these types, both built-in and custom IParsable types, are effectively a "string" OpenAPI data type, but swashbuckle and nswag need to special-case "integer", "number" and "boolean" based on ApiParameterDescription.Type. They also special case the standardized "format" to be more specific even when the OpenAPI data type is just "string". Examples of standard formats include "date", "int32". And non-standard formats include things like "uuid".

So treating user-defined IParsable types as "string" makes sense, because tools like swashbuckle or nswag could never come up with a better OpenAPI data type or format for these types anyway (absent help from something like an IEndpiontParameterMetadataProvider or something like that perhaps), but doing it for built-in types would be erasing too much information and be too breaking. Correct me if I'm wrong about any of this @brunolins16 @captainsafia @martincostello

I have talked with @captainsafia and there is a small (or zero) benefit in add all built-in types comparing with the maintenance cost.

The reason I wanted to do all built-in types is that we can trivially scan these like I did when I created RequestDelegateFactoryTests.TryParsableArrayParameters test cases. This seems like it would require a lot less ongoing maintenance than determining what subsite built-in types may or may not correspond to standard and non-standard OpenAPI formats.

For these built-in !metadata.IsComplexType types, we've always returned the original ModelType as the ApiParameterDescription.Type before, right? If so, it's regressing behavior to change these types to string now when they never were before. My understanding of #44677 is that it's only complaining about the handling of custom TryParsable types which has changed, and not the handling of built-in TryParsable types which were typically already registered via TypeConverter and therefore didn't change in behavior.