Commit 47063524 authored by Mayra Cabrera's avatar Mayra Cabrera Committed by Stan Hu

Adds cop to enforce string limits on migrations

This cop will analyze migrations that add columns with string, and
report an offense if the string has no limit enforced

Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/64505
parent b8dec7ec
......@@ -5,6 +5,7 @@
# rubocop:disable Metrics/AbcSize
# rubocop:disable Migration/AddConcurrentForeignKey
# rubocop:disable Style/WordArray
# rubocop:disable Migration/AddLimitToStringColumns
class InitSchema < ActiveRecord::Migration[4.2]
DOWNTIME = false
......@@ -1852,3 +1853,4 @@ class InitSchema < ActiveRecord::Migration[4.2]
raise ActiveRecord::IrreversibleMigration, "The initial migration is not revertable"
end
end
# rubocop:enable Migration/AddLimitToStringColumns
......@@ -4,6 +4,7 @@ class CreatePrometheusMetrics < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :prometheus_metrics do |t|
t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false
t.string :title, null: false
......@@ -14,5 +15,6 @@ class CreatePrometheusMetrics < ActiveRecord::Migration[4.2]
t.integer :group, null: false, index: true
t.timestamps_with_timezone null: false
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -8,6 +8,6 @@ class AddAutoDevopsDomainToApplicationSettings < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :application_settings, :auto_devops_domain, :string
add_column :application_settings, :auto_devops_domain, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -8,7 +8,9 @@ class AddUploadsBuilderContext < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
add_column :uploads, :mount_point, :string
add_column :uploads, :secret, :string
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -4,6 +4,6 @@ class AddExternalIpToClustersApplicationsIngress < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :clusters_applications_ingress, :external_ip, :string
add_column :clusters_applications_ingress, :external_ip, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -2,6 +2,7 @@ class CreateBadges < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :badges do |t|
t.string :link_url, null: false
t.string :image_url, null: false
......@@ -11,6 +12,7 @@ class CreateBadges < ActiveRecord::Migration[4.2]
t.timestamps_with_timezone null: false
end
# rubocop:enable Migration/AddLimitToStringColumns
# rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :badges, :namespaces, column: :group_id, on_delete: :cascade
......
......@@ -13,7 +13,7 @@ class CreateClustersApplicationsRunners < ActiveRecord::Migration[4.2]
t.index :cluster_id, unique: true
t.integer :status, null: false
t.timestamps_with_timezone null: false
t.string :version, null: false
t.string :version, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.text :status_reason
end
......
......@@ -3,6 +3,6 @@ class AddPagesDomainVerification < ActiveRecord::Migration[4.2]
def change
add_column :pages_domains, :verified_at, :datetime_with_timezone
add_column :pages_domains, :verification_code, :string
add_column :pages_domains, :verification_code, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -4,6 +4,6 @@ class AddIpAddressToRunner < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :ci_runners, :ip_address, :string
add_column :ci_runners, :ip_address, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -4,7 +4,7 @@ class AddUserInternalRegexToApplicationSetting < ActiveRecord::Migration[4.2]
DOWNTIME = false
def up
add_column :application_settings, :user_default_internal_regex, :string, null: true
add_column :application_settings, :user_default_internal_regex, :string, null: true # rubocop:disable Migration/AddLimitToStringColumns
end
def down
......
......@@ -2,6 +2,7 @@ class AddExternalAuthMutualTlsFieldsToProjectSettings < ActiveRecord::Migration[
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
add_column :application_settings,
:external_auth_client_cert, :text
add_column :application_settings,
......@@ -12,5 +13,6 @@ class AddExternalAuthMutualTlsFieldsToProjectSettings < ActiveRecord::Migration[
:encrypted_external_auth_client_key_pass, :string
add_column :application_settings,
:encrypted_external_auth_client_key_pass_iv, :string
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -2,6 +2,7 @@ class CreateDeployTokens < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :deploy_tokens do |t|
t.boolean :revoked, default: false
t.boolean :read_repository, null: false, default: false
......@@ -15,5 +16,6 @@ class CreateDeployTokens < ActiveRecord::Migration[4.2]
t.index [:token, :expires_at, :id], where: "(revoked IS FALSE)"
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -3,6 +3,7 @@ class CreateProjectMirrorData < ActiveRecord::Migration[4.2]
DOWNTIME = false
# rubocop:disable Migration/AddLimitToStringColumns
def up
if table_exists?(:project_mirror_data)
add_column :project_mirror_data, :status, :string unless column_exists?(:project_mirror_data, :status)
......@@ -17,6 +18,7 @@ class CreateProjectMirrorData < ActiveRecord::Migration[4.2]
end
end
end
# rubocop:enable Migration/AddLimitToStringColumns
def down
remove_column :project_mirror_data, :status
......
......@@ -5,6 +5,7 @@ class CreateRemoteMirrors < ActiveRecord::Migration[4.2]
disable_ddl_transaction!
# rubocop:disable Migration/AddLimitToStringColumns
def up
return if table_exists?(:remote_mirrors)
......@@ -27,6 +28,7 @@ class CreateRemoteMirrors < ActiveRecord::Migration[4.2]
t.timestamps null: false
end
end
# rubocop:enable Migration/AddLimitToStringColumns
def down
# ee/db/migrate/20160321161032_create_remote_mirrors_ee.rb will remove the table
......
......@@ -4,8 +4,8 @@ class EnsureMissingColumnsToProjectMirrorData < ActiveRecord::Migration[4.2]
DOWNTIME = false
def up
add_column :project_mirror_data, :status, :string unless column_exists?(:project_mirror_data, :status)
add_column :project_mirror_data, :jid, :string unless column_exists?(:project_mirror_data, :jid)
add_column :project_mirror_data, :status, :string unless column_exists?(:project_mirror_data, :status) # rubocop:disable Migration/AddLimitToStringColumns
add_column :project_mirror_data, :jid, :string unless column_exists?(:project_mirror_data, :jid) # rubocop:disable Migration/AddLimitToStringColumns
add_column :project_mirror_data, :last_error, :text unless column_exists?(:project_mirror_data, :last_error)
end
......
......@@ -7,17 +7,19 @@ class CreateClustersApplicationsJupyter < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :clusters_applications_jupyter do |t|
t.references :cluster, null: false, unique: true, foreign_key: { on_delete: :cascade }
t.references :oauth_application, foreign_key: { on_delete: :nullify }
t.integer :status, null: false
t.string :version, null: false
t.string :hostname
t.string :version, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.string :hostname # rubocop:disable Migration/AddLimitToStringColumns
t.timestamps_with_timezone null: false
t.text :status_reason
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -4,6 +4,7 @@ class CreateNotesDiffFiles < ActiveRecord::Migration[4.2]
disable_ddl_transaction!
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :note_diff_files do |t|
t.references :diff_note, references: :notes, null: false, index: { unique: true }
t.text :diff, null: false
......@@ -18,5 +19,6 @@ class CreateNotesDiffFiles < ActiveRecord::Migration[4.2]
# rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :note_diff_files, :notes, column: :diff_note_id, on_delete: :cascade
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -8,7 +8,7 @@ class EnsureRemoteMirrorColumns < ActiveRecord::Migration[4.2]
def up
# rubocop:disable Migration/Datetime
add_column :remote_mirrors, :last_update_started_at, :datetime unless column_exists?(:remote_mirrors, :last_update_started_at)
add_column :remote_mirrors, :remote_name, :string unless column_exists?(:remote_mirrors, :remote_name)
add_column :remote_mirrors, :remote_name, :string unless column_exists?(:remote_mirrors, :remote_name) # rubocop:disable Migration/AddLimitToStringColumns
unless column_exists?(:remote_mirrors, :only_protected_branches)
add_column_with_default(:remote_mirrors,
......
......@@ -4,6 +4,7 @@ class AddRepositoryLanguages < ActiveRecord::Migration[4.2]
DOWNTIME = false
def up
# rubocop:disable Migration/AddLimitToStringColumns
create_table(:programming_languages) do |t|
t.string :name, null: false
t.string :color, null: false
......@@ -19,6 +20,7 @@ class AddRepositoryLanguages < ActiveRecord::Migration[4.2]
add_index :programming_languages, :name, unique: true
add_index :repository_languages, [:project_id, :programming_language_id],
unique: true, name: "index_repository_languages_on_project_and_languages_id"
# rubocop:enable Migration/AddLimitToStringColumns
end
def down
......
......@@ -8,6 +8,7 @@ class CreateCiBuildsRunnerSession < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :ci_builds_runner_session, id: :bigserial do |t|
t.integer :build_id, null: false
t.string :url, null: false
......@@ -17,5 +18,6 @@ class CreateCiBuildsRunnerSession < ActiveRecord::Migration[4.2]
t.foreign_key :ci_builds, column: :build_id, on_delete: :cascade
t.index :build_id, unique: true
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,7 @@ class CreateUserStatuses < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :user_statuses, id: false, primary_key: :user_id do |t|
t.references :user,
foreign_key: { on_delete: :cascade },
......@@ -16,5 +17,6 @@ class CreateUserStatuses < ActiveRecord::Migration[4.2]
t.string :message, limit: 100
t.string :message_html
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -28,6 +28,6 @@ class AddCommitEmailToUsers < ActiveRecord::Migration[4.2]
# disable_ddl_transaction!
def change
add_column :users, :commit_email, :string
add_column :users, :commit_email, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,6 @@ class AddIdentifierToPrometheusMetric < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :prometheus_metrics, :identifier, :string
add_column :prometheus_metrics, :identifier, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,10 +6,12 @@ class AddAttrEncryptedColumnsToWebHook < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
add_column :web_hooks, :encrypted_token, :string
add_column :web_hooks, :encrypted_token_iv, :string
add_column :web_hooks, :encrypted_url, :string
add_column :web_hooks, :encrypted_url_iv, :string
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -8,7 +8,7 @@ class AddTokenDigestToPersonalAccessTokens < ActiveRecord::Migration[4.2]
def up
change_column :personal_access_tokens, :token, :string, null: true
add_column :personal_access_tokens, :token_digest, :string
add_column :personal_access_tokens, :token_digest, :string # rubocop:disable Migration/AddLimitToStringColumns
end
def down
......
......@@ -6,6 +6,7 @@ class AddKnativeApplication < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table "clusters_applications_knative" do |t|
t.references :cluster, null: false, unique: true, foreign_key: { on_delete: :cascade }
......@@ -16,5 +17,6 @@ class AddKnativeApplication < ActiveRecord::Migration[4.2]
t.string "hostname"
t.text "status_reason"
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -5,6 +5,7 @@ class CreateClustersKubernetesNamespaces < ActiveRecord::Migration[4.2]
INDEX_NAME = 'kubernetes_namespaces_cluster_and_namespace'
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :clusters_kubernetes_namespaces, id: :bigserial do |t|
t.references :cluster, null: false, index: true, foreign_key: { on_delete: :cascade }
t.references :project, index: true, foreign_key: { on_delete: :nullify }
......@@ -20,5 +21,6 @@ class CreateClustersKubernetesNamespaces < ActiveRecord::Migration[4.2]
t.index [:cluster_id, :namespace], name: INDEX_NAME, unique: true
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -5,7 +5,7 @@ class AddShardsTable < ActiveRecord::Migration[4.2]
def change
create_table :shards do |t|
t.string :name, null: false, index: { unique: true }
t.string :name, null: false, index: { unique: true } # rubocop:disable Migration/AddLimitToStringColumns
end
end
end
......@@ -6,7 +6,7 @@ class AddRepositoriesTable < ActiveRecord::Migration[4.2]
def change
create_table :repositories, id: :bigserial do |t|
t.references :shard, null: false, index: true, foreign_key: { on_delete: :restrict }
t.string :disk_path, null: false, index: { unique: true }
t.string :disk_path, null: false, index: { unique: true } # rubocop:disable Migration/AddLimitToStringColumns
end
add_column :projects, :pool_repository_id, :bigint
......
......@@ -6,6 +6,6 @@ class AddPrivateCommitEmailHostnameToApplicationSettings < ActiveRecord::Migrati
DOWNTIME = false
def change
add_column(:application_settings, :commit_email_hostname, :string, null: true)
add_column(:application_settings, :commit_email_hostname, :string, null: true) # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -10,6 +10,7 @@ class DropGcpClustersTable < ActiveRecord::Migration[4.2]
end
def down
# rubocop:disable Migration/AddLimitToStringColumns
create_table :gcp_clusters do |t|
# Order columns by best align scheme
t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade }
......@@ -49,5 +50,6 @@ class DropGcpClustersTable < ActiveRecord::Migration[4.2]
t.text :encrypted_gcp_token
t.string :encrypted_gcp_token_iv
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,7 @@ class CreateClustersApplicationsCertManager < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :clusters_applications_cert_managers do |t|
t.references :cluster, null: false, index: false, foreign_key: { on_delete: :cascade }
t.integer :status, null: false
......@@ -15,5 +16,6 @@ class CreateClustersApplicationsCertManager < ActiveRecord::Migration[4.2]
t.text :status_reason
t.index :cluster_id, unique: true
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,6 @@ class AddEncryptedRunnersTokenToSettings < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :application_settings, :runners_registration_token_encrypted, :string
add_column :application_settings, :runners_registration_token_encrypted, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -9,6 +9,6 @@ class KnativeExternalIp < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :clusters_applications_knative, :external_ip, :string
add_column :clusters_applications_knative, :external_ip, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,6 @@ class AddEncryptedRunnersTokenToNamespaces < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :namespaces, :runners_token_encrypted, :string
add_column :namespaces, :runners_token_encrypted, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,6 @@ class AddEncryptedRunnersTokenToProjects < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :projects, :runners_token_encrypted, :string
add_column :projects, :runners_token_encrypted, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,6 @@ class AddTokenEncryptedToCiRunners < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column :ci_runners, :token_encrypted, :string
add_column :ci_runners, :token_encrypted, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -11,7 +11,7 @@ class CreateProjectRepositories < ActiveRecord::Migration[5.0]
def change
create_table :project_repositories, id: :bigserial do |t|
t.references :shard, null: false, index: true, foreign_key: { on_delete: :restrict }
t.string :disk_path, null: false, index: { unique: true }
t.string :disk_path, null: false, index: { unique: true } # rubocop:disable Migration/AddLimitToStringColumns
t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade }
end
end
......
......@@ -8,7 +8,7 @@ class CreateSuggestions < ActiveRecord::Migration[5.0]
t.references :note, foreign_key: { on_delete: :cascade }, null: false
t.integer :relative_order, null: false, limit: 2
t.boolean :applied, null: false, default: false
t.string :commit_id
t.string :commit_id # rubocop:disable Migration/AddLimitToStringColumns
t.text :from_content, null: false
t.text :to_content, null: false
......
......@@ -9,7 +9,7 @@ class AddStateToPoolRepository < ActiveRecord::Migration[5.0]
# the transactions don't have to be disabled
# rubocop: disable Migration/AddConcurrentForeignKey, Migration/AddIndex
def change
add_column(:pool_repositories, :state, :string, null: true)
add_column(:pool_repositories, :state, :string, null: true) # rubocop:disable Migration/AddLimitToStringColumns
add_column :pool_repositories, :source_project_id, :integer
add_index :pool_repositories, :source_project_id, unique: true
......
......@@ -6,6 +6,6 @@ class AddTokenEncryptedToCiBuilds < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :ci_builds, :token_encrypted, :string
add_column :ci_builds, :token_encrypted, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -4,6 +4,6 @@ class AddProjectBfgObjectMapColumn < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :projects, :bfg_object_map, :string
add_column :projects, :bfg_object_map, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -7,7 +7,7 @@ class AddNameAuthorIdAndShaToReleases < ActiveRecord::Migration[5.0]
def change
add_column :releases, :author_id, :integer
add_column :releases, :name, :string
add_column :releases, :sha, :string
add_column :releases, :name, :string # rubocop:disable Migration/AddLimitToStringColumns
add_column :releases, :sha, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,7 @@ class CreateErrorTrackingSettings < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :project_error_tracking_settings, id: :int, primary_key: :project_id, default: nil do |t|
t.boolean :enabled, null: false, default: true
t.string :api_url, null: false
......@@ -13,5 +14,6 @@ class CreateErrorTrackingSettings < ActiveRecord::Migration[5.0]
t.string :encrypted_token_iv
t.foreign_key :projects, column: :project_id, on_delete: :cascade
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,7 @@ class CreateReleasesLinkTable < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :release_links, id: :bigserial do |t|
t.references :release, null: false, index: false, foreign_key: { on_delete: :cascade }
t.string :url, null: false
......@@ -15,5 +16,6 @@ class CreateReleasesLinkTable < ActiveRecord::Migration[5.0]
t.index [:release_id, :url], unique: true
t.index [:release_id, :name], unique: true
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -11,7 +11,7 @@ class AddMergeRequestExternalDiffs < ActiveRecord::Migration[5.0]
def change
# Allow the merge request diff to store details about an external file
add_column :merge_request_diffs, :external_diff, :string
add_column :merge_request_diffs, :external_diff, :string # rubocop:disable Migration/AddLimitToStringColumns
add_column :merge_request_diffs, :external_diff_store, :integer
add_column :merge_request_diffs, :stored_externally, :boolean
......
......@@ -4,6 +4,6 @@ class AddDomainToCluster < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :clusters, :domain, :string
add_column :clusters, :domain, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,8 +6,8 @@ class AddColumnsProjectErrorTrackingSettings < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :project_error_tracking_settings, :project_name, :string
add_column :project_error_tracking_settings, :organization_name, :string
add_column :project_error_tracking_settings, :project_name, :string # rubocop:disable Migration/AddLimitToStringColumns
add_column :project_error_tracking_settings, :organization_name, :string # rubocop:disable Migration/AddLimitToStringColumns
change_column_default :project_error_tracking_settings, :enabled, from: true, to: false
......
......@@ -10,8 +10,8 @@ class AddSortingFieldsToUserPreference < ActiveRecord::Migration[5.0]
DOWNTIME = false
def up
add_column :user_preferences, :issues_sort, :string
add_column :user_preferences, :merge_requests_sort, :string
add_column :user_preferences, :issues_sort, :string # rubocop:disable Migration/AddLimitToStringColumns
add_column :user_preferences, :merge_requests_sort, :string # rubocop:disable Migration/AddLimitToStringColumns
end
def down
......
......@@ -4,7 +4,7 @@ class AddExternalHostnameToIngressAndKnative < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :clusters_applications_ingress, :external_hostname, :string
add_column :clusters_applications_knative, :external_hostname, :string
add_column :clusters_applications_ingress, :external_hostname, :string # rubocop:disable Migration/AddLimitToStringColumns
add_column :clusters_applications_knative, :external_hostname, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -10,6 +10,6 @@ class AddLetsEncryptNotificationEmailToApplicationSettings < ActiveRecord::Migra
DOWNTIME = false
def change
add_column :application_settings, :lets_encrypt_notification_email, :string
add_column :application_settings, :lets_encrypt_notification_email, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -11,7 +11,7 @@ class AddFieldsToUserPreferences < ActiveRecord::Migration[5.0]
DOWNTIME = false
def up
add_column(:user_preferences, :timezone, :string)
add_column(:user_preferences, :timezone, :string) # rubocop:disable Migration/AddLimitToStringColumns
add_column(:user_preferences, :time_display_relative, :boolean)
add_column(:user_preferences, :time_format_in_24h, :boolean)
end
......
......@@ -6,6 +6,6 @@ class AddNotificationEmailToNotificationSettings < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :notification_settings, :notification_email, :string
add_column :notification_settings, :notification_email, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -2,6 +2,7 @@
# rubocop: disable Metrics/AbcSize
# rubocop: disable Migration/Datetime
# rubocop: disable Migration/AddLimitToStringColumns
class BackportEnterpriseSchema < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
......@@ -2190,3 +2191,4 @@ class BackportEnterpriseSchema < ActiveRecord::Migration[5.0]
end
# rubocop: enable Metrics/AbcSize
# rubocop: enable Migration/Datetime
# rubocop: enable Migration/AddLimitToStringColumns
......@@ -10,7 +10,7 @@ class AddNameToGeoNodes < ActiveRecord::Migration[5.0]
DOWNTIME = false
def up
add_column :geo_nodes, :name, :string
add_column :geo_nodes, :name, :string # rubocop:disable Migration/AddLimitToStringColumns
# url is also unique, and its type and size is identical to the name column,
# so this is safe.
......
......@@ -7,7 +7,7 @@ class CreateProjectMetricsSettings < ActiveRecord::Migration[5.0]
def change
create_table :project_metrics_settings, id: :int, primary_key: :project_id, default: nil do |t|
t.string :external_dashboard_url, null: false
t.string :external_dashboard_url, null: false # rubocop:disable Migration/AddLimitToStringColumns
t.foreign_key :projects, column: :project_id, on_delete: :cascade
end
end
......
......@@ -10,6 +10,7 @@ class CreatePagesDomainAcmeOrders < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :pages_domain_acme_orders do |t|
t.references :pages_domain, null: false, index: true, foreign_key: { on_delete: :cascade }, type: :integer
......@@ -24,5 +25,6 @@ class CreatePagesDomainAcmeOrders < ActiveRecord::Migration[5.1]
t.text :encrypted_private_key, null: false
t.text :encrypted_private_key_iv, null: false
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -9,6 +9,7 @@ class CreateIssueTrackerData < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :issue_tracker_data do |t|
t.references :service, foreign_key: { on_delete: :cascade }, type: :integer, index: true, null: false
t.timestamps_with_timezone
......@@ -19,5 +20,6 @@ class CreateIssueTrackerData < ActiveRecord::Migration[5.1]
t.string :encrypted_new_issue_url
t.string :encrypted_new_issue_url_iv
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -9,6 +9,7 @@ class CreateJiraTrackerData < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :jira_tracker_data do |t|
t.references :service, foreign_key: { on_delete: :cascade }, type: :integer, index: true, null: false
t.timestamps_with_timezone
......@@ -22,5 +23,6 @@ class CreateJiraTrackerData < ActiveRecord::Migration[5.1]
t.string :encrypted_password_iv
t.string :jira_issue_transition_id
end
# rubocop:enable Migration/AddLimitToStringColumns
end
end
......@@ -12,7 +12,7 @@ class CreateIpRestriction < ActiveRecord::Migration[5.1]
type: :integer,
null: false,
index: true
t.string :range, null: false
t.string :range, null: false # rubocop:disable Migration/AddLimitToStringColumns
end
add_foreign_key(:ip_restrictions, :namespaces, column: :group_id, on_delete: :cascade) # rubocop: disable Migration/AddConcurrentForeignKey
......
......@@ -10,6 +10,6 @@ class AddRequiredTemplateNameToApplicationSettings < ActiveRecord::Migration[5.1
DOWNTIME = false
def change
add_column :application_settings, :required_instance_ci_template, :string, null: true
add_column :application_settings, :required_instance_ci_template, :string, null: true # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -6,6 +6,6 @@ class AddTokenEncryptedToOperationsFeatureFlagsClients < ActiveRecord::Migration
DOWNTIME = false
def change
add_column :operations_feature_flags_clients, :token_encrypted, :string
add_column :operations_feature_flags_clients, :token_encrypted, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -4,6 +4,6 @@ class AddUsernameToDeployTokens < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
add_column :deploy_tokens, :username, :string
add_column :deploy_tokens, :username, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -8,7 +8,7 @@ class CreateProjectAliases < ActiveRecord::Migration[5.1]
def change
create_table :project_aliases do |t|
t.references :project, null: false, index: true, foreign_key: { on_delete: :cascade }, type: :integer
t.string :name, null: false, index: { unique: true }
t.string :name, null: false, index: { unique: true } # rubocop:disable Migration/AddLimitToStringColumns
t.timestamps_with_timezone null: false
end
......
......@@ -4,6 +4,6 @@ class AddMergeRequestRebaseJid < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
add_column :merge_requests, :rebase_jid, :string
add_column :merge_requests, :rebase_jid, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -8,8 +8,10 @@ class AddGrafanaUrlToSettings < ActiveRecord::Migration[5.1]
DOWNTIME = false
def up
# rubocop:disable Migration/AddLimitToStringColumns
add_column_with_default(:application_settings, :grafana_url, :string,
default: '/-/grafana', allow_null: false)
# rubocop:enable Migration/AddLimitToStringColumns
end
def down
......
......@@ -10,6 +10,7 @@ class CreateJobVariables < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
# rubocop:disable Migration/AddLimitToStringColumns
create_table :ci_job_variables do |t|
t.string :key, null: false
t.text :encrypted_value
......@@ -17,6 +18,7 @@ class CreateJobVariables < ActiveRecord::Migration[5.1]
t.references :job, null: false, index: true, foreign_key: { to_table: :ci_builds, on_delete: :cascade }
t.integer :variable_type, null: false, limit: 2, default: 1
end
# rubocop:enable Migration/AddLimitToStringColumns
add_index :ci_job_variables, [:key, :job_id], unique: true
end
......
......@@ -12,6 +12,6 @@ class RemoveKodingFromApplicationSettings < ActiveRecord::Migration[4.2]
def down
add_column :application_settings, :koding_enabled, :boolean # rubocop:disable Migration/SaferBooleanColumn
add_column :application_settings, :koding_url, :string
add_column :application_settings, :koding_url, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -16,6 +16,6 @@ class RemoveAlternateUrlFromGeoNodes < ActiveRecord::Migration[5.0]
end
def down
add_column :geo_nodes, :alternate_url, :string
add_column :geo_nodes, :alternate_url, :string # rubocop:disable Migration/AddLimitToStringColumns
end
end
......@@ -32,7 +32,7 @@ class RemoveSentryFromApplicationSettings < ActiveRecord::Migration[5.0]
end
SENTRY_DSN_COLUMNS.each do |column|
add_column(:application_settings, column, :string) unless column_exists?(:application_settings, column)
add_column(:application_settings, column, :string) unless column_exists?(:application_settings, column) # rubocop:disable Migration/AddLimitToStringColumns
end
end
end
# frozen_string_literal: true
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that enforces length constraints to string columns
class AddLimitToStringColumns < RuboCop::Cop::Cop
include MigrationHelpers
ADD_COLUMNS_METHODS = %i(add_column add_column_with_default).freeze
MSG = 'String columns should have a limit constraint. 255 is suggested'.freeze
def on_def(node)
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
next unless string_operation?(send_node)
add_offense(send_node, location: :selector) unless limit_on_string_column?(send_node)
end
end
private
def string_operation?(node)
modifier = node.children[0]
migration_method = node.children[1]
if migration_method == :string
modifier.type == :lvar
elsif ADD_COLUMNS_METHODS.include?(migration_method)
modifier.nil? && string_column?(node.children[4])
end
end
def string_column?(column_type)
column_type.type == :sym && column_type.value == :string
end
def limit_on_string_column?(node)
migration_method = node.children[1]
if migration_method == :string
limit_present?(node.children)
elsif ADD_COLUMNS_METHODS.include?(migration_method)
limit_present?(node)
end
end
def limit_present?(statement)
!(statement.to_s =~ /:limit/).nil?
end
end
end
end
end
......@@ -17,6 +17,7 @@ require_relative 'cop/migration/add_column'
require_relative 'cop/migration/add_concurrent_foreign_key'
require_relative 'cop/migration/add_concurrent_index'
require_relative 'cop/migration/add_index'
require_relative 'cop/migration/add_limit_to_string_columns'
require_relative 'cop/migration/add_reference'
require_relative 'cop/migration/add_timestamps'
require_relative 'cop/migration/datetime'
......
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/add_limit_to_string_columns'
describe RuboCop::Cop::Migration::AddLimitToStringColumns do
include CopHelper
subject(:cop) { described_class.new }
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
inspect_source(migration)
end
context 'when creating a table' do
context 'with string columns and limit' do
let(:migration) do
%q(
class CreateUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false, limit: 255
t.timestamps_with_timezone null: true
end
end
end
)
end
it 'register no offense' do
expect(cop.offenses.size).to eq(0)
end
context 'with limit in a different position' do
let(:migration) do
%q(
class CreateUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, limit: 255, null: false
t.timestamps_with_timezone null: true
end
end
end
)
end
it 'registers an offense' do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'with string columns and no limit' do
let(:migration) do
%q(
class CreateUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
t.timestamps_with_timezone null: true
end
end
end
)
end
it 'registers an offense' do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.first.message)
.to eq('String columns should have a limit constraint. 255 is suggested')
end
end
context 'with no string columns' do
let(:migration) do
%q(
class CreateMilestoneReleases < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :milestone_releases do |t|
t.integer :milestone_id
t.integer :release_id
end
end
end
)
end
it 'register no offense' do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'when adding columns' do
context 'with string columns with limit' do
let(:migration) do
%q(
class AddEmailToUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :users, :email, :string, limit: 255
end
end
)
end
it 'registers no offense' do
expect(cop.offenses.size).to eq(0)
end
context 'with limit in a different position' do
let(:migration) do
%q(
class AddEmailToUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :users, :email, :string, limit: 255, default: 'example@email.com'
end
end
)
end
it 'registers no offense' do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'with string columns with no limit' do
let(:migration) do
%q(
class AddEmailToUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :users, :email, :string
end
end
)
end
it 'registers offense' do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.first.message)
.to eq('String columns should have a limit constraint. 255 is suggested')
end
end
context 'with no string columns' do
let(:migration) do
%q(
class AddEmailToUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :users, :active, :boolean, default: false
end
end
)
end
it 'registers no offense' do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'with add_column_with_default' do
context 'with a limit' do
let(:migration) do
%q(
class AddRuleTypeToApprovalMergeRequestRules < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column_with_default(:approval_merge_request_rules, :rule_type, :string, limit: 2, default: 1)
end
end
)
end
it 'registers no offense' do
expect(cop.offenses.size).to eq(0)
end
end
context 'without a limit' do
let(:migration) do
%q(
class AddRuleTypeToApprovalMergeRequestRules < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column_with_default(:approval_merge_request_rules, :rule_type, :string, default: 1)
end
end
)
end
it 'registers an offense' do
expect(cop.offenses.size).to eq(1)
end
end
end
context 'with methods' do
let(:migration) do
%q(
class AddEmailToUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column_if_table_not_exists :users, :first_name, :string, limit: 255
search_namespace(user_name)
end
def add_column_if_not_exists(table, name, *args)
add_column(table, name, *args) unless column_exists?(table, name)
end
def search_namespace(username)
Uniquify.new.string(username) do |str|
query = "SELECT id FROM namespaces WHERE parent_id IS NULL AND path='#{str}' LIMIT 1"
connection.exec_query(query)
end
end
end
)
end
it 'registers no offense' do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'outside of migrations' do
let(:active_record_model) do
%q(
class User < ApplicationRecord
end
)
end
it 'registers no offense' do
inspect_source(active_record_model)
expect(cop.offenses.size).to eq(0)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment