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

Interface Ambiguous #909

Closed
RoarkDude opened this issue Apr 18, 2018 · 7 comments
Closed

Interface Ambiguous #909

RoarkDude opened this issue Apr 18, 2018 · 7 comments

Comments

@RoarkDude
Copy link

"The ambiguity, is in the box" - Monty Python.

Autofac is having a problem resolving an interface. See attached solution.

The Interface, IAmbiguous, is defined in project ACommon. It is implemented in project AInjectable. The AInjectable project does not / cannot reference ACommon. The AInjectable project defines IAmbiguous as an existing item brought in with a file link.

The UI project calls ACommon Inject and attempts to register the AInjectable assembly. IAmbiguous is not ambiguous initially but after a builder.RegisterAssemblyTypes command it becomes "ambiguous in the namespace." There is no error thrown when the container is built but the registration is not there.

Registration can be done "AsImplementedInterfaces" if Named and Keyed is not used. But then there is no way to Resolve the registration because the service IAmbiguous is "ambiguous in the namespace."
Ambiguous.zip

@tillig
Copy link
Member

tillig commented Apr 18, 2018

What you're doing by including the same interface in two different assemblies isn't something you should be doing. Note that by doing that, your AInjectable class is not implementing the interface from the ACommon project. It's implementing a different but identically named interface.

This sort of thing is a problem - having the same type (interface, class, whatever) name in two different assemblies. We even had a problem (#782) where we had a System.SerializableAttribute in Autofac as a shim for .NET Core. You really just can't do that.

You'll also see the same thing if you try to make a static extension method class that has the same namespace and name as some other static extension method class. Ambiguous references.

Without doing Reflection.Emit style code generation, you won't be able to declare an interface in one assembly ("Assembly A") and implement that interface in a different assembly ("Assembly B") without having Assembly B reference Assembly A. That's just how .NET works. What you're seeing is a manifestation of that when you use Autofac, but it's not caused by Autofac. It's caused by you doing something you shouldn't be doing in .NET.

The fix is to define your interfaces in a separate assembly that everyone implementing the interfaces can reference. (Or you can try to dynamically generate code using Reflection.Emit or Roslyn or something, but that's waaaay harder.)

@tillig tillig closed this as completed Apr 18, 2018
@tillig
Copy link
Member

tillig commented Apr 19, 2018

This appears to have been double-posted on StackOveflow. In the future, please avoid double-posting. We have a limited amount of time to spread around between support on issues, support on questions, and actually coding up bug fixes and enhancements - having to answer the same question in two places uses up that time.

@sguryev
Copy link

sguryev commented Dec 27, 2019

Hi @tillig
I'm trying to go away from 2k+ manual registrations of 400+ services we have now. There is micro-services architecture which makes adding new service a pain with finding out all required dependencies every single time.

So I have decided to use Assembly Scanning https://autofaccn.readthedocs.io/en/latest/register/scanning.html.

It works like a charm with some minor filters and several manual registrations. Here is my PR stats for this refactoring:
image
I'm really excited about these Extensions so I don't need to deal with reflection on my own.

I have one problem now. We have 4 services which have 1+ different registrations. I know about ways to let container know which registration should be selected in the particular case. But I would like Container to let me know about Ambiguous interfaces by exception. I prefer to fix issue when Container can't choose the registration rather than silently choosing latest one. I can fix all 4 cases now but I really want to prevent anybody to forget about it in the future.

Is there any easy (out-of-the-box) way to configure container to throw for Ambiguous Interfaces? Something like new ContainerBuilder().ThrowAmbiguousResolve() or some insights where I can correctly inject my own custom extension?

@tillig
Copy link
Member

tillig commented Dec 27, 2019

This is more of a compile and project problem than an Autofac problem. There's no way for us to know if the ambiguity was intentional. In fact, if you think about it, without something like Roslyn actually analyzing the code proper, all Autofac does is get compiled type references. There's no "name table" or anything. I'd look more at code and static analysis tools to solve this. Perhaps if we get to #905 this would be an interesting analyzer to add.

@sguryev
Copy link

sguryev commented Dec 27, 2019

I don't blame Autofac. And you are right: container doesn't know if it was intentional or not. And Container considers it as intentional. I just want to have a way to say that it was not intentional and developer has maid a mistake by missing the specific registration.

I was able to find all cases using this snippet:

private void ValidateContainer()
{
    var serviceMap = Container.ComponentRegistry.Registrations
                              .SelectMany(r => r.Services.Select(s => new {Registration = r, Service = s}))
                              .GroupBy(d => d.Service.Description)
                              .Where(g => g.Select(x => x.Registration.Activator.LimitType).Distinct().Count() > 1)
                              .ToDictionary(g => g.Key, g => g);
    foreach (var servicePair in serviceMap)
    {
        foreach (var assembly in ScanningModule.Assemblies)
        {
            var type = assembly.GetType($"{servicePair.Key}[]");
            if (type != null)
            {
                var registrations = Container.Resolve(type);
            }
        }
    }
}

I can't believe that there is no way during resolving Service to list all registrations.
As the plan B I can create a Unit test which will examine container and throw. But I would like to make some better solution. Could you please confirm that Roslyn is a single way to achieve it?

@alistairjevans
Copy link
Member

@sguryev, if the consumer of a service needs to specify which registration of that service it wants at resolve time, it suggests to me that you may want to use a Keyed registration, to select from the available registrations.

Alternatively, if you consider it to be a coding error for a service to be registered more than once, then a 'Validate' unit test may be a good idea.

Regardless, it doesn't feel like your query is particularly related to this particular issue (where the same interface is loaded from two different assemblies). I'd suggest you open a new issue, particularly if you want to discuss some new functionality in Autofac that could help to validate a container.

@sguryev
Copy link

sguryev commented Dec 27, 2019

@alistairjevans Thank you. Yep I'm going to use some of the existing ways to distinguish registrations at resolving. Maybe even Keyed registration. Didn't think about actual implementation yet.

Also I will add Validation test for sure. Because it's better to have failed test than failed application.

It's not quite related (I have all my interfaces and all implementations withing the same Assembly). I was not able to find better place to ask, sorry. I don't like people who don't try to search and create duplicated issues. Will do new issue. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants