From 041059690882d6420fd8bd087bb28e5cb2ea9123 Mon Sep 17 00:00:00 2001
From: Tom Hughes <tom@compton.nu>
Date: Sun, 17 Oct 2021 16:00:22 +0100
Subject: [PATCH 1/1] Switch traces to use ActiveStorage

---
 app/controllers/api/traces_controller.rb      |  44 +--
 app/controllers/traces_controller.rb          |  83 ++---
 app/models/trace.rb                           | 283 ++++++++++--------
 config/settings.yml                           |   3 +
 config/settings/test.yml                      |   3 +
 lib/gpx.rb                                    |   4 +-
 .../controllers/api/traces_controller_test.rb |  16 +-
 test/controllers/traces_controller_test.rb    |  19 +-
 test/factories/traces.rb                      |  14 +-
 test/models/trace_test.rb                     |  54 ++--
 10 files changed, 252 insertions(+), 271 deletions(-)

diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb
index 43bbeeb1d..aa8a06000 100644
--- a/app/controllers/api/traces_controller.rb
+++ b/app/controllers/api/traces_controller.rb
@@ -54,6 +54,8 @@ module Api
           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
@@ -97,12 +99,6 @@ module Api
       # Sanitise the user's filename
       name = file.original_filename.gsub(/[^a-zA-Z0-9.]/, "_")
 
-      # Get a temporary filename...
-      filename = "/tmp/#{rand}"
-
-      # ...and save the uploaded file to that location
-      File.open(filename, "wb") { |f| f.write(file.read) }
-
       # Create the trace object, falsely marked as already
       # inserted to stop the import daemon trying to load it
       trace = Trace.new(
@@ -110,40 +106,14 @@ module Api
         :tagstring => tags,
         :description => description,
         :visibility => visibility,
-        :inserted => true,
+        :inserted => false,
         :user => current_user,
-        :timestamp => Time.now.getutc
+        :timestamp => Time.now.getutc,
+        :file => file
       )
 
-      if trace.valid?
-        Trace.transaction do
-          begin
-            # Save the trace object
-            trace.save!
-
-            # Rename the temporary file to the final name
-            FileUtils.mv(filename, trace.trace_name)
-          rescue StandardError
-            # Remove the file as we have failed to update the database
-            FileUtils.rm_f(filename)
-
-            # Pass the exception on
-            raise
-          end
-
-          begin
-            # Clear the inserted flag to make the import daemon load the trace
-            trace.inserted = false
-            trace.save!
-          rescue StandardError
-            # Remove the file as we have failed to update the database
-            FileUtils.rm_f(trace.trace_name)
-
-            # Pass the exception on
-            raise
-          end
-        end
-      end
+      # Save the trace object
+      trace.save!
 
       # Finally save the user's preferred privacy level
       if pref = current_user.preferences.where(:k => "gps.trace.visibility").first
diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb
index 9ebad1613..a9dbc8539 100644
--- a/app/controllers/traces_controller.rb
+++ b/app/controllers/traces_controller.rb
@@ -99,12 +99,8 @@ class TracesController < ApplicationController
     logger.info(params[:trace][:gpx_file].class.name)
 
     if params[:trace][:gpx_file].respond_to?(:read)
-      begin
-        @trace = do_create(params[:trace][:gpx_file], params[:trace][:tagstring],
-                           params[:trace][:description], params[:trace][:visibility])
-      rescue StandardError => e
-        logger.debug e
-      end
+      @trace = do_create(params[:trace][:gpx_file], params[:trace][:tagstring],
+                         params[:trace][:description], params[:trace][:visibility])
 
       if @trace.id
         flash[:notice] = t ".trace_uploaded"
@@ -141,6 +137,8 @@ class TracesController < ApplicationController
         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
@@ -217,8 +215,12 @@ class TracesController < ApplicationController
 
     if trace.visible? && trace.inserted?
       if trace.public? || (current_user && current_user == trace.user)
-        expires_in 7.days, :private => !trace.public?, :public => trace.public?
-        send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => "image/gif", :disposition => "inline")
+        if trace.icon.attached?
+          redirect_to rails_blob_path(trace.image, :disposition => "inline")
+        else
+          expires_in 7.days, :private => !trace.public?, :public => trace.public?
+          send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => "image/gif", :disposition => "inline")
+        end
       else
         head :forbidden
       end
@@ -234,8 +236,12 @@ class TracesController < ApplicationController
 
     if trace.visible? && trace.inserted?
       if trace.public? || (current_user && current_user == trace.user)
-        expires_in 7.days, :private => !trace.public?, :public => trace.public?
-        send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => "image/gif", :disposition => "inline")
+        if trace.icon.attached?
+          redirect_to rails_blob_path(trace.icon, :disposition => "inline")
+        else
+          expires_in 7.days, :private => !trace.public?, :public => trace.public?
+          send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => "image/gif", :disposition => "inline")
+        end
       else
         head :forbidden
       end
@@ -252,62 +258,29 @@ class TracesController < ApplicationController
     # Sanitise the user's filename
     name = file.original_filename.gsub(/[^a-zA-Z0-9.]/, "_")
 
-    # Get a temporary filename...
-    filename = "/tmp/#{rand}"
-
-    # ...and save the uploaded file to that location
-    File.open(filename, "wb") { |f| f.write(file.read) }
-
-    # Create the trace object, falsely marked as already
-    # inserted to stop the import daemon trying to load it
+    # Create the trace object
     trace = Trace.new(
       :name => name,
       :tagstring => tags,
       :description => description,
       :visibility => visibility,
-      :inserted => true,
+      :inserted => false,
       :user => current_user,
-      :timestamp => Time.now.getutc
+      :timestamp => Time.now.getutc,
+      :file => file
     )
 
-    if trace.valid?
-      Trace.transaction do
-        begin
-          # Save the trace object
-          trace.save!
-
-          # Rename the temporary file to the final name
-          FileUtils.mv(filename, trace.trace_name)
-        rescue StandardError
-          # Remove the file as we have failed to update the database
-          FileUtils.rm_f(filename)
-
-          # Pass the exception on
-          raise
-        end
-
-        begin
-          # Clear the inserted flag to make the import daemon load the trace
-          trace.inserted = false
-          trace.save!
-        rescue StandardError
-          # Remove the file as we have failed to update the database
-          FileUtils.rm_f(trace.trace_name)
-
-          # Pass the exception on
-          raise
-        end
+    # Save the trace object
+    if trace.save
+      # Finally save the user's preferred privacy level
+      if pref = current_user.preferences.where(:k => "gps.trace.visibility").first
+        pref.v = visibility
+        pref.save
+      else
+        current_user.preferences.create(:k => "gps.trace.visibility", :v => visibility)
       end
     end
 
-    # Finally save the user's preferred privacy level
-    if pref = current_user.preferences.where(:k => "gps.trace.visibility").first
-      pref.v = visibility
-      pref.save
-    else
-      current_user.preferences.create(:k => "gps.trace.visibility", :v => visibility)
-    end
-
     trace
   end
 
diff --git a/app/models/trace.rb b/app/models/trace.rb
index b3d87fc09..17f8ebc1e 100644
--- a/app/models/trace.rb
+++ b/app/models/trace.rb
@@ -39,6 +39,10 @@ class Trace < ApplicationRecord
   scope :visible_to_all, -> { where(:visibility => %w[public identifiable]) }
   scope :tagged, ->(t) { joins(:tags).where(:gpx_file_tags => { :tag => t }) }
 
+  has_one_attached :file, :service => Settings.trace_file_storage
+  has_one_attached :image, :service => Settings.trace_image_storage
+  has_one_attached :icon, :service => Settings.trace_icon_storage
+
   validates :user, :presence => true, :associated => true
   validates :name, :presence => true, :length => 1..255, :characters => true
   validates :description, :presence => { :on => :create }, :length => 1..255, :characters => true
@@ -46,6 +50,7 @@ class Trace < ApplicationRecord
   validates :visibility, :inclusion => %w[private public trackable identifiable]
 
   after_destroy :remove_files
+  after_save :set_filename
 
   def tagstring
     tags.collect(&:tag).join(", ")
@@ -68,6 +73,18 @@ class Trace < ApplicationRecord
                 end
   end
 
+  def file=(attachable)
+    case attachable
+    when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile
+      super(:io => attachable,
+            :filename => attachable.original_filename,
+            :content_type => content_type(attachable.path),
+            :identify => false)
+    else
+      super(attachable)
+    end
+  end
+
   def public?
     visibility == "public" || visibility == "identifiable"
   end
@@ -80,29 +97,27 @@ class Trace < ApplicationRecord
     visibility == "identifiable"
   end
 
-  def large_picture=(data)
-    f = File.new(large_picture_name, "wb")
-    f.syswrite(data)
-    f.close
-  end
-
-  def icon_picture=(data)
-    f = File.new(icon_picture_name, "wb")
-    f.syswrite(data)
-    f.close
-  end
-
   def large_picture
-    f = File.new(large_picture_name, "rb")
-    data = f.sysread(File.size(f.path))
-    f.close
+    if image.attached?
+      data = image.blob.download
+    else
+      f = File.new(large_picture_name, "rb")
+      data = f.sysread(File.size(f.path))
+      f.close
+    end
+
     data
   end
 
   def icon_picture
-    f = File.new(icon_picture_name, "rb")
-    data = f.sysread(File.size(f.path))
-    f.close
+    if icon.attached?
+      data = icon.blob.download
+    else
+      f = File.new(icon_picture_name, "rb")
+      data = f.sysread(File.size(f.path))
+      f.close
+    end
+
     data
   end
 
@@ -119,46 +134,22 @@ class Trace < ApplicationRecord
   end
 
   def mime_type
-    filetype = Open3.capture2("/usr/bin/file", "-Lbz", trace_name).first.chomp
-    gzipped = filetype.include?("gzip compressed")
-    bzipped = filetype.include?("bzip2 compressed")
-    zipped = filetype.include?("Zip archive")
-    tarred = filetype.include?("tar archive")
-
-    if gzipped
-      "application/x-gzip"
-    elsif bzipped
-      "application/x-bzip2"
-    elsif zipped
-      "application/x-zip"
-    elsif tarred
-      "application/x-tar"
+    if file.attached?
+      file.content_type
     else
-      "application/gpx+xml"
+      content_type(trace_name)
     end
   end
 
   def extension_name
-    filetype = Open3.capture2("/usr/bin/file", "-Lbz", trace_name).first.chomp
-    gzipped = filetype.include?("gzip compressed")
-    bzipped = filetype.include?("bzip2 compressed")
-    zipped = filetype.include?("Zip archive")
-    tarred = filetype.include?("tar archive")
-
-    if tarred && gzipped
-      ".tar.gz"
-    elsif tarred && bzipped
-      ".tar.bz2"
-    elsif tarred
-      ".tar"
-    elsif gzipped
-      ".gpx.gz"
-    elsif bzipped
-      ".gpx.bz2"
-    elsif zipped
-      ".zip"
-    else
-      ".gpx"
+    case mime_type
+    when "application/x-tar+gzip" then ".tar.gz"
+    when "application/x-tar+x-bzip2" then ".tar.bz2"
+    when "application/x-tar" then ".tar"
+    when "application/zip" then ".zip"
+    when "application/gzip" then ".gpx.gz"
+    when "application/x-bzip2" then ".gpx.bz2"
+    else ".gpx"
     end
   end
 
