diff options
| author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-11-30 14:52:04 +0300 |
|---|---|---|
| committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-11-30 14:52:04 +0300 |
| commit | 5df4ba0a93fd266105ccf35fd49fa18e6403c15b (patch) | |
| tree | 57d32dbd0af5d54643060faba9ab6ba993357a1f /app/services | |
| parent | 1486950bc9396f9b8384763d68f36387326eb745 (diff) | |
| parent | feece7713247a063bfa71ab701f8a164e6fa71bb (diff) | |
Merge branch 'master' into backstage/gb/build-pipeline-in-a-separate-class
* master: (1794 commits)
Diffstat (limited to 'app/services')
74 files changed, 1236 insertions, 617 deletions
diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index 9c00ea789ec..46e19230328 100644 --- a/app/services/access_token_validation_service.rb +++ b/app/services/access_token_validation_service.rb @@ -39,11 +39,8 @@ class AccessTokenValidationService token_scopes = token.scopes.map(&:to_sym) required_scopes.any? do |scope| - if scope.respond_to?(:sufficient?) - scope.sufficient?(token_scopes, request) - else - API::Scope.new(scope).sufficient?(token_scopes, request) - end + scope = API::Scope.new(scope) unless scope.is_a?(API::Scope) + scope.sufficient?(token_scopes, request) end end end diff --git a/app/services/applications/create_service.rb b/app/services/applications/create_service.rb new file mode 100644 index 00000000000..35d45f25a71 --- /dev/null +++ b/app/services/applications/create_service.rb @@ -0,0 +1,13 @@ +module Applications + class CreateService + def initialize(current_user, params) + @current_user = current_user + @params = params + @ip_address = @params.delete(:ip_address) + end + + def execute(request = nil) + Doorkeeper::Application.create(@params) + end + end +end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 9a636346899..f40cd2b06c8 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -56,11 +56,22 @@ module Auth def process_scope(scope) type, name, actions = scope.split(':', 3) actions = actions.split(',') - path = ContainerRegistry::Path.new(name) - return unless type == 'repository' + case type + when 'registry' + process_registry_access(type, name, actions) + when 'repository' + path = ContainerRegistry::Path.new(name) + process_repository_access(type, path, actions) + end + end + + def process_registry_access(type, name, actions) + return unless current_user&.admin? + return unless name == 'catalog' + return unless actions == ['*'] - process_repository_access(type, path, actions) + { type: type, name: name, actions: ['*'] } end def process_repository_access(type, path, actions) diff --git a/app/services/base_count_service.rb b/app/services/base_count_service.rb new file mode 100644 index 00000000000..99cc9a196e6 --- /dev/null +++ b/app/services/base_count_service.rb @@ -0,0 +1,34 @@ +# Base class for services that count a single resource such as the number of +# issues for a project. +class BaseCountService + def relation_for_count + raise( + NotImplementedError, + '"relation_for_count" must be implemented and return an ActiveRecord::Relation' + ) + end + + def count + Rails.cache.fetch(cache_key, raw: raw?) { uncached_count }.to_i + end + + def refresh_cache + Rails.cache.write(cache_key, uncached_count, raw: raw?) + end + + def uncached_count + relation_for_count.count + end + + def delete_cache + Rails.cache.delete(cache_key) + end + + def raw? + false + end + + def cache_key + raise NotImplementedError, 'cache_key must be implemented and return a String' + end +end diff --git a/app/services/base_renderer.rb b/app/services/base_renderer.rb new file mode 100644 index 00000000000..d6e30bd7008 --- /dev/null +++ b/app/services/base_renderer.rb @@ -0,0 +1,7 @@ +class BaseRenderer + attr_reader :current_user + + def initialize(current_user = nil) + @current_user = current_user + end +end diff --git a/app/services/ci/create_cluster_service.rb b/app/services/ci/create_cluster_service.rb deleted file mode 100644 index f7ee0e468e2..00000000000 --- a/app/services/ci/create_cluster_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -module Ci - class CreateClusterService < BaseService - def execute(access_token) - params['gcp_machine_type'] ||= GoogleApi::CloudPlatform::Client::DEFAULT_MACHINE_TYPE - - cluster_params = - params.merge(user: current_user, - gcp_token: access_token) - - project.create_cluster(cluster_params).tap do |cluster| - ClusterProvisionWorker.perform_async(cluster.id) if cluster.persisted? - end - end - end -end diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb new file mode 100644 index 00000000000..dc2f49e8db1 --- /dev/null +++ b/app/services/ci/ensure_stage_service.rb @@ -0,0 +1,39 @@ +module Ci + ## + # We call this service everytime we persist a CI/CD job. + # + # In most cases a job should already have a stage assigned, but in cases it + # doesn't have we need to either find existing one or create a brand new + # stage. + # + class EnsureStageService < BaseService + def execute(build) + @build = build + + return if build.stage_id.present? + return if build.invalid? + + ensure_stage.tap do |stage| + build.stage_id = stage.id + + yield stage if block_given? + end + end + + private + + def ensure_stage + find_stage || create_stage + end + + def find_stage + @build.pipeline.stages.find_by(name: @build.stage) + end + + def create_stage + Ci::Stage.create!(name: @build.stage, + pipeline: @build.pipeline, + project: @build.project) + end + end +end diff --git a/app/services/ci/fetch_gcp_operation_service.rb b/app/services/ci/fetch_gcp_operation_service.rb deleted file mode 100644 index 0b68e4d6ea9..00000000000 --- a/app/services/ci/fetch_gcp_operation_service.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Ci - class FetchGcpOperationService - def execute(cluster) - api_client = - GoogleApi::CloudPlatform::Client.new(cluster.gcp_token, nil) - - operation = api_client.projects_zones_operations( - cluster.gcp_project_id, - cluster.gcp_cluster_zone, - cluster.gcp_operation_id) - - yield(operation) if block_given? - rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e - return cluster.make_errored!("Failed to request to CloudPlatform; #{e.message}") - end - end -end diff --git a/app/services/ci/fetch_kubernetes_token_service.rb b/app/services/ci/fetch_kubernetes_token_service.rb index 44da87cb00c..e73c6ad6780 100644 --- a/app/services/ci/fetch_kubernetes_token_service.rb +++ b/app/services/ci/fetch_kubernetes_token_service.rb @@ -34,6 +34,7 @@ module Ci kubeclient.get_secrets.as_json rescue KubeException => err raise err unless err.error_code == 404 + [] end diff --git a/app/services/ci/finalize_cluster_creation_service.rb b/app/services/ci/finalize_cluster_creation_service.rb deleted file mode 100644 index 347875c5697..00000000000 --- a/app/services/ci/finalize_cluster_creation_service.rb +++ /dev/null @@ -1,33 +0,0 @@ -module Ci - class FinalizeClusterCreationService - def execute(cluster) - api_client = - GoogleApi::CloudPlatform::Client.new(cluster.gcp_token, nil) - - begin - gke_cluster = api_client.projects_zones_clusters_get( - cluster.gcp_project_id, - cluster.gcp_cluster_zone, - cluster.gcp_cluster_name) - rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e - return cluster.make_errored!("Failed to request to CloudPlatform; #{e.message}") - end - - endpoint = gke_cluster.endpoint - api_url = 'https://' + endpoint - ca_cert = Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate) - username = gke_cluster.master_auth.username - password = gke_cluster.master_auth.password - - kubernetes_token = Ci::FetchKubernetesTokenService.new( - api_url, ca_cert, username, password).execute - - unless kubernetes_token - return cluster.make_errored!('Failed to get a default token of kubernetes') - end - - Ci::IntegrateClusterService.new.execute( - cluster, endpoint, ca_cert, kubernetes_token, username, password) - end - end -end diff --git a/app/services/ci/integrate_cluster_service.rb b/app/services/ci/integrate_cluster_service.rb deleted file mode 100644 index d123ce8d26b..00000000000 --- a/app/services/ci/integrate_cluster_service.rb +++ /dev/null @@ -1,26 +0,0 @@ -module Ci - class IntegrateClusterService - def execute(cluster, endpoint, ca_cert, token, username, password) - Gcp::Cluster.transaction do - cluster.update!( - enabled: true, - endpoint: endpoint, - ca_cert: ca_cert, - kubernetes_token: token, - username: username, - password: password, - service: cluster.project.find_or_initialize_service('kubernetes'), - status_event: :make_created) - - cluster.service.update!( - active: true, - api_url: cluster.api_url, - ca_pem: ca_cert, - namespace: cluster.project_namespace, - token: token) - end - rescue ActiveRecord::RecordInvalid => e - cluster.make_errored!("Failed to integrate cluster into kubernetes_service: #{e.message}") - end - end -end diff --git a/app/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index 120af8c1e61..a9813d774bb 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -1,5 +1,7 @@ module Ci class PipelineTriggerService < BaseService + include Gitlab::Utils::StrongMemoize + def execute if trigger_from_token create_pipeline_from_trigger(trigger_from_token) @@ -26,9 +28,9 @@ module Ci end def trigger_from_token - return @trigger if defined?(@trigger) - - @trigger = Ci::Trigger.find_by_token(params[:token].to_s) + strong_memoize(:trigger) do + Ci::Trigger.find_by_token(params[:token].to_s) + end end def create_pipeline_variables!(pipeline) diff --git a/app/services/ci/provision_cluster_service.rb b/app/services/ci/provision_cluster_service.rb deleted file mode 100644 index 52d80b01813..00000000000 --- a/app/services/ci/provision_cluster_service.rb +++ /dev/null @@ -1,36 +0,0 @@ -module Ci - class ProvisionClusterService - def execute(cluster) - api_client = - GoogleApi::CloudPlatform::Client.new(cluster.gcp_token, nil) - - begin - operation = api_client.projects_zones_clusters_create( - cluster.gcp_project_id, - cluster.gcp_cluster_zone, - cluster.gcp_cluster_name, - cluster.gcp_cluster_size, - machine_type: cluster.gcp_machine_type) - rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e - return cluster.make_errored!("Failed to request to CloudPlatform; #{e.message}") - end - - unless operation.status == 'RUNNING' || operation.status == 'PENDING' - return cluster.make_errored!("Operation status is unexpected; #{operation.status_message}") - end - - cluster.gcp_operation_id = api_client.parse_operation_id(operation.self_link) - - unless cluster.gcp_operation_id - return cluster.make_errored!('Can not find operation_id from self_link') - end - - if cluster.make_creating - WaitForClusterCreationWorker.perform_in( - WaitForClusterCreationWorker::INITIAL_INTERVAL, cluster.id) - else - return cluster.make_errored!("Failed to update cluster record; #{cluster.errors}") - end - end - end -end diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index d67b9f5cc56..c552193e66b 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -28,6 +28,8 @@ module Ci attributes.push([:user, current_user]) + build.retried = true + Ci::Build.transaction do # mark all other builds of that name as retried build.pipeline.builds.latest diff --git a/app/services/ci/update_cluster_service.rb b/app/services/ci/update_cluster_service.rb deleted file mode 100644 index 70d88fca660..00000000000 --- a/app/services/ci/update_cluster_service.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Ci - class UpdateClusterService < BaseService - def execute(cluster) - Gcp::Cluster.transaction do - cluster.update!(params) - - if params['enabled'] == 'true' - cluster.service.update!( - active: true, - api_url: cluster.api_url, - ca_pem: cluster.ca_cert, - namespace: cluster.project_namespace, - token: cluster.kubernetes_token) - else - cluster.service.update!(active: false) - end - end - rescue ActiveRecord::RecordInvalid => e - cluster.errors.add(:base, e.message) - end - end -end diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb new file mode 100644 index 00000000000..9a4ce31cb39 --- /dev/null +++ b/app/services/clusters/applications/base_helm_service.rb @@ -0,0 +1,29 @@ +module Clusters + module Applications + class BaseHelmService + attr_accessor :app + + def initialize(app) + @app = app + end + + protected + + def cluster + app.cluster + end + + def kubeclient + cluster.kubeclient + end + + def helm_api + @helm_api ||= Gitlab::Kubernetes::Helm.new(kubeclient) + end + + def install_command + @install_command ||= app.install_command + end + end + end +end diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb new file mode 100644 index 00000000000..bde090eaeec --- /dev/null +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -0,0 +1,65 @@ +module Clusters + module Applications + class CheckInstallationProgressService < BaseHelmService + def execute + return unless app.installing? + + case installation_phase + when Gitlab::Kubernetes::Pod::SUCCEEDED + on_success + when Gitlab::Kubernetes::Pod::FAILED + on_failed + else + check_timeout + end + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored? + end + + private + + def on_success + app.make_installed! + ensure + remove_installation_pod + end + + def on_failed + app.make_errored!(installation_errors || 'Installation silently failed') + ensure + remove_installation_pod + end + + def check_timeout + if timeouted? + begin + app.make_errored!('Installation timeouted') + ensure + remove_installation_pod + end + else + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) + end + end + + def timeouted? + Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT + end + + def remove_installation_pod + helm_api.delete_installation_pod!(install_command.pod_name) + rescue + # no-op + end + + def installation_phase + helm_api.installation_status(install_command.pod_name) + end + + def installation_errors + helm_api.installation_log(install_command.pod_name) + end + end + end +end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb new file mode 100644 index 00000000000..8ceeec687cd --- /dev/null +++ b/app/services/clusters/applications/install_service.rb @@ -0,0 +1,21 @@ +module Clusters + module Applications + class InstallService < BaseHelmService + def execute + return unless app.scheduled? + + begin + app.make_installing! + helm_api.install(install_command) + + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) + rescue KubeException => ke + app.make_errored!("Kubernetes error: #{ke.message}") + rescue StandardError + app.make_errored!("Can't start installation process") + end + end + end + end +end diff --git a/app/services/clusters/applications/schedule_installation_service.rb b/app/services/clusters/applications/schedule_installation_service.rb new file mode 100644 index 00000000000..eb8caa68ef7 --- /dev/null +++ b/app/services/clusters/applications/schedule_installation_service.rb @@ -0,0 +1,22 @@ +module Clusters + module Applications + class ScheduleInstallationService < ::BaseService + def execute + application_class.find_or_create_by!(cluster: cluster).try do |application| + application.make_scheduled! + ClusterInstallAppWorker.perform_async(application.name, application.id) + end + end + + private + + def application_class + params[:application_class] + end + + def cluster + params[:cluster] + end + end + end +end diff --git a/app/services/clusters/create_service.rb b/app/services/clusters/create_service.rb new file mode 100644 index 00000000000..1d407739b21 --- /dev/null +++ b/app/services/clusters/create_service.rb @@ -0,0 +1,29 @@ +module Clusters + class CreateService < BaseService + attr_reader :access_token + + def execute(access_token) + @access_token = access_token + + create_cluster.tap do |cluster| + ClusterProvisionWorker.perform_async(cluster.id) if cluster.persisted? + end + end + + private + + def create_cluster + Clusters::Cluster.create(cluster_params) + end + + def cluster_params + return @cluster_params if defined?(@cluster_params) + + params[:provider_gcp_attributes].try do |provider| + provider[:access_token] = access_token + end + + @cluster_params = params.merge(user: current_user, projects: [project]) + end + end +end diff --git a/app/services/clusters/gcp/fetch_operation_service.rb b/app/services/clusters/gcp/fetch_operation_service.rb new file mode 100644 index 00000000000..a4cd3ca5c11 --- /dev/null +++ b/app/services/clusters/gcp/fetch_operation_service.rb @@ -0,0 +1,16 @@ +module Clusters + module Gcp + class FetchOperationService + def execute(provider) + operation = provider.api_client.projects_zones_operations( + provider.gcp_project_id, + provider.zone, + provider.operation_id) + + yield(operation) if block_given? + rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e + provider.make_errored!("Failed to request to CloudPlatform; #{e.message}") + end + end + end +end diff --git a/app/services/clusters/gcp/finalize_creation_service.rb b/app/services/clusters/gcp/finalize_creation_service.rb new file mode 100644 index 00000000000..cea56f4e849 --- /dev/null +++ b/app/services/clusters/gcp/finalize_creation_service.rb @@ -0,0 +1,56 @@ +module Clusters + module Gcp + class FinalizeCreationService + attr_reader :provider + + def execute(provider) + @provider = provider + + configure_provider + configure_kubernetes + + cluster.save! + rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e + provider.make_errored!("Failed to request to CloudPlatform; #{e.message}") + rescue ActiveRecord::RecordInvalid => e + provider.make_errored!("Failed to configure GKE Cluster: #{e.message}") + end + + private + + def configure_provider + provider.endpoint = gke_cluster.endpoint + provider.status_event = :make_created + end + + def configure_kubernetes + cluster.platform_type = :kubernetes + cluster.build_platform_kubernetes( + api_url: 'https://' + gke_cluster.endpoint, + ca_cert: Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate), + username: gke_cluster.master_auth.username, + password: gke_cluster.master_auth.password, + token: request_kuberenetes_token) + end + + def request_kuberenetes_token + Ci::FetchKubernetesTokenService.new( + 'https://' + gke_cluster.endpoint, + Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate), + gke_cluster.master_auth.username, + gke_cluster.master_auth.password).execute + end + + def gke_cluster + @gke_cluster ||= provider.api_client.projects_zones_clusters_get( + provider.gcp_project_id, + provider.zone, + cluster.name) + end + + def cluster + @cluster ||= provider.cluster + end + end + end +end diff --git a/app/services/clusters/gcp/provision_service.rb b/app/services/clusters/gcp/provision_service.rb new file mode 100644 index 00000000000..8beea5a8cfb --- /dev/null +++ b/app/services/clusters/gcp/provision_service.rb @@ -0,0 +1,47 @@ +module Clusters + module Gcp + class ProvisionService + attr_reader :provider + + def execute(provider) + @provider = provider + + get_operation_id do |operation_id| + if provider.make_creating(operation_id) + WaitForClusterCreationWorker.perform_in( + Clusters::Gcp::VerifyProvisionStatusService::INITIAL_INTERVAL, + provider.cluster_id) + else + provider.make_errored!("Failed to update provider record; #{provider.errors}") + end + end + end + + private + + def get_operation_id + operation = provider.api_client.projects_zones_clusters_create( + provider.gcp_project_id, + provider.zone, + provider.cluster.name, + provider.num_nodes, + machine_type: provider.machine_type) + + unless operation.status == 'PENDING' || operation.status == 'RUNNING' + return provider.make_errored!("Operation status is unexpected; #{operation.status_message}") + end + + operation_id = provider.api_client.parse_operation_id(operation.self_link) + + unless operation_id + return provider.make_errored!('Can not find operation_id from self_link') + end + + yield(operation_id) + + rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e + provider.make_errored!("Failed to request to CloudPlatform; #{e.message}") + end + end + end +end diff --git a/app/services/clusters/gcp/verify_provision_status_service.rb b/app/services/clusters/gcp/verify_provision_status_service.rb new file mode 100644 index 00000000000..bc33756f27c --- /dev/null +++ b/app/services/clusters/gcp/verify_provision_status_service.rb @@ -0,0 +1,48 @@ +module Clusters + module Gcp + class VerifyProvisionStatusService + attr_reader :provider + + INITIAL_INTERVAL = 2.minutes + EAGER_INTERVAL = 10.seconds + TIMEOUT = 20.minutes + + def execute(provider) + @provider = provider + + request_operation do |operation| + case operation.status + when 'PENDING', 'RUNNING' + continue_creation(operation) + when 'DONE' + finalize_creation + else + return provider.make_errored!("Unexpected operation status; #{operation.status} #{operation.status_message}") + end + end + end + + private + + def continue_creation(operation) + if elapsed_time_from_creation(operation) < TIMEOUT + WaitForClusterCreationWorker.perform_in(EAGER_INTERVAL, provider.cluster_id) + else + provider.make_errored!("Cluster creation time exceeds timeout; #{TIMEOUT}") + end + end + + def elapsed_time_from_creation(operation) + Time.now.utc - operation.start_time.to_time.utc + end + + def finalize_creation + Clusters::Gcp::FinalizeCreationService.new.execute(provider) + end + + def request_operation(&blk) + Clusters::Gcp::FetchOperationService.new.execute(provider, &blk) + end + end + end +end diff --git a/app/services/clusters/update_service.rb b/app/services/clusters/update_service.rb new file mode 100644 index 00000000000..989218e32a2 --- /dev/null +++ b/app/services/clusters/update_service.rb @@ -0,0 +1,7 @@ +module Clusters + class UpdateService < BaseService + def execute(cluster) + cluster.update(params) + end + end +end diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 53f16a236d2..1db91c3c90c 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -1,17 +1,17 @@ require 'securerandom' -# Compare 2 branches for one repo or between repositories +# Compare 2 refs for one repo or between repositories # and return Gitlab::Git::Compare object that responds to commits and diffs class CompareService - attr_reader :start_project, :start_branch_name + attr_reader :start_project, :start_ref_name - def initialize(new_start_project, new_start_branch_name) + def initialize(new_start_project, new_start_ref_name) @start_project = new_start_project - @start_branch_name = new_start_branch_name + @start_ref_name = new_start_ref_name end - def execute(target_project, target_branch, straight: false) - raw_compare = target_project.repository.compare_source_branch(target_branch, start_project.repository, start_branch_name, straight: straight) + def execute(target_project, target_ref, straight: false) + raw_compare = target_project.repository.compare_source_branch(target_ref, start_project.repository, start_ref_name, straight: straight) Compare.new(raw_compare, target_project, straight: straight) if raw_compare end diff --git a/app/services/delete_merged_branches_service.rb b/app/services/delete_merged_branches_service.rb index 077268b2388..cb235a85daf 100644 --- a/app/services/delete_merged_branches_service.rb +++ b/app/services/delete_merged_branches_service.rb @@ -13,7 +13,7 @@ class DeleteMergedBranchesService < BaseService # Prevent deletion of branches relevant to open merge requests branches -= merge_request_branch_names # Prevent deletion of protected branches - branches = branches.reject { |branch| project.protected_for?(branch) } + branches = branches.reject { |branch| ProtectedBranch.protected?(project, branch) } branches.each do |branch| DeleteBranchService.new(project, current_user).execute(branch) diff --git a/app/services/events/render_service.rb b/app/services/events/render_service.rb new file mode 100644 index 00000000000..0b62d8aedf1 --- /dev/null +++ b/app/services/events/render_service.rb @@ -0,0 +1,21 @@ +module Events + class RenderService < BaseRenderer + def execute(events, atom_request: false) + events.map(&:note).compact.group_by(&:project).each do |project, notes| + render_notes(notes, project, atom_request) + end + end + + private + + def render_notes(notes, project, atom_request) + Notes::RenderService.new(current_user).execute(notes, project, render_options(atom_request)) + end + + def render_options(atom_request) + return {} unless atom_request + + { only_path: false, xhtml: true } + end + end +end diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb new file mode 100644 index 00000000000..3da21bd8b8f --- /dev/null +++ b/app/services/issuable/common_system_notes_service.rb @@ -0,0 +1,93 @@ +module Issuable + class CommonSystemNotesService < ::BaseService + attr_reader :issuable + + def execute(issuable, old_labels) + @issuable = issuable + + if issuable.previous_changes.include?('title') + create_title_change_note(issuable.previous_changes['title'].first) + end + + handle_description_change_note + + handle_time_tracking_note if issuable.is_a?(TimeTrackable) + create_labels_note(old_labels) if issuable.labels != old_labels + create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked') + create_milestone_note if issuable.previous_changes.include?('milestone_id') + end + + private + + def handle_time_tracking_note + if issuable.previous_changes.include?('time_estimate') + create_time_estimate_note + end + + if issuable.time_spent? + create_time_spent_note + end + end + + def handle_description_change_note + if issuable.previous_changes.include?('description') + if issuable.tasks? && issuable.updated_tasks.any? + create_task_status_note + else + # TODO: Show this note if non-task content was modified. + # https://gitlab.com/gitlab-org/gitlab-ce/issues/33577 + create_description_change_note + end + end + end + + def create_wip_note(old_title) + return unless issuable.is_a?(MergeRequest) + + if MergeRequest.work_in_progress?(old_title) != issuable.work_in_progress? + SystemNoteService.handle_merge_request_wip(issuable, issuable.project, current_user) + end + end + + def create_labels_note(old_labels) + added_labels = issuable.labels - old_labels + removed_labels = old_labels - issuable.labels + + SystemNoteService.change_label(issuable, issuable.project, current_user, added_labels, removed_labels) + end + + def create_title_change_note(old_title) + create_wip_note(old_title) + + if issuable.wipless_title_changed(old_title) + SystemNoteService.change_title(issuable, issuable.project, current_user, old_title) + end + end + + def create_description_change_note + SystemNoteService.change_description(issuable, issuable.project, current_user) + end + + def create_task_status_note + issuable.updated_tasks.each do |task| + SystemNoteService.change_task_status(issuable, issuable.project, current_user, task) + end + end + + def create_time_estimate_note + SystemNoteService.change_time_estimate(issuable, issuable.project, current_user) + end + + def create_time_spent_note + SystemNoteService.change_time_spent(issuable, issuable.project, issuable.time_spent_user) + end + + def create_milestone_note + SystemNoteService.change_milestone(issuable, issuable.project, current_user, issuable.milestone) + end + + def create_discussion_lock_note + SystemNoteService.discussion_lock(issuable, current_user) + end + end +end diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb new file mode 100644 index 00000000000..0610b401213 --- /dev/null +++ b/app/services/issuable/destroy_service.rb @@ -0,0 +1,9 @@ +module Issuable + class DestroyService < IssuableBaseService + def execute(issuable) + if issuable.destroy + issuable.update_project_counter_caches + end + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index f83ece7098f..2c51ac13815 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -1,56 +1,10 @@ class IssuableBaseService < BaseService private - def create_milestone_note(issuable) - SystemNoteService.change_milestone( - issuable, issuable.project, current_user, issuable.milestone) - end - - def create_labels_note(issuable, old_labels) - added_labels = issuable.labels - old_labels - removed_labels = old_labels - issuable.labels - - SystemNoteService.change_label( - issuable, issuable.project, current_user, added_labels, removed_labels) - end - - def create_title_change_note(issuable, old_title) - SystemNoteService.change_title( - issuable, issuable.project, current_user, old_title) - end - - def create_description_change_note(issuable) - SystemNoteService.change_description(issuable, issuable.project, current_user) - end - - def create_branch_change_note(issuable, branch_type, old_branch, new_branch) - SystemNoteService.change_branch( - issuable, issuable.project, current_user, branch_type, - old_branch, new_branch) - end - - def create_task_status_note(issuable) - issuable.updated_tasks.each do |task| - SystemNoteService.change_task_status(issuable, issuable.project, current_user, task) - end - end - - def create_time_estimate_note(issuable) - SystemNoteService.change_time_estimate(issuable, issuable.project, current_user) - end - - def create_time_spent_note(issuable) - SystemNoteService.change_time_spent(issuable, issuable.project, current_user) - end - - def create_discussion_lock_note(issuable) - SystemNoteService.discussion_lock(issuable, current_user) - end - def filter_params(issuable) ability_name = :"admin_#{issuable.to_ability_name}" - unless can?(current_user, ability_name, project) + unless can?(current_user, ability_name, issuable) params.delete(:milestone_id) params.delete(:labels) params.delete(:add_label_ids) @@ -215,9 +169,7 @@ class IssuableBaseService < BaseService change_todo(issuable) toggle_award(issuable) filter_params(issuable) - old_labels = issuable.labels.to_a - old_mentioned_users = issuable.mentioned_users.to_a - old_assignees = issuable.assignees.to_a + old_associations = associations_before_update(issuable) label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) @@ -233,29 +185,27 @@ class IssuableBaseService < BaseService # We have to perform this check before saving the issuable as Rails resets # the changed fields upon calling #save. - update_project_counters = issuable.update_project_counter_caches? + update_project_counters = issuable.project && update_project_counter_caches?(issuable) if issuable.with_transaction_returning_status { issuable.save } # We do not touch as it will affect a update on updated_at field ActiveRecord::Base.no_touching do - handle_common_system_notes(issuable, old_labels: old_labels) + Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_associations[:labels]) end - change_discussion_lock(issuable) - handle_changes( - issuable, - old_labels: old_labels, - old_mentioned_users: old_mentioned_users, - old_assignees: old_assignees - ) + handle_changes(issuable, old_associations: old_associations) new_assignees = issuable.assignees.to_a - affected_assignees = (old_assignees + new_assignees) - (old_assignees & new_assignees) + affected_assignees = (old_associations[:assignees] + new_assignees) - (old_associations[:assignees] & new_assignees) invalidate_cache_counts(issuable, users: affected_assignees.compact) after_update(issuable) issuable.create_new_cross_references!(current_user) - execute_hooks(issuable, 'update') + execute_hooks( + issuable, + 'update', + old_associations: old_associations + ) issuable.update_project_counter_caches if update_project_counters end @@ -300,12 +250,6 @@ class IssuableBaseService < BaseService end end - def change_discussion_lock(issuable) - if issuable.previous_changes.include?('discussion_locked') - create_discussion_lock_note(issuable) - end - end - def toggle_award(issuable) award = params.delete(:emoji_award) if award @@ -314,6 +258,18 @@ class IssuableBaseService < BaseService end end + def associations_before_update(issuable) + associations = + { + labels: issuable.labels.to_a, + mentioned_users: issuable.mentioned_users.to_a, + assignees: issuable.assignees.to_a + } + associations[:total_time_spent] = issuable.total_time_spent if issuable.respond_to?(:total_time_spent) + + associations + end + def has_changes?(issuable, old_labels: [], old_assignees: []) valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch] @@ -328,35 +284,21 @@ class IssuableBaseService < BaseService attrs_changed || labels_changed || assignees_changed end - def handle_common_system_notes(issuable, old_labels: []) - if issuable.previous_changes.include?('title') - create_title_change_note(issuable, issuable.previous_changes['title'].first) - end - - if issuable.previous_changes.include?('description') - if issuable.tasks? && issuable.updated_tasks.any? - create_task_status_note(issuable) - else - # TODO: Show this note if non-task content was modified. - # https://gitlab.com/gitlab-org/gitlab-ce/issues/33577 - create_description_change_note(issuable) - end - end - - if issuable.previous_changes.include?('time_estimate') - create_time_estimate_note(issuable) + def invalidate_cache_counts(issuable, users: []) + users.each do |user| + user.public_send("invalidate_#{issuable.model_name.singular}_cache_counts") # rubocop:disable GitlabSecurity/PublicSend end + end - if issuable.time_spent? - create_time_spent_note(issuable) - end + # override if needed + def handle_changes(issuable, options) + end - create_labels_note(issuable, old_labels) if issuable.labels != old_labels + # override if needed + def execute_hooks(issuable, action = 'open', params = {}) end - def invalidate_cache_counts(issuable, users: []) - users.each do |user| - user.public_send("invalidate_#{issuable.model_name.singular}_cache_counts") # rubocop:disable GitlabSecurity/PublicSend - end + def update_project_counter_caches?(issuable) + issuable.state_changed? end end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 4c198fc96ea..9f6cfc0f6d3 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -1,10 +1,10 @@ module Issues class BaseService < ::IssuableBaseService - def hook_data(issue, action) - issue_data = issue.to_hook_data(current_user) - issue_url = Gitlab::UrlBuilder.build(issue) - issue_data[:object_attributes].merge!(url: issue_url, action: action) - issue_data + def hook_data(issue, action, old_associations: {}) + hook_data = issue.to_hook_data(current_user, old_associations: old_associations) + hook_data[:object_attributes][:action] = action + + hook_data end def reopen_service @@ -22,8 +22,8 @@ module Issues issue, issue.project, current_user, old_assignees) end - def execute_hooks(issue, action = 'open') - issue_data = hook_data(issue, action) + def execute_hooks(issue, action = 'open', old_associations: {}) + issue_data = hook_data(issue, action, old_associations: old_associations) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks issue.project.execute_hooks(issue_data, hooks_scope) issue.project.execute_services(issue_data, hooks_scope) @@ -45,5 +45,9 @@ module Issues params.delete(:assignee_ids) end end + + def update_project_counter_caches?(issue) + super || issue.confidential_changed? + end end end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 35de4337b15..62b4b4b6a1e 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -9,6 +9,7 @@ module Issues notification_service.reopen_issue(issue, current_user) execute_hooks(issue, 'reopen') invalidate_cache_counts(issue, users: issue.assignees) + issue.update_project_counter_caches end issue diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index b4ca3966505..d7aa7e2347e 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -14,9 +14,10 @@ module Issues end def handle_changes(issue, options) - old_labels = options[:old_labels] || [] - old_mentioned_users = options[:old_mentioned_users] || [] - old_assignees = options[:old_assignees] || [] + old_associations = options.fetch(:old_associations, {}) + old_labels = old_associations.fetch(:labels, []) + old_mentioned_users = old_associations.fetch(:mentioned_users, []) + old_assignees = old_associations.fetch(:assignees, []) if has_changes?(issue, old_labels: old_labels, old_assignees: old_assignees) todo_service.mark_pending_todos_as_done(issue, current_user) @@ -27,14 +28,10 @@ module Issues todo_service.update_issue(issue, current_user, old_mentioned_users) end - if issue.previous_changes.include?('milestone_id') - create_milestone_note(issue) - end - if issue.assignees != old_assignees create_assignee_note(issue, old_assignees) notification_service.reassigned_issue(issue, current_user, old_assignees) - todo_service.reassigned_issue(issue, current_user) + todo_service.reassigned_issue(issue, current_user, old_assignees) end if issue.previous_changes.include?('confidential') diff --git a/app/services/keys/base_service.rb b/app/services/keys/base_service.rb index 545832d0bd4..f78791932a7 100644 --- a/app/services/keys/base_service.rb +++ b/app/services/keys/base_service.rb @@ -4,6 +4,7 @@ module Keys def initialize(user, params) @user, @params = user, params + @ip_address = @params.delete(:ip_address) end def notification_service diff --git a/app/services/labels/promote_service.rb b/app/services/labels/promote_service.rb index 43b539ded53..997d247be46 100644 --- a/app/services/labels/promote_service.rb +++ b/app/services/labels/promote_service.rb @@ -19,6 +19,7 @@ module Labels # We skipped validations during creation. Let's run them now, after deleting conflicting labels raise ActiveRecord::RecordInvalid.new(new_label) unless new_label.valid? + new_label end end diff --git a/app/services/merge_requests/add_todo_when_build_fails_service.rb b/app/services/merge_requests/add_todo_when_build_fails_service.rb index 727768b1a39..6805b2f7d1c 100644 --- a/app/services/merge_requests/add_todo_when_build_fails_service.rb +++ b/app/services/merge_requests/add_todo_when_build_fails_service.rb @@ -3,7 +3,7 @@ module MergeRequests # Adds a todo to the parent merge_request when a CI build fails # def execute(commit_status) - return if commit_status.allow_failure? + return if commit_status.allow_failure? || commit_status.retried? commit_status_merge_requests(commit_status) do |merge_request| todo_service.merge_request_build_failed(merge_request) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 35ccff26262..6b32d65a74b 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -4,33 +4,19 @@ module MergeRequests SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, state, nil) end - def create_title_change_note(issuable, old_title) - removed_wip = MergeRequest.work_in_progress?(old_title) && !issuable.work_in_progress? - added_wip = !MergeRequest.work_in_progress?(old_title) && issuable.work_in_progress? - changed_title = MergeRequest.wipless_title(old_title) != issuable.wipless_title - - if removed_wip - SystemNoteService.remove_merge_request_wip(issuable, issuable.project, current_user) - elsif added_wip - SystemNoteService.add_merge_request_wip(issuable, issuable.project, current_user) - end - - super if changed_title - end - - def hook_data(merge_request, action, oldrev = nil) - hook_data = merge_request.to_hook_data(current_user) - hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request) + def hook_data(merge_request, action, old_rev: nil, old_associations: {}) + hook_data = merge_request.to_hook_data(current_user, old_associations: old_associations) hook_data[:object_attributes][:action] = action - if oldrev && !Gitlab::Git.blank_ref?(oldrev) - hook_data[:object_attributes][:oldrev] = oldrev + if old_rev && !Gitlab::Git.blank_ref?(old_rev) + hook_data[:object_attributes][:oldrev] = old_rev end + hook_data end - def execute_hooks(merge_request, action = 'open', oldrev = nil) + def execute_hooks(merge_request, action = 'open', old_rev: nil, old_associations: {}) if merge_request.project - merge_data = hook_data(merge_request, action, oldrev) + merge_data = hook_data(merge_request, action, old_rev: old_rev, old_associations: old_associations) merge_request.project.execute_hooks(merge_data, :merge_request_hooks) merge_request.project.execute_services(merge_data, :merge_request_hooks) end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index bc0e7ad4e39..c2fb01466df 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -1,6 +1,8 @@ module MergeRequests class BuildService < MergeRequests::BaseService def execute + @issue_iid = params.delete(:issue_iid) + self.merge_request = MergeRequest.new(params) merge_request.compare_commits = [] merge_request.source_project = find_source_project @@ -18,7 +20,17 @@ module MergeRequests attr_accessor :merge_request - delegate :target_branch, :source_branch, :source_project, :target_project, :compare_commits, :wip_title, :description, :errors, to: :merge_request + delegate :target_branch, + :target_branch_ref, + :target_project, + :source_branch, + :source_branch_ref, + :source_project, + :compare_commits, + :wip_title, + :description, + :errors, + to: :merge_request def find_source_project return source_project if source_project.present? && can?(current_user, :read_project, source_project) @@ -28,6 +40,7 @@ module MergeRequests def find_target_project return target_project if target_project.present? && can?(current_user, :read_project, target_project) + project.default_merge_request_target end @@ -53,10 +66,10 @@ module MergeRequests def compare_branches compare = CompareService.new( source_project, - source_branch + source_branch_ref ).execute( target_project, - target_branch + target_branch_ref ) if compare @@ -105,37 +118,53 @@ module MergeRequests # more than one commit in the MR # def assign_title_and_description - if match = source_branch.match(/\A(\d+)-/) - iid = match[1] - end + assign_title_and_description_from_single_commit + assign_title_from_issue - commits = compare_commits - if commits && commits.count == 1 - commit = commits.first - merge_request.title = commit.title - merge_request.description ||= commit.description.try(:strip) - elsif iid && issue = target_project.get_issue(iid, current_user) - case issue - when Issue - merge_request.title = "Resolve \"#{issue.title}\"" - when ExternalIssue - merge_request.title = "Resolve #{issue.title}" - end + merge_request.title ||= source_branch.titleize.humanize + merge_request.title = wip_title if compare_commits.empty? + + append_closes_description + end + + def append_closes_description + return unless issue_iid + + closes_issue = "Closes ##{issue_iid}" + + if description.present? + merge_request.description += closes_issue.prepend("\n\n") else - merge_request.title = source_branch.titleize.humanize + merge_request.description = closes_issue end + end + + def assign_title_and_description_from_single_commit + commits = compare_commits + + return unless commits&.count == 1 + + commit = commits.first + merge_request.title ||= commit.title + merge_request.description ||= commit.description.try(:strip) + end - if iid - closes_issue = "Closes ##{iid}" + def assign_title_from_issue + return unless issue - if description.present? - merge_request.description += closes_issue.prepend("\n\n") - else - merge_request.description = closes_issue + merge_request.title = + case issue + when Issue then "Resolve \"#{issue.title}\"" + when ExternalIssue then "Resolve #{issue.title}" end - end + end + + def issue_iid + @issue_iid ||= source_branch.match(/\A(\d+)-/).try(:[], 1) + end - merge_request.title = wip_title if commits.empty? + def issue + @issue ||= target_project.get_issue(issue_iid, current_user) end end end diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb index 9835606812c..0f677a996f7 100644 --- a/app/services/merge_requests/conflicts/list_service.rb +++ b/app/services/merge_requests/conflicts/list_service.rb @@ -23,13 +23,13 @@ module MergeRequests # when there are no conflict files. conflicts.files.each(&:lines) @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0 - rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing + rescue Rugged::OdbError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing @conflicts_can_be_resolved_in_ui = false end end def conflicts - @conflicts ||= Gitlab::Conflict::FileCollection.read_only(merge_request) + @conflicts ||= Gitlab::Conflict::FileCollection.new(merge_request) end end end diff --git a/app/services/merge_requests/conflicts/resolve_service.rb b/app/services/merge_requests/conflicts/resolve_service.rb index 6b6e231f4f9..27cafd2d7d9 100644 --- a/app/services/merge_requests/conflicts/resolve_service.rb +++ b/app/services/merge_requests/conflicts/resolve_service.rb @@ -1,54 +1,10 @@ module MergeRequests module Conflicts class ResolveService < MergeRequests::Conflicts::BaseService - MissingFiles = Class.new(Gitlab::Conflict::ResolutionError) - def execute(current_user, params) - rugged = merge_request.source_project.repository.rugged - - Gitlab::Conflict::FileCollection.for_resolution(merge_request) do |conflicts_for_resolution| - merge_index = conflicts_for_resolution.merge_index - - params[:files].each do |file_params| - conflict_file = conflicts_for_resolution.file_for_path(file_params[:old_path], file_params[:new_path]) - - write_resolved_file_to_index(merge_index, rugged, conflict_file, file_params) - end - - unless merge_index.conflicts.empty? - missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } - - raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" - end - - commit_params = { - message: params[:commit_message] || conflicts_for_resolution.default_commit_message, - parents: [conflicts_for_resolution.our_commit, conflicts_for_resolution.their_commit].map(&:oid), - tree: merge_index.write_tree(rugged) - } - - conflicts_for_resolution - .project - .repository - .resolve_conflicts(current_user, merge_request.source_branch, commit_params) - end - end - - private - - def write_resolved_file_to_index(merge_index, rugged, file, params) - if params[:sections] - new_file = file.resolve_lines(params[:sections]).map(&:text).join("\n") - - new_file << "\n" if file.our_blob.data.ends_with?("\n") - elsif params[:content] - new_file = file.resolve_content(params[:content]) - end - - our_path = file.our_path + conflicts = Gitlab::Conflict::FileCollection.new(merge_request) - merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) - merge_index.conflict_remove(our_path) + conflicts.resolve(current_user, params[:commit_message], params[:files]) end end end diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb index da39a380451..89dab1dd028 100644 --- a/app/services/merge_requests/create_from_issue_service.rb +++ b/app/services/merge_requests/create_from_issue_service.rb @@ -1,7 +1,18 @@ module MergeRequests class CreateFromIssueService < MergeRequests::CreateService + def initialize(project, user, params) + # branch - the name of new branch + # ref - the source of new branch. + + @branch_name = params[:branch_name] + @issue_iid = params[:issue_iid] + @ref = params[:ref] + + super(project, user) + end + def execute - return error('Invalid issue iid') unless issue_iid.present? && issue.present? + return error('Invalid issue iid') unless @issue_iid.present? && issue.present? params[:label_ids] = issue.label_ids if issue.label_ids.any? @@ -21,20 +32,16 @@ module MergeRequests private - def issue_iid - @isssue_iid ||= params.delete(:issue_iid) - end - def issue - @issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: issue_iid) + @issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: @issue_iid) end def branch_name - @branch_name ||= issue.to_branch_name + @branch ||= @branch_name || issue.to_branch_name end def ref - project.default_branch || 'master' + @ref || project.default_branch || 'master' end def merge_request @@ -43,6 +50,7 @@ module MergeRequests def merge_request_params { + issue_iid: @issue_iid, source_project_id: project.id, source_branch: branch_name, target_project_id: project.id, diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index a110abf8256..cedfcb50e09 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -10,6 +10,8 @@ module MergeRequests attr_reader :merge_request, :source + delegate :merge_jid, :state, to: :@merge_request + def execute(merge_request) if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService) FfMergeService.new(project, current_user, params).execute(merge_request) @@ -18,15 +20,7 @@ module MergeRequests @merge_request = merge_request - unless @merge_request.mergeable? - return handle_merge_error(log_message: 'Merge request is not mergeable', save_message_on_model: true) - end - - @source = find_merge_source - - unless @source - return handle_merge_error(log_message: 'No source for merge', save_message_on_model: true) - end + error_check! merge_request.in_locked_state do if commit @@ -35,16 +29,32 @@ module MergeRequests success end end + log_info("Merge process finished on JID #{merge_jid} with state #{state}") rescue MergeError => e handle_merge_error(log_message: e.message, save_message_on_model: true) end private + def error_check! + error = + if @merge_request.should_be_rebased? + 'Only fast-forward merge is allowed for your project. Please update your source branch' + elsif !@merge_request.mergeable? + 'Merge request is not mergeable' + elsif !source + 'No source for merge' + end + + raise MergeError, error if error + end + def commit message = params[:commit_message] || merge_request.merge_commit_message + log_info("Git merge started on JID #{merge_jid}") commit_id = repository.merge(current_user, source, merge_request, message) + log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") raise MergeError, 'Conflicts detected during merge' unless commit_id @@ -58,15 +68,13 @@ module MergeRequests end def after_merge + log_info("Post merge started on JID #{merge_jid} with state #{state}") MergeRequests::PostMergeService.new(project, current_user).execute(merge_request) + log_info("Post merge finished on JID #{merge_jid} with state #{state}") - if params[:should_remove_source_branch].present? || @merge_request.force_remove_source_branch? - # Verify again that the source branch can be removed, since branch may be protected, - # or the source branch may have been updated. - if @merge_request.can_remove_source_branch?(branch_deletion_user) - DeleteBranchService.new(@merge_request.source_project, branch_deletion_user) - .execute(merge_request.source_branch) - end + if delete_source_branch? + DeleteBranchService.new(@merge_request.source_project, branch_deletion_user) + .execute(merge_request.source_branch) end end @@ -78,24 +86,30 @@ module MergeRequests @merge_request.force_remove_source_branch? ? @merge_request.author : current_user end - # Logs merge error message and cleans `MergeRequest#merge_jid`. + # Verify again that the source branch can be removed, since branch may be protected, + # or the source branch may have been updated, or the user may not have permission # + def delete_source_branch? + params.fetch('should_remove_source_branch', @merge_request.force_remove_source_branch?) && + @merge_request.can_remove_source_branch?(branch_deletion_user) + end + def handle_merge_error(log_message:, save_message_on_model: false) Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") + @merge_request.update(merge_error: log_message) if save_message_on_model + end - if save_message_on_model - @merge_request.update(merge_error: log_message, merge_jid: nil) - else - clean_merge_jid - end + def log_info(message) + @logger ||= Rails.logger + @logger.info("#{merge_request_info} - #{message}") end def merge_request_info merge_request.to_reference(full: true) end - def find_merge_source - merge_request.diff_head_sha + def source + @source ||= @merge_request.diff_head_sha end end end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index bc4a13cf4bc..bf3d4855122 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -35,7 +35,7 @@ module MergeRequests # target branch manually def close_merge_requests commit_ids = @commits.map(&:id) - merge_requests = @project.merge_requests.preload(:merge_request_diff).opened.where(target_branch: @branch_name).to_a + merge_requests = @project.merge_requests.preload(:latest_merge_request_diff).opened.where(target_branch: @branch_name).to_a merge_requests = merge_requests.select(&:diff_head_commit) merge_requests = merge_requests.select do |merge_request| @@ -166,7 +166,7 @@ module MergeRequests # Call merge request webhook with update branches def execute_mr_web_hooks merge_requests_for_source_branch.each do |merge_request| - execute_hooks(merge_request, 'update', @oldrev) + execute_hooks(merge_request, 'update', old_rev: @oldrev) end end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index b9c65be36ec..c599a90f9fe 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -11,6 +11,7 @@ module MergeRequests merge_request.reload_diff(current_user) merge_request.mark_as_unchecked invalidate_cache_counts(merge_request, users: merge_request.assignees) + merge_request.update_project_counter_caches end merge_request diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 2832d893e95..c153872c874 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -22,8 +22,9 @@ module MergeRequests end def handle_changes(merge_request, options) - old_labels = options[:old_labels] || [] - old_mentioned_users = options[:old_mentioned_users] || [] + old_associations = options.fetch(:old_associations, {}) + old_labels = old_associations.fetch(:labels, []) + old_mentioned_users = old_associations.fetch(:mentioned_users, []) if has_changes?(merge_request, old_labels: old_labels) todo_service.mark_pending_todos_as_done(merge_request, current_user) @@ -40,10 +41,6 @@ module MergeRequests merge_request.target_branch) end - if merge_request.previous_changes.include?('milestone_id') - create_milestone_note(merge_request) - end - if merge_request.previous_changes.include?('assignee_id') create_assignee_note(merge_request) notification_service.reassigned_merge_request(merge_request, current_user) @@ -111,5 +108,11 @@ module MergeRequests end end end + + def create_branch_change_note(issuable, branch_type, old_branch, new_branch) + SystemNoteService.change_branch( + issuable, issuable.project, current_user, branch_type, + old_branch, new_branch) + end end end diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index a02eee4961b..6b3939aeba5 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -6,8 +6,7 @@ class MetricsService Gitlab::HealthChecks::Redis::RedisCheck, Gitlab::HealthChecks::Redis::CacheCheck, Gitlab::HealthChecks::Redis::QueuesCheck, - Gitlab::HealthChecks::Redis::SharedStateCheck, - Gitlab::HealthChecks::FsShardsCheck + Gitlab::HealthChecks::Redis::SharedStateCheck ].freeze def prometheus_metrics_text diff --git a/app/services/milestones/promote_service.rb b/app/services/milestones/promote_service.rb new file mode 100644 index 00000000000..2187f26d1ed --- /dev/null +++ b/app/services/milestones/promote_service.rb @@ -0,0 +1,85 @@ +module Milestones + class PromoteService < Milestones::BaseService + PromoteMilestoneError = Class.new(StandardError) + + def execute(milestone) + check_project_milestone!(milestone) + + Milestone.transaction do + group_milestone = clone_project_milestone(milestone) + + move_children_to_group_milestone(group_milestone) + + # Destroy all milestones with same title across projects + destroy_old_milestones(milestone) + + # Rollback if milestone is not valid + unless group_milestone.valid? + raise_error(group_milestone.errors.full_messages.to_sentence) + end + + group_milestone + end + end + + private + + def milestone_ids_for_merge(group_milestone) + # Pluck need to be used here instead of select so the array of ids + # is persistent after old milestones gets deleted. + @milestone_ids_for_merge ||= begin + search_params = { title: group_milestone.title, project_ids: group_project_ids, state: 'all' } + milestones = MilestonesFinder.new(search_params).execute + milestones.pluck(:id) + end + end + + def move_children_to_group_milestone(group_milestone) + milestone_ids_for_merge(group_milestone).in_groups_of(100, false) do |milestone_ids| + update_children(group_milestone, milestone_ids) + end + end + + def check_project_milestone!(milestone) + raise_error('Only project milestones can be promoted.') unless milestone.project_milestone? + end + + def clone_project_milestone(milestone) + params = milestone.slice(:title, :description, :start_date, :due_date, :state_event) + + create_service = CreateService.new(group, current_user, params) + + milestone = create_service.execute + + # milestone won't be valid here because of duplicated title + milestone.save(validate: false) + + milestone + end + + def update_children(group_milestone, milestone_ids) + issues = Issue.where(project_id: group_project_ids, milestone_id: milestone_ids) + merge_requests = MergeRequest.where(source_project_id: group_project_ids, milestone_id: milestone_ids) + + [issues, merge_requests].each do |issuable_collection| + issuable_collection.update_all(milestone_id: group_milestone.id) + end + end + + def group + @group ||= parent.group || raise_error('Project does not belong to a group.') + end + + def destroy_old_milestones(milestone) + Milestone.where(id: milestone_ids_for_merge(milestone)).destroy_all + end + + def group_project_ids + @group_project_ids ||= group.projects.pluck(:id) + end + + def raise_error(message) + raise PromoteMilestoneError, "Promotion failed - #{message}" + end + end +end diff --git a/app/services/notes/render_service.rb b/app/services/notes/render_service.rb new file mode 100644 index 00000000000..a77e98c2b07 --- /dev/null +++ b/app/services/notes/render_service.rb @@ -0,0 +1,21 @@ +module Notes + class RenderService < BaseRenderer + # Renders a collection of Note instances. + # + # notes - The notes to render. + # project - The project to use for redacting. + # user - The user viewing the notes. + + # Possible options: + # requested_path - The request path. + # project_wiki - The project's wiki. + # ref - The current Git reference. + # only_path - flag to turn relative paths into absolute ones. + # xhtml - flag to save the html in XHTML + def execute(notes, project, **opts) + renderer = Banzai::ObjectRenderer.new(project, current_user, **opts) + + renderer.render(notes, :note) + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 8d5da459882..be3b4b2ba07 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -390,7 +390,7 @@ class NotificationService end def relabeled_resource_email(target, labels, current_user, method) - recipients = labels.flat_map { |l| l.subscribers(target.project) } + recipients = labels.flat_map { |l| l.subscribers(target.project) }.uniq recipients = notifiable_users( recipients, :subscription, target: target, diff --git a/app/services/projects/count_service.rb b/app/services/projects/count_service.rb index aa034315280..7e575b2d6f3 100644 --- a/app/services/projects/count_service.rb +++ b/app/services/projects/count_service.rb @@ -1,7 +1,7 @@ module Projects # Base class for the various service classes that count project data (e.g. # issues or forks). - class CountService + class CountService < BaseCountService # The version of the cache format. This should be bumped whenever the # underlying logic changes. This removes the need for explicitly flushing # all caches. @@ -11,29 +11,6 @@ module Projects @project = project end - def relation_for_count - raise( - NotImplementedError, - '"relation_for_count" must be implemented and return an ActiveRecord::Relation' - ) - end - - def count - Rails.cache.fetch(cache_key) { uncached_count } - end - - def refresh_cache - Rails.cache.write(cache_key, uncached_count) - end - - def uncached_count - relation_for_count.count - end - - def delete_cache - Rails.cache.delete(cache_key) - end - def cache_key_name raise( NotImplementedError, diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 19d75ff2efa..81972df9b3c 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -51,7 +51,7 @@ module Projects end def wiki_path - repo_path + '.wiki' + project.wiki.disk_path end def trash_repositories! diff --git a/app/services/projects/forks_count_service.rb b/app/services/projects/forks_count_service.rb index 3a0fa84b868..d9bdf3a8ad7 100644 --- a/app/services/projects/forks_count_service.rb +++ b/app/services/projects/forks_count_service.rb @@ -1,6 +1,6 @@ module Projects # Service class for getting and caching the number of forks of a project. - class ForksCountService < CountService + class ForksCountService < Projects::CountService def relation_for_count @project.forks end diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb new file mode 100644 index 00000000000..35624577024 --- /dev/null +++ b/app/services/projects/group_links/create_service.rb @@ -0,0 +1,15 @@ +module Projects + module GroupLinks + class CreateService < BaseService + def execute(group) + return false unless group + + project.project_group_links.create( + group: group, + group_access: params[:link_group_access], + expires_at: params[:expires_at] + ) + end + end + end +end diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb new file mode 100644 index 00000000000..e3a20b4c1e4 --- /dev/null +++ b/app/services/projects/group_links/destroy_service.rb @@ -0,0 +1,11 @@ +module Projects + module GroupLinks + class DestroyService < BaseService + def execute(group_link) + return false unless group_link + + group_link.destroy + end + end + end +end diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb new file mode 100644 index 00000000000..f8aaec8a9c0 --- /dev/null +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -0,0 +1,54 @@ +module Projects + module HashedStorage + AttachmentMigrationError = Class.new(StandardError) + + class MigrateAttachmentsService < BaseService + attr_reader :logger, :old_path, :new_path + + def initialize(project, logger = nil) + @project = project + @logger = logger || Rails.logger + end + + def execute + @old_path = project.full_path + @new_path = project.disk_path + + origin = FileUploader.dynamic_path_segment(project) + project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments] + target = FileUploader.dynamic_path_segment(project) + + result = move_folder!(origin, target) + project.save! + + if result && block_given? + yield + end + + result + end + + private + + def move_folder!(old_path, new_path) + unless File.directory?(old_path) + logger.info("Skipped attachments migration from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})") + return + end + + if File.exist?(new_path) + logger.error("Cannot migrate attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") + raise AttachmentMigrationError, "Target path '#{new_path}' already exist" + end + + # Create hashed storage base path folder + FileUtils.mkdir_p(File.dirname(new_path)) + + FileUtils.mv(old_path, new_path) + logger.info("Migrated project attachments from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})") + + true + end + end + end +end diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb new file mode 100644 index 00000000000..7212e7524ab --- /dev/null +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -0,0 +1,70 @@ +module Projects + module HashedStorage + class MigrateRepositoryService < BaseService + include Gitlab::ShellAdapter + + attr_reader :old_disk_path, :new_disk_path, :old_wiki_disk_path, :old_storage_version, :logger + + def initialize(project, logger = nil) + @project = project + @logger = logger || Rails.logger + end + + def execute + @old_disk_path = project.disk_path + has_wiki = project.wiki.repository_exists? + + @old_storage_version = project.storage_version + project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository] + project.ensure_storage_path_exists + + @new_disk_path = project.disk_path + + result = move_repository(@old_disk_path, @new_disk_path) + + if has_wiki + @old_wiki_disk_path = "#{@old_disk_path}.wiki" + result &&= move_repository("#{@old_wiki_disk_path}", "#{@new_disk_path}.wiki") + end + + unless result + rollback_folder_move + project.storage_version = nil + end + + project.repository_read_only = false + project.save! + + if result && block_given? + yield + end + + result + end + + private + + def move_repository(from_name, to_name) + from_exists = gitlab_shell.exists?(project.repository_storage_path, "#{from_name}.git") + to_exists = gitlab_shell.exists?(project.repository_storage_path, "#{to_name}.git") + + # If we don't find the repository on either original or target we should log that as it could be an issue if the + # project was not originally empty. + if !from_exists && !to_exists + logger.warn "Can't find a repository on either source or target paths for #{project.full_path} (ID=#{project.id}) ..." + return false + elsif !from_exists + # Repository have been moved already. + return true + end + + gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) + end + + def rollback_folder_move + move_repository(@new_disk_path, @old_disk_path) + move_repository("#{@new_disk_path}.wiki", "#{@old_disk_path}.wiki") + end + end + end +end diff --git a/app/services/projects/hashed_storage_migration_service.rb b/app/services/projects/hashed_storage_migration_service.rb index 41259de3a16..662702c1db5 100644 --- a/app/services/projects/hashed_storage_migration_service.rb +++ b/app/services/projects/hashed_storage_migration_service.rb @@ -1,68 +1,22 @@ module Projects class HashedStorageMigrationService < BaseService - include Gitlab::ShellAdapter - - attr_reader :old_disk_path, :new_disk_path + attr_reader :logger def initialize(project, logger = nil) @project = project - @logger ||= Rails.logger + @logger = logger || Rails.logger end def execute - return if project.hashed_storage? - - @old_disk_path = project.disk_path - has_wiki = project.wiki.repository_exists? - - project.storage_version = Storage::HashedProject::STORAGE_VERSION - project.ensure_storage_path_exists - - @new_disk_path = project.disk_path - - result = move_repository(@old_disk_path, @new_disk_path) - - if has_wiki - result &&= move_repository("#{@old_disk_path}.wiki", "#{@new_disk_path}.wiki") - end - - unless result - rollback_folder_move - return + # Migrate repository from Legacy to Hashed Storage + unless project.hashed_storage?(:repository) + return unless HashedStorage::MigrateRepositoryService.new(project, logger).execute end - project.repository_read_only = false - project.save! - - block_given? ? yield : result - end - - private - - def move_repository(from_name, to_name) - from_exists = gitlab_shell.exists?(project.repository_storage_path, "#{from_name}.git") - to_exists = gitlab_shell.exists?(project.repository_storage_path, "#{to_name}.git") - - # If we don't find the repository on either original or target we should log that as it could be an issue if the - # project was not originally empty. - if !from_exists && !to_exists - logger.warn "Can't find a repository on either source or target paths for #{project.full_path} (ID=#{project.id}) ..." - return false - elsif !from_exists - # Repository have been moved already. - return true + # Migrate attachments from Legacy to Hashed Storage + unless project.hashed_storage?(:attachments) + HashedStorage::MigrateAttachmentsService.new(project, logger).execute end - - gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) - end - - def rollback_folder_move - move_repository(@new_disk_path, @old_disk_path) - move_repository("#{@new_disk_path}.wiki", "#{@old_disk_path}.wiki") - end - - def logger - @logger end end end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index c3bf0031409..f2d676af5c3 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -4,8 +4,18 @@ module Projects Error = Class.new(StandardError) + # Returns true if this importer is supposed to perform its work in the + # background. + # + # This method will only return `true` if async importing is explicitly + # supported by an importer class (`Gitlab::GithubImport::ParallelImporter` + # for example). + def async? + has_importer? && !!importer_class.try(:async?) + end + def execute - add_repository_to_project unless project.gitlab_project_import? + add_repository_to_project import_data @@ -17,6 +27,14 @@ module Projects private def add_repository_to_project + if project.external_import? && !unknown_url? + raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) + end + + # We should skip the repository for a GitHub import or GitLab project import, + # because these importers fetch the project repositories for us. + return if has_importer? && importer_class.try(:imports_repository?) + if unknown_url? # In this case, we only want to import issues, not a repository. create_repository @@ -32,19 +50,16 @@ module Projects end def import_repository - raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) - - # We should return early for a GitHub import because the new GitHub - # importer fetch the project repositories for us. - return if project.github_import? - begin - if project.gitea_import? - fetch_repository + refmap = importer_class.try(:refmap) if has_importer? + + if refmap + project.ensure_repository + project.repository.fetch_as_mirror(project.import_url, refmap: refmap) else - clone_repository + gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, project.import_url) end - rescue Gitlab::Shell::Error => e + rescue Gitlab::Shell::Error, Gitlab::Git::RepositoryMirroring::RemoteError => e # Expire cache to prevent scenarios such as: # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true # 2. Retried import, repo is broken or not imported but +exists?+ still returns true @@ -54,17 +69,6 @@ module Projects end end - def clone_repository - gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, project.import_url) - end - - def fetch_repository - project.ensure_repository - project.repository.add_remote(project.import_type, project.import_url) - project.repository.set_remote_as_mirror(project.import_type) - project.repository.fetch_remote(project.import_type, forced: true) - end - def import_data return unless has_importer? @@ -75,12 +79,16 @@ module Projects end end + def importer_class + @importer_class ||= Gitlab::ImportSources.importer(project.import_type) + end + def has_importer? Gitlab::ImportSources.importer_names.include?(project.import_type) end def importer - Gitlab::ImportSources.importer(project.import_type).new(project) + importer_class.new(project) end def unknown_url? diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb index 3c0d186a73c..25de97325e2 100644 --- a/app/services/projects/open_issues_count_service.rb +++ b/app/services/projects/open_issues_count_service.rb @@ -1,7 +1,7 @@ module Projects # Service class for counting and caching the number of open issues of a # project. - class OpenIssuesCountService < CountService + class OpenIssuesCountService < Projects::CountService def relation_for_count # We don't include confidential issues in this number since this would # expose the number of confidential issues to non project members. diff --git a/app/services/projects/open_merge_requests_count_service.rb b/app/services/projects/open_merge_requests_count_service.rb index 2a90f78b90d..77e6448fd5e 100644 --- a/app/services/projects/open_merge_requests_count_service.rb +++ b/app/services/projects/open_merge_requests_count_service.rb @@ -1,7 +1,7 @@ module Projects # Service class for counting and caching the number of open merge requests of # a project. - class OpenMergeRequestsCountService < CountService + class OpenMergeRequestsCountService < Projects::CountService def relation_for_count @project.merge_requests.opened end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 5957f612e84..e5cd6fcdfe3 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -60,21 +60,14 @@ module Projects # Notifications project.send_move_instructions(@old_path) - # Move main repository - # TODO: check storage type and NOOP when not using Legacy - unless move_repo_folder(@old_path, @new_path) - raise TransferError.new('Cannot move project') - end - - # Move wiki repo also if present - # TODO: check storage type and NOOP when not using Legacy - move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + # Directories on disk + move_project_folders(project) # Move missing group labels to project Labels::TransferService.new(current_user, @old_group, project).execute # Move uploads - Gitlab::UploadsTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path) + move_project_uploads(project) # Move pages Gitlab::PagesTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path) @@ -131,5 +124,30 @@ module Projects def execute_system_hooks SystemHooksService.new.execute_hooks_for(project, :transfer) end + + def move_project_folders(project) + return if project.hashed_storage?(:repository) + + # Move main repository + unless move_repo_folder(@old_path, @new_path) + raise TransferError.new("Cannot move project") + end + + # Disk path is changed; we need to ensure we reload it + project.reload_repository! + + # Move wiki repo also if present + move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + end + + def move_project_uploads(project) + return if project.hashed_storage?(:attachments) + + Gitlab::UploadsTransfer.new.move_project( + project.path, + @old_namespace.full_path, + @new_namespace.full_path + ) + end end end diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index abe414d0c05..c499f384426 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -3,20 +3,26 @@ module Projects def execute return unless @project.forked? - @project.forked_from_project.lfs_objects.find_each do |lfs_object| - lfs_object.projects << @project + if fork_source = @project.fork_source + fork_source.lfs_objects.find_each do |lfs_object| + lfs_object.projects << @project + end + + refresh_forks_count(fork_source) end - merge_requests = @project.forked_from_project.merge_requests.opened.from_project(@project) + merge_requests = @project.fork_network + .merge_requests + .opened + .where.not(target_project: @project) + .from_project(@project) merge_requests.each do |mr| ::MergeRequests::CloseService.new(@project, @current_user).execute(mr) end - refresh_forks_count(@project.forked_from_project) - - @project.forked_project_link.destroy @project.fork_network_member.destroy + @project.forked_project_link.destroy end def refresh_forks_count(project) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 13e292a18bf..72eecc61c96 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -15,7 +15,7 @@ module Projects return error("Could not set the default branch") unless project.change_head(params[:default_branch]) end - if project.update_attributes(params.except(:default_branch)) + if project.update_attributes(update_params) if project.previous_changes.include?('path') project.rename_repo else @@ -31,8 +31,16 @@ module Projects end end + def run_auto_devops_pipeline? + params.dig(:run_auto_devops_pipeline_explicit) == 'true' || params.dig(:run_auto_devops_pipeline_implicit) == 'true' + end + private + def update_params + params.except(:default_branch, :run_auto_devops_pipeline_explicit, :run_auto_devops_pipeline_implicit) + end + def renaming_project_with_container_registry_tags? new_path = params[:path] diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/legacy_api_create_service.rb index f2040dfa03a..e358fd0374e 100644 --- a/app/services/protected_branches/api_create_service.rb +++ b/app/services/protected_branches/legacy_api_create_service.rb @@ -1,9 +1,9 @@ -# The protected branches API still uses the `developers_can_push` and `developers_can_merge` +# The branches#protect API still uses the `developers_can_push` and `developers_can_merge` # flags for backward compatibility, and so performs translation between that format and the # internal data model (separate access levels). The translation code is non-trivial, and so # lives in this service. module ProtectedBranches - class ApiCreateService < BaseService + class LegacyApiCreateService < BaseService def execute push_access_level = if params.delete(:developers_can_push) diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/legacy_api_update_service.rb index bdb0e0cc8bf..33176253ca2 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/legacy_api_update_service.rb @@ -1,9 +1,9 @@ -# The protected branches API still uses the `developers_can_push` and `developers_can_merge` +# The branches#protect API still uses the `developers_can_push` and `developers_can_merge` # flags for backward compatibility, and so performs translation between that format and the # internal data model (separate access levels). The translation code is non-trivial, and so # lives in this service. module ProtectedBranches - class ApiUpdateService < BaseService + class LegacyApiUpdateService < BaseService def execute(protected_branch) @developers_can_push = params.delete(:developers_can_push) @developers_can_merge = params.delete(:developers_can_merge) diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index a077b3584b0..06ac86cd5a9 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -381,7 +381,7 @@ module QuickActions end desc 'Add or substract spent time' - explanation do |time_spent| + explanation do |time_spent, time_spent_date| if time_spent if time_spent > 0 verb = 'Adds' @@ -394,16 +394,20 @@ module QuickActions "#{verb} #{Gitlab::TimeTrackingFormatter.output(value)} spent time." end end - params '<1h 30m | -1h 30m>' + params '<time(1h30m | -1h30m)> <date(YYYY-MM-DD)>' condition do current_user.can?(:"admin_#{issuable.to_ability_name}", issuable) end - parse_params do |raw_duration| - Gitlab::TimeTrackingFormatter.parse(raw_duration) + parse_params do |raw_time_date| + Gitlab::QuickActions::SpendTimeAndDateSeparator.new(raw_time_date).execute end - command :spend do |time_spent| + command :spend do |time_spent, time_spent_date| if time_spent - @updates[:spend_time] = { duration: time_spent, user: current_user } + @updates[:spend_time] = { + duration: time_spent, + user: current_user, + spent_at: time_spent_date + } end end @@ -458,7 +462,7 @@ module QuickActions target_branch_param.strip end command :target_branch do |branch_name| - @updates[:target_branch] = branch_name if project.repository.branch_names.include?(branch_name) + @updates[:target_branch] = branch_name if project.repository.branch_exists?(branch_name) end desc 'Move issue from one column of the board to another' diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index a1c2f8d0180..911cc919bb8 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -35,24 +35,22 @@ class SystemHooksService data[:old_path_with_namespace] = model.old_path_with_namespace end when User - data.merge!({ - name: model.name, - email: model.email, - user_id: model.id, - username: model.username - }) + data.merge!(user_data(model)) + + if event == :rename + data[:old_username] = model.username_was + end when ProjectMember data.merge!(project_member_data(model)) when Group - owner = model.owner + data.merge!(group_data(model)) - data.merge!( - name: model.name, - path: model.path, - group_id: model.id, - owner_name: owner.respond_to?(:name) ? owner.name : nil, - owner_email: owner.respond_to?(:email) ? owner.email : nil - ) + if event == :rename + data.merge!( + old_path: model.path_was, + old_full_path: model.full_path_was + ) + end when GroupMember data.merge!(group_member_data(model)) end @@ -83,7 +81,7 @@ class SystemHooksService project_id: model.id, owner_name: owner.name, owner_email: owner.respond_to?(:email) ? owner.email : "", - project_visibility: Project.visibility_levels.key(model.visibility_level_value).downcase + project_visibility: model.visibility.downcase } end @@ -104,6 +102,19 @@ class SystemHooksService } end + def group_data(model) + owner = model.owner + + { + name: model.name, + path: model.path, + full_path: model.full_path, + group_id: model.id, + owner_name: owner.try(:name), + owner_email: owner.try(:email) + } + end + def group_member_data(model) { group_name: model.group.name, @@ -116,4 +127,13 @@ class SystemHooksService group_access: model.human_access } end + + def user_data(model) + { + name: model.name, + email: model.email, + user_id: model.id, + username: model.username + } + end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 7b32e215c7f..30a5aab13bf 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -162,7 +162,6 @@ module SystemNoteService # "changed time estimate to 3d 5h" # # Returns the created Note object - def change_time_estimate(noteable, project, author) parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate) body = if noteable.time_estimate == 0 @@ -188,16 +187,17 @@ module SystemNoteService # "added 2h 30m of time spent" # # Returns the created Note object - def change_time_spent(noteable, project, author) time_spent = noteable.time_spent if time_spent == :reset body = "removed time spent" else + spent_at = noteable.spent_at parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs) action = time_spent > 0 ? 'added' : 'subtracted' body = "#{action} #{parsed_time} of time spent" + body << " at #{spent_at}" if spent_at end create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) @@ -241,14 +241,10 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'merge')) end - def remove_merge_request_wip(noteable, project, author) - body = 'unmarked as a **Work In Progress**' - - create_note(NoteSummary.new(noteable, project, author, body, action: 'title')) - end + def handle_merge_request_wip(noteable, project, author) + prefix = noteable.work_in_progress? ? "marked" : "unmarked" - def add_merge_request_wip(noteable, project, author) - body = 'marked as a **Work In Progress**' + body = "#{prefix} as a **Work In Progress**" create_note(NoteSummary.new(noteable, project, author, body, action: 'title')) end @@ -451,10 +447,6 @@ module SystemNoteService end end - def cross_reference?(note_text) - note_text =~ /\A#{cross_reference_note_prefix}/i - end - # Check if a cross-reference is disallowed # # This method prevents adding a "mentioned in !1" note on every single commit @@ -484,19 +476,8 @@ module SystemNoteService # mentioner - Mentionable object # # Returns Boolean - def cross_reference_exists?(noteable, mentioner) - # Initial scope should be system notes of this noteable type - notes = Note.system.where(noteable_type: noteable.class) - - notes = - if noteable.is_a?(Commit) - # Commits have non-integer IDs, so they're stored in `commit_id` - notes.where(commit_id: noteable.id) - else - notes.where(noteable_id: noteable.id) - end - + notes = noteable.notes.system notes_for_mentioner(mentioner, noteable, notes).exists? end @@ -593,11 +574,15 @@ module SystemNoteService def discussion_lock(issuable, author) action = issuable.discussion_locked? ? 'locked' : 'unlocked' - body = "#{action} this issue" + body = "#{action} this #{issuable.class.to_s.titleize.downcase}" create_note(NoteSummary.new(issuable, issuable.project, author, body, action: action)) end + def cross_reference?(note_text) + note_text =~ /\A#{cross_reference_note_prefix}/i + end + private def notes_for_mentioner(mentioner, noteable, notes) diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 6ee96d6a0f8..575853fd66b 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -31,20 +31,20 @@ class TodoService mark_pending_todos_as_done(issue, current_user) end - # When we destroy an issue we should: + # When we destroy an issuable we should: # # * refresh the todos count cache for the current user # - def destroy_issue(issue, current_user) - destroy_issuable(issue, current_user) + def destroy_issuable(issuable, user) + user.update_todos_count_cache end # When we reassign an issue we should: # # * create a pending todo for new assignee if issue is assigned # - def reassigned_issue(issue, current_user) - create_assignment_todo(issue, current_user) + def reassigned_issue(issue, current_user, old_assignees = []) + create_assignment_todo(issue, current_user, old_assignees) end # When create a merge request we should: @@ -72,14 +72,6 @@ class TodoService mark_pending_todos_as_done(merge_request, current_user) end - # When we destroy a merge request we should: - # - # * refresh the todos count cache for the current user - # - def destroy_merge_request(merge_request, current_user) - destroy_issuable(merge_request, current_user) - end - # When we reassign a merge request we should: # # * creates a pending todo for new assignee if merge request is assigned @@ -216,6 +208,7 @@ class TodoService def create_todos(users, attributes) Array(users).map do |user| next if pending_todos(user, attributes).exists? + todo = Todo.create(attributes.merge(user_id: user.id)) user.update_todos_count_cache todo @@ -234,10 +227,6 @@ class TodoService create_mention_todos(issuable.project, issuable, author, nil, skip_users) end - def destroy_issuable(issuable, user) - user.update_todos_count_cache - end - def toggling_tasks?(issuable) issuable.previous_changes.include?('description') && issuable.tasks? && issuable.updated_tasks.any? @@ -254,10 +243,11 @@ class TodoService create_mention_todos(project, target, author, note, skip_users) end - def create_assignment_todo(issuable, author) + def create_assignment_todo(issuable, author, old_assignees = []) if issuable.assignees.any? + assignees = issuable.assignees - old_assignees attributes = attributes_for_todo(issuable.project, issuable, author, Todo::ASSIGNED) - create_todos(issuable.assignees, attributes) + create_todos(assignees, attributes) end end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 6f05500adea..61f1568f366 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -34,7 +34,7 @@ module Users private def can_create_user? - (current_user.nil? && current_application_settings.signup_enabled?) || current_user&.admin? + (current_user.nil? && current_application_settings.allow_signup?) || current_user&.admin? end # Allowed params for creating a user (admins only) diff --git a/app/services/users/keys_count_service.rb b/app/services/users/keys_count_service.rb new file mode 100644 index 00000000000..f82d27eded9 --- /dev/null +++ b/app/services/users/keys_count_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Users + # Service class for getting the number of SSH keys that belong to a user. + class KeysCountService < BaseCountService + attr_reader :user + + # user - The User for which to get the number of SSH keys. + def initialize(user) + @user = user + end + + def relation_for_count + user.keys + end + + def raw? + # Since we're storing simple integers we don't need all of the additional + # Marshal data Rails includes by default. + true + end + + def cache_key + "users/key-count-service/#{user.id}" + end + end +end diff --git a/app/services/users/last_push_event_service.rb b/app/services/users/last_push_event_service.rb index f2bfb60604f..57e446d7f30 100644 --- a/app/services/users/last_push_event_service.rb +++ b/app/services/users/last_push_event_service.rb @@ -16,8 +16,8 @@ module Users user_cache_key ] - if event.project.forked? - keys << project_cache_key(event.project.forked_from_project) + if forked_from = event.project.forked_from_project + keys << project_cache_key(forked_from) end keys.each { |key| set_key(key, event.id) } diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb index 3a9c151cf9b..976017dfa82 100644 --- a/app/services/users/migrate_to_ghost_user_service.rb +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -25,7 +25,7 @@ module Users user.block # Reverse the user block if record migration fails - if !migrate_records && transition + if !migrate_records_in_transaction && transition transition.rollback user.save! end @@ -36,18 +36,22 @@ module Users private - def migrate_records + def migrate_records_in_transaction user.transaction(requires_new: true) do @ghost_user = User.ghost - migrate_issues - migrate_merge_requests - migrate_notes - migrate_abuse_reports - migrate_award_emojis + migrate_records end end + def migrate_records + migrate_issues + migrate_merge_requests + migrate_notes + migrate_abuse_reports + migrate_award_emojis + end + def migrate_issues user.issues.update_all(author_id: ghost_user.id) Issue.where(last_edited_by_id: user.id).update_all(last_edited_by_id: ghost_user.id) |