Declare api node relations as nested resources

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 2b1c664d6de6da5cdb31aab0016ae758d6b9e4fe..cc1964ffb233b12d8befc2e95cabebc47840585f 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -16,7 +16,7 @@ class ApiAbility
       can :read, Tracepoint
       can :read, User
       can :read, [Node, Way]
       can :read, Tracepoint
       can :read, User
       can :read, [Node, Way]

-      can [:read, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
+      can [:read, :relations_for_way, :relations_for_relation], Relation

       can [:history, :read], [OldNode, OldWay, OldRelation]
       can :read, UserBlock
 
       can [:history, :read], [OldNode, OldWay, OldRelation]
       can :read, UserBlock
 

diff --git a/app/controllers/api/nodes/relations_controller.rb b/app/controllers/api/nodes/relations_controller.rb
new file mode 100644 (file)
index 0000000..0f0409e
--- /dev/null
+++ b/app/controllers/api/nodes/relations_controller.rb
@@ -0,0 +1,25 @@
+module Api
+  module Nodes
+    class RelationsController < ApiController
+      authorize_resource
+
+      before_action :set_request_formats
+
+      def index
+        relation_ids = RelationMember.where(:member_type => "Node", :member_id => params[:node_id]).collect(&:relation_id).uniq
+
+        @relations = []
+
+        Relation.find(relation_ids).each do |relation|
+          @relations << relation if relation.visible
+        end
+
+        # Render the result
+        respond_to do |format|
+          format.xml
+          format.json
+        end
+      end
+    end
+  end
+end

diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb
index 006b3e8a624f88d9536e500d7f034c3bf6cb6cf8..cc6f7c2101d6c03bbd7ee3166764aabe3864cac5 100644 (file)
--- a/app/controllers/api/relations_controller.rb
+++ b/app/controllers/api/relations_controller.rb
@@ -127,10 +127,6 @@ module Api
       relations_for_object("Way")
     end
 
       relations_for_object("Way")
     end
 

-    def relations_for_node
-      relations_for_object("Node")
-    end
-

     def relations_for_relation
       relations_for_object("Relation")
     end
     def relations_for_relation
       relations_for_object("Relation")
     end

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

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

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

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

diff --git a/config/routes.rb b/config/routes.rb
index de5fd58266be9d79aed672e2570b52fc3b738a86..95b134f950ff6dfc7871a8a8e144379eb4bca2a4 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/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/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\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/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/


@@ -51,6 +50,7 @@ OpenStreetMap::Application.routes.draw do
     resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] do
       scope :module => :nodes do
         resources :ways, :only => :index
     resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] do
       scope :module => :nodes do
         resources :ways, :only => :index

+        resources :relations, :only => :index

       end
     end
     put "node/create" => "nodes#create", :as => nil
       end
     end
     put "node/create" => "nodes#create", :as => nil

diff --git a/test/controllers/api/nodes/relations_controller_test.rb b/test/controllers/api/nodes/relations_controller_test.rb
new file mode 100644 (file)
index 0000000..168fd71
--- /dev/null
+++ b/test/controllers/api/nodes/relations_controller_test.rb
@@ -0,0 +1,75 @@
+require "test_helper"
+
+module Api
+  module Nodes
+    class RelationsControllerTest < ActionDispatch::IntegrationTest
+      ##
+      # test all routes which lead to this controller
+      def test_routes
+        assert_routing(
+          { :path => "/api/0.6/node/1/relations", :method => :get },
+          { :controller => "api/nodes/relations", :action => "index", :node_id => "1" }
+        )
+        assert_routing(
+          { :path => "/api/0.6/node/1/relations.json", :method => :get },
+          { :controller => "api/nodes/relations", :action => "index", :node_id => "1", :format => "json" }
+        )
+      end
+
+      ##
+      # check that all relations containing a particular node, and no extra
+      # relations, are returned.
+      def test_index
+        node = create(:node)
+        # should include relations with that node as a member
+        relation_with_node = create(:relation_member, :member => node).relation
+        # should ignore relations without that node as a member
+        _relation_without_node = create(:relation_member).relation
+        # should ignore relations with the node involved indirectly, via a way
+        way = create(:way_node, :node => node).way
+        _relation_with_way = create(:relation_member, :member => way).relation
+        # should ignore relations with the node involved indirectly, via a relation
+        second_relation = create(:relation_member, :member => node).relation
+        _super_relation = create(:relation_member, :member => second_relation).relation
+        # should combine multiple relation_member references into just one relation entry
+        create(:relation_member, :member => node, :relation => relation_with_node)
+        # should not include deleted relations
+        deleted_relation = create(:relation, :deleted)
+        create(:relation_member, :member => node, :relation => deleted_relation)
+
+        get api_node_relations_path(node)
+
+        assert_response :success
+
+        # count one osm element
+        assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", 1
+
+        # we should have only the expected number of relations
+        expected_relations = [relation_with_node, second_relation]
+        assert_select "osm>relation", expected_relations.size
+
+        # and each of them should contain the element we originally searched for
+        expected_relations.each do |containing_relation|
+          # The relation should appear once, but the element could appear multiple times
+          assert_select "osm>relation[id='#{containing_relation.id}']", 1
+          assert_select "osm>relation[id='#{containing_relation.id}']>member[type='node'][ref='#{node.id}']"
+        end
+      end
+
+      def test_index_json
+        node = create(:node)
+        containing_relation = create(:relation_member, :member => node).relation
+
+        get api_node_relations_path(node, :format => "json")
+
+        assert_response :success
+        js = ActiveSupport::JSON.decode(@response.body)
+        assert_not_nil js
+        assert_equal 1, js["elements"].count
+        js_relations = js["elements"].filter { |e| e["type"] == "relation" }
+        assert_equal 1, js_relations.count
+        assert_equal containing_relation.id, js_relations[0]["id"]
+      end
+    end
+  end
+end

diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb
index 803a57acf52995448e082e6177a2f6184ab469d3..4dd38fc3f5a13c08116560727fd19beaeed043d6 100644 (file)
--- a/test/controllers/api/relations_controller_test.rb
+++ b/test/controllers/api/relations_controller_test.rb
@@ -42,10 +42,6 @@ module Api
         { :controller => "api/relations", :action => "destroy", :id => "1" }
       )
 
         { :controller => "api/relations", :action => "destroy", :id => "1" }
       )
 

-      assert_routing(
-        { :path => "/api/0.6/node/1/relations", :method => :get },
-        { :controller => "api/relations", :action => "relations_for_node", :id => "1" }
-      )

       assert_routing(
         { :path => "/api/0.6/way/1/relations", :method => :get },
         { :controller => "api/relations", :action => "relations_for_way", :id => "1" }
       assert_routing(
         { :path => "/api/0.6/way/1/relations", :method => :get },
         { :controller => "api/relations", :action => "relations_for_way", :id => "1" }


@@ -54,10 +50,6 @@ module Api
         { :path => "/api/0.6/relation/1/relations", :method => :get },
         { :controller => "api/relations", :action => "relations_for_relation", :id => "1" }
       )
         { :path => "/api/0.6/relation/1/relations", :method => :get },
         { :controller => "api/relations", :action => "relations_for_relation", :id => "1" }
       )

-      assert_routing(
-        { :path => "/api/0.6/node/1/relations.json", :method => :get },
-        { :controller => "api/relations", :action => "relations_for_node", :id => "1", :format => "json" }
-      )

       assert_routing(
         { :path => "/api/0.6/way/1/relations.json", :method => :get },
         { :controller => "api/relations", :action => "relations_for_way", :id => "1", :format => "json" }
       assert_routing(
         { :path => "/api/0.6/way/1/relations.json", :method => :get },
         { :controller => "api/relations", :action => "relations_for_way", :id => "1", :format => "json" }


@@ -228,32 +220,6 @@ module Api
       assert_equal node.id, js_nodes[0]["id"]
     end
 
       assert_equal node.id, js_nodes[0]["id"]
     end
 

-    ##
-    # check that all relations containing a particular node, and no extra
-    # relations, are returned from the relations_for_node call.
-    def test_relations_for_node
-      node = create(:node)
-      # should include relations with that node as a member
-      relation_with_node = create(:relation_member, :member => node).relation
-      # should ignore relations without that node as a member
-      _relation_without_node = create(:relation_member).relation
-      # should ignore relations with the node involved indirectly, via a way
-      way = create(:way_node, :node => node).way
-      _relation_with_way = create(:relation_member, :member => way).relation
-      # should ignore relations with the node involved indirectly, via a relation
-      second_relation = create(:relation_member, :member => node).relation
-      _super_relation = create(:relation_member, :member => second_relation).relation
-      # should combine multiple relation_member references into just one relation entry
-      create(:relation_member, :member => node, :relation => relation_with_node)
-      # should not include deleted relations
-      deleted_relation = create(:relation, :deleted)
-      create(:relation_member, :member => node, :relation => deleted_relation)
-
-      check_relations_for_element(node_relations_path(node), "node",
-                                  node.id,
-                                  [relation_with_node, second_relation])
-    end
-

     def test_relations_for_way
       way = create(:way)
       # should include relations with that way as a member
     def test_relations_for_way
       way = create(:way)
       # should include relations with that way as a member