Refactor tracepoint index to use an xml builder view

This avoids constructing xml by hand in both the controller and
the model, and opens the way for other rendering in future.

The complexity of deciding which point goes where, along with revisiting
previous tracks and tracksegs means that I've broken it down into
two parts - sorting the points into the right trksegs is done first,
before rendering them all as xml. I couldn't find a way to allow
revisiting using the builder.

diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb
index b2d755fe6caeaefccfac88a7e94ddf569c6467b6..e758d559f99d0bb69eaeaca8ec7cecd8a1740862 100644 (file)
--- 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/views/api/tracepoints/index.gpx.builder b/app/views/api/tracepoints/index.gpx.builder
new file mode 100644 (file)
index 0000000..291ef0d
--- /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