YairHalberstadt / stronginject

compile time dependency injection for .NET
MIT License
847 stars 24 forks source link

Optionally generated service location API #139

Open jnm2 opened 3 years ago

jnm2 commented 3 years ago

I have three real-world situations where an app can't avoid service location, yet 98% of each app has nothing to do with service location and would benefit from StrongInject's compile-time checking:

  1. An app that tightly uses Prism for navigation, and Prism itself integrates with the DI system but also heavily uses service location, requiring you to implement object Resolve(Type) around whatever container you're using. Replacing Prism is an expensive endeavor and it's not clear that this would avoid the need for service location.

  2. An app that instantiates a data-generating class given a serialized type name. It needs to be able to list the concrete types of available classes without instantiating them, then instantiate on demand when given the type name.

  3. An app that has 40 structured menu items, where invoking a menu item must instantiate the corresponding view model, but none of these view models can be instantiated at the point where the menu item UI is built which shows them all.

All of these work with StrongInject today by building a service location bridge type that I'm calling a type catalog.

Current solution

Here's what the type catalog looks like for situation 2, with made up data source names. This looks small and easily manageable, but keep in mind that the same technique is required for situations 1 and 3 and those cases have dozens and hundreds of types to list in the catalog:

// Any time a new type is registered with StrongInject, it must be added as a constructor parameter.
// Types can be skipped if they will never be resolved by a service locator.
public sealed class DataSourceCatalog : CompileTimeTypeCatalog
{
    public DataSourceCatalog(
        IFactory<EntryDataSource> entry,
        IFactory<LakeDataSource> lake,
        IFactory<EngineeringDataSource> engineering,
        IFactory<VolumeDataSource> volume,
        IFactory<FlightDataSource> flight)
        : base(entry, lake, engineering, volume, flight)
    {
    }

    // Inherited from base class:
    // public IEnumerable<Type> RegisteredTypes { get; }
    // public IFactory<object> GetFactory(Type type)
}

Here is how the type catalog is used:

public sealed class SomeReportingService
{
    private readonly DataSourceCatalog dataSourceCatalog;

    public SomeReportingService(DataSourceCatalog dataSourceCatalog, ...)
    {
        this.dataSourceCatalog = dataSourceCatalog;
    }

    public GeneratedReport Generate(Models.ReportDefinition definition, ...)
    {
        IFactory<object> factory = dataSourceCatalog.GetFactory(definition.RequiredInstanceType);
        object instance = factory.Resolve();
        try
        {
            // Invoke async method on instance, passing parameters, etc to obtain data for the report
        }
        finally
        {
            factory.Release(instance);
        }
    }
}

This is how the type catalog is implemented (abstract base class):

Click to expand ```cs using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; public abstract class CompileTimeTypeCatalog { private readonly ImmutableDictionary> registrations; protected CompileTimeTypeCatalog(params object[] factoriesForAllRegisteredTypes) { var builder = ImmutableDictionary.CreateBuilder>(); foreach (var factory in factoriesForAllRegisteredTypes) { var factoryInterfaces = factory.GetType().GetInterfaces().Where(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IFactory<>)); var hasFactoryInterface = false; foreach (var factoryInterface in factoryInterfaces) { var producedType = factoryInterface.GetGenericArguments().Single(); if (builder.ContainsKey(producedType)) throw new ArgumentException($"More than one factory was specified which creates the type {producedType.FullName}.", nameof(factoriesForAllRegisteredTypes)); builder.Add(producedType, (IFactory)Activator.CreateInstance(typeof(Factory<>).MakeGenericType(producedType), factory)!); hasFactoryInterface = true; } if (!hasFactoryInterface) throw new ArgumentException($"{factory.GetType().FullName} does not implement {typeof(IFactory<>).FullName}", nameof(factoriesForAllRegisteredTypes)); } registrations = builder.ToImmutable(); } public IFactory GetFactory(Type type) { if (!registrations.TryGetValue(type, out var factory)) throw new InvalidOperationException($"IFactory<{type.FullName}> must be added as a constructor parameter to {GetType().FullName} and passed to the base constructor."); return factory; } public IEnumerable RegisteredTypes => registrations.Keys; private sealed class Factory : IFactory { private readonly IFactory factory; public Factory(IFactory factory) => this.factory = factory; public object Resolve() => factory.Resolve()!; public void Release(object instance) => factory.Release((T)instance); } } ```

Motivation

All this is working today, so why am I asking for something more?

The pain point is the type catalog. It's not very maintainable. In situation 1 listed at the top, this is hundreds of constructor parameters. The most sensible naming convention is _1, _2, etc, and the most sensible way to maintain this file is to generate it. Any time a registration in StrongInject is touched, this type catalog should be updated to be on the safe side, but there is no reminder to do so other than writing tests involving the container that help with some things and failing at runtime for the rest.

StrongInject is already generating a container that could implement an API that is logically equivalent to what the type catalog does, but without the need for an additional class with a constructor with hundreds of parameters, and it could do so with compile-time safety that is maximized given the circumstances by ensuring that you don't have a type catalog that is missing a type that is registered in StrongInject.

Proposal

I think this should be opt-in so that unused code is not generated. Something to the effect of [ServiceLocator] on the container perhaps. When this option is turned on, the following instance members appear in the container:

public bool TryResolve(Type registeredType, out Owned<object> locatedInstance);
public IEnumerable<Type> RegisteredTypes { get; }

These two members will allow someone to implement https://prismlibrary.com/docs/dependency-injection/add-custom-container.html around it. (RegisteredTypes is required to implement IsRegistered, and Register methods can become assertions against RegisteredTypes as well.)

Alternatives

  • TryResolve could be replaced with Owned<object> Resolve(Type) by throwing an exception. This doesn't seem advantageous to me because I want a custom exception message anyway if I want an assertion that this service-located type should have been registered. Also, forcing the Try pattern on the caller keeps it in their view that there are no guarantees (or even expectations) that it will succeed.

  • RegisteredTypes could be replaced with bool CanResolve(Type). This would remove the ability to let the container registrations drive which data sources/menu items are listed in the app, though. The app would have to have a different way to discover types and would then have to call CanResolve to see which are registered.

  • CanResolve and TryResolve could be combined into a single bool FindFactory(Type registeredType, out IFactory<object> factory). This might be a preferable API even if the RegisteredTypes is still generated alongside it.

  • RegisteredTypes could be a more specific interface or concrete type, like ImmutableHashSet<Type> but then it would not be able to be implemented by returning ImmutableDictionary<Type, ...>.Keys. I would expect that RegisteredTypes.Contains should perform well if the latter is done, in spite of the property type being IEnumerable<Type>, but I haven't tested this.

jnm2 commented 3 years ago

Great conversation at https://gitter.im/stronginject/community?at=610bfcc9933ee46b753fcec4 bringing up a number of concerns and ideas, but none of the ideas felt right yet.

jnm2 commented 3 years ago

Also, the code examples above assumed that StrongInject provides automatic implementations for IFactory<T> which it does not, so the "today's solution" above will not work with StrongInject until https://github.com/YairHalberstadt/stronginject/issues/140 ships and IFactory<T> is replaced with Func<Owned<T>> in the examples above.