cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #5390 from AntonKhorev/trace-resourceful-routes

Browse source 
Resourceful routes for traces API
This commit is contained in:
Andy Allan 2024-12-11 14:04:19 +00:00 • committed by GitHub
commit a155a2fda3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 169 additions and 132 deletions
Download patch file Download diff file Expand all files Collapse all files

2
app/abilities/api_ability.rb
View file

@ -22,7 +22,7 @@ class ApiAbility
if user&.active?
can [:comment, :close, :reopen], Note
can [:create, :show, :update, :destroy, :data], Trace
can [:create, :show, :update, :destroy], Trace
can [:details, :gpx_files], User
can [:index, :show, :update, :update_all, :destroy], UserPreference

36
app/controllers/api/traces/data_controller.rb Normal file
View file

@ -0,0 +1,36 @@
module Api
module Traces
class DataController < ApiController
before_action :set_locale
before_action :authorize
authorize_resource :trace
before_action :offline_error
def show
trace = Trace.visible.find(params[:trace_id])
if trace.public? || trace.user == current_user
if 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 :forbidden
end
end
private
def offline_error
report_error "GPX files offline for maintenance", :service_unavailable if Settings.status == "gpx_offline"
end
end
end
end

20
app/controllers/api/traces_controller.rb
View file

@ -6,7 +6,7 @@ module Api
authorize_resource
before_action :offline_error, :only => [:create, :destroy, :data]
before_action :offline_error, :only => [:create, :destroy]
skip_around_action :api_call_timeout, :only => :create
def show
@ -71,24 +71,6 @@ module Api
end
end
def data
trace = Trace.visible.find(params[:id])
if trace.public? || trace.user == current_user
if 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 :forbidden
end
end
private
def do_create(file, tags, description, visibility)

15
config/routes.rb
View file

@ -86,16 +86,17 @@ OpenStreetMap::Application.routes.draw do
end
post "/user/messages/:id" => "messages#update", :as => :api_message_update
post "gpx/create" => "traces#create"
get "gpx/:id" => "traces#show", :as => :api_trace, :id => /\d+/
put "gpx/:id" => "traces#update", :id => /\d+/
delete "gpx/:id" => "traces#destroy", :id => /\d+/
get "gpx/:id/details" => "traces#show", :id => /\d+/
get "gpx/:id/data" => "traces#data", :as => :api_trace_data
end
namespace :api, :path => "api/0.6" do
resources :traces, :path => "gpx", :only => [:create, :show, :update, :destroy], :id => /\d+/ do
scope :module => :traces do
resource :data, :only => :show
end
end
post "gpx/create" => "traces#create", :id => /\d+/, :as => :trace_create
get "gpx/:id/details" => "traces#show", :id => /\d+/, :as => :trace_details
# Map notes API
resources :notes, :except => [:new, :edit, :update], :id => /\d+/, :controller => "notes" do
collection do

114
test/controllers/api/traces/data_controller_test.rb Normal file
View file

@ -0,0 +1,114 @@
require "test_helper"
module Api
module Traces
class DataControllerTest < ActionDispatch::IntegrationTest
##
# test all routes which lead to this controller
def test_routes
assert_routing(
{ :path => "/api/0.6/gpx/1/data", :method => :get },
{ :controller => "api/traces/data", :action => "show", :trace_id => "1" }
)
assert_routing(
{ :path => "/api/0.6/gpx/1/data.xml", :method => :get },
{ :controller => "api/traces/data", :action => "show", :trace_id => "1", :format => "xml" }
)
end
# Test downloading a trace through the api
def test_show
public_trace_file = create(:trace, :visibility => "public", :fixture => "a")
# First with no auth
get api_trace_data_path(public_trace_file)
assert_response :unauthorized
# Now with some other user, which should work since the trace is public
auth_header = bearer_authorization_header
get api_trace_data_path(public_trace_file), :headers => auth_header
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
auth_header = bearer_authorization_header public_trace_file.user
get api_trace_data_path(public_trace_file), :headers => auth_header
follow_redirect!
follow_redirect!
check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9"
end
# Test downloading a compressed trace through the api
def test_data_compressed
identifiable_trace_file = create(:trace, :visibility => "identifiable", :fixture => "d")
# Authenticate as the owner of the trace we will be using
auth_header = bearer_authorization_header identifiable_trace_file.user
# First get the data as is
get api_trace_data_path(identifiable_trace_file), :headers => auth_header
follow_redirect!
follow_redirect!
check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/gzip", "gpx.gz"
# Now ask explicitly for XML format
get api_trace_data_path(identifiable_trace_file, :format => "xml"), :headers => auth_header
check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d", "application/xml", "xml"
# Now ask explicitly for GPX format
get api_trace_data_path(identifiable_trace_file, :format => "gpx"), :headers => auth_header
check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d"
end
# Check an anonymous trace can't be downloaded by another user through the api
def test_data_anon
anon_trace_file = create(:trace, :visibility => "private", :fixture => "b")
# First with no auth
get api_trace_data_path(anon_trace_file)
assert_response :unauthorized
# Now with some other user, which shouldn't work since the trace is anon
auth_header = bearer_authorization_header
get api_trace_data_path(anon_trace_file), :headers => auth_header
assert_response :forbidden
# And finally we should be able to do it with the owner of the trace
auth_header = bearer_authorization_header anon_trace_file.user
get api_trace_data_path(anon_trace_file), :headers => auth_header
follow_redirect!
follow_redirect!
check_trace_data anon_trace_file, "db4cb5ed2d7d2b627b3b504296c4f701"
end
# Test downloading a trace that doesn't exist through the api
def test_data_not_found
deleted_trace_file = create(:trace, :deleted)
# Try first with no auth, as it should require it
get api_trace_data_path(0)
assert_response :unauthorized
# Login, and try again
auth_header = bearer_authorization_header
get api_trace_data_path(0), :headers => auth_header
assert_response :not_found
# Now try a trace which did exist but has been deleted
auth_header = bearer_authorization_header deleted_trace_file.user
get api_trace_data_path(deleted_trace_file), :headers => auth_header
assert_response :not_found
end
private
def check_trace_data(trace, digest, content_type = "application/gpx+xml", extension = "gpx")
assert_response :success
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
end

114
test/controllers/api/traces_controller_test.rb
View file

