diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-12-20 13:37:47 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-12-20 13:37:47 +0000 |
| commit | aee0a117a889461ce8ced6fcf73207fe017f1d99 (patch) | |
| tree | 891d9ef189227a8445d83f35c1b0fc99573f4380 /app/services | |
| parent | 8d46af3258650d305f53b819eabf7ab18d22f59e (diff) | |
| download | gitlab-ce-aee0a117a889461ce8ced6fcf73207fe017f1d99.tar.gz | |
Add latest changes from gitlab-org/gitlab@14-6-stable-eev14.6.0-rc42
Diffstat (limited to 'app/services')
101 files changed, 1070 insertions, 557 deletions
diff --git a/app/services/admin/propagate_service_template.rb b/app/services/admin/propagate_service_template.rb deleted file mode 100644 index c251537c479..00000000000 --- a/app/services/admin/propagate_service_template.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module Admin - class PropagateServiceTemplate - include PropagateService - - def propagate - # TODO: Remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/335178 - end - end -end diff --git a/app/services/audit_event_service.rb b/app/services/audit_event_service.rb index 563d4a924fc..1426bf25a00 100644 --- a/app/services/audit_event_service.rb +++ b/app/services/audit_event_service.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class AuditEventService + include AuditEventSaveType + # Instantiates a new service # # @param [User] author the user who authors the change @@ -10,13 +12,16 @@ class AuditEventService # - Group: events are visible at Group and Instance level # - User: events are visible at Instance level # @param [Hash] details extra data of audit event + # @param [Symbol] save_type the type to save the event + # Can be selected from the following, :database, :stream, :database_and_stream . # # @return [AuditEventService] - def initialize(author, entity, details = {}) + def initialize(author, entity, details = {}, save_type = :database_and_stream) @author = build_author(author) @entity = entity @details = details @ip_address = resolve_ip_address(@author) + @save_type = save_type end # Builds the @details attribute for authentication @@ -133,8 +138,8 @@ class AuditEventService end def save_or_track(event) - event.save! - stream_event_to_external_destinations(event) + event.save! if should_save_database?(@save_type) + stream_event_to_external_destinations(event) if should_save_stream?(@save_type) rescue StandardError => e Gitlab::ErrorTracking.track_exception(e, audit_event_type: event.class.to_s) end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index bc734465750..ea4723c9e28 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -156,7 +156,7 @@ module Auth return if path.has_repository? return unless actions.include?('push') - ContainerRepository.create_from_path!(path) + ContainerRepository.find_or_create_from_path(path) end # Overridden in EE diff --git a/app/services/authorized_project_update/find_records_due_for_refresh_service.rb b/app/services/authorized_project_update/find_records_due_for_refresh_service.rb index c4b18a26d0e..3a2251f15cc 100644 --- a/app/services/authorized_project_update/find_records_due_for_refresh_service.rb +++ b/app/services/authorized_project_update/find_records_due_for_refresh_service.rb @@ -47,7 +47,11 @@ module AuthorizedProjectUpdate missing_auth_found_callback.call(project_id, level) end - array << [user.id, project_id, level] + array << { + user_id: user.id, + project_id: project_id, + access_level: level + } end end diff --git a/app/services/authorized_project_update/project_group_link_create_service.rb b/app/services/authorized_project_update/project_group_link_create_service.rb index e9e7c56d7c7..10cf4c50569 100644 --- a/app/services/authorized_project_update/project_group_link_create_service.rb +++ b/app/services/authorized_project_update/project_group_link_create_service.rb @@ -65,16 +65,8 @@ module AuthorizedProjectUpdate end def update_authorizations(user_ids_to_delete, authorizations_to_create) - ProjectAuthorization.transaction do - if user_ids_to_delete.any? - ProjectAuthorization.where(project_id: project.id, user_id: user_ids_to_delete) # rubocop: disable CodeReuse/ActiveRecord - .delete_all - end - - if authorizations_to_create.any? - ProjectAuthorization.insert_all(authorizations_to_create) - end - end + project.remove_project_authorizations(user_ids_to_delete) if user_ids_to_delete.any? + ProjectAuthorization.insert_all_in_batches(authorizations_to_create) if authorizations_to_create.any? end end end diff --git a/app/services/authorized_project_update/project_recalculate_service.rb b/app/services/authorized_project_update/project_recalculate_service.rb index d70d0efc2af..17ba48cffcd 100644 --- a/app/services/authorized_project_update/project_recalculate_service.rb +++ b/app/services/authorized_project_update/project_recalculate_service.rb @@ -64,16 +64,8 @@ module AuthorizedProjectUpdate end def refresh_authorizations - ProjectAuthorization.transaction do - if user_ids_to_remove.any? - ProjectAuthorization.where(project_id: project.id, user_id: user_ids_to_remove) # rubocop: disable CodeReuse/ActiveRecord - .delete_all - end - - if authorizations_to_create.any? - ProjectAuthorization.insert_all(authorizations_to_create) - end - end + project.remove_project_authorizations(user_ids_to_remove) if user_ids_to_remove.any? + ProjectAuthorization.insert_all_in_batches(authorizations_to_create) if authorizations_to_create.any? end def apply_scopes(project_authorizations) diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index e756e8c14d8..da80211f9bb 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -64,7 +64,7 @@ module AutoMerge # NOTE: This method is to be removed when `disallow_to_create_merge_request_pipelines_in_target_project` # feature flag is removed. def self.can_add_to_merge_train?(merge_request) - if Gitlab::Ci::Features.disallow_to_create_merge_request_pipelines_in_target_project?(merge_request.target_project) + if ::Feature.enabled?(:ci_disallow_to_create_merge_request_pipelines_in_target_project, merge_request.target_project) merge_request.for_same_project? else true diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index c1becbb5609..cbf2b34b33c 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -59,7 +59,7 @@ module BulkImports ) bulk_import.create_configuration!(credentials.slice(:url, :access_token)) - params.each do |entity| + Array.wrap(params).each do |entity| BulkImports::Entity.create!( bulk_import: bulk_import, source_type: entity[:source_type], diff --git a/app/services/bulk_imports/relation_export_service.rb b/app/services/bulk_imports/relation_export_service.rb index 4718b3914b2..14f073120c5 100644 --- a/app/services/bulk_imports/relation_export_service.rb +++ b/app/services/bulk_imports/relation_export_service.rb @@ -59,7 +59,7 @@ module BulkImports end def export_service - @export_service ||= if config.tree_relation?(relation) + @export_service ||= if config.tree_relation?(relation) || config.self_relation?(relation) TreeExportService.new(portable, config.export_path, relation) elsif config.file_relation?(relation) FileExportService.new(portable, config.export_path, relation) diff --git a/app/services/bulk_imports/tree_export_service.rb b/app/services/bulk_imports/tree_export_service.rb index b8e7ac4574b..8e885e590d1 100644 --- a/app/services/bulk_imports/tree_export_service.rb +++ b/app/services/bulk_imports/tree_export_service.rb @@ -10,6 +10,8 @@ module BulkImports end def execute + return serializer.serialize_root(config.class::SELF_RELATION) if self_relation? + relation_definition = config.tree_relation_definition_for(relation) raise BulkImports::Error, 'Unsupported relation export type' unless relation_definition @@ -18,6 +20,8 @@ module BulkImports end def exported_filename + return "#{relation}.json" if self_relation? + "#{relation}.ndjson" end @@ -39,5 +43,9 @@ module BulkImports def json_writer ::Gitlab::ImportExport::Json::NdjsonWriter.new(export_path) end + + def self_relation? + relation == config.class::SELF_RELATION + end end end diff --git a/app/services/bulk_imports/uploads_export_service.rb b/app/services/bulk_imports/uploads_export_service.rb index 32cc48c152c..7f5ee7b8624 100644 --- a/app/services/bulk_imports/uploads_export_service.rb +++ b/app/services/bulk_imports/uploads_export_service.rb @@ -5,6 +5,7 @@ module BulkImports include Gitlab::ImportExport::CommandLineUtil BATCH_SIZE = 100 + AVATAR_PATH = 'avatar' def initialize(portable, export_path) @portable = portable @@ -34,7 +35,7 @@ module BulkImports def export_subdir_path(upload) subdir = if upload.path == avatar_path - 'avatar' + AVATAR_PATH else upload.try(:secret).to_s end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 540e8f7b970..c1f35afba40 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -2,10 +2,14 @@ module Ci class CreatePipelineService < BaseService - attr_reader :pipeline + attr_reader :pipeline, :logger CreateError = Class.new(StandardError) + LOG_MAX_DURATION_THRESHOLD = 3.seconds + LOG_MAX_PIPELINE_SIZE = 2_000 + LOG_MAX_CREATION_THRESHOLD = 20.seconds + SEQUENCE = [Gitlab::Ci::Pipeline::Chain::Build, Gitlab::Ci::Pipeline::Chain::Build::Associations, Gitlab::Ci::Pipeline::Chain::Validate::Abilities, @@ -24,7 +28,10 @@ module Ci Gitlab::Ci::Pipeline::Chain::Validate::External, Gitlab::Ci::Pipeline::Chain::Populate, Gitlab::Ci::Pipeline::Chain::StopDryRun, + 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::Limit::JobActivity, @@ -53,6 +60,7 @@ module Ci # @return [Ci::Pipeline] The created Ci::Pipeline object. # rubocop: disable Metrics/ParameterLists def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, merge_request: nil, external_pull_request: nil, bridge: nil, **options, &block) + @logger = build_logger @pipeline = Ci::Pipeline.new command = Gitlab::Ci::Pipeline::Chain::Command.new( @@ -76,6 +84,7 @@ module Ci push_options: params[:push_options] || {}, chat_data: params[:chat_data], bridge: bridge, + logger: @logger, **extra_options(**options)) # Ensure we never persist the pipeline when dry_run: true @@ -98,6 +107,9 @@ module Ci else ServiceResponse.success(payload: pipeline) end + + ensure + @logger.commit(pipeline: pipeline, caller: self.class.name) end # rubocop: enable Metrics/ParameterLists @@ -135,6 +147,32 @@ module Ci def extra_options(content: nil, dry_run: false) { content: content, dry_run: dry_run } end + + def build_logger + Gitlab::Ci::Pipeline::Logger.new(project: project) do |l| + l.log_when do |observations| + observations.any? do |name, values| + values.any? && + name.to_s.end_with?('duration_s') && + values.max >= LOG_MAX_DURATION_THRESHOLD + end + end + + l.log_when do |observations| + values = observations['pipeline_size_count'] + next false if values.empty? + + values.max >= LOG_MAX_PIPELINE_SIZE + end + + l.log_when do |observations| + values = observations['pipeline_creation_duration_s'] + next false if values.empty? + + values.max >= LOG_MAX_CREATION_THRESHOLD + end + end + end end end diff --git a/app/services/ci/expire_pipeline_cache_service.rb b/app/services/ci/expire_pipeline_cache_service.rb index 48a6344f576..8622b1a5863 100644 --- a/app/services/ci/expire_pipeline_cache_service.rb +++ b/app/services/ci/expire_pipeline_cache_service.rb @@ -60,6 +60,10 @@ module Ci url_helpers.graphql_etag_pipeline_sha_path(sha) end + def graphql_project_on_demand_scan_counts_path(project) + url_helpers.graphql_etag_project_on_demand_scan_counts_path(project) + end + # Updates ETag caches of a pipeline. # # This logic resides in a separate method so that EE can more easily extend @@ -70,18 +74,25 @@ module Ci def update_etag_cache(pipeline, store) project = pipeline.project - store.touch(project_pipelines_path(project)) - store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil? - store.touch(new_merge_request_pipelines_path(project)) + etag_paths = [ + project_pipelines_path(project), + new_merge_request_pipelines_path(project), + graphql_project_on_demand_scan_counts_path(project) + ] + + etag_paths << commit_pipelines_path(project, pipeline.commit) unless pipeline.commit.nil? + each_pipelines_merge_request_path(pipeline) do |path| - store.touch(path) + etag_paths << path end - pipeline.self_with_upstreams_and_downstreams.each do |relative_pipeline| - store.touch(project_pipeline_path(relative_pipeline.project, relative_pipeline)) - store.touch(graphql_pipeline_path(relative_pipeline)) - store.touch(graphql_pipeline_sha_path(relative_pipeline.sha)) + pipeline.self_with_upstreams_and_downstreams.includes(project: [:route, { namespace: :route }]).each do |relative_pipeline| # rubocop: disable CodeReuse/ActiveRecord + etag_paths << project_pipeline_path(relative_pipeline.project, relative_pipeline) + etag_paths << graphql_pipeline_path(relative_pipeline) + etag_paths << graphql_pipeline_sha_path(relative_pipeline.sha) end + + store.touch(*etag_paths) end def url_helpers diff --git a/app/services/ci/job_artifacts/destroy_all_expired_service.rb b/app/services/ci/job_artifacts/destroy_all_expired_service.rb index e4f65736a58..7fa56677a0c 100644 --- a/app/services/ci/job_artifacts/destroy_all_expired_service.rb +++ b/app/services/ci/job_artifacts/destroy_all_expired_service.rb @@ -14,6 +14,7 @@ module Ci def initialize @removed_artifacts_count = 0 + @start_at = Time.current end ## @@ -25,9 +26,9 @@ module Ci def execute in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do if ::Feature.enabled?(:ci_destroy_unlocked_job_artifacts) - destroy_unlocked_job_artifacts(Time.current) + destroy_unlocked_job_artifacts else - destroy_job_artifacts_with_slow_iteration(Time.current) + destroy_job_artifacts_with_slow_iteration end end @@ -36,16 +37,37 @@ module Ci private - def destroy_unlocked_job_artifacts(start_at) + def destroy_unlocked_job_artifacts loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do - artifacts = Ci::JobArtifact.expired_before(start_at).artifact_unlocked.limit(BATCH_SIZE) + artifacts = Ci::JobArtifact.expired_before(@start_at).artifact_unlocked.limit(BATCH_SIZE) service_response = destroy_batch(artifacts) @removed_artifacts_count += service_response[:destroyed_artifacts_count] + + update_locked_status_on_unknown_artifacts if service_response[:destroyed_artifacts_count] == 0 + + # Return a truthy value here to prevent exiting #loop_until + @removed_artifacts_count end end - def destroy_job_artifacts_with_slow_iteration(start_at) - Ci::JobArtifact.expired_before(start_at).each_batch(of: BATCH_SIZE, column: :expire_at, order: :desc) do |relation, index| + def update_locked_status_on_unknown_artifacts + build_ids = Ci::JobArtifact.expired_before(@start_at).artifact_unknown.limit(BATCH_SIZE).distinct_job_ids + + return unless build_ids.present? + + locked_pipeline_build_ids = ::Ci::Build.with_pipeline_locked_artifacts.id_in(build_ids).pluck_primary_key + unlocked_pipeline_build_ids = build_ids - locked_pipeline_build_ids + + update_unknown_artifacts(locked_pipeline_build_ids, Ci::JobArtifact.lockeds[:artifacts_locked]) + update_unknown_artifacts(unlocked_pipeline_build_ids, Ci::JobArtifact.lockeds[:unlocked]) + end + + def update_unknown_artifacts(build_ids, locked_value) + Ci::JobArtifact.for_job_ids(build_ids).update_all(locked: locked_value) if build_ids.any? + end + + def destroy_job_artifacts_with_slow_iteration + Ci::JobArtifact.expired_before(@start_at).each_batch(of: BATCH_SIZE, column: :expire_at, order: :desc) do |relation, index| # For performance reasons, join with ci_pipelines after the batch is queried. # See: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47496 artifacts = relation.unlocked @@ -53,7 +75,7 @@ module Ci service_response = destroy_batch(artifacts) @removed_artifacts_count += service_response[:destroyed_artifacts_count] - break if loop_timeout?(start_at) + break if loop_timeout? break if index >= LOOP_LIMIT end end @@ -62,8 +84,8 @@ module Ci Ci::JobArtifacts::DestroyBatchService.new(artifacts).execute end - def loop_timeout?(start_at) - Time.current > start_at + LOOP_TIMEOUT + def loop_timeout? + Time.current > @start_at + LOOP_TIMEOUT end end end diff --git a/app/services/ci/parse_dotenv_artifact_service.rb b/app/services/ci/parse_dotenv_artifact_service.rb index 725ecbcce5d..40e2cd82b4f 100644 --- a/app/services/ci/parse_dotenv_artifact_service.rb +++ b/app/services/ci/parse_dotenv_artifact_service.rb @@ -14,7 +14,7 @@ module Ci Ci::JobVariable.bulk_insert!(variables) success - rescue SizeLimitError, ParserError, ActiveRecord::RecordInvalid => error + rescue SizeLimitError, ParserError, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique => error Gitlab::ErrorTracking.track_exception(error, job_id: artifact.job_id) error(error.message, :bad_request) end @@ -33,13 +33,13 @@ module Ci end def parse!(artifact) - variables = [] + variables = {} artifact.each_blob do |blob| blob.each_line do |line| key, value = scan_line!(line) - variables << Ci::JobVariable.new(job_id: artifact.job_id, + variables[key] = Ci::JobVariable.new(job_id: artifact.job_id, source: :dotenv, key: key, value: value) end end @@ -49,7 +49,7 @@ module Ci "Dotenv files cannot have more than #{dotenv_variable_limit} variables" end - variables + variables.values end def scan_line!(line) diff --git a/app/services/ci/pipeline_processing/atomic_processing_service.rb b/app/services/ci/pipeline_processing/atomic_processing_service.rb index 236d660d829..d8ce063ffb4 100644 --- a/app/services/ci/pipeline_processing/atomic_processing_service.rb +++ b/app/services/ci/pipeline_processing/atomic_processing_service.rb @@ -36,6 +36,10 @@ module Ci update_pipeline! update_statuses_processed! + if Feature.enabled?(:expire_job_and_pipeline_cache_synchronously, pipeline.project, default_enabled: :yaml) + Ci::ExpirePipelineCacheService.new.execute(pipeline) + end + true end diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb index c1cf06a4631..e2673c763f3 100644 --- a/app/services/ci/play_build_service.rb +++ b/app/services/ci/play_build_service.rb @@ -9,7 +9,7 @@ module Ci # if build.enqueue build.tap do |build| - build.update(user: current_user, job_variables_attributes: job_variables_attributes || []) + build.update!(user: current_user, job_variables_attributes: job_variables_attributes || []) AfterRequeueJobService.new(project, current_user).execute(build) end diff --git a/app/services/ci/process_sync_events_service.rb b/app/services/ci/process_sync_events_service.rb new file mode 100644 index 00000000000..6be8c41dc6a --- /dev/null +++ b/app/services/ci/process_sync_events_service.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Ci + class ProcessSyncEventsService + include Gitlab::Utils::StrongMemoize + include ExclusiveLeaseGuard + + BATCH_SIZE = 1000 + + def initialize(sync_event_class, sync_class) + @sync_event_class = sync_event_class + @sync_class = sync_class + end + + def execute + return unless ::Feature.enabled?(:ci_namespace_project_mirrors, default_enabled: :yaml) + + # preventing parallel processing over the same event table + try_obtain_lease { process_events } + + enqueue_worker_if_there_still_event + end + + private + + def process_events + events = @sync_event_class.preload_synced_relation.first(BATCH_SIZE) + + return if events.empty? + + first = events.first + last_processed = nil + + begin + events.each do |event| + @sync_class.sync!(event) + + last_processed = event + end + ensure + # remove events till the one that was last succesfully processed + @sync_event_class.id_in(first.id..last_processed.id).delete_all if last_processed + end + end + + def enqueue_worker_if_there_still_event + @sync_event_class.enqueue_worker if @sync_event_class.exists? + end + + def lease_key + "#{super}::#{@sync_event_class}" + end + + def lease_timeout + 1.minute + end + end +end diff --git a/app/services/ci/queue/build_queue_service.rb b/app/services/ci/queue/build_queue_service.rb index 3c886cb023f..9f476c8a785 100644 --- a/app/services/ci/queue/build_queue_service.rb +++ b/app/services/ci/queue/build_queue_service.rb @@ -24,7 +24,7 @@ module Ci # rubocop:disable CodeReuse/ActiveRecord def builds_for_group_runner - if strategy.use_denormalized_namespace_traversal_ids? + if strategy.use_denormalized_data_strategy? strategy.builds_for_group_runner else # Workaround for weird Rails bug, that makes `runner.groups.to_sql` to return `runner_id = NULL` @@ -89,11 +89,9 @@ module Ci end def runner_projects_relation - if ::Feature.enabled?(:ci_pending_builds_project_runners_decoupling, runner, default_enabled: :yaml) - runner.runner_projects.select('"ci_runner_projects"."project_id"::bigint') - else - runner.projects.without_deleted.with_builds_enabled - end + runner + .runner_projects + .select('"ci_runner_projects"."project_id"::bigint') end end end diff --git a/app/services/ci/queue/builds_table_strategy.rb b/app/services/ci/queue/builds_table_strategy.rb index ac449a5289e..237dd510d50 100644 --- a/app/services/ci/queue/builds_table_strategy.rb +++ b/app/services/ci/queue/builds_table_strategy.rb @@ -57,15 +57,7 @@ module Ci relation.pluck(:id) end - def use_denormalized_shared_runners_data? - false - end - - def use_denormalized_minutes_data? - false - end - - def use_denormalized_namespace_traversal_ids? + def use_denormalized_data_strategy? false end diff --git a/app/services/ci/queue/pending_builds_strategy.rb b/app/services/ci/queue/pending_builds_strategy.rb index 7a913e47df4..47158b8ea1d 100644 --- a/app/services/ci/queue/pending_builds_strategy.rb +++ b/app/services/ci/queue/pending_builds_strategy.rb @@ -23,7 +23,7 @@ module Ci end def builds_matching_tag_ids(relation, ids) - if ::Feature.enabled?(:ci_queueing_denormalize_tags_information, runner, default_enabled: :yaml) + if use_denormalized_data_strategy? relation.for_tags(runner.tags_ids) else relation.merge(CommitStatus.matches_tag_ids(ids, table: 'ci_pending_builds', column: 'build_id')) @@ -31,7 +31,7 @@ module Ci end def builds_with_any_tags(relation) - if ::Feature.enabled?(:ci_queueing_denormalize_tags_information, runner, default_enabled: :yaml) + if use_denormalized_data_strategy? relation.where('cardinality(tag_ids) > 0') else relation.merge(CommitStatus.with_any_tags(table: 'ci_pending_builds', column: 'build_id')) @@ -50,22 +50,14 @@ module Ci relation.pluck(:build_id) end - def use_denormalized_shared_runners_data? - ::Feature.enabled?(:ci_queueing_denormalize_shared_runners_information, runner, type: :development, default_enabled: :yaml) - end - - def use_denormalized_minutes_data? - ::Feature.enabled?(:ci_queueing_denormalize_ci_minutes_information, runner, type: :development, default_enabled: :yaml) - end - - def use_denormalized_namespace_traversal_ids? - ::Feature.enabled?(:ci_queueing_denormalize_namespace_traversal_ids, runner, type: :development, default_enabled: :yaml) + def use_denormalized_data_strategy? + ::Feature.enabled?(:ci_queuing_use_denormalized_data_strategy, default_enabled: :yaml) end private def builds_available_for_shared_runners - if use_denormalized_shared_runners_data? + if use_denormalized_data_strategy? new_builds.with_instance_runners else new_builds diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 67ef4f10709..e0f0f8f58b8 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -269,14 +269,7 @@ module Ci { missing_dependency_failure: -> (build, _) { !build.has_valid_build_dependencies? }, runner_unsupported: -> (build, params) { !build.supported_runner?(params.dig(:info, :features)) }, - archived_failure: -> (build, _) { build.archived? } - }.merge(builds_enabled_checks) - end - - def builds_enabled_checks - return {} unless ::Feature.enabled?(:ci_queueing_builds_enabled_checks, runner, default_enabled: :yaml) - - { + archived_failure: -> (build, _) { build.archived? }, project_deleted: -> (build, _) { build.project.pending_delete? }, builds_disabled: -> (build, _) { !build.project.builds_enabled? } } diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index ebb07de9d29..89fe4ff9f60 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -2,12 +2,14 @@ module Ci class RetryBuildService < ::BaseService + include Gitlab::Utils::StrongMemoize + def self.clone_accessors %i[pipeline project ref tag options name allow_failure stage stage_id stage_idx trigger_request yaml_variables when environment coverage_regex description tag_list protected needs_attributes - resource_group scheduling_type].freeze + job_variables_attributes resource_group scheduling_type].freeze end def self.extra_accessors @@ -45,6 +47,11 @@ module Ci job.save! end end + + if create_deployment_in_separate_transaction? + clone_deployment!(new_build, build) + end + build.reset # refresh the data to get new values of `retried` and `processed`. new_build @@ -63,7 +70,9 @@ module Ci def clone_build(build) project.builds.new(build_attributes(build)).tap do |new_build| - new_build.assign_attributes(deployment_attributes_for(new_build, build)) + unless create_deployment_in_separate_transaction? + new_build.assign_attributes(deployment_attributes_for(new_build, build)) + end end end @@ -72,6 +81,11 @@ module Ci [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend end + if create_deployment_in_separate_transaction? && build.persisted_environment.present? + attributes[:metadata_attributes] ||= {} + attributes[:metadata_attributes][:expanded_environment_name] = build.expanded_environment_name + end + attributes[:user] = current_user attributes end @@ -80,6 +94,26 @@ module Ci ::Gitlab::Ci::Pipeline::Seed::Build .deployment_attributes_for(new_build, old_build.persisted_environment) end + + def clone_deployment!(new_build, old_build) + return unless old_build.deployment.present? + + # We should clone the previous deployment attributes instead of initializing + # new object with `Seed::Deployment`. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/347206 + deployment = ::Gitlab::Ci::Pipeline::Seed::Deployment + .new(new_build, old_build.persisted_environment).to_resource + + return unless deployment + + new_build.create_deployment!(deployment.attributes) + end + + def create_deployment_in_separate_transaction? + strong_memoize(:create_deployment_in_separate_transaction) do + ::Feature.enabled?(:create_deployment_in_separate_transaction, project, default_enabled: :yaml) + end + end end end diff --git a/app/services/ci/stuck_builds/drop_pending_service.rb b/app/services/ci/stuck_builds/drop_pending_service.rb index 4653e701973..dddd1cfb781 100644 --- a/app/services/ci/stuck_builds/drop_pending_service.rb +++ b/app/services/ci/stuck_builds/drop_pending_service.rb @@ -7,7 +7,6 @@ module Ci BUILD_PENDING_OUTDATED_TIMEOUT = 1.day BUILD_PENDING_STUCK_TIMEOUT = 1.hour - BUILD_LOOKBACK = 5.days def execute Gitlab::AppLogger.info "#{self.class}: Cleaning pending timed-out builds" @@ -30,11 +29,11 @@ module Ci # because we want to force the query planner to use the # `ci_builds_gitlab_monitor_metrics` index all the time. def pending_builds(timeout) - if Feature.enabled?(:ci_new_query_for_pending_stuck_jobs) - Ci::Build.pending.created_at_before(timeout).updated_at_before(timeout).order(created_at: :asc, project_id: :asc) - else - Ci::Build.pending.updated_before(lookback: BUILD_LOOKBACK.ago, timeout: timeout) - end + Ci::Build + .pending + .created_at_before(timeout) + .updated_at_before(timeout) + .order(created_at: :asc, project_id: :asc) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb index c1cbf031ca1..146239bb7e5 100644 --- a/app/services/ci/update_build_queue_service.rb +++ b/app/services/ci/update_build_queue_service.rb @@ -14,7 +14,7 @@ module Ci # Add a build to the pending builds queue # def push(build, transition) - return unless maintain_pending_builds_queue?(build) + return unless maintain_pending_builds_queue? raise InvalidQueueTransition unless transition.to == 'pending' @@ -33,7 +33,7 @@ module Ci # Remove a build from the pending builds queue # def pop(build, transition) - return unless maintain_pending_builds_queue?(build) + return unless maintain_pending_builds_queue? raise InvalidQueueTransition unless transition.from == 'pending' @@ -52,7 +52,7 @@ module Ci # Add shared runner build tracking entry (used for queuing). # def track(build, transition) - return unless Feature.enabled?(:ci_track_shared_runner_builds, build.project, default_enabled: :yaml) + return unless maintain_pending_builds_queue? return unless build.shared_runner_build? raise InvalidQueueTransition unless transition.to == 'running' @@ -73,7 +73,7 @@ module Ci # queuing). # def untrack(build, transition) - return unless Feature.enabled?(:ci_untrack_shared_runner_builds, build.project, default_enabled: :yaml) + return unless maintain_pending_builds_queue? return unless build.shared_runner_build? raise InvalidQueueTransition unless transition.from == 'running' @@ -113,8 +113,8 @@ module Ci end end - def maintain_pending_builds_queue?(build) - Feature.enabled?(:ci_pending_builds_queue_maintain, build.project, default_enabled: :yaml) + def maintain_pending_builds_queue? + ::Ci::PendingBuild.maintain_denormalized_data? end end end diff --git a/app/services/ci/update_build_state_service.rb b/app/services/ci/update_build_state_service.rb index 826d9a2eda3..9df36b86404 100644 --- a/app/services/ci/update_build_state_service.rb +++ b/app/services/ci/update_build_state_service.rb @@ -216,11 +216,12 @@ module Ci end def chunks_migration_enabled? - ::Gitlab::Ci::Features.accept_trace?(build.project) + ::Feature.enabled?(:ci_enable_live_trace, build.project) && + ::Feature.enabled?(:ci_accept_trace, build.project, type: :ops, default_enabled: true) end def log_invalid_chunks? - ::Gitlab::Ci::Features.log_invalid_trace_chunks?(build.project) + ::Feature.enabled?(:ci_trace_log_invalid_chunks, build.project, type: :ops, default_enabled: false) end end end diff --git a/app/services/ci/update_pending_build_service.rb b/app/services/ci/update_pending_build_service.rb index d546dbcfe3d..733b684bcc6 100644 --- a/app/services/ci/update_pending_build_service.rb +++ b/app/services/ci/update_pending_build_service.rb @@ -9,13 +9,13 @@ module Ci def initialize(model, update_params) @model = model - @update_params = update_params + @update_params = update_params.symbolize_keys validations! end def execute - return unless ::Feature.enabled?(:ci_pending_builds_maintain_shared_runners_data, @model, default_enabled: :yaml) + return unless ::Ci::PendingBuild.maintain_denormalized_data? @model.pending_builds.each_batch do |relation| relation.update_all(@update_params) diff --git a/app/services/clusters/agent_tokens/create_service.rb b/app/services/clusters/agent_tokens/create_service.rb index ae2617f510b..5b8a0e46a6c 100644 --- a/app/services/clusters/agent_tokens/create_service.rb +++ b/app/services/clusters/agent_tokens/create_service.rb @@ -11,6 +11,8 @@ module Clusters token = ::Clusters::AgentToken.new(filtered_params.merge(created_by_user: current_user)) if token.save + log_activity_event!(token) + ServiceResponse.success(payload: { secret: token.token, token: token }) else ServiceResponse.error(message: token.errors.full_messages) @@ -26,6 +28,16 @@ module Clusters def filtered_params params.slice(*ALLOWED_PARAMS) end + + def log_activity_event!(token) + token.agent.activity_events.create!( + kind: :token_created, + level: :info, + recorded_at: token.created_at, + user: current_user, + agent_token: token + ) + end end end end diff --git a/app/services/clusters/cleanup/project_namespace_service.rb b/app/services/clusters/cleanup/project_namespace_service.rb index 0173f93f625..80192aa14ab 100644 --- a/app/services/clusters/cleanup/project_namespace_service.rb +++ b/app/services/clusters/cleanup/project_namespace_service.rb @@ -26,8 +26,10 @@ module Clusters begin kubeclient_delete_namespace(kubernetes_namespace) - rescue Kubeclient::HttpError - next + rescue Kubeclient::HttpError => e + # unauthorized, forbidden: GitLab's access has been revoked + # certificate verify failed: Cluster is probably gone forever + raise unless e.message =~ /unauthorized|forbidden|certificate verify failed/i end kubernetes_namespace.destroy! diff --git a/app/services/clusters/cleanup/service_account_service.rb b/app/services/clusters/cleanup/service_account_service.rb index 53f968cd409..dce41d2a39c 100644 --- a/app/services/clusters/cleanup/service_account_service.rb +++ b/app/services/clusters/cleanup/service_account_service.rb @@ -24,6 +24,10 @@ module Clusters # The resources have already been deleted, possibly on a previous attempt that timed out rescue Gitlab::UrlBlocker::BlockedUrlError # User gave an invalid cluster from the start, or deleted the endpoint before this job ran + rescue Kubeclient::HttpError => e + # unauthorized, forbidden: GitLab's access has been revoked + # certificate verify failed: Cluster is probably gone forever + raise unless e.message =~ /unauthorized|forbidden|certificate verify failed/i end end end diff --git a/app/services/cohorts_service.rb b/app/services/cohorts_service.rb index 7bc3b267a12..1b1598b301c 100644 --- a/app/services/cohorts_service.rb +++ b/app/services/cohorts_service.rb @@ -38,7 +38,7 @@ class CohortsService { registration_month: registration_month, - activity_months: activity_months[1..-1], + activity_months: activity_months[1..], total: activity_months.first[:total], inactive: inactive } diff --git a/app/services/concerns/admin/propagate_service.rb b/app/services/concerns/admin/propagate_service.rb deleted file mode 100644 index 03e422aec54..00000000000 --- a/app/services/concerns/admin/propagate_service.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -module Admin - module PropagateService - extend ActiveSupport::Concern - - BATCH_SIZE = 10_000 - - class_methods do - def propagate(integration) - new(integration).propagate - end - end - - def initialize(integration) - @integration = integration - end - - private - - attr_reader :integration - - def create_integration_for_projects_without_integration - propagate_integrations( - Project.without_integration(integration), - PropagateIntegrationProjectWorker - ) - end - - def propagate_integrations(relation, worker_class) - relation.each_batch(of: BATCH_SIZE) do |records| - min_id, max_id = records.pick("MIN(#{relation.table_name}.id), MAX(#{relation.table_name}.id)") - worker_class.perform_async(integration.id, min_id, max_id) - end - end - end -end diff --git a/app/services/concerns/audit_event_save_type.rb b/app/services/concerns/audit_event_save_type.rb new file mode 100644 index 00000000000..6696e4adae7 --- /dev/null +++ b/app/services/concerns/audit_event_save_type.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module AuditEventSaveType + SAVE_TYPES = { + database: 0b01, + stream: 0b10, + database_and_stream: 0b11 + }.freeze + + # def should_save_stream?(type) + # def should_save_database?(type) + [:database, :stream].each do |type| + define_method("should_save_#{type}?") do |param_type| + return false unless save_type_valid?(param_type) + + # If the current type does not support query, the result of the `&` operation is 0 . + SAVE_TYPES[param_type] & SAVE_TYPES[type] != 0 + end + end + + private + + def save_type_valid?(type) + SAVE_TYPES.key?(type) + end +end diff --git a/app/services/concerns/protected_ref_name_sanitizer.rb b/app/services/concerns/protected_ref_name_sanitizer.rb new file mode 100644 index 00000000000..3966c410fec --- /dev/null +++ b/app/services/concerns/protected_ref_name_sanitizer.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module ProtectedRefNameSanitizer + def sanitize_name(name) + name = CGI.unescapeHTML(name) + name = Sanitize.fragment(name) + + # Sanitize.fragment escapes HTML chars, so unescape again to allow names + # like `feature->master` + CGI.unescapeHTML(name) + end +end diff --git a/app/services/dependency_proxy/find_or_create_manifest_service.rb b/app/services/dependency_proxy/find_cached_manifest_service.rb index aeb62be9f3a..faf0402edaa 100644 --- a/app/services/dependency_proxy/find_or_create_manifest_service.rb +++ b/app/services/dependency_proxy/find_cached_manifest_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module DependencyProxy - class FindOrCreateManifestService < DependencyProxy::BaseService + class FindCachedManifestService < DependencyProxy::BaseService def initialize(group, image, tag, token) @group = group @image = image @@ -20,36 +20,13 @@ module DependencyProxy return respond if cached_manifest_matches?(head_result) - if Feature.enabled?(:dependency_proxy_manifest_workhorse, @group, default_enabled: :yaml) - success(manifest: nil, from_cache: false) - else - pull_new_manifest - respond(from_cache: false) - end + success(manifest: nil, from_cache: false) rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS respond end private - def pull_new_manifest - DependencyProxy::PullManifestService.new(@image, @tag, @token).execute_with_manifest do |new_manifest| - params = { - file_name: @file_name, - content_type: new_manifest[:content_type], - digest: new_manifest[:digest], - file: new_manifest[:file], - size: new_manifest[:file].size - } - - if @manifest - @manifest.update!(params) - else - @manifest = @group.dependency_proxy_manifests.create!(params) - end - end - end - def cached_manifest_matches?(head_result) return false if head_result[:status] == :error diff --git a/app/services/dependency_proxy/pull_manifest_service.rb b/app/services/dependency_proxy/pull_manifest_service.rb deleted file mode 100644 index e8f0ad6374a..00000000000 --- a/app/services/dependency_proxy/pull_manifest_service.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -module DependencyProxy - class PullManifestService < DependencyProxy::BaseService - def initialize(image, tag, token) - @image = image - @tag = tag - @token = token - end - - def execute_with_manifest - raise ArgumentError, 'Block must be provided' unless block_given? - - response = Gitlab::HTTP.get(manifest_url, headers: auth_headers.merge(Accept: ::ContainerRegistry::Client::ACCEPTED_TYPES.join(','))) - - if response.success? - file = Tempfile.new - - begin - file.write(response.body) - file.flush - - yield( - success( - file: file, - digest: response.headers[DependencyProxy::Manifest::DIGEST_HEADER], - content_type: response.headers['content-type'] - ) - ) - ensure - file.close - file.unlink - end - else - yield(error(response.body, response.code)) - end - rescue Timeout::Error => exception - error(exception.message, 599) - end - - private - - def manifest_url - registry.manifest_url(@image, @tag) - end - end -end diff --git a/app/services/deployments/older_deployments_drop_service.rb b/app/services/deployments/older_deployments_drop_service.rb index 504b55b99ac..15384fb0db1 100644 --- a/app/services/deployments/older_deployments_drop_service.rb +++ b/app/services/deployments/older_deployments_drop_service.rb @@ -12,6 +12,8 @@ module Deployments return unless @deployment&.running? older_deployments_builds.each do |build| + next if build.manual? + Gitlab::OptimisticLocking.retry_lock(build, name: 'older_deployments_drop') do |build| build.drop(:forward_deployment_failure) end diff --git a/app/services/events/destroy_service.rb b/app/services/events/destroy_service.rb new file mode 100644 index 00000000000..fdb718f0fcb --- /dev/null +++ b/app/services/events/destroy_service.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Events + class DestroyService + def initialize(project) + @project = project + end + + def execute + project.events.all.delete_all + + ServiceResponse.success(message: 'Events were deleted.') + rescue StandardError + ServiceResponse.error(message: 'Failed to remove events.') + end + + private + + attr_reader :project + end +end diff --git a/app/services/feature_flags/base_service.rb b/app/services/feature_flags/base_service.rb index ca0b6b89199..86dc6188f0a 100644 --- a/app/services/feature_flags/base_service.rb +++ b/app/services/feature_flags/base_service.rb @@ -43,6 +43,7 @@ module FeatureFlags def sync_to_jira(feature_flag) return unless feature_flag.present? + return unless project.jira_subscription_exists? seq_id = ::Atlassian::JiraConnect::Client.generate_update_sequence_id feature_flag.run_after_commit do diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index 9b113be5465..aa471d3a69f 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -157,11 +157,11 @@ module Git end def unsigned_x509_shas(commits) - X509CommitSignature.unsigned_commit_shas(commits.map(&:sha)) + CommitSignatures::X509CommitSignature.unsigned_commit_shas(commits.map(&:sha)) end def unsigned_gpg_shas(commits) - GpgSignature.unsigned_commit_shas(commits.map(&:sha)) + CommitSignatures::GpgSignature.unsigned_commit_shas(commits.map(&:sha)) end def enqueue_update_signatures diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index 5bf39d98fa3..13223872e4f 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -26,7 +26,6 @@ module Git enqueue_detect_repository_languages execute_related_hooks - perform_housekeeping stop_environments unlock_artifacts @@ -71,13 +70,6 @@ module Git BranchHooksService.new(project, current_user, params).execute end - def perform_housekeeping - housekeeping = Repositories::HousekeepingService.new(project) - housekeeping.increment! - housekeeping.execute if housekeeping.needed? - rescue Repositories::HousekeepingService::LeaseTaken - end - def removing_branch? Gitlab::Git.blank_ref?(newrev) end diff --git a/app/services/git/process_ref_changes_service.rb b/app/services/git/process_ref_changes_service.rb index da05f18b5ac..d4081fc149b 100644 --- a/app/services/git/process_ref_changes_service.rb +++ b/app/services/git/process_ref_changes_service.rb @@ -9,6 +9,8 @@ module Git process_changes_by_action(:branch, changes.branch_changes) process_changes_by_action(:tag, changes.tag_changes) + + perform_housekeeping end private @@ -83,5 +85,12 @@ module Git MergeRequests::PushedBranchesService.new(project: project, current_user: current_user, params: { changes: changes }).execute end + + def perform_housekeeping + housekeeping = Repositories::HousekeepingService.new(project) + housekeeping.increment! + housekeeping.execute if housekeeping.needed? + rescue Repositories::HousekeepingService::LeaseTaken + end end end diff --git a/app/services/google_cloud/service_accounts_service.rb b/app/services/google_cloud/service_accounts_service.rb index 29ed69693b0..a512b27493d 100644 --- a/app/services/google_cloud/service_accounts_service.rb +++ b/app/services/google_cloud/service_accounts_service.rb @@ -27,6 +27,24 @@ module GoogleCloud end end + def add_for_project(environment, gcp_project_id, service_account, service_account_key) + project_var_create_or_replace( + environment, + 'GCP_PROJECT_ID', + gcp_project_id + ) + project_var_create_or_replace( + environment, + 'GCP_SERVICE_ACCOUNT', + service_account + ) + project_var_create_or_replace( + environment, + 'GCP_SERVICE_ACCOUNT_KEY', + service_account_key + ) + end + private def group_vars_by_environment @@ -36,5 +54,12 @@ module GoogleCloud grouped[variable.environment_scope][variable.key] = variable.value end end + + def project_var_create_or_replace(environment_scope, key, value) + params = { key: key, filter: { environment_scope: environment_scope } } + existing_variable = ::Ci::VariablesFinder.new(@project, params).execute.first + existing_variable.destroy if existing_variable + @project.variables.create!(key: key, value: value, environment_scope: environment_scope, protected: true) + end end end diff --git a/app/services/gravatar_service.rb b/app/services/gravatar_service.rb index 2a7a5dae291..a689b088854 100644 --- a/app/services/gravatar_service.rb +++ b/app/services/gravatar_service.rb @@ -8,7 +8,7 @@ class GravatarService return unless identifier hash = Digest::MD5.hexdigest(identifier.strip.downcase) - size = 40 unless size && size > 0 + size = Groups::GroupMembersHelper::AVATAR_SIZE unless size && size > 0 sprintf gravatar_url, hash: hash, diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index cd89eb799dc..10ff4961faf 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -29,11 +29,11 @@ module Groups update_group_attributes ensure_ownership update_integrations - update_pending_builds! end post_update_hooks(@updated_project_ids) propagate_integrations + update_pending_builds true end @@ -228,13 +228,15 @@ module Groups end end - def update_pending_builds! - update_params = { + def update_pending_builds + ::Ci::PendingBuilds::UpdateGroupWorker.perform_async(group.id, pending_builds_params) + end + + def pending_builds_params + { namespace_traversal_ids: group.traversal_ids, namespace_id: group.id } - - ::Ci::UpdatePendingBuildService.new(group, update_params).execute end end end diff --git a/app/services/groups/update_shared_runners_service.rb b/app/services/groups/update_shared_runners_service.rb index eb6b46a5613..c09dce0761f 100644 --- a/app/services/groups/update_shared_runners_service.rb +++ b/app/services/groups/update_shared_runners_service.rb @@ -8,7 +8,7 @@ module Groups validate_params update_shared_runners - update_pending_builds! + update_pending_builds_async success @@ -28,12 +28,18 @@ module Groups group.update_shared_runners_setting!(params[:shared_runners_setting]) end - def update_pending_builds! - return unless group.previous_changes.include?('shared_runners_enabled') + def update_pending_builds? + group.previous_changes.include?('shared_runners_enabled') + end + + def update_pending_builds_async + return unless update_pending_builds? - update_params = { instance_runners_enabled: group.shared_runners_enabled } + group.run_after_commit_or_now do |group| + pending_builds_params = { instance_runners_enabled: group.shared_runners_enabled } - ::Ci::UpdatePendingBuildService.new(group, update_params).execute + ::Ci::UpdatePendingBuildService.new(group, pending_builds_params).execute + end end end end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 2aaab88e778..061543b5885 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -10,7 +10,7 @@ module Import def execute(access_params, provider) if blocked_url? - return log_and_return_error("Invalid URL: #{url}", :bad_request) + return log_and_return_error("Invalid URL: #{url}", _("Invalid URL: %{url}") % { url: url }, :bad_request) end unless authorized? @@ -119,6 +119,15 @@ module Import error(_('Import failed due to a GitHub error: %{original}') % { original: exception.response_body }, :unprocessable_entity) end + + def log_and_return_error(message, translated_message, http_status) + Gitlab::GithubImport::Logger.error( + message: 'Error while attempting to import from GitHub', + error: message + ) + + error(translated_message, http_status) + end end end diff --git a/app/services/incident_management/issuable_escalation_statuses/create_service.rb b/app/services/incident_management/issuable_escalation_statuses/create_service.rb new file mode 100644 index 00000000000..e28debf0fa3 --- /dev/null +++ b/app/services/incident_management/issuable_escalation_statuses/create_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module IncidentManagement + module IssuableEscalationStatuses + class CreateService < BaseService + def initialize(issue) + @issue = issue + @alert = issue.alert_management_alert + end + + def execute + escalation_status = ::IncidentManagement::IssuableEscalationStatus.new(issue: issue, **alert_params) + + if escalation_status.save + ServiceResponse.success(payload: { escalation_status: escalation_status }) + else + ServiceResponse.error(message: escalation_status.errors&.full_messages) + end + end + + private + + attr_reader :issue, :alert + + def alert_params + return {} unless alert + + { + status_event: alert.status_event_for(alert.status_name) + } + end + end + end +end + +IncidentManagement::IssuableEscalationStatuses::CreateService.prepend_mod diff --git a/app/services/admin/propagate_integration_service.rb b/app/services/integrations/propagate_service.rb index f7a4bf1a9f9..6d27929d2d0 100644 --- a/app/services/admin/propagate_integration_service.rb +++ b/app/services/integrations/propagate_service.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true -module Admin - class PropagateIntegrationService - include PropagateService +module Integrations + class PropagateService + BATCH_SIZE = 10_000 + + def initialize(integration) + @integration = integration + end def propagate if integration.instance_level? @@ -16,8 +20,21 @@ module Admin end end + def self.propagate(integration) + new(integration).propagate + end + private + attr_reader :integration + + def create_integration_for_projects_without_integration + propagate_integrations( + Project.without_integration(integration), + PropagateIntegrationProjectWorker + ) + end + def update_inherited_integrations propagate_integrations( Integration.by_type(integration.type).inherit_from_id(integration.id), @@ -52,5 +69,12 @@ module Admin PropagateIntegrationProjectWorker ) end + + def propagate_integrations(relation, worker_class) + relation.each_batch(of: BATCH_SIZE) do |records| + min_id, max_id = records.pick("MIN(#{relation.table_name}.id), MAX(#{relation.table_name}.id)") + worker_class.perform_async(integration.id, min_id, max_id) + end + end end end diff --git a/app/services/integrations/propagate_template_service.rb b/app/services/integrations/propagate_template_service.rb new file mode 100644 index 00000000000..85a82ba4c8e --- /dev/null +++ b/app/services/integrations/propagate_template_service.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Integrations + # TODO: Remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/335178 + class PropagateTemplateService + def self.propagate(_integration) + # no-op + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 2daf098b94a..1d1d9b6bec7 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -56,6 +56,8 @@ class IssuableBaseService < ::BaseProjectService # confidential attribute is a special type of metadata and needs to be allowed to be set # by non-members on issues in public projects so that security issues can be reported as confidential. params.delete(:confidential) unless can?(current_user, :set_confidentiality, issuable) + params.delete(:add_contacts) unless can?(current_user, :set_issue_crm_contacts, issuable) + params.delete(:remove_contacts) unless can?(current_user, :set_issue_crm_contacts, issuable) filter_assignees(issuable) filter_milestone @@ -206,6 +208,9 @@ class IssuableBaseService < ::BaseProjectService params[:assignee_ids] = process_assignee_ids(params, extra_assignee_ids: issuable.assignee_ids.to_a) end + params.delete(:remove_contacts) + add_crm_contact_emails = params.delete(:add_contacts) + issuable.assign_attributes(allowed_create_params(params)) before_create(issuable) @@ -219,6 +224,7 @@ class IssuableBaseService < ::BaseProjectService handle_changes(issuable, { params: params }) after_create(issuable) + set_crm_contacts(issuable, add_crm_contact_emails) execute_hooks(issuable) users_to_invalidate = issuable.allows_reviewers? ? issuable.assignees | issuable.reviewers : issuable.assignees @@ -229,6 +235,12 @@ class IssuableBaseService < ::BaseProjectService issuable end + def set_crm_contacts(issuable, add_crm_contact_emails, remove_crm_contact_emails = []) + return unless add_crm_contact_emails.present? || remove_crm_contact_emails.present? + + ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { add_emails: add_crm_contact_emails, remove_emails: remove_crm_contact_emails }).execute(issuable) + end + def before_create(issuable) # To be overridden by subclasses end @@ -254,6 +266,7 @@ class IssuableBaseService < ::BaseProjectService assign_requested_labels(issuable) assign_requested_assignees(issuable) + assign_requested_crm_contacts(issuable) if issuable.changed? || params.present? issuable.assign_attributes(allowed_update_params(params)) @@ -414,6 +427,12 @@ class IssuableBaseService < ::BaseProjectService issuable.touch end + def assign_requested_crm_contacts(issuable) + add_crm_contact_emails = params.delete(:add_contacts) + remove_crm_contact_emails = params.delete(:remove_contacts) + set_crm_contacts(issuable, add_crm_contact_emails, remove_crm_contact_emails) + end + def assign_requested_assignees(issuable) return if issuable.is_a?(Epic) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index efb5de5b17c..577f7dd1e3a 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -30,7 +30,7 @@ module Issues gates = [issue.project, issue.project.group].compact return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) } - IssueRebalancingWorker.perform_async(nil, *issue.project.self_or_root_group_ids) + Issues::RebalancingWorker.perform_async(nil, *issue.project.self_or_root_group_ids) end private diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index fa8d380404b..79b59eee5e1 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -41,7 +41,7 @@ module Issues user = current_user issue.run_after_commit do NewIssueWorker.perform_async(issue.id, user.id) - IssuePlacementWorker.perform_async(nil, issue.project_id) + Issues::PlacementWorker.perform_async(nil, issue.project_id) Namespaces::OnboardingIssueCreatedWorker.perform_async(issue.namespace.id) end end @@ -50,6 +50,7 @@ module Issues def after_create(issue) user_agent_detail_service.create resolve_discussions_with_issue(issue) + create_escalation_status(issue) super end @@ -80,6 +81,10 @@ module Issues attr_reader :spam_params + def create_escalation_status(issue) + ::IncidentManagement::IssuableEscalationStatuses::CreateService.new(issue).execute if issue.supports_escalation? + end + def user_agent_detail_service UserAgentDetailService.new(spammable: @issue, spam_params: spam_params) end diff --git a/app/services/issues/set_crm_contacts_service.rb b/app/services/issues/set_crm_contacts_service.rb index 13fe30b5ac8..c435ab81b4d 100644 --- a/app/services/issues/set_crm_contacts_service.rb +++ b/app/services/issues/set_crm_contacts_service.rb @@ -2,10 +2,9 @@ module Issues class SetCrmContactsService < ::BaseProjectService - attr_accessor :issue, :errors - MAX_ADDITIONAL_CONTACTS = 6 + # Replacing contacts by email is not currently supported def execute(issue) @issue = issue @errors = [] @@ -13,33 +12,49 @@ module Issues return error_no_permissions unless allowed? return error_invalid_params unless valid_params? - determine_changes if params[:crm_contact_ids] - + @existing_ids = issue.customer_relations_contact_ids + determine_changes if params[:replace_ids].present? return error_too_many if too_many? - add_contacts if params[:add_crm_contact_ids] - remove_contacts if params[:remove_crm_contact_ids] + add if params[:add_ids].present? + remove if params[:remove_ids].present? + + add_by_email if params[:add_emails].present? + remove_by_email if params[:remove_emails].present? if issue.valid? + GraphqlTriggers.issue_crm_contacts_updated(issue) + issue.touch ServiceResponse.success(payload: issue) else # The default error isn't very helpful: "Issue customer relations contacts is invalid" issue.errors.delete(:issue_customer_relations_contacts) issue.errors.add(:issue_customer_relations_contacts, errors.to_sentence) - ServiceResponse.error(payload: issue, message: issue.errors.full_messages) + ServiceResponse.error(payload: issue, message: issue.errors.full_messages.to_sentence) end end private + attr_accessor :issue, :errors, :existing_ids + def determine_changes - existing_contact_ids = issue.issue_customer_relations_contacts.map(&:contact_id) - params[:add_crm_contact_ids] = params[:crm_contact_ids] - existing_contact_ids - params[:remove_crm_contact_ids] = existing_contact_ids - params[:crm_contact_ids] + params[:add_ids] = params[:replace_ids] - existing_ids + params[:remove_ids] = existing_ids - params[:replace_ids] + end + + def add + add_by_id(params[:add_ids]) + end + + def add_by_email + contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(project_group.id, params[:add_emails]) + add_by_id(contact_ids) end - def add_contacts - params[:add_crm_contact_ids].uniq.each do |contact_id| + def add_by_id(contact_ids) + contact_ids -= existing_ids + contact_ids.uniq.each do |contact_id| issue_contact = issue.issue_customer_relations_contacts.create(contact_id: contact_id) unless issue_contact.persisted? @@ -49,9 +64,19 @@ module Issues end end - def remove_contacts + def remove + remove_by_id(params[:remove_ids]) + end + + def remove_by_email + contact_ids = ::CustomerRelations::IssueContact.find_contact_ids_by_emails(issue.id, params[:remove_emails]) + remove_by_id(contact_ids) + end + + def remove_by_id(contact_ids) + contact_ids &= existing_ids issue.issue_customer_relations_contacts - .where(contact_id: params[:remove_crm_contact_ids]) # rubocop: disable CodeReuse/ActiveRecord + .where(contact_id: contact_ids) # rubocop: disable CodeReuse/ActiveRecord .delete_all end @@ -64,27 +89,43 @@ module Issues end def set_present? - params[:crm_contact_ids].present? + params[:replace_ids].present? end def add_or_remove_present? - params[:add_crm_contact_ids].present? || params[:remove_crm_contact_ids].present? + add_present? || remove_present? + end + + def add_present? + params[:add_ids].present? || params[:add_emails].present? + end + + def remove_present? + params[:remove_ids].present? || params[:remove_emails].present? end def too_many? - params[:add_crm_contact_ids] && params[:add_crm_contact_ids].length > MAX_ADDITIONAL_CONTACTS + too_many_ids? || too_many_emails? + end + + def too_many_ids? + params[:add_ids] && params[:add_ids].length > MAX_ADDITIONAL_CONTACTS + end + + def too_many_emails? + params[:add_emails] && params[:add_emails].length > MAX_ADDITIONAL_CONTACTS end def error_no_permissions - ServiceResponse.error(message: ['You have insufficient permissions to set customer relations contacts for this issue']) + ServiceResponse.error(message: _('You have insufficient permissions to set customer relations contacts for this issue')) end def error_invalid_params - ServiceResponse.error(message: ['You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids']) + ServiceResponse.error(message: _('You cannot combine replace_ids with add_ids or remove_ids')) end def error_too_many - ServiceResponse.error(payload: issue, message: ["You can only add up to #{MAX_ADDITIONAL_CONTACTS} contacts at one time"]) + ServiceResponse.error(payload: issue, message: _("You can only add up to %{max_contacts} contacts at one time" % { max_contacts: MAX_ADDITIONAL_CONTACTS })) end end end diff --git a/app/services/loose_foreign_keys/batch_cleaner_service.rb b/app/services/loose_foreign_keys/batch_cleaner_service.rb index 06c05e8ff54..de52cbba576 100644 --- a/app/services/loose_foreign_keys/batch_cleaner_service.rb +++ b/app/services/loose_foreign_keys/batch_cleaner_service.rb @@ -2,11 +2,11 @@ module LooseForeignKeys class BatchCleanerService - def initialize(parent_klass:, deleted_parent_records:, modification_tracker: LooseForeignKeys::ModificationTracker.new, models_by_table_name:) - @parent_klass = parent_klass + def initialize(parent_table:, loose_foreign_key_definitions:, deleted_parent_records:, modification_tracker: LooseForeignKeys::ModificationTracker.new) + @parent_table = parent_table + @loose_foreign_key_definitions = loose_foreign_key_definitions @deleted_parent_records = deleted_parent_records @modification_tracker = modification_tracker - @models_by_table_name = models_by_table_name @deleted_records_counter = Gitlab::Metrics.counter( :loose_foreign_key_processed_deleted_records, 'The number of processed loose foreign key deleted records' @@ -14,11 +14,11 @@ module LooseForeignKeys end def execute - parent_klass.loose_foreign_key_definitions.each do |foreign_key_definition| - run_cleaner_service(foreign_key_definition, with_skip_locked: true) + loose_foreign_key_definitions.each do |loose_foreign_key_definition| + run_cleaner_service(loose_foreign_key_definition, with_skip_locked: true) break if modification_tracker.over_limit? - run_cleaner_service(foreign_key_definition, with_skip_locked: false) + run_cleaner_service(loose_foreign_key_definition, with_skip_locked: false) break if modification_tracker.over_limit? end @@ -27,12 +27,12 @@ module LooseForeignKeys # At this point, all associations are cleaned up, we can update the status of the parent records update_count = LooseForeignKeys::DeletedRecord.mark_records_processed(deleted_parent_records) - deleted_records_counter.increment({ table: parent_klass.table_name, db_config_name: LooseForeignKeys::DeletedRecord.connection.pool.db_config.name }, update_count) + deleted_records_counter.increment({ table: parent_table, db_config_name: LooseForeignKeys::DeletedRecord.connection.pool.db_config.name }, update_count) end private - attr_reader :parent_klass, :deleted_parent_records, :modification_tracker, :models_by_table_name, :deleted_records_counter + attr_reader :parent_table, :loose_foreign_key_definitions, :deleted_parent_records, :modification_tracker, :deleted_records_counter def record_result(cleaner, result) if cleaner.async_delete? @@ -42,19 +42,22 @@ module LooseForeignKeys end end - def run_cleaner_service(foreign_key_definition, with_skip_locked:) - cleaner = CleanerService.new( - model: models_by_table_name.fetch(foreign_key_definition.to_table), - foreign_key_definition: foreign_key_definition, - deleted_parent_records: deleted_parent_records, - with_skip_locked: with_skip_locked - ) + def run_cleaner_service(loose_foreign_key_definition, with_skip_locked:) + base_models_for_gitlab_schema = Gitlab::Database.schemas_to_base_models.fetch(loose_foreign_key_definition.options[:gitlab_schema]) + base_models_for_gitlab_schema.each do |base_model| + cleaner = CleanerService.new( + loose_foreign_key_definition: loose_foreign_key_definition, + connection: base_model.connection, + deleted_parent_records: deleted_parent_records, + with_skip_locked: with_skip_locked + ) - loop do - result = cleaner.execute - record_result(cleaner, result) + loop do + result = cleaner.execute + record_result(cleaner, result) - break if modification_tracker.over_limit? || result[:affected_rows] == 0 + break if modification_tracker.over_limit? || result[:affected_rows] == 0 + end end end end diff --git a/app/services/loose_foreign_keys/cleaner_service.rb b/app/services/loose_foreign_keys/cleaner_service.rb index 8fe053e2edf..44a922aad87 100644 --- a/app/services/loose_foreign_keys/cleaner_service.rb +++ b/app/services/loose_foreign_keys/cleaner_service.rb @@ -6,11 +6,9 @@ module LooseForeignKeys DELETE_LIMIT = 1000 UPDATE_LIMIT = 500 - delegate :connection, to: :model - - def initialize(model:, foreign_key_definition:, deleted_parent_records:, with_skip_locked: false) - @model = model - @foreign_key_definition = foreign_key_definition + def initialize(loose_foreign_key_definition:, connection:, deleted_parent_records:, with_skip_locked: false) + @loose_foreign_key_definition = loose_foreign_key_definition + @connection = connection @deleted_parent_records = deleted_parent_records @with_skip_locked = with_skip_locked end @@ -18,20 +16,20 @@ module LooseForeignKeys def execute result = connection.execute(build_query) - { affected_rows: result.cmd_tuples, table: foreign_key_definition.to_table } + { affected_rows: result.cmd_tuples, table: loose_foreign_key_definition.from_table } end def async_delete? - foreign_key_definition.on_delete == :async_delete + loose_foreign_key_definition.on_delete == :async_delete end def async_nullify? - foreign_key_definition.on_delete == :async_nullify + loose_foreign_key_definition.on_delete == :async_nullify end private - attr_reader :model, :foreign_key_definition, :deleted_parent_records, :with_skip_locked + attr_reader :loose_foreign_key_definition, :connection, :deleted_parent_records, :with_skip_locked def build_query query = if async_delete? @@ -39,10 +37,10 @@ module LooseForeignKeys elsif async_nullify? update_query else - raise "Invalid on_delete argument: #{foreign_key_definition.on_delete}" + raise "Invalid on_delete argument: #{loose_foreign_key_definition.on_delete}" end - unless query.include?(%{"#{foreign_key_definition.column}" IN (}) + unless query.include?(%{"#{loose_foreign_key_definition.column}" IN (}) raise("FATAL: foreign key condition is missing from the generated query: #{query}") end @@ -50,15 +48,15 @@ module LooseForeignKeys end def arel_table - @arel_table ||= model.arel_table + @arel_table ||= Arel::Table.new(loose_foreign_key_definition.from_table) end def primary_keys - @primary_keys ||= connection.primary_keys(model.table_name).map { |key| arel_table[key] } + @primary_keys ||= connection.primary_keys(loose_foreign_key_definition.from_table).map { |key| arel_table[key] } end def quoted_table_name - @quoted_table_name ||= Arel.sql(connection.quote_table_name(model.table_name)) + @quoted_table_name ||= Arel.sql(connection.quote_table_name(loose_foreign_key_definition.from_table)) end def delete_query @@ -71,7 +69,7 @@ module LooseForeignKeys def update_query query = Arel::UpdateManager.new query.table(quoted_table_name) - query.set([[arel_table[foreign_key_definition.column], nil]]) + query.set([[arel_table[loose_foreign_key_definition.column], nil]]) add_in_query_with_limit(query, UPDATE_LIMIT) end @@ -88,7 +86,7 @@ module LooseForeignKeys def in_query_with_limit(limit) in_query = Arel::SelectManager.new in_query.from(quoted_table_name) - in_query.where(arel_table[foreign_key_definition.column].in(deleted_parent_records.map(&:primary_key_value))) + in_query.where(arel_table[loose_foreign_key_definition.column].in(deleted_parent_records.map(&:primary_key_value))) in_query.projections = primary_keys in_query.take(limit) in_query.lock(Arel.sql('FOR UPDATE SKIP LOCKED')) if with_skip_locked diff --git a/app/services/loose_foreign_keys/process_deleted_records_service.rb b/app/services/loose_foreign_keys/process_deleted_records_service.rb index 735fc8a2415..025829aa774 100644 --- a/app/services/loose_foreign_keys/process_deleted_records_service.rb +++ b/app/services/loose_foreign_keys/process_deleted_records_service.rb @@ -21,13 +21,16 @@ module LooseForeignKeys break if modification_tracker.over_limit? - model = find_parent_model!(table) + loose_foreign_key_definitions = Gitlab::Database::LooseForeignKeys.definitions_by_table[table] + + next if loose_foreign_key_definitions.empty? LooseForeignKeys::BatchCleanerService - .new(parent_klass: model, - deleted_parent_records: records, - modification_tracker: modification_tracker, - models_by_table_name: models_by_table_name) + .new( + parent_table: table, + loose_foreign_key_definitions: loose_foreign_key_definitions, + deleted_parent_records: records, + modification_tracker: modification_tracker) .execute break if modification_tracker.over_limit? @@ -45,30 +48,12 @@ module LooseForeignKeys LooseForeignKeys::DeletedRecord.load_batch_for_table(fully_qualified_table_name, BATCH_SIZE) end - def find_parent_model!(table) - models_by_table_name.fetch(table) - end - def current_schema @current_schema = connection.current_schema end def tracked_tables - @tracked_tables ||= models_by_table_name - .select { |table_name, model| model.respond_to?(:loose_foreign_key_definitions) } - .keys - end - - def models_by_table_name - @models_by_table_name ||= begin - all_models - .select(&:base_class?) - .index_by(&:table_name) - end - end - - def all_models - ApplicationRecord.descendants + @tracked_tables ||= Gitlab::Database::LooseForeignKeys.definitions_by_table.keys end end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index cb905e01613..acd00d0d1ec 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -92,7 +92,6 @@ module Members super track_invite_source(member) - track_areas_of_focus(member) end def track_invite_source(member) @@ -110,14 +109,7 @@ module Members member.invite? ? 'net_new_user' : 'existing_user' end - def track_areas_of_focus(member) - areas_of_focus.each do |area_of_focus| - Gitlab::Tracking.event(self.class.name, 'area_of_focus', label: area_of_focus, property: member.id.to_s) - end - end - def create_tasks_to_be_done - return unless experiment(:invite_members_for_task).enabled? return if params[:tasks_to_be_done].blank? || params[:tasks_project_id].blank? valid_members = members.select { |member| member.valid? && member.member_task.valid? } @@ -129,10 +121,6 @@ module Members TasksToBeDone::CreateWorker.perform_async(member_task.id, current_user.id, valid_members.map(&:user_id)) end - def areas_of_focus - params[:areas_of_focus] || [] - end - def user_limit limit = params.fetch(:limit, DEFAULT_INVITE_LIMIT) diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index f2c8a6f20a1..e766a7e9044 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -65,7 +65,6 @@ module Members end def create_member_task - return unless experiment(:invite_members_for_task).enabled? return unless member.persisted? return if member_task_attributes.value?(nil) diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index 77564521d45..d2c83f82ff8 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -2,13 +2,22 @@ module MergeRequests class AfterCreateService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize + def execute(merge_request) + prepare_for_mergeability(merge_request) if early_prepare_for_mergeability?(merge_request) prepare_merge_request(merge_request) - merge_request.mark_as_unchecked if merge_request.preparing? + mark_as_unchecked(merge_request) unless early_prepare_for_mergeability?(merge_request) end private + def prepare_for_mergeability(merge_request) + create_pipeline_for(merge_request, current_user) + merge_request.update_head_pipeline + mark_as_unchecked(merge_request) + end + def prepare_merge_request(merge_request) event_service.open_mr(merge_request, current_user) @@ -17,8 +26,10 @@ module MergeRequests notification_service.new_merge_request(merge_request, current_user) - create_pipeline_for(merge_request, current_user) - merge_request.update_head_pipeline + unless early_prepare_for_mergeability?(merge_request) + create_pipeline_for(merge_request, current_user) + merge_request.update_head_pipeline + end merge_request.diffs(include_stats: false).write_cache merge_request.create_cross_references!(current_user) @@ -37,6 +48,16 @@ module MergeRequests def link_lfs_objects(merge_request) LinkLfsObjectsService.new(project: merge_request.target_project).execute(merge_request) end + + def early_prepare_for_mergeability?(merge_request) + strong_memoize("early_prepare_for_mergeability_#{merge_request.target_project_id}".to_sym) do + Feature.enabled?(:early_prepare_for_mergeability, merge_request.target_project) + end + end + + def mark_as_unchecked(merge_request) + merge_request.mark_as_unchecked if merge_request.preparing? + end end end diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 62e599e3e27..3f39b2742c6 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -14,6 +14,7 @@ module MergeRequests create_approval_note(merge_request) mark_pending_todos_as_done(merge_request) execute_approval_hooks(merge_request, current_user) + remove_attention_requested(merge_request, current_user) merge_request_activity_counter.track_approve_mr_action(user: current_user) success diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 0a652c58aab..d744881549a 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -58,6 +58,8 @@ module MergeRequests new_reviewers = merge_request.reviewers - old_reviewers merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) + + remove_attention_requested(merge_request, current_user) end def cleanup_environments(merge_request) @@ -238,6 +240,18 @@ module MergeRequests Milestones::MergeRequestsCountService.new(milestone).delete_cache end + + def remove_all_attention_requests(merge_request) + return unless merge_request.attention_requested_enabled? + + ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request).execute + end + + def remove_attention_requested(merge_request, user) + return unless merge_request.attention_requested_enabled? + + ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: user).execute + end end end diff --git a/app/services/merge_requests/bulk_remove_attention_requested_service.rb b/app/services/merge_requests/bulk_remove_attention_requested_service.rb new file mode 100644 index 00000000000..dd2ff741ba6 --- /dev/null +++ b/app/services/merge_requests/bulk_remove_attention_requested_service.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module MergeRequests + class BulkRemoveAttentionRequestedService < MergeRequests::BaseService + attr_accessor :merge_request + + def initialize(project:, current_user:, merge_request:) + super(project: project, current_user: current_user) + + @merge_request = merge_request + end + + def execute + return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) + + merge_request.merge_request_assignees.update_all(state: :reviewed) + merge_request.merge_request_reviewers.update_all(state: :reviewed) + + success + end + end +end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index f83b14c7269..e9b253129b4 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -17,6 +17,7 @@ module MergeRequests create_note(merge_request) notification_service.async.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) + remove_all_attention_requests(merge_request) execute_hooks(merge_request, 'close') invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) merge_request.update_project_counter_caches diff --git a/app/services/merge_requests/create_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb index 6b032545230..9d7f8393ba5 100644 --- a/app/services/merge_requests/create_pipeline_service.rb +++ b/app/services/merge_requests/create_pipeline_service.rb @@ -48,7 +48,7 @@ module MergeRequests end def can_create_pipeline_in_target_project?(merge_request) - if Gitlab::Ci::Features.disallow_to_create_merge_request_pipelines_in_target_project?(merge_request.target_project) + if ::Feature.enabled?(:ci_disallow_to_create_merge_request_pipelines_in_target_project, merge_request.target_project) merge_request.for_same_project? else can?(current_user, :create_pipeline, merge_request.target_project) && diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb index 87cd6544406..1d9f7ab59f4 100644 --- a/app/services/merge_requests/handle_assignees_change_service.rb +++ b/app/services/merge_requests/handle_assignees_change_service.rb @@ -22,6 +22,8 @@ module MergeRequests merge_request_activity_counter.track_assignees_changed_action(user: current_user) execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] + + remove_attention_requested(merge_request, current_user) end private diff --git a/app/services/merge_requests/outdated_discussion_diff_lines_service.rb b/app/services/merge_requests/outdated_discussion_diff_lines_service.rb index a2de5a32963..a3d94e888df 100644 --- a/app/services/merge_requests/outdated_discussion_diff_lines_service.rb +++ b/app/services/merge_requests/outdated_discussion_diff_lines_service.rb @@ -14,13 +14,23 @@ module MergeRequests end def execute - end_position = position.line_range["end"] - diff_line_index = diff_lines.find_index do |l| - if end_position["new_line"] - l.new_line == end_position["new_line"] - elsif end_position["old_line"] - l.old_line == end_position["old_line"] + line_position = position.line_range["end"] || position.line_range["start"] + found_line = false + diff_line_index = -1 + diff_lines.each_with_index do |l, i| + if found_line + if !l.type + break + elsif l.type == 'new' + diff_line_index = i + break + end + else + # Find the old line + found_line = l.old_line == line_position["new_line"] end + + diff_line_index = i end initial_line_index = [diff_line_index - OVERFLOW_LINES_COUNT, 0].max last_line_index = [diff_line_index + OVERFLOW_LINES_COUNT, diff_lines.length].min diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index ea3071b3c2d..e475b57e4a2 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -28,6 +28,7 @@ module MergeRequests notification_service.merge_mr(merge_request, current_user) invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) merge_request.update_project_counter_caches + remove_all_attention_requests(merge_request) delete_non_latest_diffs(merge_request) cancel_review_app_jobs!(merge_request) cleanup_environments(merge_request) diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index 9423194c01d..d1f45b4b49c 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -4,7 +4,7 @@ module MergeRequests class RebaseService < MergeRequests::BaseService REBASE_ERROR = 'Rebase failed. Please rebase locally' - attr_reader :merge_request + attr_reader :merge_request, :rebase_error def execute(merge_request, skip_ci: false) @merge_request = merge_request @@ -13,7 +13,7 @@ module MergeRequests if rebase success else - error(REBASE_ERROR) + error(rebase_error) end end @@ -22,11 +22,23 @@ module MergeRequests true rescue StandardError => e - log_error(exception: e, message: REBASE_ERROR, save_message_on_model: true) + set_rebase_error(e) + log_error(exception: e, message: rebase_error, save_message_on_model: true) false ensure merge_request.update_column(:rebase_jid, nil) end + + private + + def set_rebase_error(exception) + @rebase_error = + if exception.is_a?(Gitlab::Git::PreReceiveError) + "Something went wrong during the rebase pre-receive hook: #{exception.message}." + else + REBASE_ERROR + end + end end end diff --git a/app/services/merge_requests/remove_attention_requested_service.rb b/app/services/merge_requests/remove_attention_requested_service.rb new file mode 100644 index 00000000000..b727c24415e --- /dev/null +++ b/app/services/merge_requests/remove_attention_requested_service.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module MergeRequests + class RemoveAttentionRequestedService < MergeRequests::BaseService + attr_accessor :merge_request, :user + + def initialize(project:, current_user:, merge_request:, user:) + super(project: project, current_user: current_user) + + @merge_request = merge_request + @user = user + end + + def execute + return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) + + if reviewer || assignee + update_state(reviewer) + update_state(assignee) + + success + else + error("User is not a reviewer or assignee of the merge request") + end + end + + private + + def assignee + merge_request.find_assignee(user) + end + + def reviewer + merge_request.find_reviewer(user) + end + + def update_state(reviewer_or_assignee) + reviewer_or_assignee&.update(state: :reviewed) + end + end +end diff --git a/app/services/merge_requests/resolved_discussion_notification_service.rb b/app/services/merge_requests/resolved_discussion_notification_service.rb index 03ded1512f9..6afd760386e 100644 --- a/app/services/merge_requests/resolved_discussion_notification_service.rb +++ b/app/services/merge_requests/resolved_discussion_notification_service.rb @@ -6,6 +6,7 @@ module MergeRequests return unless merge_request.discussions_resolved? SystemNoteService.resolve_all_discussions(merge_request, project, current_user) + execute_hooks(merge_request, 'update') notification_service.async.resolve_all_discussions(merge_request, current_user) end end diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index 102f78c6a9b..0600fd1d740 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -5,7 +5,7 @@ module MergeRequests def execute # If performing a squash would result in no change, then # immediately return a success message without performing a squash - if merge_request.commits_count < 2 && message.nil? + if merge_request.commits_count == 1 && message == merge_request.first_commit.safe_message return success(squash_sha: merge_request.diff_head_sha) end @@ -17,7 +17,7 @@ module MergeRequests private def squash! - squash_sha = repository.squash(current_user, merge_request, message || merge_request.default_squash_commit_message) + squash_sha = repository.squash(current_user, merge_request, message) success(squash_sha: squash_sha) rescue StandardError => e @@ -39,7 +39,7 @@ module MergeRequests end def message - params[:squash_commit_message].presence + params[:squash_commit_message].presence || merge_request.default_squash_commit_message end end end diff --git a/app/services/merge_requests/toggle_attention_requested_service.rb b/app/services/merge_requests/toggle_attention_requested_service.rb index 66c5d6fce5d..d9f81ac310f 100644 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ b/app/services/merge_requests/toggle_attention_requested_service.rb @@ -19,7 +19,14 @@ module MergeRequests update_state(assignee) if reviewer&.attention_requested? || assignee&.attention_requested? + create_attention_request_note notity_user + + if current_user.id != user.id + remove_attention_requested(merge_request, current_user) + end + else + create_remove_attention_request_note end success @@ -31,9 +38,18 @@ module MergeRequests private def notity_user + notification_service.async.attention_requested_of_merge_request(merge_request, current_user, user) todo_service.create_attention_requested_todo(merge_request, current_user, user) end + def create_attention_request_note + SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) + end + + def create_remove_attention_request_note + SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) + end + def assignee merge_request.find_assignee(user) end diff --git a/app/services/namespaces/invite_team_email_service.rb b/app/services/namespaces/invite_team_email_service.rb index 45975d1953a..78edc205990 100644 --- a/app/services/namespaces/invite_team_email_service.rb +++ b/app/services/namespaces/invite_team_email_service.rb @@ -29,13 +29,12 @@ module Namespaces return if email_for_track_sent_to_user? experiment(:invite_team_email, group: group) do |e| + e.publish_to_database e.candidate do send_email(user, group) sent_email_records.add(user, track, series) sent_email_records.save! end - - e.record! end end diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb index 52070abbad7..aeb859af4d9 100644 --- a/app/services/notification_recipients/build_service.rb +++ b/app/services/notification_recipients/build_service.rb @@ -40,5 +40,9 @@ module NotificationRecipients def self.build_requested_review_recipients(*args) ::NotificationRecipients::Builder::RequestReview.new(*args).notification_recipients end + + def self.build_attention_requested_recipients(*args) + ::NotificationRecipients::Builder::AttentionRequested.new(*args).notification_recipients + end end end diff --git a/app/services/notification_recipients/builder/attention_requested.rb b/app/services/notification_recipients/builder/attention_requested.rb new file mode 100644 index 00000000000..cdc371fcece --- /dev/null +++ b/app/services/notification_recipients/builder/attention_requested.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module NotificationRecipients + module Builder + class AttentionRequested < Base + attr_reader :merge_request, :current_user, :user + + def initialize(merge_request, current_user, user) + @merge_request = merge_request + @current_user = current_user + @user = user + end + + def target + merge_request + end + + def build! + add_recipients(user, :mention, NotificationReason::ATTENTION_REQUESTED) + end + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 6ad3a74b85d..5b1733422d0 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -301,6 +301,14 @@ class NotificationService end end + def attention_requested_of_merge_request(merge_request, current_user, user) + recipients = NotificationRecipients::BuildService.build_attention_requested_recipients(merge_request, current_user, user) + + recipients.each do |recipient| + mailer.attention_requested_merge_request_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later + end + end + # When we add labels to a merge request we should send an email to: # # * watchers of the mr's labels diff --git a/app/services/packages/debian/generate_distribution_service.rb b/app/services/packages/debian/generate_distribution_service.rb index 74b07e05aa6..33bf877a153 100644 --- a/app/services/packages/debian/generate_distribution_service.rb +++ b/app/services/packages/debian/generate_distribution_service.rb @@ -91,7 +91,7 @@ module Packages generate_component_file(component, :packages, architecture, :deb) generate_component_file(component, :di_packages, architecture, :udeb) end - generate_component_file(component, :source, nil, :dsc) + generate_component_file(component, :sources, nil, :dsc) end end diff --git a/app/services/packages/npm/create_package_service.rb b/app/services/packages/npm/create_package_service.rb index ae9c92a3d3a..655616c3a28 100644 --- a/app/services/packages/npm/create_package_service.rb +++ b/app/services/packages/npm/create_package_service.rb @@ -23,9 +23,7 @@ module Packages ::Packages::CreateDependencyService.new(package, package_dependencies).execute ::Packages::Npm::CreateTagService.new(package, dist_tag).execute - if Feature.enabled?(:packages_npm_abbreviated_metadata, project, default_enabled: :yaml) - package.create_npm_metadatum!(package_json: package_json) - end + package.create_npm_metadatum!(package_json: package_json) package end diff --git a/app/services/pages/zip_directory_service.rb b/app/services/pages/zip_directory_service.rb index 895614a84a0..c9029b9666a 100644 --- a/app/services/pages/zip_directory_service.rb +++ b/app/services/pages/zip_directory_service.rb @@ -25,7 +25,9 @@ module Pages FileUtils.rm_f(output_file) entries_count = 0 - ::Zip::File.open(output_file, ::Zip::File::CREATE) do |zipfile| + # Since we're writing not reading here, we can safely silence the cop. + # It currently cannot discern between opening for reading or writing. + ::Zip::File.open(output_file, ::Zip::File::CREATE) do |zipfile| # rubocop:disable Performance/Rubyzip write_entry(zipfile, PUBLIC_DIR) entries_count = zipfile.entries.count end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index b7ed9202b01..aef92b8adee 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -28,9 +28,7 @@ module Projects # Git data (e.g. a list of branch names). flush_caches(project) - if Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml) - ::Ci::AbortPipelinesService.new.execute(project.all_pipelines, :project_deleted) - end + ::Ci::AbortPipelinesService.new.execute(project.all_pipelines, :project_deleted) Projects::UnlinkForkService.new(project, current_user).execute @@ -75,6 +73,18 @@ module Projects response.success? end + def destroy_events! + unless remove_events + raise_error(s_('DeleteProject|Failed to remove events. Please try again or contact administrator.')) + end + end + + def remove_events + response = ::Events::DestroyService.new(project).execute + + response.success? + end + def remove_repository(repository) return true unless repository @@ -117,14 +127,10 @@ module Projects log_destroy_event trash_relation_repositories! trash_project_repositories! + destroy_events! destroy_web_hooks! destroy_project_bots! - - if ::Feature.enabled?(:ci_optimize_project_records_destruction, project, default_enabled: :yaml) && - Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml) - - destroy_ci_records! - end + destroy_ci_records! # Rails attempts to load all related records into memory before # destroying: https://github.com/rails/rails/issues/22510 @@ -150,7 +156,7 @@ module Projects ::Ci::DestroyPipelineService.new(project, current_user).execute(pipeline) end - deleted_count = project.commit_statuses.delete_all + deleted_count = ::CommitStatus.for_project(project).delete_all Gitlab::AppLogger.info( class: 'Projects::DestroyService', diff --git a/app/services/projects/prometheus/alerts/create_service.rb b/app/services/projects/prometheus/alerts/create_service.rb index dc0cacf49f3..0d7d8ab1a62 100644 --- a/app/services/projects/prometheus/alerts/create_service.rb +++ b/app/services/projects/prometheus/alerts/create_service.rb @@ -3,7 +3,7 @@ module Projects module Prometheus module Alerts - class CreateService < BaseService + class CreateService < BaseProjectService include AlertParams def execute diff --git a/app/services/projects/prometheus/alerts/destroy_service.rb b/app/services/projects/prometheus/alerts/destroy_service.rb index 14e88a2e356..243b12eb654 100644 --- a/app/services/projects/prometheus/alerts/destroy_service.rb +++ b/app/services/projects/prometheus/alerts/destroy_service.rb @@ -3,7 +3,7 @@ module Projects module Prometheus module Alerts - class DestroyService < BaseService + class DestroyService < BaseProjectService def execute(alert) alert.destroy end diff --git a/app/services/projects/prometheus/alerts/update_service.rb b/app/services/projects/prometheus/alerts/update_service.rb index a0c8a5ccc2d..1802f35dae9 100644 --- a/app/services/projects/prometheus/alerts/update_service.rb +++ b/app/services/projects/prometheus/alerts/update_service.rb @@ -3,7 +3,7 @@ module Projects module Prometheus module Alerts - class UpdateService < BaseService + class UpdateService < BaseProjectService include AlertParams def execute(alert) diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index a69e6488ebc..17da77fe950 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -104,10 +104,10 @@ module Projects update_repository_configuration(@new_path) execute_system_hooks - - update_pending_builds! end + update_pending_builds + post_update_hooks(project) rescue Exception # rubocop:disable Lint/RescueException rollback_side_effects @@ -244,13 +244,15 @@ module Projects Integration.create_from_active_default_integrations(project, :project_id) end - def update_pending_builds! - update_params = { + def update_pending_builds + ::Ci::PendingBuilds::UpdateProjectWorker.perform_async(project.id, pending_builds_params) + end + + def pending_builds_params + { namespace_id: new_namespace.id, namespace_traversal_ids: new_namespace.traversal_ids } - - ::Ci::UpdatePendingBuildService.new(project, update_params).execute end end end diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index df801311aaf..1ab3ccfcaae 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -2,6 +2,8 @@ module ProtectedBranches class BaseService < ::BaseService + include ProtectedRefNameSanitizer + # current_user - The user that performs the action # params - A hash of parameters def initialize(project, current_user = nil, params = {}) @@ -14,22 +16,13 @@ module ProtectedBranches # overridden in EE::ProtectedBranches module end + private + def filtered_params return unless params - params[:name] = sanitize_branch_name(params[:name]) if params[:name].present? + params[:name] = sanitize_name(params[:name]) if params[:name].present? params end - - private - - def sanitize_branch_name(name) - name = CGI.unescapeHTML(name) - name = Sanitize.fragment(name) - - # Sanitize.fragment escapes HTML chars, so unescape again to allow names - # like `feature->master` - CGI.unescapeHTML(name) - end end end diff --git a/app/services/protected_tags/base_service.rb b/app/services/protected_tags/base_service.rb new file mode 100644 index 00000000000..e0181815f0f --- /dev/null +++ b/app/services/protected_tags/base_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module ProtectedTags + class BaseService < ::BaseService + include ProtectedRefNameSanitizer + + private + + def filtered_params + return unless params + + params[:name] = sanitize_name(params[:name]) if params[:name].present? + params + end + end +end diff --git a/app/services/protected_tags/create_service.rb b/app/services/protected_tags/create_service.rb index 9aff55986b2..7d2b583a295 100644 --- a/app/services/protected_tags/create_service.rb +++ b/app/services/protected_tags/create_service.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true module ProtectedTags - class CreateService < BaseService + class CreateService < ProtectedTags::BaseService attr_reader :protected_tag def execute raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - project.protected_tags.create(params) + project.protected_tags.create(filtered_params) end end end diff --git a/app/services/protected_tags/update_service.rb b/app/services/protected_tags/update_service.rb index 3eb5f4955ee..e337ec39898 100644 --- a/app/services/protected_tags/update_service.rb +++ b/app/services/protected_tags/update_service.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true module ProtectedTags - class UpdateService < BaseService + class UpdateService < ProtectedTags::BaseService def execute(protected_tag) raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - protected_tag.update(params) + protected_tag.update(filtered_params) protected_tag end end diff --git a/app/services/repositories/changelog_service.rb b/app/services/repositories/changelog_service.rb index 96db00fbc1b..eafd9d7a55e 100644 --- a/app/services/repositories/changelog_service.rb +++ b/app/services/repositories/changelog_service.rb @@ -60,7 +60,7 @@ module Repositories end # rubocop: enable Metrics/ParameterLists - def execute + def execute(commit_to_changelog: true) config = Gitlab::Changelog::Config.from_git(@project, @user) from = start_of_commit_range(config) @@ -93,9 +93,13 @@ module Repositories end end - Gitlab::Changelog::Committer - .new(@project, @user) - .commit(release: release, file: @file, branch: @branch, message: @message) + if commit_to_changelog + Gitlab::Changelog::Committer + .new(@project, @user) + .commit(release: release, file: @file, branch: @branch, message: @message) + else + Gitlab::Changelog::Generator.new.add(release) + end end def start_of_commit_range(config) diff --git a/app/services/search_service.rb b/app/services/search_service.rb index 4ba1b3ade86..171d52c328d 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -2,42 +2,35 @@ class SearchService include Gitlab::Allowable + include Gitlab::Utils::StrongMemoize - SEARCH_TERM_LIMIT = 64 - SEARCH_CHAR_LIMIT = 4096 DEFAULT_PER_PAGE = Gitlab::SearchResults::DEFAULT_PER_PAGE MAX_PER_PAGE = 200 def initialize(current_user, params = {}) @current_user = current_user - @params = params.dup + @params = Gitlab::Search::Params.new(params, detect_abuse: prevent_abusive_searches?) end # rubocop: disable CodeReuse/ActiveRecord def project - return @project if defined?(@project) - - @project = - if params[:project_id].present? + strong_memoize(:project) do + if params[:project_id].present? && valid_request? the_project = Project.find_by(id: params[:project_id]) can?(current_user, :read_project, the_project) ? the_project : nil - else - nil end + end end # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord def group - return @group if defined?(@group) - - @group = - if params[:group_id].present? + strong_memoize(:group) do + if params[:group_id].present? && valid_request? the_group = Group.find_by(id: params[:group_id]) can?(current_user, :read_group, the_group) ? the_group : nil - else - nil end + end end # rubocop: enable CodeReuse/ActiveRecord @@ -45,24 +38,23 @@ class SearchService # overridden in EE end + def global_search? + project.blank? && group.blank? + end + def show_snippets? return @show_snippets if defined?(@show_snippets) @show_snippets = params[:snippets] == 'true' end - def valid_query_length? - params[:search].length <= SEARCH_CHAR_LIMIT - end - - def valid_terms_count? - params[:search].split.count { |word| word.length >= 3 } <= SEARCH_TERM_LIMIT - end - delegate :scope, to: :search_service + delegate :valid_terms_count?, :valid_query_length?, to: :params def search_results - @search_results ||= search_service.execute + strong_memoize(:search_results) do + abuse_detected? ? Gitlab::EmptySearchResults.new : search_service.execute + end end def search_objects(preload_method = nil) @@ -79,8 +71,30 @@ class SearchService search_results.aggregations(scope) end + def abuse_detected? + strong_memoize(:abuse_detected) do + params.abusive? + end + end + + def abuse_messages + return [] unless params.abusive? + + params.abuse_detection.errors.full_messages + end + + def valid_request? + strong_memoize(:valid_request) do + params.valid? + end + end + private + def prevent_abusive_searches? + Feature.enabled?(:prevent_abusive_searches, current_user) + end + def page [1, params[:page].to_i].max end diff --git a/app/services/service_ping/devops_report_service.rb b/app/services/service_ping/devops_report_service.rb new file mode 100644 index 00000000000..3b8f5dfdb82 --- /dev/null +++ b/app/services/service_ping/devops_report_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module ServicePing + class DevopsReportService + def initialize(data) + @data = data + end + + def execute + # `conv_index` was previously named `dev_ops_score` in + # version-gitlab-com, so we check both for backwards compatibility. + metrics = @data['conv_index'] || @data['dev_ops_score'] + + # Do not attempt to save a report for the first Service Ping + # response for a given GitLab instance, which comes without + # metrics. + return if metrics.keys == ['usage_data_id'] + + report = DevOpsReport::Metric.create( + metrics.slice(*DevOpsReport::Metric::METRICS) + ) + + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ActiveRecord::RecordInvalid.new(report)) unless report.persisted? + end + end +end diff --git a/app/services/service_ping/submit_service.rb b/app/services/service_ping/submit_service.rb index 63e01603d47..d3d9dcecb2b 100644 --- a/app/services/service_ping/submit_service.rb +++ b/app/services/service_ping/submit_service.rb @@ -6,29 +6,23 @@ module ServicePing STAGING_BASE_URL = 'https://gitlab-services-version-gitlab-com-staging.gs-staging.gitlab.org' USAGE_DATA_PATH = 'usage_data' - METRICS = %w[leader_issues instance_issues percentage_issues leader_notes instance_notes - percentage_notes leader_milestones instance_milestones percentage_milestones - leader_boards instance_boards percentage_boards leader_merge_requests - instance_merge_requests percentage_merge_requests leader_ci_pipelines - instance_ci_pipelines percentage_ci_pipelines leader_environments instance_environments - percentage_environments leader_deployments instance_deployments percentage_deployments - leader_projects_prometheus_active instance_projects_prometheus_active - percentage_projects_prometheus_active leader_service_desk_issues instance_service_desk_issues - percentage_service_desk_issues].freeze - SubmissionError = Class.new(StandardError) + def initialize(skip_db_write: false) + @skip_db_write = skip_db_write + end + def execute return unless ServicePing::ServicePingSettings.product_intelligence_enabled? begin usage_data = BuildPayloadService.new.execute - raw_usage_data, response = submit_usage_data_payload(usage_data) + response = submit_usage_data_payload(usage_data) rescue StandardError return unless Gitlab::CurrentSettings.usage_ping_enabled? usage_data = Gitlab::UsageData.data(force_refresh: true) - raw_usage_data, response = submit_usage_data_payload(usage_data) + response = submit_usage_data_payload(usage_data) end version_usage_data_id = response.dig('conv_index', 'usage_data_id') || response.dig('dev_ops_score', 'usage_data_id') @@ -37,9 +31,11 @@ module ServicePing raise SubmissionError, "Invalid usage_data_id in response: #{version_usage_data_id}" end - raw_usage_data.update_version_metadata!(usage_data_id: version_usage_data_id) - - store_metrics(response) + unless @skip_db_write + raw_usage_data = save_raw_usage_data(usage_data) + raw_usage_data.update_version_metadata!(usage_data_id: version_usage_data_id) + DevopsReportService.new(response).execute + end end def url @@ -60,13 +56,11 @@ module ServicePing def submit_usage_data_payload(usage_data) raise SubmissionError, 'Usage data is blank' if usage_data.blank? - raw_usage_data = save_raw_usage_data(usage_data) - response = submit_payload(usage_data) raise SubmissionError, "Unsuccessful response code: #{response.code}" unless response.success? - [raw_usage_data, response] + response end def save_raw_usage_data(usage_data) @@ -75,16 +69,6 @@ module ServicePing end end - def store_metrics(response) - metrics = response['conv_index'] || response['dev_ops_score'] # leaving dev_ops_score here, as the response data comes from the gitlab-version-com - - return unless metrics.except('usage_data_id').present? - - DevOpsReport::Metric.create!( - metrics.slice(*METRICS) - ) - end - # See https://gitlab.com/gitlab-org/gitlab/-/issues/233615 for details def base_url Rails.env.production? ? PRODUCTION_BASE_URL : STAGING_BASE_URL diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index dc5cf0fe554..0d13c73d49d 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -115,6 +115,14 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).change_status(status, source) end + def request_attention(noteable, project, author, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).request_attention(user) + end + + def remove_attention_request(noteable, project, author, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).remove_attention_request(user) + end + # Called when 'merge when pipeline succeeds' is executed def merge_when_pipeline_succeeds(noteable, project, author, sha) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).merge_when_pipeline_succeeds(sha) @@ -213,12 +221,12 @@ module SystemNoteService ::SystemNotes::MergeRequestsService.new(noteable: issue, project: project, author: author).new_merge_request(merge_request) end - def cross_reference(noteable, mentioner, author) - ::SystemNotes::IssuablesService.new(noteable: noteable, author: author).cross_reference(mentioner) + def cross_reference(mentioned, mentioned_in, author) + ::SystemNotes::IssuablesService.new(noteable: mentioned, author: author).cross_reference(mentioned_in) end - def cross_reference_exists?(noteable, mentioner) - ::SystemNotes::IssuablesService.new(noteable: noteable).cross_reference_exists?(mentioner) + def cross_reference_exists?(mentioned, mentioned_in) + ::SystemNotes::IssuablesService.new(noteable: mentioned).cross_reference_exists?(mentioned_in) end def change_task_status(noteable, project, author, new_task) @@ -249,8 +257,8 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: issuable, project: issuable.project, author: author).discussion_lock end - def cross_reference_disallowed?(noteable, mentioner) - ::SystemNotes::IssuablesService.new(noteable: noteable).cross_reference_disallowed?(mentioner) + def cross_reference_disallowed?(mentioned, mentioned_in) + ::SystemNotes::IssuablesService.new(noteable: mentioned).cross_reference_disallowed?(mentioned_in) end def zoom_link_added(issue, project, author) diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 94629ae7609..d33dcd65589 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -154,9 +154,8 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'description')) end - # Called when a Mentionable references a Noteable - # - # mentioner - Mentionable object + # Called when a Mentionable (the `mentioned_in`) references another Mentionable (the `mentioned`, + # passed to this service as `noteable`). # # Example Note text: # @@ -168,19 +167,20 @@ module SystemNotes # # See cross_reference_note_content. # - # Returns the created Note object - def cross_reference(mentioner) - return if cross_reference_disallowed?(mentioner) + # @param mentioned_in [Mentionable] + # @return [Note] + def cross_reference(mentioned_in) + return if cross_reference_disallowed?(mentioned_in) - gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group) + gfm_reference = mentioned_in.gfm_reference(noteable.project || noteable.group) body = cross_reference_note_content(gfm_reference) if noteable.is_a?(ExternalIssue) Integrations::CreateExternalCrossReferenceWorker.perform_async( noteable.project_id, noteable.id, - mentioner.class.name, - mentioner.id, + mentioned_in.class.name, + mentioned_in.id, author.id ) else @@ -195,15 +195,14 @@ module SystemNotes # in a merge request. Additionally, it prevents the creation of references to # external issues (which would fail). # - # mentioner - Mentionable object - # - # Returns Boolean - def cross_reference_disallowed?(mentioner) + # @param mentioned_in [Mentionable] + # @return [Boolean] + def cross_reference_disallowed?(mentioned_in) return true if noteable.is_a?(ExternalIssue) && !noteable.project&.external_references_supported? - return false unless mentioner.is_a?(MergeRequest) + return false unless mentioned_in.is_a?(MergeRequest) return false unless noteable.is_a?(Commit) - mentioner.commits.include?(noteable) + mentioned_in.commits.include?(noteable) end # Called when the status of a Task has changed @@ -309,38 +308,49 @@ module SystemNotes create_resource_state_event(status: status, mentionable_source: source) end - # Check if a cross reference to a noteable from a mentioner already exists + # Check if a cross reference to a Mentionable from the `mentioned_in` Mentionable + # already exists. # # This method is used to prevent multiple notes being created for a mention - # when a issue is updated, for example. The method also calls notes_for_mentioner - # to check if the mentioner is a commit, and return matches only on commit hash + # when a issue is updated, for example. The method also calls `existing_mentions_for` + # to check if the mention is in a commit, and return matches only on commit hash # instead of project + commit, to avoid repeated mentions from forks. # - # mentioner - Mentionable object - # - # Returns Boolean - def cross_reference_exists?(mentioner) + # @param mentioned_in [Mentionable] + # @return [Boolean] + def cross_reference_exists?(mentioned_in) notes = noteable.notes.system - notes_for_mentioner(mentioner, noteable, notes).exists? + existing_mentions_for(mentioned_in, noteable, notes).exists? end - # Called when a Noteable has been marked as a duplicate of another Issue + # Called when a user's attention has been requested for a Notable # - # canonical_issue - Issue that this is a duplicate of + # user - User's whos attention has been requested # # Example Note text: # - # "marked this issue as a duplicate of #1234" - # - # "marked this issue as a duplicate of other_project#5678" + # "requested attention from @eli.wisoky" # # Returns the created Note object - def mark_duplicate_issue(canonical_issue) - body = "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" + def request_attention(user) + body = "requested attention from #{user.to_reference}" - issue_activity_counter.track_issue_marked_as_duplicate_action(author: author) if noteable.is_a?(Issue) + create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_requested')) + end - create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + # Called when a user's attention request has been removed for a Notable + # + # user - User's whos attention request has been removed + # + # Example Note text: + # + # "removed attention request from @eli.wisoky" + # + # Returns the created Note object + def remove_attention_request(user) + body = "removed attention request from #{user.to_reference}" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_request_removed')) end # Called when a Noteable has been marked as the canonical Issue of a duplicate @@ -359,6 +369,25 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) end + # Called when a Noteable has been marked as a duplicate of another Issue + # + # canonical_issue - Issue that this is a duplicate of + # + # Example Note text: + # + # "marked this issue as a duplicate of #1234" + # + # "marked this issue as a duplicate of other_project#5678" + # + # Returns the created Note object + def mark_duplicate_issue(canonical_issue) + body = "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" + + issue_activity_counter.track_issue_marked_as_duplicate_action(author: author) if noteable.is_a?(Issue) + + create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + end + def add_email_participants(body) create_note(NoteSummary.new(noteable, project, author, body)) end @@ -398,12 +427,12 @@ module SystemNotes "#{self.class.cross_reference_note_prefix}#{gfm_reference}" end - def notes_for_mentioner(mentioner, noteable, notes) - if mentioner.is_a?(Commit) - text = "#{self.class.cross_reference_note_prefix}%#{mentioner.to_reference(nil)}" + def existing_mentions_for(mentioned_in, noteable, notes) + if mentioned_in.is_a?(Commit) + text = "#{self.class.cross_reference_note_prefix}%#{mentioned_in.to_reference(nil)}" notes.like_note_or_capitalized_note(text) else - gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group) + gfm_reference = mentioned_in.gfm_reference(noteable.project || noteable.group) text = cross_reference_note_content(gfm_reference) notes.for_note_or_capitalized_note(text) end diff --git a/app/services/todos/destroy/private_features_service.rb b/app/services/todos/destroy/private_features_service.rb deleted file mode 100644 index 44c3ff231f8..00000000000 --- a/app/services/todos/destroy/private_features_service.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -module Todos - module Destroy - class PrivateFeaturesService < ::Todos::Destroy::BaseService - attr_reader :project_ids, :user_id - - def initialize(project_ids, user_id = nil) - @project_ids = project_ids - @user_id = user_id - end - - # rubocop: disable CodeReuse/ActiveRecord - def execute - ProjectFeature.where(project_id: project_ids).each do |project_features| - target_types = [] - target_types << Issue.name if private?(project_features.issues_access_level) - target_types << MergeRequest.name if private?(project_features.merge_requests_access_level) - target_types << Commit.name if private?(project_features.repository_access_level) - - next if target_types.empty? - - remove_todos(project_features.project_id, target_types) - end - end - # rubocop: enable CodeReuse/ActiveRecord - - private - - def private?(feature_level) - feature_level == ProjectFeature::PRIVATE - end - - # rubocop: disable CodeReuse/ActiveRecord - def remove_todos(project_id, target_types) - items = Todo.where(project_id: project_id) - items = items.where(user_id: user_id) if user_id - - items.where.not(user_id: authorized_users) - .where(target_type: target_types) - .delete_all - end - # rubocop: enable CodeReuse/ActiveRecord - end - end -end diff --git a/app/services/todos/destroy/unauthorized_features_service.rb b/app/services/todos/destroy/unauthorized_features_service.rb new file mode 100644 index 00000000000..513def10575 --- /dev/null +++ b/app/services/todos/destroy/unauthorized_features_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Todos + module Destroy + class UnauthorizedFeaturesService < ::Todos::Destroy::BaseService + attr_reader :project_id, :user_id + + BATCH_SIZE = 1000 + + def initialize(project_id, user_id = nil) + @project_id = project_id + @user_id = user_id + end + + # rubocop: disable CodeReuse/ActiveRecord + def execute + return if user_id && authorized_users.where(user_id: user_id).exists? + + related_todos.each_batch(of: BATCH_SIZE) do |batch| + pending_delete = without_authorized(batch).includes(:target, :user).reject do |todo| + Ability.allowed?(todo.user, :read_todo, todo, scope: :user) + end + Todo.where(id: pending_delete).delete_all if pending_delete.present? + end + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + def related_todos + base_scope = Todo.for_project(project_id) + base_scope = base_scope.for_user(user_id) if user_id + base_scope + end + + # Compatibility for #authorized_users in this class we always work + # with 1 project for queries efficiency + def project_ids + [project_id] + end + end + end +end diff --git a/app/services/users/dismiss_user_callout_service.rb b/app/services/users/dismiss_callout_service.rb index 96f3f3acb57..4324e6232c2 100644 --- a/app/services/users/dismiss_user_callout_service.rb +++ b/app/services/users/dismiss_callout_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Users - class DismissUserCalloutService < BaseContainerService + class DismissCalloutService < BaseContainerService def execute callout.tap do |record| record.update(dismissed_at: Time.current) if record.valid? diff --git a/app/services/users/dismiss_group_callout_service.rb b/app/services/users/dismiss_group_callout_service.rb index 8afee6a8187..f482142b911 100644 --- a/app/services/users/dismiss_group_callout_service.rb +++ b/app/services/users/dismiss_group_callout_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Users - class DismissGroupCalloutService < DismissUserCalloutService + class DismissGroupCalloutService < DismissCalloutService private def callout diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 2d9766c3c56..fe61335f3ed 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -63,12 +63,12 @@ module Users # Updates the list of authorizations for the current user. # # remove - The IDs of the authorization rows to remove. - # add - Rows to insert in the form `[user id, project id, access level]` + # add - Rows to insert in the form `[{ user_id: user_id, project_id: project_id, access_level: access_level}, ...]` def update_authorizations(remove = [], add = []) log_refresh_details(remove, add) - user.remove_project_authorizations(remove) unless remove.empty? - ProjectAuthorization.insert_authorizations(add) unless add.empty? + user.remove_project_authorizations(remove) if remove.any? + ProjectAuthorization.insert_all_in_batches(add) if add.any? # Since we batch insert authorization rows, Rails' associations may get # out of sync. As such we force a reload of the User object. @@ -88,7 +88,7 @@ module Users # most often there's only a few entries in remove and add, but limit it to the first 5 # entries to avoid flooding the logs 'authorized_projects_refresh.rows_deleted_slice': remove.first(5), - 'authorized_projects_refresh.rows_added_slice': add.first(5)) + 'authorized_projects_refresh.rows_added_slice': add.first(5).map(&:values)) end end end diff --git a/app/services/verify_pages_domain_service.rb b/app/services/verify_pages_domain_service.rb index eab1e91dc89..408ee429a74 100644 --- a/app/services/verify_pages_domain_service.rb +++ b/app/services/verify_pages_domain_service.rb @@ -85,7 +85,8 @@ class VerifyPagesDomainService < BaseService end def check(domain_name, resolver) - records = parse(txt_records(domain_name, resolver)) + # Append '.' to domain_name, indicating absolute FQDN + records = parse(txt_records(domain_name + '.', resolver)) records.any? do |record| record == domain.keyed_verification_code || record == domain.verification_code |