From: Tom Hughes <tom@compton.nu>
Date: Sun, 14 Apr 2019 15:09:04 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/2205'
X-Git-Tag: live~4018
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/15841e8d2ad12c538ef10f0d0782a25d1d7ff156?hp=f88d11e0e6c17a0a66dc821b9d9145bb8e13dc97

Merge remote-tracking branch 'upstream/pull/2205'
---

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 54bc8fb4c..a71d05d75 100644
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -15,7 +15,7 @@ class ApiAbility
       can [:show, :download, :query], Changeset
       can [:index, :create, :comment, :feed, :show, :search], Note
       can :index, Tracepoint
-      can [:api_users, :api_read], User
+      can [:index, :show], User
       can [:index, :show], Node
       can [:index, :show, :full, :ways_for_node], Way
       can [:index, :show, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
@@ -33,8 +33,8 @@ class ApiAbility
         can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
         can [:close, :reopen], Note
         can [:new, :create], Report
-        can [:api_create, :api_read, :api_update, :api_delete, :api_data], Trace
-        can [:api_details, :api_gpx_files], User
+        can [:create, :show, :update, :destroy, :data], Trace
+        can [:details, :gpx_files], User
         can [:read, :read_one, :update, :update_one, :delete_one], UserPreference
 
         if user.terms_agreed?
diff --git a/app/abilities/api_capability.rb b/app/abilities/api_capability.rb
index 9f59b1f24..7d8a13364 100644
--- a/app/abilities/api_capability.rb
+++ b/app/abilities/api_capability.rb
@@ -6,10 +6,10 @@ class ApiCapability
   def initialize(token)
     if Settings.status != "database_offline"
       can [:create, :comment, :close, :reopen], Note if capability?(token, :allow_write_notes)
-      can [:api_read, :api_data], Trace if capability?(token, :allow_read_gpx)
-      can [:api_create, :api_update, :api_delete], Trace if capability?(token, :allow_write_gpx)
-      can [:api_details], User if capability?(token, :allow_read_prefs)
-      can [:api_gpx_files], User if capability?(token, :allow_read_gpx)
+      can [:show, :data], Trace if capability?(token, :allow_read_gpx)
+      can [:create, :update, :destroy], Trace if capability?(token, :allow_write_gpx)
+      can [:details], User if capability?(token, :allow_read_prefs)
+      can [:gpx_files], User if capability?(token, :allow_read_gpx)
       can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs)
       can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs)
 
diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb
index 49db41076..86f1370f6 100644
--- a/app/controllers/api/traces_controller.rb
+++ b/app/controllers/api/traces_controller.rb
@@ -8,14 +8,14 @@ module Api
 
     authorize_resource
 
-    before_action :check_database_readable, :except => [:api_read, :api_data]
-    before_action :check_database_writable, :only => [:api_create, :api_update, :api_delete]
-    before_action :check_api_readable, :only => [:api_read, :api_data]
-    before_action :check_api_writable, :only => [:api_create, :api_update, :api_delete]
-    before_action :offline_redirect, :only => [:api_create, :api_delete, :api_data]
+    before_action :check_database_readable, :except => [:show, :data]
+    before_action :check_database_writable, :only => [:create, :update, :destroy]
+    before_action :check_api_readable, :only => [:show, :data]
+    before_action :check_api_writable, :only => [:create, :update, :destroy]
+    before_action :offline_redirect, :only => [:create, :destroy, :data]
     around_action :api_call_handle_error
 
-    def api_read
+    def show
       trace = Trace.visible.find(params[:id])
 
       if trace.public? || trace.user == current_user
@@ -25,7 +25,7 @@ module Api
       end
     end
 
-    def api_update
+    def update
       trace = Trace.visible.find(params[:id])
 
       if trace.user == current_user
@@ -38,7 +38,7 @@ module Api
       end
     end
 
-    def api_delete
+    def destroy
       trace = Trace.visible.find(params[:id])
 
       if trace.user == current_user
@@ -52,7 +52,7 @@ module Api
       end
     end
 
-    def api_data
+    def data
       trace = Trace.visible.find(params[:id])
 
       if trace.public? || trace.user == current_user
@@ -68,7 +68,7 @@ module Api
       end
     end
 
-    def api_create
+    def create
       tags = params[:tags] || ""
       description = params[:description] || ""
       visibility = params[:visibility]
diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index f24d50cf1..3180cabcc 100644
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -2,29 +2,29 @@ module Api
   class UsersController < ApiController
     layout "site", :except => [:api_details]
 
-    before_action :disable_terms_redirect, :only => [:api_details]
-    before_action :authorize, :only => [:api_details, :api_gpx_files]
+    before_action :disable_terms_redirect, :only => [:details]
+    before_action :authorize, :only => [:details, :gpx_files]
 
     authorize_resource
 
     before_action :check_api_readable
     around_action :api_call_handle_error
-    before_action :lookup_user_by_id, :only => [:api_read]
+    before_action :lookup_user_by_id, :only => [:show]
 
-    def api_read
+    def show
       if @user.visible?
-        render :action => :api_read, :content_type => "text/xml"
+        render :action => :show, :content_type => "text/xml"
       else
         head :gone
       end
     end
 
-    def api_details
+    def details
       @user = current_user
-      render :action => :api_read, :content_type => "text/xml"
+      render :action => :show, :content_type => "text/xml"
     end
 
-    def api_users
+    def index
       raise OSM::APIBadUserInput, "The parameter users is required, and must be of the form users=id[,id[,id...]]" unless params["users"]
 
       ids = params["users"].split(",").collect(&:to_i)
@@ -33,10 +33,10 @@ module Api
 
       @users = User.visible.find(ids)
 
-      render :action => :api_users, :content_type => "text/xml"
+      render :action => :index, :content_type => "text/xml"
     end
 
-    def api_gpx_files
+    def gpx_files
       doc = OSM::API.new.get_xml_doc
       current_user.traces.reload.each do |trace|
         doc.root << trace.to_xml_node
diff --git a/app/views/api/users/_api_user.builder b/app/views/api/users/_api_user.builder
deleted file mode 100644
index 91cd562e9..000000000
--- a/app/views/api/users/_api_user.builder
+++ /dev/null
@@ -1,44 +0,0 @@
-xml.tag! "user", :id => api_user.id,
-                 :display_name => api_user.display_name,
-                 :account_created => api_user.creation_time.xmlschema do
-  xml.tag! "description", api_user.description if api_user.description
-  if current_user && current_user == api_user
-    xml.tag! "contributor-terms", :agreed => api_user.terms_agreed.present?,
-                                  :pd => api_user.consider_pd
-  else
-    xml.tag! "contributor-terms", :agreed => api_user.terms_agreed.present?
-  end
-  xml.tag! "img", :href => user_image_url(api_user) if api_user.image.file? || api_user.image_use_gravatar
-  xml.tag! "roles" do
-    api_user.roles.each do |role|
-      xml.tag! role.role
-    end
-  end
-  xml.tag! "changesets", :count => api_user.changesets.size
-  xml.tag! "traces", :count => api_user.traces.size
-  xml.tag! "blocks" do
-    xml.tag! "received", :count => api_user.blocks.size,
-                         :active => api_user.blocks.active.size
-    if api_user.moderator?
-      xml.tag! "issued", :count => api_user.blocks_created.size,
-                         :active => api_user.blocks_created.active.size
-    end
-  end
-  if current_user && current_user == api_user
-    if api_user.home_lat && api_user.home_lon
-      xml.tag! "home", :lat => api_user.home_lat,
-                       :lon => api_user.home_lon,
-                       :zoom => api_user.home_zoom
-    end
-    if api_user.languages
-      xml.tag! "languages" do
-        api_user.languages.split(",") { |lang| xml.tag! "lang", lang }
-      end
-    end
-    xml.tag! "messages" do
-      xml.tag! "received", :count => api_user.messages.size,
-                           :unread => api_user.new_messages.size
-      xml.tag! "sent", :count => api_user.sent_messages.size
-    end
-  end
-end
diff --git a/app/views/api/users/_user.builder b/app/views/api/users/_user.builder
new file mode 100644
index 000000000..7bf8e18a5
--- /dev/null
+++ b/app/views/api/users/_user.builder
@@ -0,0 +1,44 @@
+xml.tag! "user", :id => user.id,
+                 :display_name => user.display_name,
+                 :account_created => user.creation_time.xmlschema do
+  xml.tag! "description", user.description if user.description
+  if current_user && current_user == user
+    xml.tag! "contributor-terms", :agreed => user.terms_agreed.present?,
+                                  :pd => user.consider_pd
+  else
+    xml.tag! "contributor-terms", :agreed => user.terms_agreed.present?
+  end
+  xml.tag! "img", :href => user_image_url(user) if user.image.file? || user.image_use_gravatar
+  xml.tag! "roles" do
+    user.roles.each do |role|
+      xml.tag! role.role
+    end
+  end
+  xml.tag! "changesets", :count => user.changesets.size
+  xml.tag! "traces", :count => user.traces.size
+  xml.tag! "blocks" do
+    xml.tag! "received", :count => user.blocks.size,
+                         :active => user.blocks.active.size
+    if user.moderator?
+      xml.tag! "issued", :count => user.blocks_created.size,
+                         :active => user.blocks_created.active.size
+    end
+  end
+  if current_user && current_user == user
+    if user.home_lat && user.home_lon
+      xml.tag! "home", :lat => user.home_lat,
+                       :lon => user.home_lon,
+                       :zoom => user.home_zoom
+    end
+    if user.languages
+      xml.tag! "languages" do
+        user.languages.split(",") { |lang| xml.tag! "lang", lang }
+      end
+    end
+    xml.tag! "messages" do
+      xml.tag! "received", :count => user.messages.size,
+                           :unread => user.new_messages.size
+      xml.tag! "sent", :count => user.sent_messages.size
+    end
+  end
+end
diff --git a/app/views/api/users/api_users.builder b/app/views/api/users/index.builder
similarity index 59%
rename from app/views/api/users/api_users.builder
rename to app/views/api/users/index.builder
index a3c0f4d9a..68d629099 100644
--- a/app/views/api/users/api_users.builder
+++ b/app/views/api/users/index.builder
@@ -1,4 +1,4 @@
 xml.instruct! :xml, :version => "1.0"
 xml.osm(OSM::API.new.xml_root_attributes) do |osm|
-  osm << render(:partial => "api_user", :collection => @users)
+  osm << render(:partial => "user", :collection => @users)
 end
diff --git a/app/views/api/users/api_read.builder b/app/views/api/users/show.builder
similarity index 61%
rename from app/views/api/users/api_read.builder
rename to app/views/api/users/show.builder
index 78533279c..dd5e93689 100644
--- a/app/views/api/users/api_read.builder
+++ b/app/views/api/users/show.builder
@@ -1,4 +1,4 @@
 xml.instruct! :xml, :version => "1.0"
 xml.osm(OSM::API.new.xml_root_attributes) do |osm|
-  osm << render(:partial => "api_user", :object => @user)
+  osm << render(:partial => "user", :object => @user)
 end
diff --git a/config/routes.rb b/config/routes.rb
index 32cd397aa..198932583 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -66,10 +66,10 @@ OpenStreetMap::Application.routes.draw do
     get "relations/search" => "api/search#search_relations"
     get "nodes/search" => "api/search#search_nodes"
 
-    get "user/:id" => "api/users#api_read", :id => /\d+/
-    get "user/details" => "api/users#api_details"
-    get "user/gpx_files" => "api/users#api_gpx_files"
-    get "users" => "api/users#api_users", :as => :api_users
+    get "user/:id" => "api/users#show", :id => /\d+/
+    get "user/details" => "api/users#details"
+    get "user/gpx_files" => "api/users#gpx_files"
+    get "users" => "api/users#index", :as => :api_users
 
     get "user/preferences" => "api/user_preferences#read"
     get "user/preferences/:preference_key" => "api/user_preferences#read_one"
@@ -77,12 +77,12 @@ OpenStreetMap::Application.routes.draw do
     put "user/preferences/:preference_key" => "api/user_preferences#update_one"
     delete "user/preferences/:preference_key" => "api/user_preferences#delete_one"
 
-    post "gpx/create" => "api/traces#api_create"
-    get "gpx/:id" => "api/traces#api_read", :id => /\d+/
-    put "gpx/:id" => "api/traces#api_update", :id => /\d+/
-    delete "gpx/:id" => "api/traces#api_delete", :id => /\d+/
-    get "gpx/:id/details" => "api/traces#api_read", :id => /\d+/
-    get "gpx/:id/data" => "api/traces#api_data"
+    post "gpx/create" => "api/traces#create"
+    get "gpx/:id" => "api/traces#show", :id => /\d+/
+    put "gpx/:id" => "api/traces#update", :id => /\d+/
+    delete "gpx/:id" => "api/traces#destroy", :id => /\d+/
+    get "gpx/:id/details" => "api/traces#show", :id => /\d+/
+    get "gpx/:id/data" => "api/traces#data"
 
     # AMF (ActionScript) API
     post "amf/read" => "api/amf#amf_read"
diff --git a/test/controllers/api/traces_controller_test.rb b/test/controllers/api/traces_controller_test.rb
index 0466ac996..24ff6ee66 100644
--- a/test/controllers/api/traces_controller_test.rb
+++ b/test/controllers/api/traces_controller_test.rb
@@ -13,177 +13,177 @@ module Api
     def test_routes
       assert_routing(
         { :path => "/api/0.6/gpx/create", :method => :post },
-        { :controller => "api/traces", :action => "api_create" }
+        { :controller => "api/traces", :action => "create" }
       )
       assert_routing(
         { :path => "/api/0.6/gpx/1", :method => :get },
-        { :controller => "api/traces", :action => "api_read", :id => "1" }
+        { :controller => "api/traces", :action => "show", :id => "1" }
       )
       assert_routing(
         { :path => "/api/0.6/gpx/1", :method => :put },
-        { :controller => "api/traces", :action => "api_update", :id => "1" }
+        { :controller => "api/traces", :action => "update", :id => "1" }
       )
       assert_routing(
         { :path => "/api/0.6/gpx/1", :method => :delete },
-        { :controller => "api/traces", :action => "api_delete", :id => "1" }
+        { :controller => "api/traces", :action => "destroy", :id => "1" }
       )
       assert_recognizes(
-        { :controller => "api/traces", :action => "api_read", :id => "1" },
+        { :controller => "api/traces", :action => "show", :id => "1" },
         { :path => "/api/0.6/gpx/1/details", :method => :get }
       )
       assert_routing(
         { :path => "/api/0.6/gpx/1/data", :method => :get },
-        { :controller => "api/traces", :action => "api_data", :id => "1" }
+        { :controller => "api/traces", :action => "data", :id => "1" }
       )
       assert_routing(
         { :path => "/api/0.6/gpx/1/data.xml", :method => :get },
-        { :controller => "api/traces", :action => "api_data", :id => "1", :format => "xml" }
+        { :controller => "api/traces", :action => "data", :id => "1", :format => "xml" }
       )
     end
 
     # Check getting a specific trace through the api
-    def test_api_read
+    def test_show
       public_trace_file = create(:trace, :visibility => "public")
 
       # First with no auth
-      get :api_read, :params => { :id => public_trace_file.id }
+      get :show, :params => { :id => public_trace_file.id }
       assert_response :unauthorized
 
       # Now with some other user, which should work since the trace is public
       basic_authorization create(:user).display_name, "test"
-      get :api_read, :params => { :id => public_trace_file.id }
+      get :show, :params => { :id => public_trace_file.id }
       assert_response :success
 
       # And finally we should be able to do it with the owner of the trace
       basic_authorization public_trace_file.user.display_name, "test"
-      get :api_read, :params => { :id => public_trace_file.id }
+      get :show, :params => { :id => public_trace_file.id }
       assert_response :success
     end
 
     # Check an anoymous trace can't be specifically fetched by another user
-    def test_api_read_anon
+    def test_show_anon
       anon_trace_file = create(:trace, :visibility => "private")
 
       # First with no auth
-      get :api_read, :params => { :id => anon_trace_file.id }
+      get :show, :params => { :id => anon_trace_file.id }
       assert_response :unauthorized
 
       # Now try with another user, which shouldn't work since the trace is anon
       basic_authorization create(:user).display_name, "test"
-      get :api_read, :params => { :id => anon_trace_file.id }
+      get :show, :params => { :id => anon_trace_file.id }
       assert_response :forbidden
 
       # And finally we should be able to get the trace details with the trace owner
       basic_authorization anon_trace_file.user.display_name, "test"
-      get :api_read, :params => { :id => anon_trace_file.id }
+      get :show, :params => { :id => anon_trace_file.id }
       assert_response :success
     end
 
     # Check the api details for a trace that doesn't exist
-    def test_api_read_not_found
+    def test_show_not_found
       deleted_trace_file = create(:trace, :deleted)
 
       # Try first with no auth, as it should require it
-      get :api_read, :params => { :id => 0 }
+      get :show, :params => { :id => 0 }
       assert_response :unauthorized
 
       # Login, and try again
       basic_authorization deleted_trace_file.user.display_name, "test"
-      get :api_read, :params => { :id => 0 }
+      get :show, :params => { :id => 0 }
       assert_response :not_found
 
       # Now try a trace which did exist but has been deleted
       basic_authorization deleted_trace_file.user.display_name, "test"
-      get :api_read, :params => { :id => deleted_trace_file.id }
+      get :show, :params => { :id => deleted_trace_file.id }
       assert_response :not_found
     end
 
     # Test downloading a trace through the api
-    def test_api_data
+    def test_data
       public_trace_file = create(:trace, :visibility => "public", :fixture => "a")
 
       # First with no auth
-      get :api_data, :params => { :id => public_trace_file.id }
+      get :data, :params => { :id => public_trace_file.id }
       assert_response :unauthorized
 
       # Now with some other user, which should work since the trace is public
       basic_authorization create(:user).display_name, "test"
-      get :api_data, :params => { :id => public_trace_file.id }
+      get :data, :params => { :id => public_trace_file.id }
       check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9"
 
       # And finally we should be able to do it with the owner of the trace
       basic_authorization public_trace_file.user.display_name, "test"
-      get :api_data, :params => { :id => public_trace_file.id }
+      get :data, :params => { :id => public_trace_file.id }
       check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9"
     end
 
     # Test downloading a compressed trace through the api
-    def test_api_data_compressed
+    def test_data_compressed
       identifiable_trace_file = create(:trace, :visibility => "identifiable", :fixture => "d")
 
       # Authenticate as the owner of the trace we will be using
       basic_authorization identifiable_trace_file.user.display_name, "test"
 
       # First get the data as is
-      get :api_data, :params => { :id => identifiable_trace_file.id }
+      get :data, :params => { :id => identifiable_trace_file.id }
       check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/x-gzip", "gpx.gz"
 
       # Now ask explicitly for XML format
-      get :api_data, :params => { :id => identifiable_trace_file.id, :format => "xml" }
+      get :data, :params => { :id => identifiable_trace_file.id, :format => "xml" }
       check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d", "application/xml", "xml"
 
       # Now ask explicitly for GPX format
-      get :api_data, :params => { :id => identifiable_trace_file.id, :format => "gpx" }
+      get :data, :params => { :id => identifiable_trace_file.id, :format => "gpx" }
       check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d"
     end
 
     # Check an anonymous trace can't be downloaded by another user through the api
-    def test_api_data_anon
+    def test_data_anon
       anon_trace_file = create(:trace, :visibility => "private", :fixture => "b")
 
       # First with no auth
-      get :api_data, :params => { :id => anon_trace_file.id }
+      get :data, :params => { :id => anon_trace_file.id }
       assert_response :unauthorized
 
       # Now with some other user, which shouldn't work since the trace is anon
       basic_authorization create(:user).display_name, "test"
-      get :api_data, :params => { :id => anon_trace_file.id }
+      get :data, :params => { :id => anon_trace_file.id }
       assert_response :forbidden
 
       # And finally we should be able to do it with the owner of the trace
       basic_authorization anon_trace_file.user.display_name, "test"
-      get :api_data, :params => { :id => anon_trace_file.id }
+      get :data, :params => { :id => anon_trace_file.id }
       check_trace_data anon_trace_file, "66179ca44f1e93d8df62e2b88cbea732"
     end
 
     # Test downloading a trace that doesn't exist through the api
-    def test_api_data_not_found
+    def test_data_not_found
       deleted_trace_file = create(:trace, :deleted)
 
       # Try first with no auth, as it should require it
-      get :api_data, :params => { :id => 0 }
+      get :data, :params => { :id => 0 }
       assert_response :unauthorized
 
       # Login, and try again
       basic_authorization create(:user).display_name, "test"
-      get :api_data, :params => { :id => 0 }
+      get :data, :params => { :id => 0 }
       assert_response :not_found
 
       # Now try a trace which did exist but has been deleted
       basic_authorization deleted_trace_file.user.display_name, "test"
-      get :api_data, :params => { :id => deleted_trace_file.id }
+      get :data, :params => { :id => deleted_trace_file.id }
       assert_response :not_found
     end
 
     # Test creating a trace through the api
-    def test_api_create
+    def test_create
       # Get file to use
       fixture = Rails.root.join("test", "gpx", "fixtures", "a.gpx")
       file = Rack::Test::UploadedFile.new(fixture, "application/gpx+xml")
       user = create(:user)
 
       # First with no auth
-      post :api_create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }
+      post :create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }
       assert_response :unauthorized
 
       # Rewind the file
@@ -193,7 +193,7 @@ module Api
       create(:user_preference, :user => user, :k => "gps.trace.visibility", :v => "identifiable")
       assert_not_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
       basic_authorization user.display_name, "test"
-      post :api_create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }
+      post :create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }
       assert_response :success
       trace = Trace.find(response.body.to_i)
       assert_equal "a.gpx", trace.name
@@ -211,7 +211,7 @@ module Api
       # Now authenticated, with the legacy public flag
       assert_not_equal "public", user.preferences.where(:k => "gps.trace.visibility").first.v
       basic_authorization user.display_name, "test"
-      post :api_create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 1 }
+      post :create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 1 }
       assert_response :success
       trace = Trace.find(response.body.to_i)
       assert_equal "a.gpx", trace.name
@@ -230,7 +230,7 @@ module Api
       second_user = create(:user)
       assert_nil second_user.preferences.where(:k => "gps.trace.visibility").first
       basic_authorization second_user.display_name, "test"
-      post :api_create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 0 }
+      post :create, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 0 }
       assert_response :success
       trace = Trace.find(response.body.to_i)
       assert_equal "a.gpx", trace.name
@@ -244,33 +244,33 @@ module Api
     end
 
     # Check updating a trace through the api
-    def test_api_update
+    def test_update
       public_trace_file = create(:trace, :visibility => "public", :fixture => "a")
       deleted_trace_file = create(:trace, :deleted)
       anon_trace_file = create(:trace, :visibility => "private")
 
       # First with no auth
-      put :api_update, :params => { :id => public_trace_file.id }, :body => public_trace_file.to_xml.to_s
+      put :update, :params => { :id => public_trace_file.id }, :body => public_trace_file.to_xml.to_s
       assert_response :unauthorized
 
       # Now with some other user, which should fail
       basic_authorization create(:user).display_name, "test"
-      put :api_update, :params => { :id => public_trace_file.id }, :body => public_trace_file.to_xml.to_s
+      put :update, :params => { :id => public_trace_file.id }, :body => public_trace_file.to_xml.to_s
       assert_response :forbidden
 
       # Now with a trace which doesn't exist
       basic_authorization create(:user).display_name, "test"
-      put :api_update, :params => { :id => 0 }, :body => public_trace_file.to_xml.to_s
+      put :update, :params => { :id => 0 }, :body => public_trace_file.to_xml.to_s
       assert_response :not_found
 
       # Now with a trace which did exist but has been deleted
       basic_authorization deleted_trace_file.user.display_name, "test"
-      put :api_update, :params => { :id => deleted_trace_file.id }, :body => deleted_trace_file.to_xml.to_s
+      put :update, :params => { :id => deleted_trace_file.id }, :body => deleted_trace_file.to_xml.to_s
       assert_response :not_found
 
       # Now try an update with the wrong ID
       basic_authorization public_trace_file.user.display_name, "test"
-      put :api_update, :params => { :id => public_trace_file.id }, :body => anon_trace_file.to_xml.to_s
+      put :update, :params => { :id => public_trace_file.id }, :body => anon_trace_file.to_xml.to_s
       assert_response :bad_request,
                       "should not be able to update a trace with a different ID from the XML"
 
@@ -279,7 +279,7 @@ module Api
       t = public_trace_file
       t.description = "Changed description"
       t.visibility = "private"
-      put :api_update, :params => { :id => t.id }, :body => t.to_xml.to_s
+      put :update, :params => { :id => t.id }, :body => t.to_xml.to_s
       assert_response :success
       nt = Trace.find(t.id)
       assert_equal nt.description, t.description
@@ -287,12 +287,12 @@ module Api
     end
 
     # Test that updating a trace doesn't duplicate the tags
-    def test_api_update_tags
+    def test_update_tags
       tracetag = create(:tracetag)
       trace = tracetag.trace
       basic_authorization trace.user.display_name, "test"
 
-      put :api_update, :params => { :id => trace.id }, :body => trace.to_xml.to_s
+      put :update, :params => { :id => trace.id }, :body => trace.to_xml.to_s
       assert_response :success
 
       updated = Trace.find(trace.id)
@@ -303,31 +303,31 @@ module Api
     end
 
     # Check deleting a trace through the api
-    def test_api_delete
+    def test_destroy
       public_trace_file = create(:trace, :visibility => "public")
 
       # First with no auth
-      delete :api_delete, :params => { :id => public_trace_file.id }
+      delete :destroy, :params => { :id => public_trace_file.id }
       assert_response :unauthorized
 
       # Now with some other user, which should fail
       basic_authorization create(:user).display_name, "test"
-      delete :api_delete, :params => { :id => public_trace_file.id }
+      delete :destroy, :params => { :id => public_trace_file.id }
       assert_response :forbidden
 
       # Now with a trace which doesn't exist
       basic_authorization create(:user).display_name, "test"
-      delete :api_delete, :params => { :id => 0 }
+      delete :destroy, :params => { :id => 0 }
       assert_response :not_found
 
       # And finally we should be able to do it with the owner of the trace
       basic_authorization public_trace_file.user.display_name, "test"
-      delete :api_delete, :params => { :id => public_trace_file.id }
+      delete :destroy, :params => { :id => public_trace_file.id }
       assert_response :success
 
       # Try it a second time, which should fail
       basic_authorization public_trace_file.user.display_name, "test"
-      delete :api_delete, :params => { :id => public_trace_file.id }
+      delete :destroy, :params => { :id => public_trace_file.id }
       assert_response :not_found
     end
 
diff --git a/test/controllers/api/users_controller_test.rb b/test/controllers/api/users_controller_test.rb
index 5483731db..93a7eeb55 100644
--- a/test/controllers/api/users_controller_test.rb
+++ b/test/controllers/api/users_controller_test.rb
@@ -11,26 +11,26 @@ module Api
     def test_routes
       assert_routing(
         { :path => "/api/0.6/user/1", :method => :get },
-        { :controller => "api/users", :action => "api_read", :id => "1" }
+        { :controller => "api/users", :action => "show", :id => "1" }
       )
       assert_routing(
         { :path => "/api/0.6/user/details", :method => :get },
-        { :controller => "api/users", :action => "api_details" }
+        { :controller => "api/users", :action => "details" }
       )
       assert_routing(
         { :path => "/api/0.6/user/gpx_files", :method => :get },
-        { :controller => "api/users", :action => "api_gpx_files" }
+        { :controller => "api/users", :action => "gpx_files" }
       )
       assert_routing(
         { :path => "/api/0.6/users", :method => :get },
-        { :controller => "api/users", :action => "api_users" }
+        { :controller => "api/users", :action => "index" }
       )
     end
 
