X-Git-Url: https://git.openstreetmap.org/rails.git/blobdiff_plain/ef40b61ff40881966ae4280cfdcb9f92965e9d10..0f5646d7f0e0ccaf3e7b5fd36c9ac4047d5df210:/app/models/changeset.rb

diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 5c961a064..8e51cead2 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -56,26 +56,38 @@ class Changeset < ActiveRecord::Base
       p = XML::Parser.string(xml)
       doc = p.parse
 
-      cs = Changeset.new
-
       doc.find('//osm/changeset').each do |pt|
-        if create
-          cs.created_at = Time.now.getutc
-          # initial close time is 1h ahead, but will be increased on each
-          # modification.
-          cs.closed_at = cs.created_at + IDLE_TIMEOUT
-          # initially we have no changes in a changeset
-          cs.num_changes = 0
-        end
-
-        pt.find('tag').each do |tag|
-          cs.add_tag_keyval(tag['k'], tag['v'])
-        end
+        return Changeset.from_xml_node(pt, create)
       end
-    rescue Exception => ex
-      cs = nil
+      raise OSM::APIBadXMLError.new("changeset", xml, "XML doesn't contain an osm/changeset element.")
+    rescue LibXML::XML::Error, ArgumentError => ex
+      raise OSM::APIBadXMLError.new("changeset", xml, ex.message)
+    end
+  end
+  
+  def self.from_xml_node(pt, create=false)
+    cs = Changeset.new
+    if create
+      cs.created_at = Time.now.getutc
+      # initial close time is 1h ahead, but will be increased on each
+      # modification.
+      cs.closed_at = cs.created_at + IDLE_TIMEOUT
+      # initially we have no changes in a changeset
+      cs.num_changes = 0
+    else
+      raise OSM::APIBadXMLError.new("changeset", pt, "ID is required when updating.") if pt['id'].nil?
+      cs.id = pt['id'].to_i
+      # .to_i will return 0 if there is no number that can be parsed.
+      # We want to make sure that there is no id with zero anyway.
+      raise OSM::APIBadUserInput.new("ID of changeset cannot be zero when updating.") if cs.id == 0
     end
 
+    pt.find('tag').each do |tag|
+      raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag['k'].nil?
+      raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag['v'].nil?
+      cs.add_tag_keyval(tag['k'], tag['v'])
+    end
+    
     return cs
   end
 
@@ -112,10 +124,10 @@ class Changeset < ActiveRecord::Base
 
     # FIXME - this looks nasty and violates DRY... is there any prettier 
     # way to do this? 
-    @bbox[0] = array[0] + EXPAND * (@bbox[0] - @bbox[2]) if array[0] < @bbox[0]
-    @bbox[1] = array[1] + EXPAND * (@bbox[1] - @bbox[3]) if array[1] < @bbox[1]
-    @bbox[2] = array[2] + EXPAND * (@bbox[2] - @bbox[0]) if array[2] > @bbox[2]
-    @bbox[3] = array[3] + EXPAND * (@bbox[3] - @bbox[1]) if array[3] > @bbox[3]
+    @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.
@@ -150,6 +162,11 @@ class Changeset < ActiveRecord::Base
 
   def add_tag_keyval(k, v)
     @tags = Hash.new unless @tags
+
+    # duplicate tags are now forbidden, so we can't allow values
+    # in the hash to be overwritten.
+    raise OSM::APIDuplicateTagsError.new("changeset", self.id, k) if @tags.include? k
+
     @tags[k] = v
   end