diff --git a/lib/active_record_doctor/detectors/incorrect_length_validation.rb b/lib/active_record_doctor/detectors/incorrect_length_validation.rb index f19a0767..db2db99a 100644 --- a/lib/active_record_doctor/detectors/incorrect_length_validation.rb +++ b/lib/active_record_doctor/detectors/incorrect_length_validation.rb @@ -33,14 +33,16 @@ def message(model:, attribute:, table:, database_maximum:, model_maximum:) def detect each_model(except: config(:ignore_models), existing_tables_only: true) do |model| each_attribute(model, except: config(:ignore_attributes), type: [:string, :text]) do |column| + table = model.table_name model_maximum = maximum_allowed_by_validations(model, column.name.to_sym) - next if model_maximum == column.limit + database_maximum = [column.limit, check_constraint_length_limit(table, column)].compact.min + next if model_maximum == database_maximum problem!( model: model.name, attribute: column.name, - table: model.table_name, - database_maximum: column.limit, + table: table, + database_maximum: database_maximum, model_maximum: model_maximum ) end @@ -55,6 +57,22 @@ def maximum_allowed_by_validations(model, column) end length_validator ? length_validator.options[:maximum] : nil end + + def check_constraint_length_limit(table, column) + # Example: char_length(name::text) <= 64 + pattern = /(char_|character_)?length\(['"`]?#{column.name}(::text)?['"`]?\)\s*(?<=?)\s*(?\d+)/i + + check_constraints(table).each do |definition| + match = definition.match(pattern) + next unless match + + limit = match[:limit].to_i + limit -= 1 if match[:op] == "<" + return limit + end + + nil + end end end end diff --git a/test/active_record_doctor/detectors/incorrect_length_validation_test.rb b/test/active_record_doctor/detectors/incorrect_length_validation_test.rb index f30b9cc4..91eb247a 100644 --- a/test/active_record_doctor/detectors/incorrect_length_validation_test.rb +++ b/test/active_record_doctor/detectors/incorrect_length_validation_test.rb @@ -13,9 +13,70 @@ def test_validation_and_limit_equal_is_ok refute_problems end + def test_validation_and_lteq_check_constraint_limit_equal_is_ok + Context.create_table(:users) do |t| + t.string :email + t.check_constraint "length(email) <= 64" + end.define_model do + validates :email, length: { maximum: 64 } + end + + refute_problems + end + + def test_validation_and_lt_check_constraint_limit_equal_is_ok + Context.create_table(:users) do |t| + t.string :email + t.check_constraint "length(email) < 65" + end.define_model do + validates :email, length: { maximum: 64 } + end + + refute_problems + end + def test_validation_and_limit_different_is_error Context.create_table(:users) do |t| t.string :email, limit: 64 + end.define_model do + validates :email, length: { maximum: 63 } + end + + assert_problems(<<~OUTPUT) + the schema limits users.email to 64 characters but the length validator on Context::User.email enforces a maximum of 63 characters - set both limits to the same value or remove both + OUTPUT + end + + def test_validation_and_check_lteq_constraint_limit_different_is_error + Context.create_table(:users) do |t| + t.string :email + t.check_constraint "length(email) <= 33" + end.define_model do + validates :email, length: { maximum: 32 } + end + + assert_problems(<<~OUTPUT) + the schema limits users.email to 33 characters but the length validator on Context::User.email enforces a maximum of 32 characters - set both limits to the same value or remove both + OUTPUT + end + + def test_validation_and_check_lt_constraint_limit_different_is_error + Context.create_table(:users) do |t| + t.string :email + t.check_constraint "length(email) < 32" + end.define_model do + validates :email, length: { maximum: 32 } + end + + assert_problems(<<~OUTPUT) + the schema limits users.email to 31 characters but the length validator on Context::User.email enforces a maximum of 32 characters - set both limits to the same value or remove both + OUTPUT + end + + def test_validation_and_check_constraint_limit_and_column_limit_different_is_error + Context.create_table(:users) do |t| + t.string :email, limit: 128 + t.check_constraint "length(email) <= 64" end.define_model do validates :email, length: { maximum: 32 } end @@ -50,6 +111,18 @@ def test_no_validation_and_limit_is_error OUTPUT end + def test_no_validation_and_check_constraint_limit_is_error + Context.create_table(:users) do |t| + t.string :email + t.check_constraint "length(email) <= 64" + end.define_model do + end + + assert_problems(<<~OUTPUT) + the schema limits users.email to 64 characters but there's no length validator on Context::User.email - remove the database limit or add the validator + OUTPUT + end + def test_no_validation_and_no_limit_is_ok require_arbitrary_long_text_columns!