diff --git a/app/models/issue.rb b/app/models/issue.rb index 35e08fa915b..ddb51ad5775 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -108,9 +108,9 @@ class Issue < ActiveRecord::Base end def related_branches - return [] if self.project.empty_repo? - - self.project.repository.branch_names.select { |branch| branch.end_with?("-#{iid}") } + project.repository.branch_names.select do |branch| + branch.end_with?("-#{iid}") + end end # Reset issue events cache diff --git a/app/models/project.rb b/app/models/project.rb index 412c6c6732d..fd36dca0cad 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -876,6 +876,7 @@ class Project < ActiveRecord::Base # Forked import is handled asynchronously unless forked? if gitlab_shell.add_repository(path_with_namespace) + repository.after_create true else errors.add(:base, 'Failed to create repository via gitlab-shell') diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 59b1b86d1fb..7c1a61bb0bf 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -123,23 +123,27 @@ class ProjectWiki end def repository - Repository.new(path_with_namespace, @project) + @repository ||= Repository.new(path_with_namespace, @project) end def default_branch wiki.class.default_ref end - private - def create_repo! if init_repo(path_with_namespace) - Gollum::Wiki.new(path_to_repo) + wiki = Gollum::Wiki.new(path_to_repo) else raise CouldNotCreateWikiError end + + repository.after_create + + wiki end + private + def init_repo(path_with_namespace) gitlab_shell.add_repository(path_with_namespace) end diff --git a/app/models/repository.rb b/app/models/repository.rb index 25d24493f6e..13154eb4205 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -42,12 +42,15 @@ class Repository end def exists? - return false unless raw_repository + return @exists unless @exists.nil? - raw_repository.rugged - true - rescue Gitlab::Git::Repository::NoRepository - false + @exists = cache.fetch(:exists?) do + begin + raw_repository && raw_repository.rugged ? true : false + rescue Gitlab::Git::Repository::NoRepository + false + end + end end def empty? @@ -320,12 +323,23 @@ class Repository @avatar = nil end + def expire_exists_cache + cache.expire(:exists?) + @exists = nil + end + + # Runs code after a repository has been created. + def after_create + expire_exists_cache + end + # Runs code just before a repository is deleted. def before_delete expire_cache if exists? expire_root_ref_cache expire_emptiness_caches + expire_exists_cache end # Runs code just before the HEAD of a repository is changed. @@ -351,6 +365,7 @@ class Repository # Runs code after a repository has been forked/imported. def after_import expire_emptiness_caches + expire_exists_cache end # Runs code after a new commit has been pushed. diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 21d311579e3..f9e32337983 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -20,14 +20,15 @@ class RepositoryForkWorker return end + project.repository.after_import + unless project.valid_repo? - logger.error("Project #{id} had an invalid repository after fork") + logger.error("Project #{project_id} had an invalid repository after fork") project.update(import_error: "The forked repository is invalid.") project.import_fail return end - project.repository.after_import project.import_finish end end diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index ff21c7d1b83..de7e2b37725 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -160,6 +160,7 @@ Feature: Project Issues Scenario: Issues on empty project Given empty project "Empty Project" + And I have an ssh key When I visit empty project page And I see empty project details with ssh clone info When I visit empty project's issues page diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 8c31fa890b2..aff5ca676be 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -5,6 +5,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps include SharedNote include SharedPaths include SharedMarkdown + include SharedUser step 'I should see "Release 0.4" in issues' do expect(page).to have_content "Release 0.4" @@ -240,7 +241,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps end step 'empty project "Empty Project"' do - create :empty_project, name: 'Empty Project', namespace: @user.namespace + create :project_empty_repo, name: 'Empty Project', namespace: @user.namespace end When 'I visit empty project page' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 2cd81231144..a49bd8960ee 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -2,7 +2,7 @@ require('spec_helper') describe Projects::IssuesController do describe "GET #index" do - let(:project) { create(:project) } + let(:project) { create(:project_empty_repo) } let(:user) { create(:user) } let(:issue) { create(:issue, project: project) } @@ -41,7 +41,7 @@ describe Projects::IssuesController do end describe 'Confidential Issues' do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:project_empty_repo, :public) } let(:assignee) { create(:assignee) } let(:author) { create(:user) } let(:non_member) { create(:user) } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 05e4233243d..8334545b759 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -183,9 +183,9 @@ describe Issue, models: true do describe '#related_branches' do it "selects the right branches" do allow(subject.project.repository).to receive(:branch_names). - and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) + and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) - expect(subject.related_branches).to eq [subject.to_branch_name] + expect(subject.related_branches).to eq([subject.to_branch_name]) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b8b9a455b83..624022c1dda 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -720,4 +720,45 @@ describe Project, models: true do expect(described_class.search_by_title('KITTENS')).to eq([project]) end end + + describe '#create_repository' do + let(:project) { create(:project) } + let(:shell) { Gitlab::Shell.new } + + before do + allow(project).to receive(:gitlab_shell).and_return(shell) + end + + context 'using a regular repository' do + it 'creates the repository' do + expect(shell).to receive(:add_repository). + with(project.path_with_namespace). + and_return(true) + + expect(project.repository).to receive(:after_create) + + expect(project.create_repository).to eq(true) + end + + it 'adds an error if the repository could not be created' do + expect(shell).to receive(:add_repository). + with(project.path_with_namespace). + and_return(false) + + expect(project.repository).not_to receive(:after_create) + + expect(project.create_repository).to eq(false) + expect(project.errors).not_to be_empty + end + end + + context 'using a forked repository' do + it 'does nothing' do + expect(project).to receive(:forked?).and_return(true) + expect(shell).not_to receive(:add_repository) + + project.create_repository + end + end + end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index a2085df5bcd..532e3f013fd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -244,6 +244,18 @@ describe ProjectWiki, models: true do end end + describe '#create_repo!' do + it 'creates a repository' do + expect(subject).to receive(:init_repo). + with(subject.path_with_namespace). + and_return(true) + + expect(subject.repository).to receive(:after_create) + + expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki) + end + end + private def create_temp_repo(path) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index a57229a4fdf..7eac70ae948 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -537,6 +537,12 @@ describe Repository, models: true do repository.before_delete end + + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.before_delete + end end describe 'when a repository exists' do @@ -593,6 +599,12 @@ describe Repository, models: true do repository.after_import end + + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.after_import + end end describe '#after_push_commit' do @@ -619,6 +631,14 @@ describe Repository, models: true do end end + describe '#after_create' do + it 'flushes the exists cache' do + expect(repository).to receive(:expire_exists_cache) + + repository.after_create + end + end + describe "#main_language" do it 'shows the main language of the project' do expect(repository.main_language).to eq("Ruby") @@ -781,6 +801,16 @@ describe Repository, models: true do end end + describe '#expire_exists_cache' do + let(:cache) { repository.send(:cache) } + + it 'expires the cache' do + expect(cache).to receive(:expire).with(:exists?) + + repository.expire_exists_cache + end + end + describe '#build_cache' do let(:cache) { repository.send(:cache) } diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 172537474ee..4ef05eb29d2 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -3,12 +3,17 @@ require 'spec_helper' describe RepositoryForkWorker do let(:project) { create(:project) } let(:fork_project) { create(:project, forked_from_project: project) } + let(:shell) { Gitlab::Shell.new } subject { RepositoryForkWorker.new } + before do + allow(subject).to receive(:gitlab_shell).and_return(shell) + end + describe "#perform" do it "creates a new repository from a fork" do - expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).with( + expect(shell).to receive(:fork_repository).with( project.path_with_namespace, fork_project.namespace.path ).and_return(true) @@ -19,20 +24,26 @@ describe RepositoryForkWorker do fork_project.namespace.path) end - it 'flushes the empty caches' do - expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository). + it 'flushes various caches' do + expect(shell).to receive(:fork_repository). with(project.path_with_namespace, fork_project.namespace.path). and_return(true) expect_any_instance_of(Repository).to receive(:expire_emptiness_caches). and_call_original + expect_any_instance_of(Repository).to receive(:expire_exists_cache). + and_call_original + subject.perform(project.id, project.path_with_namespace, fork_project.namespace.path) end it "handles bad fork" do - expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(false) + expect(shell).to receive(:fork_repository).and_return(false) + + expect(subject.logger).to receive(:error) + subject.perform( project.id, project.path_with_namespace,