From 541d89941014137762dff696c83b3357eba8efeb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 16:56:25 +0300 Subject: [PATCH] Project.repository should never be nil so you can call repository.exists? or repository.empty? Also specify separate project factory for project with filled repo --- app/helpers/application_helper.rb | 2 +- app/models/project.rb | 14 ++----- app/models/repository.rb | 10 +++++ app/views/projects/_clone_panel.html.haml | 2 +- features/steps/shared/project.rb | 4 +- lib/api/projects.rb | 2 +- lib/extracts_path.rb | 4 +- lib/gitlab/markdown.rb | 2 +- spec/factories.rb | 6 ++- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- spec/models/commit_spec.rb | 41 ++++++++++----------- spec/models/project_spec.rb | 10 ++--- spec/spec_helper.rb | 6 +-- 13 files changed, 51 insertions(+), 54 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f03039e4..cb9cb1a3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -96,7 +96,7 @@ module ApplicationHelper ] project_nav = [] - if @project && @project.repository && @project.repository.root_ref + if @project && @project.repository.exists? && @project.repository.root_ref project_nav = [ { label: "#{simple_sanitize(@project.name_with_namespace)} - Issues", url: project_issues_path(@project) }, { label: "#{simple_sanitize(@project.name_with_namespace)} - Commits", url: project_commits_path(@project, @ref || @project.repository.root_ref) }, diff --git a/app/models/project.rb b/app/models/project.rb index 934dd6b2..0263ffef 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -141,13 +141,7 @@ class Project < ActiveRecord::Base end def repository - if path - @repository ||= Repository.new(path_with_namespace, default_branch) - else - nil - end - rescue Gitlab::Git::NoRepository - nil + @repository ||= Repository.new(path_with_namespace, default_branch) end def saved? @@ -332,14 +326,14 @@ class Project < ActiveRecord::Base end def valid_repo? - repo + repository.exists? rescue errors.add(:path, "Invalid repository path") false end def empty_repo? - !repository || repository.empty? + !repository.exists? || repository.empty? end def ensure_satellite_exists @@ -363,7 +357,7 @@ class Project < ActiveRecord::Base end def repo_exists? - @repo_exists ||= (repository && repository.branches.present?) + @repo_exists ||= repository.exists? rescue @repo_exists = false end diff --git a/app/models/repository.rb b/app/models/repository.rb index 0a4431f1..ed600e29 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -3,6 +3,16 @@ class Repository def initialize(path_with_namespace, default_branch) @raw_repository = Gitlab::Git::Repository.new(path_with_namespace, default_branch) + rescue Gitlab::Git::Repository::NoRepository + nil + end + + def exists? + raw_repository + end + + def empty? + raw_repository.empty? end def commit(id = nil) diff --git a/app/views/projects/_clone_panel.html.haml b/app/views/projects/_clone_panel.html.haml index 9a2be429..91353147 100644 --- a/app/views/projects/_clone_panel.html.haml +++ b/app/views/projects/_clone_panel.html.haml @@ -4,7 +4,7 @@ .form-horizontal= render "shared/clone_panel" .span4.pull-right .pull-right - - unless @project.empty_repo? + - if @project.empty_repo? - if can? current_user, :download_code, @project = link_to archive_project_repository_path(@project), class: "btn-small btn grouped" do %i.icon-download-alt diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 81863a54..b16032a8 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -3,14 +3,14 @@ module SharedProject # Create a project without caring about what it's called And "I own a project" do - @project = create(:project) + @project = create(:project_with_code) @project.team << [@user, :master] end # Create a specific project called "Shop" And 'I own project "Shop"' do @project = Project.find_by_name "Shop" - @project ||= create(:project, name: "Shop") + @project ||= create(:project_with_code, name: "Shop") @project.team << [@user, :master] end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index d4f50fda..ce94c34b 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -372,7 +372,7 @@ module Gitlab ref = params[:ref_name] || user_project.try(:default_branch) || 'master' commits = user_project.repository.commits(ref, nil, per_page, page * per_page) - present CommitDecorator.decorate(commits), with: Entities::RepoCommit + present commits, with: Entities::RepoCommit end # Get a project snippets diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index d4871a49..2e3ab1b2 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -85,8 +85,8 @@ module ExtractsPath # - @id - A string representing the joined ref and path # - @ref - A string representing the ref (e.g., the branch, tag, or commit SHA) # - @path - A string representing the filesystem path - # - @commit - A CommitDecorator representing the commit from the given ref - # - @tree - A TreeDecorator representing the tree at the given ref/path + # - @commit - A Commit representing the commit from the given ref + # - @tree - A Tree representing the tree at the given ref/path # # If the :id parameter appears to be requesting a specific response format, # that will be handled as well. diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 762eb372..ad6ba3e8 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -187,7 +187,7 @@ module Gitlab def reference_commit(identifier) if @project.valid_repo? && commit = @project.repository.commit(identifier) - link_to(identifier, project_commit_url(@project, commit), html_options.merge(title: CommitDecorator.new(commit).link_title, class: "gfm gfm-commit #{html_options[:class]}")) + link_to(identifier, project_commit_url(@project, commit), html_options.merge(title: commit.link_title, class: "gfm gfm-commit #{html_options[:class]}")) end end end diff --git a/spec/factories.rb b/spec/factories.rb index 8f620cd7..d904e337 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -25,7 +25,7 @@ FactoryGirl.define do factory :project do sequence(:name) { |n| "project#{n}" } - path { 'gitlabhq' } + path { name.downcase.gsub(/\s/, '_') } creator end @@ -34,6 +34,10 @@ FactoryGirl.define do issues_tracker_id { "project_name_in_redmine" } end + factory :project_with_code, parent: :project do + path { 'gitlabhq' } + end + factory :group do sequence(:name) { |n| "group#{n}" } path { name.downcase.gsub(/\s/, '_') } diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 3abeaeef..234608c1 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -7,7 +7,7 @@ describe GitlabMarkdownHelper do let!(:project) { create(:project) } let(:user) { create(:user, username: 'gfm') } - let(:commit) { CommitDecorator.decorate(Commit.new(project.repository.commit)) } + let(:commit) { project.repository.commit) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, project: project) } let(:snippet) { create(:snippet, project: project) } diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7b063d2a..7713a33d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -3,35 +3,32 @@ require 'spec_helper' describe Commit do let(:commit) { create(:project).repository.commit } - describe CommitDecorator do - let(:decorator) { CommitDecorator.new(commit) } - describe '#title' do - it "returns no_commit_message when safe_message is blank" do - decorator.stub(:safe_message).and_return('') - decorator.title.should == "--no commit message" - end + describe '#title' do + it "returns no_commit_message when safe_message is blank" do + commit.stub(:safe_message).and_return('') + commit.title.should == "--no commit message" + end - it "truncates a message without a newline at 70 characters" do - message = commit.safe_message * 10 + it "truncates a message without a newline at 70 characters" do + message = commit.safe_message * 10 - decorator.stub(:safe_message).and_return(message) - decorator.title.should == "#{message[0..69]}…" - end + commit.stub(:safe_message).and_return(message) + commit.title.should == "#{message[0..69]}…" + end - it "truncates a message with a newline before 80 characters at the newline" do - message = commit.safe_message.split(" ").first + it "truncates a message with a newline before 80 characters at the newline" do + message = commit.safe_message.split(" ").first - decorator.stub(:safe_message).and_return(message + "\n" + message) - decorator.title.should == message - end + commit.stub(:safe_message).and_return(message + "\n" + message) + commit.title.should == message + end - it "truncates a message with a newline after 80 characters at 70 characters" do - message = (commit.safe_message * 10) + "\n" + it "truncates a message with a newline after 80 characters at 70 characters" do + message = (commit.safe_message * 10) + "\n" - decorator.stub(:safe_message).and_return(message) - decorator.title.should == "#{message[0..69]}…" - end + commit.stub(:safe_message).and_return(message) + commit.title.should == "#{message[0..69]}…" end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 90b5e08e..cbc7f278 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -119,7 +119,7 @@ describe Project do end describe :update_merge_requests do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } before do @merge_request = create(:merge_request, project: project) @@ -187,11 +187,7 @@ describe Project do let(:project) { create(:project) } it "should return valid repo" do - project.repository.should be_kind_of(Gitlab::Git::Repository) - end - - it "should return nil" do - Project.new(path: "empty").repository.should be_nil + project.repository.should be_kind_of(Repository) end end @@ -249,7 +245,7 @@ describe Project do end describe :open_branches do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } before do project.protected_branches.create(name: 'master') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 03c586f8..8a01c930 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -47,11 +47,7 @@ Spork.prefork do config.use_transactional_fixtures = false config.before do - # Use tmp dir for FS manipulations - temp_repos_path = Rails.root.join('tmp', 'test-git-base-path') - Gitlab.config.gitlab_shell.stub(repos_path: temp_repos_path) - FileUtils.rm_rf temp_repos_path - FileUtils.mkdir_p temp_repos_path + TestEnv.init end end end