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

Detect services based on service provider #32737

Merged
merged 5 commits into from
Jun 13, 2021
Merged

Conversation

davidfowl
Copy link
Member

@davidfowl davidfowl commented May 15, 2021

  • Added an IServiceProvider argument to RequestDelegateFactory.Create
  • As a final fallback, try to detect services from the DI container before falling back to body behavior.

There are some gotchas:

  • We can't do this at compile time, it's a runtime only feature (for the future). Not a big deal, we know how to handle this for AOT scenarios in the future.
  • There are side effects. Your object will get created once to "test" if it's indeed a service. We could build an optional capability into the DI container for this to avoid the activation.
  • As part of instantiating the object, we need to dispose it. If this object has a scoped lifetime and is IAsyncDisposable only, this will currently throw an exception.
  • Any ambiguity can be avoided by using the [FromServices] attribute and by being explicit

It cleans up this scenario nicely:

app.MapGet("/todos", async (TodoDbContext db) =>
{
    return await db.Todos.ToListAsync();
});

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

This PR is now blocked on dotnet/runtime#53919

@davidfowl davidfowl added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label May 15, 2021
@davidfowl davidfowl requested a review from halter73 May 15, 2021 16:32
@ghost
Copy link

ghost commented May 15, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@ghost ghost added the area-runtime label May 15, 2021
@pranavkm pranavkm added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels May 17, 2021
@davidfowl davidfowl added * NO MERGE * Do not merge this PR as long as this label is present. blocked The work on this issue is blocked due to some dependency labels Jun 10, 2021
@davidfowl davidfowl force-pushed the davidfowl/rdf-detect-services branch from 01d048c to 432b02a Compare June 12, 2021 14:11
@davidfowl davidfowl removed * NO MERGE * Do not merge this PR as long as this label is present. blocked The work on this issue is blocked due to some dependency labels Jun 12, 2021
@davidfowl davidfowl merged commit d2ab01b into main Jun 13, 2021
@davidfowl davidfowl deleted the davidfowl/rdf-detect-services branch June 13, 2021 01:16
@ghost ghost added this to the 6.0-preview6 milestone Jun 13, 2021
@davidfowl
Copy link
Member Author

@halter73 feel free to review this on Monday. I'm merging all of the PRs I've had open to get them in the end to end build for further testing.

@amcasey amcasey added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Aug 24, 2023
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 area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants