Make Job.job a Property and Make Jobs Hashable (#3250)

telegram/ext/_jobqueue.py

@@ -205,7 +205,7 @@ class JobQueue:
             **job_kwargs,
         )
 
-        job.job = j
+        job._job = j  # pylint: disable=protected-access
         return job
 
     def run_repeating(
@@ -320,7 +320,7 @@ class JobQueue:
             **job_kwargs,
         )
 
-        job.job = j
+        job._job = j  # pylint: disable=protected-access
         return job
 
     def run_monthly(
@@ -398,7 +398,7 @@ class JobQueue:
             timezone=when.tzinfo or self.scheduler.timezone,
             **job_kwargs,
         )
-        job.job = j
+        job._job = j  # pylint: disable=protected-access
         return job
 
     def run_daily(
@@ -487,7 +487,7 @@ class JobQueue:
             **job_kwargs,
         )
 
-        job.job = j
+        job._job = j  # pylint: disable=protected-access
         return job
 
     def run_custom(
@@ -539,7 +539,7 @@ class JobQueue:
 
         j = self.scheduler.add_job(job.run, args=(self.application,), name=name, **job_kwargs)
 
-        job.job = j
+        job._job = j  # pylint: disable=protected-access
         return job
 
     async def start(self) -> None:
@@ -601,15 +601,18 @@ class Job:
     considered equal, if their :class:`id <apscheduler.job.Job>` is equal.
 
     Note:
-        * All attributes and instance methods of :attr:`job` are also directly available as
-          attributes/methods of the corresponding :class:`telegram.ext.Job` object.
-        * If :attr:`job` isn't passed on initialization, it must be set manually afterwards for
-          this :class:`telegram.ext.Job` to be useful.
+        All attributes and instance methods of :attr:`job` are also directly available as
+        attributes/methods of the corresponding :class:`telegram.ext.Job` object.
+
+    Warning:
+        This class should not be instantiated manually.
+        Use the methods of :class:`telegram.ext.JobQueue` to schedule jobs.
 
     .. versionchanged:: 20.0
 
        * Removed argument and attribute ``job_queue``.
        * Renamed ``Job.context`` to :attr:`Job.data`.
+       * Removed argument ``job``
 
     Args:
         callback (:term:`coroutine function`): The callback function that should be executed by the
@@ -621,7 +624,6 @@ class Job:
             accessed through :attr:`Job.data` in the callback. Defaults to :obj:`None`.
         name (:obj:`str`, optional): The name of the new job. Defaults to
             :external:obj:`callback.__name__ <definition.__name__>`.
-        job (:class:`apscheduler.job.Job`, optional): The APS Job this job is a wrapper for.
         chat_id (:obj:`int`, optional): Chat id of the chat that this job is associated with.
 
             .. versionadded:: 20.0
@@ -634,7 +636,6 @@ class Job:
             new job.
         data (:obj:`object`): Optional. Additional data needed for the callback function.
         name (:obj:`str`): Optional. The name of the new job.
-        job (:class:`apscheduler.job.Job`): Optional. The APS Job this job is a wrapper for.
         chat_id (:obj:`int`): Optional. Chat id of the chat that this job is associated with.
 
             .. versionadded:: 20.0
@@ -649,7 +650,7 @@ class Job:
         "name",
         "_removed",
         "_enabled",
-        "job",
+        "_job",
         "chat_id",
         "user_id",
     )
@@ -659,7 +660,6 @@ class Job:
         callback: JobCallback,
         data: object = None,
         name: str = None,
-        job: APSJob = None,
         chat_id: int = None,
         user_id: int = None,
     ):
@@ -673,7 +673,16 @@ class Job:
         self._removed = False
         self._enabled = False
 
-        self.job = cast(APSJob, job)  # skipcq: PTC-W0052
+        self._job = cast(APSJob, None)  # skipcq: PTC-W0052
+
+    @property
+    def job(self) -> APSJob:
+        """:class:`apscheduler.job.Job`: The APS Job this job is a wrapper for.
+
+        .. versionchanged:: 20.0
+            This property is now read-only.
+        """
+        return self._job
 
     async def run(self, application: "Application") -> None:
         """Executes the callback function independently of the jobs schedule. Also calls
@@ -758,3 +767,6 @@ class Job:
         if isinstance(other, self.__class__):
             return self.id == other.id
         return False
+
+    def __hash__(self) -> int:
+        return hash(self.id)

tests/test_jobqueue.py

@@ -45,7 +45,7 @@ async def job_queue(bot, app):
 
 
 @pytest.mark.skipif(
-    os.getenv("GITHUB_ACTIONS", False) and platform.system() in ["Windows", "Darwin"],
+    os.getenv("GITHUB_ACTIONS", False) and platform.system() in ["Darwin"],
     reason="On Windows & MacOS precise timings are not accurate.",
 )
 @flaky(10, 1)  # Timings aren't quite perfect
@@ -450,10 +450,24 @@ class TestJobQueue:
         await asyncio.sleep(0.5)
         assert self.result == 2
 
-    async def test_job_lt_eq(self, job_queue):
+    async def test_equality(self, job_queue):
         job = job_queue.run_repeating(self.job_run_once, 0.2)
-        assert not job == job_queue
+        job_2 = job_queue.run_repeating(self.job_run_once, 0.2)
+        job_3 = Job(self.job_run_once, 0.2)
+        job_3._job = job.job
+        assert job == job
+        assert job != job_queue
+        assert job != job_2
+        assert job == job_3
+
+        assert hash(job) == hash(job)
+        assert hash(job) != hash(job_queue)
+        assert hash(job) != hash(job_2)
+        assert hash(job) == hash(job_3)
+
         assert not job < job
+        assert not job < job_2
+        assert not job < job_3
 
     async def test_process_error_context(self, job_queue, app):
         app.add_error_handler(self.error_handler_context)