Merge branch 'upgrade-rails-5-2-ce' into 'master'

[CE] Upgrade to Rails 5.2

See merge request gitlab-org/gitlab-ce!30052

15 files changed, 53 insertions, 51 deletions

diff --git a/app/controllers/concerns/requires_whitelisted_monitoring_client.rb b/app/controllers/concerns/requires_whitelisted_monitoring_client.rb
index f47ead2f0da..2e9905997db 100644
--- a/app/controllers/concerns/requires_whitelisted_monitoring_client.rb
+++ b/app/controllers/concerns/requires_whitelisted_monitoring_client.rb
@@ -28,7 +28,7 @@ module RequiresWhitelistedMonitoringClient
   def valid_token?
     token = params[:token].presence || request.headers['TOKEN']
     token.present? &&
-      ActiveSupport::SecurityUtils.variable_size_secure_compare(
+      ActiveSupport::SecurityUtils.secure_compare(
         token,
         Gitlab::CurrentSettings.health_check_access_token
       )
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 797833e3f91..dbddee47997 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -107,7 +107,7 @@ class GroupsController < Groups::ApplicationController
     if Groups::UpdateService.new(@group, current_user, group_params).execute
       redirect_to edit_group_path(@group, anchor: params[:update_section]), notice: "Group '#{@group.name}' was successfully updated."
     else
-      @group.restore_path!
+      @group.path = @group.path_before_last_save || @group.path_was
 
       render action: "edit"
     end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index ae7a1108841..635fcc86166 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -578,7 +578,7 @@ module Ci
     end
 
     def valid_token?(token)
-      self.token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token)
+      self.token && ActiveSupport::SecurityUtils.secure_compare(token, self.token)
     end
 
     def has_tags?
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 20ca4a9ab24..2262282e647 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -196,7 +196,7 @@ module Ci
       sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC'
       query = ApplicationRecord.send(:sanitize_sql_array, [sql, sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
 
-      order(query)
+      order(Arel.sql(query))
     end
 
     scope :for_user, -> (user) { where(user: user) }
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 78bcce2f592..27a5c3d5286 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -33,22 +33,24 @@ module HasStatus
       canceled = scope_relevant.canceled.select('count(*)').to_sql
       warnings = scope_warnings.select('count(*) > 0').to_sql.presence || 'false'
 
-      "(CASE
-        WHEN (#{builds})=(#{skipped}) AND (#{warnings}) THEN 'success'
-        WHEN (#{builds})=(#{skipped}) THEN 'skipped'
-        WHEN (#{builds})=(#{success}) THEN 'success'
-        WHEN (#{builds})=(#{created}) THEN 'created'
-        WHEN (#{builds})=(#{preparing}) THEN 'preparing'
-        WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'success'
-        WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled'
-        WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending'
-        WHEN (#{running})+(#{pending})>0 THEN 'running'
-        WHEN (#{manual})>0 THEN 'manual'
-        WHEN (#{scheduled})>0 THEN 'scheduled'
-        WHEN (#{preparing})>0 THEN 'preparing'
-        WHEN (#{created})>0 THEN 'running'
-        ELSE 'failed'
-      END)"
+      Arel.sql(
+        "(CASE
+          WHEN (#{builds})=(#{skipped}) AND (#{warnings}) THEN 'success'
+          WHEN (#{builds})=(#{skipped}) THEN 'skipped'
+          WHEN (#{builds})=(#{success}) THEN 'success'
+          WHEN (#{builds})=(#{created}) THEN 'created'
+          WHEN (#{builds})=(#{preparing}) THEN 'preparing'
+          WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'success'
+          WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled'
+          WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending'
+          WHEN (#{running})+(#{pending})>0 THEN 'running'
+          WHEN (#{manual})>0 THEN 'manual'
+          WHEN (#{scheduled})>0 THEN 'scheduled'
+          WHEN (#{preparing})>0 THEN 'preparing'
+          WHEN (#{created})>0 THEN 'running'
+          ELSE 'failed'
+        END)"
+      )
     end
 
     def status
@@ -88,22 +90,22 @@ module HasStatus
       state :scheduled, value: 'scheduled'
     end
 
-    scope :created, -> { where(status: 'created') }
-    scope :preparing, -> { where(status: 'preparing') }
-    scope :relevant, -> { where(status: AVAILABLE_STATUSES - ['created']) }
-    scope :running, -> { where(status: 'running') }
-    scope :pending, -> { where(status: 'pending') }
-    scope :success, -> { where(status: 'success') }
-    scope :failed, -> { where(status: 'failed') }
-    scope :canceled, -> { where(status: 'canceled') }
-    scope :skipped, -> { where(status: 'skipped') }
-    scope :manual, -> { where(status: 'manual') }
-    scope :scheduled, -> { where(status: 'scheduled') }
-    scope :alive, -> { where(status: [:created, :preparing, :pending, :running]) }
-    scope :created_or_pending, -> { where(status: [:created, :pending]) }
-    scope :running_or_pending, -> { where(status: [:running, :pending]) }
-    scope :finished, -> { where(status: [:success, :failed, :canceled]) }
-    scope :failed_or_canceled, -> { where(status: [:failed, :canceled]) }
+    scope :created, -> { with_status(:created) }
+    scope :preparing, -> { with_status(:preparing) }
+    scope :relevant, -> { without_status(:created) }
+    scope :running, -> { with_status(:running) }
+    scope :pending, -> { with_status(:pending) }
+    scope :success, -> { with_status(:success) }
+    scope :failed, -> { with_status(:failed) }
+    scope :canceled, -> { with_status(:canceled) }
+    scope :skipped, -> { with_status(:skipped) }
+    scope :manual, -> { with_status(:manual) }
+    scope :scheduled, -> { with_status(:scheduled) }
+    scope :alive, -> { with_status(:created, :preparing, :pending, :running) }
+    scope :created_or_pending, -> { with_status(:created, :pending) }
+    scope :running_or_pending, -> { with_status(:running, :pending) }
+    scope :finished, -> { with_status(:success, :failed, :canceled) }
+    scope :failed_or_canceled, -> { with_status(:failed, :canceled) }
 
     scope :cancelable, -> do
       where(status: [:running, :preparing, :pending, :created, :scheduled])
diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index 22b6b1d720c..e4fe46d722a 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -179,7 +179,7 @@ module RelativePositioning
     relation = yield relation if block_given?
 
     relation
-      .pluck(self.class.parent_column, "#{calculation}(relative_position) AS position")
+      .pluck(self.class.parent_column, Arel.sql("#{calculation}(relative_position) AS position"))
       .first&.
       last
   end
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index b9ffc64e4a9..9becab632f3 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -46,7 +46,7 @@ module Routable
       # See https://gitlab.com/gitlab-org/gitlab-ce/issues/18603. Also note that
       # our unique index is case-sensitive in Postgres.
       binary = Gitlab::Database.mysql? ? 'BINARY' : ''
-      order_sql = "(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)"
+      order_sql = Arel.sql("(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)")
       found = where_full_path_in([path]).reorder(order_sql).take
       return found if found
 
diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb
index 8c769be0489..1293df571a3 100644
--- a/app/models/concerns/token_authenticatable.rb
+++ b/app/models/concerns/token_authenticatable.rb
@@ -52,7 +52,7 @@ module TokenAuthenticatable
 
       mod.define_method("#{token_field}_matches?") do |other_token|
         token = read_attribute(token_field)
-        token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare(other_token, token)
+        token.present? && ActiveSupport::SecurityUtils.secure_compare(other_token, token)
       end
     end
 
diff --git a/app/models/email.rb b/app/models/email.rb
index 0ddaa049c3b..580633d3232 100644
--- a/app/models/email.rb
+++ b/app/models/email.rb
@@ -4,9 +4,8 @@ class Email < ApplicationRecord
   include Sortable
   include Gitlab::SQL::Pattern
 
-  belongs_to :user
+  belongs_to :user, optional: false
 
-  validates :user_id, presence: true
   validates :email, presence: true, uniqueness: true, devise_email: true
   validate :unique_email, if: ->(email) { email.email_changed? }
 
diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb
index 22cedf57b86..5c53cfd8c27 100644
--- a/app/models/merge_requests_closing_issues.rb
+++ b/app/models/merge_requests_closing_issues.rb
@@ -25,7 +25,7 @@ class MergeRequestsClosingIssues < ApplicationRecord
 
   class << self
     def count_for_collection(ids, current_user)
-      closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', 'COUNT(*) as count')
+      closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', Arel.sql('COUNT(*) as count'))
     end
 
     def count_for_issue(id, current_user)
diff --git a/app/models/project.rb b/app/models/project.rb
index a6e0b5722b6..b278b2792f4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -357,7 +357,7 @@ class Project < ApplicationRecord
   scope :with_unmigrated_storage, -> { where('storage_version < :version OR storage_version IS NULL', version: LATEST_STORAGE_VERSION) }
 
   # last_activity_at is throttled every minute, but last_repository_updated_at is updated with every push
-  scope :sorted_by_activity, -> { reorder("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC") }
+  scope :sorted_by_activity, -> { reorder(Arel.sql("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC")) }
   scope :sorted_by_stars_desc, -> { reorder(star_count: :desc) }
   scope :sorted_by_stars_asc, -> { reorder(star_count: :asc) }
 
@@ -612,7 +612,7 @@ class Project < ApplicationRecord
     end
   end
 
-  def initialize(attributes = {})
+  def initialize(attributes = nil)
     # We can't use default_value_for because the database has a default
     # value of 0 for visibility_level. If someone attempts to create a
     # private project, default_value_for will assume that the
@@ -622,6 +622,8 @@ class Project < ApplicationRecord
     #
     # To fix the problem, we assign the actual default in the application if
     # no explicit visibility has been initialized.
+    attributes ||= {}
+
     unless visibility_attribute_present?(attributes)
       attributes[:visibility_level] = Gitlab::CurrentSettings.default_project_visibility
     end
@@ -1557,7 +1559,7 @@ class Project < ApplicationRecord
   end
 
   def valid_runners_token?(token)
-    self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
+    self.runners_token && ActiveSupport::SecurityUtils.secure_compare(token, self.runners_token)
   end
 
   # rubocop: disable CodeReuse/ServiceClass
diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb
index f0ef2d925ab..47106d7bdbb 100644
--- a/app/models/project_services/ci_service.rb
+++ b/app/models/project_services/ci_service.rb
@@ -7,7 +7,7 @@ class CiService < Service
   default_value_for :category, 'ci'
 
   def valid_token?(token)
-    self.respond_to?(:token) && self.token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token)
+    self.respond_to?(:token) && self.token.present? && ActiveSupport::SecurityUtils.secure_compare(token, self.token)
   end
 
   def self.supported_events
diff --git a/app/models/project_services/slash_commands_service.rb b/app/models/project_services/slash_commands_service.rb
index bfabc6d262c..5f5cff97808 100644
--- a/app/models/project_services/slash_commands_service.rb
+++ b/app/models/project_services/slash_commands_service.rb
@@ -12,7 +12,7 @@ class SlashCommandsService < Service
   def valid_token?(token)
     self.respond_to?(:token) &&
       self.token.present? &&
-      ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token)
+      ActiveSupport::SecurityUtils.secure_compare(token, self.token)
   end
 
   def self.supported_events
diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index 4a7ce00b8e2..aaf56048b5c 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -44,7 +44,7 @@ module Ci
 
     # rubocop: disable CodeReuse/ActiveRecord
     def stage_indexes_of_created_processables
-      created_processables.order(:stage_idx).pluck('distinct stage_idx')
+      created_processables.order(:stage_idx).pluck(Arel.sql('DISTINCT stage_idx'))
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
@@ -68,7 +68,7 @@ module Ci
       latest_statuses = pipeline.statuses.latest
         .group(:name)
         .having('count(*) > 1')
-        .pluck('max(id)', 'name')
+        .pluck(Arel.sql('MAX(id)'), 'name')
 
       # mark builds that are retried
       pipeline.statuses.latest
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index e9659f5489a..e78e5d5fc2c 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -27,8 +27,7 @@ module Groups
         @group.build_chat_team(name: response['name'], team_id: response['id'])
       end
 
-      @group.save
-      @group.add_owner(current_user)
+      @group.add_owner(current_user) if @group.save
       @group
     end