Skip to content

G
gitlab-ce
Commit fb274568 authored by Felipe Artur's avatar Felipe Artur
Browse files
Options

Revert "Merge branch 'dm-fix-api-create-file-on-empty-repo' into 'master'" 

This reverts commit 4b692487.
parent 1782ec0b
Hide whitespace changes
Inline Side-by-side
Showing with 42 additions and 341 deletions
app/controllers/concerns/creates_commit.rb
View file @ fb274568
......@@ -4,8 +4,6 @@ module CreatesCommit
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil)
set_commit_variables
start_branch = @mr_target_branch
commit_params = @commit_params.merge(
start_project: @mr_target_project,
start_branch: @mr_target_branch,
......@@ -119,23 +117,8 @@ module CreatesCommit
# Merge request to this project
@mr_target_project = @project
@mr_target_branch ||= @ref || @target_branch
@mr_target_branch = @ref || @target_branch
@mr_source_branch = guess_mr_source_branch
end
def guess_mr_source_branch
# XXX: Happens when viewing a commit without a branch. In this case,
# @target_branch would be the default branch for @mr_source_project,
# however we want a generated new branch here. Thus we can't use
# @target_branch, but should pass nil to indicate that we want a new
# branch instead of @target_branch.
return if
create_merge_request? &&
# XXX: Don't understand why rubocop prefers this indention
@mr_source_project.repository.branch_exists?(@target_branch)
@target_branch
@mr_source_branch = @target_branch
end
end
app/models/repository.rb
View file @ fb274568
......@@ -1075,8 +1075,6 @@ class Repository
end
def with_repo_branch_commit(start_repository, start_branch_name)
return yield(nil) if start_repository.empty_repo?
branch_name_or_sha =
if start_repository == self
start_branch_name
......
app/services/files/base_service.rb
View file @ fb274568
......@@ -58,12 +58,16 @@ module Files
raise_error("You are not allowed to push into this branch")
end
if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch)
raise ValidationError, 'You can only create or edit files when you are on a branch'
end
if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name)
raise ValidationError, "A branch called #{@branch_name} already exists. Switch to that branch in order to make changes"
unless project.empty_repo?
unless @start_project.repository.branch_exists?(@start_branch)
raise_error('You can only create or edit files when you are on a branch')
end
if different_branch?
if repository.branch_exists?(@target_branch)
raise_error('Branch with such name already exists. You need to switch to this branch in order to make changes')
end
end
end
end
......
app/services/git_operation_service.rb
View file @ fb274568
......@@ -56,16 +56,12 @@ class GitOperationService
start_project: repository.project,
&block)
start_repository = start_project.repository
start_branch_name = nil if start_repository.empty_repo?
if start_branch_name && !start_repository.branch_exists?(start_branch_name)
raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.path_with_namespace}"
end
check_with_branch_arguments!(
branch_name, start_branch_name, start_project)
update_branch_with_hooks(branch_name) do
repository.with_repo_branch_commit(
start_repository,
start_project.repository,
start_branch_name || branch_name,
&block)
end
......@@ -153,4 +149,31 @@ class GitOperationService
repository.raw_repository.autocrlf = :input
end
end
def check_with_branch_arguments!(
branch_name, start_branch_name, start_project)
return if repository.branch_exists?(branch_name)
if repository.project != start_project
unless start_branch_name
raise ArgumentError,
'Should also pass :start_branch_name if' +
' :start_project is different from current project'
end
unless start_project.repository.branch_exists?(start_branch_name)
raise ArgumentError,
"Cannot find branch #{branch_name} nor" \
" #{start_branch_name} from" \
" #{start_project.path_with_namespace}"
end
elsif start_branch_name
unless repository.branch_exists?(start_branch_name)
raise ArgumentError,
"Cannot find branch #{branch_name} nor" \
" #{start_branch_name} from" \
" #{repository.project.path_with_namespace}"
end
end
end
end
changelogs/unreleased/dm-fix-api-create-file-on-empty-repo.yml deleted 100644 → 0
View file @ 1782ec0b
---
title: Fix creating a file in an empty repo using the API
merge_request: 9632
author:
spec/factories/projects.rb
View file @ fb274568
......@@ -39,10 +39,6 @@ FactoryGirl.define do
trait :empty_repo do
after(:create) do |project|
project.create_repository
# We delete hooks so that gitlab-shell will not try to authenticate with
# an API that isn't running
FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.path_with_namespace}.git", 'hooks'))
end
end
......
spec/requests/api/files_spec.rb
View file @ fb274568
......@@ -147,20 +147,6 @@ describe API::Files, api: true do
expect(last_commit.author_name).to eq(author_name)
end
end
context 'when the repo is empty' do
let!(:project) { create(:project_empty_repo, namespace: user.namespace ) }
it "creates a new file in project repo" do
post api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(201)
expect(json_response['file_path']).to eq('newfile.rb')
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(user.email)
expect(last_commit.author_name).to eq(user.name)
end
end
end
describe "PUT /projects/:id/repository/files" do
......
spec/requests/api/v3/files_spec.rb deleted 100644 → 0
View file @ 1782ec0b
require 'spec_helper'
describe API::V3::Files, api: true do
include ApiHelpers
# I have to remove periods from the end of the name
# This happened when the user's name had a suffix (i.e. "Sr.")
# This seems to be what git does under the hood. For example, this commit:
#
# $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?'
#
# results in this:
#
# $ git show --pretty
# ...
# Author: Foo Sr <foo@example.com>
# ...
let(:user) { create(:user) }
let!(:project) { create(:project, :repository, namespace: user.namespace ) }
let(:guest) { create(:user) { |u| project.add_guest(u) } }
let(:file_path) { 'files/ruby/popen.rb' }
let(:params) do
{
file_path: file_path,
ref: 'master'
}
end
let(:author_email) { FFaker::Internet.email }
let(:author_name) { FFaker::Name.name.chomp("\.") }
before { project.team << [user, :developer] }
describe "GET /projects/:id/repository/files" do
let(:route) { "/projects/#{project.id}/repository/files" }
shared_examples_for 'repository files' do
it "returns file info" do
get v3_api(route, current_user), params
expect(response).to have_http_status(200)
expect(json_response['file_path']).to eq(file_path)
expect(json_response['file_name']).to eq('popen.rb')
expect(json_response['last_commit_id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d')
expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n")
end
context 'when no params are given' do
it_behaves_like '400 response' do
let(:request) { get v3_api(route, current_user) }
end
end
context 'when file_path does not exist' do
let(:params) do
{
file_path: 'app/models/application.rb',
ref: 'master',
}
end
it_behaves_like '404 response' do
let(:request) { get v3_api(route, current_user), params }
let(:message) { '404 File Not Found' }
end
end
context 'when repository is disabled' do
include_context 'disabled repository'
it_behaves_like '403 response' do
let(:request) { get v3_api(route, current_user), params }
end
end
end
context 'when unauthenticated', 'and project is public' do
it_behaves_like 'repository files' do
let(:project) { create(:project, :public) }
let(:current_user) { nil }
end
end
context 'when unauthenticated', 'and project is private' do
it_behaves_like '404 response' do
let(:request) { get v3_api(route), params }
let(:message) { '404 Project Not Found' }
end
end
context 'when authenticated', 'as a developer' do
it_behaves_like 'repository files' do
let(:current_user) { user }
end
end
context 'when authenticated', 'as a guest' do
it_behaves_like '403 response' do
let(:request) { get v3_api(route, guest), params }
end
end
end
describe "POST /projects/:id/repository/files" do
let(:valid_params) do
{
file_path: 'newfile.rb',
branch_name: 'master',
content: 'puts 8',
commit_message: 'Added newfile'
}
end
it "creates a new file in project repo" do
post v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(201)
expect(json_response['file_path']).to eq('newfile.rb')
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(user.email)
expect(last_commit.author_name).to eq(user.name)
end
it "returns a 400 bad request if no params given" do
post v3_api("/projects/#{project.id}/repository/files", user)
expect(response).to have_http_status(400)
end
it "returns a 400 if editor fails to create file" do
allow_any_instance_of(Repository).to receive(:create_file).
and_return(false)
post v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(400)
end
context "when specifying an author" do
it "creates a new file with the specified author" do
valid_params.merge!(author_email: author_email, author_name: author_name)
post v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(201)
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(author_email)
expect(last_commit.author_name).to eq(author_name)
end
end
context 'when the repo is empty' do
let!(:project) { create(:project_empty_repo, namespace: user.namespace ) }
it "creates a new file in project repo" do
post v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(201)
expect(json_response['file_path']).to eq('newfile.rb')
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(user.email)
expect(last_commit.author_name).to eq(user.name)
end
end
end
describe "PUT /projects/:id/repository/files" do
let(:valid_params) do
{
file_path: file_path,
branch_name: 'master',
content: 'puts 8',
commit_message: 'Changed file'
}
end
it "updates existing file in project repo" do
put v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(200)
expect(json_response['file_path']).to eq(file_path)
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(user.email)
expect(last_commit.author_name).to eq(user.name)
end
it "returns a 400 bad request if no params given" do
put v3_api("/projects/#{project.id}/repository/files", user)
expect(response).to have_http_status(400)
end
context "when specifying an author" do
it "updates a file with the specified author" do
valid_params.merge!(author_email: author_email, author_name: author_name, content: "New content")
put v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(200)
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(author_email)
expect(last_commit.author_name).to eq(author_name)
end
end
end
describe "DELETE /projects/:id/repository/files" do
let(:valid_params) do
{
file_path: file_path,
branch_name: 'master',
commit_message: 'Changed file'
}
end
it "deletes existing file in project repo" do
delete v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(200)
expect(json_response['file_path']).to eq(file_path)
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(user.email)
expect(last_commit.author_name).to eq(user.name)
end
it "returns a 400 bad request if no params given" do
delete v3_api("/projects/#{project.id}/repository/files", user)
expect(response).to have_http_status(400)
end
it "returns a 400 if fails to create file" do
allow_any_instance_of(Repository).to receive(:delete_file).and_return(false)
delete v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(400)
end
context "when specifying an author" do
it "removes a file with the specified author" do
valid_params.merge!(author_email: author_email, author_name: author_name)
delete v3_api("/projects/#{project.id}/repository/files", user), valid_params
expect(response).to have_http_status(200)
last_commit = project.repository.commit.raw
expect(last_commit.author_email).to eq(author_email)
expect(last_commit.author_name).to eq(author_name)
end
end
end
describe "POST /projects/:id/repository/files with binary file" do
let(:file_path) { 'test.bin' }
let(:put_params) do
{
file_path: file_path,
branch_name: 'master',
content: 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAAA1BMVEUAAACnej3aAAAAAXRSTlMAQObYZgAAAApJREFUCNdjYAAAAAIAAeIhvDMAAAAASUVORK5CYII=',
commit_message: 'Binary file with a \n should not be touched',
encoding: 'base64'
}
end
let(:get_params) do
{
file_path: file_path,
ref: 'master',
}
end
before do
post v3_api("/projects/#{project.id}/repository/files", user), put_params
end
it "remains unchanged" do
get v3_api("/projects/#{project.id}/repository/files", user), get_params
expect(response).to have_http_status(200)
expect(json_response['file_path']).to eq(file_path)
expect(json_response['file_name']).to eq(file_path)
expect(json_response['content']).to eq(put_params[:content])
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7