Refactor classes involved in git workflow

Refactor some methods, classes and specs in
preparation for versioned snippets.
parent 5a8ca1cb
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class Projects::GitHttpClientController < Projects::ApplicationController class Projects::GitHttpClientController < Projects::ApplicationController
include ActionController::HttpAuthentication::Basic include ActionController::HttpAuthentication::Basic
include KerberosSpnegoHelper include KerberosSpnegoHelper
include Gitlab::Utils::StrongMemoize
attr_reader :authentication_result, :redirected_path attr_reader :authentication_result, :redirected_path
...@@ -47,7 +48,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -47,7 +48,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
send_final_spnego_response send_final_spnego_response
return # Allow access return # Allow access
end end
elsif project && download_request? && http_allowed? && Guest.can?(:download_code, project) elsif http_download_allowed?
@authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code]) @authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code])
...@@ -89,11 +90,9 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -89,11 +90,9 @@ class Projects::GitHttpClientController < Projects::ApplicationController
end end
def repository def repository
repo_type.repository_for(project) strong_memoize(:repository) do
end repo_type.repository_for(project)
end
def wiki?
repo_type.wiki?
end end
def repo_type def repo_type
...@@ -113,8 +112,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -113,8 +112,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController
authentication_result.ci?(project) authentication_result.ci?(project)
end end
def http_allowed? def http_download_allowed?
Gitlab::ProtocolAccess.allowed?('http') Gitlab::ProtocolAccess.allowed?('http') &&
download_request? &&
project && Guest.can?(:download_code, project)
end end
end end
......
...@@ -75,17 +75,19 @@ class Projects::GitHttpController < Projects::GitHttpClientController ...@@ -75,17 +75,19 @@ class Projects::GitHttpController < Projects::GitHttpClientController
end end
def enqueue_fetch_statistics_update def enqueue_fetch_statistics_update
return if wiki? return if repo_type.wiki?
return unless project.daily_statistics_enabled? return unless project&.daily_statistics_enabled?
ProjectDailyStatisticsWorker.perform_async(project.id) ProjectDailyStatisticsWorker.perform_async(project.id)
end end
def access def access
@access ||= access_klass.new(access_actor, project, @access ||= access_klass.new(access_actor, project, 'http',
'http', authentication_abilities: authentication_abilities, authentication_abilities: authentication_abilities,
namespace_path: params[:namespace_id], project_path: project_path, namespace_path: params[:namespace_id],
redirected_path: redirected_path, auth_result_type: auth_result_type) project_path: project_path,
redirected_path: redirected_path,
auth_result_type: auth_result_type)
end end
def access_actor def access_actor
......
...@@ -95,7 +95,7 @@ class Repository ...@@ -95,7 +95,7 @@ class Repository
def path_to_repo def path_to_repo
@path_to_repo ||= @path_to_repo ||=
begin begin
storage = Gitlab.config.repositories.storages[@project.repository_storage] storage = Gitlab.config.repositories.storages[project.repository_storage]
File.expand_path( File.expand_path(
File.join(storage.legacy_disk_path, disk_path + '.git') File.join(storage.legacy_disk_path, disk_path + '.git')
...@@ -128,7 +128,7 @@ class Repository ...@@ -128,7 +128,7 @@ class Repository
commits = Gitlab::Git::Commit.batch_by_oid(raw_repository, oids) commits = Gitlab::Git::Commit.batch_by_oid(raw_repository, oids)
if commits.present? if commits.present?
Commit.decorate(commits, @project) Commit.decorate(commits, project)
else else
[] []
end end
...@@ -159,14 +159,14 @@ class Repository ...@@ -159,14 +159,14 @@ class Repository
} }
commits = Gitlab::Git::Commit.where(options) commits = Gitlab::Git::Commit.where(options)
commits = Commit.decorate(commits, @project) if commits.present? commits = Commit.decorate(commits, project) if commits.present?
CommitCollection.new(project, commits, ref) CommitCollection.new(project, commits, ref)
end end
def commits_between(from, to) def commits_between(from, to)
commits = Gitlab::Git::Commit.between(raw_repository, from, to) commits = Gitlab::Git::Commit.between(raw_repository, from, to)
commits = Commit.decorate(commits, @project) if commits.present? commits = Commit.decorate(commits, project) if commits.present?
commits commits
end end
...@@ -695,13 +695,13 @@ class Repository ...@@ -695,13 +695,13 @@ class Repository
commits = raw_repository.list_last_commits_for_tree(sha, path, offset: offset, limit: limit) commits = raw_repository.list_last_commits_for_tree(sha, path, offset: offset, limit: limit)
commits.each do |path, commit| commits.each do |path, commit|
commits[path] = ::Commit.new(commit, @project) commits[path] = ::Commit.new(commit, project)
end end
end end
def last_commit_for_path(sha, path) def last_commit_for_path(sha, path)
commit = raw_repository.last_commit_for_path(sha, path) commit = raw_repository.last_commit_for_path(sha, path)
::Commit.new(commit, @project) if commit ::Commit.new(commit, project) if commit
end end
def last_commit_id_for_path(sha, path) def last_commit_id_for_path(sha, path)
...@@ -1136,7 +1136,7 @@ class Repository ...@@ -1136,7 +1136,7 @@ class Repository
Gitlab::Git::Commit.find(raw_repository, oid_or_ref) Gitlab::Git::Commit.find(raw_repository, oid_or_ref)
end end
::Commit.new(commit, @project) if commit ::Commit.new(commit, project) if commit
end end
def cache def cache
......
...@@ -9,7 +9,7 @@ module MergeRequests ...@@ -9,7 +9,7 @@ module MergeRequests
end end
def execute(changes) def execute(changes)
return [] unless project.printing_merge_request_link_enabled return [] unless project&.printing_merge_request_link_enabled
branches = get_branches(changes) branches = get_branches(changes)
merge_requests_map = opened_merge_requests_from_source_branches(branches) merge_requests_map = opened_merge_requests_from_source_branches(branches)
......
...@@ -13,7 +13,7 @@ module EE ...@@ -13,7 +13,7 @@ module EE
protected protected
def project_or_wiki def project_or_wiki
@project project
end end
private private
......
...@@ -120,21 +120,13 @@ module API ...@@ -120,21 +120,13 @@ module API
end end
def gl_project_path def gl_project_path
if wiki? repository.full_path
project.wiki.full_path
else
project.full_path
end
end end
# Return the repository depending on whether we want the wiki or the # Return the repository depending on whether we want the wiki or the
# regular repository # regular repository
def repository def repository
if repo_type.wiki? @repository ||= repo_type.repository_for(project)
project.wiki.repository
else
project.repository
end
end end
# Return the Gitaly Address if it is enabled # Return the Gitaly Address if it is enabled
......
...@@ -224,9 +224,9 @@ module API ...@@ -224,9 +224,9 @@ module API
response.add_merge_request_urls(merge_request_urls) response.add_merge_request_urls(merge_request_urls)
# A user is not guaranteed to be returned; an orphaned write deploy # Neither User nor Project are guaranteed to be returned; an orphaned write deploy
# key could be used # key could be used
if user if user && project
redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id) redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id)
project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id) project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id)
......
...@@ -32,9 +32,12 @@ module Gitlab ...@@ -32,9 +32,12 @@ module Gitlab
def self.find_project(project_path) def self.find_project(project_path)
project = Project.find_by_full_path(project_path, follow_redirects: true) project = Project.find_by_full_path(project_path, follow_redirects: true)
was_redirected = project && project.full_path.casecmp(project_path) != 0
[project, was_redirected] [project, redirected?(project, project_path)]
end
def self.redirected?(project, project_path)
project && project.full_path.casecmp(project_path) != 0
end end
end end
end end
......
...@@ -7,7 +7,8 @@ module Gitlab ...@@ -7,7 +7,8 @@ module Gitlab
def initialize(repository, extra_namespace: nil, backend: Rails.cache) def initialize(repository, extra_namespace: nil, backend: Rails.cache)
@repository = repository @repository = repository
@namespace = "#{repository.full_path}:#{repository.project.id}" @namespace = "#{repository.full_path}"
@namespace += ":#{repository.project.id}" if repository.project
@namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace @namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace
@backend = backend @backend = backend
end end
......
...@@ -7,7 +7,8 @@ module Gitlab ...@@ -7,7 +7,8 @@ module Gitlab
def initialize(repository, extra_namespace: nil, expires_in: 2.weeks) def initialize(repository, extra_namespace: nil, expires_in: 2.weeks)
@repository = repository @repository = repository
@namespace = "#{repository.full_path}:#{repository.project.id}" @namespace = "#{repository.full_path}"
@namespace += ":#{repository.project.id}" if repository.project
@namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace @namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace
@expires_in = expires_in @expires_in = expires_in
end end
......
...@@ -22,18 +22,16 @@ module Gitlab ...@@ -22,18 +22,16 @@ module Gitlab
def git_http_ok(repository, repo_type, user, action, show_all_refs: false) def git_http_ok(repository, repo_type, user, action, show_all_refs: false)
raise "Unsupported action: #{action}" unless ALLOWED_GIT_HTTP_ACTIONS.include?(action.to_s) raise "Unsupported action: #{action}" unless ALLOWED_GIT_HTTP_ACTIONS.include?(action.to_s)
project = repository.project
attrs = { attrs = {
GL_ID: Gitlab::GlId.gl_id(user), GL_ID: Gitlab::GlId.gl_id(user),
GL_REPOSITORY: repo_type.identifier_for_subject(project), GL_REPOSITORY: repo_type.identifier_for_subject(repository.project),
GL_USERNAME: user&.username, GL_USERNAME: user&.username,
ShowAllRefs: show_all_refs, ShowAllRefs: show_all_refs,
Repository: repository.gitaly_repository.to_h, Repository: repository.gitaly_repository.to_h,
GitConfigOptions: [], GitConfigOptions: [],
GitalyServer: { GitalyServer: {
address: Gitlab::GitalyClient.address(project.repository_storage), address: Gitlab::GitalyClient.address(repository.storage),
token: Gitlab::GitalyClient.token(project.repository_storage), token: Gitlab::GitalyClient.token(repository.storage),
features: Feature::Gitaly.server_feature_flags features: Feature::Gitaly.server_feature_flags
} }
} }
......
...@@ -3,10 +3,17 @@ ...@@ -3,10 +3,17 @@
require 'spec_helper' require 'spec_helper'
describe Projects::GitHttpController do describe Projects::GitHttpController do
let_it_be(:project) { create(:project, :public, :repository) }
let(:project_params) do
{
namespace_id: project.namespace.to_param,
project_id: project.path + '.git'
}
end
let(:params) { project_params }
describe 'HEAD #info_refs' do describe 'HEAD #info_refs' do
it 'returns 403' do it 'returns 403' do
project = create(:project, :public, :repository)
head :info_refs, params: { namespace_id: project.namespace.to_param, project_id: project.path + '.git' } head :info_refs, params: { namespace_id: project.namespace.to_param, project_id: project.path + '.git' }
expect(response.status).to eq(403) expect(response.status).to eq(403)
...@@ -14,18 +21,17 @@ describe Projects::GitHttpController do ...@@ -14,18 +21,17 @@ describe Projects::GitHttpController do
end end
describe 'GET #info_refs' do describe 'GET #info_refs' do
let(:params) { project_params.merge(service: 'git-upload-pack') }
it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do
stub_application_setting(enabled_git_access_protocol: 'ssh') stub_application_setting(enabled_git_access_protocol: 'ssh')
project = create(:project, :public, :repository)
get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } get :info_refs, params: params
expect(response.status).to eq(401) expect(response.status).to eq(401)
end end
context 'with exceptions' do context 'with exceptions' do
let(:project) { create(:project, :public, :repository) }
before do before do
allow(controller).to receive(:verify_workhorse_api!).and_return(true) allow(controller).to receive(:verify_workhorse_api!).and_return(true)
end end
...@@ -33,7 +39,7 @@ describe Projects::GitHttpController do ...@@ -33,7 +39,7 @@ describe Projects::GitHttpController do
it 'returns 503 with GRPC Unavailable' do it 'returns 503 with GRPC Unavailable' do
allow(controller).to receive(:access_check).and_raise(GRPC::Unavailable) allow(controller).to receive(:access_check).and_raise(GRPC::Unavailable)
get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } get :info_refs, params: params
expect(response.status).to eq(503) expect(response.status).to eq(503)
end end
...@@ -41,11 +47,27 @@ describe Projects::GitHttpController do ...@@ -41,11 +47,27 @@ describe Projects::GitHttpController do
it 'returns 503 with timeout error' do it 'returns 503 with timeout error' do
allow(controller).to receive(:access_check).and_raise(Gitlab::GitAccess::TimeoutError) allow(controller).to receive(:access_check).and_raise(Gitlab::GitAccess::TimeoutError)
get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } get :info_refs, params: params
expect(response.status).to eq(503) expect(response.status).to eq(503)
expect(response.body).to eq 'Gitlab::GitAccess::TimeoutError' expect(response.body).to eq 'Gitlab::GitAccess::TimeoutError'
end end
end end
end end
describe 'POST #git_upload_pack' do
before do
allow(controller).to receive(:authenticate_user).and_return(true)
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
allow(controller).to receive(:access_check).and_return(nil)
end
after do
post :git_upload_pack, params: params
end
it 'updates project statistics' do
expect(ProjectDailyStatisticsWorker).to receive(:perform_async)
end
end
end end
...@@ -3,8 +3,8 @@ ...@@ -3,8 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::RepositoryCache do describe Gitlab::RepositoryCache do
let_it_be(:project) { create(:project) }
let(:backend) { double('backend').as_null_object } let(:backend) { double('backend').as_null_object }
let(:project) { create(:project) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:namespace) { "#{repository.full_path}:#{project.id}" } let(:namespace) { "#{repository.full_path}:#{project.id}" }
let(:cache) { described_class.new(repository, backend: backend) } let(:cache) { described_class.new(repository, backend: backend) }
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:namespace) { "#{repository.full_path}:#{project.id}" } let(:namespace) { "#{repository.full_path}:#{project.id}" }
let(:cache) { described_class.new(repository) } let(:cache) { described_class.new(repository) }
......
...@@ -1000,6 +1000,22 @@ describe API::Internal::Base do ...@@ -1000,6 +1000,22 @@ describe API::Internal::Base do
it 'does not try to notify that project moved' do it 'does not try to notify that project moved' do
allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil) allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil)
expect(Gitlab::Checks::ProjectMoved).not_to receive(:fetch_message)
post api('/internal/post_receive'), params: valid_params
expect(response).to have_gitlab_http_status(200)
end
end
context 'when project is nil' do
let(:gl_repository) { 'project-foo' }
it 'does not try to notify that project moved' do
allow(Gitlab::GlRepository).to receive(:parse).and_return([nil, Gitlab::GlRepository::PROJECT])
expect(Gitlab::Checks::ProjectMoved).not_to receive(:fetch_message)
post api('/internal/post_receive'), params: valid_params post api('/internal/post_receive'), params: valid_params
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
......
...@@ -45,6 +45,13 @@ describe MergeRequests::GetUrlsService do ...@@ -45,6 +45,13 @@ describe MergeRequests::GetUrlsService do
end end
end end
context 'when project is nil' do
let(:project) { nil }
let(:changes) { default_branch_changes }
it_behaves_like 'no_merge_request_url'
end
context 'pushing to default branch' do context 'pushing to default branch' do
let(:changes) { default_branch_changes } let(:changes) { default_branch_changes }
......
...@@ -245,8 +245,8 @@ module TestEnv ...@@ -245,8 +245,8 @@ module TestEnv
end end
end end
def copy_repo(project, bare_repo:, refs:) def copy_repo(subject, bare_repo:, refs:)
target_repo_path = File.expand_path(repos_path + "/#{project.disk_path}.git") target_repo_path = File.expand_path(repos_path + "/#{subject.disk_path}.git")
FileUtils.mkdir_p(target_repo_path) FileUtils.mkdir_p(target_repo_path)
FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path) FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path)
......
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