From 5f4445c3d384741c45242f077b3c0dbf76234ee8 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 2 Apr 2013 21:30:36 +0300
Subject: [PATCH] store commits for MR as array of hashes

---
 app/controllers/refs_controller.rb |  2 +-
 app/models/merge_request.rb        | 30 +++++--------
 app/models/tree.rb                 | 32 ++++----------
 app/services/git_push_service.rb   |  2 +-
 app/views/commits/_diffs.html.haml |  2 +-
 lib/extracts_path.rb               |  2 +-
 lib/gitlab/git/commit.rb           | 71 +++++++++++++++++++++---------
 lib/gitlab/git/tree.rb             | 38 ++++++++++++++++
 8 files changed, 113 insertions(+), 66 deletions(-)
 create mode 100644 lib/gitlab/git/tree.rb

diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb
index eb8d1e19616..c67912b97a6 100644
--- a/app/controllers/refs_controller.rb
+++ b/app/controllers/refs_controller.rb
@@ -48,7 +48,7 @@ class RefsController < ProjectResourceController
 
     @repo = project.repository
     @commit = @repo.commit(@ref)
-    @tree = Tree.new(@commit.tree, @ref, params[:path])
+    @tree = Tree.new(@repo, @commit.id, @ref, params[:path])
     @hex_path = Digest::SHA1.hexdigest(params[:path] || "")
 
     if params[:path]
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6ce944173cd..0bab88313c1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -152,17 +152,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def 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
+    load_commits(st_commits) || []
   end
 
   def probably_merged?
@@ -172,13 +162,7 @@ class MergeRequest < ActiveRecord::Base
 
   def reloaded_commits
     if opened? && unmerged_commits.any?
-      # we need to reset st_commits field first
-      # in order to prevent internal rails comparison
-      self.st_commits = []
-      save
-
-      # Then we can safely write unmerged commits
-      self.st_commits = unmerged_commits
+      self.st_commits = dump_commits(unmerged_commits)
       save
     end
     commits
@@ -228,4 +212,14 @@ class MergeRequest < ActiveRecord::Base
   def last_commit_short_sha
     @last_commit_short_sha ||= last_commit.sha[0..10]
   end
+
+  private
+
+  def dump_commits(commits)
+    commits.map(&:to_hash)
+  end
+
+  def load_commits(array)
+    array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash)) }
+  end
 end
diff --git a/app/models/tree.rb b/app/models/tree.rb
index 4b6c5b133e9..e726c596f7e 100644
--- a/app/models/tree.rb
+++ b/app/models/tree.rb
@@ -1,37 +1,21 @@
 class Tree
-  include Linguist::BlobHelper
-
   attr_accessor :path, :tree, :ref
 
-  delegate  :contents, :basename, :name, :data, :mime_type,
-            :mode, :size, :text?, :colorize, to: :tree
-
-  def initialize(raw_tree, ref = nil, path = nil)
-    @ref, @path = ref, path
-    @tree = if path.present?
-              raw_tree / path
-            else
-              raw_tree
-            end
-  end
-
-  def is_blob?
-    tree.is_a?(Grit::Blob)
+  def initialize(repository, sha, ref = nil, path = nil)
+    @raw = Gitlab::Git::Tree.new(repository, sha, ref, path)
   end
 
   def invalid?
-    tree.nil?
+    @raw.nil?
   end
 
-  def empty?
-    data.blank?
+  def method_missing(m, *args, &block)
+    @raw.send(m, *args, &block)
   end
 
-  def up_dir?
-    path.present?
-  end
+  def respond_to?(method)
+    return true if @raw.respond_to?(method)
 
-  def readme
-    @readme ||= contents.find { |c| c.is_a?(Grit::Blob) and c.name =~ /^readme/i }
+    super
   end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 383e6398b74..e8b32f52ce1 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -104,7 +104,7 @@ class GitPushService
       data[:commits] << {
         id: commit.id,
         message: commit.safe_message,
-        timestamp: commit.date.xmlschema,
+        timestamp: commit.committed_date.xmlschema,
         url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/#{commit.id}",
         author: {
           name: commit.author_name,
diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml
index b2da4796db6..9d57edb1d97 100644
--- a/app/views/commits/_diffs.html.haml
+++ b/app/views/commits/_diffs.html.haml
@@ -16,7 +16,7 @@
   - unless @suppress_diff
     - diffs.each_with_index do |diff, i|
       - next if diff.diff.empty?
-      - file = (@commit.tree / diff.new_path)
+      - file = Tree.new(@repository, @commit.id, @ref, diff.new_path)
       - file = (@commit.prev_commit.tree / diff.old_path) unless file
       - next unless file
       .file{id: "diff-#{i}"}
diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb
index 2e3ab1b2701..c3e2441ff21 100644
--- a/lib/extracts_path.rb
+++ b/lib/extracts_path.rb
@@ -102,7 +102,7 @@ module ExtractsPath
     # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name.
     @commit = @project.repository.commits(@ref, @path, 1, 0).first
 
-    @tree = Tree.new(@commit.tree, @ref, @path)
+    @tree = Tree.new(@project.repository, @commit.id, @ref, @path)
 
     raise InvalidPathError if @tree.invalid?
   rescue RuntimeError, NoMethodError, InvalidPathError
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index 35991a383f6..c691ea3d223 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -4,13 +4,19 @@
 module Gitlab
   module Git
     class Commit
-      attr_accessor :raw_commit, :head, :refs
+      attr_accessor :raw_commit, :head, :refs,
+        :sha, :authored_date, :committed_date, :message,
+        :author_name, :author_email,
+        :committer_name, :committer_email
 
-      delegate  :message, :authored_date, :committed_date, :parents, :sha,
-        :date, :committer, :author, :diffs, :tree, :id, :stats, :to_patch,
+      delegate :parents, :diffs, :tree, :stats, :to_patch,
         to: :raw_commit
 
       class << self
+        def serialize_keys
+          %w(sha authored_date committed_date author_name author_email committer_name committer_email message)
+        end
+
         def find_or_first(repo, commit_id = nil, root_ref)
           commit = if commit_id
                      repo.commit(commit_id)
@@ -73,10 +79,19 @@ module Gitlab
       def initialize(raw_commit, head = nil)
         raise "Nil as raw commit passed" unless raw_commit
 
-        @raw_commit = raw_commit
+        if raw_commit.is_a?(Hash)
+          init_from_hash(raw_commit)
+        else
+          init_from_grit(raw_commit)
+        end
+
         @head = head
       end
 
+      def id
+        sha
+      end
+
       def short_id(length = 10)
         id.to_s[0..length]
       end
@@ -89,27 +104,11 @@ module Gitlab
         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)
@@ -148,6 +147,38 @@ module Gitlab
       def no_commit_message
         "--no commit message"
       end
+
+      def to_hash
+        hash = {}
+
+        keys = Commit.serialize_keys
+
+        keys.each do |key|
+          hash[key] = send(key)
+        end
+
+        hash
+      end
+
+      private
+
+      def init_from_grit(grit)
+        @raw_commit = grit
+        @sha = grit.sha
+        @message = grit.message
+        @authored_date = grit.authored_date
+        @committed_date = grit.committed_date
+        @author_name = grit.author.name
+        @author_email = grit.author.email
+        @committer_name = grit.committer.name
+        @committer_email = grit.committer.email
+      end
+
+      def init_from_hash(hash)
+        Commit.serialize_keys.each do |key|
+          send(:"#{key}=", hash[key])
+        end
+      end
     end
   end
 end
diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb
new file mode 100644
index 00000000000..b81ce550f4c
--- /dev/null
+++ b/lib/gitlab/git/tree.rb
@@ -0,0 +1,38 @@
+module Gitlab
+  module Git
+    class Tree
+      include Linguist::BlobHelper
+
+      attr_accessor :repository, :sha, :path, :ref, :raw_tree
+
+      def initialize(repository, sha, ref = nil, path = nil)
+        @repository, @sha, @ref = repository, sha, ref
+
+        # Load tree from repository
+        @commit = @repository.commit(sha)
+        @raw_tree = @repository.tree(@commit, path)
+      end
+
+      def empty?
+        data.blank?
+      end
+
+      def data
+        raw_tree.data
+      end
+
+      def is_blob?
+        tree.is_a?(Grit::Blob)
+      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
+  end
+end
+
-- 
GitLab