#752 ✓not-applicable
Jeremy Nicoll

validates_with_method slightly borked

Reported by Jeremy Nicoll | January 7th, 2009 @ 04:52 PM | in 0.10.0

When using validates_with_method with the column name, the errors hash always has a key with an array with nil in it. This trips up the valid? method. A demonstration:

class SomeModel
  include DataMapper::Resource

  property :id,    Serial
  property :some_column, String

  validates_with_method :some_column, :some_method

  def some_method
    true # Should not trip up validation


sm = SomeModel.new
sm.valid? -> false
sm.errors.to_hash -> {:some_column=>[nil]}

As you can see, valid? should have returned true, but returned false since :some_column is a key with an array with a value of nil. Nil should not be present in this case, and neither should the key of :some_column in the error collection. This does not happen when validates_with_method :some_method is used.

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) January 8th, 2009 @ 02:44 PM

    • Assigned user set to “Michael Klishin (antares)”
  • Michael Klishin (antares)

    Michael Klishin (antares) March 25th, 2009 @ 04:59 PM

    • Milestone set to 0.10.0
    • State changed from “unconfirmed” to “confirmed”
  • Michael Klishin (antares)

    Michael Klishin (antares) March 28th, 2009 @ 09:55 AM

    • Tag set to dm-validations, validations
    • State changed from “confirmed” to “accepted”
  • Michael Klishin (antares)

    Michael Klishin (antares) March 31st, 2009 @ 01:40 PM

    I could reproduce it. On it now.

  • Michael Klishin (antares)

    Michael Klishin (antares) March 31st, 2009 @ 01:50 PM

    • State changed from “accepted” to “not-applicable”

    After some investigation I figured out that it is a doc error that leads to misunderstanding. First argument in the case provided by OP is not a property name, but yet another method that is being called in the instance. And that method also required to follow validating method rules and either return true, or a pair of [false, error message].

    Here is what it looks in the code of method validator:

          def initialize(field_name, options={})
            @options[:method] = @field_name unless @options.has_key?(:method)
          def call(target)
            result, message = target.send(@options[:method])
            add_error(target, message, field_name) unless result

    So, since some_property on a newly initialized instance is nil, and method :some_property returns it, message (that is also nil) is added to object errors. I am going to correct the docs now.

  • Michael Klishin (antares)

    Michael Klishin (antares) March 31st, 2009 @ 01:55 PM

    • State changed from “not-applicable” to “confirmed”

    One more thing. It may be a desired behavior for 1st argument to be considered property name. However, I found that method and block validators are often used for virtual attributes.

    I am not sure what the behavior should be in this case. You easily can mimic behavior where 1st argument is a property name with a block validator. On the other hand I realize that a lot of people would expect the same thing as OP.

  • Michael Klishin (antares)

    Michael Klishin (antares) March 31st, 2009 @ 02:27 PM

    • State changed from “confirmed” to “not-applicable”

    After discussion in developers channel on the IRC I have to make this hard but necessary decision: current behavior is here to stay with documentation corrections that explains my reasoning:

    antares_: namelessjon: see, all other validators take arbitrary number of field names and options hash
    antares_: here we would go against that, with 1st argument being special, and the rest being method names
    antares_: :method => :method_name that works right now as dbussink pointed out, does not struggle from this inconsistency
    antares_: validates_with_method :prop1, :prop2, :prop3, :method => :validator will validate 3 properties with one method
    antares_: probably makes little sense, but is 100% consistent with other validators
    antares_: it is also not clear how often people validate many properties with method validator
    antares_: method validator is less commonly used from my experience
    antares_: another reason is to not change validator behavior
    antares_: validator behavior changed may mean data inconsistency in user apps
    antares_: and it is just.. a too serious problem
    antares_: a few invalid objects slip through and you may have lost control over situation
    antares_: so I'll probably document current behavior better, and suggest to only use validates_method with one method at a time

    Let me know if you totally disagree :)

  • Jeremy Nicoll

    Jeremy Nicoll March 31st, 2009 @ 05:18 PM

    Makes sense, actually. I'm someone who makes fairly liberal use of custom validations. I'm someone who sometimes likes to add multiple error messages with one method. I'd be just fine manually adding errors to the error field and then returning false from the method. I'd prefer being able to define multiple methods with one call to validates_with_method as well.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »