Update Rails patches

Removes patches that are already in Rails 6:
Gitlab::ContentDisposition patch
ActiveRecord::QueryCache patch
ActiveJob::Arguments patch
Gitlab::ActionViewOutput patch

Updates Activerecord::Locking::Optimistic patch to follow changes in
https://github.com/rails/rails/blob/v6.0.1/activerecord/lib/active_record/locking/optimistic.rb

Fix missing timezone for datetime_with_timezone by adding this custom
type to ActiveRecord's time_zone_aware_attributes

.rubocop.yml

@@ -235,12 +235,6 @@ RSpec/FactoriesInMigrationSpecs:
     - 'spec/lib/ee/gitlab/background_migration/**/*.rb'
     - 'ee/spec/lib/ee/gitlab/background_migration/**/*.rb'
 
-Cop/IncludeActionViewContext:
-  Enabled: true
-  Exclude:
-    - 'spec/**/*'
-    - 'ee/spec/**/*'
-
 Cop/IncludeSidekiqWorker:
   Enabled: true
   Exclude:

app/controllers/concerns/send_file_upload.rb

@@ -3,7 +3,7 @@
 module SendFileUpload
   def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment')
     if attachment
-      response_disposition = ::Gitlab::ContentDisposition.format(disposition: disposition, filename: attachment)
+      response_disposition = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: attachment)
 
       # Response-Content-Type will not override an existing Content-Type in
       # Google Cloud Storage, so the metadata needs to be cleared on GCS for
@@ -15,7 +15,7 @@ module SendFileUpload
       # cross-origin JavaScript protection.
       send_params[:content_type] = 'text/plain' if File.extname(attachment) == '.js'
 
-      send_params.merge!(filename: attachment, disposition: utf8_encoded_disposition(disposition, attachment))
+      send_params.merge!(filename: attachment, disposition: disposition)
     end
 
     if file_upload.file_storage?
@@ -28,18 +28,6 @@ module SendFileUpload
     end
   end
 
-  # Since Rails 5 doesn't properly support support non-ASCII filenames,
-  # we have to add our own to ensure RFC 5987 compliance. However, Rails
-  # 5 automatically appends `filename#{filename}` here:
-  # https://github.com/rails/rails/blob/v5.0.7/actionpack/lib/action_controller/metal/data_streaming.rb#L137
-  # Rails 6 will have https://github.com/rails/rails/pull/33829, so we
-  # can get rid of this special case handling when we upgrade.
-  def utf8_encoded_disposition(disposition, filename)
-    content = ::Gitlab::ContentDisposition.new(disposition: disposition, filename: filename)
-
-    "#{disposition}; #{content.utf8_filename}"
-  end
-
   def guess_content_type(filename)
     types = MIME::Types.type_for(filename)
 

app/helpers/markup_helper.rb

@@ -4,7 +4,7 @@ require 'nokogiri'
 
 module MarkupHelper
   include ActionView::Helpers::TextHelper
-  include ::Gitlab::ActionViewOutput::Context
+  include ActionView::Context
 
   def plain?(filename)
     Gitlab::MarkupHelper.plain?(filename)

app/serializers/concerns/user_status_tooltip.rb

@@ -3,7 +3,7 @@
 module UserStatusTooltip
   extend ActiveSupport::Concern
   include ActionView::Helpers::TagHelper
-  include ::Gitlab::ActionViewOutput::Context
+  include ActionView::Context
   include EmojiHelper
   include UsersHelper
 

app/workers/mail_scheduler/notification_service_worker.rb

@@ -26,49 +26,26 @@ module MailScheduler
     end
 
     def self.perform_async(*args)
-      super(*Arguments.serialize(args))
+      super(*ActiveJob::Arguments.serialize(args))
     end
 
     private
 
-    # If an argument is in the ActiveJob::Arguments::TYPE_WHITELIST list,
+    # This is copied over from https://github.com/rails/rails/blob/v6.0.1/activejob/lib/active_job/arguments.rb#L50
+    # because it is declared as a private constant
+    PERMITTED_TYPES = [NilClass, String, Integer, Float, BigDecimal, TrueClass, FalseClass].freeze
+
+    private_constant :PERMITTED_TYPES
+
+    # If an argument is in the PERMITTED_TYPES list,
     # it means the argument cannot be deserialized.
     # Which means there's something wrong with our code.
     def check_arguments!(args)
       args.each do |arg|
-        if arg.class.in?(ActiveJob::Arguments::TYPE_WHITELIST)
+        if arg.class.in?(PERMITTED_TYPES)
           raise(ArgumentError, "Argument `#{arg}` cannot be deserialized because of its type")
         end
       end
     end
-
-    # Permit ActionController::Parameters for serializable Hash
-    #
-    # Port of
-    # https://github.com/rails/rails/commit/945fdd76925c9f615bf016717c4c8db2b2955357#diff-fc90ec41ef75be8b2259526fe1a8b663
-    module Arguments
-      include ActiveJob::Arguments
-      extend self
-
-      private
-
-      def serialize_argument(argument)
-        case argument
-        when -> (arg) { arg.respond_to?(:permitted?) }
-          serialize_hash(argument.to_h).tap do |result|
-            result[WITH_INDIFFERENT_ACCESS_KEY] = serialize_argument(true)
-          end
-        else
-          super
-        end
-      end
-    end
-
-    # Make sure we remove this patch starting with Rails 6.0.
-    if Rails.version.start_with?('6.0')
-      raise <<~MSG
-        Please remove the patch `Arguments` module and use `ActiveJob::Arguments` again.
-      MSG
-    end
   end
 end

config/initializers/active_record_data_types.rb

@@ -24,10 +24,11 @@ module RegisterDateTimeWithTimeZone
   def initialize_type_map(mapping = type_map)
     super mapping
 
-    mapping.register_type 'timestamptz' do |_, _, sql_type|
-      precision = extract_precision(sql_type)
-      ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::OID::DateTimeWithTimeZone.new(precision: precision)
-    end
+    register_class_with_precision(
+      mapping,
+      'timestamptz',
+      ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::OID::DateTimeWithTimeZone
+    )
   end
 end
 
@@ -46,3 +47,5 @@ end
 if (ActiveRecord::Base.connection.active? rescue false)
   ActiveRecord::Base.connection.send :reload_type_map
 end
+
+ActiveRecord::Base.time_zone_aware_types += [:datetime_with_timezone]

config/initializers/active_record_query_cache.rb

-# frozen_string_literal: true
-
-ActiveRecord::ConnectionAdapters::ConnectionPool.prepend Gitlab::Patch::ActiveRecordQueryCache

config/initializers/config_initializers_active_record_locking.rb

@@ -26,7 +26,7 @@ module ActiveRecord
             locking_column => possible_previous_lock_value,
             self.class.primary_key => id_in_database
           ).update_all(
-            attributes_with_values_for_update(attribute_names)
+            attributes_with_values(attribute_names)
           )
 
           if affected_rows != 1

ee/spec/controllers/projects/designs_controller_spec.rb

@@ -37,7 +37,7 @@ describe Projects::DesignsController do
 
         subject
 
-        expect(response.header['Content-Disposition']).to eq(%Q(attachment; filename*=UTF-8''#{filename}; filename=\"#{filename}\"))
+        expect(response.header['Content-Disposition']).to eq(%Q(attachment; filename=\"#{filename}\"; filename*=UTF-8''#{filename}))
       end
 
       context 'when the design is not an LFS file' do

lib/api/helpers.rb

@@ -434,7 +434,7 @@ module API
 
     def present_disk_file!(path, filename, content_type = 'application/octet-stream')
       filename ||= File.basename(path)
-      header['Content-Disposition'] = ::Gitlab::ContentDisposition.format(disposition: 'attachment', filename: filename)
+      header['Content-Disposition'] = ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: filename)
       header['Content-Transfer-Encoding'] = 'binary'
       content_type content_type
 
@@ -542,7 +542,7 @@ module API
     def send_git_blob(repository, blob)
       env['api.format'] = :txt
       content_type 'text/plain'
-      header['Content-Disposition'] = ::Gitlab::ContentDisposition.format(disposition: 'inline', filename: blob.name)
+      header['Content-Disposition'] = ActionDispatch::Http::ContentDisposition.format(disposition: 'inline', filename: blob.name)
 
       # Let Workhorse examine the content and determine the better content disposition
       header[Gitlab::Workhorse::DETECT_HEADER] = "true"

lib/gitlab/action_view_output/context.rb

-# frozen_string_literal: true
-
-# This file was simplified from https://raw.githubusercontent.com/rails/rails/195f39804a7a4a0034f25e8704220e03d95a752a/actionview/lib/action_view/context.rb.
-#
-# It is only needed by modules that need to call ActionView helper
-# methods (e.g. those in
-# https://github.com/rails/rails/tree/c4d3e202e10ae627b3b9c34498afb45450652421/actionview/lib/action_view/helpers)
-# to generate tags outside of a Rails controller (e.g. API, Sidekiq,
-# etc.).
-#
-# In Rails 5, ActionView::Context automatically includes CompiledTemplates.
-# This means that any module that includes ActionView::Context is now a descendant
-# of CompiledTemplates.
-#
-# When a partial is rendered for the first time, it runs
-# Module#module_eval, which will evaluate a string source that defines a
-# new method. For example:
-#
-# def _app_views_profiles_show_html_haml___1285955918103175884_70307801785400(local_assigns, output_buffer)
-#   "hello world"
-# end
-#
-# When a new method is defined, the Ruby interpreter clears the method
-# cache for all descendants, and all methods for those modules will have
-# to be redefined.  This can lead to a significant performance penalty.
-#
-# Rails 6 fixes this behavior by moving out the `include
-# CompiledTemplates` into ActionView::Base so that including `ActionView::Context`
-# doesn't quietly affect other modules in this way.
-
-if Rails::VERSION::STRING.start_with?('6')
-  raise 'This module is no longer needed in Rails 6. Use ActionView::Context instead.'
-end
-
-module Gitlab
-  module ActionViewOutput
-    module Context
-      attr_accessor :output_buffer, :view_flow
-    end
-  end
-end

lib/gitlab/content_disposition.rb

-# frozen_string_literal: true
-# This ports ActionDispatch::Http::ContentDisposition (https://github.com/rails/rails/pull/33829,
-# which will be available in Rails 6.
-module Gitlab
-  class ContentDisposition # :nodoc:
-    # Make sure we remove this patch starting with Rails 6.0.
-    if Rails.version.start_with?('6.0')
-      raise <<~MSG
-        Please remove this file and use `ActionDispatch::Http::ContentDisposition` instead.
-      MSG
-    end
-
-    def self.format(disposition:, filename:)
-      new(disposition: disposition, filename: filename).to_s
-    end
-
-    attr_reader :disposition, :filename
-
-    def initialize(disposition:, filename:)
-      @disposition = disposition
-      @filename = filename
-    end
-
-    # rubocop:disable Style/VariableInterpolation
-    TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/.freeze
-
-    def ascii_filename
-      'filename="' + percent_escape(::I18n.transliterate(filename), TRADITIONAL_ESCAPED_CHAR) + '"'
-    end
-
-    RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/.freeze
-    # rubocop:enable Style/VariableInterpolation
-
-    def utf8_filename
-      "filename*=UTF-8''" + percent_escape(filename, RFC_5987_ESCAPED_CHAR)
-    end
-
-    def to_s
-      if filename
-        "#{disposition}; #{ascii_filename}; #{utf8_filename}"
-      else
-        "#{disposition}"
-      end
-    end
-
-    private
-
-    def percent_escape(string, pattern)
-      string.gsub(pattern) do |char|
-        char.bytes.map { |byte| "%%%02X" % byte }.join
-      end
-    end
-  end
-end

lib/gitlab/patch/active_record_query_cache.rb

-# frozen_string_literal: true
-
-# Fixes a bug where the query cache isn't aware of the shared
-# ActiveRecord connection used in tests
-# https://github.com/rails/rails/issues/36587
-
-# To be removed with https://gitlab.com/gitlab-org/gitlab-foss/issues/64413
-
-module Gitlab
-  module Patch
-    module ActiveRecordQueryCache
-      # rubocop:disable Gitlab/ModuleWithInstanceVariables
-      def enable_query_cache!
-        @query_cache_enabled[connection_cache_key(current_thread)] = true
-        connection.enable_query_cache! if active_connection?
-      end
-
-      def disable_query_cache!
-        @query_cache_enabled.delete connection_cache_key(current_thread)
-        connection.disable_query_cache! if active_connection?
-      end
-
-      def query_cache_enabled
-        @query_cache_enabled[connection_cache_key(current_thread)]
-      end
-
-      def active_connection?
-        @thread_cached_conns[connection_cache_key(current_thread)]
-      end
-
-      private
-
-      def current_thread
-        @lock_thread || Thread.current
-      end
-      # rubocop:enable Gitlab/ModuleWithInstanceVariables
-    end
-  end
-end

rubocop/cop/include_action_view_context.rb

-# frozen_string_literal: true
-
-module RuboCop
-  module Cop
-    # Cop that makes sure workers include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`.
-    class IncludeActionViewContext < RuboCop::Cop::Cop
-      MSG = 'Include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`, for Rails 5.'.freeze
-
-      def_node_matcher :includes_action_view_context?, <<~PATTERN
-        (send nil? :include (const (const nil? :ActionView) :Context))
-      PATTERN
-
-      def on_send(node)
-        return unless includes_action_view_context?(node)
-
-        add_offense(node.arguments.first, location: :expression)
-      end
-
-      def autocorrect(node)
-        lambda do |corrector|
-          corrector.replace(node.source_range, '::Gitlab::ActionViewOutput::Context')
-        end
-      end
-    end
-  end
-end

rubocop/rubocop.rb

@@ -5,7 +5,6 @@ require_relative 'cop/gitlab/httparty'
 require_relative 'cop/gitlab/finder_with_find_by'
 require_relative 'cop/gitlab/union'
 require_relative 'cop/gitlab/rails_logger'
-require_relative 'cop/include_action_view_context'
 require_relative 'cop/include_sidekiq_worker'
 require_relative 'cop/safe_params'
 require_relative 'cop/active_record_association_reload'

spec/controllers/concerns/send_file_upload_spec.rb

@@ -59,11 +59,9 @@ describe SendFileUpload do
       let(:params) { { disposition: 'inline', attachment: filename } }
 
       it 'sends a file with inline disposition' do
-        # Notice the filename= is omitted from the disposition; this is because
-        # Rails 5 will append this header in send_file
         expected_params = {
           filename: 'test.png',
-          disposition: "inline; filename*=UTF-8''test.png"
+          disposition: 'inline'
         }
         expect(controller).to receive(:send_file).with(uploader.path, expected_params)
 
@@ -76,34 +74,16 @@ describe SendFileUpload do
       let(:params) { { attachment: filename } }
 
       it 'sends a file with content-type of text/plain' do
-        # Notice the filename= is omitted from the disposition; this is because
-        # Rails 5 will append this header in send_file
         expected_params = {
           content_type: 'text/plain',
           filename: 'test.js',
-          disposition: "attachment; filename*=UTF-8''test.js"
+          disposition: 'attachment'
         }
         expect(controller).to receive(:send_file).with(uploader.path, expected_params)
 
         subject
       end
 
-      context 'with non-ASCII encoded filename' do
-        let(:filename) { 'テスト.txt' }
-
-        # Notice the filename= is omitted from the disposition; this is because
-        # Rails 5 will append this header in send_file
-        it 'sends content-disposition for non-ASCII encoded filenames' do
-          expected_params = {
-            filename: filename,
-            disposition: "attachment; filename*=UTF-8''%E3%83%86%E3%82%B9%E3%83%88.txt"
-          }
-          expect(controller).to receive(:send_file).with(uploader.path, expected_params)
-
-          subject
-        end
-      end
-
       context 'with a proxied file in object storage' do
         before do
           stub_uploads_object_storage(uploader: uploader_class)

spec/controllers/projects/artifacts_controller_spec.rb

@@ -138,14 +138,14 @@ describe Projects::ArtifactsController do
       let(:filename) { job.artifacts_file.filename }
 
       it 'sends the artifacts file' do
-        # Notice the filename= is omitted from the disposition; this is because
-        # Rails 5 will append this header in send_file
         expect(controller).to receive(:send_file)
                           .with(
                             job.artifacts_file.file.path,
-                            hash_including(disposition: %Q(attachment; filename*=UTF-8''#{filename}))).and_call_original
+                            hash_including(disposition: 'attachment', filename: filename)).and_call_original
 
         download_artifact
+
+        expect(response.headers['Content-Disposition']).to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename}))
       end
     end
 
@@ -170,13 +170,13 @@ describe Projects::ArtifactsController do
           end
 
           it 'sends the codequality report' do
-            # Notice the filename= is omitted from the disposition; this is because
-            # Rails 5 will append this header in send_file
             expect(controller).to receive(:send_file)
                               .with(job.job_artifacts_codequality.file.path,
-                                    hash_including(disposition: %Q(attachment; filename*=UTF-8''#{filename}))).and_call_original
+                                    hash_including(disposition: 'attachment', filename: filename)).and_call_original
 
             download_artifact(file_type: file_type)
+
+            expect(response.headers['Content-Disposition']).to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename}))
           end
         end
 

spec/controllers/uploads_controller_spec.rb

@@ -649,7 +649,7 @@ describe UploadsController do
           get :show, params: { model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'dk.png' }
 
           expect(response).to have_gitlab_http_status(:ok)
-          expect(response.header['Content-Disposition']).to end_with 'filename="dk.png"'
+          expect(response.header['Content-Disposition']).to include('filename="dk.png"')
         end
       end
 

spec/features/projects/artifacts/user_downloads_artifacts_spec.rb

@@ -9,7 +9,7 @@ describe "User downloads artifacts" do
 
   shared_examples "downloading" do
     it "downloads the zip" do
-      expect(page.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename*=UTF-8''#{job.artifacts_file.filename}; filename="#{job.artifacts_file.filename}"})
+      expect(page.response_headers['Content-Disposition']).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}})
       expect(page.response_headers['Content-Transfer-Encoding']).to eq("binary")
       expect(page.response_headers['Content-Type']).to eq("application/zip")
       expect(page.source.b).to eq(job.artifacts_file.file.read.b)

spec/features/projects/jobs_spec.rb

@@ -346,7 +346,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
 
         artifact_request = requests.find { |req| req.url.match(%r{artifacts/download}) }
 
-        expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename*=UTF-8''#{job.artifacts_file.filename}; filename="#{job.artifacts_file.filename}"})
+        expect(artifact_request.response_headers['Content-Disposition']).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}})
         expect(artifact_request.response_headers['Content-Transfer-Encoding']).to eq("binary")
         expect(artifact_request.response_headers['Content-Type']).to eq("image/gif")
         expect(artifact_request.body).to eq(job.artifacts_file.file.read.b)

spec/rubocop/cop/include_action_view_context_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-require 'rubocop'
-require 'rubocop/rspec/support'
-
-require_relative '../../../rubocop/cop/include_action_view_context'
-
-describe RuboCop::Cop::IncludeActionViewContext do
-  include CopHelper
-
-  subject(:cop) { described_class.new }
-
-  context 'when `ActionView::Context` is included' do
-    let(:source) { 'include ActionView::Context' }
-    let(:correct_source) { 'include ::Gitlab::ActionViewOutput::Context' }
-
-    it 'registers an offense' do
-      inspect_source(source)
-
-      aggregate_failures do
-        expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.map(&:line)).to eq([1])
-        expect(cop.highlights).to eq(['ActionView::Context'])
-      end
-    end
-
-    it 'autocorrects to the right version' do
-      autocorrected = autocorrect_source(source)
-
-      expect(autocorrected).to eq(correct_source)
-    end
-  end
-
-  context 'when `ActionView::Context` is not included' do
-    it 'registers no offense' do
-      inspect_source('include Context')
-
-      aggregate_failures do
-        expect(cop.offenses.size).to eq(0)
-      end
-    end
-  end
-end

spec/support/shared_examples/controllers/repository_lfs_file_load_shared_examples.rb

@@ -41,13 +41,11 @@ RSpec.shared_examples 'a controller that can serve LFS files' do |options = {}|
       it 'serves the file' do
         lfs_uploader = LfsObjectUploader.new(lfs_object)
 
-        # Notice the filename= is omitted from the disposition; this is because
-        # Rails 5 will append this header in send_file
         expect(controller).to receive(:send_file)
                           .with(
                             File.join(lfs_uploader.root, lfs_uploader.store_dir, lfs_uploader.filename),
                             filename: filename,
-                            disposition: %Q(attachment; filename*=UTF-8''#{filename}))
+                            disposition: 'attachment')
 
         subject