From: Tom Hughes <tom@compton.nu>
Date: Wed, 29 Sep 2021 18:10:22 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/3332'
X-Git-Tag: live~2474
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/cb10c10ea3effe166ce9e1790d7b79993801ab61?hp=7609c118843d66c37dd428a2a162a4e15289b6ce

Merge remote-tracking branch 'upstream/pull/3332'
---

diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb
index b2d755fe6..e758d559f 100644
--- a/app/controllers/api/tracepoints_controller.rb
+++ b/app/controllers/api/tracepoints_controller.rb
@@ -33,75 +33,11 @@ module Api
       # get all the points
       ordered_points = Tracepoint.bbox(bbox).joins(:trace).where(:gpx_files => { :visibility => %w[trackable identifiable] }).order("gpx_id DESC, trackid ASC, timestamp ASC")
       unordered_points = Tracepoint.bbox(bbox).joins(:trace).where(:gpx_files => { :visibility => %w[public private] }).order("gps_points.latitude", "gps_points.longitude", "gps_points.timestamp")
-      points = ordered_points.union_all(unordered_points).offset(offset).limit(Settings.tracepoints_per_page).preload(:trace)
-
-      doc = XML::Document.new
-      doc.encoding = XML::Encoding::UTF_8
-      root = XML::Node.new "gpx"
-      root["version"] = "1.0"
-      root["creator"] = "OpenStreetMap.org"
-      root["xmlns"] = "http://www.topografix.com/GPX/1/0"
-
-      doc.root = root
-
-      # initialise these variables outside of the loop so that they
-      # stay in scope and don't get free'd up by the GC during the
-      # loop.
-      gpx_id = -1
-      trackid = -1
-      track = nil
-      trkseg = nil
-      anon_track = nil
-      anon_trkseg = nil
-      timestamps = false
-
-      points.each do |point|
-        if gpx_id != point.gpx_id
-          gpx_id = point.gpx_id
-          trackid = -1
-
-          if point.trace.trackable?
-            track = XML::Node.new "trk"
-            doc.root << track
-            timestamps = true
-
-            if point.trace.identifiable?
-              track << (XML::Node.new("name") << point.trace.name)
-              track << (XML::Node.new("desc") << point.trace.description)
-              track << (XML::Node.new("url") << url_for(:controller => "/traces", :action => "show", :display_name => point.trace.user.display_name, :id => point.trace.id))
-            end
-          else
-            # use the anonymous track segment if the user hasn't allowed
-            # their GPX points to be tracked.
-            timestamps = false
-            if anon_track.nil?
-              anon_track = XML::Node.new "trk"
-              doc.root << anon_track
-            end
-            track = anon_track
-          end
-        end
-
-        if trackid != point.trackid
-          if point.trace.trackable?
-            trkseg = XML::Node.new "trkseg"
-            track << trkseg
-            trackid = point.trackid
-          else
-            if anon_trkseg.nil?
-              anon_trkseg = XML::Node.new "trkseg"
-              anon_track << anon_trkseg
-            end
-            trkseg = anon_trkseg
-          end
-        end
-
-        trkseg << point.to_xml_node(:print_timestamp => timestamps)
-      end
+      @points = ordered_points.union_all(unordered_points).offset(offset).limit(Settings.tracepoints_per_page).preload(:trace)
 
       response.headers["Content-Disposition"] = "attachment; filename=\"tracks.gpx\""
 
-      render :xml => doc.to_s
+      render :formats => [:gpx]
     end
   end
 end
diff --git a/app/models/tracepoint.rb b/app/models/tracepoint.rb
index 000f257b4..b3c37430d 100644
--- a/app/models/tracepoint.rb
+++ b/app/models/tracepoint.rb
@@ -31,12 +31,4 @@ class Tracepoint < ApplicationRecord
   validates :timestamp, :presence => true
 
   belongs_to :trace, :foreign_key => "gpx_id"
-
-  def to_xml_node(print_timestamp: false)
-    el1 = XML::Node.new "trkpt"
-    el1["lat"] = lat.to_s
-    el1["lon"] = lon.to_s
-    el1 << (XML::Node.new("time") << timestamp.xmlschema) if print_timestamp
-    el1
-  end
 end
diff --git a/app/views/api/tracepoints/index.gpx.builder b/app/views/api/tracepoints/index.gpx.builder
new file mode 100644
index 000000000..291ef0d45
--- /dev/null
+++ b/app/views/api/tracepoints/index.gpx.builder
@@ -0,0 +1,79 @@
+xml.instruct!
+
+xml.gpx("version" => "1.0",
+        "creator" => "OpenStreetMap.org",
+        "xmlns" => "http://www.topografix.com/GPX/1/0") do
+  # initialise these variables outside of the loop so that they
+  # stay in scope and don't get free'd up by the GC during the
+  # loop.
+  gpx_id = -1
+  trackid = -1
+  tracks = []
+  track = nil
+  trkseg = nil
+  anon_track = nil
+  anon_trkseg = nil
+
+  @points.each do |point|
+    if gpx_id != point.gpx_id
+      gpx_id = point.gpx_id
+      trackid = -1
+
+      if point.trace.trackable?
+        track = {}
+        track["trksegs"] = []
+        tracks << track
+
+        if point.trace.identifiable?
+          track["name"] = point.trace.name
+          track["desc"] = point.trace.description
+          track["url"] = url_for(:controller => "/traces", :action => "show", :display_name => point.trace.user.display_name, :id => point.trace.id)
+        end
+      else
+        # use the anonymous track segment if the user hasn't allowed
+        # their GPX points to be tracked.
+        if anon_track.nil?
+          anon_track = {}
+          anon_track["trksegs"] = []
+          tracks << anon_track
+        end
+        track = anon_track
+      end
+    end
+
+    if trackid != point.trackid
+      if point.trace.trackable?
+        trkseg = []
+        track["trksegs"] << trkseg
+        trackid = point.trackid
+      else
+        if anon_trkseg.nil?
+          anon_trkseg = []
+          anon_track["trksegs"] << anon_trkseg
+        end
+        trkseg = anon_trkseg
+      end
+    end
+
+    trkseg << point
+  end
+
+  tracks.each do |trk|
+    xml.trk do
+      if trk.key?("name")
+        xml.name trk["name"]
+        xml.desc trk["desc"]
+        xml.url trk["url"]
+      end
+      trk["trksegs"].each do |trksg|
+        xml.trkseg do
+          trksg.each do |tracepoint|
+            xml.trkpt("lat" => tracepoint.lat.to_s, "lon" => tracepoint.lon.to_s) do
+              xml.time tracepoint.timestamp.xmlschema if tracepoint.trace.trackable?
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/test/controllers/api/tracepoints_controller_test.rb b/test/controllers/api/tracepoints_controller_test.rb
index a904e8127..7d561522c 100644
--- a/test/controllers/api/tracepoints_controller_test.rb
+++ b/test/controllers/api/tracepoints_controller_test.rb
@@ -148,5 +148,14 @@ module Api
         assert_equal "The minimum latitude must be less than the maximum latitude, but it wasn't", @response.body, "bbox: #{bbox}"
       end
     end
+
+    # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
+    def test_lat_lon_xml_format
+      create(:tracepoint, :latitude => (0.00004 * GeoRecord::SCALE).to_i, :longitude => (0.00008 * GeoRecord::SCALE).to_i)
+
+      get trackpoints_path(:bbox => "0,0,0.1,0.1")
+      assert_match(/lat="0.0000400"/, response.body)
+      assert_match(/lon="0.0000800"/, response.body)
+    end
   end
 end
diff --git a/test/models/tracepoint_test.rb b/test/models/tracepoint_test.rb
index a672b7c62..2ffbef09f 100644
--- a/test/models/tracepoint_test.rb
+++ b/test/models/tracepoint_test.rb
@@ -7,12 +7,4 @@ class TracepointTest < ActiveSupport::TestCase
     tracepoint.timestamp = nil
     assert_not tracepoint.valid?
   end
-
-  # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
-  def test_lat_lon_xml_format
-    tracepoint = build(:tracepoint, :latitude => 0.00004 * GeoRecord::SCALE, :longitude => 0.00008 * GeoRecord::SCALE)
-
-    assert_match(/lat="0.0000400"/, tracepoint.to_xml_node.to_s)
-    assert_match(/lon="0.0000800"/, tracepoint.to_xml_node.to_s)
-  end
 end