Merge branch '9490-record-repository_type-on-lfs_objects_projects-ce' into 'master'

CE backport for gitlab-ee!13894 (Save repository_type to LfsObjectsProject)

See merge request gitlab-org/gitlab-ce!29179
This commit is contained in:
Grzegorz Bizon 2019-06-17 13:05:39 +00:00
commit ea1aa78573
14 changed files with 140 additions and 15 deletions

View file

@ -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) }

View file

@ -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

View file

@ -222,7 +222,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

View file

@ -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)

View file

@ -5,7 +5,7 @@ module Files
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)

View file

@ -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 @@ module Lfs
# 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)

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.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

View file

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

View file

@ -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

View file

@ -20,8 +20,8 @@ describe LfsObjectsProject do
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

View file

@ -103,6 +103,20 @@ describe Project do
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

View file

@ -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 @@ describe Lfs::FileTransformer do
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 @@ describe Lfs::FileTransformer do
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