Prevent multiple scan submission using transactions

packit · Nov 7, 2024 · 2066064 · 2066064
1 parent c772719
commit 2066064
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 49 deletions.
diff --git a/packit_service/models.py b/packit_service/models.py
@@ -23,7 +23,6 @@
 
 from cachetools import TTLCache, cached
 from cachetools.func import ttl_cache
-from packit.config import JobConfigTriggerType
 from sqlalchemy import (
     JSON,
     Boolean,
@@ -55,6 +54,7 @@
 from sqlalchemy.sql.functions import count
 from sqlalchemy.types import ARRAY
 
+from packit.config import JobConfigTriggerType
 from packit_service.constants import ALLOWLIST_CONSTANTS
 
 logger = logging.getLogger(__name__)
@@ -2329,6 +2329,44 @@ def add_scan(self, task_id: int) -> "OSHScanModel":
             session.add(scan)
             return scan
 
+    @contextmanager
+    def add_scan_transaction(self) -> Generator["OSHScanModel"]:
+        """
+        Context manager that creates a ScanModel upon entering the context,
+        provides a corresponding instance of `ScanModel` to be updated within the context
+        and commits the changes upon exiting the context, all within a single transaction.
+        """
+        session = singleton_session or Session()
+        session.begin()
+
+        submit_scan = False
+
+        try:
+            if not self.scan:
+                submit_scan = True
+                scan = OSHScanModel()
+                scan.copr_build_target = self
+                session.add(scan)
+        except Exception as ex:
+            logger.warning(f"Exception while working with database: {ex!r}")
+            session.rollback()
+            raise
+
+        try:
+            yield scan if submit_scan else None
+        except Exception:
+            session.rollback()
+            raise
+
+        try:
+            if submit_scan:
+                session.add(scan)
+                session.commit()
+        except Exception as ex:
+            logger.warning(f"Exception while working with database: {ex!r}")
+            session.rollback()
+            raise
+
 
 class KojiBuildGroupModel(ProjectAndEventsConnector, GroupModel, Base):
     __tablename__ = "koji_build_groups"

diff --git a/packit_service/worker/helpers/open_scan_hub.py b/packit_service/worker/helpers/open_scan_hub.py
@@ -15,13 +15,14 @@
     JobConfigTriggerType,
     JobType,
 )
-
+from packit.exceptions import PackitException
 from packit_service.constants import (
     OPEN_SCAN_HUB_FEATURE_DESCRIPTION,
 )
 from packit_service.models import (
     BuildStatus,
     CoprBuildTargetModel,
+    OSHScanStatus,
     SRPMBuildModel,
 )
 from packit_service.service.urls import get_copr_build_info_url
@@ -34,6 +35,10 @@
 logger = logging.getLogger(__name__)
 
 
+class OSHNoFeedback(PackitException):
+    pass
+
+
 class OpenScanHubHelper:
     def __init__(
         self,
@@ -86,60 +91,53 @@ def handle_scan(self):
 
             build_dashboard_url = get_copr_build_info_url(self.build.id)
 
-            output = self.copr_build_helper.api.run_osh_build(
-                srpm_path=paths[1],
-                base_srpm=paths[0],
-                comment=f"Submitted via Packit Service for {build_dashboard_url}",
-            )
+            try:
+                err_msg = "Scan in OpenScanHub was not submitted successfully."
+                with self.build.add_scan_transaction() as scan:
+                    output = self.copr_build_helper.api.run_osh_build(
+                        srpm_path=paths[1],
+                        base_srpm=paths[0],
+                        comment=f"Submitted via Packit Service for {build_dashboard_url}",
+                    )
 
-            if not output:
-                self.report(
-                    state=BaseCommitStatus.neutral,
-                    description="Scan in OpenScanHub was not submitted successfully.",
-                    url=None,
-                )
-                return
+                    if not output:
+                        raise OSHNoFeedback("Something went wrong, skipping the reporting.")
 
-            logger.info("Scan submitted successfully.")
+                    logger.info("Scan submitted successfully.")
 
-            response_dict = self.parse_dict_from_output(output)
+                    response_dict = self.parse_dict_from_output(output)
 
-            logger.debug(f"Parsed dict from output: {response_dict} ")
+                    logger.debug(f"Parsed dict from output: {response_dict} ")
 
-            scan = None
-            if id := response_dict.get("id"):
-                if self.build.scan:
-                    # see comment https://github.com/packit/packit-service/issues/2604#issuecomment-2444321483
-                    logger.debug(
-                        f"Scan for build {self.build.id} already submitted, "
-                        "scan task_id {self.build.scan.task_id}."
-                    )
-                    return
-                scan = self.build.add_scan(task_id=id)
-            else:
-                logger.debug(
-                    "It was not possible to get the Open Scan Hub task_id from the response.",
-                )
+                    if id := response_dict.get("id"):
+                        scan.task_id = id
+                        scan.status = OSHScanStatus.pending
+                    else:
+                        raise OSHNoFeedback(
+                            "It was not possible to get the Open Scan Hub task_id "
+                            "from the response.",
+                        )
+
+                    if not (url := response_dict.get("url")):
+                        err_msg = "It was not possible to get the task URL from the OSH response."
+                        raise OSHNoFeedback(err_msg)
+                    scan.url = url
 
-            if not (url := response_dict.get("url")):
-                msg = "It was not possible to get the task URL from the OSH response."
-                logger.debug(msg)
+                    self.report(
+                        state=BaseCommitStatus.running,
+                        description=(
+                            "Scan in OpenScanHub submitted successfully. "
+                            "Check the URL for more details."
+                        ),
+                        url=url,
+                    )
+            except OSHNoFeedback as ex:
+                logger.info(f"OpenScanHub feedback missing: {ex}")
                 self.report(
                     state=BaseCommitStatus.neutral,
-                    description=msg,
-                    url=None,
+                    description=err_msg,
+                    url=build_dashboard_url,
                 )
-                return
-            if url and scan:
-                scan.set_url(url)
-
-            self.report(
-                state=BaseCommitStatus.running,
-                description=(
-                    "Scan in OpenScanHub submitted successfully. Check the URL for more details."
-                ),
-                url=url,
-            )
 
     def report(
         self,

diff --git a/tests/unit/test_open_scan_hub.py b/tests/unit/test_open_scan_hub.py
@@ -7,6 +7,7 @@
 import pytest
 from celery.canvas import group as celery_group
 from flexmock import flexmock
+
 from packit.api import PackitAPI
 from packit.config import (
     CommonPackageConfig,
@@ -15,7 +16,6 @@
     JobType,
     PackageConfig,
 )
-
 from packit_service.models import (
     BuildStatus,
     CoprBuildTargetModel,
@@ -157,7 +157,11 @@ def test_handle_scan(build_models):
                 type=ProjectEventModelType.pull_request,
                 get_project_event_object=lambda: flexmock(),
             ),
-        ),
+        )
+        .should_receive("add_scan_transaction")
+        .once()
+        .and_return(flexmock())
+        .mock(),
         copr_build_helper=CoprBuildJobHelper(
             service_config=flexmock(),
             package_config=package_config,