JBAS-5673 - Metadata processing

[emuckenhuber]

Looking into https://jira.jboss.org/jira/browse/JBAS-5673, i noticed that something is missing in the web annotation processing with injection targets.

e.g. having 2 classes with the same EJB or Resource reference ends up in only one injection target for this reference:

public class FooServlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;
}
public class Foo2Servlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;
}

would result in a AnnotatedReferenceMetaData with one InjectionTarget, instead of two - which basically affects also @Resource, @PersistenceContext, @WebServiceRef processors.

As it's currently done like (pseudocode):

 AnnotatedEJBReferenceMetaData ref = createAnnotatedEJBRef(...);
 ref.setInjectionTargets(injectionTargets);
 refs.add(ref);

So i guess it should rather be something like:

 AnnotatedEJBReferenceMetaData newRef = createAnnotatedEJBRef(...);
 AnnotatedEJBReferenceMetaData existingRef = refs.get(newRef.getName());
 if(existingRef == null)
 refs.add(ref);
 else
 mergeInjectionTargets(existingRef, newRef)

Or should this be done in a different way ?

[emuckenhuber]

"emuckenhuber" wrote:


would result in a AnnotatedReferenceMetaData with one InjectionTarget, instead of two - which basically affects also @Resource, @PersistenceContext, @WebServiceRef processors.

Hmm actually @WebServiceRef could work, as it uses the className/fieldName for the name, therefore it would not override the existing reference.

[starksm64]

"emuckenhuber" wrote:

So i guess it should rather be something like:

 AnnotatedEJBReferenceMetaData newRef = createAnnotatedEJBRef(...);
 AnnotatedEJBReferenceMetaData existingRef = refs.get(newRef.getName());
 if(existingRef == null)
 refs.add(ref);
 else
 mergeInjectionTargets(existingRef, newRef)

Or should this be done in a different way ?

That sounds right, but you would need to validate that the injection target has the same type. This should not be allowed:

public class FooServlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;
}
public class Foo2Servlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF2.class)
 private StatefulIF2 statefulTestBean;
}

@Session
public class SomeEJb
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF2.class)
 private StatefulIF2 statefulTestBean2;
}

[starksm64]

"emuckenhuber" wrote:

Hmm actually @WebServiceRef could work, as it uses the className/fieldName for the name, therefore it would not override the existing reference.

I wonder why we use a different key for these?

[emuckenhuber]

"emuckenhuber" wrote:

Hmm actually @WebServiceRef could work, as it uses the className/fieldName for the name, therefore it would not override the existing reference.

I wonder why we use a different key for these?

Hmm that's because of a request of the WS team, when we added the @WebServiceRef processing - i can talk to them again and see if we can fix that ?

[emuckenhuber]

public class FooServlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;
}
public class Foo2Servlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF2.class)
 private StatefulIF2 statefulTestBean;
}

@Session
public class SomeEJb
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF2.class)
 private StatefulIF2 statefulTestBean2;
}

Hmm i'm not sure if i understood that correctly.

But this use case would not happen by design, as the Web25Creator does not care about @Stateful, @Stateless annotations. So SomeEjb would be treated as any other object (Servlet, Listener, ...)

Most probably we should add something to check for @Stateful, @Stateless, ... and:
1) just ignore the whole class (+ logging)
2) throw an exception
3) treat it as it's now (+ logging)

I guess nr. 1) would make most sense !?

Still having smth like:

public class Servlet
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;

 @EJB(beanName="StatefulTestBean", beanInterface=StatefulLocalIF.class)
 private StatefulLocalIF statefulTestLocalBean;
}

public class Filter
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;

 @EJB(beanName="StatefulTestBean", beanInterface=StatefulLocalIF.class)
 private StatefulLocalIF statefulTestLocalBean;
}

public class RequestListener
{
 @EJB(beanName="StatefulTestBean", beanInterface=StatefulIF.class)
 private StatefulIF statefulTestBean;

 @EJB(beanName="StatefulTestBean", beanInterface=StatefulLocalIF.class)
 private StatefulLocalIF statefulTestLocalBean;
}

should be valid.

[emuckenhuber]

So now to the actual topic of https://jira.jboss.org/jira/browse/JBAS-5673 :)

remove the duplicate processing of annotations happening in the web container TomcatInjectionContainer.

So we should be able to definitely drop those things in the TomcatInjectionContainer:

private void processServlets(JBossServletsMetaData servlets, ClassLoader webLoader)
private void processListeners(List<ListenerMetaData> listeners, ClassLoader webLoader)
private void processFilters(FiltersMetaData filters, ClassLoader webLoader)
private void processClass(String className, ClassLoader webLoader)

as those information have already been populated.

Ignoring dynamic jsp beans, etc. for now it would lead to a processAnnotations like:

 public void processAnnotations(Object object)
 {
 // Get injectors for class
 Map<AccessibleObject, Injector> injectors = encInjections.get(object.getClass().getName());

 // TODO dynamic beans, etc.

 if (injectors == null || injectors.size() == 0) return;
 for (Injector injector : injectors.values())
 {
 injector.inject(object);
 }
 }

Beside the fact that we could split the processAnnotations in doTheKnownInjection(Object) and processAnnotations(Object dynamicBean),
there are still some things not really clear:

Map<AccessibleObject, Injector> injectors = encInjections.get(className);

// instead of

Map<AccessibleObject, Injector> injectors = resolvedClassInjections.get(object.getClass().getName());

which would makes sense, as we don't use the resolvedClassInjections anymore (see processClass) and encInjections is actually already populated by the InjectionHandlers.

But e.g. the UserTransaction is handled differently in WebResourceHandler:

public class WebResourceHandler implements InjectionHandler
{
 // ...
 loadXmlResourceEnvRefs(InjectionContainer container, Collection refs)
 {

 else if (resType.equals(UserTransaction.class))
 {

 if(envRef.getInjectionTargets() != null)
 {
 createInjectors(container.getInjectors(), getClassloader(), factory, getInjectionTargets());
 continue;
 }
 // ....
}

// Where the container.getInjectors() is:

// EncInjectors/Handlers may need to add extra instance injectors
public List<Injector> getInjectors()
{
 return new ArrayList<Injector>(); // no equivalent in WAR
}

Assuming that the processAnnotations should be like that, we would need to add those injectors also to the EncInjections and so smth like:

 createInjectors(container.getEncInjections(), getClassloader(), factory, getInjectionTargets());

I think adding it to a List would not make sense in this case (same for the WebServiceHandler).

That should be it, so that we just process annotations once (still ignoring dynamic beans). Does that makes sense ? :)

[starksm64]

"emuckenhuber" wrote:

Hmm that's because of a request of the WS team, when we added the @WebServiceRef processing - i can talk to them again and see if we can fix that ?

I don't know that its wrong, its just inconsistent with other reference usage. It should be documented so one understands that doing a lookup against the ref name it going to fail. How this name is used is the question.

[starksm64]

"emuckenhuber" wrote:

which would makes sense, as we don't use the resolvedClassInjections anymore (see processClass) and encInjections is actually already populated by the InjectionHandlers.

But e.g. the UserTransaction is handled differently in WebResourceHandler:

public class WebResourceHandler implements InjectionHandler
{
 // ...
 loadXmlResourceEnvRefs(InjectionContainer container, Collection refs)
 {

 else if (resType.equals(UserTransaction.class))
 {

 if(envRef.getInjectionTargets() != null)
 {
 createInjectors(container.getInjectors(), getClassloader(), factory, getInjectionTargets());
 continue;
 }
 // ....
}

The encInjectors is setup as a side-effect of running the handlers in processMetadata:

 public void processMetadata()
 {
 // XML must be done first so that any annotation overrides are initialized

 // todo injection handlers should be pluggable from XML
 handlers = new ArrayList<InjectionHandler<Environment>>();
 handlers.add(new WebEJBInjectionHandler<Environment>());
 handlers.add(new DependsHandler<Environment>());
 handlers.add(new PersistenceContextHandler<Environment>());
 handlers.add(new PersistenceUnitHandler<Environment>());
 handlers.add(new WebResourceHandler<Environment>());
 handlers.add(new WebServiceRefHandler<Environment>());

If we don't need that anymore, this is where they could be setup.

"emuckenhuber" wrote:

Assuming that the processAnnotations should be like that, we would need to add those injectors also to the EncInjections and so smth like:

 createInjectors(container.getEncInjections(), getClassloader(), factory, getInjectionTargets());

I think adding it to a List<Injectors> would not make sense in this case (same for the WebServiceHandler).

That should be it, so that we just process annotations once (still ignoring dynamic beans). Does that makes sense ? :)

If it works, yes ;).

Cleanup the TomcatInjectionContainer as much as possible, minimizing reliance on the old ejb3 injection code as we will eventually move this to the mc based injection component factory project:
https://svn.jboss.org/repos/jbossas/projects/component-factory/trunk/

Right now were just focused on getting the tck passing.

[emuckenhuber]

If it works, yes ;).

I spent some time testing that and it looks quite ok. Although i had to fight a bit with the WebServiceRefHandler,
as the loadXml does not really do everything which is needed. Therefore i did a simple one for web processing,
because i need to put the injectors to this EncInjections map i mentioned above.

Furthermore i reuse the correct PersistenceUnitResolver, which does not seem to work properly, but at least it's the right one ;)

So then i have one remaining question, because you said:

The only annotation scanning in the web container would be dynamic elements not available via metadata such as jsp beans, etc.

What is etc. ? :)
I mean how can we determine dynamic beans? because i'm afraid that doing something like:

if (injectors == null && className.endsWith("_jsp"))
{
 // process the dynamic bean annotations
}

is not enough then. I saw that jsf injection is done somewhere else, so what else do we need to consider there ?

[starksm64]

dynamic meaning anything org.apache.InstanceManager passes in other than the web-app.xml components. If jsf does not flow through this than that's a problem. There is a jsf bean issue that would indicate these beans are being passed through:
https://jira.jboss.org/jira/browse/JBAS-4399

[emuckenhuber]

dynamic meaning anything org.apache.InstanceManager passes in other than the web-app.xml components. If jsf does not flow through this than that's a problem. There is a jsf bean issue that would indicate these beans are being passed through:
https://jira.jboss.org/jira/browse/JBAS-4399

Hmm well i briefly ran a jsf test, but could not see that the TomcatInjectionContainer was called for jsf beans.
There is also org.jboss.web.jsf.integration.injection.JBossInjectionProvider.java which seems to handle the injection for managed beans, but i haven't really looked at that.

[starksm64]

JBossInjectionProvider must be what is being called then. That's awkward to have to seperate callbacks for the same functionality. The JBossInjectionProvider should just be delegating to the TomcatInjectionContainer to avoid duplicate code.

[emuckenhuber]

dynamic meaning anything org.apache.InstanceManager passes in other than the web-app.xml components.

Hmm okay then i had a something different in mind.
Looking at that a different way - i was considering a dynamic bean everything which the AnnotationDeployer can't pick up :)

Which basically means we would be fine for jsf managed beans - or do i miss something? I mean beside that the TomcatInjectionContainer is not called.
Therefore my question was - is there something beside jsp we need to process?

[starksm64]

According to the jsp2.1 spec there are:

"jsp-2.1" wrote:

In the JSP specification, tag handlers which implement interfacesjavax.serv-
let.jsp.tagext.Tag andjavax.servlet.jsp.tagext.SimpleTag may be annotated for injection. In both cases, injection occurs immediately after an instance of the tag handler is constructed, and before any of the tag properties are initialized.
Event Listeners (See Section JSP.7.1.9, Event Listeners) can also be
annotated for resource injection. Injection occurs immediately after an instance of the event handler is constructed, and before it is registered.
The annotations supported are:
- @EJB, @EJBs
- @PersistenceContext, @PersistenceContexts
- @PersistenceUnit, @PersistenceUnits
- @PostConstruct, @PreDestroy
- @Resource,@Resources
- @WebServiceRef, @WebServiceRefs

I assigned JBAS-4399 to you to ensure the annotation processing is happening in one place.