Merge api06 branch to trunk.

app/controllers/api_controller.rb

@@ -11,12 +11,13 @@ class ApiController < ApplicationController
   @@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
@@ -84,6 +85,15 @@ class ApiController < ApplicationController
     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
@@ -109,18 +119,19 @@ class ApiController < ApplicationController
       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
 
@@ -176,15 +187,15 @@ class ApiController < ApplicationController
       end
     end 
 
-    relations = Relation.find_for_nodes(visible_nodes.keys, :conditions => "visible = 1") +
+    relations = Relation.find_for_nodes(visible_nodes.keys, :conditions => {:visible => true}) +
-                Relation.find_for_ways(way_ids, :conditions => "visible = 1")
+                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.
@@ -204,6 +215,8 @@ class ApiController < ApplicationController
     end
   end
 
+  # Get a list of the tiles that have changed within a specified time
+  # period
   def changes
     zoom = (params[:zoom] || '12').to_i
 
@@ -212,12 +225,12 @@ class ApiController < ApplicationController
       endtime = Time.parse(params[:end])
     else
       hours = (params[:hours] || '1').to_i.hours
-      endtime = Time.now
+      endtime = Time.now.getutc
       starttime = endtime - hours
     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],
@@ -245,21 +258,32 @@ class ApiController < ApplicationController
 
       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['minimum'] = "#{API_VERSION}";
-    version['maximum'] = '0.5';
+    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
 

app/controllers/application.rb

@@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base
 
   def authorize_web
     if session[:user]
-      @user = User.find(session[:user], :conditions => "visible = 1")
+      @user = User.find(session[:user], :conditions => {:visible => true})
     elsif session[:token]
       @user = User.authenticate(:token => session[:token])
       session[:user] = @user.id
@@ -22,7 +22,11 @@ class ApplicationController < ActionController::Base
     redirect_to :controller => 'user', :action => 'login', :referer => request.request_uri unless @user
   end
 
-  def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 
+  ##
+  # sets up the @user object for use by other methods. this is mostly called
+  # from the authorize method, but can be called elsewhere if authorisation
+  # is optional.
+  def setup_user_auth
     username, passwd = get_auth_data # parse from headers
     # authenticate per-scheme
     if username.nil?
@@ -32,6 +36,11 @@ class ApplicationController < ActionController::Base
     else
       @user = User.authenticate(:username => username, :password => passwd) # basic auth
     end
+  end
+
+  def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 
+    # make the @user object from any auth sources we have
+    setup_user_auth
 
     # handle authenticate pass/fail
     unless @user
@@ -73,13 +82,21 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def require_public_data
+    unless @user.data_public?
+      response.headers['Error'] = "You must make your edits public to upload new data"
+      render :nothing => true, :status => :forbidden
+      return false
+    end
+  end
+
   # Report and error to the user
   # (If anyone ever fixes Rails so it can set a http status "reason phrase",
   #  rather than only a status code and having the web engine make up a 
   #  phrase from that, we can also put the error message into the status
   #  message. For now, rails won't let us)
   def report_error(message)
-    render :nothing => true, :status => :bad_request
+    render :text => message, :status => :bad_request
     # Todo: some sort of escaping of problem characters in the message
     response.headers['Error'] = message
   end
@@ -90,6 +107,8 @@ private
   def get_auth_data 
     if request.env.has_key? 'X-HTTP_AUTHORIZATION'          # where mod_rewrite might have put it 
       authdata = request.env['X-HTTP_AUTHORIZATION'].to_s.split 
+    elsif request.env.has_key? 'REDIRECT_X_HTTP_AUTHORIZATION'          # mod_fcgi 
+      authdata = request.env['REDIRECT_X_HTTP_AUTHORIZATION'].to_s.split 
     elsif request.env.has_key? 'HTTP_AUTHORIZATION'         # regular location
       authdata = request.env['HTTP_AUTHORIZATION'].to_s.split
     end 

app/controllers/browse_controller.rb

@@ -7,12 +7,9 @@ class BrowseController < ApplicationController
   def start 
   end
   
-  def index
+  
-    @nodes = Node.find(:all, :order => "timestamp DESC", :limit=> 20)  
-  end
   
   def relation 
-    begin
     @relation = Relation.find(params[:id])
    
     @name = @relation.tags['name'].to_s 
@@ -24,12 +21,11 @@ class BrowseController < ApplicationController
     @next = Relation.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @relation.id }] ) 
     @prev = Relation.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @relation.id }] ) 
   rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
+    @type = "relation"
-    end
+    render :action => "not_found", :status => :not_found
   end
   
   def relation_history
-    begin
     @relation = Relation.find(params[:id])
      
     @name = @relation.tags['name'].to_s 
@@ -39,12 +35,11 @@ class BrowseController < ApplicationController
 	
     @title = 'Relation History | ' + (@name)
   rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
+    @type = "relation"
-    end
+    render :action => "not_found", :status => :not_found
   end
   
   def way 
-    begin
     @way = Way.find(params[:id])
      
     @name = @way.tags['name'].to_s 
@@ -56,12 +51,11 @@ class BrowseController < ApplicationController
     @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] ) 
     @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] ) 
   rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
+    @type = "way"
-    end
+    render :action => "not_found", :status => :not_found
   end
   
   def way_history 
-    begin
     @way = Way.find(params[:id])
      
     @name = @way.tags['name'].to_s 
@@ -71,12 +65,11 @@ class BrowseController < ApplicationController
 	
     @title = 'Way History | ' + (@name)
   rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
+    @type = "way"
-    end
+    render :action => "not_found", :status => :not_found
   end
 
   def node 
-    begin
     @node = Node.find(params[:id])
      
     @name = @node.tags_as_hash['name'].to_s 
@@ -88,12 +81,11 @@ class BrowseController < ApplicationController
     @next = Node.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @node.id }] ) 
     @prev = Node.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @node.id }] ) 
   rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
+    @type = "node"
-    end
+    render :action => "not_found", :status => :not_found
   end
   
   def node_history 
-    begin
     @node = Node.find(params[:id])
      
     @name = @node.tags_as_hash['name'].to_s 
@@ -103,7 +95,21 @@ class BrowseController < ApplicationController
 	
     @title = 'Node History | ' + (@name)
   rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
+    @type = "way"
+    render :action => "not_found", :status => :not_found
   end
+  
+  def changeset
+    @changeset = Changeset.find(params[:id])
+    @node_pages, @nodes = paginate(:old_nodes, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'node_page')
+    @way_pages, @ways = paginate(:old_ways, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'way_page')
+    @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page')
+      
+    @title = "Changeset | #{@changeset.id}"
+    @next = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id", { :id => @changeset.id }] ) 
+    @prev = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id", { :id => @changeset.id }] ) 
+  rescue ActiveRecord::RecordNotFound
+    @type = "changeset"
+    render :action => "not_found", :status => :not_found
   end
 end

app/controllers/changeset_controller.rb

@@ -0,0 +1,485 @@
+# The ChangesetController is the RESTful interface to Changeset objects
+
+class ChangesetController < ApplicationController
+  layout 'site'
+  require 'xml/libxml'
+
+  session :off, :except => [:list, :list_user, :list_bbox]
+  before_filter :authorize_web, :only => [:list, :list_user, :list_bbox]
+  before_filter :authorize, :only => [:create, :update, :delete, :upload, :include, :close]
+  before_filter :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close]
+  before_filter :check_api_writable, :only => [:create, :update, :delete, :upload, :include]
+  before_filter :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query]
+  after_filter :compress_output
+
+  # Help methods for checking boundary sanity and area size
+  include MapBoundary
+
+  # Helper methods for checking consistency
+  include ConsistencyValidations
+
+  # Create a changeset from XML.
+  def create
+    if request.put?
+      cs = Changeset.from_xml(request.raw_post, true)
+
+      if cs
+        cs.user_id = @user.id
+        cs.save_with_tags!
+        render :text => cs.id.to_s, :content_type => "text/plain"
+      else
+        render :nothing => true, :status => :bad_request
+      end
+    else
+      render :nothing => true, :status => :method_not_allowed
+    end
+  end
+
+  ##
+  # Return XML giving the basic info about the changeset. Does not 
+  # return anything about the nodes, ways and relations in the changeset.
+  def read
+    begin
+      changeset = Changeset.find(params[:id])
+      render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+    rescue ActiveRecord::RecordNotFound
+      render :nothing => true, :status => :not_found
+    end
+  end
+  
+  ##
+  # marks a changeset as closed. this may be called multiple times
+  # on the same changeset, so is idempotent.
+  def close 
+    unless request.put?
+      render :nothing => true, :status => :method_not_allowed
+      return
+    end
+    
+    changeset = Changeset.find(params[:id])    
+    check_changeset_consistency(changeset, @user)
+
+    # to close the changeset, we'll just set its closed_at time to
+    # now. this might not be enough if there are concurrency issues, 
+    # but we'll have to wait and see.
+    changeset.set_closed_time_now
+
+    changeset.save!
+    render :nothing => true
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+
+  ##
+  # insert a (set of) points into a changeset bounding box. this can only
+  # increase the size of the bounding box. this is a hint that clients can
+  # set either before uploading a large number of changes, or changes that
+  # the client (but not the server) knows will affect areas further away.
+  def expand_bbox
+    # only allow POST requests, because although this method is
+    # idempotent, there is no "document" to PUT really...
+    if request.post?
+      cs = Changeset.find(params[:id])
+      check_changeset_consistency(cs, @user)
+
+      # keep an array of lons and lats
+      lon = Array.new
+      lat = Array.new
+
+      # the request is in pseudo-osm format... this is kind-of an
+      # abuse, maybe should change to some other format?
+      doc = XML::Parser.string(request.raw_post).parse
+      doc.find("//osm/node").each do |n|
+        lon << n['lon'].to_f * GeoRecord::SCALE
+        lat << n['lat'].to_f * GeoRecord::SCALE
+      end
+
+      # add the existing bounding box to the lon-lat array
+      lon << cs.min_lon unless cs.min_lon.nil?
+      lat << cs.min_lat unless cs.min_lat.nil?
+      lon << cs.max_lon unless cs.max_lon.nil?
+      lat << cs.max_lat unless cs.max_lat.nil?
+
+      # collapse the arrays to minimum and maximum
+      cs.min_lon, cs.min_lat, cs.max_lon, cs.max_lat = 
+        lon.min, lat.min, lon.max, lat.max
+
+      # save the larger bounding box and return the changeset, which
+      # will include the bigger bounding box.
+      cs.save!
+      render :text => cs.to_xml.to_s, :content_type => "text/xml"
+
+    else
+      render :nothing => true, :status => :method_not_allowed
+    end
+
+  rescue LibXML::XML::Error, ArgumentError => ex
+    raise OSM::APIBadXMLError.new("osm", xml, ex.message)
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+
+  ##
+  # Upload a diff in a single transaction.
+  #
+  # This means that each change within the diff must succeed, i.e: that
+  # each version number mentioned is still current. Otherwise the entire
+  # transaction *must* be rolled back.
+  #
+  # Furthermore, each element in the diff can only reference the current
+  # changeset.
+  #
+  # Returns: a diffResult document, as described in 
+  # http://wiki.openstreetmap.org/index.php/OSM_Protocol_Version_0.6
+  def upload
+    # only allow POST requests, as the upload method is most definitely
+    # not idempotent, as several uploads with placeholder IDs will have
+    # different side-effects.
+    # see http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.2
+    unless request.post?
+      render :nothing => true, :status => :method_not_allowed
+      return
+    end
+
+    changeset = Changeset.find(params[:id])
+    check_changeset_consistency(changeset, @user)
+    
+    diff_reader = DiffReader.new(request.raw_post, changeset)
+    Changeset.transaction do
+      result = diff_reader.commit
+      render :text => result.to_s, :content_type => "text/xml"
+    end
+    
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+
+  ##
+  # download the changeset as an osmChange document.
+  #
+  # to make it easier to revert diffs it would be better if the osmChange
+  # format were reversible, i.e: contained both old and new versions of 
+  # modified elements. but it doesn't at the moment...
+  #
+  # this method cannot order the database changes fully (i.e: timestamp and
+  # version number may be too coarse) so the resulting diff may not apply
+  # to a different database. however since changesets are not atomic this 
+  # behaviour cannot be guaranteed anyway and is the result of a design
+  # choice.
+  def download
+    changeset = Changeset.find(params[:id])
+    
+    # get all the elements in the changeset and stick them in a big array.
+    elements = [changeset.old_nodes, 
+                changeset.old_ways, 
+                changeset.old_relations].flatten
+    
+    # sort the elements by timestamp and version number, as this is the 
+    # almost sensible ordering available. this would be much nicer if 
+    # global (SVN-style) versioning were used - then that would be 
+    # unambiguous.
+    elements.sort! do |a, b| 
+      if (a.timestamp == b.timestamp)
+        a.version <=> b.version
+      else
+        a.timestamp <=> b.timestamp 
+      end
+    end
+    
+    # create an osmChange document for the output
+    result = OSM::API.new.get_xml_doc
+    result.root.name = "osmChange"
+
+    # generate an output element for each operation. note: we avoid looking
+    # at the history because it is simpler - but it would be more correct to 
+    # check these assertions.
+    elements.each do |elt|
+      result.root <<
+        if (elt.version == 1) 
+          # first version, so it must be newly-created.
+          created = XML::Node.new "create"
+          created << elt.to_xml_node
+        else
+          # get the previous version from the element history
+          prev_elt = elt.class.find(:first, :conditions => 
+                                    ['id = ? and version = ?',
+                                     elt.id, elt.version])
+          unless elt.visible
+            # if the element isn't visible then it must have been deleted, so
+            # output the *previous* XML
+            deleted = XML::Node.new "delete"
+            deleted << prev_elt.to_xml_node
+          else
+            # must be a modify, for which we don't need the previous version
+            # yet...
+            modified = XML::Node.new "modify"
+            modified << elt.to_xml_node
+          end
+        end
+    end
+
+    render :text => result.to_s, :content_type => "text/xml"
+            
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+
+  ##
+  # query changesets by bounding box, time, user or open/closed status.
+  def query
+    # create the conditions that the user asked for. some or all of
+    # these may be nil.
+    conditions = conditions_bbox(params['bbox'])
+    conditions = cond_merge conditions, conditions_user(params['user'])
+    conditions = cond_merge conditions, conditions_time(params['time'])
+    conditions = cond_merge conditions, conditions_open(params['open'])
+    conditions = cond_merge conditions, conditions_closed(params['closed'])
+
+    # create the results document
+    results = OSM::API.new.get_xml_doc
+
+    # add all matching changesets to the XML results document
+    Changeset.find(:all, 
+                   :conditions => conditions, 
+                   :limit => 100,
+                   :order => 'created_at desc').each do |cs|
+      results.root << cs.to_xml_node
+    end
+
+    render :text => results.to_s, :content_type => "text/xml"
+
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+  
+  ##
+  # updates a changeset's tags. none of the changeset's attributes are
+  # user-modifiable, so they will be ignored.
+  #
+  # changesets are not (yet?) versioned, so we don't have to deal with
+  # history tables here. changesets are locked to a single user, however.
+  #
+  # after succesful update, returns the XML of the changeset.
+  def update
+    # request *must* be a PUT.
+    unless request.put?
+      render :nothing => true, :status => :method_not_allowed
+      return
+    end
+    
+    changeset = Changeset.find(params[:id])
+    new_changeset = Changeset.from_xml(request.raw_post)
+
+    unless new_changeset.nil?
+      check_changeset_consistency(changeset, @user)
+      changeset.update_from(new_changeset, @user)
+      render :text => changeset.to_xml, :mime_type => "text/xml"
+    else
+      
+      render :nothing => true, :status => :bad_request
+    end
+      
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+
+  
+  
+  ##
+  # list edits (open changesets) in reverse chronological order
+  def list
+    conditions = conditions_nonempty
+    
+    
+   # @changesets = Changeset.find(:all, :order => "closed_at DESC", :conditions => ['closed_at < ?', DateTime.now], :limit=> 20)
+   
+   
+   #@edit_pages, @edits = paginate(:changesets,
+   #                                :include => [:user, :changeset_tags],
+   #                                :conditions => conditions,
+   #                                :order => "changesets.created_at DESC",
+   #                                :per_page => 20)
+   #
+    
+   @edits =  Changeset.find(:all,
+                                   :order => "changesets.created_at DESC",
+                                   :conditions => conditions,
+                                   :limit => 20)
+    
+  end
+  
+  ##
+  # list edits (changesets) belonging to a user
+  def list_user
+    user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
+    
+    if user
+      @display_name = user.display_name
+      if not user.data_public? and @user != user
+        @edits = nil
+        render
+      else
+        conditions = cond_merge conditions, ['user_id = ?', user.id]
+        conditions = cond_merge conditions, conditions_nonempty
+        @edit_pages, @edits = paginate(:changesets,
+                                        :include => [:user, :changeset_tags],
+                                        :conditions => conditions,
+                                        :order => "changesets.created_at DESC",
+                                        :per_page => 20)
+      end
+    else
+      @not_found_user = params[:display_name]
+      render :template => 'user/no_such_user', :status => :not_found
+    end
+  end
+  
+  ##
+  # list changesets in a bbox
+  def list_bbox
+    # support 'bbox' param or alternatively 'minlon', 'minlat' etc	  
+    if params['bbox']
+       bbox = params['bbox']
+    elsif params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat']
+       bbox = h(params['minlon']) + ',' + h(params['minlat']) + ',' + h(params['maxlon']) + ',' + h(params['maxlat'])
+    else
+      #TODO: fix bugs in location determination for history tab (and other tabs) then uncomment this redirect
+      #redirect_to :action => 'list'
+    end
+       
+    conditions = conditions_bbox(bbox);
+    conditions = cond_merge conditions, conditions_nonempty
+    
+    @edit_pages, @edits = paginate(:changesets,
+                                   :include => [:user, :changeset_tags],
+                                   :conditions => conditions,
+                                   :order => "changesets.created_at DESC",
+                                   :per_page => 20)
+                                   
+    @bbox = sanitise_boundaries(bbox.split(/,/)) unless bbox==nil
+  end
+  
+private
+  #------------------------------------------------------------
+  # utility functions below.
+  #------------------------------------------------------------  
+
+  ##
+  # merge two conditions
+  def cond_merge(a, b)
+    if a and b
+      a_str = a.shift
+      b_str = b.shift
+      return [ a_str + " AND " + b_str ] + a + b
+    elsif a 
+      return a
+    else b
+      return b
+    end
+  end
+
+  ##
+  # if a bounding box was specified then parse it and do some sanity 
+  # checks. this is mostly the same as the map call, but without the 
+  # area restriction.
+  def conditions_bbox(bbox)
+    unless bbox.nil?
+      raise OSM::APIBadUserInput.new("Bounding box should be min_lon,min_lat,max_lon,max_lat") unless bbox.count(',') == 3
+      bbox = sanitise_boundaries(bbox.split(/,/))
+      raise OSM::APIBadUserInput.new("Minimum longitude should be less than maximum.") unless bbox[0] <= bbox[2]
+      raise OSM::APIBadUserInput.new("Minimum latitude should be less than maximum.") unless bbox[1] <= bbox[3]
+      return ['min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?',
+              bbox[2] * GeoRecord::SCALE, bbox[0] * GeoRecord::SCALE, bbox[3]* GeoRecord::SCALE, bbox[1] * GeoRecord::SCALE]
+    else
+      return nil
+    end
+  end
+
+  ##
+  # restrict changesets to those by a particular user
+  def conditions_user(user)
+    unless user.nil?
+      # user input checking, we don't have any UIDs < 1
+      raise OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1
+
+      u = User.find(user.to_i)
+      # should be able to get changesets of public users only, or 
+      # our own changesets regardless of public-ness.
+      unless u.data_public?
+        # get optional user auth stuff so that users can see their own
+        # changesets if they're non-public
+        setup_user_auth
+        
+        raise OSM::APINotFoundError if @user.nil? or @user.id != u.id
+      end
+      return ['user_id = ?', u.id]
+    else
+      return nil
+    end
+  end
+
+  ##
+  # restrict changes to those closed during a particular time period
+  def conditions_time(time) 
+    unless time.nil?
+      # if there is a range, i.e: comma separated, then the first is 
+      # low, second is high - same as with bounding boxes.
+      if time.count(',') == 1
+        # check that we actually have 2 elements in the array
+        times = time.split(/,/)
+        raise OSM::APIBadUserInput.new("bad time range") if times.size != 2 
+
+        from, to = times.collect { |t| DateTime.parse(t) }
+        return ['closed_at >= ? and created_at <= ?', from, to]
+      else
+        # if there is no comma, assume its a lower limit on time
+        return ['closed_at >= ?', DateTime.parse(time)]
+      end
+    else
+      return nil
+    end
+    # stupid DateTime seems to throw both of these for bad parsing, so
+    # we have to catch both and ensure the correct code path is taken.
+  rescue ArgumentError => ex
+    raise OSM::APIBadUserInput.new(ex.message.to_s)
+  rescue RuntimeError => ex
+    raise OSM::APIBadUserInput.new(ex.message.to_s)
+  end
+
+  ##
+  # return changesets which are open (haven't been closed yet)
+  # we do this by seeing if the 'closed at' time is in the future. Also if we've
+  # hit the maximum number of changes then it counts as no longer open.
+  # if parameter 'open' is nill then open and closed changsets are returned
+  def conditions_open(open)
+    return open.nil? ? nil : ['closed_at >= ? and num_changes <= ?', 
+                              Time.now.getutc, Changeset::MAX_ELEMENTS]
+  end
+  
+  ##
+  # query changesets which are closed
+  # ('closed at' time has passed or changes limit is hit)
+  def conditions_closed(closed)
+    return closed.nil? ? nil : ['closed_at < ? and num_changes > ?', 
+                                Time.now.getutc, Changeset::MAX_ELEMENTS]
+  end
+
+  ##
+  # eliminate empty changesets (where the bbox has not been set)
+  # this should be applied to all changeset list displays
+  def conditions_nonempty()
+    return ['min_lat IS NOT NULL']
+  end
+  
+end

app/controllers/changeset_tag_controller.rb

@@ -0,0 +1,9 @@
+class ChangesetTagController < ApplicationController
+  layout 'site'
+
+  def search
+    @tags = ChangesetTag.find(:all, :limit => 11, :conditions => ["match(v) against (?)", params[:query][:query].to_s] )
+  end
+
+
+end

app/controllers/diary_entry_controller.rb

@@ -39,6 +39,8 @@ class DiaryEntryController < ApplicationController
         redirect_to :controller => 'diary_entry', :action => 'view', :id => params[:id]
       end
     end
+  rescue ActiveRecord::RecordNotFound
+    render :action => "no_such_entry", :status => :not_found
   end
 
   def comment
@@ -55,7 +57,7 @@ class DiaryEntryController < ApplicationController
 
   def list
     if params[:display_name]
-      @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 + "'s diary"
@@ -71,7 +73,7 @@ class DiaryEntryController < ApplicationController
     else
       @title = "Users' diaries"
       @entry_pages, @entries = paginate(:diary_entries, :include => :user,
-                                        :conditions => "users.visible = 1",
+                                        :conditions => ["users.visible = ?", true],
                                         :order => 'created_at DESC',
                                         :per_page => 20)
     end
@@ -79,13 +81,13 @@ class DiaryEntryController < ApplicationController
 
   def rss
     if params[:display_name]
-      user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1")
+      user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
 
       if user
         @entries = DiaryEntry.find(:all, :conditions => ['user_id = ?', user.id], :order => 'created_at DESC', :limit => 20)
         @title = "OpenStreetMap diary entries for #{user.display_name}"
         @description = "Recent OpenStreetmap diary entries from #{user.display_name}"
-        @link = "http://www.openstreetmap.org/user/#{user.display_name}/diary"
+        @link = "http://#{SERVER_URL}/user/#{user.display_name}/diary"
 
         render :content_type => Mime::RSS
       else
@@ -93,21 +95,22 @@ class DiaryEntryController < ApplicationController
       end
     else
       @entries = DiaryEntry.find(:all, :include => :user,
-                                 :conditions => "users.visible = 1",
+                                 :conditions => ["users.visible = ?", true],
                                  :order => 'created_at DESC', :limit => 20)
       @title = "OpenStreetMap diary entries"
       @description = "Recent diary entries from users of OpenStreetMap"
-      @link = "http://www.openstreetmap.org/diary"
+      @link = "http://#{SERVER_URL}/diary"
 
       render :content_type => Mime::RSS
     end
   end
 
   def view
-    user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1")
+    user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true})
 
     if user
       @entry = DiaryEntry.find(:first, :conditions => ['user_id = ? AND id = ?', user.id, params[:id]])
+      @title = "Users' diaries | #{params[:display_name]}"
     else
       @not_found_user = params[:display_name]
 

app/controllers/export_controller.rb

@@ -2,18 +2,24 @@ class ExportController < ApplicationController
   def start
   end
 
+  #When the user clicks 'Export' we redirect to a URL which generates the export download
   def finish
     bbox = BoundingBox.new(params[:minlon], params[:minlat], params[:maxlon], params[:maxlat])
     format = params[:format]
 
     if format == "osm"
+      #redirect to API map get
       redirect_to "http://api.openstreetmap.org/api/#{API_VERSION}/map?bbox=#{bbox}"
+
     elsif format == "mapnik"
+      #redirect to a special 'export' cgi script
       format = params[:mapnik_format]
       scale = params[:mapnik_scale]
 
       redirect_to "http://tile.openstreetmap.org/cgi-bin/export?bbox=#{bbox}&scale=#{scale}&format=#{format}"
+
     elsif format == "osmarender"
+      #redirect to the t@h 'MapOf' service
       format = params[:osmarender_format]
       zoom = params[:osmarender_zoom].to_i
       width = bbox.slippy_width(zoom).to_i

app/controllers/message_controller.rb

@@ -6,13 +6,18 @@ class MessageController < ApplicationController
   before_filter :check_database_readable
   before_filter :check_database_writable, :only => [:new, :reply, :mark]
 
+  # Allow the user to write a new message to another user. This action also 
+  # deals with the sending of that message to the other user when the user
+  # clicks send.
+  # The user_id param is the id of the user that the message is being sent to.
   def new
     @title = 'send message'
+    @to_user = User.find(params[:user_id])
     if params[:message]
       @message = Message.new(params[:message])
-      @message.to_user_id = params[:user_id]
+      @message.to_user_id = @to_user.id
       @message.from_user_id = @user.id
-      @message.sent_on = Time.now
+      @message.sent_on = Time.now.getutc
    
       if @message.save
         flash[:notice] = 'Message sent'
@@ -22,27 +27,32 @@ class MessageController < ApplicationController
     else
       @title = params[:title]
     end
+  rescue ActiveRecord::RecordNotFound
+    render :action => 'no_such_user', :status => :not_found
   end
 
+  # Allow the user to reply to another message.
   def reply
     message = Message.find(params[:message_id], :conditions => ["to_user_id = ? or from_user_id = ?", @user.id, @user.id ])
     @body = "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}" 
     @title = "Re: #{message.title.sub(/^Re:\s*/, '')}"
-    @user_id = message.from_user_id
+    @to_user = User.find(message.from_user_id)
     render :action => 'new'
   rescue ActiveRecord::RecordNotFound
-    render :nothing => true, :status => :not_found
+    render :action => 'no_such_user', :status => :not_found
   end
 
+  # Show a message
   def read
     @title = 'read message'
     @message = Message.find(params[:message_id], :conditions => ["to_user_id = ? or from_user_id = ?", @user.id, @user.id ])
-    @message.message_read = 1 if @message.to_user_id == @user.id
+    @message.message_read = true if @message.to_user_id == @user.id
     @message.save
   rescue ActiveRecord::RecordNotFound
-    render :nothing => true, :status => :not_found
+    render :action => 'no_such_user', :status => :not_found
   end
 
+  # Display the list of messages that have been sent to the user.
   def inbox
     @title = 'inbox'
     if @user and params[:display_name] == @user.display_name
@@ -51,6 +61,7 @@ class MessageController < ApplicationController
     end
   end
 
+  # Display the list of messages that the user has sent to other users.
   def outbox
     @title = 'outbox'
     if @user and params[:display_name] == @user.display_name
@@ -59,15 +70,16 @@ class MessageController < ApplicationController
     end
   end
 
+  # Set the message as being read or unread.
   def mark
     if params[:message_id]
       id = params[:message_id]
       message = Message.find_by_id(id)
       if params[:mark] == 'unread'
-        message_read = 0 
+        message_read = false 
         mark_type = 'unread'
       else
-        message_read = 1
+        message_read = true
         mark_type = 'read'
       end
       message.message_read = message_read
@@ -76,5 +88,7 @@ class MessageController < ApplicationController
         redirect_to :controller => 'message', :action => 'inbox', :display_name => @user.display_name
       end
     end
+  rescue ActiveRecord::RecordNotFound
+    render :action => 'no_such_user', :status => :not_found
   end
 end

app/controllers/node_controller.rb

@@ -5,20 +5,19 @@ class NodeController < ApplicationController
 
   session :off
   before_filter :authorize, :only => [:create, :update, :delete]
+  before_filter :require_public_data, :only => [:create, :update, :delete]
   before_filter :check_api_writable, :only => [:create, :update, :delete]
   before_filter :check_api_readable, :except => [:create, :update, :delete]
   after_filter :compress_output
 
   # Create a node from XML.
   def create
+    begin
       if request.put?
         node = Node.from_xml(request.raw_post, true)
 
         if node
-        node.user_id = @user.id
+          node.create_with_history @user
-        node.visible = true
-        node.save_with_history!
-
           render :text => node.id.to_s, :content_type => "text/plain"
         else
           render :nothing => true, :status => :bad_request
@@ -26,13 +25,16 @@ class NodeController < ApplicationController
       else
         render :nothing => true, :status => :method_not_allowed
       end
+    rescue OSM::APIError => ex
+      render ex.render_opts
+    end
   end
 
   # Dump the details on a node given in params[:id]
   def read
     begin
       node = Node.find(params[:id])
-      if node.visible
+      if node.visible?
         response.headers['Last-Modified'] = node.timestamp.rfc822
         render :text => node.to_xml.to_s, :content_type => "text/xml"
        else
@@ -50,49 +52,40 @@ class NodeController < ApplicationController
       new_node = Node.from_xml(request.raw_post)
 
       if new_node and new_node.id == node.id
-        node.user_id = @user.id
+        node.update_from(new_node, @user)
-        node.latitude = new_node.latitude 
+        render :text => node.version.to_s, :content_type => "text/plain"
-        node.longitude = new_node.longitude
+      else
-        node.tags = new_node.tags
+        render :nothing => true, :status => :bad_request
-        node.visible = true
+      end
-        node.save_with_history!
+    rescue OSM::APIError => ex
+      render ex.render_opts
+    rescue ActiveRecord::RecordNotFound
+      render :nothing => true, :status => :not_found
+    end
+  end
 
-        render :nothing => true
+  # Delete a node. Doesn't actually delete it, but retains its history 
+  # in a wiki-like way. We therefore treat it like an update, so the delete
+  # method returns the new version number.
+  def delete
+    begin
+      node = Node.find(params[:id])
+      new_node = Node.from_xml(request.raw_post)
+      
+      if new_node and new_node.id == node.id
+        node.delete_with_history!(new_node, @user)
+        render :text => node.version.to_s, :content_type => "text/plain"
       else
         render :nothing => true, :status => :bad_request
       end
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
+    rescue OSM::APIError => ex
+      render ex.render_opts
     end
   end
 
-  # Delete a node. Doesn't actually delete it, but retains its history in a wiki-like way.
+  # Dump the details on many nodes whose ids are given in the "nodes" parameter.
-  # FIXME remove all the fricking SQL
-  def delete
-    begin
-      node = Node.find(params[:id])
-
-      if node.visible
-        if WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", :conditions => [ "current_ways.visible = 1 AND current_way_nodes.node_id = ?", node.id ])
-          render :text => "", :status => :precondition_failed
-        elsif RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='node' and member_id=?", params[:id]])
-          render :text => "", :status => :precondition_failed
-        else
-          node.user_id = @user.id
-          node.visible = 0
-          node.save_with_history!
-
-          render :nothing => true
-        end
-      else
-        render :text => "", :status => :gone
-      end
-    rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
-    end
-  end
-
-  # WTF does this do?
   def nodes
     ids = params['nodes'].split(',').collect { |n| n.to_i }
 

app/controllers/old_node_controller.rb

@@ -22,4 +22,21 @@ class OldNodeController < ApplicationController
       render :nothing => true, :status => :internal_server_error
     end
   end
+  
+  def version
+    begin
+      old_node = OldNode.find(:first, :conditions => {:id => params[:id], :version => params[:version]} )
+      
+      response.headers['Last-Modified'] = old_node.timestamp.rfc822
+
+      doc = OSM::API.new.get_xml_doc
+      doc.root << old_node.to_xml_node
+
+      render :text => doc.to_s, :content_type => "text/xml"
+    rescue ActiveRecord::RecordNotFound
+      render :nothing => true, :status => :not_found
+    rescue
+      render :nothing => true, :status => :internal_server_error
+    end
+  end
 end

app/controllers/old_relation_controller.rb

@@ -2,6 +2,7 @@ class OldRelationController < ApplicationController
   require 'xml/libxml'
 
   session :off
+  before_filter :check_api_readable
   after_filter :compress_output
 
   def history
@@ -20,4 +21,21 @@ class OldRelationController < ApplicationController
       render :nothing => true, :status => :internal_server_error
     end
   end
+  
+  def version
+    begin
+      old_relation = OldRelation.find(:first, :conditions => {:id => params[:id], :version => params[:version]} )
+      
+      response.headers['Last-Modified'] = old_relation.timestamp.rfc822
+
+      doc = OSM::API.new.get_xml_doc
+      doc.root << old_relation.to_xml_node
+
+      render :text => doc.to_s, :content_type => "text/xml"
+    rescue ActiveRecord::RecordNotFound
+      render :nothing => true, :status => :not_found
+    rescue
+      render :nothing => true, :status => :internetal_service_error
+    end
+  end
 end

app/controllers/old_way_controller.rb

@@ -22,4 +22,21 @@ class OldWayController < ApplicationController
       render :nothing => true, :status => :internal_server_error
     end
   end
+  
+  def version
+    begin
+      old_way = OldWay.find(:first, :conditions => {:id => params[:id], :version => params[:version]} )
+      
+      response.headers['Last-Modified'] = old_way.timestamp.rfc822
+      
+      doc = OSM::API.new.get_xml_doc
+      doc.root << old_way.to_xml_node
+      
+      render :text => doc.to_s, :content_type => "text/xml"
+    rescue ActiveRecord::RecordNotFound
+      render :nothing => true, :status => :not_found
+    rescue
+      render :nothing => true, :status => :internal_server_error
+    end
+  end
 end

app/controllers/relation_controller.rb

@@ -3,29 +3,30 @@ class RelationController < ApplicationController
 
   session :off
   before_filter :authorize, :only => [:create, :update, :delete]
+  before_filter :require_public_data, :only => [:create, :update, :delete]
   before_filter :check_api_writable, :only => [:create, :update, :delete]
   before_filter :check_api_readable, :except => [:create, :update, :delete]
   after_filter :compress_output
 
   def create
+    begin
       if request.put?
         relation = Relation.from_xml(request.raw_post, true)
 
-      if relation
+        # We assume that an exception has been thrown if there was an error 
-        if !relation.preconditions_ok?
+        # generating the relation
-          render :text => "", :status => :precondition_failed
+        #if relation
-        else
+          relation.create_with_history @user
-          relation.user_id = @user.id
-          relation.save_with_history!
-
           render :text => relation.id.to_s, :content_type => "text/plain"
-        end
+        #else
-      else
+         # render :text => "Couldn't get turn the input into a relation.", :status => :bad_request
-        render :nothing => true, :status => :bad_request
+        #end
-      end
       else
         render :nothing => true, :status => :method_not_allowed
       end
+    rescue OSM::APIError => ex
+      render ex.render_opts
+    end
   end
 
   def read
@@ -45,56 +46,38 @@ class RelationController < ApplicationController
   end
 
   def update
+    logger.debug request.raw_post
     begin
       relation = Relation.find(params[:id])
       new_relation = Relation.from_xml(request.raw_post)
 
       if new_relation and new_relation.id == relation.id
-        if !new_relation.preconditions_ok?
+        relation.update_from new_relation, @user
-          render :text => "", :status => :precondition_failed
+        render :text => relation.version.to_s, :content_type => "text/plain"
-        else
-          relation.user_id = @user.id
-          relation.tags = new_relation.tags
-          relation.members = new_relation.members
-          relation.visible = true
-          relation.save_with_history!
-
-          render :nothing => true
-        end
       else
         render :nothing => true, :status => :bad_request
       end
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
-    rescue
+    rescue OSM::APIError => ex
-      render :nothing => true, :status => :internal_server_error
+      render ex.render_opts
     end
   end
 
   def delete
-#XXX check if member somewhere!
     begin
       relation = Relation.find(params[:id])
-
+      new_relation = Relation.from_xml(request.raw_post)
-      if relation.visible
+      if new_relation and new_relation.id == relation.id
-        if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='relation' and member_id=?", params[:id]])
+        relation.delete_with_history!(new_relation, @user)
-          render :text => "", :status => :precondition_failed
+        render :text => relation.version.to_s, :content_type => "text/plain"
       else
-          relation.user_id = @user.id
+        render :nothing => true, :status => :bad_request
-          relation.tags = []
-          relation.members = []
-          relation.visible = false
-          relation.save_with_history!
-
-          render :nothing => true
-        end
-      else
-        render :text => "", :status => :gone
       end
+    rescue OSM::APIError => ex
+      render ex.render_opts
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
-    rescue
-      render :nothing => true, :status => :internal_server_error
     end
   end
 
@@ -115,12 +98,12 @@ class RelationController < ApplicationController
         # first collect nodes, ways, and relations referenced by this relation.
         
         ways = Way.find_by_sql("select w.* from current_ways w,current_relation_members rm where "+
-            "rm.member_type='way' and rm.member_id=w.id and rm.id=#{relation.id}");
+            "rm.member_type='Way' and rm.member_id=w.id and rm.id=#{relation.id}");
         nodes = Node.find_by_sql("select n.* from current_nodes n,current_relation_members rm where "+
-            "rm.member_type='node' and rm.member_id=n.id and rm.id=#{relation.id}");
+            "rm.member_type='Node' and rm.member_id=n.id and rm.id=#{relation.id}");
         # note query is built to exclude self just in case.
         relations = Relation.find_by_sql("select r.* from current_relations r,current_relation_members rm where "+
-            "rm.member_type='relation' and rm.member_id=r.id and rm.id=#{relation.id} and r.id<>rm.id");
+            "rm.member_type='Relation' and rm.member_id=r.id and rm.id=#{relation.id} and r.id<>rm.id");
 
         # now additionally collect nodes referenced by ways. Note how we recursively 
         # evaluate ways but NOT relations.
@@ -160,8 +143,7 @@ class RelationController < ApplicationController
         render :text => doc.to_s, :content_type => "text/xml"
 
       else
-
+        render :nothing => true, :status => :gone
-        render :text => "", :status => :gone
       end
 
     rescue ActiveRecord::RecordNotFound
@@ -184,27 +166,29 @@ class RelationController < ApplicationController
 
       render :text => doc.to_s, :content_type => "text/xml"
     else
-      render :nothing => true, :status => :bad_request
+      render :text => "You need to supply a comma separated list of ids.", :status => :bad_request
     end
+  rescue ActiveRecord::RecordNotFound
+    render :text => "Could not find one of the relations", :status => :not_found
   end
 
   def relations_for_way
-    relations_for_object("way")
+    relations_for_object("Way")
   end
   def relations_for_node
-    relations_for_object("node")
+    relations_for_object("Node")
   end
   def relations_for_relation
-    relations_for_object("relation")
+    relations_for_object("Relation")
   end
 
   def relations_for_object(objtype)
-    relationids = RelationMember.find(:all, :conditions => ['member_type=? and member_id=?', objtype, params[:id]]).collect { |ws| ws.id }.uniq
+    relationids = RelationMember.find(:all, :conditions => ['member_type=? and member_id=?', objtype, params[:id]]).collect { |ws| ws.id[0] }.uniq
 
     doc = OSM::API.new.get_xml_doc
 
     Relation.find(relationids).each do |relation|
-      doc.root << relation.to_xml_node
+      doc.root << relation.to_xml_node if relation.visible
     end
 
     render :text => doc.to_s, :content_type => "text/xml"

app/controllers/trace_controller.rb

@@ -2,7 +2,7 @@ class TraceController < ApplicationController
   layout 'site'
 
   before_filter :authorize_web
-  before_filter :require_user, :only => [:mine, :edit, :delete, :make_public]
+  before_filter :require_user, :only => [:mine, :create, :edit, :delete, :make_public]
   before_filter :authorize, :only => [:api_details, :api_data, :api_create]
   before_filter :check_database_readable, :except => [:api_details, :api_data, :api_create]
   before_filter :check_database_writable, :only => [:create, :edit, :delete, :make_public]
@@ -15,7 +15,12 @@ class TraceController < ApplicationController
     # from display name, pick up user id if one user's traces only
     display_name = params[:display_name]
     if target_user.nil? and !display_name.blank?
-      target_user = User.find(:first, :conditions => [ "visible = 1 and display_name = ?", display_name])
+      target_user = User.find(:first, :conditions => [ "visible = ? and display_name = ?", true, display_name])
+      if target_user.nil?
+        @not_found_user = display_name
+        render :action => 'no_such_user', :status => :not_found
+        return
+      end
     end
 
     # set title
@@ -36,15 +41,15 @@ class TraceController < ApplicationController
     # 4 - user's traces, not logged in as that user = all user's public traces
     if target_user.nil? # all traces
       if @user
-        conditions = ["(gpx_files.public = 1 OR gpx_files.user_id = ?)", @user.id] #1
+        conditions = ["(gpx_files.public = ? OR gpx_files.user_id = ?)", true, @user.id] #1
       else
-        conditions  = ["gpx_files.public = 1"] #2
+        conditions  = ["gpx_files.public = ?", true] #2
       end
     else
       if @user and @user == target_user
         conditions = ["gpx_files.user_id = ?", @user.id] #3 (check vs user id, so no join + can't pick up non-public traces by changing name)
       else
-        conditions = ["gpx_files.public = 1 AND gpx_files.user_id = ?", target_user.id] #4
+        conditions = ["gpx_files.public = ? AND gpx_files.user_id = ?", true, target_user.id] #4
       end
     end
     
@@ -55,7 +60,8 @@ class TraceController < ApplicationController
       conditions[0] += " AND gpx_files.id IN (#{files.join(',')})"
     end
     
-    conditions[0] += " AND gpx_files.visible = 1"
+    conditions[0] += " AND gpx_files.visible = ?"
+    conditions << true
 
     @trace_pages, @traces = paginate(:traces,
                                      :include => [:user, :tags],
@@ -100,6 +106,7 @@ class TraceController < ApplicationController
   end
 
   def create
+    if params[:trace]
       logger.info(params[:trace][:gpx_file].class.name)
       if params[:trace][:gpx_file].respond_to?(:read)
         do_create(params[:trace][:gpx_file], params[:trace][:tagstring],
@@ -117,11 +124,12 @@ class TraceController < ApplicationController
                             :description => params[:trace][:description],
                             :public => params[:trace][:public],
                             :inserted => false, :user => @user,
-                          :timestamp => Time.now})
+                            :timestamp => Time.now.getutc})
         @trace.valid?
         @trace.errors.add(:gpx_file, "can't be blank")
       end
     end
+  end
 
   def data
     trace = Trace.find(params[:id])
@@ -196,7 +204,7 @@ class TraceController < ApplicationController
   end
 
   def georss
-    conditions = ["gpx_files.public = 1"]
+    conditions = ["gpx_files.public = ?", true]
 
     if params[:display_name]
       conditions[0] += " AND users.display_name = ?"
@@ -278,7 +286,12 @@ class TraceController < ApplicationController
 
   def api_create
     if request.post?
-      do_create(params[:file], params[:tags], params[:description], params[:public])
+      tags = params[:tags] || ""
+      description = params[:description] || ""
+      pub = params[:public] || false
+      
+      if params[:file].respond_to?(:read)
+        do_create(params[:file], tags, description, pub)
 
         if @trace.id
           render :text => @trace.id.to_s, :content_type => "text/plain"
@@ -287,6 +300,9 @@ class TraceController < ApplicationController
         else
           render :nothing => true, :status => :bad_request
         end
+      else
+        render :nothing => true, :status => :bad_request
+      end
     else
       render :nothing => true, :status => :method_not_allowed
     end
@@ -313,7 +329,7 @@ private
       :public => public,
       :inserted => true,
       :user => @user,
-      :timestamp => Time.now
+      :timestamp => Time.now.getutc
     })
 
     # Save the trace object
@@ -328,6 +344,17 @@ private
       # Remove the file as we have failed to update the database
       FileUtils.rm_f(filename)
     end
+    
+    # Finally save whether the user marked the trace as being public
+    if @trace.public?
+      if @user.trace_public_default.nil?
+        @user.preferences.create(:k => "gps.trace.public", :v => "default")
+      end
+    else
+      pref = @user.trace_public_default
+      pref.destroy unless pref.nil?
+    end
+    
   end
 
 end

app/controllers/user_controller.rb

@@ -83,7 +83,7 @@ class UserController < ApplicationController
   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
@@ -120,9 +120,21 @@ class UserController < ApplicationController
 
   def new
     @title = 'create account'
+    # The user is logged in already, so don't show them the signup page, instead
+    # send them to the home page
+    redirect_to :controller => 'site', :action => 'index' if session[:user]
   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]
@@ -223,7 +235,7 @@ class UserController < ApplicationController
   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
@@ -236,7 +248,7 @@ class UserController < ApplicationController
   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
@@ -258,7 +270,7 @@ class UserController < ApplicationController
   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."

app/controllers/user_preference_controller.rb

@@ -5,11 +5,9 @@ class UserPreferenceController < ApplicationController
   def read_one
     pref = UserPreference.find(@user.id, params[:preference_key])
 
-    if pref
     render :text => pref.v.to_s
-    else
+  rescue ActiveRecord::RecordNotFound => ex
-      render :text => 'OH NOES! PREF NOT FOUND!', :status => 404
+    render :text => 'OH NOES! PREF NOT FOUND!', :status => :not_found
-    end
   end
 
   def update_one
@@ -32,6 +30,8 @@ class UserPreferenceController < ApplicationController
     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
@@ -54,6 +54,9 @@ class UserPreferenceController < ApplicationController
   def update
     begin
       p = XML::Parser.string(request.raw_post)
+    rescue LibXML::XML::Error, ArgumentError => ex
+      raise OSM::APIBadXMLError.new("preferences", xml, ex.message)
+    end
     doc = p.parse
 
     prefs = []
@@ -65,7 +68,6 @@ class UserPreferenceController < ApplicationController
 
       unless keyhash[pt['k']].nil? # already have that key
         render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable
-          return
       end
 
       keyhash[pt['k']] = 1
@@ -78,7 +80,6 @@ class UserPreferenceController < ApplicationController
 
     if prefs.size > 150
       render :text => 'Too many preferences', :status => :request_entity_too_large
-        return
     end
 
     # kill the existing ones
@@ -88,12 +89,9 @@ class UserPreferenceController < ApplicationController
     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
-      return
-    end
-
-    render :nothing => true
   end
 end

app/controllers/way_controller.rb

@@ -3,29 +3,29 @@ class WayController < ApplicationController
 
   session :off
   before_filter :authorize, :only => [:create, :update, :delete]
+  before_filter :require_public_data, :only => [:create, :update, :delete]
   before_filter :check_api_writable, :only => [:create, :update, :delete]
   before_filter :check_api_readable, :except => [:create, :update, :delete]
   after_filter :compress_output
 
   def create
+    begin
       if request.put?
         way = Way.from_xml(request.raw_post, true)
 
         if way
-        if !way.preconditions_ok?
+          way.create_with_history @user
-          render :text => "", :status => :precondition_failed
-        else
-          way.user_id = @user.id
-          way.save_with_history!
-
           render :text => way.id.to_s, :content_type => "text/plain"
-        end
         else
           render :nothing => true, :status => :bad_request
         end
       else
         render :nothing => true, :status => :method_not_allowed
       end
+    rescue OSM::APIError => ex
+      logger.warn request.raw_post
+      render ex.render_opts
+    end
   end
 
   def read
@@ -39,6 +39,8 @@ class WayController < ApplicationController
       else
         render :text => "", :status => :gone
       end
+    rescue OSM::APIError => ex
+      render ex.render_opts
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
     end
@@ -50,20 +52,14 @@ class WayController < ApplicationController
       new_way = Way.from_xml(request.raw_post)
 
       if new_way and new_way.id == way.id
-        if !new_way.preconditions_ok?
+        way.update_from(new_way, @user)
-          render :text => "", :status => :precondition_failed
+        render :text => way.version.to_s, :content_type => "text/plain"
-        else
-          way.user_id = @user.id
-          way.tags = new_way.tags
-          way.nds = new_way.nds
-          way.visible = true
-          way.save_with_history!
-
-          render :nothing => true
-        end
       else
         render :nothing => true, :status => :bad_request
       end
+    rescue OSM::APIError => ex
+      logger.warn request.raw_post
+      render ex.render_opts
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
     end
@@ -73,14 +69,16 @@ class WayController < ApplicationController
   def delete
     begin
       way = Way.find(params[:id])
-      way.delete_with_relations_and_history(@user)
+      new_way = Way.from_xml(request.raw_post)
 
-      # if we get here, all is fine, otherwise something will catch below.  
+      if new_way and new_way.id == way.id
-      render :nothing => true
+        way.delete_with_history!(new_way, @user)
-    rescue OSM::APIAlreadyDeletedError
+        render :text => way.version.to_s, :content_type => "text/plain"
-      render :text => "", :status => :gone
+      else
-    rescue OSM::APIPreconditionFailedError
+        render :nothing => true, :status => :bad_request
-      render :text => "", :status => :precondition_failed
+      end
+    rescue OSM::APIError => ex
+      render ex.render_opts
     rescue ActiveRecord::RecordNotFound
       render :nothing => true, :status => :not_found
     end
@@ -92,7 +90,7 @@ class WayController < ApplicationController
 
       if way.visible
         nd_ids = way.nds + [-1]
-        nodes = Node.find(:all, :conditions => "visible = 1 AND id IN (#{nd_ids.join(',')})")
+        nodes = Node.find(:all, :conditions => ["visible = ? AND id IN (#{nd_ids.join(',')})", true])
 
         # Render
         doc = OSM::API.new.get_xml_doc
@@ -130,13 +128,19 @@ class WayController < ApplicationController
     end
   end
 
+  ##
+  # returns all the ways which are currently using the node given in the 
+  # :id parameter. note that this used to return deleted ways as well, but
+  # this seemed not to be the expected behaviour, so it was removed.
   def ways_for_node
-    wayids = WayNode.find(:all, :conditions => ['node_id = ?', params[:id]]).collect { |ws| ws.id[0] }.uniq
+    wayids = WayNode.find(:all, 
+                          :conditions => ['node_id = ?', params[:id]]
+                          ).collect { |ws| ws.id[0] }.uniq
 
     doc = OSM::API.new.get_xml_doc
 
     Way.find(wayids).each do |way|
-      doc.root << way.to_xml_node
+      doc.root << way.to_xml_node if way.visible
     end
 
     render :text => doc.to_s, :content_type => "text/xml"

app/helpers/browse_helper.rb

@@ -1,2 +1,5 @@
 module BrowseHelper
+  def link_to_page(page, page_param)
+    return link_to(page, page_param => page)
+  end
 end

app/models/acl.rb

@@ -1,13 +1,23 @@
 class Acl < ActiveRecord::Base
   def self.find_by_address(address, options)
-    self.with_scope(:find => {:conditions => ["inet_aton(?) & netmask = address", address]}) do
+    self.with_scope(:find => {:conditions => ["#{inet_aton} & netmask = address", address]}) do
       return self.find(:first, options)
     end
   end
 
   def self.find_all_by_address(address, options)
-    self.with_scope(:find => {:conditions => ["inet_aton(?) & netmask = address", address]}) do
+    self.with_scope(:find => {:conditions => ["#{inet_aton} & netmask = address", address]}) do
       return self.find(:all, options)
     end
   end
+
+private
+
+  def self.inet_aton
+    if self.connection.adapter_name == "MySQL"
+      "inet_aton(?)"
+    else
+      "?"
+    end
+  end
 end

app/models/changeset.rb

@@ -0,0 +1,241 @@
+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.getutc) and (num_changes <= MAX_ELEMENTS))
+  end
+
+  def set_closed_time_now
+    if is_open?
+      self.closed_at = Time.now.getutc
+    end
+  end
+  
+  def self.from_xml(xml, create=false)
+    begin
+      p = XML::Parser.string(xml)
+      doc = p.parse
+
+      cs = Changeset.new
+
+      doc.find('//osm/changeset').each do |pt|
+        if create
+          cs.created_at = Time.now.getutc
+          # initial close time is 1h ahead, but will be increased on each
+          # modification.
+          cs.closed_at = cs.created_at + IDLE_TIMEOUT
+          # initially we have no changes in a changeset
+          cs.num_changes = 0
+        end
+
+        pt.find('tag').each do |tag|
+          cs.add_tag_keyval(tag['k'], tag['v'])
+        end
+      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
+  
+  def has_valid_bbox?
+    not bbox.include? nil
+  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.getutc
+
+    # 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.getutc + 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.new
+    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

app/models/changeset_tag.rb

@@ -0,0 +1,8 @@
+class ChangesetTag < ActiveRecord::Base
+  belongs_to :changeset, :foreign_key => 'id'
+
+  validates_presence_of :id
+  validates_length_of :k, :v, :maximum => 255, :allow_blank => true
+  validates_uniqueness_of :id, :scope => :k
+  validates_numericality_of :id, :only_integer => true
+end

app/models/diary_entry.rb

@@ -1,11 +1,15 @@
 class DiaryEntry < ActiveRecord::Base
   belongs_to :user
   has_many :diary_comments, :include => :user,
-                            :conditions => "users.visible = 1",
+                            :conditions => ["users.visible = ?", true],
                             :order => "diary_comments.id"
 
   validates_presence_of :title, :body
-  validates_numericality_of :latitude, :allow_nil => true
+  validates_length_of :title, :within => 1..255
-  validates_numericality_of :longitude, :allow_nil => true
+  validates_length_of :language, :within => 2..3, :allow_nil => true
+  validates_numericality_of :latitude, :allow_nil => true,
+                            :greater_than_or_equal_to => -90, :less_than_or_equal_to => 90
+  validates_numericality_of :longitude, :allow_nil => true,
+                            :greater_than_or_equal_to => -180, :less_than_or_equal_to => 180
   validates_associated :user
 end

app/models/message.rb

@@ -1,8 +1,12 @@
+require 'validators'
+
 class Message < ActiveRecord::Base
   belongs_to :sender, :class_name => "User", :foreign_key => :from_user_id
   belongs_to :recipient, :class_name => "User", :foreign_key => :to_user_id
 
-  validates_presence_of :title, :body, :sent_on
+  validates_presence_of :title, :body, :sent_on, :sender, :recipient
+  validates_length_of :title, :within => 1..255
   validates_inclusion_of :message_read, :in => [ true, false ]
   validates_associated :sender, :recipient
+  validates_as_utf8 :title
 end

app/models/node.rb

@@ -2,27 +2,34 @@ class Node < ActiveRecord::Base
   require 'xml/libxml'
 
   include GeoRecord
+  include ConsistencyValidations
 
   set_table_name 'current_nodes'
 
-  validates_presence_of :user_id, :timestamp
+  belongs_to :changeset
-  validates_inclusion_of :visible, :in => [ true, false ]
-  validates_numericality_of :latitude, :longitude
-  validate :validate_position
-
-  belongs_to :user
 
   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?
@@ -50,7 +57,7 @@ class Node < ActiveRecord::Base
     #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
 
@@ -60,13 +67,29 @@ class Node < ActiveRecord::Base
       p = XML::Parser.string(xml)
       doc = p.parse
 
+      doc.find('//osm/node').each do |pt|
+        return Node.from_xml_node(pt, create)
+      end
+    rescue LibXML::XML::Error, ArgumentError => ex
+      raise OSM::APIBadXMLError.new("node", xml, ex.message)
+    end
+  end
+
+  def self.from_xml_node(pt, create=false)
     node = Node.new
     
-      doc.find('//osm/node').each do |pt|
+    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
 
-        return nil unless node.in_world?
+    raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world?
+
+    # 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
 
     unless create
       if pt['id'] != '0'
@@ -74,68 +97,119 @@ class Node < ActiveRecord::Base
       end
     end
 
-        node.visible = pt['visible'] and pt['visible'] == 'true'
+    # 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'
 
-        if create
+    # We don't care about the time, as it is explicitly set on create/update/delete
-          node.timestamp = Time.now
-        else
-          if pt['timestamp']
-            node.timestamp = Time.parse(pt['timestamp'])
-          end
-        end
 
     tags = []
 
     pt.find('tag').each do |tag|
-          tags << [tag['k'],tag['v']]
+      node.add_tag_key_val(tag['k'],tag['v'])
-        end
-
-        node.tags = Tags.join(tags)
-      end
-    rescue
-      node = nil
     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
+      check_consistency(self, new_node, user)
-      self.save!
+      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 ])
-      old_node = OldNode.from_node(self)
+        raise OSM::APIPreconditionFailedError.new
-      old_node.save!
+      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 first
+    self.changeset_id = new_node.changeset_id
+    self.changeset = new_node.changeset
+
+    # update changeset bbox with *old* position first
+    changeset.update_bbox!(bbox);
+
+    # FIXME logic needs to be double checked
+    self.latitude = new_node.latitude 
+    self.longitude = new_node.longitude
+    self.tags = new_node.tags
+    self.visible = true
+
+    # update changeset bbox with *new* position
+    changeset.update_bbox!(bbox);
+
+    save_with_history!
+  end
+  
+  def create_with_history(user)
+    check_create_consistency(self, user)
+    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?
+    elsif self.changeset.user.data_public?
-      user_display_name_cache[self.user_id] = self.user.display_name
+      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
@@ -147,12 +221,79 @@ class Node < ActiveRecord::Base
     return el1
   end
 
-  # Return the node's tags as a Hash of keys and their values
   def tags_as_hash
-    hash = {}
+    return tags
-    Tags.split(self.tags) do |k,v|
-      hash[k] = v
   end
-    hash
+
+  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.getutc
+    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
+  end
+  
 end

app/models/old_node.rb

@@ -1,25 +1,21 @@
 class OldNode < ActiveRecord::Base
   include GeoRecord
+  include ConsistencyValidations
 
   set_table_name 'nodes'
   
-  validates_presence_of :user_id, :timestamp
+  validates_presence_of :changeset_id, :timestamp
   validates_inclusion_of :visible, :in => [ true, false ]
   validates_numericality_of :latitude, :longitude
   validate :validate_position
+  validates_associated :changeset
 
-  belongs_to :user
+  belongs_to :changeset
  
   def validate_position
     errors.add_to_base("Node is not in the world") unless in_world?
   end
 
-  def in_world?
-    return false if self.lat < -90 or self.lat > 90
-    return false if self.lon < -180 or self.lon > 180
-    return true
-  end
-
   def self.from_node(node)
     old_node = OldNode.new
     old_node.latitude = node.latitude
@@ -27,19 +23,30 @@ class OldNode < ActiveRecord::Base
     old_node.visible = node.visible
     old_node.tags = node.tags
     old_node.timestamp = node.timestamp
-    old_node.user_id = node.user_id
+    old_node.changeset_id = node.changeset_id
     old_node.id = node.id
+    old_node.version = node.version
     return old_node
   end
   
+  def to_xml
+    doc = OSM::API.new.get_xml_doc
+    doc.root << to_xml_node()
+    return doc
+  end
+
   def to_xml_node
     el1 = XML::Node.new 'node'
     el1['id'] = self.id.to_s
     el1['lat'] = self.lat.to_s
     el1['lon'] = self.lon.to_s
-    el1['user'] = self.user.display_name if self.user.data_public?
+    el1['changeset'] = self.changeset.id.to_s
+    if self.changeset.user.data_public?
+      el1['user'] = self.changeset.user.display_name
+      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
@@ -48,15 +55,45 @@ class OldNode < ActiveRecord::Base
 
     el1['visible'] = self.visible.to_s
     el1['timestamp'] = self.timestamp.xmlschema
+    el1['version'] = self.version.to_s
     return el1
   end
 
-  def tags_as_hash
+  def save_with_dependencies!
-    hash = {}
+    save!
-    Tags.split(self.tags) do |k,v|
+    #not sure whats going on here
-      hash[k] = v
+    clear_aggregation_cache
+    clear_association_cache
+    #ok from here
+    @attributes.update(OldNode.find(:first, :conditions => ['id = ? AND timestamp = ? AND version = ?', self.id, self.timestamp, self.version]).instance_variable_get('@attributes'))
+   
+    self.tags.each do |k,v|
+      tag = OldNodeTag.new
+      tag.k = k
+      tag.v = v
+      tag.id = self.id
+      tag.version = self.version
+      tag.save!
     end
-    hash
+  end
+
+  def tags
+    unless @tags
+        @tags = Hash.new
+        OldNodeTag.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |tag|
+            @tags[tag.k] = tag.v
+        end
+    end
+    @tags = Hash.new unless @tags
+    @tags
+  end
+
+  def tags=(t)
+    @tags = t 
+  end
+
+  def tags_as_hash 
+    return self.tags
   end 
  
   # Pretend we're not in any ways 

app/models/old_relation.rb

@@ -1,14 +1,19 @@
 class OldRelation < ActiveRecord::Base
+  include ConsistencyValidations
+  
   set_table_name 'relations'
 
-  belongs_to :user
+  belongs_to :changeset
+  
+  validates_associated :changeset
 
   def self.from_relation(relation)
     old_relation = OldRelation.new
     old_relation.visible = relation.visible
-    old_relation.user_id = relation.user_id
+    old_relation.changeset_id = relation.changeset_id
     old_relation.timestamp = relation.timestamp
     old_relation.id = relation.id
+    old_relation.version = relation.version
     old_relation.members = relation.members
     old_relation.tags = relation.tags
     return old_relation
@@ -33,14 +38,12 @@ class OldRelation < ActiveRecord::Base
       tag.save!
     end
 
-    i = 1
+    self.members.each_with_index do |m,i|
-    self.members.each do |m|
       member = OldRelationMember.new
-      member.id = self.id
+      member.id = [self.id, self.version, i]
-      member.member_type = m[0]
+      member.member_type = m[0].classify
       member.member_id = m[1]
       member.member_role = m[2]
-      member.version = self.version
       member.save!
     end
   end
@@ -48,7 +51,7 @@ class OldRelation < ActiveRecord::Base
   def members
     unless @members
         @members = Array.new
-        OldRelationMember.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |m|
+        OldRelationMember.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version], :order => "sequence_id").each do |m|
             @members += [[m.type,m.id,m.role]]
         end
     end
@@ -85,16 +88,27 @@ class OldRelation < ActiveRecord::Base
     OldRelationTag.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version])    
   end
 
+  def to_xml
+    doc = OSM::API.new.get_xml_doc
+    doc.root << to_xml_node()
+    return doc
+  end
+
   def to_xml_node
     el1 = XML::Node.new 'relation'
     el1['id'] = self.id.to_s
     el1['visible'] = self.visible.to_s
     el1['timestamp'] = self.timestamp.xmlschema
-    el1['user'] = self.user.display_name if self.user.data_public?
+    if self.changeset.user.data_public?
+      el1['user'] = self.changeset.user.display_name
+      el1['uid'] = self.changeset.user.id.to_s
+    end
+    el1['version'] = self.version.to_s
+    el1['changeset'] = self.changeset_id.to_s
     
     self.old_members.each do |member|
       e = XML::Node.new 'member'
-      e['type'] = member.member_type.to_s
+      e['type'] = member.member_type.to_s.downcase
       e['ref'] = member.member_id.to_s # "id" is considered uncool here as it should be unique in XML
       e['role'] = member.member_role.to_s
       el1 << e

app/models/old_relation_member.rb

@@ -1,3 +1,6 @@
 class OldRelationMember < ActiveRecord::Base
   set_table_name 'relation_members'
+
+  set_primary_keys :id, :version, :sequence_id
+  belongs_to :relation, :foreign_key=> :id
 end

app/models/old_relation_tag.rb

@@ -1,3 +1,10 @@
 class OldRelationTag < ActiveRecord::Base
   set_table_name 'relation_tags'
+  
+  belongs_to :old_relation, :foreign_key => [:id, :version]
+  
+  validates_presence_of :id, :version
+  validates_length_of :k, :v, :maximum => 255, :allow_blank => true
+  validates_uniqueness_of :id, :scope => [:k, :version]
+  validates_numericality_of :id, :version, :only_integer => true
 end

app/models/old_way.rb

@@ -1,14 +1,19 @@
 class OldWay < ActiveRecord::Base
+  include ConsistencyValidations
+  
   set_table_name 'ways'
 
-  belongs_to :user
+  belongs_to :changeset
+
+  validates_associated :changeset
   
   def self.from_way(way)
     old_way = OldWay.new
     old_way.visible = way.visible
-    old_way.user_id = way.user_id
+    old_way.changeset_id = way.changeset_id
     old_way.timestamp = way.timestamp
     old_way.id = way.id
+    old_way.version = way.version
     old_way.nds = way.nds
     old_way.tags = way.tags
     return old_way
@@ -93,7 +98,12 @@ class OldWay < ActiveRecord::Base
     el1['id'] = self.id.to_s
     el1['visible'] = self.visible.to_s
     el1['timestamp'] = self.timestamp.xmlschema
-    el1['user'] = self.user.display_name if self.user.data_public?
+    if self.changeset.user.data_public?
+      el1['user'] = self.changeset.user.display_name
+      el1['uid'] = self.changeset.user.id.to_s
+    end
+    el1['version'] = self.version.to_s
+    el1['changeset'] = self.changeset.id.to_s
     
     self.old_nodes.each do |nd| # FIXME need to make sure they come back in the right order
       e = XML::Node.new 'nd'
@@ -114,27 +124,29 @@ class OldWay < ActiveRecord::Base
   # For get_nodes_undelete, uses same nodes, even if they've moved since
   # For get_nodes_revert,   allocates new ids 
   # Currently returns Potlatch-style array
+  # where [5] indicates whether latest version is usable as is (boolean)
+  # (i.e. is it visible? are we actually reverting to an earlier version?)
 
   def get_nodes_undelete
 	points = []
 	self.nds.each do |n|
 	  node=Node.find(n)
-	  points << [node.lon, node.lat, n, node.visible ? 1 : 0, node.tags_as_hash]
+	  points << [node.lon, node.lat, n, node.version, node.tags_as_hash, node.visible]
     end
 	points
   end
   
-  def get_nodes_revert
+  def get_nodes_revert(timestamp)
     points=[]
     self.nds.each do |n|
-      oldnode=OldNode.find(:first, :conditions=>['id=? AND timestamp<=?',n,self.timestamp], :order=>"timestamp DESC")
+      oldnode=OldNode.find(:first, :conditions=>['id=? AND timestamp<=?',n,timestamp], :order=>"timestamp DESC")
       curnode=Node.find(n)
-      id=n; v=curnode.visible ? 1 : 0
+      id=n; reuse=curnode.visible
       if oldnode.lat!=curnode.lat or oldnode.lon!=curnode.lon or oldnode.tags!=curnode.tags then
         # node has changed: if it's in other ways, give it a new id
-        if curnode.ways-[self.id] then id=-1; v=nil end
+        if curnode.ways-[self.id] then id=-1; reuse=false end
       end
-      points << [oldnode.lon, oldnode.lat, id, v, oldnode.tags_as_hash]
+      points << [oldnode.lon, oldnode.lat, id, curnode.version, oldnode.tags_as_hash, reuse]
     end
     points
   end

app/models/old_way_tag.rb

@@ -1,6 +1,10 @@
 class OldWayTag < ActiveRecord::Base
-  belongs_to :user
-
   set_table_name 'way_tags'
 
+  belongs_to :old_way, :foreign_key => [:id, :version]
+
+  validates_presence_of :id
+  validates_length_of :k, :v, :maximum => 255, :allow_blank => true
+  validates_uniqueness_of :id, :scope => [:k, :version]
+  validates_numericality_of :id, :version, :only_integer => true
 end

app/models/relation.rb

@@ -1,37 +1,64 @@
 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.string(xml)
       doc = p.parse
 
+      doc.find('//osm/relation').each do |pt|
+        return Relation.from_xml_node(pt, create)
+      end
+    rescue LibXML::XML::Error, ArgumentError => ex
+      raise OSM::APIBadXMLError.new("relation", xml, ex.message)
+    end
+  end
+
+  def self.from_xml_node(pt, create=false)
     relation = Relation.new
 
-      doc.find('//osm/relation').each do |pt|
     if !create and pt['id'] != '0'
       relation.id = pt['id'].to_i
     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.timestamp = Time.now.getutc
       relation.visible = true
+      relation.version = 0
     else
       if pt['timestamp']
         relation.timestamp = Time.parse(pt['timestamp'])
       end
+      relation.version = pt['version']
     end
 
     pt.find('tag').each do |tag|
@@ -39,12 +66,17 @@ class Relation < ActiveRecord::Base
     end
 
     pt.find('member').each do |member|
-          relation.add_member(member['type'], member['ref'], member['role'])
+      #member_type = 
-        end
+      logger.debug "each member"
-      end
+      raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member['type']
-    rescue
+      logger.debug "after raise"
-      relation = nil
+      #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'].classify, member['ref'], member['role'])
     end
+    raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
 
     return relation
   end
@@ -60,18 +92,23 @@ class Relation < ActiveRecord::Base
     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?
+    elsif self.changeset.user.data_public?
-      user_display_name_cache[self.user_id] = self.user.display_name
+      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
@@ -88,7 +125,7 @@ class Relation < ActiveRecord::Base
       #end
       if p
         e = XML::Node.new 'member'
-        e['type'] = member.member_type
+        e['type'] = member.member_type.downcase
         e['ref'] = member.member_id.to_s 
         e['role'] = member.member_role
         el1 << e
@@ -108,7 +145,7 @@ class Relation < ActiveRecord::Base
     if ids.empty?
       return []
     else
-      self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'node' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do
+      self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Node' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do
         return self.find(:all, options)
       end
     end
@@ -118,7 +155,7 @@ class Relation < ActiveRecord::Base
     if ids.empty?
       return []
     else
-      self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'way' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do
+      self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Way' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do
         return self.find(:all, options)
       end
     end
@@ -128,7 +165,7 @@ class Relation < ActiveRecord::Base
     if ids.empty?
       return []
     else
-      self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'relation' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do
+      self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Relation' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do
         return self.find(:all, options)
       end
     end
@@ -170,94 +207,97 @@ class Relation < ActiveRecord::Base
 
   def add_tag_keyval(k, v)
     @tags = Hash.new unless @tags
+
+    # duplicate tags are now forbidden, so we can't allow values
+    # in the hash to be overwritten.
+    raise OSM::APIDuplicateTagsError.new("relation", self.id, k) if @tags.include? k
+
     @tags[k] = v
   end
 
-  def save_with_history!
+  ##
+  # 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
-      t = Time.now
+      check_consistency(self, new_relation, user)
-      self.timestamp = t
+      # This will check to see if this relation is used by another relation
-      self.save!
+      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")
-      tags = self.tags
+      end
-
+      self.changeset_id = new_relation.changeset_id
-      RelationTag.delete_all(['id = ?', self.id])
+      self.tags = {}
-
+      self.members = []
-      tags.each do |k,v|
+      self.visible = false
-        tag = RelationTag.new
+      save_with_history!
-        tag.k = k
+    end
-        tag.v = v
-        tag.id = self.id
-        tag.save!
   end
 
-      members = self.members
+  def update_from(new_relation, user)
-
+    check_consistency(self, new_relation, user)
-      RelationMember.delete_all(['id = ?', self.id])
+    if !new_relation.preconditions_ok?
-
+      raise OSM::APIPreconditionFailedError.new
-      members.each do |n|
+    end
-        mem = RelationMember.new
+    self.changeset_id = new_relation.changeset_id
-        mem.id = self.id
+    self.changeset = new_relation.changeset
-        mem.member_type = n[0];
+    self.tags = new_relation.tags
-        mem.member_id = n[1];
+    self.members = new_relation.members
-        mem.member_role = n[2];
+    self.visible = true
-        mem.save!
+    save_with_history!
   end
   
-      old_relation = OldRelation.from_relation(self)
+  def create_with_history(user)
-      old_relation.timestamp = t
+    check_create_consistency(self, user)
-      old_relation.save_with_dependencies!
+    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.
-    # Once we know the id of the node/way/relation exists
+    # If the member is valid and visible then we add it to the 
-    # we check to see if it is already existing in the hashtable
+    # relevant hash table, with the value true as a cache.
-    # 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
+    elements = { :node => Hash.new, :way => Hash.new, :relation => Hash.new }
-    ways = Hash.new
-    relations = Hash.new
     self.members.each do |m|
-      if (m[0] == "node")
+      # find the hash for the element type or die
-        n = Node.find(:first, :conditions => ["id = ?", m[1]])
+      logger.debug m[0]
-        unless n and n.visible 
+      hash = elements[m[0].downcase.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
-        if nodes[m[1]]
+        hash[m[1]] = true
-          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
-        end
-      else
-        return false
       end
     end
+
     return true
   rescue
     return false
@@ -267,4 +307,152 @@ class Relation < ActiveRecord::Base
   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.downcase.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.getutc
+      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
+
+        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
+        tag.v = v
+        tag.id = self.id
+        tag.save!
+      end
+      
+      # reload, so that all of the members are accessible in their
+      # new state.
+      self.reload
+
+      # 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
+
+      # 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, 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!
+
+      # 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
+        # add only changed members to the changeset
+        changed_members.each do |id, type|
+          if type != "Relation"
+            update_changeset_element(type, id)
+          end
+        end
+      end
+
+      # tell the changeset we updated one element only
+      changeset.add_changes! 1
+
+      # save the (maybe updated) changeset bounding box
+      changeset.save!
+    end
+  end
+
 end

app/models/relation_member.rb

@@ -1,19 +1,20 @@
 class RelationMember < ActiveRecord::Base
   set_table_name 'current_relation_members'
   
+  set_primary_keys :id, :sequence_id
   belongs_to :member, :polymorphic => true, :foreign_type => :member_class
   belongs_to :relation, :foreign_key => :id
 
   def after_find
-    self[:member_class] = self.member_type.capitalize
+    self[:member_class] = self.member_type.classify
   end
 
   def after_initialize
-    self[:member_class] = self.member_type.capitalize
+    self[:member_class] = self.member_type.classify unless self.member_type.nil?
   end
 
   def before_save
-    self.member_type = self[:member_class].downcase
+    self.member_type = self[:member_class].classify
   end
 
   def member_type=(type)

app/models/relation_tag.rb

@@ -3,4 +3,8 @@ class RelationTag < ActiveRecord::Base
 
   belongs_to :relation, :foreign_key => 'id'
 
+  validates_presence_of :id
+  validates_length_of :k, :v, :maximum => 255, :allow_blank => true
+  validates_uniqueness_of :id, :scope => :k
+  validates_numericality_of :id, :only_integer => true
 end

app/models/trace.rb

@@ -3,6 +3,8 @@ class Trace < ActiveRecord::Base
 
   validates_presence_of :user_id, :name, :timestamp
   validates_presence_of :description, :on => :create
+  validates_length_of :name, :maximum => 255
+  validates_length_of :description, :maximum => 255
 #  validates_numericality_of :latitude, :longitude
   validates_inclusion_of :public, :inserted, :in => [ true, false]
   

app/models/tracetag.rb

@@ -2,6 +2,7 @@ class Tracetag < ActiveRecord::Base
   set_table_name 'gpx_file_tags'
 
   validates_format_of :tag, :with => /^[^\/;.,?]*$/
+  validates_length_of :tag, :within => 1..255
 
   belongs_to :trace, :foreign_key => 'gpx_id'
 end

app/models/user.rb

@@ -4,19 +4,21 @@ class User < ActiveRecord::Base
   has_many :traces
   has_many :diary_entries, :order => 'created_at DESC'
   has_many :messages, :foreign_key => :to_user_id, :order => 'sent_on DESC'
-  has_many :new_messages, :class_name => "Message", :foreign_key => :to_user_id, :conditions => "message_read = 0", :order => 'sent_on DESC'
+  has_many :new_messages, :class_name => "Message", :foreign_key => :to_user_id, :conditions => {:message_read => false}, :order => 'sent_on DESC'
   has_many :sent_messages, :class_name => "Message", :foreign_key => :from_user_id, :order => 'sent_on DESC'
-  has_many :friends, :include => :befriendee, :conditions => "users.visible = 1"
+  has_many :friends, :include => :befriendee, :conditions => ["users.visible = ?", true]
   has_many :tokens, :class_name => "UserToken"
   has_many :preferences, :class_name => "UserPreference"
+  has_many :changesets
 
   validates_presence_of :email, :display_name
   validates_confirmation_of :email, :message => 'Email addresses must match'
   validates_confirmation_of :pass_crypt, :message => 'Password must match the confirmation password'
   validates_uniqueness_of :display_name, :allow_nil => true
   validates_uniqueness_of :email
-  validates_length_of :pass_crypt, :minimum => 8
+  validates_length_of :pass_crypt, :within => 8..255
-  validates_length_of :display_name, :minimum => 3, :allow_nil => true
+  validates_length_of :display_name, :within => 3..255, :allow_nil => true
+  validates_length_of :email, :within => 6..255
   validates_format_of :email, :with => /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i
   validates_format_of :display_name, :with => /^[^\/;.,?]*$/
   validates_numericality_of :home_lat, :allow_nil => true
@@ -28,7 +30,7 @@ class User < ActiveRecord::Base
   file_column :image, :magick => { :geometry => "100x100>" }
 
   def after_initialize
-    self.creation_time = Time.now if self.creation_time.nil?
+    self.creation_time = Time.now.getutc if self.creation_time.nil?
   end
 
   def encrypt_password
@@ -80,7 +82,7 @@ class User < ActiveRecord::Base
     if self.home_lon and self.home_lat 
       gc = OSM::GreatCircle.new(self.home_lat, self.home_lon)
       bounds = gc.bounds(radius)
-      nearby = User.find(:all, :conditions => "visible = 1 and home_lat between #{bounds[:minlat]} and #{bounds[:maxlat]} and home_lon between #{bounds[:minlon]} and #{bounds[:maxlon]} and data_public = 1 and id != #{self.id}")
+      nearby = User.find(:all, :conditions => ["visible = ? and home_lat between #{bounds[:minlat]} and #{bounds[:maxlat]} and home_lon between #{bounds[:minlon]} and #{bounds[:maxlon]} and data_public = ? and id != #{self.id}", true, true])
       nearby.delete_if { |u| gc.distance(u.home_lat, u.home_lon) > radius }
       nearby.sort! { |u1,u2| gc.distance(u1.home_lat, u1.home_lon) <=> gc.distance(u2.home_lat, u2.home_lon) }
     else
@@ -104,6 +106,10 @@ class User < ActiveRecord::Base
     return false
   end
 
+  def trace_public_default
+    return self.preferences.find(:first, :conditions => {:k => "gps.trace.public", :v => "default"})
+  end
+
   def delete
     self.active = false
     self.display_name = "user_#{self.id}"

app/models/user_preference.rb

@@ -2,6 +2,9 @@ class UserPreference < ActiveRecord::Base
   set_primary_keys :user_id, :k
   belongs_to :user
   
+  validates_length_of :k, :within => 1..255
+  validates_length_of :v, :within => 1..255
+
   # Turn this Node in to an XML Node without the <osm> wrapper.
   def to_xml_node
     el1 = XML::Node.new 'preference'

app/models/way.rb

@@ -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'
 
@@ -15,25 +17,48 @@ class Way < ActiveRecord::Base
   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.string(xml)
       doc = p.parse
 
+      doc.find('//osm/way').each do |pt|
+        return Way.from_xml_node(pt, create)
+      end
+    rescue LibXML::XML::Error, ArgumentError => ex
+      raise OSM::APIBadXMLError.new("way", xml, ex.message)
+    end
+  end
+
+  def self.from_xml_node(pt, create=false)
     way = Way.new
 
-      doc.find('//osm/way').each do |pt|
     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.timestamp = Time.now.getutc
       way.visible = true
     else
       if pt['timestamp']
         way.timestamp = Time.parse(pt['timestamp'])
       end
+      # if visible isn't present then it defaults to true
+      way.visible = (pt['visible'] or true)
     end
 
     pt.find('tag').each do |tag|
@@ -43,10 +68,6 @@ class Way < ActiveRecord::Base
     pt.find('nd').each do |nd|
       way.add_nd_num(nd['ref'])
     end
-      end
-    rescue
-      way = nil
-    end
 
     return way
   end
@@ -73,18 +94,23 @@ class Way < ActiveRecord::Base
     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?
+    elsif self.changeset.user.data_public?
-      user_display_name_cache[self.user_id] = self.user.display_name
+      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 = []
@@ -96,7 +122,7 @@ class Way < ActiveRecord::Base
         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
@@ -154,97 +180,84 @@ class Way < ActiveRecord::Base
 
   def add_tag_keyval(k, v)
     @tags = Hash.new unless @tags
+
+    # duplicate tags are now forbidden, so we can't allow values
+    # in the hash to be overwritten.
+    raise OSM::APIDuplicateTagsError.new("way", self.id, k) if @tags.include? k
+
     @tags[k] = v
   end
 
-  def save_with_history!
+  ##
-    t = Time.now
+  # the integer coords (i.e: unscaled) bounding box of the way, assuming
-
+  # straight line segments.
-    Way.transaction do
+  def bbox
-      self.timestamp = t
+    lons = nodes.collect { |n| n.longitude }
-      self.save!
+    lats = nodes.collect { |n| n.latitude }
+    [ lons.min, lats.min, lons.max, lats.max ]
   end
 
-    WayTag.transaction do
+  def update_from(new_way, user)
-      tags = self.tags
+    check_consistency(self, new_way, user)
-
+    if !new_way.preconditions_ok?
-      WayTag.delete_all(['id = ?', self.id])
+      raise OSM::APIPreconditionFailedError.new
-
-      tags.each do |k,v|
-        tag = WayTag.new
-        tag.k = k
-        tag.v = v
-        tag.id = self.id
-        tag.save!
-      end
     end
 
-    WayNode.transaction do
+    self.changeset_id = new_way.changeset_id
-      nds = self.nds
+    self.changeset = new_way.changeset
-
+    self.tags = new_way.tags
-      WayNode.delete_all(['id = ?', self.id])
+    self.nds = new_way.nds
-
+    self.visible = true
-      sequence = 1
+    save_with_history!
-      nds.each do |n|
-        nd = WayNode.new
-        nd.id = [self.id, sequence]
-        nd.node_id = n
-        nd.save!
-        sequence += 1
-      end
   end
 
-    old_way = OldWay.from_way(self)
+  def create_with_history(user)
-    old_way.timestamp = t
+    check_create_consistency(self, user)
-    old_way.save_with_dependencies!
+    if !self.preconditions_ok?
+      raise OSM::APIPreconditionFailedError.new
+    end
+    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_history!(new_way, user)
-  def delete_with_relations_and_history(user)
+    unless self.visible
-    if self.visible
+      raise OSM::APIAlreadyDeletedError
-	  # FIXME
+    end
-	  # this should actually delete the relations,
+    
-	  # not just throw a PreconditionFailed if it's a member of a relation!!
+    # 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])
+                             :conditions => [ "visible = ? AND member_type='Way' and member_id=? ", true, self.id])
-        raise OSM::APIPreconditionFailedError
+        raise OSM::APIPreconditionFailedError.new("You need to make sure that this way is not a member of a relation.")
-      # end FIXME
       else
-        self.user_id = user.id
+        self.changeset_id = new_way.changeset_id
+        self.changeset = new_way.changeset
+
         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
@@ -263,4 +276,78 @@ class Way < ActiveRecord::Base
   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.getutc
+
+    # update the bounding box, note that this has to be done both before 
+    # and after the save, so that nodes from both versions are included in the 
+    # bbox. we use a copy of the changeset so that it isn't reloaded
+    # later in the save.
+    cs = self.changeset
+    cs.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!
+
+      # reload the way so that the nodes array points to the correct
+      # new set of nodes.
+      self.reload
+
+      # update and commit the bounding box, now that way nodes 
+      # have been updated and we're in a transaction.
+      cs.update_bbox!(bbox) unless nodes.empty?
+
+      # tell the changeset we updated one element only
+      cs.add_changes! 1
+
+      cs.save!
+    end
+  end
+
 end

app/models/way_tag.rb

@@ -6,4 +6,9 @@ class WayTag < ActiveRecord::Base
   # FIXME add a real multipart key to waytags so that we can do eager loadin
 
   belongs_to :way, :foreign_key => 'id'
+  
+  validates_presence_of :id
+  validates_length_of :k, :v, :maximum => 255, :allow_blank => true
+  validates_uniqueness_of :id, :scope => :k
+  validates_numericality_of :id, :only_integer => true
 end

app/views/browse/_changeset_details.rhtml

@@ -0,0 +1,95 @@
+<table>
+
+  <tr>
+    <th>Created at:</th>
+    <td><%= h(changeset_details.created_at) %></td>
+  </tr>
+  
+  <tr>
+    <th>Closed at:</th>
+    <td><%= h(changeset_details.closed_at) %></td>
+  </tr>
+  
+  <% if changeset_details.user.data_public? %>
+    <tr>
+      <th>Belongs to:</th>
+      <td><%= link_to h(changeset_details.user.display_name), :controller => "user", :action => "view", :display_name => changeset_details.user.display_name %></td>
+    </tr>
+  <% end %>
+  
+  <%= render :partial => "tag_details", :object => changeset_details %>
+
+  <tr>
+    <th>Bounding box:</th>
+    <% unless changeset_details.has_valid_bbox? %>
+      <td>No bounding box has been stored for this changeset.</td>
+    <% else
+          minlon = changeset_details.min_lon/GeoRecord::SCALE.to_f
+          minlat = changeset_details.min_lat/GeoRecord::SCALE.to_f
+          maxlon = changeset_details.max_lon/GeoRecord::SCALE.to_f
+          maxlat = changeset_details.max_lat/GeoRecord::SCALE.to_f
+    %>
+      <td>
+        <table>
+          <tr>
+            <td colspan="3" style="text-align:center"><%=maxlat -%></td>
+          </tr>
+          <tr>
+            <td><%=minlon -%></td>
+            <td>(<a href='/?minlon=<%= minlon %>&minlat=<%= minlat %>&maxlon=<%= maxlon %>&maxlat=<%= maxlat %>&box=yes' title='show area box'>box</a>)</td>
+            <td><%=maxlon -%></td>
+          </tr>
+          <tr>
+            <td colspan="3" style="text-align:center"><%= minlon -%></td>
+          </tr>
+        </table>
+      </td>
+    <% end %>
+  </tr>
+
+  <% unless @nodes.empty? %>
+    <tr valign="top">
+      <th>Has the following <%= @node_pages.item_count %> nodes:</th>
+      <td>
+        <table padding="0">
+          <% @nodes.each do |node| %>
+            <tr><td><%= link_to "Node #{node.id.to_s}, version #{node.version.to_s}", :action => "node", :id => node.id.to_s %></td></tr>
+          <% end %>
+        </table>
+      </td>
+    </tr>
+    <%= render :partial => 'paging_nav', :locals => { :pages => @node_pages, :page_param => "node_page"} %>
+  <% end %>
+  
+  <% unless @ways.empty? %>
+    <tr valign="top">
+      <th>Has the following <%= @way_pages.item_count %> ways:</th>
+      <td>
+        <table padding="0">
+          <% @ways.each do |way| %>
+            <tr><td><%= link_to "Way #{way.id.to_s}, version #{way.version.to_s}", :action => "way", :id => way.id.to_s %></td></tr>
+          <% end %>
+          <%=
+          #render :partial => "containing_relation", :collection => changeset_details.containing_relation_members 
+          %>
+        </table>
+      </td>
+    </tr>
+    <%= render :partial => 'paging_nav', :locals => { :pages => @way_pages, :page_param => "way_page" } %>
+  <% end %>
+  
+  <% unless @relations.empty? %>
+    <tr valign="top">
+      <th>Has the following <%= @relation_pages.item_count %> relations:</th>
+      <td>
+        <table padding="0">
+          <% @relations.each do |relation| %>
+            <tr><td><%= link_to "Relation #{relation.id.to_s}, version #{relation.version.to_s}", :action => "relation", :id => relation.id.to_s %></td></tr>
+          <% end %>
+        </table>
+      </td>
+    </tr>
+    <%= render :partial => 'paging_nav', :locals => { :pages => @relation_pages, :page_param => "relation_page" } %>
+  <% end %>
+
+</table>

app/views/browse/_common_details.rhtml

@@ -3,20 +3,21 @@
   <td><%= h(common_details.timestamp) %></td>
 </tr>
 
-<% if common_details.user.data_public %>
+<% if common_details.changeset.user.data_public? %>
   <tr>
     <th>Edited by:</th>
-    <td><%= link_to h(common_details.user.display_name), :controller => "user", :action => "view", :display_name => common_details.user.display_name %></td>
+    <td><%= link_to h(common_details.changeset.user.display_name), :controller => "user", :action => "view", :display_name => common_details.changeset.user.display_name %></td>
   </tr>
 <% end %>
 
-<% unless common_details.tags_as_hash.empty? %>
+<tr>
-  <tr valign="top">
+  <th>Version:</th>
-    <th>Tags:</th>
+  <td><%= h(common_details.version) %></td>
-    <td>
+</tr>
-      <table padding="0">
+
-        <%= render :partial => "tag", :collection => common_details.tags_as_hash %>
+<tr>
-      </table>
+  <th>In changeset:</th>
-    </td>
+  <td><%= link_to common_details.changeset_id, :action => :changeset, :id => common_details.changeset_id %></td>
-  </tr>      
+</tr>
-<% end %>
+
+<%= render :partial => "tag_details", :object => common_details %>

app/views/browse/_map.rhtml

@@ -2,7 +2,7 @@
 <%= javascript_include_tag '/openlayers/OpenStreetMap.js' %>
 <%= javascript_include_tag 'map.js' %>
 <td align="right">
-  <% if map.visible %>
+  <% if map.instance_of? Changeset or map.visible %>
     <div id="small_map" style="width:250px; height: 300px; border: solid 1px black">
     </div>
     <span id="loading">Loading...</span>
@@ -13,6 +13,25 @@
 </td>
 <script type="text/javascript">
   function init() {
+    var map = createMap("small_map", {
+      controls: [ new OpenLayers.Control.Navigation() ]
+    });
+
+    <% if map.instance_of? Changeset %>
+    var minlon = <%= map.min_lon / GeoRecord::SCALE.to_f %>;
+    var minlat = <%= map.min_lat / GeoRecord::SCALE.to_f %>;
+    var maxlon = <%= map.max_lon / GeoRecord::SCALE.to_f %>;
+    var maxlat = <%= map.max_lat / GeoRecord::SCALE.to_f %>;
+    var bbox = new OpenLayers.Bounds(minlon, minlat, maxlon, maxlat);
+
+    setMapExtent(bbox);
+    addBoxToMap(bbox);
+
+    $("loading").innerHTML = "";
+
+    $("larger_map").href = '/?minlon='+minlon+'&minlat='+minlat+'&maxlon='+maxlon+'&maxlat='+maxlat+'&box=yes';
+    $("larger_map").innerHTML = "View Larger Map";
+    <% else %>
     var obj_type = "<%= map.class.name.downcase %>";
     var obj_id = <%= map.id %>;
     var url = "/api/<%= "#{API_VERSION}" %>/<%= map.class.name.downcase %>/<%= map.id %>";
@@ -21,10 +40,6 @@
       url += "/full";
     }
 
-    var map = createMap("small_map", {
-      controls: [ new OpenLayers.Control.Navigation() ]
-    });
-
     var osm_layer = new OpenLayers.Layer.GML("OSM", url, {
       format: OpenLayers.Format.OSM,
       projection: new OpenLayers.Projection("EPSG:4326")
@@ -58,6 +73,7 @@
 
     osm_layer.loadGML();
     osm_layer.loaded = true;
+    <% end %>
   }
 
   window.onload = init;

app/views/browse/_paging_nav.rhtml

@@ -0,0 +1,15 @@
+<tr><td colspan='2'>
+<% current_page = pages.current_page %>
+
+Showing page 
+<%= current_page.number %> (<%= current_page.first_item %><% 
+if (current_page.first_item < current_page.last_item) # if more than 1 trace on page 
+  %>-<%= current_page.last_item %><% 
+end %>
+of <%= pages.item_count %>)
+
+<% if pages.page_count > 1 %>
+| <%= pagination_links_each(pages, {}) { |n| link_to_page(n, page_param) } %>
+<% end %>
+</td>
+</tr>

app/views/browse/_tag_details.rhtml

@@ -0,0 +1,10 @@
+<% unless tag_details.tags_as_hash.empty? %>
+  <tr valign="top">
+    <th>Tags:</th>
+    <td>
+      <table padding="0">
+        <%= render :partial => "tag", :collection => tag_details.tags_as_hash %>
+      </table>
+    </td>
+  </tr>      
+<% end %>

app/views/browse/changeset.rhtml

@@ -0,0 +1,23 @@
+<table width="100%">
+  <tr>
+    <td>
+      <h2>Changeset: <%= h(@changeset.id) %></h2>
+    </td>
+    <td>
+      <%= render :partial => "navigation" %>
+    </td>
+  </tr>
+  <tr valign="top">
+    <td>
+    <%= render :partial => "changeset_details", :object => @changeset %>
+    <hr />
+    Download 
+    <%= link_to "Changeset XML", :controller => "changeset", :action => "read" %>
+    or
+    <%= link_to "osmChange XML", :controller => "changeset", :action => "download" %>
+    </td>
+    <% if @changeset.has_valid_bbox? %>
+    <%= render :partial => "map", :object => @changeset %>
+    <% end %>
+  </tr>
+</table>

app/views/browse/index.rhtml

@@ -1,12 +0,0 @@
-<h2><%= @nodes.length %> Recently Changed Nodes</h2> 
-<ul>
-<% @nodes.each do |node| 
-   name = node.tags_as_hash['name'].to_s 
-   if name.length == 0:
-     name = "(No name)"
-   end
-   name = name + " - " + node.id.to_s 
-%>
-   <li><%= link_to h(name), :action => "node", :id => node.id %></li>
-<% end %>
-</ul>

app/views/browse/not_found.rhtml

@@ -0,0 +1 @@
+<p>Sorry, the <%= @type -%> with the id <%= params[:id] -%>, could not be found.</p>

app/views/browse/start.rjs

@@ -189,7 +189,7 @@ page << <<EOJ
     if (size > 0.25) {
       setStatus("Unable to load: Bounding box size of " + size + " is too large (must be smaller than 0.25)");
     } else {
-      loadGML("/api/0.5/map?bbox=" + projected.toBBOX());
+      loadGML("/api/#{API_VERSION}/map?bbox=" + projected.toBBOX());
     }
   }
 
@@ -393,7 +393,7 @@ page << <<EOJ
     this.link.href = "";
     this.link.innerHTML = "Wait...";
 
-    new Ajax.Request("/api/0.5/" + this.type + "/" + this.feature.osm_id + "/history", {
+    new Ajax.Request("/api/#{API_VERSION}/" + this.type + "/" + this.feature.osm_id + "/history", {
       onComplete: OpenLayers.Function.bind(displayHistory, this)
     });
 

app/views/changeset/_changeset.rhtml

@@ -0,0 +1,49 @@
+<tr>
+  <% cl = cycle('table0', 'table1') %>
+
+  <td class="<%= cl %>">
+    #<%= changeset.id %>
+  </td>
+
+  <td class="<%= cl %>">
+    <% if changeset.closed_at > DateTime.now %> (still editing)
+    <% else %><%= changeset.closed_at.strftime("%d %b %Y %H:%M") %><% end %>
+  </td>
+
+    
+  <%if showusername %>  
+    <td class="<%= cl %>">
+    <% if changeset.user.data_public? %>
+      <%= link_to h(changeset.user.display_name), :controller => "user", :action => "view", :display_name => changeset.user.display_name %>
+    <% else %>
+      <i>Anonymous</i>
+    <% end %>
+    </td>
+  <% end %>
+    
+  <td class="<%= cl %>">
+    <% if changeset.tags['comment'] %>
+      <%= h(changeset.tags['comment']) %>
+    <% else %>
+      (none)
+    <% end %>
+  </td>
+
+  <td class="<%= cl %>">
+    <% if changeset.min_lat.nil? %>
+      (no edits)
+    <% else 
+      minlon = changeset.min_lon/GeoRecord::SCALE.to_f
+      minlat = changeset.min_lat/GeoRecord::SCALE.to_f
+      maxlon = changeset.max_lon/GeoRecord::SCALE.to_f
+      maxlat = changeset.max_lat/GeoRecord::SCALE.to_f
+    %>
+      (<a href='/?minlon=<%= minlon %>&minlat=<%= minlat %>&maxlon=<%= maxlon %>&maxlat=<%= maxlat %>&box=yes' title='show area box'><%= format("%0.3f",minlon) -%>,<%= format("%0.3f",minlat) -%>,<%= format("%0.3f",maxlon) -%>,<%= format("%0.3f",maxlat) -%></a>) 
+  <% end %>
+  </td>
+
+  <td class="<%= cl %>">
+    <%= link_to 'more', {:controller => 'browse', :action => 'changeset', :id => changeset.id}, {:title => 'View changeset details'} %> 
+  </td>
+
+</tr>

app/views/changeset/_changeset_paging_nav.rhtml

@@ -0,0 +1,18 @@
+<% current_page = @edit_pages.current_page %>
+
+Showing page 
+<%= current_page.number %> (<%= current_page.first_item %><% 
+if (current_page.first_item < current_page.last_item) # if more than 1 changeset on page 
+  %>-<%= current_page.last_item %><% 
+end %>
+of <%= @edit_pages.item_count %>)
+
+<%
+if @edit_pages.page_count > 1 
+	bboxparam = h(params['bbox'])
+	bboxparam = nil if bboxparam==""
+%>
+ | <%= pagination_links_each(@edit_pages, {}) { |n| link_to(n,  :display_name => @display_name, :bbox => bboxparam , :page => n) }  %>
+<%
+end
+%>

app/views/changeset/list.rhtml

@@ -0,0 +1,19 @@
+<h1>Recent Changes</h1>
+<p>Recently closed changesets:</p>
+
+<table id="keyvalue" cellpadding="3">
+  <tr>
+    <th>ID</th>
+    <th>Saved at</th>
+    <th>User</th>
+    <th>Comment</th>
+    <th>Area</th>
+    <th></th>
+  </tr>
+  <%= render :partial => 'changeset', :locals => {:showusername => true},  :collection => @edits unless @edits.nil?  %>
+</table>
+
+<p>
+For more changesets, select a user and view their edits, or see the editing 'history' of a specific area.
+</p>
+<br>

app/views/changeset/list_bbox.rhtml

@@ -0,0 +1,53 @@
+<h1>History</h1>
+<%
+if @bbox!=nil
+	minlon = @bbox[0] 
+	minlat = @bbox[1] 
+	maxlon = @bbox[2]
+	maxlat = @bbox[3] 
+
+	%>
+<p>
+Changesets within the area: 
+      (<a href='/?minlon=<%= minlon %>&minlat=<%= minlat %>&maxlon=<%= maxlon %>&maxlat=<%= maxlat %>&box=yes' title='show area box'><%= format("%0.3f",minlon) -%>,<%= format("%0.3f",minlat) -%>,<%= format("%0.3f",maxlon) -%>,<%= format("%0.3f",maxlat) -%></a>) 
+
+</p>
+
+<%	if @edits.nil? or @edits.empty? %>
+<p><b>No changesets</b></p>
+<%	else %>
+
+<%= render :partial => 'changeset_paging_nav' %>
+
+<table id="keyvalue" cellpadding="3">
+  <tr>
+    <th>ID</th>
+    <th>Saved at</th>
+    <th>User</th>
+    <th>Comment</th>
+    <th>Area</th>
+    <th></th>
+  </tr>
+  <%= render :partial => 'changeset', :locals => {:showusername => true}, :collection => @edits unless @edits.nil? %>
+</table>
+
+<%= render :partial => 'changeset_paging_nav' %>
+
+<p>For all changes everywhere see <%=  link_to("Recent Changes", :controller => "browse", :action => "changesets") %> </p>
+
+<%
+	end
+
+else 
+	#bbox is nil. happens if the user surfs to this page directly.
+%>
+
+<p>No area specified</p>
+<p>First use the <a href="/" title="view the map">view tab</a> to pan and zoom to an area of interest, then click the history tab.</p>
+<p>Alternatively view all <%=  link_to("recent changes", :controller => "browse", :action => "changesets") %> </p>
+
+<%
+end
+%>
+<br>
+

app/views/changeset/list_user.rhtml

@@ -0,0 +1,22 @@
+<h1>Edits by <%= link_to(h(@display_name), {:controller=>'user', :action=>'view', :display_name=>@display_name}) %></h1>
+
+<% if not @edits or @edits.empty? %>
+<p><b>No visible edits by <%= h(@display_name) %>.</b></p>
+<% else %>
+<%= render :partial => 'changeset_paging_nav' %>
+<table id="keyvalue" cellpadding="3">
+  <tr>
+    <th>ID</th>
+    <th>Saved at</th>
+    <th>Comment</th>
+    <th>Area</th>
+    <th></th>
+  </tr>
+  <%= render :partial => 'changeset', :locals => {:showusername => false},  :collection => @edits %>
+</table>
+<%= render :partial => 'changeset_paging_nav' %>
+<% end %>
+
+<p>For changes by all users see <%=  link_to("Recent Changes", :controller => "browse", :action => "changesets") %> </p>
+<br>
+

app/views/diary_entry/edit.rhtml

@@ -5,24 +5,28 @@
 <% form_for :diary_entry do |f| %>
   <table>
     <tr valign="top">
-      <th>Subject</th>
+      <td class="fieldName">Subject:</td>
       <td><%= f.text_field :title, :size => 60 %></td>
     </tr>
     <tr valign="top">
-      <th>Body</th>
+      <td class="fieldName">Body:</td>
       <td><%= f.text_area :body, :cols => 80 %></td>
     </tr>
     <tr valign="top">
-      <th>Location</th>
+      <td class="fieldName">Location:</td>
       <td>
         <div id="map" style="border: 1px solid black; position: relative; width : 90%; height : 400px; display: none;"></div>
         <span class="location">Latitude: <%= f.text_field :latitude, :size => 20, :id => "latitude" %> Longitude: <%= f.text_field :longitude, :size => 20, :id => "longitude" %></span>
         <a href="javascript:openMap()" id="usemap">use map</a>
+        <br/><br/>
       </td>
     </tr>
     <tr>
-      <th></th>
+      <td></td>
-      <td><%= submit_tag 'Save' %></td>
+      <td>
+	  <%= submit_tag 'Save' %>
+	  <%# TODO: button should say 'publish' or 'save changes' depending on new/edit state %>
+      </td>
     </tr>
   </table>
 <% end %>

app/views/diary_entry/list.rhtml

@@ -4,29 +4,33 @@
  <%= 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>
+<% else %>
+  <p>Recent diary entries:</p>
 
-<%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %>
+  <hr />
-<% if @entry_pages.current.next and @entry_pages.current.previous %>
+
-|
+  <%= 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 />
 <% end %>
-<%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %>
-
-<br />
 
 <%= rss_link_to :action => 'rss' %>
 

app/views/diary_entry/no_such_entry.rhtml

@@ -0,0 +1,2 @@
+<h2>No entry with the id: <%= h(params[:id]) %></h2>
+<p>Sorry, there is no diary entry or comment with the id <%=h params[:id] -%>, or no id was given. Please check your spelling, or maybe the link you clicked is wrong.</p>

app/views/layouts/site.rhtml

@@ -7,6 +7,7 @@
     <%= 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>
@@ -37,16 +38,19 @@
         <%
         viewclass = ''
         editclass = ''
+        historyclass = ''
         exportclass = ''
         traceclass = ''
         viewclass = 'active' if params['controller'] == 'site' and params['action'] == 'index' 
         editclass = 'active' if params['controller'] == 'site' and params['action'] == 'edit' 
+        historyclass = 'active' if params['controller'] == 'changeset' and params['action'] == 'list_bbox' 
         exportclass = 'active' if params['controller'] == 'site' and params['action'] == 'export'
         traceclass = 'active' if params['controller'] == 'trace'
         diaryclass = 'active' if params['controller'] == 'diary_entry'
         %>
         <li><%= link_to 'View', {:controller => 'site', :action => 'index'}, {:id => 'viewanchor', :title => 'view maps', :class => viewclass} %></li>
         <li><%= link_to 'Edit', {:controller => 'site', :action => 'edit'}, {:id => 'editanchor', :title => 'edit maps', :class => editclass} %></li>
+        <li><%= link_to 'History', {:controller => 'history' }, {:id => 'historyanchor', :title => 'changeset history', :class => historyclass} %></li>
         <% if params['controller'] == 'site' and (params['action'] == 'index' or params['action'] == 'export') %>
         <li><%= link_to_remote 'Export', {:url => {:controller => 'export', :action => 'start'}}, {:id => 'exportanchor', :title => 'export map data', :class => exportclass, :href => url_for(:controller => 'site', :action => 'export')} %></li>
         <% else %>
@@ -126,7 +130,7 @@
         </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>

app/views/message/_message_summary.rhtml

@@ -1,9 +1,10 @@
-<% this_colour = cycle('lightgrey', 'white') # can only call once for some dumb reason %>
+<% this_colour = cycle('lightgrey', 'white') # can only call once for some dumb reason 
+%>
 
 <tr class="inbox-row<%= "-unread" if not message_summary.message_read? %>">
-  <td class="inbox-sender" bgcolor='<%= this_colour %>'><%= link_to h(message_summary.sender.display_name), :controller => 'user', :action => message_summary.sender.display_name %></td>
+  <td class="inbox-sender" bgcolor="<%= this_colour %>"><%= link_to h(message_summary.sender.display_name), :controller => 'user', :action => message_summary.sender.display_name %></td>
-  <td class="inbox-subject" bgcolor='<%= this_colour %>'><%= link_to h(message_summary.title), :controller => 'message', :action => 'read', :message_id => message_summary.id  %></td>
+  <td class="inbox-subject" bgcolor="<%= this_colour %>"><%= link_to h(message_summary.title), :controller => 'message', :action => 'read', :message_id => message_summary.id  %></td>
-  <td class="inbox-sent" bgcolor='<%= this_colour %>'><%= message_summary.sent_on %></td>
+  <td class="inbox-sent" bgcolor="<%= this_colour %>"><%= message_summary.sent_on %></td>
   <% if message_summary.message_read? %>
     <td><%= button_to 'Mark as unread', :controller => 'message', :action => 'mark', :message_id => message_summary.id, :mark => 'unread' %></td>
   <% else %>

app/views/message/_sent_message_summary.rhtml

@@ -1,7 +1,8 @@
-<% this_colour = cycle('lightgrey', 'white') # can only call once for some dumb reason %>
+<% this_colour = cycle('lightgrey', 'white') # can only call once for some dumb reason
+%>
 
 <tr class="inbox-row">
-  <td class="inbox-sender" bgcolor='<%= this_colour %>'><%= link_to h(sent_message_summary.recipient.display_name), :controller => 'user', :action => sent_message_summary.recipient.display_name %></td>
+  <td class="inbox-sender" bgcolor="<%= this_colour %>"><%= link_to h(sent_message_summary.recipient.display_name), :controller => 'user', :action => sent_message_summary.recipient.display_name %></td>
-  <td class="inbox-subject" bgcolor='<%= this_colour %>'><%= link_to h(sent_message_summary.title), :controller => 'message', :action => 'read', :message_id => sent_message_summary.id  %></td>
+  <td class="inbox-subject" bgcolor="<%= this_colour %>"><%= link_to h(sent_message_summary.title), :controller => 'message', :action => 'read', :message_id => sent_message_summary.id  %></td>
-  <td class="inbox-sent" bgcolor='<%= this_colour %>'><%= sent_message_summary.sent_on %></td>
+  <td class="inbox-sent" bgcolor="<%= this_colour %>"><%= sent_message_summary.sent_on %></td>
 </tr>

app/views/message/new.rhtml

@@ -1,7 +1,4 @@
-<% user_id = params[:user_id] || @user_id %>
+<h2>Send a new message to <%= h(@to_user.display_name) %></h2>
-<% display_name = User.find_by_id(user_id).display_name %>
-
-<h2>Send a new message to <%= h(display_name) %></h2>
 
 <% if params[:display_name] %>
 <p>Writing a new message to <%= h(params[:display_name]) %></p>  
@@ -10,7 +7,7 @@
 
 <%= error_messages_for 'message' %>
 
-<% form_for :message, :url => { :action => "new", :user_id => user_id } do |f| %>
+<% form_for :message, :url => { :action => "new", :user_id => @to_user.id } do |f| %>
   <table>
     <tr valign="top">
       <th>Subject</th>

app/views/message/no_such_user.rhtml

@@ -0,0 +1,2 @@
+<h1>No such user or message</h1>
+<p>Sorry there is no user or message with that name or id</p>

app/views/site/edit.rhtml

@@ -21,21 +21,37 @@
 <%= render :partial => 'sidebar', :locals => { :onopen => "resizeMap();", :onclose => "resizeMap();" } %>
 <%= render :partial => 'search' %>
 
-<% session[:token] = @user.tokens.create.token unless session[:token] %>
+<%
+session[:token] = @user.tokens.create.token unless session[:token]
 
-<% if params['mlon'] and params['mlat'] %>
+# Decide on a lat lon to initialise potlatch with. Various ways of doing this
-	<% lon =  h(params['mlon']) %>
+if params['lon'] and params['lat']
-	<% lat =  h(params['mlat'])  %>
+	lon =  h(params['lon']) 
-	<% zoom =  h(params['zoom'] || '14') %>
+	lat =  h(params['lat']) 
-<% elsif @user and params['lon'].nil? and params['lat'].nil? and params['gpx'].nil? %> 
+	zoom =  h(params['zoom'])
-	<% lon =  @user.home_lon %>
+
-	<% lat =  @user.home_lat %>
+elsif params['mlon'] and params['mlat'] 
-	<% zoom = '14' %>
+	lon =  h(params['mlon']) 
-<% else %>
+	lat =  h(params['mlat']) 
-	<% lon =  h(params['lon'] || 'null') %>
+	zoom =  h(params['zoom'])
-	<% lat =  h(params['lat'] || 'null') %>
+	
-	<% zoom =  h(params['zoom'] || '14') %>
+elsif params['gpx']
-<% end %>
+	#use gpx id to locate (dealt with below)
+	
+elsif cookies.key?("_osm_location")
+	lon,lat,zoom,layers = cookies["_osm_location"].split("|")
+	
+elsif @user and !@user.home_lon.nil? and !@user.home_lat.nil? 
+	lon =  @user.home_lon
+	lat =  @user.home_lat
+
+else
+	#catch all.  Do nothing.  lat=nil, lon=nil
+	#Currently this results in potlatch starting up at 0,0 (Atlantic ocean).
+end
+
+zoom='14' if zoom.nil?
+%>
 
 <div id="map">
 	You need a Flash player to use Potlatch, the

app/views/site/index.rhtml

@@ -32,51 +32,59 @@
   </table>
 </div>
 
-<% if params['mlon'] and params['mlat'] %>
+<%
-<% marker = true %>
+if params['mlon'] and params['mlat'] 
-<% mlon = h(params['mlon']) %> 
+	marker = true
-<% mlat = h(params['mlat']) %>
+	mlon = h(params['mlon'])
-<% end %>
+	mlat = h(params['mlat'])
+end
 
-<% if params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat'] %>
+if params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat']
-<% bbox = true %>
+	bbox = true
-<% minlon = h(params['minlon']) %>
+	minlon = h(params['minlon'])
-<% minlat = h(params['minlat']) %>
+	minlat = h(params['minlat'])
-<% maxlon = h(params['maxlon']) %>
+	maxlon = h(params['maxlon'])
-<% maxlat = h(params['maxlat']) %>
+	maxlat = h(params['maxlat'])
-<% end %>
+	box = true if params['box']=="yes" 
+end
 
-<% if params['lon'] and params['lat'] %>
+# Decide on a lat lon to initialise the map with. Various ways of doing this
-<% lon =  h(params['lon']) %>
+if params['lon'] and params['lat'] 
-<% lat =  h(params['lat']) %>
+	lon =  h(params['lon'])
-<% zoom =  h(params['zoom'] || '5') %>
+	lat =  h(params['lat'])
-<% layers = h(params['layers']) %>
+	zoom =  h(params['zoom'] || '5')
-<% elsif params['mlon'] and params['mlat'] %>
+	layers = h(params['layers'])
-<% lon = h(params['mlon']) %> 
+	
-<% lat = h(params['mlat']) %>
+elsif params['mlon'] and params['mlat']
-<% zoom =  h(params['zoom'] || '12') %>
+	lon = h(params['mlon']) 
-<% layers = h(params['layers']) %>
+	lat = h(params['mlat'])
-<% elsif cookies.key?("_osm_location") %>
+	zoom =  h(params['zoom'] || '12')
-<% lon,lat,zoom,layers = cookies["_osm_location"].split("|") %>
+	layers = h(params['layers'])
-<% elsif @user and !@user.home_lon.nil? and !@user.home_lat.nil? %> 
+	
-<% lon =  @user.home_lon %>
+elsif cookies.key?("_osm_location")
-<% lat =  @user.home_lat %>
+	lon,lat,zoom,layers = cookies["_osm_location"].split("|")
-<% zoom = '10' %>
+	
-<% else %>
+elsif @user and !@user.home_lon.nil? and !@user.home_lat.nil? 
-<% session[:location] = OSM::IPLocation(request.env['REMOTE_ADDR']) unless session[:location] %>
+	lon =  @user.home_lon
-<% if session[:location] %>
+	lat =  @user.home_lat
-<% bbox = true %>
+	zoom = '10'
-<% minlon = session[:location][:minlon] %>
+else
-<% minlat = session[:location][:minlat] %>
+	session[:location] = OSM::IPLocation(request.env['REMOTE_ADDR']) unless session[:location]
-<% maxlon = session[:location][:maxlon] %>
+
-<% maxlat = session[:location][:maxlat] %>
+	if session[:location]
-<% else %>
+		bbox = true
-<% lon =  '-0.1' %>
+		minlon = session[:location][:minlon]
-<% lat =  '51.5' %>
+		minlat = session[:location][:minlat]
-<% zoom =  h(params['zoom'] || '5') %>
+		maxlon = session[:location][:maxlon]
-<% end %>
+		maxlat = session[:location][:maxlat]
-<% layers = h(params['layers']) %>
+	else
-<% end %>
+		lon =  '-0.1' 
+		lat =  '51.5' 
+		zoom =  h(params['zoom'] || '5') 
+	end
+	layers = h(params['layers']) 
+end
+%>
 
 <%= javascript_include_tag '/openlayers/OpenLayers.js' %>
 <%= javascript_include_tag '/openlayers/OpenStreetMap.js' %>
@@ -102,6 +110,10 @@
     var bbox = new OpenLayers.Bounds(<%= minlon %>, <%= minlat %>, <%= maxlon %>, <%= maxlat %>);
 
     setMapExtent(bbox);
+    <%    if box %>
+    // IE requires Vector layers be initialised on page load, and not under deferred script conditions
+    Event.observe(window, 'load', function() {addBoxToMap(bbox)});
+    <%    end %>
     <% else %>
     var centre = new OpenLayers.LonLat(<%= lon %>, <%= lat %>);
     var zoom = <%= zoom %>;
@@ -157,8 +169,9 @@
     var lonlat = getMapCenter();
     var zoom = map.getZoom();
     var layers = getMapLayers();
+    var extents = getMapExtent();
 
-    updatelinks(lonlat.lon, lonlat.lat, zoom, layers);
+    updatelinks(lonlat.lon, lonlat.lat, zoom, layers, extents);
 
     document.cookie = "_osm_location=" + lonlat.lon + "|" + lonlat.lat + "|" + zoom + "|" + layers;
   }

app/views/trace/_trace_header.rhtml

@@ -17,9 +17,9 @@
   <% end %>
 </p>
 
-<% if @user and @user.traces.count(:conditions => "inserted=0") > 4 %>
+<% if @user and @user.traces.count(:conditions => ["inserted=?", false]) > 4 %>
   <p>
-    You have <%= @user.traces.count(:conditions => "inserted=0") %> traces 
+    You have <%= @user.traces.count(:conditions => ["inserted=?", false]) %> traces 
     waiting for upload. Please consider waiting for these to finish before 
     uploading any more, so as not to block the queue for other users.
   </p>

app/views/trace/no_such_user.rhtml

@@ -0,0 +1,2 @@
+<h2><%= h(@not_found_user) %></h2>
+<p>Sorry, there is no user with the name <%= @not_found_user -%>. Please check your spelling, or maybe the link you clicked is wrong.</p>

app/views/user/_friend_map.rhtml

@@ -56,8 +56,10 @@
       var nearmarker = addMarkerToMap(new OpenLayers.LonLat(nearest[i].home_lon, nearest[i].home_lat), near_icon.clone(), description);
     }
 
+    if (document.getElementById('updatehome')) {
       map.events.register("click", map, setHome);
     }
+  }        
 
   function setHome( e ) { 
     closeMapPopup();

app/views/user/account.rhtml

@@ -1,34 +1,51 @@
-<h2>User details</h2>
+<h2>My settings</h2>
 <%= error_messages_for 'user' %>
 <% form_for :user, @user do |f| %>
-<table style="width : 100%">
+<table id="accountForm">
-  <tr><td>Email</td><td><%= f.text_field :email %></td></tr>
+  <tr><td class="fieldName">Display Name : </td><td><%= f.text_field :display_name %></td></tr>
-  <tr><td>Mapper since</td><td><%= @user.creation_time %> (<%= time_ago_in_words(@user.creation_time) %> ago)</td></tr>
+  <tr><td class="fieldName">Email : </td><td><%= f.text_field :email, {:size => 50, :maxlength => 255} %> <span class="minorNote">(never displayed publicly)</span></td></tr>
-  <tr><td>Display Name</td><td><%= f.text_field :display_name %></td></tr>
+  <tr><td class="fieldName" style="padding-bottom:0px;">Password : </td><td style="padding-bottom:0px;"><%= f.password_field :pass_crypt, {:value => '', :size => 30, :maxlength => 255} %></td></tr>
-  <tr><td>Password</td><td><%= f.password_field :pass_crypt, {:value => '', :size => 50, :maxlength => 255} %></td></tr>
+  <tr><td class="fieldName">Confirm Password : </td><td><%= f.password_field :pass_crypt_confirmation, {:value => '', :size => 30, :maxlength => 255} %></td></tr>
-  <tr><td>Confirm Password</td><td><%= f.password_field :pass_crypt_confirmation, {:value => '', :size => 50, :maxlength => 255} %></td></tr>
 
-  <tr><td valign="top">Description</td><td><%= f.text_area :description, :class => "editDescription" %></td></tr>
+  <tr>
+  <td class="fieldName" valign="top">Public editing :</td>
+  <td>
+<% if @user.data_public? %>
+  Enabled. Not anonymous <span class="minorNote">(<a href="http://wiki.openstreetmap.org/index.php/Disabling_anonymous_edits" target="_new">what's this?</a>)</span>
+<% else %>
+  Disabled and anonymous. <span class="minorNote">(<a href="#public">why's this bad?</a>)</span>
+<% end %>
+  </td>
+  </tr>
 
-  <tr id="homerow" <% unless @user.home_lat and @user.home_lon %> class="nohome" <%end%> ><td>Your home</td><td><em class="message">You have not entered your home location.</em><span class="location">Latitude: <%= f.text_field :home_lat, :size => 20, :id => "home_lat" %> Longitude <%= f.text_field :home_lon, :size => 20, :id => "home_lon" %></span></td></tr>
+  <tr><td class="fieldName" valign="top">Profile Description : </td><td><%= f.text_area :description, :rows => '5', :cols => '60' %><br /><br /></td></tr>
+
+  <tr id="homerow" <% unless @user.home_lat and @user.home_lon %> class="nohome" <%end%> ><td class="fieldName">Home Location : </td><td><em class="message">You have not entered your home location.</em><span class="location">Latitude: <%= f.text_field :home_lat, :size => 20, :id => "home_lat" %> Longitude <%= f.text_field :home_lon, :size => 20, :id => "home_lon" %></span></td></tr>
 
   <tr><td></td><td>
   <p>Update home location when I click on the map? <input type="checkbox" value="1" <% unless @user.home_lat and @user.home_lon %> checked="checked" <% end %> id="updatehome" /> </p>
-  <div id="map" style="border: 1px solid black; position: relative; width : 90%; height : 400px;"></div>
+  <div id="map" style="border:1px solid black; position:relative; width:500px; height:400px;"></div>
   </td></tr>
+  
+  <tr><td></td><td align=right><br/></br><%= submit_tag 'Save Changes' %></td></tr>
 </table>
-<%= submit_tag 'Save Changes' %>
+<br/>
+
 <% end %>
 
 <%= render :partial => 'friend_map' %>
-
+<% unless @user.data_public? %>
+<a name="public"></a>
 <h2>Public editing</h2>
-<% if @user.data_public? %>
+  Currently your edits are anonymous and people can't send you messages or see your location. To show what you edited and allow people to contact you through the website, click the button below.
-  All your edits are public.
+  <b>You will need to do this if you want to use the online editor and it is encouraged to do so</b> (<a href="http://wiki.openstreetmap.org/index.php/Disabling_anonymous_edits">find out why</a>).<br />
-<% else %>
+  Your email address will not be revealed by becoming public.<br />
-Currently your edits are anonymous and people can't send you messages or see your location. To show what you edited and allow people to contact you through the website, click the button below.
+  This action cannot be reversed and all new users are now public by default.<br />
-<b>You will need to do this if you want to use the online editor and it is encouraged</b> (<a href="http://wiki.openstreetmap.org/index.php/Disabling_anonymous_edits">find out why</a>).
-This action cannot be reversed and all new users are now public by default.
   <br /><br />
   <%= button_to "Make all my edits public", :action => :go_public %>
 <% end %>
+<br/>
+<br/>
+<%= link_to 'return to profile', :controller => 'user', :action => @user.display_name %>
+<br/>
+<br/>

app/views/user/login.rhtml

@@ -1,13 +1,13 @@
-<h1>Login:</h1><br />
+<h1>Login</h1>
-Please login or <%= link_to 'create an account', :controller => 'user', :action => 'new' %>.<br />
+
+<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])) %>
 <table>
-  <tr><td>Email Address or username:</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
+  <tr><td class="fieldName">Email Address or Username:</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255, :tabindex => 1}) %></td></tr>
-  <tr><td>Password:</td><td><%= password_field('user', 'password',{:size => 50, :maxlength => 255}) %></td></tr>
+  <tr><td class="fieldName">Password:</td><td><%= password_field('user', 'password',{:size => 28, :maxlength => 255, :tabindex => 2}) %> <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', :tabindex => 3 %></td></tr>
 </table>
-
+<% end %>
-<br />
-<%= submit_tag 'Login' %>
-<% end %> (<%= link_to 'Lost your password?', :controller => 'user', :action => 'lost_password' %>)

app/views/user/new.rhtml

@@ -1,4 +1,4 @@
-<h1>Create a user account</h1>
+<h1>Create a User Account</h1>
 
 <% if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"}) %>
 
@@ -27,17 +27,19 @@
 <%= error_messages_for 'user' %>
 
 <% form_tag :action => 'save' do %>
-<table>
+<table id="loginForm">
-  <tr><td>Email Address</td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %></td></tr>
+  <tr><td class="fieldName">Email Address : </td><td><%= text_field('user', 'email',{:size => 50, :maxlength => 255, :tabindex => 1}) %></td></tr>
-  <tr><td>Confirm Email Address</td><td><%= text_field('user', 'email_confirmation',{:size => 50, :maxlength => 255}) %></td></tr>
+  <tr><td class="fieldName">Confirm Email Address : </td><td><%= text_field('user', 'email_confirmation',{:size => 50, :maxlength => 255, :tabindex => 2}) %></td></tr>
-  <tr><td>Display Name</td><td><%= text_field('user', 'display_name',{: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>Password</td><td><%= password_field('user', 'pass_crypt',{:size => 50, :maxlength => 255}) %></td></tr>
+  <tr><td colspan=2>&nbsp;<!--vertical spacer--></td></tr>
-  <tr><td>Confirm Password</td><td><%= password_field('user', 'pass_crypt_confirmation',{:size => 50, :maxlength => 255}) %></td></tr>
+  <tr><td class="fieldName">Display Name : </td><td><%= text_field('user', 'display_name',{:size => 30, :maxlength => 255, :tabindex => 3}) %></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, :tabindex => 4}) %></td></tr>
+  <tr><td class="fieldName">Confirm Password : </td><td><%= password_field('user', 'pass_crypt_confirmation',{:size => 30, :maxlength => 255, :tabindex => 5}) %></td></tr>
+  
+  <tr><td colspan=2>&nbsp;<!--vertical spacer--></td></tr>
+  <tr><td></td><td align=right><input type="submit" value="Signup" tabindex="6"></td></tr>
 </table>
-<br>
-<br>
-<input type="submit" value="Signup">
-
 <% end %>
 
 <% end %>

app/views/user/view.rhtml

@@ -2,13 +2,17 @@
 <h2><%= h(@this_user.display_name) %></h2>
 <div id="userinformation">
 <% if @user and @this_user.id == @user.id %>
+<!-- Displaying user's own profile page -->
 <%= link_to 'my diary', :controller => 'diary_entry', :action => 'list', :display_name => @user.display_name %>
 | <%= link_to 'new diary entry', :controller => 'diary_entry', :action => 'new', :display_name => @user.display_name %>
+| <%= link_to 'my edits', :controller => 'changeset', :action => 'list_user', :display_name => @user.display_name %>
 | <%= link_to 'my traces', :controller => 'trace', :action=>'mine' %>
 | <%= link_to 'my settings', :controller => 'user', :action => 'account', :display_name => @user.display_name %>
 <% else %>
+<!-- Displaying another user's profile page -->
 <%= link_to 'send message', :controller => 'message', :action => 'new', :user_id => @this_user.id %>
 | <%= link_to 'diary', :controller => 'diary_entry', :action => 'list', :display_name => @this_user.display_name %>
+| <%= link_to 'edits', :controller => 'changeset', :action => 'list_user', :display_name => @this_user.display_name %>
 | <%= link_to 'traces', :controller => 'trace', :action => 'view', :display_name => @this_user.display_name %>
 | <% if @user and @user.is_friends_with?(@this_user) %>
   <%= link_to 'remove as friend', :controller => 'user', :action => 'remove_friend', :display_name => @this_user.display_name %>
@@ -18,6 +22,12 @@
 <% end %>
 </div>
 
+<% if @this_user != nil %>
+<P>
+<b>Mapper since : </b><%= @this_user.creation_time %> (<%= time_ago_in_words(@this_user.creation_time) %> ago)
+</P>
+<% end %>
+  
 <h3>User image</h3>
 <% if @this_user.image %>
   <%= image_tag url_for_file_column(@this_user, "image") %>
@@ -92,3 +102,9 @@
     <% end %>
   <% end %>
 <% end %>
+
+<br/>
+<br/>
+<% if @user and @this_user.id == @user.id %>
+<%= link_to 'change your settings', :controller => 'user', :action => 'account', :display_name => @user.display_name %>
+<% end %>

config/application.yml

@@ -3,8 +3,10 @@ standard_settings: &standard_settings
   max_request_area: 0.25
   # Number of GPS trace/trackpoints returned per-page
   tracepoints_per_page: 5000
-  # Maximum number of nodes
+  # Maximum number of nodes that will be returned by the api in a map request
   max_number_of_nodes: 50000
+  # Maximum number of nodes that can be in a way (checked on save)
+  max_number_of_way_nodes: 2000
  
 development:
   <<: *standard_settings

config/database.yml

@@ -16,6 +16,7 @@ development:
   username: openstreetmap
   password: openstreetmap
   host: localhost
+  encoding: utf8
 
 # Warning: The database defined as 'test' will be erased and
 # re-generated from your development database when you run 'rake'.
@@ -23,14 +24,15 @@ development:
 test:
   adapter: mysql
   database: osm_test
-  username: root
+  username: osm_test
-  password:
+  password: osm_test
   host: localhost
+  encoding: utf8
 
 production:
   adapter: mysql
-  database: openstreetmap
+  database: osm
-  username: openstreetmap
+  username: osm
-  password: openstreetmap
+  password: osm
-  host: db.openstreetmap.org
+  host: localhost
-
+  encoding: utf8

config/environment.rb

@@ -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:
 #
@@ -38,6 +41,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 => '>= 1.1.1', :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
@@ -64,6 +77,12 @@ Rails::Initializer.run do |config|
   # (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
@@ -73,5 +92,5 @@ Rails::Initializer.run do |config|
   # 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

config/environments/development.rb

@@ -12,7 +12,6 @@ config.whiny_nils = true
 config.action_controller.consider_all_requests_local = true
 config.action_view.debug_rjs                         = true
 config.action_controller.perform_caching             = false
-config.action_view.cache_template_extensions         = false
 
 # Don't care if the mailer can't send
 config.action_mailer.raise_delivery_errors = false

config/initializers/composite_primary_keys.rb

@@ -1,3 +0,0 @@
-require 'rubygems'
-gem 'composite_primary_keys', '= 0.9.93'
-require 'composite_primary_keys'

config/initializers/libxml.rb

@@ -1,7 +1,3 @@
-require 'rubygems'
-gem 'libxml-ruby', '>= 1.1.1'
-require 'libxml'
-
 # This is required otherwise libxml writes out memory errors to
 # the standard output and exits uncleanly
 LibXML::XML::Error.set_handler do |message|

config/initializers/sql_session_store.rb

@@ -1,2 +1,7 @@
-# Use the MySQL interface for SqlSessionStore
+# Work out which session store adapter to use
-SqlSessionStore.session_class = MysqlSession
+environment = Rails.configuration.environment
+adapter = Rails.configuration.database_configuration[environment]["adapter"]
+session_class = adapter + "_session"
+
+# Configure SqlSessionStore
+SqlSessionStore.session_class = session_class.camelize.constantize

config/potlatch/colours.txt

@@ -12,6 +12,7 @@ secondary		0xFDBF6F	1	-
 tertiary		0xFEFECB	1	-
 unclassified	0xE8E8E8	1	-
 residential		0xE8E8E8	1	-
+road			0xAAAAAA	1	-
 footway			0xFF6644	-	-
 cycleway		0xFF6644	-	-
 bridleway		0xFF6644	-	-

config/routes.rb

@@ -3,10 +3,20 @@ ActionController::Routing::Routes.draw do |map|
   # API
   map.connect "api/capabilities", :controller => 'api', :action => 'capabilities'
   
+  map.connect "api/#{API_VERSION}/changeset/create", :controller => 'changeset', :action => 'create'
+  map.connect "api/#{API_VERSION}/changeset/:id/upload", :controller => 'changeset', :action => 'upload', :id => /\d+/
+  map.connect "api/#{API_VERSION}/changeset/:id/download", :controller => 'changeset', :action => 'download', :id => /\d+/
+  map.connect "api/#{API_VERSION}/changeset/:id/expand_bbox", :controller => 'changeset', :action => 'expand_bbox', :id => /\d+/
+  map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'read', :id => /\d+/, :conditions => { :method => :get }
+  map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'update', :id => /\d+/, :conditions => { :method => :put }
+  map.connect "api/#{API_VERSION}/changeset/:id/close", :controller => 'changeset', :action => 'close', :id =>/\d+/
+  map.connect "api/#{API_VERSION}/changesets", :controller => 'changeset', :action => 'query'
+  
   map.connect "api/#{API_VERSION}/node/create", :controller => 'node', :action => 'create'
   map.connect "api/#{API_VERSION}/node/:id/ways", :controller => 'way', :action => 'ways_for_node', :id => /\d+/
   map.connect "api/#{API_VERSION}/node/:id/relations", :controller => 'relation', :action => 'relations_for_node', :id => /\d+/
   map.connect "api/#{API_VERSION}/node/:id/history", :controller => 'old_node', :action => 'history', :id => /\d+/
+  map.connect "api/#{API_VERSION}/node/:id/:version", :controller => 'old_node', :action => 'version', :id => /\d+/, :version => /\d+/
   map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'read', :id => /\d+/, :conditions => { :method => :get }
   map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'update', :id => /\d+/, :conditions => { :method => :put }
   map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete }
@@ -16,12 +26,12 @@ ActionController::Routing::Routes.draw do |map|
   map.connect "api/#{API_VERSION}/way/:id/history", :controller => 'old_way', :action => 'history', :id => /\d+/
   map.connect "api/#{API_VERSION}/way/:id/full", :controller => 'way', :action => 'full', :id => /\d+/
   map.connect "api/#{API_VERSION}/way/:id/relations", :controller => 'relation', :action => 'relations_for_way', :id => /\d+/
+  map.connect "api/#{API_VERSION}/way/:id/:version", :controller => 'old_way', :action => 'version', :id => /\d+/, :version => /\d+/
   map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'read', :id => /\d+/, :conditions => { :method => :get }
   map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'update', :id => /\d+/, :conditions => { :method => :put }
   map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete }
   map.connect "api/#{API_VERSION}/ways", :controller => 'way', :action => 'ways', :id => nil
 
-  map.connect "api/#{API_VERSION}/capabilities", :controller => 'api', :action => 'capabilities'
   map.connect "api/#{API_VERSION}/relation/create", :controller => 'relation', :action => 'create'
   map.connect "api/#{API_VERSION}/relation/:id/relations", :controller => 'relation', :action => 'relations_for_relation', :id => /\d+/
   map.connect "api/#{API_VERSION}/relation/:id/history", :controller => 'old_relation', :action => 'history', :id => /\d+/
@@ -54,14 +64,14 @@ ActionController::Routing::Routes.draw do |map|
   map.connect "api/#{API_VERSION}/gpx/:id/details", :controller => 'trace', :action => 'api_details'
   map.connect "api/#{API_VERSION}/gpx/:id/data", :controller => 'trace', :action => 'api_data'
   
-  # Potlatch API
+  # AMF (ActionScript) API
   
   map.connect "api/#{API_VERSION}/amf/read", :controller =>'amf', :action =>'amf_read'
   map.connect "api/#{API_VERSION}/amf/write", :controller =>'amf', :action =>'amf_write'
   map.connect "api/#{API_VERSION}/swf/trackpoints", :controller =>'swf', :action =>'trackpoints'
   
   # Data browsing
-  map.connect '/browse', :controller => 'browse', :action => 'index'
+  map.connect '/browse', :controller => 'changeset', :action => 'list'
   map.connect '/browse/start', :controller => 'browse', :action => 'start'
   map.connect '/browse/way/:id', :controller => 'browse', :action => 'way', :id => /\d+/
   map.connect '/browse/way/:id/history', :controller => 'browse', :action => 'way_history', :id => /\d+/
@@ -69,11 +79,13 @@ ActionController::Routing::Routes.draw do |map|
   map.connect '/browse/node/:id/history', :controller => 'browse', :action => 'node_history', :id => /\d+/
   map.connect '/browse/relation/:id', :controller => 'browse', :action => 'relation', :id => /\d+/
   map.connect '/browse/relation/:id/history', :controller => 'browse', :action => 'relation_history', :id => /\d+/
+  map.connect '/browse/changeset/:id', :controller => 'browse', :action => 'changeset', :id => /\d+/
+  map.connect '/browse/changesets', :controller => 'changeset', :action => 'list'
   
   # web site
-
   map.connect '/', :controller => 'site', :action => 'index'
   map.connect '/edit', :controller => 'site', :action => 'edit'
+  map.connect '/history', :controller => 'changeset', :action => 'list_bbox'
   map.connect '/export', :controller => 'site', :action => 'export'
   map.connect '/login', :controller => 'user', :action => 'login'
   map.connect '/logout', :controller => 'user', :action => 'logout'
@@ -90,6 +102,7 @@ ActionController::Routing::Routes.draw do |map|
 
   map.connect '/index.html', :controller => 'site', :action => 'index'
   map.connect '/edit.html', :controller => 'site', :action => 'edit'
+  map.connect '/history.html', :controller => 'changeset', :action => 'list_bbox'
   map.connect '/export.html', :controller => 'site', :action => 'export'
   map.connect '/search.html', :controller => 'way_tag', :action => 'search'
   map.connect '/login.html', :controller => 'user', :action => 'login'
@@ -126,6 +139,7 @@ ActionController::Routing::Routes.draw do |map|
 
   # user pages
   map.connect '/user/:display_name', :controller => 'user', :action => 'view'
+  map.connect '/user/:display_name/edits', :controller => 'changeset', :action => 'list_user'
   map.connect '/user/:display_name/make_friend', :controller => 'user', :action => 'make_friend'
   map.connect '/user/:display_name/remove_friend', :controller => 'user', :action => 'remove_friend'
   map.connect '/user/:display_name/diary', :controller => 'diary_entry', :action => 'list'
@@ -141,6 +155,7 @@ ActionController::Routing::Routes.draw do |map|
   map.connect '/diary/:language', :controller => 'diary_entry', :action => 'list'
   map.connect '/diary/:language/rss', :controller => 'diary_entry', :action => 'rss'
   
+  
   # test pages
   map.connect '/test/populate/:table/:from/:count', :controller => 'test', :action => 'populate'
   map.connect '/test/populate/:table/:count', :controller => 'test', :action => 'populate', :from => 1

db/README

@@ -25,12 +25,12 @@ $ mysql -u <uid> -p
 > flush privileges;
 > exit
 
-Creating functions
+Creating functions For MySQL
-====================
+==============================
 
 Run this command in the db/functions directory:
 
-$ make
+$ make libmyosm.so
 
 Make sure the db/functions directory is on the MySQL server's library
 path and restart the MySQL server. 
@@ -49,6 +49,22 @@ $ mysql -u <uid> -p openstreetmap
 > create function maptile_for_point returns integer soname 'libmyosm.so';
 > exit
 
+Creating functions for PgSQL
+==============================
+
+Run this command in the db/functions directory:
+
+$ make libpgosm.so
+
+Now create the function as follows:
+
+$ psql openstreetmap
+(This may need authentication or a -u <dbowneruid>)
+
+> CREATE FUNCTION maptile_for_point(int8, int8, int4) RETURNS int4 
+  AS '/path/to/rails-port/db/functions/libpgosm.so', 'maptile_for_point'
+  LANGUAGE C STRICT;
+
 Creating database skeleton tables
 ===================================
 

db/functions/Makefile

@@ -7,11 +7,22 @@ else
     LDFLAGS=-shared
 endif
 
-libmyosm.so: quadtile.o maptile.o
+all: libmyosm.so libpgosm.so
-	cc ${LDFLAGS} -o libmyosm.so quadtile.o maptile.o
+
+clean:
+	$(RM) *.so *.o
+
+libmyosm.so: quadtile.o maptile-mysql.o
+	cc ${LDFLAGS} -o libmyosm.so quadtile.o maptile-mysql.o
+
+libpgosm.so: maptile-pgsql.o
+	cc ${LDFLAGS} -o libpgosm.so maptile-pgsql.o
 
 quadtile.o: quadtile.c ${QTDIR}/quad_tile.h
 	cc `mysql_config --include` -I${QTDIR} -fPIC -O3 -c -o quadtile.o quadtile.c
 
-maptile.o: maptile.c
+maptile-mysql.o: maptile.c
-	cc `mysql_config --include` -fPIC -O3 -c -o maptile.o maptile.c
+	cc `mysql_config --include` -fPIC -O3 -DUSE_MYSQL -c -o maptile-mysql.o maptile.c
+
+maptile-pgsql.o: maptile.c
+	cc -I `pg_config --includedir-server` -O3 -fPIC -DUSE_PGSQL -c -o maptile-pgsql.o maptile.c

db/functions/maptile.c

@@ -1,3 +1,31 @@
+#ifndef USE_MYSQL
+#ifndef USE_PGSQL
+#error One of USE_MYSQL or USE_PGSQL must be defined
+#endif
+#endif
+
+#include <math.h>
+
+/* The real maptile-for-point functionality is here */
+
+static long long internal_maptile_for_point(double lat, double lon, long long zoom)
+{
+   double       scale = pow(2, zoom);
+   double       r_per_d = M_PI / 180;
+   unsigned int x;
+   unsigned int y;
+
+   x = floor((lon + 180.0) * scale / 360.0);
+   y = floor((1 - log(tan(lat * r_per_d) + 1.0 / cos(lat * r_per_d)) / M_PI) * scale / 2.0);
+
+   return (x << zoom) | y;
+}
+
+#ifdef USE_MYSQL
+#ifdef USE_PGSQL
+#error ONLY one of USE_MYSQL and USE_PGSQL should be defined
+#endif
+
 #include <my_global.h>
 #include <my_sys.h>
 #include <m_string.h>
@@ -27,13 +55,39 @@ long long maptile_for_point(UDF_INIT *initid, UDF_ARGS *args, char *is_null, cha
    double       lat = *(long long *)args->args[0] / 10000000.0;
    double       lon = *(long long *)args->args[1] / 10000000.0;
    long long    zoom = *(long long *)args->args[2];
-   double       scale = pow(2, zoom);
-   double       r_per_d = M_PI / 180;
-   unsigned int x;
-   unsigned int y;
    
-   x = floor((lon + 180.0) * scale / 360.0);
+   return internal_maptile_for_point(lat, lon, zoom);
-   y = floor((1 - log(tan(lat * r_per_d) + 1.0 / cos(lat * r_per_d)) / M_PI) * scale / 2.0);
-
-   return (x << zoom) | y;
 }
+#endif
+
+#ifdef USE_PGSQL
+#include <postgres.h>
+#include <fmgr.h>
+
+Datum
+maptile_for_point(PG_FUNCTION_ARGS)
+{
+  double lat = PG_GETARG_INT64(0) / 10000000.0;
+  double lon = PG_GETARG_INT64(1) / 10000000.0;
+  int zoom = PG_GETARG_INT32(2);
+  
+  PG_RETURN_INT32(internal_maptile_for_point(lat, lon, zoom));
+}
+
+PG_FUNCTION_INFO_V1(maptile_for_point);
+
+/*
+ * To bind this into PGSQL, try something like:
+ *
+ * CREATE FUNCTION maptile_for_point(int8, int8, int4) RETURNS int4 
+ *  AS '/path/to/rails-port/db/functions/libpgosm', 'maptile_for_point'
+ *  LANGUAGE C STRICT;
+ *
+ * (without all the *s)
+ */
+
+#ifdef PG_MODULE_MAGIC
+PG_MODULE_MAGIC;
+#endif
+
+#endif

db/migrate/001_create_osm_db.rb

@@ -16,7 +16,7 @@ class CreateOsmDb < ActiveRecord::Migration
     add_index "current_nodes", ["latitude", "longitude"], :name => "current_nodes_lat_lon_idx"
     add_index "current_nodes", ["timestamp"], :name => "current_nodes_timestamp_idx"
 
-    change_column "current_nodes", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
+    change_column :current_nodes, :id, :bigint_auto_64
 
     create_table "current_segments", innodb_table do |t|
       t.column "id",        :bigint,   :limit => 64,                 :null => false
@@ -32,7 +32,7 @@ class CreateOsmDb < ActiveRecord::Migration
     add_index "current_segments", ["node_a"], :name => "current_segments_a_idx"
     add_index "current_segments", ["node_b"], :name => "current_segments_b_idx"
 
-    change_column "current_segments", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
+    change_column :current_segments, :id, :bigint_auto_64
 
     create_table "current_way_segments", innodb_table do |t|
       t.column "id",          :bigint, :limit => 64
@@ -50,21 +50,17 @@ class CreateOsmDb < ActiveRecord::Migration
     end
 
     add_index "current_way_tags", ["id"], :name => "current_way_tags_id_idx"
-    execute "CREATE FULLTEXT INDEX `current_way_tags_v_idx` ON `current_way_tags` (`v`)"
+    add_fulltext_index "current_way_tags", "v"
 
     create_table "current_ways", myisam_table do |t|
-      t.column "id",        :bigint,   :limit => 64, :null => false
+      t.column "id",        :bigint_pk_64, :null => false
       t.column "user_id",   :bigint,   :limit => 20
       t.column "timestamp", :datetime
       t.column "visible",   :boolean
     end
 
-    add_primary_key "current_ways", ["id"]
-
-    change_column "current_ways", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "diary_entries", myisam_table do |t|
-      t.column "id",         :bigint,   :limit => 20, :null => false
+      t.column "id",         :bigint_pk, :null => false
       t.column "user_id",    :bigint,   :limit => 20, :null => false
       t.column "title",      :string
       t.column "body",       :text
@@ -72,21 +68,14 @@ class CreateOsmDb < ActiveRecord::Migration
       t.column "updated_at", :datetime
     end
 
-    add_primary_key "diary_entries", ["id"]
-
-    change_column "diary_entries", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "friends", myisam_table do |t|
-      t.column "id",             :bigint,  :limit => 20, :null => false
+      t.column "id",             :bigint_pk, :null => false
       t.column "user_id",        :bigint,  :limit => 20, :null => false
       t.column "friend_user_id", :bigint,  :limit => 20, :null => false
     end
 
-    add_primary_key "friends", ["id"]
     add_index "friends", ["friend_user_id"], :name => "user_id_idx"
 
-    change_column "friends", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "gps_points", myisam_table do |t|
       t.column "altitude",  :float
       t.column "user_id",   :integer,  :limit => 20
@@ -104,16 +93,13 @@ class CreateOsmDb < ActiveRecord::Migration
     create_table "gpx_file_tags", myisam_table do |t|
       t.column "gpx_id", :bigint,  :limit => 64, :default => 0, :null => false
       t.column "tag",    :string
-      t.column "id",     :integer, :limit => 20, :null => false
+      t.column "id",     :bigint_pk, :null => false
     end
 
-    add_primary_key "gpx_file_tags", ["id"]
     add_index "gpx_file_tags", ["gpx_id"], :name => "gpx_file_tags_gpxid_idx"
 
-    change_column "gpx_file_tags", "id", :integer, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "gpx_files", myisam_table do |t|
-      t.column "id",          :bigint,   :limit => 64,                   :null => false
+      t.column "id",          :bigint_pk_64,                   :null => false
       t.column "user_id",     :bigint,   :limit => 20
       t.column "visible",     :boolean,                :default => true, :null => false
       t.column "name",        :string,                 :default => "",   :null => false
@@ -126,12 +112,9 @@ class CreateOsmDb < ActiveRecord::Migration
       t.column "inserted",    :boolean
     end
 
-    add_primary_key "gpx_files", ["id"]
     add_index "gpx_files", ["timestamp"], :name => "gpx_files_timestamp_idx"
     add_index "gpx_files", ["visible", "public"], :name => "gpx_files_visible_public_idx"
 
-    change_column "gpx_files", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "gpx_pending_files", myisam_table do |t|
       t.column "originalname", :string
       t.column "tmpname",      :string
@@ -139,7 +122,7 @@ class CreateOsmDb < ActiveRecord::Migration
     end
 
     create_table "messages", myisam_table do |t|
-      t.column "id",                :bigint,   :limit => 20,                    :null => false
+      t.column "id",                :bigint_pk,                                 :null => false
       t.column "user_id",           :bigint,   :limit => 20,                    :null => false
       t.column "from_user_id",      :bigint,   :limit => 20,                    :null => false
       t.column "from_display_name", :string,                 :default => ""
@@ -150,21 +133,14 @@ class CreateOsmDb < ActiveRecord::Migration
       t.column "to_user_id",        :bigint,   :limit => 20,                    :null => false
     end
 
-    add_primary_key "messages", ["id"]
     add_index "messages", ["from_display_name"], :name => "from_name_idx"
 
-    change_column "messages", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "meta_areas", myisam_table do |t|
-      t.column "id",        :bigint,  :limit => 64, :null => false
+      t.column "id",        :bigint_pk_64, :null => false
       t.column "user_id",   :bigint,  :limit => 20
       t.column "timestamp", :datetime
     end
 
-    add_primary_key "meta_areas", ["id"]
-
-    change_column "meta_areas", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "nodes", myisam_table do |t|
       t.column "id",        :bigint,  :limit => 64
       t.column "latitude",  :double
@@ -194,7 +170,7 @@ class CreateOsmDb < ActiveRecord::Migration
 
     create_table "users", innodb_table do |t|
       t.column "email",         :string
-      t.column "id",            :bigint,   :limit => 20,                    :null => false
+      t.column "id",            :bigint_pk,                    :null => false
       t.column "token",         :string
       t.column "active",        :integer,                :default => 0,     :null => false
       t.column "pass_crypt",    :string
@@ -211,12 +187,9 @@ class CreateOsmDb < ActiveRecord::Migration
       t.column "home_zoom",     :integer,  :limit => 2,  :default => 3
     end
 
-    add_primary_key "users", ["id"]
     add_index "users", ["email"], :name => "users_email_idx"
     add_index "users", ["display_name"], :name => "users_display_name_idx"
 
-    change_column "users", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "way_segments", myisam_table do |t|
       t.column "id",          :bigint,  :limit => 64, :default => 0, :null => false
       t.column "segment_id",  :integer
@@ -226,7 +199,7 @@ class CreateOsmDb < ActiveRecord::Migration
 
     add_primary_key "way_segments", ["id", "version", "sequence_id"]
 
-    change_column "way_segments", "sequence_id", :bigint, :limit => 11, :null => false, :options => "AUTO_INCREMENT"
+    change_column "way_segments", "sequence_id", :bigint_auto_11
 
     create_table "way_tags", myisam_table do |t|
       t.column "id",      :bigint,  :limit => 64, :default => 0, :null => false
@@ -248,7 +221,7 @@ class CreateOsmDb < ActiveRecord::Migration
     add_primary_key "ways", ["id", "version"]
     add_index "ways", ["id"], :name => "ways_id_version_idx"
 
-    change_column "ways", "version", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
+    change_column "ways", "version", :bigint_auto_20
   end
 
   def self.down

db/migrate/002_cleanup_osm_db.rb

@@ -29,7 +29,7 @@ class CleanupOsmDb < ActiveRecord::Migration
     change_column "current_ways", "user_id", :bigint, :limit => 20, :null => false
     change_column "current_ways", "timestamp", :datetime, :null => false
     change_column "current_ways", "visible", :boolean, :null => false
-    execute "ALTER TABLE current_ways ENGINE = InnoDB"
+    change_engine "current_ways", "InnoDB"
 
     change_column "diary_entries", "title", :string, :null => false
     change_column "diary_entries", "body", :text, :null => false
@@ -39,7 +39,9 @@ class CleanupOsmDb < ActiveRecord::Migration
     add_index "friends", ["user_id"], :name => "friends_user_id_idx"
 
     remove_index "gps_points", :name => "points_uid_idx"
+    remove_index "gps_points", :name => "points_idx"
     remove_column "gps_points", "user_id"
+    add_index "gps_points", ["latitude", "longitude"], :name => "points_idx"
     change_column "gps_points", "trackid", :integer, :null => false
     change_column "gps_points", "latitude", :integer, :null => false
     change_column "gps_points", "longitude", :integer, :null => false
@@ -87,8 +89,8 @@ class CleanupOsmDb < ActiveRecord::Migration
     change_column "users", "creation_time", :datetime, :null => false
     change_column "users", "display_name", :string, :default => "", :null => false
     change_column "users", "data_public", :boolean, :default => false, :null => false
-    change_column "users", "home_lat", :double
+    change_column "users", "home_lat", :double, :default => nil
-    change_column "users", "home_lon", :double
+    change_column "users", "home_lon", :double, :default => nil
     remove_index "users", :name => "users_email_idx"
     add_index "users", ["email"], :name => "users_email_idx", :unique => true
     remove_index "users", :name => "users_display_name_idx"
@@ -150,15 +152,11 @@ class CleanupOsmDb < ActiveRecord::Migration
     change_column "nodes", "id", :bigint, :limit => 64
 
     create_table "meta_areas", myisam_table do |t|
-      t.column "id",        :bigint,  :limit => 64, :null => false
+      t.column "id",        :bigint_pk_64, :null => false
       t.column "user_id",   :bigint,  :limit => 20
       t.column "timestamp", :datetime
     end
 
-    add_primary_key "meta_areas", ["id"]
-
-    change_column "meta_areas", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
-
     remove_index "messages", :name => "messages_to_user_id_idx"
     change_column "messages", "message_read", :boolean, :default => false
     change_column "messages", "sent_on", :datetime
@@ -195,7 +193,7 @@ class CleanupOsmDb < ActiveRecord::Migration
     change_column "diary_entries", "body", :text
     change_column "diary_entries", "title", :string, :default => nil
 
-    execute "ALTER TABLE current_ways ENGINE = MyISAM"
+    change_engine "current_ways", "MyISAM"
     change_column "current_ways", "visible", :boolean
     change_column "current_ways", "timestamp", :datetime
     change_column "current_ways", "user_id", :bigint, :limit => 20
@@ -223,6 +221,6 @@ class CleanupOsmDb < ActiveRecord::Migration
     change_column "current_nodes", "user_id", :bigint, :limit => 20
     change_column "current_nodes", "longitude", :double
     change_column "current_nodes", "latitude", :double
-    change_column "current_nodes", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
+    change_column "current_nodes", "id", :bigint_auto_64
   end
 end

db/migrate/003_sql_session_store_setup.rb

@@ -1,6 +1,6 @@
 class SqlSessionStoreSetup < ActiveRecord::Migration
   def self.up
-    create_table "sessions", :options => "ENGINE=InnoDB" do |t|
+    create_table "sessions", :options => innodb_option do |t|
       t.column "session_id", :string
       t.column "data",       :text
       t.column "created_at", :timestamp

db/migrate/004_user_enhancements.rb

@@ -13,18 +13,15 @@ class UserEnhancements < ActiveRecord::Migration
     add_primary_key "user_preferences", ["user_id", "k"]
 
     create_table "user_tokens", innodb_table do |t|
-      t.column "id",      :bigint,   :limit => 20, :null => false
+      t.column "id",      :bigint_pk, :null => false
       t.column "user_id", :bigint,   :limit => 20, :null => false
       t.column "token",   :string,   :null => false
       t.column "expiry",  :datetime, :null => false
     end
 
-    add_primary_key "user_tokens", ["id"]
     add_index "user_tokens", ["token"], :name => "user_tokens_token_idx", :unique => true
     add_index "user_tokens", ["user_id"], :name => "user_tokens_user_id_idx"
 
-    change_column "user_tokens", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
-
     User.find(:all, :conditions => "token is not null").each do |user|
       UserToken.create(:user_id => user.id, :token => user.token, :expiry => 1.week.from_now)
     end

db/migrate/005_tile_tracepoints.rb

@@ -1,6 +1,6 @@
 class TileTracepoints < ActiveRecord::Migration
   def self.up
-    add_column "gps_points", "tile", :integer, :null => false, :unsigned => true
+    add_column "gps_points", "tile", :four_byte_unsigned
     add_index "gps_points", ["tile"], :name => "points_tile_idx"
     remove_index "gps_points", :name => "points_idx"
 

db/migrate/006_tile_nodes.rb

@@ -33,10 +33,12 @@ class TileNodes < ActiveRecord::Migration
   end
 
   def self.up
+    remove_index "current_nodes", :name => "current_nodes_timestamp_idx"
+
     rename_table "current_nodes", "current_nodes_v5"
 
     create_table "current_nodes", innodb_table do |t|
-      t.column "id",        :bigint,   :limit => 64,                 :null => false
+      t.column "id",        :bigint_pk_64,                           :null => false
       t.column "latitude",  :integer,                                :null => false
       t.column "longitude", :integer,                                :null => false
       t.column "user_id",   :bigint,   :limit => 20,                 :null => false
@@ -46,17 +48,17 @@ class TileNodes < ActiveRecord::Migration
       t.column "tile",      :integer,                                :null => false
     end
 
-    add_primary_key "current_nodes", ["id"]
     add_index "current_nodes", ["timestamp"], :name => "current_nodes_timestamp_idx"
     add_index "current_nodes", ["tile"], :name => "current_nodes_tile_idx"
 
-    change_column "current_nodes", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
+    change_column "current_nodes", "tile", :four_byte_unsigned
-    change_column "current_nodes", "tile", :integer, :null => false, :unsigned => true
 
     upgrade_table "current_nodes_v5", "current_nodes", Node
     
     drop_table "current_nodes_v5"
 
+    remove_index "nodes", :name=> "nodes_uid_idx"
+    remove_index "nodes", :name=> "nodes_timestamp_idx"
     rename_table "nodes", "nodes_v5"
 
     create_table "nodes", myisam_table do |t|
@@ -74,7 +76,7 @@ class TileNodes < ActiveRecord::Migration
     add_index "nodes", ["timestamp"], :name => "nodes_timestamp_idx"
     add_index "nodes", ["tile"], :name => "nodes_tile_idx"
 
-    change_column "nodes", "tile", :integer, :null => false, :unsigned => true
+    change_column "nodes", "tile", :four_byte_unsigned
 
     upgrade_table "nodes_v5", "nodes", OldNode
 
@@ -85,7 +87,7 @@ class TileNodes < ActiveRecord::Migration
     rename_table "current_nodes", "current_nodes_v6"
 
     create_table "current_nodes", innodb_table do |t|
-      t.column "id",        :bigint,   :limit => 64,                 :null => false
+      t.column "id",        :bigint_pk_64,                           :null => false
       t.column "latitude",  :double,                                 :null => false
       t.column "longitude", :double,                                 :null => false
       t.column "user_id",   :bigint,   :limit => 20,                 :null => false
@@ -94,12 +96,9 @@ class TileNodes < ActiveRecord::Migration
       t.column "timestamp", :datetime,                               :null => false
     end
 
-    add_primary_key "current_nodes", ["id"]
     add_index "current_nodes", ["latitude", "longitude"], :name => "current_nodes_lat_lon_idx"
     add_index "current_nodes", ["timestamp"], :name => "current_nodes_timestamp_idx"
 
-    change_column "current_nodes", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
-
     downgrade_table "current_nodes_v6", "current_nodes"
 
     drop_table "current_nodes_v6"

db/migrate/007_add_relations.rb

@@ -11,7 +11,7 @@ class AddRelations < ActiveRecord::Migration
       t.column "member_role", :string
     end
     # enums work like strings but are more efficient
-    execute "alter table current_relation_members change column member_type member_type enum('node','way','relation');"
+    alter_column_nwr_enum :current_relation_members, :member_type
 
     add_primary_key "current_relation_members", ["id", "member_type", "member_id", "member_role"]
     add_index "current_relation_members", ["member_type", "member_id"], :name => "current_relation_members_member_idx"
@@ -24,18 +24,15 @@ class AddRelations < ActiveRecord::Migration
     end
 
     add_index "current_relation_tags", ["id"], :name => "current_relation_tags_id_idx"
-    execute "CREATE FULLTEXT INDEX `current_relation_tags_v_idx` ON `current_relation_tags` (`v`)"
+    add_fulltext_index "current_relation_tags", "v"
 
     create_table "current_relations", innodb_table do |t|
-      t.column "id",        :bigint,   :limit => 64, :null => false
+      t.column "id",        :bigint_pk_64,           :null => false
       t.column "user_id",   :bigint,   :limit => 20, :null => false
       t.column "timestamp", :datetime, :null => false
       t.column "visible",   :boolean,  :null => false
     end
 
-    add_primary_key "current_relations", ["id"]
-    change_column "current_relations", "id", :bigint, :limit => 64, :null => false, :options => "AUTO_INCREMENT"
-
     create_table "relation_members", myisam_table do |t|
       t.column "id",          :bigint,  :limit => 64, :default => 0, :null => false
       t.column "member_type", :string, :limit => 11, :null => false
@@ -44,7 +41,7 @@ class AddRelations < ActiveRecord::Migration
       t.column "version",     :bigint,  :limit => 20, :default => 0, :null => false
     end
 
-    execute "alter table relation_members change column member_type member_type enum('node','way','relation');" 
+    alter_column_nwr_enum :relation_members, :member_type 
     add_primary_key "relation_members", ["id", "version", "member_type", "member_id", "member_role"]
     add_index "relation_members", ["member_type", "member_id"], :name => "relation_members_member_idx"
 
@@ -68,7 +65,7 @@ class AddRelations < ActiveRecord::Migration
     add_primary_key "relations", ["id", "version"]
     add_index "relations", ["timestamp"], :name => "relations_timestamp_idx"
     
-    change_column "relations", "version", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
+    change_column "relations", "version", :bigint_auto_20
   end
 
 

db/migrate/010_diary_comments.rb

@@ -1,7 +1,7 @@
 class DiaryComments < ActiveRecord::Migration
   def self.up
     create_table "diary_comments", myisam_table do |t|
-      t.column "id",             :bigint,   :limit => 20, :null => false
+      t.column "id",             :bigint_pk,              :null => false
       t.column "diary_entry_id", :bigint,   :limit => 20, :null => false
       t.column "user_id",        :bigint,   :limit => 20, :null => false
       t.column "body",           :text,                   :null => false
@@ -9,10 +9,8 @@ class DiaryComments < ActiveRecord::Migration
       t.column "updated_at",     :datetime,               :null => false
     end
 
-    add_primary_key "diary_comments", ["id"]
     add_index "diary_comments", ["diary_entry_id", "id"], :name => "diary_comments_entry_id_idx", :unique => true
 
-    change_column "diary_comments", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT"
   end
 
   def self.down

db/migrate/013_add_email_valid.rb

@@ -1,7 +1,7 @@
 class AddEmailValid < ActiveRecord::Migration
   def self.up
     add_column "users", "email_valid", :boolean, :default => false, :null => false
-    User.update_all("email_valid = active")
+    User.update_all("email_valid = (active != 0)") #email_valid is :boolean, but active is :integer. "email_valid = active" (see r11802 or earlier) will fail for stricter dbs than mysql
   end
 
   def self.down

db/migrate/015_add_user_visible.rb

@@ -1,7 +1,7 @@
 class AddUserVisible < ActiveRecord::Migration
   def self.up
     add_column "users", "visible", :boolean, :default => true, :null => false
-    User.update_all("visible = 1")
+    User.update_all(:visible => true)
   end
 
   def self.down

db/migrate/018_create_acls.rb

@@ -1,19 +1,14 @@
 class CreateAcls < ActiveRecord::Migration
   def self.up
     create_table "acls", myisam_table do |t|
-      t.column "id",      :integer, :null => false
+      t.column "id",      :integer_pk, :null => false
-      t.column "address", :integer, :null => false
+      t.column "address", :inet,       :null => false
-      t.column "netmask", :integer, :null => false
+      t.column "netmask", :inet,       :null => false
       t.column "k",       :string,     :null => false
       t.column "v",       :string
     end
 
-    add_primary_key "acls", ["id"]
     add_index "acls", ["k"], :name => "acls_k_idx"
-
-    change_column "acls", "id", :integer, :null => false, :options => "AUTO_INCREMENT"
-    change_column "acls", "address", :integer, :null => false, :unsigned => true
-    change_column "acls", "netmask", :integer, :null => false, :unsigned => true
   end
 
   def self.down

db/migrate/019_add_timestamp_indexes.rb

@@ -0,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

db/migrate/020_populate_node_tags_and_remove.rb

@@ -0,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, "020_populate_node_tags_and_remove.#{$$}."
+
+      cmd = "db/migrate/020_populate_node_tags_and_remove_helper"
+      src = "#{cmd}.c"
+      if not File.exists? cmd or File.mtime(cmd) < File.mtime(src) then 
+        system 'cc -O3 -Wall `mysql_config --cflags --libs` ' +
+          "#{src} -o #{cmd}" or fail
+      end
+
+      conn_opts = ActiveRecord::Base.connection.instance_eval { @connection_options }
+      args = conn_opts.map { |arg| arg.to_s } + [prefix]
+      fail "#{cmd} failed" unless system cmd, *args
+
+      tempfiles = ['nodes', 'node_tags', 'current_nodes', 'current_node_tags'].
+        map { |base| prefix + base }
+      nodes, node_tags, current_nodes, current_node_tags = tempfiles
+    end
+
+    execute "TRUNCATE nodes"
+    remove_column :nodes, :tags
+    remove_column :current_nodes, :tags
+
+    add_column :nodes, :version, :bigint, :limit => 20, :null => false
+
+    create_table :current_node_tags, innodb_table do |t|
+      t.column :id,          :bigint, :limit => 64, :null => false
+      t.column :k,	     :string, :default => "", :null => false
+      t.column :v,	     :string, :default => "", :null => false
+    end
+
+    create_table :node_tags, innodb_table do |t|
+      t.column :id,          :bigint, :limit => 64, :null => false
+      t.column :version,     :bigint, :limit => 20, :null => false
+      t.column :k,	     :string, :default => "", :null => false
+      t.column :v,	     :string, :default => "", :null => false
+    end
+
+    # now get the data back
+    csvopts = "FIELDS TERMINATED BY ',' ENCLOSED BY '\"' ESCAPED BY '\"' LINES TERMINATED BY '\\n'"
+
+    if have_nodes
+      execute "LOAD DATA INFILE '#{nodes}' INTO TABLE nodes #{csvopts} (id, latitude, longitude, user_id, visible, timestamp, tile, version)";
+      execute "LOAD DATA INFILE '#{node_tags}' INTO TABLE node_tags #{csvopts} (id, version, k, v)"
+      execute "LOAD DATA INFILE '#{current_node_tags}' INTO TABLE current_node_tags #{csvopts} (id, k, v)"
+    end
+
+    tempfiles.each { |fn| File.unlink fn } if have_nodes
+  end
+
+  def self.down
+    raise IrreversibleMigration.new
+#    add_column :nodes, "tags", :text, :default => "", :null => false
+#    add_column :current_nodes, "tags", :text, :default => "", :null => false
+  end
+end

db/migrate/020_populate_node_tags_and_remove_helper.c

@@ -0,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);
+}

db/migrate/021_move_to_innodb.rb

@@ -0,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

db/migrate/022_key_constraints.rb

@@ -0,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

db/migrate/023_add_changesets.rb

@@ -0,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