Defer replacing undefined options with defaults until the job is really scheduled

Allow specifying an explicit next_run_time when adding a job

8 files changed, 59 insertions, 32 deletions

diff --git a/apscheduler/job.py b/apscheduler/job.py
index 33de6ef..bb9f86f 100644
--- a/apscheduler/job.py
+++ b/apscheduler/job.py
@@ -33,7 +33,6 @@ class Job(object):
         super(Job, self).__init__()
         self._scheduler = scheduler
         self._jobstore_alias = None
-        kwargs.setdefault('next_run_time', None)
         self._modify(id=id or uuid4().hex, **kwargs)
 
     def modify(self, **changes):
diff --git a/apscheduler/schedulers/base.py b/apscheduler/schedulers/base.py
index f4d8a14..dbb22b6 100644
--- a/apscheduler/schedulers/base.py
+++ b/apscheduler/schedulers/base.py
@@ -285,16 +285,17 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
                     del self._listeners[i]
 
     def add_job(self, func, trigger=None, args=None, kwargs=None, id=None, name=None, misfire_grace_time=undefined,
-                coalesce=undefined, max_instances=undefined, jobstore='default', executor='default',
-                replace_existing=False, **trigger_args):
+                coalesce=undefined, max_instances=undefined, next_run_time=undefined, jobstore='default',
+                executor='default', replace_existing=False, **trigger_args):
         """
         add_job(func, trigger=None, args=None, kwargs=None, id=None, name=None, misfire_grace_time=undefined, \
-            coalesce=undefined, max_instances=undefined, jobstore='default', executor='default', \
-            replace_existing=False, **trigger_args)
+            coalesce=undefined, max_instances=undefined, next_run_time=undefined, jobstore='default', \
+            executor='default', replace_existing=False, **trigger_args)
 
         Adds the given job to the job list and wakes up the scheduler if it's already running.
 
-        Any argument that defaults to ``undefined`` will use scheduler defaults if no value is provided.
+        Any option that defaults to ``undefined`` will be replaced with the corresponding default value when the job is
+        scheduled (which happens when the scheduler is started, or immediately if the scheduler is already running).
 
         The ``func`` argument can be given either as a callable object or a textual reference in the
         ``package.module:some.object`` format, where the first half (separated by ``:``) is an importable module and the
@@ -315,6 +316,8 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
         :param bool coalesce: run once instead of many times if the scheduler determines that the job should be run more
                               than once in succession
         :param int max_instances: maximum number of concurrently running instances allowed for this job
+        :param datetime next_run_time: when to first run the job, regardless of the trigger (pass ``None`` to add the
+                                       job as paused)
         :param str|unicode jobstore: alias of the job store to store the job in
         :param str|unicode executor: alias of the executor to run the job with
         :param bool replace_existing: ``True`` to replace an existing job with the same ``id`` (but retain the
@@ -322,8 +325,6 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
         :rtype: Job
         """
 
-        # Assemble the final job arguments, substituting with default values where no other value is provided
-        default_replace = lambda key, value: value if value is not undefined else self._job_defaults.get(key)
         job_kwargs = {
             'trigger': self._create_trigger(trigger, trigger_args),
             'executor': executor,
@@ -332,10 +333,12 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
             'kwargs': dict(kwargs) if kwargs is not None else {},
             'id': id,
             'name': name,
-            'misfire_grace_time': default_replace('misfire_grace_time', misfire_grace_time),
-            'coalesce': default_replace('coalesce', coalesce),
-            'max_instances': default_replace('max_instances', max_instances),
+            'misfire_grace_time': misfire_grace_time,
+            'coalesce': coalesce,
+            'max_instances': max_instances,
+            'next_run_time': next_run_time
         }
+        job_kwargs = dict((key, value) for key, value in six.iteritems(job_kwargs) if value is not undefined)
         job = Job(self, **job_kwargs)
 
         # Don't really add jobs to job stores before the scheduler is up and running
