From: Matt Amos Date: Fri, 24 Apr 2009 10:08:15 +0000 (+0000) Subject: Patching better 412 error messages from mis-commit on old api06 branch. X-Git-Tag: live~7529 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/6c66507427961a22a8f282b5b2f4ab7fda1dad6f?hp=9ff7ea8d71dedb5b29f6ff0b007568b3e699b196 Patching better 412 error messages from mis-commit on old api06 branch. --- diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index de3c7583b..d729fdd45 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -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 diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index ebf729afc..ca4cbcbd7 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -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 diff --git a/app/controllers/application.rb b/app/controllers/application.rb index bfd2e9c54..3d8a02810 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -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 diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 4913a600e..243609290 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -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); diff --git a/app/controllers/export_controller.rb b/app/controllers/export_controller.rb index 754cc4b82..ab25fcbc6 100644 --- a/app/controllers/export_controller.rb +++ b/app/controllers/export_controller.rb @@ -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] diff --git a/app/controllers/old_node_controller.rb b/app/controllers/old_node_controller.rb index 0976a0c9a..2590fd24a 100644 --- a/app/controllers/old_node_controller.rb +++ b/app/controllers/old_node_controller.rb @@ -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 diff --git a/app/controllers/old_relation_controller.rb b/app/controllers/old_relation_controller.rb index f8ebbd070..65de83278 100644 --- a/app/controllers/old_relation_controller.rb +++ b/app/controllers/old_relation_controller.rb @@ -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 diff --git a/app/controllers/old_way_controller.rb b/app/controllers/old_way_controller.rb index a42496687..2f1103216 100644 --- a/app/controllers/old_way_controller.rb +++ b/app/controllers/old_way_controller.rb @@ -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 diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 6b83e3301..17326db07 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -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) diff --git a/app/controllers/swf_controller.rb b/app/controllers/swf_controller.rb index 182fb8c54..9688af3ef 100644 --- a/app/controllers/swf_controller.rb +++ b/app/controllers/swf_controller.rb @@ -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) "+ diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index dc7456c45..e6c732d2d 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -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 diff --git a/app/models/old_node.rb b/app/models/old_node.rb index be115c53e..5e3638347 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -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 diff --git a/app/models/old_node_tag.rb b/app/models/old_node_tag.rb index 3fd4bf86b..dd339ad30 100644 --- a/app/models/old_node_tag.rb +++ b/app/models/old_node_tag.rb @@ -1,6 +1,6 @@ class OldNodeTag < ActiveRecord::Base set_table_name 'node_tags' - + belongs_to :user validates_presence_of :id, :version diff --git a/app/models/relation.rb b/app/models/relation.rb index 2032e6770..8a211b84f 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -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. diff --git a/app/models/way.rb b/app/models/way.rb index cbbe6ada4..97f44c117 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -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]) diff --git a/app/views/browse/_changeset_details.rhtml b/app/views/browse/_changeset_details.rhtml index 12d4dfb25..13b64d340 100644 --- a/app/views/browse/_changeset_details.rhtml +++ b/app/views/browse/_changeset_details.rhtml @@ -40,7 +40,7 @@ <%=maxlon -%> - <%= minlon -%> + <%= minlat -%> @@ -51,7 +51,7 @@ Has the following <%= @node_pages.item_count %> nodes: - +
<% @nodes.each do |node| %> <% end %> @@ -65,7 +65,7 @@
<%= link_to "Node #{node.id.to_s}, version #{node.version.to_s}", :action => "node", :id => node.id.to_s %>
Has the following <%= @way_pages.item_count %> ways: - +
<% @ways.each do |way| %> <% end %> @@ -82,7 +82,7 @@
<%= link_to "Way #{way.id.to_s}, version #{way.version.to_s}", :action => "way", :id => way.id.to_s %>
Has the following <%= @relation_pages.item_count %> relations: - +
<% @relations.each do |relation| %> <% end %> diff --git a/app/views/browse/_node_details.rhtml b/app/views/browse/_node_details.rhtml index e3d11e0fe..1aa5bf69c 100644 --- a/app/views/browse/_node_details.rhtml +++ b/app/views/browse/_node_details.rhtml @@ -6,7 +6,7 @@
<%= link_to "Relation #{relation.id.to_s}, version #{relation.version.to_s}", :action => "relation", :id => relation.id.to_s %>
Part of: - +
<% node_details.ways.each do |way| %> <% end %> diff --git a/app/views/browse/_relation_details.rhtml b/app/views/browse/_relation_details.rhtml index b8874d9ce..e2249e92f 100644 --- a/app/views/browse/_relation_details.rhtml +++ b/app/views/browse/_relation_details.rhtml @@ -6,7 +6,7 @@ @@ -17,7 +17,7 @@ diff --git a/app/views/browse/_relation_member.rhtml b/app/views/browse/_relation_member.rhtml index 516d9e37f..fe484c3cd 100644 --- a/app/views/browse/_relation_member.rhtml +++ b/app/views/browse/_relation_member.rhtml @@ -1,7 +1,7 @@ diff --git a/app/views/browse/_way_details.rhtml b/app/views/browse/_way_details.rhtml index bc00100d8..9dc35ad53 100644 --- a/app/views/browse/_way_details.rhtml +++ b/app/views/browse/_way_details.rhtml @@ -5,7 +5,7 @@
<%= link_to "Way " + way.id.to_s, :action => "way", :id => way.id.to_s %>
Members: - +
<%= render :partial => "relation_member", :collection => relation_details.relation_members %>
Part of: - +
<%= render :partial => "containing_relation", :collection => relation_details.containing_relation_members %>
- <%= 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) %> diff --git a/app/views/browse/_tag_details.rhtml b/app/views/browse/_tag_details.rhtml index dab36266b..6f82689c2 100644 --- a/app/views/browse/_tag_details.rhtml +++ b/app/views/browse/_tag_details.rhtml @@ -2,7 +2,7 @@
Tags: - +
<%= render :partial => "tag", :collection => tag_details.tags_as_hash %>
Nodes: - +
<% way_details.way_nodes.each do |wn| %> <% end %> @@ -17,7 +17,7 @@ diff --git a/app/views/changeset/_changeset.rhtml b/app/views/changeset/_changeset.rhtml index a149a72e9..4012c41a3 100644 --- a/app/views/changeset/_changeset.rhtml +++ b/app/views/changeset/_changeset.rhtml @@ -14,7 +14,7 @@ <%if showusername %>
<%= link_to "Node " + wn.node_id.to_s, :action => "node", :id => wn.node_id.to_s %>
Part of: - +
<%= render :partial => "containing_relation", :collection => way_details.containing_relation_members %>
<% 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 %> Anonymous <% end %> diff --git a/app/views/changeset/list.rhtml b/app/views/changeset/list.rhtml index 4ac5082e1..aa745743a 100644 --- a/app/views/changeset/list.rhtml +++ b/app/views/changeset/list.rhtml @@ -1,5 +1,5 @@

