Commit 68dc4110 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'sh-refactor-file-store-mounter' into 'master'

Move file store updates and mount_uploader into a concern

Closes #232094

See merge request gitlab-org/gitlab!37907
parents 88ed3d28 125b670a
......@@ -9,6 +9,7 @@ module Ci
include Sortable
include IgnorableColumns
include Artifactable
include FileStoreMounter
extend Gitlab::Ci::Model
NotSupportedAdapterError = Class.new(StandardError)
......@@ -115,7 +116,7 @@ module Ci
belongs_to :project
belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id
mount_uploader :file, JobArtifactUploader
mount_file_store_uploader JobArtifactUploader
validates :file_format, presence: true, unless: :trace?, on: :create
validate :validate_supported_file_format!, on: :create
......@@ -124,8 +125,6 @@ module Ci
update_project_statistics project_statistics_name: :build_artifacts_size
after_save :update_file_store, if: :saved_change_to_file?
scope :not_expired, -> { where('expire_at IS NULL OR expire_at > ?', Time.current) }
scope :with_files_stored_locally, -> { where(file_store: ::JobArtifactUploader::Store::LOCAL) }
scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) }
......@@ -229,12 +228,6 @@ module Ci
end
end
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
self.update_column(:file_store, file.object_store)
end
def self.associated_file_types_for(file_type)
return unless file_types.include?(file_type)
......
# frozen_string_literal: true
module FileStoreMounter
extend ActiveSupport::Concern
class_methods do
def mount_file_store_uploader(uploader)
mount_uploader(:file, uploader)
after_save :update_file_store, if: :saved_change_to_file?
end
end
private
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
self.update_column(:file_store, file.object_store)
end
end
......@@ -5,6 +5,7 @@ class LfsObject < ApplicationRecord
include Checksummable
include EachBatch
include ObjectStorage::BackgroundMove
include FileStoreMounter
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, -> { distinct }, through: :lfs_objects_projects
......@@ -15,21 +16,13 @@ class LfsObject < ApplicationRecord
validates :oid, presence: true, uniqueness: true
mount_uploader :file, LfsObjectUploader
after_save :update_file_store, if: :saved_change_to_file?
mount_file_store_uploader LfsObjectUploader
def self.not_linked_to_project(project)
where('NOT EXISTS (?)',
project.lfs_objects_projects.select(1).where('lfs_objects_projects.lfs_object_id = lfs_objects.id'))
end
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
self.update_column(:file_store, file.object_store)
end
def project_allowed_access?(project)
if project.fork_network_member
lfs_objects_projects
......
......@@ -3,6 +3,7 @@
module Terraform
class State < ApplicationRecord
include UsageStatistics
include FileStoreMounter
DEFAULT = '{"version":1}'.freeze
HEX_REGEXP = %r{\A\h+\z}.freeze
......@@ -17,18 +18,10 @@ module Terraform
default_value_for(:uuid, allows_nil: false) { SecureRandom.hex(UUID_LENGTH / 2) }
after_save :update_file_store, if: :saved_change_to_file?
mount_uploader :file, StateUploader
mount_file_store_uploader StateUploader
default_value_for(:file) { CarrierWaveStringFile.new(DEFAULT) }
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
self.update_column(:file_store, file.object_store)
end
def file_store
super || StateUploader.default_store
end
......
---
title: Move file store updates and mount_uploader into a concern
merge_request: 37907
author:
type: other
# frozen_string_literal: true
class DependencyProxy::Blob < ApplicationRecord
include FileStoreMounter
belongs_to :group
validates :group, presence: true
validates :file, presence: true
validates :file_name, presence: true
mount_uploader :file, DependencyProxy::FileUploader
after_save :update_file_store, if: :saved_change_to_file?
mount_file_store_uploader DependencyProxy::FileUploader
def self.total_size
sum(:size)
......@@ -18,10 +18,4 @@ class DependencyProxy::Blob < ApplicationRecord
def self.find_or_build(file_name)
find_or_initialize_by(file_name: file_name)
end
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
self.update_column(:file_store, file.object_store)
end
end
......@@ -2,15 +2,15 @@
module Vulnerabilities
class Export < ApplicationRecord
include FileStoreMounter
self.table_name = "vulnerability_exports"
belongs_to :project
belongs_to :group
belongs_to :author, optional: false, class_name: 'User'
mount_uploader :file, AttachmentUploader
after_save :update_file_store, if: :saved_change_to_file?
mount_file_store_uploader AttachmentUploader
enum format: {
csv: 0
......@@ -77,12 +77,6 @@ module Vulnerabilities
Upload.find_by(model: self, path: paths)
end
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
self.update_column(:file_store, file.object_store)
end
private
def make_project_level_export(project)
......
......@@ -41,11 +41,7 @@ RSpec.describe DependencyProxy::Blob, type: :model do
subject { create(:dependency_proxy_blob) }
context 'when existing object has local store' do
it 'is stored locally' do
expect(subject.file_store).to be(ObjectStorage::Store::LOCAL)
expect(subject.file).to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL)
end
it_behaves_like 'mounted file in local store'
end
context 'when direct upload is enabled' do
......@@ -53,13 +49,7 @@ RSpec.describe DependencyProxy::Blob, type: :model do
stub_dependency_proxy_object_storage(direct_upload: true)
end
context 'when file is stored' do
it 'is stored remotely' do
expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE)
expect(subject.file).not_to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::REMOTE)
end
end
it_behaves_like 'mounted file in object store'
end
end
end
......@@ -483,11 +483,7 @@ RSpec.describe Ci::JobArtifact do
subject { create(:ci_job_artifact, :archive) }
context 'when existing object has local store' do
it 'is stored locally' do
expect(subject.file_store).to be(ObjectStorage::Store::LOCAL)
expect(subject.file).to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL)
end
it_behaves_like 'mounted file in local store'
end
context 'when direct upload is enabled' do
......@@ -496,11 +492,7 @@ RSpec.describe Ci::JobArtifact do
end
context 'when file is stored' do
it 'is stored remotely' do
expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE)
expect(subject.file).not_to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::REMOTE)
end
it_behaves_like 'mounted file in object store'
end
end
end
......
......@@ -152,14 +152,10 @@ RSpec.describe LfsObject do
end
describe 'file is being stored' do
let(:lfs_object) { create(:lfs_object, :with_file) }
subject { create(:lfs_object, :with_file) }
context 'when existing object has local store' do
it 'is stored locally' do
expect(lfs_object.file_store).to be(ObjectStorage::Store::LOCAL)
expect(lfs_object.file).to be_file_storage
expect(lfs_object.file.object_store).to eq(ObjectStorage::Store::LOCAL)
end
it_behaves_like 'mounted file in local store'
end
context 'when direct upload is enabled' do
......@@ -167,13 +163,7 @@ RSpec.describe LfsObject do
stub_lfs_object_storage(direct_upload: true)
end
context 'when file is stored' do
it 'is stored remotely' do
expect(lfs_object.file_store).to eq(ObjectStorage::Store::REMOTE)
expect(lfs_object.file).not_to be_file_storage
expect(lfs_object.file.object_store).to eq(ObjectStorage::Store::REMOTE)
end
end
it_behaves_like 'mounted file in object store'
end
end
end
......
......@@ -45,9 +45,7 @@ RSpec.describe Terraform::State do
describe '#update_file_store' do
context 'when file is stored in object storage' do
it 'sets file_store to remote' do
expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE)
end
it_behaves_like 'mounted file in object store'
end
context 'when file is stored locally' do
......@@ -55,9 +53,7 @@ RSpec.describe Terraform::State do
stub_terraform_state_object_storage(Terraform::StateUploader, enabled: false)
end
it 'sets file_store to local' do
expect(subject.file_store).to eq(ObjectStorage::Store::LOCAL)
end
it_behaves_like 'mounted file in local store'
end
end
end
# frozen_string_literal: true
RSpec.shared_examples 'mounted file in local store' do
it 'is stored locally' do
expect(subject.file_store).to be(ObjectStorage::Store::LOCAL)
expect(subject.file).to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL)
end
end
RSpec.shared_examples 'mounted file in object store' do
it 'is stored remotely' do
expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE)
expect(subject.file).not_to be_file_storage
expect(subject.file.object_store).to eq(ObjectStorage::Store::REMOTE)
end
end
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