Changeset 12069 in josm

Timestamp:

2017-05-06T14:10:40+02:00 (8 years ago)

Author:

michael2402

Message:

Fix #14737: Preserve selection order and add comments that it is ordered. Add unit tests.

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/osm/DataSelectionListener.java (modified) (5 diffs)
• src/org/openstreetmap/josm/data/osm/DataSet.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/data/osm/DataSetTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/data/osm/DataSelectionListener.java

@@ -4 +4 @@
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -39 +40 @@
 
         /**
-         * Gets the new selection
+         * Gets the new selection. New elements are added to the end of the collection.
          * <p>
          * This collection cannot be modified and will not change.
@@ -175 +176 @@
                 this.current = this.getOldSelection();
             } else {
-                this.current = new HashSet<>(old);
+                this.current = new LinkedHashSet<>(old);
                 this.current.addAll(add);
             }
@@ -217 +218 @@
                 this.current = this.getOldSelection();
             } else {
-                HashSet<OsmPrimitive> currentSet = new HashSet<>(old);
+                HashSet<OsmPrimitive> currentSet = new LinkedHashSet<>(old);
                 currentSet.removeAll(remove);
                 current = Collections.unmodifiableSet(currentSet);
@@ -257 +258 @@
         public SelectionToggleEvent(DataSet source, Set<OsmPrimitive> old, Stream<OsmPrimitive> toToggle) {
             super(source, old);
-            HashSet<OsmPrimitive> currentSet = new HashSet<>(old);
-            HashSet<OsmPrimitive> removeSet = new HashSet<>();
-            HashSet<OsmPrimitive> addSet = new HashSet<>();
+            HashSet<OsmPrimitive> currentSet = new LinkedHashSet<>(old);
+            HashSet<OsmPrimitive> removeSet = new LinkedHashSet<>();
+            HashSet<OsmPrimitive> addSet = new LinkedHashSet<>();
             toToggle.forEach(p -> {
                 if (currentSet.remove(p)) {

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -177 +177 @@
     /**
      * The current selected primitives. This is always a unmodifiable set.
+     *
+     * The set should be ordered in the order in which the primitives have been added to the selection.
      */
     private Set<OsmPrimitive> currentSelectedPrimitives = Collections.emptySet();
@@ -705 +707 @@
      * in this dataset, except deleted ones. May be empty, but not null.
      *
+     * When iterating through the set it is ordered by the order in which the primitives were added to the selection.
+     *
      * @return unmodifiable collection of primitives
      */
@@ -714 +718 @@
      * Replies an unmodifiable collection of primitives currently selected
      * in this dataset, including deleted ones. May be empty, but not null.
+     *
+     * When iterating through the set it is ordered by the order in which the primitives were added to the selection.
      *
      * @return unmodifiable collection of primitives

trunk/test/unit/org/openstreetmap/josm/data/osm/DataSetTest.java

@@ -5 +5 @@
 import static org.junit.Assert.assertTrue;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -124 +125 @@
         ds.unlinkNodeFromWays(n2);
     }
+
+    /**
+     * Test the selection order.
+     * See <a href="https://josm.openstreetmap.de/ticket/14737">#14737</a>
+     * @since 12069
+     */
+    @Test
+    public void testSelectionOrderPreserved() {
+        final DataSet ds = new DataSet();
+        Node n1 = new Node(1);
+        Node n2 = new Node(2);
+        Node n3 = new Node(3);
+        ds.addPrimitive(n1);
+        ds.addPrimitive(n2);
+        ds.addPrimitive(n3);
+
+        assertEquals(Arrays.asList(), new ArrayList<>(ds.getSelected()));
+
+        ds.setSelected(n1.getPrimitiveId(), n2.getPrimitiveId());
+        assertEquals(Arrays.asList(n1, n2), new ArrayList<>(ds.getSelected()));
+
+        ds.clearSelection();
+        assertEquals(Arrays.asList(), new ArrayList<>(ds.getSelected()));
+
+        ds.addSelected(n3.getPrimitiveId());
+        ds.addSelected(n1.getPrimitiveId(), n2.getPrimitiveId());
+        assertEquals(Arrays.asList(n3, n1, n2), new ArrayList<>(ds.getSelected()));
+
+        ds.addSelected(n3.getPrimitiveId());
+        assertEquals(Arrays.asList(n3, n1, n2), new ArrayList<>(ds.getSelected()));
+
+        ds.clearSelection(n1.getPrimitiveId());
+        assertEquals(Arrays.asList(n3, n2), new ArrayList<>(ds.getSelected()));
+
+        ds.toggleSelected(n1.getPrimitiveId());
+        assertEquals(Arrays.asList(n3, n2, n1), new ArrayList<>(ds.getSelected()));
+
+        ds.toggleSelected(n2.getPrimitiveId());
+        assertEquals(Arrays.asList(n3, n1), new ArrayList<>(ds.getSelected()));
+
+    }
 }