From: Tom Hughes <tom@compton.nu>
Date: Sun, 8 Mar 2009 13:02:37 +0000 (+0000)
Subject: Merge 12304:14009 from trunk.
X-Git-Tag: live~8582^2~61
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/c8ee1351049ef1bb4d7b50d071b2a96154266d1d?hp=-c

Merge 12304:14009 from trunk.
---

c8ee1351049ef1bb4d7b50d071b2a96154266d1d
diff --combined app/controllers/api_controller.rb
index 0724a3712,7a0a45fc8..cae510284
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@@ -11,13 -11,12 +11,13 @@@ class ApiController < ApplicationContro
    @@count = COUNT
  
    # The maximum area you're allowed to request, in square degrees
 -  MAX_REQUEST_AREA = 0.25
 +  MAX_REQUEST_AREA = APP_CONFIG['max_request_area']
  
    # Number of GPS trace/trackpoints returned per-page
 -  TRACEPOINTS_PER_PAGE = 5000
 +  TRACEPOINTS_PER_PAGE = APP_CONFIG['tracepoints_per_page']
  
 -  
 +  # 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
      @@count+=1
      #retrieve the page number
@@@ -55,7 -54,7 +55,7 @@@
      points = Tracepoint.find_by_area(min_lat, min_lon, max_lat, max_lon, :offset => offset, :limit => TRACEPOINTS_PER_PAGE, :order => "timestamp DESC" )
  
      doc = XML::Document.new
-     doc.encoding = 'UTF-8'
+     doc.encoding = XML::Encoding::UTF_8
      root = XML::Node.new 'gpx'
      root['version'] = '1.0'
      root['creator'] = 'OpenStreetMap.org'
@@@ -85,15 -84,6 +85,15 @@@
      render :text => doc.to_s, :content_type => "text/xml"
    end
  
 +  # This is probably the most common call of all. It is used for getting the 
 +  # OSM data for a specified bounding box, usually for editing. First the
 +  # bounding box (bbox) is checked to make sure that it is sane. All nodes 
 +  # are searched, then all the ways that reference those nodes are found.
 +  # All Nodes that are referenced by those ways are fetched and added to the list
 +  # of nodes.
 +  # Then all the relations that reference the already found nodes and ways are
 +  # fetched. All the nodes and ways that are referenced by those ways are then 
 +  # fetched. Finally all the xml is returned.
    def map
      GC.start
      @@count+=1
