Bug 222531 - Regression (?) between wtp 2.0.1 and 2.0.2 in J2EEFlexProjDeployable
Summary: Regression (?) between wtp 2.0.1 and 2.0.2 in J2EEFlexProjDeployable
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 minor with 3 votes (vote)
Target Milestone: 3.0   Edit
Assignee: Carl Anderson CLA
QA Contact: Chuck Bridgham CLA
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-03-13 05:04 EDT by Rob Stryker CLA
Modified: 2008-05-09 05:39 EDT (History)
13 users (show)

See Also:


Attachments
A project that demonstrates the change in behavior (100.00 KB, application/x-tar)
2008-03-13 05:04 EDT, Rob Stryker CLA
no flags Details
A new example with an embedded WAR produced elsewhere (10.00 KB, application/octet-stream)
2008-03-17 16:06 EDT, Rob Stryker CLA
no flags Details
Patch on org.eclipse.jst.jee project (990 bytes, patch)
2008-03-18 16:26 EDT, Rob Stryker CLA
bjorn.freeman-benson: iplog+
Details | Diff
1.4 EAR with a jar'd 2.5 WAR inside it which fails to publish (10.00 KB, application/x-tar)
2008-03-19 16:03 EDT, Rob Stryker CLA
no flags Details
5.0 EAR with a child jar mentioned as a module (100.00 KB, application/x-tar)
2008-03-31 14:22 EDT, Rob Stryker CLA
no flags Details
An example with a jar added via modules tag (1.04 MB, application/x-tar)
2008-04-10 19:35 EDT, Rob Stryker CLA
no flags Details
An example with an ejb jar mistaken as an application client (860.00 KB, application/x-tar)
2008-04-11 21:32 EDT, Rob Stryker CLA
no flags Details
Test case with maven project (demonstrating comment 48) (43.14 KB, application/zip)
2008-04-28 16:05 EDT, Siarhei Dudzin CLA
no flags Details
updated test with 100% spec compliant ear (1.48 MB, application/zip)
2008-04-30 15:18 EDT, Max Rydahl Andersen 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-03-13 05:04:26 EDT
Created attachment 92408 [details]
A project that demonstrates the change in behavior

Between the two most recent maintenance releases of WTP, (2.0.1 and 2.0.2) there is a substantial difference in how resources inside an Ear project are treated when converting that project into a deployable module. 

Specifically, jar files sitting inside the EarContent folder (AND registered in the application.xml) *were* being returned in the IModuleResource[] array in wtp 2.0.1 and are NOT being returned as IModuleResource[] array  (within J2EEFlexProjDeployable.members())

This is a significant change and may prevent us from using the updated patch driver without further instruction on how to properly ensure the EAR project is deployed as we expect it to be. Project attached demonstrates can be used as a test project. 

The expected IModuleResource tree we expect to see is as follows:

(root)
|-antlr-runtime.jar
|-META-INF
   |- application.xml
   |- jboss-app.xml

That is, two members at the top, with two members inside META-INF. The two inner members (*.xml) appear in both wtp 2.0.1 and wtp 2.0.2.   The antlr-runtime.jar *only* appears in a wtp-2.0.1 workspace.
Comment 1 Max Rydahl Andersen CLA 2008-03-13 05:11:38 EDT
The breaking change is most likely from the patch in Bug 204537
Comment 2 Max Rydahl Andersen CLA 2008-03-13 05:14:38 EDT
Note: at first we thought it might be an issue with our server adapter but this behavior is actually also seen for the builtin WTP server adapters so it is a general issue.
Comment 3 Rob Frost CLA 2008-03-13 12:55:56 EDT
Carl just pointed me to this: the patch for the referenced bug primarily impacted JEEFlexProjDeployable rather than J2EEFlexProjDeployable (some private fields and methods in J2EEFlexProjDeployable were made protected); the change there was to make the JEE* version subclass the J2EE version and eliminate duplicate code.

