launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+491755@xxxxxxxxxxxxxxxxxx>]

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-import-export-endpoints into launchpad:master.

Commit message:
Fix VulnerabilityJob multiple jobs were created
    
The query to get existing jobs was failing when having 3 or more jobs
created. One() method was trying to return more than 1 result.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/491755
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-import-export-endpoints into launchpad:master.

diff --git a/lib/lp/bugs/model/importvulnerabilityjob.py b/lib/lp/bugs/model/importvulnerabilityjob.py
index 6c35191..4e31f11 100644
--- a/lib/lp/bugs/model/importvulnerabilityjob.py
+++ b/lib/lp/bugs/model/importvulnerabilityjob.py
@@ -104,12 +104,12 @@ class ImportVulnerabilityJob(VulnerabilityJobDerived):
             VulnerabilityJob.job_id == Job.id,
             VulnerabilityJob.job_type == cls.class_job_type,
             VulnerabilityJob.handler == handler,
+            Job._status.is_in(
+                (JobStatus.WAITING, JobStatus.RUNNING, JobStatus.SUSPENDED)
+            ),
         ).one()
 
-        if vulnerability_job is not None and (
-            vulnerability_job.job.status == JobStatus.WAITING
-            or vulnerability_job.job.status == JobStatus.RUNNING
-        ):
+        if vulnerability_job is not None:
             raise VulnerabilityJobInProgress(cls(vulnerability_job))
 
         # Schedule the initialization.
diff --git a/lib/lp/bugs/tests/test_importvulnerabilityjob.py b/lib/lp/bugs/tests/test_importvulnerabilityjob.py
index 51468e8..297f8d7 100644
--- a/lib/lp/bugs/tests/test_importvulnerabilityjob.py
+++ b/lib/lp/bugs/tests/test_importvulnerabilityjob.py
@@ -127,9 +127,10 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
         )
         self.assertEqual(expected, repr(job))
 
-    def test_create_with_existing_in_progress_job(self):
-        """If there's already a waiting/running ImportVulnerabilityJob for the
-        handler ImportVulnerabilityJob.create() raises an exception.
+    def test_create_with_existing_pending_job(self):
+        """If there's already a waiting/running/suspended
+        ImportVulnerabilityJob for the handler ImportVulnerabilityJob.create()
+        raises the VulnerabilityJobInProgress exception.
         """
         handler = VulnerabilityHandlerEnum.SOSS
         git_repository = self.repository.git_https_url
@@ -138,7 +139,7 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
         information_type = InformationType.PRIVATESECURITY.value
         import_since_commit_sha1 = None
 
-        # Job waiting status
+        # Job WAITING status
         job = self.job_source.create(
             handler,
             git_repository,
@@ -173,6 +174,20 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
         )
         self.assertEqual(job, running_exception.job)
 
+        # Job status from RUNNING to SUSPENDED
+        job.suspend()
+        running_exception = self.assertRaises(
+            VulnerabilityJobInProgress,
+            self.job_source.create,
+            handler,
+            git_repository,
+            git_ref,
+            git_paths,
+            information_type,
+            import_since_commit_sha1,
+        )
+        self.assertEqual(job, running_exception.job)
+
     def test_create_with_existing_completed_job(self):
         """If there's already a completed ImportVulnerabilityJob for the
         handler the job can be run again.
@@ -208,6 +223,20 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
         job_duplicated.complete()
         self.assertEqual(job_duplicated.status, JobStatus.COMPLETED)
 
+        # There's a 3rd run to check that the query returns only pending jobs,
+        # and one() doesn't raise an exception
+        job_duplicated = self.job_source.create(
+            handler,
+            git_repository,
+            git_ref,
+            git_paths,
+            information_type,
+            import_since_commit_sha1,
+        )
+        job_duplicated.start()
+        job_duplicated.complete()
+        self.assertEqual(job_duplicated.status, JobStatus.COMPLETED)
+
     def test_create_with_existing_failed_job(self):
         """If there's a failed ImportVulnerabilityJob for the handler the job
         can be run again.