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

Bug 324996

Summary: Refactoring transactions are not processed in the order of arrival.
Product: z_Archived Reporter: Paul Slauenwhite <paulslau>
Component: TPTPAssignee: Paul Slauenwhite <paulslau>
Status: CLOSED FIXED QA Contact: Kathy Chan <kathy>
Severity: critical    
Priority: P1 CC: alexberns, jcayne, jerome.bozier, jptoomey
Version: unspecifiedFlags: paulslau: pmc_approved? (oec)
paulslau: pmc_approved? (ernest)
kathy: pmc_approved+
paulslau: pmc_approved+
paulslau: pmc_approved? (jgwest)
paulslau: review+
jptoomey: review+
jcayne: review+
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard: adopter
Bug Depends on: 297609    
Bug Blocks:    
Attachments:
Description Flags
Patch.
none
Patch.
none
Patch (TPTP 4.7.0.1).
none
org.eclipse.hyades.test.ui none

Description Paul Slauenwhite CLA 2010-09-10 13:33:43 EDT
Refactoring transactions are not processed in the order of arrival.

When the user invokes a refactoring operation in the Test Navigator, each org.eclipse.hyades.test.ui.navigator.IRefactoringTransaction is added to the org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringContext for processing (e.g. commit).  However, the order of arrival of refactoring transactions is not maintained, causing an unpredictable/incorrect processing (e.g. commit) order.  As a result, refactoring operations in a consuming product fail.

This symptom is caused by the org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringContext maintaining the refactoring transactions in a java.util.HashMap (unpredictable order) data structure.  The map is keyed by the domain ID of transaction as defined in the org.eclipse.hyades.test.ui.testNavigatorRefactoringTransactions extension point implementation.  The current code orders the transactions based on the key, derived from the domain ID.  TPTP defines the org.eclipse.hyades.test.ui.navigator.refactoring.EMFRefactoringTransaction domain ID for TPTP (first) refactoring transaction but implementers (e.g. consuming products) of the org.eclipse.hyades.test.ui.testNavigatorRefactoringTransactions extension point may define any domain ID size/composition.  If a defined ID is less (size/composition), the implementer's (e.g. consuming product's) transactions are processed first, which is not the order of arrival.

The solution is to use a java.util.LinkedHashMap data structure that guarantees predictable iteration order.

Note, this symptom was uncovered by the changes delivered under TPTP 4.7.0 defect https://bugs.eclipse.org/bugs/show_bug.cgi?id=297609.  Specifically, the TPTP domain ID changing from "org.eclipse.hyades.test.ui.EMFRefactoringTransaction" to "org.eclipse.hyades.test.ui.navigator.refactoring.EMFRefactoringTransaction".

Impacting a consuming product.
Comment 1 Paul Slauenwhite CLA 2010-09-10 13:38:07 EDT
Hours worked.
Comment 2 Paul Slauenwhite CLA 2010-09-10 13:43:55 EDT
Created attachment 178639 [details]
Patch.
Comment 3 Paul Slauenwhite CLA 2010-09-10 14:06:46 EDT
Created attachment 178642 [details]
Patch.
Comment 4 Paul Slauenwhite CLA 2010-09-10 14:07:21 EDT
Joe, could you please review the attached patch?
Comment 5 Joe Toomey CLA 2010-09-10 14:24:12 EDT
Looks good.  Thanks Paul.
Comment 6 Paul Slauenwhite CLA 2010-09-10 14:27:20 EDT
   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?

See above.

   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.

New symptom uncovered by TPTP 4.7.0
defect https://bugs.eclipse.org/bugs/show_bug.cgi?id=297609.

   4. Does this require new API?

No.

   5. Who performed the code review?

Joe Toomey

   6. Is there a test case attached to the bugzilla record?

Covered by existing TPTP and consuming product test cases.

   7. What is the nature of the fix? What is the scope of the fix? What is the risk associated with this fix?

See above.

   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.
Comment 7 Joe Toomey CLA 2010-09-10 14:38:53 EDT
Thanks Paul.  Looks good.
Comment 8 Paul Slauenwhite CLA 2010-09-10 14:41:59 EDT
Attached patch checked in to CVS (HEAD).
Comment 9 Paul Slauenwhite CLA 2010-09-13 10:46:14 EDT
Verified in TPTP-4.7.1-201009121900.

Closing.
Comment 10 Paul Slauenwhite CLA 2010-09-14 08:17:59 EDT
Created attachment 178814 [details]
Patch (TPTP 4.7.0.1).
Comment 11 Paul Slauenwhite CLA 2010-09-14 08:18:32 EDT
Joel, could you please review 'Patch (TPTP 4.7.0.1)'.
Comment 12 Joel Cayne CLA 2010-09-14 10:01:26 EDT
Patch (TPTP 4.7.0.1) looks good.
Comment 13 Joel Cayne CLA 2010-09-14 10:55:06 EDT
Created attachment 178839 [details]
org.eclipse.hyades.test.ui

Attaching the org.eclipse.hyades.test.ui plug-in exported manually in a workbench using Java 1.5 JVM. The component was tagged in CVS for the change with the tag v201009141010 and exported with this version tag. The org.eclipse.hyades.releng.builder map file has been updated to include the new tag.

The attached exported plug-in is signed by Eclipse and can be installed into a workbench.