Index: /trunk/src/org/openstreetmap/josm/data/SelectionChangedListener.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/SelectionChangedListener.java (revision 12097) +++ /trunk/src/org/openstreetmap/josm/data/SelectionChangedListener.java (revision 12098) @@ -4,5 +4,7 @@ import java.util.Collection; +import org.openstreetmap.josm.data.osm.DataSelectionListener; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.event.SelectionEventManager; /** @@ -14,4 +16,7 @@ * Swing event queue and packed together. So only one selection changed event * is issued within a one message dispatch routine. + * + * @see DataSelectionListener For a more advanced listener class. + * @see SelectionEventManager For managing your selection events. * * @author imi Index: /trunk/src/org/openstreetmap/josm/data/osm/event/SelectionEventManager.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/event/SelectionEventManager.java (revision 12097) +++ /trunk/src/org/openstreetmap/josm/data/osm/event/SelectionEventManager.java (revision 12098) @@ -2,10 +2,8 @@ package org.openstreetmap.josm.data.osm.event; -import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.stream.Stream; @@ -17,5 +15,4 @@ import org.openstreetmap.josm.data.osm.DataSelectionListener; import org.openstreetmap.josm.data.osm.DataSet; -import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.event.DatasetEventManager.FireMode; import org.openstreetmap.josm.gui.layer.MainLayerManager; @@ -45,9 +42,18 @@ } - private static class ListenerInfo { + private abstract static class AbstractListenerInfo { + abstract void fire(SelectionChangeEvent event); + } + + private static class ListenerInfo extends AbstractListenerInfo { private final SelectionChangedListener listener; ListenerInfo(SelectionChangedListener listener) { this.listener = listener; + } + + @Override + void fire(SelectionChangeEvent event) { + listener.selectionChanged(event.getSelection()); } @@ -66,7 +72,32 @@ } - private Collection selection; - private final CopyOnWriteArrayList inEDTListeners = new CopyOnWriteArrayList<>(); - private final CopyOnWriteArrayList normalListeners = new CopyOnWriteArrayList<>(); + private static class DataListenerInfo extends AbstractListenerInfo { + private final DataSelectionListener listener; + + DataListenerInfo(DataSelectionListener listener) { + this.listener = listener; + } + + @Override + void fire(SelectionChangeEvent event) { + listener.selectionChanged(event); + } + + @Override + public int hashCode() { + return Objects.hash(listener); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DataListenerInfo that = (DataListenerInfo) o; + return Objects.equals(listener, that.listener); + } + } + + private final CopyOnWriteArrayList inEDTListeners = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList immedatelyListeners = new CopyOnWriteArrayList<>(); /** @@ -82,4 +113,6 @@ /** * Registers a new {@code SelectionChangedListener}. + * + * It is preferred to add a DataSelectionListener - that listener will receive more information about the event. * @param listener listener to add * @param fireMode Set this to IN_EDT_CONSOLIDATED if you want the event to be fired in the EDT thread. @@ -92,6 +125,25 @@ inEDTListeners.addIfAbsent(new ListenerInfo(listener)); } else { - normalListeners.addIfAbsent(new ListenerInfo(listener)); - } + immedatelyListeners.addIfAbsent(new ListenerInfo(listener)); + } + } + + /** + * Adds a selection listener that gets notified for selections immediately. + * @param listener The listener to add. + * @since 12098 + */ + public void addSelectionListener(DataSelectionListener listener) { + immedatelyListeners.addIfAbsent(new DataListenerInfo(listener)); + } + + /** + * Adds a selection listener that gets notified for selections later in the EDT thread. + * Events are sent in the right order but may be delayed. + * @param listener The listener to add. + * @since 12098 + */ + public void addSelectionListenerForEdt(DataSelectionListener listener) { + inEDTListeners.addIfAbsent(new DataListenerInfo(listener)); } @@ -101,7 +153,19 @@ */ public void removeSelectionListener(SelectionChangedListener listener) { - ListenerInfo searchListener = new ListenerInfo(listener); + remove(new ListenerInfo(listener)); + } + + /** + * Unregisters a {@code DataSelectionListener}. + * @param listener listener to remove + * @since 12098 + */ + public void removeSelectionListener(DataSelectionListener listener) { + remove(new DataListenerInfo(listener)); + } + + private void remove(AbstractListenerInfo searchListener) { inEDTListeners.remove(searchListener); - normalListeners.remove(searchListener); + immedatelyListeners.remove(searchListener); } @@ -130,22 +194,14 @@ @Override - public void selectionChanged(SelectionChangeEvent e) { - Set newSelection = e.getSelection(); - fireEvents(normalListeners, newSelection); - selection = newSelection; - SwingUtilities.invokeLater(edtRunnable); - } - - private static void fireEvents(List listeners, Collection newSelection) { - for (ListenerInfo listener: listeners) { - listener.listener.selectionChanged(newSelection); - } - } - - private final Runnable edtRunnable = () -> { - if (selection != null) { - fireEvents(inEDTListeners, selection); - } - }; + public void selectionChanged(SelectionChangeEvent event) { + fireEvent(immedatelyListeners, event); + SwingUtilities.invokeLater(() -> fireEvent(inEDTListeners, event)); + } + + private static void fireEvent(List listeners, SelectionChangeEvent event) { + for (AbstractListenerInfo listener: listeners) { + listener.fire(event); + } + } /** @@ -155,5 +211,5 @@ public void resetState() { inEDTListeners.clear(); - normalListeners.clear(); + immedatelyListeners.clear(); Main.getLayerManager().addAndFireActiveLayerChangeListener(this); } Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/event/SelectionEventManagerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/event/SelectionEventManagerTest.java (revision 12097) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/event/SelectionEventManagerTest.java (revision 12098) @@ -3,9 +3,9 @@ import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; +import java.util.List; import org.junit.Rule; @@ -14,6 +14,8 @@ import org.openstreetmap.josm.command.CommandTest.CommandTestDataWithRelation; import org.openstreetmap.josm.data.SelectionChangedListener; +import org.openstreetmap.josm.data.osm.DataSelectionListener; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.event.DatasetEventManager.FireMode; +import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.testutils.JOSMTestRules; @@ -26,10 +28,20 @@ */ public class SelectionEventManagerTest { - private final class SelectionListener implements SelectionChangedListener { + private final class SelectionListener implements SelectionChangedListener, DataSelectionListener { private Collection newSelection; + private final String name; + + public SelectionListener(String name) { + this.name = name; + } @Override public void selectionChanged(Collection newSelection) { this.newSelection = newSelection; + } + + @Override + public void selectionChanged(SelectionChangeEvent event) { + this.newSelection = event.getSelection(); } } @@ -52,29 +64,55 @@ assertEquals(testData1.layer, Main.getLayerManager().getEditLayer()); - SelectionListener listener = new SelectionListener(); - SelectionEventManager.getInstance().addSelectionListener(listener, FireMode.IMMEDIATELY); - assertNull(listener.newSelection); + SelectionListener listener1 = new SelectionListener("IMMEDIATELY"); + SelectionListener listener2 = new SelectionListener("IN_EDT_CONSOLIDATED"); + SelectionListener listener3 = new SelectionListener("normal"); + SelectionListener listener4 = new SelectionListener("edt"); + SelectionEventManager instance = SelectionEventManager.getInstance(); + instance.addSelectionListener(listener1, FireMode.IMMEDIATELY); + instance.addSelectionListener(listener2, FireMode.IN_EDT_CONSOLIDATED); + instance.addSelectionListener(listener3); + instance.addSelectionListenerForEdt(listener4); + List listeners = Arrays.asList(listener1, listener2, listener3, listener4); + assertSelectionEquals(listeners, null); // active layer, should change testData1.layer.data.setSelected(testData1.existingNode.getPrimitiveId()); - assertEquals(new HashSet(Arrays.asList(testData1.existingNode)), listener.newSelection); + assertSelectionEquals(listeners, new HashSet(Arrays.asList(testData1.existingNode))); - listener.newSelection = null; testData1.layer.data.clearSelection(testData1.existingNode.getPrimitiveId()); - assertEquals(new HashSet(Arrays.asList()), listener.newSelection); + assertSelectionEquals(listeners, new HashSet(Arrays.asList())); - listener.newSelection = null; testData1.layer.data.addSelected(testData1.existingNode2.getPrimitiveId()); - assertEquals(new HashSet(Arrays.asList(testData1.existingNode2)), listener.newSelection); + assertSelectionEquals(listeners, new HashSet(Arrays.asList(testData1.existingNode2))); // changing to other dataset should trigger a empty selection - listener.newSelection = null; Main.getLayerManager().setActiveLayer(testData2.layer); - assertEquals(new HashSet(Arrays.asList()), listener.newSelection); + assertSelectionEquals(listeners, new HashSet(Arrays.asList())); // This should not trigger anything, since the layer is not active any more. - listener.newSelection = null; testData1.layer.data.clearSelection(testData1.existingNode.getPrimitiveId()); - assertNull(listener.newSelection); + assertSelectionEquals(listeners, null); + + testData2.layer.data.setSelected(testData2.existingNode.getPrimitiveId()); + assertSelectionEquals(listeners, new HashSet(Arrays.asList(testData2.existingNode))); + + // removal + instance.removeSelectionListener((SelectionChangedListener) listener1); + instance.removeSelectionListener((SelectionChangedListener) listener2); + instance.removeSelectionListener((DataSelectionListener) listener3); + instance.removeSelectionListener((DataSelectionListener) listener4); + + // no event triggered now + testData2.layer.data.setSelected(testData2.existingNode2.getPrimitiveId()); + assertSelectionEquals(listeners, null); + } + + private void assertSelectionEquals(List listeners, Object should) { + // sync + GuiHelper.runInEDTAndWait(() -> {}); + for (SelectionListener listener : listeners) { + assertEquals(listener.name, should, listener.newSelection); + listener.newSelection = null; + } }