Merge branch 'feature/merge_request_serialize'

app/controllers/blame_controller.rb

@@ -8,6 +8,7 @@ class BlameController < ProjectResourceController
   before_filter :require_non_empty_project
 
   def show
+    @blob = Gitlab::Git::Blob.new(@repository, @commit.id, @ref, @path)
     @blame = Gitlab::Git::Blame.new(project.repository, @commit.id, @path)
   end
 end

app/controllers/blob_controller.rb

@@ -8,15 +8,6 @@ class BlobController < ProjectResourceController
   before_filter :require_non_empty_project
 
   def show
-    if @tree.is_blob?
-      send_data(
-        @tree.data,
-        type: @tree.mime_type,
-        disposition: 'inline',
-        filename: @tree.name
-      )
-    else
-      not_found!
-    end
+    @blob = Gitlab::Git::Blob.new(@repository, @commit.id, @ref, @path)
   end
 end

app/controllers/raw_controller.rb

+# Controller for viewing a file's raw
+class RawController < ProjectResourceController
+  include ExtractsPath
+
+  # Authorize
+  before_filter :authorize_read_project!
+  before_filter :authorize_code_access!
+  before_filter :require_non_empty_project
+
+  def show
+    @blob = Gitlab::Git::Blob.new(@repository, @commit.id, @ref, @path)
+
+    if @blob.exists?
+      send_data(
+        @blob.data,
+        type: @blob.mime_type,
+        disposition: 'inline',
+        filename: @blob.name
+      )
+    else
+      not_found!
+    end
+  end
+end
+

app/controllers/refs_controller.rb

@@ -30,7 +30,7 @@ class RefsController < ProjectResourceController
   end
 
   def logs_tree
-    contents = @tree.contents
+    contents = @tree.entries
     @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
@@ -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]

app/helpers/tree_helper.rb

@@ -3,9 +3,9 @@ module TreeHelper
   # their corresponding partials
   #
   # contents - A Grit::Tree object for the current tree
-  def render_tree(contents)
+  def render_tree(tree)
     # Render Folders before Files/Submodules
-    folders, files = contents.partition { |v| v.kind_of?(Grit::Tree) }
+    folders, files = tree.trees, tree.blobs
 
     tree = ""
 
@@ -18,7 +18,7 @@ module TreeHelper
                render partial: 'tree/submodule_item', object: f
              else
                # Object is a Blob
-               render partial: 'tree/tree_item', object: f, locals: {type: 'file'}
+               render partial: 'tree/blob_item', object: f, locals: {type: 'file'}
              end
 
       tree += html if html.present?
@@ -91,5 +91,4 @@ module TreeHelper
     file = File.join(tree.path, "..")
     tree_join(tree.ref, file)
   end
-
 end

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

app/models/tree.rb

 class Tree
-  include Linguist::BlobHelper
+  attr_accessor :raw
 
-  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)
-  end
-
-  def invalid?
-    tree.nil?
+  def initialize(repository, sha, ref = nil, path = nil)
+    @raw = Gitlab::Git::Tree.new(repository, sha, ref, path)
   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

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,

app/views/blame/show.html.haml

@@ -15,9 +15,9 @@
     .file_title
       %i.icon-file
       %span.file_name
-        = @tree.name
-        %small= number_to_human_size @tree.size
-      %span.options= render "tree/blob_actions"
+        = @blob.name
+        %small= number_to_human_size @blob.size
+      %span.options= render "blob/actions"
     .file_content.blame
       %table
         - current_line = 1

app/views/tree/_blob_actions.html.haml → app/views/blob/_actions.html.haml

 .btn-group.tree-btn-group
   -# only show edit link for text files
-  - if @tree.text?
+  - if @blob.text?
     = link_to "edit", project_edit_tree_path(@project, @id), class: "btn btn-tiny", disabled: !allowed_tree_edit?
-  = link_to "raw", project_blob_path(@project, @id), class: "btn btn-tiny", target: "_blank"
+  = link_to "raw", project_raw_path(@project, @id), class: "btn btn-tiny", target: "_blank"
   -# only show normal/blame view links for text files
-  - if @tree.text?
+  - if @blob.text?
     - if current_page? project_blame_path(@project, @id)
-      = link_to "normal view", project_tree_path(@project, @id), class: "btn btn-tiny"
+      = link_to "normal view", project_blob_path(@project, @id), class: "btn btn-tiny"
     - else
       = link_to "blame", project_blame_path(@project, @id), class: "btn btn-tiny"
   = link_to "history", project_commits_path(@project, @id), class: "btn btn-tiny"

app/views/blob/_blob.html.haml

+%ul.breadcrumb
+  %li
+    %i.icon-angle-right
+    = link_to project_tree_path(@project, @ref) do
+      = @project.path
+  - tree_breadcrumbs(@tree, 6) do |title, path|
+    \/
+    %li
+      - if path
+        = link_to truncate(title, length: 40), project_tree_path(@project, path)
+      - else
+        = link_to title, '#'
+
+%div#tree-content-holder.tree-content-holder
+  .file_holder
+    .file_title
+      %i.icon-file
+      %span.file_name
+        = blob.name
+        %small= number_to_human_size blob.size
+      %span.options= render "actions"
+    - if blob.text?
+      = render "text", blob: blob
+    - elsif blob.image?
+      = render "image", blob: blob
+    - else
+      = render "download", blob: blob

app/views/blob/show.html.haml

+%div.tree-ref-holder
+  = render 'shared/ref_switcher', destination: 'tree', path: @path
+%div#tree-holder.tree-holder
+  = render 'blob', blob: @blob

app/views/blob/show.js.haml

+:plain
+  // Load Files list
+  $("#tree-holder").html("#{escape_javascript(render(partial: "blob", locals: {blob: @blob}))}");
+  $("#tree-content-holder").show("slide", { direction: "right" }, 400);
+  $('.project-refs-form #path').val("#{@path}");
+
+  //  Load last commit log for each file in tree
+  $('#tree-slider').waitForImages(function() {
+    ajaxGet('#{@logs_path}');
+  });

app/views/commits/_diffs.html.haml

@@ -16,16 +16,16 @@
   - unless @suppress_diff
     - diffs.each_with_index do |diff, i|
       - next if diff.diff.empty?
-      - file = (@commit.tree / diff.new_path)
-      - file = (@commit.prev_commit.tree / diff.old_path) unless file
+      - file = Gitlab::Git::Blob.new(@repository, @commit.id, @ref, diff.new_path)
+      - file = Gitlab::Git::Blob.new(@repository, @commit.parent_id, @ref, diff.old_path) unless file.exists?
       - next unless file
       .file{id: "diff-#{i}"}
         .header
           - if diff.deleted_file
             %span= diff.old_path
 
-            - if @commit.prev_commit
-              = link_to project_tree_path(@project, tree_join(@commit.prev_commit_id, diff.new_path)), {:class => 'btn btn-tiny pull-right view-file'} do
+            - if @commit.parent_ids.present?
+              = link_to project_tree_path(@project, tree_join(@commit.parent_id, diff.new_path)), {:class => 'btn btn-tiny pull-right view-file'} do
                 View file @
                 %span.commit-short-id= @commit.short_id(6)
           - else
@@ -43,7 +43,7 @@
           - if file.text?
             = render "commits/text_file", diff: diff, index: i
           - elsif file.image?
-            - old_file = (@commit.prev_commit.tree / diff.old_path) if !@commit.prev_commit.nil?
+            - old_file = Gitlab::Git::Blob.new(@repository, @commit.parent_id, @ref, diff.old_path) if @commit.parent_id
             = render "commits/image", diff: diff, old_file: old_file, file: file, index: i
           - else
             %p.nothing_here_message No preview for this file type

