From b6d2f261b579a803f68d4b69ba9f45ce20bc2bac Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sun, 5 Jan 2025 14:10:20 +0300 Subject: [PATCH] Support unwrapped bbox values in changeset history queries --- app/assets/javascripts/index/history.js | 2 +- app/controllers/changesets_controller.rb | 32 ++++-- .../controllers/changesets_controller_test.rb | 104 ++++++++++++++++++ 3 files changed, 126 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/index/history.js b/app/assets/javascripts/index/history.js index ae8f027ed8..e87c10f260 100644 --- a/app/assets/javascripts/index/history.js +++ b/app/assets/javascripts/index/history.js @@ -59,7 +59,7 @@ OSM.History = function (map) { var data = { list: "1" }; if (window.location.pathname === "/history") { - data.bbox = map.getBounds().wrap().toBBoxString(); + data.bbox = map.getBounds().toBBoxString(); var feedLink = $("link[type=\"application/atom+xml\"]"), feedHref = feedLink.attr("href").split("?")[0]; feedLink.attr("href", feedHref + "?bbox=" + data.bbox); diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb index 928f1c1ecf..8ba685133c 100644 --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@ -53,7 +53,10 @@ def index changesets.where("false") end elsif @params[:bbox] - changesets = conditions_bbox(changesets, BoundingBox.from_bbox_params(params)) + bbox_array = @params[:bbox].split(",").map(&:to_f) + raise OSM::APIBadUserInput, "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array.count == 4 + + changesets = conditions_bbox(changesets, *bbox_array) elsif @params[:friends] && current_user changesets = changesets.where(:user => current_user.friends.identifiable) elsif @params[:nearby] && current_user @@ -142,21 +145,28 @@ def unsubscribe #------------------------------------------------------------ ## - # if a bounding box was specified do some sanity checks. # restrict changesets to those enclosed by a bounding box - def conditions_bbox(changesets, bbox) - if bbox - bbox.check_boundaries - bbox = bbox.to_scaled - - changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?", - bbox.max_lon.to_i, bbox.min_lon.to_i, - bbox.max_lat.to_i, bbox.min_lat.to_i) - else + def conditions_bbox(changesets, min_lon, min_lat, max_lon, max_lat) + db_min_lat = (min_lat * GeoRecord::SCALE).to_i + db_max_lat = (max_lat * GeoRecord::SCALE).to_i + db_min_lon = (wrap_lon(min_lon) * GeoRecord::SCALE).to_i + db_max_lon = (wrap_lon(max_lon) * GeoRecord::SCALE).to_i + + changesets = changesets.where("min_lat < ? and max_lat > ?", db_max_lat, db_min_lat) + + if max_lon - min_lon >= 360 changesets + elsif db_min_lon > db_max_lon + changesets.where("min_lon < ? or max_lon > ?", db_max_lon, db_min_lon) + else + changesets.where("min_lon < ? and max_lon > ?", db_max_lon, db_min_lon) end end + def wrap_lon(lon) + ((lon + 180) % 360) - 180 + end + ## # eliminate empty changesets (where the bbox has not been set) # this should be applied to all changeset list displays diff --git a/test/controllers/changesets_controller_test.rb b/test/controllers/changesets_controller_test.rb index a486e4b5ee..717d283cdf 100644 --- a/test/controllers/changesets_controller_test.rb +++ b/test/controllers/changesets_controller_test.rb @@ -130,6 +130,110 @@ def test_index_bbox check_index_result(changesets) end + ## + # Test bbox spanning across the dateline with changesets close to the dateline + def test_index_bbox_dateline + western_changeset = create(:changeset, :num_changes => 1, + :min_lat => 0 * GeoRecord::SCALE, :min_lon => 176 * GeoRecord::SCALE, + :max_lat => 1 * GeoRecord::SCALE, :max_lon => 178 * GeoRecord::SCALE) + eastern_changeset = create(:changeset, :num_changes => 1, + :min_lat => 0 * GeoRecord::SCALE, :min_lon => -178 * GeoRecord::SCALE, + :max_lat => 1 * GeoRecord::SCALE, :max_lon => -176 * GeoRecord::SCALE) + + get history_path(:format => "html", :list => "1") + assert_response :success + check_index_result([eastern_changeset, western_changeset]) + + # negative longitudes + get history_path(:format => "html", :list => "1", :bbox => "-190,-10,-170,10") + assert_response :success + check_index_result([eastern_changeset, western_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "-183,-10,-177,10") + assert_response :success + check_index_result([eastern_changeset, western_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "-181,-10,-177,10") + assert_response :success + check_index_result([eastern_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "-183,-10,-179,10") + assert_response :success + check_index_result([western_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "-181,-10,-179,10") + assert_response :success + check_index_result([]) + + # positive longitudes + get history_path(:format => "html", :list => "1", :bbox => "170,-10,190,10") + assert_response :success + check_index_result([eastern_changeset, western_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "177,-10,183,10") + assert_response :success + check_index_result([eastern_changeset, western_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "177,-10,181,10") + assert_response :success + check_index_result([western_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "179,-10,183,10") + assert_response :success + check_index_result([eastern_changeset]) + + get history_path(:format => "html", :list => "1", :bbox => "179,-10,181,10") + assert_response :success + check_index_result([]) + end + + ## + # Test bbox spanning across the dateline with changesets around the globe + def test_index_bbox_global + changeset1 = create(:changeset, :num_changes => 1, + :min_lat => 40 * GeoRecord::SCALE, :min_lon => -150 * GeoRecord::SCALE, + :max_lat => 50 * GeoRecord::SCALE, :max_lon => -140 * GeoRecord::SCALE) + changeset2 = create(:changeset, :num_changes => 1, + :min_lat => -30 * GeoRecord::SCALE, :min_lon => -30 * GeoRecord::SCALE, + :max_lat => -20 * GeoRecord::SCALE, :max_lon => -20 * GeoRecord::SCALE) + changeset3 = create(:changeset, :num_changes => 1, + :min_lat => 60 * GeoRecord::SCALE, :min_lon => 10 * GeoRecord::SCALE, + :max_lat => 70 * GeoRecord::SCALE, :max_lon => 20 * GeoRecord::SCALE) + changeset4 = create(:changeset, :num_changes => 1, + :min_lat => -60 * GeoRecord::SCALE, :min_lon => 150 * GeoRecord::SCALE, + :max_lat => -50 * GeoRecord::SCALE, :max_lon => 160 * GeoRecord::SCALE) + + # no bbox, get all changesets + get history_path(:format => "html", :list => "1") + assert_response :success + check_index_result([changeset4, changeset3, changeset2, changeset1]) + + # large enough bbox within normal range + get history_path(:format => "html", :list => "1", :bbox => "-170,-80,170,80") + assert_response :success + check_index_result([changeset4, changeset3, changeset2, changeset1]) + + # bbox for [1,2] within normal range + get history_path(:format => "html", :list => "1", :bbox => "-160,-80,0,80") + assert_response :success + check_index_result([changeset2, changeset1]) + + # bbox for [1,4] containing dateline with negative lon + get history_path(:format => "html", :list => "1", :bbox => "-220,-80,-100,80") + assert_response :success + check_index_result([changeset4, changeset1]) + + # bbox for [1,4] containing dateline with positive lon + get history_path(:format => "html", :list => "1", :bbox => "100,-80,220,80") + assert_response :success + check_index_result([changeset4, changeset1]) + + # large enough bbox outside normal range + get history_path(:format => "html", :list => "1", :bbox => "-220,-80,220,80") + assert_response :success + check_index_result([changeset4, changeset3, changeset2, changeset1]) + end + ## # Checks the display of the user changesets listing def test_index_user