Declare api node ways as nested resources

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 97b461b1f602282669b706f1ef4421d5e5f8b96e..2b1c664d6de6da5cdb31aab0016ae758d6b9e4fe 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -15,8 +15,7 @@ class ApiAbility
       can [:read, :download], Changeset
       can :read, Tracepoint
       can :read, User
       can [:read, :download], Changeset
       can :read, Tracepoint
       can :read, User

-      can :read, Node
-      can [:read, :ways_for_node], Way
+      can :read, [Node, Way]

       can [:read, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
       can [:history, :read], [OldNode, OldWay, OldRelation]
       can :read, UserBlock
       can [:read, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
       can [:history, :read], [OldNode, OldWay, OldRelation]
       can :read, UserBlock

diff --git a/app/controllers/api/nodes/ways_controller.rb b/app/controllers/api/nodes/ways_controller.rb
new file mode 100644 (file)
index 0000000..2a71f19
--- /dev/null
+++ b/app/controllers/api/nodes/ways_controller.rb
@@ -0,0 +1,25 @@
+module Api
+  module Nodes
+    class WaysController < ApiController
+      authorize_resource
+
+      before_action :set_request_formats
+
+      ##
+      # returns all the ways which are currently using the node given in the
+      # :node_id parameter. note that this used to return deleted ways as well, but
+      # this seemed not to be the expected behaviour, so it was removed.
+      def index
+        way_ids = WayNode.where(:node_id => params[:node_id]).collect { |ws| ws.id[0] }.uniq
+
+        @ways = Way.where(:id => way_ids, :visible => true)
+
+        # Render the result
+        respond_to do |format|
+          format.xml
+          format.json
+        end
+      end
+    end
+  end
+end

diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb
index b1bc8d799b0747c3f35dc3b36bdadcadd21f26e7..2fd86b99750a2f0b3a76560dcd661f633b51035b 100644 (file)
--- a/app/controllers/api/ways_controller.rb
+++ b/app/controllers/api/ways_controller.rb
@@ -80,21 +80,5 @@ module Api
         head :bad_request
       end
     end
         head :bad_request
       end
     end

-
-    ##
-    # returns all the ways which are currently using the node given in the
-    # :id parameter. note that this used to return deleted ways as well, but
-    # this seemed not to be the expected behaviour, so it was removed.
-    def ways_for_node
-      wayids = WayNode.where(:node_id => params[:id]).collect { |ws| ws.id[0] }.uniq
-
-      @ways = Way.where(:id => wayids, :visible => true)
-
-      # Render the result
-      respond_to do |format|
-        format.xml
-        format.json
-      end
-    end

   end
 end
   end
 end

diff --git a/app/views/api/nodes/ways/index.json.jbuilder b/app/views/api/nodes/ways/index.json.jbuilder
new file mode 100644 (file)
index 0000000..27119ca
--- /dev/null
+++ b/app/views/api/nodes/ways/index.json.jbuilder
@@ -0,0 +1,5 @@
+json.partial! "api/root_attributes"
+
+json.elements do
+  json.array! @ways, :partial => "api/ways/way", :as => :way
+end

diff --git a/app/views/api/nodes/ways/index.xml.builder b/app/views/api/nodes/ways/index.xml.builder
new file mode 100644 (file)
index 0000000..7872d7d
--- /dev/null
+++ b/app/views/api/nodes/ways/index.xml.builder
@@ -0,0 +1,5 @@
+xml.instruct!
+
+xml.osm(OSM::API.new.xml_root_attributes) do |osm|
+  osm << (render(:partial => "api/ways/way", :collection => @ways) || "")
+end

diff --git a/app/views/api/ways/ways_for_node.json.jbuilder b/app/views/api/ways/ways_for_node.json.jbuilder
deleted file mode 100644 (file)
index 7b8de5f..0000000
--- a/app/views/api/ways/ways_for_node.json.jbuilder
+++ /dev/null
@@ -1,5 +0,0 @@
-json.partial! "api/root_attributes"
-
-json.elements do
-  json.array! @ways, :partial => "way", :as => :way
-end

diff --git a/app/views/api/ways/ways_for_node.xml.builder b/app/views/api/ways/ways_for_node.xml.builder
deleted file mode 100644 (file)
index bcb89cd..0000000
--- a/app/views/api/ways/ways_for_node.xml.builder
+++ /dev/null
@@ -1,5 +0,0 @@
-xml.instruct!
-
-xml.osm(OSM::API.new.xml_root_attributes) do |osm|
-  osm << (render(@ways) || "")
-end

diff --git a/config/routes.rb b/config/routes.rb
index bc06e5f38e123a787de5084a818e1714e79eabb3..de5fd58266be9d79aed672e2570b52fc3b738a86 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -30,7 +30,6 @@ OpenStreetMap::Application.routes.draw do
     post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/
     post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/
 
     post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/
     post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/
 

-    get "node/:id/ways" => "ways#ways_for_node", :as => :node_ways, :id => /\d+/

     get "node/:id/relations" => "relations#relations_for_node", :as => :node_relations, :id => /\d+/
     get "node/:id/history" => "old_nodes#history", :as => :api_node_history, :id => /\d+/
     post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/
     get "node/:id/relations" => "relations#relations_for_node", :as => :node_relations, :id => /\d+/
     get "node/:id/history" => "old_nodes#history", :as => :api_node_history, :id => /\d+/
     post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/


@@ -49,7 +48,11 @@ OpenStreetMap::Application.routes.draw do
 
   namespace :api, :path => "api/0.6" do
     resources :nodes, :only => [:index, :create]
 
   namespace :api, :path => "api/0.6" do
     resources :nodes, :only => [:index, :create]

-    resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy]
+    resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] do
+      scope :module => :nodes do
+        resources :ways, :only => :index
+      end
+    end

     put "node/create" => "nodes#create", :as => nil
 
     resources :ways, :only => [:index, :create]
     put "node/create" => "nodes#create", :as => nil
 
     resources :ways, :only => [:index, :create]

diff --git a/test/controllers/api/nodes/ways_controller_test.rb b/test/controllers/api/nodes/ways_controller_test.rb
new file mode 100644 (file)
index 0000000..0a728cf
--- /dev/null
+++ b/test/controllers/api/nodes/ways_controller_test.rb
@@ -0,0 +1,72 @@
+require "test_helper"
+
+module Api
+  module Nodes
+    class WaysControllerTest < ActionDispatch::IntegrationTest
+      ##
+      # test all routes which lead to this controller
+      def test_routes
+        assert_routing(
+          { :path => "/api/0.6/node/1/ways", :method => :get },
+          { :controller => "api/nodes/ways", :action => "index", :node_id => "1" }
+        )
+        assert_routing(
+          { :path => "/api/0.6/node/1/ways.json", :method => :get },
+          { :controller => "api/nodes/ways", :action => "index", :node_id => "1", :format => "json" }
+        )
+      end
+
+      ##
+      # test that a call to ways_for_node returns all ways that contain the node
+      # and none that don't.
+      def test_index
+        node = create(:node)
+        way1 = create(:way)
+        way2 = create(:way)
+        create(:way_node, :way => way1, :node => node)
+        create(:way_node, :way => way2, :node => node)
+        # create an unrelated way
+        create(:way_with_nodes, :nodes_count => 2)
+        # create a way which used to use the node
+        way3_v1 = create(:old_way, :version => 1)
+        _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2)
+        create(:old_way_node, :old_way => way3_v1, :node => node)
+
+        get api_node_ways_path(node)
+        assert_response :success
+        ways_xml = XML::Parser.string(@response.body).parse
+        assert_not_nil ways_xml, "failed to parse ways_for_node response"
+
+        # check that the set of IDs match expectations
+        expected_way_ids = [way1.id,
+                            way2.id]
+        found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i }
+        assert_equal expected_way_ids.sort, found_way_ids.sort,
+                     "expected ways for node #{node.id} did not match found"
+
+        # check the full ways to ensure we're not missing anything
+        expected_way_ids.each do |id|
+          way_xml = ways_xml.find("//osm/way[@id='#{id}']").first
+          assert_ways_are_equal(Way.find(id),
+                                Way.from_xml_node(way_xml))
+        end
+      end
+
+      def test_index_json
+        node = create(:node)
+        way = create(:way)
+        create(:way_node, :way => way, :node => node)
+
+        get api_node_ways_path(node, :format => "json")
+
+        assert_response :success
+        js = ActiveSupport::JSON.decode(@response.body)
+        assert_not_nil js
+        assert_equal 1, js["elements"].count
+        js_ways = js["elements"].filter { |e| e["type"] == "way" }
+        assert_equal 1, js_ways.count
+        assert_equal way.id, js_ways[0]["id"]
+      end
+    end
+  end
+end

diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb
index 191f9a82037b14c8f2437c0e1bb5f9661953dd81..50077f09c351b94d0149d8a4448db940821506dc 100644 (file)
--- a/test/controllers/api/ways_controller_test.rb
+++ b/test/controllers/api/ways_controller_test.rb
@@ -778,42 +778,6 @@ module Api
       assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body
     end
 
       assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body
     end
 

-    ##
-    # test that a call to ways_for_node returns all ways that contain the node
-    # and none that don't.
-    def test_ways_for_node
-      node = create(:node)
-      way1 = create(:way)
-      way2 = create(:way)
-      create(:way_node, :way => way1, :node => node)
-      create(:way_node, :way => way2, :node => node)
-      # create an unrelated way
-      create(:way_with_nodes, :nodes_count => 2)
-      # create a way which used to use the node
-      way3_v1 = create(:old_way, :version => 1)
-      _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2)
-      create(:old_way_node, :old_way => way3_v1, :node => node)
-
-      get node_ways_path(node)
-      assert_response :success
-      ways_xml = XML::Parser.string(@response.body).parse
-      assert_not_nil ways_xml, "failed to parse ways_for_node response"
-
-      # check that the set of IDs match expectations
-      expected_way_ids = [way1.id,
-                          way2.id]
-      found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i }
-      assert_equal expected_way_ids.sort, found_way_ids.sort,
-                   "expected ways for node #{node.id} did not match found"
-
-      # check the full ways to ensure we're not missing anything
-      expected_way_ids.each do |id|
-        way_xml = ways_xml.find("//osm/way[@id='#{id}']").first
-        assert_ways_are_equal(Way.find(id),
-                              Way.from_xml_node(way_xml))
-      end
-    end
-

     ##
     # test initial rate limit
     def test_initial_rate_limit
     ##
     # test initial rate limit
     def test_initial_rate_limit