Refactor bounding box code

Moved duplicated code into the BoundingBox class, and pass around
BoundingBox objects instead of lists of bounds.
This commit is contained in:
Peter Gray 2011-10-24 10:45:56 +01:00 committed by Tom Hughes
parent d70fd8ab1a
commit 95d899786a
21 changed files with 581 additions and 247 deletions

View file

@ -38,9 +38,6 @@
class AmfController < ApplicationController
include Potlatch
# Help methods for checking boundary sanity and area size
include MapBoundary
skip_before_filter :verify_authenticity_token
before_filter :check_api_writable
@ -265,15 +262,17 @@ class AmfController < ApplicationController
# check boundary is sane and area within defined
# see /config/application.yml
check_boundaries(xmin, ymin, xmax, ymax)
bbox = BoundingBox.new(xmin, ymin, xmax, ymax)
bbox.check_boundaries
bbox.check_size
if POTLATCH_USE_SQL then
ways = sql_find_ways_in_area(xmin, ymin, xmax, ymax)
points = sql_find_pois_in_area(xmin, ymin, xmax, ymax)
relations = sql_find_relations_in_area_and_ways(xmin, ymin, xmax, ymax, ways.collect {|x| x[0]})
ways = sql_find_ways_in_area(bbox)
points = sql_find_pois_in_area(bbox)
relations = sql_find_relations_in_area_and_ways(bbox, ways.collect {|x| x[0]})
else
# find the way ids in an area
nodes_in_area = Node.bbox(ymin, xmin, ymax, xmax).visible.includes(:ways)
nodes_in_area = Node.bbox(bbox).visible.includes(:ways)
ways = nodes_in_area.inject([]) { |sum, node|
visible_ways = node.ways.select { |w| w.visible? }
sum + visible_ways.collect { |w| [w.id,w.version] }
@ -305,9 +304,11 @@ class AmfController < ApplicationController
# check boundary is sane and area within defined
# see /config/application.yml
check_boundaries(xmin, ymin, xmax, ymax)
bbox = BoundingBox.new(xmin, ymin, xmax, ymax)
bbox.check_boundaries
bbox.check_size
nodes_in_area = Node.bbox(ymin, xmin, ymax, xmax).joins(:ways_via_history).where(:current_ways => { :visible => false })
nodes_in_area = Node.bbox(bbox).joins(:ways_via_history).where(:current_ways => { :visible => false })
way_ids = nodes_in_area.collect { |node| node.ways_via_history.invisible.collect { |way| way.id } }.flatten.uniq
[0,'',way_ids]
@ -904,7 +905,7 @@ class AmfController < ApplicationController
# ====================================================================
# Alternative SQL queries for getway/whichways
def sql_find_ways_in_area(xmin,ymin,xmax,ymax)
def sql_find_ways_in_area(bbox)
sql=<<-EOF
SELECT DISTINCT current_ways.id AS wayid,current_ways.version AS version
FROM current_way_nodes
@ -912,12 +913,12 @@ class AmfController < ApplicationController
INNER JOIN current_ways ON current_ways.id =current_way_nodes.id
WHERE current_nodes.visible=TRUE
AND current_ways.visible=TRUE
AND #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "current_nodes.")}
AND #{OSM.sql_for_area(bbox, "current_nodes.")}
EOF
return ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['wayid'].to_i,a['version'].to_i] }
end
def sql_find_pois_in_area(xmin,ymin,xmax,ymax)
def sql_find_pois_in_area(bbox)
pois=[]
sql=<<-EOF
SELECT current_nodes.id,current_nodes.latitude*0.0000001 AS lat,current_nodes.longitude*0.0000001 AS lon,current_nodes.version
@ -925,7 +926,7 @@ class AmfController < ApplicationController
LEFT OUTER JOIN current_way_nodes cwn ON cwn.node_id=current_nodes.id
WHERE current_nodes.visible=TRUE
AND cwn.id IS NULL
AND #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "current_nodes.")}
AND #{OSM.sql_for_area(bbox, "current_nodes.")}
EOF
ActiveRecord::Base.connection.select_all(sql).each do |row|
poitags={}
@ -937,7 +938,7 @@ class AmfController < ApplicationController
pois
end
def sql_find_relations_in_area_and_ways(xmin,ymin,xmax,ymax,way_ids)
def sql_find_relations_in_area_and_ways(bbox,way_ids)
# ** It would be more Potlatchy to get relations for nodes within ways
# during 'getway', not here
sql=<<-EOF
@ -945,7 +946,7 @@ class AmfController < ApplicationController
FROM current_relations cr
INNER JOIN current_relation_members crm ON crm.id=cr.id
INNER JOIN current_nodes cn ON crm.member_id=cn.id AND crm.member_type='Node'
WHERE #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "cn.")}
WHERE #{OSM.sql_for_area(bbox, "cn.")}
EOF
unless way_ids.empty?
sql+=<<-EOF

View file

@ -5,9 +5,6 @@ class ApiController < ApplicationController
after_filter :compress_output
around_filter :api_call_handle_error, :api_call_timeout
# Help methods for checking boundary sanity and area size
include MapBoundary
# Get an XML response containing a list of tracepoints that have been uploaded
# within the specified bounding box, and in the specified page.
def trackpoints
@ -22,25 +19,19 @@ class ApiController < ApplicationController
offset = page * TRACEPOINTS_PER_PAGE
# Figure out the bbox
bbox = params['bbox']
unless bbox and bbox.count(',') == 3
report_error("The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat")
return
end
bbox = bbox.split(',')
min_lon, min_lat, max_lon, max_lat = sanitise_boundaries(bbox)
# check boundary is sane and area within defined
# see /config/application.yml
begin
check_boundaries(min_lon, min_lat, max_lon, max_lat)
bbox = BoundingBox.from_bbox_params(params)
bbox.check_boundaries
bbox.check_size
rescue Exception => err
report_error(err.message)
return
end
# get all the points
points = Tracepoint.bbox(min_lat, min_lon, max_lat, max_lon).offset(offset).limit(TRACEPOINTS_PER_PAGE).order("gpx_id DESC, trackid ASC, timestamp ASC")
points = Tracepoint.bbox(bbox).offset(offset).limit(TRACEPOINTS_PER_PAGE).order("gpx_id DESC, trackid ASC, timestamp ASC")
doc = XML::Document.new
doc.encoding = XML::Encoding::UTF_8
@ -124,29 +115,18 @@ class ApiController < ApplicationController
# fetched. Finally all the xml is returned.
def map
# Figure out the bbox
bbox = params['bbox']
unless bbox and bbox.count(',') == 3
# alternatively: report_error(TEXT['boundary_parameter_required']
report_error("The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat")
return
end
bbox = bbox.split(',')
min_lon, min_lat, max_lon, max_lat = sanitise_boundaries(bbox)
# check boundary is sane and area within defined
# see /config/application.yml
begin
check_boundaries(min_lon, min_lat, max_lon, max_lat)
bbox = BoundingBox.from_bbox_params(params)
bbox.check_boundaries
bbox.check_size
rescue Exception => err
report_error(err.message)
return
end
# FIXME um why is this area using a different order for the lat/lon from above???
@nodes = Node.bbox(min_lat, min_lon, max_lat, max_lon).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES+1)
@nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES+1)
# get all the nodes, by tag not yet working, waiting for change from NickB
# need to be @nodes (instance var) so tests in /spec can be performed
#@nodes = Node.search(bbox, params[:tag])
@ -164,12 +144,7 @@ class ApiController < ApplicationController
doc = OSM::API.new.get_xml_doc
# add bounds
bounds = XML::Node.new 'bounds'
bounds['minlat'] = min_lat.to_s
bounds['minlon'] = min_lon.to_s
bounds['maxlat'] = max_lat.to_s
bounds['maxlon'] = max_lon.to_s
doc.root << bounds
doc.root << bbox.add_bounds_to(XML::Node.new 'bounds')
# get ways
# find which ways are needed

View file

