Map 'full' to api relation show action

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 7ce3aa599ba0d8265bb18e649e9801e410833fe8..97b461b1f602282669b706f1ef4421d5e5f8b96e 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -17,7 +17,7 @@ class ApiAbility
       can :read, User
       can :read, Node
       can [:read, :ways_for_node], Way
       can :read, User
       can :read, Node
       can [:read, :ways_for_node], Way

-      can [:read, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
+      can [:read, :relations_for_node, :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/relations_controller.rb b/app/controllers/api/relations_controller.rb
index f712534f001aebdb747234abf27d1f68a87ce982..006b3e8a624f88d9536e500d7f034c3bf6cb6cf8 100644 (file)
--- a/app/controllers/api/relations_controller.rb
+++ b/app/controllers/api/relations_controller.rb
@@ -26,9 +26,64 @@ module Api
     end
 
     def show
     end
 
     def show

-      @relation = Relation.find(params[:id])
-      response.last_modified = @relation.timestamp
-      if @relation.visible
+      relation = Relation.find(params[:id])
+
+      response.last_modified = relation.timestamp unless params[:full]
+
+      @nodes = []
+      @ways = []
+      @relations = []
+
+      if relation.visible
+        if params[:full]
+          # with parameter :full
+          # returns representation of one relation object plus all its
+          # members, plus all nodes part of member ways
+
+          # first find the ids of nodes, ways and relations referenced by this
+          # relation - note that we exclude this relation just in case.
+
+          node_ids = relation.members.select { |m| m[0] == "Node" }.pluck(1)
+          way_ids = relation.members.select { |m| m[0] == "Way" }.pluck(1)
+          relation_ids = relation.members.select { |m| m[0] == "Relation" && m[1] != relation.id }.pluck(1)
+
+          # next load the relations and the ways.
+
+          relations = Relation.where(:id => relation_ids).includes(:relation_tags)
+          ways = Way.where(:id => way_ids).includes(:way_nodes, :way_tags)
+
+          # now additionally collect nodes referenced by ways. Note how we
+          # recursively evaluate ways but NOT relations.
+
+          way_node_ids = ways.collect do |way|
+            way.way_nodes.collect(&:node_id)
+          end
+          node_ids += way_node_ids.flatten
+          nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)
+
+          @nodes = []
+          nodes.each do |node|
+            next unless node.visible? # should be unnecessary if data is consistent.
+
+            @nodes << node
+          end
+
+          ways.each do |way|
+            next unless way.visible? # should be unnecessary if data is consistent.
+
+            @ways << way
+          end
+
+          relations.each do |rel|
+            next unless rel.visible? # should be unnecessary if data is consistent.
+
+            @relations << rel
+          end
+        end
+
+        # finally add self
+        @relations << relation
+

         # Render the result
         respond_to do |format|
           format.xml
         # Render the result
         respond_to do |format|
           format.xml


@@ -68,74 +123,6 @@ module Api
       end
     end
 
       end
     end
 

-    # -----------------------------------------------------------------
-    # full
-    #
-    # input parameters: id
-    #
-    # returns XML representation of one relation object plus all its
-    # members, plus all nodes part of member ways
-    # -----------------------------------------------------------------
-    def full
-      relation = Relation.find(params[:id])
-
-      if relation.visible
-
-        # first find the ids of nodes, ways and relations referenced by this
-        # relation - note that we exclude this relation just in case.
-
-        node_ids = relation.members.select { |m| m[0] == "Node" }.pluck(1)
-        way_ids = relation.members.select { |m| m[0] == "Way" }.pluck(1)
-        relation_ids = relation.members.select { |m| m[0] == "Relation" && m[1] != relation.id }.pluck(1)
-
-        # next load the relations and the ways.
-
-        relations = Relation.where(:id => relation_ids).includes(:relation_tags)
-        ways = Way.where(:id => way_ids).includes(:way_nodes, :way_tags)
-
-        # now additionally collect nodes referenced by ways. Note how we
-        # recursively evaluate ways but NOT relations.
-
-        way_node_ids = ways.collect do |way|
-          way.way_nodes.collect(&:node_id)
-        end
-        node_ids += way_node_ids.flatten
-        nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)
-
-        @nodes = []
-        nodes.each do |node|
-          next unless node.visible? # should be unnecessary if data is consistent.
-
-          @nodes << node
-        end
-
-        @ways = []
-        ways.each do |way|
-          next unless way.visible? # should be unnecessary if data is consistent.
-
-          @ways << way
-        end
-
-        @relations = []
-        relations.each do |rel|
-          next unless rel.visible? # should be unnecessary if data is consistent.
-
-          @relations << rel
-        end
-
-        # finally add self
-        @relations << relation
-
-        # Render the result
-        respond_to do |format|
-          format.xml
-          format.json
-        end
-      else
-        head :gone
-      end
-    end
-

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

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

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

diff --git a/app/views/api/relations/show.json.jbuilder b/app/views/api/relations/show.json.jbuilder
index e5da7cd3f48fa4553f70a2b2adb6cdf2c2d41214..16331a89e6fd9aac3226a8b273877cb5fbea78b0 100644 (file)
--- a/app/views/api/relations/show.json.jbuilder
+++ b/app/views/api/relations/show.json.jbuilder
@@ -1,5 +1,7 @@
 json.partial! "api/root_attributes"
 
 json.elements do
 json.partial! "api/root_attributes"
 
 json.elements do

-  json.array! [@relation], :partial => "relation", :as => :relation
+  json.array! @nodes, :partial => "/api/nodes/node", :as => :node
+  json.array! @ways, :partial => "/api/ways/way", :as => :way
+  json.array! @relations, :partial => "/api/relations/relation", :as => :relation

 end
 end

diff --git a/app/views/api/relations/show.xml.builder b/app/views/api/relations/show.xml.builder
index 555eb4db5f73d5a53bcd41b9222bac4f758e6003..e23061727c4c1d2756d5a5a3e4cd08836a3ea71e 100644 (file)
--- a/app/views/api/relations/show.xml.builder
+++ b/app/views/api/relations/show.xml.builder
@@ -1,5 +1,7 @@
 xml.instruct!
 
 xml.osm(OSM::API.new.xml_root_attributes) do |osm|
 xml.instruct!
 
 xml.osm(OSM::API.new.xml_root_attributes) do |osm|

-  osm << (render(@relation) || "")
+  osm << (render(@nodes) || "")
+  osm << (render(@ways) || "")
+  osm << (render(@relations) || "")

 end
 end

diff --git a/config/routes.rb b/config/routes.rb
index a7631849cca6d8d58c4aab296767b6dfd8ffedd9..bc06e5f38e123a787de5084a818e1714e79eabb3 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -43,7 +43,6 @@ OpenStreetMap::Application.routes.draw do
 
     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+/
 
     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+/

-    get "relation/:id/full" => "relations#full", :as => :relation_full, :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+/
   end
     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+/
   end


@@ -62,7 +61,11 @@ OpenStreetMap::Application.routes.draw do
     put "way/create" => "ways#create", :as => nil
 
     resources :relations, :only => [:index, :create]
     put "way/create" => "ways#create", :as => nil
 
     resources :relations, :only => [:index, :create]

-    resources :relations, :path => "relation", :id => /\d+/, :only => [:show, :update, :destroy]
+    resources :relations, :path => "relation", :id => /\d+/, :only => [:show, :update, :destroy] do
+      member do
+        get :full, :action => :show, :full => true, :as => nil
+      end
+    end

     put "relation/create" => "relations#create", :as => nil
 
     resource :map, :only => :show
     put "relation/create" => "relations#create", :as => nil
 
     resource :map, :only => :show

diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb
index 34953f7b7ac62841a1e27002ab383a9130393574..803a57acf52995448e082e6177a2f6184ab469d3 100644 (file)
--- a/test/controllers/api/relations_controller_test.rb
+++ b/test/controllers/api/relations_controller_test.rb
@@ -27,11 +27,11 @@ module Api
       )
       assert_routing(
         { :path => "/api/0.6/relation/1/full", :method => :get },
       )
       assert_routing(
         { :path => "/api/0.6/relation/1/full", :method => :get },

-        { :controller => "api/relations", :action => "full", :id => "1" }
+        { :controller => "api/relations", :action => "show", :full => true, :id => "1" }

       )
       assert_routing(
         { :path => "/api/0.6/relation/1/full.json", :method => :get },
       )
       assert_routing(
         { :path => "/api/0.6/relation/1/full.json", :method => :get },

-        { :controller => "api/relations", :action => "full", :id => "1", :format => "json" }
+        { :controller => "api/relations", :action => "show", :full => true, :id => "1", :format => "json" }

       )
       assert_routing(
         { :path => "/api/0.6/relation/1", :method => :put },
       )
       assert_routing(
         { :path => "/api/0.6/relation/1", :method => :put },


@@ -149,19 +149,19 @@ module Api
     end
 
     def test_full_not_found
     end
 
     def test_full_not_found

-      get relation_full_path(999999)
+      get api_relation_path(999999, :full => true)

       assert_response :not_found
     end
 
     def test_full_deleted
       assert_response :not_found
     end
 
     def test_full_deleted

-      get relation_full_path(create(:relation, :deleted))
+      get api_relation_path(create(:relation, :deleted), :full => true)

       assert_response :gone
     end
 
     def test_full_empty
       relation = create(:relation)
 
       assert_response :gone
     end
 
     def test_full_empty
       relation = create(:relation)
 

-      get relation_full_path(relation)
+      get api_relation_path(relation, :full => true)

 
       assert_response :success
       assert_dom "relation", :count => 1 do
 
       assert_response :success
       assert_dom "relation", :count => 1 do


@@ -174,7 +174,7 @@ module Api
       node = create(:node)
       create(:relation_member, :relation => relation, :member => node)
 
       node = create(:node)
       create(:relation_member, :relation => relation, :member => node)
 

-      get relation_full_path(relation)
+      get api_relation_path(relation, :full => true)

 
       assert_response :success
       assert_dom "node", :count => 1 do
 
       assert_response :success
       assert_dom "node", :count => 1 do


@@ -190,7 +190,7 @@ module Api
       way = create(:way_with_nodes)
       create(:relation_member, :relation => relation, :member => way)
 
       way = create(:way_with_nodes)
       create(:relation_member, :relation => relation, :member => way)
 

-      get relation_full_path(relation)
+      get api_relation_path(relation, :full => true)

 
       assert_response :success
       assert_dom "node", :count => 1 do
 
       assert_response :success
       assert_dom "node", :count => 1 do


@@ -204,6 +204,30 @@ module Api
       end
     end
 
       end
     end
 

+    def test_full_with_node_member_json
+      relation = create(:relation)
+      node = create(:node)
+      create(:relation_member, :relation => relation, :member => node)
+
+      get api_relation_path(relation, :full => true, :format => "json")
+
+      assert_response :success
+      js = ActiveSupport::JSON.decode(@response.body)
+      assert_not_nil js
+      assert_equal 2, js["elements"].count
+
+      js_relations = js["elements"].filter { |e| e["type"] == "relation" }
+      assert_equal 1, js_relations.count
+      assert_equal relation.id, js_relations[0]["id"]
+      assert_equal 1, js_relations[0]["members"].count
+      assert_equal "node", js_relations[0]["members"][0]["type"]
+      assert_equal node.id, js_relations[0]["members"][0]["ref"]
+
+      js_nodes = js["elements"].filter { |e| e["type"] == "node" }
+      assert_equal 1, js_nodes.count
+      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.
     ##
     # check that all relations containing a particular node, and no extra
     # relations, are returned from the relations_for_node call.