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>2023-12-27 18:07:02 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2023-12-27 18:07:02 +0300
commitd9953dadb4f0170a32fbb3204564f2f96396656e (patch)
tree5106d7c970874f3afcf39cca0d8adfc76abc09c3
parentf8740a1ade9d4614dde927d8983eeb288e783ccf (diff)
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--.rubocop_todo/layout/empty_line_after_magic_comment.yml4
-rw-r--r--.rubocop_todo/layout/line_length.yml6
-rw-r--r--.rubocop_todo/layout/space_inside_parens.yml2
-rw-r--r--.rubocop_todo/lint/unused_block_argument.yml2
-rw-r--r--.rubocop_todo/performance/map_compact.yml2
-rw-r--r--.rubocop_todo/rspec/before_all_role_assignment.yml2
-rw-r--r--.rubocop_todo/rspec/context_wording.yml6
-rw-r--r--.rubocop_todo/rspec/expect_change.yml2
-rw-r--r--.rubocop_todo/rspec/feature_category.yml4
-rw-r--r--app/graphql/mutations/ml/models/base.rb20
-rw-r--r--app/graphql/mutations/ml/models/create.rb32
-rw-r--r--app/graphql/types/ml/model_type.rb4
-rw-r--r--app/graphql/types/mutation_type.rb1
-rw-r--r--app/helpers/ci/status_helper.rb4
-rw-r--r--app/models/member.rb8
-rw-r--r--app/models/members/group_member.rb12
-rw-r--r--app/models/members/project_member.rb8
-rw-r--r--app/services/click_house/sync_strategies/base_sync_strategy.rb124
-rw-r--r--app/services/click_house/sync_strategies/event_sync_strategy.rb62
-rw-r--r--app/services/ml/create_model_service.rb20
-rw-r--r--app/services/notification_service.rb29
-rw-r--r--app/workers/click_house/events_sync_worker.rb135
-rw-r--r--doc/api/graphql/reference/index.md26
-rw-r--r--doc/user/analytics/repository_analytics.md2
-rw-r--r--lib/api/ml/mlflow/registered_models.rb8
-rw-r--r--spec/graphql/types/ml/model_type_spec.rb2
-rw-r--r--spec/helpers/ci/status_helper_spec.rb13
-rw-r--r--spec/models/member_spec.rb26
-rw-r--r--spec/models/members/group_member_spec.rb12
-rw-r--r--spec/requests/api/graphql/mutations/ml/models/create_spec.rb48
-rw-r--r--spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb169
-rw-r--r--spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb128
-rw-r--r--spec/services/members/update_service_spec.rb2
-rw-r--r--spec/services/ml/create_model_service_spec.rb5
-rw-r--r--spec/services/notification_service_spec.rb48
-rw-r--r--spec/support/rspec_order_todo.yml6
-rw-r--r--spec/support/shared_examples/models/members_notifications_shared_example.rb16
-rw-r--r--spec/workers/click_house/events_sync_worker_spec.rb174
38 files changed, 749 insertions, 425 deletions
diff --git a/.rubocop_todo/layout/empty_line_after_magic_comment.yml b/.rubocop_todo/layout/empty_line_after_magic_comment.yml
index 409acc4b5c1..ba08bc8ae08 100644
--- a/.rubocop_todo/layout/empty_line_after_magic_comment.yml
+++ b/.rubocop_todo/layout/empty_line_after_magic_comment.yml
@@ -312,9 +312,9 @@ Layout/EmptyLineAfterMagicComment:
- 'ee/spec/models/ci/minutes/quota_spec.rb'
- 'ee/spec/models/ci/minutes/usage_spec.rb'
- 'ee/spec/models/deployments/approval_summary_spec.rb'
- - 'ee/spec/models/group_member_spec.rb'
+ - 'ee/spec/models/ee/group_member_spec.rb'
- 'ee/spec/models/packages/package_file_spec.rb'
- - 'ee/spec/models/project_member_spec.rb'
+ - 'ee/spec/models/ee/project_member_spec.rb'
- 'ee/spec/models/protected_environment_spec.rb'
- 'ee/spec/models/protected_environments/approval_rule_spec.rb'
- 'ee/spec/models/protected_environments/deploy_access_level_spec.rb'
diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml
index 8cab670cc83..b8b267d6d86 100644
--- a/.rubocop_todo/layout/line_length.yml
+++ b/.rubocop_todo/layout/line_length.yml
@@ -1794,7 +1794,7 @@ Layout/LineLength:
- 'ee/spec/models/geo_node_status_spec.rb'
- 'ee/spec/models/gitlab_subscription_spec.rb'
- 'ee/spec/models/gitlab_subscriptions/features_spec.rb'
- - 'ee/spec/models/group_member_spec.rb'
+ - 'ee/spec/models/ee/group_member_spec.rb'
- 'ee/spec/models/historical_data_spec.rb'
- 'ee/spec/models/incident_management/escalation_policy_spec.rb'
- 'ee/spec/models/incident_management/escalation_rule_spec.rb'
@@ -1807,7 +1807,7 @@ Layout/LineLength:
- 'ee/spec/models/issue_spec.rb'
- 'ee/spec/models/iteration_spec.rb'
- 'ee/spec/models/license_spec.rb'
- - 'ee/spec/models/member_spec.rb'
+ - 'ee/spec/models/ee/member_spec.rb'
- 'ee/spec/models/merge_request_spec.rb'
- 'ee/spec/models/merge_requests/compliance_violation_spec.rb'
- 'ee/spec/models/merge_requests/external_status_check_spec.rb'
@@ -1816,7 +1816,7 @@ Layout/LineLength:
- 'ee/spec/models/packages/package_file_spec.rb'
- 'ee/spec/models/project_import_data_spec.rb'
- 'ee/spec/models/project_import_state_spec.rb'
- - 'ee/spec/models/project_member_spec.rb'
+ - 'ee/spec/models/ee/project_member_spec.rb'
- 'ee/spec/models/project_security_setting_spec.rb'
- 'ee/spec/models/protected_environment_spec.rb'
- 'ee/spec/models/protected_environments/approval_rule_spec.rb'
diff --git a/.rubocop_todo/layout/space_inside_parens.yml b/.rubocop_todo/layout/space_inside_parens.yml
index 34f13f780fb..d044e48d7aa 100644
--- a/.rubocop_todo/layout/space_inside_parens.yml
+++ b/.rubocop_todo/layout/space_inside_parens.yml
@@ -35,7 +35,7 @@ Layout/SpaceInsideParens:
- 'ee/spec/models/iteration_spec.rb'
- 'ee/spec/models/ldap_group_link_spec.rb'
- 'ee/spec/models/license_spec.rb'
- - 'ee/spec/models/member_spec.rb'
+ - 'ee/spec/models/ee/member_spec.rb'
- 'ee/spec/models/release_highlight_spec.rb'
- 'ee/spec/models/security/orchestration_policy_configuration_spec.rb'
- 'ee/spec/models/vulnerabilities/feedback_spec.rb'
diff --git a/.rubocop_todo/lint/unused_block_argument.yml b/.rubocop_todo/lint/unused_block_argument.yml
index 1df273d408d..f14c02c4ecd 100644
--- a/.rubocop_todo/lint/unused_block_argument.yml
+++ b/.rubocop_todo/lint/unused_block_argument.yml
@@ -131,7 +131,7 @@ Lint/UnusedBlockArgument:
- 'ee/spec/lib/gitlab/graphql/aggregations/epics/lazy_epic_aggregate_spec.rb'
- 'ee/spec/lib/gitlab/insights/project_insights_config_spec.rb'
- 'ee/spec/lib/gitlab/usage_data_metrics_spec.rb'
- - 'ee/spec/models/member_spec.rb'
+ - 'ee/spec/models/ee/member_spec.rb'
- 'ee/spec/requests/api/graphql/project/pipeline/security_report_summary_spec.rb'
- 'ee/spec/requests/api/graphql/vulnerabilities/sort_spec.rb'
- 'ee/spec/requests/api/related_epic_links_spec.rb'
diff --git a/.rubocop_todo/performance/map_compact.yml b/.rubocop_todo/performance/map_compact.yml
index 8f831bf8f59..22cefbab067 100644
--- a/.rubocop_todo/performance/map_compact.yml
+++ b/.rubocop_todo/performance/map_compact.yml
@@ -75,7 +75,7 @@ Performance/MapCompact:
- 'ee/lib/gitlab/ci/reports/metrics/reports_comparer.rb'
- 'ee/lib/gitlab/search/aggregation_parser.rb'
- 'ee/spec/models/analytics/issues_analytics_spec.rb'
- - 'ee/spec/models/member_spec.rb'
+ - 'ee/spec/models/ee/member_spec.rb'
- 'ee/spec/requests/api/audit_events_spec.rb'
- 'ee/spec/requests/api/search_spec.rb'
- 'haml_lint/linter/no_plain_nodes.rb'
diff --git a/.rubocop_todo/rspec/before_all_role_assignment.yml b/.rubocop_todo/rspec/before_all_role_assignment.yml
index fcdaf78ffe8..3436cffc2ff 100644
--- a/.rubocop_todo/rspec/before_all_role_assignment.yml
+++ b/.rubocop_todo/rspec/before_all_role_assignment.yml
@@ -321,7 +321,7 @@ RSpec/BeforeAllRoleAssignment:
- 'ee/spec/models/ee/vulnerability_spec.rb'
- 'ee/spec/models/epic_issue_spec.rb'
- 'ee/spec/models/epic_spec.rb'
- - 'ee/spec/models/group_member_spec.rb'
+ - 'ee/spec/models/ee/group_member_spec.rb'
- 'ee/spec/models/instance_security_dashboard_spec.rb'
- 'ee/spec/models/issue_spec.rb'
- 'ee/spec/models/merge_request_spec.rb'
diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml
index 6375841e697..0a50fc4485d 100644
--- a/.rubocop_todo/rspec/context_wording.yml
+++ b/.rubocop_todo/rspec/context_wording.yml
@@ -453,7 +453,7 @@ RSpec/ContextWording:
- 'ee/spec/models/geo_node_spec.rb'
- 'ee/spec/models/geo_node_status_spec.rb'
- 'ee/spec/models/gitlab_subscription_spec.rb'
- - 'ee/spec/models/group_member_spec.rb'
+ - 'ee/spec/models/ee/group_member_spec.rb'
- 'ee/spec/models/group_wiki_repository_spec.rb'
- 'ee/spec/models/incident_management/escalation_rule_spec.rb'
- 'ee/spec/models/incident_management/oncall_rotation_spec.rb'
@@ -467,7 +467,7 @@ RSpec/ContextWording:
- 'ee/spec/models/issue_spec.rb'
- 'ee/spec/models/iteration_spec.rb'
- 'ee/spec/models/license_spec.rb'
- - 'ee/spec/models/member_spec.rb'
+ - 'ee/spec/models/ee/member_spec.rb'
- 'ee/spec/models/merge_request/blocking_spec.rb'
- 'ee/spec/models/merge_request_spec.rb'
- 'ee/spec/models/namespace_setting_spec.rb'
@@ -476,7 +476,7 @@ RSpec/ContextWording:
- 'ee/spec/models/path_lock_spec.rb'
- 'ee/spec/models/project_import_data_spec.rb'
- 'ee/spec/models/project_import_state_spec.rb'
- - 'ee/spec/models/project_member_spec.rb'
+ - 'ee/spec/models/ee/project_member_spec.rb'
- 'ee/spec/models/protected_environment_spec.rb'
- 'ee/spec/models/push_rule_spec.rb'
- 'ee/spec/models/release_highlight_spec.rb'
diff --git a/.rubocop_todo/rspec/expect_change.yml b/.rubocop_todo/rspec/expect_change.yml
index f358600f4fe..c5e3556defd 100644
--- a/.rubocop_todo/rspec/expect_change.yml
+++ b/.rubocop_todo/rspec/expect_change.yml
@@ -36,7 +36,7 @@ RSpec/ExpectChange:
- 'ee/spec/models/gitlab_subscription_spec.rb'
- 'ee/spec/models/group_wiki_spec.rb'
- 'ee/spec/models/incident_management/issuable_escalation_status_spec.rb'
- - 'ee/spec/models/member_spec.rb'
+ - 'ee/spec/models/ee/member_spec.rb'
- 'ee/spec/models/project_import_state_spec.rb'
- 'ee/spec/models/push_rule_spec.rb'
- 'ee/spec/models/security/orchestration_policy_configuration_spec.rb'
diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml
index ab4170d5a2f..163eeb207ad 100644
--- a/.rubocop_todo/rspec/feature_category.yml
+++ b/.rubocop_todo/rspec/feature_category.yml
@@ -1063,7 +1063,7 @@ RSpec/FeatureCategory:
- 'ee/spec/models/gitlab_subscriptions/features_spec.rb'
- 'ee/spec/models/gitlab_subscriptions/upcoming_reconciliation_spec.rb'
- 'ee/spec/models/group_deletion_schedule_spec.rb'
- - 'ee/spec/models/group_member_spec.rb'
+ - 'ee/spec/models/ee/group_member_spec.rb'
- 'ee/spec/models/group_merge_request_approval_setting_spec.rb'
- 'ee/spec/models/group_wiki_repository_spec.rb'
- 'ee/spec/models/historical_data_spec.rb'
@@ -1108,7 +1108,7 @@ RSpec/FeatureCategory:
- 'ee/spec/models/project_ci_cd_setting_spec.rb'
- 'ee/spec/models/project_feature_spec.rb'
- 'ee/spec/models/project_import_data_spec.rb'
- - 'ee/spec/models/project_member_spec.rb'
+ - 'ee/spec/models/ee/project_member_spec.rb'
- 'ee/spec/models/project_repository_state_spec.rb'
- 'ee/spec/models/project_security_setting_spec.rb'
- 'ee/spec/models/protected_branch/required_code_owners_section_spec.rb'
diff --git a/app/graphql/mutations/ml/models/base.rb b/app/graphql/mutations/ml/models/base.rb
new file mode 100644
index 00000000000..e3c5a7a13a8
--- /dev/null
+++ b/app/graphql/mutations/ml/models/base.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module Mutations
+ module Ml
+ module Models
+ class Base < BaseMutation
+ authorize :write_model_registry
+
+ argument :project_path, GraphQL::Types::ID,
+ required: true,
+ description: "Project the model to mutate is in."
+
+ field :model,
+ Types::Ml::ModelType,
+ null: true,
+ description: 'Model after mutation.'
+ end
+ end
+ end
+end
diff --git a/app/graphql/mutations/ml/models/create.rb b/app/graphql/mutations/ml/models/create.rb
new file mode 100644
index 00000000000..21570fc34b8
--- /dev/null
+++ b/app/graphql/mutations/ml/models/create.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Mutations
+ module Ml
+ module Models
+ class Create < Base
+ graphql_name 'MlModelCreate'
+
+ include FindsProject
+
+ argument :name, GraphQL::Types::String,
+ required: true,
+ description: 'Name of the model.'
+
+ argument :description, GraphQL::Types::String,
+ required: false,
+ description: 'Description of the model.'
+
+ def resolve(**args)
+ project = authorized_find!(args[:project_path])
+
+ model = ::Ml::CreateModelService.new(project, args[:name], current_user, args[:description]).execute
+
+ {
+ model: model.persisted? ? model : nil,
+ errors: errors_on_object(model)
+ }
+ end
+ end
+ end
+ end
+end
diff --git a/app/graphql/types/ml/model_type.rb b/app/graphql/types/ml/model_type.rb
index e677b37547a..a26d50cbdc4 100644
--- a/app/graphql/types/ml/model_type.rb
+++ b/app/graphql/types/ml/model_type.rb
@@ -9,12 +9,16 @@ module Types
connection_type_class Types::LimitedCountableConnectionType
+ present_using ::Ml::ModelPresenter
+
field :id, ::Types::GlobalIDType[::Ml::Model], null: false, description: 'ID of the model.'
field :name, ::GraphQL::Types::String, null: false, description: 'Name of the model.'
field :created_at, Types::TimeType, null: false, description: 'Date of creation.'
+ field :description, ::GraphQL::Types::String, null: false, description: 'Description of the model.'
+
field :latest_version, ::Types::Ml::ModelVersionType, null: true, description: 'Latest version of the model.'
field :version_count, ::GraphQL::Types::Int, null: true, description: 'Count of versions in the model.'
diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb
index 4c987c657ef..0a725c2e0a7 100644
--- a/app/graphql/types/mutation_type.rb
+++ b/app/graphql/types/mutation_type.rb
@@ -203,6 +203,7 @@ module Types
mount_mutation Mutations::Users::SetNamespaceCommitEmail
mount_mutation Mutations::WorkItems::Subscribe, alpha: { milestone: '16.3' }
mount_mutation Mutations::Admin::AbuseReportLabels::Create, alpha: { milestone: '16.4' }
+ mount_mutation Mutations::Ml::Models::Create, alpha: { milestone: '16.8' }
end
end
diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 21d982d42bc..15413cb05ac 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -56,10 +56,6 @@ module Ci
end
# rubocop:enable Metrics/CyclomaticComplexity
- def pipeline_status_cache_key(pipeline_status)
- "pipeline-status/#{pipeline_status.sha}-#{pipeline_status.status}"
- end
-
def render_commit_status(commit, status, ref: nil, tooltip_placement: 'left')
project = commit.project
path = pipelines_project_commit_path(project, commit, ref: ref)
diff --git a/app/models/member.rb b/app/models/member.rb
index feb0b66f63c..2db2df91cbe 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -526,6 +526,14 @@ class Member < ApplicationRecord
end
def post_update_hook
+ if saved_change_to_access_level?
+ run_after_commit { notification_service.updated_member_access_level(self) }
+ end
+
+ if saved_change_to_expires_at?
+ run_after_commit { notification_service.updated_member_expiration(self) }
+ end
+
system_hook_service.execute_hooks_for(self, :update)
end
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 0e70f9973d6..3f1fcfe8339 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -94,18 +94,6 @@ class GroupMember < Member
super
end
- def post_update_hook
- if saved_change_to_access_level?
- run_after_commit { notification_service.update_group_member(self) }
- end
-
- if saved_change_to_expires_at?
- run_after_commit { notification_service.updated_group_member_expiration(self) }
- end
-
- super
- end
-
def after_accept_invite
run_after_commit_or_now do
notification_service.accept_group_invite(self)
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index e442fc969d0..3540e7a043a 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -128,14 +128,6 @@ class ProjectMember < Member
super
end
- def post_update_hook
- if saved_change_to_access_level?
- run_after_commit { notification_service.update_project_member(self) }
- end
-
- super
- end
-
def post_destroy_hook
if expired?
event_service.expired_leave_project(self.project, self.user)
diff --git a/app/services/click_house/sync_strategies/base_sync_strategy.rb b/app/services/click_house/sync_strategies/base_sync_strategy.rb
new file mode 100644
index 00000000000..58c2161b83c
--- /dev/null
+++ b/app/services/click_house/sync_strategies/base_sync_strategy.rb
@@ -0,0 +1,124 @@
+# frozen_string_literal: true
+
+module ClickHouse
+ module SyncStrategies
+ class BaseSyncStrategy
+ include Gitlab::ExclusiveLeaseHelpers
+ include Gitlab::Utils::StrongMemoize
+
+ # the job is scheduled every 3 minutes and we will allow maximum 2.5 minutes runtime
+ MAX_TTL = 2.5.minutes.to_i
+ MAX_RUNTIME = 120.seconds
+ BATCH_SIZE = 500
+ INSERT_BATCH_SIZE = 5000
+
+ def execute
+ return { status: :disabled } unless enabled?
+
+ metadata = { status: :processed }
+
+ begin
+ # Prevent parallel jobs
+ in_lock(self.class.to_s, ttl: MAX_TTL, retries: 0) do
+ loop { break unless next_batch }
+
+ metadata.merge!(records_inserted: context.total_record_count,
+ reached_end_of_table: context.no_more_records?)
+
+ if context.last_processed_id
+ ClickHouse::SyncCursor.update_cursor_for(model_class.table_name,
+ context.last_processed_id)
+ end
+ end
+ rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError
+ # Skip retrying, just let the next worker to start after a few minutes
+ metadata = { status: :skipped }
+ end
+
+ metadata
+ end
+
+ private
+
+ def enabled?
+ ClickHouse::Client.database_configured?(:main)
+ end
+
+ def context
+ @context ||= ClickHouse::RecordSyncContext.new(
+ last_record_id: ClickHouse::SyncCursor.cursor_for(model_class.table_name),
+ max_records_per_batch: INSERT_BATCH_SIZE,
+ runtime_limiter: Analytics::CycleAnalytics::RuntimeLimiter.new(MAX_RUNTIME)
+ )
+ end
+
+ def last_id_in_postgresql
+ model_class.maximum(:id)
+ end
+
+ strong_memoize_attr :last_id_in_postgresql
+
+ def next_batch
+ context.new_batch!
+
+ CsvBuilder::Gzip.new(process_batch(context), csv_mapping).render do |tempfile, rows_written|
+ unless rows_written == 0
+ ClickHouse::Client.insert_csv(insert_query, File.open(tempfile.path),
+ :main)
+ end
+ end
+
+ !(context.over_time? || context.no_more_records?)
+ end
+
+ def process_batch(context)
+ Enumerator.new do |yielder|
+ has_more_data = false
+ batching_scope.each_batch(of: BATCH_SIZE) do |relation|
+ records = relation.select(projections).to_a
+ has_more_data = records.size == BATCH_SIZE
+ records.each do |row|
+ yielder << transform_row(row)
+ context.last_processed_id = row.id
+
+ break if context.record_limit_reached?
+ end
+
+ break if context.over_time? || context.record_limit_reached? || !has_more_data
+ end
+
+ context.no_more_records! unless has_more_data
+ end
+ end
+
+ def transform_row(row)
+ row
+ end
+
+ # rubocop: disable CodeReuse/ActiveRecord -- because model here is dynamic and is passed by child class
+ def batching_scope
+ return model_class.none unless last_id_in_postgresql
+
+ table = model_class.arel_table
+
+ model_class
+ .where(table[:id].gt(context.last_record_id))
+ .where(table[:id].lteq(last_id_in_postgresql))
+ end
+
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ def projections
+ raise NotImplementedError, "Subclasses must implement `projections`"
+ end
+
+ def csv_mapping
+ raise NotImplementedError, "Subclasses must implement `csv_mapping`"
+ end
+
+ def insert_query
+ raise NotImplementedError, "Subclasses must implement `insert_query`"
+ end
+ end
+ end
+end
diff --git a/app/services/click_house/sync_strategies/event_sync_strategy.rb b/app/services/click_house/sync_strategies/event_sync_strategy.rb
new file mode 100644
index 00000000000..3e86e8c52bc
--- /dev/null
+++ b/app/services/click_house/sync_strategies/event_sync_strategy.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module ClickHouse
+ module SyncStrategies
+ class EventSyncStrategy < BaseSyncStrategy
+ # transforms the traversal_ids to a String:
+ # Example: group_id/subgroup_id/group_or_projectnamespace_id/
+ PATH_COLUMN = <<~SQL
+ (
+ CASE
+ WHEN project_id IS NOT NULL THEN (SELECT array_to_string(traversal_ids, '/') || '/' FROM namespaces WHERE id = (SELECT project_namespace_id FROM projects WHERE id = events.project_id LIMIT 1) LIMIT 1)
+ WHEN group_id IS NOT NULL THEN (SELECT array_to_string(traversal_ids, '/') || '/' FROM namespaces WHERE id = events.group_id LIMIT 1)
+ ELSE ''
+ END
+ ) AS path
+ SQL
+
+ private
+
+ def csv_mapping
+ {
+ id: :id,
+ path: :path,
+ author_id: :author_id,
+ target_id: :target_id,
+ target_type: :target_type,
+ action: :raw_action,
+ created_at: :casted_created_at,
+ updated_at: :casted_updated_at
+ }
+ end
+
+ def projections
+ [
+ :id,
+ PATH_COLUMN,
+ :author_id,
+ :target_id,
+ :target_type,
+ 'action AS raw_action',
+ 'EXTRACT(epoch FROM created_at) AS casted_created_at',
+ 'EXTRACT(epoch FROM updated_at) AS casted_updated_at'
+ ]
+ end
+
+ def insert_query
+ <<~SQL.squish
+ INSERT INTO events (#{csv_mapping.keys.join(', ')})
+ SETTINGS async_insert=1, wait_for_async_insert=1 FORMAT CSV
+ SQL
+ end
+
+ def model_class
+ ::Event
+ end
+
+ def enabled?
+ super && Feature.enabled?(:event_sync_worker_for_click_house)
+ end
+ end
+ end
+end
diff --git a/app/services/ml/create_model_service.rb b/app/services/ml/create_model_service.rb
index b87b13dd379..7ac9c2a2737 100644
--- a/app/services/ml/create_model_service.rb
+++ b/app/services/ml/create_model_service.rb
@@ -12,21 +12,25 @@ module Ml
def execute
ApplicationRecord.transaction do
- model = Ml::Model.create!(
+ model = Ml::Model.new(
project: @project,
name: @name,
- user: (@user.is_a?(User) ? @user : nil),
+ user: @user,
description: @description,
default_experiment: default_experiment
)
- add_metadata(model, @metadata)
+ model.save
- Gitlab::InternalEvents.track_event(
- 'model_registry_ml_model_created',
- project: @project,
- user: @user
- )
+ if model.persisted?
+ add_metadata(model, @metadata)
+
+ Gitlab::InternalEvents.track_event(
+ 'model_registry_ml_model_created',
+ project: @project,
+ user: @user
+ )
+ end
model
end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 95d291b9bd6..ff712d78c8b 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -532,14 +532,21 @@ class NotificationService
mailer.member_invite_accepted_email(project_member.real_source_type, project_member.id).deliver_later
end
- def new_project_member(project_member)
- return true unless project_member.notifiable?(:mention, skip_read_ability: true)
+ def updated_member_access_level(member)
+ return true unless member.notifiable?(:mention)
- mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
+ mailer.member_access_granted_email(member.real_source_type, member.id).deliver_later
+ end
+
+ def updated_member_expiration(member)
+ return true unless member.source.is_a?(Group)
+ return true unless member.notifiable?(:mention)
+
+ mailer.member_expiration_date_updated_email(member.real_source_type, member.id).deliver_later
end
- def update_project_member(project_member)
- return true unless project_member.notifiable?(:mention)
+ def new_project_member(project_member)
+ return true unless project_member.notifiable?(:mention, skip_read_ability: true)
mailer.member_access_granted_email(project_member.real_source_type, project_member.id).deliver_later
end
@@ -564,18 +571,6 @@ class NotificationService
mailer.member_access_granted_email(group_member.real_source_type, group_member.id).deliver_later
end
- def update_group_member(group_member)
- return true unless group_member.notifiable?(:mention)
-
- mailer.member_access_granted_email(group_member.real_source_type, group_member.id).deliver_later
- end
-
- def updated_group_member_expiration(group_member)
- return true unless group_member.notifiable?(:mention)
-
- mailer.member_expiration_date_updated_email(group_member.real_source_type, group_member.id).deliver_later
- end
-
def project_was_moved(project, old_path_with_namespace)
recipients = project_moved_recipients(project)
recipients = notifiable_users(recipients, :custom, custom_action: :moved_project, project: project)
diff --git a/app/workers/click_house/events_sync_worker.rb b/app/workers/click_house/events_sync_worker.rb
index 21c10566a67..3cfd3f91a29 100644
--- a/app/workers/click_house/events_sync_worker.rb
+++ b/app/workers/click_house/events_sync_worker.rb
@@ -4,8 +4,6 @@ module ClickHouse
class EventsSyncWorker
include ApplicationWorker
include ClickHouseWorker
- include Gitlab::ExclusiveLeaseHelpers
- include Gitlab::Utils::StrongMemoize
idempotent!
queue_namespace :cronjob
@@ -13,138 +11,9 @@ module ClickHouse
worker_has_external_dependencies! # the worker interacts with a ClickHouse database
feature_category :value_stream_management
- # the job is scheduled every 3 minutes and we will allow maximum 2.5 minutes runtime
- MAX_TTL = 2.5.minutes.to_i
- MAX_RUNTIME = 120.seconds
- BATCH_SIZE = 500
- INSERT_BATCH_SIZE = 5000
- CSV_MAPPING = {
- id: :id,
- path: :path,
- author_id: :author_id,
- target_id: :target_id,
- target_type: :target_type,
- action: :raw_action,
- created_at: :casted_created_at,
- updated_at: :casted_updated_at
- }.freeze
-
- # transforms the traversal_ids to a String:
- # Example: group_id/subgroup_id/group_or_projectnamespace_id/
- PATH_COLUMN = <<~SQL
- (
- CASE
- WHEN project_id IS NOT NULL THEN (SELECT array_to_string(traversal_ids, '/') || '/' FROM namespaces WHERE id = (SELECT project_namespace_id FROM projects WHERE id = events.project_id LIMIT 1) LIMIT 1)
- WHEN group_id IS NOT NULL THEN (SELECT array_to_string(traversal_ids, '/') || '/' FROM namespaces WHERE id = events.group_id LIMIT 1)
- ELSE ''
- END
- ) AS path
- SQL
-
- EVENT_PROJECTIONS = [
- :id,
- PATH_COLUMN,
- :author_id,
- :target_id,
- :target_type,
- 'action AS raw_action',
- 'EXTRACT(epoch FROM created_at) AS casted_created_at',
- 'EXTRACT(epoch FROM updated_at) AS casted_updated_at'
- ].freeze
-
- INSERT_EVENTS_QUERY = <<~SQL.squish
- INSERT INTO events (#{CSV_MAPPING.keys.join(', ')})
- SETTINGS async_insert=1, wait_for_async_insert=1 FORMAT CSV
- SQL
-
def perform
- unless enabled?
- log_extra_metadata_on_done(:result, { status: :disabled })
-
- return
- end
-
- metadata = { status: :processed }
-
- begin
- # Prevent parallel jobs
- in_lock(self.class.to_s, ttl: MAX_TTL, retries: 0) do
- loop { break unless next_batch }
-
- metadata.merge!(records_inserted: context.total_record_count, reached_end_of_table: context.no_more_records?)
-
- ClickHouse::SyncCursor.update_cursor_for(:events, context.last_processed_id) if context.last_processed_id
- end
- rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError
- # Skip retrying, just let the next worker to start after a few minutes
- metadata = { status: :skipped }
- end
-
- log_extra_metadata_on_done(:result, metadata)
- end
-
- private
-
- def context
- @context ||= ClickHouse::RecordSyncContext.new(
- last_record_id: ClickHouse::SyncCursor.cursor_for(:events),
- max_records_per_batch: INSERT_BATCH_SIZE,
- runtime_limiter: Analytics::CycleAnalytics::RuntimeLimiter.new(MAX_RUNTIME)
- )
- end
-
- def last_event_id_in_postgresql
- Event.maximum(:id)
- end
- strong_memoize_attr :last_event_id_in_postgresql
-
- def enabled?
- ClickHouse::Client.database_configured?(:main) && Feature.enabled?(:event_sync_worker_for_click_house)
- end
-
- def next_batch
- context.new_batch!
-
- CsvBuilder::Gzip.new(process_batch(context), CSV_MAPPING).render do |tempfile, rows_written|
- unless rows_written == 0
- ClickHouse::Client.insert_csv(INSERT_EVENTS_QUERY, File.open(tempfile.path),
- :main)
- end
- end
-
- !(context.over_time? || context.no_more_records?)
- end
-
- def process_batch(context)
- Enumerator.new do |yielder|
- has_more_data = false
- batching_scope.each_batch(of: BATCH_SIZE) do |relation|
- records = relation.select(*EVENT_PROJECTIONS).to_a
- has_more_data = records.size == BATCH_SIZE
- records.each do |row|
- yielder << row
- context.last_processed_id = row.id
-
- break if context.record_limit_reached?
- end
-
- break if context.over_time? || context.record_limit_reached? || !has_more_data
- end
-
- context.no_more_records! unless has_more_data
- end
- end
-
- # rubocop: disable CodeReuse/ActiveRecord
- def batching_scope
- return Event.none unless last_event_id_in_postgresql
-
- table = Event.arel_table
-
- Event
- .where(table[:id].gt(context.last_record_id))
- .where(table[:id].lteq(last_event_id_in_postgresql))
+ result = ::ClickHouse::SyncStrategies::EventSyncStrategy.new.execute
+ log_extra_metadata_on_done(:result, result)
end
- # rubocop: enable CodeReuse/ActiveRecord
end
end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 621ac030783..ee1d49be690 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -5719,6 +5719,31 @@ Input type: `MergeRequestUpdateApprovalRuleInput`
| <a id="mutationmergerequestupdateapprovalruleerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationmergerequestupdateapprovalrulemergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
+### `Mutation.mlModelCreate`
+
+WARNING:
+**Introduced** in 16.8.
+This feature is an Experiment. It can be changed or removed at any time.
+
+Input type: `MlModelCreateInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationmlmodelcreateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationmlmodelcreatedescription"></a>`description` | [`String`](#string) | Description of the model. |
+| <a id="mutationmlmodelcreatename"></a>`name` | [`String!`](#string) | Name of the model. |
+| <a id="mutationmlmodelcreateprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the model to mutate is in. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationmlmodelcreateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationmlmodelcreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationmlmodelcreatemodel"></a>`model` | [`MlModel`](#mlmodel) | Model after mutation. |
+
### `Mutation.namespaceBanDestroy`
Input type: `NamespaceBanDestroyInput`
@@ -23051,6 +23076,7 @@ Machine learning model in the model registry.
| <a id="mlmodel_links"></a>`_links` | [`MLModelLinks!`](#mlmodellinks) | Map of links to perform actions on the model. |
| <a id="mlmodelcandidates"></a>`candidates` | [`MlCandidateConnection`](#mlcandidateconnection) | Version candidates of the model. (see [Connections](#connections)) |
| <a id="mlmodelcreatedat"></a>`createdAt` | [`Time!`](#time) | Date of creation. |
+| <a id="mlmodeldescription"></a>`description` | [`String!`](#string) | Description of the model. |
| <a id="mlmodelid"></a>`id` | [`MlModelID!`](#mlmodelid) | ID of the model. |
| <a id="mlmodellatestversion"></a>`latestVersion` | [`MlModelVersion`](#mlmodelversion) | Latest version of the model. |
| <a id="mlmodelname"></a>`name` | [`String!`](#string) | Name of the model. |
diff --git a/doc/user/analytics/repository_analytics.md b/doc/user/analytics/repository_analytics.md
index 93171dc3136..8bc163f6f3f 100644
--- a/doc/user/analytics/repository_analytics.md
+++ b/doc/user/analytics/repository_analytics.md
@@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
Use repository analytics to view information about a project's Git repository:
-- Programming languages used in the repository.
+- Programming languages used in the repository's default branch.
- Code coverage history from last 3 months ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/33743) in GitLab 13.1).
- Commit statistics (last month).
- Commits per day of month.
diff --git a/lib/api/ml/mlflow/registered_models.rb b/lib/api/ml/mlflow/registered_models.rb
index a68a2767a74..3f4996a94c0 100644
--- a/lib/api/ml/mlflow/registered_models.rb
+++ b/lib/api/ml/mlflow/registered_models.rb
@@ -31,13 +31,17 @@ module API
optional :tags, type: Array, desc: 'Additional metadata for registered model.'
end
post 'create', urgency: :low do
- present ::Ml::CreateModelService.new(
+ model = ::Ml::CreateModelService.new(
user_project,
params[:name],
current_user,
params[:description],
params[:tags]
- ).execute,
+ ).execute
+
+ resource_already_exists! unless model.persisted?
+
+ present model,
with: Entities::Ml::Mlflow::RegisteredModel,
root: :registered_model
rescue ActiveRecord::RecordInvalid
diff --git a/spec/graphql/types/ml/model_type_spec.rb b/spec/graphql/types/ml/model_type_spec.rb
index 79edea29eb8..078391f135a 100644
--- a/spec/graphql/types/ml/model_type_spec.rb
+++ b/spec/graphql/types/ml/model_type_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe GitlabSchema.types['MlModel'], feature_category: :mlops do
specify { expect(described_class.description).to eq('Machine learning model in the model registry') }
it 'includes all the package fields' do
- expected_fields = %w[id name versions candidates version_count _links created_at latest_version]
+ expected_fields = %w[id name versions candidates version_count _links created_at latest_version description]
expect(described_class).to include_graphql_fields(*expected_fields)
end
diff --git a/spec/helpers/ci/status_helper_spec.rb b/spec/helpers/ci/status_helper_spec.rb
index 502a535e102..a00a80ac06b 100644
--- a/spec/helpers/ci/status_helper_spec.rb
+++ b/spec/helpers/ci/status_helper_spec.rb
@@ -8,19 +8,6 @@ RSpec.describe Ci::StatusHelper do
let(:success_commit) { double("Ci::Pipeline", status: 'success') }
let(:failed_commit) { double("Ci::Pipeline", status: 'failed') }
- describe "#pipeline_status_cache_key" do
- it "builds a cache key for pipeline status" do
- pipeline_status = Gitlab::Cache::Ci::ProjectPipelineStatus.new(
- build_stubbed(:project),
- pipeline_info: {
- sha: "123abc",
- status: "success"
- }
- )
- expect(helper.pipeline_status_cache_key(pipeline_status)).to eq("pipeline-status/123abc-success")
- end
- end
-
describe "#render_ci_icon" do
subject { helper.render_ci_icon("success") }
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 32303f76684..38b1cf35bbf 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -1095,6 +1095,32 @@ RSpec.describe Member, feature_category: :groups_and_projects do
end
end
+ context 'when after_update :post_update_hook' do
+ let_it_be(:member) { create(:group_member, :developer) }
+
+ context 'when access_level is changed' do
+ it 'calls NotificationService.update_member' do
+ expect(NotificationService).to receive_message_chain(:new, :updated_member_access_level).with(member)
+
+ member.update_attribute(:access_level, Member::MAINTAINER)
+ end
+
+ it 'does not send an email when the access level has not changed' do
+ expect(NotificationService).not_to receive(:new)
+
+ member.touch
+ end
+ end
+
+ context 'when expiration is changed' do
+ it 'calls the notification service when membership expiry has changed' do
+ expect(NotificationService).to receive_message_chain(:new, :updated_member_expiration).with(member)
+
+ member.update!(expires_at: 5.days.from_now)
+ end
+ end
+ end
+
describe 'log_invitation_token_cleanup' do
let_it_be(:project) { create :project }
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index 7307e76272d..a7508f0e616 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -202,18 +202,6 @@ RSpec.describe GroupMember, feature_category: :cell do
end
end
- context 'when group member expiration date is updated' do
- let_it_be(:group_member) { create(:group_member) }
-
- it 'emails the user that their group membership expiry has changed' do
- expect_next_instance_of(NotificationService) do |notification|
- allow(notification).to receive(:updated_group_member_expiration).with(group_member)
- end
-
- group_member.update!(expires_at: 5.days.from_now)
- end
- end
-
describe 'refresh_member_authorized_projects' do
context 'when importing' do
it 'does not refresh' do
diff --git a/spec/requests/api/graphql/mutations/ml/models/create_spec.rb b/spec/requests/api/graphql/mutations/ml/models/create_spec.rb
new file mode 100644
index 00000000000..0daabeab0d1
--- /dev/null
+++ b/spec/requests/api/graphql/mutations/ml/models/create_spec.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Creation of a machine learning model', feature_category: :mlops do
+ include GraphqlHelpers
+
+ let_it_be(:model) { create(:ml_models) }
+ let_it_be(:project) { model.project }
+ let_it_be(:current_user) { project.owner }
+
+ let(:input) { { project_path: project.full_path, name: name, description: description } }
+ let(:name) { 'some_name' }
+ let(:description) { 'A description' }
+
+ let(:mutation) { graphql_mutation(:ml_model_create, input) }
+ let(:mutation_response) { graphql_mutation_response(:ml_model_create) }
+
+ context 'when user is not allowed write changes' do
+ before do
+ allow(Ability).to receive(:allowed?).and_call_original
+ allow(Ability).to receive(:allowed?)
+ .with(current_user, :write_model_registry, project)
+ .and_return(false)
+ end
+
+ it_behaves_like 'a mutation that returns a top-level access error'
+ end
+
+ context 'when user is allowed write changes' do
+ it 'creates a models' do
+ post_graphql_mutation(mutation, current_user: current_user)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(mutation_response['model']).to include(
+ 'name' => name,
+ 'description' => description
+ )
+ end
+
+ context 'when name already exists' do
+ err_msg = "Name has already been taken"
+ let(:name) { model.name }
+
+ it_behaves_like 'a mutation that returns errors in the response', errors: [err_msg]
+ end
+ end
+end
diff --git a/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb b/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb
new file mode 100644
index 00000000000..eb9324fd24b
--- /dev/null
+++ b/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb
@@ -0,0 +1,169 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ClickHouse::SyncStrategies::BaseSyncStrategy, feature_category: :value_stream_management do
+ let(:strategy) { described_class.new }
+
+ describe '#execute' do
+ subject(:execute) { strategy.execute }
+
+ context 'when clickhouse configuration database is available', :click_house do
+ before do
+ allow(strategy).to receive(:model_class).and_return(::Event)
+ allow(strategy).to receive(:projections).and_return([:id])
+ allow(strategy).to receive(:csv_mapping).and_return({ id: :id })
+ allow(strategy).to receive(:insert_query).and_return("INSERT INTO events (id) SETTINGS async_insert=1,
+ wait_for_async_insert=1 FORMAT CSV")
+ end
+
+ context 'when there is nothing to sync' do
+ it 'adds metadata for the worker' do
+ expect(execute).to eq({ status: :processed, records_inserted: 0, reached_end_of_table: true })
+
+ events = ClickHouse::Client.select('SELECT * FROM events', :main)
+ expect(events).to be_empty
+ end
+ end
+
+ context 'when syncing records' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, group: group) }
+ let_it_be(:issue) { create(:issue, project: project) }
+ let_it_be(:project_event2) { create(:event, :closed, project: project, target: issue) }
+ let_it_be(:event_without_parent) { create(:event, :joined, project: nil, group: nil) }
+ let_it_be(:group_event) { create(:event, :created, group: group, project: nil) }
+ let_it_be(:project_event1) { create(:event, :created, project: project, target: issue) }
+
+ it 'inserts all records' do
+ expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true })
+
+ expected_records = [
+ hash_including('id' => project_event2.id),
+ hash_including('id' => event_without_parent.id),
+ hash_including('id' => group_event.id),
+ hash_including('id' => project_event1.id)
+ ]
+
+ events = ClickHouse::Client.select('SELECT * FROM events ORDER BY id', :main)
+
+ expect(events).to match(expected_records)
+
+ last_processed_id = ClickHouse::SyncCursor.cursor_for(:events)
+ expect(last_processed_id).to eq(project_event1.id)
+ end
+
+ context 'when multiple batches are needed' do
+ before do
+ stub_const("#{described_class}::BATCH_SIZE", 1)
+ stub_const("#{described_class}::INSERT_BATCH_SIZE", 1)
+ end
+
+ it 'inserts all records' do
+ expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true })
+
+ events = ClickHouse::Client.select('SELECT * FROM events', :main)
+ expect(events.size).to eq(4)
+ end
+
+ context 'when new records are inserted while processing' do
+ it 'does not process new records created during the iteration' do
+ # Simulating the case when there is an insert during the iteration
+ call_count = 0
+ allow(strategy).to receive(:next_batch).and_wrap_original do |method|
+ call_count += 1
+ create(:event) if call_count == 3
+ method.call
+ end
+
+ expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true })
+ end
+ end
+ end
+
+ context 'when time limit is reached' do
+ before do
+ stub_const("#{described_class}::BATCH_SIZE", 1)
+ end
+
+ it 'stops the processing' do
+ allow_next_instance_of(Analytics::CycleAnalytics::RuntimeLimiter) do |runtime_limiter|
+ allow(runtime_limiter).to receive(:over_time?).and_return(false, true)
+ end
+
+ expect(execute).to eq({ status: :processed, records_inserted: 2, reached_end_of_table: false })
+
+ last_processed_id = ClickHouse::SyncCursor.cursor_for(:events)
+ expect(last_processed_id).to eq(event_without_parent.id)
+ end
+ end
+
+ context 'when syncing from a certain point' do
+ before do
+ ClickHouse::SyncCursor.update_cursor_for(:events, project_event2.id)
+ end
+
+ it 'syncs records after the cursor' do
+ expect(execute).to eq({ status: :processed, records_inserted: 3, reached_end_of_table: true })
+
+ events = ClickHouse::Client.select('SELECT id FROM events ORDER BY id', :main)
+
+ expect(events).to eq([{ 'id' => event_without_parent.id }, { 'id' => group_event.id },
+ { 'id' => project_event1.id }])
+ end
+
+ context 'when there is nothing to sync' do
+ it 'does nothing' do
+ ClickHouse::SyncCursor.update_cursor_for(:events, project_event1.id)
+
+ expect(execute).to eq({ status: :processed, records_inserted: 0, reached_end_of_table: true })
+
+ events = ClickHouse::Client.select('SELECT id FROM events ORDER BY id', :main)
+ expect(events).to be_empty
+ end
+ end
+ end
+ end
+ end
+
+ context 'when clickhouse is not configured' do
+ before do
+ allow(ClickHouse::Client.configuration).to receive(:databases).and_return({})
+ end
+
+ it 'skips execution' do
+ expect(execute).to eq({ status: :disabled })
+ end
+ end
+
+ context 'when exclusive lease error happens' do
+ it 'skips execution' do
+ allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db })
+
+ expect(strategy).to receive(:in_lock).and_raise(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError)
+ expect(execute).to eq({ status: :skipped })
+ end
+ end
+ end
+
+ describe '#projections' do
+ it 'raises a NotImplementedError' do
+ expect { strategy.send(:projections) }.to raise_error(NotImplementedError,
+ "Subclasses must implement `projections`")
+ end
+ end
+
+ describe '#csv_mapping' do
+ it 'raises a NotImplementedError' do
+ expect { strategy.send(:csv_mapping) }.to raise_error(NotImplementedError,
+ "Subclasses must implement `csv_mapping`")
+ end
+ end
+
+ describe '#insert_query' do
+ it 'raises a NotImplementedError' do
+ expect { strategy.send(:insert_query) }.to raise_error(NotImplementedError,
+ "Subclasses must implement `insert_query`")
+ end
+ end
+end
diff --git a/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb b/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb
new file mode 100644
index 00000000000..05fcf6ddeb3
--- /dev/null
+++ b/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb
@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ClickHouse::SyncStrategies::EventSyncStrategy, feature_category: :value_stream_management do
+ let(:strategy) { described_class.new }
+
+ describe '#execute' do
+ subject(:execute) { strategy.execute }
+
+ context 'when syncing records', :click_house do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, group: group) }
+ let_it_be(:issue) { create(:issue, project: project) }
+ let_it_be(:project_event2) { create(:event, :closed, project: project, target: issue) }
+ let_it_be(:event_without_parent) { create(:event, :joined, project: nil, group: nil) }
+ let_it_be(:group_event) { create(:event, :created, group: group, project: nil) }
+ let_it_be(:project_event1) { create(:event, :created, project: project, target: issue) }
+ # looks invalid but we have some records like this on PRD
+
+ it 'correctly inserts all records' do
+ expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true })
+
+ expected_records = [
+ hash_including('id' => project_event2.id, 'path' => "#{group.id}/#{project.project_namespace.id}/",
+ 'target_type' => 'Issue'),
+ hash_including('id' => event_without_parent.id, 'path' => '', 'target_type' => ''),
+ hash_including('id' => group_event.id, 'path' => "#{group.id}/", 'target_type' => ''),
+ hash_including('id' => project_event1.id, 'path' => "#{group.id}/#{project.project_namespace.id}/",
+ 'target_type' => 'Issue')
+ ]
+
+ events = ClickHouse::Client.select('SELECT * FROM events ORDER BY id', :main)
+
+ expect(events).to match(expected_records)
+
+ last_processed_id = ClickHouse::SyncCursor.cursor_for(:events)
+ expect(last_processed_id).to eq(project_event1.id)
+ end
+ end
+ end
+
+ describe '#projections' do
+ it 'returns correct projections' do
+ expect(strategy.send(:projections)).to match_array([
+ :id,
+ described_class::PATH_COLUMN,
+ :author_id,
+ :target_id,
+ :target_type,
+ 'action AS raw_action',
+ 'EXTRACT(epoch FROM created_at) AS casted_created_at',
+ 'EXTRACT(epoch FROM updated_at) AS casted_updated_at'
+ ])
+ end
+ end
+
+ describe '#csv_mapping' do
+ it 'returns correct csv mapping' do
+ expect(strategy.send(:csv_mapping)).to eq({
+ id: :id,
+ path: :path,
+ author_id: :author_id,
+ target_id: :target_id,
+ target_type: :target_type,
+ action: :raw_action,
+ created_at: :casted_created_at,
+ updated_at: :casted_updated_at
+ })
+ end
+ end
+
+ describe '#insert_query' do
+ let(:expected_query) do
+ <<~SQL.squish
+ INSERT INTO events (id, path, author_id,
+ target_id, target_type,
+ action, created_at, updated_at)
+ SETTINGS async_insert=1,
+ wait_for_async_insert=1 FORMAT CSV
+ SQL
+ end
+
+ it 'returns correct insert query' do
+ expect(strategy.send(:insert_query)).to eq(expected_query)
+ end
+ end
+
+ describe '#model_class' do
+ it 'returns the correct model class' do
+ expect(strategy.send(:model_class)).to eq(::Event)
+ end
+ end
+
+ describe '#enabled?' do
+ context 'when the clickhouse database is configured the feature flag is enabled' do
+ before do
+ allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db })
+ stub_feature_flags(event_sync_worker_for_click_house: true)
+ end
+
+ it 'returns true' do
+ expect(strategy.send(:enabled?)).to be_truthy
+ end
+ end
+
+ context 'when the clickhouse database is not configured' do
+ before do
+ allow(ClickHouse::Client.configuration).to receive(:databases).and_return({})
+ end
+
+ it 'returns false' do
+ expect(strategy.send(:enabled?)).to be_falsey
+ end
+ end
+
+ context 'when the feature flag is disabled' do
+ before do
+ allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db })
+ stub_feature_flags(event_sync_worker_for_click_house: false)
+ end
+
+ it 'returns false' do
+ expect(strategy.send(:enabled?)).to be_falsey
+ end
+ end
+ end
+end
diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb
index 3860543a85e..b23f5856575 100644
--- a/spec/services/members/update_service_spec.rb
+++ b/spec/services/members/update_service_spec.rb
@@ -263,7 +263,7 @@ RSpec.describe Members::UpdateService, feature_category: :groups_and_projects do
it 'emails the users that their group membership expiry has changed' do
members.each do |member|
- expect(notification_service).to receive(:updated_group_member_expiration).with(member)
+ expect(notification_service).to receive(:updated_member_expiration).with(member)
end
subject
diff --git a/spec/services/ml/create_model_service_spec.rb b/spec/services/ml/create_model_service_spec.rb
index 74c1dd5fec7..88e7c00d1f9 100644
--- a/spec/services/ml/create_model_service_spec.rb
+++ b/spec/services/ml/create_model_service_spec.rb
@@ -50,9 +50,10 @@ RSpec.describe ::Ml::CreateModelService, feature_category: :mlops do
let(:name) { existing_model.name }
let(:project) { existing_model.project }
- it 'raises an error', :aggregate_failures do
- expect { create_model }.to raise_error(ActiveRecord::RecordInvalid)
+ it 'returns a model with errors', :aggregate_failures do
+ expect(create_model).not_to be_persisted
expect(Gitlab::InternalEvents).not_to have_received(:track_event)
+ expect(create_model.errors.full_messages).to eq(["Name has already been taken"])
end
end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 0012cc112c1..910e1d85a6b 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -3319,18 +3319,6 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do
let(:notification_trigger) { group.add_guest(added_user) }
end
end
-
- describe '#updated_group_member_expiration' do
- let_it_be(:group_member) { create(:group_member) }
-
- it 'emails the user that their group membership expiry has changed' do
- expect_next_instance_of(NotificationService) do |notification|
- allow(notification).to receive(:updated_group_member_expiration).with(group_member)
- end
-
- group_member.update!(expires_at: 5.days.from_now)
- end
- end
end
describe 'ProjectMember', :deliver_mails_inline do
@@ -3509,6 +3497,42 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do
end
end
+ describe '#updated_member_expiration' do
+ subject(:updated_member_expiration) { notification.updated_member_expiration(member) }
+
+ context 'for group member' do
+ let_it_be(:member) { create(:group_member) }
+
+ it 'triggers a notification about the expiration change' do
+ updated_member_expiration
+
+ expect_delivery_jobs_count(1)
+ expect_enqueud_email('Group', member.id, mail: 'member_expiration_date_updated_email')
+ end
+ end
+
+ context 'for project member' do
+ let_it_be(:member) { create(:project_member) }
+
+ it 'does not trigger a notification' do
+ updated_member_expiration
+
+ expect_delivery_jobs_count(0)
+ end
+ end
+ end
+
+ describe '#updated_member_access_level' do
+ let_it_be(:member) { create(:group_member) }
+
+ it 'triggers a notification about the access_level change' do
+ notification.updated_member_access_level(member)
+
+ expect_delivery_jobs_count(1)
+ expect_enqueud_email('Group', member.id, mail: 'member_access_granted_email')
+ end
+ end
+
context 'guest user in private project', :deliver_mails_inline do
let(:private_project) { create(:project, :private) }
let(:guest) { create(:user) }
diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml
index 56a28004233..0715e56c130 100644
--- a/spec/support/rspec_order_todo.yml
+++ b/spec/support/rspec_order_todo.yml
@@ -1691,7 +1691,7 @@
- './ee/spec/models/gitlab_subscription_spec.rb'
- './ee/spec/models/gitlab_subscriptions/upcoming_reconciliation_spec.rb'
- './ee/spec/models/group_deletion_schedule_spec.rb'
-- './ee/spec/models/group_member_spec.rb'
+- './ee/spec/models/ee/group_member_spec.rb'
- './ee/spec/models/group_merge_request_approval_setting_spec.rb'
- './ee/spec/models/groups/repository_storage_move_spec.rb'
- './ee/spec/models/group_wiki_repository_spec.rb'
@@ -1715,7 +1715,7 @@
- './ee/spec/models/label_note_spec.rb'
- './ee/spec/models/ldap_group_link_spec.rb'
- './ee/spec/models/license_spec.rb'
-- './ee/spec/models/member_spec.rb'
+- './ee/spec/models/ee/member_spec.rb'
- './ee/spec/models/merge_request/blocking_spec.rb'
- './ee/spec/models/merge_request_block_spec.rb'
- './ee/spec/models/merge_requests/compliance_violation_spec.rb'
@@ -1740,7 +1740,7 @@
- './ee/spec/models/project_feature_spec.rb'
- './ee/spec/models/project_import_data_spec.rb'
- './ee/spec/models/project_import_state_spec.rb'
-- './ee/spec/models/project_member_spec.rb'
+- './ee/spec/models/ee/project_member_spec.rb'
- './ee/spec/models/project_repository_state_spec.rb'
- './ee/spec/models/project_security_setting_spec.rb'
- './ee/spec/models/protected_branch/required_code_owners_section_spec.rb'
diff --git a/spec/support/shared_examples/models/members_notifications_shared_example.rb b/spec/support/shared_examples/models/members_notifications_shared_example.rb
index 5c783b5cfa7..17dc927f3cc 100644
--- a/spec/support/shared_examples/models/members_notifications_shared_example.rb
+++ b/spec/support/shared_examples/models/members_notifications_shared_example.rb
@@ -19,22 +19,6 @@ RSpec.shared_examples 'members notifications' do |entity_type|
end
end
- describe "#after_update" do
- let(:member) { create(:"#{entity_type}_member", :developer) }
-
- it "calls NotificationService.update_#{entity_type}_member" do
- expect(notification_service).to receive(:"update_#{entity_type}_member").with(member)
-
- member.update_attribute(:access_level, Member::MAINTAINER)
- end
-
- it "does not send an email when the access level has not changed" do
- expect(notification_service).not_to receive(:"update_#{entity_type}_member")
-
- member.touch
- end
- end
-
describe '#after_commit' do
context 'on creation of a member requesting access' do
let(:member) do
diff --git a/spec/workers/click_house/events_sync_worker_spec.rb b/spec/workers/click_house/events_sync_worker_spec.rb
index 9662f26115a..dc3dea24e37 100644
--- a/spec/workers/click_house/events_sync_worker_spec.rb
+++ b/spec/workers/click_house/events_sync_worker_spec.rb
@@ -11,176 +11,20 @@ RSpec.describe ClickHouse::EventsSyncWorker, feature_category: :value_stream_man
)
end
- it_behaves_like 'an idempotent worker' do
- context 'when the event_sync_worker_for_click_house feature flag is on', :click_house do
- before do
- stub_feature_flags(event_sync_worker_for_click_house: true)
+ context 'when worker is enqueued' do
+ it 'calls ::ClickHouse::SyncStrategies::EventSyncStrategy with correct args' do
+ expect_next_instance_of(::ClickHouse::SyncStrategies::EventSyncStrategy) do |instance|
+ expect(instance).to receive(:execute)
end
- context 'when there is nothing to sync' do
- it 'adds metadata for the worker' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 0, reached_end_of_table: true })
-
- worker.perform
-
- events = ClickHouse::Client.select('SELECT * FROM events', :main)
- expect(events).to be_empty
- end
- end
-
- context 'when syncing records' do
- let_it_be(:group) { create(:group) }
- let_it_be(:project) { create(:project, group: group) }
- let_it_be(:issue) { create(:issue, project: project) }
- let_it_be(:project_event2) { create(:event, :closed, project: project, target: issue) }
- let_it_be(:event_without_parent) { create(:event, :joined, project: nil, group: nil) }
- let_it_be(:group_event) { create(:event, :created, group: group, project: nil) }
- let_it_be(:project_event1) { create(:event, :created, project: project, target: issue) }
- # looks invalid but we have some records like this on PRD
-
- it 'inserts all records' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 4, reached_end_of_table: true })
-
- worker.perform
-
- expected_records = [
- hash_including('id' => project_event2.id, 'path' => "#{group.id}/#{project.project_namespace.id}/",
- 'target_type' => 'Issue'),
- hash_including('id' => event_without_parent.id, 'path' => '', 'target_type' => ''),
- hash_including('id' => group_event.id, 'path' => "#{group.id}/", 'target_type' => ''),
- hash_including('id' => project_event1.id, 'path' => "#{group.id}/#{project.project_namespace.id}/",
- 'target_type' => 'Issue')
- ]
-
- events = ClickHouse::Client.select('SELECT * FROM events ORDER BY id', :main)
-
- expect(events).to match(expected_records)
-
- last_processed_id = ClickHouse::SyncCursor.cursor_for(:events)
- expect(last_processed_id).to eq(project_event1.id)
- end
-
- context 'when multiple batches are needed' do
- before do
- stub_const("#{described_class}::BATCH_SIZE", 1)
- stub_const("#{described_class}::INSERT_BATCH_SIZE", 1)
- end
-
- it 'inserts all records' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 4, reached_end_of_table: true })
-
- worker.perform
-
- events = ClickHouse::Client.select('SELECT * FROM events', :main)
- expect(events.size).to eq(4)
- end
-
- context 'when new records are inserted while processing' do
- it 'does not process new records created during the iteration' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 4,
- reached_end_of_table: true })
-
- # Simulating the case when there is an insert during the iteration
- call_count = 0
- allow(worker).to receive(:next_batch).and_wrap_original do |method|
- call_count += 1
- create(:event) if call_count == 3
- method.call
- end
-
- worker.perform
- end
- end
- end
-
- context 'when time limit is reached' do
- before do
- stub_const("#{described_class}::BATCH_SIZE", 1)
- end
-
- it 'stops the processing' do
- allow_next_instance_of(Analytics::CycleAnalytics::RuntimeLimiter) do |runtime_limiter|
- allow(runtime_limiter).to receive(:over_time?).and_return(false, true)
- end
-
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 2, reached_end_of_table: false })
-
- worker.perform
-
- last_processed_id = ClickHouse::SyncCursor.cursor_for(:events)
- expect(last_processed_id).to eq(event_without_parent.id)
- end
- end
-
- context 'when syncing from a certain point' do
- before do
- ClickHouse::SyncCursor.update_cursor_for(:events, project_event2.id)
- end
-
- it 'syncs records after the cursor' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 3, reached_end_of_table: true })
-
- worker.perform
-
- events = ClickHouse::Client.select('SELECT id FROM events ORDER BY id', :main)
- expect(events).to eq([{ 'id' => event_without_parent.id }, { 'id' => group_event.id },
- { 'id' => project_event1.id }])
- end
-
- context 'when there is nothing to sync' do
- it 'does nothing' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result,
- { status: :processed, records_inserted: 0, reached_end_of_table: true })
-
- ClickHouse::SyncCursor.update_cursor_for(:events, project_event1.id)
- worker.perform
-
- events = ClickHouse::Client.select('SELECT id FROM events ORDER BY id', :main)
- expect(events).to be_empty
- end
- end
- end
- end
- end
-
- context 'when clickhouse is not configured' do
- before do
- allow(ClickHouse::Client).to receive(:database_configured?).and_return(false)
- end
-
- it 'skips execution' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result, { status: :disabled })
-
- worker.perform
- end
- end
- end
-
- context 'when exclusive lease error happens' do
- it 'skips execution' do
- stub_feature_flags(event_sync_worker_for_click_house: true)
- allow(ClickHouse::Client).to receive(:database_configured?).with(:main).and_return(true)
-
- expect(worker).to receive(:in_lock).and_raise(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError)
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result, { status: :skipped })
-
worker.perform
end
- end
- context 'when the event_sync_worker_for_click_house feature flag is off' do
- before do
- stub_feature_flags(event_sync_worker_for_click_house: false)
- end
-
- it 'skips execution' do
- expect(worker).to receive(:log_extra_metadata_on_done).with(:result, { status: :disabled })
+ it 'correctly logs the metadata on done' do
+ expect_next_instance_of(::ClickHouse::SyncStrategies::EventSyncStrategy) do |instance|
+ expect(instance).to receive(:execute).and_return({ status: :ok })
+ end
+ expect(worker).to receive(:log_extra_metadata_on_done).with(:result, { status: :ok })
worker.perform
end