@@ -207,105 +198,135 @@ class Trace < ApplicationRecord
   end
 
   def xml_file
-    filetype = Open3.capture2("/usr/bin/file", "-Lbz", trace_name).first.chomp
-    gzipped = filetype.include?("gzip compressed")
-    bzipped = filetype.include?("bzip2 compressed")
-    zipped = filetype.include?("Zip archive")
-    tarred = filetype.include?("tar archive")
-
-    if gzipped || bzipped || zipped || tarred
-      file = Tempfile.new("trace.#{id}")
-
-      if tarred && gzipped
-        system("tar", "-zxOf", trace_name, :out => file.path)
-      elsif tarred && bzipped
-        system("tar", "-jxOf", trace_name, :out => file.path)
-      elsif tarred
-        system("tar", "-xOf", trace_name, :out => file.path)
-      elsif gzipped
-        system("gunzip", "-c", trace_name, :out => file.path)
-      elsif bzipped
-        system("bunzip2", "-c", trace_name, :out => file.path)
-      elsif zipped
-        system("unzip", "-p", trace_name, "-x", "__MACOSX/*", :out => file.path, :err => "/dev/null")
+    with_trace_file do |trace_name|
+      filetype = Open3.capture2("/usr/bin/file", "-Lbz", trace_name).first.chomp
+      gzipped = filetype.include?("gzip compressed")
+      bzipped = filetype.include?("bzip2 compressed")
+      zipped = filetype.include?("Zip archive")
+      tarred = filetype.include?("tar archive")
+
+      if gzipped || bzipped || zipped || tarred
+        file = Tempfile.new("trace.#{id}")
+
+        if tarred && gzipped
+          system("tar", "-zxOf", trace_name, :out => file.path)
+        elsif tarred && bzipped
+          system("tar", "-jxOf", trace_name, :out => file.path)
+        elsif tarred
+          system("tar", "-xOf", trace_name, :out => file.path)
+        elsif gzipped
+          system("gunzip", "-c", trace_name, :out => file.path)
+        elsif bzipped
+          system("bunzip2", "-c", trace_name, :out => file.path)
+        elsif zipped
+          system("unzip", "-p", trace_name, "-x", "__MACOSX/*", :out => file.path, :err => "/dev/null")
+        end
+
+        file.unlink
+      else
+        file = File.open(trace_name)
       end
 
-      file.unlink
-    else
-      file = File.open(trace_name)
+      file
     end
-
-    file
   end
 
   def import
     logger.info("GPX Import importing #{name} (#{id}) from #{user.email}")
 
-    gpx = GPX::File.new(trace_name)
-
-    f_lat = 0
-    f_lon = 0
-    first = true
-
-    # If there are any existing points for this trace then delete them
-    Tracepoint.where(:gpx_id => id).delete_all
-
-    gpx.points.each_slice(1_000) do |points|
-      # Gather the trace points together for a bulk import
-      tracepoints = []
+    with_trace_file do |trace_name|
+      gpx = GPX::File.new(trace_name)
+
+      f_lat = 0
+      f_lon = 0
+      first = true
+
+      # If there are any existing points for this trace then delete them
+      Tracepoint.where(:gpx_id => id).delete_all
+
+      gpx.points.each_slice(1_000) do |points|
+        # Gather the trace points together for a bulk import
+        tracepoints = []
+
+        points.each do |point|
+          if first
+            f_lat = point.latitude
+            f_lon = point.longitude
+            first = false
+          end
+
+          tp = Tracepoint.new
+          tp.lat = point.latitude
+          tp.lon = point.longitude
+          tp.altitude = point.altitude
+          tp.timestamp = point.timestamp
+          tp.gpx_id = id
+          tp.trackid = point.segment
+          tracepoints << tp
+        end
 
-      points.each do |point|
-        if first
-          f_lat = point.latitude
-          f_lon = point.longitude
-          first = false
+        # Run the before_save and before_create callbacks, and then import them in bulk with activerecord-import
+        tracepoints.each do |tp|
+          tp.run_callbacks(:save) { false }
+          tp.run_callbacks(:create) { false }
         end
 
-        tp = Tracepoint.new
-        tp.lat = point.latitude
-        tp.lon = point.longitude
-        tp.altitude = point.altitude
-        tp.timestamp = point.timestamp
-        tp.gpx_id = id
-        tp.trackid = point.segment
-        tracepoints << tp
+        Tracepoint.import!(tracepoints)
       end
 
-      # Run the before_save and before_create callbacks, and then import them in bulk with activerecord-import
-      tracepoints.each do |tp|
-        tp.run_callbacks(:save) { false }
-        tp.run_callbacks(:create) { false }
+      if gpx.actual_points.positive?
+        max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude)
+        min_lat = Tracepoint.where(:gpx_id => id).minimum(:latitude)
+        max_lon = Tracepoint.where(:gpx_id => id).maximum(:longitude)
+        min_lon = Tracepoint.where(:gpx_id => id).minimum(:longitude)
+
+        max_lat = max_lat.to_f / 10000000
+        min_lat = min_lat.to_f / 10000000
+        max_lon = max_lon.to_f / 10000000
+        min_lon = min_lon.to_f / 10000000
+
+        self.latitude = f_lat
+        self.longitude = f_lon
+        image.attach(:io => gpx.picture(min_lat, min_lon, max_lat, max_lon, gpx.actual_points), :filename => "#{id}.gif", :content_type => "image/gif")
+        icon.attach(:io => gpx.icon(min_lat, min_lon, max_lat, max_lon), :filename => "#{id}_icon.gif", :content_type => "image/gif")
+        self.size = gpx.actual_points
+        self.inserted = true
+        save!
       end
 
-      Tracepoint.import!(tracepoints)
-    end
+      logger.info "done trace #{id}"
 
-    if gpx.actual_points.positive?
-      max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude)
-      min_lat = Tracepoint.where(:gpx_id => id).minimum(:latitude)
-      max_lon = Tracepoint.where(:gpx_id => id).maximum(:longitude)
-      min_lon = Tracepoint.where(:gpx_id => id).minimum(:longitude)
-
-      max_lat = max_lat.to_f / 10000000
-      min_lat = min_lat.to_f / 10000000
-      max_lon = max_lon.to_f / 10000000
-      min_lon = min_lon.to_f / 10000000
-
-      self.latitude = f_lat
-      self.longitude = f_lon
-      self.large_picture = gpx.picture(min_lat, min_lon, max_lat, max_lon, gpx.actual_points)
-      self.icon_picture = gpx.icon(min_lat, min_lon, max_lat, max_lon)
-      self.size = gpx.actual_points
-      self.inserted = true
-      save!
+      gpx
     end
+  end
 
-    logger.info "done trace #{id}"
+  private
 
-    gpx
+  def content_type(file)
+    case Open3.capture2("/usr/bin/file", "-Lbz", file).first.chomp
+    when /.*\btar archive\b.*\bgzip\b/ then "application/x-tar+gzip"
+    when /.*\btar archive\b.*\bbzip2\b/ then "application/x-tar+x-bzip2"
+    when /.*\btar archive\b/ then "application/x-tar"
+    when /.*\bZip archive\b/ then "application/zip"
+    when /.*\bXML\b.*\bgzip\b/ then "application/gzip"
+    when /.*\bXML\b.*\bbzip2\b/ then "application/x-bzip2"
+    else "application/gpx+xml"
+    end
   end
 
-  private
+  def with_trace_file
+    if file.attached?
+      file.open do |file|
+        yield file.path
+      end
+    else
+      yield trace_name
+    end
+  end
+
+  def set_filename
+    file.blob.update(:filename => "#{id}#{extension_name}") if file.attached?
+  end
 
   def remove_files
     FileUtils.rm_f(trace_name)