@@ -349,11 +352,12 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
         return job
 
     def scheduled_job(self, trigger, args=None, kwargs=None, id=None, name=None, misfire_grace_time=undefined,
-                      coalesce=undefined, max_instances=undefined, jobstore='default', executor='default',
-                      **trigger_args):
+                      coalesce=undefined, max_instances=undefined, next_run_time=undefined, jobstore='default',
+                      executor='default', **trigger_args):
         """
         scheduled_job(trigger, args=None, kwargs=None, id=None, name=None, misfire_grace_time=undefined, \
-            coalesce=undefined, max_instances=undefined, jobstore='default', executor='default',**trigger_args)
+            coalesce=undefined, max_instances=undefined, next_run_time=undefined, jobstore='default', \
+            executor='default',**trigger_args)
 
         A decorator version of :meth:`add_job`, except that ``replace_existing`` is always ``True``.
 
@@ -362,8 +366,8 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
         """
 
         def inner(func):
-            self.add_job(func, trigger, args, kwargs, id, name, misfire_grace_time, coalesce, max_instances, jobstore,
-                         executor, True, **trigger_args)
+            self.add_job(func, trigger, args, kwargs, id, name, misfire_grace_time, coalesce, max_instances,
+                         next_run_time, jobstore, executor, True, **trigger_args)
             return func
         return inner
 
