More resourceful routing for nodes, ways, relations and changesets co…

…ntrollers

app/abilities/ability.rb

@@ -7,7 +7,7 @@ def initialize(user)
     can [:trackpoints, :map, :changes, :capabilities, :permissions], :api
     can [:relation, :relation_history, :way, :way_history, :node, :node_history,
          :changeset, :note, :new_note, :query], :browse
-    can [:index, :feed, :read, :download, :query], Changeset
+    can [:index, :feed, :show, :download, :query], Changeset
     can :index, ChangesetComment
     can :search, :direction
     can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id], :site
@@ -23,9 +23,9 @@ def initialize(user)
     can [:index, :show, :data, :georss, :picture, :icon], Trace
     can [:terms, :api_users, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :api_read, :auth_success, :auth_failure], User
     can [:index, :show, :blocks_on, :blocks_by], UserBlock
-    can [:read, :nodes], Node
-    can [:read, :full, :ways, :ways_for_node], Way
-    can [:read, :full, :relations, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
+    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
     can [:history, :version], OldNode
     can [:history, :version], OldWay
     can [:history, :version], OldRelation

app/controllers/changesets_controller.rb

@@ -42,7 +42,7 @@ def create
   ##
   # Return XML giving the basic info about the changeset. Does not
   # return anything about the nodes, ways and relations in the changeset.
-  def read
+  def show
     changeset = Changeset.find(params[:id])
 
     render :xml => changeset.to_xml(params[:include_discussion].presence).to_s

app/controllers/nodes_controller.rb

@@ -26,7 +26,7 @@ def create
   end
 
   # Dump the details on a node given in params[:id]
-  def read
+  def show
     node = Node.find(params[:id])
 
     response.last_modified = node.timestamp
@@ -63,7 +63,7 @@ def delete
   end
 
   # Dump the details on many nodes whose ids are given in the "nodes" parameter.
-  def nodes
+  def index
     raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" unless params["nodes"]
 
     ids = params["nodes"].split(",").collect(&:to_i)

app/controllers/relations_controller.rb

@@ -22,7 +22,7 @@ def create
     render :plain => relation.id.to_s
   end
 
-  def read
+  def show
     relation = Relation.find(params[:id])
     response.last_modified = relation.timestamp
     if relation.visible
@@ -123,7 +123,7 @@ def full
     end
   end
 
-  def relations
+  def index
     raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"]
 
     ids = params["relations"].split(",").collect(&:to_i)

app/controllers/ways_controller.rb

@@ -22,7 +22,7 @@ def create
     render :plain => way.id.to_s
   end
 
-  def read
+  def show
     way = Way.find(params[:id])
 
     response.last_modified = way.timestamp
@@ -82,7 +82,7 @@ def full
     end
   end
 
-  def ways
+  def index
     unless params["ways"]
       raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]"
     end

app/views/browse/changeset.html.erb

@@ -137,7 +137,7 @@
 <% end %>
 
 <div class='secondary-actions'>
-  <%= link_to(t('.changesetxml'), :controller => "changesets", :action => "read") %>
+  <%= link_to(t('.changesetxml'), :controller => "changesets", :action => "show") %>
   &middot;
   <%= link_to(t('.osmchangexml'), :controller => "changesets", :action => "download") %>
 </div>

app/views/browse/feature.html.erb

@@ -8,7 +8,7 @@
 <%= render :partial => @type, :object => @feature %>
 
 <div class='secondary-actions'>
-  <%= link_to(t('browse.download_xml'), :controller => @type.pluralize, :action => "read") %>
+  <%= link_to(t('browse.download_xml'), :controller => @type.pluralize, :action => :show) %>
   &middot;
   <%= link_to(t('browse.view_history'), :action => "#{@type}_history") %>
 </div>

app/views/changesets/index.atom.builder

@@ -17,7 +17,7 @@ atom_feed(:language => I18n.locale, :schema_date => 2009,
   @edits.each do |changeset|
     feed.entry(changeset, :updated => changeset.closed_at, :id => changeset_url(changeset.id, :only_path => false)) do |entry|
       entry.link :rel => "alternate",
-                 :href => changeset_read_url(changeset, :only_path => false),
+                 :href => changeset_show_url(changeset, :only_path => false),
                  :type => "application/osm+xml"
       entry.link :rel => "alternate",
                  :href => changeset_download_url(changeset, :only_path => false),

config/routes.rb

@@ -10,7 +10,7 @@
     post "changeset/:id/upload" => "changesets#upload", :id => /\d+/
     get "changeset/:id/download" => "changesets#download", :as => :changeset_download, :id => /\d+/
     post "changeset/:id/expand_bbox" => "changesets#expand_bbox", :id => /\d+/
-    get "changeset/:id" => "changesets#read", :as => :changeset_read, :id => /\d+/
+    get "changeset/:id" => "changesets#show", :as => :changeset_show, :id => /\d+/
     post "changeset/:id/subscribe" => "changesets#subscribe", :as => :changeset_subscribe, :id => /\d+/
     post "changeset/:id/unsubscribe" => "changesets#unsubscribe", :as => :changeset_unsubscribe, :id => /\d+/
     put "changeset/:id" => "changesets#update", :id => /\d+/
@@ -26,32 +26,32 @@
     get "node/:id/history" => "old_nodes#history", :id => /\d+/
     post "node/:id/:version/redact" => "old_nodes#redact", :version => /\d+/, :id => /\d+/
     get "node/:id/:version" => "old_nodes#version", :id => /\d+/, :version => /\d+/
-    get "node/:id" => "nodes#read", :id => /\d+/
+    get "node/:id" => "nodes#show", :id => /\d+/
     put "node/:id" => "nodes#update", :id => /\d+/
     delete "node/:id" => "nodes#delete", :id => /\d+/
-    get "nodes" => "nodes#nodes"
+    get "nodes" => "nodes#index"
 
     put "way/create" => "ways#create"
     get "way/:id/history" => "old_ways#history", :id => /\d+/
     get "way/:id/full" => "ways#full", :id => /\d+/
     get "way/:id/relations" => "relations#relations_for_way", :id => /\d+/
     post "way/:id/:version/redact" => "old_ways#redact", :version => /\d+/, :id => /\d+/
     get "way/:id/:version" => "old_ways#version", :id => /\d+/, :version => /\d+/
-    get "way/:id" => "ways#read", :id => /\d+/
+    get "way/:id" => "ways#show", :id => /\d+/
     put "way/:id" => "ways#update", :id => /\d+/
     delete "way/:id" => "ways#delete", :id => /\d+/
-    get "ways" => "ways#ways"
+    get "ways" => "ways#index"
 
     put "relation/create" => "relations#create"
     get "relation/:id/relations" => "relations#relations_for_relation", :id => /\d+/
     get "relation/:id/history" => "old_relations#history", :id => /\d+/
     get "relation/:id/full" => "relations#full", :id => /\d+/
     post "relation/:id/:version/redact" => "old_relations#redact", :version => /\d+/, :id => /\d+/
     get "relation/:id/:version" => "old_relations#version", :id => /\d+/, :version => /\d+/
-    get "relation/:id" => "relations#read", :id => /\d+/
+    get "relation/:id" => "relations#show", :id => /\d+/
     put "relation/:id" => "relations#update", :id => /\d+/
     delete "relation/:id" => "relations#delete", :id => /\d+/
-    get "relations" => "relations#relations"
+    get "relations" => "relations#index"
 
     get "map" => "api#map"
 

test/controllers/changesets_controller_test.rb

@@ -22,7 +22,7 @@ def test_routes
     )
     assert_routing(
       { :path => "/api/0.6/changeset/1", :method => :get },
-      { :controller => "changesets", :action => "read", :id => "1" }
+      { :controller => "changesets", :action => "show", :id => "1" }
     )
     assert_routing(
       { :path => "/api/0.6/changeset/1/subscribe", :method => :post },
@@ -152,19 +152,19 @@ def test_create_wrong_method
   end
 
   ##
-  # check that the changeset can be read and returns the correct
+  # check that the changeset can be shown and returns the correct
   # document structure.
-  def test_read
+  def test_show
     changeset_id = create(:changeset).id
 
-    get :read, :params => { :id => changeset_id }
+    get :show, :params => { :id => changeset_id }
     assert_response :success, "cannot get first changeset"
 
     assert_select "osm[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1
     assert_select "osm>changeset[id='#{changeset_id}']", 1
     assert_select "osm>changeset>discussion", 0
 
-    get :read, :params => { :id => changeset_id, :include_discussion => true }
+    get :show, :params => { :id => changeset_id, :include_discussion => true }
     assert_response :success, "cannot get first changeset with comments"
 
     assert_select "osm[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1
@@ -175,7 +175,7 @@ def test_read
     changeset_id = create(:changeset, :closed).id
     create_list(:changeset_comment, 3, :changeset_id => changeset_id)
 
-    get :read, :params => { :id => changeset_id, :include_discussion => true }
+    get :show, :params => { :id => changeset_id, :include_discussion => true }
     assert_response :success, "cannot get closed changeset with comments"
 
     assert_select "osm[version='#{API_VERSION}'][generator='OpenStreetMap server']", 1
@@ -186,10 +186,10 @@ def test_read
 
   ##
   # check that a changeset that doesn't exist returns an appropriate message
-  def test_read_not_found
+  def test_show_not_found
     [0, -32, 233455644, "afg", "213"].each do |id|
       begin
-        get :read, :params => { :id => id }
+        get :show, :params => { :id => id }
         assert_response :not_found, "should get a not found"
       rescue ActionController::UrlGenerationError => ex
         assert_match(/No route matches/, ex.to_s)
@@ -1483,7 +1483,7 @@ def test_changeset_bbox
     end
 
     # get the bounding box back from the changeset
-    get :read, :params => { :id => changeset_id }
+    get :show, :params => { :id => changeset_id }
     assert_response :success, "Couldn't read back changeset."
     assert_select "osm>changeset[min_lon='1.0000000']", 1
     assert_select "osm>changeset[max_lon='1.0000000']", 1
@@ -1498,7 +1498,7 @@ def test_changeset_bbox
     end
 
     # get the bounding box back from the changeset
-    get :read, :params => { :id => changeset_id }
+    get :show, :params => { :id => changeset_id }
     assert_response :success, "Couldn't read back changeset for the second time."
     assert_select "osm>changeset[min_lon='1.0000000']", 1
     assert_select "osm>changeset[max_lon='2.0000000']", 1
@@ -1513,7 +1513,7 @@ def test_changeset_bbox
     end
 
     # get the bounding box back from the changeset
-    get :read, :params => { :id => changeset_id }
+    get :show, :params => { :id => changeset_id }
     assert_response :success, "Couldn't read back changeset for the third time."
     assert_select "osm>changeset[min_lon='1.0000000']", 1
     assert_select "osm>changeset[max_lon='3.0000000']", 1
@@ -1789,7 +1789,7 @@ def test_changeset_limits
       assert_response :success, "can't create a new node"
       node_id = @response.body.to_i
 
-      get :read, :params => { :id => node_id }
+      get :show, :params => { :id => node_id }
       assert_response :success, "can't read back new node"
       node_doc = XML::Parser.string(@response.body).parse
       node_xml = node_doc.find("//osm/node").first

test/controllers/nodes_controller_test.rb

@@ -10,7 +10,7 @@ def test_routes
     )
     assert_routing(
       { :path => "/api/0.6/node/1", :method => :get },
-      { :controller => "nodes", :action => "read", :id => "1" }
+      { :controller => "nodes", :action => "show", :id => "1" }
     )
     assert_routing(
       { :path => "/api/0.6/node/1", :method => :put },
@@ -22,7 +22,7 @@ def test_routes
     )
     assert_routing(
       { :path => "/api/0.6/nodes", :method => :get },
-      { :controller => "nodes", :action => "nodes" }
+      { :controller => "nodes", :action => "index" }
     )
   end
 