@ -17,9 +17,6 @@ class ChangesetController < ApplicationController
around_filter :api_call_handle_error, :except => [:list]
around_filter :web_timeout, :only => [:list]
# Help methods for checking boundary sanity and area size
include MapBoundary
# Helper methods for checking consistency
include ConsistencyValidations
@ -202,11 +199,11 @@ class ChangesetController < ApplicationController
# create the conditions that the user asked for. some or all of
# these may be nil.
changesets = Changeset.scoped
changesets = conditions_bbox(changesets, params['bbox'])
changesets = conditions_user(changesets, params['user'], params['display_name'])
changesets = conditions_time(changesets, params['time'])
changesets = conditions_open(changesets, params['open'])
changesets = conditions_closed(changesets, params['closed'])
changesets, bbox = conditions_bbox(changesets, params)
changesets = conditions_user(changesets, params['user'], params['display_name'])
changesets = conditions_time(changesets, params['time'])
changesets = conditions_open(changesets, params['open'])
changesets = conditions_closed(changesets, params['closed'])
# create the results document
results = OSM::API.new.get_xml_doc
@ -270,15 +267,9 @@ class ChangesetController < ApplicationController
end
end
if params[:bbox]
bbox = params[:bbox]
elsif params[:minlon] and params[:minlat] and params[:maxlon] and params[:maxlat]
bbox = params[:minlon] + ',' + params[:minlat] + ',' + params[:maxlon] + ',' + params[:maxlat]
end
changesets, bbox = conditions_bbox(changesets, params)
if bbox
changesets = conditions_bbox(changesets, bbox)
bbox = BoundingBox.from_s(bbox)
bbox_link = render_to_string :partial => "bbox", :object => bbox
end
@ -319,22 +310,24 @@ private
#------------------------------------------------------------
##
# if a bounding box was specified then parse it and do some sanity
# checks. this is mostly the same as the map call, but without the
# area restriction.
def conditions_bbox(changesets, bbox)
unless bbox.nil?
raise OSM::APIBadUserInput.new("Bounding box should be min_lon,min_lat,max_lon,max_lat") unless bbox.count(',') == 3
bbox = sanitise_boundaries(bbox.split(/,/))
raise OSM::APIBadUserInput.new("Minimum longitude should be less than maximum.") unless bbox[0] <= bbox[2]
raise OSM::APIBadUserInput.new("Minimum latitude should be less than maximum.") unless bbox[1] <= bbox[3]
# if a bounding box was specified do some sanity checks.
# restrict changesets to those enclosed by a bounding box
# we need to return both the changesets and the bounding box
def conditions_bbox(changesets, params)
if params[:bbox]
bbox = BoundingBox.from_bbox_params(params)
elsif params[:minlon] and params[:minlat] and params[:maxlon] and params[:maxlat]
bbox = BoundingBox.from_lon_lat_params(params)
end
if bbox
bbox.check_boundaries
bbox = bbox.to_scaled
return changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?",
(bbox[2] * GeoRecord::SCALE).to_i,
(bbox[0] * GeoRecord::SCALE).to_i,
(bbox[3] * GeoRecord::SCALE).to_i,
(bbox[1] * GeoRecord::SCALE).to_i)
bbox.max_lon.to_i, bbox.min_lon.to_i,
bbox.max_lat.to_i, bbox.min_lat.to_i),
bbox
else
return changesets
return changesets, bbox
end
end

View file

@ -8,7 +8,7 @@ class ExportController < ApplicationController
#When the user clicks 'Export' we redirect to a URL which generates the export download
def finish
bbox = BoundingBox.new(params[:minlon], params[:minlat], params[:maxlon], params[:maxlat])
bbox = BoundingBox.from_lon_lat_params(params)
format = params[:format]
if format == "osm"

View file

@ -49,15 +49,16 @@ class SiteController < ApplicationController
@zoom = params['zoom'].to_i
elsif params['bbox']
bbox = params['bbox'].split(",")
bbox = BoundingBox.from_bbox_params(params)
@lon = ( bbox[0].to_f + bbox[2].to_f ) / 2.0
@lat = ( bbox[1].to_f + bbox[3].to_f ) / 2.0
@lon = bbox.centre_lon
@lat = bbox.centre_lat
@zoom = 16
elsif params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat']
@lon = ( params['maxlon'].to_f + params['minlon'].to_f ) / 2.0
@lat = ( params['maxlat'].to_f + params['minlat'].to_f ) / 2.0
bbox = BoundingBox.from_lon_lat_params(params)
@lon = bbox.centre_lon
@lat = bbox.centre_lat
@zoom = 16
elsif params['gpx']