]> git.openstreetmap.org Git - rails.git/commitdiff
Patching better 412 error messages from mis-commit on old api06 branch.
authorMatt Amos <zerebubuth@gmail.com>
Fri, 24 Apr 2009 10:08:15 +0000 (10:08 +0000)
committerMatt Amos <zerebubuth@gmail.com>
Fri, 24 Apr 2009 10:08:15 +0000 (10:08 +0000)
55 files changed:
app/controllers/amf_controller.rb
app/controllers/api_controller.rb
app/controllers/application.rb
app/controllers/changeset_controller.rb
app/controllers/export_controller.rb
app/controllers/old_node_controller.rb
app/controllers/old_relation_controller.rb
app/controllers/old_way_controller.rb
app/controllers/search_controller.rb
app/controllers/swf_controller.rb
app/controllers/trace_controller.rb
app/models/old_node.rb
app/models/old_node_tag.rb
app/models/relation.rb
app/models/way.rb
app/views/browse/_changeset_details.rhtml
app/views/browse/_node_details.rhtml
app/views/browse/_relation_details.rhtml
app/views/browse/_relation_member.rhtml
app/views/browse/_tag_details.rhtml
app/views/browse/_way_details.rhtml
app/views/changeset/_changeset.rhtml
app/views/changeset/list.rhtml
app/views/layouts/site.rhtml
app/views/site/edit.rhtml
app/views/site/index.rhtml
app/views/user/view.rhtml
config/database.yml
config/environment.rb
config/lighttpd.conf
config/mongrel_cluster.yml [new file with mode: 0644]
config/nginx.conf [new file with mode: 0644]
config/routes.rb
db/migrate/026_add_changeset_user_index.rb [new file with mode: 0644]
db/migrate/027_add_changeset_indexes.rb [new file with mode: 0644]
db/migrate/028_add_more_changeset_indexes.rb [new file with mode: 0644]
lib/migrate.rb
public/javascripts/site.js
public/potlatch/potlatch.swf
public/stylesheets/site.css
script/statistics
test/fixtures/changesets.yml
test/fixtures/current_relation_members.yml
test/fixtures/current_relation_tags.yml
test/fixtures/current_relations.yml
test/fixtures/relation_members.yml
test/fixtures/relation_tags.yml
test/fixtures/relations.yml
test/functional/api_controller_test.rb
test/functional/old_node_controller_test.rb
test/functional/relation_controller_test.rb
test/unit/old_relation_tag_test.rb
test/unit/relation_member_test.rb
test/unit/relation_tag_test.rb
test/unit/relation_test.rb

index de3c7583b42d9977f255a581afbc5fe9265121b2..d729fdd456af3e38891bd8922560e830804c9462 100644 (file)
@@ -55,7 +55,7 @@ class AmfController < ApplicationController
 
     # Parse request
 
-    headers=AMF.getint(req)                                    # Read number of headers
+       headers=AMF.getint(req)                                 # Read number of headers
 
     headers.times do                                           # Read each header
       name=AMF.getstring(req)                          #  |
@@ -76,7 +76,8 @@ class AmfController < ApplicationController
         when 'getpresets';                     results[index]=AMF.putdata(index,getpresets())
         when 'whichways';                      results[index]=AMF.putdata(index,whichways(*args))
         when 'whichways_deleted';      results[index]=AMF.putdata(index,whichways_deleted(*args))
-        when 'getway';                         results[index]=AMF.putdata(index,getway(args[0].to_i))
+        when 'getway';                         r=AMF.putdata(index,getway(args[0].to_i))
+                                    results[index]=r
         when 'getrelation';                    results[index]=AMF.putdata(index,getrelation(args[0].to_i))
         when 'getway_old';                     results[index]=AMF.putdata(index,getway_old(args[0].to_i,args[1]))
         when 'getway_history';         results[index]=AMF.putdata(index,getway_history(args[0].to_i))
@@ -276,7 +277,7 @@ class AmfController < ApplicationController
 
     [wayid, points, tags, version]
   end
-
+  
   # Get an old version of a way, and all constituent nodes.
   #
   # For undelete (version<0), always uses the most recent version of each node, 
@@ -451,7 +452,8 @@ class AmfController < ApplicationController
   # Returns
   # 0. 0 (success),
   # 1. original relation id (unchanged),
-  # 2. new relation id.
+  # 2. new relation id,
+  # 3. version.
 
   def putrelation(renumberednodes, renumberedways, usertoken, changeset_id, version, relid, tags, members, visible) #:doc:
     user = getuser(usertoken)
@@ -475,8 +477,8 @@ class AmfController < ApplicationController
       members.each do |m|
         mid = m[1].to_i
         if mid < 0
-          mid = renumberednodes[mid] if m[0] == 'node'
-          mid = renumberedways[mid] if m[0] == 'way'
+          mid = renumberednodes[mid] if m[0] == 'Node'
+          mid = renumberedways[mid] if m[0] == 'Way'
         end
         if mid
           typedmembers << [m[0], mid, m[2]]
@@ -490,7 +492,6 @@ class AmfController < ApplicationController
       new_relation.changeset_id = changeset_id
       new_relation.version = version
 
-      # NOTE: id or relid here? id doesn't seem to be set above
       if relid <= 0
         # We're creating the node
         new_relation.create_with_history(user)
@@ -503,10 +504,10 @@ class AmfController < ApplicationController
       end
     end # transaction
       
-    if id <= 0
+    if relid <= 0
       return [0, relid, new_relation.id, new_relation.version]
     else
-      return [0, relid, relation.id, relation.version]
+      return [0, relid, relid, relation.version]
     end
   rescue OSM::APIChangesetAlreadyClosedError => ex
     return [-1, "The changeset #{ex.changeset.id} was closed at #{ex.changeset.closed_at}."]
@@ -519,7 +520,7 @@ class AmfController < ApplicationController
     return [-1, "The relation has already been deleted."]
   rescue OSM::APIError => ex
     # Some error that we don't specifically catch
-    return [-2, "Something really bad happened :-( ."]
+    return [-2, "An unusual error happened (in 'putrelation' #{relid})."]
   end
 
   # Save a way to the database, including all nodes. Any nodes in the previous
@@ -620,7 +621,7 @@ class AmfController < ApplicationController
       uniques=uniques-pointlist
       uniques.each do |n|
         node = Node.find(n)
-        deleteitemrelations(user, changeset_id, id, 'node', node.version)
+        deleteitemrelations(user, changeset_id, id, 'Node', node.version)
         new_node = Node.new
         new_node.changeset_id = changeset_id
         new_node.version = node.version
@@ -643,7 +644,7 @@ class AmfController < ApplicationController
     return [-1, "The point has already been deleted."]
   rescue OSM::APIError => ex
     # Some error that we don't specifically catch
-    return [-2, "Something really bad happened :-(."]
+    return [-2, "An unusual error happened (in 'putway' #{originalway})."]
   end
 
   # Save POI to the database.
@@ -706,7 +707,7 @@ class AmfController < ApplicationController
     return [-1, "The point has already been deleted"]
   rescue OSM::APIError => ex
     # Some error that we don't specifically catch
-    return [-2, "Something really bad happened :-()"]
+    return [-2, "An unusual error happened (in 'putpoi' #{id})."]
   end
 
   # Read POI from database
@@ -748,12 +749,13 @@ class AmfController < ApplicationController
 
       # delete the way
       old_way = Way.find(way_id)
+      u = old_way.unshared_node_ids
       delete_way = Way.new
       delete_way.version = way_version
       delete_way.changeset_id = changeset_id
       old_way.delete_with_history!(delete_way, user)
 
-      old_way.unshared_node_ids.each do |node_id|
+      u.each do |node_id|
         # delete the node
         node = Node.find(node_id)
         delete_node = Node.new
@@ -763,7 +765,7 @@ class AmfController < ApplicationController
         else
           # in case the node wasn't passed (i.e. if it was previously removed
           # from the way in Potlatch)
-          deleteitemrelations(user, changeset_id, node_id, 'node', node.version)
+          deleteitemrelations(user, changeset_id, node_id, 'Node', node.version)
              delete_node.version = node.version
            end
         node.delete_with_history!(delete_node, user)
@@ -781,7 +783,7 @@ class AmfController < ApplicationController
     return [-1, "The way has already been deleted."]
   rescue OSM::APIError => ex
     # Some error that we don't specifically catch
-    return [-2, "Something really bad happened :-( ."]
+    return [-2, "An unusual error happened (in 'deleteway' #{way_id})."]
   end
 
 
@@ -795,7 +797,7 @@ class AmfController < ApplicationController
 
   def deleteitemrelations(user, changeset_id, objid, type, version) #:doc:
     relations = RelationMember.find(:all, 
-                                                                       :conditions => ['member_type = ? and member_id = ?', type, objid], 
+                                                                       :conditions => ['member_type = ? and member_id = ?', type.classify, objid], 
                                                                        :include => :relation).collect { |rm| rm.relation }.uniq
 
     relations.each do |rel|
@@ -883,7 +885,7 @@ class AmfController < ApplicationController
       SELECT DISTINCT cr.id AS relid,cr.version AS version 
       FROM current_relations cr
       INNER JOIN current_relation_members crm ON crm.id=cr.id 
-      INNER JOIN current_nodes cn ON crm.member_id=cn.id AND crm.member_type='node' 
+      INNER JOIN current_nodes cn ON crm.member_id=cn.id AND crm.member_type='Node' 
        WHERE #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "cn.")}
       EOF
     unless way_ids.empty?
@@ -892,17 +894,17 @@ class AmfController < ApplicationController
         SELECT DISTINCT cr.id AS relid,cr.version AS version
         FROM current_relations cr
         INNER JOIN current_relation_members crm ON crm.id=cr.id
-         WHERE crm.member_type='way' 
+         WHERE crm.member_type='Way' 
          AND crm.member_id IN (#{way_ids.join(',')})
         EOF
     end
-    return ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['relid'].to_i,a['version'].to_i] }
+    ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['relid'].to_i,a['version'].to_i] }
   end
        
   def sql_get_nodes_in_way(wayid)
     points=[]
     sql=<<-EOF
