diff options
Diffstat (limited to 'app/services')
76 files changed, 929 insertions, 362 deletions
diff --git a/app/services/alert_management/metric_images/upload_service.rb b/app/services/alert_management/metric_images/upload_service.rb new file mode 100644 index 00000000000..e9db10594df --- /dev/null +++ b/app/services/alert_management/metric_images/upload_service.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module AlertManagement + module MetricImages + class UploadService < BaseService + attr_reader :alert, :file, :url, :url_text, :metric + + def initialize(alert, current_user, params = {}) + super + + @alert = alert + @file = params.fetch(:file) + @url = params.fetch(:url, nil) + @url_text = params.fetch(:url_text, nil) + end + + def execute + unless can_upload_metrics? + return ServiceResponse.error( + message: _("You are not authorized to upload metric images"), + http_status: :forbidden + ) + end + + metric = AlertManagement::MetricImage.new( + alert: alert, + file: file, + url: url, + url_text: url_text + ) + + if metric.save + ServiceResponse.success(payload: { metric: metric, alert: alert }) + else + ServiceResponse.error(message: metric.errors.full_messages.join(', '), http_status: :bad_request) + end + end + + private + + def can_upload_metrics? + alert.metric_images_available? && current_user&.can?(:upload_alert_management_metric_image, alert) + end + end + end +end diff --git a/app/services/audit_event_service.rb b/app/services/audit_event_service.rb index ad733c455a9..97debccfb18 100644 --- a/app/services/audit_event_service.rb +++ b/app/services/audit_event_service.rb @@ -14,14 +14,16 @@ class AuditEventService # @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 . + # @params [DateTime] created_at the time the action occured # # @return [AuditEventService] - def initialize(author, entity, details = {}, save_type = :database_and_stream) + def initialize(author, entity, details = {}, save_type = :database_and_stream, created_at = DateTime.current) @author = build_author(author) @entity = entity @details = details @ip_address = resolve_ip_address(@author) @save_type = save_type + @created_at = created_at end # Builds the @details attribute for authentication @@ -79,7 +81,8 @@ class AuditEventService author_id: @author.id, author_name: @author.name, entity_id: @entity.id, - entity_type: @entity.class.name + entity_type: @entity.class.name, + created_at: @created_at } end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index bb6a52eb2f4..6d6d8641d9d 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -50,6 +50,12 @@ module Auth access_token(['pull'], names) end + def self.pull_nested_repositories_access_token(name) + name = name.chomp('/') if name.end_with?('/') + paths = [name, "#{name}/*"] + access_token(['pull'], paths) + end + def self.access_token(actions, names, type = 'repository') names = names.flatten registry = Gitlab.config.registry diff --git a/app/services/base_container_service.rb b/app/services/base_container_service.rb index 190d159e7f1..86df0236a7f 100644 --- a/app/services/base_container_service.rb +++ b/app/services/base_container_service.rb @@ -26,4 +26,8 @@ class BaseContainerService def group_container? container.is_a?(::Group) end + + def namespace_container? + container.is_a?(::Namespace) + end end diff --git a/app/services/bulk_imports/relation_export_service.rb b/app/services/bulk_imports/relation_export_service.rb index 14f073120c5..c43f0d8cb4f 100644 --- a/app/services/bulk_imports/relation_export_service.rb +++ b/app/services/bulk_imports/relation_export_service.rb @@ -4,6 +4,8 @@ module BulkImports class RelationExportService include Gitlab::ImportExport::CommandLineUtil + EXISTING_EXPORT_TTL = 3.minutes + def initialize(user, portable, relation, jid) @user = user @portable = portable @@ -31,6 +33,9 @@ module BulkImports validate_user_permissions! export = portable.bulk_import_exports.safe_find_or_create_by!(relation: relation) + + return export if export.finished? && export.updated_at > EXISTING_EXPORT_TTL.ago + export.update!(status_event: 'start', jid: jid) yield export diff --git a/app/services/ci/after_requeue_job_service.rb b/app/services/ci/after_requeue_job_service.rb index bc70dd3bea4..1ae4639751b 100644 --- a/app/services/ci/after_requeue_job_service.rb +++ b/app/services/ci/after_requeue_job_service.rb @@ -22,15 +22,9 @@ module Ci end def dependent_jobs - dependent_jobs = stage_dependent_jobs - .or(needs_dependent_jobs) - .ordered_by_stage - - if ::Feature.enabled?(:ci_fix_order_of_subsequent_jobs, @processable.pipeline.project, default_enabled: :yaml) - dependent_jobs = ordered_by_dag(dependent_jobs) - end - - dependent_jobs + ordered_by_dag( + stage_dependent_jobs.or(needs_dependent_jobs).ordered_by_stage + ) end def process(job) diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index 034bab93108..0a0c614bb87 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -9,7 +9,7 @@ module Ci DuplicateDownstreamPipelineError = Class.new(StandardError) - MAX_DESCENDANTS_DEPTH = 2 + MAX_NESTED_CHILDREN = 2 def execute(bridge) @bridge = bridge @@ -77,7 +77,8 @@ module Ci # TODO: Remove this condition if favour of model validation # https://gitlab.com/gitlab-org/gitlab/issues/38338 - if has_max_descendants_depth? + # only applies to parent-child pipelines not multi-project + if has_max_nested_children? @bridge.drop!(:reached_max_descendant_pipelines_depth) return false end @@ -129,11 +130,12 @@ module Ci pipeline_checksums.tally.any? { |_checksum, occurrences| occurrences > 2 } end - def has_max_descendants_depth? + def has_max_nested_children? return false unless @bridge.triggers_child_pipeline? + # only applies to parent-child pipelines not multi-project ancestors_of_new_child = @bridge.pipeline.self_and_ancestors - ancestors_of_new_child.count > MAX_DESCENDANTS_DEPTH + ancestors_of_new_child.count > MAX_NESTED_CHILDREN end def config_checksum(pipeline) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index d53e136effb..02f25a82307 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -14,6 +14,7 @@ module Ci Gitlab::Ci::Pipeline::Chain::Build::Associations, Gitlab::Ci::Pipeline::Chain::Validate::Abilities, Gitlab::Ci::Pipeline::Chain::Validate::Repository, + Gitlab::Ci::Pipeline::Chain::Limit::RateLimit, Gitlab::Ci::Pipeline::Chain::Validate::SecurityOrchestrationPolicy, Gitlab::Ci::Pipeline::Chain::Skip, Gitlab::Ci::Pipeline::Chain::Config::Content, 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 c089567ec14..4070875ffe1 100644 --- a/app/services/ci/job_artifacts/destroy_all_expired_service.rb +++ b/app/services/ci/job_artifacts/destroy_all_expired_service.rb @@ -7,16 +7,14 @@ module Ci include ::Gitlab::LoopHelpers BATCH_SIZE = 100 + LOOP_LIMIT = 500 LOOP_TIMEOUT = 5.minutes - SMALL_LOOP_LIMIT = 100 - LARGE_LOOP_LIMIT = 500 - EXCLUSIVE_LOCK_KEY = 'expired_job_artifacts:destroy:lock' LOCK_TIMEOUT = 6.minutes + EXCLUSIVE_LOCK_KEY = 'expired_job_artifacts:destroy:lock' def initialize @removed_artifacts_count = 0 @start_at = Time.current - @loop_limit = ::Feature.enabled?(:ci_artifact_fast_removal_large_loop_limit, default_enabled: :yaml) ? LARGE_LOOP_LIMIT : SMALL_LOOP_LIMIT end ## @@ -26,8 +24,6 @@ module Ci # preventing multiple `ExpireBuildArtifactsWorker` CRON jobs run concurrently, # which is scheduled every 7 minutes. def execute - return 0 unless ::Feature.enabled?(:ci_destroy_all_expired_service, default_enabled: :yaml) - in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do if ::Feature.enabled?(:ci_destroy_unlocked_job_artifacts) destroy_unlocked_job_artifacts @@ -42,7 +38,7 @@ module Ci private def destroy_unlocked_job_artifacts - loop_until(timeout: LOOP_TIMEOUT, limit: @loop_limit) do + loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do 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] @@ -59,7 +55,7 @@ module Ci @removed_artifacts_count += service_response[:destroyed_artifacts_count] break if loop_timeout? - break if index >= @loop_limit + break if index >= LOOP_LIMIT end end diff --git a/app/services/ci/job_artifacts/destroy_batch_service.rb b/app/services/ci/job_artifacts/destroy_batch_service.rb index d5a0a2dd885..90d157373c3 100644 --- a/app/services/ci/job_artifacts/destroy_batch_service.rb +++ b/app/services/ci/job_artifacts/destroy_batch_service.rb @@ -117,7 +117,7 @@ module Ci wrongly_expired_artifacts, @job_artifacts = @job_artifacts.partition { |artifact| wrongly_expired?(artifact) } - remove_expire_at(wrongly_expired_artifacts) + remove_expire_at(wrongly_expired_artifacts) if wrongly_expired_artifacts.any? end def fix_expire_at? @@ -127,7 +127,9 @@ module Ci def wrongly_expired?(artifact) return false unless artifact.expire_at.present? - match_date?(artifact.expire_at) && match_time?(artifact.expire_at) + # Although traces should never have expiration dates that don't match time & date here. + # we can explicitly exclude them by type since they should never be destroyed. + artifact.trace? || (match_date?(artifact.expire_at) && match_time?(artifact.expire_at)) end def match_date?(expire_at) diff --git a/app/services/ci/job_artifacts/update_unknown_locked_status_service.rb b/app/services/ci/job_artifacts/update_unknown_locked_status_service.rb new file mode 100644 index 00000000000..0d35a90ed04 --- /dev/null +++ b/app/services/ci/job_artifacts/update_unknown_locked_status_service.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module Ci + module JobArtifacts + class UpdateUnknownLockedStatusService + include ::Gitlab::ExclusiveLeaseHelpers + include ::Gitlab::LoopHelpers + + BATCH_SIZE = 100 + LOOP_TIMEOUT = 5.minutes + LOOP_LIMIT = 100 + LARGE_LOOP_LIMIT = 500 + EXCLUSIVE_LOCK_KEY = 'unknown_status_job_artifacts:update:lock' + LOCK_TIMEOUT = 6.minutes + + def initialize + @removed_count = 0 + @locked_count = 0 + @start_at = Time.current + @loop_limit = Feature.enabled?(:ci_job_artifacts_backlog_large_loop_limit) ? LARGE_LOOP_LIMIT : LOOP_LIMIT + end + + def execute + in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do + update_locked_status_on_unknown_artifacts + end + + { removed: @removed_count, locked: @locked_count } + end + + private + + def update_locked_status_on_unknown_artifacts + loop_until(timeout: LOOP_TIMEOUT, limit: @loop_limit) do + unknown_status_build_ids = safely_ordered_ci_job_artifacts_locked_unknown_relation.pluck_job_id.uniq + + locked_pipe_build_ids = ::Ci::Build + .with_pipeline_locked_artifacts + .id_in(unknown_status_build_ids) + .pluck_primary_key + + @locked_count += update_unknown_artifacts(locked_pipe_build_ids, Ci::JobArtifact.lockeds[:artifacts_locked]) + + unlocked_pipe_build_ids = unknown_status_build_ids - locked_pipe_build_ids + service_response = batch_destroy_artifacts(unlocked_pipe_build_ids) + @removed_count += service_response[:destroyed_artifacts_count] + end + end + + def update_unknown_artifacts(build_ids, locked_value) + return 0 unless build_ids.any? + + expired_locked_unknown_artifacts.for_job_ids(build_ids).update_all(locked: locked_value) + end + + def batch_destroy_artifacts(build_ids) + deleteable_artifacts_relation = + if build_ids.any? + expired_locked_unknown_artifacts.for_job_ids(build_ids) + else + Ci::JobArtifact.none + end + + Ci::JobArtifacts::DestroyBatchService.new(deleteable_artifacts_relation).execute + end + + def expired_locked_unknown_artifacts + # UPDATE queries perform better without the specific order and limit + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76509#note_891260455 + Ci::JobArtifact.expired_before(@start_at).artifact_unknown + end + + def safely_ordered_ci_job_artifacts_locked_unknown_relation + # Adding the ORDER and LIMIT improves performance when we don't have build_id + expired_locked_unknown_artifacts.limit(BATCH_SIZE).order_expired_asc + end + end + end +end diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb index 2d6b6aeee14..fbf2aad1991 100644 --- a/app/services/ci/play_build_service.rb +++ b/app/services/ci/play_build_service.rb @@ -14,10 +14,7 @@ module Ci AfterRequeueJobService.new(project, current_user).execute(build) end else - # Retrying in Ci::PlayBuildService is a legacy process that should be removed. - # Instead, callers should explicitly execute Ci::RetryBuildService. - # See https://gitlab.com/gitlab-org/gitlab/-/issues/347493. - build.retryable? ? Ci::Build.retry(build, current_user) : build + Ci::RetryJobService.new(project, current_user).execute(build)[:job] end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index c8b475f6c48..6c9044b5089 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -283,7 +283,8 @@ module Ci runner_unsupported: -> (build, params) { !build.supported_runner?(params.dig(:info, :features)) }, archived_failure: -> (build, _) { build.archived? }, project_deleted: -> (build, _) { build.project.pending_delete? }, - builds_disabled: -> (build, _) { !build.project.builds_enabled? } + builds_disabled: -> (build, _) { !build.project.builds_enabled? }, + user_blocked: -> (build, _) { build.user&.blocked? } } end end diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb deleted file mode 100644 index 906e5cec4f3..00000000000 --- a/app/services/ci/retry_build_service.rb +++ /dev/null @@ -1,94 +0,0 @@ -# frozen_string_literal: true - -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 - job_variables_attributes resource_group scheduling_type].freeze - end - - def self.extra_accessors - [] - end - - def execute(build) - build.ensure_scheduling_type! - - clone!(build).tap do |new_build| - check_assignable_runners!(new_build) - next if new_build.failed? - - Gitlab::OptimisticLocking.retry_lock(new_build, name: 'retry_build', &:enqueue) - AfterRequeueJobService.new(project, current_user).execute(build) - end - end - - # rubocop: disable CodeReuse/ActiveRecord - def clone!(build) - # Cloning a build requires a strict type check to ensure - # the attributes being used for the clone are taken straight - # from the model and not overridden by other abstractions. - raise TypeError unless build.instance_of?(Ci::Build) - - check_access!(build) - - new_build = clone_build(build) - - new_build.run_after_commit do - ::Ci::CopyCrossDatabaseAssociationsService.new.execute(build, new_build) - - ::Deployments::CreateForBuildService.new.execute(new_build) - - ::MergeRequests::AddTodoWhenBuildFailsService - .new(project: project) - .close(new_build) - end - - ::Ci::Pipelines::AddJobService.new(build.pipeline).execute!(new_build) do |job| - BulkInsertableAssociations.with_bulk_insert do - job.save! - end - end - - build.reset # refresh the data to get new values of `retried` and `processed`. - - new_build - end - # rubocop: enable CodeReuse/ActiveRecord - - private - - def check_access!(build) - unless can?(current_user, :update_build, build) - raise Gitlab::Access::AccessDeniedError, '403 Forbidden' - end - end - - def check_assignable_runners!(build); end - - def clone_build(build) - project.builds.new(build_attributes(build)) - end - - def build_attributes(build) - attributes = self.class.clone_accessors.to_h do |attribute| - [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend - end - - if build.persisted_environment.present? - attributes[:metadata_attributes] ||= {} - attributes[:metadata_attributes][:expanded_environment_name] = build.expanded_environment_name - end - - attributes[:user] = current_user - attributes - end - end -end - -Ci::RetryBuildService.prepend_mod_with('Ci::RetryBuildService') diff --git a/app/services/ci/retry_job_service.rb b/app/services/ci/retry_job_service.rb new file mode 100644 index 00000000000..af7e7fa16e9 --- /dev/null +++ b/app/services/ci/retry_job_service.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Ci + class RetryJobService < ::BaseService + include Gitlab::Utils::StrongMemoize + + def execute(job) + if job.retryable? + job.ensure_scheduling_type! + new_job = retry_job(job) + + ServiceResponse.success(payload: { job: new_job }) + else + ServiceResponse.error( + message: 'Job cannot be retried', + payload: { job: job, reason: :not_retryable } + ) + end + end + + # rubocop: disable CodeReuse/ActiveRecord + def clone!(job) + # Cloning a job requires a strict type check to ensure + # the attributes being used for the clone are taken straight + # from the model and not overridden by other abstractions. + raise TypeError unless job.instance_of?(Ci::Build) + + check_access!(job) + + new_job = clone_job(job) + + new_job.run_after_commit do + ::Ci::CopyCrossDatabaseAssociationsService.new.execute(job, new_job) + + ::Deployments::CreateForBuildService.new.execute(new_job) + + ::MergeRequests::AddTodoWhenBuildFailsService + .new(project: project) + .close(new_job) + end + + ::Ci::Pipelines::AddJobService.new(job.pipeline).execute!(new_job) do |processable| + BulkInsertableAssociations.with_bulk_insert do + processable.save! + end + end + + job.reset # refresh the data to get new values of `retried` and `processed`. + + new_job + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + def retry_job(job) + clone!(job).tap do |new_job| + check_assignable_runners!(new_job) + next if new_job.failed? + + Gitlab::OptimisticLocking.retry_lock(new_job, name: 'retry_build', &:enqueue) + AfterRequeueJobService.new(project, current_user).execute(job) + end + end + + def check_access!(job) + unless can?(current_user, :update_build, job) + raise Gitlab::Access::AccessDeniedError, '403 Forbidden' + end + end + + def check_assignable_runners!(job); end + + def clone_job(job) + project.builds.new(job_attributes(job)) + end + + def job_attributes(job) + attributes = job.class.clone_accessors.to_h do |attribute| + [attribute, job.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend + end + + if job.persisted_environment.present? + attributes[:metadata_attributes] ||= {} + attributes[:metadata_attributes][:expanded_environment_name] = job.expanded_environment_name + end + + attributes[:user] = current_user + attributes + end + end +end + +Ci::RetryJobService.prepend_mod_with('Ci::RetryJobService') diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index d40643e1513..85f910d05d7 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -13,7 +13,7 @@ module Ci builds_relation(pipeline).find_each do |build| next unless can_be_retried?(build) - Ci::RetryBuildService.new(project, current_user).clone!(build) + Ci::RetryJobService.new(project, current_user).clone!(build) end pipeline.processables.latest.skipped.find_each do |skipped| diff --git a/app/services/concerns/deploy_token_methods.rb b/app/services/concerns/deploy_token_methods.rb index f59a50d6878..578be53f82c 100644 --- a/app/services/concerns/deploy_token_methods.rb +++ b/app/services/concerns/deploy_token_methods.rb @@ -1,16 +1,17 @@ # frozen_string_literal: true module DeployTokenMethods - def create_deploy_token_for(entity, params) + def create_deploy_token_for(entity, current_user, params) params[:deploy_token_type] = DeployToken.deploy_token_types["#{entity.class.name.downcase}_type".to_sym] entity.deploy_tokens.create(params) do |deploy_token| deploy_token.username = params[:username].presence + deploy_token.creator_id = current_user.id end end def destroy_deploy_token(entity, params) - deploy_token = entity.deploy_tokens.find_by_id!(params[:token_id]) + deploy_token = entity.deploy_tokens.find(params[:token_id]) deploy_token.destroy end diff --git a/app/services/concerns/incident_management/usage_data.rb b/app/services/concerns/incident_management/usage_data.rb index b91aa59099d..27e60029ea3 100644 --- a/app/services/concerns/incident_management/usage_data.rb +++ b/app/services/concerns/incident_management/usage_data.rb @@ -9,10 +9,5 @@ module IncidentManagement track_usage_event(:"incident_management_#{action}", current_user.id) end - - # No-op as optionally overridden in implementing classes. - # For use to provide checks before calling #track_incident_action. - def track_event - end end end diff --git a/app/services/concerns/members/bulk_create_users.rb b/app/services/concerns/members/bulk_create_users.rb index 3f8971dde74..e60c84af89e 100644 --- a/app/services/concerns/members/bulk_create_users.rb +++ b/app/services/concerns/members/bulk_create_users.rb @@ -51,12 +51,20 @@ module Members users.concat(User.id_in(user_ids)) if user_ids.present? users.uniq! # de-duplicate just in case as there is no controlling if user records and ids are sent multiple times + users_by_emails = source.users_by_emails(emails) # preloads our request store for all emails + # in case emails belong to a user that is being invited by user or user_id, remove them from + # emails and let users/user_ids handle it. + parsed_emails = emails.select do |email| + user = users_by_emails[email] + !user || (users.exclude?(user) && user_ids.exclude?(user.id)) + end + if users.present? # helps not have to perform another query per user id to see if the member exists later on when fetching - existing_members = source.members_and_requesters.where(user_id: users).index_by(&:user_id) # rubocop:disable CodeReuse/ActiveRecord + existing_members = source.members_and_requesters.with_user(users).index_by(&:user_id) end - [emails, users, existing_members] + [parsed_emails, users, existing_members] end end end diff --git a/app/services/database/consistency_check_service.rb b/app/services/database/consistency_check_service.rb new file mode 100644 index 00000000000..e39bc8f25b8 --- /dev/null +++ b/app/services/database/consistency_check_service.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +module Database + class ConsistencyCheckService + CURSOR_REDIS_KEY_TTL = 7.days + EMPTY_RESULT = { matches: 0, mismatches: 0, batches: 0, mismatches_details: [] }.freeze + + def initialize(source_model:, target_model:, source_columns:, target_columns:) + @source_model = source_model + @target_model = target_model + @source_columns = source_columns + @target_columns = target_columns + @source_sort_column = source_columns.first + @target_sort_column = target_columns.first + end + + # This class takes two ActiveRecord models, and compares the selected columns + # of the two models tables, for the purposes of checking the consistency of + # mirroring of tables. For example Namespace and Ci::NamepaceMirror + # + # It compares up to 25 batches (1000 records / batch), or up to 30 seconds + # for all the batches in total. + # + # It saves the cursor of the next start_id (cusror) in Redis. If the start_id + # wasn't saved in Redis, for example, in the first run, it will choose some random start_id + # + # Example: + # service = Database::ConsistencyCheckService.new( + # source_model: Namespace, + # target_model: Ci::NamespaceMirror, + # source_columns: %w[id traversal_ids], + # target_columns: %w[namespace_id traversal_ids], + # ) + # result = service.execute + # + # result is a hash that has the following fields: + # - batches: Number of batches checked + # - matches: The number of matched records + # - mismatches: The number of mismatched records + # - mismatches_details: It's an array that contains details about the mismatched records. + # each record in this array is a hash of format {id: ID, source_table: [...], target_table: [...]} + # Each record represents the attributes of the records in the two tables. + # - start_id: The start id cursor of the current batch. <nil> means no records. + # - next_start_id: The ID that can be used for the next batch iteration check. <nil> means no records + def execute + start_id = next_start_id + + return EMPTY_RESULT if start_id.nil? + + result = consistency_checker.execute(start_id: start_id) + result[:start_id] = start_id + + save_next_start_id(result[:next_start_id]) + + result + end + + private + + attr_reader :source_model, :target_model, :source_columns, :target_columns, :source_sort_column, :target_sort_column + + def consistency_checker + @consistency_checker ||= Gitlab::Database::ConsistencyChecker.new( + source_model: source_model, + target_model: target_model, + source_columns: source_columns, + target_columns: target_columns + ) + end + + def next_start_id + return if min_id.nil? + + fetch_next_start_id || random_start_id + end + + # rubocop: disable CodeReuse/ActiveRecord + def min_id + @min_id ||= source_model.minimum(source_sort_column) + end + + def max_id + @max_id ||= source_model.minimum(source_sort_column) + end + # rubocop: enable CodeReuse/ActiveRecord + + def fetch_next_start_id + Gitlab::Redis::SharedState.with { |redis| redis.get(cursor_redis_shared_state_key)&.to_i } + end + + # This returns some random start_id, so that we don't always start checking + # from the start of the table, in case we lose the cursor in Redis. + def random_start_id + range_start = min_id + range_end = [min_id, max_id - Gitlab::Database::ConsistencyChecker::BATCH_SIZE].max + rand(range_start..range_end) + end + + def save_next_start_id(start_id) + Gitlab::Redis::SharedState.with do |redis| + redis.set(cursor_redis_shared_state_key, start_id, ex: CURSOR_REDIS_KEY_TTL) + end + end + + def cursor_redis_shared_state_key + "consistency_check_cursor:#{source_model.table_name}:#{target_model.table_name}" + end + end +end diff --git a/app/services/deployments/update_environment_service.rb b/app/services/deployments/update_environment_service.rb index 19b950044d0..b0ba8ecaa47 100644 --- a/app/services/deployments/update_environment_service.rb +++ b/app/services/deployments/update_environment_service.rb @@ -56,7 +56,13 @@ module Deployments end def expanded_environment_url - ExpandVariables.expand(environment_url, -> { variables }) if environment_url + return unless environment_url + + if ::Feature.enabled?(:ci_expand_environment_name_and_url, deployment.project, default_enabled: :yaml) + ExpandVariables.expand(environment_url, -> { variables.sort_and_expand_all }) + else + ExpandVariables.expand(environment_url, -> { variables }) + end end def environment_url diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb index 58fc9799673..6f2b1018a6a 100644 --- a/app/services/emails/base_service.rb +++ b/app/services/emails/base_service.rb @@ -9,6 +9,10 @@ module Emails @params = params.dup @user = params.delete(:user) end + + def notification_service + NotificationService.new + end end end diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb index 011978ba76a..d2d8b69559a 100644 --- a/app/services/emails/create_service.rb +++ b/app/services/emails/create_service.rb @@ -7,6 +7,7 @@ module Emails user.emails.create(params.merge(extra_params)).tap do |email| email&.confirm if skip_confirmation && current_user.admin? + notification_service.new_email_address_added(user, email.email) if email.persisted? && !email.user_primary_email? end end end diff --git a/app/services/environments/stop_service.rb b/app/services/environments/stop_service.rb index d9c66bd13fe..24ae658d3d6 100644 --- a/app/services/environments/stop_service.rb +++ b/app/services/environments/stop_service.rb @@ -7,7 +7,7 @@ module Environments def execute(environment) return unless can?(current_user, :stop_environment, environment) - environment.stop_with_action!(current_user) + environment.stop_with_actions!(current_user) end def execute_for_branch(branch_name) @@ -19,7 +19,9 @@ module Environments end def execute_for_merge_request(merge_request) - merge_request.environments.each { |environment| execute(environment) } + merge_request.environments_in_head_pipeline(deployment_status: :success).each do |environment| + execute(environment) + end end private diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 01a40fc6473..417680e37cf 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -184,6 +184,11 @@ class EventCreateService track_event(event_action: :pushed, event_target: Project, author_id: current_user.id) + namespace = project.namespace + if Feature.enabled?(:route_hll_to_snowplow, namespace, default_enabled: :yaml) + Gitlab::Tracking.event(self.class.to_s, 'action_active_users_project_repo', namespace: namespace, user: current_user, project: project) + end + Users::LastPushEventService.new(current_user) .cache_last_push_event(event) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index d42f718a272..1055f5ff088 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -19,6 +19,8 @@ module Files @file_content = params[:file_content] @file_content = Base64.decode64(@file_content) if params[:file_content_encoding] == 'base64' + + @execute_filemode = params[:execute_filemode] end def file_has_changed?(path, commit_id) diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index f2cd51ef4d0..f9ced112896 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -22,7 +22,8 @@ module Files author_email: @author_email, author_name: @author_name, start_project: @start_project, - start_branch_name: @start_branch) + start_branch_name: @start_branch, + execute_filemode: @execute_filemode) end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 54ab07da680..9fa966bb8a8 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -10,7 +10,8 @@ module Files author_email: @author_email, author_name: @author_name, start_project: @start_project, - start_branch_name: @start_branch) + start_branch_name: @start_branch, + execute_filemode: @execute_filemode) end private diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index 13223872e4f..3c27ad56ebb 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -24,6 +24,7 @@ module Git enqueue_update_mrs enqueue_detect_repository_languages + enqueue_record_project_target_platforms execute_related_hooks @@ -53,6 +54,12 @@ module Git DetectRepositoryLanguagesWorker.perform_async(project.id) end + def enqueue_record_project_target_platforms + return unless default_branch? + + project.enqueue_record_project_target_platforms + end + # Only stop environments if the ref is a branch that is being deleted def stop_environments return unless removing_branch? diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 67cbbaf84f6..639f7c68c40 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -57,11 +57,6 @@ module Groups end def after_create_hook - if group.persisted? && group.root? - delay = Namespaces::InviteTeamEmailService::DELIVERY_DELAY_IN_MINUTES - Namespaces::InviteTeamEmailWorker.perform_in(delay, group.id, current_user.id) - end - track_experiment_event end diff --git a/app/services/groups/deploy_tokens/create_service.rb b/app/services/groups/deploy_tokens/create_service.rb index 4b0541e78a1..e6189df0472 100644 --- a/app/services/groups/deploy_tokens/create_service.rb +++ b/app/services/groups/deploy_tokens/create_service.rb @@ -6,7 +6,7 @@ module Groups include DeployTokenMethods def execute - deploy_token = create_deploy_token_for(@group, params) + deploy_token = create_deploy_token_for(@group, current_user, params) create_deploy_token_payload_for(deploy_token) end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 10ff4961faf..f2e959396bc 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -25,10 +25,15 @@ module Groups private def proceed_to_transfer + old_root_ancestor_id = @group.root_ancestor.id + was_root_group = @group.root? + Group.transaction do update_group_attributes ensure_ownership update_integrations + remove_issue_contacts(old_root_ancestor_id, was_root_group) + update_crm_objects(was_root_group) end post_update_hooks(@updated_project_ids) @@ -53,6 +58,17 @@ module Groups raise_transfer_error(:group_contains_images) if group_projects_contain_registry_images? raise_transfer_error(:cannot_transfer_to_subgroup) if transfer_to_subgroup? raise_transfer_error(:group_contains_npm_packages) if group_with_npm_packages? + raise_transfer_error(:no_permissions_to_migrate_crm) if no_permissions_to_migrate_crm? + end + + def no_permissions_to_migrate_crm? + return false unless group && @new_parent_group + return false if group.root_ancestor == @new_parent_group.root_ancestor + + return true if group.contacts.exists? && !current_user.can?(:admin_crm_contact, @new_parent_group.root_ancestor) + return true if group.organizations.exists? && !current_user.can?(:admin_crm_organization, @new_parent_group.root_ancestor) + + false end def group_with_npm_packages? @@ -202,7 +218,8 @@ module Groups invalid_policies: s_("TransferGroup|You don't have enough permissions."), group_contains_images: s_('TransferGroup|Cannot update the path because there are projects under this group that contain Docker images in their Container Registry. Please remove the images from your projects first and try again.'), cannot_transfer_to_subgroup: s_('TransferGroup|Cannot transfer group to one of its subgroup.'), - group_contains_npm_packages: s_('TransferGroup|Group contains projects with NPM packages.') + group_contains_npm_packages: s_('TransferGroup|Group contains projects with NPM packages.'), + no_permissions_to_migrate_crm: s_("TransferGroup|Group contains contacts/organizations and you don't have enough permissions to move them to the new root group.") }.freeze end @@ -238,6 +255,20 @@ module Groups namespace_id: group.id } end + + def update_crm_objects(was_root_group) + return unless was_root_group + + CustomerRelations::Contact.move_to_root_group(group) + CustomerRelations::Organization.move_to_root_group(group) + end + + def remove_issue_contacts(old_root_ancestor_id, was_root_group) + return if was_root_group + return if old_root_ancestor_id == @group.root_ancestor.id + + CustomerRelations::IssueContact.delete_for_group(@group) + end end end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 061543b5885..a891dcc11e3 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -117,7 +117,7 @@ module Import error: exception.response_body ) - error(_('Import failed due to a GitHub error: %{original}') % { original: exception.response_body }, :unprocessable_entity) + error(_('Import failed due to a GitHub error: %{original} (HTTP %{code})') % { original: exception.response_body, code: exception.response_status }, :unprocessable_entity) end def log_and_return_error(message, translated_message, http_status) diff --git a/app/services/incident_management/issuable_escalation_statuses/build_service.rb b/app/services/incident_management/issuable_escalation_statuses/build_service.rb new file mode 100644 index 00000000000..9ebcf72a0c9 --- /dev/null +++ b/app/services/incident_management/issuable_escalation_statuses/build_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module IncidentManagement + module IssuableEscalationStatuses + class BuildService < ::BaseProjectService + def initialize(issue) + @issue = issue + @alert = issue.alert_management_alert + + super(project: issue.project) + end + + def execute + return issue.escalation_status if issue.escalation_status + + issue.build_incident_management_issuable_escalation_status(alert_params) + 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::BuildService.prepend_mod diff --git a/app/services/incident_management/issuable_escalation_statuses/create_service.rb b/app/services/incident_management/issuable_escalation_statuses/create_service.rb index e28debf0fa3..9b22fb97e0d 100644 --- a/app/services/incident_management/issuable_escalation_statuses/create_service.rb +++ b/app/services/incident_management/issuable_escalation_statuses/create_service.rb @@ -2,14 +2,15 @@ module IncidentManagement module IssuableEscalationStatuses - class CreateService < BaseService + class CreateService < ::BaseProjectService def initialize(issue) @issue = issue - @alert = issue.alert_management_alert + + super(project: issue.project) end def execute - escalation_status = ::IncidentManagement::IssuableEscalationStatus.new(issue: issue, **alert_params) + escalation_status = BuildService.new(issue).execute if escalation_status.save ServiceResponse.success(payload: { escalation_status: escalation_status }) @@ -20,17 +21,7 @@ module IncidentManagement private - attr_reader :issue, :alert - - def alert_params - return {} unless alert - - { - status_event: alert.status_event_for(alert.status_name) - } - end + attr_reader :issue end end end - -IncidentManagement::IssuableEscalationStatuses::CreateService.prepend_mod diff --git a/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb b/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb index 8f591b375ee..1d0504a6e80 100644 --- a/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb +++ b/app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb @@ -31,9 +31,7 @@ module IncidentManagement attr_reader :issuable, :param_errors def available? - issuable.supports_escalation? && - user_has_permissions? && - escalation_status.present? + issuable.supports_escalation? && user_has_permissions? end def user_has_permissions? @@ -42,7 +40,7 @@ module IncidentManagement def escalation_status strong_memoize(:escalation_status) do - issuable.escalation_status + issuable.escalation_status || BuildService.new(issuable).execute end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index a63c54df4a6..03115416607 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -525,6 +525,10 @@ class IssuableBaseService < ::BaseProjectService attrs_changed || labels_changed || assignees_changed || reviewers_changed end + def has_label_changes?(issuable, old_labels) + Set.new(issuable.labels) != Set.new(old_labels) + end + def invalidate_cache_counts(issuable, users: []) users.each do |user| user.public_send("invalidate_#{issuable.noteable_target_type_name}_cache_counts") # rubocop:disable GitlabSecurity/PublicSend @@ -532,6 +536,16 @@ class IssuableBaseService < ::BaseProjectService end # override if needed + def handle_label_changes(issuable, old_labels) + return unless has_label_changes?(issuable, old_labels) + + # reset to preserve the label sort order (title ASC) + issuable.labels.reset + + GraphqlTriggers.issuable_labels_updated(issuable) + end + + # override if needed def handle_changes(issuable, options) end diff --git a/app/services/issuable_links/create_service.rb b/app/services/issuable_links/create_service.rb index 802260c8fae..0887f04760c 100644 --- a/app/services/issuable_links/create_service.rb +++ b/app/services/issuable_links/create_service.rb @@ -2,8 +2,6 @@ module IssuableLinks class CreateService < BaseService - include IncidentManagement::UsageData - attr_reader :issuable, :current_user, :params def initialize(issuable, user, params) @@ -25,7 +23,7 @@ module IssuableLinks end @errors = [] - create_links + references = create_links if @errors.present? return error(@errors.join('. '), 422) @@ -33,7 +31,7 @@ module IssuableLinks track_event - success + success(created_references: references) end # rubocop: disable CodeReuse/ActiveRecord @@ -66,15 +64,19 @@ module IssuableLinks end def link_issuables(target_issuables) - target_issuables.each do |referenced_object| + target_issuables.map do |referenced_object| link = relate_issuables(referenced_object) - unless link.valid? + if link.valid? + after_create_for(link) + else @errors << _("%{ref} cannot be added: %{error}") % { ref: referenced_object.to_reference, error: link.errors.messages.values.flatten.to_sentence } end + + link end end @@ -142,6 +144,18 @@ module IssuableLinks def set_link_type(_link) # no-op end + + # Override on child classes to perform + # actions when the service is executed. + def track_event + # no-op + end + + # Override on child classes to + # perform actions for each object created. + def after_create_for(_link) + # no-op + end end end diff --git a/app/services/issuable_links/destroy_service.rb b/app/services/issuable_links/destroy_service.rb index 19edd008b0a..204cf7ce966 100644 --- a/app/services/issuable_links/destroy_service.rb +++ b/app/services/issuable_links/destroy_service.rb @@ -2,8 +2,6 @@ module IssuableLinks class DestroyService < BaseService - include IncidentManagement::UsageData - attr_reader :link, :current_user, :source, :target def initialize(link, user) @@ -41,5 +39,9 @@ module IssuableLinks def not_found_message 'No Issue Link found' end + + def track_event + # no op + end end end diff --git a/app/services/issue_links/create_service.rb b/app/services/issue_links/create_service.rb index 1c6621ce0a1..7f509f3b3e0 100644 --- a/app/services/issue_links/create_service.rb +++ b/app/services/issue_links/create_service.rb @@ -2,6 +2,8 @@ module IssueLinks class CreateService < IssuableLinks::CreateService + include IncidentManagement::UsageData + def linkable_issuables(issues) @linkable_issuables ||= begin issues.select { |issue| can?(current_user, :admin_issue_link, issue) } diff --git a/app/services/issue_links/destroy_service.rb b/app/services/issue_links/destroy_service.rb index e2422ecaca9..9116e9fb703 100644 --- a/app/services/issue_links/destroy_service.rb +++ b/app/services/issue_links/destroy_service.rb @@ -2,6 +2,8 @@ module IssueLinks class DestroyService < IssuableLinks::DestroyService + include IncidentManagement::UsageData + private def permission_to_remove_relation? diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 88c4ff1a8bb..d9210169005 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -63,6 +63,7 @@ module Issues handle_assignee_changes(issue, old_assignees) handle_confidential_change(issue) + handle_label_changes(issue, old_labels) handle_added_labels(issue, old_labels) handle_milestone_change(issue) handle_added_mentions(issue, old_mentioned_users) diff --git a/app/services/jira/requests/base.rb b/app/services/jira/requests/base.rb index a16f8bbd367..3e15d47e8af 100644 --- a/app/services/jira/requests/base.rb +++ b/app/services/jira/requests/base.rb @@ -68,7 +68,7 @@ module Jira end def auth_docs_link_start - auth_docs_link_url = Rails.application.routes.url_helpers.help_page_path('integration/jira', anchor: 'authentication-in-jira') + auth_docs_link_url = Rails.application.routes.url_helpers.help_page_path('integration/jira/index', anchor: 'authentication-in-jira') '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: auth_docs_link_url } end 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 2826bdb4c3c..54f54d99afb 100644 --- a/app/services/loose_foreign_keys/process_deleted_records_service.rb +++ b/app/services/loose_foreign_keys/process_deleted_records_service.rb @@ -52,7 +52,7 @@ module LooseForeignKeys end def tracked_tables - @tracked_tables ||= Gitlab::Database::LooseForeignKeys.definitions_by_table.keys + @tracked_tables ||= Gitlab::Database::LooseForeignKeys.definitions_by_table.keys.shuffle end end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 758fa2e67f1..8f7b63c32c8 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -14,8 +14,9 @@ module Members super @errors = [] - @invites = invites_from_params&.split(',')&.uniq&.flatten + @invites = invites_from_params @source = params[:source] + @tasks_to_be_done_members = [] end def execute @@ -25,6 +26,7 @@ module Members validate_invitable! add_members + create_tasks_to_be_done enqueue_onboarding_progress_action publish_event! @@ -40,10 +42,13 @@ module Members private - attr_reader :source, :errors, :invites, :member_created_namespace_id, :members + attr_reader :source, :errors, :invites, :member_created_namespace_id, :members, + :tasks_to_be_done_members, :member_created_member_task_id def invites_from_params - params[:user_ids] + return params[:user_ids] if params[:user_ids].is_a?(Array) + + params[:user_ids]&.to_s&.split(',')&.uniq&.flatten || [] end def validate_invite_source! @@ -74,33 +79,45 @@ module Members ) members.each { |member| process_result(member) } - - create_tasks_to_be_done end def process_result(member) - if member.invalid? - add_error_for_member(member) + existing_errors = member.errors.full_messages + + # calling invalid? clears any errors that were added outside of the + # rails validation process + if member.invalid? || existing_errors.present? + add_error_for_member(member, existing_errors) else after_execute(member: member) @member_created_namespace_id ||= member.namespace_id end end - def add_error_for_member(member) + # overridden + def add_error_for_member(member, existing_errors) prefix = "#{member.user.username}: " if member.user.present? - errors << "#{prefix}#{member.errors.full_messages.to_sentence}" + errors << "#{prefix}#{all_member_errors(member, existing_errors).to_sentence}" + end + + def all_member_errors(member, existing_errors) + existing_errors.concat(member.errors.full_messages).uniq end def after_execute(member:) super + build_tasks_to_be_done_members(member) track_invite_source(member) end def track_invite_source(member) - Gitlab::Tracking.event(self.class.name, 'create_member', label: invite_source, property: tracking_property(member), user: current_user) + Gitlab::Tracking.event(self.class.name, + 'create_member', + label: invite_source, + property: tracking_property(member), + user: current_user) end def invite_source @@ -114,16 +131,28 @@ module Members member.invite? ? 'net_new_user' : 'existing_user' end - def create_tasks_to_be_done - return if params[:tasks_to_be_done].blank? || params[:tasks_project_id].blank? - - valid_members = members.select { |member| member.valid? && member.member_task.valid? } - return unless valid_members.present? + def build_tasks_to_be_done_members(member) + return unless tasks_to_be_done?(member) + @tasks_to_be_done_members << member # We can take the first `member_task` here, since all tasks will have the same attributes needed # for the `TasksToBeDone::CreateWorker`, ie. `project` and `tasks_to_be_done`. - member_task = valid_members[0].member_task - TasksToBeDone::CreateWorker.perform_async(member_task.id, current_user.id, valid_members.map(&:user_id)) + @member_created_member_task_id ||= member.member_task.id + end + + def tasks_to_be_done?(member) + return false if params[:tasks_to_be_done].blank? || params[:tasks_project_id].blank? + + # Only create task issues for existing users. Tasks for new users are created when they signup. + member.member_task&.valid? && member.user.present? + end + + def create_tasks_to_be_done + return unless member_created_member_task_id # signal if there is any work to be done here + + TasksToBeDone::CreateWorker.perform_async(member_created_member_task_id, + current_user.id, + tasks_to_be_done_members.map(&:user_id)) end def user_limit diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index fcce32ead94..321658ac9c5 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -4,15 +4,13 @@ module Members # This class serves as more of an app-wide way we add/create members # All roads to add members should take this path. class CreatorService - include Gitlab::Experiment::Dsl - class << self def parsed_access_level(access_level) access_levels.fetch(access_level) { access_level.to_i } end def access_levels - raise NotImplementedError + Gitlab::Access.sym_options_with_owner end end @@ -25,7 +23,7 @@ module Members def execute find_or_build_member - update_member + commit_member create_member_task member @@ -33,23 +31,39 @@ module Members private + delegate :new_record?, to: :member attr_reader :source, :user, :access_level, :member, :args - def update_member - return unless can_update_member? - + def assign_member_attributes member.attributes = member_attributes + end - if member.request? - approve_request + def commit_member + if can_commit_member? + assign_member_attributes + commit_changes else - member.save + add_commit_error end end - def can_update_member? + def can_commit_member? # There is no current user for bulk actions, in which case anything is allowed - !current_user # inheriting classes will add more logic + return true if skip_authorization? + + if new_record? + can_create_new_member? + else + can_update_existing_member? + end + end + + def can_create_new_member? + raise NotImplementedError + end + + def can_update_existing_member? + raise NotImplementedError end # Populates the attributes of a member. @@ -64,6 +78,14 @@ module Members } end + def commit_changes + if member.request? + approve_request + else + member.save + end + end + def create_member_task return unless member.persisted? return if member_task_attributes.value?(nil) @@ -93,6 +115,20 @@ module Members args[:current_user] end + def skip_authorization? + !current_user + end + + def add_commit_error + msg = if new_record? + _('not authorized to create member') + else + _('not authorized to update member') + end + + member.errors.add(:base, msg) + end + def find_or_build_member @user = parse_user_param @@ -101,6 +137,8 @@ module Members else source.members.build(invite_email: user) end + + @member.blocking_refresh = args[:blocking_refresh] end # This method is used to find users that have been entered into the "Add members" field. @@ -114,7 +152,7 @@ module Members User.find_by(id: user) # rubocop:todo CodeReuse/ActiveRecord else # must be an email or at least we'll consider it one - User.find_by_any_email(user) || user + source.users_by_emails([user])[user] || user end end diff --git a/app/services/members/groups/creator_service.rb b/app/services/members/groups/creator_service.rb index df4d3f59d3b..a6f0daa99aa 100644 --- a/app/services/members/groups/creator_service.rb +++ b/app/services/members/groups/creator_service.rb @@ -3,14 +3,14 @@ module Members module Groups class CreatorService < Members::CreatorService - def self.access_levels - Gitlab::Access.sym_options_with_owner - end - private - def can_update_member? - super || current_user.can?(:update_group_member, member) + def can_create_new_member? + current_user.can?(:admin_group_member, member.group) + end + + def can_update_existing_member? + current_user.can?(:update_group_member, member) end end end diff --git a/app/services/members/invite_service.rb b/app/services/members/invite_service.rb index 85acb720f0f..1bf209ab79d 100644 --- a/app/services/members/invite_service.rb +++ b/app/services/members/invite_service.rb @@ -7,6 +7,8 @@ module Members def initialize(*args) super + @invites += parsed_emails + @errors = {} end @@ -14,38 +16,63 @@ module Members alias_method :formatted_errors, :errors - def invites_from_params - params[:email] + def parsed_emails + # can't put this in the initializer since `invites_from_params` is called in super class + # and needs it + @parsed_emails ||= (formatted_param(params[:email]) || []) + end + + def formatted_param(parameter) + parameter&.split(',')&.uniq&.flatten end def validate_invitable! super + return if params[:email].blank? + # we need the below due to add_users hitting Members::CreatorService.parse_users_list and ignoring invalid emails # ideally we wouldn't need this, but we can't really change the add_users method - valid, invalid = invites.partition { |email| Member.valid_email?(email) } - @invites = valid + invalid_emails.each { |email| errors[email] = s_('AddMember|Invite email is invalid') } + end + + def invalid_emails + parsed_emails.each_with_object([]) do |email, invalid| + next if Member.valid_email?(email) - invalid.each { |email| errors[email] = s_('AddMember|Invite email is invalid') } + invalid << email + @invites.delete(email) + end end override :blank_invites_message def blank_invites_message - s_('AddMember|Emails cannot be blank') + s_('AddMember|Invites cannot be blank') end override :add_error_for_member - def add_error_for_member(member) - errors[invite_email(member)] = member.errors.full_messages.to_sentence + def add_error_for_member(member, existing_errors) + errors[invited_object(member)] = all_member_errors(member, existing_errors).to_sentence end - override :create_tasks_to_be_done - def create_tasks_to_be_done - # Only create task issues for existing users. Tasks for new users are created when they signup. - end + def invited_object(member) + return member.invite_email if member.invite_email - def invite_email(member) - member.invite_email || member.user.email + # There is a case where someone was invited by email, but the `user` record exists. + # The member record returned will not have an invite_email attribute defined since + # the CreatorService finds `user` record sometimes by email. + # At that point we lose the info of whether this invite was done by `user` or by email. + # Here we will give preference to check invites by user_id first. + # There is also a case where a user could be invited by their email and + # at the same time via the API in the same request. + # This would would mean the same user is invited as user_id and email. + # However, that isn't as likely from the UI at least since the token generator checks + # for that case and doesn't allow email being used if the user exists as a record already. + if member.user_id.to_s.in?(invites) + member.user.username + else + member.user.all_emails.detect { |email| email.in?(invites) } + end end end end diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb index 4dba81acf73..d92fe60c54a 100644 --- a/app/services/members/projects/creator_service.rb +++ b/app/services/members/projects/creator_service.rb @@ -3,19 +3,28 @@ module Members module Projects class CreatorService < Members::CreatorService - def self.access_levels - Gitlab::Access.sym_options_with_owner - end - private - def can_update_member? - super || current_user.can?(:update_project_member, member) || adding_a_new_owner? + def can_create_new_member? + # order is important here! + # The `admin_project_member` check has side-effects that causes projects not be created if this area is hit + # during project creation. + # Call that triggers is current_user.can?(:admin_project_member, member.project) + # I tracked back to base_policy.rb admin check and specifically in + # Gitlab::Auth::CurrentUserMode.new(@user).admin_mode? call. + # This calls user.admin? and that specific call causes issues with project creation in + # spec/requests/api/projects_spec.rb specs and others, mostly around project creation. + # https://gitlab.com/gitlab-org/gitlab/-/issues/358931 for investigation + adding_the_creator_as_owner_in_a_personal_project? || current_user.can?(:admin_project_member, member.project) + end + + def can_update_existing_member? + current_user.can?(:update_project_member, member) end - def adding_a_new_owner? + def adding_the_creator_as_owner_in_a_personal_project? # this condition is reached during testing setup a lot due to use of `.add_user` - member.owner? && member.new_record? + member.project.personal_namespace_holder?(member.user) end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 2ab623bacf8..d197c13378a 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -72,7 +72,7 @@ module MergeRequests end def cancel_review_app_jobs!(merge_request) - environments = merge_request.environments.in_review_folder.available + environments = merge_request.environments_in_head_pipeline.in_review_folder.available environments.each { |environment| environment.cancel_deployment_jobs! } end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index c5395138902..391079223ca 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -34,6 +34,7 @@ module MergeRequests handle_target_branch_change(merge_request) handle_milestone_change(merge_request) handle_draft_status_change(merge_request, changed_fields) + handle_label_changes(merge_request, old_labels) track_title_and_desc_edits(changed_fields) track_discussion_lock_toggle(merge_request, changed_fields) diff --git a/app/services/namespaces/in_product_marketing_emails_service.rb b/app/services/namespaces/in_product_marketing_emails_service.rb index 90900698e1a..e42c3498c21 100644 --- a/app/services/namespaces/in_product_marketing_emails_service.rb +++ b/app/services/namespaces/in_product_marketing_emails_service.rb @@ -45,6 +45,11 @@ module Namespaces } }.freeze + def self.email_count_for_track(track) + interval_days = TRACKS.dig(track.to_sym, :interval_days) + interval_days&.count || 0 + end + def self.send_for_all_tracks_and_intervals TRACKS.each_key do |track| TRACKS[track][:interval_days].each do |interval| diff --git a/app/services/namespaces/invite_team_email_service.rb b/app/services/namespaces/invite_team_email_service.rb deleted file mode 100644 index 78edc205990..00000000000 --- a/app/services/namespaces/invite_team_email_service.rb +++ /dev/null @@ -1,61 +0,0 @@ -# frozen_string_literal: true - -module Namespaces - class InviteTeamEmailService - include Gitlab::Experiment::Dsl - - TRACK = :invite_team - DELIVERY_DELAY_IN_MINUTES = 20.minutes - - def self.send_email(user, group) - new(user, group).execute - end - - def initialize(user, group) - @group = group - @user = user - @sent_email_records = InProductMarketingEmailRecords.new - end - - def execute - return unless user.email_opted_in? - return unless group.root? - return unless group.setup_for_company - - # Exclude group if users other than the creator have already been - # added/invited - return unless group.member_count == 1 - - 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 - end - end - - private - - attr_reader :user, :group, :sent_email_records - - def send_email(user, group) - NotificationService.new.in_product_marketing(user.id, group.id, track, series) - end - - def track - TRACK - end - - def series - 0 - end - - def email_for_track_sent_to_user? - Users::InProductMarketingEmail.for_user_with_track_and_series(user, track, series).present? - end - end -end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 9a0db3bb9aa..d32d1c8ca12 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -111,7 +111,7 @@ module Notes def track_event(note, user) track_note_creation_usage_for_issues(note) if note.for_issue? track_note_creation_usage_for_merge_requests(note) if note.for_merge_request? - track_usage_event(:incident_management_incident_comment, user.id) if note.for_issue? && note.noteable.incident? + track_incident_action(user, note.noteable, 'incident_comment') if note.for_issue? if Feature.enabled?(:notes_create_service_tracking, project) Gitlab::Tracking.event('Notes::CreateService', 'execute', **tracking_data_for(note)) diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 1cbb5916107..04fc4c7c944 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -27,10 +27,7 @@ module Notes note.assign_attributes(last_edited_at: Time.current, updated_by: current_user) end - note.with_transaction_returning_status do - update_confidentiality(note) - note.save - end + note.save unless only_commands || note.for_personal_snippet? note.create_new_cross_references!(current_user) @@ -88,15 +85,6 @@ module Notes TodoService.new.update_note(note, current_user, old_mentioned_users) end - # This method updates confidentiality of all discussion notes at once - def update_confidentiality(note) - return unless params.key?(:confidential) - return unless note.is_a?(DiscussionNote) # we don't need to do bulk update for single notes - return unless note.start_of_discussion? # don't update all notes if a response is being updated - - Note.id_in(note.discussion.notes.map(&:id)).update_all(confidential: params[:confidential]) - end - def track_note_edit_usage_for_issues(note) Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_edited_action(author: note.author) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index aa7e636b8a4..a3f250bb235 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -109,6 +109,13 @@ class NotificationService mailer.unknown_sign_in_email(user, ip, time).deliver_later end + # Notify a user when a new email address is added to the their account + def new_email_address_added(user, email) + return unless user.can?(:receive_notifications) + + mailer.new_email_address_added_email(user, email).deliver_later + end + # When create an issue we should send an email to: # # * issue assignee if their notification level is not Disabled @@ -201,13 +208,30 @@ class NotificationService new_resource_email(merge_request, current_user, :new_merge_request_email) end + NEW_COMMIT_EMAIL_DISPLAY_LIMIT = 20 def push_to_merge_request(merge_request, current_user, new_commits: [], existing_commits: []) - new_commits = new_commits.map { |c| { short_id: c.short_id, title: c.title } } - existing_commits = existing_commits.map { |c| { short_id: c.short_id, title: c.title } } + total_new_commits_count = new_commits.count + truncated_new_commits = new_commits.first(NEW_COMMIT_EMAIL_DISPLAY_LIMIT).map do |commit| + { short_id: commit.short_id, title: commit.title } + end + + # We don't need the list of all existing commits. We need the first, the + # last, and the total number of existing commits only. + total_existing_commits_count = existing_commits.count + existing_commits = [existing_commits.first, existing_commits.last] if total_existing_commits_count > 2 + existing_commits = existing_commits.map do |commit| + { short_id: commit.short_id, title: commit.title } + end + recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: "push_to") recipients.each do |recipient| - mailer.send(:push_to_merge_request_email, recipient.user.id, merge_request.id, current_user.id, recipient.reason, new_commits: new_commits, existing_commits: existing_commits).deliver_later + mailer.send( + :push_to_merge_request_email, + recipient.user.id, merge_request.id, current_user.id, recipient.reason, + new_commits: truncated_new_commits, total_new_commits_count: total_new_commits_count, + existing_commits: existing_commits, total_existing_commits_count: total_existing_commits_count + ).deliver_later end end diff --git a/app/services/packages/rubygems/metadata_extraction_service.rb b/app/services/packages/rubygems/metadata_extraction_service.rb index b3bac1854d7..872d68e1dbd 100644 --- a/app/services/packages/rubygems/metadata_extraction_service.rb +++ b/app/services/packages/rubygems/metadata_extraction_service.rb @@ -49,7 +49,11 @@ module Packages # rubocop:enable Metrics/CyclomaticComplexity def metadatum - Packages::Rubygems::Metadatum.safe_find_or_create_by!(package: package) + # safe_find_or_create_by! was originally called here. + # We merely switched to `find_or_create_by!` + # rubocop: disable CodeReuse/ActiveRecord + Packages::Rubygems::Metadatum.find_or_create_by!(package: package) + # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/app/services/projects/apple_target_platform_detector_service.rb b/app/services/projects/apple_target_platform_detector_service.rb new file mode 100644 index 00000000000..ec4c16a1416 --- /dev/null +++ b/app/services/projects/apple_target_platform_detector_service.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Projects + # Service class to detect target platforms of a project made for the Apple + # Ecosystem. + # + # This service searches project.pbxproj and *.xcconfig files (contains build + # settings) for the string "SDKROOT = <SDK_name>" where SDK_name can be + # 'iphoneos', 'macosx', 'appletvos' or 'watchos'. Currently, the service is + # intentionally limited (for performance reasons) to detect if a project + # targets iOS. + # + # Ref: https://developer.apple.com/documentation/xcode/build-settings-reference/ + # + # Example usage: + # > AppleTargetPlatformDetectorService.new(a_project).execute + # => [] + # > AppleTargetPlatformDetectorService.new(an_ios_project).execute + # => [:ios] + # > AppleTargetPlatformDetectorService.new(multiplatform_project).execute + # => [:ios, :osx, :tvos, :watchos] + class AppleTargetPlatformDetectorService < BaseService + BUILD_CONFIG_FILENAMES = %w(project.pbxproj *.xcconfig).freeze + + # For the current iteration, we only want to detect when the project targets + # iOS. In the future, we can use the same logic to detect projects that + # target OSX, TvOS, and WatchOS platforms with SDK names 'macosx', 'appletvos', + # and 'watchos', respectively. + PLATFORM_SDK_NAMES = { ios: 'iphoneos' }.freeze + + def execute + detect_platforms + end + + private + + def file_finder + @file_finder ||= ::Gitlab::FileFinder.new(project, project.default_branch) + end + + def detect_platforms + # Return array of SDK names for which "SDKROOT = <sdk_name>" setting + # definition can be found in either project.pbxproj or *.xcconfig files. + PLATFORM_SDK_NAMES.select do |_, sdk| + config_files_containing_sdk_setting(sdk).present? + end.keys + end + + # Return array of project.pbxproj and/or *.xcconfig files + # (Gitlab::Search::FoundBlob) that contain the setting definition string + # "SDKROOT = <sdk_name>" + def config_files_containing_sdk_setting(sdk) + BUILD_CONFIG_FILENAMES.map do |filename| + file_finder.find("SDKROOT = #{sdk} filename:#{filename}") + end.flatten + end + end +end diff --git a/app/services/projects/container_repository/third_party/delete_tags_service.rb b/app/services/projects/container_repository/third_party/delete_tags_service.rb index 4184c676fc3..942df177bea 100644 --- a/app/services/projects/container_repository/third_party/delete_tags_service.rb +++ b/app/services/projects/container_repository/third_party/delete_tags_service.rb @@ -33,7 +33,7 @@ module Projects if deleted_tags.any? && @container_repository.delete_tag_by_digest(deleted_tags.each_value.first) success(deleted: deleted_tags.keys) else - error('could not delete tags') + error("could not delete tags: #{@tag_names.join(', ')}".truncate(1000)) end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 252e1d76bef..3e26c8c35b2 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -105,7 +105,8 @@ module Projects end @project.track_project_repository - @project.create_project_setting unless @project.project_setting + + create_project_settings yield if block_given? @@ -122,6 +123,14 @@ module Projects create_sast_commit if @initialize_with_sast end + def create_project_settings + if Feature.enabled?(:create_project_settings, default_enabled: :yaml) + @project.project_setting.save if @project.project_setting.changed? + else + @project.create_project_setting unless @project.project_setting + end + end + # Add an authorization for the current user authorizations inline # (so they can access the project immediately after this request # completes), and any other affected users in the background @@ -243,7 +252,7 @@ module Projects def import_schedule if @project.errors.empty? - @project.import_state.schedule if @project.import? && !@project.bare_repository_import? + @project.import_state.schedule if @project.import? && !@project.bare_repository_import? && !@project.gitlab_project_migration? else fail(error: @project.errors.full_messages.join(', ')) end diff --git a/app/services/projects/deploy_tokens/create_service.rb b/app/services/projects/deploy_tokens/create_service.rb index 2486544b150..c44a7686c04 100644 --- a/app/services/projects/deploy_tokens/create_service.rb +++ b/app/services/projects/deploy_tokens/create_service.rb @@ -6,7 +6,7 @@ module Projects include DeployTokenMethods def execute - deploy_token = create_deploy_token_for(@project, params) + deploy_token = create_deploy_token_for(@project, current_user, params) create_deploy_token_payload_for(deploy_token) end diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index b91b7f34d42..72492b6f5a5 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -23,6 +23,13 @@ module Projects cleanup end + def exporters + [ + version_saver, avatar_saver, project_tree_saver, uploads_saver, + repo_saver, wiki_repo_saver, lfs_saver, snippets_repo_saver, design_repo_saver + ] + end + protected def extra_attributes_for_measurement @@ -59,30 +66,23 @@ module Projects end def save_export_archive - Gitlab::ImportExport::Saver.save(exportable: project, shared: shared) - end - - def exporters - [ - version_saver, avatar_saver, project_tree_saver, uploads_saver, - repo_saver, wiki_repo_saver, lfs_saver, snippets_repo_saver, design_repo_saver - ] + @export_saver ||= Gitlab::ImportExport::Saver.save(exportable: project, shared: shared) end def version_saver - Gitlab::ImportExport::VersionSaver.new(shared: shared) + @version_saver ||= Gitlab::ImportExport::VersionSaver.new(shared: shared) end def avatar_saver - Gitlab::ImportExport::AvatarSaver.new(project: project, shared: shared) + @avatar_saver ||= Gitlab::ImportExport::AvatarSaver.new(project: project, shared: shared) end def project_tree_saver - tree_saver_class.new(project: project, - current_user: current_user, - shared: shared, - params: params, - logger: logger) + @project_tree_saver ||= tree_saver_class.new(project: project, + current_user: current_user, + shared: shared, + params: params, + logger: logger) end def tree_saver_class @@ -90,27 +90,31 @@ module Projects end def uploads_saver - Gitlab::ImportExport::UploadsSaver.new(project: project, shared: shared) + @uploads_saver ||= Gitlab::ImportExport::UploadsSaver.new(project: project, shared: shared) end def repo_saver - Gitlab::ImportExport::RepoSaver.new(exportable: project, shared: shared) + @repo_saver ||= Gitlab::ImportExport::RepoSaver.new(exportable: project, shared: shared) end def wiki_repo_saver - Gitlab::ImportExport::WikiRepoSaver.new(exportable: project, shared: shared) + @wiki_repo_saver ||= Gitlab::ImportExport::WikiRepoSaver.new(exportable: project, shared: shared) end def lfs_saver - Gitlab::ImportExport::LfsSaver.new(project: project, shared: shared) + @lfs_saver ||= Gitlab::ImportExport::LfsSaver.new(project: project, shared: shared) end def snippets_repo_saver - Gitlab::ImportExport::SnippetsRepoSaver.new(current_user: current_user, project: project, shared: shared) + @snippets_repo_saver ||= Gitlab::ImportExport::SnippetsRepoSaver.new( + current_user: current_user, + project: project, + shared: shared + ) end def design_repo_saver - Gitlab::ImportExport::DesignRepoSaver.new(exportable: project, shared: shared) + @design_repo_saver ||= Gitlab::ImportExport::DesignRepoSaver.new(exportable: project, shared: shared) end def cleanup diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index ef74f3e6e7a..b66435d013b 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -112,8 +112,9 @@ module Projects integration = project.find_or_initialize_integration(::Integrations::Prometheus.to_param) integration.assign_attributes(attrs) + attrs = integration.to_integration_hash.except('created_at', 'updated_at') - { prometheus_integration_attributes: integration.attributes.except(*%w[id project_id created_at updated_at]) } + { prometheus_integration_attributes: attrs } end def incident_management_setting_params diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 152590fffff..c7a34afffb3 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -39,6 +39,7 @@ module Projects GroupMember .active_without_invites_and_requests .with_source_id(visible_groups.self_and_ancestors.pluck_primary_key) + .select(*GroupMember.cached_column_list) end def visible_groups @@ -52,11 +53,12 @@ module Projects end def project_members_through_ancestral_groups - project.group.present? ? project.group.members_with_parents : Member.none + members = project.group.present? ? project.group.members_with_parents : Member.none + members.select(*GroupMember.cached_column_list) end def individual_project_members - project.project_members + project.project_members.select(*GroupMember.cached_column_list) end def project_owner? diff --git a/app/services/projects/record_target_platforms_service.rb b/app/services/projects/record_target_platforms_service.rb new file mode 100644 index 00000000000..224e16f53b3 --- /dev/null +++ b/app/services/projects/record_target_platforms_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Projects + class RecordTargetPlatformsService < BaseService + include Gitlab::Utils::StrongMemoize + + def execute + record_target_platforms + end + + private + + def target_platforms + strong_memoize(:target_platforms) do + AppleTargetPlatformDetectorService.new(project).execute + end + end + + def record_target_platforms + return unless target_platforms.present? + + setting = ::ProjectSetting.find_or_initialize_by(project: project) # rubocop:disable CodeReuse/ActiveRecord + setting.target_platforms = target_platforms + setting.save + + setting.target_platforms + end + end +end diff --git a/app/services/projects/refresh_build_artifacts_size_statistics_service.rb b/app/services/projects/refresh_build_artifacts_size_statistics_service.rb index 794c042ea39..1f86e5f4ba9 100644 --- a/app/services/projects/refresh_build_artifacts_size_statistics_service.rb +++ b/app/services/projects/refresh_build_artifacts_size_statistics_service.rb @@ -12,7 +12,7 @@ module Projects if batch.any? # We are doing the sum in ruby because the query takes too long when done in SQL - total_artifacts_size = batch.sum(&:size) + total_artifacts_size = batch.sum { |artifact| artifact.size.to_i } Projects::BuildArtifactsSizeRefresh.transaction do # Mark the refresh ready for another worker to pick up and process the next batch diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 51c0989ee55..2ad5c303be2 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -121,6 +121,7 @@ module Projects # Overridden in EE def post_update_hooks(project) move_pages(project) + ensure_personal_project_owner_membership(project) end # Overridden in EE @@ -152,6 +153,19 @@ module Projects project.track_project_repository end + def ensure_personal_project_owner_membership(project) + # In case of personal projects, we want to make sure that + # a membership record with `OWNER` access level exists for the owner of the namespace. + return unless project.personal? + + namespace_owner = project.namespace.owner + existing_membership_record = project.member(namespace_owner) + + return if existing_membership_record.present? && existing_membership_record.access_level == Gitlab::Access::OWNER + + project.add_owner(namespace_owner) + end + def refresh_permissions # This ensures we only schedule 1 job for every user that has access to # the namespaces. diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 1baa4ddf0eb..47f4b9c6898 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -77,7 +77,10 @@ module QuickActions # want to also handle bare usernames. The ReferenceExtractor also has # different behaviour, and will return all group members for groups named # using a user-style reference, which is not in scope here. + # + # nb: underscores may be passed in escaped to protect them from markdown rendering args = params.split(/\s|,/).select(&:present?).uniq - ['and'] + args.map! { _1.gsub(/\\_/, '_') } usernames = (args - ['me']).map { _1.delete_prefix('@') } found = User.by_username(usernames).to_a.select { can?(:read_user, _1) } found_names = found.map(&:username).to_set @@ -168,7 +171,7 @@ module QuickActions next unless definition definition.execute(self, arg) - usage_ping_tracking(name, arg) + usage_ping_tracking(definition.name, arg) end end @@ -186,7 +189,7 @@ module QuickActions def usage_ping_tracking(quick_action_name, arg) Gitlab::UsageDataCounters::QuickActionActivityUniqueCounter.track_unique_action( - quick_action_name, + quick_action_name.to_s, args: arg&.strip, user: current_user ) diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index 28ea1ac8296..f7ffe288d57 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -75,7 +75,6 @@ module ResourceAccessTokens end def generate_email - # Default emaildomain need to be reworked. See gitlab-org/gitlab#260305 email_pattern = "#{resource_type}#{resource.id}_bot%s@noreply.#{Gitlab.config.gitlab.host}" uniquify.string(-> (n) { Kernel.sprintf(email_pattern, n) }) do |s| diff --git a/app/services/suggestions/apply_service.rb b/app/services/suggestions/apply_service.rb index a0d26e08341..a20eb6b79c5 100644 --- a/app/services/suggestions/apply_service.rb +++ b/app/services/suggestions/apply_service.rb @@ -54,7 +54,7 @@ module Suggestions author_email: author&.email } - ::Files::MultiService.new(suggestion_set.project, current_user, params) + ::Files::MultiService.new(suggestion_set.source_project, current_user, params) end def commit_message diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 46eec082125..1ea65049dc2 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -64,6 +64,10 @@ module Users # This ensures we delete records in batches. user.destroy_dependent_associations_in_batches(exclude: [:snippets]) + if Feature.enabled?(:nullify_in_batches_on_user_deletion, default_enabled: :yaml) + user.nullify_dependent_associations_in_batches + end + # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing user_data = user.destroy namespace.destroy diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb index 604b83f621f..3eb220c0e40 100644 --- a/app/services/users/migrate_to_ghost_user_service.rb +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -100,9 +100,9 @@ module Users end # rubocop:disable CodeReuse/ActiveRecord - def batched_migrate(base_scope, column) + def batched_migrate(base_scope, column, batch_size: 50) loop do - update_count = base_scope.where(column => user.id).limit(100).update_all(column => ghost_user.id) + update_count = base_scope.where(column => user.id).limit(batch_size).update_all(column => ghost_user.id) break if update_count == 0 end end diff --git a/app/services/users/registrations_build_service.rb b/app/services/users/registrations_build_service.rb index 2d367e7b185..0065b49cc00 100644 --- a/app/services/users/registrations_build_service.rb +++ b/app/services/users/registrations_build_service.rb @@ -16,3 +16,5 @@ module Users end end end + +Users::RegistrationsBuildService.prepend_mod diff --git a/app/services/users/saved_replies/destroy_service.rb b/app/services/users/saved_replies/destroy_service.rb new file mode 100644 index 00000000000..ac08cddad0c --- /dev/null +++ b/app/services/users/saved_replies/destroy_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Users + module SavedReplies + class DestroyService + def initialize(saved_reply:) + @saved_reply = saved_reply + end + + def execute + if saved_reply.destroy + ServiceResponse.success(payload: { saved_reply: saved_reply }) + else + ServiceResponse.error(message: saved_reply.errors.full_messages) + end + end + + private + + attr_reader :saved_reply + end + end +end diff --git a/app/services/users/saved_replies/update_service.rb b/app/services/users/saved_replies/update_service.rb index ab0a3eaf87d..80d3da8a0a3 100644 --- a/app/services/users/saved_replies/update_service.rb +++ b/app/services/users/saved_replies/update_service.rb @@ -3,8 +3,7 @@ module Users module SavedReplies class UpdateService - def initialize(current_user:, saved_reply:, name:, content:) - @current_user = current_user + def initialize(saved_reply:, name:, content:) @saved_reply = saved_reply @name = name @content = content @@ -20,7 +19,7 @@ module Users private - attr_reader :current_user, :saved_reply, :name, :content + attr_reader :saved_reply, :name, :content end end end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index b1d8872aa5e..c0727e52cc3 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -36,7 +36,7 @@ class WebHookService def initialize(hook, data, hook_name, uniqueness_token = nil, force: false) @hook = hook - @data = data + @data = data.to_h @hook_name = hook_name.to_s @uniqueness_token = uniqueness_token @force = force @@ -70,9 +70,6 @@ class WebHookService end log_execution( - trigger: hook_name, - url: hook.url, - request_data: data, response: response, execution_duration: Gitlab::Metrics::System.monotonic_time - start_time ) @@ -86,9 +83,6 @@ class WebHookService Gitlab::Json::LimitedEncoder::LimitExceeded, URI::InvalidURIError => e execution_duration = Gitlab::Metrics::System.monotonic_time - start_time log_execution( - trigger: hook_name, - url: hook.url, - request_data: data, response: InternalErrorResponse.new, execution_duration: execution_duration, error_message: e.to_s @@ -139,14 +133,14 @@ class WebHookService make_request(post_url, basic_auth) end - def log_execution(trigger:, url:, request_data:, response:, execution_duration:, error_message: nil) + def log_execution(response:, execution_duration:, error_message: nil) category = response_category(response) log_data = { - trigger: trigger, - url: url, + trigger: hook_name, + url: hook.url, execution_duration: execution_duration, request_headers: build_headers, - request_data: request_data, + request_data: data, response_headers: format_response_headers(response), response_body: safe_response_body(response), response_status: response.code, |