Decouple and refactor GraphCommit

app/controllers/projects_controller.rb

-require Rails.root.join('lib', 'gitlab', 'graph_commit')
+require Rails.root.join('lib', 'gitlab', 'graph', 'json_builder')
 
 class ProjectsController < ProjectResourceController
   skip_before_filter :project, only: [:new, :create]
@@ -79,7 +79,9 @@ class ProjectsController < ProjectResourceController
   end
 
   def graph
-    @days_json, @commits_json = Gitlab::GraphCommit.to_graph(project)
+    graph = Gitlab::Graph::JsonBuilder.new(project)
+
+    @days_json, @commits_json = graph.days_json, graph.commits_json
   end
 
   def destroy

features/steps/project/project_network_graph.rb

@@ -11,8 +11,8 @@ class ProjectNetworkGraph < Spinach::FeatureSteps
   end
 
   And 'I visit project "Shop" network page' do
-    # Stub GraphCommit max_size to speed up test (10 commits vs. 650)
-    Gitlab::GraphCommit.stub(max_count: 10)
+    # Stub Graph::JsonBuilder max_size to speed up test (10 commits vs. 650)
+    Gitlab::Graph::JsonBuilder.stub(max_count: 10)
 
     project = Project.find_by_name("Shop")
     visit graph_project_path(project)

features/steps/shared/paths.rb

@@ -126,8 +126,8 @@ module SharedPaths
   end
 
   Given "I visit my project's network page" do
-    # Stub GraphCommit max_size to speed up test (10 commits vs. 650)
-    Gitlab::GraphCommit.stub(max_count: 10)
+    # Stub Graph::JsonBuilder max_size to speed up test (10 commits vs. 650)
+    Gitlab::Graph::JsonBuilder.stub(max_count: 10)
 
     visit graph_project_path(@project)
   end

lib/gitlab/graph/'

+require "grit"
+
+module Gitlab
+  module Graph
+    class JsonBuilder
+      attr_accessor :max_count, :days, :commits
+
+      def initialize project
+        @project = project
+        @repo = project.repo
+        @commits = collect_commits(@repo).dup
+        @ref_cache = {}
+
+        @commits.map! { |commit| Graph::Commit.new(Commit.new(commit))}
+        @commits.each { |commit| commit.add_refs(ref_cache, @repo) }
+
+        days = Graph::Commit.index_commits(@commits)
+
+        return @days_json, @commits_json
+      end
+
+      def collect_commits
+      end
+
+      def days_json
+        @days_json = @days.compact.map { |d| [d.day, d.strftime("%b")] }.to_json
+      end
+
+      def commits_json
+        @commits_json = @commits.map(&:to_graph_hash).to_json
+      end
+
+      # Get commits from repository
+      #
+      def collect_commits repo
+        Grit::Commit.find_all(repo, nil, {max_count: self.max_count})
+      end
+
+      def max_count
+        @max_count ||= 650
+      end
+
+      # Method is adding time and space on the
+      # list of commits. As well as returns date list
+      # corelated with time set on commits.
+      #
+      # @param [Array<Graph::Commit>] comits to index
+      #
+      # @return [Array<TimeDate>] list of commit dates corelated with time on commits
+      def index_commits(commits)
+        days, heads = [], []
+        map = {}
+
+        commits.reverse.each_with_index do |c,i|
+          c.time = i
+          days[i] = c.committed_date
+          map[c.id] = c
+          heads += c.refs unless c.refs.nil?
+        end
+
+        heads.select!{|h| h.is_a? Grit::Head or h.is_a? Grit::Remote}
+        # sort heads so the master is top and current branches are closer
+        heads.sort! do |a,b|
+          if a.name == "master"
+            -1
+          elsif b.name == "master"
+            1
+          else
+            b.commit.committed_date <=> a.commit.committed_date
+          end
+        end
+
+        @_reserved = {}
+        days.each_index do |i|
+          @_reserved[i] = []
+        end
+
+        heads.each do |h|
+          if map.include? h.commit.id then
+            place_chain(map[h.commit.id], map)
+          end
+        end
+        days
+      end
+
+      # Add space mark on commit and its parents
+      #
+      # @param [Graph::Commit] the commit object.
+      # @param [Hash<String,Graph::Commit>] map of commits
+      def place_chain(commit, map, parent_time = nil)
+        leaves = take_left_leaves(commit, map)
+        if leaves.empty? then
+          return
+        end
+        space = find_free_space(leaves.last.time..leaves.first.time)
+        leaves.each{|l| l.space = space}
+        # and mark it as reserved
+        min_time = leaves.last.time
+        parents = leaves.last.parents.collect
+        parents.each do |p|
+          if map.include? p.id then
+            parent = map[p.id]
+            if parent.time < min_time then
+              min_time = parent.time
+            end
+          end
+        end
+        if parent_time.nil? then
+          max_time = leaves.first.time
+        else
+          max_time = parent_time - 1
+        end
+        mark_reserved(min_time..max_time, space)
+        # Visit branching chains
+        leaves.each do |l|
+          parents = l.parents.collect
+          .select{|p| map.include? p.id and map[p.id].space == 0}
+          for p in parents
+            place_chain(map[p.id], map, l.time)
+          end
+        end
+      end
+
+      def mark_reserved(time_range, space)
+        for day in time_range
+          @_reserved[day].push(space)
+        end
+      end
+
+      def find_free_space(time_range)
+        reserved = []
+        for day in time_range
+          reserved += @_reserved[day]
+        end
+        space = 1
+        while reserved.include? space do
+          space += 1
+        end
+        space
+      end
+
+      # Takes most left subtree branch of commits
+      # which don't have space mark yet.
+      #
+      # @param [Graph::Commit] the commit object.
+      # @param [Hash<String,Graph::Commit>] map of commits
+      #
+      # @return [Array<Graph::Commit>] list of branch commits
+      def take_left_leaves(commit, map)
+        leaves = []
+        leaves.push(commit)  if commit.space == 0
+        while true
+          parent = commit.parents.collect
+          self.select{|p| map.include? p.id and map[p.id].space == 0}
+          if parent.count == 0 then
+            return leaves
+          else
+            commit = map[parent.first.id]
+            leaves.push(commit)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/graph/commit.rb

+require "grit"
+
+module Gitlab
+  module Graph
+    class Commit
+      include ActionView::Helpers::TagHelper
+
+      attr_accessor :time, :space, :refs
+
+      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]  = Gitlab::Encode.utf8(author.name)
+        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] = escape_once(Gitlab::Encode.utf8(message))
+        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
+  end
+end

lib/gitlab/graph/json_builder.rb

+require "grit"
+
+module Gitlab
+  module Graph
+    class JsonBuilder
+      attr_accessor :days, :commits, :ref_cache, :repo
+
+      def self.max_count
+        @max_count ||= 650
+      end
+
+      def initialize project
+        @project = project
+        @repo = project.repo
+        @ref_cache = {}
+
+        @commits = collect_commits
+        @days = index_commits
+      end
+
+      def days_json
+        @days_json = @days.compact.map { |d| [d.day, d.strftime("%b")] }.to_json
+      end
+
+      def commits_json
+        @commits_json = @commits.map(&:to_graph_hash).to_json
+      end
+
+      protected
+
+      # Get commits from repository
+      #
+      def collect_commits
+        @commits = Grit::Commit.find_all(repo, nil, {max_count: self.class.max_count}).dup
+
+        # Decorate with app/models/commit.rb
+        @commits.map! { |commit| ::Commit.new(commit) }
+
+        # Decorate with lib/gitlab/graph/commit.rb
+        @commits.map! { |commit| Gitlab::Graph::Commit.new(commit) }
+
+        # add refs to each commit
+        @commits.each { |commit| commit.add_refs(ref_cache, repo) }
+
+        @commits
+      end
+
+      # Method is adding time and space on the
+      # list of commits. As well as returns date list
+      # corelated with time set on commits.
+      #
+      # @param [Array<Graph::Commit>] comits to index
+      #
+      # @return [Array<TimeDate>] list of commit dates corelated with time on commits
+      def index_commits
+        days, heads = [], []
+        map = {}
+
+        commits.reverse.each_with_index do |c,i|
+          c.time = i
+          days[i] = c.committed_date
+          map[c.id] = c
+          heads += c.refs unless c.refs.nil?
+        end
+
+        heads.select!{|h| h.is_a? Grit::Head or h.is_a? Grit::Remote}
+        # sort heads so the master is top and current branches are closer
+        heads.sort! do |a,b|
+          if a.name == "master"
+            -1
+          elsif b.name == "master"
+            1
+          else
+            b.commit.committed_date <=> a.commit.committed_date
+          end
+        end
+
+        @_reserved = {}
+        days.each_index do |i|
+          @_reserved[i] = []
+        end
+
+        heads.each do |h|
+          if map.include? h.commit.id then
+            place_chain(map[h.commit.id], map)
+          end
+        end
+
+        days
+      end
+
+      # Add space mark on commit and its parents
+      #
+      # @param [Graph::Commit] the commit object.
+      # @param [Hash<String,Graph::Commit>] map of commits
+      def place_chain(commit, map, parent_time = nil)
+        leaves = take_left_leaves(commit, map)
+        if leaves.empty?
+          return
+        end
+        space = find_free_space(leaves.last.time..leaves.first.time)
+        leaves.each{|l| l.space = space}
+        # and mark it as reserved
+        min_time = leaves.last.time
+        parents = leaves.last.parents.collect
+        parents.each do |p|
+          if map.include? p.id
+            parent = map[p.id]
+            if parent.time < min_time
+              min_time = parent.time
+            end
+          end
+        end
+        if parent_time.nil?
+          max_time = leaves.first.time
+        else
+          max_time = parent_time - 1
+        end
+        mark_reserved(min_time..max_time, space)
+
+        # Visit branching chains
+        leaves.each do |l|
+          parents = l.parents.collect.select{|p| map.include? p.id and map[p.id].space == 0}
+          for p in parents
+            place_chain(map[p.id], map, l.time)
+          end
+        end
+      end
+
+      def mark_reserved(time_range, space)
+        for day in time_range
+          @_reserved[day].push(space)
+        end
+      end
+
+      def find_free_space(time_range)
+        reserved = []
+        for day in time_range
+          reserved += @_reserved[day]
+        end
+        space = 1
+        while reserved.include? space do
+          space += 1
+        end
+        space
+      end
+
+      # Takes most left subtree branch of commits
+      # which don't have space mark yet.
+      #
+      # @param [Graph::Commit] the commit object.
+      # @param [Hash<String,Graph::Commit>] map of commits
+      #
+      # @return [Array<Graph::Commit>] list of branch commits
+      def take_left_leaves(commit, map)
+        leaves = []
+        leaves.push(commit) if commit.space.zero?
+
+        while true
+          parent = commit.parents.collect.select do |p|
+            map.include? p.id and map[p.id].space == 0
+          end
+
+          return leaves if parent.count.zero?
+
+          commit = map[parent.first.id]
+          leaves.push(commit)
+        end
+      end
+    end
+  end
+end

lib/gitlab/graph_commit.rb

-require "grit"
-
-module Gitlab
-  class GraphCommit
-    attr_accessor :time, :space, :refs
-
-    include ActionView::Helpers::TagHelper
-
-    def self.to_graph(project)
-      @repo = project.repo
-
-      commits = collect_commits(@repo).dup
-
-      ref_cache = {}
-
-      commits.map! { |commit| GraphCommit.new(Commit.new(commit))}
-      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
-
-    # Get commits from repository
-    #
-    def self.collect_commits repo
-      Grit::Commit.find_all(repo, nil, {max_count: self.max_count})
-    end
-
-    def self.max_count
-      @max_count ||= 650
-    end
-
-    # Method is adding time and space on the
-    # list of commits. As well as returns date list
-    # corelated with time set on commits.
-    #
-    # @param [Array<GraphCommit>] comits to index
-    #
-    # @return [Array<TimeDate>] list of commit dates corelated with time on commits
-    def self.index_commits(commits)
-      days, heads = [], []
-      map = {}
-
-      commits.reverse.each_with_index do |c,i|
-        c.time = i
-        days[i] = c.committed_date
-        map[c.id] = c
-        heads += c.refs unless c.refs.nil?
-      end
-
-      heads.select!{|h| h.is_a? Grit::Head or h.is_a? Grit::Remote}
-      # sort heads so the master is top and current branches are closer
-      heads.sort! do |a,b|
-        if a.name == "master"
-          -1
-        elsif b.name == "master"
-          1
-        else
-          b.commit.committed_date <=> a.commit.committed_date
-        end
-      end
-
-      @_reserved = {}
-      days.each_index do |i|
-        @_reserved[i] = []
-      end
-
-      heads.each do |h|
-        if map.include? h.commit.id then
-          place_chain(map[h.commit.id], map)
-        end
-      end
-      days
-    end
-
-    # Add space mark on commit and its parents
-    #
-    # @param [GraphCommit] the commit object.
-    # @param [Hash<String,GraphCommit>] map of commits
-    def self.place_chain(commit, map, parent_time = nil)
-      leaves = take_left_leaves(commit, map)
-      if leaves.empty? then
-        return
-      end
-      space = find_free_space(leaves.last.time..leaves.first.time)
-      leaves.each{|l| l.space = space}
-      # and mark it as reserved
-      min_time = leaves.last.time
-      parents = leaves.last.parents.collect
-      parents.each do |p|
-        if map.include? p.id then
-          parent = map[p.id]
-          if parent.time < min_time then
-            min_time = parent.time
-          end
-        end
-      end
-      if parent_time.nil? then
-        max_time = leaves.first.time
-      else
-        max_time = parent_time - 1
-      end
-      mark_reserved(min_time..max_time, space)
-      # Visit branching chains
-      leaves.each do |l|
-        parents = l.parents.collect
-          .select{|p| map.include? p.id and map[p.id].space == 0}
-        for p in parents
-          place_chain(map[p.id], map, l.time)
-        end
-      end
-    end
-
-    def self.mark_reserved(time_range, space)
-      for day in time_range
-        @_reserved[day].push(space)
-      end
-    end
-
-    def self.find_free_space(time_range)
-      reserved = []
-      for day in time_range
-          reserved += @_reserved[day]
-      end
-      space = 1
-      while reserved.include? space do
-        space += 1
-      end
-      space
-    end
-
-    # Takes most left subtree branch of commits
-    # which don't have space mark yet.
-    #
-    # @param [GraphCommit] the commit object.
-    # @param [Hash<String,GraphCommit>] map of commits
-    #
-    # @return [Array<GraphCommit>] list of branch commits
-    def self.take_left_leaves(commit, map)
-      leaves = []
-      leaves.push(commit)  if commit.space == 0
-      while true
-        parent = commit.parents.collect
-          .select{|p| map.include? p.id and map[p.id].space == 0}
-        if parent.count == 0 then
-          return leaves
-        else
-          commit = map[parent.first.id]
-          leaves.push(commit)
-        end
-      end
-    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]  = Gitlab::Encode.utf8(author.name)
-      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] = escape_once(Gitlab::Encode.utf8(message))
-      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
-end