[Patch] Improve handling of component managers in extension points

[Olemis Lang <olemis+trac@…>]

Verify that a component manager that is also a component will only be listed in extension points instantiated in its scope.

See bh:comment:5:ticket:438

[Olemis Lang <olemis+trac@…>]

Proposed patch including a test case is available [bh:attachment:t438_r1457691_cpmngr_xtpt.diff:ticket:438 here] . Please see bh:comment:7:ticket:438 .

[Christian Boos]

So basically you say that adding this constraint will help you fix an (indirectly) related test in a plugin… That's hardly a justification. Please rather explain why it would be a bad idea in general to let a ComponentManager be a Component instantiated by another ComponentManager.

It's true that in Trac proper we never use any other ComponentManager than the Environment, so it's a bit of a gray area. While experimenting with a minimal example of a ComponentManager/Component used as a component in another ComponentManager/Component (i.e. what you tried to forbid), I noticed that it mostly works, except for the fact that new instances are being recreated each time. If there's anything to fix, it should rather be this.

from trac.core import *
from trac.core import ComponentManager

class Intf(Interface): pass

class CM1(ComponentManager, Component):
  implements(Intf)
  def say(self): print "hi"
  def __init__(self, *args): print repr(args)

class CM2(ComponentManager, Component):
  ep = ExtensionPoint(Intf)

>>> cm2 = CM2()
>>> cm1 = CM1(cm2)
(<__main__.CM2 object at 0x02CAFDD0>,)
>>> cm1 is CM1(cm2)
(<__main__.CM2 object at 0x02CAFDD0>,)
False

But other than that it "works" and that might a desired behavior:

>>> for c in cm2.ep: print c.say()
...
(<__main__.CM2 object at 0x02CAFDD0>,)
hi
None

So I see no reasons to prohibit this.

[Olemis Lang <olemis+trac@…>]

Replying to cboos:

So basically you say that adding this constraint will help you fix an (indirectly) related test in a plugin… That's hardly a justification. Please rather explain why it would be a bad idea in general to let a ComponentManager be a Component instantiated by another ComponentManager.

1. impossible to determine initializer parameters when instantiating the inner component manager in extension points …
2. … thus the assumption of them being singletons in the context of its parent manager will not be valid anymore

It's true that in Trac proper we never use any other ComponentManager than the Environment, so it's a bit of a gray area.

;)

While experimenting with a minimal example of a ComponentManager/Component used as a component in another ComponentManager/Component (i.e. what you tried to forbid), I noticed that it mostly works,

I'd rather say it does not fail … my questions are :

• Does it really makes sense ?
• Why would you use one such component manager instead of a plain old component

except for the fact that new instances are being recreated each time.

… afaict the fact is that *args is a bit useless in outermost manager scope considering the fact that it will always be ()

If there's anything to fix, it should rather be this.

from trac.core import *
from trac.core import ComponentManager

class Intf(Interface): pass

class CM1(ComponentManager, Component):
  implements(Intf)
  def say(self): print "hi"
  def __init__(self, *args): print repr(args)

class CM2(ComponentManager, Component):
  ep = ExtensionPoint(Intf)

>>> cm2 = CM2()
>>> cm1 = CM1(cm2)
(<__main__.CM2 object at 0x02CAFDD0>,)
>>> cm1 is CM1(cm2)
(<__main__.CM2 object at 0x02CAFDD0>,)
False

But other than that it "works" and that might a desired behavior:

>>> for c in cm2.ep: print c.say()
...
(<__main__.CM2 object at 0x02CAFDD0>,)
hi
None

So I see no reasons to prohibit this.

Well , I'd rather see a good point in splitting your manager in two pieces

1. the component instantiated in outermost scope implementing Intf
2. the manager hosting other components including the very same mentioned in (1) above

… but what you mention is a very interesting and specific example baked in a lab . Let's talk of the real world .

1. let's say we make ProductEnvironment to become a manager component with flexible args handling like mentioned above
2. both Environment class and ProductEnvironment will implement ISystemInfoProvider interface
3. there is a single , global components cache in component meta
4. while enumerating such extension points in global scope (i.e. Environment is manager) product environments will still be instantiated with manager instance as solely argument
   • even if product args are required the call will not fail
   • but what shall the call to ProductEnvironment return ?
5. OTOH , while enumerating such extension points in product scope (i.e. ProductEnvironment is manager) global environment will still be instantiated with manager instance as solely argument
   • it will break due to the fact the path et al. are required args
   • it will add unwanted system info in product context
   • … and all that, in theory, would be outside our control

Indeed considering your example all manager would have to accept a single manager arg without failing , and the only manager listed in other manager's extension points would be the one instantiated with outermost manager as first arg . Pretty useless afaict . Besides notice that we must isolate product environments and , as such , do not want any interference of the global manager , which is not under our control .

So , your suggestion will not work in real life … unless some other approach or hack will be handy . I'll be looking forward to your comments .

[Christian Boos]

Replying to Olemis Lang <olemis+trac@…>:

Replying to cboos:

So basically you say that adding this constraint will help you fix an (indirectly) related test in a plugin… That's hardly a justification. Please rather explain why it would be a bad idea in general to let a ComponentManager be a Component instantiated by another ComponentManager.

1. impossible to determine initializer parameters when instantiating the inner component manager in extension points …
2. … thus the assumption of them being singletons in the context of its parent manager will not be valid anymore

Right, I see now that the automatic instantiation is problematic.

Thanks for detailing more your specific use case, that helped.

the only manager listed in other manager's extension points would be the one instantiated with outermost manager as first arg . Pretty useless afaict .

… in your scenario (1 Environment, n ProductEnvironment), most probably.

I've experimented with an approach in which a ComponentManager can be seen as a Component of another "parent" ComponentManager. It works, but I'll leave that as experiments because the changes needed to make that work the way I wanted to were not so simple after all, and because I don't have a practical use case for that anyway ;-)

So I agree that we should better make the current limitation explicit and add your proposed change. Now that I'm convinced it can't break other users' code, I think it's OK to have it for 1.0.2.

[Olemis Lang <olemis+trac@…>]

Replying to cboos:

[…]

Thanks for detailing more your specific use case, that helped.

;)

the only manager listed in other manager's extension points would be the one instantiated with outermost manager as first arg . Pretty useless afaict .

… in your scenario (1 Environment, n ProductEnvironment), most probably.

yes

I've experimented with an approach in which a ComponentManager can be seen as a Component of another "parent" ComponentManager. It works, but I'll leave that as experiments because the changes needed to make that work the way I wanted to were not so simple after all, and because I don't have a practical use case for that anyway ;-)

in any case if your solution proves to work in practice I'd have no objections with including it after milestone:1.0.2 … as long as it will be backwards compatible ;)

So I agree that we should better make the current limitation explicit and add your proposed change. Now that I'm convinced it can't break other users' code, I think it's OK to have it for 1.0.2.

Thanks . I look forward to see that change incorporated into Trac core by then .

[Christian Boos]

an example of two ComponentManager/Component classes, one "parented" in the other

[Christian Boos]

the experimental changes needed to make the sample.py work as expected

[Christian Boos]

bh:comment:7:ticket:438 patch committed as r11758 (with a simplified test case).