BigW Consortium Gitlab

Skip to content

G
gitlab-ce
Commit d170133b by Douwe Maan Committed by Robert Speicher
Options

Refactor changing files in web UI

parent bfb635a4
Showing with 271 additions and 456 deletions
app/assets/javascripts/blob/blob_file_dropzone.js
......@@ -35,7 +35,7 @@ export default class BlobFileDropzone {
this.removeFile(file);
});
this.on('sending', function (file, xhr, formData) {
formData.append('target_branch', form.find('input[name="target_branch"]').val());
formData.append('branch_name', form.find('input[name="branch_name"]').val());
formData.append('create_merge_request', form.find('.js-create-merge-request').val());
formData.append('commit_message', form.find('.js-commit-message').val());
});
......
app/controllers/concerns/creates_commit.rb
module CreatesCommit
extend ActiveSupport::Concern
def set_start_branch_to_branch_name
branch_exists = @repository.find_branch(@branch_name)
@start_branch = @branch_name if branch_exists
end
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil)
set_commit_variables
if can?(current_user, :push_code, @project)
@project_to_commit_into = @project
@branch_name ||= @ref
else
@project_to_commit_into = current_user.fork_of(@project)
@branch_name ||= @project_to_commit_into.repository.next_branch('patch')
end
@start_branch ||= @ref || @branch_name
commit_params = @commit_params.merge(
start_project: @mr_target_project,
start_branch: @mr_target_branch,
target_branch: @mr_source_branch
start_project: @project,
start_branch: @start_branch,
branch_name: @branch_name
)
result = service.new(
@mr_source_project, current_user, commit_params).execute
result = service.new(@project_to_commit_into, current_user, commit_params).execute
if result[:status] == :success
update_flash_notice(success_notice)
......@@ -72,30 +84,30 @@ module CreatesCommit
def new_merge_request_path
new_namespace_project_merge_request_path(
@mr_source_project.namespace,
@mr_source_project,
@project_to_commit_into.namespace,
@project_to_commit_into,
merge_request: {
source_project_id: @mr_source_project.id,
target_project_id: @mr_target_project.id,
source_branch: @mr_source_branch,
target_branch: @mr_target_branch
source_project_id: @project_to_commit_into.id,
target_project_id: @project.id,
source_branch: @branch_name,
target_branch: @start_branch
}
)
end
def existing_merge_request_path
namespace_project_merge_request_path(@mr_target_project.namespace, @mr_target_project, @merge_request)
namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
end
def merge_request_exists?
return @merge_request if defined?(@merge_request)
@merge_request = MergeRequestsFinder.new(current_user, project_id: @mr_target_project.id).execute.opened.
find_by(source_branch: @mr_source_branch, target_branch: @mr_target_branch, source_project_id: @mr_source_project)
@merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.
find_by(source_project_id: @project_to_commit_into, source_branch: @branch_name, target_branch: @start_branch)
end
def different_project?
@mr_source_project != @mr_target_project
@project_to_commit_into != @project
end
def create_merge_request?
......@@ -103,22 +115,6 @@ module CreatesCommit
# as the target branch in the same project,
# we don't want to create a merge request.
params[:create_merge_request].present? &&
(different_project? || @mr_target_branch != @mr_source_branch)
end
def set_commit_variables
if can?(current_user, :push_code, @project)
@mr_source_project = @project
@target_branch ||= @ref
else
@mr_source_project = current_user.fork_of(@project)
@target_branch ||= @mr_source_project.repository.next_branch('patch')
end
# Merge request to this project
@mr_target_project = @project
@mr_target_branch ||= @ref || @target_branch
@mr_source_branch = @target_branch
(different_project? || @start_branch != @branch_name)
end
end
app/controllers/projects/application_controller.rb
......@@ -89,9 +89,4 @@ class Projects::ApplicationController < ApplicationController
def builds_enabled
return render_404 unless @project.feature_available?(:builds, current_user)
end
def update_ref
branch_exists = @repository.find_branch(@target_branch)
@ref = @target_branch if branch_exists
end
end
app/controllers/projects/blob_controller.rb
......@@ -25,10 +25,10 @@ class Projects::BlobController < Projects::ApplicationController
end
def create
update_ref
set_start_branch_to_branch_name
create_commit(Files::CreateService, success_notice: "The file has been successfully created.",
success_path: -> { namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)) },
success_path: -> { namespace_project_blob_path(@project.namespace, @project, File.join(@branch_name, @file_path)) },
failure_view: :new,
failure_path: namespace_project_new_blob_path(@project.namespace, @project, @ref))
end
......@@ -69,8 +69,8 @@ class Projects::BlobController < Projects::ApplicationController
end
def destroy
create_commit(Files::DestroyService, success_notice: "The file has been successfully deleted.",
success_path: -> { namespace_project_tree_path(@project.namespace, @project, @target_branch) },
create_commit(Files::DeleteService, success_notice: "The file has been successfully deleted.",
success_path: -> { namespace_project_tree_path(@project.namespace, @project, @branch_name) },
failure_view: :show,
failure_path: namespace_project_blob_path(@project.namespace, @project, @id))
end
......@@ -127,16 +127,16 @@ class Projects::BlobController < Projects::ApplicationController
def after_edit_path
from_merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:from_merge_request_iid])
if from_merge_request && @target_branch == @ref
if from_merge_request && @branch_name == @ref
diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) +
"##{hexdigest(@path)}"
else
namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path))
namespace_project_blob_path(@project.namespace, @project, File.join(@branch_name, @path))
end
end
def editor_variables
@target_branch = params[:target_branch]
@branch_name = params[:branch_name]
@file_path =
if action_name.to_s == 'create'
......
app/controllers/projects/commit_controller.rb
......@@ -56,9 +56,7 @@ class Projects::CommitController < Projects::ApplicationController
return render_404 if @start_branch.blank?
@target_branch = create_new_branch? ? @commit.revert_branch_name : @start_branch
@mr_target_branch = @start_branch
@branch_name = create_new_branch? ? @commit.revert_branch_name : @start_branch
create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully reverted.",
success_path: -> { successful_change_path }, failure_path: failed_change_path)
......@@ -69,9 +67,7 @@ class Projects::CommitController < Projects::ApplicationController
return render_404 if @start_branch.blank?
@target_branch = create_new_branch? ? @commit.cherry_pick_branch_name : @start_branch
@mr_target_branch = @start_branch
@branch_name = create_new_branch? ? @commit.cherry_pick_branch_name : @start_branch
create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully cherry-picked.",
success_path: -> { successful_change_path }, failure_path: failed_change_path)
......@@ -84,7 +80,7 @@ class Projects::CommitController < Projects::ApplicationController
end
def successful_change_path
referenced_merge_request_url || namespace_project_commits_url(@project.namespace, @project, @target_branch)
referenced_merge_request_url || namespace_project_commits_url(@project.namespace, @project, @branch_name)
end
def failed_change_path
......
app/controllers/projects/tree_controller.rb
......@@ -34,16 +34,16 @@ class Projects::TreeController < Projects::ApplicationController
def create_dir
return render_404 unless @commit_params.values.all?
update_ref
set_start_branch_to_branch_name
create_commit(Files::CreateDirService, success_notice: "The directory has been successfully created.",
success_path: namespace_project_tree_path(@project.namespace, @project, File.join(@target_branch, @dir_name)),
success_path: namespace_project_tree_path(@project.namespace, @project, File.join(@branch_name, @dir_name)),
failure_path: namespace_project_tree_path(@project.namespace, @project, @ref))
end
private
def assign_dir_vars
@target_branch = params[:target_branch]
@branch_name = params[:branch_name]
@dir_name = File.join(@path, params[:dir_name])
@commit_params = {
......
app/helpers/projects_helper.rb
......@@ -272,14 +272,14 @@ module ProjectsHelper
end
end
def add_special_file_path(project, file_name:, commit_message: nil, target_branch: nil, context: nil)
def add_special_file_path(project, file_name:, commit_message: nil, branch_name: nil, context: nil)
namespace_project_new_blob_path(
project.namespace,
project,
project.default_branch || 'master',
file_name: file_name,
commit_message: commit_message || "Add #{file_name.downcase}",
target_branch: target_branch,
branch_name: branch_name,
context: context
)
end
......
app/helpers/tree_helper.rb
......@@ -35,7 +35,7 @@ module TreeHelper
end
def on_top_of_branch?(project = @project, ref = @ref)
project.repository.branch_names.include?(ref)
project.repository.branch_exists?(ref)
end
def can_edit_tree?(project = nil, ref = nil)
......
app/services/commits/change_service.rb
module Commits
class ChangeService < ::BaseService
ValidationError = Class.new(StandardError)
ChangeError = Class.new(StandardError)
class ChangeService < Commits::CreateService
def initialize(*args)
super
def execute
@start_project = params[:start_project] || @project
@start_branch = params[:start_branch]
@target_branch = params[:target_branch]
@commit = params[:commit]
check_push_permissions
commit
rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
ValidationError, ChangeError => ex
error(ex.message)
end
private
def commit
raise NotImplementedError
end
def commit_change(action)
raise NotImplementedError unless repository.respond_to?(action)
validate_target_branch if different_branch?
repository.public_send(
action,
current_user,
@commit,
@target_branch,
@branch_name,
start_project: @start_project,
start_branch_name: @start_branch)
success
rescue Repository::CreateTreeError
error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
A #{action.to_s.dasherize} may have already been performed with this #{@commit.change_type_title(current_user)}, or a more recent commit may have updated some of its content."
This #{@commit.change_type_title(current_user)} may already have been #{action.to_s.dasherize}ed, or a more recent commit may have updated some of its content."
raise ChangeError, error_msg
end
def check_push_permissions
allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
unless allowed
raise ValidationError.new('You are not allowed to push into this branch')
end
true
end
def validate_target_branch
result = ValidateNewBranchService.new(@project, current_user)
.execute(@target_branch)
if result[:status] == :error
raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
end
end
def different_branch?
@start_branch != @target_branch || @start_project != @project
end
end
end
app/services/commits/cherry_pick_service.rb
module Commits
class CherryPickService < ChangeService
def commit
def create_commit!
commit_change(:cherry_pick)
end
end
......
app/services/commits/create_service.rb 0 → 100644
module Commits
class CreateService < ::BaseService
ValidationError = Class.new(StandardError)
ChangeError = Class.new(StandardError)
def initialize(*args)
super
@start_project = params[:start_project] || @project
@start_branch = params[:start_branch]
@branch_name = params[:branch_name]
end
def execute
validate!
new_commit = create_commit!
success(result: new_commit)
rescue ValidationError, ChangeError, Gitlab::Git::Index::IndexError, Repository::CommitError, GitHooksService::PreReceiveError => ex
error(ex.message)
end
private
def create_commit!
raise NotImplementedError
end
def raise_error(message)
raise ValidationError, message
end
def different_branch?
@start_branch != @branch_name || @start_project != @project
end
def validate!
validate_permissions!
validate_on_branch!
validate_branch_existance!
validate_new_branch_name! if different_branch?
end
def validate_permissions!
allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@branch_name)
unless allowed
raise_error("You are not allowed to push into this branch")
end
end
def validate_on_branch!
if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch)
raise_error('You can only create or edit files when you are on a branch')
end
end
def validate_branch_existance!
if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name)
raise_error("A branch called '#{@branch_name}' already exists. Switch to that branch in order to make changes")
end
end
def validate_new_branch_name!
result = ValidateNewBranchService.new(project, current_user).execute(@branch_name)
if result[:status] == :error
raise_error("Something went wrong when we tried to create '#{@branch_name}' for you: #{result[:message]}")
end
end
end
end
app/services/commits/revert_service.rb
module Commits
class RevertService < ChangeService
def commit
def create_commit!
commit_change(:revert)
end
end
......
app/services/files/base_service.rb
module Files
class BaseService < ::BaseService
ValidationError = Class.new(StandardError)
class BaseService < Commits::CreateService
def initialize(*args)
super
def execute
@start_project = params[:start_project] || @project
@start_branch = params[:start_branch]
@target_branch = params[:target_branch]
@commit_message = params[:commit_message]
@file_path = params[:file_path]
@previous_path = params[:previous_path]
@file_content = if params[:file_content_encoding] == 'base64'
Base64.decode64(params[:file_content])
else
params[:file_content]
end
@last_commit_sha = params[:last_commit_sha]
@author_email = params[:author_email]
@author_name = params[:author_name]
@commit_message = params[:commit_message]
# Validate parameters
validate
# Create new branch if it different from start_branch
validate_target_branch if different_branch?
result = commit
if result
success(result: result)
else
error('Something went wrong. Your changes were not committed')
end
rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, ValidationError => ex
error(ex.message)
end
private
def different_branch?
@start_branch != @target_branch || @start_project != @project
end
def file_has_changed?
return false unless @last_commit_sha && last_commit
@last_commit_sha != last_commit.sha
end
def raise_error(message)
raise ValidationError.new(message)
end
def validate
allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
unless allowed
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"
end
end
def validate_target_branch
result = ValidateNewBranchService.new(project, current_user).
execute(@target_branch)
@file_path = params[:file_path]
@previous_path = params[:previous_path]
if result[:status] == :error
raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}")
end
@file_content = params[:file_content]
@file_content = Base64.decode64(@file_content) if params[:file_content_encoding] == 'base64'
end
end
end
app/services/files/create_dir_service.rb
module Files
class CreateDirService < Files::BaseService
def commit
def create_commit!
repository.create_dir(
current_user,
@file_path,
message: @commit_message,
branch_name: @target_branch,
branch_name: @branch_name,
author_email: @author_email,
author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch)
end
def validate
super
unless @file_path =~ Gitlab::Regex.file_path_regex
raise_error(
'Your changes could not be committed, because the file path ' +
Gitlab::Regex.file_path_regex_message
)
end
end
end
end
app/services/files/create_service.rb
module Files
class CreateService < Files::BaseService
def commit
def create_commit!
repository.create_file(
current_user,
@file_path,
@file_content,
message: @commit_message,
branch_name: @target_branch,
branch_name: @branch_name,
author_email: @author_email,
author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch)
end
def validate
super
if @file_content.nil?
raise_error("You must provide content.")
end
if @file_path =~ Gitlab::Regex.directory_traversal_regex
raise_error(
'Your changes could not be committed, because the file name ' +
Gitlab::Regex.directory_traversal_regex_message
)
end
unless @file_path =~ Gitlab::Regex.file_path_regex
raise_error(
'Your changes could not be committed, because the file name ' +
Gitlab::Regex.file_path_regex_message
)
end
unless project.empty_repo?
@file_path.slice!(0) if @file_path.start_with?('/')
blob = repository.blob_at_branch(@start_branch, @file_path)
if blob
raise_error('Your changes could not be committed because a file with the same name already exists')
end
end
end
end
end
app/services/files/destroy_service.rb → app/services/files/delete_service.rb
module Files
class DestroyService < Files::BaseService
def commit
class DeleteService < Files::BaseService
def create_commit!
repository.delete_file(
current_user,
@file_path,
message: @commit_message,
branch_name: @target_branch,
branch_name: @branch_name,
author_email: @author_email,
author_name: @author_name,
start_project: @start_project,
......
app/services/files/multi_service.rb
module Files
class MultiService < Files::BaseService
FileChangedError = Class.new(StandardError)
ACTIONS = %w[create update delete move].freeze
def commit
def create_commit!
repository.multi_action(
user: current_user,
message: @commit_message,
branch_name: @target_branch,
branch_name: @branch_name,
actions: params[:actions],
author_email: @author_email,
author_name: @author_name,
......@@ -19,122 +15,17 @@ module Files
private
def validate
def validate!
super
params[:actions].each_with_index do |action, index|
if ACTIONS.include?(action[:action].to_s)
action[:action] = action[:action].to_sym
else
raise_error("Unknown action type `#{action[:action]}`.")
end
unless action[:file_path].present?
raise_error("You must specify a file_path.")
end
action[:file_path].slice!(0) if action[:file_path] && action[:file_path].start_with?('/')
action[:previous_path].slice!(0) if action[:previous_path] && action[:previous_path].start_with?('/')
regex_check(action[:file_path])
regex_check(action[:previous_path]) if action[:previous_path]
if project.empty_repo? && action[:action] != :create
raise_error("No files to #{action[:action]}.")
end
validate_file_exists(action)
case action[:action]
when :create
validate_create(action)
when :update
validate_update(action)
when :delete
validate_delete(action)
when :move
validate_move(action, index)
end
params[:actions].each do |action|
validate_action!(action)
end
end
def validate_file_exists(action)
return if action[:action] == :create
file_path = action[:file_path]
file_path = action[:previous_path] if action[:action] == :move
blob = repository.blob_at_branch(params[:branch], file_path)
unless blob
raise_error("File to be #{action[:action]}d `#{file_path}` does not exist.")
end
end
def last_commit
Gitlab::Git::Commit.last_for_path(repository, @start_branch, @file_path)
end
def regex_check(file)
if file =~ Gitlab::Regex.directory_traversal_regex
raise_error(
'Your changes could not be committed, because the file name, `' +
file +
'` ' +
Gitlab::Regex.directory_traversal_regex_message
)
end
unless file =~ Gitlab::Regex.file_path_regex
raise_error(
'Your changes could not be committed, because the file name, `' +
file +
'` ' +
Gitlab::Regex.file_path_regex_message
)
end
end
def validate_create(action)
return if project.empty_repo?
if repository.blob_at_branch(params[:branch], action[:file_path])
raise_error("Your changes could not be committed because a file with the name `#{action[:file_path]}` already exists.")
end
if action[:content].nil?
raise_error("You must provide content.")
end
end
def validate_update(action)
if action[:content].nil?
raise_error("You must provide content.")
end
if file_has_changed?
raise FileChangedError.new("You are attempting to update a file `#{action[:file_path]}` that has changed since you started editing it.")
end
end
def validate_delete(action)
end
def validate_move(action, index)
if action[:previous_path].nil?
raise_error("You must supply the original file path when moving file `#{action[:file_path]}`.")
end
blob = repository.blob_at_branch(params[:branch], action[:file_path])
if blob
raise_error("Move destination `#{action[:file_path]}` already exists.")
end
if action[:content].nil?
blob = repository.blob_at_branch(params[:branch], action[:previous_path])
blob.load_all_data!(repository) if blob.truncated?
params[:actions][index][:content] = blob.data
def validate_action!(action)
unless Gitlab::Git::Index::ACTIONS.include?(action[:action].to_s)
raise_error("Unknown action '#{action[:action]}'")
end
end
end
......
app/services/files/update_service.rb
......@@ -2,10 +2,16 @@ module Files
class UpdateService < Files::BaseService
FileChangedError = Class.new(StandardError)
def commit
def initialize(*args)
super
@last_commit_sha = params[:last_commit_sha]
end
def create_commit!
repository.update_file(current_user, @file_path, @file_content,
message: @commit_message,
branch_name: @target_branch,
branch_name: @branch_name,
previous_path: @previous_path,
author_email: @author_email,
author_name: @author_name,
......@@ -15,21 +21,23 @@ module Files
private
def validate
super
def file_has_changed?
return false unless @last_commit_sha && last_commit
if @file_content.nil?
raise_error("You must provide content.")
end
if file_has_changed?
raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.")
end
@last_commit_sha != last_commit.sha
end
def last_commit
@last_commit ||= Gitlab::Git::Commit.
last_for_path(@start_project.repository, @start_branch, @file_path)
end
def validate!
super
if file_has_changed?
raise FileChangedError, "You are attempting to update a file that has changed since you started editing it."
end
end
end
end
app/services/validate_new_branch_service.rb
......@@ -8,10 +8,7 @@ class ValidateNewBranchService < BaseService
return error('Branch name is invalid')
end
repository = project.repository
existing_branch = repository.find_branch(branch_name)
if existing_branch
if project.repository.branch_exists?(branch_name)
return error('Branch already exists')
end
......
app/views/projects/blob/edit.html.haml
......@@ -9,7 +9,7 @@
- if @conflict
.alert.alert-danger
Someone edited the file the same time you did. Please check out
= link_to "the file", namespace_project_blob_path(@project.namespace, @project, tree_join(@target_branch, @file_path)), target: "_blank", rel: 'noopener noreferrer'
= link_to "the file", namespace_project_blob_path(@project.namespace, @project, tree_join(@branch_name, @file_path)), target: "_blank", rel: 'noopener noreferrer'
and make sure your changes will not unintentionally remove theirs.
.editor-title-row
%h3.page-title.blob-edit-page-title
......
app/views/projects/show.html.haml
......@@ -70,7 +70,7 @@
= link_to 'Set up Koding', add_koding_stack_path(@project)
- if @repository.gitlab_ci_yml.blank? && @project.deployment_service.present?
%li.missing
= link_to add_special_file_path(@project, file_name: '.gitlab-ci.yml', commit_message: 'Set up auto deploy', target_branch: 'auto-deploy', context: 'autodeploy') do
= link_to add_special_file_path(@project, file_name: '.gitlab-ci.yml', commit_message: 'Set up auto deploy', branch_name: 'auto-deploy', context: 'autodeploy') do
Set up auto deploy
- if @repository.commit
......
app/views/shared/_branch_switcher.html.haml
- dropdown_toggle_text = @target_branch || tree_edit_branch
= hidden_field_tag 'target_branch', dropdown_toggle_text
- dropdown_toggle_text = @branch_name || tree_edit_branch
= hidden_field_tag 'branch_name', dropdown_toggle_text
.dropdown
= dropdown_toggle dropdown_toggle_text, { toggle: 'dropdown', selected: dropdown_toggle_text, field_name: 'target_branch', form_id: '.js-edit-blob-form', refs_url: namespace_project_branches_path(@project.namespace, @project) }, { toggle_class: 'js-project-branches-dropdown js-target-branch' }
= dropdown_toggle dropdown_toggle_text, { toggle: 'dropdown', selected: dropdown_toggle_text, field_name: 'branch_name', form_id: '.js-edit-blob-form', refs_url: namespace_project_branches_path(@project.namespace, @project) }, { toggle_class: 'js-project-branches-dropdown js-target-branch' }
.dropdown-menu.dropdown-menu-selectable.dropdown-menu-paging.dropdown-menu-branches
= render partial: 'shared/projects/blob/branch_page_default'
= render partial: 'shared/projects/blob/branch_page_create'
app/views/shared/_new_commit_form.html.haml
= render 'shared/commit_message_container', placeholder: placeholder
- if @project.empty_repo?
= hidden_field_tag 'target_branch', @ref
= hidden_field_tag 'branch_name', @ref
- else
- if can?(current_user, :push_code, @project)
.form-group.branch
= label_tag 'target_branch', 'Target branch', class: 'control-label'
= label_tag 'branch_name', 'Target branch', class: 'control-label'
.col-sm-10
= render 'shared/branch_switcher'
......@@ -16,7 +16,7 @@
= check_box_tag 'create_merge_request', 1, true, class: 'js-create-merge-request', id: "create_merge_request-#{nonce}"
Start a <strong>new merge request</strong> with these changes
- else
= hidden_field_tag 'target_branch', @target_branch || tree_edit_branch
= hidden_field_tag 'branch_name', @branch_name || tree_edit_branch
= hidden_field_tag 'create_merge_request', 1
= hidden_field_tag 'original_branch', @ref, class: 'js-original-branch'
features/project/merge_requests/revert.feature
......@@ -25,7 +25,5 @@ Feature: Revert Merge Requests
@javascript
Scenario: I revert a merge request in a new merge request
Given I click on the revert button
And I am on the Merge Request detail page
And I click on the revert button
And I revert the changes in a new merge request
Then I should see the new merge request notice
features/project/source/browse_files.feature
......@@ -135,7 +135,7 @@ Feature: Project Source Browse Files
And I fill the commit message
And I click on "Commit changes"
Then I am on the new file page
And I see a commit error message
And I see "Path can contain only..."
@javascript
Scenario: I can create file with a directory name
......
features/steps/project/source/browse_files.rb
......@@ -284,7 +284,11 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
end
step 'I see "Unable to create directory"' do
expect(page).to have_content('Directory already exists')
expect(page).to have_content('A directory with this name already exists')
end
step 'I see "Path can contain only..."' do
expect(page).to have_content('Path can contain only')
end
step 'I see a commit error message' do
......
lib/api/commits.rb
......@@ -62,7 +62,7 @@ module API
post ":id/repository/commits" do
authorize! :push_code, user_project
attrs = declared_params.merge(start_branch: declared_params[:branch], target_branch: declared_params[:branch])
attrs = declared_params.merge(start_branch: declared_params[:branch], branch_name: declared_params[:branch])
result = ::Files::MultiService.new(user_project, current_user, attrs).execute
......@@ -140,7 +140,7 @@ module API
commit_params = {
commit: commit,
start_branch: params[:branch],
target_branch: params[:branch]
branch_name: params[:branch]
}
result = ::Commits::CherryPickService.new(user_project, current_user, commit_params).execute
......
lib/api/files.rb
......@@ -5,7 +5,7 @@ module API
{
file_path: attrs[:file_path],
start_branch: attrs[:branch],
target_branch: attrs[:branch],
branch_name: attrs[:branch],
commit_message: attrs[:commit_message],
file_content: attrs[:content],
file_content_encoding: attrs[:encoding],
......@@ -130,7 +130,7 @@ module API
authorize! :push_code, user_project
file_params = declared_params(include_missing: false)
result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute
result = ::Files::DeleteService.new(user_project, current_user, commit_params(file_params)).execute
if result[:status] != :success
render_api_error!(result[:message], 400)
......
lib/api/v3/commits.rb
......@@ -53,7 +53,7 @@ module API
attrs = declared_params.dup
branch = attrs.delete(:branch_name)
attrs.merge!(branch: branch, start_branch: branch, target_branch: branch)
attrs.merge!(start_branch: branch, branch_name: branch)
result = ::Files::MultiService.new(user_project, current_user, attrs).execute
......@@ -131,7 +131,7 @@ module API
commit_params = {
commit: commit,
start_branch: params[:branch],
target_branch: params[:branch]
branch_name: params[:branch]
}
result = ::Commits::CherryPickService.new(user_project, current_user, commit_params).execute
......
lib/api/v3/files.rb
......@@ -6,7 +6,7 @@ module API
{
file_path: attrs[:file_path],
start_branch: attrs[:branch],
target_branch: attrs[:branch],
branch_name: attrs[:branch],
commit_message: attrs[:commit_message],
file_content: attrs[:content],
file_content_encoding: attrs[:encoding],
......@@ -123,7 +123,7 @@ module API
file_params = declared_params(include_missing: false)
file_params[:branch] = file_params.delete(:branch_name)
result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute
result = ::Files::DeleteService.new(user_project, current_user, commit_params(file_params)).execute
if result[:status] == :success
status(200)
......
lib/gitlab/git/index.rb
module Gitlab
module Git
class Index
IndexError = Class.new(StandardError)
DEFAULT_MODE = 0o100644
ACTIONS = %w(create create_dir update move delete).freeze
attr_reader :repository, :raw_index
def initialize(repository)
......@@ -23,9 +27,8 @@ module Gitlab
def create(options)
options = normalize_options(options)
file_entry = get(options[:file_path])
if file_entry
raise Gitlab::Git::Repository::InvalidBlobName.new("Filename already exists")
if get(options[:file_path])
raise IndexError, "A file with this name already exists"
end
add_blob(options)
......@@ -34,13 +37,12 @@ module Gitlab
def create_dir(options)
options = normalize_options(options)
file_entry = get(options[:file_path])
if file_entry
raise Gitlab::Git::Repository::InvalidBlobName.new("Directory already exists as a file")
if get(options[:file_path])
raise IndexError, "A file with this name already exists"
end
if dir_exists?(options[:file_path])
raise Gitlab::Git::Repository::InvalidBlobName.new("Directory already exists")
raise IndexError, "A directory with this name already exists"
end
options = options.dup
......@@ -55,7 +57,7 @@ module Gitlab
file_entry = get(options[:file_path])
unless file_entry
raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist")
raise IndexError, "A file with this name doesn't exist"
end
add_blob(options, mode: file_entry[:mode])
......@@ -66,7 +68,11 @@ module Gitlab
file_entry = get(options[:previous_path])
unless file_entry
raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist")
raise IndexError, "A file with this name doesn't exist"
end
if get(options[:file_path])
raise IndexError, "A file with this name already exists"
end
raw_index.remove(options[:previous_path])
......@@ -77,9 +83,8 @@ module Gitlab
def delete(options)
options = normalize_options(options)
file_entry = get(options[:file_path])
unless file_entry
raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist")
unless get(options[:file_path])
raise IndexError, "A file with this name doesn't exist"
end
raw_index.remove(options[:file_path])
......@@ -95,10 +100,20 @@ module Gitlab
end
def normalize_path(path)
unless path
raise IndexError, "You must provide a file path"
end
pathname = Gitlab::Git::PathHelper.normalize_path(path.dup)
if pathname.each_filename.include?('..')
raise Gitlab::Git::Repository::InvalidBlobName.new('Invalid path')
pathname.each_filename do |segment|
if segment == '..'
raise IndexError, 'Path cannot include directory traversal'
end
unless segment =~ Gitlab::Regex.file_name_regex
raise IndexError, "Path #{Gitlab::Regex.file_name_regex_message}"
end
end
pathname.to_s
......@@ -106,6 +121,10 @@ module Gitlab
def add_blob(options, mode: nil)
content = options[:content]
unless content
raise IndexError, "You must provide content"
end
content = Base64.decode64(content) if options[:encoding] == 'base64'
detect = CharlockHolmes::EncodingDetector.new.detect(content)
......@@ -119,7 +138,7 @@ module Gitlab
raw_index.add(path: options[:file_path], oid: oid, mode: mode || DEFAULT_MODE)
rescue Rugged::IndexError => e
raise Gitlab::Git::Repository::InvalidBlobName.new(e.message)
raise IndexError, e.message
end
end
end
......
lib/gitlab/regex.rb
......@@ -73,22 +73,6 @@ module Gitlab
"can contain only letters, digits, '_', '-', '@', '+' and '.'."
end
def file_path_regex
@file_path_regex ||= /\A[[[:alnum:]]_\-\.\/\@]*\z/.freeze
end
def file_path_regex_message
"can contain only letters, digits, '_', '-', '@' and '.'. Separate directories with a '/'."
end
def directory_traversal_regex
@directory_traversal_regex ||= /\.{2}/.freeze
end
def directory_traversal_regex_message
"cannot include directory traversal."
end
def archive_formats_regex
# |zip|tar| tar.gz | tar.bz2 |
@archive_formats_regex ||= /(zip|tar|tar\.gz|tgz|gz|tar\.bz2|tbz|tbz2|tb2|bz2)/.freeze
......
spec/controllers/projects/blob_controller_spec.rb
......@@ -106,7 +106,7 @@ describe Projects::BlobController do
namespace_id: project.namespace,
project_id: project,
id: 'master/CHANGELOG',
target_branch: 'master',
branch_name: 'master',
content: 'Added changes',
commit_message: 'Update CHANGELOG'
}
......@@ -178,7 +178,7 @@ describe Projects::BlobController do
context 'when editing on the original repository' do
it "redirects to forked project new merge request" do
default_params[:target_branch] = "fork-test-1"
default_params[:branch_name] = "fork-test-1"
default_params[:create_merge_request] = 1
put :update, default_params
......
spec/controllers/projects/tree_controller_spec.rb
......@@ -97,29 +97,29 @@ describe Projects::TreeController do
project_id: project,
id: 'master',
dir_name: path,
target_branch: target_branch,
branch_name: branch_name,
commit_message: 'Test commit message')
end
context 'successful creation' do
let(:path) { 'files/new_dir'}
let(:target_branch) { 'master-test'}
let(:branch_name) { 'master-test'}
it 'redirects to the new directory' do
expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/#{target_branch}/#{path}")
to redirect_to("/#{project.path_with_namespace}/tree/#{branch_name}/#{path}")
expect(flash[:notice]).to eq('The directory has been successfully created.')
end
end
context 'unsuccessful creation' do
let(:path) { 'README.md' }
let(:target_branch) { 'master'}
let(:branch_name) { 'master'}
it 'does not allow overwriting of existing files' do
expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/master")
expect(flash[:alert]).to eq('Directory already exists as a file')
expect(flash[:alert]).to eq('A file with this name already exists')
end
end
end
......
spec/features/projects/blobs/user_create_spec.rb
......@@ -77,7 +77,7 @@ feature 'New blob creation', feature: true, js: true do
project,
user,
start_branch: 'master',
target_branch: 'master',
branch_name: 'master',
commit_message: 'Create file',
file_path: 'feature.rb',
file_content: content
......@@ -87,7 +87,7 @@ feature 'New blob creation', feature: true, js: true do
end
scenario 'shows error message' do
expect(page).to have_content('Your changes could not be committed because a file with the same name already exists')
expect(page).to have_content('A file with this name already exists')
expect(page).to have_content('New file')
expect(page).to have_content('NextFeature')
end
......
spec/features/projects/files/creating_a_file_spec.rb
......@@ -29,16 +29,16 @@ feature 'User wants to create a file', feature: true do
scenario 'directory name contains Chinese characters' do
submit_new_file(file_name: '中文/测试.md')
expect(page).to have_content 'The file has been successfully created.'
expect(page).to have_content 'The file has been successfully created'
end
scenario 'file name contains invalid characters' do
submit_new_file(file_name: '\\')
expect(page).to have_content 'Your changes could not be committed, because the file name can contain only'
expect(page).to have_content 'Path can contain only'
end
scenario 'file name contains directory traversal' do
submit_new_file(file_name: '../README.md')
expect(page).to have_content 'Your changes could not be committed, because the file name cannot include directory traversal.'
expect(page).to have_content 'Path cannot include directory traversal'
end
end
spec/features/projects/files/editing_a_file_spec.rb
......@@ -8,7 +8,7 @@ feature 'User wants to edit a file', feature: true do
let(:commit_params) do
{
start_branch: project.default_branch,
target_branch: project.default_branch,
branch_name: project.default_branch,
commit_message: "Committing First Update",
file_path: ".gitignore",
file_content: "First Update",
......
spec/features/projects/view_on_env_spec.rb
......@@ -25,7 +25,7 @@ describe 'View on environment', js: true do
project,
user,
start_branch: branch_name,
target_branch: branch_name,
branch_name: branch_name,
commit_message: "Add .gitlab/route-map.yml",
file_path: '.gitlab/route-map.yml',
file_content: route_map
......@@ -36,7 +36,7 @@ describe 'View on environment', js: true do
project,
user,
start_branch: branch_name,
target_branch: branch_name,
branch_name: branch_name,
commit_message: "Update feature",
file_path: file_path,
file_content: "# Noop"
......
spec/lib/gitlab/diff/position_tracer_spec.rb
......@@ -100,7 +100,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
project,
current_user,
start_branch: branch_name,
target_branch: branch_name,
branch_name: branch_name,
commit_message: "Create file",
file_path: file_name,
file_content: content
......@@ -113,7 +113,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
project,
current_user,
start_branch: branch_name,
target_branch: branch_name,
branch_name: branch_name,
commit_message: "Update file",
file_path: file_name,
file_content: content
......@@ -122,11 +122,11 @@ describe Gitlab::Diff::PositionTracer, lib: true do
end
def delete_file(branch_name, file_name)
Files::DestroyService.new(
Files::DeleteService.new(
project,
current_user,
start_branch: branch_name,
target_branch: branch_name,
branch_name: branch_name,
commit_message: "Delete file",
file_path: file_name
).execute
......
spec/lib/gitlab/git/index_spec.rb
......@@ -33,7 +33,7 @@ describe Gitlab::Git::Index, seed_helper: true do
end
it 'raises an error' do
expect { index.create(options) }.to raise_error('Filename already exists')
expect { index.create(options) }.to raise_error('A file with this name already exists')
end
end
......@@ -89,7 +89,7 @@ describe Gitlab::Git::Index, seed_helper: true do
end
it 'raises an error' do
expect { index.create_dir(options) }.to raise_error('Directory already exists as a file')
expect { index.create_dir(options) }.to raise_error('A file with this name already exists')
end
end
......@@ -99,7 +99,7 @@ describe Gitlab::Git::Index, seed_helper: true do
end
it 'raises an error' do
expect { index.create_dir(options) }.to raise_error('Directory already exists')
expect { index.create_dir(options) }.to raise_error('A directory with this name already exists')
end
end
end
......@@ -118,7 +118,7 @@ describe Gitlab::Git::Index, seed_helper: true do
end
it 'raises an error' do
expect { index.update(options) }.to raise_error("File doesn't exist")
expect { index.update(options) }.to raise_error("A file with this name doesn't exist")
end
end
......@@ -156,7 +156,15 @@ describe Gitlab::Git::Index, seed_helper: true do
it 'raises an error' do
options[:previous_path] = 'documents/story.txt'
expect { index.move(options) }.to raise_error("File doesn't exist")
expect { index.move(options) }.to raise_error("A file with this name doesn't exist")
end
end
context 'when a file at the new path already exists' do
it 'raises an error' do
options[:file_path] = 'CHANGELOG'
expect { index.move(options) }.to raise_error("A file with this name already exists")
end
end
......@@ -203,7 +211,7 @@ describe Gitlab::Git::Index, seed_helper: true do
end
it 'raises an error' do
expect { index.delete(options) }.to raise_error("File doesn't exist")
expect { index.delete(options) }.to raise_error("A file with this name doesn't exist")
end
end
......
spec/lib/gitlab/git_access_spec.rb
......@@ -211,7 +211,7 @@ describe Gitlab::GitAccess, lib: true do
target_branch = project.repository.lookup('feature')
source_branch = project.repository.create_file(
user,
'John Doe',
'filename',
'This is the file content',
message: 'This is a good commit message',
branch_name: unprotected_branch)
......
spec/lib/gitlab/regex_spec.rb
......@@ -32,12 +32,6 @@ describe Gitlab::Regex, lib: true do
it { is_expected.to match('foo@bar') }
end
describe '.file_path_regex' do
subject { described_class.file_path_regex }
it { is_expected.to match('foo@/bar') }
end
describe '.environment_slug_regex' do
subject { described_class.environment_slug_regex }
......
spec/requests/api/commits_spec.rb
......@@ -599,8 +599,7 @@ describe API::Commits, api: true do
post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'markdown'
expect(response).to have_http_status(400)
expect(json_response['message']).to eq('Sorry, we cannot cherry-pick this commit automatically.
A cherry-pick may have already been performed with this commit, or a more recent commit may have updated some of its content.')
expect(json_response['message']).to include('Sorry, we cannot cherry-pick this commit automatically.')
end
it 'returns 400 if you are not allowed to push to the target branch' do
......
spec/requests/api/files_spec.rb
......@@ -205,7 +205,7 @@ describe API::Files, api: true do
it "returns a 400 if editor fails to create file" do
allow_any_instance_of(Repository).to receive(:create_file).
and_return(false)
and_raise(Repository::CommitError, 'Cannot create file')
post api(route("any%2Etxt"), user), valid_params
......@@ -299,8 +299,8 @@ describe API::Files, api: true do
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)
it "returns a 400 if fails to delete file" do
allow_any_instance_of(Repository).to receive(:delete_file).and_raise(Repository::CommitError, 'Cannot delete file')
delete api(route(file_path), user), valid_params
......
spec/requests/api/v3/commits_spec.rb
......@@ -485,8 +485,7 @@ describe API::V3::Commits, api: true do
post v3_api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'markdown'
expect(response).to have_http_status(400)
expect(json_response['message']).to eq('Sorry, we cannot cherry-pick this commit automatically.
A cherry-pick may have already been performed with this commit, or a more recent commit may have updated some of its content.')
expect(json_response['message']).to include('Sorry, we cannot cherry-pick this commit automatically.')
end
it 'returns 400 if you are not allowed to push to the target branch' do
......
spec/requests/api/v3/files_spec.rb
......@@ -129,7 +129,7 @@ describe API::V3::Files, api: true do
it "returns a 400 if editor fails to create file" do
allow_any_instance_of(Repository).to receive(:create_file).
and_return(false)
and_raise(Repository::CommitError, 'Cannot create file')
post v3_api("/projects/#{project.id}/repository/files", user), valid_params
......@@ -229,8 +229,8 @@ describe API::V3::Files, api: true do
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)
it "returns a 400 if fails to delete file" do
allow_any_instance_of(Repository).to receive(:delete_file).and_raise(Repository::CommitError, 'Cannot delete file')
delete v3_api("/projects/#{project.id}/repository/files", user), valid_params
......
spec/services/files/update_service_spec.rb
......@@ -7,7 +7,7 @@ describe Files::UpdateService do
let(:user) { create(:user) }
let(:file_path) { 'files/ruby/popen.rb' }
let(:new_contents) { 'New Content' }
let(:target_branch) { project.default_branch }
let(:branch_name) { project.default_branch }
let(:last_commit_sha) { nil }
let(:commit_params) do
......@@ -19,7 +19,7 @@ describe Files::UpdateService do
last_commit_sha: last_commit_sha,
start_project: project,
start_branch: project.default_branch,
target_branch: target_branch
branch_name: branch_name
}
end
......@@ -73,7 +73,7 @@ describe Files::UpdateService do
end
context 'when target branch is different than source branch' do
let(:target_branch) { "#{project.default_branch}-new" }
let(:branch_name) { "#{project.default_branch}-new" }
it 'fires hooks only once' do
expect(GitHooksService).to receive(:new).once.and_call_original
......
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