Commit a8a30d0f authored by Luke Duncalfe's avatar Luke Duncalfe
Browse files

CE backport for changes in EE MR 13894

This backports to CE changes that allow the recording of the
repository_type in the table lfs_objects_projects.

This is in order to allow future pruning of unreferenced LFS objects,
as we will need to know which repository to look in for the LFS pointer
file.

The EE MR that contains the original code and a full explanation of the
changes is
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/13894

EE Issue https://gitlab.com/gitlab-org/gitlab-ee/issues/9490

Note that there was a lot of CE code changed in the EE MR because we
want to allow the wiki repository to also use LFS. See
https://gitlab.com/gitlab-org/gitlab-ce/issues/43721. As the wiki is
an unlicensed feature, a full backport is required to enable this.
parent 0c355fde2be6
...@@ -5,7 +5,7 @@ class LfsObject < ApplicationRecord ...@@ -5,7 +5,7 @@ class LfsObject < ApplicationRecord
include ObjectStorage::BackgroundMove include ObjectStorage::BackgroundMove
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :lfs_objects_projects has_many :projects, -> { distinct }, through: :lfs_objects_projects
scope :with_files_stored_locally, -> { where(file_store: LfsObjectUploader::Store::LOCAL) } scope :with_files_stored_locally, -> { where(file_store: LfsObjectUploader::Store::LOCAL) }
......
...@@ -5,11 +5,17 @@ class LfsObjectsProject < ApplicationRecord ...@@ -5,11 +5,17 @@ class LfsObjectsProject < ApplicationRecord
belongs_to :lfs_object belongs_to :lfs_object
validates :lfs_object_id, presence: true validates :lfs_object_id, presence: true
validates :lfs_object_id, uniqueness: { scope: [:project_id], message: "already exists in project" } validates :lfs_object_id, uniqueness: { scope: [:project_id, :repository_type], message: "already exists in repository" }
validates :project_id, presence: true validates :project_id, presence: true
after_commit :update_project_statistics, on: [:create, :destroy] after_commit :update_project_statistics, on: [:create, :destroy]
enum repository_type: {
project: 0,
wiki: 1,
design: 2 ## EE-specific
}
private private
def update_project_statistics def update_project_statistics
......
...@@ -223,7 +223,7 @@ class Project < ApplicationRecord ...@@ -223,7 +223,7 @@ class Project < ApplicationRecord
has_many :starrers, through: :users_star_projects, source: :user has_many :starrers, through: :users_star_projects, source: :user
has_many :releases has_many :releases
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :lfs_objects, through: :lfs_objects_projects has_many :lfs_objects, -> { distinct }, through: :lfs_objects_projects
has_many :lfs_file_locks has_many :lfs_file_locks
has_many :project_group_links has_many :project_group_links
has_many :invited_groups, through: :project_group_links, source: :group has_many :invited_groups, through: :project_group_links, source: :group
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Files module Files
class CreateService < Files::BaseService class CreateService < Files::BaseService
def create_commit! def create_commit!
transformer = Lfs::FileTransformer.new(project, @branch_name) transformer = Lfs::FileTransformer.new(project, repository, @branch_name)
result = transformer.new_file(@file_path, @file_content) result = transformer.new_file(@file_path, @file_content)
......
...@@ -5,7 +5,7 @@ class MultiService < Files::BaseService ...@@ -5,7 +5,7 @@ class MultiService < Files::BaseService
UPDATE_FILE_ACTIONS = %w(update move delete chmod).freeze UPDATE_FILE_ACTIONS = %w(update move delete chmod).freeze
def create_commit! def create_commit!
transformer = Lfs::FileTransformer.new(project, @branch_name) transformer = Lfs::FileTransformer.new(project, repository, @branch_name)
actions = actions_after_lfs_transformation(transformer, params[:actions]) actions = actions_after_lfs_transformation(transformer, params[:actions])
actions = transform_move_actions(actions) actions = transform_move_actions(actions)
......
...@@ -8,17 +8,17 @@ module Lfs ...@@ -8,17 +8,17 @@ module Lfs
# pointer returned. If the file isn't in LFS the untransformed content # pointer returned. If the file isn't in LFS the untransformed content
# is returned to save in the commit. # is returned to save in the commit.
# #
# transformer = Lfs::FileTransformer.new(project, @branch_name) # transformer = Lfs::FileTransformer.new(project, repository, @branch_name)
# content_or_lfs_pointer = transformer.new_file(file_path, content).content # content_or_lfs_pointer = transformer.new_file(file_path, content).content
# create_transformed_commit(content_or_lfs_pointer) # create_transformed_commit(content_or_lfs_pointer)
# #
class FileTransformer class FileTransformer
attr_reader :project, :branch_name attr_reader :project, :repository, :repository_type, :branch_name
delegate :repository, to: :project def initialize(project, repository, branch_name)
def initialize(project, branch_name)
@project = project @project = project
@repository = repository
@repository_type = repository.repo_type.name
@branch_name = branch_name @branch_name = branch_name
end end
...@@ -64,7 +64,11 @@ def create_lfs_object!(lfs_pointer_file, file_content) ...@@ -64,7 +64,11 @@ def create_lfs_object!(lfs_pointer_file, file_content)
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def link_lfs_object!(lfs_object) def link_lfs_object!(lfs_object)
project.lfs_objects << lfs_object LfsObjectsProject.safe_find_or_create_by!(
project: project,
lfs_object: lfs_object,
repository_type: repository_type
)
end end
def parse_file_content(file_content, encoding: nil) def parse_file_content(file_content, encoding: nil)
......
# frozen_string_literal: true
class AddRepositoryTypeToLfsObjectsProject < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :lfs_objects_projects, :repository_type, :integer, limit: 2, null: true
end
end
# frozen_string_literal: true
class AddLfsObjectIdIndexToLfsObjectsProjects < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_index :lfs_objects_projects, :lfs_object_id
end
def down
remove_concurrent_index :lfs_objects_projects, :lfs_object_id
end
end
...@@ -1190,6 +1190,8 @@ ...@@ -1190,6 +1190,8 @@
t.integer "project_id", null: false t.integer "project_id", null: false
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.integer "repository_type", limit: 2
t.index ["lfs_object_id"], name: "index_lfs_objects_projects_on_lfs_object_id", using: :btree
t.index ["project_id"], name: "index_lfs_objects_projects_on_project_id", using: :btree t.index ["project_id"], name: "index_lfs_objects_projects_on_project_id", using: :btree
end end
......
...@@ -2,5 +2,6 @@ ...@@ -2,5 +2,6 @@
factory :lfs_objects_project do factory :lfs_objects_project do
lfs_object lfs_object
project project
repository_type :project
end end
end end
...@@ -3,6 +3,20 @@ ...@@ -3,6 +3,20 @@
require 'spec_helper' require 'spec_helper'
describe LfsObject do describe LfsObject do
it 'has a distinct has_many :projects relation through lfs_objects_projects' do
lfs_object = create(:lfs_object)
project = create(:project)
[:project, :design].each do |repository_type|
create(:lfs_objects_project, project: project,
lfs_object: lfs_object,
repository_type: repository_type)
end
expect(lfs_object.lfs_objects_projects.size).to eq(2)
expect(lfs_object.projects.size).to eq(1)
expect(lfs_object.projects.to_a).to eql([project])
end
describe '#local_store?' do describe '#local_store?' do
it 'returns true when file_store is equal to LfsObjectUploader::Store::LOCAL' do it 'returns true when file_store is equal to LfsObjectUploader::Store::LOCAL' do
subject.file_store = LfsObjectUploader::Store::LOCAL subject.file_store = LfsObjectUploader::Store::LOCAL
......
...@@ -20,8 +20,8 @@ ...@@ -20,8 +20,8 @@
it 'validates object id' do it 'validates object id' do
is_expected.to validate_uniqueness_of(:lfs_object_id) is_expected.to validate_uniqueness_of(:lfs_object_id)
.scoped_to(:project_id) .scoped_to(:project_id, :repository_type)
.with_message("already exists in project") .with_message("already exists in repository")
end end
end end
......
...@@ -103,6 +103,20 @@ ...@@ -103,6 +103,20 @@
expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project)
end end
it 'has a distinct has_many :lfs_objects relation through lfs_objects_projects' do
project = create(:project)
lfs_object = create(:lfs_object)
[:project, :design].each do |repository_type|
create(:lfs_objects_project, project: project,
lfs_object: lfs_object,
repository_type: repository_type)
end
expect(project.lfs_objects_projects.size).to eq(2)
expect(project.lfs_objects.size).to eq(1)
expect(project.lfs_objects.to_a).to eql([lfs_object])
end
context 'after initialized' do context 'after initialized' do
it "has a project_feature" do it "has a project_feature" do
expect(described_class.new.project_feature).to be_present expect(described_class.new.project_feature).to be_present
......
...@@ -3,13 +3,13 @@ ...@@ -3,13 +3,13 @@
require "spec_helper" require "spec_helper"
describe Lfs::FileTransformer do describe Lfs::FileTransformer do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository, :wiki_repo) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:file_content) { 'Test file content' } let(:file_content) { 'Test file content' }
let(:branch_name) { 'lfs' } let(:branch_name) { 'lfs' }
let(:file_path) { 'test_file.lfs' } let(:file_path) { 'test_file.lfs' }
subject { described_class.new(project, branch_name) } subject { described_class.new(project, repository, branch_name) }
describe '#new_file' do describe '#new_file' do
context 'with lfs disabled' do context 'with lfs disabled' do
...@@ -100,6 +100,12 @@ ...@@ -100,6 +100,12 @@
end.to change { project.lfs_objects.count }.by(1) end.to change { project.lfs_objects.count }.by(1)
end end
it 'saves the repository_type to LfsObjectsProject' do
subject.new_file(file_path, file_content)
expect(project.lfs_objects_projects.first.repository_type).to eq('project')
end
context 'when LfsObject already exists' do context 'when LfsObject already exists' do
let(:lfs_pointer) { Gitlab::Git::LfsPointerFile.new(file_content) } let(:lfs_pointer) { Gitlab::Git::LfsPointerFile.new(file_content) }
...@@ -113,6 +119,56 @@ ...@@ -113,6 +119,56 @@
end.to change { project.lfs_objects.count }.by(1) end.to change { project.lfs_objects.count }.by(1)
end end
end end
context 'when the LfsObject is already linked to project' do
before do
subject.new_file(file_path, file_content)
end
shared_examples 'a new LfsObject is not created' do
it do
expect do
second_service.new_file(file_path, file_content)
end.not_to change { project.lfs_objects.count }
end
end
context 'and the service is called again with the same repository type' do
let(:second_service) { described_class.new(project, repository, branch_name) }
include_examples 'a new LfsObject is not created'
it 'does not create a new LfsObjectsProject record' do
expect do
second_service.new_file(file_path, file_content)
end.not_to change { project.lfs_objects_projects.count }
end
end
context 'and the service is called again with a different repository type' do
let(:second_service) { described_class.new(project, project.wiki.repository, branch_name) }
before do
expect(second_service).to receive(:lfs_file?).and_return(true)
end
include_examples 'a new LfsObject is not created'
it 'creates a new LfsObjectsProject record' do
expect do
second_service.new_file(file_path, file_content)
end.to change { project.lfs_objects_projects.count }.by(1)
end
it 'sets the correct repository_type on the new LfsObjectsProject record' do
second_service.new_file(file_path, file_content)
repository_types = project.lfs_objects_projects.order(:id).pluck(:repository_type)
expect(repository_types).to eq(%w(project wiki))
end
end
end
end end
end end
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