From: Peter Gray <pdgtips@gmail.com>
Date: Mon, 24 Oct 2011 09:45:56 +0000 (+0100)
Subject: Refactor bounding box code
X-Git-Tag: live~6018
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/95d899786a1bbabacc0cd12ef1c4814118d9d0de

Refactor bounding box code

Moved duplicated code into the BoundingBox class, and pass around
BoundingBox objects instead of lists of bounds.
---

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index b472ae05c..86e57a2d1 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -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
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index b9bb5fe59..53e14202c 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -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
diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index c45b211d4..f6d35b440 100644
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -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
 
diff --git a/app/controllers/export_controller.rb b/app/controllers/export_controller.rb
index 225ddf86b..cc3ed70c5 100644
--- a/app/controllers/export_controller.rb
+++ b/app/controllers/export_controller.rb
@@ -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"
diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb
index 21ef26bac..36f57d6c2 100644
--- a/app/controllers/site_controller.rb
+++ b/app/controllers/site_controller.rb
@@ -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']
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 0a0b6462b..b76d0c5a7 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -90,21 +90,11 @@ class Changeset < ActiveRecord::Base
   # returns the bounding box of the changeset. it is possible that some
   # or all of the values will be nil, indicating that they are undefined.
   def bbox
-    @bbox ||= [ min_lon, min_lat, max_lon, max_lat ]
+    @bbox ||= BoundingBox.new(min_lon, min_lat, max_lon, max_lat)
   end
 
   def has_valid_bbox?
-    not bbox.include? nil
-  end
-
-  ##
-  # returns area of the changset bbox as a rough comparitive quantity for use of changset displays
-  def area
-     if has_valid_bbox?
-       (max_lon - min_lon) * (max_lat - min_lat)
-     else
-       0
-     end
+    bbox.complete?
   end
 
   ##
@@ -112,26 +102,12 @@ class Changeset < ActiveRecord::Base
   # expand a little bit more in the direction of the expansion, so that
   # further expansions may be unnecessary. this is an optimisation
   # suggested on the wiki page by kleptog.
-  def update_bbox!(array)
-    # ensure that bbox is cached and has no nils in it. if there are any
-    # nils, just use the bounding box update to write over them.
-    @bbox = bbox.zip(array).collect { |a, b| a.nil? ? b : a }
-
-    # only try to update the bbox if there is a value for every coordinate
-    # which there will be from the previous line as long as both array and
-    # bbox are all non-nil. 
-    if has_valid_bbox? and array.all?
-      # FIXME - this looks nasty and violates DRY... is there any prettier
-      # way to do this?
-      @bbox[0] = [-180 * GeoRecord::SCALE, array[0] + EXPAND * (@bbox[0] - @bbox[2])].max if array[0] < @bbox[0]
-      @bbox[1] = [ -90 * GeoRecord::SCALE, array[1] + EXPAND * (@bbox[1] - @bbox[3])].max if array[1] < @bbox[1]
-      @bbox[2] = [ 180 * GeoRecord::SCALE, array[2] + EXPAND * (@bbox[2] - @bbox[0])].min if array[2] > @bbox[2]
-      @bbox[3] = [  90 * GeoRecord::SCALE, array[3] + EXPAND * (@bbox[3] - @bbox[1])].min if array[3] > @bbox[3]
+  def update_bbox!(bbox_update)
+    bbox.expand!(bbox_update, EXPAND)
       
-      # update active record. rails 2.1's dirty handling should take care of
-      # whether this object needs saving or not.
-      self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox
-    end
+    # update active record. rails 2.1's dirty handling should take care of
+    # whether this object needs saving or not.
+    self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox.to_a if bbox.complete?
   end
 
   ##
@@ -237,10 +213,9 @@ class Changeset < ActiveRecord::Base
     el1['closed_at'] = self.closed_at.xmlschema unless is_open?
     el1['open'] = is_open?.to_s
 
-    el1['min_lon'] = (bbox[0].to_f / GeoRecord::SCALE).to_s unless bbox[0].nil?
-    el1['min_lat'] = (bbox[1].to_f / GeoRecord::SCALE).to_s unless bbox[1].nil?
-    el1['max_lon'] = (bbox[2].to_f / GeoRecord::SCALE).to_s unless bbox[2].nil?
-    el1['max_lat'] = (bbox[3].to_f / GeoRecord::SCALE).to_s unless bbox[3].nil?
+    if bbox.complete?
+      bbox.to_unscaled.add_bounds_to(el1, '_')
+    end
 
     # NOTE: changesets don't include the XML of the changes within them,
     # they are just structures for tagging. to get the osmChange of a
diff --git a/app/models/node.rb b/app/models/node.rb
index 915188e47..0f176067d 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -44,7 +44,6 @@ class Node < ActiveRecord::Base
   # Also adheres to limitations such as within max_number_of_nodes
   #
   def self.search(bounding_box, tags = {})
-    min_lon, min_lat, max_lon, max_lat = *bounding_box
     # @fixme a bit of a hack to search for only visible nodes
     # couldn't think of another to add to tags condition
     #conditions_hash = tags.merge({ 'visible' => 1 })
@@ -59,9 +58,8 @@ class Node < ActiveRecord::Base
     #end 
     #conditions = keys.join(' AND ')
  
-    find_by_area(min_lat, min_lon, max_lat, max_lon,
-                    :conditions => {:visible => true},
-                    :limit => MAX_NUMBER_OF_NODES+1)  
+    find_by_area(bounding_box, :conditions => {:visible => true},
+                       :limit => MAX_NUMBER_OF_NODES+1)
   end
 
   # Read in xml as text and return it's Node object representation
@@ -121,7 +119,7 @@ class Node < ActiveRecord::Base
   # the bounding box around a node, which is used for determining the changeset's
   # bounding box
   def bbox
-    [ longitude, latitude, longitude, latitude ]
+    BoundingBox.new(longitude, latitude, longitude, latitude)
   end
 
   # Should probably be renamed delete_from to come in line with update
diff --git a/app/models/way.rb b/app/models/way.rb
index b1870fc89..85aca4334 100644
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -205,7 +205,7 @@ class Way < ActiveRecord::Base
   def bbox
     lons = nodes.collect { |n| n.longitude }
     lats = nodes.collect { |n| n.latitude }
-    [ lons.min, lats.min, lons.max, lats.max ]
+    BoundingBox.new(lons.min, lats.min, lons.max, lats.max)
   end
 
   def update_from(new_way, user)
diff --git a/app/views/browse/_changeset_details.html.erb b/app/views/browse/_changeset_details.html.erb
index 16f236ba7..2d7808137 100644
--- a/app/views/browse/_changeset_details.html.erb
+++ b/app/views/browse/_changeset_details.html.erb
@@ -24,23 +24,21 @@
     <% unless changeset_details.has_valid_bbox? %>
       <td><%= t 'browse.changeset_details.no_bounding_box' %></td>
     <% else
-          minlon = changeset_details.min_lon/GeoRecord::SCALE.to_f
-          minlat = changeset_details.min_lat/GeoRecord::SCALE.to_f
-          maxlon = changeset_details.max_lon/GeoRecord::SCALE.to_f
-          maxlat = changeset_details.max_lat/GeoRecord::SCALE.to_f
+          bbox = changeset_details.bbox.to_unscaled
     %>
       <td>
         <table>
           <tr>
-            <td colspan="3" style="text-align:center"><%=maxlat -%></td>
+            <td colspan="3" style="text-align:center"><%=bbox.max_lat -%></td>
           </tr>
           <tr>
-            <td><%=minlon -%></td>
-            <td>(<a href='/?minlon=<%= minlon %>&minlat=<%= minlat %>&maxlon=<%= maxlon %>&maxlat=<%= maxlat %>&box=yes' title='<%= t 'browse.changeset_details.show_area_box' %>'><%= t 'browse.changeset_details.box' %></a>)</td>
-            <td><%=maxlon -%></td>
+            <td><%=bbox.min_lon -%></td>
+            <td>(<a href='/?minlon=<%= bbox.min_lon %>&minlat=<%= bbox.min_lat %>&maxlon=<%= bbox.max_lon %>&maxlat=<%= bbox.max_lat %>
+                  &box=yes' title='<%= t 'browse.changeset_details.show_area_box' %>'><%= t 'browse.changeset_details.box' %></a>)</td>
+            <td><%=bbox.max_lon -%></td>
           </tr>
           <tr>
-            <td colspan="3" style="text-align:center"><%= minlat -%></td>
+            <td colspan="3" style="text-align:center"><%= bbox.min_lat -%></td>
           </tr>
         </table>
       </td>
diff --git a/app/views/browse/_map.html.erb b/app/views/browse/_map.html.erb
index 1ff86cd4e..4b2093e3f 100644
--- a/app/views/browse/_map.html.erb
+++ b/app/views/browse/_map.html.erb
@@ -72,10 +72,11 @@
       });
 
       <% if map.instance_of? Changeset %>
-        var minlon = <%= map.min_lon / GeoRecord::SCALE.to_f %>;
-        var minlat = <%= map.min_lat / GeoRecord::SCALE.to_f %>;
-        var maxlon = <%= map.max_lon / GeoRecord::SCALE.to_f %>;
-        var maxlat = <%= map.max_lat / GeoRecord::SCALE.to_f %>;
+        <% bbox = map.bbox.to_unscaled %>
+        var minlon = <%= bbox.min_lon %>;
+        var minlat = <%= bbox.min_lat %>;
+        var maxlon = <%= bbox.max_lon %>;
+        var maxlat = <%= bbox.max_lat %>;
         var bbox = new OpenLayers.Bounds(minlon, minlat, maxlon, maxlat);
         var centre = bbox.getCenterLonLat();
 
diff --git a/app/views/changeset/_map.html.erb b/app/views/changeset/_map.html.erb
index 69d32b2c2..34cf7002a 100644
--- a/app/views/changeset/_map.html.erb
+++ b/app/views/changeset/_map.html.erb
@@ -51,10 +51,11 @@
 
     <% @edits.each do |edit| %>
     <% if edit.has_valid_bbox? %>
-    var minlon = <%= edit.min_lon / GeoRecord::SCALE.to_f %>;
-    var minlat = <%= edit.min_lat / GeoRecord::SCALE.to_f %>;
-    var maxlon = <%= edit.max_lon / GeoRecord::SCALE.to_f %>;
-    var maxlat = <%= edit.max_lat / GeoRecord::SCALE.to_f %>;
+    <% bbox = edit.bbox.to_unscaled %>
+    var minlon = <%= bbox.min_lon %>;
+    var minlat = <%= bbox.min_lat %>;
+    var maxlon = <%= bbox.max_lon %>;
+    var maxlat = <%= bbox.max_lat %>;
     var bbox = new OpenLayers.Bounds(minlon, minlat, maxlon, maxlat);
 
     bounds.extend(bbox);
