diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/api/helpers/issues_helpers.rb | 12 | ||||
| -rw-r--r-- | lib/api/helpers/members_helpers.rb | 44 | ||||
| -rw-r--r-- | lib/api/issues.rb | 2 | ||||
| -rw-r--r-- | lib/api/members.rb | 32 | ||||
| -rw-r--r-- | lib/api/users.rb | 16 | ||||
| -rw-r--r-- | lib/backup/gitaly_backup.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/database/migration_helpers.rb | 17 | ||||
| -rw-r--r-- | lib/gitlab/database/migration_helpers/v2.rb | 38 | ||||
| -rw-r--r-- | lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb | 6 | ||||
| -rw-r--r-- | lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/database/rename_table_helpers.rb | 8 | ||||
| -rw-r--r-- | lib/gitlab/database/with_lock_retries.rb | 5 |
13 files changed, 138 insertions, 52 deletions
diff --git a/lib/api/helpers/issues_helpers.rb b/lib/api/helpers/issues_helpers.rb index 37ca55120d3..185a10a250c 100644 --- a/lib/api/helpers/issues_helpers.rb +++ b/lib/api/helpers/issues_helpers.rb @@ -34,7 +34,17 @@ module API end def self.sort_options - %w[created_at updated_at priority due_date relative_position label_priority milestone_due popularity] + %w[ + created_at + due_date + label_priority + milestone_due + popularity + priority + relative_position + title + updated_at + ] end def issue_finder(args = {}) diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index e72bbb931f0..1e89f9f97a2 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -50,24 +50,48 @@ module API GroupMembersFinder.new(group).execute end - def create_member(current_user, user, source, params) - source.add_user(user, params[:access_level], current_user: current_user, expires_at: params[:expires_at]) + def present_members(members) + present members, with: Entities::Member, current_user: current_user, show_seat_info: params[:show_seat_info] end - def track_areas_of_focus(member, areas_of_focus) - return unless areas_of_focus + def present_member_invitations(invitations) + present invitations, with: Entities::Invitation, current_user: current_user + end + + def add_single_member_by_user_id(create_service_params) + source = create_service_params[:source] + user_id = create_service_params[:user_ids] + user = User.find_by(id: user_id) # rubocop: disable CodeReuse/ActiveRecord + + if user + conflict!('Member already exists') if member_already_exists?(source, user_id) + + instance = ::Members::CreateService.new(current_user, create_service_params) + instance.execute + + not_allowed! if instance.membership_locked # This currently can only be reached in EE if group membership is locked - areas_of_focus.each do |area_of_focus| - Gitlab::Tracking.event(::Members::CreateService.name, 'area_of_focus', label: area_of_focus, property: member.id.to_s) + member = instance.single_member + render_validation_error!(member) if member.invalid? + + present_members(member) + else + not_found!('User') end end - def present_members(members) - present members, with: Entities::Member, current_user: current_user, show_seat_info: params[:show_seat_info] + def add_multiple_members?(user_id) + user_id.include?(',') end - def present_member_invitations(invitations) - present invitations, with: Entities::Invitation, current_user: current_user + def add_single_member?(user_id) + user_id.present? + end + + private + + def member_already_exists?(source, user_id) + source.members.exists?(user_id: user_id) # rubocop: disable CodeReuse/ActiveRecord end end end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 9805d52dc1c..39ce6e0b062 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -78,7 +78,7 @@ module API optional :state, type: String, values: %w[opened closed all], default: 'all', desc: 'Return opened, closed, or all issues' optional :order_by, type: String, values: Helpers::IssuesHelpers.sort_options, default: 'created_at', - desc: 'Return issues ordered by `created_at` or `updated_at` fields.' + desc: 'Return issues ordered by `created_at`, `due_date`, `label_priority`, `milestone_due`, `popularity`, `priority`, `relative_position`, `title`, or `updated_at` fields.' optional :sort, type: String, values: %w[asc desc], default: 'desc', desc: 'Return issues sorted in `asc` or `desc` order.' optional :due_date, type: String, values: %w[0 overdue week month next_month_and_previous_two_weeks] << '', diff --git a/lib/api/members.rb b/lib/api/members.rb index 7130635281a..332520ccd26 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -96,42 +96,22 @@ module API optional :invite_source, type: String, desc: 'Source that triggered the member creation process', default: 'members-api' optional :areas_of_focus, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Areas the inviter wants the member to focus upon' end - # rubocop: disable CodeReuse/ActiveRecord + post ":id/members" do ::Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/333434') source = find_source(source_type, params[:id]) authorize_admin_source!(source_type, source) - if params[:user_id].to_s.include?(',') - create_service_params = params.except(:user_id).merge({ user_ids: params[:user_id], source: source }) + user_id = params[:user_id].to_s + create_service_params = params.except(:user_id).merge({ user_ids: user_id, source: source }) + if add_multiple_members?(user_id) ::Members::CreateService.new(current_user, create_service_params).execute - elsif params[:user_id].present? - member = source.members.find_by(user_id: params[:user_id]) - conflict!('Member already exists') if member - - user = User.find_by_id(params[:user_id]) - not_found!('User') unless user - - member = create_member(current_user, user, source, params) - - if !member - not_allowed! # This currently can only be reached in EE - elsif member.valid? && member.persisted? - present_members(member) - Gitlab::Tracking.event(::Members::CreateService.name, - 'create_member', - label: params[:invite_source], - property: 'existing_user', - user: current_user) - track_areas_of_focus(member, params[:areas_of_focus]) - else - render_validation_error!(member) - end + elsif add_single_member?(user_id) + add_single_member_by_user_id(create_service_params) end end - # rubocop: enable CodeReuse/ActiveRecord desc 'Updates a member of a group or project.' do success Entities::Member diff --git a/lib/api/users.rb b/lib/api/users.rb index 586c9a77b2e..e3271b8b9b2 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -615,6 +615,22 @@ module API end end + desc 'Reject a pending user. Available only for admins.' + params do + requires :id, type: Integer, desc: 'The ID of the user' + end + post ':id/reject', feature_category: :authentication_and_authorization do + user = find_user_by_id(params) + + result = ::Users::RejectService.new(current_user).execute(user) + + if result[:success] + present user + else + render_api_error!(result[:message], result[:http_status]) + end + end + # rubocop: enable CodeReuse/ActiveRecord desc 'Deactivate an active user. Available only for admins.' params do diff --git a/lib/backup/gitaly_backup.rb b/lib/backup/gitaly_backup.rb index 7c7c07394d1..587640f1a1a 100644 --- a/lib/backup/gitaly_backup.rb +++ b/lib/backup/gitaly_backup.rb @@ -22,8 +22,8 @@ module Backup end args = [] - args += ['-parallel', @parallel.to_s] if type == :create && @parallel - args += ['-parallel-storage', @parallel_storage.to_s] if type == :create && @parallel_storage + args += ['-parallel', @parallel.to_s] if @parallel + args += ['-parallel-storage', @parallel_storage.to_s] if @parallel_storage @stdin, stdout, @thread = Open3.popen2(ENV, bin_path, command, '-path', backup_repos_path, *args) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index a4190514de2..a477f40697c 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -380,6 +380,8 @@ module Gitlab # The timings can be controlled via the +timing_configuration+ parameter. # If the lock was not acquired within the retry period, a last attempt is made without using +lock_timeout+. # + # Note this helper uses subtransactions when run inside an already open transaction. + # # ==== Examples # # Invoking without parameters # with_lock_retries do @@ -411,7 +413,8 @@ module Gitlab raise_on_exhaustion = !!kwargs.delete(:raise_on_exhaustion) merged_args = { klass: self.class, - logger: Gitlab::BackgroundMigration::Logger + logger: Gitlab::BackgroundMigration::Logger, + allow_savepoints: true }.merge(kwargs) Gitlab::Database::WithLockRetries.new(**merged_args) @@ -1376,13 +1379,11 @@ into similar problems in the future (e.g. when new tables are created). # validate - Whether to validate the constraint in this call # def add_check_constraint(table, check, constraint_name, validate: true) - validate_check_constraint_name!(constraint_name) - # Transactions would result in ALTER TABLE locks being held for the # duration of the transaction, defeating the purpose of this method. - if transaction_open? - raise 'add_check_constraint can not be run inside a transaction' - end + validate_not_in_transaction!(:add_check_constraint) + + validate_check_constraint_name!(constraint_name) if check_constraint_exists?(table, constraint_name) warning_message = <<~MESSAGE @@ -1427,6 +1428,10 @@ into similar problems in the future (e.g. when new tables are created). end def remove_check_constraint(table, constraint_name) + # This is technically not necessary, but aligned with add_check_constraint + # and allows us to continue use with_lock_retries here + validate_not_in_transaction!(:remove_check_constraint) + validate_check_constraint_name!(constraint_name) # DROP CONSTRAINT requires an EXCLUSIVE lock diff --git a/lib/gitlab/database/migration_helpers/v2.rb b/lib/gitlab/database/migration_helpers/v2.rb index f20a9b30fa7..79d69676c44 100644 --- a/lib/gitlab/database/migration_helpers/v2.rb +++ b/lib/gitlab/database/migration_helpers/v2.rb @@ -6,6 +6,44 @@ module Gitlab module V2 include Gitlab::Database::MigrationHelpers + # Executes the block with a retry mechanism that alters the +lock_timeout+ and +sleep_time+ between attempts. + # The timings can be controlled via the +timing_configuration+ parameter. + # If the lock was not acquired within the retry period, a last attempt is made without using +lock_timeout+. + # + # In order to retry the block, the method wraps the block into a transaction. + # Note it cannot be used inside an already open transaction and will raise an error in that case. + # + # ==== Examples + # # Invoking without parameters + # with_lock_retries do + # drop_table :my_table + # end + # + # # Invoking with custom +timing_configuration+ + # t = [ + # [1.second, 1.second], + # [2.seconds, 2.seconds] + # ] + # + # with_lock_retries(timing_configuration: t) do + # drop_table :my_table # this will be retried twice + # end + # + # # Disabling the retries using an environment variable + # > export DISABLE_LOCK_RETRIES=true + # + # with_lock_retries do + # drop_table :my_table # one invocation, it will not retry at all + # end + # + # ==== Parameters + # * +timing_configuration+ - [[ActiveSupport::Duration, ActiveSupport::Duration], ...] lock timeout for the block, sleep time before the next iteration, defaults to `Gitlab::Database::WithLockRetries::DEFAULT_TIMING_CONFIGURATION` + # * +logger+ - [Gitlab::JsonLogger] + # * +env+ - [Hash] custom environment hash, see the example with `DISABLE_LOCK_RETRIES` + def with_lock_retries(*args, **kwargs, &block) + super(*args, **kwargs.merge(allow_savepoints: false), &block) + end + # Renames a column without requiring downtime. # # Concurrent renames work by using database triggers to ensure both the diff --git a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb index f1aa7871245..bd8ed677d77 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb @@ -6,6 +6,8 @@ module Gitlab module ForeignKeyHelpers include ::Gitlab::Database::SchemaHelpers + ERROR_SCOPE = 'foreign keys' + # Adds a foreign key with only minimal locking on the tables involved. # # In concept it works similarly to add_concurrent_foreign_key, but we have @@ -32,6 +34,8 @@ module Gitlab # name - The name of the foreign key. # def add_concurrent_partitioned_foreign_key(source, target, column:, on_delete: :cascade, name: nil) + assert_not_in_transaction_block(scope: ERROR_SCOPE) + partition_options = { column: column, on_delete: on_delete, diff --git a/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb index c0cc97de276..c9a3b5caf79 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb @@ -7,6 +7,8 @@ module Gitlab include Gitlab::Database::MigrationHelpers include Gitlab::Database::SchemaHelpers + ERROR_SCOPE = 'index' + # Concurrently creates a new index on a partitioned table. In concept this works similarly to # `add_concurrent_index`, and won't block reads or writes on the table while the index is being built. # @@ -21,6 +23,8 @@ module Gitlab # # See Rails' `add_index` for more info on the available arguments. def add_concurrent_partitioned_index(table_name, column_names, options = {}) + assert_not_in_transaction_block(scope: ERROR_SCOPE) + raise ArgumentError, 'A name is required for indexes added to partitioned tables' unless options[:name] partitioned_table = find_partitioned_table(table_name) @@ -57,6 +61,8 @@ module Gitlab # # remove_concurrent_partitioned_index_by_name :users, 'index_name_goes_here' def remove_concurrent_partitioned_index_by_name(table_name, index_name) + assert_not_in_transaction_block(scope: ERROR_SCOPE) + find_partitioned_table(table_name) unless index_name_exists?(table_name, index_name) diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index 9ccbdc9930e..0dc9f92e4c8 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -431,7 +431,7 @@ module Gitlab replace_table = Gitlab::Database::Partitioning::ReplaceTable.new(original_table_name.to_s, replacement_table_name, replaced_table_name, primary_key_name) - with_lock_retries do + transaction do drop_sync_trigger(original_table_name) replace_table.perform do |sql| diff --git a/lib/gitlab/database/rename_table_helpers.rb b/lib/gitlab/database/rename_table_helpers.rb index 7f5af038c6d..e881c0e5455 100644 --- a/lib/gitlab/database/rename_table_helpers.rb +++ b/lib/gitlab/database/rename_table_helpers.rb @@ -4,27 +4,27 @@ module Gitlab module Database module RenameTableHelpers def rename_table_safely(old_table_name, new_table_name) - with_lock_retries do + transaction do rename_table(old_table_name, new_table_name) execute("CREATE VIEW #{old_table_name} AS SELECT * FROM #{new_table_name}") end end def undo_rename_table_safely(old_table_name, new_table_name) - with_lock_retries do + transaction do execute("DROP VIEW IF EXISTS #{old_table_name}") rename_table(new_table_name, old_table_name) end end def finalize_table_rename(old_table_name, new_table_name) - with_lock_retries do + transaction do execute("DROP VIEW IF EXISTS #{old_table_name}") end end def undo_finalize_table_rename(old_table_name, new_table_name) - with_lock_retries do + transaction do execute("CREATE VIEW #{old_table_name} AS SELECT * FROM #{new_table_name}") end end diff --git a/lib/gitlab/database/with_lock_retries.rb b/lib/gitlab/database/with_lock_retries.rb index 70acbeac9e1..e55390e679a 100644 --- a/lib/gitlab/database/with_lock_retries.rb +++ b/lib/gitlab/database/with_lock_retries.rb @@ -61,9 +61,10 @@ module Gitlab [10.seconds, 10.minutes] ].freeze - def initialize(logger: NULL_LOGGER, timing_configuration: DEFAULT_TIMING_CONFIGURATION, klass: nil, env: ENV) + def initialize(logger: NULL_LOGGER, allow_savepoints: true, timing_configuration: DEFAULT_TIMING_CONFIGURATION, klass: nil, env: ENV) @logger = logger @klass = klass + @allow_savepoints = allow_savepoints @timing_configuration = timing_configuration @env = env @current_iteration = 1 @@ -122,6 +123,8 @@ module Gitlab end def run_block_with_lock_timeout + raise "WithLockRetries should not run inside already open transaction" if ActiveRecord::Base.connection.transaction_open? && @allow_savepoints.blank? + ActiveRecord::Base.transaction(requires_new: true) do # rubocop:disable Performance/ActiveRecordSubtransactions execute("SET LOCAL lock_timeout TO '#{current_lock_timeout_in_ms}ms'") |