lib/rufus/scheduler/cronline.rb in rufus-scheduler-3.0.2 vs lib/rufus/scheduler/cronline.rb in rufus-scheduler-3.0.3

@@ -120,20 +120,17 @@
     #
     # (Thanks to K Liu for the note and the examples)
     #
     def next_time(from=Time.now)
 
-      time = @timezone ? @timezone.utc_to_local(from.getutc) : from
+      time = local_time(from)
+      time = round_to_seconds(time)
 
-      time = time.respond_to?(:round) ? time.round : time - time.usec * 1e-6
-        # chop off subseconds (and yes, Ruby 1.8 doesn't have #round)
-
+      # start at the next second
       time = time + 1
-        # start at the next second
 
       loop do
-
         unless date_match?(time)
           time += (24 - time.hour) * 3600 - time.min * 60 - time.sec; next
         end
         unless sub_match?(time, :hour, @hours)
           time += (60 - time.min) * 60 - time.sec; next
@@ -146,38 +143,42 @@
         end
 
         break
       end
 
-      if @timezone
-        time = @timezone.local_to_utc(time)
-        time = time.getlocal unless from.utc?
-      end
-
-      time
+      global_time(time, from.utc?)
     end
 
-    # Returns the previous the cronline matched. It's like next_time, but
+    # Returns the previous time the cronline matched. It's like next_time, but
     # for the past.
     #
     def previous_time(from=Time.now)
 
-      # looks back by slices of two hours,
-      #
-      # finds for '* * * * sun', '* * 13 * *' and '0 12 13 * *'
-      # starting 1970, 1, 1 in 1.8 to 2 seconds (says Rspec)
+      time = local_time(from)
+      time = round_to_seconds(time)
 
-      start = current = from - 2 * 3600
-      result = nil
+      # start at the previous second
+      time = time - 1
 
       loop do
-        nex = next_time(current)
-        return (result ? result : previous_time(start)) if nex > from
-        result = current = nex
+        unless date_match?(time)
+          time -= time.hour * 3600 + time.min * 60 + time.sec + 1; next
+        end
+        unless sub_match?(time, :hour, @hours)
+          time -= time.min * 60 + time.sec + 1; next
+        end
+        unless sub_match?(time, :min, @minutes)
+          time -= time.sec + 1; next
+        end
+        unless sub_match?(time, :sec, @seconds)
+          time -= 1; next
+        end
+
+        break
       end
 
-      # never reached
+      global_time(time, from.utc?)
     end
 
     # Returns an array of 6 arrays (seconds, minutes, hours, days,
     # months, weekdays).
     # This method is used by the cronline unit tests.
@@ -197,10 +198,31 @@
     end
 
     # Returns the shortest delta between two potential occurences of the
     # schedule described by this cronline.
     #
+    # .
+    #
+    # For a simple cronline like "*/5 * * * *", obviously the frequency is
+    # five minutes. Why does this method look at a whole year of #next_time ?
+    #
+    # Consider "* * * * sun#2,sun#3", the computed frequency is 1 week
+    # (the shortest delta is the one between the second sunday and the third
+    # sunday). This method takes no chance and runs next_time for the span
+    # of a whole year and keeps the shortest.
+    #
+    # Of course, this method can get VERY slow if you call on it a second-
+    # based cronline...
+    #
+    # Since it's a rarely used method, I haven't taken the time to make it
+    # smarter/faster.
+    #
+    # One obvious improvement would be to cache the result once computed...
+    #
+    # See https://github.com/jmettraux/rufus-scheduler/issues/89
+    # for a discussion about this method.
+    #
     def frequency
 
       delta = 366 * DAY_S
 
       t0 = previous_time(Time.local(2000, 1, 1))
@@ -377,9 +399,27 @@
         break if d.month != date.month
         neg = neg - 1
       end
 
       [ "#{WEEKDAYS[date.wday]}##{pos}", "#{WEEKDAYS[date.wday]}##{neg}" ]
+    end
+
+    def local_time(time)
+      time = @timezone ? @timezone.utc_to_local(time.getutc) : time
+    end
+
+    def global_time(time, from_in_utc)
+      if @timezone
+        time = @timezone.local_to_utc(time)
+        time = time.getlocal unless from_in_utc
+      end
+
+      time
+    end
+
+    def round_to_seconds(time)
+      # Ruby 1.8 doesn't have #round
+      time.respond_to?(:round) ? time.round : time - time.usec * 1e-6
     end
   end
 end