groovy

Observable List misbehaves when using retainAll with closure

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.7.5
  • Fix Version/s: 1.8.2, 1.9-beta-3, 1.7.11
  • Component/s: groovy-jdk
  • Labels:
    None
  • Environment:
    Eclipse Plugin, Windows 7
  • Testcase included:
    yes
  • Number of attachments :
    0

Description

List liste = new ObservableList<String>()

liste.add "test"
liste.add "test2"

List andereListe = new LinkedList<String>()

liste.retainAll

{ elem -> andereListe.contains(elem) }

assert liste.isEmpty()
____________________________________________

This results in:
____________________________________________
Exception in thread "main" Assertion failed:

assert liste.isEmpty()

 
false
[test]
____________________________________________

If the ObservableList is changed to LinkedList, the testcase works.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Simon Harrer added a comment -

workaround found: liste.retainAll(liste.findAll

{andereListe.contains(it)}

)

Show
Simon Harrer added a comment - workaround found: liste.retainAll(liste.findAll {andereListe.contains(it)} )
Hide
Permalink
Paul King added a comment -

Wondering whether this patch would fix the problem:

Index: src/main/groovy/util/ObservableList.java
===================================================================
--- src/main/groovy/util/ObservableList.java	(revision 21606)
+++ src/main/groovy/util/ObservableList.java	(revision )
@@ -400,14 +400,19 @@
         }
 
         public void remove() {
-            ObservableList.this.remove(cursor--);
+            int oldSize = ObservableList.this.size();
+            Object element = ObservableList.this.get(cursor);
+            iterDelegate.remove();
+            fireElementRemovedEvent(cursor, element);
+            fireSizeChangedEvent(oldSize, size());
+            cursor--;
         }
     }
Show
Paul King added a comment - Wondering whether this patch would fix the problem: Index: src/main/groovy/util/ObservableList.java =================================================================== --- src/main/groovy/util/ObservableList.java (revision 21606) +++ src/main/groovy/util/ObservableList.java (revision ) @@ -400,14 +400,19 @@ } public void remove() { - ObservableList.this.remove(cursor--); + int oldSize = ObservableList.this.size(); + Object element = ObservableList.this.get(cursor); + iterDelegate.remove(); + fireElementRemovedEvent(cursor, element); + fireSizeChangedEvent(oldSize, size()); + cursor--; } }
Hide
Permalink
Tim Yates added a comment -

Added a test for it (and patched with your code)

Index: src/test/groovy/util/ObservableListTest.groovy
===================================================================
--- src/test/groovy/util/ObservableListTest.groovy	(revision 22647)
+++ src/test/groovy/util/ObservableListTest.groovy	(working copy)
@@ -264,6 +264,18 @@
       list << value2
       assertNull( contentListener.event )
    }
+
+   void testRetainBugGroovy4699() {
+      ObservableList list = []
+      list.add "test"
+      list.add "test2"
+
+      LinkedList otherList = []
+      
+      list.retainAll { elem -> otherList.contains( elem ) }
+
+      assert list.isEmpty()
+   }
 }
 
 class SampleListPropertyChangeListener implements PropertyChangeListener {

As long as the patch for GROOVY-4937 is applied prior to your patch, it seems to work fine

Show
Tim Yates added a comment - Added a test for it (and patched with your code) Index: src/test/groovy/util/ObservableListTest.groovy =================================================================== --- src/test/groovy/util/ObservableListTest.groovy (revision 22647) +++ src/test/groovy/util/ObservableListTest.groovy (working copy) @@ -264,6 +264,18 @@ list << value2 assertNull( contentListener.event ) } + + void testRetainBugGroovy4699() { + ObservableList list = [] + list.add "test" + list.add "test2" + + LinkedList otherList = [] + + list.retainAll { elem -> otherList.contains( elem ) } + + assert list.isEmpty() + } } class SampleListPropertyChangeListener implements PropertyChangeListener { As long as the patch for GROOVY-4937 is applied prior to your patch, it seems to work fine
Hide
Permalink
Paul King added a comment -

Patch applied

Show
Paul King added a comment - Patch applied

People

  • Assignee:
    Paul King
    Reporter:
    Simon Harrer
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: