Commit ab886fb2 authored by Markus Koller's avatar Markus Koller Committed by Francisco Javier López

Pass snippet branch to ls_files calls

Also add a `Snippet.default_branch` helper with a fallback to the
currently hardcoded 'master'.
parent 4a84dde7
......@@ -214,7 +214,7 @@ class Snippet < ApplicationRecord
def blobs
return [] unless repository_exists?
repository.ls_files(repository.root_ref).map { |file| Blob.lazy(repository, repository.root_ref, file) }
repository.ls_files(default_branch).map { |file| Blob.lazy(repository, default_branch, file) }
end
def hook_attrs
......@@ -309,6 +309,11 @@ class Snippet < ApplicationRecord
end
end
override :default_branch
def default_branch
super || 'master'
end
def repository_storage
snippet_repository&.shard_name || self.class.pick_repository_storage
end
......@@ -336,17 +341,17 @@ class Snippet < ApplicationRecord
def file_name_on_repo
return if repository.empty?
list_files(repository.root_ref).first
list_files(default_branch).first
end
def list_files(ref = nil)
return [] if repository.empty?
repository.ls_files(ref)
repository.ls_files(ref || default_branch)
end
def multiple_files?
list_files(repository.root_ref).size > 1
list_files.size > 1
end
class << self
......
......@@ -93,7 +93,7 @@ class SnippetRepository < ApplicationRecord
end
def get_last_empty_file_index
repository.ls_files(nil).inject(0) do |max, file|
repository.ls_files(snippet.default_branch).inject(0) do |max, file|
idx = file[EMPTY_FILE_PATTERN, 1].to_i
[idx, max].max
end
......
......@@ -25,7 +25,7 @@ class SnippetStatistics < ApplicationRecord
def update_file_count
count = if snippet.repository_exists?
repository.ls_files(repository.root_ref).size
repository.ls_files(snippet.default_branch).size
else
0
end
......
......@@ -82,7 +82,7 @@ module Snippets
def create_commit
commit_attrs = {
branch_name: 'master',
branch_name: @snippet.default_branch,
message: 'Initial commit'
}
......
......@@ -39,7 +39,7 @@ module Snippets
def check_branch_name_default!
branches = repository.branch_names
return if branches.first == Gitlab::Checks::SnippetCheck::DEFAULT_BRANCH
return if branches.first == snippet.default_branch
raise RepositoryValidationError, _('Repository has an invalid default branch name.')
end
......@@ -51,7 +51,7 @@ module Snippets
end
def check_file_count!
file_count = repository.ls_files(nil).size
file_count = repository.ls_files(snippet.default_branch).size
limit = Snippet.max_file_limit(current_user)
if file_count > limit
......
......@@ -93,7 +93,7 @@ module Snippets
raise UpdateError unless snippet.snippet_repository
commit_attrs = {
branch_name: 'master',
branch_name: snippet.default_branch,
message: 'Update snippet'
}
......
......@@ -3,7 +3,6 @@
module Gitlab
module Checks
class SnippetCheck < BaseChecker
DEFAULT_BRANCH = 'master'.freeze
ERROR_MESSAGES = {
create_delete_branch: 'You can not create or delete branches.'
}.freeze
......@@ -11,17 +10,18 @@ module Gitlab
ATTRIBUTES = %i[oldrev newrev ref branch_name tag_name logger].freeze
attr_reader(*ATTRIBUTES)
def initialize(change, logger:)
def initialize(change, default_branch:, logger:)
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref)
@tag_name = Gitlab::Git.tag_name(@ref)
@default_branch = default_branch
@logger = logger
@logger.append_message("Running checks for ref: #{@branch_name || @tag_name}")
end
def validate!
if creation? || deletion?
if !@default_branch || creation? || deletion?
raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_delete_branch]
end
......@@ -31,7 +31,7 @@ module Gitlab
private
def creation?
@branch_name != DEFAULT_BRANCH && super
@branch_name != @default_branch && super
end
end
end
......
......@@ -119,7 +119,7 @@ module Gitlab
override :check_single_change_access
def check_single_change_access(change, _skip_lfs_integrity_check: false)
Checks::SnippetCheck.new(change, logger: logger).validate!
Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, logger: logger).validate!
Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit(user), logger: logger).validate!
rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message
......
......@@ -327,6 +327,6 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat
end
def ls_files(snippet)
raw_repository(snippet).ls_files(nil)
raw_repository(snippet).ls_files(snippet.default_branch)
end
end
......@@ -5,10 +5,12 @@ require 'spec_helper'
RSpec.describe Gitlab::Checks::SnippetCheck do
include_context 'change access checks context'
let(:snippet) { create(:personal_snippet, :repository) }
let_it_be(:snippet) { create(:personal_snippet, :repository) }
let(:user_access) { Gitlab::UserAccessSnippet.new(user, snippet: snippet) }
let(:default_branch) { snippet.default_branch }
subject { Gitlab::Checks::SnippetCheck.new(changes, logger: logger) }
subject { Gitlab::Checks::SnippetCheck.new(changes, default_branch: default_branch, logger: logger) }
describe '#validate!' do
it 'does not raise any error' do
......@@ -39,5 +41,13 @@ RSpec.describe Gitlab::Checks::SnippetCheck do
end
end
end
context 'when default_branch is nil' do
let(:default_branch) { nil }
it 'raises an error' do
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.')
end
end
end
end
......@@ -108,6 +108,7 @@ RSpec.describe SnippetRepository do
before do
allow(snippet).to receive(:repository).and_return(repo)
allow(repo).to receive(:ls_files).and_return([])
allow(repo).to receive(:root_ref).and_return('master')
end
it 'infers the commit action based on the parameters if not present' do
......@@ -197,7 +198,7 @@ RSpec.describe SnippetRepository do
shared_examples 'snippet repository with file names' do |*filenames|
it 'sets a name for unnamed files' do
ls_files = snippet.repository.ls_files(nil)
ls_files = snippet.repository.ls_files(snippet.default_branch)
expect(ls_files).to include(*filenames)
end
end
......@@ -306,6 +307,6 @@ RSpec.describe SnippetRepository do
end
def first_blob(snippet)
snippet.repository.blob_at('master', snippet.repository.ls_files(nil).first)
snippet.repository.blob_at('master', snippet.repository.ls_files(snippet.default_branch).first)
end
end
......@@ -744,6 +744,16 @@ RSpec.describe Snippet do
subject
end
context 'when ref is nil' do
let(:ref) { nil }
it 'lists files from the repository from the deafult_branch' do
expect(snippet.repository).to receive(:ls_files).with(snippet.default_branch)
subject
end
end
end
context 'when snippet does not have a repository' do
......
......@@ -36,7 +36,7 @@ RSpec.describe SnippetStatistics do
subject { statistics.update_file_count }
it 'updates the count of files' do
file_count = snippet_with_repo.repository.ls_files(nil).count
file_count = snippet_with_repo.repository.ls_files(snippet_with_repo.default_branch).count
subject
......
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