From a031813887a203b80006e7fdc3204355fd8d02b7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 27 Nov 2011 17:35:49 +0200 Subject: [PATCH] Commit, network graph refactoring --- .../stylesheets/highlight.black.css.scss | 8 +-- app/controllers/commits_controller.rb | 9 +-- app/controllers/projects_controller.rb | 37 +----------- app/helpers/dashboard_helper.rb | 2 +- app/models/commit.rb | 35 +++++++++++ app/models/repository.rb | 25 +++++--- app/views/projects/_feed.html.haml | 2 +- app/views/refs/_tree_item.html.haml | 2 +- config/initializers/gitlabhq/20_grit_ext.rb | 4 -- lib/commit_ext.rb | 20 ------- lib/graph_commit.rb | 59 ++++++++++++++++--- spec/requests/commits_spec.rb | 6 +- spec/requests/projects_security_spec.rb | 12 ++-- spec/requests/projects_spec.rb | 1 + 14 files changed, 125 insertions(+), 97 deletions(-) delete mode 100644 lib/commit_ext.rb diff --git a/app/assets/stylesheets/highlight.black.css.scss b/app/assets/stylesheets/highlight.black.css.scss index c8d0a485..773bec48 100644 --- a/app/assets/stylesheets/highlight.black.css.scss +++ b/app/assets/stylesheets/highlight.black.css.scss @@ -11,7 +11,7 @@ .cm { color: #888888 } /* Comment.Multiline */ .cp { color: #cc0000; font-weight: bold } /* Comment.Preproc */ .c1 { color: #888888 } /* Comment.Single */ - .cs { color: #cc0000; font-weight: bold; background-color: auto } /* Comment.Special */ + .cs { color: #cc0000; font-weight: bold; background-color: transparent } /* Comment.Special */ .gd { color: #000000; background-color: #ffdddd } /* Generic.Deleted */ .ge { font-style: italic } /* Generic.Emph */ .gr { color: #aa0000 } /* Generic.Error */ @@ -30,7 +30,7 @@ .highlight .kt{color:#458;font-weight:bold;} /* Keyword.Type */ .m { color: #0000DD; font-weight: bold } /* Literal.Number */ .p { color: #eee; } - .s { color: #dd2200; background-color: auto } /* Literal.String */ + .s { color: #dd2200; background-color: transparent } /* Literal.String */ .highlight .na{color:#008080;} /* Name.Attribute */ .highlight .nb{color:#0086B3;} /* Name.Builtin */ .highlight .nc{color:#4d3;font-weight:bold;} /* Name.Class */ @@ -48,9 +48,9 @@ .mh { color: #0000DD; font-weight: bold } /* Literal.Number.Hex */ .highlight .mi {color:#099;} /* Literal.Number.Integer */ .mo { color: #0000DD; font-weight: bold } /* Literal.Number.Oct */ - .sb { color: #dd2200; background-color: auto } /* Literal.String.Backtick */ + .sb { color: #dd2200; background-color: transparent; } /* Literal.String.Backtick */ .highlight .sc{color:#d14;} /* Literal.String.Char */ - .sd { color: #dd2200; background-color: auto } /* Literal.String.Doc */ + .sd { color: #dd2200; background-color: transparent; } /* Literal.String.Doc */ .highlight .s2{color:orange;} /* Literal.String.Double */ .highlight .se{color:orange;} /* Literal.String.Escape */ .highlight .sh{color:orange;} /* Literal.String.Heredoc */ diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index 37652739..f6a19cc7 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -13,12 +13,7 @@ class CommitsController < ApplicationController def index @repo = project.repo @limit, @offset = (params[:limit] || 20), (params[:offset] || 0) - - @commits = if params[:path] - @repo.log(@ref, params[:path], :max_count => @limit, :skip => @offset) - else - @repo.commits(@ref, @limit, @offset) - end + @commits = @project.commits(@ref, params[:path], @limit, @offset) respond_to do |format| format.html # index.html.erb @@ -28,7 +23,7 @@ class CommitsController < ApplicationController end def show - @commit = project.repo.commits(params[:id]).first + @commit = project.commit(params[:id]) @notes = project.commit_notes(@commit).fresh.limit(20) @note = @project.build_commit_note(@commit) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index ee5f731e..717f8595 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -86,31 +86,7 @@ class ProjectsController < ApplicationController end def graph - @repo = project.repo - commits = Grit::Commit.find_all(@repo, nil, {:max_count => 650}) - ref_cache = {} - commits.collect! do |commit| - add_refs(commit, ref_cache) - GraphCommit.new(commit) - end - - days = GraphCommit.index_commits(commits) - @days_json = days.compact.collect{|d| [d.day, d.strftime("%b")] }.to_json - @commits_json = commits.collect do |c| - h = {} - h[:parents] = c.parents.collect do |p| - [p.id,0,0] - end - h[:author] = c.author.name.force_encoding("UTF-8") - h[:time] = c.time - h[:space] = c.space - h[:refs] = c.refs.collect{|r|r.name}.join(" ") unless c.refs.nil? - h[:id] = c.sha - h[:date] = c.date - h[:message] = c.message.force_encoding("UTF-8") - h[:login] = c.author.email - h - end.to_json + @days_json, @commits_json = GraphCommit.to_graph(project) end def destroy @@ -123,17 +99,6 @@ class ProjectsController < ApplicationController protected - def add_refs(commit, ref_cache) - if ref_cache.empty? - @repo.refs.each do |ref| - ref_cache[ref.commit.id] ||= [] - ref_cache[ref.commit.id] << ref - end - end - commit.refs = ref_cache[commit.id] if ref_cache.include? commit.id - commit.refs ||= [] - end - def project @project ||= Project.find_by_code(params[:id]) end diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 6666cabc..78cc2343 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -2,7 +2,7 @@ module DashboardHelper def dashboard_feed_path(project, object) case object.class.name.to_s when "Issue" then project_issue_path(project, project.issues.find(object.id)) - when "Grit::Commit" then project_commit_path(project, project.repo.commits(object.id).first) + when "Commit" then project_commit_path(project, project.repo.commits(object.id).first) when "Note" then note = object diff --git a/app/models/commit.rb b/app/models/commit.rb index 09030cab..6d724bc8 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -1,2 +1,37 @@ class Commit + attr_accessor :commit + attr_accessor :head + + delegate :message, + :committed_date, + :parents, + :sha, + :date, + :author, + :message, + :diffs, + :tree, + :id, + :to => :commit + + def initialize(raw_commit, head = nil) + @commit = raw_commit + @head = head + end + + def safe_message + message.force_encoding(Encoding::UTF_8) + end + + def created_at + committed_date + end + + def author_email + author.email.force_encoding(Encoding::UTF_8) + end + + def author_name + author.name.force_encoding(Encoding::UTF_8) + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 7c245a87..b87a1fba 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -64,16 +64,17 @@ class Repository end def commit(commit_id = nil) - if commit_id - repo.commits(commit_id).first - else - repo.commits.first - end + commit = if commit_id + repo.commits(commit_id).first + else + repo.commits.first + end + Commit.new(commit) if commit end def fresh_commits(n = 10) commits = heads.map do |h| - repo.commits(h.name, n).each { |c| c.head = h } + repo.commits(h.name, n).map { |c| Commit.new(c, h) } end.flatten.uniq { |c| c.id } commits.sort! do |x, y| @@ -85,7 +86,7 @@ class Repository def commits_since(date) commits = heads.map do |h| - repo.log(h.name, nil, :since => date).each { |c| c.head = 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| @@ -94,4 +95,14 @@ class Repository commits end + + def commits(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 end diff --git a/app/views/projects/_feed.html.haml b/app/views/projects/_feed.html.haml index a5cb0ad1..6e86b4c5 100644 --- a/app/views/projects/_feed.html.haml +++ b/app/views/projects/_feed.html.haml @@ -10,6 +10,6 @@ .right - klass = update.class.to_s.split("::").last.downcase %span.tag{ :class => klass }= klass - - if update.kind_of?(Grit::Commit) + - if update.kind_of?(Commit) %span.tag.commit= update.head.name diff --git a/app/views/refs/_tree_item.html.haml b/app/views/refs/_tree_item.html.haml index 75e9b906..334086af 100644 --- a/app/views/refs/_tree_item.html.haml +++ b/app/views/refs/_tree_item.html.haml @@ -1,5 +1,5 @@ - file = params[:path] ? File.join(params[:path], content.name) : content.name -- content_commit = @project.repo.log(@commit.id, file, :max_count => 1).last +- content_commit = @project.commits(@commit.id, file, 1).last - return unless content_commit %tr{ :class => "tree-item", :url => tree_file_project_ref_path(@project, @ref, file) } %td.tree-item-file-name diff --git a/config/initializers/gitlabhq/20_grit_ext.rb b/config/initializers/gitlabhq/20_grit_ext.rb index 102078e9..35684453 100644 --- a/config/initializers/gitlabhq/20_grit_ext.rb +++ b/config/initializers/gitlabhq/20_grit_ext.rb @@ -7,9 +7,5 @@ Grit::Blob.class_eval do include Utils::Colorize end -Grit::Commit.class_eval do - include CommitExt -end - Grit::Git.git_timeout = GIT_OPTS["git_timeout"] Grit::Git.git_max_size = GIT_OPTS["git_max_size"] diff --git a/lib/commit_ext.rb b/lib/commit_ext.rb deleted file mode 100644 index 0e045911..00000000 --- a/lib/commit_ext.rb +++ /dev/null @@ -1,20 +0,0 @@ -module CommitExt - attr_accessor :head - attr_accessor :refs - - def safe_message - message.force_encoding(Encoding::UTF_8) - end - - def created_at - committed_date - end - - def author_email - author.email.force_encoding(Encoding::UTF_8) - end - - def author_name - author.name.force_encoding(Encoding::UTF_8) - end -end diff --git a/lib/graph_commit.rb b/lib/graph_commit.rb index 69368f7a..18b17022 100644 --- a/lib/graph_commit.rb +++ b/lib/graph_commit.rb @@ -2,14 +2,22 @@ require "grit" class GraphCommit attr_accessor :time, :space - def initialize(commit) - @_commit = commit - @time = -1 - @space = 0 - end + attr_accessor :refs - def method_missing(m, *args, &block) - @_commit.send(m, *args, &block) + def self.to_graph(project) + @repo = project.repo + commits = Grit::Commit.find_all(@repo, nil, {:max_count => 650}) + + ref_cache = {} + + commits.map! {|c| GraphCommit.new(Commit.new(c))} + commits.each { |commit| commit.add_refs(ref_cache, @repo) } + + days = GraphCommit.index_commits(commits) + @days_json = days.compact.collect{|d| [d.day, d.strftime("%b")] }.to_json + @commits_json = commits.map(&:to_graph_hash).to_json + + return @days_json, @commits_json end # Method is adding time and space on the @@ -72,4 +80,41 @@ class GraphCommit marks.compact.max end + + def initialize(commit) + @_commit = commit + @time = -1 + @space = 0 + end + + def method_missing(m, *args, &block) + @_commit.send(m, *args, &block) + end + + def to_graph_hash + h = {} + h[:parents] = self.parents.collect do |p| + [p.id,0,0] + end + h[:author] = author.name.force_encoding("UTF-8") + h[:time] = time + h[:space] = space + h[:refs] = refs.collect{|r|r.name}.join(" ") unless refs.nil? + h[:id] = sha + h[:date] = date + h[:message] = message.force_encoding("UTF-8") + h[:login] = author.email + h + end + + def add_refs(ref_cache, repo) + if ref_cache.empty? + repo.refs.each do |ref| + ref_cache[ref.commit.id] ||= [] + ref_cache[ref.commit.id] << ref + end + end + @refs = ref_cache[@_commit.id] if ref_cache.include?(@_commit.id) + @refs ||= [] + end end diff --git a/spec/requests/commits_spec.rb b/spec/requests/commits_spec.rb index e0897632..f79e9753 100644 --- a/spec/requests/commits_spec.rb +++ b/spec/requests/commits_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe "Commits" do let(:project) { Factory :project } - let!(:commit) { project.repo.commits.first } + let!(:commit) { project.commit } before do login_as :user project.add_access(@user, :read) @@ -48,11 +48,11 @@ describe "Commits" do describe "GET /commits/:id" do before do - visit project_commit_path(project, commit) + visit project_commit_path(project, commit.id) end it "should have valid path" do - current_path.should == project_commit_path(project, commit) + current_path.should == project_commit_path(project, commit.id) end end end diff --git a/spec/requests/projects_security_spec.rb b/spec/requests/projects_security_spec.rb index 4d0c8a64..5f17cf40 100644 --- a/spec/requests/projects_security_spec.rb +++ b/spec/requests/projects_security_spec.rb @@ -55,12 +55,12 @@ describe "Projects" do end describe "GET /project_code/commit" do - it { project_commit_path(@project, @project.commit).should be_allowed_for @u1 } - it { project_commit_path(@project, @project.commit).should be_allowed_for @u3 } - it { project_commit_path(@project, @project.commit).should be_denied_for :admin } - it { project_commit_path(@project, @project.commit).should be_denied_for @u2 } - it { project_commit_path(@project, @project.commit).should be_denied_for :user } - it { project_commit_path(@project, @project.commit).should be_denied_for :visitor } + it { project_commit_path(@project, @project.commit.id).should be_allowed_for @u1 } + it { project_commit_path(@project, @project.commit.id).should be_allowed_for @u3 } + it { project_commit_path(@project, @project.commit.id).should be_denied_for :admin } + it { project_commit_path(@project, @project.commit.id).should be_denied_for @u2 } + it { project_commit_path(@project, @project.commit.id).should be_denied_for :user } + it { project_commit_path(@project, @project.commit.id).should be_denied_for :visitor } end describe "GET /project_code/team" do diff --git a/spec/requests/projects_spec.rb b/spec/requests/projects_spec.rb index 4800e8b8..07bbfccd 100644 --- a/spec/requests/projects_spec.rb +++ b/spec/requests/projects_spec.rb @@ -79,6 +79,7 @@ describe "Projects" do end it "should beahave like activities page" do + save_and_open_page within ".project-update" do page.should have_content("master") page.should have_content(@project.commit.author.name)