Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 312336

Summary: FlatComponentDeployable legacy calls creates ModuleFile without a workspace resources
Product: [WebTools] WTP Java EE Tools Reporter: Angel Vera <arvera>
Component: wst.webAssignee: Jason Peterson <jasonpet>
Status: RESOLVED FIXED QA Contact: Chuck Bridgham <cbridgha>
Severity: critical    
Priority: P3 CC: ccc, jsholl, raghunathan.srinivasan, stryker
Version: 3.2Flags: jasonpet: pmc_approved? (david_williams)
raghunathan.srinivasan: pmc_approved+
jasonpet: pmc_approved? (naci.dai)
jasonpet: pmc_approved? (deboer)
jasonpet: pmc_approved? (neil.hauge)
jasonpet: pmc_approved? (kaloyan)
cbridgha: review+
Target Milestone: 3.2 RC1   
Hardware: PC   
OS: Windows XP   
Whiteboard: PMC_approved
Attachments:
Description Flags
conditionally changes the moduleFile's constructor none

Description Angel Vera CLA 2010-05-10 16:59:36 EDT 
I noticed that in the members calls for FlatComponentDeployable.members there is a call that checks if component.isBinary, if such is the case the code will go on to create ModuleFile using a java.io.File with no references to the workspace file. 

I am not sure why this was done this way, but our adopter product is breaking because in the case of binary EARs with a web module, we are expecting to have the in workspace resources instead of the file system resource. 

It seems that this is a change from the way that the old J2EEFlexibleProjDeployable used to do it as seen in the J2EEFlexibleProjDeployable.members call, which will call the getBinaryModuleMembers and checks for the existance of workspaces resources, first and then moves on to external files.
Comment 1 Angel Vera CLA 2010-05-11 09:37:56 EDT 
We are looking to get this fixed in 3.2, as this is a regression from 3.1
Comment 2 Jason Peterson CLA 2010-05-11 19:34:29 EDT 
Rob, this was a change you had made.  Is there a specific reason why it just returns java.io.File now?
Comment 3 Jason Peterson CLA 2010-05-11 19:36:57 EDT 
The same check to determine if it is a workspace resource to return an IFile vs java.io.File could be done by clients.  Is there some reason this can't be done Angel?  We are late in the cycle and this code has been this way for a few milestones now.
Comment 4 Angel Vera CLA 2010-05-12 09:01:45 EDT 
We found this out during our adopters testing, although we could do the check in our side, I am not sure what other places could be broken. 

I vote for fixing it the JEE side since it is a regression from 3.1. Was there a reason for the change? We should determine why was the old code not ported so that we can make a decision.
Comment 5 Rob Stryker CLA 2010-05-12 10:58:25 EDT 
Created attachment 168158 [details]
conditionally changes the moduleFile's constructor

This seems like a good patch but I haven't tested it since I don't have a test case. Let me know if this fixes your usecase.
Comment 6 Jason Peterson CLA 2010-05-12 11:25:52 EDT 
Angel can you please test this as soon as possible?  I would like to put this up for review today and get it in RC1 if possible.
Comment 7 Angel Vera CLA 2010-05-12 15:05:58 EDT 
Testing completed. This works fine now. Thanks for the patch.
Comment 8 Chuck Bridgham CLA 2010-05-12 15:33:01 EDT 
approved
Comment 9 Jason Peterson CLA 2010-05-12 15:35:59 EDT 
* Explain why you believe this is a stop-ship defect. Or, if it is a
"hotbug" (requested by an adopter) please document it as such. 

Adopter's are expecting an IFile as a parameter to the ModuleFile constructor when the resource is a workspace resource and are expecting a java.io.File for external resources.  This issue exposes a regression issue where now only java.io.File is used regardless of whether the resource is a workspace or external resource.

    * Is there a work-around? If so, why do you believe the work-around is
insufficient? 

The workaround would require adopters to know of the regression and code around it.  This is not a sensible workaround.

    * How has the fix been tested? Is there a test case attached to the
bugzilla record? Has a JUnit Test been added? 

No unit tests for this bug.

    * Give a brief technical overview. Who has reviewed this fix? 

The fix is to simply call the right ModuleFile constructor based on whether the binary component's underlying resource is a workspace or external resource.  This has been reviewed and approved by Chuck.  Angel has tested that the patch solves the issue at hand.

    * What is the risk associated with this fix? 

I see little risk with this patch.  It will now work as it did pre WTP 3.2
Comment 10 Jason Sholl CLA 2010-05-12 18:50:01 EDT 
code checked into head for wtp 32 rc1