Rework coordinates to avoid scientific formatting of small numbers. Fixes #1509

app/models/node.rb

@@ -185,8 +185,8 @@ class Node < ActiveRecord::Base
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
     if visible?
-      el["lat"] = format("%.7f", lat)
-      el["lon"] = format("%.7f", lon)
+      el["lat"] = lat.to_s
+      el["lon"] = lon.to_s
     end
 
     add_tags_to_xml_node(el, node_tags)

app/views/notes/_note.json.jsonify

@@ -2,7 +2,7 @@ json.type "Feature"
 
 json.geometry do
   json.type "Point"
-  json.coordinates [ note.lon, note.lat ]
+  json.coordinates [ note.lon.to_f, note.lat.to_f ]
 end
 
 json.properties do

lib/bounding_box.rb

@@ -137,10 +137,10 @@ class BoundingBox
   # there are two forms used for bounds with and without an underscore,
   # cater for both forms eg minlon and min_lon
   def add_bounds_to(hash, underscore = "")
-    hash["min#{underscore}lat"] = min_lat.to_s
-    hash["min#{underscore}lon"] = min_lon.to_s
-    hash["max#{underscore}lat"] = max_lat.to_s
-    hash["max#{underscore}lon"] = max_lon.to_s
+    hash["min#{underscore}lat"] = format("%.7f", min_lat)
+    hash["min#{underscore}lon"] = format("%.7f", min_lon)
+    hash["max#{underscore}lat"] = format("%.7f", max_lat)
+    hash["max#{underscore}lon"] = format("%.7f", max_lon)
     hash
   end
 

lib/geo_record.rb

@@ -1,4 +1,19 @@
+require "delegate"
+
 module GeoRecord
+  # Ensure that when coordinates are printed that they are always in decimal degrees,
+  # and not e.g. 4.0e-05
+  # Unfortunately you can't extend Numeric classes directly (e.g. `Coord < Float`).
+  class Coord < DelegateClass(Float)
+    def initialize(obj)
+      super(obj)
+    end
+
+    def to_s
+      format("%.7f", self)
+    end
+  end
+
   # This scaling factor is used to convert between the float lat/lon that is
   # returned by the API, and the integer lat/lon equivalent that is stored in
   # the database.
@@ -31,11 +46,11 @@ module GeoRecord
 
   # Return WGS84 latitude
   def lat
-    latitude.to_f / SCALE
+    Coord.new(latitude.to_f / SCALE)
   end
 
   # Return WGS84 longitude
   def lon
-    longitude.to_f / SCALE
+    Coord.new(longitude.to_f / SCALE)
   end
 end

lib/potlatch.rb

@@ -85,7 +85,7 @@ module Potlatch
         a + 0.chr + 0.chr + 9.chr
       when String
         2.chr + encodestring(n)
-      when Numeric
+      when Numeric, GeoRecord::Coord
         0.chr + encodedouble(n)
       when NilClass
         5.chr
@@ -107,7 +107,7 @@ module Potlatch
 
     # Encode number as eight-byte double precision float
     def self.encodedouble(n)
-      [n].pack("G")
+      [n.to_f].pack("G")
     end
 
     # Encode number as four-byte long

test/controllers/api_controller_test.rb

@@ -70,7 +70,7 @@ class ApiControllerTest < ActionController::TestCase
     end
     assert_response :success, "Expected scucess with the map call"
     assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-      assert_select "bounds[minlon='#{minlon}'][minlat='#{minlat}'][maxlon='#{maxlon}'][maxlat='#{maxlat}']", :count => 1
+      assert_select "bounds[minlon='#{format('%.7f', minlon)}'][minlat='#{format('%.7f', minlat)}'][maxlon='#{format('%.7f', maxlon)}'][maxlat='#{format('%.7f', maxlat)}']", :count => 1
       assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
         # This should really be more generic
         assert_select "tag[k='#{tag.k}'][v='#{tag.v}']"
