Bug 828464

len or pavel .... comments on this?  can you repro?

Also .. Connection properties for 'JBoss ESB' subystem are incorrectly detected:
'JBoss Home Directory' and 'Configuration Path' are empty and marked as incorrect.

Created attachment 589516 [details]
JBoss ESB Connection settings error

Reproduced. JON 3.1.0.CR1 and SOA-P 5.3.0.ER3.

JBoss ESB Connection settings error
Manual ESB deployment with JON GUI failed.
I did not experienced java.lang.NullPointerException.

Created attachment 589529 [details]
JBoss ESB > Deployments > Connection settings error

I've just reproduced it on JON 3.1.CR2

Here's what's failing : 
                          Class remoteIClazz = classLoader.loadClass("org.rhq.plugins.jbossas5.deploy.PackageDownloader");

                          Class remoteDClazz = classLoader.loadClass("org.rhq.plugins.jbossas5.deploy.RemoteDownloader");
                          Constructor remoteDConstructor = remoteDClazz.getDeclaredConstructor(ResourceContext.class, boolean.class, ProfileServiceConnection.class);

                          Class clazz = classLoader.loadClass("org.rhq.plugins.jbossas5.deploy.ManagedComponentDeployer");
                          Constructor constructor = clazz.getDeclaredConstructor(ProfileServiceConnection.class, remoteIClazz);

In JON 3.0, the jbossas5 RemoteDeployer class was removed, which made us have to  try to load & execute its replacement (RemoteDownloader/ManagedComponentDeployer) because we need to maintain compatibility with the AS5 admin-console.   In JON 3.1 we're failing on the getDeclaredConstructor the ManagedComponentDeployer constructor.      My guess is that that class has changed between JON 3.0 and JON 3.1 and that is what is breaking us.

I'll take a look at what's changed there and see if we can patch the ESB plugin to go along with it.

Created a two argument constructor for backwards compatibility. The extra argument is not needed for the way the ESB plugin uses the content system code from the AS5 plugin. The extra constructor was marked as deprecated and is not used anywhere else in the RHQ code.

release/jon3.1.x branch commit:

http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commit;h=88560f71766cfa3e3c55150e7e485826df37946b

As per Stefan's comment this should be fixed in JON31 CR3

Moving to ON_QA since the CR3 build is available.

verified on JON 3.1.CR3

just documenting the root cause analysis on this BZ ...


Root Cause Analysis 

 BZ 828191 and 828464


Scope:
BRMS autodiscovery - https://bugzilla.redhat.com/show_bug.cgi?id=828191
ESB autodiscovery - https://bugzilla.redhat.com/show_bug.cgi?id=828464


GOALS:
Prevent regressions like this from even happening
Earlier detection

Details on the nature of these issues (provided by Tom Cunningham)

From Tom Cunningham
I've found the root cause of the first one (more details in bugzilla) - 
In JON 3.0, the jbossas5 RemoteDeployer class was removed, which made us 
have to try to load & execute its replacement in the event we got a 
ClassNotFoundException on RemoteDeployer.      We need to maintain 
compatibility with the AS5 admin-console, and we actually compile 
against the admin-console in our build, so we needed to insert some 
messy loadClass/getDeclaredConstructor/invoke stuff in the case of JON 3.

In JON 3.1, this is now failing :

                           Class clazz = 
classLoader.loadClass("org.rhq.plugins.jbossas5.deploy.ManagedComponentDeployer");
                          Constructor constructor = 
clazz.getDeclaredConstructor(ProfileServiceConnection.class, remoteIClazz);

my guess is that the method signature changed.     Charles - are there 
additional args we have to pass in?

The SOA-P plugin is extends the jbossas5 plugin.  The jbossas5 plugin changed (unexpectedly) which caused breakage in the SOA-P plugin.  




