Declare api way relations as nested resources

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index cc1964ffb233b12d8befc2e95cabebc47840585f..ebc5148dc986f661932a592a433fadc4bf973058 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_way, :relations_for_relation], Relation
+      can [:read, :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/relations_controller.rb b/app/controllers/api/relations_controller.rb
index cc6f7c2101d6c03bbd7ee3166764aabe3864cac5..fa5fbc0a3efb7b3c72bf9fd06f05eb2549dae9bd 100644 (file)
--- a/app/controllers/api/relations_controller.rb
+++ b/app/controllers/api/relations_controller.rb
@@ -123,10 +123,6 @@ module Api
       end
     end
 
       end
     end
 

-    def relations_for_way
-      relations_for_object("Way")
-    end
-

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

diff --git a/app/controllers/api/ways/relations_controller.rb b/app/controllers/api/ways/relations_controller.rb
new file mode 100644 (file)
index 0000000..fcd8b40
--- /dev/null
+++ b/app/controllers/api/ways/relations_controller.rb
@@ -0,0 +1,25 @@
+module Api
+  module Ways
+    class RelationsController < ApiController
+      authorize_resource
+
+      before_action :set_request_formats
+
+      def index
+        relation_ids = RelationMember.where(:member_type => "Way", :member_id => params[:way_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/views/api/relations/relations_for_way.json.jbuilder b/app/views/api/relations/relations_for_way.json.jbuilder
deleted file mode 100644 (file)
index 9b0f65b..0000000
--- a/app/views/api/relations/relations_for_way.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_way.xml.builder b/app/views/api/relations/relations_for_way.xml.builder
deleted file mode 100644 (file)
index f39a20b..0000000
--- a/app/views/api/relations/relations_for_way.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/app/views/api/ways/relations/index.json.jbuilder b/app/views/api/ways/relations/index.json.jbuilder
new file mode 100644 (file)
index 0000000..1d8b9f2
--- /dev/null
+++ b/app/views/api/ways/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/ways/relations/index.xml.builder b/app/views/api/ways/relations/index.xml.builder
new file mode 100644 (file)
index 0000000..efe64f1
--- /dev/null
+++ b/app/views/api/ways/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/config/routes.rb b/config/routes.rb
index 95b134f950ff6dfc7871a8a8e144379eb4bca2a4..fa33a9469f81e68c7e3b58825c0ff3c12bf53e93 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -35,7 +35,6 @@ OpenStreetMap::Application.routes.draw do
     get "node/:id/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/
 
     get "way/:id/history" => "old_ways#history", :as => :api_way_history, :id => /\d+/
     get "node/:id/:version" => "old_nodes#show", :as => :api_old_node, :id => /\d+/, :version => /\d+/
 
     get "way/:id/history" => "old_ways#history", :as => :api_way_history, :id => /\d+/

-    get "way/:id/relations" => "relations#relations_for_way", :as => :way_relations, :id => /\d+/

     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+/
 
     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+/
 


@@ -60,6 +59,9 @@ OpenStreetMap::Application.routes.draw do
       member do
         get :full, :action => :show, :full => true, :as => nil
       end
       member do
         get :full, :action => :show, :full => true, :as => nil
       end

+      scope :module => :ways do
+        resources :relations, :only => :index
+      end

     end
     put "way/create" => "ways#create", :as => nil
 
     end
     put "way/create" => "ways#create", :as => nil
 

diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb
index 4dd38fc3f5a13c08116560727fd19beaeed043d6..32dfee08fc954f27b490872d200360ff34aaf484 100644 (file)
--- a/test/controllers/api/relations_controller_test.rb
+++ b/test/controllers/api/relations_controller_test.rb
@@ -42,18 +42,10 @@ module Api
         { :controller => "api/relations", :action => "destroy", :id => "1" }
       )
 
         { :controller => "api/relations", :action => "destroy", :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/relation/1/relations", :method => :get },
         { :controller => "api/relations", :action => "relations_for_relation", :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/way/1/relations.json", :method => :get },
-        { :controller => "api/relations", :action => "relations_for_way", :id => "1", :format => "json" }
-      )

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


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

-    def test_relations_for_way
-      way = create(:way)
-      # should include relations with that way as a member
-      relation_with_way = create(:relation_member, :member => way).relation
-      # should ignore relations without that way as a member
-      _relation_without_way = create(:relation_member).relation
-      # should ignore relations with the way involved indirectly, via a relation
-      second_relation = create(:relation_member, :member => way).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 => way, :relation => relation_with_way)
-      # should not include deleted relations
-      deleted_relation = create(:relation, :deleted)
-      create(:relation_member, :member => way, :relation => deleted_relation)
-
-      check_relations_for_element(way_relations_path(way), "way",
-                                  way.id,
-                                  [relation_with_way, second_relation])
-    end
-

     def test_relations_for_relation
       relation = create(:relation)
       # should include relations with that relation as a member
     def test_relations_for_relation
       relation = create(:relation)
       # should include relations with that relation as a member

diff --git a/test/controllers/api/ways/relations_controller_test.rb b/test/controllers/api/ways/relations_controller_test.rb
new file mode 100644 (file)
index 0000000..ded9ba3
--- /dev/null
+++ b/test/controllers/api/ways/relations_controller_test.rb
@@ -0,0 +1,69 @@
+require "test_helper"
+
+module Api
+  module Ways
+    class RelationsControllerTest < ActionDispatch::IntegrationTest
+      ##
+      # test all routes which lead to this controller
+      def test_routes
+        assert_routing(
+          { :path => "/api/0.6/way/1/relations", :method => :get },
+          { :controller => "api/ways/relations", :action => "index", :way_id => "1" }
+        )
+        assert_routing(
+          { :path => "/api/0.6/way/1/relations.json", :method => :get },
+          { :controller => "api/ways/relations", :action => "index", :way_id => "1", :format => "json" }
+        )
+      end
+
+      def test_index
+        way = create(:way)
+        # should include relations with that way as a member
+        relation_with_way = create(:relation_member, :member => way).relation
+        # should ignore relations without that way as a member
+        _relation_without_way = create(:relation_member).relation
+        # should ignore relations with the way involved indirectly, via a relation
+        second_relation = create(:relation_member, :member => way).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 => way, :relation => relation_with_way)
+        # should not include deleted relations
+        deleted_relation = create(:relation, :deleted)
+        create(:relation_member, :member => way, :relation => deleted_relation)
+
+        get api_way_relations_path(way)
+
+        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_way, 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='way'][ref='#{way.id}']"
+        end
+      end
+
+      def test_index_json
+        way = create(:way)
+        containing_relation = create(:relation_member, :member => way).relation
+
+        get api_way_relations_path(way, :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