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

DateOnly custom TypeConverter is ignored when using FromQuery #46384

Closed
1 task done
killergege opened this issue Feb 1, 2023 · 3 comments · Fixed by #46577
Closed
1 task done

DateOnly custom TypeConverter is ignored when using FromQuery #46384

killergege opened this issue Feb 1, 2023 · 3 comments · Fixed by #46577
Assignees
Labels
bug This issue describes a behavior which is not expected - a bug. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Milestone

Comments

@killergege
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Since .NET 7 and the new support for DateOnly, our custom TypeConverter for DateOnly is not invoked anymore when the controller parameter is an object initialized with [FromQuery] on a GET request

Our client sends the date with time at zero (YYYY-MM-DDT00:00:00Z), so our converter is taking the date part and ignoring the time, which worked well in .NET 6.
In .NET 7, with exactly the same code, we have a validation error The value '2023-01-31T00:00:00Z' is not valid. because the new converter is taking over, and our custom is not invoked.

I'm not sure whether this is a bug or us not using it properly, but it worked previously...
A workaround that don't imply changing the contract is welcome.

Expected Behavior

No behavior change.
The default TypeConverter is ignored, and our custom one is invoked.

Steps To Reproduce

The demo are the default .NET Web API template (weather forecast), which I added a simplified version of our custom TypeConverter, a global registration of it, and an object parameter to the forecast endpoint, which contains a DateOnly.

WebApplication1 is .NET 7 => with 2023-01-31T00:00:00Z as the date parameter => TypeConverter is not invoked, validation error
WebApplication2 is .NET 6 => with 2023-01-31T00:00:00Z => TypeConverter is invoked, works

https://github.com/killergege/net_typeconverter_demo

Exceptions (if any)

No response

.NET Version

7.0.102

Anything else?

Visual Studio 2022

.NET SDK:
 Version:   7.0.102
 Commit:    4bbdd14480

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19044
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.102\

Host:
  Version:      7.0.2
  Architecture: x64
  Commit:       d037e070eb

.NET SDKs installed:
  2.2.207 [C:\Program Files\dotnet\sdk]
  3.1.425 [C:\Program Files\dotnet\sdk]
  6.0.404 [C:\Program Files\dotnet\sdk]
  7.0.101 [C:\Program Files\dotnet\sdk]
  7.0.102 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.31 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.31 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.31 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
@brunolins16 brunolins16 added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Feb 1, 2023
@brunolins16
Copy link
Member

brunolins16 commented Feb 1, 2023

@killergege Thanks for reporting this issue.

In .NET 7 was introduced the support for Try/Parse and we decided to have it as priority over the SimpleModelBinder that uses the TypeConverter. If the Try/Parse is not an important feature for you, a simple workaround is to remove the TryParseModelBinderProvider

builder.Services.AddControllers(o => o.ModelBinderProviders.RemoveType<TryParseModelBinderProvider>());

@brunolins16
Copy link
Member

I believe we should investigate if our decision is more impactful than we thought and maybe change the TryParseModelBinder priority. By change the priority, probably we will need to change how the SimpleModelBinder works today.

cc @halter73

@ghost
Copy link

ghost commented Feb 2, 2023

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document.
We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact work.

@brunolins16 brunolins16 added bug This issue describes a behavior which is not expected - a bug. and removed investigate labels Feb 7, 2023
@brunolins16 brunolins16 moved this to Committed in [.NET 8] Web Frameworks Feb 8, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 2023
@mkArtakMSFT mkArtakMSFT modified the milestones: .NET 8 Planning, 7.0.5 Nov 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue describes a behavior which is not expected - a bug. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Projects
Status: Done
Status: Committed
Development

Successfully merging a pull request may close this issue.

5 participants