Integrate core logic into _calc()

2 files changed, 100 insertions, 38 deletions

diff --git a/src/croniter/croniter.py b/src/croniter/croniter.py
index 0275061..74e4eb8 100644
--- a/src/croniter/croniter.py
+++ b/src/croniter/croniter.py
@@ -220,12 +220,14 @@ class croniter(object):
         return (td.microseconds + (td.seconds + td.days * 24 * 3600) * 10**6) \
             / 10**6
 
-    def __get_next(self, ret_type=None, is_prev=None):
+    def _get_next(self, ret_type=None, is_prev=None):
         if is_prev is None:
             is_prev = self._is_prev
         self._is_prev = is_prev
         expanded = self.expanded[:]
         nth_weekday_of_month = self.nth_weekday_of_month.copy()
+        # Q: Will we be updating this?  if not, no need to copy.... but having a local reference is nice
+        last_weekday_of_month = self.last_weekday_of_month.copy()
 
         ret_type = ret_type or self._ret_type
 
@@ -237,18 +239,20 @@ class croniter(object):
         if (expanded[2][0] != '*' and expanded[4][0] != '*') and self._day_or:
             bak = expanded[4]
             expanded[4] = ['*']
-            t1 = self._calc(self.cur, expanded, nth_weekday_of_month, is_prev)
+            t1 = self._calc(self.cur, expanded, nth_weekday_of_month,
+                            last_weekday_of_month, is_prev)
             expanded[4] = bak
             expanded[2] = ['*']
 
-            t2 = self._calc(self.cur, expanded, nth_weekday_of_month, is_prev)
+            t2 = self._calc(self.cur, expanded, nth_weekday_of_month,
+                            last_weekday_of_month, is_prev)
             if not is_prev:
                 result = t1 if t1 < t2 else t2
             else:
                 result = t1 if t1 > t2 else t2
         else:
-            result = self._calc(self.cur, expanded,
-                                nth_weekday_of_month, is_prev)
+            result = self._calc(self.cur, expanded, nth_weekday_of_month,
+                                last_weekday_of_month, is_prev)
 
         # DST Handling for cron job spanning accross days
         dtstarttime = self._timestamp_to_datetime(self.dst_start_time)
@@ -281,35 +285,6 @@ class croniter(object):
             result = dtresult
         return result
 
-    def _filter_output(self, timestamp):
-        # type: (datetime)
-        # Currently assumes ret_type=datetime; all I need
-        if self.last_weekday_of_month:
-            ts_dow = timestamp.isoweekday() % 7
-            if ts_dow in self.last_weekday_of_month:
-                last_dow = self._get_last_weekday_of_month(
-                    timestamp.year, timestamp.month, ts_dow)
-                return timestamp.day == last_dow
-            else:
-                # Have LDOM, but not for current day, return anyways???
-                # Q: Can this still happen after blocking L/non-L mixing in dow?
-                return True
-        else:
-            # For all the other "normal" cron expression, no extra filter needed
-            return True
-
-    def _get_next(self, ret_type=None, is_prev=None):
-        """ Basically hijack the next() mechanism, and keep finding going until
-        the output filter accepts.
-
-        This could be expensive, for example, if the cron expression fires every
-        second, then this could try and fail 86400 times before the next match.
-        """
-        while True:
-            result = self.__get_next(ret_type, is_prev)
-            if self._filter_output(result):
-                return result
-
     # iterator protocol, to enable direct use of croniter
     # objects in a loop, like "for dt in croniter('5 0 * * *'): ..."
     # or for combining multiple croniters into single
@@ -349,7 +324,8 @@ class croniter(object):
         return self
     __next__ = next = _get_next
 
-    def _calc(self, now, expanded, nth_weekday_of_month, is_prev):
+    def _calc(self, now, expanded, nth_weekday_of_month, last_weekday_of_month,
+              is_prev):
         if is_prev:
             now = math.ceil(now)
             nearest_diff_method = self._get_prev_nearest_diff
@@ -485,6 +461,27 @@ class croniter(object):
                 return True, d
             return False, d
 
+        def proc_day_of_week_last(d):
+            # Most of the work is handled by proc_day_of_week_nth, because internally 'L5' becomes '5#4,5#5'
+            changed, dst = proc_day_of_week_nth(d)
+            if not changed:
+                ts_dow = d.isoweekday() % 7
+                if ts_dow in self.last_weekday_of_month:
+                    last_dow = self._get_last_weekday_of_month(
+                        d.year, d.month, ts_dow)
+                    if d.day == last_dow:
+                        return False, d
+                    else:
+                        # NEED HELP here!!
+                        # XXX How do we know how much/little to bump forward/backwards?
+                        # This seems to work, even if the next match is less than 1 day away, making me even *more* confused!
+                        if is_prev:
+                            d -= relativedelta(days=1)
+                        else:
+                            d += relativedelta(days=1)
+                        return True, d
+            return changed, dst
+
         def proc_hour(d):
             try:
                 expanded[1].index('*')