-    def test_api_read
+    def test_show
       user = create(:user, :description => "test", :terms_agreed => Date.yesterday)
       # check that a visible user is returned properly
-      get :api_read, :params => { :id => user.id }
+      get :show, :params => { :id => user.id }
       assert_response :success
       assert_equal "text/xml", response.content_type
 
@@ -63,30 +63,30 @@ module Api
       assert_select "messages", false
 
       # check that a suspended user is not returned
-      get :api_read, :params => { :id => create(:user, :suspended).id }
+      get :show, :params => { :id => create(:user, :suspended).id }
       assert_response :gone
 
       # check that a deleted user is not returned
-      get :api_read, :params => { :id => create(:user, :deleted).id }
+      get :show, :params => { :id => create(:user, :deleted).id }
       assert_response :gone
 
       # check that a non-existent user is not returned
-      get :api_read, :params => { :id => 0 }
+      get :show, :params => { :id => 0 }
       assert_response :not_found
     end
 
-    def test_api_details
+    def test_details
       user = create(:user, :description => "test", :terms_agreed => Date.yesterday, :home_lat => 12.1, :home_lon => 12.1, :languages => ["en"])
       create(:message, :read, :recipient => user)
       create(:message, :sender => user)
 
       # check that nothing is returned when not logged in
-      get :api_details
+      get :details
       assert_response :unauthorized
 
       # check that we get a response when logged in
       basic_authorization user.email, "test"
-      get :api_details
+      get :details
       assert_response :success
       assert_equal "text/xml", response.content_type
 
@@ -127,12 +127,12 @@ module Api
       end
     end
 
-    def test_api_users
+    def test_index
       user1 = create(:user, :description => "test1", :terms_agreed => Date.yesterday)
       user2 = create(:user, :description => "test2", :terms_agreed => Date.yesterday)
       user3 = create(:user, :description => "test3", :terms_agreed => Date.yesterday)
 
-      get :api_users, :params => { :users => user1.id }
+      get :index, :params => { :users => user1.id }
       assert_response :success
       assert_equal "text/xml", response.content_type
       assert_select "user", :count => 1 do
@@ -141,7 +141,7 @@ module Api
         assert_select "user[id='#{user3.id}']", :count => 0
       end
 
-      get :api_users, :params => { :users => user2.id }
+      get :index, :params => { :users => user2.id }
       assert_response :success
       assert_equal "text/xml", response.content_type
       assert_select "user", :count => 1 do
@@ -150,7 +150,7 @@ module Api
         assert_select "user[id='#{user3.id}']", :count => 0
       end
 
-      get :api_users, :params => { :users => "#{user1.id},#{user3.id}" }
+      get :index, :params => { :users => "#{user1.id},#{user3.id}" }
       assert_response :success
       assert_equal "text/xml", response.content_type
       assert_select "user", :count => 2 do
@@ -159,17 +159,17 @@ module Api
         assert_select "user[id='#{user3.id}']", :count => 1
       end
 
-      get :api_users, :params => { :users => create(:user, :suspended).id }
+      get :index, :params => { :users => create(:user, :suspended).id }
       assert_response :not_found
 
-      get :api_users, :params => { :users => create(:user, :deleted).id }
+      get :index, :params => { :users => create(:user, :deleted).id }
       assert_response :not_found
 
-      get :api_users, :params => { :users => 0 }
+      get :index, :params => { :users => 0 }
       assert_response :not_found
     end
 
-    def test_api_gpx_files
+    def test_gpx_files
       user = create(:user)
       trace1 = create(:trace, :user => user) do |trace|
         create(:tracetag, :trace => trace, :tag => "London")
@@ -178,12 +178,12 @@ module Api
         create(:tracetag, :trace => trace, :tag => "Birmingham")
       end
       # check that nothing is returned when not logged in
-      get :api_gpx_files
+      get :gpx_files
       assert_response :unauthorized
 
       # check that we get a response when logged in
       basic_authorization user.email, "test"
-      get :api_gpx_files
+      get :gpx_files
       assert_response :success
       assert_equal "application/xml", response.content_type