Merge branch 'bug/500_tag_name' into 'master'

Fix 500 on tags page

Fix bug when grit parse tags wrong and raise exception on tags page.
Fixes #993

8 files changed, 76 insertions, 54 deletions

diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index 9dbb0d81888..818c5d971e9 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/app/controllers/projects/tags_controller.rb
@@ -8,7 +8,7 @@ class Projects::TagsController < Projects::ApplicationController
   before_filter :authorize_admin_project!, only: [:destroy]
 
   def index
-    @tags = Kaminari.paginate_array(@repository.tags).page(params[:page]).per(30)
+    @tags = Kaminari.paginate_array(@repository.tags.reverse).page(params[:page]).per(30)
   end
 
   def create
diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb
index eb68607f050..3b9cd67636d 100644
--- a/app/helpers/gitlab_markdown_helper.rb
+++ b/app/helpers/gitlab_markdown_helper.rb
@@ -166,13 +166,13 @@ module GitlabMarkdownHelper
 
   def file_exists?(path)
     return false if path.nil? || path.empty?
-    return @repository.blob_at(current_ref, path).present? || Tree.new(@repository, current_ref, path).entries.any?
+    return @repository.blob_at(current_ref, path).present? || @repository.tree(:head, path).entries.any?
   end
 
   # Check if the path is pointing to a directory(tree) or a file(blob)
   # eg. doc/api is directory and doc/README.md is file
   def local_path(path)
-    return "tree" if Tree.new(@repository, current_ref, path).entries.any?
+    return "tree" if @repository.tree(:head, path).entries.any?
     return "raw" if @repository.blob_at(current_ref, path).image?
     return "blob"
   end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index dd1f9801878..bcc1bcbd96a 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -16,29 +16,31 @@ class Commit
   DIFF_HARD_LIMIT_FILES = 500
   DIFF_HARD_LIMIT_LINES = 10000
 
-  def self.decorate(commits)
-    commits.map { |c| self.new(c) }
-  end
+  class << self
+    def decorate(commits)
+      commits.map { |c| self.new(c) }
+    end
 
-  # Calculate number of lines to render for diffs
-  def self.diff_line_count(diffs)
-    diffs.reduce(0){|sum, d| sum + d.diff.lines.count}
-  end
+    # Calculate number of lines to render for diffs
+    def diff_line_count(diffs)
+      diffs.reduce(0){|sum, d| sum + d.diff.lines.count}
+    end
 
-  def self.diff_suppress?(diffs, line_count = nil)
-    # optimize - check file count first
-    return true if diffs.size > DIFF_SAFE_FILES
+    def diff_suppress?(diffs, line_count = nil)
+      # optimize - check file count first
+      return true if diffs.size > DIFF_SAFE_FILES
 
-    line_count ||= Commit::diff_line_count(diffs)
-    line_count > DIFF_SAFE_LINES
-  end
+      line_count ||= Commit::diff_line_count(diffs)
+      line_count > DIFF_SAFE_LINES
+    end
 
-  def self.diff_force_suppress?(diffs, line_count = nil)
-    # optimize - check file count first
-    return true if diffs.size > DIFF_HARD_LIMIT_FILES
+    def diff_force_suppress?(diffs, line_count = nil)
+      # optimize - check file count first
+      return true if diffs.size > DIFF_HARD_LIMIT_FILES
 
-    line_count ||= Commit::diff_line_count(diffs)
-    line_count > DIFF_HARD_LIMIT_LINES
+      line_count ||= Commit::diff_line_count(diffs)
+      line_count > DIFF_HARD_LIMIT_LINES
+    end
   end
 
   attr_accessor :raw
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 1255b814533..271c2e4dbbc 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -57,7 +57,7 @@ class Repository
 
   def recent_branches(limit = 20)
     branches.sort do |a, b|
-      b.commit.committed_date <=> a.commit.committed_date
+      commit(b.target).committed_date <=> commit(a.target).committed_date
     end[0..limit]
   end
 
@@ -163,7 +163,19 @@ class Repository
 
   def readme
     Rails.cache.fetch(cache_key(:readme)) do
-      Tree.new(self, self.root_ref).readme
+      tree(:head).readme
     end
   end
+
+  def head_commit
+    commit(self.root_ref)
+  end
+
+  def tree(sha = :head, path = nil)
+    if sha == :head
+      sha = head_commit.sha
+    end
+
+    Tree.new(self, sha, path)
+  end
 end
diff --git a/app/models/tree.rb b/app/models/tree.rb
index ed06cb1a128..4f866f1a33d 100644
--- a/app/models/tree.rb
+++ b/app/models/tree.rb
@@ -23,4 +23,8 @@ class Tree
   def submodules
     @entries.select(&:submodule?)
   end
+
+  def sorted_entries
+    trees + blobs + submodules
+  end
 end
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 4fd708517fc..40b6fc5d72e 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -1,4 +1,4 @@
-- commit = Commit.new(Gitlab::Git::Commit.new(branch.commit))
+- commit = @repository.commit(branch.target)
 %li
   %h4
     = link_to project_commits_path(@project, branch.name) do
@@ -19,10 +19,14 @@
         = link_to project_branch_path(@project, branch.name), class: 'btn grouped btn-small remove-row', method: :delete, data: { confirm: 'Removed branch cannot be restored. Are you sure?'}, remote: true do
           %i.icon-trash
 
-  %p
-    = link_to project_commit_path(@project, commit.id), class: 'commit_short_id' do
-      = commit.short_id
-    = image_tag avatar_icon(commit.author_email), class: "avatar s16", alt: ''
-    %span.light
-      = gfm escape_once(truncate(commit.title, length: 40))
-    #{time_ago_with_tooltip(commit.committed_date)}
+  - if commit
+    %p
+      = link_to project_commit_path(@project, commit.id), class: 'commit_short_id' do
+        = commit.short_id
+      = image_tag avatar_icon(commit.author_email), class: "avatar s16", alt: ''
+      %span.light
+        = gfm escape_once(truncate(commit.title, length: 40))
+      #{time_ago_with_tooltip(commit.committed_date)}
+  - else
+    %p
+      Cant find HEAD commit for this branch
diff --git a/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml
new file mode 100644
index 00000000000..70dedcf9155
--- /dev/null
+++ b/app/views/projects/tags/_tag.html.haml
@@ -0,0 +1,22 @@
+- commit = @repository.commit(tag.target)
+%li
+  %h4
+    = link_to project_commits_path(@project, tag.name), class: "" do
+      %i.icon-tag
+      = tag.name
+    .pull-right
+      %small.cdark
+        %i.icon-calendar
+        #{time_ago_with_tooltip(commit.committed_date)}
+  %p.prepend-left-20
+    = link_to commit.short_id(8), project_commit_path(@project, commit), class: "monospace"
+    &ndash;
+    = link_to_gfm truncate(commit.title, length: 70), project_commit_path(@project, commit.id), class: "cdark"
+
+    %span.pull-right
+      - if can? current_user, :download_code, @project
+        = render 'projects/repositories/download_archive', ref: tag.name, btn_class: 'grouped btn-group-small'
+      - if can?(current_user, :admin_project, @project)
+        = link_to project_tag_path(@project, tag.name), class: 'btn btn-small remove-row grouped', method: :delete, data: { confirm: 'Removed tag cannot be restored. Are you sure?'}, remote: true do
+          %i.icon-trash
+
diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml
index c88e42a1073..2d53a5dd66a 100644
--- a/app/views/projects/tags/index.html.haml
+++ b/app/views/projects/tags/index.html.haml
@@ -13,29 +13,7 @@
 - unless @tags.empty?
   %ul.bordered-list
     - @tags.each do |tag|
-      - commit = Commit.new(Gitlab::Git::Commit.new(tag.commit))
-      %li
-        %h4
-          = link_to project_commits_path(@project, tag.name), class: "" do
-            %i.icon-tag
-            = tag.name
-          %small
-            = truncate(tag.message || '', length: 70)
-          .pull-right
-            %small.cdark
-              %i.icon-calendar
-              #{time_ago_with_tooltip(commit.committed_date)}
-        %p.prepend-left-20
-          = link_to commit.short_id(8), project_commit_path(@project, commit), class: "monospace"
-          &ndash;
-          = link_to_gfm truncate(commit.title, length: 70), project_commit_path(@project, commit.id), class: "cdark"
-
-          %span.pull-right
-            - if can? current_user, :download_code, @project
-              = render 'projects/repositories/download_archive', ref: tag.name, btn_class: 'grouped btn-group-small'
-            - if can?(current_user, :admin_project, @project)
-              = link_to project_tag_path(@project, tag.name), class: 'btn btn-small remove-row grouped', method: :delete, data: { confirm: 'Removed tag cannot be restored. Are you sure?'}, remote: true do
-                %i.icon-trash
+      = render 'tag', tag: tag
 
   = paginate @tags, theme: 'gitlab'