Commit 392227ee authored by Robert Speicher's avatar Robert Speicher

Merge branch 'no-ivar-in-modules-ee' into 'master'

EE:  Add cop to make sure we don't use ivar in a module

See merge request gitlab-org/gitlab-ee!3008
parents 3e665261 4974f15d
...@@ -1190,7 +1190,22 @@ RSpec/SubjectStub: ...@@ -1190,7 +1190,22 @@ RSpec/SubjectStub:
RSpec/VerifiedDoubles: RSpec/VerifiedDoubles:
Enabled: false Enabled: false
# GitlabSecurity ############################################################## # Gitlab ###################################################################
Gitlab/ModuleWithInstanceVariables:
Enable: true
Exclude:
# We ignore Rails helpers right now because it's hard to workaround it
- app/helpers/**/*_helper.rb
- ee/app/helpers/**/*_helper.rb
# We ignore Rails mailers right now because it's hard to workaround it
- app/mailers/emails/**/*.rb
- ee/**/emails/**/*.rb
# We ignore spec helpers because it usually doesn't matter
- spec/support/**/*.rb
- features/steps/**/*.rb
# GitlabSecurity ###########################################################
GitlabSecurity/DeepMunge: GitlabSecurity/DeepMunge:
Enabled: true Enabled: true
......
...@@ -24,11 +24,11 @@ module BoardsResponses ...@@ -24,11 +24,11 @@ module BoardsResponses
end end
def respond_with_boards def respond_with_boards
respond_with(@boards) respond_with(@boards) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def respond_with_board def respond_with_board
respond_with(@board) respond_with(@board) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def respond_with(resource) def respond_with(resource)
......
module CreatesCommit module CreatesCommit
extend ActiveSupport::Concern extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil)
if can?(current_user, :push_code, @project) if can?(current_user, :push_code, @project)
@project_to_commit_into = @project @project_to_commit_into = @project
...@@ -45,6 +47,7 @@ module CreatesCommit ...@@ -45,6 +47,7 @@ module CreatesCommit
end end
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def authorize_edit_tree! def authorize_edit_tree!
return if can_collaborate_with_project? return if can_collaborate_with_project?
...@@ -77,6 +80,7 @@ module CreatesCommit ...@@ -77,6 +80,7 @@ module CreatesCommit
end end
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def new_merge_request_path def new_merge_request_path
project_new_merge_request_path( project_new_merge_request_path(
@project_to_commit_into, @project_to_commit_into,
...@@ -88,20 +92,28 @@ module CreatesCommit ...@@ -88,20 +92,28 @@ module CreatesCommit
} }
) )
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def existing_merge_request_path def existing_merge_request_path
project_merge_request_path(@project, @merge_request) project_merge_request_path(@project, @merge_request) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def merge_request_exists? def merge_request_exists?
return @merge_request if defined?(@merge_request) strong_memoize(:merge_request) do
MergeRequestsFinder.new(current_user, project_id: @project.id)
@merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened .execute
.find_by(source_project_id: @project_to_commit_into, source_branch: @branch_name, target_branch: @start_branch) .opened
.find_by(
source_project_id: @project_to_commit_into,
source_branch: @branch_name,
target_branch: @start_branch)
end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def different_project? def different_project?
@project_to_commit_into != @project @project_to_commit_into != @project # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def create_merge_request? def create_merge_request?
...@@ -109,6 +121,6 @@ module CreatesCommit ...@@ -109,6 +121,6 @@ module CreatesCommit
# as the target branch in the same project, # as the target branch in the same project,
# we don't want to create a merge request. # we don't want to create a merge request.
params[:create_merge_request].present? && params[:create_merge_request].present? &&
(different_project? || @start_branch != @branch_name) (different_project? || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
module GroupTree module GroupTree
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def render_group_tree(groups) def render_group_tree(groups)
@groups = if params[:filter].present? @groups = if params[:filter].present?
Gitlab::GroupHierarchy.new(groups.search(params[:filter])) Gitlab::GroupHierarchy.new(groups.search(params[:filter]))
...@@ -20,5 +21,6 @@ module GroupTree ...@@ -20,5 +21,6 @@ module GroupTree
render json: serializer.represent(@groups) render json: serializer.represent(@groups)
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
end end
...@@ -17,7 +17,7 @@ module IssuableActions ...@@ -17,7 +17,7 @@ module IssuableActions
end end
def update def update
@issuable = update_service.execute(issuable) @issuable = update_service.execute(issuable) # rubocop:disable Gitlab/ModuleWithInstanceVariables
respond_to do |format| respond_to do |format|
format.html do format.html do
...@@ -81,7 +81,7 @@ module IssuableActions ...@@ -81,7 +81,7 @@ module IssuableActions
private private
def recaptcha_check_if_spammable(should_redirect = true, &block) def recaptcha_check_if_spammable(should_redirect = true, &block)
return yield unless @issuable.is_a? Spammable return yield unless issuable.is_a? Spammable
recaptcha_check_with_fallback(should_redirect, &block) recaptcha_check_with_fallback(should_redirect, &block)
end end
...@@ -89,7 +89,7 @@ module IssuableActions ...@@ -89,7 +89,7 @@ module IssuableActions
def render_conflict_response def render_conflict_response
respond_to do |format| respond_to do |format|
format.html do format.html do
@conflict = true @conflict = true # rubocop:disable Gitlab/ModuleWithInstanceVariables
render :edit render :edit
end end
...@@ -104,7 +104,7 @@ module IssuableActions ...@@ -104,7 +104,7 @@ module IssuableActions
end end
def labels def labels
@labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def authorize_destroy_issuable! def authorize_destroy_issuable!
...@@ -114,7 +114,7 @@ module IssuableActions ...@@ -114,7 +114,7 @@ module IssuableActions
end end
def authorize_admin_issuable! def authorize_admin_issuable!
unless can?(current_user, :"admin_#{resource_name}", @project) unless can?(current_user, :"admin_#{resource_name}", @project) # rubocop:disable Gitlab/ModuleWithInstanceVariables
return access_denied! return access_denied!
end end
end end
...@@ -149,6 +149,7 @@ module IssuableActions ...@@ -149,6 +149,7 @@ module IssuableActions
@resource_name ||= controller_name.singularize @resource_name ||= controller_name.singularize
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def render_entity_json def render_entity_json
if @issuable.valid? if @issuable.valid?
render json: serializer.represent(@issuable) render json: serializer.represent(@issuable)
...@@ -156,6 +157,7 @@ module IssuableActions ...@@ -156,6 +157,7 @@ module IssuableActions
render json: { errors: @issuable.errors.full_messages }, status: :unprocessable_entity render json: { errors: @issuable.errors.full_messages }, status: :unprocessable_entity
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def serializer def serializer
raise NotImplementedError raise NotImplementedError
...@@ -166,6 +168,6 @@ module IssuableActions ...@@ -166,6 +168,6 @@ module IssuableActions
end end
def parent def parent
@project || @group @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
...@@ -2,6 +2,7 @@ module IssuableCollections ...@@ -2,6 +2,7 @@ module IssuableCollections
extend ActiveSupport::Concern extend ActiveSupport::Concern
include SortingHelper include SortingHelper
include Gitlab::IssuableMetadata include Gitlab::IssuableMetadata
include Gitlab::Utils::StrongMemoize
included do included do
helper_method :finder helper_method :finder
...@@ -9,6 +10,7 @@ module IssuableCollections ...@@ -9,6 +10,7 @@ module IssuableCollections
private private
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def set_issuables_index def set_issuables_index
@issuables = issuables_collection @issuables = issuables_collection
@issuables = @issuables.page(params[:page]) @issuables = @issuables.page(params[:page])
...@@ -33,6 +35,7 @@ module IssuableCollections ...@@ -33,6 +35,7 @@ module IssuableCollections
@users.push(author) if author @users.push(author) if author
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def issuables_collection def issuables_collection
finder.execute.preload(preload_for_collection) finder.execute.preload(preload_for_collection)
...@@ -41,7 +44,7 @@ module IssuableCollections ...@@ -41,7 +44,7 @@ module IssuableCollections
def redirect_out_of_range(total_pages) def redirect_out_of_range(total_pages)
return false if total_pages.zero? return false if total_pages.zero?
out_of_range = @issuables.current_page > total_pages out_of_range = @issuables.current_page > total_pages # rubocop:disable Gitlab/ModuleWithInstanceVariables
if out_of_range if out_of_range
redirect_to(url_for(params.merge(page: total_pages, only_path: true))) redirect_to(url_for(params.merge(page: total_pages, only_path: true)))
...@@ -51,7 +54,7 @@ module IssuableCollections ...@@ -51,7 +54,7 @@ module IssuableCollections
end end
def issuable_page_count def issuable_page_count
page_count_for_relation(@issuables, finder.row_count) page_count_for_relation(@issuables, finder.row_count) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def page_count_for_relation(relation, row_count) def page_count_for_relation(relation, row_count)
...@@ -66,6 +69,7 @@ module IssuableCollections ...@@ -66,6 +69,7 @@ module IssuableCollections
finder_class.new(current_user, filter_params) finder_class.new(current_user, filter_params)
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def filter_params def filter_params
set_sort_order_from_cookie set_sort_order_from_cookie
set_default_state set_default_state
...@@ -90,6 +94,7 @@ module IssuableCollections ...@@ -90,6 +94,7 @@ module IssuableCollections
@filter_params.permit(IssuableFinder::VALID_PARAMS) @filter_params.permit(IssuableFinder::VALID_PARAMS)
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def set_default_state def set_default_state
params[:state] = 'opened' if params[:state].blank? params[:state] = 'opened' if params[:state].blank?
...@@ -131,9 +136,9 @@ module IssuableCollections ...@@ -131,9 +136,9 @@ module IssuableCollections
end end
def finder def finder
return @finder if defined?(@finder) strong_memoize(:finder) do
issuable_finder_for(@finder_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables
@finder = issuable_finder_for(@finder_type) end
end end
def collection_type def collection_type
......
...@@ -2,6 +2,7 @@ module IssuesAction ...@@ -2,6 +2,7 @@ module IssuesAction
extend ActiveSupport::Concern extend ActiveSupport::Concern
include IssuableCollections include IssuableCollections
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def issues def issues
@finder_type = IssuesFinder @finder_type = IssuesFinder
@label = finder.labels.first @label = finder.labels.first
...@@ -17,4 +18,5 @@ module IssuesAction ...@@ -17,4 +18,5 @@ module IssuesAction
format.atom { render layout: 'xml.atom' } format.atom { render layout: 'xml.atom' }
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
...@@ -2,6 +2,7 @@ module MergeRequestsAction ...@@ -2,6 +2,7 @@ module MergeRequestsAction
extend ActiveSupport::Concern extend ActiveSupport::Concern
include IssuableCollections include IssuableCollections
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def merge_requests def merge_requests
@finder_type = MergeRequestsFinder @finder_type = MergeRequestsFinder
@label = finder.labels.first @label = finder.labels.first
...@@ -10,6 +11,7 @@ module MergeRequestsAction ...@@ -10,6 +11,7 @@ module MergeRequestsAction
@issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type)
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
private private
......
...@@ -6,7 +6,7 @@ module MilestoneActions ...@@ -6,7 +6,7 @@ module MilestoneActions
format.html { redirect_to milestone_redirect_path } format.html { redirect_to milestone_redirect_path }
format.json do format.json do
render json: tabs_json("shared/milestones/_merge_requests_tab", { render json: tabs_json("shared/milestones/_merge_requests_tab", {
merge_requests: @milestone.sorted_merge_requests, merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables
show_project_name: true show_project_name: true
}) })
end end
...@@ -18,7 +18,7 @@ module MilestoneActions ...@@ -18,7 +18,7 @@ module MilestoneActions
format.html { redirect_to milestone_redirect_path } format.html { redirect_to milestone_redirect_path }
format.json do format.json do
render json: tabs_json("shared/milestones/_participants_tab", { render json: tabs_json("shared/milestones/_participants_tab", {
users: @milestone.participants users: @milestone.participants # rubocop:disable Gitlab/ModuleWithInstanceVariables
}) })
end end
end end
...@@ -29,7 +29,7 @@ module MilestoneActions ...@@ -29,7 +29,7 @@ module MilestoneActions
format.html { redirect_to milestone_redirect_path } format.html { redirect_to milestone_redirect_path }
format.json do format.json do
render json: tabs_json("shared/milestones/_labels_tab", { render json: tabs_json("shared/milestones/_labels_tab", {
labels: @milestone.labels labels: @milestone.labels # rubocop:disable Gitlab/ModuleWithInstanceVariables
}) })
end end
end end
...@@ -43,6 +43,7 @@ module MilestoneActions ...@@ -43,6 +43,7 @@ module MilestoneActions
} }
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def milestone_redirect_path def milestone_redirect_path
if @project if @project
project_milestone_path(@project, @milestone) project_milestone_path(@project, @milestone)
...@@ -52,4 +53,5 @@ module MilestoneActions ...@@ -52,4 +53,5 @@ module MilestoneActions
dashboard_milestone_path(@milestone.safe_title, title: @milestone.title) dashboard_milestone_path(@milestone.safe_title, title: @milestone.title)
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
module NotesActions module NotesActions
include RendersNotes include RendersNotes
include Gitlab::Utils::StrongMemoize
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
...@@ -30,6 +31,7 @@ module NotesActions ...@@ -30,6 +31,7 @@ module NotesActions
render json: notes_json render json: notes_json
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def create def create
create_params = note_params.merge( create_params = note_params.merge(
merge_request_diff_head_sha: params[:merge_request_diff_head_sha], merge_request_diff_head_sha: params[:merge_request_diff_head_sha],
...@@ -47,7 +49,9 @@ module NotesActions ...@@ -47,7 +49,9 @@ module NotesActions
format.html { redirect_back_or_default } format.html { redirect_back_or_default }
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def update def update
@note = Notes::UpdateService.new(project, current_user, note_params).execute(note) @note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
...@@ -60,6 +64,7 @@ module NotesActions ...@@ -60,6 +64,7 @@ module NotesActions
format.html { redirect_back_or_default } format.html { redirect_back_or_default }
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def destroy def destroy
if note.editable? if note.editable?
...@@ -138,7 +143,7 @@ module NotesActions ...@@ -138,7 +143,7 @@ module NotesActions
end end
else else
template = "discussions/_diff_discussion" template = "discussions/_diff_discussion"
@fresh_discussion = true @fresh_discussion = true # rubocop:disable Gitlab/ModuleWithInstanceVariables
locals = { discussions: [discussion], on_image: on_image } locals = { discussions: [discussion], on_image: on_image }
end end
...@@ -191,7 +196,7 @@ module NotesActions ...@@ -191,7 +196,7 @@ module NotesActions
end end
def noteable def noteable
@noteable ||= notes_finder.target || @note&.noteable @noteable ||= notes_finder.target || @note&.noteable # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def require_noteable! def require_noteable!
...@@ -211,20 +216,21 @@ module NotesActions ...@@ -211,20 +216,21 @@ module NotesActions
end end
def note_project def note_project
return @note_project if defined?(@note_project) strong_memoize(:note_project) do
return nil unless project return nil unless project
note_project_id = params[:note_project_id] note_project_id = params[:note_project_id]
@note_project = the_project =
if note_project_id.present? if note_project_id.present?
Project.find(note_project_id) Project.find(note_project_id)
else else
project project
end end
return access_denied! unless can?(current_user, :create_note, @note_project) return access_denied! unless can?(current_user, :create_note, the_project)
@note_project the_project
end
end end
end end
...@@ -14,6 +14,6 @@ module OauthApplications ...@@ -14,6 +14,6 @@ module OauthApplications
end end
def load_scopes def load_scopes
@scopes = Doorkeeper.configuration.scopes @scopes ||= Doorkeeper.configuration.scopes
end end
end end
module PreviewMarkdown module PreviewMarkdown
extend ActiveSupport::Concern extend ActiveSupport::Concern
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def preview_markdown def preview_markdown
result = PreviewMarkdownService.new(@project, current_user, params).execute result = PreviewMarkdownService.new(@project, current_user, params).execute
...@@ -20,4 +21,5 @@ module PreviewMarkdown ...@@ -20,4 +21,5 @@ module PreviewMarkdown
} }
} }
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
module RendersCommits module RendersCommits
def prepare_commits_for_rendering(commits) def prepare_commits_for_rendering(commits)
Banzai::CommitRenderer.render(commits, @project, current_user) Banzai::CommitRenderer.render(commits, @project, current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
commits commits
end end
......
module RendersNotes module RendersNotes
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def prepare_notes_for_rendering(notes, noteable = nil) def prepare_notes_for_rendering(notes, noteable = nil)
preload_noteable_for_regular_notes(notes) preload_noteable_for_regular_notes(notes)
preload_max_access_for_authors(notes, @project) preload_max_access_for_authors(notes, @project)
...@@ -7,6 +8,7 @@ module RendersNotes ...@@ -7,6 +8,7 @@ module RendersNotes
notes notes
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
private private
......
...@@ -67,7 +67,7 @@ module ServiceParams ...@@ -67,7 +67,7 @@ module ServiceParams
FILTER_BLANK_PARAMS = [:password].freeze FILTER_BLANK_PARAMS = [:password].freeze
def service_params def service_params
dynamic_params = @service.event_channel_names + @service.event_names dynamic_params = @service.event_channel_names + @service.event_names # rubocop:disable Gitlab/ModuleWithInstanceVariables
service_params = params.permit(:id, service: allowed_service_params + dynamic_params) service_params = params.permit(:id, service: allowed_service_params + dynamic_params)
if service_params[:service].is_a?(Hash) if service_params[:service].is_a?(Hash)
......
...@@ -4,6 +4,7 @@ module SnippetsActions ...@@ -4,6 +4,7 @@ module SnippetsActions
def edit def edit
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def raw def raw
disposition = params[:inline] == 'false' ? 'attachment' : 'inline' disposition = params[:inline] == 'false' ? 'attachment' : 'inline'
...@@ -14,6 +15,7 @@ module SnippetsActions ...@@ -14,6 +15,7 @@ module SnippetsActions
filename: @snippet.sanitized_file_name filename: @snippet.sanitized_file_name
) )
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
private private
......
...@@ -2,6 +2,7 @@ module SpammableActions ...@@ -2,6 +2,7 @@ module SpammableActions
extend ActiveSupport::Concern extend ActiveSupport::Concern
include Recaptcha::Verify include Recaptcha::Verify
include Gitlab::Utils::StrongMemoize
included do included do
before_action :authorize_submit_spammable!, only: :mark_as_spam before_action :authorize_submit_spammable!, only: :mark_as_spam
...@@ -18,9 +19,9 @@ module SpammableActions ...@@ -18,9 +19,9 @@ module SpammableActions
private private
def ensure_spam_config_loaded! def ensure_spam_config_loaded!
return @spam_config_loaded if defined?(@spam_config_loaded) strong_memoize(:spam_config_loaded) do
Gitlab::Recaptcha.load_configurations!
@spam_config_loaded = Gitlab::Recaptcha.load_configurations! end
end end
def recaptcha_check_with_fallback(should_redirect = true, &fallback) def recaptcha_check_with_fallback(should_redirect = true, &fallback)
......
...@@ -12,7 +12,7 @@ module ToggleSubscriptionAction ...@@ -12,7 +12,7 @@ module ToggleSubscriptionAction
private private
def subscribable_project def subscribable_project
@project || raise(NotImplementedError) @project ||= raise(NotImplementedError)
end end
def subscribable_resource def subscribable_resource
......
class AuditEvent < ActiveRecord::Base class AuditEvent < ActiveRecord::Base
prepend EE::AuditEvent prepend EE::AuditEvent
include Gitlab::Utils::StrongMemoize
serialize :details, Hash # rubocop:disable Cop/ActiveRecordSerialize serialize :details, Hash # rubocop:disable Cop/ActiveRecordSerialize
......
...@@ -44,13 +44,11 @@ module Mentionable ...@@ -44,13 +44,11 @@ module Mentionable
end end
def all_references(current_user = nil, extractor: nil) def all_references(current_user = nil, extractor: nil)
@extractors ||= {}
# Use custom extractor if it's passed in the function parameters. # Use custom extractor if it's passed in the function parameters.
if extractor if extractor
@extractors[current_user] = extractor extractors[current_user] = extractor
else else
extractor = @extractors[current_user] ||= Gitlab::ReferenceExtractor.new(project, current_user) extractor = extractors[current_user] ||= Gitlab::ReferenceExtractor.new(project, current_user)
extractor.reset_memoized_values extractor.reset_memoized_values
end end
...@@ -69,6 +67,10 @@ module Mentionable ...@@ -69,6 +67,10 @@ module Mentionable
extractor extractor
end end
def extractors
@extractors ||= {}
end
def mentioned_users(current_user = nil) def mentioned_users(current_user = nil)
all_references(current_user).users all_references(current_user).users
end end
......
...@@ -103,9 +103,11 @@ module Milestoneish ...@@ -103,9 +103,11 @@ module Milestoneish
end end
def memoize_per_user(user, method_name) def memoize_per_user(user, method_name)
@memoized ||= {} memoized_users[method_name][user&.id] ||= yield
@memoized[method_name] ||= {} end
@memoized[method_name][user&.id] ||= yield
def memoized_users
@memoized_users ||= Hash.new { |h, k| h[k] = {} }
end end
# override in a class that includes this module to get a faster query # override in a class that includes this module to get a faster query
......
...@@ -46,6 +46,7 @@ module Noteable ...@@ -46,6 +46,7 @@ module Noteable
notes.inc_relations_for_view.grouped_diff_discussions(*args) notes.inc_relations_for_view.grouped_diff_discussions(*args)
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def resolvable_discussions def resolvable_discussions
@resolvable_discussions ||= @resolvable_discussions ||=
if defined?(@discussions) if defined?(@discussions)
...@@ -54,6 +55,7 @@ module Noteable ...@@ -54,6 +55,7 @@ module Noteable
discussion_notes.resolvable.discussions(self) discussion_notes.resolvable.discussions(self)
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def discussions_resolvable? def discussions_resolvable?
resolvable_discussions.any?(&:resolvable?) resolvable_discussions.any?(&:resolvable?)
......
...@@ -56,15 +56,17 @@ module Participable ...@@ -56,15 +56,17 @@ module Participable
# #
# Returns an Array of User instances. # Returns an Array of User instances.
def participants(current_user = nil) def participants(current_user = nil)
@participants ||= Hash.new do |hash, user| all_participants[current_user]
hash[user] = raw_participants(user)
end
@participants[current_user]
end end
private private
def all_participants
@all_participants ||= Hash.new do |hash, user|
hash[user] = raw_participants(user)
end
end
def raw_participants(current_user = nil) def raw_participants(current_user = nil)
current_user ||= author current_user ||= author
ext = Gitlab::ReferenceExtractor.new(project, current_user) ext = Gitlab::ReferenceExtractor.new(project, current_user)
......
...@@ -52,7 +52,7 @@ module RelativePositioning ...@@ -52,7 +52,7 @@ module RelativePositioning
# to its predecessor. This process will recursively move all the predecessors until we have a place # to its predecessor. This process will recursively move all the predecessors until we have a place
if (after.relative_position - before.relative_position) < 2 if (after.relative_position - before.relative_position) < 2
before.move_before before.move_before
@positionable_neighbours = [before] @positionable_neighbours = [before] # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
self.relative_position = position_between(before.relative_position, after.relative_position) self.relative_position = position_between(before.relative_position, after.relative_position)
...@@ -65,7 +65,7 @@ module RelativePositioning ...@@ -65,7 +65,7 @@ module RelativePositioning
if before.shift_after? if before.shift_after?
issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_after) issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_after)
issue_to_move.move_after issue_to_move.move_after
@positionable_neighbours = [issue_to_move] @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
pos_after = issue_to_move.relative_position pos_after = issue_to_move.relative_position
end end
...@@ -80,7 +80,7 @@ module RelativePositioning ...@@ -80,7 +80,7 @@ module RelativePositioning
if after.shift_before? if after.shift_before?
issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_before) issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_before)
issue_to_move.move_before issue_to_move.move_before
@positionable_neighbours = [issue_to_move] @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
pos_before = issue_to_move.relative_position pos_before = issue_to_move.relative_position
end end
...@@ -132,6 +132,7 @@ module RelativePositioning ...@@ -132,6 +132,7 @@ module RelativePositioning
end end
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def save_positionable_neighbours def save_positionable_neighbours
return unless @positionable_neighbours return unless @positionable_neighbours
...@@ -140,4 +141,5 @@ module RelativePositioning ...@@ -140,4 +141,5 @@ module RelativePositioning
status status
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
...@@ -31,15 +31,11 @@ module ResolvableDiscussion ...@@ -31,15 +31,11 @@ module ResolvableDiscussion
end end
def resolvable? def resolvable?
return @resolvable if @resolvable.present? @resolvable ||= potentially_resolvable? && notes.any?(&:resolvable?)
@resolvable = potentially_resolvable? && notes.any?(&:resolvable?)
end end
def resolved? def resolved?
return @resolved if @resolved.present? @resolved ||= resolvable? && notes.none?(&:to_be_resolved?)
@resolved = resolvable? && notes.none?(&:to_be_resolved?)
end end
def first_note def first_note
...@@ -49,13 +45,13 @@ module ResolvableDiscussion ...@@ -49,13 +45,13 @@ module ResolvableDiscussion
def first_note_to_resolve def first_note_to_resolve
return unless resolvable? return unless resolvable?
@first_note_to_resolve ||= notes.find(&:to_be_resolved?) @first_note_to_resolve ||= notes.find(&:to_be_resolved?) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def last_resolved_note def last_resolved_note
return unless resolved? return unless resolved?
@last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def resolved_notes def resolved_notes
...@@ -95,7 +91,7 @@ module ResolvableDiscussion ...@@ -95,7 +91,7 @@ module ResolvableDiscussion
yield(notes_relation) yield(notes_relation)
# Set the notes array to the updated notes # Set the notes array to the updated notes
@notes = notes_relation.fresh.to_a @notes = notes_relation.fresh.to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables
self.class.memoized_values.each do |var| self.class.memoized_values.each do |var|
instance_variable_set(:"@#{var}", nil) instance_variable_set(:"@#{var}", nil)
......
...@@ -88,7 +88,7 @@ module Routable ...@@ -88,7 +88,7 @@ module Routable
def full_name def full_name
if route && route.name.present? if route && route.name.present?
@full_name ||= route.name @full_name ||= route.name # rubocop:disable Gitlab/ModuleWithInstanceVariables
else else
update_route if persisted? update_route if persisted?
...@@ -112,7 +112,7 @@ module Routable ...@@ -112,7 +112,7 @@ module Routable
def expires_full_path_cache def expires_full_path_cache
RequestStore.delete(full_path_key) if RequestStore.active? RequestStore.delete(full_path_key) if RequestStore.active?
@full_path = nil @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def build_full_path def build_full_path
...@@ -127,7 +127,7 @@ module Routable ...@@ -127,7 +127,7 @@ module Routable
def uncached_full_path def uncached_full_path
if route && route.path.present? if route && route.path.present?
@full_path ||= route.path @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables
else else
update_route if persisted? update_route if persisted?
...@@ -166,7 +166,7 @@ module Routable ...@@ -166,7 +166,7 @@ module Routable
route || build_route(source: self) route || build_route(source: self)
route.path = build_full_path route.path = build_full_path
route.name = build_full_name route.name = build_full_name
@full_path = nil @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
@full_name = nil @full_name = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
...@@ -12,6 +12,7 @@ module Spammable ...@@ -12,6 +12,7 @@ module Spammable
attr_accessor :spam attr_accessor :spam
attr_accessor :spam_log attr_accessor :spam_log
alias_method :spam?, :spam
after_validation :check_for_spam, on: [:create, :update] after_validation :check_for_spam, on: [:create, :update]
...@@ -34,10 +35,6 @@ module Spammable ...@@ -34,10 +35,6 @@ module Spammable
end end
end end
def spam?
@spam
end
def check_for_spam def check_for_spam
error_msg = if Gitlab::Recaptcha.enabled? error_msg = if Gitlab::Recaptcha.enabled?
"Your #{spammable_entity_type} has been recognized as spam. "\ "Your #{spammable_entity_type} has been recognized as spam. "\
......
...@@ -39,7 +39,7 @@ module Taskable ...@@ -39,7 +39,7 @@ module Taskable
def task_list_items def task_list_items
return [] if description.blank? return [] if description.blank?
@task_list_items ||= Taskable.get_tasks(description) @task_list_items ||= Taskable.get_tasks(description) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def tasks def tasks
......
...@@ -21,6 +21,7 @@ module TimeTrackable ...@@ -21,6 +21,7 @@ module TimeTrackable
has_many :timelogs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :timelogs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def spend_time(options) def spend_time(options)
@time_spent = options[:duration] @time_spent = options[:duration]
@time_spent_user = options[:user] @time_spent_user = options[:user]
...@@ -36,6 +37,7 @@ module TimeTrackable ...@@ -36,6 +37,7 @@ module TimeTrackable
end end
end end
alias_method :spend_time=, :spend_time alias_method :spend_time=, :spend_time
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def total_time_spent def total_time_spent
timelogs.sum(:time_spent) timelogs.sum(:time_spent)
...@@ -52,9 +54,10 @@ module TimeTrackable ...@@ -52,9 +54,10 @@ module TimeTrackable
private private
def reset_spent_time def reset_spent_time
timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def add_or_subtract_spent_time def add_or_subtract_spent_time
timelogs.new( timelogs.new(
time_spent: time_spent, time_spent: time_spent,
...@@ -62,16 +65,19 @@ module TimeTrackable ...@@ -62,16 +65,19 @@ module TimeTrackable
spent_at: @spent_at spent_at: @spent_at
) )
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def check_negative_time_spent def check_negative_time_spent
return if time_spent.nil? || time_spent == :reset return if time_spent.nil? || time_spent == :reset
# we need to cache the total time spent so multiple calls to #valid? if time_spent < 0 && (time_spent.abs > original_total_time_spent)
# doesn't give a false error
@original_total_time_spent ||= total_time_spent
if time_spent < 0 && (time_spent.abs > @original_total_time_spent)
errors.add(:time_spent, 'Time to subtract exceeds the total time spent') errors.add(:time_spent, 'Time to subtract exceeds the total time spent')
end end
end end
# we need to cache the total time spent so multiple calls to #valid?
# doesn't give a false error
def original_total_time_spent
@original_total_time_spent ||= total_time_spent
end
end end
...@@ -14,7 +14,7 @@ module WithPagination ...@@ -14,7 +14,7 @@ module WithPagination
# we shouldn't try to paginate single resources # we shouldn't try to paginate single resources
def represent(resource, opts = {}) def represent(resource, opts = {})
if paginated? && resource.respond_to?(:page) if paginated? && resource.respond_to?(:page)
super(@paginator.paginate(resource), opts) super(paginator.paginate(resource), opts)
else else
super(resource, opts) super(resource, opts)
end end
......
module Issues module Issues
module ResolveDiscussions module ResolveDiscussions
include Gitlab::Utils::StrongMemoize
attr_reader :merge_request_to_resolve_discussions_of_iid, :discussion_to_resolve_id attr_reader :merge_request_to_resolve_discussions_of_iid, :discussion_to_resolve_id
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def filter_resolve_discussion_params def filter_resolve_discussion_params
@merge_request_to_resolve_discussions_of_iid ||= params.delete(:merge_request_to_resolve_discussions_of) @merge_request_to_resolve_discussions_of_iid ||= params.delete(:merge_request_to_resolve_discussions_of)
@discussion_to_resolve_id ||= params.delete(:discussion_to_resolve) @discussion_to_resolve_id ||= params.delete(:discussion_to_resolve)
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def merge_request_to_resolve_discussions_of def merge_request_to_resolve_discussions_of
return @merge_request_to_resolve_discussions_of if defined?(@merge_request_to_resolve_discussions_of) strong_memoize(:merge_request_to_resolve_discussions_of) do
MergeRequestsFinder.new(current_user, project_id: project.id)
@merge_request_to_resolve_discussions_of = MergeRequestsFinder.new(current_user, project_id: project.id) .execute
.execute .find_by(iid: merge_request_to_resolve_discussions_of_iid)
.find_by(iid: merge_request_to_resolve_discussions_of_iid) end
end end
def discussions_to_resolve def discussions_to_resolve
return [] unless merge_request_to_resolve_discussions_of return [] unless merge_request_to_resolve_discussions_of
@discussions_to_resolve ||= @discussions_to_resolve ||= # rubocop:disable Gitlab/ModuleWithInstanceVariables
if discussion_to_resolve_id if discussion_to_resolve_id
discussion_or_nil = merge_request_to_resolve_discussions_of discussion_or_nil = merge_request_to_resolve_discussions_of
.find_discussion(discussion_to_resolve_id) .find_discussion(discussion_to_resolve_id)
......
...@@ -7,16 +7,19 @@ ...@@ -7,16 +7,19 @@
# - params with :request # - params with :request
# #
module SpamCheckService module SpamCheckService
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def filter_spam_check_params def filter_spam_check_params
@request = params.delete(:request) @request = params.delete(:request)
@api = params.delete(:api) @api = params.delete(:api)
@recaptcha_verified = params.delete(:recaptcha_verified) @recaptcha_verified = params.delete(:recaptcha_verified)
@spam_log_id = params.delete(:spam_log_id) @spam_log_id = params.delete(:spam_log_id)
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
# In order to be proceed to the spam check process, @spammable has to be # In order to be proceed to the spam check process, @spammable has to be
# a dirty instance, which means it should be already assigned with the new # a dirty instance, which means it should be already assigned with the new
# attribute values. # attribute values.
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def spam_check(spammable, user) def spam_check(spammable, user)
spam_service = SpamService.new(spammable, @request) spam_service = SpamService.new(spammable, @request)
...@@ -24,4 +27,5 @@ module SpamCheckService ...@@ -24,4 +27,5 @@ module SpamCheckService
user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true)
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
...@@ -9,15 +9,15 @@ module NewIssuable ...@@ -9,15 +9,15 @@ module NewIssuable
end end
def set_user(user_id) def set_user(user_id)
@user = User.find_by(id: user_id) @user = User.find_by(id: user_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables
log_error(User, user_id) unless @user log_error(User, user_id) unless @user # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def set_issuable(issuable_id) def set_issuable(issuable_id)
@issuable = issuable_class.find_by(id: issuable_id) @issuable = issuable_class.find_by(id: issuable_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables
log_error(issuable_class, issuable_id) unless @issuable log_error(issuable_class, issuable_id) unless @issuable # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def log_error(record_class, record_id) def log_error(record_class, record_id)
......
...@@ -10,8 +10,8 @@ module Prependable ...@@ -10,8 +10,8 @@ module Prependable
super super
base.singleton_class.send(:prepend, const_get('ClassMethods')) if const_defined?(:ClassMethods) base.singleton_class.send(:prepend, const_get('ClassMethods')) if const_defined?(:ClassMethods)
@_dependencies.each { |dep| base.send(:prepend, dep) } @_dependencies.each { |dep| base.send(:prepend, dep) } # rubocop:disable Gitlab/ModuleWithInstanceVariables
base.class_eval(&@_included_block) if instance_variable_defined?(:@_included_block) base.class_eval(&@_included_block) if instance_variable_defined?(:@_included_block) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
end end
......
...@@ -6,7 +6,7 @@ module LocalCacheRegistryCleanupWithEnsure ...@@ -6,7 +6,7 @@ module LocalCacheRegistryCleanupWithEnsure
def call(env) def call(env)
LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new) LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new)
response = @app.call(env) response = @app.call(env) # rubocop:disable Gitlab/ModuleWithInstanceVariables
response[2] = ::Rack::BodyProxy.new(response[2]) do response[2] = ::Rack::BodyProxy.new(response[2]) do
LocalCacheRegistry.set_cache_for(local_cache_key, nil) LocalCacheRegistry.set_cache_for(local_cache_key, nil)
end end
......
...@@ -19,10 +19,10 @@ module RspecProfilingExt ...@@ -19,10 +19,10 @@ module RspecProfilingExt
def example_finished(*args) def example_finished(*args)
super super
rescue => err rescue => err
return if @already_logged_example_finished_error return if @already_logged_example_finished_error # rubocop:disable Gitlab/ModuleWithInstanceVariables
$stderr.puts "rspec_profiling couldn't collect an example: #{err}. Further warnings suppressed." $stderr.puts "rspec_profiling couldn't collect an example: #{err}. Further warnings suppressed."
@already_logged_example_finished_error = true @already_logged_example_finished_error = true # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
alias_method :example_passed, :example_finished alias_method :example_passed, :example_finished
......
...@@ -15,8 +15,11 @@ module Rugged ...@@ -15,8 +15,11 @@ module Rugged
class Repository class Repository
module UseGitlabGitAttributes module UseGitlabGitAttributes
def fetch_attributes(name, *) def fetch_attributes(name, *)
attributes.attributes(name)
end
def attributes
@attributes ||= Gitlab::Git::Attributes.new(path) @attributes ||= Gitlab::Git::Attributes.new(path)
@attributes.attributes(name)
end end
end end
......
...@@ -37,6 +37,7 @@ comments: false ...@@ -37,6 +37,7 @@ comments: false
- [`Gemfile` guidelines](gemfile.md) - [`Gemfile` guidelines](gemfile.md)
- [Sidekiq debugging](sidekiq_debugging.md) - [Sidekiq debugging](sidekiq_debugging.md)
- [Gotchas](gotchas.md) to avoid - [Gotchas](gotchas.md) to avoid
- [Avoid modules with instance variables](module_with_instance_variables.md) if possible
- [Issue and merge requests state models](object_state_models.md) - [Issue and merge requests state models](object_state_models.md)
- [How to dump production data to staging](db_dump.md) - [How to dump production data to staging](db_dump.md)
- [Working with the GitHub importer](github_importer.md) - [Working with the GitHub importer](github_importer.md)
......
## Modules with instance variables could be considered harmful
### Background
Rails somehow encourages people using modules and instance variables
everywhere. For example, using instance variables in the controllers,
helpers, and views. They're also encouraging the use of
`ActiveSupport::Concern`, which further strengthens the idea of
saving everything in a giant, single object, and people could access
everything in that one giant object.
### The problems
Of course this is convenient to develop, because we just have everything
within reach. However this has a number of downsides when that chosen object
is growing, it would later become out of control for the same reason.
There are just too many things in the same context, and we don't know if
those things are tightly coupled or not, depending on each others or not.
It's very hard to tell when the complexity grows to a point, and it makes
tracking the code also extremely hard. For example, a class could be using
3 different instance variables, and all of them could be initialized and
manipulated from 3 different modules. It's hard to track when those variables
start giving us troubles. We don't know which module would suddenly change
one of the variables. Everything could touch anything.
### Similar concerns
People are saying multiple inheritance is bad. Mixing multiple modules with
multiple instance variables scattering everywhere suffer from the same issue.
The same applies to `ActiveSupport::Concern`. See:
[Consider replacing concerns with dedicated classes & composition](
https://gitlab.com/gitlab-org/gitlab-ce/issues/23786)
There's also a similar idea:
[Use decorators and interface segregation to solve overgrowing models problem](
https://gitlab.com/gitlab-org/gitlab-ce/issues/13484)
Note that `included` doesn't solve the whole issue. They define the
dependencies, but they still allow each modules to talk implicitly via the
instance variables in the final giant object, and that's where the problem is.
### Solutions
We should split the giant object into multiple objects, and they communicate
with each other with the API, i.e. public methods. In short, composition over
inheritance. This way, each smaller objects would have their own respective
limited states, i.e. instance variables. If one instance variable goes wrong,
we would be very clear that it's from that single small object, because
no one else could be touching it.
With clearly defined API, this would make things less coupled and much easier
to debug and track, and much more extensible for other objects to use, because
they communicate in a clear way, rather than implicit dependencies.
### Acceptable use
However, it's not always bad to use instance variables in a module,
as long as it's contained in the same module; that is, no other modules or
objects are touching them, then it would be an acceptable use.
We especially allow the case where a single instance variable is used with
`||=` to setup the value. This would look like:
``` ruby
module M
def f
@f ||= true
end
end
```
Unfortunately it's not easy to code more complex rules into the cop, so
we rely on people's best judgement. If we could find another good pattern
we could easily add to the cop, we should do it.
### How to rewrite and avoid disabling this cop
Even if we could just disable the cop, we should avoid doing so. Some code
could be easily rewritten in simple form. Consider this acceptable method:
``` ruby
module Gitlab
module Emoji
def emoji_unicode_version(name)
@emoji_unicode_versions_by_name ||=
JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json')))
@emoji_unicode_versions_by_name[name]
end
end
end
```
This method is totally fine because it's already self-contained. No other
methods should be using `@emoji_unicode_versions_by_name` and we're good.
However it's still offending the cop because it's not just `||=`, and the
cop is not smart enough to judge that this is fine.
On the other hand, we could split this method into two:
``` ruby
module Gitlab
module Emoji
def emoji_unicode_version(name)
emoji_unicode_versions_by_name[name]
end
private
def emoji_unicode_versions_by_name
@emoji_unicode_versions_by_name ||=
JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json')))
end
end
end
```
Now the cop won't complain. Here's a bad example which we could rewrite:
``` ruby
module SpamCheckService
def filter_spam_check_params
@request = params.delete(:request)
@api = params.delete(:api)
@recaptcha_verified = params.delete(:recaptcha_verified)
@spam_log_id = params.delete(:spam_log_id)
end
def spam_check(spammable, user)
spam_service = SpamService.new(spammable, @request)
spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do
user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true)
end
end
end
```
There are several implicit dependencies here. First, `params` should be
defined before use. Second, `filter_spam_check_params` should be called
before `spam_check`. These are all implicit and the includer could be using
those instance variables without awareness.
This should be rewritten like:
``` ruby
class SpamCheckService
def initialize(request:, api:, recaptcha_verified:, spam_log_id:)
@request = request
@api = api
@recaptcha_verified = recaptcha_verified
@spam_log_id = spam_log_id
end
def spam_check(spammable, user)
spam_service = SpamService.new(spammable, @request)
spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do
user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true)
end
end
end
```
And use it like:
``` ruby
class UpdateSnippetService < BaseService
def execute
# ...
spam = SpamCheckService.new(params.slice!(:request, :api, :recaptcha_verified, :spam_log_id))
spam.check(snippet, current_user)
# ...
end
end
```
This way, all those instance variables are isolated in `SpamCheckService`
rather than whatever includes the module, and those modules which were also
included, making it much easier to track down any issues,
and reducing the chance of having name conflicts.
### How to disable this cop
Put the disabling comment right after your code in the same line:
``` ruby
module M
def violating_method
@f + @g # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
end
```
If there are multiple lines, you could also enable and disable for a section:
``` ruby
module M
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def violating_method
@f = 0
@g = 1
@h = 2
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end
```
Note that you need to enable it at some point, otherwise everything below
won't be checked.
### Things we might need to ignore right now
Because of the way Rails helpers and mailers work, we might not be able to
avoid the use of instance variables there. For those cases, we could ignore
them at the moment. At least we're not going to share those modules with
other random objects, so they're still somewhat isolated.
### Instance variables in views
They're bad because we can't easily tell who's using the instance variables
(from controller's point of view) and where we set them up (from partials'
point of view), making it extremely hard to track data dependency.
We're trying to use something like this instead:
``` haml
= render 'projects/commits/commit', commit: commit, ref: ref, project: project
```
And in the partial:
``` haml
- ref = local_assigns.fetch(:ref)
- commit = local_assigns.fetch(:commit)
- project = local_assigns.fetch(:project)
```
This way it's clearer where those values were coming from, and we gain the
benefit to have typo check over using instance variables. In the future,
we should also forbid the use of instance variables in partials.
module EE module EE
module LfsRequest module LfsRequest
extend ActiveSupport::Concern extend ActiveSupport::Concern
include ::Gitlab::Utils::StrongMemoize
def lfs_forbidden! def lfs_forbidden!
if project.above_size_limit? || objects_exceed_repo_limit? if project.above_size_limit? || objects_exceed_repo_limit?
...@@ -13,7 +14,7 @@ module EE ...@@ -13,7 +14,7 @@ module EE
def render_size_error def render_size_error
render( render(
json: { json: {
message: ::Gitlab::RepositorySizeError.new(project).push_error(@exceeded_limit), message: ::Gitlab::RepositorySizeError.new(project).push_error(@exceeded_limit), # rubocop:disable Gitlab/ModuleWithInstanceVariables
documentation_url: help_url documentation_url: help_url
}, },
content_type: "application/vnd.git-lfs+json", content_type: "application/vnd.git-lfs+json",
...@@ -23,13 +24,14 @@ module EE ...@@ -23,13 +24,14 @@ module EE
def objects_exceed_repo_limit? def objects_exceed_repo_limit?
return false unless project.size_limit_enabled? return false unless project.size_limit_enabled?
return @limit_exceeded if defined?(@limit_exceeded)
lfs_push_size = objects.sum { |o| o[:size] } strong_memoize(:limit_exceeded) do
size_with_lfs_push = project.repository_and_lfs_size + lfs_push_size lfs_push_size = objects.sum { |o| o[:size] }
size_with_lfs_push = project.repository_and_lfs_size + lfs_push_size
@exceeded_limit = size_with_lfs_push - project.actual_size_limit @exceeded_limit = size_with_lfs_push - project.actual_size_limit # rubocop:disable Gitlab/ModuleWithInstanceVariables
@limit_exceeded = @exceeded_limit > 0 @exceeded_limit > 0 # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
end end
end end
end end
...@@ -14,6 +14,6 @@ module SafeMirrorParams ...@@ -14,6 +14,6 @@ module SafeMirrorParams
end end
def default_mirror_users def default_mirror_users
[current_user, @project.mirror_user].compact.uniq [current_user, project.mirror_user].compact.uniq
end end
end end
module EE::Admin::LogsController module EE::Admin::LogsController
include ::Gitlab::Utils::StrongMemoize
def loggers def loggers
raise NotImplementedError unless defined?(super) raise NotImplementedError unless defined?(super)
@loggers ||= super + [ strong_memoize(:loggers) do
Gitlab::GeoLogger super + [
] Gitlab::GeoLogger
]
end
end end
end end
...@@ -24,6 +24,7 @@ module EE ...@@ -24,6 +24,7 @@ module EE
end end
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def update def update
service = ::Boards::UpdateService.new(parent, current_user, board_params) service = ::Boards::UpdateService.new(parent, current_user, board_params)
...@@ -40,10 +41,11 @@ module EE ...@@ -40,10 +41,11 @@ module EE
end end
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def destroy def destroy
service = ::Boards::DestroyService.new(parent, current_user) service = ::Boards::DestroyService.new(parent, current_user)
service.execute(@board) service.execute(@board) # rubocop:disable Gitlab/ModuleWithInstanceVariables
respond_to do |format| respond_to do |format|
format.json { head :ok } format.json { head :ok }
...@@ -62,15 +64,15 @@ module EE ...@@ -62,15 +64,15 @@ module EE
end end
def find_board def find_board
@board = parent.boards.find(params[:id]) @board = parent.boards.find(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def parent def parent
@parent ||= @project || @group @parent ||= @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def boards_path def boards_path
if @group if @group # rubocop:disable Gitlab/ModuleWithInstanceVariables
group_boards_path(parent) group_boards_path(parent)
else else
project_boards_path(parent) project_boards_path(parent)
...@@ -78,7 +80,7 @@ module EE ...@@ -78,7 +80,7 @@ module EE
end end
def board_path(board) def board_path(board)
if @group if @group # rubocop:disable Gitlab/ModuleWithInstanceVariables
group_board_path(parent, board) group_board_path(parent, board)
else else
project_board_path(parent, board) project_board_path(parent, board)
......
module EE module EE
module Groups module Groups
module GroupMembersController module GroupMembersController
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def override def override
@group_member = @group.group_members.find(params[:id]) @group_member = @group.group_members.find(params[:id])
...@@ -14,6 +15,7 @@ module EE ...@@ -14,6 +15,7 @@ module EE
end end
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
protected protected
......
...@@ -3,7 +3,7 @@ module EE ...@@ -3,7 +3,7 @@ module EE
protected protected
def fail_login def fail_login
log_failed_login(@user.username, oauth['provider']) log_failed_login(@user.username, oauth['provider']) # rubocop:disable Gitlab/ModuleWithInstanceVariables
super super
end end
......
...@@ -33,7 +33,7 @@ module EE ...@@ -33,7 +33,7 @@ module EE
payload = ::Gitlab::Geo::JwtRequestDecoder.new(request.headers['Authorization']).decode payload = ::Gitlab::Geo::JwtRequestDecoder.new(request.headers['Authorization']).decode
if payload if payload
@authentication_result = ::Gitlab::Auth::Result.new(nil, project, :geo, [:download_code]) @authentication_result = ::Gitlab::Auth::Result.new(nil, project, :geo, [:download_code]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
return # grant access return # grant access
end end
......
...@@ -11,8 +11,8 @@ module EE ...@@ -11,8 +11,8 @@ module EE
end end
def service_desk def service_desk
@issues = @issuables @issues = @issuables # rubocop:disable Gitlab/ModuleWithInstanceVariables
@users.push(::User.support_bot) @users.push(::User.support_bot) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def export_csv def export_csv
......
...@@ -7,11 +7,11 @@ module EE ...@@ -7,11 +7,11 @@ module EE
private private
def set_suggested_approvers def set_suggested_approvers
if @merge_request.requires_approve? if merge_request.requires_approve?
@suggested_approvers = ::Gitlab::AuthorityAnalyzer.new( @suggested_approvers = ::Gitlab::AuthorityAnalyzer.new( # rubocop:disable Gitlab/ModuleWithInstanceVariables
@merge_request, merge_request,
@merge_request.author || current_user merge_request.author || current_user
).calculate(@merge_request.approvals_required) ).calculate(merge_request.approvals_required)
end end
end end
...@@ -38,12 +38,12 @@ module EE ...@@ -38,12 +38,12 @@ module EE
# Target the MR target project in priority, else it depends whether the project # Target the MR target project in priority, else it depends whether the project
# is forked. # is forked.
target_project = if @merge_request target_project = if @merge_request # rubocop:disable Gitlab/ModuleWithInstanceVariables
@merge_request.target_project @merge_request.target_project # rubocop:disable Gitlab/ModuleWithInstanceVariables
elsif @project.forked? && @project.id.to_s != mr_params[:target_project_id] elsif project.forked? && project.id.to_s != mr_params[:target_project_id]
@project.forked_from_project project.forked_from_project
else else
@project project
end end
if mr_params[:approvals_before_merge].to_i <= target_project.approvals_before_merge if mr_params[:approvals_before_merge].to_i <= target_project.approvals_before_merge
......
...@@ -9,19 +9,19 @@ module EE ...@@ -9,19 +9,19 @@ module EE
end end
def rebase def rebase
RebaseWorker.perform_async(@merge_request.id, current_user.id) RebaseWorker.perform_async(merge_request.id, current_user.id)
render nothing: true, status: 200 render nothing: true, status: 200
end end
def approve def approve
unless @merge_request.can_approve?(current_user) unless merge_request.can_approve?(current_user)
return render_404 return render_404
end end
::MergeRequests::ApprovalService ::MergeRequests::ApprovalService
.new(project, current_user) .new(project, current_user)
.execute(@merge_request) .execute(merge_request)
render_approvals_json render_approvals_json
end end
...@@ -31,10 +31,10 @@ module EE ...@@ -31,10 +31,10 @@ module EE
end end
def unapprove def unapprove
if @merge_request.has_approved?(current_user) if merge_request.has_approved?(current_user)
::MergeRequests::RemoveApprovalService ::MergeRequests::RemoveApprovalService
.new(project, current_user) .new(project, current_user)
.execute(@merge_request) .execute(merge_request)
end end
render_approvals_json render_approvals_json
...@@ -51,7 +51,7 @@ module EE ...@@ -51,7 +51,7 @@ module EE
def render_approvals_json def render_approvals_json
respond_to do |format| respond_to do |format|
format.json do format.json do
entity = ::API::Entities::MergeRequestApprovals.new(@merge_request, current_user: current_user) entity = ::API::Entities::MergeRequestApprovals.new(merge_request, current_user: current_user)
render json: entity render json: entity
end end
end end
...@@ -65,11 +65,11 @@ module EE ...@@ -65,11 +65,11 @@ module EE
end end
def check_user_can_push_to_source_branch! def check_user_can_push_to_source_branch!
return access_denied! unless @merge_request.source_branch_exists? return access_denied! unless merge_request.source_branch_exists?
access_check = ::Gitlab::UserAccess access_check = ::Gitlab::UserAccess
.new(current_user, project: @merge_request.source_project) .new(current_user, project: merge_request.source_project)
.can_push_to_branch?(@merge_request.source_branch) .can_push_to_branch?(merge_request.source_branch)
access_denied! unless access_check access_denied! unless access_check
end end
......
...@@ -15,15 +15,16 @@ module EE ...@@ -15,15 +15,16 @@ module EE
return unless project.feature_available?(:push_rules) return unless project.feature_available?(:push_rules)
project.create_push_rule unless project.push_rule project.create_push_rule unless project.push_rule
@push_rule = project.push_rule @push_rule = project.push_rule # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def remote_mirror def remote_mirror
return unless project.feature_available?(:repository_mirrors) return unless project.feature_available?(:repository_mirrors)
@remote_mirror = @project.remote_mirrors.first_or_initialize @remote_mirror = project.remote_mirrors.first_or_initialize # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def acces_levels_options def acces_levels_options
super.merge( super.merge(
selected_merge_access_levels: @protected_branch.merge_access_levels.map { |access_level| access_level.user_id || access_level.access_level }, selected_merge_access_levels: @protected_branch.merge_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
...@@ -31,11 +32,12 @@ module EE ...@@ -31,11 +32,12 @@ module EE
selected_create_access_levels: @protected_tag.create_access_levels.map { |access_level| access_level.user_id || access_level.access_level } selected_create_access_levels: @protected_tag.create_access_levels.map { |access_level| access_level.user_id || access_level.access_level }
) )
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def load_gon_index def load_gon_index
super super
gon.push(current_project_id: @project.id) if @project gon.push(current_project_id: project.id) if project
end end
end end
end end
......
...@@ -26,7 +26,7 @@ module EE ...@@ -26,7 +26,7 @@ module EE
return unless ::Gitlab::Geo.secondary? return unless ::Gitlab::Geo.secondary?
oauth = ::Gitlab::Geo::OauthSession.new(access_token: session[:access_token]) oauth = ::Gitlab::Geo::OauthSession.new(access_token: session[:access_token])
@geo_logout_state = oauth.generate_logout_state @geo_logout_state = oauth.generate_logout_state # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def log_failed_login def log_failed_login
......
...@@ -10,7 +10,7 @@ module EE ...@@ -10,7 +10,7 @@ module EE
return unless entity_type && entity_id return unless entity_type && entity_id
# Avoiding exception if the record doesn't exist # Avoiding exception if the record doesn't exist
@entity ||= entity_type.constantize.find_by_id(entity_id) @entity ||= entity_type.constantize.find_by_id(entity_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def present def present
......
...@@ -5,6 +5,7 @@ module EE ...@@ -5,6 +5,7 @@ module EE
# and be prepended in the `Namespace` model # and be prepended in the `Namespace` model
module Namespace module Namespace
extend ActiveSupport::Concern extend ActiveSupport::Concern
include ::Gitlab::Utils::StrongMemoize
FREE_PLAN = 'free'.freeze FREE_PLAN = 'free'.freeze
...@@ -67,19 +68,23 @@ module EE ...@@ -67,19 +68,23 @@ module EE
# for a given Namespace plan. This method should consider ancestor groups # for a given Namespace plan. This method should consider ancestor groups
# being licensed. # being licensed.
def feature_available?(feature) def feature_available?(feature)
@feature_available ||= Hash.new do |h, feature| available_features = strong_memoize(:feature_available) do
h[feature] = load_feature_available(feature) Hash.new do |h, feature|
h[feature] = load_feature_available(feature)
end
end end
@feature_available[feature] available_features[feature]
end end
def feature_available_in_plan?(feature) def feature_available_in_plan?(feature)
@features_available_in_plan ||= Hash.new do |h, feature| available_features = strong_memoize(:features_available_in_plan) do
h[feature] = (plans.map(&:name) & self.class.plans_with_feature(feature)).any? Hash.new do |h, feature|
h[feature] = (plans.map(&:name) & self.class.plans_with_feature(feature)).any?
end
end end
@features_available_in_plan[feature] available_features[feature]
end end
# The main difference between the "plan" column and this method is that "plan" # The main difference between the "plan" column and this method is that "plan"
...@@ -128,9 +133,9 @@ module EE ...@@ -128,9 +133,9 @@ module EE
# These helper methods are required to not break the Namespace API. # These helper methods are required to not break the Namespace API.
def plan=(plan_name) def plan=(plan_name)
if plan_name.is_a?(String) if plan_name.is_a?(String)
@plan_name = plan_name @plan_name = plan_name # rubocop:disable Gitlab/ModuleWithInstanceVariables
super(Plan.find_by(name: @plan_name)) super(Plan.find_by(name: @plan_name)) # rubocop:disable Gitlab/ModuleWithInstanceVariables
else else
super super
end end
...@@ -149,7 +154,7 @@ module EE ...@@ -149,7 +154,7 @@ module EE
private private
def validate_plan_name def validate_plan_name
if @plan_name.present? && PLANS.exclude?(@plan_name) if @plan_name.present? && PLANS.exclude?(@plan_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables
errors.add(:plan, 'is not included in the list') errors.add(:plan, 'is not included in the list')
end end
end end
......
...@@ -5,6 +5,7 @@ module EE ...@@ -5,6 +5,7 @@ module EE
# and be prepended in the `Project` model # and be prepended in the `Project` model
module Project module Project
extend ActiveSupport::Concern extend ActiveSupport::Concern
include ::Gitlab::Utils::StrongMemoize
prepended do prepended do
include Elastic::ProjectsSearch include Elastic::ProjectsSearch
...@@ -258,8 +259,9 @@ module EE ...@@ -258,8 +259,9 @@ module EE
def deployment_platform(environment: nil) def deployment_platform(environment: nil)
return super unless environment && feature_available?(:multiple_clusters) return super unless environment && feature_available?(:multiple_clusters)
@deployment_platform ||= clusters.enabled.on_environment(environment.name) @deployment_platform ||= # rubocop:disable Gitlab/ModuleWithInstanceVariables
.last&.platform_kubernetes clusters.enabled.on_environment(environment.name)
.last&.platform_kubernetes
super # Wildcard or KubernetesService super # Wildcard or KubernetesService
end end
...@@ -323,8 +325,11 @@ module EE ...@@ -323,8 +325,11 @@ module EE
end end
def find_path_lock(path, exact_match: false, downstream: false) def find_path_lock(path, exact_match: false, downstream: false)
@path_lock_finder ||= ::Gitlab::PathLocksFinder.new(self) path_lock_finder = strong_memoize(:path_lock_finder) do
@path_lock_finder.find(path, exact_match: exact_match, downstream: downstream) ::Gitlab::PathLocksFinder.new(self)
end
path_lock_finder.find(path, exact_match: exact_match, downstream: downstream)
end end
def import_url_updated? def import_url_updated?
...@@ -450,15 +455,15 @@ module EE ...@@ -450,15 +455,15 @@ module EE
end end
def disabled_services def disabled_services
return @disabled_services if defined?(@disabled_services) strong_memoize(:disabled_services) do
disabled_services = []
@disabled_services = [] unless feature_available?(:jenkins_integration)
disabled_services.push('jenkins', 'jenkins_deprecated')
end
unless feature_available?(:jenkins_integration) disabled_services
@disabled_services.push('jenkins', 'jenkins_deprecated')
end end
@disabled_services
end end
def remote_mirror_available? def remote_mirror_available?
...@@ -479,11 +484,13 @@ module EE ...@@ -479,11 +484,13 @@ module EE
end end
def licensed_feature_available?(feature) def licensed_feature_available?(feature)
@licensed_feature_available ||= Hash.new do |h, feature| available_features = strong_memoize(:licensed_feature_available) do
h[feature] = load_licensed_feature_available(feature) Hash.new do |h, feature|
h[feature] = load_licensed_feature_available(feature)
end
end end
@licensed_feature_available[feature] available_features[feature]
end end
def load_licensed_feature_available(feature) def load_licensed_feature_available(feature)
......
module EE module EE
module Applications module Applications
# rubocop:disable Gitlab/ModuleWithInstanceVariables
module CreateService module CreateService
def execute(request) def execute(request)
super.tap do |application| super.tap do |application|
......
module EE module EE
module AuditEventService module AuditEventService
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def for_member(member) def for_member(member)
action = @details[:action] action = @details[:action]
old_access_level = @details[:old_access_level] old_access_level = @details[:old_access_level]
...@@ -199,5 +200,6 @@ module EE ...@@ -199,5 +200,6 @@ module EE
} }
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
end end
...@@ -3,8 +3,8 @@ module EE ...@@ -3,8 +3,8 @@ module EE
module Issues module Issues
module ListService module ListService
def set_parent def set_parent
if @parent.is_a?(Group) if parent.is_a?(Group)
params[:group_id] = @parent.id params[:group_id] = parent.id
else else
super super
end end
......
...@@ -12,11 +12,11 @@ module EE ...@@ -12,11 +12,11 @@ module EE
end end
def audit_event_service def audit_event_service
::AuditEventService.new(@user, ::AuditEventService.new(user,
@user, user,
action: :custom, action: :custom,
custom_message: 'Added SSH key', custom_message: 'Added SSH key',
ip_address: @ip_address) ip_address: @ip_address) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
end end
......
...@@ -25,8 +25,8 @@ module EE ...@@ -25,8 +25,8 @@ module EE
private private
def check_size_limit def check_size_limit
if @merge_request.target_project.above_size_limit? if merge_request.target_project.above_size_limit?
message = ::Gitlab::RepositorySizeError.new(@merge_request.target_project).merge_error message = ::Gitlab::RepositorySizeError.new(merge_request.target_project).merge_error
raise ::MergeRequests::MergeService::MergeError, message raise ::MergeRequests::MergeService::MergeError, message
end end
......
...@@ -41,7 +41,7 @@ module EE ...@@ -41,7 +41,7 @@ module EE
create_predefined_push_rule create_predefined_push_rule
@project.group&.refresh_members_authorized_projects project.group&.refresh_members_authorized_projects
end end
def create_predefined_push_rule def create_predefined_push_rule
......
...@@ -8,12 +8,12 @@ module EE ...@@ -8,12 +8,12 @@ module EE
super super
EE::Audit::ProjectChangesAuditor.new(@current_user, project).execute EE::Audit::ProjectChangesAuditor.new(current_user, project).execute
::Geo::RepositoryRenamedEventStore.new( ::Geo::RepositoryRenamedEventStore.new(
project, project,
old_path: project.path, old_path: project.path,
old_path_with_namespace: @old_path old_path_with_namespace: @old_path # rubocop:disable Gitlab/ModuleWithInstanceVariables
).create ).create
end end
end end
......
...@@ -26,15 +26,15 @@ module EE ...@@ -26,15 +26,15 @@ module EE
end end
def groups_accessible? def groups_accessible?
group_ids = @merge_params.group_ids + @push_params.group_ids group_ids = @merge_params.group_ids + @push_params.group_ids # rubocop:disable Gitlab/ModuleWithInstanceVariables
allowed_groups = @project.invited_groups.where(id: group_ids) allowed_groups = @project.invited_groups.where(id: group_ids) # rubocop:disable Gitlab/ModuleWithInstanceVariables
group_ids.count == allowed_groups.count group_ids.count == allowed_groups.count
end end
def users_accessible? def users_accessible?
user_ids = @merge_params.user_ids + @push_params.user_ids user_ids = @merge_params.user_ids + @push_params.user_ids # rubocop:disable Gitlab/ModuleWithInstanceVariables
allowed_users = @project.team.users.where(id: user_ids) allowed_users = @project.team.users.where(id: user_ids) # rubocop:disable Gitlab/ModuleWithInstanceVariables
user_ids.count == allowed_users.count user_ids.count == allowed_users.count
end end
......
...@@ -3,7 +3,7 @@ module EE ...@@ -3,7 +3,7 @@ module EE
def execute(blocking: true) def execute(blocking: true)
result = super result = super
@user_ids.each do |id| @user_ids.each do |id| # rubocop:disable Gitlab/ModuleWithInstanceVariables
::Gitlab::Database::LoadBalancing::Sticking.stick(:user, id) ::Gitlab::Database::LoadBalancing::Sticking.stick(:user, id)
end end
......
...@@ -6,7 +6,7 @@ module EE ...@@ -6,7 +6,7 @@ module EE
private private
def notify_success(user_exists) def notify_success(user_exists)
notify_new_user(@user, nil) unless user_exists notify_new_user(@user, nil) unless user_exists # rubocop:disable Gitlab/ModuleWithInstanceVariables
audit_changes(:email, as: 'email address') audit_changes(:email, as: 'email address')
audit_changes(:encrypted_password, as: 'password', skip_changes: true) audit_changes(:encrypted_password, as: 'password', skip_changes: true)
......
...@@ -2,14 +2,14 @@ module EE ...@@ -2,14 +2,14 @@ module EE
module API module API
module Helpers module Helpers
def current_user def current_user
return @current_user if defined?(@current_user) strong_memoize(:current_user) do
user = super
user = super ::Gitlab::Database::LoadBalancing::RackMiddleware
.stick_or_unstick(env, :user, user.id) if user
::Gitlab::Database::LoadBalancing::RackMiddleware user
.stick_or_unstick(env, :user, user.id) if user end
user
end end
def check_project_feature_available!(feature) def check_project_feature_available!(feature)
......
...@@ -6,7 +6,7 @@ module EE ...@@ -6,7 +6,7 @@ module EE
private private
def project_or_wiki def project_or_wiki
@project.wiki project.wiki
end end
end end
end end
......
...@@ -2,24 +2,26 @@ module EE ...@@ -2,24 +2,26 @@ module EE
module Gitlab module Gitlab
module OAuth module OAuth
module AuthHash module AuthHash
include ::Gitlab::Utils::StrongMemoize
def kerberos_default_realm def kerberos_default_realm
::Gitlab::Kerberos::Authentication.kerberos_default_realm ::Gitlab::Kerberos::Authentication.kerberos_default_realm
end end
def uid def uid
return @ee_uid if defined?(@ee_uid) strong_memoize(:ee_uid) do
ee_uid = super
ee_uid = super # For Kerberos, usernames `principal` and `principal@DEFAULT.REALM`
# are equivalent and may be used indifferently, but omniauth_kerberos
# does not normalize them as of version 0.3.0, so add the default
# realm ourselves if appropriate
if provider == 'kerberos' && ee_uid.present?
ee_uid += "@#{kerberos_default_realm}" unless ee_uid.include?('@')
end
# For Kerberos, usernames `principal` and `principal@DEFAULT.REALM` ee_uid
# are equivalent and may be used indifferently, but omniauth_kerberos
# does not normalize them as of version 0.3.0, so add the default
# realm ourselves if appropriate
if provider == 'kerberos' && ee_uid.present?
ee_uid += "@#{kerberos_default_realm}" unless ee_uid.include?('@')
end end
@ee_uid = ee_uid
end end
end end
end end
......
...@@ -44,9 +44,9 @@ module Gitlab ...@@ -44,9 +44,9 @@ module Gitlab
end end
def abort_if_no_geo_config! def abort_if_no_geo_config!
@geo_config_exists ||= File.exist?(Rails.root.join(GEO_DATABASE_CONFIG)) @geo_config_exists ||= File.exist?(Rails.root.join(GEO_DATABASE_CONFIG)) # rubocop:disable Gitlab/ModuleWithInstanceVariables
unless @geo_config_exists unless @geo_config_exists # rubocop:disable Gitlab/ModuleWithInstanceVariables
abort("Failed to open #{GEO_DATABASE_CONFIG}. Consult the documentation on how to set up GitLab Geo.") abort("Failed to open #{GEO_DATABASE_CONFIG}. Consult the documentation on how to set up GitLab Geo.")
end end
end end
......
...@@ -50,11 +50,11 @@ module StdoutReporterWithScenarioLocation ...@@ -50,11 +50,11 @@ module StdoutReporterWithScenarioLocation
# Override the standard reporter to show filename and line number next to each # Override the standard reporter to show filename and line number next to each
# scenario for easy, focused re-runs # scenario for easy, focused re-runs
def before_scenario_run(scenario, step_definitions = nil) def before_scenario_run(scenario, step_definitions = nil)
@max_step_name_length = scenario.steps.map(&:name).map(&:length).max if scenario.steps.any? @max_step_name_length = scenario.steps.map(&:name).map(&:length).max if scenario.steps.any? # rubocop:disable Gitlab/ModuleWithInstanceVariables
name = scenario.name name = scenario.name
# This number has no significance, it's just to line things up # This number has no significance, it's just to line things up
max_length = @max_step_name_length + 19 max_length = @max_step_name_length + 19 # rubocop:disable Gitlab/ModuleWithInstanceVariables
out.puts "\n #{'Scenario:'.green} #{name.light_green.ljust(max_length)}" \ out.puts "\n #{'Scenario:'.green} #{name.light_green.ljust(max_length)}" \
" # #{scenario.feature.filename}:#{scenario.line}" " # #{scenario.feature.filename}:#{scenario.line}"
end end
......
...@@ -3,6 +3,7 @@ module API ...@@ -3,6 +3,7 @@ module API
prepend EE::API::Helpers prepend EE::API::Helpers
include Gitlab::Utils include Gitlab::Utils
include Gitlab::Utils::StrongMemoize
include Helpers::Pagination include Helpers::Pagination
SUDO_HEADER = "HTTP_SUDO".freeze SUDO_HEADER = "HTTP_SUDO".freeze
...@@ -34,6 +35,11 @@ module API ...@@ -34,6 +35,11 @@ module API
end end
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
# We can't rewrite this with StrongMemoize because `sudo!` would
# actually write to `@current_user`, and `sudo?` would immediately
# call `current_user` again which reads from `@current_user`.
# We should rewrite this in a way that using StrongMemoize is possible
def current_user def current_user
return @current_user if defined?(@current_user) return @current_user if defined?(@current_user)
...@@ -47,6 +53,7 @@ module API ...@@ -47,6 +53,7 @@ module API
@current_user @current_user
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def sudo? def sudo?
initial_current_user != current_user initial_current_user != current_user
...@@ -432,7 +439,7 @@ module API ...@@ -432,7 +439,7 @@ module API
end end
def job_token_authentication? def job_token_authentication?
initial_current_user && @job_token_authentication initial_current_user && @job_token_authentication # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def warden def warden
...@@ -450,10 +457,10 @@ module API ...@@ -450,10 +457,10 @@ module API
end end
def initial_current_user def initial_current_user
return @initial_current_user if defined?(@initial_current_user) return @initial_current_user if defined?(@initial_current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
begin begin
@initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user! } @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user! } # rubocop:disable Gitlab/ModuleWithInstanceVariables
rescue Gitlab::Auth::UnauthorizedError rescue Gitlab::Auth::UnauthorizedError
unauthorized! unauthorized!
end end
...@@ -477,7 +484,7 @@ module API ...@@ -477,7 +484,7 @@ module API
sudoed_user = find_user(sudo_identifier) sudoed_user = find_user(sudo_identifier)
not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user
@current_user = sudoed_user @current_user = sudoed_user # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def sudo_identifier def sudo_identifier
......
...@@ -6,18 +6,16 @@ module API ...@@ -6,18 +6,16 @@ module API
'git-upload-pack' => [:ssh_upload_pack, Gitlab::GitalyClient::MigrationStatus::OPT_OUT] 'git-upload-pack' => [:ssh_upload_pack, Gitlab::GitalyClient::MigrationStatus::OPT_OUT]
}.freeze }.freeze
attr_reader :redirected_path
def wiki? def wiki?
set_project unless defined?(@wiki) set_project unless defined?(@wiki) # rubocop:disable Gitlab/ModuleWithInstanceVariables
@wiki @wiki # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def project def project
set_project unless defined?(@project) set_project unless defined?(@project) # rubocop:disable Gitlab/ModuleWithInstanceVariables
@project @project # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
def redirected_path
@redirected_path
end end
def ssh_authentication_abilities def ssh_authentication_abilities
...@@ -69,6 +67,7 @@ module API ...@@ -69,6 +67,7 @@ module API
private private
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def set_project def set_project
if params[:gl_repository] if params[:gl_repository]
@project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository]) @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository])
...@@ -77,6 +76,7 @@ module API ...@@ -77,6 +76,7 @@ module API
@project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project]) @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project])
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
# Project id to pass between components that don't share/don't have # Project id to pass between components that don't share/don't have
# access to the same filesystem mounts # access to the same filesystem mounts
......
...@@ -3,7 +3,7 @@ module EE ...@@ -3,7 +3,7 @@ module EE
module Changes module Changes
def audit_changes(column, options = {}) def audit_changes(column, options = {})
column = options[:column] || column column = options[:column] || column
@model = options[:model] @model = options[:model] # rubocop:disable Gitlab/ModuleWithInstanceVariables
return unless changed?(column) return unless changed?(column)
...@@ -39,7 +39,7 @@ module EE ...@@ -39,7 +39,7 @@ module EE
end end
def audit_event(options) def audit_event(options)
::AuditEventService.new(@current_user, model, options) ::AuditEventService.new(@current_user, model, options) # rubocop:disable Gitlab/ModuleWithInstanceVariables
.for_changes.security_event .for_changes.security_event
end end
end end
......
...@@ -40,7 +40,7 @@ module ExtractsPath ...@@ -40,7 +40,7 @@ module ExtractsPath
def extract_ref(id) def extract_ref(id)
pair = ['', ''] pair = ['', '']
return pair unless @project return pair unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables
if id =~ /^(\h{40})(.+)/ if id =~ /^(\h{40})(.+)/
# If the ref appears to be a SHA, we're done, just split the string # If the ref appears to be a SHA, we're done, just split the string
...@@ -104,6 +104,7 @@ module ExtractsPath ...@@ -104,6 +104,7 @@ module ExtractsPath
# #
# Automatically renders `not_found!` if a valid tree path could not be # Automatically renders `not_found!` if a valid tree path could not be
# resolved (e.g., when a user inserts an invalid path or ref). # resolved (e.g., when a user inserts an invalid path or ref).
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def assign_ref_vars def assign_ref_vars
# assign allowed options # assign allowed options
allowed_options = ["filter_ref"] allowed_options = ["filter_ref"]
...@@ -131,9 +132,10 @@ module ExtractsPath ...@@ -131,9 +132,10 @@ module ExtractsPath
rescue RuntimeError, NoMethodError, InvalidPathError rescue RuntimeError, NoMethodError, InvalidPathError
render_404 render_404
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def tree def tree
@tree ||= @repo.tree(@commit.id, @path) @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
private private
...@@ -146,8 +148,8 @@ module ExtractsPath ...@@ -146,8 +148,8 @@ module ExtractsPath
end end
def ref_names def ref_names
return [] unless @project return [] unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables
@ref_names ||= @project.repository.ref_names @ref_names ||= @project.repository.ref_names # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
...@@ -57,7 +57,7 @@ module Gitlab ...@@ -57,7 +57,7 @@ module Gitlab
job = ::Ci::Build.find_by(token: token) job = ::Ci::Build.find_by(token: token)
raise UnauthorizedError unless job raise UnauthorizedError unless job
@job_token_authentication = true @job_token_authentication = true # rubocop:disable Gitlab/ModuleWithInstanceVariables
job.user job.user
end end
......
...@@ -45,11 +45,13 @@ module Gitlab ...@@ -45,11 +45,13 @@ module Gitlab
klass.prepend(extension) klass.prepend(extension)
end end
attr_accessor :request_cache_key_block
def request_cache_key(&block) def request_cache_key(&block)
if block_given? if block_given?
@request_cache_key = block self.request_cache_key_block = block
else else
@request_cache_key request_cache_key_block
end end
end end
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
query query
.group("DATE(#{::Ci::Pipeline.table_name}.created_at)") .group("DATE(#{::Ci::Pipeline.table_name}.created_at)")
.count(:created_at) .count(:created_at)
.transform_keys { |date| date.strftime(@format) } .transform_keys { |date| date.strftime(@format) } # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
def interval_step def interval_step
......
...@@ -29,15 +29,15 @@ module Gitlab ...@@ -29,15 +29,15 @@ module Gitlab
self.class.nodes.each do |key, factory| self.class.nodes.each do |key, factory|
factory factory
.value(@config[key]) .value(config[key])
.with(key: key, parent: self) .with(key: key, parent: self)
@entries[key] = factory.create! entries[key] = factory.create!
end end
yield if block_given? yield if block_given?
@entries.each_value do |entry| entries.each_value do |entry|
entry.compose!(deps) entry.compose!(deps)
end end
end end
...@@ -59,13 +59,13 @@ module Gitlab ...@@ -59,13 +59,13 @@ module Gitlab
def helpers(*nodes) def helpers(*nodes)
nodes.each do |symbol| nodes.each do |symbol|
define_method("#{symbol}_defined?") do define_method("#{symbol}_defined?") do
@entries[symbol]&.specified? entries[symbol]&.specified?
end end
define_method("#{symbol}_value") do define_method("#{symbol}_value") do
return unless @entries[symbol] && @entries[symbol].valid? return unless entries[symbol] && entries[symbol].valid?
@entries[symbol].value entries[symbol].value
end end
end end
end end
......
...@@ -90,6 +90,12 @@ module Gitlab ...@@ -90,6 +90,12 @@ module Gitlab
def self.aspects def self.aspects
@aspects ||= [] @aspects ||= []
end end
private
def entries
@entries
end
end end
end end
end end
......
...@@ -13,7 +13,7 @@ module Gitlab ...@@ -13,7 +13,7 @@ module Gitlab
end end
def errors def errors
@validator.messages + descendants.flat_map(&:errors) @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
class_methods do class_methods do
......
...@@ -53,7 +53,7 @@ module Gitlab ...@@ -53,7 +53,7 @@ module Gitlab
end end
def in_memory_application_settings def in_memory_application_settings
@in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Gitlab/ModuleWithInstanceVariables
rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError
# In case migrations the application_settings table is not created yet, # In case migrations the application_settings table is not created yet,
# we fallback to a simple OpenStruct # we fallback to a simple OpenStruct
......
...@@ -56,7 +56,9 @@ module Gitlab ...@@ -56,7 +56,9 @@ module Gitlab
end end
def allowed_ids def allowed_ids
nil @allowed_ids ||= allowed_ids_finder_class
.new(@options[:current_user], project_id: @project.id)
.execute.where(id: event_result_ids).pluck(:id)
end end
def event_result_ids def event_result_ids
......
...@@ -14,9 +14,9 @@ module Gitlab ...@@ -14,9 +14,9 @@ module Gitlab
def stage_query def stage_query
query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])) query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id]))
.join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])) .join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id]))
.where(issue_table[:project_id].eq(@project.id)) .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Gitlab/ModuleWithInstanceVariables
.where(issue_table[:deleted_at].eq(nil)) .where(issue_table[:deleted_at].eq(nil))
.where(issue_table[:created_at].gteq(@options[:from])) .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables
# Load merge_requests # Load merge_requests
query = query.join(mr_table, Arel::Nodes::OuterJoin) query = query.join(mr_table, Arel::Nodes::OuterJoin)
......
module Gitlab module Gitlab
module CycleAnalytics module CycleAnalytics
class CodeEventFetcher < BaseEventFetcher class CodeEventFetcher < BaseEventFetcher
include MergeRequestAllowed
def initialize(*args) def initialize(*args)
@projections = [mr_table[:title], @projections = [mr_table[:title],
mr_table[:iid], mr_table[:iid],
...@@ -20,6 +18,10 @@ module Gitlab ...@@ -20,6 +18,10 @@ module Gitlab
def serialize(event) def serialize(event)
AnalyticsMergeRequestSerializer.new(project: @project).represent(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event)
end end
def allowed_ids_finder_class
MergeRequestsFinder
end
end end
end end
end end
module Gitlab
module CycleAnalytics
module IssueAllowed
def allowed_ids
@allowed_ids ||= IssuesFinder.new(@options[:current_user], project_id: @project.id).execute.where(id: event_result_ids).pluck(:id)
end
end
end
end
module Gitlab module Gitlab
module CycleAnalytics module CycleAnalytics
class IssueEventFetcher < BaseEventFetcher class IssueEventFetcher < BaseEventFetcher
include IssueAllowed
def initialize(*args) def initialize(*args)
@projections = [issue_table[:title], @projections = [issue_table[:title],
issue_table[:iid], issue_table[:iid],
...@@ -18,6 +16,10 @@ module Gitlab ...@@ -18,6 +16,10 @@ module Gitlab
def serialize(event) def serialize(event)
AnalyticsIssueSerializer.new(project: @project).represent(event) AnalyticsIssueSerializer.new(project: @project).represent(event)
end end
def allowed_ids_finder_class
IssuesFinder
end
end end
end end
end end
module Gitlab
module CycleAnalytics
module MergeRequestAllowed
def allowed_ids
@allowed_ids ||= MergeRequestsFinder.new(@options[:current_user], project_id: @project.id).execute.where(id: event_result_ids).pluck(:id)
end
end
end
end
...@@ -18,6 +18,10 @@ module Gitlab ...@@ -18,6 +18,10 @@ module Gitlab
private private
def allowed_ids
nil
end
def merge_request_diff_commits def merge_request_diff_commits
@merge_request_diff_commits ||= @merge_request_diff_commits ||=
MergeRequestDiffCommit MergeRequestDiffCommit
......
...@@ -2,7 +2,9 @@ module Gitlab ...@@ -2,7 +2,9 @@ module Gitlab
module CycleAnalytics module CycleAnalytics
module ProductionHelper module ProductionHelper
def stage_query def stage_query
super.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@options[:from])) super
.where(mr_metrics_table[:first_deployed_to_production_at]
.gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
end end
......
module Gitlab module Gitlab
module CycleAnalytics module CycleAnalytics
class ReviewEventFetcher < BaseEventFetcher class ReviewEventFetcher < BaseEventFetcher
include MergeRequestAllowed
def initialize(*args) def initialize(*args)
@projections = [mr_table[:title], @projections = [mr_table[:title],
mr_table[:iid], mr_table[:iid],
...@@ -14,9 +12,15 @@ module Gitlab ...@@ -14,9 +12,15 @@ module Gitlab
super(*args) super(*args)
end end
private
def serialize(event) def serialize(event)
AnalyticsMergeRequestSerializer.new(project: @project).represent(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event)
end end
def allowed_ids_finder_class
MergeRequestsFinder
end
end end
end end
end end
...@@ -22,6 +22,10 @@ module Gitlab ...@@ -22,6 +22,10 @@ module Gitlab
private private
def allowed_ids
nil
end
def serialize(event) def serialize(event)
AnalyticsBuildSerializer.new.represent(event['build']) AnalyticsBuildSerializer.new.represent(event['build'])
end end
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
module Routable module Routable
def full_path def full_path
if route && route.path.present? if route && route.path.present?
@full_path ||= route.path @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables
else else
update_route if persisted? update_route if persisted?
...@@ -30,7 +30,7 @@ module Gitlab ...@@ -30,7 +30,7 @@ module Gitlab
def prepare_route def prepare_route
route || build_route(source: self) route || build_route(source: self)
route.path = build_full_path route.path = build_full_path
@full_path = nil @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
end end
......
...@@ -31,8 +31,7 @@ module Gitlab ...@@ -31,8 +31,7 @@ module Gitlab
end end
def emoji_unicode_version(name) def emoji_unicode_version(name)
@emoji_unicode_versions_by_name ||= JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json'))) emoji_unicode_versions_by_name[name]
@emoji_unicode_versions_by_name[name]
end end
def normalize_emoji_name(name) def normalize_emoji_name(name)
...@@ -56,5 +55,12 @@ module Gitlab ...@@ -56,5 +55,12 @@ module Gitlab
ActionController::Base.helpers.content_tag('gl-emoji', emoji_info['moji'], title: emoji_info['description'], data: data) ActionController::Base.helpers.content_tag('gl-emoji', emoji_info['moji'], title: emoji_info['description'], data: data)
end end
private
def emoji_unicode_versions_by_name
@emoji_unicode_versions_by_name ||=
JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json')))
end
end end
end end
...@@ -16,8 +16,8 @@ module Gitlab ...@@ -16,8 +16,8 @@ module Gitlab
vars['PWD'] = path vars['PWD'] = path
options = { chdir: path } options = { chdir: path }
@cmd_output = "" cmd_output = ""
@cmd_status = 0 cmd_status = 0
Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
yield(stdin) if block_given? yield(stdin) if block_given?
stdin.close stdin.close
...@@ -25,14 +25,14 @@ module Gitlab ...@@ -25,14 +25,14 @@ module Gitlab
if lazy_block if lazy_block
return lazy_block.call(stdout.lazy) return lazy_block.call(stdout.lazy)
else else
@cmd_output << stdout.read cmd_output << stdout.read
end end
@cmd_output << stderr.read cmd_output << stderr.read
@cmd_status = wait_thr.value.exitstatus cmd_status = wait_thr.value.exitstatus
end end
[@cmd_output, @cmd_status] [cmd_output, cmd_status]
end end
def popen_with_timeout(cmd, timeout, path, vars = {}) def popen_with_timeout(cmd, timeout, path, vars = {})
......
...@@ -32,7 +32,7 @@ module Gitlab ...@@ -32,7 +32,7 @@ module Gitlab
def execute(cmd) def execute(cmd)
output, status = Gitlab::Popen.popen(cmd) output, status = Gitlab::Popen.popen(cmd)
@shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Gitlab/ModuleWithInstanceVariables
status.zero? status.zero?
end end
......
...@@ -156,6 +156,7 @@ module Gitlab ...@@ -156,6 +156,7 @@ module Gitlab
# When enabled this should be set before being used as the usual pattern # When enabled this should be set before being used as the usual pattern
# "@foo ||= bar" is _not_ thread-safe. # "@foo ||= bar" is _not_ thread-safe.
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def pool def pool
if influx_metrics_enabled? if influx_metrics_enabled?
if @pool.nil? if @pool.nil?
...@@ -172,6 +173,7 @@ module Gitlab ...@@ -172,6 +173,7 @@ module Gitlab
@pool @pool
end end
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
end end
end end
...@@ -4,6 +4,7 @@ module Gitlab ...@@ -4,6 +4,7 @@ module Gitlab
module Metrics module Metrics
module Prometheus module Prometheus
include Gitlab::CurrentSettings include Gitlab::CurrentSettings
include Gitlab::Utils::StrongMemoize
REGISTRY_MUTEX = Mutex.new REGISTRY_MUTEX = Mutex.new
PROVIDER_MUTEX = Mutex.new PROVIDER_MUTEX = Mutex.new
...@@ -17,16 +18,18 @@ module Gitlab ...@@ -17,16 +18,18 @@ module Gitlab
end end
def prometheus_metrics_enabled? def prometheus_metrics_enabled?
return @prometheus_metrics_enabled if defined?(@prometheus_metrics_enabled) strong_memoize(:prometheus_metrics_enabled) do
prometheus_metrics_enabled_unmemoized
@prometheus_metrics_enabled = prometheus_metrics_enabled_unmemoized end
end end
def registry def registry
return @registry if @registry strong_memoize(:registry) do
REGISTRY_MUTEX.synchronize do
REGISTRY_MUTEX.synchronize do strong_memoize(:registry) do
@registry ||= ::Prometheus::Client.registry ::Prometheus::Client.registry
end
end
end end
end end
......
...@@ -11,37 +11,41 @@ module Gitlab ...@@ -11,37 +11,41 @@ module Gitlab
end end
def project def project
@resource.project resource.project
end end
def author def author
@resource.author resource.author
end end
def fields def fields
[ [
{ {
title: "Assignee", title: "Assignee",
value: @resource.assignees.any? ? @resource.assignees.first.name : "_None_", value: resource.assignees.any? ? resource.assignees.first.name : "_None_",
short: true short: true
}, },
{ {
title: "Milestone", title: "Milestone",
value: @resource.milestone ? @resource.milestone.title : "_None_", value: resource.milestone ? resource.milestone.title : "_None_",
short: true short: true
}, },
{ {
title: "Labels", title: "Labels",
value: @resource.labels.any? ? @resource.label_names.join(', ') : "_None_", value: resource.labels.any? ? resource.label_names.join(', ') : "_None_",
short: true short: true
}, },
{ {
title: "Weight", title: "Weight",
value: @resource.weight? ? @resource.weight : "_None_", value: resource.weight? ? resource.weight : "_None_",
short: true short: true
} }
] ]
end end
private
attr_reader :resource
end end
end end
end end
......
...@@ -23,6 +23,7 @@ namespace :gettext do ...@@ -23,6 +23,7 @@ namespace :gettext do
desc 'Lint all po files in `locale/' desc 'Lint all po files in `locale/'
task lint: :environment do task lint: :environment do
require 'simple_po_parser' require 'simple_po_parser'
require 'gitlab/utils'
FastGettext.silence_errors FastGettext.silence_errors
files = Dir.glob(Rails.root.join('locale/*/gitlab.po')) files = Dir.glob(Rails.root.join('locale/*/gitlab.po'))
......
require 'rainbow/ext/string' require 'rainbow/ext/string'
require 'gitlab/utils/strong_memoize'
module Gitlab module Gitlab
TaskFailedError = Class.new(StandardError) TaskFailedError = Class.new(StandardError)
TaskAbortedByUserError = Class.new(StandardError) TaskAbortedByUserError = Class.new(StandardError)
module TaskHelpers module TaskHelpers
include Gitlab::Utils::StrongMemoize
extend self extend self
# Ask if the user wants to continue # Ask if the user wants to continue
...@@ -105,16 +108,16 @@ module Gitlab ...@@ -105,16 +108,16 @@ module Gitlab
end end
def gitlab_user? def gitlab_user?
return @is_gitlab_user unless @is_gitlab_user.nil? strong_memoize(:is_gitlab_user) do
current_user = run_command(%w(whoami)).chomp
current_user = run_command(%w(whoami)).chomp current_user == gitlab_user
@is_gitlab_user = current_user == gitlab_user end
end end
def warn_user_is_not_gitlab def warn_user_is_not_gitlab
return if @warned_user_not_gitlab return if gitlab_user?
unless gitlab_user? strong_memoize(:warned_user_not_gitlab) do
current_user = run_command(%w(whoami)).chomp current_user = run_command(%w(whoami)).chomp
puts " Warning ".color(:black).background(:yellow) puts " Warning ".color(:black).background(:yellow)
...@@ -122,8 +125,6 @@ module Gitlab ...@@ -122,8 +125,6 @@ module Gitlab
puts " Things may work\/fail for the wrong reasons." puts " Things may work\/fail for the wrong reasons."
puts " For correct results you should run this as user #{gitlab_user.color(:magenta)}." puts " For correct results you should run this as user #{gitlab_user.color(:magenta)}."
puts "" puts ""
@warned_user_not_gitlab = true
end end
end end
......
...@@ -6,13 +6,15 @@ module QA ...@@ -6,13 +6,15 @@ module QA
module Scenario module Scenario
extend self extend self
attr_reader :attributes def attributes
@attributes ||= {}
end
def define(attribute, value) def define(attribute, value)
(@attributes ||= {}).store(attribute.to_sym, value) attributes.store(attribute.to_sym, value)
define_singleton_method(attribute) do define_singleton_method(attribute) do
@attributes[attribute.to_sym].tap do |value| attributes[attribute.to_sym].tap do |value|
if value.to_s.empty? if value.to_s.empty?
raise ArgumentError, "Empty `#{attribute}` attribute!" raise ArgumentError, "Empty `#{attribute}` attribute!"
end end
......
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
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