From f696b5439eab14f50c8f2f74ae39eedd6efa64d1 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sun, 2 Feb 2025 14:27:20 +0300 Subject: [PATCH] Declare api relation relations as nested resources --- app/abilities/api_ability.rb | 3 +- .../api/relations/relations_controller.rb | 25 +++++++ app/controllers/api/relations_controller.rb | 4 -- .../relations/relations/index.json.jbuilder | 5 ++ .../api/relations/relations/index.xml.builder | 5 ++ .../relations_for_relation.json.jbuilder | 5 -- .../relations_for_relation.xml.builder | 5 -- config/routes.rb | 4 +- .../relations/relations_controller_test.rb | 69 +++++++++++++++++++ .../api/relations_controller_test.rb | 28 -------- 10 files changed, 108 insertions(+), 45 deletions(-) create mode 100644 app/controllers/api/relations/relations_controller.rb create mode 100644 app/views/api/relations/relations/index.json.jbuilder create mode 100644 app/views/api/relations/relations/index.xml.builder delete mode 100644 app/views/api/relations/relations_for_relation.json.jbuilder delete mode 100644 app/views/api/relations/relations_for_relation.xml.builder create mode 100644 test/controllers/api/relations/relations_controller_test.rb diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index ebc5148dc..55476ab53 100644 --- 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 index 000000000..1769e1396 --- /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 fa5fbc0a3..ce52382e7 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_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 index 000000000..1d8b9f2cd --- /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 index 000000000..efe64f159 --- /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 index 9b0f65bf1..000000000 --- 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 index f39a20bb9..000000000 --- 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 fa33a9469..2b8632698 100644 --- 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 index 000000000..f3009b9bf --- /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 32dfee08f..e34dc616a 100644 --- 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. # ------------------------------------- -- 2.39.5