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

Module.AttachToComponentRegistration() does not detect new registrations in nested scopes #218

Closed
alexmg opened this issue Jan 22, 2014 · 20 comments
Labels

Comments

@alexmg
Copy link
Member

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on May 10, 2010 06:23:53

See: http://groups.google.com/group/autofac/browse_thread/thread/86592faf0bb5aad9

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

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on May 28, 2010 03:52:27

Labels: -Priority-Medium -Milestone-Release2.2 Priority-High Milestone-Release2.3

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on October 15, 2010 23:05:23

Labels: -Milestone-Release2.3

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From mrdont@mail.ru on October 17, 2010 18:38:20

I've tried to implement NestedLifetimeScopeCreated event when new lifetimeScope is created, but this seems to be dead-end way, 'cause IModule has no idea of lifetimes and cannot track their creation, even with this event.
So I've tried to come from other side: if I have to register module in any nested lifetime (not any actually, but those who create new ComponentRegistry), let autofac do it for me. I called such modules inherited, and these modules are stored in InheritedModules collection of ComponentRegistry much like as registration sources collection.
When lifetimeScope creates new ComponentRegistry it applies all inherited modules from parent ComponentRegistry.

Patch is attached.

Attachment: AutoFac.2.3_rev635.patch

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on October 18, 2010 13:40:32

Thanks for persisting with this - I'll take a look at the patch ASAP.

Cheers,
Nick

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From mrdont@mail.ru on January 07, 2011 20:46:28

Hi, Nicholas,

Have you already taken a look at this patch? Could it be imported into the central repo?

For your convenience I've rebased it to the latest revision #682 (56979dd4e765).

Attachment: Autofac_rev682.patch

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on January 08, 2011 19:33:03

Hi Ilya,

Thanks for following up on this, sorry about the extended delay.

The patch is good - I've just held off on this because it seems like something that should be inherent in the core of how modules work, rather than an opt-in. Unfortunately I haven't worked out just what that core change should be :)

Coincidentally, I've just added a new set of APIs in the trunk that will allow you to do what you need. The new APIs are really intended to support diagnostic scenarios, but I've attached an example of how you can implement inherited modules with them.

Please let me know what you think, and thanks again for your help with this.

Cheers,
Nick

Attachment: InheritedModuleExample.zip

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From mrdont@mail.ru on January 11, 2011 03:41:19

I've reimplemented my modules inheriting them from InheritedModule in your example — and I've got behavior I was needed. I like this way (with IContainerAwareComponent) 'cause it is more general than InheritedModules collection in ComponentRegistry.

So it fits my usage scenarios, but there is one pitfall: if I register module in the nested scope and not in the root container, or if I update existing container with my module, it doesn't become attached to ChildLifetimeScopeBeginning because SetContainer method is not called.

Maybe some ILifetimeScopeAwareComponent interface should also be added?

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on January 18, 2011 14:52:52

Thanks for the feedback - another good catch regarding modules registered in child scopes or after initialisation.

I suspect these are limitations we may have to live with; I'll leave this issue open as a reminder, an opportunity to fix it might come up in the future.

Labels: -Priority-High Priority-Low

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From mrdont@mail.ru on January 18, 2011 15:32:46

I'm not willing to give up, and have one and a half idea how we can overcome these limitations.

  1. Create two different types of modules. One builds component registry only once, as the current implementation does now. It should be used for plain registrations. Another is intended for registration's event attaching and is called to configure any new component registry created in nested scopes. For example:

    public interface IModule
    {
    void Configure(IComponentRegistry componentRegistry);
    }

    public interface IInheritedModule: IModule
    {
    void ConfigureNested(IComponentRegistry componentRegistry); // or Reconfigure(...)
    }

1.5 Another way: make event not in container/lifetime, but in ComponentRegistry. It should be fired when child component registry is created, so module can attach to that registry's events.

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From mrdont@mail.ru on March 31, 2011 12:33:37

I've just managed to implement the first way, and it works fine.
If you're interested in how it looks like, you can pull it from http://code.google.com/r/ilyaindustry-issue218fix/ Now I can derive my modules direct from Autofac.Module, and they will handle events in nested scopes as well.

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on April 04, 2011 03:21:39

Thanks - I'll check it out ASAP.

Cheers

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From mrdont@mail.ru on November 11, 2011 14:13:09

Hi Nicholas, what is your decision about the proposed http://code.google.com/p/autofac/issues/detail?id=218#c10 solution?
It works well and doesn't breaks existing Autofac.Module inheritors (at least at the time of commit), so could you tell your objections against merging it into the main clone and closing this issue?
In case you would like to review the changes once again, here is link to the changeset: http://code.google.com/r/ilyaindustry-issue218fix/source/detail?r=a49f7226f72de73f35401759e9b280840abe924f

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on November 13, 2011 21:41:46

Howdy - brain dump below.

The code looks good, but I'm hesitant to make changes in this particular area of Autofac as I'm sure you have noticed :) it is fairly fragile. Autofac's fairly complex in the way modifiable child scopes are handled - every time we fix one of these limitations another sufaces.

On the suggested solution: the idea of 'modules' is currently purely in the container building mechanism, not a part of the core container; I don't think a dependency from LifetimeScope -> module is the right direction to couple the implementation.

Also, adding IInheritedModule or a change to IModule encodes the internal limitation into the public API, so it may be harder to fix the internals down the track without breaking users.

Looking back through Autofac trunk, it seems like the long-term requirement would be to do some work to make ComponentRegistry behave properly in a hierarchy.

  • We wouldn't need to think about all of this module inheritance stuff because components added to child ComponentRegistries would fire the ComponentRegistered event in the parent
  • The need to filter out adapter components would go away, because child registries would know internally how to correctly re-apply adapters without duplication of work already done in the parent
  • ScopeRestrictedRegistry, CopyOnWriteRegistry etc would go away

This is pretty tricky and requires more trade-offs, so not likely to happen in a hurry. Maybe something for Autofac 3 :)

The workaround to this issue, registering the modules in nested scopes, seems straightforward even if it is a little awkward. Users who can't apply the workaround can get even further using your InheritedModule helper class/events.

So, although taking a shot at a deeper solution may be some time off, hopefully no one is too badly inconvenienced in the meantime.

Thanks for all the ideas and effort on this, sorry to leave it dangling yet again.

Cheers!
Nick

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From travis.illig on September 21, 2012 09:26:42

Labels: Module-Core

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From d...@drusellers.com on January 09, 2014 13:41:41

Has there been any progress on this issue?

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From travis.illig on January 09, 2014 17:10:29

Sorry, no. Since it'll probably be a non-trivial shift to the internals, it'd be something we'd look at for the 4.0 release. This and a few others that require deep internals and possibly behavior changes.

@tillig
Copy link
Member

tillig commented Mar 20, 2014

The earlier mention of getting component registries to work in a hierarchy would probably solve other issues, too, like #272. Might peripherally affect things like #341 with the ordering of registrations becoming increasingly important.

@tillig
Copy link
Member

tillig commented Oct 26, 2016

This didn't make it into Autofac 4 but is not forgotten and not off the table. It's just... pretty non-trivial, so the .NET Core stuff took precedence and pushed it out.

@tillig
Copy link
Member

tillig commented Oct 26, 2016

This issue appears to be somewhat related to, yet sort of the opposite of, #608.

@evgeny-burmakov
Copy link

Any news after 8 years?

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

No branches or pull requests

3 participants