Commit 68c1b154 authored by Vijay Hawoldar's avatar Vijay Hawoldar

Snippet REST API update with multi file support

Adds support for `files` parameter in the personal snippet
update endpoint
parent 9823cc80
......@@ -345,6 +345,10 @@ class Snippet < ApplicationRecord
repository.ls_files(ref)
end
def multiple_files?
list_files(repository.root_ref).size > 1
end
class << self
# Searches for snippets with a matching title, description or file name.
#
......
......@@ -537,6 +537,10 @@ module API
)
end
def with_api_params(&block)
yield({ api: true, request: request })
end
protected
def project_finder_params_visibility_ce
......
......@@ -27,6 +27,20 @@ module API
exactly_one_of :files, :content
end
params :update_file_params do |options|
optional :files, type: Array, desc: 'An array of files to update' do
requires :action, type: String,
values: SnippetInputAction::ACTIONS.map(&:to_s),
desc: "The type of action to perform on the file, must be one of: #{SnippetInputAction::ACTIONS.join(", ")}"
optional :content, type: String, desc: 'The content of a snippet'
optional :file_path, file_path: true, type: String, desc: 'The file path of a snippet file'
optional :previous_path, file_path: true, type: String, desc: 'The previous path of a snippet file'
end
mutually_exclusive :files, :content
mutually_exclusive :files, :file_name
end
def content_for(snippet)
if snippet.empty_repo?
env['api.format'] = :txt
......@@ -53,10 +67,30 @@ module API
end
end
def process_file_args(args)
args[:snippet_actions] = args.delete(:files)&.map do |file|
file[:action] = :create
file.symbolize_keys
def process_create_params(args)
with_api_params do |api_params|
args[:snippet_actions] = args.delete(:files)&.map do |file|
file[:action] = :create
file.symbolize_keys
end
args.merge(api_params)
end
end
def process_update_params(args)
with_api_params do |api_params|
args[:snippet_actions] = args.delete(:files)&.map(&:symbolize_keys)
args.merge(api_params)
end
end
def validate_params_for_multiple_files(snippet)
return unless params[:content] || params[:file_name]
if Feature.enabled?(:snippet_multiple_files, current_user) && snippet.multiple_files?
render_api_error!({ error: _('To update Snippets with multiple files, you must use the `files` parameter') }, 400)
end
end
end
......
......@@ -64,12 +64,8 @@ module API
end
post ":id/snippets" do
authorize! :create_snippet, user_project
snippet_params = declared_params(include_missing: false).tap do |create_args|
create_args[:request] = request
create_args[:api] = true
process_file_args(create_args)
end
snippet_params = process_create_params(declared_params(include_missing: false))
service_response = ::Snippets::CreateService.new(user_project, current_user, snippet_params).execute
snippet = service_response.payload[:snippet]
......
......@@ -76,12 +76,7 @@ module API
post do
authorize! :create_snippet
attrs = declared_params(include_missing: false).tap do |create_args|
create_args[:request] = request
create_args[:api] = true
process_file_args(create_args)
end
attrs = process_create_params(declared_params(include_missing: false))
service_response = ::Snippets::CreateService.new(nil, current_user, attrs).execute
snippet = service_response.payload[:snippet]
......@@ -99,16 +94,20 @@ module API
detail 'This feature was introduced in GitLab 8.15.'
success Entities::PersonalSnippet
end
params do
requires :id, type: Integer, desc: 'The ID of a snippet'
optional :title, type: String, allow_blank: false, desc: 'The title of a snippet'
optional :file_name, type: String, desc: 'The name of a snippet file'
optional :content, type: String, allow_blank: false, desc: 'The content of a snippet'
optional :description, type: String, desc: 'The description of a snippet'
optional :file_name, type: String, desc: 'The name of a snippet file'
optional :title, type: String, allow_blank: false, desc: 'The title of a snippet'
optional :visibility, type: String,
values: Gitlab::VisibilityLevel.string_values,
desc: 'The visibility of the snippet'
at_least_one_of :title, :file_name, :content, :visibility
use :update_file_params
at_least_one_of :title, :file_name, :content, :files, :visibility
end
put ':id' do
snippet = snippets_for_current_user.find_by_id(params.delete(:id))
......@@ -116,8 +115,12 @@ module API
authorize! :update_snippet, snippet
attrs = declared_params(include_missing: false).merge(request: request, api: true)
validate_params_for_multiple_files(snippet)
attrs = process_update_params(declared_params(include_missing: false))
service_response = ::Snippets::UpdateService.new(nil, current_user, attrs).execute(snippet)
snippet = service_response.payload[:snippet]
if service_response.success?
......
......@@ -25977,6 +25977,9 @@ msgstr ""
msgid "To unsubscribe from this issue, please paste the following link into your browser:"
msgstr ""
msgid "To update Snippets with multiple files, you must use the `files` parameter"
msgstr ""
msgid "To view all %{scannedResourcesCount} scanned URLs, please download the CSV file"
msgstr ""
......
......@@ -787,4 +787,26 @@ RSpec.describe Snippet do
end
end
end
describe '#multiple_files?' do
subject { snippet.multiple_files? }
context 'when snippet has multiple files' do
let(:snippet) { create(:snippet, :repository) }
it { is_expected.to be_truthy }
end
context 'when snippet does not have multiple files' do
let(:snippet) { create(:snippet, :empty_repo) }
it { is_expected.to be_falsey }
end
context 'when the snippet does not have a repository' do
let(:snippet) { build(:snippet) }
it { is_expected.to be_falsey }
end
end
end
......@@ -391,21 +391,98 @@ RSpec.describe API::Snippets do
create(:personal_snippet, :repository, author: user, visibility_level: visibility_level)
end
shared_examples 'snippet updates' do
it 'updates a snippet' do
new_content = 'New content'
let(:create_action) { { action: 'create', file_path: 'foo.txt', content: 'bar' } }
let(:update_action) { { action: 'update', file_path: 'CHANGELOG', content: 'bar' } }
let(:move_action) { { action: 'move', file_path: '.old-gitattributes', previous_path: '.gitattributes' } }
let(:delete_action) { { action: 'delete', file_path: 'CONTRIBUTING.md' } }
let(:bad_file_path) { { action: 'create', file_path: '../../etc/passwd', content: 'bar' } }
let(:bad_previous_path) { { action: 'create', previous_path: '../../etc/passwd', file_path: 'CHANGELOG', content: 'bar' } }
let(:invalid_move) { { action: 'move', file_path: 'missing_previous_path.txt' } }
context 'with snippet file changes' do
using RSpec::Parameterized::TableSyntax
where(:is_multi_file, :file_name, :content, :files, :status) do
true | nil | nil | [create_action] | :success
true | nil | nil | [update_action] | :success
true | nil | nil | [move_action] | :success
true | nil | nil | [delete_action] | :success
true | nil | nil | [create_action, update_action] | :success
true | 'foo.txt' | 'bar' | [create_action] | :bad_request
true | 'foo.txt' | 'bar' | nil | :bad_request
true | nil | nil | nil | :bad_request
true | 'foo.txt' | nil | [create_action] | :bad_request
true | nil | 'bar' | [create_action] | :bad_request
true | '' | nil | [create_action] | :bad_request
true | nil | '' | [create_action] | :bad_request
true | nil | nil | [bad_file_path] | :bad_request
true | nil | nil | [bad_previous_path] | :bad_request
true | nil | nil | [invalid_move] | :forbidden
false | 'foo.txt' | 'bar' | nil | :success
false | 'foo.txt' | nil | nil | :success
false | nil | 'bar' | nil | :success
false | 'foo.txt' | 'bar' | [create_action] | :bad_request
false | nil | nil | nil | :bad_request
false | nil | '' | nil | :bad_request
false | nil | nil | [bad_file_path] | :bad_request
false | nil | nil | [bad_previous_path] | :bad_request
end
with_them do
before do
allow_any_instance_of(Snippet).to receive(:multiple_files?).and_return(is_multi_file)
end
it 'has the correct response' do
update_params = {}.tap do |params|
params[:files] = files if files
params[:file_name] = file_name if file_name
params[:content] = content if content
end
update_snippet(params: update_params)
expect(response).to have_gitlab_http_status(status)
end
end
context 'when save fails due to a repository commit error' do
before do
allow_next_instance_of(Repository) do |instance|
allow(instance).to receive(:multi_action).and_raise(Gitlab::Git::CommitError)
end
update_snippet(params: { files: [create_action] })
end
it 'returns a bad request response' do
expect(response).to have_gitlab_http_status(:bad_request)
end
end
end
shared_examples 'snippet non-file updates' do
it 'updates a snippet non-file attributes' do
new_description = 'New description'
new_title = 'New title'
new_visibility = 'internal'
update_snippet(params: { content: new_content, description: new_description, visibility: 'internal' })
update_snippet(params: { title: new_title, description: new_description, visibility: new_visibility })
expect(response).to have_gitlab_http_status(:ok)
snippet.reload
expect(snippet.content).to eq(new_content)
expect(snippet.description).to eq(new_description)
expect(snippet.visibility).to eq('internal')
aggregate_failures do
expect(response).to have_gitlab_http_status(:ok)
expect(snippet.description).to eq(new_description)
expect(snippet.visibility).to eq(new_visibility)
expect(snippet.title).to eq(new_title)
end
end
end
it_behaves_like 'snippet non-file updates'
context 'with restricted visibility settings' do
before do
stub_application_setting(restricted_visibility_levels:
......@@ -413,11 +490,9 @@ RSpec.describe API::Snippets do
Gitlab::VisibilityLevel::PRIVATE])
end
it_behaves_like 'snippet updates'
it_behaves_like 'snippet non-file updates'
end
it_behaves_like 'snippet updates'
it 'returns 404 for invalid snippet id' do
update_snippet(snippet_id: non_existing_record_id, params: { title: 'Foo' })
......@@ -438,13 +513,6 @@ RSpec.describe API::Snippets do
expect(response).to have_gitlab_http_status(:bad_request)
end
it 'returns 400 if content is blank' do
update_snippet(params: { content: '' })
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'content is empty'
end
it 'returns 400 if title is blank' do
update_snippet(params: { title: '' })
......
......@@ -2,6 +2,10 @@
RSpec.shared_examples 'update with repository actions' do
context 'when the repository exists' do
before do
allow_any_instance_of(Snippet).to receive(:multiple_files?).and_return(false)
end
it 'commits the changes to the repository' do
existing_blob = snippet.blobs.first
new_file_name = existing_blob.path + '_new'
......
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