Rails - Mar 2005 - [SOLVED] time validation (the feb. 31st problem)

the essence of this solution is as follows: Time::mktime, Time::local
and friends all have the possibility of ''normalizing'' their
arguments.
that is to say a time created with

  Time::local 2000, 2, 31

can be ''normalized'' into

  Wed Mar 03 00:00:00 MST 1999

this is consistent with the underlying system calls.  however, it
clearly was constructed from ''bad'' input since the input month
is not
the same as the output one.  this is the logic behind the following
example of time validation using rails and should be complete for
Time::mktime, Time::local, Time::gm, and Time::utc



class DbTableWithTimestampField < ActiveRecord::Base
  protected
    def validate(*a, &b)
      t = timestamp_fieldname
      if t.normalized?
        values = t.original_arguments
        errors.add self.class.to_s, "bad time from
<#{values.inspect}>"
      end
      super
    end
end

class ::Time
  class << self
    def normalized? t, values
      tvalues = %w(year month day hour min sec usec).map{|m| t.send m}
      values.zip(tvalues){|vin,vout| return true if vin != vout}
      return false
    end

    %w( local mktime gm utc ).each do |method|
      eval <<-code
        alias __org_#{ method }__ #{ method }
        def #{ method }(*a, &b)
          t = __org_#{ method }__(*a, &b)
          t.instance_eval{@original_arguments = a}
          t.instance_eval{@normalized = self.class.normalized? self, a}
          t
        end
      code
    end
  end

  attr_reader ''normalized''
  alias_method ''normalized?'', ''normalized''
  attr_reader ''original_arguments''
end

this results in an error page which looks something like:

  1 error prohibited this timestamp_fieldname from being saved

  There were problems with the following fields:

    * DbTableWithTimestampField bad time from <[2005, 2, 31, 12, 36]>

kind regards.

ps. my email is fried and i''m posting from messenger so this email may
or may not be formatted as i''d like it be.

-a

Ara T Howard wrote:
>the essence of this solution is as follows: Time::mktime, Time::local
>and friends all have the possibility of ''normalizing''
their arguments.
>that is to say a time created with
>
>  Time::local 2000, 2, 31
>
>can be ''normalized'' into
>
>  Wed Mar 03 00:00:00 MST 1999
>
>this is consistent with the underlying system calls.  however, it
>clearly was constructed from ''bad'' input since the input
month is not
>the same as the output one. [code snipped]
>  
>
As a fix to the immediate problem this looks great.

(A minor point - I''d personally prefer to bring the day back into
bounds
for the user''s choice of month rather than changing two values.)

Thinking more generally (and longer-term), is it necessary to 
''normalize'' the date? For other types, as I understand it, if
Rails
can''t achieve a conversion it just substitutes an innocuous value (e.g.
0 for a field that was supposed to be an integer, but wouldn''t convert 
to one) but leaves the raw form accessible via an accessor ending with 
"_before_type_cast". Isn''t this another situation where the
aim should
be to have a "before conversion" value that can be validated?

(Note: Date validation can be done with Date::valid_civil?)

On validation failure, the user should see whatever he/she entered, 
rather than an adjusted value. (This happens at present, e.g. if you 
enter alpha characters into a field that is supposed to be a number.)

Time and Date entry from multiple fields or drop-downs is a specific use 
of a more general "multi-parameter assignment" mechanism in Rails. The
code is in active_record/base.rb in the methods:

    attributes    assign_multiparameter_attributes
    execute_callstack_for_multiparameter_attributes
    extract_callstack_for_multiparameter_attributes

The comment for assign_multiparameter_attributes is helpful:

      # Instantiates objects for all attribute classes that needs more 
than one constructor parameter. This is done
      # by calling new on the column type or aggregation type (through 
composed_of) object with these parameters.
      # So having the pairs written_on(1) = "2004", written_on(2) =
"6",
written_on(3) = "24", will instantiate
      # written_on (a date type) with Date.new("2004", "6",
"24"). You
can also specify a typecast character in the
      # parentheses to have the parameters typecasted before they''re 
used in the constructor. Use i for Fixnum, f for Float,
      # s for String, and a for Array. If all the values for a given 
attribute is empty, the attribute will be set to nil.
      def assign_multiparameter_attributes(pairs)
        execute_callstack_for_multiparameter_attributes(
          extract_callstack_for_multiparameter_attributes(pairs)
        )
      end

It would be good to find a fix for all uses of this mechanism.

I haven''t dug into this to find  what the current  "before
conversion"
representation is, or if there is one - but while I was out this 
evening, away from the code, I was thinking about it, and came to the 
conclusion that either of the following would be appropriate:

i) hash from the part of the original key that was in parentheses to the 
original value, e.g.
  { "1i" => "2005", "2i" => "02",
"3i" => "29" }, or

ii) parallel arrays of type and value, e.g.
  [ ["i", "i", "i"], ["2005",
"02", "29"] ]
  (I guess the types would be nil if not specified)

I prefer the latter.

N.B. "Before conversion" means before both the simple value conversion
(string to integer) and the construction of the Date from the simple 
values. Either of these could fail.

That''s all I have to offer at the moment - I''m a newcomer to
Ruby and
Rails, and I''m sure if there''s any value in these thoughts
that someone
else would be able to translate them into a code change far more quickly 
and effectively than I  could.

    Justin

----- Original Message -----
From: Justin Forder
<justin-zSfPWr5aQuznITO/+xaoB7VCufUGDwFn@public.gmane.org>
Date: Sunday, March 13, 2005 7:26 pm
Subject: Re: [Rails] [SOLVED] time validation (the feb. 31st problem)
> (A minor point - I''d personally prefer to bring the day back into
bounds for
> the user''s choice of month rather than changing two values.)
>
> 
> Thinking more generally (and longer-term), is it necessary to
''normalize'' the
> date? For other types, as I understand it, if Rails can''t achieve
a
conversion
> it just substitutes an innocuous value (e.g.  0 for a field that was
supposed
> to be an integer, but wouldn''t convert to one) but leaves the raw
form
> accessible via an accessor ending with "_before_type_cast".
Isn''t this
another
> situation where the aim should be to have a "before conversion"
value
that can
> be validated?
er, nothing is done here?  it''s the system that normalizes the time in
the first
place... - man mktime.

  for example

    if your form passes in year=1999, month=02, day=31

  ruby/c will, if given the chance, ''normalize'' this.  eg:

    irb(main):001:0> Time::local 1999, 2, 31
    => Wed Mar 03 00:00:00 MST 1999

NOTHING in my code normalizes ANYTHING.  it does two things only:

  * stores the orignal arguments passed to the contructor (mktime, utc,
gm, or
    local) as an instance variable of the time object

  * notes, in the instace flag ''normalized?'' whether or not
the ANY of
the input
    values were normalized in the call to the system mktime.  It''s not
just days
    that can wrap:

      irb(main):004:0> Time::local 1999, 1, 1, 23, 59, 60
      => Sat Jan 02 00:00:00 MST 1999

    the check flags the time object as having been
''normalized?'' by the
system
    if ANY input value has wrapped but does nothing about it.  indeed, to
    through an execption here would severly hinder validation since it would
    occur deep in controller code and throw a
MultiParameterValieException which
    would all would need to happen BEFORE we ever got to validation and,
thus,
    would defeat the whole purpose.

    so we are left with a situation - we have to contruct a fubar time -
but we
    also want to make later validation possible.  thus, store input args
(for
    error messages later) and flag the time as normalized or not.  rails
    validation can use this later - manually or generically.
> (Note: Date validation can be done with Date::valid_civil?)
really? - i think it will blow up early on:

  [ahoward@localhost ~]$ irb
  irb(main):001:0> require ''date''
  => true
  irb(main):002:0> Date::new 2002, 2, 31
  ArgumentError: invalid date
          from /usr/local/lib/ruby/1.8/date.rb:591:in `new''
          from (irb):2

so this would happen creating the multi parm call stack...  before
validation.
or maybe i''m missing something here?
> On validation failure, the user should see whatever he/she entered, rather
> than an adjusted value. (This happens at present, e.g. if you enter alpha
> characters into a field that is supposed to be a number.)
that''s __precisely__ what my example does, including showing the user
exactly
what they entered?  did you try it?
> Time and Date entry from multiple fields or drop-downs is a specific
use of a
> more general "multi-parameter assignment" mechanism in Rails. 
The
code is in
> active_record/base.rb in the methods:
> 
>    attributes>    assign_multiparameter_attributes
>    execute_callstack_for_multiparameter_attributes
>    extract_callstack_for_multiparameter_attributes
> 
> The comment for assign_multiparameter_attributes is helpful:
> 
>      # Instantiates objects for all attribute classes that needs more
than one
>      constructor parameter. This is done
>      # by calling new on the column type or aggregation type (through
>      composed_of) object with these parameters.
>      # So having the pairs written_on(1) = "2004", written_on(2)
= "6",
>      written_on(3) = "24", will instantiate
>      # written_on (a date type) with Date.new("2004",
"6", "24").  You can
>      also specify a typecast character in the
>      # parentheses to have the parameters typecasted before
they''re
used in
>      the constructor. Use i for Fixnum, f for Float,
>      # s for String, and a for Array. If all the values for a given
attribute
>      is empty, the attribute will be set to nil.  def
>      assign_multiparameter_attributes(pairs)
>      execute_callstack_for_multiparameter_attributes(
>      extract_callstack_for_multiparameter_attributes(pairs)) end
> 
> It would be good to find a fix for all uses of this mechanism.
> 
> I haven''t dug into this to find  what the current  "before
conversion"
> representation is, or if there is one - but while I was out this
evening, away
> from the code, I was thinking about it, and came to the conclusion
that either
> of the following would be appropriate:
> 
> i) hash from the part of the original key that was in parentheses to the
> original value, e.g.  { "1i" => "2005",
"2i" => "02", "3i" => "29" }, or
> 
> ii) parallel arrays of type and value, e.g.  [ ["i",
"i", "i"],
["2005", "02",
> "29"] ] (I guess the types would be nil if not specified)
> 
> I prefer the latter.
write a validation of those values handling leap years and leap months
in less
that 100 lines of code and i''ll buy you a beer!  seriously,
it''s really,
really
hard.  the code in date.rb to do this is a hundred lines of code or more and
it''s not even handling times...  remember, sometimes 02/30 is a good
day :
there are leap years and leap months and different calenders depending on
locale.  it would be truely cruel to dump the numbers (in whatever data
structure) into the user''s lap and say : "validate this." 
just check
out the
code for date.rb to see what i mean.
> N.B. "Before conversion" means before both the simple value
conversion
(string
> to integer) and the construction of the Date from the simple values.
Either of
> these could fail.
> 
> That''s all I have to offer at the moment - I''m a newcomer
to Ruby and
Rails,
> and I''m sure if there''s any value in these thoughts that
someone else
would be
> able to translate them into a code change far more quickly and effectively
> than I  could.
i totally agree with you that all parameters, including those unpacked as
multiparamters, need to be accessible to the user directly and untouched
- but,
in the case of time validation that''s not even close to enough on rails
part.
it need to offer a method of validation.  i think that my huristic - if what
went it did not come out - will be correct in 99.9% of situations and,
for those
where it is not, the user should, as you suggest, have access to the
original
input values.

kind regards.

-a

Ara, thanks for your prompt response. I''ll keep this brief, because
it''s
5am here.

I have come to this problem looking at date validation, rather than 
date/time. I''m just working through "Four Days on Rails, and the
date
entry example there uses date_select.

On entry, this ends up calling Date.new. As you point out Date.new 
throws an exception if you give it bad values - it does not do any 
''normalization''. It does this precisely as a result of
validating using
the method I mentioned:

----------------------------------------------------- Date::valid_civil?
     Date::valid_civil?(y, m, d, sg=ITALY)
------------------------------------------------------------------------
     Do year +y+, month +m+, and day-of-month +d+ make a valid Civil
     Date? Returns the corresponding Julian Day Number if they do, nil
     if they don''t.

     +m+ and +d+ can be negative, in which case they count backwards
     from the end of the year and the end of the month respectively. No
     wraparound is performed, however, and invalid values cause an
     ArgumentError to be raised. A date falling in the period skipped in
     the Day of Calendar Reform adjustment is not valid.

     +sg+ specifies the Day of Calendar Reform.

irb(main):010:0> Date::valid_civil?(2005, 3, 14)
=> 2453444
irb(main):011:0> Date::valid_civil?(2005, 2, 29)
=> nil

That will be a pint of Guinness, please :-)

kind regards also,

     Justin

On Mon, 14 Mar 2005, Justin Forder wrote:
> Ara, thanks for your prompt response. I''ll keep this brief,
because it''s 5am
> here.
man - you should sleep more!
> I have come to this problem looking at date validation, rather than
> date/time. I''m just working through "Four Days on Rails, and
the date entry
> example there uses date_select.
me too actually...
> On entry, this ends up calling Date.new. As you point out Date.new throws
an
> exception if you give it bad values - it does not do any
''normalization''. It
> does this precisely as a result of validating using the method I mentioned:
>
> ----------------------------------------------------- Date::valid_civil?
>    Date::valid_civil?(y, m, d, sg=ITALY)
> ------------------------------------------------------------------------
>    Do year +y+, month +m+, and day-of-month +d+ make a valid Civil
>    Date? Returns the corresponding Julian Day Number if they do, nil
>    if they don''t.
>
>    +m+ and +d+ can be negative, in which case they count backwards
>    from the end of the year and the end of the month respectively. No
>    wraparound is performed, however, and invalid values cause an
>    ArgumentError to be raised. A date falling in the period skipped in
>    the Day of Calendar Reform adjustment is not valid.
>
>    +sg+ specifies the Day of Calendar Reform.
>
> irb(main):010:0> Date::valid_civil?(2005, 3, 14)
> => 2453444
> irb(main):011:0> Date::valid_civil?(2005, 2, 29)
> => nil
>
> That will be a pint of Guinness, please :-)
dang.  i owe you.

i''m still not clear one how you think this could be used though.  i see
that
an exception will be thrown - but this will occur deep in bowels of
rails''
controller code long before validation can occur.

the fundemental issue, of course, is that rails mails form parms to objects a
bit too early that makes sense.  or at least a bit too early to discard the
input paramter list.  again, for normal validation techniques to work the
contructor must NOT throw an exception yet provide a method to later see that
the input was invalid.  on the surface this does not make sense : surely you
want to explode early if input parameters to a multiparm object ctor are
fubar?  well, yes - you do.  the thing is feb 31st is NOT fubar.  it is
doccumented as being correct input and thus should NOT raise an exception.
many rails developers, might not agree.  to them it IS exceptional because
they did not want that selection to be possible and have not coded some
horrible javascript to prevent it - thus validation.  the key to validation is
that we have to have an object to validate.  so i re-submit my original
proposal which does all those things:

   * does not throw an exception

   * remembers original input paramters

   * makes it possible to determine if the inputs where not used the way the
     web developer intended (even if the actual use was
''legal'')

in thinking about it further i realize that few objects fall in this category:
where you can have the ctor succeed and also have that not be what you wanted
to happen.  in this sense Time is unique and therefore makes sense to be
handled specially.  again, i DO agree that a gereral method of obtaining input
parms to multi parms would be very useful.

cheers.

-a
-- 
==============================================================================|
EMAIL   :: Ara [dot] T [dot] Howard [at] noaa [dot] gov
| PHONE   :: 303.497.6469
| When you do something, you should burn yourself completely, like a good
| bonfire, leaving no trace of yourself.  --Shunryu Suzuki
===============================================================================

Ara.T.Howard wrote:
> [discussion of Date::valid_civil? snipped]
> i''m still not clear one how you think this could be used though. i
see
> that
> an exception will be thrown - but this will occur deep in bowels of 
> rails''
> controller code long before validation can occur.
>
> the fundemental issue, of course, is that rails mails form parms to 
> objects a
> bit too early that makes sense. or at least a bit too early to discard 
> the
> input paramter list. again, for normal validation techniques to work the
> contructor must NOT throw an exception yet provide a method to later 
> see that
> the input was invalid. on the surface this does not make sense : 
> surely you
> want to explode early if input parameters to a multiparm object ctor are
> fubar? well, yes - you do. the thing is feb 31st is NOT fubar. it is
> doccumented as being correct input and thus should NOT raise an 
> exception.
It isn''t correct input to a Date constructor.
> many rails developers, might not agree. to them it IS exceptional because
> they did not want that selection to be possible and have not coded some
> horrible javascript to prevent it - thus validation. the key to 
> validation is
> that we have to have an object to validate. so i re-submit my original
> proposal which does all those things:
>
> * does not throw an exception
>
> * remembers original input paramters
>
> * makes it possible to determine if the inputs where not used the way the
> web developer intended (even if the actual use was
''legal'')
These are good requirements.
>
> in thinking about it further i realize that few objects fall in this 
> category:
> where you can have the ctor succeed and also have that not be what you 
> wanted
> to happen. in this sense Time is unique and therefore makes sense to be
> handled specially. again, i DO agree that a gereral method of 
> obtaining input
> parms to multi parms would be very useful. 
Let me digress for a moment:

Going back to "Four Days on Rails", there''s a bit on page 16
(that''s the
number printed on the page; Acrobat Reader says "page 20 of 37") that
says:

- when a user types input for a numeric field, Rails will always convert 
it to a number - if all else fails, a zero. If
you want to check that the user has actually typed in a number, then you 
need to validate the input
_before_type_cast, which lets you access the ‘raw’ input.

Now I haven''t put in the fancy "New To Do" page yet, so I
still have
fields to type into for "Done", "Priority", and
"Private".
If I type "fred" into the Priority field and submit the form, I get an
error saying "Priority must be between 1 (high) and 5 (low)", the 
Priority field has a red box around it, and the value displayed is still 
"fred".

The validation specified for the Priority attribute was just:

validates_inclusion_of :priority, :in => 1..5, :message => "must be 
between 1 (high) and 5 (low)"

- there was no check specified on :priority_before_type_cast.

So what has happened here is that Rails treated the integer equivalent 
of "fred" as zero, and this failed the range inclusion check - but
when
the screen was redisplayed for correction, it still showed "fred".

Surprisingly,
irb(main):001:0> "fred".to_i
=> 0

(I didn''t expect this - this is unlike Date.new.)

If we want dates to behave like the example above, when we find that the 
raw data won''t make a valid date we can make any old date - the easiest
(and the equivalent of 0 for integers) is just the result of Date.new. 
So long as we bounce the user back to the input screen, that date isn''t
going to be saved to the database.

If we were to replicate the good and bad points of the integer 
behaviour, we could silently convert invalid dates to Date.new and 
require users that didn''t want invalid dates to go into the database to
validate that the converted date value was within a sensible range (or 
at least that the converted date value was not equal to Date.new):

    validates_each :due_date do |record, attr|
      record.errors.add attr, ''invalid date'' if attr ==
Date.new
    end

(Untested!)

This relies on the fact that the result of Date.new is not a very useful 
date. (This assumption would be invalid if the application used this 
value as a "sentinel value".) Apart from this, I think it satisfies
your
criteria:

* does not throw an exception

* remembers original input parameters

* makes it possible to determine if the inputs where not used the way the
web developer intended (even if the actual use was ''legal'')

Hmmm... to generalise this, a brutal (but consistent) approach to the 
general multi-parameter assignment issue would be to require any class 
being constructed via multi-parameter assignment to also be 
constructable with no arguments. If construction with the given 
arguments raised an exception, construct with no arguments instead. 
Leave it to the caller to either validate the raw data (e.g. in the form 
I suggested yesterday) or the result (being able to discriminate the 
result of no-args construction from other values).

Is this making any sense? I don''t like the silent conversion of invalid
input, but if that''s what Ruby does...

One other point - in version 0.10.0, at least, validation failure on 
xxx_before_type_cast isn''t putting the red box around the offending 
field. I should test on 0.10.1 before flagging this as a bug, but given 
the problems being reported with 0.10.1 I''m inclined to wait for
0.10.2.

Can you point me to the place in the current code where a Date is 
constructed from the "multiple parameters"? I haven''t dug
that far in yet.

Justin

On Tue, 15 Mar 2005, Justin Forder wrote:
> It isn''t correct input to a Date constructor.
but is IS correct input to a Time constructor - at that is what will be used
in most cases.  this is really important.
> These are good requirements.
check.
> Let me digress for a moment:
>
> Going back to "Four Days on Rails", there''s a bit on
page 16 (that''s the
> number printed on the page; Acrobat Reader says "page 20 of 37")
that says:
i haven''t read that - but i have read the source ;-)
> - when a user types input for a numeric field, Rails will always convert it
> to a number - if all else fails, a zero. If you want to check that the user
> has actually typed in a number, then you need to validate the input
> _before_type_cast, which lets you access the raw input.
fine.  sketch out a method of

   * providing a _before_type_cast patch for multi-parameters

   * validating them for times

i agree this needs to be done - it''s just that the easiest, and most
correct,
way i could think of is what i did.  if you want to validate time/data input i
think you''ll end up writing something very much like what is already
written
in the builtin time/date classes - and then someone will need to maintain it
;-)
> Now I haven''t put in the fancy "New To Do" page yet, so
I still have fields
> to type into for "Done", "Priority", and
"Private".  If I type "fred" into
> the Priority field and submit the form, I get an error saying
"Priority must
> be between 1 (high) and 5 (low)", the Priority field has a red box
around
> it, and the value displayed is still "fred".
>
> The validation specified for the Priority attribute was just:
>
> validates_inclusion_of :priority, :in => 1..5, :message => "must
be between 1
> (high) and 5 (low)"
>
> - there was no check specified on :priority_before_type_cast.
ah.  yes.  but this is VERY, VERY differnet because

   * here ctor did NOT munge the original input values, it kept them because
     THEY WERE NOT A LIST

   * the test of original input values can be expressed in < 100 lines of
code
> So what has happened here is that Rails treated the integer equivalent of
> "fred" as zero, and this failed the range inclusion check - but
when the
> screen was redisplayed for correction, it still showed "fred".
>
> Surprisingly,
> irb(main):001:0> "fred".to_i
> => 0
>
> (I didn''t expect this - this is unlike Date.new.)
again, this is different for several reasons:

   * frist, we could have exploded early using

     irb(main):001:0> Integer "fred"
     ArgumentError: invalid value for Integer: "fred"
       from (irb):1:in `Integer''
       from (irb):1

   * but we did not, instead we warped the value into ''0''

   * now, later we can see if this is inside a simple range

how is this different from letting times be normalized, but noting that they
were (my patch) so that something can be later?

   * the time is accepted, it does not explode early - this bit is the same

   * we warp time into ''normalized'' values (default system
behaviour!), again
     this is the same

       "fred"      -> 0

       2000, 2, 31 -> 2000, 3, 2

     but a key point is that for "fred" the original string is kept
and for
     times the input list is lost.  my patch remembers it.

   * for validation it breaks down - we cannot use a simple range test of any
   sort?  why?  because any test of year, month, day, etc. to be within a
   certain range will succeed : here there is no way to know that input values
   were outside of ranges UNLESS they were remembered (this is what my patch
   does).  furthermore it has precalculated whether they were outside of
   allowable ranges already - you only need ask.
> If we want dates to behave like the example above, when we find that the
raw
> data won''t make a valid date we can make any old date - the
easiest (and the
> equivalent of 0 for integers) is just the result of Date.new. So long as we
> bounce the user back to the input screen, that date isn''t going to
be saved
> to the database.
yes - that is what the patch does?  and this isn''t quite right - the
value
won''t be saved if, and only if, validation is requested.
> If we were to replicate the good and bad points of the integer behaviour,
we
> could silently convert invalid dates to Date.new and require users that
> didn''t want invalid dates to go into the database to validate that
the
> converted date value was within a sensible range (or at least that the
> converted date value was not equal to Date.new):
>
>   validates_each :due_date do |record, attr|
>     record.errors.add attr, ''invalid date'' if attr ==
Date.new
>   end
this is not right - validation occurs before data hits the database!  if
validation fails the data is not saved!  this is true both for you
integer/range example AND my patch for times.
> (Untested!)
>
> This relies on the fact that the result of Date.new is not a very useful
> date. (This assumption would be invalid if the application used this value
> as a "sentinel value".) Apart from this, I think it satisfies
your criteria:
>
> * does not throw an exception
>
> * remembers original input parameters
>
> * makes it possible to determine if the inputs where not used the way the
> web developer intended (even if the actual use was
''legal'')
but this already exactly what''s happening?  the only thing you missed
is that
all this happend before we hit disk?
> Hmmm... to generalise this, a brutal (but consistent) approach to the
> general multi-parameter assignment issue would be to require any class
being
> constructed via multi-parameter assignment to also be constructable with no
> arguments. If construction with the given arguments raised an exception,
> construct with no arguments instead. Leave it to the caller to either
> validate the raw data (e.g. in the form I suggested yesterday) or the
result
> (being able to discriminate the result of no-args construction from other
> values).
no argument constructors for safety is a very c++ way of thinging no?  again,
this is all totally moot unless you can provide a simply algorithm for users
to ''validate'' time values.  in general i agree with you but
for time objects i
do not.
> Is this making any sense? I don''t like the silent conversion of
invalid
> input, but if that''s what Ruby does...
it is making sense - but i think you need to try my patch.  i really think it
already does what you want.  again, Ruby does NOT DO any silent convertion,
the underlying c call does however.  this is the crux of the problem - if you
do not allow Ruby to do the same you violate POLS, if you do you suprise web
developers.  the happy medium is to flag when ''developer pols''
has been
violated, but not raise an exception, then the web developers themself can
decide what to to.  this is exactly what my patch does and, in general, what
validation sets out to do: validate AFTER form values have been mapped to
objects but BEFORE data hits disk.  my patch is totally consistent with this.
> One other point - in version 0.10.0, at least, validation failure on
> xxx_before_type_cast isn''t putting the red box around the
offending field. I
> should test on 0.10.1 before flagging this as a bug, but given the problems
> being reported with 0.10.1 I''m inclined to wait for 0.10.2.
>
> Can you point me to the place in the current code where a Date is
> constructed from the "multiple parameters"? I haven''t
dug that far in yet.
line 1298 of activerecord-1.8.0/lib/active_record/base.rb, looks like

   def extract_callstack_for_multiparameter_attributes(pairs)

if you haven''t looked here yet the discussion is largely moot - i think
you''ll
see the difficulties if you try to create a patch after reading this code.

in any event i''m happy someone is thinking about this too - so please
don''t
take my objections as motivation to quit!  i''d just like to see the
''right
thing'' done ;-)

kind regards.

-a
-- 
==============================================================================|
EMAIL   :: Ara [dot] T [dot] Howard [at] noaa [dot] gov
| PHONE   :: 303.497.6469
| When you do something, you should burn yourself completely, like a good
| bonfire, leaving no trace of yourself.  --Shunryu Suzuki
==============================================================================_______________________________________________
Rails mailing list
Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org
http://lists.rubyonrails.org/mailman/listinfo/rails

Ara

Thanks very much for your patient and thorough response. I had a long 
day at work today - just got home before midnight (UK time). I have the 
day off tomorrow, and shall start looking properly at the code (and 
running stuff in the console) before responding to your points below. 
Having read your mail and taken a quick look at the code I''m not sure
if
the description in "Four Days on Rails" was correct - was
"fred" being
converted to 0 before the inclusion test, or was I just seeing the 
result of :
irb(main):007:0> (1..5).include? "fred"
=> false
?

thanks again

Justin

Ara.T.Howard wrote:
> On Tue, 15 Mar 2005, Justin Forder wrote:
>
>> It isn''t correct input to a Date constructor.
>
>
> but is IS correct input to a Time constructor - at that is what will 
> be used
> in most cases. this is really important.
>
>> These are good requirements.
>
>
> check.
>
>> Let me digress for a moment:
>>
>> Going back to "Four Days on Rails", there''s a bit on
page 16 (that''s the
>> number printed on the page; Acrobat Reader says "page 20 of
37") that
>> says:
>
>
> i haven''t read that - but i have read the source ;-)
>
>> - when a user types input for a numeric field, Rails will always 
>> convert it
>> to a number - if all else fails, a zero. If you want to check that 
>> the user
>> has actually typed in a number, then you need to validate the input
>> _before_type_cast, which lets you access the �raw� input.
>
>
> fine. sketch out a method of
>
> * providing a _before_type_cast patch for multi-parameters
>
> * validating them for times
>
> i agree this needs to be done - it''s just that the easiest, and
most
> correct,
> way i could think of is what i did. if you want to validate time/data 
> input i
> think you''ll end up writing something very much like what is
already
> written
> in the builtin time/date classes - and then someone will need to 
> maintain it
> ;-)
>
>> Now I haven''t put in the fancy "New To Do" page yet,
so I still have
>> fields
>> to type into for "Done", "Priority", and
"Private". If I type "fred"
>> into
>> the Priority field and submit the form, I get an error saying 
>> "Priority must
>> be between 1 (high) and 5 (low)", the Priority field has a red box
>> around
>> it, and the value displayed is still "fred".
>>
>> The validation specified for the Priority attribute was just:
>>
>> validates_inclusion_of :priority, :in => 1..5, :message =>
"must be
>> between 1 (high) and 5 (low)"
>>
>> - there was no check specified on :priority_before_type_cast.
>
>
> ah. yes. but this is VERY, VERY differnet because
>
> * here ctor did NOT munge the original input values, it kept them because
> THEY WERE NOT A LIST
>
> * the test of original input values can be expressed in < 100 lines of 
> code
>
>> So what has happened here is that Rails treated the integer 
>> equivalent of
>> "fred" as zero, and this failed the range inclusion check -
but when the
>> screen was redisplayed for correction, it still showed
"fred".
>>
>> Surprisingly,
>> irb(main):001:0> "fred".to_i
>> => 0
>>
>> (I didn''t expect this - this is unlike Date.new.)
>
>
> again, this is different for several reasons:
>
> * frist, we could have exploded early using
>
> irb(main):001:0> Integer "fred"
> ArgumentError: invalid value for Integer: "fred"
> from (irb):1:in `Integer''
> from (irb):1
>
> * but we did not, instead we warped the value into ''0''
>
> * now, later we can see if this is inside a simple range
>
> how is this different from letting times be normalized, but noting 
> that they
> were (my patch) so that something can be later?
>
> * the time is accepted, it does not explode early - this bit is the same
>
> * we warp time into ''normalized'' values (default system
behaviour!),
> again
> this is the same
>
> "fred" -> 0
>
> 2000, 2, 31 -> 2000, 3, 2
>
> but a key point is that for "fred" the original string is kept
and for
> times the input list is lost. my patch remembers it.
>
> * for validation it breaks down - we cannot use a simple range test of 
> any
> sort? why? because any test of year, month, day, etc. to be within a
> certain range will succeed : here there is no way to know that input 
> values
> were outside of ranges UNLESS they were remembered (this is what my patch
> does). furthermore it has precalculated whether they were outside of
> allowable ranges already - you only need ask.
>
>> If we want dates to behave like the example above, when we find that 
>> the raw
>> data won''t make a valid date we can make any old date - the
easiest
>> (and the
>> equivalent of 0 for integers) is just the result of Date.new. So long 
>> as we
>> bounce the user back to the input screen, that date isn''t
going to be
>> saved
>> to the database.
>
>
> yes - that is what the patch does? and this isn''t quite right -
the value
> won''t be saved if, and only if, validation is requested.
>
>> If we were to replicate the good and bad points of the integer 
>> behaviour, we
>> could silently convert invalid dates to Date.new and require users that
>> didn''t want invalid dates to go into the database to validate
that the
>> converted date value was within a sensible range (or at least that the
>> converted date value was not equal to Date.new):
>>
>> validates_each :due_date do |record, attr|
>> record.errors.add attr, ''invalid date'' if attr ==
Date.new
>> end
>
>
> this is not right - validation occurs before data hits the database! if
> validation fails the data is not saved! this is true both for you
> integer/range example AND my patch for times.
>
>> (Untested!)
>>
>> This relies on the fact that the result of Date.new is not a very
useful
>> date. (This assumption would be invalid if the application used this 
>> value
>> as a "sentinel value".) Apart from this, I think it satisfies
your
>> criteria:
>>
>> * does not throw an exception
>>
>> * remembers original input parameters
>>
>> * makes it possible to determine if the inputs where not used the way 
>> the
>> web developer intended (even if the actual use was
''legal'')
>
>
> but this already exactly what''s happening? the only thing you
missed
> is that
> all this happend before we hit disk?
>
>> Hmmm... to generalise this, a brutal (but consistent) approach to the
>> general multi-parameter assignment issue would be to require any 
>> class being
>> constructed via multi-parameter assignment to also be constructable 
>> with no
>> arguments. If construction with the given arguments raised an
exception,
>> construct with no arguments instead. Leave it to the caller to either
>> validate the raw data (e.g. in the form I suggested yesterday) or the 
>> result
>> (being able to discriminate the result of no-args construction from 
>> other
>> values).
>
>
> no argument constructors for safety is a very c++ way of thinging no? 
> again,
> this is all totally moot unless you can provide a simply algorithm for 
> users
> to ''validate'' time values. in general i agree with you
but for time
> objects i
> do not.
>
>> Is this making any sense? I don''t like the silent conversion
of invalid
>> input, but if that''s what Ruby does...
>
>
> it is making sense - but i think you need to try my patch. i really 
> think it
> already does what you want. again, Ruby does NOT DO any silent 
> convertion,
> the underlying c call does however. this is the crux of the problem - 
> if you
> do not allow Ruby to do the same you violate POLS, if you do you 
> suprise web
> developers. the happy medium is to flag when ''developer
pols'' has been
> violated, but not raise an exception, then the web developers themself 
> can
> decide what to to. this is exactly what my patch does and, in general, 
> what
> validation sets out to do: validate AFTER form values have been mapped to
> objects but BEFORE data hits disk. my patch is totally consistent with 
> this.
>
>> One other point - in version 0.10.0, at least, validation failure on
>> xxx_before_type_cast isn''t putting the red box around the
offending
>> field. I
>> should test on 0.10.1 before flagging this as a bug, but given the 
>> problems
>> being reported with 0.10.1 I''m inclined to wait for 0.10.2.
>>
>> Can you point me to the place in the current code where a Date is
>> constructed from the "multiple parameters"? I
haven''t dug that far in
>> yet.
>
>
> line 1298 of activerecord-1.8.0/lib/active_record/base.rb, looks like
>
> def extract_callstack_for_multiparameter_attributes(pairs)
>
> if you haven''t looked here yet the discussion is largely moot - i 
> think you''ll
> see the difficulties if you try to create a patch after reading this 
> code.
>
> in any event i''m happy someone is thinking about this too - so
please
> don''t
> take my objections as motivation to quit! i''d just like to see the
''right
> thing'' done ;-)
>
> kind regards.
>
> -a
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Rails mailing list
>Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org
>http://lists.rubyonrails.org/mailman/listinfo/rails
>  
>

Ara.T.Howard wrote:
> On Tue, 15 Mar 2005, Justin Forder wrote:
>
>> It isn''t correct input to a Date constructor.
>
> but is IS correct input to a Time constructor - at that is what will 
> be used
> in most cases. this is really important.

*Both* are important!
>> - when a user types input for a numeric field, Rails will always 
>> convert it
>> to a number - if all else fails, a zero. If you want to check that 
>> the user
>> has actually typed in a number, then you need to validate the input
>> _before_type_cast, which lets you access the �raw� input.
>
> fine. sketch out a method of
>
> * providing a _before_type_cast patch for multi-parameters
>
> * validating them for times
>
> i agree this needs to be done - it''s just that the easiest, and
most
> correct,
> way i could think of is what i did. if you want to validate time/data 
> input i
> think you''ll end up writing something very much like what is
already
> written
> in the builtin time/date classes - and then someone will need to 
> maintain it
> ;-)
Well, we already discussed Date::valid_civil?
Read on - I think it may be even easier.

What I found as soon as I ran script/console and looked at my to do 
items really surprised me:

C:\www\webroot\ToDo>ruby script/console
Loading development environment.
irb(main):001:0> item = Item.find(2)
=> #<Item:0x3309278
@attributes={"created_on"=>"20050310110853",
"note_id"=>nil, "done"=>"0",
"private"=>"0",
"updated_on"=>"20050316092637",
"priority"=>"2", "id"=>"2",
"description"=>"Something to do",
"category_id"=>"0",
"due_date"=>"2005-03-16"}>
irb(main):002:0> item.due_date
=> #<Date: 4906891/2,0,2299161>
irb(main):003:0> item.created_on
=> Thu Mar 10 11:08:53 GMT Standard Time 2005
irb(main):004:0> item.priority = "fred"
=> "fred"
irb(main):005:0> item.priority
=> 0
irb(main):006:0> item.due_date = "2005-02-30"
=> "2005-02-30"
irb(main):007:0> item.due_date
=> nil
irb(main):008:0> item.priority_before_type_cast
=> "fred"
irb(main):009:0> item.due_date_before_type_cast
=> "2005-02-30"

Looking at the code, I see that the "magic accessors" for the 
ActiveRecord state use the ActiveRecord::ConnectionAdapters::Column for 
a given attribute to determine how to convert the type on retrieval:

irb(main):010:0> item.column_for_attribute("due_date").type
=> :date

in 
C:\ruby\lib\ruby\gems\1.8\gems\activerecord-1.6.0\lib\active_record\connection_adapters\abstract_adapter.rb:

def type_cast(value)
if value.nil? then return nil end
case type
when :string then value
when :text then value
when :integer then value.to_i
when :float then value.to_f
when :datetime then string_to_time(value)
when :timestamp then string_to_time(value)
when :time then string_to_dummy_time(value)
when :date then string_to_date(value)
when :binary then binary_to_string(value)
when :boolean then (value == "t" or value == true ? true : false)
else value
end
end

# ...

private
def string_to_date(string)
return string if string.is_a?(Date)
date_array = ParseDate.parsedate(string)
# treat 0000-00-00 as nil
Date.new(date_array[0], date_array[1], date_array[2]) rescue nil
end

def string_to_time(string)
return string if string.is_a?(Time)
time_array = ParseDate.parsedate(string).compact
# treat 0000-00-00 00:00:00 as nil
Time.send(Base.default_timezone, *time_array) rescue nil
end

def string_to_dummy_time(string)
return string if string.is_a?(Time)
time_array = ParseDate.parsedate(string)
# pad the resulting array with dummy date information
time_array[0] = 2000; time_array[1] = 1; time_array[2] = 1;
Time.send(Base.default_timezone, *time_array) rescue nil
end

So... given that Time and Date are already treated as special cases by 
Rails, and their values are normally held in string form, I am starting 
to feel that converting to Time or Date when doing multi-parameter 
assignment is a Bad Thing.

Better to convert to the string form. Then we also have to pick up the 
string form and parse it when redisplaying input to the user on 
validation failure (POLS: users should see exactly what they entered - 
we agree on this, and your code addresses it).

One other surprising result (with MySQL - other databases wouldn''t do 
this): saving with "2005-02-30" in the
@attributes["due_date"] succeeds,
and retrieving the same instance brings back the invalid date value. 
This seems wrong - the domain model is returning nil as the value of the 
attribute, but is neither failing nor writing nil to the database.

I suppose it''s my fault that it didn''t fail - I
hadn''t specified a
validation rule, and MySQL has an |ALLOW_INVALID_DATES| SQL mode, which 
must be turned on in the out-of-the-box MySQL and Rails setup .

Add this to Item:

validates_presence_of :due_date, :message => "must be a valid date"

Now:

C:\www\webroot\ToDo>ruby script/console
Loading development environment.
irb(main):001:0> item = Item.find 2
=> #<Item:0x33089f0
@attributes={"created_on"=>"20050310110853",
"note_id"=>nil, "done"=>"0",
"private"=>"0",
"updated_on"=>"20050316092637",
"priority"=>"2", "id"=>"2",
"description"=>"Something to do",
"category_id"=>"0",
"due_date"=>"2005-03-16"}>
irb(main):002:0> item.due_date = "2006-02-30"
=> "2006-02-30"
irb(main):003:0> item.save
=> false
irb(main):004:0> item.errors
=> #<ActiveRecord::Errors:0x3280640
@errors={"due_date"=>["must be a
valid date"]}, @base=#<Item:0x33089f0 
@errors=#<ActiveRecord::Errors:0x3280640 ...>, 
@attributes={"created_on"=>"20050310110853",
"note_id"=>nil,
"done"=>"0", "private"=>"0",
"updated_on"=>"20050316092637",
"priority"=>"2", "id"=>"2",
"description"=>"Something to do",
"category_id"=>"0",
"due_date"=>"2006-02-30"}, @category=nil>>

This is looking promising.
>> Now I haven''t put in the fancy "New To Do" page yet,
so I still have
>> fields
>> to type into for "Done", "Priority", and
"Private". If I type "fred"
>> into
>> the Priority field and submit the form, I get an error saying 
>> "Priority must
>> be between 1 (high) and 5 (low)", the Priority field has a red box
>> around
>> it, and the value displayed is still "fred".
>>
>> The validation specified for the Priority attribute was just:
>>
>> validates_inclusion_of :priority, :in => 1..5, :message =>
"must be
>> between 1 (high) and 5 (low)"
>>
>> - there was no check specified on :priority_before_type_cast.
>
>
> ah. yes. but this is VERY, VERY differnet because
>
> * here ctor did NOT munge the original input values, it kept them because
> THEY WERE NOT A LIST
>
> * the test of original input values can be expressed in < 100 lines of 
> code

Well, now I know that Rails expresses the multiple input values, 
internally, as a single string - and this can hold invalid entered 
values as well as valid ones. The test (for dates, at least - I haven''t
tried with times) can just be based on the value retrieved through the 
magic accessor being nil, which is easy to express with the current 
validation methods.
>> So what has happened here is that Rails treated the integer 
>> equivalent of
>> "fred" as zero, and this failed the range inclusion check -
but when the
>> screen was redisplayed for correction, it still showed
"fred".
>>
>> Surprisingly,
>> irb(main):001:0> "fred".to_i
>> => 0
>>
>> (I didn''t expect this - this is unlike Date.new.)
>
>
> again, this is different for several reasons:
>
> * frist, we could have exploded early using
>
> irb(main):001:0> Integer "fred"
> ArgumentError: invalid value for Integer: "fred"
> from (irb):1:in `Integer''
> from (irb):1
>
> * but we did not, instead we warped the value into ''0''
>
> * now, later we can see if this is inside a simple range

....and in fact the "warping" is done via to_i, in the attribute 
accessor. The invalid value still exists in the ActiveRecord attributes 
hash.
>
> how is this different from letting times be normalized, but noting 
> that they
> were (my patch) so that something can be later?
>
> * the time is accepted, it does not explode early - this bit is the same
>
> * we warp time into ''normalized'' values (default system
behaviour!),
> again
> this is the same
>
> "fred" -> 0
>
> 2000, 2, 31 -> 2000, 3, 2
>
> but a key point is that for "fred" the original string is kept
and for
> times the input list is lost. my patch remembers it.
>
> * for validation it breaks down - we cannot use a simple range test of 
> any
> sort? why? because any test of year, month, day, etc. to be within a
> certain range will succeed : here there is no way to know that input 
> values
> were outside of ranges UNLESS they were remembered (this is what my patch
> does). furthermore it has precalculated whether they were outside of
> allowable ranges already - you only need ask.
>
>> If we want dates to behave like the example above, when we find that 
>> the raw
>> data won''t make a valid date we can make any old date - the
easiest
>> (and the
>> equivalent of 0 for integers) is just the result of Date.new. So long 
>> as we
>> bounce the user back to the input screen, that date isn''t
going to be
>> saved
>> to the database.
>
>
> yes - that is what the patch does? and this isn''t quite right -
the value
> won''t be saved if, and only if, validation is requested.
>
>> If we were to replicate the good and bad points of the integer 
>> behaviour, we
>> could silently convert invalid dates to Date.new and require users that
>> didn''t want invalid dates to go into the database to validate
that the
>> converted date value was within a sensible range (or at least that the
>> converted date value was not equal to Date.new):
>>
>> validates_each :due_date do |record, attr|
>> record.errors.add attr, ''invalid date'' if attr ==
Date.new
>> end
>
>
> this is not right - validation occurs before data hits the database! if
> validation fails the data is not saved! this is true both for you
> integer/range example AND my patch for times.

Points taken.
>> (Untested!)
>>
>> This relies on the fact that the result of Date.new is not a very
useful
>> date. (This assumption would be invalid if the application used this 
>> value
>> as a "sentinel value".) Apart from this, I think it satisfies
your
>> criteria:
>>
>> * does not throw an exception
>>
>> * remembers original input parameters
>>
>> * makes it possible to determine if the inputs where not used the way 
>> the
>> web developer intended (even if the actual use was
''legal'')
>
>
> but this already exactly what''s happening? the only thing you
missed
> is that
> all this happend before we hit disk?
>
>> Hmmm... to generalise this, a brutal (but consistent) approach to the
>> general multi-parameter assignment issue would be to require any 
>> class being
>> constructed via multi-parameter assignment to also be constructable 
>> with no
>> arguments. If construction with the given arguments raised an
exception,
>> construct with no arguments instead. Leave it to the caller to either
>> validate the raw data (e.g. in the form I suggested yesterday) or the 
>> result
>> (being able to discriminate the result of no-args construction from 
>> other
>> values).
>
>
> no argument constructors for safety is a very c++ way of thinging no? 
> again,
> this is all totally moot unless you can provide a simply algorithm for 
> users
> to ''validate'' time values. in general i agree with you
but for time
> objects i
> do not.

Yes, and I don''t agree with me either, now :-)

I''m pleased to find that Time and Date are already being treated as 
special cases, because I think we can find a solution for them, and that 
it wouldn''t extend to user-defined composite values.
>> Is this making any sense? I don''t like the silent conversion
of invalid
>> input, but if that''s what Ruby does...
>
>
> it is making sense - but i think you need to try my patch. i really 
> think it
> already does what you want. again, Ruby does NOT DO any silent 
> convertion,
> the underlying c call does however. this is the crux of the problem - 
> if you
> do not allow Ruby to do the same you violate POLS, if you do you 
> suprise web
> developers. the happy medium is to flag when ''developer
pols'' has been
> violated, but not raise an exception, then the web developers themself 
> can
> decide what to to. this is exactly what my patch does and, in general, 
> what
> validation sets out to do: validate AFTER form values have been mapped to
> objects but BEFORE data hits disk. my patch is totally consistent with 
> this.

Valuable points. With regard to your patch, I would like to avoid adding 
state to every Time instance

(@original_arguments, @normalized).

>> One other point - in version 0.10.0, at least, validation failure on
>> xxx_before_type_cast isn''t putting the red box around the
offending
>> field. I
>> should test on 0.10.1 before flagging this as a bug, but given the 
>> problems
>> being reported with 0.10.1 I''m inclined to wait for 0.10.2.
>>
>> Can you point me to the place in the current code where a Date is
>> constructed from the "multiple parameters"? I
haven''t dug that far in
>> yet.
>
>
> line 1298 of activerecord-1.8.0/lib/active_record/base.rb, looks like
>
> def extract_callstack_for_multiparameter_attributes(pairs)

Thanks! I had missed the important part when I first looked at this. 
Powerful stuff in Ruby is so concise!
>
> if you haven''t looked here yet the discussion is largely moot - i 
> think you''ll
> see the difficulties if you try to create a patch after reading this 
> code.
>
> in any event i''m happy someone is thinking about this too - so
please
> don''t
> take my objections as motivation to quit! i''d just like to see the
''right
> thing'' done ;-)

It''s a pleasure discussing this.

best regards

Justin

Justin Forder wrote:
> Ara.T.Howard wrote:
>> ah. yes. but this is VERY, VERY differnet because
>>
>> * here ctor did NOT munge the original input values, it kept them
because
>> THEY WERE NOT A LIST
>>
>> * the test of original input values can be expressed in < 100 lines
of
>> code
> 
> Well, now I know that Rails expresses the multiple input values, 
> internally, as a single string - and this can hold invalid entered 
> values as well as valid ones. The test (for dates, at least - I
haven''t
> tried with times) can just be based on the value retrieved through the 
> magic accessor being nil, which is easy to express with the current 
> validation methods.
> 
Testing with times: 30th Feb "warps", 0th Feb is treated as invalid.

irb(main):005:0> item.created_on = "20050230110853"
=> "20050230110853"
irb(main):006:0> item.created_on
=> Wed Mar 02 11:08:53 GMT Standard Time 2005
irb(main):007:0> item.created_on = "20050200110853"
=> "20050200110853"
irb(main):008:0> item.created_on
=> nil

So the nil check that works for dates is not enough. To check for 
"warping" need to check whether raw day (or month) is equal to day (or
month) retrieved via accessor. Not hard, but it does require a specific 
validation method, whereas date does not.

    Justin

[At last I''ve found out how to set up Mozilla Thunderbird - which
I''ve
just started using - to stick to plain text. Sorry about the poor 
formatting in the last post - Thunderbird removed all the code 
indentation when sending. Should be OK now.]