diff --git a/app/views/changeset/list.atom.builder b/app/views/changeset/list.atom.builder
index d9d53593c..9512ef827 100644
--- a/app/views/changeset/list.atom.builder
+++ b/app/views/changeset/list.atom.builder
@@ -78,15 +78,12 @@ atom_feed(:language => I18n.locale, :schema_date => 2009,
         end
       end
 
-      unless changeset.min_lat.nil?
-        minlon = changeset.min_lon/GeoRecord::SCALE.to_f
-        minlat = changeset.min_lat/GeoRecord::SCALE.to_f
-        maxlon = changeset.max_lon/GeoRecord::SCALE.to_f
-        maxlat = changeset.max_lat/GeoRecord::SCALE.to_f
+      if changeset.has_valid_bbox?
+        bbox = changeset.bbox.to_unscaled
 
         # See http://georss.org/Encodings#Geometry
-        lower_corner = "#{minlat} #{minlon}"
-        upper_corner = "#{maxlat} #{maxlon}"
+        lower_corner = "#{bbox.min_lat} #{bbox.min_lon}"
+        upper_corner = "#{bbox.max_lat} #{bbox.max_lon}"
 
         feed.georss :box, lower_corner + " " + upper_corner
       end
diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb
index 30bbebb8d..e560dbefa 100644
--- a/lib/bounding_box.rb
+++ b/lib/bounding_box.rb
@@ -1,58 +1,113 @@
 class BoundingBox
-  def initialize(min_lon, min_lat, max_lon, max_lat)
-    @bbox = [min_lon.to_f, min_lat.to_f, max_lon.to_f, max_lat.to_f]
-  end
-
-  def self.from_s(s)
-    BoundingBox.new(*s.split(/,/))
-  end
-
-  def min_lon
-    @bbox[0]
-  end
-
-  def min_lon=(min_lon)
-    @bbox[0] = min_lon
-  end
-
-  def min_lat
-    @bbox[1]
-  end
-
-  def min_lat=(min_lat)
-    @bbox[1] = min_lat
-  end
+  attr_reader :min_lon, :min_lat, :max_lon, :max_lat
 
-  def max_lon
-    @bbox[2]
-  end
-
-  def max_lon=(max_lon)
-    @bbox[2] = max_lon
-  end
+private
+  LON_LIMIT = 180.0
+  LAT_LIMIT = 90.0
+  SCALED_LON_LIMIT = LON_LIMIT *  GeoRecord::SCALE
+  SCALED_LAT_LIMIT = LAT_LIMIT *  GeoRecord::SCALE
 
-  def max_lat
-    @bbox[3]
+public
+  def initialize(min_lon, min_lat, max_lon, max_lat)
+    @min_lon = min_lon.to_f unless min_lon.nil?
+    @min_lat = min_lat.to_f unless min_lat.nil?
+    @max_lon = max_lon.to_f unless max_lon.nil?
+    @max_lat = max_lat.to_f unless max_lat.nil?
   end
 
-  def max_lat=(max_lat)
-    @bbox[3] = max_lat
+  def self.from_s(s)
+    if s.count(',') == 3
+      BoundingBox.new(*s.split(/,/))
+    else
+      nil
+    end
+  end
+
+  def self.from_bbox_params(params)
+    if params[:bbox] and params[:bbox].count(',') == 3
+      bbox_array = params[:bbox].split(',')
+    end
+    from_bbox_array(bbox_array)
+  end
+
+  def self.from_lon_lat_params(params)
+    if params[:minlon] and params[:minlat] and params[:maxlon] and params[:maxlat]
+      bbox_array = [params[:minlon], params[:minlat], params[:maxlon], params[:maxlat]]
+    end
+    from_bbox_array(bbox_array)
+  end
+
+  def expand!(bbox, margin = 0)
+    update!(bbox) unless complete?
+    # only try to expand the bbox if there is a value for every coordinate
+    # which there will be from the previous line as long as array does not contain a nil
+    if bbox.complete?
+      @min_lon = [-SCALED_LON_LIMIT,
+                  bbox.min_lon + margin * (min_lon - max_lon)].max if bbox.min_lon < min_lon
+      @min_lat = [-SCALED_LAT_LIMIT,
+                  bbox.min_lat + margin * (min_lat - max_lat)].max if bbox.min_lat < min_lat
+      @max_lon = [+SCALED_LON_LIMIT,
+                  bbox.max_lon + margin * (max_lon - min_lon)].min if bbox.max_lon > max_lon
+      @max_lat = [+SCALED_LAT_LIMIT,
+                  bbox.max_lat + margin * (max_lat - min_lat)].min if bbox.max_lat > max_lat
+    end
+    self
+  end
+
+  def check_boundaries
+    # check the bbox is sane
+    if min_lon > max_lon
+      raise OSM::APIBadBoundingBox.new(
+        "The minimum longitude must be less than the maximum longitude, but it wasn't")
+    end
+    if min_lat > max_lat
+      raise OSM::APIBadBoundingBox.new(
+        "The minimum latitude must be less than the maximum latitude, but it wasn't")
+    end
+    if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT
+      raise OSM::APIBadBoundingBox.new("The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," +
+                                       " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}")
+    end
+    self
+  end
+
+  def check_size
+    # check the bbox isn't too large
+    if area > MAX_REQUEST_AREA
+      raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + MAX_REQUEST_AREA.to_s +
+        ", and your request was too large. Either request a smaller area, or use planet.osm")
+    end
+    self
+  end
+
+  ##
+  # returns area of the bbox as a rough comparative quantity
+  def area
+    if complete?
+      (max_lon - min_lon) * (max_lat - min_lat)
+    else
+      0
+    end
+  end
+
+  def complete?
+    not to_a.include?(nil)
   end
 
   def centre_lon
-    (@bbox[0] + @bbox[2]) / 2.0
+    (min_lon + max_lon) / 2.0
   end
 
   def centre_lat
-    (@bbox[1] + @bbox[3]) / 2.0
+    (min_lat + max_lat) / 2.0
   end
 
   def width
-    @bbox[2] - @bbox[0]
+    max_lon - min_lon
   end
 
   def height
-    @bbox[3] - @bbox[1]
+    max_lat - min_lat
   end
 
   def slippy_width(zoom)
@@ -63,10 +118,64 @@ class BoundingBox
     min = min_lat * Math::PI / 180.0
     max = max_lat * Math::PI / 180.0
 
