From: Tom Hughes Date: Sat, 13 Jul 2024 14:02:23 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/4979' X-Git-Tag: live~944 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/76a7e98f0715cf10792a0b611ae97948879d7a2b?hp=57a17980922b8d275ad222f00c2d458b8b2c70d5 Merge remote-tracking branch 'upstream/pull/4979' --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fdc7c35a5..6fe5b2e57 100644 --- 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 05fa76658..c6223fb9d 100644 --- 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 8ec0ab6b7..ca6bab6df 100644 --- 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])\s*(?\d{1,3}(?:\.\d+)?)°?\W*(?[EW])\s*(?\d{1,3}(?:\.\d+)?)°?$/) || # [NSEW] decimal degrees + query.match(/^(?\d{1,3}(?:\.\d+)?)°?\s*(?[NS])\W*(?\d{1,3}(?:\.\d+)?)°?\s*(?[EW])$/) || # decimal degrees [NSEW] + query.match(/^(?[NS])\s*(?\d{1,3})°?(?:\s*(?\d{1,3}(?:\.\d+)?)['′]?)\W*(?[EW])\s*(?\d{1,3})°?(?:\s*(?\d{1,3}(?:\.\d+)?)['′]?)$/) || # [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])$/) || # degrees, decimal minutes [NSEW] + 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+)?)["″]?)$/) || # [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])$/) # 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{^(?[+-]?\d+(?:\.\d+)?)(?:\s+|\s*[,/]\s*)(?[+-]?\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 adeabb184..ec38f7421 100644 --- 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 1d86de208..0278bb8f2 100644 --- 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 d32521c3b..5b7605321 100644 --- 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)