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

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index fdc7c35a5343242903305b93d5ec5ab9ff4b2ecf..6fe5b2e57a222f91f042f6ee05ded916b027ee11 100644 (file)
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -24,7 +24,7 @@ FactoryBot/ExcessiveCreateList:
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https
 Layout/LineLength:
-  Max: 248
+  Max: 266
 
 # Offense count: 29
 # This cop supports unsafe autocorrection (--autocorrect-all).

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 05fa76658d55bedeecf412c4a5d1927cb6ce3899..c6223fb9d6228890ea86c6bc6420a14d58f2f9fe 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -181,7 +181,7 @@ class ApplicationController < ActionController::Base
     # TODO: some sort of escaping of problem characters in the message
     response.headers["Error"] = message
 
-    if request.headers["X-Error-Format"]&.casecmp("xml")&.zero?
+    if request.headers["X-Error-Format"]&.casecmp?("xml")
       result = OSM::API.new.xml_doc
       result.root.name = "osmError"
       result.root << (XML::Node.new("status") << "#{Rack::Utils.status_code(status)} #{Rack::Utils::HTTP_STATUS_CODES[status]}")

diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb
index 8ec0ab6b70acbb6c6e645e9a25229347ebfa0c9b..ca6bab6df88a1e79ed4947391d22864e639c6d80 100644 (file)
--- a/app/controllers/geocoder_controller.rb
+++ b/app/controllers/geocoder_controller.rb
@@ -35,15 +35,15 @@ class GeocoderController < ApplicationController
       @results = []
 
       if lat >= -90 && lat <= 90 && lon >= -180 && lon <= 180
-        @results.push(:lat => lat, :lon => lon,
+        @results.push(:lat => params[:lat], :lon => params[:lon],
                       :zoom => params[:zoom],
-                      :name => "#{lat}, #{lon}")
+                      :name => "#{params[:lat]}, #{params[:lon]}")
       end
 
       if lon >= -90 && lon <= 90 && lat >= -180 && lat <= 180
-        @results.push(:lat => lon, :lon => lat,
+        @results.push(:lat => params[:lon], :lon => params[:lat],
                       :zoom => params[:zoom],
-                      :name => "#{lon}, #{lat}")
+                      :name => "#{params[:lon]}, #{params[:lat]}")
       end
 
       if @results.empty?
@@ -61,9 +61,9 @@ class GeocoderController < ApplicationController
         @error = "Longitude #{lon} out of range"
         render :action => "error"
       else
-        @results = [{ :lat => lat, :lon => lon,
+        @results = [{ :lat => params[:lat], :lon => params[:lon],
                       :zoom => params[:zoom],
-                      :name => "#{lat}, #{lon}" }]
+                      :name => "#{params[:lat]}, #{params[:lon]}" }]
 
         render :action => "results"
       end
@@ -206,20 +206,16 @@ class GeocoderController < ApplicationController
     if query = params[:query]
       query.strip!
 
