Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-07-20 06:08:21 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2021-07-20 06:08:21 +0300
commit93c966ae2abeb481cfa894351b74f954e406582d (patch)
tree6e1b7619e3d5e7acac52e7de1135dd67d0cb53d5
parent48641d6d58d6d5d19b8b2ffc66646c7e94d552a1 (diff)
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/assets/javascripts/boards/stores/actions.js7
-rw-r--r--app/controllers/admin/cohorts_controller.rb4
-rw-r--r--app/controllers/admin/usage_trends_controller.rb4
-rw-r--r--app/controllers/dashboard/todos_controller.rb1
-rw-r--r--app/controllers/projects/cycle_analytics_controller.rb4
-rw-r--r--app/controllers/projects/graphs_controller.rb4
-rw-r--r--app/controllers/projects/pipelines_controller.rb4
-rw-r--r--app/helpers/analytics/unique_visits_helper.rb31
-rw-r--r--app/models/concerns/atomic_internal_id.rb23
-rw-r--r--app/models/internal_id.rb171
-rw-r--r--config/feature_flags/development/generate_iids_without_explicit_locking.yml8
-rw-r--r--doc/topics/set_up_organization.md1
-rw-r--r--doc/user/workspace/img/1.1-Instance_overview.pngbin0 -> 30491 bytes
-rw-r--r--doc/user/workspace/img/1.2-Groups_overview.pngbin0 -> 24876 bytes
-rw-r--r--doc/user/workspace/img/1.3-Admin.pngbin0 -> 32534 bytes
-rw-r--r--doc/user/workspace/img/Admin_Settings.pngbin0 -> 519556 bytes
-rw-r--r--doc/user/workspace/index.md34
-rw-r--r--lib/gitlab/analytics/unique_visits.rb4
-rw-r--r--spec/frontend/boards/mock_data.js2
-rw-r--r--spec/frontend/boards/stores/actions_spec.js13
-rw-r--r--spec/helpers/analytics/unique_visits_helper_spec.rb34
-rw-r--r--spec/lib/gitlab/analytics/unique_visits_spec.rb81
-rw-r--r--spec/models/concerns/atomic_internal_id_spec.rb18
-rw-r--r--spec/models/internal_id_spec.rb279
-rw-r--r--spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb6
25 files changed, 376 insertions, 357 deletions
diff --git a/app/assets/javascripts/boards/stores/actions.js b/app/assets/javascripts/boards/stores/actions.js
index b52a9a71ae8..0f1b72146c9 100644
--- a/app/assets/javascripts/boards/stores/actions.js
+++ b/app/assets/javascripts/boards/stores/actions.js
@@ -173,8 +173,9 @@ export default {
addList: ({ commit, dispatch, getters }, list) => {
commit(types.RECEIVE_ADD_LIST_SUCCESS, updateListPosition(list));
+
dispatch('fetchItemsForList', {
- listId: getters.getListByTitle(ListTypeTitles.backlog).id,
+ listId: getters.getListByTitle(ListTypeTitles.backlog)?.id,
});
},
@@ -294,7 +295,7 @@ export default {
commit(types.REMOVE_LIST_FAILURE, listsBackup);
} else {
dispatch('fetchItemsForList', {
- listId: getters.getListByTitle(ListTypeTitles.backlog).id,
+ listId: getters.getListByTitle(ListTypeTitles.backlog)?.id,
});
}
},
@@ -305,6 +306,8 @@ export default {
},
fetchItemsForList: ({ state, commit }, { listId, fetchNext = false }) => {
+ if (!listId) return null;
+
if (!fetchNext) {
commit(types.RESET_ITEMS_FOR_LIST, listId);
}
diff --git a/app/controllers/admin/cohorts_controller.rb b/app/controllers/admin/cohorts_controller.rb
index 723f4145c43..e750b5c5ad4 100644
--- a/app/controllers/admin/cohorts_controller.rb
+++ b/app/controllers/admin/cohorts_controller.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class Admin::CohortsController < Admin::ApplicationController
- include Analytics::UniqueVisitsHelper
+ include RedisTracking
feature_category :devops_reports
@@ -21,6 +21,6 @@ class Admin::CohortsController < Admin::ApplicationController
end
def track_cohorts_visit
- track_visit('i_analytics_cohorts') if trackable_html_request?
+ track_unique_redis_hll_event('i_analytics_cohorts') if trackable_html_request?
end
end
diff --git a/app/controllers/admin/usage_trends_controller.rb b/app/controllers/admin/usage_trends_controller.rb
index 7073f71a1a8..0b315517594 100644
--- a/app/controllers/admin/usage_trends_controller.rb
+++ b/app/controllers/admin/usage_trends_controller.rb
@@ -1,9 +1,9 @@
# frozen_string_literal: true
class Admin::UsageTrendsController < Admin::ApplicationController
- include Analytics::UniqueVisitsHelper
+ include RedisTracking
- track_unique_visits :index, target_id: 'i_analytics_instance_statistics'
+ track_redis_hll_event :index, name: 'i_analytics_instance_statistics'
feature_category :devops_reports
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 782c8c293fd..25ac0af9731 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -3,7 +3,6 @@
class Dashboard::TodosController < Dashboard::ApplicationController
include ActionView::Helpers::NumberHelper
include PaginatedCollection
- include Analytics::UniqueVisitsHelper
before_action :authorize_read_project!, only: :index
before_action :authorize_read_group!, only: :index
diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb
index 3142ef0f404..db5ba51ee01 100644
--- a/app/controllers/projects/cycle_analytics_controller.rb
+++ b/app/controllers/projects/cycle_analytics_controller.rb
@@ -4,12 +4,12 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
include ActionView::Helpers::DateHelper
include ActionView::Helpers::TextHelper
include CycleAnalyticsParams
- include Analytics::UniqueVisitsHelper
include GracefulTimeoutHandling
+ include RedisTracking
before_action :authorize_read_cycle_analytics!
- track_unique_visits :show, target_id: 'p_analytics_valuestream'
+ track_redis_hll_event :show, name: 'p_analytics_valuestream'
feature_category :planning_analytics
diff --git a/app/controllers/projects/graphs_controller.rb b/app/controllers/projects/graphs_controller.rb
index ad39b317b31..7a7961c28bb 100644
--- a/app/controllers/projects/graphs_controller.rb
+++ b/app/controllers/projects/graphs_controller.rb
@@ -2,14 +2,14 @@
class Projects::GraphsController < Projects::ApplicationController
include ExtractsPath
- include Analytics::UniqueVisitsHelper
+ include RedisTracking
# Authorize
before_action :require_non_empty_project
before_action :assign_ref_vars
before_action :authorize_read_repository_graphs!
- track_unique_visits :charts, target_id: 'p_analytics_repo'
+ track_redis_hll_event :charts, name: 'p_analytics_repo'
feature_category :source_code_management
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 576492110d8..b4196878c4f 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -2,7 +2,7 @@
class Projects::PipelinesController < Projects::ApplicationController
include ::Gitlab::Utils::StrongMemoize
- include Analytics::UniqueVisitsHelper
+ include RedisTracking
before_action :disable_query_limiting, only: [:create, :retry]
before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables]
@@ -24,7 +24,7 @@ class Projects::PipelinesController < Projects::ApplicationController
around_action :allow_gitaly_ref_name_caching, only: [:index, :show]
- track_unique_visits :charts, target_id: 'p_analytics_pipelines'
+ track_redis_hll_event :charts, name: 'p_analytics_pipelines'
wrap_parameters Ci::Pipeline
diff --git a/app/helpers/analytics/unique_visits_helper.rb b/app/helpers/analytics/unique_visits_helper.rb
deleted file mode 100644
index d5abfccf9c0..00000000000
--- a/app/helpers/analytics/unique_visits_helper.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-module Analytics
- module UniqueVisitsHelper
- include Gitlab::Tracking::Helpers
- extend ActiveSupport::Concern
-
- def visitor_id
- return cookies[:visitor_id] if cookies[:visitor_id].present?
- return unless current_user
-
- uuid = SecureRandom.uuid
- cookies[:visitor_id] = { value: uuid, expires: 24.months }
- uuid
- end
-
- def track_visit(target_id)
- return unless visitor_id
-
- Gitlab::Analytics::UniqueVisits.new.track_visit(target_id, values: visitor_id)
- end
-
- class_methods do
- def track_unique_visits(controller_actions, target_id:)
- after_action only: controller_actions, if: -> { trackable_html_request? } do
- track_visit(target_id)
- end
- end
- end
- end
-end
diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb
index 80cf6260b0b..88f577c3e23 100644
--- a/app/models/concerns/atomic_internal_id.rb
+++ b/app/models/concerns/atomic_internal_id.rb
@@ -159,9 +159,8 @@ module AtomicInternalId
# Defines class methods:
#
# - with_{scope}_{column}_supply
- # This method can be used to allocate a block of IID values during
- # bulk operations (importing/copying, etc). This can be more efficient
- # than creating instances one-by-one.
+ # This method can be used to allocate a stream of IID values during
+ # bulk operations (importing/copying, etc).
#
# Pass in a block that receives a `Supply` instance. To allocate a new
# IID value, call `Supply#next_value`.
@@ -181,14 +180,8 @@ module AtomicInternalId
scope_attrs = ::AtomicInternalId.scope_attrs(scope_value)
usage = ::AtomicInternalId.scope_usage(self)
- generator = InternalId::InternalIdGenerator.new(subject, scope_attrs, usage, init)
-
- generator.with_lock do
- supply = Supply.new(generator.record.last_value)
- block.call(supply)
- ensure
- generator.track_greatest(supply.current_value) if supply
- end
+ supply = Supply.new(-> { InternalId.generate_next(subject, scope_attrs, usage, init) })
+ block.call(supply)
end
end
end
@@ -236,14 +229,14 @@ module AtomicInternalId
end
class Supply
- attr_reader :current_value
+ attr_reader :generator
- def initialize(start_value)
- @current_value = start_value
+ def initialize(generator)
+ @generator = generator
end
def next_value
- @current_value += 1
+ @generator.call
end
end
end
diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb
index b56bac58705..f114094d69c 100644
--- a/app/models/internal_id.rb
+++ b/app/models/internal_id.rb
@@ -16,7 +16,7 @@
# * Add `usage` value to enum
# * (Optionally) add columns to `internal_ids` if needed for scope.
class InternalId < ApplicationRecord
- include Gitlab::Utils::StrongMemoize
+ extend Gitlab::Utils::StrongMemoize
belongs_to :project
belongs_to :namespace
@@ -25,6 +25,10 @@ class InternalId < ApplicationRecord
validates :usage, presence: true
+ scope :filter_by, -> (scope, usage) do
+ where(**scope, usage: usage)
+ end
+
# Increments #last_value and saves the record
#
# The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
@@ -53,18 +57,15 @@ class InternalId < ApplicationRecord
class << self
def track_greatest(subject, scope, usage, new_value, init)
- InternalIdGenerator.new(subject, scope, usage, init)
- .track_greatest(new_value)
+ build_generator(subject, scope, usage, init).track_greatest(new_value)
end
def generate_next(subject, scope, usage, init)
- InternalIdGenerator.new(subject, scope, usage, init)
- .generate
+ build_generator(subject, scope, usage, init).generate
end
def reset(subject, scope, usage, value)
- InternalIdGenerator.new(subject, scope, usage)
- .reset(value)
+ build_generator(subject, scope, usage).reset(value)
end
# Flushing records is generally safe in a sense that those
@@ -77,11 +78,36 @@ class InternalId < ApplicationRecord
where(filter).delete_all
end
+
+ def internal_id_transactions_increment(operation:, usage:)
+ self.internal_id_transactions_total.increment(
+ operation: operation,
+ usage: usage.to_s,
+ in_transaction: ActiveRecord::Base.connection.transaction_open?.to_s
+ )
+ end
+
+ def internal_id_transactions_total
+ strong_memoize(:internal_id_transactions_total) do
+ name = :gitlab_internal_id_transactions_total
+ comment = 'Counts all the internal ids happening within transaction'
+
+ Gitlab::Metrics.counter(name, comment)
+ end
+ end
+
+ private
+
+ def build_generator(subject, scope, usage, init = nil)
+ if Feature.enabled?(:generate_iids_without_explicit_locking)
+ ImplicitlyLockingInternalIdGenerator.new(subject, scope, usage, init)
+ else
+ InternalIdGenerator.new(subject, scope, usage, init)
+ end
+ end
end
class InternalIdGenerator
- extend Gitlab::Utils::StrongMemoize
-
# Generate next internal id for a given scope and usage.
#
# For currently supported usages, see #usage enum.
@@ -117,7 +143,7 @@ class InternalId < ApplicationRecord
# init: Block that gets called to initialize InternalId record if not present
# Make sure to not throw exceptions in the absence of records (if this is expected).
def generate
- self.class.internal_id_transactions_increment(operation: :generate, usage: usage)
+ InternalId.internal_id_transactions_increment(operation: :generate, usage: usage)
subject.transaction do
# Create a record in internal_ids if one does not yet exist
@@ -134,7 +160,7 @@ class InternalId < ApplicationRecord
def reset(value)
return false unless value
- self.class.internal_id_transactions_increment(operation: :reset, usage: usage)
+ InternalId.internal_id_transactions_increment(operation: :reset, usage: usage)
updated =
InternalId
@@ -149,8 +175,9 @@ class InternalId < ApplicationRecord
# and set its new_value if it is higher than the current last_value
#
# Note this will acquire a ROW SHARE lock on the InternalId record
+
def track_greatest(new_value)
- self.class.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
+ InternalId.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
subject.transaction do
record.track_greatest_and_save!(new_value)
@@ -162,7 +189,7 @@ class InternalId < ApplicationRecord
end
def with_lock(&block)
- self.class.internal_id_transactions_increment(operation: :with_lock, usage: usage)
+ InternalId.internal_id_transactions_increment(operation: :with_lock, usage: usage)
record.with_lock(&block)
end
@@ -199,22 +226,118 @@ class InternalId < ApplicationRecord
rescue ActiveRecord::RecordNotUnique
lookup
end
+ end
- def self.internal_id_transactions_increment(operation:, usage:)
- self.internal_id_transactions_total.increment(
- operation: operation,
- usage: usage.to_s,
- in_transaction: ActiveRecord::Base.connection.transaction_open?.to_s
- )
+ class ImplicitlyLockingInternalIdGenerator
+ # Generate next internal id for a given scope and usage.
+ #
+ # For currently supported usages, see #usage enum.
+ #
+ # The method implements a locking scheme that has the following properties:
+ # 1) Generated sequence of internal ids is unique per (scope and usage)
+ # 2) The method is thread-safe and may be used in concurrent threads/processes.
+ # 3) The generated sequence is gapless.
+ # 4) In the absence of a record in the internal_ids table, one will be created
+ # and last_value will be calculated on the fly.
+ #
+ # subject: The instance or class we're generating an internal id for.
+ # scope: Attributes that define the scope for id generation.
+ # Valid keys are `project/project_id` and `namespace/namespace_id`.
+ # usage: Symbol to define the usage of the internal id, see InternalId.usages
+ # init: Proc that accepts the subject and the scope and returns Integer|NilClass
+ attr_reader :subject, :scope, :scope_attrs, :usage, :init
+
+ def initialize(subject, scope, usage, init = nil)
+ @subject = subject
+ @scope = scope
+ @usage = usage
+ @init = init
+
+ raise ArgumentError, 'Scope is not well-defined, need at least one column for scope (given: 0)' if scope.empty?
+
+ unless InternalId.usages.has_key?(usage.to_s)
+ raise ArgumentError, "Usage '#{usage}' is unknown. Supported values are #{InternalId.usages.keys} from InternalId.usages"
+ end
end
- def self.internal_id_transactions_total
- strong_memoize(:internal_id_transactions_total) do
- name = :gitlab_internal_id_transactions_total
- comment = 'Counts all the internal ids happening within transaction'
+ # Generates next internal id and returns it
+ # init: Block that gets called to initialize InternalId record if not present
+ # Make sure to not throw exceptions in the absence of records (if this is expected).
+ def generate
+ InternalId.internal_id_transactions_increment(operation: :generate, usage: usage)
- Gitlab::Metrics.counter(name, comment)
+ next_iid = update_record!(subject, scope, usage, arel_table[:last_value] + 1)
+
+ return next_iid if next_iid
+
+ create_record!(subject, scope, usage, init) do |iid|
+ iid.last_value += 1
end
+ rescue ActiveRecord::RecordNotUnique
+ retry
+ end
+
+ # Reset tries to rewind to `value-1`. This will only succeed,
+ # if `value` stored in database is equal to `last_value`.
+ # value: The expected last_value to decrement
+ def reset(value)
+ return false unless value
+
+ InternalId.internal_id_transactions_increment(operation: :reset, usage: usage)
+
+ iid = update_record!(subject, scope.merge(last_value: value), usage, arel_table[:last_value] - 1)
+ iid == value - 1
+ end
+
+ # Create a record in internal_ids if one does not yet exist
+ # and set its new_value if it is higher than the current last_value
+ def track_greatest(new_value)
+ InternalId.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
+
+ function = Arel::Nodes::NamedFunction.new('GREATEST', [
+ arel_table[:last_value],
+ new_value.to_i
+ ])
+
+ next_iid = update_record!(subject, scope, usage, function)
+ return next_iid if next_iid
+
+ create_record!(subject, scope, usage, init) do |object|
+ object.last_value = [object.last_value, new_value].max
+ end
+ rescue ActiveRecord::RecordNotUnique
+ retry
+ end
+
+ private
+
+ def update_record!(subject, scope, usage, new_value)
+ stmt = Arel::UpdateManager.new
+ stmt.table(arel_table)
+ stmt.set(arel_table[:last_value] => new_value)
+ stmt.wheres = InternalId.filter_by(scope, usage).arel.constraints
+
+ ActiveRecord::Base.connection.insert(stmt, 'Update InternalId', 'last_value')
+ end
+
+ def create_record!(subject, scope, usage, init)
+ raise ArgumentError, 'Cannot initialize without init!' unless init
+
+ instance = subject.is_a?(::Class) ? nil : subject
+
+ subject.transaction(requires_new: true) do
+ last_value = init.call(instance, scope) || 0
+
+ internal_id = InternalId.create!(**scope, usage: usage, last_value: last_value) do |subject|
+ yield subject if block_given?
+ end
+
+ internal_id.last_value
+ end
+ end
+
+ def arel_table
+ InternalId.arel_table
end
end
end
diff --git a/config/feature_flags/development/generate_iids_without_explicit_locking.yml b/config/feature_flags/development/generate_iids_without_explicit_locking.yml
new file mode 100644
index 00000000000..d2a7aeb8619
--- /dev/null
+++ b/config/feature_flags/development/generate_iids_without_explicit_locking.yml
@@ -0,0 +1,8 @@
+---
+name: generate_iids_without_explicit_locking
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65590
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335431
+milestone: '14.2'
+type: development
+group: group::database
+default_enabled: false
diff --git a/doc/topics/set_up_organization.md b/doc/topics/set_up_organization.md
index 2b263143b46..3758435d297 100644
--- a/doc/topics/set_up_organization.md
+++ b/doc/topics/set_up_organization.md
@@ -10,6 +10,7 @@ Configure your organization and its users. Determine user roles
and give everyone access to the projects they need.
- [Members](../user/project/members/index.md)
+- [Workspace](../user/workspace/index.md) _(Coming soon)_
- [Groups](../user/group/index.md)
- [User account options](../user/profile/index.md)
- [SSH keys](../ssh/index.md)
diff --git a/doc/user/workspace/img/1.1-Instance_overview.png b/doc/user/workspace/img/1.1-Instance_overview.png
new file mode 100644
index 00000000000..7612cc7c092
--- /dev/null
+++ b/doc/user/workspace/img/1.1-Instance_overview.png
Binary files differ
diff --git a/doc/user/workspace/img/1.2-Groups_overview.png b/doc/user/workspace/img/1.2-Groups_overview.png
new file mode 100644
index 00000000000..b4f034bba16
--- /dev/null
+++ b/doc/user/workspace/img/1.2-Groups_overview.png
Binary files differ
diff --git a/doc/user/workspace/img/1.3-Admin.png b/doc/user/workspace/img/1.3-Admin.png
new file mode 100644
index 00000000000..018ed8a1bfc
--- /dev/null
+++ b/doc/user/workspace/img/1.3-Admin.png
Binary files differ
diff --git a/doc/user/workspace/img/Admin_Settings.png b/doc/user/workspace/img/Admin_Settings.png
new file mode 100644
index 00000000000..5c4656ff342
--- /dev/null
+++ b/doc/user/workspace/img/Admin_Settings.png
Binary files differ
diff --git a/doc/user/workspace/index.md b/doc/user/workspace/index.md
new file mode 100644
index 00000000000..f098cef6053
--- /dev/null
+++ b/doc/user/workspace/index.md
@@ -0,0 +1,34 @@
+---
+stage: Manage
+group: Access
+info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
+---
+
+# Workspace
+
+Workspace will be the top-level [namespace](../group/index.md#namespaces) for you to manage
+everything GitLab, including:
+
+- Defining and applying settings to all of your groups, subgroups, and projects.
+- Aggregating data from all your groups, subgroups, and projects.
+
+Workspace will take many of the features from the
+[Admin Area](../admin_area/index.md), and there will be one workspace per:
+
+- Instance, for self-managed instances.
+- Namespace, for GitLab.com.
+
+NOTE:
+Workspace is currently in development.
+
+## Concept previews
+
+The following provide a preview to the Workspace concept.
+
+![Workspace Overview](img/1.1-Instance_overview.png)
+
+![Groups Overview](img/1.2-Groups_overview.png)
+
+![Admin Overview](img/1.3-Admin.png)
+
+![Admin Overview](img/Admin_Settings.png)
diff --git a/lib/gitlab/analytics/unique_visits.rb b/lib/gitlab/analytics/unique_visits.rb
index 723486231b1..3546a7e3ddb 100644
--- a/lib/gitlab/analytics/unique_visits.rb
+++ b/lib/gitlab/analytics/unique_visits.rb
@@ -3,10 +3,6 @@
module Gitlab
module Analytics
class UniqueVisits
- def track_visit(*args, **kwargs)
- Gitlab::UsageDataCounters::HLLRedisCounter.track_event(*args, **kwargs)
- end
-
# Returns number of unique visitors for given targets in given time frame
#
# @param [String, Array[<String>]] targets ids of targets to count visits on. Special case for :any
diff --git a/spec/frontend/boards/mock_data.js b/spec/frontend/boards/mock_data.js
index 73372eafb89..6ac4db8cdaa 100644
--- a/spec/frontend/boards/mock_data.js
+++ b/spec/frontend/boards/mock_data.js
@@ -291,7 +291,7 @@ export const setMockEndpoints = (opts = {}) => {
export const mockList = {
id: 'gid://gitlab/List/1',
- title: 'Backlog',
+ title: 'Open',
position: -Infinity,
listType: 'backlog',
collapsed: false,
diff --git a/spec/frontend/boards/stores/actions_spec.js b/spec/frontend/boards/stores/actions_spec.js
index ebed33e5b34..5e16e389ddc 100644
--- a/spec/frontend/boards/stores/actions_spec.js
+++ b/spec/frontend/boards/stores/actions_spec.js
@@ -715,6 +715,19 @@ describe('fetchItemsForList', () => {
[listId]: pageInfo,
};
+ describe('when list id is undefined', () => {
+ it('does not call the query', async () => {
+ jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse);
+
+ await actions.fetchItemsForList(
+ { state, getters: () => {}, commit: () => {} },
+ { listId: undefined },
+ );
+
+ expect(gqlClient.query).toHaveBeenCalledTimes(0);
+ });
+ });
+
it('should commit mutations REQUEST_ITEMS_FOR_LIST and RECEIVE_ITEMS_FOR_LIST_SUCCESS on success', (done) => {
jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse);
diff --git a/spec/helpers/analytics/unique_visits_helper_spec.rb b/spec/helpers/analytics/unique_visits_helper_spec.rb
deleted file mode 100644
index b4b370c169d..00000000000
--- a/spec/helpers/analytics/unique_visits_helper_spec.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-require "spec_helper"
-
-RSpec.describe Analytics::UniqueVisitsHelper do
- include Devise::Test::ControllerHelpers
-
- describe '#track_visit' do
- let(:target_id) { 'p_analytics_valuestream' }
- let(:current_user) { create(:user) }
-
- it 'does not track visit if user is not logged in' do
- expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
-
- helper.track_visit(target_id)
- end
-
- it 'tracks visit if user is logged in' do
- sign_in(current_user)
-
- expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit)
-
- helper.track_visit(target_id)
- end
-
- it 'tracks visit if user is not logged in, but has the cookie already' do
- helper.request.cookies[:visitor_id] = { value: SecureRandom.uuid, expires: 24.months }
-
- expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit)
-
- helper.track_visit(target_id)
- end
- end
-end
diff --git a/spec/lib/gitlab/analytics/unique_visits_spec.rb b/spec/lib/gitlab/analytics/unique_visits_spec.rb
deleted file mode 100644
index f4d5c0b1eca..00000000000
--- a/spec/lib/gitlab/analytics/unique_visits_spec.rb
+++ /dev/null
@@ -1,81 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state do
- let(:unique_visits) { Gitlab::Analytics::UniqueVisits.new }
- let(:target1_id) { 'g_analytics_contribution' }
- let(:target2_id) { 'g_analytics_insights' }
- let(:target3_id) { 'g_analytics_issues' }
- let(:target4_id) { 'g_compliance_dashboard' }
- let(:target5_id) { 'i_compliance_credential_inventory' }
- let(:visitor1_id) { 'dfb9d2d2-f56c-4c77-8aeb-6cddc4a1f857' }
- let(:visitor2_id) { '1dd9afb2-a3ee-4de1-8ae3-a405579c8584' }
- let(:visitor3_id) { '34rfjuuy-ce56-sa35-ds34-dfer567dfrf2' }
-
- around do |example|
- # We need to freeze to a reference time
- # because visits are grouped by the week number in the year
- # Without freezing the time, the test may behave inconsistently
- # depending on which day of the week test is run.
- reference_time = Time.utc(2020, 6, 1)
- travel_to(reference_time) { example.run }
- end
-
- describe '#track_visit' do
- it 'tracks the unique weekly visits for targets' do
- unique_visits.track_visit(target1_id, values: visitor1_id, time: 7.days.ago)
- unique_visits.track_visit(target1_id, values: visitor1_id, time: 7.days.ago)
- unique_visits.track_visit(target1_id, values: visitor2_id, time: 7.days.ago)
-
- unique_visits.track_visit(target2_id, values: visitor2_id, time: 7.days.ago)
- unique_visits.track_visit(target2_id, values: visitor1_id, time: 8.days.ago)
- unique_visits.track_visit(target2_id, values: visitor1_id, time: 15.days.ago)
-
- unique_visits.track_visit(target4_id, values: visitor3_id, time: 7.days.ago)
-
- unique_visits.track_visit(target5_id, values: visitor3_id, time: 15.days.ago)
- unique_visits.track_visit(target5_id, values: visitor2_id, time: 15.days.ago)
-
- expect(unique_visits.unique_visits_for(targets: target1_id)).to eq(2)
- expect(unique_visits.unique_visits_for(targets: target2_id)).to eq(1)
- expect(unique_visits.unique_visits_for(targets: target4_id)).to eq(1)
-
- expect(unique_visits.unique_visits_for(targets: target2_id, start_date: 15.days.ago)).to eq(1)
-
- expect(unique_visits.unique_visits_for(targets: target3_id)).to eq(0)
-
- expect(unique_visits.unique_visits_for(targets: target5_id, start_date: 15.days.ago)).to eq(2)
-
- expect(unique_visits.unique_visits_for(targets: :analytics)).to eq(2)
- expect(unique_visits.unique_visits_for(targets: :analytics, start_date: 15.days.ago)).to eq(1)
- expect(unique_visits.unique_visits_for(targets: :analytics, start_date: 30.days.ago)).to eq(0)
-
- expect(unique_visits.unique_visits_for(targets: :analytics, start_date: 4.weeks.ago, end_date: Date.current)).to eq(2)
-
- expect(unique_visits.unique_visits_for(targets: :compliance)).to eq(1)
- expect(unique_visits.unique_visits_for(targets: :compliance, start_date: 15.days.ago)).to eq(2)
- expect(unique_visits.unique_visits_for(targets: :compliance, start_date: 30.days.ago)).to eq(0)
-
- expect(unique_visits.unique_visits_for(targets: :compliance, start_date: 4.weeks.ago, end_date: Date.current)).to eq(2)
- end
-
- it 'sets the keys in Redis to expire automatically after 12 weeks' do
- unique_visits.track_visit(target1_id, values: visitor1_id)
-
- Gitlab::Redis::SharedState.with do |redis|
- redis.scan_each(match: "{#{target1_id}}-*").each do |key|
- expect(redis.ttl(key)).to be_within(5.seconds).of(12.weeks)
- end
- end
- end
-
- it 'raises an error if an invalid target id is given' do
- invalid_target_id = "x_invalid"
-
- expect do
- unique_visits.track_visit(invalid_target_id, values: visitor1_id)
- end.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent)
- end
- end
-end
diff --git a/spec/models/concerns/atomic_internal_id_spec.rb b/spec/models/concerns/atomic_internal_id_spec.rb
index 35b0f107676..b803e699b25 100644
--- a/spec/models/concerns/atomic_internal_id_spec.rb
+++ b/spec/models/concerns/atomic_internal_id_spec.rb
@@ -240,18 +240,12 @@ RSpec.describe AtomicInternalId do
end
describe '.with_project_iid_supply' do
- let(:iid) { 100 }
-
- it 'wraps generate and track_greatest in a concurrency-safe lock' do
- expect_next_instance_of(InternalId::InternalIdGenerator) do |g|
- expect(g).to receive(:with_lock).and_call_original
- expect(g.record).to receive(:last_value).and_return(iid)
- expect(g).to receive(:track_greatest).with(iid + 4)
- end
-
- ::Milestone.with_project_iid_supply(milestone.project) do |supply|
- 4.times { supply.next_value }
- end
+ it 'supplies a stream of iid values' do
+ expect do
+ ::Milestone.with_project_iid_supply(milestone.project) do |supply|
+ 4.times { supply.next_value }
+ end
+ end.to change { InternalId.find_by(project: milestone.project, usage: :milestones)&.last_value.to_i }.by(4)
end
end
end
diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb
index 390d1552c16..696b5b48cbf 100644
--- a/spec/models/internal_id_spec.rb
+++ b/spec/models/internal_id_spec.rb
@@ -39,216 +39,217 @@ RSpec.describe InternalId do
end
end
- describe '.generate_next' do
- subject { described_class.generate_next(id_subject, scope, usage, init) }
+ shared_examples_for 'a monotonically increasing id generator' do
+ describe '.generate_next' do
+ subject { described_class.generate_next(id_subject, scope, usage, init) }
- context 'in the absence of a record' do
- it 'creates a record if not yet present' do
- expect { subject }.to change { described_class.count }.from(0).to(1)
- end
+ context 'in the absence of a record' do
+ it 'creates a record if not yet present' do
+ expect { subject }.to change { described_class.count }.from(0).to(1)
+ end
- it 'stores record attributes' do
- subject
+ it 'stores record attributes' do
+ subject
- described_class.first.tap do |record|
- expect(record.project).to eq(project)
- expect(record.usage).to eq(usage.to_s)
+ described_class.first.tap do |record|
+ expect(record.project).to eq(project)
+ expect(record.usage).to eq(usage.to_s)
+ end
end
- end
- context 'with existing issues' do
- before do
- create_list(:issue, 2, project: project)
- described_class.delete_all
- end
+ context 'with existing issues' do
+ before do
+ create_list(:issue, 2, project: project)
+ described_class.delete_all
+ end
- it 'calculates last_value values automatically' do
- expect(subject).to eq(project.issues.size + 1)
+ it 'calculates last_value values automatically' do
+ expect(subject).to eq(project.issues.size + 1)
+ end
end
end
- context 'with concurrent inserts on table' do
- it 'looks up the record if it was created concurrently' do
- args = { **scope, usage: described_class.usages[usage.to_s] }
- record = double
- expect(described_class).to receive(:find_by).with(args).and_return(nil) # first call, record not present
- expect(described_class).to receive(:find_by).with(args).and_return(record) # second call, record was created by another process
- expect(described_class).to receive(:create!).and_raise(ActiveRecord::RecordNotUnique, 'record not unique')
- expect(record).to receive(:increment_and_save!)
-
- subject
+ it 'generates a strictly monotone, gapless sequence' do
+ seq = Array.new(10).map do
+ described_class.generate_next(issue, scope, usage, init)
end
- end
- end
+ normalized = seq.map { |i| i - seq.min }
- it 'generates a strictly monotone, gapless sequence' do
- seq = Array.new(10).map do
- described_class.generate_next(issue, scope, usage, init)
+ expect(normalized).to eq((0..seq.size - 1).to_a)
end
- normalized = seq.map { |i| i - seq.min }
-
- expect(normalized).to eq((0..seq.size - 1).to_a)
- end
- context 'there are no instances to pass in' do
- let(:id_subject) { Issue }
+ context 'there are no instances to pass in' do
+ let(:id_subject) { Issue }
- it 'accepts classes instead' do
- expect(subject).to eq(1)
+ it 'accepts classes instead' do
+ expect(subject).to eq(1)
+ end
end
- end
- context 'when executed outside of transaction' do
- it 'increments counter with in_transaction: "false"' do
- allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
+ context 'when executed outside of transaction' do
+ it 'increments counter with in_transaction: "false"' do
+ allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
- expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
- .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original
+ expect(InternalId.internal_id_transactions_total).to receive(:increment)
+ .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original
- subject
+ subject
+ end
end
- end
- context 'when executed within transaction' do
- it 'increments counter with in_transaction: "true"' do
- expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
- .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original
+ context 'when executed within transaction' do
+ it 'increments counter with in_transaction: "true"' do
+ expect(InternalId.internal_id_transactions_total).to receive(:increment)
+ .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original
- InternalId.transaction { subject }
+ InternalId.transaction { subject }
+ end
end
end
- end
- describe '.reset' do
- subject { described_class.reset(issue, scope, usage, value) }
+ describe '.reset' do
+ subject { described_class.reset(issue, scope, usage, value) }
- context 'in the absence of a record' do
- let(:value) { 2 }
+ context 'in the absence of a record' do
+ let(:value) { 2 }
- it 'does not revert back the value' do
- expect { subject }.not_to change { described_class.count }
- expect(subject).to be_falsey
+ it 'does not revert back the value' do
+ expect { subject }.not_to change { described_class.count }
+ expect(subject).to be_falsey
+ end
end
- end
- context 'when valid iid is used to reset' do
- let!(:value) { generate_next }
+ context 'when valid iid is used to reset' do
+ let!(:value) { generate_next }
- context 'and iid is a latest one' do
- it 'does rewind and next generated value is the same' do
- expect(subject).to be_truthy
- expect(generate_next).to eq(value)
+ context 'and iid is a latest one' do
+ it 'does rewind and next generated value is the same' do
+ expect(subject).to be_truthy
+ expect(generate_next).to eq(value)
+ end
end
- end
- context 'and iid is not a latest one' do
- it 'does not rewind' do
- generate_next
+ context 'and iid is not a latest one' do
+ it 'does not rewind' do
+ generate_next
- expect(subject).to be_falsey
- expect(generate_next).to be > value
+ expect(subject).to be_falsey
+ expect(generate_next).to be > value
+ end
end
- end
- def generate_next
- described_class.generate_next(issue, scope, usage, init)
+ def generate_next
+ described_class.generate_next(issue, scope, usage, init)
+ end
end
- end
- context 'when executed outside of transaction' do
- let(:value) { 2 }
+ context 'when executed outside of transaction' do
+ let(:value) { 2 }
- it 'increments counter with in_transaction: "false"' do
- allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
+ it 'increments counter with in_transaction: "false"' do
+ allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
- expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
- .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original
+ expect(InternalId.internal_id_transactions_total).to receive(:increment)
+ .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original
- subject
+ subject
+ end
end
- end
- context 'when executed within transaction' do
- let(:value) { 2 }
+ context 'when executed within transaction' do
+ let(:value) { 2 }
- it 'increments counter with in_transaction: "true"' do
- expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
- .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original
+ it 'increments counter with in_transaction: "true"' do
+ expect(InternalId.internal_id_transactions_total).to receive(:increment)
+ .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original
- InternalId.transaction { subject }
+ InternalId.transaction { subject }
+ end
end
end
- end
- describe '.track_greatest' do
- let(:value) { 9001 }
+ describe '.track_greatest' do
+ let(:value) { 9001 }
- subject { described_class.track_greatest(id_subject, scope, usage, value, init) }
+ subject { described_class.track_greatest(id_subject, scope, usage, value, init) }
- context 'in the absence of a record' do
- it 'creates a record if not yet present' do
- expect { subject }.to change { described_class.count }.from(0).to(1)
+ context 'in the absence of a record' do
+ it 'creates a record if not yet present' do
+ expect { subject }.to change { described_class.count }.from(0).to(1)
+ end
end
- end
- it 'stores record attributes' do
- subject
+ it 'stores record attributes' do
+ subject
- described_class.first.tap do |record|
- expect(record.project).to eq(project)
- expect(record.usage).to eq(usage.to_s)
- expect(record.last_value).to eq(value)
+ described_class.first.tap do |record|
+ expect(record.project).to eq(project)
+ expect(record.usage).to eq(usage.to_s)
+ expect(record.last_value).to eq(value)
+ end
end
- end
- context 'with existing issues' do
- before do
- create(:issue, project: project)
- described_class.delete_all
- end
+ context 'with existing issues' do
+ before do
+ create(:issue, project: project)
+ described_class.delete_all
+ end
- it 'still returns the last value to that of the given value' do
- expect(subject).to eq(value)
+ it 'still returns the last value to that of the given value' do
+ expect(subject).to eq(value)
+ end
end
- end
- context 'when value is less than the current last_value' do
- it 'returns the current last_value' do
- described_class.create!(**scope, usage: usage, last_value: 10_001)
+ context 'when value is less than the current last_value' do
+ it 'returns the current last_value' do
+ described_class.create!(**scope, usage: usage, last_value: 10_001)
- expect(subject).to eq 10_001
+ expect(subject).to eq 10_001
+ end
end
- end
- context 'there are no instances to pass in' do
- let(:id_subject) { Issue }
+ context 'there are no instances to pass in' do
+ let(:id_subject) { Issue }
- it 'accepts classes instead' do
- expect(subject).to eq(value)
+ it 'accepts classes instead' do
+ expect(subject).to eq(value)
+ end
end
- end
- context 'when executed outside of transaction' do
- it 'increments counter with in_transaction: "false"' do
- allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
+ context 'when executed outside of transaction' do
+ it 'increments counter with in_transaction: "false"' do
+ allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
- expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
- .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original
+ expect(InternalId.internal_id_transactions_total).to receive(:increment)
+ .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original
- subject
+ subject
+ end
end
- end
- context 'when executed within transaction' do
- it 'increments counter with in_transaction: "true"' do
- expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
- .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original
+ context 'when executed within transaction' do
+ it 'increments counter with in_transaction: "true"' do
+ expect(InternalId.internal_id_transactions_total).to receive(:increment)
+ .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original
- InternalId.transaction { subject }
+ InternalId.transaction { subject }
+ end
end
end
end
+ context 'when the feature flag is disabled' do
+ stub_feature_flags(generate_iids_without_explicit_locking: false)
+
+ it_behaves_like 'a monotonically increasing id generator'
+ end
+
+ context 'when the feature flag is enabled' do
+ stub_feature_flags(generate_iids_without_explicit_locking: true)
+
+ it_behaves_like 'a monotonically increasing id generator'
+ end
+
describe '#increment_and_save!' do
let(:id) { create(:internal_id) }
diff --git a/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb b/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb
index 42f82987989..03f565e0aac 100644
--- a/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb
+++ b/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb
@@ -165,9 +165,9 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
3.times { supply.next_value }
end
- current_value = described_class.public_send(method_name, scope_value, &:current_value)
-
- expect(current_value).to eq(iid + 3)
+ described_class.public_send(method_name, scope_value) do |supply|
+ expect(supply.next_value).to eq(iid + 4)
+ end
end
end