Fixed wrong job sort order in some job stores and a related exception in the redis store (fixes #86)

6 files changed, 31 insertions, 7 deletions

diff --git a/apscheduler/jobstores/base.py b/apscheduler/jobstores/base.py
index e09e40a..dba0b78 100644
--- a/apscheduler/jobstores/base.py
+++ b/apscheduler/jobstores/base.py
@@ -50,6 +50,15 @@ class BaseJobStore(six.with_metaclass(ABCMeta)):
     def shutdown(self):
         """Frees any resources still bound to this job store."""
 
+    def _fix_paused_jobs_sorting(self, jobs):
+        for i, job in enumerate(jobs):
+            if job.next_run_time is not None:
+                if i > 0:
+                    paused_jobs = jobs[:i]
+                    del jobs[:i]
+                    jobs.extend(paused_jobs)
+                break
+
     @abstractmethod
     def lookup_job(self, job_id):
         """
diff --git a/apscheduler/jobstores/mongodb.py b/apscheduler/jobstores/mongodb.py
index ff762f7..8b50c4e 100644
--- a/apscheduler/jobstores/mongodb.py
+++ b/apscheduler/jobstores/mongodb.py
@@ -64,7 +64,9 @@ class MongoDBJobStore(BaseJobStore):
         return utc_timestamp_to_datetime(document['next_run_time']) if document else None
 
     def get_all_jobs(self):
-        return self._get_jobs({})
+        jobs = self._get_jobs({})
+        self._fix_paused_jobs_sorting(jobs)
+        return jobs
 
     def add_job(self, job):
         try:
diff --git a/apscheduler/jobstores/redis.py b/apscheduler/jobstores/redis.py
index 2b4ffd5..311b094 100644
--- a/apscheduler/jobstores/redis.py
+++ b/apscheduler/jobstores/redis.py
@@ -1,5 +1,7 @@
 from __future__ import absolute_import
+from datetime import datetime
 
+from pytz import utc
 import six
 
 from apscheduler.jobstores.base import BaseJobStore, JobLookupError, ConflictingIdError
@@ -65,7 +67,8 @@ class RedisJobStore(BaseJobStore):
     def get_all_jobs(self):
         job_states = self.redis.hgetall(self.jobs_key)
         jobs = self._reconstitute_jobs(six.iteritems(job_states))
-        return sorted(jobs, key=lambda job: job.next_run_time)
+        paused_sort_key = datetime(9999, 12, 31, tzinfo=utc)
+        return sorted(jobs, key=lambda job: job.next_run_time or paused_sort_key)
 
     def add_job(self, job):
         if self.redis.hexists(self.jobs_key, job.id):
@@ -74,7 +77,8 @@ class RedisJobStore(BaseJobStore):
         with self.redis.pipeline() as pipe:
             pipe.multi()
             pipe.hset(self.jobs_key, job.id, pickle.dumps(job.__getstate__(), self.pickle_protocol))
-            pipe.zadd(self.run_times_key, datetime_to_utc_timestamp(job.next_run_time), job.id)
+            if job.next_run_time:
+                pipe.zadd(self.run_times_key, datetime_to_utc_timestamp(job.next_run_time), job.id)
             pipe.execute()
 
     def update_job(self, job):
diff --git a/apscheduler/jobstores/sqlalchemy.py b/apscheduler/jobstores/sqlalchemy.py
index f8a3c15..5a4b7ef 100644
--- a/apscheduler/jobstores/sqlalchemy.py
+++ b/apscheduler/jobstores/sqlalchemy.py
@@ -70,7 +70,9 @@ class SQLAlchemyJobStore(BaseJobStore):
         return utc_timestamp_to_datetime(next_run_time)
 
     def get_all_jobs(self):
-        return self._get_jobs()
+        jobs = self._get_jobs()
+        self._fix_paused_jobs_sorting(jobs)
+        return jobs
 
     def add_job(self, job):
         insert = self.jobs_t.insert().values(**{
diff --git a/docs/versionhistory.rst b/docs/versionhistory.rst
index 819e329..b83dce9 100644
--- a/docs/versionhistory.rst
+++ b/docs/versionhistory.rst
@@ -10,6 +10,10 @@ APScheduler, see the :doc:`migration section <migration>`.
 
 * Fixed ValueError when the target callable has a default keyword argument that wasn't overridden
 
+* Fixed wrong job sort order in some job stores
+
+* Fixed exception when loading all jobs from the redis job store when there are paused jobs in it
+
 
 3.0.1
 -----
diff --git a/tests/test_jobstores.py b/tests/test_jobstores.py
index 58e8533..35879eb 100644
--- a/tests/test_jobstores.py
+++ b/tests/test_jobstores.py
@@ -75,10 +75,10 @@ def persistent_jobstore(request):
 
 @pytest.fixture
 def create_add_job(timezone, create_job):
-    def create(jobstore, func=dummy_job, run_date=datetime(2999, 1, 1), id=None, **kwargs):
+    def create(jobstore, func=dummy_job, run_date=datetime(2999, 1, 1), id=None, paused=False, **kwargs):
         run_date = timezone.localize(run_date)
         job = create_job(func=func, trigger='date', trigger_args={'run_date': run_date}, id=id, **kwargs)
-        job.next_run_time = job.trigger.get_next_fire_time(None, run_date)
+        job.next_run_time = None if paused else job.trigger.get_next_fire_time(None, run_date)
         if jobstore:
             jobstore.add_job(job)
         return job
@@ -99,14 +99,16 @@ def test_lookup_nonexistent_job(jobstore):
 def test_get_all_jobs(jobstore, create_add_job):
     job1 = create_add_job(jobstore, dummy_job, datetime(2016, 5, 3))
     job2 = create_add_job(jobstore, dummy_job2, datetime(2013, 8, 14))
+    job3 = create_add_job(jobstore, dummy_job2, datetime(2013, 7, 11), paused=True)
     jobs = jobstore.get_all_jobs()
-    assert jobs == [job2, job1]
+    assert jobs == [job2, job1, job3]
 
 
 def test_get_pending_jobs(jobstore, create_add_job, timezone):
     create_add_job(jobstore, dummy_job, datetime(2016, 5, 3))
     job2 = create_add_job(jobstore, dummy_job2, datetime(2014, 2, 26))
     job3 = create_add_job(jobstore, dummy_job3, datetime(2013, 8, 14))
+    create_add_job(jobstore, dummy_job3, datetime(2013, 7, 11), paused=True)
     jobs = jobstore.get_due_jobs(timezone.localize(datetime(2014, 2, 27)))
     assert jobs == [job3, job2]
 
@@ -126,6 +128,7 @@ def test_get_next_run_time(jobstore, create_add_job, timezone):
     create_add_job(jobstore, dummy_job, datetime(2016, 5, 3))
     create_add_job(jobstore, dummy_job2, datetime(2014, 2, 26))
     create_add_job(jobstore, dummy_job3, datetime(2013, 8, 14))
+    create_add_job(jobstore, dummy_job3, datetime(2013, 7, 11), paused=True)
     assert jobstore.get_next_run_time() == timezone.localize(datetime(2013, 8, 14))