1
\$\begingroup\$

I have a Asp.Net API (.NET Framework 4.7.2). We recently updated Autofac to version 6.2.0. Autofac does not like some of our classes that don't have a public constructor.

Example class:

public sealed class User
{   
    private static readonly Lazy<User> Lazy = new Lazy<User>(() => new User());

    public static User Instance => Lazy.Value;

    private User()
    {
    }

    public DateTime UserDate { get; set; }
    public string UserId { get; set; }
    public string UserName { get; set; }
    public string Theme { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public bool IsLoggedIn { get; set; }

    public PrismApplication LoggedInTo {get;set;}
}

I put in a work around to exclude these classes for the Autofac ContainerBuilder in Global.asax.cs:

        ...
        var builder = new ContainerBuilder();
        builder.RegisterApiControllers(Assembly.GetExecutingAssembly());

        var targetAssembly = typeof(SalesDocumentCommandHandler).GetTypeInfo().Assembly;

        var _skippedTypes = new HashSet<Type>();
        _skippedTypes.Add(typeof(User));
        _skippedTypes.Add(typeof(ReportClass));
        _skippedTypes.Add(typeof(PrismApplication));
        _skippedTypes.Add(typeof(OutboundEmailType));
        _skippedTypes.Add(typeof(OEDataset.OERow));
        _skippedTypes.Add(typeof(OEDataset.OESOPUserDefinedRow));
        _skippedTypes.Add(typeof(OEDataset.OESOPPOLinkRow));
        _skippedTypes.Add(typeof(OEDataset.OELineCommentRow));
        _skippedTypes.Add(typeof(OEDataset.OEHeaderCommentRow));
        _skippedTypes.Add(typeof(OEDataset.SOPTaxBreakupRow));
        _skippedTypes.Add(typeof(OEDataset.OEHDetailRow));
        _skippedTypes.Add(typeof(OEDataset.OEHHeaderRow));
        _skippedTypes.Add(typeof(OEDataset.OEDetailRow));
        _skippedTypes.Add(typeof(OEDataset.OEHeaderRow));

        builder.RegisterAssemblyTypes(targetAssembly).Where(x => !_skippedTypes.Contains(x))
            .As(type => type.GetInterfaces()
                .Where(interfacetype => interfacetype.IsAssignableFrom(typeof(ICommandHandler<>))
                                        || interfacetype.IsAssignableFrom(typeof(IEventHandler<>))))
            .AsSelf()
            .InstancePerDependency();

        targetAssembly = typeof(SalesDocumentCreatedEvent).GetTypeInfo().Assembly;

        builder.RegisterAssemblyTypes(targetAssembly).Where(x => !_skippedTypes.Contains(x))
            .As(type => type.GetInterfaces()
                .Where(interfacetype => interfacetype.IsAssignableFrom(typeof(ICommandHandler<>))
                                        || interfacetype.IsAssignableFrom(typeof(IEventHandler<>))))
            .AsSelf()
            .InstancePerDependency().PublicOnly();
        ...

Isn't there a better way?

\$\endgroup\$
2

2 Answers 2

2
\$\begingroup\$

Generally speaking, classes with private-only constructors are irrelevant in terms of DI. It makes no sense to include them in your container because the container can't do anything with them.

The only exception here is when you register the specific instance of a type with the container, which only really makes sense in scope of a singleton, e.g. something like:

builder.Register(c => User.Instance).As<User>().SingleInstance();

However, since you are clearly dealing with multiple of these classes, and you're trying to iteratively handle these, this custom-tailored-per-type approach seems out of scope for your intentions.

The question all boils down to "How do I find all of these types that I don't want to register?", which for your case means filling in _skippedTypes without needing to manually write down each class.

The easiest way to do so is to use a marker interface and reflection.

A marker interface is an empty interface:

public interface IUnmapped {}

Which your intended classes then implement:

public class User : IUnmapped
{
    // ...
}

And then you can use reflection to find all of the types that implement this interface:

var type = typeof(IUnmapped);
var skippedTypes = AppDomain.CurrentDomain
                            .GetAssemblies()
                            .SelectMany(s => s.GetTypes())
                            .Where(p => type.IsAssignableFrom(p));

Which in turn leads to being able to refactor the initial code:

var type = typeof(IUnmapped);

builder
    .RegisterAssemblyTypes(targetAssembly)
    .Where(x => !type.IsAssignableFrom(x))
    .As(type => 
        type.GetInterfaces()
            .Where(interfacetype => 
                   interfacetype.IsAssignableFrom(typeof(ICommandHandler<>))
                   || interfacetype.IsAssignableFrom(typeof(IEventHandler<>))))
    .AsSelf()
    .InstancePerDependency();

Some further notes:

