From 5afcbe03ead9ada87621888a31a62652b10a7e4f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 20 Sep 2023 11:18:08 +0000 Subject: Add latest changes from gitlab-org/gitlab@16-4-stable-ee --- .../admin/abuse_report_labels/create_service.rb | 17 +++ .../admin/abuse_reports/moderate_user_service.rb | 7 + app/services/admin/abuse_reports/update_service.rb | 32 +++++ .../application_settings/update_service.rb | 2 +- .../container_registry_authentication_service.rb | 4 +- app/services/auto_merge/base_service.rb | 21 ++- app/services/boards/update_service.rb | 2 +- .../create_pipeline_trackers_service.rb | 72 ---------- app/services/bulk_imports/create_service.rb | 5 - app/services/bulk_imports/file_download_service.rb | 10 +- app/services/ci/create_commit_status_service.rb | 158 +++++++++++++++++++++ app/services/ci/create_pipeline_service.rb | 7 - .../cancel_redundant_pipelines_service.rb | 6 +- app/services/ci/register_job_service.rb | 22 +-- .../ci/update_instance_variables_service.rb | 2 +- .../clusters/agent_tokens/track_usage_service.rb | 2 +- .../create_or_update_service_account_service.rb | 12 +- app/services/commits/create_service.rb | 7 +- app/services/compare_service.rb | 5 +- .../concerns/alert_management/alert_processing.rb | 2 +- app/services/concerns/rate_limited_service.rb | 8 +- .../concerns/service_desk/custom_emails/logger.rb | 41 ++++++ .../concerns/update_repository_storage_methods.rb | 12 +- .../copy_design_collection/copy_service.rb | 4 +- .../design_management/delete_designs_service.rb | 6 +- .../design_management/runs_design_actions.rb | 10 +- .../design_management/save_designs_service.rb | 8 +- app/services/error_tracking/base_service.rb | 6 +- app/services/event_create_service.rb | 12 +- app/services/feature_flags/base_service.rb | 2 +- app/services/files/multi_service.rb | 2 +- app/services/files/update_service.rb | 22 +-- .../create_cloudsql_instance_service.rb | 38 ++--- .../google_cloud/fetch_google_ip_list_service.rb | 8 +- .../google_cloud/generate_pipeline_service.rb | 8 +- app/services/gpg_keys/destroy_service.rb | 15 ++ app/services/groups/create_service.rb | 12 +- app/services/groups/destroy_service.rb | 7 +- .../groups/ssh_certificates/create_service.rb | 51 +++++++ .../groups/ssh_certificates/destroy_service.rb | 35 +++++ app/services/groups/transfer_service.rb | 18 ++- app/services/groups/update_service.rb | 19 --- app/services/import/bitbucket_server_service.rb | 2 +- app/services/import/fogbugz_service.rb | 2 +- .../import/github/cancel_project_import_service.rb | 6 +- app/services/import/github_service.rb | 2 +- .../file_acquisition_strategies/remote_file.rb | 2 +- .../file_acquisition_strategies/remote_file_s3.rb | 2 +- .../import/validate_remote_git_endpoint_service.rb | 2 +- app/services/import_export_clean_up_service.rb | 2 +- .../pager_duty/create_incident_issue_service.rb | 2 +- .../pager_duty/process_webhook_service.rb | 2 +- app/services/issuable/bulk_update_service.rb | 2 +- app/services/issuable_base_service.rb | 51 ++++--- app/services/issuable_links/create_service.rb | 6 +- app/services/issues/base_service.rb | 5 +- app/services/issues/close_service.rb | 12 +- app/services/issues/create_service.rb | 11 +- app/services/issues/move_service.rb | 20 ++- .../relative_position_rebalancing_service.rb | 2 +- app/services/labels/available_labels_service.rb | 15 +- app/services/labels/create_service.rb | 4 +- app/services/labels/update_service.rb | 6 +- .../process_deleted_records_service.rb | 6 +- app/services/members/creator_service.rb | 51 +++---- app/services/members/destroy_service.rb | 4 + app/services/merge_requests/approval_service.rb | 18 ++- app/services/merge_requests/base_service.rb | 18 +-- app/services/merge_requests/build_service.rb | 8 +- app/services/merge_requests/create_ref_service.rb | 94 ++++++------ app/services/merge_requests/ff_merge_service.rb | 31 ---- app/services/merge_requests/merge_base_service.rb | 36 ----- app/services/merge_requests/merge_service.rb | 97 ++----------- .../merge_strategies/from_source_branch.rb | 4 +- .../merge_requests/merge_to_ref_service.rb | 8 +- app/services/merge_requests/refresh_service.rb | 4 +- app/services/merge_requests/update_service.rb | 2 +- .../metrics/global_metrics_update_service.rb | 24 ---- app/services/metrics/sample_metrics_service.rb | 36 ----- .../in_product_marketing_emails_service.rb | 102 ------------- app/services/notes/create_service.rb | 4 +- app/services/notes/post_process_service.rb | 2 - .../notification_recipients/builder/base.rb | 1 + app/services/notification_service.rb | 20 ++- .../debian/generate_distribution_service.rb | 4 +- .../packages/npm/generate_metadata_service.rb | 52 +++++-- .../packages/nuget/check_duplicates_service.rb | 88 ++++++++++++ .../nuget/extract_metadata_file_service.rb | 15 +- .../nuget/extract_remote_metadata_file_service.rb | 82 +++++++++++ .../packages/nuget/metadata_extraction_service.rb | 18 +-- .../packages/nuget/odata_package_entry_service.rb | 70 +++++++++ .../nuget/update_package_from_metadata_service.rb | 2 +- app/services/preview_markdown_service.rb | 2 +- .../apple_target_platform_detector_service.rb | 2 +- .../cleanup_tags_base_service.rb | 2 +- .../gitlab/delete_tags_service.rb | 2 +- app/services/projects/create_service.rb | 28 ++-- app/services/projects/destroy_service.rb | 6 +- app/services/projects/download_service.rb | 2 +- .../hashed_storage/migrate_attachments_service.rb | 2 +- app/services/projects/import_error_filter.rb | 2 +- ...in_product_marketing_campaign_emails_service.rb | 6 +- .../lfs_object_download_list_service.rb | 4 +- app/services/projects/transfer_service.rb | 42 +++--- app/services/projects/update_pages_service.rb | 79 ++++++----- .../projects/update_repository_storage_service.rb | 4 +- app/services/projects/update_service.rb | 13 +- app/services/releases/destroy_service.rb | 10 ++ app/services/repositories/base_service.rb | 2 +- .../repository_archive_clean_up_service.rb | 4 +- .../resource_access_tokens/create_service.rb | 2 +- .../resource_access_tokens/revoke_service.rb | 2 +- .../resource_events/base_change_timebox_service.rb | 2 +- .../resource_events/change_labels_service.rb | 2 +- .../resource_events/change_state_service.rb | 2 +- app/services/search/global_service.rb | 2 +- app/services/search/project_service.rb | 2 +- .../custom_email_verifications/base_service.rb | 8 ++ .../custom_email_verifications/create_service.rb | 5 +- .../custom_email_verifications/update_service.rb | 7 +- .../service_desk/custom_emails/base_service.rb | 3 + .../service_desk/custom_emails/create_service.rb | 1 + .../service_desk/custom_emails/destroy_service.rb | 1 + .../service_desk_settings/update_service.rb | 7 + app/services/service_response.rb | 22 +-- app/services/snippets/update_service.rb | 2 +- app/services/spam/akismet_service.rb | 6 +- app/services/spam/spam_action_service.rb | 22 +-- app/services/spam/spam_verdict_service.rb | 21 +-- app/services/submodules/update_service.rb | 12 +- app/services/suggestions/create_service.rb | 26 ++-- .../system_notes/alert_management_service.rb | 4 +- app/services/system_notes/issuables_service.rb | 13 +- .../todos/destroy/destroyed_issuable_service.rb | 2 +- app/services/todos/destroy/entity_leave_service.rb | 2 +- app/services/users/activity_service.rb | 4 +- app/services/users/authorized_build_service.rb | 2 +- app/services/users/build_service.rb | 4 +- app/services/users/destroy_service.rb | 11 +- ...ate_records_to_ghost_user_in_batches_service.rb | 8 +- .../users/migrate_records_to_ghost_user_service.rb | 2 +- .../users/refresh_authorized_projects_service.rb | 20 +-- .../users/upsert_credit_card_validation_service.rb | 8 +- app/services/webauthn/authenticate_service.rb | 11 +- app/services/work_items/callbacks/award_emoji.rb | 3 +- app/services/work_items/create_service.rb | 14 +- .../related_work_item_links/create_service.rb | 2 +- .../related_work_item_links/destroy_service.rb | 85 +++++++++++ 148 files changed, 1424 insertions(+), 938 deletions(-) create mode 100644 app/services/admin/abuse_report_labels/create_service.rb create mode 100644 app/services/admin/abuse_reports/update_service.rb delete mode 100644 app/services/bulk_imports/create_pipeline_trackers_service.rb create mode 100644 app/services/ci/create_commit_status_service.rb create mode 100644 app/services/concerns/service_desk/custom_emails/logger.rb create mode 100644 app/services/groups/ssh_certificates/create_service.rb create mode 100644 app/services/groups/ssh_certificates/destroy_service.rb delete mode 100644 app/services/merge_requests/ff_merge_service.rb delete mode 100644 app/services/metrics/global_metrics_update_service.rb delete mode 100644 app/services/metrics/sample_metrics_service.rb create mode 100644 app/services/packages/nuget/check_duplicates_service.rb create mode 100644 app/services/packages/nuget/extract_remote_metadata_file_service.rb create mode 100644 app/services/packages/nuget/odata_package_entry_service.rb create mode 100644 app/services/work_items/related_work_item_links/destroy_service.rb (limited to 'app/services') diff --git a/app/services/admin/abuse_report_labels/create_service.rb b/app/services/admin/abuse_report_labels/create_service.rb new file mode 100644 index 00000000000..937890a9f51 --- /dev/null +++ b/app/services/admin/abuse_report_labels/create_service.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Admin + module AbuseReportLabels + class CreateService < Labels::BaseService + def initialize(params = {}) + @params = params + end + + def execute + params[:color] = convert_color_name_to_hex if params[:color].present? + + ::Admin::AbuseReportLabel.create(params) + end + end + end +end diff --git a/app/services/admin/abuse_reports/moderate_user_service.rb b/app/services/admin/abuse_reports/moderate_user_service.rb index da61a4dc8f6..823568d9db8 100644 --- a/app/services/admin/abuse_reports/moderate_user_service.rb +++ b/app/services/admin/abuse_reports/moderate_user_service.rb @@ -61,10 +61,17 @@ module Admin def close_report return error('Report already closed') if abuse_report.closed? + close_similar_open_reports abuse_report.closed! success end + def close_similar_open_reports + # admins see the abuse report and other open reports for the same user in one page + # hence, if the request is to close the report, close other open reports for the same user too + abuse_report.similar_open_reports_for_user.update_all(status: 'closed') + end + def close_report_and_record_event event = action diff --git a/app/services/admin/abuse_reports/update_service.rb b/app/services/admin/abuse_reports/update_service.rb new file mode 100644 index 00000000000..36992e1aa25 --- /dev/null +++ b/app/services/admin/abuse_reports/update_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Admin + module AbuseReports + class UpdateService < BaseService + attr_reader :abuse_report, :params, :current_user + + def initialize(abuse_report, current_user, params) + @abuse_report = abuse_report + @current_user = current_user + @params = params + end + + def execute + return ServiceResponse.error(message: 'Admin is required') unless current_user&.can_admin_all_resources? + + abuse_report.label_ids = label_ids + + ServiceResponse.success + end + + private + + def label_ids + params[:label_ids].filter_map do |id| + GitlabSchema.parse_gid(id, expected_type: ::Admin::AbuseReportLabel).model_id + rescue Gitlab::Graphql::Errors::ArgumentError + end + end + end + end +end diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index 6d484c4fa22..a46ecc3eee6 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -6,7 +6,7 @@ module ApplicationSettings attr_reader :params, :application_setting - MARKDOWN_CACHE_INVALIDATING_PARAMS = %w(asset_proxy_enabled asset_proxy_url asset_proxy_secret_key asset_proxy_whitelist).freeze + MARKDOWN_CACHE_INVALIDATING_PARAMS = %w[asset_proxy_enabled asset_proxy_url asset_proxy_secret_key asset_proxy_whitelist].freeze def execute result = update_settings diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index eaee5ce70fc..9b010272995 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -39,11 +39,11 @@ module Auth end def self.full_access_token(*names) - access_token(%w(*), names) + access_token(%w[*], names) end def self.import_access_token - access_token(%w(*), ['import'], 'registry') + access_token(%w[*], ['import'], 'registry') end def self.pull_access_token(*names) diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index 1660ddb934f..77ed0369624 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -20,6 +20,10 @@ module AutoMerge :failed end + def process(_) + raise NotImplementedError + end + def update(merge_request) assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy)) @@ -87,16 +91,21 @@ module AutoMerge merge_request.auto_merge_enabled = false merge_request.merge_user = nil - merge_request.merge_params&.except!( - 'should_remove_source_branch', - 'commit_message', - 'squash_commit_message', - 'auto_merge_strategy' - ) + merge_request.merge_params&.except!(*clearable_auto_merge_parameters) merge_request.save! end + # Overridden in EE child classes + def clearable_auto_merge_parameters + %w[ + should_remove_source_branch + commit_message + squash_commit_message + auto_merge_strategy + ] + end + def track_exception(error, merge_request) Gitlab::ErrorTracking.track_exception(error, merge_request_id: merge_request&.id) end diff --git a/app/services/boards/update_service.rb b/app/services/boards/update_service.rb index 6ba8f68a4cb..c702398e89e 100644 --- a/app/services/boards/update_service.rb +++ b/app/services/boards/update_service.rb @@ -2,7 +2,7 @@ module Boards class UpdateService < Boards::BaseService - PERMITTED_PARAMS = %i(name hide_backlog_list hide_closed_list).freeze + PERMITTED_PARAMS = %i[name hide_backlog_list hide_closed_list].freeze def execute(board) filter_params diff --git a/app/services/bulk_imports/create_pipeline_trackers_service.rb b/app/services/bulk_imports/create_pipeline_trackers_service.rb deleted file mode 100644 index 7fa62e0ce8a..00000000000 --- a/app/services/bulk_imports/create_pipeline_trackers_service.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -module BulkImports - class CreatePipelineTrackersService - def initialize(entity) - @entity = entity - end - - def execute! - entity.class.transaction do - entity.pipelines.each do |pipeline| - status = skip_pipeline?(pipeline) ? -2 : 0 - - entity.trackers.create!( - stage: pipeline[:stage], - pipeline_name: pipeline[:pipeline], - status: status - ) - end - end - end - - private - - attr_reader :entity - - def skip_pipeline?(pipeline) - return false unless source_version.valid? - - minimum_version, maximum_version = pipeline.values_at(:minimum_source_version, :maximum_source_version) - - if minimum_version && non_patch_source_version < Gitlab::VersionInfo.parse(minimum_version) - log_skipped_pipeline(pipeline, minimum_version, maximum_version) - return true - end - - if maximum_version && non_patch_source_version > Gitlab::VersionInfo.parse(maximum_version) - log_skipped_pipeline(pipeline, minimum_version, maximum_version) - return true - end - - false - end - - def source_version - @source_version ||= entity.bulk_import.source_version_info - end - - def non_patch_source_version - source_version.without_patch - end - - def log_skipped_pipeline(pipeline, minimum_version, maximum_version) - logger.info( - message: 'Pipeline skipped as source instance version not compatible with pipeline', - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_name: pipeline[:pipeline], - minimum_source_version: minimum_version, - maximum_source_version: maximum_version, - source_version: source_version.to_s, - importer: 'gitlab_migration' - ) - end - - def logger - @logger ||= Gitlab::Import::Logger.build - end - end -end diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index 7fc3511a253..d58620eb089 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -118,11 +118,6 @@ module BulkImports end client.get("/#{entity_type}/#{source_entity_identifier}/export_relations/status") - rescue BulkImports::NetworkError => e - # the source instance will return a 404 if the feature is disabled as the endpoint won't be available - return if e.cause.is_a?(Gitlab::HTTP::BlockedUrlError) - - raise ::BulkImports::Error.setting_not_enabled end def track_access_level(entity_params) diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb index 48adb90fb4c..1f2437d783d 100644 --- a/app/services/bulk_imports/file_download_service.rb +++ b/app/services/bulk_imports/file_download_service.rb @@ -15,7 +15,7 @@ module BulkImports ServiceError = Class.new(StandardError) - DEFAULT_ALLOWED_CONTENT_TYPES = %w(application/gzip application/octet-stream).freeze + DEFAULT_ALLOWED_CONTENT_TYPES = %w[application/gzip application/octet-stream].freeze def initialize( configuration:, @@ -83,6 +83,8 @@ module BulkImports end def raise_error(message) + logger.warn(message: message, response_headers: response_headers, importer: 'gitlab_migration') + raise ServiceError, message end @@ -109,12 +111,16 @@ module BulkImports @filename.presence || remote_filename end + def logger + @logger ||= Gitlab::Import::Logger.build + end + def validate_url ::Gitlab::UrlBlocker.validate!( http_client.resource_url(relative_url), allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, - schemes: %w(http https) + schemes: %w[http https] ) end diff --git a/app/services/ci/create_commit_status_service.rb b/app/services/ci/create_commit_status_service.rb new file mode 100644 index 00000000000..e5b446a07e2 --- /dev/null +++ b/app/services/ci/create_commit_status_service.rb @@ -0,0 +1,158 @@ +# frozen_string_literal: true + +module Ci + class CreateCommitStatusService < BaseService + include ::Gitlab::ExclusiveLeaseHelpers + include ::Gitlab::Utils::StrongMemoize + include ::Services::ReturnServiceResponses + + delegate :sha, to: :commit + + def execute(optional_commit_status_params:) + in_lock(pipeline_lock_key, **pipeline_lock_params) do + @optional_commit_status_params = optional_commit_status_params + unsafe_execute + end + end + + private + + attr_reader :pipeline, :stage, :commit_status, :optional_commit_status_params + + def unsafe_execute + return not_found('Commit') if commit.blank? + return bad_request('State is required') if params[:state].blank? + return not_found('References for commit') if ref.blank? + + @pipeline = first_matching_pipeline || create_pipeline + return forbidden unless ::Ability.allowed?(current_user, :update_pipeline, pipeline) + + @stage = find_or_create_external_stage + @commit_status = find_or_build_external_commit_status + + return bad_request(commit_status.errors.messages) if commit_status.invalid? + + response = add_or_update_external_job + + return bad_request(response.message) if response.error? + + update_merge_request_head_pipeline + response + end + + def ref + params[:ref] || first_matching_pipeline&.ref || + repository.branch_names_contains(sha).first + end + strong_memoize_attr :ref + + def commit + project.commit(params[:sha]) + end + strong_memoize_attr :commit + + def first_matching_pipeline + pipelines = project.ci_pipelines.newest_first(sha: sha) + pipelines = pipelines.for_ref(params[:ref]) if params[:ref] + pipelines = pipelines.id_in(params[:pipeline_id]) if params[:pipeline_id] + pipelines.first + end + strong_memoize_attr :first_matching_pipeline + + def name + params[:name] || params[:context] || 'default' + end + + def create_pipeline + project.ci_pipelines.build( + source: :external, + sha: sha, + ref: ref, + user: current_user, + protected: project.protected_for?(ref) + ).tap do |new_pipeline| + new_pipeline.ensure_project_iid! + new_pipeline.save! + end + end + + def find_or_create_external_stage + pipeline.stages.safe_find_or_create_by!(name: 'external') do |stage| # rubocop:disable Performance/ActiveRecordSubtransactionMethods + stage.position = ::GenericCommitStatus::EXTERNAL_STAGE_IDX + stage.project = project + end + end + + def find_or_build_external_commit_status + ::GenericCommitStatus.running_or_pending.find_or_initialize_by( # rubocop:disable CodeReuse/ActiveRecord + project: project, + pipeline: pipeline, + name: name, + ref: ref, + user: current_user, + protected: project.protected_for?(ref), + ci_stage: stage, + stage_idx: stage.position, + stage: 'external' + ).tap do |new_commit_status| + new_commit_status.assign_attributes(optional_commit_status_params) + end + end + + def add_or_update_external_job + ::Ci::Pipelines::AddJobService.new(pipeline).execute!(commit_status) do |job| + apply_job_state!(job) + end + end + + def update_merge_request_head_pipeline + return unless pipeline.latest? + + ::MergeRequest + .from_project(project).from_source_branches(ref) + .update_all(head_pipeline_id: pipeline.id) + end + + def apply_job_state!(job) + case params[:state] + when 'pending' + job.enqueue! + when 'running' + job.enqueue + job.run! + when 'success' + job.success! + when 'failed' + job.drop!(:api_failure) + when 'canceled' + job.cancel! + else + raise('invalid state') + end + end + + def pipeline_lock_key + "api:commit_statuses:project:#{project.id}:sha:#{params[:sha]}" + end + + def pipeline_lock_params + { + ttl: 5.seconds, + sleep_sec: 0.1.seconds, + retries: 20 + } + end + + def not_found(message) + error("404 #{message} Not Found", :not_found) + end + + def bad_request(message) + error(message, :bad_request) + end + + def forbidden + error("403 Forbidden", :forbidden) + end + end +end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index fe0e842f542..2231b1dd6bd 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -154,13 +154,6 @@ module Ci duration >= LOG_MAX_CREATION_THRESHOLD end - - l.log_when do |observations| - pipeline_includes_count = observations['pipeline_includes_count'] - next false unless pipeline_includes_count - - pipeline_includes_count.to_i > Gitlab::Ci::Config::External::Context::TEMP_MAX_INCLUDES - end end end end diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index 953432a9dd3..05cd20a152b 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -105,11 +105,7 @@ module Ci end def pipelines_created_after - if Feature.enabled?(:lower_interval_for_canceling_redundant_pipelines, project) - 3.days.ago - else - 1.week.ago - end + 3.days.ago end # Finding the pipelines to cancel is an expensive task that is not well diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 68ebb376ccd..470a1d3951b 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -10,7 +10,7 @@ module Ci TEMPORARY_LOCK_TIMEOUT = 3.seconds - Result = Struct.new(:build, :build_json, :valid?) + Result = Struct.new(:build, :build_json, :build_presented, :valid?) ## # The queue depth limit number has been determined by observing 95 @@ -25,8 +25,8 @@ module Ci end def execute(params = {}) - db_all_caught_up = - ::Ci::Runner.sticking.all_caught_up?(:runner, runner.id) + replica_caught_up = + ::Ci::Runner.sticking.find_caught_up_replica(:runner, runner.id, use_primary_on_failure: false) @metrics.increment_queue_operation(:queue_attempt) @@ -40,10 +40,10 @@ module Ci # we might still have some CI builds to be picked. Instead we should say to runner: # "Hi, we don't have any more builds now, but not everything is right anyway, so try again". # Runner will retry, but again, against replica, and again will check if replication lag did catch-up. - if !db_all_caught_up && !result.build + if !replica_caught_up && !result.build metrics.increment_queue_operation(:queue_replication_lag) - ::Ci::RegisterJobService::Result.new(nil, nil, false) # rubocop:disable Cop/AvoidReturnFromBlocks + ::Ci::RegisterJobService::Result.new(nil, nil, nil, false) # rubocop:disable Cop/AvoidReturnFromBlocks else result end @@ -86,7 +86,7 @@ module Ci next unless result if result.valid? - @metrics.register_success(result.build) + @metrics.register_success(result.build_presented) @metrics.observe_queue_depth(:found, depth) return result # rubocop:disable Cop/AvoidReturnFromBlocks @@ -102,7 +102,7 @@ module Ci @metrics.observe_queue_depth(:not_found, depth) if valid @metrics.register_failure - Result.new(nil, nil, valid) + Result.new(nil, nil, nil, valid) end # rubocop: disable CodeReuse/ActiveRecord @@ -159,7 +159,7 @@ module Ci # this operation. # if ::Ci::UpdateBuildQueueService.new.remove!(build) - return Result.new(nil, nil, false) + return Result.new(nil, nil, nil, false) end return @@ -190,11 +190,11 @@ module Ci # to make sure that this is properly handled by runner. @metrics.increment_queue_operation(:build_conflict_lock) - Result.new(nil, nil, false) + Result.new(nil, nil, nil, false) rescue StateMachines::InvalidTransition @metrics.increment_queue_operation(:build_conflict_transition) - Result.new(nil, nil, false) + Result.new(nil, nil, nil, false) rescue StandardError => ex @metrics.increment_queue_operation(:build_conflict_exception) @@ -221,7 +221,7 @@ module Ci log_build_dependencies_size(presented_build) build_json = Gitlab::Json.dump(::API::Entities::Ci::JobRequest::Response.new(presented_build)) - Result.new(build, build_json, true) + Result.new(build, build_json, presented_build, true) end def log_build_dependencies_size(presented_build) diff --git a/app/services/ci/update_instance_variables_service.rb b/app/services/ci/update_instance_variables_service.rb index ee513647d08..2f941118a1c 100644 --- a/app/services/ci/update_instance_variables_service.rb +++ b/app/services/ci/update_instance_variables_service.rb @@ -5,7 +5,7 @@ module Ci class UpdateInstanceVariablesService - UNASSIGNABLE_KEYS = %w(id _destroy).freeze + UNASSIGNABLE_KEYS = %w[id _destroy].freeze def initialize(params) @params = params[:variables_attributes] diff --git a/app/services/clusters/agent_tokens/track_usage_service.rb b/app/services/clusters/agent_tokens/track_usage_service.rb index fdc79ac0f8b..18fe236c44d 100644 --- a/app/services/clusters/agent_tokens/track_usage_service.rb +++ b/app/services/clusters/agent_tokens/track_usage_service.rb @@ -4,7 +4,7 @@ module Clusters module AgentTokens class TrackUsageService # The `UPDATE_USED_COLUMN_EVERY` defines how often the token DB entry can be updated - UPDATE_USED_COLUMN_EVERY = (40.minutes..55.minutes).freeze + UPDATE_USED_COLUMN_EVERY = (40.minutes..55.minutes) delegate :agent, to: :token diff --git a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb index e87640f4c76..94781422686 100644 --- a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb +++ b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb @@ -137,9 +137,9 @@ module Clusters name: Clusters::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME, namespace: service_account_namespace, rules: [{ - apiGroups: %w(serving.knative.dev), - resources: %w(configurations configurationgenerations routes revisions revisionuids autoscalers services), - verbs: %w(get list create update delete patch watch) + apiGroups: %w[serving.knative.dev], + resources: %w[configurations configurationgenerations routes revisions revisionuids autoscalers services], + verbs: %w[get list create update delete patch watch] }] ).generate end @@ -159,9 +159,9 @@ module Clusters name: Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_NAME, namespace: service_account_namespace, rules: [{ - apiGroups: %w(database.crossplane.io), - resources: %w(postgresqlinstances), - verbs: %w(get list create watch) + apiGroups: %w[database.crossplane.io], + resources: %w[postgresqlinstances], + verbs: %w[get list create watch] }] ).generate end diff --git a/app/services/commits/create_service.rb b/app/services/commits/create_service.rb index a498d39d34e..89370bd8abb 100644 --- a/app/services/commits/create_service.rb +++ b/app/services/commits/create_service.rb @@ -39,7 +39,12 @@ module Commits Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => ex Gitlab::ErrorTracking.log_exception(ex) - error(ex.message) + + if Feature.enabled?(:errors_utf_8_encoding) + error(Gitlab::EncodingHelper.encode_utf8_no_detect(ex.message)) + else + error(ex.message) + end end private diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 569b91de73e..b02cfea151d 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -17,9 +17,6 @@ class CompareService return unless raw_compare && raw_compare.base && raw_compare.head - Compare.new(raw_compare, - start_project, - base_sha: base_sha, - straight: straight) + Compare.new(raw_compare, start_project, base_sha: base_sha, straight: straight) end end diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index 9fe82507edd..5d35658a2b3 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -36,7 +36,7 @@ module AlertManagement SystemNoteService.log_resolving_alert(alert, alert_source) if alert.resolve(incoming_payload.ends_at) - SystemNoteService.change_alert_status(alert, User.alert_bot) + SystemNoteService.change_alert_status(alert, Users::Internal.alert_bot) close_issue(alert.issue_id) if auto_close_incident? end diff --git a/app/services/concerns/rate_limited_service.rb b/app/services/concerns/rate_limited_service.rb index fa366c1ccd0..79be952ac14 100644 --- a/app/services/concerns/rate_limited_service.rb +++ b/app/services/concerns/rate_limited_service.rb @@ -61,9 +61,11 @@ module RateLimitedService cattr_accessor :rate_limiter_scoped_and_keyed def self.rate_limit(key:, opts:, rate_limiter: ::Gitlab::ApplicationRateLimiter) - self.rate_limiter_scoped_and_keyed = RateLimiterScopedAndKeyed.new(key: key, - opts: opts, - rate_limiter: rate_limiter) + self.rate_limiter_scoped_and_keyed = RateLimiterScopedAndKeyed.new( + key: key, + opts: opts, + rate_limiter: rate_limiter + ) end end diff --git a/app/services/concerns/service_desk/custom_emails/logger.rb b/app/services/concerns/service_desk/custom_emails/logger.rb new file mode 100644 index 00000000000..1817933c3d0 --- /dev/null +++ b/app/services/concerns/service_desk/custom_emails/logger.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module ServiceDesk + module CustomEmails + module Logger + private + + def log_warning(error_message: nil) + with_context do + Gitlab::AppLogger.warn(build_log_message(error_message: error_message)) + end + end + + def log_info(error_message: nil, project: nil) + with_context(project: project) do + Gitlab::AppLogger.info(build_log_message(error_message: error_message)) + end + end + + def with_context(project: nil, &block) + Gitlab::ApplicationContext.with_context( + related_class: self.class.to_s, + user: current_user, + project: project || self.project, + &block + ) + end + + def log_category + 'custom_email' + end + + def build_log_message(error_message: nil) + { + category: log_category, + error_message: error_message + }.compact + end + end + end +end diff --git a/app/services/concerns/update_repository_storage_methods.rb b/app/services/concerns/update_repository_storage_methods.rb index dca38abf7af..f14c79ecd7e 100644 --- a/app/services/concerns/update_repository_storage_methods.rb +++ b/app/services/concerns/update_repository_storage_methods.rb @@ -69,7 +69,17 @@ module UpdateRepositoryStorageMethods raise Error, s_('UpdateRepositoryStorage|Timeout waiting for %{type} repository pushes') % { type: type.name } end - repository = type.repository_for(container) + # `Projects::UpdateRepositoryStorageService`` expects the repository it is + # moving to have a `Project` as a container. + # This hack allows design repos to also be moved as part of a project move + # as before. + # The alternative to this hack is to setup a service like + # `Snippets::UpdateRepositoryStorageService' and a corresponding worker like + # `Snippets::UpdateRepositoryStorageWorker` for snippets. + # + # Gitlab issue: https://gitlab.com/gitlab-org/gitlab/-/issues/423429 + + repository = type.repository_for(type.design? ? container.design_management_repository : container) full_path = repository.full_path raw_repository = repository.raw checksum = repository.checksum diff --git a/app/services/design_management/copy_design_collection/copy_service.rb b/app/services/design_management/copy_design_collection/copy_service.rb index 8074a193bbf..d391c13696f 100644 --- a/app/services/design_management/copy_design_collection/copy_service.rb +++ b/app/services/design_management/copy_design_collection/copy_service.rb @@ -58,8 +58,8 @@ module DesignManagement private attr_reader :designs, :event_enum_map, :git_user, :sha_attribute, :shas, - :temporary_branch, :target_design_collection, :target_issue, - :target_repository, :target_project, :versions + :temporary_branch, :target_design_collection, :target_issue, + :target_repository, :target_project, :versions alias_method :merge_branch, :target_branch diff --git a/app/services/design_management/delete_designs_service.rb b/app/services/design_management/delete_designs_service.rb index 921c904d8de..a6a0f5e0252 100644 --- a/app/services/design_management/delete_designs_service.rb +++ b/app/services/design_management/delete_designs_service.rb @@ -16,8 +16,10 @@ module DesignManagement version = delete_designs! EventCreateService.new.destroy_designs(designs, current_user) - Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_designs_removed_action(author: current_user, - project: project) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_designs_removed_action( + author: current_user, + project: project + ) TodosDestroyer::DestroyedDesignsWorker.perform_async(designs.map(&:id)) success(version: version) diff --git a/app/services/design_management/runs_design_actions.rb b/app/services/design_management/runs_design_actions.rb index 267ed6bf29f..62db7824592 100644 --- a/app/services/design_management/runs_design_actions.rb +++ b/app/services/design_management/runs_design_actions.rb @@ -15,10 +15,12 @@ module DesignManagement def run_actions(actions, skip_system_notes: false) raise NoActions if actions.empty? - sha = repository.commit_files(current_user, - branch_name: target_branch, - message: commit_message, - actions: actions.map(&:gitaly_action)) + sha = repository.commit_files( + current_user, + branch_name: target_branch, + message: commit_message, + actions: actions.map(&:gitaly_action) + ) ::DesignManagement::Version .create_for_designs(actions, sha, current_user) diff --git a/app/services/design_management/save_designs_service.rb b/app/services/design_management/save_designs_service.rb index ea5675c6ddd..4c4e34862e8 100644 --- a/app/services/design_management/save_designs_service.rb +++ b/app/services/design_management/save_designs_service.rb @@ -131,11 +131,11 @@ module DesignManagement def track_usage_metrics(action) if action == :update - ::Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_designs_modified_action(author: current_user, - project: project) + ::Gitlab::UsageDataCounters::IssueActivityUniqueCounter + .track_issue_designs_modified_action(author: current_user, project: project) else - ::Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_designs_added_action(author: current_user, - project: project) + ::Gitlab::UsageDataCounters::IssueActivityUniqueCounter + .track_issue_designs_added_action(author: current_user, project: project) end ::Gitlab::UsageDataCounters::DesignsCounter.count(action) diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb index 8458eb1f3b8..e95d4eec3c8 100644 --- a/app/services/error_tracking/base_service.rb +++ b/app/services/error_tracking/base_service.rb @@ -17,8 +17,7 @@ module ErrorTracking private def perform - raise NotImplementedError, - "#{self.class} does not implement #{__method__}" + raise NotImplementedError, "#{self.class} does not implement #{__method__}" end def compose_response(response, &block) @@ -33,8 +32,7 @@ module ErrorTracking end def parse_response(response) - raise NotImplementedError, - "#{self.class} does not implement #{__method__}" + raise NotImplementedError, "#{self.class} does not implement #{__method__}" end def unauthorized diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 1893cfcfcff..b755f512772 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -190,10 +190,14 @@ class EventCreateService private def create_record_event(record, current_user, status, fingerprint = nil) - create_event(record.resource_parent, current_user, status, - fingerprint: fingerprint, - target_id: record.id, - target_type: record.class.name) + create_event( + record.resource_parent, + current_user, + status, + fingerprint: fingerprint, + target_id: record.id, + target_type: record.class.name + ) end # If creating several events, this method will insert them all in a single diff --git a/app/services/feature_flags/base_service.rb b/app/services/feature_flags/base_service.rb index 834409bf3c4..961e8d54efa 100644 --- a/app/services/feature_flags/base_service.rb +++ b/app/services/feature_flags/base_service.rb @@ -4,7 +4,7 @@ module FeatureFlags class BaseService < ::BaseService include Gitlab::Utils::StrongMemoize - AUDITABLE_ATTRIBUTES = %w(name description active).freeze + AUDITABLE_ATTRIBUTES = %w[name description active].freeze def success(**args) sync_to_jira(args[:feature_flag]) diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index dd09ecafb4f..a7be73aa04c 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -2,7 +2,7 @@ module Files class MultiService < Files::BaseService - UPDATE_FILE_ACTIONS = %w(update move delete chmod).freeze + UPDATE_FILE_ACTIONS = %w[update move delete chmod].freeze def create_commit! transformer = Lfs::FileTransformer.new(project, repository, @branch_name) diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 9fa966bb8a8..c11917b92ec 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -3,15 +3,19 @@ module Files class UpdateService < Files::BaseService def create_commit! - repository.update_file(current_user, @file_path, @file_content, - message: @commit_message, - branch_name: @branch_name, - previous_path: @previous_path, - author_email: @author_email, - author_name: @author_name, - start_project: @start_project, - start_branch_name: @start_branch, - execute_filemode: @execute_filemode) + repository.update_file( + current_user, + @file_path, + @file_content, + message: @commit_message, + branch_name: @branch_name, + previous_path: @previous_path, + author_email: @author_email, + author_name: @author_name, + start_project: @start_project, + start_branch_name: @start_branch, + execute_filemode: @execute_filemode + ) end private diff --git a/app/services/google_cloud/create_cloudsql_instance_service.rb b/app/services/google_cloud/create_cloudsql_instance_service.rb index 8d040c6c908..9a1263f0796 100644 --- a/app/services/google_cloud/create_cloudsql_instance_service.rb +++ b/app/services/google_cloud/create_cloudsql_instance_service.rb @@ -17,26 +17,30 @@ module GoogleCloud private def create_cloud_instance - google_api_client.create_cloudsql_instance(gcp_project_id, - instance_name, - root_password, - database_version, - region, - tier) + google_api_client.create_cloudsql_instance( + gcp_project_id, + instance_name, + root_password, + database_version, + region, + tier + ) end def trigger_instance_setup_worker - GoogleCloud::CreateCloudsqlInstanceWorker.perform_in(WORKER_INTERVAL, - current_user.id, - project.id, - { - 'google_oauth2_token': google_oauth2_token, - 'gcp_project_id': gcp_project_id, - 'instance_name': instance_name, - 'database_version': database_version, - 'environment_name': environment_name, - 'is_protected': protected? - }) + GoogleCloud::CreateCloudsqlInstanceWorker.perform_in( + WORKER_INTERVAL, + current_user.id, + project.id, + { + 'google_oauth2_token': google_oauth2_token, + 'gcp_project_id': gcp_project_id, + 'instance_name': instance_name, + 'database_version': database_version, + 'environment_name': environment_name, + 'is_protected': protected? + } + ) end def protected? diff --git a/app/services/google_cloud/fetch_google_ip_list_service.rb b/app/services/google_cloud/fetch_google_ip_list_service.rb index f7739971603..54af841d002 100644 --- a/app/services/google_cloud/fetch_google_ip_list_service.rb +++ b/app/services/google_cloud/fetch_google_ip_list_service.rb @@ -18,9 +18,11 @@ module GoogleCloud subnets = fetch_and_update_cache! - Gitlab::AppJsonLogger.info(class: self.class.name, - message: 'Successfully retrieved Google IP list', - subnet_count: subnets.count) + Gitlab::AppJsonLogger.info( + class: self.class.name, + message: 'Successfully retrieved Google IP list', + subnet_count: subnets.count + ) success({ subnets: subnets }) rescue IpListNotRetrievedError => err diff --git a/app/services/google_cloud/generate_pipeline_service.rb b/app/services/google_cloud/generate_pipeline_service.rb index 95de1fa21b7..30c358687aa 100644 --- a/app/services/google_cloud/generate_pipeline_service.rb +++ b/app/services/google_cloud/generate_pipeline_service.rb @@ -71,8 +71,12 @@ module GoogleCloud end def pipeline_content(include_path) - gitlab_ci_yml = ::Gitlab::Ci::Config::Yaml.load!(default_branch_gitlab_ci_yml || '{}') - append_remote_include(gitlab_ci_yml, "https://gitlab.com/gitlab-org/incubation-engineering/five-minute-production/library/-/raw/main/#{include_path}") + gitlab_ci_yml = ::Gitlab::Ci::Config::Yaml::Loader.new(default_branch_gitlab_ci_yml || '{}').load + + append_remote_include( + gitlab_ci_yml.content, + "https://gitlab.com/gitlab-org/incubation-engineering/five-minute-production/library/-/raw/main/#{include_path}" + ) end def append_remote_include(gitlab_ci_yml, include_url) diff --git a/app/services/gpg_keys/destroy_service.rb b/app/services/gpg_keys/destroy_service.rb index 2e82509897e..c8ee90db9f6 100644 --- a/app/services/gpg_keys/destroy_service.rb +++ b/app/services/gpg_keys/destroy_service.rb @@ -2,9 +2,24 @@ module GpgKeys class DestroyService < Keys::BaseService + BATCH_SIZE = 1000 + def execute(key) + nullify_signatures(key) key.destroy end + + private + + # When a GPG key is deleted, the related signatures have their gpg_key_id column nullified + # However, when the number of signatures is large, then a timeout may happen + # The signatures are processed in batches before GPG key delete is attempted in order to + # avoid timeouts + def nullify_signatures(key) + key.gpg_signatures.each_batch(of: BATCH_SIZE) do |batch| + batch.update_all(gpg_key_id: nil) + end + end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 0f74b2d9349..21d3c6499a0 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -35,10 +35,14 @@ module Groups @group.build_chat_team(name: response['name'], team_id: response['id']) end - Group.transaction do - if @group.save - @group.add_owner(current_user) - Integration.create_from_active_default_integrations(@group, :group_id) + Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction( + %w[routes redirect_routes], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424281' + ) do + Group.transaction do + if @group.save + @group.add_owner(current_user) + Integration.create_from_active_default_integrations(@group, :group_id) + end end end diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 45e8972213e..a896ca5cabc 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -14,8 +14,6 @@ module Groups # TODO - add a policy check here https://gitlab.com/gitlab-org/gitlab/-/issues/353082 raise DestroyError, "You can't delete this group because you're blocked." if current_user.blocked? - group.prepare_for_destroy - group.projects.includes(:project_feature).each do |project| # Execute the destruction of the models immediately to ensure atomic cleanup. success = ::Projects::DestroyService.new(project, current_user).execute @@ -83,7 +81,10 @@ module Groups # rubocop:disable CodeReuse/ActiveRecord def destroy_group_bots - bot_ids = group.members_and_requesters.joins(:user).merge(User.project_bot).pluck(:user_id) + bot_ids = group.members_and_requesters.joins(:user) + .merge(User.project_bot) + .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/422405') + .pluck(:user_id) current_user_id = current_user.id group.run_after_commit do diff --git a/app/services/groups/ssh_certificates/create_service.rb b/app/services/groups/ssh_certificates/create_service.rb new file mode 100644 index 00000000000..6890901c306 --- /dev/null +++ b/app/services/groups/ssh_certificates/create_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Groups + module SshCertificates + class CreateService + def initialize(group, params) + @group = group + @params = params + end + + def execute + key = params[:key] + fingerprint = generate_fingerprint(key) + + return ServiceResponse.error(message: 'Group', reason: :forbidden) if group.has_parent? + + # return a key error instead of fingerprint error, as the user has no knowledge of fingerprint. + unless fingerprint + return ServiceResponse.error(message: 'Validation failed: Invalid key', + reason: :unprocessable_entity) + end + + result = group.ssh_certificates.create!( + key: key, + title: params[:title], + fingerprint: fingerprint + ) + + # title and key attributes are returned as [FILTERED] + # by config/application.rb#L181-233 + # make attributes unfiltered by running find + ssh_certificate = group.ssh_certificates.find(result.id) + ServiceResponse.success(payload: ssh_certificate) + + rescue ActiveRecord::RecordInvalid, ArgumentError => e + ServiceResponse.error( + message: e.message, + reason: :unprocessable_entity + ) + end + + private + + attr_reader :group, :params + + def generate_fingerprint(key) + Gitlab::SSHPublicKey.new(key).fingerprint_sha256&.delete_prefix('SHA256:') + end + end + end +end diff --git a/app/services/groups/ssh_certificates/destroy_service.rb b/app/services/groups/ssh_certificates/destroy_service.rb new file mode 100644 index 00000000000..7a450d5bee6 --- /dev/null +++ b/app/services/groups/ssh_certificates/destroy_service.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Groups + module SshCertificates + class DestroyService + def initialize(group, params) + @group = group + @params = params + end + + def execute + ssh_certificate = group.ssh_certificates.find(params[:ssh_certificates_id]) + + ssh_certificate.destroy! + ServiceResponse.success + + rescue ActiveRecord::RecordNotFound + ServiceResponse.error( + message: 'SSH Certificate not found', + reason: :not_found + ) + + rescue ActiveRecord::RecordNotDestroyed + ServiceResponse.error( + message: 'SSH Certificate could not be deleted', + reason: :method_not_allowed + ) + end + + private + + attr_reader :group, :params + end + end +end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 64256e43ce3..6b979308d26 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -60,13 +60,17 @@ module Groups 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) - remove_namespace_commit_emails(was_root_group) + Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction( + %w[routes redirect_routes], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424280' + ) do + 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) + remove_namespace_commit_emails(was_root_group) + end end post_update_hooks(@updated_project_ids, old_root_ancestor_id) diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 7d0142fc067..d91e09d212a 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -47,10 +47,6 @@ module Groups private def valid_path_change? - unless Feature.enabled?(:npm_package_registry_fix_group_path_validation) - return valid_path_change_with_npm_packages? - end - return true unless group.packages_feature_enabled? return true if params[:path].blank? return true if group.has_parent? @@ -68,21 +64,6 @@ module Groups false end - # TODO: delete this function along with npm_package_registry_fix_group_path_validation - def valid_path_change_with_npm_packages? - return true unless group.packages_feature_enabled? - return true if params[:path].blank? - return true if !group.has_parent? && group.path == params[:path] - - npm_packages = ::Packages::GroupPackagesFinder.new(current_user, group, package_type: :npm).execute - if npm_packages.exists? - group.errors.add(:path, s_('GroupSettings|cannot change when group contains projects with NPM packages')) - return - end - - true - end - def before_assignment_hook(group, params) @full_path_before = group.full_path @path_before = group.path diff --git a/app/services/import/bitbucket_server_service.rb b/app/services/import/bitbucket_server_service.rb index 5d496dc7cc3..3d961780889 100644 --- a/app/services/import/bitbucket_server_service.rb +++ b/app/services/import/bitbucket_server_service.rb @@ -83,7 +83,7 @@ module Import url, allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, - schemes: %w(http https) + schemes: %w[http https] ) end diff --git a/app/services/import/fogbugz_service.rb b/app/services/import/fogbugz_service.rb index 9a8def43312..2f63e4e6fb7 100644 --- a/app/services/import/fogbugz_service.rb +++ b/app/services/import/fogbugz_service.rb @@ -88,7 +88,7 @@ module Import url, allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, - schemes: %w(http https) + schemes: %w[http https] ) end diff --git a/app/services/import/github/cancel_project_import_service.rb b/app/services/import/github/cancel_project_import_service.rb index 62cd0c95eaf..740b9e5c2e7 100644 --- a/app/services/import/github/cancel_project_import_service.rb +++ b/app/services/import/github/cancel_project_import_service.rb @@ -7,13 +7,13 @@ module Import return error('Not Found', :not_found) unless authorized_to_read? return error('Unauthorized access', :forbidden) unless authorized_to_cancel? - if project.import_in_progress? + if project.import_state.completed? + error(cannot_cancel_error_message, :bad_request) + else project.import_state.cancel metrics.track_canceled_import success(project: project) - else - error(cannot_cancel_error_message, :bad_request) end end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index df255a7ae24..73e0c229a9c 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -91,7 +91,7 @@ module Import url, allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, - schemes: %w(http https) + schemes: %w[http https] ) end diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb index e30818cc5d2..ed99d20d67f 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb @@ -11,7 +11,7 @@ module Import end validates :file_url, addressable_url: { - schemes: %w(https), + schemes: %w[https], allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, dns_rebind_protection: true diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb index 7599343d4e1..57ed717b966 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file_s3.rb @@ -13,7 +13,7 @@ module Import validates_presence_of :region, :bucket_name, :file_key, :access_key_id, :secret_access_key validates :file_url, addressable_url: { - schemes: %w(https), + schemes: %w[https], allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?, dns_rebind_protection: true diff --git a/app/services/import/validate_remote_git_endpoint_service.rb b/app/services/import/validate_remote_git_endpoint_service.rb index 2886bd5c9b7..a994072c4aa 100644 --- a/app/services/import/validate_remote_git_endpoint_service.rb +++ b/app/services/import/validate_remote_git_endpoint_service.rb @@ -8,7 +8,7 @@ module Import GIT_SERVICE_NAME = "git-upload-pack" GIT_EXPECTED_FIRST_PACKET_LINE = "# service=#{GIT_SERVICE_NAME}" - GIT_BODY_MESSAGE_REGEXP = /^[0-9a-f]{4}#{GIT_EXPECTED_FIRST_PACKET_LINE}/.freeze + GIT_BODY_MESSAGE_REGEXP = /^[0-9a-f]{4}#{GIT_EXPECTED_FIRST_PACKET_LINE}/ # https://github.com/git/git/blob/master/Documentation/technical/protocol-common.txt#L56-L59 GIT_PROTOCOL_PKT_LEN = 4 GIT_MINIMUM_RESPONSE_LENGTH = GIT_PROTOCOL_PKT_LEN + GIT_EXPECTED_FIRST_PACKET_LINE.length diff --git a/app/services/import_export_clean_up_service.rb b/app/services/import_export_clean_up_service.rb index 567ac065cf7..5ee2f70ec4c 100644 --- a/app/services/import_export_clean_up_service.rb +++ b/app/services/import_export_clean_up_service.rb @@ -60,7 +60,7 @@ class ImportExportCleanUpService end def directories_cmd - %W(find #{path} -mindepth #{DIR_DEPTH} -maxdepth #{DIR_DEPTH} -type d -not -path #{path} -mmin +#{mmin}) + %W[find #{path} -mindepth #{DIR_DEPTH} -maxdepth #{DIR_DEPTH} -type d -not -path #{path} -mmin +#{mmin}] end def logger diff --git a/app/services/incident_management/pager_duty/create_incident_issue_service.rb b/app/services/incident_management/pager_duty/create_incident_issue_service.rb index 0c9ca2c0add..58c3a062910 100644 --- a/app/services/incident_management/pager_duty/create_incident_issue_service.rb +++ b/app/services/incident_management/pager_duty/create_incident_issue_service.rb @@ -6,7 +6,7 @@ module IncidentManagement include IncidentManagement::Settings def initialize(project, incident_payload) - super(project, User.alert_bot, incident_payload) + super(project, Users::Internal.alert_bot, incident_payload) end def execute diff --git a/app/services/incident_management/pager_duty/process_webhook_service.rb b/app/services/incident_management/pager_duty/process_webhook_service.rb index 3ce2674616e..6f779bfdb18 100644 --- a/app/services/incident_management/pager_duty/process_webhook_service.rb +++ b/app/services/incident_management/pager_duty/process_webhook_service.rb @@ -10,7 +10,7 @@ module IncidentManagement PAGER_DUTY_PAYLOAD_SIZE_LIMIT = 55.kilobytes # https://developer.pagerduty.com/docs/db0fa8c8984fc-overview#event-types - PAGER_DUTY_PROCESSABLE_EVENT_TYPES = %w(incident.triggered).freeze + PAGER_DUTY_PROCESSABLE_EVENT_TYPES = %w[incident.triggered].freeze def initialize(project, payload) super(project: project) diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb index 166452968f4..e996aecdf97 100644 --- a/app/services/issuable/bulk_update_service.rb +++ b/app/services/issuable/bulk_update_service.rb @@ -43,7 +43,7 @@ module Issuable end def permitted_attrs(type) - attrs = %i(state_event milestone_id add_label_ids remove_label_ids subscription_event) + attrs = %i[state_event milestone_id add_label_ids remove_label_ids subscription_event] if type == 'issue' attrs.push(:assignee_ids, :confidential) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 3b007d4dba7..27cfaef2db2 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -169,7 +169,7 @@ class IssuableBaseService < ::BaseContainerService params[:incident_management_issuable_escalation_status_attributes] = result[:escalation_status] end - def process_label_ids(attributes, existing_label_ids: nil, extra_label_ids: []) + def process_label_ids(attributes, issuable:, existing_label_ids: nil, extra_label_ids: []) # rubocop:disable Lint/UnusedMethodArgument label_ids = attributes.delete(:label_ids) add_label_ids = attributes.delete(:add_label_ids) remove_label_ids = attributes.delete(:remove_label_ids) @@ -180,15 +180,29 @@ class IssuableBaseService < ::BaseContainerService new_label_ids |= add_label_ids if add_label_ids new_label_ids -= remove_label_ids if remove_label_ids - new_label_ids.uniq + filter_locked_labels(issuable, new_label_ids.uniq, existing_label_ids) + end + + # Filter out any locked labels that are attempting to be removed + def filter_locked_labels(issuable, ids, existing_label_ids) + return ids unless issuable.supports_lock_on_merge? + return ids unless existing_label_ids.present? + + removed_label_ids = existing_label_ids - ids + removed_locked_label_ids = labels_service.filter_locked_label_ids(removed_label_ids) + + ids + removed_locked_label_ids end def process_assignee_ids(attributes, existing_assignee_ids: nil, extra_assignee_ids: []) - process = Issuable::ProcessAssignees.new(assignee_ids: attributes.delete(:assignee_ids), - add_assignee_ids: attributes.delete(:add_assignee_ids), - remove_assignee_ids: attributes.delete(:remove_assignee_ids), - existing_assignee_ids: existing_assignee_ids, - extra_assignee_ids: extra_assignee_ids) + process = Issuable::ProcessAssignees.new( + assignee_ids: attributes.delete(:assignee_ids), + add_assignee_ids: attributes.delete(:add_assignee_ids), + remove_assignee_ids: attributes.delete(:remove_assignee_ids), + existing_assignee_ids: existing_assignee_ids, + extra_assignee_ids: extra_assignee_ids + ) + process.execute end @@ -221,7 +235,7 @@ class IssuableBaseService < ::BaseContainerService params.delete(:state_event) params[:author] ||= current_user - params[:label_ids] = process_label_ids(params, extra_label_ids: issuable.label_ids.to_a) + params[:label_ids] = process_label_ids(params, issuable: issuable, extra_label_ids: issuable.label_ids.to_a) if issuable.respond_to?(:assignee_ids) params[:assignee_ids] = process_assignee_ids(params, extra_assignee_ids: issuable.assignee_ids.to_a) @@ -373,9 +387,11 @@ class IssuableBaseService < ::BaseContainerService filter_params(issuable) if issuable.changed? || params.present? - issuable.assign_attributes(params.merge(updated_by: current_user, - last_edited_at: Time.current, - last_edited_by: current_user)) + issuable.assign_attributes(params.merge( + updated_by: current_user, + last_edited_at: Time.current, + last_edited_by: current_user + )) before_update(issuable, skip_spam_check: true) @@ -404,10 +420,13 @@ class IssuableBaseService < ::BaseContainerService update_task_params = params.delete(:update_task) return unless update_task_params - tasklist_toggler = TaskListToggleService.new(issuable.description, issuable.description_html, - line_source: update_task_params[:line_source], - line_number: update_task_params[:line_number].to_i, - toggle_as_checked: update_task_params[:checked]) + tasklist_toggler = TaskListToggleService.new( + issuable.description, + issuable.description_html, + line_source: update_task_params[:line_source], + line_number: update_task_params[:line_number].to_i, + toggle_as_checked: update_task_params[:checked] + ) unless tasklist_toggler.execute # if we make it here, the data is much newer than we thought it was - fail fast @@ -469,7 +488,7 @@ class IssuableBaseService < ::BaseContainerService # rubocop: enable CodeReuse/ActiveRecord def assign_requested_labels(issuable) - label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) + label_ids = process_label_ids(params, issuable: issuable, existing_label_ids: issuable.label_ids) return unless ids_changing?(issuable.label_ids, label_ids) params[:label_ids] = label_ids diff --git a/app/services/issuable_links/create_service.rb b/app/services/issuable_links/create_service.rb index 533e92f6225..761ba3f74aa 100644 --- a/app/services/issuable_links/create_service.rb +++ b/app/services/issuable_links/create_service.rb @@ -76,13 +76,13 @@ module IssuableLinks target_issuables.map do |referenced_object| link = relate_issuables(referenced_object) - if link.valid? - after_create_for(link) - else + if link.errors.any? @errors << _("%{ref} cannot be added: %{error}") % { ref: referenced_object.to_reference, error: link.errors.messages.values.flatten.to_sentence } + else + after_create_for(link) end link diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index b9b7cd08b68..a5ae5854e33 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -21,7 +21,7 @@ module Issues Issues::CloseService end - NO_REBALANCING_NEEDED = ((RelativePositioning::MIN_POSITION * 0.9999)..(RelativePositioning::MAX_POSITION * 0.9999)).freeze + NO_REBALANCING_NEEDED = ((RelativePositioning::MIN_POSITION * 0.9999)..(RelativePositioning::MAX_POSITION * 0.9999)) def rebalance_if_needed(issue) return unless issue @@ -111,9 +111,6 @@ module Issues issue.namespace.execute_integrations(issue_data, hooks_scope) execute_incident_hooks(issue, issue_data) if issue.work_item_type&.incident? - - return unless Feature.enabled?(:group_mentions, issue.project) - execute_group_mention_hooks(issue, issue_data) if action == 'open' end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index f848a8db12a..ef43e707a21 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -6,10 +6,12 @@ module Issues def execute(issue, commit: nil, notifications: true, system_note: true, skip_authorization: false) return issue unless can_close?(issue, skip_authorization: skip_authorization) - close_issue(issue, - closed_via: commit, - notifications: notifications, - system_note: system_note) + close_issue( + issue, + closed_via: commit, + notifications: notifications, + system_note: system_note + ) end # Closes the supplied issue without checking if the user is authorized to @@ -86,7 +88,7 @@ module Issues issue = alert.issue if alert.resolve - SystemNoteService.change_alert_status(alert, User.alert_bot, " because #{current_user.to_reference} closed incident #{issue.to_reference(project)}") + SystemNoteService.change_alert_status(alert, Users::Internal.alert_bot, " because #{current_user.to_reference} closed incident #{issue.to_reference(project)}") else Gitlab::AppLogger.warn( message: 'Cannot resolve an associated Alert Management alert', diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index e1ddfe47439..c828c156d50 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -7,7 +7,7 @@ module Issues include ::Services::ReturnServiceResponses rate_limit key: :issues_create, - opts: { scope: [:project, :current_user, :external_author] } + opts: { scope: [:project, :current_user, :external_author] } def initialize(container:, current_user: nil, params: {}, build_service: nil, perform_spam_check: true) @extra_params = params.delete(:extra_params) || {} @@ -90,9 +90,12 @@ module Issues def resolve_discussions_with_issue(issue) return if discussions_to_resolve.empty? - Discussions::ResolveService.new(project, current_user, - one_or_more_discussions: discussions_to_resolve, - follow_up_issue: issue).execute + Discussions::ResolveService.new( + project, + current_user, + one_or_more_discussions: discussions_to_resolve, + follow_up_issue: issue + ).execute end private diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index e26e3d0835b..c3ddf7b6709 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -141,15 +141,23 @@ module Issues end def add_note_from - SystemNoteService.noteable_moved(new_entity, target_project, - original_entity, current_user, - direction: :from) + SystemNoteService.noteable_moved( + new_entity, + target_project, + original_entity, + current_user, + direction: :from + ) end def add_note_to - SystemNoteService.noteable_moved(original_entity, old_project, - new_entity, current_user, - direction: :to) + SystemNoteService.noteable_moved( + original_entity, + old_project, + new_entity, + current_user, + direction: :to + ) end end end diff --git a/app/services/issues/relative_position_rebalancing_service.rb b/app/services/issues/relative_position_rebalancing_service.rb index a8d0ae01176..e165cb36634 100644 --- a/app/services/issues/relative_position_rebalancing_service.rb +++ b/app/services/issues/relative_position_rebalancing_service.rb @@ -141,7 +141,7 @@ module Issues def run_update_query(values, query_name) Issue.connection.exec_query(<<~SQL, query_name) - WITH cte(cte_id, new_pos) AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported} ( + WITH cte(cte_id, new_pos) AS MATERIALIZED ( SELECT * FROM (VALUES #{values}) as t (id, pos) ) diff --git a/app/services/labels/available_labels_service.rb b/app/services/labels/available_labels_service.rb index 21f92eeaf09..7809f263eb6 100644 --- a/app/services/labels/available_labels_service.rb +++ b/app/services/labels/available_labels_service.rb @@ -40,19 +40,8 @@ module Labels ids.map(&:to_i) & existing_ids end - def filter_locked_labels_ids_in_param(key) - ids = Array.wrap(params[key]) - return [] if ids.empty? - - params = finder_params - params[:locked_labels] = true - existing_labels = LabelsFinder.new(current_user, params).execute - - # rubocop:disable CodeReuse/ActiveRecord - existing_ids = existing_labels.id_in(ids).pluck(:id) - # rubocop:enable CodeReuse/ActiveRecord - - ids.map(&:to_i) & existing_ids + def filter_locked_label_ids(ids) + available_labels.with_lock_on_merge.id_in(ids).pluck(:id) # rubocop:disable CodeReuse/ActiveRecord end def available_labels diff --git a/app/services/labels/create_service.rb b/app/services/labels/create_service.rb index 675439b2f64..c69b9bd8de7 100644 --- a/app/services/labels/create_service.rb +++ b/app/services/labels/create_service.rb @@ -13,9 +13,7 @@ module Labels project_or_group = target_params[:project] || target_params[:group] if project_or_group.present? - if Feature.disabled?(:enforce_locked_labels_on_merge, project_or_group, type: :ops) - params.delete(:lock_on_merge) - end + params.delete(:lock_on_merge) unless project_or_group.supports_lock_on_merge? project_or_group.labels.create(params) elsif target_params[:template] diff --git a/app/services/labels/update_service.rb b/app/services/labels/update_service.rb index 4ac54959e84..0ffb0fabf21 100644 --- a/app/services/labels/update_service.rb +++ b/app/services/labels/update_service.rb @@ -21,8 +21,12 @@ module Labels def allow_lock_on_merge?(label) return if label.template? return unless label.respond_to?(:parent_container) + return unless label.parent_container.supports_lock_on_merge? - Feature.enabled?(:enforce_locked_labels_on_merge, label.parent_container, type: :ops) + # If we've made it here, then we're allowed to turn it on. However, we do _not_ + # want to allow it to be turned off. So if it's already set, then don't allow the possibility + # that it could be turned off. + !label.lock_on_merge end end 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 8700276c982..e0c9c19f5b9 100644 --- a/app/services/loose_foreign_keys/process_deleted_records_service.rb +++ b/app/services/loose_foreign_keys/process_deleted_records_service.rb @@ -4,13 +4,13 @@ module LooseForeignKeys class ProcessDeletedRecordsService BATCH_SIZE = 1000 - def initialize(connection:) + def initialize(connection:, modification_tracker: LooseForeignKeys::ModificationTracker.new) @connection = connection + @modification_tracker = modification_tracker end def execute raised_error = false - modification_tracker = ModificationTracker.new tracked_tables.cycle do |table| records = load_batch_for_table(table) @@ -54,7 +54,7 @@ module LooseForeignKeys private - attr_reader :connection + attr_reader :connection, :modification_tracker def db_config_name ::Gitlab::Database.db_config_name(connection) diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index a6fff3003ac..cc18aae7446 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -39,31 +39,34 @@ module Members sources = Array.wrap(sources) if sources.is_a?(ApplicationRecord) # For single source - Member.transaction do - sources.flat_map do |source| - # If this user is attempting to manage Owner members and doesn't have permission, do not allow - if managing_owners?(args[:current_user], access_level) && cannot_manage_owners?(source, args[:current_user]) - next [] + Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction( + %w[users user_preferences user_details emails identities], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424276' + ) do + Member.transaction do + sources.flat_map do |source| + # If this user is attempting to manage Owner members and doesn't have permission, do not allow + current_user = args[:current_user] + next [] if managing_owners?(current_user, access_level) && cannot_manage_owners?(source, current_user) + + emails, users, existing_members = parse_users_list(source, invitees) + + common_arguments = { + source: source, + access_level: access_level, + existing_members: existing_members, + tasks_to_be_done: args[:tasks_to_be_done] || [] + }.merge(parsed_args(args)) + + members = emails.map do |email| + new(invitee: email, builder: InviteMemberBuilder, **common_arguments).execute + end + + members += users.map do |user| + new(invitee: user, **common_arguments).execute + end + + members end - - emails, users, existing_members = parse_users_list(source, invitees) - - common_arguments = { - source: source, - access_level: access_level, - existing_members: existing_members, - tasks_to_be_done: args[:tasks_to_be_done] || [] - }.merge(parsed_args(args)) - - members = emails.map do |email| - new(invitee: email, builder: InviteMemberBuilder, **common_arguments).execute - end - - members += users.map do |user| - new(invitee: user, **common_arguments).execute - end - - members end end end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index e432016795d..d4cc60c6de0 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -47,6 +47,10 @@ module Members def enqueue_jobs_that_needs_to_be_run_only_once_per_hierarchy(member, unassign_issuables) return if recursive_call? + enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables) + end + + def enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables) enqueue_delete_todos(member) enqueue_unassign_issuables(member) if unassign_issuables end diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 8560a15b7c4..dbe5567cbc5 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -5,12 +5,17 @@ module MergeRequests def execute(merge_request) return unless eligible_for_approval?(merge_request) - approval = merge_request.approvals.new(user: current_user) + approval = merge_request.approvals.new( + user: current_user, + patch_id_sha: fetch_patch_id_sha(merge_request) + ) return success unless save_approval(approval) reset_approvals_cache(merge_request) + merge_request_activity_counter.track_approve_mr_action(user: current_user, merge_request: merge_request) + trigger_merge_request_merge_status_updated(merge_request) trigger_merge_request_reviewers_updated(merge_request) trigger_merge_request_approval_state_updated(merge_request) @@ -31,6 +36,17 @@ module MergeRequests private + def fetch_patch_id_sha(merge_request) + diff_refs = merge_request.diff_refs + base_sha = diff_refs&.base_sha + head_sha = diff_refs&.head_sha + + return unless base_sha && head_sha + return if base_sha == head_sha + + merge_request.project.repository.get_patch_id(base_sha, head_sha) + end + def eligible_for_approval?(merge_request) merge_request.eligible_for_approval_by?(current_user) end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 0fc85675e49..f36cad7139a 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -36,10 +36,7 @@ module MergeRequests merge_request.project.execute_integrations(merge_data, :merge_request_hooks) execute_external_hooks(merge_request, merge_data) - - if action == 'open' && Feature.enabled?(:group_mentions, merge_request.project) - execute_group_mention_hooks(merge_request, merge_data) - end + execute_group_mention_hooks(merge_request, merge_data) if action == 'open' enqueue_jira_connect_messages_for(merge_request) end @@ -113,7 +110,7 @@ module MergeRequests # Don't try to print expensive instance variables. def inspect - return "#<#{self.class}>" unless respond_to?(:merge_request) + return "#<#{self.class}>" unless respond_to?(:merge_request) && merge_request "#<#{self.class} #{merge_request.to_reference(full: true)}>" end @@ -176,21 +173,10 @@ module MergeRequests params.delete(:allow_collaboration) end - filter_locked_labels(merge_request) filter_reviewer(merge_request) filter_suggested_reviewers end - # Filter out any locked labels that are requested to be removed. - # Only supported for merged MRs. - def filter_locked_labels(merge_request) - return unless params[:remove_label_ids].present? - return unless merge_request.merged? - return unless Feature.enabled?(:enforce_locked_labels_on_merge, merge_request.project, type: :ops) - - params[:remove_label_ids] -= labels_service.filter_locked_labels_ids_in_param(:remove_label_ids) - end - def filter_reviewer(merge_request) return if params[:reviewer_ids].blank? diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index b8853e8bcbc..bb347096274 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -74,7 +74,7 @@ module MergeRequests # IssuableBaseService#process_label_ids and # IssuableBaseService#process_assignee_ids take care # of the removal. - params[:label_ids] = process_label_ids(params, extra_label_ids: merge_request.label_ids.to_a) + params[:label_ids] = process_label_ids(params, issuable: merge_request, extra_label_ids: merge_request.label_ids.to_a) params[:assignee_ids] = process_assignee_ids(params, extra_assignee_ids: merge_request.assignee_ids.to_a) @@ -130,10 +130,14 @@ module MergeRequests if source_branch_default? && !target_branch_specified? merge_request.target_branch = nil else - merge_request.target_branch ||= target_project.default_branch + merge_request.target_branch ||= get_target_branch end end + def get_target_branch + target_project.default_branch + end + def source_branch_specified? params[:source_branch].present? end diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index e0f10183bac..eae6845335a 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -9,101 +9,117 @@ module MergeRequests CreateRefError = Class.new(StandardError) def initialize( - current_user:, merge_request:, target_ref:, first_parent_ref:, - source_sha: nil, merge_commit_message: nil) - + current_user:, merge_request:, target_ref:, first_parent_ref:, source_sha: nil + ) @current_user = current_user @merge_request = merge_request - @initial_source_sha = source_sha + @source_sha = source_sha @target_ref = target_ref - @merge_commit_message = merge_commit_message + @first_parent_ref = first_parent_ref @first_parent_sha = target_project.commit(first_parent_ref)&.sha end def execute - commit_sha = initial_source_sha # the SHA to be at HEAD of target_ref - source_sha = initial_source_sha # the SHA to be the merged result of the source (minus the merge commit) - expected_old_oid = "" # the SHA we expect target_ref to be at prior to an update (an optimistic lock) - # TODO: Update this message with the removal of FF merge_trains_create_ref_service and update tests # This is for compatibility with MergeToRefService during the rollout. return ServiceResponse.error(message: '3:Invalid merge source') unless first_parent_sha.present? - commit_sha, source_sha, expected_old_oid = maybe_squash!(commit_sha, source_sha, expected_old_oid) - commit_sha, source_sha, expected_old_oid = maybe_rebase!(commit_sha, source_sha, expected_old_oid) - commit_sha, source_sha = maybe_merge!(commit_sha, source_sha, expected_old_oid) - - ServiceResponse.success( - payload: { - commit_sha: commit_sha, - target_sha: first_parent_sha, - source_sha: source_sha - } - ) + result = { + commit_sha: source_sha, # the SHA to be at HEAD of target_ref + expected_old_oid: "", # the SHA we expect target_ref to be at prior to an update (an optimistic lock) + source_sha: source_sha, # for pipeline.source_sha + target_sha: first_parent_sha # for pipeline.target_sha + } + + result = maybe_squash!(**result) + result = maybe_rebase!(**result) + result = maybe_merge!(**result) + + update_merge_request!(merge_request, result) + + ServiceResponse.success(payload: result) rescue CreateRefError => error ServiceResponse.error(message: error.message) end private - attr_reader :current_user, :merge_request, :target_ref, :first_parent_sha, :initial_source_sha + attr_reader :current_user, :merge_request, :target_ref, :first_parent_ref, :first_parent_sha, :source_sha delegate :target_project, to: :merge_request delegate :repository, to: :target_project - def maybe_squash!(commit_sha, source_sha, expected_old_oid) + def maybe_squash!(commit_sha:, **rest) if merge_request.squash_on_merge? squash_result = MergeRequests::SquashService.new( merge_request: merge_request, current_user: current_user, commit_message: squash_commit_message ).execute + raise CreateRefError, squash_result[:message] if squash_result[:status] == :error commit_sha = squash_result[:squash_sha] - source_sha = commit_sha + squash_commit_sha = commit_sha end # squash does not overwrite target_ref, so expected_old_oid remains the same - [commit_sha, source_sha, expected_old_oid] + rest.merge( + commit_sha: commit_sha, + squash_commit_sha: squash_commit_sha + ).compact end - def maybe_rebase!(commit_sha, source_sha, expected_old_oid) + def maybe_rebase!(commit_sha:, expected_old_oid:, squash_commit_sha: nil, **rest) if target_project.ff_merge_must_be_possible? commit_sha = safe_gitaly_operation do repository.rebase_to_ref( current_user, - source_sha: source_sha, + source_sha: commit_sha, target_ref: target_ref, - first_parent_ref: first_parent_sha + first_parent_ref: first_parent_sha, + expected_old_oid: expected_old_oid || "" ) end - source_sha = commit_sha + squash_commit_sha = commit_sha if squash_commit_sha # rebase rewrites commit SHAs after first_parent_sha expected_old_oid = commit_sha end - [commit_sha, source_sha, expected_old_oid] + rest.merge( + commit_sha: commit_sha, + squash_commit_sha: squash_commit_sha, + expected_old_oid: expected_old_oid + ).compact end - def maybe_merge!(commit_sha, source_sha, expected_old_oid) + def maybe_merge!(commit_sha:, expected_old_oid:, **rest) unless target_project.merge_requests_ff_only_enabled commit_sha = safe_gitaly_operation do repository.merge_to_ref( current_user, - source_sha: source_sha, + source_sha: commit_sha, target_ref: target_ref, message: merge_commit_message, first_parent_ref: first_parent_sha, branch: nil, - expected_old_oid: expected_old_oid + expected_old_oid: expected_old_oid || "" ) end - commit = target_project.commit(commit_sha) - _, source_sha = commit.parent_ids + + expected_old_oid = commit_sha + merge_commit_sha = commit_sha end - [commit_sha, source_sha] + rest.merge( + commit_sha: commit_sha, + merge_commit_sha: merge_commit_sha, + expected_old_oid: expected_old_oid + ).compact + end + + def update_merge_request!(merge_request, result) + # overridden in EE end def safe_gitaly_operation @@ -119,12 +135,10 @@ module MergeRequests strong_memoize_attr :squash_commit_message def merge_commit_message - return @merge_commit_message if @merge_commit_message.present? - - @merge_commit_message = ( - merge_request.merge_params['commit_message'].presence || + merge_request.merge_params['commit_message'].presence || merge_request.default_merge_commit_message(user: current_user) - ) end end end + +MergeRequests::CreateRefService.prepend_mod_with('MergeRequests::CreateRefService') diff --git a/app/services/merge_requests/ff_merge_service.rb b/app/services/merge_requests/ff_merge_service.rb deleted file mode 100644 index 1a83bbf9de6..00000000000 --- a/app/services/merge_requests/ff_merge_service.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - # MergeService class - # - # Do git fast-forward merge and in case of success - # mark merge request as merged and execute all hooks and notifications - # Executed when you do fast-forward merge via GitLab UI - # - class FfMergeService < MergeRequests::MergeService - extend ::Gitlab::Utils::Override - - private - - override :execute_git_merge - def execute_git_merge - repository.ff_merge( - current_user, - source, - merge_request.target_branch, - merge_request: merge_request - ) - end - - override :merge_success_data - def merge_success_data(commit_id) - # There is no merge commit to update, so this is just blank. - {} - end - end -end diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb index fa0a4f808e2..0c8795cfd61 100644 --- a/app/services/merge_requests/merge_base_service.rb +++ b/app/services/merge_requests/merge_base_service.rb @@ -18,24 +18,8 @@ module MergeRequests # No-op end - def source - strong_memoize(:source) do - if merge_request.squash_on_merge? - squash_sha! - else - merge_request.diff_head_sha - end - end - end - private - def check_source - unless source - raise_error('No source for merge') - end - end - # Overridden in EE. def check_size_limit # No-op @@ -53,26 +37,6 @@ module MergeRequests def handle_merge_error(*args) # No-op end - - def commit_message - params[:commit_message] || - merge_request.default_merge_commit_message(user: current_user) - end - - def squash_sha! - squash_result = ::MergeRequests::SquashService.new( - merge_request: merge_request, - current_user: current_user, - commit_message: params[:squash_commit_message] - ).execute - - case squash_result[:status] - when :success - squash_result[:squash_sha] - when :error - raise ::MergeRequests::MergeService::MergeError, squash_result[:message] - end - end end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 1398a6dbb67..29aba3c8679 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -16,38 +16,6 @@ module MergeRequests delegate :merge_jid, :state, to: :@merge_request def execute(merge_request, options = {}) - return execute_v2(merge_request, options) if Feature.enabled?(:refactor_merge_service, project) - - if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService) - FfMergeService.new(project: project, current_user: current_user, params: params).execute(merge_request) - return - end - - return if merge_request.merged? - return unless exclusive_lease(merge_request.id).try_obtain - - @merge_request = merge_request - @options = options - jid = merge_jid - - validate! - - merge_request.in_locked_state do - if commit - after_merge - clean_merge_jid - success - end - end - - log_info("Merge process finished on JID #{jid} with state #{state}") - rescue MergeError => e - handle_merge_error(log_message: e.message, save_message_on_model: true) - ensure - exclusive_lease(merge_request.id).cancel - end - - def execute_v2(merge_request, options = {}) return if merge_request.merged? return unless exclusive_lease(merge_request.id).try_obtain @@ -61,7 +29,7 @@ module MergeRequests validate! merge_request.in_locked_state do - if commit_v2 + if commit after_merge clean_merge_jid success @@ -90,28 +58,8 @@ module MergeRequests end end - # Can remove this entire method when :refactor_merge_service is enabled - def error_check! - super - - return if Feature.enabled?(:refactor_merge_service, project) - - check_source - - error = - if @merge_request.should_be_rebased? - 'Only fast-forward merge is allowed for your project. Please update your source branch' - elsif !@merge_request.mergeable?(skip_discussions_check: @options[:skip_discussions_check], check_mergeability_retry_lease: @options[:check_mergeability_retry_lease]) - 'Merge request is not mergeable' - elsif !@merge_request.squash && project.squash_always? - 'This project requires squashing commits when merge requests are accepted.' - end - - raise_error(error) if error - end - def validate_strategy! - @merge_strategy.validate! if Feature.enabled?(:refactor_merge_service, project) + @merge_strategy.validate! end def updated_check! @@ -121,7 +69,7 @@ module MergeRequests end end - def commit_v2 + def commit log_info("Git merge started on JID #{merge_jid}") merge_result = try_merge { @merge_strategy.execute_git_merge! } @@ -131,7 +79,11 @@ module MergeRequests log_info("Git merge finished on JID #{merge_jid} commit #{commit_sha}") - new_merge_request_attributes = merge_result.slice(:merge_commit_sha, :squash_commit_sha) + new_merge_request_attributes = { + merged_commit_sha: commit_sha, + merge_commit_sha: merge_result[:merge_commit_sha], + squash_commit_sha: merge_result[:squash_commit_sha] + }.compact merge_request.update!(new_merge_request_attributes) if new_merge_request_attributes.present? commit_sha @@ -140,35 +92,6 @@ module MergeRequests log_info("Merge request marked in progress") end - def commit - log_info("Git merge started on JID #{merge_jid}") - commit_id = try_merge { execute_git_merge } - - if commit_id - log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") - else - raise_error(GENERIC_ERROR_MESSAGE) - end - - update_merge_sha_metadata(commit_id) - - commit_id - ensure - merge_request.update_and_mark_in_progress_merge_commit_sha(nil) - log_info("Merge request marked in progress") - end - - def update_merge_sha_metadata(commit_id) - data_to_update = merge_success_data(commit_id) - data_to_update[:squash_commit_sha] = source if merge_request.squash_on_merge? - - merge_request.update!(**data_to_update) if data_to_update.present? - end - - def merge_success_data(commit_id) - { merge_commit_sha: commit_id } - end - def try_merge yield rescue Gitlab::Git::PreReceiveError => e @@ -178,10 +101,6 @@ module MergeRequests raise_error(GENERIC_ERROR_MESSAGE) end - def execute_git_merge - repository.merge(current_user, source, merge_request, commit_message) - end - def after_merge log_info("Post merge started on JID #{merge_jid} with state #{state}") MergeRequests::PostMergeService.new(project: project, current_user: current_user).execute(merge_request) diff --git a/app/services/merge_requests/merge_strategies/from_source_branch.rb b/app/services/merge_requests/merge_strategies/from_source_branch.rb index 9fe5fc5160b..fe0e4d8a90c 100644 --- a/app/services/merge_requests/merge_strategies/from_source_branch.rb +++ b/app/services/merge_requests/merge_strategies/from_source_branch.rb @@ -28,7 +28,7 @@ module MergeRequests check_mergeability_retry_lease: @options[:check_mergeability_retry_lease] ) 'Merge request is not mergeable' - elsif !merge_request.squash && project.squash_always? + elsif merge_request.missing_required_squash? 'This project requires squashing commits when merge requests are accepted.' end @@ -110,3 +110,5 @@ module MergeRequests end end end + +::MergeRequests::MergeStrategies::FromSourceBranch.prepend_mod diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index 8b79feb5e0f..6e1b56d9651 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -31,14 +31,13 @@ module MergeRequests private - override :source def source merge_request.diff_head_sha end override :error_check! def error_check! - check_source + raise_error('No source for merge') unless source end ## @@ -55,6 +54,11 @@ module MergeRequests params[:first_parent_ref] || merge_request.target_branch_ref end + def commit_message + params[:commit_message] || + merge_request.default_merge_commit_message(user: current_user) + end + def extracted_merge_to_ref repository.merge_to_ref(current_user, source_sha: source, diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 447f4f9428c..7a7d0dbfef2 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -94,7 +94,9 @@ module MergeRequests ) merge_requests.each do |merge_request| - merge_request.merge_commit_sha = analyzer.get_merge_commit(merge_request.diff_head_sha) + sha = analyzer.get_merge_commit(merge_request.diff_head_sha) + merge_request.merge_commit_sha = sha + merge_request.merged_commit_sha = sha MergeRequests::PostMergeService .new(project: merge_request.target_project, current_user: @current_user) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 598dbaf93a9..c435048e343 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -94,7 +94,7 @@ module MergeRequests end def track_title_and_desc_edits(changed_fields) - tracked_fields = %w(title description) + tracked_fields = %w[title description] return unless changed_fields.any? { |field| tracked_fields.include?(field) } diff --git a/app/services/metrics/global_metrics_update_service.rb b/app/services/metrics/global_metrics_update_service.rb deleted file mode 100644 index 356de58ba2e..00000000000 --- a/app/services/metrics/global_metrics_update_service.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module Metrics - # Update metrics regarding GitLab instance wide - # - # Anything that is not specific to a machine, process, request or any other context - # can be updated from this services. - # - # Examples of metrics that qualify: - # * Global counters (instance users, instance projects...) - # * State of settings stored in the database (whether a feature is active or not, tuning values...) - # - class GlobalMetricsUpdateService - def execute - return unless ::Gitlab::Metrics.prometheus_metrics_enabled? - - maintenance_mode_metric.set({}, (::Gitlab.maintenance_mode? ? 1 : 0)) - end - - def maintenance_mode_metric - ::Gitlab::Metrics.gauge(:gitlab_maintenance_mode, 'Is GitLab Maintenance Mode enabled?') - end - end -end diff --git a/app/services/metrics/sample_metrics_service.rb b/app/services/metrics/sample_metrics_service.rb deleted file mode 100644 index 9bf32b295e2..00000000000 --- a/app/services/metrics/sample_metrics_service.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module Metrics - class SampleMetricsService - DIRECTORY = "sample_metrics" - - attr_reader :identifier, :range_minutes - - def initialize(identifier, range_start:, range_end:) - @identifier = identifier - @range_minutes = convert_range_minutes(range_start, range_end) - end - - def query - return unless identifier && File.exist?(file_location) - - query_interval - end - - private - - def file_location - sanitized_string = identifier.gsub(/[^0-9A-Za-z_]/, '') - File.join(Rails.root, DIRECTORY, "#{sanitized_string}.yml") - end - - def query_interval - result = YAML.load_file(File.expand_path(file_location, __dir__)) - result[range_minutes] - end - - def convert_range_minutes(range_start, range_end) - ((range_end.to_time - range_start.to_time) / 1.minute).to_i - end - end -end diff --git a/app/services/namespaces/in_product_marketing_emails_service.rb b/app/services/namespaces/in_product_marketing_emails_service.rb index 1ce7e4cae16..14e670126c6 100644 --- a/app/services/namespaces/in_product_marketing_emails_service.rb +++ b/app/services/namespaces/in_product_marketing_emails_service.rb @@ -44,108 +44,6 @@ module Namespaces 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| - new(track, interval).execute - end - end - end - - def initialize(track, interval) - @track = track - @interval = interval - @sent_email_records = ::Users::InProductMarketingEmailRecords.new - end - - def execute - raise ArgumentError, "Track #{track} not defined" unless TRACKS.key?(track) - - groups_for_track.each_batch do |groups| - groups.each do |group| - send_email_for_group(group) - end - end - end - - private - - attr_reader :track, :interval, :sent_email_records - - def send_email(user, group) - NotificationService.new.in_product_marketing(user.id, group.id, track, series) - end - - def send_email_for_group(group) - users_for_group(group).each do |user| - if can_perform_action?(user, group) - send_email(user, group) - sent_email_records.add(user, track: track, series: series) - end - end - - sent_email_records.save! - end - - def groups_for_track - onboarding_progress_scope = Onboarding::Progress - .completed_actions_with_latest_in_range(completed_actions, range) - .incomplete_actions(incomplete_actions) - - # Filtering out sub-groups is a temporary fix to prevent calling - # `.root_ancestor` on groups that are not root groups. - # See https://gitlab.com/groups/gitlab-org/-/epics/5594 for more information. - Group - .top_most - .with_onboarding_progress - .merge(onboarding_progress_scope) - .merge(subscription_scope) - end - - def subscription_scope - {} - end - - # rubocop: disable CodeReuse/ActiveRecord - def users_for_group(group) - group.users - .where(email_opted_in: true) - .merge(Users::InProductMarketingEmail.without_track_and_series(track, series)) - end - # rubocop: enable CodeReuse/ActiveRecord - - def can_perform_action?(user, group) - case track - when :create, :verify - user.can?(:create_projects, group) - when :trial, :trial_short - user.can?(:start_trial, group) - when :team, :team_short - user.can?(:admin_group_member, group) - when :admin_verify - user.can?(:admin_group, group) - when :experience - true - end - end - - def completed_actions - TRACKS[track][:completed_actions] - end - - def range - date = (interval + 1).days.ago - date.beginning_of_day..date.end_of_day - end - - def incomplete_actions - TRACKS[track][:incomplete_actions] - end - - def series - TRACKS[track][:interval_days].index(interval) - end end end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index fdab2a07990..1af26377b71 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -187,12 +187,12 @@ module Notes namespace: project&.namespace, user: user, label: metric_key_path, - context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis, key_path: metric_key_path).to_context] + context: [Gitlab::Usage::MetricDefinition.context_for(metric_key_path).to_context] ) end def tracking_data_for(note) - label = Gitlab.ee? && note.author == User.visual_review_bot ? 'anonymous_visual_review_note' : 'note' + label = Gitlab.ee? && note.author == Users::Internal.visual_review_bot ? 'anonymous_visual_review_note' : 'note' { label: label, diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb index 9465b5218b0..6e92a887cdd 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -42,8 +42,6 @@ module Notes note.project.execute_hooks(note_data, hooks_scope) note.project.execute_integrations(note_data, hooks_scope) - return unless Feature.enabled?(:group_mentions, note.project) - execute_group_mention_hooks(note, note_data, is_confidential) end diff --git a/app/services/notification_recipients/builder/base.rb b/app/services/notification_recipients/builder/base.rb index 3fabec29c0d..afbf5747429 100644 --- a/app/services/notification_recipients/builder/base.rb +++ b/app/services/notification_recipients/builder/base.rb @@ -44,6 +44,7 @@ module NotificationRecipients def add_recipients(users, type, reason) if users.is_a?(ActiveRecord::Relation) users = users.includes(:notification_settings) + .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/421821') end users = Array(users).compact diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 648067e3452..f1781b3d3c5 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -75,6 +75,19 @@ class NotificationService end end + def resource_access_tokens_about_to_expire(bot_user, token_names) + recipients = bot_user.resource_bot_owners.select { |owner| owner.can?(:receive_notifications) } + resource = bot_user.resource_bot_resource + + recipients.each do |recipient| + mailer.resource_access_tokens_about_to_expire_email( + recipient, + resource, + token_names + ).deliver_later + end + end + # Notify the owner of the account when a new personal access token is created def access_token_created(user, token_name) return unless user.can?(:receive_notifications) @@ -430,13 +443,14 @@ class NotificationService def send_service_desk_notification(note) return unless note.noteable_type == 'Issue' return if note.confidential + return unless note.project.service_desk_enabled? issue = note.noteable recipients = issue.email_participants_emails return unless recipients.any? - support_bot = User.support_bot + support_bot = Users::Internal.support_bot recipients.delete(issue.external_author) if note.author == support_bot recipients.each do |recipient| @@ -755,10 +769,6 @@ class NotificationService end end - def in_product_marketing(user_id, group_id, track, series) - mailer.in_product_marketing_email(user_id, group_id, track, series).deliver_later - end - def approve_mr(merge_request, current_user) approve_mr_email(merge_request, merge_request.target_project, current_user) end diff --git a/app/services/packages/debian/generate_distribution_service.rb b/app/services/packages/debian/generate_distribution_service.rb index 9feb860ae87..05f6d9af581 100644 --- a/app/services/packages/debian/generate_distribution_service.rb +++ b/app/services/packages/debian/generate_distribution_service.rb @@ -12,7 +12,7 @@ module Packages DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze # From https://salsa.debian.org/ftp-team/dak/-/blob/991aaa27a7f7aa773bb9c0cf2d516e383d9cffa0/setup/core-init.d/080_metadatakeys#L9 - METADATA_KEYS = %w( + METADATA_KEYS = %w[ Package Source Binary @@ -60,7 +60,7 @@ module Packages Tag Package-Type Installer-Menu-Item - ).freeze + ].freeze def initialize(distribution) @distribution = distribution diff --git a/app/services/packages/npm/generate_metadata_service.rb b/app/services/packages/npm/generate_metadata_service.rb index e1795079513..8eaac547f7e 100644 --- a/app/services/packages/npm/generate_metadata_service.rb +++ b/app/services/packages/npm/generate_metadata_service.rb @@ -4,6 +4,7 @@ module Packages module Npm class GenerateMetadataService include API::Helpers::RelatedResourcesHelpers + include Gitlab::Utils::StrongMemoize # Allowed fields are those defined in the abbreviated form # defined here: https://github.com/npm/registry/blob/master/docs/responses/package-metadata.md#abbreviated-version-object @@ -13,6 +14,8 @@ module Packages def initialize(name, packages) @name = name @packages = packages + @dependencies = {} + @dependency_ids = Hash.new { |h, key| h[key] = {} } end def execute(only_dist_tags: false) @@ -21,7 +24,7 @@ module Packages private - attr_reader :name, :packages + attr_reader :name, :packages, :dependencies, :dependency_ids def metadata(only_dist_tags) result = { dist_tags: dist_tags } @@ -38,9 +41,11 @@ module Packages package_versions = {} packages.each_batch do |relation| - batched_packages = relation.including_dependency_links - .preload_files - .preload_npm_metadatum + load_dependencies(relation) + load_dependency_ids(relation) + + batched_packages = relation.preload_files + .preload_npm_metadatum batched_packages.each do |package| package_file = package.installable_package_files.last @@ -82,15 +87,17 @@ module Packages end def build_package_dependencies(package) - dependencies = Hash.new { |h, key| h[key] = {} } - - package.dependency_links.each do |dependency_link| - dependency = dependency_link.dependency - dependencies[dependency_link.dependency_type][dependency.name] = dependency.version_pattern + dependency_ids[package.id].each_with_object(Hash.new { |h, key| h[key] = {} }) do |(type, ids), memo| + ids.each do |id| + memo[inverted_dependency_types[type]].merge!(dependencies[id]) + end end + end - dependencies + def inverted_dependency_types + Packages::DependencyLink.dependency_types.invert.stringify_keys end + strong_memoize_attr :inverted_dependency_types def sorted_versions versions = packages.pluck_versions.compact @@ -106,6 +113,31 @@ module Packages json = package.npm_metadatum&.package_json || {} json.slice(*PACKAGE_JSON_ALLOWED_FIELDS) end + + def load_dependencies(packages) + Packages::Dependency + .id_in( + Packages::DependencyLink + .for_packages(packages) + .select_dependency_id + ) + .id_not_in(dependencies.keys) + .each_batch do |relation| + relation.each do |dependency| + dependencies[dependency.id] = { dependency.name => dependency.version_pattern } + end + end + end + + def load_dependency_ids(packages) + Packages::DependencyLink + .dependency_ids_grouped_by_type(packages) + .each_batch(column: :package_id) do |relation| + relation.each do |dependency_link| + dependency_ids[dependency_link.package_id] = dependency_link.dependency_ids_by_type + end + end + end end end end diff --git a/app/services/packages/nuget/check_duplicates_service.rb b/app/services/packages/nuget/check_duplicates_service.rb new file mode 100644 index 00000000000..7ad9038d7c1 --- /dev/null +++ b/app/services/packages/nuget/check_duplicates_service.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +module Packages + module Nuget + class CheckDuplicatesService < BaseService + include Gitlab::Utils::StrongMemoize + + ExtractionError = Class.new(StandardError) + + def execute + return ServiceResponse.success if package_settings_allow_duplicates? || !target_package_is_duplicate? + + ServiceResponse.error( + message: 'A package with the same name and version already exists', + reason: :conflict + ) + rescue ExtractionError => e + ServiceResponse.error(message: e.message, reason: :bad_request) + end + + private + + def package_settings_allow_duplicates? + package_settings.nuget_duplicates_allowed? || package_settings.class.duplicates_allowed?(existing_package) + end + + def target_package_is_duplicate? + existing_package.name.casecmp(metadata[:package_name]) == 0 && + (existing_package.version.casecmp(metadata[:package_version]) == 0 || + existing_package.normalized_nuget_version&.casecmp(metadata[:package_version]) == 0) + end + + def package_settings + project.namespace.package_settings + end + strong_memoize_attr :package_settings + + def existing_package + ::Packages::Nuget::PackageFinder + .new( + current_user, + project, + package_name: metadata[:package_name], + package_version: metadata[:package_version] + ) + .execute + .first + end + strong_memoize_attr :existing_package + + def metadata + if remote_package_file? + ExtractMetadataContentService + .new(nuspec_file_content) + .execute + .payload + else # to cover the case when package file is on disk not in object storage + MetadataExtractionService + .new(mock_package_file) + .execute + .payload + end + end + strong_memoize_attr :metadata + + def remote_package_file? + params[:remote_url].present? + end + + def nuspec_file_content + ExtractRemoteMetadataFileService + .new(params[:remote_url]) + .execute + .payload + rescue ExtractRemoteMetadataFileService::ExtractionError => e + raise ExtractionError, e.message + end + + def mock_package_file + ::Packages::PackageFile.new( + params + .slice(:file, :file_name) + .merge(package: ::Packages::Package.nuget.build) + ) + end + end + end +end diff --git a/app/services/packages/nuget/extract_metadata_file_service.rb b/app/services/packages/nuget/extract_metadata_file_service.rb index 61e4892fee7..cc040a45016 100644 --- a/app/services/packages/nuget/extract_metadata_file_service.rb +++ b/app/services/packages/nuget/extract_metadata_file_service.rb @@ -3,14 +3,12 @@ module Packages module Nuget class ExtractMetadataFileService - include Gitlab::Utils::StrongMemoize - ExtractionError = Class.new(StandardError) MAX_FILE_SIZE = 4.megabytes.freeze - def initialize(package_file_id) - @package_file_id = package_file_id + def initialize(package_file) + @package_file = package_file end def execute @@ -21,12 +19,7 @@ module Packages private - attr_reader :package_file_id - - def package_file - ::Packages::PackageFile.find_by_id(package_file_id) - end - strong_memoize_attr :package_file + attr_reader :package_file def valid_package_file? package_file && @@ -41,7 +34,7 @@ module Packages raise ExtractionError, 'nuspec file not found' unless entry raise ExtractionError, 'nuspec file too big' if MAX_FILE_SIZE < entry.size - Tempfile.open("nuget_extraction_package_file_#{package_file_id}") do |file| + Tempfile.open("nuget_extraction_package_file_#{package_file.id}") do |file| entry.extract(file.path) { true } # allow #extract to overwrite the file file.unlink file.read diff --git a/app/services/packages/nuget/extract_remote_metadata_file_service.rb b/app/services/packages/nuget/extract_remote_metadata_file_service.rb new file mode 100644 index 00000000000..37624002ce7 --- /dev/null +++ b/app/services/packages/nuget/extract_remote_metadata_file_service.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module Packages + module Nuget + class ExtractRemoteMetadataFileService + include Gitlab::Utils::StrongMemoize + + ExtractionError = Class.new(StandardError) + + MAX_FILE_SIZE = 4.megabytes.freeze + METADATA_FILE_EXTENSION = '.nuspec' + MAX_FRAGMENTS = 5 # nuspec file is usually in the first 2 fragments but we buffer 5 max + + def initialize(remote_url) + @remote_url = remote_url + end + + def execute + raise ExtractionError, 'invalid file url' if remote_url.blank? + + if nuspec_file_content.blank? || !nuspec_file_content.instance_of?(String) + raise ExtractionError, 'nuspec file not found' + end + + ServiceResponse.success(payload: nuspec_file_content) + end + + private + + attr_reader :remote_url + + def nuspec_file_content + fragments = [] + + Gitlab::HTTP.get(remote_url, stream_body: true, allow_object_storage: true) do |fragment| + break if fragments.size >= MAX_FRAGMENTS + + fragments << fragment + joined_fragments = fragments.join + + next if joined_fragments.exclude?(METADATA_FILE_EXTENSION) + + nuspec_content = extract_nuspec_file(joined_fragments) + + break nuspec_content if nuspec_content.present? + end + end + strong_memoize_attr :nuspec_file_content + + def extract_nuspec_file(fragments) + StringIO.open(fragments) do |io| + Zip::InputStream.open(io) do |zip| + process_zip_entries(zip) + end + rescue Zip::Error => e + raise ExtractionError, "Error opening zip stream: #{e.message}" + end + end + + def process_zip_entries(zip) + while (entry = zip.get_next_entry) # rubocop:disable Lint/AssignmentInCondition + next unless entry.name.end_with?(METADATA_FILE_EXTENSION) + + raise ExtractionError, 'nuspec file too big' if entry.size > MAX_FILE_SIZE + + return extract_file_content(entry) + end + end + + def extract_file_content(entry) + Tempfile.create('extract_remote_metadata_file_service') do |file| + entry.extract(file.path) { true } # allow #extract to overwrite the file + file.read + end + rescue Zip::DecompressionError + '' # Ignore decompression errors and continue reading the next fragment + rescue Zip::EntrySizeError => e + raise ExtractionError, "nuspec file has the wrong entry size: #{e.message}" + end + end + end +end diff --git a/app/services/packages/nuget/metadata_extraction_service.rb b/app/services/packages/nuget/metadata_extraction_service.rb index e1ee29ef2c6..2c758a5ec20 100644 --- a/app/services/packages/nuget/metadata_extraction_service.rb +++ b/app/services/packages/nuget/metadata_extraction_service.rb @@ -3,8 +3,8 @@ module Packages module Nuget class MetadataExtractionService - def initialize(package_file_id) - @package_file_id = package_file_id + def initialize(package_file) + @package_file = package_file end def execute @@ -13,18 +13,18 @@ module Packages private - attr_reader :package_file_id + attr_reader :package_file - def nuspec_file_content - ExtractMetadataFileService - .new(package_file_id) + def metadata + ExtractMetadataContentService + .new(nuspec_file_content) .execute .payload end - def metadata - ExtractMetadataContentService - .new(nuspec_file_content) + def nuspec_file_content + ExtractMetadataFileService + .new(package_file) .execute .payload end diff --git a/app/services/packages/nuget/odata_package_entry_service.rb b/app/services/packages/nuget/odata_package_entry_service.rb new file mode 100644 index 00000000000..0cdcc38de16 --- /dev/null +++ b/app/services/packages/nuget/odata_package_entry_service.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Packages + module Nuget + class OdataPackageEntryService + include API::Helpers::RelatedResourcesHelpers + + SEMVER_LATEST_VERSION_PLACEHOLDER = '0.0.0-latest-version' + LATEST_VERSION_FOR_V2_DOWNLOAD_ENDPOINT = 'latest' + + def initialize(project, params) + @project = project + @params = params + end + + def execute + ServiceResponse.success(payload: package_entry) + end + + private + + attr_reader :project, :params + + def package_entry + <<-XML.squish + + #{id_url} + + #{params[:package_name]} + + + #{package_version} + + + XML + end + + def package_version + params[:package_version] || SEMVER_LATEST_VERSION_PLACEHOLDER + end + + def id_url + expose_url "#{api_v4_projects_packages_nuget_v2_path(id: project.id)}" \ + "/Packages(Id='#{params[:package_name]}',Version='#{package_version}')" + end + + # TODO: use path helper when download endpoint is merged + def download_url + expose_url "#{api_v4_projects_packages_nuget_v2_path(id: project.id)}" \ + "/download/#{params[:package_name]}/#{download_url_package_version}" + end + + def download_url_package_version + if latest_version? + LATEST_VERSION_FOR_V2_DOWNLOAD_ENDPOINT + else + params[:package_version] + end + end + + def latest_version? + params[:package_version].nil? || params[:package_version] == SEMVER_LATEST_VERSION_PLACEHOLDER + end + + def xml_base + expose_url api_v4_projects_packages_nuget_v2_path(id: project.id) + end + end + end +end diff --git a/app/services/packages/nuget/update_package_from_metadata_service.rb b/app/services/packages/nuget/update_package_from_metadata_service.rb index 73a52ea569f..258f8c8f6aa 100644 --- a/app/services/packages/nuget/update_package_from_metadata_service.rb +++ b/app/services/packages/nuget/update_package_from_metadata_service.rb @@ -148,7 +148,7 @@ module Packages end def metadata - ::Packages::Nuget::MetadataExtractionService.new(@package_file.id).execute.payload + ::Packages::Nuget::MetadataExtractionService.new(@package_file).execute.payload end strong_memoize_attr :metadata diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index c8ccbe1465e..10aef87332a 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -17,7 +17,7 @@ class PreviewMarkdownService < BaseService private def quick_action_types - %w(Issue MergeRequest Commit WorkItem) + %w[Issue MergeRequest Commit WorkItem] end def explain_quick_actions(text) diff --git a/app/services/projects/apple_target_platform_detector_service.rb b/app/services/projects/apple_target_platform_detector_service.rb index ec4c16a1416..087bb1f22e1 100644 --- a/app/services/projects/apple_target_platform_detector_service.rb +++ b/app/services/projects/apple_target_platform_detector_service.rb @@ -20,7 +20,7 @@ module Projects # > AppleTargetPlatformDetectorService.new(multiplatform_project).execute # => [:ios, :osx, :tvos, :watchos] class AppleTargetPlatformDetectorService < BaseService - BUILD_CONFIG_FILENAMES = %w(project.pbxproj *.xcconfig).freeze + 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 diff --git a/app/services/projects/container_repository/cleanup_tags_base_service.rb b/app/services/projects/container_repository/cleanup_tags_base_service.rb index 45557d03502..61b09de1643 100644 --- a/app/services/projects/container_repository/cleanup_tags_base_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_base_service.rb @@ -100,7 +100,7 @@ module Projects def older_than_in_seconds strong_memoize(:older_than_in_seconds) do - ChronicDuration.parse(older_than).seconds + ChronicDuration.parse(older_than, use_complete_matcher: true).seconds end end end diff --git a/app/services/projects/container_repository/gitlab/delete_tags_service.rb b/app/services/projects/container_repository/gitlab/delete_tags_service.rb index 530cf87c338..a5b7f4bbb6f 100644 --- a/app/services/projects/container_repository/gitlab/delete_tags_service.rb +++ b/app/services/projects/container_repository/gitlab/delete_tags_service.rb @@ -39,7 +39,7 @@ module Projects end end - @deleted_tags.any? ? success(deleted: @deleted_tags) : error('could not delete tags') + @deleted_tags.any? ? success(deleted: @deleted_tags) : error("could not delete tags: #{@tag_names.join(', ')}".truncate(1000)) end end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index bca78b88630..e4987438c57 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -144,6 +144,8 @@ module Projects end def create_project_settings + Gitlab::Pages.add_unique_domain_to(project) + @project.project_setting.save if @project.project_setting.changed? end @@ -223,22 +225,26 @@ module Projects end def save_project_and_import_data - ApplicationRecord.transaction do - @project.create_or_update_import_data(data: @import_data[:data], credentials: @import_data[:credentials]) if @import_data + Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction( + %w[routes redirect_routes], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424281' + ) do + ApplicationRecord.transaction do + @project.create_or_update_import_data(data: @import_data[:data], credentials: @import_data[:credentials]) if @import_data - # Avoid project callbacks being triggered multiple times by saving the parent first. - # See https://github.com/rails/rails/issues/41701. - Namespaces::ProjectNamespace.create_from_project!(@project) if @project.valid? + # Avoid project callbacks being triggered multiple times by saving the parent first. + # See https://github.com/rails/rails/issues/41701. + Namespaces::ProjectNamespace.create_from_project!(@project) if @project.valid? - if @project.saved? - Integration.create_from_active_default_integrations(@project, :project_id) + if @project.saved? + Integration.create_from_active_default_integrations(@project, :project_id) - @project.create_labels unless @project.gitlab_project_import? + @project.create_labels unless @project.gitlab_project_import? - next if @project.import? + next if @project.import? - unless @project.create_repository(default_branch: default_branch) - raise 'Failed to create repository' + unless @project.create_repository(default_branch: default_branch) + raise 'Failed to create repository' + end end end end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 0ae6fcb4d97..a2a2f9d2800 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -255,7 +255,11 @@ module Projects # We need to remove them when a project is deleted # rubocop: disable CodeReuse/ActiveRecord def destroy_project_bots! - project.members.includes(:user).references(:user).merge(User.project_bot).each do |member| + members = project.members + .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/422405') + .includes(:user).references(:user).merge(User.project_bot) + + members.each do |member| Users::DestroyService.new(current_user).execute(member.user, skip_authorization: true) end end diff --git a/app/services/projects/download_service.rb b/app/services/projects/download_service.rb index 22104409199..d67b7832bf8 100644 --- a/app/services/projects/download_service.rb +++ b/app/services/projects/download_service.rb @@ -28,7 +28,7 @@ module Projects end def http?(url) - url =~ /\A#{URI::DEFAULT_PARSER.make_regexp(%w(http https))}\z/ + url =~ /\A#{URI::DEFAULT_PARSER.make_regexp(%w[http https])}\z/ end def valid_domain?(url) diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index 023f8494d99..40c4fd5376c 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -6,7 +6,7 @@ module Projects extend ::Gitlab::Utils::Override # List of paths that can be excluded while evaluation if a target can be discarded - DISCARDABLE_PATHS = %w(tmp tmp/cache tmp/work).freeze + DISCARDABLE_PATHS = %w[tmp tmp/cache tmp/work].freeze def initialize(project:, old_disk_path:, logger: nil) super diff --git a/app/services/projects/import_error_filter.rb b/app/services/projects/import_error_filter.rb index 737b794484d..a0fc5149bb4 100644 --- a/app/services/projects/import_error_filter.rb +++ b/app/services/projects/import_error_filter.rb @@ -4,7 +4,7 @@ module Projects # Used by project imports, it removes any potential paths # included in an error message that could be stored in the DB class ImportErrorFilter - ERROR_MESSAGE_FILTER = /[^\s]*#{File::SEPARATOR}[^\s]*(?=(\s|\z))/.freeze + ERROR_MESSAGE_FILTER = /[^\s]*#{File::SEPARATOR}[^\s]*(?=(\s|\z))/ FILTER_MESSAGE = '[FILTERED]' def self.filter_message(message) diff --git a/app/services/projects/in_product_marketing_campaign_emails_service.rb b/app/services/projects/in_product_marketing_campaign_emails_service.rb index 249a2d89fc1..a549d8f594e 100644 --- a/app/services/projects/in_product_marketing_campaign_emails_service.rb +++ b/app/services/projects/in_product_marketing_campaign_emails_service.rb @@ -26,13 +26,9 @@ module Projects sent_email_records.save! end - # rubocop: disable CodeReuse/ActiveRecord def project_users - @project_users ||= project.users - .where(email_opted_in: true) - .merge(Users::InProductMarketingEmail.without_campaign(campaign)) + @project_users ||= project.users.merge(Users::InProductMarketingEmail.without_campaign(campaign)) end - # rubocop: enable CodeReuse/ActiveRecord def project_users_max_access_levels ids = project_users.map(&:id) diff --git a/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb b/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb index 09fec9939b9..0efe9fb16f6 100644 --- a/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb +++ b/app/services/projects/lfs_pointers/lfs_object_download_list_service.rb @@ -9,7 +9,7 @@ module Projects include Gitlab::Utils::StrongMemoize HEAD_REV = 'HEAD' - LFS_ENDPOINT_PATTERN = /^\t?url\s*=\s*(.+)$/.freeze + LFS_ENDPOINT_PATTERN = /^\t?url\s*=\s*(.+)$/ LFS_BATCH_API_ENDPOINT = '/info/lfs/objects/batch' LfsObjectDownloadListError = Class.new(StandardError) @@ -101,7 +101,7 @@ module Projects # The import url must end with '.git' here we ensure it is def default_endpoint_uri @default_endpoint_uri ||= import_uri.dup.tap do |uri| - path = uri.path.gsub(%r(/$), '') + path = uri.path.gsub(%r{/$}, '') path += '.git' unless path.ends_with?('.git') uri.path = path + LFS_BATCH_API_ENDPOINT end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 642ec37619f..3d08039942b 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -110,36 +110,40 @@ module Projects end def proceed_to_transfer - Project.transaction do - project.expire_caches_before_rename(@old_path) + Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction( + %w[routes redirect_routes], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424282' + ) do + Project.transaction do + project.expire_caches_before_rename(@old_path) - # Apply changes to the project - update_namespace_and_visibility(@new_namespace) - project.reconcile_shared_runners_setting! - project.save! + # Apply changes to the project + update_namespace_and_visibility(@new_namespace) + project.reconcile_shared_runners_setting! + project.save! - # Notifications - project.send_move_instructions(@old_path) + # Notifications + project.send_move_instructions(@old_path) - # Directories on disk - move_project_folders(project) + # Directories on disk + move_project_folders(project) - transfer_missing_group_resources(@old_group) + transfer_missing_group_resources(@old_group) - # Move uploads - move_project_uploads(project) + # Move uploads + move_project_uploads(project) - update_integrations + update_integrations - remove_paid_features + remove_paid_features - project.old_path_with_namespace = @old_path + project.old_path_with_namespace = @old_path - update_repository_configuration(@new_path) + update_repository_configuration(@new_path) - remove_issue_contacts + remove_issue_contacts - execute_system_hooks + execute_system_hooks + end end update_pending_builds diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 403f645392c..dc92c501b8c 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -2,10 +2,12 @@ module Projects class UpdatePagesService < BaseService + include Gitlab::Utils::StrongMemoize + # old deployment can be cached by pages daemon # so we need to give pages daemon some time update cache # 10 minutes is enough, but 30 feels safer - OLD_DEPLOYMENTS_DESTRUCTION_DELAY = 30.minutes.freeze + OLD_DEPLOYMENTS_DESTRUCTION_DELAY = 30.minutes attr_reader :build, :deployment_update @@ -18,9 +20,7 @@ module Projects def execute register_attempt - # Create status notifying the deployment of pages - @commit_status = build_commit_status - ::Ci::Pipelines::AddJobService.new(@build.pipeline).execute!(@commit_status) do |job| + ::Ci::Pipelines::AddJobService.new(@build.pipeline).execute!(commit_status) do |job| job.enqueue! job.run! end @@ -31,13 +31,10 @@ module Projects deployment = create_pages_deployment(artifacts_path, build) break error('The uploaded artifact size does not match the expected value') unless deployment + break error(deployment_update.errors.first.full_message) unless deployment_update.valid? - if deployment_update.valid? - update_project_pages_deployment(deployment) - success - else - error(deployment_update.errors.first.full_message) - end + update_project_pages_deployment(deployment) + success end rescue StandardError => e error(e.message) @@ -47,7 +44,7 @@ module Projects private def success - @commit_status.success + commit_status.success @project.mark_pages_as_deployed publish_deployed_event super @@ -56,15 +53,14 @@ module Projects def error(message) register_failure log_error("Projects::UpdatePagesService: #{message}") - @commit_status.allow_failure = !deployment_update.latest? - @commit_status.description = message - @commit_status.drop(:script_failure) + commit_status.allow_failure = !deployment_update.latest? + commit_status.description = message + commit_status.drop(:script_failure) super end - def build_commit_status - stage = create_stage - + # Create status notifying the deployment of pages + def commit_status GenericCommitStatus.new( user: build.user, ci_stage: stage, @@ -73,26 +69,22 @@ module Projects stage_idx: stage.position ) end + strong_memoize_attr :commit_status # rubocop: disable Performance/ActiveRecordSubtransactionMethods - def create_stage + def stage build.pipeline.stages.safe_find_or_create_by(name: 'deploy', pipeline_id: build.pipeline.id) do |stage| stage.position = GenericCommitStatus::EXTERNAL_STAGE_IDX stage.project = build.project end end + strong_memoize_attr :commit_status # rubocop: enable Performance/ActiveRecordSubtransactionMethods def create_pages_deployment(artifacts_path, build) - sha256 = build.job_artifacts_archive.file_sha256 File.open(artifacts_path) do |file| - deployment = project.pages_deployments.create!( - file: file, - file_count: deployment_update.entries_count, - file_sha256: sha256, - ci_build_id: build.id, - root_directory: build.options[:publish] - ) + attributes = pages_deployment_attributes(file, build) + deployment = project.pages_deployments.create!(**attributes) break if deployment.size != file.size || deployment.file.size != file.size @@ -100,21 +92,28 @@ module Projects end end + # overridden on EE + def pages_deployment_attributes(file, build) + { + file: file, + file_count: deployment_update.entries_count, + file_sha256: build.job_artifacts_archive.file_sha256, + ci_build_id: build.id, + root_directory: build.options[:publish] + } + end + def update_project_pages_deployment(deployment) project.update_pages_deployment!(deployment) + + PagesDeployment.deactivate_deployments_older_than( + deployment, + time: OLD_DEPLOYMENTS_DESTRUCTION_DELAY.from_now) + DestroyPagesDeploymentsWorker.perform_in( OLD_DEPLOYMENTS_DESTRUCTION_DELAY, project.id, - deployment.id - ) - end - - def ref - build.ref - end - - def artifacts - build.artifacts_file.path + deployment.id) end def register_attempt @@ -126,12 +125,14 @@ module Projects end def pages_deployments_total_counter - @pages_deployments_total_counter ||= Gitlab::Metrics.counter(:pages_deployments_total, "Counter of GitLab Pages deployments triggered") + Gitlab::Metrics.counter(:pages_deployments_total, "Counter of GitLab Pages deployments triggered") end + strong_memoize_attr :pages_deployments_total_counter def pages_deployments_failed_total_counter - @pages_deployments_failed_total_counter ||= Gitlab::Metrics.counter(:pages_deployments_failed_total, "Counter of GitLab Pages deployments which failed") + Gitlab::Metrics.counter(:pages_deployments_failed_total, "Counter of GitLab Pages deployments which failed") end + strong_memoize_attr :pages_deployments_failed_total_counter def publish_deployed_event event = ::Pages::PageDeployedEvent.new(data: { @@ -144,3 +145,5 @@ module Projects end end end + +::Projects::UpdatePagesService.prepend_mod diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index f5f6bb85995..799ae5677c3 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -80,8 +80,8 @@ module Projects end def pool_repository_exists_for?(shard_name:, pool_repository:) - PoolRepository.by_source_project_and_shard_name( - pool_repository.source_project, + PoolRepository.by_disk_path_and_shard_name( + pool_repository.disk_path, shard_name ).exists? end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 8639e2f833f..e5e39247dbf 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -53,13 +53,7 @@ module Projects def add_pages_unique_domain return unless params.dig(:project_setting_attributes, :pages_unique_domain_enabled) - # If the project used a unique domain once, it'll always use the same - return if project.project_setting.pages_unique_domain_in_database.present? - - params[:project_setting_attributes][:pages_unique_domain] = Gitlab::Pages::RandomDomain.generate( - project_path: project.path, - namespace_path: project.parent.full_path - ) + Gitlab::Pages.add_unique_domain_to(project) end def validate! @@ -112,6 +106,7 @@ module Projects # overridden by EE module end + # overridden by EE module def remove_unallowed_params params.delete(:emails_enabled) unless can?(current_user, :set_emails_disabled, project) @@ -119,11 +114,11 @@ module Projects end def after_update - todos_features_changes = %w( + todos_features_changes = %w[ issues_access_level merge_requests_access_level repository_access_level - ) + ] project_changed_feature_keys = project.project_feature.previous_changes.keys if project.visibility_level_previous_changes && project.private? diff --git a/app/services/releases/destroy_service.rb b/app/services/releases/destroy_service.rb index ff2b3a7bd18..41b421662ef 100644 --- a/app/services/releases/destroy_service.rb +++ b/app/services/releases/destroy_service.rb @@ -7,6 +7,8 @@ module Releases return error(_('Access Denied'), 403) unless allowed? if release.destroy + update_catalog_resource! + success(tag: existing_tag, release: release) else error(release.errors.messages || '400 Bad request', 400) @@ -15,6 +17,14 @@ module Releases private + def update_catalog_resource! + return unless project.catalog_resource + + return unless project.catalog_resource.versions.none? + + project.catalog_resource.update!(state: 'draft') + end + def allowed? Ability.allowed?(current_user, :destroy_release, release) end diff --git a/app/services/repositories/base_service.rb b/app/services/repositories/base_service.rb index b262b4a1f7b..bf7ac2e5fd8 100644 --- a/app/services/repositories/base_service.rb +++ b/app/services/repositories/base_service.rb @@ -29,7 +29,7 @@ class Repositories::BaseService < BaseService end def move_error(path) - error = %{Repository "#{path}" could not be moved} + error = %(Repository "#{path}" could not be moved) log_error(error) error(error) diff --git a/app/services/repository_archive_clean_up_service.rb b/app/services/repository_archive_clean_up_service.rb index 0fb7dfdb85f..4258898c665 100644 --- a/app/services/repository_archive_clean_up_service.rb +++ b/app/services/repository_archive_clean_up_service.rb @@ -37,7 +37,7 @@ class RepositoryArchiveCleanUpService private def clean_up_old_archives - run(%W(find #{path} -mindepth 1 -maxdepth #{MAX_ARCHIVE_DEPTH} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -mmin +#{mmin} -delete)) + run(%W[find #{path} -mindepth 1 -maxdepth #{MAX_ARCHIVE_DEPTH} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -mmin +#{mmin} -delete]) end def clean_up_empty_directories @@ -45,7 +45,7 @@ class RepositoryArchiveCleanUpService end def clean_up_empty_directories_with_depth(depth) - run(%W(find #{path} -mindepth #{depth} -maxdepth #{depth} -type d -empty -delete)) + run(%W[find #{path} -mindepth #{depth} -maxdepth #{depth} -type d -empty -delete]) end def run(cmd) diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index 1fea894a599..1c496aa5e77 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -60,7 +60,7 @@ module ResourceAccessTokens strong_memoize_attr :username_and_email_generator def has_permission_to_create? - %w(project group).include?(resource_type) && can?(current_user, :create_resource_access_tokens, resource) + %w[project group].include?(resource_type) && can?(current_user, :create_resource_access_tokens, resource) end def create_user diff --git a/app/services/resource_access_tokens/revoke_service.rb b/app/services/resource_access_tokens/revoke_service.rb index 2aaf4cc31d2..46c71b04632 100644 --- a/app/services/resource_access_tokens/revoke_service.rb +++ b/app/services/resource_access_tokens/revoke_service.rb @@ -38,7 +38,7 @@ module ResourceAccessTokens end def can_destroy_token? - %w(project group).include?(resource.class.name.downcase) && can?(current_user, :destroy_resource_access_tokens, resource) + %w[project group].include?(resource.class.name.downcase) && can?(current_user, :destroy_resource_access_tokens, resource) end def find_member diff --git a/app/services/resource_events/base_change_timebox_service.rb b/app/services/resource_events/base_change_timebox_service.rb index ba7c9d90713..d0b0f635ed2 100644 --- a/app/services/resource_events/base_change_timebox_service.rb +++ b/app/services/resource_events/base_change_timebox_service.rb @@ -14,7 +14,7 @@ module ResourceEvents track_event - resource.expire_note_etag_cache + resource.broadcast_notes_changed end private diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb index 69e68922b91..f0ebf7fb40b 100644 --- a/app/services/resource_events/change_labels_service.rb +++ b/app/services/resource_events/change_labels_service.rb @@ -31,7 +31,7 @@ module ResourceEvents end create_timeline_events_from(added_labels: added_labels, removed_labels: removed_labels) - resource.expire_note_etag_cache + resource.broadcast_notes_changed return unless resource.is_a?(Issue) diff --git a/app/services/resource_events/change_state_service.rb b/app/services/resource_events/change_state_service.rb index a396f7a1907..303d666c8e2 100644 --- a/app/services/resource_events/change_state_service.rb +++ b/app/services/resource_events/change_state_service.rb @@ -23,7 +23,7 @@ module ResourceEvents created_at: resource.system_note_timestamp ) - resource.expire_note_etag_cache + resource.broadcast_notes_changed end private diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index f4c0a743ef0..24549b1498b 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -6,7 +6,7 @@ module Search include Gitlab::Utils::StrongMemoize DEFAULT_SCOPE = 'projects' - ALLOWED_SCOPES = %w(projects issues merge_requests milestones users).freeze + ALLOWED_SCOPES = %w[projects issues merge_requests milestones users].freeze attr_accessor :current_user, :params diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 73d46a9ba70..24613dc2564 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -5,7 +5,7 @@ module Search include Search::Filter include Gitlab::Utils::StrongMemoize - ALLOWED_SCOPES = %w(blobs issues merge_requests wiki_blobs commits notes milestones users).freeze + ALLOWED_SCOPES = %w[blobs issues merge_requests wiki_blobs commits notes milestones users].freeze attr_accessor :project, :current_user, :params diff --git a/app/services/service_desk/custom_email_verifications/base_service.rb b/app/services/service_desk/custom_email_verifications/base_service.rb index fe456e4d3f3..e92700022f1 100644 --- a/app/services/service_desk/custom_email_verifications/base_service.rb +++ b/app/services/service_desk/custom_email_verifications/base_service.rb @@ -3,6 +3,8 @@ module ServiceDesk module CustomEmailVerifications class BaseService < ::BaseProjectService + include ::ServiceDesk::CustomEmails::Logger + attr_reader :settings def initialize(project:, current_user: nil, params: {}) @@ -35,15 +37,21 @@ module ServiceDesk end def error_response(message) + log_warning(error_message: message) ServiceResponse.error(message: message) end def error_not_verified(error_identifier) + log_info(error_message: error_identifier.to_s) ServiceResponse.error( message: _('ServiceDesk|Custom email address could not be verified.'), reason: error_identifier.to_s ) end + + def log_category + 'custom_email_verification' + end end end end diff --git a/app/services/service_desk/custom_email_verifications/create_service.rb b/app/services/service_desk/custom_email_verifications/create_service.rb index db518bfdf24..9c5721446a1 100644 --- a/app/services/service_desk/custom_email_verifications/create_service.rb +++ b/app/services/service_desk/custom_email_verifications/create_service.rb @@ -17,6 +17,7 @@ module ServiceDesk if ramp_up_error handle_error_case else + log_info ServiceResponse.success end end @@ -63,11 +64,11 @@ module ServiceDesk end def error_settings_missing - error_response(_('ServiceDesk|Service Desk setting missing')) + error_response(s_('ServiceDesk|Service Desk setting missing')) end def error_user_not_authorized - error_response(_('ServiceDesk|User cannot manage project.')) + error_response(s_('ServiceDesk|User cannot manage project.')) end end end diff --git a/app/services/service_desk/custom_email_verifications/update_service.rb b/app/services/service_desk/custom_email_verifications/update_service.rb index 813624cde23..5ef36ce0576 100644 --- a/app/services/service_desk/custom_email_verifications/update_service.rb +++ b/app/services/service_desk/custom_email_verifications/update_service.rb @@ -24,6 +24,7 @@ module ServiceDesk else verification.mark_as_finished! + log_info ServiceResponse.success end end @@ -75,15 +76,15 @@ module ServiceDesk end def error_parameter_missing - error_response(_('ServiceDesk|Service Desk setting or verification object missing')) + error_response(s_('ServiceDesk|Service Desk setting or verification object missing')) end def error_already_finished - error_response(_('ServiceDesk|Custom email address has already been verified.')) + error_response(s_('ServiceDesk|Custom email address has already been verified.')) end def error_already_failed - error_response(_('ServiceDesk|Custom email address verification has already been processed and failed.')) + error_response(s_('ServiceDesk|Custom email address verification has already been processed and failed.')) end end end diff --git a/app/services/service_desk/custom_emails/base_service.rb b/app/services/service_desk/custom_emails/base_service.rb index 62152f31012..91f4100a8ca 100644 --- a/app/services/service_desk/custom_emails/base_service.rb +++ b/app/services/service_desk/custom_emails/base_service.rb @@ -3,6 +3,8 @@ module ServiceDesk module CustomEmails class BaseService < ::BaseProjectService + include Logger + private def legitimate_user? @@ -34,6 +36,7 @@ module ServiceDesk end def error_response(message) + log_warning(error_message: message) ServiceResponse.error(message: message) end end diff --git a/app/services/service_desk/custom_emails/create_service.rb b/app/services/service_desk/custom_emails/create_service.rb index c3ca98a0259..305f5b3fa11 100644 --- a/app/services/service_desk/custom_emails/create_service.rb +++ b/app/services/service_desk/custom_emails/create_service.rb @@ -25,6 +25,7 @@ module ServiceDesk # we don't use its response here. create_verification + log_info ServiceResponse.success end diff --git a/app/services/service_desk/custom_emails/destroy_service.rb b/app/services/service_desk/custom_emails/destroy_service.rb index 1aa5994edd8..abbe39646aa 100644 --- a/app/services/service_desk/custom_emails/destroy_service.rb +++ b/app/services/service_desk/custom_emails/destroy_service.rb @@ -13,6 +13,7 @@ module ServiceDesk project.reset project.service_desk_setting&.update!(custom_email: nil, custom_email_enabled: false) + log_info ServiceResponse.success end diff --git a/app/services/service_desk_settings/update_service.rb b/app/services/service_desk_settings/update_service.rb index 61cb6fce11f..182022beb1d 100644 --- a/app/services/service_desk_settings/update_service.rb +++ b/app/services/service_desk_settings/update_service.rb @@ -2,12 +2,19 @@ module ServiceDeskSettings class UpdateService < BaseService + include ::ServiceDesk::CustomEmails::Logger + def execute settings = ServiceDeskSetting.safe_find_or_create_by!(project_id: project.id) params[:project_key] = nil if params[:project_key].blank? + # We want to know when custom email got enabled + write_log_message = params[:custom_email_enabled].present? && !settings.custom_email_enabled? + if settings.update(params) + log_info if write_log_message + ServiceResponse.success else ServiceResponse.error(message: settings.errors.full_messages.to_sentence) diff --git a/app/services/service_response.rb b/app/services/service_response.rb index 86efc01bd30..fbc5660315b 100644 --- a/app/services/service_response.rb +++ b/app/services/service_response.rb @@ -2,18 +2,22 @@ class ServiceResponse def self.success(message: nil, payload: {}, http_status: :ok) - new(status: :success, - message: message, - payload: payload, - http_status: http_status) + new( + status: :success, + message: message, + payload: payload, + http_status: http_status + ) end def self.error(message:, payload: {}, http_status: nil, reason: nil) - new(status: :error, - message: message, - payload: payload, - http_status: http_status, - reason: reason) + new( + status: :error, + message: message, + payload: payload, + http_status: http_status, + reason: reason + ) end attr_reader :status, :message, :http_status, :payload, :reason diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 662e31a93aa..8cc6458227f 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -2,7 +2,7 @@ module Snippets class UpdateService < Snippets::BaseService - COMMITTABLE_ATTRIBUTES = %w(file_name content).freeze + COMMITTABLE_ATTRIBUTES = %w[file_name content].freeze UpdateError = Class.new(StandardError) diff --git a/app/services/spam/akismet_service.rb b/app/services/spam/akismet_service.rb index d31b904f549..fd2b3c9a441 100644 --- a/app/services/spam/akismet_service.rb +++ b/app/services/spam/akismet_service.rb @@ -50,8 +50,10 @@ module Spam attr_accessor :owner_name, :owner_email def akismet_client - @akismet_client ||= ::Akismet::Client.new(Gitlab::CurrentSettings.akismet_api_key, - Gitlab.config.gitlab.url) + @akismet_client ||= ::Akismet::Client.new( + Gitlab::CurrentSettings.akismet_api_key, + Gitlab.config.gitlab.url + ) end def akismet_enabled? diff --git a/app/services/spam/spam_action_service.rb b/app/services/spam/spam_action_service.rb index 5c510990b2d..6ec8d09c37c 100644 --- a/app/services/spam/spam_action_service.rb +++ b/app/services/spam/spam_action_service.rb @@ -76,10 +76,9 @@ module Spam spam_verdict_service.execute.tap do |result| case result when BLOCK_USER - # TODO: improve BLOCK_USER handling, non-existent until now - # https://gitlab.com/gitlab-org/gitlab/-/issues/329666 target.spam! create_spam_log + ban_user! when DISALLOW target.spam! create_spam_log @@ -119,6 +118,12 @@ module Spam target.spam_log = spam_log end + def ban_user! + UserCustomAttribute.set_banned_by_spam_log(target.spam_log) + + user.ban! + end + def spam_verdict_service context = { action: action, @@ -131,12 +136,13 @@ module Spam referer: spam_params&.referer } - SpamVerdictService.new(target: target, - user: user, - options: options, - context: context, - extra_features: extra_features - ) + SpamVerdictService.new( + target: target, + user: user, + options: options, + context: context, + extra_features: extra_features + ) end def noteable_type diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb index 639d99ad906..9efe51b43b8 100644 --- a/app/services/spam/spam_verdict_service.rb +++ b/app/services/spam/spam_verdict_service.rb @@ -36,16 +36,17 @@ module Spam # The target can override the verdict via the `allow_possible_spam` application setting final_verdict = OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM if override_via_allow_possible_spam?(verdict: final_verdict) - logger.info(class: self.class.name, - akismet_verdict: akismet_verdict, - spam_check_verdict: spamcheck_verdict, - spam_check_rtt: external_spam_check_round_trip_time.real, - final_verdict: final_verdict, - username: user.username, - user_id: user.id, - target_type: target.class.to_s, - project_id: target.project_id - ) + logger.info( + class: self.class.name, + akismet_verdict: akismet_verdict, + spam_check_verdict: spamcheck_verdict, + spam_check_rtt: external_spam_check_round_trip_time.real, + final_verdict: final_verdict, + username: user.username, + user_id: user.id, + target_type: target.class.to_s, + project_id: target.project_id + ) final_verdict end diff --git a/app/services/submodules/update_service.rb b/app/services/submodules/update_service.rb index a6011a920bd..4a573e595ce 100644 --- a/app/services/submodules/update_service.rb +++ b/app/services/submodules/update_service.rb @@ -26,11 +26,13 @@ module Submodules end def create_commit! - repository.update_submodule(current_user, - @submodule, - @commit_sha, - message: @commit_message, - branch: @branch_name) + repository.update_submodule( + current_user, + @submodule, + @commit_sha, + message: @commit_message, + branch: @branch_name + ) rescue ArgumentError, TypeError raise ValidationError, 'Invalid parameters' end diff --git a/app/services/suggestions/create_service.rb b/app/services/suggestions/create_service.rb index 239cd86e0ec..d7c261f1c25 100644 --- a/app/services/suggestions/create_service.rb +++ b/app/services/suggestions/create_service.rb @@ -9,20 +9,22 @@ module Suggestions def execute return unless @note.supports_suggestion? - suggestions = Gitlab::Diff::SuggestionsParser.parse(@note.note, - project: @note.project, - position: @note.position) + suggestions = Gitlab::Diff::SuggestionsParser.parse( + @note.note, + project: @note.project, + position: @note.position + ) - rows = - suggestions.map.with_index do |suggestion, index| - creation_params = - suggestion.to_hash.slice(:from_content, - :to_content, - :lines_above, - :lines_below) + rows = suggestions.map.with_index do |suggestion, index| + creation_params = suggestion.to_hash.slice( + :from_content, + :to_content, + :lines_above, + :lines_below + ) - creation_params.merge!(note_id: @note.id, relative_order: index) - end + creation_params.merge!(note_id: @note.id, relative_order: index) + end rows.in_groups_of(100, false) do |rows| ApplicationRecord.legacy_bulk_insert('suggestions', rows) # rubocop:disable Gitlab/BulkInsert diff --git a/app/services/system_notes/alert_management_service.rb b/app/services/system_notes/alert_management_service.rb index 994e3174668..1a1dd84491a 100644 --- a/app/services/system_notes/alert_management_service.rb +++ b/app/services/system_notes/alert_management_service.rb @@ -14,7 +14,7 @@ module SystemNotes def create_new_alert(monitoring_tool) body = "logged an alert from **#{monitoring_tool}**" - create_note(NoteSummary.new(noteable, project, User.alert_bot, body, action: 'new_alert_added')) + create_note(NoteSummary.new(noteable, project, Users::Internal.alert_bot, body, action: 'new_alert_added')) end # Called when the status of an AlertManagement::Alert has changed @@ -61,7 +61,7 @@ module SystemNotes def log_resolving_alert(monitoring_tool) body = "logged a recovery alert from **#{monitoring_tool}**" - create_note(NoteSummary.new(noteable, project, User.alert_bot, body, action: 'new_alert_added')) + create_note(NoteSummary.new(noteable, project, Users::Internal.alert_bot, body, action: 'new_alert_added')) end end end diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 61a4316e8ae..04ae734a8fe 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -32,8 +32,7 @@ module SystemNotes # # Returns the created Note object def relate_issuable(noteable_ref) - issuable_type = noteable.to_ability_name.humanize(capitalize: false) - body = "marked this #{issuable_type} as related to #{noteable_ref.to_reference(noteable.resource_parent)}" + body = "marked this #{noteable_name} as related to #{noteable_ref.to_reference(noteable.resource_parent)}" track_issue_event(:track_issue_related_action) @@ -351,12 +350,12 @@ module SystemNotes # Returns the created Note object def change_issue_confidentiality if noteable.confidential - body = 'made the issue confidential' + body = "made the #{noteable_name} confidential" action = 'confidential' track_issue_event(:track_issue_made_confidential_action) else - body = 'made the issue visible to everyone' + body = "made the #{noteable_name} visible to everyone" action = 'visible' track_issue_event(:track_issue_made_visible_action) @@ -534,6 +533,12 @@ module SystemNotes issue_activity_counter.public_send(event_name, author: author, project: project || noteable.project) # rubocop: disable GitlabSecurity/PublicSend end + + def noteable_name + name = noteable.try(:issue_type) || noteable.to_ability_name + + name.humanize(capitalize: false) + end end end diff --git a/app/services/todos/destroy/destroyed_issuable_service.rb b/app/services/todos/destroy/destroyed_issuable_service.rb index 759c430ec7a..6ba286458df 100644 --- a/app/services/todos/destroy/destroyed_issuable_service.rb +++ b/app/services/todos/destroy/destroyed_issuable_service.rb @@ -8,7 +8,7 @@ module Todos # Since we are moving towards work items, in some instances we create todos with # `target_type: WorkItem` in other instances we still create todos with `target_type: Issue` # So when an issue/work item is deleted, we just make sure to delete todos for both target types - BOUND_TARGET_TYPES = %w(Issue WorkItem).freeze + BOUND_TARGET_TYPES = %w[Issue WorkItem].freeze def initialize(target_id, target_type) @target_id = target_id diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 5b04d2fd3af..387c5ce063a 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -8,7 +8,7 @@ module Todos attr_reader :user, :entity def initialize(user_id, entity_id, entity_type) - unless %w(Group Project).include?(entity_type) + unless %w[Group Project].include?(entity_type) raise ArgumentError, "#{entity_type} is not an entity user can leave" end diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb index 24aa4aa1061..b490df6a134 100644 --- a/app/services/users/activity_service.rb +++ b/app/services/users/activity_service.rb @@ -30,11 +30,9 @@ module Users return if Gitlab::Database.read_only? today = Date.today - return if user.last_activity_on == today - lease = Gitlab::ExclusiveLease.new("activity_service:#{user.id}", - timeout: LEASE_TIMEOUT) + lease = Gitlab::ExclusiveLease.new("activity_service:#{user.id}", timeout: LEASE_TIMEOUT) return unless lease.try_obtain user.update_attribute(:last_activity_on, today) diff --git a/app/services/users/authorized_build_service.rb b/app/services/users/authorized_build_service.rb index 5029105b087..446c897fe5a 100644 --- a/app/services/users/authorized_build_service.rb +++ b/app/services/users/authorized_build_service.rb @@ -12,7 +12,7 @@ module Users end def signup_params - super + [:skip_confirmation] + super + [:skip_confirmation, :external] end end end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 04a11f41eb1..b51684c6899 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -5,8 +5,8 @@ module Users ALLOWED_USER_TYPES = %i[project_bot security_policy_bot].freeze delegate :user_default_internal_regex_enabled?, - :user_default_internal_regex_instance, - to: :'Gitlab::CurrentSettings.current_application_settings' + :user_default_internal_regex_instance, + to: :'Gitlab::CurrentSettings.current_application_settings' def initialize(current_user, params = {}) @current_user = current_user diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index d4c00a4dcec..a0e1167836b 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -59,9 +59,6 @@ module Users Groups::DestroyService.new(group, current_user).execute end - namespace = user.namespace - namespace.prepare_for_destroy - user.personal_projects.each do |project| success = ::Projects::DestroyService.new(project, current_user).execute raise DestroyError, "Project #{project.id} can't be deleted" unless success @@ -70,9 +67,11 @@ module Users yield(user) if block_given? hard_delete = options.fetch(:hard_delete, false) - Users::GhostUserMigration.create!(user: user, - initiator_user: current_user, - hard_delete: hard_delete) + Users::GhostUserMigration.create!( + user: user, + initiator_user: current_user, + hard_delete: hard_delete + ) update_metrics end diff --git a/app/services/users/migrate_records_to_ghost_user_in_batches_service.rb b/app/services/users/migrate_records_to_ghost_user_in_batches_service.rb index d294312cc30..e05f308343d 100644 --- a/app/services/users/migrate_records_to_ghost_user_in_batches_service.rb +++ b/app/services/users/migrate_records_to_ghost_user_in_batches_service.rb @@ -12,9 +12,11 @@ module Users ghost_user_migrations.each do |job| break if execution_tracker.over_limit? - service = Users::MigrateRecordsToGhostUserService.new(job.user, - job.initiator_user, - execution_tracker) + service = Users::MigrateRecordsToGhostUserService.new( + job.user, + job.initiator_user, + execution_tracker + ) service.execute(hard_delete: job.hard_delete) rescue Gitlab::Utils::ExecutionTracker::ExecutionTimeOutError # no-op diff --git a/app/services/users/migrate_records_to_ghost_user_service.rb b/app/services/users/migrate_records_to_ghost_user_service.rb index 5d518803315..06950292fea 100644 --- a/app/services/users/migrate_records_to_ghost_user_service.rb +++ b/app/services/users/migrate_records_to_ghost_user_service.rb @@ -18,7 +18,7 @@ module Users @user = user @initiator_user = initiator_user @execution_tracker = execution_tracker - @ghost_user = User.ghost + @ghost_user = Users::Internal.ghost end def execute(hard_delete: false) diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 197260a80ca..32acc3f170d 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -82,15 +82,17 @@ module Users attr_reader :incorrect_auth_found_callback, :missing_auth_found_callback def log_refresh_details(remove, add) - Gitlab::AppJsonLogger.info(event: 'authorized_projects_refresh', - user_id: user.id, - 'authorized_projects_refresh.source': source, - 'authorized_projects_refresh.rows_deleted_count': remove.length, - 'authorized_projects_refresh.rows_added_count': add.length, - # 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).map(&:values)) + Gitlab::AppJsonLogger.info( + event: 'authorized_projects_refresh', + user_id: user.id, + 'authorized_projects_refresh.source': source, + 'authorized_projects_refresh.rows_deleted_count': remove.length, + 'authorized_projects_refresh.rows_added_count': add.length, + # 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).map(&:values) + ) end end end diff --git a/app/services/users/upsert_credit_card_validation_service.rb b/app/services/users/upsert_credit_card_validation_service.rb index 61cf598f178..62df676db25 100644 --- a/app/services/users/upsert_credit_card_validation_service.rb +++ b/app/services/users/upsert_credit_card_validation_service.rb @@ -7,8 +7,10 @@ module Users end def execute + user_id = params.fetch(:user_id) + @params = { - user_id: params.fetch(:user_id), + user_id: user_id, credit_card_validated_at: params.fetch(:credit_card_validated_at), expiration_date: get_expiration_date(params), last_digits: Integer(params.fetch(:credit_card_mask_number), 10), @@ -16,7 +18,9 @@ module Users holder_name: params.fetch(:credit_card_holder_name) } - ::Users::CreditCardValidation.upsert(@params) + credit_card = Users::CreditCardValidation.find_or_initialize_by_user(user_id) + + credit_card.update(@params.except(:user_id)) ServiceResponse.success(message: 'CreditCardValidation was set') rescue ActiveRecord::InvalidForeignKey, ActiveRecord::NotNullViolation => e diff --git a/app/services/webauthn/authenticate_service.rb b/app/services/webauthn/authenticate_service.rb index 52437a77df8..7855b509595 100644 --- a/app/services/webauthn/authenticate_service.rb +++ b/app/services/webauthn/authenticate_service.rb @@ -40,8 +40,8 @@ module Webauthn # (which is done in #verify_webauthn_credential) def validate_webauthn_credential(webauthn_credential) webauthn_credential.type == WebAuthn::TYPE_PUBLIC_KEY && - webauthn_credential.raw_id && webauthn_credential.id && - webauthn_credential.raw_id == WebAuthn.standard_encoder.decode(webauthn_credential.id) + webauthn_credential.raw_id && webauthn_credential.id && + webauthn_credential.raw_id == WebAuthn.standard_encoder.decode(webauthn_credential.id) end ## @@ -53,9 +53,10 @@ module Webauthn rp_id = webauthn_credential.client_extension_outputs['appid'] ? WebAuthn.configuration.origin : URI(WebAuthn.configuration.origin).host webauthn_credential.response.verify( encoder.decode(challenge), - public_key: encoder.decode(stored_credential.public_key), - sign_count: stored_credential.counter, - rp_id: rp_id) + public_key: encoder.decode(stored_credential.public_key), + sign_count: stored_credential.counter, + rp_id: rp_id + ) end end end diff --git a/app/services/work_items/callbacks/award_emoji.rb b/app/services/work_items/callbacks/award_emoji.rb index 6344813d4b9..9ff5b6d049d 100644 --- a/app/services/work_items/callbacks/award_emoji.rb +++ b/app/services/work_items/callbacks/award_emoji.rb @@ -15,7 +15,8 @@ module WorkItems def execute_emoji_service(action, name) class_name = { add: ::AwardEmojis::AddService, - remove: ::AwardEmojis::DestroyService + remove: ::AwardEmojis::DestroyService, + toggle: ::AwardEmojis::ToggleService } raise_error(invalid_action_error(action)) unless class_name.key?(action) diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index 903736cf662..354a33a0384 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -32,8 +32,11 @@ module WorkItems end def before_create(work_item) - execute_widgets(work_item: work_item, callback: :before_create_callback, - widget_params: @widget_params) + execute_widgets( + work_item: work_item, + callback: :before_create_callback, + widget_params: @widget_params + ) super end @@ -41,8 +44,11 @@ module WorkItems def transaction_create(work_item) super.tap do |save_result| if save_result - execute_widgets(work_item: work_item, callback: :after_create_in_transaction, - widget_params: @widget_params) + execute_widgets( + work_item: work_item, + callback: :after_create_in_transaction, + widget_params: @widget_params + ) end end end diff --git a/app/services/work_items/related_work_item_links/create_service.rb b/app/services/work_items/related_work_item_links/create_service.rb index 6a9ddd5c83d..f313881470a 100644 --- a/app/services/work_items/related_work_item_links/create_service.rb +++ b/app/services/work_items/related_work_item_links/create_service.rb @@ -25,7 +25,7 @@ module WorkItems end def previous_related_issuables - @related_issues ||= issuable.related_issues(current_user).to_a + @related_issues ||= issuable.linked_work_items(authorize: false).to_a end private diff --git a/app/services/work_items/related_work_item_links/destroy_service.rb b/app/services/work_items/related_work_item_links/destroy_service.rb new file mode 100644 index 00000000000..6d1920d01b2 --- /dev/null +++ b/app/services/work_items/related_work_item_links/destroy_service.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module WorkItems + module RelatedWorkItemLinks + class DestroyService < BaseService + def initialize(work_item, user, params) + @work_item = work_item + @current_user = user + @params = params.dup + @failed_ids = [] + @removed_ids = [] + end + + def execute + return error(_('No work item found.'), 403) unless can?(current_user, :admin_work_item_link, work_item) + return error(_('No work item IDs provided.'), 409) if params[:item_ids].empty? + + destroy_links_for(params[:item_ids]) + + if removed_ids.any? + success(message: response_message, items_removed: removed_ids, items_with_errors: failed_ids.flatten) + else + error(error_message) + end + end + + private + + attr_reader :work_item, :current_user, :failed_ids, :removed_ids + + def destroy_links_for(item_ids) + destroy_links(source: work_item, target: item_ids, direction: :target) + destroy_links(source: item_ids, target: work_item, direction: :source) + end + + def destroy_links(source:, target:, direction:) + WorkItems::RelatedWorkItemLink.for_source_and_target(source, target).each do |link| + linked_item = link.try(direction) + + if can?(current_user, :admin_work_item_link, linked_item) + link.destroy! + removed_ids << linked_item.id + create_notes(link) + else + failed_ids << linked_item.id + end + end + end + + def create_notes(link) + SystemNoteService.unrelate_issuable(link.source, link.target, current_user) + SystemNoteService.unrelate_issuable(link.target, link.source, current_user) + end + + def error_message + not_linked = params[:item_ids] - (removed_ids + failed_ids) + error_messages = [] + + if failed_ids.any? + error_messages << format( + _('%{item_ids} could not be removed due to insufficient permissions'), item_ids: failed_ids.to_sentence + ) + end + + if not_linked.any? + error_messages << format( + _('%{item_ids} could not be removed due to not being linked'), item_ids: not_linked.to_sentence + ) + end + + return '' unless error_messages.any? + + format(_('IDs with errors: %{error_messages}.'), error_messages: error_messages.join(', ')) + end + + def response_message + success_message = format(_('Successfully unlinked IDs: %{item_ids}.'), item_ids: removed_ids.to_sentence) + + return success_message unless error_message.present? + + "#{success_message} #{error_message}" + end + end + end +end -- cgit v1.2.3