Looks like this app is a 5.0 EAR which may be why you believe this change is having this impact; the cause here may very well not be that particular patch but other changes to J2EEFlexProjDeployable (note: I just a simple test against WLS for that configuration and the antlr-runtime.jar under EarContent was correctly handled by the server adapter (haven't debugged things to verify what is actually being returned by members() in this case).
Comment 4 Rob Frost CLA 2008-03-13 14:21:39 EDT
Debugging into things...

Not seeing it returned from members() but it is returned from J2EEFlexProjDeployable.getReferences() as a non-consumable IVirtualReference, however, ComponentDeployable.getModules() is not returning it as an IModule because ServerUtil.getModule() (called from JEEFlexProjDeployable.gatherModuleReference()) is not returning an IModule (the WLS server adapter ends up grabbing it due to separate logic that processes the IVirtualReferences).

I believe it was getting returned from members() before because of JEEFlexProjDeployable.addUtilMember(). Prior to revision 1.5.2.1, that had implementation:

...
protected boolean shouldIncludeUtilityComponent(IVirtualComponent virtualComp, IVirtualReference[] components, ArtifactEdit edit) {
		return virtualComp != null && virtualComp.isBinary() && virtualComp.getProject()==component.getProject();
	}
...

It now has this implementation (this is not the signature that was in the patch, Carl: looks like you may have made that modification locally, and is actually not being called since addUtilMember() calls the version that takes EARArtifactEdit - the version that is called is functionally equivalent):

...
protected boolean shouldIncludeUtilityComponent(IVirtualComponent virtualComp,IVirtualReference[] references, IEARModelProvider model) {
		// If the component module is an EAR we know all archives are filtered out of virtual component members
		// and we will return only those archives which are not binary J2EE modules in the EAR DD.  These J2EE modules will
		// be returned by getChildModules()
		if (J2EEProjectUtilities.isEARProject(component.getProject()))
			return virtualComp != null && virtualComp.isBinary() && !isNestedJ2EEModule(virtualComp, references, model);
		else 
			return super.shouldIncludeUtilityComponent(virtualComp, references, null);
	}
...

So, this is now returning false for this reference (and this looks correct since, for this app client component, isNestedJ2EEModule() should be returning true). 

Per the comment "we will return only those archives which are not binary J2EE modules in the EAR DD.  These J2EE modules will be returned by getChildModules()", it sounds like this is working as designed and the server adapter needs to access this module as a reference rather than part of the IModuleResource tree.

Carl/Chuck: I'll let you comment here as well
Comment 5 Rob Stryker CLA 2008-03-13 21:58:44 EDT
// If the component module is an EAR we know all archives are filtered out of virtual component members
// and we will return only those archives which are not binary J2EE modules in the EAR DD.  These J2EE modules will
// be returned by getChildModules()

This presents a problem for me. And for others I'm sure, though no one's quite complained yet, judging by a search of the bugs.

First and foremost is that I've just tried deploying my project (again) on both my own custom server adapter, as well as the jboss generic server adapter using wtp 2.0.2. Both of them have the same file structure of the result, and NEITHER of them have antlr-runtime.jar file included in the result.

So this change has clearly broken not just my own adapter, but also the generic server adapter. For this reason I'm CCing Gorkem Ercan <gercan@acm.org>. I think his input may be relevant.  Also since this clearly involes the wst team, I'm cc-ing Tim DeBoer.

I use the same code in my server adapter's  getChildModules() method as does the GenericServer implementation.  The snippet of our shared (or my pilfered ;) ) getChildModules() method in our respective ServerDelegates is:

	if (moduleType != null && "jst.ear".equals(moduleType.getId())) {
		IEnterpriseApplication enterpriseApplication = (IEnterpriseApplication) module[0]
			.loadAdapter(IEnterpriseApplication.class, null);
		if (enterpriseApplication != null) {
			IModule[] earModules = enterpriseApplication.getModules(); 
			if ( earModules != null) {
				return earModules;
			}
		}
	}

I think it's notable that neither I nor Gorkem are using the ModuleDelegate.getChildModules() method, and maybe this is whats causing at least some of the problems?  Either way, this code snippet returns 0 child modules for the given project. So perhaps this interface isn't being adhered to? Or are both Gorkem and I just using it incorrectly?


Rob Frost:  Which server adapter did you test with? Can you point me to its codebase or the ServerDelegate's class I can look at to see how it has implemented its getChildModules() method? 

I suppose a question I have is, why did the changes to JEEFlexProjDeployable decide to switch bundled .jar files as children modules instead of as member resources? And if it was switched to a child module instead of a member resource, why doesn't the snippet above work?
Comment 6 Rob Stryker CLA 2008-03-13 22:14:09 EDT
I've just tried many different things in an effort to locate some reference to the antlr-runtime.jar file. 

I've tried deleting references to them from the application.xml file. This did not change anything and neither my server nor the Generic Server adapters found them either in the IModuleResource members() method or the ServerDelegate's getChildModules() method.

I also tried modifying the code in my server's getChildModules() method to include the following:

	public IModule[] getChildModules(IModule[] module) {
		int last = module.length-1;
		ModuleDelegate delegate = (ModuleDelegate)module[last].loadAdapter(ModuleDelegate.class, null);
		IModule[] mods = delegate.getChildModules();

            etc...
        }

In this case, mods was *still* empty when the IModule[] passed in was a JEEFlexProjDeployable for the EAR project (S1-ear )

I am at a loss for any way to locate the antlr-runtime.jar file via any of the approved APIs. 
Comment 7 Rob Frost CLA 2008-03-14 10:21:49 EDT
Regarding the server adapter that I tested with: the downloadable WebLogic Server adapter.

Question: as far as I can tell, this change only impacted this behavior of the shouldIncludeUtilityComponent() method for 5.0 EARs; had things been working for 1.4 EARs in this context for you before?

I'll have to let Carl and Chuck answer your questions on: "I suppose a question I have is, why did the changes to JEEFlexProjDeployable
decide to switch bundled .jar files as children modules instead of as member
resources? And if it was switched to a child module instead of a member
resource, why doesn't the snippet above work?".

Regarding the last part of that question (the getChildModules() and getModules()), here are some observations:

-ComponentDeployables.getModules() was failing to return this app client JAR because this call in gatherModuleReferences() was returning null: ServerUtil.getModule(JEEDeployableFactory.ID+":"+targetComponent.getName());. So, perhaps there were some changes in org.eclipse.wst.server that may have impacted this scenario; Tim?

NOTE: I just ran a test with the reference to antlr-runtime.jar removed from application.xml and it WAS returned as a ModuleFile under the EAR root from J2EEFlexProjDeployable.members() (which makes since given the implementation of shouldIncludeUtilityComponent()). So, not clear why the tests you ran did not work with that change... 

Comment 8 Rob Stryker CLA 2008-03-14 19:18:23 EDT
I've retested and you're right. When removing from application.xml the module deploys correctly.

The only question for me now is how to verify with certainty that the child modules are found in the case that the user demands or requires (for whatever reason) that the module remain inside the application.xml, since the snippets above don't work. 

Any help from Carl / Chuck / Rob is much appreciated. I'll be continuing to test. =] 

Thanks all in advance. 
Comment 9 Rob Stryker CLA 2008-03-14 19:36:08 EDT
Component deployable refuses to acknowledge this jar because it is not another project.

    protected IModule gatherModuleReference(IVirtualComponent component, IVirtualComponent targetComponent ) {
    	// Handle workspace project module components
		if (targetComponent != null && targetComponent.getProject()!=component.getProject()) {



The idea behind a Java utility project is that it must be a different project in its own right. Therefore ComponentDeployable will not regard this as a dependent module.

It seems to me like J2EEFlexProjDeployable.gatherModuleReference must either check for a null in the superclass's implementation, or return binary jars in the members() method *regardless* of whether they're listed in the application.xml or not. 

The first option doesn't seem that reasonable to me. There's no way / reason for WTP to create a child module out of a jar which is inside the module's project. These child-modules seem to be reserved for *separate* projects. 

I still say that the jars inside the EAR project should *always* be returned by members() and *not* getChildModules(). 
Comment 10 Rob Stryker CLA 2008-03-14 19:40:09 EDT
Also, under no circumstances should a server adapter have to do anything "clever" or out of the ordinary to figure out what child modules a particular module has.  It shouldn't have to browse the component model *at all*, 

*Everything* should be accessible via the wst.server API's in IModule and ModuleDelegate... including members() and getChildModules().   I'll be looking into providing a patch...     and trying to locate / browse the source for the WebLogic adapter. 
Comment 11 Donatas Ciuksys CLA 2008-03-17 09:50:04 EDT
I have stumbled upon this "bug" too. As soon as I read proposed workaround, I thought: "This cannot be true. Libraries must not be placed as modules".
Putting libraries as modules into application.xml breaks Java EE specification compatibility.
Java EE 5 specification (page 152) defines four module types: web, ejb, application client and resource adapter. Libraries, commonly used in ejb and/or web modules may be placed in ear file, but they *are not* modules.

Libraries should be dealt this way (Java EE 5 specification page 167):

...
c. A directory named lib is considered to be the library directory, as described in Section EE.8.2.1, “Bundled Libraries.”
d.For all files in the application package with a filename extension of .jar, but not contained in the lib directory, do the following:
i. If the JAR file contains a META-INF/MANIFEST.MF file with a Main-Class attribute, or contains a META-INF/application-client.xml file, consider the JAR file to be an application client module.
ii. If the JAR file contains a META-INF/ejb-jar.xml file, or contains any class with an EJB component annotation (Stateless, etc.), consider the JAR file to be an EJB module.
iii. All other JAR files are ignored unless referenced by a JAR file discovered above using one of the JAR file reference mechanisms such as the Class-Path header in a manifest file.

If deleting tags "modules" with libraries solves the issue, this means, that bug is not with WTP, but with JBoss Tools implementation. Libraries must not be placed within tags "module".
Comment 12 Max Rydahl Andersen CLA 2008-03-17 12:19:53 EDT
>Libraries should be dealt this way (Java EE 5 specification page 167):
>
>...
>c. A directory named lib is considered to be the library directory, as
>described in Section EE.8.2.1, &#8220;Bundled Libraries.&#8221;

I would love to do this but WTP does not support this afaik ?

>d.For all files in the application package with a filename extension of .jar,
>but not contained in the lib directory, do the following:
>i. If the JAR file contains a META-INF/MANIFEST.MF file with a Main-Class
>attribute, or contains a META-INF/application-client.xml file, consider the JAR
>file to be an application client module.
>ii. If the JAR file contains a META-INF/ejb-jar.xml file, or contains any class
>with an EJB component annotation (Stateless, etc.), consider the JAR file to be
>an EJB module.
>iii. All other JAR files are ignored unless referenced by a JAR file discovered
>above using one of the JAR file reference mechanisms such as the Class-Path
>header in a manifest file.

But this is *not* what WTP have been doing for years and especially in 2.0.1.
It published all the jars that was inside the EAR. 

Our point is that its a breaking change for appservers that has added features compared to JavaEE. 

I agree that we should fix/change our projects that uses this notion of adding things to application.xml which in jboss is a way to add the jar to the classpath of all modules without having them listed in meta-inf.

*But* the point is two-fold:

1) this is something I would call a breaking change in a 2.0.x release since
   it even breaks the generic servers in WTP publishing.

2) it does not seem anyone can point to how we should actually get around deploying this as we could previously by changes to the server adapter ?

>If deleting tags "modules" with libraries solves the issue, this means, that
>bug is not with WTP, but with JBoss Tools implementation. Libraries must not be
>placed within tags "module".

The generic servers worked with this before, they fail too.

My assumption have always been that whatever is in the EAR project would be packaged up - I do not understand the logic behind "if it is listed in application.xml we ignore it" ? No matter if it is within or outside the bounds of JEE spec.

Comment 13 Donatas Ciuksys CLA 2008-03-17 12:31:23 EDT
Well I fully understand WTP: module is something big, something that requires its own project (as you now have for web, ejb modules). If its not module, do not put it into module, there are other means to say this jar is dependency (manifest file, Eclipse own ways, etc.). 
My point is: if seam-gen would not generate faulted (in JEE view) application.xml, project would deploy to JBoss AS successfully even with WTP 2.0.2. 
Question: you have mentioned, that other server adapter stopped working too. If library module tags are removed, are they still not working?
Comment 14 Donatas Ciuksys CLA 2008-03-17 12:35:33 EDT
(In reply to comment #12)
> >c. A directory named lib is considered to be the library directory, as
> >described in Section EE.8.2.1, &#8220;Bundled Libraries.&#8221;
> 
> I would love to do this but WTP does not support this afaik ?

Well you do not have to. Case iii is fully sufficient, you just need to reference those libraries with reference mechanisms (e.g. manifest file):

> >iii. All other JAR files are ignored unless referenced by a JAR file discovered
> >above using one of the JAR file reference mechanisms such as the Class-Path
> >header in a manifest file.
Comment 15 Rob Stryker CLA 2008-03-17 16:06:12 EDT
Created attachment 92742 [details]
A new example with an embedded WAR produced elsewhere
Comment 16 Rob Stryker CLA 2008-03-17 16:06:48 EDT
This is still broken. Even assuming Donatas is correct. 

The reason this is broken is shown in the attached file. The attached file has the following tree format:

S4-ear
`-- EarContent
    |-- META-INF
    |   `-- application.xml
    `-- S4-web.war

To be clear, a WEB ARCHIVE is manually installed into the EarContent folder. It is not dependent on an Dynamic Web Project (or ANY web project). It is compiled elsewhere and bundled into the EAR.  The application.xml duly adds the module tag, as it should. 

Upon publishing, this WAR is *ignored* because it has a .war extension. 	EarVirtualRootFolder.isDynamicComponent(IVirtualFile vFile) assumes that just because it's a virtual file with a child-module extension, it's ok to ignore it. The problem is EVERYTHING is a virtual file, whether it's another project (and thus needs to be handled as a child module instead of a member), or it's an externally-located jar.

This specific use case *definitely* is broken by the change. The change ignores any file with a ".war" or ".jar" from being part of members(), and there is currently no way to get this via childModules. 

This is broken. 
Comment 17 Rob Stryker CLA 2008-03-17 18:05:24 EDT
EarVirtualRootFolder.members(int flags)

	/**
	 * For now, just rip out files with .jar, .rar, or .war file extensions, because these are
	 * the only files automatically added dyamically
	 */

Where are these files added dynamically exactly? 
Comment 18 David Williams CLA 2008-03-17 18:26:11 EDT
Since there's been so many comments in this serious bug, I felt obligated to add this note that the PMC is aware of this issue and the WTP developers are expected to investigate so that clarifications and a decision can be made this week. Just didn't want anyone thinking it was being ignored, or not taken seriously. It is. 

Also, at a "meta level", I'd like to end-up understanding how this happened. 
Was the regression introduced too late? Too late for our adopters to test? Could adopters test earlier so the bug would have been found before we released? Can someone contribute some good (better) JUnit tests in this and related areas? Off hand seems fundamental. 

Comment 19 Rob Stryker CLA 2008-03-17 19:02:43 EDT
(In reply to comment #18)
> Since there's been so many comments in this serious bug, I felt obligated to
> add this note that the PMC is aware of this issue and the WTP developers are
> expected to investigate so that clarifications and a decision can be made this
> week. Just didn't want anyone thinking it was being ignored, or not taken
> seriously. It is. 
> 

That's very good to hear. Thank you.

> Also, at a "meta level", I'd like to end-up understanding how this happened. 
> Was the regression introduced too late? Too late for our adopters to test?
> Could adopters test earlier so the bug would have been found before we
> released? Can someone contribute some good (better) JUnit tests in this and
> related areas? Off hand seems fundamental. 
> 

Honestly, the real fault is that I've just always been using stable / maintainence releases instead of the forward-looking stream. I'm fairly sure now that won't happen again ;)  I intend to begin playing with forward-looking code a lot more often and keeping up with other project-level events (calls, code, tests, etc). 

On a non-meta level, to the PMC and the owners of the code,  Here is a summary / deep dive of what I've discovered.

If for any reason there's a bundled archive that is not its own project (jar, war, etc) inside an EAR (top level), it is not exported 
   a) via the members() function 
or b) via getChildModules()

All resources should be exported via one of the two IMO. 

In the case of a, the archive is skipped *twice*.  First in the actual extension to VirtualFolder.members() called EarVirtualRootFolder.members() which skips anything with a forbidden extension. It is skipped a *second* time later on when 
J2EEFlexProjDeployable.addUtilMembers() calls shouldIncludeUtilityComponent(). The comment in shouldIncludeUtilityComponent() makes sure to mention it's omission. This is, apparently, by design.

	// If the component module is an EAR we know all archives are filtered out of virtual component members
	// and we will return only those archives which are not binary J2EE modules in the EAR DD.  These J2EE modules will
	// be returned by getChildModules()

So clearly the intention here is for children modules to be exported via getChildModules() rather than via members(). However... ... 

In the case of b, calling getChildModules(), J2EEFlexProjDeployable does not properly override its superclass for gatherModuleReference(). It tries ;)  Lord knows it tries. 

J2EEFlexProjDeployable.gatherModuleReference() could be construed as the point of failure. When the superclass returns null, it uses ServerUtil to find a module named:
   org.eclipse.jst.j2ee.server:lib/S4-ear/EarContent/S4-web.war

This also fails because the first thing J2EEDeployableFactory does to locate the module named "lib/S4-ear/EarContent/S4-web.war" is search for a project named "org.eclipse.jst.j2ee.server:lib/S4-ear/EarContent/S4-web.war", which of course doesn't exist.

Stack:
J2EEDeployableFactory(ProjectModuleFactoryDelegate).findModule(String) line: 265	
ModuleFactory.findModule(String, IProgressMonitor) line: 146	
ServerUtil.getModule(String) line: 135	
JEEFlexProjDeployable(J2EEFlexProjDeployable).gatherModuleReference(IVirtualComponent, IVirtualComponent) line: 622	
JEEFlexProjDeployable.gatherModuleReference(IVirtualComponent, IVirtualComponent) line: 116	
JEEFlexProjDeployable(ComponentDeployable).getModules() line: 114	


The factory fails to find a module by that name, the deployable doesn't get any module reference from gatherModuleReference(), and getChildModules() returns empty. 

To the best of my current knowledge, this is the point of failure ;) 

Thank you, again, David, and others, for bearing with us. 
Comment 20 Rob Stryker CLA 2008-03-17 19:05:58 EDT
> This also fails because the first thing J2EEDeployableFactory does to locate
> the module named "lib/S4-ear/EarContent/S4-web.war" is search for a project
> named "org.eclipse.jst.j2ee.server:lib/S4-ear/EarContent/S4-web.war", which of
> course doesn't exist.
> 

Correction:  

 This also fails because the first thing J2EEDeployableFactory does to locate
  the module named "lib/S4-ear/EarContent/S4-web.war" is search for a project
  named "lib/S4-ear/EarContent/S4-web.war", which of
  course doesn't exist.
  
Comment 21 Max Rydahl Andersen CLA 2008-03-17 19:37:47 EDT
Note: I'm at EclipseCon this week so any want to discuss this issue and why I think this is not just related to JBoss but to normal deployment (see Rob's example) and what we can do about it - then ping me at eclipsecon(at)xam.dk which will reach my phone.
Comment 22 Rob Stryker CLA 2008-03-18 15:53:25 EDT
Two projects contain Module Factories with the same ID. 

In org.eclipse.jst.j2ee:
      <moduleFactory
           projects="true" 
           class="org.eclipse.jst.j2ee.internal.deployables.J2EEDeployableFactory"
           id="org.eclipse.jst.j2ee.server">

In org.eclipse.jst.jee:
      <moduleFactory
            projects="true"
            class="org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory"
            id="org.eclipse.jst.jee.server">

  It seems J2EEDeployableFactory is the one being asked to find the module, while JEEDeployableFactory is the one that created it and has a reference to it. 
Comment 23 Rob Stryker CLA 2008-03-18 16:25:51 EDT
erm ignore that. the id's in the plugin.xml clearly are what they're supposed to be. HOWEVER in their respective classes, one is incorrect:

In J2EEDeployableFactory:
	public static final String ID = "org.eclipse.jst.j2ee.server"; 

In JEEDeployableFactory:
	public static final String ID = "org.eclipse.jst.j2ee.server"; 

A patch is attached for completeness (not that you need one since its a one character change...)  The patch fixes the issue of IEnterpriseApplication.getModules() not returning the children modules. 
Comment 24 Rob Stryker CLA 2008-03-18 16:26:56 EDT
Created attachment 92841 [details]
Patch on org.eclipse.jst.jee project
Comment 25 Rob Stryker CLA 2008-03-19 15:55:30 EDT
While the patch above fixes the primary use case I've been speaking about, I'm attaching another zipped project which also fails to publish properly.

I tested using 1.4-faceted Enterprise Application projects to see how they behaved **after the patch**. I tested two cases:  a bundled war file which is 2.5 compliant, and a bundled war file which was 2.4 compliant.

The results:  
  1) The 2.4 WAR was treated properly by getChildModules() and ignored by members()
  2) The 2.5 WAR was ignored by both members() and getChildModules().  

In this case I realize bundling a 2.5 war inside a 1.4 ear is not exactly called for in the spec, but this outlines the main issue in my opinion.  Wouldn't it be prudent for members() to say anything NOT found in a module factory be handled instead by members()?   

In case two above, (also the attached project), the war is ignored by members() because of its extension, but also ignored by the Module Factory because it had an unsupported module version.  

This basically means that any project that includes something inside a modules tag in the application.xml that is not recognized by or approved by the jee/j2ee tooling is ignored completely, rather than being handled by members() by default. 

I guess I find it troubling that some resource clearly inside a project, whether spec-compliant or not, be ignored as if it didn't exist at all. 
Comment 26 Rob Stryker CLA 2008-03-19 16:03:48 EDT
Created attachment 92958 [details]
1.4 EAR with a jar'd 2.5 WAR inside it which fails to publish

1.4 EAR with a jar'd 2.5 WAR inside it which fails to publish. I feel even if such archives are not picked up by a specific module factory, they should still be published in some fashion, whether through members() or getChildModules().  

getChildModules() seems more likely since that seems to be the paradigm mandated through the architecture thus far.
Comment 27 Carl Anderson CLA 2008-03-20 14:37:12 EDT
This bug was an unintended consequence of bug 204537 - we did not catch the ID collision between J2EEDeployableFactory and JEEDeployableFactory.  That fix was committed to HEAD for 3.0.  We will also work to get that fix into a patch for 2.0.2.

However, as for the Servlet 2.5 inside of an EAR 1.4 - this type of support is a line item for WTP 3.0, and was not in prior WTP releases.
Comment 28 David Williams CLA 2008-03-25 03:47:26 EDT
A patch feature has been created as is in patch builds >= P20080325024219. 

It is in the /webtools/testUpdates site, as well. 

We'll move to the official /webtools/updates site once we complete regression testing. 

Comment 29 Max Rydahl Andersen CLA 2008-03-25 10:20:00 EDT
Ok - so what is the timeschedule/process on getting this validated and into an release of WTP ?

p.s. Anyone care to explain me how a string plugin id can affect the behavior of code that lists modules ?

p.p.s why is there these "funny" checks in WTP 2.0.x that makes Rob S.'s example still fail ?
Comment 30 David Williams CLA 2008-03-25 10:38:36 EDT
(In reply to comment #29)
> Ok - so what is the timeschedule/process on getting this validated and into an
> release of WTP ?
> 

validated and into the official patch site this week. So far there is no plans for subsequent releases on the 2.0.x stream. 

> p.s. Anyone care to explain me how a string plugin id can affect the behavior
> of code that lists modules ?
> 

Not sure what you are asking here (about root problem, or asking why we need to regression test?) ... one thing I'm concerned about is that this is a final static string ... often inlined by compilers ... so anyplace we use this constant should also be included in a patch. Can developers please check where/how many places this string is used? 


> p.p.s why is there these "funny" checks in WTP 2.0.x that makes Rob S.'s
> example still fail ?
> 

I'll have to let other developers address this. 
Comment 31 Carl Anderson CLA 2008-03-25 10:50:39 EDT
(In reply to comment #29)
> Ok - so what is the timeschedule/process on getting this validated and into an
> release of WTP ?
> 
> p.s. Anyone care to explain me how a string plugin id can affect the behavior
> of code that lists modules ?
> 
> p.p.s why is there these "funny" checks in WTP 2.0.x that makes Rob S.'s
> example still fail ?
> 

For the 2.0.2 patches, David Williams will have to answer the first question. 
But, for the record, that change was also put into WTP 3.0.

As to the p.s. - the string was used to cache the information between the
server code and the Deployable.  The problem was that, since the same ID was
used by both Deployables, the wrong Deployable was being used in the latter
work.

As to the p.p.s., the checks aren't exactly "funny".  WTP (from 1.0 until 3.0)
was coded with support for various (previously J2EE, but now called) Java EE
levels.  However, the code checked to make sure that everything inside of a 1.2
EAR complied with the 1.2 (or prior) specifications, and likewise for 1.3 and
1.4 and 5.0.  The tooling was not set up to allow modules from Java EE 1.3 in a
1.2 EAR.  As of WTP 3.0, we are now allowing Java EE 5 modules in a 1.4 EAR. 
That has been a major effort (and may still have bugs).
Comment 32 Carl Anderson CLA 2008-03-25 11:14:44 EDT
(In reply to comment #30)

>                   ... one thing I'm concerned about is that this is a final
> static string ... often inlined by compilers ... so anyplace we use this
> constant should also be included in a patch. Can developers please check
> where/how many places this string is used? 
> 

David - it is used in JEEFlexProjDeployable, which is in the same package in the same plugin.  So, if the patch repackages the plugin, it will catch the usage.  (However, adopters will most likely need to rebuild if they use that ID.)
Comment 33 Max Rydahl Andersen CLA 2008-03-25 11:18:02 EDT
Carl, thanks for your comments. I grok it now ;)

David, in my mind you definitly need to release a WTP 2.0.3 or WTP 2.0.2.1 ASAP since it *breaks* deployment of EAR's. Not just our product (JBoss Tools and JBoss Developer Studio) but anyone who has a jar or war listed in their application.xml that is *not* an WTP project. 

Any chance that can happen or somewhere you can point to how we as adopters should do a rebuild of this with proper version numbers etc. to avoid any collisions/incompatibilities ?
Comment 34 David Williams CLA 2008-03-25 12:56:45 EDT
(In reply to comment #33)

> 
> Any chance that can happen or somewhere you can point to how we as adopters
> should do a rebuild of this with proper version numbers etc. to avoid any
> collisions/incompatibilities ?
> 

I don't think you need to rebuild ... and probably shouldn't, as is hard to avoid version number collisions/incompatibilities. Do you ship WTP with your product? If so, you'd now just ship the patch features also, along with all the other features. 

If you direct users to install WTP first, and then your product, then either 1) you could direct them to update first (least pleasing alternative) or 2) you could still ship the patch features with your product .... just redistribute them "as is". 

Let me know if that makes sense (or not) and glad to work with you on providing a workable solution. 

Comment 35 Max Rydahl Andersen CLA 2008-03-25 14:09:48 EDT
For JBoss Tools we don't ship WTP they have to download it and that is how we discovered this bug initially. Everyone uses the broken WTP 2.0.2 and we get alot of bug reports on it.

For JBDS we ship WTP and were planning on using WTP 2.0.2 in the upcoming release but couldn't because of this issue.

"If so, you'd now just ship the patch features also, along with all the
other features. "

What are these patch features ? And what will there version be ?
Comment 36 David Williams CLA 2008-03-25 14:16:50 EDT
(In reply to comment #35)

> 
> What are these patch features ? And what will there version be ?
> 

If you download one of the zip files from the patch builds (202 stream), you will see them. Just unzip it. You would want to pick up all the fixes, not just this one. 

http://download.eclipse.org/webtools/patches/

The exact qualifier might change, if we had to re-spin, but think the other three numbers are correct and final. 


Comment 37 Rob Stryker CLA 2008-03-31 12:20:41 EDT
After downloading the patch from http://download.eclipse.org/webtools/patches/drops/R2.0.2/P-P20080327032254-20080327032254/ and unzipping it into my eclipse folder, the error is still here, not in the test project I provided you, but in our more in-depth use case.

The question I have is, how do I go about debugging with this patch? It seems to only include updated feature information and updated binaries, but no updated src.zip. 

Any advice? 
Comment 38 David Williams CLA 2008-03-31 13:18:09 EDT
(In reply to comment #37)

> 
> Any advice? 
> 

We've never updated source with patches, not sure what it would take ... but my advice would be to get the source from CVS. That is, for the few projects changed for patches ... they are in the R2_0_2_patches branch. 
Comment 39 David Williams CLA 2008-03-31 13:21:06 EDT
I should mention in this  bug, that P20080327032254 is not the final patched version ... and in fact it was an experiment with some version numbers that did not work out. I should delete it. P20080325024219 is actually the more accurate version, with respect to feature versions. Apologies for the confusion. 

Comment 40 Rob Stryker CLA 2008-03-31 14:19:11 EDT
The use case "A new example with an embedded WAR produced elsewhere" in attachments has been fixed and now works, however one where the extension is .jar still does not. 

I'll be attaching that as well.  Is it assumed that the latest patch (or the one you mention above) is the final one? Or is this issue still being worked on? I just need to know so I can pass this on to my users.

As for a the new use case, it's virtually identical to the war example above except the extension is .jar. The file contains no descriptor. Is it expected / assumed that if it listed in the module category it MUST have a descriptor? Is my next use case incorrect?

Thanks for all the help. 
Comment 41 Rob Stryker CLA 2008-03-31 14:22:52 EDT
Created attachment 94271 [details]
5.0 EAR with a child jar mentioned as a module
Comment 42 David Williams CLA 2008-03-31 14:57:38 EDT
(In reply to comment #40)
>
> Is it assumed that the latest patch (or the
> one you mention above) is the final one? Or is this issue still being worked
> on? I just need to know so I can pass this on to my users.
> 

Pretty much final. P-P20080325024219-20080325024219 contains the patch given here. 

We're trying to fix the way update manager works, but unzipping the zip would work. 

Though, if you are saying "not fixed", you might want to wait until one of the developers responds. 


Comment 43 David Williams CLA 2008-04-03 15:03:25 EDT
I have released a fix to our official update site, that can be installed by either looking there for new features to install ... or "update existing features". I have opened bug 225626 to track some limitations of the update manager that leads to some funniness, but should not prevent people from getting the patch (a patch feature shows up twice in the list ... this was required to get both update scenarios to work, updating fresh, and updating if previous patches had already been applied). 

So, please confirm that patch fixes the main problem reported here. 

I'm not sure about the war vs. jar issue, but do recall another bug about that issue, so maybe our fix for that was incomplete? 

Please open another bug about the war vs. jar issue, or, re-open this one if the patch from the update site does not fix this original, main problem. 

Thanks. 
Comment 44 Rob Stryker CLA 2008-04-10 19:08:40 EDT
This bug is not fixed. A jar file that is not an EJB and NTO a client jar, but *is* added via module tags, is not handled properly. In fact, it causes exceptions in the JEEDeployableFactory.createBinaryModules() method.

Here's some info. Example project to be added shortly. 

First and foremost, the jar is misidentified *somewhere else* as an application client jar. This is actually done somewhere on the EMF level. We'll track that down next.

In the code mentioned above (JEEDeployableFactory.createBinaryModules()): 
 	else if (j2eeModule.isJavaModule()) {
		moduleEdit = ComponentUtilities.getArtifactEditForRead(moduleComponent, J2EEProjectUtilities.APPLICATION_CLIENT);
		ApplicationClient appClient = (ApplicationClient) moduleEdit.getContentModelRoot();

This block of code is causing an NPE because AppClientBinaryComponentHelper.getPrimaryRootObject() doesn't account for one.

	public EObject getPrimaryRootObject() {
		return ((ApplicationClientFile) getArchive()).getDeploymentDescriptor();
	}

	protected IReferenceCountedArchive getArchive() {
		if (archive == null) {
			archive = getUniqueArchive();
		}
		return archive;
	}

	protected IReferenceCountedArchive getUniqueArchive() {
		try {
			return openArchive();
		} catch (OpenFailureException e) {
			Logger.getLogger().logError(e);
		}
		return null;
	}

As we can see here, archive was null, and it calls getUniqueArchive(), which fails, logs an error, and returns null. This null causes *another* error in getPrimaryRootObject() by trying to call null.getDeploymentDescriptor().

The only reason *any* of these blocks of code is reached at all is because the jar is immediately misidentified as an application client jar. This is a lot harder for me to track down, but a stack trace to where the client jar is created is provided below.  (For the record, my methodology to test and re-test this bug is by opening and closing the project.)

Thread [Worker-25] (Suspended (breakpoint at line 24 in JavaClientModuleImpl))	
	JavaClientModuleImpl.<init>() line: 24	
	ApplicationFactoryImpl.createJavaClientModule() line: 103	
	ModuleTranslator.createEMFObject(String, String) line: 130	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).setTargetFromNode() line: 1521	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).<init>(Node, EMF2DOMRenderer, Translator) line: 117	
	EMF2DOMSSEAdapter.<init>(Node, EMF2DOMRenderer, Translator) line: 42	
	EMF2DOMSSEAdapter.primCreateAdapter(Node, Translator) line: 269	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).createAdapter(Node, Translator) line: 855	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).primUpdateMOFMultiFeature(Translator, Node, List, List) line: 431	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).primUpdateMOFMultiFeature(Translator, Node, EObject) line: 1486	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).updateMOFMultiFeature(Translator, Node, EObject) line: 1700	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).updateMOFFeature(Translator, Node, EObject) line: 1755	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).primUpdateMOF() line: 933	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).updateMOF() line: 913	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).primUpdateMOFMultiFeature(Translator, Node, List, List) line: 461	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).updateMOFRootFeature() line: 954	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).primUpdateMOF() line: 929	
	EMF2DOMSSEAdapter(EMF2DOMAdapterImpl).updateMOF() line: 913	
	EMF2DOMSSERenderer(EMF2DOMRenderer).doLoad(InputStream, Map) line: 64	
	ApplicationResourceImpl(TranslatorResourceImpl).basicDoLoad(InputStream, Map) line: 144	
	ApplicationResourceImpl(CompatibilityXMIResourceImpl).doLoad(InputStream, Map) line: 175	
	ApplicationResourceImpl(ResourceImpl).load(InputStream, Map<?,?>) line: 1354	
	ApplicationResourceImpl(TranslatorResourceImpl).load(Map) line: 393	
	ProjectResourceSetEditImpl(ResourceSetImpl).demandLoad(Resource) line: 256	
	ProjectResourceSetEditImpl(ProjectResourceSetImpl).demandLoad(Resource) line: 149	
	ProjectResourceSetEditImpl(ResourceSetImpl).demandLoadHelper(Resource) line: 271	
	ProjectResourceSetEditImpl(ProjectResourceSetImpl).getResource(URI, boolean) line: 396	
	WorkbenchResourceHelper.getOrCreateResource(URI, ResourceSet) line: 384	
	ArtifactEditModel(EditModel).getResource(URI) line: 684	
	ArtifactEditModel.getResource(URI) line: 174	
	EARArtifactEdit.getDeploymentDescriptorResource() line: 315	
	EARArtifactEdit(EnterpriseArtifactEdit).getDeploymentDescriptorRoot() line: 166	
	EARArtifactEdit.getApplication() line: 301	
	JEEDeployableFactory.createBinaryModules(IVirtualComponent) line: 124	

As we can see, this is heavily steeped all the way down to the EMF level, which is a part of the code I'm not that familiar with. The closest I could follow before getting confused was down to EMF2DOMSSEAdapter.extractReadAheadName(), lines 1121 to 1129.

On 1121, "String[] names = helper.getValues();" yields "[ejb, java, web, connector]", and "java" is the one that eventually matches.

It seems that there is no possibility here for anything OTHER than ejb, web, connector, or java client jar. The result here is that extractReadAheadName returns "java", and ModuleTranslator.createEMFObject receives ("module", "java") as its parameters. 

The implementation of this method forces me to believe this is an error. ModuleTranslator.createEMFObject allows for null or some other readAheadName, and would return a generic module at that point via fact.createModule();  Code below:

	public EObject createEMFObject(String nodeName, String readAheadName) {
		ApplicationFactory fact = ApplicationFactory.eINSTANCE;
		if (EJB.equals(readAheadName))
			return fact.createEjbModule();
		else if (WEB.equals(readAheadName))
			return fact.createWebModule();
		else if (JAVA.equals(readAheadName))
			return fact.createJavaClientModule();
		else if (CONNECTOR.equals(readAheadName))
			return fact.createConnectorModule();
		else
			return fact.createModule();
	}

Either way, a java utility client jar is created and returned, and then later createBinaryModules chokes all over it because it doesn't have the expected descriptor, and THEN NPE's to boot. 
Comment 45 Rob Stryker CLA 2008-04-10 19:27:58 EDT
Interestingly and of note, is that the "export to ear file" works just fine for the example project I'm about to add, whereas neither the Generic Server Adapter works, nor my personal adapter.

Of note, the export action uses
   org.eclipse.jst.j2ee.application.internal.impl.ArchiveImpl
whereas the module factory eventually WOULD, if no readAheadName were passed in, use 
   org.eclipse.jst.jee.archive.internal.ArchiveImpl

One is from the EMF tree (the factory), one is not (the action). They seem to come from completely different points of view. 

The end result is that as long as I have an unknown jar in my project *and* in my application.xml, I am unable to get a reference to it in any fashion, either via members() or retrieving the children modules.
Comment 46 Rob Stryker CLA 2008-04-10 19:30:13 EDT
For completeness, the error logged to the console by JEEDeployableFactory line 163 is:


*** ERROR ***: Thu Apr 10 19:27:44 EDT 2008    org.eclipse.jst.j2ee.commonarchivecore.internal.exception.OpenFailureException: IWAE0006E Archive is not a valid Application Client JAR File because the deployment descriptor can not be found (case sensitive): META-INF/application-client.xml
    	at org.eclipse.jst.j2ee.internal.componentcore.EnterpriseBinaryComponentHelper$ArchiveCache.openArchive(EnterpriseBinaryComponentHelper.java:339)
    	at org.eclipse.jst.j2ee.internal.componentcore.EnterpriseBinaryComponentHelper.openArchive(EnterpriseBinaryComponentHelper.java:149)
    	at org.eclipse.jst.j2ee.internal.componentcore.EnterpriseBinaryComponentHelper.getUniqueArchive(EnterpriseBinaryComponentHelper.java:79)
    	at org.eclipse.jst.j2ee.internal.componentcore.EnterpriseBinaryComponentHelper.getArchive(EnterpriseBinaryComponentHelper.java:97)
    	at org.eclipse.jst.j2ee.internal.componentcore.AppClientBinaryComponentHelper.getPrimaryRootObject(AppClientBinaryComponentHelper.java:110)
    	at org.eclipse.wst.common.componentcore.ArtifactEdit.getContentModelRoot(ArtifactEdit.java:540)
    	at org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory.createBinaryModules(JEEDeployableFactory.java:163)
    	at org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory.createModuleDelegates(JEEDeployableFactory.java:99)
    	at org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory.createModules(JEEDeployableFactory.java:73)
    	at org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory.createModules(JEEDeployableFactory.java:62)
    	at org.eclipse.wst.server.core.util.ProjectModuleFactoryDelegate.cacheModules(ProjectModuleFactoryDelegate.java:60)
    	at org.eclipse.wst.server.core.util.ProjectModuleFactoryDelegate.getModules(ProjectModuleFactoryDelegate.java:257)
    	at org.eclipse.wst.server.core.internal.ModuleFactory.getModules(ModuleFactory.java:131)
    	at org.eclipse.wst.server.core.ServerUtil.getModules(ServerUtil.java:97)
Comment 47 Rob Stryker CLA 2008-04-10 19:35:04 EDT
Created attachment 95611 [details]
An example with a jar added via modules tag

Example application
Comment 48 Siarhei Dudzin CLA 2008-04-11 07:30:09 EDT
We experience the same issue. 

In our case we have eclipse projects generated with maven where all ejb and web modules are listed in application.xml and but not utility jars, the application also uses externally supplied ejb module. The application we experience WTP deployment problem with has the following structure:

app-ear.ear
   |-jboss-seam.jar
   |-app-web.web
   |-app-ejb.jar
   |-META-INF/
         |- application.xml
   |-lib/
         |- [utility jars]

where app-web and app-ejb come from eclipse projects and jboss-seam.jar is externally supplied ejb module.

With WTP 2.0.1 packaging worked exactly like that. When the WTP 2.0.2 first was released the external ejb module was not included in the deployment anymore:

app-ear.ear
   |-app-web.web
   |-app-ejb.jar
   |-META-INF/
         |- application.xml
   |-lib/
         |- [utility jars]

With the latest patch for WTP 2.0.2 that was supposed to fix this issue we started seeing the following EAR layout:

app-ear.ear
   |-jboss-seam.jar
   |-app-web.web
   |-app-ejb.ejb
   |-META-INF/
         |- application.xml

*Please note*, app-ejb now has ejb extension! This causes JBoss to throw "org.jboss.deployment.DeploymentException: Failed to find module file: app-ejb.jar" obviously because app-ejb.jar isn't there anymore!
Comment 49 Rob Stryker CLA 2008-04-11 17:19:03 EDT
The problem is also present in wtp 3.0 R6, though not nearly as egregiously. There is no NPE, no blowing away of the stack, nothing horrendous, however...

In M6, following code skips making the jar a child module:

	// If it is not a j2ee module and the component project is the
	// ear, it is just an archive
	// and we can ignore as it will be processed by the EAR
	// deployable.members() method
	if (qp.getType() == JavaEEQuickPeek.UNKNOWN) {
		continue;
	}

But it is still not handled by members();  Investigating here, as well. 
Comment 50 Rob Stryker CLA 2008-04-11 20:18:34 EDT
I recognize I've been very prolific in writing on this bug and it might be hard to separate my valuable comments from the rest. Let me try to summarize the issues that still remain here.

The setup:
   A generic .jar resource is added to "module" tags in an EAR's application.xml. It is then ignored entirely during publishing and in parts of the Virtual Component Model's intersection with the wst.server API's. It is not returned as a child module for reasons I will explain. It is also not returned from the members() method of the EarVirtualRootFolder

   This is repeatable in 2.0.2 as well as 3.0M6.  I posted a comment or two here about the 3.0 branch before realizing it was probably inappropriate to this specific bug. That one, and a patch, can be found over at show_bug.cgi?id=226782

In the 2.0.2 stream there are a few issues:

  1) the variable "j2eeModule" in JEEDeployableFactory.createBinaryModules() is mistakenly returned as a JavaClientModuleImpl and is then treated like one later. This problem is hard for me to trace and seems to originate heavily in the EMF portion of the project. See comment 44. 

  2) Because this jar, which is *not* a client module, is being treated like a client module, it is causing an NPE which completely blows away the stack and prevents createBinaryModules() from ever completing. The fact is this NPE botches the entire process of creating the modules via this ModuleFactory.  This is also explained in comment 44. 

On scale of importance, I would say problem 2 is the worst. The NPE completely blows away the stack and ruins the entire process. The NPE repeats often because the module factory is constantly trying to recreate its modules. 

As for problem 1, the 3.0 branch has seemingly fixed this problem with their JavaEEQuickPeek class, which seems to correctly identify the module as unknown. 
Comment 51 Rob Stryker CLA 2008-04-11 21:32:17 EDT
Created attachment 95786 [details]
An example with an ejb jar mistaken as an application client

In this example project, the .jar file is actually an ejb jar, but is being read as an Application Client jar. 

It suffers from all the previous errors, including having it's jeemodule variable be of the blatently incorrect type, falling into the Application Client part of the if statements, and the Null Pointer Exception which blows away the entire stack. 

The jboss-seam.jar file *DOES* have a META-INF/ejb-jar.xml file which correctly identifies it as an ejb jar. This should not be mistaken as a client jar but it is.
Comment 52 Rob Stryker CLA 2008-04-12 12:31:48 EDT
Bug report with patch for the 3.0 M6 stream
Comment 53 Rob Stryker CLA 2008-04-12 12:32:48 EDT
Ugh... sorry. Let me try again:

Bug report w/ patch for the 3.0 M6 stream:  https://bugs.eclipse.org/bugs/show_bug.cgi?id=226782
Comment 54 Rob Stryker CLA 2008-04-23 11:15:28 EDT
Ping. Any comments here? Comments 50 and 51 show the problems that still exist here. Specifically, library jars added via the MODULE tag are treated as application client jars... AND ejb jars added via the MODULE tag are *also* treated as application client jars. 

In short, the segment of code that reads the binary file and determines what type of file it is simply does not work. At all. 
Comment 55 Rob Stryker CLA 2008-04-23 18:32:04 EDT
This is no longer a hotbug. Sorry for the distractions.

In fact, it turns out a substantial portion of the errors were that we were, in the application.xml, TELLING app servers and tools alike that this module is a java client module, without realizing it.  Simply reading the spec revealed that <module><java>blah.jar</java></module> is specifically to be used only for application client jars.

This is now a minor bug. The minor bug is the null pointer exception that could happen if an application client jar does not have a descriptor. (See comment 44)
Comment 56 Max Rydahl Andersen CLA 2008-04-24 03:30:00 EDT
Hi,

I would like to stress that even though we can workaround this issue it is still a major regression bug in my book.

A) WTP 2.0.2 started ignoring <module> elements and not packaging them 

B) WTP 2.0.2 started renaming .jar to .ejb

That is *wrong*.




Comment 57 David Williams CLA 2008-04-24 15:44:37 EDT
(In reply to comment #56)

> I would like to stress that even though we can workaround this issue it is
> still a major regression bug in my book.
> 
> A) WTP 2.0.2 started ignoring <module> elements and not packaging them 
> 

Hi Max, do you mean you disagree with Rob? Or, are you talking about a different problem? 

Is another way of stating the issue that we used to work one way with an out-of-spec application.xml file, and now we work a different way with that same out-of-spec file? I can see how that'd be a frustrating surprise, but bringing your files up to spec is hardly a workaround. Please clarify if I'm misunderstanding. 







Comment 58 Max Rydahl Andersen CLA 2008-04-25 02:21:15 EDT
Hi David,

Let me see if I can explain it properly ;)

In our *specific* case we can work around this by following the spec strictly. Yes, that is mostly a bug on our side. Agreed.

For #48 it is another issue that is *not* workaroundable and if that behavior is true it is really bad for all maven users. (Note: this one we have not experienced)

In my point of view WTP is taking a biig step in the wrong direction when it starts changing semantics in mid-play.

application.xml is used (in WTP) as a way to drive packaging of EAR's and as such could care less about what is actually inside these jars. If a file is referenced as xyz.jar it should not be changed to xyz.ejb, just because a .jar file contains an ejb-jar.xml file should not prohibit it from being used as an application client jar and just because it does not contain some specific descriptor it should not also not be prohibited - afaik JEE5 does not require these descriptors.

This should at most give a validation Warning about a possible spec breach/inconsistency. (This is btw. what we have had to do for our next release, add a application.xml validator to inform users to update the application.xml's that WTP does not like but e.g. JBoss AS is ok with)

I know WTP 2.x is more J2EE 1.4 compliant than it is JEE 5 compliant but in any respect I find this partial strict coherence more of a hindrance than of any practical usage...and especially since it was done in a point release.

I hope that makes my statement more clear.
Comment 59 Max Rydahl Andersen CLA 2008-04-25 03:13:22 EDT
" not also not be prohibited "

...remove any not from there to make it correct ;)
Comment 60 David Williams CLA 2008-04-25 03:27:55 EDT
(In reply to comment #58)

> 
> For #48 it is another issue that is *not* workaroundable and if that behavior
> is true it is really bad for all maven users. (Note: this one we have not
> experienced)
> 

Ok, so you are saying the case in comment #48 is still a bug, but you haven't actually run into that bug ... is that because you haven't tried? Just different from what you do? Or, is it an intermittent bug? 

Siarhei, we may need a test case that demonstrates the problem, but I don't know if any of the WTP experts have tried, so maybe its easy to reproduce. But, even if it is easy to reproduce, it would definitely help (since everyone is over worked as it is) to attach a small "bare bones" project that we could use to try and reproduce and/or debug. 

Also important, does this problem occur in WTP 3.0? It would help if you tested that case -- and, test it now, early, since we are shutting down that development. 

I'll probably butt-out now and let you experts examine that one case, especially if a test case is provided, and even more especially if it also occurs in 3.0!. 

I did want to understand the issues though, because I can "hear" the frustration in some of the comments and wanted to be sure you all knew we do take it seriously. Its a little hard to interpret though, when a bug goes from blocker to minor, so I was hoping to clarify if there are any remaining issues to consider providing a patch for. I would say there is, _if and only if_ we can reproduce and understand the case described in comment #48. 

Your help and participation with that is appreciated. 

For _any_ other issue besides comment #48, please open a new bugzilla. You can still refer back to this one if you'd like but we need to focus and prioritize on very specific issues to make progress -- and we can't do everything ourselves. For example, I don't disagree about an improved warning/validation message (surprised there isn't one already, but I don't know that much about it) ... and since it sounds like you already have one, that would be a good contribution to make to the 3.0 stream. No? 

Thank you, 

Comment 61 Rob Stryker CLA 2008-04-25 12:43:07 EDT
I will attempt to make a project that demonstrates comment 48 today. 
Comment 62 Siarhei Dudzin CLA 2008-04-28 16:05:10 EDT
Created attachment 97835 [details]
Test case with maven project (demonstrating comment 48)

Sorry for a delay. I've created a test project which demonstrates the problem described in comment 48.

It uses eclipse-maven-plugin so all the jars have to come from MAVEN_REPO (defined in eclipse).

Let me know if you need more details.
Comment 63 Max Rydahl Andersen CLA 2008-04-30 15:15:27 EDT
Ok, it took a while for me to get back on this since we were verifying wether we have an issue still or not.

So the short version is that there still is a *big* publishing bug in WTP 2.0.2patched as far as we can see.

I'll attach the simplest test project that is *100%* spec compliant.

application.xml contains:

  <module>
    <ejb>jboss-seam.jar</ejb>
  </module>

and EarContent has jboss-seam.jar and jboss-seam-nodesc.jar.

jboss-seam.jar has a ejb-jar.xml to be compliant.

Result is:

jboss-seam.jar does not get deployed.

Put it in <java> and it gets deployed.

We also tried different combinations of with or without descriptors (jboss-seam-nodesc.jar) and it has the same result most of the time.

We also had a "fun" result where the jar did not end up in root, but ended up in
lib/<theprojectname>.jar/jboss-seam.jar inside the ear.

Where it got the lib dir name and <theprojectname>.jar from I have no idea - it is not in the project.

Furthermore I most of time gets the following exception when trying to edit applciation.xml.

Error
Wed Apr 30 21:10:08 CEST 2008
A structured model client, EMF2DOMSSEAdapter(module,null) threw following exception during adapter notification (STRUCUTRED_CHANGED )

java.lang.NullPointerException
	at org.eclipse.wst.common.internal.emf.resource.IDTranslator.setMOFValue(IDTranslator.java:45)
	at org.eclipse.wst.common.internal.emf.resource.EMF2DOMAdapterImpl.primUpdateMOFFeature(EMF2DOMAdapterImpl.java:1404)
	at org.eclipse.wst.common.internal.emf.resource.EMF2DOMAdapterImpl.updateMOFFeature(EMF2DOMAdapterImpl.java:1773)
	at org.eclipse.wst.common.internal.emf.resource.EMF2DOMAdapterImpl.primUpdateMOF(EMF2DOMAdapterImpl.java:933)
	at org.eclipse.wst.common.internal.emf.resource.EMF2DOMAdapterImpl.updateMOF(EMF2DOMAdapterImpl.java:913)
	at org.eclipse.wst.xml.core.internal.emf2xml.EMF2DOMSSEAdapter.notifyChanged(EMF2DOMSSEAdapter.java:227)
	at org.eclipse.wst.sse.core.internal.provisional.AbstractNotifier.notify(AbstractNotifier.java:201)
	at org.eclipse.wst.xml.core.internal.document.XMLModelNotifierImpl.notifyStructureChanged(XMLModelNotifierImpl.java:392)
	at org.eclipse.wst.xml.core.internal.document.XMLModelNotifierImpl.endChanging(XMLModelNotifierImpl.java:180)
	at org.eclipse.wst.xml.core.internal.document.DOMModelImpl.changedModel(DOMModelImpl.java:163)
	at org.eclipse.wst.sse.core.internal.model.AbstractStructuredModel$DocumentToModelNotifier.regionChanged(AbstractStructuredModel.java:164)
	at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument._fireEvent(BasicStructuredDocument.java:544)
	at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.fireStructuredDocumentEvent(BasicStructuredDocument.java:1183)
	at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.internalReplaceText(BasicStructuredDocument.java:1955)
	at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.replaceText(BasicStructuredDocument.java:2394)
	at org.eclipse.wst.sse.core.internal.text.JobSafeStructuredDocument.access$0(JobSafeStructuredDocument.java:1)
	at org.eclipse.wst.sse.core.internal.text.JobSafeStructuredDocument$1.run(JobSafeStructuredDocument.java:98)
	at org.eclipse.wst.sse.ui.EditorExecutionContext.execute(EditorExecutionContext.java:42)
	at org.eclipse.wst.sse.core.internal.text.JobSafeStructuredDocument.replaceText(JobSafeStructuredDocument.java:109)
	at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.replaceText(BasicStructuredDocument.java:2390)
	at org.eclipse.wst.sse.ui.internal.StructuredDocumentToTextAdapter.replaceTextRange(StructuredDocumentToTextAdapter.java:1187)
	at org.eclipse.swt.custom.StyledText.modifyContent(StyledText.java:5757)
	at org.eclipse.swt.custom.StyledText.sendKeyEvent(StyledText.java:6500)
	at org.eclipse.swt.custom.StyledText.doContent(StyledText.java:2155)
	at org.eclipse.swt.custom.StyledText.handleKey(StyledText.java:5029)
	at org.eclipse.swt.custom.StyledText.handleKeyDown(StyledText.java:5054)
	at org.eclipse.swt.custom.StyledText$7.handleEvent(StyledText.java:4800)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:938)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:962)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:947)
	at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:975)
	at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:971)
	at org.eclipse.swt.widgets.Widget.wmChar(Widget.java:1285)
	at org.eclipse.swt.widgets.Control.WM_CHAR(Control.java:3772)
	at org.eclipse.swt.widgets.Control.windowProc(Control.java:3672)
	at org.eclipse.swt.widgets.Canvas.windowProc(Canvas.java:291)
	at org.eclipse.swt.widgets.Display.windowProc(Display.java:4351)
	at org.eclipse.swt.internal.win32.OS.DispatchMessageW(Native Method)
	at org.eclipse.swt.internal.win32.OS.DispatchMessage(OS.java:2265)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3291)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2389)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2353)
	at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2219)
	at org.eclipse.ui.internal.Workbench$4.run(Workbench.java:466)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:289)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:461)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:106)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:169)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:106)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:76)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:363)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:176)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:585)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:508)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:447)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1173)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1148)




Comment 64 Max Rydahl Andersen CLA 2008-04-30 15:18:46 EDT
Created attachment 98244 [details]
updated test with 100% spec compliant ear

Here is the simplest Ear project that shows even 100% spec compliant application.xml and jar fails when running with WTP 2.0.2patched.
Comment 65 David Williams CLA 2008-04-30 15:57:54 EDT
Thanks Rob and Max, I can't comment on most of your issues, but a stack track like you get when editing looks pretty bad, like maybe the "model" may be corrupted, and you may want to exit and restart Eclipse. That sort of thing usually happens when there's "odd" multithreading issues, files changing on disk unexpectedly, etc. If that's reproducible, from a fresh start, with the files you've provided, we should be able to track it down. 
Comment 66 Max Rydahl Andersen CLA 2008-04-30 16:21:58 EDT
I could live with the stacktrace if the publishing were just happening correctly ,)

And the stacktrace I saw every time I restarted (3 times while writing the last repsonse to verify I was not just seeing things)
Comment 67 Rob Stryker CLA 2008-04-30 17:07:59 EDT
Howdy all =] 

Well... the problem Max is having is exactly what we were having before essentially, and it is due entirely to the error in comment 23. After fixing it (again) in my workspace, Max's problem goes away.

It seems the R_2_0_maintenance branch does not have the change required from comment 23. I was told that it is in the patches branch.

The problem is that we are not seeing the expected behaviour from the binary patch.  The patch we're using is http://download.eclipse.org/webtools/patches/drops/R2.0.2/P-P20080325024219-20080325024219/

The build is behaving exactly as if the change to the constant has not occurred.

This makes me think maybe comment 30 and comment 32 are worth looking into. Has the compiler inlined this constant? Is there any way for me to find out by looking at the binary patch? 
Comment 68 Rob Stryker CLA 2008-04-30 18:01:50 EDT
David:

I think we've fixed the most recent part of this issue... specifically Max's use case. 

We need to make sure *both* available patches are provided to the build. 
Comment 69 David Williams CLA 2008-04-30 19:20:23 EDT
(In reply to comment #68)
> 
> We need to make sure *both* available patches are provided to the build. 
> 

Both? There's one related to this bugzilla, right? Or am I missing something. I was hoping you meant we were missing the patch associated with this bugzilla, but I don't think we are. 

First, the branch for 202 patches is R2_0_2_patches (not R2_0_maintenance). 
And, the patch does seem to be there in that branch, and according to map files, org.eclipse.jst.jee was released with tag v200803242054 and that is the tag which appears in the directory.txt file of the build at http://download.eclipse.org/webtools/patches/drops/R2.0.2/P-P20080325024219-20080325024219/ 
so ... from all this, the build would seem correct ... for this one patch. 
(It still might be in error, but ... seems kind of unlikely based on all these matches ... and, no, I don't know of a way to check if/where inlined, except you sort of could with a decompiler ... but, I've looked pretty hard at source, and can only find that one reference to   org.eclipse.jst.jee.internal.deployables.JEEDeployableFactory.ID 
(in same plugin, as Carl noted). 

So ... let me ask the obvious ... do _you_ use that constant in your code? Could it be inlined there? And you'd need to recompile? 

Comment 70 Rob Stryker CLA 2008-04-30 20:35:52 EDT
We don't use the constant anywhere in our code. 

For the record, I personally am doing all testing with wtp's GENERIC server adapter. I am not testing *our* code at all. Nada. Zero.

Over at the patch site, there are two patches listed:

1) P20080325024219	R2.0.2	Tue, 25 Mar 2008 -- 02:42 (UTC)
2) P20080313153530	R2.0.2	Thu, 13 Mar 2008 -- 15:35 (UTC)

What we've found is taking a plain wtp 2.0.2 install (with all pre-reqs) and adding in only patch 1 *DOES NOT WORK* and creates all sorts of errors with Max's example project. 

However, adding in both patches *does* work and will make publishing work fine.

The symptoms you'll see after only installing patch 1 is that jboss-seam.jar will not be published and in fact will be loaded as a separate module instead of a child module to the EAR project. You can demonstrate this by creating a GENERIC server (of type jboss 4.2) and right-clicking on it and selecting "Add / Remove Projects".  In that window, you will see TWO modules listed:  The ear project and the jboss-seam.jar lib. 

If you apply both patches, however, in that window you will see ONE module listed (the ear project), which then has a child module (the jboss-seam.jar module).

Both patches at http://download.eclipse.org/webtools/patches/ must be applied in order to have it behave as expected. 
Comment 71 David Williams CLA 2008-04-30 21:11:32 EDT
(In reply to comment #70)
> We don't use the constant anywhere in our code. 
> 
> For the record, I personally am doing all testing with wtp's GENERIC server
> adapter. I am not testing *our* code at all. Nada. Zero.
> 
> Over at the patch site, there are two patches listed:
> 
> 1) P20080325024219      R2.0.2  Tue, 25 Mar 2008 -- 02:42 (UTC)
> 2) P20080313153530      R2.0.2  Thu, 13 Mar 2008 -- 15:35 (UTC)
> 
> What we've found is taking a plain wtp 2.0.2 install (with all pre-reqs) and
> adding in only patch 1 *DOES NOT WORK* and creates all sorts of errors with
> Max's example project. 
> 
> However, adding in both patches *does* work and will make publishing work fine.
> 

Ok, not that I doubt what you are saying ... but, it doesn't make much sense :) 
so I want to confirm, and explain. 
First to explain. These "patch builds" are meant to be cumulative. Everything in the latest one should have everything in the previous ones. 
Examining the map files (via directory.txt files, does indicate the only difference in plugins is the org.eclipse.jst.jee plugin has been added in the most recent one. 
Unzipping these zip files, and diffing the directories agrees, that the only new plugin is the org.eclipse.jst.jee plugin, and is named org.eclipse.jst.jee_1.0.3.v200803242054.jar. 

So ... since what I see is different from what you are saying I want to confirm ... when you did your test, you only installed the most recent, newest zip file, what you are calling number '1' above, but if someone was numbering them from oldest to newest (the natural order) then it would be number '2' (natural, since it's the second one created) so that's partially why I want to confirm your use of number '1' and number '2'. We should go by dates ... the P20080325024219 build should contain all you need and if installing the other somehow "makes it work" then I think something else is going on, besides what's in the code. 

How do you "install", btw, just unzip on top of an existing install ... right? 
So you do anything with 'extension locations'? 

And, btw, installing the oldest one first, P20080313153530, and then installing the newest one, P20080325024219 could possibly cause funny issues, but that doesn't sound like what you are talking about, so I am assuming that is not your procedure. 
Comment 72 David Williams CLA 2008-04-30 22:28:09 EDT
Ok, I can definitively say "worksforme" and think maybe you've gotten your test process mixed up. Here's what I did. 

Installed 202, plus pre-reqs. 

Imported the last test case, "updated test with 100% spec compliant ear". 

Added generic 4.2 jboss server. 

I could then see the symptom you describe " ... right-clicking on it and selecting "Add / Remove Projects".  In that window, you will see TWO modules listed:  The ear
project and the jboss-seam.jar lib. " 

I did in fact see those two. 

I THEN installed (shut down and unzipped the zip file on top of existing install) the most recent patch build, P20080325024219, restarted eclipse and then tried again to see the symptom ... now I saw only the one project with a child ... "...in that window you will see ONE module listed (the ear project), which then has a child module (the jboss-seam.jar module)."

So ... I think we on this end are back to saying this bug is fixed. 
Comment 73 Rob Stryker CLA 2008-04-30 22:47:35 EDT
I think you're right David. I spent hte past hour doing the same and getting your results and trying to figure out why they differed before.

I want to thank you, a lot, for sticking with us on this despite our... um... bi-polar disorder? ;)  

Our build is still wrong, but the use case works which means it is not your fault. 
Comment 74 David Williams CLA 2008-05-01 00:05:47 EDT
> I want to thank you, a lot, for sticking with us on this despite our... um...
> bi-polar disorder? ;)  


No problem ... give and take is what it takes to communicate. 

So, here's what I'd like to do, 1) resolve this bug as fixed. 

2) Siarhei Dudzin, if you are still having an issue as described in #48 after installing as described in final notes here, then please open a _new_ bug, with the same projects you gave us in comment #62. And, frankly, you will need to give us more detailed instructions. 

I do not know how to use " ... eclipse-maven-plugin so all the jars have to come from MAVEN_REPO (defined in eclipse) ... ". And, I'll warn you, anything you can do to isolate the problem and provide a patch, (in addition to a test case we can run) then the more likely it will be to get fixed. None of us here work much with Maven repo's, so it would be harder for us to work on and since we are over worked already, that's just enough to push it down the work queue. (Hope that doesn't sounds mean, resistant, or unappreciative ... just being 'open and honest' about our work load and the way we juggle our many priorities. 


3) If anyone wants to, please open a new, minor bug for "The minor bug is the null pointer exception that could happen if an application client jar does not have a descriptor. (See comment 44)" And ... I'll warn you here too ... none of us core team would probably worry too much about that bug, or decide to fix that bug, but if you (or someone) provided a safe, high quality patch, with good test case, we would consider evaluating it and including in any future updates and patch builds ... if we ever decide to do another one ... if we find other blocker/critical issues that must be fixed. 

Thanks all, I appreciate the quick conversations and clear test cases to get to the bottom of things. 

Comment 75 Max Rydahl Andersen CLA 2008-05-01 00:57:36 EDT
What I find worrying is that the binaries was exactly the same and the patched versions showed up in the feature/plugin about dialog and in the runtime plugin registry.....the patch where somehow applied but not working...really weird!

We will continue to investigate and hopefully we will figure out what causes the patches to not take effect when we are apparently just doing the same process automatically (unzipping) which works manually.

Comment 76 David Williams CLA 2008-05-01 01:21:53 EDT
(In reply to comment #75)
> What I find worrying is that the binaries was exactly the same and the patched
> versions showed up in the feature/plugin about dialog and in the runtime plugin
> registry.....the patch where somehow applied but not working...really weird!
> 

If you had both patches builds installed, It may be related to the fact that one of the patch features in both builds (having one plugin in one, but two plugins in the second) and it is the same name, and same version, except for the qualifier. 

I'd been under the impression that just upping the qualifier was all that was needed, but we discovered that didn't work correctly for Update Manager (see bug 225626). You never mentioned update manager, so I am assuming that's not it, but if you use a platform.xml file, then there might be similar effects of Eclipse getting confused on which is the right feature to recognize.  (and, right there I've said all I know about platform.xml files :) 






Comment 77 Max Rydahl Andersen CLA 2008-05-09 05:39:42 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

Also see https://bugs.eclipse.org/bugs/show_bug.cgi?id=230256 which is another bug related issue for WTP erroring when dealing with plain JEE jars.