This project is archived and is in readonly mode.

#3058 ✓committed
Jamie Hill

[PATCH] Sexy Validations

Reported by Jamie Hill | August 16th, 2009 @ 01:03 AM

Following on from my previous patch https://rails.lighthouseapp.com/projects/8994/tickets/2936, this patch adds to the validation hooks with a set-up similar to sexy migrations but for validations.

The registration of validators was requested in the comments of http://ryandaigle.com/articles/2009/8/11/what-s-new-in-edge-rails-i....

This patch allows the following:

  class EmailValidator < ActiveRecord::Validator
    def validate
      field = options[:attr]
      record.errors[field] << "is not valid"
        unless record.send(field) =~ /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i
    end
  end
  
  # Register the custom validator in an initializer file.
  ActiveModel::Validations.register_validator :email, EmailValidator

  class Person < ActiveModel::Base
    attr_accessor :name, :email

    validates :name, :presence => true, :uniqueness => true, :length => { :maximum => 100 }
    validates :email, :presence => true, :email => true
  end

As with the previous patch I believe this adds even more flexibility which is what Rails 3 is all about. It also allows validators to be shared more easily.

Rails defaults can now also be overridden e.g.

  class RequiredValidator < ActiveRecord::Validator
    def validate
      field = options[:attr]
      record.errors[field] << "Required" if record.send(field).blank?
    end
  end

  ActiveModel::Validations.register_validator :email, RequiredValidator

Thanks,

Jamie

Comments and changes to this ticket

  • Jamie Hill

    Jamie Hill August 16th, 2009 @ 01:05 AM

    Apologies, that last line should have read:

    ActiveModel::Validations.register_validator :presence, RequiredValidator
    
  • darkliquid

    darkliquid August 19th, 2009 @ 12:17 PM

    Your patch seems to be missing your 'validates' class method, so your example doesn't actually work.

    A rough idea of the top of my head is that you meant to include a method something like this in ActiveModel::Validations::ClassMethods:

    def validates(*args)
    validators = args.extract_options!

    args.each do |attribute|

    validators.each do |name, configuration|
      klass = ActiveModel::Validations.validator_for(name)
      send(validation_method(configuration[:on]), configuration) do |record|
        klass.new(record, configuration.merge(:field => attribute)).validate
      end
    end
    

    end end

    Without this method or any modifications to existing validation macros, registering classes with friendly names basically does nothing.

  • Jamie Hill

    Jamie Hill August 19th, 2009 @ 12:30 PM

    Ahh, good spot. It looks like I didn't add the 2 most important files which also include the documentation.

    Attached is the ACTUAL patch.

  • Jamie Hill

    Jamie Hill August 19th, 2009 @ 12:51 PM

    I've just noticed that my email example validator was wrong too, should have been.

    class EmailValidator < ActiveRecord::Validator
      def validate
        field = options[:attr]
        record.errors[field] << "is not valid" unless record.send(field) =~ /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i
      end
    end
    
    # Register the custom validator in an initializer file.
    ActiveModel::Validations.register_validator :email, EmailValidator
    

    Sorry guys, must have been having an off day.

  • Jamie Hill

    Jamie Hill August 19th, 2009 @ 12:55 PM

    Can I get thoughts on publicising the new read_attribute_for_validation method as then this could be used in custom validators meaning they would be super-flexible.

  • josh

    josh August 19th, 2009 @ 04:06 PM

    • State changed from “new” to “open”
    • Milestone cleared.

    "registration" hooks aren't necessary in Ruby. We have const_get.

  • darkliquid

    darkliquid August 19th, 2009 @ 04:16 PM

    Rather than using the registration hooks, I take you are proposing a something that takes the validation names and just constantizes them into validator classes, e.g :email becomes EmailValidator automagically?

  • Jamie Hill

    Jamie Hill August 19th, 2009 @ 04:20 PM

    Josh, are you suggesting I just look for a class in the global namespace?

    What are your thoughts on looking for any classes that are called "#{the_symbol.to_s.camelize}Validator" that inherit from ActiveModel::Validator?

    Also I'm thinking that ActiveRecord::Validator should be moved to ActiveModel::Validator so that this functionality is available outside of ActiveRecord.

  • Jamie Hill

    Jamie Hill August 19th, 2009 @ 10:56 PM

    Attached is an updated patch to use convention over configuration as suggested by Josh. The following is now possible:

    class EmailValidator < ActiveModel::Validator
      def validate
        field = options[:attr]
        record.errors[field] << "is not valid" unless
          record.read_attribute_for_validation(field) =~ /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i
      end
    end
    
    class Person < ActiveModel::Base
      attr_accessor :name, :email
    
      validates :name, :presence => true, :uniqueness => true, :length => { :maximum => 100 }
      validates :email, :presence => true, :email => true
    end
    
    # Validator classes my also exist within the class being validated
    # allowing custom modules of validators to be included as needed e.g.
    module MyValidators
      class TitleValidator < ActiveModel::Validator
        def validate
          field = options[:attr]
          record.errors[field] << "must start with 'the'" unless
            record.read_attribute_for_validation(field) =~ /^the/i
        end
      end
    end
    
    class Person < ActiveModel::Base
      include MyValidators
    
      validates :title => true
    end
    

    I have also made a change to rectify the bug in this ticket: https://rails.lighthouseapp.com/projects/8994/tickets/3077-activere... as logged by darkliquid.

    One other modification I have made is to make read_attribute_for_validation public so that it may be used in custom validators.

    Ideally I would change validates_with and the Validator class so that it is not necessary to fetch the value for the attribute and instead pass the value in as an argument to the validate method in the Validator class but I didn't want to change the validates_with API. As validates_with is new to edge, what is the policy on changing it's API?

  • Jamie Hill

    Jamie Hill August 19th, 2009 @ 11:00 PM

    Just added a change to the documentation:

    class Person < ActiveModel::Base
      include MyValidators
     
      validates :name, :title => true
    end
    
  • Elliot Winkler

    Elliot Winkler August 21st, 2009 @ 03:06 PM

    I know the idea's pretty much accepted already but I just want to say, big +1 from me. The ability to combine validations into one line is worth it alone, and automatically picking up on Validators makes Validators even more awesome than they were. Now if #750 and #401 could be resolved that would solve all the wishes I've had for validations.

  • darkliquid

    darkliquid August 21st, 2009 @ 03:43 PM

    +1 from me as well.

    There is one possible minor issue which I doubt would actually cause any problems. Since a hash is being used to specify validations, those validations will be defined in a random order in ruby 1.8. It's not an issue at all in ruby 1.9 as hashes are ordered I believe. You could get around the ordering issue in 1.8 (from a users perspective) by just calling validates with one key-value pair at a time if ordering is important to you.

    Elliot, regarding #750, some way to do full reflection on validations like you can on associations would be even more awesome as you could show all requirements in the view or do stuff like generate client-side validation based on the reflected server-side ones for example.

  • Jamie Hill

    Jamie Hill August 24th, 2009 @ 05:14 PM

    I was wondering if anyone has any more feedback on this as I'd like to get it accepted as soon as poss? Cheers.

  • josh

    josh August 31st, 2009 @ 07:48 PM

    • Assigned user cleared.

    A bunch guys started discussing completely rewriting the internal validation api to be more like validatable. I'd bring up in a Rails core post. Yehuda, Bryan H, and José V are the guys interested.

    (As for me, I think the current api is fine, so I'm removing myself)

  • Jamie Hill

    Jamie Hill September 2nd, 2009 @ 12:56 PM

    Josh,

    This implementation currently works without any changes to the current API, I was stating however that ideally the API for validator classes would be changed slightly to make things easier. Could this patch be applied as-is in addition to the discussions for a new API.

    Thanks,

    Jamie

  • José Valim

    José Valim September 4th, 2009 @ 01:27 PM

    Sorry for the delay to reply. @Jamie, I want to do a quite simple change: instead of giving a proc to validate, we could give the own class to validate:

      def validates_with(*args)
        configuration = args.extract_options!
    
        send(validation_method(configuration[:on]), configuration) do |record|
          args.each do |klass|
            klass.new(record, configuration.except(:on, :if, :unless)).validate
          end
        end
      end
    

    Could be changed to:

      def validates_with(*args)
        configuration = args.extract_options!
    
        args.each do |klass|
          send(validation_method(configuration[:on], klass.new(configuration), configuration)
        end
      end
    

    Why? If we do this, we can access all the classes that were given to validates_with. So if we eventually change all Rails validations to use validates_with (creating something like PresenceValidator), we would be able to retrieve those classes and have validations reflection (to generate dynamically javascript, for example).

    What would need to be changed? We just need to change Validator to receive the record as argument in the validate method, which in my opinion is also a better design (the validator does not need to be attached to any object when created).

    I already had a patch for this and it should get merged soon. After that we can continue working on sexy validations ;).

    And one other thing is the DSL. Which one do you prefer?

      # Hashes
      validates :name, :presence => true, :uniqueness => true, :length => { :maximum => 100 }
    
      # Blocks
      validations_for :name do |n|
        n.presence
        n.uniqueness
        n.length :maximum => 100
      end
    

    Both are great for me, just want to be sure we discuss it. :)

  • Jamie Hill

    Jamie Hill September 4th, 2009 @ 03:51 PM

    Hi José,

    Thanks for the reply.

    I am 100% agreed on this. With regards the hash vs. block I personally prefer the hash syntax as it is more concise and requires less DSL magic.

    Passing in the record to the validator definitely works. I have included a link below to iq-validator that we use at SonicIQ for validating non-activerecord models. Our validator classes have a valid? method that accept the value to validate. I do prefer your solution of giving the record to validators as validators such as a ConfirmValidator then have access to other fields.

    Another example of using the standalone validation implementation is in our iq-form gem which takes a different approach to forms so that you can create them in an object orientated manor similar to Django. This could act as a good testbed for the flexibility of Rails validation.

    Please note. I ideally want to remove the need for iq-validation, hence my patches to expand the existing functionality of validations in Rails. Also note that these gems are not at a release state, I have just put them up for reference.

    http://github.com/iq/validation/tree/master
    http://github.com/iq/form/tree/master

    Thanks again,

    Jamie

  • José Valim

    José Valim September 4th, 2009 @ 04:00 PM

    @Jamie,

    Exactly. As I discussed with Jeremy Kemper, besides the Validator, we should implement at least an EachValidator that exposes a less raw API. We could do this:

    class PresenceValidator < EachValidator
      def for_each(record, attribute, value)
        # logic
      end
    end
    

    So we wouldn't have any problem changing all rails validations to use classes. As soon as the patch gets applied, I will ping you again. :)

  • Jeroen van Dijk

    Jeroen van Dijk September 7th, 2009 @ 04:17 PM

    +1 for this new functionality.

    I'm not sure if the following is directly related, but it might be. When you want do some custom validation which would not be generic enough for a validation class, it would be nice if the validation syntax could be shorter. See below:

    
    validates :age, :message => :something_very_small_but_custom do |record|
       record.age < 18
    end
    

    This would be equivalent to what you would could do now:

       validate_each :age do |record, attr, value|
          record.errors(attr, :something_very_small_but_custom) if age < 18
       end
    

    As stated before, not sure if this is related, but I can image that the validate method will accept a hash with options and optionally a block with the custom validation.

  • Jeroen van Dijk

    Jeroen van Dijk September 7th, 2009 @ 04:22 PM

    Sorry I made some typos.

    The second code example should be:

    
       validates_each :age do |record, attr, value|
          record.errors(attr, :something_very_small_but_custom) if attr < 18
       end
    
  • josh

    josh October 7th, 2009 @ 05:23 AM

    • Assigned user set to “José Valim”
  • josh

    josh October 9th, 2009 @ 06:18 PM

    • Milestone set to 2.x
  • josh

    josh October 9th, 2009 @ 06:18 PM

    • Milestone cleared.
  • Jamie Hill

    Jamie Hill October 10th, 2009 @ 08:23 AM

    @José Valim, has the patch you mentioned, been applied yet, if so I will update the patch. Jeroens suggestion for a short block syntax can also be included.

  • José Valim

    José Valim October 10th, 2009 @ 12:51 PM

    @Jamie, no, no yet. We've been working one other features that have higher priority. I'm not sure that the patch will be able to make it to Rails 3.

  • Jamie Hill

    Jamie Hill October 11th, 2009 @ 01:56 PM

    @José, in that case could the patch be applied as-is as it works with the existing functionality?

  • Jamie Hill

    Jamie Hill November 15th, 2009 @ 11:26 AM

    Sorry, to bug. Can this be applied?

  • José Valim

    José Valim January 2nd, 2010 @ 11:19 PM

    Ok, all validations are now Validators in core. There is just one concern left: I don't like the fact that we have two entry points: one as @_registered_validators and other as validates_presence_of. I think the lookup should be:

    1) validates#{method}to
    2) validates_#{method}
    3) raise no validation available

    I would also ask you to run this through the rails core list to see people and mainly rails core opinion on it.

    And sorry for the time taken to reply.

  • Jamie Hill

    Jamie Hill January 3rd, 2010 @ 04:01 AM

    Great news, thanks for your help on this.

    I am attaching a new patch that address your concerns by working by convention as suggested by Josh. I have also publicised the read_attribute_for_validation method as it may be needed when writing custom validators.

    Can I suggest that we merge ActiveModel::Validator and ActiveModel::EachValidator as there is no real use for what is currently ActiveModel::Validator (please correct me if I'm wrong). This way usage would be as follows:

    class EmailValidator < ActiveModel::Validator
      def validate_each(record, attribute, value)
        record.errors[attribute] << "is not an email" unless value =~ /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i
      end
    end
    
    class MyClass
      include ActiveModel::Validations
      attr_accessor :from
      validates :from, :email => true
    end
    

    I am happy to update the patch to reflect this if you are in agreement.

  • José Valim

    José Valim January 3rd, 2010 @ 08:43 AM

    Validator it's an abstract and will be handy if someone needs, so I prefer if we keep it.

    About your patch, I would prefer if the common entry point was the validates#{method}of instead of the validator, if you check validates_associated_of or validates_confirmation_of you will know why. Unless we move that logic inside validators as well.

    +1 on publicizing read_attributes_for_validation.

  • Jamie Hill

    Jamie Hill January 3rd, 2010 @ 04:06 PM

    Agreed on the Validator abstract class, I couldn't initially see a use for it but may be useful in some circumstances.

    As for the entry point being via the validates#{method}of, I don't quite follow. Wouldn't this mean you would need to write custom class methods for validators e.g. validates_email_of etc? I much prefer being able to just create a validator class and use it via:

    validates :attribute_name, :my_validator => { :my => 'options' }
    

    I don't really see the problem with validates_confirmation_of, this will just become:

    validates :password, :confirmation => true
    

    Am I missing something obvious?

  • José Valim

    José Valim January 3rd, 2010 @ 04:10 PM

    If you check validates_confirmation_of, you will see that it needs to be in the class scope to set things like attr_accessor. The same for validates_acceptance_of. Unless we send the class to the validator, which would work, but I don't like because the validator should not be attached to a class.

  • Jamie Hill

    Jamie Hill January 3rd, 2010 @ 05:54 PM

    I see what you mean about the attr_accessor. I think ideally that validations shouldn't magically add methods to the model and instead you should be expected to add this where required, however this would obviously break backwards compatibility. I believe the validate#{method}of methods should simply be proxies to the validators, this could go as far as using the new validates method e.g.

      %w(presence length numericality etc).each do |validation|
        define_method "validates_#{validation}_of", *attr_names do
          options = attr_names.extract_options!
          validates *attr_names, :length => options
        end
      end
    
      # or
    
      %w(presence length numericality etc).each do |validation|
        define_method "validates_#{validation}_of", *attr_names do
          options = attr_names.extract_options!
          validates_with "#{validation.camelize}Validator".constantize, options.merge(:attributes => attr_names)
        end
      end
    

    I suppose what I am saying is that I envisaged the validators being completely self contained and the validates??of just being convenience methods for validates/validates_with. This poses a problem like you say with confirmation so I will have a think on this and see if I can come up with a cleaner solution.

    We could always go the (dare I say) Python route an pass in self as the first argument to the Validator initializer :/ but I don't really like this.

  • Jamie Hill

    Jamie Hill January 3rd, 2010 @ 05:56 PM

    Sorry, that first code snippet was wrong, should be:

     %w(presence length numericality etc).each do |validation|
        define_method "validates_#{validation}_of", *attr_names do
          options = attr_names.extract_options!
          validates *attr_names, validation.to_sym => options
        end
      end
    
  • Jamie Hill

    Jamie Hill January 5th, 2010 @ 09:49 AM

    I am attaching a new diff of what I believe to be an elegant solution to the whole attr_accessor problem.

    I have moved the remaining logic into Validator classes and have managed to get around the need to pass in the class to the validators as we were both not happy with that. Basically, there is now a virtual_attributes instance method on validators which simply defines what virtual attributes should exist for the validator to work. With this information, validates_with now adds those accessors as long as they are not columns (check needed for acceptance_of).

    By doing things this way we are now able to delegate all validates?of calls to validates meaning that if it works with validates?of, it works with validates. All validates?of methods are just one-line calls to a private method that extracts the options and calls validates with the right arguments. This could be done with define_method as per previous comment but not sure how to get rdoc to behave with that.

    I have gone through and fixed some of the documentation as it was still referencing ActiveRecord and fleshed out the Validator documentation a little.

    I think this is a nice clean solution but please let me know if there's anything you're not happy with.

  • Jamie Hill

    Jamie Hill January 5th, 2010 @ 10:50 AM

    Just noticed an error when running on 1.8.7, attaching new patch file.

  • José Valim

    José Valim January 5th, 2010 @ 10:58 PM

    Moving this logic to validates_with is wrong, mainly because validates_with is in ActiveModel, which knows nothing database columns. So we are not there yet. Besides that, tests and docs looks good.

  • Jamie Hill

    Jamie Hill January 5th, 2010 @ 11:57 PM

    All I have done is move that logic from the validates_acceptance_of method which it should really be in either. As mentionned above I don't thing validations should be adding methods to the model on the fly anyway really. I can remove the column check and instead check to see if the model responds to the setter method but I didn't know if whoever had written that column code had done it for a reason.

  • Jamie Hill

    Jamie Hill January 5th, 2010 @ 11:59 PM

    Leave it with me and I'll try a few options.

  • Jamie Hill

    Jamie Hill January 6th, 2010 @ 08:00 PM

    Right, think we're there. Attached is new patch which instead of referencing columns, validates_with looks to see if a setter instance method is already available and skips if so.

  • Jamie Hill

    Jamie Hill January 7th, 2010 @ 01:06 AM

    New diff attached which uses setup hook instead of virtual_attributes as well as adding _merge_attributes private method. Also added further documentation for setup hook.

  • Jamie Hill

    Jamie Hill January 7th, 2010 @ 01:13 AM

    Added some comments, new patch attached.

  • Repository

    Repository January 7th, 2010 @ 06:25 PM

    (from [0a79eb7889e7ac711ff171a453d65f3df57b9237]) Add validates method as shortcut to setup validators for a given set of attributes:

    class Person < ActiveRecord::Base
    include MyValidators

    validates :name, :presence => true, :uniqueness => true, :length => { :maximum => 100 } validates :email, :presence => true, :email => true end

    [#3058 status:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/0a79eb7889e7ac711ff171a453d65f...

  • José Valim

    José Valim January 7th, 2010 @ 06:26 PM

    • State changed from “open” to “committed”
  • Jamie Hill

    Jamie Hill January 17th, 2010 @ 04:25 AM

    José,

    I've just noticed a problem in the additional code you added, I am getting:

    NoMethodError: undefined method `slice!' for {:presence=>true}:Hash
            from /Users/jamie/.gem/ruby/1.8/gems/activemodel-3.0.pre/lib/active_model/validations/validates.rb
    
  • José Valim

    José Valim January 17th, 2010 @ 09:09 AM

    That was fixed as well, thanks!

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:52 PM

    • Tag cleared.

    Automatic cleanup of spam.

Create your profile

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

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages