From 6a50a5e871db2e35eeb20f5707e467fe1442583b Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sun, 2 Feb 2025 13:11:03 +0300 Subject: [PATCH] Declare api node ways as nested resources --- app/abilities/api_ability.rb | 3 +- app/controllers/api/nodes/ways_controller.rb | 25 +++++++ app/controllers/api/ways_controller.rb | 16 ----- app/views/api/nodes/ways/index.json.jbuilder | 5 ++ app/views/api/nodes/ways/index.xml.builder | 5 ++ .../api/ways/ways_for_node.json.jbuilder | 5 -- app/views/api/ways/ways_for_node.xml.builder | 5 -- config/routes.rb | 7 +- .../api/nodes/ways_controller_test.rb | 72 +++++++++++++++++++ test/controllers/api/ways_controller_test.rb | 36 ---------- 10 files changed, 113 insertions(+), 66 deletions(-) create mode 100644 app/controllers/api/nodes/ways_controller.rb create mode 100644 app/views/api/nodes/ways/index.json.jbuilder create mode 100644 app/views/api/nodes/ways/index.xml.builder delete mode 100644 app/views/api/ways/ways_for_node.json.jbuilder delete mode 100644 app/views/api/ways/ways_for_node.xml.builder create mode 100644 test/controllers/api/nodes/ways_controller_test.rb diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 97b461b1f..2b1c664d6 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 - can [:read, :ways_for_node], Way + can :read, [Node, Way] 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/nodes/ways_controller.rb b/app/controllers/api/nodes/ways_controller.rb new file mode 100644 index 000000000..2a71f1903 --- /dev/null +++ b/app/controllers/api/nodes/ways_controller.rb @@ -0,0 +1,25 @@ +module Api + module Nodes + class WaysController < ApiController + authorize_resource + + before_action :set_request_formats + + ## + # returns all the ways which are currently using the node given in the + # :node_id parameter. note that this used to return deleted ways as well, but + # this seemed not to be the expected behaviour, so it was removed. + def index + way_ids = WayNode.where(:node_id => params[:node_id]).collect { |ws| ws.id[0] }.uniq + + @ways = Way.where(:id => way_ids, :visible => true) + + # Render the result + respond_to do |format| + format.xml + format.json + end + end + end + end +end diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index b1bc8d799..2fd86b997 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -80,21 +80,5 @@ module Api head :bad_request end end - - ## - # returns all the ways which are currently using the node given in the - # :id parameter. note that this used to return deleted ways as well, but - # this seemed not to be the expected behaviour, so it was removed. - def ways_for_node - wayids = WayNode.where(:node_id => params[:id]).collect { |ws| ws.id[0] }.uniq - - @ways = Way.where(:id => wayids, :visible => true) - - # Render the result - respond_to do |format| - format.xml - format.json - end - end end end diff --git a/app/views/api/nodes/ways/index.json.jbuilder b/app/views/api/nodes/ways/index.json.jbuilder new file mode 100644 index 000000000..27119cad5 --- /dev/null +++ b/app/views/api/nodes/ways/index.json.jbuilder @@ -0,0 +1,5 @@ +json.partial! "api/root_attributes" + +json.elements do + json.array! @ways, :partial => "api/ways/way", :as => :way +end diff --git a/app/views/api/nodes/ways/index.xml.builder b/app/views/api/nodes/ways/index.xml.builder new file mode 100644 index 000000000..7872d7d52 --- /dev/null +++ b/app/views/api/nodes/ways/index.xml.builder @@ -0,0 +1,5 @@ +xml.instruct! + +xml.osm(OSM::API.new.xml_root_attributes) do |osm| + osm << (render(:partial => "api/ways/way", :collection => @ways) || "") +end diff --git a/app/views/api/ways/ways_for_node.json.jbuilder b/app/views/api/ways/ways_for_node.json.jbuilder deleted file mode 100644 index 7b8de5f1c..000000000 --- a/app/views/api/ways/ways_for_node.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -json.partial! "api/root_attributes" - -json.elements do - json.array! @ways, :partial => "way", :as => :way -end diff --git a/app/views/api/ways/ways_for_node.xml.builder b/app/views/api/ways/ways_for_node.xml.builder deleted file mode 100644 index bcb89cdc6..000000000 --- a/app/views/api/ways/ways_for_node.xml.builder +++ /dev/null @@ -1,5 +0,0 @@ -xml.instruct! - -xml.osm(OSM::API.new.xml_root_attributes) do |osm| - osm << (render(@ways) || "") -end diff --git a/config/routes.rb b/config/routes.rb index bc06e5f38..de5fd5826 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,7 +30,6 @@ OpenStreetMap::Application.routes.draw do post "changeset/comment/:id/hide" => "changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/ post "changeset/comment/:id/unhide" => "changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/ - get "node/:id/ways" => "ways#ways_for_node", :as => :node_ways, :id => /\d+/ get "node/:id/relations" => "relations#relations_for_node", :as => :node_relations, :id => /\d+/ get "node/:id/history" => "old_nodes#history", :as => :api_node_history, :id => /\d+/ post "node/:id/:version/redact" => "old_nodes#redact", :as => :node_version_redact, :version => /\d+/, :id => /\d+/ @@ -49,7 +48,11 @@ OpenStreetMap::Application.routes.draw do namespace :api, :path => "api/0.6" do resources :nodes, :only => [:index, :create] - resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] + resources :nodes, :path => "node", :id => /\d+/, :only => [:show, :update, :destroy] do + scope :module => :nodes do + resources :ways, :only => :index + end + end put "node/create" => "nodes#create", :as => nil resources :ways, :only => [:index, :create] diff --git a/test/controllers/api/nodes/ways_controller_test.rb b/test/controllers/api/nodes/ways_controller_test.rb new file mode 100644 index 000000000..0a728cf9a --- /dev/null +++ b/test/controllers/api/nodes/ways_controller_test.rb @@ -0,0 +1,72 @@ +require "test_helper" + +module Api + module Nodes + class WaysControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/api/0.6/node/1/ways", :method => :get }, + { :controller => "api/nodes/ways", :action => "index", :node_id => "1" } + ) + assert_routing( + { :path => "/api/0.6/node/1/ways.json", :method => :get }, + { :controller => "api/nodes/ways", :action => "index", :node_id => "1", :format => "json" } + ) + end + + ## + # test that a call to ways_for_node returns all ways that contain the node + # and none that don't. + def test_index + node = create(:node) + way1 = create(:way) + way2 = create(:way) + create(:way_node, :way => way1, :node => node) + create(:way_node, :way => way2, :node => node) + # create an unrelated way + create(:way_with_nodes, :nodes_count => 2) + # create a way which used to use the node + way3_v1 = create(:old_way, :version => 1) + _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2) + create(:old_way_node, :old_way => way3_v1, :node => node) + + get api_node_ways_path(node) + assert_response :success + ways_xml = XML::Parser.string(@response.body).parse + assert_not_nil ways_xml, "failed to parse ways_for_node response" + + # check that the set of IDs match expectations + expected_way_ids = [way1.id, + way2.id] + found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i } + assert_equal expected_way_ids.sort, found_way_ids.sort, + "expected ways for node #{node.id} did not match found" + + # check the full ways to ensure we're not missing anything + expected_way_ids.each do |id| + way_xml = ways_xml.find("//osm/way[@id='#{id}']").first + assert_ways_are_equal(Way.find(id), + Way.from_xml_node(way_xml)) + end + end + + def test_index_json + node = create(:node) + way = create(:way) + create(:way_node, :way => way, :node => node) + + get api_node_ways_path(node, :format => "json") + + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 1, js["elements"].count + js_ways = js["elements"].filter { |e| e["type"] == "way" } + assert_equal 1, js_ways.count + assert_equal way.id, js_ways[0]["id"] + end + end + end +end diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb index 191f9a820..50077f09c 100644 --- a/test/controllers/api/ways_controller_test.rb +++ b/test/controllers/api/ways_controller_test.rb @@ -778,42 +778,6 @@ module Api assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body end - ## - # test that a call to ways_for_node returns all ways that contain the node - # and none that don't. - def test_ways_for_node - node = create(:node) - way1 = create(:way) - way2 = create(:way) - create(:way_node, :way => way1, :node => node) - create(:way_node, :way => way2, :node => node) - # create an unrelated way - create(:way_with_nodes, :nodes_count => 2) - # create a way which used to use the node - way3_v1 = create(:old_way, :version => 1) - _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2) - create(:old_way_node, :old_way => way3_v1, :node => node) - - get node_ways_path(node) - assert_response :success - ways_xml = XML::Parser.string(@response.body).parse - assert_not_nil ways_xml, "failed to parse ways_for_node response" - - # check that the set of IDs match expectations - expected_way_ids = [way1.id, - way2.id] - found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i } - assert_equal expected_way_ids.sort, found_way_ids.sort, - "expected ways for node #{node.id} did not match found" - - # check the full ways to ensure we're not missing anything - expected_way_ids.each do |id| - way_xml = ways_xml.find("//osm/way[@id='#{id}']").first - assert_ways_are_equal(Way.find(id), - Way.from_xml_node(way_xml)) - end - end - ## # test initial rate limit def test_initial_rate_limit -- 2.39.5