Commit 57cdf56a authored by Lin Jen-Shin's avatar Lin Jen-Shin

Merge branch 'forbid-the-usage-of-reload' into 'master'

Forbid the usage of reload

Closes #60218

See merge request gitlab-org/gitlab-ce!27125
parents fcb70d9b 650f4086
Pipeline #56899021 passed with stages
in 103 minutes and 39 seconds
......@@ -204,3 +204,9 @@ Style/ReturnNil:
# nil values on the left hand side
Performance/RegexpMatch:
Enabled: false
ActiveRecordAssociationReload:
Enabled: true
Exclude:
- 'spec/**/*'
- 'ee/spec/**/*'
......@@ -40,7 +40,7 @@ class Admin::ProjectsController < Admin::ApplicationController
namespace = Namespace.find_by(id: params[:new_namespace_id])
::Projects::TransferService.new(@project, current_user, params.dup).execute(namespace)
@project.reload
@project.reset
redirect_to admin_project_path(@project)
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -55,7 +55,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
flash[:notice] = "Password was successfully updated. Please login with it"
redirect_to new_user_session_path
else
@user.reload
@user.reset
render 'edit'
end
end
......
......@@ -14,7 +14,7 @@ class Projects::ImportsController < Projects::ApplicationController
def create
if @project.update(import_params)
@project.import_state.reload.schedule
@project.import_state.reset.schedule
end
redirect_to project_import_path(@project)
......
......@@ -237,7 +237,7 @@ class ProjectsController < Projects::ApplicationController
def toggle_star
current_user.toggle_star(@project)
@project.reload
@project.reset
render json: {
star_count: @project.star_count
......
......@@ -3,6 +3,8 @@
class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
alias_method :reset, :reload
def self.id_in(ids)
where(id: ids)
end
......
......@@ -387,7 +387,7 @@ class MergeRequestDiff < ApplicationRecord
save!
end
merge_request_diff_files.reload
merge_request_diff_files.reset
end
private
......@@ -541,10 +541,10 @@ class MergeRequestDiff < ApplicationRecord
def save_commits
MergeRequestDiffCommit.create_bulk(self.id, compare.commits.reverse)
# merge_request_diff_commits.reload is preferred way to reload associated
# merge_request_diff_commits.reset is preferred way to reload associated
# objects but it returns cached result for some reason in this case
# we can circumvent that by specifying that we need an uncached reload
commits = self.class.uncached { merge_request_diff_commits.reload }
commits = self.class.uncached { merge_request_diff_commits.reset }
self.commits_count = commits.size
end
......
......@@ -20,7 +20,7 @@ module Groups
end
# reload the relation to prevent triggering destroy hooks on the projects again
group.projects.reload
group.projects.reset
group.children.each do |group|
# This needs to be synchronous since the namespace gets destroyed below
......
......@@ -22,7 +22,7 @@ module Notes
# We need to refresh the previous suggestions call cache
# in order to get the new records.
note.reload
note.reset
end
note
......
......@@ -29,7 +29,7 @@ module Projects
set_detected_repository_languages
end
project.repository_languages.reload
project.repository_languages.reset
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -26,7 +26,7 @@ module Projects
# Remove remaining project group links from source_project
def remove_remaining_project_group_links
source_project.reload.project_group_links.destroy_all # rubocop: disable DestroyAll
source_project.reset.project_group_links.destroy_all # rubocop: disable DestroyAll
end
def group_links_in_target_project
......
......@@ -30,7 +30,7 @@ module Projects
true
rescue Projects::TransferService::TransferError => ex
project.reload
project.reset
project.errors.add(:new_namespace, ex.message)
false
end
......@@ -122,7 +122,7 @@ module Projects
def rollback_side_effects
rollback_folder_move
project.reload
project.reset
update_namespace_and_visibility(@old_namespace)
update_repository_configuration(@old_path)
end
......
......@@ -33,7 +33,7 @@ module Users
end
end
user.reload
user.reset
end
private
......
......@@ -25,7 +25,7 @@ module Users
# We need an up to date User object that has access to all relations that
# may have been created earlier. The only way to ensure this is to reload
# the User object.
user.reload
user.reset
end
def execute
......@@ -84,7 +84,7 @@ module Users
# Since we batch insert authorization rows, Rails' associations may get
# out of sync. As such we force a reload of the User object.
user.reload
user.reset
end
def fresh_access_levels_per_project
......
......@@ -2,7 +2,7 @@
- if @resource.try(:unconfirmed_email?)
%p
We're contacting you to notify you that your email is being changed to #{@resource.reload.unconfirmed_email}.
We're contacting you to notify you that your email is being changed to #{@resource.reset.unconfirmed_email}.
- else
%p
We're contacting you to notify you that your email has been changed to #{@resource.email}.
......
Hello, <%= @resource.name %>!
<% if @resource.try(:unconfirmed_email?) %>
We're contacting you to notify you that your email is being changed to <%= @resource.reload.unconfirmed_email %>.
We're contacting you to notify you that your email is being changed to <%= @resource.reset.unconfirmed_email %>.
<% else %>
We're contacting you to notify you that your email has been changed to <%= @resource.email %>.
<% end %>
......
......@@ -137,7 +137,7 @@ module API
pipeline.cancel_running
status 200
present pipeline.reload, with: Entities::Pipeline
present pipeline.reset, with: Entities::Pipeline
end
end
......
......@@ -351,7 +351,7 @@ module API
not_modified!
else
current_user.toggle_star(user_project)
user_project.reload
user_project.reset
present user_project, with: Entities::Project, current_user: current_user
end
......@@ -363,7 +363,7 @@ module API
post ':id/unstar' do
if current_user.starred?(user_project)
current_user.toggle_star(user_project)
user_project.reload
user_project.reset
present user_project, with: Entities::Project, current_user: current_user
else
......@@ -403,7 +403,7 @@ module API
result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project)
if result
present user_project.reload, with: Entities::Project, current_user: current_user
present user_project.reset, with: Entities::Project, current_user: current_user
else
render_api_error!("Project already forked", 409) if user_project.forked?
end
......
......@@ -12,7 +12,7 @@ module Gitlab
retries -= 1
raise unless retries >= 0
subject.reload
subject.reset
retry
end
......
......@@ -50,7 +50,7 @@ namespace :gitlab do
puts "Waiting for the import to finish"
sleep(5)
project.reload
project.reset
end
Projects::ImportExport::ExportService.new(project, admin).execute
......
# frozen_string_literal: true
module RuboCop
module Cop
# Cop that blacklists the use of `reload`.
class ActiveRecordAssociationReload < RuboCop::Cop::Cop
MSG = 'Use reset instead of reload. ' \
'For more details check the https://gitlab.com/gitlab-org/gitlab-ce/issues/60218.'
def_node_matcher :reload?, <<~PATTERN
(send _ :reload ...)
PATTERN
def on_send(node)
return unless reload?(node)
add_offense(node, location: :selector)
end
end
end
end
......@@ -6,6 +6,7 @@ require_relative 'cop/gitlab/finder_with_find_by'
require_relative 'cop/gitlab/union'
require_relative 'cop/include_sidekiq_worker'
require_relative 'cop/safe_params'
require_relative 'cop/active_record_association_reload'
require_relative 'cop/avoid_return_from_blocks'
require_relative 'cop/avoid_break_from_strong_memoize'
require_relative 'cop/avoid_route_redirect_leading_slash'
......
......@@ -6,7 +6,7 @@ describe Gitlab::OptimisticLocking do
describe '#retry_lock' do
it 'does not reload object if state changes' do
expect(pipeline).not_to receive(:reload)
expect(pipeline).not_to receive(:reset)
expect(pipeline).to receive(:succeed).and_call_original
described_class.retry_lock(pipeline) do |subject|
......@@ -17,7 +17,7 @@ describe Gitlab::OptimisticLocking do
it 'retries action if exception is raised' do
pipeline.succeed
expect(pipeline2).to receive(:reload).and_call_original
expect(pipeline2).to receive(:reset).and_call_original
expect(pipeline2).to receive(:drop).twice.and_call_original
described_class.retry_lock(pipeline2) do |subject|
......
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require_relative '../../../rubocop/cop/active_record_association_reload'
describe RuboCop::Cop::ActiveRecordAssociationReload do
include CopHelper
subject(:cop) { described_class.new }
context 'when using ActiveRecord::Base' do
it 'registers an offense on reload usage' do
expect_offense(<<~PATTERN.strip_indent)
users = User.all
users.reload
^^^^^^ Use reset instead of reload. For more details check the https://gitlab.com/gitlab-org/gitlab-ce/issues/60218.
PATTERN
end
it 'does not register an offense on reset usage' do
expect_no_offenses(<<~PATTERN.strip_indent)
users = User.all
users.reset
PATTERN
end
end
context 'when using ActiveRecord::Relation' do
it 'registers an offense on reload usage' do
expect_offense(<<~PATTERN.strip_indent)
user = User.new
user.reload
^^^^^^ Use reset instead of reload. For more details check the https://gitlab.com/gitlab-org/gitlab-ce/issues/60218.
PATTERN
end
it 'does not register an offense on reset usage' do
expect_no_offenses(<<~PATTERN.strip_indent)
user = User.new
user.reset
PATTERN
end
end
context 'when using on self' do
it 'registers an offense on reload usage' do
expect_offense(<<~PATTERN.strip_indent)
reload
^^^^^^ Use reset instead of reload. For more details check the https://gitlab.com/gitlab-org/gitlab-ce/issues/60218.
PATTERN
end
it 'does not register an offense on reset usage' do
expect_no_offenses(<<~PATTERN.strip_indent)
reset
PATTERN
end
end
end
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