Skip to content

Introduce the Results<TResult1, TResult2, TResultN> union types #40986

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

Merged
merged 19 commits into from
Apr 7, 2022

Conversation

DamianEdwards
Copy link
Member

This PR introduces just the Results<TResult1, TResult2, TResultN> union types themselves. The types are code generated via a tool, along with their unit tests.

When #40646 lands we can update the in-box IResult implementations to describe themselves via emitted metdata, and update the Results<TResult1, TResult2, TResultN> union types to pass through that metadata from the TResultN type args, along with introducing the new Results.Typed factory methods for creating results and preserving the concrete types.

Until then, the union types allow all the possible IResult return types of a route handler delegate to be declared explicitly and thus checked at compile for type safety, e.g.:

app.MapGet("/todos/{id}", async Task<Results<OkObjectHttpResult, NotFoundObjectHttpResult>> (int id, TodoDb db) =>
{
    return await db.Todos.FindAsync(id)
        is Todo todo
            ? (OkObjectHttpResult)Results.Ok(todo)
            : (NotFoundObjectHttpResult)Results.NotFound();
});

Fixes #40672

- Add new API to PublicAPIUnshipped.txt
- Suppress API baseline in code gen tool
- Add code gen tool to the HttpAbstractsions.slnf file
- Added README.md explaining ResultsOfT code generator tool
- Added comment to top of ResultsOfT.cs pointing to code generator tool
- Updated Http\README.md to include Http.Results
@brunolins16
Copy link
Member

Is not possible (or make sense) use a T4 template instead of introduce a new project to generate the code?

@DamianEdwards
Copy link
Member Author

Is not possible (or make sense) use a T4 template instead of introduce a new project to generate the code?

I think to date the repo has used console apps for code generation in numerous places (Kestrel, Developer Exception Page, etc.) so I followed that pattern. I'm not super fussed other than it would mean more work to convert what I've done in this PR to use T4 instead, but there'll still be a mix in the repo.

@DamianEdwards DamianEdwards added the api-approved API was approved in API review, it can be implemented label Apr 6, 2022
@rafikiassumani-msft rafikiassumani-msft added old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-runtime labels Apr 6, 2022

```
dotnet run
```
Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a test that fails if the generator and generated code get out of sync like Kestrel's GeneratedCodeIsUpToDate test?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Comment on lines 107 to 116
writer.WriteIndentedLine("public async Task ExecuteAsync(HttpContext httpContext)");
writer.WriteIndentedLine("{");
writer.WriteIndentedLine(2, "ArgumentNullException.ThrowIfNull(httpContext, nameof(httpContext));");
writer.WriteLine();
writer.WriteIndentedLine(2, "if (Result is null)");
writer.WriteIndentedLine(2, "{");
writer.WriteIndentedLine(3, "throw new InvalidOperationException(\"The IResult assigned to the Result property must not be null.\");");
writer.WriteIndentedLine(2, "}");
writer.WriteLine();
writer.WriteIndentedLine(2, "await Result.ExecuteAsync(httpContext);");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
writer.WriteIndentedLine("public async Task ExecuteAsync(HttpContext httpContext)");
writer.WriteIndentedLine("{");
writer.WriteIndentedLine(2, "ArgumentNullException.ThrowIfNull(httpContext, nameof(httpContext));");
writer.WriteLine();
writer.WriteIndentedLine(2, "if (Result is null)");
writer.WriteIndentedLine(2, "{");
writer.WriteIndentedLine(3, "throw new InvalidOperationException(\"The IResult assigned to the Result property must not be null.\");");
writer.WriteIndentedLine(2, "}");
writer.WriteLine();
writer.WriteIndentedLine(2, "await Result.ExecuteAsync(httpContext);");
writer.WriteIndentedLine("public Task ExecuteAsync(HttpContext httpContext)");
writer.WriteIndentedLine("{");
writer.WriteIndentedLine(2, "ArgumentNullException.ThrowIfNull(httpContext, nameof(httpContext));");
writer.WriteLine();
writer.WriteIndentedLine(2, "if (Result is null)");
writer.WriteIndentedLine(2, "{");
writer.WriteIndentedLine(3, "throw new InvalidOperationException(\"The IResult assigned to the Result property must not be null.\");");
writer.WriteIndentedLine(2, "}");
writer.WriteLine();
writer.WriteIndentedLine(2, "return Result.ExecuteAsync(httpContext);");

It's a little bit more efficient if we skip the tail await at the small cost of losing the stack frame if there are any exceptions.

@DamianEdwards DamianEdwards requested a review from halter73 April 7, 2022 00:45
@DamianEdwards DamianEdwards enabled auto-merge (squash) April 7, 2022 20:56
@DamianEdwards DamianEdwards merged commit 1bc4ba6 into main Apr 7, 2022
@DamianEdwards DamianEdwards deleted the damianedwards/results-union-type branch April 7, 2022 23:01
@ghost ghost added this to the 7.0-preview4 milestone Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented 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
Development

Successfully merging this pull request may close these issues.

Introduce way for route handler delegates (Minimal APIs) to return union results (i.e. multiple IResult types)
6 participants