From 59a6ed0e207cc83e286e3e9b1235427756f61f7d Mon Sep 17 00:00:00 2001 From: Kai Krueger Date: Sat, 20 Mar 2010 11:59:23 +0000 Subject: [PATCH] Don't limit get map_bugs to a small Area, by disabling the quadtile index for those searches The volumn of bugs seem small enough to get away with normal or GiST indexes (not included yet) Also includes some other fixes --- app/controllers/map_bugs_controller.rb | 52 ++++++++++----------- lib/geo_record.rb | 5 ++ lib/map_boundary.rb | 4 +- lib/osm.rb | 12 ++++- test/functional/map_bugs_controller_test.rb | 5 ++ 5 files changed, 49 insertions(+), 29 deletions(-) diff --git a/app/controllers/map_bugs_controller.rb b/app/controllers/map_bugs_controller.rb index 16d127641..022653db7 100644 --- a/app/controllers/map_bugs_controller.rb +++ b/app/controllers/map_bugs_controller.rb @@ -36,18 +36,9 @@ class MapBugsController < ApplicationController limit = getLimit conditions = closedCondition - # check boundary is sane and area within defined - # see /config/application.yml - begin - check_boundaries(min_lon, min_lat, max_lon, max_lat) - rescue Exception => err - report_error(err.message) - return - end - + check_boundaries(min_lon, min_lat, max_lon, max_lat, :false) - - @bugs = MapBug.find_by_area(min_lat, min_lon, max_lat, max_lon, :order => "last_changed DESC", :limit => limit, :conditions => conditions) + @bugs = MapBug.find_by_area_no_quadtile(min_lat, min_lon, max_lat, max_lon, :include => :map_bug_comment, :order => "last_changed DESC", :limit => limit, :conditions => conditions) respond_to do |format| format.html {render :template => 'map_bugs/get_bugs.js', :content_type => "text/javascript"} @@ -71,21 +62,28 @@ class MapBugsController < ApplicationController name = "NoName"; name = params['name'] if params['name']; - @bug = MapBug.create_bug(lat, lon) - - - #TODO: move this into a helper function - url = "http://nominatim.openstreetmap.org/reverse?lat=" + lat.to_s + "&lon=" + lon.to_s + "&zoom=16" - response = REXML::Document.new(Net::HTTP.get(URI.parse(url))) - - if result = response.get_text("reversegeocode/result") - @bug.nearby_place = result.to_s - else - @bug.nearby_place = "unknown" - end - - @bug.save; - add_comment(@bug, comment, name); + #Include in a transaction to ensure that there is always a map_bug_comment for every map_bug + MapBug.transaction do + @bug = MapBug.create_bug(lat, lon) + + + #TODO: move this into a helper function + begin + url = "http://nominatim.openstreetmap.org/reverse?lat=" + lat.to_s + "&lon=" + lon.to_s + "&zoom=16" + response = REXML::Document.new(Net::HTTP.get(URI.parse(url))) + + if result = response.get_text("reversegeocode/result") + @bug.nearby_place = result.to_s + else + @bug.nearby_place = "unknown" + end + rescue Exception => err + @bug.nearby_place = "unknown" + end + + @bug.save; + add_comment(@bug, comment, name); + end render_ok end @@ -162,7 +160,7 @@ class MapBugsController < ApplicationController #TODO: There should be a better way to do this. CloseConditions are ignored at the moment - bugs2 = MapBug.find(:all, :limit => limit, :order => "last_changed DESC", :joins => :map_bug_comment, + bugs2 = MapBug.find(:all, :limit => limit, :order => "last_changed DESC", :joins => :map_bug_comment, :include => :map_bug_comment, :conditions => conditions) @bugs = bugs2.uniq respond_to do |format| diff --git a/lib/geo_record.rb b/lib/geo_record.rb index 2740eab0c..d44227dd8 100644 --- a/lib/geo_record.rb +++ b/lib/geo_record.rb @@ -53,6 +53,11 @@ private self.with_scope(:find => {:conditions => OSM.sql_for_area(minlat, minlon, maxlat, maxlon)}) do return self.find(:all, options) end + end + def find_by_area_no_quadtile(minlat, minlon, maxlat, maxlon, options) + self.with_scope(:find => {:conditions => OSM.sql_for_area_no_quadtile(minlat, minlon, maxlat, maxlon)}) do + return self.find(:all, options) + end end end end diff --git a/lib/map_boundary.rb b/lib/map_boundary.rb index 6ac7e9f3d..f2a0c0b52 100644 --- a/lib/map_boundary.rb +++ b/lib/map_boundary.rb @@ -9,7 +9,7 @@ module MapBoundary return min_lon, min_lat, max_lon, max_lat end - def check_boundaries(min_lon, min_lat, max_lon, max_lat) + def check_boundaries(min_lon, min_lat, max_lon, max_lat, limit_small_area = :true) # check the bbox is sane unless min_lon <= max_lon raise OSM::APIBadBoundingBox.new("The minimum longitude must be less than the maximum longitude, but it wasn't") @@ -22,6 +22,8 @@ module MapBoundary raise OSM::APIBadBoundingBox.new("The latitudes must be between -90 and 90, and longitudes between -180 and 180") end + return unless limit_small_area == :true + # check the bbox isn't too large requested_area = (max_lat-min_lat)*(max_lon-min_lon) if requested_area > APP_CONFIG['max_request_area'] diff --git a/lib/osm.rb b/lib/osm.rb index cb23b0c97..1358c993a 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -489,7 +489,7 @@ module OSM # Return an SQL fragment to select a given area of the globe def self.sql_for_area(minlat, minlon, maxlat, maxlon, prefix = nil) - tilesql = QuadTile.sql_for_area(minlat, minlon, maxlat, maxlon, prefix) + tilesql = QuadTile.sql_for_area(minlat, minlon, maxlat, maxlon, prefix) minlat = (minlat * 10000000).round minlon = (minlon * 10000000).round maxlat = (maxlat * 10000000).round @@ -498,5 +498,15 @@ module OSM return "#{tilesql} AND #{prefix}latitude BETWEEN #{minlat} AND #{maxlat} AND #{prefix}longitude BETWEEN #{minlon} AND #{maxlon}" end + # Return an SQL fragment to select a given area of the globe without using the quadtile index + def self.sql_for_area_no_quadtile(minlat, minlon, maxlat, maxlon, prefix = nil, without_quadtile = :false) + minlat = (minlat * 10000000).round + minlon = (minlon * 10000000).round + maxlat = (maxlat * 10000000).round + maxlon = (maxlon * 10000000).round + + return "#{prefix}latitude BETWEEN #{minlat} AND #{maxlat} AND #{prefix}longitude BETWEEN #{minlon} AND #{maxlon}" + end + end diff --git a/test/functional/map_bugs_controller_test.rb b/test/functional/map_bugs_controller_test.rb index 8031041bd..b719e857e 100644 --- a/test/functional/map_bugs_controller_test.rb +++ b/test/functional/map_bugs_controller_test.rb @@ -54,6 +54,11 @@ class MapBugsControllerTest < ActionController::TestCase assert_response :success end + def test_get_bugs_large_area_success + get :get_bugs, {:bbox=>'-10,-10,12,12'} + assert_response :success + end + def test_get_bugs_closed_7_success get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '7'} assert_response :success -- 2.43.2