-    Math.log((Math.tan(max) + 1.0 / Math.cos(max)) / (Math.tan(min) + 1.0 / Math.cos(min))) * 128.0 * 2.0 ** zoom / Math::PI
+    Math.log((Math.tan(max) + 1.0 / Math.cos(max)) /
+             (Math.tan(min) + 1.0 / Math.cos(min))) *
+             (128.0 * 2.0 ** zoom / Math::PI)
+  end
+
+  # there are two forms used for bounds with and without an underscore,
+  # cater for both forms eg minlon and min_lon
+  def add_bounds_to(hash, underscore = "")
+    hash["min#{underscore}lat"] = min_lat.to_s
+    hash["min#{underscore}lon"] = min_lon.to_s
+    hash["max#{underscore}lat"] = max_lat.to_s
+    hash["max#{underscore}lon"] = max_lon.to_s
+    hash
+  end
+
+  def to_scaled
+    BoundingBox.new((min_lon * GeoRecord::SCALE),
+                    (min_lat * GeoRecord::SCALE),
+                    (max_lon * GeoRecord::SCALE),
+                    (max_lat * GeoRecord::SCALE))
+  end
+
+  def to_unscaled
+    BoundingBox.new((min_lon / GeoRecord::SCALE),
+                    (min_lat / GeoRecord::SCALE),
+                    (max_lon / GeoRecord::SCALE),
+                    (max_lat / GeoRecord::SCALE))
+  end
+
+  def to_a
+    [min_lon, min_lat, max_lon, max_lat]
   end
 
   def to_s
-    return @bbox.join(",")
+    "#{min_lon},#{min_lat},#{max_lon},#{max_lat}"
+  end
+
+  private
+  def self.from_bbox_array(bbox_array)
+    unless bbox_array
+      raise OSM::APIBadUserInput.new(
+        "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat")
+    end
+    # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and
+    # max_lat within their respective boundaries.
+    min_lon = [[bbox_array[0].to_f, -LON_LIMIT].max, +LON_LIMIT].min
+    min_lat = [[bbox_array[1].to_f, -LAT_LIMIT].max, +LAT_LIMIT].min
+    max_lon = [[bbox_array[2].to_f, +LON_LIMIT].min, -LON_LIMIT].max
+    max_lat = [[bbox_array[3].to_f, +LAT_LIMIT].min, -LAT_LIMIT].max
+    BoundingBox.new(min_lon, min_lat, max_lon, max_lat)
+  end
+
+  def update!(bbox)
+    # ensure that bbox has no nils in it. if there are any
+    # nils, just use the bounding box update to write over them.
+    @min_lon = bbox.min_lon if min_lon.nil?
+    @min_lat = bbox.min_lat if min_lat.nil?
+    @max_lon = bbox.max_lon if max_lon.nil?
+    @max_lat = bbox.max_lat if max_lat.nil?
   end
 end
diff --git a/lib/geo_record.rb b/lib/geo_record.rb
index d338f1b66..0d010eb86 100644
--- a/lib/geo_record.rb
+++ b/lib/geo_record.rb
@@ -5,7 +5,7 @@ module GeoRecord
   SCALE = 10000000
   
   def self.included(base)
-    base.scope :bbox, lambda { |minlat,minlon,maxlat,maxlon| base.where(OSM.sql_for_area(minlat, minlon, maxlat, maxlon)) }
+    base.scope :bbox, lambda { |bbox| base.where(OSM.sql_for_area(bbox)) }
     base.before_save :update_tile
   end
 
diff --git a/lib/map_boundary.rb b/lib/map_boundary.rb
deleted file mode 100644
index f3accf2da..000000000
--- a/lib/map_boundary.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-module MapBoundary
-  # Take an array of length 4, and return the min_lon, min_lat, max_lon and 
-  # max_lat within their respective boundaries.
-  def sanitise_boundaries(bbox)
-    min_lon = [[bbox[0].to_f,-180].max,180].min
-    min_lat = [[bbox[1].to_f,-90].max,90].min
-    max_lon = [[bbox[2].to_f,+180].min,-180].max
-    max_lat = [[bbox[3].to_f,+90].min,-90].max
-    return min_lon, min_lat, max_lon, max_lat
-  end
-
-  def check_boundaries(min_lon, min_lat, max_lon, max_lat)
-    # 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")
-    end
-    unless min_lat <= max_lat
-      raise OSM::APIBadBoundingBox.new("The minimum latitude must be less than the maximum latitude, but it wasn't")
-    end
-    unless min_lon >= -180 && min_lat >= -90 && max_lon <= 180 && max_lat <= 90
-      # Due to sanitize_boundaries, it is highly unlikely we'll actually get here
-      raise OSM::APIBadBoundingBox.new("The latitudes must be between -90 and 90, and longitudes between -180 and 180")
-    end
-
-    # 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 + 
-        ", and your request was too large. Either request a smaller area, or use planet.osm")
-    end
-  end
-end
diff --git a/lib/osm.rb b/lib/osm.rb
index 8416aab5a..02c51df78 100644
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -497,14 +497,12 @@ module OSM
   end
 
   # 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
-
-    return "#{tilesql} AND #{prefix}latitude BETWEEN #{minlat} AND #{maxlat} AND #{prefix}longitude BETWEEN #{minlon} AND #{maxlon}"
+  def self.sql_for_area(bbox, prefix = nil)
+    tilesql = QuadTile.sql_for_area(bbox, prefix)
+    bbox = bbox.to_scaled
+
+    return "#{tilesql} AND #{prefix}latitude BETWEEN #{bbox.min_lat} AND #{bbox.max_lat} " +
+                      "AND #{prefix}longitude BETWEEN #{bbox.min_lon} AND #{bbox.max_lon}"
   end
 
   # Return a spam score for a chunk of text
diff --git a/lib/quad_tile.rb b/lib/quad_tile.rb
index 70012597b..258fb9fd5 100644
--- a/lib/quad_tile.rb
+++ b/lib/quad_tile.rb
@@ -9,11 +9,11 @@ module QuadTile
       return tile_for_xy(x, y)
     end
 
-    def self.tiles_for_area(minlat, minlon, maxlat, maxlon)
-      minx = ((minlon + 180) * 65535 / 360).round
-      maxx = ((maxlon + 180) * 65535 / 360).round
-      miny = ((minlat + 90) * 65535 / 180).round
-      maxy = ((maxlat + 90) * 65535 / 180).round
+    def self.tiles_for_area(bbox)
+      minx = ((bbox.min_lon + 180) * 65535 / 360).round
+      maxx = ((bbox.max_lon + 180) * 65535 / 360).round
+      miny = ((bbox.min_lat + 90) * 65535 / 180).round
+      maxy = ((bbox.max_lat + 90) * 65535 / 180).round
       tiles = []
 
       minx.upto(maxx) do |x|
@@ -40,8 +40,8 @@ module QuadTile
       return t
     end
 
-    def self.iterate_tiles_for_area(minlat, minlon, maxlat, maxlon)
-      tiles = tiles_for_area(minlat, minlon, maxlat, maxlon)
+    def self.iterate_tiles_for_area(bbox)
+      tiles = tiles_for_area(bbox)
       first = last = nil
 
       tiles.sort.each do |tile|
@@ -60,11 +60,11 @@ module QuadTile
     end
   end
 
-  def self.sql_for_area(minlat, minlon, maxlat, maxlon, prefix)
+  def self.sql_for_area(bbox, prefix)
     sql = Array.new
     single = Array.new
 
-    iterate_tiles_for_area(minlat, minlon, maxlat, maxlon) do |first,last|
+    iterate_tiles_for_area(bbox) do |first,last|
       if first == last
         single.push(first)
       else
diff --git a/lib/quad_tile/quad_tile.c b/lib/quad_tile/quad_tile.c
index 089154597..cd45e6e76 100644
--- a/lib/quad_tile/quad_tile.c
+++ b/lib/quad_tile/quad_tile.c
@@ -63,12 +63,12 @@ static VALUE tile_for_point(VALUE self, VALUE lat, VALUE lon)
    return UINT2NUM(xy2tile(x, y));
 }
 
-static VALUE tiles_for_area(VALUE self, VALUE minlat, VALUE minlon, VALUE maxlat, VALUE maxlon)
+static VALUE tiles_for_area(VALUE self, VALUE bbox)
 {
-   unsigned int minx = lon2x(NUM2DBL(minlon));
-   unsigned int maxx = lon2x(NUM2DBL(maxlon));
-   unsigned int miny = lat2y(NUM2DBL(minlat));
-   unsigned int maxy = lat2y(NUM2DBL(maxlat));
+   unsigned int minx = lon2x(NUM2DBL(rb_iv_get(bbox, "@min_lon")));
+   unsigned int maxx = lon2x(NUM2DBL(rb_iv_get(bbox, "@max_lon")));
+   unsigned int miny = lat2y(NUM2DBL(rb_iv_get(bbox, "@min_lat")));
+   unsigned int maxy = lat2y(NUM2DBL(rb_iv_get(bbox, "@max_lat")));
    tilelist_t   tl = tilelist_for_area(minx, miny, maxx, maxy);
    VALUE        tiles = rb_ary_new();
    unsigned int t;
@@ -88,12 +88,12 @@ static VALUE tile_for_xy(VALUE self, VALUE x, VALUE y)
    return UINT2NUM(xy2tile(NUM2UINT(x), NUM2UINT(y)));
 }
 
-static VALUE iterate_tiles_for_area(VALUE self, VALUE minlat, VALUE minlon, VALUE maxlat, VALUE maxlon)
+static VALUE iterate_tiles_for_area(VALUE self, VALUE bbox)
 {
-   unsigned int minx = lon2x(NUM2DBL(minlon));
-   unsigned int maxx = lon2x(NUM2DBL(maxlon));
-   unsigned int miny = lat2y(NUM2DBL(minlat));
-   unsigned int maxy = lat2y(NUM2DBL(maxlat));
+   unsigned int minx = lon2x(NUM2DBL(rb_iv_get(bbox, "@min_lon")));
+   unsigned int maxx = lon2x(NUM2DBL(rb_iv_get(bbox, "@max_lon")));
+   unsigned int miny = lat2y(NUM2DBL(rb_iv_get(bbox, "@min_lat")));
+   unsigned int maxy = lat2y(NUM2DBL(rb_iv_get(bbox, "@max_lat")));
    tilelist_t   tl = tilelist_for_area(minx, miny, maxx, maxy);
 
    if (tl.tilec > 0)
@@ -140,9 +140,9 @@ void Init_quad_tile_so(void)
    VALUE m = rb_define_module("QuadTile");
 
    rb_define_module_function(m, "tile_for_point", tile_for_point, 2);
-   rb_define_module_function(m, "tiles_for_area", tiles_for_area, 4);
+   rb_define_module_function(m, "tiles_for_area", tiles_for_area, 1);
    rb_define_module_function(m, "tile_for_xy", tile_for_xy, 2);
-   rb_define_module_function(m, "iterate_tiles_for_area", iterate_tiles_for_area, 4);
+   rb_define_module_function(m, "iterate_tiles_for_area", iterate_tiles_for_area, 1);
 
    return;
 }
diff --git a/test/functional/relation_controller_test.rb b/test/functional/relation_controller_test.rb
index 19b3617fc..8a59adb02 100644
--- a/test/functional/relation_controller_test.rb
+++ b/test/functional/relation_controller_test.rb
@@ -506,7 +506,7 @@ class RelationControllerTest < ActionController::TestCase
   def test_tag_modify_bounding_box
     # in current fixtures, relation 5 contains nodes 3 and 5 (node 3
     # indirectly via way 3), so the bbox should be [3,3,5,5].
-    check_changeset_modify([3,3,5,5]) do |changeset_id|
+    check_changeset_modify(BoundingBox.new(3,3,5,5)) do |changeset_id|
       # add a tag to an existing relation
       relation_xml = current_relations(:visible_relation).to_xml
       relation_element = relation_xml.find("//osm/relation").first
@@ -536,7 +536,7 @@ class RelationControllerTest < ActionController::TestCase
      current_ways(:used_way),
      current_ways(:way_with_versions)
     ].each_with_index do |element, version|
