Bug 230256 - [hotbug] jst.jee JEEDeployableFactory refuses to load ejb3 module
Summary: [hotbug] jst.jee JEEDeployableFactory refuses to load ejb3 module
Status: RESOLVED FIXED
Alias: None
Product: WTP Java EE Tools
Classification: WebTools
Component: jst.j2ee (show other bugs)
Version: 2.0.2   Edit
Hardware: PC Linux
: P3 major (vote)
Target Milestone: 2.0.3   Edit
Assignee: Carl Anderson CLA
QA Contact: Chuck Bridgham CLA
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2008-05-05 12:59 EDT by Rob Stryker CLA
Modified: 2008-07-18 14:21 EDT (History)
6 users (show)

See Also:


Attachments
Example Project (743.05 KB, application/octet-stream)
2008-05-05 12:59 EDT, Rob Stryker CLA
no flags Details
Patch to make JEEDeployableFactory more permissive (4.47 KB, application/octet-stream)
2008-05-05 13:02 EDT, Rob Stryker CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Stryker CLA 2008-05-05 12:59:06 EDT
Created attachment 98674 [details]
Example Project

Basically... I'm trying to include a binary ejb3 jar in an ear project. Because it is ejb3, this file has no descriptor. 

JEEDeployableFactory crashes with a NPE trying to include it. The same is true of similar jee projects without descriptors. I'll be attaching an example project and a patch. 

In short... it tries to load a descriptor from the file, returns a null Archive, and then throws a NullPointerException.
Comment 1 Rob Stryker CLA 2008-05-05 13:02:54 EDT
Created attachment 98675 [details]
Patch to make JEEDeployableFactory more permissive

This patch is to make JEEDeployableFactory more permissive, even in place of errors.  This is necessary because currently the entire stack gets blown away by NPE's and no child modules get saved. None. 

Another line of patch that *could*, and dare I say *should* be required is that JEEDeployableFactory.getListenerPaths() should add the project's application.xml file to the list of actionable paths.
Comment 2 Rob Stryker CLA 2008-05-05 13:04:57 EDT
This is a bug who's severity I feel requires a new patch release. 
Comment 3 Rob Stryker CLA 2008-05-05 13:11:39 EDT
this issue is related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=222531

The patch is a bit of a hack, and could probably done better, however the real problem stems from the fact that it is demanding a descriptor to verify this file and to get the module version.

I'm quite aware that assigning a hard-coded module version probably isn't the best idea, but short of digging deep into the EMF-related portions of this code it's the best I can do. 

Also of note, my patch is against the 2.0.2 patches stream. 
Comment 4 Rob Stryker CLA 2008-05-06 01:33:40 EDT
Another stack trace from the same file caused in the same example (occasionally, randomly):



*** ERROR ***: Tue May 06 01:02:19 EDT 2008    java.lang.ClassCastException: org.eclipse.jst.j2ee.internal.componentcore.UtilityBinaryComponentHelper$ReferenceCountedArchiveImpl cannot be cast to org.eclipse.jst.j2ee.commonarchivecore.internal.EJBJarFile
        at org.eclipse.jst.j2ee.internal.componentcore.EJBBinaryComponentHelper.getPrimaryRootObject(EJBBinaryComponentHelper.java:109)
        at org.eclipse.wst.common.componentcore.ArtifactEdit.getContentModelRoot(ArtifactEdit.java:540)
        at org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory.createBinaryModules(JEEDeployableFactory.java:147)


I suspect strongly there is a race condition somewhere under there. I honestly have no idea how this getPrimaryRoot() method can throw both NPE's and CCE's... 

Either way, I'd adjust the patch appropriately.

Again, I really really wish I could patch this more appropriately, in an actually beneficial way, but instead my patch is basically triage for a dead stream of wtp with no more planned maintenance builds. 
Comment 5 Max Rydahl Andersen CLA 2008-05-09 05:38:13 EDT
FYI, we regret having to do this but we started telling users to use our custom patch to get reliable JEE jar deployment.

