From b77c9f863d94327fc3e579ab3a00ffcb81293cde Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Thu, 23 Jan 2025 06:00:23 +0300 Subject: [PATCH] Create trace data resource --- app/controllers/traces/data_controller.rb | 41 ++++++ app/controllers/traces_controller.rb | 24 +--- config/routes.rb | 7 +- .../traces/data_controller_test.rb | 120 ++++++++++++++++++ test/controllers/traces_controller_test.rb | 93 -------------- 5 files changed, 167 insertions(+), 118 deletions(-) create mode 100644 app/controllers/traces/data_controller.rb create mode 100644 test/controllers/traces/data_controller_test.rb diff --git a/app/controllers/traces/data_controller.rb b/app/controllers/traces/data_controller.rb new file mode 100644 index 000000000..76ed051f5 --- /dev/null +++ b/app/controllers/traces/data_controller.rb @@ -0,0 +1,41 @@ +module Traces + class DataController < ApplicationController + layout "site" + + before_action :authorize_web + before_action :set_locale + before_action :check_database_readable + + authorize_resource :class => Trace + + before_action :offline_redirect + + def show + trace = Trace.visible.find(params[:trace_id]) + + if trace.public? || (current_user && current_user == trace.user) + if Acl.no_trace_download(request.remote_ip) + head :forbidden + elsif request.format == Mime[:xml] + send_data(trace.xml_file.read, :filename => "#{trace.id}.xml", :type => request.format.to_s, :disposition => "attachment") + elsif request.format == Mime[:gpx] + send_data(trace.xml_file.read, :filename => "#{trace.id}.gpx", :type => request.format.to_s, :disposition => "attachment") + elsif trace.file.attached? + redirect_to rails_blob_path(trace.file, :disposition => "attachment") + else + send_file(trace.trace_name, :filename => "#{trace.id}#{trace.extension_name}", :type => trace.mime_type, :disposition => "attachment") + end + else + head :not_found + end + rescue ActiveRecord::RecordNotFound + head :not_found + end + + private + + def offline_redirect + render :template => "traces/offline" if Settings.status == "gpx_offline" + end + end +end diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb index 075b5a864..53c1dedd6 100644 --- a/app/controllers/traces_controller.rb +++ b/app/controllers/traces_controller.rb @@ -12,7 +12,7 @@ class TracesController < ApplicationController before_action :check_database_writable, :only => [:new, :create, :edit, :destroy] before_action :offline_warning, :only => [:mine, :show] - before_action :offline_redirect, :only => [:new, :create, :edit, :destroy, :data] + before_action :offline_redirect, :only => [:new, :create, :edit, :destroy] # Counts and selects pages of GPX traces for various criteria (by user, tags, public etc.). # target_user - if set, specifies the user to fetch traces for. if not set will fetch all traces @@ -170,28 +170,6 @@ class TracesController < ApplicationController redirect_to :action => :index, :display_name => current_user.display_name end - def data - trace = Trace.visible.find(params[:id]) - - if trace.public? || (current_user && current_user == trace.user) - if Acl.no_trace_download(request.remote_ip) - head :forbidden - elsif request.format == Mime[:xml] - send_data(trace.xml_file.read, :filename => "#{trace.id}.xml", :type => request.format.to_s, :disposition => "attachment") - elsif request.format == Mime[:gpx] - send_data(trace.xml_file.read, :filename => "#{trace.id}.gpx", :type => request.format.to_s, :disposition => "attachment") - elsif trace.file.attached? - redirect_to rails_blob_path(trace.file, :disposition => "attachment") - else - send_file(trace.trace_name, :filename => "#{trace.id}#{trace.extension_name}", :type => trace.mime_type, :disposition => "attachment") - end - else - head :not_found - end - rescue ActiveRecord::RecordNotFound - head :not_found - end - private def do_create(file, tags, description, visibility) diff --git a/config/routes.rb b/config/routes.rb index dae870a07..d8e83b97b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -213,7 +213,9 @@ OpenStreetMap::Application.routes.draw do post "/preview/:type" => "site#preview", :as => :preview # traces - resources :traces, :id => /\d+/, :except => [:show] + resources :traces, :id => /\d+/, :except => [:show] do + resource :data, :module => :traces, :only => :show + end get "/user/:display_name/traces/tag/:tag/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/traces/tag/%{tag}") get "/user/:display_name/traces/tag/:tag" => "traces#index" get "/user/:display_name/traces/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/traces") @@ -231,7 +233,8 @@ OpenStreetMap::Application.routes.draw do get "/traces/mine/page/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/traces/mine") get "/traces/mine" => "traces#mine" get "/trace/create", :to => redirect(:path => "/traces/new") - get "/trace/:id/data" => "traces#data", :id => /\d+/, :as => "trace_data" + get "/trace/:id/data", :format => false, :id => /\d+/, :to => redirect(:path => "/traces/%{id}/data") + get "/trace/:id/data.:format", :id => /\d+/, :to => redirect(:path => "/traces/%{id}/data.%{format}") get "/trace/:id/edit", :id => /\d+/, :to => redirect(:path => "/traces/%{id}/edit") namespace :traces, :path => "" do diff --git a/test/controllers/traces/data_controller_test.rb b/test/controllers/traces/data_controller_test.rb new file mode 100644 index 000000000..721520fc8 --- /dev/null +++ b/test/controllers/traces/data_controller_test.rb @@ -0,0 +1,120 @@ +require "test_helper" + +module Traces + class DataControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/traces/1/data", :method => :get }, + { :controller => "traces/data", :action => "show", :trace_id => "1" } + ) + assert_routing( + { :path => "/traces/1/data.xml", :method => :get }, + { :controller => "traces/data", :action => "show", :trace_id => "1", :format => "xml" } + ) + + get "/trace/1/data" + assert_redirected_to "/traces/1/data" + + get "/trace/1/data.xml" + assert_redirected_to "/traces/1/data.xml" + end + + # Test downloading a trace + def test_show + public_trace_file = create(:trace, :visibility => "public", :fixture => "a") + + # First with no auth, which should work since the trace is public + get trace_data_path(public_trace_file) + follow_redirect! + follow_redirect! + check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9" + + # Now with some other user, which should work since the trace is public + session_for(create(:user)) + get trace_data_path(public_trace_file) + follow_redirect! + follow_redirect! + check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9" + + # And finally we should be able to do it with the owner of the trace + session_for(public_trace_file.user) + get trace_data_path(public_trace_file) + follow_redirect! + follow_redirect! + check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9" + end + + # Test downloading a compressed trace + def test_show_compressed + identifiable_trace_file = create(:trace, :visibility => "identifiable", :fixture => "d") + + # First get the data as is + get trace_data_path(identifiable_trace_file) + follow_redirect! + follow_redirect! + check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/gzip", "gpx.gz" + + # Now ask explicitly for XML format + get trace_data_path(identifiable_trace_file, :format => "xml") + check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d", "application/xml", "xml" + + # Now ask explicitly for GPX format + get trace_data_path(identifiable_trace_file, :format => "gpx") + check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d" + end + + # Check an anonymous trace can't be downloaded by another user + def test_show_anon + anon_trace_file = create(:trace, :visibility => "private", :fixture => "b") + + # First with no auth + get trace_data_path(anon_trace_file) + assert_response :not_found + + # Now with some other user, which shouldn't work since the trace is anon + session_for(create(:user)) + get trace_data_path(anon_trace_file) + assert_response :not_found + + # And finally we should be able to do it with the owner of the trace + session_for(anon_trace_file.user) + get trace_data_path(anon_trace_file) + follow_redirect! + follow_redirect! + check_trace_data anon_trace_file, "db4cb5ed2d7d2b627b3b504296c4f701" + end + + # Test downloading a trace that doesn't exist + def test_show_not_found + deleted_trace_file = create(:trace, :deleted) + + # First with a trace that has never existed + get trace_data_path(0) + assert_response :not_found + + # Now with a trace that has been deleted + session_for(deleted_trace_file.user) + get trace_data_path(deleted_trace_file) + assert_response :not_found + end + + def test_show_offline + public_trace_file = create(:trace, :visibility => "public", :fixture => "a") + with_settings(:status => "gpx_offline") do + get trace_data_path(public_trace_file) + assert_response :success + assert_template :offline + end + end + + private + + def check_trace_data(trace, digest, content_type = "application/gpx+xml", extension = "gpx") + assert_equal digest, Digest::MD5.hexdigest(response.body) + assert_equal content_type, response.media_type + assert_equal "attachment; filename=\"#{trace.id}.#{extension}\"; filename*=UTF-8''#{trace.id}.#{extension}", @response.header["Content-Disposition"] + end + end +end diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb index 78d714289..45746e213 100644 --- a/test/controllers/traces_controller_test.rb +++ b/test/controllers/traces_controller_test.rb @@ -43,14 +43,6 @@ class TracesControllerTest < ActionDispatch::IntegrationTest { :path => "/traces", :method => :post }, { :controller => "traces", :action => "create" } ) - assert_routing( - { :path => "/trace/1/data", :method => :get }, - { :controller => "traces", :action => "data", :id => "1" } - ) - assert_routing( - { :path => "/trace/1/data.xml", :method => :get }, - { :controller => "traces", :action => "data", :id => "1", :format => "xml" } - ) assert_routing( { :path => "/traces/1/edit", :method => :get }, { :controller => "traces", :action => "edit", :id => "1" } @@ -368,85 +360,6 @@ class TracesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to :action => :index end - # Test downloading a trace - def test_data - public_trace_file = create(:trace, :visibility => "public", :fixture => "a") - - # First with no auth, which should work since the trace is public - get trace_data_path(:display_name => public_trace_file.user.display_name, :id => public_trace_file) - follow_redirect! - follow_redirect! - check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9" - - # Now with some other user, which should work since the trace is public - session_for(create(:user)) - get trace_data_path(:display_name => public_trace_file.user.display_name, :id => public_trace_file) - follow_redirect! - follow_redirect! - check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9" - - # And finally we should be able to do it with the owner of the trace - session_for(public_trace_file.user) - get trace_data_path(:display_name => public_trace_file.user.display_name, :id => public_trace_file) - follow_redirect! - follow_redirect! - check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9" - end - - # Test downloading a compressed trace - def test_data_compressed - identifiable_trace_file = create(:trace, :visibility => "identifiable", :fixture => "d") - - # First get the data as is - get trace_data_path(:display_name => identifiable_trace_file.user.display_name, :id => identifiable_trace_file) - follow_redirect! - follow_redirect! - check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/gzip", "gpx.gz" - - # Now ask explicitly for XML format - get trace_data_path(:display_name => identifiable_trace_file.user.display_name, :id => identifiable_trace_file.id, :format => "xml") - check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d", "application/xml", "xml" - - # Now ask explicitly for GPX format - get trace_data_path(:display_name => identifiable_trace_file.user.display_name, :id => identifiable_trace_file.id, :format => "gpx") - check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d" - end - - # Check an anonymous trace can't be downloaded by another user - def test_data_anon - anon_trace_file = create(:trace, :visibility => "private", :fixture => "b") - - # First with no auth - get trace_data_path(:display_name => anon_trace_file.user.display_name, :id => anon_trace_file) - assert_response :not_found - - # Now with some other user, which shouldn't work since the trace is anon - session_for(create(:user)) - get trace_data_path(:display_name => anon_trace_file.user.display_name, :id => anon_trace_file) - assert_response :not_found - - # And finally we should be able to do it with the owner of the trace - session_for(anon_trace_file.user) - get trace_data_path(:display_name => anon_trace_file.user.display_name, :id => anon_trace_file) - follow_redirect! - follow_redirect! - check_trace_data anon_trace_file, "db4cb5ed2d7d2b627b3b504296c4f701" - end - - # Test downloading a trace that doesn't exist - def test_data_not_found - deleted_trace_file = create(:trace, :deleted) - - # First with a trace that has never existed - get trace_data_path(:display_name => create(:user).display_name, :id => 0) - assert_response :not_found - - # Now with a trace that has been deleted - session_for(deleted_trace_file.user) - get trace_data_path(:display_name => deleted_trace_file.user.display_name, :id => deleted_trace_file) - assert_response :not_found - end - # Test fetching the new trace page def test_new_get # First with no auth @@ -680,10 +593,4 @@ class TracesControllerTest < ActionDispatch::IntegrationTest assert_select "td", trace.description end end - - def check_trace_data(trace, digest, content_type = "application/gpx+xml", extension = "gpx") - assert_equal digest, Digest::MD5.hexdigest(response.body) - assert_equal content_type, response.media_type - assert_equal "attachment; filename=\"#{trace.id}.#{extension}\"; filename*=UTF-8''#{trace.id}.#{extension}", @response.header["Content-Disposition"] - end end -- 2.39.5