-      bbox = element.bbox.collect { |x| x / SCALE }
+      bbox = element.bbox.to_unscaled
       check_changeset_modify(bbox) do |changeset_id|
         relation_xml = Relation.find(relation_id).to_xml
         relation_element = relation_xml.find("//osm/relation").first
@@ -566,7 +566,7 @@ class RelationControllerTest < ActionController::TestCase
   # remove a member from a relation and check the bounding box is 
   # only that element.
   def test_remove_member_bounding_box
-    check_changeset_modify([5,5,5,5]) do |changeset_id|
+    check_changeset_modify(BoundingBox.new(5,5,5,5)) do |changeset_id|
       # remove node 5 (5,5) from an existing relation
       relation_xml = current_relations(:visible_relation).to_xml
       relation_xml.
@@ -769,10 +769,10 @@ OSM
       assert_response :success, "can't re-read changeset for modify test"
       assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}"
       assert_select "osm>changeset[id=#{changeset_id}]", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}"
-      assert_select "osm>changeset[min_lon=#{bbox[0].to_f}]", 1, "Changeset min_lon wrong in #{@response.body}"
-      assert_select "osm>changeset[min_lat=#{bbox[1].to_f}]", 1, "Changeset min_lat wrong in #{@response.body}"
-      assert_select "osm>changeset[max_lon=#{bbox[2].to_f}]", 1, "Changeset max_lon wrong in #{@response.body}"
-      assert_select "osm>changeset[max_lat=#{bbox[3].to_f}]", 1, "Changeset max_lat wrong in #{@response.body}"
+      assert_select "osm>changeset[min_lon=#{bbox.min_lon}]", 1, "Changeset min_lon wrong in #{@response.body}"
+      assert_select "osm>changeset[min_lat=#{bbox.min_lat}]", 1, "Changeset min_lat wrong in #{@response.body}"
+      assert_select "osm>changeset[max_lon=#{bbox.max_lon}]", 1, "Changeset max_lon wrong in #{@response.body}"
+      assert_select "osm>changeset[max_lat=#{bbox.max_lat}]", 1, "Changeset max_lat wrong in #{@response.body}"
     end
   end
 
