Changeset 18008 in josm


Ignore:
Timestamp:
2021-07-12T23:45:16+02:00 (3 years ago)
Author:
Don-vip
Message:

fix #21105 - see #17184 - make sure that most operations of GpxLayer are harmless after call to destroy()

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java

    r17900 r18008  
    11// License: GPL. For details, see LICENSE file.
    22package org.openstreetmap.josm.gui.layer;
     3
     4import static org.openstreetmap.josm.tools.I18n.tr;
     5import static org.openstreetmap.josm.tools.I18n.trn;
     6
     7import java.awt.Color;
     8import java.awt.Dimension;
     9import java.awt.Graphics2D;
     10import java.awt.event.ActionEvent;
     11import java.io.File;
     12import java.time.Instant;
     13import java.util.ArrayList;
     14import java.util.Arrays;
     15import java.util.Collections;
     16import java.util.List;
     17import java.util.NoSuchElementException;
     18import java.util.stream.Collectors;
     19
     20import javax.swing.AbstractAction;
     21import javax.swing.Action;
     22import javax.swing.Icon;
     23import javax.swing.JScrollPane;
     24import javax.swing.SwingUtilities;
    325
    426import org.openstreetmap.josm.actions.AutoScaleAction;
    … …  
    4264import org.openstreetmap.josm.tools.date.Interval;
    4365
    44 import javax.swing.AbstractAction;
    45 import javax.swing.Action;
    46 import javax.swing.Icon;
    47 import javax.swing.JScrollPane;
    48 import javax.swing.SwingUtilities;
    49 import java.awt.Color;
    50 import java.awt.Dimension;
    51 import java.awt.Graphics2D;
    52 import java.awt.event.ActionEvent;
    53 import java.io.File;
    54 import java.time.Instant;
    55 import java.util.ArrayList;
    56 import java.util.Arrays;
    57 import java.util.Collections;
    58 import java.util.List;
    59 import java.util.NoSuchElementException;
    60 import java.util.stream.Collectors;
    61 
    62 import static org.openstreetmap.josm.tools.I18n.tr;
    63 import static org.openstreetmap.josm.tools.I18n.trn;
    64 
    6566/**
    6667 * A layer that displays data from a Gpx file / the OSM gpx downloads.
    … …  
    130131    @Override
    131132    public Color getColor() {
     133        if (data == null)
     134            return null;
    132135        Color[] c = data.getTracks().stream().map(t -> t.getColor()).distinct().toArray(Color[]::new);
    133136        return c.length == 1 ? c[0] : null; //only return if exactly one distinct color present
    … …  
    146149    @Override
    147150    public boolean hasColor() {
    148         return true;
     151        return data != null;
    149152    }
    150153
    … …  
    168171                .append("<html><head><style>td { padding: 4px 16px; }</style></head><body>");
    169172
     173        if (data != null) {
     174            fillDataInfoComponent(info);
     175        }
     176
     177        info.append("<br></body></html>");
     178
     179        final JScrollPane sp = new JScrollPane(new HtmlPanel(info.toString()));
     180        sp.setPreferredSize(new Dimension(sp.getPreferredSize().width+20, 370));
     181        SwingUtilities.invokeLater(() -> sp.getVerticalScrollBar().setValue(0));
     182        return sp;
     183    }
     184
     185    private void fillDataInfoComponent(StringBuilder info) {
    170186        if (data.attr.containsKey("name")) {
    171187            info.append(tr("Name: {0}", data.get(GpxConstants.META_NAME))).append("<br>");
    … …  
    215231        info.append(tr("Length: {0}", SystemOfMeasurement.getSystemOfMeasurement().getDistText(data.length()))).append("<br>")
    216232            .append(trn("{0} route, ", "{0} routes, ", data.getRoutes().size(), data.getRoutes().size()))
    217             .append(trn("{0} waypoint", "{0} waypoints", data.getWaypoints().size(), data.getWaypoints().size()))
    218             .append("<br></body></html>");
    219 
    220         final JScrollPane sp = new JScrollPane(new HtmlPanel(info.toString()));
    221         sp.setPreferredSize(new Dimension(sp.getPreferredSize().width+20, 370));
    222         SwingUtilities.invokeLater(() -> sp.getVerticalScrollBar().setValue(0));
    223         return sp;
     233            .append(trn("{0} waypoint", "{0} waypoints", data.getWaypoints().size(), data.getWaypoints().size()));
    224234    }
    225235
    … …  
    284294        StringBuilder info = new StringBuilder(48).append("<html>");
    285295
     296        if (data != null) {
     297            fillDataToolTipText(info);
     298        }
     299
     300        info.append("<br></html>");
     301
     302        return info.toString();
     303    }
     304
     305    private void fillDataToolTipText(StringBuilder info) {
    286306        if (data.attr.containsKey(GpxConstants.META_NAME)) {
    287307            info.append(tr("Name: {0}", data.get(GpxConstants.META_NAME))).append("<br>");
    … …  
    305325                        .collect(Collectors.joining("<br>")));
    306326        }
    307 
    308         info.append("<br></html>");
    309 
    310         return info.toString();
    311327    }
    312328
    313329    @Override
    314330    public boolean isMergable(Layer other) {
    315         return other instanceof GpxLayer;
     331        return data != null && other instanceof GpxLayer;
    316332    }
    317333
    … …  
    323339     */
    324340    public void filterTracksByDate(Instant fromDate, Instant toDate, boolean showWithoutDate) {
     341        if (data == null)
     342            return;
    325343        int i = 0;
    326344        long from = fromDate.toEpochMilli();
    … …  
    358376    @Override
    359377    public void visitBoundingBox(BoundingXYVisitor v) {
    360         v.visit(data.recalculateBounds());
     378        if (data != null) {
     379            v.visit(data.recalculateBounds());
     380        }
    361381    }
    362382
    363383    @Override
    364384    public File getAssociatedFile() {
    365         return data.storageFile;
     385        return data != null ? data.storageFile : null;
    366386    }
    367387
    … …  
    391411    @Override
    392412    public void projectionChanged(Projection oldValue, Projection newValue) {
    393         if (newValue == null) return;
     413        if (newValue == null || data == null) return;
    394414        data.resetEastNorthCache();
    395415    }
    … …  
    397417    @Override
    398418    public boolean isSavable() {
    399         return true; // With GpxExporter
     419        return data != null; // With GpxExporter
    400420    }
    401421
    … …  
    560580    @Override
    561581    public void jumpToNextMarker() {
    562         List<IGpxTrackSegment> segments = data.getTrackSegmentsStream().collect(Collectors.toList());
    563         jumpToNext(segments);
     582        if (data != null) {
     583            jumpToNext(data.getTrackSegmentsStream().collect(Collectors.toList()));
     584        }
    564585    }
    565586
    … …  
    569590    @Override
    570591    public void jumpToPreviousMarker() {
    571         List<IGpxTrackSegment> segments = data.getTrackSegmentsStream().collect(Collectors.toList());
    572         Collections.reverse(segments);
    573         jumpToNext(segments);
     592        if (data != null) {
     593            List<IGpxTrackSegment> segments = data.getTrackSegmentsStream().collect(Collectors.toList());
     594            Collections.reverse(segments);
     595            jumpToNext(segments);
     596        }
    574597    }
    575598

  • trunk/test/unit/org/openstreetmap/josm/gui/layer/GpxLayerTest.java

    r17845 r18008  
    22package org.openstreetmap.josm.gui.layer;
    33
     4import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
    45import static org.junit.jupiter.api.Assertions.assertEquals;
    56import static org.junit.jupiter.api.Assertions.assertFalse;
    67import static org.junit.jupiter.api.Assertions.assertNull;
     8import static org.junit.jupiter.api.Assertions.assertThrows;
    79import static org.junit.jupiter.api.Assertions.assertTrue;
    8 import static org.junit.jupiter.api.Assertions.assertThrows;
    910
    1011import java.awt.Color;
     12import java.awt.Component;
    1113import java.io.IOException;
    1214import java.util.ArrayList;
    … …  
    2729import org.openstreetmap.josm.data.gpx.WayPoint;
    2830import org.openstreetmap.josm.data.osm.DataSet;
     31import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor;
     32import org.openstreetmap.josm.data.projection.CustomProjection;
    2933import org.openstreetmap.josm.data.projection.Projections;
    3034import org.openstreetmap.josm.gui.MainApplication;
    … …  
    261265        assertNull(new GpxLayer(new GpxData(), "", false).getChangesetSourceTag());
    262266    }
     267
     268    /**
     269     * Checks that potential operations that could be called after destroy() are harmless
     270     */
     271    @Test
     272    void testRobustnessAfterDestroy() {
     273        GpxData data = new GpxData();
     274        GpxLayer layer = new GpxLayer(data, "1", false);
     275        GpxLayer otherLayer = new GpxLayer(new GpxData(), "2", false);
     276        assertEquals(data, layer.getData());
     277        assertTrue(layer.isMergable(otherLayer));
     278        assertTrue(layer.hasColor());
     279        assertTrue(layer.isSavable());
     280        assertTrue(layer.checkSaveConditions());
     281        assertFalse(layer.isModified());
     282        assertFalse(layer.requiresSaveToFile());
     283        assertNull(layer.getChangesetSourceTag());
     284        assertNull(layer.getAssociatedFile());
     285
     286        layer.destroy();
     287
     288        assertNull(layer.getData());
     289        assertNull(layer.getColor());
     290        assertFalse(layer.hasColor());
     291        assertFalse(layer.isMergable(otherLayer));
     292        assertFalse(layer.isSavable());
     293        assertFalse(layer.checkSaveConditions());
     294        assertFalse(layer.isModified());
     295        assertFalse(layer.requiresSaveToFile());
     296        assertNull(layer.getChangesetSourceTag());
     297        assertNull(layer.getAssociatedFile());
     298        Object infoComponent = layer.getInfoComponent();
     299        assertTrue(infoComponent instanceof JScrollPane);
     300        Component view = ((JScrollPane) infoComponent).getViewport().getView();
     301        assertTrue(view instanceof HtmlPanel);
     302        String text = ((HtmlPanel) view).getEditorPane().getText().trim();
     303        assertTrue(text.startsWith("<html>"), text);
     304        assertTrue(text.endsWith("</html>"), text);
     305        assertEquals("<html><br></html>", layer.getToolTipText());
     306        assertDoesNotThrow(() -> layer.jumpToNextMarker());
     307        assertDoesNotThrow(() -> layer.jumpToPreviousMarker());
     308        assertDoesNotThrow(() -> layer.visitBoundingBox(new BoundingXYVisitor()));
     309        assertDoesNotThrow(() -> layer.filterTracksByDate(null, null, false));
     310        assertDoesNotThrow(() -> layer.projectionChanged(new CustomProjection(), new CustomProjection()));
     311    }
    263312}
Note: See TracChangeset for help on using the changeset viewer.