Skip to content

Add nullable support to BindAsync #36114

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.

Sign up for GitHub

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

Jump to bottom
Merged
davidfowl merged 1 commit into from
Sep 5, 2021
Merged

Add nullable support to BindAsync #36114

davidfowl merged 1 commit into from
Sep 5, 2021

Conversation

@halter73
Copy link
Member

@halter73 halter73 commented Sep 2, 2021

Today, if you want to support an optional parameter with BindAsync, you cannot use a nullable struct. This PR changes it so nullable structs are supported just like nullable reference types.

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

// This will now use BindAsync rather than try to deserialize the struct from the request body as JSON.
app.MapGet("/webhook", (CloudEventStruct? cloudEvent) =>
{
    redis.Publish(cloudEvent);
});

app.Run();

public struct CloudEventStruct
{
	// This will now be matched because even though it returns returns a ValueTask<CloudEventStruct?>
    // instead of ValueTask<CloudEventStruct>
    public static async ValueTask<CloudEventStruct?> BindAsync(HttpContext context, ParameterInfo parameter)
    {
        return await CloudEventParser.ReadEventAsync(context, parameter.Name);
    }
}

Fixes #35839

@halter73
Copy link
Member Author

halter73 commented Sep 2, 2021

This will be backported to release/6.0

@halter73 halter73 changed the title @halter73 Add nullable support to BindAsync Add nullable support to BindAsync Sep 2, 2021
BrennanConroy
BrennanConroy approved these changes Sep 2, 2021
View reviewed changes
Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That rename 😮‍💨

@halter73 halter73 enabled auto-merge (squash) September 3, 2021 01:03
@halter73 halter73 disabled auto-merge September 3, 2021 01:03
@halter73
Copy link
Member Author

halter73 commented Sep 3, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1196316151

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

@halter73 backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: TryParseMethodCache -> ParameterBindingMethodCache
Using index info to reconstruct a base tree...
M	src/Http/Http.Extensions/src/RequestDelegateFactory.cs
M	src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
CONFLICT (content): Merge conflict in src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Auto-merging src/Http/Http.Extensions/src/RequestDelegateFactory.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 TryParseMethodCache -> ParameterBindingMethodCache
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@halter73 halter73 mentioned this pull request Sep 3, 2021
Merged
@Pilchie Pilchie 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 Sep 3, 2021
@davidfowl
Copy link
Member

davidfowl commented Sep 5, 2021

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Sep 5, 2021

Pull request contains merge conflicts.

@halter73 @davidfowl
Add nullable support to BindAsync
1b3acf3 
- Renamed TryParseMethodCache -> ParameterBindingMethodCache
- Add RequestDelegateHandlesBindAsyncOptionality test
@davidfowl
Copy link
Member

davidfowl commented Sep 5, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2021

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1203220257

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2021

@davidfowl backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add nullable support to BindAsync - Renamed TryParseMethodCache -> ParameterBindingMethodCache - Add RequestDelegateHandlesBindAsyncOptionality test
Using index info to reconstruct a base tree...
M	src/Http/Http.Extensions/src/RequestDelegateFactory.cs
M	src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
CONFLICT (content): Merge conflict in src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Auto-merging src/Http/Http.Extensions/src/RequestDelegateFactory.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add nullable support to BindAsync - Renamed TryParseMethodCache -> ParameterBindingMethodCache - Add RequestDelegateHandlesBindAsyncOptionality test
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@davidfowl davidfowl enabled auto-merge (squash) September 5, 2021 14:04
@davidfowl davidfowl merged commit dd3c972 into main Sep 5, 2021
@davidfowl davidfowl deleted the halter73/35839 branch September 5, 2021 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidfowl davidfowl davidfowl approved these changes

@BrennanConroy BrennanConroy BrennanConroy approved these changes

@pranavkm pranavkm Awaiting requested review from pranavkm

@captainsafia captainsafia Awaiting requested review from captainsafia captainsafia is a code owner

@rafikiassumani-msft rafikiassumani-msft Awaiting requested review from rafikiassumani-msft

@javiercn javiercn Awaiting requested review from javiercn

@Pilchie Pilchie Awaiting requested review from Pilchie

@Tratcher Tratcher Awaiting requested review from Tratcher

Assignees

No one assigned

Labels

old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add nullable struct support to BindAsync

5 participants

@halter73 @davidfowl @BrennanConroy @Pilchie