From 04c6d38649d0794eeb09e3b62e866efe6a9f95e2 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sat, 1 Feb 2025 17:48:49 +0300 Subject: [PATCH] Map 'full' to api relation show action --- app/abilities/api_ability.rb | 2 +- app/controllers/api/relations_controller.rb | 129 ++++++++---------- app/views/api/relations/full.json.jbuilder | 7 - app/views/api/relations/full.xml.builder | 7 - app/views/api/relations/show.json.jbuilder | 4 +- app/views/api/relations/show.xml.builder | 4 +- config/routes.rb | 7 +- .../api/relations_controller_test.rb | 38 +++++- 8 files changed, 101 insertions(+), 97 deletions(-) delete mode 100644 app/views/api/relations/full.json.jbuilder delete mode 100644 app/views/api/relations/full.xml.builder diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 7ce3aa599..97b461b1f 100644 --- 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, :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 diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index f712534f0..006b3e8a6 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -26,9 +26,64 @@ module Api 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 @@ -68,74 +123,6 @@ module Api 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 diff --git a/app/views/api/relations/full.json.jbuilder b/app/views/api/relations/full.json.jbuilder deleted file mode 100644 index 16331a89e..000000000 --- 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 index e23061727..000000000 --- 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 e5da7cd3f..16331a89e 100644 --- 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.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 diff --git a/app/views/api/relations/show.xml.builder b/app/views/api/relations/show.xml.builder index 555eb4db5..e23061727 100644 --- 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| - osm << (render(@relation) || "") + osm << (render(@nodes) || "") + osm << (render(@ways) || "") + osm << (render(@relations) || "") end diff --git a/config/routes.rb b/config/routes.rb index a7631849c..bc06e5f38 100644 --- 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/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 @@ -62,7 +61,11 @@ OpenStreetMap::Application.routes.draw do 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 diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index 34953f7b7..803a57acf 100644 --- 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 }, - { :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 }, - { :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 }, @@ -149,19 +149,19 @@ module Api 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 - 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) - get relation_full_path(relation) + get api_relation_path(relation, :full => true) 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) - get relation_full_path(relation) + get api_relation_path(relation, :full => true) 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) - get relation_full_path(relation) + get api_relation_path(relation, :full => true) assert_response :success assert_dom "node", :count => 1 do @@ -204,6 +204,30 @@ module Api 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. -- 2.39.5