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

Add an IfNotExists registration extension #469

Closed
alexmg opened this issue Jan 22, 2014 · 14 comments
Closed

Add an IfNotExists registration extension #469

alexmg opened this issue Jan 22, 2014 · 14 comments
Milestone

Comments

@alexmg
Copy link
Member

alexmg commented Jan 22, 2014

From alex.meyergleaves on November 12, 2013 22:37:43

This was proposed by Jacek as shown in his clone. https://code.google.com/r/jszumigaj-autofac/ The idea is to prevent duplicate registration from being made, different to PreserveExistingDefaults which still adds the registration. e.g.

[Test]
public void RegisteredTwoTimesWith_IfNotRegistered_Extension()
{
  var builder = new ContainerBuilder();
  builder.RegisterType<A1>();
  builder.RegisterType<A1>().IfNotExists(); //registers only if A1 is not registered
  var container = builder.Build();

  var instances = container.Resolve<IEnumerable<A1>>(); //without my extensions we'll get 2 instances here

  Assert.That(instances.Count(), Is.EqualTo(1)); // but we get only one!
}

The changes look good but do break the existing API, and such I suggest we leave this for the 4.0 milestone. There is already work that needs to be done for PreserveExistingDefaults in regards to its behaviour in nested lifetime scopes: #272 Perhaps we should consider introducing a single Enum to describe the different override behaviours?

Original issue: http://code.google.com/p/autofac/issues/detail?id=469

@tillig
Copy link
Member

tillig commented May 12, 2016

This has been on the backlog for a while and it appears we've lost the clone.

If we were to do this, I am curious how it would be intended to work.

For example...

  • Is it based on the component being registered or the service(s) being exposed?
  • Does service name play into it? Like if someone registers the component as a named service does the extension only see the thing as having been registered earlier if the name exists?
  • How does this work with delegate registrations?
  • How does this work with instance registrations?
  • How does this interact, if at all, with registration sources?

I think it could be interesting, but I can see it may also lead to a lot of complexity. Getting some clarification around it would be valuable.

And, of course, if it turns out the problem is too big, it may not be worth solving, in which case it'd be nice to close the issue. It has technically been open for like three years...

@tillig
Copy link
Member

tillig commented Nov 24, 2016

Another question: When does IfNotExists evaluate? For example, if you have this...

builder.RegisterType<First>().IfNotExists().As<IService>();
builder.RegisterType<First>().As<IService>();

Is IfNotExists a deferred thing, like those registrations are held for the end and then run, checking everything registered in the container? Or is it "if it doesn't exist at the point in time at which the IfNotExists registration executes?"

In the former case, the first registration wouldn't run because it would see that the second registration filled the requirement. In the latter case, you'd get two copies of the registration in the container.

I continue to struggle to understand how to determine "equality" in the context of IfNotExists().

A couple of possibilities I can think of:

Use the new Properties dictionary on ContainerBuilder to enable a more contextual equality checking.

For example, we could make some extension methods to allow a registration to add to the dictionary...

builder.RegisterType<First>().As<IService>().SetExists("First");

// which would be roughly equivalent to...
builder.RegisterType<First>().As<IService>();
builder.Properties["First_Exists"] = true;

Then a corresponding extension for checking exists:

builder.RegisterType<First>().As<IService>().IfNotExists("First");

// which would need to evaluate some sort of callback
// that roughly does...
if(!builder.Properties.ContainsKey("First_Exists"))
{
  builder.RegisterType<First>().As<IService>();
}

That latter example isn't exactly what it would do, but the basic equivalent. The idea being the registration callback being added would have that clause in it.

Provide some sort of predicate ability. Instead of just IfNotExists() it'd be IfNotExists(predicate) like:

builder.RegisterType<First>()
  .As<IService>()
  .IfNotExists(ctx => ctx.IsRegistered(typeof(First)));

I'm not sure what would be passed into that predicate. Maybe the component registry at the time the check is occurring? This would allow the caller to determine what equality means, though it may not be as simple in usage as the properties dictionary.

@tillig
Copy link
Member

tillig commented Feb 8, 2017

Minor progress(?) update: I spent a few hours on this one today, looking to see where/how something like this might best be implemented. It's definitely not straightforward and will likely incur some breaking API changes.

What we have

When you register something in a ContainerBuilder, what that is really doing is setting up a callback function against an IComponentRegistry. Basically everything from RegisterType<T> to RegisterInstance<T> becomes a call to ContainerBuilder.RegisterCallback(Action<IComponentRegistry> configurationCallback).

When you call Build() this is basically a foreach over the set of callback delegates to add individual registrations and registration sources to the registry.

RegisterType<T> is basically...

var rb = RegistrationBuilder.ForType<TImplementer>();
builder.RegisterCallback(cr => RegistrationBuilder.RegisterSingleComponent(cr, rb));
return rb;

Note the actual IRegistrationBuilder in there isn't actually the thing that does the registering - it only has some metadata. The thing actually doing the registering is that cr => RegistrationBuilder.RegisterSingleComponent(cr, rb) part.

If we wanted to add a fluent mechanism for adding a predicate, something like this...

builder.RegisterType<First>()
  .As<IService>()
  .OnlyIf(ctx => !ctx.IsRegistered(typeof(First)));

...then we'd need a way to grab that callback, the cr => RegistrationBuilder.RegisterSingleComponent(cr, rb) bit, and change that within the ContainerBuilder to wrap it in a predicate so it becomes, effectively, cr => if(predicate(cr)) { RegistrationBuilder.RegisterSingleComponent(cr, rb); }

Keep in mind this gets more complex because things like assembly scanning, registration sources, and so on have different callbacks they execute to get their jobs done.

We have a few options, but I'm not sure if any are terribly non-invasive. Totally open to additional ideas. Brainstorming here.

Option 1: Add predicate overloads to registration methods

This is probably the simplest thing that would work. Using RegisterType<T> as an example, we'd just provide an additional optional parameter to the method to allow this to work:

builder
  .RegisterType<First>(ctx => !ctx.IsRegistered(typeof(First)))
  .As<IService>();

By taking the predicate into the method at the start, we can easily capture the point at which builder.RegisterCallback is actually invoked and wrap the delegate right there.

The benefit here is that it probably wouldn't break any interfaces. It'd also be pretty simple to do.

The drawback is that it's a pretty ugly syntax, not fluent at all. That definitely goes against how other Autofac additions have been made.

Option 2: Track callbacks made in the ContainerBuilder

This is a lot more invasive but allows for a nicer syntax. The idea with this is that callback registrations would be given individual IDs at registration time. If RegisterType<T> currently looks like this...

var rb = RegistrationBuilder.ForType<TImplementer>();
builder.RegisterCallback(cr => RegistrationBuilder.RegisterSingleComponent(cr, rb));
return rb;

...it would need to become something slightly different to allow the value to be located later:

var rb = RegistrationBuilder.ForType<TImplementer>();
rb.CallbackContainer = builder.RegisterCallback(cr => RegistrationBuilder.RegisterSingleComponent(cr, rb));
return rb;

The CallbackContainer property would probably have some sort of Guid ID as well as a reference to the actual Action<IComponentRegistry> that is set up in that RegisterCallback call. Later extension methods could access the delegate stored in the CallbackContainer and modify that as needed - like to wrap it in a predicate.

An OnlyIf(Predicate<IComponentRegistry>) extension might look like...

var original = rb.CallbackContainer.Callback;
Action<IComponentRegistry> updated = reg => if(predicate(reg)) { original(reg); };
rb.CallbackContainer.Callback = updated;
return rb;

This would change the IRegistrationBuilder interface and creates a sort of circular owner/child relationship between the IRegistrationBuilder and the ContainerBuilder since the ContainerBuilder holds the list of all of the CallbackContainer objects to execute during a call to Build(). You'd also have to set that CallbackContainer any time you register a callback or you'd not be able to attach a predicate.

The upside is that it might allow for a better syntax. I'm not sure if it would open any additional doors for future enhancements or just be another property forever part of the API.

Potential gotcha

Since I haven't gotten terribly far with implementation, I don't know if it'll be a gotcha yet or not, but... Whenever you query an IComponentRegistry for something like IsRegistered<T>() it runs through some initialization and caching logic to determine whether the thing you asked for is registered.

I'm not sure what might happen if you query the registry while you're still trying to register things, which is the case during callback execution in ContainerBuilder.Build. I get a feeling it might be bad news, though admittedly that's what Update is sort of doing right now - it adds registrations to an existing component registry. I don't think we'd hit any of the challenges of #811 since we'd still have the standard Build running, but I'm not sure if there'd be any other sort of impact.

