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

Bug 321983

Summary:

[DataBinding] Identical instances of DecoratingObservableCollection should always be equal()

Product:

[Eclipse Project] Platform

Reporter:

Anthony Juckel <ajuckel>

Component:

Assignee:

Matthew Hall <qualidafial>

Status:

RESOLVED FIXED

QA Contact:

Matthew Hall <qualidafial>

Severity:

normal

Priority:

Version:

4.0

Target Milestone:

---

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Patch	none

Description Anthony Juckel

2010-08-06 09:24:57 EDT

Build Identifier: 

I have an issue where I'm asking a DecoratingObservableCollection whether it equals() itself (when refreshing the input on a TableViewer), and it's reporting no.  The implementation currently delegates the equals method to the decorated object, but of course does not attempt to unwrap the parameter to equals.  The decorated list (a HibernatePersistableEList) does a simple object identity check, which fails because it's comparing the wrapped list with the wrapper.

This simple patch would fix this case, and I can't see any harm.  I'm still hazy on the auto-tracking of getters, so I'm not sure if I should avoid calling getterCalled() unless we actually delegate the call.

       public boolean equals(Object o) {
               getterCalled();
               if (this == o)
                       return true;
               return decorated.equals(o);
       }


Reproducible: Always

Comment 1 Anthony Juckel

2010-08-12 22:26:53 EDT

I've added a test case to my databinding sample project at github (http://github.com/ajuckel/org.example.emfdb/commit/b533ae1fd6b9860423927878d45ae6519e9a1b84).

As an alternative test, we could update StructuredViewer#equals(Object, Object) to be:

protected boolean equals(Object elementA, Object elementB) {
	if (elementA == elementB) {
		return true;
	} else if (elementA == null) {
		return false;
	} else if (comparer == null) {
		return elementA.equals(elementB);
	} else {
		return comparer.equals(elementA, elementB);
	}
}

The elementA == elementB test captures both the previously tested both-are-null case, as well as my desire for identical objects to always report equality.

Comment 2 Matthew Hall

2010-08-19 00:54:48 EDT

This is a safe change, I'll go ahead and push this.

(In reply to comment #1)
> As an alternative test, we could update StructuredViewer#equals(Object, Object)
> to be:
> 
> protected boolean equals(Object elementA, Object elementB) {
> if (elementA == elementB) {
> return true;
> } else if (elementA == null) {
> return false;
> } else if (comparer == null) {
> return elementA.equals(elementB);
> } else {
> return comparer.equals(elementA, elementB);
> }
> }

One workaround would be to call StructuredViewer.setComparer(IElementComparer), and provide a comparer that does what you've outlined above.

Comment 3 Matthew Hall

2010-08-19 00:59:07 EDT

Created attachment 176957 [details]
Patch

Comment 4 Matthew Hall

2010-08-19 01:00:40 EDT

Released to HEAD > 20100818