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

Bug 317358

Summary:

[otmodel] deltas for roles are not correctly computed

Product:

[Tools] Objectteams

Reporter:

Stephan Herrmann <stephan.herrmann>

Component:

OTDT

Assignee:

Stephan Herrmann <stephan.herrmann>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

Version:

0.7

Target Milestone:

0.7

Hardware:

OS:

Linux

Whiteboard:

Attachments:

Description	Flags
proposed implementation	none
patch resolving one FIXME	none

Description Stephan Herrmann

2010-06-19 09:23:26 EDT

Fup of bug 317352:

Stepping through RoleBindingChangedListener.updateCallinMarkers(..) I noticed
that the deltas mentioned all (unchanged) method mappings as added and failed
to mention a regular method that was actually added.

Example used: edit roles of CompletionAdaptor.

Comment 1 Stephan Herrmann

2010-06-29 03:03:04 EDT

Created attachment 172986 [details]
proposed implementation

Analysis revealed that the delta builder requires that the JavaModelManager
has infos for all elements to be analyzed, incl. method mappings.

While implementing corresponding support in CompilationUnitStructureRequestor
(method createCallinInfo) it proved unfortunate that OTJavaElements are not
JavaElements.

Consequently, I re-rooted OTJavaElement to make it a subclass of Member.
This allowed me to remove *many* workarounds where JavaElement and OTJavaElement
had to be treated differently.

Remaining FIXMEs in:
 - JavaElementDeltaBuilder#findContentChange(..): compare 
   SourceMethodMappingInfo to detect content changes
 - JavaElementDeltaBuilder#recordElementInfo(..): is instanceof still needed
 - OTType#createElementInfo : unimplemented


Further fix:
 - don't let CallinMapping#equals rely on equal callin names if these are
   generated (enclosed in '<' '>') (needed to avoid unwanted deltas)

Tests for the original issue are in 
  org.eclipse.objectteams.otdt.tests.otmodel.JavaElementDeltaTests
where testAddCallinToRole2 indeed succeeded in reproducing the issue.

Comment 2 Stephan Herrmann

2010-06-29 09:09:56 EDT

Changes have been committed as r543 & r544.
Tests and actual usage of build 201006290324 show now issues to this point.

Leaving only the 3 FIXMEs from comment 1 to be resolved before the release.

Comment 3 Stephan Herrmann

2010-06-29 17:19:57 EDT

Created attachment 173049 [details]
patch resolving one FIXME

(In reply to comment #1)
> Remaining FIXMEs in:
>  - JavaElementDeltaBuilder#findContentChange(..): compare 
>    SourceMethodMappingInfo to detect content changes

This patch implements the distinction of two levels of comparing
method mappings:
 * top-level comparison using the equals method of the JavaElement
 * fine-grained comparison in findContentChange based on SourceMethodMappingInfo

Actually, CallinMapping.equals(..) had to be made smarter for this purpose.

Also extend the solution to callouts, too.

Includes 4 new tests.

Comment 4 Stephan Herrmann

2010-06-29 17:24:40 EDT

(In reply to comment #1)
> Remaining FIXMEs in:
>  - OTType#createElementInfo : unimplemented

I assume that OTTypes are never created directly (i.e., in not-open state).
Thus implementation of this method should not be needed. If it still
happens we will be alerted by the thrown UnsupportedOperationException.

Comment 5 Stephan Herrmann

2010-06-29 18:22:50 EDT

(In reply to comment #1)
> Remaining FIXMEs in:
>  - JavaElementDeltaBuilder#recordElementInfo(..): is instanceof still needed

This check has been removed together with a few more of its kind in r549
(after neither tests nor runtime workbench could trigger that branch).

This closes this issue.

Comment 6 Stephan Herrmann

2010-07-06 07:59:56 EDT

Verified using build 201007050931 by inspecting and running the tests,
as well as interactively observing updates in the Outline view.