Recent Changes

-

Recently closed changesets:

+

Recently edited changesets:

diff --git a/app/views/layouts/site.rhtml b/app/views/layouts/site.rhtml index 534003031..93abf3a5e 100644 --- a/app/views/layouts/site.rhtml +++ b/app/views/layouts/site.rhtml @@ -50,7 +50,7 @@ %>
  • <%= link_to 'View', {:controller => 'site', :action => 'index'}, {:id => 'viewanchor', :title => 'view maps', :class => viewclass} %>
  • <%= link_to 'Edit', {:controller => 'site', :action => 'edit'}, {:id => 'editanchor', :title => 'edit maps', :class => editclass} %>
    • -
  • <%= link_to 'History', {:controller => 'history' }, {:id => 'historyanchor', :title => 'changeset history', :class => historyclass} %>
    • +
  • <%= link_to 'History', {:controller => 'changeset', :action => 'list_bbox' }, {:id => 'historyanchor', :title => 'changeset history', :class => historyclass} %>
    • <% if params['controller'] == 'site' and (params['action'] == 'index' or params['action'] == 'export') %>
  • <%= link_to_remote 'Export', {:url => {:controller => 'export', :action => 'start'}}, {:id => 'exportanchor', :title => 'export map data', :class => exportclass, :href => url_for(:controller => 'site', :action => 'export')} %>
    • <% else %> diff --git a/app/views/site/edit.rhtml b/app/views/site/edit.rhtml index e341305f5..a3a2b532a 100644 --- a/app/views/site/edit.rhtml +++ b/app/views/site/edit.rhtml @@ -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); diff --git a/app/views/site/index.rhtml b/app/views/site/index.rhtml index 54f281727..b8fcf579e 100644 --- a/app/views/site/index.rhtml +++ b/app/views/site/index.rhtml @@ -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; } diff --git a/app/views/user/view.rhtml b/app/views/user/view.rhtml index a62be6a55..c76ca23ae 100644 --- a/app/views/user/view.rhtml +++ b/app/views/user/view.rhtml @@ -107,4 +107,4 @@
    <% 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 %> diff --git a/config/database.yml b/config/database.yml index cc3f9a1a5..c6fe06fdd 100644 --- a/config/database.yml +++ b/config/database.yml @@ -11,10 +11,10 @@ # 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 diff --git a/config/environment.rb b/config/environment.rb index 7f83fd4ad..c303c3535 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -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. diff --git a/config/lighttpd.conf b/config/lighttpd.conf index 419f25e32..22051eb44 100644 --- a/config/lighttpd.conf +++ b/config/lighttpd.conf @@ -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 index 000000000..954be24af --- /dev/null +++ b/config/mongrel_cluster.yml @@ -0,0 +1,5 @@ +--- +log_file: log/mongrel.log +port: 8000 +pid_file: tmp/mongrel.pid +servers: 8 \ No newline at end of file diff --git a/config/nginx.conf b/config/nginx.conf new file mode 100644 index 000000000..ae349b746 --- /dev/null +++ b/config/nginx.conf @@ -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; + } + } +} diff --git a/config/routes.rb b/config/routes.rb index fcf22bced..f2d644719 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 index 000000000..fed0af7c0 --- /dev/null +++ b/db/migrate/026_add_changeset_user_index.rb @@ -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 index 000000000..0d4bba0e8 --- /dev/null +++ b/db/migrate/027_add_changeset_indexes.rb @@ -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 index 000000000..d93cc79e7 --- /dev/null +++ b/db/migrate/028_add_more_changeset_indexes.rb @@ -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 diff --git a/lib/migrate.rb b/lib/migrate.rb index 392060b60..05b3c90f2 100644 --- a/lib/migrate.rb +++ b/lib/migrate.rb @@ -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 diff --git a/public/javascripts/site.js b/public/javascripts/site.js index 06b4152df..ae38ecb6a 100644 --- a/public/javascripts/site.js +++ b/public/javascripts/site.js @@ -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");'; diff --git a/public/potlatch/potlatch.swf b/public/potlatch/potlatch.swf index d638c379c..20112747a 100755 Binary files a/public/potlatch/potlatch.swf and b/public/potlatch/potlatch.swf differ diff --git a/public/stylesheets/site.css b/public/stylesheets/site.css index 0bab9c57f..57b6fe11c 100644 --- a/public/stylesheets/site.css +++ b/public/stylesheets/site.css @@ -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; } diff --git a/script/statistics b/script/statistics index 62c7e93a9..5a17e8be4 100755 --- a/script/statistics +++ b/script/statistics @@ -15,7 +15,7 @@ puts "" puts "

    OpenStreetMap stats report run at #{start_time.to_s}

    " 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 "" 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 "" 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 "" @@ -71,13 +71,13 @@ begin puts "
    Data TypeDayWeekMonth
    GPX Files#{day_count}#{week_count}#{month_count}
    Nodes#{day_count}#{week_count}#{month_count}
    " puts "" - 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) diff --git a/test/fixtures/changesets.yml b/test/fixtures/changesets.yml index 055381ae4..b201bed4f 100644 --- a/test/fixtures/changesets.yml +++ b/test/fixtures/changesets.yml @@ -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 %> diff --git a/test/fixtures/current_relation_members.yml b/test/fixtures/current_relation_members.yml index f05537115..f6418983a 100644 --- a/test/fixtures/current_relation_members.yml +++ b/test/fixtures/current_relation_members.yml @@ -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 diff --git a/test/fixtures/current_relation_tags.yml b/test/fixtures/current_relation_tags.yml index d2755bdfd..a797a336c 100644 --- a/test/fixtures/current_relation_tags.yml +++ b/test/fixtures/current_relation_tags.yml @@ -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' diff --git a/test/fixtures/current_relations.yml b/test/fixtures/current_relations.yml index 165f1a21e..da0343956 100644 --- a/test/fixtures/current_relations.yml +++ b/test/fixtures/current_relations.yml @@ -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 diff --git a/test/fixtures/relation_members.yml b/test/fixtures/relation_members.yml index 5a19bf3f2..b37e5beae 100644 --- a/test/fixtures/relation_members.yml +++ b/test/fixtures/relation_members.yml @@ -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 diff --git a/test/fixtures/relation_tags.yml b/test/fixtures/relation_tags.yml index 7e671672d..f7571c301 100644 --- a/test/fixtures/relation_tags.yml +++ b/test/fixtures/relation_tags.yml @@ -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 diff --git a/test/fixtures/relations.yml b/test/fixtures/relations.yml index 165f1a21e..558352782 100644 --- a/test/fixtures/relations.yml +++ b/test/fixtures/relations.yml @@ -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 diff --git a/test/functional/api_controller_test.rb b/test/functional/api_controller_test.rb index 9056931b9..fdb36628e 100644 --- a/test/functional/api_controller_test.rb +++ b/test/functional/api_controller_test.rb @@ -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 diff --git a/test/functional/old_node_controller_test.rb b/test/functional/old_node_controller_test.rb index f1328e650..69ea0f345 100644 --- a/test/functional/old_node_controller_test.rb +++ b/test/functional/old_node_controller_test.rb @@ -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 diff --git a/test/functional/relation_controller_test.rb b/test/functional/relation_controller_test.rb index d2dacd79c..994235399 100644 --- a/test/functional/relation_controller_test.rb +++ b/test/functional/relation_controller_test.rb @@ -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 "" + 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 "" + + "" + + "" + 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 "" + + ""+ + "" + 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 "" + + "" + + "" + + "" + 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 "" 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 "" + 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 = < - + @@ -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 + 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 = < + @@ -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 "" + 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) diff --git a/test/unit/old_relation_tag_test.rb b/test/unit/old_relation_tag_test.rb index aee2901cd..77874bcc6 100644 --- a/test/unit/old_relation_tag_test.rb +++ b/test/unit/old_relation_tag_test.rb @@ -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 diff --git a/test/unit/relation_member_test.rb b/test/unit/relation_member_test.rb index f0590ef71..93fa55180 100644 --- a/test/unit/relation_member_test.rb +++ b/test/unit/relation_member_test.rb @@ -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 diff --git a/test/unit/relation_tag_test.rb b/test/unit/relation_tag_test.rb index 5c008fc34..54ee57e1e 100644 --- a/test/unit/relation_tag_test.rb +++ b/test/unit/relation_tag_test.rb @@ -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 diff --git a/test/unit/relation_test.rb b/test/unit/relation_test.rb index 36aad7c25..5d46a6e59 100644 --- a/test/unit/relation_test.rb +++ b/test/unit/relation_test.rb @@ -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
    DayWeekMonth