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

Should we consider sealed the newly introduced attributes for keyed service #90321

Closed
WeihanLi opened this issue Aug 10, 2023 · 4 comments
Closed

Comments

@WeihanLi
Copy link
Contributor

WeihanLi commented Aug 10, 2023

We introduced ServiceKeyAttribute and FromKeyedServicesAttribute for keyed service, while they are not sealed, and there're no inherits from the repo search result, since they're new types introduced, should we consider making them sealed?

From the sealed proposal #49944, we may prefer sealed for these types

keyed service PR: #87183

code search result:
https://github.com/search?q=repo%3Adotnet%2Fruntime%20ServiceKeyAttribute&type=code
https://github.com/search?q=repo%3Adotnet%2Fruntime%20FromKeyedServicesAttribute&type=code

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 10, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 10, 2023
@ghost
Copy link

ghost commented Aug 10, 2023

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

We introduced ServiceKeyAttribute and FromKeyedServicesAttribute for keyed service, while they are not sealed, and there're no inherits from the repo search result, since they're new types introduced, should we consider making them sealed?

From the sealed proposal #49944, we may need to prefer sealed for these types

keyed service PR: #87183

code search result:
https://github.com/search?q=repo%3Adotnet%2Fruntime%20ServiceKeyAttribute&type=code
https://github.com/search?q=repo%3Adotnet%2Fruntime%20FromKeyedServicesAttribute&type=code

Author: WeihanLi
Assignees: -
Labels:

untriaged, area-Extensions-DependencyInjection, needs-area-label

Milestone: -

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 10, 2023
@ericstj
Copy link
Member

ericstj commented Aug 10, 2023

I checked the transcript from the API review and found @benjaminpetit mentioned this was intentional:
https://www.youtube.com/live/PwB593iwQlk?feature=share&t=3454

@WeihanLi
Copy link
Contributor Author

@ericstj thank you very much

@WeihanLi WeihanLi closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 10, 2023
@benjaminpetit
Copy link
Member

By allowing inheritance on FromKeyedServicesAttribute, we allow people to use complex object as the key.

Here is an example:

public class CustomKey
{
    public CustomKey(int n, string str)
    {
        N = n;
        Str = str;
    }

    public int N { get; }
    public string Str { get; }
}

public sealed class FromServicesWithCustomKey : FromKeyedServicesAttribute
{
    public FromServicesWithCustomKey(int n, string str)
        : base(new CustomKey(n, str))
    {
    }
}

public class CustomService
{
    private readonly CustomKey _key;

    public CustomService([ServiceKey] CustomKey key)
    {
        _key = key;
    }

    public int N => _key.N;
    public string Str => _key.Str;
}

[Fact]
public void InheritanceTest()
{
    var serviceCollection = new ServiceCollection();
    serviceCollection.AddKeyedTransient<CustomService>(KeyedService.AnyKey);

    var provider = CreateServiceProvider(serviceCollection);

    var service = provider.GetKeyedService<CustomService>(new CustomKey(12, "hello"));

    Assert.Equal(12, service.N);
    Assert.Equal("hello", service.Str);
}

@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants