Skip to content

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

@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.

@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
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?

@ericstj ericstj merged commit 3e6d25a into dotnet:main May 19, 2025
83 of 85 checks passed
@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
Copy link
Contributor

dotnet-policy-service bot commented May 20, 2025

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.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 20, 2025
@ericstj
Copy link
Member

ericstj commented Oct 3, 2025

📋 Breaking Change Documentation Required

Create a breaking change issue with AI-generated content

Generated by Breaking Change Documentation Tool - 2025-10-03 13:47:42

@ericstj ericstj assigned stephentoub and unassigned steveharter Oct 3, 2025
@stephentoub
Copy link
Member

@ericstj, it's not clear to me what the breaking change here is we want to document... just that a property which could have always returned null but was annotated to say it couldn't is now annotated to say that it can?

@ericstj
Copy link
Member

ericstj commented Oct 6, 2025

Nevermind, looks like I already documented this one in dotnet/docs#46269 but linked to the issue and not the PR.

@ericstj ericstj removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Extensions-DependencyInjection breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. new-api-needs-documentation

Projects

None yet

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