Declare api relation relations as nested resources

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index ebc5148dc986f661932a592a433fadc4bf973058..55476ab53f72440f5871b8df3a6419c3fd83c82b 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, [Node, Way]
-      can [:read, :relations_for_relation], Relation
+      can :read, [Node, Way, Relation]
       can [:history, :read], [OldNode, OldWay, OldRelation]
       can :read, UserBlock
 

diff --git a/app/controllers/api/relations/relations_controller.rb b/app/controllers/api/relations/relations_controller.rb
new file mode 100644 (file)
index 0000000..1769e13
--- /dev/null
+++ b/app/controllers/api/relations/relations_controller.rb
@@ -0,0 +1,25 @@
+module Api
+  module Relations
+    class RelationsController < ApiController
+      authorize_resource
+
+      before_action :set_request_formats
+
+      def index
+        relation_ids = RelationMember.where(:member_type => "Relation", :member_id => params[:relation_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 fa5fbc0a3efb7b3c72bf9fd06f05eb2549dae9bd..ce52382e7cbf7cf2cc83a88c2ca4079510aa6335 100644 (file)
--- a/app/controllers/api/relations_controller.rb
+++ b/app/controllers/api/relations_controller.rb
@@ -123,10 +123,6 @@ module Api
       end
     end
 
-    def relations_for_relation
-      relations_for_object("Relation")
-    end
-
     private
 
     def relations_for_object(objtype)

diff --git a/app/views/api/relations/relations/index.json.jbuilder b/app/views/api/relations/relations/index.json.jbuilder
new file mode 100644 (file)
index 0000000..1d8b9f2
--- /dev/null
+++ b/app/views/api/relations/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/relations/relations/index.xml.builder b/app/views/api/relations/relations/index.xml.builder
new file mode 100644 (file)
index 0000000..efe64f1
--- /dev/null
+++ b/app/views/api/relations/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_relation.json.jbuilder b/app/views/api/relations/relations_for_relation.json.jbuilder
deleted file mode 100644 (file)
index 9b0f65b..0000000
--- a/app/views/api/relations/relations_for_relation.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_relation.xml.builder b/app/views/api/relations/relations_for_relation.xml.builder
deleted file mode 100644 (file)
index f39a20b..0000000
--- a/app/views/api/relations/relations_for_relation.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 fa33a9469f81e68c7e3b58825c0ff3c12bf53e93..2b86326986dae9e1c07c2df8ac060aa82769e24d 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -38,7 +38,6 @@ OpenStreetMap::Application.routes.draw do
     post "way/:id/:version/redact" => "old_ways#redact", :as => :way_version_redact, :version => /\d+/, :id => /\d+/
     get "way/:id/:version" => "old_ways#show", :as => :api_old_way, :id => /\d+/, :version => /\d+/
 
-    get "relation/:id/relations" => "relations#relations_for_relation", :as => :relation_relations, :id => /\d+/
     get "relation/:id/history" => "old_relations#history", :as => :api_relation_history, :id => /\d+/
     post "relation/:id/:version/redact" => "old_relations#redact", :as => :relation_version_redact, :version => /\d+/, :id => /\d+/
     get "relation/:id/:version" => "old_relations#show", :as => :api_old_relation, :id => /\d+/, :version => /\d+/
@@ -70,6 +69,9 @@ OpenStreetMap::Application.routes.draw do
       member do
         get :full, :action => :show, :full => true, :as => nil
       end
+      scope :module => :relations do
+        resources :relations, :only => :index
+      end
     end
     put "relation/create" => "relations#create", :as => nil
 

diff --git a/test/controllers/api/relations/relations_controller_test.rb b/test/controllers/api/relations/relations_controller_test.rb
new file mode 100644 (file)
index 0000000..f3009b9
--- /dev/null
+++ b/test/controllers/api/relations/relations_controller_test.rb
@@ -0,0 +1,69 @@
+require "test_helper"
+
+module Api
+  module Relations
+    class RelationsControllerTest < ActionDispatch::IntegrationTest
+      ##
+      # test all routes which lead to this controller
+      def test_routes
+        assert_routing(
+          { :path => "/api/0.6/relation/1/relations", :method => :get },
+          { :controller => "api/relations/relations", :action => "index", :relation_id => "1" }
+        )
+        assert_routing(
+          { :path => "/api/0.6/relation/1/relations.json", :method => :get },
+          { :controller => "api/relations/relations", :action => "index", :relation_id => "1", :format => "json" }
+        )
+      end
+
+      def test_index
+        relation = create(:relation)
+        # should include relations with that relation as a member
+        relation_with_relation = create(:relation_member, :member => relation).relation
+        # should ignore any relation without that relation as a member
+        _relation_without_relation = create(:relation_member).relation
+        # should ignore relations with the relation involved indirectly, via a relation
+        second_relation = create(:relation_member, :member => relation).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 => relation, :relation => relation_with_relation)
+        # should not include deleted relations
+        deleted_relation = create(:relation, :deleted)
+        create(:relation_member, :member => relation, :relation => deleted_relation)
+
+        get api_relation_relations_path(relation)
+
+        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_relation, 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='relation'][ref='#{relation.id}']"
+        end
+      end
+
+      def test_index_json
+        relation = create(:relation)
+        containing_relation = create(:relation_member, :member => relation).relation
+
+        get api_relation_relations_path(relation, :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 32dfee08fc954f27b490872d200360ff34aaf484..e34dc616a48722b33deb66a948be3d02d78d5854 100644 (file)
--- a/test/controllers/api/relations_controller_test.rb
+++ b/test/controllers/api/relations_controller_test.rb
@@ -42,15 +42,6 @@ module Api
         { :controller => "api/relations", :action => "destroy", :id => "1" }
       )
 
-      assert_routing(
-        { :path => "/api/0.6/relation/1/relations", :method => :get },
-        { :controller => "api/relations", :action => "relations_for_relation", :id => "1" }
-      )
-      assert_routing(
-        { :path => "/api/0.6/relation/1/relations.json", :method => :get },
-        { :controller => "api/relations", :action => "relations_for_relation", :id => "1", :format => "json" }
-      )
-
       assert_recognizes(
         { :controller => "api/relations", :action => "create" },
         { :path => "/api/0.6/relation/create", :method => :put }
@@ -212,25 +203,6 @@ module Api
       assert_equal node.id, js_nodes[0]["id"]
     end
 
-    def test_relations_for_relation
-      relation = create(:relation)
-      # should include relations with that relation as a member
-      relation_with_relation = create(:relation_member, :member => relation).relation
-      # should ignore any relation without that relation as a member
-      _relation_without_relation = create(:relation_member).relation
-      # should ignore relations with the relation involved indirectly, via a relation
-      second_relation = create(:relation_member, :member => relation).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 => relation, :relation => relation_with_relation)
-      # should not include deleted relations
-      deleted_relation = create(:relation, :deleted)
-      create(:relation_member, :member => relation, :relation => deleted_relation)
-      check_relations_for_element(relation_relations_path(relation), "relation",
-                                  relation.id,
-                                  [relation_with_relation, second_relation])
-    end
-
     # -------------------------------------
     # Test simple relation creation.
     # -------------------------------------