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

diff --git a/app/models/node.rb b/app/models/node.rb
index fb9bdf68b0dd700b27c55b39fe4b4885cbced563..f4367e45954edf51964b903ce54b733c89f8de0b 100644 (file)
--- a/app/models/node.rb
+++ b/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)

diff --git a/app/views/notes/_note.json.jsonify b/app/views/notes/_note.json.jsonify
index 5e3ac518e7840990e78654089f06f6433952441f..b964399225350d30cf48aa7786e0c4eda41d9fb7 100644 (file)
--- a/app/views/notes/_note.json.jsonify
+++ b/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

diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb
index 11e831cfe08dd70cc6c214d1244224bc519fb974..51972efbccf7261462634d0c1cc7a34c9f2d15a8 100644 (file)
--- a/lib/bounding_box.rb
+++ b/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
 

diff --git a/lib/geo_record.rb b/lib/geo_record.rb
index 09ced972930cc8289ff9b7d76e80a119cca33810..e4a66f9324d81df17129aea5844fa7f65e9cbe59 100644 (file)
--- a/lib/geo_record.rb
+++ b/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

diff --git a/lib/potlatch.rb b/lib/potlatch.rb
index 898a5a07f7a5929420cc509644a91985ba5eeadd..76944e3940de39e27ec8cd9ffe3845afae56a9d3 100644 (file)
--- a/lib/potlatch.rb
+++ b/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

diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb
index 61763d128d448a099aed5dd7fa4ff73919917f9e..b07eb68c9c63416b845861d87e4bfaad04e98ede 100644 (file)
--- a/test/controllers/api_controller_test.rb
+++ b/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

diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb
index c12f1d902e7d46f276a28391aac8b61481d8be55..abdd4cdaadee0f751543b259010b67decaa6a9aa 100644 (file)
--- a/test/controllers/changeset_controller_test.rb
+++ b/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
 
   ##

diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb
index 6aafa3aab3d091253765a2cb88238a7277322064..afdab1bf203a50ce749d0db3140fc8e05e0962be 100644 (file)
--- a/test/controllers/relation_controller_test.rb
+++ b/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
 

diff --git a/test/lib/bounding_box_test.rb b/test/lib/bounding_box_test.rb
index 52d0b83886f66164b88158394c0186658b180679..53095307d244f8c040f620b1831034e1d816aae2 100644 (file)
--- a/test/lib/bounding_box_test.rb
+++ b/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

diff --git a/test/models/note_test.rb b/test/models/note_test.rb
index 0abd0136ee68760fc8aa72071ccb0d5e8c29c226..77ed7f1b2f924b75cbad17042313d42e1ebcea0f 100644 (file)
--- a/test/models/note_test.rb
+++ b/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

diff --git a/test/models/old_node_test.rb b/test/models/old_node_test.rb
index 8b05409577c88af00d0c86456723e3097944e684..42097c8531b18a612ca2e705fa6f9d4eeb7d4763 100644 (file)
--- a/test/models/old_node_test.rb
+++ b/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)

diff --git a/test/models/tracepoint_test.rb b/test/models/tracepoint_test.rb
index 45dd7496f838f836bf301b082b7d1eb772f29802..ae258f43a09b4d9a2b29c1abb44567ded53286b1 100644 (file)
--- a/test/models/tracepoint_test.rb
+++ b/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