groovy
  1. groovy
  2. GROOVY-4699

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

        Hide
        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
        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
        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
        Paul King added a comment -

        Patch applied

        Show
        Paul King added a comment - Patch applied

          People

          • Assignee:
            Paul King
            Reporter:
            Simon Harrer
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: