Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8208758

ListChangeBuilder doesn't shift indices after idempotent change pairs

    Details

    • Subcomponent:
    • CPU:
      x86_64
    • OS:
      generic

      Description

      ADDITIONAL SYSTEM INFORMATION :
      C:\Users\hwaite>ver

      Microsoft Windows [Version 10.0.16299.547]

      C:\Users\hwaite>"%JAVA_HOME%"\bin\java -version
      java version "1.8.0_181"
      Java(TM) SE Runtime Environment (build 1.8.0_181-b13)
      Java HotSpot(TM) 64-Bit Server VM (build 25.181-b13, mixed mode)

      A DESCRIPTION OF THE PROBLEM :
      Offsetting changes (e.g. an add and remove at same index) are excluded from ListChangeBuilder-spawned events. If a list change occurs after index of the first event in an idempotent pair, the sandwiched event will use incorrect index.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      1. Subclass ObservableListBase
      2. add a couple items
      3. begin change
      4. add item at index 0
      5. remove item at index 2
      6. remove item at index 0
      7. end change

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Llist change event with a single removal at index 1
      ACTUAL -
      List event with a single removal at index 2

      ---------- BEGIN SOURCE ----------
      import com.sun.javafx.collections.ObservableListWrapper;
      import javafx.collections.ListChangeListener;
      import org.junit.Assert;
      import org.junit.Ignore;
      import org.junit.Test;

      import java.util.ArrayList;
      import java.util.Arrays;
      import java.util.List;

      /**
       * Expose JavaFX {@linkplain javafx.collections.ListChangeBuilder} bugs.
       * @author hollisw
       */
      @Ignore
      public class ListChangeBuilderTest {
          /**
           * Expose {@linkplain ObservableListWrapper#beginChange()} and
           * {@linkplain ObservableListWrapper#endChange()}.
           */
          private static class ExposedObservableList<E> extends ObservableListWrapper<E> {
              ExposedObservableList(List<E> list) {super(list);}
              public void doBeginChange() {beginChange();}
              public void doEndChange() {endChange();}
          }

          /**
           * An element is added on index 0, then it is removed. A change event is not fired for
           * these changes, because no real change was made to the list. However, this is not taken
           * into account when calculating the indices of the removed elements on higher positions,
           * when the removals occur between the add and remove on same index. When a removal
           * occurs on an index for which an addition already occurred in the same commit,
           * all removals on higher positions should be recalculated.
           */
          @Test
          public void testAddRemoveOnSameIndex() {
              // [1, 2]
              final ExposedObservableList<Integer> list =
               new ExposedObservableList<>(new ArrayList<>(Arrays.asList(1, 2)));

              list.addListener(
                  (ListChangeListener<? super Integer>) change -> {
                      change.next();
                      Assert.assertEquals(1, change.getRemovedSize());
                      Assert.assertEquals(1, change.getFrom());
                      Assert.assertFalse(change.wasAdded());
                      Assert.assertFalse(change.next());
                  }
              );

              list.doBeginChange();

              // [0, 1, 2]
              list.add(0, 0);

              // [0, 1]
              list.remove(2);

              // [1]
              list.remove(0);
              list.doEndChange();
          }

          /** @see #testAddRemoveOnSameIndex() */
          @Test
          public void testAddRemoveOnSameIndex2() {
              // [1, 2]
              final ExposedObservableList<Integer> list =
               new ExposedObservableList<>(new ArrayList<>(Arrays.asList(1, 2)));

              list.addListener(
                  (ListChangeListener<? super Integer>) change -> {
                      change.next();
                      Assert.assertEquals(1, change.getAddedSize());
                      Assert.assertEquals(2, change.getFrom());
                      Assert.assertFalse(change.wasRemoved());
                      Assert.assertFalse(change.next());
                  }
              );

              list.doBeginChange();

              // [0, 1, 2]
              list.add(0, 0);

              // [0, 1, 2, 3]
              list.add(3);

              // [1, 2, 3]
              list.remove(0);

              list.doEndChange();
          }

          /**
           * An element is added on index 2, elements are removed on index 0 and 1 - the removal on
           * index 1 actually removes the element added on index 2, so a change event is not fired
           * for the addition on index 2 and the removal on index 1, because the element was added
           * and removed in the same commit. However this is not taken into account when
           * calculating the indexes of the removed elements on higher positions, when the removals
           * occur between the addition and removal of the same element in the same commit - all
           * removals on higher positions should be recalculated.
           */
          @Test
          public void testIndirectAddRemoveOnSameIndex() {
              // [1, 2, 3, 4]
              final ExposedObservableList<Integer> list =
               new ExposedObservableList<>(new ArrayList<>(Arrays.asList(1, 2, 3, 4)));

              list.addListener(
                  (ListChangeListener.Change<? extends Integer> change) -> {
                      change.next();
                      Assert.assertEquals(1, change.getRemovedSize());
                      Assert.assertEquals(0, change.getFrom());
                      Assert.assertFalse(change.wasAdded());

                      change.next();
                      Assert.assertEquals(1, change.getRemovedSize());
                      Assert.assertEquals(2, change.getFrom());
                      Assert.assertFalse(change.wasAdded());

                      Assert.assertFalse(change.next());
                  }
              );

              list.doBeginChange();

              list.add(2, 6);
              list.remove(4);
              list.remove(0);
              list.remove(1);
              list.doEndChange();
          }

          /** @see #testIndirectAddRemoveOnSameIndex() */
          @Test
          public void testIndirectAddRemoveOnSameIndex2() {
              final ExposedObservableList<Integer> list =
               new ExposedObservableList<>(new ArrayList<>(Arrays.asList(1, 2, 3, 4)));

              list.addListener(
                  (ListChangeListener.Change<? extends Integer> change) -> {
                      change.next();
                      Assert.assertEquals(1, change.getRemovedSize());
                      Assert.assertEquals(0, change.getFrom());
                      Assert.assertFalse(change.wasAdded());

                      change.next();
                      Assert.assertEquals(1, change.getAddedSize());
                      Assert.assertEquals(2, change.getFrom());
                      Assert.assertFalse(change.wasRemoved());

                      Assert.assertFalse(change.next());
                  }
              );

              list.doBeginChange();
              list.add(2, 6);
              list.add(4, 5);
              list.remove(0);
              list.remove(1);
              list.doEndChange();
          }
      }
      ---------- END SOURCE ----------

      FREQUENCY : always


        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              webbuggrp Webbug Group
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: