Add latest changes from gitlab-org/gitlab@master

6 files changed, 105 insertions, 34 deletions

diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index c08889b87a2..02f625f2130 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -6,6 +6,7 @@ module API
       include PaginationParams
 
       before do
+        check_api_enabled!
         authenticate!
         authorize! :read_secure_files, user_project
       end
@@ -64,7 +65,7 @@ module API
 
         resource do
           before do
-            read_only_feature_flag_enabled?
+            check_read_only_feature_flag_enabled!
             authorize! :admin_secure_files, user_project
           end
 
@@ -112,7 +113,11 @@ module API
       end
 
       helpers do
-        def read_only_feature_flag_enabled?
+        def check_api_enabled!
+          forbidden! unless Gitlab.config.ci_secure_files.enabled
+        end
+
+        def check_read_only_feature_flag_enabled!
           service_unavailable! if Feature.enabled?(:ci_secure_files_read_only, user_project, type: :ops)
         end
       end
diff --git a/lib/gitlab/diff/formatters/file_formatter.rb b/lib/gitlab/diff/formatters/file_formatter.rb
new file mode 100644
index 00000000000..37b9ad85ef8
--- /dev/null
+++ b/lib/gitlab/diff/formatters/file_formatter.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Diff
+    module Formatters
+      class FileFormatter < BaseFormatter
+        def initialize(attrs)
+          @ignore_whitespace_change = false
+
+          super(attrs)
+        end
+
+        def key
+          @key ||= super.push(new_path, old_path)
+        end
+
+        def position_type
+          "file"
+        end
+
+        def complete?
+          [new_path, old_path].all?(&:present?)
+        end
+
+        def ==(other)
+          other.is_a?(self.class) &&
+            old_path == other.old_path &&
+            new_path == other.new_path
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
index cbb7231261b..feee4bcc7f9 100644
--- a/lib/gitlab/diff/position.rb
+++ b/lib/gitlab/diff/position.rb
@@ -150,6 +150,10 @@ module Gitlab
         @file_hash ||= Digest::SHA1.hexdigest(file_path)
       end
 
+      def on_file?
+        position_type == 'file'
+      end
+
       def on_image?
         position_type == 'image'
       end
@@ -185,6 +189,8 @@ module Gitlab
         case type
         when 'image'
           Gitlab::Diff::Formatters::ImageFormatter
+        when 'file'
+          Gitlab::Diff::Formatters::FileFormatter
         else
           Gitlab::Diff::Formatters::TextFormatter
         end
diff --git a/lib/gitlab/diff/position_tracer.rb b/lib/gitlab/diff/position_tracer.rb
index faa8a46e6ed..a8c0108fa34 100644
--- a/lib/gitlab/diff/position_tracer.rb
+++ b/lib/gitlab/diff/position_tracer.rb
@@ -22,9 +22,8 @@ module Gitlab
         return unless old_position.diff_refs == old_diff_refs
 
         @ignore_whitespace_change = old_position.ignore_whitespace_change
-        strategy = old_position.on_text? ? LineStrategy : ImageStrategy
 
-        strategy.new(self).trace(old_position)
+        strategy(old_position).new(self).trace(old_position)
       end
 
       def ac_diffs
@@ -49,6 +48,16 @@ module Gitlab
 
       private
 
+      def strategy(old_position)
+        if old_position.on_text?
+          LineStrategy
+        elsif old_position.on_file?
+          FileStrategy
+        else
+          ImageStrategy
+        end
+      end
+
       def compare(start_sha, head_sha, straight: false)
         compare = CompareService.new(project, head_sha).execute(project, start_sha, straight: straight)
         compare.diffs(paths: paths, expanded: true, ignore_whitespace_change: @ignore_whitespace_change)
diff --git a/lib/gitlab/diff/position_tracer/file_strategy.rb b/lib/gitlab/diff/position_tracer/file_strategy.rb
new file mode 100644
index 00000000000..171d78bf46f
--- /dev/null
+++ b/lib/gitlab/diff/position_tracer/file_strategy.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Diff
+    class PositionTracer
+      class FileStrategy < BaseStrategy
+        def trace(position)
+          a_path = position.old_path
+          b_path = position.new_path
+
+          # If file exists in B->D (e.g. updated, renamed, removed), let the
+          # note become outdated.
+          bd_diff = bd_diffs.diff_file_with_old_path(b_path)
+
+          return { position: new_position(position, bd_diff), outdated: true } if bd_diff
+
+          # If file still exists in the new diff, update the position.
+          cd_diff = cd_diffs.diff_file_with_new_path(b_path)
+
+          return { position: new_position(position, cd_diff), outdated: false } if cd_diff
+
+          # If file exists in A->C (e.g. rebased and same changes were present
+          # in target branch), let the note become outdated.
+          ac_diff = ac_diffs.diff_file_with_old_path(a_path)
+
+          return { position: new_position(position, ac_diff), outdated: true } if ac_diff
+
+          # If ever there's a case that the file no longer exists in any diff,
+          # don't set a change position and let the note become outdated.
+          #
+          # This should never happen given the file should exist in one of the
+          # diffs above.
+          { outdated: true }
+        end
+
+        private
+
+        def new_position(position, diff_file)
+          Position.new(
+            diff_file: diff_file,
+            position_type: position.position_type
+          )
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/diff/position_tracer/image_strategy.rb b/lib/gitlab/diff/position_tracer/image_strategy.rb
index aac52b536f7..172eba4acd3 100644
--- a/lib/gitlab/diff/position_tracer/image_strategy.rb
+++ b/lib/gitlab/diff/position_tracer/image_strategy.rb
@@ -3,36 +3,7 @@
 module Gitlab
   module Diff
     class PositionTracer
-      class ImageStrategy < BaseStrategy
-        def trace(position)
-          a_path = position.old_path
-          b_path = position.new_path
-
-          # If file exists in B->D (e.g. updated, renamed, removed), let the
-          # note become outdated.
-          bd_diff = bd_diffs.diff_file_with_old_path(b_path)
-
-          return { position: new_position(position, bd_diff), outdated: true } if bd_diff
-
-          # If file still exists in the new diff, update the position.
-          cd_diff = cd_diffs.diff_file_with_new_path(b_path)
-
-          return { position: new_position(position, cd_diff), outdated: false } if cd_diff
-
-          # If file exists in A->C (e.g. rebased and same changes were present
-          # in target branch), let the note become outdated.
-          ac_diff = ac_diffs.diff_file_with_old_path(a_path)
-
-          return { position: new_position(position, ac_diff), outdated: true } if ac_diff
-
-          # If ever there's a case that the file no longer exists in any diff,
-          # don't set a change position and let the note become outdated.
-          #
-          # This should never happen given the file should exist in one of the
-          # diffs above.
-          { outdated: true }
-        end
-
+      class ImageStrategy < FileStrategy
         private
 
         def new_position(position, diff_file)