summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-04-07 15:09:30 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-04-07 15:09:30 +0000
commitc6b3ec3f56fa32a0e0ed3de0d0878d25f1adaddf (patch)
tree967afee9a510ff9dd503ebd83706dc760ec2e3ed
parent903ccf7c93eb9490c76857bffe744249cc07de09 (diff)
downloadgitlab-ce-c6b3ec3f56fa32a0e0ed3de0d0878d25f1adaddf.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/controllers/projects/alerting/notifications_controller.rb8
-rw-r--r--app/controllers/projects/prometheus/alerts_controller.rb7
-rw-r--r--app/helpers/submodule_helper.rb2
-rw-r--r--app/models/concerns/has_repository.rb17
-rw-r--r--app/models/concerns/update_highest_role.rb38
-rw-r--r--app/models/concerns/versioned_description.rb1
-rw-r--r--app/models/member.rb20
-rw-r--r--app/models/project_wiki.rb6
-rw-r--r--app/models/snippet.rb8
-rw-r--r--app/models/user.rb21
-rw-r--r--app/serializers/merge_request_widget_entity.rb2
-rw-r--r--app/services/members/update_highest_role_service.rb39
-rw-r--r--app/services/projects/alerting/notify_service.rb6
-rw-r--r--app/services/projects/import_export/export_service.rb7
-rw-r--r--app/services/projects/prometheus/alerts/notify_service.rb20
-rw-r--r--app/views/projects/mirrors/_authentication_method.html.haml2
-rw-r--r--app/views/projects/mirrors/_ssh_host_keys.html.haml2
-rw-r--r--app/views/shared/_personal_access_tokens_form.html.haml6
-rw-r--r--changelogs/unreleased/197312_store_lines_added_and_removed_in_mr_metrics.yml5
-rw-r--r--changelogs/unreleased/211816-remove-streaming-serializer-feature-flag.yml5
-rw-r--r--changelogs/unreleased/Resolve-Migrate--fa-spinner-app-views-projects-mirrors.yml5
-rw-r--r--changelogs/unreleased/introduce_update_highest_role_concern.yml5
-rw-r--r--db/migrate/20200402123926_add_line_metrics_to_mr_metrics.rb21
-rw-r--r--db/structure.sql5
-rw-r--r--doc/administration/high_availability/object_storage.md31
-rw-r--r--doc/administration/job_artifacts.md2
-rw-r--r--doc/administration/object_storage.md140
-rw-r--r--doc/development/documentation/styleguide.md4
-rw-r--r--doc/user/project/settings/import_export.md2
-rw-r--r--lib/gitlab/import_export/project/legacy_tree_saver.rb68
-rw-r--r--lib/gitlab/repository_url_builder.rb33
-rw-r--r--lib/gitlab/shell.rb8
-rw-r--r--qa/qa/page/profile/personal_access_tokens.rb10
-rw-r--r--qa/qa/resource/personal_access_token.rb4
-rw-r--r--spec/controllers/projects/alerting/notifications_controller_spec.rb2
-rw-r--r--spec/controllers/projects/prometheus/alerts_controller_spec.rb9
-rw-r--r--spec/features/projects/import_export/export_file_spec.rb25
-rw-r--r--spec/lib/gitlab/import_export/project/legacy_tree_saver_spec.rb352
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml2
-rw-r--r--spec/lib/gitlab/repository_url_builder_spec.rb56
-rw-r--r--spec/lib/gitlab/shell_spec.rb8
-rw-r--r--spec/models/member_spec.rb34
-rw-r--r--spec/models/project_wiki_spec.rb25
-rw-r--r--spec/models/snippet_spec.rb16
-rw-r--r--spec/models/user_spec.rb34
-rw-r--r--spec/serializers/merge_request_widget_entity_spec.rb22
-rw-r--r--spec/services/members/update_highest_role_service_spec.rb44
-rw-r--r--spec/services/projects/alerting/notify_service_spec.rb14
-rw-r--r--spec/services/projects/import_export/export_service_spec.rb24
-rw-r--r--spec/services/projects/prometheus/alerts/notify_service_spec.rb31
-rw-r--r--spec/support/shared_examples/models/update_highest_role_shared_examples.rb52
-rw-r--r--spec/support/shared_examples/models/versioned_description_shared_examples.rb52
52 files changed, 551 insertions, 811 deletions
diff --git a/app/controllers/projects/alerting/notifications_controller.rb b/app/controllers/projects/alerting/notifications_controller.rb
index 1fe31863469..358e7629958 100644
--- a/app/controllers/projects/alerting/notifications_controller.rb
+++ b/app/controllers/projects/alerting/notifications_controller.rb
@@ -14,7 +14,7 @@ module Projects
token = extract_alert_manager_token(request)
result = notify_service.execute(token)
- head(response_status(result))
+ head result.http_status
end
private
@@ -33,12 +33,6 @@ module Projects
.new(project, current_user, notification_payload)
end
- def response_status(result)
- return :ok if result.success?
-
- result.http_status
- end
-
def notification_payload
params.permit![:notification]
end
diff --git a/app/controllers/projects/prometheus/alerts_controller.rb b/app/controllers/projects/prometheus/alerts_controller.rb
index 8c74c730de9..2c0521edece 100644
--- a/app/controllers/projects/prometheus/alerts_controller.rb
+++ b/app/controllers/projects/prometheus/alerts_controller.rb
@@ -26,12 +26,9 @@ module Projects
def notify
token = extract_alert_manager_token(request)
+ result = notify_service.execute(token)
- if notify_service.execute(token)
- head :ok
- else
- head :unprocessable_entity
- end
+ head result.http_status
end
def create
diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb
index e9554300075..06ea3dd8a81 100644
--- a/app/helpers/submodule_helper.rb
+++ b/app/helpers/submodule_helper.rb
@@ -72,7 +72,7 @@ module SubmoduleHelper
project].join('')
url_with_dotgit = url_no_dotgit + '.git'
- url_with_dotgit == Gitlab::Shell.url_to_repo([namespace, '/', project].join(''))
+ url_with_dotgit == Gitlab::RepositoryUrlBuilder.build([namespace, '/', project].join(''))
end
def relative_self_url?(url)
diff --git a/app/models/concerns/has_repository.rb b/app/models/concerns/has_repository.rb
index 6a09741b903..af7afd6604a 100644
--- a/app/models/concerns/has_repository.rb
+++ b/app/models/concerns/has_repository.rb
@@ -87,26 +87,15 @@ module HasRepository
end
def url_to_repo
- Gitlab::Shell.url_to_repo(full_path)
+ ssh_url_to_repo
end
def ssh_url_to_repo
- url_to_repo
+ Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :ssh)
end
def http_url_to_repo
- custom_root = Gitlab::CurrentSettings.custom_http_clone_url_root
-
- url = if custom_root.present?
- Gitlab::Utils.append_path(
- custom_root,
- web_url(only_path: true)
- )
- else
- web_url
- end
-
- "#{url}.git"
+ Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :http)
end
def web_url(only_path: nil)
diff --git a/app/models/concerns/update_highest_role.rb b/app/models/concerns/update_highest_role.rb
new file mode 100644
index 00000000000..7efc436c6c8
--- /dev/null
+++ b/app/models/concerns/update_highest_role.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module UpdateHighestRole
+ extend ActiveSupport::Concern
+
+ HIGHEST_ROLE_LEASE_TIMEOUT = 10.minutes.to_i
+ HIGHEST_ROLE_JOB_DELAY = 10.minutes
+
+ included do
+ after_commit :update_highest_role
+ end
+
+ private
+
+ # Schedule a Sidekiq job to update the highest role for a User
+ #
+ # The job will be called outside of a transaction in order to ensure the changes
+ # to be commited before attempting to update the highest role.
+ # The exlusive lease will not be released after completion to prevent multiple jobs
+ # being executed during the defined timeout.
+ def update_highest_role
+ return unless update_highest_role?
+
+ run_after_commit_or_now do
+ lease_key = "update_highest_role:#{update_highest_role_attribute}"
+ lease = Gitlab::ExclusiveLease.new(lease_key, timeout: HIGHEST_ROLE_LEASE_TIMEOUT)
+
+ if lease.try_obtain
+ UpdateHighestRoleWorker.perform_in(HIGHEST_ROLE_JOB_DELAY, update_highest_role_attribute)
+ else
+ # use same logging as ExclusiveLeaseGuard
+ # rubocop:disable Gitlab/RailsLogger
+ Rails.logger.error('Cannot obtain an exclusive lease. There must be another instance already in execution.')
+ # rubocop:enable Gitlab/RailsLogger
+ end
+ end
+ end
+end
diff --git a/app/models/concerns/versioned_description.rb b/app/models/concerns/versioned_description.rb
index ee8d2d45357..dd602efea1c 100644
--- a/app/models/concerns/versioned_description.rb
+++ b/app/models/concerns/versioned_description.rb
@@ -16,7 +16,6 @@ module VersionedDescription
def save_description_version
self.saved_description_version = nil
- return unless Feature.enabled?(:save_description_versions, issuing_parent, default_enabled: true)
return unless saved_change_to_description?
unless description_versions.exists?
diff --git a/app/models/member.rb b/app/models/member.rb
index d7c515b650b..5b33333aa23 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -9,6 +9,7 @@ class Member < ApplicationRecord
include Presentable
include Gitlab::Utils::StrongMemoize
include FromUnion
+ include UpdateHighestRole
attr_accessor :raw_invite_token
@@ -100,7 +101,6 @@ class Member < ApplicationRecord
after_destroy :destroy_notification_setting
after_destroy :post_destroy_hook, unless: :pending?
after_commit :refresh_member_authorized_projects
- after_commit :update_highest_role
default_value_for :notification_level, NotificationSetting.levels[:global]
@@ -463,21 +463,15 @@ class Member < ApplicationRecord
end
end
- # Triggers the service to schedule a Sidekiq job to update the highest role
- # for a User
- #
- # The job will be called outside of a transaction in order to ensure the changes
- # for a Member to be commited before attempting to update the highest role.
- # rubocop: disable CodeReuse/ServiceClass
- def update_highest_role
+ def update_highest_role?
return unless user_id.present?
- return unless previous_changes[:access_level].present?
- run_after_commit_or_now do
- Members::UpdateHighestRoleService.new(user_id).execute
- end
+ previous_changes[:access_level].present?
+ end
+
+ def update_highest_role_attribute
+ user_id
end
- # rubocop: enable CodeReuse/ServiceClass
end
Member.prepend_if_ee('EE::Member')
diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb
index 4b888648b9e..708b45cf5f0 100644
--- a/app/models/project_wiki.rb
+++ b/app/models/project_wiki.rb
@@ -49,15 +49,15 @@ class ProjectWiki
end
def url_to_repo
- Gitlab::Shell.url_to_repo(full_path)
+ ssh_url_to_repo
end
def ssh_url_to_repo
- url_to_repo
+ Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :ssh)
end
def http_url_to_repo
- @project.http_url_to_repo.sub(%r{git\z}, 'wiki.git')
+ Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :http)
end
def wiki_base_path
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index cfe1c77ec48..821de70f9d9 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -258,10 +258,12 @@ class Snippet < ApplicationRecord
super
end
+ override :repository
def repository
@repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path, repo_type: Gitlab::GlRepository::SNIPPET)
end
+ override :repository_size_checker
def repository_size_checker
strong_memoize(:repository_size_checker) do
::Gitlab::RepositorySizeChecker.new(
@@ -271,6 +273,7 @@ class Snippet < ApplicationRecord
end
end
+ override :storage
def storage
@storage ||= Storage::Hashed.new(self, prefix: Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX)
end
@@ -278,6 +281,7 @@ class Snippet < ApplicationRecord
# This is the full_path used to identify the
# the snippet repository. It will be used mostly
# for logging purposes.
+ override :full_path
def full_path
return unless persisted?
@@ -290,10 +294,6 @@ class Snippet < ApplicationRecord
end
end
- def url_to_repo
- Gitlab::Shell.url_to_repo(full_path.delete('@'))
- end
-
def repository_storage
snippet_repository&.shard_name || self.class.pick_repository_storage
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 68c52751804..42972477d97 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -23,6 +23,7 @@ class User < ApplicationRecord
include BatchDestroyDependentAssociations
include HasUniqueInternalUsers
include IgnorableColumns
+ include UpdateHighestRole
DEFAULT_NOTIFICATION_LEVEL = :participating
@@ -238,7 +239,6 @@ class User < ApplicationRecord
end
end
end
- after_commit :update_highest_role, on: [:create, :update]
after_initialize :set_projects_limit
@@ -1854,20 +1854,15 @@ class User < ApplicationRecord
last_active_at.to_i <= MINIMUM_INACTIVE_DAYS.days.ago.to_i
end
- # Triggers the service to schedule a Sidekiq job to update the highest role
- # for a User
- #
- # The job will be called outside of a transaction in order to ensure the changes
- # for a Member to be commited before attempting to update the highest role.
- # rubocop: disable CodeReuse/ServiceClass
- def update_highest_role
- return unless (previous_changes.keys & %w(state user_type ghost)).any?
+ def update_highest_role?
+ return false unless persisted?
- run_after_commit_or_now do
- Members::UpdateHighestRoleService.new(id).execute
- end
+ (previous_changes.keys & %w(state user_type ghost)).any?
+ end
+
+ def update_highest_role_attribute
+ id
end
- # rubocop: enable CodeReuse/ServiceClass
end
User.prepend_if_ee('EE::User')
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index 7ba15dd9acf..74f29b36209 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -96,7 +96,7 @@ class MergeRequestWidgetEntity < Grape::Entity
def can_add_ci_config_path?(merge_request)
merge_request.source_project&.uses_default_ci_config? &&
- merge_request.all_pipelines.none? &&
+ !merge_request.source_project.has_ci? &&
merge_request.commits_count.positive? &&
can?(current_user, :read_build, merge_request.source_project) &&
can?(current_user, :create_pipeline, merge_request.source_project)
diff --git a/app/services/members/update_highest_role_service.rb b/app/services/members/update_highest_role_service.rb
deleted file mode 100644
index 5ebd2e03df1..00000000000
--- a/app/services/members/update_highest_role_service.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-# frozen_string_literal: true
-
-module Members
- class UpdateHighestRoleService < ::BaseService
- include ExclusiveLeaseGuard
-
- LEASE_TIMEOUT = 10.minutes.to_i
- DELAY = 10.minutes
-
- attr_reader :user_id
-
- def initialize(user_id)
- @user_id = user_id
- end
-
- def execute
- try_obtain_lease do
- UpdateHighestRoleWorker.perform_in(DELAY, user_id)
- end
- end
-
- private
-
- def lease_key
- "update_highest_role:#{user_id}"
- end
-
- def lease_timeout
- LEASE_TIMEOUT
- end
-
- # Do not release the lease before the timeout to
- # prevent multiple jobs being executed during the
- # defined timeout
- def lease_release?
- false
- end
- end
-end
diff --git a/app/services/projects/alerting/notify_service.rb b/app/services/projects/alerting/notify_service.rb
index d34d6f6a915..1ce1ef7a1cd 100644
--- a/app/services/projects/alerting/notify_service.rb
+++ b/app/services/projects/alerting/notify_service.rb
@@ -46,15 +46,15 @@ module Projects
end
def bad_request
- ServiceResponse.error(message: 'Bad Request', http_status: 400)
+ ServiceResponse.error(message: 'Bad Request', http_status: :bad_request)
end
def unauthorized
- ServiceResponse.error(message: 'Unauthorized', http_status: 401)
+ ServiceResponse.error(message: 'Unauthorized', http_status: :unauthorized)
end
def forbidden
- ServiceResponse.error(message: 'Forbidden', http_status: 403)
+ ServiceResponse.error(message: 'Forbidden', http_status: :forbidden)
end
end
end
diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb
index fa098e7417c..8893bf18e1f 100644
--- a/app/services/projects/import_export/export_service.rb
+++ b/app/services/projects/import_export/export_service.rb
@@ -58,12 +58,7 @@ module Projects
end
def tree_saver_class
- if ::Feature.enabled?(:streaming_serializer, project, default_enabled: true)
- Gitlab::ImportExport::Project::TreeSaver
- else
- # Once we remove :streaming_serializer feature flag, Project::LegacyTreeSaver should be removed as well
- Gitlab::ImportExport::Project::LegacyTreeSaver
- end
+ Gitlab::ImportExport::Project::TreeSaver
end
def uploads_saver
diff --git a/app/services/projects/prometheus/alerts/notify_service.rb b/app/services/projects/prometheus/alerts/notify_service.rb
index 89791aecabb..6ebc061c2e3 100644
--- a/app/services/projects/prometheus/alerts/notify_service.rb
+++ b/app/services/projects/prometheus/alerts/notify_service.rb
@@ -8,15 +8,15 @@ module Projects
include IncidentManagement::Settings
def execute(token)
- return false unless valid_payload_size?
- return false unless valid_version?
- return false unless valid_alert_manager_token?(token)
+ return bad_request unless valid_payload_size?
+ return unprocessable_entity unless valid_version?
+ return unauthorized unless valid_alert_manager_token?(token)
persist_events
send_alert_email if send_email?
process_incident_issues if process_issues?
- true
+ ServiceResponse.success
end
private
@@ -118,6 +118,18 @@ module Projects
def persist_events
CreateEventsService.new(project, nil, params).execute
end
+
+ def bad_request
+ ServiceResponse.error(message: 'Bad Request', http_status: :bad_request)
+ end
+
+ def unauthorized
+ ServiceResponse.error(message: 'Unauthorized', http_status: :unauthorized)
+ end
+
+ def unprocessable_entity
+ ServiceResponse.error(message: 'Unprocessable Entity', http_status: :unprocessable_entity)
+ end
end
end
end
diff --git a/app/views/projects/mirrors/_authentication_method.html.haml b/app/views/projects/mirrors/_authentication_method.html.haml
index a6978cba495..c4f564e26f4 100644
--- a/app/views/projects/mirrors/_authentication_method.html.haml
+++ b/app/views/projects/mirrors/_authentication_method.html.haml
@@ -12,7 +12,7 @@
.form-group
.collapse.js-well-changing-auth
- .changing-auth-method= icon('spinner spin lg')
+ .changing-auth-method
.well-password-auth.collapse.js-well-password-auth
= f.label :password, _("Password"), class: "label-bold"
= f.password_field :password, value: mirror.password, class: 'form-control qa-password', autocomplete: 'new-password'
diff --git a/app/views/projects/mirrors/_ssh_host_keys.html.haml b/app/views/projects/mirrors/_ssh_host_keys.html.haml
index 3279d3eb251..90236dc0c48 100644
--- a/app/views/projects/mirrors/_ssh_host_keys.html.haml
+++ b/app/views/projects/mirrors/_ssh_host_keys.html.haml
@@ -4,7 +4,7 @@
.form-group.js-ssh-host-keys-section{ class: ('collapse' unless mirror.ssh_mirror_url?) }
%button.btn.btn-inverted.btn-secondary.inline.js-detect-host-keys.append-right-10{ type: 'button', data: { qa_selector: 'detect_host_keys' } }
- = icon('spinner spin', class: 'js-spinner d-none')
+ .js-spinner.d-none.spinner.mr-1
= _('Detect host keys')
.fingerprint-ssh-info.js-fingerprint-ssh-info.prepend-top-10.append-bottom-10{ class: ('collapse' unless mirror.ssh_mirror_url?) }
%label.label-bold
diff --git a/app/views/shared/_personal_access_tokens_form.html.haml b/app/views/shared/_personal_access_tokens_form.html.haml
index 16f8a692635..71f3447ebc7 100644
--- a/app/views/shared/_personal_access_tokens_form.html.haml
+++ b/app/views/shared/_personal_access_tokens_form.html.haml
@@ -12,7 +12,7 @@
.row
.form-group.col-md-6
= f.label :name, _('Name'), class: 'label-bold'
- = f.text_field :name, class: "form-control qa-personal-access-token-name-field", required: true
+ = f.text_field :name, class: "form-control", required: true, data: { qa_selector: 'personal_access_token_name_field' }
.row
.form-group.col-md-6
@@ -21,11 +21,11 @@
= render_if_exists 'personal_access_tokens/callout_max_personal_access_token_lifetime'
- = f.text_field :expires_at, class: "datepicker form-control", placeholder: 'YYYY-MM-DD'
+ = f.text_field :expires_at, class: "datepicker form-control", placeholder: 'YYYY-MM-DD', data: { qa_selector: 'expiry_date_field' }
.form-group
= f.label :scopes, _('Scopes'), class: 'label-bold'
= render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: token, scopes: scopes
.prepend-top-default
- = f.submit _('Create %{type} token') % { type: type }, class: "btn btn-success qa-create-token-button"
+ = f.submit _('Create %{type} token') % { type: type }, class: "btn btn-success", data: { qa_selector: 'create_token_button' }
diff --git a/changelogs/unreleased/197312_store_lines_added_and_removed_in_mr_metrics.yml b/changelogs/unreleased/197312_store_lines_added_and_removed_in_mr_metrics.yml
new file mode 100644
index 00000000000..83be9f8973f
--- /dev/null
+++ b/changelogs/unreleased/197312_store_lines_added_and_removed_in_mr_metrics.yml
@@ -0,0 +1,5 @@
+---
+title: Add added_lines and removed_lines columns to merge_request_metrics table
+merge_request: 28658
+author:
+type: added
diff --git a/changelogs/unreleased/211816-remove-streaming-serializer-feature-flag.yml b/changelogs/unreleased/211816-remove-streaming-serializer-feature-flag.yml
new file mode 100644
index 00000000000..fae6185d3ba
--- /dev/null
+++ b/changelogs/unreleased/211816-remove-streaming-serializer-feature-flag.yml
@@ -0,0 +1,5 @@
+---
+title: Use faster streaming serializer for project exports
+merge_request: 28925
+author:
+type: performance
diff --git a/changelogs/unreleased/Resolve-Migrate--fa-spinner-app-views-projects-mirrors.yml b/changelogs/unreleased/Resolve-Migrate--fa-spinner-app-views-projects-mirrors.yml
new file mode 100644
index 00000000000..84501c1268c
--- /dev/null
+++ b/changelogs/unreleased/Resolve-Migrate--fa-spinner-app-views-projects-mirrors.yml
@@ -0,0 +1,5 @@
+---
+title: Migrate .fa-spinner to .spinner for app/views/projects/mirrors
+merge_request: 25041
+author: nuwe1
+type: other
diff --git a/changelogs/unreleased/introduce_update_highest_role_concern.yml b/changelogs/unreleased/introduce_update_highest_role_concern.yml
new file mode 100644
index 00000000000..ff848f09b92
--- /dev/null
+++ b/changelogs/unreleased/introduce_update_highest_role_concern.yml
@@ -0,0 +1,5 @@
+---
+title: Use concern instead of service to update highest role
+merge_request: 28791
+author:
+type: other
diff --git a/db/migrate/20200402123926_add_line_metrics_to_mr_metrics.rb b/db/migrate/20200402123926_add_line_metrics_to_mr_metrics.rb
new file mode 100644
index 00000000000..1ca106e1614
--- /dev/null
+++ b/db/migrate/20200402123926_add_line_metrics_to_mr_metrics.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class AddLineMetricsToMrMetrics < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def up
+ with_lock_retries do
+ add_column :merge_request_metrics, :added_lines, :integer
+ add_column :merge_request_metrics, :removed_lines, :integer
+ end
+ end
+
+ def down
+ with_lock_retries do
+ remove_column :merge_request_metrics, :added_lines, :integer
+ remove_column :merge_request_metrics, :removed_lines, :integer
+ end
+ end
+end
diff --git a/db/structure.sql b/db/structure.sql
index 2697dbbe2b1..f7c2fadef18 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -3749,7 +3749,9 @@ CREATE TABLE public.merge_request_metrics (
modified_paths_size integer,
commits_count integer,
first_approved_at timestamp with time zone,
- first_reassigned_at timestamp with time zone
+ first_reassigned_at timestamp with time zone,
+ added_lines integer,
+ removed_lines integer
);
CREATE SEQUENCE public.merge_request_metrics_id_seq
@@ -12936,6 +12938,7 @@ COPY "schema_migrations" (version) FROM STDIN;
20200330123739
20200330132913
20200331220930
+20200402123926
20200402135250
20200403184110
20200403185127
diff --git a/doc/administration/high_availability/object_storage.md b/doc/administration/high_availability/object_storage.md
index 148753657cf..eeb730d3cc7 100644
--- a/doc/administration/high_availability/object_storage.md
+++ b/doc/administration/high_availability/object_storage.md
@@ -1,32 +1,5 @@
---
-type: reference
+redirect_to: '../object_storage.md'
---
-# Cloud Object Storage
-
-GitLab supports utilizing a Cloud Object Storage service rather than [NFS](nfs.md) for holding
-numerous types of data. This is recommended in larger setups as object storage is
-typically much more performant, reliable, and scalable.
-
-For configuring GitLab to use Object Storage refer to the following guides:
-
-1. Make sure the [`git` user home directory](https://docs.gitlab.com/omnibus/settings/configuration.html#moving-the-home-directory-for-a-user) is on local disk.
-1. Configure [database lookup of SSH keys](../operations/fast_ssh_key_lookup.md)
- to eliminate the need for a shared `authorized_keys` file.
-1. Configure [object storage for backups](../../raketasks/backup_restore.md#uploading-backups-to-a-remote-cloud-storage).
-1. Configure [object storage for job artifacts](../job_artifacts.md#using-object-storage)
- including [incremental logging](../job_logs.md#new-incremental-logging-architecture).
-1. Configure [object storage for LFS objects](../lfs/lfs_administration.md#storing-lfs-objects-in-remote-object-storage).
-1. Configure [object storage for uploads](../uploads.md#using-object-storage-core-only).
-1. Configure [object storage for merge request diffs](../merge_request_diffs.md#using-object-storage).
-1. Configure [object storage for container registry](../packages/container_registry.md#container-registry-storage-driver) (optional feature).
-1. Configure [object storage for Mattermost](https://docs.mattermost.com/administration/config-settings.html#file-storage) (optional feature).
-1. Configure [object storage for packages](../packages/index.md#using-object-storage) (optional feature). **(PREMIUM ONLY)**
-1. Configure [object storage for dependency proxy](../packages/dependency_proxy.md#using-object-storage) (optional feature). **(PREMIUM ONLY)**
-1. Configure [object storage for Pseudonymizer](../pseudonymizer.md#configuration) (optional feature). **(ULTIMATE ONLY)**
-
-NOTE: **Note:**
-One current feature of GitLab that still requires a shared directory (NFS) is
-[GitLab Pages](../../user/project/pages/index.md).
-There is [work in progress](https://gitlab.com/gitlab-org/gitlab-pages/issues/196)
-to eliminate the need for NFS to support GitLab Pages.
+This document was moved to [another location](../object_storage.md).
diff --git a/doc/administration/job_artifacts.md b/doc/administration/job_artifacts.md
index c003761299e..c45388087ab 100644
--- a/doc/administration/job_artifacts.md
+++ b/doc/administration/job_artifacts.md
@@ -90,7 +90,7 @@ This configuration relies on valid AWS credentials to be configured already.
Use an object storage option like AWS S3 to store job artifacts.
DANGER: **Danger:**
-If you're enabling S3 in [GitLab HA](high_availability/README.md), you will need to have an [NFS mount set up for CI logs and artifacts](high_availability/nfs.md#a-single-nfs-mount) or enable [incremental logging](job_logs.md#new-incremental-logging-architecture). If these settings are not set, you will risk job logs disappearing or not being saved.
+If you configure GitLab to store CI logs and artifacts on object storage, you must also enable [incremental logging](job_logs.md#new-incremental-logging-architecture). Otherwise, job logs will disappear or not be saved.
#### Object Storage Settings
diff --git a/doc/administration/object_storage.md b/doc/administration/object_storage.md
new file mode 100644
index 00000000000..ba537c5019a
--- /dev/null
+++ b/doc/administration/object_storage.md
@@ -0,0 +1,140 @@
+---
+type: reference
+---
+
+# Object Storage
+
+GitLab supports using an object storage service for holding numerous types of data.
+In a high availability setup, it's recommended over [NFS](high_availability/nfs.md) and
+in general it's better in larger setups as object storage is
+typically much more performant, reliable, and scalable.
+
+## Options
+
+Object storage options that GitLab has tested, or is aware of customers using include:
+
+- SaaS/Cloud solutions such as [Amazon S3](https://aws.amazon.com/s3/), [Google cloud storage](https://cloud.google.com/storage).
+- On-premises hardware and appliances from various storage vendors.
+- MinIO. We have [a guide to deploying this](https://docs.gitlab.com/charts/advanced/external-object-storage/minio.html) within our Helm Chart documentation.
+
+## Configuration guides
+
+For configuring GitLab to use Object Storage refer to the following guides:
+
+1. Make sure the [`git` user home directory](https://docs.gitlab.com/omnibus/settings/configuration.html#moving-the-home-directory-for-a-user) is on local disk.
+1. Configure [database lookup of SSH keys](operations/fast_ssh_key_lookup.md)
+ to eliminate the need for a shared `authorized_keys` file.
+1. Configure [object storage for backups](../raketasks/backup_restore.md#uploading-backups-to-a-remote-cloud-storage).
+1. Configure [object storage for job artifacts](job_artifacts.md#using-object-storage)
+ including [incremental logging](job_logs.md#new-incremental-logging-architecture).
+1. Configure [object storage for LFS objects](lfs/lfs_administration.md#storing-lfs-objects-in-remote-object-storage).
+1. Configure [object storage for uploads](uploads.md#using-object-storage-core-only).
+1. Configure [object storage for merge request diffs](merge_request_diffs.md#using-object-storage).
+1. Configure [object storage for Container Registry](packages/container_registry.md#container-registry-storage-driver) (optional feature).
+1. Configure [object storage for Mattermost](https://docs.mattermost.com/administration/config-settings.html#file-storage) (optional feature).
+1. Configure [object storage for packages](packages/index.md#using-object-storage) (optional feature). **(PREMIUM ONLY)**
+1. Configure [object storage for Dependency Proxy](packages/dependency_proxy.md#using-object-storage) (optional feature). **(PREMIUM ONLY)**
+1. Configure [object storage for Pseudonymizer](pseudonymizer.md#configuration) (optional feature). **(ULTIMATE ONLY)**
+1. Configure [object storage for autoscale Runner caching](https://docs.gitlab.com/runner/configuration/autoscale.html#distributed-runners-caching) (optional - for improved performance).
+
+## Warnings, limitations, and known issues
+
+### Use separate buckets
+
+Using separate buckets for each data type is the recommended approach for GitLab.
+
+A limitation of our configuration is that each use of object storage is separately configured.
+[We have an issue for improving this](https://gitlab.com/gitlab-org/gitlab/-/issues/23345)
+and easily using one bucket with separate folders is one improvement that this might bring.
+
+There is at least one specific issue with using the same bucket:
+when GitLab is deployed with the Helm chart restore from backup
+[will not properly function](https://docs.gitlab.com/charts/advanced/external-object-storage/#lfs-artifacts-uploads-packages-external-diffs-pseudonymizer)
+unless separate buckets are used.
+
+One risk of using a single bucket would be that if your organisation decided to
+migrate GitLab to the Helm deployment in the future. GitLab would run, but the situation with
+backups might not be realised until the organisation had a critical requirement for the backups to work.
+
+### S3 API compatability issues
+
+Not all S3 providers [are fully compatible](../raketasks/backup_restore.md#other-s3-providers)
+with the Fog library that GitLab uses. Symptoms include:
+
+```plaintext
+411 Length Required
+```
+
+### GitLab Pages requires NFS
+
+If you're working to [scale out](high_availability/README.md) your GitLab implementation and
+one of your requirements is [GitLab Pages](../user/project/pages/index.md) this currently requires
+NFS. There is [work in progress](https://gitlab.com/gitlab-org/gitlab-pages/issues/196)
+to remove this dependency. In the future, GitLab Pages may use
+[object storage](https://gitlab.com/gitlab-org/gitlab/-/issues/208135).
+
+The dependency on disk storage also prevents Pages being deployed using the
+[GitLab Helm chart](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/37).
+
+### Incremental logging is required for CI to use object storage
+
+If you configure GitLab to use object storage for CI logs and artifacts,
+[you must also enable incremental logging](job_artifacts.md#using-object-storage).
+
+### Proxy Download
+
+A number of the use cases for object storage allow client traffic to be redirected to the
+object storage back end, like when Git clients request large files via LFS or when
+downloading CI artifacts and logs.
+
+When the files are stored on local block storage or NFS, GitLab has to act as a proxy.
+This is not the default behaviour with object storage.
+
+The `proxy_download` setting controls this behaviour: the default is generally `false`.
+Verify this in the documentation for each use case. Set it to `true` so that GitLab proxies
+the files.
+
+When not proxying files, GitLab returns an
+[HTTP 302 redirect with a pre-signed, time-limited object storage URL](https://gitlab.com/gitlab-org/gitlab/-/issues/32117#note_218532298).
+This can result in some of the following problems:
+
+- If GitLab is using non-secure HTTP to access the object storage, clients may generate
+`https->http` downgrade errors and refuse to process the redirect. The solution to this
+is for GitLab to use HTTPS. LFS, for example, will generate this error:
+
+ ```plaintext
+ LFS: lfsapi/client: refusing insecure redirect, https->http
+ ```
+
+- Clients will need to trust the certificate authority that issued the object storage
+certificate, or may return common TLS errors such as:
+
+ ```plaintext
+ x509: certificate signed by unknown authority
+ ```
+
+- Clients will need network access to the object storage. Errors that might result
+if this access is not in place include:
+
+ ```plaintext
+ Received status code 403 from server: Forbidden
+ ```
+
+Getting a `403 Forbidden` response is specifically called out on the
+[package repository documentation](packages/index.md#using-object-storage)
+as a side effect of how some build tools work.
+
+### ETag mismatch
+
+Using the default GitLab settings, some object storage back-ends such as
+[MinIO](https://gitlab.com/gitlab-org/gitlab/-/issues/23188)
+and [Alibaba](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/1564)
+might generate `ETag mismatch` errors.
+
+When using GitLab direct upload, the
+[workaround for MinIO](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/1564#note_244497658)
+is to use the `--compat` parameter on the server.
+
+We are working on a fix to GitLab component Workhorse, and also
+a workaround, in the mean time, to
+[allow ETag verification to be disabled](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18175).
diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md
index 596ba01f5e0..57c6105372f 100644
--- a/doc/development/documentation/styleguide.md
+++ b/doc/development/documentation/styleguide.md
@@ -915,7 +915,7 @@ nicely on different mobile devices.
- When providing a command without output, don't prefix the shell command with `$`.
- If you need to include triple backticks inside a code block, use four backticks
for the codeblock fences instead of three.
-- For regular code blocks, always use a highlighting class corresponding to the
+- For regular fenced code blocks, always use a highlighting class corresponding to the
language for better readability. Examples:
````markdown
@@ -936,7 +936,7 @@ nicely on different mobile devices.
```
````
-Syntax highlighting is required for code blocks added to the GitLab documentation.
+Syntax highlighting is required for fenced code blocks added to the GitLab documentation.
Refer to the table below for the most common language classes, or check the
[complete list](https://github.com/rouge-ruby/rouge/wiki/List-of-supported-languages-and-lexers)
of language classes available.
diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md
index f454144b623..7f241a74820 100644
--- a/doc/user/project/settings/import_export.md
+++ b/doc/user/project/settings/import_export.md
@@ -73,7 +73,7 @@ The following items will be exported:
- Project and wiki repositories
- Project uploads
- Project configuration, including services
-- Issues with comments, merge requests with diffs and comments, labels, milestones, snippets,
+- Issues with comments, merge requests with diffs and comments, labels, milestones, snippets, time tracking,
and other project entities
- Design Management files and data
- LFS objects
diff --git a/lib/gitlab/import_export/project/legacy_tree_saver.rb b/lib/gitlab/import_export/project/legacy_tree_saver.rb
deleted file mode 100644
index 2ed98f52c58..00000000000
--- a/lib/gitlab/import_export/project/legacy_tree_saver.rb
+++ /dev/null
@@ -1,68 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module ImportExport
- module Project
- class LegacyTreeSaver
- attr_reader :full_path
-
- def initialize(project:, current_user:, shared:, params: {})
- @params = params
- @project = project
- @current_user = current_user
- @shared = shared
- @full_path = File.join(@shared.export_path, ImportExport.project_filename)
- end
-
- def save
- project_tree = tree_saver.serialize(@project, reader.project_tree)
- fix_project_tree(project_tree)
- tree_saver.save(project_tree, @shared.export_path, ImportExport.project_filename)
-
- true
- rescue => e
- @shared.error(e)
- false
- end
-
- private
-
- # Aware that the resulting hash needs to be pure-hash and
- # does not include any AR objects anymore, only objects that run `.to_json`
- def fix_project_tree(project_tree)
- if @params[:description].present?
- project_tree['description'] = @params[:description]
- end
-
- project_tree['project_members'] += group_members_array
- end
-
- def reader
- @reader ||= Gitlab::ImportExport::Reader.new(shared: @shared)
- end
-
- def group_members_array
- group_members.as_json(reader.group_members_tree).each do |group_member|
- group_member['source_type'] = 'Project' # Make group members project members of the future import
- end
- end
-
- def group_members
- return [] unless @current_user.can?(:admin_group, @project.group)
-
- # We need `.where.not(user_id: nil)` here otherwise when a group has an
- # invitee, it would make the following query return 0 rows since a NULL
- # user_id would be present in the subquery
- # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values
- non_null_user_ids = @project.project_members.where.not(user_id: nil).select(:user_id)
-
- GroupMembersFinder.new(@project.group).execute.where.not(user_id: non_null_user_ids)
- end
-
- def tree_saver
- @tree_saver ||= Gitlab::ImportExport::LegacyRelationTreeSaver.new
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/repository_url_builder.rb b/lib/gitlab/repository_url_builder.rb
new file mode 100644
index 00000000000..2b88af1f77c
--- /dev/null
+++ b/lib/gitlab/repository_url_builder.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module RepositoryUrlBuilder
+ class << self
+ def build(path, protocol: :ssh)
+ # TODO: See https://gitlab.com/gitlab-org/gitlab/-/issues/213021
+ path = path.sub('@snippets', 'snippets')
+
+ case protocol
+ when :ssh
+ ssh_url(path)
+ when :http
+ http_url(path)
+ else
+ raise NotImplementedError.new("No URL builder defined for protocol #{protocol}")
+ end
+ end
+
+ private
+
+ def ssh_url(path)
+ Gitlab.config.gitlab_shell.ssh_path_prefix + "#{path}.git"
+ end
+
+ def http_url(path)
+ root = Gitlab::CurrentSettings.custom_http_clone_url_root.presence || Gitlab::Routing.url_helpers.root_url
+
+ Gitlab::Utils.append_path(root, "#{path}.git")
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 1f8a45e5481..24f49f6b943 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -47,14 +47,6 @@ module Gitlab
@version ||= File.read(gitlab_shell_version_file).chomp if File.readable?(gitlab_shell_version_file)
end
- # Return a SSH url for a given project path
- #
- # @param [String] full_path project path (URL)
- # @return [String] SSH URL
- def url_to_repo(full_path)
- Gitlab.config.gitlab_shell.ssh_path_prefix + "#{full_path}.git"
- end
-
private
def gitlab_shell_path
diff --git a/qa/qa/page/profile/personal_access_tokens.rb b/qa/qa/page/profile/personal_access_tokens.rb
index 6108fb4d93b..7069e7d3e4f 100644
--- a/qa/qa/page/profile/personal_access_tokens.rb
+++ b/qa/qa/page/profile/personal_access_tokens.rb
@@ -1,10 +1,13 @@
# frozen_string_literal: true
+require 'date'
+
module QA
module Page
module Profile
class PersonalAccessTokens < Page::Base
view 'app/views/shared/_personal_access_tokens_form.html.haml' do
+ element :expiry_date_field
element :personal_access_token_name_field
element :create_token_button
end
@@ -36,6 +39,13 @@ module QA
find_element(:created_personal_access_token, wait: 30).value
end
+ def fill_expiry_date(date)
+ date = date.to_s if date.is_a?(Date)
+ Date.strptime(date, '%Y-%m-%d') rescue ArgumentError raise "Expiry date must be in YYYY-MM-DD format"
+
+ fill_element(:expiry_date_field, date)
+ end
+
def has_token_row_for_name?(token_name)
page.has_css?('tr', text: token_name, wait: 1.0)
end
diff --git a/qa/qa/resource/personal_access_token.rb b/qa/qa/resource/personal_access_token.rb
index 3b61b3f337c..488138326df 100644
--- a/qa/qa/resource/personal_access_token.rb
+++ b/qa/qa/resource/personal_access_token.rb
@@ -1,5 +1,7 @@
# frozen_string_literal: true
+require 'date'
+
module QA
module Resource
##
@@ -19,6 +21,8 @@ module QA
Page::Profile::PersonalAccessTokens.perform do |token_page|
token_page.fill_token_name(name || 'api-test-token')
token_page.check_api
+ # Expire in 2 days just in case the token is created just before midnight
+ token_page.fill_expiry_date(Date.today + 2)
token_page.click_create_token_button
end
end
diff --git a/spec/controllers/projects/alerting/notifications_controller_spec.rb b/spec/controllers/projects/alerting/notifications_controller_spec.rb
index a56ac59215f..9d26c2278b1 100644
--- a/spec/controllers/projects/alerting/notifications_controller_spec.rb
+++ b/spec/controllers/projects/alerting/notifications_controller_spec.rb
@@ -48,7 +48,7 @@ describe Projects::Alerting::NotificationsController do
end
context 'when notification service fails' do
- let(:service_response) { ServiceResponse.error(message: 'Unauthorized', http_status: 401) }
+ let(:service_response) { ServiceResponse.error(message: 'Unauthorized', http_status: :unauthorized) }
it 'responds with the service response' do
make_request
diff --git a/spec/controllers/projects/prometheus/alerts_controller_spec.rb b/spec/controllers/projects/prometheus/alerts_controller_spec.rb
index e215f4b68fa..451834e0962 100644
--- a/spec/controllers/projects/prometheus/alerts_controller_spec.rb
+++ b/spec/controllers/projects/prometheus/alerts_controller_spec.rb
@@ -158,7 +158,8 @@ describe Projects::Prometheus::AlertsController do
end
describe 'POST #notify' do
- let(:notify_service) { spy }
+ let(:service_response) { ServiceResponse.success }
+ let(:notify_service) { instance_double(Projects::Prometheus::Alerts::NotifyService, execute: service_response) }
before do
sign_out(user)
@@ -170,7 +171,7 @@ describe Projects::Prometheus::AlertsController do
end
it 'returns ok if notification succeeds' do
- expect(notify_service).to receive(:execute).and_return(true)
+ expect(notify_service).to receive(:execute).and_return(ServiceResponse.success)
post :notify, params: project_params, session: { as: :json }
@@ -178,7 +179,9 @@ describe Projects::Prometheus::AlertsController do
end
it 'returns unprocessable entity if notification fails' do
- expect(notify_service).to receive(:execute).and_return(false)
+ expect(notify_service).to receive(:execute).and_return(
+ ServiceResponse.error(message: 'Unprocessable Entity', http_status: :unprocessable_entity)
+ )
post :notify, params: project_params, session: { as: :json }
diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb
index 1e13afc6033..7ee8f42e6ef 100644
--- a/spec/features/projects/import_export/export_file_spec.rb
+++ b/spec/features/projects/import_export/export_file_spec.rb
@@ -38,7 +38,11 @@ describe 'Import/Export - project export integration test', :js do
sign_in(user)
end
- shared_examples 'export file without sensitive words' do
+ context "with streaming serializer" do
+ before do
+ stub_feature_flags(project_export_as_ndjson: false)
+ end
+
it 'exports a project successfully', :sidekiq_inline do
export_project_and_download_file(page, project)
@@ -59,27 +63,8 @@ describe 'Import/Export - project export integration test', :js do
end
end
- context "with legacy export" do
- before do
- stub_feature_flags(streaming_serializer: false)
- stub_feature_flags(project_export_as_ndjson: false)
- end
-
- it_behaves_like "export file without sensitive words"
- end
-
- context "with streaming serializer" do
- before do
- stub_feature_flags(streaming_serializer: true)
- stub_feature_flags(project_export_as_ndjson: false)
- end
-
- it_behaves_like "export file without sensitive words"
- end
-
context "with ndjson" do
before do
- stub_feature_flags(streaming_serializer: true)
stub_feature_flags(project_export_as_ndjson: true)
end
diff --git a/spec/lib/gitlab/import_export/project/legacy_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project/legacy_tree_saver_spec.rb
deleted file mode 100644
index e51f6888bbb..00000000000
--- a/spec/lib/gitlab/import_export/project/legacy_tree_saver_spec.rb
+++ /dev/null
@@ -1,352 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::ImportExport::Project::LegacyTreeSaver do
- describe 'saves the project tree into a json object' do
- let(:shared) { project.import_export_shared }
- let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) }
- let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
- let(:user) { create(:user) }
- let!(:project) { setup_project }
-
- before do
- project.add_maintainer(user)
- allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
- allow_any_instance_of(MergeRequest).to receive(:source_branch_sha).and_return('ABCD')
- allow_any_instance_of(MergeRequest).to receive(:target_branch_sha).and_return('DCBA')
- end
-
- after do
- FileUtils.rm_rf(export_path)
- end
-
- it 'saves project successfully' do
- expect(project_tree_saver.save).to be true
- end
-
- # It is mostly duplicated in
- # `spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb`
- # except:
- # context 'with description override' do
- # context 'group members' do
- # ^ These are specific for the Project::TreeSaver
- context 'JSON' do
- let(:saved_project_json) do
- project_tree_saver.save
- project_json(project_tree_saver.full_path)
- end
-
- # It is not duplicated in
- # `spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb`
- context 'with description override' do
- let(:params) { { description: 'Foo Bar' } }
- let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared, params: params) }
-
- it 'overrides the project description' do
- expect(saved_project_json).to include({ 'description' => params[:description] })
- end
- end
-
- it 'saves the correct json' do
- expect(saved_project_json).to include({ 'description' => 'description', 'visibility_level' => 20 })
- end
-
- it 'has approvals_before_merge set' do
- expect(saved_project_json['approvals_before_merge']).to eq(1)
- end
-
- it 'has milestones' do
- expect(saved_project_json['milestones']).not_to be_empty
- end
-
- it 'has merge requests' do
- expect(saved_project_json['merge_requests']).not_to be_empty
- end
-
- it 'has merge request\'s milestones' do
- expect(saved_project_json['merge_requests'].first['milestone']).not_to be_empty
- end
-
- it 'has merge request\'s source branch SHA' do
- expect(saved_project_json['merge_requests'].first['source_branch_sha']).to eq('ABCD')
- end
-
- it 'has merge request\'s target branch SHA' do
- expect(saved_project_json['merge_requests'].first['target_branch_sha']).to eq('DCBA')
- end
-
- it 'has events' do
- expect(saved_project_json['merge_requests'].first['milestone']['events']).not_to be_empty
- end
-
- it 'has snippets' do
- expect(saved_project_json['snippets']).not_to be_empty
- end
-
- it 'has snippet notes' do
- expect(saved_project_json['snippets'].first['notes']).not_to be_empty
- end
-
- it 'has releases' do
- expect(saved_project_json['releases']).not_to be_empty
- end
-
- it 'has no author on releases' do
- expect(saved_project_json['releases'].first['author']).to be_nil
- end
-
- it 'has the author ID on releases' do
- expect(saved_project_json['releases'].first['author_id']).not_to be_nil
- end
-
- it 'has issues' do
- expect(saved_project_json['issues']).not_to be_empty
- end
-
- it 'has issue comments' do
- notes = saved_project_json['issues'].first['notes']
-
- expect(notes).not_to be_empty
- expect(notes.first['type']).to eq('DiscussionNote')
- end
-
- it 'has issue assignees' do
- expect(saved_project_json['issues'].first['issue_assignees']).not_to be_empty
- end
-
- it 'has author on issue comments' do
- expect(saved_project_json['issues'].first['notes'].first['author']).not_to be_empty
- end
-
- it 'has project members' do
- expect(saved_project_json['project_members']).not_to be_empty
- end
-
- it 'has merge requests diffs' do
- expect(saved_project_json['merge_requests'].first['merge_request_diff']).not_to be_empty
- end
-
- it 'has merge request diff files' do
- expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_files']).not_to be_empty
- end
-
- it 'has merge request diff commits' do
- expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_commits']).not_to be_empty
- end
-
- it 'has merge requests comments' do
- expect(saved_project_json['merge_requests'].first['notes']).not_to be_empty
- end
-
- it 'has author on merge requests comments' do
- expect(saved_project_json['merge_requests'].first['notes'].first['author']).not_to be_empty
- end
-
- it 'has pipeline stages' do
- expect(saved_project_json.dig('ci_pipelines', 0, 'stages')).not_to be_empty
- end
-
- it 'has pipeline statuses' do
- expect(saved_project_json.dig('ci_pipelines', 0, 'stages', 0, 'statuses')).not_to be_empty
- end
-
- it 'has pipeline builds' do
- builds_count = saved_project_json
- .dig('ci_pipelines', 0, 'stages', 0, 'statuses')
- .count { |hash| hash['type'] == 'Ci::Build' }
-
- expect(builds_count).to eq(1)
- end
-
- it 'has no when YML attributes but only the DB column' do
- expect_any_instance_of(Gitlab::Ci::YamlProcessor).not_to receive(:build_attributes)
-
- saved_project_json
- end
-
- it 'has pipeline commits' do
- expect(saved_project_json['ci_pipelines']).not_to be_empty
- end
-
- it 'has ci pipeline notes' do
- expect(saved_project_json['ci_pipelines'].first['notes']).not_to be_empty
- end
-
- it 'has labels with no associations' do
- expect(saved_project_json['labels']).not_to be_empty
- end
-
- it 'has labels associated to records' do
- expect(saved_project_json['issues'].first['label_links'].first['label']).not_to be_empty
- end
-
- it 'has project and group labels' do
- label_types = saved_project_json['issues'].first['label_links'].map { |link| link['label']['type'] }
-
- expect(label_types).to match_array(%w(ProjectLabel GroupLabel))
- end
-
- it 'has priorities associated to labels' do
- priorities = saved_project_json['issues'].first['label_links'].flat_map { |link| link['label']['priorities'] }
-
- expect(priorities).not_to be_empty
- end
-
- it 'has issue resource label events' do
- expect(saved_project_json['issues'].first['resource_label_events']).not_to be_empty
- end
-
- it 'has merge request resource label events' do
- expect(saved_project_json['merge_requests'].first['resource_label_events']).not_to be_empty
- end
-
- it 'saves the correct service type' do
- expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService')
- end
-
- it 'saves the properties for a service' do
- expect(saved_project_json['services'].first['properties']).to eq('one' => 'value')
- end
-
- it 'has project feature' do
- project_feature = saved_project_json['project_feature']
- expect(project_feature).not_to be_empty
- expect(project_feature["issues_access_level"]).to eq(ProjectFeature::DISABLED)
- expect(project_feature["wiki_access_level"]).to eq(ProjectFeature::ENABLED)
- expect(project_feature["builds_access_level"]).to eq(ProjectFeature::PRIVATE)
- end
-
- it 'has custom attributes' do
- expect(saved_project_json['custom_attributes'].count).to eq(2)
- end
-
- it 'has badges' do
- expect(saved_project_json['project_badges'].count).to eq(2)
- end
-
- it 'does not complain about non UTF-8 characters in MR diff files' do
- ActiveRecord::Base.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n YXR'")
-
- expect(project_tree_saver.save).to be true
- end
-
- context 'group members' do
- let(:user2) { create(:user, email: 'group@member.com') }
- let(:member_emails) do
- saved_project_json['project_members'].map do |pm|
- pm['user']['email']
- end
- end
-
- before do
- Group.first.add_developer(user2)
- end
-
- it 'does not export group members if it has no permission' do
- Group.first.add_developer(user)
-
- expect(member_emails).not_to include('group@member.com')
- end
-
- it 'does not export group members as maintainer' do
- Group.first.add_maintainer(user)
-
- expect(member_emails).not_to include('group@member.com')
- end
-
- it 'exports group members as group owner' do
- Group.first.add_owner(user)
-
- expect(member_emails).to include('group@member.com')
- end
-
- context 'as admin' do
- let(:user) { create(:admin) }
-
- it 'exports group members as admin' do
- expect(member_emails).to include('group@member.com')
- end
-
- it 'exports group members as project members' do
- member_types = saved_project_json['project_members'].map { |pm| pm['source_type'] }
-
- expect(member_types).to all(eq('Project'))
- end
- end
- end
-
- context 'project attributes' do
- it 'does not contain the runners token' do
- expect(saved_project_json).not_to include("runners_token" => 'token')
- end
- end
-
- it 'has a board and a list' do
- expect(saved_project_json['boards'].first['lists']).not_to be_empty
- end
- end
- end
-
- def setup_project
- release = create(:release)
- group = create(:group)
-
- project = create(:project,
- :public,
- :repository,
- :issues_disabled,
- :wiki_enabled,
- :builds_private,
- description: 'description',
- 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)
- create(:label_link, label: group_label, target: issue)
- create(:label_priority, label: group_label, priority: 1)
- milestone = create(:milestone, project: project)
- merge_request = create(:merge_request, source_project: project, milestone: milestone)
-
- ci_build = create(:ci_build, project: project, when: nil)
- ci_build.pipeline.update(project: project)
- create(:commit_status, project: project, pipeline: ci_build.pipeline)
-
- create(:milestone, project: project)
- create(:discussion_note, noteable: issue, project: project)
- create(:note, noteable: merge_request, project: project)
- create(:note, noteable: snippet, project: project)
- create(:note_on_commit,
- author: user,
- project: project,
- commit_id: ci_build.pipeline.sha)
-
- create(:resource_label_event, label: project_label, issue: issue)
- create(:resource_label_event, label: group_label, merge_request: merge_request)
-
- create(:event, :created, target: milestone, project: project, author: user)
- create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' })
-
- create(:project_custom_attribute, project: project)
- create(:project_custom_attribute, project: project)
-
- create(:project_badge, project: project)
- create(:project_badge, project: project)
-
- board = create(:board, project: project, name: 'TestBoard')
- create(:list, board: board, position: 0, label: project_label)
-
- project
- end
-
- def project_json(filename)
- ::JSON.parse(IO.read(filename))
- end
-end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 653b011084e..55b907fff7c 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -283,6 +283,8 @@ MergeRequest::Metrics:
- commits_count
- first_approved_at
- first_reassigned_at
+- added_lines
+- removed_lines
Ci::Pipeline:
- id
- project_id
diff --git a/spec/lib/gitlab/repository_url_builder_spec.rb b/spec/lib/gitlab/repository_url_builder_spec.rb
new file mode 100644
index 00000000000..3d8870ecb53
--- /dev/null
+++ b/spec/lib/gitlab/repository_url_builder_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::RepositoryUrlBuilder do
+ describe '.build' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:factory, :path_generator) do
+ :project | ->(project) { project.full_path }
+ :project_snippet | ->(snippet) { "#{snippet.project.full_path}/snippets/#{snippet.id}" }
+ :project_wiki | ->(wiki) { "#{wiki.project.full_path}.wiki" }
+
+ :personal_snippet | ->(snippet) { "snippets/#{snippet.id}" }
+ end
+
+ with_them do
+ let(:container) { build_stubbed(factory) }
+ let(:repository) { container.repository }
+ let(:path) { path_generator.call(container) }
+ let(:url) { subject.build(repository.full_path, protocol: protocol) }
+
+ context 'when passing SSH protocol' do
+ let(:protocol) { :ssh }
+
+ it 'returns the SSH URL to the repository' do
+ expect(url).to eq("#{Gitlab.config.gitlab_shell.ssh_path_prefix}#{path}.git")
+ end
+ end
+
+ context 'when passing HTTP protocol' do
+ let(:protocol) { :http }
+
+ it 'returns the HTTP URL to the repo without a username' do
+ expect(url).to eq("#{Gitlab.config.gitlab.url}/#{path}.git")
+ expect(url).not_to include('@')
+ end
+
+ it 'includes the custom HTTP clone root if set' do
+ clone_root = 'https://git.example.com:51234/mygitlab'
+ stub_application_setting(custom_http_clone_url_root: clone_root)
+
+ expect(url).to eq("#{clone_root}/#{path}.git")
+ end
+ end
+
+ context 'when passing an unsupported protocol' do
+ let(:protocol) { :ftp }
+
+ it 'raises an exception' do
+ expect { url }.to raise_error(NotImplementedError)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index e4c33863ac2..1f515cffdbf 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -10,14 +10,6 @@ describe Gitlab::Shell do
it { is_expected.to respond_to :remove_repository }
- describe '.url_to_repo' do
- let(:full_path) { 'diaspora/disaspora-rails' }
-
- subject { described_class.url_to_repo(full_path) }
-
- it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + full_path + '.git') }
- end
-
describe 'memoized secret_token' do
let(:secret_file) { 'tmp/tests/.secret_shell_test' }
let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' }
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index e922542a984..eeb2350359c 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
describe Member do
+ include ExclusiveLeaseHelpers
+
using RSpec::Parameterized::TableSyntax
describe "Associations" do
@@ -593,6 +595,9 @@ describe Member do
end
context 'when after_commit :update_highest_role' do
+ let!(:user) { create(:user) }
+ let(:user_id) { user.id }
+
where(:member_type, :source_type) do
:project_member | :project
:group_member | :group
@@ -600,43 +605,34 @@ describe Member do
with_them do
describe 'create member' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- source = create(source_type) # source owner initializes a new service object too
- user = create(:user)
+ let!(:source) { create(source_type) }
- expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original
+ subject { create(member_type, :guest, user: user, source_type => source) }
- create(member_type, :guest, user: user, source_type => source)
- end
+ include_examples 'update highest role with exclusive lease'
end
context 'when member exists' do
- let!(:member) { create(member_type) }
+ let!(:member) { create(member_type, user: user) }
describe 'update member' do
context 'when access level was changed' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original
+ subject { member.update(access_level: Gitlab::Access::GUEST) }
- member.update(access_level: Gitlab::Access::GUEST)
- end
+ include_examples 'update highest role with exclusive lease'
end
context 'when access level was not changed' do
- it 'does not initialize a new Members::UpdateHighestRoleService object' do
- expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id)
+ subject { member.update(notification_level: NotificationSetting.levels[:disabled]) }
- member.update(notification_level: NotificationSetting.levels[:disabled])
- end
+ include_examples 'does not update the highest role'
end
end
describe 'destroy member' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original
+ subject { member.destroy }
- member.destroy
- end
+ include_examples 'update highest role with exclusive lease'
end
end
end
diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb
index 9f5fd3a9495..1b121b7dee1 100644
--- a/spec/models/project_wiki_spec.rb
+++ b/spec/models/project_wiki_spec.rb
@@ -34,7 +34,7 @@ describe ProjectWiki do
describe "#url_to_repo" do
it "returns the correct ssh url to the repo" do
- expect(subject.url_to_repo).to eq(Gitlab::Shell.url_to_repo(subject.full_path))
+ expect(subject.url_to_repo).to eq(Gitlab::RepositoryUrlBuilder.build(subject.repository.full_path, protocol: :ssh))
end
end
@@ -45,27 +45,8 @@ describe ProjectWiki do
end
describe "#http_url_to_repo" do
- let(:project) { create :project }
-
- context 'when a custom HTTP clone URL root is not set' do
- it 'returns the full http url to the repo' do
- expected_url = "#{Gitlab.config.gitlab.url}/#{subject.full_path}.git"
-
- expect(project_wiki.http_url_to_repo).to eq(expected_url)
- expect(project_wiki.http_url_to_repo).not_to include('@')
- end
- end
-
- context 'when a custom HTTP clone URL root is set' do
- before do
- stub_application_setting(custom_http_clone_url_root: 'https://git.example.com:51234')
- end
-
- it 'returns the full http url to the repo, with the root replaced with the custom one' do
- expected_url = "https://git.example.com:51234/#{subject.full_path}.git"
-
- expect(project_wiki.http_url_to_repo).to eq(expected_url)
- end
+ it "returns the correct http url to the repo" do
+ expect(subject.http_url_to_repo).to eq(Gitlab::RepositoryUrlBuilder.build(subject.repository.full_path, protocol: :http))
end
end
diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb
index 9cced1f24ab..8c4d1951697 100644
--- a/spec/models/snippet_spec.rb
+++ b/spec/models/snippet_spec.rb
@@ -735,22 +735,6 @@ describe Snippet do
end
end
- describe '#url_to_repo' do
- subject { snippet.url_to_repo }
-
- context 'with personal snippet' do
- let(:snippet) { create(:personal_snippet) }
-
- it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "snippets/#{snippet.id}.git") }
- end
-
- context 'with project snippet' do
- let(:snippet) { create(:project_snippet) }
-
- it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "#{snippet.project.full_path}/snippets/#{snippet.id}.git") }
- end
- end
-
describe '#versioned_enabled_for?' do
let_it_be(:user) { create(:user) }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 679f4a19f5b..5a3e16baa87 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
describe User, :do_not_mock_admin_mode do
include ProjectForksHelper
include TermsHelper
+ include ExclusiveLeaseHelpers
it_behaves_like 'having unique enum values'
@@ -4535,17 +4536,22 @@ describe User, :do_not_mock_admin_mode do
context 'when after_commit :update_highest_role' do
describe 'create user' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- expect_next_instance_of(Members::UpdateHighestRoleService) do |service|
- expect(service).to receive(:execute)
+ subject { create(:user) }
+
+ it 'schedules a job in the future', :aggregate_failures, :clean_gitlab_redis_shared_state do
+ allow_next_instance_of(Gitlab::ExclusiveLease) do |instance|
+ allow(instance).to receive(:try_obtain).and_return('uuid')
end
- create(:user)
+ expect(UpdateHighestRoleWorker).to receive(:perform_in).and_call_original
+
+ expect { subject }.to change(UpdateHighestRoleWorker.jobs, :size).by(1)
end
end
context 'when user already exists' do
let!(:user) { create(:user) }
+ let(:user_id) { user.id }
describe 'update user' do
using RSpec::Parameterized::TableSyntax
@@ -4560,24 +4566,24 @@ describe User, :do_not_mock_admin_mode do
with_them do
context 'when state was changed' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- expect_next_instance_of(Members::UpdateHighestRoleService) do |service|
- expect(service).to receive(:execute)
- end
+ subject { user.update(attributes) }
- user.update(attributes)
- end
+ include_examples 'update highest role with exclusive lease'
end
end
context 'when state was not changed' do
- it 'does not initialize a new Members::UpdateHighestRoleService object' do
- expect(Members::UpdateHighestRoleService).not_to receive(:new)
+ subject { user.update(email: 'newmail@example.com') }
- user.update(email: 'newmail@example.com')
- end
+ include_examples 'does not update the highest role'
end
end
+
+ describe 'destroy user' do
+ subject { user.destroy }
+
+ include_examples 'does not update the highest role'
+ end
end
end
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index 31f8bcbfef0..40a0f09d1f3 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -5,9 +5,9 @@ require 'spec_helper'
describe MergeRequestWidgetEntity do
include ProjectForksHelper
- let(:project) { create :project, :repository }
+ let(:project) { create :project, :repository }
let(:resource) { create(:merge_request, source_project: project, target_project: project) }
- let(:user) { create(:user) }
+ let(:user) { create(:user) }
let(:request) { double('request', current_user: user, project: project) }
@@ -54,15 +54,17 @@ describe MergeRequestWidgetEntity do
end
describe 'merge_request_add_ci_config_path' do
+ let!(:project_auto_devops) { create(:project_auto_devops, :disabled, project: project) }
+
before do
project.add_role(user, role)
end
- context 'when there are pipelines' do
+ context 'when there is a standard ci config file in the source project' do
let(:role) { :developer }
before do
- create(:ci_empty_pipeline, project: project, sha: resource.all_commit_shas.first, ref: resource.source_branch)
+ project.repository.create_file(user, Gitlab::FileDetector::PATTERNS[:gitlab_ci], 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master')
end
it 'no ci config path' do
@@ -70,7 +72,7 @@ describe MergeRequestWidgetEntity do
end
end
- context 'when there are no pipelines' do
+ context 'when there is no standard ci config file in the source project' do
context 'when user has permissions' do
let(:role) { :developer }
@@ -80,6 +82,16 @@ describe MergeRequestWidgetEntity do
expect(subject[:merge_request_add_ci_config_path]).to eq(expected_path)
end
+ context 'when auto devops is enabled' do
+ before do
+ project_auto_devops.enabled = true
+ end
+
+ it 'returns a blank ci config path' do
+ expect(subject[:merge_request_add_ci_config_path]).to be_nil
+ end
+ end
+
context 'when source project is missing' do
before do
resource.source_project = nil
diff --git a/spec/services/members/update_highest_role_service_spec.rb b/spec/services/members/update_highest_role_service_spec.rb
deleted file mode 100644
index 6fcb939203d..00000000000
--- a/spec/services/members/update_highest_role_service_spec.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-require 'sidekiq/testing'
-
-describe Members::UpdateHighestRoleService, :clean_gitlab_redis_shared_state do
- include ExclusiveLeaseHelpers
-
- let_it_be(:user) { create(:user) }
- let_it_be(:lease_key) { "update_highest_role:#{user.id}" }
- let(:service) { described_class.new(user.id) }
-
- describe '#perform' do
- subject { service.execute }
-
- context 'when lease is obtained' do
- it 'takes the lease but does not release it', :aggregate_failures do
- expect_to_obtain_exclusive_lease(lease_key, 'uuid', timeout: described_class::LEASE_TIMEOUT)
-
- subject
-
- expect(service.exclusive_lease.exists?).to be_truthy
- end
-
- it 'schedules a job in the future', :aggregate_failures do
- expect(UpdateHighestRoleWorker).to receive(:perform_in).with(described_class::DELAY, user.id).and_call_original
-
- Sidekiq::Testing.fake! do
- expect { subject }.to change(UpdateHighestRoleWorker.jobs, :size).by(1)
- end
- end
- end
-
- context 'when lease cannot be obtained' do
- it 'only schedules one job' do
- Sidekiq::Testing.fake! do
- stub_exclusive_lease_taken(lease_key, timeout: described_class::LEASE_TIMEOUT)
-
- expect { subject }.not_to change(UpdateHighestRoleWorker.jobs, :size)
- end
- end
- end
- end
-end
diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb
index 289d812f498..f08ecd397ec 100644
--- a/spec/services/projects/alerting/notify_service_spec.rb
+++ b/spec/services/projects/alerting/notify_service_spec.rb
@@ -20,7 +20,7 @@ describe Projects::Alerting::NotifyService do
.exactly(amount).times
Sidekiq::Testing.inline! do
- expect(subject.status).to eq(:success)
+ expect(subject).to be_success
end
end
end
@@ -36,7 +36,7 @@ describe Projects::Alerting::NotifyService do
expect(notification_service)
.to receive_message_chain(:async, :prometheus_alerts_fired)
- expect(subject.status).to eq(:success)
+ expect(subject).to be_success
end
end
@@ -45,7 +45,7 @@ describe Projects::Alerting::NotifyService do
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
- expect(subject.status).to eq(:success)
+ expect(subject).to be_success
end
end
@@ -54,7 +54,7 @@ describe Projects::Alerting::NotifyService do
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
- expect(subject.status).to eq(:error)
+ expect(subject).to be_error
expect(subject.http_status).to eq(http_status)
end
end
@@ -102,7 +102,7 @@ describe Projects::Alerting::NotifyService do
.and_raise(Gitlab::Alerting::NotificationPayloadParser::BadPayloadError)
end
- it_behaves_like 'does not process incident issues due to error', http_status: 400
+ it_behaves_like 'does not process incident issues due to error', http_status: :bad_request
end
end
@@ -114,13 +114,13 @@ describe Projects::Alerting::NotifyService do
end
context 'with invalid token' do
- it_behaves_like 'does not process incident issues due to error', http_status: 401
+ it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized
end
context 'with deactivated Alerts Service' do
let!(:alerts_service) { create(:alerts_service, :inactive, project: project) }
- it_behaves_like 'does not process incident issues due to error', http_status: 403
+ it_behaves_like 'does not process incident issues due to error', http_status: :forbidden
end
end
end
diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb
index 1315ae26322..e00507d1827 100644
--- a/spec/services/projects/import_export/export_service_spec.rb
+++ b/spec/services/projects/import_export/export_service_spec.rb
@@ -26,28 +26,10 @@ describe Projects::ImportExport::ExportService do
service.execute
end
- context 'when :streaming_serializer feature is enabled' do
- before do
- stub_feature_flags(streaming_serializer: true)
- end
-
- it 'saves the models' do
- expect(Gitlab::ImportExport::Project::TreeSaver).to receive(:new).and_call_original
-
- service.execute
- end
- end
+ it 'saves the models' do
+ expect(Gitlab::ImportExport::Project::TreeSaver).to receive(:new).and_call_original
- context 'when :streaming_serializer feature is disabled' do
- before do
- stub_feature_flags(streaming_serializer: false)
- end
-
- it 'saves the models' do
- expect(Gitlab::ImportExport::Project::LegacyTreeSaver).to receive(:new).and_call_original
-
- service.execute
- end
+ service.execute
end
it 'saves the uploads' do
diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb
index ce850e65329..dce96dda1e3 100644
--- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb
+++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb
@@ -30,7 +30,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
expect(notification_service)
.to receive_message_chain(:async, :prometheus_alerts_fired)
- expect(subject).to eq(true)
+ expect(subject).to be_success
end
end
@@ -44,7 +44,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
.exactly(amount).times
Sidekiq::Testing.inline! do
- expect(subject).to eq(true)
+ expect(subject).to be_success
end
end
end
@@ -54,7 +54,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
expect(IncidentManagement::ProcessPrometheusAlertWorker)
.not_to receive(:perform_async)
- expect(subject).to eq(true)
+ expect(subject).to be_success
end
end
@@ -69,7 +69,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
expect(create_events_service)
.to receive(:execute)
- expect(subject).to eq(true)
+ expect(subject).to be_success
end
end
@@ -78,7 +78,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
it_behaves_like 'persists events'
end
- shared_examples 'no notifications' do
+ shared_examples 'no notifications' do |http_status:|
let(:notification_service) { spy }
let(:create_events_service) { spy }
@@ -86,7 +86,8 @@ describe Projects::Prometheus::Alerts::NotifyService do
expect(notification_service).not_to receive(:async)
expect(create_events_service).not_to receive(:execute)
- expect(subject).to eq(false)
+ expect(subject).to be_error
+ expect(subject.http_status).to eq(http_status)
end
end
@@ -130,7 +131,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
when :success
it_behaves_like 'notifies alerts'
when :failure
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :unauthorized
else
raise "invalid result: #{result.inspect}"
end
@@ -140,7 +141,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
context 'without project specific cluster' do
let!(:cluster) { create(:cluster, enabled: true) }
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :unauthorized
end
context 'with manual prometheus installation' do
@@ -171,7 +172,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
when :success
it_behaves_like 'notifies alerts'
when :failure
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :unauthorized
else
raise "invalid result: #{result.inspect}"
end
@@ -193,7 +194,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
expect_any_instance_of(NotificationService)
.not_to receive(:async)
- expect(subject).to eq(true)
+ expect(subject).to be_success
end
end
@@ -211,7 +212,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
it 'does not send notification' do
expect(NotificationService).not_to receive(:new)
- expect(subject).to eq(true)
+ expect(subject).to be_success
end
end
end
@@ -260,19 +261,19 @@ describe Projects::Prometheus::Alerts::NotifyService do
context 'without version' do
let(:payload) { {} }
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :unprocessable_entity
end
context 'when version is not "4"' do
let(:payload) { { 'version' => '5' } }
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :unprocessable_entity
end
context 'with missing alerts' do
let(:payload) { { 'version' => '4' } }
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :unauthorized
end
context 'when the payload is too big' do
@@ -283,7 +284,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
allow(Gitlab::Utils::DeepSize).to receive(:new).and_return(deep_size_object)
end
- it_behaves_like 'no notifications'
+ it_behaves_like 'no notifications', http_status: :bad_request
it 'does not process issues' do
expect(IncidentManagement::ProcessPrometheusAlertWorker)
diff --git a/spec/support/shared_examples/models/update_highest_role_shared_examples.rb b/spec/support/shared_examples/models/update_highest_role_shared_examples.rb
new file mode 100644
index 00000000000..34c4ada1718
--- /dev/null
+++ b/spec/support/shared_examples/models/update_highest_role_shared_examples.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+# requires a subject and a user_id
+RSpec.shared_examples 'update highest role with exclusive lease' do
+ include ExclusiveLeaseHelpers
+
+ let(:lease_key) { "update_highest_role:#{user_id}" }
+
+ before do
+ allow(Gitlab::ExclusiveLease).to receive(:new).and_call_original
+ end
+
+ context 'when lease is obtained', :clean_gitlab_redis_shared_state do
+ it 'takes the lease but does not release it', :aggregate_failures do
+ expect_to_obtain_exclusive_lease(lease_key, 'uuid', timeout: described_class::HIGHEST_ROLE_LEASE_TIMEOUT)
+ expect(Gitlab::ExclusiveLease).not_to receive(:cancel).with(lease_key, 'uuid')
+
+ subject
+ end
+
+ it 'schedules a job in the future', :aggregate_failures do
+ allow_next_instance_of(Gitlab::ExclusiveLease) do |instance|
+ allow(instance).to receive(:try_obtain).and_return('uuid')
+ end
+
+ expect(UpdateHighestRoleWorker).to receive(:perform_in).with(described_class::HIGHEST_ROLE_JOB_DELAY, user_id).and_call_original
+
+ expect { subject }.to change(UpdateHighestRoleWorker.jobs, :size).by(1)
+ end
+ end
+
+ context 'when lease cannot be obtained', :clean_gitlab_redis_shared_state do
+ it 'only schedules one job' do
+ stub_exclusive_lease_taken(lease_key, timeout: described_class::HIGHEST_ROLE_LEASE_TIMEOUT)
+
+ expect { subject }.not_to change(UpdateHighestRoleWorker.jobs, :size)
+ end
+ end
+end
+
+# requires a subject and a user_id
+RSpec.shared_examples 'does not update the highest role' do
+ it 'does not obtain an exclusive lease' do
+ allow(Gitlab::ExclusiveLease).to receive(:new).and_call_original
+
+ lease = stub_exclusive_lease("update_highest_role:#{user_id}", 'uuid', timeout: described_class::HIGHEST_ROLE_LEASE_TIMEOUT)
+
+ expect(lease).not_to receive(:try_obtain)
+
+ subject
+ end
+end
diff --git a/spec/support/shared_examples/models/versioned_description_shared_examples.rb b/spec/support/shared_examples/models/versioned_description_shared_examples.rb
index 59124af19ec..8fee2494af7 100644
--- a/spec/support/shared_examples/models/versioned_description_shared_examples.rb
+++ b/spec/support/shared_examples/models/versioned_description_shared_examples.rb
@@ -9,54 +9,36 @@ RSpec.shared_examples 'versioned description' do
let(:factory_name) { described_class.name.underscore.to_sym }
let!(:model) { create(factory_name, description: 'Original description') }
- context 'when feature is enabled' do
+ context 'when description was changed' do
before do
- stub_feature_flags(save_description_versions: true)
+ model.update!(description: 'New description')
end
- context 'when description was changed' do
- before do
- model.update!(description: 'New description')
- end
-
- it 'saves the old and new description for the first update' do
- expect(model.description_versions.first.description).to eq('Original description')
- expect(model.description_versions.last.description).to eq('New description')
- end
-
- it 'only saves the new description for subsequent updates' do
- expect { model.update!(description: 'Another description') }.to change { model.description_versions.count }.by(1)
-
- expect(model.description_versions.last.description).to eq('Another description')
- end
+ it 'saves the old and new description for the first update' do
+ expect(model.description_versions.first.description).to eq('Original description')
+ expect(model.description_versions.last.description).to eq('New description')
+ end
- it 'sets the new description version to `saved_description_version`' do
- expect(model.saved_description_version).to eq(model.description_versions.last)
- end
+ it 'only saves the new description for subsequent updates' do
+ expect { model.update!(description: 'Another description') }.to change { model.description_versions.count }.by(1)
- it 'clears `saved_description_version` after another save that does not change description' do
- model.save!
+ expect(model.description_versions.last.description).to eq('Another description')
+ end
- expect(model.saved_description_version).to be_nil
- end
+ it 'sets the new description version to `saved_description_version`' do
+ expect(model.saved_description_version).to eq(model.description_versions.last)
end
- context 'when description was not changed' do
- it 'does not save any description version' do
- expect { model.save! }.not_to change { model.description_versions.count }
+ it 'clears `saved_description_version` after another save that does not change description' do
+ model.save!
- expect(model.saved_description_version).to be_nil
- end
+ expect(model.saved_description_version).to be_nil
end
end
- context 'when feature is disabled' do
- before do
- stub_feature_flags(save_description_versions: false)
- end
-
+ context 'when description was not changed' do
it 'does not save any description version' do
- expect { model.update!(description: 'New description') }.not_to change { model.description_versions.count }
+ expect { model.save! }.not_to change { model.description_versions.count }
expect(model.saved_description_version).to be_nil
end