Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions lib/active_record_doctor/detectors/incorrect_length_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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*(?<op><=?)\s*(?<limit>\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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!

Expand Down