Merge remote-tracking branch 'origin/master' into 40781-os-to-ce

14 files changed, 123 insertions, 69 deletions

diff --git a/app/services/ci/fetch_kubernetes_token_service.rb b/app/services/ci/fetch_kubernetes_token_service.rb
index e73c6ad6780..bca883ec0a0 100644
--- a/app/services/ci/fetch_kubernetes_token_service.rb
+++ b/app/services/ci/fetch_kubernetes_token_service.rb
@@ -32,7 +32,7 @@ module Ci
       kubeclient = build_kubeclient!
 
       kubeclient.get_secrets.as_json
-    rescue KubeException => err
+    rescue Kubeclient::HttpError => err
       raise err unless err.error_code == 404
 
       []
diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb
index bde090eaeec..90393e951a4 100644
--- a/app/services/clusters/applications/check_installation_progress_service.rb
+++ b/app/services/clusters/applications/check_installation_progress_service.rb
@@ -12,7 +12,7 @@ module Clusters
         else
           check_timeout
         end
-      rescue KubeException => ke
+      rescue Kubeclient::HttpError => ke
         app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored?
       end
 
diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb
index 8ceeec687cd..4c25a09814b 100644
--- a/app/services/clusters/applications/install_service.rb
+++ b/app/services/clusters/applications/install_service.rb
@@ -10,7 +10,7 @@ module Clusters
 
           ClusterWaitForAppInstallationWorker.perform_in(
             ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id)
-        rescue KubeException => ke
+        rescue Kubeclient::HttpError => ke
           app.make_errored!("Kubernetes error: #{ke.message}")
         rescue StandardError
           app.make_errored!("Can't start installation process")
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index 1db91c3c90c..2a69a205629 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -10,9 +10,14 @@ class CompareService
     @start_ref_name = new_start_ref_name
   end
 
-  def execute(target_project, target_ref, straight: false)
+  def execute(target_project, target_ref, base_sha: nil, straight: false)
     raw_compare = target_project.repository.compare_source_branch(target_ref, start_project.repository, start_ref_name, straight: straight)
 
-    Compare.new(raw_compare, target_project, straight: straight) if raw_compare
+    return unless raw_compare
+
+    Compare.new(raw_compare,
+                target_project,
+                base_sha: base_sha,
+                straight: straight)
   end
 end
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index 46acdc5406c..a954564946b 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -1,11 +1,11 @@
 module Files
   class CreateService < Files::BaseService
     def create_commit!
-      handler = Lfs::FileModificationHandler.new(project, @branch_name)
+      transformer = Lfs::FileTransformer.new(project, @branch_name)
 
-      handler.new_file(@file_path, @file_content) do |content_or_lfs_pointer|
-        create_transformed_commit(content_or_lfs_pointer)
-      end
+      result = transformer.new_file(@file_path, @file_content)
+
+      create_transformed_commit(result.content)
     end
 
     private
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index a03c59f569d..13a1dee4173 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -3,11 +3,33 @@ module Files
     UPDATE_FILE_ACTIONS = %w(update move delete).freeze
 
     def create_commit!
