Commit c9515ca5 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'jej/fix-lfs-integrity-with-forks' into 'master'

Handle forks in Gitlab::Checks::LfsIntegrity

Closes #39902

See merge request gitlab-org/gitlab-ce!15279
parents 58bd04ab ebd51744
......@@ -92,15 +92,7 @@ module LfsRequest
end
def storage_project
@storage_project ||= begin
result = project
# TODO: Make this go to the fork_network root immeadiatly
# dependant on the discussion in: https://gitlab.com/gitlab-org/gitlab-ce/issues/39769
result = result.fork_source while result.forked?
result
end
@storage_project ||= project.lfs_storage_project
end
def objects
......
......@@ -6,16 +6,8 @@ class LfsObject < ActiveRecord::Base
mount_uploader :file, LfsObjectUploader
def storage_project(project)
if project && project.forked?
storage_project(project.forked_from_project)
else
project
end
end
def project_allowed_access?(project)
projects.exists?(storage_project(project).id)
projects.exists?(project.lfs_storage_project.id)
end
def self.destroy_unreferenced
......
......@@ -1047,6 +1047,18 @@ class Project < ActiveRecord::Base
forked_from_project || fork_network&.root_project
end
def lfs_storage_project
@lfs_storage_project ||= begin
result = self
# TODO: Make this go to the fork_network root immeadiatly
# dependant on the discussion in: https://gitlab.com/gitlab-org/gitlab-ce/issues/39769
result = result.fork_source while result&.forked?
result || self
end
end
def personal?
!group
end
......
......@@ -15,7 +15,10 @@ module Gitlab
return false unless new_lfs_pointers.present?
existing_count = @project.lfs_objects.where(oid: new_lfs_pointers.map(&:lfs_oid)).count
existing_count = @project.lfs_storage_project
.lfs_objects
.where(oid: new_lfs_pointers.map(&:lfs_oid))
.count
existing_count != new_lfs_pointers.count
end
......
......@@ -165,48 +165,17 @@ describe Gitlab::Checks::ChangeAccess do
end
context 'LFS integrity check' do
let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') }
before do
allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects) do |&lazy_block|
lazy_block.call([blob_object.id])
end
end
context 'with LFS not enabled' do
it 'skips integrity check' do
expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects)
subject.exec
end
end
context 'with LFS enabled' do
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
context 'deletion' do
let(:changes) { { oldrev: oldrev, ref: ref } }
it 'skips integrity check' do
expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects)
subject.exec
end
end
it 'fails if any LFS blobs are missing' do
allow_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).and_return(true)
expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/)
end
it 'succeeds if LFS objects have already been uploaded' do
lfs_object = create(:lfs_object, oid: blob_object.lfs_oid)
create(:lfs_objects_project, project: project, lfs_object: lfs_object)
allow_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).and_return(false)
expect { subject.exec }.not_to raise_error
end
end
end
end
end
require 'spec_helper'
describe Gitlab::Checks::LfsIntegrity do
include ProjectForksHelper
let(:project) { create(:project, :repository) }
let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' }
subject { described_class.new(project, newrev) }
describe '#objects_missing?' do
let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') }
before do
allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects) do |&lazy_block|
lazy_block.call([blob_object.id])
end
end
context 'with LFS not enabled' do
it 'skips integrity check' do
expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects)
subject.objects_missing?
end
end
context 'with LFS enabled' do
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
context 'deletion' do
let(:newrev) { nil }
it 'skips integrity check' do
expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects)
expect(subject.objects_missing?).to be_falsey
end
end
it 'is true if any LFS blobs are missing' do
expect(subject.objects_missing?).to be_truthy
end
it 'is false if LFS objects have already been uploaded' do
lfs_object = create(:lfs_object, oid: blob_object.lfs_oid)
create(:lfs_objects_project, project: project, lfs_object: lfs_object)
expect(subject.objects_missing?).to be_falsey
end
end
context 'for forked project' do
let(:parent_project) { create(:project, :repository) }
let(:project) { fork_project(parent_project, nil, repository: true) }
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
it 'is true parent project is missing LFS objects' do
expect(subject.objects_missing?).to be_truthy
end
it 'is false parent project already conatins LFS objects for the fork' do
lfs_object = create(:lfs_object, oid: blob_object.lfs_oid)
create(:lfs_objects_project, project: parent_project, lfs_object: lfs_object)
expect(subject.objects_missing?).to be_falsey
end
end
end
end
......@@ -1938,6 +1938,24 @@ describe Project do
expect(second_fork.fork_source).to eq(project)
end
end
describe '#lfs_storage_project' do
it 'returns self for non-forks' do
expect(project.lfs_storage_project).to eq project
end
it 'returns the fork network root for forks' do
second_fork = fork_project(forked_project)
expect(second_fork.lfs_storage_project).to eq project
end
it 'returns self when fork_source is nil' do
expect(forked_project).to receive(:fork_source).and_return(nil)
expect(forked_project.lfs_storage_project).to eq forked_project
end
end
end
describe '#pushes_since_gc' do
......
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