CDIEJBManagedObjectFactoryImpl.getEjbDescriptor execution flow is confusing

[Azquelt]

There have been several changes to the code which looks up the ejbDescriptor for a named EJB to accomodate different situations which were initially overlooked.

However, the current code is a mess and it's very difficult to verify that it does the correct thing.

Highlights

The first check discards the ejbDescriptor if there's no bean, or it's the wrong type

            if (bean == null || ! bean.getBeanClass().equals(getManagedObjectClass())) {

However later checks only null out ejbDescriptor if if there is a bean and it has the wrong type:

                    if (bean != null && ! bean.getBeanClass().equals(getManagedObjectClass())) {
                        partialMatchDescriptors.add(ejbDescriptor);
                        ejbDescriptor = null;
                        bean = null;
                    }

However, this doesn't matter too much since there's also no break out if we find a matching ejbDescriptor, only if we find a matching bean, which means we can find an ejbDescriptor without a valid bean and then we'll immediately discard it when we check the next bda.

It also means we can drop out of the search section with either ejbDescriptor and bean being null, just bean being null or neither being null and that seems to be solely dependant on what the last BDA scanned was.

This makes the next check rather pointless

                if (ejbDescriptor == null) {

since the value of ejbDescriptor is not the matching one that we found, it's merely the result of looking for the ejbDescriptor in the last BDA searched.

The guarantees we do seem to have when exiting the search loop are:

• that firstDescriptor will be set if we found an ejbDescriptor (though it might not have a bean)
• if we find a bean, ejbDescriptor and bean will both be set

We trace out partialMatchDescriptors, but that will only contain ejbDescriptors which had a bean which was the wrong type. I don't see why we wouldn't want to also trace out ejbDescriptors which didn't have a bean.

Suggested actions

• Refactor the doInitialization method so that the search logic is encapsulated in a static method which can be unit tested
• Go through the history of this class and look at the past issues to understand what problems we were trying to solve with this mess
• Rewrite the search logic so that it's clear what's happening at each stage

[Azquelt]

Relevant changes appear to be:

• 15631

• 17439

[benjamin-confino]

Continue iterating if the ejbDescripter fetches the wrong bean

Before this patch we'd return the first ejbDiscriptor that had the correct EJB name. However it turned out that you could have multiple ejbDiscriptors with the same name and there was no guarantee the first one we found is the correct one.

So the update makes it check every ejbDiscriptor it finds to see if it produces the correct bean and retain the one that does. With a fallback to the old behavior where it keeps the first one it finds.

set ejb descriptor to the first one found if bean is null aka an MBD

This one is simple. Message Driven Beans are a special edge case where you want to return an ejbDiscriptor and null for the bean itself.