diff options
Diffstat (limited to 'app/services')
141 files changed, 1812 insertions, 706 deletions
diff --git a/app/services/audit_events/build_service.rb b/app/services/audit_events/build_service.rb new file mode 100644 index 00000000000..f5322fa5ff4 --- /dev/null +++ b/app/services/audit_events/build_service.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module AuditEvents + class BuildService + # Handle missing attributes + MissingAttributeError = Class.new(StandardError) + + # @raise [MissingAttributeError] when required attributes are blank + # + # @return [BuildService] + def initialize( + author:, scope:, target:, message:, + created_at: DateTime.current, additional_details: {}, ip_address: nil, target_details: nil) + raise MissingAttributeError if missing_attribute?(author, scope, target, message) + + @author = build_author(author) + @scope = scope + @target = build_target(target) + @ip_address = ip_address || build_ip_address + @message = build_message(message) + @created_at = created_at + @additional_details = additional_details + @target_details = target_details + end + + # Create an instance of AuditEvent + # + # @return [AuditEvent] + def execute + AuditEvent.new(payload) + end + + private + + def missing_attribute?(author, scope, target, message) + author.blank? || scope.blank? || target.blank? || message.blank? + end + + def payload + base_payload.merge(details: base_details_payload) + end + + def base_payload + { + author_id: @author.id, + author_name: @author.name, + entity_id: @scope.id, + entity_type: @scope.class.name, + created_at: @created_at + } + end + + def base_details_payload + @additional_details.merge({ + author_name: @author.name, + author_class: @author.class.name, + target_id: @target.id, + target_type: @target.type, + target_details: @target_details || @target.details, + custom_message: @message + }) + end + + def build_author(author) + author.id = -2 if author.instance_of? DeployToken + author.id = -3 if author.instance_of? DeployKey + + author + end + + def build_target(target) + return target if target.is_a? ::Gitlab::Audit::NullTarget + + ::Gitlab::Audit::Target.new(target) + end + + def build_message(message) + message + end + + def build_ip_address + Gitlab::RequestContext.instance.client_ip || @author.current_sign_in_ip + end + end +end + +AuditEvents::BuildService.prepend_mod_with('AuditEvents::BuildService') diff --git a/app/services/authorized_project_update/project_recalculate_service.rb b/app/services/authorized_project_update/project_recalculate_service.rb index 17ba48cffcd..e0b8158417c 100644 --- a/app/services/authorized_project_update/project_recalculate_service.rb +++ b/app/services/authorized_project_update/project_recalculate_service.rb @@ -47,7 +47,7 @@ module AuthorizedProjectUpdate def user_ids_to_remove strong_memoize(:user_ids_to_remove) do (current_authorizations - fresh_authorizations) - .map {|user_id, _| user_id } + .map { |user_id, _| user_id } end end diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index 9e49bd86ec0..1660ddb934f 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -59,6 +59,7 @@ module AutoMerge !merge_request.broken? && !merge_request.draft? && merge_request.mergeable_discussions_state? && + !merge_request.merge_blocked_by_other_mrs? && yield end end diff --git a/app/services/base_count_service.rb b/app/services/base_count_service.rb index ff1949ce4dd..eff3eb33c71 100644 --- a/app/services/base_count_service.rb +++ b/app/services/base_count_service.rb @@ -45,7 +45,7 @@ class BaseCountService end def update_cache_for_key(key, &block) - Rails.cache.write(key, block_given? ? yield : uncached_count, raw: raw?) + Rails.cache.write(key, block ? yield : uncached_count, raw: raw?) end end diff --git a/app/services/boards/destroy_service.rb b/app/services/boards/destroy_service.rb index 0b1cd61b119..ceda005044e 100644 --- a/app/services/boards/destroy_service.rb +++ b/app/services/boards/destroy_service.rb @@ -3,10 +3,6 @@ module Boards class DestroyService < Boards::BaseService def execute(board) - if boards.size == 1 - return ServiceResponse.error(message: "The board could not be deleted, because the parent doesn't have any other boards.") - end - board.destroy! ServiceResponse.success diff --git a/app/services/boards/lists/move_service.rb b/app/services/boards/lists/move_service.rb index 93f81837d1a..4bb7b4dbc6d 100644 --- a/app/services/boards/lists/move_service.rb +++ b/app/services/boards/lists/move_service.rb @@ -23,7 +23,7 @@ module Boards def valid_move? new_position.present? && new_position != old_position && - new_position >= 0 && new_position < board.lists.movable.size + new_position >= 0 && new_position <= board.lists.movable.last.position end def reorder_intermediate_lists diff --git a/app/services/branches/create_service.rb b/app/services/branches/create_service.rb index 7300b31e3b3..5cbd587e546 100644 --- a/app/services/branches/create_service.rb +++ b/app/services/branches/create_service.rb @@ -2,35 +2,91 @@ module Branches class CreateService < BaseService + def initialize(project, user = nil, params = {}) + super(project, user, params) + + @errors = [] + end + def execute(branch_name, ref, create_default_branch_if_empty: true) create_default_branch if create_default_branch_if_empty && project.empty_repo? - result = ::Branches::ValidateNewService.new(project).execute(branch_name) + result = branch_validation_service.execute(branch_name) return result if result[:status] == :error - begin - new_branch = repository.add_branch(current_user, branch_name, ref) - rescue Gitlab::Git::CommandError => e - return error("Failed to create branch '#{branch_name}': #{e}") + create_branch(branch_name, ref) + end + + def bulk_create(branches) + reset_errors + + created_branches = + branches + .then { |branches| only_valid_branches(branches) } + .then { |branches| create_branches(branches) } + .then { |branches| expire_branches_cache(branches) } + + return error(errors) if errors.present? + + success(branches: created_branches) + end + + private + + attr_reader :errors + + def reset_errors + @errors = [] + end + + def only_valid_branches(branches) + branches.select do |branch_name, _ref| + result = branch_validation_service.execute(branch_name) + + if result[:status] == :error + errors << result[:message] + next + end + + true end + end + + def create_branches(branches) + branches.filter_map do |branch_name, ref| + result = create_branch(branch_name, ref, expire_cache: false) + + if result[:status] == :error + errors << result[:message] + next + end + + result[:branch] + end + end + + def expire_branches_cache(branches) + repository.expire_branches_cache if branches.present? + + branches + end + + def create_branch(branch_name, ref, expire_cache: true) + new_branch = repository.add_branch(current_user, branch_name, ref, expire_cache: expire_cache) if new_branch - success(new_branch) + success(branch: new_branch) else error("Failed to create branch '#{branch_name}': invalid reference name '#{ref}'") end + rescue Gitlab::Git::CommandError => e + error("Failed to create branch '#{branch_name}': #{e}") rescue Gitlab::Git::PreReceiveError => e Gitlab::ErrorTracking.log_exception(e, pre_receive_message: e.raw_message, branch_name: branch_name, ref: ref) error(e.message) end - def success(branch) - super().merge(branch: branch) - end - - private - def create_default_branch project.repository.create_file( current_user, @@ -40,5 +96,9 @@ module Branches branch_name: project.default_branch_or_main ) end + + def branch_validation_service + @branch_validation_service ||= ::Branches::ValidateNewService.new(project) + end end end diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index cbf2b34b33c..31e1a822e78 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -64,7 +64,7 @@ module BulkImports bulk_import: bulk_import, source_type: entity[:source_type], source_full_path: entity[:source_full_path], - destination_name: entity[:destination_name], + destination_slug: entity[:destination_slug], destination_namespace: entity[:destination_namespace] ) end diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb index 8d6ba54cd50..a2c8ba5b1cd 100644 --- a/app/services/bulk_imports/file_download_service.rb +++ b/app/services/bulk_imports/file_download_service.rb @@ -55,12 +55,17 @@ module BulkImports bytes_downloaded = 0 http_client.stream(relative_url) do |chunk| + next if bytes_downloaded == 0 && [301, 302, 303, 307, 308].include?(chunk.code) + bytes_downloaded += chunk.size validate_size!(bytes_downloaded) - raise(ServiceError, "File download error #{chunk.code}") unless chunk.code == 200 - file.write(chunk) + if chunk.code == 200 + file.write(chunk) + else + raise(ServiceError, "File download error #{chunk.code}") + end end end rescue StandardError => e diff --git a/app/services/chat_names/authorize_user_service.rb b/app/services/chat_names/authorize_user_service.rb index f7780488923..6c28a1cea7e 100644 --- a/app/services/chat_names/authorize_user_service.rb +++ b/app/services/chat_names/authorize_user_service.rb @@ -4,8 +4,8 @@ module ChatNames class AuthorizeUserService include Gitlab::Routing - def initialize(service, params) - @service = service + def initialize(integration, params) + @integration = integration @params = params end @@ -29,11 +29,11 @@ module ChatNames def chat_name_params { - service_id: @service.id, - team_id: @params[:team_id], + integration_id: @integration.id, + team_id: @params[:team_id], team_domain: @params[:team_domain], - chat_id: @params[:user_id], - chat_name: @params[:user_name] + chat_id: @params[:user_id], + chat_name: @params[:user_name] } end end diff --git a/app/services/ci/archive_trace_service.rb b/app/services/ci/archive_trace_service.rb index 7b1d2207460..9705a236d98 100644 --- a/app/services/ci/archive_trace_service.rb +++ b/app/services/ci/archive_trace_service.rb @@ -62,8 +62,8 @@ module Ci failed_archive_counter.increment Sidekiq.logger.warn(class: worker_name, - message: "Failed to archive trace. message: #{error.message}.", - job_id: job.id) + message: "Failed to archive trace. message: #{error.message}.", + job_id: job.id) Gitlab::ErrorTracking .track_and_raise_for_dev_exception(error, diff --git a/app/services/ci/deployments/destroy_service.rb b/app/services/ci/deployments/destroy_service.rb new file mode 100644 index 00000000000..ac51fa55537 --- /dev/null +++ b/app/services/ci/deployments/destroy_service.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Ci + module Deployments + class DestroyService < BaseService + def execute(deployment) + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_deployment, deployment) + + return ServiceResponse.error(message: 'Cannot destroy running deployment') if deployment&.running? + return ServiceResponse.error(message: 'Deployment currently deployed to environment') if deployment&.last? + + project.destroy_deployment_by_id(deployment) + + ServiceResponse.success(message: 'Deployment destroyed') + end + end + end +end diff --git a/app/services/ci/destroy_pipeline_service.rb b/app/services/ci/destroy_pipeline_service.rb index d85e52e1312..1c563396162 100644 --- a/app/services/ci/destroy_pipeline_service.rb +++ b/app/services/ci/destroy_pipeline_service.rb @@ -7,7 +7,7 @@ module Ci Ci::ExpirePipelineCacheService.new.execute(pipeline, delete: true) - pipeline.cancel_running if pipeline.cancelable? + pipeline.cancel_running(cascade_to_children: true, execute_async: false) if pipeline.cancelable? # The pipeline, the builds, job and pipeline artifacts all get destroyed here. # Ci::Pipeline#destroy triggers fast destroy on job_artifacts and diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index 05f8e804c67..af56eb221d5 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -126,6 +126,8 @@ module Ci job.update_column(:artifacts_expire_at, artifact.expire_at) end + Gitlab::Ci::Artifacts::Logger.log_created(artifact) + success(artifact: artifact) rescue ActiveRecord::RecordNotUnique => error track_exception(error, params) diff --git a/app/services/ci/job_artifacts/destroy_batch_service.rb b/app/services/ci/job_artifacts/destroy_batch_service.rb index 9d6b413ce59..54ec2c671c6 100644 --- a/app/services/ci/job_artifacts/destroy_batch_service.rb +++ b/app/services/ci/job_artifacts/destroy_batch_service.rb @@ -53,8 +53,10 @@ module Ci update_project_statistics! if update_stats increment_monitoring_statistics(artifacts_count, artifacts_bytes) + Gitlab::Ci::Artifacts::Logger.log_deleted(@job_artifacts, 'Ci::JobArtifacts::DestroyBatchService#execute') + success(destroyed_artifacts_count: artifacts_count, - statistics_updates: affected_project_statistics) + statistics_updates: affected_project_statistics) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index 88dac514bb9..c791a89b804 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -26,8 +26,8 @@ module Ci return {} unless config result = Gitlab::Ci::YamlProcessor.new(config, project: project, - user: current_user, - sha: sha).execute + user: current_user, + sha: sha).execute result.valid? ? result.variables_with_data : {} end diff --git a/app/services/ci/parse_dotenv_artifact_service.rb b/app/services/ci/parse_dotenv_artifact_service.rb index 40e2cd82b4f..fd13ed245cf 100644 --- a/app/services/ci/parse_dotenv_artifact_service.rb +++ b/app/services/ci/parse_dotenv_artifact_service.rb @@ -40,7 +40,7 @@ module Ci key, value = scan_line!(line) variables[key] = Ci::JobVariable.new(job_id: artifact.job_id, - source: :dotenv, key: key, value: value) + source: :dotenv, key: key, value: value) end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 8969b95b81f..b357855db12 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -4,6 +4,8 @@ module Ci # This class responsible for assigning # proper pending build to runner on runner API request class RegisterJobService + include ::Gitlab::Ci::Artifacts::Logger + attr_reader :runner, :metrics TEMPORARY_LOCK_TIMEOUT = 3.seconds @@ -220,10 +222,26 @@ module Ci # We need to use the presenter here because Gitaly calls in the presenter # may fail, and we need to ensure the response has been generated. presented_build = ::Ci::BuildRunnerPresenter.new(build) # rubocop:disable CodeReuse/Presenter + + log_artifacts_context(build) + log_build_dependencies_size(presented_build) + build_json = ::API::Entities::Ci::JobRequest::Response.new(presented_build).to_json Result.new(build, build_json, true) end + def log_build_dependencies_size(presented_build) + return unless ::Feature.enabled?(:ci_build_dependencies_artifacts_logger, type: :ops) + + presented_build.all_dependencies.then do |dependencies| + size = dependencies.sum do |build| + build.available_artifacts? ? build.artifacts_file.size : 0 + end + + log_build_dependencies(size: size, count: dependencies.size) if size > 0 + end + end + def assign_runner!(build, params) build.runner_id = runner.id build.runner_session_attributes = params[:session] if params[:session].present? diff --git a/app/services/ci/retry_job_service.rb b/app/services/ci/retry_job_service.rb index e0ced3d0197..25bda8a6380 100644 --- a/app/services/ci/retry_job_service.rb +++ b/app/services/ci/retry_job_service.rb @@ -4,10 +4,10 @@ module Ci class RetryJobService < ::BaseService include Gitlab::Utils::StrongMemoize - def execute(job) + def execute(job, variables: []) if job.retryable? job.ensure_scheduling_type! - new_job = retry_job(job) + new_job = retry_job(job, variables: variables) ServiceResponse.success(payload: { job: new_job }) else @@ -19,7 +19,7 @@ module Ci end # rubocop: disable CodeReuse/ActiveRecord - def clone!(job) + def clone!(job, variables: []) # Cloning a job requires a strict type check to ensure # the attributes being used for the clone are taken straight # from the model and not overridden by other abstractions. @@ -27,7 +27,7 @@ module Ci check_access!(job) - new_job = job.clone(current_user: current_user) + new_job = job.clone(current_user: current_user, new_job_variables_attributes: variables) new_job.run_after_commit do ::Ci::CopyCrossDatabaseAssociationsService.new.execute(job, new_job) @@ -55,8 +55,8 @@ module Ci def check_assignable_runners!(job); end - def retry_job(job) - clone!(job).tap do |new_job| + def retry_job(job, variables: []) + clone!(job, variables: variables).tap do |new_job| check_assignable_runners!(new_job) if new_job.is_a?(Ci::Build) next if new_job.failed? diff --git a/app/services/ci/runners/assign_runner_service.rb b/app/services/ci/runners/assign_runner_service.rb index 886cd3a4e44..290f945cc72 100644 --- a/app/services/ci/runners/assign_runner_service.rb +++ b/app/services/ci/runners/assign_runner_service.rb @@ -13,9 +13,15 @@ module Ci end def execute - return false unless @user.present? && @user.can?(:assign_runner, @runner) + unless @user.present? && @user.can?(:assign_runner, @runner) + return ServiceResponse.error(message: 'user not allowed to assign runner', http_status: :forbidden) + end - @runner.assign_to(@project, @user) + if @runner.assign_to(@project, @user) + ServiceResponse.success + else + ServiceResponse.error(message: 'failed to assign runner') + end end private diff --git a/app/services/ci/runners/bulk_delete_runners_service.rb b/app/services/ci/runners/bulk_delete_runners_service.rb new file mode 100644 index 00000000000..ce07aa541c2 --- /dev/null +++ b/app/services/ci/runners/bulk_delete_runners_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Ci + module Runners + class BulkDeleteRunnersService + attr_reader :runners + + RUNNER_LIMIT = 50 + + # @param runners [Array<Ci::Runner, Integer>] the runners to unregister/destroy + def initialize(runners:) + @runners = runners + end + + def execute + if @runners + # Delete a few runners immediately + return ServiceResponse.success(payload: delete_runners) + end + + ServiceResponse.success(payload: { deleted_count: 0, deleted_ids: [] }) + end + + private + + def delete_runners + # rubocop:disable CodeReuse/ActiveRecord + runners_to_be_deleted = Ci::Runner.where(id: @runners).limit(RUNNER_LIMIT) + # rubocop:enable CodeReuse/ActiveRecord + deleted_ids = runners_to_be_deleted.destroy_all.map(&:id) # rubocop: disable Cop/DestroyAll + + { deleted_count: deleted_ids.count, deleted_ids: deleted_ids } + end + end + end +end diff --git a/app/services/ci/runners/process_runner_version_update_service.rb b/app/services/ci/runners/process_runner_version_update_service.rb new file mode 100644 index 00000000000..c8a5e42ccab --- /dev/null +++ b/app/services/ci/runners/process_runner_version_update_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Ci + module Runners + class ProcessRunnerVersionUpdateService + def initialize(version) + @version = version + end + + def execute + return ServiceResponse.error(message: 'version not present') unless @version + + _, status = upgrade_check_service.check_runner_upgrade_suggestion(@version) + return ServiceResponse.error(message: 'upgrade version check failed') if status == :error + + Ci::RunnerVersion.upsert({ version: @version, status: status }) + ServiceResponse.success(payload: { upgrade_status: status.to_s }) + end + + private + + def upgrade_check_service + @runner_upgrade_check ||= Gitlab::Ci::RunnerUpgradeCheck.new(::Gitlab::VERSION) + end + end + end +end diff --git a/app/services/ci/runners/reconcile_existing_runner_versions_service.rb b/app/services/ci/runners/reconcile_existing_runner_versions_service.rb index e04079bfe27..1950d82845b 100644 --- a/app/services/ci/runners/reconcile_existing_runner_versions_service.rb +++ b/app/services/ci/runners/reconcile_existing_runner_versions_service.rb @@ -3,8 +3,6 @@ module Ci module Runners class ReconcileExistingRunnerVersionsService - include BaseServiceUtility - VERSION_BATCH_SIZE = 100 def execute @@ -12,7 +10,7 @@ module Ci total_deleted = cleanup_runner_versions(insert_result[:versions_from_runners]) total_updated = update_status_on_outdated_runner_versions(insert_result[:versions_from_runners]) - success({ + ServiceResponse.success(payload: { total_inserted: insert_result[:new_record_count], total_updated: total_updated, total_deleted: total_deleted @@ -22,7 +20,7 @@ module Ci private def upgrade_check - Gitlab::Ci::RunnerUpgradeCheck.instance + @runner_upgrade_check ||= Gitlab::Ci::RunnerUpgradeCheck.new(::Gitlab::VERSION) end # rubocop: disable CodeReuse/ActiveRecord @@ -74,13 +72,11 @@ module Ci end def runner_version_with_updated_status(runner_version) - version = runner_version['version'] - suggestion = upgrade_check.check_runner_upgrade_status(version) - new_status = suggestion.each_key.first + _, new_status = upgrade_check.check_runner_upgrade_suggestion(runner_version.version) - if new_status != :error && new_status != runner_version['status'].to_sym + if new_status != :error && new_status != runner_version.status.to_sym { - version: version, + version: runner_version.version, status: Ci::RunnerVersion.statuses[new_status] } end diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb index 6588cd7e248..ae9b8bc8a16 100644 --- a/app/services/ci/runners/register_runner_service.rb +++ b/app/services/ci/runners/register_runner_service.rb @@ -6,7 +6,7 @@ module Ci def execute(registration_token, attributes) runner_type_attrs = extract_runner_type_attrs(registration_token) - return unless runner_type_attrs + return ServiceResponse.error(message: 'invalid token supplied', http_status: :forbidden) unless runner_type_attrs runner = ::Ci::Runner.new(attributes.merge(runner_type_attrs)) @@ -20,7 +20,7 @@ module Ci end end - runner + ServiceResponse.success(payload: { runner: runner }) end private diff --git a/app/services/ci/runners/reset_registration_token_service.rb b/app/services/ci/runners/reset_registration_token_service.rb index 81a70a771cf..dddbfb78d44 100644 --- a/app/services/ci/runners/reset_registration_token_service.rb +++ b/app/services/ci/runners/reset_registration_token_service.rb @@ -11,15 +11,19 @@ module Ci end def execute - return unless @user.present? && @user.can?(:update_runners_registration_token, scope) + unless @user.present? && @user.can?(:update_runners_registration_token, scope) + return ServiceResponse.error(message: 'user not allowed to update runners registration token') + end if scope.respond_to?(:runners_registration_token) scope.reset_runners_registration_token! - scope.runners_registration_token + runners_token = scope.runners_registration_token else scope.reset_runners_token! - scope.runners_token + runners_token = scope.runners_token end + + ServiceResponse.success(payload: { new_registration_token: runners_token }) end private diff --git a/app/services/ci/runners/unassign_runner_service.rb b/app/services/ci/runners/unassign_runner_service.rb index 1e46cf6add8..c40e5e0d44e 100644 --- a/app/services/ci/runners/unassign_runner_service.rb +++ b/app/services/ci/runners/unassign_runner_service.rb @@ -13,9 +13,15 @@ module Ci end def execute - return false unless @user.present? && @user.can?(:assign_runner, @runner) + unless @user.present? && @user.can?(:assign_runner, @runner) + return ServiceResponse.error(message: 'user not allowed to assign runner') + end - @runner_project.destroy + if @runner_project.destroy + ServiceResponse.success + else + ServiceResponse.error(message: 'failed to destroy runner project') + end end private diff --git a/app/services/ci/runners/unregister_runner_service.rb b/app/services/ci/runners/unregister_runner_service.rb index 4ee1e73c458..742b21f77df 100644 --- a/app/services/ci/runners/unregister_runner_service.rb +++ b/app/services/ci/runners/unregister_runner_service.rb @@ -14,6 +14,7 @@ module Ci def execute @runner&.destroy + ServiceResponse.success end end end diff --git a/app/services/ci/stuck_builds/drop_helpers.rb b/app/services/ci/stuck_builds/drop_helpers.rb index 048b52c6e13..dca50963883 100644 --- a/app/services/ci/stuck_builds/drop_helpers.rb +++ b/app/services/ci/stuck_builds/drop_helpers.rb @@ -56,12 +56,12 @@ module Ci def log_dropping_message(type, build, reason) Gitlab::AppLogger.info(class: self.class.name, - message: "Dropping #{type} build", - build_stuck_type: type, - build_id: build.id, - runner_id: build.runner_id, - build_status: build.status, - build_failure_reason: reason) + message: "Dropping #{type} build", + build_stuck_type: type, + build_id: build.id, + runner_id: build.runner_id, + build_status: build.status, + build_failure_reason: reason) end end end diff --git a/app/services/ci/track_failed_build_service.rb b/app/services/ci/track_failed_build_service.rb new file mode 100644 index 00000000000..caf7034234c --- /dev/null +++ b/app/services/ci/track_failed_build_service.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +# This service tracks failed CI builds using Snowplow. +# +# @param build [Ci::Build] the build that failed. +# @param exit_code [Int] the resulting exit code. +module Ci + class TrackFailedBuildService + SCHEMA_URL = 'iglu:com.gitlab/ci_build_failed/jsonschema/1-0-0' + + def initialize(build:, exit_code:, failure_reason:) + @build = build + @exit_code = exit_code + @failure_reason = failure_reason + end + + def execute + # rubocop:disable Style/IfUnlessModifier + unless @build.failed? + return ServiceResponse.error(message: 'Attempted to track a non-failed CI build') + end + + # rubocop:enable Style/IfUnlessModifier + + context = SnowplowTracker::SelfDescribingJson.new(SCHEMA_URL, payload) + + ::Gitlab::Tracking.event( + 'ci::build', + 'failed', + context: [context], + user: @build.user, + project: @build.project_id) + + ServiceResponse.success + end + + private + + def payload + { + build_id: @build.id, + build_name: @build.name, + build_artifact_types: @build.job_artifact_types, + exit_code: @exit_code, + failure_reason: @failure_reason + } + end + end +end diff --git a/app/services/ci/update_build_state_service.rb b/app/services/ci/update_build_state_service.rb index a74ddcfaf06..835d5f9a16c 100644 --- a/app/services/ci/update_build_state_service.rb +++ b/app/services/ci/update_build_state_service.rb @@ -105,7 +105,7 @@ module Ci Result.new(status: 200) when 'failed' - build.drop_with_exit_code!(params[:failure_reason] || :unknown_failure, params[:exit_code]) + build.drop_with_exit_code!(params[:failure_reason], params[:exit_code]) Result.new(status: 200) else diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index f10ff4e6f19..8c6c7b15d28 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -39,12 +39,6 @@ module AlertManagement SystemNoteService.change_alert_status(alert, User.alert_bot) close_issue(alert.issue_id) if auto_close_incident? - else - logger.warn( - message: 'Unable to update AlertManagement::Alert status to resolved', - project_id: project.id, - alert_id: alert.id - ) end end @@ -64,13 +58,18 @@ module AlertManagement if alert.save alert.execute_integrations SystemNoteService.create_new_alert(alert, alert_source) + elsif alert.errors[:fingerprint].any? + refind_and_increment_alert else logger.warn( - message: "Unable to create AlertManagement::Alert from #{alert_source}", + message: "Unable to create AlertManagement::Alert", project_id: project.id, - alert_errors: alert.errors.messages + alert_errors: alert.errors.messages, + alert_source: alert_source ) end + rescue ActiveRecord::RecordNotUnique + refind_and_increment_alert end def process_incident_issues @@ -107,6 +106,12 @@ module AlertManagement AlertManagement::Alert.new(**incoming_payload.alert_params, ended_at: nil) end + def refind_and_increment_alert + clear_memoization(:alert) + + process_firing_alert + end + def resolving_alert? incoming_payload.ends_at.present? end diff --git a/app/services/concerns/work_items/widgetable_service.rb b/app/services/concerns/work_items/widgetable_service.rb index 5665b07dce1..beb614c7b76 100644 --- a/app/services/concerns/work_items/widgetable_service.rb +++ b/app/services/concerns/work_items/widgetable_service.rb @@ -18,7 +18,7 @@ module WorkItems # rubocop:enable Gitlab/ModuleWithInstanceVariables def widget_service_class(widget) - "WorkItems::Widgets::#{widget.type.capitalize}Service::#{self.class.name.demodulize}".constantize + "WorkItems::Widgets::#{widget.type.to_s.camelize}Service::#{self.class.name.demodulize}".constantize rescue NameError nil end diff --git a/app/services/database/consistency_check_service.rb b/app/services/database/consistency_check_service.rb index e39bc8f25b8..fee2e79a6cb 100644 --- a/app/services/database/consistency_check_service.rb +++ b/app/services/database/consistency_check_service.rb @@ -80,7 +80,7 @@ module Database end def max_id - @max_id ||= source_model.minimum(source_sort_column) + @max_id ||= source_model.maximum(source_sort_column) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/deployments/update_environment_service.rb b/app/services/deployments/update_environment_service.rb index b0eb153a7af..3cacedc7d6e 100644 --- a/app/services/deployments/update_environment_service.rb +++ b/app/services/deployments/update_environment_service.rb @@ -58,11 +58,7 @@ module Deployments def expanded_environment_url return unless environment_url - if ::Feature.enabled?(:ci_expand_environment_name_and_url, deployment.project) - ExpandVariables.expand(environment_url, -> { variables.sort_and_expand_all }) - else - ExpandVariables.expand(environment_url, -> { variables }) - end + ExpandVariables.expand(environment_url, -> { variables.sort_and_expand_all }) end def environment_url @@ -88,7 +84,7 @@ module Deployments def renew_deployment_tier return unless deployable - if (tier = deployable.environment_deployment_tier) + if (tier = deployable.environment_tier_from_options) environment.tier = tier end end diff --git a/app/services/design_management/generate_image_versions_service.rb b/app/services/design_management/generate_image_versions_service.rb index e56d163c461..3ff239b59cc 100644 --- a/app/services/design_management/generate_image_versions_service.rb +++ b/app/services/design_management/generate_image_versions_service.rb @@ -43,7 +43,7 @@ module DesignManagement end # Skip attempting to process images that would be rejected by CarrierWave. - return unless DesignManagement::DesignV432x230Uploader::MIME_TYPE_WHITELIST.include?(raw_file.content_type) + return unless DesignManagement::DesignV432x230Uploader::MIME_TYPE_ALLOWLIST.include?(raw_file.content_type) # Store and process the file action.image_v432x230.store!(raw_file) diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb index d2ecd0a6d5a..8458eb1f3b8 100644 --- a/app/services/error_tracking/base_service.rb +++ b/app/services/error_tracking/base_service.rb @@ -25,7 +25,7 @@ module ErrorTracking errors = parse_errors(response) return errors if errors - yield if block_given? + yield if block track_usage_event(params[:tracking_event], current_user.id) if params[:tracking_event] diff --git a/app/services/google_cloud/base_service.rb b/app/services/google_cloud/base_service.rb index 016ab15408f..01aee2231c9 100644 --- a/app/services/google_cloud/base_service.rb +++ b/app/services/google_cloud/base_service.rb @@ -22,7 +22,7 @@ module GoogleCloud def unique_gcp_project_ids filter_params = { key: 'GCP_PROJECT_ID' } - ::Ci::VariablesFinder.new(project, filter_params).execute.map(&:value).uniq + @unique_gcp_project_ids ||= ::Ci::VariablesFinder.new(project, filter_params).execute.map(&:value).uniq end def group_vars_by_environment(keys) diff --git a/app/services/google_cloud/create_cloudsql_instance_service.rb b/app/services/google_cloud/create_cloudsql_instance_service.rb new file mode 100644 index 00000000000..f7fca277c52 --- /dev/null +++ b/app/services/google_cloud/create_cloudsql_instance_service.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module GoogleCloud + DEFAULT_REGION = 'us-east1' + + class CreateCloudsqlInstanceService < ::GoogleCloud::BaseService + WORKER_INTERVAL = 30.seconds + + def execute + create_cloud_instance + trigger_instance_setup_worker + success + rescue Google::Apis::Error => err + error(err.to_json) + end + + private + + def create_cloud_instance + 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? + }) + end + + def protected? + project.protected_for?(environment_name) + end + + def instance_name + # Generates an `instance_name` for the to-be-created Cloud SQL instance + # Example: `gitlab-34647-postgres-14-staging` + environment_alias = environment_name == '*' ? 'ALL' : environment_name + name = "gitlab-#{project.id}-#{database_version}-#{environment_alias}" + name.tr("_", "-").downcase + end + + def root_password + SecureRandom.hex(16) + end + + def database_version + params[:database_version] + end + + def region + region = ::Ci::VariablesFinder + .new(project, { key: Projects::GoogleCloud::GcpRegionsController::GCP_REGION_CI_VAR_KEY, + environment_scope: environment_name }) + .execute.first + region&.value || DEFAULT_REGION + end + + def tier + params[:tier] + end + end +end diff --git a/app/services/google_cloud/enable_cloudsql_service.rb b/app/services/google_cloud/enable_cloudsql_service.rb new file mode 100644 index 00000000000..a466b2f3696 --- /dev/null +++ b/app/services/google_cloud/enable_cloudsql_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module GoogleCloud + class EnableCloudsqlService < ::GoogleCloud::BaseService + def execute + return no_projects_error if unique_gcp_project_ids.empty? + + unique_gcp_project_ids.each do |gcp_project_id| + google_api_client.enable_cloud_sql_admin(gcp_project_id) + google_api_client.enable_compute(gcp_project_id) + google_api_client.enable_service_networking(gcp_project_id) + end + + success({ gcp_project_ids: unique_gcp_project_ids }) + end + + private + + def no_projects_error + error("No GCP projects found. Configure a service account or GCP_PROJECT_ID CI variable.") + end + end +end diff --git a/app/services/google_cloud/get_cloudsql_instances_service.rb b/app/services/google_cloud/get_cloudsql_instances_service.rb new file mode 100644 index 00000000000..701e83d556d --- /dev/null +++ b/app/services/google_cloud/get_cloudsql_instances_service.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module GoogleCloud + class GetCloudsqlInstancesService < ::GoogleCloud::BaseService + CLOUDSQL_KEYS = %w[GCP_PROJECT_ID GCP_CLOUDSQL_INSTANCE_NAME GCP_CLOUDSQL_VERSION].freeze + + def execute + group_vars_by_environment(CLOUDSQL_KEYS).map do |environment_scope, value| + { + ref: environment_scope, + gcp_project: value['GCP_PROJECT_ID'], + instance_name: value['GCP_CLOUDSQL_INSTANCE_NAME'], + version: value['GCP_CLOUDSQL_VERSION'] + } + end + end + end +end diff --git a/app/services/google_cloud/setup_cloudsql_instance_service.rb b/app/services/google_cloud/setup_cloudsql_instance_service.rb index 73650ee752f..10237f83b37 100644 --- a/app/services/google_cloud/setup_cloudsql_instance_service.rb +++ b/app/services/google_cloud/setup_cloudsql_instance_service.rb @@ -16,29 +16,29 @@ module GoogleCloud return error("CloudSQL instance not RUNNABLE: #{get_instance_response.to_json}") end - database_response = google_api_client.create_cloudsql_database(gcp_project_id, instance_name, database_name) + save_instance_ci_vars(get_instance_response) - if database_response.status != OPERATION_STATE_DONE - return error("Database creation failed: #{database_response.to_json}") - end + list_database_response = google_api_client.list_cloudsql_databases(gcp_project_id, instance_name) + list_user_response = google_api_client.list_cloudsql_users(gcp_project_id, instance_name) - user_response = google_api_client.create_cloudsql_user(gcp_project_id, instance_name, username, password) + existing_database = list_database_response.items.find { |database| database.name == database_name } + existing_user = list_user_response.items.find { |user| user.name == username } - if user_response.status != OPERATION_STATE_DONE - return error("User creation failed: #{user_response.to_json}") + if existing_database && existing_user + save_database_ci_vars + save_user_ci_vars(existing_user) + return success end - primary_ip_address = get_instance_response.ip_addresses.first.ip_address - connection_name = get_instance_response.connection_name + database_response = execute_database_setup(existing_database) + return database_response if database_response[:status] == :error - save_ci_var('GCP_PROJECT_ID', gcp_project_id) - save_ci_var('GCP_CLOUDSQL_INSTANCE_NAME', instance_name) - save_ci_var('GCP_CLOUDSQL_CONNECTION_NAME', connection_name) - save_ci_var('GCP_CLOUDSQL_PRIMARY_IP_ADDRESS', primary_ip_address) - save_ci_var('GCP_CLOUDSQL_VERSION', database_version) - save_ci_var('GCP_CLOUDSQL_DATABASE_NAME', database_name) - save_ci_var('GCP_CLOUDSQL_DATABASE_USER', username) - save_ci_var('GCP_CLOUDSQL_DATABASE_PASS', password, true) + save_database_ci_vars + + user_response = execute_user_setup(existing_user) + return user_response if user_response[:status] == :error + + save_user_ci_vars(existing_user) success rescue Google::Apis::Error => err @@ -64,11 +64,55 @@ module GoogleCloud end def password - SecureRandom.hex(16) + @password ||= SecureRandom.hex(16) end def save_ci_var(key, value, is_masked = false) create_or_replace_project_vars(environment_name, key, value, @params[:is_protected], is_masked) end + + def save_instance_ci_vars(cloudsql_instance) + primary_ip_address = cloudsql_instance.ip_addresses.first.ip_address + connection_name = cloudsql_instance.connection_name + + save_ci_var('GCP_PROJECT_ID', gcp_project_id) + save_ci_var('GCP_CLOUDSQL_INSTANCE_NAME', instance_name) + save_ci_var('GCP_CLOUDSQL_CONNECTION_NAME', connection_name) + save_ci_var('GCP_CLOUDSQL_PRIMARY_IP_ADDRESS', primary_ip_address) + save_ci_var('GCP_CLOUDSQL_VERSION', database_version) + end + + def save_database_ci_vars + save_ci_var('GCP_CLOUDSQL_DATABASE_NAME', database_name) + end + + def save_user_ci_vars(user_exists) + save_ci_var('GCP_CLOUDSQL_DATABASE_USER', username) + save_ci_var('GCP_CLOUDSQL_DATABASE_PASS', user_exists ? user_exists.password : password, true) + end + + def execute_database_setup(database_exists) + return success if database_exists + + database_response = google_api_client.create_cloudsql_database(gcp_project_id, instance_name, database_name) + + if database_response.status != OPERATION_STATE_DONE + return error("Database creation failed: #{database_response.to_json}") + end + + success + end + + def execute_user_setup(existing_user) + return success if existing_user + + user_response = google_api_client.create_cloudsql_user(gcp_project_id, instance_name, username, password) + + if user_response.status != OPERATION_STATE_DONE + return error("User creation failed: #{user_response.to_json}") + end + + success + end end end diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index bcf3110ca21..02a760ccf29 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -45,6 +45,8 @@ module Groups .execute(blocking: true) end + publish_event + group end # rubocop: enable CodeReuse/ActiveRecord @@ -91,6 +93,17 @@ module Groups end end # rubocop:enable CodeReuse/ActiveRecord + + def publish_event + event = Groups::GroupDeletedEvent.new( + data: { + group_id: group.id, + root_namespace_id: group.root_ancestor.id + } + ) + + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/groups/import_export/export_service.rb b/app/services/groups/import_export/export_service.rb index 2bfd5a5ebab..bd54b48c5f4 100644 --- a/app/services/groups/import_export/export_service.rb +++ b/app/services/groups/import_export/export_service.rb @@ -49,13 +49,23 @@ module Groups # We cannot include the file_saver with the other savers because # it removes the tmp dir. This means that if we want to add new savers # in EE the data won't be available. - if savers.all?(&:save) && file_saver.save + if save_exporters && file_saver.save notify_success else notify_error! end end + def save_exporters + log_info('Group export started') + + savers.all? do |exporter| + log_info("#{exporter.class.name} saver started") + + exporter.save + end + end + def savers [version_saver, tree_exporter] end @@ -99,12 +109,16 @@ module Groups raise Gitlab::ImportExport::Error, shared.errors.to_sentence end - def notify_success + def log_info(message) @logger.info( - message: 'Group Export succeeded', + message: message, group_id: group.id, group_name: group.name ) + end + + def notify_success + log_info('Group Export succeeded') notification_service.group_was_exported(group, current_user) end diff --git a/app/services/groups/import_export/import_service.rb b/app/services/groups/import_export/import_service.rb index f026f1698a9..db52a272bf2 100644 --- a/app/services/groups/import_export/import_service.rb +++ b/app/services/groups/import_export/import_service.rb @@ -97,17 +97,17 @@ module Groups def notify_success @logger.info( - group_id: group.id, + group_id: group.id, group_name: group.name, - message: 'Group Import/Export: Import succeeded' + message: 'Group Import/Export: Import succeeded' ) end def notify_error @logger.error( - group_id: group.id, + group_id: group.id, group_name: group.name, - message: "Group Import/Export: Errors occurred, see '#{Gitlab::ErrorTracking::Logger.file_name}' for details" + message: "Group Import/Export: Errors occurred, see '#{Gitlab::ErrorTracking::Logger.file_name}' for details" ) end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 29e3a9473ab..6fbf7daeb81 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -36,7 +36,7 @@ module Groups update_crm_objects(was_root_group) end - post_update_hooks(@updated_project_ids) + post_update_hooks(@updated_project_ids, old_root_ancestor_id) propagate_integrations update_pending_builds @@ -44,9 +44,10 @@ module Groups end # Overridden in EE - def post_update_hooks(updated_project_ids) + def post_update_hooks(updated_project_ids, old_root_ancestor_id) refresh_project_authorizations refresh_descendant_groups if @new_parent_group + publish_event(old_root_ancestor_id) end def ensure_allowed_transfer @@ -266,6 +267,18 @@ module Groups CustomerRelations::IssueContact.delete_for_group(@group) end + + def publish_event(old_root_ancestor_id) + event = ::Groups::GroupTransferedEvent.new( + data: { + group_id: group.id, + old_root_namespace_id: old_root_ancestor_id, + new_root_namespace_id: group.root_ancestor.id + } + ) + + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index b3b0397eac3..2135892a95a 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -61,15 +61,18 @@ module Groups end def before_assignment_hook(group, params) - # overridden in EE + @full_path_before = group.full_path + @path_before = group.path end def renaming_group_with_container_registry_images? + renaming? && group.has_container_repository_including_subgroups? + end + + def renaming? new_path = params[:path] - new_path && - new_path != group.path && - group.has_container_repository_including_subgroups? + new_path && new_path != @path_before end def container_images_error @@ -83,6 +86,8 @@ module Groups end update_two_factor_requirement_for_subgroups + + publish_event end def update_two_factor_requirement_for_subgroups @@ -154,6 +159,21 @@ module Groups group.errors.add(:update_shared_runners, result[:message]) false end + + def publish_event + return unless renaming? + + event = Groups::GroupPathChangedEvent.new( + data: { + group_id: group.id, + root_namespace_id: group.root_ancestor.id, + old_path: @full_path_before, + new_path: group.full_path + } + ) + + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/import/prepare_service.rb b/app/services/import/prepare_service.rb new file mode 100644 index 00000000000..278bd463dcd --- /dev/null +++ b/app/services/import/prepare_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Import + class PrepareService < ::BaseService + def execute + uploader = UploadService.new(project, params[:file]).execute + + if uploader + enqueue_import(uploader.upload) + + ServiceResponse.success(message: success_message) + else + ServiceResponse.error(message: _('File upload error.')) + end + end + + private + + def enqueue_import(upload) + worker.perform_async(current_user.id, project.id, upload.id) + end + + def worker + raise NotImplementedError + end + + def success_message + raise NotImplementedError + end + end +end diff --git a/app/services/incident_management/timeline_events/create_service.rb b/app/services/incident_management/timeline_events/create_service.rb index 3cb67ccf2b1..40ce9097c88 100644 --- a/app/services/incident_management/timeline_events/create_service.rb +++ b/app/services/incident_management/timeline_events/create_service.rb @@ -48,6 +48,26 @@ module IncidentManagement new(incident, user, note: note, occurred_at: occurred_at, action: action, auto_created: true).execute end + + def change_labels(incident, user, added_labels: [], removed_labels: []) + return if Feature.disabled?(:incident_timeline_events_from_labels, incident.project) + + if added_labels.blank? && removed_labels.blank? + return ServiceResponse.error(message: _('There are no changed labels')) + end + + labels_note = -> (verb, labels) { + "#{verb} #{labels.map(&:to_reference).join(' ')} #{'label'.pluralize(labels.count)}" if labels.present? + } + + added_note = labels_note.call('added', added_labels) + removed_note = labels_note.call('removed', removed_labels) + note = "@#{user.username} #{[added_note, removed_note].compact.join(' and ')}" + occurred_at = incident.updated_at + action = 'label' + + new(incident, user, note: note, occurred_at: occurred_at, action: action, auto_created: true).execute + end end def execute diff --git a/app/services/incident_management/timeline_events/update_service.rb b/app/services/incident_management/timeline_events/update_service.rb index 8217c8125c2..5c5de4717bc 100644 --- a/app/services/incident_management/timeline_events/update_service.rb +++ b/app/services/incident_management/timeline_events/update_service.rb @@ -34,7 +34,7 @@ module IncidentManagement attr_reader :timeline_event, :incident, :user, :note, :occurred_at def update_params - { updated_by_user: user, note: note.presence, occurred_at: occurred_at.presence }.compact + { updated_by_user: user, note: note, occurred_at: occurred_at }.compact end def add_system_note(timeline_event) diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb index 98c50347719..3c13944cfbc 100644 --- a/app/services/issuable/clone/base_service.rb +++ b/app/services/issuable/clone/base_service.rb @@ -16,6 +16,7 @@ module Issuable # ApplicationRecord.transaction do @new_entity = create_new_entity + @new_entity.system_note_timestamp = nil update_new_entity update_old_entity diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb index 5cf32ee3e40..db28be864a7 100644 --- a/app/services/issuable/common_system_notes_service.rb +++ b/app/services/issuable/common_system_notes_service.rb @@ -21,7 +21,7 @@ module Issuable create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked') end - create_due_date_note if issuable.previous_changes.include?('due_date') + handle_start_date_or_due_date_change_note create_milestone_change_event(old_milestone) if issuable.previous_changes.include?('milestone_id') create_labels_note(old_labels) if old_labels && issuable.labels != old_labels end @@ -29,6 +29,13 @@ module Issuable private + def handle_start_date_or_due_date_change_note + # Type check needed as some issuables do their own date change handling for date fields other than due_date + change_date_fields = issuable.is_a?(Issue) ? %w[due_date start_date] : %w[due_date] + changed_dates = issuable.previous_changes.slice(*change_date_fields) + create_start_date_or_due_date_note(changed_dates) + end + def handle_time_tracking_note if issuable.previous_changes.include?('time_estimate') create_time_estimate_note @@ -99,8 +106,10 @@ module Issuable .execute end - def create_due_date_note - SystemNoteService.change_due_date(issuable, issuable.project, current_user, issuable.due_date) + def create_start_date_or_due_date_note(changed_dates) + return if changed_dates.blank? + + SystemNoteService.change_start_date_or_due_date(issuable, issuable.project, current_user, changed_dates) end def create_discussion_lock_note diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index 9b41c88159f..822e3cd787c 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -21,13 +21,9 @@ module Issuable def process_csv with_csv_lines.each do |row, line_no| - issuable_attributes = { - title: row[:title], - description: row[:description], - due_date: row[:due_date] - } + attributes = issuable_attributes_for(row) - if create_issuable(issuable_attributes).persisted? + if create_issuable(attributes).persisted? @results[:success] += 1 else @results[:error_lines].push(line_no) @@ -37,6 +33,14 @@ module Issuable @results[:parse_error] = true end + def issuable_attributes_for(row) + { + title: row[:title], + description: row[:description], + due_date: row[:due_date] + } + end + def with_csv_lines csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8) validate_headers_presence!(csv_data.lines.first) diff --git a/app/services/issues/clone_service.rb b/app/services/issues/clone_service.rb index 896b15a14b8..07dd9a98f89 100644 --- a/app/services/issues/clone_service.rb +++ b/app/services/issues/clone_service.rb @@ -41,7 +41,6 @@ module Issues def update_new_entity # we don't call `super` because we want to be able to decide whether or not to copy all comments over. update_new_entity_description - copy_award_emoji if with_notes copy_notes @@ -96,9 +95,14 @@ module Issues end def add_note_from - SystemNoteService.noteable_cloned(new_entity, target_project, - original_entity, current_user, - direction: :from) + SystemNoteService.noteable_cloned( + new_entity, + target_project, + original_entity, + current_user, + direction: :from, + created_at: new_entity.created_at + ) end def add_note_to diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 30d4cb68840..92cf4811439 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -45,7 +45,7 @@ module Issues # current_user (defined in BaseService) is not available within run_after_commit block user = current_user issue.run_after_commit do - NewIssueWorker.perform_async(issue.id, user.id) + NewIssueWorker.perform_async(issue.id, user.id, issue.class.to_s) Issues::PlacementWorker.perform_async(nil, issue.project_id) Namespaces::OnboardingIssueCreatedWorker.perform_async(issue.project.namespace_id) end diff --git a/app/services/issues/export_csv_service.rb b/app/services/issues/export_csv_service.rb index 7076e858155..6209127bd86 100644 --- a/app/services/issues/export_csv_service.rb +++ b/app/services/issues/export_csv_service.rb @@ -25,24 +25,24 @@ module Issues { 'Title' => 'title', 'Description' => 'description', - 'Issue ID' => 'iid', - 'URL' => -> (issue) { issue_url(issue) }, - 'State' => -> (issue) { issue.closed? ? 'Closed' : 'Open' }, - 'Author' => 'author_name', - 'Author Username' => -> (issue) { issue.author&.username }, - 'Assignee' => -> (issue) { issue.assignees.map(&:name).join(', ') }, - 'Assignee Username' => -> (issue) { issue.assignees.map(&:username).join(', ') }, - 'Confidential' => -> (issue) { issue.confidential? ? 'Yes' : 'No' }, - 'Locked' => -> (issue) { issue.discussion_locked? ? 'Yes' : 'No' }, - 'Due Date' => -> (issue) { issue.due_date&.to_s(:csv) }, - 'Created At (UTC)' => -> (issue) { issue.created_at&.to_s(:csv) }, - 'Updated At (UTC)' => -> (issue) { issue.updated_at&.to_s(:csv) }, - 'Closed At (UTC)' => -> (issue) { issue.closed_at&.to_s(:csv) }, - 'Milestone' => -> (issue) { issue.milestone&.title }, - 'Weight' => -> (issue) { issue.weight }, - 'Labels' => -> (issue) { issue_labels(issue) }, - 'Time Estimate' => ->(issue) { issue.time_estimate.to_s(:csv) }, - 'Time Spent' => -> (issue) { issue_time_spent(issue) } + 'Issue ID' => 'iid', + 'URL' => -> (issue) { issue_url(issue) }, + 'State' => -> (issue) { issue.closed? ? 'Closed' : 'Open' }, + 'Author' => 'author_name', + 'Author Username' => -> (issue) { issue.author&.username }, + 'Assignee' => -> (issue) { issue.assignees.map(&:name).join(', ') }, + 'Assignee Username' => -> (issue) { issue.assignees.map(&:username).join(', ') }, + 'Confidential' => -> (issue) { issue.confidential? ? 'Yes' : 'No' }, + 'Locked' => -> (issue) { issue.discussion_locked? ? 'Yes' : 'No' }, + 'Due Date' => -> (issue) { issue.due_date&.to_s(:csv) }, + 'Created At (UTC)' => -> (issue) { issue.created_at&.to_s(:csv) }, + 'Updated At (UTC)' => -> (issue) { issue.updated_at&.to_s(:csv) }, + 'Closed At (UTC)' => -> (issue) { issue.closed_at&.to_s(:csv) }, + 'Milestone' => -> (issue) { issue.milestone&.title }, + 'Weight' => -> (issue) { issue.weight }, + 'Labels' => -> (issue) { issue_labels(issue) }, + 'Time Estimate' => ->(issue) { issue.time_estimate.to_s(:csv) }, + 'Time Spent' => -> (issue) { issue_time_spent(issue) } } end diff --git a/app/services/issues/prepare_import_csv_service.rb b/app/services/issues/prepare_import_csv_service.rb new file mode 100644 index 00000000000..7afe363117e --- /dev/null +++ b/app/services/issues/prepare_import_csv_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Issues + class PrepareImportCsvService < Import::PrepareService + extend ::Gitlab::Utils::Override + + private + + override :worker + def worker + ImportIssuesCsvWorker + end + + override :success_message + def success_message + _("Your issues are being imported. Once finished, you'll get a confirmation email.") + end + end +end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index afc61eed287..46c28d82ddc 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -70,6 +70,7 @@ module Issues handle_severity_change(issue, old_severity) handle_escalation_status_change(issue) handle_issue_type_change(issue) + handle_date_changes(issue) end def handle_assignee_changes(issue, old_assignees) @@ -116,6 +117,12 @@ module Issues attr_reader :spam_params + def handle_date_changes(issue) + return unless issue.previous_changes.slice('due_date', 'start_date').any? + + GraphqlTriggers.issuable_dates_updated(issue) + end + def clone_issue(issue) target_project = params.delete(:target_clone_project) with_notes = params.delete(:clone_with_notes) diff --git a/app/services/jira/requests/base.rb b/app/services/jira/requests/base.rb index 8e8511e5180..d0ca8863c29 100644 --- a/app/services/jira/requests/base.rb +++ b/app/services/jira/requests/base.rb @@ -9,10 +9,10 @@ module Jira ERRORS = { connection: [Errno::ECONNRESET, Errno::ECONNREFUSED], - jira_ruby: JIRA::HTTPError, - ssl: OpenSSL::SSL::SSLError, - timeout: [Timeout::Error, Errno::ETIMEDOUT], - uri: [URI::InvalidURIError, SocketError] + jira_ruby: JIRA::HTTPError, + ssl: OpenSSL::SSL::SSLError, + timeout: [Timeout::Error, Errno::ETIMEDOUT], + uri: [URI::InvalidURIError, SocketError] }.freeze ALL_ERRORS = ERRORS.values.flatten.freeze diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index b8d817a15f3..dcc4cf4bb1e 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -10,14 +10,27 @@ module MergeRequests return success unless save_approval(approval) reset_approvals_cache(merge_request) - create_event(merge_request) - stream_audit_event(merge_request) - create_approval_note(merge_request) - mark_pending_todos_as_done(merge_request) - execute_approval_hooks(merge_request, current_user) - remove_attention_requested(merge_request) merge_request_activity_counter.track_approve_mr_action(user: current_user, merge_request: merge_request) + # Approval side effects (things not required to be done immediately but + # should happen after a successful approval) should be done asynchronously + # utilizing the `Gitlab::EventStore`. + # + # Workers can subscribe to the `MergeRequests::ApprovedEvent`. + if Feature.enabled?(:async_after_approval, project) + Gitlab::EventStore.publish( + MergeRequests::ApprovedEvent.new( + data: { current_user_id: current_user.id, merge_request_id: merge_request.id } + ) + ) + else + create_event(merge_request) + stream_audit_event(merge_request) + create_approval_note(merge_request) + mark_pending_todos_as_done(merge_request) + execute_approval_hooks(merge_request, current_user) + end + success end @@ -27,21 +40,22 @@ module MergeRequests current_user.can?(:approve_merge_request, merge_request) end + def save_approval(approval) + Approval.safe_ensure_unique do + approval.save + end + end + def reset_approvals_cache(merge_request) merge_request.approvals.reset end - def execute_approval_hooks(merge_request, current_user) - # Only one approval is required for a merge request to be approved - notification_service.async.approve_mr(merge_request, current_user) - - execute_hooks(merge_request, 'approved') + def create_event(merge_request) + event_service.approve_mr(merge_request, current_user) end - def save_approval(approval) - Approval.safe_ensure_unique do - approval.save - end + def stream_audit_event(merge_request) + # Defined in EE end def create_approval_note(merge_request) @@ -52,12 +66,11 @@ module MergeRequests todo_service.resolve_todos_for_target(merge_request, current_user) end - def create_event(merge_request) - event_service.approve_mr(merge_request, current_user) - end + def execute_approval_hooks(merge_request, current_user) + # Only one approval is required for a merge request to be approved + notification_service.async.approve_mr(merge_request, current_user) - def stream_audit_event(merge_request) - # Defined in EE + execute_hooks(merge_request, 'approved') end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 9bd38478796..bda8dc64ac0 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -61,10 +61,6 @@ module MergeRequests merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) bulk_update_reviewers_state(merge_request, new_reviewers) - - unless new_reviewers.include?(current_user) - remove_attention_requested(merge_request) - end end def cleanup_environments(merge_request) @@ -252,20 +248,6 @@ module MergeRequests Milestones::MergeRequestsCountService.new(milestone).delete_cache end - def remove_all_attention_requests(merge_request) - return unless current_user.mr_attention_requests_enabled? - - users = merge_request.reviewers + merge_request.assignees - - ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, users: users.uniq).execute - end - - def remove_attention_requested(merge_request) - return unless current_user.mr_attention_requests_enabled? - - ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: current_user).execute - end - def bulk_update_assignees_state(merge_request, new_assignees) return unless current_user.mr_attention_requests_enabled? return if new_assignees.empty? diff --git a/app/services/merge_requests/bulk_remove_attention_requested_service.rb b/app/services/merge_requests/bulk_remove_attention_requested_service.rb deleted file mode 100644 index 774f2c2ee35..00000000000 --- a/app/services/merge_requests/bulk_remove_attention_requested_service.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class BulkRemoveAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request - attr_accessor :users - - def initialize(project:, current_user:, merge_request:, users:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @users = users - end - - # rubocop: disable CodeReuse/ActiveRecord - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - merge_request.merge_request_assignees.where(user_id: users).update_all(state: :reviewed) - merge_request.merge_request_reviewers.where(user_id: users).update_all(state: :reviewed) - - users.each { |user| user.invalidate_attention_requested_count } - - success - end - # rubocop: enable CodeReuse/ActiveRecord - end -end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index e9b253129b4..f83b14c7269 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -17,7 +17,6 @@ module MergeRequests create_note(merge_request) notification_service.async.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) - remove_all_attention_requests(merge_request) execute_hooks(merge_request, 'close') invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) merge_request.update_project_counter_caches diff --git a/app/services/merge_requests/create_approval_event_service.rb b/app/services/merge_requests/create_approval_event_service.rb new file mode 100644 index 00000000000..1678bf31139 --- /dev/null +++ b/app/services/merge_requests/create_approval_event_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module MergeRequests + class CreateApprovalEventService < MergeRequests::BaseService + def execute(merge_request) + event_service.approve_mr(merge_request, current_user) + end + end +end + +MergeRequests::CreateApprovalEventService.prepend_mod diff --git a/app/services/merge_requests/create_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb index c6a91a3b61e..4f20ade2a42 100644 --- a/app/services/merge_requests/create_pipeline_service.rb +++ b/app/services/merge_requests/create_pipeline_service.rb @@ -50,7 +50,8 @@ module MergeRequests end def can_create_pipeline_in_target_project?(merge_request) - can?(current_user, :create_pipeline, merge_request.target_project) && + merge_request.target_project.ci_allow_fork_pipelines_to_run_in_parent_project? && + can?(current_user, :create_pipeline, merge_request.target_project) && can_update_source_branch_in_target_project?(merge_request) end diff --git a/app/services/merge_requests/execute_approval_hooks_service.rb b/app/services/merge_requests/execute_approval_hooks_service.rb new file mode 100644 index 00000000000..7beeb9ea3f9 --- /dev/null +++ b/app/services/merge_requests/execute_approval_hooks_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module MergeRequests + class ExecuteApprovalHooksService < MergeRequests::BaseService + def execute(merge_request) + # Only one approval is required for a merge request to be approved + notification_service.async.approve_mr(merge_request, current_user) + execute_hooks(merge_request, 'approved') + end + end +end + +MergeRequests::ExecuteApprovalHooksService.prepend_mod diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb index 78c93d10f2a..87cd6544406 100644 --- a/app/services/merge_requests/handle_assignees_change_service.rb +++ b/app/services/merge_requests/handle_assignees_change_service.rb @@ -22,10 +22,6 @@ module MergeRequests merge_request_activity_counter.track_assignees_changed_action(user: current_user) execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] - - unless new_assignees.include?(current_user) - remove_attention_requested(merge_request) - end end private diff --git a/app/services/merge_requests/mergeability/check_base_service.rb b/app/services/merge_requests/mergeability/check_base_service.rb index d5ddcb4b828..e614a7c27fe 100644 --- a/app/services/merge_requests/mergeability/check_base_service.rb +++ b/app/services/merge_requests/mergeability/check_base_service.rb @@ -24,12 +24,12 @@ module MergeRequests private - def success(*args) - Gitlab::MergeRequests::Mergeability::CheckResult.success(*args) + def success(**args) + Gitlab::MergeRequests::Mergeability::CheckResult.success(payload: args) end - def failure(*args) - Gitlab::MergeRequests::Mergeability::CheckResult.failed(*args) + def failure(**args) + Gitlab::MergeRequests::Mergeability::CheckResult.failed(payload: args) end end end diff --git a/app/services/merge_requests/mergeability/check_broken_status_service.rb b/app/services/merge_requests/mergeability/check_broken_status_service.rb index 9a54a4292c8..6fe4eb4a57f 100644 --- a/app/services/merge_requests/mergeability/check_broken_status_service.rb +++ b/app/services/merge_requests/mergeability/check_broken_status_service.rb @@ -4,7 +4,7 @@ module MergeRequests class CheckBrokenStatusService < CheckBaseService def execute if merge_request.broken? - failure + failure(reason: failure_reason) else success end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :broken_status + end end end end diff --git a/app/services/merge_requests/mergeability/check_ci_status_service.rb b/app/services/merge_requests/mergeability/check_ci_status_service.rb index c0ef5ba1c30..9e09b513c57 100644 --- a/app/services/merge_requests/mergeability/check_ci_status_service.rb +++ b/app/services/merge_requests/mergeability/check_ci_status_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.mergeable_ci_state? success else - failure + failure(reason: failure_reason) end end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :ci_must_pass + end end end end diff --git a/app/services/merge_requests/mergeability/check_discussions_status_service.rb b/app/services/merge_requests/mergeability/check_discussions_status_service.rb index 9b4eab9d399..3421d96e8ae 100644 --- a/app/services/merge_requests/mergeability/check_discussions_status_service.rb +++ b/app/services/merge_requests/mergeability/check_discussions_status_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.mergeable_discussions_state? success else - failure + failure(reason: failure_reason) end end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :discussions_not_resolved + end end end end diff --git a/app/services/merge_requests/mergeability/check_draft_status_service.rb b/app/services/merge_requests/mergeability/check_draft_status_service.rb index bc940e2116d..a1524317155 100644 --- a/app/services/merge_requests/mergeability/check_draft_status_service.rb +++ b/app/services/merge_requests/mergeability/check_draft_status_service.rb @@ -5,7 +5,7 @@ module MergeRequests class CheckDraftStatusService < CheckBaseService def execute if merge_request.draft? - failure + failure(reason: failure_reason) else success end @@ -18,6 +18,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :draft_status + end end end end diff --git a/app/services/merge_requests/mergeability/check_open_status_service.rb b/app/services/merge_requests/mergeability/check_open_status_service.rb index 361af946e3f..29f3d0d3ccb 100644 --- a/app/services/merge_requests/mergeability/check_open_status_service.rb +++ b/app/services/merge_requests/mergeability/check_open_status_service.rb @@ -7,7 +7,7 @@ module MergeRequests if merge_request.open? success else - failure + failure(reason: failure_reason) end end @@ -18,6 +18,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :not_open + end end end end diff --git a/app/services/merge_requests/mergeability/run_checks_service.rb b/app/services/merge_requests/mergeability/run_checks_service.rb index 1d4b96b3090..68f842b3322 100644 --- a/app/services/merge_requests/mergeability/run_checks_service.rb +++ b/app/services/merge_requests/mergeability/run_checks_service.rb @@ -10,36 +10,50 @@ module MergeRequests end def execute - merge_request.mergeability_checks.each_with_object([]) do |check_class, results| + @results = merge_request.mergeability_checks.each_with_object([]) do |check_class, result_hash| check = check_class.new(merge_request: merge_request, params: params) next if check.skip? check_result = run_check(check) - results << check_result + result_hash << check_result - break results if check_result.failed? + break result_hash if check_result.failed? end + + self + end + + def success? + raise 'Execute needs to be called before' if results.nil? + + results.all?(&:success?) + end + + def failure_reason + raise 'Execute needs to be called before' if results.nil? + + results.find(&:failed?)&.payload&.fetch(:reason) end private - attr_reader :merge_request, :params + attr_reader :merge_request, :params, :results def run_check(check) return check.execute unless Feature.enabled?(:mergeability_caching, merge_request.project) return check.execute unless check.cacheable? - cached_result = results.read(merge_check: check) + cached_result = cached_results.read(merge_check: check) return cached_result if cached_result.respond_to?(:status) check.execute.tap do |result| - results.write(merge_check: check, result_hash: result.to_hash) + cached_results.write(merge_check: check, result_hash: result.to_hash) end end - def results - strong_memoize(:results) do + def cached_results + strong_memoize(:cached_results) do Gitlab::MergeRequests::Mergeability::ResultsStore.new(merge_request: merge_request) end end diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 30531fcc17b..1ce44f465cd 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -78,8 +78,8 @@ module MergeRequests lease_key = "mergeability_check:#{merge_request.id}" lease_opts = { - ttl: 1.minute, - retries: retry_lease ? 10 : 0, + ttl: 1.minute, + retries: retry_lease ? 10 : 0, sleep_sec: retry_lease ? 1.second : 0 } diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 286c082ac8a..9fca2b0d19e 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -28,7 +28,6 @@ module MergeRequests notification_service.merge_mr(merge_request, current_user) invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) merge_request.update_project_counter_caches - remove_all_attention_requests(merge_request) delete_non_latest_diffs(merge_request) cancel_review_app_jobs!(merge_request) cleanup_environments(merge_request) diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index 076fe8c3b21..ef251f121ae 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -105,7 +105,7 @@ module MergeRequests project: project, current_user: current_user, params: merge_request.attributes.merge(assignees: merge_request.assignees, - label_ids: merge_request.label_ids) + label_ids: merge_request.label_ids) ).execute end diff --git a/app/services/merge_requests/remove_approval_service.rb b/app/services/merge_requests/remove_approval_service.rb index d9bb17a7b1b..52628729519 100644 --- a/app/services/merge_requests/remove_approval_service.rb +++ b/app/services/merge_requests/remove_approval_service.rb @@ -17,7 +17,6 @@ module MergeRequests reset_approvals_cache(merge_request) create_note(merge_request) merge_request_activity_counter.track_unapprove_mr_action(user: current_user) - remove_attention_requested(merge_request) end success diff --git a/app/services/merge_requests/remove_attention_requested_service.rb b/app/services/merge_requests/remove_attention_requested_service.rb deleted file mode 100644 index 8a410fda691..00000000000 --- a/app/services/merge_requests/remove_attention_requested_service.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class RemoveAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request, :user - - def initialize(project:, current_user:, merge_request:, user:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @user = user - end - - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - if reviewer || assignee - return success if reviewer&.reviewed? || assignee&.reviewed? - - update_state(reviewer) - update_state(assignee) - - user.invalidate_attention_requested_count - create_remove_attention_request_note - - success - else - error("User is not a reviewer or assignee of the merge request") - end - end - - private - - def assignee - @assignee ||= merge_request.find_assignee(user) - end - - def reviewer - @reviewer ||= merge_request.find_reviewer(user) - end - - def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: :reviewed) - end - - def create_remove_attention_request_note - SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) - end - end -end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index 4612688f78b..d2247a6d4c1 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -20,8 +20,6 @@ module MergeRequests merge_request.cache_merge_request_closes_issues!(current_user) merge_request.cleanup_schedule&.destroy merge_request.update_column(:merge_ref_sha, nil) - - users.each { |user| user.invalidate_attention_requested_count } end merge_request diff --git a/app/services/merge_requests/request_attention_service.rb b/app/services/merge_requests/request_attention_service.rb deleted file mode 100644 index 07e9996f87b..00000000000 --- a/app/services/merge_requests/request_attention_service.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class RequestAttentionService < MergeRequests::BaseService - attr_accessor :merge_request, :user - - def initialize(project:, current_user:, merge_request:, user:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @user = user - end - - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - if reviewer || assignee - return success if reviewer&.attention_requested? || assignee&.attention_requested? - - update_state(reviewer) - update_state(assignee) - - user.invalidate_attention_requested_count - create_attention_request_note - notity_user - - if current_user.id != user.id - remove_attention_requested(merge_request) - end - - success - else - error("User is not a reviewer or assignee of the merge request") - end - end - - private - - def notity_user - notification_service.async.attention_requested_of_merge_request(merge_request, current_user, user) - todo_service.create_attention_requested_todo(merge_request, current_user, user) - end - - def create_attention_request_note - SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) - end - - def assignee - @assignee ||= merge_request.find_assignee(user) - end - - def reviewer - @reviewer ||= merge_request.find_reviewer(user) - end - - def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: :attention_requested, updated_state_by: current_user) - end - end -end diff --git a/app/services/merge_requests/toggle_attention_requested_service.rb b/app/services/merge_requests/toggle_attention_requested_service.rb deleted file mode 100644 index 64cdcd725a2..00000000000 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class ToggleAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request, :user - - def initialize(project:, current_user:, merge_request:, user:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @user = user - end - - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - if reviewer || assignee - update_state(reviewer) - update_state(assignee) - - user.invalidate_attention_requested_count - - if reviewer&.attention_requested? || assignee&.attention_requested? - create_attention_request_note - notity_user - - if current_user.id != user.id - remove_attention_requested(merge_request) - end - else - create_remove_attention_request_note - end - - success - else - error("User is not a reviewer or assignee of the merge request") - end - end - - private - - def notity_user - notification_service.async.attention_requested_of_merge_request(merge_request, current_user, user) - todo_service.create_attention_requested_todo(merge_request, current_user, user) - end - - def create_attention_request_note - SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) - end - - def create_remove_attention_request_note - SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) - end - - def assignee - merge_request.find_assignee(user) - end - - def reviewer - merge_request.find_reviewer(user) - end - - def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: reviewer_or_assignee&.attention_requested? ? :reviewed : :attention_requested, - updated_state_by: current_user) - end - end -end diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb index 5b23f69ac4a..a6b0235c525 100644 --- a/app/services/merge_requests/update_assignees_service.rb +++ b/app/services/merge_requests/update_assignees_service.rb @@ -11,7 +11,7 @@ module MergeRequests old_assignees = merge_request.assignees.to_a old_ids = old_assignees.map(&:id) - new_ids = new_assignee_ids(merge_request) + new_ids = new_user_ids(merge_request, update_attrs[:assignee_ids], :assignees) return merge_request if merge_request.errors.any? return merge_request if new_ids.size != update_attrs[:assignee_ids].size @@ -32,27 +32,8 @@ module MergeRequests private - def new_assignee_ids(merge_request) - # prime the cache - prevent N+1 lookup during authorization loop. - user_ids = update_attrs[:assignee_ids] - return [] if user_ids.empty? - - merge_request.project.team.max_member_access_for_user_ids(user_ids) - User.id_in(user_ids).map do |user| - if user.can?(:read_merge_request, merge_request) - user.id - else - merge_request.errors.add( - :assignees, - "Cannot assign #{user.to_reference} to #{merge_request.to_reference}" - ) - nil - end - end.compact - end - def assignee_ids - params.fetch(:assignee_ids).reject { _1 == 0 }.first(1) + filter_sentinel_values(params.fetch(:assignee_ids)).first(1) end def params diff --git a/app/services/merge_requests/update_reviewers_service.rb b/app/services/merge_requests/update_reviewers_service.rb new file mode 100644 index 00000000000..8e974d75676 --- /dev/null +++ b/app/services/merge_requests/update_reviewers_service.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module MergeRequests + class UpdateReviewersService < UpdateService + def execute(merge_request) + return merge_request unless current_user&.can?(:update_merge_request, merge_request) + + old_reviewers = merge_request.reviewers.to_a + old_ids = old_reviewers.map(&:id) + new_ids = new_user_ids(merge_request, update_attrs[:reviewer_ids], :reviewers) + + return merge_request if merge_request.errors.any? + return merge_request if new_ids.size != update_attrs[:reviewer_ids].size + return merge_request if old_ids.to_set == new_ids.to_set # no-change + + merge_request.update!(update_attrs.merge(reviewer_ids: new_ids)) + handle_reviewers_change(merge_request, old_reviewers) + resolve_todos_for(merge_request) + execute_reviewers_hooks(merge_request, old_reviewers) + + merge_request + end + + private + + def reviewer_ids + filter_sentinel_values(params.fetch(:reviewer_ids)).first(1) + end + + def update_attrs + @attrs ||= { updated_by: current_user, reviewer_ids: reviewer_ids } + end + + def execute_reviewers_hooks(merge_request, old_reviewers) + execute_hooks( + merge_request, + 'update', + old_associations: { reviewers: old_reviewers } + ) + end + end +end + +MergeRequests::UpdateReviewersService.prepend_mod diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 603da4ef535..0902b5195a1 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -155,11 +155,7 @@ module MergeRequests def resolve_todos(merge_request, old_labels, old_assignees, old_reviewers) return unless has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees, old_reviewers: old_reviewers) - service_user = current_user - - merge_request.run_after_commit_or_now do - ::MergeRequests::ResolveTodosService.new(merge_request, service_user).async_execute - end + resolve_todos_for(merge_request) end def handle_target_branch_change(merge_request) @@ -296,6 +292,36 @@ module MergeRequests def add_time_spent_service @add_time_spent_service ||= ::MergeRequests::AddSpentTimeService.new(project: project, current_user: current_user, params: params) end + + def new_user_ids(merge_request, user_ids, attribute) + # prime the cache - prevent N+1 lookup during authorization loop. + return [] if user_ids.empty? + + merge_request.project.team.max_member_access_for_user_ids(user_ids) + User.id_in(user_ids).map do |user| + if user.can?(:read_merge_request, merge_request) + user.id + else + merge_request.errors.add( + attribute, + "Cannot assign #{user.to_reference} to #{merge_request.to_reference}" + ) + nil + end + end.compact + end + + def resolve_todos_for(merge_request) + service_user = current_user + + merge_request.run_after_commit_or_now do + ::MergeRequests::ResolveTodosService.new(merge_request, service_user).async_execute + end + end + + def filter_sentinel_values(param) + param.reject { _1 == 0 } + end end end diff --git a/app/services/notes/build_service.rb b/app/services/notes/build_service.rb index 8c250526efc..cc5c81cf280 100644 --- a/app/services/notes/build_service.rb +++ b/app/services/notes/build_service.rb @@ -16,6 +16,14 @@ module Notes params.merge!(discussion.reply_attributes) end + # The `confidential` param for notes is deprecated with 15.3 + # and renamed to `internal`. + # We still accept `confidential` until the param gets removed from the API. + # Until we have not migrated the database column to `internal` we need to rename + # the parameter. Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/367923. + params[:confidential] = params[:internal] || params[:confidential] + params.delete(:internal) + new_note(params, discussion) end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 4074b1d1182..b7e6a50fa5c 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -88,8 +88,13 @@ module Notes return if quick_actions_service.commands_executed_count.to_i == 0 if update_params.present? - quick_actions_service.apply_updates(update_params, note) - note.commands_changes = update_params + if check_for_reviewer_validity(message, update_params) + quick_actions_service.apply_updates(update_params, note) + note.commands_changes = update_params + else + message = "Reviewers #{MergeRequest.max_number_of_assignees_or_reviewers_message}" + note.errors.add(:validation, message) + end end # We must add the error after we call #save because errors are reset @@ -109,6 +114,18 @@ module Notes } end + def check_for_reviewer_validity(message, update_params) + return true unless Feature.enabled?(:limit_reviewer_and_assignee_size) + + if update_params.key?(:reviewer_ids) + possible_reviewers = update_params[:reviewer_ids]&.uniq&.size + + return false if possible_reviewers > MergeRequest::MAX_NUMBER_OF_ASSIGNEES_OR_REVIEWERS + end + + true + end + def track_event(note, user) track_note_creation_usage_for_issues(note) if note.for_issue? track_note_creation_usage_for_merge_requests(note) if note.for_merge_request? @@ -130,7 +147,8 @@ module Notes end def track_note_creation_usage_for_issues(note) - Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_added_action(author: note.author) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_added_action(author: note.author, + project: project) end def track_note_creation_usage_for_merge_requests(note) diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index c25b1ab0379..eda8bbcbc2e 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -15,7 +15,8 @@ module Notes private def track_note_removal_usage_for_issues(note) - Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_removed_action(author: note.author) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_removed_action(author: note.author, + project: project) end def track_note_removal_usage_for_merge_requests(note) diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 04fc4c7c944..2dae76feb0b 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -86,7 +86,8 @@ module Notes end def track_note_edit_usage_for_issues(note) - Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_edited_action(author: note.author) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_edited_action(author: note.author, + project: project) end def track_note_edit_usage_for_merge_requests(note) diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb index e63e19e365c..bdeebc641b8 100644 --- a/app/services/notification_recipients/build_service.rb +++ b/app/services/notification_recipients/build_service.rb @@ -36,9 +36,5 @@ module NotificationRecipients def self.build_requested_review_recipients(*args) ::NotificationRecipients::Builder::RequestReview.new(*args).notification_recipients end - - def self.build_attention_requested_recipients(*args) - ::NotificationRecipients::Builder::AttentionRequested.new(*args).notification_recipients - end end end diff --git a/app/services/notification_recipients/builder/attention_requested.rb b/app/services/notification_recipients/builder/attention_requested.rb deleted file mode 100644 index cdc371fcece..00000000000 --- a/app/services/notification_recipients/builder/attention_requested.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -module NotificationRecipients - module Builder - class AttentionRequested < Base - attr_reader :merge_request, :current_user, :user - - def initialize(merge_request, current_user, user) - @merge_request = merge_request - @current_user = current_user - @user = user - end - - def target - merge_request - end - - def build! - add_recipients(user, :mention, NotificationReason::ATTENTION_REQUESTED) - end - end - end -end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 2477fcd02e5..5a92adfd25a 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -333,14 +333,6 @@ class NotificationService end end - def attention_requested_of_merge_request(merge_request, current_user, user) - recipients = NotificationRecipients::BuildService.build_attention_requested_recipients(merge_request, current_user, user) - - recipients.each do |recipient| - mailer.attention_requested_merge_request_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later - end - end - # When we add labels to a merge request we should send an email to: # # * watchers of the mr's labels @@ -799,7 +791,7 @@ class NotificationService end recipients = NotificationRecipients::BuildService.build_recipients(target, current_user, action: "new") - recipients = recipients.select {|r| new_mentioned_users.include?(r.user) } + recipients = recipients.select { |r| new_mentioned_users.include?(r.user) } recipients.each do |recipient| mailer.send(method, recipient.user.id, target.id, current_user.id, recipient.reason).deliver_later diff --git a/app/services/packages/conan/create_package_file_service.rb b/app/services/packages/conan/create_package_file_service.rb index 1bde9606492..904a1d10bcb 100644 --- a/app/services/packages/conan/create_package_file_service.rb +++ b/app/services/packages/conan/create_package_file_service.rb @@ -13,11 +13,11 @@ module Packages def execute package_file = package.package_files.build( - file: file, - size: params['file.size'], + file: file, + size: params['file.size'], file_name: params[:file_name], file_sha1: params['file.sha1'], - file_md5: params['file.md5'], + file_md5: params['file.md5'], conan_file_metadatum_attributes: { recipe_revision: params[:recipe_revision], package_revision: params[:package_revision], diff --git a/app/services/packages/create_package_file_service.rb b/app/services/packages/create_package_file_service.rb index 5723b0b4717..6e1a5672a52 100644 --- a/app/services/packages/create_package_file_service.rb +++ b/app/services/packages/create_package_file_service.rb @@ -10,12 +10,12 @@ module Packages def execute package_file = package.package_files.build( - file: params[:file], - size: params[:size], - file_name: params[:file_name], - file_sha1: params[:file_sha1], + file: params[:file], + size: params[:size], + file_name: params[:file_name], + file_sha1: params[:file_sha1], file_sha256: params[:file_sha256], - file_md5: params[:file_md5] + file_md5: params[:file_md5] ) if params[:build].present? diff --git a/app/services/packages/debian/create_package_file_service.rb b/app/services/packages/debian/create_package_file_service.rb index fbbc8159ca0..53275fdc9bb 100644 --- a/app/services/packages/debian/create_package_file_service.rb +++ b/app/services/packages/debian/create_package_file_service.rb @@ -17,12 +17,12 @@ module Packages # Debian package file are first uploaded to incoming with empty metadata, # and are moved later by Packages::Debian::ProcessChangesService package.package_files.create!( - file: params[:file], - size: params[:file]&.size, - file_name: params[:file_name], - file_sha1: params[:file_sha1], + file: params[:file], + size: params[:file]&.size, + file_name: params[:file_name], + file_sha1: params[:file_sha1], file_sha256: params[:file]&.sha256, - file_md5: params[:file_md5], + file_md5: params[:file_md5], debian_file_metadatum_attributes: { file_type: 'unknown', architecture: nil, diff --git a/app/services/packages/debian/extract_metadata_service.rb b/app/services/packages/debian/extract_metadata_service.rb index f94587919b9..eb8227d1296 100644 --- a/app/services/packages/debian/extract_metadata_service.rb +++ b/app/services/packages/debian/extract_metadata_service.rb @@ -61,12 +61,12 @@ module Packages def fields strong_memoize(:fields) do if file_type_debian? - package_file.file.use_file do |file_path| - ::Packages::Debian::ExtractDebMetadataService.new(file_path).execute + package_file.file.use_open_file(unlink_early: false) do |file| + ::Packages::Debian::ExtractDebMetadataService.new(file.file_path).execute end elsif file_type_meta? - package_file.file.use_file do |file_path| - ::Packages::Debian::ParseDebian822Service.new(File.read(file_path)).execute.each_value.first + package_file.file.use_open_file do |file| + ::Packages::Debian::ParseDebian822Service.new(file.read).execute.each_value.first end end end diff --git a/app/services/packages/npm/create_package_service.rb b/app/services/packages/npm/create_package_service.rb index b0a5f37cfa3..a3596314199 100644 --- a/app/services/packages/npm/create_package_service.rb +++ b/app/services/packages/npm/create_package_service.rb @@ -87,11 +87,11 @@ module Packages def file_params { - file: CarrierWaveStringFile.new(Base64.decode64(attachment['data'])), - size: calculated_package_file_size, + file: CarrierWaveStringFile.new(Base64.decode64(attachment['data'])), + size: calculated_package_file_size, file_sha1: version_data[:dist][:shasum], file_name: package_file_name, - build: params[:build] + build: params[:build] } end diff --git a/app/services/projects/alerting/notify_service.rb b/app/services/projects/alerting/notify_service.rb index e5d40b60747..c21a61bcb52 100644 --- a/app/services/projects/alerting/notify_service.rb +++ b/app/services/projects/alerting/notify_service.rb @@ -32,7 +32,7 @@ module Projects attr_reader :project, :payload, :integration def valid_payload_size? - Gitlab::Utils::DeepSize.new(payload).valid? + Gitlab::Utils::DeepSize.new(payload.to_h).valid? end override :alert_source diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 9bc8bb428fb..6381ee67ce7 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -26,7 +26,7 @@ module Projects return ::Projects::CreateFromTemplateService.new(current_user, params).execute end - @project = Project.new(params) + @project = Project.new(params.merge(creator: current_user)) validate_import_source_enabled! @@ -45,20 +45,14 @@ module Projects set_project_name_from_path # get namespace id - namespace_id = params[:namespace_id] - - if namespace_id - # Find matching namespace and check if it allowed - # for current user if namespace_id passed. - unless current_user.can?(:create_projects, parent_namespace) - @project.namespace_id = nil - deny_namespace - return @project - end - else - # Set current user namespace if namespace_id is nil - @project.namespace_id = current_user.namespace_id - end + namespace_id = params[:namespace_id] || current_user.namespace_id + @project.namespace_id = namespace_id.to_i + + @project.check_personal_projects_limit + return @project if @project.errors.any? + + validate_create_permissions + return @project if @project.errors.any? @relations_block&.call(@project) yield(@project) if block_given? @@ -92,7 +86,9 @@ module Projects protected - def deny_namespace + def validate_create_permissions + return if current_user.can?(:create_projects, parent_namespace) + @project.errors.add(:namespace, "is not valid") end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 70a04cd556a..5fce816064b 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -46,15 +46,15 @@ module Projects def new_fork_params new_params = { - forked_from_project: @project, - visibility_level: target_visibility_level, - description: target_description, - name: target_name, - path: target_path, - shared_runners_enabled: @project.shared_runners_enabled, - namespace_id: target_namespace.id, - fork_network: fork_network, - ci_config_path: @project.ci_config_path, + forked_from_project: @project, + visibility_level: target_visibility_level, + description: target_description, + name: target_name, + path: target_path, + shared_runners_enabled: @project.shared_runners_enabled, + namespace_id: target_namespace.id, + fork_network: fork_network, + ci_config_path: @project.ci_config_path, # We need to set ci_default_git_depth to 0 for the forked project when # @project.ci_default_git_depth is nil in order to keep the same behaviour # and not get ProjectCiCdSetting::DEFAULT_GIT_DEPTH set on create @@ -63,8 +63,8 @@ module Projects # been instantiated to avoid ActiveRecord trying to create it when # initializing the project, as that would cause a foreign key constraint # exception. - relations_block: -> (project) { build_fork_network_member(project) }, - skip_disk_validation: skip_disk_validation, + relations_block: -> (project) { build_fork_network_member(project) }, + skip_disk_validation: skip_disk_validation, external_authorization_classification_label: @project.external_authorization_classification_label, suggestion_commit_message: @project.suggestion_commit_message, merge_commit_template: @project.merge_commit_template, diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index d8d35422590..ddbcfbb675c 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -54,15 +54,21 @@ module Projects end def save_all! + log_info('Project export started') + if save_exporters && save_export_archive - notify_success + log_info('Project successfully exported') else notify_error! end end def save_exporters - exporters.all?(&:save) + exporters.all? do |exporter| + log_info("#{exporter.class.name} saver started") + + exporter.save + end end def save_export_archive @@ -78,11 +84,12 @@ module Projects end def project_tree_saver - @project_tree_saver ||= tree_saver_class.new(project: project, - current_user: current_user, - shared: shared, - params: params, - logger: logger) + @project_tree_saver ||= tree_saver_class.new( + project: project, + current_user: current_user, + shared: shared, + params: params, + logger: logger) end def tree_saver_class @@ -127,11 +134,10 @@ module Projects raise Gitlab::ImportExport::Error, shared.errors.to_sentence end - def notify_success + def log_info(message) logger.info( - message: 'Project successfully exported', - project_name: project.name, - project_id: project.id + message: message, + **log_base_data ) end @@ -139,8 +145,7 @@ module Projects logger.error( message: 'Project export error', export_errors: shared.errors.join(', '), - project_name: project.name, - project_id: project.id + **log_base_data ) user = current_user @@ -150,6 +155,10 @@ module Projects NotificationService.new.project_not_exported(project, user, errors) end end + + def log_base_data + @log_base_data ||= Gitlab::ImportExport::LogUtil.exportable_to_log_payload(project) + end end end end diff --git a/app/services/projects/import_export/relation_export_service.rb b/app/services/projects/import_export/relation_export_service.rb new file mode 100644 index 00000000000..dce40cf18ba --- /dev/null +++ b/app/services/projects/import_export/relation_export_service.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module Projects + module ImportExport + class RelationExportService + include Gitlab::ImportExport::CommandLineUtil + + def initialize(relation_export, jid) + @relation_export = relation_export + @jid = jid + @logger = Gitlab::Export::Logger.build + end + + def execute + relation_export.update!(status_event: :start, jid: jid) + + mkdir_p(shared.export_path) + mkdir_p(shared.archive_path) + + if relation_saver.save + compress_export_path + upload_compressed_file + relation_export.finish! + else + fail_export(shared.errors.join(', ')) + end + rescue StandardError => e + fail_export(e.message) + ensure + FileUtils.remove_entry(shared.export_path) if File.exist?(shared.export_path) + FileUtils.remove_entry(shared.archive_path) if File.exist?(shared.archive_path) + end + + private + + attr_reader :relation_export, :jid, :logger + + delegate :relation, :project_export_job, to: :relation_export + delegate :project, to: :project_export_job + + def shared + project.import_export_shared + end + + def relation_saver + case relation + when Projects::ImportExport::RelationExport::UPLOADS_RELATION + Gitlab::ImportExport::UploadsSaver.new(project: project, shared: shared) + when Projects::ImportExport::RelationExport::REPOSITORY_RELATION + Gitlab::ImportExport::RepoSaver.new(exportable: project, shared: shared) + when Projects::ImportExport::RelationExport::WIKI_REPOSITORY_RELATION + Gitlab::ImportExport::WikiRepoSaver.new(exportable: project, shared: shared) + when Projects::ImportExport::RelationExport::LFS_OBJECTS_RELATION + Gitlab::ImportExport::LfsSaver.new(project: project, shared: shared) + when Projects::ImportExport::RelationExport::SNIPPETS_REPOSITORY_RELATION + Gitlab::ImportExport::SnippetsRepoSaver.new(project: project, shared: shared, current_user: nil) + when Projects::ImportExport::RelationExport::DESIGN_REPOSITORY_RELATION + Gitlab::ImportExport::DesignRepoSaver.new(exportable: project, shared: shared) + else + Gitlab::ImportExport::Project::RelationSaver.new( + project: project, + shared: shared, + relation: relation + ) + end + end + + def upload_compressed_file + upload = relation_export.build_upload + File.open(archive_file_full_path) { |file| upload.export_file = file } + upload.save! + end + + def compress_export_path + tar_czf(archive: archive_file_full_path, dir: shared.export_path) + end + + def archive_file_full_path + @archive_file ||= File.join(shared.archive_path, "#{relation}.tar.gz") + end + + def fail_export(error_message) + relation_export.update!(status_event: :fail_op, export_error: error_message.truncate(300)) + + logger.error( + message: 'Project relation export failed', + export_error: error_message, + project_export_job_id: project_export_job.id, + project_name: project.name, + project_id: project.id + ) + end + end + end +end diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index c032fbf1508..eaf73b78c1c 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -50,7 +50,7 @@ module Projects def find_or_create_lfs_object(tmp_file) lfs_obj = LfsObject.safe_find_or_create_by!( - oid: lfs_oid, + oid: lfs_oid, size: lfs_size ) diff --git a/app/services/projects/prometheus/alerts/notify_service.rb b/app/services/projects/prometheus/alerts/notify_service.rb index bc517ee3d6f..6265a74fad2 100644 --- a/app/services/projects/prometheus/alerts/notify_service.rb +++ b/app/services/projects/prometheus/alerts/notify_service.rb @@ -56,7 +56,7 @@ module Projects attr_reader :project, :payload def valid_payload_size? - Gitlab::Utils::DeepSize.new(payload).valid? + Gitlab::Utils::DeepSize.new(payload.to_h).valid? end def max_alerts_exceeded? diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 666227951c6..3cb5a564ba5 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -121,6 +121,8 @@ module Projects # Overridden in EE def post_update_hooks(project) ensure_personal_project_owner_membership(project) + + publish_event end # Overridden in EE @@ -268,6 +270,18 @@ module Projects CustomerRelations::IssueContact.delete_for_project(project.id) end + + def publish_event + event = ::Projects::ProjectTransferedEvent.new(data: { + project_id: project.id, + old_namespace_id: old_namespace.id, + old_root_namespace_id: old_namespace.root_ancestor.id, + new_namespace_id: new_namespace.id, + new_root_namespace_id: new_namespace.root_ancestor.id + }) + + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index 705d23ec704..f686f14b5b3 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -76,11 +76,11 @@ module Projects if message.present? Gitlab::AppJsonLogger.info(message: "Error synching remote mirror", - project_id: project.id, - project_path: project.full_path, - remote_mirror_id: remote_mirror.id, - lfs_sync_failed: lfs_sync_failed, - divergent_ref_list: response.divergent_refs) + project_id: project.id, + project_path: project.full_path, + remote_mirror_id: remote_mirror.id, + lfs_sync_failed: lfs_sync_failed, + divergent_ref_list: response.divergent_refs) end [failed, message] diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 5708421014a..d757b0700b9 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -121,6 +121,8 @@ module Projects end update_pending_builds if runners_settings_toggled? + + publish_event end def after_rename_service(project) @@ -209,6 +211,18 @@ module Projects [] end end + + def publish_event + return unless project.archived_previously_changed? + + event = Projects::ProjectArchivedEvent.new(data: { + project_id: @project.id, + namespace_id: @project.namespace_id, + root_namespace_id: @project.root_namespace.id + }) + + Gitlab::EventStore.publish(event) + end end end diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index f48e02ab4b5..d26c1b148bf 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -13,5 +13,9 @@ module ProtectedBranches def after_execute(*) # overridden in EE::ProtectedBranches module end + + def refresh_cache + CacheService.new(@project, @current_user, @params).refresh + end end end diff --git a/app/services/protected_branches/cache_service.rb b/app/services/protected_branches/cache_service.rb new file mode 100644 index 00000000000..8c521f4ebcb --- /dev/null +++ b/app/services/protected_branches/cache_service.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module ProtectedBranches + class CacheService < ProtectedBranches::BaseService + CACHE_ROOT_KEY = 'cache:gitlab:protected_branch' + TTL_UNSET = -1 + CACHE_EXPIRE_IN = 1.day + CACHE_LIMIT = 1000 + + def fetch(ref_name, dry_run: false) + record = OpenSSL::Digest::SHA256.hexdigest(ref_name) + + Gitlab::Redis::Cache.with do |redis| + cached_result = redis.hget(redis_key, record) + + decoded_result = Gitlab::Redis::Boolean.decode(cached_result) unless cached_result.nil? + + # If we're dry-running, don't break because we need to check against + # the real value to ensure the cache is working properly. + # If the result is nil we'll need to run the block, so don't break yet. + break decoded_result unless dry_run || decoded_result.nil? + + calculated_value = yield + + check_and_log_discrepancy(decoded_result, calculated_value, ref_name) if dry_run + + redis.hset(redis_key, record, Gitlab::Redis::Boolean.encode(calculated_value)) + + # We don't want to extend cache expiration time + if redis.ttl(redis_key) == TTL_UNSET + redis.expire(redis_key, CACHE_EXPIRE_IN) + end + + # If the cache record has too many elements, then something went wrong and + # it's better to drop the cache key. + if redis.hlen(redis_key) > CACHE_LIMIT + redis.unlink(redis_key) + end + + calculated_value + end + end + + def refresh + Gitlab::Redis::Cache.with { |redis| redis.unlink(redis_key) } + end + + private + + def check_and_log_discrepancy(cached_value, real_value, ref_name) + return if cached_value.nil? + return if cached_value == real_value + + encoded_ref_name = Gitlab::EncodingHelper.encode_utf8_with_replacement_character(ref_name) + + log_error( + 'class' => self.class.name, + 'message' => "Cache mismatch '#{encoded_ref_name}': cached value: #{cached_value}, real value: #{real_value}", + 'project_id' => @project.id, + 'project_path' => @project.full_path + ) + end + + def redis_key + @redis_key ||= [CACHE_ROOT_KEY, @project.id].join(':') + end + end +end diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index dada449989a..903addf7afc 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -7,6 +7,8 @@ module ProtectedBranches save_protected_branch + refresh_cache + protected_branch end diff --git a/app/services/protected_branches/destroy_service.rb b/app/services/protected_branches/destroy_service.rb index 47332ace417..01d3b68314f 100644 --- a/app/services/protected_branches/destroy_service.rb +++ b/app/services/protected_branches/destroy_service.rb @@ -5,7 +5,7 @@ module ProtectedBranches def execute(protected_branch) raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_protected_branch, protected_branch) - protected_branch.destroy + protected_branch.destroy.tap { refresh_cache } end end end diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb index 1e70f2d9793..c155e0022f5 100644 --- a/app/services/protected_branches/update_service.rb +++ b/app/services/protected_branches/update_service.rb @@ -10,6 +10,8 @@ module ProtectedBranches if protected_branch.update(params) after_execute(protected_branch: protected_branch, old_merge_access_levels: old_merge_access_levels, old_push_access_levels: old_push_access_levels) + + refresh_cache end protected_branch diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index e3134070231..2588d2187a5 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -19,10 +19,6 @@ module Releases create_release(tag, evidence_pipeline) end - def find_or_build_release - release || build_release(existing_tag) - end - private def ensure_tag diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb index 03ac839c509..04f917ec8ef 100644 --- a/app/services/resource_events/change_labels_service.rb +++ b/app/services/resource_events/change_labels_service.rb @@ -24,6 +24,9 @@ module ResourceEvents end ApplicationRecord.legacy_bulk_insert(ResourceLabelEvent.table_name, labels) # rubocop:disable Gitlab/BulkInsert + + create_timeline_events_from(added_labels: added_labels, removed_labels: removed_labels) + resource.expire_note_etag_cache Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_label_changed_action(author: user) if resource.is_a?(Issue) @@ -41,6 +44,17 @@ module ResourceEvents raise ArgumentError, "Unknown resource type #{resource.class.name}" end end + + def create_timeline_events_from(added_labels: [], removed_labels: []) + return unless resource.incident? + + IncidentManagement::TimelineEvents::CreateService.change_labels( + resource, + user, + added_labels: added_labels, + removed_labels: removed_labels + ) + end end end diff --git a/app/services/security/ci_configuration/sast_parser_service.rb b/app/services/security/ci_configuration/sast_parser_service.rb index cae9a90f0a0..16a9efcefdf 100644 --- a/app/services/security/ci_configuration/sast_parser_service.rb +++ b/app/services/security/ci_configuration/sast_parser_service.rb @@ -75,7 +75,11 @@ module Security def sast_excluded_analyzers strong_memoize(:sast_excluded_analyzers) do excluded_analyzers = gitlab_ci_yml_attributes["SAST_EXCLUDED_ANALYZERS"] || sast_template_attributes["SAST_EXCLUDED_ANALYZERS"] - excluded_analyzers.split(',').map(&:strip) rescue [] + begin + excluded_analyzers.split(',').map(&:strip) + rescue StandardError + [] + end end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index d7e4b53b5de..9de73a00eac 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -57,7 +57,7 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: noteable, project: noteable.project, author: user).unrelate_issuable(noteable_ref) end - # Called when the due_date of a Noteable is changed + # Called when the due_date or start_date of a Noteable is changed # # noteable - Noteable object # project - Project owning noteable @@ -68,11 +68,15 @@ module SystemNoteService # # "removed due date" # - # "changed due date to September 20, 2018" + # "changed due date to September 20, 2018 and changed start date to September 25, 2018" # # Returns the created Note object - def change_due_date(noteable, project, author, due_date) - ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_due_date(due_date) + def change_start_date_or_due_date(noteable, project, author, changed_dates) + ::SystemNotes::TimeTrackingService.new( + noteable: noteable, + project: project, + author: author + ).change_start_date_or_due_date(changed_dates) end # Called when the estimated time of a Noteable is changed @@ -111,6 +115,24 @@ module SystemNoteService ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent end + # Called when a timelog is added to an issuable + # + # issuable - Issuable object (Issue, WorkItem or MergeRequest) + # project - Project owning the issuable + # author - User performing the change + # timelog - Created timelog + # + # Example Note text: + # + # "subtracted 1h 15m of time spent" + # + # "added 2h 30m of time spent" + # + # Returns the created Note object + def created_timelog(issuable, project, author, timelog) + ::SystemNotes::TimeTrackingService.new(noteable: issuable, project: project, author: author).created_timelog(timelog) + end + # Called when a timelog is removed from a Noteable # # noteable - Noteable object @@ -134,14 +156,6 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).change_status(status, source) end - def request_attention(noteable, project, author, user) - ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).request_attention(user) - end - - def remove_attention_request(noteable, project, author, user) - ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).remove_attention_request(user) - end - # Called when 'merge when pipeline succeeds' is executed def merge_when_pipeline_succeeds(noteable, project, author, sha) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).merge_when_pipeline_succeeds(sha) @@ -256,8 +270,8 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).noteable_moved(noteable_ref, direction) end - def noteable_cloned(noteable, project, noteable_ref, author, direction:) - ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).noteable_cloned(noteable_ref, direction) + def noteable_cloned(noteable, project, noteable_ref, author, direction:, created_at: nil) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).noteable_cloned(noteable_ref, direction, created_at: created_at) end def mark_duplicate_issue(noteable, project, author, canonical_issue) @@ -280,6 +294,18 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: mentioned).cross_reference_disallowed?(mentioned_in) end + def relate_work_item(noteable, work_item, user) + ::SystemNotes::IssuablesService + .new(noteable: noteable, project: noteable.project, author: user) + .hierarchy_changed(work_item, 'relate') + end + + def unrelate_work_item(noteable, work_item, user) + ::SystemNotes::IssuablesService + .new(noteable: noteable, project: noteable.project, author: user) + .hierarchy_changed(work_item, 'unrelate') + end + def zoom_link_added(issue, project, author) ::SystemNotes::ZoomService.new(noteable: issue, project: project, author: author).zoom_link_added end diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index f9e5c3725d8..75903fde39e 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -178,6 +178,24 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'title')) end + # Called when the hierarchy of a work item is changed + # + # noteable - Noteable object that responds to `work_item_parent` and `work_item_children` + # project - Project owning noteable + # author - User performing the change + # + # Example Note text: + # + # "added #1 as child Task" + # + # Returns the created Note object + def hierarchy_changed(work_item, action) + params = hierarchy_note_params(action, noteable, work_item) + + create_note(NoteSummary.new(noteable, project, author, params[:parent_note_body], action: params[:parent_action])) + create_note(NoteSummary.new(work_item, project, author, params[:child_note_body], action: params[:child_action])) + end + # Called when the description of a Noteable is changed # # noteable - Noteable object that responds to `description` @@ -255,12 +273,12 @@ module SystemNotes # # Example Note text: # - # "marked the task Whatever as completed." + # "marked the checklist item Whatever as completed." # # Returns the created Note object def change_task_status(new_task) status_label = new_task.complete? ? Taskable::COMPLETED : Taskable::INCOMPLETE - body = "marked the task **#{new_task.source}** as #{status_label}" + body = "marked the checklist item **#{new_task.source}** as #{status_label}" issue_activity_counter.track_issue_description_changed_action(author: author) if noteable.is_a?(Issue) @@ -294,13 +312,14 @@ module SystemNotes # # noteable_ref - Referenced noteable # direction - symbol, :to or :from + # created_at - timestamp for the system note, defaults to current time # # Example Note text: # # "cloned to some_namespace/project_new#11" # # Returns the created Note object - def noteable_cloned(noteable_ref, direction) + def noteable_cloned(noteable_ref, direction, created_at: nil) unless [:to, :from].include?(direction) raise ArgumentError, "Invalid direction `#{direction}`" end @@ -308,9 +327,11 @@ module SystemNotes cross_reference = noteable_ref.to_reference(project) body = "cloned #{direction} #{cross_reference}" - issue_activity_counter.track_issue_cloned_action(author: author) if noteable.is_a?(Issue) && direction == :to + if noteable.is_a?(Issue) && direction == :to + issue_activity_counter.track_issue_cloned_action(author: author, project: project) + end - create_note(NoteSummary.new(noteable, project, author, body, action: 'cloned')) + create_note(NoteSummary.new(noteable, project, author, body, action: 'cloned', created_at: created_at)) end # Called when the confidentiality changes @@ -367,36 +388,6 @@ module SystemNotes existing_mentions_for(mentioned_in, noteable, notes).exists? end - # Called when a user's attention has been requested for a Notable - # - # user - User's whos attention has been requested - # - # Example Note text: - # - # "requested attention from @eli.wisoky" - # - # Returns the created Note object - def request_attention(user) - body = "requested attention from #{user.to_reference}" - - create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_requested')) - end - - # Called when a user's attention request has been removed for a Notable - # - # user - User's whos attention request has been removed - # - # Example Note text: - # - # "removed attention request from @eli.wisoky" - # - # Returns the created Note object - def remove_attention_request(user) - body = "removed attention request from #{user.to_reference}" - - create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_request_removed')) - end - # Called when a Noteable has been marked as the canonical Issue of a duplicate # # duplicate_issue - Issue that was a duplicate of this @@ -506,6 +497,29 @@ module SystemNotes def track_cross_reference_action issue_activity_counter.track_issue_cross_referenced_action(author: author) if noteable.is_a?(Issue) end + + def hierarchy_note_params(action, parent, child) + return {} unless child && parent + + child_type = child.issue_type.humanize(capitalize: false) + parent_type = parent.issue_type.humanize(capitalize: false) + + if action == 'relate' + { + parent_note_body: "added #{child.to_reference} as child #{child_type}", + child_note_body: "added #{parent.to_reference} as parent #{parent_type}", + parent_action: 'relate_to_child', + child_action: 'relate_to_parent' + } + else + { + parent_note_body: "removed child #{child_type} #{child.to_reference}", + child_note_body: "removed parent #{parent_type} #{parent.to_reference}", + parent_action: 'unrelate_from_child', + child_action: 'unrelate_from_parent' + } + end + end end end diff --git a/app/services/system_notes/time_tracking_service.rb b/app/services/system_notes/time_tracking_service.rb index a9b1f6d3d37..68df52a03c7 100644 --- a/app/services/system_notes/time_tracking_service.rb +++ b/app/services/system_notes/time_tracking_service.rb @@ -2,8 +2,9 @@ module SystemNotes class TimeTrackingService < ::SystemNotes::BaseService - # Called when the due_date of a Noteable is changed + # Called when the start_date or due_date of an Issue/WorkItem is changed # + # start_date - Start date being assigned, or nil # due_date - Due date being assigned, or nil # # Example Note text: @@ -11,14 +12,23 @@ module SystemNotes # "removed due date" # # "changed due date to September 20, 2018" + + # "changed start date to September 20, 2018 and changed due date to September 25, 2018" # # Returns the created Note object - def change_due_date(due_date) - body = due_date ? "changed due date to #{due_date.to_s(:long)}" : 'removed due date' + def change_start_date_or_due_date(changed_dates = {}) + return if changed_dates.empty? + + # Using instance_of because WorkItem < Issue. We don't want to track work item updates as issue updates + if noteable.instance_of?(Issue) && changed_dates.key?('due_date') + issue_activity_counter.track_issue_due_date_changed_action(author: author) + end - issue_activity_counter.track_issue_due_date_changed_action(author: author) if noteable.is_a?(Issue) + work_item_activity_counter.track_work_item_date_changed_action(author: author) if noteable.is_a?(WorkItem) - create_note(NoteSummary.new(noteable, project, author, body, action: 'due_date')) + create_note( + NoteSummary.new(noteable, project, author, changed_date_body(changed_dates), action: 'start_date_or_due_date') + ) end # Called when the estimated time of a Noteable is changed @@ -76,6 +86,32 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) end + # Called when a timelog is added to an issuable + # + # timelog - Added timelog + # + # Example Note text: + # + # "subtracted 1h 15m of time spent" + # + # "added 2h 30m of time spent" + # + # Returns the created Note object + def created_timelog(timelog) + time_spent = timelog.time_spent + spent_at = timelog.spent_at&.to_date + parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs) + action = time_spent > 0 ? 'added' : 'subtracted' + + text_parts = ["#{action} #{parsed_time} of time spent"] + text_parts << "at #{spent_at}" if spent_at && spent_at != DateTime.current.to_date + body = text_parts.join(' ') + + issue_activity_counter.track_issue_time_spent_changed_action(author: author) if noteable.is_a?(Issue) + + create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) + end + def remove_timelog(timelog) time_spent = timelog.time_spent spent_at = timelog.spent_at&.to_date @@ -90,8 +126,33 @@ module SystemNotes private + def changed_date_body(changed_dates) + %w[start_date due_date].each_with_object([]) do |date_field, word_array| + next unless changed_dates.key?(date_field) + + word_array << 'and' if word_array.any? + + word_array << message_for_changed_date(changed_dates, date_field) + end.join(' ') + end + + def message_for_changed_date(changed_dates, date_key) + changed_date = changed_dates[date_key].last + readable_date = date_key.humanize.downcase + + if changed_date.nil? + "removed #{readable_date}" + else + "changed #{readable_date} to #{changed_date.to_s(:long)}" + end + end + def issue_activity_counter Gitlab::UsageDataCounters::IssueActivityUniqueCounter end + + def work_item_activity_counter + Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter + end end end diff --git a/app/services/timelogs/base_service.rb b/app/services/timelogs/base_service.rb index be46c26e047..e09264864fd 100644 --- a/app/services/timelogs/base_service.rb +++ b/app/services/timelogs/base_service.rb @@ -5,11 +5,26 @@ module Timelogs include BaseServiceUtility include Gitlab::Utils::StrongMemoize - attr_accessor :timelog, :current_user + attr_accessor :current_user - def initialize(timelog, user) - @timelog = timelog + def initialize(user) @current_user = user end + + def success(timelog) + ServiceResponse.success(payload: { + timelog: timelog + }) + end + + def error(message, http_status = nil) + ServiceResponse.error(message: message, http_status: http_status) + end + + def error_in_save(timelog) + return error(_("Failed to save timelog")) if timelog.errors.empty? + + error(timelog.errors.full_messages.to_sentence) + end end end diff --git a/app/services/timelogs/create_service.rb b/app/services/timelogs/create_service.rb new file mode 100644 index 00000000000..12181cec20a --- /dev/null +++ b/app/services/timelogs/create_service.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Timelogs + class CreateService < Timelogs::BaseService + attr_accessor :issuable, :time_spent, :spent_at, :summary + + def initialize(issuable, time_spent, spent_at, summary, user) + super(user) + + @issuable = issuable + @time_spent = time_spent + @spent_at = spent_at + @summary = summary + end + + def execute + unless can?(current_user, :create_timelog, issuable) + return error( + _("%{issuable_class_name} doesn't exist or you don't have permission to add timelog to it.") % { + issuable_class_name: issuable.nil? ? 'Issuable' : issuable.base_class_name + }, 404) + end + + issue = issuable if issuable.is_a?(Issue) + merge_request = issuable if issuable.is_a?(MergeRequest) + + timelog = Timelog.new( + time_spent: time_spent, + spent_at: spent_at, + summary: summary, + user: current_user, + issue: issue, + merge_request: merge_request, + note: nil + ) + + if !timelog.save + error_in_save(timelog) + else + SystemNoteService.created_timelog(issuable, issuable.project, current_user, timelog) + success(timelog) + end + end + end +end diff --git a/app/services/timelogs/delete_service.rb b/app/services/timelogs/delete_service.rb index 0df888a3706..e72dfd98494 100644 --- a/app/services/timelogs/delete_service.rb +++ b/app/services/timelogs/delete_service.rb @@ -2,11 +2,17 @@ module Timelogs class DeleteService < Timelogs::BaseService + attr_accessor :timelog + + def initialize(timelog, user) + super(user) + + @timelog = timelog + end + def execute unless can?(current_user, :admin_timelog, timelog) - return ServiceResponse.error( - message: "Timelog doesn't exist or you don't have permission to delete it", - http_status: 404) + return error(_("Timelog doesn't exist or you don't have permission to delete it"), 404) end if timelog.destroy @@ -17,9 +23,9 @@ module Timelogs SystemNoteService.remove_timelog(issuable, issuable.project, current_user, timelog) end - ServiceResponse.success(payload: timelog) + success(timelog) else - ServiceResponse.error(message: 'Failed to remove timelog', http_status: 400) + error(_('Failed to remove timelog'), 400) end end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 14cf264cc51..6ae394072c6 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -218,11 +218,6 @@ class TodoService create_todos(reviewers, attributes) end - def create_attention_requested_todo(target, author, users) - attributes = attributes_for_todo(target.project, target, author, Todo::ATTENTION_REQUESTED) - create_todos(users, attributes) - end - private def create_todos(users, attributes) diff --git a/app/services/todos/destroy/destroyed_issuable_service.rb b/app/services/todos/destroy/destroyed_issuable_service.rb index 7a85b59eeea..759c430ec7a 100644 --- a/app/services/todos/destroy/destroyed_issuable_service.rb +++ b/app/services/todos/destroy/destroyed_issuable_service.rb @@ -5,9 +5,14 @@ module Todos class DestroyedIssuableService BATCH_SIZE = 100 + # 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 + def initialize(target_id, target_type) @target_id = target_id - @target_type = target_type + @target_type = BOUND_TARGET_TYPES.include?(target_type) ? BOUND_TARGET_TYPES : target_type end def execute diff --git a/app/services/topics/merge_service.rb b/app/services/topics/merge_service.rb new file mode 100644 index 00000000000..0d256579fe0 --- /dev/null +++ b/app/services/topics/merge_service.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Topics + class MergeService + attr_accessor :source_topic, :target_topic + + def initialize(source_topic, target_topic) + @source_topic = source_topic + @target_topic = target_topic + end + + def execute + validate_parameters! + + ::Projects::ProjectTopic.transaction do + move_project_topics + refresh_target_topic_counters + delete_source_topic + end + end + + private + + def validate_parameters! + raise ArgumentError, 'The source topic is not a topic.' unless source_topic.is_a?(Projects::Topic) + raise ArgumentError, 'The target topic is not a topic.' unless target_topic.is_a?(Projects::Topic) + raise ArgumentError, 'The source topic and the target topic are identical.' if source_topic == target_topic + end + + # rubocop: disable CodeReuse/ActiveRecord + def move_project_topics + project_ids_for_projects_currently_using_source_and_target = ::Projects::ProjectTopic + .where(topic_id: target_topic).select(:project_id) + # Only update for projects that exclusively use the source topic + ::Projects::ProjectTopic.where(topic_id: source_topic.id) + .where.not(project_id: project_ids_for_projects_currently_using_source_and_target) + .update_all(topic_id: target_topic.id) + + # Delete source topic for projects that were using source and target + ::Projects::ProjectTopic.where(topic_id: source_topic.id).delete_all + end + + def refresh_target_topic_counters + target_topic.update!( + total_projects_count: total_projects_count(target_topic.id), + non_private_projects_count: non_private_projects_count(target_topic.id) + ) + end + + def delete_source_topic + source_topic.destroy! + end + + def total_projects_count(topic_id) + ::Projects::ProjectTopic.where(topic_id: topic_id).count + end + + def non_private_projects_count(topic_id) + ::Projects::ProjectTopic.joins(:project).where(topic_id: topic_id).where('projects.visibility_level in (10, 20)') + .count + end + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/app/services/uploads/destroy_service.rb b/app/services/uploads/destroy_service.rb new file mode 100644 index 00000000000..1f0d99ff7bb --- /dev/null +++ b/app/services/uploads/destroy_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Uploads + class DestroyService < BaseService + attr_accessor :model, :current_user + + def initialize(model, user = nil) + @model = model + @current_user = user + end + + def execute(secret, filename) + upload = find_upload(secret, filename) + + unless current_user && upload && current_user.can?(:destroy_upload, upload) + return error(_("The resource that you are attempting to access does not "\ + "exist or you don't have permission to perform this action.")) + end + + if upload.destroy + success(upload: upload) + else + error(_('Upload could not be deleted.')) + end + end + + private + + # rubocop: disable CodeReuse/ActiveRecord + def find_upload(secret, filename) + uploader = uploader_class.new(model, secret: secret) + upload_paths = uploader.upload_paths(filename) + + Upload.find_by(model: model, uploader: uploader_class.to_s, path: upload_paths) + rescue FileUploader::InvalidSecret + nil + end + # rubocop: enable CodeReuse/ActiveRecord + + def uploader_class + case model + when Group + NamespaceFileUploader + when Project + FileUploader + else + raise ArgumentError, "unknown uploader for #{model.class.name}" + end + end + end +end diff --git a/app/services/users/dismiss_namespace_callout_service.rb b/app/services/users/dismiss_namespace_callout_service.rb new file mode 100644 index 00000000000..51261a93e20 --- /dev/null +++ b/app/services/users/dismiss_namespace_callout_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Users + class DismissNamespaceCalloutService < DismissCalloutService + private + + def callout + current_user.find_or_initialize_namespace_callout(params[:feature_name], params[:namespace_id]) + end + end +end diff --git a/app/services/users/dismiss_project_callout_service.rb b/app/services/users/dismiss_project_callout_service.rb new file mode 100644 index 00000000000..23549b3b194 --- /dev/null +++ b/app/services/users/dismiss_project_callout_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Users + class DismissProjectCalloutService < DismissCalloutService + private + + def callout + current_user.find_or_initialize_project_callout(params[:feature_name], params[:project_id]) + end + end +end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index c3df9b153a1..cb2711b6fee 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -17,7 +17,7 @@ module Users end def execute(validate: true, check_password: false, &block) - yield(@user) if block_given? + yield(@user) if block user_exists = @user.persisted? @user.user_detail # prevent assignment diff --git a/app/services/web_hooks/admin_destroy_service.rb b/app/services/web_hooks/admin_destroy_service.rb new file mode 100644 index 00000000000..e9835801a39 --- /dev/null +++ b/app/services/web_hooks/admin_destroy_service.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module WebHooks + # A variant of the destroy service that can only be used by an administrator + # from a rake task. + class AdminDestroyService < WebHooks::DestroyService + def initialize(rake_task:) + super(nil) + @rake_task = rake_task + end + + def authorized?(web_hook) + @rake_task.present? # Not impossible to circumvent, but you need to provide something + end + + def log_message(hook) + "An administrator scheduled a deletion of logs for hook ID #{hook.id} from #{@rake_task.name}" + end + end +end diff --git a/app/services/web_hooks/destroy_service.rb b/app/services/web_hooks/destroy_service.rb index 54c6c7ea71b..dbd164ab20e 100644 --- a/app/services/web_hooks/destroy_service.rb +++ b/app/services/web_hooks/destroy_service.rb @@ -1,25 +1,41 @@ # frozen_string_literal: true module WebHooks + # Destroy a hook, and schedule the logs for deletion. class DestroyService + include Services::ReturnServiceResponses + attr_accessor :current_user + DENIED = 'Insufficient permissions' + def initialize(current_user) @current_user = current_user end - # Destroy the hook immediately, schedule the logs for deletion def execute(web_hook) + return error(DENIED, 401) unless authorized?(web_hook) + hook_id = web_hook.id if web_hook.destroy WebHooks::LogDestroyWorker.perform_async({ 'hook_id' => hook_id }) - Gitlab::AppLogger.info("User #{current_user&.id} scheduled a deletion of logs for hook ID #{hook_id}") + Gitlab::AppLogger.info(log_message(web_hook)) - ServiceResponse.success(payload: { async: false }) + success({ async: false }) else - ServiceResponse.error(message: "Unable to destroy #{web_hook.model_name.human}") + error("Unable to destroy #{web_hook.model_name.human}", 500) end end + + private + + def log_message(hook) + "User #{current_user&.id} scheduled a deletion of logs for hook ID #{hook.id}" + end + + def authorized?(web_hook) + Ability.allowed?(current_user, :destroy_web_hook, web_hook) + end end end diff --git a/app/services/web_hooks/log_execution_service.rb b/app/services/web_hooks/log_execution_service.rb index 17dcf615830..5be8aee3ae8 100644 --- a/app/services/web_hooks/log_execution_service.rb +++ b/app/services/web_hooks/log_execution_service.rb @@ -14,7 +14,6 @@ module WebHooks @hook = hook @log_data = log_data.transform_keys(&:to_sym) @response_category = response_category - @prev_state = hook.active_state(ignore_flag: true) end def execute @@ -43,36 +42,12 @@ module WebHooks hook.failed! end - log_state_change hook.update_last_failure end rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError raise if raise_lock_error? end - def log_state_change - new_state = hook.active_state(ignore_flag: true) - - return if @prev_state == new_state - - Gitlab::AuthLogger.info( - message: 'WebHook change active_state', - # identification - hook_id: hook.id, - hook_type: hook.type, - project_id: hook.project_id, - group_id: hook.group_id, - # relevant data - prev_state: @prev_state, - new_state: new_state, - duration: log_data[:execution_duration], - response_status: log_data[:response_status], - recent_hook_failures: hook.recent_failures, - # context - **Gitlab::ApplicationContext.current - ) - end - def lock_name "web_hooks:update_hook_failure_state:#{hook.id}" end diff --git a/app/services/webauthn/authenticate_service.rb b/app/services/webauthn/authenticate_service.rb index a575a853995..52437a77df8 100644 --- a/app/services/webauthn/authenticate_service.rb +++ b/app/services/webauthn/authenticate_service.rb @@ -30,6 +30,8 @@ module Webauthn false end + private + ## # Validates that webauthn_credential is syntactically valid # diff --git a/app/services/work_items/create_and_link_service.rb b/app/services/work_items/create_and_link_service.rb index 6a773a84225..5cc358c4b4f 100644 --- a/app/services/work_items/create_and_link_service.rb +++ b/app/services/work_items/create_and_link_service.rb @@ -7,19 +7,20 @@ module WorkItems # new work items that were never associated with other work items as expected. class CreateAndLinkService def initialize(project:, current_user: nil, params: {}, spam_params:, link_params: {}) - @create_service = CreateService.new( - project: project, - current_user: current_user, - params: params, - spam_params: spam_params - ) @project = project @current_user = current_user + @params = params @link_params = link_params + @spam_params = spam_params end def execute - create_result = @create_service.execute + create_result = CreateService.new( + project: @project, + current_user: @current_user, + params: @params.merge(title: @params[:title].strip).reverse_merge(confidential: confidential_parent), + spam_params: @spam_params + ).execute return create_result if create_result.error? work_item = create_result[:work_item] @@ -40,6 +41,10 @@ module WorkItems private + def confidential_parent + !!@link_params[:parent_work_item]&.confidential + end + def payload(work_item) { work_item: work_item } end diff --git a/app/services/work_items/parent_links/create_service.rb b/app/services/work_items/parent_links/create_service.rb index 9940776e367..e7906f1fcdd 100644 --- a/app/services/work_items/parent_links/create_service.rb +++ b/app/services/work_items/parent_links/create_service.rb @@ -41,10 +41,8 @@ module WorkItems params[:issuable_references] end - # TODO: Create system notes when work item's parent or children are updated - # See https://gitlab.com/gitlab-org/gitlab/-/issues/362213 def create_notes(work_item) - # no-op + SystemNoteService.relate_work_item(issuable, work_item, current_user) end def target_issuable_type diff --git a/app/services/work_items/parent_links/destroy_service.rb b/app/services/work_items/parent_links/destroy_service.rb index 55870d44db9..19770b3e4b5 100644 --- a/app/services/work_items/parent_links/destroy_service.rb +++ b/app/services/work_items/parent_links/destroy_service.rb @@ -14,10 +14,8 @@ module WorkItems private - # TODO: Create system notes when work item's parent or children are removed - # See https://gitlab.com/gitlab-org/gitlab/-/issues/362213 def create_notes - # no-op + SystemNoteService.unrelate_work_item(parent, child, current_user) end def not_found_message diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb index 98818fda263..2deb8c82741 100644 --- a/app/services/work_items/update_service.rb +++ b/app/services/work_items/update_service.rb @@ -26,8 +26,8 @@ module WorkItems private - def update(work_item) - execute_widgets(work_item: work_item, callback: :update, widget_params: @widget_params) + def before_update(work_item, skip_spam_check: false) + execute_widgets(work_item: work_item, callback: :before_update_callback, widget_params: @widget_params) super end diff --git a/app/services/work_items/widgets/assignees_service/update_service.rb b/app/services/work_items/widgets/assignees_service/update_service.rb new file mode 100644 index 00000000000..9176b71c85e --- /dev/null +++ b/app/services/work_items/widgets/assignees_service/update_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module WorkItems + module Widgets + module AssigneesService + class UpdateService < WorkItems::Widgets::BaseService + def before_update_in_transaction(params:) + return unless params.present? && params.has_key?(:assignee_ids) + return unless has_permission?(:set_work_item_metadata) + + assignee_ids = filter_assignees_count(params[:assignee_ids]) + assignee_ids = filter_assignee_permissions(assignee_ids) + + return if assignee_ids.sort == work_item.assignee_ids.sort + + work_item.assignee_ids = assignee_ids + work_item.touch + end + + private + + def filter_assignees_count(assignee_ids) + return assignee_ids if work_item.allows_multiple_assignees? + + assignee_ids.first(1) + end + + def filter_assignee_permissions(assignee_ids) + assignees = User.id_in(assignee_ids) + + assignees.select { |assignee| assignee.can?(:read_work_item, work_item) }.map(&:id) + end + end + end + end +end diff --git a/app/services/work_items/widgets/base_service.rb b/app/services/work_items/widgets/base_service.rb index 037733bbed5..37ed2bf4b05 100644 --- a/app/services/work_items/widgets/base_service.rb +++ b/app/services/work_items/widgets/base_service.rb @@ -5,12 +5,19 @@ module WorkItems class BaseService < ::BaseService WidgetError = Class.new(StandardError) - attr_reader :widget, :current_user + attr_reader :widget, :work_item, :current_user def initialize(widget:, current_user:) @widget = widget + @work_item = widget.work_item @current_user = current_user end + + private + + def has_permission?(permission) + can?(current_user, permission, widget.work_item) + end end end end diff --git a/app/services/work_items/widgets/description_service/update_service.rb b/app/services/work_items/widgets/description_service/update_service.rb index e63b6b2ee6c..fe591ba605e 100644 --- a/app/services/work_items/widgets/description_service/update_service.rb +++ b/app/services/work_items/widgets/description_service/update_service.rb @@ -4,10 +4,12 @@ module WorkItems module Widgets module DescriptionService class UpdateService < WorkItems::Widgets::BaseService - def update(params: {}) - return unless params.present? && params[:description] + def before_update_callback(params: {}) + return unless params.present? && params.key?(:description) + return unless has_permission?(:update_work_item) - widget.work_item.description = params[:description] + work_item.description = params[:description] + work_item.assign_attributes(last_edited_at: Time.current, last_edited_by: current_user) end end end diff --git a/app/services/work_items/widgets/hierarchy_service/base_service.rb b/app/services/work_items/widgets/hierarchy_service/base_service.rb index 085d6c6b0e7..bb681ef0083 100644 --- a/app/services/work_items/widgets/hierarchy_service/base_service.rb +++ b/app/services/work_items/widgets/hierarchy_service/base_service.rb @@ -15,7 +15,7 @@ module WorkItems elsif params.key?(:children) update_work_item_children(params.delete(:children)) else - invalid_args_error + invalid_args_error(params) end end @@ -29,13 +29,13 @@ module WorkItems def set_parent(parent) ::WorkItems::ParentLinks::CreateService - .new(parent, current_user, { target_issuable: widget.work_item }) + .new(parent, current_user, { target_issuable: work_item }) .execute end # rubocop: disable CodeReuse/ActiveRecord def remove_parent - link = ::WorkItems::ParentLink.find_by(work_item: widget.work_item) + link = ::WorkItems::ParentLink.find_by(work_item: work_item) return success unless link.present? ::WorkItems::ParentLinks::DestroyService.new(link, current_user).execute @@ -44,12 +44,12 @@ module WorkItems def update_work_item_children(children) ::WorkItems::ParentLinks::CreateService - .new(widget.work_item, current_user, { issuable_references: children }) + .new(work_item, current_user, { issuable_references: children }) .execute end def feature_flag_enabled? - Feature.enabled?(:work_items_hierarchy, widget.work_item&.project) + Feature.enabled?(:work_items_hierarchy, work_item&.project) end def incompatible_args?(params) @@ -64,11 +64,14 @@ module WorkItems error(_('A Work Item can be a parent or a child, but not both.')) end - def invalid_args_error + def invalid_args_error(params) error(_("One or more arguments are invalid: %{args}." % { args: params.keys.to_sentence } )) end def service_response!(result) + work_item.reload_work_item_parent + work_item.work_item_children.reset + return result unless result[:status] == :error raise WidgetError, result[:message] diff --git a/app/services/work_items/widgets/start_and_due_date_service/update_service.rb b/app/services/work_items/widgets/start_and_due_date_service/update_service.rb new file mode 100644 index 00000000000..6a5dc0d5ef3 --- /dev/null +++ b/app/services/work_items/widgets/start_and_due_date_service/update_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module WorkItems + module Widgets + module StartAndDueDateService + class UpdateService < WorkItems::Widgets::BaseService + def before_update_callback(params: {}) + return if params.blank? + + widget.work_item.assign_attributes(params.slice(:start_date, :due_date)) + end + end + end + end +end diff --git a/app/services/work_items/widgets/weight_service/update_service.rb b/app/services/work_items/widgets/weight_service/update_service.rb deleted file mode 100644 index cd62a25358f..00000000000 --- a/app/services/work_items/widgets/weight_service/update_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module WorkItems - module Widgets - module WeightService - class UpdateService < WorkItems::Widgets::BaseService - def update(params: {}) - return unless params.present? && params[:weight] - - widget.work_item.weight = params[:weight] - end - end - end - end -end |