@@@ -119,19 -109,18 +119,19 @@@
        return
      end
  
 -    @nodes = Node.find_by_area(min_lat, min_lon, max_lat, max_lon, :conditions => "visible = 1", :limit => APP_CONFIG['max_number_of_nodes']+1)
 +    # FIXME um why is this area using a different order for the lat/lon from above???
 +    @nodes = Node.find_by_area(min_lat, min_lon, max_lat, max_lon, :conditions => {:visible => true}, :limit => APP_CONFIG['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])
  
      node_ids = @nodes.collect(&:id)
      if node_ids.length > APP_CONFIG['max_number_of_nodes']
 -      report_error("You requested too many nodes (limit is 50,000). Either request a smaller area, or use planet.osm")
 +      report_error("You requested too many nodes (limit is #{APP_CONFIG['max_number_of_nodes']}). Either request a smaller area, or use planet.osm")
        return
      end
      if node_ids.length == 0
 -      render :text => "<osm version='0.5'></osm>", :content_type => "text/xml"
 +      render :text => "<osm version='#{API_VERSION}' generator='#{GENERATOR}'></osm>", :content_type => "text/xml"
        return
      end
  
@@@ -187,15 -176,15 +187,15 @@@
        end
      end 
  
 -    relations = Relation.find_for_nodes(visible_nodes.keys, :conditions => "visible = 1") +
 -                Relation.find_for_ways(way_ids, :conditions => "visible = 1")
 +    relations = Relation.find_for_nodes(visible_nodes.keys, :conditions => {:visible => true}) +
 +                Relation.find_for_ways(way_ids, :conditions => {:visible => true})
  
      # we do not normally return the "other" partners referenced by an relation, 
      # e.g. if we return a way A that is referenced by relation X, and there's 
      # another way B also referenced, that is not returned. But we do make 
      # an exception for cases where an relation references another *relation*; 
      # in that case we return that as well (but we don't go recursive here)
 -    relations += Relation.find_for_relations(relations.collect { |r| r.id }, :conditions => "visible = 1")
 +    relations += Relation.find_for_relations(relations.collect { |r| r.id }, :conditions => {:visible => true})
  
      # this "uniq" may be slightly inefficient; it may be better to first collect and output
      # all node-related relations, then find the *not yet covered* way-related ones etc.
@@@ -215,8 -204,6 +215,8 @@@
      end
    end
  
 +  # Get a list of the tiles that have changed within a specified time
 +  # period
    def changes
      zoom = (params[:zoom] || '12').to_i
  
@@@ -230,7 -217,7 +230,7 @@@
      end
  
      if zoom >= 1 and zoom <= 16 and
 -       endtime >= starttime and endtime - starttime <= 24.hours
 +       endtime > starttime and endtime - starttime <= 24.hours
        mask = (1 << zoom) - 1
  
        tiles = Node.count(:conditions => ["timestamp BETWEEN ? AND ?", starttime, endtime],
@@@ -258,32 -245,21 +258,32 @@@
  
        render :text => doc.to_s, :content_type => "text/xml"
      else
 -      render :nothing => true, :status => :bad_request
 +      render :text => "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", :status => :bad_request
      end
    end
  
 +  # External apps that use the api are able to query the api to find out some 
 +  # parameters of the API. It currently returns: 
 +  # * minimum and maximum API versions that can be used.
 +  # * maximum area that can be requested in a bbox request in square degrees
 +  # * number of tracepoints that are returned in each tracepoints page
    def capabilities
      doc = OSM::API.new.get_xml_doc
  
      api = XML::Node.new 'api'
      version = XML::Node.new 'version'
 -    version['minimum'] = '0.5';
 -    version['maximum'] = '0.5';
 +    version['minimum'] = "#{API_VERSION}";
 +    version['maximum'] = "#{API_VERSION}";
      api << version
      area = XML::Node.new 'area'
      area['maximum'] = MAX_REQUEST_AREA.to_s;
      api << area
 +    tracepoints = XML::Node.new 'tracepoints'
 +    tracepoints['per_page'] = APP_CONFIG['tracepoints_per_page'].to_s
 +    api << tracepoints
 +    waynodes = XML::Node.new 'waynodes'
 +    waynodes['maximum'] = APP_CONFIG['max_number_of_way_nodes'].to_s
 +    api << waynodes
      
      doc.root << api
  
diff --combined app/controllers/user_controller.rb
index 7ebe6b6b6,6a60917f2..47c1cff9f
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@@ -11,19 -11,24 +11,24 @@@ class UserController < ApplicationContr
  
    def save
      @title = 'create account'
-     @user = User.new(params[:user])
  
-     @user.visible = true
-     @user.data_public = true
-     @user.description = "" if @user.description.nil?
-     @user.creation_ip = request.remote_ip
- 
-     if @user.save
-       flash[:notice] = "User was successfully created. Check your email for a confirmation note, and you\'ll be mapping in no time :-)<br>Please note that you won't be able to login until you've received and confirmed your email address."
-       Notifier.deliver_signup_confirm(@user, @user.tokens.create)
-       redirect_to :action => 'login'
-     else
+     if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"})
        render :action => 'new'
+     else
+       @user = User.new(params[:user])
+ 
+       @user.visible = true
+       @user.data_public = true
+       @user.description = "" if @user.description.nil?
+       @user.creation_ip = request.remote_ip
+ 
+       if @user.save
+         flash[:notice] = "User was successfully created. Check your email for a confirmation note, and you\'ll be mapping in no time :-)<br>Please note that you won't be able to login until you've received and confirmed your email address."
+         Notifier.deliver_signup_confirm(@user, @user.tokens.create)
+         redirect_to :action => 'login'
+       else
+         render :action => 'new'
+       end
      end
    end
  
@@@ -77,7 -82,7 +82,7 @@@
    def lost_password
      @title = 'lost password'
      if params[:user] and params[:user][:email]
 -      user = User.find_by_email(params[:user][:email], :conditions => "visible = 1")
 +      user = User.find_by_email(params[:user][:email], :conditions => {:visible => true})
  
        if user
          token = user.tokens.create
@@@ -117,15 -122,6 +122,15 @@@
    end
  
    def login
 +    if session[:user]
 +      # The user is logged in already, if the referer param exists, redirect them to that
 +      if params[:referer]
 +        redirect_to params[:referer]
 +      else
 +        redirect_to :controller => 'site', :action => 'index'
 +      end
 +      return
 +    end
      @title = 'login'
      if params[:user]
        email_or_display_name = params[:user][:email]
@@@ -226,7 -222,7 +231,7 @@@
    end
  
    def view
 -    @this_user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1")
 +    @this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
  
      if @this_user
        @title = @this_user.display_name
@@@ -239,7 -235,7 +244,7 @@@
    def make_friend
      if params[:display_name]     
        name = params[:display_name]
 -      new_friend = User.find_by_display_name(name, :conditions => "visible = 1")
 +      new_friend = User.find_by_display_name(name, :conditions => {:visible => true})
        friend = Friend.new
        friend.user_id = @user.id
        friend.friend_user_id = new_friend.id
@@@ -261,7 -257,7 +266,7 @@@
    def remove_friend
      if params[:display_name]     
        name = params[:display_name]
 -      friend = User.find_by_display_name(name, :conditions => "visible = 1")
 +      friend = User.find_by_display_name(name, :conditions => {:visible => true})
        if @user.is_friends_with?(friend)
          Friend.delete_all "user_id = #{@user.id} AND friend_user_id = #{friend.id}"
          flash[:notice] = "#{friend.display_name} was removed from your friends."
diff --combined app/controllers/user_preference_controller.rb
index 3b56c257b,3a48ee65e..68ea88eea
--- a/app/controllers/user_preference_controller.rb
+++ b/app/controllers/user_preference_controller.rb
@@@ -5,9 -5,11 +5,9 @@@ class UserPreferenceController < Applic
    def read_one
      pref = UserPreference.find(@user.id, params[:preference_key])
  
 -    if pref
 -      render :text => pref.v.to_s
 -    else
 -      render :text => 'OH NOES! PREF NOT FOUND!', :status => 404
 -    end
 +    render :text => pref.v.to_s
 +  rescue ActiveRecord::RecordNotFound => ex
 +    render :text => 'OH NOES! PREF NOT FOUND!', :status => :not_found
    end
  
    def update_one
@@@ -30,8 -32,6 +30,8 @@@
      UserPreference.delete(@user.id, params[:preference_key])
  
      render :nothing => true
 +  rescue ActiveRecord::RecordNotFound => ex
 +    render :text => "param: #{params[:preference_key]} not found", :status => :not_found
    end
  
    # print out all the preferences as a big xml block
@@@ -52,43 -52,48 +52,42 @@@
  
    # 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
diff --combined app/models/changeset.rb
index 31d927e9a,000000000..d420f537a
mode 100644,000000..100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@@ -1,238 -1,0 +1,237 @@@
 +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.new
-       p.string = xml
++      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
diff --combined app/models/node.rb
index f2ad3a78a,af88a117d..d3e0a7e8d
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@@ -2,34 -2,27 +2,34 @@@ class Node < ActiveRecord::Bas
    require 'xml/libxml'
  
    include GeoRecord
 +  include ConsistencyValidations
  
    set_table_name 'current_nodes'
 -  
 -  validates_presence_of :user_id, :timestamp
 -  validates_inclusion_of :visible, :in => [ true, false ]
 -  validates_numericality_of :latitude, :longitude
 -  validate :validate_position
  
 -  belongs_to :user
 +  belongs_to :changeset
  
    has_many :old_nodes, :foreign_key => :id
  
    has_many :way_nodes
    has_many :ways, :through => :way_nodes
  
 +  has_many :node_tags, :foreign_key => :id
 +  
    has_many :old_way_nodes
    has_many :ways_via_history, :class_name=> "Way", :through => :old_way_nodes, :source => :way
  
    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 :latitude, :longitude, :changeset_id, :version, :integer_only => true
 +  validates_numericality_of :id, :on => :update, :integer_only => true
 +  validate :validate_position
 +  validates_associated :changeset
 +
    # Sanity check the latitude and longitude and add an error if it's broken
    def validate_position
      errors.add_to_base("Node is not in the world") unless in_world?
@@@ -57,158 -50,92 +57,157 @@@
      #conditions = keys.join(' AND ')
   
      find_by_area(min_lat, min_lon, max_lat, max_lon,
 -                    :conditions => 'visible = 1',
 +                    :conditions => {:visible => true},
                      :limit => APP_CONFIG['max_number_of_nodes']+1)  
    end
  
    # 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
    end
  
 -  # Save this node with the appropriate OldNode object to represent it's history.
 -  def save_with_history!
 +  ##
 +  # the bounding box around a node, which is used for determining the changeset's
 +  # bounding box
 +  def bbox
 +    [ longitude, latitude, longitude, latitude ]
 +  end
 +
 +  # Should probably be renamed delete_from to come in line with update
 +  def delete_with_history!(new_node, user)
 +    unless self.visible
 +      raise OSM::APIAlreadyDeletedError.new
 +    end
 +
 +    # need to start the transaction here, so that the database can 
 +    # provide repeatable reads for the used-by checks. this means it
 +    # shouldn't be possible to get race conditions.
      Node.transaction do
 -      self.timestamp = Time.now
 -      self.save!
 -      old_node = OldNode.from_node(self)
 -      old_node.save!
 +      check_consistency(self, new_node, user)
 +      if WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", :conditions => [ "current_ways.visible = ? AND current_way_nodes.node_id = ?", true, self.id ])
 +        raise OSM::APIPreconditionFailedError.new
 +      elsif RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = ? AND member_type='node' and member_id=? ", true, self.id])
 +        raise OSM::APIPreconditionFailedError.new
 +      else
 +        self.changeset_id = new_node.changeset_id
 +        self.visible = false
 +        
 +        # update the changeset with the deleted position
 +        changeset.update_bbox!(bbox)
 +        
 +        save_with_history!
 +      end
      end
    end
  
 -  # Turn this Node in to a complete OSM XML object with <osm> wrapper
 +  def update_from(new_node, user)
 +    check_consistency(self, new_node, user)
 +
 +    # update changeset with *old* position first
 +    changeset.update_bbox!(bbox);
 +
 +    # FIXME logic needs to be double checked
 +    self.changeset_id = new_node.changeset_id
 +    self.latitude = new_node.latitude 
 +    self.longitude = new_node.longitude
 +    self.tags = new_node.tags
 +    self.visible = true
 +
 +    # update changeset with *new* position
 +    changeset.update_bbox!(bbox);
 +
 +    save_with_history!
 +  end
 +  
 +  def create_with_history(user)
 +    check_create_consistency(self, user)
 +    self.id = nil
 +    self.version = 0
 +    self.visible = true
 +
 +    # update the changeset to include the new location
 +    changeset.update_bbox!(bbox)
 +
 +    save_with_history!
 +  end
 +
    def to_xml
      doc = OSM::API.new.get_xml_doc
      doc.root << to_xml_node()
      return doc
    end
  
 -  # Turn this Node in to an XML Node without the <osm> wrapper.
    def to_xml_node(user_display_name_cache = nil)
      el1 = XML::Node.new 'node'
      el1['id'] = self.id.to_s
      el1['lat'] = self.lat.to_s
      el1['lon'] = self.lon.to_s
 -
 +    el1['version'] = self.version.to_s
 +    el1['changeset'] = self.changeset_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)
 +    if user_display_name_cache and user_display_name_cache.key?(self.changeset.user_id)
        # use the cache if available
 -    elsif self.user.data_public?
 -      user_display_name_cache[self.user_id] = self.user.display_name
 +    elsif self.changeset.user.data_public?
 +      user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name
      else
 -      user_display_name_cache[self.user_id] = nil
 +      user_display_name_cache[self.changeset.user_id] = nil
      end
  
 -    el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil?
 +    if not user_display_name_cache[self.changeset.user_id].nil?
 +      el1['user'] = user_display_name_cache[self.changeset.user_id]
 +      el1['uid'] = self.changeset.user_id.to_s
 +    end
  
 -    Tags.split(self.tags) do |k,v|
 +    self.tags.each do |k,v|
        el2 = XML::Node.new('tag')
        el2['k'] = k.to_s
        el2['v'] = v.to_s
@@@ -220,79 -147,12 +219,79 @@@
      return el1
    end
  
 -  # Return the node's tags as a Hash of keys and their values
    def tags_as_hash
 -    hash = {}
 -    Tags.split(self.tags) do |k,v|
 -      hash[k] = v
 +    return tags
 +  end
 +
 +  def tags
 +    unless @tags
 +      @tags = {}
 +      self.node_tags.each do |tag|
 +        @tags[tag.k] = tag.v
 +      end
 +    end
 +    @tags
 +  end
 +
 +  def tags=(t)
 +    @tags = t 
 +  end 
 +
 +  def add_tag_key_val(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("node", self.id, k) if @tags.include? k
 +
 +    @tags[k] = v
 +  end
 +
 +  ##
 +  # are the preconditions OK? this is mainly here to keep the duck
 +  # typing interface the same between nodes, ways and relations.
 +  def preconditions_ok?
 +    in_world?
 +  end
 +
 +  ##
 +  # dummy method to make the interfaces of node, way and relation
 +  # more consistent.
 +  def fix_placeholders!(id_map)
 +    # nodes don't refer to anything, so there is nothing to do here
 +  end
 +  
 +  private
 +
 +  def save_with_history!
 +    t = Time.now
 +    Node.transaction do
 +      self.version += 1
 +      self.timestamp = t
 +      self.save!
 +
 +      # Create a NodeTag
 +      tags = self.tags
 +      NodeTag.delete_all(['id = ?', self.id])
 +      tags.each do |k,v|
 +        tag = NodeTag.new
 +        tag.k = k 
 +        tag.v = v 
 +        tag.id = self.id
 +        tag.save!
 +      end 
 +
 +      # Create an OldNode
 +      old_node = OldNode.from_node(self)
 +      old_node.timestamp = t
 +      old_node.save_with_dependencies!
 +
 +      # tell the changeset we updated one element only
 +      changeset.add_changes! 1
 +
 +      # save the changeset in case of bounding box updates
 +      changeset.save!
      end
 -    hash
    end
 +  
  end
diff --combined app/models/relation.rb
index 6be106159,d9dba303f..64af4ecc1
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@@ -1,84 -1,51 +1,83 @@@
  class Relation < ActiveRecord::Base
    require 'xml/libxml'
    
 +  include ConsistencyValidations
 +  
    set_table_name 'current_relations'
  
 -  belongs_to :user
 +  belongs_to :changeset
  
    has_many :old_relations, :foreign_key => 'id', :order => 'version'
  
 -  has_many :relation_members, :foreign_key => 'id'
 +  has_many :relation_members, :foreign_key => 'id', :order => 'sequence_id'
    has_many :relation_tags, :foreign_key => 'id'
  
    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
  
@@@ -93,23 -60,18 +92,23 @@@
      el1['id'] = self.id.to_s
      el1['visible'] = self.visible.to_s
      el1['timestamp'] = self.timestamp.xmlschema
 +    el1['version'] = self.version.to_s
 +    el1['changeset'] = self.changeset_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)
 +    if user_display_name_cache and user_display_name_cache.key?(self.changeset.user_id)
        # use the cache if available
 -    elsif self.user.data_public?
 -      user_display_name_cache[self.user_id] = self.user.display_name
 +    elsif self.changeset.user.data_public?
 +      user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name
      else
 -      user_display_name_cache[self.user_id] = nil
 +      user_display_name_cache[self.changeset.user_id] = nil
      end
  
 -    el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil?
 +    if not user_display_name_cache[self.changeset.user_id].nil?
 +      el1['user'] = user_display_name_cache[self.changeset.user_id]
 +      el1['uid'] = self.changeset.user_id.to_s
 +    end
  
      self.relation_members.each do |member|
        p=0
@@@ -208,164 -170,19 +207,164 @@@
  
    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("relation", self.id, k) if @tags.include? k
 +
      @tags[k] = v
    end
  
 +  ##
 +  # updates the changeset bounding box to contain the bounding box of 
 +  # the element with given +type+ and +id+. this only works with nodes
 +  # and ways at the moment, as they're the only elements to respond to
 +  # the :bbox call.
 +  def update_changeset_element(type, id)
 +    element = Kernel.const_get(type.capitalize).find(id)
 +    changeset.update_bbox! element.bbox
 +  end    
 +
 +  def delete_with_history!(new_relation, user)
 +    unless self.visible
 +      raise OSM::APIAlreadyDeletedError.new
 +    end
 +
 +    # need to start the transaction here, so that the database can 
 +    # provide repeatable reads for the used-by checks. this means it
 +    # shouldn't be possible to get race conditions.
 +    Relation.transaction do
 +      check_consistency(self, new_relation, user)
 +      # This will check to see if this relation is used by another relation
 +      if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = ? AND member_type='relation' and member_id=? ", true, self.id ])
 +        raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is a used in another relation")
 +      end
 +      self.changeset_id = new_relation.changeset_id
 +      self.tags = {}
 +      self.members = []
 +      self.visible = false
 +      save_with_history!
 +    end
 +  end
 +
 +  def update_from(new_relation, user)
 +    check_consistency(self, new_relation, user)
 +    if !new_relation.preconditions_ok?
 +      raise OSM::APIPreconditionFailedError.new
 +    end
 +    self.changeset_id = new_relation.changeset_id
 +    self.tags = new_relation.tags
 +    self.members = new_relation.members
 +    self.visible = true
 +    save_with_history!
 +  end
 +  
 +  def create_with_history(user)
 +    check_create_consistency(self, user)
 +    if !self.preconditions_ok?
 +      raise OSM::APIPreconditionFailedError.new
 +    end
 +    self.version = 0
 +    self.visible = true
 +    save_with_history!
 +  end
 +
 +  def preconditions_ok?
 +    # These are hastables that store an id in the index of all 
 +    # the nodes/way/relations that have already been added.
 +    # If the member is valid and visible then we add it to the 
 +    # relevant hash table, with the value true as a cache.
 +    # Thus if you have nodes with the ids of 50 and 1 already in the
 +    # relation, then the hash table nodes would contain:
 +    # => {50=>true, 1=>true}
 +    elements = { :node => Hash.new, :way => Hash.new, :relation => Hash.new }
 +    self.members.each do |m|
 +      # find the hash for the element type or die
 +      hash = elements[m[0].to_sym] or return false
 +
 +      # unless its in the cache already
 +      unless hash.key? m[1]
 +        # use reflection to look up the appropriate class
 +        model = Kernel.const_get(m[0].capitalize)
 +
 +        # get the element with that ID
 +        element = model.find(m[1])
 +
 +        # and check that it is OK to use.
 +        unless element and element.visible? and element.preconditions_ok?
 +          return false
 +        end
 +        hash[m[1]] = true
 +      end
 +    end
 +
 +    return true
 +  rescue
 +    return false
 +  end
 +
 +  # Temporary method to match interface to nodes
 +  def tags_as_hash
 +    return self.tags
 +  end
 +
 +  ##
 +  # if any members are referenced by placeholder IDs (i.e: negative) then
 +  # this calling this method will fix them using the map from placeholders 
 +  # to IDs +id_map+. 
 +  def fix_placeholders!(id_map)
 +    self.members.map! do |type, id, role|
 +      old_id = id.to_i
 +      if old_id < 0
 +        new_id = id_map[type.to_sym][old_id]
 +        raise "invalid placeholder" if new_id.nil?
 +        [type, new_id, role]
 +      else
 +        [type, id, role]
 +      end
 +    end
 +  end
 +
 +  private
 +  
    def save_with_history!
      Relation.transaction do
 +      # have to be a little bit clever here - to detect if any tags
 +      # changed then we have to monitor their before and after state.
 +      tags_changed = false
 +
        t = Time.now
 +      self.version += 1
        self.timestamp = t
        self.save!
  
        tags = self.tags
 +      self.relation_tags.each do |old_tag|
 +        key = old_tag.k
 +        # if we can match the tags we currently have to the list
 +        # of old tags, then we never set the tags_changed flag. but
 +        # if any are different then set the flag and do the DB 
 +        # update.
 +        if tags.has_key? key 
 +          # rails 2.1 dirty handling should take care of making this
 +          # somewhat efficient... hopefully...
 +          old_tag.v = tags[key]
 +          tags_changed |= old_tag.changed?
 +          old_tag.save!
 +
 +          # remove from the map, so that we can expect an empty map
 +          # at the end if there are no new tags
 +          tags.delete key
  
 -      RelationTag.delete_all(['id = ?', self.id])
 -
 +        else
 +          # this means a tag was deleted
 +          tags_changed = true
 +          RelationTag.delete_all ['id = ? and k = ?', self.id, old_tag.k]
 +        end
 +      end
 +      # if there are left-over tags then they are new and will have to
 +      # be added.
 +      tags_changed |= (not tags.empty?)
        tags.each do |k,v|
          tag = RelationTag.new
          tag.k = k
@@@ -374,80 -191,80 +373,80 @@@
          tag.save!
        end
  
 -      members = self.members
 -
 -      RelationMember.delete_all(['id = ?', self.id])
 +      # same pattern as before, but this time we're collecting the
 +      # changed members in an array, as the bounding box updates for
 +      # elements are per-element, not blanked on/off like for tags.
 +      changed_members = Array.new
 +      members = Hash.new
 +      self.members.each do |m|
 +        # should be: h[[m.id, m.type]] = m.role, but someone prefers arrays
 +        members[[m[1], m[0]]] = m[2]
 +      end
 +      relation_members.each do |old_member|
 +        key = [old_member.member_id.to_s, old_member.member_type]
 +        if members.has_key? key
 +          members.delete key
 +        else
 +          changed_members << key
 +        end
 +      end
 +      # any remaining members must be new additions
 +      changed_members += members.keys
  
 -      members.each do |n|
 +      # update the members. first delete all the old members, as the new
 +      # members may be in a different order and i don't feel like implementing
 +      # a longest common subsequence algorithm to optimise this.
 +      members = self.members
 +      RelationMember.delete_all(:id => self.id)
 +      members.each_with_index do |m,i|
          mem = RelationMember.new
 -        mem.id = self.id
 -        mem.member_type = n[0];
 -        mem.member_id = n[1];
 -        mem.member_role = n[2];
 +        mem.id = [self.id, i]
 +        mem.member_type = m[0]
 +        mem.member_id = m[1]
 +        mem.member_role = m[2]
          mem.save!
        end
  
        old_relation = OldRelation.from_relation(self)
        old_relation.timestamp = t
        old_relation.save_with_dependencies!
 -    end
 -  end
  
 -  def preconditions_ok?
 -    # These are hastables that store an id in the index of all 
 -    # the nodes/way/relations that have already been added.
 -    # Once we know the id of the node/way/relation exists
 -    # we check to see if it is already existing in the hashtable
 -    # if it does, then we return false. Otherwise
 -    # we add it to the relevant hash table, with the value true..
 -    # Thus if you have nodes with the ids of 50 and 1 already in the
 -    # relation, then the hash table nodes would contain:
 -    # => {50=>true, 1=>true}
 -    nodes = Hash.new
 -    ways = Hash.new
 -    relations = Hash.new
 -    self.members.each do |m|
 -      if (m[0] == "node")
 -        n = Node.find(:first, :conditions => ["id = ?", m[1]])
 -        unless n and n.visible 
 -          return false
 -        end
 -        if nodes[m[1]]
 -          return false
 -        else
 -          nodes[m[1]] = true
 -        end
 -      elsif (m[0] == "way")
 -        w = Way.find(:first, :conditions => ["id = ?", m[1]])
 -        unless w and w.visible and w.preconditions_ok?
 -          return false
 -        end
 -        if ways[m[1]]
 -          return false
 -        else
 -          ways[m[1]] = true
 -        end
 -      elsif (m[0] == "relation")
 -        e = Relation.find(:first, :conditions => ["id = ?", m[1]])
 -        unless e and e.visible and e.preconditions_ok?
 -          return false
 -        end
 -        if relations[m[1]]
 -          return false
 -        else
 -          relations[m[1]] = true
 +      # update the bbox of the changeset and save it too.
 +      # discussion on the mailing list gave the following definition for
 +      # the bounding box update procedure of a relation:
 +      #
 +      # adding or removing nodes or ways from a relation causes them to be
 +      # added to the changeset bounding box. adding a relation member or
 +      # changing tag values causes all node and way members to be added to the
 +      # bounding box. this is similar to how the map call does things and is
 +      # reasonable on the assumption that adding or removing members doesn't
 +      # materially change the rest of the relation.
 +      any_relations = 
 +        changed_members.collect { |id,type| type == "relation" }.
 +        inject(false) { |b,s| b or s }
 +
 +      if tags_changed or any_relations
 +        # add all non-relation bounding boxes to the changeset
 +        # FIXME: check for tag changes along with element deletions and
 +        # make sure that the deleted element's bounding box is hit.
 +        self.members.each do |type, id, role|
 +          if type != "relation"
 +            update_changeset_element(type, id)
 +          end
          end
        else
 -        return false
 +        # add only changed members to the changeset
 +        changed_members.each do |id, type|
 +          update_changeset_element(type, id)
 +        end
        end
 +
 +      # tell the changeset we updated one element only
 +      changeset.add_changes! 1
 +
 +      # save the (maybe updated) changeset bounding box
 +      changeset.save!
      end
 -    return true
 -  rescue
 -    return false
    end
  
 -  # Temporary method to match interface to nodes
 -  def tags_as_hash
 -    return self.tags
 -  end
  end
diff --combined app/models/way.rb
index 86b25e08e,6c3ea9e46..dbc1197a9
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@@ -1,11 -1,9 +1,11 @@@
  class Way < ActiveRecord::Base
    require 'xml/libxml'
 +  
 +  include ConsistencyValidations
  
    set_table_name 'current_ways'
 -
 -  belongs_to :user
 +  
 +  belongs_to :changeset
  
    has_many :old_ways, :foreign_key => 'id', :order => 'version'
  
@@@ -17,57 -15,37 +17,56 @@@
    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
@@@ -95,23 -73,18 +94,23 @@@
      el1['id'] = self.id.to_s
      el1['visible'] = self.visible.to_s
      el1['timestamp'] = self.timestamp.xmlschema
 +    el1['version'] = self.version.to_s
 +    el1['changeset'] = self.changeset_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)
 +    if user_display_name_cache and user_display_name_cache.key?(self.changeset.user_id)
        # use the cache if available
 -    elsif self.user.data_public?
 -      user_display_name_cache[self.user_id] = self.user.display_name
 +    elsif self.changeset.user.data_public?
 +      user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name
      else
 -      user_display_name_cache[self.user_id] = nil
 +      user_display_name_cache[self.changeset.user_id] = nil
      end
  
 -    el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil?
 +    if not user_display_name_cache[self.changeset.user_id].nil?
 +      el1['user'] = user_display_name_cache[self.changeset.user_id]
 +      el1['uid'] = self.changeset.user_id.to_s
 +    end
  
      # make sure nodes are output in sequence_id order
      ordered_nodes = []
@@@ -123,7 -96,7 +122,7 @@@
          end
        else
          # otherwise, manually go to the db to check things
 -        if nd.node.visible? and nd.node.visible?
 +        if nd.node and nd.node.visible?
            ordered_nodes[nd.sequence_id] = nd.node_id.to_s
          end
        end
@@@ -181,82 -154,99 +180,82 @@@
  
    def add_tag_keyval(k, v)
      @tags = Hash.new unless @tags
 -    @tags[k] = v
 -  end
  
 -  def save_with_history!
 -    t = Time.now
 +    # duplicate tags are now forbidden, so we can't allow values
 +    # in the hash to be overwritten.
 +    raise OSM::APIDuplicateTagsError.new("way", self.id, k) if @tags.include? k
  
 -    Way.transaction do
 -      self.timestamp = t
 -      self.save!
 -    end
 -
 -    WayTag.transaction do
 -      tags = self.tags
 +    @tags[k] = v
 +  end
  
 -      WayTag.delete_all(['id = ?', self.id])
 +  ##
 +  # the integer coords (i.e: unscaled) bounding box of the way, assuming
 +  # straight line segments.
 +  def bbox
 +    lons = nodes.collect { |n| n.longitude }
 +    lats = nodes.collect { |n| n.latitude }
 +    [ lons.min, lats.min, lons.max, lats.max ]
 +  end
  
 -      tags.each do |k,v|
 -        tag = WayTag.new
 -        tag.k = k
 -        tag.v = v
 -        tag.id = self.id
 -        tag.save!
 -      end
 +  def update_from(new_way, user)
 +    check_consistency(self, new_way, user)
 +    if !new_way.preconditions_ok?
 +      raise OSM::APIPreconditionFailedError.new
      end
 +    self.changeset_id = new_way.changeset_id
 +    self.tags = new_way.tags
 +    self.nds = new_way.nds
 +    self.visible = true
 +    save_with_history!
 +  end
  
 -    WayNode.transaction do
 -      nds = self.nds
 -
 -      WayNode.delete_all(['id = ?', self.id])
 -
 -      sequence = 1
 -      nds.each do |n|
 -        nd = WayNode.new
 -        nd.id = [self.id, sequence]
 -        nd.node_id = n
 -        nd.save!
 -        sequence += 1
 -      end
 +  def create_with_history(user)
 +    check_create_consistency(self, user)
 +    if !self.preconditions_ok?
 +      raise OSM::APIPreconditionFailedError.new
      end
 -
 -    old_way = OldWay.from_way(self)
 -    old_way.timestamp = t
 -    old_way.save_with_dependencies!
 +    self.version = 0
 +    self.visible = true
 +    save_with_history!
    end
  
    def preconditions_ok?
      return false if self.nds.empty?
 +    if self.nds.length > APP_CONFIG['max_number_of_way_nodes']
 +      raise OSM::APITooManyWayNodesError.new(self.nds.count, APP_CONFIG['max_number_of_way_nodes'])
 +    end
      self.nds.each do |n|
        node = Node.find(:first, :conditions => ["id = ?", n])
        unless node and node.visible
 -        return false
 +        raise OSM::APIPreconditionFailedError.new("The node with id #{n} either does not exist, or is not visible")
        end
      end
      return true
    end
  
 -  # Delete the way and it's relations, but don't really delete it - set its visibility to false and update the history etc to maintain wiki-like functionality.
 -  def delete_with_relations_and_history(user)
 -    if self.visible
 -	  # FIXME
 -	  # this should actually delete the relations,
 -	  # not just throw a PreconditionFailed if it's a member of a relation!!
 +  def delete_with_history!(new_way, user)
 +    unless self.visible
 +      raise OSM::APIAlreadyDeletedError
 +    end
 +    
 +    # need to start the transaction here, so that the database can 
 +    # provide repeatable reads for the used-by checks. this means it
 +    # shouldn't be possible to get race conditions.
 +    Way.transaction do
 +      check_consistency(self, new_way, user)
        if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id",
 -                             :conditions => [ "visible = 1 AND member_type='way' and member_id=?", self.id])
 -        raise OSM::APIPreconditionFailedError
 -      # end FIXME
 +                             :conditions => [ "visible = ? AND member_type='way' and member_id=? ", true, self.id])
 +        raise OSM::APIPreconditionFailedError.new("You need to make sure that this way is not a member of a relation.")
        else
 -        self.user_id = user.id
 +        self.changeset_id = new_way.changeset_id
          self.tags = []
          self.nds = []
          self.visible = false
 -        self.save_with_history!
 +        save_with_history!
        end
 -    else
 -      raise OSM::APIAlreadyDeletedError
      end
    end
  
 -  # delete a way and it's nodes that aren't part of other ways, with history
 -  def delete_with_relations_and_nodes_and_history(user)
 -    # delete the nodes not used by other ways
 -    self.unshared_node_ids.each do |node_id|
 -      n = Node.find(node_id)
 -      n.user_id = user.id
 -      n.visible = false
 -      n.save_with_history!
 -    end
 -    
 -    self.user_id = user.id
 -
 -    self.delete_with_relations_and_history(user)
 -  end
 -
    # Find nodes that belong to this way only
    def unshared_node_ids
      node_ids = self.nodes.collect { |node| node.id }
@@@ -273,73 -263,4 +272,73 @@@
    def tags_as_hash
      return self.tags
    end
 +
 +  ##
 +  # if any referenced nodes are placeholder IDs (i.e: are negative) then
 +  # this calling this method will fix them using the map from placeholders 
 +  # to IDs +id_map+. 
 +  def fix_placeholders!(id_map)
 +    self.nds.map! do |node_id|
 +      if node_id < 0
 +        new_id = id_map[:node][node_id]
 +        raise "invalid placeholder for #{node_id.inspect}: #{new_id.inspect}" if new_id.nil?
 +        new_id
 +      else
 +        node_id
 +      end
 +    end
 +  end
 +
 +  private
 +  
 +  def save_with_history!
 +    t = Time.now
 +
 +    # update the bounding box, but don't save it as the controller knows the 
 +    # lifetime of the change better. note that this has to be done both before 
 +    # and after the save, so that nodes from both versions are included in the 
 +    # bbox.
 +    changeset.update_bbox!(bbox) unless nodes.empty?
 +
 +    Way.transaction do
 +      self.version += 1
 +      self.timestamp = t
 +      self.save!
 +
 +      tags = self.tags
 +      WayTag.delete_all(['id = ?', self.id])
 +      tags.each do |k,v|
 +        tag = WayTag.new
 +        tag.k = k
 +        tag.v = v
 +        tag.id = self.id
 +        tag.save!
 +      end
 +
 +      nds = self.nds
 +      WayNode.delete_all(['id = ?', self.id])
 +      sequence = 1
 +      nds.each do |n|
 +        nd = WayNode.new
 +        nd.id = [self.id, sequence]
 +        nd.node_id = n
 +        nd.save!
 +        sequence += 1
 +      end
 +
 +      old_way = OldWay.from_way(self)
 +      old_way.timestamp = t
 +      old_way.save_with_dependencies!
 +
 +      # update and commit the bounding box, now that way nodes 
 +      # have been updated and we're in a transaction.
 +      changeset.update_bbox!(bbox) unless nodes.empty?
 +
 +      # tell the changeset we updated one element only
 +      changeset.add_changes! 1
 +
 +      changeset.save!
 +    end
 +  end
 +
  end
diff --combined app/views/diary_entry/list.rhtml
index 9852313bb,3d1f0bbcb..38157b183
--- a/app/views/diary_entry/list.rhtml
+++ b/app/views/diary_entry/list.rhtml
@@@ -4,40 -4,32 +4,36 @@@
   <%= image_tag url_for_file_column(@this_user, "image") %>
  <% end %>
  
 -<br />
  
  <% if @this_user %>
    <% if @user == @this_user %>
 -    <%= link_to 'New diary entry', :controller => 'diary_entry', :action => 'new', :display_name => @user.display_name %>
 +    <%= link_to image_tag("new.png", :border=>0) + 'New diary entry', {:controller => 'diary_entry', :action => 'new', :display_name => @user.display_name}, {:title => 'Compose a new entry in your user diary'} %>
    <% end %>
  <% else %>
    <% if @user %>
 -    <%= link_to 'New diary entry', :controller => 'diary_entry', :action => 'new', :display_name => @user.display_name %>
 +    <%= link_to image_tag("new.png", :border=>0) + 'New diary entry', {:controller => 'diary_entry', :action => 'new', :display_name => @user.display_name}, {:title => 'Compose a new entry in your user diary'} %>
    <% 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 %>
diff --combined app/views/layouts/site.rhtml
index cf7ad9fc8,34d2ecb30..cc8f7192d
--- a/app/views/layouts/site.rhtml
+++ b/app/views/layouts/site.rhtml
@@@ -1,5 -1,5 +1,5 @@@
  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
- <html>
+ <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
    <head>
      <%= javascript_include_tag 'prototype' %>
      <%= javascript_include_tag 'site' %>
@@@ -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>
@@@ -69,11 -69,19 +70,19 @@@
  
        <% unless @user %>
        <div id="intro">
-         OpenStreetMap is a free editable map of the whole world. It is made by people like you.
-         <p/>
-         OpenStreetMap allows you to view, edit and use geographical data in a collaborative way from anywhere on Earth.
-         <p/>
-         OpenStreetMap's hosting is kindly supported by the <a href="http://www.vr.ucl.ac.uk">UCL VR Centre</a> and <a href="http://www.bytemark.co.uk">bytemark</a>.
+         <p>
+           OpenStreetMap is a free editable map of the whole world. It
+           is made by people like you.
+         </p>
+         <p>
+           OpenStreetMap allows you to view, edit and use geographical
+           data in a collaborative way from anywhere on Earth.
+         </p>
+         <p>
+           OpenStreetMap's hosting is kindly supported by the
+           <a href="http://www.vr.ucl.ac.uk">UCL VR Centre</a> and
+           <a href="http://www.bytemark.co.uk">bytemark</a>.
+         </p>
        </div>
        <% end %>
  
@@@ -89,6 -97,14 +98,14 @@@
        </div>
        <% end %>
  
+       <% if false %>
+       <div id="donate">
+         Support OpenStreetMap by
+         <a href="http://donate.openstreetmap.org/">donating</a>
+         to the Hardware Upgrade Fund.
+       </div>
+       <% end %>
+ 
        <div id="left_menu" class="left_menu">
          <a href="http://wiki.openstreetmap.org">Help &amp; Wiki</a><br />
          <a href="http://www.opengeodata.org/">News blog</a><br />
@@@ -97,21 -113,16 +114,16 @@@
        </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>
  
+         <div id="cclogo" class="button" style="width: 88px">
 -          <a href="http://creativecommons.org/licenses/by-sa/2.0/"><img src="/images/cc_button.png" border="0" alt="" /></a>
++          <%= 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>
diff --combined app/views/user/login.rhtml
index 770ad8873,ff988f070..3c40906ca
--- a/app/views/user/login.rhtml
+++ b/app/views/user/login.rhtml
@@@ -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 %>
diff --combined app/views/user/new.rhtml
index 1b7f6e9b4,d0b5a9667..823dccf52
--- a/app/views/user/new.rhtml
+++ b/app/views/user/new.rhtml
@@@ -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 %>
diff --combined config/environment.rb
index ffed548bd,e6af619eb..b3481c024
--- a/config/environment.rb
+++ b/config/environment.rb
@@@ -5,16 -5,13 +5,16 @@@
  ENV['RAILS_ENV'] ||= 'production'
  
  # Specifies gem version of Rails to use when vendor/rails is not present
 -RAILS_GEM_VERSION = '2.0.2' unless defined? RAILS_GEM_VERSION
 +RAILS_GEM_VERSION = '2.1.2' unless defined? RAILS_GEM_VERSION
  
  # Set the server URL
  SERVER_URL = ENV['OSM_SERVER_URL'] || 'www.openstreetmap.org'
  
 +# Set the generator
 +GENERATOR = ENV['OSM_SERVER_GENERATOR'] || 'OpenStreetMap server'
 +
  # Application constants needed for routes.rb - must go before Initializer call
 -API_VERSION = ENV['OSM_API_VERSION'] || '0.5'
 +API_VERSION = ENV['OSM_API_VERSION'] || '0.6'
  
  # Set application status - possible settings are:
  #
@@@ -40,16 -37,6 +40,16 @@@ Rails::Initializer.run do |config
      config.frameworks -= [ :active_record ]
    end
  
 +  # 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 => '0.9.4', :lib => 'libxml'
++  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
@@@ -76,12 -63,6 +76,12 @@@
    # (create the session table with 'rake db:sessions:create')
    config.action_controller.session_store = :sql_session_store
  
 +  # We will use the old style of migrations, rather than the newer
 +  # timestamped migrations that were introduced with Rails 2.1, as
 +  # it will be confusing to have the numbered and timestamped migrations
 +  # together in the same folder.
 +  config.active_record.timestamped_migrations = false
 +
    # Use SQL instead of Active Record's schema dumper when creating the test database.
    # This is necessary if your schema can't be completely dumped by the schema dumper,
    # like if you have constraints or database-specific column types
@@@ -91,5 -72,5 +91,5 @@@
    # config.active_record.observers = :cacher, :garbage_collector
  
    # Make Active Record use UTC-base instead of local time
 -  # config.active_record.default_timezone = :utc
 +  config.active_record.default_timezone = :utc
  end
diff --combined config/initializers/libxml.rb
index ae636a9a3,f783cda1e..07f79660f
--- a/config/initializers/libxml.rb
+++ b/config/initializers/libxml.rb
@@@ -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
diff --combined db/migrate/019_add_timestamp_indexes.rb
index c6b3bc7c2,000000000..c6b3bc7c2
mode 100644,000000..100644
--- a/db/migrate/019_add_timestamp_indexes.rb
+++ b/db/migrate/019_add_timestamp_indexes.rb
@@@ -1,11 -1,0 +1,11 @@@
 +class AddTimestampIndexes < ActiveRecord::Migration
 +  def self.up
 +    add_index :current_ways, :timestamp, :name => :current_ways_timestamp_idx
 +    add_index :current_relations, :timestamp, :name => :current_relations_timestamp_idx
 +  end
 +
 +  def self.down
 +    remove_index :current_ways, :name => :current_ways_timestamp_idx
 +    remove_index :current_relations, :name => :current_relations_timestamp_idx
 +  end
 +end
diff --combined db/migrate/020_populate_node_tags_and_remove.rb
index 860358646,000000000..dc048f190
mode 100644,000000..100644
--- a/db/migrate/020_populate_node_tags_and_remove.rb
+++ b/db/migrate/020_populate_node_tags_and_remove.rb
@@@ -1,60 -1,0 +1,60 @@@
 +class PopulateNodeTagsAndRemove < ActiveRecord::Migration
 +  def self.up
 +    have_nodes = select_value("SELECT count(*) FROM current_nodes").to_i != 0
 +
 +    if have_nodes
-       prefix = File.join Dir.tmpdir, "019_populate_node_tags_and_remove.#{$$}."
++      prefix = File.join Dir.tmpdir, "020_populate_node_tags_and_remove.#{$$}."
 +