-      SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,current_nodes.id 
+      SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,current_nodes.id,current_nodes.version 
       FROM current_way_nodes,current_nodes 
        WHERE current_way_nodes.id=#{wayid.to_i} 
                   AND current_way_nodes.node_id=current_nodes.id 
@@ -915,7 +917,7 @@ class AmfController < ApplicationController
         nodetags[n['k']]=n['v']
       end
       nodetags.delete('created_by')
-      points << [row['lon'].to_f,row['lat'].to_f,row['id'].to_i,nodetags]
+      points << [row['lon'].to_f,row['lat'].to_f,row['id'].to_i,nodetags,row['version'].to_i]
     end
     points
   end
index ebf729afc322812002231adeeefe6e486a756367..ca4cbcbd7c3439304a6ed38179c25c6fb2ad6bfa 100644 (file)
@@ -284,6 +284,9 @@ class ApiController < ApplicationController
     waynodes = XML::Node.new 'waynodes'
     waynodes['maximum'] = APP_CONFIG['max_number_of_way_nodes'].to_s
     api << waynodes
+    changesets = XML::Node.new 'changesets'
+    changesets['maximum_elements'] = Changeset::MAX_ELEMENTS.to_s
+    api << changesets
     
     doc.root << api
 
index bfd2e9c546e852dcabad7f11ee2af4694a3df373..3d8a028104e93e9540cf66d868be561bbda788cf 100644 (file)
@@ -96,9 +96,9 @@ class ApplicationController < ActionController::Base
   #  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 :text => message, :status => :bad_request
     # Todo: some sort of escaping of problem characters in the message
     response.headers['Error'] = message
+    render :text => message, :status => :bad_request
   end
 
 private 
index 4913a600e1d036a5c0e41dcdbd35b97764acd32d..243609290e9fc0e8b5379b7ce2a9b9979ac1c926 100644 (file)
@@ -356,6 +356,10 @@ class ChangesetController < ApplicationController
     else
       #TODO: fix bugs in location determination for history tab (and other tabs) then uncomment this redirect
       #redirect_to :action => 'list'
+      
+      # For now just render immediately, and skip the db
+      render
+      return
     end
        
     conditions = conditions_bbox(bbox);
index 754cc4b8216b15acd900d3646cdb399c31d6faba..ab25fcbc67a2ed407955953287d3550c99c46812 100644 (file)
@@ -10,14 +10,14 @@ class ExportController < ApplicationController
     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]
index 0976a0c9aae87c679bd254b028b45830f9a47077..2590fd24aefbac64035b0e31e7c04dfa76de1612 100644 (file)
@@ -26,6 +26,11 @@ class OldNodeController < ApplicationController
   def version
     begin
       old_node = OldNode.find(:first, :conditions => {:id => params[:id], :version => params[:version]} )
+      if old_node.nil?
+        # (RecordNotFound is not raised with find :first...)
+        render :nothing => true, :status => :not_found
+        return
+      end
       
       response.headers['Last-Modified'] = old_node.timestamp.rfc822
 
@@ -33,8 +38,6 @@ class OldNodeController < ApplicationController
       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
index f8ebbd070be3609901aeed5c7cc0cf60dc441763..65de83278c7ff4b67181eefa473baedc9ef993b0 100644 (file)
@@ -25,6 +25,11 @@ class OldRelationController < ApplicationController
   def version
     begin
       old_relation = OldRelation.find(:first, :conditions => {:id => params[:id], :version => params[:version]} )
+      if old_relation.nil?
+        # (RecordNotFound is not raised with find :first...)
+        render :nothing => true, :status => :not_found
+        return
+      end
       
       response.headers['Last-Modified'] = old_relation.timestamp.rfc822
 
@@ -32,10 +37,8 @@ class OldRelationController < ApplicationController
       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
+      render :nothing => true, :status => :internal_server_error
     end
   end
 end
index a42496687311d255c3d9c391634f9b6fe55cf728..2f110321607e6d66225055a22d1a4ad05c1801b2 100644 (file)
@@ -26,6 +26,11 @@ class OldWayController < ApplicationController
   def version
     begin
       old_way = OldWay.find(:first, :conditions => {:id => params[:id], :version => params[:version]} )
+      if old_way.nil?
+        # (RecordNotFound is not raised with find :first...)
+        render :nothing => true, :status => :not_found
+        return
+      end
       
       response.headers['Last-Modified'] = old_way.timestamp.rfc822
       
@@ -33,8 +38,6 @@ class OldWayController < ApplicationController
       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
index 6b83e33016d3c9dc77afd7d1be901f6aed44d3d5..17326db07e70f79c66fc64733057b4dcc47f1f9e 100644 (file)
@@ -46,7 +46,20 @@ class SearchController < ApplicationController
     nodes = Array.new
     relations = Array.new
 
-    # Matching for tags table
+    # Matching for node tags table
+    cond_node = Array.new
+    sql = '1=1'
+    if type
+      sql += ' AND current_node_tags.k=?'
+      cond_node += [type]
+    end
+    if value
+      sql += ' AND current_node_tags.v=?'
+      cond_node += [value]
+    end
+    cond_node = [sql] + cond_node
+
+    # Matching for way tags table
     cond_way = Array.new
     sql = '1=1'
     if type
@@ -54,12 +67,12 @@ class SearchController < ApplicationController
       cond_way += [type]
     end
     if value