@@ -141,7 +141,7 @@ class ApiControllerTest < ActionController::TestCase
     get :map, :bbox => "179.998,89.998,179.999.1,89.999"
     assert_response :success, "The map call should have succeeded"
     assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
-      assert_select "bounds[minlon='179.998'][minlat='89.998'][maxlon='179.999'][maxlat='89.999']", :count => 1
+      assert_select "bounds[minlon='179.9980000'][minlat='89.9980000'][maxlon='179.9990000'][maxlat='89.9990000']", :count => 1
       assert_select "node", :count => 0
       assert_select "way", :count => 0
       assert_select "relation", :count => 0

test/controllers/changeset_controller_test.rb

@@ -1540,10 +1540,10 @@ EOF
     # get the bounding box back from the changeset
     get :read, :id => changeset_id
     assert_response :success, "Couldn't read back changeset."
-    assert_select "osm>changeset[min_lon='1.0']", 1
-    assert_select "osm>changeset[max_lon='1.0']", 1
-    assert_select "osm>changeset[min_lat='2.0']", 1
-    assert_select "osm>changeset[max_lat='2.0']", 1
+    assert_select "osm>changeset[min_lon='1.0000000']", 1
+    assert_select "osm>changeset[max_lon='1.0000000']", 1
+    assert_select "osm>changeset[min_lat='2.0000000']", 1
+    assert_select "osm>changeset[max_lat='2.0000000']", 1
 
     # add another node to it
     with_controller(NodeController.new) do
@@ -1555,10 +1555,10 @@ EOF
     # get the bounding box back from the changeset
     get :read, :id => changeset_id
     assert_response :success, "Couldn't read back changeset for the second time."
-    assert_select "osm>changeset[min_lon='1.0']", 1
-    assert_select "osm>changeset[max_lon='2.0']", 1
-    assert_select "osm>changeset[min_lat='1.0']", 1
-    assert_select "osm>changeset[max_lat='2.0']", 1
+    assert_select "osm>changeset[min_lon='1.0000000']", 1
+    assert_select "osm>changeset[max_lon='2.0000000']", 1
+    assert_select "osm>changeset[min_lat='1.0000000']", 1
+    assert_select "osm>changeset[max_lat='2.0000000']", 1
 
     # add (delete) a way to it, which contains a point at (3,3)
     with_controller(WayController.new) do
@@ -1571,10 +1571,10 @@ EOF
     get :read, :id => changeset_id
     assert_response :success, "Couldn't read back changeset for the third time."
     # note that the 3.1 here is because of the bbox overexpansion
-    assert_select "osm>changeset[min_lon='1.0']", 1
-    assert_select "osm>changeset[max_lon='3.1']", 1
-    assert_select "osm>changeset[min_lat='1.0']", 1
-    assert_select "osm>changeset[max_lat='3.1']", 1
+    assert_select "osm>changeset[min_lon='1.0000000']", 1
+    assert_select "osm>changeset[max_lon='3.1000000']", 1
+    assert_select "osm>changeset[min_lat='1.0000000']", 1
+    assert_select "osm>changeset[max_lat='3.1000000']", 1
   end
 
   ##

test/controllers/relation_controller_test.rb

@@ -967,10 +967,10 @@ OSM
       assert_response :success, "can't re-read changeset for modify test"
       assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}"
       assert_select "osm>changeset[id='#{changeset_id}']", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}"