diff --git a/config/settings.yml b/config/settings.yml
index 929df7b8a..ffee16114 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -134,3 +134,6 @@ smtp_user_name: null
 smtp_password: null
 # Storage services
 avatar_storage: "local"
+trace_file_storage: "local"
+trace_image_storage: "local"
+trace_icon_storage: "local"
diff --git a/config/settings/test.yml b/config/settings/test.yml
index 77afe95ee..3d940aec9 100644
--- a/config/settings/test.yml
+++ b/config/settings/test.yml
@@ -21,3 +21,6 @@ wikipedia_auth_secret: "dummy"
 server_url: "test.host"
 # Storage services for testing
 avatar_storage: "test"
+trace_file_storage: "test"
+trace_image_storage: "test"
+trace_icon_storage: "test"
diff --git a/lib/gpx.rb b/lib/gpx.rb
index 71b2823b8..3dc448e46 100644
--- a/lib/gpx.rb
+++ b/lib/gpx.rb
@@ -121,7 +121,7 @@ module GPX
 
       output = StringIO.new
       image.export(output)
-      output.read
+      output
     end
 
     def icon(min_lat, min_lon, max_lat, max_lon)
@@ -161,7 +161,7 @@ module GPX
         end
       end
 
-      image.gif
+      StringIO.new(image.gif)
     end
   end
 
diff --git a/test/controllers/api/traces_controller_test.rb b/test/controllers/api/traces_controller_test.rb
index 52df899ba..9719c7c92 100644
--- a/test/controllers/api/traces_controller_test.rb
+++ b/test/controllers/api/traces_controller_test.rb
@@ -119,11 +119,15 @@ module Api
       # Now with some other user, which should work since the trace is public
       auth_header = basic_authorization_header create(:user).display_name, "test"
       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 = basic_authorization_header public_trace_file.user.display_name, "test"
       get api_trace_data_path(public_trace_file), :headers => auth_header
+      follow_redirect!
+      follow_redirect!
       check_trace_data public_trace_file, "848caa72f2f456d1bd6a0fdf228aa1b9"
     end
 
@@ -136,7 +140,9 @@ module Api
 
       # First get the data as is
       get api_trace_data_path(identifiable_trace_file), :headers => auth_header
-      check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/x-gzip", "gpx.gz"
+      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
@@ -163,6 +169,8 @@ module Api
       # And finally we should be able to do it with the owner of the trace
       auth_header = basic_authorization_header anon_trace_file.user.display_name, "test"
       get api_trace_data_path(anon_trace_file), :headers => auth_header
+      follow_redirect!
+      follow_redirect!
       check_trace_data anon_trace_file, "db4cb5ed2d7d2b627b3b504296c4f701"
     end
 
@@ -211,7 +219,7 @@ module Api
       assert_equal %w[new trace], trace.tags.order(:tag).collect(&:tag)
       assert_equal "trackable", trace.visibility
       assert_not trace.inserted
-      assert_equal File.new(fixture).read, File.new(trace.trace_name).read
+      assert_equal File.new(fixture).read, trace.file.blob.download
       trace.destroy
       assert_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
 
@@ -229,7 +237,7 @@ module Api
       assert_equal %w[new trace], trace.tags.order(:tag).collect(&:tag)
       assert_equal "public", trace.visibility
       assert_not trace.inserted
-      assert_equal File.new(fixture).read, File.new(trace.trace_name).read
+      assert_equal File.new(fixture).read, trace.file.blob.download
       trace.destroy
       assert_equal "public", user.preferences.where(:k => "gps.trace.visibility").first.v
 
@@ -248,7 +256,7 @@ module Api
       assert_equal %w[new trace], trace.tags.order(:tag).collect(&:tag)
       assert_equal "private", trace.visibility
       assert_not trace.inserted
