Skip to content

FromServiceKeyAttribute to support nullable (for unkeyed) and to use current service key #114929

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
merged 4 commits into from
May 19, 2025
Merged

FromServiceKeyAttribute to support nullable (for unkeyed) and to use current service key #114929

merged 4 commits into from
May 19, 2025

Conversation

steveharter
Copy link
Contributor

Fixes #113585

(draft to verify tests)

This is API approved except for this which is being done via email:

public static object FromServiceKey { get; }

@steveharter steveharter added this to the 10.0.0 milestone Apr 22, 2025
@steveharter steveharter self-assigned this Apr 22, 2025
@ghost
Copy link

ghost commented Apr 22, 2025

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

1 similar comment
@ghost
Copy link

ghost commented Apr 22, 2025

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

This was referenced Apr 23, 2025
Open
Open
@ericstj ericstj marked this pull request as ready for review May 15, 2025 22:48
@Copilot Copilot AI review requested due to automatic review settings May 15, 2025 22:48
Copilot
Copilot AI reviewed May 15, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the handling of keyed services to support nullable (unkeyed) services and to use the current service key by distinguishing between lookup modes.

  • Refactored CallSiteFactory logic to separate InheritKey and ExplicitKey scenarios.
  • Updated FromKeyedServicesAttribute to set LookupMode based on whether the provided key is null.
  • Added tests for resolving both keyed and unkeyed services.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs Updated keyed resolution logic to handle InheritKey and ExplicitKey modes.
src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/KeyedDependencyInjectionSpecificationTests.cs Added tests for resolving services under various key scenarios, including unkeyed services.
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceKeyLookupMode.cs Introduced ServiceKeyLookupMode with three possible modes.
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/FromKeyedServicesAttribute.cs Modified attribute constructor to set LookupMode appropriately.
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/ref/Microsoft.Extensions.DependencyInjection.Abstractions.cs Updated public API reference to reflect the changes in the attribute constructors and enum.

@ericstj
Copy link
Member

ericstj commented May 15, 2025

I applied the API review feedback from #113585 (comment). @stephentoub @halter73 can you please have a review?

This was referenced May 16, 2025
Open
Open
Open
Open
@build-analysis build-analysis bot mentioned this pull request May 16, 2025
Closed
@ericstj ericstj merged commit 3e6d25a into dotnet:main May 19, 2025
83 of 85 checks passed
@akoeplinger akoeplinger mentioned this pull request May 20, 2025
Merged
@ericstj ericstj added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label May 20, 2025
@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label May 20, 2025
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

dotnet-policy-service bot commented May 20, 2025 •
edited by ericstj
Loading

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@ericstj ericstj mentioned this pull request May 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericstj ericstj ericstj left review comments

Copilot code review Copilot Copilot left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees

@steveharter steveharter

Labels
area-Extensions-DependencyInjection breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet new-api-needs-documentation
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.

API Proposal: FromServiceKeyAttribute(object) to support nullable (for unkeyed) and to use current service key
3 participants
@steveharter @ericstj @stephentoub