From bfd9f9a1d22ae24c4c59a243ecd72aefe6161847 Mon Sep 17 00:00:00 2001 From: Bojan Marjanovic Date: Fri, 12 Sep 2025 15:51:48 +0200 Subject: [PATCH 1/6] Add deleted claim table --- app/models/cells/deleted_claim.rb | 31 ++++ .../json_schemas/deleted_row_data.json | 81 ++++++++++ db/docs/cells_deleted_claims.yml | 11 ++ ...50912115956_create_cells_deleted_claims.rb | 72 +++++++++ db/schema_migrations/20250912115956 | 1 + db/structure.sql | 63 ++++++++ spec/factories/cells/deleted_claim.rb | 9 ++ spec/models/cells/deleted_claim_spec.rb | 143 ++++++++++++++++++ 8 files changed, 411 insertions(+) create mode 100644 app/models/cells/deleted_claim.rb create mode 100644 app/validators/json_schemas/deleted_row_data.json create mode 100644 db/docs/cells_deleted_claims.yml create mode 100644 db/migrate/20250912115956_create_cells_deleted_claims.rb create mode 100644 db/schema_migrations/20250912115956 create mode 100644 spec/factories/cells/deleted_claim.rb create mode 100644 spec/models/cells/deleted_claim_spec.rb diff --git a/app/models/cells/deleted_claim.rb b/app/models/cells/deleted_claim.rb new file mode 100644 index 00000000000000..f9dcf7a7c14361 --- /dev/null +++ b/app/models/cells/deleted_claim.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Cells + class DeletedClaim < ApplicationRecord + validates :deleted_row_data, json_schema: { filename: "deleted_row_data", size_limit: 64.kilobytes } + + scope :for_table, ->(table_name) { where(source_table: table_name) } + scope :recent, -> { order(created_at: :desc) } + scope :old, -> { where(created_at: ...1.hour.ago) } + + def unique_attributes + case source_table + when 'gpg_key_subkeys' + { + fingerprint: deleted_row_data['fingerprint'], + keyid: deleted_row_data['keyid'] + }.compact + when 'emails' + { + email: deleted_row_data['email'] + }.compact + else + {} + end + end + + def claim_key + [source_table, deleted_row_id] + end + end +end diff --git a/app/validators/json_schemas/deleted_row_data.json b/app/validators/json_schemas/deleted_row_data.json new file mode 100644 index 00000000000000..ad88a2d7bf4bbb --- /dev/null +++ b/app/validators/json_schemas/deleted_row_data.json @@ -0,0 +1,81 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Deleted Claims Data", + "description": "Schema for deleted row data captured by deletion triggers", + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "type": "integer", + "description": "Primary key of the deleted record" + }, + "user_id": { + "type": "integer", + "description": "Foreign key to users table" + }, + "email": { + "type": "string", + "description": "Email address" + }, + "gpg_key_id": { + "type": "integer", + "description": "Foreign key to gpg_keys table" + }, + "keyid": { + "type": [ + "string", + "null" + ], + "pattern": "^(\\\\x[0-9a-fA-F]*|[0-9a-fA-F]*)?$", + "description": "Key ID in bytea hex format" + }, + "fingerprint": { + "type": [ + "string", + "null" + ], + "pattern": "^(\\\\x[0-9a-fA-F]*|[0-9a-fA-F]*)?$", + "description": "Key fingerprint in bytea hex format" + }, + "created_at": { + "type": [ + "string", + "null" + ] + }, + "updated_at": { + "type": [ + "string", + "null" + ] + }, + "confirmation_token": { + "type": [ + "string", + "null" + ] + }, + "confirmed_at": { + "type": [ + "string", + "null" + ] + }, + "confirmation_sent_at": { + "type": [ + "string", + "null" + ] + }, + "detumbled_email": { + "type": [ + "string", + "null" + ], + "maxLength": 255 + } + }, + "additionalProperties": true +} diff --git a/db/docs/cells_deleted_claims.yml b/db/docs/cells_deleted_claims.yml new file mode 100644 index 00000000000000..f5bbcbdbdd0964 --- /dev/null +++ b/db/docs/cells_deleted_claims.yml @@ -0,0 +1,11 @@ +--- +table_name: cells_deleted_claims +classes: +- Cells::DeletedClaim +feature_categories: +- cell +description: Cells Deleted claims related to lease releases. +introduced_by_url: +milestone: '18.4' +gitlab_schema: gitlab_main_cell_local +table_size: small diff --git a/db/migrate/20250912115956_create_cells_deleted_claims.rb b/db/migrate/20250912115956_create_cells_deleted_claims.rb new file mode 100644 index 00000000000000..3b6ba2f768b797 --- /dev/null +++ b/db/migrate/20250912115956_create_cells_deleted_claims.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +class CreateCellsDeletedClaims < Gitlab::Database::Migration[2.3] + milestone '18.4' + disable_ddl_transaction! + + def up + create_table :cells_deleted_claims do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive + t.bigint :deleted_row_id, null: false + t.text :source_table, null: false, limit: 63 + t.jsonb :deleted_row_data, null: false + t.timestamps_with_timezone null: false + end + + execute <<~SQL + CREATE OR REPLACE FUNCTION record_claim_deletion() + RETURNS TRIGGER AS $$ + DECLARE + v_deleted_row_data JSONB; + v_deleted_row_id BIGINT; + BEGIN + v_deleted_row_data := to_jsonb(OLD); + + BEGIN + v_deleted_row_id := (v_deleted_row_data->>'id')::BIGINT; + EXCEPTION WHEN OTHERS THEN + v_deleted_row_id := NULL; + END; + + IF v_deleted_row_id IS NOT NULL THEN + INSERT INTO cells_deleted_claims ( + deleted_row_id, + source_table, + deleted_row_data, + created_at, + updated_at + ) VALUES ( + v_deleted_row_id, + TG_TABLE_NAME, + v_deleted_row_data, + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP + ); + END IF; + + RETURN OLD; + END; + $$ LANGUAGE plpgsql; + SQL + + execute <<~SQL + CREATE TRIGGER gpg_key_subkeys_deletion_audit + BEFORE DELETE ON gpg_key_subkeys + FOR EACH ROW + EXECUTE FUNCTION record_claim_deletion(); + SQL + + execute <<~SQL + CREATE TRIGGER emails_deletion_audit + BEFORE DELETE ON emails + FOR EACH ROW + EXECUTE FUNCTION record_claim_deletion(); + SQL + end + + def down + execute "DROP TRIGGER IF EXISTS gpg_key_subkeys_deletion_audit ON gpg_key_subkeys;" + execute "DROP TRIGGER IF EXISTS emails_deletion_audit ON emails;" + execute "DROP FUNCTION IF EXISTS record_claim_deletion() CASCADE;" + drop_table :cells_deleted_claims + end +end diff --git a/db/schema_migrations/20250912115956 b/db/schema_migrations/20250912115956 new file mode 100644 index 00000000000000..30ffa64f2773fd --- /dev/null +++ b/db/schema_migrations/20250912115956 @@ -0,0 +1 @@ +ce87de5292ea4d1a0438f186f1e9bdc5736f1bebedf95de6abd43f1a23ea6f28 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 4a8ed4a43b55f3..27033c2deb6a29 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -773,6 +773,41 @@ RETURN OLD; END $$; +CREATE FUNCTION record_claim_deletion() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE + v_deleted_row_data JSONB; + v_deleted_row_id BIGINT; +BEGIN + v_deleted_row_data := to_jsonb(OLD); + + BEGIN + v_deleted_row_id := (v_deleted_row_data->>'id')::BIGINT; + EXCEPTION WHEN OTHERS THEN + v_deleted_row_id := NULL; + END; + + IF v_deleted_row_id IS NOT NULL THEN + INSERT INTO cells_deleted_claims ( + deleted_row_id, + source_table, + deleted_row_data, + created_at, + updated_at + ) VALUES ( + v_deleted_row_id, + TG_TABLE_NAME, + v_deleted_row_data, + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP + ); + END IF; + + RETURN OLD; +END; +$$; + CREATE FUNCTION set_has_external_issue_tracker() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -13330,6 +13365,25 @@ CREATE SEQUENCE catalog_verified_namespaces_id_seq ALTER SEQUENCE catalog_verified_namespaces_id_seq OWNED BY catalog_verified_namespaces.id; +CREATE TABLE cells_deleted_claims ( + id bigint NOT NULL, + deleted_row_id bigint NOT NULL, + source_table text NOT NULL, + deleted_row_data jsonb NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_1d15f820fa CHECK ((char_length(source_table) <= 63)) +); + +CREATE SEQUENCE cells_deleted_claims_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE cells_deleted_claims_id_seq OWNED BY cells_deleted_claims.id; + CREATE TABLE cells_outstanding_leases ( uuid uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -30683,6 +30737,8 @@ ALTER TABLE ONLY catalog_resources ALTER COLUMN id SET DEFAULT nextval('catalog_ ALTER TABLE ONLY catalog_verified_namespaces ALTER COLUMN id SET DEFAULT nextval('catalog_verified_namespaces_id_seq'::regclass); +ALTER TABLE ONLY cells_deleted_claims ALTER COLUMN id SET DEFAULT nextval('cells_deleted_claims_id_seq'::regclass); + ALTER TABLE ONLY chat_names ALTER COLUMN id SET DEFAULT nextval('chat_names_id_seq'::regclass); ALTER TABLE ONLY chat_teams ALTER COLUMN id SET DEFAULT nextval('chat_teams_id_seq'::regclass); @@ -33236,6 +33292,9 @@ ALTER TABLE ONLY catalog_resources ALTER TABLE ONLY catalog_verified_namespaces ADD CONSTRAINT catalog_verified_namespaces_pkey PRIMARY KEY (id); +ALTER TABLE ONLY cells_deleted_claims + ADD CONSTRAINT cells_deleted_claims_pkey PRIMARY KEY (id); + ALTER TABLE ONLY cells_outstanding_leases ADD CONSTRAINT cells_outstanding_leases_pkey PRIMARY KEY (uuid); @@ -46931,6 +46990,10 @@ CREATE TRIGGER clusters_loose_fk_trigger AFTER DELETE ON clusters REFERENCING OL CREATE TRIGGER duo_workflows_workflows_loose_fk_trigger AFTER DELETE ON duo_workflows_workflows REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); +CREATE TRIGGER emails_deletion_audit BEFORE DELETE ON emails FOR EACH ROW EXECUTE FUNCTION record_claim_deletion(); + +CREATE TRIGGER gpg_key_subkeys_deletion_audit BEFORE DELETE ON gpg_key_subkeys FOR EACH ROW EXECUTE FUNCTION record_claim_deletion(); + CREATE TRIGGER group_type_ci_runner_machines_loose_fk_trigger AFTER DELETE ON group_type_ci_runner_machines REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records_override_table('ci_runner_machines'); CREATE TRIGGER group_type_ci_runners_loose_fk_trigger AFTER DELETE ON group_type_ci_runners REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records_override_table('ci_runners'); diff --git a/spec/factories/cells/deleted_claim.rb b/spec/factories/cells/deleted_claim.rb new file mode 100644 index 00000000000000..933e0b0e5e1630 --- /dev/null +++ b/spec/factories/cells/deleted_claim.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :cells_deleted_claim, class: 'Cells::DeletedClaim' do + deleted_row_id { rand(1..1000) } + source_table { %w[users gpg_key_subkeys].sample } + jsonb { {} } + end +end diff --git a/spec/models/cells/deleted_claim_spec.rb b/spec/models/cells/deleted_claim_spec.rb new file mode 100644 index 00000000000000..94682a5f9e4db4 --- /dev/null +++ b/spec/models/cells/deleted_claim_spec.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Cells::DeletedClaim, feature_category: :cell do + describe 'scopes' do + let_it_be(:email_claim) do + described_class.create!( + deleted_row_id: 123, + source_table: 'emails', + deleted_row_data: { 'id' => 123, 'email' => 'test@example.com', 'user_id' => 1 } + ) + end + + let_it_be(:gpg_claim) do + described_class.create!( + deleted_row_id: 456, + source_table: 'gpg_key_subkeys', + deleted_row_data: { 'id' => 456, 'fingerprint' => '1234', 'keyid' => '5678' } + ) + end + + let_it_be(:old_claim) do + described_class.create!( + deleted_row_id: 789, + source_table: 'emails', + deleted_row_data: { 'id' => 789 }, + created_at: 2.hours.ago + ) + end + + describe '.for_table' do + it 'filters by source table' do + expect(described_class.for_table('emails')).to contain_exactly(email_claim, old_claim) + expect(described_class.for_table('gpg_key_subkeys')).to contain_exactly(gpg_claim) + end + end + + describe '.recent' do + it 'orders by created_at descending' do + expect(described_class.recent).to eq([gpg_claim, email_claim, old_claim]) + end + end + + describe '.old' do + it 'returns claims older than 1 hour' do + expect(described_class.old).to contain_exactly(old_claim) + end + end + end + + describe '#unique_attributes' do + context 'for emails table' do + let(:deleted_claim) do + described_class.new( + source_table: 'emails', + deleted_row_id: 123, + deleted_row_data: { + 'id' => 123, + 'email' => 'user@example.com', + 'user_id' => 456, + 'confirmed_at' => '2024-01-01' + } + ) + end + + it 'extracts email attribute' do + expect(deleted_claim.unique_attributes).to eq( + email: 'user@example.com' + ) + end + end + + context 'for gpg_key_subkeys table' do + let(:deleted_claim) do + described_class.new( + source_table: 'gpg_key_subkeys', + deleted_row_id: 789, + deleted_row_data: { + 'id' => 789, + 'gpg_key_id' => 111, + 'fingerprint' => '1234abcd', + 'keyid' => '5678ef01' + } + ) + end + + it 'extracts fingerprint and keyid attributes' do + expect(deleted_claim.unique_attributes).to eq( + fingerprint: '1234abcd', + keyid: '5678ef01' + ) + end + end + + context 'for invalid table' do + let(:deleted_claim) do + described_class.new( + source_table: 'invalid_table', + deleted_row_id: 999, + deleted_row_data: { 'id' => 999, 'data' => 'value' } + ) + end + + it 'returns empty hash' do + expect(deleted_claim.unique_attributes).to eq({}) + end + end + end + + describe '#claim_key' do + let(:deleted_claim) do + described_class.new( + source_table: 'emails', + deleted_row_id: 123, + deleted_row_data: {} + ) + end + + it 'returns array with source_table and deleted_row_id' do + expect(deleted_claim.claim_key).to eq(['emails', 123]) + end + end + + describe 'integration with trigger' do + it 'can be created from trigger data' do + claim = described_class.create!( + deleted_row_id: 100, + source_table: 'emails', + deleted_row_data: { + 'id' => 100, + 'email' => 'deleted@example.com', + 'user_id' => 50, + 'created_at' => '2024-01-01T00:00:00', + 'updated_at' => '2024-01-01T00:00:00' + } + ) + + expect(claim).to be_persisted + expect(claim.deleted_row_data['email']).to eq('deleted@example.com') + end + end +end -- GitLab From 708dc2ba8049b4ed89465b38d20712b4f29131d1 Mon Sep 17 00:00:00 2001 From: Bojan Marjanovic Date: Fri, 12 Sep 2025 16:30:01 +0200 Subject: [PATCH 2/6] Fixes failing specs --- spec/db/schema_spec.rb | 1 + spec/factories/cells/deleted_claim.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index f944b868528177..88922110e1101a 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -68,6 +68,7 @@ catalog_resource_component_last_usages: %w[used_by_project_id], # No FK constraint because we want to preserve usage data even if project is deleted. chat_names: %w[chat_id team_id], chat_teams: %w[team_id], + cells_deleted_claims: %w[deleted_row_id], ci_build_needs: %w[project_id], ci_build_pending_states: %w[project_id], ci_build_trace_chunks: %w[project_id], diff --git a/spec/factories/cells/deleted_claim.rb b/spec/factories/cells/deleted_claim.rb index 933e0b0e5e1630..77458e9db4d73e 100644 --- a/spec/factories/cells/deleted_claim.rb +++ b/spec/factories/cells/deleted_claim.rb @@ -4,6 +4,6 @@ factory :cells_deleted_claim, class: 'Cells::DeletedClaim' do deleted_row_id { rand(1..1000) } source_table { %w[users gpg_key_subkeys].sample } - jsonb { {} } + deleted_row_data { {} } end end -- GitLab From 3187db0a99f36363b6872d22de66d964ea76fad1 Mon Sep 17 00:00:00 2001 From: Bojan Marjanovic Date: Mon, 15 Sep 2025 16:17:15 +0200 Subject: [PATCH 3/6] Apply reviewers feedback --- app/models/cells/deleted_claim.rb | 2 +- db/docs/cells_deleted_claims.yml | 2 +- ...50912115956_create_cells_deleted_claims.rb | 27 ++++++++++--------- db/structure.sql | 20 +++++++------- spec/db/schema_spec.rb | 1 - spec/factories/cells/deleted_claim.rb | 2 +- spec/models/cells/deleted_claim_spec.rb | 18 ++++++------- 7 files changed, 38 insertions(+), 34 deletions(-) diff --git a/app/models/cells/deleted_claim.rb b/app/models/cells/deleted_claim.rb index f9dcf7a7c14361..faa4ce5ff560e9 100644 --- a/app/models/cells/deleted_claim.rb +++ b/app/models/cells/deleted_claim.rb @@ -25,7 +25,7 @@ def unique_attributes end def claim_key - [source_table, deleted_row_id] + [source_table, deleted_row_identifier] end end end diff --git a/db/docs/cells_deleted_claims.yml b/db/docs/cells_deleted_claims.yml index f5bbcbdbdd0964..99e98fb97fce92 100644 --- a/db/docs/cells_deleted_claims.yml +++ b/db/docs/cells_deleted_claims.yml @@ -6,6 +6,6 @@ feature_categories: - cell description: Cells Deleted claims related to lease releases. introduced_by_url: -milestone: '18.4' +milestone: '18.5' gitlab_schema: gitlab_main_cell_local table_size: small diff --git a/db/migrate/20250912115956_create_cells_deleted_claims.rb b/db/migrate/20250912115956_create_cells_deleted_claims.rb index 3b6ba2f768b797..aca64f28951a7d 100644 --- a/db/migrate/20250912115956_create_cells_deleted_claims.rb +++ b/db/migrate/20250912115956_create_cells_deleted_claims.rb @@ -1,15 +1,18 @@ # frozen_string_literal: true class CreateCellsDeletedClaims < Gitlab::Database::Migration[2.3] - milestone '18.4' + milestone '18.5' disable_ddl_transaction! def up - create_table :cells_deleted_claims do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive - t.bigint :deleted_row_id, null: false + create_table :cells_deleted_claims, if_not_exists: true do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive + t.timestamps_with_timezone null: false + t.bigint :deleted_row_identifier, null: false t.text :source_table, null: false, limit: 63 t.jsonb :deleted_row_data, null: false - t.timestamps_with_timezone null: false + + t.index [:deleted_row_identifier, :source_table], unique: true, + name: 'index_deleted_claims_on_deleted_row_id_and_source_table' end execute <<~SQL @@ -17,25 +20,25 @@ def up RETURNS TRIGGER AS $$ DECLARE v_deleted_row_data JSONB; - v_deleted_row_id BIGINT; + v_deleted_row_identifier BIGINT; BEGIN v_deleted_row_data := to_jsonb(OLD); BEGIN - v_deleted_row_id := (v_deleted_row_data->>'id')::BIGINT; + v_deleted_row_identifier := OLD.id; EXCEPTION WHEN OTHERS THEN - v_deleted_row_id := NULL; + v_deleted_row_identifier := NULL; END; - IF v_deleted_row_id IS NOT NULL THEN + IF v_deleted_row_identifier IS NOT NULL THEN INSERT INTO cells_deleted_claims ( - deleted_row_id, + deleted_row_identifier, source_table, deleted_row_data, created_at, updated_at ) VALUES ( - v_deleted_row_id, + v_deleted_row_identifier, TG_TABLE_NAME, v_deleted_row_data, CURRENT_TIMESTAMP, @@ -49,14 +52,14 @@ def up SQL execute <<~SQL - CREATE TRIGGER gpg_key_subkeys_deletion_audit + CREATE OR REPLACE TRIGGER gpg_key_subkeys_deletion_audit BEFORE DELETE ON gpg_key_subkeys FOR EACH ROW EXECUTE FUNCTION record_claim_deletion(); SQL execute <<~SQL - CREATE TRIGGER emails_deletion_audit + CREATE OR REPLACE TRIGGER emails_deletion_audit BEFORE DELETE ON emails FOR EACH ROW EXECUTE FUNCTION record_claim_deletion(); diff --git a/db/structure.sql b/db/structure.sql index 27033c2deb6a29..5fd310cb55514c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -778,25 +778,25 @@ CREATE FUNCTION record_claim_deletion() RETURNS trigger AS $$ DECLARE v_deleted_row_data JSONB; - v_deleted_row_id BIGINT; + v_deleted_row_identifier BIGINT; BEGIN v_deleted_row_data := to_jsonb(OLD); BEGIN - v_deleted_row_id := (v_deleted_row_data->>'id')::BIGINT; + v_deleted_row_identifier := OLD.id; EXCEPTION WHEN OTHERS THEN - v_deleted_row_id := NULL; + v_deleted_row_identifier := NULL; END; - IF v_deleted_row_id IS NOT NULL THEN + IF v_deleted_row_identifier IS NOT NULL THEN INSERT INTO cells_deleted_claims ( - deleted_row_id, + deleted_row_identifier, source_table, deleted_row_data, created_at, updated_at ) VALUES ( - v_deleted_row_id, + v_deleted_row_identifier, TG_TABLE_NAME, v_deleted_row_data, CURRENT_TIMESTAMP, @@ -13367,11 +13367,11 @@ ALTER SEQUENCE catalog_verified_namespaces_id_seq OWNED BY catalog_verified_name CREATE TABLE cells_deleted_claims ( id bigint NOT NULL, - deleted_row_id bigint NOT NULL, - source_table text NOT NULL, - deleted_row_data jsonb NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, + deleted_row_identifier bigint NOT NULL, + source_table text NOT NULL, + deleted_row_data jsonb NOT NULL, CONSTRAINT check_1d15f820fa CHECK ((char_length(source_table) <= 63)) ); @@ -39668,6 +39668,8 @@ CREATE INDEX index_dast_sites_on_dast_site_validation_id ON dast_sites USING btr CREATE UNIQUE INDEX index_dast_sites_on_project_id_and_url ON dast_sites USING btree (project_id, url); +CREATE UNIQUE INDEX index_deleted_claims_on_deleted_row_id_and_source_table ON cells_deleted_claims USING btree (deleted_row_identifier, source_table); + CREATE UNIQUE INDEX index_dep_prox_manifests_on_group_id_file_name_and_status ON dependency_proxy_manifests USING btree (group_id, file_name, status); CREATE INDEX index_dependency_list_export_parts_on_dependency_list_export_id ON dependency_list_export_parts USING btree (dependency_list_export_id); diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 88922110e1101a..f944b868528177 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -68,7 +68,6 @@ catalog_resource_component_last_usages: %w[used_by_project_id], # No FK constraint because we want to preserve usage data even if project is deleted. chat_names: %w[chat_id team_id], chat_teams: %w[team_id], - cells_deleted_claims: %w[deleted_row_id], ci_build_needs: %w[project_id], ci_build_pending_states: %w[project_id], ci_build_trace_chunks: %w[project_id], diff --git a/spec/factories/cells/deleted_claim.rb b/spec/factories/cells/deleted_claim.rb index 77458e9db4d73e..8e829f0640204a 100644 --- a/spec/factories/cells/deleted_claim.rb +++ b/spec/factories/cells/deleted_claim.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :cells_deleted_claim, class: 'Cells::DeletedClaim' do - deleted_row_id { rand(1..1000) } + deleted_row_identifier { rand(1..1000) } source_table { %w[users gpg_key_subkeys].sample } deleted_row_data { {} } end diff --git a/spec/models/cells/deleted_claim_spec.rb b/spec/models/cells/deleted_claim_spec.rb index 94682a5f9e4db4..9d84aa8dc65bbb 100644 --- a/spec/models/cells/deleted_claim_spec.rb +++ b/spec/models/cells/deleted_claim_spec.rb @@ -6,7 +6,7 @@ describe 'scopes' do let_it_be(:email_claim) do described_class.create!( - deleted_row_id: 123, + deleted_row_identifier: 123, source_table: 'emails', deleted_row_data: { 'id' => 123, 'email' => 'test@example.com', 'user_id' => 1 } ) @@ -14,7 +14,7 @@ let_it_be(:gpg_claim) do described_class.create!( - deleted_row_id: 456, + deleted_row_identifier: 456, source_table: 'gpg_key_subkeys', deleted_row_data: { 'id' => 456, 'fingerprint' => '1234', 'keyid' => '5678' } ) @@ -22,7 +22,7 @@ let_it_be(:old_claim) do described_class.create!( - deleted_row_id: 789, + deleted_row_identifier: 789, source_table: 'emails', deleted_row_data: { 'id' => 789 }, created_at: 2.hours.ago @@ -54,7 +54,7 @@ let(:deleted_claim) do described_class.new( source_table: 'emails', - deleted_row_id: 123, + deleted_row_identifier: 123, deleted_row_data: { 'id' => 123, 'email' => 'user@example.com', @@ -75,7 +75,7 @@ let(:deleted_claim) do described_class.new( source_table: 'gpg_key_subkeys', - deleted_row_id: 789, + deleted_row_identifier: 789, deleted_row_data: { 'id' => 789, 'gpg_key_id' => 111, @@ -97,7 +97,7 @@ let(:deleted_claim) do described_class.new( source_table: 'invalid_table', - deleted_row_id: 999, + deleted_row_identifier: 999, deleted_row_data: { 'id' => 999, 'data' => 'value' } ) end @@ -112,12 +112,12 @@ let(:deleted_claim) do described_class.new( source_table: 'emails', - deleted_row_id: 123, + deleted_row_identifier: 123, deleted_row_data: {} ) end - it 'returns array with source_table and deleted_row_id' do + it 'returns array with source_table and deleted_row_identifier' do expect(deleted_claim.claim_key).to eq(['emails', 123]) end end @@ -125,7 +125,7 @@ describe 'integration with trigger' do it 'can be created from trigger data' do claim = described_class.create!( - deleted_row_id: 100, + deleted_row_identifier: 100, source_table: 'emails', deleted_row_data: { 'id' => 100, -- GitLab From 2576f71aea2d346f6af92d91cf924fadb6dce5b4 Mon Sep 17 00:00:00 2001 From: Bojan Marjanovic Date: Tue, 16 Sep 2025 14:08:00 +0200 Subject: [PATCH 4/6] Adds missing tests for trigger --- db/docs/cells_deleted_claims.yml | 2 +- spec/models/email_spec.rb | 12 ++++++++++++ spec/models/gpg_key_subkey_spec.rb | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/db/docs/cells_deleted_claims.yml b/db/docs/cells_deleted_claims.yml index 99e98fb97fce92..65a7f516f2159f 100644 --- a/db/docs/cells_deleted_claims.yml +++ b/db/docs/cells_deleted_claims.yml @@ -5,7 +5,7 @@ classes: feature_categories: - cell description: Cells Deleted claims related to lease releases. -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/204871 milestone: '18.5' gitlab_schema: gitlab_main_cell_local table_size: small diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index bdc4945bc729e0..8c7a1153bfa683 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -244,4 +244,16 @@ end end end + + describe 'emails_deletion_audit trigger' do + before do + create_list(:email, 4, user: user) + end + + it 'invokes the record_claim_deletion() trigger and creates the record in cells_deleted_claims' do + expect do + user.destroy! + end.to change { Cells::DeletedClaim.count }.by(5) + end + end end diff --git a/spec/models/gpg_key_subkey_spec.rb b/spec/models/gpg_key_subkey_spec.rb index c1d9e2bde43bd9..a5aa52c96cbb05 100644 --- a/spec/models/gpg_key_subkey_spec.rb +++ b/spec/models/gpg_key_subkey_spec.rb @@ -14,4 +14,21 @@ it { is_expected.to validate_presence_of(:fingerprint) } it { is_expected.to validate_presence_of(:keyid) } end + + describe 'gpg_key_subkeys_deletion_audit trigger' do + let_it_be(:user) { create(:user) } + + before do + create(:gpg_key, user: user) + end + + it 'invokes the record_claim_deletion() trigger and creates the record in cells_deleted_claims' do + expect do + user.destroy! + end.to change { Cells::DeletedClaim.count }.by(2) + + sources = Cells::DeletedClaim.limit(2).pluck(:source_table) + expect(sources).to match_array(%w[emails gpg_key_subkeys]) + end + end end -- GitLab From 559c51c3e50086fa90fcb43cebdb99d1d49b54d7 Mon Sep 17 00:00:00 2001 From: Bojan Marjanovic Date: Mon, 6 Oct 2025 12:48:22 +0200 Subject: [PATCH 5/6] Improves delete claim trigger logic --- app/models/cells/deleted_claim.rb | 20 +-- .../json_schemas/deleted_row_data.json | 81 ------------ ...1006102914_create_cells_deleted_claims.rb} | 28 ++--- db/schema_migrations/20250912115956 | 1 - db/schema_migrations/20251006102914 | 1 + db/structure.sql | 26 ++-- spec/factories/cells/deleted_claim.rb | 3 +- spec/models/cells/deleted_claim_spec.rb | 116 ++---------------- 8 files changed, 31 insertions(+), 245 deletions(-) delete mode 100644 app/validators/json_schemas/deleted_row_data.json rename db/migrate/{20250912115956_create_cells_deleted_claims.rb => 20251006102914_create_cells_deleted_claims.rb} (69%) delete mode 100644 db/schema_migrations/20250912115956 create mode 100644 db/schema_migrations/20251006102914 diff --git a/app/models/cells/deleted_claim.rb b/app/models/cells/deleted_claim.rb index faa4ce5ff560e9..20b12ec0e7e3dd 100644 --- a/app/models/cells/deleted_claim.rb +++ b/app/models/cells/deleted_claim.rb @@ -2,30 +2,12 @@ module Cells class DeletedClaim < ApplicationRecord - validates :deleted_row_data, json_schema: { filename: "deleted_row_data", size_limit: 64.kilobytes } - scope :for_table, ->(table_name) { where(source_table: table_name) } scope :recent, -> { order(created_at: :desc) } scope :old, -> { where(created_at: ...1.hour.ago) } - def unique_attributes - case source_table - when 'gpg_key_subkeys' - { - fingerprint: deleted_row_data['fingerprint'], - keyid: deleted_row_data['keyid'] - }.compact - when 'emails' - { - email: deleted_row_data['email'] - }.compact - else - {} - end - end - def claim_key - [source_table, deleted_row_identifier] + [source_table, source_record_id] end end end diff --git a/app/validators/json_schemas/deleted_row_data.json b/app/validators/json_schemas/deleted_row_data.json deleted file mode 100644 index ad88a2d7bf4bbb..00000000000000 --- a/app/validators/json_schemas/deleted_row_data.json +++ /dev/null @@ -1,81 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "Deleted Claims Data", - "description": "Schema for deleted row data captured by deletion triggers", - "type": "object", - "required": [ - "id" - ], - "properties": { - "id": { - "type": "integer", - "description": "Primary key of the deleted record" - }, - "user_id": { - "type": "integer", - "description": "Foreign key to users table" - }, - "email": { - "type": "string", - "description": "Email address" - }, - "gpg_key_id": { - "type": "integer", - "description": "Foreign key to gpg_keys table" - }, - "keyid": { - "type": [ - "string", - "null" - ], - "pattern": "^(\\\\x[0-9a-fA-F]*|[0-9a-fA-F]*)?$", - "description": "Key ID in bytea hex format" - }, - "fingerprint": { - "type": [ - "string", - "null" - ], - "pattern": "^(\\\\x[0-9a-fA-F]*|[0-9a-fA-F]*)?$", - "description": "Key fingerprint in bytea hex format" - }, - "created_at": { - "type": [ - "string", - "null" - ] - }, - "updated_at": { - "type": [ - "string", - "null" - ] - }, - "confirmation_token": { - "type": [ - "string", - "null" - ] - }, - "confirmed_at": { - "type": [ - "string", - "null" - ] - }, - "confirmation_sent_at": { - "type": [ - "string", - "null" - ] - }, - "detumbled_email": { - "type": [ - "string", - "null" - ], - "maxLength": 255 - } - }, - "additionalProperties": true -} diff --git a/db/migrate/20250912115956_create_cells_deleted_claims.rb b/db/migrate/20251006102914_create_cells_deleted_claims.rb similarity index 69% rename from db/migrate/20250912115956_create_cells_deleted_claims.rb rename to db/migrate/20251006102914_create_cells_deleted_claims.rb index aca64f28951a7d..d7c026865a776f 100644 --- a/db/migrate/20250912115956_create_cells_deleted_claims.rb +++ b/db/migrate/20251006102914_create_cells_deleted_claims.rb @@ -7,44 +7,32 @@ class CreateCellsDeletedClaims < Gitlab::Database::Migration[2.3] def up create_table :cells_deleted_claims, if_not_exists: true do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive t.timestamps_with_timezone null: false - t.bigint :deleted_row_identifier, null: false + t.bigint :source_record_id, null: false t.text :source_table, null: false, limit: 63 - t.jsonb :deleted_row_data, null: false - t.index [:deleted_row_identifier, :source_table], unique: true, - name: 'index_deleted_claims_on_deleted_row_id_and_source_table' + t.index [:source_record_id, :source_table], unique: true, + name: 'index_deleted_claims_on_source_record_id_and_source_table' end execute <<~SQL CREATE OR REPLACE FUNCTION record_claim_deletion() RETURNS TRIGGER AS $$ - DECLARE - v_deleted_row_data JSONB; - v_deleted_row_identifier BIGINT; BEGIN - v_deleted_row_data := to_jsonb(OLD); - BEGIN - v_deleted_row_identifier := OLD.id; - EXCEPTION WHEN OTHERS THEN - v_deleted_row_identifier := NULL; - END; - - IF v_deleted_row_identifier IS NOT NULL THEN INSERT INTO cells_deleted_claims ( - deleted_row_identifier, + source_record_id, source_table, - deleted_row_data, created_at, updated_at ) VALUES ( - v_deleted_row_identifier, + OLD.id, TG_TABLE_NAME, - v_deleted_row_data, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP ); - END IF; + EXCEPTION WHEN OTHERS THEN + NULL; + END; RETURN OLD; END; diff --git a/db/schema_migrations/20250912115956 b/db/schema_migrations/20250912115956 deleted file mode 100644 index 30ffa64f2773fd..00000000000000 --- a/db/schema_migrations/20250912115956 +++ /dev/null @@ -1 +0,0 @@ -ce87de5292ea4d1a0438f186f1e9bdc5736f1bebedf95de6abd43f1a23ea6f28 \ No newline at end of file diff --git a/db/schema_migrations/20251006102914 b/db/schema_migrations/20251006102914 new file mode 100644 index 00000000000000..95928f19566164 --- /dev/null +++ b/db/schema_migrations/20251006102914 @@ -0,0 +1 @@ +5eb9386d5131421877434c882b10eb11ab00704979e5031f4e31c5d887aefd40 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 5fd310cb55514c..929889da884d02 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -776,33 +776,22 @@ $$; CREATE FUNCTION record_claim_deletion() RETURNS trigger LANGUAGE plpgsql AS $$ -DECLARE - v_deleted_row_data JSONB; - v_deleted_row_identifier BIGINT; BEGIN - v_deleted_row_data := to_jsonb(OLD); - BEGIN - v_deleted_row_identifier := OLD.id; - EXCEPTION WHEN OTHERS THEN - v_deleted_row_identifier := NULL; - END; - - IF v_deleted_row_identifier IS NOT NULL THEN INSERT INTO cells_deleted_claims ( - deleted_row_identifier, + source_record_id, source_table, - deleted_row_data, created_at, updated_at ) VALUES ( - v_deleted_row_identifier, + OLD.id, TG_TABLE_NAME, - v_deleted_row_data, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP ); - END IF; + EXCEPTION WHEN OTHERS THEN + NULL; + END; RETURN OLD; END; @@ -13369,9 +13358,8 @@ CREATE TABLE cells_deleted_claims ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, - deleted_row_identifier bigint NOT NULL, + source_record_id bigint NOT NULL, source_table text NOT NULL, - deleted_row_data jsonb NOT NULL, CONSTRAINT check_1d15f820fa CHECK ((char_length(source_table) <= 63)) ); @@ -39668,7 +39656,7 @@ CREATE INDEX index_dast_sites_on_dast_site_validation_id ON dast_sites USING btr CREATE UNIQUE INDEX index_dast_sites_on_project_id_and_url ON dast_sites USING btree (project_id, url); -CREATE UNIQUE INDEX index_deleted_claims_on_deleted_row_id_and_source_table ON cells_deleted_claims USING btree (deleted_row_identifier, source_table); +CREATE UNIQUE INDEX index_deleted_claims_on_source_record_id_and_source_table ON cells_deleted_claims USING btree (source_record_id, source_table); CREATE UNIQUE INDEX index_dep_prox_manifests_on_group_id_file_name_and_status ON dependency_proxy_manifests USING btree (group_id, file_name, status); diff --git a/spec/factories/cells/deleted_claim.rb b/spec/factories/cells/deleted_claim.rb index 8e829f0640204a..8ace176c4e676b 100644 --- a/spec/factories/cells/deleted_claim.rb +++ b/spec/factories/cells/deleted_claim.rb @@ -2,8 +2,7 @@ FactoryBot.define do factory :cells_deleted_claim, class: 'Cells::DeletedClaim' do - deleted_row_identifier { rand(1..1000) } + source_record_id { rand(1..1000) } source_table { %w[users gpg_key_subkeys].sample } - deleted_row_data { {} } end end diff --git a/spec/models/cells/deleted_claim_spec.rb b/spec/models/cells/deleted_claim_spec.rb index 9d84aa8dc65bbb..ce74c4f516cb00 100644 --- a/spec/models/cells/deleted_claim_spec.rb +++ b/spec/models/cells/deleted_claim_spec.rb @@ -4,29 +4,10 @@ RSpec.describe Cells::DeletedClaim, feature_category: :cell do describe 'scopes' do - let_it_be(:email_claim) do - described_class.create!( - deleted_row_identifier: 123, - source_table: 'emails', - deleted_row_data: { 'id' => 123, 'email' => 'test@example.com', 'user_id' => 1 } - ) - end - - let_it_be(:gpg_claim) do - described_class.create!( - deleted_row_identifier: 456, - source_table: 'gpg_key_subkeys', - deleted_row_data: { 'id' => 456, 'fingerprint' => '1234', 'keyid' => '5678' } - ) - end - + let_it_be(:email_claim) { described_class.create!(source_record_id: 123, source_table: 'emails') } + let_it_be(:gpg_claim) { described_class.create!(source_record_id: 456, source_table: 'gpg_key_subkeys') } let_it_be(:old_claim) do - described_class.create!( - deleted_row_identifier: 789, - source_table: 'emails', - deleted_row_data: { 'id' => 789 }, - created_at: 2.hours.ago - ) + described_class.create!(source_record_id: 789, source_table: 'emails', created_at: 2.hours.ago) end describe '.for_table' do @@ -49,95 +30,24 @@ end end - describe '#unique_attributes' do - context 'for emails table' do - let(:deleted_claim) do - described_class.new( - source_table: 'emails', - deleted_row_identifier: 123, - deleted_row_data: { - 'id' => 123, - 'email' => 'user@example.com', - 'user_id' => 456, - 'confirmed_at' => '2024-01-01' - } - ) - end - - it 'extracts email attribute' do - expect(deleted_claim.unique_attributes).to eq( - email: 'user@example.com' - ) - end - end - - context 'for gpg_key_subkeys table' do - let(:deleted_claim) do - described_class.new( - source_table: 'gpg_key_subkeys', - deleted_row_identifier: 789, - deleted_row_data: { - 'id' => 789, - 'gpg_key_id' => 111, - 'fingerprint' => '1234abcd', - 'keyid' => '5678ef01' - } - ) - end - - it 'extracts fingerprint and keyid attributes' do - expect(deleted_claim.unique_attributes).to eq( - fingerprint: '1234abcd', - keyid: '5678ef01' - ) - end - end - - context 'for invalid table' do - let(:deleted_claim) do - described_class.new( - source_table: 'invalid_table', - deleted_row_identifier: 999, - deleted_row_data: { 'id' => 999, 'data' => 'value' } - ) - end - - it 'returns empty hash' do - expect(deleted_claim.unique_attributes).to eq({}) - end - end - end - describe '#claim_key' do - let(:deleted_claim) do - described_class.new( - source_table: 'emails', - deleted_row_identifier: 123, - deleted_row_data: {} - ) - end + let(:deleted_claim) { described_class.new(source_table: 'emails', source_record_id: 123) } - it 'returns array with source_table and deleted_row_identifier' do + it 'returns array with source_table and source_record_id' do expect(deleted_claim.claim_key).to eq(['emails', 123]) end end describe 'integration with trigger' do - it 'can be created from trigger data' do - claim = described_class.create!( - deleted_row_identifier: 100, - source_table: 'emails', - deleted_row_data: { - 'id' => 100, - 'email' => 'deleted@example.com', - 'user_id' => 50, - 'created_at' => '2024-01-01T00:00:00', - 'updated_at' => '2024-01-01T00:00:00' - } - ) + it 'creates a deleted claim when an email is destroyed' do + email = create(:email) + + expect { email.destroy! }.to change { described_class.count }.by(1) + + deleted_claim = described_class.last - expect(claim).to be_persisted - expect(claim.deleted_row_data['email']).to eq('deleted@example.com') + expect(deleted_claim.source_table).to eq('emails') + expect(deleted_claim.source_record_id).to eq(email.id) end end end -- GitLab From 616dbc3841e1ef86543750782ab20189f61aacaa Mon Sep 17 00:00:00 2001 From: Bojan Marjanovic Date: Mon, 6 Oct 2025 14:08:29 +0200 Subject: [PATCH 6/6] Fixes failing specs --- spec/db/schema_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index f944b868528177..4f97e7001cdbcc 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -66,6 +66,7 @@ boards: %w[milestone_id iteration_id], broadcast_messages: %w[namespace_id], catalog_resource_component_last_usages: %w[used_by_project_id], # No FK constraint because we want to preserve usage data even if project is deleted. + cells_deleted_claims: %w[source_record_id], chat_names: %w[chat_id team_id], chat_teams: %w[team_id], ci_build_needs: %w[project_id], -- GitLab