From: Anton Khorev Date: Sun, 2 Feb 2025 11:06:12 +0000 (+0300) Subject: Declare api way relations as nested resources X-Git-Tag: live~168^2~3 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/2a38dca0b780b972ff93b4cae1bc37c1d476dffa?ds=inline Declare api way relations as nested resources --- diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index cc1964ffb..ebc5148dc 100644 --- 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, :relations_for_way, :relations_for_relation], Relation + can [:read, :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 cc6f7c210..fa5fbc0a3 100644 --- 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_way - relations_for_object("Way") - 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 index 000000000..fcd8b40dd --- /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 index 9b0f65bf1..000000000 --- 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 index f39a20bb9..000000000 --- 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 index 000000000..1d8b9f2cd --- /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 index 000000000..efe64f159 --- /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 95b134f95..fa33a9469 100644 --- 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 "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+/ @@ -60,6 +59,9 @@ OpenStreetMap::Application.routes.draw do 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 diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index 4dd38fc3f..32dfee08f 100644 --- 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" } ) - 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/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" } @@ -220,26 +212,6 @@ module Api 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 diff --git a/test/controllers/api/ways/relations_controller_test.rb b/test/controllers/api/ways/relations_controller_test.rb new file mode 100644 index 000000000..ded9ba3e9 --- /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