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.
This commit is contained in:
Luke Duncalfe 2019-06-06 18:51:50 +12:00
parent 82822945d4
commit f7163afb8a
14 changed files with 140 additions and 15 deletions

View File

@ -5,7 +5,7 @@ class LfsObject < ApplicationRecord
include ObjectStorage::BackgroundMove
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) }

View File

@ -5,11 +5,17 @@ class LfsObjectsProject < ApplicationRecord
belongs_to :lfs_object
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
after_commit :update_project_statistics, on: [:create, :destroy]
enum repository_type: {
project: 0,
wiki: 1,
design: 2 ## EE-specific
}
private
def update_project_statistics

View File

@ -223,7 +223,7 @@ class Project < ApplicationRecord
has_many :starrers, through: :users_star_projects, source: :user
has_many :releases
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 :project_group_links
has_many :invited_groups, through: :project_group_links, source: :group

View File

@ -3,7 +3,7 @@
module Files
class CreateService < Files::BaseService
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)

View File

@ -5,7 +5,7 @@ module Files
UPDATE_FILE_ACTIONS = %w(update move delete chmod).freeze
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 = transform_move_actions(actions)

View File

@ -8,17 +8,17 @@ module Lfs
# pointer returned. If the file isn't in LFS the untransformed content
# 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
# create_transformed_commit(content_or_lfs_pointer)
#
class FileTransformer
attr_reader :project, :branch_name
attr_reader :project, :repository, :repository_type, :branch_name
delegate :repository, to: :project
def initialize(project, branch_name)
def initialize(project, repository, branch_name)
@project = project
@repository = repository
@repository_type = repository.repo_type.name
@branch_name = branch_name
end
@ -64,7 +64,11 @@ module Lfs
# rubocop: enable CodeReuse/ActiveRecord
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
def parse_file_content(file_content, encoding: nil)

View File

@ -0,0 +1,11 @@
# 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

View File

@ -0,0 +1,17 @@
# 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

View File

@ -1190,6 +1190,8 @@ ActiveRecord::Schema.define(version: 20190611161641) do
t.integer "project_id", null: false
t.datetime "created_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
end

View File

@ -2,5 +2,6 @@ FactoryBot.define do
factory :lfs_objects_project do
lfs_object
project
repository_type :project
end
end

View File

@ -3,6 +3,20 @@
require 'spec_helper'
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
it 'returns true when file_store is equal to LfsObjectUploader::Store::LOCAL' do
subject.file_store = LfsObjectUploader::Store::LOCAL

View File

@ -20,8 +20,8 @@ describe LfsObjectsProject do
it 'validates object id' do
is_expected.to validate_uniqueness_of(:lfs_object_id)
.scoped_to(:project_id)
.with_message("already exists in project")
.scoped_to(:project_id, :repository_type)
.with_message("already exists in repository")
end
end

View File

@ -103,6 +103,20 @@ describe Project do
expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project)
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
it "has a project_feature" do
expect(described_class.new.project_feature).to be_present

View File

@ -3,13 +3,13 @@
require "spec_helper"
describe Lfs::FileTransformer do
let(:project) { create(:project, :repository) }
let(:project) { create(:project, :repository, :wiki_repo) }
let(:repository) { project.repository }
let(:file_content) { 'Test file content' }
let(:branch_name) { '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
context 'with lfs disabled' do
@ -100,6 +100,12 @@ describe Lfs::FileTransformer do
end.to change { project.lfs_objects.count }.by(1)
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
let(:lfs_pointer) { Gitlab::Git::LfsPointerFile.new(file_content) }
@ -113,6 +119,56 @@ describe Lfs::FileTransformer do
end.to change { project.lfs_objects.count }.by(1)
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