Use Active Storage to import GPX files

Gemfile

@@ -27,7 +27,6 @@ gem 'rgeo'
 gem 'rgeo-activerecord'
 gem 'rswag-api'
 gem 'rswag-ui'
-gem 'shrine', '~> 3.6'
 gem 'sidekiq'
 gem 'sidekiq-cron'
 gem 'sidekiq-limit_fetch'

app/controllers/imports_controller.rb

@@ -29,20 +29,12 @@ class ImportsController < ApplicationController
         source: params[:import][:source]
       )
 
-      file = File.read(file)
+      import.file.attach(file)
 
-      raw_data =
-        case params[:import][:source]
-        when 'gpx' then Hash.from_xml(file)
-        when 'owntracks' then OwnTracks::RecParser.new(file).call
-        else JSON.parse(file)
-        end
-
-      import.update(raw_data:)
       import.id
     end
 
-    import_ids.each { ImportJob.perform_later(current_user.id, _1) }
+    import_ids.each { Import::ProcessJob.perform_later(_1) }
 
     redirect_to imports_url, notice: "#{files.size} files are queued to be imported in background", status: :see_other
   rescue StandardError => e

app/jobs/import/process_job.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class Import::ProcessJob < ApplicationJob
+  queue_as :imports
+
+  def perform(import_id)
+    import = Import.find(import_id)
+
+    import.process!
+  end
+end

app/jobs/import_job.rb

@@ -1,12 +0,0 @@
-# frozen_string_literal: true
-
-class ImportJob < ApplicationJob
-  queue_as :imports
-
-  def perform(user_id, import_id)
-    user = User.find(user_id)
-    import = user.imports.find(import_id)
-
-    import.process!
-  end
-end

app/models/import.rb

@@ -6,7 +6,9 @@ class Import < ApplicationRecord
 
   delegate :count, to: :points, prefix: true
 
-  include ImportUploader::Attachment(:raw)
+  has_one_attached :file
+
+  after_commit -> { Import::ProcessJob.perform_later(id) }, on: :create
 
   enum :source, {
     google_semantic_history: 0, owntracks: 1, google_records: 2,

app/models/user.rb

@@ -15,6 +15,8 @@ class User < ApplicationRecord
   has_many :places, through: :visits
   has_many :trips,  dependent: :destroy
 
+  has_many_attached :import_files
+
   after_create :create_api_key
   after_create :import_sample_points
   after_commit :activate, on: :create, if: -> { DawarichSettings.self_hosted? }
@@ -123,16 +125,15 @@ class User < ApplicationRecord
                   Rails.env.production? ||
                   (Rails.env.test? && ENV['IMPORT_SAMPLE_POINTS'])
 
-    raw_data = Hash.from_xml(
-      File.read(Rails.root.join('lib/assets/sample_points.gpx'))
-    )
-
     import = imports.create(
       name: 'DELETE_ME_this_is_a_demo_import_DELETE_ME',
-      source: 'gpx',
+      source: 'gpx'
-      raw_data:
     )
 
-    ImportJob.perform_later(id, import.id)
+    import.file.attach(
+      Rack::Test::UploadedFile.new(
+        Rails.root.join('lib/assets/sample_points.gpx'), 'application/xml'
+      )
+    )
   end
 end

app/services/gpx/track_importer.rb

@@ -3,22 +3,25 @@
 class Gpx::TrackImporter
   include Imports::Broadcaster
 
-  attr_reader :import, :json, :user_id
+  attr_reader :import, :user_id
 
   def initialize(import, user_id)
     @import = import
-    @json = import.raw_data
     @user_id = user_id
   end
 
   def call
-    tracks = json['gpx']['trk']
+    import.file.open do |file|
-    tracks_arr = tracks.is_a?(Array) ? tracks : [tracks]
+      json = Hash.from_xml(file)
 
-    points = tracks_arr.map { parse_track(_1) }.flatten.compact
+      tracks = json['gpx']['trk']
-    points_data = points.map.with_index(1) { |point, index| prepare_point(point, index) }.compact
+      tracks_arr = tracks.is_a?(Array) ? tracks : [tracks]
 
-    bulk_insert_points(points_data)
+      points = tracks_arr.map { parse_track(_1) }.flatten.compact
+      points_data = points.map { prepare_point(_1) }.compact
+
+      bulk_insert_points(points_data)
+    end
   end
 
   private
@@ -32,7 +35,7 @@ class Gpx::TrackImporter
     segments_array.compact.map { |segment| segment['trkpt'] }
   end
 
-  def prepare_point(point, index)
+  def prepare_point(point)
     return if point['lat'].blank? || point['lon'].blank? || point['time'].blank?
 
     {

app/services/immich/import_geodata.rb

@@ -23,7 +23,7 @@ class Immich::ImportGeodata
     import.raw_data = immich_data_json
     import.save!
 
-    ImportJob.perform_later(user.id, import.id)
+    Import::ProcessJob.perform_later(import.id)
   end
 
   private

app/services/imports/watcher.rb

@@ -51,7 +51,7 @@ class Imports::Watcher
 
     import.save!
 
-    ImportJob.perform_later(user.id, import.id)
+    Import::ProcessJob.perform_later(import.id)
   end
 
   def find_or_initialize_import(user, file_name)

app/services/photoprism/import_geodata.rb

@@ -24,7 +24,7 @@ class Photoprism::ImportGeodata
     return create_import_failed_notification(import.name) unless import.new_record?
 
     import.update!(raw_data: json_data)
-    ImportJob.perform_later(user.id, import.id)
+    Import::ProcessJob.perform_later(import.id)
   end
 
   def find_or_create_import(json_data)

app/uploaders/import_uploader.rb

@@ -1,5 +0,0 @@
-# frozen_string_literal: true
-
-class ImportUploader < Shrine
-  # plugins and uploading logic
-end

config/shrine.rb

@@ -1,13 +0,0 @@
-# frozen_string_literal: true
-
-require 'shrine'
-require 'shrine/storage/file_system'
-
-Shrine.storages = {
-  cache: Shrine::Storage::FileSystem.new('public', prefix: 'uploads/cache'), # temporary
-  store: Shrine::Storage::FileSystem.new('public', prefix: 'uploads')        # permanent
-}
-
-Shrine.plugin :activerecord           # loads Active Record integration
-Shrine.plugin :cached_attachment_data # enables retaining cached file across form redisplays
-Shrine.plugin :restore_cached_data    # extracts metadata for assigned cached files

db/migrate/20240324161309_create_active_storage_tables.active_storage.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 # This migration comes from active_storage (originally 20170806125915)
 class CreateActiveStorageTables < ActiveRecord::Migration[7.0]
   def change
@@ -19,7 +21,7 @@ class CreateActiveStorageTables < ActiveRecord::Migration[7.0]
         t.datetime :created_at, null: false
       end
 
-      t.index [ :key ], unique: true
+      t.index [:key], unique: true
     end
 
     create_table :active_storage_attachments, id: primary_key_type do |t|
@@ -33,7 +35,8 @@ class CreateActiveStorageTables < ActiveRecord::Migration[7.0]
         t.datetime :created_at, null: false
       end
 
-      t.index [ :record_type, :record_id, :name, :blob_id ], name: :index_active_storage_attachments_uniqueness, unique: true
+      t.index %i[record_type record_id name blob_id], name: :index_active_storage_attachments_uniqueness,
+unique: true
       t.foreign_key :active_storage_blobs, column: :blob_id
     end
 
@@ -41,17 +44,18 @@ class CreateActiveStorageTables < ActiveRecord::Migration[7.0]
       t.belongs_to :blob, null: false, index: false, type: foreign_key_type
       t.string :variation_digest, null: false
 
-      t.index [ :blob_id, :variation_digest ], name: :index_active_storage_variant_records_uniqueness, unique: true
+      t.index %i[blob_id variation_digest], name: :index_active_storage_variant_records_uniqueness, unique: true
       t.foreign_key :active_storage_blobs, column: :blob_id
     end
   end
 
   private
-    def primary_and_foreign_key_types
+
-      config = Rails.configuration.generators
+  def primary_and_foreign_key_types
-      setting = config.options[config.orm][:primary_key_type]
+    config = Rails.configuration.generators
-      primary_key_type = setting || :primary_key
+    setting = config.options[config.orm][:primary_key_type]
-      foreign_key_type = setting || :bigint
+    primary_key_type = setting || :primary_key
-      [primary_key_type, foreign_key_type]
+    foreign_key_type = setting || :bigint
-    end
+    [primary_key_type, foreign_key_type]
+  end
 end

spec/jobs/import/process_job_spec.rb

@@ -2,9 +2,9 @@
 
 require 'rails_helper'
 
-RSpec.describe ImportJob, type: :job do
+RSpec.describe Import::ProcessJob, type: :job do
   describe '#perform' do
-    subject(:perform) { described_class.new.perform(user.id, import.id) }
+    subject(:perform) { described_class.new.perform(import.id) }
 
     let(:user) { create(:user) }
     let!(:import) { create(:import, user:, name: 'owntracks_export.json') }

spec/models/user_spec.rb

@@ -72,7 +72,7 @@ RSpec.describe User, type: :model do
         expect(user.imports.first.name).to eq('DELETE_ME_this_is_a_demo_import_DELETE_ME')
         expect(user.imports.first.source).to eq('gpx')
 
-        expect(ImportJob).to have_been_enqueued.with(user.id, user.imports.first.id)
+        expect(Import::ProcessJob).to have_been_enqueued.with(user.imports.first.id)
       end
     end
   end

spec/requests/imports_spec.rb

@@ -46,7 +46,7 @@ RSpec.describe 'Imports', type: :request do
         it 'queues import job' do
           expect do
             post imports_path, params: { import: { source: 'owntracks', files: [file] } }
-          end.to have_enqueued_job(ImportJob).on_queue('imports').at_least(1).times
+          end.to have_enqueued_job(Import::ProcessJob).on_queue('imports').at_least(1).times
         end
 
         it 'creates a new import' do
@@ -64,7 +64,7 @@ RSpec.describe 'Imports', type: :request do
         it 'queues import job' do
           expect do
             post imports_path, params: { import: { source: 'gpx', files: [file] } }
-          end.to have_enqueued_job(ImportJob).on_queue('imports').at_least(1).times
+          end.to have_enqueued_job(Import::ProcessJob).on_queue('imports').at_least(1).times
         end
 
         it 'creates a new import' do

spec/services/gpx/track_importer_spec.rb

@@ -8,8 +8,12 @@ RSpec.describe Gpx::TrackImporter do
 
     let(:user) { create(:user) }
     let(:file_path) { Rails.root.join('spec/fixtures/files/gpx/gpx_track_single_segment.gpx') }
-    let(:raw_data) { Hash.from_xml(File.read(file_path)) }
+    let(:file) { Rack::Test::UploadedFile.new(file_path, 'application/xml') }
-    let(:import) { create(:import, user:, name: 'gpx_track.gpx', raw_data:) }
+    let(:import) { create(:import, user:, name: 'gpx_track.gpx', source: 'gpx') }
+
+    before do
+      import.file.attach(file)
+    end
 
     context 'when file has a single segment' do
       it 'creates points' do

spec/services/immich/import_geodata_spec.rb

@@ -88,8 +88,8 @@ RSpec.describe Immich::ImportGeodata do
       expect { service }.to change { Import.count }.by(1)
     end
 
-    it 'enqueues ImportJob' do
+    it 'enqueues Import::ProcessJob' do
-      expect(ImportJob).to receive(:perform_later)
+      expect(Import::ProcessJob).to receive(:perform_later)
 
       service
     end
@@ -101,8 +101,8 @@ RSpec.describe Immich::ImportGeodata do
         expect { service }.not_to(change { Import.count })
       end
 
-      it 'does not enqueue ImportJob' do
+      it 'does not enqueue Import::ProcessJob' do
-        expect(ImportJob).to_not receive(:perform_later)
+        expect(Import::ProcessJob).to_not receive(:perform_later)
 
         service
       end

spec/services/imports/watcher_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe Imports::Watcher do
       end
 
       it 'enqueues importing jobs for the user' do
-        expect { service }.to have_enqueued_job(ImportJob).exactly(6).times
+        expect { service }.to have_enqueued_job(Import::ProcessJob).exactly(6).times
       end
 
       context 'when the import already exists' do
@@ -41,8 +41,8 @@ RSpec.describe Imports::Watcher do
     end
 
     context 'when user does not exist' do
-      it 'does not call ImportJob' do
+      it 'does not call Import::ProcessJob' do
-        expect(ImportJob).not_to receive(:perform_later)
+        expect(Import::ProcessJob).not_to receive(:perform_later)
 
         service
       end

spec/services/photoprism/import_geodata_spec.rb

@@ -154,8 +154,8 @@ RSpec.describe Photoprism::ImportGeodata do
       expect { service }.to change { Import.count }.by(1)
     end
 
-    it 'enqueues ImportJob' do
+    it 'enqueues Import::ProcessJob' do
-      expect(ImportJob).to receive(:perform_later)
+      expect(Import::ProcessJob).to receive(:perform_later)
 
       service
     end
@@ -167,8 +167,8 @@ RSpec.describe Photoprism::ImportGeodata do
         expect { service }.not_to(change { Import.count })
       end
 
-      it 'does not enqueue ImportJob' do
+      it 'does not enqueue Import::ProcessJob' do
-        expect(ImportJob).to_not receive(:perform_later)
+        expect(Import::ProcessJob).to_not receive(:perform_later)
 
         service
       end