Set defaul value for file_store

The file.object_store is set during `uploader.store!`
which happens after object is inserted/updated. This
tries to mimic the default values in ObjectStorage
concern.
parent 87f9da1c
...@@ -73,12 +73,14 @@ module Ci ...@@ -73,12 +73,14 @@ module Ci
validates :file_format, presence: true, unless: :trace?, on: :create validates :file_format, presence: true, unless: :trace?, on: :create
validate :valid_file_format?, unless: :trace?, on: :create validate :valid_file_format?, unless: :trace?, on: :create
before_save :set_size, if: :file_changed?
update_project_statistics project_statistics_name: :build_artifacts_size before_save :set_size, if: :file_changed?
before_save :set_file_store, if: ->(job_artifact) { job_artifact.file_store.nil? }
after_save :update_file_store, if: :saved_change_to_file? after_save :update_file_store, if: :saved_change_to_file?
update_project_statistics project_statistics_name: :build_artifacts_size
scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) } scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) } scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) }
scope :for_sha, ->(sha, project_id) { joins(job: :pipeline).where(ci_pipelines: { sha: sha, project_id: project_id }) } scope :for_sha, ->(sha, project_id) { joins(job: :pipeline).where(ci_pipelines: { sha: sha, project_id: project_id }) }
...@@ -226,6 +228,15 @@ module Ci ...@@ -226,6 +228,15 @@ module Ci
self.size = file.size self.size = file.size
end end
def set_file_store
self.file_store =
if JobArtifactUploader.object_store_enabled? && JobArtifactUploader.direct_upload_enabled?
JobArtifactUploader::Store::REMOTE
else
file.object_store
end
end
def project_destroyed? def project_destroyed?
# Use job.project to avoid extra DB query for project # Use job.project to avoid extra DB query for project
job.project.pending_delete? job.project.pending_delete?
......
...@@ -17,6 +17,8 @@ class LfsObject < ApplicationRecord ...@@ -17,6 +17,8 @@ class LfsObject < ApplicationRecord
mount_uploader :file, LfsObjectUploader mount_uploader :file, LfsObjectUploader
before_save :set_file_store, if: ->(lfs_object) { lfs_object.file_store.nil? }
after_save :update_file_store, if: :saved_change_to_file? after_save :update_file_store, if: :saved_change_to_file?
def self.not_linked_to_project(project) def self.not_linked_to_project(project)
...@@ -55,6 +57,17 @@ class LfsObject < ApplicationRecord ...@@ -55,6 +57,17 @@ class LfsObject < ApplicationRecord
def self.calculate_oid(path) def self.calculate_oid(path)
self.hexdigest(path) self.hexdigest(path)
end end
private
def set_file_store
self.file_store =
if LfsObjectUploader.object_store_enabled? && LfsObjectUploader.direct_upload_enabled?
LfsObjectUploader::Store::REMOTE
else
file.object_store
end
end
end end
LfsObject.prepend_if_ee('EE::LfsObject') LfsObject.prepend_if_ee('EE::LfsObject')
...@@ -56,10 +56,31 @@ module RecordsUploads ...@@ -56,10 +56,31 @@ module RecordsUploads
size: file.size, size: file.size,
path: upload_path, path: upload_path,
model: model, model: model,
mount_point: mounted_as mount_point: mounted_as,
store: initial_store
) )
end end
def initial_store
if immediately_remote_stored?
::ObjectStorage::Store::REMOTE
else
::ObjectStorage::Store::LOCAL
end
end
def immediately_remote_stored?
object_storage_available? && direct_upload_enabled?
end
def object_storage_available?
self.class.ancestors.include?(ObjectStorage::Concern)
end
def direct_upload_enabled?
self.class.object_store_enabled? && self.class.direct_upload_enabled?
end
# Before removing an attachment, destroy any Upload records at the same path # Before removing an attachment, destroy any Upload records at the same path
# #
# Called `before :remove` # Called `before :remove`
......
...@@ -51,6 +51,7 @@ describe Security::StoreScansService do ...@@ -51,6 +51,7 @@ describe Security::StoreScansService do
before do before do
create(:ee_ci_job_artifact, :dast_with_missing_file, job: build) create(:ee_ci_job_artifact, :dast_with_missing_file, job: build)
end end
it 'stores 0 scanned resources on the scan' do it 'stores 0 scanned resources on the scan' do
subject subject
......
...@@ -13,7 +13,7 @@ FactoryBot.define do ...@@ -13,7 +13,7 @@ FactoryBot.define do
end end
trait :remote_store do trait :remote_store do
file_store { JobArtifactUploader::Store::REMOTE} file_store { JobArtifactUploader::Store::REMOTE }
end end
after :build do |artifact| after :build do |artifact|
......
...@@ -349,16 +349,13 @@ describe Ci::JobArtifact do ...@@ -349,16 +349,13 @@ describe Ci::JobArtifact do
end end
describe 'file is being stored' do describe 'file is being stored' do
subject { create(:ci_job_artifact, :archive) }
context 'when object has nil store' do context 'when object has nil store' do
before do
subject.update_column(:file_store, nil)
subject.reload
end
it 'is stored locally' do it 'is stored locally' do
expect(subject.file_store).to be(nil) subject = build(:ci_job_artifact, :archive, file_store: nil)
subject.save
expect(subject.file_store).to be(ObjectStorage::Store::LOCAL)
expect(subject.file).to be_file_storage expect(subject.file).to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL) expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL)
end end
...@@ -366,6 +363,10 @@ describe Ci::JobArtifact do ...@@ -366,6 +363,10 @@ describe Ci::JobArtifact do
context 'when existing object has local store' do context 'when existing object has local store' do
it 'is stored locally' do it 'is stored locally' do
subject = build(:ci_job_artifact, :archive)
subject.save
expect(subject.file_store).to be(ObjectStorage::Store::LOCAL) expect(subject.file_store).to be(ObjectStorage::Store::LOCAL)
expect(subject.file).to be_file_storage expect(subject.file).to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL) expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL)
...@@ -379,6 +380,10 @@ describe Ci::JobArtifact do ...@@ -379,6 +380,10 @@ describe Ci::JobArtifact do
context 'when file is stored' do context 'when file is stored' do
it 'is stored remotely' do it 'is stored remotely' do
subject = build(:ci_job_artifact, :archive)
subject.save
expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE) expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE)
expect(subject.file).not_to be_file_storage expect(subject.file).not_to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::REMOTE) expect(subject.file.object_store).to eq(ObjectStorage::Store::REMOTE)
......
...@@ -78,7 +78,8 @@ describe RecordsUploads do ...@@ -78,7 +78,8 @@ describe RecordsUploads do
path: File.join('uploads', 'rails_sample.jpg'), path: File.join('uploads', 'rails_sample.jpg'),
size: 512.kilobytes, size: 512.kilobytes,
model: build_stubbed(:user), model: build_stubbed(:user),
uploader: uploader.class.to_s uploader: uploader.class.to_s,
store: ::ObjectStorage::Store::LOCAL
) )
uploader.upload = existing uploader.upload = existing
...@@ -98,7 +99,8 @@ describe RecordsUploads do ...@@ -98,7 +99,8 @@ describe RecordsUploads do
path: File.join('uploads', 'rails_sample.jpg'), path: File.join('uploads', 'rails_sample.jpg'),
size: 512.kilobytes, size: 512.kilobytes,
model: project, model: project,
uploader: uploader.class.to_s uploader: uploader.class.to_s,
store: ::ObjectStorage::Store::LOCAL
) )
uploader.store!(upload_fixture('rails_sample.jpg')) uploader.store!(upload_fixture('rails_sample.jpg'))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment