From: Tom Hughes Date: Mon, 20 Apr 2009 09:12:03 +0000 (+0000) Subject: Merge api06 branch to trunk. X-Git-Tag: live~7615 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/fd6f00b069af5a7b87227ea4c3401dc461d7311a?hp=80c6c685bcf097754e872ba61444fc4bb93f2515 Merge api06 branch to trunk. --- diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 7f85280b7..b0b3f13cf 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -3,7 +3,7 @@ # OSM database takes place using this controller. Messages are # encoded in the Actionscript Message Format (AMF). # -# Helper functions are in /lib/potlatch. +# Helper functions are in /lib/potlatch.rb # # Author:: editions Systeme D / Richard Fairhurst 2004-2008 # Licence:: public domain. @@ -14,6 +14,12 @@ # from the AMF message), each method generally takes arguments in the order # they were sent by the Potlatch SWF. Do not assume typing has been preserved. # Methods all return an array to the SWF. +# +# == API 0.6 +# +# Note that this requires a patched version of composite_primary_keys 1.1.0 +# (see http://groups.google.com/group/compositekeys/t/a00e7562b677e193) +# if you are to run with POTLATCH_USE_SQL=false . # # == Debugging # @@ -21,13 +27,19 @@ # return(-1,"message") <-- just puts up a dialogue # return(-2,"message") <-- also asks the user to e-mail me # -# To write to the Rails log, use RAILS_DEFAULT_LOGGER.info("message"). +# To write to the Rails log, use logger.info("message"). + +# Remaining issues: +# * version conflict when POIs and ways are reverted class AmfController < ApplicationController require 'stringio' include Potlatch + # Help methods for checking boundary sanity and area size + include MapBoundary + session :off before_filter :check_api_writable @@ -36,235 +48,403 @@ class AmfController < ApplicationController # ** FIXME: refactor to reduce duplication of code across read/write def amf_read - req=StringIO.new(request.raw_post+0.chr)# Get POST data as request - # (cf http://www.ruby-forum.com/topic/122163) - req.read(2) # Skip version indicator and client ID - results={} # Results of each body + req=StringIO.new(request.raw_post+0.chr)# Get POST data as request + # (cf http://www.ruby-forum.com/topic/122163) + req.read(2) # Skip version indicator and client ID + results={} # Results of each body - # Parse request + # Parse request headers=AMF.getint(req) # Read number of headers - headers.times do # Read each header - name=AMF.getstring(req) # | - req.getc # | skip boolean - value=AMF.getvalue(req) # | - header["name"]=value # | - end - - bodies=AMF.getint(req) # Read number of bodies - bodies.times do # Read each body - message=AMF.getstring(req) # | get message name - index=AMF.getstring(req) # | get index in response sequence - bytes=AMF.getlong(req) # | get total size in bytes - args=AMF.getvalue(req) # | get response (probably an array) - - case message - 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 '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].to_i)) - when 'getway_history'; results[index]=AMF.putdata(index,getway_history(args[0].to_i)) - when 'getnode_history'; results[index]=AMF.putdata(index,getnode_history(args[0].to_i)) - when 'findrelations'; results[index]=AMF.putdata(index,findrelations(*args)) - when 'getpoi'; results[index]=AMF.putdata(index,getpoi(*args)) - end - end + headers.times do # Read each header + name=AMF.getstring(req) # | + req.getc # | skip boolean + value=AMF.getvalue(req) # | + header["name"]=value # | + end + + bodies=AMF.getint(req) # Read number of bodies + bodies.times do # Read each body + message=AMF.getstring(req) # | get message name + index=AMF.getstring(req) # | get index in response sequence + bytes=AMF.getlong(req) # | get total size in bytes + args=AMF.getvalue(req) # | get response (probably an array) + logger.info("Executing AMF #{message}:#{index}") + + case message + 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 '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)) + when 'getnode_history'; results[index]=AMF.putdata(index,getnode_history(args[0].to_i)) + when 'findgpx'; results[index]=AMF.putdata(index,findgpx(*args)) + when 'findrelations'; results[index]=AMF.putdata(index,findrelations(*args)) + when 'getpoi'; results[index]=AMF.putdata(index,getpoi(*args)) + end + end + logger.info("encoding AMF results") sendresponse(results) end def amf_write - req=StringIO.new(request.raw_post+0.chr) - req.read(2) - results={} - renumberednodes={} # Shared across repeated putways - renumberedways={} # Shared across repeated putways - - headers=AMF.getint(req) # Read number of headers - headers.times do # Read each header - name=AMF.getstring(req) # | - req.getc # | skip boolean - value=AMF.getvalue(req) # | - header["name"]=value # | - end - - bodies=AMF.getint(req) # Read number of bodies - bodies.times do # Read each body - message=AMF.getstring(req) # | get message name - index=AMF.getstring(req) # | get index in response sequence - bytes=AMF.getlong(req) # | get total size in bytes - args=AMF.getvalue(req) # | get response (probably an array) - - case message - when 'putway'; r=putway(renumberednodes,*args) + req=StringIO.new(request.raw_post+0.chr) + req.read(2) + results={} + renumberednodes={} # Shared across repeated putways + renumberedways={} # Shared across repeated putways + + headers=AMF.getint(req) # Read number of headers + headers.times do # Read each header + name=AMF.getstring(req) # | + req.getc # | skip boolean + value=AMF.getvalue(req) # | + header["name"]=value # | + end + + bodies=AMF.getint(req) # Read number of bodies + bodies.times do # Read each body + message=AMF.getstring(req) # | get message name + index=AMF.getstring(req) # | get index in response sequence + bytes=AMF.getlong(req) # | get total size in bytes + args=AMF.getvalue(req) # | get response (probably an array) + + logger.info("Executing AMF #{message}:#{index}") + case message + when 'putway'; r=putway(renumberednodes,*args) renumberednodes=r[3] - if r[1] != r[2] - renumberedways[r[1]] = r[2] - end + if r[1] != r[2] then renumberedways[r[1]] = r[2] end results[index]=AMF.putdata(index,r) - when 'putrelation'; results[index]=AMF.putdata(index,putrelation(renumberednodes, renumberedways, *args)) - when 'deleteway'; results[index]=AMF.putdata(index,deleteway(args[0],args[1].to_i)) - when 'putpoi'; results[index]=AMF.putdata(index,putpoi(*args)) - end - end + when 'putrelation'; results[index]=AMF.putdata(index,putrelation(renumberednodes, renumberedways, *args)) + when 'deleteway'; results[index]=AMF.putdata(index,deleteway(*args)) + when 'putpoi'; r=putpoi(*args) + if r[1] != r[2] then renumberednodes[r[1]] = r[2] end + results[index]=AMF.putdata(index,r) + when 'startchangeset'; results[index]=AMF.putdata(index,startchangeset(*args)) + end + end + logger.info("encoding AMF results") sendresponse(results) end private + # Start new changeset + + def startchangeset(usertoken, cstags, closeid, closecomment) + user = getuser(usertoken) + if !user then return -1,"You are not logged in, so Potlatch can't write any changes to the database." end + + # close previous changeset and add comment + if closeid + cs = Changeset.find(closeid) + cs.set_closed_time_now + if cs.user_id!=user.id + return -2,"You cannot close that changeset because you're not the person who opened it." + elsif closecomment.empty? + cs.save! + else + cs.tags['comment']=closecomment + cs.save_with_tags! + end + end + + # open a new changeset + cs = Changeset.new + cs.tags = cstags + cs.user_id = user.id + # smsm1 doesn't like the next two lines and thinks they need to be abstracted to the model more/better + cs.created_at = Time.now.getutc + cs.closed_at = cs.created_at + Changeset::IDLE_TIMEOUT + cs.save_with_tags! + return [0,cs.id] + end + # Return presets (default tags, localisation etc.): # uses POTLATCH_PRESETS global, set up in OSM::Potlatch. def getpresets() #:doc: - return POTLATCH_PRESETS + return POTLATCH_PRESETS end + ## # Find all the ways, POI nodes (i.e. not part of ways), and relations # in a given bounding box. Nodes are returned in full; ways and relations # are IDs only. - + # + # return is of the form: + # [error_code, + # [[way_id, way_version], ...], + # [[node_id, lat, lon, [tags, ...], node_version], ...], + # [[rel_id, rel_version], ...]] + # where the ways are any visible ways which refer to any visible + # nodes in the bbox, nodes are any visible nodes in the bbox but not + # used in any way, rel is any relation which refers to either a way + # or node that we're returning. def whichways(xmin, ymin, xmax, ymax) #:doc: - enlarge = [(xmax-xmin)/8,0.01].min - xmin -= enlarge; ymin -= enlarge - xmax += enlarge; ymax += enlarge - - if POTLATCH_USE_SQL then - way_ids = sql_find_way_ids_in_area(xmin, ymin, xmax, ymax) - points = sql_find_pois_in_area(xmin, ymin, xmax, ymax) - relation_ids = sql_find_relations_in_area_and_ways(xmin, ymin, xmax, ymax, way_ids) - else - # find the way ids in an area - nodes_in_area = Node.find_by_area(ymin, xmin, ymax, xmax, :conditions => "current_nodes.visible = 1", :include => :ways) - way_ids = nodes_in_area.collect { |node| node.way_ids }.flatten.uniq - - # find the node ids in an area that aren't part of ways - nodes_not_used_in_area = nodes_in_area.select { |node| node.ways.empty? } - points = nodes_not_used_in_area.collect { |n| [n.id, n.lon, n.lat, n.tags_as_hash] } - - # find the relations used by those nodes and ways - relations = Relation.find_for_nodes(nodes_in_area.collect { |n| n.id }, :conditions => "visible = 1") + - Relation.find_for_ways(way_ids, :conditions => "visible = 1") - relation_ids = relations.collect { |relation| relation.id }.uniq - end - - [way_ids, points, relation_ids] + enlarge = [(xmax-xmin)/8,0.01].min + xmin -= enlarge; ymin -= enlarge + xmax += enlarge; ymax += enlarge + + # check boundary is sane and area within defined + # see /config/application.yml + check_boundaries(xmin, ymin, xmax, ymax) + + if POTLATCH_USE_SQL then + ways = sql_find_ways_in_area(xmin, ymin, xmax, ymax) + points = sql_find_pois_in_area(xmin, ymin, xmax, ymax) + relations = sql_find_relations_in_area_and_ways(xmin, ymin, xmax, ymax, ways.collect {|x| x[0]}) + else + # find the way ids in an area + nodes_in_area = Node.find_by_area(ymin, xmin, ymax, xmax, :conditions => ["current_nodes.visible = ?", true], :include => :ways) + ways = nodes_in_area.inject([]) { |sum, node| + visible_ways = node.ways.select { |w| w.visible? } + sum + visible_ways.collect { |w| [w.id,w.version] } + }.uniq + ways.delete([]) + + # find the node ids in an area that aren't part of ways + nodes_not_used_in_area = nodes_in_area.select { |node| node.ways.empty? } + points = nodes_not_used_in_area.collect { |n| [n.id, n.lon, n.lat, n.tags, n.version] }.uniq + + # find the relations used by those nodes and ways + relations = Relation.find_for_nodes(nodes_in_area.collect { |n| n.id }, :conditions => {:visible => true}) + + Relation.find_for_ways(ways.collect { |w| w[0] }, :conditions => {:visible => true}) + relations = relations.collect { |relation| [relation.id,relation.version] }.uniq + end + + [0, ways, points, relations] + + rescue Exception => err + [-2,"Sorry - I can't get the map for that area."] end # Find deleted ways in current bounding box (similar to whichways, but ways # with a deleted node only - not POIs or relations). def whichways_deleted(xmin, ymin, xmax, ymax) #:doc: - xmin -= 0.01; ymin -= 0.01 - xmax += 0.01; ymax += 0.01 - - nodes_in_area = Node.find_by_area(ymin, xmin, ymax, xmax, :conditions => "current_nodes.visible = 0 AND current_ways.visible = 0", :include => :ways_via_history) - way_ids = nodes_in_area.collect { |node| node.ways_via_history_ids }.flatten.uniq - - [way_ids] + enlarge = [(xmax-xmin)/8,0.01].min + xmin -= enlarge; ymin -= enlarge + xmax += enlarge; ymax += enlarge + + # check boundary is sane and area within defined + # see /config/application.yml + begin + check_boundaries(xmin, ymin, xmax, ymax) + rescue Exception => err + return [-2,"Sorry - I can't get the map for that area."] + end + + nodes_in_area = Node.find_by_area(ymin, xmin, ymax, xmax, :conditions => ["current_ways.visible = ?", false], :include => :ways_via_history) + way_ids = nodes_in_area.collect { |node| node.ways_via_history_ids }.flatten.uniq + + [0,way_ids] end # Get a way including nodes and tags. - # Returns 0 (success), a Potlatch-style array of points, and a hash of tags. + # Returns the way id, a Potlatch-style array of points, a hash of tags, and the version number. def getway(wayid) #:doc: - if POTLATCH_USE_SQL then - points = sql_get_nodes_in_way(wayid) - tags = sql_get_tags_in_way(wayid) - else - # Ideally we would do ":include => :nodes" here but if we do that - # then rails only seems to return the first copy of a node when a - # way includes a node more than once - way = Way.find(wayid) - points = way.nodes.collect do |node| - nodetags=node.tags_as_hash - nodetags.delete('created_by') - [node.lon, node.lat, node.id, nodetags] - end - tags = way.tags - end - - [wayid, points, tags] + if POTLATCH_USE_SQL then + points = sql_get_nodes_in_way(wayid) + tags = sql_get_tags_in_way(wayid) + version = sql_get_way_version(wayid) + else + # Ideally we would do ":include => :nodes" here but if we do that + # then rails only seems to return the first copy of a node when a + # way includes a node more than once + begin + way = Way.find(wayid) + rescue ActiveRecord::RecordNotFound + return [wayid,[],{}] + end + + # check case where way has been deleted or doesn't exist + return [wayid,[],{}] if way.nil? or !way.visible + + points = way.nodes.collect do |node| + nodetags=node.tags + nodetags.delete('created_by') + [node.lon, node.lat, node.id, nodetags, node.version] + end + tags = way.tags + version = way.version + end + + [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, - # even if it's moved. For revert (version=1+), uses the node in existence + # For undelete (version<0), always uses the most recent version of each node, + # even if it's moved. For revert (version >= 0), uses the node in existence # at the time, generating a new id if it's still visible and has been moved/ # retagged. - - def getway_old(id, version) #:doc: - if version < 0 - old_way = OldWay.find(:first, :conditions => ['visible = 1 AND id = ?', id], :order => 'version DESC') - points = old_way.get_nodes_undelete - else - old_way = OldWay.find(:first, :conditions => ['id = ? AND version = ?', id, version]) - points = old_way.get_nodes_revert - end - - old_way.tags['history'] = "Retrieved from v#{old_way.version}" - - [0, id, points, old_way.tags, old_way.version] + # + # Returns: + # 0. success code, + # 1. id, + # 2. array of points, + # 3. hash of tags, + # 4. version, + # 5. is this the current, visible version? (boolean) + + def getway_old(id, timestamp) #:doc: + if timestamp == '' + # undelete + old_way = OldWay.find(:first, :conditions => ['visible = ? AND id = ?', true, id], :order => 'version DESC') + points = old_way.get_nodes_undelete unless old_way.nil? + else + begin + # revert + timestamp = DateTime.strptime(timestamp.to_s, "%d %b %Y, %H:%M:%S") + old_way = OldWay.find(:first, :conditions => ['id = ? AND timestamp <= ?', id, timestamp], :order => 'timestamp DESC') + unless old_way.nil? + points = old_way.get_nodes_revert(timestamp) + if !old_way.visible + return [-1, "Sorry, the way was deleted at that time - please revert to a previous version."] + end + end + rescue ArgumentError + # thrown by date parsing method. leave old_way as nil for + # the superb error handler below. + end + end + + if old_way.nil? + # *** FIXME: shouldn't this be returning an error? + return [-1, id, [], {}, -1,0] + else + curway=Way.find(id) + old_way.tags['history'] = "Retrieved from v#{old_way.version}" + return [0, id, points, old_way.tags, curway.version, (curway.version==old_way.version and curway.visible)] + end end - # Find history of a way. Returns 'way', id, and - # an array of previous versions. + # Find history of a way. + # Returns 'way', id, and an array of previous versions: + # - formerly [old_way.version, old_way.timestamp.strftime("%d %b %Y, %H:%M"), old_way.visible ? 1 : 0, user, uid] + # - now [timestamp,user,uid] + # + # Heuristic: Find all nodes that have ever been part of the way; + # get a list of their revision dates; add revision dates of the way; + # sort and collapse list (to within 2 seconds); trim all dates before the + # start date of the way. def getway_history(wayid) #:doc: - history = Way.find(wayid).old_ways.reverse.collect do |old_way| - user = old_way.user.data_public? ? old_way.user.display_name : 'anonymous' - uid = old_way.user.data_public? ? old_way.user.id : 0 - [old_way.version, old_way.timestamp.strftime("%d %b %Y, %H:%M"), old_way.visible ? 1 : 0, user, uid] - end - ['way',wayid,history] + begin + # Find list of revision dates for way and all constituent nodes + revdates=[] + revusers={} + Way.find(wayid).old_ways.collect do |a| + revdates.push(a.timestamp) + unless revusers.has_key?(a.timestamp.to_i) then revusers[a.timestamp.to_i]=change_user(a) end + a.nds.each do |n| + Node.find(n).old_nodes.collect do |o| + revdates.push(o.timestamp) + unless revusers.has_key?(o.timestamp.to_i) then revusers[o.timestamp.to_i]=change_user(o) end + end + end + end + waycreated=revdates[0] + revdates.uniq! + revdates.sort! + revdates.reverse! + + # Remove any dates (from nodes) before first revision date of way + revdates.delete_if { |d| d0 then + gpx = Trace.find(searchterm.to_i, :conditions => ["visible=? AND (public=? OR user_id=?)",true,true,user.id] ) + if gpx then + gpxs.push([gpx.id, gpx.name, gpx.description]) + end + else + Trace.find(:all, :limit => 21, :conditions => ["visible=? AND (public=? OR user_id=?) AND MATCH(name) AGAINST (?)",true,true,user.id,searchterm] ).each do |gpx| + gpxs.push([gpx.id, gpx.name, gpx.description]) + end + end + gpxs end # Get a relation with all tags and members. # Returns: # 0. relation id, # 1. hash of tags, - # 2. list of members. + # 2. list of members, + # 3. version. def getrelation(relid) #:doc: - rel = Relation.find(relid) - - [relid, rel.tags, rel.members] + begin + rel = Relation.find(relid) + rescue ActiveRecord::RecordNotFound + return [relid, {}, []] + end + + return [relid, {}, [], nil] if rel.nil? or !rel.visible + [relid, rel.tags, rel.members, rel.version] end # Find relations with specified name/id. # Returns array of relations, each in same form as getrelation. def findrelations(searchterm) - rels = [] - if searchterm.to_i>0 then - rel = Relation.find(searchterm.to_i) - if rel and rel.visible then - rels.push([rel.id, rel.tags, rel.members]) - end - else - RelationTag.find(:all, :limit => 11, :conditions => ["match(v) against (?)", searchterm] ).each do |t| - if t.relation.visible then + rels = [] + if searchterm.to_i>0 then + rel = Relation.find(searchterm.to_i) + if rel and rel.visible then + rels.push([rel.id, rel.tags, rel.members]) + end + else + RelationTag.find(:all, :limit => 11, :conditions => ["match(v) against (?)", searchterm] ).each do |t| + if t.relation.visible then rels.push([t.relation.id, t.relation.tags, t.relation.members]) end end end - rels + rels end # Save a relation. @@ -273,226 +453,335 @@ class AmfController < ApplicationController # 1. original relation id (unchanged), # 2. new relation id. - def putrelation(renumberednodes, renumberedways, usertoken, relid, tags, members, visible) #:doc: - uid = getuserid(usertoken) - if !uid then return -1,"You are not logged in, so the relation could not be saved." end - - relid = relid.to_i - visible = visible.to_i - - # create a new relation, or find the existing one - if relid <= 0 - rel = Relation.new - else - rel = Relation.find(relid) - end - - # check the members are all positive, and correctly type - typedmembers = [] - 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' - end - if mid - typedmembers << [m[0], mid, m[2]] - end - end - - # assign new contents - rel.members = typedmembers - rel.tags = tags - rel.visible = visible - rel.user_id = uid - - # check it then save it - # BUG: the following is commented out because it always fails on my - # install. I think it's a Rails bug. - - #if !rel.preconditions_ok? - # return -2, "Relation preconditions failed" - #else - rel.save_with_history! - #end - - [0, relid, rel.id] + def putrelation(renumberednodes, renumberedways, usertoken, changeset_id, version, relid, tags, members, visible) #:doc: + user = getuser(usertoken) + if !user then return -1,"You are not logged in, so the relation could not be saved." end + + relid = relid.to_i + visible = (visible.to_i != 0) + + new_relation = nil + relation = nil + Relation.transaction do + # create a new relation, or find the existing one + if relid > 0 + relation = Relation.find(relid) + end + # We always need a new node, based on the data that has been sent to us + new_relation = Relation.new + + # check the members are all positive, and correctly type + typedmembers = [] + 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' + end + if mid + typedmembers << [m[0], mid, m[2]] + end + end + + # assign new contents + new_relation.members = typedmembers + new_relation.tags = tags + new_relation.visible = visible + 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) + elsif visible + # We're updating the node + relation.update_from(new_relation, user) + else + # We're deleting the node + relation.delete_with_history!(new_relation, user) + end + end # transaction + + if id <= 0 + return [0, relid, new_relation.id, new_relation.version] + else + return [0, relid, relation.id, relation.version] + end + rescue OSM::APIChangesetAlreadyClosedError => ex + return [-1, "The changeset #{ex.changeset.id} was closed at #{ex.changeset.closed_at}."] + rescue OSM::APIVersionMismatchError => ex + # Really need to check to see whether this is a server load issue, and the + # last version was in the same changeset, or belongs to the same user, then + # we can return something different + return [-3, "Sorry, someone else has changed this relation since you started editing. Please click the 'Edit' tab to reload the area."] + rescue OSM::APIAlreadyDeletedError => ex + 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 :-( ."] end # Save a way to the database, including all nodes. Any nodes in the previous # version and no longer used are deleted. # + # Parameters: + # 0. hash of renumbered nodes (added by amf_controller) + # 1. current user token (for authentication) + # 2. current changeset + # 3. new way version + # 4. way ID + # 5. list of nodes in way + # 6. hash of way tags + # 7. array of nodes to change (each one is [lon,lat,id,version,tags]) + # # Returns: # 0. '0' (code for success), # 1. original way id (unchanged), # 2. new way id, - # 3. hash of renumbered nodes (old id=>new id) + # 3. hash of renumbered nodes (old id=>new id), + # 4. way version, + # 5. hash of node versions (node=>version) - def putway(renumberednodes, usertoken, originalway, points, attributes) #:doc: + def putway(renumberednodes, usertoken, changeset_id, wayversion, originalway, pointlist, attributes, nodes) #:doc: - # -- Initialise and carry out checks + # -- Initialise - uid = getuserid(usertoken) - if !uid then return -1,"You are not logged in, so the way could not be saved." end - - originalway = originalway.to_i - - points.each do |a| - if a[2] == 0 or a[2].nil? then return -2,"Server error - node with id 0 found in way #{originalway}." end - if a[1] == 90 then return -2,"Server error - node with lat -90 found in way #{originalway}." end - end - - if points.length < 2 then return -2,"Server error - way is only #{points.length} points long." end - - # -- Get unique nodes - - if originalway < 0 - way = Way.new - uniques = [] - else - way = Way.find(originalway) - uniques = way.unshared_node_ids - end - - # -- Compare nodes and save changes to any that have changed - - nodes = [] - - points.each do |n| - lon = n[0].to_f - lat = n[1].to_f - id = n[2].to_i - savenode = false - - if renumberednodes[id] - id = renumberednodes[id] - elsif id < 0 - # Create new node - node = Node.new - savenode = true - else - node = Node.find(id) - nodetags=node.tags_as_hash - nodetags.delete('created_by') - if !fpcomp(lat, node.lat) or !fpcomp(lon, node.lon) or - n[4] != nodetags or !node.visible? - savenode = true - end - end - - if savenode - node.user_id = uid - node.lat = lat + user = getuser(usertoken) + if !user then return -1,"You are not logged in, so the way could not be saved." end + if pointlist.length < 2 then return -2,"Server error - way is only #{points.length} points long." end + + originalway = originalway.to_i + pointlist.collect! {|a| a.to_i } + + way=nil # this is returned, so scope it outside the transaction + nodeversions = {} + Way.transaction do + + # -- Get unique nodes + + if originalway <= 0 + uniques = [] + else + way = Way.find(originalway) + uniques = way.unshared_node_ids + end + + #-- Update each changed node + + nodes.each do |a| + lon = a[0].to_f + lat = a[1].to_f + id = a[2].to_i + version = a[3].to_i + if id == 0 then return -2,"Server error - node with id 0 found in way #{originalway}." end + if lat== 90 then return -2,"Server error - node with latitude -90 found in way #{originalway}." end + if renumberednodes[id] then id = renumberednodes[id] end + + node = Node.new + node.changeset_id = changeset_id + node.lat = lat node.lon = lon - node.tags = Tags.join(n[4]) - node.visible = true - node.save_with_history! - - if id != node.id - renumberednodes[id] = node.id - id = node.id - end - end - - uniques = uniques - [id] - nodes.push(id) - end - - # -- Save revised way - - way.tags = attributes - way.nds = nodes - way.user_id = uid - way.visible = true - way.save_with_history! - - # -- Delete any unique nodes - - uniques.each do |n| - deleteitemrelations(n, 'node') - - node = Node.find(n) - node.user_id = uid - node.visible = false - node.save_with_history! - end - - [0, originalway, way.id, renumberednodes] + node.tags = a[4] + node.tags.delete('created_by') + node.version = version + if id <= 0 + # We're creating the node + node.create_with_history(user) + renumberednodes[id] = node.id + nodeversions[node.id] = node.version + else + # We're updating an existing node + previous=Node.find(id) + previous.update_from(node, user) + nodeversions[previous.id] = previous.version + end + end + + # -- Save revised way + + pointlist.collect! {|a| + renumberednodes[a] ? renumberednodes[a]:a + } # renumber nodes + new_way = Way.new + new_way.tags = attributes + new_way.nds = pointlist + new_way.changeset_id = changeset_id + new_way.version = wayversion + if originalway <= 0 + new_way.create_with_history(user) + way=new_way # so we can get way.id and way.version + elsif way.tags!=attributes or way.nds!=pointlist or !way.visible? + way.update_from(new_way, user) + end + + # -- Delete any unique nodes no longer used + + uniques=uniques-pointlist + uniques.each do |n| + node = Node.find(n) + deleteitemrelations(user, changeset_id, id, 'node', node.version) + new_node = Node.new + new_node.changeset_id = changeset_id + new_node.version = node.version + node.delete_with_history!(new_node, user) + end + + end # transaction + + [0, originalway, way.id, renumberednodes, way.version, nodeversions] + rescue OSM::APIChangesetAlreadyClosedError => ex + return [-2, "Sorry, your changeset #{ex.changeset.id} has been closed (at #{ex.changeset.closed_at})."] + rescue OSM::APIVersionMismatchError => ex + # Really need to check to see whether this is a server load issue, and the + # last version was in the same changeset, or belongs to the same user, then + # we can return something different + return [-3, "Sorry, someone else has changed this way since you started editing. Please click the 'Edit' tab to reload the area."] + rescue OSM::APITooManyWayNodesError => ex + return [-1, "You have tried to upload a really long way with #{ex.provided} points: only #{ex.max} are allowed."] + rescue OSM::APIAlreadyDeletedError => ex + 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 :-(."] end # Save POI to the database. # Refuses save if the node has since become part of a way. - # Returns: + # Returns array with: # 0. 0 (success), # 1. original node id (unchanged), - # 2. new node id. - - def putpoi(usertoken, id, lon, lat, tags, visible) #:doc: - uid = getuserid(usertoken) - if !uid then return -1,"You are not logged in, so the point could not be saved." end - - id = id.to_i - visible = (visible.to_i == 1) - - if id > 0 then - node = Node.find(id) - - if !visible then - unless node.ways.empty? then return -1,"The point has since become part of a way, so you cannot save it as a POI." end - deleteitemrelations(id, 'node') - end - else - node = Node.new - end - - node.user_id = uid - node.lat = lat - node.lon = lon - node.tags = Tags.join(tags) - node.visible = visible - node.save_with_history! - - [0, id, node.id] + # 2. new node id, + # 3. version. + + def putpoi(usertoken, changeset_id, version, id, lon, lat, tags, visible) #:doc: + user = getuser(usertoken) + if !user then return -1,"You are not logged in, so the point could not be saved." end + + id = id.to_i + visible = (visible.to_i == 1) + node = nil + new_node = nil + Node.transaction do + if id > 0 then + node = Node.find(id) + + if !visible then + unless node.ways.empty? then return -1,"The point has since become part of a way, so you cannot save it as a POI." end + end + end + # We always need a new node, based on the data that has been sent to us + new_node = Node.new + + new_node.changeset_id = changeset_id + new_node.version = version + new_node.lat = lat + new_node.lon = lon + new_node.tags = tags + if id <= 0 + # We're creating the node + new_node.create_with_history(user) + elsif visible + # We're updating the node + node.update_from(new_node, user) + else + # We're deleting the node + node.delete_with_history!(new_node, user) + end + end # transaction + + if id <= 0 + return [0, id, new_node.id, new_node.version] + else + return [0, id, node.id, node.version] + end + rescue OSM::APIChangesetAlreadyClosedError => ex + return [-1, "The changeset #{ex.changeset.id} was closed at #{ex.changeset.closed_at}"] + rescue OSM::APIVersionMismatchError => ex + # Really need to check to see whether this is a server load issue, and the + # last version was in the same changeset, or belongs to the same user, then + # we can return something different + return [-3, "Sorry, someone else has changed this point since you started editing. Please click the 'Edit' tab to reload the area."] + rescue OSM::APIAlreadyDeletedError => ex + 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 :-()"] end # Read POI from database # (only called on revert: POIs are usually read by whichways). # - # Returns array of id, long, lat, hash of tags. + # Returns array of id, long, lat, hash of tags, version. def getpoi(id,timestamp) #:doc: - if timestamp>0 then - n = OldNode.find(id, :conditions=>['UNIX_TIMESTAMP(timestamp)=?',timestamp]) - else - n = Node.find(id) - end - - if n - return [n.id, n.lon, n.lat, n.tags_as_hash] - else - return [nil, nil, nil, ''] - end + if timestamp == '' then + n = Node.find(id) + else + n = OldNode.find(id, :conditions=>['timestamp=?',DateTime.strptime(timestamp, "%d %b %Y, %H:%M:%S")]) + end + + if n + return [n.id, n.lon, n.lat, n.tags, n.version] + else + return [nil, nil, nil, {}, nil] + end end # Delete way and all constituent nodes. Also removes from any relations. + # Params: + # * The user token + # * the changeset id + # * the id of the way to change + # * the version of the way that was downloaded + # * a hash of the id and versions of all the nodes that are in the way, if any + # of the nodes have been changed by someone else then, there is a problem! # Returns 0 (success), unchanged way id. - def deleteway(usertoken, way_id) #:doc: - uid = getuserid(usertoken) - if !uid then return -1,"You are not logged in, so the way could not be deleted." end - - # FIXME: would be good not to make two history entries when removing - # two nodes from the same relation - user = User.find(uid) - way = Way.find(way_id) - way.unshared_node_ids.each do |n| - deleteitemrelations(n, 'node') - end - deleteitemrelations(way_id, 'way') - - way.delete_with_relations_and_nodes_and_history(user) - - [0, way_id] + def deleteway(usertoken, changeset_id, way_id, way_version, node_id_version) #:doc: + user = getuser(usertoken) + unless user then return -1,"You are not logged in, so the way could not be deleted." end + + way_id = way_id.to_i + # Need a transaction so that if one item fails to delete, the whole delete fails. + Way.transaction do + + # delete the way + old_way = Way.find(way_id) + 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| + # delete the node + node = Node.find(node_id) + delete_node = Node.new + delete_node.changeset_id = changeset_id + if node_id_version[node_id.to_s] + delete_node.version = node_id_version[node_id.to_s] + 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) + delete_node.version = node.version + end + node.delete_with_history!(delete_node, user) + end + end # transaction + [0, way_id] + rescue OSM::APIChangesetAlreadyClosedError => ex + return [-1, "The changeset #{ex.changeset.id} was closed at #{ex.changeset.closed_at}"] + rescue OSM::APIVersionMismatchError => ex + # Really need to check to see whether this is a server load issue, and the + # last version was in the same changeset, or belongs to the same user, then + # we can return something different + return [-3, "Sorry, someone else has changed this way since you started editing. Please click the 'Edit' tab to reload the area."] + rescue OSM::APIAlreadyDeletedError => ex + 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 :-( ."] end @@ -500,141 +789,147 @@ class AmfController < ApplicationController # Support functions # Remove a node or way from all relations + # This is only used by putway and deleteway when deleting nodes removed + # from a way (because Potlatch itself doesn't keep track of these - + # possible FIXME). - def deleteitemrelations(objid, type) #:doc: - relations = RelationMember.find(:all, + def deleteitemrelations(user, changeset_id, objid, type, version) #:doc: + relations = RelationMember.find(:all, :conditions => ['member_type = ? and member_id = ?', type, objid], :include => :relation).collect { |rm| rm.relation }.uniq - relations.each do |rel| - rel.members.delete_if { |x| x[0] == type and x[1] == objid } - rel.save_with_history! - end - end - - # Break out node tags into a hash - # (should become obsolete as of API 0.6) - - def tagstring_to_hash(a) #:doc: - tags={} - Tags.split(a) do |k, v| - tags[k]=v - end - tags + relations.each do |rel| + rel.members.delete_if { |x| x[0] == type and x[1] == objid } + new_rel = Relation.new + new_rel.tags = rel.tags + new_rel.visible = rel.visible + new_rel.version = rel.version + new_rel.members = rel.members + new_rel.changeset_id = changeset_id + rel.update_from(new_rel, user) + end end # Authenticate token # (can also be of form user:pass) - - def getuserid(token) #:doc: - if (token =~ /^(.+)\:(.+)$/) then - user = User.authenticate(:username => $1, :password => $2) - else - user = User.authenticate(:token => token) - end - - return user ? user.id : nil; - end - - # Compare two floating-point numbers to within 0.0000001 - - def fpcomp(a,b) #:doc: - return ((a/0.0000001).round==(b/0.0000001).round) + # When we are writing to the api, we need the actual user model, + # not just the id, hence this abstraction + + def getuser(token) #:doc: + if (token =~ /^(.+)\:(.+)$/) then + user = User.authenticate(:username => $1, :password => $2) + else + user = User.authenticate(:token => token) + end + return user end # Send AMF response def sendresponse(results) - a,b=results.length.divmod(256) - render :content_type => "application/x-amf", :text => proc { |response, output| - output.write 0.chr+0.chr+0.chr+0.chr+a.chr+b.chr - results.each do |k,v| - output.write(v) - end - } + a,b=results.length.divmod(256) + render :content_type => "application/x-amf", :text => proc { |response, output| + # ** move amf writing loop into here - + # basically we read the messages in first (into an array of some sort), + # then iterate through that array within here, and do all the AMF writing + output.write 0.chr+0.chr+0.chr+0.chr+a.chr+b.chr + results.each do |k,v| + output.write(v) + end + } end # ==================================================================== # Alternative SQL queries for getway/whichways - def sql_find_way_ids_in_area(xmin,ymin,xmax,ymax) - sql=<<-EOF - SELECT DISTINCT current_way_nodes.id AS wayid - FROM current_way_nodes - INNER JOIN current_nodes ON current_nodes.id=current_way_nodes.node_id - INNER JOIN current_ways ON current_ways.id =current_way_nodes.id - WHERE current_nodes.visible=1 - AND current_ways.visible=1 - AND #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "current_nodes.")} - EOF - return ActiveRecord::Base.connection.select_all(sql).collect { |a| a['wayid'].to_i } + def sql_find_ways_in_area(xmin,ymin,xmax,ymax) + sql=<<-EOF + SELECT DISTINCT current_ways.id AS wayid,current_ways.version AS version + FROM current_way_nodes + INNER JOIN current_nodes ON current_nodes.id=current_way_nodes.node_id + INNER JOIN current_ways ON current_ways.id =current_way_nodes.id + WHERE current_nodes.visible=TRUE + AND current_ways.visible=TRUE + AND #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "current_nodes.")} + EOF + return ActiveRecord::Base.connection.select_all(sql).collect { |a| [a['wayid'].to_i,a['version'].to_i] } end def sql_find_pois_in_area(xmin,ymin,xmax,ymax) - sql=<<-EOF - SELECT current_nodes.id,current_nodes.latitude*0.0000001 AS lat,current_nodes.longitude*0.0000001 AS lon,current_nodes.tags + pois=[] + sql=<<-EOF + SELECT current_nodes.id,current_nodes.latitude*0.0000001 AS lat,current_nodes.longitude*0.0000001 AS lon,current_nodes.version FROM current_nodes - LEFT OUTER JOIN current_way_nodes cwn ON cwn.node_id=current_nodes.id - WHERE current_nodes.visible=1 + LEFT OUTER JOIN current_way_nodes cwn ON cwn.node_id=current_nodes.id + WHERE current_nodes.visible=TRUE AND cwn.id IS NULL AND #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "current_nodes.")} - EOF - return ActiveRecord::Base.connection.select_all(sql).collect { |n| [n['id'].to_i,n['lon'].to_f,n['lat'].to_f,tagstring_to_hash(n['tags'])] } + EOF + ActiveRecord::Base.connection.select_all(sql).each do |row| + poitags={} + ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n| + poitags[n['k']]=n['v'] + end + pois << [row['id'].to_i, row['lon'].to_f, row['lat'].to_f, poitags, row['version'].to_i] + end + pois end def sql_find_relations_in_area_and_ways(xmin,ymin,xmax,ymax,way_ids) - # ** It would be more Potlatchy to get relations for nodes within ways - # during 'getway', not here - sql=<<-EOF - SELECT DISTINCT cr.id AS relid - 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' - WHERE #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "cn.")} - EOF - unless way_ids.empty? - sql+=<<-EOF - UNION - SELECT DISTINCT cr.id AS relid - FROM current_relations cr - INNER JOIN current_relation_members crm ON crm.id=cr.id - 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 }.uniq + # ** It would be more Potlatchy to get relations for nodes within ways + # during 'getway', not here + sql=<<-EOF + 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' + WHERE #{OSM.sql_for_area(ymin, xmin, ymax, xmax, "cn.")} + EOF + unless way_ids.empty? + sql+=<<-EOF + UNION + 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' + 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] } 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,tags - FROM current_way_nodes,current_nodes - WHERE current_way_nodes.id=#{wayid.to_i} + points=[] + sql=<<-EOF + SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,current_nodes.id + FROM current_way_nodes,current_nodes + WHERE current_way_nodes.id=#{wayid.to_i} AND current_way_nodes.node_id=current_nodes.id - AND current_nodes.visible=1 - ORDER BY sequence_id + AND current_nodes.visible=TRUE + ORDER BY sequence_id EOF - ActiveRecord::Base.connection.select_all(sql).each do |row| - nodetags=tagstring_to_hash(row['tags']) - nodetags.delete('created_by') - points << [row['lon'].to_f,row['lat'].to_f,row['id'].to_i,nodetags] - end - points + ActiveRecord::Base.connection.select_all(sql).each do |row| + nodetags={} + ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n| + nodetags[n['k']]=n['v'] + end + nodetags.delete('created_by') + points << [row['lon'].to_f,row['lat'].to_f,row['id'].to_i,nodetags] + end + points end def sql_get_tags_in_way(wayid) - tags={} - ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_way_tags WHERE id=#{wayid.to_i}").each do |row| - tags[row['k']]=row['v'] - end - tags + tags={} + ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_way_tags WHERE id=#{wayid.to_i}").each do |row| + tags[row['k']]=row['v'] + end + tags end + def sql_get_way_version(wayid) + ActiveRecord::Base.connection.select_one("SELECT version FROM current_ways WHERE id=#{wayid.to_i}") + end end -# Local Variables: -# indent-tabs-mode: t -# tab-width: 4 -# End: diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 029e4c8a6..ebf729afc 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -11,12 +11,13 @@ class ApiController < ApplicationController @@count = COUNT # The maximum area you're allowed to request, in square degrees - MAX_REQUEST_AREA = 0.25 + MAX_REQUEST_AREA = APP_CONFIG['max_request_area'] # Number of GPS trace/trackpoints returned per-page - TRACEPOINTS_PER_PAGE = 5000 + TRACEPOINTS_PER_PAGE = APP_CONFIG['tracepoints_per_page'] - + # Get an XML response containing a list of tracepoints that have been uploaded + # within the specified bounding box, and in the specified page. def trackpoints @@count+=1 #retrieve the page number @@ -84,6 +85,15 @@ class ApiController < ApplicationController render :text => doc.to_s, :content_type => "text/xml" end + # This is probably the most common call of all. It is used for getting the + # OSM data for a specified bounding box, usually for editing. First the + # bounding box (bbox) is checked to make sure that it is sane. All nodes + # are searched, then all the ways that reference those nodes are found. + # All Nodes that are referenced by those ways are fetched and added to the list + # of nodes. + # Then all the relations that reference the already found nodes and ways are + # fetched. All the nodes and ways that are referenced by those ways are then + # fetched. Finally all the xml is returned. def map GC.start @@count+=1 @@ -109,18 +119,19 @@ class ApiController < ApplicationController return end - @nodes = Node.find_by_area(min_lat, min_lon, max_lat, max_lon, :conditions => "visible = 1", :limit => APP_CONFIG['max_number_of_nodes']+1) + # FIXME um why is this area using a different order for the lat/lon from above??? + @nodes = Node.find_by_area(min_lat, min_lon, max_lat, max_lon, :conditions => {:visible => true}, :limit => APP_CONFIG['max_number_of_nodes']+1) # get all the nodes, by tag not yet working, waiting for change from NickB # need to be @nodes (instance var) so tests in /spec can be performed #@nodes = Node.search(bbox, params[:tag]) node_ids = @nodes.collect(&:id) if node_ids.length > APP_CONFIG['max_number_of_nodes'] - report_error("You requested too many nodes (limit is 50,000). Either request a smaller area, or use planet.osm") + report_error("You requested too many nodes (limit is #{APP_CONFIG['max_number_of_nodes']}). Either request a smaller area, or use planet.osm") return end if node_ids.length == 0 - render :text => "", :content_type => "text/xml" + render :text => "", :content_type => "text/xml" return end @@ -176,15 +187,15 @@ class ApiController < ApplicationController end end - relations = Relation.find_for_nodes(visible_nodes.keys, :conditions => "visible = 1") + - Relation.find_for_ways(way_ids, :conditions => "visible = 1") + relations = Relation.find_for_nodes(visible_nodes.keys, :conditions => {:visible => true}) + + Relation.find_for_ways(way_ids, :conditions => {:visible => true}) # we do not normally return the "other" partners referenced by an relation, # e.g. if we return a way A that is referenced by relation X, and there's # another way B also referenced, that is not returned. But we do make # an exception for cases where an relation references another *relation*; # in that case we return that as well (but we don't go recursive here) - relations += Relation.find_for_relations(relations.collect { |r| r.id }, :conditions => "visible = 1") + relations += Relation.find_for_relations(relations.collect { |r| r.id }, :conditions => {:visible => true}) # this "uniq" may be slightly inefficient; it may be better to first collect and output # all node-related relations, then find the *not yet covered* way-related ones etc. @@ -204,6 +215,8 @@ class ApiController < ApplicationController end end + # Get a list of the tiles that have changed within a specified time + # period def changes zoom = (params[:zoom] || '12').to_i @@ -212,12 +225,12 @@ class ApiController < ApplicationController endtime = Time.parse(params[:end]) else hours = (params[:hours] || '1').to_i.hours - endtime = Time.now + endtime = Time.now.getutc starttime = endtime - hours end if zoom >= 1 and zoom <= 16 and - endtime >= starttime and endtime - starttime <= 24.hours + endtime > starttime and endtime - starttime <= 24.hours mask = (1 << zoom) - 1 tiles = Node.count(:conditions => ["timestamp BETWEEN ? AND ?", starttime, endtime], @@ -245,21 +258,32 @@ class ApiController < ApplicationController render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => :bad_request + render :text => "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", :status => :bad_request end end + # External apps that use the api are able to query the api to find out some + # parameters of the API. It currently returns: + # * minimum and maximum API versions that can be used. + # * maximum area that can be requested in a bbox request in square degrees + # * number of tracepoints that are returned in each tracepoints page def capabilities doc = OSM::API.new.get_xml_doc api = XML::Node.new 'api' version = XML::Node.new 'version' - version['minimum'] = '0.5'; - version['maximum'] = '0.5'; + version['minimum'] = "#{API_VERSION}"; + version['maximum'] = "#{API_VERSION}"; api << version area = XML::Node.new 'area' area['maximum'] = MAX_REQUEST_AREA.to_s; api << area + tracepoints = XML::Node.new 'tracepoints' + tracepoints['per_page'] = APP_CONFIG['tracepoints_per_page'].to_s + api << tracepoints + waynodes = XML::Node.new 'waynodes' + waynodes['maximum'] = APP_CONFIG['max_number_of_way_nodes'].to_s + api << waynodes doc.root << api diff --git a/app/controllers/application.rb b/app/controllers/application.rb index 615022656..bfd2e9c54 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base def authorize_web if session[:user] - @user = User.find(session[:user], :conditions => "visible = 1") + @user = User.find(session[:user], :conditions => {:visible => true}) elsif session[:token] @user = User.authenticate(:token => session[:token]) session[:user] = @user.id @@ -22,7 +22,11 @@ class ApplicationController < ActionController::Base redirect_to :controller => 'user', :action => 'login', :referer => request.request_uri unless @user end - def authorize(realm='Web Password', errormessage="Couldn't authenticate you") + ## + # sets up the @user object for use by other methods. this is mostly called + # from the authorize method, but can be called elsewhere if authorisation + # is optional. + def setup_user_auth username, passwd = get_auth_data # parse from headers # authenticate per-scheme if username.nil? @@ -32,6 +36,11 @@ class ApplicationController < ActionController::Base else @user = User.authenticate(:username => username, :password => passwd) # basic auth end + end + + def authorize(realm='Web Password', errormessage="Couldn't authenticate you") + # make the @user object from any auth sources we have + setup_user_auth # handle authenticate pass/fail unless @user @@ -73,13 +82,21 @@ class ApplicationController < ActionController::Base end end + def require_public_data + unless @user.data_public? + response.headers['Error'] = "You must make your edits public to upload new data" + render :nothing => true, :status => :forbidden + return false + end + end + # Report and error to the user # (If anyone ever fixes Rails so it can set a http status "reason phrase", # rather than only a status code and having the web engine make up a # phrase from that, we can also put the error message into the status # message. For now, rails won't let us) def report_error(message) - render :nothing => true, :status => :bad_request + render :text => message, :status => :bad_request # Todo: some sort of escaping of problem characters in the message response.headers['Error'] = message end @@ -90,6 +107,8 @@ private def get_auth_data if request.env.has_key? 'X-HTTP_AUTHORIZATION' # where mod_rewrite might have put it authdata = request.env['X-HTTP_AUTHORIZATION'].to_s.split + elsif request.env.has_key? 'REDIRECT_X_HTTP_AUTHORIZATION' # mod_fcgi + authdata = request.env['REDIRECT_X_HTTP_AUTHORIZATION'].to_s.split elsif request.env.has_key? 'HTTP_AUTHORIZATION' # regular location authdata = request.env['HTTP_AUTHORIZATION'].to_s.split end diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index df48a2090..6ace0817b 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -7,103 +7,109 @@ class BrowseController < ApplicationController def start end - def index - @nodes = Node.find(:all, :order => "timestamp DESC", :limit=> 20) - end + def relation - begin - @relation = Relation.find(params[:id]) - - @name = @relation.tags['name'].to_s - if @name.length == 0: - @name = "#" + @relation.id.to_s - end - - @title = 'Relation | ' + (@name) - @next = Relation.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @relation.id }] ) - @prev = Relation.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @relation.id }] ) - rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + @relation = Relation.find(params[:id]) + + @name = @relation.tags['name'].to_s + if @name.length == 0: + @name = "#" + @relation.id.to_s end + + @title = 'Relation | ' + (@name) + @next = Relation.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @relation.id }] ) + @prev = Relation.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @relation.id }] ) + rescue ActiveRecord::RecordNotFound + @type = "relation" + render :action => "not_found", :status => :not_found end def relation_history - begin - @relation = Relation.find(params[:id]) + @relation = Relation.find(params[:id]) - @name = @relation.tags['name'].to_s - if @name.length == 0: - @name = "#" + @relation.id.to_s - end - - @title = 'Relation History | ' + (@name) - rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + @name = @relation.tags['name'].to_s + if @name.length == 0: + @name = "#" + @relation.id.to_s end + + @title = 'Relation History | ' + (@name) + rescue ActiveRecord::RecordNotFound + @type = "relation" + render :action => "not_found", :status => :not_found end def way - begin - @way = Way.find(params[:id]) + @way = Way.find(params[:id]) - @name = @way.tags['name'].to_s - if @name.length == 0: - @name = "#" + @way.id.to_s - end - - @title = 'Way | ' + (@name) - @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] ) - @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] ) - rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + @name = @way.tags['name'].to_s + if @name.length == 0: + @name = "#" + @way.id.to_s end + + @title = 'Way | ' + (@name) + @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] ) + @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] ) + rescue ActiveRecord::RecordNotFound + @type = "way" + render :action => "not_found", :status => :not_found end def way_history - begin - @way = Way.find(params[:id]) + @way = Way.find(params[:id]) - @name = @way.tags['name'].to_s - if @name.length == 0: - @name = "#" + @way.id.to_s - end - - @title = 'Way History | ' + (@name) - rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + @name = @way.tags['name'].to_s + if @name.length == 0: + @name = "#" + @way.id.to_s end + + @title = 'Way History | ' + (@name) + rescue ActiveRecord::RecordNotFound + @type = "way" + render :action => "not_found", :status => :not_found end def node - begin - @node = Node.find(params[:id]) + @node = Node.find(params[:id]) - @name = @node.tags_as_hash['name'].to_s - if @name.length == 0: - @name = "#" + @node.id.to_s - end - - @title = 'Node | ' + (@name) - @next = Node.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @node.id }] ) - @prev = Node.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @node.id }] ) - rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + @name = @node.tags_as_hash['name'].to_s + if @name.length == 0: + @name = "#" + @node.id.to_s end + + @title = 'Node | ' + (@name) + @next = Node.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @node.id }] ) + @prev = Node.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @node.id }] ) + rescue ActiveRecord::RecordNotFound + @type = "node" + render :action => "not_found", :status => :not_found end def node_history - begin - @node = Node.find(params[:id]) + @node = Node.find(params[:id]) - @name = @node.tags_as_hash['name'].to_s - if @name.length == 0: - @name = "#" + @node.id.to_s - end - - @title = 'Node History | ' + (@name) - rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + @name = @node.tags_as_hash['name'].to_s + if @name.length == 0: + @name = "#" + @node.id.to_s end + + @title = 'Node History | ' + (@name) + rescue ActiveRecord::RecordNotFound + @type = "way" + render :action => "not_found", :status => :not_found + end + + def changeset + @changeset = Changeset.find(params[:id]) + @node_pages, @nodes = paginate(:old_nodes, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'node_page') + @way_pages, @ways = paginate(:old_ways, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'way_page') + @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page') + + @title = "Changeset | #{@changeset.id}" + @next = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id", { :id => @changeset.id }] ) + @prev = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id", { :id => @changeset.id }] ) + rescue ActiveRecord::RecordNotFound + @type = "changeset" + render :action => "not_found", :status => :not_found end end diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb new file mode 100644 index 000000000..4913a600e --- /dev/null +++ b/app/controllers/changeset_controller.rb @@ -0,0 +1,485 @@ +# The ChangesetController is the RESTful interface to Changeset objects + +class ChangesetController < ApplicationController + layout 'site' + require 'xml/libxml' + + session :off, :except => [:list, :list_user, :list_bbox] + before_filter :authorize_web, :only => [:list, :list_user, :list_bbox] + before_filter :authorize, :only => [:create, :update, :delete, :upload, :include, :close] + before_filter :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close] + before_filter :check_api_writable, :only => [:create, :update, :delete, :upload, :include] + before_filter :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query] + after_filter :compress_output + + # Help methods for checking boundary sanity and area size + include MapBoundary + + # Helper methods for checking consistency + include ConsistencyValidations + + # Create a changeset from XML. + def create + if request.put? + cs = Changeset.from_xml(request.raw_post, true) + + if cs + cs.user_id = @user.id + cs.save_with_tags! + render :text => cs.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request + end + else + render :nothing => true, :status => :method_not_allowed + end + end + + ## + # Return XML giving the basic info about the changeset. Does not + # return anything about the nodes, ways and relations in the changeset. + def read + begin + changeset = Changeset.find(params[:id]) + render :text => changeset.to_xml.to_s, :content_type => "text/xml" + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + end + end + + ## + # marks a changeset as closed. this may be called multiple times + # on the same changeset, so is idempotent. + def close + unless request.put? + render :nothing => true, :status => :method_not_allowed + return + end + + changeset = Changeset.find(params[:id]) + check_changeset_consistency(changeset, @user) + + # to close the changeset, we'll just set its closed_at time to + # now. this might not be enough if there are concurrency issues, + # but we'll have to wait and see. + changeset.set_closed_time_now + + changeset.save! + render :nothing => true + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + ## + # insert a (set of) points into a changeset bounding box. this can only + # increase the size of the bounding box. this is a hint that clients can + # set either before uploading a large number of changes, or changes that + # the client (but not the server) knows will affect areas further away. + def expand_bbox + # only allow POST requests, because although this method is + # idempotent, there is no "document" to PUT really... + if request.post? + cs = Changeset.find(params[:id]) + check_changeset_consistency(cs, @user) + + # keep an array of lons and lats + lon = Array.new + lat = Array.new + + # the request is in pseudo-osm format... this is kind-of an + # abuse, maybe should change to some other format? + doc = XML::Parser.string(request.raw_post).parse + doc.find("//osm/node").each do |n| + lon << n['lon'].to_f * GeoRecord::SCALE + lat << n['lat'].to_f * GeoRecord::SCALE + end + + # add the existing bounding box to the lon-lat array + lon << cs.min_lon unless cs.min_lon.nil? + lat << cs.min_lat unless cs.min_lat.nil? + lon << cs.max_lon unless cs.max_lon.nil? + lat << cs.max_lat unless cs.max_lat.nil? + + # collapse the arrays to minimum and maximum + cs.min_lon, cs.min_lat, cs.max_lon, cs.max_lat = + lon.min, lat.min, lon.max, lat.max + + # save the larger bounding box and return the changeset, which + # will include the bigger bounding box. + cs.save! + render :text => cs.to_xml.to_s, :content_type => "text/xml" + + else + render :nothing => true, :status => :method_not_allowed + end + + rescue LibXML::XML::Error, ArgumentError => ex + raise OSM::APIBadXMLError.new("osm", xml, ex.message) + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + ## + # Upload a diff in a single transaction. + # + # This means that each change within the diff must succeed, i.e: that + # each version number mentioned is still current. Otherwise the entire + # transaction *must* be rolled back. + # + # Furthermore, each element in the diff can only reference the current + # changeset. + # + # Returns: a diffResult document, as described in + # http://wiki.openstreetmap.org/index.php/OSM_Protocol_Version_0.6 + def upload + # only allow POST requests, as the upload method is most definitely + # not idempotent, as several uploads with placeholder IDs will have + # different side-effects. + # see http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.2 + unless request.post? + render :nothing => true, :status => :method_not_allowed + return + end + + changeset = Changeset.find(params[:id]) + check_changeset_consistency(changeset, @user) + + diff_reader = DiffReader.new(request.raw_post, changeset) + Changeset.transaction do + result = diff_reader.commit + render :text => result.to_s, :content_type => "text/xml" + end + + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + ## + # download the changeset as an osmChange document. + # + # to make it easier to revert diffs it would be better if the osmChange + # format were reversible, i.e: contained both old and new versions of + # modified elements. but it doesn't at the moment... + # + # this method cannot order the database changes fully (i.e: timestamp and + # version number may be too coarse) so the resulting diff may not apply + # to a different database. however since changesets are not atomic this + # behaviour cannot be guaranteed anyway and is the result of a design + # choice. + def download + changeset = Changeset.find(params[:id]) + + # get all the elements in the changeset and stick them in a big array. + elements = [changeset.old_nodes, + changeset.old_ways, + changeset.old_relations].flatten + + # sort the elements by timestamp and version number, as this is the + # almost sensible ordering available. this would be much nicer if + # global (SVN-style) versioning were used - then that would be + # unambiguous. + elements.sort! do |a, b| + if (a.timestamp == b.timestamp) + a.version <=> b.version + else + a.timestamp <=> b.timestamp + end + end + + # create an osmChange document for the output + result = OSM::API.new.get_xml_doc + result.root.name = "osmChange" + + # generate an output element for each operation. note: we avoid looking + # at the history because it is simpler - but it would be more correct to + # check these assertions. + elements.each do |elt| + result.root << + if (elt.version == 1) + # first version, so it must be newly-created. + created = XML::Node.new "create" + created << elt.to_xml_node + else + # get the previous version from the element history + prev_elt = elt.class.find(:first, :conditions => + ['id = ? and version = ?', + elt.id, elt.version]) + unless elt.visible + # if the element isn't visible then it must have been deleted, so + # output the *previous* XML + deleted = XML::Node.new "delete" + deleted << prev_elt.to_xml_node + else + # must be a modify, for which we don't need the previous version + # yet... + modified = XML::Node.new "modify" + modified << elt.to_xml_node + end + end + end + + render :text => result.to_s, :content_type => "text/xml" + + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + ## + # query changesets by bounding box, time, user or open/closed status. + def query + # create the conditions that the user asked for. some or all of + # these may be nil. + conditions = conditions_bbox(params['bbox']) + conditions = cond_merge conditions, conditions_user(params['user']) + conditions = cond_merge conditions, conditions_time(params['time']) + conditions = cond_merge conditions, conditions_open(params['open']) + conditions = cond_merge conditions, conditions_closed(params['closed']) + + # create the results document + results = OSM::API.new.get_xml_doc + + # add all matching changesets to the XML results document + Changeset.find(:all, + :conditions => conditions, + :limit => 100, + :order => 'created_at desc').each do |cs| + results.root << cs.to_xml_node + end + + render :text => results.to_s, :content_type => "text/xml" + + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + ## + # updates a changeset's tags. none of the changeset's attributes are + # user-modifiable, so they will be ignored. + # + # changesets are not (yet?) versioned, so we don't have to deal with + # history tables here. changesets are locked to a single user, however. + # + # after succesful update, returns the XML of the changeset. + def update + # request *must* be a PUT. + unless request.put? + render :nothing => true, :status => :method_not_allowed + return + end + + changeset = Changeset.find(params[:id]) + new_changeset = Changeset.from_xml(request.raw_post) + + unless new_changeset.nil? + check_changeset_consistency(changeset, @user) + changeset.update_from(new_changeset, @user) + render :text => changeset.to_xml, :mime_type => "text/xml" + else + + render :nothing => true, :status => :bad_request + end + + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + + + ## + # list edits (open changesets) in reverse chronological order + def list + conditions = conditions_nonempty + + + # @changesets = Changeset.find(:all, :order => "closed_at DESC", :conditions => ['closed_at < ?', DateTime.now], :limit=> 20) + + + #@edit_pages, @edits = paginate(:changesets, + # :include => [:user, :changeset_tags], + # :conditions => conditions, + # :order => "changesets.created_at DESC", + # :per_page => 20) + # + + @edits = Changeset.find(:all, + :order => "changesets.created_at DESC", + :conditions => conditions, + :limit => 20) + + end + + ## + # list edits (changesets) belonging to a user + def list_user + user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true}) + + if user + @display_name = user.display_name + if not user.data_public? and @user != user + @edits = nil + render + else + conditions = cond_merge conditions, ['user_id = ?', user.id] + conditions = cond_merge conditions, conditions_nonempty + @edit_pages, @edits = paginate(:changesets, + :include => [:user, :changeset_tags], + :conditions => conditions, + :order => "changesets.created_at DESC", + :per_page => 20) + end + else + @not_found_user = params[:display_name] + render :template => 'user/no_such_user', :status => :not_found + end + end + + ## + # list changesets in a bbox + def list_bbox + # support 'bbox' param or alternatively 'minlon', 'minlat' etc + if params['bbox'] + bbox = params['bbox'] + elsif params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat'] + bbox = h(params['minlon']) + ',' + h(params['minlat']) + ',' + h(params['maxlon']) + ',' + h(params['maxlat']) + else + #TODO: fix bugs in location determination for history tab (and other tabs) then uncomment this redirect + #redirect_to :action => 'list' + end + + conditions = conditions_bbox(bbox); + conditions = cond_merge conditions, conditions_nonempty + + @edit_pages, @edits = paginate(:changesets, + :include => [:user, :changeset_tags], + :conditions => conditions, + :order => "changesets.created_at DESC", + :per_page => 20) + + @bbox = sanitise_boundaries(bbox.split(/,/)) unless bbox==nil + end + +private + #------------------------------------------------------------ + # utility functions below. + #------------------------------------------------------------ + + ## + # merge two conditions + def cond_merge(a, b) + if a and b + a_str = a.shift + b_str = b.shift + return [ a_str + " AND " + b_str ] + a + b + elsif a + return a + else b + return b + end + end + + ## + # if a bounding box was specified then parse it and do some sanity + # checks. this is mostly the same as the map call, but without the + # area restriction. + def conditions_bbox(bbox) + unless bbox.nil? + raise OSM::APIBadUserInput.new("Bounding box should be min_lon,min_lat,max_lon,max_lat") unless bbox.count(',') == 3 + bbox = sanitise_boundaries(bbox.split(/,/)) + raise OSM::APIBadUserInput.new("Minimum longitude should be less than maximum.") unless bbox[0] <= bbox[2] + raise OSM::APIBadUserInput.new("Minimum latitude should be less than maximum.") unless bbox[1] <= bbox[3] + return ['min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?', + bbox[2] * GeoRecord::SCALE, bbox[0] * GeoRecord::SCALE, bbox[3]* GeoRecord::SCALE, bbox[1] * GeoRecord::SCALE] + else + return nil + end + end + + ## + # restrict changesets to those by a particular user + def conditions_user(user) + unless user.nil? + # user input checking, we don't have any UIDs < 1 + raise OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1 + + u = User.find(user.to_i) + # should be able to get changesets of public users only, or + # our own changesets regardless of public-ness. + unless u.data_public? + # get optional user auth stuff so that users can see their own + # changesets if they're non-public + setup_user_auth + + raise OSM::APINotFoundError if @user.nil? or @user.id != u.id + end + return ['user_id = ?', u.id] + else + return nil + end + end + + ## + # restrict changes to those closed during a particular time period + def conditions_time(time) + unless time.nil? + # if there is a range, i.e: comma separated, then the first is + # low, second is high - same as with bounding boxes. + if time.count(',') == 1 + # check that we actually have 2 elements in the array + times = time.split(/,/) + raise OSM::APIBadUserInput.new("bad time range") if times.size != 2 + + from, to = times.collect { |t| DateTime.parse(t) } + return ['closed_at >= ? and created_at <= ?', from, to] + else + # if there is no comma, assume its a lower limit on time + return ['closed_at >= ?', DateTime.parse(time)] + end + else + return nil + end + # stupid DateTime seems to throw both of these for bad parsing, so + # we have to catch both and ensure the correct code path is taken. + rescue ArgumentError => ex + raise OSM::APIBadUserInput.new(ex.message.to_s) + rescue RuntimeError => ex + raise OSM::APIBadUserInput.new(ex.message.to_s) + end + + ## + # return changesets which are open (haven't been closed yet) + # we do this by seeing if the 'closed at' time is in the future. Also if we've + # hit the maximum number of changes then it counts as no longer open. + # if parameter 'open' is nill then open and closed changsets are returned + def conditions_open(open) + return open.nil? ? nil : ['closed_at >= ? and num_changes <= ?', + Time.now.getutc, Changeset::MAX_ELEMENTS] + end + + ## + # query changesets which are closed + # ('closed at' time has passed or changes limit is hit) + def conditions_closed(closed) + return closed.nil? ? nil : ['closed_at < ? and num_changes > ?', + Time.now.getutc, Changeset::MAX_ELEMENTS] + end + + ## + # eliminate empty changesets (where the bbox has not been set) + # this should be applied to all changeset list displays + def conditions_nonempty() + return ['min_lat IS NOT NULL'] + end + +end diff --git a/app/controllers/changeset_tag_controller.rb b/app/controllers/changeset_tag_controller.rb new file mode 100644 index 000000000..3e8db3fc2 --- /dev/null +++ b/app/controllers/changeset_tag_controller.rb @@ -0,0 +1,9 @@ +class ChangesetTagController < ApplicationController + layout 'site' + + def search + @tags = ChangesetTag.find(:all, :limit => 11, :conditions => ["match(v) against (?)", params[:query][:query].to_s] ) + end + + +end diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb index 52e2ab22b..3ee36af21 100644 --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@ -39,6 +39,8 @@ class DiaryEntryController < ApplicationController redirect_to :controller => 'diary_entry', :action => 'view', :id => params[:id] end end + rescue ActiveRecord::RecordNotFound + render :action => "no_such_entry", :status => :not_found end def comment @@ -55,7 +57,7 @@ class DiaryEntryController < ApplicationController def list if params[:display_name] - @this_user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1") + @this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true}) if @this_user @title = @this_user.display_name + "'s diary" @@ -71,7 +73,7 @@ class DiaryEntryController < ApplicationController else @title = "Users' diaries" @entry_pages, @entries = paginate(:diary_entries, :include => :user, - :conditions => "users.visible = 1", + :conditions => ["users.visible = ?", true], :order => 'created_at DESC', :per_page => 20) end @@ -79,13 +81,13 @@ class DiaryEntryController < ApplicationController def rss if params[:display_name] - user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1") + user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true}) if user @entries = DiaryEntry.find(:all, :conditions => ['user_id = ?', user.id], :order => 'created_at DESC', :limit => 20) @title = "OpenStreetMap diary entries for #{user.display_name}" @description = "Recent OpenStreetmap diary entries from #{user.display_name}" - @link = "http://www.openstreetmap.org/user/#{user.display_name}/diary" + @link = "http://#{SERVER_URL}/user/#{user.display_name}/diary" render :content_type => Mime::RSS else @@ -93,21 +95,22 @@ class DiaryEntryController < ApplicationController end else @entries = DiaryEntry.find(:all, :include => :user, - :conditions => "users.visible = 1", + :conditions => ["users.visible = ?", true], :order => 'created_at DESC', :limit => 20) @title = "OpenStreetMap diary entries" @description = "Recent diary entries from users of OpenStreetMap" - @link = "http://www.openstreetmap.org/diary" + @link = "http://#{SERVER_URL}/diary" render :content_type => Mime::RSS end end def view - user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1") + user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true}) if user @entry = DiaryEntry.find(:first, :conditions => ['user_id = ? AND id = ?', user.id, params[:id]]) + @title = "Users' diaries | #{params[:display_name]}" else @not_found_user = params[:display_name] diff --git a/app/controllers/export_controller.rb b/app/controllers/export_controller.rb index a773d4b72..ab25fcbc6 100644 --- a/app/controllers/export_controller.rb +++ b/app/controllers/export_controller.rb @@ -2,18 +2,24 @@ class ExportController < ApplicationController def start end + #When the user clicks 'Export' we redirect to a URL which generates the export download def finish bbox = BoundingBox.new(params[:minlon], params[:minlat], params[:maxlon], params[:maxlat]) format = params[:format] if format == "osm" + #redirect to API map get redirect_to "http://api.openstreetmap.org/api/#{API_VERSION}/map?bbox=#{bbox}" + elsif format == "mapnik" + #redirect to a special 'export' cgi script format = params[:mapnik_format] scale = params[:mapnik_scale] redirect_to "http://tile.openstreetmap.org/cgi-bin/export?bbox=#{bbox}&scale=#{scale}&format=#{format}" + elsif format == "osmarender" + #redirect to the t@h 'MapOf' service format = params[:osmarender_format] zoom = params[:osmarender_zoom].to_i width = bbox.slippy_width(zoom).to_i diff --git a/app/controllers/message_controller.rb b/app/controllers/message_controller.rb index d6e5d7fda..2f71107f4 100644 --- a/app/controllers/message_controller.rb +++ b/app/controllers/message_controller.rb @@ -6,13 +6,18 @@ class MessageController < ApplicationController before_filter :check_database_readable before_filter :check_database_writable, :only => [:new, :reply, :mark] + # Allow the user to write a new message to another user. This action also + # deals with the sending of that message to the other user when the user + # clicks send. + # The user_id param is the id of the user that the message is being sent to. def new @title = 'send message' + @to_user = User.find(params[:user_id]) if params[:message] @message = Message.new(params[:message]) - @message.to_user_id = params[:user_id] + @message.to_user_id = @to_user.id @message.from_user_id = @user.id - @message.sent_on = Time.now + @message.sent_on = Time.now.getutc if @message.save flash[:notice] = 'Message sent' @@ -22,27 +27,32 @@ class MessageController < ApplicationController else @title = params[:title] end + rescue ActiveRecord::RecordNotFound + render :action => 'no_such_user', :status => :not_found end + # Allow the user to reply to another message. def reply message = Message.find(params[:message_id], :conditions => ["to_user_id = ? or from_user_id = ?", @user.id, @user.id ]) @body = "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}" @title = "Re: #{message.title.sub(/^Re:\s*/, '')}" - @user_id = message.from_user_id + @to_user = User.find(message.from_user_id) render :action => 'new' rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + render :action => 'no_such_user', :status => :not_found end + # Show a message def read @title = 'read message' @message = Message.find(params[:message_id], :conditions => ["to_user_id = ? or from_user_id = ?", @user.id, @user.id ]) - @message.message_read = 1 if @message.to_user_id == @user.id + @message.message_read = true if @message.to_user_id == @user.id @message.save rescue ActiveRecord::RecordNotFound - render :nothing => true, :status => :not_found + render :action => 'no_such_user', :status => :not_found end + # Display the list of messages that have been sent to the user. def inbox @title = 'inbox' if @user and params[:display_name] == @user.display_name @@ -51,6 +61,7 @@ class MessageController < ApplicationController end end + # Display the list of messages that the user has sent to other users. def outbox @title = 'outbox' if @user and params[:display_name] == @user.display_name @@ -59,15 +70,16 @@ class MessageController < ApplicationController end end + # Set the message as being read or unread. def mark if params[:message_id] id = params[:message_id] message = Message.find_by_id(id) if params[:mark] == 'unread' - message_read = 0 + message_read = false mark_type = 'unread' else - message_read = 1 + message_read = true mark_type = 'read' end message.message_read = message_read @@ -76,5 +88,7 @@ class MessageController < ApplicationController redirect_to :controller => 'message', :action => 'inbox', :display_name => @user.display_name end end + rescue ActiveRecord::RecordNotFound + render :action => 'no_such_user', :status => :not_found end end diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 6ce18f566..6e96d31ca 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -5,26 +5,28 @@ class NodeController < ApplicationController session :off before_filter :authorize, :only => [:create, :update, :delete] + before_filter :require_public_data, :only => [:create, :update, :delete] before_filter :check_api_writable, :only => [:create, :update, :delete] before_filter :check_api_readable, :except => [:create, :update, :delete] after_filter :compress_output # Create a node from XML. def create - if request.put? - node = Node.from_xml(request.raw_post, true) - - if node - node.user_id = @user.id - node.visible = true - node.save_with_history! + begin + if request.put? + node = Node.from_xml(request.raw_post, true) - render :text => node.id.to_s, :content_type => "text/plain" + if node + node.create_with_history @user + render :text => node.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request + end else - render :nothing => true, :status => :bad_request + render :nothing => true, :status => :method_not_allowed end - else - render :nothing => true, :status => :method_not_allowed + rescue OSM::APIError => ex + render ex.render_opts end end @@ -32,7 +34,7 @@ class NodeController < ApplicationController def read begin node = Node.find(params[:id]) - if node.visible + if node.visible? response.headers['Last-Modified'] = node.timestamp.rfc822 render :text => node.to_xml.to_s, :content_type => "text/xml" else @@ -42,7 +44,7 @@ class NodeController < ApplicationController render :nothing => true, :status => :not_found end end - + # Update a node from given XML def update begin @@ -50,49 +52,40 @@ class NodeController < ApplicationController new_node = Node.from_xml(request.raw_post) if new_node and new_node.id == node.id - node.user_id = @user.id - node.latitude = new_node.latitude - node.longitude = new_node.longitude - node.tags = new_node.tags - node.visible = true - node.save_with_history! - - render :nothing => true + node.update_from(new_node, @user) + render :text => node.version.to_s, :content_type => "text/plain" else render :nothing => true, :status => :bad_request end + rescue OSM::APIError => ex + render ex.render_opts rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found end end - # Delete a node. Doesn't actually delete it, but retains its history in a wiki-like way. - # FIXME remove all the fricking SQL + # Delete a node. Doesn't actually delete it, but retains its history + # in a wiki-like way. We therefore treat it like an update, so the delete + # method returns the new version number. def delete begin node = Node.find(params[:id]) - - if node.visible - if WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", :conditions => [ "current_ways.visible = 1 AND current_way_nodes.node_id = ?", node.id ]) - render :text => "", :status => :precondition_failed - elsif RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='node' and member_id=?", params[:id]]) - render :text => "", :status => :precondition_failed - else - node.user_id = @user.id - node.visible = 0 - node.save_with_history! - - render :nothing => true - end + new_node = Node.from_xml(request.raw_post) + + if new_node and new_node.id == node.id + node.delete_with_history!(new_node, @user) + render :text => node.version.to_s, :content_type => "text/plain" else - render :text => "", :status => :gone + render :nothing => true, :status => :bad_request end rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts end end - # WTF does this do? + # Dump the details on many nodes whose ids are given in the "nodes" parameter. def nodes ids = params['nodes'].split(',').collect { |n| n.to_i } diff --git a/app/controllers/old_node_controller.rb b/app/controllers/old_node_controller.rb index 40f4093e3..0976a0c9a 100644 --- a/app/controllers/old_node_controller.rb +++ b/app/controllers/old_node_controller.rb @@ -22,4 +22,21 @@ class OldNodeController < ApplicationController render :nothing => true, :status => :internal_server_error end end + + def version + begin + old_node = OldNode.find(:first, :conditions => {:id => params[:id], :version => params[:version]} ) + + response.headers['Last-Modified'] = old_node.timestamp.rfc822 + + doc = OSM::API.new.get_xml_doc + doc.root << old_node.to_xml_node + + render :text => doc.to_s, :content_type => "text/xml" + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end + end end diff --git a/app/controllers/old_relation_controller.rb b/app/controllers/old_relation_controller.rb index 0b5aa89be..f8ebbd070 100644 --- a/app/controllers/old_relation_controller.rb +++ b/app/controllers/old_relation_controller.rb @@ -2,6 +2,7 @@ class OldRelationController < ApplicationController require 'xml/libxml' session :off + before_filter :check_api_readable after_filter :compress_output def history @@ -20,4 +21,21 @@ class OldRelationController < ApplicationController render :nothing => true, :status => :internal_server_error end end + + def version + begin + old_relation = OldRelation.find(:first, :conditions => {:id => params[:id], :version => params[:version]} ) + + response.headers['Last-Modified'] = old_relation.timestamp.rfc822 + + doc = OSM::API.new.get_xml_doc + doc.root << old_relation.to_xml_node + + render :text => doc.to_s, :content_type => "text/xml" + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internetal_service_error + end + end end diff --git a/app/controllers/old_way_controller.rb b/app/controllers/old_way_controller.rb index 3d913c0f7..a42496687 100644 --- a/app/controllers/old_way_controller.rb +++ b/app/controllers/old_way_controller.rb @@ -13,7 +13,7 @@ class OldWayController < ApplicationController way.old_ways.each do |old_way| doc.root << old_way.to_xml_node - end + end render :text => doc.to_s, :content_type => "text/xml" rescue ActiveRecord::RecordNotFound @@ -22,4 +22,21 @@ class OldWayController < ApplicationController render :nothing => true, :status => :internal_server_error end end + + def version + begin + old_way = OldWay.find(:first, :conditions => {:id => params[:id], :version => params[:version]} ) + + response.headers['Last-Modified'] = old_way.timestamp.rfc822 + + doc = OSM::API.new.get_xml_doc + doc.root << old_way.to_xml_node + + render :text => doc.to_s, :content_type => "text/xml" + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end + end end diff --git a/app/controllers/relation_controller.rb b/app/controllers/relation_controller.rb index 457249162..a249e42a0 100644 --- a/app/controllers/relation_controller.rb +++ b/app/controllers/relation_controller.rb @@ -3,28 +3,29 @@ class RelationController < ApplicationController session :off before_filter :authorize, :only => [:create, :update, :delete] + before_filter :require_public_data, :only => [:create, :update, :delete] before_filter :check_api_writable, :only => [:create, :update, :delete] before_filter :check_api_readable, :except => [:create, :update, :delete] after_filter :compress_output def create - if request.put? - relation = Relation.from_xml(request.raw_post, true) - - if relation - if !relation.preconditions_ok? - render :text => "", :status => :precondition_failed - else - relation.user_id = @user.id - relation.save_with_history! - - render :text => relation.id.to_s, :content_type => "text/plain" - end + begin + if request.put? + relation = Relation.from_xml(request.raw_post, true) + + # We assume that an exception has been thrown if there was an error + # generating the relation + #if relation + relation.create_with_history @user + render :text => relation.id.to_s, :content_type => "text/plain" + #else + # render :text => "Couldn't get turn the input into a relation.", :status => :bad_request + #end else - render :nothing => true, :status => :bad_request + render :nothing => true, :status => :method_not_allowed end - else - render :nothing => true, :status => :method_not_allowed + rescue OSM::APIError => ex + render ex.render_opts end end @@ -45,56 +46,38 @@ class RelationController < ApplicationController end def update + logger.debug request.raw_post begin relation = Relation.find(params[:id]) new_relation = Relation.from_xml(request.raw_post) if new_relation and new_relation.id == relation.id - if !new_relation.preconditions_ok? - render :text => "", :status => :precondition_failed - else - relation.user_id = @user.id - relation.tags = new_relation.tags - relation.members = new_relation.members - relation.visible = true - relation.save_with_history! - - render :nothing => true - end + relation.update_from new_relation, @user + render :text => relation.version.to_s, :content_type => "text/plain" else render :nothing => true, :status => :bad_request end rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found - rescue - render :nothing => true, :status => :internal_server_error + rescue OSM::APIError => ex + render ex.render_opts end end def delete -#XXX check if member somewhere! begin relation = Relation.find(params[:id]) - - if relation.visible - if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='relation' and member_id=?", params[:id]]) - render :text => "", :status => :precondition_failed - else - relation.user_id = @user.id - relation.tags = [] - relation.members = [] - relation.visible = false - relation.save_with_history! - - render :nothing => true - end + new_relation = Relation.from_xml(request.raw_post) + if new_relation and new_relation.id == relation.id + relation.delete_with_history!(new_relation, @user) + render :text => relation.version.to_s, :content_type => "text/plain" else - render :text => "", :status => :gone + render :nothing => true, :status => :bad_request end + rescue OSM::APIError => ex + render ex.render_opts rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found - rescue - render :nothing => true, :status => :internal_server_error end end @@ -115,12 +98,12 @@ class RelationController < ApplicationController # first collect nodes, ways, and relations referenced by this relation. ways = Way.find_by_sql("select w.* from current_ways w,current_relation_members rm where "+ - "rm.member_type='way' and rm.member_id=w.id and rm.id=#{relation.id}"); + "rm.member_type='Way' and rm.member_id=w.id and rm.id=#{relation.id}"); nodes = Node.find_by_sql("select n.* from current_nodes n,current_relation_members rm where "+ - "rm.member_type='node' and rm.member_id=n.id and rm.id=#{relation.id}"); + "rm.member_type='Node' and rm.member_id=n.id and rm.id=#{relation.id}"); # note query is built to exclude self just in case. relations = Relation.find_by_sql("select r.* from current_relations r,current_relation_members rm where "+ - "rm.member_type='relation' and rm.member_id=r.id and rm.id=#{relation.id} and r.id<>rm.id"); + "rm.member_type='Relation' and rm.member_id=r.id and rm.id=#{relation.id} and r.id<>rm.id"); # now additionally collect nodes referenced by ways. Note how we recursively # evaluate ways but NOT relations. @@ -160,8 +143,7 @@ class RelationController < ApplicationController render :text => doc.to_s, :content_type => "text/xml" else - - render :text => "", :status => :gone + render :nothing => true, :status => :gone end rescue ActiveRecord::RecordNotFound @@ -184,27 +166,29 @@ class RelationController < ApplicationController render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => :bad_request + render :text => "You need to supply a comma separated list of ids.", :status => :bad_request end + rescue ActiveRecord::RecordNotFound + render :text => "Could not find one of the relations", :status => :not_found end def relations_for_way - relations_for_object("way") + relations_for_object("Way") end def relations_for_node - relations_for_object("node") + relations_for_object("Node") end def relations_for_relation - relations_for_object("relation") + relations_for_object("Relation") end def relations_for_object(objtype) - relationids = RelationMember.find(:all, :conditions => ['member_type=? and member_id=?', objtype, params[:id]]).collect { |ws| ws.id }.uniq + relationids = RelationMember.find(:all, :conditions => ['member_type=? and member_id=?', objtype, params[:id]]).collect { |ws| ws.id[0] }.uniq doc = OSM::API.new.get_xml_doc Relation.find(relationids).each do |relation| - doc.root << relation.to_xml_node + doc.root << relation.to_xml_node if relation.visible end render :text => doc.to_s, :content_type => "text/xml" diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index 3942cb7fe..dc7456c45 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -1,8 +1,8 @@ class TraceController < ApplicationController layout 'site' - before_filter :authorize_web - before_filter :require_user, :only => [:mine, :edit, :delete, :make_public] + before_filter :authorize_web + before_filter :require_user, :only => [:mine, :create, :edit, :delete, :make_public] before_filter :authorize, :only => [:api_details, :api_data, :api_create] before_filter :check_database_readable, :except => [:api_details, :api_data, :api_create] before_filter :check_database_writable, :only => [:create, :edit, :delete, :make_public] @@ -15,7 +15,12 @@ class TraceController < ApplicationController # from display name, pick up user id if one user's traces only display_name = params[:display_name] if target_user.nil? and !display_name.blank? - target_user = User.find(:first, :conditions => [ "visible = 1 and display_name = ?", display_name]) + target_user = User.find(:first, :conditions => [ "visible = ? and display_name = ?", true, display_name]) + if target_user.nil? + @not_found_user = display_name + render :action => 'no_such_user', :status => :not_found + return + end end # set title @@ -36,15 +41,15 @@ class TraceController < ApplicationController # 4 - user's traces, not logged in as that user = all user's public traces if target_user.nil? # all traces if @user - conditions = ["(gpx_files.public = 1 OR gpx_files.user_id = ?)", @user.id] #1 + conditions = ["(gpx_files.public = ? OR gpx_files.user_id = ?)", true, @user.id] #1 else - conditions = ["gpx_files.public = 1"] #2 + conditions = ["gpx_files.public = ?", true] #2 end else if @user and @user == target_user conditions = ["gpx_files.user_id = ?", @user.id] #3 (check vs user id, so no join + can't pick up non-public traces by changing name) else - conditions = ["gpx_files.public = 1 AND gpx_files.user_id = ?", target_user.id] #4 + conditions = ["gpx_files.public = ? AND gpx_files.user_id = ?", true, target_user.id] #4 end end @@ -55,7 +60,8 @@ class TraceController < ApplicationController conditions[0] += " AND gpx_files.id IN (#{files.join(',')})" end - conditions[0] += " AND gpx_files.visible = 1" + conditions[0] += " AND gpx_files.visible = ?" + conditions << true @trace_pages, @traces = paginate(:traces, :include => [:user, :tags], @@ -100,26 +106,28 @@ class TraceController < ApplicationController end def create - logger.info(params[:trace][:gpx_file].class.name) - if params[:trace][:gpx_file].respond_to?(:read) - do_create(params[:trace][:gpx_file], params[:trace][:tagstring], - params[:trace][:description], params[:trace][:public]) + if params[:trace] + logger.info(params[:trace][:gpx_file].class.name) + if params[:trace][:gpx_file].respond_to?(:read) + do_create(params[:trace][:gpx_file], params[:trace][:tagstring], + params[:trace][:description], params[:trace][:public]) - if @trace.id - logger.info("id is #{@trace.id}") - flash[:notice] = "Your GPX file has been uploaded and is awaiting insertion in to the database. This will usually happen within half an hour, and an email will be sent to you on completion." + if @trace.id + logger.info("id is #{@trace.id}") + flash[:notice] = "Your GPX file has been uploaded and is awaiting insertion in to the database. This will usually happen within half an hour, and an email will be sent to you on completion." - redirect_to :action => 'mine' + redirect_to :action => 'mine' + end + else + @trace = Trace.new({:name => "Dummy", + :tagstring => params[:trace][:tagstring], + :description => params[:trace][:description], + :public => params[:trace][:public], + :inserted => false, :user => @user, + :timestamp => Time.now.getutc}) + @trace.valid? + @trace.errors.add(:gpx_file, "can't be blank") end - else - @trace = Trace.new({:name => "Dummy", - :tagstring => params[:trace][:tagstring], - :description => params[:trace][:description], - :public => params[:trace][:public], - :inserted => false, :user => @user, - :timestamp => Time.now}) - @trace.valid? - @trace.errors.add(:gpx_file, "can't be blank") end end @@ -196,7 +204,7 @@ class TraceController < ApplicationController end def georss - conditions = ["gpx_files.public = 1"] + conditions = ["gpx_files.public = ?", true] if params[:display_name] conditions[0] += " AND users.display_name = ?" @@ -278,12 +286,20 @@ class TraceController < ApplicationController def api_create if request.post? - do_create(params[:file], params[:tags], params[:description], params[:public]) - - if @trace.id - render :text => @trace.id.to_s, :content_type => "text/plain" - elsif @trace.valid? - render :nothing => true, :status => :internal_server_error + tags = params[:tags] || "" + description = params[:description] || "" + pub = params[:public] || false + + if params[:file].respond_to?(:read) + do_create(params[:file], tags, description, pub) + + if @trace.id + render :text => @trace.id.to_s, :content_type => "text/plain" + elsif @trace.valid? + render :nothing => true, :status => :internal_server_error + else + render :nothing => true, :status => :bad_request + end else render :nothing => true, :status => :bad_request end @@ -313,7 +329,7 @@ private :public => public, :inserted => true, :user => @user, - :timestamp => Time.now + :timestamp => Time.now.getutc }) # Save the trace object @@ -328,6 +344,17 @@ private # Remove the file as we have failed to update the database FileUtils.rm_f(filename) end + + # Finally save whether the user marked the trace as being public + if @trace.public? + if @user.trace_public_default.nil? + @user.preferences.create(:k => "gps.trace.public", :v => "default") + end + else + pref = @user.trace_public_default + pref.destroy unless pref.nil? + end + end end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 1cd34900a..9544dd8a8 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -83,7 +83,7 @@ class UserController < ApplicationController def lost_password @title = 'lost password' if params[:user] and params[:user][:email] - user = User.find_by_email(params[:user][:email], :conditions => "visible = 1") + user = User.find_by_email(params[:user][:email], :conditions => {:visible => true}) if user token = user.tokens.create @@ -120,9 +120,21 @@ class UserController < ApplicationController def new @title = 'create account' + # The user is logged in already, so don't show them the signup page, instead + # send them to the home page + redirect_to :controller => 'site', :action => 'index' if session[:user] end def login + if session[:user] + # The user is logged in already, if the referer param exists, redirect them to that + if params[:referer] + redirect_to params[:referer] + else + redirect_to :controller => 'site', :action => 'index' + end + return + end @title = 'login' if params[:user] email_or_display_name = params[:user][:email] @@ -223,7 +235,7 @@ class UserController < ApplicationController end def view - @this_user = User.find_by_display_name(params[:display_name], :conditions => "visible = 1") + @this_user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true}) if @this_user @title = @this_user.display_name @@ -236,7 +248,7 @@ class UserController < ApplicationController def make_friend if params[:display_name] name = params[:display_name] - new_friend = User.find_by_display_name(name, :conditions => "visible = 1") + new_friend = User.find_by_display_name(name, :conditions => {:visible => true}) friend = Friend.new friend.user_id = @user.id friend.friend_user_id = new_friend.id @@ -258,7 +270,7 @@ class UserController < ApplicationController def remove_friend if params[:display_name] name = params[:display_name] - friend = User.find_by_display_name(name, :conditions => "visible = 1") + friend = User.find_by_display_name(name, :conditions => {:visible => true}) if @user.is_friends_with?(friend) Friend.delete_all "user_id = #{@user.id} AND friend_user_id = #{friend.id}" flash[:notice] = "#{friend.display_name} was removed from your friends." diff --git a/app/controllers/user_preference_controller.rb b/app/controllers/user_preference_controller.rb index 3a48ee65e..59573047a 100644 --- a/app/controllers/user_preference_controller.rb +++ b/app/controllers/user_preference_controller.rb @@ -5,11 +5,9 @@ class UserPreferenceController < ApplicationController def read_one pref = UserPreference.find(@user.id, params[:preference_key]) - if pref - render :text => pref.v.to_s - else - render :text => 'OH NOES! PREF NOT FOUND!', :status => 404 - end + render :text => pref.v.to_s + rescue ActiveRecord::RecordNotFound => ex + render :text => 'OH NOES! PREF NOT FOUND!', :status => :not_found end def update_one @@ -32,6 +30,8 @@ class UserPreferenceController < ApplicationController UserPreference.delete(@user.id, params[:preference_key]) render :nothing => true + rescue ActiveRecord::RecordNotFound => ex + render :text => "param: #{params[:preference_key]} not found", :status => :not_found end # print out all the preferences as a big xml block @@ -54,46 +54,44 @@ class UserPreferenceController < ApplicationController def update begin p = XML::Parser.string(request.raw_post) - doc = p.parse - - prefs = [] - - keyhash = {} - - doc.find('//preferences/preference').each do |pt| - pref = UserPreference.new + rescue LibXML::XML::Error, ArgumentError => ex + raise OSM::APIBadXMLError.new("preferences", xml, ex.message) + end + doc = p.parse - unless keyhash[pt['k']].nil? # already have that key - render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable - return - end + prefs = [] - keyhash[pt['k']] = 1 + keyhash = {} - pref.k = pt['k'] - pref.v = pt['v'] - pref.user_id = @user.id - prefs << pref - end + doc.find('//preferences/preference').each do |pt| + pref = UserPreference.new - if prefs.size > 150 - render :text => 'Too many preferences', :status => :request_entity_too_large - return + unless keyhash[pt['k']].nil? # already have that key + render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable end - # kill the existing ones - UserPreference.delete_all(['user_id = ?', @user.id]) + keyhash[pt['k']] = 1 - # save the new ones - prefs.each do |pref| - pref.save! - end + pref.k = pt['k'] + pref.v = pt['v'] + pref.user_id = @user.id + prefs << pref + end - rescue Exception => ex - render :text => 'OH NOES! FAIL!: ' + ex.to_s, :status => :internal_server_error - return + if prefs.size > 150 + render :text => 'Too many preferences', :status => :request_entity_too_large end + # kill the existing ones + UserPreference.delete_all(['user_id = ?', @user.id]) + + # save the new ones + prefs.each do |pref| + pref.save! + end render :nothing => true + + rescue Exception => ex + render :text => 'OH NOES! FAIL!: ' + ex.to_s, :status => :internal_server_error end end diff --git a/app/controllers/way_controller.rb b/app/controllers/way_controller.rb index 9b93ec2b3..ab83d4ec3 100644 --- a/app/controllers/way_controller.rb +++ b/app/controllers/way_controller.rb @@ -3,28 +3,28 @@ class WayController < ApplicationController session :off before_filter :authorize, :only => [:create, :update, :delete] + before_filter :require_public_data, :only => [:create, :update, :delete] before_filter :check_api_writable, :only => [:create, :update, :delete] before_filter :check_api_readable, :except => [:create, :update, :delete] after_filter :compress_output def create - if request.put? - way = Way.from_xml(request.raw_post, true) - - if way - if !way.preconditions_ok? - render :text => "", :status => :precondition_failed - else - way.user_id = @user.id - way.save_with_history! + begin + if request.put? + way = Way.from_xml(request.raw_post, true) + if way + way.create_with_history @user render :text => way.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request end else - render :nothing => true, :status => :bad_request + render :nothing => true, :status => :method_not_allowed end - else - render :nothing => true, :status => :method_not_allowed + rescue OSM::APIError => ex + logger.warn request.raw_post + render ex.render_opts end end @@ -39,6 +39,8 @@ class WayController < ApplicationController else render :text => "", :status => :gone end + rescue OSM::APIError => ex + render ex.render_opts rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found end @@ -50,20 +52,14 @@ class WayController < ApplicationController new_way = Way.from_xml(request.raw_post) if new_way and new_way.id == way.id - if !new_way.preconditions_ok? - render :text => "", :status => :precondition_failed - else - way.user_id = @user.id - way.tags = new_way.tags - way.nds = new_way.nds - way.visible = true - way.save_with_history! - - render :nothing => true - end + way.update_from(new_way, @user) + render :text => way.version.to_s, :content_type => "text/plain" else render :nothing => true, :status => :bad_request end + rescue OSM::APIError => ex + logger.warn request.raw_post + render ex.render_opts rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found end @@ -73,14 +69,16 @@ class WayController < ApplicationController def delete begin way = Way.find(params[:id]) - way.delete_with_relations_and_history(@user) - - # if we get here, all is fine, otherwise something will catch below. - render :nothing => true - rescue OSM::APIAlreadyDeletedError - render :text => "", :status => :gone - rescue OSM::APIPreconditionFailedError - render :text => "", :status => :precondition_failed + new_way = Way.from_xml(request.raw_post) + + if new_way and new_way.id == way.id + way.delete_with_history!(new_way, @user) + render :text => way.version.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request + end + rescue OSM::APIError => ex + render ex.render_opts rescue ActiveRecord::RecordNotFound render :nothing => true, :status => :not_found end @@ -92,7 +90,7 @@ class WayController < ApplicationController if way.visible nd_ids = way.nds + [-1] - nodes = Node.find(:all, :conditions => "visible = 1 AND id IN (#{nd_ids.join(',')})") + nodes = Node.find(:all, :conditions => ["visible = ? AND id IN (#{nd_ids.join(',')})", true]) # Render doc = OSM::API.new.get_xml_doc @@ -130,13 +128,19 @@ class WayController < ApplicationController end end + ## + # returns all the ways which are currently using the node given in the + # :id parameter. note that this used to return deleted ways as well, but + # this seemed not to be the expected behaviour, so it was removed. def ways_for_node - wayids = WayNode.find(:all, :conditions => ['node_id = ?', params[:id]]).collect { |ws| ws.id[0] }.uniq + wayids = WayNode.find(:all, + :conditions => ['node_id = ?', params[:id]] + ).collect { |ws| ws.id[0] }.uniq doc = OSM::API.new.get_xml_doc Way.find(wayids).each do |way| - doc.root << way.to_xml_node + doc.root << way.to_xml_node if way.visible end render :text => doc.to_s, :content_type => "text/xml" diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index c86ad5b71..34302a8af 100644 --- a/app/helpers/browse_helper.rb +++ b/app/helpers/browse_helper.rb @@ -1,2 +1,5 @@ module BrowseHelper + def link_to_page(page, page_param) + return link_to(page, page_param => page) + end end diff --git a/app/models/acl.rb b/app/models/acl.rb index 5fb99b9e5..3ff19d35f 100644 --- a/app/models/acl.rb +++ b/app/models/acl.rb @@ -1,13 +1,23 @@ class Acl < ActiveRecord::Base def self.find_by_address(address, options) - self.with_scope(:find => {:conditions => ["inet_aton(?) & netmask = address", address]}) do + self.with_scope(:find => {:conditions => ["#{inet_aton} & netmask = address", address]}) do return self.find(:first, options) end end def self.find_all_by_address(address, options) - self.with_scope(:find => {:conditions => ["inet_aton(?) & netmask = address", address]}) do + self.with_scope(:find => {:conditions => ["#{inet_aton} & netmask = address", address]}) do return self.find(:all, options) end end + +private + + def self.inet_aton + if self.connection.adapter_name == "MySQL" + "inet_aton(?)" + else + "?" + end + end end diff --git a/app/models/changeset.rb b/app/models/changeset.rb new file mode 100644 index 000000000..fa2d556b1 --- /dev/null +++ b/app/models/changeset.rb @@ -0,0 +1,241 @@ +class Changeset < ActiveRecord::Base + require 'xml/libxml' + + belongs_to :user + + has_many :changeset_tags, :foreign_key => 'id' + + has_many :nodes + has_many :ways + has_many :relations + has_many :old_nodes + has_many :old_ways + has_many :old_relations + + validates_presence_of :id, :on => :update + validates_presence_of :user_id, :created_at, :closed_at, :num_changes + validates_uniqueness_of :id + validates_numericality_of :id, :on => :update, :integer_only => true + validates_numericality_of :min_lat, :max_lat, :min_lon, :max_lat, :allow_nil => true, :integer_only => true + validates_numericality_of :user_id, :integer_only => true + validates_numericality_of :num_changes, :integer_only => true, :greater_than_or_equal_to => 0 + validates_associated :user + + # over-expansion factor to use when updating the bounding box + EXPAND = 0.1 + + # maximum number of elements allowed in a changeset + MAX_ELEMENTS = 50000 + + # maximum time a changeset is allowed to be open for. + MAX_TIME_OPEN = 1.day + + # idle timeout increment, one hour seems reasonable. + IDLE_TIMEOUT = 1.hour + + # Use a method like this, so that we can easily change how we + # determine whether a changeset is open, without breaking code in at + # least 6 controllers + def is_open? + # a changeset is open (that is, it will accept further changes) when + # it has not yet run out of time and its capacity is small enough. + # note that this may not be a hard limit - due to timing changes and + # concurrency it is possible that some changesets may be slightly + # longer than strictly allowed or have slightly more changes in them. + return ((closed_at > Time.now.getutc) and (num_changes <= MAX_ELEMENTS)) + end + + def set_closed_time_now + if is_open? + self.closed_at = Time.now.getutc + end + end + + def self.from_xml(xml, create=false) + begin + p = XML::Parser.string(xml) + doc = p.parse + + cs = Changeset.new + + doc.find('//osm/changeset').each do |pt| + if create + cs.created_at = Time.now.getutc + # initial close time is 1h ahead, but will be increased on each + # modification. + cs.closed_at = cs.created_at + IDLE_TIMEOUT + # initially we have no changes in a changeset + cs.num_changes = 0 + end + + pt.find('tag').each do |tag| + cs.add_tag_keyval(tag['k'], tag['v']) + end + end + rescue Exception => ex + cs = nil + end + + return cs + end + + ## + # returns the bounding box of the changeset. it is possible that some + # or all of the values will be nil, indicating that they are undefined. + def bbox + @bbox ||= [ min_lon, min_lat, max_lon, max_lat ] + end + + def has_valid_bbox? + not bbox.include? nil + end + + ## + # expand the bounding box to include the given bounding box. also, + # expand a little bit more in the direction of the expansion, so that + # further expansions may be unnecessary. this is an optimisation + # suggested on the wiki page by kleptog. + def update_bbox!(array) + # ensure that bbox is cached and has no nils in it. if there are any + # nils, just use the bounding box update to write over them. + @bbox = bbox.zip(array).collect { |a, b| a.nil? ? b : a } + + # FIXME - this looks nasty and violates DRY... is there any prettier + # way to do this? + @bbox[0] = array[0] + EXPAND * (@bbox[0] - @bbox[2]) if array[0] < @bbox[0] + @bbox[1] = array[1] + EXPAND * (@bbox[1] - @bbox[3]) if array[1] < @bbox[1] + @bbox[2] = array[2] + EXPAND * (@bbox[2] - @bbox[0]) if array[2] > @bbox[2] + @bbox[3] = array[3] + EXPAND * (@bbox[3] - @bbox[1]) if array[3] > @bbox[3] + + # update active record. rails 2.1's dirty handling should take care of + # whether this object needs saving or not. + self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox + end + + ## + # the number of elements is also passed in so that we can ensure that + # a single changeset doesn't contain too many elements. this, of course, + # destroys the optimisation described in the bbox method above. + def add_changes!(elements) + self.num_changes += elements + end + + def tags_as_hash + return tags + end + + def tags + unless @tags + @tags = {} + self.changeset_tags.each do |tag| + @tags[tag.k] = tag.v + end + end + @tags + end + + def tags=(t) + @tags = t + end + + def add_tag_keyval(k, v) + @tags = Hash.new unless @tags + @tags[k] = v + end + + def save_with_tags! + t = Time.now.getutc + + # do the changeset update and the changeset tags update in the + # same transaction to ensure consistency. + Changeset.transaction do + # set the auto-close time to be one hour in the future unless + # that would make it more than 24h long, in which case clip to + # 24h, as this has been decided is a reasonable time limit. + if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT) + self.closed_at = created_at + MAX_TIME_OPEN + else + self.closed_at = Time.now.getutc + IDLE_TIMEOUT + end + self.save! + + tags = self.tags + ChangesetTag.delete_all(['id = ?', self.id]) + + tags.each do |k,v| + tag = ChangesetTag.new + tag.k = k + tag.v = v + tag.id = self.id + tag.save! + end + end + end + + def to_xml + doc = OSM::API.new.get_xml_doc + doc.root << to_xml_node() + return doc + end + + def to_xml_node(user_display_name_cache = nil) + el1 = XML::Node.new 'changeset' + el1['id'] = self.id.to_s + + user_display_name_cache = {} if user_display_name_cache.nil? + + if user_display_name_cache and user_display_name_cache.key?(self.user_id) + # use the cache if available + elsif self.user.data_public? + user_display_name_cache[self.user_id] = self.user.display_name + else + user_display_name_cache[self.user_id] = nil + end + + el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil? + el1['uid'] = self.user_id.to_s if self.user.data_public? + + self.tags.each do |k,v| + el2 = XML::Node.new('tag') + el2['k'] = k.to_s + el2['v'] = v.to_s + el1 << el2 + end + + el1['created_at'] = self.created_at.xmlschema + el1['closed_at'] = self.closed_at.xmlschema unless is_open? + el1['open'] = is_open?.to_s + + el1['min_lon'] = (bbox[0].to_f / GeoRecord::SCALE).to_s unless bbox[0].nil? + el1['min_lat'] = (bbox[1].to_f / GeoRecord::SCALE).to_s unless bbox[1].nil? + el1['max_lon'] = (bbox[2].to_f / GeoRecord::SCALE).to_s unless bbox[2].nil? + el1['max_lat'] = (bbox[3].to_f / GeoRecord::SCALE).to_s unless bbox[3].nil? + + # NOTE: changesets don't include the XML of the changes within them, + # they are just structures for tagging. to get the osmChange of a + # changeset, see the download method of the controller. + + return el1 + end + + ## + # update this instance from another instance given and the user who is + # doing the updating. note that this method is not for updating the + # bounding box, only the tags of the changeset. + def update_from(other, user) + # ensure that only the user who opened the changeset may modify it. + unless user.id == self.user_id + raise OSM::APIUserChangesetMismatchError.new + end + + # can't change a closed changeset + unless is_open? + raise OSM::APIChangesetAlreadyClosedError.new(self) + end + + # copy the other's tags + self.tags = other.tags + + save_with_tags! + end +end diff --git a/app/models/changeset_tag.rb b/app/models/changeset_tag.rb new file mode 100644 index 000000000..6a414a0fc --- /dev/null +++ b/app/models/changeset_tag.rb @@ -0,0 +1,8 @@ +class ChangesetTag < ActiveRecord::Base + belongs_to :changeset, :foreign_key => 'id' + + validates_presence_of :id + validates_length_of :k, :v, :maximum => 255, :allow_blank => true + validates_uniqueness_of :id, :scope => :k + validates_numericality_of :id, :only_integer => true +end diff --git a/app/models/diary_entry.rb b/app/models/diary_entry.rb index dd1f9882a..46d96fec7 100644 --- a/app/models/diary_entry.rb +++ b/app/models/diary_entry.rb @@ -1,11 +1,15 @@ class DiaryEntry < ActiveRecord::Base belongs_to :user has_many :diary_comments, :include => :user, - :conditions => "users.visible = 1", + :conditions => ["users.visible = ?", true], :order => "diary_comments.id" validates_presence_of :title, :body - validates_numericality_of :latitude, :allow_nil => true - validates_numericality_of :longitude, :allow_nil => true + validates_length_of :title, :within => 1..255 + validates_length_of :language, :within => 2..3, :allow_nil => true + validates_numericality_of :latitude, :allow_nil => true, + :greater_than_or_equal_to => -90, :less_than_or_equal_to => 90 + validates_numericality_of :longitude, :allow_nil => true, + :greater_than_or_equal_to => -180, :less_than_or_equal_to => 180 validates_associated :user end diff --git a/app/models/message.rb b/app/models/message.rb index 97e411192..464c55028 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -1,8 +1,12 @@ +require 'validators' + class Message < ActiveRecord::Base belongs_to :sender, :class_name => "User", :foreign_key => :from_user_id belongs_to :recipient, :class_name => "User", :foreign_key => :to_user_id - validates_presence_of :title, :body, :sent_on + validates_presence_of :title, :body, :sent_on, :sender, :recipient + validates_length_of :title, :within => 1..255 validates_inclusion_of :message_read, :in => [ true, false ] validates_associated :sender, :recipient + validates_as_utf8 :title end diff --git a/app/models/node.rb b/app/models/node.rb index af88a117d..1392fc650 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -2,27 +2,34 @@ class Node < ActiveRecord::Base require 'xml/libxml' include GeoRecord + include ConsistencyValidations set_table_name 'current_nodes' - - validates_presence_of :user_id, :timestamp - validates_inclusion_of :visible, :in => [ true, false ] - validates_numericality_of :latitude, :longitude - validate :validate_position - belongs_to :user + belongs_to :changeset has_many :old_nodes, :foreign_key => :id has_many :way_nodes has_many :ways, :through => :way_nodes + has_many :node_tags, :foreign_key => :id + has_many :old_way_nodes has_many :ways_via_history, :class_name=> "Way", :through => :old_way_nodes, :source => :way has_many :containing_relation_members, :class_name => "RelationMember", :as => :member has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder + validates_presence_of :id, :on => :update + validates_presence_of :timestamp,:version, :changeset_id + validates_uniqueness_of :id + validates_inclusion_of :visible, :in => [ true, false ] + validates_numericality_of :latitude, :longitude, :changeset_id, :version, :integer_only => true + validates_numericality_of :id, :on => :update, :integer_only => true + validate :validate_position + validates_associated :changeset + # Sanity check the latitude and longitude and add an error if it's broken def validate_position errors.add_to_base("Node is not in the world") unless in_world? @@ -50,7 +57,7 @@ class Node < ActiveRecord::Base #conditions = keys.join(' AND ') find_by_area(min_lat, min_lon, max_lat, max_lon, - :conditions => 'visible = 1', + :conditions => {:visible => true}, :limit => APP_CONFIG['max_number_of_nodes']+1) end @@ -59,83 +66,150 @@ class Node < ActiveRecord::Base begin p = XML::Parser.string(xml) doc = p.parse - - node = Node.new doc.find('//osm/node').each do |pt| - node.lat = pt['lat'].to_f - node.lon = pt['lon'].to_f + return Node.from_xml_node(pt, create) + end + rescue LibXML::XML::Error, ArgumentError => ex + raise OSM::APIBadXMLError.new("node", xml, ex.message) + end + end + + def self.from_xml_node(pt, create=false) + node = Node.new + + raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt['lat'].nil? + raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt['lon'].nil? + node.lat = pt['lat'].to_f + node.lon = pt['lon'].to_f + raise OSM::APIBadXMLError.new("node", pt, "changeset id missing") if pt['changeset'].nil? + node.changeset_id = pt['changeset'].to_i - return nil unless node.in_world? + raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world? - unless create - if pt['id'] != '0' - node.id = pt['id'].to_i - end - end + # version must be present unless creating + raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create or not pt['version'].nil? + node.version = create ? 0 : pt['version'].to_i - node.visible = pt['visible'] and pt['visible'] == 'true' + unless create + if pt['id'] != '0' + node.id = pt['id'].to_i + end + end - if create - node.timestamp = Time.now - else - if pt['timestamp'] - node.timestamp = Time.parse(pt['timestamp']) - end - end + # visible if it says it is, or as the default if the attribute + # is missing. + # Don't need to set the visibility, when it is set explicitly in the create/update/delete + #node.visible = pt['visible'].nil? or pt['visible'] == 'true' - tags = [] + # We don't care about the time, as it is explicitly set on create/update/delete - pt.find('tag').each do |tag| - tags << [tag['k'],tag['v']] - end + tags = [] - node.tags = Tags.join(tags) - end - rescue - node = nil + pt.find('tag').each do |tag| + node.add_tag_key_val(tag['k'],tag['v']) end return node end - # Save this node with the appropriate OldNode object to represent it's history. - def save_with_history! + ## + # the bounding box around a node, which is used for determining the changeset's + # bounding box + def bbox + [ longitude, latitude, longitude, latitude ] + end + + # Should probably be renamed delete_from to come in line with update + def delete_with_history!(new_node, user) + unless self.visible + raise OSM::APIAlreadyDeletedError.new + end + + # need to start the transaction here, so that the database can + # provide repeatable reads for the used-by checks. this means it + # shouldn't be possible to get race conditions. Node.transaction do - self.timestamp = Time.now - self.save! - old_node = OldNode.from_node(self) - old_node.save! + check_consistency(self, new_node, user) + if WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", :conditions => [ "current_ways.visible = ? AND current_way_nodes.node_id = ?", true, self.id ]) + raise OSM::APIPreconditionFailedError.new + elsif RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = ? AND member_type='Node' and member_id=? ", true, self.id]) + raise OSM::APIPreconditionFailedError.new + else + self.changeset_id = new_node.changeset_id + self.visible = false + + # update the changeset with the deleted position + changeset.update_bbox!(bbox) + + save_with_history! + end end end - # Turn this Node in to a complete OSM XML object with wrapper + def update_from(new_node, user) + check_consistency(self, new_node, user) + + # update changeset first + self.changeset_id = new_node.changeset_id + self.changeset = new_node.changeset + + # update changeset bbox with *old* position first + changeset.update_bbox!(bbox); + + # FIXME logic needs to be double checked + self.latitude = new_node.latitude + self.longitude = new_node.longitude + self.tags = new_node.tags + self.visible = true + + # update changeset bbox with *new* position + changeset.update_bbox!(bbox); + + save_with_history! + end + + def create_with_history(user) + check_create_consistency(self, user) + self.version = 0 + self.visible = true + + # update the changeset to include the new location + changeset.update_bbox!(bbox) + + save_with_history! + end + def to_xml doc = OSM::API.new.get_xml_doc doc.root << to_xml_node() return doc end - # Turn this Node in to an XML Node without the wrapper. def to_xml_node(user_display_name_cache = nil) el1 = XML::Node.new 'node' el1['id'] = self.id.to_s el1['lat'] = self.lat.to_s el1['lon'] = self.lon.to_s - + el1['version'] = self.version.to_s + el1['changeset'] = self.changeset_id.to_s + user_display_name_cache = {} if user_display_name_cache.nil? - if user_display_name_cache and user_display_name_cache.key?(self.user_id) + if user_display_name_cache and user_display_name_cache.key?(self.changeset.user_id) # use the cache if available - elsif self.user.data_public? - user_display_name_cache[self.user_id] = self.user.display_name + elsif self.changeset.user.data_public? + user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name else - user_display_name_cache[self.user_id] = nil + user_display_name_cache[self.changeset.user_id] = nil end - el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil? + if not user_display_name_cache[self.changeset.user_id].nil? + el1['user'] = user_display_name_cache[self.changeset.user_id] + el1['uid'] = self.changeset.user_id.to_s + end - Tags.split(self.tags) do |k,v| + self.tags.each do |k,v| el2 = XML::Node.new('tag') el2['k'] = k.to_s el2['v'] = v.to_s @@ -147,12 +221,79 @@ class Node < ActiveRecord::Base return el1 end - # Return the node's tags as a Hash of keys and their values def tags_as_hash - hash = {} - Tags.split(self.tags) do |k,v| - hash[k] = v + return tags + end + + def tags + unless @tags + @tags = {} + self.node_tags.each do |tag| + @tags[tag.k] = tag.v + end + end + @tags + end + + def tags=(t) + @tags = t + end + + def add_tag_key_val(k,v) + @tags = Hash.new unless @tags + + # duplicate tags are now forbidden, so we can't allow values + # in the hash to be overwritten. + raise OSM::APIDuplicateTagsError.new("node", self.id, k) if @tags.include? k + + @tags[k] = v + end + + ## + # are the preconditions OK? this is mainly here to keep the duck + # typing interface the same between nodes, ways and relations. + def preconditions_ok? + in_world? + end + + ## + # dummy method to make the interfaces of node, way and relation + # more consistent. + def fix_placeholders!(id_map) + # nodes don't refer to anything, so there is nothing to do here + end + + private + + def save_with_history! + t = Time.now.getutc + Node.transaction do + self.version += 1 + self.timestamp = t + self.save! + + # Create a NodeTag + tags = self.tags + NodeTag.delete_all(['id = ?', self.id]) + tags.each do |k,v| + tag = NodeTag.new + tag.k = k + tag.v = v + tag.id = self.id + tag.save! + end + + # Create an OldNode + old_node = OldNode.from_node(self) + old_node.timestamp = t + old_node.save_with_dependencies! + + # tell the changeset we updated one element only + changeset.add_changes! 1 + + # save the changeset in case of bounding box updates + changeset.save! end - hash end + end diff --git a/app/models/old_node.rb b/app/models/old_node.rb index 76eab8427..be115c53e 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -1,25 +1,21 @@ class OldNode < ActiveRecord::Base include GeoRecord + include ConsistencyValidations set_table_name 'nodes' - validates_presence_of :user_id, :timestamp + validates_presence_of :changeset_id, :timestamp validates_inclusion_of :visible, :in => [ true, false ] validates_numericality_of :latitude, :longitude validate :validate_position + validates_associated :changeset - belongs_to :user + belongs_to :changeset def validate_position errors.add_to_base("Node is not in the world") unless in_world? end - def in_world? - return false if self.lat < -90 or self.lat > 90 - return false if self.lon < -180 or self.lon > 180 - return true - end - def self.from_node(node) old_node = OldNode.new old_node.latitude = node.latitude @@ -27,19 +23,30 @@ class OldNode < ActiveRecord::Base old_node.visible = node.visible old_node.tags = node.tags old_node.timestamp = node.timestamp - old_node.user_id = node.user_id + old_node.changeset_id = node.changeset_id old_node.id = node.id + old_node.version = node.version return old_node end + + def to_xml + doc = OSM::API.new.get_xml_doc + doc.root << to_xml_node() + return doc + end def to_xml_node el1 = XML::Node.new 'node' el1['id'] = self.id.to_s el1['lat'] = self.lat.to_s el1['lon'] = self.lon.to_s - el1['user'] = self.user.display_name if self.user.data_public? + el1['changeset'] = self.changeset.id.to_s + if self.changeset.user.data_public? + el1['user'] = self.changeset.user.display_name + el1['uid'] = self.changeset.user.id.to_s + end - Tags.split(self.tags) do |k,v| + self.tags.each do |k,v| el2 = XML::Node.new('tag') el2['k'] = k.to_s el2['v'] = v.to_s @@ -48,24 +55,54 @@ class OldNode < ActiveRecord::Base el1['visible'] = self.visible.to_s el1['timestamp'] = self.timestamp.xmlschema + el1['version'] = self.version.to_s return el1 end - - def tags_as_hash - hash = {} - Tags.split(self.tags) do |k,v| - hash[k] = v + + def save_with_dependencies! + save! + #not sure whats going on here + clear_aggregation_cache + clear_association_cache + #ok from here + @attributes.update(OldNode.find(:first, :conditions => ['id = ? AND timestamp = ? AND version = ?', self.id, self.timestamp, self.version]).instance_variable_get('@attributes')) + + self.tags.each do |k,v| + tag = OldNodeTag.new + tag.k = k + tag.v = v + tag.id = self.id + tag.version = self.version + tag.save! end - hash end - # Pretend we're not in any ways - def ways - return [] + def tags + unless @tags + @tags = Hash.new + OldNodeTag.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |tag| + @tags[tag.k] = tag.v + end + end + @tags = Hash.new unless @tags + @tags end - # Pretend we're not in any relations - def containing_relation_members - return [] + def tags=(t) + @tags = t end + + def tags_as_hash + return self.tags + end + + # Pretend we're not in any ways + def ways + return [] + end + + # Pretend we're not in any relations + def containing_relation_members + return [] + end end diff --git a/app/models/old_relation.rb b/app/models/old_relation.rb index bac03c4d2..b2fdf926e 100644 --- a/app/models/old_relation.rb +++ b/app/models/old_relation.rb @@ -1,14 +1,19 @@ class OldRelation < ActiveRecord::Base + include ConsistencyValidations + set_table_name 'relations' - belongs_to :user + belongs_to :changeset + + validates_associated :changeset def self.from_relation(relation) old_relation = OldRelation.new old_relation.visible = relation.visible - old_relation.user_id = relation.user_id + old_relation.changeset_id = relation.changeset_id old_relation.timestamp = relation.timestamp old_relation.id = relation.id + old_relation.version = relation.version old_relation.members = relation.members old_relation.tags = relation.tags return old_relation @@ -33,14 +38,12 @@ class OldRelation < ActiveRecord::Base tag.save! end - i = 1 - self.members.each do |m| + self.members.each_with_index do |m,i| member = OldRelationMember.new - member.id = self.id - member.member_type = m[0] + member.id = [self.id, self.version, i] + member.member_type = m[0].classify member.member_id = m[1] member.member_role = m[2] - member.version = self.version member.save! end end @@ -48,7 +51,7 @@ class OldRelation < ActiveRecord::Base def members unless @members @members = Array.new - OldRelationMember.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |m| + OldRelationMember.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version], :order => "sequence_id").each do |m| @members += [[m.type,m.id,m.role]] end end @@ -85,16 +88,27 @@ class OldRelation < ActiveRecord::Base OldRelationTag.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version]) end + def to_xml + doc = OSM::API.new.get_xml_doc + doc.root << to_xml_node() + return doc + end + def to_xml_node el1 = XML::Node.new 'relation' el1['id'] = self.id.to_s el1['visible'] = self.visible.to_s el1['timestamp'] = self.timestamp.xmlschema - el1['user'] = self.user.display_name if self.user.data_public? + if self.changeset.user.data_public? + el1['user'] = self.changeset.user.display_name + el1['uid'] = self.changeset.user.id.to_s + end + el1['version'] = self.version.to_s + el1['changeset'] = self.changeset_id.to_s self.old_members.each do |member| e = XML::Node.new 'member' - e['type'] = member.member_type.to_s + e['type'] = member.member_type.to_s.downcase e['ref'] = member.member_id.to_s # "id" is considered uncool here as it should be unique in XML e['role'] = member.member_role.to_s el1 << e diff --git a/app/models/old_relation_member.rb b/app/models/old_relation_member.rb index d8b685854..f0294d339 100644 --- a/app/models/old_relation_member.rb +++ b/app/models/old_relation_member.rb @@ -1,3 +1,6 @@ class OldRelationMember < ActiveRecord::Base set_table_name 'relation_members' + + set_primary_keys :id, :version, :sequence_id + belongs_to :relation, :foreign_key=> :id end diff --git a/app/models/old_relation_tag.rb b/app/models/old_relation_tag.rb index 7ce6f694e..0fcb11326 100644 --- a/app/models/old_relation_tag.rb +++ b/app/models/old_relation_tag.rb @@ -1,3 +1,10 @@ class OldRelationTag < ActiveRecord::Base set_table_name 'relation_tags' + + belongs_to :old_relation, :foreign_key => [:id, :version] + + validates_presence_of :id, :version + validates_length_of :k, :v, :maximum => 255, :allow_blank => true + validates_uniqueness_of :id, :scope => [:k, :version] + validates_numericality_of :id, :version, :only_integer => true end diff --git a/app/models/old_way.rb b/app/models/old_way.rb index 63265d6bf..425478a5b 100644 --- a/app/models/old_way.rb +++ b/app/models/old_way.rb @@ -1,14 +1,19 @@ class OldWay < ActiveRecord::Base + include ConsistencyValidations + set_table_name 'ways' - belongs_to :user + belongs_to :changeset + validates_associated :changeset + def self.from_way(way) old_way = OldWay.new old_way.visible = way.visible - old_way.user_id = way.user_id + old_way.changeset_id = way.changeset_id old_way.timestamp = way.timestamp old_way.id = way.id + old_way.version = way.version old_way.nds = way.nds old_way.tags = way.tags return old_way @@ -93,7 +98,12 @@ class OldWay < ActiveRecord::Base el1['id'] = self.id.to_s el1['visible'] = self.visible.to_s el1['timestamp'] = self.timestamp.xmlschema - el1['user'] = self.user.display_name if self.user.data_public? + if self.changeset.user.data_public? + el1['user'] = self.changeset.user.display_name + el1['uid'] = self.changeset.user.id.to_s + end + el1['version'] = self.version.to_s + el1['changeset'] = self.changeset.id.to_s self.old_nodes.each do |nd| # FIXME need to make sure they come back in the right order e = XML::Node.new 'nd' @@ -114,27 +124,29 @@ class OldWay < ActiveRecord::Base # For get_nodes_undelete, uses same nodes, even if they've moved since # For get_nodes_revert, allocates new ids # Currently returns Potlatch-style array - + # where [5] indicates whether latest version is usable as is (boolean) + # (i.e. is it visible? are we actually reverting to an earlier version?) + def get_nodes_undelete points = [] self.nds.each do |n| node=Node.find(n) - points << [node.lon, node.lat, n, node.visible ? 1 : 0, node.tags_as_hash] + points << [node.lon, node.lat, n, node.version, node.tags_as_hash, node.visible] end points end - def get_nodes_revert + def get_nodes_revert(timestamp) points=[] self.nds.each do |n| - oldnode=OldNode.find(:first, :conditions=>['id=? AND timestamp<=?',n,self.timestamp], :order=>"timestamp DESC") + oldnode=OldNode.find(:first, :conditions=>['id=? AND timestamp<=?',n,timestamp], :order=>"timestamp DESC") curnode=Node.find(n) - id=n; v=curnode.visible ? 1 : 0 + id=n; reuse=curnode.visible if oldnode.lat!=curnode.lat or oldnode.lon!=curnode.lon or oldnode.tags!=curnode.tags then # node has changed: if it's in other ways, give it a new id - if curnode.ways-[self.id] then id=-1; v=nil end + if curnode.ways-[self.id] then id=-1; reuse=false end end - points << [oldnode.lon, oldnode.lat, id, v, oldnode.tags_as_hash] + points << [oldnode.lon, oldnode.lat, id, curnode.version, oldnode.tags_as_hash, reuse] end points end diff --git a/app/models/old_way_tag.rb b/app/models/old_way_tag.rb index b02fd45b9..801532dba 100644 --- a/app/models/old_way_tag.rb +++ b/app/models/old_way_tag.rb @@ -1,6 +1,10 @@ class OldWayTag < ActiveRecord::Base - belongs_to :user - set_table_name 'way_tags' + belongs_to :old_way, :foreign_key => [:id, :version] + + validates_presence_of :id + validates_length_of :k, :v, :maximum => 255, :allow_blank => true + validates_uniqueness_of :id, :scope => [:k, :version] + validates_numericality_of :id, :version, :only_integer => true end diff --git a/app/models/relation.rb b/app/models/relation.rb index d9dba303f..36d6943d9 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -1,51 +1,83 @@ class Relation < ActiveRecord::Base require 'xml/libxml' + include ConsistencyValidations + set_table_name 'current_relations' - belongs_to :user + belongs_to :changeset has_many :old_relations, :foreign_key => 'id', :order => 'version' - has_many :relation_members, :foreign_key => 'id' + has_many :relation_members, :foreign_key => 'id', :order => 'sequence_id' has_many :relation_tags, :foreign_key => 'id' has_many :containing_relation_members, :class_name => "RelationMember", :as => :member has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder + validates_presence_of :id, :on => :update + validates_presence_of :timestamp,:version, :changeset_id + validates_uniqueness_of :id + validates_inclusion_of :visible, :in => [ true, false ] + validates_numericality_of :id, :on => :update, :integer_only => true + validates_numericality_of :changeset_id, :version, :integer_only => true + validates_associated :changeset + + TYPES = ["node", "way", "relation"] + def self.from_xml(xml, create=false) begin p = XML::Parser.string(xml) doc = p.parse - relation = Relation.new - doc.find('//osm/relation').each do |pt| - if !create and pt['id'] != '0' - relation.id = pt['id'].to_i - end + return Relation.from_xml_node(pt, create) + end + rescue LibXML::XML::Error, ArgumentError => ex + raise OSM::APIBadXMLError.new("relation", xml, ex.message) + end + end - if create - relation.timestamp = Time.now - relation.visible = true - else - if pt['timestamp'] - relation.timestamp = Time.parse(pt['timestamp']) - end - end + def self.from_xml_node(pt, create=false) + relation = Relation.new - pt.find('tag').each do |tag| - relation.add_tag_keyval(tag['k'], tag['v']) - end + if !create and pt['id'] != '0' + relation.id = pt['id'].to_i + end - pt.find('member').each do |member| - relation.add_member(member['type'], member['ref'], member['role']) - end + raise OSM::APIBadXMLError.new("relation", pt, "You are missing the required changeset in the relation") if pt['changeset'].nil? + relation.changeset_id = pt['changeset'] + + # The follow block does not need to be executed because they are dealt with + # in create_with_history, update_from and delete_with_history + if create + relation.timestamp = Time.now.getutc + relation.visible = true + relation.version = 0 + else + if pt['timestamp'] + relation.timestamp = Time.parse(pt['timestamp']) end - rescue - relation = nil + relation.version = pt['version'] + end + + pt.find('tag').each do |tag| + relation.add_tag_keyval(tag['k'], tag['v']) end + pt.find('member').each do |member| + #member_type = + logger.debug "each member" + raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member['type'] + logger.debug "after raise" + #member_ref = member['ref'] + #member_role + member['role'] ||= "" # Allow the upload to not include this, in which case we default to an empty string. + logger.debug member['role'] + relation.add_member(member['type'].classify, member['ref'], member['role']) + end + raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil? + return relation end @@ -60,18 +92,23 @@ class Relation < ActiveRecord::Base el1['id'] = self.id.to_s el1['visible'] = self.visible.to_s el1['timestamp'] = self.timestamp.xmlschema + el1['version'] = self.version.to_s + el1['changeset'] = self.changeset_id.to_s user_display_name_cache = {} if user_display_name_cache.nil? - if user_display_name_cache and user_display_name_cache.key?(self.user_id) + if user_display_name_cache and user_display_name_cache.key?(self.changeset.user_id) # use the cache if available - elsif self.user.data_public? - user_display_name_cache[self.user_id] = self.user.display_name + elsif self.changeset.user.data_public? + user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name else - user_display_name_cache[self.user_id] = nil + user_display_name_cache[self.changeset.user_id] = nil end - el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil? + if not user_display_name_cache[self.changeset.user_id].nil? + el1['user'] = user_display_name_cache[self.changeset.user_id] + el1['uid'] = self.changeset.user_id.to_s + end self.relation_members.each do |member| p=0 @@ -88,7 +125,7 @@ class Relation < ActiveRecord::Base #end if p e = XML::Node.new 'member' - e['type'] = member.member_type + e['type'] = member.member_type.downcase e['ref'] = member.member_id.to_s e['role'] = member.member_role el1 << e @@ -108,7 +145,7 @@ class Relation < ActiveRecord::Base if ids.empty? return [] else - self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'node' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do + self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Node' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do return self.find(:all, options) end end @@ -118,7 +155,7 @@ class Relation < ActiveRecord::Base if ids.empty? return [] else - self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'way' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do + self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Way' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do return self.find(:all, options) end end @@ -128,7 +165,7 @@ class Relation < ActiveRecord::Base if ids.empty? return [] else - self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'relation' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do + self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Relation' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do return self.find(:all, options) end end @@ -170,19 +207,164 @@ class Relation < ActiveRecord::Base def add_tag_keyval(k, v) @tags = Hash.new unless @tags + + # duplicate tags are now forbidden, so we can't allow values + # in the hash to be overwritten. + raise OSM::APIDuplicateTagsError.new("relation", self.id, k) if @tags.include? k + @tags[k] = v end + ## + # updates the changeset bounding box to contain the bounding box of + # the element with given +type+ and +id+. this only works with nodes + # and ways at the moment, as they're the only elements to respond to + # the :bbox call. + def update_changeset_element(type, id) + element = Kernel.const_get(type.capitalize).find(id) + changeset.update_bbox! element.bbox + end + + def delete_with_history!(new_relation, user) + unless self.visible + raise OSM::APIAlreadyDeletedError.new + end + + # need to start the transaction here, so that the database can + # provide repeatable reads for the used-by checks. this means it + # shouldn't be possible to get race conditions. + Relation.transaction do + check_consistency(self, new_relation, user) + # This will check to see if this relation is used by another relation + if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = ? AND member_type='Relation' and member_id=? ", true, self.id ]) + raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is a used in another relation") + end + self.changeset_id = new_relation.changeset_id + self.tags = {} + self.members = [] + self.visible = false + save_with_history! + end + end + + def update_from(new_relation, user) + check_consistency(self, new_relation, user) + if !new_relation.preconditions_ok? + raise OSM::APIPreconditionFailedError.new + end + self.changeset_id = new_relation.changeset_id + self.changeset = new_relation.changeset + self.tags = new_relation.tags + self.members = new_relation.members + self.visible = true + save_with_history! + end + + def create_with_history(user) + check_create_consistency(self, user) + if !self.preconditions_ok? + raise OSM::APIPreconditionFailedError.new + end + self.version = 0 + self.visible = true + save_with_history! + end + + def preconditions_ok? + # These are hastables that store an id in the index of all + # the nodes/way/relations that have already been added. + # If the member is valid and visible then we add it to the + # relevant hash table, with the value true as a cache. + # Thus if you have nodes with the ids of 50 and 1 already in the + # relation, then the hash table nodes would contain: + # => {50=>true, 1=>true} + elements = { :node => Hash.new, :way => Hash.new, :relation => Hash.new } + self.members.each do |m| + # find the hash for the element type or die + logger.debug m[0] + hash = elements[m[0].downcase.to_sym] or return false + # unless its in the cache already + unless hash.key? m[1] + # use reflection to look up the appropriate class + model = Kernel.const_get(m[0].capitalize) + # get the element with that ID + element = model.find(m[1]) + + # and check that it is OK to use. + unless element and element.visible? and element.preconditions_ok? + return false + end + hash[m[1]] = true + end + end + + return true + rescue + return false + end + + # Temporary method to match interface to nodes + def tags_as_hash + return self.tags + end + + ## + # if any members are referenced by placeholder IDs (i.e: negative) then + # this calling this method will fix them using the map from placeholders + # to IDs +id_map+. + def fix_placeholders!(id_map) + self.members.map! do |type, id, role| + old_id = id.to_i + if old_id < 0 + new_id = id_map[type.downcase.to_sym][old_id] + raise "invalid placeholder" if new_id.nil? + [type, new_id, role] + else + [type, id, role] + end + end + end + + private + def save_with_history! Relation.transaction do - t = Time.now + # have to be a little bit clever here - to detect if any tags + # changed then we have to monitor their before and after state. + tags_changed = false + + t = Time.now.getutc + self.version += 1 self.timestamp = t self.save! tags = self.tags + self.relation_tags.each do |old_tag| + key = old_tag.k + # if we can match the tags we currently have to the list + # of old tags, then we never set the tags_changed flag. but + # if any are different then set the flag and do the DB + # update. + if tags.has_key? key + # rails 2.1 dirty handling should take care of making this + # somewhat efficient... hopefully... + old_tag.v = tags[key] + tags_changed |= old_tag.changed? + old_tag.save! + + # remove from the map, so that we can expect an empty map + # at the end if there are no new tags + tags.delete key - RelationTag.delete_all(['id = ?', self.id]) - + else + # this means a tag was deleted + tags_changed = true + RelationTag.delete_all ['id = ? and k = ?', self.id, old_tag.k] + end + end + # if there are left-over tags then they are new and will have to + # be added. + tags_changed |= (not tags.empty?) tags.each do |k,v| tag = RelationTag.new tag.k = k @@ -190,81 +372,87 @@ class Relation < ActiveRecord::Base tag.id = self.id tag.save! end + + # reload, so that all of the members are accessible in their + # new state. + self.reload + + # same pattern as before, but this time we're collecting the + # changed members in an array, as the bounding box updates for + # elements are per-element, not blanked on/off like for tags. + changed_members = Array.new + members = Hash.new + self.members.each do |m| + # should be: h[[m.id, m.type]] = m.role, but someone prefers arrays + members[[m[1], m[0]]] = m[2] + end + relation_members.each do |old_member| + key = [old_member.member_id.to_s, old_member.member_type] + if members.has_key? key + members.delete key + else + changed_members << key + end + end + # any remaining members must be new additions + changed_members += members.keys + # update the members. first delete all the old members, as the new + # members may be in a different order and i don't feel like implementing + # a longest common subsequence algorithm to optimise this. members = self.members - - RelationMember.delete_all(['id = ?', self.id]) - - members.each do |n| + RelationMember.delete_all(:id => self.id) + members.each_with_index do |m,i| mem = RelationMember.new - mem.id = self.id - mem.member_type = n[0]; - mem.member_id = n[1]; - mem.member_role = n[2]; + mem.id = [self.id, i] + mem.member_type = m[0] + mem.member_id = m[1] + mem.member_role = m[2] mem.save! end old_relation = OldRelation.from_relation(self) old_relation.timestamp = t old_relation.save_with_dependencies! - end - end - def preconditions_ok? - # These are hastables that store an id in the index of all - # the nodes/way/relations that have already been added. - # Once we know the id of the node/way/relation exists - # we check to see if it is already existing in the hashtable - # if it does, then we return false. Otherwise - # we add it to the relevant hash table, with the value true.. - # Thus if you have nodes with the ids of 50 and 1 already in the - # relation, then the hash table nodes would contain: - # => {50=>true, 1=>true} - nodes = Hash.new - ways = Hash.new - relations = Hash.new - self.members.each do |m| - if (m[0] == "node") - n = Node.find(:first, :conditions => ["id = ?", m[1]]) - unless n and n.visible - return false - end - if nodes[m[1]] - return false - else - nodes[m[1]] = true - end - elsif (m[0] == "way") - w = Way.find(:first, :conditions => ["id = ?", m[1]]) - unless w and w.visible and w.preconditions_ok? - return false - end - if ways[m[1]] - return false - else - ways[m[1]] = true - end - elsif (m[0] == "relation") - e = Relation.find(:first, :conditions => ["id = ?", m[1]]) - unless e and e.visible and e.preconditions_ok? - return false - end - if relations[m[1]] - return false - else - relations[m[1]] = true + # update the bbox of the changeset and save it too. + # discussion on the mailing list gave the following definition for + # the bounding box update procedure of a relation: + # + # adding or removing nodes or ways from a relation causes them to be + # added to the changeset bounding box. adding a relation member or + # changing tag values causes all node and way members to be added to the + # bounding box. this is similar to how the map call does things and is + # reasonable on the assumption that adding or removing members doesn't + # materially change the rest of the relation. + any_relations = + changed_members.collect { |id,type| type == "relation" }. + inject(false) { |b,s| b or s } + + if tags_changed or any_relations + # add all non-relation bounding boxes to the changeset + # FIXME: check for tag changes along with element deletions and + # make sure that the deleted element's bounding box is hit. + self.members.each do |type, id, role| + if type != "Relation" + update_changeset_element(type, id) + end end else - return false + # add only changed members to the changeset + changed_members.each do |id, type| + if type != "Relation" + update_changeset_element(type, id) + end + end end + + # tell the changeset we updated one element only + changeset.add_changes! 1 + + # save the (maybe updated) changeset bounding box + changeset.save! end - return true - rescue - return false end - # Temporary method to match interface to nodes - def tags_as_hash - return self.tags - end end diff --git a/app/models/relation_member.rb b/app/models/relation_member.rb index 9ff4f46f3..b385dd6d1 100644 --- a/app/models/relation_member.rb +++ b/app/models/relation_member.rb @@ -1,19 +1,20 @@ class RelationMember < ActiveRecord::Base set_table_name 'current_relation_members' + set_primary_keys :id, :sequence_id belongs_to :member, :polymorphic => true, :foreign_type => :member_class belongs_to :relation, :foreign_key => :id def after_find - self[:member_class] = self.member_type.capitalize + self[:member_class] = self.member_type.classify end def after_initialize - self[:member_class] = self.member_type.capitalize + self[:member_class] = self.member_type.classify unless self.member_type.nil? end def before_save - self.member_type = self[:member_class].downcase + self.member_type = self[:member_class].classify end def member_type=(type) diff --git a/app/models/relation_tag.rb b/app/models/relation_tag.rb index 939165ebd..812b2ec35 100644 --- a/app/models/relation_tag.rb +++ b/app/models/relation_tag.rb @@ -3,4 +3,8 @@ class RelationTag < ActiveRecord::Base belongs_to :relation, :foreign_key => 'id' + validates_presence_of :id + validates_length_of :k, :v, :maximum => 255, :allow_blank => true + validates_uniqueness_of :id, :scope => :k + validates_numericality_of :id, :only_integer => true end diff --git a/app/models/trace.rb b/app/models/trace.rb index 10e867bad..03dbeb0b3 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -3,6 +3,8 @@ class Trace < ActiveRecord::Base validates_presence_of :user_id, :name, :timestamp validates_presence_of :description, :on => :create + validates_length_of :name, :maximum => 255 + validates_length_of :description, :maximum => 255 # validates_numericality_of :latitude, :longitude validates_inclusion_of :public, :inserted, :in => [ true, false] diff --git a/app/models/tracetag.rb b/app/models/tracetag.rb index f1d5967d5..f9833e141 100644 --- a/app/models/tracetag.rb +++ b/app/models/tracetag.rb @@ -2,6 +2,7 @@ class Tracetag < ActiveRecord::Base set_table_name 'gpx_file_tags' validates_format_of :tag, :with => /^[^\/;.,?]*$/ + validates_length_of :tag, :within => 1..255 belongs_to :trace, :foreign_key => 'gpx_id' end diff --git a/app/models/user.rb b/app/models/user.rb index fae037110..4113662aa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,19 +4,21 @@ class User < ActiveRecord::Base has_many :traces has_many :diary_entries, :order => 'created_at DESC' has_many :messages, :foreign_key => :to_user_id, :order => 'sent_on DESC' - has_many :new_messages, :class_name => "Message", :foreign_key => :to_user_id, :conditions => "message_read = 0", :order => 'sent_on DESC' + has_many :new_messages, :class_name => "Message", :foreign_key => :to_user_id, :conditions => {:message_read => false}, :order => 'sent_on DESC' has_many :sent_messages, :class_name => "Message", :foreign_key => :from_user_id, :order => 'sent_on DESC' - has_many :friends, :include => :befriendee, :conditions => "users.visible = 1" + has_many :friends, :include => :befriendee, :conditions => ["users.visible = ?", true] has_many :tokens, :class_name => "UserToken" has_many :preferences, :class_name => "UserPreference" + has_many :changesets validates_presence_of :email, :display_name validates_confirmation_of :email, :message => 'Email addresses must match' validates_confirmation_of :pass_crypt, :message => 'Password must match the confirmation password' validates_uniqueness_of :display_name, :allow_nil => true validates_uniqueness_of :email - validates_length_of :pass_crypt, :minimum => 8 - validates_length_of :display_name, :minimum => 3, :allow_nil => true + validates_length_of :pass_crypt, :within => 8..255 + validates_length_of :display_name, :within => 3..255, :allow_nil => true + validates_length_of :email, :within => 6..255 validates_format_of :email, :with => /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i validates_format_of :display_name, :with => /^[^\/;.,?]*$/ validates_numericality_of :home_lat, :allow_nil => true @@ -28,7 +30,7 @@ class User < ActiveRecord::Base file_column :image, :magick => { :geometry => "100x100>" } def after_initialize - self.creation_time = Time.now if self.creation_time.nil? + self.creation_time = Time.now.getutc if self.creation_time.nil? end def encrypt_password @@ -80,7 +82,7 @@ class User < ActiveRecord::Base if self.home_lon and self.home_lat gc = OSM::GreatCircle.new(self.home_lat, self.home_lon) bounds = gc.bounds(radius) - nearby = User.find(:all, :conditions => "visible = 1 and home_lat between #{bounds[:minlat]} and #{bounds[:maxlat]} and home_lon between #{bounds[:minlon]} and #{bounds[:maxlon]} and data_public = 1 and id != #{self.id}") + nearby = User.find(:all, :conditions => ["visible = ? and home_lat between #{bounds[:minlat]} and #{bounds[:maxlat]} and home_lon between #{bounds[:minlon]} and #{bounds[:maxlon]} and data_public = ? and id != #{self.id}", true, true]) nearby.delete_if { |u| gc.distance(u.home_lat, u.home_lon) > radius } nearby.sort! { |u1,u2| gc.distance(u1.home_lat, u1.home_lon) <=> gc.distance(u2.home_lat, u2.home_lon) } else @@ -104,6 +106,10 @@ class User < ActiveRecord::Base return false end + def trace_public_default + return self.preferences.find(:first, :conditions => {:k => "gps.trace.public", :v => "default"}) + end + def delete self.active = false self.display_name = "user_#{self.id}" diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index 3985a527e..28ef40f1d 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -1,6 +1,9 @@ class UserPreference < ActiveRecord::Base set_primary_keys :user_id, :k belongs_to :user + + validates_length_of :k, :within => 1..255 + validates_length_of :v, :within => 1..255 # Turn this Node in to an XML Node without the wrapper. def to_xml_node diff --git a/app/models/way.rb b/app/models/way.rb index 6c3ea9e46..e134d697d 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -1,9 +1,11 @@ class Way < ActiveRecord::Base require 'xml/libxml' + + include ConsistencyValidations set_table_name 'current_ways' - - belongs_to :user + + belongs_to :changeset has_many :old_ways, :foreign_key => 'id', :order => 'version' @@ -15,37 +17,56 @@ class Way < ActiveRecord::Base has_many :containing_relation_members, :class_name => "RelationMember", :as => :member has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder + validates_presence_of :id, :on => :update + validates_presence_of :changeset_id,:version, :timestamp + validates_uniqueness_of :id + validates_inclusion_of :visible, :in => [ true, false ] + validates_numericality_of :changeset_id, :version, :integer_only => true + validates_numericality_of :id, :on => :update, :integer_only => true + validates_associated :changeset + def self.from_xml(xml, create=false) begin p = XML::Parser.string(xml) doc = p.parse - way = Way.new - doc.find('//osm/way').each do |pt| - if !create and pt['id'] != '0' - way.id = pt['id'].to_i - end - - if create - way.timestamp = Time.now - way.visible = true - else - if pt['timestamp'] - way.timestamp = Time.parse(pt['timestamp']) - end - end + return Way.from_xml_node(pt, create) + end + rescue LibXML::XML::Error, ArgumentError => ex + raise OSM::APIBadXMLError.new("way", xml, ex.message) + end + end - pt.find('tag').each do |tag| - way.add_tag_keyval(tag['k'], tag['v']) - end + def self.from_xml_node(pt, create=false) + way = Way.new - pt.find('nd').each do |nd| - way.add_nd_num(nd['ref']) - end + if !create and pt['id'] != '0' + way.id = pt['id'].to_i + end + + way.version = pt['version'] + raise OSM::APIBadXMLError.new("node", pt, "Changeset is required") if pt['changeset'].nil? + way.changeset_id = pt['changeset'] + + # This next section isn't required for the create, update, or delete of ways + if create + way.timestamp = Time.now.getutc + way.visible = true + else + if pt['timestamp'] + way.timestamp = Time.parse(pt['timestamp']) end - rescue - way = nil + # if visible isn't present then it defaults to true + way.visible = (pt['visible'] or true) + end + + pt.find('tag').each do |tag| + way.add_tag_keyval(tag['k'], tag['v']) + end + + pt.find('nd').each do |nd| + way.add_nd_num(nd['ref']) end return way @@ -73,18 +94,23 @@ class Way < ActiveRecord::Base el1['id'] = self.id.to_s el1['visible'] = self.visible.to_s el1['timestamp'] = self.timestamp.xmlschema + el1['version'] = self.version.to_s + el1['changeset'] = self.changeset_id.to_s user_display_name_cache = {} if user_display_name_cache.nil? - if user_display_name_cache and user_display_name_cache.key?(self.user_id) + if user_display_name_cache and user_display_name_cache.key?(self.changeset.user_id) # use the cache if available - elsif self.user.data_public? - user_display_name_cache[self.user_id] = self.user.display_name + elsif self.changeset.user.data_public? + user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name else - user_display_name_cache[self.user_id] = nil + user_display_name_cache[self.changeset.user_id] = nil end - el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil? + if not user_display_name_cache[self.changeset.user_id].nil? + el1['user'] = user_display_name_cache[self.changeset.user_id] + el1['uid'] = self.changeset.user_id.to_s + end # make sure nodes are output in sequence_id order ordered_nodes = [] @@ -96,7 +122,7 @@ class Way < ActiveRecord::Base end else # otherwise, manually go to the db to check things - if nd.node.visible? and nd.node.visible? + if nd.node and nd.node.visible? ordered_nodes[nd.sequence_id] = nd.node_id.to_s end end @@ -154,99 +180,86 @@ class Way < ActiveRecord::Base def add_tag_keyval(k, v) @tags = Hash.new unless @tags - @tags[k] = v - end - def save_with_history! - t = Time.now + # duplicate tags are now forbidden, so we can't allow values + # in the hash to be overwritten. + raise OSM::APIDuplicateTagsError.new("way", self.id, k) if @tags.include? k - Way.transaction do - self.timestamp = t - self.save! - end - - WayTag.transaction do - tags = self.tags + @tags[k] = v + end - WayTag.delete_all(['id = ?', self.id]) + ## + # the integer coords (i.e: unscaled) bounding box of the way, assuming + # straight line segments. + def bbox + lons = nodes.collect { |n| n.longitude } + lats = nodes.collect { |n| n.latitude } + [ lons.min, lats.min, lons.max, lats.max ] + end - tags.each do |k,v| - tag = WayTag.new - tag.k = k - tag.v = v - tag.id = self.id - tag.save! - end + def update_from(new_way, user) + check_consistency(self, new_way, user) + if !new_way.preconditions_ok? + raise OSM::APIPreconditionFailedError.new end - WayNode.transaction do - nds = self.nds - - WayNode.delete_all(['id = ?', self.id]) + self.changeset_id = new_way.changeset_id + self.changeset = new_way.changeset + self.tags = new_way.tags + self.nds = new_way.nds + self.visible = true + save_with_history! + end - sequence = 1 - nds.each do |n| - nd = WayNode.new - nd.id = [self.id, sequence] - nd.node_id = n - nd.save! - sequence += 1 - end + def create_with_history(user) + check_create_consistency(self, user) + if !self.preconditions_ok? + raise OSM::APIPreconditionFailedError.new end - - old_way = OldWay.from_way(self) - old_way.timestamp = t - old_way.save_with_dependencies! + self.version = 0 + self.visible = true + save_with_history! end def preconditions_ok? return false if self.nds.empty? + if self.nds.length > APP_CONFIG['max_number_of_way_nodes'] + raise OSM::APITooManyWayNodesError.new(self.nds.count, APP_CONFIG['max_number_of_way_nodes']) + end self.nds.each do |n| node = Node.find(:first, :conditions => ["id = ?", n]) unless node and node.visible - return false + raise OSM::APIPreconditionFailedError.new("The node with id #{n} either does not exist, or is not visible") end end return true end - # Delete the way and it's relations, but don't really delete it - set its visibility to false and update the history etc to maintain wiki-like functionality. - def delete_with_relations_and_history(user) - if self.visible - # FIXME - # this should actually delete the relations, - # not just throw a PreconditionFailed if it's a member of a relation!! + def delete_with_history!(new_way, user) + unless self.visible + raise OSM::APIAlreadyDeletedError + end + + # need to start the transaction here, so that the database can + # provide repeatable reads for the used-by checks. this means it + # shouldn't be possible to get race conditions. + Way.transaction do + check_consistency(self, new_way, user) if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", - :conditions => [ "visible = 1 AND member_type='way' and member_id=?", self.id]) - raise OSM::APIPreconditionFailedError - # end FIXME + :conditions => [ "visible = ? AND member_type='Way' and member_id=? ", true, self.id]) + raise OSM::APIPreconditionFailedError.new("You need to make sure that this way is not a member of a relation.") else - self.user_id = user.id + self.changeset_id = new_way.changeset_id + self.changeset = new_way.changeset + self.tags = [] self.nds = [] self.visible = false - self.save_with_history! + save_with_history! end - else - raise OSM::APIAlreadyDeletedError end end - # delete a way and it's nodes that aren't part of other ways, with history - def delete_with_relations_and_nodes_and_history(user) - # delete the nodes not used by other ways - self.unshared_node_ids.each do |node_id| - n = Node.find(node_id) - n.user_id = user.id - n.visible = false - n.save_with_history! - end - - self.user_id = user.id - - self.delete_with_relations_and_history(user) - end - # Find nodes that belong to this way only def unshared_node_ids node_ids = self.nodes.collect { |node| node.id } @@ -263,4 +276,78 @@ class Way < ActiveRecord::Base def tags_as_hash return self.tags end + + ## + # if any referenced nodes are placeholder IDs (i.e: are negative) then + # this calling this method will fix them using the map from placeholders + # to IDs +id_map+. + def fix_placeholders!(id_map) + self.nds.map! do |node_id| + if node_id < 0 + new_id = id_map[:node][node_id] + raise "invalid placeholder for #{node_id.inspect}: #{new_id.inspect}" if new_id.nil? + new_id + else + node_id + end + end + end + + private + + def save_with_history! + t = Time.now.getutc + + # update the bounding box, note that this has to be done both before + # and after the save, so that nodes from both versions are included in the + # bbox. we use a copy of the changeset so that it isn't reloaded + # later in the save. + cs = self.changeset + cs.update_bbox!(bbox) unless nodes.empty? + + Way.transaction do + self.version += 1 + self.timestamp = t + self.save! + + tags = self.tags + WayTag.delete_all(['id = ?', self.id]) + tags.each do |k,v| + tag = WayTag.new + tag.k = k + tag.v = v + tag.id = self.id + tag.save! + end + + nds = self.nds + WayNode.delete_all(['id = ?', self.id]) + sequence = 1 + nds.each do |n| + nd = WayNode.new + nd.id = [self.id, sequence] + nd.node_id = n + nd.save! + sequence += 1 + end + + old_way = OldWay.from_way(self) + old_way.timestamp = t + old_way.save_with_dependencies! + + # reload the way so that the nodes array points to the correct + # new set of nodes. + self.reload + + # update and commit the bounding box, now that way nodes + # have been updated and we're in a transaction. + cs.update_bbox!(bbox) unless nodes.empty? + + # tell the changeset we updated one element only + cs.add_changes! 1 + + cs.save! + end + end + end diff --git a/app/models/way_tag.rb b/app/models/way_tag.rb index 4548674d4..fa9b43361 100644 --- a/app/models/way_tag.rb +++ b/app/models/way_tag.rb @@ -6,4 +6,9 @@ class WayTag < ActiveRecord::Base # FIXME add a real multipart key to waytags so that we can do eager loadin belongs_to :way, :foreign_key => 'id' + + validates_presence_of :id + validates_length_of :k, :v, :maximum => 255, :allow_blank => true + validates_uniqueness_of :id, :scope => :k + validates_numericality_of :id, :only_integer => true end diff --git a/app/views/browse/_changeset_details.rhtml b/app/views/browse/_changeset_details.rhtml new file mode 100644 index 000000000..12d4dfb25 --- /dev/null +++ b/app/views/browse/_changeset_details.rhtml @@ -0,0 +1,95 @@ + + + + + + + + + + + + + <% if changeset_details.user.data_public? %> + + + + + <% end %> + + <%= render :partial => "tag_details", :object => changeset_details %> + + + + <% unless changeset_details.has_valid_bbox? %> + + <% else + minlon = changeset_details.min_lon/GeoRecord::SCALE.to_f + minlat = changeset_details.min_lat/GeoRecord::SCALE.to_f + maxlon = changeset_details.max_lon/GeoRecord::SCALE.to_f + maxlat = changeset_details.max_lat/GeoRecord::SCALE.to_f + %> + + <% end %> + + + <% unless @nodes.empty? %> + + + + + <%= render :partial => 'paging_nav', :locals => { :pages => @node_pages, :page_param => "node_page"} %> + <% end %> + + <% unless @ways.empty? %> + + + + + <%= render :partial => 'paging_nav', :locals => { :pages => @way_pages, :page_param => "way_page" } %> + <% end %> + + <% unless @relations.empty? %> + + + + + <%= render :partial => 'paging_nav', :locals => { :pages => @relation_pages, :page_param => "relation_page" } %> + <% end %> + +
Created at:<%= h(changeset_details.created_at) %>
Closed at:<%= h(changeset_details.closed_at) %>
Belongs to:<%= link_to h(changeset_details.user.display_name), :controller => "user", :action => "view", :display_name => changeset_details.user.display_name %>
Bounding box:No bounding box has been stored for this changeset. + + + + + + + + + + + + +
<%=maxlat -%>
<%=minlon -%>(box)<%=maxlon -%>
<%= minlon -%>
+
Has the following <%= @node_pages.item_count %> nodes: + + <% @nodes.each do |node| %> + + <% end %> +
<%= 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 %> + <%= + #render :partial => "containing_relation", :collection => changeset_details.containing_relation_members + %> +
<%= 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 %> +
<%= link_to "Relation #{relation.id.to_s}, version #{relation.version.to_s}", :action => "relation", :id => relation.id.to_s %>
+
diff --git a/app/views/browse/_common_details.rhtml b/app/views/browse/_common_details.rhtml index ee5f22cee..09cf4cf2d 100644 --- a/app/views/browse/_common_details.rhtml +++ b/app/views/browse/_common_details.rhtml @@ -3,20 +3,21 @@ <%= h(common_details.timestamp) %> -<% if common_details.user.data_public %> +<% if common_details.changeset.user.data_public? %> Edited by: - <%= link_to h(common_details.user.display_name), :controller => "user", :action => "view", :display_name => common_details.user.display_name %> + <%= link_to h(common_details.changeset.user.display_name), :controller => "user", :action => "view", :display_name => common_details.changeset.user.display_name %> <% end %> -<% unless common_details.tags_as_hash.empty? %> - - Tags: - - - <%= render :partial => "tag", :collection => common_details.tags_as_hash %> -
- - -<% end %> + + Version: + <%= h(common_details.version) %> + + + + In changeset: + <%= link_to common_details.changeset_id, :action => :changeset, :id => common_details.changeset_id %> + + +<%= render :partial => "tag_details", :object => common_details %> diff --git a/app/views/browse/_map.rhtml b/app/views/browse/_map.rhtml index ad2d2d307..d972104ba 100644 --- a/app/views/browse/_map.rhtml +++ b/app/views/browse/_map.rhtml @@ -2,7 +2,7 @@ <%= javascript_include_tag '/openlayers/OpenStreetMap.js' %> <%= javascript_include_tag 'map.js' %> - <% if map.visible %> + <% if map.instance_of? Changeset or map.visible %>
Loading... @@ -13,6 +13,25 @@