Split the rest action into sparate read, update and delete actions thus
authorTom Hughes <tom@compton.nu>
Wed, 27 Jun 2007 17:27:10 +0000 (17:27 +0000)
committerTom Hughes <tom@compton.nu>
Wed, 27 Jun 2007 17:27:10 +0000 (17:27 +0000)
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.

17 files changed:
app/controllers/amf_controller.rb
app/controllers/api_controller.rb
app/controllers/application.rb
app/controllers/diary_entry_controller.rb
app/controllers/message_controller.rb
app/controllers/node_controller.rb
app/controllers/old_node_controller.rb
app/controllers/old_segment_controller.rb
app/controllers/old_way_controller.rb
app/controllers/search_controller.rb
app/controllers/segment_controller.rb
app/controllers/swf_controller.rb
app/controllers/trace_controller.rb
app/controllers/user_controller.rb
app/controllers/way_controller.rb
app/models/trace.rb
config/routes.rb

index 72f1dd74eb9bed75047aa2beacdf6c2cb90baaaf..1049b51b61d0735cb28d31986815df51c1d1005c 100644 (file)
@@ -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
        }
index be00b9f9c8b1d39a7763008a1e246de94f49561a..1804ceb49f3f5af2c2c0b293c8c1e21a02685655 100644 (file)
@@ -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
index df94000be9e8307400587fc67d69ddb8915b52e1..72938409625315eb92b677a19aaa086bee9a773e 100644 (file)
@@ -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
index 2dbf8a0cbbc8d3373e0c29f6622182c02a186bb6..b072fc0e364e5dd58f86f05bc6e5f78c0bb31bb6 100644 (file)
@@ -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
index e9ad1968c295beb7c0afd9dd83b25e1fe06d3f2f..a712931c554f8343a74d1e17e8d281100c126501 100644 (file)
@@ -1,6 +1,6 @@
 class MessageController < ApplicationController
   layout 'site'
-  #  before_filter :authorize
+
   before_filter :authorize_web
   before_filter :require_user
 
index 6815470b8f19771599266e906b6e0da1fd88fcf5..7a841769b9df77f997972bcb7d26a6d79ba3ec22 100644 (file)
@@ -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
index d8a833b1da7de86e3045212a9b19d1a5d04228c9..5734fefd7017c4fd8bb0cd7d79fd30dd560d32e9 100644 (file)
@@ -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
index 1f64987ed6402f9647ad01bf7f14984cf775741b..5d11ec906b24e8dc3c4558433ca3f04c574aae73 100644 (file)
@@ -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
index a253e3e2051f4984e034622060c77af56cc117e7..6b0c9dd2ecd059adfa40c727499a5268bcbc6915 100644 (file)
@@ -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
index cb97624f95e984905b687d6bdef6098273ec766c..70bf23824a3e3167cf83d204e1d7a9ce9214c7d5 100644 (file)
@@ -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
index bc41b85a39a116a02dadebb660a344242603f0ba..31738ce139df4410d50e20d158edf79865714b07 100644 (file)
 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
index a93abc8ddd31987edb7d0a088a10a0424ad43a6b..a58e899e01741d0daeec900a210de98587a20a0a 100644 (file)
@@ -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
index c5552f39d07f44b2359338852049a3743727102a..37c74f8e0e1676ddc4d46b28f3d0494ac7777d63 100644 (file)
@@ -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
+    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
index b1a37872bd6922a485b080a3dc72caed3ec24219..9972ca3ee6c203dd0d6ea2ac0ca348aef82c611e 100644 (file)
@@ -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
 
index dca9241a698e490ba2d497bddbff54a5f880d237..42219d9b0fce74a0f1be6ed05a4b070d0eecf94d 100644 (file)
 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
index 2d4e9ef8ab12381a92b22cd13d7ae2b2ca6b47cb..92cc39678d767b32e209d96a86fd254063be75af 100644 (file)
@@ -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
index d0142ac204da3d4abd5815f1a15a23cbcc0475ee..e8113bd51f66a97878a0dea14877f08e78f9b18c 100644 (file)
@@ -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'