TryAddSingleton custom AuthorizationPolicyProvider

[janek9971]

Describe the bug

Exception at startup webApi class is thrown, because of unexcpected behaviour when use TryAddSingletion custom AuthorizationPolicyProvider.

To Reproduce

Steps to reproduce the behavior:

1. Using newest version of .net core 2.2 of package Microsoft.Extensions.DependencyInjection,
   class ServiceCollectionDescriptorExtensions
2. Running

services.TryAddSingleton<IAuthorizationPolicyProvider, AdGroupsAuthorizationPolicyProvider>();

where AdGroupsAuthorizationPolicyProvider derive from IAuthorizationPolicyProvider,
so it is custom AuthorizationPolicyProvider.

3. This give me an error in Startup.cs, when calling

app.UseMvc();

System.InvalidOperationException: 'The AuthorizationPolicy named: 'AdGroupsic-asset-validator,ic-asset-editor' was not found.'

Expected behavior

I copied that code below from ServiceCollectionDescriptorExtensions, to my static class and debugged

  public static void TryAddSingleton<TService, TImplementation>(this IServiceCollection collection)
            where TService : class
            where TImplementation : class, TService
        {
            if (collection == null)
                throw new ArgumentNullException(nameof(collection));
            collection.TryAddSingleton(typeof(TService), typeof(TImplementation));
        }
        public static void TryAddSingleton(
            this IServiceCollection collection,
            Type service,
            Type implementationType)
        {
            if (collection == null)
                throw new ArgumentNullException(nameof(collection));
            if (service == (Type)null)
                throw new ArgumentNullException(nameof(service));
            if (implementationType == (Type)null)
                throw new ArgumentNullException(nameof(implementationType));
            ServiceDescriptor descriptor = ServiceDescriptor.Singleton(service, implementationType);
            collection.TryAdd(descriptor);
        }
        public static void TryAdd(this IServiceCollection collection, ServiceDescriptor descriptor)
        {
            if (collection == null)
                throw new ArgumentNullException(nameof(collection));
            if (descriptor == null)
                throw new ArgumentNullException(nameof(descriptor));
            if (collection.Any<ServiceDescriptor>((Func<ServiceDescriptor, bool>)(d => d.ServiceType == descriptor.ServiceType)))
                return;
            collection.Add(descriptor);
        }

and the reason why error is throw is beacuse, when program reach line

            if (collection.Any<ServiceDescriptor>((Func<ServiceDescriptor, bool>)(d => d.ServiceType == descriptor.ServiceType)))

it goes to return and not add the service, cause probably the IAuthorizationPolicy default is added earlier and I can't add again the same serviceType with difrrent implementation.
For me it should inform me about that, not silently just return and then I have no idea at app.UseMvc(), why exception is throwed.

Screenshots

[analogrelay]

So the request here is for a way to know when TryAdd skips adding the service? Perhaps we could return a bool indicating if the add was successful but that would be a breaking API change so it would have a high cost.

[janek9971]

I usually associate "Try..." methods that returns a boolean value, but you're right that would be a breaking change, so I don't know if it's worth the enhancement. However that was really confusing to me at the first time.

[analogrelay]

Yeah, I do agree that Try methods should generally return a boolean. We'll keep this on the backlog for now and consider the breaking change aspect.

[janek9971]

Thank you for paying attention to this.

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
Notify danmosemsft if you want to be subscribed.

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.