From: Tom Hughes Date: Sun, 3 Sep 2023 17:55:39 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/4223' X-Git-Tag: live~1783 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/b207cf78a3e2263c59ae2b43837c2fbe5990d3a7?hp=2b1019544149f9c9e91fc5d2d08f494b694336d7 Merge remote-tracking branch 'upstream/pull/4223' --- diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb index 0d123fc3e..5a05f6de2 100644 --- a/app/controllers/api/map_controller.rb +++ b/app/controllers/api/map_controller.rb @@ -22,6 +22,8 @@ module Api # check boundary is sane and area within defined # see /config/application.yml begin + raise OSM::APIBadUserInput, "The parameter bbox is required" unless params[:bbox] + @bounds = BoundingBox.from_bbox_params(params) @bounds.check_boundaries @bounds.check_size diff --git a/app/controllers/api/notes_controller.rb b/app/controllers/api/notes_controller.rb index c489f96be..95466781f 100644 --- a/app/controllers/api/notes_controller.rb +++ b/app/controllers/api/notes_controller.rb @@ -18,13 +18,10 @@ module Api # support the old, deprecated, method with four arguments if params[:bbox] bbox = BoundingBox.from_bbox_params(params) - else - raise OSM::APIBadUserInput, "No l was given" unless params[:l] - raise OSM::APIBadUserInput, "No r was given" unless params[:r] - raise OSM::APIBadUserInput, "No b was given" unless params[:b] - raise OSM::APIBadUserInput, "No t was given" unless params[:t] - + elsif params[:l] && params[:r] && params[:b] && params[:t] bbox = BoundingBox.from_lrbt_params(params) + else + raise OSM::APIBadUserInput, "The parameter bbox is required" end # Get any conditions that need to be applied @@ -235,20 +232,7 @@ module Api def feed # Get any conditions that need to be applied notes = closed_condition(Note.all) - - # Process any bbox - if params[:bbox] - bbox = BoundingBox.from_bbox_params(params) - - bbox.check_boundaries - bbox.check_size(Settings.max_note_request_area) - - notes = notes.bbox(bbox) - @min_lon = bbox.min_lon - @min_lat = bbox.min_lat - @max_lon = bbox.max_lon - @max_lat = bbox.max_lat - end + notes = bbox_condition(notes) # Find the comments we want to return @comments = NoteComment.where(:note => notes) @@ -266,6 +250,7 @@ module Api def search # Get the initial set of notes @notes = closed_condition(Note.all) + @notes = bbox_condition(@notes) # Add any user filter if params[:display_name] || params[:user] @@ -378,6 +363,27 @@ module Api end end + ## + # Generate a condition to choose which notes we want based + # on the user's bounding box request parameters + def bbox_condition(notes) + if params[:bbox] + bbox = BoundingBox.from_bbox_params(params) + + bbox.check_boundaries + bbox.check_size(Settings.max_note_request_area) + + @min_lon = bbox.min_lon + @min_lat = bbox.min_lat + @max_lon = bbox.max_lon + @max_lat = bbox.max_lat + + notes.bbox(bbox) + else + notes + end + end + ## # Add a comment to a note def add_comment(note, text, event, notify: true) diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb index e8bd97b64..f38351de9 100644 --- a/app/controllers/api/tracepoints_controller.rb +++ b/app/controllers/api/tracepoints_controller.rb @@ -23,6 +23,8 @@ module Api # check boundary is sane and area within defined # see /config/application.yml begin + raise OSM::APIBadUserInput, "The parameter bbox is required" unless params[:bbox] + bbox = BoundingBox.from_bbox_params(params) bbox.check_boundaries bbox.check_size diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index 9eba0a831..0cc4c5fd4 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -157,7 +157,7 @@ class BoundingBox private def from_bbox_array(bbox_array) - raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array + raise OSM::APIBadUserInput, "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and # max_lat within their respective boundaries. diff --git a/test/controllers/api/map_controller_test.rb b/test/controllers/api/map_controller_test.rb index 1e96e353c..c050100ae 100644 --- a/test/controllers/api/map_controller_test.rb +++ b/test/controllers/api/map_controller_test.rb @@ -278,7 +278,7 @@ module Api def test_map_without_bbox get map_path assert_response :bad_request - assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "A bbox param was expected" + assert_equal "The parameter bbox is required", @response.body, "A bbox param was expected" end def test_bbox_too_big @@ -293,7 +293,7 @@ module Api @badmalformedbbox.each do |bbox| get map_path(:bbox => bbox) assert_response :bad_request, "The bbox:#{bbox} was expected to be malformed" - assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "bbox: #{bbox}" + assert_equal "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "bbox: #{bbox}" end end diff --git a/test/controllers/api/notes_controller_test.rb b/test/controllers/api/notes_controller_test.rb index 81e6b0ba1..874ac3ccd 100644 --- a/test/controllers/api/notes_controller_test.rb +++ b/test/controllers/api/notes_controller_test.rb @@ -786,6 +786,10 @@ module Api end def test_index_bad_params + get api_notes_path + assert_response :bad_request + assert_equal "The parameter bbox is required", @response.body + get api_notes_path(:bbox => "-2.5,-2.5,2.5") assert_response :bad_request @@ -927,6 +931,28 @@ module Api end end + def test_search_by_bbox_success + notes = Array.new(5) do |i| + position = ((1.0 + (i * 0.1)) * GeoRecord::SCALE).to_i + create(:note_with_comments, :created_at => Time.parse("2020-01-01T00:00:00Z") + i.day, :latitude => position, :longitude => position) + end + + get search_api_notes_path(:bbox => "1.0,1.0,1.6,1.6", :sort => "created_at", :order => "oldest", :format => "xml") + assert_response :success + assert_equal "application/xml", @response.media_type + assert_notes_in_order notes + + get search_api_notes_path(:bbox => "1.25,1.25,1.45,1.45", :sort => "created_at", :order => "oldest", :format => "xml") + assert_response :success + assert_equal "application/xml", @response.media_type + assert_notes_in_order [notes[3], notes[4]] + + get search_api_notes_path(:bbox => "2.0,2.0,2.5,2.5", :sort => "created_at", :order => "oldest", :format => "xml") + assert_response :success + assert_equal "application/xml", @response.media_type + assert_notes_in_order [] + end + def test_search_no_match create(:note_with_comments) @@ -1061,5 +1087,14 @@ module Api get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.max_note_query_limit + 1, :format => "rss") assert_response :bad_request end + + private + + def assert_notes_in_order(notes) + assert_select "osm>note", notes.size + notes.each_with_index do |note, index| + assert_select "osm>note:nth-child(#{index + 1})>id", :text => note.id.to_s, :count => 1 + end + end end end diff --git a/test/controllers/api/tracepoints_controller_test.rb b/test/controllers/api/tracepoints_controller_test.rb index 7d561522c..aeea3f4f4 100644 --- a/test/controllers/api/tracepoints_controller_test.rb +++ b/test/controllers/api/tracepoints_controller_test.rb @@ -102,7 +102,7 @@ module Api def test_index_without_bbox get trackpoints_path assert_response :bad_request - assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "A bbox param was expected" + assert_equal "The parameter bbox is required", @response.body, "A bbox param was expected" end def test_traces_page_less_than_zero @@ -129,7 +129,7 @@ module Api @badmalformedbbox.each do |bbox| get trackpoints_path(:bbox => bbox) assert_response :bad_request, "The bbox:#{bbox} was expected to be malformed" - assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "bbox: #{bbox}" + assert_equal "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "bbox: #{bbox}" end end diff --git a/test/lib/bounding_box_test.rb b/test/lib/bounding_box_test.rb index d82e2c9ae..d176e0fa7 100644 --- a/test/lib/bounding_box_test.rb +++ b/test/lib/bounding_box_test.rb @@ -3,7 +3,7 @@ require "test_helper" class BoundingBoxTest < ActiveSupport::TestCase def setup @size_error_message = "The maximum bbox size is 0.25, and your request was too large. Either request a smaller area, or use planet.osm" - @malformed_error_message = "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" + @malformed_error_message = "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat" @lon_order_error_message = "The minimum longitude must be less than the maximum longitude, but it wasn't" @lat_order_error_message = "The minimum latitude must be less than the maximum latitude, but it wasn't" @bbox_out_of_limits_error_message = "The latitudes must be between -90.0 and 90.0, and longitudes between -180.0 and 180.0"