Create trace data resource

diff --git a/app/controllers/traces/data_controller.rb b/app/controllers/traces/data_controller.rb
new file mode 100644 (file)
index 0000000..76ed051
--- /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 075b5a8649493bc216ba3b84faa6e56df0af015c..53c1dedd6aeecb27e0c82dceaeb0184ffd12816e 100644 (file)
--- 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 :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
 
   # 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
 
     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)
   private
 
   def do_create(file, tags, description, visibility)

diff --git a/config/routes.rb b/config/routes.rb
index dae870a0768d005425cbe4c5f65af2f1119e7ff0..d8e83b97b3838cdb725249dd0824b9f312af5a8f 100644 (file)
--- 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
   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")
   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 "/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
   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 (file)
index 0000000..721520f
--- /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 78d714289deebec6d6ecf8a0912ce049333d5c3d..45746e213ce9234a10e8d93f2cbe6187d790459f 100644 (file)
--- 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" }
     )
       { :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" }
     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
 
     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
   # 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
       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
 end