allow_nil and allow_blank do not work correctly with defaulted attribute accessors

[aardvarkk]

Steps to reproduce

• I have a model that has a default accessor in the model, such that if the underlying value in the database is nil it will return a default value
• It should still be possible to set a nil/empty value when creating a new instance
• When I dove into the code, it looks like allow_nil/allow_blank end up being checked against the defaulted accessor
• Once the actual numericality check happens, it happens against the raw value, which is actually empty/blank
• The instance is marked invalid even though a blank value should be allowed

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :foos, force: true do |t|
    t.integer :bar
  end
end

class Foo < ActiveRecord::Base
  validates :bar, numericality: { greater_than: 100, allow_blank: true }

  # This is the problematic line
  # If we comment out the defaulted accessor, the model is valid with a blank value for `bar`
  def bar
    self[:bar] || 123
  end
end

class BugTest < Minitest::Test
  def test_validation
    foo = Foo.new
    foo[:bar] = ''

    assert foo.valid?
  end
end

Expected behavior

foo should be valid since blank values are allowed

Actual behavior

foo is invalid, I believe due to the fact that allow_blank is checked against the defaulted value, and once numericality is checked on the raw value the blank string is not a proper number so a validation error occurs.

System configuration

Rails version:
master (6.1.0 alpha), but I first noticed the issue in Rails v5.2.3

Ruby version:
2.6.3

[aardvarkk]

A workaround for me is to not use validates on the model, and instead to write a custom validate method in which I can explicitly check for a blank value before continuing on to check the numericality constraints.

[joshmn]

@aardvarkk, I have this patched on one of my projects. Do you mind if I make a PR for it?

[aardvarkk]

Go for it @joshmn ! This issue didn't get any response, so I assumed maybe I was crazy or nobody else thought this was a problem.

[lxxxvi]

Hello

Firstly, allow_nil and allow_blank are not valid options for the numericality validation (see the guides).

I tried your example with allow_blank: true outside of the numericality validation, like so:

validates :bar, numericality: { greater_than: 100 }, allow_blank: true

But this didn't work either because ActiveRecord seems to cast '' into nil

def test_validation
  foo = Foo.new
  foo[:bar] = ''

  puts foo[:bar].class
  # NilClass

  foo[:bar] = 'bar'
  puts foo[:bar]
  # 0
end

Before the numericality validition runs, allow_blank and allow_nil call the #bar method

  def bar
    self[:bar] || 123
  end

which in this case always returns 123 since self[:bar] is nil, thus the numericality check is executed and fails because self[:bar] is not a number.

What confused me is that allow_blank and allow_nil use the result of #bar to decide whether to skip the actual validation, and the numericality validation uses self[:bar].

Then I found that allow_blank and allow_nil use the result of method called #read_attribute_for_validation (source), and this one can be overridden if required to customize that behaviour, meaning you can do this:

class Foo < ActiveRecord::Base
  validates :bar, numericality: { greater_than: 100 }, allow_blank: true

  def bar
    self[:bar] || 123
  end

  def read_attribute_for_validation(key)
    self[key]
  end
end

But in my opinion this might have a downside, since this "rule" is then applied to all validations, which may have side effects again... so eventually you could use this validation instead:

validates :bar, numericality: { greater_than: 100 }, unless: -> { self[:bar].blank? }

I hope I did my research accurately... 😉

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-0-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.