@@ -132,17 +132,17 @@ def test_create_invalid_xml
     assert_equal ["NodeTag ", " v: is too long (maximum is 255 characters) (\"#{'x' * 256}\")"], @response.body.split(/[0-9]+,foo:/)
   end
 
-  def test_read
+  def test_show
     # check that a visible node is returned properly
-    get :read, :params => { :id => create(:node).id }
+    get :show, :params => { :id => create(:node).id }
     assert_response :success
 
     # check that an deleted node is not returned
-    get :read, :params => { :id => create(:node, :deleted).id }
+    get :show, :params => { :id => create(:node, :deleted).id }
     assert_response :gone
 
     # check chat a non-existent node is not returned
-    get :read, :params => { :id => 0 }
+    get :show, :params => { :id => 0 }
     assert_response :not_found
   end
 
@@ -427,23 +427,23 @@ def test_update
 
   ##
   # test fetching multiple nodes
-  def test_nodes
+  def test_index
     node1 = create(:node)
     node2 = create(:node, :deleted)
     node3 = create(:node)
     node4 = create(:node, :with_history, :version => 2)
     node5 = create(:node, :deleted, :with_history, :version => 2)
 
     # check error when no parameter provided
-    get :nodes
+    get :index
     assert_response :bad_request
 
     # check error when no parameter value provided
-    get :nodes, :params => { :nodes => "" }
+    get :index, :params => { :nodes => "" }
     assert_response :bad_request
 
     # test a working call
-    get :nodes, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}" }
+    get :index, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}" }
     assert_response :success
     assert_select "osm" do
       assert_select "node", :count => 5
@@ -455,7 +455,7 @@ def test_nodes
     end
 
     # check error when a non-existent node is included
-    get :nodes, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id},0" }
+    get :index, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id},0" }
     assert_response :not_found
   end
 
@@ -518,7 +518,7 @@ def test_string_injection
     assert_not_nil checknode, "node not found in data base after upload"
 
     # and grab it using the api
-    get :read, :params => { :id => nodeid }
+    get :show, :params => { :id => nodeid }
     assert_response :success
     apinode = Node.from_xml(@response.body)
     assert_not_nil apinode, "downloaded node is nil, but shouldn't be"

test/controllers/old_nodes_controller_test.rb

@@ -386,7 +386,7 @@ def do_redact_node(node, redaction)
   def check_current_version(node_id)
     # get the current version of the node
     current_node = with_controller(NodesController.new) do
-      get :read, :params => { :id => node_id }
+      get :show, :params => { :id => node_id }
       assert_response :success, "cant get current node #{node_id}"
       Node.from_xml(@response.body)
     end

test/controllers/old_relations_controller_test.rb

@@ -225,7 +225,7 @@ def test_unredact_relation_moderator
   def check_current_version(relation_id)
     # get the current version
     current_relation = with_controller(RelationsController.new) do
-      get :read, :params => { :id => relation_id }
+      get :show, :params => { :id => relation_id }
       assert_response :success, "can't get current relation #{relation_id}"
       Relation.from_xml(@response.body)
     end

test/controllers/old_ways_controller_test.rb

@@ -265,7 +265,7 @@ def test_unredact_way_moderator
   def check_current_version(way_id)
     # get the current version
     current_way = with_controller(WaysController.new) do
-      get :read, :params => { :id => way_id }
+      get :show, :params => { :id => way_id }
       assert_response :success, "can't get current way #{way_id}"
       Way.from_xml(@response.body)
     end