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

Bug 482008

Summary: Remove content synchronization operations, enforce content related synchronization is performed through AbstractContentPart
Product: [Tools] GEF Reporter: Matthias Wienand <matthias.wienand>
Component: GEF MVCAssignee: Alexander Nyßen <nyssen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: nyssen
Version: 0.2.0   
Target Milestone: 4.0.0 / 3.11.0 (Neon) M4   
Hardware: All   
OS: All   
Whiteboard:

Description Matthias Wienand CLA 2015-11-12 08:27:58 EST
Currently, a content synchronization is either manually performed via an operation or in response to certain events, such as a change to the ContentModel or to the CONTENT_PROPERTY of an IContentPart. However, manually performing the content synchronization via an operation is error-prone, because 1) the effects of the synchronization are not strictly defined and 2) the synchronization might be performed on an already removed part.

When content is added to a viewer or removed from a viewer through a user interaction, a content synchronization has to be performed to create/remove content parts accordingly. This is currently done within operations returned by ContentPolicy. Due to the above tradeoffs, this is undesirable.

As an alternative, the synchronization can be performed from within the AbstractContentPart#addContentChild(), #removeContentChild(), #attachToContentAnchorage(), and #detachFromContentAnchorage() methods.
Comment 1 Alexander Nyßen CLA 2015-11-12 14:46:14 EST
Pushed the following changes to origin/master:
- Deleted SynchronizeContentAnchoragesOperation and SynchronizeContentChildrenOperations.
- Changed contract of IContentPart to fire notifications when content children or content anchorages are changed. Turned addContentChild(), removeContentChild(), attachToContentAnchorage(), and detachFromContentAnchorage() operations into final operations that realize this contract and delegate to newly introduced doXXX() hook methods, which are declared as abstract.
- In order to ensure that clients implement all content related operations, removed getContentChildren() and getContentAnchorages() operations, which returned empty collections, so that clients now have to provide these explicitly.
- Adjusted clients (Zest.FX, MVC Logo example) and tests to follow the new contract.

Resolving as fixed in 3.11.0 M4.
Comment 2 Alexander Nyßen CLA 2015-11-12 15:50:58 EST
I amended the following changes:

- Added reorderContentChild() to IContentPart and related doReorderContentChild() operation to AbstractContentPart.
- Made the abstract doXXX() methods in AbstractContentPart concrete, throwing NotYetImplementedExceptions, so that subclasses only need to overwrite them in case they actually support adding/removing of children and anchorages.

Leaving as fixed in 3.11.0 M4.
Comment 3 Alexander Nyßen CLA 2015-11-13 04:41:45 EST
And further the following ones:

- Removed index from removeContentChild() and doRemoveContentChild() methods.
- Added defensive code in addContentChild(), removeContentChild(), attachToContentAnchorage(), and detachFromContentAnchorage() to ensure doXXX() methods.
- Fixed logo example to properly follow contract. 

Leaving as fixed in 3.11.0 M4.