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

@@ -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
 	}

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 => "<osm version='0.4'></osm>"
+      render :text => "<osm version='0.4'></osm>", :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

app/controllers/application.rb

@@ -11,32 +11,30 @@ 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
-      username, passwd = get_auth_data # parse from headers
+    # authenticate per-scheme
-      # authenticate per-scheme
+    if username.nil?
-      if username.nil?
+      @user = nil # no authentication provided - perhaps first connect (client should retry after 401)
-        @user = nil # no authentication provided - perhaps first connect (client should retry after 401)
+    elsif username == 'token' 
-      elsif username == 'token' 
+      @user = User.authenticate_token(passwd) # preferred - random token for user from db, passed in basic auth
-        @user = User.authenticate_token(passwd) # preferred - random token for user from db, passed in basic auth
+    else
-      else
+      @user = User.authenticate(username, passwd) # basic auth
-        @user = User.authenticate(username, passwd) # basic auth
+    end
-      end
 
-      # handle authenticate pass/fail
+    # handle authenticate pass/fail
-      if @user
+    if @user
-        # user exists and password is correct ... horray! 
+      # user exists and password is correct ... horray! 
-        if @user.methods.include? 'lastlogin'         # note last login 
+      if @user.methods.include? 'lastlogin'         # note last login 
-          @session['lastlogin'] = user.lastlogin 
+        @session['lastlogin'] = user.lastlogin 
-          @user.last.login = Time.now 
+        @user.last.login = Time.now 
-          @user.save() 
+        @user.save() 
-          @session["User.id"] = @user.id 
+        @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             
+    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 
 
@@ -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

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, :content_type => "application/rss+xml"
-
-    render :text => rss.to_s
   end
 
 end

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
 

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
+        render :nothing => true, :status => :bad_request
-        return
       end
+    else
+      render :nothing => true, :status => :method_not_allowed
     end
-
-    render :nothing => true, :status => 500 # something went very wrong
   end
 
-  def rest
+  def read
-    response.headers["Content-Type"] = 'text/xml'
+    begin
-    unless Node.exists?(params[:id])
+      node = Node.find(params[:id])
-      render :nothing => true, :status => 404
+
-      return
+      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
+        if new_node and new_node.id == node.id
-      unless node
+          node.timestamp = Time.now
-        render :nothing => true, :status => 500
+          node.user_id = @user.id
-        return
+
+          node.latitude = new_node.latitude 
+          node.longitude = new_node.longitude
+          node.tags = new_node.tags
+
+          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
 
-      unless node.visible
+  def delete
-        render :nothing => true, :status => 410
+    begin
-        return
+      node = Node.find(params[:id])
-      end
 
-      render :text => node.to_xml.to_s
-      return
-
-    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
-
+    rescue ActiveRecord::RecordNotFound
-    when :put
+      render :nothing => true, :status => :not_found
-      new_node = Node.from_xml(request.raw_post)
+    rescue
-
+      render :nothing => true, :status => :internal_server_error
-      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
     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 = OSM::API.new.get_xml_doc
-      doc = get_xml_doc
+
-      nodelist.each do |node|
+      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

app/controllers/old_node_controller.rb

@@ -1,22 +1,21 @@
 class OldNodeController < ApplicationController
+  require 'xml/libxml'
 
   def history
-    response.headers["Content-Type"] = 'text/xml'
+    begin
-    node = Node.find(params[:id])
+      node = Node.find(params[:id])
 
-    unless node
+      doc = OSM::API.new.get_xml_doc
-      render :nothing => true, :staus => 404
+
-      return
+      node.old_nodes.each do |old_node|
+        doc.root << old_node.to_xml_node
+      end
+
+      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
-
-    doc = OSM::API.new.get_xml_doc
-
-    node.old_nodes.each do |old_node|
-      doc.root << old_node.to_xml_node
-    end
-
-    render :text => doc.to_s
   end
-
-
 end

app/controllers/old_segment_controller.rb

@@ -1,20 +1,21 @@
 class OldSegmentController < ApplicationController
+  require 'xml/libxml'
 
   def history
-    response.headers["Content-Type"] = 'text/xml'
+    begin
-    segment = Segment.find(params[:id])
+      segment = Segment.find(params[:id])
 
-    unless segment
+      doc = OSM::API.new.get_xml_doc
-      render :nothing => true, :staus => 404
+
-      return
+      segment.old_segments.each do |old_segment|
+        doc.root << old_segment.to_xml_node
+      end
+
+     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
-
-    doc = OSM::API.new.get_xml_doc
-
-    segment.old_segments.each do |old_segment|
-      doc.root << old_segment.to_xml_node
-    end
-
-    render :text => doc.to_s
   end
 end

app/controllers/old_way_controller.rb

@@ -1,21 +1,21 @@
 class OldWayController < ApplicationController
+  require 'xml/libxml'
+
   def history
-    response.headers["Content-Type"] = 'text/xml'
+    begin
-    way = Way.find(params[:id])
+      way = Way.find(params[:id])
     
-    unless way
+      doc = OSM::API.new.get_xml_doc
-      render :nothing => true, :staus => 404
+
-      return
+      way.old_ways.each do |old_way|
+        doc.root << old_way.to_xml_node
+     end
+
+      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
-    
-    doc = OSM::API.new.get_xml_doc
-
-    way.old_ways.each do |old_way|
-      doc.root << old_way.to_xml_node
-    end
-
-    render :text => doc.to_s
   end
-
-
 end

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

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
+        if segment.node_a == segment.node_b
-
+          render :nothing => true, :status => :expectation_failed
-        segment.from_node = Node.find(segment.node_a.to_i)
+        elsif !segment.preconditions_ok?
-        segment.to_node = Node.find(segment.node_b.to_i)
+          render :nothing => true, :status => :precondition_failed
-          
-        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
         else
-          render :nothing => true, :status => 500
+          segment.user_id = @user.id
-        end
+          segment.from_node = Node.find(segment.node_a.to_i)
-        return
+          segment.to_node = Node.find(segment.node_b.to_i)
-      else
-        render :nothing => true, :status => 400 # if we got here the doc didnt parse
-        return
-      end
-    end
 
-    render :nothing => true, :status => 500 # something went very wrong
+          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
+      else
+        render :nothing => true, :status => :bad_request
+      end
+    else
+      render :nothing => true, :status => :method_not_allowed
+    end
   end
 
-  def rest
+  def read
-    response.headers["Content-Type"] = 'text/xml'
+    begin
-    unless Segment.exists?(params[:id])
+      segment = Segment.find(params[:id])
-      render :nothing => true, :status => 404
+
-      return
+      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
 
-    segment = Segment.find(params[:id])
+  def update
+    begin
+      segment = Segment.find(params[:id])
 
-    case request.method
+      if segment.visible
+        new_segment = Segment.from_xml(request.raw_post)
 
-    when :get
+        if new_segment and new_segment.id == segment.id
-      render :text => segment.to_xml.to_s
+          if new_segment.node_a == new_segment.node_b
-      return
+            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
+          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
+
+  def delete
+    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
+          render :nothing => true, :status => :precondition_failed
         else
           segment.user_id = @user.id
           segment.visible = 0
-          segment.save_with_history
+
-          render :nothing => true
+          if segment.save_with_history
+            render :nothing => true
+          else
+            render :nothing => true, :status => :internal_server_error
+          end
         end
       else
-        render :nothing => true, :status => 410
+        render :nothing => true, :status => :gone
-      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
-
-        if segment.id == new_segment.id and segment.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
+    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
+
+      render :text => doc.to_s, :content_type => "text/xml"
     else
-      render :nothing => true, :status => 400
+      render :nothing => true, :status => :bad_request
     end
   end
-
 end

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, :content_type => "application/x-shockwave-flash"
-		render :text=>m
 	end
 
 	private

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}), "<img src='#{url_for({:controller => 'trace', :action => '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, :content_type => "application/rss+xml"
-
-    render :text => rss.to_s
   end
 
   def picture
-    trace = Trace.find(params[:id])
+    begin
-    if trace and (trace.public? or (@user and @user == trace.user))
+      trace = Trace.find(params[:id])
-      send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline')
+
-    else
+      if trace.public? or (@user and @user == trace.user)
-      render :nothing, :status => 404
+        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])
+    begin
-    if trace and (trace.public? or (@user and @user == trace.user))
+      trace = Trace.find(params[:id])
-      send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline')
+
-    else
+      if trace.public? or (@user and @user == trace.user)
-      render :nothing, :status => 404
+        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])
+    begin
-    doc = OSM::API.new.get_xml_doc
+      trace = Trace.find(params[:id])
-    doc.root << trace.to_xml_node() if trace.public? or trace.user == @user
+
-    render :text => doc.to_s
+      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

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
 

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
+        if !way.preconditions_ok?
-
+          render :nothing => true, :status => :precondition_failed
-        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
         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
+        render :nothing => true, :status => :bad_request
-        return
       end
+    else
+      render :nothing => true, :status => :method_not_allowed
     end
-
-    render :nothing => true, :status => 500 # something went very wrong
   end
 
-  def full
+  def read
-    unless Way.exists?(params[:id])
+    begin
-      render :nothing => true, :status => 404
+      way = Way.find(params[:id])
-      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
-
-    way = Way.find(params[:id])
-
-    unless way.visible
-      render :nothing => true, :status => 410
-      return
-    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(',')})")
-
-    # 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()
-    end
-    doc.root << way.to_xml_node()
-
-    render :text => doc.to_s
   end
 
-  def rest
+  def update
-    response.headers["Content-Type"] = 'text/xml'
+    begin
-    unless Way.exists?(params[:id])
+      way = Way.find(params[:id])
-      render :nothing => true, :status => 404
-      return
-    end
 
-    way = Way.find(params[:id])
+      if way.visible
+        new_way = Way.from_xml(request.raw_post)
 
-    case request.method
+        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
 
-    when :get
+            if way.save_with_history
-      unless way.visible
+              render :nothing => true
-        render :nothing => true, :status => 410
+            else
-        return
+              render :nothing => true, :status => :internal_server_error
+            end
+          end
+        else
+          render :nothing => true, :status => :bad_request
+        end
+      else
+        render :nothing => true, :status => :gone
       end
-      render :text => way.to_xml.to_s
+    rescue ActiveRecord::RecordNotFound
+      render :nothing => true, :status => :not_found
+    rescue
+      render :nothing => true, :status => :internal_server_error
+    end
+  end
+
+  def delete
+    begin
+      way = Way.find(params[:id])
 
-    when :delete
       if way.visible
         way.user_id = @user.id
         way.visible = false
-        way.save_with_history
-        render :nothing => true
-      else
-        render :nothing => true, :status => 410
-      end
 
-    when :put
+        if way.save_with_history
-      new_way = Way.from_xml(request.raw_post)
-
-      if new_way
-        unless new_way.preconditions_ok? # are the segments (and their nodes) visible?
-          render :nothing => true, :status => HTTP_PRECONDITION_FAILED
-          return
-        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
+          render :nothing => true, :status => :internal_server_error
         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 full
+    begin
+      way = Way.find(params[:id])
+
+      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
+        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 => :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

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

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'