On Wed, 16 Mar 2005, Justin Forder wrote:
> *Both* are important!
yes, of course.  but you can make a Date from a Time and not the otherway
around so - i''ve you can skin the cat from existing Time code you are
done.
> Well, we already discussed Date::valid_civil?
yes.  but that is

a) half of the problem
b) occurs too early in rails processing chain to be used in validation.

i may be wrong but i think it is IMPOSSIBLE to allow valid_civil? to be used
at the validation stage and before data has been written to the db.
> Read on - I think it may be even easier.
<snip rails'' source>
> So... given that Time and Date are already treated as special cases by
> Rails, and their values are normally held in string form, I am starting to
> feel that converting to Time or Date when doing multi-parameter assignment
> is a Bad Thing.
i strongly agree.  DHH - can you weigh in here?  i feel that it''s odd
that
multiparameter args get mapped to objects and everything else gets mapped to
strings.  symmetry says single parms come back as strings and multi parms come
back as arrays of strings.  if this were about symmetry only it''d be
silly -
but this cleaning up would aid GREATLY in validation of any, not just
Time/Date, multi param classes - many could face exactly the same issue we see
with Time/Date now.
> Better to convert to the string form. Then we also have to pick up the
> string form and parse it when redisplaying input to the user on validation
> failure (POLS: users should see exactly what they entered - we agree on
> this, and your code addresses it).
sure - if we can get the rails crew to buy it.  it makes mucho sense to me.
my patch was really aimed at the fewest lines of code to get correct behaviour
NOW and without modifying rails.  if rails might be changed then i''m
totally
in agreement with what you are suggesting.
> One other surprising result (with MySQL - other databases wouldn''t
do this):
> saving with "2005-02-30" in the @attributes["due_date"]
succeeds, and
> retrieving the same instance brings back the invalid date value. This seems
> wrong - the domain model is returning nil as the value of the attribute,
but
> is neither failing nor writing nil to the database.
>
> I suppose it''s my fault that it didn''t fail - I
hadn''t specified a
> validation rule, and MySQL has an |ALLOW_INVALID_DATES| SQL mode, which
must
> be turned on in the out-of-the-box MySQL and Rails setup .
didn''t you know - mysql blows for times and dates:

   * The MySQL server performs only basic checking on the validity of a date:
    The ranges for year, month, and day are 1000 to 9999, 00 to 12, and 00 to
    31, respectively. Any date containing parts not within these ranges is
    subject to conversion to ''0000-00-00''. Please note that
this still allows
    you to store invalid dates such as ''2002-04-31''. To ensure
that a date is
    valid, perform a check in your application.

add this to lack of complete referential integrity implementaion and you can
rest assured of corrupting your data relationships - albeit at a rate much
faster than other open source databases.  ;-)
> Add this to Item:
>
> validates_presence_of :due_date, :message => "must be a valid
date"
>
> Now:
>
> C:\www\webroot\ToDo>ruby script/console
> Loading development environment.
> irb(main):001:0> item = Item.find 2
> => #<Item:0x33089f0
@attributes={"created_on"=>"20050310110853",
> "note_id"=>nil, "done"=>"0",
"private"=>"0",
"updated_on"=>"20050316092637",
> "priority"=>"2", "id"=>"2",
"description"=>"Something to do",
> "category_id"=>"0",
"due_date"=>"2005-03-16"}>
> irb(main):002:0> item.due_date = "2006-02-30"
> => "2006-02-30"
> irb(main):003:0> item.save
> => false
> irb(main):004:0> item.errors
> => #<ActiveRecord::Errors:0x3280640
@errors={"due_date"=>["must be a valid
> date"]}, @base=#<Item:0x33089f0
@errors=#<ActiveRecord::Errors:0x3280640
> ...>,
@attributes={"created_on"=>"20050310110853",
"note_id"=>nil,
> "done"=>"0", "private"=>"0",
"updated_on"=>"20050316092637",
"priority"=>"2",
> "id"=>"2",
"description"=>"Something to do",
"category_id"=>"0",
> "due_date"=>"2006-02-30"}, @category=nil>>
>
> This is looking promising.
yes it does.

i think this is an artifact of mysql mapping bad times to
''0000-00-00'' and
only works in this case.  consider sqlite, which stores everything as strings
- the field will be present and the validation will succeed.  so this is not a
complete solution.  if you think about it - any scheme that validates using
the underlying db is not complete - rails must do it using builtin or custom
classes so it can be performed BEFORE the data hits disk.
> Well, now I know that Rails expresses the multiple input values,
internally,
> as a single string - and this can hold invalid entered values as well as
> valid ones. The test (for dates, at least - I haven''t tried with
times) can
> just be based on the value retrieved through the magic accessor being nil,
> which is easy to express with the current validation methods.
remember rails works with sqlite - 2 and 3.

   http://www.sqlite.org/datatypes.html

i haven''t tested by am %99.99 postive it''ll fail with those
dbs since, at
least for 2, it''ll quietly accept any string in a timestamp field...
> ....and in fact the "warping" is done via to_i, in the attribute
accessor.
> The invalid value still exists in the ActiveRecord attributes hash.
right - but not for multi parms...

<snip>
>> this is not right - validation occurs before data hits the database! if
>> validation fails the data is not saved! this is true both for you
>> integer/range example AND my patch for times.
>
> Points taken.
again circling back to sqlite example...
> Yes, and I don''t agree with me either, now :-)
>
> I''m pleased to find that Time and Date are already being treated
as special
> cases, because I think we can find a solution for them, and that it
wouldn''t
> extend to user-defined composite values.
there could be a

   if klass.respond_to? ''validate''
     klass.validate(*values)
     ...

hook though...

but then what?  can''t throw an exception...

hmm.

<snip>
>> decide what to to. this is exactly what my patch does and, in general,
what
>> validation sets out to do: validate AFTER form values have been mapped
to
>> objects but BEFORE data hits disk. my patch is totally consistent with
>> this.
>
> Valuable points. With regard to your patch, I would like to avoid adding
> state to every Time instance
why?  this is nothing compared with everyting else going on in rails...
>> def extract_callstack_for_multiparameter_attributes(pairs)
>
> Thanks! I had missed the important part when I first looked at this.
> Powerful stuff in Ruby is so concise!
true true.  it took be some staring too - only the ''('' char
caught my
attention!
> It''s a pleasure discussing this.
you are a sick man ;-)

at least we seem to be converging on something.  i think our assesment at this
point is very accurate and we need the rails crew to speak up - or do we just
write a patch?

kind regards.

-a
-- 
==============================================================================|
EMAIL   :: Ara [dot] T [dot] Howard [at] noaa [dot] gov
| PHONE   :: 303.497.6469
| When you do something, you should burn yourself completely, like a good
| bonfire, leaving no trace of yourself.  --Shunryu Suzuki
===============================================================================

Ara.T.Howard wrote:
> at least we seem to be converging on something.  i think our assesment 
> at this point is very accurate and we need the rails crew to speak 
 > up - or do we just write a patch?

A little guidance from the management would be welcome! Experimenting 
can continue in parallel.

With regards to your points about other databases, I don''t want bad 
values to be stored, and I was surprised that the out-of-the-box 
MySQL/Rails setup would do that. I have PostgreSQL on this box as well...

best regards

    Justin