@ -6,9 +6,13 @@ module Api
# test all routes which lead to this controller
def test_routes
assert_routing(
{ :path => "/api/0.6/gpx/create", :method => :post },
{ :path => "/api/0.6/gpx", :method => :post },
{ :controller => "api/traces", :action => "create" }
)
assert_recognizes(
{ :controller => "api/traces", :action => "create" },
{ :path => "/api/0.6/gpx/create", :method => :post }
)
assert_routing(
{ :path => "/api/0.6/gpx/1", :method => :get },
{ :controller => "api/traces", :action => "show", :id => "1" }
@ -25,14 +29,6 @@ module Api
{ :controller => "api/traces", :action => "show", :id => "1" },
{ :path => "/api/0.6/gpx/1/details", :method => :get }
)
assert_routing(
{ :path => "/api/0.6/gpx/1/data", :method => :get },
{ :controller => "api/traces", :action => "data", :id => "1" }
)
assert_routing(
{ :path => "/api/0.6/gpx/1/data.xml", :method => :get },
{ :controller => "api/traces", :action => "data", :id => "1", :format => "xml" }
)
end
# Check getting a specific trace through the api
@ -93,91 +89,6 @@ module Api
assert_response :not_found
end
# Test downloading a trace through the api
def test_data
public_trace_file = create(:trace, :visibility => "public", :fixture => "a")
# First with no auth
get api_trace_data_path(public_trace_file)
assert_response :unauthorized
# Now with some other user, which should work since the trace is public
auth_header = bearer_authorization_header
get api_trace_data_path(public_trace_file), :headers => auth_header
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
auth_header = bearer_authorization_header public_trace_file.user
get api_trace_data_path(public_trace_file), :headers => auth_header
follow_redirect!
follow_redirect!
check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9"
end
# Test downloading a compressed trace through the api
def test_data_compressed
identifiable_trace_file = create(:trace, :visibility => "identifiable", :fixture => "d")
# Authenticate as the owner of the trace we will be using
auth_header = bearer_authorization_header identifiable_trace_file.user
# First get the data as is
get api_trace_data_path(identifiable_trace_file), :headers => auth_header
follow_redirect!
follow_redirect!
check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/gzip", "gpx.gz"
# Now ask explicitly for XML format
get api_trace_data_path(identifiable_trace_file, :format => "xml"), :headers => auth_header
check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d", "application/xml", "xml"
# Now ask explicitly for GPX format
get api_trace_data_path(identifiable_trace_file, :format => "gpx"), :headers => auth_header
check_trace_data identifiable_trace_file, "abd6675fdf3024a84fc0a1deac147c0d"
end
# Check an anonymous trace can't be downloaded by another user through the api
def test_data_anon
anon_trace_file = create(:trace, :visibility => "private", :fixture => "b")
# First with no auth
get api_trace_data_path(anon_trace_file)
assert_response :unauthorized
# Now with some other user, which shouldn't work since the trace is anon
auth_header = bearer_authorization_header
get api_trace_data_path(anon_trace_file), :headers => auth_header
assert_response :forbidden
# And finally we should be able to do it with the owner of the trace
auth_header = bearer_authorization_header anon_trace_file.user
get api_trace_data_path(anon_trace_file), :headers => auth_header
follow_redirect!
follow_redirect!
check_trace_data anon_trace_file, "db4cb5ed2d7d2b627b3b504296c4f701"
end
# Test downloading a trace that doesn't exist through the api
def test_data_not_found
deleted_trace_file = create(:trace, :deleted)
# Try first with no auth, as it should require it
get api_trace_data_path(:id => 0)
assert_response :unauthorized
# Login, and try again
auth_header = bearer_authorization_header
get api_trace_data_path(:id => 0), :headers => auth_header
assert_response :not_found
# Now try a trace which did exist but has been deleted
auth_header = bearer_authorization_header deleted_trace_file.user
get api_trace_data_path(deleted_trace_file), :headers => auth_header
assert_response :not_found
end
# Test creating a trace through the api
def test_create
# Get file to use
@ -186,7 +97,7 @@ module Api
user = create(:user)
# First with no auth
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }
post api_traces_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }
assert_response :unauthorized
# Rewind the file
@ -200,7 +111,7 @@ module Api
# Create trace and import tracepoints in background job
perform_enqueued_jobs do
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }, :headers => auth_header
post api_traces_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }, :headers => auth_header
end
assert_response :success
@ -232,7 +143,7 @@ module Api
# Now authenticated, with the legacy public flag
assert_not_equal "public", user.preferences.find_by(:k => "gps.trace.visibility").v
auth_header = bearer_authorization_header user
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 1 }, :headers => auth_header
post api_traces_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 1 }, :headers => auth_header
assert_response :success
trace = Trace.find(response.body.to_i)
assert_equal "a.gpx", trace.name
@ -251,7 +162,7 @@ module Api
second_user = create(:user)
assert_nil second_user.preferences.find_by(:k => "gps.trace.visibility")
auth_header = bearer_authorization_header second_user
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 0 }, :headers => auth_header
post api_traces_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 0 }, :headers => auth_header
assert_response :success
trace = Trace.find(response.body.to_i)
assert_equal "a.gpx", trace.name
@ -354,13 +265,6 @@ module Api
private
def check_trace_data(trace, digest, content_type = "application/gpx+xml", extension = "gpx")
assert_response :success
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
##
# build XML for traces
# this builds a minimum viable XML for the tests in this suite