JBossDeveloper

I already changed the names you complain about here:

"alesj" wrote:

btw: I don't like type="default" as it doesn't say/explain anything

"alesj" wrote:

This looks wrong or at least not the Guice/Weld concept like.

Sorry, there was a problem with the example. Here is an updated one with the new names.

This will not resolve as you say:

<bean name="bean1" class="SomeBean">
 <!-- type='Supplied' by default on bean level -->
 <qualifier>a</qualifier>
</bean>
<bean name="bean2"class="SomeBean">
 <qualifier>a</qualifier>
 </qualifier>b</qualifier>
</bean>
<bean name="bean3" class="OtherBean">
 <qualifier type="Wanted">a</qualifier>
</bean>

This will inject bean2:

<bean name="bean1" class="SomeBean">
 <!-- type='Supplied' by default on bean level -->
 <qualifier>a</qualifier>
</bean>
<bean name="bean2"class="SomeBean">
 <qualifier>a</qualifier>
 </qualifier>b</qualifier>
</bean>
<bean name="bean3" class="OtherBean">
 <qualifier type="Wanted">a</qualifier>
 <qualifier type="Wanted">b</qualifier>
</bean>

The 'relaxed' matching if only Wanted bean level qualifiers is shown here, and will inject bean1 in both cases.

<bean name="bean1" class="SomeBean">
 <!-- type='Supplied' by default on bean level -->
 <qualifier>a</qualifier>
</bean>
<bean name="bean2"class="SomeBean">
 </qualifier>b</qualifier>
</bean>
<bean name="bean3" class="OtherBean">
 <qualifier type="Wanted">a</qualifier>
 <qualifier type="Wanted">c</qualifier>
</bean>

<bean name="bean1" class="SomeBean">
 <!-- type='Supplied' by default on bean level -->
 <qualifier>a</qualifier>
 </qualifier>b</qualifier>
</bean>
<bean name="bean2"class="SomeBean">
 </qualifier>a</qualifier>
</bean>
<bean name="bean3" class="OtherBean">
 <qualifier type="Wanted">a</qualifier>
 <qualifier type="Wanted">b</qualifier>
 <qualifier type="Wanted">c</qualifier>
</bean>

Maybe I should change 'Wanted' to 'Required'? We could add 'Optional' etc.

"alesj" wrote:

The default qualifier on the bean/class shouldn't imply exact matching,
it should just imply that we always need that qualifier for all (non-explicit?) injection points.

I am not sure if you are saying you think I should turn off the 'relaxed' stuff shown above requiring exact matches?

 

"alesj" wrote:

"kabir.khan@jboss.com" wrote:

I've done this. This differs from the bean-level qualifiers in that if a qualifier exists on an injection point, then the bean-level qualifiers are ignored, so the property level ones completely override the bean level ones e.g.

I would actually go the other way - I wouldn't ignore higher level qualifiers by default.
e.g. @Inject(ignoreOtherQualifiers=true)

ok, I can make that configurable

"alesj" wrote:

An idea, perhaps we could even add an injection-point type to a qualifier.
e.g. <qualifier type="default" injection-point="ctor, property">
Which would mean this qualifier is only used for ctors and properties, but not methods and fields.

I like that idea

 

"alesj" wrote:

"kabir.khan@jboss.com" wrote:

I don't know if the qualifiers for the injection points should be read from the MDR or not? At first glance doing that does not make sense since the component metadata lives below instance level, but I might be wrong.

Don't forget property qualifiers via annotations (as we already support them).
btw: a fix is needed there, since afair we currently don't support just property element w/o value specified,
which is what you will need in order to override/add instance level annotations/qualifiers

This looks fine at xml level:
https://svn.jboss.org/repos/jbossas/projects/kernel/trunk/kernel/src/test/resources/org/jboss/test/kernel/deployment/xml/test/PropertyWithAnnotation.xml

<?xml version="1.0" encoding="UTF-8"?>

<bean xmlns="urn:jboss:bean-deployer:2.0" class="Dummy">
 <property name="PropertyName">
 <annotation>@org.jboss.test.kernel.deployment.xml.support.Annotation1</annotation>
 </property>
</bean>

although I need to check if this works at runtime, e.g. in combination with @Inject

There is a problem with qualifier annotations applied via xml/beanmetadata. Currently, they are only picked up if @Inject is used, e.g.

public class InjectAnnotationPlugin extends PropertyAnnotationPlugin<Inject>
{
 public final static InjectAnnotationPlugin INSTANCE = new InjectAnnotationPlugin();

 protected InjectAnnotationPlugin()
 {
 super(Inject.class);
 }

 @Override
 protected ValueMetaData createValueMetaData(PropertyInfo info, MetaData retrieval, Inject annotation)
 {
 return createTheMetaData(retrieval, annotation);
 }

 @Override
 public ValueMetaData createValueMetaData(ParameterInfo parameterInfo, MetaData retrieval, Inject annotation, ValueMetaData previousValue)
 {
 return createTheMetaData(retrieval, annotation);
 }

 private ValueMetaData createTheMetaData(MetaData retrieval, Inject annotation)
 {
 HashSet<Object> annotationSet = null;
 if (retrieval != null)
 {
 Annotation[] annotations = retrieval.getAnnotationsAnnotatedWith(Qualifier.class);
 if (annotations.length > 0)
 {
 annotationSet = new HashSet<Object>(annotations.length);
 for (Annotation ann : annotations)
 annotationSet.add(ann);
 }
 }
 return ValueUtil.createValueMetaData(annotation, annotationSet);
 }

}

The annotationSet goes into AbstractInjectionValueMetaData as qualifiers. The annotation qualifiers are ignored if

<inject/>

is used since then the InjectAnnotationPlugin is not triggered.

I am not handling these in any special way at the moment. I probably need to do something similar to what I have done for the supplied annotation qualifiers in CommonAnnotationAdapter

 protected void handleAnnotations(BeanInfo info, MetaData retrieval, U handle, boolean isApplyPhase) throws Throwable
 {
 ...
 //Existing code
 // class
 ClassInfo classInfo = info.getClassInfo();
 for (Annotation annotation : retrieval.getAnnotations())
 {
 for(T plugin : getPlugins(ElementType.TYPE, annotation, null, annotationClasses))
 {
 if (isApplyPhase)
 applyPlugin(plugin, annotation, classInfo, retrieval, handle);
 else
 cleanPlugin(plugin, annotation, classInfo, retrieval, handle);
 }
 }

 //New code to handle meta-annotations on class
 Map<Class<? extends Annotation>, Set<T>> metaAnnotationsForType = metaAnnotationsPluginsMap.get(ElementType.TYPE);
 if (metaAnnotationsForType != null)
 {
 for (Entry<Class<? extends Annotation>, Set<T>> entry : metaAnnotationsForType.entrySet()) //Qualifer is registered here
 {
 for (Annotation annotation : retrieval.getAnnotationsAnnotatedWith(entry.getKey()))
 {
 for (T plugin : entry.getValue())
 {
 if (isApplyPhase)
 applyPlugin(plugin, annotation, classInfo, retrieval, handle);
 else
 cleanPlugin(plugin, annotation, classInfo, retrieval, handle);
 }
 }
 }
 }
 ...

The plugin that handles adding the qualifiers

public class SuppliedQualifierAnnotationPlugin extends ClassAnnotationPlugin<Annotation> implements MetaAnnotationPlugin<ClassInfo, Qualifier>
{
 public static final SuppliedQualifierAnnotationPlugin INSTANCE = new SuppliedQualifierAnnotationPlugin();

 protected SuppliedQualifierAnnotationPlugin()
 {
 super(Annotation.class);
 addTypes(ElementType.TYPE);
 }

 protected boolean isElementTypeSupported(ElementType type)
 {
 return true;
 }

 @Override
 protected boolean isCleanup()
 {
 return true;
 }

 @Override
 protected List<? extends MetaDataVisitorNode> internalApplyAnnotation(ClassInfo info, MetaData retrieval, Annotation annotation, KernelControllerContext context) throws Throwable
 {
 if (context != null)
 {
 MetaDataRetrieval instanceMetaData = context.getKernel().getMetaDataRepository().getMetaDataRepository().getMetaDataRetrieval(context.getScopeInfo().getMutableScope());
 QualifiersMdrUtil.addQualifiersToMdrRetrieval(instanceMetaData, QualifierType.SUPPLIED, null, annotation);
 }
 return null;
 }

 @Override
 protected void internalCleanAnnotation(ClassInfo info, MetaData retrieval, Annotation annotation, KernelControllerContext context) throws Throwable
 {
 if (context != null)
 {
 MetaDataRetrieval instanceMetaData = context.getKernel().getMetaDataRepository().getMetaDataRepository().getMetaDataRetrieval(context.getScopeInfo().getMutableScope());
 QualifiersMdrUtil.removeQualifiersFromMdrRetrieval(instanceMetaData, QualifierType.SUPPLIED, null, annotation);
 }
 }

 //From MetaAnnotationPlugin
 public Class<Qualifier> getMetaAnnotation()
 {
 return Qualifier.class;
 }
}

However, that means checking everything for qualifiers annotations in the annotation adapter. My initial idea is to do something along the lines of this pseudo-code for each parameter/property, but I will sleep on it

for (ParameterMetaData pmd : constructorInfo.getParameters())
{
 if (pmd.getValue() != null && pmd.getValue() instanceof AbstractInjectionValueMetaData)
 {
 Map<Class<? extends Annotation>, Set<T>> metaAnnotationsForType = metaAnnotationsPluginsMap.get(ElementType.PARAMETER);

 for (Entry<Class<? extends Annotation>, Set<T>> entry : metaAnnotationsForType.entrySet()) //Qualifer is registered here
 {
 for (Annotation annotation : retrieval.getAnnotationsAnnotatedWith(entry.getKey()))
 {
 for (T plugin : entry.getValue())
 applyMetaAnnotationPlugin(plugin, annotation, parameterInfo, pmd, retrieval, handle); //Can get the AbstractInjectionValueMetaData from pmd internally and set the annotation in the set of qualifiers there
 }
 }
 }
}

If the value is a collection, array etc. I would also need to go into each element checking for AIVMD.

 

"alesj" wrote:

I wouldn't bother too much.
If the user overrides class info or existing MDR info with xml, then that's what we should use.
If he wants something more on the plain inject, let him add qualifiers via xml as well.

Actually it is worse than I said yesterday. It doesn't really matter whether the injection point qualifier annotations come from bmd/xml or not, the trigger for looking for the annotations is @Inject.

This will work

public class AnnotatedTarget
{
 @Inject @SomeQualifier
 public void setBean(Bean bean){}
}

but this will not

public class Target
{
 @SomeQualifier
 public void setBean(Bean bean){}
}

<bean name="Target" class="Target">
 <property name="bean">
 <inject/>
 </property>
</bean>

I would have to do

<bean name="Target" class="Target">
 <property name="bean">
 <inject>
 <qualifier>@SomeQualifier</qualifier>
 </inject>
 </property>
</bean>

but @SomeQualifier already exists in the class, so that is not ideal. The qualifier annotations need to be added some other way.

I've had a look at plugging this into the annotation adapters, but it doesn't really "fit".

What is happening at the moment is
1) PreInstallAction describeVisits the BeanMetaData
2) Inspect the parent nodes in AbstractInjectionValueMetaData during describeVisit() to determine the injection point type.
3) AIVMD.describeVisit() creates a qualifier dependency item if explicit qualifiers are in place. Otherwise a normal SearchClassContextDependencyItem is created.
4) PreInstallAction populates the MDR
5) PreInstallAction pushes the context's explicit qualifiers to MDR

I think what I will do is

1) PreInstallAction describeVisits the BeanMetaData as before
2) Determine the relevant parent nodes in AbstractInjectionValueMetaData during describeVisit().
3) Either
a) See if I can get rid of SearchClassContextDependencyItem, since it should just be a special case of a qualifier dependency item with no qualifiers.
b) Create dependency items as before
Stash the nodes determined in 1) into the dependency item
4) PreInstallAction populates the MDR as before
5) Extend this step to look at all the dependency items. For the relevant ones (SearchClassContextDependencyItem/qualifier dependency item) check the parent nodes for qualifiers and add to the dependency item.

Is this too much of a hack? At least this way we don't have to go through everything again.

Note to self :-)

This pushes (un)install method annotations to the MDR

https://jira.jboss.org/jira/browse/JBKERNEL-68

"kabir.khan@jboss.com" wrote:

1) PreInstallAction describeVisits the BeanMetaData as before
2) Determine the relevant parent nodes in AbstractInjectionValueMetaData during describeVisit().
3) Either
a) See if I can get rid of SearchClassContextDependencyItem, since it should just be a special case of a qualifier dependency item with no qualifiers.
b) Create dependency items as before
Stash the nodes determined in 1) into the dependency item
4) PreInstallAction populates the MDR as before
5) Extend this step to look at all the dependency items. For the relevant ones (SearchClassContextDependencyItem/qualifier dependency item) check the parent nodes for qualifiers and add to the dependency item.

Is this too much of a hack? At least this way we don't have to go through everything again.

This is now working with

<inject/>

However, I undid the stuff I did in the InjectAnnotationPlugin, so @Inject + qualifiers is broken. The problem is that 5) happens during PRE_INSTALL and the annotation plugins kick in during DESCRIBE so the annotation plugins need to pick up the qualifiers as before. I'll readd the stuff I did in InjectAnnotationPlugin on Monday.

Also, I need to test field properties and see if something can be done with ValueFactories