-      assert_equal File.new(fixture).read, File.new(trace.trace_name).read
+      assert_equal File.new(fixture).read, trace.file.blob.download
       trace.destroy
       assert_equal "private", second_user.preferences.where(:k => "gps.trace.visibility").first.v
     end
diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb
index 1c3dc2d31..48b5c457f 100644
--- a/test/controllers/traces_controller_test.rb
+++ b/test/controllers/traces_controller_test.rb
@@ -369,16 +369,22 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
 
     # 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
 
@@ -388,7 +394,9 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
 
     # First get the data as is
     get trace_data_path(:display_name => identifiable_trace_file.user.display_name, :id => identifiable_trace_file)
-    check_trace_data identifiable_trace_file, "c6422a3d8750faae49ed70e7e8a51b93", "application/x-gzip", "gpx.gz"
+    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")
@@ -415,6 +423,8 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
     # 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
 
@@ -598,7 +608,7 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
     assert_equal %w[new trace], trace.tags.order(:tag).collect(&:tag)
     assert_equal "trackable", trace.visibility
     assert_not trace.inserted
-    assert_equal File.new(fixture).read, File.new(trace.trace_name).read
+    assert_equal File.new(fixture).read, trace.file.blob.download
     trace.destroy
     assert_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
   end
@@ -789,19 +799,22 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
   end
 
   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
 
   def check_trace_picture(trace)
+    follow_redirect!
+    follow_redirect!
     assert_response :success
     assert_equal "image/gif", response.media_type
     assert_equal trace.large_picture, response.body
   end
 
   def check_trace_icon(trace)
+    follow_redirect!
+    follow_redirect!
     assert_response :success
     assert_equal "image/gif", response.media_type
     assert_equal trace.icon_picture, response.body
diff --git a/test/factories/traces.rb b/test/factories/traces.rb
index 961d52988..687339e7f 100644
--- a/test/factories/traces.rb
+++ b/test/factories/traces.rb
@@ -17,14 +17,14 @@ FactoryBot.define do
       fixture { nil }
     end
 
-    after(:create) do |trace, evaluator|
+    after(:build) do |user, evaluator|
       if evaluator.fixture
-        FileUtils.copy(Rails.root.join("test", "gpx", "fixtures", "#{evaluator.fixture}.gpx"),
-                       File.join(Settings.gpx_trace_dir, "#{trace.id}.gpx"))
-        FileUtils.copy(Rails.root.join("test", "gpx", "fixtures", "#{evaluator.fixture}.gif"),
-                       File.join(Settings.gpx_image_dir, "#{trace.id}.gif"))
-        FileUtils.copy(Rails.root.join("test", "gpx", "fixtures", "#{evaluator.fixture}_icon.gif"),
-                       File.join(Settings.gpx_image_dir, "#{trace.id}_icon.gif"))
+        user.file.attach(Rack::Test::UploadedFile.new(Rails.root.join("test", "gpx", "fixtures", "#{evaluator.fixture}.gpx")))
+
+        if evaluator.inserted
+          user.image.attach(Rack::Test::UploadedFile.new(Rails.root.join("test", "gpx", "fixtures", "#{evaluator.fixture}.gif")))
+          user.icon.attach(Rack::Test::UploadedFile.new(Rails.root.join("test", "gpx", "fixtures", "#{evaluator.fixture}_icon.gif")))
+        end
       end
     end
   end
diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb
index 9682d6b28..1322964c2 100644
--- a/test/models/trace_test.rb
+++ b/test/models/trace_test.rb
@@ -136,11 +136,11 @@ class TraceTest < ActiveSupport::TestCase
     check_mime_type("a", "application/gpx+xml")
     check_mime_type("b", "application/gpx+xml")
     check_mime_type("c", "application/x-bzip2")
-    check_mime_type("d", "application/x-gzip")
-    check_mime_type("f", "application/x-zip")
+    check_mime_type("d", "application/gzip")
+    check_mime_type("f", "application/zip")
     check_mime_type("g", "application/x-tar")
-    check_mime_type("h", "application/x-gzip")
-    check_mime_type("i", "application/x-bzip2")
+    check_mime_type("h", "application/x-tar+gzip")
+    check_mime_type("i", "application/x-tar+x-bzip2")
   end
 
   def test_extension_name
@@ -168,24 +168,16 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_large_picture
     picture = File.read(Rails.root.join("test/gpx/fixtures/a.gif"), :mode => "rb")
+    trace = create(:trace, :fixture => "a")
 
-    trace = Trace.create
-    trace.large_picture = picture
-    assert_equal "7c841749e084ee4a5d13f12cd3bef456", md5sum(File.new(trace.large_picture_name))
     assert_equal picture, trace.large_picture
-
-    trace.destroy
   end
 
   def test_icon_picture
     picture = File.read(Rails.root.join("test/gpx/fixtures/a_icon.gif"), :mode => "rb")
+    trace = create(:trace, :fixture => "a")
 
-    trace = Trace.create
-    trace.icon_picture = picture
-    assert_equal "b47baf22ed0e85d77e808694fad0ee27", md5sum(File.new(trace.icon_picture_name))
     assert_equal picture, trace.icon_picture
-
-    trace.destroy
   end
 
   def test_import_removes_previous_tracepoints
@@ -215,29 +207,27 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_creates_icon
-    trace = create(:trace, :fixture => "a")
-    icon_path = File.join(Settings.gpx_image_dir, "#{trace.id}_icon.gif")
-    FileUtils.rm(icon_path)
-    assert_not File.exist?(icon_path)
+    trace = create(:trace, :inserted => false, :fixture => "a")
+
+    assert_not trace.icon.attached?
 
     trace.import
 
-    assert_path_exists(icon_path)
+    assert trace.icon.attached?
   end
 
   def test_import_creates_large_picture
-    trace = create(:trace, :fixture => "a")
-    large_picture_path = File.join(Settings.gpx_image_dir, "#{trace.id}.gif")
-    FileUtils.rm(large_picture_path)
-    assert_not File.exist?(large_picture_path)
+    trace = create(:trace, :inserted => false, :fixture => "a")
+
+    assert_not trace.image.attached?
 
     trace.import
 
-    assert_path_exists(large_picture_path)
+    assert trace.image.attached?
   end
 
   def test_import_handles_bz2
-    trace = create(:trace, :fixture => "c")
+    trace = create(:trace, :inserted => false, :fixture => "c")
 
     trace.import
 
@@ -245,7 +235,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_plain
-    trace = create(:trace, :fixture => "a")
+    trace = create(:trace, :inserted => false, :fixture => "a")
 
     trace.import
 
@@ -253,7 +243,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_plain_with_bom
-    trace = create(:trace, :fixture => "b")
+    trace = create(:trace, :inserted => false, :fixture => "b")
 
     trace.import
 
@@ -261,7 +251,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_gz
-    trace = create(:trace, :fixture => "d")
+    trace = create(:trace, :inserted => false, :fixture => "d")
 
     trace.import
 
@@ -269,7 +259,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_zip
-    trace = create(:trace, :fixture => "f")
+    trace = create(:trace, :inserted => false, :fixture => "f")
 
     trace.import
 
@@ -277,7 +267,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_tar
-    trace = create(:trace, :fixture => "g")
+    trace = create(:trace, :inserted => false, :fixture => "g")
 
     trace.import
 
@@ -285,7 +275,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_tar_gz
-    trace = create(:trace, :fixture => "h")
+    trace = create(:trace, :inserted => false, :fixture => "h")
 
     trace.import
 
@@ -293,7 +283,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_import_handles_tar_bz2
-    trace = create(:trace, :fixture => "i")
+    trace = create(:trace, :inserted => false, :fixture => "i")
 
     trace.import
 
-- 
2.39.5