Maintain current timezone when changing time during DST overlap

Currently if a time is changed during DST overlap in the autumn then the
method `period_for_local` will return the DST period. However if the
original time is not DST then this can be surprising and is not what is
generally wanted. This commit changes that behavior to maintain the current
period if it's in the list of periods returned by `periods_for_local`.

It is possible to alter the behavior of `period_for_local` by specifying a
second argument but since we may be change from another time that could be
either DST or not then this would give inconsistent results.

Fixes #12163.

activesupport/CHANGELOG.md

+*   Maintain the current timezone when calling `change` during DST overlap
+
+    Currently if a time is changed during DST overlap in the autumn then the method
+    `period_for_local` will return the DST period. However if the original time is
+    not DST then this can be surprising and is not what is generally wanted. This
+    commit changes that behavior to maintain the current period if it's in the list
+    of periods returned by `periods_for_local`.
+
+    Fixes #12163.
+
+    *Andrew White*
+
 *   Added `Hash#compact` and `Hash#compact!` for removing items with nil value from hash.
 
     *Celestino Gomes*

activesupport/lib/active_support/time_with_zone.rb

@@ -45,7 +45,7 @@ def self.name
 
     def initialize(utc_time, time_zone, local_time = nil, period = nil)
       @utc, @time_zone, @time = utc_time, time_zone, local_time
-      @period = @utc ? period : get_period_and_ensure_valid_local_time
+      @period = @utc ? period : get_period_and_ensure_valid_local_time(period)
     end
 
     # Returns a Time or DateTime instance that represents the time in +time_zone+.
@@ -292,6 +292,12 @@ def advance(options)
       end
     end
 
+    def change(options)
+      new_time = time.change(options)
+      periods = time_zone.periods_for_local(new_time)
+      self.class.new(nil, time_zone, new_time, periods.include?(period) ? period : nil)
+    end
+
     %w(year mon month day mday wday yday hour min sec usec nsec to_date).each do |method_name|
       class_eval <<-EOV, __FILE__, __LINE__ + 1
         def #{method_name}    # def month
@@ -367,12 +373,12 @@ def method_missing(sym, *args, &block)
     end
 
     private
-      def get_period_and_ensure_valid_local_time
+      def get_period_and_ensure_valid_local_time(period)
         # we don't want a Time.local instance enforcing its own DST rules as well,
         # so transfer time values to a utc constructor if necessary
         @time = transfer_time_values_to_utc_constructor(@time) unless @time.utc?
         begin
-          @time_zone.period_for_local(@time)
+          period || @time_zone.period_for_local(@time)
         rescue ::TZInfo::PeriodNotFound
           # time is in the "spring forward" hour gap, so we're moving the time forward one hour and trying again
           @time += 1.hour

activesupport/lib/active_support/values/time_zone.rb

@@ -352,6 +352,10 @@ def period_for_local(time, dst=true)
       tzinfo.period_for_local(time, dst)
     end
 
+    def periods_for_local(time) #:nodoc:
+      tzinfo.periods_for_local(time)
+    end
+
     def self.find_tzinfo(name)
       TZInfo::TimezoneProxy.new(MAPPING[name] || name)
     end

activesupport/test/core_ext/time_with_zone_test.rb

@@ -511,6 +511,11 @@ def test_change
     assert_equal "Fri, 31 Dec 1999 19:00:30 EST -05:00", @twz.change(:sec => 30).inspect
   end
 
+  def test_change_at_dst_boundary
+    twz = ActiveSupport::TimeWithZone.new(Time.at(1319936400).getutc, ActiveSupport::TimeZone['Madrid'])
+    assert_equal twz, twz.change(:min => 0)
+  end
+
   def test_advance
     assert_equal "Fri, 31 Dec 1999 19:00:00 EST -05:00", @twz.inspect
     assert_equal "Mon, 31 Dec 2001 19:00:00 EST -05:00", @twz.advance(:years => 2).inspect