From 0f2aa939d447dcf137309ac2ed80e0b23ee65c63 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sat, 1 Feb 2025 16:49:08 +0300 Subject: [PATCH] Map 'full' to api way show action --- app/abilities/api_ability.rb | 2 +- app/controllers/api/ways_controller.rb | 35 ++++++---------- app/views/api/ways/full.json.jbuilder | 6 --- app/views/api/ways/full.xml.builder | 6 --- app/views/api/ways/show.json.jbuilder | 1 + app/views/api/ways/show.xml.builder | 1 + config/routes.rb | 7 +++- test/controllers/api/ways_controller_test.rb | 44 +++++++++++++++++--- 8 files changed, 58 insertions(+), 44 deletions(-) delete mode 100644 app/views/api/ways/full.json.jbuilder delete mode 100644 app/views/api/ways/full.xml.builder diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index c74a4d099..7ce3aa599 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 - can [:read, :full, :ways_for_node], Way + can [:read, :ways_for_node], Way can [:read, :full, :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/ways_controller.rb b/app/controllers/api/ways_controller.rb index 285ed4604..b1bc8d799 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -26,12 +26,21 @@ module Api end def show - @way = Way.find(params[:id]) + @way = Way + @way = @way.includes(:nodes => :node_tags) if params[:full] + @way = @way.find(params[:id]) - response.last_modified = @way.timestamp + response.last_modified = @way.timestamp unless params[:full] if @way.visible - # Render the result + if params[:full] + @nodes = [] + + @way.nodes.uniq.each do |node| + @nodes << node if node.visible + end + end + respond_to do |format| format.xml format.json @@ -72,26 +81,6 @@ module Api end end - def full - @way = Way.includes(:nodes => :node_tags).find(params[:id]) - - if @way.visible - @nodes = [] - - @way.nodes.uniq.each do |node| - @nodes << node if node.visible - end - - # Render the result - respond_to do |format| - format.xml - format.json - end - else - head :gone - 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 diff --git a/app/views/api/ways/full.json.jbuilder b/app/views/api/ways/full.json.jbuilder deleted file mode 100644 index 1bd127dac..000000000 --- a/app/views/api/ways/full.json.jbuilder +++ /dev/null @@ -1,6 +0,0 @@ -json.partial! "api/root_attributes" - -json.elements do - json.array! @nodes, :partial => "/api/nodes/node", :as => :node - json.array! [@way], :partial => "/api/ways/way", :as => :way -end diff --git a/app/views/api/ways/full.xml.builder b/app/views/api/ways/full.xml.builder deleted file mode 100644 index 025291638..000000000 --- a/app/views/api/ways/full.xml.builder +++ /dev/null @@ -1,6 +0,0 @@ -xml.instruct! - -xml.osm(OSM::API.new.xml_root_attributes) do |osm| - osm << (render(@nodes) || "") - osm << (render(@way) || "") -end diff --git a/app/views/api/ways/show.json.jbuilder b/app/views/api/ways/show.json.jbuilder index 92e570d86..adf8beab2 100644 --- a/app/views/api/ways/show.json.jbuilder +++ b/app/views/api/ways/show.json.jbuilder @@ -1,5 +1,6 @@ json.partial! "api/root_attributes" json.elements do + json.array! @nodes, :partial => "/api/nodes/node", :as => :node if @nodes json.array! [@way], :partial => "way", :as => :way end diff --git a/app/views/api/ways/show.xml.builder b/app/views/api/ways/show.xml.builder index d520a0844..72b22e737 100644 --- a/app/views/api/ways/show.xml.builder +++ b/app/views/api/ways/show.xml.builder @@ -1,5 +1,6 @@ xml.instruct! xml.osm(OSM::API.new.xml_root_attributes) do |osm| + osm << (render(@nodes) || "") if @nodes osm << (render(@way) || "") end diff --git a/config/routes.rb b/config/routes.rb index b562ca9f4..a7631849c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,7 +37,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/full" => "ways#full", :as => :way_full, :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+/ @@ -55,7 +54,11 @@ OpenStreetMap::Application.routes.draw do put "node/create" => "nodes#create", :as => nil resources :ways, :only => [:index, :create] - resources :ways, :path => "way", :id => /\d+/, :only => [:show, :update, :destroy] + resources :ways, :path => "way", :id => /\d+/, :only => [:show, :update, :destroy] do + member do + get :full, :action => :show, :full => true, :as => nil + end + end put "way/create" => "ways#create", :as => nil resources :relations, :only => [:index, :create] diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb index 2ff5e6f29..191f9a820 100644 --- a/test/controllers/api/ways_controller_test.rb +++ b/test/controllers/api/ways_controller_test.rb @@ -27,11 +27,11 @@ module Api ) assert_routing( { :path => "/api/0.6/way/1/full", :method => :get }, - { :controller => "api/ways", :action => "full", :id => "1" } + { :controller => "api/ways", :action => "show", :full => true, :id => "1" } ) assert_routing( { :path => "/api/0.6/way/1/full.json", :method => :get }, - { :controller => "api/ways", :action => "full", :id => "1", :format => "json" } + { :controller => "api/ways", :action => "show", :full => true, :id => "1", :format => "json" } ) assert_routing( { :path => "/api/0.6/way/1", :method => :put }, @@ -136,10 +136,10 @@ module Api ## # check the "full" mode - def test_full + def test_show_full way = create(:way_with_nodes, :nodes_count => 3) - get way_full_path(way) + get api_way_path(way, :full => true) assert_response :success @@ -154,10 +154,42 @@ module Api end end - def test_full_deleted + def test_show_full_json + way = create(:way_with_nodes, :nodes_count => 3) + + get api_way_path(way, :full => true, :format => "json") + + assert_response :success + + # Check the way is correctly returned + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 4, 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"] + assert_equal 1, js_ways[0]["version"] + + # check that each node in the way appears once in the output as a + # reference and as the node element. + js_nodes = js["elements"].filter { |e| e["type"] == "node" } + assert_equal 3, js_nodes.count + + way.nodes.each_with_index do |n, i| + assert_equal n.id, js_ways[0]["nodes"][i] + js_nodes_with_id = js_nodes.filter { |e| e["id"] == n.id } + assert_equal 1, js_nodes_with_id.count + assert_equal n.id, js_nodes_with_id[0]["id"] + assert_equal 1, js_nodes_with_id[0]["version"] + assert_equal n.lat, js_nodes_with_id[0]["lat"] + assert_equal n.lon, js_nodes_with_id[0]["lon"] + end + end + + def test_show_full_deleted way = create(:way, :deleted) - get way_full_path(way) + get api_way_path(way, :full => true) assert_response :gone end -- 2.39.5