Prevent server blow up when iterating over TimeWithZone Range

[bogdan]

Iterating over a Range of Time is impossible in ruby:

((Time.now - 1.month)..Time.now).to_a # raises TypeError: can't iterate from Time

But it is still possible with ActiveSupport::TimeWithZone

((Time.now.in_time_zone - 1.month)..Time.now.in_time_zone).to_a
    # => Tons of deprecations blow up your console

In good case current process is stuck with IO.
In worth case server disk space blow up with production.log file.

This was a huge problem for us and caused a down time on our production app.

In order to prevent that we can generate #succ deprecation warning only once.
And remove TimeWithZone#succ as soon as Time#succ will be removed.

[senny]

@pixeltrix can you take a look?

[pixeltrix]

I think I'd rather override Range to check if it acts_like?(:time) and raise TypeError: can't iterate from Time - it's what they do internally in Ruby.

[bogdan]

Reworked as @pixeltrix suggested.

[pixeltrix]

@bogdan you might need to do Range#step as well - also test that Range#include? raises the error.

Also you probably need to require 'active_support/core_ext/object/acts_like' as one of the Active Model tests is failing with a NoMethodError.

[bogdan]

@pixeltrix made all changes you've suggested.

[pixeltrix]

I think this should be step_without_time_with_zone 😉

[pixeltrix]

@bogdan thanks - sorry for the delay

[bogdan]

I would backport it to 3.2 as it is critical issue when people migrate from 1.8 to 1.9.

But it is for you to decide.

[mjtko]

@pixeltrix @bogdan The merge of this has broken iteration over DateTime objects as, with the core extensions, they rightly indicate that they act like a time object. However, this is a change from standard Ruby behaviour, so I believe this to be a bug.

I can monkey patch for now - my solution is to raise only if it acts_like_time? but doesn't acts_like_date?. How does that sound? I can throw together a PR if that seems like a sensible solution to you.

[bogdan]

@mjtko I personally was against those acts_like magic.
If this patch disallows iteration over DateTime than we should fix it as ruby itself allows it.

So, I agree.

[mjtko]

@bogdan Yeah, I was looking for your original patch pre-acts_like, but Github doesn't want me to find it easily, so I gave up. :) Do you still have it somewhere?

[bogdan]

@mjtko didn't find it with git fsck. Maybe push --force make it lost forever.

[pixeltrix]

😧 I didn't realise that you can't iterate over Time but you can over DateTime. I suspect this maybe an unintentional side effect of DateTime < Date - I guess changing the check to is_a?(Time) will work, okay with you @bogdan ?

[bogdan]

@pixeltrix sure.

Its quite strange that ruby iterates over DateTime in the same way as over Date but not as Time. We didn't take that into count in this patch.

[madwire]

Any news on this problem? Just had to monkey patch the Range Class 😟

[pixeltrix]

@madwire do you mean the original issue or that DateTime ranges can't be iterated?

[madwire]

@pixeltrix sorry I mean the DateTime issue