Ticket #18436: 18436.1.patch

File 18436.1.patch, 24.5 KB (added by taylor.smock, 5 years ago)

Don't add duplicate DataSource entries to dataSources (DataSetMerger adds them, and then mergeFrom was adding them again). See source:trunk/src/org/openstreetmap/josm/data/osm/DataSet.java@15559:1084-1087#L1084

  • src/org/openstreetmap/josm/data/osm/AbstractDataSourceChangeEvent.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm;
     3
     4import java.util.Set;
     5
     6import org.openstreetmap.josm.data.DataSource;
     7import org.openstreetmap.josm.tools.CheckParameterUtil;
     8
     9/**
     10 * The base class for data source change events
     11 *
     12 * @author Taylor Smock
     13 * @since xxx
     14 */
     15public abstract class AbstractDataSourceChangeEvent implements DataSourceChangeEvent {
     16
     17    private DataSet source;
     18    private Set<DataSource> old;
     19
     20    /**
     21     * Create a Data Source change event
     22     *
     23     * @param source The DataSet that is originating the change
     24     * @param old    The previous set of DataSources
     25     */
     26    public AbstractDataSourceChangeEvent(DataSet source, Set<DataSource> old) {
     27        CheckParameterUtil.ensureParameterNotNull(source, "source");
     28        CheckParameterUtil.ensureParameterNotNull(old, "old");
     29        this.source = source;
     30        this.old = old;
     31    }
     32
     33    @Override
     34    public Set<DataSource> getOldDataSources() {
     35        return old;
     36    }
     37
     38    @Override
     39    public DataSet getSource() {
     40        return source;
     41    }
     42}
     43
  • src/org/openstreetmap/josm/data/osm/DataSet.java

     
    1010import java.util.HashMap;
    1111import java.util.HashSet;
    1212import java.util.Iterator;
     13import java.util.LinkedHashSet;
    1314import java.util.LinkedList;
    1415import java.util.List;
    1516import java.util.Map;
     
    4243import org.openstreetmap.josm.data.osm.event.ChangesetIdChangedEvent;
    4344import org.openstreetmap.josm.data.osm.event.DataChangedEvent;
    4445import org.openstreetmap.josm.data.osm.event.DataSetListener;
     46import org.openstreetmap.josm.data.osm.event.DataSourceAddedEvent;
     47import org.openstreetmap.josm.data.osm.event.DataSourceRemovedEvent;
    4548import org.openstreetmap.josm.data.osm.event.FilterChangedEvent;
    4649import org.openstreetmap.josm.data.osm.event.NodeMovedEvent;
    4750import org.openstreetmap.josm.data.osm.event.PrimitiveFlagsChangedEvent;
     
    165168     */
    166169    private final Collection<DataSource> dataSources = new LinkedList<>();
    167170
     171    /**
     172     * A list of listeners that listen to DataSource changes on this layer
     173     */
     174    private final ListenerList<DataSourceListener> dataSourceListeners = ListenerList.create();
     175
    168176    private final ConflictCollection conflicts = new ConflictCollection();
    169177
    170178    private short mappaintCacheIdx = 1;
     
    225233                        .map(rm -> new RelationMember(rm.getRole(), primMap.get(rm.getMember())))
    226234                        .collect(Collectors.toList()));
    227235            }
     236            DataSourceAddedEvent addedEvent = new DataSourceAddedEvent(this,
     237                    new LinkedHashSet<>(dataSources), copyFrom.dataSources.stream());
    228238            for (DataSource source : copyFrom.dataSources) {
    229239                dataSources.add(new DataSource(source));
    230240            }
     241            dataSourceListeners.fireEvent(d -> d.dataSourceChange(addedEvent));
    231242            version = copyFrom.version;
    232243            uploadPolicy = copyFrom.uploadPolicy;
    233244            downloadPolicy = copyFrom.downloadPolicy;
     
    271282     * @since 11626
    272283     */
    273284    public synchronized boolean addDataSources(Collection<DataSource> sources) {
     285        DataSourceAddedEvent addedEvent = new DataSourceAddedEvent(this,
     286                new LinkedHashSet<>(dataSources), sources.stream());
    274287        boolean changed = dataSources.addAll(sources);
    275288        if (changed) {
    276289            cachedDataSourceArea = null;
    277290            cachedDataSourceBounds = null;
    278291        }
     292        dataSourceListeners.fireEvent(d -> d.dataSourceChange(addedEvent));
    279293        return changed;
    280294    }
    281295
     
    574588        highlightUpdateListeners.removeListener(listener);
    575589    }
    576590
     591    /**
     592     * Adds a listener that gets notified whenever the data sources change
     593     *
     594     * @param listener The listener
     595     * @see #removeDataSourceListener
     596     * @see #getDataSources
     597     * @since xxx
     598     */
     599    public void addDataSourceListener(DataSourceListener listener) {
     600        dataSourceListeners.addListener(listener);
     601    }
     602
     603    /**
     604     * Removes a listener that gets notified whenever the data sources change
     605     *
     606     * @param listener The listener
     607     * @see #addDataSourceListener
     608     * @see #getDataSources
     609     * @since xxx
     610     */
     611    public void removeDataSourceListener(DataSourceListener listener) {
     612        dataSourceListeners.removeListener(listener);
     613    }
     614
    577615    @Override
    578616    public Collection<OsmPrimitive> getAllSelected() {
    579617        return currentSelectedPrimitives;
     
    10841122            new DataSetMerger(this, from).merge(progressMonitor);
    10851123            synchronized (from) {
    10861124                if (!from.dataSources.isEmpty()) {
    1087                     if (dataSources.addAll(from.dataSources)) {
     1125                    DataSourceAddedEvent addedEvent = new DataSourceAddedEvent(
     1126                            this, new LinkedHashSet<>(dataSources), from.dataSources.stream());
     1127                    DataSourceRemovedEvent clearEvent = new DataSourceRemovedEvent(
     1128                            this, new LinkedHashSet<>(from.dataSources), from.dataSources.stream());
     1129                    if (from.dataSources.stream().filter(dataSource -> !dataSources.contains(dataSource))
     1130                            .map(dataSources::add).filter(Boolean.TRUE::equals).count() > 0) {
    10881131                        cachedDataSourceArea = null;
    10891132                        cachedDataSourceBounds = null;
    10901133                    }
     
    10911134                    from.dataSources.clear();
    10921135                    from.cachedDataSourceArea = null;
    10931136                    from.cachedDataSourceBounds = null;
     1137                    dataSourceListeners.fireEvent(d -> d.dataSourceChange(addedEvent));
     1138                    from.dataSourceListeners.fireEvent(d -> d.dataSourceChange(clearEvent));
    10941139                }
    10951140            }
    10961141        }
  • src/org/openstreetmap/josm/data/osm/DataSourceChangeEvent.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm;
     3
     4import java.util.Set;
     5
     6import org.openstreetmap.josm.data.DataSource;
     7
     8/**
     9 * The event that is fired when the data source list is changed.
     10 *
     11 * @author Taylor Smock
     12 * @since xxx
     13 */
     14public interface DataSourceChangeEvent {
     15    /**
     16     * Gets the previous data source list
     17     * <p>
     18     * This collection cannot be modified and will not change.
     19     *
     20     * @return The old data source list
     21     */
     22    Set<DataSource> getOldDataSources();
     23
     24    /**
     25     * Gets the new data sources. New data sources are added to the end of the
     26     * collection.
     27     * <p>
     28     * This collection cannot be modified and will not change.
     29     *
     30     * @return The new data sources
     31     */
     32    Set<DataSource> getDataSources();
     33
     34    /**
     35     * Gets the Data Sources that have been removed from the selection.
     36     * <p>
     37     * Those are the primitives contained in {@link #getOldDataSources()} but not in
     38     * {@link #getDataSources()}
     39     * <p>
     40     * This collection cannot be modified and will not change.
     41     *
     42     * @return The DataSources that were removed
     43     */
     44    Set<DataSource> getRemoved();
     45
     46    /**
     47     * Gets the data sources that have been added to the selection.
     48     * <p>
     49     * Those are the data sources contained in {@link #getDataSources()} but not in
     50     * {@link #getOldDataSources()}
     51     * <p>
     52     * This collection cannot be modified and will not change.
     53     *
     54     * @return The data sources that were added
     55     */
     56    Set<DataSource> getAdded();
     57
     58    /**
     59     * Gets the data set that triggered this selection event.
     60     *
     61     * @return The data set.
     62     */
     63    DataSet getSource();
     64
     65    /**
     66     * Test if this event did not change anything.
     67     * <p>
     68     * This will return <code>false</code> for all events that are sent to
     69     * listeners, so you don't need to test it.
     70     *
     71     * @return <code>true</code> if this did not change the selection.
     72     */
     73    default boolean isNop() {
     74        return getAdded().isEmpty() && getRemoved().isEmpty();
     75    }
     76}
     77
  • src/org/openstreetmap/josm/data/osm/DataSourceListener.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm;
     3
     4/**
     5 * This is a listener that listens to selection change events in the data set.
     6 *
     7 * @author Taylor Smock
     8 * @since xxx
     9 */
     10@FunctionalInterface
     11public interface DataSourceListener {
     12    /**
     13     * Called whenever the data source list is changed.
     14     *
     15     * You get notified about the new data source list, the sources that were added
     16     * and removed and the dataset that triggered the event.
     17     *
     18     * @param event The data source change event.
     19     * @see DataSourceChangeEvent
     20     */
     21    void dataSourceChange(DataSourceChangeEvent event);
     22}
  • src/org/openstreetmap/josm/data/osm/event/DataSourceAddedEvent.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm.event;
     3
     4import java.util.LinkedHashSet;
     5import java.util.Set;
     6import java.util.stream.Collectors;
     7import java.util.stream.Stream;
     8
     9import org.openstreetmap.josm.data.DataSource;
     10import org.openstreetmap.josm.data.osm.AbstractDataSourceChangeEvent;
     11import org.openstreetmap.josm.data.osm.DataSet;
     12import org.openstreetmap.josm.tools.CheckParameterUtil;
     13
     14/**
     15 * There is a new data source
     16 *
     17 * @author Taylor Smock
     18 * @since xxx
     19 */
     20public class DataSourceAddedEvent extends AbstractDataSourceChangeEvent {
     21    private Set<DataSource> current;
     22    private Set<DataSource> removed;
     23    private final Set<DataSource> added;
     24
     25    /**
     26     * Create a Data Source change event
     27     *
     28     * @param source         The DataSet that is originating the change
     29     * @param old            The previous set of DataSources
     30     * @param newDataSources The data sources that are being added
     31     */
     32    public DataSourceAddedEvent(DataSet source, Set<DataSource> old, Stream<DataSource> newDataSources) {
     33        super(source, old);
     34        CheckParameterUtil.ensureParameterNotNull(newDataSources, "newDataSources");
     35        this.added = newDataSources.collect(Collectors.toCollection(LinkedHashSet::new));
     36    }
     37
     38    @Override
     39    public Set<DataSource> getDataSources() {
     40        if (current == null) {
     41            current = new LinkedHashSet<>(getOldDataSources());
     42            current.addAll(added);
     43        }
     44        return current;
     45    }
     46
     47    @Override
     48    public synchronized Set<DataSource> getRemoved() {
     49        if (removed == null) {
     50            removed = getOldDataSources().stream().filter(s -> !getDataSources().contains(s))
     51                    .collect(Collectors.toCollection(LinkedHashSet::new));
     52        }
     53        return removed;
     54    }
     55
     56    @Override
     57    public synchronized Set<DataSource> getAdded() {
     58        return added;
     59    }
     60
     61    @Override
     62    public String toString() {
     63        return "DataSourceAddedEvent [current=" + current + ", removed=" + removed + ", added=" + added + ']';
     64    }
     65}
     66
  • src/org/openstreetmap/josm/data/osm/event/DataSourceRemovedEvent.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm.event;
     3
     4import java.util.LinkedHashSet;
     5import java.util.Set;
     6import java.util.stream.Collectors;
     7import java.util.stream.Stream;
     8
     9import org.openstreetmap.josm.data.DataSource;
     10import org.openstreetmap.josm.data.osm.AbstractDataSourceChangeEvent;
     11import org.openstreetmap.josm.data.osm.DataSet;
     12import org.openstreetmap.josm.tools.CheckParameterUtil;
     13
     14/**
     15 * A data source was removed
     16 *
     17 * @author Taylor Smock
     18 * @since xxx
     19 */
     20public class DataSourceRemovedEvent extends AbstractDataSourceChangeEvent {
     21    private Set<DataSource> current;
     22    private final Set<DataSource> removed;
     23    private Set<DataSource> added;
     24
     25    /**
     26     * Create a Data Source change event
     27     *
     28     * @param source             The DataSet that is originating the change
     29     * @param old                The previous set of DataSources
     30     * @param removedDataSources The data sources that are being removed
     31     */
     32    public DataSourceRemovedEvent(DataSet source, Set<DataSource> old, Stream<DataSource> removedDataSources) {
     33        super(source, old);
     34        CheckParameterUtil.ensureParameterNotNull(removedDataSources, "removedDataSources");
     35        this.removed = removedDataSources.collect(Collectors.toCollection(LinkedHashSet::new));
     36    }
     37
     38    @Override
     39    public Set<DataSource> getDataSources() {
     40        if (current == null) {
     41            current = getOldDataSources().stream().filter(s -> !removed.contains(s))
     42                    .collect(Collectors.toCollection(LinkedHashSet::new));
     43        }
     44        return current;
     45    }
     46
     47    @Override
     48    public synchronized Set<DataSource> getRemoved() {
     49        return removed;
     50    }
     51
     52    @Override
     53    public synchronized Set<DataSource> getAdded() {
     54        if (added == null) {
     55            added = getDataSources().stream().filter(s -> !getOldDataSources().contains(s))
     56                    .collect(Collectors.toCollection(LinkedHashSet::new));
     57        }
     58        return added;
     59    }
     60
     61    @Override
     62    public String toString() {
     63        return "DataSourceAddedEvent [current=" + current + ", removed=" + removed + ", added=" + added + ']';
     64    }
     65}
     66
  • test/unit/org/openstreetmap/josm/data/osm/DataSetTest.java

     
    1313import org.junit.Assert;
    1414import org.junit.Rule;
    1515import org.junit.Test;
     16import org.openstreetmap.josm.data.Bounds;
     17import org.openstreetmap.josm.data.DataSource;
    1618import org.openstreetmap.josm.data.coor.LatLon;
     19import org.openstreetmap.josm.data.osm.event.DataSourceAddedEvent;
     20import org.openstreetmap.josm.data.osm.event.DataSourceRemovedEvent;
    1721import org.openstreetmap.josm.testutils.JOSMTestRules;
    1822
    1923import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
     
    247251        assertTrue(UploadPolicy.BLOCKED.compareTo(UploadPolicy.DISCOURAGED) > 0);
    248252        assertTrue(UploadPolicy.DISCOURAGED.compareTo(UploadPolicy.NORMAL) > 0);
    249253    }
     254
     255    /**
     256     * Checks that data source listeners get called when a data source is added
     257     */
     258    @Test
     259    public void testAddDataSourceListener() {
     260        DataSourceListener addListener = new DataSourceListener() {
     261            @Override
     262            public void dataSourceChange(DataSourceChangeEvent event) {
     263                assertTrue(event instanceof DataSourceAddedEvent);
     264            }
     265        };
     266
     267        DataSet ds = new DataSet();
     268        ds.addDataSourceListener(addListener);
     269        ds.addDataSource(new DataSource(new Bounds(0, 0, 0.1, 0.1), "fake source"));
     270
     271    }
     272
     273    /**
     274     * Checks that data source listeners get called when a data source is removed
     275     */
     276    @Test
     277    public void testRemoveDataSourceListener() {
     278        DataSourceListener removeListener = new DataSourceListener() {
     279            @Override
     280            public void dataSourceChange(DataSourceChangeEvent event) {
     281                assertTrue(event instanceof DataSourceRemovedEvent);
     282            }
     283        };
     284
     285        DataSet ds = new DataSet();
     286        ds.addDataSource(new DataSource(new Bounds(0, 0, 0.1, 0.1), "fake source"));
     287        ds.addDataSourceListener(removeListener);
     288        new DataSet().mergeFrom(ds);
     289    }
    250290}
  • test/unit/org/openstreetmap/josm/data/osm/event/DataSourceAddedEventTest.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm.event;
     3
     4import static org.junit.Assert.assertSame;
     5import static org.junit.Assert.assertTrue;
     6
     7import java.util.Collections;
     8import java.util.stream.Stream;
     9
     10import org.junit.Test;
     11import org.openstreetmap.josm.data.Bounds;
     12import org.openstreetmap.josm.data.DataSource;
     13import org.openstreetmap.josm.data.osm.DataSet;
     14import org.openstreetmap.josm.data.osm.DataSourceChangeEvent;
     15
     16/**
     17 * Test class for {@link DataSourceAddedEvent}
     18 *
     19 * @author Taylor Smock
     20 */
     21public class DataSourceAddedEventTest {
     22    /**
     23     * Get getting the originating data source
     24     */
     25    @Test
     26    public void testGetDataEventSource() {
     27        DataSource fakeAdd = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     28        DataSet ds = new DataSet();
     29        assertSame(ds, new DataSourceAddedEvent(ds, Collections.emptySet(), Stream.of(fakeAdd)).getSource());
     30    }
     31
     32    /**
     33     * Test that added sources are processed properly
     34     */
     35    @Test
     36    public void testGetAddedSource() {
     37        DataSource fakeAdd = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     38        assertTrue(
     39                new DataSourceAddedEvent(new DataSet(), Collections.emptySet(), Stream.empty()).getAdded().isEmpty());
     40        DataSourceChangeEvent event = new DataSourceAddedEvent(new DataSet(), Collections.emptySet(),
     41                Stream.of(fakeAdd));
     42        assertSame(event.getAdded(), event.getAdded());
     43        assertSame(fakeAdd, new DataSourceAddedEvent(new DataSet(), Collections.emptySet(), Stream.of(fakeAdd))
     44                .getAdded().iterator().next());
     45    }
     46
     47    /**
     48     * Test that there are no removed sources
     49     */
     50    @Test
     51    public void testGetRemovedSource() {
     52        DataSource fakeAdd = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     53        assertTrue(
     54                new DataSourceAddedEvent(new DataSet(), Collections.emptySet(), Stream.empty()).getRemoved().isEmpty());
     55        DataSourceChangeEvent event = new DataSourceAddedEvent(new DataSet(), Collections.emptySet(),
     56                Stream.of(fakeAdd));
     57        assertSame(event.getRemoved(), event.getRemoved());
     58        assertTrue(new DataSourceAddedEvent(new DataSet(), Collections.emptySet(), Stream.of(fakeAdd)).getRemoved()
     59                .isEmpty());
     60    }
     61
     62    /**
     63     * Check that the sources include newly added data
     64     */
     65    @Test
     66    public void testGetDataSources() {
     67        DataSource fakeAdd = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     68        DataSourceChangeEvent event = new DataSourceAddedEvent(new DataSet(), Collections.emptySet(),
     69                Stream.of(fakeAdd));
     70        assertSame(event.getDataSources(), event.getDataSources());
     71        assertSame(fakeAdd, event.getDataSources().iterator().next());
     72    }
     73
     74    /**
     75     * Check that a string is returned with added/current/deleted
     76     */
     77    @Test
     78    public void testToString() {
     79        String toString = new DataSourceAddedEvent(new DataSet(), Collections.emptySet(), Stream.empty()).toString();
     80        assertTrue(toString.contains("added"));
     81        assertTrue(toString.contains("current"));
     82        assertTrue(toString.contains("removed"));
     83    }
     84}
  • test/unit/org/openstreetmap/josm/data/osm/event/DataSourceRemovedEventTest.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm.event;
     3
     4import static org.junit.Assert.assertFalse;
     5import static org.junit.Assert.assertSame;
     6import static org.junit.Assert.assertTrue;
     7
     8import java.util.Collections;
     9import java.util.stream.Stream;
     10
     11import org.junit.Test;
     12import org.openstreetmap.josm.data.Bounds;
     13import org.openstreetmap.josm.data.DataSource;
     14import org.openstreetmap.josm.data.osm.DataSet;
     15import org.openstreetmap.josm.data.osm.DataSourceChangeEvent;
     16
     17/**
     18 * Test class for {@link DataSourceRemovedEvent}
     19 *
     20 * @author Taylor Smock
     21 */
     22
     23public class DataSourceRemovedEventTest {
     24    /**
     25     * Get getting the originating data source
     26     */
     27    @Test
     28    public void testGetDataEventSource() {
     29        DataSource fakeRemove = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     30        DataSet ds = new DataSet();
     31        assertSame(ds, new DataSourceRemovedEvent(ds, Collections.emptySet(), Stream.of(fakeRemove)).getSource());
     32    }
     33
     34    /**
     35     * Test that no sources are added
     36     */
     37    @Test
     38    public void testGetAddedSource() {
     39        DataSource fakeRemove = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     40        assertTrue(
     41                new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(), Stream.empty()).getAdded().isEmpty());
     42        DataSourceChangeEvent event = new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(),
     43                Stream.of(fakeRemove));
     44        assertSame(event.getAdded(), event.getAdded());
     45        assertTrue(new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(), Stream.of(fakeRemove)).getAdded()
     46                .isEmpty());
     47    }
     48
     49    /**
     50     * Test that the getting the removed source(s) works properly
     51     */
     52    @Test
     53    public void testGetRemovedSource() {
     54        DataSource fakeRemove = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     55        assertTrue(new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(), Stream.empty()).getRemoved()
     56                .isEmpty());
     57        DataSourceChangeEvent event = new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(),
     58                Stream.of(fakeRemove));
     59        assertSame(event.getRemoved(), event.getRemoved());
     60        assertSame(fakeRemove, new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(), Stream.of(fakeRemove))
     61                .getRemoved().iterator().next());
     62    }
     63
     64    /**
     65     * Check that the sources don't include removed data
     66     */
     67    @Test
     68    public void testGetDataSources() {
     69        DataSource fakeRemove = new DataSource(new Bounds(0, 0, 0, 0), "fake-source");
     70        DataSourceChangeEvent event = new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(),
     71                Stream.of(fakeRemove));
     72        assertSame(event.getDataSources(), event.getDataSources());
     73        assertFalse(event.getDataSources().contains(fakeRemove));
     74        assertTrue(new DataSourceRemovedEvent(new DataSet(), Collections.singleton(fakeRemove), Stream.of(fakeRemove))
     75                .getDataSources().isEmpty());
     76        assertSame(fakeRemove,
     77                new DataSourceRemovedEvent(new DataSet(), Collections.singleton(fakeRemove), Stream.empty())
     78                        .getDataSources().iterator().next());
     79    }
     80
     81    /**
     82     * Check that a string is returned with added/current/deleted
     83     */
     84    @Test
     85    public void testToString() {
     86        String toString = new DataSourceRemovedEvent(new DataSet(), Collections.emptySet(), Stream.empty()).toString();
     87        assertTrue(toString.contains("added"));
     88        assertTrue(toString.contains("current"));
     89        assertTrue(toString.contains("removed"));
     90    }
     91}