diff --git a/test/unit/bounding_box_test.rb b/test/unit/bounding_box_test.rb
new file mode 100644
index 000000000..1a1059ddf
--- /dev/null
+++ b/test/unit/bounding_box_test.rb
@@ -0,0 +1,316 @@
+require File.dirname(__FILE__) + '/../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"
+    @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"
+    @nil_error_message = "Should not contain nil"
+
+    @bbox_from_nils = BoundingBox.new(nil, nil, nil, nil)
+    @bbox_expand = BoundingBox.new(0,0,0,0)
+    @bbox_expand_ten = BoundingBox.new(10,10,10,10)
+    @bbox_expand_minus_two = BoundingBox.new(-2,-2,-2,-2)
+    @bbox_from_string = BoundingBox.from_s("1,2,3,4")
+    @min_lon = 1.0
+    @min_lat = 2.0
+    @max_lon = 3.0
+    @max_lat = 4.0
+
+    @bad_positive_boundary_bbox  = %w{ 181,91,0,0 0,0,181,91 }
+    @bad_negative_boundary_bbox  = %w{ -181,-91,0,0 0,0,-181,-91 }
+    @bad_big_bbox       = %w{ -0.1,-0.1,1.1,1.1  10,10,11,11 }
+    @bad_malformed_bbox = %w{ -0.1  hello 10N2W10.1N2.1W }
+    @bad_lat_mixed_bbox  = %w{ 0,0.1,0.1,0  -0.1,80,0.1,70  0.24,54.34,0.25,54.33 }
+    @bad_lon_mixed_bbox  = %w{ 80,-0.1,70,0.1  54.34,0.24,54.33,0.25 }
+    @bad_limit_bbox = %w{ -180.1,-90,180,90 -180,-90.1,180,90 -180,-90,180.1,90 -180,-90,180,90.1}
+    @good_bbox         = %w{ -0.1,-0.1,0.1,0.1  51.1,-0.1,51.2,0 -0.1,%20-0.1,%200.1,%200.1
+                            -0.1edcd,-0.1d,0.1,0.1  -0.1E,-0.1E,0.1S,0.1N S0.1,W0.1,N0.1,E0.1}
+
+    @expand_min_lon_array = %w{ 2,10,10,10 1,10,10,10 0,10,10,10 -1,10,10,10 -2,10,10,10 -8,10,10,10 }
+    @expand_min_lat_array = %w{ 10,2,10,10 10,1,10,10 10,0,10,10 10,-1,10,10 10,-2,10,10 10,-8,10,10 }
+    @expand_max_lon_array = %w{ -2,-2,-1,-2 -2,-2,0,-2 -2,-2,1,-2 -2,-2,2,-2 }
+    @expand_max_lat_array = %w{ -2,-2,-2,-1 -2,-2,-2,0 -2,-2,-2,1 -2,-2,-2,2 }
+    @expand_min_lon_margin_response = [[2,10,10,10], [-7,10,10,10], [-7,10,10,10], [-7,10,10,10], [-7,10,10,10], [-25,10,10,10]]
+    @expand_min_lat_margin_response = [[10,2,10,10], [10,-7,10,10], [10,-7,10,10], [10,-7,10,10], [10,-7,10,10], [10,-25,10,10]]
+    @expand_max_lon_margin_response = [[-2,-2,-1,-2], [-2,-2,1,-2], [-2,-2,1,-2], [-2,-2,5,-2]]
+    @expand_max_lat_margin_response = [[-2,-2,-2,-1], [-2,-2,-2,1], [-2,-2,-2,1], [-2,-2,-2,5]]
+  end
+
+  def test_good_bbox_from_string
+    @good_bbox.each do |string|
+      bbox = BoundingBox.from_s(string)
+      array = string.split(',').collect {|s| s.to_f}
+      check_bbox(bbox, array)
+    end
+  end
+
+  def test_bbox_from_s_malformed
+    @bad_malformed_bbox.each do |bbox_string|
+      bbox = BoundingBox.from_s(bbox_string)
+      assert_nil bbox
+    end
+  end
+
+  def test_good_bbox_from_params
+    @good_bbox.each do |string|
+      bbox = BoundingBox.from_bbox_params({:bbox => string})
+      array = string.split(',').collect {|s| s.to_f}
+      check_bbox(bbox, array)
+    end
+  end
+
+  def test_good_bbox_from_lon_lat_params
+    @good_bbox.each do |string|
+      array = string.split(',')
+      bbox = BoundingBox.from_lon_lat_params({:minlon => array[0], :minlat => array[1], :maxlon => array[2], :maxlat => array[3]})
+      check_bbox(bbox, array.collect {|s| s.to_f})
+    end
+  end
+
+  def test_bbox_from_params_malformed
+    @bad_malformed_bbox.each do |bbox_string|
+      exception = assert_raise(OSM::APIBadUserInput) {BoundingBox.from_bbox_params({:bbox => bbox_string})}
+      assert_equal(@malformed_error_message, exception.message)
+    end
+  end
+
+  def test_good_bbox_from_new
+    @good_bbox.each do |string|
+      array = string.split(',')
+      bbox = BoundingBox.new(array[0], array[1], array[2], array[3])
+      check_bbox(bbox, array.collect {|s| s.to_f})
+    end
+  end
+
+  def test_creation_from_new_with_nils
+    check_bbox(@bbox_from_nils, [nil, nil, nil, nil])
+  end
+
+  def test_expand_min_lon_boundary
+    @bbox_expand.expand!(BoundingBox.new(-1810000000,0,0,0))
+    check_expand(@bbox_expand, "-1800000000,0,0,0")
+  end
+
+  def test_expand_min_lat_boundary
+    @bbox_expand.expand!(BoundingBox.new(0,-910000000,0,0))
+    check_expand(@bbox_expand, "0,-900000000,0,0")
+  end
+
+  def test_expand_max_lon_boundary
+    @bbox_expand.expand!(BoundingBox.new(0,0,1810000000,0))
+    check_expand(@bbox_expand, "0,0,1800000000,0")
+  end
+
+  def test_expand_max_lat_boundary
+    @bbox_expand.expand!(BoundingBox.new(0,0,0,910000000))
+    check_expand(@bbox_expand, "0,0,0,900000000")
+  end
+
+  def test_expand_min_lon_without_margin
+    @expand_min_lon_array.each {|array_string| check_expand(@bbox_expand_ten, array_string)}
+  end
+
+  def test_expand_min_lon_with_margin
+    @expand_min_lon_array.each_with_index do |array_string, index|
+      check_expand(@bbox_expand_ten, array_string, 1, @expand_min_lon_margin_response[index])
+    end
+  end
+
+  def test_expand_min_lat_without_margin
+    @expand_min_lat_array.each {|array_string| check_expand(@bbox_expand_ten, array_string)}
+  end
+
+  def test_expand_min_lat_with_margin
+    @expand_min_lat_array.each_with_index do |array_string, index|
+      check_expand(@bbox_expand_ten, array_string, 1, @expand_min_lat_margin_response[index])
+    end
+  end
+
+  def test_expand_max_lon_without_margin
+    @expand_max_lon_array.each {|array_string| check_expand(@bbox_expand_minus_two, array_string)}
+  end
+
+  def test_expand_max_lon_with_margin
+    @expand_max_lon_array.each_with_index do |array_string, index|
+      check_expand(@bbox_expand_minus_two, array_string, 1, @expand_max_lon_margin_response[index])
+    end
+  end
+
+  def test_expand_max_lat_without_margin
+    @expand_max_lat_array.each {|array_string| check_expand(@bbox_expand_minus_two, array_string)}
+  end
+
+  def test_expand_max_lat_with_margin
+    bbox = @bbox_expand
+    @expand_max_lat_array.each_with_index do |array_string, index|
+      check_expand(@bbox_expand_minus_two, array_string, 1, @expand_max_lat_margin_response[index])
+    end
+  end
+
+  def test_good_bbox_boundaries
+    @good_bbox.each do |bbox_string|
+      assert_nothing_raised(OSM::APIBadBoundingBox) {BoundingBox.from_s(bbox_string).check_boundaries}
+    end
+  end
+
+  def test_from_params_with_positive_out_of_boundary
+    @bad_positive_boundary_bbox.each do |bbox_string|
+      bbox = BoundingBox.from_bbox_params({:bbox => bbox_string})
+      array = bbox.to_a
+      assert_equal 180, [array[0], array[2]].max
+      assert_equal 90, [array[1], array[3]].max
+    end
+  end
+
+  def test_from_params_with_negative_out_of_boundary
+    @bad_negative_boundary_bbox.each do |bbox_string|
+      bbox = BoundingBox.from_bbox_params({:bbox => bbox_string})
+      array = bbox.to_a
+      assert_equal -180, [array[0], array[2]].min
+      assert_equal -90, [array[1], array[3]].min
+    end
+  end
+
+  def test_boundaries_mixed_lon
+    @bad_lon_mixed_bbox.each do |bbox_string|
+      exception = assert_raise(OSM::APIBadBoundingBox) {BoundingBox.from_s(bbox_string).check_boundaries}
+      assert_equal(@lon_order_error_message, exception.message)
+    end
+  end
+
+  def test_boundaries_mixed_lat
+    @bad_lat_mixed_bbox.each do |bbox_string|
+      exception = assert_raise(OSM::APIBadBoundingBox) {BoundingBox.from_s(bbox_string).check_boundaries}
+      assert_equal(@lat_order_error_message, exception.message)
+    end
+  end
+
+  def test_boundaries_out_of_limits
+    @bad_limit_bbox.each do |bbox_string|
+      exception = assert_raise(OSM::APIBadBoundingBox) {BoundingBox.from_s(bbox_string).check_boundaries}
+      assert_equal(@bbox_out_of_limits_error_message, exception.message)
+    end
+  end
+
+  def test_good_bbox_size
+    @good_bbox.each do |bbox_string|
+      assert_nothing_raised(OSM::APIBadBoundingBox) {BoundingBox.from_s(bbox_string).check_size}
+    end
+  end
+
+  def test_size_to_big
+    @bad_big_bbox.each do |bbox_string|
+      bbox = nil
+      assert_nothing_raised(OSM::APIBadBoundingBox) {bbox = BoundingBox.from_bbox_params({:bbox => bbox_string}).check_boundaries}
+      exception = assert_raise(OSM::APIBadBoundingBox) {bbox.check_size}
+      assert_equal(@size_error_message, exception.message)
+    end
+  end
+
+  def test_bbox_area
+    @good_bbox.each do |string|
+      bbox = BoundingBox.from_s(string)
+      array = string.split(',')
+      assert_equal ((array[2].to_f - array[0].to_f) * (array[3].to_f - array[1].to_f)), bbox.area
+    end
+  end
+
+  def test_complete
+    assert !@bbox_from_nils.complete?, "should contain a nil"
+    assert @bbox_from_string.complete?, "should not contain a nil"
+  end
+
+  def test_centre_lon
+    @good_bbox.each do |bbox_string|
+      array = bbox_string.split(',')
+      assert_equal ((array[0].to_f + array[2].to_f) / 2.0), BoundingBox.from_s(bbox_string).centre_lon
+    end
+  end
+
+  def test_centre_lat
+    @good_bbox.each do |bbox_string|
+      array = bbox_string.split(',')
+      assert_equal ((array[1].to_f + array[3].to_f) / 2.0), BoundingBox.from_s(bbox_string).centre_lat
+    end
+  end
+
+  def test_width
+    @good_bbox.each do |bbox_string|
+      array = bbox_string.split(',')
+      assert_equal (array[2].to_f - array[0].to_f), BoundingBox.from_s(bbox_string).width
+    end
+  end
+
+  def test_height
+    @good_bbox.each do |bbox_string|
+      array = bbox_string.split(',')
+      assert_equal (array[3].to_f - array[1].to_f), BoundingBox.from_s(bbox_string).height
+    end
+  end
+
+  def test_slippy_width
+    assert_in_delta 5.68888888888889, @bbox_from_string.slippy_width(2), 0.000000000000001
+  end
+
+  def test_slippy_height
+    assert_in_delta 5.69698684268433, @bbox_from_string.slippy_height(2), 0.000000000000001
+  end
+
+  def test_add_bounds_to_no_underscore
+    bounds = @bbox_from_string.add_bounds_to({})
+    assert_equal 4, bounds.size
+    assert_equal @min_lon.to_s, bounds['minlon']
+    assert_equal @min_lat.to_s, bounds['minlat']
+    assert_equal @max_lon.to_s, bounds['maxlon']
+    assert_equal @max_lat.to_s, bounds['maxlat']
+  end
+
+  def test_add_bounds_to_with_underscore
+    bounds = @bbox_from_string.add_bounds_to({}, '_')
+    assert_equal 4, bounds.size
+    assert_equal @min_lon.to_s, bounds['min_lon']
+    assert_equal @min_lat.to_s, bounds['min_lat']
+    assert_equal @max_lon.to_s, bounds['max_lon']
+    assert_equal @max_lat.to_s, bounds['max_lat']
+  end
+
+  def test_to_scaled
+    bbox = @bbox_from_string.to_scaled
+    assert_equal @min_lon * GeoRecord::SCALE, bbox.min_lon
+    assert_equal @min_lat * GeoRecord::SCALE, bbox.min_lat
+    assert_equal @max_lon * GeoRecord::SCALE, bbox.max_lon
+    assert_equal @max_lat * GeoRecord::SCALE, bbox.max_lat
+  end
+
+  def test_to_unscaled
+    scale = GeoRecord::SCALE
+    bbox = BoundingBox.new(1.0 * scale, 2.0 * scale, 3.0 * scale, 4.0 * scale).to_unscaled
+    check_bbox(bbox, [@min_lon, @min_lat, @max_lon, @max_lat])
+  end
+
+  def test_to_a
+    assert_equal [1.0,2.0,3.0,4.0], @bbox_from_string.to_a
+  end
+
+  def test_to_string
+    assert_equal "#{@min_lon},#{@min_lat},#{@max_lon},#{@max_lat}", @bbox_from_string.to_s
+  end
+
+  private
+  def check_expand(bbox, array_string, margin = 0, result = nil)
+    array = array_string.split(',').collect {|s| s.to_f}
+    result = array unless result
+    bbox.expand!(BoundingBox.new(array[0], array[1], array[2], array[3]), margin)
+    check_bbox(bbox, result)
+  end
+
+  def check_bbox(bbox, result)
+    assert_equal result[0], bbox.min_lon, 'min_lon'
+    assert_equal result[1], bbox.min_lat, 'min_lat'
+    assert_equal result[2], bbox.max_lon, 'max_lon'
+    assert_equal result[3], bbox.max_lat, 'max_lat'
+  end
+end
diff --git a/test/unit/way_test.rb b/test/unit/way_test.rb
index 9c9ffe59a..ca5b75176 100644
--- a/test/unit/way_test.rb
+++ b/test/unit/way_test.rb
@@ -15,7 +15,10 @@ class WayTest < ActiveSupport::TestCase
       :invisible_way,
       :used_way ].each do |way_symbol|
       way = current_ways(way_symbol)
-      assert_equal node.bbox, way.bbox
+      assert_equal node.bbox.min_lon, way.bbox.min_lon, 'min_lon'
+      assert_equal node.bbox.min_lat, way.bbox.min_lat, 'min_lat'
+      assert_equal node.bbox.max_lon, way.bbox.max_lon, 'max_lon'
+      assert_equal node.bbox.max_lat, way.bbox.max_lat, 'max_lat'
     end
   end