Merge 12304:14009 from trunk.
authorTom Hughes <tom@compton.nu>
Sun, 8 Mar 2009 13:02:37 +0000 (13:02 +0000)
committerTom Hughes <tom@compton.nu>
Sun, 8 Mar 2009 13:02:37 +0000 (13:02 +0000)
26 files changed:
1  2 
app/controllers/api_controller.rb
app/controllers/user_controller.rb
app/controllers/user_preference_controller.rb
app/models/changeset.rb
app/models/node.rb
app/models/relation.rb
app/models/way.rb
app/views/diary_entry/list.rhtml
app/views/layouts/site.rhtml
app/views/user/login.rhtml
app/views/user/new.rhtml
config/environment.rb
config/initializers/libxml.rb
db/migrate/019_add_timestamp_indexes.rb
db/migrate/020_populate_node_tags_and_remove.rb
db/migrate/020_populate_node_tags_and_remove_helper.c
db/migrate/021_move_to_innodb.rb
db/migrate/022_key_constraints.rb
db/migrate/023_add_changesets.rb
db/migrate/024_order_relation_members.rb
db/migrate/025_add_end_time_to_changesets.rb
lib/diff_reader.rb
lib/osm.rb
public/stylesheets/site.css
test/functional/node_controller_test.rb
test/functional/relation_controller_test.rb

Simple merge
Simple merge
index 3b56c257be32b822864b08f984c959d8f549fc27,3a48ee65e85667222c1e2994f93efa18bd16363c..68ea88eeac5922def89f7e7e947f73c68332e072
@@@ -52,43 -52,48 +52,42 @@@ class UserPreferenceController < Applic
  
    # update the entire set of preferences
    def update
-     p = XML::Parser.new
-     p.string = request.raw_post
 -    begin
 -      p = XML::Parser.string(request.raw_post)
 -      doc = p.parse
 -
 -      prefs = []
 -
 -      keyhash = {}
++    p = XML::Parser.string(request.raw_post)
 +    doc = p.parse
  
 -      doc.find('//preferences/preference').each do |pt|
 -        pref = UserPreference.new
 +    prefs = []
  
 -        unless keyhash[pt['k']].nil? # already have that key
 -          render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable
 -          return
 -        end
 +    keyhash = {}
  
 -        keyhash[pt['k']] = 1
 -
 -        pref.k = pt['k']
 -        pref.v = pt['v']
 -        pref.user_id = @user.id
 -        prefs << pref
 -      end
 +    doc.find('//preferences/preference').each do |pt|
 +      pref = UserPreference.new
  
 -      if prefs.size > 150
 -        render :text => 'Too many preferences', :status => :request_entity_too_large
 -        return
 +      unless keyhash[pt['k']].nil? # already have that key
 +        render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable
        end
  
 -      # kill the existing ones
 -      UserPreference.delete_all(['user_id = ?', @user.id])
 +      keyhash[pt['k']] = 1
  
 -      # save the new ones
 -      prefs.each do |pref|
 -        pref.save!
 -      end
 +      pref.k = pt['k']
 +      pref.v = pt['v']
 +      pref.user_id = @user.id
 +      prefs << pref
 +    end
  
 -    rescue Exception => ex
 -      render :text => 'OH NOES! FAIL!: ' + ex.to_s, :status => :internal_server_error
 -      return
 +    if prefs.size > 150
 +      render :text => 'Too many preferences', :status => :request_entity_too_large
      end
  
 +    # kill the existing ones
 +    UserPreference.delete_all(['user_id = ?', @user.id])
 +
 +    # save the new ones
 +    prefs.each do |pref|
 +      pref.save!
 +    end
      render :nothing => true
 +
 +  rescue Exception => ex
 +    render :text => 'OH NOES! FAIL!: ' + ex.to_s, :status => :internal_server_error
    end
  end
index 31d927e9a6649094b7ff9825519bad8ccff26365,0000000000000000000000000000000000000000..d420f537a2472d8fe0a0f0058dee62952dd8adcf
mode 100644,000000..100644
--- /dev/null
@@@ -1,238 -1,0 +1,237 @@@
-       p = XML::Parser.new
-       p.string = xml
 +class Changeset < ActiveRecord::Base
 +  require 'xml/libxml'
 +
 +  belongs_to :user
 +
 +  has_many :changeset_tags, :foreign_key => 'id'
 +  
 +  has_many :nodes
 +  has_many :ways
 +  has_many :relations
 +  has_many :old_nodes
 +  has_many :old_ways
 +  has_many :old_relations
 +  
 +  validates_presence_of :id, :on => :update
 +  validates_presence_of :user_id, :created_at, :closed_at, :num_changes
 +  validates_uniqueness_of :id
 +  validates_numericality_of :id, :on => :update, :integer_only => true
 +  validates_numericality_of :min_lat, :max_lat, :min_lon, :max_lat, :allow_nil => true, :integer_only => true
 +  validates_numericality_of :user_id,  :integer_only => true
 +  validates_numericality_of :num_changes, :integer_only => true, :greater_than_or_equal_to => 0
 +  validates_associated :user
 +
 +  # over-expansion factor to use when updating the bounding box
 +  EXPAND = 0.1
 +
 +  # maximum number of elements allowed in a changeset
 +  MAX_ELEMENTS = 50000
 +
 +  # maximum time a changeset is allowed to be open for.
 +  MAX_TIME_OPEN = 1.day
 +
 +  # idle timeout increment, one hour seems reasonable.
 +  IDLE_TIMEOUT = 1.hour
 +
 +  # Use a method like this, so that we can easily change how we
 +  # determine whether a changeset is open, without breaking code in at 
 +  # least 6 controllers
 +  def is_open?
 +    # a changeset is open (that is, it will accept further changes) when
 +    # it has not yet run out of time and its capacity is small enough.
 +    # note that this may not be a hard limit - due to timing changes and
 +    # concurrency it is possible that some changesets may be slightly 
 +    # longer than strictly allowed or have slightly more changes in them.
 +    return ((closed_at > Time.now) and (num_changes <= MAX_ELEMENTS))
 +  end
 +
 +  def set_closed_time_now
 +    if is_open?
 +      self.closed_at = Time.now
 +    end
 +  end
 +  
 +  def self.from_xml(xml, create=false)
 +    begin
++      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
 +          # initial close time is 1h ahead, but will be increased on each
 +          # modification.
 +          cs.closed_at = Time.now + 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
 +      end
 +    rescue Exception => ex
 +      cs = nil
 +    end
 +
 +    return cs
 +  end
 +
 +  ##
 +  # 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 ]
 +  end
 +
 +  ##
 +  # expand the bounding box to include the given bounding box. also, 
 +  # 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 }
 +
 +    # 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]
 +
 +    # 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
 +
 +  ##
 +  # the number of elements is also passed in so that we can ensure that
 +  # a single changeset doesn't contain too many elements. this, of course,
 +  # destroys the optimisation described in the bbox method above.
 +  def add_changes!(elements)
 +    self.num_changes += elements
 +  end
 +
 +  def tags_as_hash
 +    return tags
 +  end
 +
 +  def tags
 +    unless @tags
 +      @tags = {}
 +      self.changeset_tags.each do |tag|
 +        @tags[tag.k] = tag.v
 +      end
 +    end
 +    @tags
 +  end
 +
 +  def tags=(t)
 +    @tags = t
 +  end
 +
 +  def add_tag_keyval(k, v)
 +    @tags = Hash.new unless @tags
 +    @tags[k] = v
 +  end
 +
 +  def save_with_tags!
 +    t = Time.now
 +
 +    # do the changeset update and the changeset tags update in the
 +    # same transaction to ensure consistency.
 +    Changeset.transaction do
 +      # set the auto-close time to be one hour in the future unless
 +      # that would make it more than 24h long, in which case clip to
 +      # 24h, as this has been decided is a reasonable time limit.
 +      if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT)
 +        self.closed_at = created_at + MAX_TIME_OPEN
 +      else
 +        self.closed_at = Time.now + IDLE_TIMEOUT
 +      end
 +      self.save!
 +
 +      tags = self.tags
 +      ChangesetTag.delete_all(['id = ?', self.id])
 +
 +      tags.each do |k,v|
 +        tag = ChangesetTag.new
 +        tag.k = k
 +        tag.v = v
 +        tag.id = self.id
 +        tag.save!
 +      end
 +    end
 +  end
 +  
 +  def to_xml
 +    doc = OSM::API.new.get_xml_doc
 +    doc.root << to_xml_node()
 +    return doc
 +  end
 +  
 +  def to_xml_node(user_display_name_cache = nil)
 +    el1 = XML::Node.new 'changeset'
 +    el1['id'] = self.id.to_s
 +
 +    user_display_name_cache = {} if user_display_name_cache.nil?
 +
 +    if user_display_name_cache and user_display_name_cache.key?(self.user_id)
 +      # use the cache if available
 +    elsif self.user.data_public?
 +      user_display_name_cache[self.user_id] = self.user.display_name
 +    else
 +      user_display_name_cache[self.user_id] = nil
 +    end
 +
 +    el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil?
 +    el1['uid'] = self.user_id.to_s if self.user.data_public?
 +
 +    self.tags.each do |k,v|
 +      el2 = XML::Node.new('tag')
 +      el2['k'] = k.to_s
 +      el2['v'] = v.to_s
 +      el1 << el2
 +    end
 +    
 +    el1['created_at'] = self.created_at.xmlschema
 +    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?
 +    
 +    # NOTE: changesets don't include the XML of the changes within them,
 +    # they are just structures for tagging. to get the osmChange of a
 +    # changeset, see the download method of the controller.
 +
 +    return el1
 +  end
 +
 +  ##
 +  # update this instance from another instance given and the user who is
 +  # doing the updating. note that this method is not for updating the
 +  # bounding box, only the tags of the changeset.
 +  def update_from(other, user)
 +    # ensure that only the user who opened the changeset may modify it.
 +    unless user.id == self.user_id 
 +      raise OSM::APIUserChangesetMismatchError 
 +    end
 +    
 +    # can't change a closed changeset
 +    unless is_open?
 +      raise OSM::APIChangesetAlreadyClosedError.new(self)
 +    end
 +
 +    # copy the other's tags
 +    self.tags = other.tags
 +
 +    save_with_tags!
 +  end
 +end
index f2ad3a78add2196f117214816489d3c7bdc686cb,af88a117d83879ef0eb21d438a11d211cb670dc7..d3e0a7e8d392a470ed193eb6de4fff0738ee8e29
@@@ -64,51 -57,43 +64,50 @@@ class Node < ActiveRecord::Bas
    # Read in xml as text and return it's Node object representation
    def self.from_xml(xml, create=false)
      begin
-       p = XML::Parser.new
-       p.string = xml
+       p = XML::Parser.string(xml)
        doc = p.parse
 -  
 -      node = Node.new
  
        doc.find('//osm/node').each do |pt|
 -        node.lat = pt['lat'].to_f
 -        node.lon = pt['lon'].to_f
 +        return Node.from_xml_node(pt, create)
 +      end
 +    rescue LibXML::XML::Error => ex
 +      raise OSM::APIBadXMLError.new("node", xml, ex.message)
 +    end
 +  end
  
 -        return nil unless node.in_world?
 +  def self.from_xml_node(pt, create=false)
 +    node = Node.new
 +    
 +    raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt['lat'].nil?
 +    raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt['lon'].nil?
 +    node.lat = pt['lat'].to_f
 +    node.lon = pt['lon'].to_f
 +    raise OSM::APIBadXMLError.new("node", pt, "changeset id missing") if pt['changeset'].nil?
 +    node.changeset_id = pt['changeset'].to_i
  
 -        unless create
 -          if pt['id'] != '0'
 -            node.id = pt['id'].to_i
 -          end
 -        end
 +    raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world?
  
 -        node.visible = pt['visible'] and pt['visible'] == 'true'
 +    # version must be present unless creating
 +    raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create or not pt['version'].nil?
 +    node.version = create ? 0 : pt['version'].to_i
  
 -        if create
 -          node.timestamp = Time.now
 -        else
 -          if pt['timestamp']
 -            node.timestamp = Time.parse(pt['timestamp'])
 -          end
 -        end
 +    unless create
 +      if pt['id'] != '0'
 +        node.id = pt['id'].to_i
 +      end
 +    end
  
 -        tags = []
 +    # visible if it says it is, or as the default if the attribute
 +    # is missing.
 +    # Don't need to set the visibility, when it is set explicitly in the create/update/delete
 +    #node.visible = pt['visible'].nil? or pt['visible'] == 'true'
  
 -        pt.find('tag').each do |tag|
 -          tags << [tag['k'],tag['v']]
 -        end
 +    # We don't care about the time, as it is explicitly set on create/update/delete
  
 -        node.tags = Tags.join(tags)
 -      end
 -    rescue
 -      node = nil
 +    tags = []
 +
 +    pt.find('tag').each do |tag|
 +      node.add_tag_key_val(tag['k'],tag['v'])
      end
  
      return node
index 6be1061591dda7b665b20ceed5b5b81ae79e96d3,d9dba303fd8a6bbca4450dfa0260b8e4d4e8d4d9..64af4ecc1eb9d59eff27e8b46d292f3a2aa560f6
@@@ -15,70 -13,39 +15,69 @@@ class Relation < ActiveRecord::Bas
    has_many :containing_relation_members, :class_name => "RelationMember", :as => :member
    has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder
  
 +  validates_presence_of :id, :on => :update
 +  validates_presence_of :timestamp,:version,  :changeset_id 
 +  validates_uniqueness_of :id
 +  validates_inclusion_of :visible, :in => [ true, false ]
 +  validates_numericality_of :id, :on => :update, :integer_only => true
 +  validates_numericality_of :changeset_id, :version, :integer_only => true
 +  validates_associated :changeset
 +  
 +  TYPES = ["node", "way", "relation"]
 +
    def self.from_xml(xml, create=false)
      begin
-       p = XML::Parser.new
-       p.string = xml
+       p = XML::Parser.string(xml)
        doc = p.parse
  
 -      relation = Relation.new
 -
        doc.find('//osm/relation').each do |pt|
 -        if !create and pt['id'] != '0'
 -          relation.id = pt['id'].to_i
 -        end
 +        return Relation.from_xml_node(pt, create)
 +      end
 +    rescue LibXML::XML::Error => ex
 +      raise OSM::APIBadXMLError.new("relation", xml, ex.message)
 +    end
 +  end
  
 -        if create
 -          relation.timestamp = Time.now
 -          relation.visible = true
 -        else
 -          if pt['timestamp']
 -            relation.timestamp = Time.parse(pt['timestamp'])
 -          end
 -        end
 +  def self.from_xml_node(pt, create=false)
 +    relation = Relation.new
  
 -        pt.find('tag').each do |tag|
 -          relation.add_tag_keyval(tag['k'], tag['v'])
 -        end
 +    if !create and pt['id'] != '0'
 +      relation.id = pt['id'].to_i
 +    end
  
 -        pt.find('member').each do |member|
 -          relation.add_member(member['type'], member['ref'], member['role'])
 -        end
 +    raise OSM::APIBadXMLError.new("relation", pt, "You are missing the required changeset in the relation") if pt['changeset'].nil?
 +    relation.changeset_id = pt['changeset']
 +
 +    # The follow block does not need to be executed because they are dealt with 
 +    # in create_with_history, update_from and delete_with_history
 +    if create
 +      relation.timestamp = Time.now
 +      relation.visible = true
 +      relation.version = 0
 +    else
 +      if pt['timestamp']
 +        relation.timestamp = Time.parse(pt['timestamp'])
        end
 -    rescue
 -      relation = nil
 +      relation.version = pt['version']
 +    end
 +
 +    pt.find('tag').each do |tag|
 +      relation.add_tag_keyval(tag['k'], tag['v'])
      end
  
 +    pt.find('member').each do |member|
 +      #member_type = 
 +      logger.debug "each member"
 +      raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member['type']
 +      logger.debug "after raise"
 +      #member_ref = member['ref']
 +      #member_role
 +      member['role'] ||= "" # Allow  the upload to not include this, in which case we default to an empty string.
 +      logger.debug member['role']
 +      relation.add_member(member['type'], member['ref'], member['role'])
 +    end
 +    raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
 +
      return relation
    end
  
index 86b25e08e2fb44a9660bc197221e35d109b46f69,6c3ea9e462dcda9b5d18b2a1d23e23a48d38ca0e..dbc1197a9e039ec003b1e623e92bdf97e9c1faf2
@@@ -17,57 -15,37 +17,56 @@@ class Way < ActiveRecord::Bas
    has_many :containing_relation_members, :class_name => "RelationMember", :as => :member
    has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder
  
 +  validates_presence_of :id, :on => :update
 +  validates_presence_of :changeset_id,:version,  :timestamp
 +  validates_uniqueness_of :id
 +  validates_inclusion_of :visible, :in => [ true, false ]
 +  validates_numericality_of :changeset_id, :version, :integer_only => true
 +  validates_numericality_of :id, :on => :update, :integer_only => true
 +  validates_associated :changeset
 +
    def self.from_xml(xml, create=false)
      begin
-       p = XML::Parser.new
-       p.string = xml
+       p = XML::Parser.string(xml)
        doc = p.parse
  
 -      way = Way.new
 -
        doc.find('//osm/way').each do |pt|
 -        if !create and pt['id'] != '0'
 -          way.id = pt['id'].to_i
 -        end
 -
 -        if create
 -          way.timestamp = Time.now
 -          way.visible = true
 -        else
 -          if pt['timestamp']
 -            way.timestamp = Time.parse(pt['timestamp'])
 -          end
 -        end
 +        return Way.from_xml_node(pt, create)
 +      end
 +    rescue LibXML::XML::Error => ex
 +      raise OSM::APIBadXMLError.new("way", xml, ex.message)
 +    end
 +  end
  
 -        pt.find('tag').each do |tag|
 -          way.add_tag_keyval(tag['k'], tag['v'])
 -        end
 +  def self.from_xml_node(pt, create=false)
 +    way = Way.new
  
 -        pt.find('nd').each do |nd|
 -          way.add_nd_num(nd['ref'])
 -        end
 +    if !create and pt['id'] != '0'
 +      way.id = pt['id'].to_i
 +    end
 +    
 +    way.version = pt['version']
 +    raise OSM::APIBadXMLError.new("node", pt, "Changeset is required") if pt['changeset'].nil?
 +    way.changeset_id = pt['changeset']
 +
 +    # This next section isn't required for the create, update, or delete of ways
 +    if create
 +      way.timestamp = Time.now
 +      way.visible = true
 +    else
 +      if pt['timestamp']
 +        way.timestamp = Time.parse(pt['timestamp'])
        end
 -    rescue
 -      way = nil
 +      # if visible isn't present then it defaults to true
 +      way.visible = (pt['visible'] or true)
 +    end
 +
 +    pt.find('tag').each do |tag|
 +      way.add_tag_keyval(tag['k'], tag['v'])
 +    end
 +
 +    pt.find('nd').each do |nd|
 +      way.add_nd_num(nd['ref'])
      end
  
      return way
index 9852313bbd7e669d308af8586fd2ed0c49562580,3d1f0bbcb16acf3c2270e92be3e6d5a04d939f1a..38157b183b976883282d86ba2bdcca717ccf0857
    <% end %>
  <% end %>
  
 -<h3>Recent diary entries:</h3>
  
 -<%= render :partial => 'diary_entry', :collection => @entries %>
 +<% if @entries.empty? %>
-       <p>No diary entries</p>
++  <p>No diary entries</p>
 +<% else %>
++  <p>Recent diary entries:</p>
 -<%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %>
 -<% if @entry_pages.current.next and @entry_pages.current.previous %>
 -|
 -<% end %>
 -<%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %>
++  <hr />
 -<br />
++  <%= render :partial => 'diary_entry', :collection => @entries %>
 +      
-       <p>Recent diary entries:</p>
-       <hr />
-       <%= render :partial => 'diary_entry', :collection => @entries %>
-       
-       <%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %>
-       <% if @entry_pages.current.next and @entry_pages.current.previous %>
-       |
-       <% end %>
-       <%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %>
-       
-       <br />
-       
++  <%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %>
++  <% if @entry_pages.current.next and @entry_pages.current.previous %>|<% end %>
++  <%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %>
++
++  <br />
 +<% end %>
  
  <%= rss_link_to :action => 'rss' %>
- <%= auto_discovery_link_tag :atom, :action => 'rss' %>
  
- <br />
- <br />
+ <% content_for :head do %>
+ <%= auto_discovery_link_tag :atom, :action => 'rss' %>
+ <% end %>
index cf7ad9fc811f839adc9fe2cf413831ebb6954996,34d2ecb309c81afa994d1bfc8b9b5350d25617ab..cc8f7192d6953c06be92ae39af19114badfdca1a
@@@ -7,7 -7,7 +7,8 @@@
      <%= stylesheet_link_tag 'site' %>
      <%= stylesheet_link_tag 'print', :media => "print" %>
      <%= tag("link", { :rel => "search", :type => "application/opensearchdescription+xml", :title => "OpenStreetMap Search", :href => "/opensearch/osm.xml" }) %>
 +    <%= tag("meta", { :name => "description", :content => "OpenStreetMap is the free wiki world map." }) %>
+     <%= yield :head %>
      <title>OpenStreetMap<%= ' | '+ h(@title) if @title %></title>
    </head>
    <body>
        </div>
  
        <%= yield :optionals %>
-       <div id="cclogo">
-         <center>
  
