Merge branch 'serve_lfs_object' into 'master'

Serve LFS object

Depends on gitlab-org/gitlab_git!57

See merge request !1976

Gemfile.lock

@@ -314,7 +314,7 @@ GEM
       posix-spawn (~> 0.3)
     gitlab_emoji (0.2.0)
       gemojione (~> 2.1)
-    gitlab_git (7.2.20)
+    gitlab_git (7.2.21)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)

app/controllers/projects/raw_controller.rb

@@ -10,15 +10,13 @@ class Projects::RawController < Projects::ApplicationController
     @blob = @repository.blob_at(@commit.id, @path)
 
     if @blob
-      type = get_blob_type
-
       headers['X-Content-Type-Options'] = 'nosniff'
 
-      send_data(
-        @blob.data,
-        type: type,
-        disposition: 'inline'
-      )
+      if @blob.lfs_pointer?
+        send_lfs_object
+      else
+        stream_data
+      end
     else
       render_404
     end
@@ -35,4 +33,33 @@ class Projects::RawController < Projects::ApplicationController
       'application/octet-stream'
     end
   end
+
+  def stream_data
+    type = get_blob_type
+
+    send_data(
+      @blob.data,
+      type: type,
+      disposition: 'inline'
+    )
+  end
+
+  def send_lfs_object
+    lfs_object = find_lfs_object
+
+    if lfs_object && lfs_object.project_allowed_access?(@project)
+      send_file lfs_object.file.path, filename: @blob.name, disposition: 'attachment'
+    else
+      render_404
+    end
+  end
+
+  def find_lfs_object
+    lfs_object = LfsObject.find_by_oid(@blob.lfs_oid)
+    if lfs_object && lfs_object.file.exists?
+      lfs_object
+    else
+      nil
+    end
+  end
 end

app/helpers/blob_helper.rb

@@ -30,7 +30,7 @@ module BlobHelper
         nil
       end
 
-    if blob && blob.text?
+    if blob_viewable?(blob)
       text = 'Edit'
       after = options[:after] || ''
       from_mr = options[:from_merge_request_id]
@@ -71,4 +71,16 @@ module BlobHelper
   def blob_icon(mode, name)
     icon("#{file_type_icon_class('file', mode, name)} fw")
   end
+
+  def blob_viewable?(blob)
+    blob && blob.text? && !blob.lfs_pointer?
+  end
+
+  def blob_size(blob)
+    if blob.lfs_pointer?
+      blob.lfs_size
+    else
+      blob.size
+    end
+  end
 end

app/helpers/tree_helper.rb

@@ -54,6 +54,10 @@ module TreeHelper
     ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
   end
 
+  def can_delete_or_replace?(blob)
+    allowed_tree_edit? && !blob.lfs_pointer?
+  end
+
   def tree_breadcrumbs(tree, max_links = 2)
     if @path.present?
       part_path = ""

app/models/lfs_object.rb

@@ -5,4 +5,16 @@ class LfsObject < ActiveRecord::Base
   validates :oid, presence: true, uniqueness: true
 
   mount_uploader :file, LfsObjectUploader
+
+  def storage_project(project)
+    if project && project.forked?
+      storage_project(project.forked_from_project)
+    else
+      project
+    end
+  end
+
+  def project_allowed_access?(project)
+    projects.exists?(storage_project(project).id)
+  end
 end

app/views/projects/blob/_actions.html.haml

@@ -3,7 +3,7 @@
   = link_to 'Raw', namespace_project_raw_path(@project.namespace, @project, @id),
       class: 'btn btn-sm', target: '_blank'
   -# only show normal/blame view links for text files
-  - if @blob.text?
+  - if blob_viewable?(@blob)
     - if current_page? namespace_project_blame_path(@project.namespace, @project, @id)
       = link_to 'Normal View', namespace_project_blob_path(@project.namespace, @project, @id),
           class: 'btn btn-sm'
@@ -16,7 +16,7 @@
     = link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project,
         tree_join(@commit.sha, @path)), class: 'btn btn-sm'
 
-- if allowed_tree_edit?
+- if can_delete_or_replace?(@blob)
   .btn-group{ role: "group" }
     %button.btn.btn-default{ 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } Replace
     %button.btn.btn-remove{ 'data-target' => '#modal-remove-blob', 'data-toggle' => 'modal' } Delete

app/views/projects/blob/_blob.html.haml

@@ -29,10 +29,12 @@
       %strong
         = blob.name
       %small
-        = number_to_human_size(blob.size)
+        = number_to_human_size(blob_size(blob))
       .file-actions.hidden-xs
         = render "actions"
-    - if blob.text?
+    - if blob.lfs_pointer?
+      = render "download", blob: blob
+    - elsif blob.text?
       = render "text", blob: blob
     - elsif blob.image?
       = render "image", blob: blob

app/views/projects/blob/_download.html.haml

@@ -4,4 +4,4 @@
       %h1.light
         %i.fa.fa-download
       %h4
-        Download (#{number_to_human_size blob.size})
+        Download (#{number_to_human_size blob_size(blob)})

app/views/projects/blob/show.html.haml

@@ -6,7 +6,7 @@
 %div#tree-holder.tree-holder
   = render 'blob', blob: @blob
 
-- if allowed_tree_edit?
+- if can_delete_or_replace?(@blob)
   = render 'projects/blob/remove'
 
   - title = "Replace #{@blob.name}"

app/views/projects/diffs/_file.html.haml

@@ -24,7 +24,7 @@
             = "#{diff_file.diff.a_mode} → #{diff_file.diff.b_mode}"
 
       .diff-controls
-        - if blob.text?
+        - if blob_viewable?(blob)
           = link_to '#', class: 'js-toggle-diff-comments btn btn-sm active has_tooltip', title: "Toggle comments for this file" do
             %i.fa.fa-comments
            
@@ -39,7 +39,7 @@
   .diff-content.diff-wrap-lines
     -# Skipp all non non-supported blobs
     - return unless blob.respond_to?('text?')
-    - if blob.text?
+    - if blob_viewable?(blob)
       - if diff_view == 'parallel'
         = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
       - else

features/project/source/browse_files.feature

@@ -221,3 +221,9 @@ Feature: Project Source Browse Files
     Given I switch ref to fix
     And I visit the fix tree
     Then I see the commit data for a directory with a leading dot
+
+  Scenario: I browse LFS object
+    Given I click on "files/lfs/lfs_object.iso" file in repo
+    Then I should see download link and object size
+    And I should not see lfs pointer details
+    And I should see buttons for allowed commands

features/steps/project/source/browse_files.rb

@@ -305,6 +305,33 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     expect(page).not_to have_content('Loading commit data...')
   end
 
+  step 'I click on "files/lfs/lfs_object.iso" file in repo' do
+    visit namespace_project_tree_path(@project.namespace, @project, "lfs")
+    click_link 'files'
+    click_link "lfs"
+    click_link "lfs_object.iso"
+  end
+
+  step 'I should see download link and object size' do
+    expect(page).to have_content 'Download (1.5 MB)'
+  end
+
+  step 'I should not see lfs pointer details' do
+    expect(page).not_to have_content 'version https://git-lfs.github.com/spec/v1'
+    expect(page).not_to have_content 'oid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897'
+    expect(page).not_to have_content 'size 1575078'
+  end
+
+  step 'I should see buttons for allowed commands' do
+    expect(page).to have_content 'Raw'
+    expect(page).to have_content 'History'
+    expect(page).to have_content 'Permalink'
+    expect(page).not_to have_content 'Edit'
+    expect(page).not_to have_content 'Blame'
+    expect(page).not_to have_content 'Delete'
+    expect(page).not_to have_content 'Replace'
+  end
+
   private
 
   def set_new_content

lib/gitlab/lfs/response.rb

@@ -220,7 +220,7 @@ module Gitlab
 
       def storage_project(project)
         if project.forked?
-          project.forked_from_project
+          storage_project(project.forked_from_project)
         else
           project
         end

spec/controllers/projects/raw_controller_spec.rb

@@ -33,5 +33,39 @@ describe Projects::RawController do
         expect(response.header['Content-Type']).to eq('image/jpeg')
       end
     end
+
+    context 'lfs object' do
+      let(:id) { 'be93687/files/lfs/lfs_object.iso' }
+      let!(:lfs_object) { create(:lfs_object, oid: '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', size: '1575078') }
+
+      context 'when project has access' do
+        before do
+          public_project.lfs_objects << lfs_object
+          allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true)
+          allow(controller).to receive(:send_file) { controller.render nothing: true }
+        end
+
+        it 'serves the file' do
+          expect(controller).to receive(:send_file).with("#{Gitlab.config.shared.path}/lfs-objects/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: "lfs_object.iso", disposition: 'attachment')
+          get(:show,
+              namespace_id: public_project.namespace.to_param,
+              project_id: public_project.to_param,
+              id: id)
+
+          expect(response.status).to eq(200)
+        end
+      end
+
+      context 'when project does not have access' do
+        it 'does not serve the file' do
+          get(:show,
+              namespace_id: public_project.namespace.to_param,
+              project_id: public_project.to_param,
+              id: id)
+
+          expect(response.status).to eq(404)
+        end
+      end
+    end
   end
 end

spec/support/test_env.rb

@@ -12,6 +12,7 @@ module TestEnv
     'fix'              => '48f0be4',
     'improve/awesome'  => '5937ac0',
     'markdown'         => '0ed8c6c',
+    'lfs'              => 'be93687',
     'master'           => '5937ac0',
     "'test'"           => 'e56497b',
   }