Skip to content

G
gitlab-ce
Commit fdb32ee4 authored by Nick Thomas's avatar Nick Thomas
Browse files
Options

Move the blob LFS enabled lookup to the repository 

Blobs can "look through" LFS pointers to the backing object, but only
if the blob belongs to a project or project wiki repository (for now).
This change moves the logic for that to the repository as a precursor
to making it static data.
parent dfb38bf8
Hide whitespace changes
Inline Side-by-side
Showing with 150 additions and 444 deletions
app/models/blob.rb
View file @ fdb32ee4
...@@ -129,7 +129,7 @@ class Blob < SimpleDelegator ...@@ -129,7 +129,7 @@ class Blob < SimpleDelegator
def external_storage_error? def external_storage_error?
if external_storage == :lfs if external_storage == :lfs
!project&.lfs_enabled? !repository.lfs_enabled?
else else
false false
end end
......
app/models/repository.rb
View file @ fdb32ee4
...@@ -1120,6 +1120,17 @@ class Repository ...@@ -1120,6 +1120,17 @@ class Repository
end end
end end
# TODO: pass this in directly to `Blob` rather than delegating it to here
#
# https://gitlab.com/gitlab-org/gitlab/-/issues/201886
def lfs_enabled?
if container.is_a?(Project)
container.lfs_enabled?
else
false # LFS is not supported for snippet or group repositories
end
end
private private
# TODO Genericize finder, later split this on finders by Ref or Oid # TODO Genericize finder, later split this on finders by Ref or Oid
......
ee/spec/models/repository_spec.rb
View file @ fdb32ee4
...@@ -219,6 +219,29 @@ describe Repository do ...@@ -219,6 +219,29 @@ describe Repository do
end end
end end
describe '#lfs_enabled? (design repositories)' do
let(:project) { create(:project, :design_repo, lfs_enabled: lfs_enabled) }
let(:repository) { project.design_repository }
before do
stub_lfs_setting(enabled: true)
end
subject { repository.lfs_enabled? }
context 'project has LFS disabled' do
let(:lfs_enabled) { false }
it { is_expected.to be_falsy }
end
context 'project has LFS enabled' do
let(:lfs_enabled) { true }
it { is_expected.to be_truthy }
end
end
describe '#upstream_branch_name' do describe '#upstream_branch_name' do
let(:pull_mirror_branch_prefix) { 'upstream/' } let(:pull_mirror_branch_prefix) { 'upstream/' }
let(:branch_name) { 'upstream/master' } let(:branch_name) { 'upstream/master' }
......
spec/models/blob_spec.rb
View file @ fdb32ee4
...@@ -5,12 +5,17 @@ require 'spec_helper' ...@@ -5,12 +5,17 @@ require 'spec_helper'
describe Blob do describe Blob do
include FakeBlobHelpers include FakeBlobHelpers
let(:project) { build(:project, lfs_enabled: true) } using RSpec::Parameterized::TableSyntax
let(:project) { build(:project) }
let(:personal_snippet) { build(:personal_snippet) } let(:personal_snippet) { build(:personal_snippet) }
let(:project_snippet) { build(:project_snippet, project: project) } let(:project_snippet) { build(:project_snippet, project: project) }
let(:repository) { project.repository }
let(:lfs_enabled) { true }
before do before do
allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) allow(repository).to receive(:lfs_enabled?) { lfs_enabled }
end end
describe '.decorate' do describe '.decorate' do
...@@ -128,399 +133,84 @@ describe Blob do ...@@ -128,399 +133,84 @@ describe Blob do
end end
describe '#external_storage_error?' do describe '#external_storage_error?' do
shared_examples 'no error' do subject { blob.external_storage_error? }
it do
expect(blob.external_storage_error?).to be_falsey
end
end
shared_examples 'returns error' do
it do
expect(blob.external_storage_error?).to be_truthy
end
end
context 'if the blob is stored in LFS' do context 'if the blob is stored in LFS' do
let(:blob) { fake_blob(path: 'file.pdf', lfs: true, container: container) } let(:blob) { fake_blob(path: 'file.pdf', lfs: true) }
context 'when the project has LFS enabled' do
context 'with project' do
let(:container) { project }
it_behaves_like 'no error'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns error'
end
context 'with project snippet' do context 'when LFS is enabled' do
let(:container) { project_snippet } let(:lfs_enabled) { true }
it_behaves_like 'no error' it { is_expected.to be_falsy }
end
end end
context 'when the project does not have LFS enabled' do context 'when LFS is not enabled' do
before do let(:lfs_enabled) { false }
project.lfs_enabled = false
end
context 'with project' do
let(:container) { project }
it_behaves_like 'returns error' it { is_expected.to be_truthy }
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns error'
end
end end
end end
context 'if the blob is not stored in LFS' do context 'if the blob is not stored in LFS' do
let(:blob) { fake_blob(path: 'file.md', container: container) } let(:blob) { fake_blob(path: 'file.md') }
context 'with project' do
let(:container) { project }
it_behaves_like 'no error'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'no error'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'no error' it { is_expected.to be_falsy }
end
end end
end end
describe '#stored_externally?' do describe '#stored_externally?' do
subject { blob.stored_externally? }
context 'if the blob is stored in LFS' do context 'if the blob is stored in LFS' do
let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } let(:blob) { fake_blob(path: 'file.pdf', lfs: true) }
shared_examples 'returns true' do context 'when LFS is enabled' do
it do let(:lfs_enabled) { true }
expect(blob.stored_externally?).to be_truthy
end
end
shared_examples 'returns false' do
it do
expect(blob.stored_externally?).to be_falsey
end
end
context 'when the project has LFS enabled' do
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do it { is_expected.to be_truthy }
let(:container) { personal_snippet }
it_behaves_like 'returns true'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end end
context 'when the project does not have LFS enabled' do context 'when LFS is not enabled' do
before do let(:lfs_enabled) { false }
project.lfs_enabled = false
end
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false' it { is_expected.to be_falsy }
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false'
end
end end
end end
context 'if the blob is not stored in LFS' do context 'if the blob is not stored in LFS' do
let(:blob) { fake_blob(path: 'file.md') } let(:blob) { fake_blob(path: 'file.md') }
it 'returns false' do it { is_expected.to be_falsy }
expect(blob.stored_externally?).to be_falsey
end
end end
end end
describe '#binary?' do describe '#binary?' do
shared_examples 'returns true' do context 'an lfs object' do
it do where(:filename, :is_binary) do
expect(blob.binary?).to be_truthy 'file.pdf' | true
end 'file.md' | false
end 'file.txt' | false
'file.ics' | false
shared_examples 'returns false' do 'file.rb' | false
it do 'file.exe' | true
expect(blob.binary?).to be_falsey 'file.ini' | false
end 'file.wtf' | true
end
context 'if the blob is stored externally' do
let(:blob) { fake_blob(path: file, lfs: true) }
context 'if the extension has a rich viewer' do
context 'if the viewer is binary' do
let(:file) { 'file.pdf' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns true'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end
context 'if the viewer is text-based' do
let(:file) { 'file.md' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false'
end
end
end end
context "if the extension doesn't have a rich viewer" do with_them do
context 'if the extension has a text mime type' do let(:blob) { fake_blob(path: filename, lfs: true, container: project) }
context 'if the extension is for a programming language' do
let(:file) { 'file.txt' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false'
end
end
context 'if the extension is not for a programming language' do
let(:file) { 'file.ics' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false' it { expect(blob.binary?).to eq(is_binary) }
end
end
end
context 'if the extension has a binary mime type' do
context 'if the extension is for a programming language' do
let(:file) { 'file.rb' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false'
end
end
context 'if the extension is not for a programming language' do
let(:file) { 'file.exe' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns true'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end
end
context 'if the extension has an unknown mime type' do
context 'if the extension is for a programming language' do
let(:file) { 'file.ini' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false'
end
end
context 'if the extension is not for a programming language' do
let(:file) { 'file.wtf' }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns true'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end
end
end end
end end
context 'if the blob is not stored externally' do context 'a non-lfs object' do
context 'if the blob is binary' do let(:blob) { fake_blob(path: 'anything', container: project) }
let(:blob) { fake_blob(path: 'file.pdf', binary: true, container: container) }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns true'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end
context 'if the blob is text-based' do
let(:blob) { fake_blob(path: 'file.md', container: container) }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do it 'delegates to binary_in_repo?' do
let(:container) { project_snippet } expect(blob).to receive(:binary_in_repo?) { :result }
it_behaves_like 'returns false' expect(blob.binary?).to eq(:result)
end
end end
end end
end end
...@@ -569,9 +259,7 @@ describe Blob do ...@@ -569,9 +259,7 @@ describe Blob do
describe '#rich_viewer' do describe '#rich_viewer' do
context 'when the blob has an external storage error' do context 'when the blob has an external storage error' do
before do let(:lfs_enabled) { false }
project.lfs_enabled = false
end
it 'returns nil' do it 'returns nil' do
blob = fake_blob(path: 'file.pdf', lfs: true) blob = fake_blob(path: 'file.pdf', lfs: true)
...@@ -631,9 +319,7 @@ describe Blob do ...@@ -631,9 +319,7 @@ describe Blob do
describe '#auxiliary_viewer' do describe '#auxiliary_viewer' do
context 'when the blob has an external storage error' do context 'when the blob has an external storage error' do
before do let(:lfs_enabled) { false }
project.lfs_enabled = false
end
it 'returns nil' do it 'returns nil' do
blob = fake_blob(path: 'LICENSE', lfs: true) blob = fake_blob(path: 'LICENSE', lfs: true)
...@@ -676,63 +362,21 @@ describe Blob do ...@@ -676,63 +362,21 @@ describe Blob do
end end
describe '#rendered_as_text?' do describe '#rendered_as_text?' do
shared_examples 'returns true' do subject { blob.rendered_as_text?(ignore_errors: ignore_errors) }
it do
expect(blob.rendered_as_text?(ignore_errors: ignore_errors)).to be_truthy
end
end
shared_examples 'returns false' do
it do
expect(blob.rendered_as_text?(ignore_errors: ignore_errors)).to be_falsey
end
end
context 'when ignoring errors' do context 'when ignoring errors' do
let(:ignore_errors) { true } let(:ignore_errors) { true }
context 'when the simple viewer is text-based' do context 'when the simple viewer is text-based' do
let(:blob) { fake_blob(path: 'file.md', size: 100.megabytes, container: container) } let(:blob) { fake_blob(path: 'file.md', size: 100.megabytes) }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns true' it { is_expected.to be_truthy }
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end end
context 'when the simple viewer is binary' do context 'when the simple viewer is binary' do
let(:blob) { fake_blob(path: 'file.pdf', binary: true, size: 100.megabytes, container: container) } let(:blob) { fake_blob(path: 'file.pdf', binary: true, size: 100.megabytes) }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false' it { is_expected.to be_falsy }
end
end end
end end
...@@ -740,47 +384,15 @@ describe Blob do ...@@ -740,47 +384,15 @@ describe Blob do
let(:ignore_errors) { false } let(:ignore_errors) { false }
context 'when the viewer has render errors' do context 'when the viewer has render errors' do
let(:blob) { fake_blob(path: 'file.md', size: 100.megabytes, container: container) } let(:blob) { fake_blob(path: 'file.md', size: 100.megabytes) }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns false'
end
context 'with personal snippet' do
let(:container) { personal_snippet }
it_behaves_like 'returns false'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns false' it { is_expected.to be_falsy }
end
end end
context "when the viewer doesn't have render errors" do context "when the viewer doesn't have render errors" do
let(:blob) { fake_blob(path: 'file.md', container: container) } let(:blob) { fake_blob(path: 'file.md') }
context 'with project' do
let(:container) { project }
it_behaves_like 'returns true'
end
context 'with personal snippet' do it { is_expected.to be_truthy }
let(:container) { personal_snippet }
it_behaves_like 'returns true'
end
context 'with project snippet' do
let(:container) { project_snippet }
it_behaves_like 'returns true'
end
end end
end end
end end
......
spec/models/repository_spec.rb
View file @ fdb32ee4
...@@ -2874,4 +2874,64 @@ describe Repository do ...@@ -2874,4 +2874,64 @@ describe Repository do
expect(repository.submodule_links).to be_a(Gitlab::SubmoduleLinks) expect(repository.submodule_links).to be_a(Gitlab::SubmoduleLinks)
end end
end end
describe '#lfs_enabled?' do
let_it_be(:project) { create(:project, :repository, lfs_enabled: true) }
subject { repository.lfs_enabled? }
context 'for a project repository' do
let(:repository) { project.repository }
it 'returns true when LFS is enabled' do
stub_lfs_setting(enabled: true)
is_expected.to be_truthy
end
it 'returns false when LFS is disabled' do
stub_lfs_setting(enabled: false)
is_expected.to be_falsy
end
end
context 'for a project wiki repository' do
let(:repository) { project.wiki.repository }
it 'returns true when LFS is enabled' do
stub_lfs_setting(enabled: true)
is_expected.to be_truthy
end
it 'returns false when LFS is disabled' do
stub_lfs_setting(enabled: false)
is_expected.to be_falsy
end
end
context 'for a project snippet repository' do
let(:snippet) { create(:project_snippet, project: project) }
let(:repository) { snippet.repository }
it 'returns false when LFS is enabled' do
stub_lfs_setting(enabled: true)
is_expected.to be_falsy
end
end
context 'for a personal snippet repository' do
let(:snippet) { create(:personal_snippet) }
let(:repository) { snippet.repository }
it 'returns false when LFS is enabled' do
stub_lfs_setting(enabled: true)
is_expected.to be_falsy
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7