JBMICROCONT-309; aliases and scoping

[alesj]

With Kabir's issue:
- http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4154037#4154037
I've re-factored scoped kernel/controller code a bit.

The new addition makes aliases pretty useful now. :-)
The detail is in the state they are added - past PRE_INSTALL (after scoping).
It solves what we mostly want to do with an alias and scoped beans.
But the problem we're inconsistent with how we do the aliases - scoping aliases to be exact.

Focusing just on scoping + alias:

In my new code, where I handle aliases on annotation, my controller is already scoped, so my alias is unique in that controller scope.
This makes combination of global unique bean name + scoped alias a great asset for other scoped beans (Kabir's issue use case) - but see issue1.

Issue#1: Controller should have option to do just local lookup - otherwise order of bean deployment is important; e.g. picking up bean from parent (already available), where you would actually want scoped one (not available at dependency item resolution)
This can also lead to CCE issues; parent bean being of wrong type

Currently scoping controllers are completely transparent, visible only to top level controller. And as such, install and uninstall only take place at the top, propagating the action to the right controller based on the (hopefully) unique name.

Issue#2: If we do alias addition after scoping (post PreInstall), the alias (context) name is unique, since it's signed by controller id (new notion I needed to add). But currently we register context's aliases in Controller:install, which is way too early. :-) Hence we loose alias (context) name uniqueness - they are all signed by top controller.
Alias addition should be done in Describe state, since I very much doubt anybody depends on the bean via alias before Describe state. This would solve the uniqueness problem.

Aliases can also be deployed separately - see DeploymentAliasDeployer in deployers project.

Issue#3: NamedAliasMetaData is missing a way to specify to which scope this alias should be installed.
In order to do this, there are two ways that I see:
1) Controller::add/removeAlias take ScopeKey parameter, NamedAliasMetaData knows how to create that scope key
2) NamedAliasMetaData takes annotations, AliasControllerContext uses MetaData to keep that annotations, and we use the same lookup mechanism as we do in PreInstallAction

With all this, once we abandon context name notion - making it automatically unique - defining aliases and coding against them would make scoping truly usable.

[alesj]

"alesj" wrote:

Issue#1: Controller should have option to do just local lookup - otherwise order of bean deployment is important; e.g. picking up bean from parent (already available), where you would actually want scoped one (not available at dependency item resolution)
This can also lead to CCE issues; parent bean being of wrong type

I'm pretty much done on other issues (I won't be moving direct alias registration into post PreInstall, since there is a simple workaround).

But I would need some new SPI on the Controller to handle issue#1.
I'm thinking of something like this

 ControllerContext getLocalContext(Object name, ControllerState state);

Which would be noop.
It's just that scoped controller wouldn't return parent's contexts as well.

Then user could mark inject as local.

<inject bean="mybean" local="true"/>

And then alias could also have option to be just locally resolved - probably similar way as I did scoped alias -> via new alias controller context.

[adrian.brock]

"alesj" wrote:

But I would need some new SPI on the Controller to handle issue#1.
I'm thinking of something like this

 ControllerContext getLocalContext(Object name, ControllerState state);

Scoping is an implementation detail and so such a call should
be part of those implementation details not the SPI.

I think if you've got something called "ScopedAlias" and
"ScopedController" that are both implementation details then it is better
they talk to each other using non-SPI.
You can see this by just asking "What does a LocalContext mean?" when you
look at the Controller SPI.

If you want to introduce scoping into the SPI then think about and do it properly
instead of just hacking one method into it which is unrelated to its
"do one thing well" contract.
i.e. add the methods to the contract that allow you to specify the structure
and have an alternate "scoping" implementation (e.g. peer instead hierarchical).

But this route seems like overkill for the sake of one method call unless you
can see further enhancements in this area in the future?

[adrian.brock]

"alesj" wrote:

Then user could mark inject as local.

<inject bean="mybean" local="true"/>

Such a semantic would lead me to the alternate conclusion,
i.e. that the structure of the controller delegation should be a part of SPI

[alesj]

Such a semantic would lead me to the alternate conclusion,
i.e. that the structure of the controller delegation should be a part of SPI

Which spi? Dependency spi? Bean metadata spi?
I don't see what you mean here. :-)

[adrian.brock]

dependency with knock on affects into kernel

[alesj]

dependency with knock on affects into kernel

How would you then introduce this information?

OK, it should be more generic that simple 'local', since, as you said, controller scope could/should be more generic (peer, hierarchy, ...).

To actually get the context from the wanted scope, some simple interface could be introduced.

public interface GraphController
{
 ControllerContext getContext(Object name, ControllerState state, LookupStrategy strategy) throws Throwable
}

Where LookupStrategy would dictate how lookup should be done, e.g. local first, local only, parent only, ...

If underlying Controller didn't impl this, we could either throw error or silently fall-over to plain context lookup.

[alesj]

I've added this

public interface GraphController
{
 /**
 * Get the context.
 *
 * @param name the context name
 * @param state the required state
 * @param info the seatch info
 * @return found context or null if not available
 * @throws Throwable for any error
 */
 ControllerContext getContext(Object name, ControllerState state, SearchInfo info) throws Throwable;

where normally Controller(Impl) would implement this interface.

And then decide on passed SearchInfo param what to do.
e.g. implement the SearchInfo::type itself - due to some private/protected modifiers, internal info
or let SearchInfo's strategy handle the lookup.

In my current impl, I let strategy handle all different lookups.

 DEFAULT(new DefaultSearchInfoWrapper()),
 LOCAL(new LocalLookupStrategy()),
 TOP(new TopLevelLookupStrategy()),
 PARENT(new ParentOnlyLookupStrategy()),
 DEPTH(new DepthLookupStrategy()),
 LEAVES(new LeavesFirstLookupStrategy()),
 WIDTH(new WidthLookupStrategy()),
 CHILD_ONLY_DEPTH(new ChildrenOnlyDepthLookupStrategy()),
 CHILD_ONLY_LEAVES(new ChildrenOnlyLeavesFirstLookupStrategy()),
 CHILD_ONLY_WIDTH(new ChildrenOnlyWidthLookupStrategy());

But I still don't see where to pass-in this search type info?

I could add new annotation (@SearchType(Search)), which I would only handle in my annotation plugins, but not in XML (too much hassle to get ValueMetaData owner for this simple use case).