Bugzilla – Full Text Bug Listing
| Summary: | Compile errors in EJB when client references exist in both manifest and component file | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Java EE Tools | Reporter: | Jason Peterson <jasonpet> | ||||
| Component: | jst.j2ee | Assignee: | Jason Peterson <jasonpet> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Chuck Bridgham <cbridgha> | ||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | ccc, david_williams, deboer, jsholl, kaloyan | ||||
| Version: | 3.2 | Flags: | david_williams:
pmc_approved+
jasonpet: pmc_approved? (raghunathan.srinivasan) jasonpet: pmc_approved? (naci.dai) deboer: pmc_approved+ jasonpet: pmc_approved? (neil.hauge) kaloyan: pmc_approved+ cbridgha: review+ |
||||
| Target Milestone: | 3.2.2 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | PMC | ||||||
| Attachments: |
|
||||||
|
Description
Jason Peterson
Created attachment 178849 [details]
patch
approved * 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.
Compile errors are thrown due to missing references in the EAR library. The reasons they are missing is due to API that is filtering out the manifest references since they are duplicated (exist in the manifest and component file). The EAR library however is only interested in the manifest references and will not pick up the duplicated reference from the component file.
This duplication is common for an EJB's client project reference, due to a pre-3.2 bug that would add a reference to both places when an EJB client project was added to an EJB.
* Is there a work-around? If so, why do you believe the work-around is
insufficient?
User would have to manually remove the duplicated reference from the component file. This is not a feasible works-around though since a user would have no way of knowing this would correct the compile errors.
* How has the fix been tested? Is there a test case attached to the
bugzilla record? Has a JUnit Test been added?
Fix has been tested in the UI.
* Give a brief technical overview. Who has reviewed this fix?
The fix is to update the manifest references API to always return all manifest
references. The filtering out of duplicates will happen in separate API that is responsible for returning both component and manifest references.
Chuck has reviewed this fix.
* What is the risk associated with this fix?
minimal - The filtering out of duplicates still occurs when necessary
Hi Jason, I'm sure you're feeling ignored by now :) So I'll at least write my reason for silence. This doesn't seem serious enough or wide-spread enough to justify a respin. It doesn't sound blocking, per se ... just bad. And, honestly, I don't know how duplicate entries can possibly cause compile problems ... though I'm sure it does. It just sounds more complicated than presented here, or that I understand. So I won't veto it outright, in case others understand it better and think it is worth the risk of a respin to pick it up ... they can still say so. I do understand its serious ... but at this point ... it's pretty late to be taking a chance on changes. How do we know something else more serious won't regress as a result? Not much time left for testing. Please do continue the discussion if you think its worth it ... its just not clear to me, so ... that's why I hadn't said (or approved) anything. Thanks, Not sure what needs to be explained here. It was pretty clear that a regression caused by bug 323757 now causes compile errors. Not sure how to better explain why there are compile errors, but I will try my best. I guess I will need to go into further code details here. The EAR library classpath container used to get all the manifest references and component file references to compute the classpath. The duplicate entry from the manifest file would be filtered out in the get manifest API. However, it still got the entry from the component file so there were no compile issues. In bug 323757 the EAR library classpath container was updated to only get the manifest references. This API was still filtering out the manifest references that were duplicated with references in the component file. Because it was filtering out the manifest reference and was no longer picking it up from the component file the reference was lost. This causes the compile error. The fix was to remove the filtering of the duplicate manifest references from the get manifest references API. This way all the manifest references will be returned when a client only wants the manifest references and calls the get manifest references API. For clients that want both manifest and component file references the duplicates will still be filtered out. This is because the filtering is done in the higher level API now. I hope this is detailed enough. Not sure how to better explain it than this other than pasting in the code. This is a bad problem. Users who have existing EJB projects using EJB Clients created on versions prior to 3.2 will have compilation errors when they start 3.2.2. Users will be frustrated because projects and workspaces that are perfectly OK on 3.2.1 will now be broken. I have reviewed the fix and believe the risk for regression to be minimal. This bug is a bad enough regression from 3.2.1 that I think it should be included for 3.2.2. Thanks, Jason and Jason, it does sound bad. One thing that caught my attention was this statement: "The fix was to remove the filtering of the duplicate manifest references from the get manifest references API." That sounds like an API behavior change. Is it? Isn't that risky? Even if it now has the "correct" behavior, what if someone was depending on the incorrect behavior accidentally? (I'm just asking ... no idea if that's even possible, much less likely). Just out of curiosity, if the fix for bug 323757 cause the problem, is one solution to revert that fix? I've sent an email to other PMC members asking them to comment here, since this is a serious issue and deserves the attention, discussion, and an explicit decision. Thanks again, The API remains intact; only internal uses of the API would be affected, and we confirmed that all internal uses of the API should actually be getting the results this fix will return. As for reverting bug 323757; yes that is an option, but not a good one. It would be better to move forward with this fix. This is a serious regression. We should have the fix in 3.2.2. I agree with Jason's comment 8. If we do a respin, then it's better to have a complete fix, than reverting to a previous state of buggy behavior. Agreed. It's a regression and the fix is fairly contained, so it is worth respinning. Committed to R3_2_maintenance for WTP 3.2.2. I will leave this bug open, and will resolve it when I commit it to HEAD for WTP 3.3. (Shortly.) Committed to HEAD for WTP 3.3. |