summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Edwards-Jones <jedwardsjones@gitlab.com>2018-01-11 23:12:34 +0000
committerJames Edwards-Jones <jedwardsjones@gitlab.com>2018-02-06 15:26:25 +0000
commitd2a77094ae4a44b63fbe22ca910e836cb336a729 (patch)
tree4c2a33b4ad440355202c8dc9d915957a04a78f44
parent321ef1d45b7fad9a73451b97fc55476c77118477 (diff)
downloadgitlab-ce-d2a77094ae4a44b63fbe22ca910e836cb336a729.tar.gz
File upload UI obeys LFS filters
Uses Lfs::FileModificationHandler to coordinate LFS detection, creation of LfsObject, etc Caveats: 1. This isn't used by the multi-file editor / Web IDE 2. This isn't used on rename. We'd need to be able to download LFS files and add them to the commit if they no longer match so not as simple. 3. We only check the root .gitattributes file, so this should be improved to correctly check for nested .gitattributes files in subfolders.
-rw-r--r--app/services/files/create_service.rb12
-rw-r--r--app/services/lfs/file_modification_handler.rb42
-rw-r--r--changelogs/unreleased/jej-upload-file-tracks-lfs.yml5
-rw-r--r--lib/carrier_wave_string_file.rb5
-rw-r--r--lib/gitlab/git/lfs_pointer_file.rb25
-rw-r--r--spec/lib/gitlab/git/lfs_pointer_file_spec.rb37
-rw-r--r--spec/services/files/create_service_spec.rb78
7 files changed, 203 insertions, 1 deletions
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index 00a8dcf0934..46acdc5406c 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -1,10 +1,20 @@
module Files
class CreateService < Files::BaseService
def create_commit!
+ handler = Lfs::FileModificationHandler.new(project, @branch_name)
+
+ handler.new_file(@file_path, @file_content) do |content_or_lfs_pointer|
+ create_transformed_commit(content_or_lfs_pointer)
+ end
+ end
+
+ private
+
+ def create_transformed_commit(content_or_lfs_pointer)
repository.create_file(
current_user,
@file_path,
- @file_content,
+ content_or_lfs_pointer,
message: @commit_message,
branch_name: @branch_name,
author_email: @author_email,
diff --git a/app/services/lfs/file_modification_handler.rb b/app/services/lfs/file_modification_handler.rb
new file mode 100644
index 00000000000..fe9091a6e5d
--- /dev/null
+++ b/app/services/lfs/file_modification_handler.rb
@@ -0,0 +1,42 @@
+module Lfs
+ class FileModificationHandler
+ attr_reader :project, :branch_name
+
+ delegate :repository, to: :project
+
+ def initialize(project, branch_name)
+ @project = project
+ @branch_name = branch_name
+ end
+
+ def new_file(file_path, file_content)
+ if project.lfs_enabled? && lfs_file?(file_path)
+ lfs_pointer_file = Gitlab::Git::LfsPointerFile.new(file_content)
+ lfs_object = create_lfs_object!(lfs_pointer_file, file_content)
+ content = lfs_pointer_file.pointer
+
+ success = yield(content)
+
+ link_lfs_object!(lfs_object) if success
+ else
+ yield(file_content)
+ end
+ end
+
+ private
+
+ def lfs_file?(file_path)
+ repository.attributes_at(branch_name, file_path)['filter'] == 'lfs'
+ end
+
+ def create_lfs_object!(lfs_pointer_file, file_content)
+ LfsObject.find_or_create_by(oid: lfs_pointer_file.sha256, size: lfs_pointer_file.size) do |lfs_object|
+ lfs_object.file = CarrierWaveStringFile.new(file_content)
+ end
+ end
+
+ def link_lfs_object!(lfs_object)
+ project.lfs_objects << lfs_object
+ end
+ end
+end
diff --git a/changelogs/unreleased/jej-upload-file-tracks-lfs.yml b/changelogs/unreleased/jej-upload-file-tracks-lfs.yml
new file mode 100644
index 00000000000..a7cf6b6ba2c
--- /dev/null
+++ b/changelogs/unreleased/jej-upload-file-tracks-lfs.yml
@@ -0,0 +1,5 @@
+---
+title: File Upload UI can create LFS pointers based on .gitattributes
+merge_request: 16412
+author:
+type: fixed
diff --git a/lib/carrier_wave_string_file.rb b/lib/carrier_wave_string_file.rb
new file mode 100644
index 00000000000..6c848902e4a
--- /dev/null
+++ b/lib/carrier_wave_string_file.rb
@@ -0,0 +1,5 @@
+class CarrierWaveStringFile < StringIO
+ def original_filename
+ ""
+ end
+end
diff --git a/lib/gitlab/git/lfs_pointer_file.rb b/lib/gitlab/git/lfs_pointer_file.rb
new file mode 100644
index 00000000000..da12ed7d125
--- /dev/null
+++ b/lib/gitlab/git/lfs_pointer_file.rb
@@ -0,0 +1,25 @@
+module Gitlab
+ module Git
+ class LfsPointerFile
+ def initialize(data)
+ @data = data
+ end
+
+ def pointer
+ @pointer ||= <<~FILE
+ version https://git-lfs.github.com/spec/v1
+ oid sha256:#{sha256}
+ size #{size}
+ FILE
+ end
+
+ def size
+ @size ||= @data.bytesize
+ end
+
+ def sha256
+ @sha256 ||= Digest::SHA256.hexdigest(@data)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/git/lfs_pointer_file_spec.rb b/spec/lib/gitlab/git/lfs_pointer_file_spec.rb
new file mode 100644
index 00000000000..d7f76737f3f
--- /dev/null
+++ b/spec/lib/gitlab/git/lfs_pointer_file_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+describe Gitlab::Git::LfsPointerFile do
+ let(:data) { "1234\n" }
+
+ subject { described_class.new(data) }
+
+ describe '#size' do
+ it 'counts the bytes' do
+ expect(subject.size).to eq 5
+ end
+
+ it 'handles non ascii data' do
+ expect(described_class.new("รครครครค").size).to eq 8
+ end
+ end
+
+ describe '#sha256' do
+ it 'hashes the content correctly' do
+ expect(subject.sha256).to eq 'a883dafc480d466ee04e0d6da986bd78eb1fdd2178d04693723da3a8f95d42f4'
+ end
+ end
+
+ describe '#pointer' do
+ it 'starts with the LFS version' do
+ expect(subject.pointer).to start_with('version https://git-lfs.github.com/spec/v1')
+ end
+
+ it 'includes sha256' do
+ expect(subject.pointer).to match(/^oid sha256:[0-9a-fA-F]{64}/)
+ end
+
+ it 'ends with the size' do
+ expect(subject.pointer).to end_with("\nsize 5\n")
+ end
+ end
+end
diff --git a/spec/services/files/create_service_spec.rb b/spec/services/files/create_service_spec.rb
new file mode 100644
index 00000000000..030263b1502
--- /dev/null
+++ b/spec/services/files/create_service_spec.rb
@@ -0,0 +1,78 @@
+require "spec_helper"
+
+describe Files::CreateService do
+ let(:project) { create(:project, :repository) }
+ let(:repository) { project.repository }
+ let(:user) { create(:user) }
+ let(:file_content) { 'Test file content' }
+ let(:branch_name) { project.default_branch }
+ let(:start_branch) { branch_name }
+
+ let(:commit_params) do
+ {
+ file_path: file_path,
+ commit_message: "Update File",
+ file_content: file_content,
+ file_content_encoding: "text",
+ start_project: project,
+ start_branch: start_branch,
+ branch_name: branch_name
+ }
+ end
+
+ subject { described_class.new(project, user, commit_params) }
+
+ before do
+ project.add_master(user)
+ end
+
+ describe "#execute" do
+ context 'when file matches LFS filter' do
+ let(:file_path) { 'test_file.lfs' }
+ let(:branch_name) { 'lfs' }
+
+ context 'with LFS disabled' do
+ it 'skips gitattributes check' do
+ expect(repository).not_to receive(:attributes_at)
+
+ subject.execute
+ end
+
+ it "doesn't create LFS pointers" do
+ subject.execute
+
+ blob = repository.blob_at('lfs', file_path)
+
+ expect(blob.data).not_to start_with('version https://git-lfs.github.com/spec/v1')
+ expect(blob.data).to eq(file_content)
+ end
+ end
+
+ context 'with LFS enabled' do
+ before do
+ allow(project).to receive(:lfs_enabled?).and_return(true)
+ end
+
+ it 'creates an LFS pointer' do
+ subject.execute
+
+ blob = repository.blob_at('lfs', file_path)
+
+ expect(blob.data).to start_with('version https://git-lfs.github.com/spec/v1')
+ end
+
+ it "creates an LfsObject with the file's content" do
+ subject.execute
+
+ expect(LfsObject.last.file.read).to eq file_content
+ end
+
+ it 'links the LfsObject to the project' do
+ expect do
+ subject.execute
+ end.to change { project.lfs_objects.count }.by(1)
+ end
+ end
+ end
+ end
+end