Use quad tiling to select bugs in an area

Remove the _no_quadtile versiond of the selection routines and switch
to using the ordinary ones but with a (large) limit applied on the size
of the area to prevent us spending ages working out the list of tiles
to be selected.

diff --git a/app/controllers/map_bugs_controller.rb b/app/controllers/map_bugs_controller.rb
index 063c06fbe380c28dd5dea8540c36ae9fa12456f3..f4105929ffe06a3ef4e3f80a266e739da08eeeb5 100644 (file)
--- a/app/controllers/map_bugs_controller.rb
+++ b/app/controllers/map_bugs_controller.rb
@@ -37,9 +37,9 @@ class MapBugsController < ApplicationController
     limit = getLimit
     conditions = closedCondition
        
-    check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, :false)
+    check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_BUG_REQUEST_AREA)
 
-    @bugs = MapBug.find_by_area_no_quadtile(@min_lat, @min_lon, @max_lat, @max_lon, :include => :comments, :order => "last_changed DESC", :limit => limit, :conditions => conditions)
+    @bugs = MapBug.find_by_area(@min_lat, @min_lon, @max_lat, @max_lon, :include => :comments, :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"}
@@ -138,9 +138,9 @@ class MapBugsController < ApplicationController
       bbox = bbox.split(',')
       @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(bbox)
 
-      conditions = cond_merge conditions, [OSM.sql_for_area_no_quadtile(@min_lat, @min_lon, @max_lat, @max_lon)]
+      check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_BUG_REQUEST_AREA)
 
-      check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, :false)
+      conditions = cond_merge conditions, [OSM.sql_for_area(@min_lat, @min_lon, @max_lat, @max_lon)]
     end
 
     @comments = MapBugComment.find(:all, :limit => limit, :order => "date_created DESC", :joins => :map_bug, :include => :map_bug, :conditions => conditions)

diff --git a/config/example.application.yml b/config/example.application.yml
index cf7565856883570d8fd960f1762e2850e26a4efa..7d7d9ad8b71bd35ab006b9bc36efb938f9af77de 100644 (file)
--- a/config/example.application.yml
+++ b/config/example.application.yml
@@ -24,6 +24,8 @@ standard_settings: &standard_settings
   max_number_of_nodes: 50000
   # Maximum number of nodes that can be in a way (checked on save)
   max_number_of_way_nodes: 2000
+  # The maximum area you're allowed to request bugs from, in square degrees
+  max_bug_request_area: 25
   # Zoom level to use for postcode results from the geocoder
   postcode_zoom: 15
   # Zoom level to use for geonames results from the geocoder

diff --git a/lib/geo_record.rb b/lib/geo_record.rb
index 4aa45531f37a6719c43aa7277896d358c96eda7c..90dee5f1dc43ceb424d1f78c934082d2370e0ce1 100644 (file)
--- a/lib/geo_record.rb
+++ b/lib/geo_record.rb
@@ -54,12 +54,5 @@ private
         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 7d20d2920ba06f25fe317bec859d02a8553fc6a0..b3085d0ec0c83ed6ceb3150bca329eb35d28dd1e 100644 (file)
--- 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, limit_small_area = :true)
+  def check_boundaries(min_lon, min_lat, max_lon, max_lat, max_area = MAX_REQUEST_AREA)
     # 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,12 +22,10 @@ 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 > MAX_REQUEST_AREA
-      raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + MAX_REQUEST_AREA.to_s + 
+    if requested_area > max_area
+      raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + max_area.to_s + 
         ", and your request was too large. Either request a smaller area, or use planet.osm")
     end
   end

diff --git a/lib/osm.rb b/lib/osm.rb
index 78d32e739e39cb7a0c259665ff544e4b4352780e..9ee36e0c7aa6b26db8b085b152d2acf73d52eeec 100644 (file)
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -498,23 +498,14 @@ 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)
-    minlat = (minlat * 10000000).round
-    minlon = (minlon * 10000000).round
-    maxlat = (maxlat * 10000000).round
-    maxlon = (maxlon * 10000000).round
+    tilesql = QuadTile.sql_for_area(minlat, minlon, maxlat, maxlon, prefix)
 
-    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}"
+    return "#{tilesql} AND #{prefix}latitude BETWEEN #{minlat} AND #{maxlat} AND #{prefix}longitude BETWEEN #{minlon} AND #{maxlon}"
   end
 
   # Return a spam score for a chunk of text

diff --git a/test/functional/map_bugs_controller_test.rb b/test/functional/map_bugs_controller_test.rb
index 9aa155c3eb907502710dcd72db3c65e3ae00889c..ba47a007cdbe0b53d3d1510147ac64e97ab422c3 100644 (file)
--- a/test/functional/map_bugs_controller_test.rb
+++ b/test/functional/map_bugs_controller_test.rb
@@ -79,10 +79,15 @@ class MapBugsControllerTest < ActionController::TestCase
   end
 
   def test_get_bugs_large_area_success
-    get :get_bugs, {:bbox=>'-10,-10,12,12'}
+    get :get_bugs, {:bbox=>'-2.5,-2.5,2.5,2.5'}
     assert_response :success
   end
 
+  def test_get_bugs_large_area_bad_request
+    get :get_bugs, {:bbox=>'-10,-10,12,12'}
+    assert_response :bad_request
+  end
+
   def test_get_bugs_closed_7_success
     get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '7'}
     assert_response :success