You can see our latest release announcement and notes explaining this at http://in.relation.to/8932.lace
Comment 6 David Williams CLA 2008-05-09 07:15:42 EDT
(In reply to comment #5)
> FYI, we regret having to do this but we started telling users to use our custom
> patch to get reliable JEE jar deployment.
> 

Thanks for letting us know. It was a fair writeup. 
Apologies for the delays ... too few for too much. 

And ... you know, it's not that unusual for adopters to provide their own patches for open source software ... since it is, after all, open source. 

But we will continue to work with you as we get someone freed up from 3.0 crunch to take a look at this and assess it and once it's understood we'll decide if/when we should do a 2.0.3 release (as you requested) or just do another patch. 

Thanks, 


Comment 7 Max Rydahl Andersen CLA 2008-05-09 18:19:41 EDT
The problem is that for JBDS i'm fine since I here control the environment and can maintain the patches, but for JBoss Tools it becomes problematic since the intention is to just run on top of vanilla eclipse....having to tell users to use this specific patch as an addition to vanilla eclipse scares users ,(

But yes I hope for a 2.0.3 or simply a better patch...we can't help much since the issue occurs in the middle of EMF land where we don't fight well.
Comment 8 Carl Anderson CLA 2008-05-29 12:31:16 EDT
After much discussion, we have decided to accept this as a hotbug.  However, while the attached patch may be adequate for the direct needs of the JBoss products, it also introduces behavior that is undesirable, even if it is only in a patched stream of 2.0.2.  

Our two main concerns are: while binary archive support is properly working in 3.0, technically the 3.0 behavior could be termed as a regression since it would not default to Java EE 5 if there was a problem loading the descriptor.
Second: this change in behavior could cause regressions/problems with other adopters.

I am adding in the helpwanted keyword because, while I am taking ownership of this bug, I do not have the time to work on this.  I will help you get this through the process and into the 2.0.2 patches stream, but improving the patch and testing/addressing regressions is something that I do not have the time to do.
Comment 9 Max Rydahl Andersen CLA 2008-05-30 00:37:46 EDT
What is the expected behavior for unrecognized binaries supposed to be in 3.x ?

Comment 10 Jason Sholl CLA 2008-06-19 11:40:17 EDT
We have fixed these problems in the WTP 3.0 stream; it was actually quite of bit of new code.  You may have a look at how we handle it there.  As far as the patch goes; it definitely seems like a hack, but any fix for this problem in 2.x is likely going to be a hack because the full binary support for EE 5 projects is implemented in WTP 3.0.  So, if we decide to go forward with this in 2.x we must not put these same fixes in 3.x.

To answer the question in comment 9, the way we handle determining types in 3.0 is as follows:

1. If a module linked to an EAR via an EAR's DD, then use the type defined regardless of what is actually in the module.  (i.e. if the EAR says it is a EJB, and the module has an applicationclient.xml we're gonna treat it as an EJB).
2. If the module is not linked to an EAR via an EAR's DD, the do the following:
2a. If there is a DD, we use it; I forget the exact order we check in (look at JavaEEARchiveUtilities in the 3.0 stream), but if a jar has multipe DDs (e.g. an ejb-jar.xml and an applicationclient.xml, then one of them always wins)
2b. If there are no DDs, we check the manifest for a main class; if it has one then it is an application client.
2c. Now class files are checked for ejb annotations; if there are any, then it is an EJB.
2d. Failing all the above tests, the module is considered a utility.
Comment 11 Rob Stryker CLA 2008-06-19 11:58:21 EDT
Jason:

Thank you for that very very informative post. 

Yes... this patch will not apply at all to wtp 3.0

Thanks again!
Comment 12 Rob Stryker CLA 2008-06-25 18:41:27 EDT
Jason:

Do you think this patch should make it into a 2.0.3 release or not?

Carl mentioned that this could cause regressions, but after reviewing the patch and testing it, I don't see what regressions it could cause. 

The basic situation is that JEE5 is not supported in the 2.0.x stream and a JEE5 jar causes all sorts of problems, NPE's, and incomplete deployments. 

So the question is, should this hack be eligible to make it into the release? If not, any suggestions on how to fix it to ensure all files in a project are published properly?

Thanks again. 
Comment 13 David Williams CLA 2008-06-26 12:33:24 EDT
I've chatted with Jason and unless there's objections from other adopters, we feel ok about putting this patch in 2.0.3, even though sort of a hack ... that is, seems to fix immediate problem Rob was having, and as far as we can tell, won't cause problems for others ... but, hard for us to say, so hope all interested parties can test well! 

Comment 14 David Williams CLA 2008-07-18 14:21:36 EDT
Fixed in 2.0.3