-      sql += ' AND current_way_tags.v=? AND MATCH (current_way_tags.v) AGAINST (? IN BOOLEAN MODE)'
-      cond_way += [value,'"' + value.sub(/[-+*<>"~()]/, ' ') + '"']
+      sql += ' AND current_way_tags.v=?'
+      cond_way += [value]
     end
     cond_way = [sql] + cond_way
 
-    # Matching for tags table
+    # Matching for relation tags table
     cond_rel = Array.new
     sql = '1=1'
     if type
@@ -67,30 +80,11 @@ class SearchController < ApplicationController
       cond_rel += [type]
     end
     if value
-      sql += ' AND current_relation_tags.v=? AND MATCH (current_relation_tags.v) AGAINST (? IN BOOLEAN MODE)'
-      cond_rel += [value,'"' + value.sub(/[-+*<>"~()]/, ' ') + '"']
+      sql += ' AND current_relation_tags.v=?'
+      cond_rel += [value]
     end
     cond_rel = [sql] + cond_rel
 
-    # Matching for tags column
-    if type and value
-      cond_tags = ['tags LIKE ? OR tags LIKE ? OR tags LIKE ? OR tags LIKE ?', 
-      ''+type+'='+value+'',
-      ''+type+'='+value+';%',
-      '%;'+type+'='+value+';%',
-      '%;'+type+'='+value+'' ]
-    elsif type
-      cond_tags = ['tags LIKE ? OR tags LIKE ?',
-      ''+type+'=%',
-      '%;'+type+'=%' ]
-    elsif value
-      cond_tags = ['tags LIKE ? OR tags LIKE ?',
-      '%='+value+';%',
-      '%='+value+'' ]
-    else
-      cond_tags = ['1=1']
-    end
-
     # First up, look for the relations we want
     if do_relations
       relations = Relation.find(:all,
@@ -107,7 +101,9 @@ class SearchController < ApplicationController
 
     # Now, nodes
     if do_nodes
-      nodes = Node.find(:all, :conditions => cond_tags, :limit => 2000)
+      nodes = Node.find(:all,
+                        :joins => "INNER JOIN current_node_tags ON current_node_tags.id = current_nodes.id",
+                        :conditions => cond_node, :limit => 2000)
     end
 
     # Fetch any node needed for our ways (only have matching nodes so far)
index 182fb8c549f934291c87f457dd4b069ae24e092a..9688af3ef169ac57ddfc35428904f4bd98ccb701 100644 (file)
@@ -33,7 +33,7 @@ class SwfController < ApplicationController
                bounds_top   =240*20
 
                m =''
-               m+=swfRecord(9,255.chr + 155.chr + 155.chr)                     #ÊBackground
+               m+=swfRecord(9,255.chr + 155.chr + 155.chr)                     # Background
                absx=0
                absy=0
                xl=yb= 9999999
@@ -47,7 +47,7 @@ class SwfController < ApplicationController
        
                if params['token']
                  user=User.authenticate(:token => params[:token])
-                 sql="SELECT gps_points.latitude*0.0000001 AS lat,gps_points.longitude*0.0000001 AS lon,gpx_files.id AS fileid,UNIX_TIMESTAMP(gps_points.timestamp) AS ts "+
+                 sql="SELECT gps_points.latitude*0.0000001 AS lat,gps_points.longitude*0.0000001 AS lon,gpx_files.id AS fileid,EXTRACT(EPOCH FROM gps_points.timestamp) AS ts "+
                           " FROM gpx_files,gps_points "+
                           "WHERE gpx_files.id=gpx_id "+
                           "  AND gpx_files.user_id=#{user.id} "+
@@ -56,7 +56,7 @@ class SwfController < ApplicationController
                           "ORDER BY fileid DESC,ts "+
                           "LIMIT 10000"
                  else
-                       sql="SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,gpx_id AS fileid,UNIX_TIMESTAMP(timestamp) AS ts "+
+                       sql="SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,gpx_id AS fileid,EXTRACT(EPOCH FROM timestamp) AS ts "+
                                 " FROM gps_points "+
                                 "WHERE "+OSM.sql_for_area(ymin,xmin,ymax,xmax,"gps_points.")+
                                 "  AND (gps_points.timestamp IS NOT NULL) "+
index dc7456c45099ec923731ccd43944be57001bd459..e6c732d2d3b760c29fbb48e4e685a32db37a56f0 100644 (file)
@@ -57,7 +57,12 @@ class TraceController < ApplicationController
       @tag = params[:tag]
 
       files = Tracetag.find_all_by_tag(params[:tag]).collect { |tt| tt.gpx_id }
-      conditions[0] += " AND gpx_files.id IN (#{files.join(',')})"
+
+      if files.length > 0
+        conditions[0] += " AND gpx_files.id IN (#{files.join(',')})"
+      else
+        conditions[0] += " AND 0 = 1"
+      end
     end
     
     conditions[0] += " AND gpx_files.visible = ?"
@@ -233,6 +238,7 @@ class TraceController < ApplicationController
 
     if trace.inserted?
       if trace.public? or (@user and @user == trace.user)
+        expires_in 7.days, :private => !trace.public, :public => trace.public
         send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline')
       else
         render :nothing => true, :status => :forbidden
@@ -249,6 +255,7 @@ class TraceController < ApplicationController
 
     if trace.inserted?
       if trace.public? or (@user and @user == trace.user)
+        expires_in 7.days, :private => !trace.public, :public => trace.public
         send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline')
       else
         render :nothing => true, :status => :forbidden
index be115c53eaaaa026d38559ad63c5316326872968..5e3638347a642a55ec8e33ca180012532709233b 100644 (file)
@@ -4,6 +4,10 @@ class OldNode < ActiveRecord::Base
 
   set_table_name 'nodes'
   
+  # Should probably have the composite primary key set in the model
+  # however there are some weird bugs happening when you do
+  #set_primary_keys :id, :version
+  
   validates_presence_of :changeset_id, :timestamp
   validates_inclusion_of :visible, :in => [ true, false ]
   validates_numericality_of :latitude, :longitude
index 3fd4bf86b4fb71504e5c92f58fa6da570f8f0c07..dd339ad307b6400d1fed177cea4020c89883a4b2 100644 (file)
@@ -1,6 +1,6 @@
 class OldNodeTag < ActiveRecord::Base
   set_table_name 'node_tags'
-  
+
   belongs_to :user
 
   validates_presence_of :id, :version
index 2032e677046684067287dddb14e0f7eddecf528c..8a211b84f0498888108754c4b2e1bc70032a906c 100644 (file)
@@ -249,7 +249,7 @@ class Relation < ActiveRecord::Base
 
   def update_from(new_relation, user)
     check_consistency(self, new_relation, user)
-    unless preconditions_ok?
+    unless new_relation.preconditions_ok?
       raise OSM::APIPreconditionFailedError.new("Cannot update relation #{self.id}: data or member data is invalid.")
     end
     self.changeset_id = new_relation.changeset_id
@@ -338,7 +338,7 @@ class Relation < ActiveRecord::Base
       self.timestamp = t
       self.save!
 
-      tags = self.tags
+      tags = self.tags.clone
       self.relation_tags.each do |old_tag|
         key = old_tag.k
         # if we can match the tags we currently have to the list
@@ -346,11 +346,7 @@ class Relation < ActiveRecord::Base
         # 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!
+          tags_changed |= (old_tag.v != tags[key])
 
           # remove from the map, so that we can expect an empty map
           # at the end if there are no new tags
@@ -359,13 +355,14 @@ class Relation < ActiveRecord::Base
         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|
+      RelationTag.delete_all(:id => self.id)
+      self.tags.each do |k,v|
+        logger.info "TAG added: #{k} -> #{v}"
         tag = RelationTag.new
         tag.k = k
         tag.v = v
@@ -373,10 +370,6 @@ class Relation < ActiveRecord::Base
         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.
index cbbe6ada41f540eca762b22786e6e6aa45da362f..97f44c1171337375bb72166153f86be7f3837fff 100644 (file)
@@ -224,7 +224,7 @@ class Way < ActiveRecord::Base
   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'])
+      raise OSM::APITooManyWayNodesError.new(self.nds.length, APP_CONFIG['max_number_of_way_nodes'])
     end
     self.nds.each do |n|
       node = Node.find(:first, :conditions => ["id = ?", n])
index 12d4dfb25b84e616d8d0c9423bbdd0c9a92fba88..13b64d3405f2cff622ee8e7ef45f950bab2a9749 100644 (file)
@@ -40,7 +40,7 @@
             <td><%=maxlon -%></td>
           </tr>
           <tr>
-            <td colspan="3" style="text-align:center"><%= minlon -%></td>
+            <td colspan="3" style="text-align:center"><%= minlat -%></td>
           </tr>
         </table>
       </td>
@@ -51,7 +51,7 @@
     <tr valign="top">
       <th>Has the following <%= @node_pages.item_count %> nodes:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="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 %>
@@ -65,7 +65,7 @@
     <tr valign="top">
       <th>Has the following <%= @way_pages.item_count %> ways:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="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 %>
@@ -82,7 +82,7 @@
     <tr valign="top">
       <th>Has the following <%= @relation_pages.item_count %> relations:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="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 %>
index e3d11e0fee126583a0302618dcec81112a1565cd..1aa5bf69cdf57eeb3cd51918e1ba2d72213c59fb 100644 (file)
@@ -6,7 +6,7 @@
     <tr valign="top">
       <th>Part of:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="0">
           <% node_details.ways.each do |way| %>
             <tr><td><%= link_to "Way " + way.id.to_s, :action => "way", :id => way.id.to_s %></td></tr>
           <% end %>
index b8874d9ce4703b4628e625221e819cdf350f3ca2..e2249e92fb753ffbd053d5d97ac45e654f798f4e 100644 (file)
@@ -6,7 +6,7 @@
     <tr valign="top">
       <th>Members:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="0">
           <%= render :partial => "relation_member", :collection => relation_details.relation_members %>
         </table>
       </td>
@@ -17,7 +17,7 @@
     <tr>
       <th>Part of:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="0">
           <%= render :partial => "containing_relation", :collection => relation_details.containing_relation_members %>
         </table>
       </td>
index 516d9e37ff69cd45d5d66beb56742d95c5d6b6cc..fe484c3cd8820255304ecbdcfa6b3b1812a7078a 100644 (file)
@@ -1,7 +1,7 @@
 <tr>
   <td>
-    <%= h(relation_member.member_type.capitalize) %>
-    <%= link_to relation_member.member_id.to_s, :action => relation_member.member_type, :id => relation_member.member_id %>
+    <%= relation_member.member_type.capitalize %>
+    <%= link_to relation_member.member_id.to_s, :action => relation_member.member_type.downcase, :id => relation_member.member_id %>
     <% unless relation_member.member_role.blank? %>
       as
       <%= h(relation_member.member_role) %>
index dab36266bbb31218ee3eac53913f9569582fa1d4..6f82689c2cfb94311bcfc3cbe55e268df3e36413 100644 (file)
@@ -2,7 +2,7 @@
   <tr valign="top">
     <th>Tags:</th>
     <td>
-      <table padding="0">
+      <table cellpadding="0">
         <%= render :partial => "tag", :collection => tag_details.tags_as_hash %>
       </table>
     </td>
index bc00100d888dbef43cf74acbfca9cbdf2d217d84..9dc35ad53aabe12a3334eb444ce2bbecb17d81c9 100644 (file)
@@ -5,7 +5,7 @@
   <tr valign="top">
     <th>Nodes:</th>
     <td>
-      <table padding="0">
+      <table cellpadding="0">
         <% way_details.way_nodes.each do |wn| %>
           <tr><td><%= link_to "Node " + wn.node_id.to_s, :action => "node", :id => wn.node_id.to_s %></td></tr>
         <% end %>
@@ -17,7 +17,7 @@
     <tr valign="top">
       <th>Part of:</th>
       <td>
-        <table padding="0">
+        <table cellpadding="0">
           <%= render :partial => "containing_relation", :collection => way_details.containing_relation_members %>
         </table>
       </td>
index a149a72e90b8db6dba796a87b17aa006bf421e4e..4012c41a3aa209e3afeca476da6f7749363a3b9e 100644 (file)
@@ -14,7 +14,7 @@
   <%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 %>
+      <%= link_to h(changeset.user.display_name), :controller => "changeset", :action => "list_user", :display_name => changeset.user.display_name %>
     <% else %>
       <i>Anonymous</i>
     <% end %>
index 4ac5082e1ca434e327ca74aefbd0b90c4f834414..aa745743a1fc53b5ef1dc2ae55ebc9a055860b22 100644 (file)
@@ -1,5 +1,5 @@
 <h1>Recent Changes</h1>
-<p>Recently closed changesets:</p>
+<p>Recently edited changesets:</p>
 
 <table id="keyvalue" cellpadding="3">
   <tr>
index 534003031a4d9903fc522333b32d6cf2d505632b..93abf3a5efac6ffd870e74a415f3aaaa8ea36d58 100644 (file)
@@ -50,7 +50,7 @@
         %>
         <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>
+        <li><%= link_to 'History', {:controller => 'changeset', :action => 'list_bbox' }, {: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 %>
index e341305f591cad809733c2ca1e14e21f4ceb257c..a3a2b532a81858d3a45c93fdabcf65c48e92ea91 100644 (file)
@@ -75,6 +75,8 @@ zoom='14' if zoom.nil?
        }
   }
 
+  function markChanged(a) { changesaved=a; }
+
   function doSWF(lat,lon,sc) {
     if (sc < 11) sc = 11;
     fo.addVariable('scale',sc);
index 54f28172739f2521a33a6dfd4bddc098eb3b4230..b8fcf579ee2c6071d78257eb204210052ad97456 100644 (file)
@@ -171,8 +171,8 @@ end
     var layers = getMapLayers();
     var extents = getMapExtent();
 
-    updatelinks(lonlat.lon, lonlat.lat, zoom, layers, extents);
-
+    updatelinks(lonlat.lon, lonlat.lat, zoom, layers, extents.left, extents.bottom, extents.right, extents.top);
+      
     document.cookie = "_osm_location=" + lonlat.lon + "|" + lonlat.lat + "|" + zoom + "|" + layers;
   }
 
index a62be6a55d94ad9a9a35a0b861dc78f6cae71b13..c76ca23ae620055bdbe1db9aca8569bfaf371468 100644 (file)
 <br/>
 <% if @user and @this_user.id == @user.id %>
 <%= link_to 'change your settings', :controller => 'user', :action => 'account', :display_name => @user.display_name %>
-<% end %>
\ No newline at end of file
+<% end %>
index cc3f9a1a5fb26e4fd5b2186728c3e4d5c6b45127..c6fe06fdd937ddb1af23b5097a5620fd0cbeb8c2 100644 (file)
 # And be sure to use new-style password hashing:
 #   http://dev.mysql.com/doc/refman/5.0/en/old-client.html
 development:
-  adapter: mysql
+  adapter: postgresql
   database: openstreetmap
-  username: openstreetmap
-  password: openstreetmap
+#  username: openstreetmap
+#  password: openstreetmap
   host: localhost
   encoding: utf8
 
@@ -22,17 +22,17 @@ development:
 # re-generated from your development database when you run 'rake'.
 # Do not set this db to the same as development or production.
 test:
-  adapter: mysql
+  adapter: postgresql
   database: osm_test
-  username: osm_test
-  password: osm_test
+#  username: osm_test
+#  password: osm_test
   host: localhost
   encoding: utf8
 
 production:
-  adapter: mysql
+  adapter: postgresql
   database: osm
-  username: osm
-  password: osm
+#  username: osm
+#  password: osm
   host: localhost
   encoding: utf8
index 7f83fd4ad7f87670a6b4e17a770bcce4345796e0..c303c353531b4cf15347925747e1a051a27a79d8 100644 (file)
@@ -49,7 +49,6 @@ Rails::Initializer.run do |config|
   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.
index 419f25e3266fb26cb78a5ddbca31de83c1a26fb7..22051eb44657c667356d8f50b5e93a4b319407a3 100644 (file)
@@ -20,6 +20,8 @@ server.username = "www-data"
 server.groupname = "www-data"
 server.pid-file = "/var/run/lighttpd.pid"
 
+server.max-fds = 8192
+
 #
 # Setup logging
 #
@@ -124,13 +126,13 @@ server.document-root = "/home/rails/public"
 $HTTP["useragent"] == "tilesAtHome" {
   server.error-handler-404 = "/dispatch.tah"
 }
-else $HTTP["url"] =~ "^/api/0\.5/(map|trackpoints|amf|amf/read|swf/trackpoints)$" {
+else $HTTP["url"] =~ "^/api/0\.6/(map|trackpoints|amf|amf/read|swf/trackpoints)$" {
   server.error-handler-404 = "/dispatch.bulkapi"
 }
-else $HTTP["url"] =~ "^/api/0\.5/.*/search$" {
+else $HTTP["url"] =~ "^/api/0\.6/.*/search$" {
   server.error-handler-404 = "/dispatch.bulkapi"
 }
-else $HTTP["url"] =~ "^/api/0\.5/" {
+else $HTTP["url"] =~ "^/api/0\.6/" {
   server.error-handler-404 = "/dispatch.api"
 }
 else $HTTP["url"] =~ "^/api/0\.[0-9]+/" {
diff --git a/config/mongrel_cluster.yml b/config/mongrel_cluster.yml
new file mode 100644 (file)
index 0000000..954be24
--- /dev/null
@@ -0,0 +1,5 @@
+---\r
+log_file: log/mongrel.log\r
+port: 8000\r
+pid_file: tmp/mongrel.pid\r
+servers: 8
\ No newline at end of file
diff --git a/config/nginx.conf b/config/nginx.conf
new file mode 100644 (file)
index 0000000..ae349b7
--- /dev/null
@@ -0,0 +1,266 @@
+# Run as www-data
+user www-data www-data;
+
+# Use two worker processes
+worker_processes  2;
+
+# Define PID files
+pid /var/run/nginx.pid;
+
+# Define error log
+error_log /var/log/nginx/error.log;
+
+events {
+    # max clients = worker_processes * worker_connections
+    worker_connections  1024;
+}
+
+http {
+    # Configure MIME types 
+    include /etc/nginx/mime.types;
+    default_type  application/octet-stream;
+
+    # Configure network details
+    sendfile on;
+    keepalive_timeout 65;
+    tcp_nodelay on;
+
+    # Define access log
+    access_log /var/log/nginx/access.log;
+
+    # Configure compression (text/html is compressed by default)
+    gzip on;
+    gzip_min_length 1100;
+    gzip_buffers 4 8k;
+    gzip_types text/plain application/x-javascript application/x-shockwave-flash text/css;
+    
+    #NO CGI SUPPORT IN NGINX fix stat .pl later
+
+    # Define fastcgi backend for web pages
+    upstream web_backend {
+        server 127.0.0.1:8000;
+        server 127.0.0.1:8001;
+        server 127.0.0.1:8002;
+        server 127.0.0.1:8003;
+        server 127.0.0.1:8004;
+        server 127.0.0.1:8005;
+        server 127.0.0.1:8006;
+        server 127.0.0.1:8007;
+        server 127.0.0.1:8008;
+        server 127.0.0.1:8009;
+        server 127.0.0.1:8010;
+        server 127.0.0.1:8011;
+        server 127.0.0.1:8012;
+        server 127.0.0.1:8013;
+        server 127.0.0.1:8014;
+        server 127.0.0.1:8015;
+        server 127.0.0.1:8016;
+        server 127.0.0.1:8017;
+        server 127.0.0.1:8018;
+        server 127.0.0.1:8019;
+        server 127.0.0.1:8020;
+        server 127.0.0.1:8021;
+        server 127.0.0.1:8022;
+        server 127.0.0.1:8023;
+        server 127.0.0.1:8024;
+        server 127.0.0.1:8025;
+    }
+
+    # Define fastcgi backend for geocoder searches
+    upstream geocoder_backend {
+        server 127.0.0.1:8026;
+        server 127.0.0.1:8027;
+        server 127.0.0.1:8028;
+        server 127.0.0.1:8029;
+    }
+
+    # Define fastcgi backend for api requests
+    upstream api_backend {
+        server 127.0.0.1:8030;
+        server 127.0.0.1:8031;
+        server 127.0.0.1:8032;
+        server 127.0.0.1:8033;
+        server 127.0.0.1:8034;
+        server 127.0.0.1:8035;
+        server 127.0.0.1:8036;
+        server 127.0.0.1:8037;
+        server 127.0.0.1:8038;
+        server 127.0.0.1:8039;
+        server 127.0.0.1:8040;
+        server 127.0.0.1:8041;
+        server 127.0.0.1:8042;
+        server 127.0.0.1:8043;
+        server 127.0.0.1:8044;
+    }
+
+    # Define fastcgi backend for bulk api requests
+    upstream bulkapi_backend {
+        server 10.0.0.10:8000;
+        server 10.0.0.11:8000;
+        server 10.0.0.12:8000;
+        server 10.0.0.10:8001;
+        server 10.0.0.11:8001;
+        server 10.0.0.12:8001;
+        server 10.0.0.10:8002;
+        server 10.0.0.11:8002;
+        server 10.0.0.12:8002;
+        server 10.0.0.10:8003;
+        server 10.0.0.11:8003;
+        server 10.0.0.12:8003;
+        server 10.0.0.10:8004;
+        server 10.0.0.11:8004;
+        server 10.0.0.12:8004;
+    }
+    
+    # Define fastcgi backend for tiles@home requests
+    upstream tah_backend {
+        server 10.0.0.10:8005;
+        server 10.0.0.11:8005;
+        server 10.0.0.12:8005;
+    }
+
+    server {
+        # Listen on port 80
+        listen 80;
+
+        # Serve rails public files
+        root /home/rails/public;
+
+        # Use index.html as the index page
+        index index.html;
+
+        # Redirect trac requests for historical reasons
+        location /trac/ {
+            rewrite ^/trac/(.*)$ http://trac.openstreetmap.org/$1 permanent;
+        }
+
+        # Redirect wiki requests for historical reasons
+        location /wiki/ {
+            rewrite ^/wiki/(.*)$ http://wiki.openstreetmap.org/$1 permanent;
+        }
+
+        # Placeholder for blocking abuse
+        include /etc/nginx/blocked_hosts;
+        allow all; 
+
+        # Block some bulk download agents
+        if ($http_user_agent ~* LWP::Simple|downloadosm|BBBike) {
+            return 403;
+        }
+
+        # Block some robots
+        if ($http_user_agent ~* msnbot|twiceler) {
+            return 403;
+        }
+        # Map api.openstreetmap/0.n/... to api.openstreetmap/api/0.n/...
+        if ($host ~* ^api\.) {
+            rewrite ^/(0\.[0-9]+)/(.*)$ /api/$1/$2;
+            rewrite ^/capabilities$ /api/capabilities;
+        }
+        # Strip asset tags
+        location ~ ^/(images|javascripts|openlayers|stylesheets|user/image)/ {
+            # Strip asset tags
+            rewrite ^/(.*)/[0-9]+$ /$1;
+
+            # Set expiry to the maximum - the asset tag will change
+            # when there is a new version
+            expires max;
+
+            # Only cache OpenLayers for seven days though
+            if ($uri ~ ^/openlayers/) {
+                expires 7d;
+            }
+        }
+
+        # Cache the embedded map page for seven days
+        location ~ ^/export/embed.html$ {
+            expires 7d;
+        }
+
+        # Include fastcgi configuration
+        include /etc/nginx/fastcgi_params;
+        fastcgi_param REQUEST_URI $uri;
+
+        # Handle tiles@home requests
+        location /api/ {
+            if ($http_user_agent ~ "^tilesAtHome") {
+                #deny all;
+                fastcgi_pass tah_backend;
+                break;
+            }
+        }
+
+        # Handle bulk api requests
+        location ~ ^/api/0\.6/(map|relation|trackpoints|amf|amf/read|swf/trackpoints|trace/[0-9]+/data)$ {
+            fastcgi_read_timeout 300;
+            fastcgi_pass bulkapi_backend;
+            break;
+        }
+        
+        # Send search requests to the bulk api backend
+        location ~ ^/api/0\.6/.*/search$ {
+            fastcgi_read_timeout 300;
+            fastcgi_pass bulkapi_backend;
+            break;
+        }
+
+        # Send requests for full objects to the bulk api backend
+        location ~ ^/api/0\.6/.*/full$ {
+            fastcgi_read_timeout 300;
+            fastcgi_pass bulkapi_backend;
+            break;
+        }
+
+        # Handle the remaining api requests
+        location ~ ^/api/0\.6/ {
+            fastcgi_pass api_backend;
+            break;
+        }
+
+        # Deny old and unknown API versions
+        location ~ ^/api/0\.[0-9]+/ {
+            return 404;
+        }
+            
+        # Send unversioned capabilities requests to the api backend
+        location = /api/capabilities {
+            fastcgi_pass api_backend;
+            break;
+        }
+
+        # Send geocoder searches to the geocoder backend
+        location /geocoder/ { 
+            fastcgi_pass geocoder_backend;
+            break;
+        }
+
+        # Send everything else to the web backend unless it exists
+        # in the rails public tree    
+        location / {
+            fastcgi_index index.html;
+
+            if (!-f $request_filename) {
+                fastcgi_pass web_backend;
+                break;
+            }
+        }
+
+        # Set the MIME type for crossdomain.xml policy files
+        # or flash will ignore it
+        location ~ /crossdomain\.xml$ {
+            types {
+                text/x-cross-domain-policy xml;
+            }
+        }
+
+        # Give munin access to some statistics
+        location /server-status {
+            stub_status on;
+            access_log off;
+            allow 127.0.0.1;
+            deny all;
+        }
+    }
+}
index fcf22bced98d12cd21a18101bc8b5073174917d6..f2d64471900dc3142beaa378240dce829ca8d4b5 100644 (file)
@@ -2,7 +2,8 @@ ActionController::Routing::Routes.draw do |map|
 
   # API
   map.connect "api/capabilities", :controller => 'api', :action => 'capabilities'
-  
+  map.connect "api/#{API_VERSION}/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+/
@@ -36,6 +37,7 @@ ActionController::Routing::Routes.draw do |map|
   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+/
   map.connect "api/#{API_VERSION}/relation/:id/full", :controller => 'relation', :action => 'full', :id => /\d+/
+  map.connect "api/#{API_VERSION}/relation/:id/:version", :controller => 'old_relation', :action => 'version', :id => /\d+/, :version => /\d+/
   map.connect "api/#{API_VERSION}/relation/:id", :controller => 'relation', :action => 'read', :id => /\d+/, :conditions => { :method => :get }
   map.connect "api/#{API_VERSION}/relation/:id", :controller => 'relation', :action => 'update', :id => /\d+/, :conditions => { :method => :put }
   map.connect "api/#{API_VERSION}/relation/:id", :controller => 'relation', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete }
diff --git a/db/migrate/026_add_changeset_user_index.rb b/db/migrate/026_add_changeset_user_index.rb
new file mode 100644 (file)
index 0000000..fed0af7
--- /dev/null
@@ -0,0 +1,9 @@
+class AddChangesetUserIndex < ActiveRecord::Migration
+  def self.up
+    add_index "changesets", ["user_id"], :name => "changesets_user_id_idx"
+  end
+
+  def self.down
+    remove_index "changesets", :name => "changesets_user_id_idx"
+  end
+end
diff --git a/db/migrate/027_add_changeset_indexes.rb b/db/migrate/027_add_changeset_indexes.rb
new file mode 100644 (file)
index 0000000..0d4bba0
--- /dev/null
@@ -0,0 +1,13 @@
+class AddChangesetIndexes < ActiveRecord::Migration
+  def self.up
+    add_index "nodes", ["changeset_id"], :name => "nodes_changeset_id_idx"
+    add_index "ways", ["changeset_id"], :name => "ways_changeset_id_idx"
+    add_index "relations", ["changeset_id"], :name => "relations_changeset_id_idx"
+  end
+
+  def self.down
+    remove_index "relations", :name => "relations_changeset_id_idx"
+    remove_index "ways", :name => "ways_changeset_id_idx"
+    remove_index "nodes", :name => "nodes_changeset_id_idx"
+  end
+end
diff --git a/db/migrate/028_add_more_changeset_indexes.rb b/db/migrate/028_add_more_changeset_indexes.rb
new file mode 100644 (file)
index 0000000..d93cc79
--- /dev/null
@@ -0,0 +1,13 @@
+class AddMoreChangesetIndexes < ActiveRecord::Migration
+  def self.up
+    add_index "changesets", ["created_at"], :name => "changesets_created_at_idx"
+    add_index "changesets", ["closed_at"], :name => "changesets_closed_at_idx"
+    add_index "changesets", ["min_lat","max_lat","min_lon","max_lon"], :name => "changesets_bbox_idx", :method => "GIST"
+  end
+
+  def self.down
+    remove_index "changesets", :name => "changesets_bbox_idx"
+    remove_index "changesets", :name => "changesets_closed_at_idx"
+    remove_index "changesets", :name => "changesets_created_at_idx"
+  end
+end
index 392060b6015a8cdb34d5fcfa6172e8a215b05eb4..05b3c90f24da86a9cf541ec9cf60c7e90c9c7520 100644 (file)
@@ -158,6 +158,21 @@ module ActiveRecord
       def interval_constant(interval)
         "'#{interval}'::interval"
       end
+
+      def add_index(table_name, column_name, options = {})
+        column_names = Array(column_name)
+        index_name   = index_name(table_name, :column => column_names)
+
+        if Hash === options # legacy support, since this param was a string
+          index_type = options[:unique] ? "UNIQUE" : ""
+          index_name = options[:name] || index_name
+          index_method = options[:method] || "BTREE"
+        else
+          index_type = options
+        end
+        quoted_column_names = column_names.map { |e| quote_column_name(e) }.join(", ")
+        execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} USING #{index_method} (#{quoted_column_names})"
+      end
     end
   end
 end
index 06b4152df9a1267cef030762145d5c2946e117c1..ae38ecb6a53914239c903a252ba9aa4d83c13d57 100644 (file)
@@ -1,6 +1,6 @@
 //Called as the user scrolls/zooms around.
 //Maniplate hrefs of the view tab and various other links
-function updatelinks(lon,lat,zoom,layers,extents) {
+function updatelinks(lon,lat,zoom,layers,minlon,minlat,maxlon,maxlat) {
   var decimals = Math.pow(10, Math.floor(zoom/3));
   var node;
 
@@ -63,16 +63,19 @@ function updatelinks(lon,lat,zoom,layers,extents) {
     if (zoom >= 11) {
       var args = new Object();
       //set bbox param from 'extents' object
-      minlon = extents.left;
-      minlat = extents.bottom;
-      maxlon = extents.right;
-      maxlat = extents.top;
-      minlon = Math.round(minlon * decimals) / decimals;
-      minlat = Math.round(minlat * decimals) / decimals;
-      maxlon = Math.round(maxlon * decimals) / decimals;
-      maxlat = Math.round(maxlat * decimals) / decimals;
-      args.bbox = minlon + "," + minlat + "," + maxlon + "," + maxlat;
-      node.href = setArgs("history/", args);
+      if (typeof minlon == "number" &&
+         typeof minlat == "number" &&
+         typeof maxlon == "number" &&
+         typeof maxlat == "number") {
+      
+        minlon = Math.round(minlon * decimals) / decimals;
+        minlat = Math.round(minlat * decimals) / decimals;
+        maxlon = Math.round(maxlon * decimals) / decimals;
+        maxlat = Math.round(maxlat * decimals) / decimals;
+        args.bbox = minlon + "," + minlat + "," + maxlon + "," + maxlat;
+      }
+      
+      node.href = setArgs("/history", args);
       node.style.fontStyle = 'normal';
     } else {
       node.href = 'javascript:alert("zoom in to see editing history");';
index d638c379c71dcfec8931c320b128b16b17cb65fa..20112747a9dc2eef3c462aa8550e5b03871242d2 100755 (executable)
Binary files a/public/potlatch/potlatch.swf and b/public/potlatch/potlatch.swf differ
index 0bab9c57ff4232d3f8a68638edfe0a75a0f587a3..57b6fe11c4c4806586c0cdda5eff68a88680488e 100644 (file)
@@ -202,7 +202,7 @@ body {
 #header {
   float: left;
   width: 100%;
-  background: #DAE0D2 url("bg.gif") repeat-x bottom;
+  background: #DAE0D2;
   font-size: 93%;
   line-height: normal;
 }
@@ -213,14 +213,12 @@ body {
 }
 #header li {
   float: left;
-  /*background:url("left.gif") no-repeat left top;*/
   margin: 0px;
   padding: 0px 0px 0px 9px;
 }
 #header li a {
   float: left;
   display: block;
-  /*background:url("right.gif") no-repeat right top;*/
   padding: 5px 15px 4px 6px;
   text-decoration: none;
   font-weight: bold;
@@ -233,11 +231,7 @@ hides rule from IE5-Mac \*/
 #header li a:hover {
   color: #333;
 }
-#header #current {
-  /* background-image:url("left_on.gif"); */
-}
 #header #current a {
-  background-image: url("right_on.gif");
   color: #333;
   padding-bottom: 5px;
 }
index 62c7e93a966eddfd229dc6ae3c14b01d831729c2..5a17e8be40da68fd10a4d74be0536418e8614e7d 100755 (executable)
@@ -15,7 +15,7 @@ puts "<body>"
 puts "<h2>OpenStreetMap stats report run at #{start_time.to_s}</h2>"
 
 begin
-  user_count = User.count(:conditions => "active = true")
+  user_count = User.count(:conditions => "active = 1")
   tracepoint_count = Tracepoint.count()
   node_count = Node.count(:conditions => "visible = true")
   way_count = Way.count(:conditions => "visible = true")
@@ -45,23 +45,23 @@ begin
   puts "<tr><th>Data Type</th><th>Day</th><th>Week</th><th>Month</th></tr>"
 
   day_count = Trace.count(:user_id, :distinct => true, 
-                          :conditions => "timestamp > NOW() - INTERVAL 1 DAY")
+                          :conditions => "timestamp > NOW() - '1 DAY'::INTERVAL")
   week_count = Trace.count(:user_id, :distinct => true, 
-                           :conditions => "timestamp > NOW() - INTERVAL 7 DAY")
+                           :conditions => "timestamp > NOW() - '7 DAYS'::INTERVAL")
   month_count = Trace.count(:user_id, :distinct => true, 
-                            :conditions => "timestamp > NOW() - INTERVAL 28 DAY")
+                            :conditions => "timestamp > NOW() - '28 DAYS'::INTERVAL")
 
   puts "<tr><th>GPX Files</th><td>#{day_count}</td><td>#{week_count}</td><td>#{month_count}</td></tr>"
 
   day_count = OldNode.count(:user_id, :distinct => true,
                             :include => :changeset,
-                            :conditions => "timestamp > NOW() - INTERVAL 1 DAY")
+                            :conditions => "timestamp > NOW() - '1 DAY'::INTERVAL")
   week_count = OldNode.count(:user_id, :distinct => true, 
                              :include => :changeset,
-                             :conditions => "timestamp > NOW() - INTERVAL 7 DAY")
+                             :conditions => "timestamp > NOW() - '7 DAYS'::INTERVAL")
   month_count = OldNode.count(:user_id, :distinct => true, 
                               :include => :changeset,
-                              :conditions => "timestamp > NOW() - INTERVAL 28 DAY")
+                              :conditions => "timestamp > NOW() - '28 DAYS'::INTERVAL")
 
   puts "<tr><th>Nodes</th><td>#{day_count}</td><td>#{week_count}</td><td>#{month_count}</td></tr>"
 
@@ -71,13 +71,13 @@ begin
   puts "<table>"
   puts "<tr><th>Day</th><th>Week</th><th>Month</th></tr>"
 
-  day_users = OldNode.count(:conditions => "timestamp > NOW() - INTERVAL 1 DAY",
+  day_users = OldNode.count(:conditions => "timestamp > NOW() - '1 DAY'::INTERVAL",
                             :include => :changeset, :group => :user_id,
                             :order => "count_all DESC")
-  week_users = OldNode.count(:conditions => "timestamp > NOW() - INTERVAL 7 DAY",
+  week_users = OldNode.count(:conditions => "timestamp > NOW() - '7 DAYS'::INTERVAL",
                              :include => :changeset, :group => :user_id,
                              :order => "count_all DESC", :limit => 60)
-  month_users = OldNode.count(:conditions => "timestamp > NOW() - INTERVAL 28 DAY",
+  month_users = OldNode.count(:conditions => "timestamp > NOW() - '28 DAYS'::INTERVAL",
                               :include => :changeset, :group => :user_id,
                               :order => "count_all DESC", :limit => 60)
 
index 055381ae48140af59b22c901a8f2d8bea85ffb38..b201bed4f984d7245f9b489736d9c90ab731ac67 100644 (file)
@@ -35,9 +35,9 @@ public_user_closed_change:
   closed_at: "2007-01-02 00:00:00"
   num_changes: 0
 
-normal_user_version_change:
+public_user_version_change:
   id: 4
-  user_id: 1
+  user_id: 2
   created_at: "2008-01-01 00:00:00"
   closed_at: <%= DateTime.now + Rational(1,24) %>
   min_lon: <%= 1 * SCALE %>
index f055371151e96483e074c98da66ab51094334742..f6418983ac51cc9acba22f538b14f6422418c0ef 100644 (file)
@@ -27,3 +27,9 @@ t5:
   member_role: "some"
   member_type: "Node"
   member_id: 5
+  
+public_used:
+  id: 4
+  member_role: "used by other relation"
+  member_type: "Relation"
+  member_id: 5
index d2755bdfd4edbe8781399363233f671a5de91f98..a797a336ce2be9ad4cd38380f5025edffc577a71 100644 (file)
@@ -12,3 +12,23 @@ t3:
   id: 3
   k: 'test'
   v: 'yes'
+
+mt_1:
+  id: 4
+  k: 'tag1'
+  v: 'val1'
+
+mt_2:
+  id: 4
+  k: 'tag2'
+  v: 'val2'
+
+mt_3:
+  id: 4
+  k: 'tag3'
+  v: 'val3'
+
+mt_4:
+  id: 4
+  k: 'tag4'
+  v: 'val4'
index 165f1a21e032a7623a9735cfc9d50d8e2d9eef97..da034395699c51b5f0b8be71b6c739443c28d0a3 100644 (file)
@@ -7,7 +7,7 @@ visible_relation:
 
 invisible_relation:
   id: 2
-  changeset_id: 1
+  changeset_id: 3
   timestamp: 2007-01-01 00:00:00
   visible: false
   version: 1
@@ -18,3 +18,17 @@ used_relation:
   timestamp: 2007-01-01 00:00:00
   visible: true
   version: 1
+  
+multi_tag_relation:
+  id: 4
+  changeset_id: 4
+  timestamp: 2009-04-21 09:50:57
+  visible: true
+  version: 1
+  
+public_used_relation:
+  id: 5
+  changeset_id: 2
+  timestamp: 2009-04-22 00:30:33
+  visible: true
+  version: 1
index 5a19bf3f26615034389af5b14223abc928153699..b37e5beae7cb46bd8890de546cc59ee56522c7b4 100644 (file)
@@ -22,3 +22,17 @@ t4:
   member_type: "Node"
   member_id: 5
   version: 1
+
+t5:
+  id: 2
+  member_role: "some"
+  member_type: "Node"
+  member_id: 5
+  version: 1
+  
+public_used:
+  id: 4
+  member_role: "used by other relation"
+  member_type: "Relation"
+  member_id: 5
+  version: 1
index 7e671672d5e4079b9ec3926dd1a478cdadbd15ff..f7571c30133d5ed344cf62f84c92bb142bba8a38 100644 (file)
@@ -15,3 +15,27 @@ t3:
   k: 'test'
   v: 'yes'
   version: 1
+
+mt_1:
+  id: 4
+  k: 'tag1'
+  v: 'val1'
+  version: 1
+
+mt_2:
+  id: 4
+  k: 'tag2'
+  v: 'val2'
+  version: 1
+
+mt_3:
+  id: 4
+  k: 'tag3'
+  v: 'val3'
+  version: 1
+
+mt_4:
+  id: 4
+  k: 'tag4'
+  v: 'val4'
+  version: 1
index 165f1a21e032a7623a9735cfc9d50d8e2d9eef97..5583527827f634b9b2932affffa27bd240784538 100644 (file)
@@ -7,7 +7,7 @@ visible_relation:
 
 invisible_relation:
   id: 2
-  changeset_id: 1
+  changeset_id: 3
   timestamp: 2007-01-01 00:00:00
   visible: false
   version: 1
@@ -18,3 +18,17 @@ used_relation:
   timestamp: 2007-01-01 00:00:00
   visible: true
   version: 1
+  
+multi_tag_relation:
+  id: 4
+  changeset_id: 4
+  timestamp: 2009-04-21 09:50:57
+  visible: true
+  version: 1
+  
+public_used_relation:
+  id: 5
+  changeset_id: 2
+  timestamp: 2009-04-22 00:30:03
+  visible: true
+  version: 1
index 9056931b95f700af970c2b338e7fd7ce98f59e16..fdb36628e4c82d6ef8c8ff32a6eac34be3fbda1f 100644 (file)
@@ -219,6 +219,7 @@ class ApiControllerTest < ActionController::TestCase
         assert_select "version[minimum=#{API_VERSION}][maximum=#{API_VERSION}]", :count => 1
         assert_select "area[maximum=#{APP_CONFIG['max_request_area']}]", :count => 1
         assert_select "tracepoints[per_page=#{APP_CONFIG['tracepoints_per_page']}]", :count => 1
+        assert_select "changesets[maximum_elements=#{Changeset::MAX_ELEMENTS}]", :count => 1
       end
     end
   end
index f1328e65091d7f7c26ac08176284db5efb080108..69ea0f345720a6e1d821c7bde8f5894c21d9e3fc 100644 (file)
@@ -12,10 +12,14 @@ class OldNodeControllerTest < ActionController::TestCase
   # test the version call by submitting several revisions of a new node
   # to the API and ensuring that later calls to version return the 
   # matching versions of the object.
+  #
+  ##
+  # FIXME Move this test to being an integration test since it spans multiple controllers 
   def test_version
+    ## First try this with a non-public user
     basic_authorization(users(:normal_user).email, "test")
     changeset_id = changesets(:normal_user_first_change).id
-
+    
     # setup a simple XML node
     xml_doc = current_nodes(:visible_node).to_xml
     xml_node = xml_doc.find("//osm/node").first
@@ -28,6 +32,57 @@ class OldNodeControllerTest < ActionController::TestCase
     # save a version for later checking
     versions[xml_node['version']] = xml_doc.to_s
 
+    # randomly move the node about
+    20.times do 
+      # move the node somewhere else
+      xml_node['lat'] = precision(rand * 180 -  90).to_s
+      xml_node['lon'] = precision(rand * 360 - 180).to_s
+      with_controller(NodeController.new) do
+        content xml_doc
+        put :update, :id => nodeid
+        assert_response :forbidden, "Should have rejected node update"
+        xml_node['version'] = @response.body.to_s
+      end
+      # save a version for later checking
+      versions[xml_node['version']] = xml_doc.to_s
+    end
+
+    # add a bunch of random tags
+    30.times do 
+      xml_tag = XML::Node.new("tag")
+      xml_tag['k'] = random_string
+      xml_tag['v'] = random_string
+      xml_node << xml_tag
+      with_controller(NodeController.new) do
+        content xml_doc
+        put :update, :id => nodeid
+        assert_response :forbidden,
+        "should have rejected node #{nodeid} (#{@response.body}) with forbidden"
+        xml_node['version'] = @response.body.to_s
+      end
+      # save a version for later checking
+      versions[xml_node['version']] = xml_doc.to_s
+    end
+
+    # probably should check that they didn't get written to the database
+
+    
+    ## Now do it with the public user
+    basic_authorization(users(:public_user).email, "test")
+    changeset_id = changesets(:public_user_first_change).id
+
+    # setup a simple XML node
+    xml_doc = current_nodes(:node_with_versions).to_xml
+    xml_node = xml_doc.find("//osm/node").first
+    nodeid = current_nodes(:node_with_versions).id
+
+    # keep a hash of the versions => string, as we'll need something
+    # to test against later
+    versions = Hash.new
+
+    # save a version for later checking
+    versions[xml_node['version']] = xml_doc.to_s
+
     # randomly move the node about
     20.times do 
       # move the node somewhere else
@@ -73,7 +128,19 @@ class OldNodeControllerTest < ActionController::TestCase
       assert_nodes_are_equal check_node, api_node
     end
   end
-
+  
+  def test_not_found_version
+    check_not_found_id_version(70000,312344)
+    check_not_found_id_version(-1, -13)
+    check_not_found_id_version(nodes(:visible_node).id, 24354)
+    check_not_found_id_version(24356,   nodes(:visible_node).version)
+  end
+  
+  def check_not_found_id_version(id, version)
+    get :version, :id => id, :version => version
+    assert_response :not_found
+  end
+  
   ##
   # Test that getting the current version is identical to picking
   # that version with the version URI call.
@@ -121,13 +188,4 @@ class OldNodeControllerTest < ActionController::TestCase
   def precision(f)
     return (f * GeoRecord::SCALE).round.to_f / GeoRecord::SCALE
   end
-
-  def basic_authorization(user, pass)
-    @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
-  end
-
-  def content(c)
-    @request.env["RAW_POST_DATA"] = c.to_s
-  end
-
 end
index d2dacd79c2408e15f2f0c53c2498b6f0ea226c42..9942353991286ba104c57687ee83e328953adf72 100644 (file)
@@ -77,11 +77,63 @@ class RelationControllerTest < ActionController::TestCase
   # -------------------------------------
 
   def test_create
-    basic_authorization "test@openstreetmap.org", "test"
+    basic_authorization users(:normal_user).email, "test"
     
     # put the relation in a dummy fixture changset
     changeset_id = changesets(:normal_user_first_change).id
 
+    # create an relation without members
+    content "<osm><relation changeset='#{changeset_id}'><tag k='test' v='yes' /></relation></osm>"
+    put :create
+    # hope for forbidden, due to user
+    assert_response :forbidden, 
+    "relation upload should have failed with forbidden"
+
+    ###
+    # create an relation with a node as member
+    # This time try with a role attribute in the relation
+    nid = current_nodes(:used_node_1).id
+    content "<osm><relation changeset='#{changeset_id}'>" +
+      "<member  ref='#{nid}' type='node' role='some'/>" +
+      "<tag k='test' v='yes' /></relation></osm>"
+    put :create
+    # hope for forbidden due to user
+    assert_response :forbidden, 
+    "relation upload did not return forbidden status"
+    
+    ###
+    # create an relation with a node as member, this time test that we don't 
+    # need a role attribute to be included
+    nid = current_nodes(:used_node_1).id
+    content "<osm><relation changeset='#{changeset_id}'>" +
+      "<member  ref='#{nid}' type='node'/>"+
+      "<tag k='test' v='yes' /></relation></osm>"
+    put :create
+    # hope for forbidden due to user
+    assert_response :forbidden, 
+    "relation upload did not return forbidden status"
+
+    ###
+    # create an relation with a way and a node as members
+    nid = current_nodes(:used_node_1).id
+    wid = current_ways(:used_way).id
+    content "<osm><relation changeset='#{changeset_id}'>" +
+      "<member type='node' ref='#{nid}' role='some'/>" +
+      "<member type='way' ref='#{wid}' role='other'/>" +
+      "<tag k='test' v='yes' /></relation></osm>"
+    put :create
+    # hope for forbidden, due to user
+    assert_response :forbidden, 
+        "relation upload did not return success status"
+
+
+
+    ## Now try with the public user
+    basic_authorization users(:public_user).email, "test"
+    
+    # put the relation in a dummy fixture changset
+    changeset_id = changesets(:public_user_first_change).id
+
     # create an relation without members
     content "<osm><relation changeset='#{changeset_id}'><tag k='test' v='yes' /></relation></osm>"
     put :create
@@ -100,7 +152,7 @@ class RelationControllerTest < ActionController::TestCase
         "saved relation does not contain exactly one tag"
     assert_equal changeset_id, checkrelation.changeset.id,
         "saved relation does not belong in the changeset it was assigned to"
-    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
+    assert_equal users(:public_user).id, checkrelation.changeset.user_id, 
         "saved relation does not belong to user that created it"
     assert_equal true, checkrelation.visible, 
         "saved relation is not visible"
@@ -132,7 +184,7 @@ class RelationControllerTest < ActionController::TestCase
         "saved relation does not contain exactly one tag"
     assert_equal changeset_id, checkrelation.changeset.id,
         "saved relation does not belong in the changeset it was assigned to"
-    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
+    assert_equal users(:public_user).id, checkrelation.changeset.user_id, 
         "saved relation does not belong to user that created it"
     assert_equal true, checkrelation.visible, 
         "saved relation is not visible"
@@ -165,7 +217,7 @@ class RelationControllerTest < ActionController::TestCase
         "saved relation does not contain exactly one tag"
     assert_equal changeset_id, checkrelation.changeset.id,
         "saved relation does not belong in the changeset it was assigned to"
-    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
+    assert_equal users(:public_user).id, checkrelation.changeset.user_id, 
         "saved relation does not belong to user that created it"
     assert_equal true, checkrelation.visible, 
         "saved relation is not visible"
@@ -198,7 +250,7 @@ class RelationControllerTest < ActionController::TestCase
         "saved relation does not contain exactly one tag"
     assert_equal changeset_id, checkrelation.changeset.id,
         "saved relation does not belong in the changeset it was assigned to"
-    assert_equal users(:normal_user).id, checkrelation.changeset.user_id, 
+    assert_equal users(:public_user).id, checkrelation.changeset.user_id, 
         "saved relation does not belong to user that created it"
     assert_equal true, checkrelation.visible, 
         "saved relation is not visible"
@@ -208,6 +260,68 @@ class RelationControllerTest < ActionController::TestCase
 
   end
 
+  # ------------------------------------
+  # Test updating relations
+  # ------------------------------------
+
+  ##
+  # test that, when tags are updated on a relation, the correct things
+  # happen to the correct tables and the API gives sensible results. 
+  # this is to test a case that gregory marler noticed and posted to
+  # josm-dev.
+  def test_update_relation_tags
+    basic_authorization "test@example.com", "test"
+    rel_id = current_relations(:multi_tag_relation).id
+    cs_id = changesets(:public_user_first_change).id
+
+    with_relation(rel_id) do |rel|
+      # alter one of the tags
+      tag = rel.find("//osm/relation/tag").first
+      tag['v'] = 'some changed value'
+      update_changeset(rel, cs_id)
+
+      # check that the downloaded tags are the same as the uploaded tags...
+      new_version = with_update(rel) do |new_rel|
+        assert_tags_equal rel, new_rel
+      end
+
+      # check the original one in the current_* table again
+      with_relation(rel_id) { |r| assert_tags_equal rel, r }
+
+      # now check the version in the history
+      with_relation(rel_id, new_version) { |r| assert_tags_equal rel, r }
+    end
+  end
+
+  ##
+  # test that, when tags are updated on a relation when using the diff
+  # upload function, the correct things happen to the correct tables 
+  # and the API gives sensible results. this is to test a case that 
+  # gregory marler noticed and posted to josm-dev.
+  def test_update_relation_tags_via_upload
+    basic_authorization "test@example.com", "test"
+    rel_id = current_relations(:multi_tag_relation).id
+    cs_id = changesets(:public_user_first_change).id
+
+    with_relation(rel_id) do |rel|
+      # alter one of the tags
+      tag = rel.find("//osm/relation/tag").first
+      tag['v'] = 'some changed value'
+      update_changeset(rel, cs_id)
+
+      # check that the downloaded tags are the same as the uploaded tags...
+      new_version = with_update_diff(rel) do |new_rel|
+        assert_tags_equal rel, new_rel
+      end
+
+      # check the original one in the current_* table again
+      with_relation(rel_id) { |r| assert_tags_equal rel, r }
+
+      # now check the version in the history
+      with_relation(rel_id, new_version) { |r| assert_tags_equal rel, r }
+    end
+  end
+
   # -------------------------------------
   # Test creating some invalid relations.
   # -------------------------------------
@@ -257,9 +371,59 @@ class RelationControllerTest < ActionController::TestCase
     # first try to delete relation without auth
     delete :delete, :id => current_relations(:visible_relation).id
     assert_response :unauthorized
+    
+    ## First try with the private user, to make sure that you get a forbidden
+    basic_authorization(users(:normal_user).email, "test")
+    
+    # this shouldn't work, as we should need the payload...
+    delete :delete, :id => current_relations(:visible_relation).id
+    assert_response :forbidden
+
+    # try to delete without specifying a changeset
+    content "<osm><relation id='#{current_relations(:visible_relation).id}'/></osm>"
+    delete :delete, :id => current_relations(:visible_relation).id
+    assert_response :forbidden
+
+    # try to delete with an invalid (closed) changeset
+    content update_changeset(current_relations(:visible_relation).to_xml,
+                             changesets(:normal_user_closed_change).id)
+    delete :delete, :id => current_relations(:visible_relation).id
+    assert_response :forbidden
+
+    # try to delete with an invalid (non-existent) changeset
+    content update_changeset(current_relations(:visible_relation).to_xml,0)
+    delete :delete, :id => current_relations(:visible_relation).id
+    assert_response :forbidden
 
-    # now set auth
-    basic_authorization("test@openstreetmap.org", "test");  
+    # this won't work because the relation is in-use by another relation
+    content(relations(:used_relation).to_xml)
+    delete :delete, :id => current_relations(:used_relation).id
+    assert_response :forbidden
+
+    # this should work when we provide the appropriate payload...
+    content(relations(:visible_relation).to_xml)
+    delete :delete, :id => current_relations(:visible_relation).id
+    assert_response :forbidden
+
+    # this won't work since the relation is already deleted
+    content(relations(:invisible_relation).to_xml)
+    delete :delete, :id => current_relations(:invisible_relation).id
+    assert_response :forbidden
+
+    # this works now because the relation which was using this one 
+    # has been deleted.
+    content(relations(:used_relation).to_xml)
+    delete :delete, :id => current_relations(:used_relation).id
+    assert_response :forbidden
+
+    # this won't work since the relation never existed
+    delete :delete, :id => 0
+    assert_response :forbidden
+
+    
+
+    # now set auth for the private user
+    basic_authorization(users(:public_user).email, "test");  
 
     # this shouldn't work, as we should need the payload...
     delete :delete, :id => current_relations(:visible_relation).id
@@ -282,15 +446,27 @@ class RelationControllerTest < ActionController::TestCase
     delete :delete, :id => current_relations(:visible_relation).id
     assert_response :conflict
 
-    # this won't work because the relation is in-use by another relation
+    # this won't work because the relation is in a changeset owned by someone else
     content(relations(:used_relation).to_xml)
     delete :delete, :id => current_relations(:used_relation).id
+    assert_response :conflict, 
+    "shouldn't be able to delete a relation in a changeset owned by someone else (#{@response.body})"
+
+    # this won't work because the relation in the payload is different to that passed
+    content(relations(:public_used_relation).to_xml)
+    delete :delete, :id => current_relations(:used_relation).id
+    assert_not_equal relations(:public_used_relation).id, current_relations(:used_relation).id
+    assert_response :bad_request, "shouldn't be able to delete a relation when payload is different to the url"
+    
+    # this won't work because the relation is in-use by another relation
+    content(relations(:public_used_relation).to_xml)
+    delete :delete, :id => current_relations(:public_used_relation).id
     assert_response :precondition_failed, 
        "shouldn't be able to delete a relation used in a relation (#{@response.body})"
 
     # this should work when we provide the appropriate payload...
-    content(relations(:visible_relation).to_xml)
-    delete :delete, :id => current_relations(:visible_relation).id
+    content(relations(:multi_tag_relation).to_xml)
+    delete :delete, :id => current_relations(:multi_tag_relation).id
     assert_response :success
 
     # valid delete should return the new version number, which should
@@ -305,8 +481,8 @@ class RelationControllerTest < ActionController::TestCase
 
     # this works now because the relation which was using this one 
     # has been deleted.
-    content(relations(:used_relation).to_xml)
-    delete :delete, :id => current_relations(:used_relation).id
+    content(relations(:public_used_relation).to_xml)
+    delete :delete, :id => current_relations(:public_used_relation).id
     assert_response :success, 
        "should be able to delete a relation used in an old relation (#{@response.body})"
 
@@ -388,11 +564,11 @@ class RelationControllerTest < ActionController::TestCase
   ##
   # check that relations are ordered
   def test_relation_member_ordering
-    basic_authorization("test@openstreetmap.org", "test");  
-
+    basic_authorization(users(:public_user).email, "test")
+    
     doc_str = <<OSM
 <osm>
- <relation changeset='1'>
+ <relation changeset='4'>
   <member ref='1' type='node' role='first'/>
   <member ref='3' type='node' role='second'/>
   <member ref='1' type='way' role='third'/>
@@ -437,11 +613,32 @@ OSM
   ## 
   # check that relations can contain duplicate members
   def test_relation_member_duplicates
-    basic_authorization("test@openstreetmap.org", "test");  
+    ## First try with the private user
+    basic_authorization(users(:normal_user).email, "test");  
 
     doc_str = <<OSM
 <osm>
- <relation changeset='1'>
+ <relation changeset='4'>
+  <member ref='1' type='node' role='forward'/>
+  <member ref='3' type='node' role='forward'/>
+  <member ref='1' type='node' role='forward'/>
+  <member ref='3' type='node' role='forward'/>
+ </relation>
+</osm>
+OSM
+    doc = XML::Parser.string(doc_str).parse
+
+    content doc
+    put :create
+    assert_response :forbidden
+
+
+    ## Now try with the public user
+    basic_authorization(users(:public_user).email, "test");  
+
+    doc_str = <<OSM
+<osm>
+ <relation changeset='4'>
   <member ref='1' type='node' role='forward'/>
   <member ref='3' type='node' role='forward'/>
   <member ref='1' type='node' role='forward'/>
@@ -489,8 +686,21 @@ OSM
   # create a changeset and yield to the caller to set it up, then assert
   # that the changeset bounding box is +bbox+.
   def check_changeset_modify(bbox)
-    basic_authorization("test@openstreetmap.org", "test");  
+    ## First test with the private user to check that you get a forbidden
+    basic_authorization(users(:normal_user).email, "test");  
+
+    # create a new changeset for this operation, so we are assured
+    # that the bounding box will be newly-generated.
+    changeset_id = with_controller(ChangesetController.new) do
+      content "<osm><changeset/></osm>"
+      put :create
+      assert_response :forbidden, "shouldn't be able to create changeset for modify test, as should get forbidden"
+    end
 
+    
+    ## Now do the whole thing with the public user
+    basic_authorization(users(:public_user).email, "test")
+    
     # create a new changeset for this operation, so we are assured
     # that the bounding box will be newly-generated.
     changeset_id = with_controller(ChangesetController.new) do
@@ -516,6 +726,101 @@ OSM
     end
   end
 
+  ##
+  # yields the relation with the given +id+ (and optional +version+
+  # to read from the history tables) into the block. the parsed XML
+  # doc is returned.
+  def with_relation(id, ver = nil)
+    if ver.nil?
+      get :read, :id => id
+    else
+      with_controller(OldRelationController.new) do
+        get :version, :id => id, :version => ver
+      end
+    end
+    assert_response :success
+    yield xml_parse(@response.body)
+  end
+
+  ##
+  # updates the relation (XML) +rel+ and 
+  # yields the new version of that relation into the block. 
+  # the parsed XML doc is retured.
+  def with_update(rel)
+    rel_id = rel.find("//osm/relation").first["id"].to_i
+    content rel
+    put :update, :id => rel_id
+    assert_response :success, "can't update relation: #{@response.body}"
+    version = @response.body.to_i
+
+    # now get the new version
+    get :read, :id => rel_id
+    assert_response :success
+    new_rel = xml_parse(@response.body)
+
+    yield new_rel
+
+    return version
+  end
+
+  ##
+  # updates the relation (XML) +rel+ via the diff-upload API and
+  # yields the new version of that relation into the block. 
+  # the parsed XML doc is retured.
+  def with_update_diff(rel)
+    rel_id = rel.find("//osm/relation").first["id"].to_i
+    cs_id = rel.find("//osm/relation").first['changeset'].to_i
+    version = nil
+
+    with_controller(ChangesetController.new) do
+      doc = OSM::API.new.get_xml_doc
+      change = XML::Node.new 'osmChange'
+      doc.root = change
+      modify = XML::Node.new 'modify'
+      change << modify
+      modify << doc.import(rel.find("//osm/relation").first)
+
+      content doc.to_s
+      post :upload, :id => cs_id
+      assert_response :success, "can't upload diff relation: #{@response.body}"
+      version = xml_parse(@response.body).find("//diffResult/relation").first["new_version"].to_i
+    end      
+    
+    # now get the new version
+    get :read, :id => rel_id
+    assert_response :success
+    new_rel = xml_parse(@response.body)
+    
+    yield new_rel
+    
+    return version
+  end
+
+  ##
+  # returns a k->v hash of tags from an xml doc
+  def get_tags_as_hash(a) 
+    a.find("//osm/relation/tag").inject({}) do |h,v|
+      h[v['k']] = v['v']
+      h
+    end
+  end
+  
+  ##
+  # assert that all tags on relation documents +a+ and +b+ 
+  # are equal
+  def assert_tags_equal(a, b)
+    # turn the XML doc into tags hashes
+    a_tags = get_tags_as_hash(a)
+    b_tags = get_tags_as_hash(b)
+
+    assert_equal a_tags.keys, b_tags.keys, "Tag keys should be identical."
+    a_tags.each do |k, v|
+      assert_equal v, b_tags[k], 
+        "Tags which were not altered should be the same. " +
+        "#{a_tags.inspect} != #{b_tags.inspect}"
+    end
+  end
+
   ##
   # update the changeset_id of a node element
   def update_changeset(xml, changeset_id)
index aee2901cdfe8df7464506254447d9c465a20244d..77874bcc6fd18787bca968082b538525dfa059bd 100644 (file)
@@ -4,7 +4,7 @@ class OldRelationTagTest < Test::Unit::TestCase
   api_fixtures
   
   def test_tag_count
-    assert_equal 3, OldRelationTag.count
+    assert_equal 7, OldRelationTag.count
   end
   
   def test_length_key_valid
index f0590ef71a7e10d45c5d5db29e549f6bb2af03a0..93fa551800b2dc0277ce82656ac9f9cce88f3b96 100644 (file)
@@ -4,7 +4,7 @@ class RelationMemberTest < Test::Unit::TestCase
   api_fixtures
   
   def test_relation_member_count
-    assert_equal 5, RelationMember.count
+    assert_equal 6, RelationMember.count
   end
   
 end
index 5c008fc34c415759dc2e8bda5f2819177e207416..54ee57e1e235a8392cb8b3d1f190c5ab7cc5cd5f 100644 (file)
@@ -4,7 +4,7 @@ class RelationTagTest < Test::Unit::TestCase
   api_fixtures
   
   def test_relation_tag_count
-    assert_equal 3, RelationTag.count
+    assert_equal 7, RelationTag.count
   end
   
   def test_length_key_valid
@@ -67,4 +67,20 @@ class RelationTagTest < Test::Unit::TestCase
     assert_raise(ActiveRecord::RecordInvalid) {tag.save!}
     assert tag.new_record?
   end
+
+  ##
+  # test that tags can be updated and saved uniquely, i.e: tag.save!
+  # only affects the single tag that the activerecord object 
+  # represents. this amounts to testing that the primary key is
+  # unique.
+  def test_update
+    v = "probably unique string here 3142592654"
+    assert_equal 0, RelationTag.count(:conditions => ['v=?', v])
+
+    tag = RelationTag.find(:first)
+    tag.v = v
+    tag.save!
+
+    assert_equal 1, RelationTag.count(:conditions => ['v=?', v])
+  end
 end
index 36aad7c253257f78353558a1e7fd0c8bf3ffa13b..5d46a6e599dc0c79eb3ea6c15033c13635bd3d1f 100644 (file)
@@ -4,7 +4,7 @@ class RelationTest < Test::Unit::TestCase
   api_fixtures
   
   def test_relation_count
-    assert_equal 3, Relation.count
+    assert_equal 5, Relation.count
   end
   
 end