Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #10307 (closed defect: duplicate)

Opened 2 years ago

Last modified 2 years ago

Invalid date parameters sent from the form date helper cause ActiveRecord::MultiparameterAssignmentErrors

Reported by: leikind Assigned to: core
Priority: normal Milestone: 2.x
Component: ActiveRecord Version: edge
Severity: normal Keywords: MultiparameterAssignmentErrors, Date
Cc:

Description

If the user has chosen an invalid date in the standard Rails date helper (i.e. 31 of February), creating a new ActiveRecord instance will lead to ActiveRecord::MultiparameterAssignmentErrors

Example: Suppose CalendarEvent model has a date field called start_date.

Running this line:

 CalendarEvent.new({"start_date(1i)"=>"2007", "start_date(2i)"=>"11", "start_date(3i)"=>"31"})

will lead to this:

ActiveRecord::MultiparameterAssignmentErrors: 1 error(s) on assignment of multiparameter attributes
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.15.5/lib/active_record/base.rb:2097:in `execute_callstack_for_multiparameter_attributes'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.15.5/lib/active_record/base.rb:2077:in `assign_multiparameter_attributes'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.15.5/lib/active_record/base.rb:1678:in `attributes='
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.15.5/lib/active_record/base.rb:1508:in `initialize_without_callbacks'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.15.5/lib/active_record/callbacks.rb:225:in `initialize'

This stacktrace belongs to Rails 2.0 RC2 ( http://dev.rubyonrails.org/svn/rails/tags/rel_2-0-0_RC2/ ) but has existed for a while in older versions.

Creating a new ActiveRecord instance should not raise exceptions if input data is invalid, this should happen when validating the object!

The solution has also been around for a while in the form of a plugin - http://wiki.rubyonrails.org/rails/pages/Validates+Multiparameter+Assignments , which is actually a short fix:

module ActiveRecord
  module Validations
    module ClassMethods
      def validates_multiparameter_assignments(options = {})
        configuration = { :message => ActiveRecord::Errors.default_error_messages[:invalid] }.update(options)
        
        alias_method :assign_multiparameter_attributes_without_rescuing, :assign_multiparameter_attributes
        attr_accessor :assignment_error_attrs
        
        define_method(:assign_multiparameter_attributes) do |pairs|
          self.assignment_error_attrs = []
          begin
            assign_multiparameter_attributes_without_rescuing(pairs)
          rescue ActiveRecord::MultiparameterAssignmentErrors
            $!.errors.each do |error|
              self.assignment_error_attrs << error.attribute
            end
          end
        end
        private :assign_multiparameter_attributes
        
        validate do |record|
          record.assignment_error_attrs && record.assignment_error_attrs.each do |attr|
            record.errors.add(attr, configuration[:message])
          end
        end
      end
    end
  end
end

Change History

(follow-up: ↓ 3 ) 11/29/07 19:45:18 changed by danielmorrison

I agree that this should be changed. Shall we work this up into a patch?

11/29/07 19:47:50 changed by danielmorrison

An old patch exists in #7754, but it is outdated.

(in reply to: ↑ 1 ) 11/29/07 21:28:17 changed by leikind

Replying to danielmorrison:

I agree that this should be changed. Shall we work this up into a patch?

Oh yes.

But on a second thought I considered this approach and here is what I think: with this approach the exception is intercepted when creating an object, and errors are stored in some private instance variable until validation, when they get copied into the Errors object. This has 2 consequences:

1) After object initialization the date field will be nil for no evident reason. This will not fit nicely with the Rails error handling approach when the user sees invalid data he had entered before he pressed the Submit button. This invalid date will be lost

2) Date validation will be happening even without any validate_* method in the declaration of an ActiveRecord class. A bit unusual.

But I think I found a nicer way to handle it.

If you submit the 31 of November to the Time object, unlike the Date class it won't throw any errors and will equal the 1st of December.

So why don't we

1) Intercept ArgumentError if the class is Date (and only Date)

2) If ArgumentError is raised, create a Time object using the same parameters

3) Convert it to Date

This is quite a different approach, no validation errors are created, but all that it does is making it consistent the behavior already present in Rails, that is, with those cases when we have a Time field and invalid parameters are sent from the form to create an model instance with such a Time field, so why don't we follow the same pattern with dates?

I have the working code now, if you agree to the approach, I'll clean it up and learn what else I should do to submit a kosher Rails patch.

11/29/07 21:54:17 changed by danielmorrison

Yes, I like that approach much better, especially since it already behaves that way when its a Time.

I look forward to seeing the patch. Let me know if I can help.

12/03/07 22:46:53 changed by leikind

Hey,

Just a question about the testing infrastructure of ActiveRecord. Directory test/fixtures/db_definitions/ contains sql code to populate various databases, but for MySQL there are migrations schema.rb and schema2.rb, where it's written that

we are under a transition of moving the sql files under activerecord/test/fixtures/db_definitions to this file, schema.rb.

There are rake tasks to create empty databases, but I couldn't find a quick way to initialize a MySQL database, so I had to run a script like this

require 'rubygems'
require '../lib/active_record.rb'
require 'connections/native_mysql/connection.rb'
require 'fixtures/db_definitions/schema.rb'
require 'fixtures/db_definitions/schema2.rb'

in directory activerecord/test

But there must be a better way, I don't believe people do it manually like this. What am I missing?

(follow-up: ↓ 7 ) 12/18/07 23:07:29 changed by leikind

I finally submitted a path http://dev.rubyonrails.org/ticket/10556

(in reply to: ↑ 6 ) 12/19/07 02:16:57 changed by chuyeow

Replying to leikind:

I finally submitted a path http://dev.rubyonrails.org/ticket/10556

Awesome :)

If you consider that your new ticket has sufficiently covered the issue in this ticket, I suggest closing this one (or attaching your patch here and renaming the ticket title with '[PATCH]' in front).

12/19/07 07:01:46 changed by leikind

  • status changed from new to closed.
  • resolution set to duplicate.