-      assert_select "osm>changeset[min_lon='#{bbox.min_lon}']", 1, "Changeset min_lon wrong in #{@response.body}"
-      assert_select "osm>changeset[min_lat='#{bbox.min_lat}']", 1, "Changeset min_lat wrong in #{@response.body}"
-      assert_select "osm>changeset[max_lon='#{bbox.max_lon}']", 1, "Changeset max_lon wrong in #{@response.body}"
-      assert_select "osm>changeset[max_lat='#{bbox.max_lat}']", 1, "Changeset max_lat wrong in #{@response.body}"
+      assert_select "osm>changeset[min_lon='#{format('%.7f', bbox.min_lon)}']", 1, "Changeset min_lon wrong in #{@response.body}"
+      assert_select "osm>changeset[min_lat='#{format('%.7f', bbox.min_lat)}']", 1, "Changeset min_lat wrong in #{@response.body}"
+      assert_select "osm>changeset[max_lon='#{format('%.7f', bbox.max_lon)}']", 1, "Changeset max_lon wrong in #{@response.body}"
+      assert_select "osm>changeset[max_lat='#{format('%.7f', bbox.max_lat)}']", 1, "Changeset max_lat wrong in #{@response.body}"
     end
   end
 

test/lib/bounding_box_test.rb

@@ -265,19 +265,19 @@ class BoundingBoxTest < ActiveSupport::TestCase
   def test_add_bounds_to_no_underscore
     bounds = @bbox_from_string.add_bounds_to({})
     assert_equal 4, bounds.size
-    assert_equal @min_lon.to_s, bounds["minlon"]
-    assert_equal @min_lat.to_s, bounds["minlat"]
-    assert_equal @max_lon.to_s, bounds["maxlon"]
-    assert_equal @max_lat.to_s, bounds["maxlat"]
+    assert_equal format("%.7f", @min_lon), bounds["minlon"]
+    assert_equal format("%.7f", @min_lat), bounds["minlat"]
+    assert_equal format("%.7f", @max_lon), bounds["maxlon"]
+    assert_equal format("%.7f", @max_lat), bounds["maxlat"]
   end
 
   def test_add_bounds_to_with_underscore
     bounds = @bbox_from_string.add_bounds_to({}, "_")
     assert_equal 4, bounds.size
-    assert_equal @min_lon.to_s, bounds["min_lon"]
-    assert_equal @min_lat.to_s, bounds["min_lat"]
-    assert_equal @max_lon.to_s, bounds["max_lon"]
-    assert_equal @max_lat.to_s, bounds["max_lat"]
+    assert_equal format("%.7f", @min_lon), bounds["min_lon"]
+    assert_equal format("%.7f", @min_lat), bounds["min_lat"]
+    assert_equal format("%.7f", @max_lon), bounds["max_lon"]
+    assert_equal format("%.7f", @max_lat), bounds["max_lat"]
   end
 
   def test_to_scaled

test/models/note_test.rb

@@ -64,4 +64,12 @@ class NoteTest < ActiveSupport::TestCase
     comment = create(:note_comment, :author_ip => IPAddr.new("192.168.1.1"))
     assert_equal IPAddr.new("192.168.1.1"), comment.note.author_ip
   end
+
+  # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
+  def test_lat_lon_format
+    note = build(:note, :latitude => 0.00004 * GeoRecord::SCALE, :longitude => 0.00008 * GeoRecord::SCALE)
+
+    assert_equal "0.0000400", note.lat.to_s
+    assert_equal "0.0000800", note.lon.to_s
+  end
 end

test/models/old_node_test.rb

@@ -59,6 +59,14 @@ class OldNodeTest < ActiveSupport::TestCase
     assert_in_delta 76.543 * OldNode::SCALE, node.longitude, 0.000001
   end
 
+  # Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
+  def test_lat_lon_xml_format
+    old_node = build(:old_node, :latitude => 0.00004 * OldNode::SCALE, :longitude => 0.00008 * OldNode::SCALE)
+
+    assert_match /lat="0.0000400"/, old_node.to_xml.to_s
+    assert_match /lon="0.0000800"/, old_node.to_xml.to_s
+  end
+
   def test_node_tags
     node_v1 = create(:old_node, :version => 1)
     node_v2 = create(:old_node, :node_id => node_v1.node_id, :version => 2)

test/models/tracepoint_test.rb

@@ -7,4 +7,12 @@ class TracepointTest < ActiveSupport::TestCase
     tracepoint.timestamp = nil
     assert !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