Don't compare changeset bbox coordinates when they may be nil
authorMatt Amos <zerebubuth@gmail.com>
Wed, 6 Jul 2011 00:45:14 +0000 (01:45 +0100)
committerTom Hughes <tom@compton.nu>
Wed, 6 Jul 2011 07:50:08 +0000 (08:50 +0100)
The code assumed that the changeset bounding box would only be
updated with a valid bounding box, but this isn't the case when
dealing with certain corner cases, including when removing
deleted members from relations.

app/models/changeset.rb

index d13974fd992e9e281b1a3c231b7fce5cad6bfe34..8f9a1e69b57f2adee2a7b3a728c9bf38fae2c22a 100644 (file)
@@ -115,16 +115,21 @@ class Changeset < ActiveRecord::Base
     # nils, just use the bounding box update to write over them.
     @bbox = bbox.zip(array).collect { |a, b| a.nil? ? b : a }
 
-    # 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]
-
-    # 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
+    # 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]
+      
+      # 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
   end
 
   ##