Skip to content

[release/6.0] Add nullable support to BindAsync #36128

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
dougbu merged 4 commits into from
Sep 6, 2021
Merged

[release/6.0] Add nullable support to BindAsync #36128

dougbu merged 4 commits into from
Sep 6, 2021

Conversation

@halter73
Copy link
Member

@halter73 halter73 commented Sep 3, 2021

Backport of #36114 to release/6.0

Customer Impact

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);
    }
}

Testing

Unit tests. Manual testing.

Risk

Low. This supports a new signature for BindAsync (ValueTask<Nullable<{Type}>>-returning) and doesn't effect the handling of the BindAsync signature we added support for in rc1 (ValueTask<{Type}>-returning), so it should have no impact on existing code.

@halter73 halter73 changed the title Add nullable support to BindAsync [release/6.0] Add nullable support to BindAsync Sep 3, 2021
@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 davidfowl closed this Sep 5, 2021
@davidfowl davidfowl reopened this Sep 5, 2021
@davidfowl
Copy link
Member

davidfowl commented Sep 5, 2021

cc @dotnet/aspnet-build for merge

@dougbu dougbu merged commit e3b6f3c into release/6.0 Sep 6, 2021
@dougbu dougbu deleted the halter73/35839-backport branch September 6, 2021 00:32
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 Awaiting requested review from BrennanConroy

@javiercn javiercn Awaiting requested review from javiercn

@Pilchie Pilchie Awaiting requested review from Pilchie

@pranavkm pranavkm Awaiting requested review from pranavkm

@Tratcher Tratcher Awaiting requested review from Tratcher

@captainsafia captainsafia Awaiting requested review from captainsafia

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

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.

5 participants

@halter73 @davidfowl @Pilchie @dougbu