-      if latlon = query.match(/^([NS])\s*(\d{1,3}(\.\d*)?)\W*([EW])\s*(\d{1,3}(\.\d*)?)$/).try(:captures) || # [NSEW] decimal degrees
-                  query.match(/^(\d{1,3}(\.\d*)?)\s*([NS])\W*(\d{1,3}(\.\d*)?)\s*([EW])$/).try(:captures)    # decimal degrees [NSEW]
-        params.merge!(nsew_to_decdeg(latlon)).delete(:query)
+      if latlon = query.match(/^(?<ns>[NS])\s*(?<nsd>\d{1,3}(?:\.\d+)?)°?\W*(?<ew>[EW])\s*(?<ewd>\d{1,3}(?:\.\d+)?)°?$/) ||                                                                                                     # [NSEW] decimal degrees
+                  query.match(/^(?<nsd>\d{1,3}(?:\.\d+)?)°?\s*(?<ns>[NS])\W*(?<ewd>\d{1,3}(?:\.\d+)?)°?\s*(?<ew>[EW])$/) ||                                                                                                     # decimal degrees [NSEW]
+                  query.match(/^(?<ns>[NS])\s*(?<nsd>\d{1,3})°?(?:\s*(?<nsm>\d{1,3}(?:\.\d+)?)['′]?)\W*(?<ew>[EW])\s*(?<ewd>\d{1,3})°?(?:\s*(?<ewm>\d{1,3}(?:\.\d+)?)['′]?)$/) ||                                               # [NSEW] degrees, decimal minutes
+                  query.match(/^(?<nsd>\d{1,3})°?(?:\s*(?<nsm>\d{1,3}(?:\.\d+)?)['′]?)\s*(?<ns>[NS])\W*(?<ewd>\d{1,3})°?(?:\s*(?<ewm>\d{1,3}(?:\.\d+)?)['′]?)\s*(?<ew>[EW])$/) ||                                               # degrees, decimal minutes [NSEW]
+                  query.match(/^(?<ns>[NS])\s*(?<nsd>\d{1,3})°?\s*(?<nsm>\d{1,2})['′]?(?:\s*(?<nss>\d{1,3}(?:\.\d+)?)["″]?)\W*(?<ew>[EW])\s*(?<ewd>\d{1,3})°?\s*(?<ewm>\d{1,2})['′]?(?:\s*(?<ews>\d{1,3}(?:\.\d+)?)["″]?)$/) || # [NSEW] degrees, minutes, decimal seconds
+                  query.match(/^(?<nsd>\d{1,3})°?\s*(?<nsm>\d{1,2})['′]?(?:\s*(?<nss>\d{1,3}(?:\.\d+)?)["″]?)\s*(?<ns>[NS])\W*(?<ewd>\d{1,3})°?\s*(?<ewm>\d{1,2})['′]?(?:\s*(?<ews>\d{1,3}(?:\.\d+)?)["″]?)\s*(?<ew>[EW])$/)    # degrees, minutes, decimal seconds [NSEW]
+        params.merge!(to_decdeg(latlon.named_captures)).delete(:query)
 
-      elsif latlon = query.match(/^([NS])\s*(\d{1,3})°?(?:\s*(\d{1,3}(\.\d*)?)?['′]?)?\W*([EW])\s*(\d{1,3})°?(?:\s*(\d{1,3}(\.\d*)?)?['′]?)?$/).try(:captures) || # [NSEW] degrees, decimal minutes
-                     query.match(/^(\d{1,3})°?(?:\s*(\d{1,3}(\.\d*)?)?['′]?)?\s*([NS])\W*(\d{1,3})°?(?:\s*(\d{1,3}(\.\d*)?)?['′]?)?\s*([EW])$/).try(:captures)    # degrees, decimal minutes [NSEW]
-        params.merge!(ddm_to_decdeg(latlon)).delete(:query)
-
-      elsif latlon = query.match(/^([NS])\s*(\d{1,3})°?\s*(\d{1,2})['′]?(?:\s*(\d{1,3}(\.\d*)?)?["″]?)?\W*([EW])\s*(\d{1,3})°?\s*(\d{1,2})['′]?(?:\s*(\d{1,3}(\.\d*)?)?["″]?)?$/).try(:captures) || # [NSEW] degrees, minutes, decimal seconds
-                     query.match(/^(\d{1,3})°?\s*(\d{1,2})['′]?(?:\s*(\d{1,3}(\.\d*)?)?["″]?)?\s*([NS])\W*(\d{1,3})°?\s*(\d{1,2})['′]?(?:\s*(\d{1,3}(\.\d*)?)?["″]?)?\s*([EW])$/).try(:captures)    # degrees, minutes, decimal seconds [NSEW]
-        params.merge!(dms_to_decdeg(latlon)).delete(:query)
-
-      elsif latlon = query.match(%r{^([+-]?\d+(\.\d*)?)(?:\s+|\s*[,/]\s*)([+-]?\d+(\.\d*)?)$})
-        params.merge!(:lat => latlon[1].to_f, :lon => latlon[3].to_f).delete(:query)
+      elsif latlon = query.match(%r{^(?<lat>[+-]?\d+(?:\.\d+)?)(?:\s+|\s*[,/]\s*)(?<lon>[+-]?\d+(?:\.\d+)?)$})
+        params.merge!(:lat => latlon["lat"], :lon => latlon["lon"]).delete(:query)
 
         params[:latlon_digits] = true
       end
@@ -228,39 +224,20 @@ class GeocoderController < ApplicationController
     params.permit(:query, :lat, :lon, :latlon_digits, :zoom, :minlat, :minlon, :maxlat, :maxlon)
   end
 
-  def nsew_to_decdeg(captures)
-    begin
-      Float(captures[0])
-      lat = captures[2].casecmp("s").zero? ? -captures[0].to_f : captures[0].to_f
-      lon = captures[5].casecmp("w").zero? ? -captures[3].to_f : captures[3].to_f
-    rescue StandardError
-      lat = captures[0].casecmp("s").zero? ? -captures[1].to_f : captures[1].to_f
-      lon = captures[3].casecmp("w").zero? ? -captures[4].to_f : captures[4].to_f
-    end
-    { :lat => lat, :lon => lon }
-  end
+  def to_decdeg(captures)
+    ns = captures.fetch("ns").casecmp?("s") ? -1 : 1
+    nsd = BigDecimal(captures.fetch("nsd", "0"))
+    nsm = BigDecimal(captures.fetch("nsm", "0"))
+    nss = BigDecimal(captures.fetch("nss", "0"))
 
-  def ddm_to_decdeg(captures)
-    begin
-      Float(captures[0])
-      lat = captures[3].casecmp("s").zero? ? -(captures[0].to_f + (captures[1].to_f / 60)) : captures[0].to_f + (captures[1].to_f / 60)
-      lon = captures[7].casecmp("w").zero? ? -(captures[4].to_f + (captures[5].to_f / 60)) : captures[4].to_f + (captures[5].to_f / 60)
-    rescue StandardError
-      lat = captures[0].casecmp("s").zero? ? -(captures[1].to_f + (captures[2].to_f / 60)) : captures[1].to_f + (captures[2].to_f / 60)
-      lon = captures[4].casecmp("w").zero? ? -(captures[5].to_f + (captures[6].to_f / 60)) : captures[5].to_f + (captures[6].to_f / 60)
-    end
-    { :lat => lat, :lon => lon }
-  end
+    ew = captures.fetch("ew").casecmp?("w") ? -1 : 1
+    ewd = BigDecimal(captures.fetch("ewd", "0"))
+    ewm = BigDecimal(captures.fetch("ewm", "0"))
+    ews = BigDecimal(captures.fetch("ews", "0"))
 
-  def dms_to_decdeg(captures)
-    begin
-      Float(captures[0])
-      lat = captures[4].casecmp("s").zero? ? -(captures[0].to_f + ((captures[1].to_f + (captures[2].to_f / 60)) / 60)) : captures[0].to_f + ((captures[1].to_f + (captures[2].to_f / 60)) / 60)
-      lon = captures[9].casecmp("w").zero? ? -(captures[5].to_f + ((captures[6].to_f + (captures[7].to_f / 60)) / 60)) : captures[5].to_f + ((captures[6].to_f + (captures[7].to_f / 60)) / 60)
-    rescue StandardError
-      lat = captures[0].casecmp("s").zero? ? -(captures[1].to_f + ((captures[2].to_f + (captures[3].to_f / 60)) / 60)) : captures[1].to_f + ((captures[2].to_f + (captures[3].to_f / 60)) / 60)
-      lon = captures[5].casecmp("w").zero? ? -(captures[6].to_f + ((captures[7].to_f + (captures[8].to_f / 60)) / 60)) : captures[6].to_f + ((captures[7].to_f + (captures[8].to_f / 60)) / 60)
-    end
-    { :lat => lat, :lon => lon }
+    lat = ns * (nsd + (nsm / 60) + (nss / 3600))
+    lon = ew * (ewd + (ewm / 60) + (ews / 3600))
+
+    { :lat => lat.round(6).to_s("F"), :lon => lon.round(6).to_s("F") }
   end
 end

diff --git a/app/models/request_token.rb b/app/models/request_token.rb
index adeabb18497ff4fd7e0556df56aecd16579a9be1..ec38f7421f47f70101b5bebc44a30279d11258f6 100644 (file)
--- a/app/models/request_token.rb
+++ b/app/models/request_token.rb
@@ -73,7 +73,7 @@ class RequestToken < OauthToken
   end
 
   def oob?
-    callback_url.nil? || callback_url.casecmp("oob").zero?
+    callback_url.nil? || callback_url.casecmp?("oob")
   end
 
   def oauth10?

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 1d86de20820ca29752ac71fd226ee5f956fae2e7..0278bb8f2fc6a2a3ef0a46286ca7de337de51ddc 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -386,7 +386,7 @@ en:
         way: "way"
         relation: "relation"
     start_rjs:
-      feature_warning: "Loading %{num_features} features, which may make your browser slow or unresponsive. Are sure you want to display this data?"
+      feature_warning: "Loading %{num_features} features, which may make your browser slow or unresponsive. Are you sure you want to display this data?"
       load_data: "Load Data"
       loading: "Loading..."
     tag_details:

diff --git a/test/controllers/geocoder_controller_test.rb b/test/controllers/geocoder_controller_test.rb
index d32521c3bf42565e7663a0a529666ed3ffb537c5..5b76053214b22a8281e13f890790dfb515666606 100644 (file)
--- a/test/controllers/geocoder_controller_test.rb
+++ b/test/controllers/geocoder_controller_test.rb
@@ -60,6 +60,19 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
+  ##
+  # Test identification of integer lat/lon pairs using N/E with degrees
+  def test_identify_latlon_ne_d_int_deg
+    [
+      "N50 E14",
+      "N50° E14°",
+      "50N 14E",
+      "50°N 14°E"
+    ].each do |code|
+      latlon_check code, 50, 14
+    end
+  end
+
   ##
   # Test identification of lat/lon pairs using N/W with degrees
   def test_identify_latlon_nw_d
@@ -223,6 +236,43 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
+  ##
+  # Test identification of lat/lon pairs with missing fractions
+  def test_no_identify_latlon_ne_missing_fraction_part
+    [
+      "N50. E14.",
+      "N50.° E14.°",
+      "50.N 14.E",
+      "50.°N 14.°E",
+      "N50 1.' E14 2.'",
+      "N50° 1.' E14° 2.'",
+      "50N 1.' 14 2.'E",
+      "50° 1.'N 14° 2.'E",
+      "N50 1' 3,\" E14 2' 4.\"",
+      "N50° 1' 3.\" E14° 2' 4.\"",
+      "50N 1' 3.\" 14 2' 4.\"E",
+      "50° 1' 3.\"N 14° 2' 4.\"E"
+    ].each do |code|
+      get search_path(:query => code)
+      assert_response :success
+      assert_template :search
+      assert_template :layout => "map"
+      assert_equal %w[osm_nominatim], assigns(:sources).pluck(:name)
+    end
+  end
+
+  #
+  # Test identification of lat/lon pairs with values close to zero
+  def test_identify_latlon_close_to_zero
+    [
+      "0.0000123 -0.0000456",
+      "+0.0000123 -0.0000456",
+      "N 0° 0' 0.4428\", W 0° 0' 1.6416\""
+    ].each do |code|
+      latlon_check code, 0.0000123, -0.0000456
+    end
+  end
+
   ##
   # Test identification of US zipcodes
   def test_identify_us_postcode
@@ -368,8 +418,10 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     assert_template :layout => "map"
     assert_equal %w[latlon osm_nominatim_reverse], assigns(:sources).pluck(:name)
     assert_nil @controller.params[:query]
-    assert_in_delta lat, @controller.params[:lat]
-    assert_in_delta lon, @controller.params[:lon]
+    assert_match(/^[+-]?\d+(?:\.\d+)?$/, @controller.params[:lat])
+    assert_match(/^[+-]?\d+(?:\.\d+)?$/, @controller.params[:lon])
+    assert_in_delta lat, @controller.params[:lat].to_f
+    assert_in_delta lon, @controller.params[:lon].to_f
 
     get search_path(:query => query), :xhr => true
     assert_response :success
@@ -377,8 +429,10 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     assert_template :layout => "xhr"
     assert_equal %w[latlon osm_nominatim_reverse], assigns(:sources).pluck(:name)
     assert_nil @controller.params[:query]
-    assert_in_delta lat, @controller.params[:lat]
-    assert_in_delta lon, @controller.params[:lon]
+    assert_match(/^[+-]?\d+(?:\.\d+)?$/, @controller.params[:lat])
+    assert_match(/^[+-]?\d+(?:\.\d+)?$/, @controller.params[:lon])
+    assert_in_delta lat, @controller.params[:lat].to_f
+    assert_in_delta lon, @controller.params[:lon].to_f
   end
 
   def search_check(query, sources)