From 9ae92b8caa6c11d8860f86b7d6378062215d1b72 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 12 Jul 2017 02:29:33 +0800 Subject: Add cop to make sure we don't use ivar in a module --- lib/after_commit_queue.rb | 1 + lib/api/api_guard.rb | 1 + lib/api/helpers.rb | 1 + lib/api/helpers/internal_helpers.rb | 2 ++ lib/api/helpers/runner.rb | 1 + lib/extracts_path.rb | 1 + lib/gitlab/cache/request_cache.rb | 1 + lib/gitlab/ci/charts.rb | 3 +++ lib/gitlab/ci/config/entry/configurable.rb | 1 + lib/gitlab/ci/config/entry/validatable.rb | 1 + lib/gitlab/ci/model.rb | 1 + lib/gitlab/current_settings.rb | 1 + lib/gitlab/cycle_analytics/base_event_fetcher.rb | 6 ++++++ lib/gitlab/cycle_analytics/base_query.rb | 1 + lib/gitlab/cycle_analytics/code_event_fetcher.rb | 10 ++++++++-- lib/gitlab/cycle_analytics/issue_allowed.rb | 9 --------- lib/gitlab/cycle_analytics/issue_event_fetcher.rb | 10 ++++++++-- lib/gitlab/cycle_analytics/merge_request_allowed.rb | 9 --------- lib/gitlab/cycle_analytics/production_helper.rb | 1 + lib/gitlab/cycle_analytics/review_event_fetcher.rb | 12 ++++++++++-- .../rename_reserved_paths_migration/v1/migration_classes.rb | 1 + lib/gitlab/email/handler/reply_processing.rb | 1 + lib/gitlab/emoji.rb | 1 + lib/gitlab/git/popen.rb | 12 ++++++------ lib/gitlab/identifier.rb | 1 + lib/gitlab/import_export/command_line_util.rb | 1 + lib/gitlab/metrics/influx_db.rb | 1 + lib/gitlab/metrics/prometheus.rb | 1 + lib/gitlab/path_regex.rb | 1 + lib/gitlab/prometheus/additional_metrics_parser.rb | 1 + lib/gitlab/prometheus/queries/query_additional_metrics.rb | 1 + lib/gitlab/regex.rb | 1 + lib/gitlab/slash_commands/presenters/issue_base.rb | 1 + lib/gitlab/themes.rb | 1 + lib/tasks/gitlab/task_helpers.rb | 1 + 35 files changed, 69 insertions(+), 30 deletions(-) delete mode 100644 lib/gitlab/cycle_analytics/issue_allowed.rb delete mode 100644 lib/gitlab/cycle_analytics/merge_request_allowed.rb (limited to 'lib') diff --git a/lib/after_commit_queue.rb b/lib/after_commit_queue.rb index 4750a2c373a..f3fba4fe389 100644 --- a/lib/after_commit_queue.rb +++ b/lib/after_commit_queue.rb @@ -20,6 +20,7 @@ module AfterCommitQueue end end + # rubocop:disable Cop/ModuleWithInstanceVariables def _after_commit_queue @after_commit_queue ||= [] end diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index c4c0fdda665..05f55097a80 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -2,6 +2,7 @@ require 'rack/oauth2' +# rubocop:disable Cop/ModuleWithInstanceVariables module API module APIGuard extend ActiveSupport::Concern diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 00dbc2aee7a..49e659d3d27 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module API module Helpers include Gitlab::Utils diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 4c0db4d42b1..a187517a66a 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module API module Helpers module InternalHelpers @@ -57,6 +58,7 @@ module API private + # rubocop:disable Cop/ModuleWithInstanceVariables def set_project if params[:gl_repository] @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository]) diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index 282af32ca94..1b21594487d 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -21,6 +21,7 @@ module API forbidden! unless current_runner end + # rubocop:disable Cop/ModuleWithInstanceVariables def current_runner @runner ||= ::Ci::Runner.find_by_token(params[:token].to_s) end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 721ed97bb6b..f3e5b1c1109 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -1,5 +1,6 @@ # Module providing methods for dealing with separating a tree-ish string and a # file path string when combined in a request parameter +# rubocop:disable Cop/ModuleWithInstanceVariables module ExtractsPath # Raised when given an invalid file path InvalidPathError = Class.new(StandardError) diff --git a/lib/gitlab/cache/request_cache.rb b/lib/gitlab/cache/request_cache.rb index 754a45c3257..65e3e672894 100644 --- a/lib/gitlab/cache/request_cache.rb +++ b/lib/gitlab/cache/request_cache.rb @@ -45,6 +45,7 @@ module Gitlab klass.prepend(extension) end + # rubocop:disable Cop/ModuleWithInstanceVariables def request_cache_key(&block) if block_given? @request_cache_key = block diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index 7df7b542d91..a7040a8fa03 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -2,6 +2,7 @@ module Gitlab module Ci module Charts module DailyInterval + # rubocop:disable Cop/ModuleWithInstanceVariables def grouped_count(query) query .group("DATE(#{::Ci::Pipeline.table_name}.created_at)") @@ -9,6 +10,7 @@ module Gitlab .transform_keys { |date| date.strftime(@format) } end + # rubocop:disable Cop/ModuleWithInstanceVariables def interval_step @interval_step ||= 1.day end @@ -28,6 +30,7 @@ module Gitlab end end + # rubocop:disable Cop/ModuleWithInstanceVariables def interval_step @interval_step ||= 1.month end diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index 68b6742385a..e34f4c9e101 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -13,6 +13,7 @@ module Gitlab # script: ... # artifacts: ... # + # rubocop:disable Cop/ModuleWithInstanceVariables module Configurable extend ActiveSupport::Concern diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb index 5ced778d311..524d349c094 100644 --- a/lib/gitlab/ci/config/entry/validatable.rb +++ b/lib/gitlab/ci/config/entry/validatable.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Ci class Config diff --git a/lib/gitlab/ci/model.rb b/lib/gitlab/ci/model.rb index 3994a50772b..213301d245e 100644 --- a/lib/gitlab/ci/model.rb +++ b/lib/gitlab/ci/model.rb @@ -5,6 +5,7 @@ module Gitlab "ci_" end + # rubocop:disable Cop/ModuleWithInstanceVariables def model_name @model_name ||= ActiveModel::Name.new(self, nil, self.name.split("::").last) end diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 642f0944354..4e0dadcc3c7 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -52,6 +52,7 @@ module Gitlab ::ApplicationSetting.create_from_defaults || in_memory_application_settings end + # rubocop:disable Cop/ModuleWithInstanceVariables def in_memory_application_settings @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index ab115afcaa5..bec201f309c 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -59,6 +59,12 @@ module Gitlab nil end + def load_allowed_ids + allowed_ids_finder_class + .new(@options[:current_user], project_id: @project.id) + .execute.where(id: event_result_ids).pluck(:id) + end + def event_result_ids event_result.map { |event| event['id'] } end diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index 58729d3ced8..3f6fb227a67 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -7,6 +7,7 @@ module Gitlab private + # rubocop:disable Cop/ModuleWithInstanceVariables def base_query @base_query ||= stage_query end diff --git a/lib/gitlab/cycle_analytics/code_event_fetcher.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb index d5bf6149749..39df80352b6 100644 --- a/lib/gitlab/cycle_analytics/code_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics class CodeEventFetcher < BaseEventFetcher - include MergeRequestAllowed - def initialize(*args) @projections = [mr_table[:title], mr_table[:iid], @@ -20,6 +18,14 @@ module Gitlab def serialize(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event) end + + def allowed_ids + load_allowed_ids + end + + def allowed_ids_finder_class + MergeRequestsFinder + end end end end diff --git a/lib/gitlab/cycle_analytics/issue_allowed.rb b/lib/gitlab/cycle_analytics/issue_allowed.rb deleted file mode 100644 index a7652a70641..00000000000 --- a/lib/gitlab/cycle_analytics/issue_allowed.rb +++ /dev/null @@ -1,9 +0,0 @@ -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 diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb index 3df9cbdcfce..cc79e2dfe88 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics class IssueEventFetcher < BaseEventFetcher - include IssueAllowed - def initialize(*args) @projections = [issue_table[:title], issue_table[:iid], @@ -18,6 +16,14 @@ module Gitlab def serialize(event) AnalyticsIssueSerializer.new(project: @project).represent(event) end + + def allowed_ids + load_allowed_ids + end + + def allowed_ids_finder_class + IssuesFinder + end end end end diff --git a/lib/gitlab/cycle_analytics/merge_request_allowed.rb b/lib/gitlab/cycle_analytics/merge_request_allowed.rb deleted file mode 100644 index 28f6db44759..00000000000 --- a/lib/gitlab/cycle_analytics/merge_request_allowed.rb +++ /dev/null @@ -1,9 +0,0 @@ -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 diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb index d693443bfa4..cd7ee39d9ca 100644 --- a/lib/gitlab/cycle_analytics/production_helper.rb +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module CycleAnalytics module ProductionHelper diff --git a/lib/gitlab/cycle_analytics/review_event_fetcher.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb index 4c7b3f4467f..5a7f1eb00b3 100644 --- a/lib/gitlab/cycle_analytics/review_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics class ReviewEventFetcher < BaseEventFetcher - include MergeRequestAllowed - def initialize(*args) @projections = [mr_table[:title], mr_table[:iid], @@ -14,9 +12,19 @@ module Gitlab super(*args) end + private + def serialize(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event) end + + def allowed_ids + load_allowed_ids + end + + def allowed_ids_finder_class + MergeRequestsFinder + end end end end diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb index 5481024db8e..6959fa74293 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb @@ -3,6 +3,7 @@ module Gitlab module RenameReservedPathsMigration module V1 module MigrationClasses + # rubocop:disable Cop/ModuleWithInstanceVariables module Routable def full_path if route && route.path.present? diff --git a/lib/gitlab/email/handler/reply_processing.rb b/lib/gitlab/email/handler/reply_processing.rb index 32c5caf93e8..f077d64d75e 100644 --- a/lib/gitlab/email/handler/reply_processing.rb +++ b/lib/gitlab/email/handler/reply_processing.rb @@ -12,6 +12,7 @@ module Gitlab raise NotImplementedError end + # rubocop:disable Cop/ModuleWithInstanceVariables def message @message ||= process_message end diff --git a/lib/gitlab/emoji.rb b/lib/gitlab/emoji.rb index e3e36b35ce9..c8689d85c0c 100644 --- a/lib/gitlab/emoji.rb +++ b/lib/gitlab/emoji.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Emoji extend self diff --git a/lib/gitlab/git/popen.rb b/lib/gitlab/git/popen.rb index 25fa62ce4bd..10c15b316f5 100644 --- a/lib/gitlab/git/popen.rb +++ b/lib/gitlab/git/popen.rb @@ -13,15 +13,15 @@ module Gitlab vars = { "PWD" => path } options = { chdir: path } - @cmd_output = "" - @cmd_status = 0 + cmd_output = "" + cmd_status = 0 Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| - @cmd_output << stdout.read - @cmd_output << stderr.read - @cmd_status = wait_thr.value.exitstatus + cmd_output << stdout.read + cmd_output << stderr.read + cmd_status = wait_thr.value.exitstatus end - [@cmd_output, @cmd_status] + [cmd_output, cmd_status] end end end diff --git a/lib/gitlab/identifier.rb b/lib/gitlab/identifier.rb index 94678b6ec40..54fd9d15e7b 100644 --- a/lib/gitlab/identifier.rb +++ b/lib/gitlab/identifier.rb @@ -52,6 +52,7 @@ module Gitlab end end + # rubocop:disable Cop/ModuleWithInstanceVariables def identification_cache @identification_cache ||= { email: {}, diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index 90942774a2e..5ac57c5775a 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -30,6 +30,7 @@ module Gitlab execute(%W(tar -#{options} #{archive} -C #{dir})) end + # rubocop:disable Cop/ModuleWithInstanceVariables def execute(cmd) output, status = Gitlab::Popen.popen(cmd) @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 7b06bb953aa..4d0f79ef163 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Metrics module InfluxDb diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb index 460dab47276..476aad2f4dd 100644 --- a/lib/gitlab/metrics/prometheus.rb +++ b/lib/gitlab/metrics/prometheus.rb @@ -1,5 +1,6 @@ require 'prometheus/client' +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Metrics module Prometheus diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 7c02c9c5c48..6df9d60721e 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module PathRegex extend self diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index cb95daf2260..37112ca3cdb 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -26,6 +26,7 @@ module Gitlab load_yaml_file&.map(&:deep_symbolize_keys).freeze end + # rubocop:disable Cop/ModuleWithInstanceVariables def load_yaml_file @loaded_yaml_file ||= YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) end diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index 7ac6162b54d..6e377a24e57 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -56,6 +56,7 @@ module Gitlab query end + # rubocop:disable Cop/ModuleWithInstanceVariables def available_metrics @available_metrics ||= client_label_values || [] end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 58f6245579a..fdd5c86c698 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Regex extend self diff --git a/lib/gitlab/slash_commands/presenters/issue_base.rb b/lib/gitlab/slash_commands/presenters/issue_base.rb index 341f2aabdd0..2cec307867c 100644 --- a/lib/gitlab/slash_commands/presenters/issue_base.rb +++ b/lib/gitlab/slash_commands/presenters/issue_base.rb @@ -1,3 +1,4 @@ +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module SlashCommands module Presenters diff --git a/lib/gitlab/themes.rb b/lib/gitlab/themes.rb index d43eff5ba4a..7805f89831b 100644 --- a/lib/gitlab/themes.rb +++ b/lib/gitlab/themes.rb @@ -72,6 +72,7 @@ module Gitlab private + # rubocop:disable Cop/ModuleWithInstanceVariables def default_id @default_id ||= begin id = Gitlab.config.gitlab.default_theme.to_i diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 8a63f486fa3..24b65630b87 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -1,5 +1,6 @@ require 'rainbow/ext/string' +# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab TaskFailedError = Class.new(StandardError) TaskAbortedByUserError = Class.new(StandardError) -- cgit v1.2.1 From 6a4ee9aa7140862075cafae1ddebd133eec52b5b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 19 Sep 2017 01:25:23 +0800 Subject: Allow simple ivar ||= form. Update accordingly --- lib/after_commit_queue.rb | 1 - lib/api/api_guard.rb | 8 +++----- lib/api/helpers.rb | 4 +++- lib/api/helpers/internal_helpers.rb | 9 ++++----- lib/api/helpers/runner.rb | 1 - lib/extracts_path.rb | 5 ++++- lib/gitlab/ci/charts.rb | 2 -- lib/gitlab/ci/config/entry/configurable.rb | 2 +- lib/gitlab/ci/config/entry/validatable.rb | 2 +- lib/gitlab/ci/model.rb | 1 - lib/gitlab/cycle_analytics/base_query.rb | 2 +- lib/gitlab/cycle_analytics/production_helper.rb | 2 +- lib/gitlab/email/handler/reply_processing.rb | 1 - lib/gitlab/emoji.rb | 11 ++++++++--- lib/gitlab/identifier.rb | 1 - lib/gitlab/metrics/influx_db.rb | 2 +- lib/gitlab/metrics/prometheus.rb | 2 +- lib/gitlab/prometheus/additional_metrics_parser.rb | 1 - lib/gitlab/prometheus/queries/query_additional_metrics.rb | 1 - lib/gitlab/regex.rb | 1 - lib/gitlab/slash_commands/presenters/issue_base.rb | 4 +++- lib/gitlab/themes.rb | 1 - lib/tasks/gitlab/task_helpers.rb | 3 ++- 23 files changed, 33 insertions(+), 34 deletions(-) (limited to 'lib') diff --git a/lib/after_commit_queue.rb b/lib/after_commit_queue.rb index f3fba4fe389..4750a2c373a 100644 --- a/lib/after_commit_queue.rb +++ b/lib/after_commit_queue.rb @@ -20,7 +20,6 @@ module AfterCommitQueue end end - # rubocop:disable Cop/ModuleWithInstanceVariables def _after_commit_queue @after_commit_queue ||= [] end diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 05f55097a80..9933439c43b 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -2,7 +2,6 @@ require 'rack/oauth2' -# rubocop:disable Cop/ModuleWithInstanceVariables module API module APIGuard extend ActiveSupport::Concern @@ -43,6 +42,8 @@ module API # Helper Methods for Grape Endpoint module HelperMethods + attr_reader :current_user + # Invokes the doorkeeper guard. # # If token is presented and valid, then it sets @current_user. @@ -61,6 +62,7 @@ module API # scopes: (optional) scopes required for this guard. # Defaults to empty array. # + # rubocop:disable Cop/ModuleWithInstanceVariables def doorkeeper_guard(scopes: []) access_token = find_access_token return nil unless access_token @@ -88,10 +90,6 @@ module API find_user_by_authentication_token(token_string) || find_user_by_personal_access_token(token_string, scopes) end - def current_user - @current_user - end - private def find_user_by_authentication_token(token_string) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 49e659d3d27..abbe2e9ba3e 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module API module Helpers include Gitlab::Utils @@ -33,6 +32,7 @@ module API end end + # rubocop:disable Cop/ModuleWithInstanceVariables def current_user return @current_user if defined?(@current_user) @@ -396,6 +396,7 @@ module API warden.try(:authenticate) if verified_request? end + # rubocop:disable Cop/ModuleWithInstanceVariables def initial_current_user return @initial_current_user if defined?(@initial_current_user) Gitlab::Auth::UniqueIpsLimiter.limit_user! do @@ -411,6 +412,7 @@ module API end end + # rubocop:disable Cop/ModuleWithInstanceVariables def sudo! return unless sudo_identifier return unless initial_current_user diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index a187517a66a..6bb85dd2619 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module API module Helpers module InternalHelpers @@ -7,20 +6,20 @@ module API 'git-upload-pack' => :ssh_upload_pack }.freeze + attr_reader :redirected_path + + # rubocop:disable Cop/ModuleWithInstanceVariables def wiki? set_project unless defined?(@wiki) @wiki end + # rubocop:disable Cop/ModuleWithInstanceVariables def project set_project unless defined?(@project) @project end - def redirected_path - @redirected_path - end - def ssh_authentication_abilities [ :read_project, diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index 1b21594487d..282af32ca94 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -21,7 +21,6 @@ module API forbidden! unless current_runner end - # rubocop:disable Cop/ModuleWithInstanceVariables def current_runner @runner ||= ::Ci::Runner.find_by_token(params[:token].to_s) end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index f3e5b1c1109..9e01eed06f3 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -1,6 +1,5 @@ # Module providing methods for dealing with separating a tree-ish string and a # file path string when combined in a request parameter -# rubocop:disable Cop/ModuleWithInstanceVariables module ExtractsPath # Raised when given an invalid file path InvalidPathError = Class.new(StandardError) @@ -38,6 +37,7 @@ module ExtractsPath # # Returns an Array where the first value is the tree-ish and the second is the # path + # rubocop:disable Cop/ModuleWithInstanceVariables def extract_ref(id) pair = ['', ''] @@ -105,6 +105,7 @@ module ExtractsPath # # Automatically renders `not_found!` if a valid tree path could not be # resolved (e.g., when a user inserts an invalid path or ref). + # rubocop:disable Cop/ModuleWithInstanceVariables def assign_ref_vars # assign allowed options allowed_options = ["filter_ref"] @@ -133,6 +134,7 @@ module ExtractsPath render_404 end + # rubocop:disable Cop/ModuleWithInstanceVariables def tree @tree ||= @repo.tree(@commit.id, @path) end @@ -146,6 +148,7 @@ module ExtractsPath id end + # rubocop:disable Cop/ModuleWithInstanceVariables def ref_names return [] unless @project diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index a7040a8fa03..fe2fd08a67a 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -10,7 +10,6 @@ module Gitlab .transform_keys { |date| date.strftime(@format) } end - # rubocop:disable Cop/ModuleWithInstanceVariables def interval_step @interval_step ||= 1.day end @@ -30,7 +29,6 @@ module Gitlab end end - # rubocop:disable Cop/ModuleWithInstanceVariables def interval_step @interval_step ||= 1.month end diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index e34f4c9e101..2c96e5f65d7 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -13,7 +13,6 @@ module Gitlab # script: ... # artifacts: ... # - # rubocop:disable Cop/ModuleWithInstanceVariables module Configurable extend ActiveSupport::Concern @@ -25,6 +24,7 @@ module Gitlab end end + # rubocop:disable Cop/ModuleWithInstanceVariables def compose!(deps = nil) return unless valid? diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb index 524d349c094..1850c652c09 100644 --- a/lib/gitlab/ci/config/entry/validatable.rb +++ b/lib/gitlab/ci/config/entry/validatable.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Ci class Config @@ -13,6 +12,7 @@ module Gitlab end end + # rubocop:disable Cop/ModuleWithInstanceVariables def errors @validator.messages + descendants.flat_map(&:errors) end diff --git a/lib/gitlab/ci/model.rb b/lib/gitlab/ci/model.rb index 213301d245e..3994a50772b 100644 --- a/lib/gitlab/ci/model.rb +++ b/lib/gitlab/ci/model.rb @@ -5,7 +5,6 @@ module Gitlab "ci_" end - # rubocop:disable Cop/ModuleWithInstanceVariables def model_name @model_name ||= ActiveModel::Name.new(self, nil, self.name.split("::").last) end diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index 3f6fb227a67..52fdae84c58 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -7,11 +7,11 @@ module Gitlab private - # rubocop:disable Cop/ModuleWithInstanceVariables def base_query @base_query ||= stage_query end + # rubocop:disable Cop/ModuleWithInstanceVariables def stage_query 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])) diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb index cd7ee39d9ca..9a05c910ba0 100644 --- a/lib/gitlab/cycle_analytics/production_helper.rb +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -1,7 +1,7 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module CycleAnalytics module ProductionHelper + # rubocop:disable Cop/ModuleWithInstanceVariables def stage_query super.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@options[:from])) end diff --git a/lib/gitlab/email/handler/reply_processing.rb b/lib/gitlab/email/handler/reply_processing.rb index f077d64d75e..32c5caf93e8 100644 --- a/lib/gitlab/email/handler/reply_processing.rb +++ b/lib/gitlab/email/handler/reply_processing.rb @@ -12,7 +12,6 @@ module Gitlab raise NotImplementedError end - # rubocop:disable Cop/ModuleWithInstanceVariables def message @message ||= process_message end diff --git a/lib/gitlab/emoji.rb b/lib/gitlab/emoji.rb index c8689d85c0c..89cf659bce4 100644 --- a/lib/gitlab/emoji.rb +++ b/lib/gitlab/emoji.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Emoji extend self @@ -32,8 +31,7 @@ module Gitlab end 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 def normalize_emoji_name(name) @@ -57,5 +55,12 @@ module Gitlab ActionController::Base.helpers.content_tag('gl-emoji', emoji_info['moji'], title: emoji_info['description'], data: data) 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 diff --git a/lib/gitlab/identifier.rb b/lib/gitlab/identifier.rb index 54fd9d15e7b..94678b6ec40 100644 --- a/lib/gitlab/identifier.rb +++ b/lib/gitlab/identifier.rb @@ -52,7 +52,6 @@ module Gitlab end end - # rubocop:disable Cop/ModuleWithInstanceVariables def identification_cache @identification_cache ||= { email: {}, diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 4d0f79ef163..c4dc061eda1 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Metrics module InfluxDb @@ -150,6 +149,7 @@ module Gitlab # When enabled this should be set before being used as the usual pattern # "@foo ||= bar" is _not_ thread-safe. + # rubocop:disable Cop/ModuleWithInstanceVariables def pool if influx_metrics_enabled? if @pool.nil? diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb index 476aad2f4dd..b5f9dafccab 100644 --- a/lib/gitlab/metrics/prometheus.rb +++ b/lib/gitlab/metrics/prometheus.rb @@ -1,6 +1,5 @@ require 'prometheus/client' -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Metrics module Prometheus @@ -14,6 +13,7 @@ module Gitlab ::File.writable?(multiprocess_files_dir) end + # rubocop:disable Cop/ModuleWithInstanceVariables def prometheus_metrics_enabled? return @prometheus_metrics_enabled if defined?(@prometheus_metrics_enabled) diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index 37112ca3cdb..cb95daf2260 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -26,7 +26,6 @@ module Gitlab load_yaml_file&.map(&:deep_symbolize_keys).freeze end - # rubocop:disable Cop/ModuleWithInstanceVariables def load_yaml_file @loaded_yaml_file ||= YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) end diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index 6e377a24e57..7ac6162b54d 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -56,7 +56,6 @@ module Gitlab query end - # rubocop:disable Cop/ModuleWithInstanceVariables def available_metrics @available_metrics ||= client_label_values || [] end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index fdd5c86c698..58f6245579a 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module Regex extend self diff --git a/lib/gitlab/slash_commands/presenters/issue_base.rb b/lib/gitlab/slash_commands/presenters/issue_base.rb index 2cec307867c..5aaf3655396 100644 --- a/lib/gitlab/slash_commands/presenters/issue_base.rb +++ b/lib/gitlab/slash_commands/presenters/issue_base.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module SlashCommands module Presenters @@ -11,14 +10,17 @@ module Gitlab issuable.open? ? 'Open' : 'Closed' end + # rubocop:disable Cop/ModuleWithInstanceVariables def project @resource.project end + # rubocop:disable Cop/ModuleWithInstanceVariables def author @resource.author end + # rubocop:disable Cop/ModuleWithInstanceVariables def fields [ { diff --git a/lib/gitlab/themes.rb b/lib/gitlab/themes.rb index 7805f89831b..d43eff5ba4a 100644 --- a/lib/gitlab/themes.rb +++ b/lib/gitlab/themes.rb @@ -72,7 +72,6 @@ module Gitlab private - # rubocop:disable Cop/ModuleWithInstanceVariables def default_id @default_id ||= begin id = Gitlab.config.gitlab.default_theme.to_i diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 24b65630b87..2feacbb4a09 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -1,6 +1,5 @@ require 'rainbow/ext/string' -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab TaskFailedError = Class.new(StandardError) TaskAbortedByUserError = Class.new(StandardError) @@ -105,6 +104,7 @@ module Gitlab Gitlab.config.gitlab.user end + # rubocop:disable Cop/ModuleWithInstanceVariables def gitlab_user? return @is_gitlab_user unless @is_gitlab_user.nil? @@ -112,6 +112,7 @@ module Gitlab @is_gitlab_user = current_user == gitlab_user end + # rubocop:disable Cop/ModuleWithInstanceVariables def warn_user_is_not_gitlab return if @warned_user_not_gitlab -- cgit v1.2.1 From f8b681f6e985d49b39d399d60666b051a60a6502 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 6 Nov 2017 22:40:19 +0800 Subject: WIP --- lib/gitlab/ci/pipeline/chain/helpers.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index 02d81286f21..36ed87dbd32 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -3,17 +3,21 @@ module Gitlab module Pipeline module Chain module Helpers + # rubocop:disable Cop/ModuleWithInstanceVariables def branch_exists? return @is_branch if defined?(@is_branch) @is_branch = project.repository.branch_exists?(pipeline.ref) end + # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:disable Cop/ModuleWithInstanceVariables def tag_exists? return @is_tag if defined?(@is_tag) @is_tag = project.repository.tag_exists?(pipeline.ref) end + # rubocop:enable Cop/ModuleWithInstanceVariables def error(message) pipeline.errors.add(:base, message) -- cgit v1.2.1 From 9ac0c76b78cd04b2505924f003dd720a0f155959 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 17 Nov 2017 20:27:16 +0800 Subject: Use StrongMemoize and enable/disable cops properly --- lib/api/helpers.rb | 10 ++++++++-- lib/api/helpers/internal_helpers.rb | 11 +++++------ lib/extracts_path.rb | 12 +++++------- lib/gitlab/cache/request_cache.rb | 7 ++++--- lib/gitlab/ci/charts.rb | 3 +-- lib/gitlab/ci/config/entry/configurable.rb | 7 +++---- lib/gitlab/ci/config/entry/node.rb | 6 ++++++ lib/gitlab/ci/config/entry/validatable.rb | 3 +-- lib/gitlab/ci/pipeline/chain/helpers.rb | 18 ++++++++---------- lib/gitlab/current_settings.rb | 3 +-- lib/gitlab/cycle_analytics/base_query.rb | 5 ++--- lib/gitlab/cycle_analytics/production_helper.rb | 5 +++-- .../v1/migration_classes.rb | 5 ++--- lib/gitlab/import_export/command_line_util.rb | 3 +-- lib/gitlab/metrics/influx_db.rb | 1 + lib/gitlab/metrics/prometheus.rb | 18 ++++++++++-------- lib/gitlab/path_regex.rb | 1 - lib/gitlab/slash_commands/presenters/issue_base.rb | 17 +++++++++-------- lib/tasks/gettext.rake | 1 + lib/tasks/gitlab/task_helpers.rb | 19 +++++++++---------- 20 files changed, 80 insertions(+), 75 deletions(-) (limited to 'lib') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7f436b69091..c4f81443282 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -33,6 +33,10 @@ module API end # rubocop:disable Cop/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 return @current_user if defined?(@current_user) @@ -46,6 +50,7 @@ module API @current_user end + # rubocop:enable Cop/ModuleWithInstanceVariables def sudo? initial_current_user != current_user @@ -394,6 +399,7 @@ module API private + # rubocop:disable Cop/ModuleWithInstanceVariables def initial_current_user return @initial_current_user if defined?(@initial_current_user) @@ -403,8 +409,8 @@ module API unauthorized! end end + # rubocop:enable Cop/ModuleWithInstanceVariables - # rubocop:disable Cop/ModuleWithInstanceVariables def sudo! return unless sudo_identifier @@ -423,7 +429,7 @@ module API sudoed_user = find_user(sudo_identifier) not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user - @current_user = sudoed_user + @current_user = sudoed_user # rubocop:disable Cop/ModuleWithInstanceVariables end def sudo_identifier diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 0d57c822578..e2077d33b9f 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -8,16 +8,14 @@ module API attr_reader :redirected_path - # rubocop:disable Cop/ModuleWithInstanceVariables def wiki? - set_project unless defined?(@wiki) - @wiki + set_project unless defined?(@wiki) # rubocop:disable Cop/ModuleWithInstanceVariables + @wiki # rubocop:disable Cop/ModuleWithInstanceVariables end - # rubocop:disable Cop/ModuleWithInstanceVariables def project - set_project unless defined?(@project) - @project + set_project unless defined?(@project) # rubocop:disable Cop/ModuleWithInstanceVariables + @project # rubocop:disable Cop/ModuleWithInstanceVariables end def ssh_authentication_abilities @@ -78,6 +76,7 @@ module API @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project]) end end + # rubocop:enable Cop/ModuleWithInstanceVariables # Project id to pass between components that don't share/don't have # access to the same filesystem mounts diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 9e01eed06f3..40a65aad631 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -37,11 +37,10 @@ module ExtractsPath # # Returns an Array where the first value is the tree-ish and the second is the # path - # rubocop:disable Cop/ModuleWithInstanceVariables def extract_ref(id) pair = ['', ''] - return pair unless @project + return pair unless @project # rubocop:disable Cop/ModuleWithInstanceVariables if id =~ /^(\h{40})(.+)/ # If the ref appears to be a SHA, we're done, just split the string @@ -133,10 +132,10 @@ module ExtractsPath rescue RuntimeError, NoMethodError, InvalidPathError render_404 end + # rubocop:enable Cop/ModuleWithInstanceVariables - # rubocop:disable Cop/ModuleWithInstanceVariables def tree - @tree ||= @repo.tree(@commit.id, @path) + @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Cop/ModuleWithInstanceVariables end private @@ -148,10 +147,9 @@ module ExtractsPath id end - # rubocop:disable Cop/ModuleWithInstanceVariables def ref_names - return [] unless @project + return [] unless @project # rubocop:disable Cop/ModuleWithInstanceVariables - @ref_names ||= @project.repository.ref_names + @ref_names ||= @project.repository.ref_names # rubocop:disable Cop/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/cache/request_cache.rb b/lib/gitlab/cache/request_cache.rb index 65e3e672894..ecc85f847d4 100644 --- a/lib/gitlab/cache/request_cache.rb +++ b/lib/gitlab/cache/request_cache.rb @@ -45,12 +45,13 @@ module Gitlab klass.prepend(extension) end - # rubocop:disable Cop/ModuleWithInstanceVariables + attr_accessor :request_cache_key_block + def request_cache_key(&block) if block_given? - @request_cache_key = block + self.request_cache_key_block = block else - @request_cache_key + request_cache_key_block end end diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index fe2fd08a67a..e94166aee0c 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -2,12 +2,11 @@ module Gitlab module Ci module Charts module DailyInterval - # rubocop:disable Cop/ModuleWithInstanceVariables def grouped_count(query) query .group("DATE(#{::Ci::Pipeline.table_name}.created_at)") .count(:created_at) - .transform_keys { |date| date.strftime(@format) } + .transform_keys { |date| date.strftime(@format) } # rubocop:disable Cop/ModuleWithInstanceVariables end def interval_step diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index 2c96e5f65d7..63b803c15af 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -24,21 +24,20 @@ module Gitlab end end - # rubocop:disable Cop/ModuleWithInstanceVariables def compose!(deps = nil) return unless valid? self.class.nodes.each do |key, factory| factory - .value(@config[key]) + .value(config[key]) .with(key: key, parent: self) - @entries[key] = factory.create! + entries[key] = factory.create! end yield if block_given? - @entries.each_value do |entry| + entries.each_value do |entry| entry.compose!(deps) end end diff --git a/lib/gitlab/ci/config/entry/node.rb b/lib/gitlab/ci/config/entry/node.rb index c868943c42e..1fba0b2db0b 100644 --- a/lib/gitlab/ci/config/entry/node.rb +++ b/lib/gitlab/ci/config/entry/node.rb @@ -90,6 +90,12 @@ module Gitlab def self.aspects @aspects ||= [] end + + private + + def entries + @entries + end end end end diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb index 1850c652c09..0dc359a86c3 100644 --- a/lib/gitlab/ci/config/entry/validatable.rb +++ b/lib/gitlab/ci/config/entry/validatable.rb @@ -12,9 +12,8 @@ module Gitlab end end - # rubocop:disable Cop/ModuleWithInstanceVariables def errors - @validator.messages + descendants.flat_map(&:errors) + @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Cop/ModuleWithInstanceVariables end class_methods do diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index 36ed87dbd32..7ab7a64c7e3 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -3,21 +3,19 @@ module Gitlab module Pipeline module Chain module Helpers - # rubocop:disable Cop/ModuleWithInstanceVariables - def branch_exists? - return @is_branch if defined?(@is_branch) + include Gitlab::Utils::StrongMemoize - @is_branch = project.repository.branch_exists?(pipeline.ref) + def branch_exists? + strong_memoize(:is_branch) do + project.repository.branch_exists?(pipeline.ref) + end end - # rubocop:enable Cop/ModuleWithInstanceVariables - # rubocop:disable Cop/ModuleWithInstanceVariables def tag_exists? - return @is_tag if defined?(@is_tag) - - @is_tag = project.repository.tag_exists?(pipeline.ref) + strong_memoize(:is_tag) do + project.repository.tag_exists?(pipeline.ref) + end end - # rubocop:enable Cop/ModuleWithInstanceVariables def error(message) pipeline.errors.add(:base, message) diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 4e0dadcc3c7..dfb2cfe42b7 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -52,9 +52,8 @@ module Gitlab ::ApplicationSetting.create_from_defaults || in_memory_application_settings end - # rubocop:disable Cop/ModuleWithInstanceVariables def in_memory_application_settings - @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) + @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Cop/ModuleWithInstanceVariables rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError # In case migrations the application_settings table is not created yet, # we fallback to a simple OpenStruct diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index 52fdae84c58..05b4928c3b9 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -11,13 +11,12 @@ module Gitlab @base_query ||= stage_query end - # rubocop:disable Cop/ModuleWithInstanceVariables def stage_query 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])) - .where(issue_table[:project_id].eq(@project.id)) + .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Cop/ModuleWithInstanceVariables .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 Cop/ModuleWithInstanceVariables # Load merge_requests query = query.join(mr_table, Arel::Nodes::OuterJoin) diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb index 9a05c910ba0..ebbff1b2f33 100644 --- a/lib/gitlab/cycle_analytics/production_helper.rb +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -1,9 +1,10 @@ module Gitlab module CycleAnalytics module ProductionHelper - # rubocop:disable Cop/ModuleWithInstanceVariables 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 Cop/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb index 6959fa74293..9ae1f66a182 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb @@ -3,11 +3,10 @@ module Gitlab module RenameReservedPathsMigration module V1 module MigrationClasses - # rubocop:disable Cop/ModuleWithInstanceVariables module Routable def full_path if route && route.path.present? - @full_path ||= route.path + @full_path ||= route.path # rubocop:disable Cop/ModuleWithInstanceVariables else update_route if persisted? @@ -31,7 +30,7 @@ module Gitlab def prepare_route route || build_route(source: self) route.path = build_full_path - @full_path = nil + @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index 5ac57c5775a..19dab0037bb 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -30,10 +30,9 @@ module Gitlab execute(%W(tar -#{options} #{archive} -C #{dir})) end - # rubocop:disable Cop/ModuleWithInstanceVariables def execute(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 Cop/ModuleWithInstanceVariables status.zero? end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 3c5f9099584..fa88d41be73 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -171,6 +171,7 @@ module Gitlab @pool end end + # rubocop:enable Cop/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb index 75461b45005..b0b8e8436db 100644 --- a/lib/gitlab/metrics/prometheus.rb +++ b/lib/gitlab/metrics/prometheus.rb @@ -4,6 +4,7 @@ module Gitlab module Metrics module Prometheus include Gitlab::CurrentSettings + include Gitlab::Utils::StrongMemoize REGISTRY_MUTEX = Mutex.new PROVIDER_MUTEX = Mutex.new @@ -16,18 +17,19 @@ module Gitlab ::File.writable?(multiprocess_files_dir) end - # rubocop:disable Cop/ModuleWithInstanceVariables def prometheus_metrics_enabled? - return @prometheus_metrics_enabled if defined?(@prometheus_metrics_enabled) - - @prometheus_metrics_enabled = prometheus_metrics_enabled_unmemoized + strong_memoize(:prometheus_metrics_enabled) do + prometheus_metrics_enabled_unmemoized + end end def registry - return @registry if @registry - - REGISTRY_MUTEX.synchronize do - @registry ||= ::Prometheus::Client.registry + strong_memoize(:registry) do + REGISTRY_MUTEX.synchronize do + strong_memoize(:registry) do + ::Prometheus::Client.registry + end + end end end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 9a3817ff00a..9a91f8bf96a 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -1,4 +1,3 @@ -# rubocop:disable Cop/ModuleWithInstanceVariables module Gitlab module PathRegex extend self diff --git a/lib/gitlab/slash_commands/presenters/issue_base.rb b/lib/gitlab/slash_commands/presenters/issue_base.rb index 5aaf3655396..31c1e97efba 100644 --- a/lib/gitlab/slash_commands/presenters/issue_base.rb +++ b/lib/gitlab/slash_commands/presenters/issue_base.rb @@ -10,36 +10,37 @@ module Gitlab issuable.open? ? 'Open' : 'Closed' end - # rubocop:disable Cop/ModuleWithInstanceVariables def project - @resource.project + resource.project end - # rubocop:disable Cop/ModuleWithInstanceVariables def author - @resource.author + resource.author end - # rubocop:disable Cop/ModuleWithInstanceVariables def fields [ { title: "Assignee", - value: @resource.assignees.any? ? @resource.assignees.first.name : "_None_", + value: resource.assignees.any? ? resource.assignees.first.name : "_None_", short: true }, { title: "Milestone", - value: @resource.milestone ? @resource.milestone.title : "_None_", + value: resource.milestone ? resource.milestone.title : "_None_", short: true }, { title: "Labels", - value: @resource.labels.any? ? @resource.label_names.join(', ') : "_None_", + value: resource.labels.any? ? resource.label_names.join(', ') : "_None_", short: true } ] end + + private + + attr_reader :resource end end end diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake index 35ba729c156..247d7be7d78 100644 --- a/lib/tasks/gettext.rake +++ b/lib/tasks/gettext.rake @@ -23,6 +23,7 @@ namespace :gettext do desc 'Lint all po files in `locale/' task lint: :environment do require 'simple_po_parser' + require 'gitlab/utils' FastGettext.silence_errors files = Dir.glob(Rails.root.join('locale/*/gitlab.po')) diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 2feacbb4a09..6723662703c 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -1,10 +1,13 @@ require 'rainbow/ext/string' +require 'gitlab/utils/strong_memoize' module Gitlab TaskFailedError = Class.new(StandardError) TaskAbortedByUserError = Class.new(StandardError) module TaskHelpers + include Gitlab::Utils::StrongMemoize + extend self # Ask if the user wants to continue @@ -104,19 +107,17 @@ module Gitlab Gitlab.config.gitlab.user end - # rubocop:disable Cop/ModuleWithInstanceVariables def gitlab_user? - return @is_gitlab_user unless @is_gitlab_user.nil? - - current_user = run_command(%w(whoami)).chomp - @is_gitlab_user = current_user == gitlab_user + strong_memoize(:is_gitlab_user) do + current_user = run_command(%w(whoami)).chomp + current_user == gitlab_user + end end - # rubocop:disable Cop/ModuleWithInstanceVariables 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 puts " Warning ".color(:black).background(:yellow) @@ -124,8 +125,6 @@ module Gitlab puts " Things may work\/fail for the wrong reasons." puts " For correct results you should run this as user #{gitlab_user.color(:magenta)}." puts "" - - @warned_user_not_gitlab = true end end -- cgit v1.2.1 From 45568bed36095db0df94cf89a8a04458f56f33dc Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 21 Nov 2017 23:15:24 +0800 Subject: Updates based on feedback --- lib/gitlab/ci/config/entry/configurable.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index 63b803c15af..db47c2f6185 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -59,13 +59,13 @@ module Gitlab def helpers(*nodes) nodes.each do |symbol| define_method("#{symbol}_defined?") do - @entries[symbol]&.specified? + entries[symbol]&.specified? end 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 -- cgit v1.2.1 From 0b6d01ed775e957161e1304de44d0bd5251f4098 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 22 Nov 2017 00:15:52 +0800 Subject: Just define allowed_ids and override it with empty value if not needed for those sub-classes. --- lib/gitlab/cycle_analytics/base_event_fetcher.rb | 4 ---- lib/gitlab/cycle_analytics/code_event_fetcher.rb | 4 ---- lib/gitlab/cycle_analytics/issue_event_fetcher.rb | 4 ---- lib/gitlab/cycle_analytics/plan_event_fetcher.rb | 4 ++++ lib/gitlab/cycle_analytics/review_event_fetcher.rb | 4 ---- lib/gitlab/cycle_analytics/staging_event_fetcher.rb | 4 ++++ 6 files changed, 8 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index bec201f309c..62fc3ee3b5f 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -56,10 +56,6 @@ module Gitlab end def allowed_ids - nil - end - - def load_allowed_ids allowed_ids_finder_class .new(@options[:current_user], project_id: @project.id) .execute.where(id: event_result_ids).pluck(:id) diff --git a/lib/gitlab/cycle_analytics/code_event_fetcher.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb index 39df80352b6..06357c9b377 100644 --- a/lib/gitlab/cycle_analytics/code_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb @@ -19,10 +19,6 @@ module Gitlab AnalyticsMergeRequestSerializer.new(project: @project).represent(event) end - def allowed_ids - load_allowed_ids - end - def allowed_ids_finder_class MergeRequestsFinder end diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb index cc79e2dfe88..1754f91dccb 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -17,10 +17,6 @@ module Gitlab AnalyticsIssueSerializer.new(project: @project).represent(event) end - def allowed_ids - load_allowed_ids - end - def allowed_ids_finder_class IssuesFinder end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index 2479b4a7706..ca2742f2aae 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -19,6 +19,10 @@ module Gitlab private + def allowed_ids + nil + end + def merge_request_diff_commits @merge_request_diff_commits ||= MergeRequestDiffCommit diff --git a/lib/gitlab/cycle_analytics/review_event_fetcher.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb index 5a7f1eb00b3..dada819a2a8 100644 --- a/lib/gitlab/cycle_analytics/review_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb @@ -18,10 +18,6 @@ module Gitlab AnalyticsMergeRequestSerializer.new(project: @project).represent(event) end - def allowed_ids - load_allowed_ids - end - def allowed_ids_finder_class MergeRequestsFinder end diff --git a/lib/gitlab/cycle_analytics/staging_event_fetcher.rb b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb index 36c0260dbfe..2f014153ca5 100644 --- a/lib/gitlab/cycle_analytics/staging_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb @@ -22,6 +22,10 @@ module Gitlab private + def allowed_ids + nil + end + def serialize(event) AnalyticsBuildSerializer.new.represent(event['build']) end -- cgit v1.2.1 From 07d3d44775f6cc5b7a1b768cb4e5b7900d543815 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 22 Nov 2017 15:50:36 +0800 Subject: Move ModuleWithInstanceVariables to Gitlab namespace And use .rubocop.yml to exclude paths we don't care, rather than using the cop itself to exclude. --- lib/api/helpers.rb | 10 +++++----- lib/api/helpers/internal_helpers.rb | 12 ++++++------ lib/extracts_path.rb | 12 ++++++------ lib/gitlab/ci/charts.rb | 2 +- lib/gitlab/ci/config/entry/validatable.rb | 2 +- lib/gitlab/current_settings.rb | 2 +- lib/gitlab/cycle_analytics/base_query.rb | 4 ++-- lib/gitlab/cycle_analytics/production_helper.rb | 2 +- .../rename_reserved_paths_migration/v1/migration_classes.rb | 4 ++-- lib/gitlab/import_export/command_line_util.rb | 2 +- lib/gitlab/metrics/influx_db.rb | 4 ++-- 11 files changed, 28 insertions(+), 28 deletions(-) (limited to 'lib') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c4f81443282..fc1a3948bb4 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -32,7 +32,7 @@ module API end end - # rubocop:disable Cop/ModuleWithInstanceVariables + # 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`. @@ -50,7 +50,7 @@ module API @current_user end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def sudo? initial_current_user != current_user @@ -399,7 +399,7 @@ module API private - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def initial_current_user return @initial_current_user if defined?(@initial_current_user) @@ -409,7 +409,7 @@ module API unauthorized! end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def sudo! return unless sudo_identifier @@ -429,7 +429,7 @@ module API sudoed_user = find_user(sudo_identifier) not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user - @current_user = sudoed_user # rubocop:disable Cop/ModuleWithInstanceVariables + @current_user = sudoed_user # rubocop:disable Gitlab/ModuleWithInstanceVariables end def sudo_identifier diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index e2077d33b9f..520bf65c3b3 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -9,13 +9,13 @@ module API attr_reader :redirected_path def wiki? - set_project unless defined?(@wiki) # rubocop:disable Cop/ModuleWithInstanceVariables - @wiki # rubocop:disable Cop/ModuleWithInstanceVariables + set_project unless defined?(@wiki) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @wiki # rubocop:disable Gitlab/ModuleWithInstanceVariables end def project - set_project unless defined?(@project) # rubocop:disable Cop/ModuleWithInstanceVariables - @project # rubocop:disable Cop/ModuleWithInstanceVariables + set_project unless defined?(@project) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @project # rubocop:disable Gitlab/ModuleWithInstanceVariables end def ssh_authentication_abilities @@ -67,7 +67,7 @@ module API private - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def set_project if params[:gl_repository] @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository]) @@ -76,7 +76,7 @@ module API @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project]) end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables # Project id to pass between components that don't share/don't have # access to the same filesystem mounts diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 40a65aad631..26f699f4c9d 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -40,7 +40,7 @@ module ExtractsPath def extract_ref(id) pair = ['', ''] - return pair unless @project # rubocop:disable Cop/ModuleWithInstanceVariables + return pair unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables if id =~ /^(\h{40})(.+)/ # If the ref appears to be a SHA, we're done, just split the string @@ -104,7 +104,7 @@ module ExtractsPath # # Automatically renders `not_found!` if a valid tree path could not be # resolved (e.g., when a user inserts an invalid path or ref). - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def assign_ref_vars # assign allowed options allowed_options = ["filter_ref"] @@ -132,10 +132,10 @@ module ExtractsPath rescue RuntimeError, NoMethodError, InvalidPathError render_404 end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables def tree - @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Cop/ModuleWithInstanceVariables + @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Gitlab/ModuleWithInstanceVariables end private @@ -148,8 +148,8 @@ module ExtractsPath end def ref_names - return [] unless @project # rubocop:disable Cop/ModuleWithInstanceVariables + return [] unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables - @ref_names ||= @project.repository.ref_names # rubocop:disable Cop/ModuleWithInstanceVariables + @ref_names ||= @project.repository.ref_names # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index e94166aee0c..525563a97f5 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -6,7 +6,7 @@ module Gitlab query .group("DATE(#{::Ci::Pipeline.table_name}.created_at)") .count(:created_at) - .transform_keys { |date| date.strftime(@format) } # rubocop:disable Cop/ModuleWithInstanceVariables + .transform_keys { |date| date.strftime(@format) } # rubocop:disable Gitlab/ModuleWithInstanceVariables end def interval_step diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb index 0dc359a86c3..e45787773a8 100644 --- a/lib/gitlab/ci/config/entry/validatable.rb +++ b/lib/gitlab/ci/config/entry/validatable.rb @@ -13,7 +13,7 @@ module Gitlab end def errors - @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Cop/ModuleWithInstanceVariables + @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Gitlab/ModuleWithInstanceVariables end class_methods do diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index dfb2cfe42b7..91fd9cc7631 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -53,7 +53,7 @@ module Gitlab end def in_memory_application_settings - @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Cop/ModuleWithInstanceVariables + @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Gitlab/ModuleWithInstanceVariables rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError # In case migrations the application_settings table is not created yet, # we fallback to a simple OpenStruct diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index 05b4928c3b9..dcbdf9a64b0 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -14,9 +14,9 @@ module Gitlab def stage_query 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])) - .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Cop/ModuleWithInstanceVariables + .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Gitlab/ModuleWithInstanceVariables .where(issue_table[:deleted_at].eq(nil)) - .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Cop/ModuleWithInstanceVariables + .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables # Load merge_requests query = query.join(mr_table, Arel::Nodes::OuterJoin) diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb index ebbff1b2f33..7a889b3877f 100644 --- a/lib/gitlab/cycle_analytics/production_helper.rb +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -4,7 +4,7 @@ module Gitlab def stage_query super .where(mr_metrics_table[:first_deployed_to_production_at] - .gteq(@options[:from])) # rubocop:disable Cop/ModuleWithInstanceVariables + .gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb index 9ae1f66a182..403afbe3b9a 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb @@ -6,7 +6,7 @@ module Gitlab module Routable def full_path if route && route.path.present? - @full_path ||= route.path # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -30,7 +30,7 @@ module Gitlab def prepare_route route || build_route(source: self) route.path = build_full_path - @full_path = nil # rubocop:disable Cop/ModuleWithInstanceVariables + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index 19dab0037bb..0135b3c6f22 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -32,7 +32,7 @@ module Gitlab def execute(cmd) output, status = Gitlab::Popen.popen(cmd) - @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Cop/ModuleWithInstanceVariables + @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Gitlab/ModuleWithInstanceVariables status.zero? end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index fa88d41be73..6ea132fc5bf 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -154,7 +154,7 @@ module Gitlab # When enabled this should be set before being used as the usual pattern # "@foo ||= bar" is _not_ thread-safe. - # rubocop:disable Cop/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def pool if influx_metrics_enabled? if @pool.nil? @@ -171,7 +171,7 @@ module Gitlab @pool end end - # rubocop:enable Cop/ModuleWithInstanceVariables + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end -- cgit v1.2.1 From 689658456f706be7278fbf50fcde9c7f43cd0655 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 22 Nov 2017 17:32:10 +0800 Subject: Cache allowed_ids --- lib/gitlab/cycle_analytics/base_event_fetcher.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index 62fc3ee3b5f..e3e3767cc75 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -56,7 +56,7 @@ module Gitlab end def allowed_ids - allowed_ids_finder_class + @allowed_ids ||= allowed_ids_finder_class .new(@options[:current_user], project_id: @project.id) .execute.where(id: event_result_ids).pluck(:id) end -- cgit v1.2.1 From dec420feb39975fefd15460f372369071c346d53 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 11 Dec 2017 16:22:22 +0000 Subject: fixed project homepage not having correct variable --- lib/extracts_path.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib') diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 721ed97bb6b..bab46281922 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -128,6 +128,9 @@ module ExtractsPath @hex_path = Digest::SHA1.hexdigest(@path) @logs_path = logs_file_project_ref_path(@project, @ref, @path) + blob_ids = tree.blobs.map(&:id) + @lfs_blobs = Gitlab::Git::Blob.batch_lfs_pointers(@repo, blob_ids) + rescue RuntimeError, NoMethodError, InvalidPathError render_404 end -- cgit v1.2.1 From e391fe1d6d39837d6aebb14f90e200fc6ff42d81 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 15:11:34 +0100 Subject: Reduce cardinality of gitlab_cache_operation_duration_seconds histogram --- lib/gitlab/metrics/subscribers/rails_cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/subscribers/rails_cache.rb b/lib/gitlab/metrics/subscribers/rails_cache.rb index efd3c9daf79..250897a79c2 100644 --- a/lib/gitlab/metrics/subscribers/rails_cache.rb +++ b/lib/gitlab/metrics/subscribers/rails_cache.rb @@ -66,7 +66,7 @@ module Gitlab :gitlab_cache_operation_duration_seconds, 'Cache access time', Transaction::BASE_LABELS.merge({ action: nil }), - [0.001, 0.002, 0.005, 0.01, 0.02, 0.05, 0.1, 0.500, 2.0, 10.0] + [0.001, 0.01, 0.1, 1, 10] ) end -- cgit v1.2.1 From b02db1f4931060d9e28bd0d651dbea34c79340e2 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 16:54:11 +0100 Subject: Fix gitaly_call_histogram to observe times in seconds correctly --- lib/gitlab/gitaly_client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index b753ac46291..7aa2eafbb73 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -132,7 +132,7 @@ module Gitlab self.query_time += duration gitaly_call_histogram.observe( current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s), - duration) + duration / 1000.0) end def self.current_transaction_labels -- cgit v1.2.1 From b1849ee2e66b6355776397717a33dc7ada772332 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 28 Nov 2017 17:16:50 +0100 Subject: Use a dedicated queue for each worker --- lib/gitlab/sidekiq_config.rb | 37 +++++++++++++++++++++----------- lib/gitlab/sidekiq_versioning.rb | 25 +++++++++++++++++++++ lib/gitlab/sidekiq_versioning/manager.rb | 12 +++++++++++ 3 files changed, 61 insertions(+), 13 deletions(-) create mode 100644 lib/gitlab/sidekiq_versioning.rb create mode 100644 lib/gitlab/sidekiq_versioning/manager.rb (limited to 'lib') diff --git a/lib/gitlab/sidekiq_config.rb b/lib/gitlab/sidekiq_config.rb index dc9886732b5..c3d7814551c 100644 --- a/lib/gitlab/sidekiq_config.rb +++ b/lib/gitlab/sidekiq_config.rb @@ -1,16 +1,35 @@ require 'yaml' +require 'set' module Gitlab module SidekiqConfig - def self.redis_queues - @redis_queues ||= Sidekiq::Queue.all.map(&:name) + # This method is called by `bin/sidekiq-cluster` in EE, which runs outside + # of bundler/Rails context, so we cannot use any gem or Rails methods. + def self.worker_queues(rails_path = Rails.root.to_s) + @worker_queues ||= {} + @worker_queues[rails_path] ||= YAML.load_file(File.join(rails_path, 'app/workers/all_queues.yml')) end # This method is called by `bin/sidekiq-cluster` in EE, which runs outside # of bundler/Rails context, so we cannot use any gem or Rails methods. - def self.config_queues(rails_path = Rails.root.to_s) + def self.expand_queues(queues, all_queues = self.worker_queues) + return [] if queues.empty? + + queues_set = all_queues.to_set + + queues.flat_map do |queue| + [queue, *queues_set.grep(/\A#{queue}:/)] + end + end + + def self.redis_queues + # Not memoized, because this can change during the life of the application + Sidekiq::Queue.all.map(&:name) + end + + def self.config_queues @config_queues ||= begin - config = YAML.load_file(File.join(rails_path, 'config', 'sidekiq_queues.yml')) + config = YAML.load_file(Rails.root.join('config/sidekiq_queues.yml')) config[:queues].map(&:first) end end @@ -23,14 +42,6 @@ module Gitlab @workers ||= find_workers(Rails.root.join('app', 'workers')) end - def self.default_queues - [ActionMailer::DeliveryJob.queue_name, 'default'] - end - - def self.worker_queues - @worker_queues ||= (workers.map(&:queue) + default_queues).uniq - end - def self.find_workers(root) concerns = root.join('concerns').to_s @@ -43,7 +54,7 @@ module Gitlab ns.camelize.constantize end - # Skip concerns + # Skip things that aren't workers workers.select { |w| w < Sidekiq::Worker } end end diff --git a/lib/gitlab/sidekiq_versioning.rb b/lib/gitlab/sidekiq_versioning.rb new file mode 100644 index 00000000000..9683214ec18 --- /dev/null +++ b/lib/gitlab/sidekiq_versioning.rb @@ -0,0 +1,25 @@ +module Gitlab + module SidekiqVersioning + def self.install! + Sidekiq::Manager.prepend SidekiqVersioning::Manager + + # The Sidekiq client API always adds the queue to the Sidekiq queue + # list, but mail_room and gitlab-shell do not. This is only necessary + # for monitoring. + begin + queues = SidekiqConfig.worker_queues + + if queues.any? + Sidekiq.redis do |conn| + conn.pipelined do + queues.each do |queue| + conn.sadd('queues', queue) + end + end + end + end + rescue ::Redis::BaseError, SocketError, Errno::ENOENT, Errno::EADDRNOTAVAIL, Errno::EAFNOSUPPORT, Errno::ECONNRESET, Errno::ECONNREFUSED + end + end + end +end diff --git a/lib/gitlab/sidekiq_versioning/manager.rb b/lib/gitlab/sidekiq_versioning/manager.rb new file mode 100644 index 00000000000..308be0fdf76 --- /dev/null +++ b/lib/gitlab/sidekiq_versioning/manager.rb @@ -0,0 +1,12 @@ +module Gitlab + module SidekiqVersioning + module Manager + def initialize(options = {}) + options[:strict] = false + options[:queues] = SidekiqConfig.expand_queues(options[:queues]) + Sidekiq.logger.info "Listening on queues #{options[:queues].uniq.sort}" + super + end + end + end +end -- cgit v1.2.1 From a8ebed6016726722a2283458e4176fc9177558af Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 18:12:49 +0100 Subject: Make `System.monotonic_time` retun seconds represented by float with microsecond precision --- lib/gitlab/gitaly_client.rb | 2 +- lib/gitlab/metrics/method_call.rb | 18 +++++++++++------- lib/gitlab/metrics/samplers/ruby_sampler.rb | 2 +- lib/gitlab/metrics/system.rb | 9 +++++---- lib/gitlab/metrics/transaction.rb | 8 ++++++-- 5 files changed, 24 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 7aa2eafbb73..b753ac46291 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -132,7 +132,7 @@ module Gitlab self.query_time += duration gitaly_call_histogram.observe( current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s), - duration / 1000.0) + duration) end def self.current_transaction_labels diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 65d55576ac2..6fb8f564237 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -4,7 +4,7 @@ module Gitlab class MethodCall MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze - attr_reader :real_time, :cpu_time, :call_count, :labels + attr_reader :real_time_seconds, :cpu_time, :call_count, :labels def self.call_duration_histogram return @call_duration_histogram if @call_duration_histogram @@ -27,37 +27,41 @@ module Gitlab @transaction = transaction @name = name @labels = { module: @module_name, method: @method_name } - @real_time = 0 + @real_time_seconds = 0 @cpu_time = 0 @call_count = 0 end # Measures the real and CPU execution time of the supplied block. def measure - start_real = System.monotonic_time + start_real_seconds = System.monotonic_time start_cpu = System.cpu_time retval = yield - real_time = System.monotonic_time - start_real + real_time_seconds = System.monotonic_time - start_real_seconds cpu_time = System.cpu_time - start_cpu - @real_time += real_time + @real_time_seconds += real_time_seconds @cpu_time += cpu_time @call_count += 1 if call_measurement_enabled? && above_threshold? - self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time / 1000.0) + self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time_seconds) end retval end + def real_time_milliseconds + (real_time_seconds * 1000.0).to_i + end + # Returns a Metric instance of the current method call. def to_metric Metric.new( Instrumentation.series, { - duration: real_time, + duration: real_time_milliseconds, cpu_duration: cpu_time, call_count: call_count }, diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index b68800417a2..4e1ea62351f 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -52,7 +52,7 @@ module Gitlab metrics[:memory_usage].set(labels, System.memory_usage) metrics[:file_descriptors].set(labels, System.file_descriptor_count) - metrics[:sampler_duration].observe(labels.merge(worker_label), (System.monotonic_time - start_time) / 1000.0) + metrics[:sampler_duration].observe(labels.merge(worker_label), System.monotonic_time - start_time) ensure GC::Profiler.clear end diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb index c2cbd3c16a1..b31cc6236d1 100644 --- a/lib/gitlab/metrics/system.rb +++ b/lib/gitlab/metrics/system.rb @@ -51,11 +51,12 @@ module Gitlab Process.clock_gettime(Process::CLOCK_REALTIME, precision) end - # Returns the current monotonic clock time in a given precision. + # Returns the current monotonic clock time as seconds with microseconds precision. # - # Returns the time as a Fixnum. - def self.monotonic_time(precision = :millisecond) - Process.clock_gettime(Process::CLOCK_MONOTONIC, precision) + # Returns the time as a Float. + def self.monotonic_time + + Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) end end end diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb index ee3afc5ffdb..16f0969ab74 100644 --- a/lib/gitlab/metrics/transaction.rb +++ b/lib/gitlab/metrics/transaction.rb @@ -35,6 +35,10 @@ module Gitlab @finished_at ? (@finished_at - @started_at) : 0.0 end + def duration_milliseconds + (duration * 1000).to_i + end + def allocated_memory @memory_after - @memory_before end @@ -50,7 +54,7 @@ module Gitlab @memory_after = System.memory_usage @finished_at = System.monotonic_time - self.class.metric_transaction_duration_seconds.observe(labels, duration * 1000) + self.class.metric_transaction_duration_seconds.observe(labels, duration) self.class.metric_transaction_allocated_memory_bytes.observe(labels, allocated_memory * 1024.0) Thread.current[THREAD_KEY] = nil @@ -97,7 +101,7 @@ module Gitlab end def track_self - values = { duration: duration, allocated_memory: allocated_memory } + values = { duration: duration_milliseconds, allocated_memory: allocated_memory } @values.each do |name, value| values[name] = value -- cgit v1.2.1 From 53dc9e83c34b2a0ee2651046de031566f5b925d2 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 7 Dec 2017 19:49:44 +0100 Subject: Cache feature check for 5 minutes for MethodCall instrumentation toggle --- lib/gitlab/metrics/method_call.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 65d55576ac2..ab7e7a06a77 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -72,7 +72,9 @@ module Gitlab end def call_measurement_enabled? - Feature.get(:prometheus_metrics_method_instrumentation).enabled? + Rails.cache.fetch(:prometheus_metrics_method_instrumentation_enabled, expires_in: 5.minutes) do + Feature.get(:prometheus_metrics_method_instrumentation).enabled? + end end end end -- cgit v1.2.1 From b503e6ff423699f7556fb948051e9c180a7b89f0 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 8 Dec 2017 19:36:30 +0100 Subject: Implement simple in memory cache that expires after 5 minutes --- lib/gitlab/metrics/method_call.rb | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index ab7e7a06a77..744c489d46e 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -18,6 +18,24 @@ module Gitlab end end + def self.call_measurement_enabled? + return @call_measurement_enabled unless call_measurement_enabled_cache_expired? + MUTEX.synchronize do + return @call_measurement_enabled unless call_measurement_enabled_cache_expired? + @call_measurement_enabled_cache_expires_at = Time.now + 5.minutes + @call_measurement_enabled = Feature.get(:prometheus_metrics_method_instrumentation).enabled? + end + end + + def self.call_measurement_enabled_cache_expired? + @call_measurement_enabled.nil? || @call_measurement_enabled_cache_expires_at.nil? || @call_measurement_enabled_cache_expires_at < Time.now + end + + def self.call_measurement_enabled_cache_expire + @call_measurement_enabled = nil + @call_measurement_enabled_cache_expires_at = nil + end + # name - The full name of the method (including namespace) such as # `User#sign_in`. # @@ -45,7 +63,7 @@ module Gitlab @cpu_time += cpu_time @call_count += 1 - if call_measurement_enabled? && above_threshold? + if self.class.call_measurement_enabled? && above_threshold? self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time / 1000.0) end @@ -70,12 +88,6 @@ module Gitlab def above_threshold? real_time >= Metrics.method_call_threshold end - - def call_measurement_enabled? - Rails.cache.fetch(:prometheus_metrics_method_instrumentation_enabled, expires_in: 5.minutes) do - Feature.get(:prometheus_metrics_method_instrumentation).enabled? - end - end end end end -- cgit v1.2.1 From 6af849644dd097f7b41a26ca8b49d274444db196 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 8 Dec 2017 20:19:01 +0100 Subject: Set cache expire only once the cache is filled, to avoid situation where old result is returned in parallel thread. --- lib/gitlab/metrics/method_call.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 744c489d46e..2f5c45966ff 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -20,10 +20,13 @@ module Gitlab def self.call_measurement_enabled? return @call_measurement_enabled unless call_measurement_enabled_cache_expired? + MUTEX.synchronize do return @call_measurement_enabled unless call_measurement_enabled_cache_expired? - @call_measurement_enabled_cache_expires_at = Time.now + 5.minutes + @call_measurement_enabled = Feature.get(:prometheus_metrics_method_instrumentation).enabled? + @call_measurement_enabled_cache_expires_at = Time.now + 5.minutes + @call_measurement_enabled end end -- cgit v1.2.1 From 5904b033dba553636ae2a06cbf1469d8f19df040 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 11 Dec 2017 22:24:07 +0100 Subject: Implemente measurement enabled cache using AtomicReference --- lib/gitlab/metrics/method_call.rb | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 2f5c45966ff..dc56a10957d 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -2,6 +2,7 @@ module Gitlab module Metrics # Class for tracking timing information about method calls class MethodCall + MEASUREMENT_ENABLED_CACHE = Concurrent::AtomicReference.new({ enabled: false, expires_at: Time.now }) MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze attr_reader :real_time, :cpu_time, :call_count, :labels @@ -18,25 +19,19 @@ module Gitlab end end - def self.call_measurement_enabled? - return @call_measurement_enabled unless call_measurement_enabled_cache_expired? - - MUTEX.synchronize do - return @call_measurement_enabled unless call_measurement_enabled_cache_expired? - - @call_measurement_enabled = Feature.get(:prometheus_metrics_method_instrumentation).enabled? - @call_measurement_enabled_cache_expires_at = Time.now + 5.minutes - @call_measurement_enabled + def call_measurement_enabled? + res = MEASUREMENT_ENABLED_CACHE.update do |cache| + if cache[:expires_at] < Time.now + { + enabled: Feature.get(:prometheus_metrics_method_instrumentation).enabled?, + expires_at: Time.now + 5.minutes + } + else + cache + end end - end - - def self.call_measurement_enabled_cache_expired? - @call_measurement_enabled.nil? || @call_measurement_enabled_cache_expires_at.nil? || @call_measurement_enabled_cache_expires_at < Time.now - end - def self.call_measurement_enabled_cache_expire - @call_measurement_enabled = nil - @call_measurement_enabled_cache_expires_at = nil + res[:enabled] end # name - The full name of the method (including namespace) such as @@ -66,7 +61,7 @@ module Gitlab @cpu_time += cpu_time @call_count += 1 - if self.class.call_measurement_enabled? && above_threshold? + if call_measurement_enabled? && above_threshold? self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time / 1000.0) end -- cgit v1.2.1 From 408208bc2b75d28235092e9bb3821242fdad08fb Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 11 Dec 2017 23:39:40 +0100 Subject: Use AtomicFixNum to implement CAS isolated cache update. i.e. Using compare and swap we update the expires_at value. The thread that actually is able to update this value will also set the cache holding method_call enabled state --- lib/gitlab/metrics/method_call.rb | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index dc56a10957d..2c1cb789314 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -2,7 +2,8 @@ module Gitlab module Metrics # Class for tracking timing information about method calls class MethodCall - MEASUREMENT_ENABLED_CACHE = Concurrent::AtomicReference.new({ enabled: false, expires_at: Time.now }) + MEASUREMENT_ENABLED_CACHE = Concurrent::AtomicBoolean.new(false) + MEASUREMENT_ENABLED_CACHE_EXPIRES_AT = Concurrent::AtomicFixnum.new(Time.now.to_i) MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze attr_reader :real_time, :cpu_time, :call_count, :labels @@ -20,18 +21,14 @@ module Gitlab end def call_measurement_enabled? - res = MEASUREMENT_ENABLED_CACHE.update do |cache| - if cache[:expires_at] < Time.now - { - enabled: Feature.get(:prometheus_metrics_method_instrumentation).enabled?, - expires_at: Time.now + 5.minutes - } - else - cache + expires_at = MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value + if expires_at < Time.now.to_i + if MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) + MEASUREMENT_ENABLED_CACHE.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? end end - res[:enabled] + MEASUREMENT_ENABLED_CACHE.value end # name - The full name of the method (including namespace) such as -- cgit v1.2.1 From ca176a9bfe2ce9555da16e134bb92779b85952ad Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 11 Dec 2017 23:52:28 +0100 Subject: move call_measurement_enabled? method to the bottom of the file --- lib/gitlab/metrics/method_call.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 2c1cb789314..a6900225a27 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -20,17 +20,6 @@ module Gitlab end end - def call_measurement_enabled? - expires_at = MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value - if expires_at < Time.now.to_i - if MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) - MEASUREMENT_ENABLED_CACHE.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? - end - end - - MEASUREMENT_ENABLED_CACHE.value - end - # name - The full name of the method (including namespace) such as # `User#sign_in`. # @@ -83,6 +72,17 @@ module Gitlab def above_threshold? real_time >= Metrics.method_call_threshold end + + def call_measurement_enabled? + expires_at = MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value + if expires_at < Time.now.to_i + if MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) + MEASUREMENT_ENABLED_CACHE.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? + end + end + + MEASUREMENT_ENABLED_CACHE.value + end end end end -- cgit v1.2.1 From fd0a5168542f3a69815c99dc21f39587064f872d Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 00:23:56 +0100 Subject: use class variables instead of CONSTANTs --- lib/gitlab/metrics/method_call.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index a6900225a27..a844100e5a8 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -2,8 +2,8 @@ module Gitlab module Metrics # Class for tracking timing information about method calls class MethodCall - MEASUREMENT_ENABLED_CACHE = Concurrent::AtomicBoolean.new(false) - MEASUREMENT_ENABLED_CACHE_EXPIRES_AT = Concurrent::AtomicFixnum.new(Time.now.to_i) + @@measurement_enabled_cache = Concurrent::AtomicBoolean.new(false) + @@measurement_enabled_cache_expires_at = Concurrent::AtomicFixnum.new(Time.now.to_i) MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze attr_reader :real_time, :cpu_time, :call_count, :labels @@ -20,6 +20,10 @@ module Gitlab end end + def self.measurement_enabled_cache_expires_at + @@measurement_enabled_cache_expires_at + end + # name - The full name of the method (including namespace) such as # `User#sign_in`. # @@ -74,14 +78,14 @@ module Gitlab end def call_measurement_enabled? - expires_at = MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value + expires_at = @@measurement_enabled_cache_expires_at.value if expires_at < Time.now.to_i - if MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) - MEASUREMENT_ENABLED_CACHE.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? + if @@measurement_enabled_cache_expires_at.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) + @@measurement_enabled_cache.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? end end - MEASUREMENT_ENABLED_CACHE.value + @@measurement_enabled_cache.value end end end -- cgit v1.2.1 From da19ce625ba8a0db9921fd85e331e70b14a5625b Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 18:52:08 +0100 Subject: Expire feature flag cache after 1minute --- lib/gitlab/metrics/method_call.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index a844100e5a8..c58e54916e6 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -80,7 +80,7 @@ module Gitlab def call_measurement_enabled? expires_at = @@measurement_enabled_cache_expires_at.value if expires_at < Time.now.to_i - if @@measurement_enabled_cache_expires_at.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) + if @@measurement_enabled_cache_expires_at.compare_and_set(expires_at, 1.minute.from_now.to_i) @@measurement_enabled_cache.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? end end -- cgit v1.2.1 From 51668d3e1faa448ceecf5457db552c89314c9dbe Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 22:30:27 +0100 Subject: Use class variable and add rubocop exception --- lib/gitlab/metrics/method_call.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index c58e54916e6..9112164f22e 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -1,3 +1,5 @@ +# rubocop:disable Style/ClassVars + module Gitlab module Metrics # Class for tracking timing information about method calls -- cgit v1.2.1 From 54f13b1ec8542dc5085e0367734e8344c2c3d01e Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 9 Dec 2017 01:01:42 -0800 Subject: Add rate limiting to guard against excessive scheduling of pipelines --- lib/gitlab/action_rate_limiter.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 lib/gitlab/action_rate_limiter.rb (limited to 'lib') diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb new file mode 100644 index 00000000000..c3af583a3ed --- /dev/null +++ b/lib/gitlab/action_rate_limiter.rb @@ -0,0 +1,31 @@ +module Gitlab + # This class implements a simple rate limiter that can be used to throttle + # certain actions. Unlike Rack Attack and Rack::Throttle, which operate at + # the middleware level, this can be used at the controller level. + class ActionRateLimiter + TIME_TO_EXPIRE = 60 # 1 min + + attr_accessor :action, :expiry_time + + def initialize(action:, expiry_time: TIME_TO_EXPIRE) + @action = action + @expiry_time = expiry_time + end + + def increment(key) + value = 0 + + Gitlab::Redis::Cache.with do |redis| + cache_key = "action_rate_limiter:#{action}:#{key}" + value = redis.incr(cache_key) + redis.expire(cache_key, expiry_time) if value == 1 + end + + value.to_i + end + + def throttled?(key, threshold_value) + self.increment(key) > threshold_value + end + end +end -- cgit v1.2.1 From 8b939bfab769810ba56f5f4ca18632fd7ae435db Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 9 Dec 2017 15:45:01 -0800 Subject: Add spec for ActionRateLimiter --- lib/gitlab/action_rate_limiter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb index c3af583a3ed..7b231ac14ca 100644 --- a/lib/gitlab/action_rate_limiter.rb +++ b/lib/gitlab/action_rate_limiter.rb @@ -16,12 +16,12 @@ module Gitlab value = 0 Gitlab::Redis::Cache.with do |redis| - cache_key = "action_rate_limiter:#{action}:#{key}" + cache_key = "action_rate_limiter:#{action.to_s}:#{key}" value = redis.incr(cache_key) redis.expire(cache_key, expiry_time) if value == 1 end - value.to_i + value end def throttled?(key, threshold_value) -- cgit v1.2.1 From 02e7e499d4011733c1cf0f6fb675dd2d309f0d52 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 9 Dec 2017 21:00:34 -0800 Subject: Fix Rubocop offense and use a symbol instead of a string --- lib/gitlab/action_rate_limiter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb index 7b231ac14ca..53add503c60 100644 --- a/lib/gitlab/action_rate_limiter.rb +++ b/lib/gitlab/action_rate_limiter.rb @@ -16,7 +16,7 @@ module Gitlab value = 0 Gitlab::Redis::Cache.with do |redis| - cache_key = "action_rate_limiter:#{action.to_s}:#{key}" + cache_key = "action_rate_limiter:#{action}:#{key}" value = redis.incr(cache_key) redis.expire(cache_key, expiry_time) if value == 1 end -- cgit v1.2.1 From 4b0465f20de1bf58326c7daf6876b63438f00d84 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 12 Dec 2017 15:46:05 -0800 Subject: Address review comments with playing pipeline scheduler --- lib/gitlab/action_rate_limiter.rb | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb index 53add503c60..4cd3bdefda3 100644 --- a/lib/gitlab/action_rate_limiter.rb +++ b/lib/gitlab/action_rate_limiter.rb @@ -12,11 +12,15 @@ module Gitlab @expiry_time = expiry_time end + # Increments the given cache key and increments the value by 1 with the + # given expiration time. Returns the incremented value. + # + # key - An array of ActiveRecord instances def increment(key) value = 0 Gitlab::Redis::Cache.with do |redis| - cache_key = "action_rate_limiter:#{action}:#{key}" + cache_key = action_key(key) value = redis.incr(cache_key) redis.expire(cache_key, expiry_time) if value == 1 end @@ -24,8 +28,20 @@ module Gitlab value end + # Increments the given key and returns true if the action should + # be throttled. + # + # key - An array of ActiveRecord instances + # threshold_value - The maximum number of times this action should occur in the given time interval def throttled?(key, threshold_value) self.increment(key) > threshold_value end + + private + + def action_key(key) + serialized = key.map { |obj| "#{obj.class.model_name.to_s.underscore}:#{obj.id}" }.join(":") + "action_rate_limiter:#{action}:#{serialized}" + end end end -- cgit v1.2.1 From ab4fa64308176cc069e6a731d35a53c886af805e Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 6 Dec 2017 09:39:25 +0000 Subject: Add a gitlab:tcp_check rake task This allows us to avoid relying on telnet / netcat being installed --- lib/gitlab/tcp_checker.rb | 45 +++++++++++++++++++++++++++++++++++++++++ lib/tasks/gitlab/tcp_check.rake | 20 ++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 lib/gitlab/tcp_checker.rb create mode 100644 lib/tasks/gitlab/tcp_check.rake (limited to 'lib') diff --git a/lib/gitlab/tcp_checker.rb b/lib/gitlab/tcp_checker.rb new file mode 100644 index 00000000000..6e24e46d0ea --- /dev/null +++ b/lib/gitlab/tcp_checker.rb @@ -0,0 +1,45 @@ +module Gitlab + class TcpChecker + attr_reader :remote_host, :remote_port, :local_host, :local_port, :error + + def initialize(remote_host, remote_port, local_host = nil, local_port = nil) + @remote_host = remote_host + @remote_port = remote_port + @local_host = local_host + @local_port = local_port + end + + def local + join_host_port(local_host, local_port) + end + + def remote + join_host_port(remote_host, remote_port) + end + + def check(timeout: 10) + Socket.tcp( + remote_host, remote_port, + local_host, local_port, + connect_timeout: timeout + ) do |sock| + @local_port, @local_host = Socket.unpack_sockaddr_in(sock.local_address) + @remote_port, @remote_host = Socket.unpack_sockaddr_in(sock.remote_address) + end + + true + rescue => err + @error = err + + false + end + + private + + def join_host_port(host, port) + host = "[#{host}]" if host.include?(':') + + "#{host}:#{port}" + end + end +end diff --git a/lib/tasks/gitlab/tcp_check.rake b/lib/tasks/gitlab/tcp_check.rake new file mode 100644 index 00000000000..1400f57d6b9 --- /dev/null +++ b/lib/tasks/gitlab/tcp_check.rake @@ -0,0 +1,20 @@ +namespace :gitlab do + desc "GitLab | Check TCP connectivity to a specific host and port" + task :tcp_check, [:host, :port] => :environment do |_t, args| + unless args.host && args.port + puts "Please specify a host and port: `rake gitlab:tcp_check[example.com,80]`".color(:red) + exit 1 + end + + checker = Gitlab::TcpChecker.new(args.host, args.port) + + if checker.check + puts "TCP connection from #{checker.local} to #{checker.remote} succeeded".color(:green) + else + puts "TCP connection to #{checker.remote} failed: #{checker.error}".color(:red) + puts + puts 'Check that host and port are correct, and that the traffic is permitted through any firewalls.' + exit 1 + end + end +end -- cgit v1.2.1 From 835a5db376a69dce20ba616d480a5a9ab15b2577 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Wed, 6 Dec 2017 16:54:57 +0100 Subject: Migrate Gitlab::Git::Repository#merge_base_commit to Gitaly Closes gitaly#808 --- lib/gitlab/git/operation_service.rb | 2 +- lib/gitlab/git/repository.rb | 9 ++++++++- lib/gitlab/gitaly_client/repository_service.rb | 10 ++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb index 7e8fe173056..ef5bdbaf819 100644 --- a/lib/gitlab/git/operation_service.rb +++ b/lib/gitlab/git/operation_service.rb @@ -126,7 +126,7 @@ module Gitlab oldrev = branch.target - if oldrev == repository.rugged.merge_base(newrev, branch.target) + if oldrev == repository.merge_base(newrev, branch.target) oldrev else raise Gitlab::Git::CommitError.new('Branch diverged') diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 73889328f36..816fbbeaef8 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -516,8 +516,15 @@ module Gitlab # Returns the SHA of the most recent common ancestor of +from+ and +to+ def merge_base_commit(from, to) - rugged.merge_base(from, to) + gitaly_migrate(:merge_base) do |is_enabled| + if is_enabled + gitaly_repository_client.find_merge_base(from, to) + else + rugged.merge_base(from, to) + end + end end + alias_method :merge_base, :merge_base_commit # Gitaly note: JV: check gitlab-ee before removing this method. def rugged_is_ancestor?(ancestor_id, descendant_id) diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index a477d618f63..c1f95396878 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -69,6 +69,16 @@ module Gitlab response.value end + def find_merge_base(*revisions) + request = Gitaly::FindMergeBaseRequest.new( + repository: @gitaly_repo, + revisions: revisions.map { |r| GitalyClient.encode(r) } + ) + + response = GitalyClient.call(@storage, :repository_service, :find_merge_base, request) + response.base.presence + end + def fetch_source_branch(source_repository, source_branch, local_ref) request = Gitaly::FetchSourceBranchRequest.new( repository: @gitaly_repo, -- cgit v1.2.1 From 55f322085d0507640366b7a774fe7819771ff54b Mon Sep 17 00:00:00 2001 From: Jacopo Date: Sat, 18 Nov 2017 15:06:55 +0100 Subject: Adds ordering to projects contributors in API Allows ordering in GET api/v4/projects/:project_id/repository/contributors through `order_by` and `sort` params. The available `order_by` options are: name|email|commits. The available `sort` options are: asc|desc. --- lib/api/repositories.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 7887b886c03..4f36bbd760f 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -110,10 +110,12 @@ module API end params do use :pagination + optional :order_by, type: String, values: %w[email name commits], default: nil, desc: 'Return contributors ordered by `name` or `email` or `commits`' + optional :sort, type: String, values: %w[asc desc], default: nil, desc: 'Sort by asc (ascending) or desc (descending)' end get ':id/repository/contributors' do begin - contributors = ::Kaminari.paginate_array(user_project.repository.contributors) + contributors = ::Kaminari.paginate_array(user_project.repository.contributors(order_by: params[:order_by], sort: params[:sort])) present paginate(contributors), with: Entities::Contributor rescue not_found! -- cgit v1.2.1 From b51e6d6ddc7bf9ffdb9d82debfeaee4942e01659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 14 Dec 2017 01:26:07 +0100 Subject: Update flipper to 0.11.0 and take advantage of the new features MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added an ActiveSupport (using Rails.cache) caching adapter - Overview of the new features can be found at https://johnnunemaker.com/flippin-features-at-runtime/ - Full Changelog can be found at https://github.com/jnunemaker/flipper/blob/v0.11.0/Changelog.md Signed-off-by: Rémy Coutable --- lib/feature.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/feature.rb b/lib/feature.rb index ac3bc65c0d5..8e9ba5c530a 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -1,5 +1,3 @@ -require 'flipper/adapters/active_record' - class Feature # Classes to override flipper table names class FlipperFeature < Flipper::Adapters::ActiveRecord::Feature @@ -62,12 +60,7 @@ class Feature end def flipper - @flipper ||= begin - adapter = Flipper::Adapters::ActiveRecord.new( - feature_class: FlipperFeature, gate_class: FlipperGate) - - Flipper.new(adapter) - end + @flipper ||= Flipper.instance end # This method is called from config/initializers/flipper.rb and can be used -- cgit v1.2.1 From 7d2affeff1885fb9984fed1088c8dffdc41a7320 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Dec 2017 10:10:20 +0000 Subject: moved lfs blob fetch from extractspath file --- lib/extracts_path.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'lib') diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index bab46281922..f576ef74603 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -127,10 +127,6 @@ module ExtractsPath @hex_path = Digest::SHA1.hexdigest(@path) @logs_path = logs_file_project_ref_path(@project, @ref, @path) - - blob_ids = tree.blobs.map(&:id) - @lfs_blobs = Gitlab::Git::Blob.batch_lfs_pointers(@repo, blob_ids) - rescue RuntimeError, NoMethodError, InvalidPathError render_404 end -- cgit v1.2.1 From cbd3ce8f41fc5691a1d23aca0ffe3221ab5d26af Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Dec 2017 11:59:01 +0000 Subject: moved lfs_blob_ids method into ExtractsPath module --- lib/extracts_path.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib') diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index f576ef74603..27c712a84d4 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -135,6 +135,11 @@ module ExtractsPath @tree ||= @repo.tree(@commit.id, @path) end + def lfs_blob_ids + blob_ids = tree.blobs.map(&:id) + @lfs_blob_ids = Gitlab::Git::Blob.batch_lfs_pointers(@project.repository, blob_ids).map(&:id) + end + private # overriden in subclasses, do not remove -- cgit v1.2.1 From e7b40c2f6e79e3f32e45baa5a037e14e02f7165d Mon Sep 17 00:00:00 2001 From: haseeb Date: Thu, 14 Dec 2017 13:42:15 +0000 Subject: sorting for tags api --- lib/api/tags.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 0d394a7b441..5e0afc6a7e4 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -14,10 +14,15 @@ module API success Entities::Tag end params do + optional :sort, type: String, values: %w[asc desc], default: 'desc', + desc: 'Return tags sorted in updated by `asc` or `desc` order.' + optional :order_by, type: String, values: %w[name updated], default: 'updated', + desc: 'Return tags ordered by `name` or `updated` fields.' use :pagination end get ':id/repository/tags' do - tags = ::Kaminari.paginate_array(user_project.repository.tags.sort_by(&:name).reverse) + tags = ::Kaminari.paginate_array(::TagsFinder.new(user_project.repository, sort: "#{params[:order_by]}_#{params[:sort]}").execute) + present paginate(tags), with: Entities::Tag, project: user_project end -- cgit v1.2.1 From 4b785df27baa78b2ebe51e66de25edcb8566ab2d Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 11 Dec 2017 17:52:07 +0000 Subject: Import gitlab_projects.rb from gitlab-shell By importing this Ruby code into gitlab-rails (and gitaly-ruby), we avoid 200ms of startup time for each gitlab_projects subprocess we are eliminating. By not having a gitlab_projects subprocess between gitlab-rails / sidekiq and any git subprocesses (e.g. for fork_project, fetch_remote, etc, calls), we can also manage these git processes more cleanly, and avoid sending SIGKILL to them --- lib/gitlab/git/gitlab_projects.rb | 258 ++++++++++++++++++++++++++++++++++++++ lib/gitlab/git/repository.rb | 29 ++++- lib/gitlab/shell.rb | 113 ++++++++++++----- 3 files changed, 365 insertions(+), 35 deletions(-) create mode 100644 lib/gitlab/git/gitlab_projects.rb (limited to 'lib') diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb new file mode 100644 index 00000000000..d948d7895ed --- /dev/null +++ b/lib/gitlab/git/gitlab_projects.rb @@ -0,0 +1,258 @@ +module Gitlab + module Git + class GitlabProjects + include Gitlab::Git::Popen + + # Absolute path to directory where repositories are stored. + # Example: /home/git/repositories + attr_reader :shard_path + + # Relative path is a directory name for repository with .git at the end. + # Example: gitlab-org/gitlab-test.git + attr_reader :repository_relative_path + + # Absolute path to the repository. + # Example: /home/git/repositorities/gitlab-org/gitlab-test.git + attr_reader :repository_absolute_path + + # This is the path at which the gitlab-shell hooks directory can be found. + # It's essential for integration between git and GitLab proper. All new + # repositories should have their hooks directory symlinked here. + attr_reader :global_hooks_path + + attr_reader :logger + + def initialize(shard_path, repository_relative_path, global_hooks_path:, logger:) + @shard_path = shard_path + @repository_relative_path = repository_relative_path + + @logger = logger + @global_hooks_path = global_hooks_path + @repository_absolute_path = File.join(shard_path, repository_relative_path) + @output = StringIO.new + end + + def output + io = @output.dup + io.rewind + io.read + end + + def rm_project + logger.info "Removing repository <#{repository_absolute_path}>." + FileUtils.rm_rf(repository_absolute_path) + end + + # Move repository from one directory to another + # + # Example: gitlab/gitlab-ci.git -> randx/six.git + # + # Won't work if target namespace directory does not exist + # + def mv_project(new_path) + new_absolute_path = File.join(shard_path, new_path) + + # verify that the source repo exists + unless File.exist?(repository_absolute_path) + logger.error "mv-project failed: source path <#{repository_absolute_path}> does not exist." + return false + end + + # ...and that the target repo does not exist + if File.exist?(new_absolute_path) + logger.error "mv-project failed: destination path <#{new_absolute_path}> already exists." + return false + end + + logger.info "Moving repository from <#{repository_absolute_path}> to <#{new_absolute_path}>." + FileUtils.mv(repository_absolute_path, new_absolute_path) + end + + # Import project via git clone --bare + # URL must be publicly cloneable + def import_project(source, timeout) + # Skip import if repo already exists + return false if File.exist?(repository_absolute_path) + + masked_source = mask_password_in_url(source) + + logger.info "Importing project from <#{masked_source}> to <#{repository_absolute_path}>." + cmd = %W(git clone --bare -- #{source} #{repository_absolute_path}) + + success = run_with_timeout(cmd, timeout, nil) + + unless success + logger.error("Importing project from <#{masked_source}> to <#{repository_absolute_path}> failed.") + FileUtils.rm_rf(repository_absolute_path) + return false + end + + Gitlab::Git::Repository.create_hooks(repository_absolute_path, global_hooks_path) + + # The project was imported successfully. + # Remove the origin URL since it may contain password. + remove_origin_in_repo + + true + end + + def fork_repository(new_shard_path, new_repository_relative_path) + from_path = repository_absolute_path + to_path = File.join(new_shard_path, new_repository_relative_path) + + # The repository cannot already exist + if File.exist?(to_path) + logger.error "fork-repository failed: destination repository <#{to_path}> already exists." + return false + end + + # Ensure the namepsace / hashed storage directory exists + FileUtils.mkdir_p(File.dirname(to_path), mode: 0770) + + logger.info "Forking repository from <#{from_path}> to <#{to_path}>." + cmd = %W(git clone --bare --no-local -- #{from_path} #{to_path}) + + run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path) + end + + def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil) + tags_option = tags ? '--tags' : '--no-tags' + + logger.info "Fetching remote #{name} for repository #{repository_absolute_path}." + cmd = %W(git fetch #{name} --prune --quiet) + cmd << '--force' if force + cmd << tags_option + + setup_ssh_auth(ssh_key, known_hosts) do |env| + success = run_with_timeout(cmd, timeout, repository_absolute_path, env) + + unless success + logger.error "Fetching remote #{name} for repository #{repository_absolute_path} failed." + end + + success + end + end + + def push_branches(remote_name, timeout, force, branch_names) + logger.info "Pushing branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" + cmd = %w(git push) + cmd << '--force' if force + cmd += %W(-- #{remote_name}).concat(branch_names) + + success = run_with_timeout(cmd, timeout, repository_absolute_path) + + unless success + logger.error("Pushing branches to remote #{remote_name} failed.") + end + + success + end + + def delete_remote_branches(remote_name, branch_names) + branches = branch_names.map { |branch_name| ":#{branch_name}" } + + logger.info "Pushing deleted branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" + cmd = %W(git push -- #{remote_name}).concat(branches) + + success = run(cmd, repository_absolute_path) + + unless success + logger.error("Pushing deleted branches to remote #{remote_name} failed.") + end + + success + end + + protected + + def run(*args) + output, exitstatus = popen(*args) + @output << output + + exitstatus&.zero? + end + + def run_with_timeout(*args) + output, exitstatus = popen_with_timeout(*args) + @output << output + + exitstatus&.zero? + rescue Timeout::Error + @output.puts('Timed out') + + false + end + + def mask_password_in_url(url) + result = URI(url) + result.password = "*****" unless result.password.nil? + result.user = "*****" unless result.user.nil? # it's needed for oauth access_token + result + rescue + url + end + + def remove_origin_in_repo + cmd = %w(git remote rm origin) + run(cmd, repository_absolute_path) + end + + # Builds a small shell script that can be used to execute SSH with a set of + # custom options. + # + # Options are expanded as `'-oKey="Value"'`, so SSH will correctly interpret + # paths with spaces in them. We trust the user not to embed single or double + # quotes in the key or value. + def custom_ssh_script(options = {}) + args = options.map { |k, v| %Q{'-o#{k}="#{v}"'} }.join(' ') + + [ + "#!/bin/sh", + "exec ssh #{args} \"$@\"" + ].join("\n") + end + + # Known hosts data and private keys can be passed to gitlab-shell in the + # environment. If present, this method puts them into temporary files, writes + # a script that can substitute as `ssh`, setting the options to respect those + # files, and yields: { "GIT_SSH" => "/tmp/myScript" } + def setup_ssh_auth(key, known_hosts) + options = {} + + if key + key_file = Tempfile.new('gitlab-shell-key-file') + key_file.chmod(0o400) + key_file.write(key) + key_file.close + + options['IdentityFile'] = key_file.path + options['IdentitiesOnly'] = 'yes' + end + + if known_hosts + known_hosts_file = Tempfile.new('gitlab-shell-known-hosts') + known_hosts_file.chmod(0o400) + known_hosts_file.write(known_hosts) + known_hosts_file.close + + options['StrictHostKeyChecking'] = 'yes' + options['UserKnownHostsFile'] = known_hosts_file.path + end + + return yield({}) if options.empty? + + script = Tempfile.new('gitlab-shell-ssh-wrapper') + script.chmod(0o755) + script.write(custom_ssh_script(options)) + script.close + + yield('GIT_SSH' => script.path) + ensure + key_file&.close! + known_hosts_file&.close! + script&.close! + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 0e0a1987c7d..c4c6ed7b619 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -39,10 +39,31 @@ module Gitlab repo = Rugged::Repository.init_at(repo_path, bare) repo.close - if symlink_hooks_to.present? - hooks_path = File.join(repo_path, 'hooks') - FileUtils.rm_rf(hooks_path) - FileUtils.ln_s(symlink_hooks_to, hooks_path) + create_hooks(repo_path, symlink_hooks_to) if symlink_hooks_to.present? + + true + end + + def create_hooks(repo_path, global_hooks_path) + local_hooks_path = File.join(repo_path, 'hooks') + real_local_hooks_path = :not_found + + begin + real_local_hooks_path = File.realpath(local_hooks_path) + rescue Errno::ENOENT + # real_local_hooks_path == :not_found + end + + # Do nothing if hooks already exist + unless real_local_hooks_path == File.realpath(global_hooks_path) + # Move the existing hooks somewhere safe + FileUtils.mv( + local_hooks_path, + "#{local_hooks_path}.old.#{Time.now.to_i}" + ) if File.exist?(local_hooks_path) + + # Create the hooks symlink + FileUtils.ln_sf(global_hooks_path, local_hooks_path) end true diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index a22a63665be..9cdd3d22f18 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -66,7 +66,7 @@ module Gitlab # Init new repository # # storage - project's storage name - # name - project path with namespace + # name - project disk path # # Ex. # add_repository("/path/to/storage", "gitlab/gitlab-ci") @@ -94,23 +94,28 @@ module Gitlab # Import repository # # storage - project's storage path - # name - project path with namespace + # name - project disk path + # url - URL to import from # # Ex. - # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://github.com/randx/six.git") + # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def import_repository(storage, name, url) # The timeout ensures the subprocess won't hang forever - cmd = [gitlab_shell_projects_path, 'import-project', - storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"] - gitlab_shell_fast_execute_raise_error(cmd) + cmd = gitlab_projects(storage, "#{name}.git") + success = cmd.import_project(url, git_timeout) + + raise Error, cmd.output unless success + + success end # Fetch remote for repository # # repository - an instance of Git::Repository # remote - remote name + # ssh_auth - SSH known_hosts data and a private key to use for public-key authentication # forced - should we use --force flag? # no_tags - should we use --no-tags flag? # @@ -131,16 +136,15 @@ module Gitlab # Move repository # storage - project's storage path - # path - project path with namespace - # new_path - new project path with namespace + # path - project disk path + # new_path - new project disk path # # Ex. # mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def mv_repository(storage, path, new_path) - gitlab_shell_fast_execute([gitlab_shell_projects_path, 'mv-project', - storage, "#{path}.git", "#{new_path}.git"]) + gitlab_projects(storage, "#{path}.git").mv_project("#{new_path}.git") end # Fork repository to new path @@ -154,30 +158,21 @@ module Gitlab # # Gitaly note: JV: not easy to migrate because this involves two Gitaly servers, not one. def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) - gitlab_shell_fast_execute( - [ - gitlab_shell_projects_path, - 'fork-repository', - forked_from_storage, - "#{forked_from_disk_path}.git", - forked_to_storage, - "#{forked_to_disk_path}.git" - ] - ) + gitlab_projects(forked_from_storage, "#{forked_from_disk_path}.git") + .fork_repository(forked_to_storage, "#{forked_to_disk_path}.git") end # Remove repository from file system # # storage - project's storage path - # name - project path with namespace + # name - project disk path # # Ex. # remove_repository("/path/to/storage", "gitlab/gitlab-ci") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def remove_repository(storage, name) - gitlab_shell_fast_execute([gitlab_shell_projects_path, - 'rm-project', storage, "#{name}.git"]) + gitlab_projects(storage, "#{name}.git").rm_project end # Add new key to gitlab-shell @@ -311,6 +306,47 @@ module Gitlab end end + # Push branch to remote repository + # + # storage - project's storage path + # project_name - project's disk path + # remote_name - remote name + # branch_names - remote branch names to push + # forced - should we use --force flag + # + # Ex. + # push_remote_branches('/path/to/storage', 'gitlab-org/gitlab-test' 'upstream', ['feature']) + # + def push_remote_branches(storage, project_name, remote_name, branch_names, forced: true) + cmd = gitlab_projects(storage, "#{project_name}.git") + + success = cmd.push_branches(remote_name, git_timeout, forced, branch_names) + + raise Error, cmd.output unless success + + success + end + + # Delete branch from remote repository + # + # storage - project's storage path + # project_name - project's disk path + # remote_name - remote name + # branch_names - remote branch names + # + # Ex. + # delete_remote_branches('/path/to/storage', 'gitlab-org/gitlab-test', 'upstream', ['feature']) + # + def delete_remote_branches(storage, project_name, remote_name, branch_names) + cmd = gitlab_projects(storage, "#{project_name}.git") + + success = cmd.delete_remote_branches(remote_name, branch_names) + + raise Error, cmd.output unless success + + success + end + protected def gitlab_shell_path @@ -341,24 +377,35 @@ module Gitlab private - def local_fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) - args = [gitlab_shell_projects_path, 'fetch-remote', storage, name, remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] - args << '--force' if forced - args << '--no-tags' if no_tags + def gitlab_projects(shard_path, disk_path) + Gitlab::Git::GitlabProjects.new( + shard_path, + disk_path, + global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, + logger: Rails.logger + ) + end - vars = {} + def local_fetch_remote(storage_path, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false) + vars = { force: forced, tags: !no_tags } if ssh_auth&.ssh_import? if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? - vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key + vars[:ssh_key] = ssh_auth.ssh_private_key end if ssh_auth.ssh_known_hosts.present? - vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + vars[:known_hosts] = ssh_auth.ssh_known_hosts end end - gitlab_shell_fast_execute_raise_error(args, vars) + cmd = gitlab_projects(storage_path, repository_relative_path) + + success = cmd.fetch_remote(remote, git_timeout, vars) + + raise Error, cmd.output unless success + + success end def gitlab_shell_fast_execute(cmd) @@ -394,6 +441,10 @@ module Gitlab Gitlab::GitalyClient::NamespaceService.new(storage) end + def git_timeout + Gitlab.config.gitlab_shell.git_timeout + end + def gitaly_migrate(method, &block) Gitlab::GitalyClient.migrate(method, &block) rescue GRPC::NotFound, GRPC::BadStatus => e -- cgit v1.2.1 From 240945f87edf80e7f9a4e991b99fd4ade28aaabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 12 Dec 2017 22:55:30 -0300 Subject: Simplify conflict resolver interface This does two things: - Pass commit oids instead of `Gitlab::Git::Commit`s. We only need the former. - Depend on only the target repository for conflict listing. For conflict resolution, treat one repository as a remote one so that we can implement it as such in Gitaly. --- lib/gitlab/conflict/file_collection.rb | 6 ++-- lib/gitlab/git/conflict/resolver.rb | 59 +++++++++++++++++----------------- 2 files changed, 32 insertions(+), 33 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index b9099ce256a..76aee5a3deb 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -4,11 +4,11 @@ module Gitlab attr_reader :merge_request, :resolver def initialize(merge_request) - source_repo = merge_request.source_project.repository.raw our_commit = merge_request.source_branch_head.raw their_commit = merge_request.target_branch_head.raw target_repo = merge_request.target_project.repository.raw - @resolver = Gitlab::Git::Conflict::Resolver.new(source_repo, our_commit, target_repo, their_commit) + @source_repo = merge_request.source_project.repository.raw + @resolver = Gitlab::Git::Conflict::Resolver.new(target_repo, our_commit.id, their_commit.id) @merge_request = merge_request end @@ -18,7 +18,7 @@ module Gitlab target_branch: merge_request.target_branch, commit_message: commit_message || default_commit_message } - resolver.resolve_conflicts(user, files, args) + resolver.resolve_conflicts(@source_repo, user, files, args) ensure @merge_request.clear_memoized_shas end diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index de8cce41b6d..03e5c0fcd6f 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -5,38 +5,31 @@ module Gitlab ConflictSideMissing = Class.new(StandardError) ResolutionError = Class.new(StandardError) - def initialize(repository, our_commit, target_repository, their_commit) - @repository = repository - @our_commit = our_commit.rugged_commit + def initialize(target_repository, our_commit_oid, their_commit_oid) @target_repository = target_repository - @their_commit = their_commit.rugged_commit + @our_commit_oid = our_commit_oid + @their_commit_oid = their_commit_oid end def conflicts @conflicts ||= begin - target_index = @target_repository.rugged.merge_commits(@our_commit, @their_commit) + target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) # We don't need to do `with_repo_branch_commit` here, because the target # project always fetches source refs when creating merge request diffs. - target_index.conflicts.map do |conflict| - raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours] - - Gitlab::Git::Conflict::File.new( - @target_repository, - @our_commit.oid, - conflict, - target_index.merge_file(conflict[:ours][:path])[:data] - ) - end + conflict_files(@target_repository, target_index) end end - def resolve_conflicts(user, files, source_branch:, target_branch:, commit_message:) - @repository.with_repo_branch_commit(@target_repository, target_branch) do + def resolve_conflicts(source_repository, user, files, source_branch:, target_branch:, commit_message:) + source_repository.with_repo_branch_commit(@target_repository, target_branch) do + index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) + conflicts = conflict_files(source_repository, index) + files.each do |file_params| - conflict_file = conflict_for_path(file_params[:old_path], file_params[:new_path]) + conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path]) - write_resolved_file_to_index(conflict_file, file_params) + write_resolved_file_to_index(source_repository, index, conflict_file, file_params) end unless index.conflicts.empty? @@ -47,14 +40,14 @@ module Gitlab commit_params = { message: commit_message, - parents: [@our_commit, @their_commit].map(&:oid) + parents: [@our_commit_oid, @their_commit_oid] } - @repository.commit_index(user, source_branch, index, commit_params) + source_repository.commit_index(user, source_branch, index, commit_params) end end - def conflict_for_path(old_path, new_path) + def conflict_for_path(conflicts, old_path, new_path) conflicts.find do |conflict| conflict.their_path == old_path && conflict.our_path == new_path end @@ -62,15 +55,20 @@ module Gitlab private - # We can only write when getting the merge index from the source - # project, because we will write to that project. We don't use this all - # the time because this fetches a ref into the source project, which - # isn't needed for reading. - def index - @index ||= @repository.rugged.merge_commits(@our_commit, @their_commit) + def conflict_files(repository, index) + index.conflicts.map do |conflict| + raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours] + + Gitlab::Git::Conflict::File.new( + repository, + @our_commit_oid, + conflict, + index.merge_file(conflict[:ours][:path])[:data] + ) + end end - def write_resolved_file_to_index(file, params) + def write_resolved_file_to_index(repository, index, file, params) if params[:sections] resolved_lines = file.resolve_lines(params[:sections]) new_file = resolved_lines.map { |line| line[:full_line] }.join("\n") @@ -82,7 +80,8 @@ module Gitlab our_path = file.our_path - index.add(path: our_path, oid: @repository.rugged.write(new_file, :blob), mode: file.our_mode) + oid = repository.rugged.write(new_file, :blob) + index.add(path: our_path, oid: oid, mode: file.our_mode) index.conflict_remove(our_path) end end -- cgit v1.2.1 From 02994fbe7715ef4539f720b6d395eeb9a3d71f14 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 15 Dec 2017 19:37:57 +0800 Subject: Backport changes from EE --- lib/extracts_path.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index aa9996c7685..d8aca3304c5 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -139,7 +139,7 @@ module ExtractsPath def lfs_blob_ids blob_ids = tree.blobs.map(&:id) - @lfs_blob_ids = Gitlab::Git::Blob.batch_lfs_pointers(@project.repository, blob_ids).map(&:id) + @lfs_blob_ids = Gitlab::Git::Blob.batch_lfs_pointers(@project.repository, blob_ids).map(&:id) # rubocop:disable Gitlab/ModuleWithInstanceVariables end private -- cgit v1.2.1 From 481b8a71f8ee63758d26a57a6367c091d4b76b09 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 13 Dec 2017 18:02:49 +0100 Subject: Make sure user email is read only when synced with LDAP --- lib/gitlab/ldap/user.rb | 4 ---- lib/gitlab/o_auth/provider.rb | 12 ++++++++++++ lib/gitlab/o_auth/user.rb | 38 ++++++++++++++++++++------------------ 3 files changed, 32 insertions(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index 3945df27eed..84ee94e38e4 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -36,10 +36,6 @@ module Gitlab ldap_config.block_auto_created_users end - def sync_profile_from_provider? - true - end - def allowed? Gitlab::LDAP::Access.allowed?(gl_user) end diff --git a/lib/gitlab/o_auth/provider.rb b/lib/gitlab/o_auth/provider.rb index ac9d66c836d..657db29c85a 100644 --- a/lib/gitlab/o_auth/provider.rb +++ b/lib/gitlab/o_auth/provider.rb @@ -19,6 +19,18 @@ module Gitlab name.to_s.start_with?('ldap') end + def self.sync_profile_from_provider?(provider) + return true if ldap_provider?(provider) + + providers = Gitlab.config.omniauth.sync_profile_from_provider + + if providers.is_a?(Array) + providers.include?(provider) + else + providers + end + end + def self.config_for(name) name = name.to_s if ldap_provider?(name) diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 552133234a3..d33f33d192f 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -12,7 +12,7 @@ module Gitlab def initialize(auth_hash) self.auth_hash = auth_hash - update_profile if sync_profile_from_provider? + update_profile add_or_update_user_identities end @@ -195,29 +195,31 @@ module Gitlab end def sync_profile_from_provider? - providers = Gitlab.config.omniauth.sync_profile_from_provider - - if providers.is_a?(Array) - providers.include?(auth_hash.provider) - else - providers - end + Gitlab::OAuth::Provider.sync_profile_from_provider?(auth_hash.provider) end def update_profile - user_synced_attributes_metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata - - UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key| - if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) - gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend - user_synced_attributes_metadata.set_attribute_synced(key, true) - else - user_synced_attributes_metadata.set_attribute_synced(key, false) + return unless sync_profile_from_provider? || creating_linked_ldap_user? + + metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata + + if sync_profile_from_provider? + UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key| + if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) + gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend + metadata.set_attribute_synced(key, true) + else + metadata.set_attribute_synced(key, false) + end end + + metadata.provider = auth_hash.provider end - user_synced_attributes_metadata.provider = auth_hash.provider - gl_user.user_synced_attributes_metadata = user_synced_attributes_metadata + if creating_linked_ldap_user? && gl_user.email == ldap_person.email.first + metadata.set_attribute_synced(:email, true) + metadata.provider = ldap_person.provider + end end def log -- cgit v1.2.1 From c6edae38870a4228e3b964d647b9ef588df11f27 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 5 Dec 2017 14:15:30 +0100 Subject: Load commit in batches for pipelines#index Uses `list_commits_by_oid` on the CommitService, to request the needed commits for pipelines. These commits are needed to display the user that created the commit and the commit title. This includes fixes for tests failing that depended on the commit being `nil`. However, now these are batch loaded, this doesn't happen anymore and the commits are an instance of BatchLoader. --- lib/gitlab/git/commit.rb | 13 +++++++++++++ lib/gitlab/gitaly_client/commit_service.rb | 9 +++++++++ 2 files changed, 22 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index e90b158fb34..145721dea76 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -228,6 +228,19 @@ module Gitlab end end end + + # Only to be used when the object ids will not necessarily have a + # relation to each other. The last 10 commits for a branch for example, + # should go through .where + def batch_by_oid(repo, oids) + repo.gitaly_migrate(:list_commits_by_oid) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.list_commits_by_oid(oids) + else + oids.map { |oid| find(repo, oid) }.compact + end + end + end end def initialize(repository, raw_commit, head = nil) diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 7985f5b5457..fb3e27770b4 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -169,6 +169,15 @@ module Gitlab consume_commits_response(response) end + def list_commits_by_oid(oids) + request = Gitaly::ListCommitsByOidRequest.new(repository: @gitaly_repo, oid: oids) + + response = GitalyClient.call(@repository.storage, :commit_service, :list_commits_by_oid, request, timeout: GitalyClient.medium_timeout) + consume_commits_response(response) + rescue GRPC::Unknown # If no repository is found, happens mainly during testing + [] + end + def commits_by_message(query, revision: '', path: '', limit: 1000, offset: 0) request = Gitaly::CommitsByMessageRequest.new( repository: @gitaly_repo, -- cgit v1.2.1 From 3c545133e8f23b57698046bae8be23e2bc457aca Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 19 Dec 2017 17:45:25 +0100 Subject: Fix tests and formatting --- lib/gitlab/metrics/influx_db.rb | 1 + lib/gitlab/metrics/method_call.rb | 2 +- lib/gitlab/metrics/system.rb | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index bdf7910b7c7..153e236d018 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -38,6 +38,7 @@ module Gitlab # This is memoized since this method is called for every instrumented # method. Loading data from an external cache on every method call slows # things down too much. + # in milliseconds @method_call_threshold ||= settings[:method_call_threshold] end diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 6fb8f564237..a030092df37 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -72,7 +72,7 @@ module Gitlab # Returns true if the total runtime of this method exceeds the method call # threshold. def above_threshold? - real_time >= Metrics.method_call_threshold + real_time_milliseconds >= Metrics.method_call_threshold end def call_measurement_enabled? diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb index b31cc6236d1..4852017bf38 100644 --- a/lib/gitlab/metrics/system.rb +++ b/lib/gitlab/metrics/system.rb @@ -55,7 +55,6 @@ module Gitlab # # Returns the time as a Float. def self.monotonic_time - Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) end end -- cgit v1.2.1 From 3e4b45fc216875ff25647675d92448a53a740d9b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 14 Dec 2017 13:32:55 -0600 Subject: Only include the user's ID in the time_spent command's update hash Previously, this would include the entire User record in the update hash, which was rendered in the response using `to_json`, erroneously exposing every attribute of that record, including their (now removed) private token. Now we only include the user ID, and perform the lookup on-demand. --- lib/api/time_tracking_endpoints.rb | 4 ++-- lib/api/v3/time_tracking_endpoints.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb index df4632346dd..2bb451dea89 100644 --- a/lib/api/time_tracking_endpoints.rb +++ b/lib/api/time_tracking_endpoints.rb @@ -85,7 +85,7 @@ module API update_issuable(spend_time: { duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)), - user: current_user + user_id: current_user.id }) end @@ -97,7 +97,7 @@ module API authorize! update_issuable_key, load_issuable status :ok - update_issuable(spend_time: { duration: :reset, user: current_user }) + update_issuable(spend_time: { duration: :reset, user_id: current_user.id }) end desc "Show time stats for a project #{issuable_name}" diff --git a/lib/api/v3/time_tracking_endpoints.rb b/lib/api/v3/time_tracking_endpoints.rb index d5b90e435ba..1aad39815f9 100644 --- a/lib/api/v3/time_tracking_endpoints.rb +++ b/lib/api/v3/time_tracking_endpoints.rb @@ -86,7 +86,7 @@ module API update_issuable(spend_time: { duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)), - user: current_user + user_id: current_user.id }) end @@ -98,7 +98,7 @@ module API authorize! update_issuable_key, load_issuable status :ok - update_issuable(spend_time: { duration: :reset, user: current_user }) + update_issuable(spend_time: { duration: :reset, user_id: current_user.id }) end desc "Show time stats for a project #{issuable_name}" -- cgit v1.2.1 From c9732b3c6dc07b4ffd0df9c2ae88e1f67cd56243 Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Tue, 19 Dec 2017 21:57:46 +0000 Subject: Employ declared_params in finder methods for MR and Issue API lists - Ensure that unwanted params are no passed to actual finder classes --- lib/api/issues.rb | 2 +- lib/api/merge_requests.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 5f943ba27d1..b29c5848aef 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -8,7 +8,7 @@ module API helpers do def find_issues(args = {}) - args = params.merge(args) + args = declared_params.merge(args) args.delete(:id) args[:milestone_title] = args.delete(:milestone) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index d34886fca2e..c95e28edbef 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -8,7 +8,7 @@ module API helpers do def find_merge_requests(args = {}) - args = params.merge(args) + args = declared_params.merge(args) args[:milestone_title] = args.delete(:milestone) args[:label_name] = args.delete(:labels) -- cgit v1.2.1 From 055543b915c13df57e13629cd49ca9d63b6e3e76 Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Tue, 19 Dec 2017 22:00:17 +0000 Subject: Add optional `search` param for Merge Requests API --- lib/api/merge_requests.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index c95e28edbef..02f2b75ab9d 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -41,6 +41,7 @@ module API optional :scope, type: String, values: %w[created-by-me assigned-to-me all], desc: 'Return merge requests for the given scope: `created-by-me`, `assigned-to-me` or `all`' optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji' + optional :search, type: String, desc: 'Search merge requests for text present in the title or description' use :pagination end end -- cgit v1.2.1 From ed715b7926c31fddb1c835823d509672663e23e6 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 19 Dec 2017 19:40:43 +0100 Subject: use in_milliseconds rails helper --- lib/gitlab/metrics/method_call.rb | 4 ++-- lib/gitlab/metrics/transaction.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index a030092df37..bb39b1d5462 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -27,7 +27,7 @@ module Gitlab @transaction = transaction @name = name @labels = { module: @module_name, method: @method_name } - @real_time_seconds = 0 + @real_time_seconds = 0.0 @cpu_time = 0 @call_count = 0 end @@ -53,7 +53,7 @@ module Gitlab end def real_time_milliseconds - (real_time_seconds * 1000.0).to_i + real_time_seconds.in_milliseconds.to_i end # Returns a Metric instance of the current method call. diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb index 16f0969ab74..e7975c023a9 100644 --- a/lib/gitlab/metrics/transaction.rb +++ b/lib/gitlab/metrics/transaction.rb @@ -36,7 +36,7 @@ module Gitlab end def duration_milliseconds - (duration * 1000).to_i + duration.in_milliseconds.to_i end def allocated_memory -- cgit v1.2.1 From 338f1eaf354a31663afbda3d91776f7f506b90f1 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 20 Dec 2017 19:13:11 +0100 Subject: Migrate to Project#empty_repo? --- lib/gitlab/github_import/importer/repository_importer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/github_import/importer/repository_importer.rb b/lib/gitlab/github_import/importer/repository_importer.rb index 9cf2e7fd871..7dd68a0d1cd 100644 --- a/lib/gitlab/github_import/importer/repository_importer.rb +++ b/lib/gitlab/github_import/importer/repository_importer.rb @@ -29,7 +29,7 @@ module Gitlab # this code, e.g. because we had to retry this job after # `import_wiki?` raised a rate limit error. In this case we'll skip # re-importing the main repository. - if project.repository.empty_repo? + if project.empty_repo? import_repository else true -- cgit v1.2.1 From 28fba5ed99c6f8b4e7e534f9c2046d1c5ab38607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20Carlb=C3=A4cker?= Date: Wed, 20 Dec 2017 18:29:52 +0000 Subject: Revert "Merge branch 'repo-write-ref-client-prep' into 'master'" This reverts merge request !15712 --- lib/gitlab/git/repository.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 848a782446a..044c60caa05 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1101,17 +1101,12 @@ module Gitlab end end - def write_ref(ref_path, ref, force: false) + def write_ref(ref_path, ref) raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ') raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00") - ref = "refs/heads/#{ref}" unless ref.start_with?("refs") || ref =~ /\A[a-f0-9]+\z/i - - rugged.references.create(ref_path, ref, force: force) - rescue Rugged::ReferenceError => ex - raise GitError, "could not create ref #{ref_path}: #{ex}" - rescue Rugged::OSError => ex - raise GitError, "could not create ref #{ref_path}: #{ex}" + input = "update #{ref_path}\x00#{ref}\x00\x00" + run_git!(%w[update-ref --stdin -z]) { |stdin| stdin.write(input) } end def fetch_ref(source_repository, source_ref:, target_ref:) -- cgit v1.2.1 From 040167f0724027020f2d63b6e43481fb3e29dbfc Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 20 Dec 2017 19:30:58 +0100 Subject: Use seconds where possible, and convert to milliseconds for Influxdb consumption --- lib/gitlab/metrics/method_call.rb | 24 ++++++++++-------------- lib/gitlab/metrics/system.rb | 8 ++++---- 2 files changed, 14 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index bb39b1d5462..f4a916f154d 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -4,7 +4,7 @@ module Gitlab class MethodCall MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze - attr_reader :real_time_seconds, :cpu_time, :call_count, :labels + attr_reader :real_time, :cpu_time, :call_count, :labels def self.call_duration_histogram return @call_duration_histogram if @call_duration_histogram @@ -27,42 +27,38 @@ module Gitlab @transaction = transaction @name = name @labels = { module: @module_name, method: @method_name } - @real_time_seconds = 0.0 - @cpu_time = 0 + @real_time = 0.0 + @cpu_time = 0.0 @call_count = 0 end # Measures the real and CPU execution time of the supplied block. def measure - start_real_seconds = System.monotonic_time + start_real = System.monotonic_time start_cpu = System.cpu_time retval = yield - real_time_seconds = System.monotonic_time - start_real_seconds + real_time = System.monotonic_time - start_real cpu_time = System.cpu_time - start_cpu - @real_time_seconds += real_time_seconds + @real_time += real_time @cpu_time += cpu_time @call_count += 1 if call_measurement_enabled? && above_threshold? - self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time_seconds) + self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time) end retval end - def real_time_milliseconds - real_time_seconds.in_milliseconds.to_i - end - # Returns a Metric instance of the current method call. def to_metric Metric.new( Instrumentation.series, { - duration: real_time_milliseconds, - cpu_duration: cpu_time, + duration: real_time.in_milliseconds.to_i, + cpu_duration: cpu_time.in_milliseconds.to_i, call_count: call_count }, method: @name @@ -72,7 +68,7 @@ module Gitlab # Returns true if the total runtime of this method exceeds the method call # threshold. def above_threshold? - real_time_milliseconds >= Metrics.method_call_threshold + real_time.in_milliseconds >= Metrics.method_call_threshold end def call_measurement_enabled? diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb index 4852017bf38..e60e245cf89 100644 --- a/lib/gitlab/metrics/system.rb +++ b/lib/gitlab/metrics/system.rb @@ -35,19 +35,19 @@ module Gitlab if Process.const_defined?(:CLOCK_THREAD_CPUTIME_ID) def self.cpu_time Process - .clock_gettime(Process::CLOCK_THREAD_CPUTIME_ID, :millisecond) + .clock_gettime(Process::CLOCK_THREAD_CPUTIME_ID, :float_second) end else def self.cpu_time Process - .clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID, :millisecond) + .clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID, :float_second) end end # Returns the current real time in a given precision. # - # Returns the time as a Fixnum. - def self.real_time(precision = :millisecond) + # Returns the time as a Float for precision = :float_second. + def self.real_time(precision = :float_second) Process.clock_gettime(Process::CLOCK_REALTIME, precision) end -- cgit v1.2.1