Bugzilla – Full Text Bug Listing
| Summary: | Template XML file isn't correctly queried | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Sinee Paungam <spaungam> | ||||||||
| Component: | TPTP | Assignee: | Alex Nan <apnan> | ||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||
| Severity: | critical | ||||||||||
| Priority: | P1 | CC: | apnan, jcayne, jkubasta, sluiman | ||||||||
| Version: | unspecified | Keywords: | plan | ||||||||
| Target Milestone: | --- | Flags: | jcayne:
iplog+
apnan: review+ |
||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | closed460 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Sinee Paungam
Created attachment 80949 [details]
Workaround/patch for this problem
4.5 i1 development is closed. I will check in the patch when i2 begins. Assuming problem found in TPTP 4.2.2. Please correct if necessary. Created attachment 81438 [details]
Patch
Patch has been tested and verified with the existing JUnit tests for this component.
Created attachment 81439 [details]
Test Plug-in
Ann, please verify that the fix resolves your issue (without the work-around) using this TPTP 4.5 plug-in.
Note, this test plug-in is ONLY for testing purposes and should not be packaged, deployed, or shipped.
Correcting target. Alex, can you please review this patch? Patch reviewed. Stop-ship template: 1. Explain why you believe this is a stop-ship defect. How does the defect manifest itself, and how will users of TPTP / consuming products be affected if the defect is not fixed? In the Common Base Evnet implmentation, a template XML file is loaded by resilving its inputstream from the class loader. The code uses a class loader look-up sequeunce (e.g. EventXMLFileEventFactoryHomeImpl's class loader, system's class loader, and the current thread's context class loader) to resolve the inputstream. The current code expects that a failed look-up will throw an exception, but the ClassLoader#getResourceAsStream(String) API returns null when the resource cannot be found. As a result, the template XML file is not loaded. An IBM consumming product requires this fix. 2. Is there a work-around? If so, why do you believe the work-around is insufficient? No. 3. Is this a regression or API breakage? Explain. No. 4. Does this require new API? No. 5. Who performed the code review? Alex Nan 6. Is there a test case attached to the bugzilla record? Yes. 7. What is the risk associated with this fix? Low to medium. 8. Is this fix related to any standards that TPTP adheres to? If so, who has validated that the fix continues to adhere to the standard? No. Paul there seem to be a lot of caught and ignored exceptions which is not an encouraged practice. I realize this was in the end the previous behavior as well but the code in general is a complete replace of even the comments, so catching and at least logging the exception is one option or simply let the exception flow, although that is an api change that would ripple up. This all implies that the old code never worked a was clearly not tested either. Although clearly bad, I find it odd that only now this problem is surfacing and now with high importance. BTW the contributed patch will need to be marked as a non-committer contribution. (In reply to comment #10) > Paul there seem to be a lot of caught and ignored exceptions which is not an > encouraged practice. I realize this was in the end the previous behavior as > well but the code in general is a complete replace of even the comments, so > catching and at least logging the exception is one option or simply let the > exception flow, although that is an api change that would ripple up. The multiple catch blocks (opposed to one) are required in the event that caller does not sufficient privileges for one particular class loader. The Common Base Event implementation does not have logging instrumentation (we do not log from a log structure) and we should not change the API (re-throw the exception) to resolve a behaviour defect. > This all implies that the old code never worked a was clearly not tested > either. Although clearly bad, I find it odd that only now this problem is > surfacing and now with high importance. Correct, this code has never worked or been tested correctly. The symptom has only surfaced now since in the consuming prodcut, the current class' class loader could not resolve the template file (e.g. segregated class loaders), for example, in a server environment where the template XML file is used by a different class loader. This behaviour is not typical since in most cases, the current class' class loader resolves the template file. Note, I did not attach the new test case: Abstract: Loading an event configuration template XML file in a multiple class loader environment. Version: October 28, 2007 Since: October 28, 2007 Authors: 1. Paul Slauenwhite Requires: Platforms: 1. Windows 2. Linux Notes: Steps: 1. Create the EMF Common Base Event v1.0.1 Sample (File >> New >> Example... >> Logging). 2. Create a second plug-in project (e.g. TestPluginProject). 3. Move the HyadesEMFCommonBaseEventv101Project\src\HyadesEMFCommonBaseEventv101Sample.event.xml file to the source tree of the second plug-in project. 4. Run the HyadesEMFCommonBaseEventv101Project\src\HyadesEMFCommonBaseEventv101Sample.java class, following the example's instructions. Results: 1. See the example's instructions for desired output. > BTW the contributed patch will need to be marked as a non-committer > contribution. Although Anne's patch was for resolving the issue in the consuming product by overloading the resolveTemplateXMLFileInputStream(String) method, we can mark it as a non-committer contribution and add Anne to the project IP log for 4.5. Anne: There are three questions that should be answered for this contribution (see http://www.eclipse.org/legal/EclipseLegalProcessPoster.pdf). Please provide the answers in this defect: 1) Did you write this code yourself? 2) Do you have the right to contribute this code to Eclipse? 3) Do you submit this patch under the terms of the Eclipse Public License (http://www.eclipse.org/org/documents/epl-v10.php)? Comment on attachment 80949 [details]
Workaround/patch for this problem
Not the patch submitted for Project/PMC approval.
Transferring to Alex. Sinee, can you let us know on which release level this fix is required? Do you need a fix on 4.2.2 or the current 4.4.1 fix would be good? Thanks. (In reply to comment #14) > Sinee, can you let us know on which release level this fix is required? Do you > need a fix on 4.2.2 or the current 4.4.1 fix would be good? > Thanks. > or the default which would be 4.5 Changing target to 4.5 i5 since it was not approved in 4.4.1. The code will be checked into the 4.5 stream in i5 since this defect is not neccessarily needed in M4. Fix commited to 4.5 and manual test case created for testing the defect. adding iplog+ per Ganymede IP log As of TPTP 4.6.0, TPTP is in maintenance mode and focusing on improving quality by resolving relevant enhancements/defects and increasing test coverage through test creation, automation, Build Verification Tests (BVTs), and expanded run-time execution. As part of the TPTP Bugzilla housecleaning process (see http://wiki.eclipse.org/Bugzilla_Housecleaning_Processes), this enhancement/defect is verified/closed by the Project Lead since this enhancement/defect has been resolved and unverified for more than 1 year and considered to be fixed. If this enhancement/defect is still unresolved and reproducible in the latest TPTP release (http://www.eclipse.org/tptp/home/downloads/), please re-open. |