Skip to content

Add Keyed DI support to SignalR #49729

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 8 commits into from
Aug 3, 2023
Merged

Add Keyed DI support to SignalR #49729

merged 8 commits into from
Aug 3, 2023

Conversation

BrennanConroy
Copy link
Member

Closes #49635

dotnet-maestro bot and others added 6 commits July 28, 2023 12:01
@dotnet-maestro
Update dependencies from https://github.com/dotnet/runtime build 2023…
ed5e135 
…0727.8

Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting
 From Version 8.0.0-rc.1.23376.11 -> To Version 8.0.0-rc.1.23377.8
@dotnet-maestro
Update dependencies from https://github.com/dotnet/efcore build 20230…
88a5418 
…728.1

dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.Design , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.Tools
 From Version 8.0.0-rc.1.23376.2 -> To Version 8.0.0-rc.1.23378.1
@dotnet-maestro
Update dependencies from https://github.com/dotnet/runtime build 2023…
526488b 
…0728.11

Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting
 From Version 8.0.0-rc.1.23376.11 -> To Version 8.0.0-rc.1.23378.11
@BrennanConroy
Add Keyed DI support to SignalR
184e131
@BrennanConroy
more test
1c8cc55
@BrennanConroy
destruct
afdb3bc
@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Jul 30, 2023
@BrennanConroy BrennanConroy requested a review from halter73 as a code owner July 30, 2023 00:21
mitchdenny
mitchdenny reviewed Aug 1, 2023
View reviewed changes
src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs
return 11;
}

public int MultipleKeyedServices([FromKeyedServices("service1")] Service1 service, [FromKeyedServices("service2")] Service1 service2)
Copy link
Member

Choose a reason for hiding this comment

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

Test case for two services with the same key?

mitchdenny
mitchdenny approved these changes Aug 1, 2023
View reviewed changes
Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Suggested one extra test case (definately an edge case). But this change seems reasonable to me.

Base automatically changed from darc-main-42aacfbe-ed21-4d49-84b9-a3a4395f168f to main August 2, 2023 23:22
@dotnet-maestro dotnet-maestro bot requested review from wtgodbe and a team as code owners August 2, 2023 23:22
@BrennanConroy BrennanConroy enabled auto-merge (squash) August 2, 2023 23:29
@BrennanConroy BrennanConroy disabled auto-merge August 2, 2023 23:44
@BrennanConroy BrennanConroy enabled auto-merge (squash) August 3, 2023 03:06
@BrennanConroy BrennanConroy merged commit 524b35f into main Aug 3, 2023
@BrennanConroy BrennanConroy deleted the brecon/keyedDI branch August 3, 2023 04:25
@ghost ghost added this to the 8.0-rc1 milestone Aug 3, 2023
@adityamandaleeka adityamandaleeka added the blog-candidate Consider mentioning this in the release blog post label Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

@BrennanConroy, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

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

@mitchdenny mitchdenny mitchdenny approved these changes

@benjaminpetit benjaminpetit benjaminpetit approved these changes

@davidfowl davidfowl Awaiting requested review from davidfowl

@captainsafia captainsafia Awaiting requested review from captainsafia

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@wtgodbe wtgodbe Awaiting requested review from wtgodbe

Assignees
No one assigned
Labels
area-signalr Includes: SignalR clients and servers blog-candidate Consider mentioning this in the release blog post
Projects
None yet
Milestone
8.0-rc1
Development

Successfully merging this pull request may close these issues.

Support Keyed Services in SignalR
4 participants
@BrennanConroy @mitchdenny @benjaminpetit @adityamandaleeka