diff --git a/Gemfile b/Gemfile index 4ed5dad3..564515c4 100644 --- a/Gemfile +++ b/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' diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index a6359e67..3b363e6b 100644 --- a/app/controllers/imports_controller.rb +++ b/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 diff --git a/app/jobs/import/process_job.rb b/app/jobs/import/process_job.rb new file mode 100644 index 00000000..4529d5f7 --- /dev/null +++ b/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 diff --git a/app/jobs/import_job.rb b/app/jobs/import_job.rb deleted file mode 100644 index a07cfa46..00000000 --- a/app/jobs/import_job.rb +++ /dev/null @@ -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 diff --git a/app/models/import.rb b/app/models/import.rb index 045e8b5f..89a2387f 100644 --- a/app/models/import.rb +++ b/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, diff --git a/app/models/user.rb b/app/models/user.rb index ee4d84f8..3a1c6071 100644 --- a/app/models/user.rb +++ b/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', - raw_data: + source: 'gpx' ) - 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 diff --git a/app/services/gpx/track_importer.rb b/app/services/gpx/track_importer.rb index 62f327cc..ee2c7338 100644 --- a/app/services/gpx/track_importer.rb +++ b/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'] - tracks_arr = tracks.is_a?(Array) ? tracks : [tracks] + import.file.open do |file| + json = Hash.from_xml(file) - points = tracks_arr.map { parse_track(_1) }.flatten.compact - points_data = points.map.with_index(1) { |point, index| prepare_point(point, index) }.compact + tracks = json['gpx']['trk'] + 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? { diff --git a/app/services/immich/import_geodata.rb b/app/services/immich/import_geodata.rb index 469761d6..63a9232b 100644 --- a/app/services/immich/import_geodata.rb +++ b/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 diff --git a/app/services/imports/watcher.rb b/app/services/imports/watcher.rb index de9ca262..f29cacb8 100644 --- a/app/services/imports/watcher.rb +++ b/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) diff --git a/app/services/photoprism/import_geodata.rb b/app/services/photoprism/import_geodata.rb index 182681e6..2ad5fa6e 100644 --- a/app/services/photoprism/import_geodata.rb +++ b/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) diff --git a/app/uploaders/import_uploader.rb b/app/uploaders/import_uploader.rb deleted file mode 100644 index b4513d77..00000000 --- a/app/uploaders/import_uploader.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -class ImportUploader < Shrine - # plugins and uploading logic -end diff --git a/config/shrine.rb b/config/shrine.rb deleted file mode 100644 index 42fb0ef9..00000000 --- a/config/shrine.rb +++ /dev/null @@ -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 diff --git a/db/migrate/20240324161309_create_active_storage_tables.active_storage.rb b/db/migrate/20240324161309_create_active_storage_tables.active_storage.rb index e4706aa2..3b750c43 100644 --- a/db/migrate/20240324161309_create_active_storage_tables.active_storage.rb +++ b/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 - setting = config.options[config.orm][:primary_key_type] - primary_key_type = setting || :primary_key - foreign_key_type = setting || :bigint - [primary_key_type, foreign_key_type] - end + + def primary_and_foreign_key_types + config = Rails.configuration.generators + setting = config.options[config.orm][:primary_key_type] + primary_key_type = setting || :primary_key + foreign_key_type = setting || :bigint + [primary_key_type, foreign_key_type] + end end diff --git a/spec/jobs/import_job_spec.rb b/spec/jobs/import/process_job_spec.rb similarity index 89% rename from spec/jobs/import_job_spec.rb rename to spec/jobs/import/process_job_spec.rb index b6655a0c..d805ac9f 100644 --- a/spec/jobs/import_job_spec.rb +++ b/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') } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6fecebfd..3e808a4d 100644 --- a/spec/models/user_spec.rb +++ b/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 diff --git a/spec/requests/imports_spec.rb b/spec/requests/imports_spec.rb index 57074735..d9703a63 100644 --- a/spec/requests/imports_spec.rb +++ b/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 diff --git a/spec/services/gpx/track_importer_spec.rb b/spec/services/gpx/track_importer_spec.rb index bef491c9..5aeb7117 100644 --- a/spec/services/gpx/track_importer_spec.rb +++ b/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(:import) { create(:import, user:, name: 'gpx_track.gpx', raw_data:) } + let(:file) { Rack::Test::UploadedFile.new(file_path, 'application/xml') } + 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 diff --git a/spec/services/immich/import_geodata_spec.rb b/spec/services/immich/import_geodata_spec.rb index b5460526..03147d6b 100644 --- a/spec/services/immich/import_geodata_spec.rb +++ b/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 - expect(ImportJob).to receive(:perform_later) + it 'enqueues Import::ProcessJob' do + 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 - expect(ImportJob).to_not receive(:perform_later) + it 'does not enqueue Import::ProcessJob' do + expect(Import::ProcessJob).to_not receive(:perform_later) service end diff --git a/spec/services/imports/watcher_spec.rb b/spec/services/imports/watcher_spec.rb index c155d23c..ac3041c8 100644 --- a/spec/services/imports/watcher_spec.rb +++ b/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 - expect(ImportJob).not_to receive(:perform_later) + it 'does not call Import::ProcessJob' do + expect(Import::ProcessJob).not_to receive(:perform_later) service end diff --git a/spec/services/photoprism/import_geodata_spec.rb b/spec/services/photoprism/import_geodata_spec.rb index 341348fc..c0c4190b 100644 --- a/spec/services/photoprism/import_geodata_spec.rb +++ b/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 - expect(ImportJob).to receive(:perform_later) + it 'enqueues Import::ProcessJob' do + 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 - expect(ImportJob).to_not receive(:perform_later) + it 'does not enqueue Import::ProcessJob' do + expect(Import::ProcessJob).to_not receive(:perform_later) service end