diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-20 13:49:51 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-20 13:49:51 +0000 |
| commit | 71786ddc8e28fbd3cb3fcc4b3ff15e5962a1c82e (patch) | |
| tree | 6a2d93ef3fb2d353bb7739e4b57e6541f51cdd71 /app/services | |
| parent | a7253423e3403b8c08f8a161e5937e1488f5f407 (diff) | |
| download | gitlab-ce-71786ddc8e28fbd3cb3fcc4b3ff15e5962a1c82e.tar.gz | |
Add latest changes from gitlab-org/gitlab@15-9-stable-eev15.9.0-rc42
Diffstat (limited to 'app/services')
132 files changed, 1253 insertions, 501 deletions
diff --git a/app/services/analytics/cycle_analytics/stages/base_service.rb b/app/services/analytics/cycle_analytics/stages/base_service.rb index b676eff0a0b..0f5415c9f9e 100644 --- a/app/services/analytics/cycle_analytics/stages/base_service.rb +++ b/app/services/analytics/cycle_analytics/stages/base_service.rb @@ -37,7 +37,7 @@ module Analytics end def value_stream - @value_stream ||= params[:value_stream] + @value_stream ||= params.fetch(:value_stream) end end end diff --git a/app/services/analytics/cycle_analytics/stages/list_service.rb b/app/services/analytics/cycle_analytics/stages/list_service.rb index a6b94ef8295..1cd7d3f5c6d 100644 --- a/app/services/analytics/cycle_analytics/stages/list_service.rb +++ b/app/services/analytics/cycle_analytics/stages/list_service.rb @@ -13,7 +13,7 @@ module Analytics private def allowed? - can?(current_user, :read_cycle_analytics, parent) + can?(current_user, :read_cycle_analytics, parent.project) end def success(stages) diff --git a/app/services/authorized_project_update/project_access_changed_service.rb b/app/services/authorized_project_update/project_access_changed_service.rb index dafec1fef59..ca039187c50 100644 --- a/app/services/authorized_project_update/project_access_changed_service.rb +++ b/app/services/authorized_project_update/project_access_changed_service.rb @@ -6,16 +6,12 @@ module AuthorizedProjectUpdate @project_ids = Array.wrap(project_ids) end - def execute(blocking: true) + def execute return if @project_ids.empty? bulk_args = @project_ids.map { |id| [id] } - if blocking - AuthorizedProjectUpdate::ProjectRecalculateWorker.bulk_perform_and_wait(bulk_args) - else - AuthorizedProjectUpdate::ProjectRecalculateWorker.bulk_perform_async(bulk_args) # rubocop:disable Scalability/BulkPerformWithContext - end + AuthorizedProjectUpdate::ProjectRecalculateWorker.bulk_perform_async(bulk_args) # rubocop:disable Scalability/BulkPerformWithContext end end end diff --git a/app/services/boards/issues/create_service.rb b/app/services/boards/issues/create_service.rb index e3d4da7fb07..77e297b6b11 100644 --- a/app/services/boards/issues/create_service.rb +++ b/app/services/boards/issues/create_service.rb @@ -32,7 +32,7 @@ module Boards def create_issue(params) # NOTE: We are intentionally not doing a spam/CAPTCHA check for issues created via boards. # See https://gitlab.com/gitlab-org/gitlab/-/issues/29400#note_598479184 for more context. - ::Issues::CreateService.new(project: project, current_user: current_user, params: params, spam_params: nil).execute + ::Issues::CreateService.new(container: project, current_user: current_user, params: params, spam_params: nil).execute end end end diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 4de4d7c8f69..e80ff9cf857 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -52,7 +52,7 @@ module Boards end def update(issue, issue_modification_params) - ::Issues::UpdateService.new(project: issue.project, current_user: current_user, params: issue_modification_params).execute(issue) + ::Issues::UpdateService.new(container: issue.project, current_user: current_user, params: issue_modification_params).execute(issue) end def moving_to_list_items_relation diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index 35a35e7b7c9..ac019d9ec5b 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -70,33 +70,52 @@ module BulkImports ) bulk_import.create_configuration!(credentials.slice(:url, :access_token)) - Array.wrap(params).each do |entity| - track_access_level(entity) + Array.wrap(params).each do |entity_params| + track_access_level(entity_params) + + validate_destination_full_path(entity_params) BulkImports::Entity.create!( bulk_import: bulk_import, - source_type: entity[:source_type], - source_full_path: entity[:source_full_path], - destination_slug: entity[:destination_slug], - destination_namespace: entity[:destination_namespace], - migrate_projects: Gitlab::Utils.to_boolean(entity[:migrate_projects], default: true) + source_type: entity_params[:source_type], + source_full_path: entity_params[:source_full_path], + destination_slug: entity_params[:destination_slug] || entity_params[:destination_name], + destination_namespace: entity_params[:destination_namespace], + migrate_projects: Gitlab::Utils.to_boolean(entity_params[:migrate_projects], default: true) ) end - bulk_import end end - def track_access_level(entity) + def track_access_level(entity_params) Gitlab::Tracking.event( self.class.name, 'create', label: 'import_access_level', user: current_user, - extra: { user_role: user_role(entity[:destination_namespace]), import_type: 'bulk_import_group' } + extra: { user_role: user_role(entity_params[:destination_namespace]), import_type: 'bulk_import_group' } ) end + def validate_destination_full_path(entity_params) + source_type = entity_params[:source_type] + + full_path = [ + entity_params[:destination_namespace], + entity_params[:destination_slug] || entity_params[:destination_name] + ].reject(&:blank?).join('/') + + case source_type + when 'group_entity' + return if Namespace.find_by_full_path(full_path).nil? + when 'project_entity' + return if Project.find_by_full_path(full_path).nil? + end + + raise BulkImports::Error.destination_full_path_validation_failure(full_path) + end + def user_role(destination_namespace) namespace = Namespace.find_by_full_path(destination_namespace) # if there is no parent namespace we assume user will be group creator/owner diff --git a/app/services/ci/archive_trace_service.rb b/app/services/ci/archive_trace_service.rb index 3d548c824c8..4b62580e670 100644 --- a/app/services/ci/archive_trace_service.rb +++ b/app/services/ci/archive_trace_service.rb @@ -2,6 +2,36 @@ module Ci class ArchiveTraceService + include ::Gitlab::ExclusiveLeaseHelpers + + EXCLUSIVE_LOCK_KEY = 'archive_trace_service:batch_execute:lock' + LOCK_TIMEOUT = 56.minutes + LOOP_TIMEOUT = 55.minutes + LOOP_LIMIT = 2000 + BATCH_SIZE = 100 + + # rubocop: disable CodeReuse/ActiveRecord + def batch_execute(worker_name:) + start_time = Time.current + in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do + Ci::Build.with_stale_live_trace.find_each(batch_size: BATCH_SIZE).with_index do |build, index| + break if Time.current - start_time > LOOP_TIMEOUT + + if index > LOOP_LIMIT + Sidekiq.logger.warn(class: worker_name, message: 'Loop limit reached.', job_id: build.id) + break + end + + begin + execute(build, worker_name: worker_name) + rescue StandardError + next + end + end + end + end + # rubocop: enable CodeReuse/ActiveRecord + def execute(job, worker_name:) unless job.trace.archival_attempts_available? Sidekiq.logger.warn(class: worker_name, message: 'The job is out of archival attempts.', job_id: job.id) diff --git a/app/services/ci/components/fetch_service.rb b/app/services/ci/components/fetch_service.rb new file mode 100644 index 00000000000..45abb415174 --- /dev/null +++ b/app/services/ci/components/fetch_service.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Ci + module Components + class FetchService + include Gitlab::Utils::StrongMemoize + + TEMPLATE_FILE = 'template.yml' + + COMPONENT_PATHS = [ + ::Gitlab::Ci::Components::InstancePath + ].freeze + + def initialize(address:, current_user:) + @address = address + @current_user = current_user + end + + def execute + unless component_path_class + return ServiceResponse.error( + message: "#{error_prefix} the component path is not supported", + reason: :unsupported_path) + end + + component_path = component_path_class.new(address: address, content_filename: TEMPLATE_FILE) + content = component_path.fetch_content!(current_user: current_user) + + if content.present? + ServiceResponse.success(payload: { content: content, path: component_path }) + else + ServiceResponse.error(message: "#{error_prefix} content not found", reason: :content_not_found) + end + rescue Gitlab::Access::AccessDeniedError + ServiceResponse.error( + message: "#{error_prefix} project does not exist or you don't have sufficient permissions", + reason: :not_allowed) + end + + private + + attr_reader :current_user, :address + + def component_path_class + COMPONENT_PATHS.find { |klass| klass.match?(address) } + end + strong_memoize_attr :component_path_class + + def error_prefix + "component '#{address}' -" + end + end + end +end diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index 3d0a7fb99ea..b281f942a14 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -89,7 +89,7 @@ module Ci return false end - if Feature.enabled?(:ci_limit_complete_hierarchy_size) && pipeline_tree_too_large? + if pipeline_tree_too_large? @bridge.drop!(:reached_max_pipeline_hierarchy_size) return false end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index eb25aeaf5a5..390675ab80b 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -33,7 +33,6 @@ module Ci Gitlab::Ci::Pipeline::Chain::EnsureEnvironments, Gitlab::Ci::Pipeline::Chain::EnsureResourceGroups, Gitlab::Ci::Pipeline::Chain::Create, - Gitlab::Ci::Pipeline::Chain::CreateDeployments, Gitlab::Ci::Pipeline::Chain::CreateCrossDatabaseAssociations, Gitlab::Ci::Pipeline::Chain::Limit::Activity, Gitlab::Ci::Pipeline::Chain::CancelPendingPipelines, diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index 6e2ba76682f..3d19fec6617 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -132,8 +132,6 @@ module Ci job.update_column(:artifacts_expire_at, artifact.expire_at) end - Gitlab::Ci::Artifacts::Logger.log_created(artifact) - success(artifact: artifact) rescue ActiveRecord::RecordNotUnique => error track_exception(error, params) diff --git a/app/services/ci/job_token_scope/add_project_service.rb b/app/services/ci/job_token_scope/add_project_service.rb index d03ae434b69..15553ad6e92 100644 --- a/app/services/ci/job_token_scope/add_project_service.rb +++ b/app/services/ci/job_token_scope/add_project_service.rb @@ -5,10 +5,14 @@ module Ci class AddProjectService < ::BaseService include EditScopeValidations - def execute(target_project) + def execute(target_project, direction: :outbound) + direction = :outbound if Feature.disabled?(:ci_inbound_job_token_scope) + validate_edit!(project, target_project, current_user) - link = add_project!(target_project) + link = allowlist(direction) + .add!(target_project, user: current_user) + ServiceResponse.success(payload: { project_link: link }) rescue ActiveRecord::RecordNotUnique @@ -19,12 +23,10 @@ module Ci ServiceResponse.error(message: e.message) end - def add_project!(target_project) - ::Ci::JobToken::ProjectScopeLink.create!( - source_project: project, - target_project: target_project, - added_by: current_user - ) + private + + def allowlist(direction) + Ci::JobToken::Allowlist.new(project, direction: direction) end end end diff --git a/app/services/ci/job_token_scope/remove_project_service.rb b/app/services/ci/job_token_scope/remove_project_service.rb index 15644e529d9..864f9318c68 100644 --- a/app/services/ci/job_token_scope/remove_project_service.rb +++ b/app/services/ci/job_token_scope/remove_project_service.rb @@ -5,14 +5,16 @@ module Ci class RemoveProjectService < ::BaseService include EditScopeValidations - def execute(target_project) + def execute(target_project, direction) validate_edit!(project, target_project, current_user) if project == target_project return ServiceResponse.error(message: "Source project cannot be removed from the job token scope") end - link = ::Ci::JobToken::ProjectScopeLink.for_source_and_target(project, target_project) + link = ::Ci::JobToken::ProjectScopeLink + .with_access_direction(direction) + .for_source_and_target(project, target_project) unless link return ServiceResponse.error(message: "Target project is not in the job token scope") diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index df4963d1b33..dbea270b7c6 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -17,7 +17,9 @@ module Ci new(project, user) end - def execute(sha) + def execute(ref) + sha = project.commit(ref).try(:sha) + with_reactive_cache(sha) { |result| result } end diff --git a/app/services/ci/parse_dotenv_artifact_service.rb b/app/services/ci/parse_dotenv_artifact_service.rb index 14e8dc41cf5..d4d5acef44e 100644 --- a/app/services/ci/parse_dotenv_artifact_service.rb +++ b/app/services/ci/parse_dotenv_artifact_service.rb @@ -3,6 +3,7 @@ module Ci class ParseDotenvArtifactService < ::BaseService include ::Gitlab::Utils::StrongMemoize + include ::Gitlab::EncodingHelper SizeLimitError = Class.new(StandardError) ParserError = Class.new(StandardError) @@ -36,6 +37,10 @@ module Ci variables = {} artifact.each_blob do |blob| + # Windows powershell may output UTF-16LE files, so convert the whole file + # to UTF-8 before proceeding. + blob = strip_bom(encode_utf8_with_replacement_character(blob)) + blob.each_line do |line| key, value = scan_line!(line) diff --git a/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb b/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb index 57b663dc293..f392681eb85 100644 --- a/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb +++ b/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb @@ -13,7 +13,7 @@ module Ci return if pipeline.has_codequality_mr_diff_report? return unless new_errors_introduced? - pipeline.pipeline_artifacts.create!(**artifact_attributes) + Ci::PipelineArtifact.create_or_replace_for_pipeline!(**artifact_attributes) end private @@ -24,12 +24,10 @@ module Ci file = build_carrierwave_file! { - project_id: pipeline.project_id, + pipeline: pipeline, file_type: :code_quality_mr_diff, - file_format: Ci::PipelineArtifact::REPORT_TYPES.fetch(:code_quality_mr_diff), size: file["tempfile"].size, file: file, - expire_at: Ci::PipelineArtifact::EXPIRATION_DATE.from_now, locked: pipeline.locked } end diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb new file mode 100644 index 00000000000..48c3e6490ae --- /dev/null +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Ci + module PipelineCreation + class CancelRedundantPipelinesService + include Gitlab::Utils::StrongMemoize + + BATCH_SIZE = 25 + + def initialize(pipeline) + @pipeline = pipeline + @project = @pipeline.project + end + + # rubocop: disable CodeReuse/ActiveRecord + def execute + return if pipeline.parent_pipeline? # skip if child pipeline + return unless project.auto_cancel_pending_pipelines? + + Gitlab::OptimisticLocking + .retry_lock(parent_and_child_pipelines, name: 'cancel_pending_pipelines') do |cancelables| + cancelables.select(:id).each_batch(of: BATCH_SIZE) do |cancelables_batch| + auto_cancel_interruptible_pipelines(cancelables_batch.ids) + end + end + end + + private + + attr_reader :pipeline, :project + + def parent_auto_cancelable_pipelines + project.all_pipelines + .created_after(1.week.ago) + .for_ref(pipeline.ref) + .where_not_sha(project.commit(pipeline.ref).try(:id)) + .where("created_at < ?", pipeline.created_at) + .ci_sources + end + + def parent_and_child_pipelines + Ci::Pipeline.object_hierarchy(parent_auto_cancelable_pipelines, project_condition: :same) + .base_and_descendants + .alive_or_scheduled + end + # rubocop: enable CodeReuse/ActiveRecord + + def auto_cancel_interruptible_pipelines(pipeline_ids) + ::Ci::Pipeline + .id_in(pipeline_ids) + .with_only_interruptible_builds + .each do |cancelable_pipeline| + Gitlab::AppLogger.info( + class: self.class.name, + message: "Pipeline #{pipeline.id} auto-canceling pipeline #{cancelable_pipeline.id}", + canceled_pipeline_id: cancelable_pipeline.id, + canceled_by_pipeline_id: pipeline.id, + canceled_by_pipeline_source: pipeline.source + ) + + # cascade_to_children not needed because we iterate through descendants here + cancelable_pipeline.cancel_running( + auto_canceled_by_pipeline_id: pipeline.id, + cascade_to_children: false + ) + end + end + end + end +end diff --git a/app/services/ci/pipeline_processing/atomic_processing_service.rb b/app/services/ci/pipeline_processing/atomic_processing_service.rb index 508d9c3f2e1..2b8eb104be5 100644 --- a/app/services/ci/pipeline_processing/atomic_processing_service.rb +++ b/app/services/ci/pipeline_processing/atomic_processing_service.rb @@ -42,13 +42,14 @@ module Ci end def update_stages! - pipeline.stages.ordered.each(&method(:update_stage!)) + pipeline.stages.ordered.each { |stage| update_stage!(stage) } end def update_stage!(stage) # Update processables for a given stage in bulk/slices - ids = @collection.created_processable_ids_for_stage_position(stage.position) - ids.in_groups_of(BATCH_SIZE, false, &method(:update_processables!)) + @collection + .created_processable_ids_for_stage_position(stage.position) + .in_groups_of(BATCH_SIZE, false) { |ids| update_processables!(ids) } status = @collection.status_for_stage_position(stage.position) stage.set_status(status) @@ -62,7 +63,7 @@ module Ci .ordered_by_stage .select_with_aggregated_needs(project) - created_processables.each(&method(:update_processable!)) + created_processables.each { |processable| update_processable!(processable) } end def update_pipeline! diff --git a/app/services/ci/pipeline_schedules/update_service.rb b/app/services/ci/pipeline_schedules/update_service.rb new file mode 100644 index 00000000000..2412b5cbd81 --- /dev/null +++ b/app/services/ci/pipeline_schedules/update_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Ci + module PipelineSchedules + class UpdateService + def initialize(schedule, user, params) + @schedule = schedule + @user = user + @params = params + end + + def execute + return forbidden unless allowed? + + if schedule.update(@params) + ServiceResponse.success(payload: schedule) + else + ServiceResponse.error(message: schedule.errors.full_messages) + end + end + + private + + attr_reader :schedule, :user + + def allowed? + user.can?(:update_pipeline_schedule, schedule) + end + + def forbidden + ServiceResponse.error( + message: _('The current user is not authorized to update the pipeline schedule'), + reason: :forbidden + ) + end + end + end +end diff --git a/app/services/ci/prometheus_metrics/observe_histograms_service.rb b/app/services/ci/prometheus_metrics/observe_histograms_service.rb index 6bd3d2121ba..10b3d61247b 100644 --- a/app/services/ci/prometheus_metrics/observe_histograms_service.rb +++ b/app/services/ci/prometheus_metrics/observe_histograms_service.rb @@ -27,7 +27,7 @@ module Ci def execute params .fetch(:histograms, []) - .each(&method(:observe)) + .each { |data| observe(data) } ServiceResponse.success(http_status: :created) end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index cd879e9bc07..205da2632c2 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -6,7 +6,7 @@ module Ci class RegisterJobService include ::Gitlab::Ci::Artifacts::Logger - attr_reader :runner, :metrics + attr_reader :runner, :runner_machine, :metrics TEMPORARY_LOCK_TIMEOUT = 3.seconds @@ -18,8 +18,9 @@ module Ci # affect 5% of the worst case scenarios. MAX_QUEUE_DEPTH = 45 - def initialize(runner) + def initialize(runner, runner_machine) @runner = runner + @runner_machine = runner_machine @metrics = ::Gitlab::Ci::Queue::Metrics.new(runner) end @@ -243,6 +244,7 @@ module Ci def assign_runner!(build, params) build.runner_id = runner.id build.runner_session_attributes = params[:session] if params[:session].present? + build.ensure_metadata.runner_machine = runner_machine if runner_machine failure_reason, _ = pre_assign_runner_checks.find { |_, check| check.call(build, params) } @@ -260,7 +262,7 @@ module Ci end def acquire_temporary_lock(build_id) - return true unless Feature.enabled?(:ci_register_job_temporary_lock, runner) + return true if Feature.disabled?(:ci_register_job_temporary_lock, runner, type: :ops) key = "build/register/#{build_id}" diff --git a/app/services/ci/runners/create_runner_service.rb b/app/services/ci/runners/create_runner_service.rb new file mode 100644 index 00000000000..2de9ee4d38e --- /dev/null +++ b/app/services/ci/runners/create_runner_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Ci + module Runners + class CreateRunnerService + RUNNER_CLASS_MAPPING = { + 'instance_type' => Ci::Runners::RunnerCreationStrategies::InstanceRunnerStrategy, + nil => Ci::Runners::RunnerCreationStrategies::InstanceRunnerStrategy + }.freeze + + attr_accessor :user, :type, :params, :strategy + + def initialize(user:, type:, params:) + @user = user + @type = type + @params = params + @strategy = RUNNER_CLASS_MAPPING[type].new(user: user, type: type, params: params) + end + + def execute + normalize_params + + return ServiceResponse.error(message: 'Validation error') unless strategy.validate_params + return ServiceResponse.error(message: 'Insufficient permissions') unless strategy.authorized_user? + + runner = ::Ci::Runner.new(params) + + return ServiceResponse.success(payload: { runner: runner }) if runner.save + + ServiceResponse.error(message: runner.errors.full_messages) + end + + def normalize_params + params[:registration_type] = :authenticated_user + params[:runner_type] = type + params[:active] = !params.delete(:paused) if params[:paused].present? + params[:creator] = user + + strategy.normalize_params + end + end + end +end diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb index abd32610cec..db16b86d5e6 100644 --- a/app/services/ci/runners/register_runner_service.rb +++ b/app/services/ci/runners/register_runner_service.rb @@ -46,10 +46,10 @@ module Ci # Create shared runner. Requires admin access { runner_type: :instance_type } elsif runner_registrar_valid?('project') && project = ::Project.find_by_runners_token(registration_token) - # Create a specific runner for the project + # Create a project runner { runner_type: :project_type, scope: project } elsif runner_registrar_valid?('group') && group = ::Group.find_by_runners_token(registration_token) - # Create a specific runner for the group + # Create a group runner { runner_type: :group_type, scope: group } end end diff --git a/app/services/ci/runners/runner_creation_strategies/instance_runner_strategy.rb b/app/services/ci/runners/runner_creation_strategies/instance_runner_strategy.rb new file mode 100644 index 00000000000..f195c3e88f9 --- /dev/null +++ b/app/services/ci/runners/runner_creation_strategies/instance_runner_strategy.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Ci + module Runners + module RunnerCreationStrategies + class InstanceRunnerStrategy + attr_accessor :user, :type, :params + + def initialize(user:, type:, params:) + @user = user + @type = type + @params = params + end + + def normalize_params + params[:runner_type] = :instance_type + end + + def validate_params + true + end + + def authorized_user? + user.present? && user.can?(:create_instance_runners) + end + end + end + end +end diff --git a/app/services/ci/runners/stale_machines_cleanup_service.rb b/app/services/ci/runners/stale_machines_cleanup_service.rb new file mode 100644 index 00000000000..3e5706d24a6 --- /dev/null +++ b/app/services/ci/runners/stale_machines_cleanup_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Ci + module Runners + class StaleMachinesCleanupService + MAX_DELETIONS = 1000 + + def execute + ServiceResponse.success(payload: { + # the `stale` relationship can return duplicates, so we don't try to return a precise count here + deleted_machines: delete_stale_runner_machines > 0 + }) + end + + private + + def delete_stale_runner_machines + total_deleted_count = 0 + loop do + sub_batch_limit = [100, MAX_DELETIONS].min + + # delete_all discards part of the `stale` scope query, so we expliclitly wrap it with a SELECT as a workaround + deleted_count = Ci::RunnerMachine.id_in(Ci::RunnerMachine.stale.limit(sub_batch_limit)).delete_all + total_deleted_count += deleted_count + + break if deleted_count == 0 || total_deleted_count >= MAX_DELETIONS + end + + total_deleted_count + end + end + end +end diff --git a/app/services/clusters/agents/refresh_authorization_service.rb b/app/services/clusters/agents/refresh_authorization_service.rb index 53b14ab54da..23ececef6a1 100644 --- a/app/services/clusters/agents/refresh_authorization_service.rb +++ b/app/services/clusters/agents/refresh_authorization_service.rb @@ -58,7 +58,7 @@ module Clusters if project_entries allowed_projects.where_full_path_in(project_entries.keys).map do |project| - { project_id: project.id, config: project_entries[project.full_path] } + { project_id: project.id, config: project_entries[project.full_path.downcase] } end end end @@ -70,7 +70,7 @@ module Clusters if group_entries allowed_groups.where_full_path_in(group_entries.keys).map do |group| - { group_id: group.id, config: group_entries[group.full_path] } + { group_id: group.id, config: group_entries[group.full_path.downcase] } end end end @@ -79,7 +79,7 @@ module Clusters def extract_config_entries(entity:) config.dig('ci_access', entity) &.first(AUTHORIZED_ENTITY_LIMIT) - &.index_by { |config| config.delete('id') } + &.index_by { |config| config.delete('id').downcase } end def allowed_projects diff --git a/app/services/concerns/users/participable_service.rb b/app/services/concerns/users/participable_service.rb index 281b2508090..1a03b444b68 100644 --- a/app/services/concerns/users/participable_service.rb +++ b/app/services/concerns/users/participable_service.rb @@ -38,7 +38,7 @@ module Users end def render_participants_as_hash(participants) - participants.map(&method(:participant_as_hash)) + participants.map { |participant| participant_as_hash(participant) } end def participant_as_hash(participant) diff --git a/app/services/design_management/copy_design_collection/copy_service.rb b/app/services/design_management/copy_design_collection/copy_service.rb index 3bc30f62a81..8074a193bbf 100644 --- a/app/services/design_management/copy_design_collection/copy_service.rb +++ b/app/services/design_management/copy_design_collection/copy_service.rb @@ -128,9 +128,9 @@ module DesignManagement target_repository.raw.merge( git_user, - source_sha, - merge_branch, - 'CopyDesignCollectionService finalize merge' + source_sha: source_sha, + target_branch: merge_branch, + message: 'CopyDesignCollectionService finalize merge' ) { nil } target_design_collection.end_copy! diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb index 54fc452ac85..20b4ec0921f 100644 --- a/app/services/discussions/resolve_service.rb +++ b/app/services/discussions/resolve_service.rb @@ -16,7 +16,7 @@ module Discussions end def execute - discussions.each(&method(:resolve_discussion)) + discussions.each { |discussion| resolve_discussion(discussion) } after_resolve_cleanup end diff --git a/app/services/environments/stop_service.rb b/app/services/environments/stop_service.rb index 774e3ffe273..fb14ee40c05 100644 --- a/app/services/environments/stop_service.rb +++ b/app/services/environments/stop_service.rb @@ -28,6 +28,8 @@ module Environments created_environments = merge_request.created_environments if created_environments.any? + # This log message can be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/372965 + Gitlab::AppJsonLogger.info(message: 'Running new dynamic environment stop logic', project_id: project.id) created_environments.each { |env| execute(env) } else environments_in_head_pipeline = merge_request.environments_in_head_pipeline(deployment_status: :success) diff --git a/app/services/error_tracking/issue_update_service.rb b/app/services/error_tracking/issue_update_service.rb index ca5e8d656a6..f5ce7da0de7 100644 --- a/app/services/error_tracking/issue_update_service.rb +++ b/app/services/error_tracking/issue_update_service.rb @@ -37,7 +37,7 @@ module ErrorTracking def close_issue(issue) Issues::CloseService - .new(project: project, current_user: current_user) + .new(container: project, current_user: current_user) .execute(issue, system_note: false) end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index bf4a26400e1..d848f694598 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -229,23 +229,21 @@ class EventCreateService track_event(event_action: :pushed, event_target: Project, author_id: current_user.id) namespace = project.namespace - if Feature.enabled?(:route_hll_to_snowplow, namespace) - Gitlab::Tracking.event( - self.class.to_s, - :push, - label: 'usage_activity_by_stage_monthly.create.action_monthly_active_users_project_repo', - namespace: namespace, - user: current_user, - project: project, - property: 'project_action', - context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'project_action').to_context] - ) - end + Gitlab::Tracking.event( + self.class.to_s, + :push, + label: 'usage_activity_by_stage_monthly.create.action_monthly_active_users_project_repo', + namespace: namespace, + user: current_user, + project: project, + property: 'project_action', + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'project_action').to_context] + ) Users::LastPushEventService.new(current_user) .cache_last_push_event(event) - Users::ActivityService.new(current_user).execute + Users::ActivityService.new(author: current_user, namespace: namespace, project: project).execute end def create_event(resource_parent, current_user, status, attributes = {}) @@ -275,8 +273,8 @@ class EventCreateService { resource_parent_attr => resource_parent.id } end - def track_event(**params) - Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(**params) + def track_event(...) + Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(...) end def track_snowplow_event(action:, project:, user:, label:, property:) diff --git a/app/services/export_csv/base_service.rb b/app/services/export_csv/base_service.rb new file mode 100644 index 00000000000..84d44fd75fc --- /dev/null +++ b/app/services/export_csv/base_service.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module ExportCsv + class BaseService + # Target attachment size before base64 encoding + TARGET_FILESIZE = 15.megabytes + + def initialize(relation, resource_parent, fields = []) + @objects = relation + @resource_parent = resource_parent + @fields = fields + end + + def csv_data + csv_builder.render(TARGET_FILESIZE) + end + + def email(user) + raise NotImplementedError + end + + def invalid_fields + ::ExportCsv::MapExportFieldsService.new(fields, header_to_value_hash).invalid_fields + end + + private + + attr_reader :resource_parent, :objects, :fields + + # rubocop: disable CodeReuse/ActiveRecord + def csv_builder + @csv_builder ||= begin + data_hash = MapExportFieldsService.new(fields, header_to_value_hash).execute + + if preload_associations_in_batches? + CsvBuilder.new(objects, data_hash, associations_to_preload) + else + CsvBuilder.new(objects.preload(associations_to_preload), data_hash, []) + end + end + end + # rubocop: enable CodeReuse/ActiveRecord + + def associations_to_preload + [] + end + + def header_to_value_hash + raise NotImplementedError + end + + def preload_associations_in_batches? + false + end + end +end diff --git a/app/services/export_csv/map_export_fields_service.rb b/app/services/export_csv/map_export_fields_service.rb new file mode 100644 index 00000000000..d4f46c65328 --- /dev/null +++ b/app/services/export_csv/map_export_fields_service.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module ExportCsv + class MapExportFieldsService < BaseService + attr_reader :fields, :data + + def initialize(fields, data) + @fields = fields + @data = data + end + + def execute + return data if fields.empty? + + selected_fields_to_hash + end + + def invalid_fields + fields.reject { |field| permitted_field?(field) } + end + + private + + def selected_fields_to_hash + data.select { |key| requested_field?(key) } + end + + def requested_field?(field) + field.downcase.in?(fields.map(&:downcase)) + end + + def permitted_field?(field) + field.downcase.in?(keys.map(&:downcase)) + end + + def keys + data.keys + end + end +end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 68bb6427350..25a1e9a9873 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -39,7 +39,6 @@ module Groups if @group.save @group.add_owner(current_user) Integration.create_from_active_default_integrations(@group, :group_id) - Onboarding::Progress.onboard(@group) end end diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 02a760ccf29..45e8972213e 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -42,7 +42,7 @@ module Groups if user_ids_for_project_authorizations_refresh.present? UserProjectAccessChangedService .new(user_ids_for_project_authorizations_refresh) - .execute(blocking: true) + .execute end publish_event diff --git a/app/services/groups/group_links/create_service.rb b/app/services/groups/group_links/create_service.rb index 52180c39972..9c1a003ff36 100644 --- a/app/services/groups/group_links/create_service.rb +++ b/app/services/groups/group_links/create_service.rb @@ -31,7 +31,7 @@ module Groups end def setup_authorizations - shared_with_group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) + shared_with_group.refresh_members_authorized_projects(direct_members_only: true) end end end diff --git a/app/services/groups/group_links/destroy_service.rb b/app/services/groups/group_links/destroy_service.rb index d1f16775ab3..dc3cab927be 100644 --- a/app/services/groups/group_links/destroy_service.rb +++ b/app/services/groups/group_links/destroy_service.rb @@ -18,7 +18,7 @@ module Groups groups_to_refresh.uniq.each do |group| next if Feature.enabled?(:skip_group_share_unlink_auth_refresh, group.root_ancestor) - group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) + group.refresh_members_authorized_projects(direct_members_only: true) end else Gitlab::AppLogger.info( diff --git a/app/services/groups/group_links/update_service.rb b/app/services/groups/group_links/update_service.rb index 244ec2254a8..66d0d63cb9b 100644 --- a/app/services/groups/group_links/update_service.rb +++ b/app/services/groups/group_links/update_service.rb @@ -13,7 +13,7 @@ module Groups group_link.update!(group_link_params) if requires_authorization_refresh?(group_link_params) - group_link.shared_with_group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) + group_link.shared_with_group.refresh_members_authorized_projects(direct_members_only: true) end end diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb index e179a14c497..e30818cc5d2 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb @@ -16,10 +16,8 @@ module Import allow_local_network: allow_local_requests?, dns_rebind_protection: true } - validate :aws_s3, if: :validate_aws_s3? - # When removing the import_project_from_remote_file_s3 remove the - # whole condition of this validation: - validates_with RemoteFileValidator, if: -> { validate_aws_s3? || !s3_request? } + + validates_with RemoteFileValidator, if: -> { !s3_request? } def initialize(params:, current_user: nil) @params = params @@ -47,20 +45,10 @@ module Import attr_reader :params - def aws_s3 - if s3_request? - errors.add(:base, 'To import from AWS S3 use `projects/remote-import-s3`') - end - end - def s3_request? headers['Server'] == 'AmazonS3' && headers['x-amz-request-id'].present? end - def validate_aws_s3? - ::Feature.enabled?(:import_project_from_remote_file_s3) - end - def headers return {} if file_url.blank? diff --git a/app/services/import_csv/base_service.rb b/app/services/import_csv/base_service.rb new file mode 100644 index 00000000000..feb76425fb4 --- /dev/null +++ b/app/services/import_csv/base_service.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module ImportCsv + class BaseService + def initialize(user, project, csv_io) + @user = user + @project = project + @csv_io = csv_io + @results = { success: 0, error_lines: [], parse_error: false } + end + + def execute + process_csv + email_results_to_user + + results + end + + def email_results_to_user + raise NotImplementedError + end + + private + + attr_reader :user, :project, :csv_io, :results + + def attributes_for(row) + raise NotImplementedError + end + + def validate_headers_presence!(headers) + raise NotImplementedError + end + + def create_object_class + raise NotImplementedError + end + + def process_csv + with_csv_lines.each do |row, line_no| + attributes = attributes_for(row) + + if create_object(attributes)&.persisted? + results[:success] += 1 + else + results[:error_lines].push(line_no) + end + end + rescue ArgumentError, CSV::MalformedCSVError + results[:parse_error] = true + end + + def with_csv_lines + csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8) + validate_headers_presence!(csv_data.lines.first) + + CSV.new( + csv_data, + col_sep: detect_col_sep(csv_data.lines.first), + headers: true, + header_converters: :symbol + ).each.with_index(2) + end + + def detect_col_sep(header) + if header.include?(",") + "," + elsif header.include?(";") + ";" + elsif header.include?("\t") + "\t" + else + raise CSV::MalformedCSVError.new('Invalid CSV format', 1) + end + end + + def create_object(attributes) + # NOTE: CSV imports are performed by workers, so we do not have a request context in order + # to create a SpamParams object to pass to the issuable create service. + spam_params = nil + + # default_params can be extracted into a method if we need + # to support creation of objects that belongs to groups. + default_params = { container: project, + current_user: user, + params: attributes, + spam_params: spam_params } + + create_service = create_object_class.new(**default_params.merge(extra_create_service_params)) + + create_service.execute_without_rate_limiting + end + + # Overidden in subclasses to support specific parameters + def extra_create_service_params + {} + end + end +end diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index 49019278871..a75c5d2e75c 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -16,7 +16,7 @@ module IncidentManagement def execute create_result = Issues::CreateService.new( - project: project, + container: project, current_user: current_user, params: { title: title, diff --git a/app/services/incident_management/timeline_events/base_service.rb b/app/services/incident_management/timeline_events/base_service.rb index e0ca4320091..e997d940ed4 100644 --- a/app/services/incident_management/timeline_events/base_service.rb +++ b/app/services/incident_management/timeline_events/base_service.rb @@ -5,8 +5,6 @@ module IncidentManagement class BaseService include Gitlab::Utils::UsageData - AUTOCREATE_TAGS = [TimelineEventTag::START_TIME_TAG_NAME, TimelineEventTag::END_TIME_TAG_NAME].freeze - def allowed? user&.can?(:admin_incident_management_timeline_event, incident) end @@ -47,7 +45,7 @@ module IncidentManagement def auto_create_predefined_tags(new_tags) new_tags = new_tags.map(&:downcase) - tags_to_create = AUTOCREATE_TAGS.select { |tag| tag.downcase.in?(new_tags) } + tags_to_create = TimelineEventTag::PREDEFINED_TAGS.select { |tag| tag.downcase.in?(new_tags) } tags_to_create.each do |name| project.incident_management_timeline_event_tags.create(name: name) diff --git a/app/services/incident_management/timeline_events/create_service.rb b/app/services/incident_management/timeline_events/create_service.rb index 06e8fc32335..b2ea1f1b020 100644 --- a/app/services/incident_management/timeline_events/create_service.rb +++ b/app/services/incident_management/timeline_events/create_service.rb @@ -155,15 +155,14 @@ module IncidentManagement def validate_tags(project, tag_names) return [] unless tag_names&.any? - start_time_tag = AUTOCREATE_TAGS[0].downcase - end_time_tag = AUTOCREATE_TAGS[1].downcase + predefined_tags = TimelineEventTag::PREDEFINED_TAGS.map(&:downcase) tag_names_downcased = tag_names.map(&:downcase) tags = project.incident_management_timeline_event_tags.by_names(tag_names).pluck_names.map(&:downcase) # remove tags from given tag_names and also remove predefined tags which can be auto created - tag_names_downcased - tags - [start_time_tag, end_time_tag] + tag_names_downcased - tags - predefined_tags end end end diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb index 30444fa3938..c01509bc4d1 100644 --- a/app/services/issuable/bulk_update_service.rb +++ b/app/services/issuable/bulk_update_service.rb @@ -13,9 +13,9 @@ module Issuable end def execute(type) - ids = params.delete(:issuable_ids).split(",") + model_ids = ids_from_params(params.delete(:issuable_ids)) set_update_params(type) - updated_issuables = update_issuables(type, ids) + updated_issuables = update_issuables(type, model_ids) if updated_issuables.present? && requires_count_cache_reset?(type) schedule_group_issues_count_reset(updated_issuables) @@ -28,9 +28,14 @@ module Issuable private + def ids_from_params(issuable_ids) + return issuable_ids if issuable_ids.is_a?(Array) + + issuable_ids.split(',') + end + def set_update_params(type) params.slice!(*permitted_attrs(type)) - params.delete_if { |k, v| v.blank? } if params[:assignee_ids] == [IssuableFinder::Params::NONE.to_s] params[:assignee_ids] = [] @@ -40,8 +45,6 @@ module Issuable def permitted_attrs(type) attrs = %i(state_event milestone_id add_label_ids remove_label_ids subscription_event) - attrs.push(:sprint_id) if type == 'issue' - if type == 'issue' || type == 'merge_request' attrs.push(:assignee_ids) else @@ -53,10 +56,12 @@ module Issuable model_class = type.classify.constantize update_class = type.classify.pluralize.constantize::UpdateService items = find_issuables(parent, model_class, ids) + authorized_issuables = [] items.each do |issuable| next unless can?(current_user, :"update_#{type}", issuable) + authorized_issuables << issuable update_class.new( **update_class.constructor_container_arg(issuable.issuing_parent), current_user: current_user, @@ -64,23 +69,22 @@ module Issuable ).execute(issuable) end - items + authorized_issuables end def find_issuables(parent, model_class, ids) + issuables = model_class.id_in(ids) + case parent when Project - projects = parent + issuables = issuables.of_projects(parent) when Group - projects = parent.all_projects + issuables = issuables.of_projects(parent.all_projects) else - return + raise ArgumentError, _('A parent must be provided when bulk updating issuables') end - model_class - .id_in(ids) - .of_projects(projects) - .includes_for_bulk_update + issuables.includes_for_bulk_update end # Duplicates params and its top-level values diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb index 3c13944cfbc..02beaaf5d83 100644 --- a/app/services/issuable/clone/base_service.rb +++ b/app/services/issuable/clone/base_service.rb @@ -7,6 +7,11 @@ module Issuable alias_method :old_project, :project + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(original_entity, target_parent) @original_entity = original_entity @target_parent = target_parent @@ -77,7 +82,7 @@ module Issuable end def close_issue - close_service = Issues::CloseService.new(project: old_project, current_user: current_user) + close_service = Issues::CloseService.new(container: old_project, current_user: current_user) close_service.execute(original_entity, notifications: false, system_note: true) end diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb index 6aab56f0f68..4c3e518d62b 100644 --- a/app/services/issuable/destroy_service.rb +++ b/app/services/issuable/destroy_service.rb @@ -2,6 +2,11 @@ module Issuable class DestroyService < IssuableBaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(issuable) after_destroy(issuable) if issuable.destroy end diff --git a/app/services/issuable/discussions_list_service.rb b/app/services/issuable/discussions_list_service.rb index 10e7660289b..cb9271de11d 100644 --- a/app/services/issuable/discussions_list_service.rb +++ b/app/services/issuable/discussions_list_service.rb @@ -25,7 +25,7 @@ module Issuable paginated_discussions_by_type = paginator.records.group_by(&:table_name) notes = if paginated_discussions_by_type['notes'].present? - notes.with_discussion_ids(paginated_discussions_by_type['notes'].map(&:discussion_id)) + notes.id_in(paginated_discussions_by_type['notes'].flat_map(&:ids)) else notes.none end diff --git a/app/services/issuable/export_csv/base_service.rb b/app/services/issuable/export_csv/base_service.rb deleted file mode 100644 index 49ff05935c9..00000000000 --- a/app/services/issuable/export_csv/base_service.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -module Issuable - module ExportCsv - class BaseService - # Target attachment size before base64 encoding - TARGET_FILESIZE = 15.megabytes - - def initialize(issuables_relation, project) - @issuables = issuables_relation - @project = project - end - - def csv_data - csv_builder.render(TARGET_FILESIZE) - end - - private - - attr_reader :project, :issuables - - # rubocop: disable CodeReuse/ActiveRecord - def csv_builder - @csv_builder ||= - CsvBuilder.new(issuables.preload(associations_to_preload), header_to_value_hash) - end - # rubocop: enable CodeReuse/ActiveRecord - - def associations_to_preload - [] - end - - def header_to_value_hash - raise NotImplementedError - end - end - end -end diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index e84d1032e41..83cf5a67453 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -2,38 +2,13 @@ module Issuable module ImportCsv - class BaseService - def initialize(user, project, csv_io) - @user = user - @project = project - @csv_io = csv_io - @results = { success: 0, error_lines: [], parse_error: false } - end - - def execute - process_csv - email_results_to_user - - @results - end + class BaseService < ::ImportCsv::BaseService + extend ::Gitlab::Utils::Override private - def process_csv - with_csv_lines.each do |row, line_no| - attributes = issuable_attributes_for(row) - - if create_issuable(attributes)&.persisted? - @results[:success] += 1 - else - @results[:error_lines].push(line_no) - end - end - rescue ArgumentError, CSV::MalformedCSVError - @results[:parse_error] = true - end - - def issuable_attributes_for(row) + override :attributes_for + def attributes_for(row) { title: row[:title], description: row[:description], @@ -41,58 +16,13 @@ module Issuable } end - def with_csv_lines - csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8) - validate_headers_presence!(csv_data.lines.first) - - CSV.new( - csv_data, - col_sep: detect_col_sep(csv_data.lines.first), - headers: true, - header_converters: :symbol - ).each.with_index(2) - end - + override :validate_headers_presence! def validate_headers_presence!(headers) headers.downcase! if headers return if headers && headers.include?('title') && headers.include?('description') raise CSV::MalformedCSVError end - - def detect_col_sep(header) - if header.include?(",") - "," - elsif header.include?(";") - ";" - elsif header.include?("\t") - "\t" - else - raise CSV::MalformedCSVError - end - end - - def create_issuable(attributes) - # NOTE: CSV imports are performed by workers, so we do not have a request context in order - # to create a SpamParams object to pass to the issuable create service. - spam_params = nil - create_service = create_issuable_class.new(project: @project, current_user: @user, params: attributes, spam_params: spam_params) - - # For now, if create_issuable_class prepends RateLimitedService let's bypass rate limiting - if create_issuable_class < RateLimitedService - create_service.execute_without_rate_limiting - else - create_service.execute - end - end - - def email_results_to_user - # defined in ImportCsvService - end - - def create_issuable_class - # defined in ImportCsvService - end end end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index e24ae8f59f0..911d04d6b7a 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -34,8 +34,9 @@ class IssuableBaseService < ::BaseProjectService end def filter_params(issuable) + params.delete(:milestone) + unless can_set_issuable_metadata?(issuable) - params.delete(:milestone) params.delete(:milestone_id) params.delete(:labels) params.delete(:add_label_ids) diff --git a/app/services/issues/after_create_service.rb b/app/services/issues/after_create_service.rb index 5d10eca2979..011a78029c8 100644 --- a/app/services/issues/after_create_service.rb +++ b/app/services/issues/after_create_service.rb @@ -2,6 +2,11 @@ module Issues class AfterCreateService < Issues::BaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(issue) todo_service.new_issue(issue, current_user) delete_milestone_total_issue_counter_cache(issue.milestone) diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index 75bd2b88e86..877ce09e065 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -4,6 +4,11 @@ module Issues class BuildService < Issues::BaseService include ResolveDiscussions + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute filter_resolve_discussion_params diff --git a/app/services/issues/clone_service.rb b/app/services/issues/clone_service.rb index 8b05a1c2acd..c2a724254a7 100644 --- a/app/services/issues/clone_service.rb +++ b/app/services/issues/clone_service.rb @@ -76,7 +76,7 @@ module Issues # The system notes of the old issue are copied over so we don't want to end up with duplicate notes. # When cloning without notes, we want to generate system notes for the attributes that were copied. create_result = CreateService.new( - project: target_project, + container: target_project, current_user: current_user, params: new_params, spam_params: spam_params diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 4f6a859e20e..9fde1cc2ac2 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -2,6 +2,11 @@ module Issues class CloseService < Issues::BaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + # Closes the supplied issue if the current user is able to do so. def execute(issue, commit: nil, notifications: true, system_note: true, skip_authorization: false) return issue unless can_close?(issue, skip_authorization: skip_authorization) @@ -51,6 +56,11 @@ module Issues private + # TODO: remove once MergeRequests::CloseService or IssuableBaseService method is changed. + def self.constructor_container_arg(value) + { container: value } + end + def can_close?(issue, skip_authorization: false) skip_authorization || can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue) end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index f6a1db2dcaa..fa5233da489 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -13,11 +13,11 @@ module Issues # spam_checking is likely to be necessary. However, if there is not a request available in scope # in the caller (for example, an issue created via email) and the required arguments to the # SpamParams constructor are not otherwise available, spam_params: must be explicitly passed as nil. - def initialize(project:, spam_params:, current_user: nil, params: {}, build_service: nil) + def initialize(container:, spam_params:, current_user: nil, params: {}, build_service: nil) @extra_params = params.delete(:extra_params) || {} - super(project: project, current_user: current_user, params: params) + super(project: container, current_user: current_user, params: params) @spam_params = spam_params - @build_service = build_service || BuildService.new(project: project, current_user: current_user, params: params) + @build_service = build_service || BuildService.new(container: project, current_user: current_user, params: params) end def execute(skip_system_notes: false) @@ -100,6 +100,18 @@ module Issues private + def self.constructor_container_arg(value) + { container: value } + end + + def handle_quick_actions(issue) + # Do not handle quick actions unless the work item is the default Issue. + # The available quick actions for a work item depend on its type and widgets. + return if @params[:work_item_type].present? && @params[:work_item_type] != WorkItems::Type.default_by_type(:issue) + + super + end + def authorization_action :create_issue end diff --git a/app/services/issues/duplicate_service.rb b/app/services/issues/duplicate_service.rb index 9547698d916..a3213c50f86 100644 --- a/app/services/issues/duplicate_service.rb +++ b/app/services/issues/duplicate_service.rb @@ -2,6 +2,11 @@ module Issues class DuplicateService < Issues::BaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(duplicate_issue, canonical_issue) return if canonical_issue == duplicate_issue return unless can?(current_user, :update_issue, duplicate_issue) @@ -10,7 +15,7 @@ module Issues create_issue_duplicate_note(duplicate_issue, canonical_issue) create_issue_canonical_note(canonical_issue, duplicate_issue) - close_service.new(project: project, current_user: current_user).execute(duplicate_issue) + close_service.new(container: project, current_user: current_user).execute(duplicate_issue) duplicate_issue.update(duplicated_to: canonical_issue) relate_two_issues(duplicate_issue, canonical_issue) diff --git a/app/services/issues/export_csv_service.rb b/app/services/issues/export_csv_service.rb index 46e4b865dc3..d7c1ea276de 100644 --- a/app/services/issues/export_csv_service.rb +++ b/app/services/issues/export_csv_service.rb @@ -1,18 +1,18 @@ # frozen_string_literal: true module Issues - class ExportCsvService < Issuable::ExportCsv::BaseService + class ExportCsvService < ExportCsv::BaseService include Gitlab::Routing.url_helpers include GitlabRoutingHelper - def initialize(issuables_relation, project, user = nil) - super(issuables_relation, project) + def initialize(relation, resource_parent, user = nil) + super(relation, resource_parent) - @labels = @issuables.labels_hash.transform_values { |labels| labels.sort.join(',').presence } + @labels = objects.labels_hash.transform_values { |labels| labels.sort.join(',').presence } end def email(mail_to_user) - Notify.issues_csv_email(mail_to_user, project, csv_data, csv_builder.status).deliver_now + Notify.issues_csv_email(mail_to_user, resource_parent, csv_data, csv_builder.status).deliver_now end private @@ -55,6 +55,10 @@ module Issues issue.timelogs.sum(&:time_spent) end # rubocop: enable CodeReuse/ActiveRecord + + def preload_associations_in_batches? + Feature.enabled?(:export_csv_preload_in_batches, resource_parent) + end end end diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb index 83e550583f6..c3d6af952b4 100644 --- a/app/services/issues/import_csv_service.rb +++ b/app/services/issues/import_csv_service.rb @@ -9,21 +9,21 @@ module Issues end def email_results_to_user - Notify.import_issues_csv_email(@user.id, @project.id, @results).deliver_later + Notify.import_issues_csv_email(user.id, project.id, results).deliver_later end private - def create_issuable(attributes) + def create_object(attributes) super[:issue] end - def create_issuable_class + def create_object_class Issues::CreateService end def record_import_attempt - Issues::CsvImport.create!(user: @user, project: @project) + Issues::CsvImport.create!(user: user, project: project) end end end diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index f7f7d85611b..a2180dabdea 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -97,7 +97,7 @@ module Issues # Skip creation of system notes for existing attributes of the issue. The system notes of the old # issue are copied over so we don't want to end up with duplicate notes. create_result = CreateService.new( - project: @target_project, + container: @target_project, current_user: @current_user, params: new_params, spam_params: spam_params diff --git a/app/services/issues/referenced_merge_requests_service.rb b/app/services/issues/referenced_merge_requests_service.rb index a69cd324b1e..ba03927136a 100644 --- a/app/services/issues/referenced_merge_requests_service.rb +++ b/app/services/issues/referenced_merge_requests_service.rb @@ -2,6 +2,11 @@ module Issues class ReferencedMergeRequestsService < Issues::BaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + # rubocop: disable CodeReuse/ActiveRecord def execute(issue) referenced = referenced_merge_requests(issue) diff --git a/app/services/issues/related_branches_service.rb b/app/services/issues/related_branches_service.rb index 2ecd3e561c9..3f4413fdfd7 100644 --- a/app/services/issues/related_branches_service.rb +++ b/app/services/issues/related_branches_service.rb @@ -4,6 +4,11 @@ # those with a merge request open referencing the current issue. module Issues class RelatedBranchesService < Issues::BaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(issue) branch_names_with_mrs = branches_with_merge_request_for(issue) branches = branches_with_iid_of(issue).reject { |b| branch_names_with_mrs.include?(b[:name]) } @@ -27,7 +32,7 @@ module Issues def branches_with_merge_request_for(issue) Issues::ReferencedMergeRequestsService - .new(project: project, current_user: current_user) + .new(container: project, current_user: current_user) .referenced_merge_requests(issue) .map(&:source_branch) end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index f4f81e9455a..ebcf2fb5c83 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -2,6 +2,11 @@ module Issues class ReopenService < Issues::BaseService + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(issue, skip_authorization: false) return issue unless can_reopen?(issue, skip_authorization: skip_authorization) @@ -22,6 +27,14 @@ module Issues private + # overriding this because IssuableBaseService#constructor_container_arg returns { project: value } + # Issues::ReopenService constructor signature is different now, it takes container instead of project also + # IssuableBaseService#change_state dynamically picks one of the `Issues::ReopenService`, `Epics::ReopenService` or + # MergeRequests::ReopenService, so we need this method to return { }container: value } for Issues::ReopenService + def self.constructor_container_arg(value) + { container: value } + end + def can_reopen?(issue, skip_authorization: false) skip_authorization || can?(current_user, :reopen_issue, issue) end diff --git a/app/services/issues/reorder_service.rb b/app/services/issues/reorder_service.rb index 5443d41ac30..059b4196b23 100644 --- a/app/services/issues/reorder_service.rb +++ b/app/services/issues/reorder_service.rb @@ -4,6 +4,11 @@ module Issues class ReorderService < Issues::BaseService include Gitlab::Utils::StrongMemoize + # TODO: this is to be removed once we get to rename the IssuableBaseService project param to container + def initialize(container:, current_user: nil, params: {}) + super(project: container, current_user: current_user, params: params) + end + def execute(issue) return false unless can?(current_user, :update_issue, issue) return false unless move_between_ids @@ -14,7 +19,7 @@ module Issues private def update(issue, attrs) - ::Issues::UpdateService.new(project: project, current_user: current_user, params: attrs).execute(issue) + ::Issues::UpdateService.new(container: project, current_user: current_user, params: attrs).execute(issue) rescue ActiveRecord::RecordNotFound false end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 71cc5581ae6..71324b3f044 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -5,8 +5,8 @@ module Issues # NOTE: For Issues::UpdateService, we default the spam_params to nil, because spam_checking is not # necessary in many cases, and we don't want to require every caller to explicitly pass it as nil # to disable spam checking. - def initialize(project:, current_user: nil, params: {}, spam_params: nil) - super(project: project, current_user: current_user, params: params) + def initialize(container:, current_user: nil, params: {}, spam_params: nil) + super(project: container, current_user: current_user, params: params) @spam_params = spam_params end @@ -96,7 +96,7 @@ module Issues canonical_issue = IssuesFinder.new(current_user).find_by(id: canonical_issue_id) if canonical_issue - Issues::DuplicateService.new(project: project, current_user: current_user).execute(issue, canonical_issue) + Issues::DuplicateService.new(container: project, current_user: current_user).execute(issue, canonical_issue) end end # rubocop: enable CodeReuse/ActiveRecord @@ -109,13 +109,30 @@ module Issues target_project != issue.project update(issue) - Issues::MoveService.new(project: project, current_user: current_user).execute(issue, target_project) + Issues::MoveService.new(container: project, current_user: current_user).execute(issue, target_project) end private attr_reader :spam_params + # TODO: remove this once MergeRequests::UpdateService#initialize is changed to take container as named argument. + # + # Issues::UpdateService is used together with MergeRequests::UpdateService in Mutations::Assignable#assign! method + # however MergeRequests::UpdateService#initialize still takes `project` as param and Issues::UpdateService is being + # changed to take `container` as param. So we are adding this workaround in the meantime. + def self.constructor_container_arg(value) + { container: value } + end + + def handle_quick_actions(issue) + # Do not handle quick actions unless the work item is the default Issue. + # The available quick actions for a work item depend on its type and widgets. + return unless issue.work_item_type.default_issue? + + super + end + def handle_date_changes(issue) return unless issue.previous_changes.slice('due_date', 'start_date').any? @@ -131,7 +148,7 @@ module Issues # we've pre-empted this from running in #execute, so let's go ahead and update the Issue now. update(issue) - Issues::CloneService.new(project: project, current_user: current_user).execute(issue, target_project, with_notes: with_notes) + Issues::CloneService.new(container: project, current_user: current_user).execute(issue, target_project, with_notes: with_notes) end def create_merge_request_from_quick_action @@ -181,9 +198,9 @@ module Issues return if skip_milestone_email if issue.milestone.nil? - notification_service.async.removed_milestone_issue(issue, current_user) + notification_service.async.removed_milestone(issue, current_user) else - notification_service.async.changed_milestone_issue(issue, issue.milestone, current_user) + notification_service.async.changed_milestone(issue, issue.milestone, current_user) end end diff --git a/app/services/issues/zoom_link_service.rb b/app/services/issues/zoom_link_service.rb index 1ce459aa7e6..4144c293990 100644 --- a/app/services/issues/zoom_link_service.rb +++ b/app/services/issues/zoom_link_service.rb @@ -2,8 +2,8 @@ module Issues class ZoomLinkService < Issues::BaseService - def initialize(project:, current_user:, params:) - super + def initialize(container:, current_user:, params:) + super(project: container, current_user: current_user, params: params) @issue = params.fetch(:issue) @added_meeting = ZoomMeeting.canonical_meeting(@issue) diff --git a/app/services/jira/requests/projects/list_service.rb b/app/services/jira/requests/projects/list_service.rb index ac9e9bf0be9..09cab3c659b 100644 --- a/app/services/jira/requests/projects/list_service.rb +++ b/app/services/jira/requests/projects/list_service.rb @@ -29,7 +29,9 @@ module Jira end def map_projects(response) - response.map { |v| JIRA::Resource::Project.build(client, v) }.select(&method(:match_query?)) + response + .map { |v| JIRA::Resource::Project.build(client, v) } + .select { |jira_project| match_query?(jira_project) } end def match_query?(jira_project) diff --git a/app/services/jira_connect_installations/update_service.rb b/app/services/jira_connect_installations/update_service.rb index b2b6f2a91f2..ff5b9671e2b 100644 --- a/app/services/jira_connect_installations/update_service.rb +++ b/app/services/jira_connect_installations/update_service.rb @@ -24,7 +24,7 @@ module JiraConnectInstallations end end - send_uninstalled_hook if instance_url_changed? + send_uninstalled_hook if instance_url_changed? && @installation.instance_url.blank? ServiceResponse.new(status: :success) end diff --git a/app/services/keys/revoke_service.rb b/app/services/keys/revoke_service.rb new file mode 100644 index 00000000000..42ea9ab73be --- /dev/null +++ b/app/services/keys/revoke_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Keys + class RevokeService < ::Keys::DestroyService + def execute(key) + key.transaction do + unverify_associated_signatures(key) + + raise ActiveRecord::Rollback unless super(key) + end + end + + private + + def unverify_associated_signatures(key) + return unless Feature.enabled?(:revoke_ssh_signatures) + + key.ssh_signatures.each_batch do |batch| + batch.update_all( + verification_status: CommitSignatures::SshSignature.verification_statuses[:revoked_key], + updated_at: Time.zone.now + ) + end + end + end +end + +Keys::DestroyService.prepend_mod diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb index 5e73d7a957b..20f96ac2949 100644 --- a/app/services/members/approve_access_request_service.rb +++ b/app/services/members/approve_access_request_service.rb @@ -15,6 +15,12 @@ module Members private + def after_execute(member:, skip_log_audit_event:) + super + + resolve_access_request_todos(current_user, member) + end + def validate_access!(access_requester) raise Gitlab::Access::AccessDeniedError unless can_approve_access_requester?(access_requester) diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb index 62b8fc5d6f7..801f77ae082 100644 --- a/app/services/members/base_service.rb +++ b/app/services/members/base_service.rb @@ -53,6 +53,10 @@ module Members end end + def resolve_access_request_todos(current_user, requester) + todo_service.resolve_access_request_todos(current_user, requester) + end + def enqueue_delete_todos(member) type = member.is_a?(GroupMember) ? 'Group' : 'Project' # don't enqueue immediately to prevent todos removal in case of a mistake diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index 2d378a64c02..3ce8390d07d 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -21,8 +21,7 @@ module Members expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil, - ldap: nil, - blocking_refresh: nil + ldap: nil ) return [] unless invitees.present? @@ -40,8 +39,7 @@ module Members expires_at: expires_at, tasks_to_be_done: tasks_to_be_done, tasks_project_id: tasks_project_id, - ldap: ldap, - blocking_refresh: blocking_refresh + ldap: ldap } members = emails.map do |email| @@ -62,16 +60,14 @@ module Members access_level, current_user: nil, expires_at: nil, - ldap: nil, - blocking_refresh: nil + ldap: nil ) add_members(source, [invitee], access_level, current_user: current_user, expires_at: expires_at, - ldap: ldap, - blocking_refresh: blocking_refresh).first + ldap: ldap).first end private @@ -250,8 +246,6 @@ module Members def find_or_build_member @member = builder.new(source, invitee, existing_members).execute - - @member.blocking_refresh = args[:blocking_refresh] end def ldap diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 24c5b12b335..dd84b890385 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -63,6 +63,7 @@ module Members delete_subresources(member) unless skip_subresources delete_project_invitations_by(member) unless skip_subresources + resolve_access_request_todos(current_user, member) enqueue_delete_todos(member) enqueue_unassign_issuables(member) if unassign_issuables diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index 9e39aa94246..11251e56ee3 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -9,6 +9,8 @@ module MergeRequests prepare_for_mergeability(merge_request) prepare_merge_request(merge_request) + + mark_merge_request_as_prepared(merge_request) end private @@ -53,6 +55,10 @@ module MergeRequests merge_request.mark_as_unchecked merge_request.check_mergeability(async: true) end + + def mark_merge_request_as_prepared(merge_request) + merge_request.update!(prepared_at: Time.current) + end end end diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb index c107280efb1..54283ea0676 100644 --- a/app/services/merge_requests/assign_issues_service.rb +++ b/app/services/merge_requests/assign_issues_service.rb @@ -14,7 +14,7 @@ module MergeRequests def execute assignable_issues.each do |issue| - Issues::UpdateService.new(project: issue.project, current_user: current_user, params: { assignee_ids: [current_user.id] }).execute(issue) + Issues::UpdateService.new(container: issue.project, current_user: current_user, params: { assignee_ids: [current_user.id] }).execute(issue) end { diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 8fa80dc3513..75e1adec41b 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -39,11 +39,7 @@ module MergeRequests # open while the Gitaly RPC waits. To avoid an idle in transaction # timeout, we do this before we attempt to save the merge request. - if Feature.enabled?(:async_merge_request_diff_creation, merge_request.target_project) - merge_request.skip_ensure_merge_request_diff = true - else - merge_request.eager_fetch_ref! - end + merge_request.skip_ensure_merge_request_diff = true end def set_projects! diff --git a/app/services/merge_requests/export_csv_service.rb b/app/services/merge_requests/export_csv_service.rb index 1f8dec69ef0..96b4cdd0fe5 100644 --- a/app/services/merge_requests/export_csv_service.rb +++ b/app/services/merge_requests/export_csv_service.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true module MergeRequests - class ExportCsvService < Issuable::ExportCsv::BaseService + class ExportCsvService < ExportCsv::BaseService include Gitlab::Routing.url_helpers include GitlabRoutingHelper def email(user) - Notify.merge_requests_csv_email(user, project, csv_data, csv_builder.status).deliver_now + Notify.merge_requests_csv_email(user, resource_parent, csv_data, csv_builder.status).deliver_now end private diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 6b4f9dbe509..e6b0ffbf716 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -98,6 +98,7 @@ module MergeRequests commit_id ensure merge_request.update_and_mark_in_progress_merge_commit_sha(nil) + log_info("Merge request marked in progress") end def update_merge_sha_metadata(commit_id) diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 9fca2b0d19e..e32895a3cb6 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -55,7 +55,7 @@ module MergeRequests merge_request.id ) else - Issues::CloseService.new(project: project, current_user: current_user).execute(issue, commit: merge_request) + Issues::CloseService.new(container: project, current_user: current_user).execute(issue, commit: merge_request) end end end diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index 711978dc3f7..235dc6678df 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -145,7 +145,7 @@ module MergeRequests if push_options[:milestone] milestone = Milestone.for_projects_and_groups(@project, @project.ancestors_upto)&.find_by_name(push_options[:milestone]) - params[:milestone] = milestone if milestone + params[:milestone_id] = milestone.id if milestone end if params.key?(:description) diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index ce49d5dd43c..61831a624c7 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -16,7 +16,7 @@ module MergeRequests def refresh_merge_requests! # n + 1: https://gitlab.com/gitlab-org/gitlab-foss/issues/60289 - Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits)) + Gitlab::GitalyClient.allow_n_plus_1_calls { find_new_commits } # Be sure to close outstanding MRs before reloading them to avoid generating an # empty diff during a manual merge @@ -229,7 +229,7 @@ module MergeRequests :source, @push.branch_name, presence) end - # Add comment about pushing new commits to merge requests and send nofitication emails + # Add comment about pushing new commits to merge requests and send notification emails # def notify_about_push(merge_request) return unless @commits.present? diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index a273b853c0d..255d96f4969 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -215,9 +215,9 @@ module MergeRequests delete_milestone_total_merge_requests_counter_cache(previous_milestone) if merge_request.milestone.nil? - notification_service.async.removed_milestone_merge_request(merge_request, current_user) + notification_service.async.removed_milestone(merge_request, current_user) else - notification_service.async.changed_milestone_merge_request(merge_request, merge_request.milestone, current_user) + notification_service.async.changed_milestone(merge_request, merge_request.milestone, current_user) delete_milestone_total_merge_requests_counter_cache(merge_request.milestone) end diff --git a/app/services/milestones/destroy_service.rb b/app/services/milestones/destroy_service.rb index 2563f2f5390..191a8711cbd 100644 --- a/app/services/milestones/destroy_service.rb +++ b/app/services/milestones/destroy_service.rb @@ -4,10 +4,10 @@ module Milestones class DestroyService < Milestones::BaseService def execute(milestone) Milestone.transaction do - update_params = { milestone: nil, skip_milestone_email: true } + update_params = { milestone_id: nil, skip_milestone_email: true } milestone.issues.each do |issue| - Issues::UpdateService.new(project: parent, current_user: current_user, params: update_params).execute(issue) + Issues::UpdateService.new(container: parent, current_user: current_user, params: update_params).execute(issue) end milestone.merge_requests.each do |merge_request| diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 5f05b613288..f5efc480fef 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -137,8 +137,6 @@ module Notes end def invalid_reviewers?(update_params) - return false unless Feature.enabled?(:limit_reviewer_and_assignee_size) - if update_params.key?(:reviewer_ids) possible_reviewers = update_params[:reviewer_ids]&.uniq&.size @@ -167,6 +165,20 @@ module Notes if Feature.enabled?(:notes_create_service_tracking, project) Gitlab::Tracking.event('Notes::CreateService', 'execute', **tracking_data_for(note)) end + + if Feature.enabled?(:route_hll_to_snowplow_phase4, project&.namespace) && note.for_commit? + metric_key_path = 'counts.commit_comment' + + Gitlab::Tracking.event( + 'Notes::CreateService', + 'create_commit_comment', + project: project, + namespace: project&.namespace, + user: user, + label: metric_key_path, + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis, key_path: metric_key_path).to_context] + ) + end end def tracking_data_for(note) diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index eda8bbcbc2e..ccee94a5cea 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -10,6 +10,7 @@ module Notes clear_noteable_diffs_cache(note) track_note_removal_usage_for_issues(note) if note.for_issue? track_note_removal_usage_for_merge_requests(note) if note.for_merge_request? + track_note_removal_usage_for_design(note) if note.for_design? end private @@ -22,6 +23,13 @@ module Notes def track_note_removal_usage_for_merge_requests(note) Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_remove_comment_action(note: note) end + + def track_note_removal_usage_for_design(note) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_design_comment_removed_action( + author: note.author, + project: project + ) + end end end diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb index bdeebc641b8..04563d180b5 100644 --- a/app/services/notification_recipients/build_service.rb +++ b/app/services/notification_recipients/build_service.rb @@ -17,24 +17,24 @@ module NotificationRecipients ::NotificationRecipients::Builder::Default.new(target, current_user, **args).notification_recipients end - def self.build_new_note_recipients(*args) - ::NotificationRecipients::Builder::NewNote.new(*args).notification_recipients + def self.build_new_note_recipients(...) + ::NotificationRecipients::Builder::NewNote.new(...).notification_recipients end - def self.build_merge_request_unmergeable_recipients(*args) - ::NotificationRecipients::Builder::MergeRequestUnmergeable.new(*args).notification_recipients + def self.build_merge_request_unmergeable_recipients(...) + ::NotificationRecipients::Builder::MergeRequestUnmergeable.new(...).notification_recipients end def self.build_project_maintainers_recipients(target, **args) ::NotificationRecipients::Builder::ProjectMaintainers.new(target, **args).notification_recipients end - def self.build_new_review_recipients(*args) - ::NotificationRecipients::Builder::NewReview.new(*args).notification_recipients + def self.build_new_review_recipients(...) + ::NotificationRecipients::Builder::NewReview.new(...).notification_recipients end - def self.build_requested_review_recipients(*args) - ::NotificationRecipients::Builder::RequestReview.new(*args).notification_recipients + def self.build_requested_review_recipients(...) + ::NotificationRecipients::Builder::RequestReview.new(...).notification_recipients end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 777d02c590d..47bc36fce70 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -212,14 +212,6 @@ class NotificationService relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email) end - def removed_milestone_issue(issue, current_user) - removed_milestone_resource_email(issue, current_user, :removed_milestone_issue_email) - end - - def changed_milestone_issue(issue, new_milestone, current_user) - changed_milestone_resource_email(issue, new_milestone, current_user, :changed_milestone_issue_email) - end - # When create a merge request we should send an email to: # # * mr author @@ -366,14 +358,6 @@ class NotificationService relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email) end - def removed_milestone_merge_request(merge_request, current_user) - removed_milestone_resource_email(merge_request, current_user, :removed_milestone_merge_request_email) - end - - def changed_milestone_merge_request(merge_request, new_milestone, current_user) - changed_milestone_resource_email(merge_request, new_milestone, current_user, :changed_milestone_merge_request_email) - end - def close_mr(merge_request, current_user) close_resource_email(merge_request, current_user, :closed_merge_request_email) end @@ -788,6 +772,44 @@ class NotificationService end end + def removed_milestone(target, current_user) + method = case target + when Issue + :removed_milestone_issue_email + when MergeRequest + :removed_milestone_merge_request_email + end + + recipients = NotificationRecipients::BuildService.build_recipients( + target, + current_user, + action: 'removed_milestone' + ) + + recipients.each do |recipient| + mailer.send(method, recipient.user.id, target.id, current_user.id).deliver_later + end + end + + def changed_milestone(target, milestone, current_user) + method = case target + when Issue + :changed_milestone_issue_email + when MergeRequest + :changed_milestone_merge_request_email + end + + recipients = NotificationRecipients::BuildService.build_recipients( + target, + current_user, + action: 'changed_milestone' + ) + + recipients.each do |recipient| + mailer.send(method, recipient.user.id, target.id, milestone, current_user.id).deliver_later + end + end + protected def new_resource_email(target, current_user, method) @@ -847,30 +869,6 @@ class NotificationService end end - def removed_milestone_resource_email(target, current_user, method) - recipients = NotificationRecipients::BuildService.build_recipients( - target, - current_user, - action: 'removed_milestone' - ) - - recipients.each do |recipient| - mailer.send(method, recipient.user.id, target.id, current_user.id).deliver_later - end - end - - def changed_milestone_resource_email(target, milestone, current_user, method) - recipients = NotificationRecipients::BuildService.build_recipients( - target, - current_user, - action: 'changed_milestone' - ) - - recipients.each do |recipient| - mailer.send(method, recipient.user.id, target.id, milestone, current_user.id).deliver_later - end - end - def reopen_resource_email(target, current_user, method, status) recipients = NotificationRecipients::BuildService.build_recipients(target, current_user, action: "reopen") @@ -941,12 +939,12 @@ class NotificationService NotificationRecipients::BuildService.build_project_maintainers_recipients(target, action: action) end - def notifiable?(*args) - NotificationRecipients::BuildService.notifiable?(*args) + def notifiable?(...) + NotificationRecipients::BuildService.notifiable?(...) end - def notifiable_users(*args) - NotificationRecipients::BuildService.notifiable_users(*args) + def notifiable_users(...) + NotificationRecipients::BuildService.notifiable_users(...) end def deliver_access_request_email(recipient, member) diff --git a/app/services/packages/create_event_service.rb b/app/services/packages/create_event_service.rb index 8fed6e2def8..82c4292fca8 100644 --- a/app/services/packages/create_event_service.rb +++ b/app/services/packages/create_event_service.rb @@ -21,6 +21,17 @@ module Packages end end + def originator_type + case current_user + when User + :user + when DeployToken + :deploy_token + else + :guest + end + end + private def event_scope @@ -34,20 +45,5 @@ module Packages def event_name params[:event_name] end - - def originator_type - case current_user - when User - :user - when DeployToken - :deploy_token - else - :guest - end - end - - def guest? - originator_type == :guest - end end end diff --git a/app/services/packages/debian/create_package_file_service.rb b/app/services/packages/debian/create_package_file_service.rb index 19e68183ea2..24e40b5c986 100644 --- a/app/services/packages/debian/create_package_file_service.rb +++ b/app/services/packages/debian/create_package_file_service.rb @@ -3,8 +3,6 @@ module Packages module Debian class CreatePackageFileService - include ::Packages::FIPS - def initialize(package:, current_user:, params: {}) @package = package @current_user = current_user @@ -12,7 +10,6 @@ module Packages end def execute - raise DisabledError, 'Debian registry is not FIPS compliant' if Gitlab::FIPS.enabled? raise ArgumentError, "Invalid package" unless package.present? raise ArgumentError, "Invalid user" unless current_user.present? @@ -32,7 +29,13 @@ module Packages } ) - if params[:file_name].end_with? '.changes' + if params[:distribution].present? && params[:component].present? + ::Packages::Debian::ProcessPackageFileWorker.perform_async( + package_file.id, + params[:distribution], + params[:component] + ) + elsif params[:file_name].end_with? '.changes' ::Packages::Debian::ProcessChangesWorker.perform_async(package_file.id, current_user.id) end diff --git a/app/services/packages/debian/extract_changes_metadata_service.rb b/app/services/packages/debian/extract_changes_metadata_service.rb index 30480834748..43a4db5bdfc 100644 --- a/app/services/packages/debian/extract_changes_metadata_service.rb +++ b/app/services/packages/debian/extract_changes_metadata_service.rb @@ -4,7 +4,6 @@ module Packages module Debian class ExtractChangesMetadataService include Gitlab::Utils::StrongMemoize - include ::Packages::FIPS ExtractionError = Class.new(StandardError) @@ -14,8 +13,6 @@ module Packages end def execute - raise DisabledError, 'Debian registry is not FIPS compliant' if Gitlab::FIPS.enabled? - { file_type: file_type, architecture: metadata[:architecture], diff --git a/app/services/packages/debian/find_or_create_package_service.rb b/app/services/packages/debian/find_or_create_package_service.rb index 3b2be7b6874..cb765e956e7 100644 --- a/app/services/packages/debian/find_or_create_package_service.rb +++ b/app/services/packages/debian/find_or_create_package_service.rb @@ -10,7 +10,7 @@ module Packages .debian .with_name(params[:name]) .with_version(params[:version]) - .with_debian_codename(params[:distribution_name]) + .with_debian_codename_or_suite(params[:distribution_name]) .not_pending_destruction .first @@ -26,7 +26,10 @@ module Packages def distribution strong_memoize(:distribution) do - Packages::Debian::DistributionsFinder.new(project, codename: params[:distribution_name]).execute.last! + Packages::Debian::DistributionsFinder.new( + project, + codename_or_suite: params[:distribution_name] + ).execute.last! end end end diff --git a/app/services/packages/debian/generate_distribution_service.rb b/app/services/packages/debian/generate_distribution_service.rb index 9b313202400..12ae6c68918 100644 --- a/app/services/packages/debian/generate_distribution_service.rb +++ b/app/services/packages/debian/generate_distribution_service.rb @@ -4,7 +4,6 @@ module Packages module Debian class GenerateDistributionService include Gitlab::Utils::StrongMemoize - include ::Packages::FIPS include ExclusiveLeaseGuard ONE_HOUR = 1.hour.freeze @@ -66,13 +65,10 @@ module Packages def initialize(distribution) @distribution = distribution @oldest_kept_generated_at = nil - @md5sum = [] @sha256 = [] end def execute - raise DisabledError, 'Debian registry is not FIPS compliant' if Gitlab::FIPS.enabled? - try_obtain_lease do @distribution.transaction do # We consider `apt-get update` can take at most one hour @@ -106,7 +102,7 @@ module Packages .with_debian_architecture_name(architecture&.name) .with_debian_file_type(package_file_type) .find_each - .map(&method(:package_stanza_from_fields)) + .map { |package_file| package_stanza_from_fields(package_file) } reuse_or_create_component_file(component, component_file_type, architecture, paragraphs.join("\n")) end @@ -143,10 +139,10 @@ module Packages rfc822_field('Directory', package_dirname(package_file)) ] else + # NB: MD5sum was removed for FIPS compliance [ rfc822_field('Filename', "#{package_dirname(package_file)}/#{package_file.file_name}"), rfc822_field('Size', package_file.size), - rfc822_field('MD5sum', package_file.file_md5), rfc822_field('SHA256', package_file.file_sha256) ] end @@ -190,7 +186,6 @@ module Packages ) end - @md5sum.append(" #{file_md5} #{component_file.size.to_s.rjust(8)} #{component_file.relative_path}") @sha256.append(" #{file_sha256} #{component_file.size.to_s.rjust(8)} #{component_file.relative_path}") end @@ -234,7 +229,8 @@ module Packages end def release_sums - ["MD5Sum:", @md5sum, "SHA256:", @sha256].flatten.compact.join("\n") + "\n" + # NB: MD5Sum was removed for FIPS compliance + ["SHA256:", @sha256].flatten.compact.join("\n") + "\n" end def rfc822_field(name, value, condition = true) diff --git a/app/services/packages/debian/process_changes_service.rb b/app/services/packages/debian/process_changes_service.rb index a29cbd3f65f..129f2e5c9bc 100644 --- a/app/services/packages/debian/process_changes_service.rb +++ b/app/services/packages/debian/process_changes_service.rb @@ -15,12 +15,12 @@ module Packages end def execute - try_obtain_lease do - # return if changes file has already been processed - break if package_file.debian_file_metadatum&.changes? + # return if changes file has already been processed + return if package_file.debian_file_metadatum&.changes? - validate! + validate! + try_obtain_lease do package_file.transaction do update_files_metadata update_changes_metadata @@ -38,6 +38,9 @@ module Packages raise ArgumentError, 'invalid package file' unless package_file.debian_file_metadatum raise ArgumentError, 'invalid package file' unless package_file.debian_file_metadatum.unknown? raise ArgumentError, 'invalid package file' unless metadata[:file_type] == :changes + raise ArgumentError, 'missing Source field' unless metadata.dig(:fields, 'Source').present? + raise ArgumentError, 'missing Version field' unless metadata.dig(:fields, 'Version').present? + raise ArgumentError, 'missing Distribution field' unless metadata.dig(:fields, 'Distribution').present? end def update_files_metadata diff --git a/app/services/packages/debian/process_package_file_service.rb b/app/services/packages/debian/process_package_file_service.rb index 59e8ac3425b..7d2d71184e6 100644 --- a/app/services/packages/debian/process_package_file_service.rb +++ b/app/services/packages/debian/process_package_file_service.rb @@ -10,19 +10,23 @@ module Packages # used by ExclusiveLeaseGuard DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze - def initialize(package_file, creator, distribution_name, component_name) + def initialize(package_file, distribution_name, component_name) @package_file = package_file - @creator = creator @distribution_name = distribution_name @component_name = component_name end def execute - try_obtain_lease do - validate! + return if @package_file.package.pending_destruction? + + validate! - @package_file.transaction do + try_obtain_lease do + package.transaction do + rename_package_and_set_version + update_package update_file_metadata + cleanup_temp_package end ::Packages::Debian::GenerateDistributionWorker.perform_async(:project, package.debian_distribution.id) @@ -32,6 +36,8 @@ module Packages private def validate! + raise ArgumentError, 'missing distribution name' unless @distribution_name.present? + raise ArgumentError, 'missing component name' unless @component_name.present? raise ArgumentError, 'package file without Debian metadata' unless @package_file.debian_file_metadatum raise ArgumentError, 'already processed package file' unless @package_file.debian_file_metadatum.unknown? @@ -40,6 +46,80 @@ module Packages raise ArgumentError, "invalid package file type: #{file_metadata[:file_type]}" end + def file_metadata + ::Packages::Debian::ExtractMetadataService.new(@package_file).execute + end + strong_memoize_attr :file_metadata + + def package + package = temp_package.project + .packages + .debian + .with_name(package_name) + .with_version(package_version) + .with_debian_codename_or_suite(@distribution_name) + .not_pending_destruction + .last + package || temp_package + end + strong_memoize_attr :package + + def temp_package + @package_file.package + end + strong_memoize_attr :temp_package + + def package_name + package_name_and_version[0] + end + + def package_version + package_name_and_version[1] + end + + def package_name_and_version + package_name = file_metadata[:fields]['Package'] + package_version = file_metadata[:fields]['Version'] + + if file_metadata[:fields]['Source'] + # "sample" or "sample (1.2.3~alpha2)" + source_field_parts = file_metadata[:fields]['Source'].split(SOURCE_FIELD_SPLIT_REGEX) + package_name = source_field_parts[0] + package_version = source_field_parts[2] || package_version + end + + [package_name, package_version] + end + strong_memoize_attr :package_name_and_version + + def rename_package_and_set_version + package.update!( + name: package_name, + version: package_version, + status: :default + ) + end + + def update_package + return unless using_temporary_package? + + package.update!( + debian_publication_attributes: { distribution_id: distribution.id } + ) + end + + def using_temporary_package? + package.id == temp_package.id + end + + def distribution + Packages::Debian::DistributionsFinder.new( + @package_file.package.project, + codename_or_suite: @distribution_name + ).execute.last! + end + strong_memoize_attr :distribution + def update_file_metadata ::Packages::UpdatePackageFileService.new(@package_file, package_id: package.id) .execute @@ -55,36 +135,8 @@ module Packages ) end - def package - strong_memoize(:package) do - package_name = file_metadata[:fields]['Package'] - package_version = file_metadata[:fields]['Version'] - - if file_metadata[:fields]['Source'] - # "sample" or "sample (1.2.3~alpha2)" - source_field_parts = file_metadata[:fields]['Source'].split(SOURCE_FIELD_SPLIT_REGEX) - package_name = source_field_parts[0] - package_version = source_field_parts[2] || package_version - end - - params = { - 'name': package_name, - 'version': package_version, - 'distribution_name': @distribution_name - } - response = Packages::Debian::FindOrCreatePackageService.new(project, @creator, params).execute - response.payload[:package] - end - end - - def file_metadata - strong_memoize(:metadata) do - ::Packages::Debian::ExtractMetadataService.new(@package_file).execute - end - end - - def project - @package_file.package.project + def cleanup_temp_package + temp_package.destroy unless using_temporary_package? end # used by ExclusiveLeaseGuard diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index 03844c2dc7e..b3a9beabba5 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -17,7 +17,7 @@ class PreviewMarkdownService < BaseService private def quick_action_types - %w(Issue MergeRequest Commit) + %w(Issue MergeRequest Commit WorkItem) end def explain_quick_actions(text) diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index a3e533c670e..9378bb31360 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -22,8 +22,9 @@ module Projects private def delete_tags - delete_service.execute - .tap(&method(:log_response)) + delete_service + .execute + .tap { |response| log_response(response) } end def delete_service diff --git a/app/services/projects/container_repository/destroy_service.rb b/app/services/projects/container_repository/destroy_service.rb index 6db6b449671..6cb0d55aea4 100644 --- a/app/services/projects/container_repository/destroy_service.rb +++ b/app/services/projects/container_repository/destroy_service.rb @@ -10,12 +10,15 @@ module Projects }.freeze def execute(container_repository, disable_timeout: true) - return false unless can?(current_user, :update_container_image, project) + return error('Unauthorized access') unless can_destroy? # Delete tags outside of the transaction to avoid hitting an idle-in-transaction timeout - unless delete_tags(container_repository, disable_timeout) && + if delete_tags(container_repository, disable_timeout) && destroy_container_repository(container_repository) + success + else container_repository.delete_failed! + error('Deletion failed for container repository') end end @@ -40,9 +43,19 @@ module Projects false end + def can_destroy? + return true if skip_permission_check? + + can?(current_user, :destroy_container_image, project) + end + def error_message(container_repository, message) - "Container repository with ID: #{container_repository.id} and path: #{container_repository.path}" \ - " failed with message: #{message}" + "Container repository with ID: #{container_repository.id} and path: #{container_repository.path} " \ + "failed with message: #{message}" + end + + def skip_permission_check? + !!params[:skip_permission_check] end end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index d3313526eaf..94cc4700a49 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -125,7 +125,7 @@ module Projects setup_authorizations - current_user.invalidate_personal_projects_count + project.invalidate_personal_projects_count_of_owner Projects::PostCreationWorker.perform_async(@project.id) @@ -160,7 +160,6 @@ module Projects # AuthorizedProjectsWorker but with some delay and lower urgency as a # safety net. @project.group.refresh_members_authorized_projects( - blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) else @@ -198,7 +197,7 @@ module Projects end def create_sast_commit - ::Security::CiConfiguration::SastCreateService.new(@project, current_user, {}, commit_on_default: true).execute + ::Security::CiConfiguration::SastCreateService.new(@project, current_user, { initialize_with_sast: true }, commit_on_default: true).execute end def readme_content diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 4e883f682fb..2279ab301dc 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -34,7 +34,7 @@ module Projects publish_project_deleted_event_for(project) - current_user.invalidate_personal_projects_count + project.invalidate_personal_projects_count_of_owner true rescue StandardError => error @@ -257,12 +257,12 @@ module Projects return true unless Gitlab.config.registry.enabled return false unless remove_legacy_registry_tags + results = [] project.container_repositories.find_each do |container_repository| - service = Projects::ContainerRepository::DestroyService.new(project, current_user) - service.execute(container_repository) + results << destroy_repository(project, container_repository) end - true + results.all? end ## @@ -272,9 +272,14 @@ module Projects def remove_legacy_registry_tags return true unless Gitlab.config.registry.enabled - ::ContainerRepository.build_root_repository(project).tap do |repository| - break repository.has_tags? ? repository.delete_tags! : true - end + root_repository = ::ContainerRepository.build_root_repository(project) + root_repository.has_tags? ? destroy_repository(project, root_repository) : true + end + + def destroy_repository(project, repository) + service = ContainerRepository::DestroyService.new(project, current_user, { skip_permission_check: true }) + response = service.execute(repository) + response[:status] == :success end def raise_error(message) diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb index 72036aaff35..f77bae71d63 100644 --- a/app/services/projects/group_links/create_service.rb +++ b/app/services/projects/group_links/create_service.rb @@ -36,7 +36,6 @@ module Projects # AuthorizedProjectsWorker but with some delay and lower urgency as a # safety net. shared_with_group.refresh_members_authorized_projects( - blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) end diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb index 19df0dc2c73..a2307bfebf0 100644 --- a/app/services/projects/group_links/destroy_service.rb +++ b/app/services/projects/group_links/destroy_service.rb @@ -19,7 +19,6 @@ module Projects # the old approach, we still run AuthorizedProjectsWorker # but with some delay and lower urgency as a safety net. link.group.refresh_members_authorized_projects( - blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) end diff --git a/app/services/projects/group_links/update_service.rb b/app/services/projects/group_links/update_service.rb index c271b0a2307..9b2565adaca 100644 --- a/app/services/projects/group_links/update_service.rb +++ b/app/services/projects/group_links/update_service.rb @@ -26,7 +26,6 @@ module Projects # the old approach, we still run AuthorizedProjectsWorker # but with some delay and lower urgency as a safety net. group_link.group.refresh_members_authorized_projects( - blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) end diff --git a/app/services/projects/protect_default_branch_service.rb b/app/services/projects/protect_default_branch_service.rb index 03d1c49657d..5360902038b 100644 --- a/app/services/projects/protect_default_branch_service.rb +++ b/app/services/projects/protect_default_branch_service.rb @@ -45,7 +45,11 @@ module Projects end def protected_branch_exists? - project.protected_branches.find_by_name(default_branch).present? + if Feature.enabled?(:group_protected_branches) + project.all_protected_branches.find_by_name(default_branch).present? + else + project.protected_branches.find_by_name(default_branch).present? + end end def default_branch diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 3cb5a564ba5..ed99c69be07 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -32,9 +32,9 @@ module Projects raise TransferError, s_("TransferProject|You don't have permission to transfer projects into that namespace.") end - transfer(project) + @owner_of_personal_project_before_transfer = project.namespace.owner if project.personal? - current_user.invalidate_personal_projects_count + transfer(project) true rescue Projects::TransferService::TransferError => ex @@ -121,6 +121,7 @@ module Projects # Overridden in EE def post_update_hooks(project) ensure_personal_project_owner_membership(project) + invalidate_personal_projects_counts publish_event end @@ -129,6 +130,18 @@ module Projects def remove_paid_features end + def invalidate_personal_projects_counts + # If the project was moved out of a personal namespace, + # the cache of the namespace owner, before the transfer, should be cleared. + if @owner_of_personal_project_before_transfer.present? + @owner_of_personal_project_before_transfer.invalidate_personal_projects_count + end + + # If the project has now moved into a personal namespace, + # the cache of the target namespace owner should be cleared. + project.invalidate_personal_projects_count_of_owner + end + def transfer_missing_group_resources(group) Labels::TransferService.new(current_user, group, project).execute @@ -179,7 +192,6 @@ module Projects # the old approach, we still run AuthorizedProjectsWorker # but with some delay and lower urgency as a safety net. UserProjectAccessChangedService.new(user_ids).execute( - blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) end diff --git a/app/services/protected_branches/cache_service.rb b/app/services/protected_branches/cache_service.rb index af8c9ce74bb..4a9fc335421 100644 --- a/app/services/protected_branches/cache_service.rb +++ b/app/services/protected_branches/cache_service.rb @@ -81,7 +81,11 @@ module ProtectedBranches end def metrics - @metrics ||= Gitlab::Cache::Metrics.new( + @metrics ||= Gitlab::Cache::Metrics.new(cache_metadata) + end + + def cache_metadata + Gitlab::Cache::Metadata.new( caller_id: Gitlab::ApplicationContext.current_context_attribute(:caller_id), cache_identifier: "#{self.class}#fetch", feature_category: :source_code_management, diff --git a/app/services/protected_branches/destroy_service.rb b/app/services/protected_branches/destroy_service.rb index a32a867491e..011dbf3515d 100644 --- a/app/services/protected_branches/destroy_service.rb +++ b/app/services/protected_branches/destroy_service.rb @@ -5,7 +5,10 @@ module ProtectedBranches def execute(protected_branch) raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_protected_branch, protected_branch) - protected_branch.destroy.tap { refresh_cache } + protected_branch.destroy.tap do + refresh_cache + after_execute + end end end end diff --git a/app/services/quick_actions/target_service.rb b/app/services/quick_actions/target_service.rb index 6eda3c89e6c..04ae5287302 100644 --- a/app/services/quick_actions/target_service.rb +++ b/app/services/quick_actions/target_service.rb @@ -2,37 +2,45 @@ module QuickActions class TargetService < BaseService - def execute(type, type_id) + def execute(type, type_iid) case type&.downcase + when 'workitem' + work_item(type_iid) when 'issue' - issue(type_id) + issue(type_iid) when 'mergerequest' - merge_request(type_id) + merge_request(type_iid) when 'commit' - commit(type_id) + commit(type_iid) end end private # rubocop: disable CodeReuse/ActiveRecord - def issue(type_id) - return project.issues.build if type_id.nil? + def work_item(type_iid) + WorkItems::WorkItemsFinder.new(current_user, project_id: project.id).find_by(iid: type_iid) + end + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord + def issue(type_iid) + return project.issues.build if type_iid.nil? - IssuesFinder.new(current_user, project_id: project.id).find_by(iid: type_id) || project.issues.build + IssuesFinder.new(current_user, project_id: project.id).find_by(iid: type_iid) || project.issues.build end # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord - def merge_request(type_id) - return project.merge_requests.build if type_id.nil? + def merge_request(type_iid) + return project.merge_requests.build if type_iid.nil? - MergeRequestsFinder.new(current_user, project_id: project.id).find_by(iid: type_id) || project.merge_requests.build + MergeRequestsFinder.new(current_user, project_id: project.id).find_by(iid: type_iid) || project.merge_requests.build end # rubocop: enable CodeReuse/ActiveRecord - def commit(type_id) - project.commit(type_id) + def commit(type_iid) + project.commit(type_iid) end end end diff --git a/app/services/releases/base_service.rb b/app/services/releases/base_service.rb index 7fb59dad508..5d6cb372653 100644 --- a/app/services/releases/base_service.rb +++ b/app/services/releases/base_service.rb @@ -58,7 +58,7 @@ module Releases end def milestones - return [] unless param_for_milestone_titles_provided? + return [] unless param_for_milestones_exists? strong_memoize(:milestones) do MilestonesFinder.new( @@ -67,22 +67,44 @@ module Releases project_ids: Array(project.id), group_ids: Array(project_group_id), state: 'all', - title: params[:milestones] + title: params[:milestones], + ids: params[:milestone_ids] ).execute end end - def inexistent_milestones + def inexistent_milestone_titles return [] unless param_for_milestone_titles_provided? existing_milestone_titles = milestones.map(&:title) + Array(params[:milestones]) - existing_milestone_titles end + def inexistent_milestone_ids + return [] unless param_for_milestone_ids_provided? + + existing_milestone_ids = milestones.map(&:id) + + Array(params[:milestone_ids]) - existing_milestone_ids + end + def param_for_milestone_titles_provided? !!params[:milestones] end + def param_for_milestone_ids_provided? + !!params[:milestone_ids] + end + + def param_for_milestones_provided? + param_for_milestone_titles_provided? || param_for_milestone_ids_provided? + end + + def param_for_milestones_exists? + params[:milestones].present? || params[:milestone_ids].present? + end + def execute_hooks(release, action = 'create') release.execute_hooks(action) end diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index 01dd6323d94..a3289f9e552 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -6,7 +6,8 @@ module Releases return error(_('Access Denied'), 403) unless allowed? return error(_('You are not allowed to create this tag as it is protected.'), 403) unless can_create_tag? return error(_('Release already exists'), 409) if release - return error(format(_("Milestone(s) not found: %{milestones}"), milestones: inexistent_milestones.join(', ')), 400) if inexistent_milestones.any? # rubocop:disable Layout/LineLength + return error(format(_("Milestone(s) not found: %{milestones}"), milestones: inexistent_milestone_titles.join(', ')), 400) if inexistent_milestone_titles.any? # rubocop:disable Layout/LineLength + return error(format(_("Milestone id(s) not found: %{milestones}"), milestones: inexistent_milestone_ids.join(', ')), 400) if inexistent_milestone_ids.any? # rubocop:disable Layout/LineLength # should be found before the creation of new tag # because tag creation can spawn new pipeline diff --git a/app/services/releases/update_service.rb b/app/services/releases/update_service.rb index b9b2aba9805..c11d9468814 100644 --- a/app/services/releases/update_service.rb +++ b/app/services/releases/update_service.rb @@ -7,8 +7,8 @@ module Releases return error end - if param_for_milestone_titles_provided? - previous_milestones = release.milestones.map(&:title) + if param_for_milestones_provided? + previous_milestones = release.milestones.map(&:id) params[:milestones] = milestones end @@ -35,7 +35,8 @@ module Releases return error(_('Release does not exist'), 404) unless release return error(_('Access Denied'), 403) unless allowed? return error(_('params is empty'), 400) if empty_params? - return error(format(_("Milestone(s) not found: %{milestones}"), milestones: inexistent_milestones.join(', ')), 400) if inexistent_milestones.any? # rubocop:disable Layout/LineLength + return error(format(_("Milestone(s) not found: %{milestones}"), milestones: inexistent_milestone_titles.join(', ')), 400) if inexistent_milestone_titles.any? # rubocop:disable Layout/LineLength + return error(format(_("Milestone id(s) not found: %{milestones}"), milestones: inexistent_milestone_ids.join(', ')), 400) if inexistent_milestone_ids.any? # rubocop:disable Layout/LineLength end def allowed? @@ -47,9 +48,9 @@ module Releases end def milestones_updated?(previous_milestones) - return false unless param_for_milestone_titles_provided? + return false unless param_for_milestones_provided? - previous_milestones.to_set != release.milestones.map(&:title) + previous_milestones.to_set != release.milestones.map(&:id) end end end diff --git a/app/services/resource_events/base_synthetic_notes_builder_service.rb b/app/services/resource_events/base_synthetic_notes_builder_service.rb index 36de70dc291..e675bb61072 100644 --- a/app/services/resource_events/base_synthetic_notes_builder_service.rb +++ b/app/services/resource_events/base_synthetic_notes_builder_service.rb @@ -32,7 +32,7 @@ module ResourceEvents return events if params[:paginated_notes].nil? return events.none if params[:paginated_notes][table_name].blank? - events.id_in(params[:paginated_notes][table_name].map(&:id)) + events.id_in(params[:paginated_notes][table_name].flat_map(&:ids)) end def apply_last_fetched_at(events) diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb index 7e176f95db0..02182bc3a77 100644 --- a/app/services/resource_events/change_labels_service.rb +++ b/app/services/resource_events/change_labels_service.rb @@ -23,16 +23,22 @@ module ResourceEvents label_hash.merge(label_id: label.id, action: ResourceLabelEvent.actions['remove']) end - ApplicationRecord.legacy_bulk_insert(ResourceLabelEvent.table_name, labels) # rubocop:disable Gitlab/BulkInsert + ids = ApplicationRecord.legacy_bulk_insert(ResourceLabelEvent.table_name, labels, return_ids: true) # rubocop:disable Gitlab/BulkInsert - create_timeline_events_from(added_labels: added_labels, removed_labels: removed_labels) + if resource.is_a?(Issue) + events = ResourceLabelEvent.id_in(ids) + events.first.trigger_note_subscription_create(events: events.to_a) if events.any? + end + create_timeline_events_from(added_labels: added_labels, removed_labels: removed_labels) resource.expire_note_etag_cache return unless resource.is_a?(Issue) - Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_label_changed_action(author: user, - project: resource.project) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_label_changed_action( + author: user, project: resource.project) + + events end private diff --git a/app/services/resource_events/synthetic_milestone_notes_builder_service.rb b/app/services/resource_events/synthetic_milestone_notes_builder_service.rb index 0e2b171e192..18c32ef1152 100644 --- a/app/services/resource_events/synthetic_milestone_notes_builder_service.rb +++ b/app/services/resource_events/synthetic_milestone_notes_builder_service.rb @@ -18,7 +18,7 @@ module ResourceEvents def milestone_change_events return [] unless resource.respond_to?(:resource_milestone_events) - events = resource.resource_milestone_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord + events = resource.resource_milestone_events.includes(:milestone, user: :status) # rubocop: disable CodeReuse/ActiveRecord apply_common_filters(events) end diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 3e93346bfdf..6acc32ea0a8 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -8,9 +8,9 @@ module Search attr_accessor :project, :current_user, :params - def initialize(project_or_projects, user, params) - @project = project_or_projects + def initialize(user, project_or_projects, params) @current_user = user + @project = project_or_projects @params = params.dup end diff --git a/app/services/search_service.rb b/app/services/search_service.rb index b4344a009b2..7fca6ed7a20 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -187,7 +187,7 @@ class SearchService def search_service @search_service ||= if project - Search::ProjectService.new(project, current_user, params) + Search::ProjectService.new(current_user, project, params) elsif show_snippets? Search::SnippetService.new(current_user, params) elsif group diff --git a/app/services/security/ci_configuration/base_create_service.rb b/app/services/security/ci_configuration/base_create_service.rb index aaa850fde39..3e8865d3dff 100644 --- a/app/services/security/ci_configuration/base_create_service.rb +++ b/app/services/security/ci_configuration/base_create_service.rb @@ -12,6 +12,16 @@ module Security end def execute + if project.repository.empty? && !(@params && @params[:initialize_with_sast]) + docs_link = ActionController::Base.helpers.link_to _('add at least one file to the repository'), + Rails.application.routes.url_helpers.help_page_url('user/project/repository/index.md', + anchor: 'add-files-to-a-repository'), + target: '_blank', + rel: 'noopener noreferrer' + raise Gitlab::Graphql::Errors::MutationError, + _(format('You must %s before using Security features.', docs_link.html_safe)).html_safe + end + project.repository.add_branch(current_user, branch_name, project.default_branch) attributes_for_commit = attributes diff --git a/app/services/snippets/count_service.rb b/app/services/snippets/count_service.rb index 9a3d33c75cf..ba421c5777e 100644 --- a/app/services/snippets/count_service.rb +++ b/app/services/snippets/count_service.rb @@ -70,7 +70,7 @@ module Snippets count(case when visibility_level=#{Snippet::PUBLIC} OR visibility_level=#{Snippet::INTERNAL} then 1 else null end) as are_public_or_internal, count(*) as total ") - .first + .take end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb index 0dcb3546034..4ec07bb4c5f 100644 --- a/app/services/spam/spam_verdict_service.rb +++ b/app/services/spam/spam_verdict_service.rb @@ -42,7 +42,7 @@ module Spam # Favour the most restrictive result. verdict = valid_results.min_by { |v| SUPPORTED_VERDICTS[v][:priority] } - # The target can override the verdict via the `allow_possible_spam` feature flag + # The target can override the verdict via the `allow_possible_spam` application setting verdict = OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM if override_via_allow_possible_spam?(verdict: verdict) logger.info(class: self.class.name, diff --git a/app/services/system_notes/base_service.rb b/app/services/system_notes/base_service.rb index ee7784c127b..1f6d8ab2409 100644 --- a/app/services/system_notes/base_service.rb +++ b/app/services/system_notes/base_service.rb @@ -19,8 +19,8 @@ module SystemNotes Note.create(note_params) end - def content_tag(*args) - ActionController::Base.helpers.content_tag(*args) + def content_tag(...) + ActionController::Base.helpers.content_tag(...) end def url_helpers diff --git a/app/services/tasks_to_be_done/base_service.rb b/app/services/tasks_to_be_done/base_service.rb index a5648ad10c4..5851a2cb9e5 100644 --- a/app/services/tasks_to_be_done/base_service.rb +++ b/app/services/tasks_to_be_done/base_service.rb @@ -4,22 +4,22 @@ module TasksToBeDone class BaseService < ::IssuableBaseService LABEL_PREFIX = 'tasks to be done' - def initialize(project:, current_user:, assignee_ids: []) + def initialize(container:, current_user:, assignee_ids: []) params = { assignee_ids: assignee_ids, title: title, description: description, add_labels: label_name } - super(project: project, current_user: current_user, params: params) + super(project: container, current_user: current_user, params: params) end def execute if (issue = existing_task_issue) - update_service = Issues::UpdateService.new(project: project, current_user: current_user, params: { add_assignee_ids: params[:assignee_ids] }) + update_service = Issues::UpdateService.new(container: project, current_user: current_user, params: { add_assignee_ids: params[:assignee_ids] }) update_service.execute(issue) else - build_service = Issues::BuildService.new(project: project, current_user: current_user, params: params) + build_service = Issues::BuildService.new(container: project, current_user: current_user, params: params) create(build_service.execute) end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index bfd1e55507c..42a8aca17d3 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -198,6 +198,23 @@ class TodoService current_user.update_todos_count_cache end + def resolve_access_request_todos(current_user, member) + return if current_user.nil? || member.nil? + + target = member.source + + finder_params = { + state: :pending, + author_id: member.user_id, + action_id: ::Todo::MEMBER_ACCESS_REQUESTED, + type: target.class.polymorphic_name, + target: target.id + } + + todos = TodosFinder.new(current_user, finder_params).execute + resolve_todos(todos, current_user) + end + def restore_todos(todos, current_user) todos_ids = todos.batch_update(state: :pending) diff --git a/app/services/user_project_access_changed_service.rb b/app/services/user_project_access_changed_service.rb index f7178ee9bb6..c282dcf176c 100644 --- a/app/services/user_project_access_changed_service.rb +++ b/app/services/user_project_access_changed_service.rb @@ -12,21 +12,19 @@ class UserProjectAccessChangedService @user_ids = Array.wrap(user_ids) end - def execute(blocking: true, priority: HIGH_PRIORITY) + def execute(priority: HIGH_PRIORITY) return if @user_ids.empty? bulk_args = @user_ids.map { |id| [id] } result = - if blocking - AuthorizedProjectsWorker.bulk_perform_and_wait(bulk_args) - else - case priority - when HIGH_PRIORITY - AuthorizedProjectsWorker.bulk_perform_async(bulk_args) # rubocop:disable Scalability/BulkPerformWithContext - when MEDIUM_PRIORITY - AuthorizedProjectUpdate::UserRefreshWithLowUrgencyWorker.bulk_perform_in(MEDIUM_DELAY, bulk_args, batch_size: 100, batch_delay: 30.seconds) # rubocop:disable Scalability/BulkPerformWithContext - else + case priority + when HIGH_PRIORITY + AuthorizedProjectsWorker.bulk_perform_async(bulk_args) # rubocop:disable Scalability/BulkPerformWithContext + when MEDIUM_PRIORITY + AuthorizedProjectUpdate::UserRefreshWithLowUrgencyWorker.bulk_perform_in(MEDIUM_DELAY, bulk_args, batch_size: 100, batch_delay: 30.seconds) # rubocop:disable Scalability/BulkPerformWithContext + when LOW_PRIORITY + if Feature.disabled?(:do_not_run_safety_net_auth_refresh_jobs) with_related_class_context do # We wrap the execution in `with_related_class_context`so as to obtain # the location of the original caller diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb index 4978f778870..c8f9c28061f 100644 --- a/app/services/users/activity_service.rb +++ b/app/services/users/activity_service.rb @@ -4,38 +4,56 @@ module Users class ActivityService LEASE_TIMEOUT = 1.minute.to_i - def initialize(author) + def initialize(author:, namespace: nil, project: nil) @user = if author.respond_to?(:username) author elsif author.respond_to?(:user) author.user end - @user = nil unless @user.is_a?(User) + @user = nil unless user.is_a?(User) + @namespace = namespace + @project = project end def execute - return unless @user + return unless user ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes { record_activity } end private + attr_reader :user, :namespace, :project + def record_activity return if Gitlab::Database.read_only? today = Date.today - return if @user.last_activity_on == today + return if user.last_activity_on == today - lease = Gitlab::ExclusiveLease.new("activity_service:#{@user.id}", + lease = Gitlab::ExclusiveLease.new("activity_service:#{user.id}", timeout: LEASE_TIMEOUT) return unless lease.try_obtain - @user.update_attribute(:last_activity_on, today) + user.update_attribute(:last_activity_on, today) + + Gitlab::UsageDataCounters::HLLRedisCounter.track_event('unique_active_user', values: user.id) + + return unless Feature.enabled?(:route_hll_to_snowplow_phase3) - Gitlab::UsageDataCounters::HLLRedisCounter.track_event('unique_active_user', values: @user.id) + Gitlab::Tracking.event( + 'Users::ActivityService', + 'perform_action', + user: user, + namespace: namespace, + project: project, + label: 'redis_hll_counters.manage.unique_active_users_monthly', + context: [ + Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'unique_active_user').to_context + ] + ) end end end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 064bf132d3d..934dccf2f76 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -163,6 +163,7 @@ module Users :skype, :theme_id, :twitter, + :discord, :username, :website_url, :private_profile, @@ -177,19 +178,17 @@ module Users # Allowed params for user signup def signup_params - signup_params = [ + [ :email, :name, :password, :password_automatically_set, + :preferred_language, :username, :user_type, :first_name, :last_name ] - signup_params << :preferred_language if ::Feature.enabled?(:preferred_language_switcher) - - signup_params end end end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index d32dcd73734..9ab6fcc9832 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -142,6 +142,7 @@ class WebHookService log_data = { trigger: hook_name, url: hook.url, + interpolated_url: hook.interpolated_url, execution_duration: execution_duration, request_headers: build_headers, request_data: data, diff --git a/app/services/work_items/create_and_link_service.rb b/app/services/work_items/create_and_link_service.rb index 351ebc14564..ae09e44b952 100644 --- a/app/services/work_items/create_and_link_service.rb +++ b/app/services/work_items/create_and_link_service.rb @@ -16,7 +16,7 @@ module WorkItems def execute create_result = CreateService.new( - project: @project, + container: @project, current_user: @current_user, params: @params.merge(title: @params[:title].strip).reverse_merge(confidential: confidential_parent), spam_params: @spam_params diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index c89ebc75b80..eff2132039f 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -4,13 +4,13 @@ module WorkItems class CreateService < Issues::CreateService include WidgetableService - def initialize(project:, spam_params:, current_user: nil, params: {}, widget_params: {}) + def initialize(container:, spam_params:, current_user: nil, params: {}, widget_params: {}) super( - project: project, + container: container, current_user: current_user, params: params, spam_params: spam_params, - build_service: ::WorkItems::BuildService.new(project: project, current_user: current_user, params: params) + build_service: ::WorkItems::BuildService.new(container: container, current_user: current_user, params: params) ) @widget_params = widget_params end diff --git a/app/services/work_items/delete_task_service.rb b/app/services/work_items/delete_task_service.rb index 2a82a993b71..3d66716543a 100644 --- a/app/services/work_items/delete_task_service.rb +++ b/app/services/work_items/delete_task_service.rb @@ -25,7 +25,7 @@ module WorkItems break ::ServiceResponse.error(message: replacement_result.errors, http_status: 422) if replacement_result.error? delete_result = ::WorkItems::DeleteService.new( - project: @task.project, + container: @task.project, current_user: @current_user ).execute(@task) diff --git a/app/services/work_items/export_csv_service.rb b/app/services/work_items/export_csv_service.rb new file mode 100644 index 00000000000..9bef75e2c40 --- /dev/null +++ b/app/services/work_items/export_csv_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module WorkItems + class ExportCsvService < ExportCsv::BaseService + NotAvailableError = StandardError.new('This feature is currently behind a feature flag and it is not available.') + + def csv_data + raise NotAvailableError unless Feature.enabled?(:import_export_work_items_csv, resource_parent) + + super + end + + def email(mail_to_user) + # TODO - will be implemented as part of https://gitlab.com/gitlab-org/gitlab/-/issues/379082 + end + + private + + def associations_to_preload + [:work_item_type, :author] + end + + def header_to_value_hash + { + 'Id' => 'iid', + 'Title' => 'title', + 'Type' => ->(work_item) { work_item.work_item_type.name }, + 'Author' => 'author_name', + 'Author Username' => ->(work_item) { work_item.author.username }, + 'Created At (UTC)' => ->(work_item) { work_item.created_at.to_s(:csv) } + } + end + end +end diff --git a/app/services/work_items/task_list_reference_removal_service.rb b/app/services/work_items/task_list_reference_removal_service.rb index 9152580bef0..843b03906ac 100644 --- a/app/services/work_items/task_list_reference_removal_service.rb +++ b/app/services/work_items/task_list_reference_removal_service.rb @@ -39,7 +39,7 @@ module WorkItems end ::WorkItems::UpdateService.new( - project: @work_item.project, + container: @work_item.project, current_user: @current_user, params: { description: source_lines.join("\n"), lock_version: @lock_version } ).execute(@work_item) diff --git a/app/services/work_items/task_list_reference_replacement_service.rb b/app/services/work_items/task_list_reference_replacement_service.rb index b098d67561b..d81576909d9 100644 --- a/app/services/work_items/task_list_reference_replacement_service.rb +++ b/app/services/work_items/task_list_reference_replacement_service.rb @@ -34,7 +34,7 @@ module WorkItems remove_additional_lines!(source_lines) ::WorkItems::UpdateService.new( - project: @work_item.project, + container: @work_item.project, current_user: @current_user, params: { description: source_lines.join("\n"), lock_version: @lock_version } ).execute(@work_item) diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb index 1351445f6f3..d4acadbc851 100644 --- a/app/services/work_items/update_service.rb +++ b/app/services/work_items/update_service.rb @@ -4,10 +4,10 @@ module WorkItems class UpdateService < ::Issues::UpdateService include WidgetableService - def initialize(project:, current_user: nil, params: {}, spam_params: nil, widget_params: {}) + def initialize(container:, current_user: nil, params: {}, spam_params: nil, widget_params: {}) params[:widget_params] = true if widget_params.present? - super(project: project, current_user: current_user, params: params, spam_params: nil) + super(container: container, current_user: current_user, params: params, spam_params: spam_params) @widget_params = widget_params end |