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

Support for the composite pattern. #1152

Merged
merged 7 commits into from
Jun 23, 2020
Merged

Conversation

alistairjevans
Copy link
Member

This PR implements support for the composite pattern in Autofac. Fixes #970.

cc @johneking

Demo

First off, define your service, and the associated composite:

public interface IService
{
    void DoSomething();
}

public class DemoComposite : IService
{
    private readonly IEnumerable<IService> _implementations;

    public DemoComposite(IEnumerable<IService> implementations)
    {
        _implementations = implementations;
    }

    public void DoSomething()
    {
        foreach (var item in _implementations)
        {
            item.DoSomething();
        }
    }
}

You can then specify that DemoComposite is a composite of IService:

var builder = new ContainerBuilder();
builder.RegisterType<Implementation1>().As<IService>();
builder.RegisterType<Implementation2>().As<IService>();

builder.RegisterComposite<DemoComposite, IService>();

var container = builder.Build();

// This is an instance of DemoComposite...
container.Resolve<IService>();

The composite implementation receives the set of actual registered implementations of IService.

Composites can consume services in exactly the same manner as a normal component, including accessing the concrete implementations in interesting ways:

public class DemoComposite : IService
{
    public DemoComposite(IEnumerable<Func<Meta<IService>>> implementations, ISomeOtherService anotherService)
    {
        _implementations = implementations;
    }
}

Composites can have their own Metadata (separate from the backing implementations); they function entirely like a normal registration:

var builder = new ContainerBuilder();
builder.RegisterComposite<DemoComposite, IService>()
           .WithMetadata("key", "value");

var container = builder.Build();

var resolved = container.Resolve<Meta<IService>>();
resolve.Metadata["key"] == "value"

In fact you can apply any of the normal decorating adapters like Func, Lazy, etc to the composite.

You can also register an open generic composite:

var builder = new ContainerBuilder();
builder.RegisterType<Implementation1>().As<IService<int>>();
builder.RegisterType<Implementation2>().As<IService<int>>();

builder.RegisterGenericComposite(typeof(DemoComposite<>), typeof(IService<>));

var container = builder.Build();

// This is an instance of DemoComposite<int>
container.Resolve<IService<int>>();

Delegate composites are cool too:

builder.RegisterComposite<IService>((ctx, implementations) => new DemoComposite(implementations));

Decorators

One choice I have made is that decorators are only applied to the individual concrete implementations, and not to the composite implementation.

My thinking is that it seems weird to decorate both the composite itself and its implementations, some duplication could occur that we don't want.

I did it that way round because not decorating the composite is easy, but only decorating the composite would be super tricky.

Please weigh in if this seems wrong.

Lifetime

Composites can have their own lifetime, including being marked SingleInstance. This might mean that a SingleInstance composite ignores any new registrations added in nested lifetime scopes. This sort of makes sense, but could be confusing if someone registers a new implementation in a lifetime scope, it won't be included in the composite when you then resolve that service.

I've considered raising an error if someone defines a SingleInstance composite, but figured it might be handy in certain scenarios, so didn't want to apply a blanket ban. Again, let me know if you think that we do need an error for that scenario.

How it works

The main change that powers this is that ComponentRegistration has a new property IsServiceOverride. This property indicates that:

  • The registration should always be returned for any individual calls to Resolve<IService>(), regardless of any other default, preserve-default or dynamic source registrations.
  • The registration will never be returned for any calls to Resolve<IEnumerable<IService>>() (or any of the collection enumerations).

Amusingly (or not depending on your perspective) I didn't actually end up using pipelines for this. I started out using service
middleware, but that meant you couldn't resolve Meta<ISomeServiceIHaveComposited> or any of the other wrappers, so I
had to go down a different route.

The IsServiceOverride flag is passed down through any target registrations, so Meta<IOverriddenService> has IsServiceOverride set because so does the original service.

The ServiceRegistrationInfo stores the service override when it is added as an implementation, and always returns it as the registration for the service. You can only have one IsServiceOverride registration for a service, last one wins.

The CollectionRegistrationSource excludes all IsServiceOverride registrations from it's list when resolving the set of implementations, closing the loop and letting us resolve IEnumerable without getting our composite back in the set.

Copy link
Member

@tillig tillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sweet - we've been thinking about composites for a long time. It's nice to see it living!

src/Autofac/Core/IComponentRegistration.cs Outdated Show resolved Hide resolved
src/Autofac/Core/Registration/ComponentRegistration.cs Outdated Show resolved Hide resolved
@johneking
Copy link

Amazing stuff @alistairjevans - I've been trying to keep up with all the pipeline changes and the details here but haven't quite got my head around it all. In general it looks great to me, although I do wonder if we're missing a trick by not fully taking advantage of the new service pipeline fundamentals for this specifically (are things like metadata actually that important for composite use-cases, if it means other aspects are out of scope?) Just generally it seems like there's still a lot of special-casing for decorators and composites, or legacy parts of the implementation which are a bit of confusing (e.g. a couple of different parts of the code which represent something like service-level sharing). I'm planning to keep looking at this but don't let me stop this progressing, because clearly it's a perfectly functional and useful extension. I'll just leave that out there though - could there be a more elegant way to pull all of this together, or would there necessarily be downsides?...

@alistairjevans
Copy link
Member Author

I did consider not supporting the 'special' relationship types on composites; the reason I decided that the composites needed to support meta/func/owned, etc is that if we didn't, it would mean that:

  1. There was one particular special type of service which had different behaviour than all other services, but without having a different resolve method to distinguish it at run-time (i.e. there is no ResolveComposite). This would be confusing.

  2. You could not drop in a composite without potentially changing a lot of your other code. Composites would cease to be a 'transparent' feature which simply conceals a list, and would have their own behaviour.

    Imagine you are already using IService, and resolving it via Func<IService> all over the place, then someone adds another IService implementation, that needs to also be invoked, so you register a composite to wrap the two implementations. The problem is, all your existing code will no longer work!

As for utilising the service pipeline more fully, I did spend some time a few weeks back prototyping a more 'complete' service pipelines implementation, where the registration to resolve wouldn't actually be known when the pipeline starts. It would start with just the service pipeline, then select the registration, and then invoke it. It would have made it possible to do meta+composites in the way you may have imagined.

The problem with that change though was that:

  1. It would have required the concept of Registration Sources to effectively go in the bin almost entirely, and the replacement would not look similar. That was a level of breaking change to a library people are used to that I just wasn't prepared to accept. I got squeamish enough changing one type name in the IRegistrationSource signature!

  2. Service pipelines would have got more complicated, and difficult to test, because there would have to be a 'discovery mode', that locates services so IEnumerable resolving can find all the instances to create. Multiple conditional branches would have been present in all service pipeline stages.

  3. I became convinced that the performance might actually be worse, due to the overhead of invoking a pipeline more often, and the difficulty in supporting 'get me all the registrations'.

  4. It starts off feeling very elegant, but once you try and address some of the edge cases, it starts to feel clunky, and harder to understand than the current model.

So I abandoned that approach, and bought it back closer to how it currently functions. I would sum up my emotional journey with pure pipelines as: 👨‍💻👨‍💻🤞👍❤🤔❓❓😒🤦‍♂️😢.

Copy link
Member

@tillig tillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that we're able to get rid of IsAdapterForIndividualComponent and I like the flexibility of the flags for options. There's a question about the need for HasOption right in the interface and the possibility of an extension method on RegistrationOptions. On a larger note, outside this PR we should probably define what InstancePerMatchingLifetimeScope actually means (or what we intend it to mean, exactly) so we can determine whether something is behaving inconsistently with that definition.

So, basically, HasOption question and we're good to go.

Copy link
Member

@tillig tillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦄

@tillig tillig merged commit 6043c08 into autofac:v6 Jun 23, 2020
@alistairjevans alistairjevans deleted the composites branch June 25, 2020 06:22
@tillig
Copy link
Member

tillig commented Jun 25, 2020

On IsAdapterForIndividualComponents - does this mean we can remove that property from IRegistrationSource?

@alistairjevans
Copy link
Member Author

No, that property is used to determine whether to copy registration sources to nested component registries when we create a new scope.

A rename might be required? Not sure.

@tillig
Copy link
Member

tillig commented Jun 25, 2020

Nah, if it's still used, probably should leave it. I don't feel strongly about the name and keeping it that way is one less breaking change.

@alistairjevans alistairjevans added this to the v6.0 milestone Aug 6, 2020
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

Successfully merging this pull request may close these issues.

3 participants