diff options
69 files changed, 1174 insertions, 54 deletions
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 72e76f56540..872497992e1 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -111,6 +111,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic end end + if Gitlab::Utils.to_boolean(params[:diff_head]) && @merge_request.diffable_merge_ref? + return CompareService.new(@project, @merge_request.merge_ref_head.sha) + .execute(@project, @merge_request.target_branch) + end + if @start_sha @merge_request_diff.compare_with(@start_sha) else diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f3e03ed22d7..78d815e5858 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -91,6 +91,7 @@ module Issuable validate :description_max_length_for_new_records_is_valid, on: :update before_validation :truncate_description_on_import! + after_save :store_mentions!, if: :any_mentionable_attributes_changed? scope :authored, ->(user) { where(author_id: user) } scope :recent, -> { reorder(id: :desc) } diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index b43b91699ab..d157404f7bc 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -99,18 +99,23 @@ module Mentionable # threw the `ActiveRecord::RecordNotUnique` exception in first place. self.class.safe_ensure_unique(retries: 1) do user_mention = model_user_mention + + # this may happen due to notes polymorphism, so noteable_id may point to a record that no longer exists + # as we cannot have FK on noteable_id + break if user_mention.blank? + user_mention.mentioned_users_ids = references[:mentioned_users_ids] user_mention.mentioned_groups_ids = references[:mentioned_groups_ids] user_mention.mentioned_projects_ids = references[:mentioned_projects_ids] if user_mention.has_mentions? user_mention.save! - elsif user_mention.persisted? + else user_mention.destroy! end - - true end + + true end def referenced_users @@ -218,6 +223,12 @@ module Mentionable source.select { |key, val| mentionable.include?(key) } end + def any_mentionable_attributes_changed? + self.class.mentionable_attrs.any? do |attr| + saved_changes.key?(attr.first) + end + end + # Determine whether or not a cross-reference Note has already been created between this Mentionable and # the specified target. def cross_reference_exists?(target) diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 0c679a3075b..5450357fe1b 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -39,6 +39,7 @@ class Deployment < ApplicationRecord scope :for_status, -> (status) { where(status: status) } scope :visible, -> { where(status: %i[running success failed canceled]) } + scope :stoppable, -> { where.not(on_stop: nil).where.not(deployable_id: nil).success } state_machine :status, initial: :created do event :run do diff --git a/app/models/environment.rb b/app/models/environment.rb index 973f1243e6b..4635b05fcc7 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -61,6 +61,7 @@ class Environment < ApplicationRecord scope :in_review_folder, -> { where(environment_type: "review") } scope :for_name, -> (name) { where(name: name) } scope :preload_cluster, -> { preload(last_deployment: :cluster) } + scope :auto_stoppable, -> (limit) { available.where('auto_stop_at < ?', Time.zone.now).limit(limit) } ## # Search environments which have names like the given query. @@ -107,6 +108,44 @@ class Environment < ApplicationRecord find_or_create_by(name: name) end + class << self + ## + # This method returns stop actions (jobs) for multiple environments within one + # query. It's useful to avoid N+1 problem. + # + # NOTE: The count of environments should be small~medium (e.g. < 5000) + def stop_actions + cte = cte_for_deployments_with_stop_action + ci_builds = Ci::Build.arel_table + + inner_join_stop_actions = ci_builds.join(cte.table).on( + ci_builds[:project_id].eq(cte.table[:project_id]) + .and(ci_builds[:ref].eq(cte.table[:ref])) + .and(ci_builds[:name].eq(cte.table[:on_stop])) + ).join_sources + + pipeline_ids = ci_builds.join(cte.table).on( + ci_builds[:id].eq(cte.table[:deployable_id]) + ).project(:commit_id) + + Ci::Build.joins(inner_join_stop_actions) + .with(cte.to_arel) + .where(ci_builds[:commit_id].in(pipeline_ids)) + .where(status: HasStatus::BLOCKED_STATUS) + .preload_project_and_pipeline_project + .preload(:user, :metadata, :deployment) + end + + private + + def cte_for_deployments_with_stop_action + Gitlab::SQL::CTE.new(:deployments_with_stop_action, + Deployment.where(environment_id: select(:id)) + .distinct_on_environment + .stoppable) + end + end + def clear_prometheus_reactive_cache!(query_name) cluster_prometheus_adapter&.clear_prometheus_reactive_cache!(query_name, self) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 684108b1a7a..d01aa78a2c1 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -45,7 +45,7 @@ class Issue < ApplicationRecord has_many :issue_assignees has_many :assignees, class_name: "User", through: :issue_assignees has_many :zoom_meetings - has_many :user_mentions, class_name: "IssueUserMention" + has_many :user_mentions, class_name: "IssueUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_one :sentry_issue accepts_nested_attributes_for :sentry_issue diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b8f51b58fd3..14aa6ac066e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -77,7 +77,7 @@ class MergeRequest < ApplicationRecord has_many :merge_request_assignees has_many :assignees, class_name: "User", through: :merge_request_assignees - has_many :user_mentions, class_name: "MergeRequestUserMention" + has_many :user_mentions, class_name: "MergeRequestUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :deployment_merge_requests @@ -840,6 +840,10 @@ class MergeRequest < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass + def diffable_merge_ref? + Feature.enabled?(:diff_compare_with_head, target_project) && can_be_merged? && merge_ref_head.present? + end + # Returns boolean indicating the merge_status should be rechecked in order to # switch to either can_be_merged or cannot_be_merged. def recheck_merge_status? diff --git a/app/models/note.rb b/app/models/note.rb index 8af650e27aa..97e84bb79f6 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -157,6 +157,7 @@ class Note < ApplicationRecord after_save :expire_etag_cache, unless: :importing? after_save :touch_noteable, unless: :importing? after_destroy :expire_etag_cache + after_save :store_mentions!, if: :any_mentionable_attributes_changed? class << self def model_name @@ -498,6 +499,8 @@ class Note < ApplicationRecord end def user_mentions + return Note.none unless noteable.present? + noteable.user_mentions.where(note: self) end @@ -506,6 +509,8 @@ class Note < ApplicationRecord # Using this method followed by a call to `save` may result in ActiveRecord::RecordNotUnique exception # in a multithreaded environment. Make sure to use it within a `safe_ensure_unique` block. def model_user_mention + return if user_mentions.is_a?(ActiveRecord::NullRelation) + user_mentions.first_or_initialize end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index e2b72dfde7a..4ba8e6a94e6 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -41,7 +41,7 @@ class Snippet < ApplicationRecord belongs_to :project has_many :notes, as: :noteable, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :user_mentions, class_name: "SnippetUserMention" + has_many :user_mentions, class_name: "SnippetUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_one :snippet_repository, inverse_of: :snippet delegate :name, :email, to: :author, prefix: true, allow_nil: true @@ -66,6 +66,8 @@ class Snippet < ApplicationRecord validates :visibility_level, inclusion: { in: Gitlab::VisibilityLevel.values } + after_save :store_mentions!, if: :any_mentionable_attributes_changed? + # Scopes scope :are_internal, -> { where(visibility_level: Snippet::INTERNAL) } scope :are_private, -> { where(visibility_level: Snippet::PRIVATE) } diff --git a/app/serializers/merge_request_diff_entity.rb b/app/serializers/merge_request_diff_entity.rb index 5c79b165ee9..aa0ac7d2a7e 100644 --- a/app/serializers/merge_request_diff_entity.rb +++ b/app/serializers/merge_request_diff_entity.rb @@ -34,6 +34,14 @@ class MergeRequestDiffEntity < Grape::Entity merge_request_version_path(project, merge_request, merge_request_diff) end + expose :head_version_path do |merge_request_diff| + project = merge_request.target_project + + next unless project && merge_request.diffable_merge_ref? + + diffs_project_merge_request_path(project, merge_request, diff_head: true) + end + expose :version_path do |merge_request_diff| start_sha = options[:start_sha] project = merge_request.target_project diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb index d9a800791f2..14ef744ada1 100644 --- a/app/services/ci/stop_environments_service.rb +++ b/app/services/ci/stop_environments_service.rb @@ -16,6 +16,22 @@ module Ci merge_request.environments.each { |environment| stop(environment) } end + ## + # This method is for stopping multiple environments in a batch style. + # The maximum acceptable count of environments is roughly 5000. Please + # apply acceptable `LIMIT` clause to the `environments` relation. + def self.execute_in_batch(environments) + stop_actions = environments.stop_actions.load + + environments.update_all(auto_stop_at: nil, state: 'stopped') + + stop_actions.each do |stop_action| + stop_action.play(stop_action.user) + rescue => e + Gitlab::ErrorTracking.track_error(e, deployable_id: stop_action.id) + end + end + private def environments diff --git a/app/services/environments/auto_stop_service.rb b/app/services/environments/auto_stop_service.rb new file mode 100644 index 00000000000..6eef8138493 --- /dev/null +++ b/app/services/environments/auto_stop_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Environments + class AutoStopService + include ::Gitlab::ExclusiveLeaseHelpers + include ::Gitlab::LoopHelpers + + BATCH_SIZE = 100 + LOOP_TIMEOUT = 45.minutes + LOOP_LIMIT = 1000 + EXCLUSIVE_LOCK_KEY = 'environments:auto_stop:lock' + LOCK_TIMEOUT = 50.minutes + + ## + # Stop expired environments on GitLab instance + # + # This auto stop process cannot run for more than 45 minutes. This is for + # preventing multiple `AutoStopCronWorker` CRON jobs run concurrently, + # which is scheduled at every hour. + def execute + in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do + loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do + stop_in_batch + end + end + end + + private + + def stop_in_batch + environments = Environment.auto_stoppable(BATCH_SIZE) + + return false unless environments.exists? && Feature.enabled?(:auto_stop_environments) + + Ci::StopEnvironmentsService.execute_in_batch(environments) + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index b59bb803778..830afbf4a43 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -168,7 +168,7 @@ class IssuableBaseService < BaseService before_create(issuable) issuable_saved = issuable.with_transaction_returning_status do - issuable.save && issuable.store_mentions! + issuable.save end if issuable_saved @@ -233,7 +233,7 @@ class IssuableBaseService < BaseService ensure_milestone_available(issuable) issuable_saved = issuable.with_transaction_returning_status do - issuable.save(touch: should_touch) && issuable.store_mentions! + issuable.save(touch: should_touch) end if issuable_saved diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 50dc98b88e9..4a0d85038ee 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -2,7 +2,6 @@ module Notes class CreateService < ::Notes::BaseService - # rubocop:disable Metrics/CyclomaticComplexity def execute note = Notes::BuildService.new(project, current_user, params.except(:merge_request_diff_head_sha)).execute @@ -34,7 +33,7 @@ module Notes end note_saved = note.with_transaction_returning_status do - !only_commands && note.save && note.store_mentions! + !only_commands && note.save end if note_saved @@ -67,7 +66,6 @@ module Notes note end - # rubocop:enable Metrics/CyclomaticComplexity private diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index c8b0dc30209..3070e7b0e53 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -10,7 +10,7 @@ module Notes note.assign_attributes(params.merge(updated_by: current_user)) note.with_transaction_returning_status do - note.save && note.store_mentions! + note.save end only_commands = false diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index 250e99c466a..51860adca77 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -24,7 +24,7 @@ module Snippets spam_check(snippet, current_user) snippet_saved = snippet.with_transaction_returning_status do - snippet.save && snippet.store_mentions! + snippet.save end if snippet_saved diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 8d2c8cac148..c0c0aec2050 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -21,7 +21,7 @@ module Snippets spam_check(snippet, current_user) snippet_saved = snippet.with_transaction_returning_status do - snippet.save && snippet.store_mentions! + snippet.save end if snippet_saved diff --git a/app/views/clusters/clusters/gcp/_form.html.haml b/app/views/clusters/clusters/gcp/_form.html.haml index 66da76d7a43..e83bf61ab9b 100644 --- a/app/views/clusters/clusters/gcp/_form.html.haml +++ b/app/views/clusters/clusters/gcp/_form.html.haml @@ -70,7 +70,7 @@ label_class: 'label-bold' } .form-text.text-muted = s_('ClusterIntegration|Uses the Cloud Run, Istio, and HTTP Load Balancing addons for this cluster.') - = link_to _('More information'), help_page_path('user/project/clusters/index.md', anchor: 'cloud-run-on-gke'), target: '_blank' + = link_to _('More information'), help_page_path('user/project/clusters/add_remove_clusters.md', anchor: 'cloud-run-for-anthos'), target: '_blank' .form-group = field.check_box :managed, { label: s_('ClusterIntegration|GitLab-managed cluster'), diff --git a/app/views/clusters/clusters/show.html.haml b/app/views/clusters/clusters/show.html.haml index 59ce28d2d26..e1f011a3225 100644 --- a/app/views/clusters/clusters/show.html.haml +++ b/app/views/clusters/clusters/show.html.haml @@ -34,7 +34,7 @@ environments_help_path: help_page_path('ci/environments', anchor: 'defining-environments'), clusters_help_path: help_page_path('user/project/clusters/index.md', anchor: 'deploying-to-a-kubernetes-cluster'), deploy_boards_help_path: help_page_path('user/project/deploy_boards.html', anchor: 'enabling-deploy-boards'), - cloud_run_help_path: help_page_path('user/project/clusters/index.md', anchor: 'cloud-run-on-gke'), + cloud_run_help_path: help_page_path('user/project/clusters/add_remove_clusters.md', anchor: 'cloud-run-for-anthos'), manage_prometheus_path: manage_prometheus_path, cluster_id: @cluster.id } } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 3bf46675ad5..5ff1a331b09 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -75,6 +75,12 @@ :latency_sensitive: :resource_boundary: :unknown :weight: 1 +- :name: cronjob:environments_auto_stop_cron + :feature_category: :continuous_delivery + :has_external_dependencies: + :latency_sensitive: + :resource_boundary: :unknown + :weight: 1 - :name: cronjob:expire_build_artifacts :feature_category: :continuous_integration :has_external_dependencies: diff --git a/app/workers/environments/auto_stop_cron_worker.rb b/app/workers/environments/auto_stop_cron_worker.rb new file mode 100644 index 00000000000..8fcda35b414 --- /dev/null +++ b/app/workers/environments/auto_stop_cron_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Environments + class AutoStopCronWorker + include ApplicationWorker + include CronjobQueue # rubocop:disable Scalability/CronWorkerContext + + feature_category :continuous_delivery + + def perform + return unless Feature.enabled?(:auto_stop_environments) + + AutoStopService.new.execute + end + end +end diff --git a/changelogs/unreleased/204856-replicating-objects-in-object-storage-schedules-removal-and-re-syn.yml b/changelogs/unreleased/204856-replicating-objects-in-object-storage-schedules-removal-and-re-syn.yml new file mode 100644 index 00000000000..5ffa950f0d4 --- /dev/null +++ b/changelogs/unreleased/204856-replicating-objects-in-object-storage-schedules-removal-and-re-syn.yml @@ -0,0 +1,6 @@ +--- +title: 'Geo: Don''t clean up files in object storage when Geo is responsible of syncing + them' +merge_request: 24901 +author: +type: fixed diff --git a/changelogs/unreleased/32079-misleading-suggestion-is-created-when-made-at-the-last-line-of-a-fi.yml b/changelogs/unreleased/32079-misleading-suggestion-is-created-when-made-at-the-last-line-of-a-fi.yml new file mode 100644 index 00000000000..ef5a400d52c --- /dev/null +++ b/changelogs/unreleased/32079-misleading-suggestion-is-created-when-made-at-the-last-line-of-a-fi.yml @@ -0,0 +1,5 @@ +--- +title: Fixes a new line issue with suggestions in the last line of a file +merge_request: 22732 +author: +type: fixed diff --git a/changelogs/unreleased/ali-fix-cloud-run-doc-links.yml b/changelogs/unreleased/ali-fix-cloud-run-doc-links.yml new file mode 100644 index 00000000000..986d131197b --- /dev/null +++ b/changelogs/unreleased/ali-fix-cloud-run-doc-links.yml @@ -0,0 +1,5 @@ +--- +title: Update broken links to Cloud Run for Anthos documentation +merge_request: 25159 +author: +type: fixed diff --git a/changelogs/unreleased/feature-flag-strategy-scope-tables.yml b/changelogs/unreleased/feature-flag-strategy-scope-tables.yml new file mode 100644 index 00000000000..7382d06b419 --- /dev/null +++ b/changelogs/unreleased/feature-flag-strategy-scope-tables.yml @@ -0,0 +1,5 @@ +--- +title: Create operations strategies and scopes tables +merge_request: 24819 +author: +type: added diff --git a/changelogs/unreleased/migration-save-instance-administrators-id.yml b/changelogs/unreleased/migration-save-instance-administrators-id.yml new file mode 100644 index 00000000000..9160518d98d --- /dev/null +++ b/changelogs/unreleased/migration-save-instance-administrators-id.yml @@ -0,0 +1,5 @@ +--- +title: Add migration to save Instance Administrators group ID in application_settings table +merge_request: 24796 +author: +type: changed diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 9515ebaea62..20c75a6e255 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -373,6 +373,9 @@ production: &base # Remove expired build artifacts expire_build_artifacts_worker: cron: "50 * * * *" + # Stop expired environments + environments_auto_stop_cron_worker: + cron: "24 * * * *" # Periodically run 'git fsck' on all repositories. If started more than # once per hour you will have concurrent 'git fsck' jobs. repository_check_worker: diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 76391983e6e..aa743416e99 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -403,6 +403,9 @@ Settings.cron_jobs['pipeline_schedule_worker']['job_class'] = 'PipelineScheduleW Settings.cron_jobs['expire_build_artifacts_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['expire_build_artifacts_worker']['cron'] ||= '50 * * * *' Settings.cron_jobs['expire_build_artifacts_worker']['job_class'] = 'ExpireBuildArtifactsWorker' +Settings.cron_jobs['environments_auto_stop_cron_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['environments_auto_stop_cron_worker']['cron'] ||= '24 * * * *' +Settings.cron_jobs['environments_auto_stop_cron_worker']['job_class'] = 'Environments::AutoStopCronWorker' Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *' Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::DispatchWorker' diff --git a/db/migrate/20200131140428_create_index_on_auto_stop_in.rb b/db/migrate/20200131140428_create_index_on_auto_stop_in.rb new file mode 100644 index 00000000000..526e7dcfe30 --- /dev/null +++ b/db/migrate/20200131140428_create_index_on_auto_stop_in.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateIndexOnAutoStopIn < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :environments, %i[state auto_stop_at], where: "auto_stop_at IS NOT NULL AND state = 'available'" + end + + def down + remove_concurrent_index :environments, %i[state auto_stop_at] + end +end diff --git a/db/migrate/20200209131152_add_feature_filter_type_to_user_preferences.rb b/db/migrate/20200209131152_add_feature_filter_type_to_user_preferences.rb new file mode 100644 index 00000000000..c05b624c438 --- /dev/null +++ b/db/migrate/20200209131152_add_feature_filter_type_to_user_preferences.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddFeatureFilterTypeToUserPreferences < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :user_preferences, :feature_filter_type, :bigint + end +end diff --git a/db/migrate/20200210184410_create_operations_strategies_table.rb b/db/migrate/20200210184410_create_operations_strategies_table.rb new file mode 100644 index 00000000000..1046bd11bc7 --- /dev/null +++ b/db/migrate/20200210184410_create_operations_strategies_table.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CreateOperationsStrategiesTable < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :operations_strategies do |t| + t.references :feature_flag, index: true, null: false, foreign_key: { to_table: :operations_feature_flags, on_delete: :cascade } + t.string :name, null: false, limit: 255 + t.jsonb :parameters, null: false, default: {} + end + end +end diff --git a/db/migrate/20200210184420_create_operations_scopes_table.rb b/db/migrate/20200210184420_create_operations_scopes_table.rb new file mode 100644 index 00000000000..0b33882fe3d --- /dev/null +++ b/db/migrate/20200210184420_create_operations_scopes_table.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateOperationsScopesTable < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :operations_scopes do |t| + t.references :strategy, null: false, index: false, foreign_key: { to_table: :operations_strategies, on_delete: :cascade } + t.string :environment_scope, null: false, limit: 255 + end + + add_index :operations_scopes, [:strategy_id, :environment_scope], unique: true + end +end diff --git a/db/post_migrate/20200210092405_save_instance_administrators_group_id.rb b/db/post_migrate/20200210092405_save_instance_administrators_group_id.rb new file mode 100644 index 00000000000..e539a187672 --- /dev/null +++ b/db/post_migrate/20200210092405_save_instance_administrators_group_id.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +class SaveInstanceAdministratorsGroupId < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + execute( + <<-SQL + UPDATE + application_settings + SET + instance_administrators_group_id = ( + SELECT + namespace_id + FROM + projects + WHERE + id = application_settings.instance_administration_project_id + ) + WHERE + instance_administrators_group_id IS NULL + AND + instance_administration_project_id IS NOT NULL + AND + ID in ( + SELECT + max(id) + FROM + application_settings + ) + SQL + ) + end + + def down + # no-op + + # The change performed by `up` cannot be reversed because once the migration runs, + # we do not know what value application_settings.instance_administrators_group_id + # had before the migration was run. + end +end diff --git a/db/schema.rb b/db/schema.rb index 2fd7341c4d9..fc30081cedf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1494,10 +1494,12 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do t.string "state", default: "available", null: false t.string "slug", null: false t.datetime_with_timezone "auto_stop_at" + t.index ["auto_stop_at"], name: "index_environments_on_auto_stop_at", where: "(auto_stop_at IS NOT NULL)" t.index ["name"], name: "index_environments_on_name_varchar_pattern_ops", opclass: :varchar_pattern_ops t.index ["project_id", "name"], name: "index_environments_on_project_id_and_name", unique: true t.index ["project_id", "slug"], name: "index_environments_on_project_id_and_slug", unique: true t.index ["project_id", "state", "environment_type"], name: "index_environments_on_project_id_state_environment_type" + t.index ["state", "auto_stop_at"], name: "index_environments_on_state_and_auto_stop_at", where: "((auto_stop_at IS NOT NULL) AND ((state)::text = 'available'::text))" end create_table "epic_issues", id: :serial, force: :cascade do |t| @@ -2934,6 +2936,19 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do t.index ["project_id", "token_encrypted"], name: "index_feature_flags_clients_on_project_id_and_token_encrypted", unique: true end + create_table "operations_scopes", force: :cascade do |t| + t.bigint "strategy_id", null: false + t.string "environment_scope", limit: 255, null: false + t.index ["strategy_id", "environment_scope"], name: "index_operations_scopes_on_strategy_id_and_environment_scope", unique: true + end + + create_table "operations_strategies", force: :cascade do |t| + t.bigint "feature_flag_id", null: false + t.string "name", limit: 255, null: false + t.jsonb "parameters", default: {}, null: false + t.index ["feature_flag_id"], name: "index_operations_strategies_on_feature_flag_id" + end + create_table "packages_build_infos", force: :cascade do |t| t.integer "package_id", null: false t.integer "pipeline_id" @@ -4169,6 +4184,7 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do t.boolean "setup_for_company" t.boolean "render_whitespace_in_code" t.integer "tab_width", limit: 2 + t.bigint "feature_filter_type" t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true end @@ -4867,6 +4883,8 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do add_foreign_key "operations_feature_flag_scopes", "operations_feature_flags", column: "feature_flag_id", on_delete: :cascade add_foreign_key "operations_feature_flags", "projects", on_delete: :cascade add_foreign_key "operations_feature_flags_clients", "projects", on_delete: :cascade + add_foreign_key "operations_scopes", "operations_strategies", column: "strategy_id", on_delete: :cascade + add_foreign_key "operations_strategies", "operations_feature_flags", column: "feature_flag_id", on_delete: :cascade add_foreign_key "packages_build_infos", "ci_pipelines", column: "pipeline_id", on_delete: :nullify add_foreign_key "packages_build_infos", "packages_packages", column: "package_id", on_delete: :cascade add_foreign_key "packages_conan_file_metadata", "packages_package_files", column: "package_file_id", on_delete: :cascade diff --git a/doc/development/geo.md b/doc/development/geo.md index 38a64cbadca..a7ce09f822f 100644 --- a/doc/development/geo.md +++ b/doc/development/geo.md @@ -546,3 +546,8 @@ old method: - Replication is synchronous and we preserve the order of events. - Replication of the events happen at the same time as the changes in the database. + +## Self-service framework + +If you want to add easy Geo replication of a resource you're working +on, check out our [self-service framework](geo/framework.md). diff --git a/doc/development/geo/framework.md b/doc/development/geo/framework.md new file mode 100644 index 00000000000..ada04cf2281 --- /dev/null +++ b/doc/development/geo/framework.md @@ -0,0 +1,144 @@ +# Geo self-service framework (alpha) + +NOTE: **Note:** This document might be subjected to change. It's a +proposal we're working on and once the implementation is complete this +documentation will be updated. Follow progress in the +[epic](https://gitlab.com/groups/gitlab-org/-/epics/2161). + +NOTE: **Note:** The Geo self-service framework is currently in +alpha. If you need to replicate a new data type, reach out to the Geo +team to discuss the options. You can contact them in `#g_geo` on Slack +or mention `@geo-team` in the issue or merge request. + +Geo provides an API to make it possible to easily replicate data types +across Geo nodes. This API is presented as a Ruby Domain-Specific +Language (DSL) and aims to make it possible to replicate data with +minimal effort of the engineer who created a data type. + +## Nomenclature + +Before digging into the API, developers need to know some Geo-specific +naming conventions. + +Model +: A model is an Active Model, which is how it is known in the entire + Rails codebase. It usually is tied to a database table. From Geo + perspective, a model can have one or more resources. + +Resource +: A resource is a piece of data that belongs to a model and is + produced by a GitLab feature. It is persisted using a storage + mechanism. By default, a resource is not a replicable. + +Data type +: Data type is how a resource is stored. Each resource should + fit in one of the data types Geo supports: +:- Git repository +:- Blob +:- Database +: For more detail, see [Data types](../../administration/geo/replication/datatypes.md). + +Geo Replicable +: A Replicable is a resource Geo wants to sync across Geo nodes. There + is a limited set of supported data types of replicables. The effort + required to implement replication of a resource that belongs to one + of the known data types is minimal. + +Geo Replicator +: A Geo Replicator is the object that knows how to replicate a + replicable. It's responsible for: +:- Firing events (producer) +:- Consuming events (consumer) +: It's tied to the Geo Replicable data type. All replicators have a + common interface that can be used to process (that is, produce and + consume) events. It takes care of the communication between the + primary node (where events are produced) and the secondary node + (where events are consumed). The engineer who wants to incorporate + Geo in their feature will use the API of replicators to make this + happen. + +Geo Domain-Specific Language +: The syntactic sugar that allows engineers to easily specify which + resources should be replicated and how. + +## Geo Domain-Specific Language + +### The replicator + +First of all, you need to write a replicator. The replicators live in +[`ee/app/replicators/geo`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/app/replicators/geo). +For each resource that needs to be replicated, there should be a +separate replicator specified, even if multiple resources are tied to +the same model. + +For example, the following replicator replicates a package file: + +```ruby +module Geo + class PackageFileReplicator < Gitlab::Geo::Replicator + # Include one of the strategies your resource needs + include ::Geo::BlobReplicatorStrategy + + # Specify the CarrierWave uploader needed by the used strategy + def carrierwave_uploader + model_record.file + end + + private + + # Specify the model this replicator belongs to + def model + ::Packages::PackageFile + end + end +end +``` + +The class name should be unique. It also is tightly coupled to the +table name for the registry, so for this example the registry table +will be `package_file_registry`. + +For the different data types Geo supports there are different +strategies to include. Pick one that fits your needs. + +### Linking to a model + +To tie this replicator to the model, you need to add the following to +the model code: + +```ruby +class Packages::PackageFile < ApplicationRecord + include ::Gitlab::Geo::ReplicableModel + + with_replicator Geo::PackageFileReplicator +end +``` + +### API + +When this is set in place, it's easy to access the replicator through +the model: + +```ruby +package_file = Packages::PackageFile.find(4) # just a random id as example +replicator = package_file.replicator +``` + +Or get the model back from the replicator: + +```ruby +replicator.model_record +=> <Packages::PackageFile id:4> +``` + +The replicator can be used to generate events, for example in +ActiveRecord hooks: + +```ruby + after_create_commit -> { replicator.publish_created_event } +``` + +#### Library + +The framework behind all this is located in +[`ee/lib/gitlab/geo/`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/lib/gitlab/geo). diff --git a/doc/user/group/epics/index.md b/doc/user/group/epics/index.md index d8e4b0d8934..96d04598b12 100644 --- a/doc/user/group/epics/index.md +++ b/doc/user/group/epics/index.md @@ -5,7 +5,7 @@ type: reference, howto # Epics **(PREMIUM)** > Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 10.2. -> In [GitLab 12.8](https://gitlab.com/gitlab-org/gitlab/issues/37081), single-level Epics were moved to **(PREMIUM)**. +> In [GitLab 12.8](https://gitlab.com/gitlab-org/gitlab/issues/37081), single-level Epics were moved to Premium tier. Epics let you manage your portfolio of projects more efficiently and with less effort by tracking groups of issues that share a theme, across projects and diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 7e3ac38b627..cf98c002331 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -204,7 +204,7 @@ You can save a copy of a GitLab defined dashboard that can be customized and ada 1. Click on the "Duplicate dashboard" in the dashboard dropdown. - NOTE:**Note:** + NOTE: **Note:** Only GitLab-defined dashboards can be duplicated. 1. Input the file name and other information, such as a new commit message, and click on "Duplicate". diff --git a/lib/gitlab/diff/suggestion_diff.rb b/lib/gitlab/diff/suggestion_diff.rb index ee153c226b7..783264fe999 100644 --- a/lib/gitlab/diff/suggestion_diff.rb +++ b/lib/gitlab/diff/suggestion_diff.rb @@ -18,7 +18,7 @@ module Gitlab private def raw_diff - "#{diff_header}\n#{from_content_as_diff}#{to_content_as_diff}" + "#{diff_header}\n#{from_content_as_diff}\n#{to_content_as_diff}" end def diff_header @@ -26,7 +26,7 @@ module Gitlab end def from_content_as_diff - from_content.lines.map { |line| line.prepend('-') }.join + from_content.lines.map { |line| line.prepend('-') }.join.delete_suffix("\n") end def to_content_as_diff diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 9ceaa742f51..7c59267c0b6 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -20,7 +20,7 @@ module Gitlab paid_signup_flow: { feature_toggle: :paid_signup_flow, environment: ::Gitlab.dev_env_or_com?, - enabled_ratio: 0.25, + enabled_ratio: 0.5, tracking_category: 'Growth::Acquisition::Experiment::PaidSignUpFlow' }, suggest_pipeline: { diff --git a/lib/gitlab/metrics/dashboard/url.rb b/lib/gitlab/metrics/dashboard/url.rb index b3cbfde828c..1239b103cde 100644 --- a/lib/gitlab/metrics/dashboard/url.rb +++ b/lib/gitlab/metrics/dashboard/url.rb @@ -7,6 +7,7 @@ module Gitlab class Url class << self include Gitlab::Utils::StrongMemoize + # Matches urls for a metrics dashboard. This could be # either the /metrics endpoint or the /metrics_dashboard # endpoint. diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 65a89d890c3..88c14e03fd8 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -203,6 +203,41 @@ describe Projects::MergeRequests::DiffsController do end end + context 'with diff_head param passed' do + before do + allow(merge_request).to receive(:diffable_merge_ref?) + .and_return(diffable_merge_ref) + end + + context 'the merge request can be compared with head' do + let(:diffable_merge_ref) { true } + + it 'compares diffs with the head' do + MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) + + expect(CompareService).to receive(:new).with( + project, merge_request.merge_ref_head.sha + ).and_call_original + + go(diff_head: true) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'the merge request cannot be compared with head' do + let(:diffable_merge_ref) { false } + + it 'compares diffs with the base' do + expect(CompareService).not_to receive(:new) + + go(diff_head: true) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + context 'with MR regular diff params' do it 'returns success' do go diff --git a/spec/factories/environments.rb b/spec/factories/environments.rb index 323ea2d478b..998672ebe7c 100644 --- a/spec/factories/environments.rb +++ b/spec/factories/environments.rb @@ -45,7 +45,7 @@ FactoryBot.define do self.when { 'manual' } end - trait :auto_stopped do + trait :auto_stoppable do auto_stop_at { 1.day.ago } end diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb index 666a45485b9..4a20d1b7d60 100644 --- a/spec/features/cycle_analytics_spec.rb +++ b/spec/features/cycle_analytics_spec.rb @@ -40,9 +40,6 @@ describe 'Value Stream Analytics', :js do context "when there's value stream analytics data" do before do - allow_next_instance_of(Gitlab::ReferenceExtractor) do |instance| - allow(instance).to receive(:issues).and_return([issue]) - end project.add_maintainer(user) @build = create_cycle(user, project, issue, mr, milestone, pipeline) @@ -101,9 +98,6 @@ describe 'Value Stream Analytics', :js do project.add_developer(user) project.add_guest(guest) - allow_next_instance_of(Gitlab::ReferenceExtractor) do |instance| - allow(instance).to receive(:issues).and_return([issue]) - end create_cycle(user, project, issue, mr, milestone, pipeline) deploy_master(user, project) diff --git a/spec/features/markdown/metrics_spec.rb b/spec/features/markdown/metrics_spec.rb index e7fec41fae3..69e93268b57 100644 --- a/spec/features/markdown/metrics_spec.rb +++ b/spec/features/markdown/metrics_spec.rb @@ -93,10 +93,20 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek # Ensure we identify urls with the appropriate host. # Configure host to include port in app: Gitlab.config.gitlab[:url] = root_url.chomp('/') + + clear_host_from_memoized_variables end def restore_host default_url_options[:host] = @original_default_host Gitlab.config.gitlab[:url] = @original_gitlab_url + + clear_host_from_memoized_variables + end + + def clear_host_from_memoized_variables + [:metrics_regex, :grafana_regex].each do |method_name| + Gitlab::Metrics::Dashboard::Url.clear_memoization(method_name) + end end end diff --git a/spec/lib/gitlab/diff/suggestion_diff_spec.rb b/spec/lib/gitlab/diff/suggestion_diff_spec.rb index 5a32c2bea37..0d4fe33bc47 100644 --- a/spec/lib/gitlab/diff/suggestion_diff_spec.rb +++ b/spec/lib/gitlab/diff/suggestion_diff_spec.rb @@ -51,5 +51,20 @@ describe Gitlab::Diff::SuggestionDiff do expect(diff_lines[index].to_hash).to include(expected_line) end end + + describe 'when the suggestion is for the last line of a file' do + it 'returns a correct value if there is no newline at the end of the file' do + from_content = "One line test" + to_content = "Successful test!" + suggestion = instance_double(Suggestion, from_line: 1, + from_content: from_content, + to_content: to_content) + + diff_lines = described_class.new(suggestion).diff_lines + + expect(diff_lines.first.text).to eq("-One line test") + expect(diff_lines.last.text).to eq("+Successful test!") + end + end end end diff --git a/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb b/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb index 6f90798f815..56ec6ec0f59 100644 --- a/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb @@ -216,8 +216,6 @@ describe Gitlab::ImportExport::FastHashSerializer do end def setup_project - issue = create(:issue, assignees: [user]) - snippet = create(:project_snippet) release = create(:release) group = create(:group) @@ -228,12 +226,14 @@ describe Gitlab::ImportExport::FastHashSerializer do :wiki_enabled, :builds_private, description: 'description', - issues: [issue], - snippets: [snippet], releases: [release], group: group, approvals_before_merge: 1 ) + allow(project).to receive(:commit).and_return(Commit.new(RepoHelpers.sample_commit, project)) + + issue = create(:issue, assignees: [user], project: project) + snippet = create(:project_snippet, project: project) project_label = create(:label, project: project) group_label = create(:group_label, group: group) create(:label_link, label: project_label, target: issue) diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 29d0099d5c1..126ac289a56 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -334,8 +334,6 @@ describe Gitlab::ImportExport::ProjectTreeSaver do end def setup_project - issue = create(:issue, assignees: [user]) - snippet = create(:project_snippet) release = create(:release) group = create(:group) @@ -346,12 +344,14 @@ describe Gitlab::ImportExport::ProjectTreeSaver do :wiki_enabled, :builds_private, description: 'description', - issues: [issue], - snippets: [snippet], releases: [release], group: group, approvals_before_merge: 1 ) + allow(project).to receive(:commit).and_return(Commit.new(RepoHelpers.sample_commit, project)) + + issue = create(:issue, assignees: [user], project: project) + snippet = create(:project_snippet, project: project) project_label = create(:label, project: project) group_label = create(:group_label, group: group) create(:label_link, label: project_label, target: issue) diff --git a/spec/lib/gitlab/metrics/dashboard/url_spec.rb b/spec/lib/gitlab/metrics/dashboard/url_spec.rb index d98aa5e3697..9ccd1c06d6b 100644 --- a/spec/lib/gitlab/metrics/dashboard/url_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/url_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Metrics::Dashboard::Url do - describe '#regex' do + describe '#metrics_regex' do let(:url) do Gitlab::Routing.url_helpers.metrics_namespace_project_environment_url( 'foo', diff --git a/spec/migrations/save_instance_administrators_group_id_spec.rb b/spec/migrations/save_instance_administrators_group_id_spec.rb new file mode 100644 index 00000000000..eab41017480 --- /dev/null +++ b/spec/migrations/save_instance_administrators_group_id_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200210092405_save_instance_administrators_group_id') + +describe SaveInstanceAdministratorsGroupId, :migration do + let(:application_settings_table) { table(:application_settings) } + + let(:instance_administrators_group) do + table(:namespaces).create!( + id: 1, + name: 'GitLab Instance Administrators', + path: 'gitlab-instance-administrators-random', + type: 'Group' + ) + end + + let(:self_monitoring_project) do + table(:projects).create!( + id: 2, + name: 'Self Monitoring', + path: 'self_monitoring', + namespace_id: instance_administrators_group.id + ) + end + + context 'when project ID is saved but group ID is not' do + let(:application_settings) do + application_settings_table.create!(instance_administration_project_id: self_monitoring_project.id) + end + + it 'saves instance administrators group ID' do + expect(application_settings.instance_administration_project_id).to eq(self_monitoring_project.id) + expect(application_settings.instance_administrators_group_id).to be_nil + + migrate! + + expect(application_settings.reload.instance_administrators_group_id).to eq(instance_administrators_group.id) + expect(application_settings.instance_administration_project_id).to eq(self_monitoring_project.id) + end + end + + context 'when group ID is saved but project ID is not' do + let(:application_settings) do + application_settings_table.create!(instance_administrators_group_id: instance_administrators_group.id) + end + + it 'does not make changes' do + expect(application_settings.instance_administrators_group_id).to eq(instance_administrators_group.id) + expect(application_settings.instance_administration_project_id).to be_nil + + migrate! + + expect(application_settings.reload.instance_administrators_group_id).to eq(instance_administrators_group.id) + expect(application_settings.instance_administration_project_id).to be_nil + end + end + + context 'when group ID and project ID are both saved' do + let(:application_settings) do + application_settings_table.create!( + instance_administrators_group_id: instance_administrators_group.id, + instance_administration_project_id: self_monitoring_project.id + ) + end + + it 'does not make changes' do + expect(application_settings.instance_administrators_group_id).to eq(instance_administrators_group.id) + expect(application_settings.instance_administration_project_id).to eq(self_monitoring_project.id) + + migrate! + + expect(application_settings.reload.instance_administrators_group_id).to eq(instance_administrators_group.id) + expect(application_settings.instance_administration_project_id).to eq(self_monitoring_project.id) + end + end + + context 'when neither group ID nor project ID is saved' do + let(:application_settings) do + application_settings_table.create! + end + + it 'does not make changes' do + expect(application_settings.instance_administrators_group_id).to be_nil + expect(application_settings.instance_administration_project_id).to be_nil + + migrate! + + expect(application_settings.reload.instance_administrators_group_id).to be_nil + expect(application_settings.instance_administration_project_id).to be_nil + end + end + + context 'when application_settings table has no rows' do + it 'does not fail' do + migrate! + end + end +end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 883f678b8f5..13a3d1cdd82 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -26,6 +26,42 @@ describe Mentionable do expect(mentionable.referenced_mentionables).to be_empty end end + + describe '#any_mentionable_attributes_changed?' do + Message = Struct.new(:text) + + let(:mentionable) { Example.new } + let(:changes) do + msg = Message.new('test') + + changes = {} + changes[msg] = ['', 'some message'] + changes[:random_sym_key] = ['', 'some message'] + changes["random_string_key"] = ['', 'some message'] + changes + end + + it 'returns true with key string' do + changes["message"] = ['', 'some message'] + + allow(mentionable).to receive(:saved_changes).and_return(changes) + + expect(mentionable.send(:any_mentionable_attributes_changed?)).to be true + end + + it 'returns false with key symbol' do + changes[:message] = ['', 'some message'] + allow(mentionable).to receive(:saved_changes).and_return(changes) + + expect(mentionable.send(:any_mentionable_attributes_changed?)).to be false + end + + it 'returns false when no attr_mentionable keys' do + allow(mentionable).to receive(:saved_changes).and_return(changes) + + expect(mentionable.send(:any_mentionable_attributes_changed?)).to be false + end + end end describe Issue, "Mentionable" do diff --git a/spec/models/cycle_analytics/group_level_spec.rb b/spec/models/cycle_analytics/group_level_spec.rb index 03fe8c3b50b..1f410a7c539 100644 --- a/spec/models/cycle_analytics/group_level_spec.rb +++ b/spec/models/cycle_analytics/group_level_spec.rb @@ -22,10 +22,6 @@ describe CycleAnalytics::GroupLevel do describe '#stats' do before do - allow_next_instance_of(Gitlab::ReferenceExtractor) do |instance| - allow(instance).to receive(:issues).and_return([issue]) - end - create_cycle(user, project, issue, mr, milestone, pipeline) deploy_master(user, project) end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index bdbe38afc56..89fb4eb3ff2 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -51,6 +51,22 @@ describe Deployment do end end + describe '.stoppable' do + subject { described_class.stoppable } + + context 'when deployment is stoppable' do + let!(:deployment) { create(:deployment, :success, on_stop: 'stop-review') } + + it { is_expected.to eq([deployment]) } + end + + context 'when deployment is not stoppable' do + let!(:deployment) { create(:deployment, :failed) } + + it { is_expected.to be_empty } + end + end + describe '.success' do subject { described_class.success } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index af7ab24d7d6..72143d69fc8 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -7,6 +7,7 @@ describe Environment, :use_clean_rails_memory_store_caching do using RSpec::Parameterized::TableSyntax include RepoHelpers include StubENV + include CreateEnvironmentsHelpers let(:project) { create(:project, :repository) } @@ -114,6 +115,72 @@ describe Environment, :use_clean_rails_memory_store_caching do end end + describe '.auto_stoppable' do + subject { described_class.auto_stoppable(limit) } + + let(:limit) { 100 } + + context 'when environment is auto-stoppable' do + let!(:environment) { create(:environment, :auto_stoppable) } + + it { is_expected.to eq([environment]) } + end + + context 'when environment is not auto-stoppable' do + let!(:environment) { create(:environment) } + + it { is_expected.to be_empty } + end + end + + describe '.stop_actions' do + subject { environments.stop_actions } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:environments) { Environment.all } + + before_all do + project.add_developer(user) + project.repository.add_branch(user, 'review/feature-1', 'master') + project.repository.add_branch(user, 'review/feature-2', 'master') + end + + shared_examples_for 'correct filtering' do + it 'returns stop actions for available environments only' do + expect(subject.count).to eq(1) + expect(subject.first.name).to eq('stop_review_app') + expect(subject.first.ref).to eq('review/feature-1') + end + end + + before do + create_review_app(user, project, 'review/feature-1') + create_review_app(user, project, 'review/feature-2') + end + + it 'returns stop actions for environments' do + expect(subject.count).to eq(2) + expect(subject).to match_array(Ci::Build.where(name: 'stop_review_app')) + end + + context 'when one of the stop actions has already been executed' do + before do + Ci::Build.where(ref: 'review/feature-2').find_by_name('stop_review_app').enqueue! + end + + it_behaves_like 'correct filtering' + end + + context 'when one of the deployments does not have stop action' do + before do + Deployment.where(ref: 'review/feature-2').update_all(on_stop: nil) + end + + it_behaves_like 'correct filtering' + end + end + describe '.pluck_names' do subject { described_class.pluck_names } @@ -449,7 +516,7 @@ describe Environment, :use_clean_rails_memory_store_caching do describe '#reset_auto_stop' do subject { environment.reset_auto_stop } - let(:environment) { create(:environment, :auto_stopped) } + let(:environment) { create(:environment, :auto_stoppable) } it 'nullifies the auto_stop_at' do expect { subject }.to change(environment, :auto_stop_at).from(Time).to(nil) diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 990141cf511..97ea32a120d 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -173,6 +173,7 @@ describe Event do end context 'commit note event' do + let(:project) { create(:project, :public, :repository) } let(:target) { note_on_commit } it do @@ -185,7 +186,7 @@ describe Event do end context 'private project' do - let(:project) { create(:project, :private) } + let(:project) { create(:project, :private, :repository) } it do aggregate_failures do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 08348e3767a..36fd5d21e73 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3576,4 +3576,44 @@ describe MergeRequest do expect(merge_request.recent_visible_deployments.count).to eq(10) end end + + describe '#diffable_merge_ref?' do + context 'diff_compare_with_head enabled' do + context 'merge request can be merged' do + context 'merge_to_ref is not calculated' do + it 'returns true' do + expect(subject.diffable_merge_ref?).to eq(false) + end + end + + context 'merge_to_ref is calculated' do + before do + MergeRequests::MergeToRefService.new(subject.project, subject.author).execute(subject) + end + + it 'returns true' do + expect(subject.diffable_merge_ref?).to eq(true) + end + end + end + + context 'merge request cannot be merged' do + it 'returns false' do + subject.mark_as_unchecked! + + expect(subject.diffable_merge_ref?).to eq(false) + end + end + end + + context 'diff_compare_with_head disabled' do + before do + stub_feature_flags(diff_compare_with_head: { enabled: false, thing: subject.target_project }) + end + + it 'returns false' do + expect(subject.diffable_merge_ref?).to eq(false) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 79d9e42666c..b3d8ac83075 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1299,8 +1299,8 @@ describe Project do describe '.trending' do let(:group) { create(:group, :public) } - let(:project1) { create(:project, :public, group: group) } - let(:project2) { create(:project, :public, group: group) } + let(:project1) { create(:project, :public, :repository, group: group) } + let(:project2) { create(:project, :public, :repository, group: group) } before do create_list(:note_on_commit, 2, project: project1) diff --git a/spec/models/trending_project_spec.rb b/spec/models/trending_project_spec.rb index 4a248b71574..39f5d686eb4 100644 --- a/spec/models/trending_project_spec.rb +++ b/spec/models/trending_project_spec.rb @@ -4,11 +4,11 @@ require 'spec_helper' describe TrendingProject do let(:user) { create(:user) } - let(:public_project1) { create(:project, :public) } - let(:public_project2) { create(:project, :public) } - let(:public_project3) { create(:project, :public) } - let(:private_project) { create(:project, :private) } - let(:internal_project) { create(:project, :internal) } + let(:public_project1) { create(:project, :public, :repository) } + let(:public_project2) { create(:project, :public, :repository) } + let(:public_project3) { create(:project, :public, :repository) } + let(:private_project) { create(:project, :private, :repository) } + let(:internal_project) { create(:project, :internal, :repository) } before do create_list(:note_on_commit, 3, project: public_project1) diff --git a/spec/serializers/merge_request_diff_entity_spec.rb b/spec/serializers/merge_request_diff_entity_spec.rb index 59ec0b22158..2e3b0d092fe 100644 --- a/spec/serializers/merge_request_diff_entity_spec.rb +++ b/spec/serializers/merge_request_diff_entity_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequestDiffEntity do - let(:project) { create(:project, :repository) } + let_it_be(:project) { create(:project, :repository) } let(:request) { EntityRequest.new(project: project) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request_diffs) { merge_request.merge_request_diffs } @@ -36,4 +36,29 @@ describe MergeRequestDiffEntity do expect(subject[:short_commit_sha]).to eq(nil) end end + + describe '#head_version_path' do + before do + allow(merge_request).to receive(:diffable_merge_ref?) + .and_return(diffable_merge_ref) + end + + context 'merge request can be merged' do + let(:diffable_merge_ref) { true } + + it 'returns diff path with diff_head param set' do + expect(subject[:head_version_path]).to eq( + "/#{project.full_path}/-/merge_requests/#{merge_request.iid}/diffs?diff_head=true" + ) + end + end + + context 'merge request cannot be merged' do + let(:diffable_merge_ref) { false } + + it 'returns diff path with diff_head param set' do + expect(subject[:head_version_path]).to be_nil + end + end + end end diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb index ed92625a2cc..88f2f5618c2 100644 --- a/spec/services/ci/stop_environments_service_spec.rb +++ b/spec/services/ci/stop_environments_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Ci::StopEnvironmentsService do + include CreateEnvironmentsHelpers + let(:project) { create(:project, :private, :repository) } let(:user) { create(:user) } @@ -181,6 +183,55 @@ describe Ci::StopEnvironmentsService do end end + describe '.execute_in_batch' do + subject { described_class.execute_in_batch(environments) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:environments) { Environment.available } + + before_all do + project.add_developer(user) + project.repository.add_branch(user, 'review/feature-1', 'master') + project.repository.add_branch(user, 'review/feature-2', 'master') + end + + before do + create_review_app(user, project, 'review/feature-1') + create_review_app(user, project, 'review/feature-2') + end + + it 'stops environments' do + expect { subject } + .to change { project.environments.all.map(&:state).uniq } + .from(['available']).to(['stopped']) + + expect(project.environments.all.map(&:auto_stop_at).uniq).to eq([nil]) + end + + it 'plays stop actions' do + expect { subject } + .to change { Ci::Build.where(name: 'stop_review_app').map(&:status).uniq } + .from(['manual']).to(['pending']) + end + + context 'when user does not have a permission to play the stop action' do + before do + Ci::Build.find_by_ref('review/feature-2').update_column(:user_id, nil) + end + + it 'tracks the exception' do + deployable = Ci::Build.find_by_ref('review/feature-2') + + expect(Gitlab::ErrorTracking) + .to receive(:track_error) + .with(Gitlab::Access::AccessDeniedError, deployable_id: deployable.id).once + + subject + end + end + end + def expect_environment_stopped_on(branch) expect_any_instance_of(Environment) .to receive(:stop!) diff --git a/spec/services/environments/auto_stop_service_spec.rb b/spec/services/environments/auto_stop_service_spec.rb new file mode 100644 index 00000000000..3620bf8fe87 --- /dev/null +++ b/spec/services/environments/auto_stop_service_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Environments::AutoStopService, :clean_gitlab_redis_shared_state do + include CreateEnvironmentsHelpers + include ExclusiveLeaseHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:service) { described_class.new } + + before_all do + project.add_developer(user) + end + + describe '#execute' do + subject { service.execute } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:environments) { Environment.all } + + before_all do + project.add_developer(user) + project.repository.add_branch(user, 'review/feature-1', 'master') + project.repository.add_branch(user, 'review/feature-2', 'master') + end + + before do + create_review_app(user, project, 'review/feature-1') + create_review_app(user, project, 'review/feature-2') + end + + it 'stops environments and play stop jobs' do + expect { subject } + .to change { Environment.all.map(&:state).uniq } + .from(['available']).to(['stopped']) + + expect(Ci::Build.where(name: 'stop_review_app').map(&:status).uniq).to eq(['pending']) + end + + context 'when auto_stop_environments feature flag is disabled' do + before do + stub_feature_flags(auto_stop_environments: false) + end + + it 'does not execute Ci::StopEnvironmentsService' do + expect(Ci::StopEnvironmentsService).not_to receive(:execute_in_batch) + + subject + end + end + + context 'when the other sidekiq worker has already been running' do + before do + stub_exclusive_lease_taken(described_class::EXCLUSIVE_LOCK_KEY) + end + + it 'does not execute stop_in_batch' do + expect_next_instance_of(described_class) do |service| + expect(service).not_to receive(:stop_in_batch) + end + + expect { subject }.to raise_error(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + end + end + + context 'when loop reached timeout' do + before do + stub_const("#{described_class}::LOOP_TIMEOUT", 0.seconds) + stub_const("#{described_class}::LOOP_LIMIT", 100_000) + allow_next_instance_of(described_class) do |service| + allow(service).to receive(:stop_in_batch) { true } + end + end + + it 'returns false and does not continue the process' do + is_expected.to eq(false) + end + end + + context 'when loop reached loop limit' do + before do + stub_const("#{described_class}::LOOP_LIMIT", 1) + stub_const("#{described_class}::BATCH_SIZE", 1) + end + + it 'stops only one available environment' do + expect { subject }.to change { Environment.available.count }.by(-1) + end + end + end +end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 44d4cd70f9a..c9701e5d194 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -171,6 +171,31 @@ describe Issues::CreateService do described_class.new(project, user, opts).execute end + + context 'after_save callback to store_mentions' do + context 'when mentionable attributes change' do + let(:opts) { { title: 'Title', description: "Description with #{user.to_reference}" } } + + it 'saves mentions' do + expect_next_instance_of(Issue) do |instance| + expect(instance).to receive(:store_mentions!).and_call_original + end + expect(issue.user_mentions.count).to eq 1 + end + end + + context 'when save fails' do + let(:opts) { { title: '', label_ids: labels.map(&:id), milestone_id: milestone.id } } + + it 'does not call store_mentions' do + expect_next_instance_of(Issue) do |instance| + expect(instance).not_to receive(:store_mentions!).and_call_original + end + expect(issue.valid?).to be false + expect(issue.user_mentions.count).to eq 0 + end + end + end end context 'issue create service' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index ee809aabac0..ccd4dd4231b 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -6,7 +6,7 @@ describe Issues::MoveService do let(:user) { create(:user) } let(:author) { create(:user) } let(:title) { 'Some issue' } - let(:description) { 'Some issue description' } + let(:description) { "Some issue description with mention to #{user.to_reference}" } let(:group) { create(:group, :private) } let(:sub_group_1) { create(:group, :private, parent: group) } let(:sub_group_2) { create(:group, :private, parent: group) } @@ -36,6 +36,9 @@ describe Issues::MoveService do end context 'issue movable' do + let!(:note_with_mention) { create(:note, noteable: old_issue, author: author, project: old_project, note: "note with mention #{user.to_reference}") } + let!(:note_with_no_mention) { create(:note, noteable: old_issue, author: author, project: old_project, note: "note without mention") } + include_context 'user can move issue' context 'generic issue' do @@ -94,6 +97,15 @@ describe Issues::MoveService do it 'moves the award emoji' do expect(old_issue.award_emoji.first.name).to eq new_issue.reload.award_emoji.first.name end + + context 'when issue has notes with mentions' do + it 'saves user mentions with actual mentions for new issue' do + expect(new_issue.user_mentions.where(note_id: nil).first.mentioned_users_ids).to match_array([user.id]) + expect(new_issue.user_mentions.where.not(note_id: nil).first.mentioned_users_ids).to match_array([user.id]) + expect(new_issue.user_mentions.where.not(note_id: nil).count).to eq 1 + expect(new_issue.user_mentions.count).to eq 2 + end + end end context 'issue with assignee' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 64bca770d5b..888a63980f6 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -211,6 +211,49 @@ describe Issues::UpdateService, :mailer do expect(note.note).to eq 'locked this issue' end end + + context 'after_save callback to store_mentions' do + let(:issue) { create(:issue, title: 'Old title', description: "simple description", project: project, author: create(:user)) } + let(:labels) { create_pair(:label, project: project) } + let(:milestone) { create(:milestone, project: project) } + + context 'when mentionable attributes change' do + let(:opts) { { description: "Description with #{user.to_reference}" } } + + it 'saves mentions' do + expect(issue).to receive(:store_mentions!).and_call_original + + expect { update_issue(opts) }.to change { IssueUserMention.count }.by(1) + + expect(issue.referenced_users).to match_array([user]) + end + end + + context 'when mentionable attributes do not change' do + let(:opts) { { label_ids: labels.map(&:id), milestone_id: milestone.id } } + + it 'does not call store_mentions' do + expect(issue).not_to receive(:store_mentions!).and_call_original + + expect { update_issue(opts) }.not_to change { IssueUserMention.count } + + expect(issue.referenced_users).to be_empty + end + end + + context 'when save fails' do + let(:opts) { { title: '', label_ids: labels.map(&:id), milestone_id: milestone.id } } + + it 'does not call store_mentions' do + expect(issue).not_to receive(:store_mentions!).and_call_original + + expect { update_issue(opts) }.not_to change { IssueUserMention.count } + + expect(issue.referenced_users).to be_empty + expect(issue.valid?).to be false + end + end + end end context 'when description changed' do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 8490127058c..aebead481ce 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -291,6 +291,46 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do expect { service.execute }.to change { counter.read(:create) }.by(1) end + + context 'after_save callback to store_mentions' do + let(:labels) { create_pair(:label, project: project) } + let(:milestone) { create(:milestone, project: project) } + let(:req_opts) { { source_branch: 'feature', target_branch: 'master' } } + + context 'when mentionable attributes change' do + let(:opts) { { title: 'Title', description: "Description with #{user.to_reference}" }.merge(req_opts) } + + it 'saves mentions' do + expect_next_instance_of(MergeRequest) do |instance| + expect(instance).to receive(:store_mentions!).and_call_original + end + expect(merge_request.user_mentions.count).to eq 1 + end + end + + context 'when mentionable attributes do not change' do + let(:opts) { { label_ids: labels.map(&:id), milestone_id: milestone.id }.merge(req_opts) } + + it 'does not call store_mentions' do + expect_next_instance_of(MergeRequest) do |instance| + expect(instance).not_to receive(:store_mentions!).and_call_original + end + expect(merge_request.valid?).to be false + expect(merge_request.user_mentions.count).to eq 0 + end + end + + context 'when save fails' do + let(:opts) { { label_ids: labels.map(&:id), milestone_id: milestone.id } } + + it 'does not call store_mentions' do + expect_next_instance_of(MergeRequest) do |instance| + expect(instance).not_to receive(:store_mentions!).and_call_original + end + expect(merge_request.valid?).to be false + end + end + end end it_behaves_like 'new issuable record that supports quick actions' do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index baa0ecf27e3..f295f3c4a81 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -162,6 +162,52 @@ describe MergeRequests::UpdateService, :mailer do end end + context 'after_save callback to store_mentions' do + let(:merge_request) { create(:merge_request, title: 'Old title', description: "simple description", source_branch: 'test', source_project: project, author: user) } + let(:labels) { create_pair(:label, project: project) } + let(:milestone) { create(:milestone, project: project) } + let(:req_opts) { { source_branch: 'feature', target_branch: 'master' } } + + subject { MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) } + + context 'when mentionable attributes change' do + let(:opts) { { description: "Description with #{user.to_reference}" }.merge(req_opts) } + + it 'saves mentions' do + expect(merge_request).to receive(:store_mentions!).and_call_original + + expect { subject }.to change { MergeRequestUserMention.count }.by(1) + + expect(merge_request.referenced_users).to match_array([user]) + end + end + + context 'when mentionable attributes do not change' do + let(:opts) { { label_ids: [label.id, label2.id], milestone_id: milestone.id }.merge(req_opts) } + + it 'does not call store_mentions' do + expect(merge_request).not_to receive(:store_mentions!).and_call_original + + expect { subject }.not_to change { MergeRequestUserMention.count } + + expect(merge_request.referenced_users).to be_empty + end + end + + context 'when save fails' do + let(:opts) { { title: '', label_ids: labels.map(&:id), milestone_id: milestone.id } } + + it 'does not call store_mentions' do + expect(merge_request).not_to receive(:store_mentions!).and_call_original + + expect { subject }.not_to change { MergeRequestUserMention.count } + + expect(merge_request.referenced_users).to be_empty + expect(merge_request.valid?).to be false + end + end + end + context 'merge' do let(:opts) do { diff --git a/spec/support/helpers/create_environments_helpers.rb b/spec/support/helpers/create_environments_helpers.rb new file mode 100644 index 00000000000..be105f5862b --- /dev/null +++ b/spec/support/helpers/create_environments_helpers.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module CreateEnvironmentsHelpers + def create_review_app(user, project, ref) + common = { project: project, ref: ref, user: user } + pipeline = create(:ci_pipeline, **common) + start_review = create(:ci_build, :start_review_app, :success, **common, pipeline: pipeline) + stop_review = create(:ci_build, :stop_review_app, :manual, **common, pipeline: pipeline) + environment = create(:environment, :auto_stoppable, project: project, name: ref) + create(:deployment, :success, **common, on_stop: stop_review.name, + deployable: start_review, environment: environment) + end +end diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index 29c7e65fc21..b6b131338cc 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -86,7 +86,7 @@ RSpec.shared_examples 'a mentionable' do end it 'sends in cached markdown fields when appropriate' do - if subject.is_a?(CacheMarkdownField) + if subject.is_a?(CacheMarkdownField) && subject.extractors[author].blank? expect_next_instance_of(Gitlab::ReferenceExtractor) do |ext| attrs = subject.class.mentionable_attrs.collect(&:first) & subject.cached_markdown_fields.markdown_fields attrs.each do |field| @@ -136,7 +136,7 @@ RSpec.shared_examples 'an editable mentionable' do set_mentionable_text.call('This is a text') - if subject.is_a?(CacheMarkdownField) + if subject.is_a?(CacheMarkdownField) && subject.extractors[author].blank? expect_next_instance_of(Gitlab::ReferenceExtractor) do |ext| subject.cached_markdown_fields.markdown_fields.each do |field| expect(ext).to receive(:analyze).with(subject.send(field), hash_including(rendered: anything)) diff --git a/spec/workers/environments/auto_stop_cron_worker_spec.rb b/spec/workers/environments/auto_stop_cron_worker_spec.rb new file mode 100644 index 00000000000..6773637d4a7 --- /dev/null +++ b/spec/workers/environments/auto_stop_cron_worker_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Environments::AutoStopCronWorker do + subject { worker.perform } + + let(:worker) { described_class.new } + + it 'executes Environments::AutoStopService' do + expect_next_instance_of(Environments::AutoStopService) do |service| + expect(service).to receive(:execute) + end + + subject + end +end |