diff options
Diffstat (limited to 'app/services/snippets')
| -rw-r--r-- | app/services/snippets/base_service.rb | 41 | ||||
| -rw-r--r-- | app/services/snippets/create_service.rb | 40 | ||||
| -rw-r--r-- | app/services/snippets/update_service.rb | 47 |
3 files changed, 86 insertions, 42 deletions
diff --git a/app/services/snippets/base_service.rb b/app/services/snippets/base_service.rb index 2b450db0b83..81d12997335 100644 --- a/app/services/snippets/base_service.rb +++ b/app/services/snippets/base_service.rb @@ -2,8 +2,32 @@ module Snippets class BaseService < ::BaseService + include SpamCheckMethods + + CreateRepositoryError = Class.new(StandardError) + + attr_reader :uploaded_files + + def initialize(project, user = nil, params = {}) + super + + @uploaded_files = Array(@params.delete(:files).presence) + + filter_spam_check_params + end + private + def visibility_allowed?(snippet, visibility_level) + Gitlab::VisibilityLevel.allowed_for?(current_user, visibility_level) + end + + def error_forbidden_visibility(snippet) + deny_visibility_level(snippet) + + snippet_error_response(snippet, 403) + end + def snippet_error_response(snippet, http_status) ServiceResponse.error( message: snippet.errors.full_messages.to_sentence, @@ -11,5 +35,22 @@ module Snippets payload: { snippet: snippet } ) end + + def add_snippet_repository_error(snippet:, error:) + message = repository_error_message(error) + + snippet.errors.add(:repository, message) + end + + def repository_error_message(error) + message = self.is_a?(Snippets::CreateService) ? _("Error creating the snippet") : _("Error updating the snippet") + + # We only want to include additional error detail in the message + # if the error is not a CommitError because we cannot guarantee the message + # will be user-friendly + message += " - #{error.message}" unless error.instance_of?(SnippetRepository::CommitError) + + message + end end end diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index 155013db344..ed6da3a0ad0 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -2,23 +2,11 @@ module Snippets class CreateService < Snippets::BaseService - include SpamCheckMethods - - CreateRepositoryError = Class.new(StandardError) - def execute - filter_spam_check_params - - @snippet = if project - project.snippets.build(params) - else - PersonalSnippet.new(params) - end - - unless Gitlab::VisibilityLevel.allowed_for?(current_user, @snippet.visibility_level) - deny_visibility_level(@snippet) + @snippet = build_from_params - return snippet_error_response(@snippet, 403) + unless visibility_allowed?(@snippet, @snippet.visibility_level) + return error_forbidden_visibility(@snippet) end @snippet.author = current_user @@ -29,6 +17,8 @@ module Snippets UserAgentDetailService.new(@snippet, @request).create Gitlab::UsageDataCounters::SnippetCounter.count(:create) + move_temporary_files + ServiceResponse.success(payload: { snippet: @snippet } ) else snippet_error_response(@snippet, 400) @@ -37,10 +27,18 @@ module Snippets private + def build_from_params + if project + project.snippets.build(params) + else + PersonalSnippet.new(params) + end + end + def save_and_commit snippet_saved = @snippet.save - if snippet_saved && Feature.enabled?(:version_snippets, current_user) + if snippet_saved create_repository create_commit end @@ -60,7 +58,7 @@ module Snippets @snippet = @snippet.dup end - @snippet.errors.add(:base, e.message) + add_snippet_repository_error(snippet: @snippet, error: e) false end @@ -83,5 +81,13 @@ module Snippets def snippet_files [{ file_path: params[:file_name], content: params[:content] }] end + + def move_temporary_files + return unless @snippet.is_a?(PersonalSnippet) + + uploaded_files.each do |file| + FileMover.new(file, from_model: current_user, to_model: @snippet).execute + end + end end end diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index e56b20c6057..2dc9266dbd0 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -2,24 +2,15 @@ module Snippets class UpdateService < Snippets::BaseService - include SpamCheckMethods + COMMITTABLE_ATTRIBUTES = %w(file_name content).freeze UpdateError = Class.new(StandardError) - CreateRepositoryError = Class.new(StandardError) def execute(snippet) - # check that user is allowed to set specified visibility_level - new_visibility = visibility_level - - if new_visibility && new_visibility.to_i != snippet.visibility_level - unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) - deny_visibility_level(snippet, new_visibility) - - return snippet_error_response(snippet, 403) - end + if visibility_changed?(snippet) && !visibility_allowed?(snippet, visibility_level) + return error_forbidden_visibility(snippet) end - filter_spam_check_params snippet.assign_attributes(params) spam_check(snippet, current_user) @@ -34,30 +25,32 @@ module Snippets private + def visibility_changed?(snippet) + visibility_level && visibility_level.to_i != snippet.visibility_level + end + def save_and_commit(snippet) return false unless snippet.save - # In order to avoid non migrated snippets scenarios, - # if the snippet does not have a repository we created it - # We don't need to check if the repository exists - # because `create_repository` already handles it - if Feature.enabled?(:version_snippets, current_user) - create_repository_for(snippet) - end + # If the updated attributes does not need to update + # the repository we can just return + return true unless committable_attributes? - # If the snippet repository exists we commit always - # the changes - create_commit(snippet) if snippet.repository_exists? + create_repository_for(snippet) + create_commit(snippet) true rescue => e - # Restore old attributes + # Restore old attributes but re-assign changes so they're not lost unless snippet.previous_changes.empty? snippet.previous_changes.each { |attr, value| snippet[attr] = value[0] } snippet.save + + snippet.assign_attributes(params) end - snippet.errors.add(:repository, 'Error updating the snippet') + add_snippet_repository_error(snippet: snippet, error: e) + log_error(e.message) # If the commit action failed we remove it because @@ -92,7 +85,7 @@ module Snippets end def snippet_files(snippet) - [{ previous_path: snippet.blobs.first&.path, + [{ previous_path: snippet.file_name_on_repo, file_path: params[:file_name], content: params[:content] }] end @@ -104,5 +97,9 @@ module Snippets def repository_empty?(snippet) snippet.repository._uncached_exists? && !snippet.repository._uncached_has_visible_content? end + + def committable_attributes? + (params.stringify_keys.keys & COMMITTABLE_ATTRIBUTES).present? + end end end |