-       cmd = "db/migrate/019_populate_node_tags_and_remove_helper"
++      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
diff --combined db/migrate/020_populate_node_tags_and_remove_helper.c
index c41ea33da,000000000..c41ea33da
mode 100644,000000..100644
--- a/db/migrate/020_populate_node_tags_and_remove_helper.c
+++ b/db/migrate/020_populate_node_tags_and_remove_helper.c
@@@ -1,241 -1,0 +1,241 @@@
 +#include <mysql.h>
 +#include <stdint.h>
 +#include <stdio.h>
 +#include <stdlib.h>
 +#include <string.h>
 +
 +static void exit_mysql_err(MYSQL *mysql) {
 +  const char *err = mysql_error(mysql);
 +  if (err) {
 +    fprintf(stderr, "019_populate_node_tags_and_remove_helper: MySQL error: %s\n", err);
 +  } else {
 +    fprintf(stderr, "019_populate_node_tags_and_remove_helper: MySQL error\n");
 +  }
 +  abort();
 +  exit(EXIT_FAILURE);
 +}
 +
 +static void write_csv_col(FILE *f, const char *str, char end) {
 +  char *out = (char *) malloc(2 * strlen(str) + 4);
 +  char *o = out;
 +  size_t len;
 +
 +  *(o++) = '\"';
 +  for (; *str; str++) {
 +    if (*str == '\0') {
 +      break;
 +    } else if (*str == '\"') {
 +      *(o++) = '\"';
 +      *(o++) = '\"';
 +    } else {
 +      *(o++) = *str;
 +    }
 +  }
 +  *(o++) = '\"';
 +  *(o++) = end;
 +  *(o++) = '\0';
 +
 +  len = strlen(out);
 +  if (fwrite(out, len, 1, f) != 1) {
 +    perror("fwrite");
 +    exit(EXIT_FAILURE);
 +  }
 +
 +  free(out);
 +}
 +
 +static void unescape(char *str) {
 +  char *i = str, *o = str, tmp;
 +
 +  while (*i) {
 +    if (*i == '\\') {
 +      i++;
 +      switch (tmp = *i++) {
 +        case 's': *o++ = ';'; break;
 +        case 'e': *o++ = '='; break;
 +        case '\\': *o++ = '\\'; break;
 +        default: *o++ = tmp; break;
 +      }
 +    } else {
 +      *o++ = *i++;
 +    }
 +  }
 +}
 +
 +static int read_node_tags(char **tags, char **k, char **v) {
 +  if (!**tags) return 0;
 +  char *i = strchr(*tags, ';');
 +  if (!i) i = *tags + strlen(*tags);
 +  char *j = strchr(*tags, '=');
 +  *k = *tags;
 +  if (j && j < i) {
 +    *v = j + 1;
 +  } else {
 +    *v = i;
 +  }
 +  *tags = *i ? i + 1 : i;
 +  *i = '\0';
 +  if (j) *j = '\0';
 +
 +  unescape(*k);
 +  unescape(*v);
 +
 +  return 1;
 +}
 +
 +struct data {
 +  MYSQL *mysql;
 +  size_t version_size;
 +  uint16_t *version;
 +};
 +
 +static void proc_nodes(struct data *d, const char *tbl, FILE *out, FILE *out_tags, int hist) {
 +  MYSQL_RES *res;
 +  MYSQL_ROW row;
 +  char query[256];
 +
 +  snprintf(query, sizeof(query),  "SELECT id, latitude, longitude, "
 +      "user_id, visible, tags, timestamp, tile FROM %s", tbl);
 +  if (mysql_query(d->mysql, query))
 +    exit_mysql_err(d->mysql);
 +
 +  res = mysql_use_result(d->mysql);
 +  if (!res) exit_mysql_err(d->mysql);
 +
 +  while ((row = mysql_fetch_row(res))) {
 +    unsigned long id = strtoul(row[0], NULL, 10);
 +    uint32_t version;
 +
 +    if (id >= d->version_size) {
 +      fprintf(stderr, "preallocated nodes size exceeded");
 +      abort();
 +    }
 +
 +    if (hist) {
 +      version = ++(d->version[id]);
 +
 +      fprintf(out, "\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%u\"\n",
 +        row[0], row[1], row[2], row[3], row[4], row[6], row[7], version);
 +    } else {
 +      /*fprintf(out, "\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%s\"\n",
 +	row[0], row[1], row[2], row[3], row[4], row[6], row[7]);*/
 +    }
 +
 +    char *tags_it = row[5], *k, *v;
 +    while (read_node_tags(&tags_it, &k, &v)) {
 +      if (hist) {
 +        fprintf(out_tags, "\"%s\",\"%u\",", row[0], version);
 +      } else {
 +        fprintf(out_tags, "\"%s\",", row[0]);
 +      }
 +
 +      write_csv_col(out_tags, k, ',');
 +      write_csv_col(out_tags, v, '\n');
 +    }
 +  }
 +  if (mysql_errno(d->mysql)) exit_mysql_err(d->mysql);
 +
 +  mysql_free_result(res);
 +}
 +
 +static size_t select_size(MYSQL *mysql, const char *q) {
 +  MYSQL_RES *res;
 +  MYSQL_ROW row;
 +  size_t ret;
 +
 +  if (mysql_query(mysql, q))
 +    exit_mysql_err(mysql);
 +
 +  res = mysql_store_result(mysql);
 +  if (!res) exit_mysql_err(mysql);
 +
 +  row = mysql_fetch_row(res);
 +  if (!row) exit_mysql_err(mysql);
 +
 +  if (row[0]) {
 +    ret = strtoul(row[0], NULL, 10);
 +  } else {
 +    ret = 0;
 +  }
 +
 +  mysql_free_result(res);
 +
 +  return ret;
 +}
 +
 +static MYSQL *connect_to_mysql(char **argv) {
 +  MYSQL *mysql = mysql_init(NULL);
 +  if (!mysql) exit_mysql_err(mysql);
 +
 +  if (!mysql_real_connect(mysql, argv[1], argv[2], argv[3], argv[4],
 +      argv[5][0] ? atoi(argv[5]) : 0, argv[6][0] ? argv[6] : NULL, 0))
 +    exit_mysql_err(mysql);
 +
 +  if (mysql_set_character_set(mysql, "utf8"))
 +    exit_mysql_err(mysql);
 +
 +  return mysql;
 +}
 +
 +static void open_file(FILE **f, char *fn) {
 +  *f = fopen(fn, "w+");
 +  if (!*f) {
 +    perror("fopen");
 +    exit(EXIT_FAILURE);
 +  }
 +}
 +
 +int main(int argc, char **argv) {
 +  size_t prefix_len;
 +  FILE *current_nodes, *current_node_tags, *nodes, *node_tags;
 +  char *tempfn;
 +  struct data data, *d = &data;
 +
 +  if (argc != 8) {
 +    printf("Usage: 019_populate_node_tags_and_remove_helper host user passwd database port socket prefix\n");
 +    exit(EXIT_FAILURE);
 +  }
 +
 +  d->mysql = connect_to_mysql(argv);
 +
 +  d->version_size = 1 + select_size(d->mysql, "SELECT max(id) FROM current_nodes");
 +  d->version = (uint16_t *) malloc(sizeof(uint16_t) * d->version_size);
 +  if (!d->version) {
 +    perror("malloc");
 +    abort();
 +    exit(EXIT_FAILURE);
 +  }
 +  memset(d->version, 0, sizeof(uint16_t) * d->version_size);
 +
 +  prefix_len = strlen(argv[7]);
 +  tempfn = (char *) malloc(prefix_len + 32);
 +  strcpy(tempfn, argv[7]);
 +
 +  strcpy(tempfn + prefix_len, "current_nodes");
 +  open_file(&current_nodes, tempfn);
 +
 +  strcpy(tempfn + prefix_len, "current_node_tags");
 +  open_file(&current_node_tags, tempfn);
 +
 +  strcpy(tempfn + prefix_len, "nodes");
 +  open_file(&nodes, tempfn);
 +
 +  strcpy(tempfn + prefix_len, "node_tags");
 +  open_file(&node_tags, tempfn);
 +
 +  free(tempfn);
 +
 +  proc_nodes(d, "nodes", nodes, node_tags, 1);
 +  proc_nodes(d, "current_nodes", current_nodes, current_node_tags, 0);
 +
 +  free(d->version);
 +
 +  mysql_close(d->mysql);
 +
 +  fclose(current_nodes);
 +  fclose(current_node_tags);
 +  fclose(nodes);
 +  fclose(node_tags);
 +
 +  exit(EXIT_SUCCESS);
 +}
diff --combined db/migrate/021_move_to_innodb.rb
index da0488ca5,000000000..da0488ca5
mode 100644,000000..100644
--- a/db/migrate/021_move_to_innodb.rb
+++ b/db/migrate/021_move_to_innodb.rb
@@@ -1,45 -1,0 +1,45 @@@
 +class MoveToInnodb < ActiveRecord::Migration
 +  @@conv_tables = ['nodes', 'ways', 'way_tags', 'way_nodes',
 +    'current_way_tags', 'relation_members',
 +    'relations', 'relation_tags', 'current_relation_tags']
 +
 +  @@ver_tbl = ['nodes', 'ways', 'relations']
 +
 +  def self.up
 +    remove_index :current_way_tags, :name=> :current_way_tags_v_idx
 +    remove_index :current_relation_tags, :name=> :current_relation_tags_v_idx
 +
 +    @@ver_tbl.each { |tbl|
 +      change_column tbl, "version", :bigint, :limit => 20, :null => false
 +    }
 +
 +    @@conv_tables.each { |tbl|
 +      change_engine (tbl, "InnoDB")
 +    }
 +
 +    @@ver_tbl.each { |tbl|
 +      add_column "current_#{tbl}", "version", :bigint, :limit => 20, :null => false
 +      # As the initial version of all nodes, ways and relations is 0, we set the 
 +      # current version to something less so that we can update the version in 
 +      # batches of 10000
 +      tbl.classify.constantize.update_all("version=-1")
 +      while tbl.classify.constantize.count(:conditions => {:version => -1}) > 0
 +        tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", {:version => -1}, :limit => 10000)
 +      end
 +     # execute "UPDATE current_#{tbl} SET version = " +
 +      #  "(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)"
 +        # The above update causes a MySQL error:
 +        # -- add_column("current_nodes", "version", :bigint, {:null=>false, :limit=>20})
 +        # -> 1410.9152s
 +        # -- execute("UPDATE current_nodes SET version = (SELECT max(version) FROM nodes WHERE nodes.id = current_nodes.id)")
 +        # rake aborted!
 +        # Mysql::Error: The total number of locks exceeds the lock table size: UPDATE current_nodes SET version = (SELECT max(version) FROM nodes WHERE nodes.id = current_nodes.id)
 +
 +        # The above rails version will take longer, however will no run out of locks
 +    }
 +  end
 +
 +  def self.down
 +    raise IrreversibleMigration.new
 +  end
 +end
diff --combined db/migrate/022_key_constraints.rb
index 40f98be02,000000000..40f98be02
mode 100644,000000..100644
--- a/db/migrate/022_key_constraints.rb
+++ b/db/migrate/022_key_constraints.rb
@@@ -1,50 -1,0 +1,50 @@@
 +class KeyConstraints < ActiveRecord::Migration
 +  def self.up
 +    # Primary keys
 +    add_primary_key :current_node_tags, [:id, :k]
 +    add_primary_key :current_way_tags, [:id, :k]
 +    add_primary_key :current_relation_tags, [:id, :k]
 +
 +    add_primary_key :node_tags, [:id, :version, :k]
 +    add_primary_key :way_tags, [:id, :version, :k]
 +    add_primary_key :relation_tags, [:id, :version, :k]
 +
 +    add_primary_key :nodes, [:id, :version]
 +
 +    # Remove indexes superseded by primary keys
 +    remove_index :current_way_tags, :name => :current_way_tags_id_idx
 +    remove_index :current_relation_tags, :name => :current_relation_tags_id_idx
 +
 +    remove_index :way_tags, :name => :way_tags_id_version_idx
 +    remove_index :relation_tags, :name => :relation_tags_id_version_idx
 +
 +    remove_index :nodes, :name => :nodes_uid_idx
 +
 +    # Foreign keys (between ways, way_tags, way_nodes, etc.)
 +    add_foreign_key :current_node_tags, [:id], :current_nodes
 +    add_foreign_key :node_tags, [:id, :version], :nodes
 +
 +    add_foreign_key :current_way_tags, [:id], :current_ways
 +    add_foreign_key :current_way_nodes, [:id], :current_ways
 +    add_foreign_key :way_tags, [:id, :version], :ways
 +    add_foreign_key :way_nodes, [:id, :version], :ways
 +
 +    add_foreign_key :current_relation_tags, [:id], :current_relations
 +    add_foreign_key :current_relation_members, [:id], :current_relations
 +    add_foreign_key :relation_tags, [:id, :version], :relations
 +    add_foreign_key :relation_members, [:id, :version], :relations
 +
 +    # Foreign keys (between different types of primitives)
 +    add_foreign_key :current_way_nodes, [:node_id], :current_nodes, [:id]
 +
 +    # FIXME: We don't have foreign keys for relation members since the id
 +    # might point to a different table depending on the `type' column.
 +    # We'd probably need different current_relation_member_nodes,
 +    # current_relation_member_ways and current_relation_member_relations
 +    # tables for this to work cleanly.
 +  end
 +
 +  def self.down
 +    raise IrreversibleMigration.new
 +  end
 +end
