From 2508ede65cec4b4b3efb80cf345aaa7ff9f7ace4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 19 Feb 2024 09:51:46 +0100 Subject: [PATCH] Fix prepared upload specs --- app/models/attachment.rb | 22 +++++++++++-------- .../attachments/finish_direct_upload_job.rb | 2 +- spec/factories/attachment_factory.rb | 3 +-- spec/models/attachment_spec.rb | 4 +++- spec/requests/api/v3/attachments_spec.rb | 6 ++--- ...prepare_upload_service_integration_spec.rb | 5 ++--- ...anup_uncontainered_job_integration_spec.rb | 6 ++--- ...nish_direct_upload_job_integration_spec.rb | 3 ++- 8 files changed, 28 insertions(+), 23 deletions(-) diff --git a/app/models/attachment.rb b/app/models/attachment.rb index da8a646dc77..2dbce28730e 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -72,8 +72,8 @@ class Attachment < ApplicationRecord after_commit :enqueue_jobs, on: :create, if: -> { !internal_container? } - scope :pending_direct_upload, -> { where(digest: "", downloads: -1) } - scope :not_pending_direct_upload, -> { where.not(digest: "", downloads: -1) } + scope :pending_direct_upload, -> { status_prepared } + scope :not_pending_direct_upload, -> { not_status_prepared } ## # Returns an URL if the attachment is stored in an external (fog) attachment storage @@ -248,15 +248,19 @@ class Attachment < ApplicationRecord end def enqueue_jobs - if OpenProject::Database.allows_tsv? && (!container || container.class.attachment_tsv_extracted?) - Attachments::ExtractFulltextJob.perform_later(id) - end + extract_fulltext if pending_virus_scan? Attachments::VirusScanJob.perform_later(self) end end + def extract_fulltext + if OpenProject::Database.allows_tsv? && (!container || container.class.attachment_tsv_extracted?) + Attachments::ExtractFulltextJob.perform_later(id) + end + end + # Extract the fulltext of any attachments where fulltext is still nil. # This runs inline and not in an asynchronous worker. def self.extract_fulltext_where_missing(run_now: true) @@ -315,6 +319,10 @@ class Attachment < ApplicationRecord digest == "" && downloads == -1 end + def internal_container? + container&.is_a?(Export) + end + private def filesize_below_allowed_maximum @@ -323,10 +331,6 @@ class Attachment < ApplicationRecord end end - def internal_container? - container&.is_a?(Export) - end - def container_changed_more_than_once if container_id_changed_more_than_once? || container_type_changed_more_than_once? errors.add(:container, :unchangeable) diff --git a/app/workers/attachments/finish_direct_upload_job.rb b/app/workers/attachments/finish_direct_upload_job.rb index 4454abc6a0c..b99ed1ca0e1 100644 --- a/app/workers/attachments/finish_direct_upload_job.rb +++ b/app/workers/attachments/finish_direct_upload_job.rb @@ -91,7 +91,7 @@ class Attachments::FinishDirectUploadJob < ApplicationJob options: end - def schedule_jobs + def schedule_jobs(attachment) attachment.extract_fulltext end diff --git a/spec/factories/attachment_factory.rb b/spec/factories/attachment_factory.rb index d4c89749aa2..885c49cd394 100644 --- a/spec/factories/attachment_factory.rb +++ b/spec/factories/attachment_factory.rb @@ -58,8 +58,7 @@ FactoryBot.define do end factory :pending_direct_upload do - digest { "" } - downloads { -1 } + status { 'prepared' } created_at { DateTime.now - 2.weeks } end end diff --git a/spec/models/attachment_spec.rb b/spec/models/attachment_spec.rb index 5f5b9ccbcce..cc0017451f7 100644 --- a/spec/models/attachment_spec.rb +++ b/spec/models/attachment_spec.rb @@ -323,7 +323,9 @@ RSpec.describe Attachment do end end - context 'with setting enabled', with_settings: { antivirus_scan_mode: :clamav_socket } do + context 'with setting enabled', + with_ee: %i[virus_scanning], + with_settings: { antivirus_scan_mode: :clamav_socket } do it 'runs the job' do attachment.save expect(attachment.pending_virus_scan?).to be true diff --git a/spec/requests/api/v3/attachments_spec.rb b/spec/requests/api/v3/attachments_spec.rb index b05203db31f..d84b9e539c0 100644 --- a/spec/requests/api/v3/attachments_spec.rb +++ b/spec/requests/api/v3/attachments_spec.rb @@ -81,9 +81,9 @@ RSpec.describe API::V3::Attachments::AttachmentsAPI do let(:container_href) { nil } describe 'GET /uploaded' do - let(:digest) { "" } + let(:status) { :prepared } let(:attachment) do - create(:attachment, digest:, author: current_user, container: nil, container_type: nil, downloads: -1) + create(:attachment, status:, author: current_user, container: nil, container_type: nil) end before do @@ -91,7 +91,7 @@ RSpec.describe API::V3::Attachments::AttachmentsAPI do end context 'with no pending attachments' do - let(:digest) { "0xFF" } + let(:status) { :uploaded } it 'returns 404' do expect(last_response.status).to eq 404 diff --git a/spec/services/attachments/prepare_upload_service_integration_spec.rb b/spec/services/attachments/prepare_upload_service_integration_spec.rb index a5db9a919d4..9af9f848721 100644 --- a/spec/services/attachments/prepare_upload_service_integration_spec.rb +++ b/spec/services/attachments/prepare_upload_service_integration_spec.rb @@ -79,9 +79,8 @@ RSpec.describe Attachments::PrepareUploadService, .to eql "" end - it 'sets the download count to -1' do - expect(attachment.downloads) - .to be -1 + it 'sets the status to prepared' do + expect(attachment.status).to eq 'prepared' end context 'with a special character in the filename' do diff --git a/spec/workers/attachments/cleanup_uncontainered_job_integration_spec.rb b/spec/workers/attachments/cleanup_uncontainered_job_integration_spec.rb index bc94bf8a6e4..4b38aa370e1 100644 --- a/spec/workers/attachments/cleanup_uncontainered_job_integration_spec.rb +++ b/spec/workers/attachments/cleanup_uncontainered_job_integration_spec.rb @@ -40,13 +40,13 @@ RSpec.describe Attachments::CleanupUncontaineredJob, type: :job do end let!(:finished_upload) do - create(:attachment, created_at: Time.now - grace_period.minutes, digest: "0x42") + create(:attachment, created_at: Time.now - grace_period.minutes, status: :uploaded) end let!(:old_pending_upload) do - create(:attachment, created_at: Time.now - grace_period.minutes, digest: "", downloads: -1) + create(:attachment, created_at: Time.now - grace_period.minutes, status: :prepared) end let!(:new_pending_upload) do - create(:attachment, created_at: Time.now - (grace_period - 1).minutes, digest: "", downloads: -1) + create(:attachment, created_at: Time.now - (grace_period - 1).minutes, status: :prepared) end let(:job) { described_class.new } diff --git a/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb b/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb index 2e13ef8326f..d647aeb3e0a 100644 --- a/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb +++ b/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb @@ -34,7 +34,7 @@ RSpec.describe Attachments::FinishDirectUploadJob, 'integration', type: :job do let!(:pending_attachment) do create(:attachment, author: user, - downloads: -1, + status: :prepared, digest: '', container:) end @@ -47,6 +47,7 @@ RSpec.describe Attachments::FinishDirectUploadJob, 'integration', type: :job do attachment = Attachment.find(pending_attachment.id) + expect(attachment.status).to eq 'uploaded' expect(attachment.downloads) .to be(0) # expect to replace the content type with the actual value