Changeset 15070 in josm

Timestamp:

2019-05-11T20:24:40+02:00 (6 years ago)

Author:

Don-vip

Message:

fix #17701, fix #17702 - add robustness when loading imagery layers from session

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/SessionLoadAction.java (modified) (4 diffs)
• src/org/openstreetmap/josm/io/session/SessionReader.java (modified) (4 diffs)
• test/unit/org/openstreetmap/josm/actions/SessionLoadActionTest.java (added)
• test/unit/org/openstreetmap/josm/io/session/SessionReaderTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/actions/SessionLoadAction.java

@@ -22 +22 @@
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
 import org.openstreetmap.josm.gui.layer.Layer;
@@ -37 +38 @@
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
+import org.openstreetmap.josm.tools.bugreport.ReportedException;
 
 /**
@@ -138 +140 @@
                     if (canceled)
                         return;
-                    // NoteImporter directly loads notes into current note layer
-                    if (!MainApplication.getLayerManager().containsLayer(l)) {
-                        MainApplication.getLayerManager().addLayer(l);
-                    }
+                    addLayer(l);
                 }
                 if (active != null) {
@@ -150 +149 @@
                 }
             }
+        }
+
+        /**
+         * Tries to add a new layer.
+         * @param l layer to add
+         * @return {@code true} if layer has been added, {@code false} if it wasn't needed or if an error occurred
+         */
+        static boolean addLayer(Layer l) {
+            // NoteImporter directly loads notes into current note layer
+            if (!MainApplication.getLayerManager().containsLayer(l)) {
+                try {
+                    MainApplication.getLayerManager().addLayer(l);
+                } catch (ReportedException e) {
+                    Logging.error(e);
+                    new Notification(tr("Unable to add layer ''{0}'': {1}", l.getName(), e.getMessage()))
+                        .setIcon(JOptionPane.ERROR_MESSAGE).setDuration(Notification.TIME_LONG).show();
+                    if (MainApplication.getLayerManager().containsLayer(l)) {
+                        MainApplication.getLayerManager().removeLayer(l);
+                    }
+                    return false;
+                }
+            }
+            return true;
         }
 

trunk/src/org/openstreetmap/josm/io/session/SessionReader.java

@@ -565 +565 @@
                         throw new IllegalStateException("Importer " + imp + " returned null for " + support);
                     }
-                } catch (IllegalDataException | IllegalStateException | IOException ex) {
+                } catch (IllegalDataException | IllegalArgumentException | IllegalStateException | IOException ex) {
                     exception = ex;
                 }
@@ -704 +704 @@
     public void loadSession(File sessionFile, boolean zip, ProgressMonitor progressMonitor) throws IllegalDataException, IOException {
         try (InputStream josIS = createInputStream(sessionFile, zip)) {
-            loadSession(josIS, sessionFile.toURI(), zip, progressMonitor != null ? progressMonitor : NullProgressMonitor.INSTANCE);
+            loadSession(josIS, sessionFile.toURI(), zip, progressMonitor);
         }
     }
@@ -737 +737 @@
     }
 
-    private void loadSession(InputStream josIS, URI sessionFileURI, boolean zip, ProgressMonitor progressMonitor)
+    /**
+     * Loads session from the given input stream.
+     * @param josIS session stream to load
+     * @param zip {@code true} if it's a zipped session (.joz)
+     * @param sessionFileURI URI of the underlying session file
+     * @param progressMonitor progress monitor
+     * @throws IllegalDataException if invalid data is detected
+     * @throws IOException if any I/O error occurs
+     * @since xxx
+     */
+    public void loadSession(InputStream josIS, URI sessionFileURI, boolean zip, ProgressMonitor progressMonitor)
             throws IOException, IllegalDataException {
 
@@ -744 +754 @@
 
         try {
-            parseJos(XmlUtils.parseSafeDOM(josIS), progressMonitor);
+            parseJos(XmlUtils.parseSafeDOM(josIS), progressMonitor != null ? progressMonitor : NullProgressMonitor.INSTANCE);
         } catch (SAXException e) {
             throw new IllegalDataException(e);

trunk/test/unit/org/openstreetmap/josm/io/session/SessionReaderTest.java

@@ -6 +6 @@
 import static org.junit.Assert.assertTrue;
 
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 
@@ -154 +157 @@
         assertEquals(174, layer.getNoteData().getNotes().size());
     }
+
+    /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/17701">Bug #17701</a>.
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testTicket17701() throws Exception {
+        try (InputStream in = new ByteArrayInputStream(("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" +
+                "<josm-session version=\"0.1\">\n" +
+                "    <layers active=\"1\">\n" +
+                "        <layer index=\"1\" name=\"GPS-треки OpenStreetMap\" type=\"imagery\" version=\"0.1\" visible=\"true\">\n" +
+                "            <id>osm-gps</id>\n" +
+                "            <type>tms</type>\n" +
+                "            <url>https://{switch:a,b,c}.gps-tile.openstreetmap.org/lines/{zoom}/{x}/{y}.png</url>\n" +
+                "            <attribution-text>© OpenStreetMap contributors</attribution-text>\n" +
+                "            <attribution-url>https://www.openstreetmap.org/copyright</attribution-url>\n" +
+                "            <max-zoom>20</max-zoom>\n" +
+                "            <cookies/>\n" +
+                "            <description>Общедоступные GPS-треки, загруженные на OpenStreetMap.</description>\n" +
+                "            <valid-georeference>true</valid-georeference>\n" +
+                "            <overlay>true</overlay>\n" +
+                "            <show-errors>true</show-errors>\n" +
+                "            <automatic-downloading>true</automatic-downloading>\n" +
+                "            <automatically-change-resolution>true</automatically-change-resolution>\n" +
+                "        </layer>\r\n" +
+                "    </layers>\n" +
+                "</josm-session>").getBytes(StandardCharsets.UTF_8))) {
+            SessionReader reader = new SessionReader();
+            reader.loadSession(in, null, false, null);
+            assertTrue(reader.getLayers().isEmpty());
+        }
+    }
 }