diff options
author | Rubén Dávila <ruben@gitlab.com> | 2017-12-20 18:57:27 +0300 |
---|---|---|
committer | Rubén Dávila <ruben@gitlab.com> | 2017-12-20 18:57:27 +0300 |
commit | c927e57466b6c705891f09c95f566259d8e1ec0e (patch) | |
tree | c464abfd7c7f7138ea5ae0dabebd7e9f3a0680b2 /app/services/files | |
parent | c210ddab9289e29fadc9a5a0462ffbe864af736c (diff) |
Updates from last code review:
- Apply some refactoring for code reuse
- Add file status validation for Files::DeleteService
- Write additional specs
Diffstat (limited to 'app/services/files')
-rw-r--r-- | app/services/files/base_service.rb | 14 | ||||
-rw-r--r-- | app/services/files/delete_service.rb | 10 | ||||
-rw-r--r-- | app/services/files/multi_service.rb | 15 | ||||
-rw-r--r-- | app/services/files/update_service.rb | 21 |
4 files changed, 33 insertions, 27 deletions
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 38231f66009..8d4b9f14780 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -1,11 +1,14 @@ module Files class BaseService < Commits::CreateService + FileChangedError = Class.new(StandardError) + def initialize(*args) super @author_email = params[:author_email] @author_name = params[:author_name] @commit_message = params[:commit_message] + @last_commit_sha = params[:last_commit_sha] @file_path = params[:file_path] @previous_path = params[:previous_path] @@ -13,5 +16,16 @@ module Files @file_content = params[:file_content] @file_content = Base64.decode64(@file_content) if params[:file_content_encoding] == 'base64' end + + def file_has_changed?(path, commit_id) + return false unless commit_id + + last_commit = Gitlab::Git::Commit + .last_for_path(@start_project.repository, @start_branch, path) + + return false unless last_commit + + last_commit.sha != commit_id + end end end diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 7952e5c95d4..32a57484d4e 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -11,5 +11,15 @@ module Files start_project: @start_project, start_branch_name: @start_branch) end + + private + + def validate! + super + + if file_has_changed?(@file_path, @last_commit_sha) + raise FileChangedError, "You are attempting to delete a file that has been previously updated." + end + end end end diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index eb2773733b1..98a3e83c130 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -1,5 +1,7 @@ module Files class MultiService < Files::BaseService + UPDATE_FILE_ACTIONS = %w(update move delete).freeze + def create_commit! repository.multi_action( user: current_user, @@ -20,7 +22,7 @@ module Files params[:actions].each do |action| validate_action!(action) - validate_file_status(action) + validate_file_status!(action) end end @@ -30,14 +32,13 @@ module Files end end - def validate_file_status(action) - return unless action[:last_commit_id] + def validate_file_status!(action) + return unless UPDATE_FILE_ACTIONS.include?(action[:action]) - current_commit = Gitlab::Git::Commit.last_for_path( - @start_project.repository, @start_branch, action[:file_path]) + file_path = action[:previous_path] || action[:file_path] - if current_commit.sha != action[:last_commit_id] - raise_error("The file has changed since you started editing it: #{action[:file_path]}") + if file_has_changed?(file_path, action[:last_commit_id]) + raise_error("The file has changed since you started editing it: #{file_path}") end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index bcca1386bed..1902d1cea72 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -1,13 +1,5 @@ module Files class UpdateService < Files::BaseService - FileChangedError = Class.new(StandardError) - - 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, @@ -21,21 +13,10 @@ module Files private - def file_has_changed? - return false unless @last_commit_sha && last_commit - - @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? + if file_has_changed?(@file_path, @last_commit_sha) raise FileChangedError, "You are attempting to update a file that has changed since you started editing it." end end |