diff options
Diffstat (limited to 'app/models/concerns')
| -rw-r--r-- | app/models/concerns/avatarable.rb | 11 | ||||
| -rw-r--r-- | app/models/concerns/cache_markdown_field.rb | 8 | ||||
| -rw-r--r-- | app/models/concerns/ci/artifactable.rb | 20 | ||||
| -rw-r--r-- | app/models/concerns/ci/contextable.rb | 20 | ||||
| -rw-r--r-- | app/models/concerns/ci/has_status.rb | 58 | ||||
| -rw-r--r-- | app/models/concerns/counter_attribute.rb | 143 | ||||
| -rw-r--r-- | app/models/concerns/file_store_mounter.rb | 21 | ||||
| -rw-r--r-- | app/models/concerns/has_wiki.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/issuable.rb | 4 | ||||
| -rw-r--r-- | app/models/concerns/milestoneable.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/relative_positioning.rb | 403 | ||||
| -rw-r--r-- | app/models/concerns/sha_attribute.rb | 13 | ||||
| -rw-r--r-- | app/models/concerns/time_trackable.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/triggerable_hooks.rb | 3 | ||||
| -rw-r--r-- | app/models/concerns/update_project_statistics.rb | 2 |
15 files changed, 564 insertions, 148 deletions
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index 60de20c3b31..0dd55ab67b5 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -3,6 +3,17 @@ module Avatarable extend ActiveSupport::Concern + ALLOWED_IMAGE_SCALER_WIDTHS = [ + 400, + 200, + 64, + 48, + 40, + 26, + 20, + 16 + ].freeze + included do prepend ShadowMethods include ObjectStorage::BackgroundMove diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 04eb4659469..49fc780f372 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -39,6 +39,10 @@ module CacheMarkdownField context[:markdown_engine] = :common_mark + if Feature.enabled?(:personal_snippet_reference_filters, context[:author]) + context[:user] = self.parent_user + end + context end @@ -132,6 +136,10 @@ module CacheMarkdownField end end + def parent_user + nil + end + included do cattr_reader :cached_markdown_fields do Gitlab::MarkdownCache::FieldData.new diff --git a/app/models/concerns/ci/artifactable.rb b/app/models/concerns/ci/artifactable.rb new file mode 100644 index 00000000000..54fb9021f2f --- /dev/null +++ b/app/models/concerns/ci/artifactable.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Ci + module Artifactable + extend ActiveSupport::Concern + + FILE_FORMAT_ADAPTERS = { + gzip: Gitlab::Ci::Build::Artifacts::Adapters::GzipStream, + raw: Gitlab::Ci::Build::Artifacts::Adapters::RawStream + }.freeze + + included do + enum file_format: { + raw: 1, + zip: 2, + gzip: 3 + }, _suffix: true + end + end +end diff --git a/app/models/concerns/ci/contextable.rb b/app/models/concerns/ci/contextable.rb index 10df5e1a8dc..c8b55e7b39f 100644 --- a/app/models/concerns/ci/contextable.rb +++ b/app/models/concerns/ci/contextable.rb @@ -9,7 +9,7 @@ module Ci ## # Variables in the environment name scope. # - def scoped_variables(environment: expanded_environment_name) + def scoped_variables(environment: expanded_environment_name, dependencies: true) Gitlab::Ci::Variables::Collection.new.tap do |variables| variables.concat(predefined_variables) variables.concat(project.predefined_variables) @@ -18,7 +18,7 @@ module Ci variables.concat(deployment_variables(environment: environment)) variables.concat(yaml_variables) variables.concat(user_variables) - variables.concat(dependency_variables) + variables.concat(dependency_variables) if dependencies variables.concat(secret_instance_variables) variables.concat(secret_group_variables) variables.concat(secret_project_variables(environment: environment)) @@ -45,6 +45,12 @@ module Ci end end + def simple_variables_without_dependencies + strong_memoize(:variables_without_dependencies) do + scoped_variables(environment: nil, dependencies: false).to_runner_variables + end + end + def user_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| break variables if user.blank? @@ -64,7 +70,7 @@ module Ci variables.append(key: 'CI_PIPELINE_TRIGGERED', value: 'true') if trigger_request variables.append(key: 'CI_NODE_INDEX', value: self.options[:instance].to_s) if self.options&.include?(:instance) - variables.append(key: 'CI_NODE_TOTAL', value: (self.options&.dig(:parallel) || 1).to_s) + variables.append(key: 'CI_NODE_TOTAL', value: ci_node_total_value.to_s) # legacy variables variables.append(key: 'CI_BUILD_NAME', value: name) @@ -96,5 +102,13 @@ module Ci def secret_project_variables(environment: persisted_environment) project.ci_variables_for(ref: git_ref, environment: environment) end + + private + + def ci_node_total_value + parallel = self.options&.dig(:parallel) + parallel = parallel.dig(:total) if parallel.is_a?(Hash) + parallel || 1 + end end end diff --git a/app/models/concerns/ci/has_status.rb b/app/models/concerns/ci/has_status.rb index c52807ec501..1cc2e8a51e3 100644 --- a/app/models/concerns/ci/has_status.rb +++ b/app/models/concerns/ci/has_status.rb @@ -20,60 +20,10 @@ module Ci UnknownStatusError = Class.new(StandardError) class_methods do - def legacy_status_sql - scope_relevant = respond_to?(:exclude_ignored) ? exclude_ignored : all - scope_warnings = respond_to?(:failed_but_allowed) ? failed_but_allowed : none - - builds = scope_relevant.select('count(*)').to_sql - created = scope_relevant.created.select('count(*)').to_sql - success = scope_relevant.success.select('count(*)').to_sql - manual = scope_relevant.manual.select('count(*)').to_sql - scheduled = scope_relevant.scheduled.select('count(*)').to_sql - preparing = scope_relevant.preparing.select('count(*)').to_sql - waiting_for_resource = scope_relevant.waiting_for_resource.select('count(*)').to_sql - pending = scope_relevant.pending.select('count(*)').to_sql - running = scope_relevant.running.select('count(*)').to_sql - skipped = scope_relevant.skipped.select('count(*)').to_sql - canceled = scope_relevant.canceled.select('count(*)').to_sql - warnings = scope_warnings.select('count(*) > 0').to_sql.presence || 'false' - - Arel.sql( - "(CASE - WHEN (#{builds})=(#{skipped}) AND (#{warnings}) THEN 'success' - WHEN (#{builds})=(#{skipped}) THEN 'skipped' - WHEN (#{builds})=(#{success}) THEN 'success' - WHEN (#{builds})=(#{created}) THEN 'created' - WHEN (#{builds})=(#{preparing}) THEN 'preparing' - WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'success' - WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled' - WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending' - WHEN (#{running})+(#{pending})>0 THEN 'running' - WHEN (#{waiting_for_resource})>0 THEN 'waiting_for_resource' - WHEN (#{manual})>0 THEN 'manual' - WHEN (#{scheduled})>0 THEN 'scheduled' - WHEN (#{preparing})>0 THEN 'preparing' - WHEN (#{created})>0 THEN 'running' - ELSE 'failed' - END)" - ) - end - - def legacy_status - all.pluck(legacy_status_sql).first - end - - # This method should not be used. - # This method performs expensive calculation of status: - # 1. By plucking all related objects, - # 2. Or executes expensive SQL query - def slow_composite_status(project:) - if ::Gitlab::Ci::Features.composite_status?(project) - Gitlab::Ci::Status::Composite - .new(all, with_allow_failure: columns_hash.key?('allow_failure')) - .status - else - legacy_status - end + def composite_status + Gitlab::Ci::Status::Composite + .new(all, with_allow_failure: columns_hash.key?('allow_failure')) + .status end def started_at diff --git a/app/models/concerns/counter_attribute.rb b/app/models/concerns/counter_attribute.rb new file mode 100644 index 00000000000..a5c7393e8f7 --- /dev/null +++ b/app/models/concerns/counter_attribute.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +# Add capabilities to increment a numeric model attribute efficiently by +# using Redis and flushing the increments asynchronously to the database +# after a period of time (10 minutes). +# When an attribute is incremented by a value, the increment is added +# to a Redis key. Then, FlushCounterIncrementsWorker will execute +# `flush_increments_to_database!` which removes increments from Redis for a +# given model attribute and updates the values in the database. +# +# @example: +# +# class ProjectStatistics +# include CounterAttribute +# +# counter_attribute :commit_count +# counter_attribute :storage_size +# end +# +# To increment the counter we can use the method: +# delayed_increment_counter(:commit_count, 3) +# +module CounterAttribute + extend ActiveSupport::Concern + extend AfterCommitQueue + include Gitlab::ExclusiveLeaseHelpers + + LUA_STEAL_INCREMENT_SCRIPT = <<~EOS.freeze + local increment_key, flushed_key = KEYS[1], KEYS[2] + local increment_value = redis.call("get", increment_key) or 0 + local flushed_value = redis.call("incrby", flushed_key, increment_value) + if flushed_value == 0 then + redis.call("del", increment_key, flushed_key) + else + redis.call("del", increment_key) + end + return flushed_value + EOS + + WORKER_DELAY = 10.minutes + WORKER_LOCK_TTL = 10.minutes + + class_methods do + def counter_attribute(attribute) + counter_attributes << attribute + end + + def counter_attributes + @counter_attributes ||= Set.new + end + end + + # This method must only be called by FlushCounterIncrementsWorker + # because it should run asynchronously and with exclusive lease. + # This will + # 1. temporarily move the pending increment for a given attribute + # to a relative "flushed" Redis key, delete the increment key and return + # the value. If new increments are performed at this point, the increment + # key is recreated as part of `delayed_increment_counter`. + # The "flushed" key is used to ensure that we can keep incrementing + # counters in Redis while flushing existing values. + # 2. then the value is used to update the counter in the database. + # 3. finally the "flushed" key is deleted. + def flush_increments_to_database!(attribute) + lock_key = counter_lock_key(attribute) + + with_exclusive_lease(lock_key) do + increment_key = counter_key(attribute) + flushed_key = counter_flushed_key(attribute) + increment_value = steal_increments(increment_key, flushed_key) + + next if increment_value == 0 + + transaction do + unsafe_update_counters(id, attribute => increment_value) + redis_state { |redis| redis.del(flushed_key) } + end + end + end + + def delayed_increment_counter(attribute, increment) + return if increment == 0 + + run_after_commit_or_now do + if counter_attribute_enabled?(attribute) + redis_state do |redis| + redis.incrby(counter_key(attribute), increment) + end + + FlushCounterIncrementsWorker.perform_in(WORKER_DELAY, self.class.name, self.id, attribute) + else + legacy_increment!(attribute, increment) + end + end + + true + end + + def counter_key(attribute) + "project:{#{project_id}}:counters:#{self.class}:#{id}:#{attribute}" + end + + def counter_flushed_key(attribute) + counter_key(attribute) + ':flushed' + end + + def counter_lock_key(attribute) + counter_key(attribute) + ':lock' + end + + private + + def counter_attribute_enabled?(attribute) + Feature.enabled?(:efficient_counter_attribute, project) && + self.class.counter_attributes.include?(attribute) + end + + def steal_increments(increment_key, flushed_key) + redis_state do |redis| + redis.eval(LUA_STEAL_INCREMENT_SCRIPT, keys: [increment_key, flushed_key]) + end + end + + def legacy_increment!(attribute, increment) + increment!(attribute, increment) + end + + def unsafe_update_counters(id, increments) + self.class.update_counters(id, increments) + end + + def redis_state(&block) + Gitlab::Redis::SharedState.with(&block) + end + + def with_exclusive_lease(lock_key) + in_lock(lock_key, ttl: WORKER_LOCK_TTL) do + yield + end + rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError + # a worker is already updating the counters + end +end diff --git a/app/models/concerns/file_store_mounter.rb b/app/models/concerns/file_store_mounter.rb new file mode 100644 index 00000000000..9d4463e5297 --- /dev/null +++ b/app/models/concerns/file_store_mounter.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module FileStoreMounter + extend ActiveSupport::Concern + + class_methods do + def mount_file_store_uploader(uploader) + mount_uploader(:file, uploader) + + after_save :update_file_store, if: :saved_change_to_file? + end + end + + private + + def update_file_store + # The file.object_store is set during `uploader.store!` + # which happens after object is inserted/updated + self.update_column(:file_store, file.object_store) + end +end diff --git a/app/models/concerns/has_wiki.rb b/app/models/concerns/has_wiki.rb index 4dd72216e77..3e7cb940a62 100644 --- a/app/models/concerns/has_wiki.rb +++ b/app/models/concerns/has_wiki.rb @@ -17,7 +17,7 @@ module HasWiki def wiki strong_memoize(:wiki) do - Wiki.for_container(self, self.owner) + Wiki.for_container(self, self.default_owner) end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 715cbd15d93..dd5aedbb760 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -61,11 +61,13 @@ module Issuable end end + has_many :note_authors, -> { distinct }, through: :notes, source: :author + has_many :label_links, as: :target, dependent: :destroy, inverse_of: :target # rubocop:disable Cop/ActiveRecordDependent has_many :labels, through: :label_links has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_one :metrics + has_one :metrics, inverse_of: model_name.singular.to_sym, autosave: true delegate :name, :email, diff --git a/app/models/concerns/milestoneable.rb b/app/models/concerns/milestoneable.rb index 8f8494a9678..ccb334343ff 100644 --- a/app/models/concerns/milestoneable.rb +++ b/app/models/concerns/milestoneable.rb @@ -15,7 +15,7 @@ module Milestoneable validate :milestone_is_valid scope :of_milestones, ->(ids) { where(milestone_id: ids) } - scope :any_milestone, -> { where('milestone_id IS NOT NULL') } + scope :any_milestone, -> { where.not(milestone_id: nil) } scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) } scope :without_particular_milestone, ->(title) { left_outer_joins(:milestone).where("milestones.title != ? OR milestone_id IS NULL", title) } scope :any_release, -> { joins_milestone_releases } diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 1d89a4497d9..d1f04609693 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -3,11 +3,15 @@ # This module makes it possible to handle items as a list, where the order of items can be easily altered # Requirements: # -# - Only works for ActiveRecord models -# - relative_position integer field must present on the model -# - This module uses GROUP BY: the model should have a parent relation, example: project -> issues, project is the parent relation (issues table has a parent_id column) +# The model must have the following named columns: +# - id: integer +# - relative_position: integer # -# Setup like this in the body of your class: +# The model must support a concept of siblings via a child->parent relationship, +# to enable rebalancing and `GROUP BY` in queries. +# - example: project -> issues, project is the parent relation (issues table has a parent_id column) +# +# Two class methods must be defined when including this concern: # # include RelativePositioning # @@ -24,53 +28,167 @@ module RelativePositioning extend ActiveSupport::Concern - MIN_POSITION = 0 - START_POSITION = Gitlab::Database::MAX_INT_VALUE / 2 + STEPS = 10 + IDEAL_DISTANCE = 2**(STEPS - 1) + 1 + + MIN_POSITION = Gitlab::Database::MIN_INT_VALUE + START_POSITION = 0 MAX_POSITION = Gitlab::Database::MAX_INT_VALUE - IDEAL_DISTANCE = 500 - class_methods do - def move_nulls_to_end(objects) - objects = objects.reject(&:relative_position) + MAX_GAP = IDEAL_DISTANCE * 2 + MIN_GAP = 2 - return if objects.empty? + NoSpaceLeft = Class.new(StandardError) - max_relative_position = objects.first.max_relative_position + class_methods do + def move_nulls_to_end(objects) + move_nulls(objects, at_end: true) + end - self.transaction do - objects.each do |object| - relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION) - object.relative_position = relative_position - max_relative_position = relative_position - object.save(touch: false) - end - end + def move_nulls_to_start(objects) + move_nulls(objects, at_end: false) end # This method takes two integer values (positions) and # calculates the position between them. The range is huge as - # the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time - # when we have enough space. If distance is less than IDEAL_DISTANCE, we are calculating an average number. + # the maximum integer value is 2147483647. + # + # We avoid open ranges by clamping the range to [MIN_POSITION, MAX_POSITION]. + # + # Then we handle one of three cases: + # - If the gap is too small, we raise NoSpaceLeft + # - If the gap is larger than MAX_GAP, we place the new position at most + # IDEAL_DISTANCE from the edge of the gap. + # - otherwise we place the new position at the midpoint. + # + # The new position will always satisfy: pos_before <= midpoint <= pos_after + # + # As a precondition, the gap between pos_before and pos_after MUST be >= 2. + # If the gap is too small, NoSpaceLeft is raised. + # + # This class method should only be called by instance methods of this module, which + # include handling for minimum gap size. + # + # @raises NoSpaceLeft + # @api private def position_between(pos_before, pos_after) pos_before ||= MIN_POSITION pos_after ||= MAX_POSITION pos_before, pos_after = [pos_before, pos_after].sort - halfway = (pos_after + pos_before) / 2 - distance_to_halfway = pos_after - halfway + gap_width = pos_after - pos_before + midpoint = [pos_after - 1, pos_before + (gap_width / 2)].min - if distance_to_halfway < IDEAL_DISTANCE - halfway - else + if gap_width < MIN_GAP + raise NoSpaceLeft + elsif gap_width > MAX_GAP if pos_before == MIN_POSITION pos_after - IDEAL_DISTANCE elsif pos_after == MAX_POSITION pos_before + IDEAL_DISTANCE else - halfway + midpoint + end + else + midpoint + end + end + + private + + # @api private + def gap_size(object, gaps:, at_end:, starting_from:) + total_width = IDEAL_DISTANCE * gaps + size = if at_end && starting_from + total_width >= MAX_POSITION + (MAX_POSITION - starting_from) / gaps + elsif !at_end && starting_from - total_width <= MIN_POSITION + (starting_from - MIN_POSITION) / gaps + else + IDEAL_DISTANCE + end + + # Shift max elements leftwards if there isn't enough space + return [size, starting_from] if size >= MIN_GAP + + order = at_end ? :desc : :asc + terminus = object + .send(:relative_siblings) # rubocop:disable GitlabSecurity/PublicSend + .where('relative_position IS NOT NULL') + .order(relative_position: order) + .first + + if at_end + terminus.move_sequence_before(true) + max_relative_position = terminus.reset.relative_position + [[(MAX_POSITION - max_relative_position) / gaps, IDEAL_DISTANCE].min, max_relative_position] + else + terminus.move_sequence_after(true) + min_relative_position = terminus.reset.relative_position + [[(min_relative_position - MIN_POSITION) / gaps, IDEAL_DISTANCE].min, min_relative_position] + end + end + + # @api private + # @param [Array<RelativePositioning>] objects The objects to give positions to. The relative + # order will be preserved (i.e. when this method returns, + # objects.first.relative_position < objects.last.relative_position) + # @param [Boolean] at_end: The placement. + # If `true`, then all objects with `null` positions are placed _after_ + # all siblings with positions. If `false`, all objects with `null` + # positions are placed _before_ all siblings with positions. + # @returns [Number] The number of moved records. + def move_nulls(objects, at_end:) + objects = objects.reject(&:relative_position) + return 0 if objects.empty? + + representative = objects.first + number_of_gaps = objects.size + 1 # 1 at left, one between each, and one at right + position = if at_end + representative.max_relative_position + else + representative.min_relative_position + end + + position ||= START_POSITION # If there are no positioned siblings, start from START_POSITION + + gap, position = gap_size(representative, gaps: number_of_gaps, at_end: at_end, starting_from: position) + + # Raise if we could not make enough space + raise NoSpaceLeft if gap < MIN_GAP + + indexed = objects.each_with_index.to_a + starting_from = at_end ? position : position - (gap * number_of_gaps) + + # Some classes are polymorphic, and not all siblings are in the same table. + by_model = indexed.group_by { |pair| pair.first.class } + + by_model.each do |model, pairs| + model.transaction do + pairs.each_slice(100) do |batch| + # These are known to be integers, one from the DB, and the other + # calculated by us, and thus safe to interpolate + values = batch.map do |obj, i| + pos = starting_from + gap * (i + 1) + obj.relative_position = pos + "(#{obj.id}, #{pos})" + end.join(', ') + + model.connection.exec_query(<<~SQL, "UPDATE #{model.table_name} positions") + WITH cte(cte_id, new_pos) AS ( + SELECT * + FROM (VALUES #{values}) as t (id, pos) + ) + UPDATE #{model.table_name} + SET relative_position = cte.new_pos + FROM cte + WHERE cte_id = id + SQL + end end end + + objects.size end end @@ -82,11 +200,12 @@ module RelativePositioning calculate_relative_position('MAX', &block) end - def prev_relative_position + def prev_relative_position(ignoring: nil) prev_pos = nil if self.relative_position prev_pos = max_relative_position do |relation| + relation = relation.id_not_in(ignoring.id) if ignoring.present? relation.where('relative_position < ?', self.relative_position) end end @@ -94,11 +213,12 @@ module RelativePositioning prev_pos end - def next_relative_position + def next_relative_position(ignoring: nil) next_pos = nil if self.relative_position next_pos = min_relative_position do |relation| + relation = relation.id_not_in(ignoring.id) if ignoring.present? relation.where('relative_position > ?', self.relative_position) end end @@ -110,24 +230,44 @@ module RelativePositioning return move_after(before) unless after return move_before(after) unless before - # If there is no place to insert an item we need to create one by moving the item - # before this and all preceding items until there is a gap before, after = after, before if after.relative_position < before.relative_position - if (after.relative_position - before.relative_position) < 2 - after.move_sequence_before - before.reset + + pos_left = before.relative_position + pos_right = after.relative_position + + if pos_right - pos_left < MIN_GAP + # Not enough room! Make space by shifting all previous elements to the left + # if there is enough space, else to the right + gap = after.send(:find_next_gap_before) # rubocop:disable GitlabSecurity/PublicSend + + if gap.present? + after.move_sequence_before(next_gap: gap) + pos_left -= optimum_delta_for_gap(gap) + else + before.move_sequence_after + pos_right = after.reset.relative_position + end end - self.relative_position = self.class.position_between(before.relative_position, after.relative_position) + new_position = self.class.position_between(pos_left, pos_right) + + self.relative_position = new_position end def move_after(before = self) pos_before = before.relative_position - pos_after = before.next_relative_position + pos_after = before.next_relative_position(ignoring: self) + + if pos_before == MAX_POSITION || gap_too_small?(pos_after, pos_before) + gap = before.send(:find_next_gap_after) # rubocop:disable GitlabSecurity/PublicSend - if pos_after && (pos_after - pos_before) < 2 - before.move_sequence_after - pos_after = before.next_relative_position + if gap.nil? + before.move_sequence_before(true) + pos_before = before.reset.relative_position + else + before.move_sequence_after(next_gap: gap) + pos_after += optimum_delta_for_gap(gap) + end end self.relative_position = self.class.position_between(pos_before, pos_after) @@ -135,80 +275,186 @@ module RelativePositioning def move_before(after = self) pos_after = after.relative_position - pos_before = after.prev_relative_position + pos_before = after.prev_relative_position(ignoring: self) + + if pos_after == MIN_POSITION || gap_too_small?(pos_before, pos_after) + gap = after.send(:find_next_gap_before) # rubocop:disable GitlabSecurity/PublicSend - if pos_before && (pos_after - pos_before) < 2 - after.move_sequence_before - pos_before = after.prev_relative_position + if gap.nil? + after.move_sequence_after(true) + pos_after = after.reset.relative_position + else + after.move_sequence_before(next_gap: gap) + pos_before -= optimum_delta_for_gap(gap) + end end self.relative_position = self.class.position_between(pos_before, pos_after) end def move_to_end - self.relative_position = self.class.position_between(max_relative_position || START_POSITION, MAX_POSITION) + max_pos = max_relative_position + + if max_pos.nil? + self.relative_position = START_POSITION + elsif gap_too_small?(max_pos, MAX_POSITION) + max = relative_siblings.order(Gitlab::Database.nulls_last_order('relative_position', 'DESC')).first + max.move_sequence_before(true) + max.reset + self.relative_position = self.class.position_between(max.relative_position, MAX_POSITION) + else + self.relative_position = self.class.position_between(max_pos, MAX_POSITION) + end end def move_to_start - self.relative_position = self.class.position_between(min_relative_position || START_POSITION, MIN_POSITION) + min_pos = min_relative_position + + if min_pos.nil? + self.relative_position = START_POSITION + elsif gap_too_small?(min_pos, MIN_POSITION) + min = relative_siblings.order(Gitlab::Database.nulls_last_order('relative_position', 'ASC')).first + min.move_sequence_after(true) + min.reset + self.relative_position = self.class.position_between(MIN_POSITION, min.relative_position) + else + self.relative_position = self.class.position_between(MIN_POSITION, min_pos) + end end # Moves the sequence before the current item to the middle of the next gap - # For example, we have 5 11 12 13 14 15 and the current item is 15 - # This moves the sequence 11 12 13 14 to 8 9 10 11 - def move_sequence_before - next_gap = find_next_gap_before + # For example, we have + # + # 5 . . . . . 11 12 13 14 [15] 16 . 17 + # ----------- + # + # This moves the sequence [11 12 13 14] to [8 9 10 11], so we have: + # + # 5 . . 8 9 10 11 . . . [15] 16 . 17 + # --------- + # + # Creating a gap to the left of the current item. We can understand this as + # dividing the 5 spaces between 5 and 11 into two smaller gaps of 2 and 3. + # + # If `include_self` is true, the current item will also be moved, creating a + # gap to the right of the current item: + # + # 5 . . 8 9 10 11 [14] . . . 16 . 17 + # -------------- + # + # As an optimization, the gap can be precalculated and passed to this method. + # + # @api private + # @raises NoSpaceLeft if the sequence cannot be moved + def move_sequence_before(include_self = false, next_gap: find_next_gap_before) + raise NoSpaceLeft unless next_gap.present? + delta = optimum_delta_for_gap(next_gap) - move_sequence(next_gap[:start], relative_position, -delta) + move_sequence(next_gap[:start], relative_position, -delta, include_self) end # Moves the sequence after the current item to the middle of the next gap - # For example, we have 11 12 13 14 15 21 and the current item is 11 - # This moves the sequence 12 13 14 15 to 15 16 17 18 - def move_sequence_after - next_gap = find_next_gap_after + # For example, we have: + # + # 8 . 10 [11] 12 13 14 15 . . . . . 21 + # ----------- + # + # This moves the sequence [12 13 14 15] to [15 16 17 18], so we have: + # + # 8 . 10 [11] . . . 15 16 17 18 . . 21 + # ----------- + # + # Creating a gap to the right of the current item. We can understand this as + # dividing the 5 spaces between 15 and 21 into two smaller gaps of 3 and 2. + # + # If `include_self` is true, the current item will also be moved, creating a + # gap to the left of the current item: + # + # 8 . 10 . . . [14] 15 16 17 18 . . 21 + # ---------------- + # + # As an optimization, the gap can be precalculated and passed to this method. + # + # @api private + # @raises NoSpaceLeft if the sequence cannot be moved + def move_sequence_after(include_self = false, next_gap: find_next_gap_after) + raise NoSpaceLeft unless next_gap.present? + delta = optimum_delta_for_gap(next_gap) - move_sequence(relative_position, next_gap[:start], delta) + move_sequence(relative_position, next_gap[:start], delta, include_self) end private - # Supposing that we have a sequence of items: 1 5 11 12 13 and the current item is 13 - # This would return: `{ start: 11, end: 5 }` + def gap_too_small?(pos_a, pos_b) + return false unless pos_a && pos_b + + (pos_a - pos_b).abs < MIN_GAP + end + + # Find the first suitable gap to the left of the current position. + # + # Satisfies the relations: + # - gap[:start] <= relative_position + # - abs(gap[:start] - gap[:end]) >= MIN_GAP + # - MIN_POSITION <= gap[:start] <= MAX_POSITION + # - MIN_POSITION <= gap[:end] <= MAX_POSITION + # + # Supposing that the current item is 13, and we have a sequence of items: + # + # 1 . . . 5 . . . . 11 12 [13] 14 . . 17 + # ^---------^ + # + # Then we return: `{ start: 11, end: 5 }` + # + # Here start refers to the end of the gap closest to the current item. def find_next_gap_before items_with_next_pos = scoped_items .select('relative_position AS pos, LEAD(relative_position) OVER (ORDER BY relative_position DESC) AS next_pos') .where('relative_position <= ?', relative_position) .order(relative_position: :desc) - find_next_gap(items_with_next_pos).tap do |gap| - gap[:end] ||= MIN_POSITION - end + find_next_gap(items_with_next_pos, MIN_POSITION) end - # Supposing that we have a sequence of items: 13 14 15 20 24 and the current item is 13 - # This would return: `{ start: 15, end: 20 }` + # Find the first suitable gap to the right of the current position. + # + # Satisfies the relations: + # - gap[:start] >= relative_position + # - abs(gap[:start] - gap[:end]) >= MIN_GAP + # - MIN_POSITION <= gap[:start] <= MAX_POSITION + # - MIN_POSITION <= gap[:end] <= MAX_POSITION + # + # Supposing the current item is 13, and that we have a sequence of items: + # + # 9 . . . [13] 14 15 . . . . 20 . . . 24 + # ^---------^ + # + # Then we return: `{ start: 15, end: 20 }` + # + # Here start refers to the end of the gap closest to the current item. def find_next_gap_after items_with_next_pos = scoped_items .select('relative_position AS pos, LEAD(relative_position) OVER (ORDER BY relative_position ASC) AS next_pos') .where('relative_position >= ?', relative_position) .order(:relative_position) - find_next_gap(items_with_next_pos).tap do |gap| - gap[:end] ||= MAX_POSITION - end + find_next_gap(items_with_next_pos, MAX_POSITION) end - def find_next_gap(items_with_next_pos) - gap = self.class.from(items_with_next_pos, :items_with_next_pos) - .where('ABS(pos - next_pos) > 1 OR next_pos IS NULL') - .limit(1) - .pluck(:pos, :next_pos) - .first + def find_next_gap(items_with_next_pos, end_is_nil) + gap = self.class + .from(items_with_next_pos, :items) + .where('next_pos IS NULL OR ABS(pos::bigint - next_pos::bigint) >= ?', MIN_GAP) + .limit(1) + .pluck(:pos, :next_pos) + .first + + return if gap.nil? || gap.first == end_is_nil - { start: gap[0], end: gap[1] } + { start: gap.first, end: gap.second || end_is_nil } end def optimum_delta_for_gap(gap) @@ -217,9 +463,10 @@ module RelativePositioning [delta, IDEAL_DISTANCE].min end - def move_sequence(start_pos, end_pos, delta) - scoped_items - .where.not(id: self.id) + def move_sequence(start_pos, end_pos, delta, include_self = false) + relation = include_self ? scoped_items : relative_siblings + + relation .where('relative_position BETWEEN ? AND ?', start_pos, end_pos) .update_all("relative_position = relative_position + #{delta}") end @@ -240,6 +487,10 @@ module RelativePositioning .first&.last end + def relative_siblings(relation = scoped_items) + relation.id_not_in(id) + end + def scoped_items self.class.relative_positioning_query_base(self) end diff --git a/app/models/concerns/sha_attribute.rb b/app/models/concerns/sha_attribute.rb index c807dcbf418..cbac6a210c7 100644 --- a/app/models/concerns/sha_attribute.rb +++ b/app/models/concerns/sha_attribute.rb @@ -7,7 +7,7 @@ module ShaAttribute def sha_attribute(name) return if ENV['STATIC_VERIFICATION'] - validate_binary_column_exists!(name) unless Rails.env.production? + validate_binary_column_exists!(name) if Rails.env.development? attribute(name, Gitlab::Database::ShaAttribute.new) end @@ -17,18 +17,11 @@ module ShaAttribute # See https://gitlab.com/gitlab-org/gitlab/merge_requests/5502 for more discussion def validate_binary_column_exists!(name) return unless database_exists? - - unless table_exists? - warn "WARNING: sha_attribute #{name.inspect} is invalid since the table doesn't exist - you may need to run database migrations" - return - end + return unless table_exists? column = columns.find { |c| c.name == name.to_s } - unless column - warn "WARNING: sha_attribute #{name.inspect} is invalid since the column doesn't exist - you may need to run database migrations" - return - end + return unless column unless column.type == :binary raise ArgumentError.new("sha_attribute #{name.inspect} is invalid since the column type is not :binary") diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index dddf96837b7..a1e7d06b1c1 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -26,6 +26,7 @@ module TimeTrackable # rubocop:disable Gitlab/ModuleWithInstanceVariables def spend_time(options) @time_spent = options[:duration] + @time_spent_note_id = options[:note_id] @time_spent_user = User.find(options[:user_id]) @spent_at = options[:spent_at] @original_total_time_spent = nil @@ -67,6 +68,7 @@ module TimeTrackable def add_or_subtract_spent_time timelogs.new( time_spent: time_spent, + note_id: @time_spent_note_id, user: @time_spent_user, spent_at: @spent_at ) diff --git a/app/models/concerns/triggerable_hooks.rb b/app/models/concerns/triggerable_hooks.rb index c52baa0524c..b64a9e4f70b 100644 --- a/app/models/concerns/triggerable_hooks.rb +++ b/app/models/concerns/triggerable_hooks.rb @@ -12,7 +12,8 @@ module TriggerableHooks merge_request_hooks: :merge_requests_events, job_hooks: :job_events, pipeline_hooks: :pipeline_events, - wiki_page_hooks: :wiki_page_events + wiki_page_hooks: :wiki_page_events, + deployment_hooks: :deployment_events }.freeze extend ActiveSupport::Concern diff --git a/app/models/concerns/update_project_statistics.rb b/app/models/concerns/update_project_statistics.rb index c0fa14d3369..a7028e18451 100644 --- a/app/models/concerns/update_project_statistics.rb +++ b/app/models/concerns/update_project_statistics.rb @@ -75,7 +75,7 @@ module UpdateProjectStatistics end def schedule_update_project_statistic(delta) - return if delta.zero? + return if delta == 0 return if project.nil? run_after_commit do |