Details on the how these regressions occurred
(let's ask "why?" 5 5 times to understand the root cause)

 Why did the jbossas5 plugin change?  a deprecated method was removed  https://bugzilla.redhat.com/show_bug.cgi?id=828464  ( Created a two argument constructor for backwards compatibility. The extra argument is not needed for the way the ESB plugin uses the content system code from the AS5 plugin. The extra constructor was marked as deprecated and is not used anywhere else in the RHQ code.)   
why? The plugin was changed to address an issue found by Eiichi Nagai when QE'ing https://bugzilla.redhat.com/show_bug.cgi?id=646631
why? The change required a new parameter to be passed to one of the constructors in the jbossas5 plugin, adding this new parameter caused the issue. plugin is calling a non-public API
why?  no specific indicator that would prevent a plugin developer from making calls not to the public plugin API ... but to the internals 
why? plugin can extend other plugins see line #28  
why? trying to create a plugin that uses JON and EAP5 embedded console   ???? supposition





Details on why these regressions were not detected sooner

From Libor:  by accident, looking at another issue.  found this one.  

From Pavel:  https://bugzilla.redhat.com/show_bug.cgi?id=821904
    couldn't detect is sooner because plugin pack was broken ER4
it could only be detected after JON ER4  (bug was introduced in ER4)
it was a late regression that was detected quickly (having Brew builds earlier is NOT a valid countermeasure for this specific issue)


It was later in the dev cycle because the problem with https://bugzilla.redhat.com/show_bug.cgi?id=646631 was only discovered on 5/14. The fix was then done on 5/23.
ESB plugin is not built in the JON test suite  (clarifying question:  would that classz stuff have thrown a compile time error if the ESB plugin were built in the JON test suite? that looks to me like it would only get caught at runtime, not compile)  
The SOA-P/EDS regression automation deploys ESB archives by ANT and there was not covered a JON GUI manual deployment for the archives. The case will be added:https://issues.jboss.org/browse/JBQA-6484 
For the BRMS case there is a Drools application in GIT that is compiled with various BRMS deployable ZIP files, eg. http://jawa05.englab.brq.redhat.com/released/BRMS-5.2.0.GA/brms-p-5.2.0.GA-deployable.zip There was a problem with compilation and running the application that was solved with BRMS 5.3.0.ER6. Task: https://issues.jboss.org/browse/JBQA-6485
Information about new builds of JON 3.1.0 get a couple of days later after the release. I was testing ER4 build and there was ER5/ER6 already. I joined jboss-on mailing list to fix the problem.
why?  

why? 
why?




Possible Countermeasures to prevent these regressions and detect issues sooner


Document that describes the write way to create a plugin that is not susceptible to this type of breakage....+1+1 (this seems like an easy thing to do)
already exists
doesn't include comments "Do not make classloader statements to load explicit classes in the implementation (which could change)"
test automation that detected this failure pattern
already Plugin verifier that could be augmented - related to line 86
static analysis that explicitly looks for patterns like this ... such as FindBugs
write a plugin Check Tool that could be distributed to other plugin teams +1+1+1
If you want earlier warnings on things like this, we can change the ESB plugin to compile against current RHQ & jbossas5 and you can compile against your nightlies - which would catch an error like this.  (    However, since ESB & the ESB plugin won't be a part of SOA-P 6 (SwitchYard replaces it), I'm not sure how much effort it'll be worth.      
Search the ESB/BRMS plugins when obsoleting methods in jbossas5 plugin  (why are we not using the public API?  why are we referncing classes in the implementation?) -1 (we can't search through every possible plugin)
Adding the automation test casses to cover all variations of deployment ESB/VDB archives.
Integrate the BRMS/SOA-P/EDS integration reggression testing automation into JON team automation to: http://git.engineering.redhat.com/?p=users/jweiss/automatjon.git;a=summary similar to EAP6/7.   Sahi tests running in the JON QE integration test suite +1+1+1
Pavel ... is willing to do this ... needs advice on where to put his sources 
The SOA-P/EDS regression automation deploys ESB archives by ANT and there was not covered a JON GUI manual deployment for the archives. The case will be added:https://issues.jboss.org/browse/JBQA-6484 +1+1 <<<this seems like an obvious thing
:https://issues.jboss.org/browse/JBQA-6484 
https://issues.jboss.org/browse/JBQA-6485
Branching SOA plugin one for EAP and one for JON would allow both to be optimized which should enable some of the funky classloading to be avoided +1
JON team adding API change detection (e.g. animal-sniffer) to its build to help detect potentially breaking changes such as this +1
Ensure future plugin APIs are more locked down 
Have JON team review other teams plugins for usage of best practices (+1 related to line 80)