app/views/graph/show.json.erb

@@ -7,9 +7,9 @@
       {
         parents: parents_zip_spaces(c.parents(@graph.map), c.parent_spaces),
         author: {
-          name: c.author.name,
-          email: c.author.email,
-          icon: gravatar_icon(c.author.email, 20)
+          name: c.author_name,
+          email: c.author_email,
+          icon: gravatar_icon(c.author_email, 20)
         },
         time: c.time,
         space: c.spaces.first,

app/views/tree/_blob.html.haml

-.file_holder
-  .file_title
-    %i.icon-file
-    %span.file_name
-      = blob.name
-      %small= number_to_human_size blob.size
-    %span.options= render "tree/blob_actions"
-  - if blob.text?
-    = render "tree/blob/text", blob: blob
-  - elsif blob.image?
-    = render "tree/blob/image", blob: blob
-  - else
-    = render "tree/blob/download", blob: blob

app/views/tree/_blob_item.html.haml

+%tr{ class: "tree-item #{tree_hex_class(blob_item)}" }
+  %td.tree-item-file-name
+    = tree_icon(type)
+    %strong= link_to truncate(blob_item.name, length: 40), project_blob_path(@project, tree_join(@id || @commit.id, blob_item.name))
+  %td.tree_time_ago.cgray
+    %span.log_loading.hide
+      Loading commit data...
+      = image_tag "ajax_loader_tree.gif", width: 14
+  %td.tree_commit{ colspan: 2 }

app/views/tree/_tree.html.haml

@@ -12,9 +12,6 @@
         = link_to title, '#'
 
 %div#tree-content-holder.tree-content-holder
-  - if tree.is_blob?
-    = render "tree/blob", blob: tree
-  - else
   %table#tree-slider{class: "table_#{@hex_path} tree-table" }
     %thead
       %tr
@@ -32,15 +29,14 @@
         %td
         %td
 
-      = render_tree(tree.contents)
+    = render_tree(tree)
 
   - if tree.readme
     = render "tree/readme", readme: tree.readme
 
 %div.tree_progress
 
-- unless tree.is_blob?
-  :javascript
+:javascript
   // Load last commit log for each file in tree
   $(window).load(function(){
     ajaxGet('#{@logs_path}');

app/workers/post_receive.rb

@@ -23,7 +23,7 @@ class PostReceive
 
     user = if identifier.blank?
              # Local push from gitlab
-             email = project.repository.commit(newrev).author.email rescue nil
+             email = project.repository.commit(newrev).author_email rescue nil
              User.find_by_email(email) if email
 
            elsif identifier =~ /\Auser-\d+\Z/

config/routes.rb

@@ -170,6 +170,7 @@ Gitlab::Application.routes.draw do
     end
 
     resources :blob,    only: [:show], constraints: {id: /.+/}
+    resources :raw,    only: [:show], constraints: {id: /.+/}
     resources :tree,    only: [:show], constraints: {id: /.+/, format: /(html|js)/ }
     resources :edit_tree,    only: [:show, :update], constraints: {id: /.+/}, path: 'edit'
     resources :commit,  only: [:show], constraints: {id: /[[:alnum:]]{6,40}/}

features/steps/shared/paths.rb

@@ -205,7 +205,7 @@ module SharedPaths
   end
 
   Given 'I visit blob file from repo' do
-    visit project_tree_path(@project, File.join(ValidCommit::ID, ValidCommit::BLOB_FILE_PATH))
+    visit project_blob_path(@project, File.join(ValidCommit::ID, ValidCommit::BLOB_FILE_PATH))
   end
 
   Given 'I visit project source page for "8470d70"' do

lib/api/projects.rb

@@ -493,14 +493,16 @@ module Gitlab
 
         ref = params[:sha]
 
-        commit = user_project.repository.commit ref
+        repo = user_project.repository
+
+        commit = repo.commit(ref)
         not_found! "Commit" unless commit
 
-        tree = Tree.new commit.tree, ref, params[:filepath]
-        not_found! "File" unless tree.try(:tree)
+        blob = Gitlab::Git::Blob.new(repo, commit.id, ref, params[:filepath])
+        not_found! "File" unless blob.exists?
 
-        content_type tree.mime_type
-        present tree.data
+        content_type blob.mime_type
+        present blob.data
       end
 
       # Get a specific project's keys

lib/extracts_path.rb

@@ -102,9 +102,9 @@ 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?
+    raise InvalidPathError unless @tree.exists?
   rescue RuntimeError, NoMethodError, InvalidPathError
     not_found!
   end

lib/gitlab/git/blob.rb

+module Gitlab
+  module Git
+    class Blob
+      include Linguist::BlobHelper
+
+      attr_accessor :raw_blob
+
+      delegate :name, to: :raw_blob
+
+      def initialize(repository, sha, ref, path)
+        @repository, @sha, @ref = repository, sha, ref
+
+        @commit = @repository.commit(sha)
+        @raw_blob = @repository.tree(@commit, path)
+      end
+
+      def data
+        if raw_blob
+          raw_blob.data
+        else
+          nil
+        end
+      end
+
+      def exists?
+        raw_blob
+      end
+
+      def empty?
+        data.blank?
+      end
+
+      def mode
+        raw_blob.mode
+      end
+
+      def size
+        raw_blob.size
+      end
+    end
+  end
+end

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,
+        :id, :authored_date, :committed_date, :message,
+        :author_name, :author_email, :parent_ids,
+        :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(id authored_date committed_date author_name author_email committer_name committer_email message parent_ids)
+        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 sha
+        id
+      end
+
       def short_id(length = 10)
         id.to_s[0..length]
       end
@@ -89,37 +104,13 @@ 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)
-                         else
-                           nil
-                         end
-      end
-
-      def prev_commit_id
-        prev_commit.try :id
+      def parent_id
+        parent_ids.first
       end
 
       # Shows the diff between the commit's parent and the commit.
@@ -148,6 +139,43 @@ 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
+
+      def date
+        committed_date
+      end
+
+      private
+
+      def init_from_grit(grit)
+        @raw_commit = grit
+        @id = grit.id
+        @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
+        @parent_ids = grit.parents.map(&:id)
+      end
+
+      def init_from_hash(hash)
+        Commit.serialize_keys.each do |key|
+          send(:"#{key}=", hash[key])
+        end
+      end
     end
   end
 end

lib/gitlab/git/compare.rb

@@ -20,10 +20,8 @@ module Gitlab
           return
         end
 
-        @commit = Commit.new(first)
-
+        @commit = first
         @commits = repository.commits_between(last.id, first.id)
-        @commits = @commits.map { |c| Commit.new(c) }
 
         @diffs = if @commits.size > 100
                    []

lib/gitlab/git/tree.rb

+module Gitlab
+  module Git
+    class Tree
+      attr_accessor :repository, :sha, :path, :ref, :raw_tree, :id
+
+      def initialize(repository, sha, ref = nil, path = nil)
+        @repository, @sha, @ref, @path = repository, sha, ref, path
+
+        @path = nil if @path.blank?
+
+        # Load tree from repository
+        @commit = @repository.commit(@sha)
+        @raw_tree = @repository.tree(@commit, @path)
+      end
+
+      def exists?
+        raw_tree
+      end
+
+      def empty?
+        data.blank?
+      end
+
+      def trees
+        entries.select { |t| t.is_a?(Grit::Tree) }
+      end
+
+      def blobs
+        entries.select { |t| t.is_a?(Grit::Blob) }
+      end
+
+      def is_blob?
+        raw_tree.is_a?(Grit::Blob)
+      end
+
+      def up_dir?
+        path.present?
+      end
+
+      def readme
+        @readme ||= entries.find { |c| c.is_a?(Grit::Blob) and c.name =~ /^readme/i }
+      end
+
+      protected
+
+      def entries
+        raw_tree.contents
+      end
+    end
+  end
+end
+

spec/controllers/blob_controller_spec.rb

+require 'spec_helper'
+
+describe BlobController do
+  let(:project) { create(:project_with_code) }
+  let(:user)    { create(:user) }
+
+  before do
+    sign_in(user)
+
+    project.team << [user, :master]
+
+    project.stub(:branches).and_return(['master', 'foo/bar/baz'])
+    project.stub(:tags).and_return(['v1.0.0', 'v2.0.0'])
+    controller.instance_variable_set(:@project, project)
+  end
+
+  describe "GET show" do
+    render_views
+
+    before { get :show, project_id: project.code, id: id }
+
+    context "valid branch, valid file" do
+      let(:id) { 'master/README.md' }
+      it { should respond_with(:success) }
+    end
+
+    context "valid branch, invalid file" do
+      let(:id) { 'master/invalid-path.rb' }
+      it { should respond_with(:not_found) }
+    end
+
+    context "invalid branch, valid file" do
+      let(:id) { 'invalid-branch/README.md' }
+      it { should respond_with(:not_found) }
+    end
+  end
+end

spec/controllers/tree_controller_spec.rb

@@ -26,17 +26,17 @@ describe TreeController do
     end
 
     context "valid branch, valid path" do
-      let(:id) { 'master/README.md' }
+      let(:id) { 'master/app/' }
       it { should respond_with(:success) }
     end
 
     context "valid branch, invalid path" do
-      let(:id) { 'master/invalid-path.rb' }
+      let(:id) { 'master/invalid-path/' }
       it { should respond_with(:not_found) }
     end
 
     context "invalid branch, valid path" do
-      let(:id) { 'invalid-branch/README.md' }
+      let(:id) { 'invalid-branch/app/' }
       it { should respond_with(:not_found) }
     end
   end

spec/factories.rb

@@ -86,9 +86,11 @@ FactoryGirl.define do
       target_branch "master" # pretend bcf03b5d~3
       source_branch "stable" # pretend bcf03b5d
       st_commits do
-        [Commit.new(project.repository.commit('bcf03b5d')),
-         Commit.new(project.repository.commit('bcf03b5d~1')),
-         Commit.new(project.repository.commit('bcf03b5d~2'))]
+        [
+          project.repository.commit('bcf03b5d').to_hash,
+          project.repository.commit('bcf03b5d~1').to_hash,
+          project.repository.commit('bcf03b5d~2').to_hash
+        ]
       end
       st_diffs do
         project.repo.diff("bcf03b5d~3", "bcf03b5d")

spec/lib/git/commit_spec.rb

@@ -20,6 +20,8 @@ describe Gitlab::Git::Commit do
         author: @author,
         committer: @committer,
         committed_date: Date.yesterday,
+        authored_date: Date.yesterday,
+        parents: [],
         message: 'Refactoring specs'
       )
 

spec/models/commit_spec.rb

@@ -38,10 +38,10 @@ describe Commit do
     it { should respond_to(:message) }
     it { should respond_to(:authored_date) }
     it { should respond_to(:committed_date) }
+    it { should respond_to(:committer_email) }
+    it { should respond_to(:author_email) }
     it { should respond_to(:parents) }
     it { should respond_to(:date) }
-    it { should respond_to(:committer) }
-    it { should respond_to(:author) }
     it { should respond_to(:diffs) }
     it { should respond_to(:tree) }
     it { should respond_to(:id) }

spec/support/matchers.rb

@@ -3,7 +3,7 @@ RSpec::Matchers.define :be_valid_commit do
     actual != nil
     actual.id == ValidCommit::ID
     actual.message == ValidCommit::MESSAGE
-    actual.author.name == ValidCommit::AUTHOR_FULL_NAME
+    actual.author_name == ValidCommit::AUTHOR_FULL_NAME
   end
 end