Merge branch '25012-add-rubocop-cop-danger-check-for-enums-not-using-smallint' into 'master'

Check for enums not using smallint

Closes #25012

See merge request gitlab-org/gitlab!18179

doc/development/README.md

@@ -118,6 +118,7 @@ description: 'Learn how to contribute to GitLab.'
 - [Query Count Limits](query_count_limits.md)
 - [Database helper modules](database_helpers.md)
 - [Code comments](code_comments.md)
+- [Creating enums](creating_enums.md)
 
 ### Case studies
 

doc/development/creating_enums.md

+# Creating enums
+
+When creating a new enum, it should use the database type `SMALLINT`.
+The `SMALLINT` type size is 2 bytes, which is sufficient for an enum.
+This would help to save space in the database.
+
+To use this type, add `limit: 2` to the migration that creates the column.
+
+Example:
+
+```rb
+def change
+  add_column :ci_job_artifacts, :file_format, :integer, limit: 2
+end
+```

ee/spec/db/schema_support.rb

@@ -29,11 +29,20 @@ module EE
           vulnerability_scanners: %w[external_id],
           web_hooks: %w[group_id]
         }.with_indifferent_access.freeze
+
+        IGNORED_LIMIT_ENUMS = {
+          'SoftwareLicensePolicy' => %w[approval_status],
+          'User' => %w[group_view]
+        }.freeze
       end
 
       def ignored_fk_columns(column)
         super + EE_IGNORED_FK_COLUMNS.fetch(column, [])
       end
+
+      def ignored_limit_enums(model)
+        super + IGNORED_LIMIT_ENUMS.fetch(model, [])
+      end
     end
   end
 end

spec/db/schema_spec.rb

@@ -120,9 +120,55 @@ describe 'Database schema' do
     end
   end
 
+  # These pre-existing enums have limits > 2 bytes
+  IGNORED_LIMIT_ENUMS = {
+    'Analytics::CycleAnalytics::GroupStage' => %w[start_event_identifier end_event_identifier],
+    'Analytics::CycleAnalytics::ProjectStage' => %w[start_event_identifier end_event_identifier],
+    'Ci::Bridge' => %w[failure_reason],
+    'Ci::Build' => %w[failure_reason],
+    'Ci::BuildMetadata' => %w[timeout_source],
+    'Ci::BuildTraceChunk' => %w[data_store],
+    'Ci::JobArtifact' => %w[file_type],
+    'Ci::Pipeline' => %w[source config_source failure_reason],
+    'Ci::Runner' => %w[access_level],
+    'Ci::Stage' => %w[status],
+    'Clusters::Applications::Ingress' => %w[ingress_type],
+    'Clusters::Cluster' => %w[platform_type provider_type],
+    'CommitStatus' => %w[failure_reason],
+    'GenericCommitStatus' => %w[failure_reason],
+    'Gitlab::DatabaseImporters::CommonMetrics::PrometheusMetric' => %w[group],
+    'InternalId' => %w[usage],
+    'List' => %w[list_type],
+    'NotificationSetting' => %w[level],
+    'Project' => %w[auto_cancel_pending_pipelines],
+    'ProjectAutoDevops' => %w[deploy_strategy],
+    'PrometheusMetric' => %w[group],
+    'ResourceLabelEvent' => %w[action],
+    'User' => %w[layout dashboard project_view],
+    'UserCallout' => %w[feature_name],
+    'PrometheusAlert' => %w[operator]
+  }.freeze
+
+  context 'for enums' do
+    ApplicationRecord.descendants.each do |model|
+      describe model do
+        let(:ignored_enums) { ignored_limit_enums(model.name) }
+        let(:enums) { model.defined_enums.keys - ignored_enums }
+
+        it 'uses smallint for enums' do
+          expect(model).to use_smallint_for_enums(enums)
+        end
+      end
+    end
+  end
+
   private
 
   def ignored_fk_columns(column)
     IGNORED_FK_COLUMNS.fetch(column, [])
   end
+
+  def ignored_limit_enums(model)
+    IGNORED_LIMIT_ENUMS.fetch(model, [])
+  end
 end

spec/support/matchers/db_schema_matchers.rb

+# frozen_string_literal: true
+
+EXPECTED_SMALLINT_LIMIT = 2
+
+RSpec::Matchers.define :use_smallint_for_enums do |enums|
+  match do |actual|
+    @failing_enums = enums.select do |enum|
+      enum_type = actual.type_for_attribute(enum)
+      actual_limit = enum_type.send(:subtype).limit
+      actual_limit != EXPECTED_SMALLINT_LIMIT
+    end
+    @failing_enums.empty?
+  end
+
+  failure_message do
+    <<~FAILURE_MESSAGE
+      Expected #{actual.name} enums: #{failing_enums.join(', ')} to use the smallint type.
+
+      The smallint type is 2 bytes which is more than sufficient for an enum.
+      Using the smallint type would help us save space in the database.
+      To fix this, please add `limit: 2` in the migration file, for example:
+
+      def change
+        add_column :ci_job_artifacts, :file_format, :integer, limit: 2
+      end
+    FAILURE_MESSAGE
+  end
+
+  def failing_enums
+    @failing_enums ||= []
+  end
+end