diff --combined db/migrate/023_add_changesets.rb
index e0cf3904a,000000000..e0cf3904a
mode 100644,000000..100644
--- a/db/migrate/023_add_changesets.rb
+++ b/db/migrate/023_add_changesets.rb
@@@ -1,46 -1,0 +1,46 @@@
 +class AddChangesets < ActiveRecord::Migration
 +  @@conv_user_tables = ['current_nodes',
 +  'current_relations', 'current_ways', 'nodes', 'relations', 'ways' ]
 +  
 +  def self.up
 +    create_table "changesets", innodb_table do |t|
 +      t.column "id",             :bigint_pk,              :null => false
 +      t.column "user_id",        :bigint,   :limit => 20, :null => false
 +      t.column "created_at",     :datetime,               :null => false
 +      t.column "open",           :boolean,                :null => false, :default => true
 +      t.column "min_lat",        :integer,                :null => true
 +      t.column "max_lat",        :integer,                :null => true
 +      t.column "min_lon",        :integer,                :null => true
 +      t.column "max_lon",        :integer,                :null => true
 +    end
 +
 +    create_table "changeset_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
 +
 +    add_index "changeset_tags", ["id"], :name => "changeset_tags_id_idx"
 +    
 +    #
 +    # Initially we will have one changeset for every user containing 
 +    # all edits up to the API change,  
 +    # all the changesets will have the id of the user that made them.
 +    # We need to generate a changeset for each user in the database
 +    execute "INSERT INTO changesets (id, user_id, created_at, open)" + 
 +      "SELECT id, id, creation_time, false from users;"
 +
 +    @@conv_user_tables.each { |tbl|
 +      rename_column tbl, :user_id, :changeset_id
 +      #foreign keys too
 +      add_foreign_key tbl, [:changeset_id], :changesets, [:id]
 +    }
 +  end
 +
 +  def self.down
 +    # It's not easy to generate the user ids from the changesets
 +    raise IrreversibleMigration.new
 +    #drop_table "changesets"
 +    #drop_table "changeset_tags"
 +  end
 +end
diff --combined db/migrate/024_order_relation_members.rb
index 5500edfcf,000000000..5500edfcf
mode 100644,000000..100644
--- a/db/migrate/024_order_relation_members.rb
+++ b/db/migrate/024_order_relation_members.rb
@@@ -1,33 -1,0 +1,33 @@@
 +class OrderRelationMembers < ActiveRecord::Migration
 +  def self.up
 +    # add sequence column. rails won't let us define an ordering here,
 +    # as defaults must be constant.
 +    add_column(:relation_members, :sequence_id, :integer,
 +               :default => 0, :null => false)
 +
 +    # update the sequence column with default (partial) ordering by 
 +    # element ID. the sequence ID is a smaller int type, so we can't
 +    # just copy the member_id.
 +    execute("update relation_members set sequence_id = mod(member_id, 16384)")
 +
 +    # need to update the primary key to include the sequence number, 
 +    # otherwise the primary key will barf when we have repeated members.
 +    # mysql barfs on this anyway, so we need a single command. this may
 +    # not work in postgres... needs testing.
 +    alter_primary_key("relation_members", [:id, :version, :member_type, :member_id, :member_role, :sequence_id])
 +
 +    # do the same for the current tables
 +    add_column(:current_relation_members, :sequence_id, :integer,
 +               :default => 0, :null => false)
 +    execute("update current_relation_members set sequence_id = mod(member_id, 16384)")
 +    alter_primary_key("current_relation_members", [:id, :member_type, :member_id, :member_role, :sequence_id])
 +  end
 +
 +  def self.down
 +    alter_primary_key("current_relation_members", [:id, :member_type, :member_id, :member_role])
 +    remove_column :relation_members, :sequence_id
 +
 +    alter_primary_key("relation_members", [:id, :version, :member_type, :member_id, :member_role])
 +    remove_column :current_relation_members, :sequence_id
 +  end
 +end
diff --combined db/migrate/025_add_end_time_to_changesets.rb
index b87ce3fde,000000000..b87ce3fde
mode 100644,000000..100644
--- a/db/migrate/025_add_end_time_to_changesets.rb
+++ b/db/migrate/025_add_end_time_to_changesets.rb
@@@ -1,34 -1,0 +1,34 @@@
 +class AddEndTimeToChangesets < ActiveRecord::Migration
 +  def self.up
 +    # swap the boolean closed-or-not for a time when the changeset will
 +    # close or has closed.
 +    add_column(:changesets, :closed_at, :datetime, :null => false)
 +    
 +    # it appears that execute will only accept string arguments, so
 +    # this is an ugly, ugly hack to get some sort of mysql/postgres
 +    # independence. now i have to go wash my brain with bleach.
 +    execute("update changesets set closed_at=(now()-'1 hour') where open=(1=0)")
 +    execute("update changesets set closed_at=(now()+'1 hour') where open=(1=1)")
 +
 +    # remove the open column as it is unnecessary now and denormalises 
 +    # the table.
 +    remove_column :changesets, :open
 +
 +    # add a column to keep track of the number of changes in a changeset.
 +    # could probably work out how many changes there are here, but i'm not
 +    # sure its actually important.
 +    add_column(:changesets, :num_changes, :integer, 
 +               :null => false, :default => 0)
 +  end
 +
 +  def self.down
 +    # in the reverse direction, we can look at the closed_at to figure out
 +    # if changesets are closed or not.
 +    add_column(:changesets, :open, :boolean, :null => false, :default => true)
 +    execute("update changesets set open=(closed_at > now())")
 +    remove_column :changesets, :closed_at
 +
 +    # remove the column for tracking number of changes
 +    remove_column :changesets, :num_changes
 +  end
 +end
diff --combined lib/diff_reader.rb
index 37de8ea59,000000000..217e9309d
mode 100644,000000..100644
--- a/lib/diff_reader.rb
+++ b/lib/diff_reader.rb
@@@ -1,223 -1,0 +1,224 @@@
 +##
 +# 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)
-     @reader = XML::Reader.new data
++    @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 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.")
++    # 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 --combined lib/osm.rb
index f6646503d,365ddf68e..f372979a8
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@@ -10,157 -10,18 +10,157 @@@ module OS
  
    # The base class for API Errors.
    class APIError < RuntimeError
 +    def render_opts
 +      { :text => "Generic API Error", :status => :internal_server_error, :content_type => "text/plain" }
 +    end
    end
  
    # Raised when an API object is not found.
    class APINotFoundError < APIError
 +    def render_opts
 +      { :text => "The API wasn't found", :status => :not_found, :content_type => "text/plain" }
 +    end
    end
  
    # Raised when a precondition to an API action fails sanity check.
    class APIPreconditionFailedError < APIError
 +    def initialize(message = "")
 +      @message = message
 +    end
 +    
 +    def render_opts
 +      { :text => "Precondition failed: #{@message}", :status => :precondition_failed, :content_type => "text/plain" }
 +    end
    end
  
    # Raised when to delete an already-deleted object.
    class APIAlreadyDeletedError < APIError
 +    def render_opts
 +      { :text => "The object has already been deleted", :status => :gone, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  # Raised when the user logged in isn't the same as the changeset
 +  class APIUserChangesetMismatchError < APIError
 +    def render_opts
 +      { :text => "The user doesn't own that changeset", :status => :conflict, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  # Raised when the changeset provided is already closed
 +  class APIChangesetAlreadyClosedError < APIError
 +    def initialize(changeset)
 +      @changeset = changeset
 +    end
 +
 +    attr_reader :changeset
 +    
 +    def render_opts
 +      { :text => "The changeset #{@changeset.id} was closed at #{@changeset.closed_at}.", :status => :conflict, :content_type => "text/plain" }
 +    end
 +  end
 +  
 +  # Raised when a change is expecting a changeset, but the changeset doesn't exist
 +  class APIChangesetMissingError < APIError
 +    def render_opts
 +      { :text => "You need to supply a changeset to be able to make a change", :status => :conflict, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  # Raised when a diff is uploaded containing many changeset IDs which don't match
 +  # the changeset ID that the diff was uploaded to.
 +  class APIChangesetMismatchError < APIError
 +    def initialize(provided, allowed)
 +      @provided, @allowed = provided, allowed
 +    end
 +    
 +    def render_opts
 +      { :text => "Changeset mismatch: Provided #{@provided} but only " +
 +      "#{@allowed} is allowed.", :status => :conflict, :content_type => "text/plain" }
 +    end
 +  end
 +  
 +  # Raised when a diff upload has an unknown action. You can only have create,
 +  # modify, or delete
 +  class APIChangesetActionInvalid < APIError
 +    def initialize(provided)
 +      @provided = provided
 +    end
 +    
 +    def render_opts
 +      { :text => "Unknown action #{@provided}, choices are create, modify, delete.",
 +      :status => :bad_request, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  # Raised when bad XML is encountered which stops things parsing as
 +  # they should.
 +  class APIBadXMLError < APIError
 +    def initialize(model, xml, message="")
 +      @model, @xml, @message = model, xml, message
 +    end
 +
 +    def render_opts
 +      { :text => "Cannot parse valid #{@model} from xml string #{@xml}. #{@message}",
 +      :status => :bad_request, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  # Raised when the provided version is not equal to the latest in the db.
 +  class APIVersionMismatchError < APIError
 +    def initialize(id, type, provided, latest)
 +      @id, @type, @provided, @latest = id, type, provided, latest
 +    end
 +
 +    attr_reader :provided, :latest, :id, :type
 +
 +    def render_opts
 +      { :text => "Version mismatch: Provided " + provided.to_s +
 +        ", server had: " + latest.to_s + " of " + type + " " + id.to_s, 
 +        :status => :conflict, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  # raised when a two tags have a duplicate key string in an element.
 +  # this is now forbidden by the API.
 +  class APIDuplicateTagsError < APIError
 +    def initialize(type, id, tag_key)
 +      @type, @id, @tag_key = type, id, tag_key
 +    end
 +
 +    attr_reader :type, :id, :tag_key
 +
 +    def render_opts
 +      { :text => "Element #{@type}/#{@id} has duplicate tags with key #{@tag_key}.",
 +        :status => :bad_request, :content_type => "text/plain" }
 +    end
 +  end
 +  
 +  # Raised when a way has more than the configured number of way nodes.
 +  # This prevents ways from being to long and difficult to work with
 +  class APITooManyWayNodesError < APIError
 +    def initialize(provided, max)
 +      @provided, @max = provided, max
 +    end
 +    
 +    attr_reader :provided, :max
 +    
 +    def render_opts
 +      { :text => "You tried to add #{provided} nodes to the way, however only #{max} are allowed",
 +        :status => :bad_request, :content_type => "text/plain" }
 +    end
 +  end
 +
 +  ##
 +  # raised when user input couldn't be parsed
 +  class APIBadUserInput < APIError
 +    def initialize(message)
 +      @message = message
 +    end
 +
 +    def render_opts
 +      { :text => @message, :content_type => "text/plain", :status => :bad_request }
 +    end
    end
  
    # Helper methods for going to/from mercator and lat/lng.
@@@ -240,7 -101,7 +240,7 @@@
    class GeoRSS
      def initialize(feed_title='OpenStreetMap GPS Traces', feed_description='OpenStreetMap GPS Traces', feed_url='http://www.openstreetmap.org/traces/')
        @doc = XML::Document.new
-       @doc.encoding = 'UTF-8' 
+       @doc.encoding = XML::Encoding::UTF_8
  
        rss = XML::Node.new 'rss'
        @doc.root = rss
@@@ -326,10 -187,10 +326,10 @@@
    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
diff --combined public/stylesheets/site.css
index 764d8971b,a6b6c8702..87d325ddb
--- a/public/stylesheets/site.css
+++ b/public/stylesheets/site.css
@@@ -64,13 -64,14 +64,14 @@@ body 
  }
  
  #intro {
-   width: 150px;
+   width: 170px;
    margin: 10px;
-   padding: 10px;
    border: 1px solid #ccc;
    font-size: 11px;
  }
  
+ #intro p { margin: 10px; }
+ 
  #alert {
    width: 150px;
    margin: 10px;
@@@ -82,6 -83,17 +83,17 @@@
    font-size: 14px;
  }
  
+ #donate {
+   width: 150px;
+   margin: 10px;
+   padding: 10px;
+   border: 1px solid #ccc;
+   background: #ea0;
+   line-height: 1.2em;
+   text-align: left;
+   font-size: 12px;
+ }
+ 
  .left_menu {
    width: 150px;
    min-width: 150px;
@@@ -277,12 -289,6 +289,12 @@@ hides rule from IE5-Mac \*
    font-size: 10px;
  }
  
 +hr {
 +  border: none;
 +  background-color: #ccc;
 +  color: #ccc;
 +  height: 1px;
 +}
  
  .gpxsummary {
    font-size: 12px;
@@@ -361,17 -367,9 +373,9 @@@
    top: 4px;
  }
  
- #cclogo {
-   width: 150px;
-   min-width: 150px;
-   margin: 10px;
-   padding: 10px;
-   left: 0px;
-   line-height: 1.2em;
-   text-align: left;
-   font-size: 14px;
-   font-weight: bold;
-   background: #fff;
+ .button {
+   margin-top: 10px;
+   margin-bottom: 10px;
  }
  
  #controls img
@@@ -533,16 -531,6 +537,16 @@@ input[type="submit"] 
    border: 1px solid black;
  }
  
 +#accountForm td {
 +	padding-bottom:10px;
 +}
 +
 +.fieldName {
 +	text-align:right;
 +	font-weight:bold;
 +}
 +
 +
  .nohome .location {
    display: none;
  }
@@@ -555,8 -543,9 +559,8 @@@
    display: inline !important;
  }
  
 -.editDescription {
 -  height: 10ex;
 -  width: 30em;
 +.minorNote {
 +	font-size:0.8em;
  }
  
  .nowrap {
diff --combined test/functional/node_controller_test.rb
index bc9ffa489,a380eeb20..8d019bf79
--- a/test/functional/node_controller_test.rb
+++ b/test/functional/node_controller_test.rb
@@@ -1,23 -1,28 +1,23 @@@
  require File.dirname(__FILE__) + '/../test_helper'
 -require 'node_controller'
  
 -# Re-raise errors caught by the controller.
 -class NodeController; def rescue_action(e) raise e end; end
 -
 -class NodeControllerTest < Test::Unit::TestCase
 +class NodeControllerTest < ActionController::TestCase
    api_fixtures
  
 -  def setup
 -    @controller = NodeController.new
 -    @request    = ActionController::TestRequest.new
 -    @response   = ActionController::TestResponse.new
 -  end
 -
    def test_create
      # cannot read password from fixture as it is stored as MD5 digest
 -    basic_authorization("test@openstreetmap.org", "test");  
 +    basic_authorization(users(:normal_user).email, "test")
 +    
      # create a node with random lat/lon
      lat = rand(100)-50 + rand
      lon = rand(100)-50 + rand
 -    content("<osm><node lat='#{lat}' lon='#{lon}' /></osm>")
 +    # normal user has a changeset open, so we'll use that.
 +    changeset = changesets(:normal_user_first_change)
 +    # create a minimal xml file
 +    content("<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'/></osm>")
      put :create
      # hope for success
      assert_response :success, "node upload did not return success status"
 +
      # read id of created node and search for it
      nodeid = @response.body
      checknode = Node.find(nodeid)
@@@ -25,36 -30,10 +25,36 @@@
      # compare values
      assert_in_delta lat * 10000000, checknode.latitude, 1, "saved node does not match requested latitude"
      assert_in_delta lon * 10000000, checknode.longitude, 1, "saved node does not match requested longitude"
 -    assert_equal users(:normal_user).id, checknode.user_id, "saved node does not belong to user that created it"
 +    assert_equal changesets(:normal_user_first_change).id, checknode.changeset_id, "saved node does not belong to changeset that it was created in"
      assert_equal true, checknode.visible, "saved node is not visible"
    end
  
 +  def test_create_invalid_xml
 +    # Initial setup
 +    basic_authorization(users(:normal_user).email, "test")
 +    # normal user has a changeset open, so we'll use that.
 +    changeset = changesets(:normal_user_first_change)
 +    lat = 3.434
 +    lon = 3.23
 +    
 +    # test that the upload is rejected when no lat is supplied
 +    # create a minimal xml file
 +    content("<osm><node lon='#{lon}' changeset='#{changeset.id}'/></osm>")
 +    put :create
 +    # hope for success
 +    assert_response :bad_request, "node upload did not return bad_request status"
 +    assert_equal 'Cannot parse valid node from xml string <node lon="3.23" changeset="1"/>. lat missing', @response.body
 +
 +    # test that the upload is rejected when no lon is supplied
 +    # create a minimal xml file
 +    content("<osm><node lat='#{lat}' changeset='#{changeset.id}'/></osm>")
 +    put :create
 +    # hope for success
 +    assert_response :bad_request, "node upload did not return bad_request status"
 +    assert_equal 'Cannot parse valid node from xml string <node lat="3.434" changeset="1"/>. lon missing', @response.body
 +
 +  end
 +
    def test_read
      # check that a visible node is returned properly
      get :read, :id => current_nodes(:visible_node).id
@@@ -72,36 -51,19 +72,36 @@@
    # this tests deletion restrictions - basic deletion is tested in the unit
    # tests for node!
    def test_delete
 -
      # first try to delete node without auth
      delete :delete, :id => current_nodes(:visible_node).id
      assert_response :unauthorized
  
      # now set auth
 -    basic_authorization("test@openstreetmap.org", "test");  
 +    basic_authorization(users(:normal_user).email, "test");  
  
 -    # this should work
 +    # try to delete with an invalid (closed) changeset
 +    content update_changeset(current_nodes(:visible_node).to_xml,
 +                             changesets(:normal_user_closed_change).id)
 +    delete :delete, :id => current_nodes(:visible_node).id
 +    assert_response :conflict
 +
 +    # try to delete with an invalid (non-existent) changeset
 +    content update_changeset(current_nodes(:visible_node).to_xml,0)
 +    delete :delete, :id => current_nodes(:visible_node).id
 +    assert_response :conflict
 +
 +    # valid delete now takes a payload
 +    content(nodes(:visible_node).to_xml)
      delete :delete, :id => current_nodes(:visible_node).id
      assert_response :success
  
 +    # valid delete should return the new version number, which should
 +    # be greater than the old version number
 +    assert @response.body.to_i > current_nodes(:visible_node).version,
 +       "delete request should return a new version number for node"
 +
      # this won't work since the node is already deleted
 +    content(nodes(:invisible_node).to_xml)
      delete :delete, :id => current_nodes(:invisible_node).id
      assert_response :gone
  
@@@ -109,175 -71,17 +109,174 @@@
      delete :delete, :id => 0
      assert_response :not_found
  
 -    # this won't work since the node is in use
 +    ## these test whether nodes which are in-use can be deleted:
 +    # in a way...
 +    content(nodes(:used_node_1).to_xml)
      delete :delete, :id => current_nodes(:used_node_1).id
 -    assert_response :precondition_failed
 +    assert_response :precondition_failed,
 +       "shouldn't be able to delete a node used in a way (#{@response.body})"
 +
 +    # in a relation...
 +    content(nodes(:node_used_by_relationship).to_xml)
 +    delete :delete, :id => current_nodes(:node_used_by_relationship).id
 +    assert_response :precondition_failed,
 +       "shouldn't be able to delete a node used in a relation (#{@response.body})"
 +  end
 +
 +  ##
 +  # tests whether the API works and prevents incorrect use while trying
 +  # to update nodes.
 +  def test_update
 +    # try and update a node without authorisation
 +    # first try to delete node without auth
 +    content current_nodes(:visible_node).to_xml
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :unauthorized
 +    
 +    # setup auth
 +    basic_authorization(users(:normal_user).email, "test")
 +
 +    ## trying to break changesets
 +
 +    # try and update in someone else's changeset
 +    content update_changeset(current_nodes(:visible_node).to_xml,
 +                             changesets(:second_user_first_change).id)
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :conflict, "update with other user's changeset should be rejected"
 +
 +    # try and update in a closed changeset
 +    content update_changeset(current_nodes(:visible_node).to_xml,
 +                             changesets(:normal_user_closed_change).id)
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :conflict, "update with closed changeset should be rejected"
 +
 +    # try and update in a non-existant changeset
 +    content update_changeset(current_nodes(:visible_node).to_xml, 0)
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :conflict, "update with changeset=0 should be rejected"
 +
 +    ## try and submit invalid updates
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lat', 91.0);
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :bad_request, "node at lat=91 should be rejected"
 +
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lat', -91.0);
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :bad_request, "node at lat=-91 should be rejected"
 +    
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lon', 181.0);
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :bad_request, "node at lon=181 should be rejected"
 +
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 'lon', -181.0);
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :bad_request, "node at lon=-181 should be rejected"
 +
 +    ## next, attack the versioning
 +    current_node_version = current_nodes(:visible_node).version
 +
 +    # try and submit a version behind
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 
 +                             'version', current_node_version - 1);
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :conflict, "should have failed on old version number"
 +    
 +    # try and submit a version ahead
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 
 +                             'version', current_node_version + 1);
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :conflict, "should have failed on skipped version number"
 +
 +    # try and submit total crap in the version field
 +    content xml_attr_rewrite(current_nodes(:visible_node).to_xml, 
 +                             'version', 'p1r4t3s!');
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :conflict, 
 +       "should not be able to put 'p1r4at3s!' in the version field"
 +    
 +    ## finally, produce a good request which should work
 +    content current_nodes(:visible_node).to_xml
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :success, "a valid update request failed"
    end
  
 +  ##
 +  # test adding tags to a node
 +  def test_duplicate_tags
 +    # setup auth
 +    basic_authorization(users(:normal_user).email, "test")
 +
 +    # add an identical tag to the node
 +    tag_xml = XML::Node.new("tag")
 +    tag_xml['k'] = current_node_tags(:t1).k
 +    tag_xml['v'] = current_node_tags(:t1).v
 +
 +    # add the tag into the existing xml
 +    node_xml = current_nodes(:visible_node).to_xml
 +    node_xml.find("//osm/node").first << tag_xml
 +
 +    # try and upload it
 +    content node_xml
 +    put :update, :id => current_nodes(:visible_node).id
 +    assert_response :bad_request, 
 +      "adding duplicate tags to a node should fail with 'bad request'"
 +    assert_equal "Element node/#{current_nodes(:visible_node).id} has duplicate tags with key #{current_node_tags(:t1).k}.", @response.body
 +  end
 +
 +  # test whether string injection is possible
 +  def test_string_injection
 +    basic_authorization(users(:normal_user).email, "test")
 +    changeset_id = changesets(:normal_user_first_change).id
 +
 +    # try and put something into a string that the API might 
 +    # use unquoted and therefore allow code injection...
 +    content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" +
 +      '<tag k="#{@user.inspect}" v="0"/>' +
 +      '</node></osm>'
 +    put :create
 +    assert_response :success
 +    nodeid = @response.body
 +
 +    # find the node in the database
 +    checknode = Node.find(nodeid)
 +    assert_not_nil checknode, "node not found in data base after upload"
 +    
 +    # and grab it using the api
 +    get :read, :id => nodeid
 +    assert_response :success
 +    apinode = Node.from_xml(@response.body)
 +    assert_not_nil apinode, "downloaded node is nil, but shouldn't be"
 +    
 +    # check the tags are not corrupted
 +    assert_equal checknode.tags, apinode.tags
 +    assert apinode.tags.include?('#{@user.inspect}')
 +  end
  
    def basic_authorization(user, pass)
      @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
    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