@@ -705,9 +709,19 @@ class BaseScheduler(six.with_metaclass(ABCMeta)):
         :param bool wakeup: ``True`` to wake up the scheduler after adding the job
         """
 
-        # Calculate the next run time
-        now = datetime.now(self.timezone)
-        job.next_run_time = job.trigger.get_next_fire_time(None, now)
+        # Fill in undefined values with defaults
+        replacements = {}
+        for key, value in six.iteritems(self._job_defaults):
+            if not hasattr(job, key):
+                replacements[key] = value
+
+        # Calculate the next run time if there is none defined
+        if not hasattr(job, 'next_run_time'):
+            now = datetime.now(self.timezone)
+            replacements['next_run_time'] = job.trigger.get_next_fire_time(None, now)
+
+        # Apply any replacements
+        job._modify(**replacements)
 
         # Add the job to the given job store
         store = self._lookup_jobstore(jobstore_alias)
diff --git a/apscheduler/util.py b/apscheduler/util.py
index a530de7..1ef8646 100644
--- a/apscheduler/util.py
+++ b/apscheduler/util.py
@@ -21,7 +21,18 @@ __all__ = ('asint', 'asbool', 'astimezone', 'convert_to_datetime', 'datetime_to_
            'utc_timestamp_to_datetime', 'timedelta_seconds', 'datetime_ceil', 'get_callable_name', 'obj_to_ref',
            'ref_to_obj', 'maybe_ref', 'repr_escape', 'check_callable_args')
 
-undefined = object()  #: a unique object that only signifies that no value is defined
+
+class _Undefined(object):
+    def __nonzero__(self):
+        return False
+
+    def __bool__(self):
+        return False
+
+    def __repr__(self):
+        return '<undefined>'
+
+undefined = _Undefined()  #: a unique object that only signifies that no value is defined
 
 
 def asint(text):
diff --git a/docs/userguide.rst b/docs/userguide.rst
index 009babb..7ce6c99 100644
--- a/docs/userguide.rst
+++ b/docs/userguide.rst
@@ -213,9 +213,6 @@ Method 3::
 
     scheduler.configure(jobstores=jobstores, executors=executors, job_defaults=job_defaults, timezone=utc)
 
-.. note:: If you add jobs before configuring the scheduler, any undefined values will be filled with the hardcoded
-          defaults.
-
 
 Starting the scheduler
 ----------------------
diff --git a/tests/conftest.py b/tests/conftest.py
index 4dd00cf..669ddc7 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -86,5 +86,6 @@ def create_job(job_defaults):
         job_kwargs.update(kwargs)
         job_kwargs['trigger'] = BlockingScheduler()._create_trigger(job_kwargs.pop('trigger'),
                                                                     job_kwargs.pop('trigger_args'))
+        job_kwargs.setdefault('next_run_time', None)
         return Job(**job_kwargs)
     return create
diff --git a/tests/test_executors.py b/tests/test_executors.py
index 207a728..cd686f6 100644
--- a/tests/test_executors.py
+++ b/tests/test_executors.py
@@ -57,7 +57,7 @@ def test_max_instances(mock_scheduler, executor, create_job, freeze_time):
 
     events = []
     mock_scheduler._dispatch_event = lambda event: events.append(event)
-    job = create_job(func=wait_event, max_instances=2)
+    job = create_job(func=wait_event, max_instances=2, next_run_time=None)
     executor.submit_job(job, [freeze_time.current])
     executor.submit_job(job, [freeze_time.current])
 
diff --git a/tests/test_job.py b/tests/test_job.py
index 9457269..62b615f 100644
--- a/tests/test_job.py
+++ b/tests/test_job.py
@@ -33,8 +33,6 @@ def test_constructor(job_id):
         assert job._jobstore_alias is None
 
         modify_kwargs = _modify.call_args[1]
-        assert modify_kwargs['next_run_time'] is None
-
         if job_id is None:
             assert len(modify_kwargs['id']) == 32
         else:
diff --git a/tests/test_schedulers.py b/tests/test_schedulers.py
index 87c97c2..2149606 100644
--- a/tests/test_schedulers.py
+++ b/tests/test_schedulers.py
@@ -344,14 +344,13 @@ class TestBaseScheduler(object):
         func = lambda x, y: None
         scheduler._stopped = stopped
         scheduler._real_add_job = MagicMock()
-        scheduler._job_defaults = {'misfire_grace_time': 3, 'coalesce': False, 'max_instances': 6}
         job = scheduler.add_job(func, 'date', [1], {'y': 2}, 'my-id', 'dummy', run_date='2014-06-01 08:41:00')
 
         assert isinstance(job, Job)
         assert job.id == 'my-id'
-        assert job.misfire_grace_time == 3
-        assert job.coalesce is False
-        assert job.max_instances == 6
+        assert not hasattr(job, 'misfire_grace_time')
+        assert not hasattr(job, 'coalesce')
+        assert not hasattr(job, 'max_instances')
         assert len(scheduler._pending_jobs) == (1 if stopped else 0)
         assert scheduler._real_add_job.call_count == (0 if stopped else 1)
 
@@ -362,7 +361,8 @@ class TestBaseScheduler(object):
         decorator(func)
 
         scheduler.add_job.assert_called_once_with(func, 'date', [1], {'y': 2}, 'my-id', 'dummy', undefined, undefined,
-                                                  undefined, 'default', 'default', True, run_date='2014-06-01 08:41:00')
+                                                  undefined, undefined, 'default', 'default', True,
+                                                  run_date='2014-06-01 08:41:00')
 
     @pytest.mark.parametrize('pending', [True, False], ids=['pending job', 'scheduled job'])
     def test_modify_job(self, scheduler, pending):
@@ -664,10 +664,11 @@ Jobstore baz:
     @pytest.mark.parametrize('replace_existing', [True, False], ids=['replace', 'no replace'])
     @pytest.mark.parametrize('wakeup', [True, False], ids=['wakeup', 'no wakeup'])
     def test_real_add_job(self, scheduler, job_exists, replace_existing, wakeup):
-        job = MagicMock(Job, id='foo')
+        job = Job(scheduler, id='foo', func=lambda: None, args=(), kwargs={}, next_run_time=None)
         jobstore = MagicMock(BaseJobStore, _alias='bar',
                              add_job=MagicMock(side_effect=ConflictingIdError('foo') if job_exists else None))
         scheduler.wakeup = MagicMock()
+        scheduler._job_defaults = {'misfire_grace_time': 3, 'coalesce': False, 'max_instances': 6}
         scheduler._dispatch_event = MagicMock()
         scheduler._jobstores = {'bar': jobstore}
 
@@ -678,6 +679,12 @@ Jobstore baz:
 
         scheduler._real_add_job(job, 'bar', replace_existing, wakeup)
 
+        # Check that the undefined values were replaced with scheduler defaults
+        assert job.misfire_grace_time == 3
+        assert job.coalesce is False
+        assert job.max_instances == 6
+        assert job.next_run_time is None
+
         if job_exists:
             jobstore.update_job.assert_called_once_with(job)
         else: