diff --git a/app/models/namespace.rb b/app/models/namespace.rb index f39c33655704ec2bff42d8ab1578364aa112e793..f52d7d970b48295cb0fe5f1516b71031bfe8a189 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -63,7 +63,7 @@ class Namespace < ApplicationRecord inverse_of: :namespace, class_name: 'NamespaceSetting', primary_key: :id, foreign_key: :namespace_id has_one :ci_cd_settings, inverse_of: :namespace, class_name: 'NamespaceCiCdSetting', autosave: true - has_one :namespace_details, inverse_of: :namespace, class_name: 'Namespace::Detail', autosave: false + has_one :namespace_details, inverse_of: :namespace, class_name: 'Namespace::Detail', autosave: true has_one :namespace_statistics has_one :namespace_route, foreign_key: :namespace_id, autosave: false, inverse_of: :namespace, class_name: 'Route' has_one :catalog_verified_namespace, class_name: 'Namespaces::VerifiedNamespace', inverse_of: :namespace @@ -203,13 +203,12 @@ class Namespace < ApplicationRecord end end + after_initialize :namespace_details, if: :new_record? before_create :sync_share_with_group_lock_with_parent before_update :sync_share_with_group_lock_with_parent, if: :parent_changed? after_update :force_share_with_group_lock_on_descendants, if: -> { saved_change_to_share_with_group_lock? && share_with_group_lock? } after_update :expire_first_auto_devops_config_cache, if: -> { saved_change_to_auto_devops_enabled? } - after_save :save_namespace_details_changes - after_commit :refresh_access_of_projects_invited_groups, on: :update, if: -> { previous_changes.key?('share_with_group_lock') } after_sync_traversal_ids :schedule_sync_event_worker # custom callback defined in Namespaces::Traversal::Linear @@ -987,16 +986,6 @@ def validate_parent_type end end - def save_namespace_details_changes - attribute_names_to_sync = Namespace::Detail.attribute_names - ['namespace_id'] - attributes_to_sync = namespace_details.changes.slice(*attribute_names_to_sync) - .transform_values { |val| val[1] } - - self.namespace_details = Namespace::Detail.find_by_namespace_id(id) || build_namespace_details - namespace_details.assign_attributes(attributes_to_sync) - namespace_details.save! - end - def sync_share_with_group_lock_with_parent if parent&.share_with_group_lock? self.share_with_group_lock = true diff --git a/db/migrate/20250929123558_remove_sync_namespace_details_triggers.rb b/db/migrate/20250929123558_remove_sync_namespace_details_triggers.rb new file mode 100644 index 0000000000000000000000000000000000000000..75274a27eae93e95a0e85cc3834fdff262827b60 --- /dev/null +++ b/db/migrate/20250929123558_remove_sync_namespace_details_triggers.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +class RemoveSyncNamespaceDetailsTriggers < Gitlab::Database::Migration[2.3] + include Gitlab::Database::SchemaHelpers + + milestone '18.5' + + UPDATE_TRIGGER_NAME = 'trigger_update_details_on_namespace_update' + INSERT_TRIGGER_NAME = 'trigger_update_details_on_namespace_insert' + FUNCTION_NAME = 'update_namespace_details_from_namespaces' + + def up + drop_trigger(:namespaces, UPDATE_TRIGGER_NAME) + drop_trigger(:namespaces, INSERT_TRIGGER_NAME) + drop_function(FUNCTION_NAME) + end + + def down + create_trigger_function(FUNCTION_NAME, replace: true) do + <<~SQL + INSERT INTO + namespace_details ( + description, + description_html, + cached_markdown_version, + updated_at, + created_at, + namespace_id + ) + VALUES + ( + NEW.description, + NEW.description_html, + NEW.cached_markdown_version, + NEW.updated_at, + NEW.updated_at, + NEW.id + ) ON CONFLICT (namespace_id) DO + UPDATE + SET + description = NEW.description, + description_html = NEW.description_html, + cached_markdown_version = NEW.cached_markdown_version, + updated_at = NEW.updated_at + WHERE + namespace_details.namespace_id = NEW.id;RETURN NULL; + SQL + end + + execute(<<~SQL) + CREATE TRIGGER #{UPDATE_TRIGGER_NAME} + AFTER UPDATE ON namespaces + FOR EACH ROW + WHEN ( + NEW.type <> 'Project' AND ( + OLD.description IS DISTINCT FROM NEW.description OR + OLD.description_html IS DISTINCT FROM NEW.description_html OR + OLD.cached_markdown_version IS DISTINCT FROM NEW.cached_markdown_version) + ) + EXECUTE PROCEDURE #{FUNCTION_NAME}(); + SQL + + execute(<<~SQL) + CREATE TRIGGER #{INSERT_TRIGGER_NAME} + AFTER INSERT ON namespaces + FOR EACH ROW + WHEN (NEW.type <> 'Project') + EXECUTE PROCEDURE #{FUNCTION_NAME}(); + SQL + end +end diff --git a/db/schema_migrations/20250929123558 b/db/schema_migrations/20250929123558 new file mode 100644 index 0000000000000000000000000000000000000000..659d3fd8dcff289e59ba97efab7457577d742344 --- /dev/null +++ b/db/schema_migrations/20250929123558 @@ -0,0 +1 @@ +e4925073b86f09dd89a99ea52e94ccc2bba9ac4336dad482a8e10253b8704a84 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 080cfa1218a0f7d9677b2716b394072efc2927c6..13932a62914258d14229c8f18a8a01384f24fe70 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4570,40 +4570,6 @@ RETURN NULL; END $$; -CREATE FUNCTION update_namespace_details_from_namespaces() RETURNS trigger - LANGUAGE plpgsql - AS $$ -BEGIN -INSERT INTO - namespace_details ( - description, - description_html, - cached_markdown_version, - updated_at, - created_at, - namespace_id - ) -VALUES - ( - NEW.description, - NEW.description_html, - NEW.cached_markdown_version, - NEW.updated_at, - NEW.updated_at, - NEW.id - ) ON CONFLICT (namespace_id) DO -UPDATE -SET - description = NEW.description, - description_html = NEW.description_html, - cached_markdown_version = NEW.cached_markdown_version, - updated_at = NEW.updated_at -WHERE - namespace_details.namespace_id = NEW.id;RETURN NULL; - -END -$$; - CREATE FUNCTION update_namespace_details_from_projects() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -47101,10 +47067,6 @@ CREATE TRIGGER trigger_sync_redirect_routes_namespace_id BEFORE INSERT OR UPDATE CREATE TRIGGER trigger_sync_work_item_transitions_from_issues AFTER INSERT OR UPDATE OF moved_to_id, duplicated_to_id, promoted_to_epic_id, namespace_id ON issues FOR EACH ROW EXECUTE FUNCTION sync_work_item_transitions_from_issues(); -CREATE TRIGGER trigger_update_details_on_namespace_insert AFTER INSERT ON namespaces FOR EACH ROW WHEN (((new.type)::text <> 'Project'::text)) EXECUTE FUNCTION update_namespace_details_from_namespaces(); - -CREATE TRIGGER trigger_update_details_on_namespace_update AFTER UPDATE ON namespaces FOR EACH ROW WHEN ((((new.type)::text <> 'Project'::text) AND (((old.description)::text IS DISTINCT FROM (new.description)::text) OR (old.description_html IS DISTINCT FROM new.description_html) OR (old.cached_markdown_version IS DISTINCT FROM new.cached_markdown_version)))) EXECUTE FUNCTION update_namespace_details_from_namespaces(); - CREATE TRIGGER trigger_update_details_on_project_insert AFTER INSERT ON projects FOR EACH ROW EXECUTE FUNCTION update_namespace_details_from_projects(); CREATE TRIGGER trigger_update_details_on_project_update AFTER UPDATE ON projects FOR EACH ROW WHEN (((old.description IS DISTINCT FROM new.description) OR (old.description_html IS DISTINCT FROM new.description_html) OR (old.cached_markdown_version IS DISTINCT FROM new.cached_markdown_version))) EXECUTE FUNCTION update_namespace_details_from_projects(); diff --git a/ee/spec/services/ee/search/group_service_milestones_visibility_spec.rb b/ee/spec/services/ee/search/group_service_milestones_visibility_spec.rb index e9a165cc04c22bc54257129ba6ad162c8874be55..204583d3f83d911b36f080591eca1a25a76dd1f0 100644 --- a/ee/spec/services/ee/search/group_service_milestones_visibility_spec.rb +++ b/ee/spec/services/ee/search/group_service_milestones_visibility_spec.rb @@ -36,17 +36,11 @@ with_them do before do - # We are not modifying namespaces anywhere in this test - # so let's skip this expensive operation that could cause pg query timeout - # example failed job with save_namespace_details_changes causing pg query timeout error: - # https://gitlab.com/gitlab-org/gitlab/-/jobs/11282506950 - Namespace.skip_callback(:save, :after, :save_namespace_details_changes) project.update!( visibility_level: Gitlab::VisibilityLevel.level_value(project_level.to_s), issues_access_level: issues_access_level, merge_requests_access_level: merge_requests_access_level ) - Namespace.set_callback(:save, :after, :save_namespace_details_changes) Elastic::ProcessInitialBookkeepingService.track!(milestone) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 4d61e45ba60eab108a9993519ff1a1a29b509321..af4c2133fa5f7fe442db6bcdc22866dc6d1fb9d6 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -26,7 +26,7 @@ it { is_expected.to have_one :root_storage_statistics } it { is_expected.to have_one :aggregation_schedule } it { is_expected.to have_one :namespace_settings } - it { is_expected.to have_one(:namespace_details).autosave(false) } + it { is_expected.to have_one(:namespace_details).autosave(true) } it { is_expected.to have_one(:namespace_statistics) } it { is_expected.to have_one(:catalog_verified_namespace) } it { is_expected.to have_many :custom_emoji } @@ -326,34 +326,28 @@ it { is_expected.to match("@.q-w_e") } end - describe 'save_namespace_details_changes' do - let(:namespace) { create(:namespace) } - let(:description) { 'my-namespace-description' } - - it 'saves the namespace_details changes' do - namespace_details = namespace.namespace_details - namespace_details.description = description + describe 'after_initialize :namespace_details' do + context 'when creating a new record' do + it 'persists associated namespace_details' do + namespace = create(:namespace) - namespace.save! + namespace_details = namespace.namespace_details + expect(namespace_details.persisted?).to be(true) + expect(namespace_details.namespace_id).to be(namespace.id) + end - namespace_details.reload - expect(namespace_details.description).to eq(description) - end + it 'initializes namespace_details on build' do + namespace = build(:namespace) - context 'when associated namespace_details does not exist' do - before do - namespace.namespace_details.delete - namespace.reload + expect(namespace.namespace_details).not_to be_nil end + end - it 'create namespace_details and saves the changes' do - namespace_details = namespace.namespace_details - namespace_details.description = description - - namespace.save! + context 'when loading an existing record' do + let!(:existing_namespace) { create(:namespace) } - namespace_details.reload - expect(namespace_details.description).to eq(description) + it 'does not raise any errors' do + expect { described_class.find(existing_namespace.id) }.not_to raise_error end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index d1ef32ae5df9342f85dc7459dc82caedc31e586a..4bea62f874c308b8d14bc5f553d3ea6aa0146f71 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1204,7 +1204,7 @@ def make_upload_request expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(new_group_name) - expect(json_response['description']).to eq('') + expect(json_response['description']).to be_nil expect(json_response['visibility']).to eq('public') expect(json_response['share_with_group_lock']).to eq(false) expect(json_response['require_two_factor_authentication']).to eq(false)