From 676bce2ab941fcc50ef5796ff77229e238eb9b35 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 18:49:06 +0300 Subject: [PATCH 01/25] Move Commit and Repository logic to lib/gitlab/git --- lib/gitlab/git/commit.rb | 179 ++++++++++++++++++++++++++++++++++ lib/gitlab/git/repository.rb | 181 +++++++++++++++++++++++++++++++++++ 2 files changed, 360 insertions(+) create mode 100644 lib/gitlab/git/commit.rb create mode 100644 lib/gitlab/git/repository.rb diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb new file mode 100644 index 00000000..86e0dfbb --- /dev/null +++ b/lib/gitlab/git/commit.rb @@ -0,0 +1,179 @@ +# Gitlab::Git::Gitlab::Git::Commit is a wrapper around native Grit::Commit object +# We dont want to use grit objects inside app/ +# It helps us easily migrate to rugged in future +module Gitlab + module Git + class Gitlab::Git::Commit + attr_accessor :raw_commit, :head, :refs + + delegate :message, :authored_date, :committed_date, :parents, :sha, + :date, :committer, :author, :diffs, :tree, :id, :stats, + :to_patch, to: :raw_commit + + class << self + def find_or_first(repo, commit_id = nil, root_ref) + commit = if commit_id + repo.commit(commit_id) + else + repo.commits(root_ref).first + end + + Gitlab::Git::Commit.new(commit) if commit + end + + def fresh_commits(repo, n = 10) + commits = repo.heads.map do |h| + repo.commits(h.name, n).map { |c| Gitlab::Git::Commit.new(c, h) } + end.flatten.uniq { |c| c.id } + + commits.sort! do |x, y| + y.committed_date <=> x.committed_date + end + + commits[0...n] + end + + def commits_with_refs(repo, n = 20) + commits = repo.branches.map { |ref| Gitlab::Git::Commit.new(ref.commit, ref) } + + commits.sort! do |x, y| + y.committed_date <=> x.committed_date + end + + commits[0..n] + end + + def commits_since(repo, date) + commits = repo.heads.map do |h| + repo.log(h.name, nil, since: date).each { |c| Gitlab::Git::Commit.new(c, h) } + end.flatten.uniq { |c| c.id } + + commits.sort! do |x, y| + y.committed_date <=> x.committed_date + end + + commits + end + + def commits(repo, ref, path = nil, limit = nil, offset = nil) + if path + repo.log(ref, path, max_count: limit, skip: offset) + elsif limit && offset + repo.commits(ref, limit, offset) + else + repo.commits(ref) + end.map{ |c| Gitlab::Git::Commit.new(c) } + end + + def commits_between(repo, from, to) + repo.commits_between(from, to).map { |c| Gitlab::Git::Commit.new(c) } + end + + def compare(project, from, to) + result = { + commits: [], + diffs: [], + commit: nil, + same: false + } + + return result unless from && to + + first = project.repository.commit(to.try(:strip)) + last = project.repository.commit(from.try(:strip)) + + if first && last + result[:same] = (first.id == last.id) + result[:commits] = project.repo.commits_between(last.id, first.id).map {|c| Gitlab::Git::Commit.new(c)} + + # Dont load diff for 100+ commits + result[:diffs] = if result[:commits].size > 100 + [] + else + project.repo.diff(last.id, first.id) rescue [] + end + + result[:commit] = Gitlab::Git::Commit.new(first) + end + + result + end + end + + def initialize(raw_commit, head = nil) + raise "Nil as raw commit passed" unless raw_commit + + @raw_commit = raw_commit + @head = head + end + + def short_id(length = 10) + id.to_s[0..length] + end + + def safe_message + @safe_message ||= message + end + + def created_at + committed_date + end + + def author_email + author.email + end + + def author_name + author.name + end + + # Was this commit committed by a different person than the original author? + def different_committer? + author_name != committer_name || author_email != committer_email + end + + def committer_name + committer.name + end + + def committer_email + committer.email + end + + def prev_commit + @prev_commit ||= if parents.present? + Gitlab::Git::Commit.new(parents.first) + else + nil + end + end + + def prev_commit_id + prev_commit.try :id + end + + # Shows the diff between the commit's parent and the commit. + # + # Cuts out the header and stats from #to_patch and returns only the diff. + def to_diff + # see Grit::Gitlab::Git::Commit#show + patch = to_patch + + # discard lines before the diff + lines = patch.split("\n") + while !lines.first.start_with?("diff --git") do + lines.shift + end + lines.pop if lines.last =~ /^[\d.]+$/ # Git version + lines.pop if lines.last == "-- " # end of diff + lines.join("\n") + end + + def has_zero_stats? + stats.total.zero? + rescue + true + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb new file mode 100644 index 00000000..53d9c735 --- /dev/null +++ b/lib/gitlab/git/repository.rb @@ -0,0 +1,181 @@ +# Gitlab::Git::Gitlab::Git::Commit is a wrapper around native Grit::Repository object +# We dont want to use grit objects inside app/ +# It helps us easily migrate to rugged in future +module Gitlab + module Git + class Repository + include Gitlab::Popen + + class NoRepository < StandardError; end + + # Repository directory name with namespace direcotry + # Examples: + # gitlab/gitolite + # diaspora + # + attr_accessor :path_with_namespace + + # Grit repo object + attr_accessor :repo + + # Default branch in the repository + attr_accessor :root_ref + + def initialize(path_with_namespace, root_ref = 'master') + @root_ref = root_ref || "master" + @path_with_namespace = path_with_namespace + + # Init grit repo object + repo + end + + def raw + repo + end + + def path_to_repo + @path_to_repo ||= File.join(Gitlab.config.gitlab_shell.repos_path, "#{path_with_namespace}.git") + end + + def repo + @repo ||= Grit::Repo.new(path_to_repo) + rescue Grit::NoSuchPathError + raise NoRepository.new('no repository for such path') + end + + def commit(commit_id = nil) + Gitlab::Git::Commit.find_or_first(repo, commit_id, root_ref) + end + + def fresh_commits(n = 10) + Gitlab::Git::Commit.fresh_commits(repo, n) + end + + def commits_with_refs(n = 20) + Gitlab::Git::Commit.commits_with_refs(repo, n) + end + + def commits_since(date) + Gitlab::Git::Commit.commits_since(repo, date) + end + + def commits(ref, path = nil, limit = nil, offset = nil) + Gitlab::Git::Commit.commits(repo, ref, path, limit, offset) + end + + def last_commit_for(ref, path = nil) + commits(ref, path, 1).first + end + + def commits_between(from, to) + Gitlab::Git::Commit.commits_between(repo, from, to) + end + + # Returns an Array of branch names + # sorted by name ASC + def branch_names + branches.map(&:name) + end + + # Returns an Array of Branches + def branches + repo.branches.sort_by(&:name) + end + + # Returns an Array of tag names + def tag_names + repo.tags.collect(&:name).sort.reverse + end + + # Returns an Array of Tags + def tags + repo.tags.sort_by(&:name).reverse + end + + # Returns an Array of branch and tag names + def ref_names + [branch_names + tag_names].flatten + end + + def heads + @heads ||= repo.heads + end + + def tree(fcommit, path = nil) + fcommit = commit if fcommit == :head + tree = fcommit.tree + path ? (tree / path) : tree + end + + def has_commits? + !!commit + rescue Grit::NoSuchPathError + false + end + + def empty? + !has_commits? + end + + # Discovers the default branch based on the repository's available branches + # + # - If no branches are present, returns nil + # - If one branch is present, returns its name + # - If two or more branches are present, returns the one that has a name + # matching root_ref (default_branch or 'master' if default_branch is nil) + def discover_default_branch + if branch_names.length == 0 + nil + elsif branch_names.length == 1 + branch_names.first + else + branch_names.select { |v| v == root_ref }.first + end + end + + # Archive Project to .tar.gz + # + # Already packed repo archives stored at + # app_root/tmp/repositories/project_name/project_name-commit-id.tag.gz + # + def archive_repo(ref) + ref = ref || self.root_ref + commit = self.commit(ref) + return nil unless commit + + # Build file path + file_name = self.path_with_namespace.gsub("/","_") + "-" + commit.id.to_s + ".tar.gz" + storage_path = Rails.root.join("tmp", "repositories") + file_path = File.join(storage_path, self.path_with_namespace, file_name) + + # Put files into a directory before archiving + prefix = File.basename(self.path_with_namespace) + "/" + + # Create file if not exists + unless File.exists?(file_path) + FileUtils.mkdir_p File.dirname(file_path) + file = self.repo.archive_to_file(ref, prefix, file_path) + end + + file_path + end + + # Return repo size in megabytes + # Cached in redis + def size + Rails.cache.fetch(cache_key(:size)) do + size = popen('du -s', path_to_repo).first.strip.to_i + (size.to_f / 1024).round(2) + end + end + + def expire_cache + Rails.cache.delete(cache_key(:size)) + end + + def cache_key(type) + "#{type}:#{path_with_namespace}" + end + end + end +end From d444a23ad6d9c2455a0159435d0c63342aca5edb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 18:49:31 +0300 Subject: [PATCH 02/25] specs for Gitlab::Git::Repository and Gitlab::Git::Commit --- spec/lib/git/commit_spec.rb | 50 +++++++++++++++ spec/lib/git/repository_spec.rb | 105 ++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 spec/lib/git/commit_spec.rb create mode 100644 spec/lib/git/repository_spec.rb diff --git a/spec/lib/git/commit_spec.rb b/spec/lib/git/commit_spec.rb new file mode 100644 index 00000000..93f579d3 --- /dev/null +++ b/spec/lib/git/commit_spec.rb @@ -0,0 +1,50 @@ +require "spec_helper" + +describe Gitlab::Git::Commit do + let(:commit) { create(:project).repository.commit } + + describe "Commit info" do + before do + @committer = double( + email: 'mike@smith.com', + name: 'Mike Smith' + ) + + @author = double( + email: 'john@smith.com', + name: 'John Smith' + ) + + @raw_commit = double( + id: "bcf03b5de6abcf03b5de6c", + author: @author, + committer: @committer, + committed_date: Date.yesterday, + message: 'Refactoring specs' + ) + + @commit = Gitlab::Git::Commit.new(@raw_commit) + end + + it { @commit.short_id.should == "bcf03b5de6a" } + it { @commit.safe_message.should == @raw_commit.message } + it { @commit.created_at.should == @raw_commit.committed_date } + it { @commit.author_email.should == @author.email } + it { @commit.author_name.should == @author.name } + it { @commit.committer_name.should == @committer.name } + it { @commit.committer_email.should == @committer.email } + it { @commit.different_committer?.should be_true } + end + + describe "Class methods" do + subject { Gitlab::Git::Commit } + + it { should respond_to(:find_or_first) } + it { should respond_to(:fresh_commits) } + it { should respond_to(:commits_with_refs) } + it { should respond_to(:commits_since) } + it { should respond_to(:commits_between) } + it { should respond_to(:commits) } + it { should respond_to(:compare) } + end +end diff --git a/spec/lib/git/repository_spec.rb b/spec/lib/git/repository_spec.rb new file mode 100644 index 00000000..e0ff93ea --- /dev/null +++ b/spec/lib/git/repository_spec.rb @@ -0,0 +1,105 @@ +require "spec_helper" + +describe Gitlab::Git::Repository do + let(:project) { create(:project) } + let(:repository) { project.repository } + + describe "Respond to" do + subject { repository } + + it { should respond_to(:repo) } + it { should respond_to(:tree) } + it { should respond_to(:root_ref) } + it { should respond_to(:tags) } + it { should respond_to(:commit) } + it { should respond_to(:commits) } + it { should respond_to(:commits_between) } + it { should respond_to(:commits_with_refs) } + it { should respond_to(:commits_since) } + it { should respond_to(:commits_between) } + end + + + describe "#discover_default_branch" do + let(:master) { 'master' } + let(:stable) { 'stable' } + + it "returns 'master' when master exists" do + repository.should_receive(:branch_names).at_least(:once).and_return([stable, master]) + repository.discover_default_branch.should == 'master' + end + + it "returns non-master when master exists but default branch is set to something else" do + repository.root_ref = 'stable' + repository.should_receive(:branch_names).at_least(:once).and_return([stable, master]) + repository.discover_default_branch.should == 'stable' + end + + it "returns a non-master branch when only one exists" do + repository.should_receive(:branch_names).at_least(:once).and_return([stable]) + repository.discover_default_branch.should == 'stable' + end + + it "returns nil when no branch exists" do + repository.should_receive(:branch_names).at_least(:once).and_return([]) + repository.discover_default_branch.should be_nil + end + end + + describe :commit do + it "should return first head commit if without params" do + repository.commit.id.should == repository.repo.commits.first.id + end + + it "should return valid commit" do + repository.commit(ValidCommit::ID).should be_valid_commit + end + + it "should return nil" do + repository.commit("+123_4532530XYZ").should be_nil + end + end + + describe :tree do + before do + @commit = repository.commit(ValidCommit::ID) + end + + it "should raise error w/o arguments" do + lambda { repository.tree }.should raise_error + end + + it "should return root tree for commit" do + tree = repository.tree(@commit) + tree.contents.size.should == ValidCommit::FILES_COUNT + tree.contents.map(&:name).should == ValidCommit::FILES + end + + it "should return root tree for commit with correct path" do + tree = repository.tree(@commit, ValidCommit::C_FILE_PATH) + tree.contents.map(&:name).should == ValidCommit::C_FILES + end + + it "should return root tree for commit with incorrect path" do + repository.tree(@commit, "invalid_path").should be_nil + end + end + + describe "fresh commits" do + it { repository.fresh_commits(3).count.should == 3 } + it { repository.fresh_commits.first.id.should == "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" } + it { repository.fresh_commits.last.id.should == "f403da73f5e62794a0447aca879360494b08f678" } + end + + describe "commits_between" do + subject do + commits = repository.commits_between("3a4b4fb4cde7809f033822a171b9feae19d41fff", + "8470d70da67355c9c009e4401746b1d5410af2e3") + commits.map { |c| c.id } + end + + it { should have(3).elements } + it { should include("f0f14c8eaba69ebddd766498a9d0b0e79becd633") } + it { should_not include("bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a") } + end +end From 154e54b46e01615bc13f5e3ac2d0f07f7a27464d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 18:49:58 +0300 Subject: [PATCH 03/25] Remove grit logic from app/ --- app/models/commit.rb | 169 ++--------------------------------- app/models/gollum_wiki.rb | 2 +- app/models/merge_request.rb | 2 +- app/models/network/commit.rb | 2 +- app/models/note.rb | 2 +- app/models/project.rb | 4 +- 6 files changed, 11 insertions(+), 170 deletions(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index 4d0c57b3..0164ae66 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -8,174 +8,15 @@ class Commit # DIFF_SAFE_SIZE = 100 - attr_accessor :commit, :head, :refs + attr_accessor :raw - delegate :message, :authored_date, :committed_date, :parents, :sha, - :date, :committer, :author, :diffs, :tree, :id, :stats, - :to_patch, to: :commit - - class << self - def find_or_first(repo, commit_id = nil, root_ref) - commit = if commit_id - repo.commit(commit_id) - else - repo.commits(root_ref).first - end - - Commit.new(commit) if commit - end - - def fresh_commits(repo, n = 10) - commits = repo.heads.map do |h| - repo.commits(h.name, n).map { |c| Commit.new(c, h) } - end.flatten.uniq { |c| c.id } - - commits.sort! do |x, y| - y.committed_date <=> x.committed_date - end - - commits[0...n] - end - - def commits_with_refs(repo, n = 20) - commits = repo.branches.map { |ref| Commit.new(ref.commit, ref) } - - commits.sort! do |x, y| - y.committed_date <=> x.committed_date - end - - commits[0..n] - end - - def commits_since(repo, date) - commits = repo.heads.map do |h| - repo.log(h.name, nil, since: date).each { |c| Commit.new(c, h) } - end.flatten.uniq { |c| c.id } - - commits.sort! do |x, y| - y.committed_date <=> x.committed_date - end - - commits - end - - def commits(repo, ref, path = nil, limit = nil, offset = nil) - if path - repo.log(ref, path, max_count: limit, skip: offset) - elsif limit && offset - repo.commits(ref, limit, offset) - else - repo.commits(ref) - end.map{ |c| Commit.new(c) } - end - - def commits_between(repo, from, to) - repo.commits_between(from, to).map { |c| Commit.new(c) } - end - - def compare(project, from, to) - result = { - commits: [], - diffs: [], - commit: nil, - same: false - } - - return result unless from && to - - first = project.repository.commit(to.try(:strip)) - last = project.repository.commit(from.try(:strip)) - - if first && last - result[:same] = (first.id == last.id) - result[:commits] = project.repo.commits_between(last.id, first.id).map {|c| Commit.new(c)} - - # Dont load diff for 100+ commits - result[:diffs] = if result[:commits].size > 100 - [] - else - project.repo.diff(last.id, first.id) rescue [] - end - - result[:commit] = Commit.new(first) - end - - result - end - end - - def initialize(raw_commit, head = nil) + def initialize(raw_commit) raise "Nil as raw commit passed" unless raw_commit - @commit = raw_commit - @head = head + @raw = raw_commit end - def short_id(length = 10) - id.to_s[0..length] - end - - def safe_message - @safe_message ||= message - end - - def created_at - committed_date - end - - def author_email - author.email - end - - def author_name - author.name - end - - # Was this commit committed by a different person than the original author? - def different_committer? - author_name != committer_name || author_email != committer_email - end - - def committer_name - committer.name - end - - def committer_email - committer.email - end - - def prev_commit - @prev_commit ||= if parents.present? - Commit.new(parents.first) - else - nil - end - end - - def prev_commit_id - prev_commit.try :id - end - - # Shows the diff between the commit's parent and the commit. - # - # Cuts out the header and stats from #to_patch and returns only the diff. - def to_diff - # see Grit::Commit#show - patch = to_patch - - # discard lines before the diff - lines = patch.split("\n") - while !lines.first.start_with?("diff --git") do - lines.shift - end - lines.pop if lines.last =~ /^[\d.]+$/ # Git version - lines.pop if lines.last == "-- " # end of diff - lines.join("\n") - end - - def has_zero_stats? - stats.total.zero? - rescue - true + def method_missing(m, *args, &block) + @raw.send(m, *args, &block) end end diff --git a/app/models/gollum_wiki.rb b/app/models/gollum_wiki.rb index a1ee3a08..cdfcd567 100644 --- a/app/models/gollum_wiki.rb +++ b/app/models/gollum_wiki.rb @@ -50,7 +50,7 @@ class GollumWiki # Returns the last 30 Commit objects across the entire # repository. def recent_history - Commit.fresh_commits(wiki.repo, 30) + Gitlab::Git::Commit.fresh_commits(wiki.repo, 30) end # Finds a page within the repository based on a tile diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9d42b1e1..505f6637 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -169,7 +169,7 @@ class MergeRequest < ActiveRecord::Base end def unmerged_commits - self.project.repo. + self.project.repository. commits_between(self.target_branch, self.source_branch). map {|c| Commit.new(c)}. sort_by(&:created_at). diff --git a/app/models/network/commit.rb b/app/models/network/commit.rb index d0bc61c3..3cd0c015 100644 --- a/app/models/network/commit.rb +++ b/app/models/network/commit.rb @@ -8,7 +8,7 @@ module Network attr_accessor :time, :spaces, :parent_spaces def initialize(raw_commit, refs) - @commit = ::Commit.new(raw_commit) + @commit = Gitlab::Git::Commit.new(raw_commit) @time = -1 @spaces = [] @parent_spaces = [] diff --git a/app/models/note.rb b/app/models/note.rb index f26420ca..17ceb541 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -130,7 +130,7 @@ class Note < ActiveRecord::Base # override to return commits, which are not active record def noteable if for_commit? - project.repository.commit(commit_id) + Commit.new(project.repository.commit(commit_id)) else super end diff --git a/app/models/project.rb b/app/models/project.rb index 6871afca..54abbc3e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -142,11 +142,11 @@ class Project < ActiveRecord::Base def repository if path - @repository ||= Repository.new(path_with_namespace, default_branch) + @repository ||= Gitlab::Git::Repository.new(path_with_namespace, default_branch) else nil end - rescue Grit::NoSuchPathError + rescue Gitlab::Git::NoRepository nil end From 9dc644635f99dffa26555eeb1c92f988221ccb6a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 18:50:17 +0300 Subject: [PATCH 04/25] Fix tests and remove app/models/repository.rb --- app/models/repository.rb | 170 ----------------------------- lib/extracts_path.rb | 4 +- spec/models/commit_spec.rb | 45 -------- spec/models/project_spec.rb | 2 +- spec/models/repository_spec.rb | 105 ------------------ spec/support/stubbed_repository.rb | 4 +- 6 files changed, 5 insertions(+), 325 deletions(-) delete mode 100644 app/models/repository.rb delete mode 100644 spec/models/repository_spec.rb diff --git a/app/models/repository.rb b/app/models/repository.rb deleted file mode 100644 index 7f56047b..00000000 --- a/app/models/repository.rb +++ /dev/null @@ -1,170 +0,0 @@ -class Repository - include Gitlab::Popen - - # Repository directory name with namespace direcotry - # Examples: - # gitlab/gitolite - # diaspora - # - attr_accessor :path_with_namespace - - # Grit repo object - attr_accessor :repo - - # Default branch in the repository - attr_accessor :root_ref - - def initialize(path_with_namespace, root_ref = 'master') - @root_ref = root_ref || "master" - @path_with_namespace = path_with_namespace - - # Init grit repo object - repo - end - - def raw - repo - end - - def path_to_repo - @path_to_repo ||= File.join(Gitlab.config.gitlab_shell.repos_path, "#{path_with_namespace}.git") - end - - def repo - @repo ||= Grit::Repo.new(path_to_repo) - end - - def commit(commit_id = nil) - Commit.find_or_first(repo, commit_id, root_ref) - end - - def fresh_commits(n = 10) - Commit.fresh_commits(repo, n) - end - - def commits_with_refs(n = 20) - Commit.commits_with_refs(repo, n) - end - - def commits_since(date) - Commit.commits_since(repo, date) - end - - def commits(ref, path = nil, limit = nil, offset = nil) - Commit.commits(repo, ref, path, limit, offset) - end - - def last_commit_for(ref, path = nil) - commits(ref, path, 1).first - end - - def commits_between(from, to) - Commit.commits_between(repo, from, to) - end - - # Returns an Array of branch names - # sorted by name ASC - def branch_names - branches.map(&:name) - end - - # Returns an Array of Branches - def branches - repo.branches.sort_by(&:name) - end - - # Returns an Array of tag names - def tag_names - repo.tags.collect(&:name).sort.reverse - end - - # Returns an Array of Tags - def tags - repo.tags.sort_by(&:name).reverse - end - - # Returns an Array of branch and tag names - def ref_names - [branch_names + tag_names].flatten - end - - def heads - @heads ||= repo.heads - end - - def tree(fcommit, path = nil) - fcommit = commit if fcommit == :head - tree = fcommit.tree - path ? (tree / path) : tree - end - - def has_commits? - !!commit - rescue Grit::NoSuchPathError - false - end - - def empty? - !has_commits? - end - - # Discovers the default branch based on the repository's available branches - # - # - If no branches are present, returns nil - # - If one branch is present, returns its name - # - If two or more branches are present, returns the one that has a name - # matching root_ref (default_branch or 'master' if default_branch is nil) - def discover_default_branch - if branch_names.length == 0 - nil - elsif branch_names.length == 1 - branch_names.first - else - branch_names.select { |v| v == root_ref }.first - end - end - - # Archive Project to .tar.gz - # - # Already packed repo archives stored at - # app_root/tmp/repositories/project_name/project_name-commit-id.tag.gz - # - def archive_repo(ref) - ref = ref || self.root_ref - commit = self.commit(ref) - return nil unless commit - - # Build file path - file_name = self.path_with_namespace.gsub("/","_") + "-" + commit.id.to_s + ".tar.gz" - storage_path = Rails.root.join("tmp", "repositories") - file_path = File.join(storage_path, self.path_with_namespace, file_name) - - # Put files into a directory before archiving - prefix = File.basename(self.path_with_namespace) + "/" - - # Create file if not exists - unless File.exists?(file_path) - FileUtils.mkdir_p File.dirname(file_path) - file = self.repo.archive_to_file(ref, prefix, file_path) - end - - file_path - end - - # Return repo size in megabytes - # Cached in redis - def size - Rails.cache.fetch(cache_key(:size)) do - size = popen('du -s', path_to_repo).first.strip.to_i - (size.to_f / 1024).round(2) - end - end - - def expire_cache - Rails.cache.delete(cache_key(:size)) - end - - def cache_key(type) - "#{type}:#{path_with_namespace}" - end -end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 351fc2f2..4ad485c5 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -100,8 +100,8 @@ module ExtractsPath # It is used "@project.repository.commits(@ref, @path, 1, 0)", # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. - commits = @project.repository.commits(@ref, @path, 1, 0) - @commit = CommitDecorator.decorate(commits.first) + @commit = @project.repository.commits(@ref, @path, 1, 0).first + @commit = CommitDecorator.decorate(@commit) @tree = Tree.new(@commit.tree, @ref, @path) @tree = TreeDecorator.new(@tree) diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 91301029..7b063d2a 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -35,51 +35,6 @@ describe Commit do end end - describe "Commit info" do - before do - @committer = double( - email: 'mike@smith.com', - name: 'Mike Smith' - ) - - @author = double( - email: 'john@smith.com', - name: 'John Smith' - ) - - @raw_commit = double( - id: "bcf03b5de6abcf03b5de6c", - author: @author, - committer: @committer, - committed_date: Date.yesterday, - message: 'Refactoring specs' - ) - - @commit = Commit.new(@raw_commit) - end - - it { @commit.short_id.should == "bcf03b5de6a" } - it { @commit.safe_message.should == @raw_commit.message } - it { @commit.created_at.should == @raw_commit.committed_date } - it { @commit.author_email.should == @author.email } - it { @commit.author_name.should == @author.name } - it { @commit.committer_name.should == @committer.name } - it { @commit.committer_email.should == @committer.email } - it { @commit.different_committer?.should be_true } - end - - describe "Class methods" do - subject { Commit } - - it { should respond_to(:find_or_first) } - it { should respond_to(:fresh_commits) } - it { should respond_to(:commits_with_refs) } - it { should respond_to(:commits_since) } - it { should respond_to(:commits_between) } - it { should respond_to(:commits) } - it { should respond_to(:compare) } - end - describe "delegation" do subject { commit } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 53388b93..90b5e08e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -187,7 +187,7 @@ describe Project do let(:project) { create(:project) } it "should return valid repo" do - project.repository.should be_kind_of(Repository) + project.repository.should be_kind_of(Gitlab::Git::Repository) end it "should return nil" do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb deleted file mode 100644 index 71f9b964..00000000 --- a/spec/models/repository_spec.rb +++ /dev/null @@ -1,105 +0,0 @@ -require "spec_helper" - -describe Repository do - let(:project) { create(:project) } - let(:repository) { project.repository } - - describe "Respond to" do - subject { repository } - - it { should respond_to(:repo) } - it { should respond_to(:tree) } - it { should respond_to(:root_ref) } - it { should respond_to(:tags) } - it { should respond_to(:commit) } - it { should respond_to(:commits) } - it { should respond_to(:commits_between) } - it { should respond_to(:commits_with_refs) } - it { should respond_to(:commits_since) } - it { should respond_to(:commits_between) } - end - - - describe "#discover_default_branch" do - let(:master) { 'master' } - let(:stable) { 'stable' } - - it "returns 'master' when master exists" do - repository.should_receive(:branch_names).at_least(:once).and_return([stable, master]) - repository.discover_default_branch.should == 'master' - end - - it "returns non-master when master exists but default branch is set to something else" do - repository.root_ref = 'stable' - repository.should_receive(:branch_names).at_least(:once).and_return([stable, master]) - repository.discover_default_branch.should == 'stable' - end - - it "returns a non-master branch when only one exists" do - repository.should_receive(:branch_names).at_least(:once).and_return([stable]) - repository.discover_default_branch.should == 'stable' - end - - it "returns nil when no branch exists" do - repository.should_receive(:branch_names).at_least(:once).and_return([]) - repository.discover_default_branch.should be_nil - end - end - - describe :commit do - it "should return first head commit if without params" do - repository.commit.id.should == repository.repo.commits.first.id - end - - it "should return valid commit" do - repository.commit(ValidCommit::ID).should be_valid_commit - end - - it "should return nil" do - repository.commit("+123_4532530XYZ").should be_nil - end - end - - describe :tree do - before do - @commit = repository.commit(ValidCommit::ID) - end - - it "should raise error w/o arguments" do - lambda { repository.tree }.should raise_error - end - - it "should return root tree for commit" do - tree = repository.tree(@commit) - tree.contents.size.should == ValidCommit::FILES_COUNT - tree.contents.map(&:name).should == ValidCommit::FILES - end - - it "should return root tree for commit with correct path" do - tree = repository.tree(@commit, ValidCommit::C_FILE_PATH) - tree.contents.map(&:name).should == ValidCommit::C_FILES - end - - it "should return root tree for commit with incorrect path" do - repository.tree(@commit, "invalid_path").should be_nil - end - end - - describe "fresh commits" do - it { repository.fresh_commits(3).count.should == 3 } - it { repository.fresh_commits.first.id.should == "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" } - it { repository.fresh_commits.last.id.should == "f403da73f5e62794a0447aca879360494b08f678" } - end - - describe "commits_between" do - subject do - commits = repository.commits_between("3a4b4fb4cde7809f033822a171b9feae19d41fff", - "8470d70da67355c9c009e4401746b1d5410af2e3") - commits.map { |c| c.id } - end - - it { should have(3).elements } - it { should include("f0f14c8eaba69ebddd766498a9d0b0e79becd633") } - it { should_not include("bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a") } - end -end diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 6376c8d5..68c380e6 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -1,4 +1,4 @@ -require "repository" +require "gitlab/git/repository" require "project" require "merge_request" require "shell" @@ -39,7 +39,7 @@ class MergeRequest end end -class GitLabTestRepo < Repository +class GitLabTestRepo < Gitlab::Git::Repository def repo @repo ||= Grit::Repo.new(Rails.root.join('tmp', 'repositories', 'gitlabhq')) end From 26323046fda07b2353ee427afcd0253cea935047 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 19:00:45 +0300 Subject: [PATCH 05/25] Decorate Gitlab::Git::Commit with Commit --- app/contexts/commit_load_context.rb | 1 + app/controllers/commits_controller.rb | 1 + app/models/commit.rb | 8 ++++++++ lib/gitlab/git/commit.rb | 4 ++-- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/contexts/commit_load_context.rb b/app/contexts/commit_load_context.rb index 1f23f633..c8d77d9b 100644 --- a/app/contexts/commit_load_context.rb +++ b/app/contexts/commit_load_context.rb @@ -12,6 +12,7 @@ class CommitLoadContext < BaseContext commit = project.repository.commit(params[:id]) if commit + commit = Commit.new(commit) commit = CommitDecorator.decorate(commit) line_notes = project.notes.for_commit_id(commit.id).inline diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index 9dc0d968..fded5f0a 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -13,6 +13,7 @@ class CommitsController < ProjectResourceController @limit, @offset = (params[:limit] || 40), (params[:offset] || 0) @commits = @repo.commits(@ref, @path, @limit, @offset) + @commits = Commit.decorate(@commits) @commits = CommitDecorator.decorate_collection(@commits) respond_to do |format| diff --git a/app/models/commit.rb b/app/models/commit.rb index 0164ae66..ea5b451b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -10,12 +10,20 @@ class Commit attr_accessor :raw + def self.decorate(commits) + commits.map { |c| Commit.new(c) } + end + def initialize(raw_commit) raise "Nil as raw commit passed" unless raw_commit @raw = raw_commit end + def id + @raw.id + end + def method_missing(m, *args, &block) @raw.send(m, *args, &block) end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 86e0dfbb..023672f9 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -7,8 +7,8 @@ module Gitlab attr_accessor :raw_commit, :head, :refs delegate :message, :authored_date, :committed_date, :parents, :sha, - :date, :committer, :author, :diffs, :tree, :id, :stats, - :to_patch, to: :raw_commit + :date, :committer, :author, :diffs, :tree, :id, :stats, :to_patch, + to: :raw_commit class << self def find_or_first(repo, commit_id = nil, root_ref) diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index b9025026..3abeaeef 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(project.repository.commit) } + let(:commit) { CommitDecorator.decorate(Commit.new(project.repository.commit)) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, project: project) } let(:snippet) { create(:snippet, project: project) } From 1ea385625b481e405fbbffc2b2465553b20d90de Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:44:59 +0300 Subject: [PATCH 06/25] Remove Commit & Tree decorators --- app/decorators/commit_decorator.rb | 93 ------------------------------ app/decorators/tree_decorator.rb | 33 ----------- 2 files changed, 126 deletions(-) delete mode 100644 app/decorators/commit_decorator.rb delete mode 100644 app/decorators/tree_decorator.rb diff --git a/app/decorators/commit_decorator.rb b/app/decorators/commit_decorator.rb deleted file mode 100644 index 0337d8d4..00000000 --- a/app/decorators/commit_decorator.rb +++ /dev/null @@ -1,93 +0,0 @@ -class CommitDecorator < ApplicationDecorator - decorates :commit - - # Returns a string describing the commit for use in a link title - # - # Example - # - # "Commit: Alex Denisov - Project git clone panel" - def link_title - "Commit: #{author_name} - #{title}" - end - - # Returns the commits title. - # - # Usually, the commit title is the first line of the commit message. - # In case this first line is longer than 80 characters, it is cut off - # after 70 characters and ellipses (`&hellp;`) are appended. - def title - title = safe_message - - return no_commit_message if title.blank? - - title_end = title.index(/\n/) - if (!title_end && title.length > 80) || (title_end && title_end > 80) - title[0..69] << "…".html_safe - else - title.split(/\n/, 2).first - end - end - - # Returns the commits description - # - # cut off, ellipses (`&hellp;`) are prepended to the commit message. - def description - description = safe_message - - title_end = description.index(/\n/) - if (!title_end && description.length > 80) || (title_end && title_end > 80) - "…".html_safe << description[70..-1] - else - description.split(/\n/, 2)[1].try(:chomp) - end - end - - # Returns a link to the commit author. If the author has a matching user and - # is a member of the current @project it will link to the team member page. - # Otherwise it will link to the author email as specified in the commit. - # - # options: - # avatar: true will prepend the avatar image - # size: size of the avatar image in px - def author_link(options = {}) - person_link(options.merge source: :author) - end - - # Just like #author_link but for the committer. - def committer_link(options = {}) - person_link(options.merge source: :committer) - end - - protected - - def no_commit_message - "--no commit message" - end - - # Private: Returns a link to a person. If the person has a matching user and - # is a member of the current @project it will link to the team member page. - # Otherwise it will link to the person email as specified in the commit. - # - # options: - # source: one of :author or :committer - # avatar: true will prepend the avatar image - # size: size of the avatar image in px - def person_link(options = {}) - source_name = send "#{options[:source]}_name".to_sym - source_email = send "#{options[:source]}_email".to_sym - text = if options[:avatar] - avatar = h.image_tag h.gravatar_icon(source_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]}", width: options[:size], alt: "" - %Q{#{avatar} #{source_name}} - else - source_name - end - - user = User.where('name like ? or email like ?', source_name, source_email).first - - if user.nil? - h.mail_to(source_email, text.html_safe, class: "commit-#{options[:source]}-link") - else - h.link_to(text.html_safe, h.user_path(user), class: "commit-#{options[:source]}-link") - end - end -end diff --git a/app/decorators/tree_decorator.rb b/app/decorators/tree_decorator.rb deleted file mode 100644 index 0e760f97..00000000 --- a/app/decorators/tree_decorator.rb +++ /dev/null @@ -1,33 +0,0 @@ -class TreeDecorator < ApplicationDecorator - decorates :tree - - def breadcrumbs(max_links = 2) - if path - part_path = "" - parts = path.split("\/") - - yield('..', nil) if parts.count > max_links - - parts.each do |part| - part_path = File.join(part_path, part) unless part_path.empty? - part_path = part if part_path.empty? - - next unless parts.last(2).include?(part) if parts.count > max_links - yield(part, h.tree_join(ref, part_path)) - end - end - end - - def up_dir? - path.present? - end - - def up_dir_path - file = File.join(path, "..") - h.tree_join(ref, file) - end - - def readme - @readme ||= contents.find { |c| c.is_a?(Grit::Blob) and c.name =~ /^readme/i } - end -end From a0bca5b71d11454b51f890f4dd36bbf948f1edf5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:45:38 +0300 Subject: [PATCH 07/25] Add Repository model to proxy request to Gitlab::Git::Repositoty and decorate commits with Commit model --- app/models/repository.rb | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 app/models/repository.rb diff --git a/app/models/repository.rb b/app/models/repository.rb new file mode 100644 index 00000000..048795b2 --- /dev/null +++ b/app/models/repository.rb @@ -0,0 +1,41 @@ +class Repository + attr_accessor :raw_repository + + def initialize(path_with_namespace, default_branch) + @raw_repository = Gitlab::Git::Repository.new(path_with_namespace, default_branch) + end + + def commit(id = nil) + commit = raw_repository.commit(id) + commit = Commit.new(commit) if commit + commit + end + + def commits(ref, path = nil, limit = nil, offset = nil) + commits = raw_repository.commits(ref, path, limit, offset) + commits = decorate_commits(commits) if commits.present? + commits + end + + def commits_between(target, source) + commits = raw_repository.commits_between(target, source) + commits = decorate_commits(commits) if commits.present? + commits + end + + def method_missing(m, *args, &block) + @raw_repository.send(m, *args, &block) + end + + def respond_to?(method) + return true if @raw_repository.respond_to?(method) + + super + end + + protected + + def decorate_commits(commits) + commits.map { |c| Commit.new(c) } + end +end From 2a6b4f965e97867dbec9c3c5091b61c4ec27bb5d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:45:58 +0300 Subject: [PATCH 08/25] rake task to clear redis cache --- lib/tasks/cache.rake | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 lib/tasks/cache.rake diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake new file mode 100644 index 00000000..8320b9b2 --- /dev/null +++ b/lib/tasks/cache.rake @@ -0,0 +1,6 @@ +namespace :cache do + desc "GITLAB | Clear redis cache" + task :clear => :environment do + Rails.cache.clear + end +end From 685681e28af2cae7c5ba208130ad5b6b4e5c4ed9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:46:25 +0300 Subject: [PATCH 09/25] remove Tree/Commit decorator usage from controllers --- app/controllers/commits_controller.rb | 2 -- app/controllers/compare_controller.rb | 2 -- app/controllers/merge_requests_controller.rb | 3 --- app/controllers/refs_controller.rb | 3 --- 4 files changed, 10 deletions(-) diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index fded5f0a..cde1f459 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -13,8 +13,6 @@ class CommitsController < ProjectResourceController @limit, @offset = (params[:limit] || 40), (params[:offset] || 0) @commits = @repo.commits(@ref, @path, @limit, @offset) - @commits = Commit.decorate(@commits) - @commits = CommitDecorator.decorate_collection(@commits) respond_to do |format| format.html # index.html.erb diff --git a/app/controllers/compare_controller.rb b/app/controllers/compare_controller.rb index bd3f1115..b72da783 100644 --- a/app/controllers/compare_controller.rb +++ b/app/controllers/compare_controller.rb @@ -15,8 +15,6 @@ class CompareController < ProjectResourceController @diffs = result[:diffs] @refs_are_same = result[:same] @line_notes = [] - - @commits = CommitDecorator.decorate_collection(@commits) end def create diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index e2185361..1950ebb2 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -94,12 +94,10 @@ class MergeRequestsController < ProjectResourceController def branch_from @commit = @repository.commit(params[:ref]) - @commit = CommitDecorator.decorate(@commit) end def branch_to @commit = @repository.commit(params[:ref]) - @commit = CommitDecorator.decorate(@commit) end def ci_status @@ -143,7 +141,6 @@ class MergeRequestsController < ProjectResourceController # Get commits from repository # or from cache if already merged @commits = @merge_request.commits - @commits = CommitDecorator.decorate_collection(@commits) @allowed_to_merge = allowed_to_merge? @show_merge_controls = @merge_request.opened? && @commits.any? && @allowed_to_merge diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 0e4dba3d..eb8d1e19 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -34,7 +34,6 @@ class RefsController < ProjectResourceController @logs = contents.map do |content| file = params[:path] ? File.join(params[:path], content.name) : content.name last_commit = @repo.commits(@commit.id, file, 1).last - last_commit = CommitDecorator.decorate(last_commit) { file_name: content.name, commit: last_commit @@ -49,9 +48,7 @@ class RefsController < ProjectResourceController @repo = project.repository @commit = @repo.commit(@ref) - @commit = CommitDecorator.decorate(@commit) @tree = Tree.new(@commit.tree, @ref, params[:path]) - @tree = TreeDecorator.new(@tree) @hex_path = Digest::SHA1.hexdigest(params[:path] || "") if params[:path] From da5b0c91dc79136be4315cf61e5520692e19be8a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:46:54 +0300 Subject: [PATCH 10/25] Move some decorator logic to helpers --- app/helpers/commits_helper.rb | 74 +++++++++++++++++++++++++++++++++++ app/helpers/tree_helper.rb | 34 ++++++++-------- 2 files changed, 90 insertions(+), 18 deletions(-) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index acdd48e0..da209e06 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -1,4 +1,20 @@ module CommitsHelper + # Returns a link to the commit author. If the author has a matching user and + # is a member of the current @project it will link to the team member page. + # Otherwise it will link to the author email as specified in the commit. + # + # options: + # avatar: true will prepend the avatar image + # size: size of the avatar image in px + def commit_author_link(commit, options = {}) + commit_person_link(commit, options.merge(source: :author)) + end + + # Just like #author_link but for the committer. + def commit_committer_link(commit, options = {}) + commit_person_link(commit, options.merge(source: :committer)) + end + def identification_type(line) if line[0] == "+" "new" @@ -105,4 +121,62 @@ module CommitsHelper line end end + + # Breadcrumb links for a Project and, if applicable, a tree path + def commits_breadcrumbs + return unless @project && @ref + + # Add the root project link and the arrow icon + crumbs = content_tag(:li) do + content_tag(:span, nil, class: 'arrow') + + link_to(@project.name, project_commits_path(@project, @ref)) + end + + if @path + parts = @path.split('/') + + parts.each_with_index do |part, i| + crumbs += content_tag(:span, '/', class: 'divider') + crumbs += content_tag(:li) do + # The text is just the individual part, but the link needs all the parts before it + link_to part, project_commits_path(@project, tree_join(@ref, parts[0..i].join('/'))) + end + end + end + + crumbs.html_safe + end + + protected + + def no_commit_message + "--no commit message" + end + + # Private: Returns a link to a person. If the person has a matching user and + # is a member of the current @project it will link to the team member page. + # Otherwise it will link to the person email as specified in the commit. + # + # options: + # source: one of :author or :committer + # avatar: true will prepend the avatar image + # size: size of the avatar image in px + def commit_person_link(commit, options = {}) + source_name = commit.send "#{options[:source]}_name".to_sym + source_email = commit.send "#{options[:source]}_email".to_sym + text = if options[:avatar] + avatar = image_tag(gravatar_icon(source_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]}", width: options[:size], alt: "") + %Q{#{avatar} #{source_name}} + else + source_name + end + + user = User.where('name like ? or email like ?', source_name, source_email).first + + if user.nil? + mail_to(source_email, text.html_safe, class: "commit-#{options[:source]}-link") + else + link_to(text.html_safe, user_path(user), class: "commit-#{options[:source]}-link") + end + end end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index fab0085b..1cba9476 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -70,28 +70,26 @@ module TreeHelper end end - # Breadcrumb links for a Project and, if applicable, a tree path - def breadcrumbs - return unless @project && @ref + def tree_breadcrumbs(tree, max_links = 2) + if tree.path + part_path = "" + parts = tree.path.split("\/") - # Add the root project link and the arrow icon - crumbs = content_tag(:li) do - content_tag(:span, nil, class: 'arrow') + - link_to(@project.name, project_commits_path(@project, @ref)) - end + yield('..', nil) if parts.count > max_links - if @path - parts = @path.split('/') + parts.each do |part| + part_path = File.join(part_path, part) unless part_path.empty? + part_path = part if part_path.empty? - parts.each_with_index do |part, i| - crumbs += content_tag(:span, '/', class: 'divider') - crumbs += content_tag(:li) do - # The text is just the individual part, but the link needs all the parts before it - link_to part, project_commits_path(@project, tree_join(@ref, parts[0..i].join('/'))) - end + next unless parts.last(2).include?(part) if parts.count > max_links + yield(part, tree_join(tree.ref, part_path)) end end - - crumbs.html_safe end + + def up_dir_path tree + file = File.join(tree.path, "..") + tree_join(tree.ref, file) + end + end From b53557aca64fbf55f9bbd59849d83daa10b7361f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:47:26 +0300 Subject: [PATCH 11/25] Remove decorator calls and methods from views. Repalace with helper calls when needed --- app/views/blame/show.html.haml | 4 ++-- app/views/commit/_commit_box.html.haml | 4 ++-- app/views/commits/_commit.html.haml | 2 +- app/views/commits/show.html.haml | 2 +- app/views/events/_commit.html.haml | 1 - app/views/repositories/_branch.html.haml | 3 +-- app/views/repositories/_feed.html.haml | 1 - app/views/repositories/tags.html.haml | 3 +-- app/views/tree/_tree.html.haml | 4 ++-- app/views/tree/_tree_commit_column.html.haml | 2 +- app/views/wikis/history.html.haml | 5 +++-- app/views/wikis/pages.html.haml | 4 ++-- app/views/wikis/show.html.haml | 4 ++-- 13 files changed, 18 insertions(+), 21 deletions(-) diff --git a/app/views/blame/show.html.haml b/app/views/blame/show.html.haml index b2a45ef5..b07a514f 100644 --- a/app/views/blame/show.html.haml +++ b/app/views/blame/show.html.haml @@ -22,13 +22,13 @@ %table - current_line = 1 - @blame.each do |commit, lines| - - commit = CommitDecorator.decorate(Commit.new(commit)) + - commit = Commit.new(commit) %tr %td.blame-commit %span.commit = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id"   - = commit.author_link avatar: true, size: 16 + = commit_author_link(commit, avatar: true, size: 16)   = link_to_gfm truncate(commit.title, length: 20), project_commit_path(@project, commit.id), class: "row_title" %td.lines.blame-numbers diff --git a/app/views/commit/_commit_box.html.haml b/app/views/commit/_commit_box.html.haml index 4c80c13c..64679177 100644 --- a/app/views/commit/_commit_box.html.haml +++ b/app/views/commit/_commit_box.html.haml @@ -24,14 +24,14 @@ .row .span5 .author - = @commit.author_link avatar: true, size: 32 + = commit_author_link(@commit, avatar: true, size: 32) authored %time{title: @commit.authored_date.stamp("Aug 21, 2011 9:23pm")} #{time_ago_in_words(@commit.authored_date)} ago - if @commit.different_committer? .committer → - = @commit.committer_link + = commit_committer_link(@commit) committed %time{title: @commit.committed_date.stamp("Aug 21, 2011 9:23pm")} #{time_ago_in_words(@commit.committed_date)} ago diff --git a/app/views/commits/_commit.html.haml b/app/views/commits/_commit.html.haml index 2f5ff130..65d92030 100644 --- a/app/views/commits/_commit.html.haml +++ b/app/views/commits/_commit.html.haml @@ -4,7 +4,7 @@ %strong= link_to "Browse Code ยป", project_tree_path(@project, commit), class: "right" %p = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id" - = commit.author_link avatar: true, size: 24 + = commit_author_link(commit, avatar: true, size: 24)   = link_to_gfm truncate(commit.title, length: 70), project_commit_path(@project, commit.id), class: "row_title" diff --git a/app/views/commits/show.html.haml b/app/views/commits/show.html.haml index d180b8ec..586b21df 100644 --- a/app/views/commits/show.html.haml +++ b/app/views/commits/show.html.haml @@ -2,7 +2,7 @@ - if @path.present? %ul.breadcrumb - = breadcrumbs + = commits_breadcrumbs %div{id: dom_id(@project)} #commits-list= render "commits" diff --git a/app/views/events/_commit.html.haml b/app/views/events/_commit.html.haml index ea417aa9..f2f2d47e 100644 --- a/app/views/events/_commit.html.haml +++ b/app/views/events/_commit.html.haml @@ -1,4 +1,3 @@ -- commit = CommitDecorator.decorate(commit) %li.commit %p = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" diff --git a/app/views/repositories/_branch.html.haml b/app/views/repositories/_branch.html.haml index a6faa5fd..dd91e14b 100644 --- a/app/views/repositories/_branch.html.haml +++ b/app/views/repositories/_branch.html.haml @@ -1,5 +1,4 @@ -- commit = Commit.new(branch.commit) -- commit = CommitDecorator.decorate(commit) +- commit = Commit.new(Gitlab::Git::Commit.new(branch.commit)) %tr %td = link_to project_commits_path(@project, branch.name) do diff --git a/app/views/repositories/_feed.html.haml b/app/views/repositories/_feed.html.haml index eaf15ca7..6bb75265 100644 --- a/app/views/repositories/_feed.html.haml +++ b/app/views/repositories/_feed.html.haml @@ -1,5 +1,4 @@ - commit = update -- commit = CommitDecorator.new(commit) %tr %td = link_to project_commits_path(@project, commit.head.name) do diff --git a/app/views/repositories/tags.html.haml b/app/views/repositories/tags.html.haml index d4b8bbe1..2d311124 100644 --- a/app/views/repositories/tags.html.haml +++ b/app/views/repositories/tags.html.haml @@ -7,8 +7,7 @@ %th Last commit %th - @tags.each do |tag| - - commit = Commit.new(tag.commit) - - commit = CommitDecorator.decorate(commit) + - commit = Commit.new(Gitlab::Git::Commit.new(tag.commit)) %tr %td %strong diff --git a/app/views/tree/_tree.html.haml b/app/views/tree/_tree.html.haml index 24a57ae7..caab6e45 100644 --- a/app/views/tree/_tree.html.haml +++ b/app/views/tree/_tree.html.haml @@ -3,7 +3,7 @@ %i.icon-angle-right = link_to project_tree_path(@project, @ref) do = @project.path - - tree.breadcrumbs(6) do |title, path| + - tree_breadcrumbs(tree, 6) do |title, path| \/ %li - if path @@ -27,7 +27,7 @@ %tr.tree-item %td.tree-item-file-name = image_tag "file_empty.png", size: '16x16' - = link_to "..", project_tree_path(@project, tree.up_dir_path) + = link_to "..", project_tree_path(@project, up_dir_path(tree)) %td %td %td diff --git a/app/views/tree/_tree_commit_column.html.haml b/app/views/tree/_tree_commit_column.html.haml index 9d02132b..7ae2582c 100644 --- a/app/views/tree/_tree_commit_column.html.haml +++ b/app/views/tree/_tree_commit_column.html.haml @@ -1,2 +1,2 @@ -%span.tree_author= commit.author_link avatar: true +%span.tree_author= commit_author_link(commit, avatar: true) = link_to_gfm truncate(commit.title, length: 80), project_commit_path(@project, commit.id), class: "tree-commit-link" diff --git a/app/views/wikis/history.html.haml b/app/views/wikis/history.html.haml index 599e9cf6..f4946ed0 100644 --- a/app/views/wikis/history.html.haml +++ b/app/views/wikis/history.html.haml @@ -14,12 +14,13 @@ %th Format %tbody - @wiki.versions.each do |version| - - commit = CommitDecorator.new(version) + - commit = version %tr %td = link_to project_wiki_path(@project, @wiki, version_id: commit.id) do = commit.short_id - %td= commit.author_link avatar: true, size: 24 + %td + = commit_author_link(commit, avatar: true, size: 24) %td = commit.title %td diff --git a/app/views/wikis/pages.html.haml b/app/views/wikis/pages.html.haml index eb65599d..95d5eef1 100644 --- a/app/views/wikis/pages.html.haml +++ b/app/views/wikis/pages.html.haml @@ -21,5 +21,5 @@ = wiki_page.created_at.to_s(:short) do (#{time_ago_in_words(wiki_page.created_at)} ago) - - commit = CommitDecorator.decorate(wiki_page.version) - %td= commit.author_link avatar: true, size: 24 + %td + = commit_author_link(wiki_page.version, avatar: true, size: 24) diff --git a/app/views/wikis/show.html.haml b/app/views/wikis/show.html.haml index b660a15e..4102182e 100644 --- a/app/views/wikis/show.html.haml +++ b/app/views/wikis/show.html.haml @@ -13,5 +13,5 @@ = preserve do = render_wiki_content(@wiki) -- commit = CommitDecorator.new(@wiki.version) -%p.time Last edited by #{commit.author_link(avatar: true, size: 16)} #{time_ago_in_words @wiki.created_at} ago +- commit = Commit.new(@wiki.version) +%p.time Last edited by #{commit_author_link(commit, avatar: true, size: 16)} #{time_ago_in_words @wiki.created_at} ago From bbfbff3add4c78ce1256ac3bbe787cc6eb9fe1b9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:48:12 +0300 Subject: [PATCH 12/25] Extend models functionality with old decorator methods. Use Repository model --- app/contexts/commit_load_context.rb | 1 - app/mailers/emails/notes.rb | 1 - app/models/commit.rb | 51 ++++++++++++++++++++++++++--- app/models/merge_request.rb | 13 ++++++-- app/models/project.rb | 2 +- app/models/tree.rb | 8 +++++ lib/extracts_path.rb | 2 -- 7 files changed, 67 insertions(+), 11 deletions(-) diff --git a/app/contexts/commit_load_context.rb b/app/contexts/commit_load_context.rb index c8d77d9b..a0652377 100644 --- a/app/contexts/commit_load_context.rb +++ b/app/contexts/commit_load_context.rb @@ -13,7 +13,6 @@ class CommitLoadContext < BaseContext if commit commit = Commit.new(commit) - commit = CommitDecorator.decorate(commit) line_notes = project.notes.for_commit_id(commit.id).inline result[:commit] = commit diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index de51debf..769b6e0b 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -3,7 +3,6 @@ module Emails def note_commit_email(recipient_id, note_id) @note = Note.find(note_id) @commit = @note.noteable - @commit = CommitDecorator.decorate(@commit) @project = @note.project mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title)) end diff --git a/app/models/commit.rb b/app/models/commit.rb index ea5b451b..96c8577f 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -10,10 +10,6 @@ class Commit attr_accessor :raw - def self.decorate(commits) - commits.map { |c| Commit.new(c) } - end - def initialize(raw_commit) raise "Nil as raw commit passed" unless raw_commit @@ -24,7 +20,54 @@ class Commit @raw.id end + # Returns a string describing the commit for use in a link title + # + # Example + # + # "Commit: Alex Denisov - Project git clone panel" + def link_title + "Commit: #{author_name} - #{title}" + end + + # Returns the commits title. + # + # Usually, the commit title is the first line of the commit message. + # In case this first line is longer than 80 characters, it is cut off + # after 70 characters and ellipses (`&hellp;`) are appended. + def title + title = safe_message + + return no_commit_message if title.blank? + + title_end = title.index(/\n/) + if (!title_end && title.length > 80) || (title_end && title_end > 80) + title[0..69] << "…".html_safe + else + title.split(/\n/, 2).first + end + end + + # Returns the commits description + # + # cut off, ellipses (`&hellp;`) are prepended to the commit message. + def description + description = safe_message + + title_end = description.index(/\n/) + if (!title_end && description.length > 80) || (title_end && title_end > 80) + "…".html_safe << description[70..-1] + else + description.split(/\n/, 2)[1].try(:chomp) + end + end + def method_missing(m, *args, &block) @raw.send(m, *args, &block) end + + def respond_to?(method) + return true if @raw.respond_to?(method) + + super + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 505f6637..8d378053 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -152,7 +152,17 @@ class MergeRequest < ActiveRecord::Base end def commits - st_commits || [] + if st_commits.present? + # check if merge request commits are valid + if st_commits.first.respond_to?(:short_id) + st_commits + else + # if commits are invalid - simply reload it from repo + reloaded_commits + end + else + [] + end end def probably_merged? @@ -171,7 +181,6 @@ class MergeRequest < ActiveRecord::Base def unmerged_commits self.project.repository. commits_between(self.target_branch, self.source_branch). - map {|c| Commit.new(c)}. sort_by(&:created_at). reverse end diff --git a/app/models/project.rb b/app/models/project.rb index 54abbc3e..934dd6b2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -142,7 +142,7 @@ class Project < ActiveRecord::Base def repository if path - @repository ||= Gitlab::Git::Repository.new(path_with_namespace, default_branch) + @repository ||= Repository.new(path_with_namespace, default_branch) else nil end diff --git a/app/models/tree.rb b/app/models/tree.rb index 96395a42..4b6c5b13 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -26,4 +26,12 @@ class Tree def empty? data.blank? end + + def up_dir? + path.present? + end + + def readme + @readme ||= contents.find { |c| c.is_a?(Grit::Blob) and c.name =~ /^readme/i } + end end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 4ad485c5..d4871a49 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -101,10 +101,8 @@ module ExtractsPath # It is used "@project.repository.commits(@ref, @path, 1, 0)", # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. @commit = @project.repository.commits(@ref, @path, 1, 0).first - @commit = CommitDecorator.decorate(@commit) @tree = Tree.new(@commit.tree, @ref, @path) - @tree = TreeDecorator.new(@tree) raise InvalidPathError if @tree.invalid? rescue RuntimeError, NoMethodError, InvalidPathError From 458631c5ba8c830021bd7c219affdb0afe6f0efe Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 23:58:17 +0300 Subject: [PATCH 13/25] remove unnecessary Commit.new --- app/contexts/commit_load_context.rb | 1 - app/helpers/commits_helper.rb | 4 +--- app/models/note.rb | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/contexts/commit_load_context.rb b/app/contexts/commit_load_context.rb index a0652377..2cf5420d 100644 --- a/app/contexts/commit_load_context.rb +++ b/app/contexts/commit_load_context.rb @@ -12,7 +12,6 @@ class CommitLoadContext < BaseContext commit = project.repository.commit(params[:id]) if commit - commit = Commit.new(commit) line_notes = project.notes.for_commit_id(commit.id).inline result[:commit] = commit diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index da209e06..66603c97 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -109,9 +109,7 @@ module CommitsHelper end def commit_to_html commit - if commit.model - escape_javascript(render 'commits/commit', commit: commit) - end + escape_javascript(render 'commits/commit', commit: commit) end def diff_line_content(line) diff --git a/app/models/note.rb b/app/models/note.rb index 17ceb541..f26420ca 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -130,7 +130,7 @@ class Note < ActiveRecord::Base # override to return commits, which are not active record def noteable if for_commit? - Commit.new(project.repository.commit(commit_id)) + project.repository.commit(commit_id) else super end From 7bb71bb088e17578482e7f934147b0fd11c7ad0e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 09:21:31 +0300 Subject: [PATCH 14/25] Fix stubbed repo --- app/models/repository.rb | 4 ++-- spec/support/stubbed_repository.rb | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 048795b2..be6502eb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -24,11 +24,11 @@ class Repository end def method_missing(m, *args, &block) - @raw_repository.send(m, *args, &block) + raw_repository.send(m, *args, &block) end def respond_to?(method) - return true if @raw_repository.respond_to?(method) + return true if raw_repository.respond_to?(method) super end diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 68c380e6..3dfdb353 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -10,7 +10,7 @@ class Project if path == "empty" || !path nil else - GitLabTestRepo.new(path_with_namespace) + GitLabTestRepo.new(Rails.root.join('tmp', 'repositories', 'gitlabhq'), 'master') end end @@ -39,11 +39,7 @@ class MergeRequest end end -class GitLabTestRepo < Gitlab::Git::Repository - def repo - @repo ||= Grit::Repo.new(Rails.root.join('tmp', 'repositories', 'gitlabhq')) - end - +class GitLabTestRepo < Repository # patch repo size (in mb) def size 12.45 From 22817398e6c1cf9a479fecd99c55369fd81717cb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 14:39:19 +0300 Subject: [PATCH 15/25] define TestEnv and keep all global stubs in one place --- features/support/env.rb | 17 ++----- lib/gitlab/git/repository.rb | 6 ++- spec/factories.rb | 2 +- spec/support/stubbed_repository.rb | 71 ------------------------------ spec/support/test_env.rb | 63 ++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 87 deletions(-) delete mode 100644 spec/support/stubbed_repository.rb create mode 100644 spec/support/test_env.rb diff --git a/features/support/env.rb b/features/support/env.rb index 90a61dd1..08b627f5 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -14,7 +14,7 @@ require 'spinach/capybara' require 'sidekiq/testing/inline' -%w(stubbed_repository valid_commit select2_helper).each do |f| +%w(valid_commit select2_helper test_env).each do |f| require Rails.root.join('spec', 'support', f) end @@ -35,13 +35,8 @@ Capybara.default_wait_time = 10 DatabaseCleaner.strategy = :truncation Spinach.hooks.before_scenario do - # Use tmp dir for FS manipulations - Gitlab.config.gitlab_shell.stub(repos_path: Rails.root.join('tmp', 'test-git-base-path')) - Gitlab::Shell.any_instance.stub(:add_repository) do |path| - create_temp_repo("#{Rails.root}/tmp/test-git-base-path/#{path}.git") - end - FileUtils.rm_rf Gitlab.config.gitlab_shell.repos_path - FileUtils.mkdir_p Gitlab.config.gitlab_shell.repos_path + TestEnv.init + DatabaseCleaner.start end @@ -54,9 +49,3 @@ Spinach.hooks.before_run do include FactoryGirl::Syntax::Methods end - -def create_temp_repo(path) - FileUtils.mkdir_p path - command = "git init --quiet --bare #{path};" - system(command) -end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 53d9c735..30344a3d 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -34,7 +34,11 @@ module Gitlab end def path_to_repo - @path_to_repo ||= File.join(Gitlab.config.gitlab_shell.repos_path, "#{path_with_namespace}.git") + @path_to_repo ||= File.join(repos_path, "#{path_with_namespace}.git") + end + + def repos_path + Gitlab.config.gitlab_shell.repos_path end def repo diff --git a/spec/factories.rb b/spec/factories.rb index 41766859..8f620cd7 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 { name.downcase.gsub(/\s/, '_') } + path { 'gitlabhq' } creator end diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb deleted file mode 100644 index 3dfdb353..00000000 --- a/spec/support/stubbed_repository.rb +++ /dev/null @@ -1,71 +0,0 @@ -require "gitlab/git/repository" -require "project" -require "merge_request" -require "shell" - -# Stubs out all Git repository access done by models so that specs can run -# against fake repositories without Grit complaining that they don't exist. -class Project - def repository - if path == "empty" || !path - nil - else - GitLabTestRepo.new(Rails.root.join('tmp', 'repositories', 'gitlabhq'), 'master') - end - end - - def satellite - FakeSatellite.new - end - - class FakeSatellite - def exists? - true - end - - def destroy - true - end - - def create - true - end - end -end - -class MergeRequest - def check_if_can_be_merged - true - end -end - -class GitLabTestRepo < Repository - # patch repo size (in mb) - def size - 12.45 - end -end - -module Gitlab - class Shell - def add_repository name - true - end - - def mv_repository name, new_name - true - end - - def remove_repository name - true - end - - def add_key id, key - true - end - - def remove_key id, key - true - end - end -end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb new file mode 100644 index 00000000..769405b6 --- /dev/null +++ b/spec/support/test_env.rb @@ -0,0 +1,63 @@ +module TestEnv + extend self + + # Test environment + # + # all repositories and namespaces stored at + # RAILS_APP/tmp/test-git-base-path + # + # Next shell methods are stubbed and return true + # - mv_repository + # - remove_repository + # - add_key + # - remove_key + # + def init + # Use tmp dir for FS manipulations + repos_path = Rails.root.join('tmp', 'test-git-base-path') + Gitlab.config.gitlab_shell.stub(repos_path: repos_path) + + Gitlab::Shell.any_instance.stub( + add_repository: ->(path) { create_temp_repo(File.join(repos_path, "#{path}.git")) }, + mv_repository: true, + remove_repository: true, + add_key: true, + remove_key: true + ) + + fake_satellite = double( + exists?: true, + destroy: true, + create: true + ) + + Project.any_instance.stub( + satellite: fake_satellite + ) + + MergeRequest.any_instance.stub( + check_if_can_be_merged: true + ) + + Repository.any_instance.stub( + size: 12.45 + ) + + # Remove tmp/test-git-base-path + FileUtils.rm_rf Gitlab.config.gitlab_shell.repos_path + + # Recreate tmp/test-git-base-path + FileUtils.mkdir_p Gitlab.config.gitlab_shell.repos_path + + # Symlink tmp/repositories/gitlabhq to tmp/test-git-base-path/gitlabhq + seed_repo = Rails.root.join('tmp', 'repositories', 'gitlabhq') + target_repo = File.join(repos_path, 'gitlabhq.git') + system("ln -s #{seed_repo} #{target_repo}") + end + + def create_temp_repo(path) + FileUtils.mkdir_p path + command = "git init --quiet --bare #{path};" + system(command) + end +end From 51c167554cf492be98cecad182a6870cd6febb82 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 16:02:54 +0300 Subject: [PATCH 16/25] added Gitlab::Git::Blame for git blame feature --- lib/gitlab/git/blame.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 lib/gitlab/git/blame.rb diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb new file mode 100644 index 00000000..d6e988b6 --- /dev/null +++ b/lib/gitlab/git/blame.rb @@ -0,0 +1,22 @@ +module Gitlab + module Git + class Blame + + attr_accessor :repository, :sha, :path + + def initialize(repository, sha, path) + @repository, @sha, @path = repository, sha, path + + end + + def each + raw_blame = Grit::Blob.blame(repository.repo, sha, path) + + raw_blame.each do |commit, lines| + commit = Gitlab::Git::Commit.new(commit) + yield(commit, lines) + end + end + end + end +end From bb06e905efb1722502d71059c21add8cfde851aa Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 16:03:11 +0300 Subject: [PATCH 17/25] added Gitlab::Git::Compare for git compare feature --- lib/gitlab/git/compare.rb | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 lib/gitlab/git/compare.rb diff --git a/lib/gitlab/git/compare.rb b/lib/gitlab/git/compare.rb new file mode 100644 index 00000000..1fa43062 --- /dev/null +++ b/lib/gitlab/git/compare.rb @@ -0,0 +1,37 @@ +module Gitlab + module Git + class Compare + attr_accessor :commits, :commit, :diffs, :same + + def initialize(repository, from, to) + @commits, @diffs = [], [] + @commit = nil + @same = false + + return unless from && to + + first = repository.commit(to.try(:strip)) + last = repository.commit(from.try(:strip)) + + return unless first && last + + if first.id == last.id + @same = true + return + end + + @commit = Commit.new(first) + + @commits = repository.commits_between(last.id, first.id) + @commits = @commits.map { |c| Commit.new(c) } + + @diffs = if @commits.size > 100 + [] + else + repository.repo.diff(last.id, first.id) rescue [] + end + end + end + end +end + From 49b024f5f5b88d406b895f050943db1e75adfa2a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 16:04:35 +0300 Subject: [PATCH 18/25] Use Gitlab::Git:: for git features across application --- app/controllers/blame_controller.rb | 3 +- app/controllers/compare_controller.rb | 10 +++--- app/models/commit.rb | 4 +++ app/models/gollum_wiki.rb | 1 - app/models/repository.rb | 10 ++---- app/models/wiki_page.rb | 4 +-- app/views/blame/show.html.haml | 2 +- app/views/compare/show.html.haml | 2 +- app/views/wikis/show.html.haml | 3 +- lib/gitlab/git/commit.rb | 50 ++++++--------------------- spec/support/test_env.rb | 7 ++-- 11 files changed, 32 insertions(+), 64 deletions(-) diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb index 76caa4a6..310b567c 100644 --- a/app/controllers/blame_controller.rb +++ b/app/controllers/blame_controller.rb @@ -8,7 +8,6 @@ class BlameController < ProjectResourceController before_filter :require_non_empty_project def show - @repo = @project.repo - @blame = Grit::Blob.blame(@repo, @commit.id, @path) + @blame = Gitlab::Git::Blame.new(project.repository, @commit.id, @path) end end diff --git a/app/controllers/compare_controller.rb b/app/controllers/compare_controller.rb index b72da783..750e9c23 100644 --- a/app/controllers/compare_controller.rb +++ b/app/controllers/compare_controller.rb @@ -8,12 +8,12 @@ class CompareController < ProjectResourceController end def show - result = Commit.compare(project, params[:from], params[:to]) + compare = Gitlab::Git::Compare.new(project.repository, params[:from], params[:to]) - @commits = result[:commits] - @commit = result[:commit] - @diffs = result[:diffs] - @refs_are_same = result[:same] + @commits = compare.commits + @commit = compare.commit + @diffs = compare.diffs + @refs_are_same = compare.same @line_notes = [] end diff --git a/app/models/commit.rb b/app/models/commit.rb index 96c8577f..e3363350 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -8,6 +8,10 @@ class Commit # DIFF_SAFE_SIZE = 100 + def self.decorate(commits) + commits.map { |c| self.new(c) } + end + attr_accessor :raw def initialize(raw_commit) diff --git a/app/models/gollum_wiki.rb b/app/models/gollum_wiki.rb index cdfcd567..647058e8 100644 --- a/app/models/gollum_wiki.rb +++ b/app/models/gollum_wiki.rb @@ -114,5 +114,4 @@ class GollumWiki def path_to_repo @path_to_repo ||= File.join(Gitlab.config.gitlab_shell.repos_path, "#{path_with_namespace}.git") end - end diff --git a/app/models/repository.rb b/app/models/repository.rb index be6502eb..0a4431f1 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -13,13 +13,13 @@ class Repository def commits(ref, path = nil, limit = nil, offset = nil) commits = raw_repository.commits(ref, path, limit, offset) - commits = decorate_commits(commits) if commits.present? + commits = Commit.decorate(commits) if commits.present? commits end def commits_between(target, source) commits = raw_repository.commits_between(target, source) - commits = decorate_commits(commits) if commits.present? + commits = Commit.decorate(commits) if commits.present? commits end @@ -32,10 +32,4 @@ class Repository super end - - protected - - def decorate_commits(commits) - commits.map { |c| Commit.new(c) } - end end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index adc77b22..497d69e8 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -79,14 +79,14 @@ class WikiPage def version return nil unless persisted? - @version ||= Commit.new(@page.version) + @version ||= Commit.new(Gitlab::Git::Commit.new(@page.version)) end # Returns an array of Gitlab Commit instances. def versions return [] unless persisted? - @page.versions.map { |v| Commit.new(v) } + @page.versions.map { |v| Commit.new(Gitlab::Git::Commit.new(v)) } end # Returns the Date that this latest version was diff --git a/app/views/blame/show.html.haml b/app/views/blame/show.html.haml index b07a514f..96d153e6 100644 --- a/app/views/blame/show.html.haml +++ b/app/views/blame/show.html.haml @@ -6,7 +6,7 @@ %i.icon-angle-right = link_to project_tree_path(@project, @ref) do = @project.name - - @tree.breadcrumbs(6) do |link| + - tree_breadcrumbs(@tree, 6) do |link| \/ %li= link .clear diff --git a/app/views/compare/show.html.haml b/app/views/compare/show.html.haml index 476be255..56c4a113 100644 --- a/app/views/compare/show.html.haml +++ b/app/views/compare/show.html.haml @@ -16,7 +16,7 @@ %div.ui-box %h5.title Commits (#{@commits.count}) - %ul.well-list= render @commits + %ul.well-list= render Commit.decorate(@commits) - unless @diffs.empty? %h4 Diff diff --git a/app/views/wikis/show.html.haml b/app/views/wikis/show.html.haml index 4102182e..b237bc52 100644 --- a/app/views/wikis/show.html.haml +++ b/app/views/wikis/show.html.haml @@ -13,5 +13,4 @@ = preserve do = render_wiki_content(@wiki) -- commit = Commit.new(@wiki.version) -%p.time Last edited by #{commit_author_link(commit, avatar: true, size: 16)} #{time_ago_in_words @wiki.created_at} ago +%p.time Last edited by #{commit_author_link(@wiki.version, avatar: true, size: 16)} #{time_ago_in_words @wiki.created_at} ago diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 023672f9..d7e1a5ca 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -1,9 +1,9 @@ -# Gitlab::Git::Gitlab::Git::Commit is a wrapper around native Grit::Commit object +# Gitlab::Git::Commit is a wrapper around native Grit::Commit object # We dont want to use grit objects inside app/ # It helps us easily migrate to rugged in future module Gitlab module Git - class Gitlab::Git::Commit + class Commit attr_accessor :raw_commit, :head, :refs delegate :message, :authored_date, :committed_date, :parents, :sha, @@ -18,12 +18,12 @@ module Gitlab repo.commits(root_ref).first end - Gitlab::Git::Commit.new(commit) if commit + Commit.new(commit) if commit end def fresh_commits(repo, n = 10) commits = repo.heads.map do |h| - repo.commits(h.name, n).map { |c| Gitlab::Git::Commit.new(c, h) } + repo.commits(h.name, n).map { |c| Commit.new(c, h) } end.flatten.uniq { |c| c.id } commits.sort! do |x, y| @@ -34,7 +34,7 @@ module Gitlab end def commits_with_refs(repo, n = 20) - commits = repo.branches.map { |ref| Gitlab::Git::Commit.new(ref.commit, ref) } + commits = repo.branches.map { |ref| Commit.new(ref.commit, ref) } commits.sort! do |x, y| y.committed_date <=> x.committed_date @@ -45,7 +45,7 @@ module Gitlab def commits_since(repo, date) commits = repo.heads.map do |h| - repo.log(h.name, nil, since: date).each { |c| Gitlab::Git::Commit.new(c, h) } + repo.log(h.name, nil, since: date).each { |c| Commit.new(c, h) } end.flatten.uniq { |c| c.id } commits.sort! do |x, y| @@ -62,41 +62,11 @@ module Gitlab repo.commits(ref, limit, offset) else repo.commits(ref) - end.map{ |c| Gitlab::Git::Commit.new(c) } + end.map{ |c| Commit.new(c) } end def commits_between(repo, from, to) - repo.commits_between(from, to).map { |c| Gitlab::Git::Commit.new(c) } - end - - def compare(project, from, to) - result = { - commits: [], - diffs: [], - commit: nil, - same: false - } - - return result unless from && to - - first = project.repository.commit(to.try(:strip)) - last = project.repository.commit(from.try(:strip)) - - if first && last - result[:same] = (first.id == last.id) - result[:commits] = project.repo.commits_between(last.id, first.id).map {|c| Gitlab::Git::Commit.new(c)} - - # Dont load diff for 100+ commits - result[:diffs] = if result[:commits].size > 100 - [] - else - project.repo.diff(last.id, first.id) rescue [] - end - - result[:commit] = Gitlab::Git::Commit.new(first) - end - - result + repo.commits_between(from, to).map { |c| Commit.new(c) } end end @@ -142,7 +112,7 @@ module Gitlab def prev_commit @prev_commit ||= if parents.present? - Gitlab::Git::Commit.new(parents.first) + Commit.new(parents.first) else nil end @@ -156,7 +126,7 @@ module Gitlab # # Cuts out the header and stats from #to_patch and returns only the diff. def to_diff - # see Grit::Gitlab::Git::Commit#show + # see Grit::Commit#show patch = to_patch # discard lines before the diff diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 769405b6..0f81347d 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -17,15 +17,18 @@ module TestEnv repos_path = Rails.root.join('tmp', 'test-git-base-path') Gitlab.config.gitlab_shell.stub(repos_path: repos_path) + Gitlab::Shell.any_instance.stub(:add_repository) do |path| + create_temp_repo(File.join(repos_path, "#{path}.git")) + end + Gitlab::Shell.any_instance.stub( - add_repository: ->(path) { create_temp_repo(File.join(repos_path, "#{path}.git")) }, mv_repository: true, remove_repository: true, add_key: true, remove_key: true ) - fake_satellite = double( + fake_satellite = stub( exists?: true, destroy: true, create: true From 541d89941014137762dff696c83b3357eba8efeb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 16:56:25 +0300 Subject: [PATCH 19/25] 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 From 9a26e9a0d634c8bb796f0b08f6397d1e343bd4be Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 17:27:44 +0300 Subject: [PATCH 20/25] Dont init repo on every create(:repo) --- app/helpers/commits_helper.rb | 4 ---- app/models/gollum_wiki.rb | 6 +++++- lib/gitlab/git/commit.rb | 4 ++++ spec/factories.rb | 8 +++++--- spec/models/commit_spec.rb | 2 +- spec/models/gollum_wiki_spec.rb | 2 +- spec/support/test_env.rb | 3 ++- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 66603c97..95ca294c 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -147,10 +147,6 @@ module CommitsHelper protected - def no_commit_message - "--no commit message" - end - # Private: Returns a link to a person. If the person has a matching user and # is a member of the current @project it will link to the team member page. # Otherwise it will link to the person email as specified in the commit. diff --git a/app/models/gollum_wiki.rb b/app/models/gollum_wiki.rb index 647058e8..16e801c1 100644 --- a/app/models/gollum_wiki.rb +++ b/app/models/gollum_wiki.rb @@ -90,13 +90,17 @@ class GollumWiki private def create_repo! - if gitlab_shell.add_repository(path_with_namespace) + if init_repo(path_with_namespace) Gollum::Wiki.new(path_to_repo) else raise CouldNotCreateWikiError end end + def init_repo(path_with_namespace) + gitlab_shell.add_repository(path_with_namespace) + end + def commit_details(action, message = nil, title = nil) commit_message = message || default_message(action, title) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index d7e1a5ca..35991a38 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -144,6 +144,10 @@ module Gitlab rescue true end + + def no_commit_message + "--no commit message" + end end end end diff --git a/spec/factories.rb b/spec/factories.rb index d904e337..76bd3ebb 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -86,9 +86,9 @@ FactoryGirl.define do target_branch "master" # pretend bcf03b5d~3 source_branch "stable" # pretend bcf03b5d st_commits do - [Commit.new(project.repo.commit('bcf03b5d')), - Commit.new(project.repo.commit('bcf03b5d~1')), - Commit.new(project.repo.commit('bcf03b5d~2'))] + [Commit.new(project.repository.commit('bcf03b5d')), + Commit.new(project.repository.commit('bcf03b5d~1')), + Commit.new(project.repository.commit('bcf03b5d~2'))] end st_diffs do project.repo.diff("bcf03b5d~3", "bcf03b5d") @@ -120,6 +120,7 @@ FactoryGirl.define do factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] trait :on_commit do + project factory: :project_with_code commit_id "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" noteable_type "Commit" end @@ -129,6 +130,7 @@ FactoryGirl.define do end trait :on_merge_request do + project factory: :project_with_code noteable_id 1 noteable_type "MergeRequest" end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7713a33d..6cf777be 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Commit do - let(:commit) { create(:project).repository.commit } + let(:commit) { create(:project_with_code).repository.commit } describe '#title' do diff --git a/spec/models/gollum_wiki_spec.rb b/spec/models/gollum_wiki_spec.rb index 87601683..aa850dfd 100644 --- a/spec/models/gollum_wiki_spec.rb +++ b/spec/models/gollum_wiki_spec.rb @@ -81,7 +81,7 @@ describe GollumWiki do end it "raises CouldNotCreateWikiError if it can't create the wiki repository" do - Gitlab::Shell.any_instance.stub(:add_repository).and_return(false) + GollumWiki.any_instance.stub(:init_repo).and_return(false) expect { GollumWiki.new(project, user).wiki }.to raise_exception(GollumWiki::CouldNotCreateWikiError) end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 0f81347d..19be8029 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -17,11 +17,12 @@ module TestEnv repos_path = Rails.root.join('tmp', 'test-git-base-path') Gitlab.config.gitlab_shell.stub(repos_path: repos_path) - Gitlab::Shell.any_instance.stub(:add_repository) do |path| + GollumWiki.any_instance.stub(:init_repo) do |path| create_temp_repo(File.join(repos_path, "#{path}.git")) end Gitlab::Shell.any_instance.stub( + add_repository: true, mv_repository: true, remove_repository: true, add_key: true, From f5dec306fda53a6b2e90f64c0407077ab0022e8f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 18:16:08 +0300 Subject: [PATCH 21/25] Use project_with_code factory where necessary --- app/views/admin/projects/show.html.haml | 23 +++++++++++-------- .../steps/project/project_browse_commits.rb | 2 +- spec/controllers/commit_controller_spec.rb | 2 +- spec/controllers/commits_controller_spec.rb | 2 +- .../merge_requests_controller_spec.rb | 2 +- spec/controllers/tree_controller_spec.rb | 2 +- .../features/gitlab_flavored_markdown_spec.rb | 2 +- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- 8 files changed, 20 insertions(+), 17 deletions(-) diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 27c687bb..92b89601 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -46,18 +46,21 @@ %span.light ssh: %strong = link_to @project.ssh_url_to_repo - %li - %span.light fs: - %strong - = @repository.path_to_repo + - if @project.repository.exists? + %li + %span.light fs: + %strong + = @repository.path_to_repo - %li - %span.light last commit: - %strong - - if @repository + %li + %span.light last commit: + %strong = last_commit(@project) - - else - never + - else + %li + %span.light repository: + %strong.cred + does not exist %li %span.light access: diff --git a/features/steps/project/project_browse_commits.rb b/features/steps/project/project_browse_commits.rb index 3433c2ba..b4c595fa 100644 --- a/features/steps/project/project_browse_commits.rb +++ b/features/steps/project/project_browse_commits.rb @@ -15,7 +15,7 @@ class ProjectBrowseCommits < Spinach::FeatureSteps end Then 'I see commits atom feed' do - commit = CommitDecorator.decorate(@project.repository.commit) + commit = @project.repository.commit page.response_headers['Content-Type'].should have_content("application/atom+xml") page.body.should have_selector("title", :text => "Recent commits to #{@project.name}") page.body.should have_selector("author email", :text => commit.author_email) diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb index 7bf13822..5fffbf0e 100644 --- a/spec/controllers/commit_controller_spec.rb +++ b/spec/controllers/commit_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe CommitController do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:user) { create(:user) } let(:commit) { project.repository.last_commit_for("master") } diff --git a/spec/controllers/commits_controller_spec.rb b/spec/controllers/commits_controller_spec.rb index 99cbcd13..ce402917 100644 --- a/spec/controllers/commits_controller_spec.rb +++ b/spec/controllers/commits_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe CommitsController do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:user) { create(:user) } before do diff --git a/spec/controllers/merge_requests_controller_spec.rb b/spec/controllers/merge_requests_controller_spec.rb index 37e36efc..e8dd9bf9 100644 --- a/spec/controllers/merge_requests_controller_spec.rb +++ b/spec/controllers/merge_requests_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MergeRequestsController do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, project: project, target_branch: "bcf03b5d~3", source_branch: "bcf03b5d") } diff --git a/spec/controllers/tree_controller_spec.rb b/spec/controllers/tree_controller_spec.rb index 81c7656d..8232f147 100644 --- a/spec/controllers/tree_controller_spec.rb +++ b/spec/controllers/tree_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe TreeController do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:user) { create(:user) } before do diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index a57e34ac..653ff865 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "Gitlab Flavored Markdown" do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, project: project) } let(:fred) do diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 234608c1..e67e211c 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) { 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) } From adccf3b49912df4ea630eb6e96e34204ddf932e6 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 18:24:05 +0300 Subject: [PATCH 22/25] fix facotries --- spec/factories.rb | 2 +- spec/features/notes_on_merge_requests_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 76bd3ebb..3205cabd 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -77,7 +77,7 @@ FactoryGirl.define do factory :merge_request do title author - project + project factory: :project_with_code source_branch "master" target_branch "stable" diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index d48dffc0..82f44279 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "On a merge request", js: true do - let!(:project) { create(:project) } + let!(:project) { create(:project_with_code) } let!(:merge_request) { create(:merge_request, project: project) } before do @@ -83,7 +83,7 @@ end describe "On a merge request diff", js: true, focus: true do - let!(:project) { create(:project) } + let!(:project) { create(:project_with_code) } let!(:merge_request) { create(:merge_request_with_diffs, project: project) } before do From 3b88636d3c3a85d8dea83149b1f401d505d84d60 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 18:35:29 +0300 Subject: [PATCH 23/25] fix tests --- spec/features/profile_spec.rb | 5 +++-- spec/features/security/project_access_spec.rb | 2 +- spec/support/test_env.rb | 6 +----- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index c18d8f92..51b95c25 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -12,8 +12,9 @@ describe "Profile account page" do Gitlab.config.gitlab.stub(:signup_enabled).and_return(true) visit account_profile_path end + it { page.should have_content("Remove account") } - + it "should delete the account", js: true do expect { click_link "Delete account" }.to change {User.count}.by(-1) current_path.should == new_user_session_path @@ -45,4 +46,4 @@ describe "Profile account page" do current_path.should == account_profile_path end end -end \ No newline at end of file +end diff --git a/spec/features/security/project_access_spec.rb b/spec/features/security/project_access_spec.rb index 179ebffc..cfbb8f13 100644 --- a/spec/features/security/project_access_spec.rb +++ b/spec/features/security/project_access_spec.rb @@ -14,7 +14,7 @@ describe "Application access" do end describe "Project" do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:master) { create(:user) } let(:guest) { create(:user) } diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 19be8029..370094d3 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -29,16 +29,12 @@ module TestEnv remove_key: true ) - fake_satellite = stub( + Gitlab::Satellite::Satellite.any_instance.stub( exists?: true, destroy: true, create: true ) - Project.any_instance.stub( - satellite: fake_satellite - ) - MergeRequest.any_instance.stub( check_if_can_be_merged: true ) From f536c133550b74b3083c66adaebf0ae6d425f81b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 19:06:47 +0300 Subject: [PATCH 24/25] fixed test using repo with commits but old factory --- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- spec/lib/git/commit_spec.rb | 3 +-- spec/lib/git/repository_spec.rb | 3 +-- spec/mailers/notify_spec.rb | 13 +++---------- spec/requests/api/merge_requests_spec.rb | 2 +- spec/requests/api/projects_spec.rb | 2 +- spec/services/git_push_service_spec.rb | 2 +- spec/workers/post_receive_spec.rb | 2 +- 8 files changed, 10 insertions(+), 19 deletions(-) diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index e67e211c..4140ba48 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -4,7 +4,7 @@ describe GitlabMarkdownHelper do include ApplicationHelper include IssuesHelper - let!(:project) { create(:project) } + let!(:project) { create(:project_with_code) } let(:user) { create(:user, username: 'gfm') } let(:commit) { project.repository.commit } diff --git a/spec/lib/git/commit_spec.rb b/spec/lib/git/commit_spec.rb index 93f579d3..475bc359 100644 --- a/spec/lib/git/commit_spec.rb +++ b/spec/lib/git/commit_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Commit do - let(:commit) { create(:project).repository.commit } + let(:commit) { create(:project_with_code).repository.commit } describe "Commit info" do before do @@ -45,6 +45,5 @@ describe Gitlab::Git::Commit do it { should respond_to(:commits_since) } it { should respond_to(:commits_between) } it { should respond_to(:commits) } - it { should respond_to(:compare) } end end diff --git a/spec/lib/git/repository_spec.rb b/spec/lib/git/repository_spec.rb index e0ff93ea..b2b6f196 100644 --- a/spec/lib/git/repository_spec.rb +++ b/spec/lib/git/repository_spec.rb @@ -1,8 +1,7 @@ require "spec_helper" describe Gitlab::Git::Repository do - let(:project) { create(:project) } - let(:repository) { project.repository } + let(:repository) { Gitlab::Git::Repository.new('gitlabhq', 'master') } describe "Respond to" do subject { repository } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 472458e8..84ce7e86 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -5,7 +5,7 @@ describe Notify do include EmailSpec::Matchers let(:recipient) { create(:user, email: 'recipient@example.com') } - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } shared_examples 'a multiple recipients email' do it 'is sent to the given recipient' do @@ -277,14 +277,7 @@ describe Notify do end describe 'on a commit' do - let(:commit) do - mock(:commit).tap do |commit| - commit.stub(:id).and_return('fauxsha1') - commit.stub(:project).and_return(project) - commit.stub(:short_id).and_return('fauxsha1') - commit.stub(:safe_message).and_return('some message') - end - end + let(:commit) { project.repository.commit } before(:each) { note.stub(:noteable).and_return(commit) } @@ -297,7 +290,7 @@ describe Notify do end it 'contains a link to the commit' do - should have_body_text /fauxsha1/ + should have_body_text commit.short_id end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index e7af056a..25bbd986 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::API do include ApiHelpers let(:user) { create(:user ) } - let!(:project) { create(:project, namespace: user.namespace ) } + let!(:project) { create(:project_with_code, creator_id: user.id) } let!(:merge_request) { create(:merge_request, author: user, assignee: user, project: project, title: "Test") } before { project.team << [user, :reporters] } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index cddb7264..aca3919a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::API do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:admin) { create(:admin) } - let!(:project) { create(:project, namespace: user.namespace ) } + let!(:project) { create(:project_with_code, creator_id: user.id) } let!(:hook) { create(:project_hook, project: project, url: "http://example.com") } let!(:snippet) { create(:snippet, author: user, project: project, title: 'example') } let!(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 9fc5fd62..286a8cda 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe GitPushService do let (:user) { create :user } - let (:project) { create :project } + let (:project) { create :project_with_code } let (:service) { GitPushService.new } before do diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index a4751bd0..46e86dbe 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -9,7 +9,7 @@ describe PostReceive do end context "web hook" do - let(:project) { create(:project) } + let(:project) { create(:project_with_code) } let(:key) { create(:key, user: project.owner) } let(:key_id) { key.shell_id } From 8a6bf09ad0f95a859406c86c3a925f824dfbd633 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 1 Apr 2013 20:35:41 +0300 Subject: [PATCH 25/25] Pass project into factory for teams tests --- features/steps/userteams/userteams.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/steps/userteams/userteams.rb b/features/steps/userteams/userteams.rb index 70ad3cca..9a86572e 100644 --- a/features/steps/userteams/userteams.rb +++ b/features/steps/userteams/userteams.rb @@ -132,7 +132,7 @@ class Userteams < Spinach::FeatureSteps team = UserTeam.last team.projects.each do |project| team.members.each do |member| - 3.times { project.merge_requests << create(:merge_request, assignee: member) } + 3.times { create(:merge_request, assignee: member, project: project) } end end end @@ -157,7 +157,7 @@ class Userteams < Spinach::FeatureSteps team = UserTeam.last team.projects.each do |project| team.members.each do |member| - 3.times { project.merge_requests << create(:merge_request, assignee: member) } + 3.times { create(:merge_request, assignee: member, project: project) } end end end