Fixed Issue #169 - ObservableBuffer "update" notification was ignored…

… (not detectedd and not reported).

scalafx/src/main/scala/scalafx/collections/ObservableBuffer.scala

@@ -24,6 +24,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+
 package scalafx.collections
 
 import java.{util => ju}
@@ -74,6 +75,20 @@ object ObservableBuffer extends SeqFactory[ObservableBuffer] {
    * Trait that indicates a Change in an $OB. It is a simpler version of JavaFX's
    * [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html `ListChangeListener.Change`]],
    * where each subclass indicates a specific change operation.
+   * Unlike JavaFX, all subcalsses are exclusive to each other. This enables using pattern matching:
+   * {{{
+   * items.onChange((_, changes) => {
+   *   for (change <- changes)
+   *     change match {
+   *       case Add(pos, added)                => ...
+   *       case Remove(pos, removed)           => ...
+   *       case Reorder(from, to, permutation) => ...
+   *       case Update(pos, updated)           => ...
+   *     }
+   * })
+   * }}}
+   *
+   * "replace" is represented as two changes `Remove` and `Add`.
    */
   sealed trait Change
 
@@ -82,6 +97,8 @@ object ObservableBuffer extends SeqFactory[ObservableBuffer] {
    *
    * @param position Position from where new elements were added
    * @param added elements added
+   *
+   * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#wasUpdated() `ListChangeListener.Change.wasAdded()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getFrom() `ListChangeListener.Change.getFrom()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getAddedSubList() `ListChangeListener.Change.getAddedSubList()`]]
    */
@@ -92,6 +109,8 @@ object ObservableBuffer extends SeqFactory[ObservableBuffer] {
    *
    * @param position Position from where elements were removed
    * @param removed elements removed
+   *
+   * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#wasUpdated() `ListChangeListener.Change.wasRemoved()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getFrom() `ListChangeListener.Change.getFrom()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getRemoved() `ListChangeListener.Change.getRemoved()`]]
    */
@@ -104,12 +123,26 @@ object ObservableBuffer extends SeqFactory[ObservableBuffer] {
    * @param end The end of the change interval.
    * @param permutation Function thst indicates the permutation that happened. The argument indicates the old index
    *                    that contained the element prior to this change. Its return is the new index of the same element.
+   *
+   * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#wasUpdated() `ListChangeListener.Change.wasPermutated()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getFrom() `ListChangeListener.Change.getFrom()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getTo() `ListChangeListener.Change.getTo()`]]
    * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getPermutation(int) `ListChangeListener.Change.getPermutation(int)`]]
    */
   case class Reorder(start: Int, end: Int, permutation: (Int => Int)) extends Change
 
+  /**
+   * Indicates an Update in an $OB.
+   *
+   * @param position Position from where new elements were updated
+   * @param updated elements updated
+   * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#wasUpdated() `ListChangeListener.Change.wasUpdated()`]]
+   * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getFrom() `ListChangeListener.Change.getFrom()`]]
+   * @see [[http://docs.oracle.com/javafx/2/api/javafx/collections/ListChangeListener.Change.html#getAddedSubList() `ListChangeListener.Change.getAddedSubList()`]]
+   */
+  case class Update[T](position: Int, updated: Traversable[T]) extends Change
+
+
   // CHANGING INDICATORS - END
 
   // CREATION METHODS - BEGIN
@@ -492,7 +525,7 @@ class ObservableBuffer[T](override val delegate: jfxc.ObservableList[T] = jfxc.F
    * @param typeTag information about if this type is a `Comparable` subclass or not.
    */
   def sort()(implicit typeTag: WeakTypeTag[T]) {
-    if(typeTag.tpe <:< typeOf[Comparable[_]]) {
+    if (typeTag.tpe <:< typeOf[Comparable[_]]) {
       jfxc.FXCollections.sort(delegate, new ju.Comparator[T] {
         def compare(p1: T, p2: T) = p1.asInstanceOf[Comparable[T]].compareTo(p2)
       })
@@ -529,16 +562,19 @@ class ObservableBuffer[T](override val delegate: jfxc.ObservableList[T] = jfxc.F
       def onChanged(c: jfxc.ListChangeListener.Change[_ <: T1]) {
         var changes = ArrayBuffer.empty[Change]
         while (c.next()) {
-          if (c.wasRemoved()) {
-            changes += Remove(c.getFrom, c.getRemoved)
-          }
-          if (c.wasAdded()) {
-            changes += Add(c.getFrom, c.getAddedSubList)
-          }
           if (c.wasPermutated()) {
             changes += Reorder(c.getFrom, c.getTo, {
               x => c.getPermutation(x)
             })
+          } else if (c.wasUpdated()) {
+            changes += Update(c.getFrom, c.getAddedSubList)
+          } else {
+            if (c.wasRemoved()) {
+              changes += Remove(c.getFrom, c.getRemoved)
+            }
+            if (c.wasAdded()) {
+              changes += Add(c.getFrom, c.getAddedSubList)
+            }
           }
         }
         op(ObservableBuffer.this, changes)

scalafx/src/test/scala/issues/issue169/Example1App.scala

@@ -32,7 +32,6 @@ import scalafx.collections.ObservableBuffer
 /**
  * Based on example case from reporter `scalasolist` [[https://github.com/scalafx/scalafx/issues/169#issuecomment-67260390]]:
  *
- *
  * I expect that update method would generate update change,
  * but javafx generated instead pair of delete-remove changes. It differs from its own documentation.
  * Theoretically I still can create delegate object that would generate such change.
@@ -47,11 +46,12 @@ object Example1App extends App {
         case ObservableBuffer.Add(_, _)        => println(s"  case Add    : $change")
         case ObservableBuffer.Remove(_, _)     => println(s"  case Remove : $change")
         case ObservableBuffer.Reorder(_, _, _) => println(s"  case Reorder: $change")
+        case ObservableBuffer.Update(_, _) => println(s"  case Update: $change")
       }
   })
 
-  println("items.append(\"test\")")
-  items.append("test")
+  println("items += \"test\"")
+  items += "test"
   println("items(0) = \"update\"")
   items(0) = "update"
 }

scalafx/src/test/scala/issues/issue169/Example2App.scala

@@ -27,7 +27,7 @@
 
 package issues.issue169
 
-import javafx.collections.ListChangeListener
+import javafx.{collections => jfxc}
 
 import scalafx.collections.ObservableBuffer
 
@@ -44,8 +44,8 @@ import scalafx.collections.ObservableBuffer
 object Example2App extends App {
   val items: ObservableBuffer[String] = ObservableBuffer()
 
-  val listener = new ListChangeListener[String] {
-    def onChanged(change: ListChangeListener.Change[_ <: String]) {
+  val listener = new jfxc.ListChangeListener[String] {
+    def onChanged(change: jfxc.ListChangeListener.Change[_ <: String]) {
       println(change)
       var order = 0
       while (change.next()) {

scalafx/src/test/scala/issues/issue169/Example3App.scala

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2011-2014, ScalaFX Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of the ScalaFX Project nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE SCALAFX PROJECT OR ITS CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package issues.issue169
+
+import javafx.{beans => jfxb, collections => jfxc}
+
+import scalafx.Includes._
+import scalafx.collections.ObservableBuffer
+
+/**
+ * Example of `Update` notification. Based on example for Issue #169:
+ * [[https://github.com/scalafx/scalafx/issues/169#issuecomment-67577800]]
+ *
+ */
+object Example3App extends App {
+
+  val items: ObservableBuffer[jfxc.ObservableList[String]] = new ObservableBuffer(
+    jfxc.FXCollections.observableArrayList[jfxc.ObservableList[String]]((elem: jfxc.ObservableList[String]) => Array[jfxb.Observable](elem)))
+
+  items.onChange((_, changes) => {
+    println(s"onChange(_, $changes")
+    for (change <- changes)
+      change match {
+        case ObservableBuffer.Add(_, _)        => println(s"  case Add: $change")
+        case ObservableBuffer.Remove(_, _)     => println(s"  case Remove: $change")
+        case ObservableBuffer.Reorder(_, _, _) => println(s"  case Reorder: $change")
+        case ObservableBuffer.Update(_, _)     => println(s"  case Update: $change")
+      }
+  })
+
+  // Should produce `Add` notification
+  println("items += ObservableBuffer(\"test\")")
+  items += ObservableBuffer("test")
+  println("Items: " + items)
+  println()
+
+  // Should produce `Update` notification
+  println("items(0) += \"update\"")
+  items(0) += "update"
+  println("Items: " + items)
+}

scalafx/src/test/scala/scalafx/collections/ObservableBufferSpec.scala

@@ -28,15 +28,15 @@
 package scalafx.collections
 
 import java.{util => ju}
-import javafx.{collections => jfxc}
+import javafx.{beans => jfxb, collections => jfxc}
 
 import org.junit.runner.RunWith
 import org.scalatest.Matchers._
 import org.scalatest.junit.JUnitRunner
 
 import scala.collection.mutable.Buffer
 import scalafx.Includes._
-import scalafx.collections.ObservableBuffer.{Add, Remove, Reorder, observableBuffer2ObservableList}
+import scalafx.collections.ObservableBuffer._
 import scalafx.testutil.SimpleSFXDelegateSpec
 
 /**
@@ -150,7 +150,7 @@ class ObservableBufferSpec[T]
           case (List(Add(position, elements))) =>
             changeCount += 1
             position should be(expectedPosition)
-            elements should have size (1)
+            elements should have size 1
             elements.toSeq(0) should equal(addedElement)
           case _                               => fail("Unexpected changes: " + changes)
         }
@@ -228,7 +228,7 @@ class ObservableBufferSpec[T]
           case (List(Remove(position, elements))) =>
             changeCount += 1
             position should be(expectedPosition)
-            elements should have size (1)
+            elements should have size 1
             elements.toSeq(0) should equal(removedElement)
           case _                                  => fail("Unexpected changes: " + changes)
         }
@@ -611,6 +611,8 @@ class ObservableBufferSpec[T]
             val p = Buffer.empty[(Int, Int)]
             (start until end).foreach(i => p += ((i, permutation(i))))
             permutations += p
+          case Update(pas, updated) => println(s"  case Update: $change")
+
         }
       }
     }
@@ -632,4 +634,42 @@ class ObservableBufferSpec[T]
     permutations(0).toList should equal(List((0, 3), (1, 2), (2, 5), (3, 0), (4, 1), (5, 4)))
   }
 
+  it should "not ignore updates (Issue #169)" in {
+
+    type ElementType = jfxc.ObservableList[String]
+
+    val items = new ObservableBuffer(jfxc.FXCollections.observableArrayList[ElementType]((elem: ElementType) => Array[jfxb.Observable](elem)))
+
+    items.append(jfxc.FXCollections.observableArrayList("test"))
+
+    var expectedPosition = -1
+    var changed = false
+    items.onChange((obs, changes) => {
+      changed = true
+      for (change <- changes)
+        change match {
+          case ObservableBuffer.Update(position, update) =>
+            position should be(expectedPosition)
+            val list = update.toList
+            list.length should equal(1)
+            list(0) match {
+              case e: ElementType =>
+                e.length should equal(2)
+                e(0) should equal("test")
+                e(1) should equal("update")
+              case _@t            => fail("Wrong list type: " + t)
+            }
+
+          case _@otherChange => fail("Wrong change: " + otherChange.toString)
+        }
+    })
+
+    expectedPosition = 0
+    items(0) += "update"
+
+    changed should be(true)
+
+
+  }
+
 }