Changeset 35944 in osm for applications/editors

Timestamp:

2022-03-23T00:11:16+01:00 (3 years ago)

Author:

taylor.smock

Message:

DirectDownload: fix #21935 and #21952

#21935: "Error fetching URL" when trying to download a GPS track

This was caused due to a change in where OSM stores GPX data files.
OSM now uses s3 buckets, and redirects using a signed URL. S3 does
not like multiple authentication methods.

#21952: Only the first tag in the Tags column in "Download Track" window is shown

This was caused due to only keeping the last encountered tag.

Location:

applications/editors/josm/plugins/DirectDownload

Files:

• build.xml (modified) (1 diff)
• src/org/openstreetmap/josm/plugins/directdownload/DirectDownload.java (modified) (5 diffs)
• src/org/openstreetmap/josm/plugins/directdownload/DownloadDataGui.java (modified) (3 diffs)
• src/org/openstreetmap/josm/plugins/directdownload/GpxServerReader.java (modified) (2 diffs)
• src/org/openstreetmap/josm/plugins/directdownload/UserTrack.java (modified) (2 diffs)
• src/org/openstreetmap/josm/plugins/directdownload/UserTrackReader.java (modified) (2 diffs)

applications/editors/josm/plugins/DirectDownload/build.xml

@@ -13 +13 @@
     <property name="plugin.description" value="Download your GPX tracks from openstreetmap.org"/>
     <property name="plugin.icon" value="images/DownloadAction.svg"/>
+    <property name="plugin.canloadatruntime" value="true"/>
     <!-- <property name="plugin.link" value="..."/>-->
     <!--<property name="plugin.early" value="..."/>-->

applications/editors/josm/plugins/DirectDownload/src/org/openstreetmap/josm/plugins/directdownload/DirectDownload.java

@@ -5 +5 @@
 
 import java.awt.event.ActionEvent;
+import java.awt.event.KeyEvent;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 
 import org.openstreetmap.josm.actions.JosmAction;
@@ -20 +22 @@
 import org.openstreetmap.josm.plugins.PluginInformation;
 import org.openstreetmap.josm.spi.preferences.Config;
+import org.openstreetmap.josm.tools.Shortcut;
 
 public class DirectDownload extends Plugin {
-
-    private DownloadAction openaction;
-
     /**
      * Will be invoked by JOSM to bootstrap the plugin
@@ -33 +33 @@
         super(info);
 
-        openaction = new DownloadAction();
-        MainMenu.add(MainApplication.getMenu().gpsMenu, openaction);
+        MainMenu.add(MainApplication.getMenu().gpsMenu, new DownloadAction());
     }
 
     static class DownloadAction extends JosmAction {
         DownloadAction() {
-            super(tr("Download Track ..."), "DownloadAction",
-                    tr("Download GPX track from openstreetmap.org"), null, false);
+            super(tr("Download Track ..."), "DownloadAction", tr("Download GPX track from openstreetmap.org"),
+                    Shortcut.registerShortcut("directdownload:downloadgpxaction", tr("DirectDownload: Download GPX Track"),
+                            KeyEvent.CHAR_UNDEFINED, Shortcut.NONE), false);
         }
 
@@ -48 +48 @@
             go.setVisible(true);
 
-            ArrayList<UserTrack> tracks = go.getSelectedUserTracks();
+            List<UserTrack> tracks = go.getSelectedUserTracks();
 
             if (!((go.getValue() == 1) && (tracks != null))) {
@@ -84 +84 @@
                 if (Config.getPref().getBoolean("marker.makeautomarkers", true) && !data.waypoints.isEmpty()) {
                     MarkerLayer ml = new MarkerLayer(data, tr("Markers from {0}", track.filename), null, gpxLayer);
-                    if (ml.data.size() > 0) {
+                    if (!ml.data.isEmpty()) {
                         MainApplication.getLayerManager().addLayer(ml);
                     }

applications/editors/josm/plugins/DirectDownload/src/org/openstreetmap/josm/plugins/directdownload/DownloadDataGui.java

@@ -105 +105 @@
     }
 
-    public ArrayList<UserTrack> getSelectedUserTracks() {
-        ArrayList<UserTrack> DataArray = model.getDataArrayList();
+    public List<UserTrack> getSelectedUserTracks() {
+        List<UserTrack> dataArray = model.getDataArrayList();
         int[] selected = tblSearchResults.getSelectedRows();
-        ArrayList<UserTrack> selectedTracks = new ArrayList<>(selected.length);
-        for (int i = 0; i < selected.length; i++) {
-            selectedTracks.add(DataArray.get(selected[i]));
-        }
+        List<UserTrack> selectedTracks = new ArrayList<>(selected.length);
+
+        for (int i : selected) {
+            selectedTracks.add(dataArray.get(i));
+        }
+
         return selectedTracks;
     }
@@ -121 +123 @@
     static class NamedResultTableColumnModel extends DefaultTableColumnModel {
         protected void createColumns() {
-            TableColumn col = null;
+            TableColumn col;
             NamedResultCellRenderer renderer = new NamedResultCellRenderer();
 
@@ -207 +209 @@
 
             case 3:
-                setText(sr.tags);
+                setText(String.join(";", sr.tags));
                 break;
 

applications/editors/josm/plugins/DirectDownload/src/org/openstreetmap/josm/plugins/directdownload/GpxServerReader.java

@@ -23 +23 @@
 
     public GpxData loadGpx(final long id) {
-        final String urlString = OsmApi.getOsmApi().getBaseUrl() + "gpx/" + id + "/data";
+        return fetchUrl(OsmApi.getOsmApi().getBaseUrl() + "gpx/" + id + "/data");
+    }
 
+    private GpxData fetchUrl(final String urlString) {
+        HttpClient client = null;
         try {
-            final HttpClient client = HttpClient.create(new URL(urlString));
-            addAuth(client);
+            client = HttpClient.create(new URL(urlString));
+            // At some point OSM started hosting the files on Amazon S3. S3 does not like multiple authentication methods.
+            // See JOSM #21935.
+            if (!urlString.contains("X-Amz-Algorithm") && !urlString.contains("Signature")) {
+                addAuth(client);
+            }
 
             try (InputStream in = client.connect().uncompressAccordingToContentDisposition(true).getContent()) {
@@ -34 +41 @@
                 if (!parsedProperly) {
                     JOptionPane.showMessageDialog(MainApplication.getMainFrame(),
-                            tr("Error occurred while parsing gpx file {0}. Only a part of the file will be available.", urlString));
+                            tr("Error occurred while parsing gpx file {0}. Only a part of the file will be available.",
+                                    urlString));
                 }
                 return r.getGpxData();
             }
         } catch (IOException | OsmTransferException e) {
+            if (client != null && client.getResponse() != null
+                    && !client.getResponse().getURL().toExternalForm().equals(urlString) /* Check if we were redirected */
+                    && client.getRequestHeader("Authorization") != null) {
+                // OK. We may have hit JOSM #21935.
+                return fetchUrl(client.getResponse().getURL().toExternalForm());
+            }
             Logging.warn(e);
             JOptionPane.showMessageDialog(MainApplication.getMainFrame(), tr("Error fetching URL {0}", urlString));

applications/editors/josm/plugins/DirectDownload/src/org/openstreetmap/josm/plugins/directdownload/UserTrack.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.directdownload;
+
+import java.util.ArrayList;
+import java.util.List;
 
 class UserTrack {
@@ -6 +9 @@
     public String filename;
     public String description;
-    public String tags;
+    public final List<String> tags = new ArrayList<>();
     public String datetime;
 }

applications/editors/josm/plugins/DirectDownload/src/org/openstreetmap/josm/plugins/directdownload/UserTrackReader.java

@@ -66 +66 @@
 
         @Override
-        public void startElement(String namespaceURI, String localName, String qName, Attributes atts) throws SAXException {
+        public void startElement(String namespaceURI, String localName, String qName, Attributes atts) {
             if (qName.equals("gpx_file")) {
                 UserTrack track = new UserTrack();
@@ -80 +80 @@
 
         @Override
-        public void characters(char[] ch, int start, int length)
-                throws SAXException {
+        public void characters(char[] ch, int start, int length) {
             cdata += new String(ch, start, length);
         }
 
         @Override
-        public void endElement(String uri, String localName, String qName) throws SAXException {
+        public void endElement(String uri, String localName, String qName) {
             if (qName.equals("description")) {
                 data.getFirst().description = cdata;
             } else if (qName.equals("tag")) {
-                data.getFirst().tags = cdata;
-                cdata = new String();
+                data.getFirst().tags.add(cdata);
+                cdata = "";
             }