diff --git a/app/models/cells/outstanding_lease.rb b/app/models/cells/outstanding_lease.rb index f74763e6b828aee1a164b14748573b816477a59a..d9f3701668f53ad1d7b9d1b1f0329ac33f4c191a 100644 --- a/app/models/cells/outstanding_lease.rb +++ b/app/models/cells/outstanding_lease.rb @@ -3,5 +3,50 @@ module Cells class OutstandingLease < ApplicationRecord self.primary_key = :uuid + + def self.claim_service + ::Gitlab::TopologyServiceClient::ClaimService.instance # rubocop:disable CodeReuse/ServiceClass -- this is a gRPC client + end + + def self.create_from_request!(create_records:, destroy_records:, deadline: nil) + req = Gitlab::Cells::TopologyService::Claims::V1::BeginUpdateRequest.new( + create_records: create_records, + destroy_records: destroy_records, + cell_id: claim_service.cell_id + ) + + pp("send_begin_update req #{req}") + res = claim_service.begin_update(req, deadline: deadline) + + pp("send_begin_update res #{res.lease_uuid}") + create!(uuid: res.lease_uuid.value) + end + + def send_commit_update!(deadline: nil) + req = Gitlab::Cells::TopologyService::Claims::V1::CommitUpdateRequest.new( + lease_uuid: Gitlab::Cells::TopologyService::Types::V1::UUID.new(value: uuid), + cell_id: self.class.claim_service.cell_id + ) + + pp("send_commit_update #{object_id} #{req}") + self.class.claim_service.commit_update(req, deadline: deadline) + + pp("send_commit_update done #{object_id} #{req}") + end + + def send_rollback_update!(deadline: nil) + return unless uuid + + req = Gitlab::Cells::TopologyService::Claims::V1::RollbackUpdateRequest.new( + lease_uuid: Gitlab::Cells::TopologyService::Types::V1::UUID.new(value: uuid), + cell_id: self.class.claim_service.cell_id + ) + + pp("send_rollback_update #{object_id} #{req}") + + self.class.claim_service.rollback_update(req, deadline: deadline) + + pp("send_rollback_update done #{object_id} #{req}") + end end end diff --git a/app/models/cells/transaction_record.rb b/app/models/cells/transaction_record.rb new file mode 100644 index 0000000000000000000000000000000000000000..ce052761c3965ac3e86d37d6e521c1283c4104d6 --- /dev/null +++ b/app/models/cells/transaction_record.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +module Cells + # This class is a fake ActiveRecord::Base which implements the interface + # partially so that it can be used with + # ActiveRecord::ConnectionAdapters::Transaction#add_record + # https://github.com/rails/rails/blob/v7.1.5.2/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L147-L155 + # From there, several methods are required to be implemented: + # * #before_committed! + # * Normally implemented by ActiveRecord::Transactions + # * #committed! + # * Normally implemented by ActiveRecord::Transactions + # * #rolledback! + # * Normally implemented by ActiveRecord::Transactions + # * #trigger_transactional_callbacks? + # * Normally implemented by ActiveRecord::Transactions + # * Overridden by Cells::TransactionRecord to avoid more interfaces needed + # * #destroyed? + # * Normally implemented by ActiveRecord::Persistence but we ignore it + # * #_new_record_before_last_commit + # * Normally implemented by ActiveRecord::Transactions + # We ignore some methods because they won't be called under specific + # conditions, specifically when the following both return true: + # * #trigger_transactional_callbacks? + class TransactionRecord + module TransactionExtension + attr_accessor :cells_current_transaction_record + end + + TIMEOUT_IN_SECONDS = 0.2 + + # TODO: hack to assign to current transaction + # Figure out a better way to do this + # Current? but what if nested transactions? + # what is the transactions gets rolled back? + def self.current_transaction(connection) + return unless Current.cells_claims_leases? + + # The Cells::OutstandingLease requires a transaction to be open + # to ensure that the lease is only created if the transaction + # within a transaction and not outside of one + if connection.current_transaction.closed? # rubocop:disable Style/IfUnlessModifier -- this is more clear + raise 'The Cells::TransactionRecord requires transaction to be open' + end + + current_transaction = connection.current_transaction + + instance = current_transaction.cells_current_transaction_record + return instance if instance + + TransactionRecord.new(connection, current_transaction).tap do |instance| + current_transaction.cells_current_transaction_record = instance + # https://api.rubyonrails.org/v7.1.5.2/classes/ActiveRecord/ConnectionAdapters/DatabaseStatements.html#method-i-add_transaction_record + # https://github.com/rails/rails/blob/v7.1.5.2/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L147-L155 + current_transaction.add_record(instance) + end + end + + def initialize(connection, transaction) + @connection = connection + @transaction = transaction + @create_records = [] + @destroy_records = [] + @outstanding_lease = nil + @done = false + @deadline = GRPC::Core::TimeConsts.from_relative_time(TIMEOUT_IN_SECONDS) + end + + def create_record(metadata) + raise 'Lease already created' if @outstanding_lease + + @create_records << metadata + pp("create_record #{object_id} #{metadata}") + end + + def destroy_record(metadata) + raise 'Lease already created' if @outstanding_lease + + @destroy_records << metadata + pp("destroy_record #{object_id} #{metadata}") + end + + # Always trigger callbacks. See: + # ActiveRecord::ConnectionAdapters::Transaction# + # prepare_instances_to_run_callbacks_on + # https://github.com/rails/rails/blob/v7.1.5.2/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L269 + def trigger_transactional_callbacks? + true + end + + def before_committed! + raise 'Already done' if @done + raise 'Already created lease' if @outstanding_lease + raise 'Cross-database lease' if Cells::OutstandingLease.connection != @connection + + @outstanding_lease = Cells::OutstandingLease.create_from_request!( + create_records: @create_records, + destroy_records: @destroy_records, + deadline: @deadline + ) + end + + def rolledback!(force_restore_state: false, should_run_callbacks: true) # rubocop:disable Lint/UnusedMethodArgument -- this needs to follow the interface + raise 'Already done' if @done + + # It is possible that lease might be not created yet, + # since the transaction might be rolledback prematurely + return unless @outstanding_lease + + @outstanding_lease.send_rollback_update!(deadline: @deadline) + @outstanding_lease.destroy! # the lease is no longer needed + @done = true + end + + def committed!(should_run_callbacks: true) # rubocop:disable Lint/UnusedMethodArgument -- this needs to follow the interface + raise 'Already done' if @done + raise 'No lease created' unless @outstanding_lease + + @outstanding_lease.send_commit_update!(deadline: @deadline) + @outstanding_lease.destroy! # the lease is no longer needed + @done = true + end + end +end diff --git a/app/models/concerns/cells/claimable.rb b/app/models/concerns/cells/claimable.rb new file mode 100644 index 0000000000000000000000000000000000000000..932b5a7471d4c5f42f6de9d3badedbfd0c4fd11c --- /dev/null +++ b/app/models/concerns/cells/claimable.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +module Cells + module Claimable + extend ActiveSupport::Concern + + CLAIMS_BUCKET_TYPE = Gitlab::Cells::TopologyService::Claims::V1::Bucket::Type + CLAIMS_SUBJECT_TYPE = Gitlab::Cells::TopologyService::Claims::V1::Subject::Type + CLAIMS_SOURCE_TYPE = Gitlab::Cells::TopologyService::Claims::V1::Source::Type + + included do + after_save :cells_claims_save_changes + before_destroy :cells_claims_destroy_changes + + class_attribute :cells_claims_subject_type, instance_accessor: false + class_attribute :cells_claims_subject_key, instance_accessor: false + class_attribute :cells_claims_source_type, instance_accessor: false + class_attribute :cells_claims_attributes, instance_accessor: false, default: {}.freeze + end + + class_methods do + def cells_claims_metadata(subject_type:, subject_key: nil, source_type: nil) + self.cells_claims_subject_type = subject_type + self.cells_claims_subject_key = subject_key || :organization_id + self.cells_claims_source_type = source_type || + Gitlab::Cells::TopologyService::Claims::V1::Source::Type + .const_get("RAILS_TABLE_#{table_name.upcase}", false) + end + + def cells_claims_attribute(name, type:) + self.cells_claims_attributes = cells_claims_attributes + .merge(name => { type: type }) + .freeze + end + end + + private + + def cells_claims_save_changes + transaction_record = ::Cells::TransactionRecord.current_transaction(connection) + return unless transaction_record + + pp("claim_save_changes #{object_id}: #{self.class.name} #{id}") + + default_metadata = cells_claims_default_metadata + + self.class.cells_claims_attributes.each do |attribute, config| + next unless saved_change_to_attribute?(attribute) + + was, is = saved_change_to_attribute(attribute) + pp(" attribute '#{attribute}' changed from '#{was}' to '#{is}'") + + if !was.nil? && was != is + transaction_record.destroy_record(default_metadata.merge({ + bucket: { + type: config[:type], + value: was + } + })) + end + + if !is.nil? # rubocop:disable Style/NegatedIf, Style/Next -- this is more clear + transaction_record.create_record(default_metadata.merge( + bucket: { + type: config[:type], + value: public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend -- developer hard coded + } + )) + end + end + end + + def cells_claims_destroy_changes + transaction_record = ::Cells::TransactionRecord.current_transaction(connection) + return unless transaction_record + + pp("claim_destroy_changes #{object_id}: #{self.class.name} #{id}") + + default_metadata = cells_claims_default_metadata + + self.class.cells_claims_attributes.each do |attribute, config| + transaction_record.destroy_record(default_metadata.merge( + bucket: { + type: config[:type], + value: public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend -- developer hard coded + } + )) + end + end + + def cells_claims_default_metadata + { + subject: { + type: self.class.cells_claims_subject_type, + id: public_send(self.class.cells_claims_subject_key) # rubocop:disable GitlabSecurity/PublicSend -- developer hard coded + }, + source: { + type: self.class.cells_claims_source_type, + rails_primary_key_id: public_send(self.class.primary_key) # rubocop:disable GitlabSecurity/PublicSend -- this should work by definition + } + } + end + end +end diff --git a/app/models/current.rb b/app/models/current.rb index 6747b08526f7a6ac650a7dbdfefabc9d474229b2..e9ee743554ba0483372b9737986af144ab9eb1c6 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -18,6 +18,18 @@ def message attribute :organization, :organization_assigned attribute :token_info + attribute :cells_claims_leases + + def cells_claims_leases? + if cells_claims_leases.nil? + self.cells_claims_leases = + Gitlab.config.cell.enabled && + Feature.enabled?(:cells_unique_claims) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- We can't easily tie this to an actor + end + + cells_claims_leases + end + def organization=(value) # We want to explicitly allow only one organization assignment per thread # This fits the request/response cycle, but of course for rake tasks/background jobs that use the same thread, diff --git a/app/models/email.rb b/app/models/email.rb index c2e6919322358d950a0bf416ad8617c468b25fbe..7ac7a51b525b267d5887890c7c9cf7f07cbfe1d9 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -3,6 +3,11 @@ class Email < ApplicationRecord include Sortable include Gitlab::SQL::Pattern + include Cells::Claimable + + cells_claims_attribute :email, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED # TODO: EMAIL? + + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::USER, subject_key: :user_id belongs_to :user, optional: false belongs_to :banned_user, class_name: '::Users::BannedUser', foreign_key: 'user_id', inverse_of: 'emails' diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 43c2a70b8670a45d96a21110a1ecf4d599f44c06..e55642d055446b737deb0796b44335a518ba13a4 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -5,13 +5,21 @@ class GpgKey < ApplicationRecord KEY_SUFFIX = '-----END PGP PUBLIC KEY BLOCK-----' include ShaAttribute + include Cells::Claimable + + cells_claims_attribute :key, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED + cells_claims_attribute :fingerprint, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED + cells_claims_attribute :primary_keyid, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED + + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::USER, source_type: CLAIMS_SOURCE_TYPE::UNSPECIFIED, + subject_key: :user_id sha_attribute :primary_keyid sha_attribute :fingerprint belongs_to :user has_many :gpg_signatures, class_name: 'CommitSignatures::GpgSignature' - has_many :subkeys, class_name: 'GpgKeySubkey' + has_many :subkeys, class_name: 'GpgKeySubkey', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- needed to unclaim scope :with_subkeys, -> { includes(:subkeys) } scope :externally_invalid, -> { where(externally_verified: false) } diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index 110bf45113652ebbe8b2ac4a7b3a1b90a1f8e6af..ff944898199cddb867309d2aa86260386b40aafc 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -2,6 +2,12 @@ class GpgKeySubkey < ApplicationRecord include ShaAttribute + include Cells::Claimable + + cells_claims_attribute :fingerprint, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED + + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::UNSPECIFIED, source_type: CLAIMS_SOURCE_TYPE::UNSPECIFIED, + subject_key: :gpg_key_id sha_attribute :keyid sha_attribute :fingerprint diff --git a/app/models/key.rb b/app/models/key.rb index 78c4518b46c33196bf4964d6ce7c6e915a0ea776..64152acea978d3285ab824c900057d2b243d4d12 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -8,6 +8,12 @@ class Key < ApplicationRecord include FromUnion include Todoable include CreatedAtFilterable + include Cells::Claimable + + cells_claims_attribute :key, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED # TODO: KEY? + cells_claims_attribute :fingerprint_sha256, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED # TODO: KEY_FINGERPRINT? + + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::UNSPECIFIED, source_type: CLAIMS_SOURCE_TYPE::UNSPECIFIED sha256_attribute :fingerprint_sha256 diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb index 37253257299f5169815ada4f00a11e1f73d4607f..eb97b2a4d3bc4c774019013228952ac748733f84 100644 --- a/app/models/organizations/organization.rb +++ b/app/models/organizations/organization.rb @@ -8,6 +8,11 @@ class Organization < ApplicationRecord include Gitlab::Routing.url_helpers include FeatureGate include Organizations::Isolatable + include Cells::Claimable + + cells_claims_attribute :path, type: CLAIMS_BUCKET_TYPE::ORGANIZATION_PATH + + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::ORGANIZATION DEFAULT_ORGANIZATION_ID = 1 diff --git a/app/models/route.rb b/app/models/route.rb index 7b12bdbadc1fc93cdb7ce2fc5ab4ddfa7226d3c7..e85d6ae9cfed6a7801d93c01e8d81fdc24794bde 100644 --- a/app/models/route.rb +++ b/app/models/route.rb @@ -4,6 +4,11 @@ class Route < ApplicationRecord include CaseSensitivity include Gitlab::SQL::Pattern include EachBatch + include Cells::Claimable + + cells_claims_attribute :path, type: CLAIMS_BUCKET_TYPE::ROUTES + + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::GROUP, subject_key: :namespace_id belongs_to :source, polymorphic: true, inverse_of: :route # rubocop:disable Cop/PolymorphicAssociations belongs_to :namespace, inverse_of: :namespace_route diff --git a/app/models/user.rb b/app/models/user.rb index b4403061ddafd6ce16c3b430e2fc3fe89807cfa7..f8d5ddcfeac0d8c8d1ec25a9bb71495c9b2bb605 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,6 +36,11 @@ class User < ApplicationRecord include Gitlab::InternalEventsTracking include Ci::PipelineScheduleOwnershipValidator include Users::DependentAssociations + include Cells::Claimable + + # cells_claims_attribute :email # This clashes with emails.email + cells_claims_attribute :username, type: CLAIMS_BUCKET_TYPE::UNSPECIFIED # TODO: USERNAME? + cells_claims_metadata subject_type: CLAIMS_SUBJECT_TYPE::USER ignore_column %i[role skype], remove_after: '2025-09-18', remove_with: '18.4' @@ -165,9 +170,9 @@ def update_tracked_fields!(request) has_many :expired_today_and_unnotified_keys, -> { expired_today_and_not_notified }, class_name: 'Key' has_many :expiring_soon_and_unnotified_keys, -> { expiring_soon_and_not_notified }, class_name: 'Key' has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent - has_many :gpg_keys + has_many :gpg_keys, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- needed to unclaim - has_many :emails + has_many :emails, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- needed to unclaim has_many :personal_access_tokens, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :expiring_soon_and_unnotified_personal_access_tokens, -> { expiring_and_not_notified_without_impersonation }, class_name: 'PersonalAccessToken' diff --git a/config/feature_flags/beta/cells_unique_claims.yml b/config/feature_flags/beta/cells_unique_claims.yml new file mode 100644 index 0000000000000000000000000000000000000000..da3d94089711b83307409413e7248ff8b6a0eb77 --- /dev/null +++ b/config/feature_flags/beta/cells_unique_claims.yml @@ -0,0 +1,10 @@ +--- +name: cells_unique_claims +description: +feature_issue_url: https://gitlab.com/gitlab-com/gl-infra/tenant-scale/cells-infrastructure/team/-/issues/365 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/200701 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/572819 +milestone: '18.5' +group: group::cells infrastructure +type: beta +default_enabled: false diff --git a/config/initializers/active_record_connection_adapters_transaction.rb b/config/initializers/active_record_connection_adapters_transaction.rb new file mode 100644 index 0000000000000000000000000000000000000000..049a5103e357a3749ca8e13eb1e75f8fe43032cd --- /dev/null +++ b/config/initializers/active_record_connection_adapters_transaction.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +if Rails.gem_version >= Gem::Version.new('7.2') + raise 'Given that we are relying on a few Rails internal in `app/models/cells/transaction_record.rb`, we should verify if the contract still holds. If it does, please bump the version here.' +end + +ActiveRecord::ConnectionAdapters::Transaction + .prepend(Cells::TransactionRecord::TransactionExtension) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 215ed6f4ca96078236a90947e560aaa32a6f5518..9252a8cdf7f58afe337dae34fc1b5be6f615f4c5 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -151,6 +151,15 @@ custom_access_token_attributes [:organization_id] enable_dynamic_scopes + + # Following doorkeeper monkey patches are to claim required resources + + Doorkeeper::Application.class_eval do + include Cells::Claimable + + cells_claims_attribute :uid, type: Cells::Claimable::CLAIMS_BUCKET_TYPE::UNSPECIFIED + end + # Following doorkeeper monkey patches are to identify the organization on best effort basis Doorkeeper::OAuth::PasswordAccessTokenRequest.class_eval do diff --git a/spec/models/cells/outstanding_lease_spec.rb b/spec/models/cells/outstanding_lease_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ac7b139a36596bea744d1f48871e9d629f73e7a7 --- /dev/null +++ b/spec/models/cells/outstanding_lease_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Cells::OutstandingLease, feature_category: :cell do + let(:cell_id) { 1 } + let(:uuid_value) { SecureRandom.uuid } + let(:mock_service) { instance_double(::Gitlab::TopologyServiceClient::ClaimService) } + + before do + allow(described_class).to receive(:claim_service).and_return(mock_service) + allow(mock_service).to receive(:cell_id).and_return(cell_id) + end + + describe '.create_from_request!' do + let(:create_records) { [{}] } + let(:destroy_records) { [{}] } + + let(:mock_response) do + Gitlab::Cells::TopologyService::Claims::V1::BeginUpdateResponse.new( + lease_uuid: Gitlab::Cells::TopologyService::Types::V1::UUID.new(value: uuid_value) + ) + end + + it 'calls begin_update and creates an OutstandingLease' do + expect(mock_service).to receive(:begin_update).with( + an_instance_of(Gitlab::Cells::TopologyService::Claims::V1::BeginUpdateRequest), + deadline: nil + ).and_return(mock_response) + + lease = described_class.create_from_request!(create_records: create_records, destroy_records: destroy_records) + + expect(lease).to be_persisted + expect(lease.uuid).to eq(uuid_value) + end + + context 'with deadline' do + let(:deadline) { GRPC::Core::TimeConsts.from_relative_time(5.0) } + + it 'calls begin_update with deadline and creates an OutstandingLease' do + expect(mock_service).to receive(:begin_update).with( + an_instance_of(Gitlab::Cells::TopologyService::Claims::V1::BeginUpdateRequest), + deadline: deadline + ).and_return(mock_response) + + lease = described_class.create_from_request!( + create_records: create_records, + destroy_records: destroy_records, + deadline: deadline + ) + + expect(lease).to be_persisted + expect(lease.uuid).to eq(uuid_value) + end + end + end + + describe '#send_commit_update!' do + let(:lease) { described_class.create!(uuid: uuid_value) } + + it 'sends a commit_update request with the correct parameters' do + expect(mock_service).to receive(:commit_update).with( + an_instance_of(Gitlab::Cells::TopologyService::Claims::V1::CommitUpdateRequest), + deadline: nil + ) + + lease.send_commit_update! + end + + context 'with deadline' do + let(:deadline) { GRPC::Core::TimeConsts.from_relative_time(5.0) } + + it 'sends a commit_update request with the correct parameters' do + expect(mock_service).to receive(:commit_update).with( + an_instance_of(Gitlab::Cells::TopologyService::Claims::V1::CommitUpdateRequest), + deadline: deadline + ) + + lease.send_commit_update!(deadline: deadline) + end + end + end + + describe '#send_rollback_update!' do + context 'when uuid is present' do + let(:lease) { described_class.create!(uuid: uuid_value) } + + it 'sends a rollback_update request with the correct parameters' do + expect(mock_service).to receive(:rollback_update).with( + an_instance_of(Gitlab::Cells::TopologyService::Claims::V1::RollbackUpdateRequest), + deadline: nil + ) + + lease.send_rollback_update! + end + + context 'with deadline' do + let(:deadline) { GRPC::Core::TimeConsts.from_relative_time(5.0) } + + it 'sends a rollback_update request with the correct parameters' do + expect(mock_service).to receive(:rollback_update).with( + an_instance_of(Gitlab::Cells::TopologyService::Claims::V1::RollbackUpdateRequest), + deadline: deadline + ) + + lease.send_rollback_update!(deadline: deadline) + end + end + end + + context 'when uuid is nil' do + let(:lease) { described_class.new(uuid: nil) } + + it 'does not send a rollback_update request' do + expect(mock_service).not_to receive(:rollback_update) + + lease.send_rollback_update! + end + end + end + + describe '.claim_service' do + it 'returns the ClaimService instance' do + expect(described_class.claim_service).to eq(mock_service) + end + end +end diff --git a/spec/models/cells/transaction_record_spec.rb b/spec/models/cells/transaction_record_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..aca2fab02d5946cd0a0afcd8ea59f8a323ed59de --- /dev/null +++ b/spec/models/cells/transaction_record_spec.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Cells::TransactionRecord, feature_category: :cell do + let(:connection) { ApplicationRecord.connection } + let(:transaction) do + instance_double(ActiveRecord::ConnectionAdapters::NullTransaction, closed?: false, add_record: nil) + end + + let(:current_transaction) { transaction } + + before do + allow(connection).to receive(:current_transaction).and_return(current_transaction) + allow(GRPC::Core::TimeConsts).to receive(:from_relative_time).and_return("fake-deadline") + end + + describe ".current_transaction" do + subject(:transaction_record) { described_class.current_transaction(connection) } + + before do + transaction.extend(Cells::TransactionRecord::TransactionExtension) + + allow(Current).to receive(:cells_claims_leases?).and_return(true) + end + + context "when Current.cells_claims_leases? is false" do + before do + allow(Current).to receive(:cells_claims_leases?).and_return(false) + end + + it { is_expected.to be_nil } + end + + context "when transaction is closed" do + before do + allow(transaction).to receive(:closed?).and_return(true) + end + + it "raises an error" do + expect { transaction_record }.to raise_error("The Cells::TransactionRecord requires transaction to be open") + end + end + + context "when transaction already has a TransactionRecord" do + let(:existing_record) { instance_double(described_class) } + + before do + transaction.cells_current_transaction_record = existing_record + end + + it { is_expected.to eq(existing_record) } + end + + context "when transaction does not have a TransactionRecord" do + it "creates and attaches a new TransactionRecord" do + new_record = transaction_record + expect(new_record).to be_a(described_class) + expect(transaction.cells_current_transaction_record).to eq(new_record) + end + end + end + + describe "#create_record and #destroy_record" do + let(:record) { described_class.new(connection, transaction) } + + it "stores create metadata" do + expect { record.create_record("meta1") }.to change { + record.instance_variable_get(:@create_records) + }.to include("meta1") + end + + it "stores destroy metadata" do + expect { record.destroy_record("meta2") }.to change { + record.instance_variable_get(:@destroy_records) + }.to include("meta2") + end + + context "when after lease is created" do + before do + record.instance_variable_set(:@outstanding_lease, instance_double(Cells::OutstandingLease)) + end + + it "raises if create_record is called" do + expect { record.create_record("meta") }.to raise_error("Lease already created") + end + + it "raises if destroy_record is called" do + expect { record.destroy_record("meta") }.to raise_error("Lease already created") + end + end + end + + describe "transaction lifecycle callbacks" do + let(:record) { described_class.new(connection, transaction) } + let(:lease) do + instance_double(Cells::OutstandingLease, send_commit_update!: nil, send_rollback_update!: nil, destroy!: nil) + end + + before do + allow(Cells::OutstandingLease).to receive(:connection).and_return(connection) + end + + describe "#before_committed!" do + it "creates a lease" do + expect(Cells::OutstandingLease).to receive(:create_from_request!).with( + create_records: [], + destroy_records: [], + deadline: "fake-deadline" + ).and_return(lease) + + record.before_committed! + expect(record.instance_variable_get(:@outstanding_lease)).to eq(lease) + end + + it "raises if already done" do + record.instance_variable_set(:@done, true) + expect { record.before_committed! }.to raise_error("Already done") + end + + it "raises if lease already created" do + record.instance_variable_set(:@outstanding_lease, lease) + expect { record.before_committed! }.to raise_error("Already created lease") + end + + it "raises if connection mismatch" do + allow(Cells::OutstandingLease) + .to receive(:connection) + .and_return(instance_double(Gitlab::Database::LoadBalancing::ConnectionProxy)) + + expect { record.before_committed! }.to raise_error("Cross-database lease") + end + end + + describe "#committed!" do + before do + record.instance_variable_set(:@outstanding_lease, lease) + end + + it "sends commit update and destroys lease" do + expect(lease).to receive(:send_commit_update!).with(deadline: "fake-deadline") + expect(lease).to receive(:destroy!) + record.committed! + expect(record.instance_variable_get(:@done)).to be true + end + + it "raises if already done" do + record.instance_variable_set(:@done, true) + expect { record.committed! }.to raise_error("Already done") + end + + it "raises if no lease created" do + record.instance_variable_set(:@outstanding_lease, nil) + expect { record.committed! }.to raise_error("No lease created") + end + end + + describe "#rolledback!" do + context "with lease" do + before do + record.instance_variable_set(:@outstanding_lease, lease) + end + + it "sends rollback update and destroys lease" do + expect(lease).to receive(:send_rollback_update!).with(deadline: "fake-deadline") + expect(lease).to receive(:destroy!) + record.rolledback! + expect(record.instance_variable_get(:@done)).to be true + end + end + + context "without lease" do + it "does not raise" do + expect { record.rolledback! }.not_to raise_error + end + end + + it "raises if already done" do + record.instance_variable_set(:@done, true) + expect { record.rolledback! }.to raise_error("Already done") + end + end + end +end diff --git a/spec/models/concerns/cells/claimable_spec.rb b/spec/models/concerns/cells/claimable_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..22f23ab5c052dd2137ff0cf883c9ee8d080ac1fc --- /dev/null +++ b/spec/models/concerns/cells/claimable_spec.rb @@ -0,0 +1,182 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Cells::Claimable, feature_category: :cell do + let_it_be(:user) { create(:user) } + let(:test_klass) { ::User } + + describe 'configuration' do + it 'sets and retrieves cell configuration' do + test_klass.cells_claims_metadata(subject_type: :user, source_type: :rails_table_users) + test_klass.cells_claims_attribute(:email, type: :string) + test_klass.cells_claims_attribute(:username, type: :string) + + expect(test_klass.cells_claims_subject_type).to eq(:user) + expect(test_klass.cells_claims_source_type).to eq(:rails_table_users) + expect(test_klass.cells_claims_attributes).to eq(email: { type: :string }, username: { type: :string }) + end + + it 'uses default subject_key when not provided' do + test_klass.cells_claims_metadata(subject_type: :user, source_type: :rails_table_users) + + expect(test_klass.cells_claims_subject_key).to eq(:organization_id) + end + + it 'allows custom subject_key' do + test_klass.cells_claims_metadata( + subject_type: :user, + subject_key: :custom_id, + source_type: :rails_table_users + ) + + expect(test_klass.cells_claims_subject_key).to eq(:custom_id) + end + + it 'derives source_type from table_name when not provided' do + allow(test_klass).to receive(:table_name).and_return('users') + test_klass.cells_claims_metadata(subject_type: :user) + + expect(test_klass.cells_claims_source_type).to eq( + Gitlab::Cells::TopologyService::Claims::V1::Source::Type::RAILS_TABLE_USERS + ) + end + end + + describe 'callbacks' do + let(:instance) { build(:user) } + let(:transaction_record) { instance_double(Cells::TransactionRecord) } + + before do + allow(Cells::TransactionRecord).to receive(:current_transaction) + .with(instance.connection) + .and_return(transaction_record) + end + + describe '#cells_claims_save_changes' do + context 'when transaction record exists' do + context 'when creating a new record' do + it 'creates claims for all configured attributes' do + instance.email = 'new@example.com' + instance.username = 'newuser' + + expect(transaction_record).to receive(:create_record).once.with( + { + bucket: { type: 0, value: 'newuser' }, + source: { type: 1, rails_primary_key_id: be_a(Integer) }, + subject: { type: 4, id: instance.organization_id } + } + ) + expect(transaction_record).to receive(:create_record).once.with( + { + bucket: { type: 0, value: 'new@example.com' }, + source: { type: 2, rails_primary_key_id: be_a(Integer) }, + subject: { type: 4, id: be_a(Integer) } + } + ) + + instance.save! + end + end + + context 'when updating an existing record' do + let(:instance) { user } + + it 'destroys old claim and creates new claim when attribute changes' do + old_username = instance.username + new_username = generate(:username) + + expect(transaction_record) + .to receive(:destroy_record).with(a_hash_including(bucket: { type: 0, value: old_username })) + expect(transaction_record) + .to receive(:create_record).with(a_hash_including(bucket: { type: 0, value: new_username })) + + instance.update!(username: new_username) + end + + it 'does not process unchanged attributes' do + expect(transaction_record).not_to receive(:destroy_record) + expect(transaction_record).not_to receive(:create_record) + + instance.save! + end + + it 'only destroys claim when attribute changes to nil' do + old_username = instance.username + + expect(transaction_record) + .to receive(:destroy_record).with(hash_including(bucket: { type: 0, value: old_username })) + expect(transaction_record).not_to receive(:create_record) + + instance.update_attribute(:username, nil) + end + end + end + + context 'when transaction record does not exist' do + before do + allow(Cells::TransactionRecord).to receive(:current_transaction).and_return(nil) + end + + it 'does not process claims' do + expect(transaction_record).not_to receive(:create_record) + expect(transaction_record).not_to receive(:destroy_record) + + instance.save! + end + end + end + + describe '#cells_claims_destroy_changes' do + let(:instance) { user } + + context 'when transaction record exists' do + it 'destroys claims for all configured attributes' do + old_username = instance.username + old_email = instance.email + + expect(transaction_record) + .to receive(:destroy_record).with(a_hash_including(bucket: { type: 0, value: old_username })) + expect(transaction_record) + .to receive(:destroy_record).with(a_hash_including(bucket: { type: 0, value: old_email })) + + instance.destroy! + end + end + + context 'when transaction record does not exist' do + before do + allow(Cells::TransactionRecord).to receive(:current_transaction).and_return(nil) + end + + it 'does not process claims' do + expect(transaction_record).not_to receive(:destroy_record) + + instance.destroy! + end + end + end + end + + describe '#cells_claims_default_metadata' do + let(:instance) { user } + + before do + allow(instance.class).to receive_messages( + cells_claims_subject_type: :user, + cells_claims_subject_key: :organization_id, + cells_claims_source_type: :users + ) + allow(instance).to receive(:organization_id).and_return(123) + end + + it 'returns metadata with subject and source information' do + metadata = instance.send(:cells_claims_default_metadata) + + expect(metadata).to eq({ + subject: { type: :user, id: 123 }, + source: { type: :users, rails_primary_key_id: instance.id } + }) + end + end +end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index bdc4945bc729e0c2c24982863d1cc7a573135170..5c92590905e84aa962887aa1f3e859cc9bec00a6 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -5,6 +5,12 @@ RSpec.describe Email do let_it_be(:user) { create(:user) } + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::USER, + subject_key: :user_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::RAILS_TABLE_EMAILS, + unique_attributes: [:email] + describe 'modules' do subject { described_class } diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index fbd7db170ce1278edf157c31b8d7ad2fc8570710..433508097c11b417ada58d2602d468309d8cee68 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -5,6 +5,12 @@ RSpec.describe GpgKey, feature_category: :source_code_management do subject { build(:gpg_key) } + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::USER, + subject_key: :user_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::UNSPECIFIED, + unique_attributes: [:key, :fingerprint, :primary_keyid] + describe "associations" do it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:subkeys) } diff --git a/spec/models/gpg_key_subkey_spec.rb b/spec/models/gpg_key_subkey_spec.rb index c1d9e2bde43bd93ef5208864674b245659ab4d5d..069206fbf29b93769a00b8e2a36cc52e82937ec6 100644 --- a/spec/models/gpg_key_subkey_spec.rb +++ b/spec/models/gpg_key_subkey_spec.rb @@ -14,4 +14,10 @@ it { is_expected.to validate_presence_of(:fingerprint) } it { is_expected.to validate_presence_of(:keyid) } end + + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::UNSPECIFIED, + subject_key: :gpg_key_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::UNSPECIFIED, + unique_attributes: [:fingerprint] end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 1701bbd17ad108fdfe2c1f2449400f0d12d2aa5c..96cf58c3c3310619711caa4d973f0d22ad8327dc 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -5,6 +5,12 @@ RSpec.describe Key, :mailer, feature_category: :system_access do it_behaves_like 'having unique enum values' + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::UNSPECIFIED, + subject_key: :organization_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::UNSPECIFIED, + unique_attributes: [:key, :fingerprint_sha256] + describe "Associations" do it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:todos).dependent(:destroy) } diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index d0c79229379737cd8e2aa9cc716a0778cb285b77..7b343a5eca3f4bbea8ca29f49048c44bd6ac7657 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -5,6 +5,12 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :organization do let_it_be_with_refind(:organization) { create(:organization) } + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::ORGANIZATION, + subject_key: :organization_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::RAILS_TABLE_ORGANIZATIONS, + unique_attributes: [:path] + describe 'associations' do it { is_expected.to have_one(:organization_detail).inverse_of(:organization).autosave(true) } diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 16bb8fe9b3f340475964b9ae678782929d160ce0..308cde317e6930983ccc834d91b537c3f16e4e1b 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -6,6 +6,12 @@ let(:group) { create(:group, path: 'git_lab', name: 'git_lab') } let(:route) { group.route } + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::GROUP, + subject_key: :namespace_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::RAILS_TABLE_ROUTES, + unique_attributes: [:path] + describe 'relationships' do it { is_expected.to belong_to(:source) } it { is_expected.to belong_to(:namespace) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 07d0f2de9b83877845fbfa657e6ae5571e474725..928b9cdd1feef22b14e17d8954d3958506048f21 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -49,6 +49,12 @@ it_behaves_like 'having unique enum values' + it_behaves_like 'cells claimable model', + subject_type: Cells::Claimable::CLAIMS_SUBJECT_TYPE::USER, + subject_key: :organization_id, + source_type: Cells::Claimable::CLAIMS_SOURCE_TYPE::RAILS_TABLE_USERS, + unique_attributes: [:username] + describe 'modules' do subject { described_class } diff --git a/spec/support/shared_examples/cells/unique_shared_examples.rb b/spec/support/shared_examples/cells/unique_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..0aa2beed0f260d3290efb39a13e8268e9b10d252 --- /dev/null +++ b/spec/support/shared_examples/cells/unique_shared_examples.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'cells claimable model' do |subject_type:, subject_key:, source_type:, unique_attributes:| + it 'has the expected subject_type' do + expect(described_class.cells_claims_subject_type).to eq(subject_type) + end + + it 'has the expected subject_key' do + expect(described_class.cells_claims_subject_key).to eq(subject_key) + end + + it 'has the expected source_type' do + expect(described_class.cells_claims_source_type).to eq(source_type) + end + + it 'has the expected unique attributes' do + unique_attributes.each do |attr_name| + expect(described_class.cells_claims_attributes).to have_key(attr_name) + end + end +end