From dcad29dad0d29e22ffa0c34a8d9b43cbf5d64f12 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Wed, 27 Jun 2007 17:27:10 +0000 Subject: [PATCH] Split the rest action into sparate read, update and delete actions thus allowing authorization to be done on a per-action basis without worring about the method. This should make the user API work. Also do a lot of cleanup of the controllers. --- app/controllers/amf_controller.rb | 6 +- app/controllers/api_controller.rb | 11 +- app/controllers/application.rb | 52 +++--- app/controllers/diary_entry_controller.rb | 4 +- app/controllers/message_controller.rb | 2 +- app/controllers/node_controller.rb | 127 ++++++------- app/controllers/old_node_controller.rb | 25 ++- app/controllers/old_segment_controller.rb | 23 +-- app/controllers/old_way_controller.rb | 28 +-- app/controllers/search_controller.rb | 4 +- app/controllers/segment_controller.rb | 179 +++++++++--------- app/controllers/swf_controller.rb | 3 +- app/controllers/trace_controller.rb | 62 ++++--- app/controllers/user_controller.rb | 2 +- app/controllers/way_controller.rb | 210 ++++++++++++---------- app/models/trace.rb | 6 + config/routes.rb | 12 +- 17 files changed, 406 insertions(+), 350 deletions(-) diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 72f1dd74e..1049b51b6 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -33,8 +33,6 @@ class AmfController < ApplicationController bytes=getlong(req) # | get total size in bytes args=getvalue(req) # | get response (probably an array) - RAILS_DEFAULT_LOGGER.info(" Message: #{message}") - case message when 'getpresets'; results[index]=putdata(index,getpresets) when 'whichways'; results[index]=putdata(index,whichways(args)) @@ -48,12 +46,10 @@ class AmfController < ApplicationController # Write out response RAILS_DEFAULT_LOGGER.info(" Response: start") - response.headers["Content-Type"]="application/x-amf" a,b=results.length.divmod(256) - render :text => proc { |response, output| + 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| - RAILS_DEFAULT_LOGGER.info(" Response: encode #{k}") output.write(v) end } diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index be00b9f9c..1804ceb49 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,6 +1,5 @@ class ApiController < ApplicationController - before_filter :authorize after_filter :compress_output helper :user @@ -18,7 +17,6 @@ class ApiController < ApplicationController def trackpoints @@count+=1 - response.headers["Content-Type"] = 'text/xml' #retrieve the page number page = params['page'].to_i unless page @@ -96,12 +94,12 @@ class ApiController < ApplicationController #exit when we have too many requests if @@count > MAX_COUNT - render :text => doc.to_s + render :text => doc.to_s, :content_type => "text/xml" @@count = COUNT exit! end - render :text => doc.to_s + render :text => doc.to_s, :content_type => "text/xml" end @@ -109,7 +107,6 @@ class ApiController < ApplicationController GC.start @@count+=1 - response.headers["Content-Type"] = 'text/xml' # Figure out the bbox bbox = params['bbox'] unless bbox and bbox.count(',') == 3 @@ -155,7 +152,7 @@ class ApiController < ApplicationController end if node_ids.length == 0 - render :text => "" + render :text => "", :content_type => "text/xml" return end @@ -240,7 +237,7 @@ class ApiController < ApplicationController doc.root << way.to_xml_node(visible_segments, user_display_name_cache) if way.visible? end - render :text => doc.to_s + render :text => doc.to_s, :content_type => "text/xml" #exit when we have too many requests if @@count > MAX_COUNT diff --git a/app/controllers/application.rb b/app/controllers/application.rb index df94000be..729384096 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -11,33 +11,31 @@ class ApplicationController < ActionController::Base end def authorize(realm='Web Password', errormessage="Couldn't authenticate you") - unless request.get? - username, passwd = get_auth_data # parse from headers - # authenticate per-scheme - if username.nil? - @user = nil # no authentication provided - perhaps first connect (client should retry after 401) - elsif username == 'token' - @user = User.authenticate_token(passwd) # preferred - random token for user from db, passed in basic auth - else - @user = User.authenticate(username, passwd) # basic auth - end - - # handle authenticate pass/fail - if @user - # user exists and password is correct ... horray! - if @user.methods.include? 'lastlogin' # note last login - @session['lastlogin'] = user.lastlogin - @user.last.login = Time.now - @user.save() - @session["User.id"] = @user.id - end - else - # no auth, the user does not exist or the password was wrong - response.headers["Status"] = "Unauthorized" - response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\"" - render_text(errormessage, 401) # :unauthorized - end + username, passwd = get_auth_data # parse from headers + # authenticate per-scheme + if username.nil? + @user = nil # no authentication provided - perhaps first connect (client should retry after 401) + elsif username == 'token' + @user = User.authenticate_token(passwd) # preferred - random token for user from db, passed in basic auth + else + @user = User.authenticate(username, passwd) # basic auth end + + # handle authenticate pass/fail + if @user + # user exists and password is correct ... horray! + if @user.methods.include? 'lastlogin' # note last login + @session['lastlogin'] = user.lastlogin + @user.last.login = Time.now + @user.save() + @session["User.id"] = @user.id + end + else + # no auth, the user does not exist or the password was wrong + response.headers["Status"] = "Unauthorized" + response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\"" + render_text(errormessage, 401) # :unauthorized + end end # Report and error to the user @@ -46,7 +44,7 @@ class ApplicationController < ActionController::Base # phrase from that, we can also put the error message into the status # message. For now, rails won't let us) def report_error(message) - render :nothing => true, :status => 400 + render :nothing => true, :status => :bad_request # Todo: some sort of escaping of problem characters in the message response.headers['Error'] = message end diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb index 2dbf8a0cb..b072fc0e3 100644 --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@ -47,9 +47,7 @@ class DiaryEntryController < ApplicationController rss.add(latitude, longitude, entry.title, url_for({:controller => 'diary_entry', :action => 'list', :id => entry.id, :display_name => entry.user.display_name}), entry.body, entry.created_at) end - response.headers["Content-Type"] = 'application/rss+xml' - - render :text => rss.to_s + render :text => rss.to_s, :content_type => "application/rss+xml" end end diff --git a/app/controllers/message_controller.rb b/app/controllers/message_controller.rb index e9ad1968c..a712931c5 100644 --- a/app/controllers/message_controller.rb +++ b/app/controllers/message_controller.rb @@ -1,6 +1,6 @@ class MessageController < ApplicationController layout 'site' - # before_filter :authorize + before_filter :authorize_web before_filter :require_user diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 6815470b8..7a841769b 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -1,62 +1,86 @@ class NodeController < ApplicationController require 'xml/libxml' - before_filter :authorize + before_filter :authorize, :only => [:create, :update, :destroy] after_filter :compress_output def create - response.headers["Content-Type"] = 'text/xml' if request.put? node = Node.from_xml(request.raw_post, true) if node node.user_id = @user.id - node.visible = 1 + node.visible = true + if node.save_with_history - render :text => node.id.to_s + render :text => node.id.to_s, :content_type => "text/plain" else - render :nothing => true, :status => 500 + render :nothing => true, :status => :internal_server_error end - return - else - render :nothing => true, :status => 400 # if we got here the doc didnt parse - return + render :nothing => true, :status => :bad_request end + else + render :nothing => true, :status => :method_not_allowed end - - render :nothing => true, :status => 500 # something went very wrong end - def rest - response.headers["Content-Type"] = 'text/xml' - unless Node.exists?(params[:id]) - render :nothing => true, :status => 404 - return + def read + begin + node = Node.find(params[:id]) + + if node.visible + render :text => node.to_xml.to_s, :content_type => "text/xml" + else + render :nothing => true, :status => :gone + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end + end - node = Node.find(params[:id]) + def update + begin + node = Node.find(params[:id]) - case request.method + if node.visible + new_node = Node.from_xml(request.raw_post) - when :get - unless node - render :nothing => true, :status => 500 - return - end + if new_node and new_node.id == node.id + node.timestamp = Time.now + node.user_id = @user.id + + node.latitude = new_node.latitude + node.longitude = new_node.longitude + node.tags = new_node.tags - unless node.visible - render :nothing => true, :status => 410 - return + if node.save_with_history + render :nothing => true + else + render :nothing => true, :status => :internal_server_error + end + else + render :nothing => true, :status => :bad_request + end + else + render :nothing => true, :status => :gone end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end + end - render :text => node.to_xml.to_s - return + def delete + begin + node = Node.find(params[:id]) - when :delete if node.visible if Segment.find(:first, :conditions => [ "visible = 1 and (node_a = ? or node_b = ?)", node.id, node.id]) - render :nothing => true, :status => HTTP_PRECONDITION_FAILED + render :nothing => true, :status => :precondition_failed else node.user_id = @user.id node.visible = 0 @@ -64,45 +88,28 @@ class NodeController < ApplicationController render :nothing => true end else - render :nothing => true, :status => 410 + render :nothing => true, :status => :gone end - - when :put - new_node = Node.from_xml(request.raw_post) - - if new_node - node.timestamp = Time.now - node.user_id = @user.id - - node.latitude = new_node.latitude - node.longitude = new_node.longitude - node.tags = new_node.tags - - if node.id == new_node.id and node.save_with_history - render :nothing => true - else - render :nothing => true, :status => 500 - end - else - render :nothing => true, :status => 400 # if we got here the doc didnt parse - end - return + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end - end def nodes - response.headers["Content-Type"] = 'text/xml' - ids = params['nodes'].split(',').collect {|n| n.to_i } + ids = params['nodes'].split(',').collect { |n| n.to_i } + if ids.length > 0 - nodelist = Node.find(ids) - doc = get_xml_doc - nodelist.each do |node| + doc = OSM::API.new.get_xml_doc + + Node.find(ids).each do |node| doc.root << node.to_xml_node end - render :text => doc.to_s + + render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => 400 + render :nothing => true, :status => :bad_request end end end diff --git a/app/controllers/old_node_controller.rb b/app/controllers/old_node_controller.rb index d8a833b1d..5734fefd7 100644 --- a/app/controllers/old_node_controller.rb +++ b/app/controllers/old_node_controller.rb @@ -1,22 +1,21 @@ class OldNodeController < ApplicationController + require 'xml/libxml' def history - response.headers["Content-Type"] = 'text/xml' - node = Node.find(params[:id]) + begin + node = Node.find(params[:id]) - unless node - render :nothing => true, :staus => 404 - return - end + doc = OSM::API.new.get_xml_doc - doc = OSM::API.new.get_xml_doc + node.old_nodes.each do |old_node| + doc.root << old_node.to_xml_node + end - node.old_nodes.each do |old_node| - 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 - - render :text => doc.to_s end - - end diff --git a/app/controllers/old_segment_controller.rb b/app/controllers/old_segment_controller.rb index 1f64987ed..5d11ec906 100644 --- a/app/controllers/old_segment_controller.rb +++ b/app/controllers/old_segment_controller.rb @@ -1,20 +1,21 @@ class OldSegmentController < ApplicationController + require 'xml/libxml' def history - response.headers["Content-Type"] = 'text/xml' - segment = Segment.find(params[:id]) + begin + segment = Segment.find(params[:id]) - unless segment - render :nothing => true, :staus => 404 - return - end + doc = OSM::API.new.get_xml_doc - doc = OSM::API.new.get_xml_doc + segment.old_segments.each do |old_segment| + doc.root << old_segment.to_xml_node + end - segment.old_segments.each do |old_segment| - doc.root << old_segment.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 - - render :text => doc.to_s end end diff --git a/app/controllers/old_way_controller.rb b/app/controllers/old_way_controller.rb index a253e3e20..6b0c9dd2e 100644 --- a/app/controllers/old_way_controller.rb +++ b/app/controllers/old_way_controller.rb @@ -1,21 +1,21 @@ class OldWayController < ApplicationController - def history - response.headers["Content-Type"] = 'text/xml' - way = Way.find(params[:id]) + require 'xml/libxml' - unless way - render :nothing => true, :staus => 404 - return - end + def history + begin + way = Way.find(params[:id]) - doc = OSM::API.new.get_xml_doc + doc = OSM::API.new.get_xml_doc - way.old_ways.each do |old_way| - doc.root << old_way.to_xml_node - end + way.old_ways.each do |old_way| + doc.root << old_way.to_xml_node + end - render :text => doc.to_s + 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/search_controller.rb b/app/controllers/search_controller.rb index cb97624f9..70bf23824 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -21,8 +21,6 @@ class SearchController < ApplicationController def do_search(do_ways,do_segments,do_nodes) - response.headers["Content-Type"] = 'text/xml' - type = params['type'] value = params['value'] unless type or value @@ -121,6 +119,6 @@ class SearchController < ApplicationController doc.root << way.to_xml_node() end - render :text => doc.to_s + render :text => doc.to_s, :content_type => "text/xml" end end diff --git a/app/controllers/segment_controller.rb b/app/controllers/segment_controller.rb index bc41b85a3..31738ce13 100644 --- a/app/controllers/segment_controller.rb +++ b/app/controllers/segment_controller.rb @@ -1,135 +1,148 @@ class SegmentController < ApplicationController require 'xml/libxml' - before_filter :authorize + before_filter :authorize, :only => [:create, :update, :destroy] after_filter :compress_output def create - response.headers["Content-Type"] = 'text/xml' if request.put? segment = Segment.from_xml(request.raw_post, true) if segment - segment.user_id = @user.id - - segment.from_node = Node.find(segment.node_a.to_i) - segment.to_node = Node.find(segment.node_b.to_i) - - if segment.from_node == segment.to_node - render :nothing => true, :status => HTTP_EXPECTATION_FAILED - return - end - - unless segment.preconditions_ok? # are the nodes visible? - render :nothing => true, :status => HTTP_PRECONDITION_FAILED - return - end - - if segment.save_with_history - render :text => segment.id.to_s + if segment.node_a == segment.node_b + render :nothing => true, :status => :expectation_failed + elsif !segment.preconditions_ok? + render :nothing => true, :status => :precondition_failed else - render :nothing => true, :status => 500 + segment.user_id = @user.id + segment.from_node = Node.find(segment.node_a.to_i) + segment.to_node = Node.find(segment.node_b.to_i) + + if segment.save_with_history + render :text => segment.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :internal_server_error + end end - return else - render :nothing => true, :status => 400 # if we got here the doc didnt parse - return + render :nothing => true, :status => :bad_request end + else + render :nothing => true, :status => :method_not_allowed end - - render :nothing => true, :status => 500 # something went very wrong end - def rest - response.headers["Content-Type"] = 'text/xml' - unless Segment.exists?(params[:id]) - render :nothing => true, :status => 404 - return - end - - segment = Segment.find(params[:id]) + def read + begin + segment = Segment.find(params[:id]) - case request.method + if segment.visible + render :text => segment.to_xml.to_s, :content_type => "text/xml" + else + render :nothing => true, :status => :gone + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end + end - when :get - render :text => segment.to_xml.to_s - return + def update + begin + segment = Segment.find(params[:id]) - when :delete if segment.visible - if WaySegment.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_segments.id", :conditions => [ "current_ways.visible = 1 AND current_way_segments.segment_id = ?", segment.id ]) - render :nothing => true, :status => HTTP_PRECONDITION_FAILED + new_segment = Segment.from_xml(request.raw_post) + + if new_segment and new_segment.id == segment.id + if new_segment.node_a == new_segment.node_b + render :nothing => true, :status => :expectation_failed + elsif !new_segment.preconditions_ok? + render :nothing => true, :status => :precondition_failed + else + segment.timestamp = Time.now + segment.user_id = @user.id + segment.node_a = new_segment.node_a + segment.node_b = new_segment.node_b + segment.tags = new_segment.tags + segment.visible = new_segment.visible + + if segment.save_with_history + render :nothing => true + else + render :nothing => true, :status => :internal_server_error + end + end else - segment.user_id = @user.id - segment.visible = 0 - segment.save_with_history - render :nothing => true + render :nothing => true, :status => :bad_request end else - render :nothing => true, :status => 410 + render :nothing => true, :status => :gone end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end + end - when :put - new_segment = Segment.from_xml(request.raw_post) - - if new_segment - if new_segment.node_a == new_segment.node_b - render :nothing => true, :status => HTTP_EXPECTATION_FAILED - return - end - - unless new_segment.preconditions_ok? # are the nodes visible? - render :nothing => true, :status => HTTP_PRECONDITION_FAILED - return - end - - segment.timestamp = Time.now - segment.user_id = @user.id - segment.node_a = new_segment.node_a - segment.node_b = new_segment.node_b - segment.tags = new_segment.tags - segment.visible = new_segment.visible + def delete + begin + segment = Segment.find(params[:id]) - if segment.id == new_segment.id and segment.save_with_history - render :nothing => true + if segment.visible + if WaySegment.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_segments.id", :conditions => [ "current_ways.visible = 1 AND current_way_segments.segment_id = ?", segment.id ]) + render :nothing => true, :status => :precondition_failed else - render :nothing => true, :status => 500 + segment.user_id = @user.id + segment.visible = 0 + + if segment.save_with_history + render :nothing => true + else + render :nothing => true, :status => :internal_server_error + end end else - render :nothing => true, :status => 400 # if we got here the doc didnt parse + render :nothing => true, :status => :gone end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end - end def segments - response.headers["Content-Type"] = 'text/xml' - ids = params['segments'].split(',').collect {|s| s.to_i } + ids = params['segments'].split(',').collect { |s| s.to_i } + if ids.length > 0 - segmentlist = Segment.find(ids) doc = OSM::API.new.get_xml_doc - segmentlist.each do |segment| + + Segment.find(ids).each do |segment| doc.root << segment.to_xml_node end - render :text => doc.to_s + + render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => 400 + render :nothing => true, :status => :bad_request end end def segments_for_node - response.headers["Content-Type"] = 'text/xml' segmentids = Segment.find(:all, :conditions => ['node_a = ? OR node_b = ?', params[:id], params[:id]]).collect { |s| s.id }.uniq + if segmentids.length > 0 - segmentlist = Segment.find(segmentids) doc = OSM::API.new.get_xml_doc - segmentlist.each do |segment| + + Segment.find(segmentids).each do |segment| doc.root << segment.to_xml_node - end - render :text => doc.to_s + end + + render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => 400 + render :nothing => true, :status => :bad_request end end - end diff --git a/app/controllers/swf_controller.rb b/app/controllers/swf_controller.rb index a93abc8dd..a58e899e0 100644 --- a/app/controllers/swf_controller.rb +++ b/app/controllers/swf_controller.rb @@ -139,8 +139,7 @@ class SwfController < ApplicationController m=packRect(bounds_left,bounds_right,bounds_bottom,bounds_top) + 0.chr + 12.chr + packUI16(1) + m m='FWS' + 6.chr + packUI32(m.length+8) + m - response.headers["Content-Type"]="application/x-shockwave-flash" - render :text=>m + render :text => m, :content_type => "application/x-shockwave-flash" end private diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index c5552f39d..37c74f8e0 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -95,7 +95,7 @@ class TraceController < ApplicationController @trace = Trace.find(params[:id]) unless @trace.public if @user - render :nothing, :status => 401 if @trace.user.id != @user.id + render :nothing, :status => :forbidden if @trace.user.id != @user.id end end end @@ -126,7 +126,7 @@ class TraceController < ApplicationController if trace and (trace.public? or (@user and @user == trace.user)) send_file(trace.trace_name, :filename => "#{trace.id}.gpx", :type => trace.mime_type, :disposition => 'attachment') else - render :nothing, :status => 404 + render :nothing, :status => :not_found end end @@ -150,34 +150,55 @@ class TraceController < ApplicationController rss.add(trace.latitude, trace.longitude, trace.name, url_for({:controller => 'trace', :action => 'view', :id => trace.id, :display_name => trace.user.display_name}), " 'icon', :id => trace.id, :user_login => trace.user.display_name})}'> GPX file with #{trace.size} points from #{trace.user.display_name}", trace.timestamp) end - response.headers["Content-Type"] = 'application/rss+xml' - - render :text => rss.to_s + render :text => rss.to_s, :content_type => "application/rss+xml" end def picture - trace = Trace.find(params[:id]) - if trace and (trace.public? or (@user and @user == trace.user)) - send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline') - else - render :nothing, :status => 404 + begin + trace = Trace.find(params[:id]) + + if trace.public? or (@user and @user == trace.user) + send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline') + else + render :nothing, :status => :forbidden + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end end def icon - trace = Trace.find(params[:id]) - if trace and (trace.public? or (@user and @user == trace.user)) - send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline') - else - render :nothing, :status => 404 + begin + trace = Trace.find(params[:id]) + + if trace.public? or (@user and @user == trace.user) + send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline') + else + render :nothing, :status => :forbidden + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end end def api_details - trace = Trace.find(params[:id]) - doc = OSM::API.new.get_xml_doc - doc.root << trace.to_xml_node() if trace.public? or trace.user == @user - render :text => doc.to_s + begin + trace = Trace.find(params[:id]) + + if trace.public? or trace.user == @user + render :text => trace.to_xml.to_s, :content_type => "text/xml" + else + render :nothing => true, :status => :forbidden + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end end def api_data @@ -205,8 +226,7 @@ class TraceController < ApplicationController 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." render :nothing => true else - render :nothing => true, :status => 400 # er FIXME what fricking code to return? + render :nothing => true, :status => :internal_server_error end - end end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index b1a37872b..9972ca3ee 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -165,7 +165,7 @@ class UserController < ApplicationController @user.save! render :nothing => true else - render :status => 400, :nothing => true + render :nothing => true, :status => :method_not_allowed end end diff --git a/app/controllers/way_controller.rb b/app/controllers/way_controller.rb index dca9241a6..42219d9b0 100644 --- a/app/controllers/way_controller.rb +++ b/app/controllers/way_controller.rb @@ -1,154 +1,172 @@ class WayController < ApplicationController require 'xml/libxml' - before_filter :authorize + before_filter :authorize, :only => [:create, :update, :destroy] after_filter :compress_output def create - response.headers["Content-Type"] = 'text/xml' if request.put? way = Way.from_xml(request.raw_post, true) if way - way.user_id = @user.id - - unless way.preconditions_ok? # are the segments (and their nodes) visible? - render :nothing => true, :status => HTTP_PRECONDITION_FAILED - return - end - - if way.save_with_history - render :text => way.id.to_s + if !way.preconditions_ok? + render :nothing => true, :status => :precondition_failed else - render :nothing => true, :status => 500 + way.user_id = @user.id + + if way.save_with_history + render :text => way.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :internal_server_error + end end - return else - render :nothing => true, :status => 400 # if we got here the doc didnt parse - return + render :nothing => true, :status => :bad_request end + else + render :nothing => true, :status => :method_not_allowed end - - render :nothing => true, :status => 500 # something went very wrong end - def full - unless Way.exists?(params[:id]) - render :nothing => true, :status => 404 - return - end - - way = Way.find(params[:id]) + def read + begin + way = Way.find(params[:id]) - unless way.visible - render :nothing => true, :status => 410 - return + if way.visible + render :text => way.to_xml.to_s, :content_type => "text/xml" + else + render :nothing => true, :status => :gone + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end + end - # In future, we might want to do all the data fetch in one step - seg_ids = way.segs + [-1] - segments = Segment.find_by_sql "select * from current_segments where visible = 1 and id IN (#{seg_ids.join(',')})" - - node_ids = segments.collect {|segment| segment.node_a } - node_ids += segments.collect {|segment| segment.node_b } - node_ids += [-1] - nodes = Node.find(:all, :conditions => "visible = 1 AND id IN (#{node_ids.join(',')})") + def update + begin + way = Way.find(params[:id]) - # Render - doc = OSM::API.new.get_xml_doc - nodes.each do |node| - doc.root << node.to_xml_node() - end - segments.each do |segment| - doc.root << segment.to_xml_node() + if way.visible + new_way = Way.from_xml(request.raw_post) + + if new_way and new_way.id == way.id + if !new_way.preconditions_ok? + render :nothing => true, :status => :precondition_failed + else + way.user_id = @user.id + way.tags = new_way.tags + way.segs = new_way.segs + way.timestamp = new_way.timestamp + way.visible = true + + if way.save_with_history + render :nothing => true + else + render :nothing => true, :status => :internal_server_error + end + end + else + render :nothing => true, :status => :bad_request + end + else + render :nothing => true, :status => :gone + end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end - doc.root << way.to_xml_node() - - render :text => doc.to_s end - def rest - response.headers["Content-Type"] = 'text/xml' - unless Way.exists?(params[:id]) - render :nothing => true, :status => 404 - return - end - - way = Way.find(params[:id]) + def delete + begin + way = Way.find(params[:id]) - case request.method - - when :get - unless way.visible - render :nothing => true, :status => 410 - return - end - render :text => way.to_xml.to_s - - when :delete if way.visible way.user_id = @user.id way.visible = false - way.save_with_history - render :nothing => true + + if way.save_with_history + render :nothing => true + else + render :nothing => true, :status => :internal_server_error + end else - render :nothing => true, :status => 410 + render :nothing => true, :status => :gone end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error + end + end - when :put - new_way = Way.from_xml(request.raw_post) + def full + begin + way = Way.find(params[:id]) - if new_way - unless new_way.preconditions_ok? # are the segments (and their nodes) visible? - render :nothing => true, :status => HTTP_PRECONDITION_FAILED - return + if way.visible + # In future, we might want to do all the data fetch in one step + seg_ids = way.segs + [-1] + segments = Segment.find_by_sql "select * from current_segments where visible = 1 and id IN (#{seg_ids.join(',')})" + + node_ids = segments.collect {|segment| segment.node_a } + node_ids += segments.collect {|segment| segment.node_b } + node_ids += [-1] + nodes = Node.find(:all, :conditions => "visible = 1 AND id IN (#{node_ids.join(',')})") + + # Render + doc = OSM::API.new.get_xml_doc + nodes.each do |node| + doc.root << node.to_xml_node() end - - way.user_id = @user.id - way.tags = new_way.tags - way.segs = new_way.segs - way.timestamp = new_way.timestamp - way.visible = true - - if way.id == new_way.id and way.save_with_history - render :nothing => true - else - render :nothing => true, :status => 500 + segments.each do |segment| + doc.root << segment.to_xml_node() end + doc.root << way.to_xml_node() + + render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => 400 # if we got here the doc didnt parse + render :nothing => true, :status => :gone end + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue + render :nothing => true, :status => :internal_server_error end end def ways - response.headers["Content-Type"] = 'text/xml' - ids = params['ways'].split(',').collect {|w| w.to_i } + ids = params['ways'].split(',').collect { |w| w.to_i } + if ids.length > 0 - waylist = Way.find(ids) doc = OSM::API.new.get_xml_doc - waylist.each do |way| + + Way.find(ids).each do |way| doc.root << way.to_xml_node end - render :text => doc.to_s + + render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => 400 + render :nothing => true, :status => :bad_request end end def ways_for_segment - response.headers["Content-Type"] = 'text/xml' wayids = WaySegment.find(:all, :conditions => ['segment_id = ?', params[:id]]).collect { |ws| ws.id }.uniq + if wayids.length > 0 - waylist = Way.find(wayids) doc = OSM::API.new.get_xml_doc - waylist.each do |way| + + Way.find(wayids).each do |way| doc.root << way.to_xml_node end - render :text => doc.to_s + + render :text => doc.to_s, :content_type => "text/xml" else - render :nothing => true, :status => 400 + render :nothing => true, :status => :bad_request end end - end diff --git a/app/models/trace.rb b/app/models/trace.rb index 2d4e9ef8a..92cc39678 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -76,6 +76,12 @@ class Trace < ActiveRecord::Base return `file -bi #{trace_name}`.chomp 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 'gpx_file' el1['id'] = self.id.to_s diff --git a/config/routes.rb b/config/routes.rb index d0142ac20..e8113bd51 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,19 +4,25 @@ ActionController::Routing::Routes.draw do |map| map.connect "api/#{API_VERSION}/node/create", :controller => 'node', :action => 'create' map.connect "api/#{API_VERSION}/node/:id/segments", :controller => 'segment', :action => 'segments_for_node', :id => /\d+/ map.connect "api/#{API_VERSION}/node/:id/history", :controller => 'old_node', :action => 'history', :id => /\d+/ - map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'rest', :id => /\d+/ + map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'read', :id => /\d+/, :conditions => { :method => :get } + map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'update', :id => /\d+/, :conditions => { :method => :put } + map.connect "api/#{API_VERSION}/node/:id", :controller => 'node', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete } map.connect "api/#{API_VERSION}/nodes", :controller => 'node', :action => 'nodes', :id => nil map.connect "api/#{API_VERSION}/segment/create", :controller => 'segment', :action => 'create' map.connect "api/#{API_VERSION}/segment/:id/ways", :controller => 'way', :action => 'ways_for_segment', :id => /\d+/ map.connect "api/#{API_VERSION}/segment/:id/history", :controller => 'old_segment', :action => 'history', :id => /\d+/ - map.connect "api/#{API_VERSION}/segment/:id", :controller => 'segment', :action => 'rest', :id => /\d+/ + map.connect "api/#{API_VERSION}/segment/:id", :controller => 'segment', :action => 'read', :id => /\d+/, :conditions => { :method => :get } + map.connect "api/#{API_VERSION}/segment/:id", :controller => 'segment', :action => 'update', :id => /\d+/, :conditions => { :method => :put } + map.connect "api/#{API_VERSION}/segment/:id", :controller => 'segment', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete } map.connect "api/#{API_VERSION}/segments", :controller => 'segment', :action => 'segments', :id => nil map.connect "api/#{API_VERSION}/way/create", :controller => 'way', :action => 'create' map.connect "api/#{API_VERSION}/way/:id/history", :controller => 'old_way', :action => 'history', :id => /\d+/ map.connect "api/#{API_VERSION}/way/:id/full", :controller => 'way', :action => 'full', :id => /\d+/ - map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'rest', :id => /\d+/ + map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'read', :id => /\d+/, :conditions => { :method => :get } + map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'update', :id => /\d+/, :conditions => { :method => :put } + map.connect "api/#{API_VERSION}/way/:id", :controller => 'way', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete } map.connect "api/#{API_VERSION}/ways", :controller => 'way', :action => 'ways', :id => nil map.connect "api/#{API_VERSION}/map", :controller => 'api', :action => 'map' -- 2.39.5