Commit 0d28fa86 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu Committed by Douglas Barbosa Alexandre

Fix various minor deprecations and changes

* Disable locking at the class-level for `update_all`

* Fixes DEPRECATION WARNING: `ActiveRecord::Result#to_hash` has been
  renamed to `to_a`. `to_hash` is deprecated and will be removed in
  Rails 6.1.

* Fix lookup of CE views

  `view_paths=` was removed so we just create a new LookupContext
  here instead of duplicating then modifying the paths

* Fix some view related spec failures

* Fix database configuration for connection pools

  ActiveRecord::Base.configurations now returns a
  ActiveRecord::DatabaseConfigurations object
parent 5bc38ff7
...@@ -200,6 +200,10 @@ class CommitStatus < ApplicationRecord ...@@ -200,6 +200,10 @@ class CommitStatus < ApplicationRecord
update_all('processed=TRUE, lock_version=COALESCE(lock_version,0)+1') update_all('processed=TRUE, lock_version=COALESCE(lock_version,0)+1')
end end
def self.locking_enabled?
false
end
def locking_enabled? def locking_enabled?
will_save_change_to_status? will_save_change_to_status?
end end
......
...@@ -130,6 +130,10 @@ module Issuable ...@@ -130,6 +130,10 @@ module Issuable
strip_attributes :title strip_attributes :title
def self.locking_enabled?
false
end
# We want to use optimistic lock for cases when only title or description are involved # We want to use optimistic lock for cases when only title or description are involved
# http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
def locking_enabled? def locking_enabled?
......
...@@ -712,7 +712,7 @@ class MergeRequest < ApplicationRecord ...@@ -712,7 +712,7 @@ class MergeRequest < ApplicationRecord
end end
def validate_branch_name(attr) def validate_branch_name(attr)
return unless changes_include?(attr) return unless will_save_change_to_attribute?(attr)
branch = read_attribute(attr) branch = read_attribute(attr)
......
...@@ -125,7 +125,7 @@ module ObjectStorage ...@@ -125,7 +125,7 @@ module ObjectStorage
included do included do
include AfterCommitQueue include AfterCommitQueue
after_save on: [:create, :update] do after_save do
background_upload(changed_mounts) background_upload(changed_mounts)
end end
end end
......
<% self.formats = ["html"] %> <% self.formats = [:html] %>
<%= raw( <%= raw(
{ {
......
...@@ -6,7 +6,7 @@ module ActiveRecord ...@@ -6,7 +6,7 @@ module ActiveRecord
self self
end end
def self.run(preloader) def self.run
end end
def self.preloaded_records def self.preloaded_records
......
...@@ -19,7 +19,7 @@ class MergeRequestsComplianceFinder < MergeRequestsFinder ...@@ -19,7 +19,7 @@ class MergeRequestsComplianceFinder < MergeRequestsFinder
.limit(1) .limit(1)
.to_sql .to_sql
sql = find_group_projects.as('projects').to_sql sql = find_group_projects.arel.as('projects').to_sql
records = Project records = Project
.select('projects.id, events.target_id as merge_request_id') .select('projects.id, events.target_id as merge_request_id')
.from([Arel.sql("#{sql} JOIN LATERAL (#{lateral}) #{Event.table_name} ON true")]) .from([Arel.sql("#{sql} JOIN LATERAL (#{lateral}) #{Event.table_name} ON true")])
......
...@@ -58,17 +58,11 @@ module EE ...@@ -58,17 +58,11 @@ module EE
def ce_lookup_context def ce_lookup_context
@ce_lookup_context ||= begin @ce_lookup_context ||= begin
context = lookup_context.dup ce_view_paths = lookup_context.view_paths.paths.reject do |resolver|
# This could duplicate the paths we're going to modify
context.view_paths = lookup_context.view_paths.paths
# Discard lookup path ee/ for the new paths
context.view_paths.paths.delete_if do |resolver|
resolver.to_path.start_with?("#{Rails.root}/ee") resolver.to_path.start_with?("#{Rails.root}/ee")
end end
context ActionView::LookupContext.new(ce_view_paths)
end end
end end
......
...@@ -19,7 +19,7 @@ module EE ...@@ -19,7 +19,7 @@ module EE
belongs_to :user belongs_to :user
belongs_to :group belongs_to :group
protected_type = self.parent.model_name.singular protected_type = self.module_parent.model_name.singular
validates :group_id, uniqueness: { scope: protected_type, allow_nil: true } validates :group_id, uniqueness: { scope: protected_type, allow_nil: true }
validates :user_id, uniqueness: { scope: protected_type, allow_nil: true } validates :user_id, uniqueness: { scope: protected_type, allow_nil: true }
validates :access_level, uniqueness: { scope: protected_type, if: :role?, validates :access_level, uniqueness: { scope: protected_type, if: :role?,
......
...@@ -22,7 +22,7 @@ module Elastic ...@@ -22,7 +22,7 @@ module Elastic
alias_method :real_class, :class alias_method :real_class, :class
def version_namespace def version_namespace
self.class.parent self.class.module_parent
end end
class_methods do class_methods do
......
...@@ -22,7 +22,7 @@ module Elastic ...@@ -22,7 +22,7 @@ module Elastic
end end
def version_namespace def version_namespace
real_class.parent real_class.module_parent
end end
class_methods do class_methods do
......
...@@ -7,7 +7,7 @@ module Elastic ...@@ -7,7 +7,7 @@ module Elastic
extend Elasticsearch::Model::Indexing::ClassMethods extend Elasticsearch::Model::Indexing::ClassMethods
extend Elasticsearch::Model::Naming::ClassMethods extend Elasticsearch::Model::Naming::ClassMethods
self.index_name = [Rails.application.class.parent_name.downcase, Rails.env].join('-') self.index_name = [Rails.application.class.module_parent_name.downcase, Rails.env].join('-')
# ES6 requires a single type per index # ES6 requires a single type per index
self.document_type = 'doc' self.document_type = 'doc'
......
...@@ -46,7 +46,7 @@ module Gitlab ...@@ -46,7 +46,7 @@ module Gitlab
end end
def migration_context def migration_context
ActiveRecord::MigrationContext.new(ActiveRecord::Migrator.migrations_paths) ActiveRecord::MigrationContext.new(ActiveRecord::Migrator.migrations_paths, ActiveRecord::SchemaMigration)
end end
def abort_if_no_geo_config! def abort_if_no_geo_config!
......
...@@ -79,7 +79,7 @@ describe Groups::InsightsController do ...@@ -79,7 +79,7 @@ describe Groups::InsightsController do
it 'does return the default config' do it 'does return the default config' do
subject subject
expect(response.parsed_body).to eq(parent_group.default_insights_config.to_json) expect(response.parsed_body).to eq(parent_group.default_insights_config.as_json)
end end
end end
......
...@@ -25,7 +25,7 @@ describe Groups::SamlProvidersController, '(JavaScript fixtures)', type: :contro ...@@ -25,7 +25,7 @@ describe Groups::SamlProvidersController, '(JavaScript fixtures)', type: :contro
get :show, params: { group_id: group } get :show, params: { group_id: group }
expect(response).to be_success expect(response).to be_successful
expect(response).to render_template 'groups/saml_providers/show' expect(response).to render_template 'groups/saml_providers/show'
end end
end end
...@@ -169,21 +169,21 @@ describe ApplicationHelper do ...@@ -169,21 +169,21 @@ describe ApplicationHelper do
it 'finds the CE partial' do it 'finds the CE partial' do
ce_partial = helper.find_ce_template(partial) ce_partial = helper.find_ce_template(partial)
expect(ce_partial.inspect).to eq(expected_partial_path) expect(ce_partial.short_identifier).to eq(expected_partial_path)
# And it could still find the EE partial # And it could still find the EE partial
ee_partial = helper.lookup_context.find(partial, [], true) ee_partial = helper.lookup_context.find(partial, [], true)
expect(ee_partial.inspect).to eq("ee/#{expected_partial_path}") expect(ee_partial.short_identifier).to eq("ee/#{expected_partial_path}")
end end
it 'finds the CE view' do it 'finds the CE view' do
ce_view = helper.find_ce_template(view) ce_view = helper.find_ce_template(view)
expect(ce_view.inspect).to eq(expected_view_path) expect(ce_view.short_identifier).to eq(expected_view_path)
# And it could still find the EE view # And it could still find the EE view
ee_view = helper.lookup_context.find(view, [], false) ee_view = helper.lookup_context.find(view, [], false)
expect(ee_view.inspect).to eq("ee/#{expected_view_path}") expect(ee_view.short_identifier).to eq("ee/#{expected_view_path}")
end end
end end
end end
......
...@@ -33,7 +33,7 @@ module Gitlab ...@@ -33,7 +33,7 @@ module Gitlab
def template_name def template_name
return unless template_name_valid? return unless template_name_valid?
location.first(-SUFFIX.length) location.delete_suffix(SUFFIX)
end end
def template_name_valid? def template_name_valid?
......
...@@ -204,15 +204,16 @@ module Gitlab ...@@ -204,15 +204,16 @@ module Gitlab
# pool_size - The size of the DB pool. # pool_size - The size of the DB pool.
# host - An optional host name to use instead of the default one. # host - An optional host name to use instead of the default one.
def self.create_connection_pool(pool_size, host = nil, port = nil) def self.create_connection_pool(pool_size, host = nil, port = nil)
# See activerecord-4.2.7.1/lib/active_record/connection_adapters/connection_specification.rb
env = Rails.env env = Rails.env
original_config = ActiveRecord::Base.configurations original_config = ActiveRecord::Base.configurations.to_h
env_config = original_config[env].merge('pool' => pool_size) env_config = original_config[env].merge('pool' => pool_size)
env_config['host'] = host if host env_config['host'] = host if host
env_config['port'] = port if port env_config['port'] = port if port
config = original_config.merge(env => env_config) config = ActiveRecord::DatabaseConfigurations.new(
original_config.merge(env => env_config)
)
spec = spec =
ActiveRecord:: ActiveRecord::
......
...@@ -382,7 +382,7 @@ module Gitlab ...@@ -382,7 +382,7 @@ module Gitlab
count_arel = table.project(Arel.star.count.as('count')) count_arel = table.project(Arel.star.count.as('count'))
count_arel = yield table, count_arel if block_given? count_arel = yield table, count_arel if block_given?
total = exec_query(count_arel.to_sql).to_hash.first['count'].to_i total = exec_query(count_arel.to_sql).to_a.first['count'].to_i
return if total == 0 return if total == 0
...@@ -399,7 +399,7 @@ module Gitlab ...@@ -399,7 +399,7 @@ module Gitlab
start_arel = table.project(table[:id]).order(table[:id].asc).take(1) start_arel = table.project(table[:id]).order(table[:id].asc).take(1)
start_arel = yield table, start_arel if block_given? start_arel = yield table, start_arel if block_given?
start_id = exec_query(start_arel.to_sql).to_hash.first['id'].to_i start_id = exec_query(start_arel.to_sql).to_a.first['id'].to_i
loop do loop do
stop_arel = table.project(table[:id]) stop_arel = table.project(table[:id])
...@@ -409,7 +409,7 @@ module Gitlab ...@@ -409,7 +409,7 @@ module Gitlab
.skip(batch_size) .skip(batch_size)
stop_arel = yield table, stop_arel if block_given? stop_arel = yield table, stop_arel if block_given?
stop_row = exec_query(stop_arel.to_sql).to_hash.first stop_row = exec_query(stop_arel.to_sql).to_a.first
update_arel = Arel::UpdateManager.new update_arel = Arel::UpdateManager.new
.table(table) .table(table)
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
describe Gitlab::Application do # rubocop:disable RSpec/FilePath describe Gitlab::Application do # rubocop:disable RSpec/FilePath
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
FILTERED_PARAM = ActionDispatch::Http::ParameterFilter::FILTERED FILTERED_PARAM = ActiveSupport::ParameterFilter::FILTERED
context 'when parameters are logged' do context 'when parameters are logged' do
describe 'rails does not leak confidential parameters' do describe 'rails does not leak confidential parameters' do
......
...@@ -23,7 +23,7 @@ describe MetricsDashboard do ...@@ -23,7 +23,7 @@ describe MetricsDashboard do
routes.draw { get "metrics_dashboard" => "anonymous#metrics_dashboard" } routes.draw { get "metrics_dashboard" => "anonymous#metrics_dashboard" }
response = get :metrics_dashboard, format: :json response = get :metrics_dashboard, format: :json
JSON.parse(response.parsed_body) response.parsed_body
end end
context 'when no parameters are provided' do context 'when no parameters are provided' do
......
...@@ -604,7 +604,7 @@ describe 'Merge request > User sees merge widget', :js do ...@@ -604,7 +604,7 @@ describe 'Merge request > User sees merge widget', :js do
click_button 'addTest' click_button 'addTest'
expect(page).to have_content('6.66') expect(page).to have_content('6.66')
expect(page).to have_content(sample_java_failed_message.gsub!(/\s+/, ' ').strip) expect(page).to have_content(sample_java_failed_message.gsub(/\s+/, ' ').strip)
end end
end end
end end
...@@ -649,7 +649,7 @@ describe 'Merge request > User sees merge widget', :js do ...@@ -649,7 +649,7 @@ describe 'Merge request > User sees merge widget', :js do
click_button 'Test#sum when a is 1 and b is 3 returns summary' click_button 'Test#sum when a is 1 and b is 3 returns summary'
expect(page).to have_content('2.22') expect(page).to have_content('2.22')
expect(page).to have_content(sample_rspec_failed_message.gsub!(/\s+/, ' ').strip) expect(page).to have_content(sample_rspec_failed_message.gsub(/\s+/, ' ').strip)
end end
end end
end end
......
...@@ -21,7 +21,7 @@ module MigrationsHelpers ...@@ -21,7 +21,7 @@ module MigrationsHelpers
end end
def migration_context def migration_context
ActiveRecord::MigrationContext.new(migrations_paths) ActiveRecord::MigrationContext.new(migrations_paths, ActiveRecord::SchemaMigration)
end end
def migrations def migrations
......
# frozen_string_literal: true # frozen_string_literal: true
require 'active_support/core_ext/hash/transform_values'
require 'active_support/hash_with_indifferent_access' require 'active_support/hash_with_indifferent_access'
require 'active_support/dependencies' require 'active_support/dependencies'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment