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.
This commit is contained in:
James Edwards-Jones 2018-01-11 23:12:34 +00:00
parent 321ef1d45b
commit d2a77094ae
7 changed files with 203 additions and 1 deletions

View file

@ -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,

View file

@ -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

View file

@ -0,0 +1,5 @@
---
title: File Upload UI can create LFS pointers based on .gitattributes
merge_request: 16412
author:
type: fixed

View file

@ -0,0 +1,5 @@
class CarrierWaveStringFile < StringIO
def original_filename
""
end
end

View file

@ -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

View file

@ -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

View file

@ -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