@tillig
Copy link
Member

tillig commented Feb 8, 2017

As part of my experimentation, I made some assumptions to answer the questions about the design. Perhaps changing some of the decisions would impact the design.

  • The user could provide a Predicate<IComponentRegistry> to determine whether a registration callback should execute. This method could be called something like OnlyIf() and would be run like builder.RegisterType<T>().OnlyIf(predicate);.
  • We could implement a simple IfNotExists() method using OnlyIf() and providing a pre-baked predicate.
  • The predicate would run at the point in time when the callback executes. This avoids runtime execution costs (after the container is built).
  • Given it runs at callback execution time, order of registration is important. It's not deferred and doesn't wait until the entire container is built to then execute post-build.

@tillig
Copy link
Member

tillig commented Feb 9, 2017

I just pushed something to a branch called onlyif that may work here. You can see some example usages in the unit test fixture. At a high level, you have two options:

// Provide a Predicate<IComponentRegistry>
builder
  .RegisterType<Something>()
  .OnlyIf(reg => { /* Do something with IComponentRegistry */ });

// or provide a service type
builder
  .RegisterType<First>()
  .As<IService>();
builder
  .RegisterType<Second>()
  .As<IService>()
  .IfNotRegistered(typeof(IService));

The only interface broken is ContainerBuilder.RegisterCallback in that now, instead of returning void, it returns a ContainerCallback object - a way we can get a reference to the original callback method and replace it with something wrapped in a predicate.

Interested in opinions/feedback.

@tillig
Copy link
Member

tillig commented Feb 20, 2017

Version 4.4.0-394 is on the MyGet feed and has both OnlyIf and IfNotRegistered ready to go. Give it a shot, see what you think.

@tillig
Copy link
Member

tillig commented Mar 1, 2017

I published Autofac 4.4.0 to NuGet which includes the OnlyIf and IfNotRegistered functions. Documentation here.

@tillig tillig closed this as completed Mar 1, 2017
@ashmind
Copy link

ashmind commented Apr 21, 2019

Just a note that it was a binary breaking change (void RegisterCallback to Builder.DeferredCallback RegisterCallback) in a minor version (4.4). Noticed because my small extension library breaks between Autofac 4.0 and Autofac 4.9.

Quite easy to fix, but I thought I'd post it as it might be useful for someone else googling the error:

System.MissingMethodException : Method not found: 'Void Autofac.ContainerBuilder.RegisterCallback(System.Action`1<Autofac.Core.IComponentRegistry>)'

@alexmg
Copy link
Member Author

alexmg commented Apr 22, 2019

@ashmind Thanks for leaving the note for others to find.

@alexinea
Copy link

alexinea commented Apr 29, 2019

In fact, it is also very useful to detect directly from ContainerBuilder whether a pair of serviceType-implementaionType mapping has been registered. The purpose of testing is not necessarily to avoid double registration. Instead, we may not register the mapping and only check if it is registered or not..

So I think such api is very useful:

var b = builder.IsRegistered<IService>();
//and so on...

@drauch
Copy link

drauch commented Mar 17, 2020

Is there a way to register a specific IStartable only once? Two modules of our code base both register the same startable. Is there a way to execute it only once?

Can the modules use IfNotRegistered() somehow to do this? The service type is IStartable and I don't want to not register it if some other IStartable is already registered. Only if this specific one is not registered.

@drauch
Copy link

drauch commented Mar 17, 2020

Hmm, I came up with the following helper method. Maybe it helps somebody out there:

    public static void RegisterStartableIfNotYetRegistered<TStartable> (this ContainerBuilder builder)
    {
      var startableType = typeof (TStartable);
      var startableTypeKey = $"PreventDuplicateStartable({startableType.FullName})";

      if (builder.Properties.ContainsKey(startableTypeKey))
        return;

      builder.Properties.Add(startableTypeKey, null);
      builder.RegisterType<TStartable>().As<IStartable>();
    }

@NigelThorne
Copy link

I think the missing features is "RegisterDefault" which means... at the point that the type is resolved. If there is not other implementation defined, then use this one.

Great for implementing a NullObject pattern.

@tillig
Copy link
Member

tillig commented Sep 10, 2021

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

No branches or pull requests

6 participants