From: mmd Date: Thu, 27 Jun 2019 20:40:51 +0000 (+0200) Subject: Merge branch 'master' into patch/view_migration X-Git-Tag: live~3353^2~3 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/11cc4a5e601dd288d601e6e35a72d159062f18b5?hp=-c Merge branch 'master' into patch/view_migration --- 11cc4a5e601dd288d601e6e35a72d159062f18b5 diff --combined app/controllers/api/map_controller.rb index 4790e2f60,e1d1a3b37..e8a27042d --- a/app/controllers/api/map_controller.rb +++ b/app/controllers/api/map_controller.rb @@@ -19,15 -19,15 +19,15 @@@ module Ap # check boundary is sane and area within defined # see /config/application.yml begin - bbox = BoundingBox.from_bbox_params(params) - bbox.check_boundaries - bbox.check_size + @bounds = BoundingBox.from_bbox_params(params) + @bounds.check_boundaries + @bounds.check_size - rescue StandardError => err - report_error(err.message) + rescue StandardError => e + report_error(e.message) return end - nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(Settings.max_number_of_nodes + 1) + nodes = Node.bbox(@bounds).where(:visible => true).includes(:node_tags).limit(Settings.max_number_of_nodes + 1) node_ids = nodes.collect(&:id) if node_ids.length > Settings.max_number_of_nodes @@@ -35,6 -35,11 +35,6 @@@ return end - doc = OSM::API.new.get_xml_doc - - # add bounds - doc.root << bbox.add_bounds_to(XML::Node.new("bounds")) - # get ways # find which ways are needed ways = [] @@@ -57,42 -62,43 +57,42 @@@ nodes += Node.includes(:node_tags).find(nodes_to_fetch) unless nodes_to_fetch.empty? visible_nodes = {} - changeset_cache = {} - user_display_name_cache = {} - + @nodes = [] nodes.each do |node| if node.visible? - doc.root << node.to_xml_node(changeset_cache, user_display_name_cache) visible_nodes[node.id] = node + @nodes << node end end + @ways = [] way_ids = [] ways.each do |way| if way.visible? - doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache) way_ids << way.id + @ways << way end end - relations = Relation.nodes(visible_nodes.keys).visible + - Relation.ways(way_ids).visible + @relations = Relation.nodes(visible_nodes.keys).visible + + Relation.ways(way_ids).visible # we do not normally return the "other" partners referenced by an relation, # e.g. if we return a way A that is referenced by relation X, and there's # another way B also referenced, that is not returned. But we do make # an exception for cases where an relation references another *relation*; # in that case we return that as well (but we don't go recursive here) - relations += Relation.relations(relations.collect(&:id)).visible + @relations += Relation.relations(@relations.collect(&:id)).visible # this "uniq" may be slightly inefficient; it may be better to first collect and output # all node-related relations, then find the *not yet covered* way-related ones etc. - relations.uniq.each do |relation| - doc.root << relation.to_xml_node(changeset_cache, user_display_name_cache) - end + @relations.uniq! response.headers["Content-Disposition"] = "attachment; filename=\"map.osm\"" - - render :xml => doc.to_s + # Render the result + respond_to do |format| + format.xml + end end end end diff --combined config/routes.rb index 3d018a53f,002ee58ea..498c819c3 --- a/config/routes.rb +++ b/config/routes.rb @@@ -1,7 -1,8 +1,8 @@@ OpenStreetMap::Application.routes.draw do # API namespace :api do - get "capabilities" => "capabilities#show" + get "capabilities" => "capabilities#show" # Deprecated, remove when 0.6 support is removed + get "versions" => "versions#show" end scope "api/0.6" do @@@ -23,39 -24,39 +24,39 @@@ post "changeset/comment/:id/unhide" => "api/changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/ put "node/create" => "api/nodes#create" - get "node/:id/ways" => "api/ways#ways_for_node", :id => /\d+/ - get "node/:id/relations" => "api/relations#relations_for_node", :id => /\d+/ - get "node/:id/history" => "api/old_nodes#history", :id => /\d+/ + get "node/:id/ways" => "api/ways#ways_for_node", :id => /\d+/, :defaults => { :format => "xml" } + get "node/:id/relations" => "api/relations#relations_for_node", :id => /\d+/, :defaults => { :format => "xml" } + get "node/:id/history" => "api/old_nodes#history", :id => /\d+/, :defaults => { :format => "xml" } post "node/:id/:version/redact" => "api/old_nodes#redact", :version => /\d+/, :id => /\d+/ - get "node/:id/:version" => "api/old_nodes#version", :id => /\d+/, :version => /\d+/ - get "node/:id" => "api/nodes#show", :id => /\d+/ + get "node/:id/:version" => "api/old_nodes#version", :id => /\d+/, :version => /\d+/, :defaults => { :format => "xml" } + get "node/:id" => "api/nodes#show", :id => /\d+/, :defaults => { :format => "xml" } put "node/:id" => "api/nodes#update", :id => /\d+/ delete "node/:id" => "api/nodes#delete", :id => /\d+/ - get "nodes" => "api/nodes#index" + get "nodes" => "api/nodes#index", :defaults => { :format => "xml" } put "way/create" => "api/ways#create" - get "way/:id/history" => "api/old_ways#history", :id => /\d+/ - get "way/:id/full" => "api/ways#full", :id => /\d+/ - get "way/:id/relations" => "api/relations#relations_for_way", :id => /\d+/ + get "way/:id/history" => "api/old_ways#history", :id => /\d+/, :defaults => { :format => "xml" } + get "way/:id/full" => "api/ways#full", :id => /\d+/, :defaults => { :format => "xml" } + get "way/:id/relations" => "api/relations#relations_for_way", :id => /\d+/, :defaults => { :format => "xml" } post "way/:id/:version/redact" => "api/old_ways#redact", :version => /\d+/, :id => /\d+/ - get "way/:id/:version" => "api/old_ways#version", :id => /\d+/, :version => /\d+/ - get "way/:id" => "api/ways#show", :id => /\d+/ + get "way/:id/:version" => "api/old_ways#version", :id => /\d+/, :version => /\d+/, :defaults => { :format => "xml" } + get "way/:id" => "api/ways#show", :id => /\d+/, :defaults => { :format => "xml" } put "way/:id" => "api/ways#update", :id => /\d+/ delete "way/:id" => "api/ways#delete", :id => /\d+/ - get "ways" => "api/ways#index" + get "ways" => "api/ways#index", :defaults => { :format => "xml" } put "relation/create" => "api/relations#create" - get "relation/:id/relations" => "api/relations#relations_for_relation", :id => /\d+/ - get "relation/:id/history" => "api/old_relations#history", :id => /\d+/ - get "relation/:id/full" => "api/relations#full", :id => /\d+/ + get "relation/:id/relations" => "api/relations#relations_for_relation", :id => /\d+/, :defaults => { :format => "xml" } + get "relation/:id/history" => "api/old_relations#history", :id => /\d+/, :defaults => { :format => "xml" } + get "relation/:id/full" => "api/relations#full", :id => /\d+/, :defaults => { :format => "xml" } post "relation/:id/:version/redact" => "api/old_relations#redact", :version => /\d+/, :id => /\d+/ - get "relation/:id/:version" => "api/old_relations#version", :id => /\d+/, :version => /\d+/ - get "relation/:id" => "api/relations#show", :id => /\d+/ + get "relation/:id/:version" => "api/old_relations#version", :id => /\d+/, :version => /\d+/, :defaults => { :format => "xml" } + get "relation/:id" => "api/relations#show", :id => /\d+/, :defaults => { :format => "xml" } put "relation/:id" => "api/relations#update", :id => /\d+/ delete "relation/:id" => "api/relations#delete", :id => /\d+/ - get "relations" => "api/relations#index" + get "relations" => "api/relations#index", :defaults => { :format => "xml" } - get "map" => "api/map#index" + get "map" => "api/map#index", :defaults => { :format => "xml" } get "trackpoints" => "api/tracepoints#index" @@@ -87,7 -88,6 +88,6 @@@ # AMF (ActionScript) API post "amf/read" => "api/amf#amf_read" post "amf/write" => "api/amf#amf_write" - get "swf/trackpoints" => "api/swf#trackpoints" # Map notes API resources :notes, :except => [:new, :edit, :update], :constraints => { :id => /\d+/ }, :defaults => { :format => "xml" }, :controller => "api/notes" do @@@ -216,9 -216,12 +216,12 @@@ post "/trace/:id/delete" => "traces#delete", :id => /\d+/ # diary pages - match "/diary/new" => "diary_entries#new", :via => [:get, :post] - get "/diary/friends" => "diary_entries#index", :friends => true, :as => "friend_diaries" - get "/diary/nearby" => "diary_entries#index", :nearby => true, :as => "nearby_diaries" + resources :diary_entries, :path => "diary", :only => [:new, :create, :index] do + collection do + get "friends" => "diary_entries#index", :friends => true + get "nearby" => "diary_entries#index", :nearby => true + end + end get "/user/:display_name/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss } get "/diary/:language/rss" => "diary_entries#rss", :defaults => { :format => :rss } get "/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss } @@@ -226,12 -229,14 +229,14 @@@ get "/user/:display_name/diary/comments/" => "diary_entries#comments" get "/user/:display_name/diary" => "diary_entries#index" get "/diary/:language" => "diary_entries#index" - get "/diary" => "diary_entries#index" - get "/user/:display_name/diary/:id" => "diary_entries#show", :id => /\d+/, :as => :diary_entry + scope "/user/:display_name" do + resources :diary_entries, :path => "diary", :only => [:edit, :update, :show] + end post "/user/:display_name/diary/:id/newcomment" => "diary_entries#comment", :id => /\d+/ - match "/user/:display_name/diary/:id/edit" => "diary_entries#edit", :via => [:get, :post], :id => /\d+/ post "/user/:display_name/diary/:id/hide" => "diary_entries#hide", :id => /\d+/, :as => :hide_diary_entry + post "/user/:display_name/diary/:id/unhide" => "diary_entries#unhide", :id => /\d+/, :as => :unhide_diary_entry post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_entries#hidecomment", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment + post "/user/:display_name/diary/:id/unhidecomment/:comment" => "diary_entries#unhidecomment", :id => /\d+/, :comment => /\d+/, :as => :unhide_diary_comment post "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :as => :diary_entry_subscribe, :id => /\d+/ post "/user/:display_name/diary/:id/unsubscribe" => "diary_entries#unsubscribe", :as => :diary_entry_unsubscribe, :id => /\d+/ diff --combined test/controllers/api/changesets_controller_test.rb index 2d0e85c43,9f2b64c0a..bffa3ebff --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@@ -176,8 -176,8 +176,8 @@@ module Ap [0, -32, 233455644, "afg", "213"].each do |id| get :show, :params => { :id => id } assert_response :not_found, "should get a not found" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) + rescue ActionController::UrlGenerationError => e + assert_match(/No route matches/, e.to_s) end end @@@ -248,8 -248,8 +248,8 @@@ cs_ids.each do |id| put :close, :params => { :id => id } assert_response :unauthorized, "Shouldn't be able close the non-existant changeset #{id}, when not authorized" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) + rescue ActionController::UrlGenerationError => e + assert_match(/No route matches/, e.to_s) end # Now try with auth @@@ -257,8 -257,8 +257,8 @@@ cs_ids.each do |id| put :close, :params => { :id => id } assert_response :not_found, "The changeset #{id} doesn't exist, so can't be closed" - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) + rescue ActionController::UrlGenerationError => e + assert_match(/No route matches/, e.to_s) end end @@@ -1768,7 -1768,7 +1768,7 @@@ CHANGESE assert_response :success, "can't create a new node" node_id = @response.body.to_i - get :show, :params => { :id => node_id } + get :show, :params => { :id => node_id }, :format => "xml" assert_response :success, "can't read back new node" node_doc = XML::Parser.string(@response.body).parse node_xml = node_doc.find("//osm/node").first diff --combined test/controllers/api/old_nodes_controller_test.rb index e43707245,0f8954541..c94a4bb1d --- a/test/controllers/api/old_nodes_controller_test.rb +++ b/test/controllers/api/old_nodes_controller_test.rb @@@ -11,11 -11,11 +11,11 @@@ module Ap def test_routes assert_routing( { :path => "/api/0.6/node/1/history", :method => :get }, - { :controller => "api/old_nodes", :action => "history", :id => "1" } + { :controller => "api/old_nodes", :action => "history", :id => "1", :format => "xml" } ) assert_routing( { :path => "/api/0.6/node/1/2", :method => :get }, - { :controller => "api/old_nodes", :action => "version", :id => "1", :version => "2" } + { :controller => "api/old_nodes", :action => "version", :id => "1", :version => "2", :format => "xml" } ) assert_routing( { :path => "/api/0.6/node/1/2/redact", :method => :post }, @@@ -134,7 -134,7 +134,7 @@@ # check all the versions versions.each_key do |key| - get :version, :params => { :id => nodeid, :version => key.to_i } + get :version, :params => { :id => nodeid, :version => key.to_i }, :format => :xml assert_response :success, "couldn't get version #{key.to_i} of node #{nodeid}" @@@ -154,10 -154,10 +154,10 @@@ end def check_not_found_id_version(id, version) - get :version, :params => { :id => id, :version => version } + get :version, :params => { :id => id, :version => version }, :format => :xml assert_response :not_found - rescue ActionController::UrlGenerationError => ex - assert_match(/No route matches/, ex.to_s) + rescue ActionController::UrlGenerationError => e + assert_match(/No route matches/, e.to_s) end ## @@@ -234,12 -234,12 +234,12 @@@ node_v1 = node.old_nodes.find_by(:version => 1) node_v1.redact!(create(:redaction)) - get :version, :params => { :id => node_v1.node_id, :version => node_v1.version } + get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }, :format => :xml assert_response :forbidden, "Redacted node shouldn't be visible via the version API." # not even to a logged-in user basic_authorization create(:user).email, "test" - get :version, :params => { :id => node_v1.node_id, :version => node_v1.version } + get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }, :format => :xml assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in." end @@@ -250,13 -250,13 +250,13 @@@ node_v1 = node.old_nodes.find_by(:version => 1) node_v1.redact!(create(:redaction)) - get :history, :params => { :id => node_v1.node_id } + get :history, :params => { :id => node_v1.node_id }, :format => :xml assert_response :success, "Redaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0, "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history." # not even to a logged-in user basic_authorization create(:user).email, "test" - get :history, :params => { :id => node_v1.node_id } + get :history, :params => { :id => node_v1.node_id }, :format => :xml assert_response :success, "Redaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0, "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history, even when logged in." end @@@ -274,16 -274,16 +274,16 @@@ # check moderator can still see the redacted data, when passing # the appropriate flag - get :version, :params => { :id => node_v3.node_id, :version => node_v3.version } + get :version, :params => { :id => node_v3.node_id, :version => node_v3.version }, :format => :xml assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed." - get :version, :params => { :id => node_v3.node_id, :version => node_v3.version, :show_redactions => "true" } + get :version, :params => { :id => node_v3.node_id, :version => node_v3.version, :show_redactions => "true" }, :format => :xml assert_response :success, "After redaction, node should not be gone for moderator, when flag passed." # and when accessed via history - get :history, :params => { :id => node_v3.node_id } + get :history, :params => { :id => node_v3.node_id }, :format => :xml assert_response :success, "Redaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0, "node #{node_v3.node_id} version #{node_v3.version} should not be present in the history for moderators when not passing flag." - get :history, :params => { :id => node_v3.node_id, :show_redactions => "true" } + get :history, :params => { :id => node_v3.node_id, :show_redactions => "true" }, :format => :xml assert_response :success, "Redaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 1, "node #{node_v3.node_id} version #{node_v3.version} should still be present in the history for moderators when passing flag." end @@@ -302,11 -302,11 +302,11 @@@ basic_authorization create(:user).email, "test" # check can't see the redacted data - get :version, :params => { :id => node_v3.node_id, :version => node_v3.version } + get :version, :params => { :id => node_v3.node_id, :version => node_v3.version }, :format => :xml assert_response :forbidden, "Redacted node shouldn't be visible via the version API." # and when accessed via history - get :history, :params => { :id => node_v3.node_id } + get :history, :params => { :id => node_v3.node_id }, :format => :xml assert_response :success, "Redaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0, "redacted node #{node_v3.node_id} version #{node_v3.version} shouldn't be present in the history." end @@@ -354,22 -354,22 +354,22 @@@ # check moderator can now see the redacted data, when not # passing the aspecial flag - get :version, :params => { :id => node_v1.node_id, :version => node_v1.version } + get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }, :format => :xml assert_response :success, "After unredaction, node should not be gone for moderator." # and when accessed via history - get :history, :params => { :id => node_v1.node_id } + get :history, :params => { :id => node_v1.node_id }, :format => :xml assert_response :success, "Unredaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1, "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for moderators without passing flag." basic_authorization create(:user).email, "test" # check normal user can now see the redacted data - get :version, :params => { :id => node_v1.node_id, :version => node_v1.version } + get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }, :format => :xml assert_response :success, "After unredaction, node should be visible to normal users." # and when accessed via history - get :history, :params => { :id => node_v1.node_id } + get :history, :params => { :id => node_v1.node_id }, :format => :xml assert_response :success, "Unredaction shouldn't have stopped history working." assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1, "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for normal users without passing flag." end @@@ -377,7 -377,7 +377,7 @@@ private def do_redact_node(node, redaction) - get :version, :params => { :id => node.node_id, :version => node.version } + get :version, :params => { :id => node.node_id, :version => node.version }, :format => :xml assert_response :success, "should be able to get version #{node.version} of node #{node.node_id}." # now redact it @@@ -387,14 -387,14 +387,14 @@@ def check_current_version(node_id) # get the current version of the node current_node = with_controller(NodesController.new) do - get :show, :params => { :id => node_id } + get :show, :params => { :id => node_id }, :format => :xml assert_response :success, "cant get current node #{node_id}" Node.from_xml(@response.body) end assert_not_nil current_node, "getting node #{node_id} returned nil" # get the "old" version of the node from the old_node interface - get :version, :params => { :id => node_id, :version => current_node.version } + get :version, :params => { :id => node_id, :version => current_node.version }, :format => :xml assert_response :success, "cant get old node #{node_id}, v#{current_node.version}" old_node = Node.from_xml(@response.body)