diff --combined test/functional/relation_controller_test.rb
index 4ace316a4,202a015a8..f52981233
--- a/test/functional/relation_controller_test.rb
+++ b/test/functional/relation_controller_test.rb
@@@ -1,15 -1,27 +1,15 @@@
  require File.dirname(__FILE__) + '/../test_helper'
  require 'relation_controller'
  
 -# Re-raise errors caught by the controller.
 -class RelationController; def rescue_action(e) raise e end; end
 -
 -class RelationControllerTest < Test::Unit::TestCase
 +class RelationControllerTest < ActionController::TestCase
    api_fixtures
 -  fixtures :relations, :current_relations, :relation_members, :current_relation_members, :relation_tags, :current_relation_tags
 -  set_fixture_class :current_relations => :Relation
 -  set_fixture_class :relations => :OldRelation
 -
 -  def setup
 -    @controller = RelationController.new
 -    @request    = ActionController::TestRequest.new
 -    @response   = ActionController::TestResponse.new
 -  end
  
    def basic_authorization(user, pass)
      @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
    end
  
    def content(c)
 -    @request.env["RAW_POST_DATA"] = c
 +    @request.env["RAW_POST_DATA"] = c.to_s
    end
  
    # -------------------------------------
@@@ -28,49 -40,31 +28,49 @@@
      # check chat a non-existent relation is not returned
      get :read, :id => 0
      assert_response :not_found
 +  end
  
 -    # check the "relations for node" mode
 -    get :relations_for_node, :id => current_nodes(:node_used_by_relationship).id
 -    assert_response :success
 -    # FIXME check whether this contains the stuff we want!
 -    if $VERBOSE
 -        print @response.body
 -    end
 +  ##
 +  # check that all relations containing a particular node, and no extra
 +  # relations, are returned from the relations_for_node call.
 +  def test_relations_for_node
 +    check_relations_for_element(:relations_for_node, "node", 
 +                                current_nodes(:node_used_by_relationship).id,
 +                                [ :visible_relation, :used_relation ])
 +  end
  
 -    # check the "relations for way" mode
 -    get :relations_for_way, :id => current_ways(:used_way).id
 -    assert_response :success
 -    # FIXME check whether this contains the stuff we want!
 -    if $VERBOSE
 -        print @response.body
 -    end
 +  def test_relations_for_way
 +    check_relations_for_element(:relations_for_way, "way",
 +                                current_ways(:used_way).id,
 +                                [ :visible_relation ])
 +  end
  
 +  def test_relations_for_relation
 +    check_relations_for_element(:relations_for_relation, "relation",
 +                                current_relations(:used_relation).id,
 +                                [ :visible_relation ])
 +  end
 +
 +  def check_relations_for_element(method, type, id, expected_relations)
      # check the "relations for relation" mode
 -    get :relations_for_relation, :id => current_relations(:used_relation).id
 +    get method, :id => id
      assert_response :success
 -    # FIXME check whether this contains the stuff we want!
 -    if $VERBOSE
 -        print @response.body
 +
 +    # count one osm element
 +    assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1
 +
 +    # we should have only the expected number of relations
 +    assert_select "osm>relation", expected_relations.size
 +
 +    # and each of them should contain the node we originally searched for
 +    expected_relations.each do |r|
 +      relation_id = current_relations(r).id
 +      assert_select "osm>relation#?", relation_id
 +      assert_select "osm>relation#?>member[type=\"#{type}\"][ref=#{id}]", relation_id
      end
 +  end
  
 +  def test_full
      # check the "full" mode
      get :full, :id => current_relations(:visible_relation).id
      assert_response :success
@@@ -86,12 -80,9 +86,12 @@@
  
    def test_create
      basic_authorization "test@openstreetmap.org", "test"
 +    
 +    # put the relation in a dummy fixture changset
 +    changeset_id = changesets(:normal_user_first_change).id
  
      # create an relation without members
 -    content "<osm><relation><tag k='test' v='yes' /></relation></osm>"
 +    content "<osm><relation changeset='#{changeset_id}'><tag k='test' v='yes' /></relation></osm>"
      put :create
      # hope for success
      assert_response :success, 
@@@ -106,9 -97,7 +106,9 @@@
          "saved relation contains members but should not"
      assert_equal checkrelation.tags.length, 1, 
          "saved relation does not contain exactly one tag"
 -    assert_equal users(:normal_user).id, checkrelation.user_id, 
 +    assert_equal changeset_id, checkrelation.changeset.id,
 +        "saved relation does not belong in the changeset it was assigned to"
 +    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
          "saved relation does not belong to user that created it"
      assert_equal true, checkrelation.visible, 
          "saved relation is not visible"
@@@ -117,46 -106,10 +117,46 @@@
      assert_response :success
  
  
 +    ###
      # create an relation with a node as member
 +    # This time try with a role attribute in the relation
 +    nid = current_nodes(:used_node_1).id
 +    content "<osm><relation changeset='#{changeset_id}'>" +
 +      "<member  ref='#{nid}' type='node' role='some'/>" +
 +      "<tag k='test' v='yes' /></relation></osm>"
 +    put :create
 +    # hope for success
 +    assert_response :success, 
 +        "relation upload did not return success status"
 +    # read id of created relation and search for it
 +    relationid = @response.body
 +    checkrelation = Relation.find(relationid)
 +    assert_not_nil checkrelation, 
 +        "uploaded relation not found in data base after upload"
 +    # compare values
 +    assert_equal checkrelation.members.length, 1, 
 +        "saved relation does not contain exactly one member"
 +    assert_equal checkrelation.tags.length, 1, 
 +        "saved relation does not contain exactly one tag"
 +    assert_equal changeset_id, checkrelation.changeset.id,
 +        "saved relation does not belong in the changeset it was assigned to"
 +    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
 +        "saved relation does not belong to user that created it"
 +    assert_equal true, checkrelation.visible, 
 +        "saved relation is not visible"
 +    # ok the relation is there but can we also retrieve it?
 +    
 +    get :read, :id => relationid
 +    assert_response :success
 +    
 +    
 +    ###
 +    # create an relation with a node as member, this time test that we don't 
 +    # need a role attribute to be included
      nid = current_nodes(:used_node_1).id
 -    content "<osm><relation><member type='node' ref='#{nid}' role='some'/>" +
 -        "<tag k='test' v='yes' /></relation></osm>"
 +    content "<osm><relation changeset='#{changeset_id}'>" +
 +      "<member  ref='#{nid}' type='node'/>"+
 +      "<tag k='test' v='yes' /></relation></osm>"
      put :create
      # hope for success
      assert_response :success, 
@@@ -171,9 -124,7 +171,9 @@@
          "saved relation does not contain exactly one member"
      assert_equal checkrelation.tags.length, 1, 
          "saved relation does not contain exactly one tag"
 -    assert_equal users(:normal_user).id, checkrelation.user_id, 
 +    assert_equal changeset_id, checkrelation.changeset.id,
 +        "saved relation does not belong in the changeset it was assigned to"
 +    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
          "saved relation does not belong to user that created it"
      assert_equal true, checkrelation.visible, 
          "saved relation is not visible"
@@@ -182,14 -133,12 +182,14 @@@
      get :read, :id => relationid
      assert_response :success
  
 +    ###
      # create an relation with a way and a node as members
      nid = current_nodes(:used_node_1).id
      wid = current_ways(:used_way).id
 -    content "<osm><relation><member type='node' ref='#{nid}' role='some'/>" +
 -        "<member type='way' ref='#{wid}' role='other'/>" +
 -        "<tag k='test' v='yes' /></relation></osm>"
 +    content "<osm><relation changeset='#{changeset_id}'>" +
 +      "<member type='node' ref='#{nid}' role='some'/>" +
 +      "<member type='way' ref='#{wid}' role='other'/>" +
 +      "<tag k='test' v='yes' /></relation></osm>"
      put :create
      # hope for success
      assert_response :success, 
@@@ -204,9 -153,7 +204,9 @@@
          "saved relation does not have exactly two members"
      assert_equal checkrelation.tags.length, 1, 
          "saved relation does not contain exactly one tag"
 -    assert_equal users(:normal_user).id, checkrelation.user_id, 
 +    assert_equal changeset_id, checkrelation.changeset.id,
 +        "saved relation does not belong in the changeset it was assigned to"
 +    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
          "saved relation does not belong to user that created it"
      assert_equal true, checkrelation.visible, 
          "saved relation is not visible"
@@@ -223,45 -170,21 +223,45 @@@
    def test_create_invalid
      basic_authorization "test@openstreetmap.org", "test"
  
 +    # put the relation in a dummy fixture changset
 +    changeset_id = changesets(:normal_user_first_change).id
 +
      # create a relation with non-existing node as member
 -    content "<osm><relation><member type='node' ref='0'/><tag k='test' v='yes' /></relation></osm>"
 +    content "<osm><relation changeset='#{changeset_id}'>" +
 +      "<member type='node' ref='0'/><tag k='test' v='yes' />" +
 +      "</relation></osm>"
      put :create
      # expect failure
      assert_response :precondition_failed, 
          "relation upload with invalid node did not return 'precondition failed'"
    end
  
 +  # -------------------------------------
 +  # Test creating a relation, with some invalid XML
 +  # -------------------------------------
 +  def test_create_invalid_xml
 +    basic_authorization "test@openstreetmap.org", "test"
 +    
 +    # put the relation in a dummy fixture changeset that works
 +    changeset_id = changesets(:normal_user_first_change).id
 +    
 +    # create some xml that should return an error
 +    content "<osm><relation changeset='#{changeset_id}'>" +
 +    "<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" +
 +    "<tag k='tester' v='yep'/></relation></osm>"
 +    put :create
 +    # expect failure
 +    assert_response :bad_request
 +    assert_match(/Cannot parse valid relation from xml string/, @response.body)
 +    assert_match(/The type is not allowed only, /, @response.body)
 +  end
 +  
 +  
    # -------------------------------------
    # Test deleting relations.
    # -------------------------------------
    
    def test_delete
 -  return true
 -
      # first try to delete relation without auth
      delete :delete, :id => current_relations(:visible_relation).id
      assert_response :unauthorized
@@@ -269,279 -192,17 +269,278 @@@
      # now set auth
      basic_authorization("test@openstreetmap.org", "test");  
  
 -    # this should work
 +    # this shouldn't work, as we should need the payload...
 +    delete :delete, :id => current_relations(:visible_relation).id
 +    assert_response :bad_request
 +
 +    # try to delete without specifying a changeset
 +    content "<osm><relation id='#{current_relations(:visible_relation).id}'/></osm>"
 +    delete :delete, :id => current_relations(:visible_relation).id
 +    assert_response :bad_request
 +    assert_match(/You are missing the required changeset in the relation/, @response.body)
 +
 +    # try to delete with an invalid (closed) changeset
 +    content update_changeset(current_relations(:visible_relation).to_xml,
 +                             changesets(:normal_user_closed_change).id)
 +    delete :delete, :id => current_relations(:visible_relation).id
 +    assert_response :conflict
 +
 +    # try to delete with an invalid (non-existent) changeset
 +    content update_changeset(current_relations(:visible_relation).to_xml,0)
 +    delete :delete, :id => current_relations(:visible_relation).id
 +    assert_response :conflict
 +
 +    # this won't work because the relation is in-use by another relation
 +    content(relations(:used_relation).to_xml)
 +    delete :delete, :id => current_relations(:used_relation).id
 +    assert_response :precondition_failed, 
 +       "shouldn't be able to delete a relation used in a relation (#{@response.body})"
 +
 +    # this should work when we provide the appropriate payload...
 +    content(relations(:visible_relation).to_xml)
      delete :delete, :id => current_relations(:visible_relation).id
      assert_response :success
  
 +    # valid delete should return the new version number, which should
 +    # be greater than the old version number
 +    assert @response.body.to_i > current_relations(:visible_relation).version,
 +       "delete request should return a new version number for relation"
 +
      # this won't work since the relation is already deleted
 +    content(relations(:invisible_relation).to_xml)
      delete :delete, :id => current_relations(:invisible_relation).id
      assert_response :gone
  
 +    # this works now because the relation which was using this one 
 +    # has been deleted.
 +    content(relations(:used_relation).to_xml)
 +    delete :delete, :id => current_relations(:used_relation).id
 +    assert_response :success, 
 +       "should be able to delete a relation used in an old relation (#{@response.body})"
 +
      # this won't work since the relation never existed
      delete :delete, :id => 0
      assert_response :not_found
    end
  
 +  ##
 +  # 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.new
-     parser.string = xml
++    parser = XML::Parser.string(xml)
 +    parser.parse
 +  end
  end