+      transformer = Lfs::FileTransformer.new(project, @branch_name)
+
+      actions = actions_after_lfs_transformation(transformer, params[:actions])
+
+      commit_actions!(actions)
+    end
+
+    private
+
+    def actions_after_lfs_transformation(transformer, actions)
+      actions.map do |action|
+        if action[:action] == 'create'
+          result = transformer.new_file(action[:file_path], action[:content], encoding: action[:encoding])
+          action[:content] = result.content
+          action[:encoding] = result.encoding
+        end
+
+        action
+      end
+    end
+
+    def commit_actions!(actions)
       repository.multi_action(
         current_user,
         message: @commit_message,
         branch_name: @branch_name,
-        actions: params[:actions],
+        actions: actions,
         author_email: @author_email,
         author_name: @author_name,
         start_project: @start_project,
@@ -17,8 +39,6 @@ module Files
       raise_error(e)
     end
 
-    private
-
     def validate!
       super
 
diff --git a/app/services/lfs/file_modification_handler.rb b/app/services/lfs/file_modification_handler.rb
deleted file mode 100644
index fe9091a6e5d..00000000000
--- a/app/services/lfs/file_modification_handler.rb
+++ /dev/null
@@ -1,42 +0,0 @@
-module Lfs
-  class FileModificationHandler
-    attr_reader :project, :branch_name
-
-    delegate :repository, to: :project
-
-    def initialize(project, branch_name)
-      @project = project
-      @branch_name = branch_name
-    end
-
-    def new_file(file_path, file_content)
-      if project.lfs_enabled? && lfs_file?(file_path)
-        lfs_pointer_file = Gitlab::Git::LfsPointerFile.new(file_content)
-        lfs_object = create_lfs_object!(lfs_pointer_file, file_content)
-        content = lfs_pointer_file.pointer
-
-        success = yield(content)
-
-        link_lfs_object!(lfs_object) if success
-      else
-        yield(file_content)
-      end
-    end
-
-    private
-
-    def lfs_file?(file_path)
-      repository.attributes_at(branch_name, file_path)['filter'] == 'lfs'
-    end
-
-    def create_lfs_object!(lfs_pointer_file, file_content)
-      LfsObject.find_or_create_by(oid: lfs_pointer_file.sha256, size: lfs_pointer_file.size) do |lfs_object|
-        lfs_object.file = CarrierWaveStringFile.new(file_content)
-      end
-    end
-
-    def link_lfs_object!(lfs_object)
-      project.lfs_objects << lfs_object
-    end
-  end
-end
diff --git a/app/services/lfs/file_transformer.rb b/app/services/lfs/file_transformer.rb
new file mode 100644
index 00000000000..69281ee3137
--- /dev/null
+++ b/app/services/lfs/file_transformer.rb
@@ -0,0 +1,66 @@
+module Lfs
+  # Usage: Calling `new_file` check to see if a file should be in LFS and
+  #        return a transformed result with `content` and `encoding` to commit.
+  #
+  #        For LFS an LfsObject linked to the project is stored and an LFS
+  #        pointer returned. If the file isn't in LFS the untransformed content
+  #        is returned to save in the commit.
+  #
+  # transformer = Lfs::FileTransformer.new(project, @branch_name)
+  # content_or_lfs_pointer = transformer.new_file(file_path, content).content
+  # create_transformed_commit(content_or_lfs_pointer)
+  #
+  class FileTransformer
+    attr_reader :project, :branch_name
+
+    delegate :repository, to: :project
+
+    def initialize(project, branch_name)
+      @project = project
+      @branch_name = branch_name
+    end
+
+    def new_file(file_path, file_content, encoding: nil)
+      if project.lfs_enabled? && lfs_file?(file_path)
+        file_content = Base64.decode64(file_content) if encoding == 'base64'
+        lfs_pointer_file = Gitlab::Git::LfsPointerFile.new(file_content)
+        lfs_object = create_lfs_object!(lfs_pointer_file, file_content)
+
+        link_lfs_object!(lfs_object)
+
+        Result.new(content: lfs_pointer_file.pointer, encoding: 'text')
+      else
+        Result.new(content: file_content, encoding: encoding)
+      end
+    end
+
+    class Result
+      attr_reader :content, :encoding
+
+      def initialize(content:, encoding:)
+        @content = content
+        @encoding = encoding
+      end
+    end
+
+    private
+
+    def lfs_file?(file_path)
+      cached_attributes.attributes(file_path)['filter'] == 'lfs'
+    end
+
+    def cached_attributes
+      @cached_attributes ||= Gitlab::Git::AttributesAtRefParser.new(repository, branch_name)
+    end
+
+    def create_lfs_object!(lfs_pointer_file, file_content)
+      LfsObject.find_or_create_by(oid: lfs_pointer_file.sha256, size: lfs_pointer_file.size) do |lfs_object|
+        lfs_object.file = CarrierWaveStringFile.new(file_content)
+      end
+    end
+
+    def link_lfs_object!(lfs_object)
+      project.lfs_objects << lfs_object
+    end
+  end
+end
diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb
index ca9a33678e4..72cbc49adb2 100644
--- a/app/services/merge_requests/conflicts/list_service.rb
+++ b/app/services/merge_requests/conflicts/list_service.rb
@@ -17,15 +17,7 @@ module MergeRequests
         return @conflicts_can_be_resolved_in_ui = false unless merge_request.has_complete_diff_refs?
         return @conflicts_can_be_resolved_in_ui = false if merge_request.branch_missing?
 
-        begin
-          # Try to parse each conflict. If the MR's mergeable status hasn't been
-          # updated, ensure that we don't say there are conflicts to resolve
-          # when there are no conflict files.
-          conflicts.files.each(&:lines)
-          @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0
-        rescue Gitlab::Git::CommandError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing
-          @conflicts_can_be_resolved_in_ui = false
-        end
+        @conflicts_can_be_resolved_in_ui = conflicts.can_be_resolved_in_ui?
       end
 
       def conflicts
diff --git a/app/services/merge_requests/merge_request_diff_cache_service.rb b/app/services/merge_requests/merge_request_diff_cache_service.rb
index 2945a7fd4e4..10aa9ae609c 100644
--- a/app/services/merge_requests/merge_request_diff_cache_service.rb
+++ b/app/services/merge_requests/merge_request_diff_cache_service.rb
@@ -1,8 +1,17 @@
 module MergeRequests
   class MergeRequestDiffCacheService
-    def execute(merge_request)
+    def execute(merge_request, new_diff)
       # Executing the iteration we cache all the highlighted diff information
       merge_request.diffs.diff_files.to_a
+
+      # Remove cache for all diffs on this MR. Do not use the association on the
+      # model, as that will interfere with other actions happening when
+      # reloading the diff.
+      MergeRequestDiff.where(merge_request: merge_request).each do |merge_request_diff|
+        next if merge_request_diff == new_diff
+
+        merge_request_diff.diffs.clear_cache!
+      end
     end
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index e07ecda27b5..d7d2cde1004 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -208,9 +208,9 @@ class NotificationService
   def new_access_request(member)
     return true unless member.notifiable?(:subscription)
 
-    recipients = member.source.members.owners_and_masters
+    recipients = member.source.members.active_without_invites_and_requests.owners_and_masters
     if fallback_to_group_owners_masters?(recipients, member)
-      recipients = member.source.group.members.owners_and_masters
+      recipients = member.source.group.members.active_without_invites_and_requests.owners_and_masters
     end
 
     recipients.each { |recipient| deliver_access_request_email(recipient, member) }
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 81972df9b3c..4b8f955ae69 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -88,7 +88,11 @@ module Projects
     def attempt_rollback(project, message)
       return unless project
 
-      project.update_attributes(delete_error: message, pending_delete: false)
+      # It's possible that the project was destroyed, but some after_commit
+      # hook failed and caused us to end up here. A destroyed model will be a frozen hash,
+      # which cannot be altered.
+      project.update_attributes(delete_error: message, pending_delete: false) unless project.destroyed?
+
       log_error("Deletion failed on #{project.full_path} with the following message: #{message}")
     end
 
diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb
index af41ce82f65..d16aa3de639 100644
--- a/app/services/projects/import_export/export_service.rb
+++ b/app/services/projects/import_export/export_service.rb
@@ -26,7 +26,7 @@ module Projects
       end
 
       def project_tree_saver
-        Gitlab::ImportExport::ProjectTreeSaver.new(project: project, current_user: @current_user, shared: @shared)
+        Gitlab::ImportExport::ProjectTreeSaver.new(project: project, current_user: @current_user, shared: @shared, params: @params)
       end
 
       def uploads_saver
diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index 379a8068023..5f2615a2c01 100644
--- a/app/services/projects/update_service.rb
+++ b/app/services/projects/update_service.rb
@@ -58,7 +58,7 @@ module Projects
     def enabling_wiki?
       return false if @project.wiki_enabled?
 
-      params[:project_feature_attributes][:wiki_access_level].to_i > ProjectFeature::DISABLED
+      params.dig(:project_feature_attributes, :wiki_access_level).to_i > ProjectFeature::DISABLED
     end
 
     def ensure_wiki_exists