-           <form action="https://www.paypal.com/cgi-bin/webscr" method="post">
-             <input type="hidden" name="cmd" value="_s-xclick" />
-             <input type="image" src="https://www.paypal.com/en_US/i/btn/x-click-but21.gif" style="border: none;" name="submit" alt="Make payments with PayPal - it's fast, free and secure!" />
-             <img alt="" border="0" src="https://www.paypal.com/en_GB/i/scr/pixel.gif" width="1" height="1" />
-             <input type="hidden" name="encrypted" value="-----BEGIN PKCS7-----MIIHTwYJKoZIhvcNAQcEoIIHQDCCBzwCAQExggEwMIIBLAIBADCBlDCBjjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAkNBMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQKEwtQYXlQYWwgSW5jLjETMBEGA1UECxQKbGl2ZV9jZXJ0czERMA8GA1UEAxQIbGl2ZV9hcGkxHDAaBgkqhkiG9w0BCQEWDXJlQHBheXBhbC5jb20CAQAwDQYJKoZIhvcNAQEBBQAEgYCsNDDDDa7OZFojBzDvG4HSPXOiJSO3VNuLoc8HGwsds3LsZYYtv4cPGw7Z/SoVVda+RELM+5FQn0D3Kv7hjA2Z6QdwEkFH2kDDlXCvyPt53ENHkQrzC1KOueRpimsQMH5hl03nvuVXij0hEYlMFqTH0UZr80vyczB+lJU6ZKYtrDELMAkGBSsOAwIaBQAwgcwGCSqGSIb3DQEHATAUBggqhkiG9w0DBwQIZa12CIRB0geAgahqF6Otz0oY0+Wg56fSuEpZvbUmNGEQznjWqBXkJqTkZT0jOwekOrlEi7bNEU8yVIie2u5L1gOhBDSl6rmgpxxVURSa4Jig5qiSioyK5baH6HjXVPQ+MDEWg1gZ4LtjYYtroZ8SBE/1eikQWmG7EOEgU62Vn/jqJJ77/mgS7mdEQhlEWYMiyJBZs35yCB/pK5FUxhZnrquL4sS+2QKHPPOGPDfRc/dnhMKgggOHMIIDgzCCAuygAwIBAgIBADANBgkqhkiG9w0BAQUFADCBjjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAkNBMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQKEwtQYXlQYWwgSW5jLjETMBEGA1UECxQKbGl2ZV9jZXJ0czERMA8GA1UEAxQIbGl2ZV9hcGkxHDAaBgkqhkiG9w0BCQEWDXJlQHBheXBhbC5jb20wHhcNMDQwMjEzMTAxMzE1WhcNMzUwMjEzMTAxMzE1WjCBjjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAkNBMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQKEwtQYXlQYWwgSW5jLjETMBEGA1UECxQKbGl2ZV9jZXJ0czERMA8GA1UEAxQIbGl2ZV9hcGkxHDAaBgkqhkiG9w0BCQEWDXJlQHBheXBhbC5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMFHTt38RMxLXJyO2SmS+Ndl72T7oKJ4u4uw+6awntALWh03PewmIJuzbALScsTS4sZoS1fKciBGoh11gIfHzylvkdNe/hJl66/RGqrj5rFb08sAABNTzDTiqqNpJeBsYs/c2aiGozptX2RlnBktH+SUNpAajW724Nv2Wvhif6sFAgMBAAGjge4wgeswHQYDVR0OBBYEFJaffLvGbxe9WT9S1wob7BDWZJRrMIG7BgNVHSMEgbMwgbCAFJaffLvGbxe9WT9S1wob7BDWZJRroYGUpIGRMIGOMQswCQYDVQQGEwJVUzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDU1vdW50YWluIFZpZXcxFDASBgNVBAoTC1BheVBhbCBJbmMuMRMwEQYDVQQLFApsaXZlX2NlcnRzMREwDwYDVQQDFAhsaXZlX2FwaTEcMBoGCSqGSIb3DQEJARYNcmVAcGF5cGFsLmNvbYIBADAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4GBAIFfOlaagFrl71+jq6OKidbWFSE+Q4FqROvdgIONth+8kSK//Y/4ihuE4Ymvzn5ceE3S/iBSQQMjyvb+s2TWbQYDwcp129OPIbD9epdr4tJOUNiSojw7BHwYRiPh58S1xGlFgHFXwrEBb3dgNbMUa+u4qectsMAXpVHnD9wIyfmHMYIBmjCCAZYCAQEwgZQwgY4xCzAJBgNVBAYTAlVTMQswCQYDVQQIEwJDQTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEUMBIGA1UEChMLUGF5UGFsIEluYy4xEzARBgNVBAsUCmxpdmVfY2VydHMxETAPBgNVBAMUCGxpdmVfYXBpMRwwGgYJKoZIhvcNAQkBFg1yZUBwYXlwYWwuY29tAgEAMAkGBSsOAwIaBQCgXTAYBgkqhkiG9w0BCQMxCwYJKoZIhvcNAQcBMBwGCSqGSIb3DQEJBTEPFw0wNjA4MjYwODQ2NDdaMCMGCSqGSIb3DQEJBDEWBBTyC1ZchvuTMtcYeudPPSP/w8HiEDANBgkqhkiG9w0BAQEFAASBgJPpBf69pRAJfhzv/MfPiMncuq3TSlvpX7VtG9p4dXzSko4i2lWUDD72r5zdF2NwDgZ6avf630PutgpOzYJQ525If1xU2olc9DWI43UZTqY+FArgFuCJ8VnkPsy9mcbXPoSjLRqNwrsA2yoETxMISO3ASELzELJTJgpPk4bU57eZ-----END PKCS7-----" />
-           </form>
-           <%= link_to (image_tag "cc_button.png", :alt => "CC by-sa 2.0", :border => "0"), "http://creativecommons.org/licenses/by-sa/2.0/" %>
-         </center>
-       </div>
+       <center>
+         <div class="button" style="width: 115px">
+           <a href="http://donate.openstreetmap.org/"><img src="/images/donate.png" border="0" alt="Make a Donation" /></a>
+         </div>
  
 -          <a href="http://creativecommons.org/licenses/by-sa/2.0/"><img src="/images/cc_button.png" border="0" alt="" /></a>
+         <div id="cclogo" class="button" style="width: 88px">
++          <%= link_to image_tag("cc_button.png", :alt => "CC by-sa 2.0", :border => "0"), "http://creativecommons.org/licenses/by-sa/2.0/" %>
+         </div>
+       </center>
      </div>
    </body>
  </html>
index 770ad8873d018390d866f6008425a617e12c5184,ff988f070be1ff76b97a4e1811e8306ac61fc714..3c40906ca92bc4e086e15d0e9faa14d3130ded63
@@@ -1,15 -1,13 +1,13 @@@
--<h1>Login:</h1><br />
--Please login or <%= link_to 'create an account', :controller => 'user', :action => 'new' %>.<br />
++<h1>Login</h1>
++
++<p>Please login or <%= link_to 'create an account', :controller => 'user', :action => 'new' %>.</p>
  
  <% form_tag :action => 'login' do %>
  <%= hidden_field_tag('referer', h(params[:referer])) %>
- <br/>
  <table>
-   <tr><td class="fieldName">Email Address or username:</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
 -  <tr><td>Email Address or username:</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
 -  <tr><td>Password:</td><td><%= password_field('user', 'password',{:size => 50, :maxlength => 255}) %></td></tr>
++  <tr><td class="fieldName">Email Address or Username:</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
 +  <tr><td class="fieldName">Password:</td><td><%= password_field('user', 'password',{:size => 28, :maxlength => 255}) %> <span class="minorNote">(<%= link_to 'Lost your password?', :controller => 'user', :action => 'lost_password' %>)</span></td></tr>
 +  <tr><td colspan=2>&nbsp;<!--vertical spacer--></td></tr>
 +  <tr><td></td><td align="right"><%= submit_tag 'Login' %></td></tr>
  </table>
--
--<br />
 -<%= submit_tag 'Login' %>
 -<% end %> (<%= link_to 'Lost your password?', :controller => 'user', :action => 'lost_password' %>)
 +<% end %>
index 1b7f6e9b433bf58b30b9f2fc343f4910b3975699,d0b5a9667174d51e93104bf5e3d83627d51fe6a6..823dccf520db76dd0a2032bd93ad6f568ed7cb64
@@@ -1,28 -1,43 +1,45 @@@
- <h1>Create a user account</h1><br>
- Fill in the form and we'll send you a quick email to activate your account.
- <br><br>
 -<h1>Create a user account</h1>
++<h1>Create a User Account</h1>
  
- By creating an account, you agree that all work uploaded to openstreetmap.org and all data created by use of any tools which connect to openstreetmap.org is to be (non-exclusively) licensed under <a href="http://creativecommons.org/licenses/by-sa/2.0/">this Creative Commons license (by-sa)</a>.<br><br>
+ <% if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"}) %>
+ <p>Unfortunately we are not currently able to create an account for
+    you automatically.
+ </p>
+ <p>Please contact the <a href="mailto:webmaster@openstreetmap.org">webmaster</a>
+    to arrange for an account to be created - we will try and deal with
+    the request as quickly as possible. 
+ </p>
+ <% else %>
+ <p>Fill in the form and we'll send you a quick email to activate your
+    account.
+ </p>
+ <p>By creating an account, you agree that all work uploaded to
+    openstreetmap.org and all data created by use of any tools which
+    connect to openstreetmap.org is to be (non-exclusively) licensed under
+    <a href="http://creativecommons.org/licenses/by-sa/2.0/">this Creative
+    Commons license (by-sa)</a>.
+ </p>
  
  <%= error_messages_for 'user' %>
  
  <% form_tag :action => 'save' do %>
 -<table>
 -  <tr><td>Email Address</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
 -  <tr><td>Confirm Email Address</td><td><%= text_field('user', 'email_confirmation',{:size => 50, :maxlength => 255}) %></td></tr>
 -  <tr><td>Display Name</td><td><%= text_field('user', 'display_name',{:size => 50, :maxlength => 255}) %></td></tr>
 -  <tr><td>Password</td><td><%= password_field('user', 'pass_crypt',{:size => 50, :maxlength => 255}) %></td></tr>
 -  <tr><td>Confirm Password</td><td><%= password_field('user', 'pass_crypt_confirmation',{:size => 50, :maxlength => 255}) %></td></tr>
 +<table id="loginForm">
 +  <tr><td class="fieldName">Email Address : </td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
 +  <tr><td class="fieldName">Confirm Email Address : </td><td><%= text_field('user', 'email_confirmation',{:size => 50, :maxlength => 255}) %></td></tr>
 +  <tr><td></td><td><span class="minorNote">Not displayed publicly (see <a href="http://wiki.openstreetmap.org/index.php/Privacy_Policy" title="wiki privacy policy including section on email addresses">privacy policy)</span></td></tr>
 +  <tr><td colspan=2>&nbsp;<!--vertical spacer--></td></tr>
 +  <tr><td class="fieldName">Display Name : </td><td><%= text_field('user', 'display_name',{:size => 30, :maxlength => 255}) %></td></tr>
 +  <tr><td colspan=2>&nbsp;<!--vertical spacer--></td></tr>
 +  <tr><td class="fieldName">Password : </td><td><%= password_field('user', 'pass_crypt',{:size => 30, :maxlength => 255}) %></td></tr>
 +  <tr><td class="fieldName">Confirm Password : </td><td><%= password_field('user', 'pass_crypt_confirmation',{:size => 30, :maxlength => 255}) %></td></tr>
 +  
 +  <tr><td colspan=2>&nbsp;<!--vertical spacer--></td></tr>
 +  <tr><td></td><td align=right><input type="submit" value="Signup"></td></tr>
  </table>
--<br>
--<br>
- <!--
- See also <a href="http://wiki.openstreetmap.org/index.php/Creating_an_Account" title="wiki help information about this screen">'Creating an Account' help</a>
- -->
 -<input type="submit" value="Signup">
 -
+ <% end %>
  <% end %>
index ffed548bd4360f2715b9a83b82dda5fdc171ebd3,e6af619eb82b03aaaf8400db39ea951dbba227b6..b3481c024434dcaba4fc36584e7e0aca0e7b5666
@@@ -40,16 -37,6 +40,16 @@@ Rails::Initializer.run do |config
      config.frameworks -= [ :active_record ]
    end
  
-   config.gem 'libxml-ruby', :version => '0.9.4', :lib => 'libxml'
 +  # Specify gems that this application depends on. 
 +  # They can then be installed with "rake gems:install" on new installations.
 +  # config.gem "bj"
 +  # config.gem "hpricot", :version => '0.6', :source => "http://code.whytheluckystiff.net"
 +  # config.gem "aws-s3", :lib => "aws/s3"
 +  config.gem 'composite_primary_keys', :version => '1.1.0'
++  config.gem 'libxml-ruby', :version => '>= 1.0.0', :lib => 'libxml'
 +  config.gem 'rmagick', :lib => 'RMagick'
 +  config.gem 'mysql'
 +
    # Only load the plugins named here, in the order given. By default, all plugins 
    # in vendor/plugins are loaded in alphabetical order.
    # :all can be used as a placeholder for all plugins not explicitly named
index ae636a9a31039a260539fd2d195fa49e9ce56684,f783cda1eef1a226053b2aa7a98feecd2ffe64a6..07f79660f8a8b13107a3bc84dd6f41430b960200
@@@ -1,9 -1,9 +1,5 @@@
 -require 'rubygems'
 -gem 'libxml-ruby', '>= 1.0.0'
 -require 'libxml'
 -
  # This is required otherwise libxml writes out memory errors to
- # the standard output and exits uncleanly 
- # Changed method due to deprecation of the old register_error_handler
- # http://libxml.rubyforge.org/rdoc/classes/LibXML/XML/Parser.html#M000076
- # So set_handler is used instead
- # http://libxml.rubyforge.org/rdoc/classes/LibXML/XML/Error.html#M000334
+ # the standard output and exits uncleanly
  LibXML::XML::Error.set_handler do |message|
    raise message
  end
index c6b3bc7c2cd3e303f606dabd234483bcd6750b1b,0000000000000000000000000000000000000000..c6b3bc7c2cd3e303f606dabd234483bcd6750b1b
mode 100644,000000..100644
--- /dev/null
index 8603586463f8b8330583a33bf67d87dd737d5257,0000000000000000000000000000000000000000..dc048f1906134eab74d7f3b7bf35c09d3bfb3e77
mode 100644,000000..100644
--- /dev/null
@@@ -1,60 -1,0 +1,60 @@@
-       prefix = File.join Dir.tmpdir, "019_populate_node_tags_and_remove.#{$$}."
 +class PopulateNodeTagsAndRemove < ActiveRecord::Migration
 +  def self.up
 +    have_nodes = select_value("SELECT count(*) FROM current_nodes").to_i != 0
 +
 +    if have_nodes
-       cmd = "db/migrate/019_populate_node_tags_and_remove_helper"
++      prefix = File.join Dir.tmpdir, "020_populate_node_tags_and_remove.#{$$}."
 +
++      cmd = "db/migrate/020_populate_node_tags_and_remove_helper"
 +      src = "#{cmd}.c"
 +      if not File.exists? cmd or File.mtime(cmd) < File.mtime(src) then 
 +        system 'cc -O3 -Wall `mysql_config --cflags --libs` ' +
 +          "#{src} -o #{cmd}" or fail
 +      end
 +
 +      conn_opts = ActiveRecord::Base.connection.instance_eval { @connection_options }
 +      args = conn_opts.map { |arg| arg.to_s } + [prefix]
 +      fail "#{cmd} failed" unless system cmd, *args
 +
 +      tempfiles = ['nodes', 'node_tags', 'current_nodes', 'current_node_tags'].
 +        map { |base| prefix + base }
 +      nodes, node_tags, current_nodes, current_node_tags = tempfiles
 +    end
 +
 +    execute "TRUNCATE nodes"
 +    remove_column :nodes, :tags
 +    remove_column :current_nodes, :tags
 +
 +    add_column :nodes, :version, :bigint, :limit => 20, :null => false
 +
 +    create_table :current_node_tags, innodb_table do |t|
 +      t.column :id,          :bigint, :limit => 64, :null => false
 +      t.column :k,         :string, :default => "", :null => false
 +      t.column :v,         :string, :default => "", :null => false
 +    end
 +
 +    create_table :node_tags, innodb_table do |t|
 +      t.column :id,          :bigint, :limit => 64, :null => false
 +      t.column :version,     :bigint, :limit => 20, :null => false
 +      t.column :k,         :string, :default => "", :null => false
 +      t.column :v,         :string, :default => "", :null => false
 +    end
 +
 +    # now get the data back
 +    csvopts = "FIELDS TERMINATED BY ',' ENCLOSED BY '\"' ESCAPED BY '\"' LINES TERMINATED BY '\\n'"
 +
 +    if have_nodes
 +      execute "LOAD DATA INFILE '#{nodes}' INTO TABLE nodes #{csvopts} (id, latitude, longitude, user_id, visible, timestamp, tile, version)";
 +      execute "LOAD DATA INFILE '#{node_tags}' INTO TABLE node_tags #{csvopts} (id, version, k, v)"
 +      execute "LOAD DATA INFILE '#{current_node_tags}' INTO TABLE current_node_tags #{csvopts} (id, k, v)"
 +    end
 +
 +    tempfiles.each { |fn| File.unlink fn } if have_nodes
 +  end
 +
 +  def self.down
 +    raise IrreversibleMigration.new
 +#    add_column :nodes, "tags", :text, :default => "", :null => false
 +#    add_column :current_nodes, "tags", :text, :default => "", :null => false
 +  end
 +end
index c41ea33da77e1912bd0ceba7291b6ce04ebf919d,0000000000000000000000000000000000000000..c41ea33da77e1912bd0ceba7291b6ce04ebf919d
mode 100644,000000..100644
--- /dev/null
index da0488ca5f7a3d6bb40f6ccc7717ceb34487096e,0000000000000000000000000000000000000000..da0488ca5f7a3d6bb40f6ccc7717ceb34487096e
mode 100644,000000..100644
--- /dev/null
index 40f98be02b9eb3dd61cc0538612fff288d353492,0000000000000000000000000000000000000000..40f98be02b9eb3dd61cc0538612fff288d353492
mode 100644,000000..100644
--- /dev/null
index e0cf3904a50d8ea98f79e40d1e20f5f8707336ae,0000000000000000000000000000000000000000..e0cf3904a50d8ea98f79e40d1e20f5f8707336ae
mode 100644,000000..100644
--- /dev/null
index 5500edfcfbf4ac42e9a35e9d754dfdc049a96a47,0000000000000000000000000000000000000000..5500edfcfbf4ac42e9a35e9d754dfdc049a96a47
mode 100644,000000..100644
--- /dev/null
index b87ce3fdebabcf12abd2d1a192eb36547b9f155b,0000000000000000000000000000000000000000..b87ce3fdebabcf12abd2d1a192eb36547b9f155b
mode 100644,000000..100644
--- /dev/null
index 37de8ea59f043822bff8faa353b88bef2e5ec3bf,0000000000000000000000000000000000000000..217e9309d7329d681141fd3a1115a04b693cc6a0
mode 100644,000000..100644
--- /dev/null
@@@ -1,223 -1,0 +1,224 @@@
-     @reader = XML::Reader.new data
 +##
 +# DiffReader reads OSM diffs and applies them to the database.
 +#
 +# Uses the streaming LibXML "Reader" interface to cut down on memory
 +# usage, so hopefully we can process fairly large diffs.
 +class DiffReader
 +  include ConsistencyValidations
 +
 +  # maps each element type to the model class which handles it
 +  MODELS = { 
 +    "node"     => Node, 
 +    "way"      => Way, 
 +    "relation" => Relation
 +  }
 +
 +  ##
 +  # Construct a diff reader by giving it a bunch of XML +data+ to parse
 +  # in OsmChange format. All diffs must be limited to a single changeset
 +  # given in +changeset+.
 +  def initialize(data, changeset)
-     # NOTE: XML::Reader#read returns 0 for EOF and -1 for error.
-     # we allow an EOF because we are expecting this to always happen
-     # at the end of a document.
-     if @reader.read < 0
-       raise APIBadUserInput.new("Unexpected end of XML document.")
++    @reader = XML::Reader.string(data)
 +    @changeset = changeset
 +  end
 +
 +  ##
 +  # Reads the next element from the XML document. Checks the return value
 +  # and throws an exception if an error occurred.
 +  def read_or_die
++    # NOTE: XML::Reader#read returns false for EOF and raises an
++    # exception if an error occurs.
++    begin
++      @reader.read
++    rescue LibXML::XML::Error => ex
++      raise OSM::APIBadXMLError.new("changeset", xml, ex.message)
 +    end
 +  end
 +
 +  ##
 +  # An element-block mapping for using the LibXML reader interface. 
 +  #
 +  # Since a lot of LibXML reader usage is boilerplate iteration through
 +  # elements, it would be better to DRY and do this in a block. This
 +  # could also help with error handling...?
 +  def with_element
 +    # if the start element is empty then don't do any processing, as
 +    # there won't be any child elements to process!
 +    unless @reader.empty_element?
 +      # read the first element
 +      read_or_die
 +
 +      while @reader.node_type != 15 do # end element
 +        # because we read elements in DOM-style to reuse their DOM
 +        # parsing code, we don't always read an element on each pass
 +        # as the call to @reader.next in the innermost loop will take
 +        # care of that for us.
 +        if @reader.node_type == 1 # element
 +          yield @reader.name
 +        else
 +          read_or_die
 +        end
 +      end 
 +    end
 +    read_or_die
 +  end
 +
 +  ##
 +  # An element-block mapping for using the LibXML reader interface. 
 +  #
 +  # Since a lot of LibXML reader usage is boilerplate iteration through
 +  # elements, it would be better to DRY and do this in a block. This
 +  # could also help with error handling...?
 +  def with_model
 +    with_element do |model_name|
 +      model = MODELS[model_name]
 +      raise "Unexpected element type #{model_name}, " +
 +        "expected node, way, relation." if model.nil?
 +      yield model, @reader.expand
 +      @reader.next
 +    end
 +  end
 +
 +  ##
 +  # Checks a few invariants. Others are checked in the model methods
 +  # such as save_ and delete_with_history.
 +  def check(model, xml, new)
 +    raise OSM::APIBadXMLError.new(model, xml) if new.nil?
 +    unless new.changeset_id == @changeset.id 
 +      raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id)
 +    end
 +  end
 +
 +  ##
 +  # Consume the XML diff and try to commit it to the database. This code
 +  # is *not* transactional, so code which calls it should ensure that the
 +  # appropriate transaction block is in place.
 +  #
 +  # On a failure to meet preconditions (e.g: optimistic locking fails) 
 +  # an exception subclassing OSM::APIError will be thrown.
 +  def commit
 +
 +    # data structure used for mapping placeholder IDs to real IDs
 +    node_ids, way_ids, rel_ids = {}, {}, {}
 +    ids = { :node => node_ids, :way => way_ids, :relation => rel_ids}
 +
 +    # take the first element and check that it is an osmChange element
 +    @reader.read
 +    raise APIBadUserInput.new("Document element should be 'osmChange'.") if @reader.name != 'osmChange'
 +
 +    result = OSM::API.new.get_xml_doc
 +    result.root.name = "diffResult"
 +
 +    # loop at the top level, within the <osmChange> element
 +    with_element do |action_name|
 +      if action_name == 'create'
 +        # create a new element. this code is agnostic of the element type
 +        # because all the elements support the methods that we're using.
 +        with_model do |model, xml|
 +          new = model.from_xml_node(xml, true)
 +          check(model, xml, new)
 +
 +          # when this element is saved it will get a new ID, so we save it
 +          # to produce the mapping which is sent to other elements.
 +          placeholder_id = xml['id'].to_i
 +          raise OSM::APIBadXMLError.new(model, xml) if placeholder_id.nil?
 +
 +          # check if the placeholder ID has been given before and throw
 +          # an exception if it has - we can't create the same element twice.
 +          model_sym = model.to_s.downcase.to_sym
 +          raise OSM::APIBadUserInput.new("Placeholder IDs must be unique for created elements.") if ids[model_sym].include? placeholder_id
 +
 +          # some elements may have placeholders for other elements in the
 +          # diff, so we must fix these before saving the element.
 +          new.fix_placeholders!(ids)
 +
 +          # create element given user
 +          new.create_with_history(@changeset.user)
 +          
 +          # save placeholder => allocated ID map
 +          ids[model_sym][placeholder_id] = new.id
 +
 +          # add the result to the document we're building for return.
 +          xml_result = XML::Node.new model.to_s.downcase
 +          xml_result["old_id"] = placeholder_id.to_s
 +          xml_result["new_id"] = new.id.to_s
 +          xml_result["new_version"] = new.version.to_s
 +          result.root << xml_result
 +        end
 +        
 +      elsif action_name == 'modify'
 +        # modify an existing element. again, this code doesn't directly deal
 +        # with types, but uses duck typing to handle them transparently.
 +        with_model do |model, xml|
 +          # get the new element from the XML payload
 +          new = model.from_xml_node(xml, false)
 +          check(model, xml, new)
 +
 +          # if the ID is a placeholder then map it to the real ID
 +          model_sym = model.to_s.downcase.to_sym
 +          is_placeholder = ids[model_sym].include? new.id
 +          id = is_placeholder ? ids[model_sym][new.id] : new.id
 +
 +          # and the old one from the database
 +          old = model.find(id)
 +
 +          new.fix_placeholders!(ids)
 +          old.update_from(new, @changeset.user)
 +
 +          xml_result = XML::Node.new model.to_s.downcase
 +          # oh, the irony... the "new" element actually contains the "old" ID
 +          # a better name would have been client/server, but anyway...
 +          xml_result["old_id"] = new.id.to_s
 +          xml_result["new_id"] = id.to_s 
 +          # version is updated in "old" through the update, so we must not
 +          # return new.version here but old.version!
 +          xml_result["new_version"] = old.version.to_s
 +          result.root << xml_result
 +        end
 +
 +      elsif action_name == 'delete'
 +        # delete action. this takes a payload in API 0.6, so we need to do
 +        # most of the same checks that are done for the modify.
 +        with_model do |model, xml|
 +          # delete doesn't have to contain a full payload, according to
 +          # the wiki docs, so we just extract the things we need.
 +          new_id = xml['id'].to_i
 +          raise API::APIBadXMLError.new(model, xml, "ID attribute is required") if new_id.nil?
 +
 +          # if the ID is a placeholder then map it to the real ID
 +          model_sym = model.to_s.downcase.to_sym
 +          is_placeholder = ids[model_sym].include? new_id
 +          id = is_placeholder ? ids[model_sym][new_id] : new_id
 +
 +          # build the "new" element by modifying the existing one
 +          new = model.find(id)
 +          new.changeset_id = xml['changeset'].to_i
 +          new.version = xml['version'].to_i
 +          check(model, xml, new)
 +
 +          # fetch the matching old element from the DB
 +          old = model.find(id)
 +
 +          # can a delete have placeholders under any circumstances?
 +          # if a way is modified, then deleted is that a valid diff?
 +          new.fix_placeholders!(ids)
 +          old.delete_with_history!(new, @changeset.user)
 +
 +          xml_result = XML::Node.new model.to_s.downcase
 +          # oh, the irony... the "new" element actually contains the "old" ID
 +          # a better name would have been client/server, but anyway...
 +          xml_result["old_id"] = new_id.to_s
 +          result.root << xml_result
 +        end
 +
 +      else
 +        # no other actions to choose from, so it must be the users fault!
 +        raise OSM::APIChangesetActionInvalid.new(action_name)
 +      end
 +    end
 +
 +    # return the XML document to be rendered back to the client
 +    return result
 +  end
 +
 +end
diff --cc lib/osm.rb
index f6646503d0e37e0d68e04efbaf01755be4c6195e,365ddf68ef080bc0aef2e72dce292584edfbbaab..f372979a8d784bf99a49b14746e1753ee7b74927
@@@ -326,10 -187,10 +326,10 @@@ module OS
    class API
      def get_xml_doc
        doc = XML::Document.new
-       doc.encoding = 'UTF-8' 
+       doc.encoding = XML::Encoding::UTF_8
        root = XML::Node.new 'osm'
        root['version'] = API_VERSION
 -      root['generator'] = 'OpenStreetMap server'
 +      root['generator'] = GENERATOR
        doc.root = root
        return doc
      end
Simple merge
index bc9ffa489f1d1673bc297c67c191c6e60e29e708,a380eeb208313f08672104595eef0d188ec72e06..8d019bf7902fa549ec259b9061f71676dc5dbc49
@@@ -257,27 -82,6 +257,26 @@@ class NodeControllerTest < ActionContro
    end
  
    def content(c)
 -    @request.env["RAW_POST_DATA"] = c
 +    @request.env["RAW_POST_DATA"] = c.to_s
 +  end
 +
 +  ##
 +  # update the changeset_id of a node element
 +  def update_changeset(xml, changeset_id)
 +    xml_attr_rewrite(xml, 'changeset', changeset_id)
 +  end
 +
 +  ##
 +  # update an attribute in the node element
 +  def xml_attr_rewrite(xml, name, value)
 +    xml.find("//osm/node").first[name] = value.to_s
 +    return xml
 +  end
 +
 +  ##
 +  # parse some xml
 +  def xml_parse(xml)
-     parser = XML::Parser.new
-     parser.string = xml
++    parser = XML::Parser.string(xml)
 +    parser.parse
    end
  end
index 4ace316a4e96ab479abf152b9a669d1e4fa13d5f,202a015a87f737459b13ec30a4055e9dc5c67a37..f52981233371097a9cd3ad94a1a25f592b5c5405
@@@ -323,225 -205,4 +323,224 @@@ class RelationControllerTest < ActionCo
      assert_response :not_found
    end
  
-     parser = XML::Parser.new
-     parser.string = xml
 +  ##
 +  # when a relation's tag is modified then it should put the bounding
 +  # box of all its members into the changeset.
 +  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|
 +      # add a tag to an existing relation
 +      relation_xml = current_relations(:visible_relation).to_xml
 +      relation_element = relation_xml.find("//osm/relation").first
 +      new_tag = XML::Node.new("tag")
 +      new_tag['k'] = "some_new_tag"
 +      new_tag['v'] = "some_new_value"
 +      relation_element << new_tag
 +      
 +      # update changeset ID to point to new changeset
 +      update_changeset(relation_xml, changeset_id)
 +      
 +      # upload the change
 +      content relation_xml
 +      put :update, :id => current_relations(:visible_relation).id
 +      assert_response :success, "can't update relation for tag/bbox test"
 +    end
 +  end
 +
 +  ##
 +  # add a member to a relation and check the bounding box is only that
 +  # element.
 +  def test_add_member_bounding_box
 +    check_changeset_modify([4,4,4,4]) do |changeset_id|
 +      # add node 4 (4,4) to an existing relation
 +      relation_xml = current_relations(:visible_relation).to_xml
 +      relation_element = relation_xml.find("//osm/relation").first
 +      new_member = XML::Node.new("member")
 +      new_member['ref'] = current_nodes(:used_node_2).id.to_s
 +      new_member['type'] = "node"
 +      new_member['role'] = "some_role"
 +      relation_element << new_member
 +      
 +      # update changeset ID to point to new changeset
 +      update_changeset(relation_xml, changeset_id)
 +      
 +      # upload the change
 +      content relation_xml
 +      put :update, :id => current_relations(:visible_relation).id
 +      assert_response :success, "can't update relation for add node/bbox test"
 +    end
 +  end
 +  
 +  ##
 +  # 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|
 +      # remove node 5 (5,5) from an existing relation
 +      relation_xml = current_relations(:visible_relation).to_xml
 +      relation_xml.
 +        find("//osm/relation/member[@type='node'][@ref='5']").
 +        first.remove!
 +      
 +      # update changeset ID to point to new changeset
 +      update_changeset(relation_xml, changeset_id)
 +      
 +      # upload the change
 +      content relation_xml
 +      put :update, :id => current_relations(:visible_relation).id
 +      assert_response :success, "can't update relation for remove node/bbox test"
 +    end
 +  end
 +  
 +  ##
 +  # check that relations are ordered
 +  def test_relation_member_ordering
 +    basic_authorization("test@openstreetmap.org", "test");  
 +
 +    doc_str = <<OSM
 +<osm>
 + <relation changeset='1'>
 +  <member ref='1' type='node' role='first'/>
 +  <member ref='3' type='node' role='second'/>
 +  <member ref='1' type='way' role='third'/>
 +  <member ref='3' type='way' role='fourth'/>
 + </relation>
 +</osm>
 +OSM
 +    doc = XML::Parser.string(doc_str).parse
 +
 +    content doc
 +    put :create
 +    assert_response :success, "can't create a relation: #{@response.body}"
 +    relation_id = @response.body.to_i
 +
 +    # get it back and check the ordering
 +    get :read, :id => relation_id
 +    assert_response :success, "can't read back the relation: #{@response.body}"
 +    check_ordering(doc, @response.body)
 +
 +    # insert a member at the front
 +    new_member = XML::Node.new "member"
 +    new_member['ref'] = 5.to_s
 +    new_member['type'] = 'node'
 +    new_member['role'] = 'new first'
 +    doc.find("//osm/relation").first.child.prev = new_member
 +    # update the version, should be 1?
 +    doc.find("//osm/relation").first['id'] = relation_id.to_s
 +    doc.find("//osm/relation").first['version'] = 1.to_s
 +
 +    # upload the next version of the relation
 +    content doc
 +    put :update, :id => relation_id
 +    assert_response :success, "can't update relation: #{@response.body}"
 +    new_version = @response.body.to_i
 +
 +    # get it back again and check the ordering again
 +    get :read, :id => relation_id
 +    assert_response :success, "can't read back the relation: #{@response.body}"
 +    check_ordering(doc, @response.body)
 +  end
 +
 +  ## 
 +  # check that relations can contain duplicate members
 +  def test_relation_member_duplicates
 +    basic_authorization("test@openstreetmap.org", "test");  
 +
 +    doc_str = <<OSM
 +<osm>
 + <relation changeset='1'>
 +  <member ref='1' type='node' role='forward'/>
 +  <member ref='3' type='node' role='forward'/>
 +  <member ref='1' type='node' role='forward'/>
 +  <member ref='3' type='node' role='forward'/>
 + </relation>
 +</osm>
 +OSM
 +    doc = XML::Parser.string(doc_str).parse
 +
 +    content doc
 +    put :create
 +    assert_response :success, "can't create a relation: #{@response.body}"
 +    relation_id = @response.body.to_i
 +
 +    # get it back and check the ordering
 +    get :read, :id => relation_id
 +    assert_response :success, "can't read back the relation: #{@response.body}"
 +    check_ordering(doc, @response.body)
 +  end
 +
 +  # ============================================================
 +  # utility functions
 +  # ============================================================
 +
 +  ##
 +  # checks that the XML document and the string arguments have
 +  # members in the same order.
 +  def check_ordering(doc, xml)
 +    new_doc = XML::Parser.string(xml).parse
 +
 +    doc_members = doc.find("//osm/relation/member").collect do |m|
 +      [m['ref'].to_i, m['type'].to_sym, m['role']]
 +    end
 +
 +    new_members = new_doc.find("//osm/relation/member").collect do |m|
 +      [m['ref'].to_i, m['type'].to_sym, m['role']]
 +    end
 +
 +    doc_members.zip(new_members).each do |d, n|
 +      assert_equal d, n, "members are not equal - ordering is wrong? (#{doc}, #{xml})"
 +    end
 +  end
 +
 +  ##
 +  # create a changeset and yield to the caller to set it up, then assert
 +  # that the changeset bounding box is +bbox+.
 +  def check_changeset_modify(bbox)
 +    basic_authorization("test@openstreetmap.org", "test");  
 +
 +    # create a new changeset for this operation, so we are assured
 +    # that the bounding box will be newly-generated.
 +    changeset_id = with_controller(ChangesetController.new) do
 +      content "<osm><changeset/></osm>"
 +      put :create
 +      assert_response :success, "couldn't create changeset for modify test"
 +      @response.body.to_i
 +    end
 +
 +    # go back to the block to do the actual modifies
 +    yield changeset_id
 +
 +    # now download the changeset to check its bounding box
 +    with_controller(ChangesetController.new) do
 +      get :read, :id => changeset_id
 +      assert_response :success, "can't re-read changeset for modify test"
 +      assert_select "osm>changeset", 1
 +      assert_select "osm>changeset[id=#{changeset_id}]", 1
 +      assert_select "osm>changeset[min_lon=#{bbox[0].to_f}]", 1
 +      assert_select "osm>changeset[min_lat=#{bbox[1].to_f}]", 1
 +      assert_select "osm>changeset[max_lon=#{bbox[2].to_f}]", 1
 +      assert_select "osm>changeset[max_lat=#{bbox[3].to_f}]", 1
 +    end
 +  end
 +
 +  ##
 +  # update the changeset_id of a node element
 +  def update_changeset(xml, changeset_id)
 +    xml_attr_rewrite(xml, 'changeset', changeset_id)
 +  end
 +
 +  ##
 +  # update an attribute in the node element
 +  def xml_attr_rewrite(xml, name, value)
 +    xml.find("//osm/relation").first[name] = value.to_s
 +    return xml
 +  end
 +
 +  ##
 +  # parse some xml
 +  def xml_parse(xml)
++    parser = XML::Parser.string(xml)
 +    parser.parse
 +  end
  end