  • When using marker interfaces, I'd be more inclined to use a positive marker, i.e. IMapped to indicate the types that you do want to add to the container. The only difference in your reflection logic is inverting the Where:
var type = typeof(IMapped);
var skippedTypes = AppDomain.CurrentDomain
                            .GetAssemblies()
                            .SelectMany(s => s.GetTypes())
                            .Where(p => !type.IsAssignableFrom(p));

Or when refactoring the old code:

var type = typeof(IMapped);

builder
    .RegisterAssemblyTypes(targetAssembly)
    .Where(x => type.IsAssignableFrom(x))
    .As(type => 
        type.GetInterfaces()
            .Where(interfacetype => 
                   interfacetype.IsAssignableFrom(typeof(ICommandHandler<>))
                   || interfacetype.IsAssignableFrom(typeof(IEventHandler<>))))
    .AsSelf()
    .InstancePerDependency();
  • You can cut out the middle man marker interface, and use reflection to look up each type's public constructors, and then pick the ones that don't have any. However, I am unsure how much of a performance hit this might entail.
var skippedTypes = AppDomain.CurrentDomain
                            .GetAssemblies()
                            .SelectMany(s => s.GetTypes())
                            .Where(p => !p.GetConstructors().Any());

I think I'd still prefer the marker interface, even though I generally dislikee marker interfaces, simply because it means you're able to also skip a type that has a public constructor when there is an ulterior reason to want to skip it.

\$\endgroup\$
4
  • \$\begingroup\$ Marker interface is not really good solution, because you are polluting your core code with implementation specific stuff. This is not how you build extensible and maintable systems \$\endgroup\$ Aug 18, 2021 at 10:22
  • \$\begingroup\$ @ViktorLova: I agree, and I'm also not a fan of marker interfaces to boot. But given the constraints set by OP, there's little other recourse other than redesigning the domain from scratch, which I don't have the necessary information for (and is IMO out of scope). Marker interfaces are an easy way to avoid having to hand-craft this list of excluded types. There are other ways to do this via reflection, but it's all just a matter of picking the indicator that you prefer (namespace, class name, interface implementation, ...) and your feedback would apply equally to all of those indicators. \$\endgroup\$
    – Flater
    Aug 18, 2021 at 10:57
  • \$\begingroup\$ @ViktorLova: To be fair, Autofac is partly to blame here. I don't like broad-sweeping registration logic (barring specific use cases) specifically because any exception to the broad rule becomes cumbersome to (manually) manage. I'm much more a fan of explicit registration (again, barring certain use cases) which, while requiring a bit more typing, gives you much more direct control over the container. \$\endgroup\$
    – Flater
    Aug 18, 2021 at 11:00
  • \$\begingroup\$ well, the problem is that author tries to register all types in assembly, including DTO. See my answer for details. \$\endgroup\$ Aug 18, 2021 at 11:30
2
\$\begingroup\$

Let's take a look on your code:

    builder.RegisterAssemblyTypes(targetAssembly).Where(x => !_skippedTypes.Contains(x))
        .As(type => type.GetInterfaces()
            .Where(interfacetype => interfacetype.IsAssignableFrom(typeof(ICommandHandler<>))
                                    || interfacetype.IsAssignableFrom(typeof(IEventHandler<>))))
        .AsSelf()
        .InstancePerDependency();

What it says:

  • register all types in assembly, except skipped types
  • and if type implements ICommandHandler<>/IEventHandler<>, then make it available as ICommandHandler<>/IEventHandler<>
  • and also register type available as themselves

Why do you need to register all types in assembly? I don't believe that you need it.

That's what's wrong with the registration of User class. It's not related to having a non-public constructor. You simply don't need to inject it! So you don't need to register it.

It's much better to do the next thing:

  1. Automate registration of repetitive things. Only for cases when the rule is suitable for every class.
  2. Register manually each type

Sample of code:

builder.RegisterAssemblyTypes(targetAssembly)
       .AsClosedTypesOf(typeof(ICommandHandler<>));

builder.RegisterAssemblyTypes(targetAssembly)
       .AsClosedTypesOf(typeof(IEventHandler<>));

builder.RegisterType<MyCustomService>();

If sugar AsClosedTypesOf is not available or not suitable, you can do next:

builder.RegisterAssemblyTypes(targetAssembly)
    .Where(x => type.GetInterfaces()
        .Any(interfaceType => interfaceType.IsAssignableFrom(typeof(ICommandHandler<>))))
    .As(type => type.GetInterfaces()
        .Where(interfaceType => interfaceType.IsAssignableFrom(typeof(ICommandHandler<>))))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.