diff --git a/.rubocop_todo/migration/ensure_factory_for_table.yml b/.rubocop_todo/migration/ensure_factory_for_table.yml index 762a2b120358515ecc4138d9e1582268b39771ee..10fb9c293dee383e2c834dbe246052ed9169a126 100644 --- a/.rubocop_todo/migration/ensure_factory_for_table.yml +++ b/.rubocop_todo/migration/ensure_factory_for_table.yml @@ -5,26 +5,16 @@ Migration/EnsureFactoryForTable: - 'db/migrate/20230926092944_add_approval_group_rules_groups.rb' - 'db/migrate/20230926093004_add_approval_group_rules_users.rb' - 'db/migrate/20230926093025_add_approval_group_rules_protected_branches.rb' - - 'db/migrate/20231017095738_create_activity_pub_releases_subscriptions.rb' - - 'db/migrate/20231017134349_create_ml_model_metadata.rb' - 'db/migrate/20231020095624_create_audit_events_streaming_http_group_namespace_filters.rb' - - 'db/migrate/20231025123238_create_compliance_framework_security_policies.rb' - 'db/migrate/20231031164724_create_sbom_occurrences_vulnerabilities.rb' - - 'db/migrate/20231102142553_add_zoekt_nodes.rb' - - 'db/migrate/20231107140642_create_audit_events_instance_amazon_s3_configurations.rb' - 'db/migrate/20231110044738_create_abuse_report_assignees_table.rb' - 'db/migrate/20231115064007_create_audit_events_streaming_http_instance_namespace_filters.rb' - 'db/migrate/20231124191759_add_catalog_resource_sync_events_table.rb' - 'db/migrate/20231207150738_add_work_item_dates_sources.rb' - - 'db/migrate/20231215192706_create_ml_model_version_metadata.rb' - 'db/migrate/20231220113459_add_work_item_color.rb' - - 'db/migrate/20231221033539_create_sbom_source_packages_table.rb' - - 'db/migrate/20231221113948_create_cloud_connector_access.rb' - 'db/migrate/20240108123023_create_ai_agents_table.rb' - 'db/migrate/20240108123115_create_ai_agent_versions_table.rb' - 'db/migrate/20240112124030_create_audit_events_group_external_streaming_destinations.rb' - - 'db/migrate/20240115185037_create_packages_terraform_module_metadata.rb' - - 'db/migrate/20240122165824_create_ci_job_token_group_scope_links.rb' - 'db/migrate/20240130162148_create_audit_events_instance_external_streaming_destinations.rb' - 'db/migrate/20240131052824_create_catalog_verified_namespaces.rb' - 'db/migrate/20240201112236_create_group_saved_replies_table.rb' @@ -34,7 +24,3 @@ Migration/EnsureFactoryForTable: - 'db/migrate/20240304184128_create_ci_build_names_table.rb' - 'db/migrate/20240306121653_create_relation_import_tracker.rb' - 'db/migrate/20240404192955_create_early_access_program_tracking_events.rb' - - 'db/migrate/20240419082037_create_ai_self_hosted_models.rb' - - 'db/migrate/20240423064716_create_ci_build_execution_config.rb' - - 'db/migrate/20240430110033_create_ai_feature_settings.rb' - - 'db/migrate/20241127092714_create_container_registry_protection_tag_rules.rb' diff --git a/db/migrate/20240515022318_create_duo_workflows_tables.rb b/db/migrate/20240515022318_create_duo_workflows_tables.rb index ac1ce36bb1763c4765ef8c4392da24e0e115840d..ba00d8e9398801225e8083cf3ef226ed291226bd 100644 --- a/db/migrate/20240515022318_create_duo_workflows_tables.rb +++ b/db/migrate/20240515022318_create_duo_workflows_tables.rb @@ -4,7 +4,7 @@ class CreateDuoWorkflowsTables < Gitlab::Database::Migration[2.2] milestone '17.2' def change - create_table :duo_workflows_workflows do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 + create_table :duo_workflows_workflows do |t| # rubocop:disable Migration/EnsureFactoryForTable, Lint/RedundantCopDisableDirective -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 t.bigint :user_id, null: false, index: true t.bigint :project_id, null: false, index: true diff --git a/db/migrate/20240626125526_create_dependency_list_export_parts_table.rb b/db/migrate/20240626125526_create_dependency_list_export_parts_table.rb index 2f7f2ce54bc7428bbf198582f719fb8d1581ac64..20851dfe73c7a6a9a10506ac9da85184076df62d 100644 --- a/db/migrate/20240626125526_create_dependency_list_export_parts_table.rb +++ b/db/migrate/20240626125526_create_dependency_list_export_parts_table.rb @@ -3,7 +3,6 @@ class CreateDependencyListExportPartsTable < Gitlab::Database::Migration[2.2] milestone '17.2' - # rubocop:disable Migration/EnsureFactoryForTable -- False Positive def change create_table :dependency_list_export_parts do |t| t.references :dependency_list_export, foreign_key: { on_delete: :cascade }, null: false, index: true @@ -15,5 +14,4 @@ def change t.text :file, limit: 255 end end - # rubocop:enable Migration/EnsureFactoryForTable end diff --git a/db/migrate/20240809134250_create_p_ci_build_trace_metadata.rb b/db/migrate/20240809134250_create_p_ci_build_trace_metadata.rb index 822467939937873c68e2a07151dedbc1bacd82f6..36c8bde3310bd5659c17bf8b75e078bf38df4c91 100644 --- a/db/migrate/20240809134250_create_p_ci_build_trace_metadata.rb +++ b/db/migrate/20240809134250_create_p_ci_build_trace_metadata.rb @@ -3,7 +3,6 @@ class CreatePCiBuildTraceMetadata < Gitlab::Database::Migration[2.2] milestone '17.4' - # rubocop:disable Migration/EnsureFactoryForTable -- No factory needed def change create_table(:p_ci_build_trace_metadata, primary_key: [:build_id, :partition_id], options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t| @@ -19,5 +18,4 @@ def change t.index :trace_artifact_id end end - # rubocop:enable Migration/EnsureFactoryForTable -- No factory needed end diff --git a/db/migrate/20240829143538_create_system_access_instance_microsoft_applications.rb b/db/migrate/20240829143538_create_system_access_instance_microsoft_applications.rb index 6c24f7fa617add74abbd2c20fa6eb59a68aee783..0e5e84706f6064baacf9ee3e8c4873d13f24da5c 100644 --- a/db/migrate/20240829143538_create_system_access_instance_microsoft_applications.rb +++ b/db/migrate/20240829143538_create_system_access_instance_microsoft_applications.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop:disable Migration/EnsureFactoryForTable -- False positive class CreateSystemAccessInstanceMicrosoftApplications < Gitlab::Database::Migration[2.2] milestone '17.5' @@ -17,5 +16,3 @@ def change end end end - -# rubocop:enable Migration/EnsureFactoryForTable diff --git a/db/migrate/20240829143654_create_system_access_instance_microsoft_graph_access_tokens.rb b/db/migrate/20240829143654_create_system_access_instance_microsoft_graph_access_tokens.rb index 2a42f2472f0771b728aed0de5b104657799e2699..df804e8dc0b3bab5d440423af9a4a9f399a33b2c 100644 --- a/db/migrate/20240829143654_create_system_access_instance_microsoft_graph_access_tokens.rb +++ b/db/migrate/20240829143654_create_system_access_instance_microsoft_graph_access_tokens.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop:disable Migration/EnsureFactoryForTable -- False positive class CreateSystemAccessInstanceMicrosoftGraphAccessTokens < Gitlab::Database::Migration[2.2] milestone '17.5' @@ -16,5 +15,3 @@ def change end end end - -# rubocop:enable Migration/EnsureFactoryForTable diff --git a/db/migrate/20240829163210_create_instance_integrations_table.rb b/db/migrate/20240829163210_create_instance_integrations_table.rb index 560b18185da1ecaad94677b1b349a6f8d814b81e..15cfbc57927add057a0a2d811ede31b44df10a62 100644 --- a/db/migrate/20240829163210_create_instance_integrations_table.rb +++ b/db/migrate/20240829163210_create_instance_integrations_table.rb @@ -4,7 +4,7 @@ class CreateInstanceIntegrationsTable < Gitlab::Database::Migration[2.2] milestone '17.4' def up - create_table :instance_integrations, id: :bigserial do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 + create_table :instance_integrations, id: :bigserial do |t| t.timestamps_with_timezone null: false t.integer :comment_detail t.boolean :active, default: false, null: false diff --git a/db/migrate/20240829205910_create_ci_job_artifact_reports.rb b/db/migrate/20240829205910_create_ci_job_artifact_reports.rb index 1430653f38cb3eb5c2fb71762136f84187f1affb..394907cd18bc86a9442ad1b527f3506da88c20be 100644 --- a/db/migrate/20240829205910_create_ci_job_artifact_reports.rb +++ b/db/migrate/20240829205910_create_ci_job_artifact_reports.rb @@ -5,7 +5,7 @@ class CreateCiJobArtifactReports < Gitlab::Database::Migration[2.2] disable_ddl_transaction! def up - create_table(:p_ci_job_artifact_reports, # rubocop:disable Migration/EnsureFactoryForTable -- Factory exists + create_table(:p_ci_job_artifact_reports, primary_key: [:job_artifact_id, :partition_id], options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t| t.bigint :job_artifact_id, null: false diff --git a/db/migrate/20240902100524_create_packages_conan_recipe_revisions.rb b/db/migrate/20240902100524_create_packages_conan_recipe_revisions.rb index f75a28679407c4256293f347a61f9420687a3e21..d6758775a87e8ef1c7ea625bc4c1e9e5b0a35bc2 100644 --- a/db/migrate/20240902100524_create_packages_conan_recipe_revisions.rb +++ b/db/migrate/20240902100524_create_packages_conan_recipe_revisions.rb @@ -6,7 +6,7 @@ class CreatePackagesConanRecipeRevisions < Gitlab::Database::Migration[2.2] INDEX_PACKAGE_ID_REVISION = 'idx_on_packages_conan_recipe_revisions_package_id_revision' def up - create_table :packages_conan_recipe_revisions do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 + create_table :packages_conan_recipe_revisions do |t| t.bigint :package_id, null: false t.bigint :project_id, null: false t.timestamps_with_timezone null: false diff --git a/db/migrate/20240906134338_create_packages_conan_package_references.rb b/db/migrate/20240906134338_create_packages_conan_package_references.rb index ea90c4c5c0c6055a89fc577581b8c23f2db3399e..6a29b7f7c3e614b74157d437345f89acae929f25 100644 --- a/db/migrate/20240906134338_create_packages_conan_package_references.rb +++ b/db/migrate/20240906134338_create_packages_conan_package_references.rb @@ -7,7 +7,7 @@ class CreatePackagesConanPackageReferences < Gitlab::Database::Migration[2.2] CONSTRAINT_NAME = 'chk_conan_references_info_length' def up - create_table :packages_conan_package_references do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 + create_table :packages_conan_package_references do |t| t.bigint :package_id, null: false t.bigint :project_id, null: false t.bigint :recipe_revision_id diff --git a/db/migrate/20240910132920_create_ci_job_token_authorizations.rb b/db/migrate/20240910132920_create_ci_job_token_authorizations.rb index e4709ebc2223eaf8b978b1581f083aef47021bb6..4e83f4340957d085775af7bcfba6cbd99d3c73cf 100644 --- a/db/migrate/20240910132920_create_ci_job_token_authorizations.rb +++ b/db/migrate/20240910132920_create_ci_job_token_authorizations.rb @@ -6,7 +6,7 @@ class CreateCiJobTokenAuthorizations < Gitlab::Database::Migration[2.2] INDEX_NAME = 'idx_ci_job_token_authorizations_on_accessed_and_origin_project' def change - create_table(:ci_job_token_authorizations, if_not_exists: true) do |t| # rubocop:disable Migration/EnsureFactoryForTable -- factory at ci/job_token/authorizations.rb + create_table(:ci_job_token_authorizations, if_not_exists: true) do |t| t.bigint :accessed_project_id, null: false t.bigint :origin_project_id, null: false, index: true t.datetime_with_timezone :last_authorized_at, null: false diff --git a/db/migrate/20241106114853_create_system_access_group_microsoft_applications.rb b/db/migrate/20241106114853_create_system_access_group_microsoft_applications.rb index d8f0fea8f43ed83df0e14a686c3472ba732986fa..76eaa21e96f24b1178e52656a828446363d0da74 100644 --- a/db/migrate/20241106114853_create_system_access_group_microsoft_applications.rb +++ b/db/migrate/20241106114853_create_system_access_group_microsoft_applications.rb @@ -5,7 +5,7 @@ class CreateSystemAccessGroupMicrosoftApplications < Gitlab::Database::Migration milestone '17.7' def change - create_table :system_access_group_microsoft_applications do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive, factory name is prefixed with system_access + create_table :system_access_group_microsoft_applications do |t| t.timestamps_with_timezone null: false t.references :group, foreign_key: { to_table: :namespaces, on_delete: :cascade }, null: false t.bigint :temp_source_id, index: { unique: true, name: 'index_group_microsoft_applications_on_temp_source_id' }, diff --git a/db/migrate/20241106115015_create_system_access_group_microsoft_graph_access_tokens.rb b/db/migrate/20241106115015_create_system_access_group_microsoft_graph_access_tokens.rb index ab715dd93c4960965a81e2a3ea4e307c953f1276..a0898f69740d7a9c0800a40e0a7210dabfaaa169 100644 --- a/db/migrate/20241106115015_create_system_access_group_microsoft_graph_access_tokens.rb +++ b/db/migrate/20241106115015_create_system_access_group_microsoft_graph_access_tokens.rb @@ -5,7 +5,7 @@ class CreateSystemAccessGroupMicrosoftGraphAccessTokens < Gitlab::Database::Migr milestone '17.7' def change - create_table :system_access_group_microsoft_graph_access_tokens do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive, factory name is prefixed with system_access + create_table :system_access_group_microsoft_graph_access_tokens do |t| t.timestamps_with_timezone null: false t.references :system_access_group_microsoft_application, index: { name: 'unique_index_group_ms_access_tokens_on_ms_app_id', unique: true } diff --git a/rubocop/cop/migration/ensure_factory_for_table.rb b/rubocop/cop/migration/ensure_factory_for_table.rb index e453605d0606f5992646a4df82307c9a22585c26..1636671bf97182849bc8a8cd3244688de59ebe06 100644 --- a/rubocop/cop/migration/ensure_factory_for_table.rb +++ b/rubocop/cop/migration/ensure_factory_for_table.rb @@ -47,7 +47,8 @@ def on_send(node) return if !ee? && disabled_comment_absent? table_definition(node) do |table_name_node, table_name| - unless factory?(table_name.to_s) + # Partioned tables are prefix with `p_`. + unless factory?(table_name.to_s.delete_prefix('p_')) msg = format(MSG, name: table_name) add_offense(table_name_node, message: msg) end @@ -57,13 +58,13 @@ def on_send(node) private def factory?(table_name) - end_with = "/#{table_name}.rb" - - self.class.factories.any? { |path| path.end_with?(end_with) } + self.class.factories.any? { |name| name.end_with?(table_name) } end def self.factories - @factories ||= Dir.glob("{,ee/,jh/}spec/factories/**/*.rb") + @factories ||= Dir.glob("{,ee/,jh/}spec/factories/**/*.rb").map do |factory| + factory.gsub(%r{^(ee/|jh/|)spec/factories/}, '').delete_suffix('.rb').tr('/', '_') + end end def disabled_comment_absent? diff --git a/spec/rubocop/cop/migration/ensure_factory_for_table_spec.rb b/spec/rubocop/cop/migration/ensure_factory_for_table_spec.rb index e10864dc1d81b612c32f6402732060dbcb7b6f98..208b123b7f7138294ddde8a67cd5a87e502923f7 100644 --- a/spec/rubocop/cop/migration/ensure_factory_for_table_spec.rb +++ b/spec/rubocop/cop/migration/ensure_factory_for_table_spec.rb @@ -83,22 +83,53 @@ end context 'with matching factories' do - let(:factories) { ['spec/factories/users.rb'] } + context 'with regular table' do + let(:factories) { ['users'] } - it 'does not register an offense when a table has a corresponding factory' do - expect_no_offenses(<<~RUBY) - create_table :users do |t| - t.string :name - t.timestamps + it 'does not register an offense when a table has a corresponding factory' do + expect_no_offenses(<<~RUBY) + create_table :users do |t| + t.string :name + t.timestamps + end + RUBY + end + end + + context 'with partitioned table' do + let(:factories) { ['users'] } + + it 'does not register an offense when a table has a corresponding factory' do + expect_no_offenses(<<~RUBY) + create_table :p_users do |t| + t.string :name + t.timestamps + end + RUBY end - RUBY end end end describe '.factories' do + let(:table_names) { %w[unnested_ce nested_ce_factory unnested_ee nested_ee_factory unnested_jh nested_jh_factory] } + let(:factories) do + %w[ + spec/factories/unnested_ce.rb + spec/factories/nested/ce/factory.rb + ee/spec/factories/unnested_ee.rb + ee/spec/factories/nested/ee_factory.rb + jh/spec/factories/unnested_jh.rb + jh/spec/factories/nested_jh/factory.rb + ] + end + + before do + allow(Dir).to receive(:glob).and_return(factories) + end + subject { described_class.factories } - it { is_expected.not_to be_empty } + it { is_expected.to eq(table_names) } end end