@@ -525,10 +522,16 @@ class croniter(object):
                 d += relativedelta(second=0)
             return False, d
 
+        if last_weekday_of_month:
+            proc_dow = proc_day_of_week_last
+        elif nth_weekday_of_month:
+            proc_dow = proc_day_of_week_nth
+        else:
+            proc_dow = proc_day_of_week
+
         procs = [proc_month,
                  proc_day_of_month,
-                 (proc_day_of_week_nth if nth_weekday_of_month
-                     else proc_day_of_week),
+                 proc_dow,
                  proc_hour,
                  proc_minute,
                  proc_second]
diff --git a/src/croniter/tests/test_croniter.py b/src/croniter/tests/test_croniter.py
index 0948a5a..9857a53 100755
--- a/src/croniter/tests/test_croniter.py
+++ b/src/croniter/tests/test_croniter.py
@@ -322,6 +322,7 @@ class CroniterTest(base.TestCase):
         self.assertEqual(croniter('0 0 0 1-12 0').expanded[mon], wildcard)
         self.assertEqual(croniter('0 0 0 0 0-6').expanded[dow], wildcard)
         self.assertEqual(croniter('0 0 0 0 1-7').expanded[dow], wildcard)
+        self.assertEqual(croniter('0 0 0 0 1-7,sat#3').expanded[dow], wildcard)
         self.assertEqual(croniter('0 0 0 0 0 0-59').expanded[s], wildcard)
         # Real life examples
         self.assertEqual(croniter('30 1-12,0,10-23 15-21 * fri').expanded[h], wildcard)
@@ -1126,7 +1127,7 @@ class CroniterTest(base.TestCase):
     def test_croniter_last_friday(self):
         it = croniter("0 0 * * L5", datetime(1987, 1, 15), ret_type=datetime)
         items = [next(it) for i in range(12)]
-        self.maxDiff = 100000
+        self.maxDiff = 1000
         self.assertListEqual(items, [
             datetime(1987, 1, 30),
             datetime(1987, 2, 27),
@@ -1142,6 +1143,64 @@ class CroniterTest(base.TestCase):
             datetime(1987, 12, 25),
         ])
 
+    def test_croniter_last_friday_2hours(self):
+        # This works with +/- 'days=1' in proc_day_of_week_last() and I don't know WHY?!?
+        it = croniter("0 1,5 * * L5", datetime(1987, 1, 15), ret_type=datetime)
+        items = [next(it) for i in range(12)]
+        self.maxDiff = 1000
+        self.assertListEqual(items, [
+            datetime(1987, 1, 30, 1),
+            datetime(1987, 1, 30, 5),
+            datetime(1987, 2, 27, 1),
+            datetime(1987, 2, 27, 5),
+            datetime(1987, 3, 27, 1),
+            datetime(1987, 3, 27, 5),
+            datetime(1987, 4, 24, 1),
+            datetime(1987, 4, 24, 5),
+            datetime(1987, 5, 29, 1),
+            datetime(1987, 5, 29, 5),
+            datetime(1987, 6, 26, 1),
+            datetime(1987, 6, 26, 5),
+        ])
+
+    def test_croniter_last_friday_2xh_2xm(self):
+        it = croniter("0,30 1,5 * * L5", datetime(1987, 1, 15), ret_type=datetime)
+        items = [next(it) for i in range(12)]
+        self.maxDiff = 1000
+        self.assertListEqual(items, [
+            datetime(1987, 1, 30, 1, 0),
+            datetime(1987, 1, 30, 1, 30),
+            datetime(1987, 1, 30, 5, 0),
+            datetime(1987, 1, 30, 5, 30),
+            datetime(1987, 2, 27, 1, 0),
+            datetime(1987, 2, 27, 1, 30),
+            datetime(1987, 2, 27, 5, 0),
+            datetime(1987, 2, 27, 5, 30),
+            datetime(1987, 3, 27, 1, 0),
+            datetime(1987, 3, 27, 1, 30),
+            datetime(1987, 3, 27, 5, 0),
+            datetime(1987, 3, 27, 5, 30),
+        ])
+
+    def test_croniter_last_saturday_rev(self):
+        it = croniter("0 0 * * L6", datetime(2017, 12, 31), ret_type=datetime, is_prev=True)
+        items = [next(it) for i in range(12)]
+        self.maxDiff = 1000
+        self.assertListEqual(items, [
+            datetime(2017, 12, 30),
+            datetime(2017, 11, 25),
+            datetime(2017, 10, 28),
+            datetime(2017, 9, 30),
+            datetime(2017, 8, 26),
+            datetime(2017, 7, 29),
+            datetime(2017, 6, 24),
+            datetime(2017, 5, 27),
+            datetime(2017, 4, 29),
+            datetime(2017, 3, 25),
+            datetime(2017, 2, 25),
+            datetime(2017, 1, 28),
+        ])
+    
     def test_issue_142_dow(self):
         ret = []
         for i in range(1, 31):